Skip to content

fix(metadata-protocol): a refused sys_packages delete fails the uninstall before anything is removed - #21438

Draft
objectstack-fleet[bot] wants to merge 12 commits into
mainfrom
claude/issue-21276-delete-package-refusal
Draft

objectstack-fleet[bot] wants to merge 12 commits into
mainfrom
claude/issue-21276-delete-package-refusal

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #21276
Clause-②: no

When the store refuses the sys_packages delete, DELETE /api/v1/packages/:id now answers that failure and changes nothing. The running process keeps serving the package, a disabled package stays disabled across a restart, and its metadata, tables and grants stay in place. There are two halves:

  • Protocol half. deletePackage now deletes the stored row as its first durable step, and a refusal on either channel throws before anything else is removed.
  • Door half. The dispatcher now asks deletePackage before it withdraws the package from the running registry or clears its disable record.

Cross-lane surfaces, named before the change list.

The door, measured live

pnpm dev:crm was booted twice over one SQLite file (a private OS_HOME, no --fresh) and driven as the seeded platform admin. A trigger refuses DELETE on sys_packages for the forced package only: CREATE TRIGGER refuse_forced_delete BEFORE DELETE ON sys_packages WHEN OLD.id = 'com.example.forced' BEGIN SELECT RAISE(ABORT, '…'); END. The control package has no trigger.

step main 22c2d6f4d, forced protocol half only (6d823ae7a), forced this branch (608838a73), forced control, every tree
DELETE /api/v1/packages/:id 200, success: true 500 DATABASE_ERROR 500 DATABASE_ERROR 200
GET, same process 404 404 200 404
GET after a restart 200: comes back 200 200 404
the package disabled first, then the forced delete; GET after a restart not measured enabled: true, status: installed enabled: false, status: disabled —

The disable leg has a control leg on 6d823ae7a: the package was disabled and the server restarted with no DELETE, and it came back disabled.

The 500 message reads: "Package 'com.example.forced' was not uninstalled: the package registry could not delete its stored record, and a package whose record is kept comes back on the next restart, so its metadata, data and grants were left in place. The reason is in the server log." The driver's SQLITE_CONSTRAINT_TRIGGER line stays in the server log and on cause.

The steps of deletePackage, in their new order

# step durable? can it refuse? on refusal
1 tenant-scope check no yes, 400 TENANT_SCOPE_REQUIRED thrown; nothing changed
2 sys_metadata read no yes, 503 or a classified refusal thrown; nothing changed
3 sys_packages delete (on main: after step 4) yes, the first yes: returned { success: false }, or thrown thrown through packagePersistFailureError; nothing changed (on main: console.warn, then success)
4 per-item sys_metadata deletes and table teardown yes yes: authorization, lock, store fault collected in failed[]; the door answers 400 PACKAGE_DELETE_PARTIAL
5 registry withdrawal (SchemaRegistry.uninstallPackage, which also releases the namespace) process state yes: another package extends an object this one owns (ADR-0029) console.warn; the package leaves at the next restart
6 uninstall cleanups yes yes reported in cleanups[]

The error shape is #21243's, unchanged. A declared 4xx leaves as the producer answered it. Anything else is a 500 that quotes nothing, with the original on cause and a catalogued code carried (DATABASE_ERROR from a live SQL driver; INTERNAL_ERROR derived for a returned { success: false }). No code is minted. There is no undo machinery, because nothing durable precedes step 3.

