Skip to content

fix(cloud-connection): install-local purge deletes seed rows through the engine by their seed key - #21773

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-21728-install-local-purge
Oct 4, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-21728-install-local-purge

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21728
Clause-②: no

POST /api/v1/marketplace/install-local/:manifestId/purge-sample-data answered 500 DRIVER_UNAVAILABLE on every runtime. It now deletes an installed package's seed rows through the ObjectQL engine, matched by the seed's own key, in the install's own scope.

What was wrong (three defects, stacked)

  1. The handler looked up a bare driver service. No kernel registers that name; drivers register as driver.NAME. So the door always refused.
  2. Behind the lookup, it matched seed records by rec.id. The CRM example's 28 records carry no id (they key by name / email / subject), so fixing the lookup alone answers deleted: 0, skipped: 28. Ablation leg 2 below measures exactly that.
  3. It called the driver's delete directly, past every engine hook and the audit trail.

The unit suites stayed green because four of them mocked a bare driver service.

What changes

  • New packages/cloud-connection/src/marketplace-install-local-purge.ts (purgeSeedRows).
    • Key. Each dataset's externalId is read through the spec's own declaration (SeedSchema.shape.externalId, default name). Rows are keyed the way SeedLoaderService keys them for an upsert. A row whose key no seed record declares is never touched.
    • Composite keys. Reference parts (the showcase's ['team', 'project'] junction) are translated through the parent rows this purge matched, as the loader does for same-load targets.
    • No guessing. A key carried by more than one row in scope, a seed record with no key value, a key part pointing at an object this package does not seed, and an engine refusal each count in errors, with the reason logged. None of them counts as skipped, and none deletes anything.
    • Order. The reverse of the loader's own topological insert order (SeedLoaderService.buildDependencyGraph), so children go before parents. No order is written by hand.
    • Context. engine.delete(object, { where: { id } }) under SEED_WRITE_EXECUTION_CONTEXT, the posture the seed was written with. Record-change automation is suppressed; lifecycle hooks and audit run.
  • handlePurge resolves objectql and metadata (500 DRIVER_UNAVAILABLE if either is missing, now naming which). It picks the scope with the install's own two primitives, organizationWallActive and resolveActiveOrgId, and hands both to purgeSeedRows. The docblock's "bypass ACL / lifecycle hooks (same pattern as cloud purge)" sentence is gone. The response shape is unchanged.
  • Bare-driver mocks.
    • Rewritten against the engine, with a pinned delete that opens with assertEngineDeleteDispatch: heal and capability-enumeration, the two suites that drive the purge.
    • Removed, with the reason left in place: seed-replayer and tenancy-posture, where the mock was dead fixture.
  • Changeset: patch on @objectstack/cloud-connection. This is a bug fix in a released package. It adds no export and changes no schema; the door now does what its documented response says.

Zone 2 measurements

  • A1, reach (origin/main 025008a, unchanged code).

    • Setup: a scratch copy of the showcase on os dev --fresh with OS_CLOUD_URL=off, then os package install examples/app-crm/dist/objectstack.json as the admin. Result: 3/3/12/5/5 rows, all carrying the default organization.
    • POST …/purge-sample-data {} answered 500 {"code":"DRIVER_UNAVAILABLE",…}. Afterwards crm_account still held 3 rows and crm_opportunity 12.
    • The reseed answered 422 RESEED_NO_ROWS.
    • The same sequence on the fixed build:
      • The purge answered 200 {"deleted":28,"skipped":0,"errors":0} and the user row User Authored Co survived.
      • sys_audit_log gained 28 delete rows.
      • The reseed answered 200 inserted: 28. A second purge deleted 28, and a third answered skipped: 28.
  • A2, scope. The install and the reseed seed under organizationWallActive(ctx).

    • Under a wall: they seed into resolveActiveOrgId's organization, and the loader pins organization_id on both its upsert match and its writes.
    • Without a wall (single, the card's boot): no organization is pinned, and the loader's upsert match is table-wide. Measured at the door: POST /auth/organization/create on that boot answers 403 "Creating additional organizations is disabled on this deployment.", so the table is one tenant.
    • The purge reuses those two primitives exactly. Under a wall, every read is filtered to the active organization and the delete is by primary key. A walled session with no active organization gets the reseed's skip shape: 400 RESEED_SKIPPED, Purge did not run: multi-tenant-no-active-org….
    • Pinned on a real walled boot: a purge in organization A leaves organization B's 28 seed rows untouched, and no delete hook ever saw one of them.
  • A3, key. The CRM's five datasets declare externalId name / email / name / email / subject (3+3+12+5+5 = 28), with 0 authored ids. The loader writes the key to that same field (loadDataset: dataset.externalId || 'name'; externalIdKey). The purge matches on exactly that field, in scope.

  • A4, through the engine.

    • Context: SEED_WRITE_EXECUTION_CONTEXT (isSystem, skipTriggers, seedReplay). triggerHooks runs every lifecycle hook unless skipAutomations is set.
    • Real boot: beforeDelete and afterDelete each fired once per seed row (28), and the audit plugin wrote 28 delete rows.
    • Order: the loader's graph reversed; the route test proves it against the real SeedLoaderService with a manifest listing the child first.
    • Constraint, measured: with a user opportunity still requiring seed account Acme Corp (crm_opportunity.account is required + set_null), the engine refused that account's delete ("still referenced by 1 Opportunity record(s)… required and cannot be cleared"). The purge answered deleted: 27, errors: 1 with the reason logged, and the user's row was untouched.
    • No CRM hook refuses a delete; its only hook is beforeInsert/beforeUpdate.
  • A5, ledger flag. sampleDataPurged / withSampleData stay install-wide; the ledger shape is unchanged here.

    • Measured on a walled boot with a real restart: after a purge in A, GET /install-local read as B answers withSampleData: false while B still holds its 28 rows.
    • After the restart, A has 0 rows and B has 28. The healer returns at its wall check before reading anything, so it neither resurrects A nor touches B.
    • No data harm to B. The listing flag is wrong for B, and that is reported to the seat as an out-of-scope finding.
  • A6, reverse verification. Both legs ran through scripts/ablation-replace.mjs on marketplace-install-local-purge.ts, which the dogfood resolves through its source alias, so no build leg was needed.

    leg anchor → replacement blob dogfood pin
    delete no-op const result = await engine.delete(…SEED_WRITE_EXECUTION_CONTEXT }); → const result = true as boolean | number; a6856b93 → 2d004c57 5 red / 3 green
    match by rec.id const ids = rowsByKey.get(keyOf(parts)) ?? []; → const ids: string[] = rec?.id ? [String(rec.id)] : []; a6856b93 → 7e1e9c83 5 red / 3 green; purge answered deleted: 0, skipped: 28
    • The 3 that stayed green both times are the two preconditions and "organization B's seed rows are untouched".
    • Restore, both legs: blob after restore == HEAD (a6856b93) and git diff HEAD is empty.

Tests (at 2b140e9)

  • pnpm --filter @objectstack/cloud-connection exec vitest run: 37 files, 459 tests pass. The new marketplace-install-local-purge.test.ts has 16 cases (match, no-guessing, order/context/scope, reference parts, and the route under the real loader).
  • pnpm --filter @objectstack/cloud-connection typecheck: clean (both programs).
  • New dogfood install-local-purge-sample-data.dogfood.test.ts on a real boot: 8/8 pass. pnpm --filter @objectstack/dogfood typecheck is clean.
  • pnpm lint (full, eslint . --no-inline-config): exit 0.

Gates (at 2b140e9)

  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 86 families (the claim's 46 grew with the dogfood wiring, the lockfile and the ratchet baseline).
  • All 86 ran with exit 0. --ran reconciles 86 derived / 86 run / 0 NOT-MEASURED, every line carrying its exit code.
  • check:dual-build-cjs-loads first answered PREREQUISITE NOT MET (eight unrelated packages unbuilt). After building them it read 106 entry points across 66 packages, green.
  • check:slot-lookup ratcheted down 16 → 15 for the plugin file (the untyped driver lookup is gone). The baseline is committed.

Deviations

  • Two stay-out dogfood files edited: packages/qa/dogfood/package.json (devDependency @objectstack/cloud-connection) and packages/qa/dogfood/vitest.config.ts (one anchored source alias), plus their lockfile importer lines.
    • Measured reason: a relative import of the plugin from the new dogfood file puts the plugin's dynamic @objectstack/runtime import into dogfood's shrink-only check:test-source-alias set (NEW unaliased artifact import(s): @objectstack/runtime).
    • The alias is the route the config already takes for its other subjects. No test file another lane holds was touched.
  • The purge door gains a refusal it did not have: 400 RESEED_SKIPPED when a wall is up and the session has no active organization. This was dispatched as "reseed's existing skip shape and code". A dedicated code would need a spec ledger row (domain:spec).
  • @objectstack/cloud-connection gains a devDependency on @objectstack/metadata-core, for assertEngineDeleteDispatch in its fake engines, as check:engine-double-contract prescribes. It is aliased to source in its vitest config.
  • No merge commit. origin/main stayed at 025008a from dispatch to this PR, so git merge origin/main is a no-op.

Acceptance notes


Generated by Claude Code

claude added 4 commits October 4, 2026 19:01
…the engine by the seed key

The purge looked up a bare 'driver' service no kernel registers (always
500 DRIVER_UNAVAILABLE) and, behind it, matched seed records by an id
they do not carry. It now resolves the objectql engine, keys rows by each
dataset's externalId in the install's own scope, and deletes child before
parent under the seed write context.

Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz
Co-authored-by: Claude <noreply@anthropic.com>
…e the bare-driver mocks

Unit pins for purgeSeedRows (seed-key match, user rows untouched, no
guessing on ambiguous or keyless records, engine refusals counted as
errors, child-before-parent under the seed write context, organization
scope) and for the route (real SeedLoaderService order, wall scope,
no-active-org refusal). The suites that mocked a bare 'driver' service no
kernel registers are rewritten against the engine, or drop the mock.

Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz
Co-authored-by: Claude <noreply@anthropic.com>
Installs the CRM example from its build-shaped artifact through the
install-local door on the showcase, adds a user-authored row, purges and
reseeds: { deleted: 28, skipped: 0, errors: 0 }, the user row kept, every
delete seen by beforeDelete/afterDelete and the audit trail. A second boot
under an organization wall purges one organization and leaves the other's
28 seed rows untouched. Dogfood declares cloud-connection and aliases it to
source, the way it does for its other subjects.

Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz
Co-authored-by: Claude <noreply@anthropic.com>
…es combinators in its matcher; ratchet slot-lookup down

marketplace-install-local-plugin.ts reads one fewer service slot without a
type (the bare 'driver' lookup is gone): 16 -> 15.

Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation tests tooling labels Oct 4, 2026
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/cloud-connection, @objectstack/dogfood, touching 15 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/cloud-connection/vitest.config.ts, packages/qa/dogfood/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/automation/flows.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/automation/hooks.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/data-modeling/field-types.mdx (via externalId (symbol, a field of interface PurgeSeedDataset))
  • content/docs/data-modeling/fields.mdx (via externalId (symbol, a field of interface PurgeSeedDataset))
  • content/docs/data-modeling/seed-data.mdx (via externalId (symbol, a field of interface PurgeSeedDataset), organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/data-modeling/validation-rules.mdx (via externalId (symbol, a field of interface PurgeSeedDataset))
  • content/docs/deployment/cli.mdx (via MarketplaceInstallLocalPlugin (symbol, a top-level class))
  • content/docs/deployment/seed-tenancy-repair.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/getting-started/examples.mdx (via externalId (symbol, a field of interface PurgeSeedDataset))
  • content/docs/kernel/events.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/kernel/runtime-services/audit-service.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/kernel/runtime-services/sharing-service.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/permissions/authentication.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/permissions/system-context.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/protocol/kernel/config-resolution.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))

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

  • content/docs/releases/index.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/releases/v16.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))
  • content/docs/releases/v17/17-5.mdx (via organizationId (symbol, a field of interface SamplePurgeInput))

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
  • 2 changed file(s) yielded no anchor (packages/cloud-connection/vitest.config.ts, packages/qa/dogfood/vitest.config.ts) — pages documenting those are invisible to this run
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (5 routes)
  • 7 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 5 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 025008ae954311ab91ef264d1b750a00ed193b32 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 025008ae954311ab91ef264d1b750a00ed193b32

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

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 4, 2026 20:38
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 4, 2026 20:38
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit d7fff21 Oct 4, 2026
38 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21728-install-local-purge branch October 4, 2026 21:16
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Oct 7, 2026
… with no active organization (ADR-0123 D2/D4) (objectstack-ai#21780)

Fixes objectstack-ai#21774

Clause-②: no

Under an organization wall, a session with no active organization is
ADR-0123 D1's declared state. The install-local sample-data doors now
answer it the way ADR-0123 D2 / D4 require, instead of skipping quietly.

## What changed
- `resolveActiveOrgId`
(`packages/cloud-connection/src/marketplace-install-local-plugin.ts`):
the "first membership" fallback is **deleted**, not repaired. It read
`sys_organization_member`, an object nothing defines.
- **Reseed and purge** answer `403 PERMISSION_DENIED` (standard catalog,
no new code) with one sentence built in one place
(`noActiveOrganizationRefusal`). The sentence says the session has no
active organization and gives the remedy.
- **Install** still registers the env-wide package. Its `seeded` block
is `{ mode: "refused", reason }`, with the same sentence in `reason`
(was `{ mode: "skipped", reason: "multi-tenant-no-active-org" }`).
- `RESEED_SKIPPED` stays for reseed's other declines: `no-datasets`,
`objectql-or-metadata-missing`, `seed-error: …`.
- `storage-service-plugin.ts` (comment only, declared cross-lane): the
clause citing the deleted fallback now cites ADR-0123 D1. The comment's
own rule is unchanged.
- Ratchets moved down by one each for this file
(`scripts/slot-lookup-baseline.json` 15 to 14,
`scripts/query-options-erasure-baseline.json` 2 to 1). The deleted
fallback carried those sites.

## Measured before the change (`origin/main` `ebfe658c72`, real walled
boot, posture-only)
Tenancy `isolated`, `isolationActive: true`; session
`activeOrganizationId` null; 1 `sys_member` row for the admin.
- install: `200`, `seeded {mode: "skipped", reason:
"multi-tenant-no-active-org"}`
- reseed: `400 RESEED_SKIPPED` "Reseed did not run:
multi-tenant-no-active-org"
- purge: `400 RESEED_SKIPPED` "Purge did not run:
multi-tenant-no-active-org. …"
- the fallback's read: `Object 'sys_organization_member' not found`
(thrown, swallowed)

