feat(http): bind response cookie attributes to result fields

[disintegrator]

Add CookieAttributes(name, fn) DSL and the per-cookie binders CookieMaxAgeFrom, CookieDomainFrom, CookiePathFrom, CookieSecureFrom, CookieHTTPOnlyFrom, CookieSameSiteFrom. Each binder takes a result-type attribute name; the server populates the corresponding http.Cookie field from the bound result attribute when emitting the response, and the generated client decoder writes the matching *http.Cookie field back into the same result attribute.

The existing literal setters (CookieMaxAge, CookieDomain, CookiePath, CookieSecure, CookieHTTPOnly, CookieSameSite) remain unchanged and write response-global metadata as before. Bindings are stored as per-cookie metadata (cookie::from) on the cookie attribute and take precedence over the response-global literals on a per-cookie basis. Cookies without bindings are unaffected.

Validation rejects bindings to attributes that do not exist on the result type or whose primitive kind does not match the cookie attribute (Int* for Max-Age, String for Domain/Path/SameSite, Boolean for Secure/HttpOnly).

Example - pure bindings:

    Method("login", func() {
        Result(LoginResult)
        HTTP(func() {
            POST("/login")
            Response(StatusOK, func() {
                Cookie("sessionID:SID", String)
                CookieAttributes("sessionID", func() {
                    MaxAgeFrom("expiresIn")
                    DomainFrom("cookieDomain")
                    SecureFrom("isSecure")
                    SameSiteFrom("sameSite")
                })
            })
        })
    })

Example - mixing bindings with the existing literal setters. The session cookie's Max-Age comes from a per-user "expiresIn" result attribute, while the CSRF cookie keeps the fixed literal Max-Age. Domain, Secure and HttpOnly are shared by both cookies via the existing response-wide setters:

    Method("login", func() {
        Result(LoginResult) // sessionID, csrfToken, expiresIn
        HTTP(func() {
            POST("/login")
            Response(StatusOK, func() {
                Cookie("sessionID:SID", String)
                Cookie("csrfToken:CSRF", String)
                CookieAttributes("sessionID", func() {
                    MaxAgeFrom("expiresIn") // overrides 3600 below
                })
                CookieMaxAge(3600)         // applies to CSRF
                CookieDomain("goa.design") // applies to both
                CookieSecure()             // applies to both
                CookieHTTPOnly()           // applies to both
            })
        })
    })

Coding agent disclosure

I used Claude Opus 4.7 to plan and implement this feature. I have reviewed the code it produced as well as driving it to produce some more interesting test cases.

Manual verification process

In my project, I am building an OAuth flow using Goa DSL where:

• Client starts at /admin/auth.login which redirects to external provider with a state query param and sets the same state as a cookie.
• On OAuth consent, external provider redirects to /admin/auth.callback?code=...&state=...
• In the service method that handles this callback I read back the state query param and the cookie and then compare them to ensure the same browser session crafted the state param.
• When successful, I delete the state cookie by setting max age to -1 and redirect client back to application.

I was able to confirm this flow can be built with this new DSL. In particular, during local dev, some of our developers run the service behind HTTPS and others don't. Previously in Go it was not easily possible toggle a cookie's Secure directive based on the local dev setup and, with CookieAttributes + SecureForm, it's now very easy.

[raphael]

Thanks for putting this together. I like the direction, especially keeping the existing literal cookie setters intact and making the dynamic behavior per-cookie. I noticed a few behavioral issues that I think need tightening before this lands:

• CookieSameSiteFrom appears to use a different value contract than the existing DSL constants. The generated server code switches on "Strict", "Lax", and "None", but the public constants are lower-case strings ("strict", "lax", "none", "default"). A service returning string(CookieSameSiteStrict) would currently fall through to SameSiteDefaultMode, silently weakening the cookie policy.

• Optional bound attributes lose presence semantics on the generated client. For pointer fields, the decoder always assigns a pointer to the zero value read from http.Cookie (&0, &"", &false) even when the cookie attribute was omitted by the server. That makes it impossible for clients to distinguish “not present” from an explicit zero value.

• The bound cookie-control fields still seem to be included in the default response body. Today buildHTTPResponseBody removes the cookie value attributes from the result body via resp.Cookies, but this PR’s extra bound fields (expiresIn, cookieDomain, isHTTPOnly, etc.) are only metadata on the cookie attribute. In the no-explicit-Body(...) examples, those fields would be serialized in JSON and also used to set cookie attributes.

