Skip to content

Commit 6b2ec3b

Browse files
claude[bot]claude
andauthored
fix(client): oauth.applications.register declares redirect_uris optional, matching the body schema of the route it posts to (#17215) (#17834)
Fixes #17215 Clause-②: yes Declared by the dispatching `domain:cli` seat (#6024) and unchanged by the delivered diff: relaxing `redirect_uris` **enlarges a published SDK method's accepted set**, which is the widening direction and is graded conservatively regardless of the fact that it restores parity with the vendor. `needs:contract-review` is on the card; this PR does not enqueue until an at-tier contract review of record exists, which is the PM seat's half and is ⛔ not satisfied by green CI. ## ⭐ The falsifier, resolved FIRST Triage (`5621351376`) fenced this card: before writing the `?`, establish whether `redirect_uris`' required-ness is a **deliberate guard** rather than an oversight — RFC 7591 makes `redirect_uris` required for authorization-code clients, so a wrapper stricter than a permissive vendor may be correct on purpose. An assertion found ⇒ stop, write no code. **Verdict: no such assertion exists anywhere.** Every search below is reported with its count and a lit control, because a bare "found nothing" is not a result. | surface searched | how | hits | lit control (proves the matcher fired) | |---|---|---|---| | the declaration's own file, in context | `grep -n redirect_uris packages/client/src/index.ts`, **every hit read in context** | **8**, none an assertion | the same 8 hits are the control — the matcher fires and each was read, not counted | | repo-wide `redirect_uris` | `git grep -c` | **20 files**, none an assertion | `create-client` -> 8 files | | ADRs | `git grep` in `docs/adr/` | **0** files for `redirect_uris`, **0** hits for `7591` | `Status` -> **136** ADR files | | RFC 7591 anywhere in-tree | `git grep 7591` | **20 files** — none ties 7591 to this member's required-ness | `RFC ` -> **150** files | | guard-shaped prose (`deliberately/intentionally required`, `stricter than`, `guard rather than`, `on purpose`) | `git grep -niE` over client, plugin-auth, docs, content, changesets | **12 hits, zero about this member** | `deliberately` in `packages/client` -> 51 in CHANGELOG alone | | the pin test named by the docblock | read in full (253 lines) | pins the **key set**, never this member's optionality | it pins `client_name` and `scope` value types one screen up — so the idiom exists and was deliberately not applied here | | `oauth-provider-schema-parity.test.ts` | read in full | **0** mentions of `redirect_uris`, `create-client`, `body` | it is a DB-column parity gate; its own subject matter is the control | | PR review threads on the sibling PR #17209 | REST `/pulls/17209/comments` and `/reviews` | **0** and **0** | repo-wide `/pulls/comments` -> **100** returned, so the channel is lit | | review threads on the commits that CREATED the required-ness | REST `/commits/{sha}/pulls` | **none — both are direct commits, no PR, so no review thread ever existed** | the same endpoint resolves PRs for other shas | | the sibling issue threads | `/issues/15447/comments`, `/issues/17210/comments` | **13** and **9** `redirect_uris` lines — every one calls it a *measured mismatch / divergence to report* | comment counts 10 and 7 returned | **What the history shows instead of a guard.** The member was required from the method's first commit, `170ecfa4c` ("feat: add OAuth application management features"), which declared `client_name: string` required too. Nine hours later `bc5bd719b` ("feat(auth): update OAuth application handling and consent flow with improved parameter management") re-pointed the method at `/oauth2/create-client`, **relaxed `client_name` to optional**, added the three bogus members #15447 later removed — and left `redirect_uris` untouched as unchanged context, with no comment and a one-line commit message. Both are direct commits with no PR. Nothing, anywhere, ever stated a reason. ⇒ Per triage's own branch: **no assertion found ⇒ proceed; the `?` restores parity.** ### ⚠️ Near-misses, so the next reader does not re-file this The dispatch order handed over two; reading every hit in context turned up **three more**. None speaks to whether the REQUEST member may be omitted: 1. `index.ts` "Only `client_id` and `redirect_uris` are always present … `redirect_uris` is unconditional" — the **response** projection's serialiser. 2. `index.ts` "⚠️ `redirect_uris` is always `[]` here" — the **public response** projection. 3. *(new)* `index.ts`, inside this very method's docblock: "the array the vendor schema requires … `redirect_uris` must arrive **pre-split**". About the **element shape** (array vs the Console textarea), not about presence. 4. *(new)* `auth-plugin.ts`: "better-auth's Zod body schema requires `redirect_uris: string[]`" — about the **type**, and it names its own failure mode, `expected array, received string`. It is the Console wrapper's split, in another package. 5. *(new)* #17210 comment `5642689415` calls the request type "the vendor shape" while pointing at `redirect_uris: string[]` — an acceptance row asserting that a prose-only PR moved no type. ## Acceptance 2 — the vendor schema re-introspected at RUNTIME ⛔ Not read from a `.d.ts`. Method as the card prescribes: `createRequire` the package, `await import` the resolved entry, instantiate, walk `endpoints`, select the endpoint whose `path` contains `create-client`, read `options.body`. ⚠️ **The package version has MOVED: the card measured `1.7.2`; the installed version is `1.7.3`** (`packages/plugins/plugin-auth/package.json` pins `1.7.3`, lockfile resolves `1.7.3`, and the package's own `package.json` read off the resolved entry's directory reports `1.7.3` at runtime). Re-measuring was therefore load-bearing, not ceremony. ⚠️ A second difference at 1.7.3: a zero-argument `oauthProvider()` **throws** (`Cannot read properties of undefined (reading 'clientRegistrationAllowedScopes')`), so it was instantiated the way this repo instantiates it, `oauthProvider({ loginPage, consentPage })`. ``` endpoints walked : 33 create-client endpoint: key=createOAuthClient path=/oauth2/create-client options.body present : yes body constructor: ZodObject member count : 21 (card recorded 21 at 1.7.2 — no drift) ``` The row under test, read today, with optionality **driven** rather than inferred: | member | optionality (driven: `safeParse(undefined)`) | type | |---|---|---| | `redirect_uris` | **optional** | optional of array of string | ``` safeParse(undefined).success : true <- the member may be omitted safeParse([]).success : false <- an EMPTY array is refused safeParse(["https://a/cb"]) : true body.safeParse({ client_name: 'probe' }).success : true parsed keys: client_name unknown key => success: true | STRIPPED (no catchall — zod default strip, as the card recorded) ``` **All 21 members are optional**, and a body omitting `redirect_uris` entirely parses `ok`. The divergence the card describes is confirmed at 1.7.3. ⚠️ Carried into the docblock and the changeset, because it is a trap in the other direction: optional does **not** mean `[]` will do — the vendor refuses an empty array, so omitting the member and passing `[]` are different requests and only the first is legal. ## Acceptance 3 — a DRIVEN reading that the call is now expressible ⛔ Not a type-level argument; compiled. `pnpm --filter @objectstack/client typecheck` -> **exit 0**, and that script is two halves — `tsc --noEmit && pnpm check:test-typecheck` — the second of which compiles the test layer under `tsconfig.test.json`. New pin `registerRedirectUrisOptionalPin17215`, compiled and never invoked, in the file the docblock already names: - an `expectTypeOf` on `RegisterRequest['redirect_uris']`, asserted equal to the union of `string[]` and `undefined` — the parity assertion, in the file's existing house idiom (spelled in words here because the body sanitiser eats angle-bracket fragments; the file carries the real generic syntax); - `register({ client_name: 'PROBE-17215-OMITTED' })` — **previously inexpressible**; - `register({})` — the emptiest legal call; - `register({ client_name, redirect_uris })` — still compiles, unchanged; - plus one runtime `it()` proving the SDK synthesises no default: the body sent is exactly `{"client_name":"PROBE-17215-OMITTED"}` with no `redirect_uris` key. ⚠️ **The existing `keyof` key-set equality gives this change ZERO coverage** — `keyof` is insensitive to optionality, so it reads the same eleven names in both states. That is why a new assertion was owed rather than relying on the one already there, and it is stated in the new pin's docblock. ⛔ The equality itself is **untouched**: `git diff` on that test file has **0 removed lines** — the file is purely additive. ### Ablation, and a correction I am reporting rather than hiding ⚠️ **My first ablation run was wrong and its green is void.** I predicted RED, mutated `?` back to required, ran `pnpm --filter @objectstack/client exec tsc --noEmit`, and got **exit 0**. The cause is not the pin: `packages/client/tsconfig.json` carries `"exclude": [… "**/*.test.ts"]`, so the bare `tsc` program never contains the pin file. Proved with `--listFiles`: **0** occurrences of the pin file and **0** `.test.ts` files in that program, against a lit control of **1** for `src/index.ts`. That run is **NOT MEASURED**, not a pass. Re-run against the program that actually compiles the test layer: ``` MUTATION PROVEN ON DISK (optional spelling 1 -> 0, required spelling 0 -> 1, blob differs from HEAD's) pnpm --filter @objectstack/client typecheck -> exit 1 check:test-typecheck: src/oauth-applications-register-request-members.test.ts: 4 type error(s) packages/client/src/oauth-applications-register-request-members.test.ts(189,66): error TS2344: Type 'string[] | undefined' does not satisfy the constraint 'string[]'. packages/client/src/oauth-applications-register-request-members.test.ts(194,50): error TS2345: Argument of type '{ client_name: string; }' is not assignable ... packages/client/src/oauth-applications-register-request-members.test.ts(197,50): error TS2345: Argument of type '{}' is not assignable ... packages/client/src/oauth-applications-register-request-members.test.ts(269,41): error TS2345: Argument of type '{ client_name: string; }' is not assignable ... ``` Direction RED as predicted; **count 4 where I predicted 3**, and the extra is nameable: the runtime `it()` I added also omits the member, so it stops compiling too. Both legs ran from the committed state under `trap … EXIT INT TERM` with absolute paths; restore proved by blob equality with `HEAD:packages/client/src/index.ts` **and** empty `git diff HEAD` **and** empty `git status --porcelain`. No dist preflight applies — the pin imports `./index`, a relative source specifier inside its own package, so no built artifact sits in the resolution path (`check:test-source-alias` exit 0). ## Acceptance 4 — nothing else in the signature moves ``` 1 file changed, 1 insertion(+), 1 deletion(-) (the type change itself) - redirect_uris: string[]; + redirect_uris?: string[]; ``` Member-by-member against the merge base, extracted from both revisions and compared: ``` members at BASE: 11 members at HEAD: 11 lines IDENTICAL between the two: 10 client_name? client_uri? contacts? grant_types? logo_uri? policy_uri? response_types? scope? token_endpoint_auth_method? tos_uri? the single differing line: redirect_uris: string[]; -> redirect_uris?: string[]; ``` ## Acceptance 5 — changeset `.changeset/17215-oauth-register-redirect-uris-optional.md`, **minor**: a published package's public surface moves, per the maintainer's ruling of 2026-09-04 (decision batch #35) that such a change takes at least `minor` — the same grade the sibling #15447 carried. ⛔ Not breaking in this direction: relaxing a required member to optional keeps every existing call valid, so the changeset carries no BREAKING banner and `check:adr-0087-registration` correctly reports no declared-breaking changeset (exit 0). ## Verification Exit codes captured by redirect-then-`$?`, ⛔ never through a pipe. Lock runs read their own `VERDICT command-exit` line. - `pnpm --filter @objectstack/client typecheck` — **exit 0** (test layer compiles under `tsconfig.test.json`; 0 files / 0 errors in `test-typecheck-debt.json`). - `pnpm --filter @objectstack/client test` — **exit 0**, **43 files / 519 tests passed**. - `pnpm --filter '@objectstack/client^...' build` — **exit 0**. Required first: on an unbuilt `dist` the typecheck reported 17 phantom `TS2307 Cannot find module '@objectstack/spec/*'` errors that have nothing to do with this diff. - Gate families derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (⛔ never a hand list) and reconciled: **`✓ 60 derived famil(ies) accounted for — 59 run, 1 NOT-MEASURED, 0 UNRUN`**. - The first derivation carried a **STALE TREE** warning naming `scripts/pm/check-half-states.mjs`; rather than run a list about a tree nobody is on, `origin/main` was fetched and merged (clean, no `os-regen` deferral, and neither incoming file overlaps this diff) and the families re-derived at `834e79059`. - `pnpm check:dual-build-cjs-loads` — **exit 3, `PREREQUISITE NOT MET`**: it reads built output and 32 packages have no `dist/`. ⛔ Declared NOT MEASURED, never counted as a pass; its own text says "This is NOT a pass: nothing was measured". CI checks out fresh and builds. This diff changes one type annotation and adds test code, so it emits no JavaScript for that gate to read. - Two others refused their prerequisites on the first pass and were remediated, ⛔ not counted as passes until re-run: `check:skill-examples` exit 1 (`packages/client-react/dist` held no declarations) -> after building `client-react` + `client`, **exit 0**, 258 prose examples type-check across 3 surfaces; `check:type-check-debt` exit 3 (the whole-repo re-measure OOMed under `--max-old-space-size=4096`) -> at 8192, **exit 0**, "5 ledger entr(ies) re-measured, 55 raw tsc error(s), none above its recorded number". - `pnpm lint` population, run in full rather than narrowed: `eslint . --no-inline-config` — **exit 0**, **6645 files checked, 0 errors, 0 warnings**, at `834e79059`. Type-aware linting is not enabled (`eslint.config.mjs` states "no `parserOptions.project`, no typed `@typescript-eslint` rules"), so per-file verdicts are independent. ## Acceptance notes - **The `[]` asymmetry, noted not filed.** The TYPE cannot express "non-empty array", so `register({ redirect_uris: [] })` compiles and the vendor refuses it at runtime. This is unchanged by this PR — it was equally true while the member was required, which is also why the old required-ness guarded nothing the vendor did not already guard better. Recorded in the docblock and in the new pin so the compiling call is not mistaken for a legal one. - **`return-type-precision.test.ts` passes `redirect_uris` at both its call sites (including `[]`) and both still compile** either way; no change was needed there. - The card's item 3 — checking the answer against the two other registration doors (`POST /oauth2/register` DCR, and the ObjectStack `sys-oauth-application/register` mount) — is ⛔ deliberately not taken here: neither door's shape moves, and #17210 already ruled the mount stays `server-only`. --- _Generated by [Claude Code](https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f14d7b5 commit 6b2ec3b

3 files changed

Lines changed: 120 additions & 1 deletion

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/client": minor
3+
---
4+
5+
fix(client): `oauth.applications.register` declares `redirect_uris` optional, matching the body schema of the route it posts to (#17215)
6+
7+
`ObjectStackClient.oauth.applications.register` declared `redirect_uris` **required**. `POST /api/v1/auth/oauth2/create-client` is mounted verbatim from `@better-auth/oauth-provider`, and that route's body schema declares the member **optional** — so a request the route accepts had no spelling through this SDK. The caller never got a wrong answer; they got a call they could not write.
8+
9+
## What changes for a caller
10+
11+
Nothing they have to do. Every existing call still compiles — this only *adds* spellings:
12+
13+
```ts
14+
// now expressible, and accepted by the route:
15+
await client.oauth.applications.register({ client_name: 'My App' });
16+
17+
// unchanged, and still the right call when you have redirect URIs:
18+
await client.oauth.applications.register({
19+
client_name: 'My App',
20+
redirect_uris: ['https://app.example.com/cb'],
21+
});
22+
```
23+
24+
⛔ Not breaking in this direction — relaxing a required member to optional keeps every existing call valid. Tightening it back later would be breaking, which is why the parity is now pinned.
25+
26+
## Measured at runtime, not read off a `.d.ts`
27+
28+
The vendor body schema was re-introspected the way the card's original measurement was taken: instantiate `oauthProvider()`, walk `endpoints`, find the endpoint whose `path` is `/oauth2/create-client`, read `options.body`. At the installed **1.7.3** (the card measured 1.7.2; the package has since moved) the object still declares **21 members and every one of them is optional**, and `body.safeParse({ client_name: '…' })` succeeds with `redirect_uris` absent.
29+
30+
⚠️ Optional does **not** mean an empty array will do: the vendor refuses `[]`, so when the member is present it must be non-empty. Omitting it and passing `[]` are different requests and only the first is legal. Nor does it mean a client registered without redirect URIs is *usable* — it cannot complete an `authorization_code` flow. The type states what the route accepts, never that every accepted call yields a client fit for every grant; the docblock now says both.
31+
32+
## Why it was required, for the record
33+
34+
Not as a guard. It is residue from the method's first commit, which declared `client_name` required too; the same-day follow-up relaxed `client_name` and left this one behind. No comment, test, ADR or review thread ever asserted a reason for it — which is exactly why it read as a defect to the next auditor.
35+
36+
Nothing else on the signature moves: the other ten members are byte-identical.

packages/client/src/index.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4090,11 +4090,32 @@ export class ObjectStackClient {
40904090
* performs no such split — `redirect_uris` must arrive **pre-split**,
40914091
* one entry per URL, which is what an SDK caller holds anyway.
40924092
*
4093+
* ## ⚠️ `redirect_uris` is OPTIONAL here, and that is measured parity (#17215)
4094+
*
4095+
* It used to be the one required member on this type. It was never a
4096+
* deliberate guard — it is residue from the method's first commit, which
4097+
* declared `client_name` required too; the same-day follow-up relaxed
4098+
* `client_name` and left this one behind, and no comment, test, ADR or
4099+
* review thread ever asserted a reason for it.
4100+
*
4101+
* Re-introspected at runtime against `@better-auth/oauth-provider@1.7.3`
4102+
* — instantiate `oauthProvider()`, walk `endpoints`, read `options.body`
4103+
* — the member is `optional`, and a body omitting it entirely parses
4104+
* `ok`. All 21 members of that schema are optional.
4105+
*
4106+
* ⚠️ Optional does NOT mean `[]` will do. The vendor refuses an empty
4107+
* array, so when the member is present it must be non-empty: omitting it
4108+
* and passing `[]` are different requests, and only the first is legal.
4109+
* ⚠️ Nor does it mean a client registered without redirect URIs is
4110+
* usable — it cannot complete an `authorization_code` flow. This type
4111+
* states what the route accepts, never that every accepted call yields a
4112+
* client fit for every grant.
4113+
*
40934114
* Pinned by `oauth-applications-register-request-members.test.ts`.
40944115
*/
40954116
register: async (req: {
40964117
client_name?: string;
4097-
redirect_uris: string[];
4118+
redirect_uris?: string[];
40984119
token_endpoint_auth_method?: 'none' | 'client_secret_basic' | 'client_secret_post';
40994120
grant_types?: string[];
41004121
response_types?: string[];

packages/client/src/oauth-applications-register-request-members.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,55 @@ export async function registerRequestMemberPins15447(): Promise<void> {
160160
}));
161161
}
162162

163+
// ─────────────────────────────────────────────────────────────────────────
164+
// ①b [#17215] `redirect_uris` is OPTIONAL — parity with the vendor schema
165+
// ─────────────────────────────────────────────────────────────────────────
166+
167+
/**
168+
* [#17215] The eleventh member was the one required member on this type, and
169+
* the route it posts to declares it **optional**. Re-introspected at runtime
170+
* against `@better-auth/oauth-provider@1.7.3` — instantiate `oauthProvider()`,
171+
* walk `endpoints`, read `options.body` — every one of that schema's 21
172+
* members is optional, and `body.safeParse({ client_name })` succeeds with
173+
* `redirect_uris` absent. So a request the route accepts had no spelling here.
174+
*
175+
* ⚠️ This pin is type-level for the same reason `registerRequestMemberPins15447`
176+
* is: the route answers the same way either way, and only a compile-time
177+
* assertion can observe a member's OPTIONALITY changing. A runtime assertion
178+
* on the request bytes cannot — `JSON.stringify` omits an absent member
179+
* whether the type required it or not, so the body is byte-identical in both
180+
* states and any such test is green before the fix and green after it.
181+
*
182+
* ⛔ The key-set equality above is deliberately NOT the guard for this:
183+
* `keyof` is blind to optionality, so it reads the same eleven names in both
184+
* states. That is why it keeps holding across this change, and why this needs
185+
* its own assertion rather than relying on the one already there.
186+
*/
187+
export async function registerRedirectUrisOptionalPin17215(): Promise<void> {
188+
// ── the parity assertion — red if anyone re-tightens it ──────────────
189+
expectTypeOf<RegisterRequest['redirect_uris']>().toEqualTypeOf<string[] | undefined>();
190+
191+
// ── the call that was previously INEXPRESSIBLE ───────────────────────
192+
// Before this card `redirect_uris` was required, so this did not compile at
193+
// all. It is the whole point of the change: the vendor accepts this body.
194+
void (await client.oauth.applications.register({ client_name: 'PROBE-17215-OMITTED' }));
195+
196+
// The emptiest legal call: every member of the vendor schema is optional.
197+
void (await client.oauth.applications.register({}));
198+
199+
// ── and the call that always worked still does ───────────────────────
200+
void (await client.oauth.applications.register({
201+
client_name: 'CTRL-17215-SUPPLIED',
202+
redirect_uris: ['https://app.example.com/cb'],
203+
}));
204+
205+
// ⚠️ Optional is not "any array will do": the vendor refuses `[]`
206+
// (`safeParse([])` fails at runtime). The TYPE cannot express non-empty, so
207+
// this still compiles — recorded here so the next reader does not mistake
208+
// the compiling call for a legal one.
209+
void (await client.oauth.applications.register({ redirect_uris: [] }));
210+
}
211+
163212
// ─────────────────────────────────────────────────────────────────────────
164213
// ② The negative control — what the route DOES honour still arrives verbatim
165214
// ─────────────────────────────────────────────────────────────────────────
@@ -211,6 +260,19 @@ describe('#15447 oauth.applications.register — the honoured members still reac
211260
expect(init.body).toBe(JSON.stringify(req));
212261
});
213262

263+
it('[#17215] a call omitting `redirect_uris` sends a body without the key at all', async () => {
264+
// The type-level pin above cannot witness this half: it proves the call
265+
// COMPILES, never what reaches the wire. This proves the SDK adds no
266+
// default — no `redirect_uris: []` synthesised on the caller's behalf,
267+
// which the vendor would refuse outright.
268+
const { client: c, fetchMock } = clientCapturingRequest();
269+
await c.oauth.applications.register({ client_name: 'PROBE-17215-OMITTED' });
270+
const [url, init] = soleRequest(fetchMock);
271+
expect(url).toBe(CREATE_CLIENT_URL);
272+
expect(init.body).toBe(JSON.stringify({ client_name: 'PROBE-17215-OMITTED' }));
273+
expect(JSON.parse(init.body as string)).not.toHaveProperty('redirect_uris');
274+
});
275+
214276
it("surfaces the route's refusal of an array-form `scope` rather than swallowing it", async () => {
215277
// ⚠️ The 400 below is a RECORDED response, replayed — never one this test
216278
// produces. It is the verbatim answer the driven run got (issue #15447,

0 commit comments

Comments
 (0)