feat(client): validate API responses at the remaining client boundaries (#479, #451)

[thomasluizon]

What

Extends the #478 trust-boundary validation seam to the two client ingestion boundaries it deliberately left open, so mistyped API data can no longer flow inward unchecked (rule 8).

Web client (Closes #451)

apps/web/lib/api-fetch.ts — apiFetch/fetchJson gain an optional Zod schema param. On a successful response the body is safeParsed and a typed ApiClientError(502, INVALID_RESPONSE_SCHEMA) is thrown on mismatch; without a schema the body is returned as-is (opt-in). Threaded through the high-value reads: gamification profile, streak info, and notifications.

Mobile offline queue (Closes #479)

Mobile mutations are offline-queue-coupled: a queued mutation is a persisted plain object that can't carry a runtime schema function. A new MutationType → ZodType registry (apps/mobile/lib/mutation-response-schemas.ts) is consulted by both paths:

• the immediate execute path (performQueuedApiMutation), and
• the deferred flush replay (processQueuedMutationFlush),

so a queued mutation's response is validated identically whether it runs online now or on flush later. Registered logHabit + the four bulk habit mutations (bulkCreate/bulkDelete/bulkLog/bulkSkip) — each schema verified field-by-field against its orbit-api DTO. Types absent from the registry are not validated (opt-in), so all other callers are behavior-unchanged.

Shared consolidation

The safeParse → typed-throw logic from #478's two copies (server-fetch + mobile api-client) plus the two new seams was extracted into one validateApiResponse helper in @orbit/shared/utils — a single boundary-validation function, no cross-app duplication. Schemas tolerate additive API fields (Zod strips unknown keys rather than rejecting) per the append-only contract.

Tests

On both the web client seam and the mobile registry boundary:

• a valid response passes,
• a contract mismatch throws the typed 502 INVALID_RESPONSE_SCHEMA,
• an additive-unknown-field response still passes (extra field stripped).

Verification

• type-check green: packages/shared, apps/web, apps/mobile
• Full vitest suites green: shared (1508), web (2246), mobile (1066)
• eslint (web + shared) and expo lint (mobile) clean
• React Doctor --scope changed: 0 issues (no React component files in the diff)
• jscpd: no new clones introduced (validation logic lives once in @orbit/shared)

Closes #479, Closes #451

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
orbit-ui-mobile-web	Ignored		Jul 13, 2026 9:36pm

[claude]

Code Review: PR #498

Scope: PR #498 in thomasluizon/orbit-ui-mobile — "feat(client): validate API responses at the remaining client boundaries (#479, #451)"
Recommendation: APPROVE

Summary

This PR extends #478's client-side trust-boundary Zod validation to the two ingestion seams it deliberately left open: an opt-in schema param threaded through apiFetch/fetchJson on web (gamification profile, streak, notifications) and a MutationType → ZodType registry consumed by both the mobile offline-queue's immediate-execute and deferred-flush paths (logHabit + the four bulk habit mutations). The safeParse-then-typed-throw logic from #478's two duplicated copies was correctly consolidated into one validateApiResponse helper in @orbit/shared/utils. The change is small, well-tested for the paths it touches, and free of any, console.log, or narration comments. Two non-blocking gaps are worth a fast follow-up.

Findings

Critical

None.

High

None.

Medium

[MEDIUM] Registered-type schema wiring through the offline-queue paths is untested end-to-end
· dimension: SOLID/clean-arch — test coverage
· location: apps/mobile/__tests__/lib/offline-mutations.test.ts, apps/mobile/__tests__/lib/queued-api-mutation.test.ts
· issue: Both test files were updated only to assert that apiClient is called with a trailing undefined schema for unregistered mutation types (createHabit, setTimeZone, assignTags, etc.). Neither file has a test that exercises processQueuedMutationFlush (apps/mobile/lib/offline-mutations.ts:616-624) or the default executor in performQueuedApiMutation (apps/mobile/lib/queued-api-mutation.ts:28-39) with a registered type such as logHabit, so nothing in these two suites actually proves getMutationResponseSchema(mutation.type) resolves to a real schema and validates when replayed through the queue. The only test that exercises a registered type against apiClient is mutation-response-schemas.test.ts, which calls apiClient directly and bypasses both queue entry points — so the PR's own claim ("validated identically whether it runs online now or on flush later") is unverified for the flush path.
· risk: A future refactor of processQueuedMutationFlush or the default executor could silently stop forwarding the schema (e.g. drop the third argument) and no test would catch it, since only the undefined case is pinned.
· fix: Add one case per suite that queues/replays a logHabit (or bulk) mutation and asserts apiClient was called with logHabitResponseSchema (or the matching bulk schema), not undefined.
· reference: CLAUDE.md rule 8 (validate at trust boundaries) / rubric dimension "missing test"

[MEDIUM, downgraded from an initial High after adversarial review] Mobile gamification/notification GET reads remain unvalidated while the identical web hooks were fixed — likely pre-existing and out of this PR's stated scope
· dimension: Parity (#9)
· location: apps/mobile/hooks/use-gamification.ts:36 (useGamificationProfile), :101 (useStreakInfo), apps/mobile/hooks/use-notifications.ts:37 (useNotifications)
· issue: This PR adds gamificationProfileSchema / streakInfoSchema / notificationsResponseSchema to the three matching web hooks. The mobile mirrors of these exact three hooks still call apiClient<T>(path) with no schema, unchanged by this diff — even though mobile's apiClient has supported an optional schema param since #478, the three shared schemas already exist, and the identical pattern is used two functions below in the same file (useReportEvent, use-gamification.ts:134-141).
· Adversarial review outcome: a skeptic pass confirmed this pattern is pre-existing and repo-wide — roughly 15 other mobile hooks (use-habit-queries.ts, use-goal-queries.ts, use-profile.ts, use-tags.ts, use-recap.ts, etc.) call apiClient<T>(path) the same unvalidated way, and the PR body scopes itself explicitly and only to two named, separately-tracked issues (#451 web reads, #479 mobile offline-queue mutations) — neither of which claims mobile GET-read validation as in-scope. Because the gap is systemic and pre-existing rather than introduced or selectively left open by this diff, it does not meet the bar for a blocking Parity finding on this PR; it is downgraded from High to Medium and not treated as a merge-blocking mirror gap.
· risk: mobile keeps trusting these three response shapes unvalidated; a future API drift on these endpoints fails loudly on web (typed 502) but silently corrupts state on mobile.
· fix: Not required for this PR. Recommend a small follow-up issue (or fold into a future /audit-code-quality pass) to extend schema validation to mobile's GET reads broadly, starting with the three sites named above using the already-imported shared schemas.
· reference: root CLAUDE.md "Cross-platform parity (MANDATORY)"

Low / Info

None posted (signal gate).

Subagents

Agent	Verdict
parity-checker	PARTIAL (see Medium finding above — downgraded after adversarial review, non-blocking)
i18n-syncer	N/A — no user-facing strings or locale JSON changed
contract-aligner	N/A — diff doesn't touch packages/shared/src/types/* or endpoints.ts, and neither repo pairing gate is met; also not verifiable in CI (orbit-api not checked out)
security-reviewer	N/A — orbit-api untouched
design-reviewer	N/A — no apps/* UI/component files changed (hooks, lib, tests only)

Validation

Per CI adaptation: this PR already runs Build / Unit Tests / SonarCloud as separate required GitHub checks, so the skill's own validate phase was skipped here.

Check	Result
Lint	N/A — covered by separate required CI check
Type check	N/A — covered by separate required CI check
Tests	N/A — covered by separate required CI check
Build (api)	N/A — orbit-api not touched

Deferred — N/A dimensions & files not verdicted

• DESIGN.md / AI-slop (#8): N/A — no apps/* UI/component files in the diff.
• i18n (#10): N/A — no user-facing string or locale-JSON changes.
• Contract drift field-comparison (#11): N/A — packages/shared/src/types/* and endpoints.ts untouched; the orbit-api side is additionally unverifiable in this CI job (sibling repo not checked out). The backward-compat judgment itself was performed manually: no DTO/Zod schema fields were added, removed, or renamed by this diff, so there is no ⚠️ breaks old mobile clients candidate.
• Backend hard rules (#13): N/A — orbit-api untouched.
• FEATURES.md parity (#14): N/A — internal validation/robustness change only; no new screen, route, tool, or gating behavior.
• All 15 changed files were read and given a verdict; nothing was left unexamined for size or time reasons.

What's good

• Correct root-cause consolidation: the safeParse → typed-ApiClientError-throw logic that previously lived twice (server-fetch.ts, mobile api-client.ts) is now one validateApiResponse helper in @orbit/shared/utils, and the dead ApiClientError import was cleanly dropped from apps/mobile/lib/api-client.ts once no longer directly referenced.
• The mutation-response registry (apps/mobile/lib/mutation-response-schemas.ts) is a clean, minimal, opt-in solution to the real constraint that a persisted queued mutation can't carry a runtime schema function — and it's correctly consulted by both the immediate-execute and deferred-flush paths.
• Every registered schema (logHabitResponseSchema, the four bulk schemas) is verified against the actual use-habits.ts call sites and matches 1:1 by mutation type.
• Tests directly cover the three claimed behaviors (valid response passes, contract mismatch throws typed 502, additive unknown field is stripped and still passes) on both the web seam and the mobile registry boundary.
• No any, no console.log, no narration comments; the one JSDoc block with an issue-tracker WHY link (mutation-response-schemas.ts) is exactly the sanctioned exception to the comment ban.

Recommendation

Approve as-is. Open a fast follow-up (not blocking this PR) to: (1) add the two missing registered-type test cases in the offline-queue suites, and (2) extend schema validation to mobile's GET reads more broadly — starting with use-gamification.ts and use-notifications.ts — as a small, separate parity/hardening pass.

[claude]

Code Review: PR #498

Scope: PR #498 in thomasluizon/orbit-ui-mobile — feat(client): validate API responses at the remaining client boundaries (#479, #451)
Recommendation: APPROVE

Severity	Count
Critical (incl. ⚠️ old-client breaks)	0
High	0
Medium	0
Low / Info	1

---

Summary

Extends the #478 trust-boundary Zod-validation seam to the two client ingestion boundaries it deliberately left open: web's apiFetch/fetchJson gain an opt-in schema param (threaded into gamification profile / streak / notifications reads), and mobile's offline-queue mutation path gets a MutationType -> ZodType registry consulted identically by the immediate execute path (performQueuedApiMutation) and the deferred flush replay (processQueuedMutationFlush). The safeParse → typed-ApiClientError logic from four prior copies is consolidated into one validateApiResponse helper in @orbit/shared/utils. The diff is internal/reliability-only — no UI, no i18n, no contract-field changes, no orbit-api changes — and is well tested on both boundaries. No Critical or High findings survived review.

Note on iteration: my prior review of this PR (on commit 6d46672c) flagged two non-blocking Mediums as fast-follow suggestions: (1) missing end-to-end test coverage proving the offline-queue registry forwards a real schema (not just undefined) through both the immediate-execute and deferred-flush paths, and (2) mobile GET reads (use-gamification.ts, use-notifications.ts) lacking the same validation the web hooks got. Commit 348bae13 ("test(mobile): prove queue schema forwarding + mirror web read validation") addresses both directly — this review confirms they are resolved and does not re-flag them.

Findings

Critical / High / Medium

None.

Low / Info

• Info · dimension: Type safety · apps/mobile/lib/queued-api-mutation.ts:38 — getMutationResponseSchema(resolvedMutation.type) as ZodType<TResult> | undefined narrows an untyped registry lookup to the caller's TResult via assertion. Verified every current call site (apps/mobile/hooks/use-habits.ts:119-120, 733-734, 815-816, 867-868, 933-934) passes a TResult matching the registered schema's inferred type exactly, so there is no live mismatch. Flagging only as a forward-looking note: a future registry entry with a mismatched TResult at its call site would compile without error. Not concretely actionable today — no fix required.

Subagents

Agent	Verdict
parity-checker	PAIRED — GET-read schema wiring (use-gamification.ts, use-notifications.ts) is identical web/mobile; the mobile-only offline-queue files are a legitimate platform adapter (web has no offline queue; web mutations already validate the same five schemas via Server Actions from #478)
i18n-syncer	N/A — no user-facing strings or locale files touched
contract-aligner	N/A — no packages/shared/src/types/* / endpoints.ts changes; no orbit-api changes
security-reviewer	N/A — no orbit-api code touched
design-reviewer	N/A — no apps/* UI files touched (only lib/, hooks/, tests)

Validation

Per the CI adaptation for this PR: Build / Unit Tests / SonarCloud run as separate required GitHub checks, so the skill's own validate phase (Phase 6/7) was skipped here. Per the PR body: type-check green (shared/web/mobile), full vitest green (shared 1508, web 2246, mobile 1066), eslint/expo lint clean, React Doctor 0 issues, jscpd no new clones — not independently re-run in this review.

Check	Result
Lint	N/A — covered by separate required CI check
Type check	N/A — covered by separate required CI check
Tests	N/A — covered by separate required CI check
Build (api)	N/A — orbit-api not touched

Deferred — N/A dimensions & files not verdicted

• DESIGN.md / AI-slop (#8) — N/A, no apps/* UI files in the diff (only lib/, hooks/, tests).
• i18n (#10) — N/A, no user-facing strings or packages/shared/src/i18n/*.json changed.
• Contract drift + backward-compat guard (#11) — N/A for field drift: no packages/shared/src/types/* schema field was added/removed/renamed (the diff only wires existing schemas into new call sites plus a shared validation helper). The orbit-api sibling repo is not checked out in this CI environment, so the DTO side of the field-by-field comparison is not independently re-verified — the PR body's claim that each mobile-registry schema was "verified field-by-field against its orbit-api DTO" is taken at face value.
• Backend hard rules (#13) — N/A, orbit-api not touched.
• FEATURES.md parity (#14) — N/A, internal reliability change, no new/changed/removed user-facing feature surface.

All changed files across the PR (mobile: 9, web: 5, shared: 4) were read and given a verdict.

What's good

• Genuine root-cause consolidation, not a workaround: four independent copies of safeParse → typed-ApiClientError logic collapse into one validateApiResponse in @orbit/shared/utils; the old mobile validateResponseSchema local function is deleted outright rather than left dead alongside the new shared one.
• Validation stays opt-in (schema?: ZodType<T>) everywhere, so unregistered callers are behavior-unchanged — correctly scoped, low-risk rollout.
• The mobile registry is wired into both the immediate execute path and the deferred flush replay so a queued mutation validates identically online-now vs. replayed-later — now explicitly tested end-to-end (offline-mutations.test.ts: "forwards the registered response schema when flushing a schema-backed mutation").
• Zod's "strip unknown keys" behavior is deliberately leaned on and tested at every layer (shared, web, mobile) to prove the append-only contract holds.
• Comment policy is clean: only JSDoc on exported functions, plus one WHY-with-URL note tying the registry's dual-path design to issue #479.
• Follow-up commits directly and completely addressed both fast-follow suggestions from the prior review round (queue schema-forwarding tests, mobile GET-read parity with web).

Recommendation

Approve as-is. No changes required before merge.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #498

Scope: PR #498 in thomasluizon/orbit-ui-mobile — feat(client): validate API responses at the remaining client boundaries (#479, #451)
Recommendation: APPROVE

Summary

This PR extends the #478 trust-boundary Zod-validation seam to the two client ingestion boundaries it deliberately left open: the web apiFetch/fetchJson (gamification profile, streak, notifications reads) and the mobile offline-mutation queue (logHabit + the four bulk habit mutations, validated identically on the immediate-execute and deferred-flush-replay paths). The duplicate safeParse-then-throw logic from server-fetch.ts and mobile api-client.ts was correctly consolidated into one validateApiResponse helper in @orbit/shared/utils. All 18 changed files were reviewed with a verdict; the diff is clean, additive, and matched by tests on both boundaries (valid pass, contract-mismatch throw, additive-field pass-through).

Findings

Critical

None.

High

None.

Medium

None.

Low / Info

[Info] Registry lookup requires a type assertion at the call site
· dimension: Type safety (#6)
· location: apps/mobile/lib/queued-api-mutation.ts:38
· issue: getMutationResponseSchema(resolvedMutation.type) as ZodType<TResult> | undefined casts the registry's bare ZodType (zod v4 defaults this to ZodType<unknown>) to the caller's generic TResult. Nothing at the type level ties the MutationType key to TResult.
· risk: Low today — every current call site (use-habits.ts) pairs type: 'logHabit' with TResult = LogHabitResponse, etc., correctly, and a future mismatch would still be caught at runtime by the Zod safeParse throwing INVALID_RESPONSE_SCHEMA rather than silently corrupting data. Not a data-safety hole, just a compile-time gap.
· fix: Optional follow-up — a mapped type (MutationType → response type) could make getMutationResponseSchema return the correlated type without a cast, but given the Partial<Record<...>> registry only covers 5 of ~50 mutation types today, this is a nice-to-have, not a blocker.
· reference: CLAUDE.md rule 3

Subagents

Agent	Verdict
parity-checker	PAIRED — use-gamification.ts/use-notifications.ts schema-threading is behaviorally identical web↔mobile; the mobile-only offline-queue files are a legitimate platform adapter (web has no offline mutation queue — mutations go straight through Server Actions), not a parity gap
i18n-syncer	N/A — no user-facing strings added or changed
contract-aligner	N/A — gate not met: diff touches neither orbit-api nor packages/shared/src/types/*/endpoints.ts
security-reviewer	N/A — orbit-api not touched
design-reviewer	N/A — no apps/* UI/component files changed (lib + hooks only)

Validation

Check	Result
Lint	N/A — skipped per CI-wrapper instruction (Build/Tests/SonarCloud run as separate required checks)
Type check	N/A — skipped per CI-wrapper instruction
Tests	N/A — skipped per CI-wrapper instruction (PR body reports shared 1508 / web 2246 / mobile 1066 green)
Build (api)	N/A — orbit-api not touched

Deferred — N/A dimensions & files not verdicted

• Dimension 8 (DESIGN.md/AI-slop): N/A — no apps/* UI files in the diff.
• Dimension 10 (i18n): N/A — no new user-facing strings.
• Dimension 11, backward-compat half: no field removed/renamed/newly-required anywhere in the diff (no packages/shared/src/types/* or orbit-api DTO changes at all) — the guard's trigger condition isn't met, so this is a clean N/A rather than a checked-and-passed judgment. The PR body's claim that the five new registry schemas were "verified field-by-field against its orbit-api DTO" is not independently verifiable in CI — the orbit-api sibling repo is not checked out in this environment.
• Dimension 13 (backend hard rules): N/A — orbit-api not touched.
• Dimension 14 (FEATURES.md parity): N/A — this PR is internal validation plumbing (error-path hardening), not a user-facing feature add/change/removal.
• Phase 7 (Validate): skipped per the CI-wrapper's explicit instruction (Build/Tests/SonarCloud already run as separate required checks).

Nothing else deferred — every one of the 18 changed files received a verdict.

What's good

• Textbook root-cause consolidation: the two pre-existing copies of the safeParse→typed-throw logic plus the two new seams collapse into one validateApiResponse in @orbit/shared, with zero duplication (rules 1, 10).
• Fully opt-in and additive: callers without a schema argument are behavior-unchanged; Zod strips unknown keys, so the append-only contract (root CLAUDE.md) is respected and proven by a dedicated "additive field still passes" test on every seam.
• The mobile registry's JSDoc correctly explains why it's needed (a queued mutation is a persisted plain object, can't carry a runtime schema function) and links the tracking issue — compliant with the comment policy (rule 5).
• Web and mobile reads were migrated in lockstep in the same PR (parity-checker confirms), and the mobile offline-queue's two independent call paths (immediate execute + deferred flush) were both wired to the same registry lookup, closing exactly the gap the PR describes.

Recommendation

Merge as-is. The one Info-level note (queued-api-mutation.ts:38) is a non-blocking observation for a future, larger registry-typing pass — not required for this PR.