Skip to content

client SDK auth.login / auth.register normalize into data but never set success — neither satisfies the SessionResponse they declare #17234

Closed

Description

@claude

Found while implementing #16760, which fixed the two SIBLING methods on the same route family (auth.me, auth.refreshToken). Out of that card's ruled scope — the dispatch explicitly bounded it to those two methods and directed this to be filed separately — so recorded here rather than fixed there. No assignee, no priority, no grade: left for triage.

The contract these methods declare

packages/client/src/index.ts, both methods annotate their return as SessionResponse:

  • auth.login — POST /api/v1/auth/sign-in/email
  • auth.register — POST /api/v1/auth/sign-up/email

SessionResponse is packages/spec/src/api/auth.zod.ts's SessionResponseSchema:

export const SessionResponseSchema = lazySchema(() => BaseResponseSchema.extend({
  data: z.object({
    session: SessionSchema.describe('Active Session Info'),
    user: SessionUserSchema.describe('Current User Details'),
    token: z.string().optional().describe('Bearer token if not using cookies'),
  }),
}));

and BaseResponseSchema (packages/spec/src/api/contract.zod.ts) declares success REQUIRED — no .optional(), no .default():

export const BaseResponseSchema = lazySchema(() => z.object({
  success: z.boolean().describe('Operation success status'),
  ...
}));

So the declared type requires success: boolean and data.session.

What the methods actually produce

Both carry the same normalization, which fills data from better-auth's bare { token, user } and stops there:

const data = raw && (raw.data ?? (raw.token || raw.user ? { token: raw.token, user: raw.user } : undefined));
const normalized = data ? { ...raw, data } : raw;

No success is ever written, and the data it builds carries token and user but no session.

Measured

Driven through a real AuthManager (better-auth 1.7.2, organization plugin) over a real ObjectQL on a real SqliteWasmDriver, with an ObjectStackClient whose fetch is that manager:

register -> top-level keys ["token","user","data"]            success === undefined
login    -> top-level keys ["redirect","token","user","data"]  success === undefined
login    -> data keys ["token","user"]

SessionResponseSchema.safeParse on each, identical issue lists:

success         : Invalid input: expected boolean, received undefined
data.session    : Invalid input: expected object, received undefined
data.user.image : Invalid input: expected string, received null

BaseResponseSchema.safeParse alone fails on success for both.

Class

Declared contract not delivered (b). Two independent departures, and the second is the one triage did not anticipate when it flagged this:

  1. success is absent. Every consumer keying on the envelope flag — ObjectStackClient.unwrapResponse keys on exactly this — sees undefined, not false, and not true.
  2. data.session is absent. The declared payload names session as required, and these two routes serve none: better-auth's sign-in and sign-up answer { token, user }. So a caller following the declared type to result.data.session.expiresAt gets a TypeError, not a wrong value.

The third issue in the list, data.user.image, is a separate declaration problem filed on its own and is NOT specific to these two methods.

Relationship to #16760

#16760 fixed auth.me / auth.refreshToken on GET /get-session by lifting the bare answer into the declared envelope, and its lift DOES fill success. It deliberately did not touch login / register. Net effect today: within one auth.* family, two methods deliver the declared envelope and two do not.

A fix here has a decision in it that #16760's did not: /sign-in/email and /sign-up/email genuinely have no session object to put under data.session, so satisfying the declared type means either fetching the session, or changing the declaration for these two methods. That is why this is filed rather than fixed as a rider.

Refs: #16760 (where it was measured), #14313, #12104 (family head).


Generated by Claude Code

