From 7a83ce21f2a5e87d221bc893cff51f9d709c40f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 23:25:36 +0000 Subject: [PATCH 1/3] test(runtime,objectql): pin PATCH /packages/:id dependencies edit (red before the fix) Claude-Session: https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6 Co-authored-by: Claude --- packages/objectql/src/registry.test.ts | 40 +++ .../packages-patch-dependencies.test.ts | 227 +++++++++++++++ ...age-dependencies-patch.integration.test.ts | 263 ++++++++++++++++++ 3 files changed, 530 insertions(+) create mode 100644 packages/runtime/src/domains/packages-patch-dependencies.test.ts create mode 100644 packages/runtime/src/package-dependencies-patch.integration.test.ts diff --git a/packages/objectql/src/registry.test.ts b/packages/objectql/src/registry.test.ts index 90a2b41ab9a..9854cf827a2 100644 --- a/packages/objectql/src/registry.test.ts +++ b/packages/objectql/src/registry.test.ts @@ -578,6 +578,46 @@ describe('SchemaRegistry', () => { it('updatePackageManifest returns undefined for an unknown package', () => { expect(registry.updatePackageManifest('nope', { name: 'x' })).toBeUndefined(); }); + + // [#24670] `dependencies` is the declaration the save door's closure + // verdict reads, and its refusal prescribes editing it — so the edit + // has to reach the manifest the next save reads. + it('[#24670] updatePackageManifest replaces `dependencies` WHOLE — the row as given, never a deep merge', () => { + registry.installPackage({ + id: 'com.test', + name: 'Keep', + description: 'kept', + version: '1.0.0', + dependencies: { 'com.acme.core': '^1.0.0', 'com.acme.old': '^1.0.0' }, + } as any, 'authored'); + registry.disablePackage('com.test'); + const row = { 'com.acme.core': '^2.0.0', 'com.acme.new': '^1.0.0' }; + + const updated = registry.updatePackageManifest('com.test', { dependencies: row }); + + // `com.acme.old` is gone: an entry the row leaves out is removed, not merged back. + expect((updated?.manifest as any).dependencies).toEqual(row); + // The registry keeps its own copy, not the caller's object. + row['com.acme.core'] = 'mutated by the caller'; + expect((registry.getPackage('com.test')?.manifest as any).dependencies['com.acme.core']).toBe('^2.0.0'); + // The keys the patch did not name, and the lifecycle, are untouched. + expect(updated?.manifest.name).toBe('Keep'); + expect((updated?.manifest as any).description).toBe('kept'); + expect(updated?.manifest.version).toBe('1.0.0'); + expect(updated?.enabled).toBe(false); + }); + + it('[#24670] an empty `dependencies` row clears the declaration; a patch without the key leaves it', () => { + registry.installPackage({ + id: 'com.test', name: 'Keep', version: '1.0.0', dependencies: { 'com.acme.core': '^1.0.0' }, + } as any, 'authored'); + + registry.updatePackageManifest('com.test', { description: 'no dependencies key' }); + expect((registry.getPackage('com.test')?.manifest as any).dependencies).toEqual({ 'com.acme.core': '^1.0.0' }); + + registry.updatePackageManifest('com.test', { dependencies: {} }); + expect((registry.getPackage('com.test')?.manifest as any).dependencies).toEqual({}); + }); }); // ========================================== diff --git a/packages/runtime/src/domains/packages-patch-dependencies.test.ts b/packages/runtime/src/domains/packages-patch-dependencies.test.ts new file mode 100644 index 00000000000..a886d10a4a0 --- /dev/null +++ b/packages/runtime/src/domains/packages-patch-dependencies.test.ts @@ -0,0 +1,227 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24670] `PATCH /api/v1/packages/:id` accepts the manifest's `dependencies` + * row, judged by `ManifestSchema`'s own `dependencies` row from + * `@objectstack/spec` and never restated here. + * + * ## The defect + * + * The door built its patch from `name`, `description` and `version` only, so a + * package's `dependencies` could be set at create and never again. The save + * door's closure verdict reads exactly that declaration, and its refusal tells + * the author to "declare a dependency … in the `dependencies` of P's + * manifest" — a remedy no door applied to a package that already exists. + * + * ## What this file pins (the in-memory registry and the door's own judgment) + * + * - the row is persisted: flat or under the `manifest` wrapper, the registry's + * manifest carries it and `GET /packages/:id` serves it; the row replaces + * the stored one whole; + * - a malformed row is refused `400 VALIDATION_ERROR` — the envelope + * `POST /packages` answers for the same row at create — and NOTHING is + * written: not the row, not a valid `name` sent beside it, and on the + * protocol branch the protocol is never called; + * - `name`, `description` and `version` behave as before, and a patch that + * does not send `dependencies` leaves the stored row alone. + * + * The protocol branch's durable half (`sys_packages`, a restart) and the + * remedy end to end are pinned over the shipped pieces in + * `../package-dependencies-patch.integration.test.ts`. + * + * ## Harness + * + * The real `HttpDispatcher` over a real `SchemaRegistry`. `@objectstack/objectql` + * resolves through `exports` to its `dist/`: rebuild it before reading a + * result here after touching `registry.ts`. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { SchemaRegistry } from '@objectstack/objectql'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +const PKG_ADMIN = () => ({ + request: {}, + executionContext: { + userId: 'u_pkg_admin', + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], + }, +}) as any; + +let home: string; +let priorHome: string | undefined; + +beforeAll(() => { + priorHome = process.env.OS_HOME; + home = mkdtempSync(join(tmpdir(), 'os-24670-')); + process.env.OS_HOME = home; +}); + +afterAll(() => { + if (priorHome === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = priorHome; + rmSync(home, { recursive: true, force: true }); +}); + +const ID = 'com.acme.crm'; +const DECLARED = { 'com.acme.core': '^1.0.0' }; + +/** A complete authoring-stage manifest, declaring one dependency. */ +const manifest = () => ({ + id: ID, + name: 'Acme CRM', + namespace: 'crm', + version: '1.0.0', + type: 'app', + description: 'before', + dependencies: { ...DECLARED }, +}); + +/** + * One process: a registry holding the package, and the door. With + * `protocol`, the door's protocol branch reaches that double; without it, the + * door takes its in-memory fallback. + */ +function boot(protocol?: { updatePackage: (...args: any[]) => any }) { + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + (registry as any).logLevel = 'silent'; + registry.installPackage(manifest() as any, 'authored'); + const kernel: any = { + getService: (name: string) => { + if (name === 'objectql') return Promise.resolve({ registry }); + if (name === 'protocol' && protocol) return Promise.resolve(protocol); + return null; + }, + context: { getService: () => null }, + }; + const dispatcher = new HttpDispatcher(kernel); + const call = async (method: string, path: string, body?: unknown) => { + const res = await dispatcher.handlePackages(path, method, body, {}, PKG_ADMIN()); + const json = JSON.parse(JSON.stringify(res.response?.body ?? null)); + return { status: res.response?.status ?? 0, code: json?.error?.code as string | undefined, body: json }; + }; + /** The manifest the registry holds, as a plain value. */ + const stored = () => JSON.parse(JSON.stringify(registry.getPackage(ID)?.manifest ?? null)); + return { registry, call, stored }; +} + +/** Rows `ManifestSchema`'s `dependencies` row refuses: a non-string version, a list, a scalar, a null. */ +const MALFORMED: ReadonlyArray<[string, unknown]> = [ + ['a non-string version requirement', { 'com.acme.core': 1 }], + ['a list of ids', ['com.acme.core']], + ['a bare string', 'com.acme.core'], + ['null', null], +]; + +describe('[#24670] PATCH /packages/:id — `dependencies` is persisted (in-memory fallback)', () => { + for (const [form, wrap] of [ + ['flat', (patch: object) => patch], + ['under the `manifest` wrapper', (patch: object) => ({ manifest: patch })], + ] as const) { + it(`${form}: the row reaches the registry's manifest and GET /packages/:id serves it`, async () => { + const p = boot(); + const row = { 'com.acme.core': '^2.0.0', 'com.acme.billing': '^1.0.0' }; + + const patched = await p.call('PATCH', `/${ID}`, wrap({ dependencies: row })); + + expect(patched.status, JSON.stringify(patched.body)).toBe(200); + expect(p.stored().dependencies).toEqual(row); + const read = await p.call('GET', `/${ID}`); + expect(read.status).toBe(200); + expect(read.body?.data?.manifest?.dependencies).toEqual(row); + }); + } + + it('the row replaces the stored one whole: an entry it leaves out is removed, and `{}` clears it', async () => { + const p = boot(); + + await p.call('PATCH', `/${ID}`, { dependencies: { 'com.acme.billing': '^1.0.0' } }); + expect(p.stored().dependencies).toEqual({ 'com.acme.billing': '^1.0.0' }); + + const cleared = await p.call('PATCH', `/${ID}`, { dependencies: {} }); + expect(cleared.status).toBe(200); + expect(p.stored().dependencies).toEqual({}); + }); +}); + +describe('[#24670] PATCH /packages/:id — a malformed `dependencies` row is refused, and nothing is written', () => { + for (const [what, row] of MALFORMED) { + it(`${what}: 400 VALIDATION_ERROR naming \`dependencies\`, the stored manifest unchanged`, async () => { + const p = boot(); + const before = p.stored(); + + const answer = await p.call('PATCH', `/${ID}`, { dependencies: row }); + + expect({ status: answer.status, code: answer.code }).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + expect(answer.body?.error?.message).toContain('dependencies'); + expect(p.stored()).toEqual(before); + }); + } + + it('a valid `name` sent beside a malformed row is not written either', async () => { + const p = boot(); + const before = p.stored(); + + const answer = await p.call('PATCH', `/${ID}`, { manifest: { name: 'Renamed', dependencies: { x: 2 } } }); + + expect({ status: answer.status, code: answer.code }).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + expect(p.stored()).toEqual(before); + }); + + it('the protocol branch: refused before the protocol is called', async () => { + const updatePackage = vi.fn(); + const p = boot({ updatePackage }); + + const answer = await p.call('PATCH', `/${ID}`, { dependencies: { 'com.acme.core': 1 } }); + + expect({ status: answer.status, code: answer.code }).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + expect(updatePackage).not.toHaveBeenCalled(); + }); + + it('the same envelope `POST /packages` answers for the same row at create', async () => { + const p = boot(); + const row = { 'com.acme.core': 1 }; + + const atCreate = await p.call('POST', '', { manifest: { ...manifest(), id: 'com.acme.other', namespace: 'other', dependencies: row } }); + const atEdit = await p.call('PATCH', `/${ID}`, { dependencies: row }); + + expect({ status: atCreate.status, code: atCreate.code }).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + expect({ status: atEdit.status, code: atEdit.code }).toEqual({ status: atCreate.status, code: atCreate.code }); + }); +}); + +describe('[#24670] PATCH /packages/:id — the three existing keys behave as before', () => { + it('a name / description / version patch persists exactly those and leaves `dependencies` as stored', async () => { + const p = boot(); + + const answer = await p.call('PATCH', `/${ID}`, { name: ' Renamed ', description: 'after', version: '2.0.0' }); + + expect(answer.status).toBe(200); + expect(p.stored()).toMatchObject({ name: 'Renamed', description: 'after', version: '2.0.0', dependencies: DECLARED }); + }); + + it('an empty name and a non-SemVer version are still refused 400, and an empty body names `dependencies` among the keys', async () => { + const p = boot(); + const before = p.stored(); + + expect((await p.call('PATCH', `/${ID}`, { name: ' ' })).status).toBe(400); + expect((await p.call('PATCH', `/${ID}`, { version: '1.2' })).status).toBe(400); + const empty = await p.call('PATCH', `/${ID}`, {}); + expect({ status: empty.status, code: empty.code }).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + expect(empty.body?.error?.message).toContain('dependencies'); + expect(p.stored()).toEqual(before); + }); + + it('the protocol branch receives the parsed row beside the other keys', async () => { + const updatePackage = vi.fn().mockResolvedValue({ package: { id: ID } }); + const p = boot({ updatePackage }); + + const answer = await p.call('PATCH', `/${ID}`, { manifest: { description: 'after', dependencies: DECLARED } }); + + expect(answer.status).toBe(200); + expect(updatePackage).toHaveBeenCalledWith({ packageId: ID, patch: { description: 'after', dependencies: DECLARED } }); + }); +}); diff --git a/packages/runtime/src/package-dependencies-patch.integration.test.ts b/packages/runtime/src/package-dependencies-patch.integration.test.ts new file mode 100644 index 00000000000..70c0fa8ff05 --- /dev/null +++ b/packages/runtime/src/package-dependencies-patch.integration.test.ts @@ -0,0 +1,263 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24670] `PATCH /api/v1/packages/:id` applies the manifest's `dependencies` + * row on the protocol branch, durably, and that edit is the remedy the save + * door's closure refusal prescribes. + * + * ## The defect + * + * The door built its patch from `name`, `description` and `version` only, and + * `SchemaRegistry.updatePackageManifest` copied only those three, so + * `manifest.dependencies` could be set at create and never again. The save + * door judges a write into a package against that package's closure — its + * declared `dependencies` among it (ruling #9612) — and refuses a binding + * outside it with the remedy "declare a dependency on the package that + * defines X in the `dependencies` of P's manifest". For a package that already + * existed, no door applied that remedy. + * + * ## The pins + * + * (a) the row is persisted: `GET /packages/:id` serves it, the `sys_packages` + * row stores it, and after a restart (the stored rows hydrated into a new + * registry) the door serves it again; + * (b) a malformed row is refused `400 VALIDATION_ERROR` and neither the + * registry nor the `sys_packages` row moves; + * (d) end to end: a list view saved into P bound to an object of a package P + * never declared is refused `422 INVALID_METADATA` + * (`list-view-object-outside-closure`), and after the PATCH declares that + * package the same save is admitted. + * + * (c), the three existing keys, is pinned at the door in + * `domains/packages-patch-dependencies.test.ts`. + * + * ## The composition — the shipped pieces, booted twice over one database file + * + * The `package-uninstall-store-refusal.integration.test.ts` harness: a REAL + * `ObjectQL` over a REAL `SqlDriver` (better-sqlite3, on disk); the REAL + * `PackageServicePlugin`, whose `start()` creates `sys_packages`, registers the + * `package` service and hydrates the stored rows into the registry; the REAL + * `ObjectStackProtocolImplementation`; and the REAL `HttpDispatcher`. One + * difference, on purpose: the protocol is built on the DEFAULT authoring + * channel (`'environment'`), the one whose saves the runtime authoring gate + * judges — the `'package-author'` channel that harness uses skips the gate, + * and with it the closure verdict (d) is about. + * + * ⚠️ `@objectstack/objectql` and `@objectstack/metadata-protocol` resolve + * through `exports` to their **`dist/`** (`check-test-source-alias.mjs`, + * `KNOWN_UNALIASED_TEST_IMPORTS`): rebuild them before reading a result here + * after touching `registry.ts` or `protocol.ts`. + */ + +import { describe, it, expect, beforeAll, afterAll, afterEach, beforeEach, vi } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { + SysMetadataObject, + SysMetadataHistoryObject, + SysMetadataAuditObject, + SysMetadataCommitObject, +} from '@objectstack/metadata-core'; +import { PackageServicePlugin } from '@objectstack/service-package'; +import { HttpDispatcher } from './http-dispatcher.js'; + +/** The package written into; namespace `own`. */ +const OWN = 'com.probe.own'; +/** The package that defines the bound object; namespace `depcore`. */ +const DEP = 'com.probe.depcore'; +const OBJ = 'depcore_thing'; +const VIEW = `${OBJ}.own_list`; +const PLATFORM_PKG = 'com.objectstack.platform'; +const ENV = 'platform'; +const OUTSIDE = 'list-view-object-outside-closure'; + +/** A signed-in admin acting in an organization. */ +const CALLER: any = { + request: {}, + environmentId: ENV, + executionContext: { + userId: 'u_admin', + isSystem: false, + systemPermissions: ['manage_metadata', 'studio.access'], + tenantId: 'org_acme', + }, +}; + +const quiet = { debug() {}, info() {}, warn() {}, error() {} }; + +let cleanup: Array<() => void | Promise> = []; +afterEach(async () => { + for (const c of cleanup.reverse()) await c(); + cleanup = []; +}); + +const envSnapshot = { OS_HOME: process.env.OS_HOME }; +let home: string; +beforeAll(() => { + home = mkdtempSync(join(tmpdir(), 'os-24670-home-')); + process.env.OS_HOME = home; +}); +afterAll(() => { + if (envSnapshot.OS_HOME === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = envSnapshot.OS_HOME; + rmSync(home, { recursive: true, force: true }); +}); + +let warnSpy: ReturnType; +beforeEach(() => { warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); }); +afterEach(() => { warnSpy.mockRestore(); }); + +function newDir() { + const dir = mkdtempSync(join(tmpdir(), 'os-24670-db-')); + cleanup.push(() => rmSync(dir, { recursive: true, force: true })); + return dir; +} + +/** One process over the database in `dir`. */ +async function boot(dir: string) { + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: join(dir, 'data.sqlite') }, + useNullAsDefault: true, + }); + const objects = [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject, SysMetadataCommitObject] as any[]; + await driver.initObjects(objects); + + const engine = new ObjectQL(); + engine.registerDriver(driver as any, true); + await engine.init(); + for (const o of objects) engine.registry.registerObject(o, PLATFORM_PKG); + + const services = new Map([['objectql', engine]]); + const ctx: any = { + logger: quiet, + getService: (name: string) => services.get(name), + registerService: (name: string, service: unknown) => { services.set(name, service); }, + }; + const plugin = new PackageServicePlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + + // The DEFAULT authoring channel — see the header. + const protocol: any = new ObjectStackProtocolImplementation(engine as any, () => services as any); + services.set('protocol', protocol); + const get = (name: string) => services.get(name) ?? null; + const dispatcher = new HttpDispatcher({ context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) } as any); + + let destroyed = false; + const destroy = async () => { + if (destroyed) return; + destroyed = true; + await engine.destroy(); + }; + cleanup.push(destroy); + + return { + engine, + protocol, + destroy, + async call(method: string, path: string, body?: unknown) { + const res = await dispatcher.handlePackages(path, method, body, {}, CALLER); + const json = JSON.parse(JSON.stringify(res.response?.body ?? null)); + return { status: res.response?.status ?? 0, code: json?.error?.code as string | undefined, body: json }; + }, + async install(id: string) { + await protocol.installPackage({ manifest: { id, name: id, version: '1.0.0', type: 'app' } }); + }, + /** The `sys_packages` row's manifest, read from the store — not the registry. */ + async storedManifest(id: string) { + const record = await (services.get('package') as any).get(id); + return record?.manifest; + }, + /** The registry's manifest, as a plain value. */ + registryManifest: (id: string) => JSON.parse(JSON.stringify(engine.registry.getPackage(id)?.manifest ?? null)), + /** A publish-mode save of DEP's object. */ + saveDepObject: () => protocol.saveMetaItem({ + type: 'object', + name: OBJ, + packageId: DEP, + item: { name: OBJ, label: 'Thing', sharingModel: 'private', fields: { title: { name: 'title', type: 'text', label: 'Title' } } }, + }), + /** A publish-mode save of a list view written into OWN, bound to DEP's object. */ + saveView: () => protocol.saveMetaItem({ + type: 'view', + name: VIEW, + packageId: OWN, + item: { + name: VIEW, + object: OBJ, + viewKind: 'list', + label: 'Things', + type: 'grid', + data: { provider: 'object', object: OBJ }, + columns: [{ field: 'title' }], + }, + }), + /** The active stored views of OWN, by name — the store, not the door's word for it. */ + async activeViews() { + const rows = (await engine.find('sys_metadata', { where: { type: 'view', package_id: OWN, state: 'active' } })) as any[]; + return rows.map((r) => r.name as string).sort(); + }, + }; +} + +describe('[#24670] PATCH /packages/:id — `dependencies` on the protocol branch', () => { + it('(a) the row is persisted: GET serves it, sys_packages stores it, and a restart serves it again', async () => { + const dir = newDir(); + const first = await boot(dir); + await first.install(OWN); + const row = { [DEP]: '^1.0.0' }; + + const patched = await first.call('PATCH', `/${OWN}`, { dependencies: row }); + + expect(patched.status, JSON.stringify(patched.body)).toBe(200); + expect((await first.call('GET', `/${OWN}`)).body?.data?.manifest?.dependencies).toEqual(row); + expect((await first.storedManifest(OWN))?.dependencies, 'the sys_packages row').toEqual(row); + + await first.destroy(); + const restarted = await boot(dir); + + const read = await restarted.call('GET', `/${OWN}`); + expect(read.status).toBe(200); + expect(read.body?.data?.manifest?.dependencies, 'served after a restart').toEqual(row); + }, 60_000); + + it('(b) a malformed row is refused 400 VALIDATION_ERROR, and neither the registry nor sys_packages moves', async () => { + const p = await boot(newDir()); + await p.install(OWN); + const declared = { [DEP]: '^1.0.0' }; + expect((await p.call('PATCH', `/${OWN}`, { dependencies: declared })).status, 'precondition').toBe(200); + const registryBefore = p.registryManifest(OWN); + const storedBefore = await p.storedManifest(OWN); + + const answer = await p.call('PATCH', `/${OWN}`, { dependencies: { [DEP]: 1 } }); + + expect({ status: answer.status, code: answer.code }).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + expect(p.registryManifest(OWN)).toEqual(registryBefore); + expect(await p.storedManifest(OWN)).toEqual(storedBefore); + }, 60_000); + + it('(d) the remedy end to end: a save refused for an undeclared package is admitted once the PATCH declares it', async () => { + const p = await boot(newDir()); + await p.install(OWN); + await p.install(DEP); + expect((await p.saveDepObject()).success, 'precondition: DEP defines the object').toBe(true); + + const refused = await p.saveView().then(() => null, (e: any) => e); + expect(refused, 'the save resolved — the closure verdict did not run').toBeInstanceOf(Error); + expect({ code: refused.code, status: refused.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect((refused.issues ?? []).map((i: any) => i.rule)).toEqual([OUTSIDE]); + expect(await p.activeViews(), 'a refused save stores nothing').toEqual([]); + + const patched = await p.call('PATCH', `/${OWN}`, { dependencies: { [DEP]: '^1.0.0' } }); + expect(patched.status, JSON.stringify(patched.body)).toBe(200); + + const admitted = await p.saveView(); + expect(admitted.success).toBe(true); + expect(await p.activeViews()).toEqual([VIEW]); + }, 60_000); +}); From c8849267236e93c6631b6dc3e3c6cceb40728d0a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 23:38:16 +0000 Subject: [PATCH 2/3] fix(runtime): PATCH /packages/:id applies the manifest's dependencies row The door judges the row by ManifestSchema's dependencies row and refuses a malformed one 400 before any write; SchemaRegistry.updatePackageManifest replaces the row whole and the protocol's updatePackage persists it. Claude-Session: https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6 Co-authored-by: Claude --- packages/metadata-protocol/src/protocol.ts | 14 +++-- packages/objectql/src/registry.ts | 18 +++++- packages/runtime/src/domains/packages.ts | 63 ++++++++++++++++--- ...age-dependencies-patch.integration.test.ts | 24 ++++++- 4 files changed, 101 insertions(+), 18 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 5b317be9576..12e927adb7a 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -32323,11 +32323,13 @@ export class ObjectStackProtocolImplementation implements } /** - * Edit an installed package's manifest (name / description / version) — the - * durable half of `PATCH /packages/:id`. Merges the patch into the registry - * (preserving lifecycle state — see {@link SchemaRegistry.updatePackageManifest}) - * then re-persists the merged manifest to `sys_packages` via the `package` - * service so the edit survives a restart. + * Edit an installed package's manifest (name / description / version / + * dependencies) — the durable half of `PATCH /packages/:id`. Merges the + * patch into the registry (preserving lifecycle state — see + * {@link SchemaRegistry.updatePackageManifest}; `dependencies` replaces the + * stored row whole) then re-persists the merged manifest to `sys_packages` + * via the `package` service so the edit survives a restart. The + * `dependencies` row arrives judged by the door (`ManifestSchema`'s row). * * [#21243] A failed persist FAILS the edit, exactly as it fails * `installPackage`: the registry row is put back to what it held before the @@ -32340,7 +32342,7 @@ export class ObjectStackProtocolImplementation implements */ async updatePackage(request: { packageId: string; - patch: { name?: string; description?: string; version?: string }; + patch: { name?: string; description?: string; version?: string; dependencies?: Record }; }): Promise<{ package: any; message: string }> { // [#21243] The row as it stood before the patch — copied, because // `updatePackageManifest` edits the row and its manifest IN PLACE. diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 6991094f504..0d0948aec11 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -6138,18 +6138,29 @@ export class SchemaRegistry { } /** - * Merge editable manifest fields (name / description / version) into an - * installed package — the in-memory half of `PATCH /packages/:id`. + * Merge editable manifest fields (name / description / version / + * dependencies) into an installed package — the in-memory half of + * `PATCH /packages/:id`. * * Only the human-editable fields are touched: `id` is identity (never * changes here), and lifecycle facets (`status` / `enabled` / `installedAt`) * are preserved — this is a metadata edit, NOT a reinstall (which * `installPackage` would be, resetting exactly those). Undefined patch * fields are ignored so a partial PATCH only overwrites what it sends. + * + * [#24670] `dependencies` is replaced WHOLE, never deep-merged: the row the + * caller sends is the declaration, so an entry it leaves out is removed and + * `{}` clears it. The registry keeps its own copy of the row. Replacing the + * object (rather than editing the stored one in place) is also what keeps + * the protocol's pre-patch snapshot — a shallow copy of the manifest — able + * to put the prior row back when the durable write fails. The row is judged + * by the door before it gets here (`ManifestSchema`'s `dependencies` row); + * the closure the save door reads off it is resolved per write, so the next + * save sees the edit with nothing cached to retire. */ updatePackageManifest( id: string, - patch: { name?: string; description?: string; version?: string }, + patch: { name?: string; description?: string; version?: string; dependencies?: Record }, ): InstalledPackage | undefined { const pkg = this.getPackage(id); if (!pkg) return undefined; @@ -6157,6 +6168,7 @@ export class SchemaRegistry { if (patch.name !== undefined) manifest.name = patch.name; if (patch.description !== undefined) manifest.description = patch.description; if (patch.version !== undefined) manifest.version = patch.version; + if (patch.dependencies !== undefined) manifest.dependencies = { ...patch.dependencies }; pkg.updatedAt = new Date().toISOString(); this.log(`[Registry] Updated package manifest: ${id}`); return pkg; diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 0978f794ef2..76bcc0f8077 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -2245,18 +2245,25 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin } // PATCH /packages/:id → edit the manifest (name / description / - // version). A partial patch: only the fields present are changed; - // lifecycle state (enabled / status / installedAt) is preserved. - // `id` / `scope` / `type` are identity/structure and are NOT editable - // here. Body accepts the fields flat or under a `manifest` wrapper. + // version / dependencies). A partial patch: only the fields present + // are changed; lifecycle state (enabled / status / installedAt) is + // preserved. `id` / `scope` / `type` are identity/structure and are + // NOT editable here. Body accepts the fields flat or under a + // `manifest` wrapper. if (parts.length === 1 && m === 'PATCH') { const denied = requireManageMetadata(deps, _context); if (denied) return denied; const id = decodeURIComponent(parts[0]); // [ADR-0131 D12 items 7–8] Under a wall, the platform operator's. const walled = await refuseWalledCatalogPackageWrite(deps, _context, `Updating package "${id}"`); if (walled) return walled; - const src = (body?.manifest && typeof body.manifest === 'object' ? body.manifest : body) ?? {}; - const patch: { name?: string; description?: string; version?: string } = {}; + const wrapped = Boolean(body?.manifest && typeof body.manifest === 'object'); + const src = (wrapped ? body.manifest : body) ?? {}; + const patch: { + name?: string; + description?: string; + version?: string; + dependencies?: Record; + } = {}; if (typeof src.name === 'string') patch.name = src.name.trim(); if (typeof src.description === 'string') patch.description = src.description; if (typeof src.version === 'string') patch.version = src.version.trim(); @@ -2267,8 +2274,48 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin if (patch.version !== undefined && !SEMVER_2_0_0_VERSION_PATTERN.test(patch.version)) { return { handled: true, response: deps.error('version must be a SemVer 2.0.0 string (e.g. 1.0.0, 2.0.0-beta.1)', 400) }; } - if (patch.name === undefined && patch.description === undefined && patch.version === undefined) { - return { handled: true, response: deps.error('Body { name?, description?, version? } — nothing to update', 400) }; + // [#24670] `dependencies` — the declaration the save door's closure + // verdict reads (ruling #9612), and the key its refusal tells the + // author to edit («declare a dependency … in the `dependencies` of + // P's manifest»). It could be set at create and never again, so for + // a package that already existed that remedy had no door. + // + // ⭐ THE ROW IS JUDGED BY THE DECLARATION, by reference: + // `ManifestSchema.shape.dependencies`, the row `POST /packages` + // parses the same key with at create — never a shape restated + // here. A malformed row is refused `400` (`VALIDATION_ERROR`, the + // envelope the create door answers for it) BEFORE the protocol or + // the registry is reached, so a refusal writes nothing, not even a + // valid key sent beside it. + // + // The parsed row REPLACES the stored one whole — removing an entry + // is the same write, with the entry left out, and `{}` clears the + // declaration. ⛔ No in-use check is made here: an edit that takes + // an object some stored item binds to out of the closure is + // applied as given (the stored item is judged at its next save). + if (src.dependencies !== undefined) { + const declared = ManifestSchema.shape.dependencies.safeParse(src.dependencies); + if (!declared.success) { + const key = wrapped ? 'manifest.dependencies' : 'dependencies'; + const located = declared.error.issues + .map((issue) => `${[key, ...issue.path.map(String)].join('.')}: ${issue.message}`) + .join('; '); + return { + handled: true, + response: deps.error( + `Invalid package dependencies — ${located}. \`dependencies\` maps package ids to ` + + `version requirements, e.g. { "com.acme.core": "^1.0.0" }`, + 400, + ), + }; + } + patch.dependencies = declared.data; + } + if ( + patch.name === undefined && patch.description === undefined + && patch.version === undefined && patch.dependencies === undefined + ) { + return { handled: true, response: deps.error('Body { name?, description?, version?, dependencies? } — nothing to update', 400) }; } const protocol = await resolveProtocol(deps, _context); diff --git a/packages/runtime/src/package-dependencies-patch.integration.test.ts b/packages/runtime/src/package-dependencies-patch.integration.test.ts index 70c0fa8ff05..7b960169d6f 100644 --- a/packages/runtime/src/package-dependencies-patch.integration.test.ts +++ b/packages/runtime/src/package-dependencies-patch.integration.test.ts @@ -22,7 +22,8 @@ * row stores it, and after a restart (the stored rows hydrated into a new * registry) the door serves it again; * (b) a malformed row is refused `400 VALIDATION_ERROR` and neither the - * registry nor the `sys_packages` row moves; + * registry nor the `sys_packages` row moves; and when `sys_packages` + * refuses a well-formed edit, the prior row is put back everywhere; * (d) end to end: a list view saved into P bound to an object of a package P * never declared is refused `422 INVALID_METADATA` * (`list-view-object-outside-closure`), and after the PATCH declares that @@ -241,6 +242,27 @@ describe('[#24670] PATCH /packages/:id — `dependencies` on the protocol branch expect(await p.storedManifest(OWN)).toEqual(storedBefore); }, 60_000); + it('(b) a refused durable write puts the prior row back: the registry, GET and sys_packages keep it', async () => { + // The row is replaced WHOLE in the registry, which is what lets the + // protocol's shallow pre-patch snapshot restore it when `sys_packages` + // refuses the write (#21243's undo). + const p = await boot(newDir()); + await p.install(OWN); + const declared = { [DEP]: '^1.0.0' }; + expect((await p.call('PATCH', `/${OWN}`, { dependencies: declared })).status, 'precondition').toBe(200); + await (p.engine as any).execute({ + sql: `CREATE TRIGGER refuse_own_update BEFORE UPDATE ON sys_packages ` + + `WHEN OLD.id = '${OWN}' BEGIN SELECT RAISE(ABORT, 'refused by the test trigger'); END`, + }); + + const answer = await p.call('PATCH', `/${OWN}`, { dependencies: {} }); + + expect(answer.status, JSON.stringify(answer.body)).toBe(500); + expect(p.registryManifest(OWN)?.dependencies).toEqual(declared); + expect((await p.call('GET', `/${OWN}`)).body?.data?.manifest?.dependencies).toEqual(declared); + expect((await p.storedManifest(OWN))?.dependencies).toEqual(declared); + }, 60_000); + it('(d) the remedy end to end: a save refused for an undeclared package is admitted once the PATCH declares it', async () => { const p = await boot(newDir()); await p.install(OWN); From 0fcd8e42c267862c8a10f8f7a89d40b5a4999ed1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 23:43:38 +0000 Subject: [PATCH 3/3] chore(changeset): minor on runtime, objectql and metadata-protocol for the dependencies edit Claude-Session: https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6 Co-authored-by: Claude --- .changeset/24670-package-dependencies-patch.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .changeset/24670-package-dependencies-patch.md diff --git a/.changeset/24670-package-dependencies-patch.md b/.changeset/24670-package-dependencies-patch.md new file mode 100644 index 00000000000..5cae982ef98 --- /dev/null +++ b/.changeset/24670-package-dependencies-patch.md @@ -0,0 +1,15 @@ +--- +'@objectstack/runtime': minor +'@objectstack/objectql': minor +'@objectstack/metadata-protocol': minor +--- + +feat(runtime): `PATCH /api/v1/packages/:id` edits a package's manifest `dependencies` + +Clause-②: yes (widening) + +- **The door.** `PATCH /api/v1/packages/:id` accepts `dependencies` beside `name`, `description` and `version`, flat or under the `manifest` wrapper. The row is judged by `ManifestSchema`'s own `dependencies` row (package ids mapped to version requirements), the row `POST /api/v1/packages` judges at create. A malformed row (a non-string version, a list, a bare string, `null`) is refused `400 VALIDATION_ERROR`, the envelope the create door answers for it, and nothing is written, a valid key sent beside it included. +- **The row replaces the stored one whole.** An entry the row leaves out is removed, and `{}` clears the declaration. On a host with the `package` service the edit is written to `sys_packages` and survives a restart; when that write is refused, the prior row is put back and the door answers the failure. +- **Why.** A save into a package is judged against the package's closure, its declared `dependencies` included, and a binding outside it is refused with the remedy "declare a dependency on the package that defines it in the `dependencies` of the package's manifest". Before this change `dependencies` could be set only at create, so for a package that already existed that remedy had no door. The closure is read per save, so the next save sees the edit. +- **Removing an entry is applied as given.** No in-use check is made. An item already stored that binds to an object the edit takes out of the closure keeps loading and serving after a restart; its next save is refused with the closure verdict (`422 INVALID_METADATA`; for a list view, `list-view-object-outside-closure`). +- **Published signatures.** `SchemaRegistry.updatePackageManifest` (`@objectstack/objectql`) and `ObjectStackProtocolImplementation.updatePackage` (`@objectstack/metadata-protocol`) each accept an optional `dependencies` row in their patch. No export is added, removed or renamed.