fix(engine-core): propagate package disable to metadata listings and the data plane (#7557) - #7700
Merged
os-help merged 2 commits intoAug 11, 2026
Conversation
…the data plane (#7557) Half (A) of #7557: a package at `status: 'disabled'` had its nav and views correctly dropped while `GET /api/v1/data/<object>` still answered 200 with every row and `GET /meta/objects` still listed the object. Reproduced on a real server before the change, over two disable/enable cycles. Both skips were deliberate and gave the same reason — filtering objects "would break data queries that depend on their schema". That conflated two classes of reader, which are now separated: * RESOLUTION readers keep serving. `registry.getObject` and `registry.listItems('object')` still return a disabled package's objects; migrations, cross-package references and the runtime authoring gate's object universe all resolve through them. Disable stays reversible and still destroys no data. * API readers now stop. `getMetaItems` drops the `object`/`objects` exemption (`package` is still never filtered, or a disable could never be undone), and the data plane refuses in `assertObjectRegistered` — the one gate every findData/getData entry point funnels through. The data-plane refusal is loud rather than silent: a new `OBJECT_PACKAGE_DISABLED` / 404 naming the cause and the remedy, instead of a bare `OBJECT_NOT_FOUND` that sends a caller hunting for a typo. The 404 status matches the closest sibling switch, `OBJECT_API_DISABLED`. Registered in the ADR-0112 ledger. `SchemaRegistry.isObjectPackageDisabled` resolves names through the same `resolveObjectKey` the read path uses, so the gate and the read cannot disagree about which contributor entry a bare name addresses (#6808's lesson). Half (B), envelope only: `DELETE /packages/:id` on the dispatcher door stated `success: true` unconditionally and forwarded the protocol's own `{ success: false, deletedCount: 0 }` underneath it. Per-item failures now answer 400 `PACKAGE_DELETE_PARTIAL` with the failed items and the uninstall cleanup outcomes, matching the direct-mount REST door of the same route — including its carve-out that zero metadata rows is still a successful uninstall. The remaining half of (B), a persistence defect where `deletePackage` finds zero rows while package-bound rows exist, is reported for transfer rather than patched from the consumer side. Tests: one table-driven test walks every reader of the disable predicate and states which class each belongs to, so a new reader that forgets to consult it fails there rather than in one test per surface. Both halves reverse-verified independently.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 4 package(s): 113 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
`OBJECT_PACKAGE_DISABLED` joins the ADR-0112 ledger, so the generated `content/docs/references/` pages move with it: the ledger page gains the entry, and every page rendering the ErrorCode union bumps its "+259 more" count to "+260". Regenerated with `gen:schema && gen:docs`, never hand-edited. Release-owned `content/docs/releases/` is untouched.
Collaborator
Author
|
CI status on Fixed and pushed ( Not mine — runner network/TLS, both before any repo code executes. Not retried yet: GitHub refuses
For the record, the suites this change actually touches were run locally and are green: objectql 3230, metadata-protocol 1062, runtime 2022, spec 9867, rest 1403, plus Generated by Claude Code |
This was referenced Aug 11, 2026
os-help
marked this pull request as ready for review
August 11, 2026 12:43
Collaborator
Author
|
Ready + queued by the engine-core seat ( Review verdict and follow-up bookkeeping are on the issue: #7557. On merge, that card goes Generated by Claude Code |
os-help
deleted the
claude/issue-7557-package-disable-uninstall-propagation
branch
August 11, 2026 12:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #7557 — ships half (A) in full plus the envelope half of (B). The persistence half of (B) is located with evidence and reported for transfer, not patched from the consumer side (see "Half (B): what shipped and what is transferred").
Verification record
Every premise re-measured against
origin/main(5db0dcaat branch point) on a real running server (pnpm dev --fresh, showcase app), not from the working tree.Premise (A) — CONFIRMED, and the asymmetry located
With a runtime-installed package holding two package-bound
sys_metadataobjects and three rows:GET /api/v1/data/reprob_aGET /api/v1/meta/objects(from pkg)The PM's pointer to
registry.ts:2157(isPackageDisabled) and the disable-hiding read at:2139is accurate, and the asymmetry is one line above the filter:listItemsreturns early forobject/objectsatregistry.ts:2128-2131(return this.getAllObjects(packageId)) before ever reaching its own disable filter at:2145. The protocol-level filter then exempts the same two types explicitly (protocol.ts:4330-4334).Only two call sites consult the predicate repo-wide:
registry.ts:2145andprotocol.ts:4336. That is the whole enforcement surface.Premise (B) — the envelope producer is NOT where the card expected
The card pointed at
packages/rest/src/package-routes.ts:376. Falsified by measurement: that route already handles the failure correctly — it answers400 PACKAGE_DELETE_PARTIALwhenresult.failedCount !== 0(package-routes.ts:381-390) and400 PACKAGE_DELETE_FAILEDotherwise. It never produces apersistedkey at all.The real producer is the dispatcher twin,
packages/runtime/src/domains/packages.ts— reproduced live, byte-for-byte the shape in the issue:…with the package-bound
sys_metadatarows still present afterwards.DELETE /packages/:idhas two doors (this dispatcher and the direct-mount REST registrar, which shadows it only when apackageservice is registered); they disagreed, and the dispatcher is the one that was lying. That door ispackages/runtime— engine-core territory, in lane.Half (A): the fix
The two skips were deliberate and both gave the same reason — filtering objects "would break data queries that depend on their schema". That conflated two classes of reader, and this PR separates them explicitly:
registry.getObjectandregistry.listItems('object')still return a disabled package's objects. That is load-bearing, not caution:protocol.ts:3119resolves the runtime authoring gate's object universe throughlistItems('object'), so blanking it would make unrelated authoring fail reference validation. Migrations and cross-package references resolve throughgetObject. Untouched.getMetaItemsdrops theobject/objectsexemption.packageis still never filtered — filtering it would make disable irreversible, since the Packages page is the only surface that can re-enable.Nothing in the repo reads
getMetaItems({type:'object'})internally, so the exemption bought no safety and cost the enforcement.The "refuse loudly vs hide" choice — settled by measurement, not escalated
Recorded here per the three-axis discipline; it did not diverge on product semantics, so no
needs_decision.OBJECT_API_DISABLED(enable.apiEnabled: false) → 404 with a distinct code (rest-server.ts:1602-1610). "This object exists but is switched off" already had a house answer; this follows it rather than inventing a second one. It also keeps the data plane consistent with the listing, which now drops the object too — the object is absent everywhere on the API.OBJECT_NOT_FOUNDsends a caller — an AI agent especially — hunting for a typo or re-creating an object that is merely switched off. The distinct code names the cause and therefore the fix. This is why "hide silently" was rejected even though the status is the same.Verified on the wire for the list, single-record and write paths. Registered in the ADR-0112 ledger under
@objectstack/metadata-protocol. Note the data-plane envelope keepscodeas a sibling oferror— that is the existing shapeOBJECT_NOT_FOUNDuses inmapDataError; flattening it toerror.codeper ADR-0112 D5 is pre-existing and out of scope here.The gate lands in
assertObjectRegistered— the single seam everyfindData/getDataentry point already funnels through (protocol.ts:6252,:6730,:6794,:6802,:6848,:6906,:7004), so one gate covers every data entry point. It is optional-called (typeof registry.isObjectPackageDisabled === 'function') because registry doubles across the suites implementisPackageDisabledbut not this.SchemaRegistry.isObjectPackageDisabledresolves names through the same privateresolveObjectKeythe read path uses — #6808's lesson applied to enforcement, so the gate and the read cannot disagree about which contributor entry a bare name addresses. It asks about the owner, so an overlay from a disabled package cannot take the object it decorates offline.Live after-state
GET /data/reprob_aOBJECT_PACKAGE_DISABLED/meta/objectsfrom pkgsys_metadatarowsThe card's honest negative is preserved and pinned: disable destroys no data — rows and metadata identical before, during and after, and re-enable restores service completely.
Half (B): what shipped and what is transferred
Shipped (in lane): the dispatcher door now answers
400 PACKAGE_DELETE_PARTIALon per-item failures, carrying the failed items and the uninstall cleanup outcomes (a failed permission revocation is a ghost grant — it must survive the failure path). The rule is copied deliberately from the REST door of the same route so the two doors stop disagreeing, including its carve-out: zero metadata rows is still a successful uninstall (a runtime-registered package that never published metadata has nothing insys_metadata), so the predicate isfailedCount > 0, not!persisted.success. Checked before the not-found test, because an all-rows-failed uninstall also hasdeletedCount === 0and answering 404 for rows that are demonstrably present and stuck is the same lie one layer over.Transferred (out of lane) —
domain:metadata. The persistence defect is separate and untouched:protocol.deletePackage(protocol.ts:11486-11488) runsand found zero rows while three rows with exactly that
package_iddemonstrably existed — the same predicate returns 3 through the data plane (GET /data/sys_metadata?filters=[["package_id","=","com.repro.b"]]→ 3 records). So thewhereclause is not intrinsically wrong; the scoping the protocol's engine applies to it is. The strongest candidate is the strictorganization_idequality against rows stored env-wide (organization_id IS NULL) — the same defect class this package already fixed once, insys-metadata-repository.ts:887-901, where the identical strict equality "silently dropped" env-wide drafts and was replaced by an$orover[{organization_id: oid}, {organization_id: null}](the "orphaned draft" bug). Patching that from here would be papering over the producer, so it is reported rather than fixed.Note this half is invisible to the envelope fix by construction: it yields
failedCount: 0, so it legitimately still reads as success at the dispatcher. It stops being a lie only once the producer stops missing rows.Open finding #7221 (
unregisterPackageremoves only object contributors) was read for shape; the measured fix path does not run through it, so it is untouched.Tests
One table walks every reader rather than one test per surface, since the bug is a divergence between readers of a shared predicate.
packages/objectql/src/package-disable-enforcement.test.tsenumerates all six readers with the class each belongs to, and asserts the whole table in a singletoEqual— deliberately notexpect()inside the loop, which would throw on the first failing reader and hide the rest, in a file whose purpose is showing which readers diverged. Adding a reader means adding a row and declaring its class. RealSchemaRegistry+ real protocol over an in-memorysys_metadataengine; a registry double would be measuring the double.The refusal is asserted on
code+statusper ADR-0112, never "it threw" — plus a control that a genuinely absent object still answersOBJECT_NOT_FOUND, so the two cases stay distinguishable.Reverse verification — direction predicted before running, both halves independently
getMetaItemsobject exemption restored/metaobject rows red; data-plane and both resolution rows greenisObjectPackageDisabledbranch removed fromassertObjectRegisteredfailedCount > 0branch removedThe success cases cannot distinguish the versions on their own — that asymmetry is intended: they exist to stop the fix over-reaching into "any non-empty
failedarray 400s", which would break the never-published-metadata uninstall.Gates
pnpm check:durability-log-levelpnpm check:engine-double-contractnode scripts/check-engine-split-ratio.mjspnpm check:nul-bytespnpm check:error-code-casingpnpm build(closure)@objectstack/objectql@objectstack/metadata-protocol@objectstack/runtime@objectstack/spec@objectstack/restChangeset included (user-visible behaviour on both halves).
Generated by Claude Code