Skip to content

fix(client): auth.me / auth.refreshToken deliver the SessionResponse envelope they declare, and refreshToken reads session.token - #17237

Merged
os-project-manager merged 4 commits into
mainfrom
claude/issue-16760-get-session-shape-and-refresh-read
Sep 9, 2026
Merged

fix(client): auth.me / auth.refreshToken deliver the SessionResponse envelope they declare, and refreshToken reads session.token#17237
os-project-manager merged 4 commits into
mainfrom
claude/issue-16760-get-session-shape-and-refresh-read

Conversation

@claude

@claude claude Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #16760

Clause-②: no

auth.me and auth.refreshToken annotate their return as SessionResponse — ObjectStack's REST { success, data } envelope — for GET /api/v1/auth/get-session, a route better-auth owns and answers bare. This lifts the bare answer into the envelope both methods declare, with SessionResponse and both published return annotations unchanged, and corrects refreshToken to read the token the route actually serves.

This is triage's route 3: the lift auth.login has always carried, applied to the two methods that never got it. Not route 1 (rebinding to the wire shape) and not route 2 (enveloping /auth/* at the producer) — neither was needed, and neither is in this diff.

The measurement this card turned on

The card claimed refreshToken's broken data.data?.token read was a consequence of the envelope misdeclaration. Triage judged that causal claim wrong but could not finish the reading, because the card's pasted body elides "session":{...}. That measurement is here, driven signed-in against a real AuthManager (better-auth 1.7.2, organization plugin on by its own default) over a real ObjectQL on a real SqliteWasmDriver:

GET /api/v1/auth/get-session  (signed in) -> 200
  top-level keys : ["user","session"]
  session keys   : ["expiresAt","token","createdAt","updatedAt","ipAddress",
                    "userAgent","userId","activeOrganizationId","activeTeamId","id"]
  body.token         === undefined
  body.session.token === "RMjPkloC0BqTdI4gQxZUNWm4arQYsliY"

GET /api/v1/auth/get-session  (anonymous) -> 200 null

session.token exists, so the dispatch's branch 2 applies: refreshToken changes its read and genuinely captures a credential. Triage's correction stands unchanged — there is no top-level token, so enveloping the body would not have put one at data.token either. The old read named a field this route does not produce at any nesting, which is why fixing the shape alone would have left the method exactly as inert as it was.

Two further facts the measurement turned up, neither of which triage had:

  • session.token is the UNSIGNED spelling; bearer() hands clients the signed token.signature form. Both authenticate — plugin-auth's resolveActor strips the signature on the bearer branch, and the measurement confirms both resolve to the same principal — so storing it keeps the caller signed in. Case ④ asserts exactly that, so the spelling swap is proven safe rather than merely observed.
  • /get-session neither rotates the token nor moves expiresAt across two calls seconds apart. ⚠️ This is not evidence that it never refreshes: AuthManager configures better-auth's session.updateAge at a 1-day default, so a brand-new session is exactly the case where nothing is expected to move. The reading is inconclusive by construction and no claim is built on it.

What changed

  • normalizeSessionResponse (module-private, no new export) lifts a bare { user, session } into { success: true, ...body, data: { user, session } }.
  • success is filled, not only data. SessionResponseSchema is BaseResponseSchema.extend(...) and that base declares success as a required boolean, so a body carrying data alone still does not parse as the declared type. This is what makes the fix closeable: without it the card's own class — declared contract not delivered — stays open on me.
  • The raw .user / .session keys are kept. They are the read the field was pushed onto while the declared shape was unreachable; dropping them would trade one silent breakage for another.
  • data.token is deliberately NOT synthesized from session.token. The declared key is optional, and the two spellings are not one string — login puts the signed form there. Populating it would file two different credentials under one key depending on which method produced the body.
  • refreshToken reads data.session.token.

Tests

packages/client/src/auth-get-session-envelope.test.ts — a real AuthManager over a real driver, transport stood in only so the client's fetch hands the Request to manager.handleRequest. Five blocks: the envelope parses against the declared schema; the raw keys survive; the anonymous null passes through; refreshToken captures a credential that resolves to the right principal; and a negative control that data.token is absent before and after the lift, so a regression to the old read cannot pass by accident.

The unit fixture in client.test.ts was replaced, not adjusted: it served { data: { token } }, a body /get-session has never produced, so it pinned the very read that made the method a no-op. A fixture modelling the misdeclaration cannot witness the fix.

Ablation — direction predicted in writing before any leg ran

leg mutation predicted observed
A1 drop success: true from the lift envelope case reddens 2 failed / 4 passed — both ① cases
A2 restore me to return res.json() every me() case reddens 4 failed / 2 passed — ①a ①b ② ⑤
A3 restore the data.data?.token read credential case reddens 1 failed / 5 passed — ④ only
A3u same mutation, unit suite replaced fixture reddens 1 failed / 215 passed

Each leg proved the mutation reached disk by occurrence count and a blob hash differing from HEAD's, and each restore by a blob equal to HEAD's plus an empty git diff HEAD; the whole script ran under trap ... EXIT INT TERM with absolute paths, and the final git status --porcelain was empty.

⚠️ Two honest deviations from the predictions, reported as observed:

  1. A1 reddened two cases, not one — the residue tripwire in ①b also fires when the envelope regresses, which is what it is for.
  2. Case ③ (anonymous) is green under A2 and cannot redden on it: the anonymous answer is null with or without the lift. It pins the residue, not the fix, and is named that way in the file.

