refactor(service): extract internal/service layer; migrate SignUp (Phase 1)

[lakhansamani]

Stacked on top of #614. Review and merge after Phase 0 is in.

Summary

• Introduces internal/service as the home for transport-agnostic public-API operations. Each method takes RequestMetadata (host, IP, UA, raw request) and returns a typed response plus a ResponseSideEffects bag the transport applies (today: cookies; later: redirects, trailing headers).
• SignUp is the first migrated operation — chosen as the seam-proof because it exercises every gin coupling the legacy resolvers had: `GetHost`, `GetIP`/`GetUserAgent`, token issuance, session + mfa cookie writes, verification email/SMS dispatch.
• The GraphQL resolver in `internal/graphql/signup.go` becomes a thin adapter: build `RequestMetadata` from `gin.Context`, call the service, apply side-effects. This is the seam Phase 2's gRPC and grpc-gateway REST handlers will reuse.

Supporting cleanups (all transport-agnostic mirrors of existing helpers)

• `parsers.GetHostFromRequest` / `GetAppURLFromRequest` (raw `*http.Request`); gin wrappers now delegate.
• `cookie.BuildSessionCookies` / `BuildMfaSessionCookies` (return `[]*http.Cookie`); gin wrappers now delegate.

Wiring

• `service.Provider` constructed in `cmd/root.go` and `integration_tests/test_helper.go`.
• `graphql.Dependencies` and `http_handlers.Dependencies` each grow a `ServiceProvider` field.

Known follow-up

`TokenProvider.CreateAuthToken` still takes `*gin.Context` even though it doesn't read from it. The service synthesizes a minimal shim (`&gin.Context{Request: meta.Request}`) at the call site, with a TODO comment. Refactoring `CreateAuthToken` to take `*http.Request` directly is a Phase 2 cleanup (~10 callers).

Test plan

• All 11 `TestSignup` sub-cases pass
• Full SQLite integration suite (71s) still green — no behaviour drift
• `go build ./...` succeeds
• `go vet ./...` clean (one pre-existing mongodb warning unchanged)
• CI green on this stacked PR

Follow-ups

• Phase 2 — proto for 9 public services + gRPC server + grpc-gateway REST (will migrate remaining 17 public ops into `internal/service` along the way).
• Phase 4 — MCP server reading proto annotations.

🤖 Generated with Claude Code

[lakhansamani]

Principal-engineer review — Phase 1 (refactor(service): extract internal/service)

Reviewed against feat/proto-skeleton. Diff is ~1.2k lines, mostly the SignUp move and the gin/http helper splits.

Strengths

• Seam shape is solid. RequestMetadata + ResponseSideEffects + MetaFromGin / ApplyToGin is a clean transport adapter. The Build*Cookies / GetHostFromRequest mirrors are the right way to split — old gin wrappers are now thin shims, so call sites pay zero cost.
• Cookie semantics are preserved exactly. Both _session and _session_domain are emitted with the same Name/MaxAge/Path/Domain/Secure/HttpOnly/SameSite shape; the "." + domain rule (skipped for localhost) is intact in BuildSessionCookies. SameSite policy for MFA (Lax-when-insecure / None-when-secure) is preserved verbatim.
• ApplyToGin is per-cookie SetSameSite-then-SetCookie — this is actually more correct than the old code, because future ops can return cookies with mixed SameSite without stomping each other.
• Audit/IP/UA preserved. meta.IPAddress / meta.UserAgent captured by value before the goroutine — same behavior as the inline utils.GetIP(gc.Request) call.
• Embedding pattern (*config.Config + Dependencies) matches graphqlProvider / httpProvider exactly. Idiomatic for this codebase, no surprises.
• gcShim claim is correct. Verified auth_token.go: CreateAuthToken(gc, cfg) only forwards to CreateSessionToken/CreateAccessToken/CreateIDToken — none touch gc. The shim is safe today; the TODO is appropriate.

Issues

Important

