Skip to content

fix(plugin-auth): phone send-otp with no deliverable SMS service answers 400 SMS_SERVICE_REQUIRED instead of a bare 500 - #21858

Merged
objectstack-fleet[bot] merged 8 commits into
mainfrom
claude/issue-21793-phone-otp-no-provider-4xx
Oct 5, 2026
Merged

objectstack-fleet[bot] merged 8 commits into
mainfrom
claude/issue-21793-phone-otp-no-provider-4xx

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21793
Clause-②: yes (widening)

What changed

POST /api/v1/auth/phone-number/send-otp on a deployment with phone sign-in on but no SMS service that can deliver a code (none wired, or only the log transport in production) answered 500 with an empty body. It now answers 400 with { "code": "SMS_SERVICE_REQUIRED", "message": "..." }.

  • packages/plugins/plugin-auth/src/auth-manager.ts: the no-provider branch of deliverPhoneOtp throws better-auth's APIError('BAD_REQUEST', { message, code: 'SMS_SERVICE_REQUIRED' }), the same construction the sibling refusals in this file use (for example PASSWORD_POLICY_VIOLATION). The message names the missing SMS delivery service and where an administrator configures it (Setup, Settings, SMS Delivery), and points the user at phone and password sign-in. It never carries the one-time code. The quota branch and the delivery path are unchanged. Four doc comments that said NOT_SUPPORTED now name the new answer.
  • packages/spec/src/api/error-code-ledger.zod.ts: SMS_SERVICE_REQUIRED is registered for @objectstack/plugin-auth, beside EMAIL_SERVICE_REQUIRED. The reference pages content/docs/references/api/{contract,error-code-ledger}.mdx are regenerated (gen:docs, as check:generated named).
  • content/docs/permissions/authentication.mdx: the two passages that said "fails loudly with NOT_SUPPORTED" now state the 400 and the code. The first also says that request-password-reset keeps answering {status:true}.
  • docs/qa/platform-checklist/areas/identity-auth.json: identity-auth.auth-method-matrix (revision 5) names the shipped refusal in clause 4, its negative, step 4, the fixtures row and the variant. A bare 500 is now named a FAIL, and the clause cites the door pin.
  • Changeset: @objectstack/spec minor (the ledger accepts one more value), @objectstack/plugin-auth patch (the bug fix).

H2: which branch, and the measurement that decided it

Branch (b): a new registered code. No registered code honestly names "phone OTP needs an SMS delivery service" (measured at merge base 6fb71152c):

  • @objectstack/plugin-auth's row: EMAIL_SERVICE_REQUIRED names the email service. PHONE_NOT_ENABLED means the phone plugin is off, which is not this case. INVITE_SMS_FAILED is a failed invitation send. No other row is about SMS.
  • The standard catalog has no SMS member. SERVICE_UNAVAILABLE and NOT_IMPLEMENTED misname the cause and are 5xx.
  • No other package's row names SMS delivery.

The spelling SMS_SERVICE_REQUIRED already exists in this package: sendPhoneInviteSms throws it as a plain-Error prefix for the same condition. So one name now covers one condition. It passes the #8211 synonym rule, because the token SMS is in no standard member. The status is 400, the status of the email sibling (admin-import-users.ts answers EMAIL_SERVICE_REQUIRED with fail(400, ...)).

Mechanism hypotheses, measured

All door readings use the real AuthManager.handleRequest over the installed better-auth 1.7.3 / better-call 1.4.0.

  • H1, confirmed. Before the fix, the no-provider branch answered 500, no content-type, body "" (measured under the ablation below). The quota branch answered 429, application/json, {"message":"Too many verification codes requested. Please try again later."}, with no code field. After the fix, no-provider answers 400, application/json, {"message":"Phone verification codes are unavailable: ...","code":"SMS_SERVICE_REQUIRED"}. The quota answer is unchanged.
  • H2: see above.
  • H3: 400, from the email sibling.
  • H4, confirmed at objectui 9dfaca65 (the .objectui-sha pin). packages/auth/src/createAuthClient.ts postPhoneNumberEndpoint reads the top-level payload.code and payload.message of this vendor-shaped body. LoginForm.handleSendOtp shows errorMessages[code] if one is mapped, and the message otherwise. Before the fix the payload was null, so the user saw "Auth request failed with status 500".

Tests

  • New packages/plugins/plugin-auth/src/phone-otp-no-sms-service-refusal.test.ts is the door pin. It sends a real request through a real better-auth pipeline and a pinned memory engine. It checks:
    • With no SMS service: 400, code === 'SMS_SERVICE_REQUIRED', and ErrorCode.safeParse(code) succeeds, so the code is registered.
    • The refusal text never contains the OTP that better-auth stored.
    • With NODE_ENV=production and a log-only transport: the same 400 and code, and nothing is sent.
    • request-password-reset for a registered number still answers 200 {status:true}. A pass-through spy proves the route reached the refusing send.
    • Outside production, a log-only transport still delivers.
  • In auth-manager.test.ts, the old rejects.toThrow(/NOT_SUPPORTED/) case became two cases on the error object: isAPIError, BAD_REQUEST/400, body.code, and no code in the message.
  • Ablation. The plain Error was put back through scripts/ablation-replace.mjs (mutation landed: anchor 1 to 0, blob 9d24bb3f to 9c6b1b90). Result: 5 failed / 282 passed. That is 3 door cases (expected 500 to be 400, and the deliver spy rejects with Error: NOT_SUPPORTED... instead of the 400 shape) and 2 unit cases (isAPIError false, statusCode undefined). The tool's restore leg proved blob == HEAD 9d24bb3f and an empty git diff HEAD. The first attempt was refused by the tool before any test ran, because the replacement text contained the anchor. It was redone with a whole-block anchor.
  • pnpm --filter @objectstack/plugin-auth exec vitest run: 120 files, 2515 passed, 10 skipped (at b37edd67). Typecheck exit 0. After the last test-file edit, the two changed files were re-run at 3e8ab846: 286 passed.
  • pnpm --filter @objectstack/spec exec vitest run: 668 files, 19286 passed, 1 todo (at b37edd67). Typecheck exit 0 (at 3e8ab846).
  • pnpm --filter @objectstack/spec check:generated: all 15 artifacts up to date, after a spec rebuild on the final merge c6b17163.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at c6b17163 derived 124 commands. All 124 were run on that head and exited 0. --ran with the exit codes recorded: 124 derived, 124 run, 0 NOT-MEASURED, 0 UNRUN. On the first pass (at b37edd67), check:engine-double-contract and check:objectql-double-limit were red on the new test's engine double. The double now applies limit/offset by presence, and --write recorded the pinned double (additions only). The local scope is the targeted set above; the rest of the farm is CI's.

Riding comments (domain:spec pointer 5989650462)

