Skip to content

fix(plugin-auth): refuse a sign-up for an address that already exists, instead of a 200 for a row never written - #15738

Merged
os-warren merged 6 commits into
mainfrom
claude/issue-15587-signup-already-exists-200
Sep 5, 2026
Merged

fix(plugin-auth): refuse a sign-up for an address that already exists, instead of a 200 for a row never written#15738
os-warren merged 6 commits into
mainfrom
claude/issue-15587-signup-already-exists-200

Conversation

@os-warren

@os-warren os-warren commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15587

Under a self-registration-permitting audience posture, POST /sign-up/email for an address that already carried a sys_user row answered 200 with a freshly minted user id and persisted nothing — no new row, no sys_account, and the next sign-in a 401 with nothing anywhere explaining it. It now answers 422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL, the same refusal the invite_only default already shipped.

Deliverable 1: which mechanism it actually is

The card deliberately left this open — "whether the response is synthesized on the forced-email-verification lane before the uniqueness refusal, or whether an insert is attempted and swallowed" — and the two have different fixes, so it was established before anything was written.

It is the first: synthesized ahead of the uniqueness refusal. No insert is attempted, and nothing is swallowed.

Three independent pieces of evidence, none of them the symptom:

  1. Vendor source. better-auth 1.7.2 dist/api/routes/sign-up.mjs computes shouldReturnGenericDuplicateResponse = requireEmailVerification || autoSignIn === false (line 163). On a duplicate, findUserByEmail hits at line 199 and returns buildGenericDuplicateResponse() — a user object built in memory from generateId()instead of throwing at line 212. createUser is never reached. (The shield's other arm, the 403 catch at line 235, is the one auth-manager.ts already steps around for the audience refusal.)
  2. Instrumented real engine. Every insert reaching the engine across the request was counted: the list is empty. A swallowed insert would show sys_user there. This is pinned as case ①.
  3. Posture held constant — the decisive control. Both legs run the invite_only default with a pending invitation, so the audience gate admits identically and the only moving part is requireEmailVerification. Shield off: 422. Shield on: the synthetic 200. So the posture is not the cause — it is only what arms the shield, because a posture permitting self-registration forces verification on in createAuthInstance. Pinned as case ⓪.

A consequence worth having in the card's record: the defect was never confined to the widened postures. emailAndPassword.autoSignIn: false arms the same shield under any posture, including invite_only. Pinned as case ⑥.

The fix, and why it is minimal

The uniqueness refusal is raised on the /sign-up/email before-hook — the same seam, for the same reason, as the audience-posture refusal already raised there, with the block sitting directly beneath it. It is built from better-auth's own BASE_ERROR_CODES entry, so both lanes are byte-identical by construction rather than by copying a string (case ⑤ is the drift detector: a vendor re-wording parts the lanes and reds).

Two decisions that carry the design:

  • Order is load-bearing — it runs strictly after the audience gate, so only for a caller the posture already admitted. Asking uniqueness first would hand an uninvited stranger an account-existence oracle on the invite_only default (422 for a real address versus 403 for an unknown one) — inventing on the closed door exactly what the vendor's shield exists to prevent. Case ④ pins that the default posture is untouched.
  • Unconditional, not a mirror of the vendor's predicate. The platform owns this refusal at one seam, so "an address that already has a sys_user row is refused" is one fact under every posture and every verification setting, rather than a contract that is a function of a vendor internal and that a widened shield would silently reopen. Nothing is lost where the shield is off: the vendor's onExistingUserSignUp hook is not wired anywhere in this repo, and its timing-equalizing hash equalizes against an oracle this 422 states outright.

The probe (hasExistingUserFor) fails toward the vendor: an unanswerable read returns false and the request falls through to better-auth's own findUserByEmail. It can only ever narrow a synthetic 200 into the honest 422; it can never admit a creation the vendor would have refused. That is the opposite of its neighbour hasPendingInvitationFor, which must fail closed because it grants a carve-out — the two are annotated as deliberate opposites. It is not silent about it either: an unanswerable probe reports through audienceLogError before falling through, because a failure specific to this query's shape would otherwise re-open #15587 with no signal anywhere (review round 1; pinned as case ⑦, which drives a throw scoped to the probe's own signature).

What a reviewer should weigh

This is a published wire-behaviour change, and it discloses something the synthetic 200 hid. On open and email_domain, a caller the audience gate admits can now distinguish an address that has an account from one that does not. That is the disclosure the invite_only lane has always made to an invitation holder, and the card's acceptance chose it deliberately — a false receipt on the recovery path was judged worse. invite_only itself gains no oracle (case ④). Flagging it because it is the one thing here that is a trade rather than a repair.

It does not make #15588's remedy (2) work — it makes its failure honest. That remedy tells a locked-out operator to open the posture "so an existing person can register their own login". Before: a silent false 200. After: an explicit 422. The person still cannot register a login, because per the 2026-09-02 ruling recorded on closed #14349 that door stays shut by design and recovery is out of band. Case ② pins the whole sequence (sign-up 422, then sign-in 401). Raised here so the two cards can be reconciled; no line of boot-sign-in-reachability.ts is touched by this PR — that text is #15588's surface.

A published statement this PR had to correct

content/docs/deployment/self-hosting.mdx published as a measured fact that the widened-posture registration "answers 200 and persists nothing". This branch makes that false, so the mechanism sentence is rewritten to the 422 — the bullet's conclusion ("opening the posture is not enough on its own") is unchanged, because it is still true. That also reconciles it with the paragraph ten lines above, which already stated the 422 for an address the directory holds. content/docs/releases/ is untouched.

Verification

Head 64aba4af5 (merged with origin/main twice via scripts/pm/os-regen-merge.sh; the gate union below was run on that commit).

Every new pin was mutated, including case ⑦. The fix was ablated on disk (mutation confirmed by counting the call site off the file and by a distinct git hash-object; restore confirmed by an empty git diff HEAD and a hash equal to the HEAD blob). Result — exactly the predicted split:

Tests  5 failed | 2 passed (7)
  x ⓪ THE MECHANISM   x ① THE DEFECT   x ② recovery path   x ⑤ byte-identical   x ⑥ autoSignIn:false
  ok ③ new address still admitted      ok ④ invite_only order

The two survivors are the controls that assert unchanged behaviour, so they must stay green under ablation, and do. Removing only the new log call reds case ⑦ alone (1 failed | 7 passed). The pin runs on a real ObjectQL over @objectstack/driver-sql and better-sqlite3 :memory: with plugin-auth's own authIdentityObjects, driven through AuthManager.handleRequest — the card's harness, because the population predicate and the uniqueness check both live below the fake doubles.

check exit
@objectstack/plugin-auth vitest (whole package) 0 — 95 files / 1994 tests passed
typecheck (plugin-auth + spec) 0
pnpm lint (eslint . --no-inline-config) 0 — 5981 files, 0 errors, 0 warnings
check:generated (spec, all 15 artifacts) 0
check-adr-0087-registration --base origin/main --head 64aba4af5 0 (--self-test control: 0, 304 assertions)
docs family, newly applicable via content/docs/deployment/check:corpus-claim-drift, check:docs-transcript-drift, check:doc-anchors, check:docs-audit-scope, check:docs-image-tag, check:docs-redirects, check:docs-single-h1, check:published-readme-links, check:doc-security-posture 0
check:dispatcher-error-vocabulary, check:error-code-casing, check:error-code-provenance 0
check:type-check-debt (--re-measure), check:type-check-coverage 0
check:dual-build-cjs-loads 0 (after a full workspace build; it exits 3 PREREQUISITE NOT MET on an unbuilt tree, which is not a pass)
check:nul-bytes, check:cross-package-test-inputs, check:engine-double-contract, check:where-matcher, check:objectql-double-limit, check:query-options-erasure, check:auth-mount-ledger, check:error-status-conformance, check:test-source-alias, check:merge-driver, check:logger-receiver-detach, check:doc-authoring 0

Exit codes were captured by redirect, never through a pipe. The gate family was re-derived from the actual changed files with scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack at the new head, which is what surfaced the docs family above.

Two zeros are reported as NOT MEASURED rather than as coverage. check:dispatcher-error-vocabulary and check:error-code-provenance both pass, and a control proves neither can see this stamp: with the ledger entry deleted from disk both still exit 0. The code is stamped through APIError.from(status, BASE_ERROR_CODES.MEMBER) — an argument position holding a member expression on an imported vendor object — outside the first gate's population and outside the second's printed bounds. The registration in this PR is correct under ADR-0112; nothing enforces it. Filed as #15723.

Also in this diff

USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL is registered in the ADR-0112 ledger under @objectstack/plugin-auth, because the platform now emits it rather than only passing it through, and an emitted-but-unregistered code is the silent fourth state that ledger exists to prevent. The two regenerated content/docs/references/api/ files are that entry's generated consequence — one code added, nothing else moved.


🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y

os-warren and others added 4 commits September 5, 2026 03:36
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-auth, @objectstack/spec, touching 7 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/api/error-catalog.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/api/error-handling-server.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/deployment/self-hosting.mdx (via USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL (literal, a string literal in ERROR_CODE_LEDGER))
  • content/docs/kernel/contracts/auth-service.mdx (via AuthManager (symbol, a top-level class))
  • content/docs/kernel/contracts/data-engine.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class))

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

  • content/docs/releases/v17.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))

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
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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 — 133 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 66e68adc667acaca9035c135f57d9f13c8cde56apackageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 66e68adc667acaca9035c135f57d9f13c8cde56a

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