## Pins
- Unit: `marketplace-install-local-no-active-organization.test.ts`
(new). It covers walled with no active org and two memberships served by
the store, walled with an active org, and `single` (a spy proves
`resolveActiveOrgId` is never called). The purge unit pin moves to 403.
- Door pin on a real walled boot:
`packages/qa/dogfood/test/install-local-no-active-organization.dogfood.test.ts`
(new). In one session: no-org install/reseed/purge, then the same
session after `set-active` to org B, as the control. 7/7 green.
- Reverse verification (A5), committed fix at `a1a5513852`, through
`scripts/ablation-replace.mjs`, two literal anchors, nested WRAP:
- anchor 1, the seed refusal line, replaced by the old `mode: 'skipped'`
line: blob `5f88579b38e0` to `24f914c847b4`
- anchor 2, the purge 403, replaced by the old `RESEED_SKIPPED` / 400:
blob to `505004807245`
  - On disk: removed lines 0/0, injected lines 1/1.
- Unit: 4 red (the 3 no-org pins and the purge no-org pin; received
`skipped` and `400`), 33 green.
- Door: 3 red (no-org install/reseed/purge), 4 green (preconditions,
nothing seeded, org-B control).
- Restore: three legs prove blob == HEAD `5f88579b38e0` and an empty
`git diff HEAD`.