1. service.Meta is dead code in this PR. Provider interface declares Meta(ctx, meta) (*model.Meta, *ResponseSideEffects, error) and meta.go implements it, but nothing calls it — the GraphQL Meta() resolver still has the original signature Meta(ctx context.Context) (*model.Meta, error) and was not migrated to delegate. Either (a) migrate the resolver in this PR for consistency with SignUp, or (b) drop Meta from the interface and the file, defer to a later phase. As-is it's silent scope creep — the PR title/body promises only SignUp.

2. No service-layer-direct tests. The whole point of extracting a transport-agnostic layer is to make it testable without spinning up gin. TestSignup still exercises the resolver path — fine as a regression net, but the seam itself is unverified. Bare minimum I'd want before Phase 2:

   • service/signup_test.go calling provider.SignUp directly with a hand-built RequestMetadata and stub providers (at least the happy email-verification + mobile-OTP + auth-token branches).
   • cookie/cookie_test.go for BuildSessionCookies and BuildMfaSessionCookies — these are now load-bearing helpers, and the "." + domain/localhost branch is exactly the kind of thing a copy-paste in Phase 2 will get wrong.
   • The new TestGetHostFromRequest / TestGetAppURLFromRequest are good, keep doing that.

3. RequestMetadata.Request escape hatch will outlive its TODO. Documented as a "legacy" field for TokenProvider, but Phase 2 will introduce gRPC handlers that won't have a real *http.Request (or will need to synthesize one with empty headers). The gcShim trick works for CreateAuthToken because that call doesn't read it, but the moment someone calls GetAccessToken(gc) from the service layer, gRPC breaks silently. Worth either (a) refactoring CreateAuthToken in this PR to take *http.Request (the TODO promises a "Phase 2 cleanup, ~10 callers" — do it before gRPC lands, not after), or (b) explicit panic/log in the shim path when meta.Request == nil so gRPC fails loud instead of NPE.

Nits

4. ResponseSideEffects.AddCookie doc says "safe on a zero-value receiver" — true for a non-nil *ResponseSideEffects pointing to a zero struct, but a nil pointer panics. Reword to "safe on a zero-value struct".
5. Email-verification branch returns side, nil where side is empty — fine, but inconsistent with the early validation returns of nil, nil, err. Pick one style.
6. service.New returns (Provider, error) but never errors. Matches the codebase pattern, so OK — but the TODO // TODO - Add any validation here should either be filled in (validate non-nil deps + non-nil config) or removed.
7. MetaFromGin returns zero RequestMetadata{} on nil gc/Request. Defensive but downstream code will silently produce nonsense (empty hostname, no audit IP/UA). Probably better to make it a programming error — caller should never pass nil.
8. gcShim literal &gin.Context{Request: meta.Request} creates a gin.Context with no engine, Keys, or middleware. If any helper down-the-stack calls gc.GetString/gc.Get it will return zero values silently. Document this constraint on the shim's TODO line, or wrap in a helper newSyntheticGin(r *http.Request) that future readers can grep for.

Test coverage gaps (recap)

• internal/service/signup_test.go — direct, no gin
• internal/cookie/cookie_test.go — BuildSessionCookies / BuildMfaSessionCookies (domain edge cases: localhost, single-label, IPv6, ports)
• internal/service/sideeffects_test.go — MetaFromGin with realistic gin.Context, ApplyToGin with mixed-SameSite cookies, nil-safety of both
• A regression test that proves Set-Cookie headers on the SignUp HTTP response are byte-identical pre/post-refactor — easy to assert against httptest recorder, and it's the one thing the existing test suite doesn't cover.

Verdict

Mergeable after addressing (1) and a sentence-level decision on (3). (2) ideally lands in this PR but I'd accept a follow-up issue tracking it before Phase 2 merges — these tests get harder to write once gRPC is layered on top. The refactor itself is faithful and the seam is well-chosen; SignUp was the right pick to prove it.

[lakhansamani]

Superseded by #620, which consolidates this stack into a single PR against main. All blocking review findings from this PR were addressed in #620; see its body for the per-finding traceback.