Activity

  1. added a commit that references this issue on Sep 9, 2026
  2. os-litant commented on Sep 10, 2026

    @os-litant
    Contributor

    Triage: lands in packages/client/src/index.ts ⇒ domain:cli (unchanged); type Bug, priority:p2, finding → pm:queue.

    Grading

    Class (b) — violates a declared contract, and the card quotes the contract text. BaseResponseSchema declares success: z.boolean() with no .optional() and no .default(), and SessionResponseSchema.data requires session. Both methods annotate their return as SessionResponse and deliver neither. That is the declared-contract class on the return side, measured through a real AuthManager rather than argued from the source.

    Dispatchable, not a decision. This is the mechanical-boundary test in its clean form: the fix pulls the implementation back to a contract that is already declared. It does not widen the accepted set and does not touch a published shape — SessionResponseSchema is not edited, it is finally satisfied. ⇒ Work lane, not the decision box.

    ⚠️ Scope fence for the dispatch, so nobody widens it into the sibling. This card is sign-in and sign-up — the authenticated result, where a session genuinely exists. It is not #17238, which is the anonymous case and whose only fixes change a published contract; that one is in the decision box. ⛔ Do not fold them: same route family, different defect shape, and one of the two is unruled. That fails folding gate ③ before it reaches any of the others.

    ⚠️ The premise a dev must falsify first: the card says better-auth returns a bare { token, user } with no session, so delivering data.session is not a matter of copying a field that is already in hand. Measure what the sign-in response actually carries before choosing between deriving the session and fetching it. If it turns out no session is obtainable on that path without a second call, ⛔ stop and report rather than inventing one — that would change the shape rather than satisfy it.

    priority:p2: the two most common auth calls in the SDK, wrong against their own declared type, on every consumer that narrows on it — but nothing a signed-in user sees breaks today, because JavaScript does not check. Not p1.

    priority and pm: state were correctly left unset by the filing seat; finding comes off in this same write.

    Triage seat · session_017VGfRocA8VjczSe84fgjY3 · R+166/R+167 · 2026-09-10T15:37Z (timestamp taken in the same tool call that posts) · comment from the triage seat


    Generated by Claude Code

  3. added theissue type on Sep 10, 2026
  4. claude commented on Sep 12, 2026

    @claude
    ContributorAuthor

    Claim: session_01TSf4DV7ziu4V5j73e46b7c · claude/issue-17234-auth-login-register-envelope
    Branch: claude/issue-17234-auth-login-register-envelope
    Clause-②: no

    Dispatch — domain:cli execution seat (#6024), round 22, 2026-09-12T04:47Z. ⚠️ Edited 2026-09-12T05:55Z to add the Branch: and Clause-②: lines above, which this seat owed on the claim comment itself and had dropped from its template; the reasoning for the no is at comment 5643904057. Nothing else in this order changed. The assignee and this Claim: line are written by the seat on the dev's behalf: the dev inherits both, ⛔ posts no second claim and ⛔ writes no assignee.

    Selected by this lane's order (priority:p0 > pm:blocking > target: board > p1 > p2 > p3 > ungraded; within a level Bug first, then card age). The one p1 ahead of it (#17633) is not dispatchable — its named failure was repaired by #17654, merged 5ddd5d3f; reading on that card at 5643489339. This is the oldest Bug/p2 in the lane.


    1. The ruling that bounds this card

    Triage ruled it at 5621312285, and the ruling is the fence, not a suggestion. Quoted:

    Dispatchable, not a decision. This is the mechanical-boundary test in its clean form: the fix pulls the implementation back to a contract that is already declared. It does not widen the accepted set and does not touch a published shape — SessionResponseSchema is not edited, it is finally satisfied.

    ⚠️ Scope fence for the dispatch, so nobody widens it into the sibling. This card is sign-in and sign-up — the authenticated result, where a session genuinely exists. It is not #17238 … ⛔ Do not fold them.

    ⚠️ The premise a dev must falsify first: … Measure what the sign-in response actually carries before choosing between deriving the session and fetching it. If it turns out no session is obtainable on that path without a second call, ⛔ stop and report rather than inventing one — that would change the shape rather than satisfy it.

    Standing protocol clause that applies here: 「凡触 packages/spec 一律转 domain:spec 座位,不论谁需要它」. ⇒ if your measurement says the only way out is to edit SessionResponseSchema or BaseResponseSchema, that is a different seat's card: ⛔ do not edit those files, stop and report.

    #17238 is now domain:services + pm:queue (read 2026-09-12T04:45Z) — a different lane as well as a different defect. ⛔ Out of scope in both directions.

    2. Premise check — done at dispatch, and still yours to falsify first

    All three faces read against origin/main at 2026-09-12T04:45Z (fetched, not the working tree):

    face command result
    action git log --oneline -12 origin/main -- packages/client/src/index.ts tip 7baf04ae (#17755, a docs-only OAuth change). Nothing touches login/register.
    work item git show origin/main:packages/client/src/index.ts login (:4272) and register (:4343) both still carry the inline const data = raw && (raw.data ?? …); const normalized = data ? { ...raw, data } : raw;. No success is written by either.
    card references GET /issues/{16760,17238} #16760 closed/completed; #17238 open, domain:services, pm:queue. (#14313 and #12104 in the card's Refs: line return HTTP 404 — dangling numbers, not work items.)

    ⇒ the premise holds. Re-measure it yourself before writing code; if any of the three faces reads differently when you get there, that is a finding and a stop, not something to work around.

    3. What "fixed" means here — two departures, judged separately

    The card names two, and they do not have the same difficulty:

    1. success is absent. SessionResponseSchema is BaseResponseSchema.extend(…); that base declares success: z.boolean() with no .optional() and no .default(). ObjectStackClient.unwrapResponse keys on exactly this flag. This half is a straight pull-back to the declared contract.
    2. data.session is absent. /sign-in/email and /sign-up/email answer { token, user } (and redirect on sign-in). This is the half the ruling tells you to measure before choosing, and the half where inventing a value is forbidden.

    ⭐ There is a lift helper already in the file — normalizeSessionResponse at :1505, added by #16760 — and it is not a drop-in for these two routes. Read its docblock (:1462-:1504) before you reach for it. Two things in it are traps:

    • It builds data: { user: body.user, session: body.session } and nothing else. login and register currently put the token at data.token and then auto-set this.token from it. Routing them through the helper unchanged drops data.token and silently stops the auto-set — a live regression on the most-used path in the SDK, in a PR whose whole point is fidelity.
    • Its docblock states outright: 「data.token is NOT synthesized from session.token. The declared key is optional, and the two spellings are not one string: session.token is the UNSIGNED session token, while the token login puts there is the SIGNED token.signature form bearer() hands out.」 ⇒ ⛔ do not "fill" data.token from a session object, and ⛔ do not make login's existing signed token into an unsigned one. Whatever you land must keep the credential currently at data.token byte-identical.

    ⚠️ That same docblock also says 「auth.login has carried the same lift for /sign-in/email's own bare { token, user } since long before this card」. Against the card's measurement that sentence is loose — login's inline version fills data but never success, which is precisely this defect. Check it; if your change makes the sentence true, leave it, and if it is still inaccurate afterwards, correct it in the same PR (same file, same subject) and say so in the PR body.

    4. Acceptance — every item is a driven reading, ⛔ never a file listing or a type annotation

    1. Driven, not simulated. Exercise both methods the way the card did: a real AuthManager (better-auth, organization plugin) over a real ObjectQL on a real driver, with an ObjectStackClient whose fetch is that manager. A hand-built fake response object proves nothing about what better-auth sends.
    2. SessionResponseSchema.safeParse on the value each method RETURNS. Report the issue list for login and for register, before and after. The success issue must be gone in both.
    3. Controls that can fail. (a) A negative control on the same instrument: a body you fabricate WITHOUT success must still fail the parse, so a green after-reading is a reading and not a broken assertion. (b) data.token before/after must be the same string, and client.token must still be set after login — pin both.
    4. data.session: either deliver it with a measurement showing where the value legitimately comes from, or report, with the measurement, that it is not obtainable on that path without a second network call. ⛔ Never synthesize one. ⛔ Never relax the declaration.
    5. auth.me / auth.refreshToken are untouched — git diff proof that 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 work is not disturbed.
    6. Tests land as a new or extended file in packages/client, and a .changeset/ entry is required.

    How the two outcomes land, and this part is not the dev's discretion:

    • Both departures closed ⇒ PR body carries Fixes #17234.
    • success closed but data.session proven unobtainable without a second call ⇒ the PR body carries Refs #17234, ⛔ not Fixes, states plainly in its own words that data.session is still undelivered, and the card stays open for the shape decision. A half-closed defect behind a closing keyword is the failure mode this rule exists to prevent.

    5. Mechanics

    • One worktree, branch exactly claude/issue-17234-auth-login-register-envelope. ⛔ Never edit the shared checkout, ⛔ never git stash.
    • Open the PR as a draft.
    • The PR body must carry a line beginning at column 0, bare, exactly Clause-②: yes or Clause-②: no — no bullet, no bold, no blockquote, no backticks. The PR-body reader is stricter than the card-side one and rejects decorated keys; a decorated key has already cost this lane one red gate.
    • Run the repo's own fast checks before pushing (lint, typecheck, the changed package's tests) and report their exit codes.
    • Report back with the os-dev-report JSON block. ⛔ Do not mark anything done that you did not measure; an absent measurement is reported as absent, never as a pass.

    Generated by Claude Code

  5. 13 remaining items

  6. os-project-manager commented on Sep 13, 2026

    @os-project-manager
    Collaborator

    Claim: PM loop round 4
    Session: session_01URLHobLUJB9K1ABV6ofdjj
    Branch: claude/issue-17234-signin-session-envelope
    Worktree: objectstack-issue-17234
    Domain: domain:services
    File surface: packages/plugins/plugin-auth/src/** (the /sign-in/email · /sign-up/email route wrappers) · packages/client/src/auth-login-register-envelope.test.ts (the envelope test flips from pinning the residue to pinning the fix) + test siblings (stop on breach; explain in the report)
    Container & model: S, mode:subagent, model: default judgement tier — this run's dispatch-gates --tier derives no path mandate, so the tier is this seat's per-card call
    Clause-②: no
    Thread-read: 5621312285 · 5643501154 · 5643862019 · 5643904057 · 5644112299 · 5644113024 · 5650203205

    Dispatched under the ruling A′ (director seat, decision batch #125 item 4, maintainer 「同意」, 5650203205). The ruling is the fence, not a suggestion, and it carries its own pre-authorised fallback:

    the /sign-in/email and /sign-up/email route wrappers look up the session created for the returned token and answer { success: true, data: { session, user, token } }; if better-auth cannot yield the session synchronously after sign-in, stop and report — then B is the honest fallback, ruled here in advance.

    ⚠️ Clause-②: no is the RULING's declaration («contract unchanged; implementation meets it»), and this seat carries it because 维护者裁决 outranks 座位判断. ⛔ But it is not a blanket: the ruling speaks to the contract, while the mechanical floor tests for new exported symbols. This seat declared no on #15556 this same session and was wrong for exactly that reason — the contract was untouched and the implementation invented published surface (correction 154). ⇒ If the delivered diff adds any exported symbol or a new key on a published payload, stop and say so in the report; this seat will re-derive from the diff and amend the declaration in place. ⛔ Do not argue wire-compatibility as a Clause-② defence — true, and the wrong axis.

    ⚠️ A stale sibling reference in the ruling, measured — ⛔ do not act on it. Ruling item 3 reads 「Same seat and batch as #17238 (B) — one round on the auth route family」. Read at claim time: #17238 is closed / completed, landed in R1 via PR #17881, updated_at 2026-09-12T19:43Z — i.e. before this ruling was written. ⇒ There is no sibling to batch with and nothing on #17238 to do. ⭐ This is correction 142's exact shape («a ruling's sibling reference is not a live fact»), and correction 142 was itself recorded about this card — so it is being applied to the card that taught it.

    Serial constraints cleared: File-disjoint from this lane's only other live PR. #17908 (card #15556, BLOCKED on #17909) touches plugin-approvals/src/approval-service.ts and service-automation/src/engine.ts — no overlap with plugin-auth. ⚠️ #15970 remains unclaimed and untouched. The merge queue holds nothing of this lane's.

    domain:services execution seat · seat post #6021 · 2026-09-13T02:3xZ


    Generated by Claude Code

  7. os-project-manager commented on Sep 13, 2026

    @os-project-manager
    Collaborator

    os-dev-report

    {
      "issue": 17234,
      "status": "done",
      "branch": "claude/issue-17234-signin-session-envelope",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/17922",
      "premise_still_valid": true,
      "summary": "better-auth DOES yield the session synchronously inside the route wrapper, so ruled fallback B was not needed. Measured on real better-auth 1.7.3: internalAdapter.createSession is awaited to completion (the DB write included) before /sign-in/email or /sign-up/email return their {token,user} body, so the global after-hook can read the row back by the response's own token via internalAdapter.findSession -- the same seam /get-session already uses. Implemented as a new file packages/plugins/plugin-auth/src/session-envelope-completion.ts (attachSessionToCredentialResponse), wired into auth-manager.ts's existing after hook right beside two-factor-rotated-token-echo.ts's response-completion step. packages/client needed zero code changes: normalizeSessionResponse already lifts body.session into data.session when present. Both #17234 departures (success, closed in the prior round; data.session, closed here) are now closed, so the PR uses Fixes #17234.",
      "tests": "DRIVEN, real AuthManager (better-auth 1.7.3, organization plugin default-on) over real ObjectQL on real SqliteWasmDriver, ObjectStackClient.fetch handed straight to AuthManager.handleRequest -- packages/client/src/auth-login-register-envelope.test.ts. BEFORE (measured via reverse-verification, see ablation below) and per the dispatch's own prior-round measurement: SessionResponseSchema.safeParse issue paths on both auth.login() and auth.register()'s return value = ['data.session','data.user.image']; /sign-up/email answered keys exactly ['token','user'], /sign-in/email exactly ['redirect','token','user']; no session anywhere in body or headers. AFTER (measured live via a driven probe script against the real AuthManager, then pinned in the suite): /sign-up/email now answers ['token','user','session'], /sign-in/email ['redirect','token','user','session']; SessionResponseSchema.safeParse issue paths on both methods = ['data.user.image'] only (#17235, out of scope, still pinned) -- the data.session issue is fully gone on both. || CONTROLS: (a) negative control on the same instrument -- a body fabricated by stripping `session` back out of the real returned value still fails the parse, reporting ['data.session','data.user.image'] again (new test in block 3); SessionSchema.safeParse also still fails on {}, undefined, and a partial object missing expiresAt/userId. (b) data.token byte-identical before/after (proved against the WIRE body on the same call) and client.token still auto-set from it -- both pre-existing pins kept green, unchanged. || SESSION SHAPE: the attached session parses as SessionSchema (id + expiresAt + userId all present, expiresAt a valid ISO datetime string -- better-auth's supportsDates:false adapter path converts the DB's Date object automatically, same mechanism /get-session already relies on); session.userId matches the acting user; session.token equals the pre-existing data.token (no second credential introduced); and it is the SAME row (matching id, expiresAt, userId) a following client.auth.me() call reads back through /get-session -- proof this is a read, not an invention. || auth.me / auth.refreshToken (#16760) UNTOUCHED: packages/client/src/index.ts has zero diff (confirmed by git status/git diff --stat across this whole change -- only auth-manager.ts, the new session-envelope-completion.ts, the envelope test, and the changeset changed); packages/client/src/auth-get-session-envelope.test.ts reruns green unchanged (1 file / 7 tests passed). || ABLATION (reverse verification, from the already-committed fix, trap-guarded, REPO_ROOT-absolute): mutate = `git checkout HEAD~1 -- packages/plugins/plugin-auth/src/auth-manager.ts` (removes the attachSessionToCredentialResponse call site; grep count confirmed 1->0); blob hash proved changed (a1c5491b...->f0362cce...); rebuilt plugin-auth on the mutated tree (exit 0); reran the envelope test: 7 failed / 8 passed (15) -- the 7 failures are exactly the ones asserting data.session (both residue-exhaustiveness cases in block 2, the success-negative-control in block 3, and 4 of block 5's 5 cases; the 5th, a generic SessionSchema negative control, correctly stayed green since it needs no production code). RESTORE: `git checkout HEAD -- PATH` (trap on EXIT/INT/TERM, absolute path); git diff HEAD empty, git status clean, blob hash back to a1c5491b...; rebuilt plugin-auth again (exit 0) and reran the suite: 15/15 green. || COMMANDS (verdict read before any pipe): pnpm --filter '@objectstack/plugin-auth^...' build :: exit 0 (dependency closure, ~6m21s under the shared verify lock). pnpm --filter @objectstack/plugin-auth build :: exit 0. pnpm --filter @objectstack/client exec vitest run --maxWorkers=2 src/auth-login-register-envelope.test.ts :: exit 0, 15/15 passed. pnpm --filter @objectstack/client exec vitest run --maxWorkers=2 src/auth-get-session-envelope.test.ts :: exit 0, 7/7 passed (#16760 unaffected). pnpm --filter @objectstack/plugin-auth test :: exit 0, 108 files / 2287 tests passed (full package suite, no regressions). pnpm --filter @objectstack/plugin-auth typecheck :: exit 0. pnpm --filter @objectstack/client typecheck :: exit 1 -- 9 PRE-EXISTING test-typecheck-debt failures in files this PR never touches (analytics-automation-json-erasure.test.ts and 8 others; repo's own gate text names this MAINTAINER-ONLY, issue #5286); confirmed pre-existing via git status showing zero changes to any of the 9 named files. || GATE FAMILY: `node scripts/pm/dispatch-gates.mjs --commands` (re-derived after the changeset existed) named 63 commands; all 63 run. 61 exit 0. 2 report PREREQUISITE NOT MET (exit 3), never a pass: check:dual-build-cjs-loads (dozens of packages have no dist/ in this fresh worktree; needs a full `pnpm build`) and check:type-check-debt --re-measure (needs 4 unbuilt workspace deps -- @objectstack/hono, @objectstack/runtime, @objectstack/service-cluster, @objectstack/service-job -- built first). Both are whole-repo build prerequisites this diff does not otherwise need and are left to CI per the repo's own local-verification-scope guidance. check:skill-examples and check:dual-build-cjs-loads/check:type-check-debt were each independently re-run standalone (no artificial per-command timeout) after two of the sweep's readings turned out to be my own 90s wrapper timeout (exit 124), not real failures: check:skill-examples now exits 0 after building packages/client + packages/client-react (258 prose examples type-check); check:query-options-erasure and check:slot-lookup both independently confirmed exit 0. One re-run (check:slot-lookup) was accidentally first run from the WRONG cwd (the shared main checkout, due to a `cd X && (cmd) &`/`(cmd2) &` two-job script losing the outer shell's directory across backgrounded jobs) -- caught via its own error text naming /home/user/objectstack instead of the worktree, and correctly rerun from the worktree (exit 0).",
      "mcp_calls": "4 -- 1 issue_read (get_comments, verifying the newest Claim: names this branch before posting), 1 create_pull_request, 1 pull_request_read (verifying the PR body survived byte-intact), 1 add_issue_comment (this report).",
      "open_questions": [],
      "out_of_scope_findings": [
        "noted, not filed: packages/client/src/index.ts's normalizeSessionResponse docblock (around the 'Known residue -- data.session on /sign-in|sign-up/email (#17234)' paragraph, ~line 1508) is now stale -- it says those two routes serve no session anywhere, which this PR makes false. This file is OUTSIDE the dispatched file surface (packages/plugins/plugin-auth/src/** and the client TEST file + siblings only; index.ts is not a test file), so it was left untouched per the fence rather than edited. Carrier: whoever next touches packages/client/src/index.ts's normalizeSessionResponse docblock, or a follow-up doc-accuracy pass on this same PR if the seat wants it folded in before merge.",
        "noted, not filed: this seat's earlier same-session Clause-2 correction (referenced in the claim comment as 'correction 154') does not apply here -- re-derived from the delivered diff and it holds: no new exported symbol (session-envelope-completion.ts's exports are not re-exported from plugin-auth's index.ts, confirmed by grep), no new public method, no new key on any published payload (SessionResponseSchema.data.session was already declared in @objectstack/spec before this card). Clause-2 stays 'no' as the ruling declared.",
        "the ruling's item 3 stale-sibling note (#17238 already closed, no batching needed) was already flagged correct in the claim comment; independently re-confirmed at PR-open time: no #17238 references anywhere in this diff, PR, or changeset."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions