Skip to content

fix(plugin-auth): verify-domain answers DOMAIN_VERIFICATION_DISABLED for the disabled condition - #11004

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-10859-verify-domain-disabled-code
Aug 22, 2026
Merged

fix(plugin-auth): verify-domain answers DOMAIN_VERIFICATION_DISABLED for the disabled condition#11004
os-warren merged 1 commit into
mainfrom
claude/issue-10859-verify-domain-disabled-code

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#10859

What each route answered, before and after

Both SSO domain-verification bridge routes recognise the same condition — domain
verification is off for this environment, so @better-auth/sso never mounts the
inner endpoint and it answers 404 with no code — and they answered it
differently.

routeconditionbeforeafter
POST /admin/sso/request-domain-verificationdisabled400DOMAIN_VERIFICATION_DISABLEDunchanged
POST /admin/sso/verify-domaindisabled404DOMAIN_VERIFICATION_FAILED400DOMAIN_VERIFICATION_DISABLED
POST /admin/sso/verify-domaingenuine failure, uncoded<inner status>DOMAIN_VERIFICATION_FAILEDunchanged
POST /admin/sso/verify-domainvendor answered with a code<inner status><vendor code>unchanged

runVerifyDomain's branch for the disabled condition rewrote only the message
and let the code fall through to the route's generic default, so the response
carried "the feature is off" copy under a code that means "verification failed".
A caller can only act on the machine-readable half, and the two halves
disagreed.

Premise re-derived on origin/main at 047ac86ee before any edit: it holds
exactly as the card quotes it.

The status question, and the measurement behind it

The card asks whether verify-domain should stay 404 while its sibling
rewrites to 400. Answer: 400, parity with the sibling. Four measurements,
in the order they moved the decision:

  1. The file's own stated design intent already required it. The section
    header in register-sso-provider.ts says: "A 404 from the inner endpoint
    means the feature is OFF for this env (endpoints unmounted) → surfaced as
    such, not a bare 'not found'."
    The sibling implements that; verify-domain
    did not.
  2. The 404 was describing the wrong resource. The outer bridge route is
    mounted unconditionally (auth-plugin.ts, rawApp.post(.../admin/sso/verify-domain))
    — it is not behind the feature flag. Only the inner endpoint is unmounted,
    so passing its status through said "no such endpoint" about a resource that
    demonstrably exists.
  3. No repo-wide convention pointed the other way. Surveyed how this tree
    answers "capability off": 403 (FEEDS_DISABLED), 409 (FLOW_DISABLED),
    501 (NOT_IMPLEMENTED, admin plugin off), 503 (OIDC not enabled), 404
    (MCP not enabled), 400 (the sibling). There is no house rule to appeal to,
    so nothing outweighed same-file, same-feature, same-condition parity.
  4. Nothing depends on the 404. The dogfood bucket loop asserts only that
    the admin's status is not in [401,403] and the code is not
    PERMISSION_DENIED; the console action runtime uses the HTTP status only as
    a fallback label when the envelope carries no error. Neither is disturbed
    by 400.

The status moves with the code deliberately: answering the sibling's code at a
different status would have left "one condition, two answers" half-standing.

Both directions are pinned

packages/plugins/plugin-auth/src/sso-domain-verification-error-codes.test.ts
(6 pre-existing tests from #10716, 6 added; all assert codeandstatus
per ADR-0112):

  • disabled ⇒ DOMAIN_VERIFICATION_DISABLED + 400, on both routes, plus a
    direct parity assertion that the two routes answer the same code and the
    same status for the same condition.
  • the load-bearing direction — DISABLED is not stamped on every failure: a
    genuine uncoded failure still answers DOMAIN_VERIFICATION_FAILED with the
    inner status passed through, and a 404 that carries a vendor code is the
    vendor's diagnosis, passed through untouched. The rewrite is keyed to the
    disabled shape specifically — 404without a code.
  • the vendor parsed?.code pass-through arm is untouched on both routes.

