Skip to content

Commit a26abbc

Browse files
fix(objectql): an uninstalled package no longer reads as read-only until a restart (#30280)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgEcGxYY8iXRzD52FM7U9N4o)_ 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 #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. <details><summary>Ablation (twice, through <code>scripts/ablation-replace.mjs</code>; the test imports source)</summary> ``` # 1. the engine's subscription deleted (engine.ts) ablation-replace: ok mutation landed: anchor 1 -> 0, blob 947b072 -> 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 (947b072) and `git diff HEAD` is empty # 2. the registry's announcement deleted (registry.ts) ablation-replace: ok mutation landed: anchor 1 -> 0, blob 9b0443f -> 8b24e75cbb5c Tests 4 failed | 3 passed (7) ablation-replace: ok restored: blob == HEAD (9b0443f) 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. </details> <details><summary>End to end, two <code>os dev --fresh</code> showcase runtimes</summary> 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. </details> <details><summary>Tests and gates on the final head</summary> 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. </details> ## 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 - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (Not touched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CbfmKbpDmbzwGpfLLp)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4be9734 commit a26abbc

5 files changed

Lines changed: 219 additions & 0 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
fix(objectql): an uninstalled package no longer reads as read-only until a restart
6+
7+
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.
8+
Metadata-protocol: none
9+
10+
An administrator who uninstalled a package through `DELETE /api/v1/marketplace/install-local/:id` got a 200 "withdrawn from the running kernel", but on the same instance the package id still read as a booted code package until the server restarted. Uninstalling a `<base>.custom` pair that an older runtime had installed as code, as the pair install now prescribes, left `POST /api/v1/packages/<base>/ensure-customization` answering `created: true` with `writable: false`, so nothing could be customized into the new pair. The engine's manifest map, which the ADR-0070 writability check reads first, kept the id after the registry had withdrawn the package.
11+
12+
`SchemaRegistry.uninstallPackage` now announces each completed uninstall to `onPackageUninstalled` listeners, after the package record is gone (never for a refused uninstall), and the engine drops its manifest entry there, so the id is released on whichever door withdrew the package, not per door. A reinstall of the same id registers it again as before. A listener that throws does not turn the completed uninstall into a refusal; it is logged at `error`.
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #30215 — an uninstalled package stops reading as a booted code package on
5+
* the running instance.
6+
*
7+
* `registerApp` puts a package's manifest in the engine's `manifests` map, and
8+
* ADR-0070's `isWritablePackage` reads that map FIRST: an id in it is a code
9+
* package, read-only. Every uninstall door (the protocol's `deletePackage`,
10+
* `DELETE /packages/:id`, the install-local DELETE) withdraws the package
11+
* through `SchemaRegistry.uninstallPackage`, and nothing took the id out of
12+
* the engine's map. So an administrator who uninstalled a package got a 200,
13+
* then `ensure-customization` on the same instance answered `writable: false`
14+
* for it until the server restarted.
15+
*
16+
* Pinned here, against a real engine and its own registry:
17+
* - the registry's uninstall drops the engine's manifest entry, so the
18+
* predicate answers writable again;
19+
* - an uninstall the registry REFUSES (ADR-0029: another package extends an
20+
* object this one owns) removes nothing, the manifest entry included;
21+
* - a reinstall of the same id puts the entry back (read-only again);
22+
* - the registry's announcement: once per withdrawn package, never for an id
23+
* it does not hold, never after unsubscribe, and a throwing listener does
24+
* not turn a completed uninstall into a refused one.
25+
*/
26+
27+
import { describe, it, expect, vi, afterEach } from 'vitest';
28+
import { isWritablePackage } from '@objectstack/metadata-protocol';
29+
import { ObjectQL } from './engine.js';
30+
import { SchemaRegistry } from './registry.js';
31+
32+
const PKG = 'com.example.tasksapp';
33+
34+
function manifestsOf(engine: ObjectQL): Map<string, unknown> {
35+
return (engine as unknown as { manifests: Map<string, unknown> }).manifests;
36+
}
37+
38+
function bootWithPackage(): ObjectQL {
39+
const engine = new ObjectQL();
40+
engine.registerApp({
41+
id: PKG,
42+
namespace: 'tasks_app',
43+
objects: [{ name: 'tasks_app_task', fields: { name: { type: 'text' } } }],
44+
} as any);
45+
return engine;
46+
}
47+
48+
describe('ObjectQL — an uninstall forgets the package\'s manifest (#30215)', () => {
49+
it('reads a registered code package as read-only, and writable once the registry uninstalls it', () => {
50+
const engine = bootWithPackage();
51+
expect(manifestsOf(engine).has(PKG)).toBe(true);
52+
expect(isWritablePackage(engine, PKG)).toBe(false);
53+
54+
expect(engine.registry.uninstallPackage(PKG)).toBe(true);
55+
56+
expect(engine.registry.getPackage(PKG)).toBeUndefined();
57+
expect(manifestsOf(engine).has(PKG)).toBe(false);
58+
expect(isWritablePackage(engine, PKG)).toBe(true);
59+
});
60+
61+
it('keeps the manifest entry when the registry refuses the uninstall (an extended object)', () => {
62+
const engine = bootWithPackage();
63+
engine.registry.registerObject(
64+
{ name: 'tasks_app_task', fields: {} } as any, 'com.example.addon', undefined, 'extend',
65+
);
66+
67+
expect(() => engine.registry.uninstallPackage(PKG)).toThrow();
68+
69+
expect(engine.registry.getPackage(PKG)).toBeDefined();
70+
expect(manifestsOf(engine).has(PKG)).toBe(true);
71+
expect(isWritablePackage(engine, PKG)).toBe(false);
72+
});
73+
74+
it('puts the entry back on a reinstall of the same id', () => {
75+
const engine = bootWithPackage();
76+
engine.registry.uninstallPackage(PKG);
77+
expect(isWritablePackage(engine, PKG)).toBe(true);
78+
79+
engine.registerApp({
80+
id: PKG,
81+
namespace: 'tasks_app',
82+
objects: [{ name: 'tasks_app_task', fields: { name: { type: 'text' } } }],
83+
} as any);
84+
85+
expect(manifestsOf(engine).has(PKG)).toBe(true);
86+
expect(isWritablePackage(engine, PKG)).toBe(false);
87+
});
88+
89+
it('leaves every other package\'s manifest entry in place', () => {
90+
const engine = bootWithPackage();
91+
engine.registerApp({ id: 'com.example.notesapp', namespace: 'notes_app' } as any);
92+
93+
engine.registry.uninstallPackage(PKG);
94+
95+
expect(manifestsOf(engine).has('com.example.notesapp')).toBe(true);
96+
expect(isWritablePackage(engine, 'com.example.notesapp')).toBe(false);
97+
});
98+
});
99+
100+
describe('SchemaRegistry.onPackageUninstalled (#30215)', () => {
101+
afterEach(() => { vi.restoreAllMocks(); });
102+
103+
function registryWith(id: string): SchemaRegistry {
104+
const registry = new SchemaRegistry();
105+
registry.installPackage({ id, name: id, version: '1.0.0', type: 'app' } as any, 'code');
106+
return registry;
107+
}
108+
109+
it('announces a withdrawn package once, after its record is gone', () => {
110+
const registry = registryWith(PKG);
111+
const seen: Array<{ id: string; recordGone: boolean }> = [];
112+
registry.onPackageUninstalled((id) => {
113+
seen.push({ id, recordGone: registry.getPackage(id) === undefined });
114+
});
115+
116+
vi.spyOn(console, 'warn').mockImplementation(() => {});
117+
registry.uninstallPackage(PKG);
118+
registry.uninstallPackage(PKG); // nothing left to withdraw
119+
120+
expect(seen).toEqual([{ id: PKG, recordGone: true }]);
121+
});
122+
123+
it('announces nothing for an id the registry does not hold, or after unsubscribe', () => {
124+
const registry = registryWith(PKG);
125+
const listener = vi.fn();
126+
vi.spyOn(console, 'warn').mockImplementation(() => {});
127+
const unsubscribe = registry.onPackageUninstalled(listener);
128+
129+
registry.uninstallPackage('com.example.never-installed');
130+
expect(listener).not.toHaveBeenCalled();
131+
132+
unsubscribe();
133+
registry.uninstallPackage(PKG);
134+
expect(listener).not.toHaveBeenCalled();
135+
});
136+
137+
it('a throwing listener neither refuses the uninstall nor starves the next listener, and is logged at error', () => {
138+
const registry = registryWith(PKG);
139+
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
140+
const next = vi.fn();
141+
registry.onPackageUninstalled(() => { throw new Error('boom'); });
142+
registry.onPackageUninstalled(next);
143+
144+
expect(registry.uninstallPackage(PKG)).toBe(true);
145+
146+
expect(registry.getPackage(PKG)).toBeUndefined();
147+
expect(next).toHaveBeenCalledWith(PKG);
148+
expect(error).toHaveBeenCalledTimes(1);
149+
expect(String(error.mock.calls[0][0])).toContain(`package "${PKG}" was uninstalled`);
150+
expect(String(error.mock.calls[0][0])).toContain('boom');
151+
});
152+
});

‎packages/objectql/src/engine.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,12 @@ export class ObjectQL implements IObjectQLEngine {
740740
constructor(hostContext: Record<string, any> = {}) {
741741
// Use provided logger or create a new one
742742
this.logger = hostContext.logger || createLogger({ level: 'info', format: 'pretty' });
743+
// [#30215] `registerApp` writes `manifests`; an uninstall withdraws the
744+
// package through the registry's own verb, on every door, so the entry goes
745+
// with the registry's record. Left behind, ADR-0070's `isWritablePackage`
746+
// (which reads this map first) kept an uninstalled package read-only until
747+
// a restart.
748+
this._registry.onPackageUninstalled((packageId) => { this.manifests.delete(packageId); });
743749
// Pick up production hardening switches from env so deployers can
744750
// enforce strict-body without code changes:
745751
// OBJECTQL_STRICT_HOOKS=1 → unresolved hooks throw at bind time

‎packages/objectql/src/registry-module-mock.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,11 @@ export function createRegistryModuleMock(
183183
// a refused registration is put back. The double runs the body as given:
184184
// it records nothing, so it has nothing to put back.
185185
restoringOnRefusal: vi.fn((register: () => unknown) => register()),
186+
// [#30215] Every engine subscribes in its constructor, so its manifest map
187+
// forgets a package the registry uninstalls. The double never uninstalls,
188+
// so it never announces: it records the subscription and answers the
189+
// unsubscribe the real member answers.
190+
onPackageUninstalled: vi.fn(() => () => {}),
186191
reset: vi.fn(),
187192
metadata: { get: vi.fn(() => new Map()) },
188193
...instanceOverrides,

‎packages/objectql/src/registry.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3619,6 +3619,47 @@ export class SchemaRegistry {
36193619
this.initialDisabledPackageIds = new Set(ids);
36203620
}
36213621

3622+
/** See {@link onPackageUninstalled}. */
3623+
private packageUninstalledListeners = new Set<(packageId: string) => void>();
3624+
3625+
/**
3626+
* [#30215] Be told, once per package, that {@link uninstallPackage} has
3627+
* withdrawn it — after its record is gone, never for a refused uninstall
3628+
* (that one removes nothing). Returns the unsubscribe.
3629+
*
3630+
* For a holder of state keyed by package id OUTSIDE this registry, so it is
3631+
* withdrawn on every uninstall door at once (the protocol's `deletePackage`,
3632+
* `DELETE /packages/:id`, the install-local DELETE), never per door. The
3633+
* engine is the one today: its manifest map (`registerApp` → `manifests`) is
3634+
* the code-package signal ADR-0070's `isWritablePackage` reads first, and an
3635+
* id left in it kept an uninstalled package read-only until a restart.
3636+
*/
3637+
onPackageUninstalled(listener: (packageId: string) => void): () => void {
3638+
this.packageUninstalledListeners.add(listener);
3639+
return () => { this.packageUninstalledListeners.delete(listener); };
3640+
}
3641+
3642+
/**
3643+
* Tell every {@link onPackageUninstalled} listener. Never throws: the
3644+
* uninstall has already happened, so a throwing listener must not read as a
3645+
* refused one to the door that called {@link uninstallPackage}. It is logged
3646+
* at `error` instead, because that listener's state now disagrees with this
3647+
* registry while everything looks healthy.
3648+
*/
3649+
private announcePackageUninstalled(id: string): void {
3650+
for (const listener of this.packageUninstalledListeners) {
3651+
try {
3652+
listener(id);
3653+
} catch (err) {
3654+
console.error(
3655+
`[Registry] package "${id}" was uninstalled, but a listener keeping state keyed by its id failed to drop it `
3656+
+ `(${(err as Error)?.message ?? err}) — that state still names the package until the next restart, `
3657+
+ 'while the registry no longer does. Restart the runtime to bring the two back in line.',
3658+
);
3659+
}
3660+
}
3661+
}
3662+
36223663
// ==========================================
36233664
// Restore on refusal
36243665
// ==========================================
@@ -8219,6 +8260,9 @@ export class SchemaRegistry {
82198260
if (collection) {
82208261
collection.delete(id);
82218262
this.log(`[Registry] Uninstalled package: ${id}`);
8263+
// …and, last, whoever keeps its own state keyed by this package's id
8264+
// (the engine's manifest map) drops it, on every uninstall door at once.
8265+
this.announcePackageUninstalled(id);
82228266
return true;
82238267
}
82248268
return false;

0 commit comments

Comments
 (0)