Steps that can still refuse after the store delete (triage's ruling asks for them to be moved ahead or reported here):

  • Step 4. An authorization or lock refusal could in principle be decided first, but only by a dry run of deleteMetaItem's own checks, which is another protocol.ts region. A store fault cannot be decided ahead. This is unchanged from main, where the store delete ran after these steps whatever they answered. The door answers 400 PACKAGE_DELETE_PARTIAL and lists what was left.
  • Step 5. SchemaRegistry has no verb that answers "would this uninstall be refused?" without performing it, and a copy of its extender predicate would be a second place that must agree with the first. This is the one behaviour this PR moves later at the door. Before this PR, the door's own up-front uninstallPackage refused this case with a 500 before anything was deleted. Now the stored rows go first, and the door answers what the store bears out: 200 with registryRemoved: false. The process keeps serving the package until it restarts. This is pinned below and reported to the seat.
  • Step 6. A cleanup is a data-plane delete, so deciding it means doing it. This is unchanged: each refusal is reported in cleanups[].

Door half

DELETE /api/v1/packages/:id used to run registry.uninstallPackage(id) and setPackageDisabled(environmentId, id, false) before it called protocol.deletePackage. It now runs in this order:

  1. the unchanged refusals: requireManageMetadata, the ADR-0070 read-only gate, and the organization-scope mirror;
  2. an existence read: registry.getPackage(id);
  3. deletePackage. A throw is answered through errorFromThrown, exactly as before, and nothing has been touched at that point;
  4. only then the withdrawal from the running registry (skipped when deletePackage already withdrew it), and the clear of the disable record for a package this request found.

The refusal envelopes are unchanged: 403, 422 WRITABLE_PACKAGE_REQUIRED, 400 TENANT_SCOPE_REQUIRED, the thrown deletePackage failure, 400 PACKAGE_DELETE_PARTIAL and the 404. The 404 now asks whether the package existed rather than whether it was withdrawn, so a package whose withdrawal was refused after its stored row was deleted is not answered "not found".

A host with no persisted half keeps its old behaviour. There the withdrawal is the uninstall, and its refusal is the request's refusal.

Tests (code at 608838a73; the head e54dee56e differs from it only in the changeset)

New door pins in packages/runtime/src/package-uninstall-store-refusal.integration.test.ts (4 tests). They use the shipped pieces, booted twice over one SQLite file: a real ObjectQL and SqlDriver (better-sqlite3); the real PackageServicePlugin, whose start() hydrates sys_packages; the real protocol; the real HttpDispatcher; and the disable seed AppPlugin plants at boot. A trigger refuses the sys_packages delete. The pins:

  • a forced refusal answers 500 DATABASE_ERROR, and the same process still serves the package, disabled, with its view row;
  • after a restart the package is still installed, still disabled, and still has its view row;
  • CONTROL: an ordinary delete answers 200, then 404 in the same process and 404 after a restart. Its rows are gone and its disable record is cleared;
  • the ADR-0029 extender case answers 200 with registryRemoved: false. The process keeps serving the package, its rows are gone, and the restart does not bring it back.

Protocol pins in packages/metadata-protocol/src/protocol.package-delete-refusal.test.ts (6 tests):

  • a returned { success: false } gives 500 INTERNAL_ERROR, and a thrown DATABASE_ERROR gives 500 DATABASE_ERROR. The store delete is the only step that runs, and nothing is removed;
  • a declared 409 passes through unchanged;
  • after either refusal, a restart still has the package, its metadata and its grants;
  • CONTROL: an ordinary uninstall runs in the order store delete, metadata, registry, cleanup.

Fixture change. In packages-uninstall-envelope.test.ts, the registry double answered getPackage with a package even in its "unknown package" case, and modelled "unknown" only through uninstallPackage's return value. The door now reads existence with getPackage, so both verbs read one registered flag. The case still asserts the same 404.

Results:

  • runtime, the full local project (vitest run --project local --maxWorkers=2, shards 1/2 and 2/2): Test Files 154 and 154 passed; Tests 2040 passed with 4 skipped, and 2320 passed with 15 skipped.
  • runtime typecheck (tsc --noEmit and check:test-typecheck): exit 0. The test layer's shrink-only ledger is unchanged.
  • metadata-protocol: the 6 pins passed. Its full suite passed on 6d823ae7a (Test Files 103 and 100 passed with 3 skipped; Tests 1326 passed, then 1668 passed with 19 skipped), and protocol.ts has not changed since.
  • objectql passed in full on 6d823ae7a (Test Files 183 and 182; Tests 3610 and 3777). rest passed with --project local on 6d823ae7a (255 files; 4814 tests passed and 322 skipped).

Ablations (every mutation through scripts/ablation-replace.mjs; each anchor hit once; each restore proven with the blob equal to HEAD and git diff HEAD empty)

Both pin files import their subject from src, so no dist is involved.

ablation tree red
A1, protocol: the returned-result check removed 6d823ae7a 2 of 6, both returned-channel pins
A2, protocol: the catch that logs a warning and goes on restored 6d823ae7a 3 of 6: thrown, declared 409, thrown-restart
A3, protocol: the store delete moved back after the metadata deletes 6d823ae7a 6 of 6, including both restart pins
D1, door: withdraw from the running registry before deletePackage 608838a73 2 of 4: the same-process pin (expected 404 to be 200) and the extender pin (500)
D2, door: clear the disable record before deletePackage 608838a73 1 of 4: the restart-disabled pin (enabled: true, status: installed)

Void attempts, declared: the first A1 attempt, and the first two attempts of A3's call leg, were refused by the tool before any test ran, because each replacement contained its own anchor. They were re-run.

Gates (head e54dee56e)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 65 commands, including check:route-envelope for the door file. All 65 ran on that head and exited 0. Reconciling with --ran gives "65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN", with every exit code recorded. origin/main was merged at 7d29e5c5d. The 2 commits that landed after that merge touch none of this PR's files.

ESLint was narrowed to the five touched TypeScript files, which is a measured narrowing:

  • ESLint's own --print-config resolves every one of them;
  • --format json counts 5 files, with 0 errors and 0 warnings;
  • the resolved configs carry no parserOptions.project and no projectService, so type-aware linting is off and no untouched file's verdict can move.

The repo-wide pnpm lint is CI's.

Docs

I grepped content/docs/** (outside releases/ and references/) and skills/** for uninstall and package-delete semantics: api/metadata-api.mdx, kernel/contracts/metadata-service.mdx, permissions/permission-sets.mdx, protocol/kernel/plugin-spec.mdx and deployment/publish-and-preview.mdx. Each describes a successful uninstall, and that path is unchanged, so none of their sentences is now false. No doc edits.

Acceptance notes

  • The door half is in this PR (above). The one door behaviour that moved later is the ADR-0029 extender refusal. Keeping it ahead of the store delete needs a non-mutating question on SchemaRegistry (packages/objectql/src/registry.ts, outside this claim), which is reported to the seat.
  • Observed and not filed: an ordinary uninstall of a package with no sys_metadata rows carries persisted.success: false inside the door's 200, because deletePackage computes success as failed.length === 0 && deleted.length > 0. The door does not read that field.

Generated by Claude Code

claude added 4 commits October 2, 2026 15:05
…st and fails when the store refuses

deletePackage ran the sys_packages delete after the metadata deletes,
inside a catch that logged a returned or thrown refusal and answered
success. The delete is now the first durable step; a refusal on either
channel throws packagePersistFailureError before anything is removed.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…tart and the ordinary uninstall

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…aller's limit and refuses combinators

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

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/metadata-protocol, @objectstack/objectql, @objectstack/runtime, touching 9 documentable anchor(s).

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

  • content/docs/concepts/metadata-lifecycle.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class), SchemaRegistry (symbol, a top-level class))
  • content/docs/deployment/environment-variables.mdx (via SchemaRegistry (symbol, a top-level class))
  • content/docs/kernel/contracts/metadata-service.mdx (via /api/v1/packages/:id (route, a path literal in a comment in deletePackage))
  • content/docs/kernel/services-checklist.mdx (via SchemaRegistry (symbol, a top-level class))
  • content/docs/permissions/permission-sets.mdx (via /api/v1/packages/:id (route, a path literal in a comment in deletePackage))
  • content/docs/permissions/system-context.mdx (via handlePackagesRequest (symbol, a top-level function))
  • content/docs/plugins/adding-a-metadata-type.mdx (via SchemaRegistry (symbol, a top-level class))
  • content/docs/protocol/kernel/error-handling.mdx (via /api/v1/packages/:id (route, a path literal in a comment in deletePackage))

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

  • content/docs/releases/v16.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class), deletePackage (symbol, a method of class ObjectStackProtocolImplementation), /api/v1/packages/:id (route, a path literal in a comment in deletePackage))
  • content/docs/releases/v17/17-4.mdx (via SchemaRegistry (symbol, a top-level class), /api/v1/packages/:id (route, a path literal in a comment in deletePackage))

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 — 39 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 53fd35e3e3a0b18b790ab79bd2c65f353e11ab65 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 53fd35e3e3a0b18b790ab79bd2c65f353e11ab65

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

claude added 3 commits October 2, 2026 16:31
…raws the package or clears its disable record

The door ran registry.uninstallPackage and setPackageDisabled(..., false)
before protocol.deletePackage, so a store that refused the sys_packages
delete got a 500 while the running process had already dropped the package
and its durable disable record. Existence is now read with getPackage, and
the withdrawal and the disable clear follow a deletePackage that answered.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…ete, across a restart

A real SQLite composition (ObjectQL, SqlDriver, PackageServicePlugin,
the metadata protocol and the dispatcher) booted twice over one file. The
#7557 envelope double now models an unregistered package through
getPackage, the read the door now makes.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l and removed size/m labels Oct 2, 2026
claude added 3 commits October 2, 2026 17:53
…ninstall refusal before the sys_packages delete

The ADR-0029 extender refusal was decided only by performing the
uninstall, which now runs after the store delete. Its refusal pass becomes
SchemaRegistry.assertPackageUninstallable, which unregisterObjectsByPackage
itself calls (one predicate), and deletePackage asks it before its first
durable step, so the refusal is thrown with nothing removed. The door's
late-withdrawal comment no longer says the refusal arrives there.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…try, protocol and door

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/xl and removed size/l labels Oct 2, 2026

This branch has not been deployed

No deployments
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/xl tests tooling

Projects

None yet

2 participants