One fixture was re-fixtured, not just re-spelled. The #10716 test
verify-domain: an uncoded vendor failure answers our SCREAMING default was
driven by fakeHandle(404, undefined) — which is the disabled shape, not a
verification failure. Left pointing there it would have kept passing while
testing the wrong branch, and the generic-default arm of verify-domain would
have had no coverage at all. It now drives a real uncoded failure (502), the
same shape its sibling case uses.

Ablation — prediction written before mutating, both legs

Predictions were recorded to a file before either mutation. plugin-auth has no
dist/ and its vitest.config.ts declares no alias, and the test imports the
subject by the relative specifier ./register-sso-provider.js, so the resolved
artifact issrc/ and no rebuild step can silently invalidate a leg. The
positive control for that claim: the suite runs and its result changes from a
src/-only edit, with no dist/ present anywhere in the package.

legpredictedobserved
A — remove the fix (message-only rewrite)exactly 2 red: the disabled case, and the both-routes parity case; 10 greenexactly 2 red, those two; 10 green. expected 'DOMAIN_VERIFICATION_FAILED' to be 'DOMAIN_VERIFICATION_DISABLED'
B — over-broad guard if (resp.status === 404), dropping && !parsed?.codeexactly 1 red: the vendor-coded-404 case; 11 greenexactly 1 red, that one; 11 green. expected 'DOMAIN_VERIFICATION_DISABLED' to be 'NO_PENDING_VERIFICATION'

Leg B is the hazard worth naming: an implementation that stamped DISABLED on
every 404 passes the disabled case and the pre-existing #10716 pass-through
test (which drives status 400, not 404). Only the newly added leg catches
it.

Restores proved byte-identical with git hash-object, and the ablation marker
was confirmed absent afterwards:

PRISTINE 429a1399ba1cf23ee5708de0e3aeb2ca1fbbfff2
A mutated 19e47f3a6bd7e704c3227de43bf6ca7410bf399d → restored 429a1399… (marker count 0)
B mutated b155a69cfa25d8678bd6490a7e80664a0ddff008 → restored 429a1399… (marker count 0)
FINAL 429a1399ba1cf23ee5708de0e3aeb2ca1fbbfff2

The dogfood note

admin-route-nonadmin-refusal.dogfood.test.ts:261 is a note: string —
documentation, not an assertion. The bucket loop never reads note, so nothing
here goes red on its own; it was updated deliberately rather than by being
forced to.

Gates — union derived on the final commit 93c4f7c9b, clean tree

node scripts/pm/dispatch-gates.mjs with no path arguments (derived 4 paths
vs merge base 047ac86ee; committed 4, working tree 0, untracked 0). Every exit
code captured before any pipe. All 24 green:

15 derived — check:changeset-gate-self-tests, spec:check:empty-state,
spec:check:liveness, check:objectui-changeset, check:slot-lookup,
spec:check:strictness-ledger, check:test-source-alias,
check:type-source-resolution, spec:check:variant-docs,
check-adr-0087-registration, check-changeset-no-major,
check-ci-filter-parity, check-empty-changeset, check-plugin-teardown-shape,
check-affected-docs.

5 convention-triggered (this change adds test code) — check:query-options-erasure,
check:type-check-coverage, check:engine-double-contract, check:where-matcher,
and check:type-check-debt --re-measure (run against the built workspace closure,
turbo run build exactly as lint.yml does: "33 ledger entries re-measured,
1908 raw tsc errors total, none above its recorded number"
).

Plus check:nul-bytes, and class #10309 run explicitly:
check:error-code-casing, check:dispatcher-error-vocabulary,
check:route-envelope. The path derivation did not name any of those three
they were run because the class is live, and all three are green on their own
verdict lines (✓ no unlisted lowercase error codes in 4414 scanned file(s);
OK — 21 unregistered code-stamping site(s), all classified; ✓ 4 module(s) discovered and audited). No log contains a PREREQUISITE NOT MET / Nothing was checked refusal or a zero-match no-op.

