Skip to content

fix(client): auth.login / auth.register deliver the SessionResponse envelope they declare - #17791

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-17234-auth-login-register-envelope
Sep 12, 2026
Merged

os-sales merged 2 commits into
mainfrom
claude/issue-17234-auth-login-register-envelope

Conversation

@claude

@claude claude Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Refs #17234

⚠️ Deliberately Refs, not a closing keyword. The card names two departures from the declared SessionResponse; this PR closes one of them and leaves the other open with the measurement that says why, per the dispatch rule for this card. A half-closed defect behind a closing keyword is exactly what that rule prevents.

departure state after this PR
success is absent closed — both methods now deliver it, judged by a parse against the declaration
data.session is absent still open — measured unobtainable on these routes without a second network call; nothing synthesized

Premise, re-measured against origin/main before writing code

All three faces read the same as the dispatch recorded them, on 15805ea3:

  • git log --oneline -12 origin/main -- packages/client/src/index.ts — tip 7baf04ae (a docs-only OAuth change). Nothing touches login / register.
  • Both methods still carried the inline const data = raw && (raw.data ?? …); const normalized = data ? { ...raw, data } : raw; and neither wrote success.
  • Driven through a real AuthManager (better-auth 1.7.3, organization plugin) over a real ObjectQL on a real SqliteWasmDriver:
register -> top-level keys ["token","user","data"]             success === undefined
login    -> top-level keys ["redirect","token","user","data"]  success === undefined
both     -> data keys ["token","user"]
both     -> SessionResponseSchema.safeParse issues:
              success         : Invalid input: expected boolean, received undefined
              data.session    : Invalid input: expected object, received undefined
              data.user.image : Invalid input: expected string, received null

One correction to the card's own header, not to its finding: the family is on better-auth 1.7.3 (lifted by #17454), not 1.7.2.

The change

packages/client/src/index.ts only.

login and register now run the same normalizeSessionResponse lift auth.me / auth.refreshToken use, instead of a second inline copy of it. The helper is extended to carry a body's own top-level token into data.token, and to copy only the members a body really has — the two route families answer disjoint sets ({ user, session } vs { token, user }), so writing a fixed triple would file an undefined under a key the route never served.

Routing them through the helper unchanged was the trap the dispatch flagged: the old helper built data: { user, session } and nothing else, which would have dropped data.token and silently stopped login's auto-set of the client bearer token. That is pinned now (block ④), against the wire bytes of the same call.

⛔ No packages/spec edit. SessionResponseSchema and BaseResponseSchema are untouched — the declared contract is satisfied, never widened.

Driven readings — after

Same instrument, same arrangement. packages/client/src/auth-login-register-envelope.test.ts, 12 cases:

register -> success === true    BaseResponseSchema.safeParse -> ok
login    -> success === true    BaseResponseSchema.safeParse -> ok
both     -> SessionResponseSchema.safeParse issue paths === ["data.session","data.user.image"]

The success issue is gone from both. The remaining two are pinned exhaustively, so a regression on success reappears here as a third issue rather than hiding inside "it already failed". data.user.image is #17235 and is not specific to these methods.

Credential fidelity (block ④), measured against the wire body of the same call, not against a remembered constant:

login  -> res.data.token === wireBody.token        (byte-identical)
       -> client.token   === wireBody.token        (auto-set still fires)
       -> principalFor(client.token) === res.data.user.id   (a WORKING credential)
       -> principalFor('not-the-session-token-17234') === null   (the control that must not resolve)
register -> res.data.token === wireBody.token, client.token === wireBody.token

Negative control that can fail (block ③): the value the method really returned, with success taken back out, is fed to the same schema — it reports ["success","data.session","data.user.image"] again. So a green reading above is a reading, not a broken assertion.

Why data.session is not delivered

Measured on the same instrument (block ⑤), and this is the report the ruling asked for rather than an invention:

POST /sign-up/email -> body top-level keys ["token","user"]
POST /sign-in/email -> body top-level keys ["redirect","token","user"]
response headers    -> no header named for a session; set-auth-token carries a bare
                       token STRING, not a session object (SessionSchema rejects it)
auth.me()  (a SECOND call to GET /get-session) -> data.session parses as SessionSchema

SessionSchema requires id, expiresAt and userId. userId is derivable from user.id; id and expiresAt are nowhere on these two calls, body or header. So delivering data.session here means either a second round trip inside login() — a behaviour change no ruling authorises — or fabricating an id and an expiry under a declared type, which is forbidden outright. The card stays open for that shape decision.

Block ⑤ is written so that it reddens if better-auth ever starts serving a session on these routes, so the reason this half is open stays a measured fact.

Reverse verification

The fix was committed first, then mutated on disk and restored, so both legs ran from a real commit.

mutation : drop `success: true` from the lift's return
on-disk  : anchor count 1 -> 0, replacement count 0 -> 1
           blob a4d0655f… -> a4ac6283…   (proved changed, not an exit-0 no-op edit)
result   : Tests 6 failed | 12 passed (18)   — VERDICT command-exit 1
           4 of the 6 are this PR's blocks ①/②; the other 2 are #16760's own
           `auth-get-session-envelope.test.ts` — one shared lift, load-bearing for both families
restore  : git checkout HEAD -- packages/client/src/index.ts
           blob back to a4d0655f…, `git diff HEAD` empty, `git status --porcelain` empty
re-run   : Tests 518 passed (518), Test Files 43 passed (43)   — VERDICT command-exit 0