A third correction, on the first attempt at case ④: the firing control was initially a deliberately-wrong seeded token. That unauthenticates the client, so /get-session answers null for the anonymous reason and the case fails against a correct implementation. It was rebuilt on the signed/unsigned spelling difference, which is a real measured difference that moves only when the capture works.

Type-level face

The card's first consequence is a type-level one, so the pin is a parse against the declared schema, not a key spot-check. me's annotation does not move in this diff, so the runtime assertions genuinely redden on the defect (A1/A2 above measure that they do). What could not have caught this: the session.data.user example in content/docs/permissions/authentication.mdx is not marked os:check, and even marked it would type-check both before and after — the annotation was right all along, the body was wrong.

Evidence

  • Gates — derived on the final tree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, 58 commands, reconciled with --ran: 58 derived, 58 run, 0 NOT-MEASURED, 0 UNRUN. All 58 exit 0. Two needed a build first and exited PREREQUISITE-NOT-MET (check:dual-build-cjs-loads exit 3, check:skill-examples exit 1, both naming an unbuilt dist); after pnpm --filter @objectstack/client build both measured green — 104 require entry points across 67 packages, and 258 prose examples type-checking. ⛔ Neither was read as a pass while unmeasured.
  • Lint — the repo-level run, not a narrowed one: eslint . --no-inline-config at c4cbdb0db6, exit 0, zero output lines.
  • Tests@objectstack/client: 40 files / 486 tests pass; typecheck clean, and tsc -p tsconfig.test.json --listFiles confirms index.ts, client.test.ts and the new suite are all in the compiled program (not merely assumed covered).
  • Consumerpackages/cli's whoami reads auth.me() through a response.data || response fallback written for this very defect; it now takes the data branch. @objectstack/cli unit tier: 190 files / 2635 tests pass. The integration tier is declared to CI — this diff touches no spawn entry point.
  • Every exit code above was captured by redirect-then-$?, never through a pipe.
  • Single-writer — measured from the open PR list (23 PRs), each against its own merge base via /pulls/{n}/files: no open PR touches packages/client/src/index.ts. Positive controls fire on the same predicate — packages/client/package.json names chore: version packages #17076, packages/spec/src/api/contract.zod.ts names feat(spec): ADR-0112 error envelope gains a producer-side refusal declaration so a deliberate 5xx refusal keeps its caller-authored message (#16335) #17090.
  • Docs-drift — five pages named; none goes stale and one is made true: content/docs/permissions/authentication.mdx already documented session.data.user, which was undefined before this change. That page is independent evidence that the declared envelope was the intended contract, i.e. that route 3 fixes the body rather than the promise. Checked by hand for the two blind spots the tool has: the prose class (sso.mdx, error-catalog.mdx — general envelope prose, untouched; references/** is auto-generated from a packages/spec this diff does not touch) and the docs/ tree the tool never walks (only HTTP-level QA checklist rows, which already record better-auth's 200-with-null convention). ⛔ No edit under content/docs/releases/**.

验收备注

Out of scope, filed rather than fixed here:

Noted, not filed:


Generated by Claude Code

… envelope

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
…se envelope both methods declare

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/client, touching 5 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via expiresAt (literal, a string literal in auth), auth.me (sdk, the route ledger binds it to GET /api/v1/auth/get-session))
  • content/docs/deployment/cli.mdx (via /api/v1/auth/* (route, a path literal on a changed line))
  • content/docs/deployment/self-hosting.mdx (via /api/v1/auth/* (route, a path literal on a changed line))
  • content/docs/kernel/contracts/auth-service.mdx (via expiresAt (literal, a string literal in auth))
  • content/docs/permissions/authentication.mdx (via expiresAt (literal, a string literal in auth), auth.me (sdk, the route ledger binds it to GET /api/v1/auth/get-session), /api/v1/auth/* (route, a path literal on a changed line), /api/v1/auth/get-session (route, a path literal on a changed line))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 559e531a94dab4938d1ab54fd6631acbc5303d61packageMentionDocs.

Which tree this was computed on

This run read content/docs from e2fe2f76e4d8b0b02118d9ebf7c183eef4d6198d — the merge of head c4cbdb0db6c01602bc1bd0e7df924a8ae27f443f into base 559e531a94dab4938d1ab54fd6631acbc5303d61, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e2fe2f76e4d8b0b02118d9ebf7c183eef4d6198d && git checkout e2fe2f76e4d8b0b02118d9ebf7c183eef4d6198d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 559e531a94dab4938d1ab54fd6631acbc5303d61 c4cbdb0db6c01602bc1bd0e7df924a8ae27f443f && git checkout -B drift-repro 559e531a94dab4938d1ab54fd6631acbc5303d61 && git merge --no-ff c4cbdb0db6c01602bc1bd0e7df924a8ae27f443f

node scripts/docs-audit/affected-docs.mjs --json 559e531a94dab4938d1ab54fd6631acbc5303d61

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 559e531a94dab4938d1ab54fd6631acbc5303d61 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-project-manager
os-project-manager marked this pull request as ready for review September 9, 2026 21:17
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit 5de9372 Sep 9, 2026
35 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-16760-get-session-shape-and-refresh-read branch September 9, 2026 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants