From 3914fc5a31d10d190e9a108788db70e3343350fd Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Fri, 3 Jul 2026 18:55:58 +0800 Subject: [PATCH 1/4] fix(protocol): persist versionless package installs; drop sys_packages on delete (#2532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .changeset/durable-package-install.md | 14 ++++ .../src/durable-package.test.ts | 79 +++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 53 +++++++++++-- 3 files changed, 139 insertions(+), 7 deletions(-) create mode 100644 .changeset/durable-package-install.md create mode 100644 packages/metadata-protocol/src/durable-package.test.ts diff --git a/.changeset/durable-package-install.md b/.changeset/durable-package-install.md new file mode 100644 index 0000000000..45eff9ab69 --- /dev/null +++ b/.changeset/durable-package-install.md @@ -0,0 +1,14 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(protocol): versionless package installs now persist to sys_packages (#2532) + +`installPackage` writes both package stores, but its durable half was guarded by +`pkgSvc?.publish && manifest.version` — silently skipping every versionless +runtime-created base (`{id, name}` from the builder / Setup). Those packages +lived only in the in-memory registry and vanished on restart, while their +metadata and tables survived. The version is now defaulted (`0.1.0`) instead of +skipping, a failed persist logs loudly instead of silently, and `deletePackage` +drops the `sys_packages` record so an uninstalled package no longer resurrects +at the next boot (service-package hydrates that table into the registry). diff --git a/packages/metadata-protocol/src/durable-package.test.ts b/packages/metadata-protocol/src/durable-package.test.ts new file mode 100644 index 0000000000..61d7815d95 --- /dev/null +++ b/packages/metadata-protocol/src/durable-package.test.ts @@ -0,0 +1,79 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// #2532 — durable package registration. `protocol.installPackage` writes BOTH +// package stores (in-memory registry + sys_packages via the `package` service), +// but its persistence guard used to be `pkgSvc?.publish && manifest.version` — +// silently SKIPPING every versionless runtime-created base ({id, name} from the +// builder / Setup), which is exactly why those packages vanished on restart. +// These tests pin the fixed contract: version is defaulted (never skipped) and +// uninstall drops the durable row so packages don't resurrect at boot. + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './index.js'; + +function makeImpl(overrides?: { + publish?: (d: { manifest: unknown; metadata: unknown }) => Promise; + del?: (id: string) => Promise; + find?: (obj: string, q: unknown) => Promise; +}) { + const registryCalls: Array<{ manifest: any; settings: any }> = []; + const engine = { + registry: { + installPackage: (manifest: any, settings: any) => { + registryCalls.push({ manifest, settings }); + return { manifest, status: 'installed', enabled: true }; + }, + }, + find: overrides?.find ?? (async () => []), + }; + const publish = vi.fn(overrides?.publish ?? (async () => ({ success: true }))); + const del = vi.fn(overrides?.del ?? (async () => ({ success: true }))); + const services = new Map([['package', { publish, delete: del }]]); + const impl = new ObjectStackProtocolImplementation(engine as any, () => services); + return { impl, registryCalls, publish, del }; +} + +describe('installPackage — durable persistence (#2532)', () => { + it('persists a VERSIONLESS manifest by defaulting version (the vanish-on-restart bug)', async () => { + const { impl, registryCalls, publish } = makeImpl(); + const res: any = await (impl as any).installPackage({ + manifest: { id: 'com.example.orders', name: '订单中心' }, + }); + + // In-memory half. + expect(registryCalls).toHaveLength(1); + expect(registryCalls[0].manifest.version).toBe('0.1.0'); + // Durable half — the old guard skipped publish entirely for this shape. + expect(publish).toHaveBeenCalledTimes(1); + const persisted = publish.mock.calls[0][0] as any; + expect(persisted.manifest.id).toBe('com.example.orders'); + expect(persisted.manifest.version).toBe('0.1.0'); + expect(res.package.status).toBe('installed'); + }); + + it('keeps an explicit version untouched', async () => { + const { impl, publish } = makeImpl(); + await (impl as any).installPackage({ manifest: { id: 'com.example.v', version: '2.3.4' } }); + expect((publish.mock.calls[0][0] as any).manifest.version).toBe('2.3.4'); + }); + + it('stays non-fatal when the durable write fails (registry install already succeeded)', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const { impl } = makeImpl({ publish: async () => ({ success: false, error: 'boom' }) }); + const res: any = await (impl as any).installPackage({ manifest: { id: 'com.example.fail' } }); + expect(res.package.status).toBe('installed'); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('persist FAILED')); + } finally { + warn.mockRestore(); + } + }); +}); + +describe('deletePackage — durable un-registration (#2532 counterpart)', () => { + it('drops the sys_packages record so the package cannot resurrect at boot', async () => { + const { impl, del } = makeImpl(); + await (impl as any).deletePackage({ packageId: 'com.example.orders' }); + expect(del).toHaveBeenCalledWith('com.example.orders'); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 32105335ca..544e626817 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4455,6 +4455,21 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { } } + // #2532 counterpart: also drop the durable `sys_packages` record — + // service-package hydrates that table back into the registry at boot, + // so leaving the row behind would RESURRECT an uninstalled package on + // the next restart. Best-effort, same posture as install persistence. + try { + const pkgSvc = this.getServicesRegistry?.()?.get('package') as + | { delete?: (id: string) => Promise } + | undefined; + if (pkgSvc?.delete) await pkgSvc.delete(request.packageId); + } catch (e) { + console.warn( + `[protocol.deletePackage] sys_packages cleanup skipped for '${request.packageId}': ${(e as Error)?.message}`, + ); + } + return { success: failed.length === 0 && deleted.length > 0, deletedCount: deleted.length, @@ -5622,25 +5637,49 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { * registered in-memory and visible for the lifetime of the process. */ async installPackage(request: InstallPackageRequest): Promise { - const manifest = request.manifest; + // #2532 — runtime-created base packages routinely arrive versionless + // ({id, name} from the builder / Setup). `sys_packages.version` is NOT + // NULL, and the old guard here (`pkgSvc?.publish && manifest.version`) + // silently SKIPPED persistence for exactly those packages — so they + // lived only in the in-memory registry and vanished on restart, while + // their metadata (objects, tables) survived. Default the version + // instead of skipping: the registry and the durable row must agree. + const manifest: any = { ...(request.manifest as any) }; + if (typeof manifest.version !== 'string' || !manifest.version) { + manifest.version = '0.1.0'; + } const pkg = this.engine.registry.installPackage(manifest as any, request.settings); - // Best-effort durable persistence to `sys_packages`. + // Best-effort durable persistence to `sys_packages` (non-fatal by + // design — without the `package` service the install stays visible + // for the process lifetime) — but never SILENT: a skipped persist is + // a restart-loss, so it must at least leave a trace. try { const services = this.getServicesRegistry?.(); const pkgSvc = services?.get('package') as - | { publish?: (data: { manifest: unknown; metadata: unknown }) => Promise } + | { publish?: (data: { manifest: unknown; metadata: unknown }) => Promise<{ success?: boolean; error?: string } | unknown> } | undefined; - if (pkgSvc?.publish && (manifest as any)?.version) { - await pkgSvc.publish({ manifest, metadata: {} }); + if (pkgSvc?.publish) { + const out = (await pkgSvc.publish({ manifest, metadata: {} })) as + | { success?: boolean; error?: string } + | undefined; + if (out && out.success === false) { + console.warn( + `[protocol.installPackage] sys_packages persist FAILED for '${manifest?.id}': ${out.error ?? 'unknown error'} — package will not survive a restart`, + ); + } + } else { + console.warn( + `[protocol.installPackage] no 'package' service — '${manifest?.id}' registered in-memory only (will not survive a restart)`, + ); } } catch (e) { // Non-fatal: registry write already succeeded; log and continue. console.warn( - `[protocol.installPackage] sys_packages persist skipped for '${(manifest as any)?.id}': ${(e as Error)?.message}`, + `[protocol.installPackage] sys_packages persist skipped for '${manifest?.id}': ${(e as Error)?.message}`, ); } - return { package: pkg as any, message: `Installed package: ${(manifest as any)?.id}` }; + return { package: pkg as any, message: `Installed package: ${manifest?.id}` }; } } From 69e67cc4a12a188ecaad64fc9f74de2ce2c25a08 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Fri, 3 Jul 2026 19:01:15 +0800 Subject: [PATCH 2/4] fix(protocol): duplicatePackage routes through installPackage so copies persist too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/metadata-protocol/src/protocol.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 544e626817..113a750d6d 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4546,12 +4546,17 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { if (srcPkg?.manifest && typeof registry?.installPackage === 'function') { try { - registry.installPackage({ - ...srcPkg.manifest, - id: request.targetPackageId, - name: request.targetName ?? `${srcPkg.manifest.name ?? request.sourcePackageId} (copy)`, - namespace: targetNs, - }); + // Route through installPackage (not a bare registry write) so the + // duplicated base ALSO lands in sys_packages — otherwise the copy + // would vanish from GET /packages on the next restart (#2532). + await this.installPackage({ + manifest: { + ...srcPkg.manifest, + id: request.targetPackageId, + name: request.targetName ?? `${srcPkg.manifest.name ?? request.sourcePackageId} (copy)`, + namespace: targetNs, + }, + } as InstallPackageRequest); } catch { /* best-effort — the per-item package binding still works without a manifest row */ } From 4cdb925f0f206a4842c87b63730d8f1ba845ab3b Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Fri, 3 Jul 2026 23:18:00 +0800 Subject: [PATCH 3/4] fix(runtime,protocol): mount missing package sub-routes; duplicates are scope-less MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/metadata-protocol/src/protocol.ts | 20 ++++--- packages/runtime/src/dispatcher-plugin.ts | 66 ++++++++++++++++++++++ 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 113a750d6d..d75bb96ff1 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4549,14 +4549,18 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { // Route through installPackage (not a bare registry write) so the // duplicated base ALSO lands in sys_packages — otherwise the copy // would vanish from GET /packages on the next restart (#2532). - await this.installPackage({ - manifest: { - ...srcPkg.manifest, - id: request.targetPackageId, - name: request.targetName ?? `${srcPkg.manifest.name ?? request.sourcePackageId} (copy)`, - namespace: targetNs, - }, - } as InstallPackageRequest); + // Spread-then-strip: the source may carry `scope` (e.g. 'project' + // on a code package) — copying it would brand the duplicate as + // read-only in every writability heuristic, when the whole point + // of a duplicate is a WRITABLE base. The copy is scope-less. + const dupManifest: Record = { + ...srcPkg.manifest, + id: request.targetPackageId, + name: request.targetName ?? `${srcPkg.manifest.name ?? request.sourcePackageId} (copy)`, + namespace: targetNs, + }; + delete dupManifest.scope; + await this.installPackage({ manifest: dupManifest } as InstallPackageRequest); } catch { /* best-effort — the per-item package binding still works without a manifest row */ } diff --git a/packages/runtime/src/dispatcher-plugin.ts b/packages/runtime/src/dispatcher-plugin.ts index 8cb135e048..50214ff4fe 100644 --- a/packages/runtime/src/dispatcher-plugin.ts +++ b/packages/runtime/src/dispatcher-plugin.ts @@ -694,6 +694,72 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu } }); + // The dispatcher's handlePackages grew branches that were never + // mounted here, so they 404'd at the HTTP layer on every serve + // stack (hono notFound) despite working handler code: duplicate + // (ADR-0070 D4), adopt-orphans (D5), discard-drafts, and the + // ADR-0067 commit-history / rollback family. Mount them all — + // the handlers already exist; this is routing only. + server.post(`${prefix}/packages/:id/duplicate`, async (req: any, res: any) => { + try { + const result = await dispatcher.handlePackages(`/${req.params.id}/duplicate`, 'POST', req.body, {}, { request: req }); + sendResult(result, res); + } catch (err: any) { + errorResponse(err, res); + } + }); + + server.post(`${prefix}/packages/:id/adopt-orphans`, async (req: any, res: any) => { + try { + const result = await dispatcher.handlePackages(`/${req.params.id}/adopt-orphans`, 'POST', req.body, {}, { request: req }); + sendResult(result, res); + } catch (err: any) { + errorResponse(err, res); + } + }); + + server.post(`${prefix}/packages/:id/discard-drafts`, async (req: any, res: any) => { + try { + const result = await dispatcher.handlePackages(`/${req.params.id}/discard-drafts`, 'POST', req.body, {}, { request: req }); + sendResult(result, res); + } catch (err: any) { + errorResponse(err, res); + } + }); + + server.get(`${prefix}/packages/:id/commits`, async (req: any, res: any) => { + try { + const result = await dispatcher.handlePackages(`/${req.params.id}/commits`, 'GET', undefined, req.query ?? {}, { request: req }); + sendResult(result, res); + } catch (err: any) { + errorResponse(err, res); + } + }); + + server.post(`${prefix}/packages/:id/commits/:commitId/revert`, async (req: any, res: any) => { + try { + const result = await dispatcher.handlePackages( + `/${req.params.id}/commits/${req.params.commitId}/revert`, + 'POST', + req.body, + {}, + { request: req }, + ); + sendResult(result, res); + } catch (err: any) { + errorResponse(err, res); + } + }); + + server.post(`${prefix}/packages/:id/rollback`, async (req: any, res: any) => { + try { + const result = await dispatcher.handlePackages(`/${req.params.id}/rollback`, 'POST', req.body, {}, { request: req }); + sendResult(result, res); + } catch (err: any) { + errorResponse(err, res); + } + }); + // ── Storage ───────────────────────────────────────────────── server.post(`${prefix}/storage/upload`, async (req: any, res: any) => { try { From 4c561be4e89c09eff83812efebf437d9a500164f Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Sat, 4 Jul 2026 02:51:30 +0800 Subject: [PATCH 4/4] 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 --- .../objectql/src/protocol-install-package.test.ts | 10 ++++++++-- .../src/protocol-package-lifecycle.test.ts | 14 ++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/objectql/src/protocol-install-package.test.ts b/packages/objectql/src/protocol-install-package.test.ts index fccf4280f4..75041caea4 100644 --- a/packages/objectql/src/protocol-install-package.test.ts +++ b/packages/objectql/src/protocol-install-package.test.ts @@ -81,7 +81,10 @@ describe('protocol.installPackage (ADR-0033 consolidation)', () => { warn.mockRestore(); }); - it('skips persistence when the manifest has no version (cannot key sys_packages)', async () => { + it('persists a versionless manifest with a defaulted version (#2540: base packages must survive restart)', async () => { + // Builder/Setup create base packages as bare { id, name } with no version. + // Pre-#2540 these were skipped for persistence and vanished on restart (#2532). + // Now installPackage defaults version to '0.1.0' so they key into sys_packages. const publish = vi.fn(async () => ({ success: true })); const { protocol, registry } = makeProtocol({ pkgSvc: { publish } }); const manifest = { id: 'app.nov', name: 'NoVer', type: 'application' }; @@ -89,6 +92,9 @@ describe('protocol.installPackage (ADR-0033 consolidation)', () => { await protocol.installPackage({ manifest } as never); expect(registry.installPackage).toHaveBeenCalledOnce(); - expect(publish).not.toHaveBeenCalled(); + expect(publish).toHaveBeenCalledTimes(1); + expect((publish.mock.calls[0] as unknown[])[0]).toMatchObject({ + manifest: { id: 'app.nov', version: '0.1.0' }, + }); }); }); diff --git a/packages/objectql/src/protocol-package-lifecycle.test.ts b/packages/objectql/src/protocol-package-lifecycle.test.ts index 20eddfff53..7875128bcb 100644 --- a/packages/objectql/src/protocol-package-lifecycle.test.ts +++ b/packages/objectql/src/protocol-package-lifecycle.test.ts @@ -145,10 +145,16 @@ describe('protocol.duplicatePackage (ADR-0070 D4)', () => { sourcePackageId: 'app.iojn', targetPackageId: 'app.iojn2', targetNamespace: 'iojn2', }); - // target package installed as a writable copy (new id + namespace, scope kept) - expect(installPackage).toHaveBeenCalledWith( - expect.objectContaining({ id: 'app.iojn2', namespace: 'iojn2', scope: 'environment' }), - ); + // #2540: the copy routes through installPackage (not a bare registry + // write) so it ALSO lands in sys_packages and survives a restart. And + // the copy is SCOPE-LESS on purpose — the source carries scope + // 'environment', but branding that onto the duplicate would make it + // read-only in the writability heuristics; a duplicate must be a + // writable base. + expect(installPackage).toHaveBeenCalledTimes(1); + const dupManifest = (installPackage as any).mock.calls[0][0]; + expect(dupManifest).toMatchObject({ id: 'app.iojn2', namespace: 'iojn2' }); + expect(dupManifest.scope).toBeUndefined(); const calls = (saveMetaItem as any).mock.calls.map((c: any) => c[0]); const ticket = calls.find((c: any) => c.name === 'iojn2_repair_ticket');