Skip to content

Commit b5be479

Browse files
os-zhuangclaude
andauthored
fix(protocol): persist versionless package installs; drop sys_packages on delete (#2540)
* fix(protocol): persist versionless package installs; drop sys_packages on delete (#2532) installPackage's durable half was guarded by `pkgSvc?.publish && manifest.version`, silently skipping every versionless runtime-created base ({id,name} from the builder/Setup) — in-memory only, gone after restart, while metadata/tables survived. Default the version (0.1.0) instead of skipping, log persist failures loudly, and make deletePackage drop the sys_packages row so uninstalled packages don't resurrect at boot via service-package hydration. Verified: unit tests (8/8) + three-boot E2E on one db — install → restart → still listed; DELETE → restart → stays gone. Fixes #2532 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(protocol): duplicatePackage routes through installPackage so copies persist too The duplicate path did a bare registry.installPackage — the copied base would vanish from GET /packages on restart, same #2532 mechanics. Route through installPackage to get the durable sys_packages write (and version default). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(runtime,protocol): mount missing package sub-routes; duplicates are scope-less Two more holes found browser-verifying the duplicate journey: 1. dispatcher-plugin never MOUNTED the newer handlePackages branches — duplicate (ADR-0070 D4), adopt-orphans (D5), discard-drafts, commits / commit-revert / rollback (ADR-0067) all 404'd at the hono layer on every serve stack despite working handler code. Mount them all; routing only. 2. duplicatePackage copied the SOURCE manifest verbatim — including scope (e.g. 'project'), branding the writable copy as read-only in every writability heuristic. The duplicate's manifest is now scope-less. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(objectql): align install/duplicate tests with #2540 durable persistence Two objectql protocol tests asserted the pre-#2540 behavior: - installPackage used to SKIP sys_packages persistence for versionless manifests (the {id,name} shape the builder/Setup create), so base packages vanished on restart (#2532). It now defaults version to '0.1.0' and persists. Test updated to expect persistence with the defaulted version. - duplicatePackage used to do a bare registry.installPackage write and keep the source scope. It now routes through installPackage (so the copy also lands in sys_packages) and strips scope (a duplicate must be a writable base, not inherit the source's read-only scope). Test updated to assert the routed call + scope stripped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2d34e69 commit b5be479

6 files changed

Lines changed: 234 additions & 15 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(protocol): versionless package installs now persist to sys_packages (#2532)
6+
7+
`installPackage` writes both package stores, but its durable half was guarded by
8+
`pkgSvc?.publish && manifest.version` — silently skipping every versionless
9+
runtime-created base (`{id, name}` from the builder / Setup). Those packages
10+
lived only in the in-memory registry and vanished on restart, while their
11+
metadata and tables survived. The version is now defaulted (`0.1.0`) instead of
12+
skipping, a failed persist logs loudly instead of silently, and `deletePackage`
13+
drops the `sys_packages` record so an uninstalled package no longer resurrects
14+
at the next boot (service-package hydrates that table into the registry).
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// #2532 — durable package registration. `protocol.installPackage` writes BOTH
4+
// package stores (in-memory registry + sys_packages via the `package` service),
5+
// but its persistence guard used to be `pkgSvc?.publish && manifest.version` —
6+
// silently SKIPPING every versionless runtime-created base ({id, name} from the
7+
// builder / Setup), which is exactly why those packages vanished on restart.
8+
// These tests pin the fixed contract: version is defaulted (never skipped) and
9+
// uninstall drops the durable row so packages don't resurrect at boot.
10+
11+
import { describe, it, expect, vi } from 'vitest';
12+
import { ObjectStackProtocolImplementation } from './index.js';
13+
14+
function makeImpl(overrides?: {
15+
publish?: (d: { manifest: unknown; metadata: unknown }) => Promise<unknown>;
16+
del?: (id: string) => Promise<unknown>;
17+
find?: (obj: string, q: unknown) => Promise<unknown[]>;
18+
}) {
19+
const registryCalls: Array<{ manifest: any; settings: any }> = [];
20+
const engine = {
21+
registry: {
22+
installPackage: (manifest: any, settings: any) => {
23+
registryCalls.push({ manifest, settings });
24+
return { manifest, status: 'installed', enabled: true };
25+
},
26+
},
27+
find: overrides?.find ?? (async () => []),
28+
};
29+
const publish = vi.fn(overrides?.publish ?? (async () => ({ success: true })));
30+
const del = vi.fn(overrides?.del ?? (async () => ({ success: true })));
31+
const services = new Map<string, any>([['package', { publish, delete: del }]]);
32+
const impl = new ObjectStackProtocolImplementation(engine as any, () => services);
33+
return { impl, registryCalls, publish, del };
34+
}
35+
36+
describe('installPackage — durable persistence (#2532)', () => {
37+
it('persists a VERSIONLESS manifest by defaulting version (the vanish-on-restart bug)', async () => {
38+
const { impl, registryCalls, publish } = makeImpl();
39+
const res: any = await (impl as any).installPackage({
40+
manifest: { id: 'com.example.orders', name: '订单中心' },
41+
});
42+
43+
// In-memory half.
44+
expect(registryCalls).toHaveLength(1);
45+
expect(registryCalls[0].manifest.version).toBe('0.1.0');
46+
// Durable half — the old guard skipped publish entirely for this shape.
47+
expect(publish).toHaveBeenCalledTimes(1);
48+
const persisted = publish.mock.calls[0][0] as any;
49+
expect(persisted.manifest.id).toBe('com.example.orders');
50+
expect(persisted.manifest.version).toBe('0.1.0');
51+
expect(res.package.status).toBe('installed');
52+
});
53+
54+
it('keeps an explicit version untouched', async () => {
55+
const { impl, publish } = makeImpl();
56+
await (impl as any).installPackage({ manifest: { id: 'com.example.v', version: '2.3.4' } });
57+
expect((publish.mock.calls[0][0] as any).manifest.version).toBe('2.3.4');
58+
});
59+
60+
it('stays non-fatal when the durable write fails (registry install already succeeded)', async () => {
61+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
62+
try {
63+
const { impl } = makeImpl({ publish: async () => ({ success: false, error: 'boom' }) });
64+
const res: any = await (impl as any).installPackage({ manifest: { id: 'com.example.fail' } });
65+
expect(res.package.status).toBe('installed');
66+
expect(warn).toHaveBeenCalledWith(expect.stringContaining('persist FAILED'));
67+
} finally {
68+
warn.mockRestore();
69+
}
70+
});
71+
});
72+
73+
describe('deletePackage — durable un-registration (#2532 counterpart)', () => {
74+
it('drops the sys_packages record so the package cannot resurrect at boot', async () => {
75+
const { impl, del } = makeImpl();
76+
await (impl as any).deletePackage({ packageId: 'com.example.orders' });
77+
expect(del).toHaveBeenCalledWith('com.example.orders');
78+
});
79+
});

packages/metadata-protocol/src/protocol.ts

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4455,6 +4455,21 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
44554455
}
44564456
}
44574457

4458+
// #2532 counterpart: also drop the durable `sys_packages` record —
4459+
// service-package hydrates that table back into the registry at boot,
4460+
// so leaving the row behind would RESURRECT an uninstalled package on
4461+
// the next restart. Best-effort, same posture as install persistence.
4462+
try {
4463+
const pkgSvc = this.getServicesRegistry?.()?.get('package') as
4464+
| { delete?: (id: string) => Promise<unknown> }
4465+
| undefined;
4466+
if (pkgSvc?.delete) await pkgSvc.delete(request.packageId);
4467+
} catch (e) {
4468+
console.warn(
4469+
`[protocol.deletePackage] sys_packages cleanup skipped for '${request.packageId}': ${(e as Error)?.message}`,
4470+
);
4471+
}
4472+
44584473
return {
44594474
success: failed.length === 0 && deleted.length > 0,
44604475
deletedCount: deleted.length,
@@ -4531,12 +4546,21 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
45314546

45324547
if (srcPkg?.manifest && typeof registry?.installPackage === 'function') {
45334548
try {
4534-
registry.installPackage({
4549+
// Route through installPackage (not a bare registry write) so the
4550+
// duplicated base ALSO lands in sys_packages — otherwise the copy
4551+
// would vanish from GET /packages on the next restart (#2532).
4552+
// Spread-then-strip: the source may carry `scope` (e.g. 'project'
4553+
// on a code package) — copying it would brand the duplicate as
4554+
// read-only in every writability heuristic, when the whole point
4555+
// of a duplicate is a WRITABLE base. The copy is scope-less.
4556+
const dupManifest: Record<string, unknown> = {
45354557
...srcPkg.manifest,
45364558
id: request.targetPackageId,
45374559
name: request.targetName ?? `${srcPkg.manifest.name ?? request.sourcePackageId} (copy)`,
45384560
namespace: targetNs,
4539-
});
4561+
};
4562+
delete dupManifest.scope;
4563+
await this.installPackage({ manifest: dupManifest } as InstallPackageRequest);
45404564
} catch {
45414565
/* best-effort — the per-item package binding still works without a manifest row */
45424566
}
@@ -5622,25 +5646,49 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
56225646
* registered in-memory and visible for the lifetime of the process.
56235647
*/
56245648
async installPackage(request: InstallPackageRequest): Promise<InstallPackageResponse> {
5625-
const manifest = request.manifest;
5649+
// #2532 — runtime-created base packages routinely arrive versionless
5650+
// ({id, name} from the builder / Setup). `sys_packages.version` is NOT
5651+
// NULL, and the old guard here (`pkgSvc?.publish && manifest.version`)
5652+
// silently SKIPPED persistence for exactly those packages — so they
5653+
// lived only in the in-memory registry and vanished on restart, while
5654+
// their metadata (objects, tables) survived. Default the version
5655+
// instead of skipping: the registry and the durable row must agree.
5656+
const manifest: any = { ...(request.manifest as any) };
5657+
if (typeof manifest.version !== 'string' || !manifest.version) {
5658+
manifest.version = '0.1.0';
5659+
}
56265660
const pkg = this.engine.registry.installPackage(manifest as any, request.settings);
56275661

5628-
// Best-effort durable persistence to `sys_packages`.
5662+
// Best-effort durable persistence to `sys_packages` (non-fatal by
5663+
// design — without the `package` service the install stays visible
5664+
// for the process lifetime) — but never SILENT: a skipped persist is
5665+
// a restart-loss, so it must at least leave a trace.
56295666
try {
56305667
const services = this.getServicesRegistry?.();
56315668
const pkgSvc = services?.get('package') as
5632-
| { publish?: (data: { manifest: unknown; metadata: unknown }) => Promise<unknown> }
5669+
| { publish?: (data: { manifest: unknown; metadata: unknown }) => Promise<{ success?: boolean; error?: string } | unknown> }
56335670
| undefined;
5634-
if (pkgSvc?.publish && (manifest as any)?.version) {
5635-
await pkgSvc.publish({ manifest, metadata: {} });
5671+
if (pkgSvc?.publish) {
5672+
const out = (await pkgSvc.publish({ manifest, metadata: {} })) as
5673+
| { success?: boolean; error?: string }
5674+
| undefined;
5675+
if (out && out.success === false) {
5676+
console.warn(
5677+
`[protocol.installPackage] sys_packages persist FAILED for '${manifest?.id}': ${out.error ?? 'unknown error'} — package will not survive a restart`,
5678+
);
5679+
}
5680+
} else {
5681+
console.warn(
5682+
`[protocol.installPackage] no 'package' service — '${manifest?.id}' registered in-memory only (will not survive a restart)`,
5683+
);
56365684
}
56375685
} catch (e) {
56385686
// Non-fatal: registry write already succeeded; log and continue.
56395687
console.warn(
5640-
`[protocol.installPackage] sys_packages persist skipped for '${(manifest as any)?.id}': ${(e as Error)?.message}`,
5688+
`[protocol.installPackage] sys_packages persist skipped for '${manifest?.id}': ${(e as Error)?.message}`,
56415689
);
56425690
}
56435691

5644-
return { package: pkg as any, message: `Installed package: ${(manifest as any)?.id}` };
5692+
return { package: pkg as any, message: `Installed package: ${manifest?.id}` };
56455693
}
56465694
}

packages/objectql/src/protocol-install-package.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,20 @@ describe('protocol.installPackage (ADR-0033 consolidation)', () => {
8181
warn.mockRestore();
8282
});
8383

84-
it('skips persistence when the manifest has no version (cannot key sys_packages)', async () => {
84+
it('persists a versionless manifest with a defaulted version (#2540: base packages must survive restart)', async () => {
85+
// Builder/Setup create base packages as bare { id, name } with no version.
86+
// Pre-#2540 these were skipped for persistence and vanished on restart (#2532).
87+
// Now installPackage defaults version to '0.1.0' so they key into sys_packages.
8588
const publish = vi.fn(async () => ({ success: true }));
8689
const { protocol, registry } = makeProtocol({ pkgSvc: { publish } });
8790
const manifest = { id: 'app.nov', name: 'NoVer', type: 'application' };
8891

8992
await protocol.installPackage({ manifest } as never);
9093

9194
expect(registry.installPackage).toHaveBeenCalledOnce();
92-
expect(publish).not.toHaveBeenCalled();
95+
expect(publish).toHaveBeenCalledTimes(1);
96+
expect((publish.mock.calls[0] as unknown[])[0]).toMatchObject({
97+
manifest: { id: 'app.nov', version: '0.1.0' },
98+
});
9399
});
94100
});

packages/objectql/src/protocol-package-lifecycle.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,16 @@ describe('protocol.duplicatePackage (ADR-0070 D4)', () => {
145145
sourcePackageId: 'app.iojn', targetPackageId: 'app.iojn2', targetNamespace: 'iojn2',
146146
});
147147

148-
// target package installed as a writable copy (new id + namespace, scope kept)
149-
expect(installPackage).toHaveBeenCalledWith(
150-
expect.objectContaining({ id: 'app.iojn2', namespace: 'iojn2', scope: 'environment' }),
151-
);
148+
// #2540: the copy routes through installPackage (not a bare registry
149+
// write) so it ALSO lands in sys_packages and survives a restart. And
150+
// the copy is SCOPE-LESS on purpose — the source carries scope
151+
// 'environment', but branding that onto the duplicate would make it
152+
// read-only in the writability heuristics; a duplicate must be a
153+
// writable base.
154+
expect(installPackage).toHaveBeenCalledTimes(1);
155+
const dupManifest = (installPackage as any).mock.calls[0][0];
156+
expect(dupManifest).toMatchObject({ id: 'app.iojn2', namespace: 'iojn2' });
157+
expect(dupManifest.scope).toBeUndefined();
152158

153159
const calls = (saveMetaItem as any).mock.calls.map((c: any) => c[0]);
154160
const ticket = calls.find((c: any) => c.name === 'iojn2_repair_ticket');

packages/runtime/src/dispatcher-plugin.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,72 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu
694694
}
695695
});
696696

