chore(shared): re-baseline the contract snapshot after the OAuth client_id removal - #600
Merged
Merged
Conversation
…nt_id removal The Contract Drift gate went red on every open ui PR. It was correct: orbit-api PR #435 dropped the unread `client_id` form field from `/oauth/token`, which changed the published OpenAPI spec, so the committed Zod snapshot no longer matched orbit-api main. Regenerated with `npm run generate:zod -w @orbit/shared`. The diff is exactly the two lines the api change removed, nothing else. No consumer change is needed: grepping `packages/shared/src`, `apps/web` and `apps/mobile` outside `__generated__` finds zero references to `client_id`, so no hand-written type or callsite depended on it. The field was write-only from the client's side and was never read server-side either, which is why it was removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M2vH4pDvNnxUPecWWvwBc
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #600 — chore(shared): re-baseline the contract snapshot after the OAuth client_id removal
Scope: PR #600 in thomasluizon/orbit-ui-mobile (chore/rebaseline-contract-snapshot → main)
Recommendation: APPROVE
Summary
The diff removes the unused client_id field from the generated PostOauthTokenBody Zod schema (packages/shared/src/types/__generated__/api.generated.ts, -2/+0), re-syncing the committed contract snapshot with orbit-api after PR #435 (merged) dropped the same unread field from OAuthController.Token. The change is exactly the two lines the upstream OpenAPI diff removed, nothing else. Verified end-to-end: the field is unreferenced anywhere in hand-written code in either app, the endpoint itself is not consumed by any client code, and the upstream API PR itself confirms unbound extra form fields are ignored server-side — no reachable path, old client or new, breaks.
Findings
Critical / High / Medium
None.
Low / Info
[INFO] Request-schema field removal correctly judged non-breaking
· dimension: 11 — Contract drift + backward-compat guard
· location: orbit-ui-mobile/packages/shared/src/types/__generated__/api.generated.ts:1226-1234
· issue: The rubric's blanket rule reads "field removed from a request DTO/schema → old
clients still send the old shape → server validation rejects it → Critical." Taken
literally this diff would be flagged breaks old mobile clients, since
PostOauthTokenBody is a request-body schema and client_id is a removed field.
· risk: none in practice, on three independent facts. (1) client_id was
zod.string().optional(), never required. (2) grep -rn "client_id" across
packages/shared/src, apps/web, apps/mobile (excluding __generated__) returns zero hits,
and a second grep for PostOauthTokenBody / oauth/token shows the generated type is never
imported or called by any hand-written code in this repo. (3) orbit-api PR #435 (merged)
states explicitly: "Callers that still send client_id are unaffected, since an unbound
form field is ignored" — confirming the endpoint ignores unrecognized extra fields rather
than rejecting the request; oasdiff breaking on the API side reported 0 errors.
· fix: none needed — correct deploy-API-first sequencing: API removed the field first
(live), this PR only catches the client-side generated snapshot up to match.
· reference: rubric.md dimension 11; orbit-api PR #435
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A — no apps/web/**/apps/mobile/** file changed |
| i18n-syncer | N/A — no user-facing string or i18n JSON changed |
| contract-aligner | MATCH — sibling orbit-api not checked out in this runner; verified equivalently via gh pr view/diff 435 --repo thomasluizon/orbit-api: the merged PR's openapi.json diff removes the identical client_id allOf clause, matching this 2-line diff 1:1 |
| security-reviewer | N/A — no orbit-api code in this diff |
| design-reviewer | N/A — no apps/*/orbit-landing-page UI file changed |
Validation
Skipped per CI adaptation instructions: this PR runs Build / Unit Tests / SonarCloud as separate required checks.
Deferred
- Dimensions 2-10, 12-14 marked N/A: the diff's changed range never touches logic, comments, UI, app-layer files, API code, or the feature surface — it is a two-line deletion in a generated contract snapshot.
contract-alignerran via direct cross-repoghverification instead of its normal filesystem diff (sibling repo not checked out in this CI job) — noted so "verified" isn't confused with "the subagent executed against a live checkout."
What's good
- Textbook deploy-API-first sequencing, minimal mechanical diff (
npm run generate:zodoutput only, no drive-by edits). - PR body pre-empts and correctly answers the compat question; independently re-verified here and holds.
Recommendation
Approve. No changes requested.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Contract Drift gate went red on every open ui PR. It was correct: orbit-api PR #435 dropped the unread client_id form field from /oauth/token, which changed the published OpenAPI spec, so the committed Zod snapshot no longer matched orbit-api main.
Regenerated with npm run generate:zod -w @orbit/shared. The diff is exactly the two lines the api change removed, nothing else.
No consumer change is needed: grepping packages/shared/src, apps/web and apps/mobile outside generated finds zero references to client_id, so no hand-written type or callsite depended on it. The field was write-only from the client's side and was never read server-side either, which is why it was removed.
This unblocks #597, #598 and #599.