From 7560e7f9e0abbb6ba27fd1a132d7f39c9ece7024 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 10:55:52 +0000 Subject: [PATCH 1/2] feat(cli): os meta delete and os meta register take --package, forwarded as the SDK's packageId `os meta delete --package ` forwards DeleteMetaItemOptions.packageId (?package=), the remedy the reset door's 409 RESOURCE_CONFLICT prescribes for an item two packages hold. `os meta register --package ` forwards SaveMetaItemOptions.packageId, so a save can name the row it writes. An empty --package is refused before a client exists, as --if-match is. The DeleteMetaItemOptions header and metaDeleteOptions' docblock name the reset door's carriers instead of counting them. Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude --- .../meta/delete-reset-carriers.test.ts | 167 ++++++- packages/cli/src/commands/meta/delete.ts | 57 ++- .../commands/meta/register-package.test.ts | 430 ++++++++++++++++++ packages/cli/src/commands/meta/register.ts | 63 ++- packages/client/src/index.ts | 14 +- 5 files changed, 718 insertions(+), 13 deletions(-) create mode 100644 packages/cli/src/commands/meta/register-package.test.ts diff --git a/packages/cli/src/commands/meta/delete-reset-carriers.test.ts b/packages/cli/src/commands/meta/delete-reset-carriers.test.ts index b6f9c518f0f..9f7c7888f3f 100644 --- a/packages/cli/src/commands/meta/delete-reset-carriers.test.ts +++ b/packages/cli/src/commands/meta/delete-reset-carriers.test.ts @@ -78,7 +78,11 @@ import { } from '@objectstack/platform-objects'; import { RestServer } from '@objectstack/runtime'; import { ObjectStackClient } from '@objectstack/client'; -import MetaDelete, { metaDeleteOptions, EMPTY_IF_MATCH_REFUSAL } from './delete.js'; +import MetaDelete, { + metaDeleteOptions, + EMPTY_IF_MATCH_REFUSAL, + EMPTY_PACKAGE_DELETE_REFUSAL, +} from './delete.js'; /** * The state the `createApiClient` stub reads, and the call counter that makes @@ -388,10 +392,10 @@ describe('[#13024] the flags `os meta delete` declares', () => { expect(String((refused.thrown as Error)?.message)).toMatch(/state/i); }, 60_000); - it('the withheld third carrier has no flag, under either spelling', () => { + it('the withheld `?dropStorage` carrier has no flag, under either spelling', () => { const names = Object.keys(MetaDelete.flags as Record); - // Commit cf71d73f8 shipped two of the door's three carriers on purpose; `?dropStorage` - // is the one that ADDS destructive reach. A CLI flag for it would reverse + // `?dropStorage` is withheld on purpose (commit cf71d73f8): it is the + // carrier that ADDS destructive reach. A CLI flag for it would reverse // that ruling from the layer above. expect(names).not.toContain('dropStorage'); expect(names).not.toContain('drop-storage'); @@ -399,6 +403,7 @@ describe('[#13024] the flags `os meta delete` declares', () => { // object sees the flags that ARE declared. expect(names).toContain('if-match'); expect(names).toContain('draft'); + expect(names).toContain('package'); }); it('`metaDeleteOptions` returns UNDEFINED when neither flag is given', () => { @@ -762,3 +767,157 @@ describe('[#13024] the CLI\'s option bag behaves identically through both SDK tw expect(Object.keys(scoped.headers).sort()).toEqual(['content-type']); }, 60_000); }); + +// --------------------------------------------------------------------------- +// Part 6 — `--package`: the remedy the door's 409 RESOURCE_CONFLICT prescribes +// --------------------------------------------------------------------------- + +/** + * [#23350] Since #23165 the reset door refuses a delete that names no package + * of an item two or more packages hold — `409 RESOURCE_CONFLICT`, naming the + * packages, prescribing `?package=` — and since #23293 the SDK can send it + * (`DeleteMetaItemOptions.packageId`). This command declared no flag for it, + * so a CLI operator who met that refusal was told to do something the command + * could not send. These cases drive the same real door as the parts above. + */ +const PKG = 'com.qa.alpha'; +const OTHER_PKG = 'com.qa.beta'; + +/** One item saved into two packages THROUGH THE SDK: one row per package (ADR-0048). */ +async function twoPackageItem(client: ObjectStackClient, name: string) { + await client.meta.saveItem('view', name, VIEW(name, 'A'), { packageId: PKG }); + await client.meta.saveItem('view', name, VIEW(name, 'B'), { packageId: OTHER_PKG }); +} + +/** The package each stored row of `name` is bound to, sorted. */ +async function packagesOf(engine: any, name: string): Promise { + return (await overlayRows(engine, name)).map((r: any) => String(r.package_id)).sort(); +} + +describe('[#23350] the `--package` flag `os meta delete` declares', () => { + it('declares `--package` as a value flag whose help names the refusal it answers', () => { + const flags = MetaDelete.flags as Record; + expect(flags.package.type).toBe('option'); + // An operator who meets the 409 has to be able to find the remedy from + // `--help`: the flag says which refusal it answers. + expect(flags.package.description).toMatch(/409/); + expect(flags.package.description).toMatch(/RESOURCE_CONFLICT/); + const examples = (MetaDelete.examples as string[]).join('\n'); + expect(examples).toContain('--package'); + }); + + it('`metaDeleteOptions` forwards the id VERBATIM as `packageId`, beside the other two flags', () => { + expect(metaDeleteOptions({ package: PKG })).toEqual({ packageId: PKG }); + expect(metaDeleteOptions({ package: PKG, draft: true, 'if-match': 'v1' })) + .toEqual({ packageId: PKG, state: 'draft', ifMatch: 'v1' }); + // No reshaping: the door, not this command, decides what an id names. + expect(metaDeleteOptions({ package: ' com.qa.alpha ' })).toEqual({ packageId: ' com.qa.alpha ' }); + // Unset stays `undefined`, so the unflagged request is unchanged. + expect(metaDeleteOptions({ package: undefined })).toBeUndefined(); + }); + + it('an empty or blank `--package` is refused, never sent as a package-less delete', () => { + expect(() => metaDeleteOptions({ package: '' })).toThrow(EMPTY_PACKAGE_DELETE_REFUSAL); + expect(() => metaDeleteOptions({ package: ' ' })).toThrow(EMPTY_PACKAGE_DELETE_REFUSAL); + }); +}); + +describe('[#23350] `os meta delete --package` against the real reset door', () => { + it('NAMING NONE (the only delete this command could express before): the command reports the 409 and nothing is deleted', async () => { + const { engine, client, deleteRequests, wire } = await bootDoor(); + await twoPackageItem(client, 'two_pkg_probe'); + // The two-package state is reachable through the door: one row per package. + expect(await packagesOf(engine, 'two_pkg_probe')).toEqual([PKG, OTHER_PKG].sort()); + + const run = await runCli(['view', 'two_pkg_probe', '--format', 'json']); + + // The refusal reaches the operator in the machine-readable envelope — + // `code` and `httpStatus`, not merely a failure. + expect(run.exitCode).toBe(1); + const payload = JSON.parse(run.out); + expect(payload.success).toBe(false); + expect(payload.code).toBe('RESOURCE_CONFLICT'); + expect(payload.httpStatus).toBe(409); + // It names both packages, so the operator can pick one for `--package`. + expect(String(payload.error)).toContain(PKG); + expect(String(payload.error)).toContain(OTHER_PKG); + // Nothing was deleted. + expect(await packagesOf(engine, 'two_pkg_probe')).toEqual([PKG, OTHER_PKG].sort()); + // The probe: no package reached the protocol, and none was on the wire. + expect(deleteRequests).toHaveLength(1); + expect(deleteRequests[0]).not.toHaveProperty('packageId'); + expect(wire.filter((w) => w.method === 'DELETE').at(-1)!.search).toBe(''); + }, 60_000); + + it('NAMING ONE: `--package` removes that package\'s row only, and the other package\'s row stays', async () => { + const { engine, client, deleteRequests, wire } = await bootDoor(); + await twoPackageItem(client, 'two_pkg_probe'); + + const run = await runCli(['view', 'two_pkg_probe', '--package', PKG, '--format', 'json']); + + expect(run.exitCode).toBe(0); + expect(JSON.parse(run.out)).toMatchObject({ success: true, deleted: true }); + expect(await packagesOf(engine, 'two_pkg_probe')).toEqual([OTHER_PKG]); + // POSITIVE CONTROL for the previous case's absence: the same probe sees + // the package arrive, read off `?package=` by the door. + expect(deleteRequests).toHaveLength(1); + expect(deleteRequests[0].packageId).toBe(PKG); + expect(wire.filter((w) => w.method === 'DELETE').at(-1)!.search).toBe(`?package=${PKG}`); + }, 60_000); + + it('NAMING A PACKAGE THAT HOLDS NO ROW: nothing is removed — the flag scopes the delete, it does not widen it', async () => { + const { engine, client, deleteRequests } = await bootDoor(); + await twoPackageItem(client, 'two_pkg_probe'); + + const run = await runCli(['view', 'two_pkg_probe', '--package', 'com.qa.gamma', '--format', 'json']); + + expect(run.exitCode).toBe(0); + expect(JSON.parse(run.out)).toMatchObject({ success: true, deleted: false }); + expect(await packagesOf(engine, 'two_pkg_probe')).toEqual([PKG, OTHER_PKG].sort()); + expect(deleteRequests.at(-1).packageId).toBe('com.qa.gamma'); + }, 60_000); + + it('`--package` composes with `--draft` and `--if-match` on the wire', async () => { + const { client, wire } = await bootDoor(); + await client.meta.saveItem('view', 'compose_probe', VIEW('compose_probe', 'A'), { packageId: PKG }); + await runCli(['view', 'compose_probe', '--draft', '--package', PKG, '--if-match', 'sha256:probe']); + const sent = wire.filter((w) => w.method === 'DELETE').at(-1)!; + expect(sent.search).toBe(`?state=draft&package=${PKG}`); + expect(sent.headers['if-match']).toBe('sha256:probe'); + }, 60_000); + + it('an empty `--package` is refused BEFORE the client is built — nothing reaches the door', async () => { + const { engine, client, deleteRequests } = await bootDoor(); + await client.meta.saveItem('view', 'empty_pkg', VIEW('empty_pkg', 'A'), { packageId: PKG }); + const requestsBefore = deleteRequests.length; + stub.createCalls = 0; + + // `--package "$PKG"` with `PKG` unset. Sent as the package-less delete, + // the door would resolve the item's one package and remove its row while + // the operator's command line reads as naming a package. + const run = await runCli(['view', 'empty_pkg', '--package', '']); + expect(run.exitCode).toBe(1); + expect(run.out).toContain(EMPTY_PACKAGE_DELETE_REFUSAL); + expect(stub.createCalls).toBe(0); + expect(deleteRequests).toHaveLength(requestsBefore); + expect(await packagesOf(engine, 'empty_pkg')).toEqual([PKG]); + + // POSITIVE CONTROL — the same instruments move on a run that is let through. + const ok = await runCli(['view', 'empty_pkg', '--package', PKG]); + expect(ok.exitCode).toBe(0); + expect(stub.createCalls).toBe(1); + expect(deleteRequests).toHaveLength(requestsBefore + 1); + expect(await packagesOf(engine, 'empty_pkg')).toEqual([]); + }, 60_000); + + it('the bag the CLI builds for `--package` sends identical bytes through both SDK twins', async () => { + const { client, wire } = await bootDoor(); + const options = metaDeleteOptions({ package: PKG }); + await client.meta.deleteItem('view', 'in_step_pkg', options).catch(() => undefined); + await client.environment('env-23350').meta.deleteItem('view', 'in_step_pkg', options).catch(() => undefined); + + const [unscoped, scoped] = wire.filter((w) => w.method === 'DELETE'); + expect(scoped.search).toBe(unscoped.search); + expect(unscoped.search).toBe(`?package=${PKG}`); + }, 60_000); +}); diff --git a/packages/cli/src/commands/meta/delete.ts b/packages/cli/src/commands/meta/delete.ts index b2ee33b0840..614a82e6f31 100644 --- a/packages/cli/src/commands/meta/delete.ts +++ b/packages/cli/src/commands/meta/delete.ts @@ -30,6 +30,26 @@ export const EMPTY_IF_MATCH_REFUSAL = + 'Echo the `version` a previous `os meta` save or publish resolved, or omit --if-match ' + 'to accept the unpinned (last-write-wins) reset.'; +/** + * [#23350] The refusal an EMPTY `--package` earns, before anything reaches + * the door — the `--if-match` rule above, for the same reason. + * + * The SDK sends nothing for `packageId: ''` (`DeleteMetaItemOptions.packageId`: + * "`undefined` and `''` both send nothing"), and a delete that names no + * package is not a refusal on every item: the door resolves it, removing the + * row of the one package that holds the item, or the package-unbound row when + * none does. So `os meta delete view x --package "$PKG"` with `PKG` unset would + * remove whichever row that resolution picks while the operator reads their + * command line as naming one package — and where the package they meant holds + * no row, the named delete would have removed nothing. The CLI refuses rather + * than degrades. A non-empty id is forwarded verbatim: the door, not this + * command, decides what an id names. + */ +export const EMPTY_PACKAGE_DELETE_REFUSAL = + '--package needs the id of the package whose row to delete, and was given an empty value. ' + + 'Name the package (for example com.acme.crm), or omit --package to delete the row ' + + 'the server resolves when the item belongs to one package.'; + /** * [#13024] The `DeleteMetaItemOptions` bag this command's flags mean — the one * place the CLI decides what goes on the wire. @@ -45,26 +65,43 @@ export const EMPTY_IF_MATCH_REFUSAL = * `os meta delete ` hands the SDK the same argument list it has * always handed it and the request stays byte-identical. * - * ⛔ `?dropStorage` is NOT here and gets no flag. Commit cf71d73f8 shipped two of the - * door's three carriers on purpose: the third ADDS destructive reach (it drops - * the object's physical table), no caller was measured needing it, and the - * door's repeated-parameter refusal exists because of that destructiveness. - * Publishing it from the CLI would reverse that ruling from the layer above. + * The reset door (`DELETE /meta/:type/:name`) reads these carriers: the + * `If-Match` header and the `state`, `package`, `dropStorage` and `member` + * query parameters (`META_ITEM_RESET_PARAMS` in `@objectstack/rest` is the + * query half). This command sends the first three, each from its own flag: + * `--if-match`, `--draft` and `--package` (#23350). The other two get no flag: + * + * ⛔ `?dropStorage` is NOT here and gets no flag, on purpose (commit cf71d73f8): + * it ADDS destructive reach (it drops the object's physical table), no caller + * was measured needing it, and the door's repeated-parameter refusal exists + * because of that destructiveness. Publishing it from the CLI would reverse + * that ruling from the layer above. + * + * `?member` (one member of a bundled type) gets no flag because + * `DeleteMetaItemOptions` declares no member for it, and this command sends + * only what the SDK bag carries. */ export function metaDeleteOptions(flags: { 'if-match'?: string; draft?: boolean; + package?: string; }): DeleteMetaItemOptions | undefined { const pin = flags['if-match']; if (pin !== undefined && pin.trim() === '') { throw new Error(EMPTY_IF_MATCH_REFUSAL); } + const packageId = flags.package; + if (packageId !== undefined && packageId.trim() === '') { + throw new Error(EMPTY_PACKAGE_DELETE_REFUSAL); + } const options: DeleteMetaItemOptions = {}; if (pin !== undefined) options.ifMatch = pin; // `state: 'active'` is never sent: it is the door's default said out loud, // and the SDK drops it. The flag is therefore a boolean, matching every // other binary opt-in in this CLI. if (flags.draft) options.state = 'draft'; + // [#23350] Verbatim, as the SDK takes it: `packageId` → `?package=`. + if (packageId !== undefined) options.packageId = packageId; return Object.keys(options).length > 0 ? options : undefined; } @@ -77,6 +114,7 @@ export default class MetaDelete extends Command { '$ os meta delete object my_custom_object --format json', '$ os meta delete view shared_grid --if-match "$VERSION"', '$ os meta delete view shared_grid --draft', + '$ os meta delete view shared_grid --package com.acme.crm', ]; static override args = { @@ -132,6 +170,15 @@ export default class MetaDelete extends Command { + 'Without it the reset is the full one and drops the published overlay too.', default: false, }), + // [#23350] The package whose row the delete removes, forwarded verbatim as + // `DeleteMetaItemOptions.packageId` (`?package=`). It is the remedy the + // door's `409 RESOURCE_CONFLICT` prescribes for an item two packages hold. + package: Flags.string({ + description: + 'Delete only the row of this package (a package id, e.g. com.acme.crm). Required when ' + + 'two or more packages hold the item: without it the server refuses with 409 ' + + 'RESOURCE_CONFLICT and names the packages. Other packages\' rows are left in place.', + }), }; async run(): Promise { diff --git a/packages/cli/src/commands/meta/register-package.test.ts b/packages/cli/src/commands/meta/register-package.test.ts new file mode 100644 index 00000000000..be1e23cf630 --- /dev/null +++ b/packages/cli/src/commands/meta/register-package.test.ts @@ -0,0 +1,430 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23350] `os meta register` can name the package its save writes into — + * `--package `, forwarded as `SaveMetaItemOptions.packageId` (`?package=`). + * + * ## The defect + * + * A `sys_metadata` row is keyed per package (ADR-0048), and the save door + * `PUT /meta/:type/:name` reads `?package=` as the row it writes. Since #23165 + * a save that names no package of an item two or more packages hold is + * refused `409 RESOURCE_CONFLICT`, naming the packages, with `?package=` as + * the remedy. `os meta register` called `client.meta.saveItem(type, name, + * metadata)` with no options bag, so it could neither follow that remedy nor + * address a package's row at all — although the SDK option has existed since + * 17.3. + * + * ## What is measured here, and on what + * + * The REAL oclif command, the REAL `ObjectStackClient`, the REAL registered + * route handler (`RestServer`), the REAL `ObjectStackProtocolImplementation` + * and REAL `sys_metadata*` tables on SQLite — the harness + * `delete-reset-carriers.test.ts` uses for `os meta delete`, with the same two + * stubs, both at the credential boundary: `createApiClient` (a real client + * whose `fetch` is a bridge into the registered handlers) and + * `resolveExecCtx` (the session holds `manage_metadata`). + * + * - Unflagged, the command saves ACTIVE (no `?mode=`) and sends no query + * string — unchanged by this card, and the state the 409 below is + * measured at. + * - Unflagged, an item two packages hold is refused `409 RESOURCE_CONFLICT` + * at the active save, and nothing is written. This is the negative control + * for the pin below, and the measurement the card left open. + * - `--package P` writes P's row, and only P's. + * - An empty `--package` is refused before a client exists. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +// The REAL stores the protocol writes to — the same specifier +// `delete-reset-carriers.test.ts` reaches them through, for the reason its +// import note gives. +import { + SysMetadataObject, + SysMetadataHistoryObject, + SysMetadataAuditObject, +} from '@objectstack/platform-objects'; +import { RestServer } from '@objectstack/runtime'; +import { ObjectStackClient } from '@objectstack/client'; +import MetaRegister, { metaRegisterOptions, EMPTY_PACKAGE_SAVE_REFUSAL } from './register.js'; + +const stub = vi.hoisted(() => ({ + client: undefined as any, + token: 'test-token' as string | undefined, + createCalls: 0, +})); + +vi.mock('../../utils/api-client.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createApiClient: async () => { + stub.createCalls += 1; + return { client: stub.client, token: stub.token }; + }, + }; +}); + +/** `packages/cli` — the oclif root the command is loaded against below. */ +const CLI_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..', '..'); + +/** SGR codes, written as `\x1b` and never as the raw byte. */ +const SGR = /\x1b\[[0-9;]*m/g; +const plain = (s: string) => s.replace(SGR, ''); + +interface CliRun { out: string; exitCode: number; thrown: unknown } + +/** Drive the real oclif command with a real argv (see the delete suite's `runCli`). */ +async function runCli(argv: string[]): Promise { + const chunks: string[] = []; + const logSpy = vi + .spyOn(console, 'log') + .mockImplementation((...args: unknown[]) => { chunks.push(args.map(String).join(' ')); }); + // `emitText` awaits `stdout.write`'s callback; a stub that swallows it hangs. + const writeSpy = vi + .spyOn(process.stdout, 'write') + .mockImplementation(((chunk: unknown, encodingOrCb?: unknown, maybeCb?: unknown) => { + chunks.push(String(chunk)); + const done = typeof encodingOrCb === 'function' ? encodingOrCb : maybeCb; + if (typeof done === 'function') (done as (err?: Error | null) => void)(null); + return true; + }) as never); + const savedExitCode = process.exitCode; + let exitCode = 0; + let thrown: unknown; + try { + await MetaRegister.run(argv, { root: CLI_ROOT }); + } catch (error: unknown) { + thrown = error; + const oclif = (error as { oclif?: { exit?: number } })?.oclif; + exitCode = typeof oclif?.exit === 'number' ? oclif.exit : 1; + } finally { + logSpy.mockRestore(); + writeSpy.mockRestore(); + process.exitCode = savedExitCode; + } + return { out: plain(chunks.join('\n')), exitCode, thrown }; +} + +// --------------------------------------------------------------------------- +// The real stack the command talks to +// --------------------------------------------------------------------------- + +const TEST_PACKAGE_ID = 'objectstack-test'; +const PKG = 'com.qa.alpha'; +const OTHER_PKG = 'com.qa.beta'; + +const TASK = { + name: 'task', + label: 'Task', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + }, +}; + +const VIEW = (name: string, label: string) => ({ + name, + label, + object: 'task', + viewKind: 'list', + columns: [{ field: 'name', label: 'Name' }], +}); + +function createMockHttpServer() { + const noop = () => {}; + return { + get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, + listen: async () => {}, close: async () => {}, + }; +} + +function makeRes() { + const res: any = { + _status: 200, + write: () => true, + end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +function matchRoute(routes: any[], method: string, pathname: string) { + for (const route of routes) { + if (String(route.method).toUpperCase() !== method) continue; + const pattern = String(route.path).split('/'); + const actual = pathname.split('/'); + if (pattern.length !== actual.length) continue; + const params: Record = {}; + let ok = true; + for (let i = 0; i < pattern.length; i++) { + if (pattern[i].startsWith(':')) params[pattern[i].slice(1)] = decodeURIComponent(actual[i]); + else if (pattern[i] !== actual[i]) { ok = false; break; } + } + if (ok) return { route, params }; + } + return undefined; +} + +interface WireRecord { method: string; pathname: string; search: string; headers: Record } + +/** A `fetch` that hands the client's request to the REAL registered handler. */ +function doorFetch(rest: RestServer, wire: WireRecord[]) { + const routes = (rest as any).getRoutes(); + return async (input: any, init: any = {}) => { + const url = new URL(String(input)); + const method = String(init.method ?? 'GET').toUpperCase(); + const headers: Record = {}; + for (const [k, v] of Object.entries((init.headers ?? {}) as Record)) { + headers[k.toLowerCase()] = String(v); + } + wire.push({ method, pathname: url.pathname, search: url.search, headers }); + const hit = matchRoute(routes, method, url.pathname); + if (!hit) throw new Error(`no route registered for ${method} ${url.pathname}`); + const query: Record = {}; + for (const key of new Set(url.searchParams.keys())) { + const all = url.searchParams.getAll(key); + query[key] = all.length > 1 ? all : all[0]; + } + const res = makeRes(); + await hit.route.handler( + { params: hit.params, query, headers, body: init.body ? JSON.parse(String(init.body)) : undefined } as any, + res, + ); + const status = res._status ?? 200; + return { + ok: status >= 200 && status < 300, + status, + statusText: status === 200 ? 'OK' : 'Error', + json: async () => res._json, + headers: new Headers(), + } as any; + }; +} + +const liveEngines: ObjectQL[] = []; +const tempDirs: string[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } + while (tempDirs.length) rmSync(tempDirs.pop()!, { recursive: true, force: true }); + stub.client = undefined; + stub.token = 'test-token'; + stub.createCalls = 0; +}); + +/** Boot the real stack and point the command's client at it. */ +async function bootDoor() { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver( + new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }) as never, + true, + ); + await engine.init(); + engine.registry.registerObject(TASK as any, TEST_PACKAGE_ID); + engine.registry.registerObject(SysMetadataObject as any, TEST_PACKAGE_ID); + engine.registry.registerObject(SysMetadataHistoryObject as any, TEST_PACKAGE_ID); + engine.registry.registerObject(SysMetadataAuditObject as any, TEST_PACKAGE_ID); + await engine.syncSchemas(); + + const protocol: any = new ObjectStackProtocolImplementation(engine as any); + + /** The PROBE: every save request the door hands the protocol. */ + const saveRequests: any[] = []; + const realSave = protocol.saveMetaItem.bind(protocol); + protocol.saveMetaItem = async (request: any) => { + saveRequests.push(request); + return realSave(request); + }; + + const rest = new RestServer( + createMockHttpServer() as any, + protocol as any, + { api: { requireAuth: false, enableProjectScoping: true, projectResolution: 'auto' } } as any, + ); + (rest as any).resolveExecCtx = async () => ({ + userId: 'usr_admin_23350', + systemPermissions: ['manage_metadata'], + }); + rest.registerRoutes(); + + const wire: WireRecord[] = []; + const client = new ObjectStackClient({ baseUrl: 'http://door.test', fetch: doorFetch(rest, wire) }); + stub.client = client; + return { engine, client, saveRequests, wire }; +} + +/** The stored rows of one item, straight out of `sys_metadata`. */ +async function overlayRows(engine: any, name: string) { + return engine.find('sys_metadata', { where: { name }, context: { isSystem: true } }); +} + +/** `package_id → label` of every stored row of `name`, with each row's state. */ +async function labelsByPackage(engine: any, name: string): Promise> { + const out: Record = {}; + for (const row of await overlayRows(engine, name)) { + const body = typeof row.metadata === 'string' ? JSON.parse(row.metadata) : row.metadata; + out[String(row.package_id ?? '')] = { label: body?.label, state: row.state }; + } + return out; +} + +/** Write the `--data` file the command reads. */ +function dataFile(body: unknown): string { + const dir = mkdtempSync(path.join(os.tmpdir(), 'os-meta-register-23350-')); + tempDirs.push(dir); + const file = path.join(dir, 'view.json'); + writeFileSync(file, JSON.stringify(body)); + return file; +} + +/** One item saved into two packages THROUGH THE SDK: one row per package (ADR-0048). */ +async function twoPackageItem(client: ObjectStackClient, name: string) { + await client.meta.saveItem('view', name, VIEW(name, 'A'), { packageId: PKG }); + await client.meta.saveItem('view', name, VIEW(name, 'B'), { packageId: OTHER_PKG }); +} + +// --------------------------------------------------------------------------- +// The declared surface +// --------------------------------------------------------------------------- + +describe('[#23350] the `--package` flag `os meta register` declares', () => { + it('declares `--package` as a value flag whose help names the refusal it answers', () => { + const flags = MetaRegister.flags as Record; + expect(flags.package.type).toBe('option'); + expect(flags.package.description).toMatch(/409/); + expect(flags.package.description).toMatch(/RESOURCE_CONFLICT/); + expect((MetaRegister.examples as string[]).join('\n')).toContain('--package'); + }); + + it('`metaRegisterOptions` forwards the id VERBATIM as `packageId`, and is `undefined` unflagged', () => { + expect(metaRegisterOptions({})).toBeUndefined(); + expect(metaRegisterOptions({ package: undefined })).toBeUndefined(); + expect(metaRegisterOptions({ package: PKG })).toEqual({ packageId: PKG }); + // No reshaping: the door, not this command, decides what an id names. + expect(metaRegisterOptions({ package: ' com.qa.alpha ' })).toEqual({ packageId: ' com.qa.alpha ' }); + }); + + it('an empty or blank `--package` is refused, never sent as a package-less save', () => { + expect(() => metaRegisterOptions({ package: '' })).toThrow(EMPTY_PACKAGE_SAVE_REFUSAL); + expect(() => metaRegisterOptions({ package: ' ' })).toThrow(EMPTY_PACKAGE_SAVE_REFUSAL); + }); +}); + +// --------------------------------------------------------------------------- +// The real save door +// --------------------------------------------------------------------------- + +describe('[#23350] `os meta register --package` against the real save door', () => { + it('UNFLAGGED, an item with no package: an ACTIVE save, with no query string — the request this command always sent', async () => { + const { engine, saveRequests, wire } = await bootDoor(); + + const run = await runCli(['view', '--data', dataFile(VIEW('plain_probe', 'A'))]); + + expect(run.exitCode).toBe(0); + expect(run.out).toContain('Metadata registered: view/plain_probe'); + const sent = wire.filter((w) => w.method === 'PUT').at(-1)!; + expect(sent.pathname).toBe('/api/v1/meta/view/plain_probe'); + // No `?mode=draft` and no `?package=`: the save is the active one. + expect(sent.search).toBe(''); + expect(Object.keys(sent.headers).sort()).toEqual(['content-type']); + expect(saveRequests.at(-1)).not.toHaveProperty('packageId'); + expect(await labelsByPackage(engine, 'plain_probe')).toEqual({ '': { label: 'A', state: 'active' } }); + }, 60_000); + + it('UNFLAGGED, an item TWO packages hold: the active save is refused 409 RESOURCE_CONFLICT, and nothing is written', async () => { + const { engine, client, saveRequests, wire } = await bootDoor(); + await twoPackageItem(client, 'two_pkg_save'); + const before = await labelsByPackage(engine, 'two_pkg_save'); + expect(before).toEqual({ + [PKG]: { label: 'A', state: 'active' }, + [OTHER_PKG]: { label: 'B', state: 'active' }, + }); + + const run = await runCli(['view', '--data', dataFile(VIEW('two_pkg_save', 'C')), '--format', 'json']); + + expect(run.exitCode).toBe(1); + const payload = JSON.parse(run.out); + expect(payload.success).toBe(false); + expect(payload.code).toBe('RESOURCE_CONFLICT'); + expect(payload.httpStatus).toBe(409); + // It names both packages, so the operator can pick one for `--package`. + expect(String(payload.error)).toContain(PKG); + expect(String(payload.error)).toContain(OTHER_PKG); + // Nothing written: both rows keep their bodies. + expect(await labelsByPackage(engine, 'two_pkg_save')).toEqual(before); + // The probe: no package reached the protocol, and none was on the wire. + expect(saveRequests.at(-1)).not.toHaveProperty('packageId'); + expect(wire.filter((w) => w.method === 'PUT').at(-1)!.search).toBe(''); + }, 60_000); + + it('`--package P` writes P\'s row, and the other package\'s row is untouched', async () => { + const { engine, client, saveRequests, wire } = await bootDoor(); + await twoPackageItem(client, 'two_pkg_save'); + + const run = await runCli(['view', '--data', dataFile(VIEW('two_pkg_save', 'C')), '--package', PKG]); + + expect(run.exitCode).toBe(0); + expect(run.out).toContain('Metadata registered: view/two_pkg_save'); + expect(await labelsByPackage(engine, 'two_pkg_save')).toEqual({ + [PKG]: { label: 'C', state: 'active' }, + [OTHER_PKG]: { label: 'B', state: 'active' }, + }); + // POSITIVE CONTROL for the previous case's absence: the same probe sees + // the package arrive, read off `?package=` by the door. + expect(saveRequests.at(-1).packageId).toBe(PKG); + expect(wire.filter((w) => w.method === 'PUT').at(-1)!.search).toBe(`?package=${PKG}`); + }, 60_000); + + it('`--package` names a NEW package: the save writes that package\'s row beside the existing one', async () => { + const { engine, client } = await bootDoor(); + await client.meta.saveItem('view', 'one_pkg_save', VIEW('one_pkg_save', 'A'), { packageId: PKG }); + + const run = await runCli(['view', '--data', dataFile(VIEW('one_pkg_save', 'B')), '--package', OTHER_PKG]); + + expect(run.exitCode).toBe(0); + expect(await labelsByPackage(engine, 'one_pkg_save')).toEqual({ + [PKG]: { label: 'A', state: 'active' }, + [OTHER_PKG]: { label: 'B', state: 'active' }, + }); + }, 60_000); + + it('an empty `--package` is refused BEFORE the client is built — nothing reaches the door', async () => { + const { engine, client, saveRequests } = await bootDoor(); + await client.meta.saveItem('view', 'empty_pkg', VIEW('empty_pkg', 'A'), { packageId: PKG }); + const requestsBefore = saveRequests.length; + stub.createCalls = 0; + + // `--package "$PKG"` with `PKG` unset. Sent as the package-less save, the + // door would resolve the item's one package and write its row while the + // operator's command line reads as naming a package. + const run = await runCli(['view', '--data', dataFile(VIEW('empty_pkg', 'B')), '--package', '']); + expect(run.exitCode).toBe(1); + expect(run.out).toContain(EMPTY_PACKAGE_SAVE_REFUSAL); + expect(stub.createCalls).toBe(0); + expect(saveRequests).toHaveLength(requestsBefore); + expect(await labelsByPackage(engine, 'empty_pkg')).toEqual({ [PKG]: { label: 'A', state: 'active' } }); + + // POSITIVE CONTROL — the same instruments move on a run that is let through. + const ok = await runCli(['view', '--data', dataFile(VIEW('empty_pkg', 'B')), '--package', PKG]); + expect(ok.exitCode).toBe(0); + expect(stub.createCalls).toBe(1); + expect(saveRequests).toHaveLength(requestsBefore + 1); + expect(await labelsByPackage(engine, 'empty_pkg')).toEqual({ [PKG]: { label: 'B', state: 'active' } }); + }, 60_000); +}); diff --git a/packages/cli/src/commands/meta/register.ts b/packages/cli/src/commands/meta/register.ts index a919acd7662..0b04f2567df 100644 --- a/packages/cli/src/commands/meta/register.ts +++ b/packages/cli/src/commands/meta/register.ts @@ -1,16 +1,63 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; +import type { SaveMetaItemOptions } from '@objectstack/client'; import { printError, printSuccess, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; +/** + * [#23350] The refusal an EMPTY `--package` earns, before anything reaches the + * door — the rule `os meta delete --package` follows, for the same reason. + * + * The SDK sends nothing for `packageId: ''` (`SaveMetaItemOptions.packageId` + * is truthy-guarded), and a save that names no package is not a refusal on + * every item: the door resolves it, writing the row of the one package that + * holds the item, or the package-unbound overlay when none does. So + * `os meta register view --data v.json --package "$PKG"` with `PKG` unset would + * write whichever row that resolution picks while the operator reads their + * command line as naming one package. The CLI refuses rather than degrades. A + * non-empty id is forwarded verbatim: the door, not this command, decides what + * an id names. + */ +export const EMPTY_PACKAGE_SAVE_REFUSAL = + '--package needs the id of the package the save writes into, and was given an empty value. ' + + 'Name the package (for example com.acme.crm), or omit --package to save into the row ' + + 'the server resolves when the item belongs to one package.'; + +/** + * [#23350] The `SaveMetaItemOptions` bag this command's flags mean — the one + * place the command decides what goes on the wire, as `metaDeleteOptions` is + * for `os meta delete`. Exported so the wire claim is measured on this value. + * + * Returns `undefined` — never `{}` — when no flag is set, so an ordinary + * `os meta register --data ` hands the SDK no options and the + * request stays byte-identical to what it sent before the flag existed: an + * active (published) save, `PUT /meta/:type/:name` with no query string. + * + * Only `packageId` is mapped. `SaveMetaItemOptions` also carries `ifMatch`, + * `force` and `mode`; this command declares no flag for them, and adding one + * is a separate widening. + */ +export function metaRegisterOptions(flags: { + package?: string; +}): SaveMetaItemOptions | undefined { + const packageId = flags.package; + if (packageId === undefined) return undefined; + if (packageId.trim() === '') { + throw new Error(EMPTY_PACKAGE_SAVE_REFUSAL); + } + // Verbatim, as the SDK takes it: `packageId` → `?package=`. + return { packageId }; +} + export default class MetaRegister extends Command { static override description = 'Register metadata (create or update)'; static override examples = [ '$ os meta register object --data object-def.json', '$ os meta register plugin --data plugin-manifest.json', + '$ os meta register view --data view-def.json --package com.acme.crm', ]; static override args = { @@ -42,12 +89,26 @@ export default class MetaRegister extends Command { options: ['json', 'table', 'yaml'], default: 'table', }), + // [#23350] The package whose row the save writes, forwarded verbatim as + // `SaveMetaItemOptions.packageId` (`?package=`). It is the remedy the save + // door's `409 RESOURCE_CONFLICT` prescribes for an item two packages hold. + package: Flags.string({ + description: + 'Save into this package\'s row (a package id, e.g. com.acme.crm). Required when two or ' + + 'more packages hold the item: without it the server refuses with 409 RESOURCE_CONFLICT ' + + 'and names the packages.', + }), }; async run(): Promise { const { args, flags } = await this.parse(MetaRegister); try { + // FIRST — before the client exists and before anything can reach the + // network, as in `os meta delete`: an empty `--package` must not run a + // save the operator's command line reads as addressed to one package. + const options = metaRegisterOptions(flags); + const { client, token } = await createApiClient({ url: flags.url, token: flags.token, @@ -73,7 +134,7 @@ export default class MetaRegister extends Command { } // Register the metadata - const result = await client.meta.saveItem(args.type, name, metadata); + const result = await client.meta.saveItem(args.type, name, metadata, options); if (flags.format === 'json') { await formatOutput(result, 'json'); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 3d93926e9ec..29bd23c088a 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -825,8 +825,11 @@ function metaSaveHeaders(options?: SaveMetaItemOptions): Record * reads, made reachable from the SDK (commit cf71d73f8). * * `DELETE /meta/:type/:name` ("reset metadata item to artifact default") - * reads FOUR carriers. This bag declares THREE of them, and the fourth's - * absence is a ruling rather than an oversight: + * reads these carriers: the `If-Match` header and the `state`, `package`, + * `dropStorage` and `member` query parameters (`META_ITEM_RESET_PARAMS` in + * `@objectstack/rest` is the query half). This bag declares `ifMatch`, + * `state` and `packageId`; `dropStorage` and `member` are not members, and + * the reason for each is stated below: * * - {@link DeleteMetaItemOptions.ifMatch} — the ADR-0008 OCC pin. The door * already reads `If-Match` and threads it as `parentVersion`, and @@ -845,7 +848,7 @@ function metaSaveHeaders(options?: SaveMetaItemOptions): Record * parameter their client could not send. * * ⛔ `?dropStorage=true` is deliberately NOT a member, and adding it "for - * completeness" reverses a decision. It is the one carrier of the four that + * completeness" reverses a decision. It is the one carrier the door reads that * ADDS destructive reach — it drops the object's physical table after the * metadata row goes — no caller was measured needing it from this client, * and the door's repeated-parameter refusal exists because of that @@ -853,6 +856,11 @@ function metaSaveHeaders(options?: SaveMetaItemOptions): Record * with no measured pull is not published. A caller that needs it is a * separate, separately reviewable widening. * + * `?member=` — reset ONE member of a bundled type (#22877) — is not a + * member either, and no `meta` method on this client declares it. That + * absence records the bag as it stands, not a ruling against it: declaring + * it is a separate widening, with its own review. + * * Same bag on BOTH `deleteItem` declarations — the unscoped client and the * environment-scoped twin — for the reason #7019 states: the scoped mount is * not a second implementation, it is one `registerForBase` call replayed From 7ec2fb6c717c2095b2645eb234d1d3e0d8ef3845 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 11:17:29 +0000 Subject: [PATCH 2/2] chore(changeset): @objectstack/cli minor, @objectstack/client patch for the --package flags Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude --- .changeset/23350-cli-meta-package-flag.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .changeset/23350-cli-meta-package-flag.md diff --git a/.changeset/23350-cli-meta-package-flag.md b/.changeset/23350-cli-meta-package-flag.md new file mode 100644 index 00000000000..1b65b624483 --- /dev/null +++ b/.changeset/23350-cli-meta-package-flag.md @@ -0,0 +1,21 @@ +--- +'@objectstack/cli': minor +'@objectstack/client': patch +--- + +feat(cli): `os meta delete` and `os meta register` take `--package `, so a CLI user can follow the `409 RESOURCE_CONFLICT` remedy the metadata doors prescribe for an item two packages hold (#23350) + +Clause-②: yes (widening) + +**What is new.** Both commands declare an optional `--package` flag, a package id such as `com.acme.crm`, and forward it verbatim as the SDK's `packageId`, sent as `?package=`: + +```bash +os meta delete view shared_grid --package com.acme.crm # removes com.acme.crm's row of the item, no other package's row +os meta register view --data view-def.json --package com.acme.crm # writes com.acme.crm's row of the item +``` + +**Why.** A metadata item can hold one row per package (ADR-0048). When two or more packages hold an item, the reset door `DELETE /api/v1/meta/:type/:name` and the save door `PUT /api/v1/meta/:type/:name` both refuse a request that names no package, with `409 RESOURCE_CONFLICT`. The refusal names the packages and prescribes `?package=`. Neither command could send it, so a CLI user was told to do something the CLI could not do. + +**Unchanged.** Without `--package`, both commands send the same request as before: `os meta delete` adds no query string unless `--draft` is given, and `os meta register` saves active with no query string. `--package` combines with `--draft` and `--if-match` on `os meta delete`. An empty `--package` (for example `--package "$PKG"` with `PKG` unset) is refused before anything is sent, as an empty `--if-match` already is, because a package-less request is not always refused: the server may resolve it to a row the operator did not mean. + +**`@objectstack/client` (patch).** The `DeleteMetaItemOptions` docblock now names the reset door's carriers (`If-Match`, `state`, `package`, `dropStorage`, `member`) instead of stating a count that had gone stale. No type or behavior changes.