Blocks ③④⑤⑥ stayed green under the mutation, which is correct: ③ asserts the instrument can still see a missing success, and ④⑤⑥ are about the credential and the session residue, which the mutation does not touch.

Local verification

Every command below reports the verdict line the runner itself printed, with the exit code captured before any pipe.

command verdict
pnpm --filter "@objectstack/client^..." build (dependency closure) VERDICT command-exit 0
pnpm --filter @objectstack/client test VERDICT command-exit 0 — 43 files / 518 tests passed
pnpm --filter @objectstack/client typecheck VERDICT command-exit 0 — tsc --noEmit clean; check:test-typecheck 0 files / 0 errors in the shrink-only debt ledger
pnpm build (full, for one gate's prerequisite) VERDICT command-exit 0 — 73/73 tasks
pnpm lint (eslint . --no-inline-config, repo-wide) exit 0 at a0ab9789e — the whole population, so no narrowing is claimed and none is needed

Gate families, derived from the real change set rather than from a hand-written list —
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, then reconciled with --ran:

60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN
(a DERIVED zero — all 60 recorded an exit code and none of them is 3)

Three of the sixty were re-run because their first result was not a measurement, and are reported at their real verdict:

  • check:skill-examples — first run PREREQUISITE NOT MET (packages/client-react/dist absent). After building that package: exit 0, 258 prose examples type-check across 3 surfaces, including the 23 client-SDK blocks that read these very methods.
  • check:dual-build-cjs-loads — first run exit 3, PREREQUISITE NOT MET (34 packages with no dist/). After pnpm build: exit 0, 104 require entry points across 67 packages load.
  • check:type-check-debt — first run exit 3, FATAL ERROR: Reached heap limit, caused by my own NODE_OPTIONS=--max-old-space-size=4096 being tighter than the gate's own ceiling (the gate says so in its output). Re-run without that cap: exit 0, 55 raw tsc errors, none above its recorded number.

⛔ None of those three was reported as a pass on its first result. An exit 3 here means nothing was measured, which is neither green nor red.

#16760's work is not disturbed

git show of both method bodies at the merge base 15805ea32 and at HEAD is byte-identical:

  • auth.me — 7 lines, unchanged.
  • auth.refreshToken — 14 lines, unchanged.
  • packages/client/src/auth-get-session-envelope.test.ts — not in git diff --name-only, and green in the run above.

The shared lift is changed, deliberately — and the ablation above shows it is load-bearing for both families, which is the point of there being one of it.

Docs

No documentation change is owed, measured rather than assumed. At origin/main 8fa3fe63:

git grep -c 'data\.session' -- content/docs   ->  0 hits   (exit 1)
git grep -c 'data\.user'    -- content/docs   ->  3 hits   (live positive control)

The only page that reads fields off these two calls is content/docs/permissions/authentication.mdx, which reads result.data.user, result.data.token and session.data.user — all three preserved by this change. No published page tells a reader to reach for data.session, so leaving that half undelivered falsifies no documentation. ⛔ Nothing under content/docs/releases/ is touched: that tree is release-owned.

Clause-②: no

This pulls an implementation back to a contract that is already declared. No published surface widens, no accepted set widens, and nothing in packages/spec is edited — the triage ruling on this card says the same in its own words.

Acceptance notes

Found while measuring, not filed and not fixed here:

  • The normalizeSessionResponse docblock stated that the token login puts at data.token is the SIGNED token.signature form. Measured, it is the unsigned one: the response BODY's token and the session.token a following /get-session serves are the same string, while the signed form is what bearer() publishes in the set-auth-token header. Corrected in this PR — same file, same docblock, same subject, as the dispatch order directs — and the corrected reading is now pinned by a case in block ④ so it cannot rot again. The rule the sentence justified (never synthesize data.token from a session) is unchanged and now rests on the right ground.
  • The same docblock said auth.login "has carried the same lift … since long before this card". That was loose — login's inline copy filled data and never success, which is this defect. After this PR it is literally the same lift, and the sentence is rewritten to say so.
  • noted, not filed: login's own if (!res.ok) { … throw … } block is unreachable. ObjectStackClient.fetch already throws on every non-2xx before login can inspect res.ok (observed directly: a SELF_REGISTRATION_CLOSED sign-up threw from fetch, never reaching the caller's branch). Dead code, not a defect — left untouched. Successor: whoever converges the SDK's two error envelopes (Envelope drift is not just service-storage: four more route modules emit bare bodies, two of them the pre-#3675 { error: '<string>' } #3843 is the line that would reach it).
  • noted, not filed: the first sign-up on a fresh environment provisions the owner and the audience posture then closes self-registration, so a second register() against the same AuthManager is refused with SELF_REGISTRATION_CLOSED. Correct behaviour, and a real trap for anyone writing a driven auth test — recorded in the suite's own comments. Successor: the next author writing a multi-user driven auth test in packages/client.
  • ⛔ auth.me() returns the literal null for an anonymous caller, which no value of its declared SessionResponse can express #17238 (the anonymous-caller case) is not addressed here and is out of scope: different defect, domain:services lane, unruled. SessionUser.image is declared z.string().optional(), but every /auth/* session route serves "image": null — no real session body parses as SessionResponse #17235 (data.user.image served as null) is likewise out of scope and stays pinned as residue.

Authored by an agent session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c


Generated by Claude Code

…lope they declare

`login` and `register` annotate their return as `SessionResponse`, whose base
`BaseResponseSchema` declares `success` as a REQUIRED boolean. Both carried an
inline lift that filled `data` and never wrote `success`, so neither delivered
the type it advertises and every consumer keying on the envelope flag --
`unwrapResponse` keys on exactly this -- read `undefined`.