## Verification (head `0590b46d38`)
- `pnpm --filter @objectstack/cloud-connection test`: 38 files, 466
tests passed.
- typecheck green for cloud-connection, dogfood and service-storage;
`--listFiles` includes both new test files.
- full `pnpm lint`: exit 0.
- `dispatch-gates --commands` derived 79 commands; 78 ran green on the
final head. `check-empty-changeset` is red **on purpose**; see below.

## Pending release note corrected (the empty-changeset gate stays red by
design)
`.changeset/21728-install-local-purge.md` is pending, from objectstack-ai#21773; the
last version PR `617f25f8a4` predates that merge. It said a
no-active-organization purge is answered `400 RESEED_SKIPPED`. This PR
makes that sentence false, and both changesets ship in one release, so
its clause now reads `403 PERMISSION_DENIED`, naming the missing active
organization. Nothing else in that note changed. The gate's own text
classes this as a deliberate correction that a person confirms on the
PR. **Please confirm or reject this edit.**

## Acceptance notes
- The console (objectui at the pinned `.objectui-sha`,
`marketplaceApi.ts` `postLocalSampleAction`) shows `error.message` for
any non-ok status and does not branch on the code or status. The 400 to
403 move is shown to the user as the new sentence. The `os package
install` CLI does not read `seeded`.
- `resolveActiveOrgId` reads only the better-auth session. An API-key
caller (no session cookie) on a walled boot is refused with the session
wording. Before, it was skipped. This was not measured further; carrier:
none.

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

---------

Co-authored-by: Claude <noreply@anthropic.com>
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Oct 7, 2026
…ers 400 SMS_SERVICE_REQUIRED instead of a bare 500 (objectstack-ai#21858)

Fixes objectstack-ai#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 objectstack-ai#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 objectstack-ai#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 objectstack-ai#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 objectstack-ai#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](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

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

2 participants