• Error responses look only partially wired. The shared single-response decoder extracts the bound cookie fields for both success and error responses, but response_decoder.go.tpl only copies those values back onto success result objects. A CookieAttributes binding on an error response would decode the values and then drop them when constructing the error result.

• I'm wondering if we could/should remove the Cookie prefix from the DSL functions that live within CookieAttributes - feels like it stutters a bit as is. The benefit of the current approach is that it mirrors nicely the existing cookie DSL but that DSL being top level needed the prefix.

I think adding focused golden/tests for lower-case SameSite values, omitted optional bound fields, default body computation, and an error-response binding would make the intended contracts much clearer.

[disintegrator]

Thanks @raphael! I think I've addressed all the review points. I've also included the output from a local /review with Claude Opus 4.7 below. I don't think any of its remarks are actionable but happy to work through anything you find concerning from below:

Opus 4.7 /review

Reviewing the local branch state (8 commits, +1779 LOC across 30 files), since the remote PR currently shows only the original commit and the local branch carries six follow-on fixes/refactors that should ship together.

Overview

Adds a CookieAttributes(name, fn) DSL with six per-cookie binders (MaxAgeFrom, DomainFrom, PathFrom, SecureFrom, HTTPOnlyFrom, SameSiteFrom) so a service method can compute Max-Age / Domain / Path / Secure / HttpOnly / SameSite at runtime from result attributes. Bindings are stored as per-cookie cookie:<kind>:from metadata on the cookie's *expr.AttributeExpr, validated against the result/error type, and emitted symmetrically by the server response writer and the client decoder. The follow-on commits address: error-result decoder dropping bound values, default response body leaking bound source attributes, optional bound fields losing presence on decode, the lower-case canonical SameSite value contract, and an extracted set of testable runtime helpers.

Code quality and style

Strengths

• DSL surface is additive and backwards-compatible: existing literal CookieMaxAge/CookieDomain/etc. are untouched, and CookieAttributes opens a transient eval context (cookieAttrBindingsExpr) so the binders can't be invoked outside it (eval.IncompatibleDSL() guard).
• Metadata storage is per-cookie (cattr.Meta["cookie:<kind>:from"]), distinct from the response-global cookie:<kind> keys used by the literals — no aliasing risk.
• The duplicated copy-back in the response decoder was correctly extracted into partial_cookie_attr_bindings, parameterised on the target local var name. Used three times (success path, viewed-result path, both error branches) without further duplication.
• CookieAttrBinding carries FieldName/FieldPointer/Type/TypeRef/VarName populated from svcAtt.Find + codegen.GoifyAtt + scope.GoTypeRef + IsPrimitivePointer — same idiom as the existing header binding extraction.
• The presence-fix refactor (http/cookie.go) moves the contract into testable helpers and shrinks the partial from 4 gate shapes inlined to 6 one-line helper calls. CookieIntAttr is generic over ~int | ~int32 | ~int64 | ~uint | ~uint32 | ~uint64 and uses ~ correctly (matches type aliases). This is the codebase's first use of generics — worth flagging in the PR description but defensible and contained.
• validateCookieAttrBindings returns nil on the empty-meta fast path (post-fix), matching http_sse.go convention.
• Validation runs against both result and error types via HTTPErrorExpr.Validate, with diagnostic noun parameterised ("result type" vs "error type").
• removeCookieAttrBindings correctly applied to both the top-level body and each computed view.

Concerns

1. CookieData struct is now 12 fields wide (6 literals + 6 bindings). Not a blocker, but worth a doc comment noting that bindings take precedence per-property over the response-global literals — that contract is asserted in commit messages but isn't on the type itself.
2. removeCookieAttrBindings allocates a 6-slot bindingKeys slice on every call. Could be a package-level var to avoid the allocation in tight codegen loops. Micro, probably noise.
3. cattr shadowing in extractCookies: the closure now captures cattr (was _); fine, but the body checks cattr != nil && expr.IsObject(svcAtt.Type) even though WalkMappedAttr's callback always passes a non-nil attribute. The cattr != nil guard is defensive but not load-bearing.
4. response.go.tpl has parallel pointer/value branches for every property (≈90 lines added). Repetitive but unavoidable given the template language. A future cleanup could extract a sub-partial keyed on the property; not urgent.
5. SameSiteFrom's value contract is documented as "must hold one of CookieSameSiteStrict/Lax/None/Default" but the runtime path silently falls through to SameSiteDefaultMode for unknown values. Consider documenting the silent-default behaviour — or better, validating at the binding site (the runtime values aren't enum-validated).