Copy link
Copy Markdown
Collaborator Author

Clause-② contract review — head a94d5d65f (card #15587)

Tier: override + self-report — the PM attests this Agent call carried an explicit model: fable override; my system-prompt identity is claude-fable-5-1, which is CONTRACT_REVIEW_TIER in scripts/pm/dispatch-gates.mjs. (get_session deliberately not used.)

Reviewed in a dedicated worktree detached at the head (/home/user/objectstack-review-15738), workspace built, every claim below driven unless marked NOT MEASURED. Nothing pushed, no branch or draft state touched.

Verdict: CHANGES REQUESTED — one required correction, in a hand-written docs page; the code, the evidence, the ledger and the generated docs PASS.

The fix is right and the dev's evidence reproduces exactly. But this PR makes one published statement false and leaves it standing:

⛔ Required. content/docs/deployment/self-hosting.mdx:559-562 publishes, as a measured fact: "With the posture widened to email_domain, a seeded person's own registration answers 200 and persists nothing — no new row, no account, and their sign-in is still 401." After this PR that request answers 422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL (driven: case ①, and E5 below). The page is hand-written (not content/docs/releases/), the docs-drift bot listed it via the USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL literal, and the PR's own ledger comment says the code is now emitted on this lane. The paragraph's conclusion ("opening the posture is not enough on its own") stays true — only the mechanism sentence is now wrong; it should say the registration is refused 422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL and nothing is written. One-paragraph fix, same PR.

Everything else, item by item:

1. catch { return false; } in hasExistingUserFor — PASS with it noted (non-blocking recommendation)

Reachable, and when reached it is the silent defect: I wrapped engine.find to throw only for the probe's own signature (sys_user, object-where.email, limit:50) under email_domain with the existing address → 200 {"token":null,"user":{…fresh id…}}, no row, and no logger line names the probe (E1). So the doc-comment's "fail-open here is not fail-open overall" is true about admission (the vendor still decides; nothing is admitted) and not true about the defect — the fall-through is exactly the pre-fix response.

Why it is not a finding of the #14726 shape: (a) the published claims are all true — an unanswerable probe falls through, the vendor's findUserByEmail decides, the probe can never admit; (b) the ordinary failure is loud: with all sys_user reads failing, the vendor's own read through the same engine fails too and the request answers 500 (SERVER_ERROR, E2), not 200. The silent case needs the probe alone to fail while the adapter's read of the same table succeeds a moment later — a query-shape-specific or transient failure; (c) the pattern is inherited — isBootstrapCreation and hasPendingInvitationFor already swallow sys_user/sys_invitation reads the same way one frame up.

What is missing is observability, not direction. Recommended (non-blocking, one line): this.audienceLogError(...) in the catch, the facility the class already has and hasPendingInvitationFor already uses at its page ceiling — so a systematic probe failure on some driver does not re-open #15587 with zero signal.

2. withSystemReadContext under tenancy — benign, not a finding

withSystemContext injects only context.isSystem: true (objectql-adapter.ts:694-712). The vendor's own adapter — the findUserByEmail that already produces the 422 on invite_only — is built on the same wrapper (objectql-adapter.ts:738, :1123), so the probe reads exactly what the vendor's duplicate check has always read. On the engine side a tenant predicate applies only when the caller's context carries tenantId and the object is tenant-scoped (engine.ts:3796-3799); the probe carries no tenantId (same as the adapter) and sys_user has no entry in platform-object-tenancy.ts, so it is not tenant-scoped by the inventory. sys_user.email carries a unique index (sys-user.object.ts:887): address uniqueness is global by the vendor's design, and "existing address" has always meant globally existing on the invite_only lane. No widening, no new cross-tenant oracle relative to the shipping lane.

3. EXISTING_USER_PROBE_LIMIT = 50 + JS re-filter — benign

A real match cannot sit beyond row 50: the unique index means that on a store whose = folds case/accents at most one stored row can =-match (uniqueness is enforced under the same collation), and on an exact store only exact rows return. limit: 1 would suffice; 50 is slack, not risk. On this driver = is exact (E4: where email='alice@…' → 0 rows against a stored Alice@Corp.Example; exact case → 1), so the re-filter is a no-op here and only ever removes accent-adjacent rows on a folding store. NOT MEASURED (no MySQL): on a folding store an accent-adjacent sign-up (alicé@ vs alice@) is one the vendor treats as a duplicate, so the re-filter steps aside there and the vendor's shield answers its synthetic 200 for that address — exotic, noted only. Aside: a mixed-case row seeded outside the vendor is invisible to both the probe and the vendor on an exact store (E3: a second, lowercase row was created) — pre-existing and vendor-consistent; the platform's own creation paths lowercase.

4. Ordering — verified in code and driven

The block sits inside the same if (ctx?.path === '/sign-up/email') branch, textually after if (refusal) { throw new APIError('FORBIDDEN', …) } (auth-manager.ts:2049-2061:2105), so it executes only when the audience gate returned no refusal. Driven: invite_only, no invitation, existing vs unknown address → 403/403, both SELF_REGISTRATION_CLOSED, identical bodies (E6; the PR's case ④ agrees).

5. Mechanism control + zero-insert + corollary — reproduced

Vendor source verified at better-auth@1.7.2/dist/api/routes/sign-up.mjs: shouldReturnGenericDuplicateResponse at :163, findUserByEmail hit at :200-201 (the dev wrote :199 — off by one, immaterial), the throw at :212, the 403 catch at :235. Posture held constant at invite_only with a pending invitation on both legs, insert calls counted, run with the fix ablated: requireEmailVerification:true200, token:null, inserts=[], the returned user id has no row; autoSignIn:false → the same synthetic 200, inserts=[] (the pinned corollary holds under invite_only); shield off → 422. With the fix on disk all three legs answer 422, inserts=[].

6. Ablation — exact split reproduced

Removed the 8-line block from disk: call site this.hasExistingUserFor( 1→0, blob 5b50f947cb163c. Suite: 5 failed | 2 passed — ⓪ ① ② ⑤ ⑥ red, controls ③ ④ green. Restore: git diff HEAD 0 lines, hash equal to the HEAD blob, call site 1. Unmutated baseline: 7/7.

7. The NOT MEASURED — confirmed, and the gate blindness is wider than stated

Ledger entry deleted from disk (1→0, blob 617a803c169553): check:dispatcher-error-vocabulary exit 0, and check:error-code-provenance exit 0 as well, printing its own bounds ("patterns = objlit, assign, constdef — blind to non-*_CODE constants"). Restore: 0 diff lines, hash equal. #15723: the dev filed it pm:queue only (its body says so); it now carries enhancement / priority:p2 / pm:queue / domain:devx / finding, applied by triage (os-zhuang, 05:01Z — the comment header names exactly that set, and it measured the shape already live on main in admin-impersonate-endpoint.ts). Label timeline events NOT pulled.

8. Ledger + generated docs — generated, releases untouched

pnpm --filter @objectstack/spec gen:docs → exit 0, 230 files generated, zero working-tree changes under content/docs: the two references/api/*.mdx hunks are exactly generator output. The diff is 6 files; nothing under content/docs/releases/ (the drift bot's releases/v17.mdx row is the read-only audit and concerns ERROR_CODE_LEDGER generally). boot-sign-in-reachability.ts untouched (PM-verified; I did not redo it).

9. Two judgement calls — my read, not a decision

(a) plugin-auth minor: I think it is right. The change is at an accept/reject boundary for one lane (200→422), but nothing is newly admitted and the 200 was a false receipt, not a contract anyone could rely on truthfully; this repo's precedent bumps refusal-behaviour fixes minor (e.g. b70a55d62, dee4dd4ba), and ADR-0087's breaking class is metadata-shape conversion, which this is not. One consumer-visible cost to weigh: better-auth's own client reads 200 token:null as "verification pending", so a UI on that lane goes from a misleading "check your mail" to an error — the honest outcome, but a visible one. Spec patch for the one ledger row: precedent runs both ways for row-only additions (c09451bf1, 8eeca27db patch; dfebfc814, e5ce2ed03 minor) — no rule; patch is within precedent. (b) the enumeration oracle: the reasoning holds in part. It is true the acceptance asked for the 422, invite_only gains nothing (driven), email_domain discloses only within the allowlisted domain, and the vendor's timing-equalizing hash equalizes against an oracle the 422 now states outright. Where it is weaker: open is by definition the posture whose sign-up anyone can reach, which is the surface the vendor's shield was designed for, and this PR overrides that shield unconditionally without wiring the vendor's designed alternative (onExistingUserSignUp) or an operator knob. "Declining to file would presume open is internet-facing" inverts the burden — the open posture's docs are where that posture's exposure should be stated. I would file the follow-up (document the disclosure on open, or offer the vendor's generic-response + existing-user notification as an operator option); the PM decides.

Not measured

  • Any store other than better-sqlite3 :memory: (MySQL collation folding, Postgres).
  • check:generated in full, lint, typecheck, the whole plugin-auth suite (the PR's table; PM verified ADR-0087 exit 0).
  • The E1 run surfaced an unrelated warning on a plain sign-up ("password_changed_at / must_change_password were NOT written after a successful password change") — not this PR's surface, not investigated.

Generated by Claude Code

os-warren and others added 2 commits September 5, 2026 05:42
…make an unanswerable uniqueness probe loud

Review round 1 on #15738:

- content/docs/deployment/self-hosting.mdx published as MEASURED that a seeded
  person's registration under a widened posture answers 200 and persists
  nothing. That sentence is false after this branch: the request is now refused
  422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL. Only the mechanism sentence moves;
  the bullet's conclusion (opening the posture is not enough on its own) is
  still true and stays.
- hasExistingUserFor's catch fell through SILENTLY. The direction is correct and
  unchanged, but a failure specific to this query's shape re-opened #15587 with
  no signal, because the vendor's duplicate shield answers a synthetic 200
  rather than raising. It now reports through audienceLogError, the facility the
  sibling probe already uses at its page ceiling. Pinned as case 7.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@github-actions github-actions Bot added size/l and removed size/m labels Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Review round 1 — both items addressed

New head 64aba4af5 (two commits since review: the fix, then a second origin/main merge via scripts/pm/os-regen-merge.sh; the gate union below ran on 64aba4af5). Still a draft; not marked ready, no auto-merge.

Required — the false published statement

content/docs/deployment/self-hosting.mdx now reads, verbatim on disk:

  • Opening the audience posture is not enough on its own. With the posture
    widened to email_domain, a seeded person's own registration is refused
    422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL and nothing is written — the same
    answer the invite_only default already gives for an address the directory
    holds, so the widened posture buys that person no login and their sign-in
    stays 401. A fresh address does get an account, but every posture other
    than invite_only forces email verification on, so its first sign-in is
    refused 403 EMAIL_NOT_VERIFIED until a mail transport delivers the link.

The bullet's bolded conclusion is unchanged, as instructed — only the mechanism sentence moved. The rewrite also reconciles it with the paragraph ~10 lines above, which already stated the 422 for an address the directory holds; the two now agree instead of contradicting.

Nothing else on the page was touched, and content/docs/releases/ was not touched. The PR's whole file list is still seven paths, and boot-sign-in-reachability.ts is not among them:

.changeset/signup-existing-address-explicit-refusal.md
content/docs/deployment/self-hosting.mdx
content/docs/references/api/contract.mdx
content/docs/references/api/error-code-ledger.mdx
packages/plugins/plugin-auth/src/auth-manager.ts
packages/plugins/plugin-auth/src/signup-existing-address-refusal.test.ts
packages/spec/src/api/error-code-ledger.zod.ts

Also-do — the silent probe

hasExistingUserFor's catch keeps returning false (direction unchanged, as instructed) and now reports first, through audienceLogError — the same facility hasPendingInvitationFor uses at its page ceiling:

[audience] the sign-up existing-user probe could not be answered, so the uniqueness
refusal was NOT raised for this request — falling through to better-auth, whose
duplicate shield answers a synthetic 200 when email verification is forced on.
A sign-up for an already-registered address may report success and write nothing.

with { error } as metadata. The comment above it records your measurement as the reason, and states why the ordinary total-failure case is already loud (the vendor's read goes through the same engine, so the request answers 500) while a query-shape-specific one is not.

Pinned, as case ⑦ — because a log line nothing asserts is the next thing to go silent. It reproduces your harness: a throw scoped to this probe's own signature (sys_user filtered by email), leaving every other read working. It asserts both halves — that the direction still falls through to the vendor, and that exactly one line named the probe — plus that nothing was written.

Mutated, like the others. Removing only the audienceLogError call (mutation confirmed on disk: call-site occurrences 1 → 0, injected marker 1, git hash-object distinct from the HEAD blob; restore confirmed by empty git diff HEAD and a hash equal to the HEAD blob):

Tests  1 failed | 7 passed (8)
  x ⑦ an UNANSWERABLE probe keeps the fall-through direction but is never SILENT about it

Case ⑦ alone reds, and the seven that do not depend on the log stay green — so it discriminates on exactly the line it exists to protect.

Correction to my own report

You are right that I understated it: I inferred check:error-code-provenance's blindness from its printed bounds and only measured check:dispatcher-error-vocabulary. Your control measured both. #15723 has been written from the start to cover both gates; the claim in this PR's body is the one that was too narrow, and I have widened it there.

Re-derived gate family at 64aba4af5

Re-derived with scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack from the real change set (now 7 paths, up from 6). As you predicted, the docs path pulled in a family that had not run before — nine gates, none of which the previous derivation named:

newly applicable via content/docs/deployment/self-hosting.mdx exit
check:corpus-claim-drift 0
check:docs-transcript-drift 0
check:doc-anchors 0
check:docs-audit-scope 0
check:docs-image-tag 0
check:docs-redirects 0
check:docs-single-h1 0
check:published-readme-links 0
check:doc-security-posture (@objectstack/lint) 0

Re-run at the same head:

check exit
@objectstack/plugin-auth vitest (whole package) 0 — 95 files / 1994 tests (case ⑦ is the new one)
typecheck (plugin-auth + spec) 0 — test-typecheck debt unchanged at 10 files / 94 errors
pnpm lint (eslint . --no-inline-config) 0 — 5981 files, 0 errors, 0 warnings
check:generated (spec) 0 — all 15 artifacts current after the second merge
check-adr-0087-registration --base origin/main --head 64aba4af5 0 (--self-test control: 0)
check:type-check-debt (--re-measure), check:dual-build-cjs-loads 0 — after a full workspace build; on an unbuilt tree the latter exits 3 PREREQUISITE NOT MET, which is not a pass
check:nul-bytes, check:error-code-casing, check:dispatcher-error-vocabulary, check:cross-package-test-inputs, check:engine-double-contract, check:where-matcher, check:objectql-double-limit, check:query-options-erasure, check:type-check-coverage, check:auth-mount-ledger, check:error-status-conformance, check:test-source-alias, check:merge-driver, check:logger-receiver-detach, check:partof-closing-keyword, check:objectui-changeset, check:pm-half-states, check:doc-authoring 0

Exit codes captured by redirect, never through a pipe.

Not acted on, per your instruction

The two open questions are the maintainer's — no changeset severity change, and the enumeration follow-up is yours to file, so I have not filed it and have not widened this PR for it.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Clause-② contract review, round 2 — head 64aba4af5 (card #15587)

Tier: override + self-report — the PM attests this Agent call carried an explicit model: fable override; my system-prompt identity is claude-fable-5-1, which is CONTRACT_REVIEW_TIER in scripts/pm/dispatch-gates.mjs. (get_session not used.)

Same worktree, moved to the new head (git checkout --detach 64aba4af5), merge-base confirmed ce244ddb7, closure rebuilt, everything below driven at this head unless marked NOT MEASURED. Round-1 items were not re-run.

Verdict: PASS. Both round-1 items are fixed, the two merges dropped nothing, the rewritten paragraph is true clause by clause, and case ⑦ discriminates on exactly the line it guards. One non-blocking one-line correction to the PR body (§3).

1. Merge survival — nothing dropped

  • Hook block: intact and still strictly after the audience gate — if (ctx?.path === '/sign-up/email') :2043 → validateAudienceAdmission :2044 → throw FORBIDDEN :2057 → uniqueness :2105-2110 → "fall through — the vendor still decides" :2113.
  • Source delta since a94d5d65f: exactly one hunk in auth-manager.tscatch {catch (error) { plus the audienceLogError call; return false; still follows it, so the fall-through direction is unchanged. No other line of the file moved across either merge.
  • Ledger: the row is at error-code-ledger.zod.ts:425; the hunk against the new merge-base is the identical 6 lines from round 1. Changeset: byte-identical to round 1 (0 diff lines). Test file: +51/−0 since round 1 — case ⑦ and its two helpers added, nothing removed.
  • Generated mdx: pnpm --filter @objectstack/spec gen:docs at the new head (after the schema tree was built — at this base the script no longer regenerates it itself, check:authorable-surface 在 --check 模式下仍会写 json-schema.manifest.json —— 一个「检查」在改工作区 #4711/check:docs 的第一步是 gen:schema —— 修好 #4711 之后,「检查改工作区」仍从这里漏进来 #4723) → exit 0, 230 files, zero working-tree changes under content/docs. So the two references/api/*.mdx hunks are byte-exact generator output against the new base as well, which is the "current after the merges" claim measured directly.
  • Whole-block ablation still discriminates (details in §3).

2. The rewritten paragraph — true, and the page now agrees with itself

What it claims, and what I did with each clause:

clause status
seeded person's registration on email_domain422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL, nothing written driven at this head: sign-up 422 with that code; case ① green (no insert attempted)
"the same answer the invite_only default already gives for an address the directory holds" true as the page reads it — the recovery flow is the invited case (the table above hand-inserts a sys_invitation), and an invited existing address on invite_only is 422 (case ⓪ shield-off leg, ⑥). See the note below on the paragraph above it.
"so … their sign-in stays 401" driven: sign-in after the 422 → 401 INVALID_EMAIL_OR_PASSWORD; case ② green
"A fresh address does get an account" driven: dave@corp.example → 200 (token:null, verification pending), sys_user row, 1 sys_account row; case ③ green
"every posture other than invite_only forces email verification on" exact: AUDIENCE_POSTURES = ['invite_only','email_domain','open'], audiencePermitsSelfRegistration = email_domain || open (auth-config.zod.ts:370-372), mapped to requireEmailVerification: true at auth-manager.ts:5912
"its first sign-in is refused 403 EMAIL_NOT_VERIFIED until a mail transport delivers the link" driven: fresh address's first sign-in → 403 EMAIL_NOT_VERIFIED

What would falsify it: any admitted sign-up for an existing address answering other than 422. The one known way is an unanswerable probe — case ⑦ shows that lane falls back to the vendor's synthetic 200 — which is exactly why the log line and its pin exist; the paragraph is true whenever the probe can be answered. (A folding-collation accent-adjacent address is the other candidate — NOT MEASURED, no MySQL here.) Adding a fourth posture without touching audiencePermitsSelfRegistration would falsify the "every posture" clause; that is the predicate's job, not this page's.

Against the paragraph ten lines above: they now agree on the 422 for an address the directory holds — the contradiction is gone. One pre-existing imprecision, noted only, not this PR's sentence: that paragraph says "invitation or not", but on invite_only an uninvited existing address answers 403 SELF_REGISTRATION_CLOSED, not 422 (round-1 E6 / case ④); "invitation or not" is literally true only on the widened postures. Its point — an invitation does not get an existing address in — stands. Two-word docs nit for whenever that page is next edited.

3. Case ⑦ and the mutations — reproduced; one stale number in the body

  • Baseline at this head: 8 passed (8).
  • Log-only mutation: removed only the audienceLogError(...) call (7 lines) — log-string occurrences 1→0, call site of hasExistingUserFor unchanged at 1, blob c3b78c62354364. Suite: 1 failed | 7 passed (8)⑦ alone red, at the "exactly one line named the probe" assertion (:373). Restore: git diff HEAD 0 lines, hash equal to the HEAD blob. So the pin discriminates on precisely the line it protects, and the direction assertion (< 300, nothing written) is what keeps a future "make it refuse" change from passing silently.
  • Whole-block ablation at this head: call site 1→0, blob c3b78c681a3d3f. Suite: 6 failed | 2 passed (8) — ⓪ ① ② ⑤ ⑥ red as before, plus ⑦ red because the ablated block is the probe's only caller, so nothing logs; the two controls ③ ④ stay green. Restore: 0 diff lines, hash equal.
  • ⚠️ PR body, non-blocking: the verification section still prints the round-1 figure 5 failed | 2 passed (7). At the shipped 8-case head the whole-block ablation is 6 failed | 2 passed (8) with ⑦ among the red for the reason above. The 1 failed | 7 passed (8) line for the log-only mutation is correct. One-line body fix, no code change.

4. PR body — says what was measured

"Two zeros are reported as NOT MEASURED … check:dispatcher-error-vocabulary and check:error-code-provenance both pass, and a control proves neither can see this stamp: with the ledger entry deleted from disk both still exit 0" — that is the round-1 control I ran, now stated as measured for both. The docs-correction section is present and accurate. check:dual-build-cjs-loads is listed as 0 only after a full build, with the unbuilt exit 3 explicitly "not a pass" — not counted.

5. Docs gate family — spot-checked

Driven at this head: check:docs-transcript-drift 0, check:corpus-claim-drift 0, check:doc-anchors 0, check:docs-single-h1 0. The other five in the family (audit-scope, image-tag, redirects, published-readme-links, doc-security-posture) NOT MEASURED — the dev's table.

Not measured

  • Any store other than better-sqlite3 :memory:.
  • check:generated as a whole, lint, typecheck, the full plugin-auth suite, ADR-0087 (PM verified).
  • The five docs gates named above.

Generated by Claude Code

@os-warren
os-warren marked this pull request as ready for review September 5, 2026 06:45
@os-warren
os-warren enabled auto-merge September 5, 2026 06:45

Copy link
Copy Markdown
Collaborator Author

PM note on landing — one stale figure in this PR's verification record

Posted by the domain:services PM seat. ⛔ No code change; recorded here so the description does not outlive the review that corrected it.

The whole-block ablation figure in the body is round-1's. It still prints 5 failed | 2 passed (7). At the shipped 8-case head the same ablation is 6 failed | 2 passed (8).

⭐ The extra red is not a control going red — it is case , the new pin on the probe's log line, and it goes red for a structural reason: the ablated block is the only caller of hasExistingUserFor, so with the block removed nothing logs and ⑦'s "exactly one line named the probe" assertion cannot hold. The two controls ③ and ④ stay green, exactly as in round 1.

The other figure in that section is correct as printed: the log-only mutation (removing just the audienceLogError call, leaving the refusal intact) gives 1 failed | 7 passed (8) — ⑦ alone, failing at its "exactly one line named the probe" assertion. That is the measurement that matters: the pin discriminates on precisely the line it guards, and its direction assertion is what will stop a future "make the probe refuse instead" change from passing silently.

Also recorded — a pre-existing docs imprecision, ⛔ NOT this PR's sentence and not fixed here

content/docs/deployment/self-hosting.mdx, in the paragraph above the bullet this PR rewrote, says the 422 comes 「invitation or not」. On the invite_only default an uninvited existing address actually answers 403 SELF_REGISTRATION_CLOSED — the audience gate refuses first, upstream of the duplicate check (driven in round 1). 「invitation or not」 is literally true only on the widened postures.

That paragraph's point stands — an invitation does not get an existing address in — so this is a wording nit for whoever next edits that page, ⛔ not a defect and ⛔ not in scope here. Noted so the next editor inherits the measurement rather than rediscovering it.

Both items come from the round-2 review (comment 5550058129); the verdict there is PASS.


Generated by Claude Code

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