Package suites: @objectstack/plugin-auth typecheck exit 0; full suite 67
files / 1406 tests passed
. @objectstack/dogfood typecheck exit 0.

Breaking or not — argued here, not settled here

Shipped as minor, matching the call the casing rename on these same two routes
made (#10716). For: the emitted vocabulary gains no member (both codes are
already registered for @objectstack/plugin-auth), and the old pairing was
self-contradictory — a code meaning "verification failed" under a message saying
"the feature is off" — rather than a contract anyone could have relied on
deliberately. Against, stated rather than settled: unlike that rename, whose
old spellings were undeclared values no schema admitted,
DOMAIN_VERIFICATION_FAILEDis a declared, registered code, so a client keyed
to it for this case was keyed to something the published contract admitted — and
here both halves of the answer change, code and status. A reviewer who reads
that as major is not reading it wrong. This PR does not decide it silently.

Scope

No packages/spec change — none was needed and this lane has none.
content/docs/releases/** untouched. #10700 is in flight in the same package
on the two-factor surface and was not touched; we share only .changeset/, one
file each.

⚠️Draft, and staying draft. Clause-② applies (the emitted error vocabulary
of a published endpoint changes), needs:contract-review is the compensating
control on #10859, and this seat does not clear it, mark ready, enable
auto-merge, or merge.


Generated by Claude Code

…for the disabled condition
Both SSO domain-verification bridge routes recognise the same shape - the inner
@better-auth/sso endpoint is unmounted for this env and answers 404 with no code
- and answered it differently. runRequestDomainVerification returns 400 +
DOMAIN_VERIFICATION_DISABLED; runVerifyDomain rewrote only the message and let
the code fall through to the generic failure default, so the response carried
"the feature is off" copy under a code meaning "verification failed".
verify-domain now returns the sibling's answer for that condition: 400 +
DOMAIN_VERIFICATION_DISABLED. The status moves with the code because the inner
404 describes the INNER endpoint, while this bridge route is mounted
unconditionally.
The rewrite stays keyed to the disabled shape (404 WITHOUT a code): a genuine
failure still answers DOMAIN_VERIFICATION_FAILED, and the vendor pass-through
arm is untouched on both routes. Both directions are pinned.
Part of #10859
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 11 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 bd003d821787d3d72e68c5297d3d2088a473c5c0packageMentionDocs.

Which tree this was computed on

This run read content/docs from e01f70370f12a1f49189641ae2f8ea8474ad116d — the merge of head 93c4f7c9b5795e7a3376325f3c913826ee478e5a into base bd003d821787d3d72e68c5297d3d2088a473c5c0, 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 e01f70370f12a1f49189641ae2f8ea8474ad116d && git checkout e01f70370f12a1f49189641ae2f8ea8474ad116d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bd003d821787d3d72e68c5297d3d2088a473c5c0 93c4f7c9b5795e7a3376325f3c913826ee478e5a && git checkout -B drift-repro bd003d821787d3d72e68c5297d3d2088a473c5c0 && git merge --no-ff 93c4f7c9b5795e7a3376325f3c913826ee478e5a
node scripts/docs-audit/affected-docs.mjs --json bd003d821787d3d72e68c5297d3d2088a473c5c0

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 22, 2026
@os-warren
os-warren marked this pull request as ready for review August 22, 2026 07:41
@os-warren
os-warren added this pull request to the merge queueAug 22, 2026
Merged via the queue into main with commit d9353b9Aug 22, 2026
33 checks passed
@os-warren
os-warren deleted the claude/issue-10859-verify-domain-disabled-code branch August 22, 2026 07:52
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32560319641 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 6.09s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 87 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

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

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-auth: verify-domain answers a FAILURE code when domain verification is DISABLED, while its sibling route answers DOMAIN_VERIFICATION_DISABLED

2 participants

@os-warren@claude