Specific suggestions

• CookieData.MaxAgeFrom / etc. — add a one-liner GoDoc clarifying precedence over MaxAge literal, and note that exactly one of the two can be active per cookie attribute on the wire. (Georges: addressed)
• TestHTTPResponseBodyExcludesCookieAttrBindings (expr/http_cookie_test.go:97) declares cookieValue := "cookie" then loops the body twice — could collapse the two loops into one with both checks.
• cookie_attr_bindings.go.tpl — six near-identical 6-line blocks. Worth a comment at the top pointing at http/cookie.go for the runtime contract; otherwise readers have to grep across files.
• Add a brief release-note / migration entry: existing fixtures with the new DSL had their default response bodies emptied (the cookie-attr-bindings-{,optional,mixed} goldens regressed to empty body). This is correct, but downstream services that previously generated a body containing those source attributes will silently produce an empty body. Worth flagging in the PR description.

Potential issues / risks

1. Force-push required. The remote PR has the original (pre-rename) commit; pushing this branch needs --force-with-lease. Reviewers who pulled the original Cookie<X>From names will see them disappear. (Georges: Invalid remark from Claude)
2. Presence semantics conflate "explicit zero" with "absent". Documented in CookieAttributes GoDoc, but worth surfacing in release notes — services that genuinely want to send MaxAge=0 (delete cookie) bound through MaxAgeFrom to a *int cannot distinguish that from "no Max-Age" on the receiving client.
3. Server-side error encoder is not directly tested — the existing TestEncode reads sections[1] (success encoder); the error encoder lives in a separate section served by partial_response, which already handles bindings. Coverage is transitive via the server-encode goldens for the success path; no direct error-encoder golden. Minor gap.
4. HTTPErrorExpr.Validate only validates cookie bindings, not the cookie value attributes themselves. Pre-existing gap (the error response cookie validation was already missing the success path's Cookies.Validate + per-attribute primitive check); this PR doesn't widen the gap, but doesn't fix it either. Out-of-scope flag worth leaving in a follow-up issue.

Test coverage

Scenario	Coverage
DSL meta propagation (success)	TestHTTPResponseCookieAttrBindings (positive) + 3 negative cases
DSL meta propagation (error)	TestHTTPErrorCookieAttrBindings + 2 negative cases
Default body excludes bound attrs	TestHTTPResponseBodyExcludesCookieAttrBindings (direct) + 4 goldens
Decoder shape — required bindings	cookie-attr-bindings + -mixed + -body goldens
Decoder shape — optional bindings (all 4 gate shapes)	cookie-attr-bindings-optional-all golden
Error-response decoder copy-back	cookie-attr-bindings-error goldens
Presence helpers (behavioural)	http/cookie_test.go — table-driven, covers each helper × {zero, non-zero, edge}
SameSite lower-case constants	TestCookieSameSiteConstantsAreLowercase

Coverage is strong. The presence-helper extraction was specifically motivated by closing a structural-only gap and the table-driven helper tests now lock the contract independently of golden output.

Security considerations

• SameSiteFrom falling through default for unknown values silently downgrades to SameSiteDefaultMode. With the lowercase fix in place, this only triggers if a service explicitly returns a string outside {strict,lax,none,default,""}. Worth either: (a) validating runtime values via an enum on the bound attribute in the design (Enum(...)), or (b) documenting the silent-default behaviour as a known sharp edge.
• Bindings can overwrite response-global literals on a per-cookie basis. A binding to a missing result attribute is stripped silently in extractCookies (the if battr == nil { continue } guard). Validation now catches this at design time, but if validation is bypassed (e.g., the user uses an internal API to construct expressions), the cookie is emitted without the literal either. Defense in depth would be: emit a code-generation warning. Non-blocking.

Bottom line

The original feature plus the five fixes have converged on a tight, well-tested design. The presence-helpers refactor in particular is a meaningful cleanup that locks the runtime contract independently of the codegen pipeline.