697+
// The dispatcher's handlePackages grew branches that were never
698+
// mounted here, so they 404'd at the HTTP layer on every serve
699+
// stack (hono notFound) despite working handler code: duplicate
700+
// (ADR-0070 D4), adopt-orphans (D5), discard-drafts, and the
701+
// ADR-0067 commit-history / rollback family. Mount them all —
702+
// the handlers already exist; this is routing only.
703+
server.post(`${prefix}/packages/:id/duplicate`, async (req: any, res: any) => {
704+
try {
705+
const result = await dispatcher.handlePackages(`/${req.params.id}/duplicate`, 'POST', req.body, {}, { request: req });
706+
sendResult(result, res);
707+
} catch (err: any) {
708+
errorResponse(err, res);
709+
}
710+
});
711+
712+
server.post(`${prefix}/packages/:id/adopt-orphans`, async (req: any, res: any) => {
713+
try {
714+
const result = await dispatcher.handlePackages(`/${req.params.id}/adopt-orphans`, 'POST', req.body, {}, { request: req });
715+
sendResult(result, res);
716+
} catch (err: any) {
717+
errorResponse(err, res);
718+
}
719+
});
720+
721+
server.post(`${prefix}/packages/:id/discard-drafts`, async (req: any, res: any) => {
722+
try {
723+
const result = await dispatcher.handlePackages(`/${req.params.id}/discard-drafts`, 'POST', req.body, {}, { request: req });
724+
sendResult(result, res);
725+
} catch (err: any) {
726+
errorResponse(err, res);
727+
}
728+
});
729+
730+
server.get(`${prefix}/packages/:id/commits`, async (req: any, res: any) => {
731+
try {
732+
const result = await dispatcher.handlePackages(`/${req.params.id}/commits`, 'GET', undefined, req.query ?? {}, { request: req });
733+
sendResult(result, res);
734+
} catch (err: any) {
735+
errorResponse(err, res);
736+
}
737+
});
738+
739+
server.post(`${prefix}/packages/:id/commits/:commitId/revert`, async (req: any, res: any) => {
740+
try {
741+
const result = await dispatcher.handlePackages(
742+
`/${req.params.id}/commits/${req.params.commitId}/revert`,
743+
'POST',
744+
req.body,
745+
{},
746+
{ request: req },
747+
);
748+
sendResult(result, res);
749+
} catch (err: any) {
750+
errorResponse(err, res);
751+
}
752+
});
753+
754+
server.post(`${prefix}/packages/:id/rollback`, async (req: any, res: any) => {
755+
try {
756+
const result = await dispatcher.handlePackages(`/${req.params.id}/rollback`, 'POST', req.body, {}, { request: req });
757+
sendResult(result, res);
758+
} catch (err: any) {
759+
errorResponse(err, res);
760+
}
761+
});
762+
697763
// ── Storage ─────────────────────────────────────────────────
698764
server.post(`${prefix}/storage/upload`, async (req: any, res: any) => {
699765
try {

0 commit comments

Comments
 (0)