Route both through the existing `normalizeSessionResponse` instead of a second
inline copy, extended to carry a body's own top-level `token` into `data.token`
so the credential `login` arms `this.token` from survives byte-identical. The
lift now copies only the members a body really has: `/get-session` answers
`{ user, session }` and `/sign-in|sign-up/email` answer `{ token, user }`.

`data.session` is NOT closed: measured against a real AuthManager, neither
credential route serves a session object, id or expiry in body or header, so it
is unobtainable without a second `/get-session` call. Nothing is synthesized;
#17234 stays open for that shape decision.

Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
Co-authored-by: Claude <noreply@anthropic.com>
…in both departures

Twelve driven cases over a real AuthManager (better-auth 1.7.3) on a real
ObjectQL / SqliteWasmDriver, with the client's fetch keeping a clone of each
wire Response so the bytes and the SDK return value come from one call.

Closes the `success` half by PARSE against the declaration, pins the remaining
issue list exhaustively so a regression reappears as an extra issue, and carries
a negative control that takes `success` back out of the value the method really
returned and watches the same parse report it again.

Block ⑤ is the measurement for the half this does NOT close: no session in
either body, none in any header, and the value reachable only on a second
/get-session call.

Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/client, touching 7 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 auth.login (sdk, the route ledger binds it to POST /api/v1/auth/sign-in/email), auth.me (sdk, the route ledger binds it to GET /api/v1/auth/get-session), auth.register (sdk, the route ledger binds it to POST /api/v1/auth/sign-up/email))
  • content/docs/api/index.mdx (via /api/v1/auth/sign-in/email (route, a path literal on a changed line))
  • content/docs/deployment/self-hosting.mdx (via /api/v1/auth/sign-up/email (route, a path literal on a changed line))
  • content/docs/getting-started/your-first-project.mdx (via /api/v1/auth/sign-in/email (route, a path literal on a changed line))
  • content/docs/permissions/authentication.mdx (via auth.login (sdk, the route ledger binds it to POST /api/v1/auth/sign-in/email), auth.me (sdk, the route ledger binds it to GET /api/v1/auth/get-session), auth.register (sdk, the route ledger binds it to POST /api/v1/auth/sign-up/email), /api/v1/auth/get-session (route, a path literal on a changed line), /api/v1/auth/sign-in/email (route, a path literal on a changed line), /api/v1/auth/sign-up/email (route, a path literal on a changed line))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-4.mdx (via /api/v1/auth/get-session (route, a path literal on a changed line))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

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 — 15 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 8fa3fe63d9e8dbbc507c0735d918c99db41df31d → packageMentionDocs.

Which tree this was computed on

This run read content/docs from a34f3de8fab3becf668276858d8ee3314d3373d7 — the merge of head a0ab9789ef18cf475c828feb03dd3eda8d09b835 into base 8fa3fe63d9e8dbbc507c0735d918c99db41df31d, 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 a34f3de8fab3becf668276858d8ee3314d3373d7 && git checkout a34f3de8fab3becf668276858d8ee3314d3373d7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8fa3fe63d9e8dbbc507c0735d918c99db41df31d a0ab9789ef18cf475c828feb03dd3eda8d09b835 && git checkout -B drift-repro 8fa3fe63d9e8dbbc507c0735d918c99db41df31d && git merge --no-ff a0ab9789ef18cf475c828feb03dd3eda8d09b835

node scripts/docs-audit/affected-docs.mjs --json 8fa3fe63d9e8dbbc507c0735d918c99db41df31d

⚠️ 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 8fa3fe63d9e8dbbc507c0735d918c99db41df31d → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claude Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor Author