These are comment-only edits in error-code-ledger.zod.ts. No code, status or owner moved. Each claim was checked against the tree:

  1. DRIVER_UNAVAILABLE (@objectstack/cloud-connection): rewritten. Measured: the only emitter is the purge-sample-data door (marketplace-install-local-plugin.ts, the !ql || !metadata branch, 500, introduced by fix(cloud-connection): install-local purge deletes seed rows through the engine by their seed key #21773). So the comment now states that condition, rather than adding it as an "also".
  2. RESEED_SKIPPED: not edited. The claim does not hold on this tree. Since fix(cloud-connection): refuse install-local sample data for a session with no active organization (ADR-0123 D2/D4) #21780 (e09f1aca), a walled session with no active organization gets 403 PERMISSION_DENIED on the purge (and on the reseed), through NO_ACTIVE_ORGANIZATION_CODE. RESEED_SKIPPED is now emitted only by the reseed, for its other declines. The existing comment ("reseed declined to run; message carries why") is accurate.
  3. OS_PROTOCOL_INCOMPATIBLE (@objectstack/metadata-core): rewritten to name both doors. POST /api/v1/packages (runtime/src/domains/packages.ts) and, since fix(cloud-connection): install-local runs the ADR-0087 D1 protocol handshake and refuses with the packages door answer (422) #21805, POST /api/v1/marketplace/install-local both answer through the shared protocolIncompatibleAnswer.

Acceptance notes

  • The quota branch answers 429 with no code in its body (measured above). This is deliberate: auth-manager.test.ts pins bodyCode: undefined, so both walls look the same from outside. It is untouched here.
  • sendPhoneInviteSms still throws a plain Error('SMS_SERVICE_REQUIRED: ...') when no SMS service is wired. No door reaches that throw, because its one caller gates on isSmsServiceAvailable() first. It is the delivery path, so it was out of scope and is unchanged.
  • Files outside the expected surface, all made false or required by this change: the checklist item, the two regenerated reference pages, and scripts/engine-double-contract.pinned.json. That last one records the new pinned test double, written by check-engine-double-contract.mjs --write, additions only.

Generated by Claude Code

claude added 8 commits October 5, 2026 06:52
…ers 400 SMS_SERVICE_REQUIRED

The no-provider branch of deliverPhoneOtp threw a plain Error, which
better-call maps to a 500 with a null body. It now throws better-auth's
APIError (400) carrying SMS_SERVICE_REQUIRED, registered for
@objectstack/plugin-auth in the ADR-0112 error-code ledger beside
EMAIL_SERVICE_REQUIRED.

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
…COMPATIBLE match their emitters

Comment-only, riding the SMS_SERVICE_REQUIRED registration in the same file.

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
…S_SERVICE_REQUIRED (revision 5)

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
…nd the pinned-double ledger records it

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the size/m label Oct 5, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 5, 2026
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

14 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/environment-routing.mdx (via /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • 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/getting-started/examples.mdx (via /api/v1/packages (route, a path literal in a comment 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/contracts/metadata-service.mdx (via /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class), /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class), SMS_SERVICE_REQUIRED (literal, a string literal in ERROR_CODE_LEDGER; a string literal in deliverPhoneOtp))
  • content/docs/permissions/permission-sets.mdx (via /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • content/docs/protocol/kernel/error-handling.mdx (via /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • content/docs/protocol/kernel/http-protocol.mdx (via /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • content/docs/ui/apps.mdx (via /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))

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

  • content/docs/releases/v17/17-0.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object), /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • content/docs/releases/v17/17-1.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/releases/v17/17-4.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object), /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • content/docs/releases/v17/17-5.mdx (via /api/v1/packages (route, a path literal in a comment in ERROR_CODE_LEDGER))

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 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 142 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 5b2d189e28d5563cbcaa84ac912219017692ea97 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 5b2d189e28d5563cbcaa84ac912219017692ea97

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: c6b17163b6f5f913d143262621a9e328faede810
Local-runs: none

① Derived judgments

Net diff read three-dot against origin/main (merge-base 27991556): 10 files, +417/−30. Every accept-set or public-surface change it implies, judged:

  1. ErrorCode / REGISTERED_ERROR_CODES / ApiErrorSchema.code gain SMS_SERVICE_REQUIRED under @objectstack/plugin-auth (packages/spec/src/api/error-code-ledger.zod.ts:530). This is the one widening: the ledger header itself says registering a code widens the published contract face. Right. (a) No existing row names the condition: in plugin-auth's row EMAIL_SERVICE_REQUIRED is email, PHONE_NOT_ENABLED is the plugin off, INVITE_SMS_FAILED is a failed send; NOT_SUPPORTED (the card's parenthetical) is not a StandardErrorCode member at all, and UNSUPPORTED is registered only under two other packages' rows, so an honest code for this package needs a plugin-auth row either way. (b) [finding] @objectstack/rest registers four generic synonyms the standard catalog already covers (CONFLICT, NOT_FOUND, FORBIDDEN, INTERNAL) — contract call, not a cleanup #8211 synonym rule: standardSynonymOf matches on an HTTP reason phrase or on every token being inside one standard member; SMS_SERVICE_REQUIRED is no reason phrase and the token SMS is in no standard member, so no waiver is owed; the admission suite (error-code-ledger.test.ts) ran green in Test Core. (c) The spelling already existed in this package as the sendPhoneInviteSms plain-Error prefix (auth-manager.ts:5436), so one name now covers one condition. (d) Row placement beside the email sibling rather than sorted: no gate sorts the row and the row was already unsorted (INVALID_REQUEST, OAUTH_REGISTER_FAILED sit out of order on main), acceptable. (e) check:error-code-provenance, check:error-code-casing, check:generated, check:api-surface all green on the head.
  2. Wire behaviour of POST /api/v1/auth/phone-number/send-otp with no deliverable SMS service: a 500 with an empty body becomes 400 with body {message, code: "SMS_SERVICE_REQUIRED"}, thrown as better-auth's APIError('BAD_REQUEST', …) (auth-manager.ts:5358-5366), the same construction the quota branch and every other typed refusal in this file use. Right, and it is the card's Done-when verbatim (same typed API error as the quota branch, a pin on status and code). No input is newly accepted, so plugin-auth's accept set is unchanged. The net diff of auth-manager.ts is that one branch plus four comment lines (:740, :3562, :5316, :5333); the quota branch (smsQuotaExceededApiError, 429) and the delivery path below it are byte-unchanged. isPhoneOtpDeliverable() is untouched, so features.phoneNumberOtp in /api/v1/auth/config still reads false on such a deployment and the no-provider vs production-log-only distinction is the existing one.
  3. Status 400 (the card asked for "a 4xx"): right, the email sibling answers fail(400, 'EMAIL_SERVICE_REQUIRED', …) in admin-import-users.ts:432.
  4. Message text: names the capability, the admin fix and the password fallback; carries no OTP and no tracker number (check:doc-authoring green). Right.
  5. request-password-reset: unchanged, still 200 {status:true} with nothing sent; the door pin proves the refusing send was reached for a registered number and the status does not leak existence. Right.
  6. Regenerated reference pages (content/docs/references/api/{contract,error-code-ledger}.mdx, one row each, +322 more): generated output of the ledger row, check:generated and Build Docs green. Right.
  7. content/docs/permissions/authentication.mdx: the two "fails loudly with NOT_SUPPORTED" passages now state 400 + code; the first also states the reset endpoint's unchanged {status:true}. Right. No other content/docs sentence at the head still names NOT_SUPPORTED for phone OTP.
  8. docs/qa/platform-checklist/areas/identity-auth.json (auth-method-matrix rev 5): clause 4, its negative, step 4, the fixtures row and the variant re-pointed, a bare 500 named a FAIL, the verify cites the new door pin. Right (check:platform-checklist green); the clause was literally false after this diff without it.
  9. scripts/engine-double-contract.pinned.json: +15/−0, three rows for the new test double's delete/findOne/update, written by the gate's own --write. Right, additions only.
  10. Riding comments in the ledger (DRIVER_UNAVAILABLE, OS_PROTOCOL_INCOMPATIBLE): comment-only, no code, status or owner moved, so no surface change. Verified against the head: the only DRIVER_UNAVAILABLE emitter is cloud-connection/src/marketplace-install-local-plugin.ts:1997, the purge-sample-data !ql || !metadata branch at 500; OS_PROTOCOL_INCOMPATIBLE is answered by runtime/src/domains/packages.ts:1300 and marketplace-install-local-plugin.ts:1086, both through protocolIncompatibleAnswer. Right. They exceed the claim's letter ("no spec edit beyond that one ledger line") but carry no contract weight; the card records their acceptance as surface revision 1.
  11. Tests: phone-otp-no-sms-service-refusal.test.ts drives real AuthManager.handleRequest over the installed better-auth pipeline and pins 400, the code, ErrorCode.safeParse(code).success, OTP absent from the body, production log-only refused with nothing sent, dev log-only still delivering; auth-manager.test.ts pins isAPIError, BAD_REQUEST/400, body.code at the unit seam. Imports (@objectstack/spec/api, @objectstack/objectql) are declared deps (check:undeclared-dep-imports, check:cross-package-test-inputs green). Right.
  12. Residue, no verdict weight: packages/plugins/plugin-auth/src/auth-plugin.ts:805 is a fifth comment that still says "(NOT_SUPPORTED)", in a file the diff did not reach. Same class as the four it rewrote; a one-line follow-up, not a contract matter.

Check-runs on the head, latest run per name, 35 names: 31 success; Console Pin Gate and Packed-tarball smoke (opt-in) skipped by design; Auto Label and Check PR Size skipped on the 09:38 labeled-event run (both succeeded on the 09:32 run; neither is a derived gate family). Check Changeset succeeded on both runs. No red.

② Semver level

Changeset .changeset/21793-phone-otp-no-provider-4xx.md: @objectstack/spec: minor, @objectstack/plugin-auth: patch. Matches what the diff publishes. The widening is the ledger registration (a T4 registration tell), housed in spec, so spec takes at least minor; plugin-auth publishes a bug fix in a released package (a bare 500 becomes a typed 400, nothing newly accepted), so patch, the level the quota-branch fix of the same function took. Not skip-changeset: both packages ship the change. No narrowing: nothing that parsed before is refused, no author-writable spelling is removed or renamed, so no migration and no ADR-0087 marker is owed.

Clause-②: yes (widening) — the PR body's line 2 and the changeset body carry the identical line, well-formed under the closed yes|no + (widening)|(narrowing) spelling. The claim (comment 5989470829) declared no conditionally and pre-authorized this exact amendment for the new-code branch; the card's review (5991880705) amended it in the same act. check-changeset-no-major, check-adr-0087-registration, check-empty-changeset all green in Lint & Repo Gates; check:pm-widening-tells green.

③ Boundary flags

open_questions: empty. Dev deviations, each answered:

  1. Files beyond the claim's surface (checklist item, two regenerated reference pages, the pinned-double ledger): each required or made false by the change (①.6–①.9). Answered, accepted.
  2. Four comment-only lines in auth-manager.ts outside the no-provider branch: the net diff confirms they are comments naming the new answer, and mergeable_state is clean on the head. Answered.
  3. Door pin as a plugin-auth unit-layer file instead of packages/qa/dogfood/test/: the seam is the real handleRequest over the real better-auth router, which is the wire answer the card asked to pin; nothing a dogfood boot would add is in question. Answered; the [PM seat] domain:cli — 🟢 os-elon-musk · session_01BmsuLyUeuG5CNpZFMH1jzS #6024 declaration lapses.
  4. Riding comments: two rewritten and verified against the head (①.10); the third (RESEED_SKIPPED) not written, which changes nothing in this diff. Answered.
  5. Commit trailers in AGENTS.md's model-free form: not a contract matter, hooks accepted the push. Answered.
  6. The objectui read for H4: outside this brief's inputs; the wire body is the vendor-shaped {message, code} the quota branch already answered with, so nothing in the contract turns on it. Answered.
  7. Full package suites at b37edd67, gates at c6b17163: superseded by the head's check-runs (Test Core 1/6–6/6 and aggregate, four Type Check jobs, three Dogfood gates, Build Core, Temporal Conformance all green). Answered.

The two out-of-scope findings (quota branch answers 429 without code; sendPhoneInviteSms keeps its plain Error behind the isSmsServiceAvailable() gate) are correctly left out of this diff and recorded in the PR's acceptance notes. Nothing escalated.

Implemented-by: claude/issue-21793-phone-otp-no-provider-4xx
Reviewed-by: session_011K3zqE8Pv1Evw5hc8tZCnN

VERDICT: PASS

akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Oct 7, 2026
…d decision in words instead of a tracker number (stage 18) (objectstack-ai#21870)

Part of objectstack-ai#20749
Clause-②: no

Stage 18 of this card: the next area of class (e), the test strings
shipped under `packages/spec/src`, as ruled in `5902360492` on objectstack-ai#20513.
This stage takes the fourth name-ordered file group directly under
`packages/spec/src/data/`: the 17 test files that carry an id from
`filter.test.ts` to `object.test.ts`. They carried 103 messages and 114
tracker ids, citing 81 records. Every one of those ids now either states
what its record decided, in words (form D), or is dropped where the
title already says it. Text only: no assertion, identifier, test count
or code comment changes.

## Census at the base (`c9be1f179d`)

Instruments: `census10.cjs` (md5 `9d08602ab972b4b8643c90d64d40fa41`),
`census.cjs` (md5 `6e42a45a926d375013c32d62f16a296e`), `census-wide.cjs`
(md5 `c98410a19529c439adb0afbfb00026a2`) and `dirtable.cjs` (md5
`dda605c54745b4a60cc14c9a686e4eff`). They are byte-identical to the
copies stages 10 to 17 used. A literal counts as a test title when its
folded message is argument 0 of a `describe` / `it` / `test` call,
`.each` / `.skip` / `.only` chains included. Everything else is an
"other" string.

The base is `c9be1f179d`, one commit past the claim's `969ffba25e`. That
commit (objectstack-ai#21857) touches only `plugin-security`, so the test census is
the same. Both instruments read **1045 messages / 1108 ids in 225
files**, the seat's reading and stage 17's head reading.

| directory | files | messages / ids | titles | other |
|:--|--:|--:|--:|--:|
| `ui/` | 84 | 396 / 419 | 378 / 401 | 18 / 18 |
| `api/` | 40 | 189 / 201 | 181 / 193 | 8 / 8 |
| `data/` (this PR: the fourth group) | 35 | 185 / 200 | 184 / 199 | 1 /
1 |
| `system/` | 34 | 154 / 165 | 128 / 138 | 26 / 27 |
| (files directly in `src/`) | 30 | 118 / 120 | 117 / 119 | 1 / 1 |
| `ai/` | 1 | 2 / 2 | 0 | 2 / 2 |
| `contracts/` | 1 | 1 / 1 | 0 | 1 / 1 |
| **total** | **225** | **1045 / 1108** | **988 / 1050** | **57 / 58** |

The group reads **103 messages / 114 ids in 17 files**, the seat's
figures, file for file. Every one of them is a test title:

| file (under `data/`) | messages / ids | titles | other |
|:--|--:|--:|--:|
| `filter.test.ts` | 18 / 19 | 18 / 19 | 0 |
| `form-delete-behavior-options.test.ts` | 1 / 1 | 1 / 1 | 0 |
| `form-return-type-options.test.ts` | 1 / 1 | 1 / 1 | 0 |
| `hook-body.test.ts` | 4 / 4 | 4 / 4 | 0 |
| `hook.test.ts` | 10 / 10 | 10 / 10 | 0 |
| `import-coercion.test.ts` | 2 / 2 | 2 / 2 | 0 |
| `import-mapping-target.test.ts` | 2 / 2 | 2 / 2 | 0 |
| `injected-system-column-provenance.test.ts` | 2 / 2 | 2 / 2 | 0 |
| `injected-system-columns.test.ts` | 1 / 1 | 1 / 1 | 0 |
| `inline-grid-column-currency-scale-refused.test.ts` | 5 / 5 | 5 / 5 |
0 |
| `inline-related-columns.test.ts` | 3 / 3 | 3 / 3 | 0 |
| `managed-api-affordance.test.ts` | 2 / 2 | 2 / 2 | 0 |
| `masked-field-types.test.ts` | 1 / 1 | 1 / 1 | 0 |
| `numeric-column-representation.test.ts` | 1 / 1 | 1 / 1 | 0 |
| `object-image-field.test.ts` | 1 / 1 | 1 / 1 | 0 |
| `object-strictness-batch20.test.ts` | 13 / 17 | 13 / 17 | 0 |
| `object.test.ts` | 36 / 42 | 36 / 42 | 0 |
| **17 files** | **103 / 114** | **103 / 114** | **0** |

Five more test files sit in the same name range and carry no id:
`hook-api`, `hook-body-stored-metadata-target`, `inline-grid-columns`,
`mapping-connector-source` and `mapping`. They are not touched.

- **Controls.** Lit, a title and an "other" string:
`data/query.test.ts`, outside the group, reads 11 / 11 at the head as at
the base, its "other" string at `:202` included. Dark:
`data/import-mapping-target.test.ts` reads 0 / 0 at the head while 2 of
its comment lines still carry a number. Planted in scratch copies of
head files: an id put into a `hook-body.test.ts` title reads 1 / 1, and
an id put into a `masked-field-types.test.ts` comment reads 0.
- **A wider pattern** (any `#` plus digits) reads the same totals as the
gate pattern in 16 of the 17 files at the base. `object.test.ts` reads
one more, at `:857`, which is the colour value `'#00FF00'` and not a
tracker id. At the head the wider pattern reads 0 in 16 files and that
same colour in `object.test.ts`.
- **At the head:** 942 messages / 994 ids in 208 files. The 17 files
read 0 / 0, and no other file moved.

## How the area was chosen

`data/` has no subdirectory to split by, so its stages take name-ordered
file groups near the ~100-id bound. Stage 17's re-cut named this group
at 114, with `object.test.ts` alone carrying 42, and this census reads
114, so the rule needed no re-cut.

**Named for the next stages** (re-cut from the head census, 942 / 994;
`data/` 82 / 86 left, in 18 files):
- `data/`, one more stage: `query-transport.test.ts` to
`validation.test.ts` (11 files, 34 / 34) with `data/driver/` (7 files,
48 / 52): 82 messages / 86 ids.
- `ui/` 419, about four stages. `api/` 201, two. `system/` 165, two. The
files directly in `src/`, 120, one.
- The three docblock needles (`ai/build-progress.test.ts:236`, `:237`,
`contracts/approval-service.test.ts:274`), one stage with their
docblocks.

## What each id became

34 literals (41 ids) now state a decision in words. 69 literals (73 ids)
drop a number the title already explains.

Every cited record was read with its comments through REST. 73 answer
200; `framework#2536` is counted there, because the repository was
renamed `framework` → `objectstack` (`apps/docs/lib/layout.shared.tsx`
records the rename). The old name answers 403 from this session, and the
same number under the current name is the compactLayout retirement that
the title names. Eight answer 404, and each decision was read from what
landed, by REST GET of the landing commit and its CHANGELOG entry: objectstack-ai#6571
(`2f3e79351e`), objectstack-ai#10165 (`8012960508`), objectstack-ai#10347 (`530c1df653`), objectstack-ai#10527
(`5649efbf93`), objectstack-ai#11195 (`b372318836`), objectstack-ai#11408 (`f11fc61c51`), objectstack-ai#13644
(`34ce8e7dbe`) and objectstack-ai#18012 (`176b03582e`).

| record(s) | literal (under `data/`) | now reads |
|:--|:--|:--|
| objectstack-ai#5685 | `filter.test.ts:77` | "string comparands — the ordering slots
take a string, which is what the date macros emit". `string` joined the
four ordering slots because the platform's own date-macro resolver
produces only strings. |
| objectstack-ai#14080 | `filter.test.ts:166` | "null comparand — refused in the four
ordering slots, like a null list member". Ruled A: refused at the same
entrance, in the same shape, as the null `$in` member. |
| objectstack-ai#6571 (404) | `filter.test.ts:364` | "string endpoints — `$between`
takes the ISO and clock strings the platform produces". The sibling half
of objectstack-ai#5685, from `2f3e79351e`. |
| objectstack-ai#7711 (2) | `filter.test.ts:549` | "the whole-filter face refuses
these field conditions too — green while the group branch was a
non-strict catch-all". The group branch became `.strict()`, so a refused
member has nowhere to land. |
| objectstack-ai#5222 | `filter.test.ts:576` | "leaves the four ORDERING slots taking
a reference — a column-to-column comparison, and the prescribed
alternative". `$field` compiles to a same-table column comparison on SQL
push-down. |
| objectstack-ai#5322 | `filter.test.ts:1541` | "leaves the empty-combinator
identities accepted — each reduces to its boolean unit". Ruled:
`{$and:[]}` is every row, `{$or:[]}` none, `{$not:{}}` none. |
| objectstack-ai#14104 | `filter.test.ts:1851` | "FieldReferenceSchema.addDays — a
whole-day offset on a reference, an integer or another column". Ruled A:
an offset on the field reference. |
| objectstack-ai#16923 | `filter.test.ts:1998` | "filter.zod.ts docblock @examples — a
`$field` comparand names a column of the same row". The relation-path
example was the wrong half; the same-table prose was right. |
| objectstack-ai#14010 | `hook.test.ts:352` | "runAs — a hook may run as `system` or
`user`, and inherits by default". Ruled: `runAs: 'system' \| 'user' \|
'inherit'`, default `'inherit'`. |
| objectstack-ai#13644 (404) | `hook.test.ts:529` | "Referential-Cleanup Marker —
declared, and true only on a reference-cleanup write by the engine".
Adopted by maintainer ruling, from `34ce8e7dbe`. |
| objectstack-ai#4269 | `hook.test.ts:924` | "defineHook — the authoring factory, so a
hook is validated where it is written". The `defineDatasource` pattern:
the convention-scan path got a parse at authoring time. |
| objectstack-ai#3493 | `hook.test.ts:1148` | "PRESERVES `preserveAudit` through a
parse (the opt-in a historical import uses to keep its audit stamps)". |
| objectstack-ai#5945 | `hook.test.ts:1286` | "HookContext.api typing — the minimum
scoped context the docs teach: `object()` and `transaction()`". Ruled
option C. |
| objectstack-ai#4173 (2) | `import-coercion.test.ts:23`, `:48` | "import boolean
tokens — one table for the server coercion and the Import Wizard
preview" and "import reference types — exported from spec, not copied at
each consumer". |
| objectstack-ai#8116 | `injected-system-column-provenance.test.ts:53` | "provenance
derivation at its spec home — where the author-time linter can reach
it". Ruled option 1: the derivation moved into the contract package,
which the linter may import. |
| objectstack-ai#5378 | `injected-system-columns.test.ts:17` |
"resolveInjectedSystemColumns — the injected columns, so author-time
validation resolves them too". |
| objectstack-ai#20045 | `inline-grid-column-currency-scale-refused.test.ts:172` |
"SHAPE PARITY with the currency FIELD refusal (its ruled text carried,
not reworded)". "ruling B" is the ruling that retired `scale` on a
currency field; the letter went with the id. |
| objectstack-ai#7521 | `managed-api-affordance.test.ts:57` | "is the exact shape the
boot only warned about, now named at authoring time (sys_environment /
sys_package)". Ruled: an authoring-time check, with boot left at warn
and strip. |
| objectstack-ai#16318 | `numeric-column-representation.test.ts:24` | "the numeric
physical-representation table — one table the driver and the migration
generators both read". Ruled C: one table in `packages/spec`, both
producers read it. |
| objectstack-ai#4001 (2) | `object-strictness-batch20.test.ts:93`, `:229` | "批 20,
unknown keys refused — …", as stage 15 wrote "batch D, unknown keys
refused". `:132` already says it and keeps only "批 20". |
| objectstack-ai#1535, objectstack-ai#4519, objectstack-ai#4522 | `object-strictness-batch20.test.ts:124` | "the
root itself was already closed, on parse as well as create() — this
batch is the level BELOW it". |
| objectstack-ai#5014 | `object-strictness-batch20.test.ts:187` | "⚠️ `systemFields`
is the batch's ONE union flattened to a bare `Invalid input` — …". |
| objectstack-ai#5677, objectstack-ai#6365 | `object-strictness-batch20.test.ts:380` | "… — since
the unit anchor is injected, the property governs
`owning_business_unit_id` too". objectstack-ai#6365 is dropped: the title already
states its correction. |
| objectstack-ai#11195 (404) | `object-strictness-batch20.test.ts:422` | "the two
`userActions` vocabularies stay disjoint, the three adopted view keys
included — …". `b372318836` adopted `group` / `hideFields` / `rowColor`
onto the view block. |
| objectstack-ai#4001, objectui#4772 | `object-strictness-batch20.test.ts:528` | "批 20
— `IndexSchema` is closed (the held 14th site, once the console index
editor converged on it)". |
| objectstack-ai#10527 (404) | `object.test.ts:188` | "retention + ttl + archive
triple — refused unless the ttl restates the age bound". From
`5649efbf93`. |
| objectstack-ai#10347 (404) | `object.test.ts:192` | "still accepts the ttl + archive
pair, whose ttl cutoff picks the rows to archive (no retention)". From
`530c1df653`. |
| objectstack-ai#2834 | `object.test.ts:264` | "accepts retention.onlyWhen with scalar
and $in predicates (mixed tables, where only terminal rows age out)". |
| objectstack-ai#10165 (404) | `object.test.ts:289` | "accepts ttl.onlyWhen with the
canonical null predicate — so rows whose value is absent are spared".
From `8012960508`. |
| objectstack-ai#3175 | `object.test.ts:1101` | "ownership record-model field —
declared, so the opt-out the registry reads can be authored". |
| objectstack-ai#11408 (404), objectstack-ai#10144 | `object.test.ts:1713` | "ObjectSchema editMode
(declared by maintainer ruling: the renderer reads it, so the spec
declares it)". From `f11fc61c51`, in objectstack-ai#10144's declare-or-rule-out
family. |
| objectstack-ai#14935, objectstack-ai#14637 | `object.test.ts:2135` | "isPublicSharingEnabled — the
one canonical standing share-link policy predicate". The runtime mirror
was retired. |

**Dropped where already stated** (69 literals, 73 ids). A number goes
only where the title already says its decision, for example "$field
members are refused (objectstack-ai#7596)", the four `crypto.hash` titles in
`hook-body.test.ts` (objectstack-ai#4391), the `objectstack-ai#20045 —` prefix on the four other
describes of the currency-scale file, and twenty-nine `object.test.ts`
titles such as "managedBy: retiring the overloaded `system` bucket
(objectstack-ai#3355)". `(ADR-0049)`, `(ADR-0066)`, `(ADR-0100)` and `(ADR-0087 …)`
stay: they cite decision records by number, not tracker ids.

**Three judgements, each declared:**
- `filter.test.ts:298` keeps "ruled 2026-08-31" and drops only
`(objectstack-ai#13357)`. The sibling title at `:656` names "the 2026-08-31 ruling",
so the date stays as its anchor.
- The `批 20` label stays on the four batch-20 describes, because the
file's own name and header carry it. Only `objectstack-ai#4001` and `objectui#4772`
went.
- `object-strictness-batch20.test.ts:563` drops "(objectstack-ai#5114 class)": the
title already says what is pinned, that the tombstones keep their
prescription through the strict close.

## Readers

- **Test-name filters:** none. No tracked script, workflow or config
passes `-t` / `--testNamePattern`.
- **Snapshots:** none. No `__snapshots__` directory is tracked under
`packages/spec`.
- **Projects:** none of the 17 files is listed in
`packages/spec/vitest.repo-tests.json`; all 17 run in the `local`
project.
- **By substring:** every old literal, plus a window around each id (311
needles), was searched across the tracked tree outside its own file. No
gate, doc, filter, snapshot or `scripts/check-*.mjs` self-test reads
one. The 8 needle hits fall on 7 lines:
  - a code comment in `object.zod.ts:2532`;
  - two release-owned CHANGELOG lines;
- a sibling title in this card's `system/` stage
(`system/job.test.ts:471`);
- sibling titles in `cli` (`extract-hook-body.test.ts:179`),
`driver-sql` (`sql-driver-16318-numeric-representation.test.ts:64`) and
`objectql` (`engine.test.ts:831`).
  None reads a spec test title.
- **The files by name:** 96 references to these file names outside
CHANGELOGs. The gate ledgers among them (`test-typecheck-debt.json`,
`engine-double-contract.pinned.json`,
`objectql-double-limit.baseline.json`) key on the file and on error
signatures, not on a title, and each gate exits 0 at the head.
`scripts/check-org-identifier.mjs` counts `session: { … tenantId … }`
literals in `hook.test.ts`, which this PR does not touch. ADR-0129
quotes "name-as-identity", a title this PR does not change.

## Text-only proof

Stage 10's scratch tool (`textonly10.cjs`, md5
`d5e4801dbb4329ab1984da91e92fc47c`) compares base and head file by file
on three legs:
1. **Skeleton:** the full AST, with string pieces masked. It must be
identical.
2. **Comments:** every comment, byte-equal.
3. **Strings:** each changed string leaf must sit in a test-call title
position or on a declared line, must carry a tracker id before, and must
carry no `#` plus digits after. This stage declares no line: every
changed leaf is a title.

- **Result:** 17 of 17 files SAME on all three legs, with the per-file
counts predicted in writing before the run.
- **Totals:** 103 changed string leaves in 103 literals, all titles. The
diff's `+` and `-` lines are exactly the 103 planned lines, and every
file keeps its line count.
- **Controls (10 of 10 as predicted, on scratch copies, each anchor hit
once in the reported run):** identifier rename DIFF; numeric literal
DIFF; comment edit COMMENT DIFF; a non-title string given an id
VIOLATION; a rewritten title given a new id VIOLATION; a title that was
id-free at base edited VIOLATION; one title reverted to base SAME; an
`it.each` row name given an id VIOLATION; an undeclared `expect` message
changed VIOLATION; a title re-split into a `+` chain DIFF. The non-title
control's first anchor matched nothing (0 hits, so nothing ran). Its
anchor was corrected and all ten were run again.

**Test counts:** the 17 files were run at the base, in a separate base
worktree, and at the head, with `--project local --project repo`. Both
sides read 670 tests, all passed, with the same count and status
sequence per file in 17 of 17. 378 full test names change, and each
equals the base name with the planned replacements applied (0
mismatches). No full name repeats on either side.

## `main` since the base

Re-fetched just before this PR opened, `origin/main` was two commits
past the base (`9f9510f25e`: objectstack-ai#21858, objectstack-ai#21862). Neither touches any of the
17 files; the one `packages/spec` path among them is
`api/error-code-ledger.zod.ts`. So `main` was not merged. `git
merge-tree` onto `9f9510f25e` is clean. No open PR touches the 17 files.

## Changeset: `skip-changeset`

Measured, not assumed:
- `npm pack --dry-run` of `@objectstack/spec` lists 2068 files. 0 of the
17 touched files are in it, and no `*.test.ts` at all. The controls
`src/data/filter.zod.ts` and `dist/index.mjs` are in it.
- In the built `dist/`, a new phrase and an old literal each read in 0
files. The control `Unrecognized key` reads in 42.

So this PR publishes nothing, and no changeset is added.

## Verification (at `21f39afe57`)

- `pnpm turbo run build` over all packages: 71 / 71.
- `@objectstack/spec`:
  - `vitest run --project local`: 615 files, 18387 passed, 1 todo.
- `typecheck` exit 0, including `check:test-typecheck` (52 files / 246
errors / 135 pinned signatures held). Its program holds all 17 touched
files, counted with `tsc --listFilesOnly -p tsconfig.test.json`.
  - `check:generated`: all 15 generated artifacts up to date.
- **Gates:** `dispatch-gates --commands` derived 79 families, the same
set as stages 13 to 17, and all 79 exit 0. `--ran` reconciles: 79
derived, 79 run, 0 NOT-MEASURED, 0 UNRUN, every family with its exit
code recorded.
- The five roster families whose rosters sit under a touched directory
were also run, and each exits 0: `check:meta-url-spelling`,
`check:spec-changes`, `check:authz-resolver`, `check:error-code-casing`
and `check:filter-alias-parity`.
- The derivation printed `STALE TREE`:
`scripts/engine-double-contract.pinned.json` moved on `main` after the
base. This diff adds and changes no engine double,
`check:engine-double-contract` exits 0 on this tree, and the queue
re-runs it on the merged generation.
- **ESLint, a proven narrowing:** `--no-inline-config` over the 17 files
reads 0 errors and 0 warnings. The population comes from ESLint's own
config: 17 configured, 0 ignored. No file sets `parserOptions.project`
or `projectService`, so no untouched file's verdict can move.
- `check-governed-merges --test`: NOT governed, 206 changed lines.

## Acceptance notes

- **No needle in this group.** Every id was a test title; no expected
value of an assertion over a source docblock was found. The three known
needles are untouched.
- **Same-id test titles in this card's later stages** go with those
stages: 38 lines in `packages/spec/src`, 9 in `system/` (for example
`system/job.test.ts:471`, the `job.timeout` twin of `hook.test.ts:1382`)
and 29 in `ui/` (for example the `objectstack-ai#4001 批 15` / `批 16` / `批 18` / `批 19`
describes and `ui/view-filter-rule-wire-id.test.ts`'s four `objectstack-ai#5114`
describes).
- **Same-id test titles in other packages** are their lanes' test-string
shares. A search of `describe` / `it` / `test` lines outside
`packages/spec/src` finds 140 lines citing ids this PR handled:
`objectql` 23 (13 files), `service-analytics` 18 (8), `lint` 17 (9),
`driver-sql` 11 (7), `cli` 10 (4), `runtime` 10 (7), `platform-objects`
5, `plugin-audit` 5, `plugin-security` 5, and fewer in 16 more places,
among them two `packages/spec/scripts/*.test.ts` titles (outside `src/`)
and one title each in `examples/app-crm` and `examples/app-showcase`.
- **Code comments still carry ids** in these files and their sources,
for example the `// [objectstack-ai#20150]` block at
`import-mapping-target.test.ts:17`, the `objectstack-ai#4001 批 20` header of
`object-strictness-batch20.test.ts` and `object.zod.ts:2532`. Comments
are not this card's share, and none is touched here.

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

Co-authored-by: Claude <noreply@anthropic.com>
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Oct 7, 2026
… code package ships, so a runtime-package set's only stored row is no longer deleted (objectstack-ai#21873)

Fixes objectstack-ai#21860
Clause-②: no

## What this changes

The Discard Overlay action (`POST
/api/v1/security/permission-sets/:id/discard-overlay`) is declared to
refuse any permission set that is not package-declared, so that it can
never destroy a set the environment authored (its docblock, and the
permission-sets docs page). It decided "package-declared" by asking
whether any engine-registry item of the set's name carried a package id
(`_packageId ?? packageId`). The registry holds stored rows as well as
artifacts, and the metadata list read (`GET /api/v1/meta/permission`,
issued by every Studio page load) stamps a stored row's `package_id`
onto its body as `_packageId`. So a set saved into a writable runtime
package passed the check after the first list read: the action answered
`200` and deleted the set's only `sys_metadata` row. The drift
diagnostics (`computePermissionSetDriftDiagnostics`) read the package id
the same way, and their `overlay_shadow` detail names Discard Overlay as
the remedy.

Two edits, both in `packages/plugins/plugin-security/src`:

1. `permission-set-overlay-discard.ts`: eligibility is
`classifyPackagedPermissionSet`'s verdict for the set's name. That is
the classifier the lock's two write doors and the overlay detection
reading already ask, over the same engine registry. Only `packaged`
proceeds. `org` and `unknown` are both refused with the action's
existing refusal (`PermissionDeniedError`, `403 PERMISSION_DENIED`),
because the safe direction for a destructive action is the reading's: a
set this environment cannot prove a code package ships is never deleted.
The refusal's sentence now says the set is not shipped by any installed
code package (for `unknown`: that this could not be determined, with the
classifier's reason).
2. `permission-set-drift.ts`: the declared population is gated on the
same verdict, per name.

No second evaluator. In both files the per-item test that remains below
the verdict (`_packageId ?? packageId`) no longer decides anything: it
only picks, among the registry items of a name the classifier has
already called code-shipped, which body is used (the degraded-kernel
resync body in discard, the compared bodies in drift), by the same
selector as before. So for every code-shipped set the accepted
population, the bodies and the audited or reported package are what they
were.

This builds on PR objectstack-ai#21857 (landed as c9be1f1, merged into this branch):
there the classifier learned to skip a tenant-authored registry item
(`_provenance: 'org'`), so a runtime-package set's stored row reads
`org`. It already excluded the `'sys_metadata'` runtime-shadow sentinel.

Not touched: `packaged-permission-set-lock.ts`,
`permission-set-projection.ts`, `packages/spec`, any error code, route,
export or exported signature.

Docs: the permission-sets page's Discard Overlay sentence now names the
sets the action refuses (a set created in the environment, a clone, a
set saved into a writable runtime package). "Package-declared" on its
own reads as covering a runtime package's set.

## Measurements, at the door (showcase, booted through
`@objectstack/verify`, two boots on one database file)

| shape | old reading (ablation A below, and this branch before objectstack-ai#21857
landed) | this PR (62143b0) |
|---|---|---|
| set saved into a writable runtime package, after the list read |
`200`, `overlaysDiscarded: 1`: its only `sys_metadata` row deleted |
`403 PERMISSION_DENIED`, every row intact |
| org-owned set (data door) | `403 PERMISSION_DENIED` (no package id on
its item) | `403 PERMISSION_DENIED`, every row intact |
| clone ("Clone to customize") | `403 PERMISSION_DENIED` (no package id
on its item) | `403 PERMISSION_DENIED`, every row intact |
| control: `showcase_contributor` (shipped by `com.example.showcase`)
with a legacy overlay | `200`, overlay discarded, record healed to the
artifact | unchanged |

The runtime-package set's registry row after the list read is `{
_packageId: 'com.dogfood.discard21860', _provenance: 'org' }` (asserted
as a precondition before any refusal is read). The control's
precondition: after the cold boot, its record enforces the legacy
overlay's grants and the boot's drift pass wrote `drift_status:
'overlay_shadow'`.

## Mechanism hypotheses, measured

- **H1 holds.** Reproduced at both levels before the fix took effect:
the unit pin read "promise resolved instead of rejecting", and the door
answered `200` with `overlaysDiscarded: 1` (the set's only stored
definition deleted).
- **H2 holds, with one refinement.** On `main` at dispatch (`5b2d189e`)
the classifier alone did NOT exclude the runtime-package set: it read
any non-sentinel package id. That is exactly what PR objectstack-ai#21857 changed in
the lock (`isTenantAuthored`), and why this PR waited for it and is
built on it. Measured on this branch before that merge: the three
runtime-package legs (two unit, one door) stayed red with the classifier
swap alone, and everything else was green. Only the `packaged` verdict
may discard. `unknown` is refused, mirroring the overlay detection
reading's direction rather than the write door's.
- **H3 holds.** The drift filter had the same reading. It is gated on
the same verdict, and a pin checks that the report and the action agree,
shape by shape: reported exactly when eligible.
- **H4 holds.** Every existing pin in both test files is unedited and
green. The door control still discards a shipped set's overlay and heals
its record.

## Pins

- `permission-set-overlay-discard.test.ts`, new block: each
environment-authored registry body is refused with `code`
`PERMISSION_DENIED` and `status` `403`, and both tables are counted and
compared whole before and after. The bodies are the runtime-package row
(`_packageId` plus `_provenance: 'org'`), the org-owned set, the clone,
and the lock's documented `'sys_metadata'` runtime shadow. A
code-shipped set sits beside them in the registry. Further cases: a
registry read that throws (`unknown`) is refused with the rows intact,
and a control discards a code-shipped set's overlay beside a hydrated
overlay item that wears the artifact's envelope.
- `permission-set-drift.test.ts`, new block: a drifted
environment-authored set with a stored definition is never diagnosed
(for each of the four shapes). The agreement pin runs the drift report
and Discard Overlay over one registry: the code-shipped set is reported
and eligible (`409 INVALID_STATE`, nothing to discard), each other shape
is unreported and refused (`403 PERMISSION_DENIED`), and no stored
definition is deleted.
-
`packages/qa/dogfood/test/permission-set-discard-overlay-eligibility.dogfood.test.ts`
(new): the three shapes made through their real doors (`POST /packages`
then `PUT /meta/permission/NAME?package=PKG`; `POST
/data/sys_permission_set`; the shipped `clone_permission_set` action's
own payload). Then a legacy overlay row for the shipped control, a cold
boot, and the list read. Then a precondition for the stamp, then each
shape's refusal with its record ids and stored rows compared before and
after. The drift report run over the booted registry judges the shipped
set and none of the three shapes. The control's discard returns `200`,
its overlay row is gone, its record is kept, and its grants equal the
shipped artifact's again.

## Ablations

The fix and its pins were committed first. Each ablation went through
`node scripts/ablation-replace.mjs` (anchor 1 to 0, blob changed), then
`pnpm --filter @objectstack/plugin-security build` and `node
scripts/ablation-dist-preflight.mjs @objectstack/plugin-security MARKER`
(exit 0, marker in `dist/index.js` and `dist/index.mjs`). The dogfood
suite resolves `plugin-security` from `dist/`. Each restore was proven:
blob equal to `HEAD`, `git diff HEAD` empty, a rebuild, `--absent`
preflight exit 0, and an empty `git status --porcelain`.

They ran before objectstack-ai#21857 landed, on a local, never-pushed composition:
this branch at a4d82d3 plus objectstack-ai#21857's two source files, blob-identical
to 9e3e32e. The plugin-security source in that composition is
byte-identical to this PR's head. Only objectstack-ai#21857's two test files differ,
and they were not in the composition.

| ablation | what was put back | unit result | dogfood result |
|---|---|---|---|
| A | discard's eligibility reads "an item of this name carries a
package id" again (the verdict rebuilt as: some `readDeclared` item has
the row's name and a truthy `_packageId ?? packageId`) | 3 failed / 29
passed: runtime-package and sentinel refusals, and the agreement pin | 1
failed / 6 passed: runtime-package set, `200` with `overlaysDiscarded:
1` |
| B | drift's population reads the package id again (the classifier gate
made always-true) | 3 failed / 29 passed: runtime-package and sentinel
population pins, and the agreement pin | 1 failed / 6 passed: the
drift-population leg |

The org-owned, clone and control legs stayed green under both, as they
must: the old reading already refused the first two and accepted the
third. A first attempt at ablation A used a typed replacement that
narrowed the verdict's type, so the DTS step failed. The JS bundle still
carried the marker and the same three unit pins and one door pin went
red. It was redone with a cast that keeps the declared type, and the
numbers above are from the clean run.

## Tests and gates (head 62143b0 unless noted)

- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2`: 167 files, 3611 passed, 45 skipped (on 50f93c1, the
merge of `origin/main` at c9be1f1; the only later commit is a comment
in the dogfood file).
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0,
including `check:test-typecheck` (0 errors). `pnpm --filter
@objectstack/dogfood typecheck`: exit 0 (its `tsconfig.json` includes
`test/**`).
- `pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2
test/permission-set-discard-overlay-eligibility.dogfood.test.ts`: 7
passed on 62143b0. Together with objectstack-ai#21857's
`permission-set-lock-row-provenance.dogfood.test.ts`: 21 passed on
50f93c1.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 97 commands on 62143b0; all 97 ran,
every exit 0. `--ran`: 97 derived, 97 run, 0 NOT-MEASURED, 0 UNRUN. The
first battery, on 50f93c1, found one real red:
`check:cross-package-test-inputs` read a docs path spelled in the
dogfood file's header comment as a test input. The comment now names the
page in words (62143b0). It also hit two PREREQUISITE NOT MET (exit 3:
`check:skill-examples`, `check:dual-build-cjs-loads`), cleared by
building the eight packages they named before the second battery. The
derivation printed a stale-tree note: `origin/main` moved two commits
(objectstack-ai#21858, objectstack-ai#21862) after the merge, and one of them edits
`scripts/engine-double-contract.pinned.json`. Those commits touch
neither this diff's packages nor its files, and `git merge-tree` against
them is clean.
- Lint, narrowed and proven: `pnpm exec eslint --no-inline-config
--format json` over the five touched TypeScript files gives 5 files in
the JSON output, 0 errors, 0 warnings, on 62143b0. The population is
the files this diff touches. The `.md` and `.mdx` files are outside the
lint globs in `eslint.config.mjs`, which never enables type-aware
linting (no `parserOptions.project`, no typed rules, stated in its own
comment), so this diff cannot move the verdict on any untouched file.
The repo-wide `pnpm lint` is CI's.

## Acceptance notes

- **Route.** The suggested route was to replace both `_packageId ??
packageId` readings with the verdict. Measured, the eligibility half is
replaced. The per-item selector is kept below the verdict, because the
two alternatives both change something for code-shipped sets. Keying it
on the verdict's package id changes which bodies are compared when two
code packages ship one name (ADR-0048 §3.4 lets them coexist under
distinct registry keys). Dropping it would let a projection echo with no
package id into the drift comparison.
- **The `'sys_metadata'` sentinel shape** is pinned at the unit level
only. It is the runtime shadow the lock module's docs describe; no door
producing it was measured here. The old reading accepted it, and the
classifier already refused it on `main`.
- **Drift's reach.** The boot's drift pass runs before any list read,
and the boot's hydration does not stamp the package id (measured by
objectstack-ai#21857). So on the showcase the drift misjudgment shows only in a later
run of the diagnostics after a list read. The door pin runs
`computePermissionSetDriftDiagnostics` over the booted stack's registry
after the list read.
- **Observation, not filed (carrier: none).** In the degraded-kernel
branch (no metadata protocol), discard's resync body is the first
registry item of the name that carries any package id. If a hydrated
overlay of a code-shipped name preceded its artifact in registry order,
that body would be the overlay's. Nothing measured shows that order. It
is unchanged here, by H4.
- **Changeset.** `.changeset/21860-discard-overlay-eligibility.md`,
`patch` for `@objectstack/plugin-security`, with `Clause-②: no` copied
from the claim. The action's declared population (sets a code package
ships) is restored, and no accepted input widens.

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

---------

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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-auth: phone-number send-otp with no SMS provider answers 500 with an empty body in production instead of a 4xx naming NOT_SUPPORTED

2 participants