Skip to content

fix(objectql): an uninstalled package no longer reads as read-only until a restart - #30280

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-et4qps
Nov 5, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-et4qps

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Bill · project thread

Fixes #30215

Clause-②: yes (widening) — @objectstack/objectql's exported SchemaRegistry gains the public method onPackageUninstalled(listener), which returns its unsubscribe. Nothing is refused that was accepted before.
Metadata-protocol: none

维护者速读

  • 改了什么: 注册表 uninstallPackage 撤下一个包后,向订阅者宣告一次;引擎在构造时订阅,从自己的 manifests 表里删掉该包 id。于是卸载答 200 之后,同一实例上该 id 不再被当成代码包,ensure-customization 答 writable: true。
  • 为什么改: 以前只有 registerApp 往 manifests 写,没有任何卸载门删它;ADR-0070 的 isWritablePackage 先读这张表,所以卸载过的包(例如旧运行时当代码装的 .custom 配对包,PR fix(cloud-connection,cli): install a .custom export into the target's writable pair #30207 现在叫管理员去卸)一直只读,直到重启,而卸载答复只说「已从运行内核撤下」。
  • 风险与回滚: 只在卸载成功、包记录已删之后才删 manifests 项;被拒的卸载(ADR-0029 有扩展者)什么都不动(有 pin);同 id 重装照旧写回。订阅者抛错不会把已完成的卸载变成拒绝,只记一条 error 日志。回滚即撤 PR。
  • 先看: packages/objectql/src/registry.ts 的 onPackageUninstalled / announcePackageUninstalled 与 uninstallPackage 末尾一行;packages/objectql/src/engine.ts 构造函数里的订阅一行;packages/objectql/src/registry-module-mock.ts 新增的替身成员。
  • 两分钟验证: pnpm --filter @objectstack/objectql exec vitest run --project local src/engine-uninstall-forgets-manifest.test.ts,预期 7 条全过。
  • 需要你做的: 无,席位落地。

What changes

The gap. ObjectQL.registerApp writes the package's manifest into the engine's private manifests map, and nothing ever deleted an entry. Every uninstall door withdraws a package through SchemaRegistry.uninstallPackage (the install-local DELETE, the protocol's deletePackage, the runtime's DELETE /packages/:id), and that verb never told the engine. ADR-0070's isWritablePackage (packages/metadata-protocol/src/package-writability.ts) reads engine.manifests first, so an uninstalled id kept reading as a booted code package, read-only, until a restart.

The fix: the state follows the registry, not the reader. SchemaRegistry gains onPackageUninstalled(listener), and uninstallPackage announces each completed uninstall as its last step, after the package record is gone. The engine subscribes in its constructor and drops its manifests entry. One place, so whichever door withdrew the package releases the id, and no door needs to know the engine's map exists.

  • A refused uninstall (ADR-0029: another package extends an object this one owns) throws before any mutation, so it announces nothing and the entry stays.
  • A reinstall of the same id writes the entry back (registerApp, unchanged).
  • A listener that throws is caught: the uninstall has already happened, so the door must not read it as a refusal. It is logged at error with the consequence and the remedy (restart), because the listener's state now disagrees with the registry while everything looks healthy.

Why not the other route the card offered (the uninstall answer says a restart is needed). The card prefers releasing the id. Releasing it is also the only route that is right for every door: an answer-only fix would cover the install-local DELETE and leave the same stale entry behind every other caller of the registry verb.

Why not teach isWritablePackage to cross-check the registry. That would be a reader skipping a stale entry, which the install-local door's own withdrawFromRunningKernel note rejects for the same class of defect: withdraw the registration, so every reader is right. The map's other reader (the package defaultDatasource lookup in engine-secrets-and-routing.ts) is keyed by the object's owner, which the registry already withdraws, so it was not wrong in practice; it now reads a map that agrees with the registry too.

Clause-② measured. The seat's claim predicted no; it is yes (widening) because SchemaRegistry is exported from @objectstack/objectql and gains a public method. Nothing is narrowed: the only answer that changes is that an uninstalled id is no longer read-only on the same instance.

Test double. Every engine now calls onPackageUninstalled in its constructor, so the shared vi.mock('./registry') factory (registry-module-mock.ts, used by 19 engine suites) models the member: it records the subscription and answers an unsubscribe. Without it those 19 files failed at construction (391 tests, measured on the first full run).

Verification

Unit pins. New packages/objectql/src/engine-uninstall-forgets-manifest.test.ts, 7 cases, against a real engine and its own registry, asserting through the real isWritablePackage:

  • a registered code package reads read-only, and writable once the registry uninstalls it (record gone, map entry gone);
  • a refused uninstall (an extended object) keeps the record and the map entry, still read-only;
  • a reinstall of the same id puts the entry back, read-only again;
  • another package's entry is untouched;
  • the registry announces once, after the record is gone, not for an id it does not hold, not after unsubscribe;
  • a throwing listener neither refuses the uninstall nor starves the next listener, and is logged at error once.
Ablation (twice, through scripts/ablation-replace.mjs; the test imports source)
# 1. the engine's subscription deleted (engine.ts)
ablation-replace: ok mutation landed: anchor 1 -> 0, blob 947b0720cde1 -> 64db3051ce58
     × reads a registered code package as read-only, and writable once the registry uninstalls it
     × puts the entry back on a reinstall of the same id
      Tests  2 failed | 5 passed (7)
ablation-replace: ok restored: blob == HEAD (947b0720cde1) and `git diff HEAD` is empty

# 2. the registry's announcement deleted (registry.ts)
ablation-replace: ok mutation landed: anchor 1 -> 0, blob 9b0443fea234 -> 8b24e75cbb5c
      Tests  4 failed | 3 passed (7)
ablation-replace: ok restored: blob == HEAD (9b0443fea234) and `git diff HEAD` is empty

Under ablation 1 the refusal, other-package and three registry cases stay green: they assert what the fix must not change or the registry's half alone.

End to end, two os dev --fresh showcase runtimes

Fixture: a ledger entry for com.example.showcase.custom in examples/app-showcase/.objectstack/installed-packages/, the shape an older runtime wrote when it installed a pair as code; the boot rehydrates it as code (GET /api/v1/marketplace/install-local lists it). Signed in as the dev admin.

Fixed build (head with the fix, full pnpm build):

POST /api/v1/packages/com.example.showcase/ensure-customization   (before the DELETE)
  409 RESOURCE_CONFLICT  "Package 'com.example.showcase.custom' is installed but is not the paired customization package …"
DELETE /api/v1/marketplace/install-local/com.example.showcase.custom
  200 "Cached manifest removed, the package withdrawn from the running kernel, and the uninstall cleanups … ran"
POST /api/v1/packages/com.example.showcase/ensure-customization
  201 {"packageId":"com.example.showcase.custom","created":true,"writable":true}

Control, the same flow with the engine's subscription ablated and @objectstack/objectql rebuilt (scripts/ablation-dist-preflight.mjs confirmed the line absent from all 14 built files):

DELETE …/install-local/com.example.showcase.custom   200 (same note)
POST …/com.example.showcase/ensure-customization     201 {"created":true,"writable":false}

That writable: false is the card's defect, reproduced; the restored build answers writable: true. Both servers were started on random ports and stopped by recorded PID; the fixture directory was removed.

Tests and gates on the final head

On head f8e6c780f. node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 62 families; each was run with its exit code recorded to a file before any pipe, then reconciled:

Run reconciliation — 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 62 derived famil(ies) accounted for — 62 run, 0 NOT-MEASURED (a DERIVED zero — all 62 recorded an exit code and none of them is 3).

Beyond the derivation, each exit 0:

  • pnpm lint
  • pnpm --filter @objectstack/objectql test: Test Files 549 passed (549), Tests 10870 passed (10870)
  • pnpm --filter @objectstack/objectql typecheck (includes check:test-typecheck)
  • pnpm --filter @objectstack/metadata-protocol test
  • pnpm --filter @objectstack/cloud-connection test (the install-local uninstall suites included)
  • pnpm --filter @objectstack/runtime test: Test Files 541 passed (541), Tests 8619 passed, 19 skipped

The CLI end-to-end uninstall suite (package-install-local-uninstall-cleanups.integration.test.ts, integration tier) was not run locally: it is left to CI.

Acceptance notes

  • #30217 (same install-local door) stays serial behind this card and is not touched here.
  • Noted, not filed: registerApp also indexes the manifest's datasources (registerDatasourceDef), and nothing withdraws those definitions on uninstall either. A datasource definition is keyed by its own name, not the package id, and whether a package's datasource should outlive it is a separate decision; nothing in this card measured a wrong answer from it.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (Not touched.)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: not touched.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 11 documentable anchor(s).

38 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 04f36b63f8d2f32b00428929b07268130881e30f.

⛔ 9 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 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; 103 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 — 21 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 04f36b63f8d2f32b00428929b07268130881e30f → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 04f36b63f8d2f32b00428929b07268130881e30f

⚠️ 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 04f36b63f8d2f32b00428929b07268130881e30f → 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

domain:server#3 · os-bill · session_01Utp6CbPxYq1wQmUNWvMZkC

Contract review

Served-tier: TIER_DEFAULT
Head-sha: f8e6c780f0c46932be761c5f692fc62f047cd460
Local-runs: none
Scope: packages/objectql/src/registry.ts, packages/objectql/src/engine.ts, packages/objectql/src/registry-module-mock.ts, packages/objectql/src/engine-uninstall-forgets-manifest.test.ts, .changeset/30215-uninstall-forgets-engine-manifest.md

Face: declaration (Clause-②: yes (widening) line-initial on the PR body and the changeset). No published schema, no governed path, no narrowing in the committed tree, so the lane seat records this review at TIER_DEFAULT. Reads: the full diff, card #30215 (claim 6648674444, os-dev report 6649790028), the PR body.

① Derived judgments

  1. Public surface widened, accept set unchanged. SchemaRegistry (exported from @objectstack/objectql) gains one public method, onPackageUninstalled(listener: (packageId: string) => void): () => void. Everything else added is private (packageUninstalledListeners, announcePackageUninstalled). No existing signature changes, nothing previously accepted is refused. The claim's Clause-②: no was the seat's reading before the dev measured the shape of the fix; the dev's yes (widening) is the right declaration and the claim is amended to it.
  2. Announcement point. uninstallPackage announces only on the collection.delete(id) → return true branch, after the record is gone; the return false branch (nothing to remove) announces nothing. A refused uninstall therefore never reaches a listener.
  3. Listener failure. announcePackageUninstalled catches per listener and logs at console.error, so a throwing listener cannot turn a completed uninstall into a refusal at any door, and the others still run.
  4. Subscriber lifetime. The engine subscribes in its constructor to its own _registry (private _registry: SchemaRegistry = new SchemaRegistry(), never reassigned in engine.ts), so the listener lives exactly as long as the engine and its registry; no cross-engine retention. The entry it deletes is manifests, keyed by the same id registerApp sets (this.manifests.set(id, manifest)).
  5. Test double. registry-module-mock.ts gains onPackageUninstalled: vi.fn(() => () => {}): the engine suites that mock the registry construct engines, and without the member each constructor throws. The double never uninstalls, so it never announces; this is the member the real one answers, not a behaviour change of any suite.
  6. Writability. isWritablePackage (ADR-0070) and the install-local door are not touched; the fix releases the id at its source, on every uninstall door at once.

② Semver level

@objectstack/objectql patch: AGENTS.md step 3, a non-protocol change (widening) ships patch; Metadata-protocol: none (nothing an author can write changes). No BREAKING banner is owed (nothing removed or refused), so no ADR-0087 disposition is owed.

③ Boundary flags

Implemented-by: claude/project-thread-et4qps
Reviewed-by: session_01Utp6CbPxYq1wQmUNWvMZkC

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-bill · session_01Utp6CbPxYq1wQmUNWvMZkC

席位核对

ACCEPT · head f8e6c780f0c46932be761c5f692fc62f047cd460 · card #30215 (claim 6648674444, amended 6649834975) · 2026-11-05T04:58Z

  • Shape: draft PR to main, author objectstack-fleet[bot] (relay), attribution block then Fixes #30215. 5 files, +219 −0. check-governed-merges.mjs --pr 30280: NOT governed ⇒ ordinary queue landing.
  • Declarations: Clause-②: yes (widening) (SchemaRegistry.onPackageUninstalled), Metadata-protocol: none; changeset @objectstack/objectql patch, no BREAKING (nothing removed or refused). Claim amended to match.
  • Review of record: the lane seat's TIER_DEFAULT record 6649824491 on this head, PASS (the declaration face alone; no schema, governed or narrowing face).
  • Proof (dev): reproduced on showcase --fresh with a .custom pair an older runtime installed as code: fixed build DELETE 200 then ensure-customization answers writable: true; with the engine's subscription removed from dist/ it answers writable: false (the card's defect). Ablations: 2 and 4 red, restored.
  • Gates (check-runs on this head): 48 runs, 35 success, 13 skipped, 0 failed; Lint & Repo Gates, TypeScript Type Check, Test Core, Check Changeset, Dogfood Regression Gate success. Skips read by name (path-gated UI/docs, opt-in harnesses incl. Dogfood Verify CLI, the schedule gate): check-expected-skips.mjs cannot run in this container (yaml not installed).
  • Accepted deviation: packages/objectql/src/registry-module-mock.ts (+5): the shared registry double gains the member every engine constructor now calls.
  • Carried: install-local: the cached-manifest directory is resolved from the working directory, not OS_HOME, so os dev --fresh rehydrates a previous install #30217 (install cache resolved from cwd, not OS_HOME) next from this seat.
  • Writes: dev api_writes 2, mcp_calls 3 (read-only issue_read).
  • Landing: ready + auto-merge by the seat.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 5, 2026 05:00
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 5, 2026 05:00
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit a26abbc Nov 5, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-et4qps branch November 5, 2026 05:27
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.

install-local: an uninstalled package still reads read-only until a restart, because engine.manifests keeps its id

1 participant