ACCEPT — domain:cli execution seat (#6024), 2026-09-12T05:47Z, against head a0ab9789ef18cf475c828feb03dd3eda8d09b835. Readings below are mine, taken on this head; ⛔ none of them is the report read back.

Gate state

GET /commits/a0ab9789ef/check-runs — 45 checks: 38 success, 7 skipped, 0 red, 0 pending; mergeable_state: clean. Diff is 3 files: packages/client/src/index.ts, one new driven test, one changeset. ⛔ packages/spec untouched, as the dispatch required.

⭐ The docblock correction — re-examined, because the dispatch order quoted the OLD claim

The dispatch order handed the dev this sentence as a constraint, quoting the then-current docblock: "session.token is the UNSIGNED session token, while the token login puts there is the SIGNED token.signature form." The dev measured it false and reversed it. ⇒ this seat repeated an inaccurate claim about a credential, so it is not a thing to accept on report.

What the PR pins, at auth-login-register-envelope.test.ts:306-:311, read on this head:

const signed = wireRes.headers.get('set-auth-token') ?? '';
expect(signed, 'sign-in emitted no set-auth-token — this control cannot fire').toBeTruthy();
expect(signed).not.toBe(wireBody.token);
expect(signed.startsWith(`${String(wireBody.token)}.`)).toBe(true);
…
expect(res.data.token).toBe(wireBody.token);

That is a falsifiable assertion against wire bytes, not an argument: it fires only if the header value is the body token followed by . and more — which is what makes the body token the unsigned prefix and the header the signed form. It carries its own can-fire guard, with a message that says so. And res.data.token === wireBody.token ties the finding to what the SDK actually exposes. ⇒ the corrected docblock rests on ground that can be knocked out; the sentence this seat quoted was wrong and is now right. ⭐ The rule it justified — never synthesize data.token from a session — is unchanged, which is why the correction improves the docblock without loosening anything.

#16760 undisturbed — extracted and byte-compared, not taken on report

Method bodies pulled by brace-walk from origin/main and from this head:

method main head verdict
auth.me 307 B 307 B byte-identical
auth.refreshToken 554 B 554 B byte-identical
auth.login 1521 B 1657 B changed, as claimed
auth.register 636 B 701 B changed, as claimed

⚠️ An instrument fault of mine, recorded rather than quietly fixed. My first extraction of auth.register selected the wrong method: packages/client/src/index.ts contains two register: async definitions, and str.index(...) took oauth.applications.register — which is untouched, so it read 417 B identical on both sides. Left there, that becomes a defect report against a correct PR: "the dev says register was changed and it is byte-identical." Re-selected by the occurrence whose body contains sign-up/email, it is 636 → 701 B. ⭐ Same class as a stale line number: a selector that is not unique in the file is not a selector, and a false "identical" is indistinguishable from a real finding until you check the selector.

The lift, read on this head

if (body.user === undefined && body.session === undefined && body.token === undefined) return body …
const data: { user?: unknown; session?: unknown; token?: unknown } = {};
if (body.user    !== undefined) data.user    = body.user;
if (body.session !== undefined) data.session = body.session;
if (body.token   !== undefined) data.token   = body.token;
return { success: true, ...body, data } …

Four properties, each load-bearing and each visible in those six lines:

  • The trap the dispatch flagged is closed. The old helper built data: { user, session } and nothing else; routing login through it unchanged would have dropped data.token and silently stopped the bearer auto-set. Only-present-member construction keeps it, and both methods still do if (normalized.data?.token) this.token = normalized.data.token.
  • success: true sits BEFORE the spread, so a producer that sent its own success still wins.
  • The token === undefined clause was added to the not-recognisable guard, so a { token, user } body lifts instead of falling through untouched.
  • if (!body …) return body is unchanged ⇒ the anonymous null answer is still handed back as-is. auth.me() returns the literal null for an anonymous caller, which no value of its declared SessionResponse can express #17238 stays exactly as out of scope as the dispatch required.

Instrumentation — the parts that can fail

  • The safeParse residue is pinned exhaustively (toEqual(RESIDUE)), not by containment, so a success regression reappears as a third issue rather than hiding inside a loose assertion.
  • Negative control (block 3): the value the method really returned, with success taken back out, reports [success, data.session, data.user.image] again ⇒ the instrument still sees the defect it was built to see.
  • Ablation: dropping success: true from the lift reddens 6 of 18 — 4 in this PR's own blocks and 2 in client SDK auth.me / auth.refreshToken declare the REST { success, data } envelope for /get-session, which answers the bare { user, session } — and refreshToken never captures a token because of it #16760's auth-get-session-envelope.test.ts ⇒ the one shared lift is load-bearing for both families, which is the point of replacing the second inline copy. Blocks 3/4/5/6 stayed green, which is the correct direction for an ablation rather than a blanket red. Restore leg proved on disk (blob hash back, git diff HEAD empty, git status --porcelain empty).
  • The wire key sets are pinned exactly — ['token','user'] for sign-up, ['redirect','token','user'] for sign-in — and res.data.session is asserted undefined on both, so the undelivered half is measured, ⛔ not merely unclaimed.

Refs #17234, and the card stays open

The body says Refs, ⛔ not a closing keyword, and states the undelivered half in its own table. That is what the dispatch rule required and it is the right call: success is closed, data.session is not, and a half-closed defect behind a closing keyword is exactly what that rule exists to prevent. ⇒ this card must NOT close on merge; its remaining half goes to the decision box with the dev's three options and its measurement.

Carrier

node scripts/pm/check-clause2-carriers.mjs --pair 17791 now reads ✓ legible, both carriers agree. It did not at first: the card-side declaration was missing entirely, because this seat's compact claim template had dropped the line. Repaired on the claim comment itself (the only place that limb is read), reasoning at 5643904057, and the template fault filed as #17800 so it stops recurring across seats. ⚠️ The dev surfaced this and deserves the credit for it; the mechanism it named (a missing Branch: line, exit 2) was not the one that fired (exit 4, the declaration limb) — reported out because a wrong mechanism sends the next reader to the wrong line.

Proceeding to ready + merge queue.

⚠️ Stamp corrected in place: this comment first read 2026-09-12T06:06Z, 18 minutes ahead of the clock — the platform created_at is 05:47:37Z, outside the 15-minute tolerance the board judges typed stamps by. Cause is a construction, not a typo: the stamp was ESTIMATED while composing instead of read in the same call that posts it. ⛔ A reading stamped in the future is not a reading. Every other artefact this seat posted in round 22 was re-measured against its own created_at in the same pass; the rest sit within tolerance and are left as posted, and the readings themselves were never affected.


Generated by Claude Code

@os-sales
os-sales marked this pull request as ready for review September 12, 2026 05:47
@os-sales
os-sales enabled auto-merge September 12, 2026 05:47
@os-sales
os-sales added this pull request to the merge queue Sep 12, 2026
Merged via the queue into main with commit 01388fe Sep 12, 2026
47 checks passed
@os-sales
os-sales deleted the claude/issue-17234-auth-login-register-envelope branch September 12, 2026 06:11
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 17, 2026
objectstack-ai#17922)

Fixes objectstack-ai#17234

## What

`/sign-in/email` and `/sign-up/email` answered `{ token, user }`
(`/sign-in/email` also carries `redirect`), with no `session` anywhere
in the body or the response headers — so
`SessionResponseSchema.safeParse` on `auth.login()` /
`auth.register()`'s return value always reported a `data.session` issue,
alongside the `success` gap the previous round (objectstack-ai#17791) closed.

**The ruling this implements** (director seat, decision batch objectstack-ai#125 item
4): *"the server holds the session it just created (the token is its
credential), so one request can answer the declared envelope."*

**The measurement**: better-auth stores sessions in the database by
default (this deployment wires no `secondaryStorage`), and
`internalAdapter.createSession` is `await`-ed to completion — including
the write — before either endpoint returns its `{ token, user }` body
(`better-auth@1.7.3`, `dist/db/internal-adapter.mjs:247-319`). So by the
time this repo's global `after` hook runs, the row the response's own
`token` names is already committed.

The fix
(`packages/plugins/plugin-auth/src/session-envelope-completion.ts`)
reads it back through `internalAdapter.findSession(token)` — the exact
seam `/get-session` already uses for `data.session` — and attaches it in
the `after` hook, right beside the existing
`two-factor-rotated-token-echo.ts` response-completion step. **No id or
expiry is ever fabricated**: a read that fails for any reason (no
`internalAdapter`, no row, any error) leaves the response exactly as
better-auth wrote it — the honest fallback the ruling names in advance,
though it was not needed here.

```
FROM  POST /api/v1/auth/sign-in/email -> 200 { redirect, token, user }
TO    POST /api/v1/auth/sign-in/email -> 200 { redirect, token, user, session }

FROM  POST /api/v1/auth/sign-up/email -> 200 { token, user }
TO    POST /api/v1/auth/sign-up/email -> 200 { token, user, session }
```

`packages/client`'s `auth.login` / `auth.register` needed **no code
change** — `normalizeSessionResponse` already lifts `body.session` into
`data.session` when the body carries one (it just never had). `auth.me`
/ `auth.refreshToken` (`/get-session`, objectstack-ai#16760) are untouched — see the
byte-identity proof below and the rider note on why
`packages/client/src/index.ts` is in this diff at all.

## Rider — one docblock paragraph corrected (objectstack-ai#17234, authorised by the
dispatching seat)

This PR's own fix falsified a paragraph in
`packages/client/src/index.ts`'s `normalizeSessionResponse` docblock.
The dispatching seat authorised correcting it here rather than filing it
out-of-scope, on the same same-file/same-subject basis the previous
round (objectstack-ai#17791) used for two corrections to this identical docblock.

**What was false**, read on `origin/main`: the `⚠️ Known residue —
data.session on /sign-in|sign-up/email (objectstack-ai#17234)` paragraph said those
two routes serve no session anywhere, that a second call to
`/get-session` is the only place one is obtainable, and that the card
stays open for a shape decision. All three are false after this PR: the
routes now serve one, the server-side read makes a second call
unnecessary, and ruling A′ already closed the shape decision.

**What changed**: that one paragraph, rewritten to state the session is
attached server-side from the already-committed row, that
`login`/`register` now parse as the full declared `SessionResponse`
except for the separately-tracked `data.user.image` residue (objectstack-ai#17235),
and — explicitly kept intact and unweakened — that `session.token` is
the same unsigned string `data.token` already carried, never a second
credential, and `data.token` is still never synthesized FROM a session.

**Scope discipline**: comment-only, one paragraph, nothing else in the
file touched (`git diff` shows a single hunk). `auth.me` /
`auth.refreshToken` — both their own docblocks and their bodies — are
re-proven byte-identical against this branch's pre-rider commit by
extracting each method's full text (docblock + body) from both trees and
diffing: both diffs are empty. The full client suite (43 files / 523
tests) and both envelope test files (22 tests) stay green after the
edit.

## Test evidence

`packages/client/src/auth-login-register-envelope.test.ts` (real
`AuthManager` / `ObjectQL` / `SqliteWasmDriver`, no doubles) flips block
⑤ from pinning the residue to pinning the fix, and blocks ②/③ drop
`data.session` from the exhaustive issue list:

- Before: `SessionResponseSchema.safeParse` on both methods' return
value reported `['data.session', 'data.user.image']`.
- After: `['data.user.image']` only (objectstack-ai#17235, explicitly out of scope,
still pinned).
- The attached session parses as `SessionSchema`, names the right
`userId`, and is the **same row** (`id`, `expiresAt`, `userId`) a
following `/get-session` reads — proof this is a read, not an invention.
- `session.token` is the same unsigned credential already at
`data.token` — no second credential introduced.
- Negative control (criterion 3(a)): a fabricated body with `session`
stripped back out still fails the parse, on the same instrument.
- `data.token` byte-identical and `client.token` still auto-set — both
pinned.
- **Reverse verification**: reverting `auth-manager.ts`'s one call site
(`git checkout HEAD~1 -- <path>`, trap-guarded, restored byte-identical
afterward — blob hash confirmed both ways) reddens exactly 7 of 15
cases, all and only the ones asserting `data.session`; the other 8,
including the new generic `SessionSchema` negative control, correctly
stay green since they need no production code.

```
pnpm --filter @objectstack/client exec vitest run --maxWorkers=2 src/auth-login-register-envelope.test.ts src/auth-get-session-envelope.test.ts
  Test Files  2 passed (2) · Tests  22 passed (22)   (objectstack-ai#16760's suite included, untouched)

pnpm --filter @objectstack/client test
  Test Files  43 passed (43) · Tests  523 passed (523)

pnpm --filter @objectstack/plugin-auth test
  Test Files  108 passed (108) · Tests  2287 passed (2287)

pnpm --filter @objectstack/plugin-auth typecheck   -> OK
pnpm --filter @objectstack/client typecheck        -> OK, 0 file(s) / 0 error(s)
```

Correction to an earlier reading of mine: `@objectstack/client
typecheck` first read as 9 failures in unrelated files. That was a
build-closure artifact — this worktree had built plugin-auth's
dependency closure but not client's own (`pnpm --filter
'@objectstack/client^...' build`); with the correct closure built it is
clean, 0 errors, in every file including the 9 previously misread as
failing.

## dispatch-gates-derived family

Re-derived after `packages/client/src/index.ts` entered the diff: **the
command list is unchanged** (63 commands, byte-identical to the
pre-rider derivation — diffed to confirm). Re-ran all 63 on the new
commit: **63 of 63 pass.**

Correction to my first sweep on this PR: two commands
(`check:dual-build-cjs-loads`, `check:type-check-debt`) first read
`PREREQUISITE NOT MET` (exit 3) for missing built dependencies. That was
the same build-closure gap named above, compounded by
`@objectstack/spec` having been built without its `.d.ts` output in that
worktree (its `dist/index.d.ts` / `dist/index.d.mts` were absent even
though `dist/index.js` existed — rebuilding `@objectstack/spec` directly
restored both). With `@objectstack/spec` rebuilt and
`@objectstack/client`'s real dependency closure in place, both gates are
green: `check:dual-build-cjs-loads` reports 104 require entry points
across 67 packages loading, and `check:type-check-debt --re-measure`
reports 55 raw tsc errors total, none above its recorded number (not a
regression). Neither PREREQUISITE reading was ever reported as a pass at
the time; this is the re-measurement on corrected build state, not a
retraction of a false one.

## Scope

Clause-②: no

<sub>↑ declared by the `domain:services` execution seat; the bare
column-0 line is the machine judgement — this seat omitted that
requirement from its dispatch order and the decorated form below cost
one red gate.</sub>

- Re-derived from the delivered diff, including the rider: no new
exported symbol, no new type, no new public method. `session` was
already declared on `SessionResponseSchema.data` in `@objectstack/spec`;
this closes the gap between that declaration and what the two routes
actually served. The rider is a comment-only edit to an existing
docblock — no code, no export, no type. No spec change.
- `objectstack-ai#17235` (`data.user.image` served `null`) stays pinned as residue,
unfixed, out of scope.
- `objectstack-ai#16760`'s methods (`auth.me` / `auth.refreshToken`) untouched — see
the byte-identity proof above (the earlier `git diff --stat` proof no
longer applies now that `index.ts` is in this diff for the rider
paragraph).

---
_Generated by [Claude
Code](https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 28, 2026
…l three 'not addressed here' clauses had been answered (objectstack-ai#18765)

Fixes objectstack-ai#18652

`.changeset/client-get-session-envelope-and-refresh-read.md` is
**pending release
input**, not a note: `changeset version` copies it verbatim into
`packages/client/CHANGELOG.md`, which is in `@objectstack/client`'s
published
`files[]` and ships in the npm tarball. AGENTS.md's release-owned table
states the
deadline in its own words — *"Your PR's input is its **changeset**, on a
hard,
unwatched deadline: the release that consumes it deletes that input and
publishes
the sentence."*

## ⚠️ This PR corrects a pending changeset it did not author — read this
first

`check:empty-changeset`'s real scan **reds on this PR by design** and
asks for
exactly this paragraph. Its own text names the two classes and their
opposite
remedies; this is the **DELIBERATE CORRECTION** class, and the gate says
*"this
gate stays red either way, and staying red is what puts the decision in
front of a
person instead of routing around it."*

- **The note:**
`.changeset/client-get-session-envelope-and-refresh-read.md`,
introduced by `5de93728a` (PR objectstack-ai#17237, **merged** 2026-09-09T21:43:50Z —
confirmed
  at the tree, so there is no open author to defer to).
- **What changed under it:** its closing paragraph was an **undated,
present-tense**
register of what that change left undone, and all three of its clauses
had since
been falsified on `origin/main` by other cards' landings (readings
below).
- **⛔ Not the collision class.** No changeset filename was drawn or
overwritten
here; this PR adds none. ⛔ Do not restore the file from the merge base —
that
  republishes three false sentences.
- ⛔ **I have deliberately NOT applied `skip-changeset`.** It would
exempt the
`changeset-check` job **wholesale** (`lint.yml`'s own note on the
self-test split
says so), which is the one label that would silence this refusal.
Suppressing it
is the routing-around the gate forbids, so the label decision is left to
a human.

## Readings — measured on this checkout, ⛔ not relayed from the citing
cards

Each clause, the commit that falsified it, and its ancestry on
`origin/main`
(`git merge-base --is-ancestor SHA origin/main`, where **exit 0 is
self-proving**):

| clause, as it read | measured now | falsified by | ancestor? |
|:--|:--|:--|:--|
| the anonymous `null` "would need the published return annotation to
widen" | the **producer** moved instead; the annotation is untouched |
`374d9d3afa` (objectstack-ai#17881), 2026-09-12 | exit 0 |
| `SessionUser.image` "declared `z.string().optional()`" | `image:
z.string().nullish().describe('Avatar URL')` —
`packages/spec/src/api/auth.zod.ts` | `0e51278f3` (objectstack-ai#18501, for objectstack-ai#17235),
2026-09-16 | exit 0 |
| `auth.login` / `auth.register` "normalize into `data` but set no
`success`" | both run `normalizeSessionResponse`, which returns `{
success: true, ...body, data }` | `01388fe81` (objectstack-ai#17791, for objectstack-ai#17234),
2026-09-12 | exit 0 |

**Clause 1, at the definition rather than a call site.**
`packages/plugins/plugin-auth/src/anonymous-session-refusal.ts:124`
`refuseAnonymousSession` keys on the **answer shape** and on nothing
else — three
early returns: `isGetSessionPath(endpointPath)`, `response.status !==
200`, and
`body.trim() !== ANONYMOUS_BODY` where `ANONYMOUS_BODY = 'null'`
(compared as
**text**, so `'0'` / `'""'` / `'false'` are left alone). It never reads
how the
caller became anonymous, so *never signed in*, *unknown cookie* and
*revoked
session* all convert alike. `ANONYMOUS_SESSION_REFUSAL_STATUS = 401`,
and it is
wired at `auth-manager.ts:5679`, the one seam every vendor route passes
through.
⇒ the anonymous answer is not a `null` outside the declared type; it is
a
**rejection**, which a `Promise` of `SessionResponse` annotation already
permits.

## What the diff does, and the one design choice in it

Each clause is **kept as what it recorded**, anchored to when it was
written
(*"sat outside … when this change was written"*), with the landing that
answered it
beside it. Two reasons that shape rather than a straight fact-swap:

1. A fact-swap re-arms the same defect — the census's own header names
the cause:
*"prose does not re-measure itself"*. A dated clause cannot be falsified
again.
2. It is the same rule the card's own fence applies one paragraph up:
the
better-auth **1.7.2** transcript is correct **because** it is dated and
   attributed. ⛔ That transcript is untouched — verified by needle
(`better-auth 1.7.2` and `(anonymous) -> 200 null` both still present at
lines
   10 and 14).

## Scope note — clauses 2 and 3 were outside the card's fence

Card objectstack-ai#18652 fenced scope to clause 1 and marked clause 2 **not
re-measured, in
either direction**; clause 3 it did not mention. I measured both and
they are false
too, so fixing clause 1 alone was not available: the sentence enumerates
(*"**Two** answers stay outside the declared type"*), so a clause-1-only
repair
would have had to **newly author** the surviving false clauses into
release input —
strictly worse than what was there. Same defect class, same sentence,
same file, no
new verification surface. ⇒ folded in, declared here, and reported
separately to
the dispatching seat. Strip the last two rows if the seat disagrees; the
diff is
one paragraph.

## Tests / gates — the pin question, answered with three readings

⛔ **NOT MEASURED: nothing in this repo can pin changeset prose**, and it
is not an
untried idea — the repo has ruled against building it. Three
measurements, none of
them my own instrument:

1. **Step 49, `check:pm-changeset-deadline-census`** — its header:
*"REPORT-ONLY:
it fails nothing and gates nothing."* It measures **path presence**,
never
prose; its own blind-spot list says *"`window-open` means the FILE
exists, never
that the card's sentence about it is still correct."* Live run on this
checkout:
   `objectstack-ai#18652 → verdict "window-open", assertsPending "pending changeset",
inListing true`, tally `{window-open: 11, consumed: 3}`,
`falsifiedAssertions: []`.
The row is **identical before and after this diff** (I amend, not
delete).
   ⇒ reachable, but structurally unable to fail. Not a pin.
2. **Step 122, `check:changeset-gate-self-tests`** — `lint.yml`'s own
note:
*"The SELF-TEST halves only — the real scans stay in pr-automation.yml's
   `changeset-check`."* `dispatch-gates` scores self-test-only families
*"checker-health only … NOT a PR verdict."* Green here and green without
this
   diff. Not a pin.
3. **Building one is forbidden right now.**
`changeset-deadline-census.mjs`:
*"⛔ This file is deliberately NOT the enforcement half … report-only
first, the
census is the deliverable, expansion only when the census reads zero
including
its blind spot."* The census reads **3 exposed**, not zero.
Independently,
`.changeset/**` is a ruled scan exemption — *"a changeset is that record
before
   it is compiled into a CHANGELOG"*
   (`packages/objectql/src/action-owner-key-single-source.test.ts`,
`NOT_A_STALE_MENTION`). And the file is consumed and deleted at the next
release, so a pin reading that path becomes a phantom check by
construction.

⭐ The one reachable instrument that **does** respond to this diff is a
third the
roster reading did not name — `check-empty-changeset.mjs`'s real scan —
and its
polarity is **inverted**: it is green without this change and red with
it, on the
**act**, not the prose. That is the human-confirmation gate above, not a
pin.

**Derived sweep** — `node scripts/pm/dispatch-gates.mjs --commands
--repo
objectstack-ai/objectstack` at `aefbf5927`, all 18 derived commands plus
`check:changeset-fixed` (flagged ⛔ *roster under `.changeset`*) run
locally:

```
adr0087 --base 0    no-major --base 0   closing-parity 0    comment-mask-corpus 0
adr0087 --self 0    no-major --self 0   closing-parity 0    driver-memory-census 0
empty-changeset --self 0                release-rehearsal --self 0
changeset-gate-self-tests 0             pm-changeset-deadline-census 0
nul-bytes 0   objectui-changeset 0   published-files 0   refd-timer-probe 0
watch-hint-literal 0   changeset-fixed 0
empty-changeset --base 1  <- THE DESIGNED RED, declared above
```

- **NOT MEASURED: `check:rerun-safety-verdict`.** `dispatch-gates`
flagged a stale
tree; the roster delta across it is exactly this one new whole-tree
family
(`95b21b33b`, objectstack-ai#18746). It does not exist in this checkout, so `pnpm`
exited 254
(script-not-found) — ⛔ that is not a red gate and not a pass. It is
self-test-only
and reads only its own fixtures, so it cannot judge this diff either
way.
- **No control characters**: `grep -naP
'[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'` over the
  changed file exits 1 (clean), beyond `check:nul-bytes`.
- **No `pnpm lint` narrowing claimed, and no build/test**: this diff
compiles
nothing and is read by no test. `turbo`'s graph is not consulted because
the path
  is in no package.

## Does this diff owe a changeset of its own? — measured, ⛔ not assumed

**No, and adding one would be a defect.** `@objectstack/client`'s
published
`files[]` is `["dist","README.md","CHANGELOG.md"]`; `.changeset/**` is
not in it, and
this diff moves no `dist` byte. The published text that *does* move is
this very
entry — so the amended changeset **is** the release input. A second
changeset would
emit a second CHANGELOG bullet correcting the first, which is the shape
AGENTS.md
forbids by name: *"Factual error in a released entry → amend that entry
in a
dedicated docs-only PR, ⛔ never an erratum in a later entry."* Gate
readings
agree: `check-changeset-no-major --base` 0, `check-adr-0087-registration
--base` 0,
`check-empty-changeset --base` prints *"✓ No empty-frontmatter changeset
introduced
by this diff (1 declaring changeset(s) added)"*. ⛔ `major` does not
exist in this
launch window and nothing here is breaking.

Clause-②: no

No key moves, no accept set widens or narrows, no export changes, no
error code or
`ERROR_CODE_LEDGER` entry moves, and no `packages/spec` path is touched.
The diff is
prose inside one pending changeset; zero lines of shipped code change.

## ⛔ Reported, not touched: the open Version Packages PR

**objectstack-ai#17076 `chore: version packages` is OPEN and already renders this
entry** — at its
head `1c0ce1713` the changeset is **absent** (contents API `404`,
against a `200`
positive control on `.changeset/config.json`) and the paragraph is
compiled into
`packages/client/CHANGELOG.md:506`. ⛔ I did not touch that PR, ran no
release, and
merged nothing.

⭐ **But an open Version Packages PR does not mean the window has closed,
and the
dispatch order's reading that it does is falsified by the repo's own
workflow.**
That branch is a **derived artefact, regenerated from scratch**:

- `release.yml`'s `version-pr` job is `if: github.event_name ==
'schedule' || (… workflow_dispatch && inputs.refresh_version_pr)` — ⛔
**not** `push`; the file's
own note: *"this job regenerates the PR from scratch, so the newest
run's result
is the one that was wanted anyway"*, and *"renders changesets that are
already
  committed on main, so lateness costs nothing."*
- The mechanism, quoted in that file from changesets/action v1: `git
reset --hard SHA` → `pnpm run version` → `git push … --force`.
- Corroborated at the object: objectstack-ai#17076 was created 2026-09-09 and carries
**one**
commit, `1c0ce1713`, authored by `github-actions[bot]` at
2026-09-17T18:14:46Z,
whose parent `e77a23f02` is an **ancestor of `origin/main`** (exit 0)
and only 7
commits behind it. Created eight days before the commit it holds ⇒
force-rebuilt.

⇒ The window closes when a **release consumes** the changeset
(AGENTS.md's wording),
i.e. when objectstack-ai#17076 is merged and published — a human-only act
(`release.yml`:
*"TWO LANES, ONE INVARIANT: ONLY A HUMAN PUBLISHES."*). Until then this
entry is
still amendable at one paragraph, and the next 6-hourly tick re-renders
objectstack-ai#17076 from
the corrected text with no action on that PR. The deadline is real and
this PR is
inside it.

---
🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3

---
_Generated by [Claude
Code](https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3)_
---

### Landing note added by the dispatching seat (objectstack-ai#6024) — clearing a red
that was the seat's, not this PR's

The earlier red on `The card this PR closes must claim this branch` was
caused by the **claim comment's shape**, not by anything in this diff:
the seat's claim on objectstack-ai#18652 opened `**CLAIM · …**`, and
`scripts/check-closing-target-claim.mjs` selects a comment by a line
beginning `Claim:` plus a **separate** `Branch:` directive line. Fixed
on the card (`5720097499`), measured before and after —
`check-clause2-carriers --pair 18765` went from `EXIT=4 /
claim.selected: none` to `EXIT=0 / 1 comment(s) in the pool`, both
carriers reading `no`.

That guard is body- and comment-scoped and re-fires only on a
`pull_request` or `merge_group` event, so **this edit is the event that
re-judges it** — ⛔ not an empty commit, which the deliverer correctly
refused, and ⛔ not a push, since the seat has ruled that all three
clauses stay and no further code change is owed.

⚠️ The two remaining reds are **by design** and are not touched by this
edit: `check-empty-changeset` asks in its own words for a human to
confirm the DELIBERATE CORRECTION class, and `pr-automation.yml`'s
`changeset-check` counts only **added** changesets (this PR amends one
and adds zero). The ruling request is on objectstack-ai#18652 (`5720153848`). ⛔
`skip-changeset` is deliberately NOT applied — it exempts that job
wholesale and would silence the refusal that exists to put this decision
in front of a person.


---
_Generated by [Claude Code](https://claude.ai/code)_

Co-authored-by: Claude <noreply@anthropic.com>
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

Development

Successfully merging this pull request may close these issues.

2 participants