diff --git a/.changeset/23068-migrate-actor.md b/.changeset/23068-migrate-actor.md new file mode 100644 index 00000000000..ab266477d58 --- /dev/null +++ b/.changeset/23068-migrate-actor.md @@ -0,0 +1,16 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/metadata-core': patch +'@objectstack/rest': patch +'@objectstack/runtime': patch +'@objectstack/cli': patch +--- + +fix(metadata-protocol): a stored-metadata migration records the user who ran it, or nobody, never a label + +Clause-②: no + +- **What was wrong.** The stored-metadata migration pass (`os migrate meta --stored`, `POST /api/v1/meta/_migrate-stored`, the runtime dispatcher's `POST /metadata/_migrate-stored`, and `migrateStoredMetadata()` called in process) recorded a label as the writer of every row it rewrote: `'os migrate meta --stored'`, `' (POST /api/v1/meta/_migrate-stored)'`, `'POST /api/v1/meta/_migrate-stored'`, or `'migrate-stored'`. The writer lands in `sys_metadata_history.recorded_by` and in the row's `updated_by`. Both columns are references to `sys_user`, so the label pointed at no user. Nothing refused it, because metadata writes run as the system. +- **What it does now.** The pass records the id of the user who ran it, exactly as every other metadata write does. A REST or runtime call by a signed-in user with `manage_metadata` records that user's id. A run with no user records `null`, which means "system-initiated". That covers the CLI, which opens the database directly with no session, and a system caller. The pass still names itself on each history row's `source` (`migrate-stored`), as it did before. +- **What an operator sees.** History rows and rewritten rows written by a migration from this release on name the person who ran it, or nobody. They never name a command or a route. Rows a migration rewrote before this release keep the label they were given. +- **Contract text.** `MetadataItem.authoredBy`'s description now gives the same rule as `PutOptions.actor`: the acting user's id, or `null` for a system-initiated write, never a label such as `"system"` or `"cli"`. The schema accepts the same values as before. diff --git a/packages/cli/src/commands/migrate/meta.stored-actor.test.ts b/packages/cli/src/commands/migrate/meta.stored-actor.test.ts new file mode 100644 index 00000000000..f093bd22ed9 --- /dev/null +++ b/packages/cli/src/commands/migrate/meta.stored-actor.test.ts @@ -0,0 +1,105 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os migrate meta --stored` — who a rewritten row records (#23068). + * + * The command calls `migrateStoredMetadata`, whose `actor` becomes + * `PutOptions.actor`: it lands in `sys_metadata_history.recorded_by` and in the + * rewritten row's `updated_by`, both `lookup('sys_user')` columns. That is the + * acting user's id or nothing (#4556). This command has no user to name — it + * opens the deployment's database directly, with no session — so it passes no + * actor, and the rows record `null`, the "system-initiated" spelling. The pass + * names itself on `source: 'migrate-stored'`; the command's name is never an + * actor (it used to be: `'os migrate meta --stored'` landed in both columns as + * a reference to no user). + * + * The boot is replaced, so no database is opened; the command's own parse and + * control flow run for real, up to and including the protocol call. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import MigrateMeta from './meta.js'; + +const boot = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: boot })); +vi.mock('../../utils/migrate-occupancy-gate.js', () => ({ + OCCUPANCY_HINT: 'occupancy hint', + probeMigrationTarget: vi.fn(async () => ({ status: 'free' })), +})); +vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: vi.fn(async () => []) })); + +const HERE = dirname(fileURLToPath(import.meta.url)); +const CLI_ROOT = resolve(HERE, '..', '..', '..'); +/** oclif builds its whole command table on the first `run()` in a process. */ +const RUN_TIMEOUT = 60_000; + +/** A clean report: the command exits 0 and never calls `this.exit()`. */ +const CLEAN_REPORT = { + apply: true, + protocol: '17.0.0', + scanned: 1, + canonical: 0, + pending: 0, + rewritten: 1, + skipped: 0, + failed: 0, + rows: [], + decisionModeReview: [], +}; + +const migrateStoredMetadata = vi.fn(); +let stdout: ReturnType; +let stderr: ReturnType; +let log: ReturnType; + +beforeEach(() => { + migrateStoredMetadata.mockReset(); + migrateStoredMetadata.mockImplementation(async (req: { apply?: boolean }) => ({ + ...CLEAN_REPORT, + apply: req.apply === true, + rewritten: req.apply === true ? 1 : 0, + canonical: req.apply === true ? 0 : 1, + })); + boot.mockReset(); + boot.mockResolvedValue({ + kernel: { getService: (name: string) => (name === 'protocol' ? { migrateStoredMetadata } : undefined) }, + dbLabel: 'file:test.db', + tableAbsent: () => false, + shutdown: vi.fn(async () => {}), + }); + // `emitJson` awaits the write's DRAIN callback, so the double invokes it. + stdout = vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown, enc?: unknown, cb?: unknown) => { + const done = typeof enc === 'function' ? enc : cb; + if (typeof done === 'function') done(); + return true; + }) as typeof process.stdout.write); + stderr = vi.spyOn(process.stderr, 'write').mockImplementation((() => true) as typeof process.stderr.write); + log = vi.spyOn(console, 'log').mockImplementation(() => {}); +}); +afterEach(() => { + stdout.mockRestore(); + stderr.mockRestore(); + log.mockRestore(); +}); + +describe('os migrate meta --stored — the rewrite records nobody, never the command (#23068)', () => { + it('--apply hands the pass no actor, so recorded_by and updated_by store null', async () => { + await MigrateMeta.run(['--stored', '--apply', '--yes', '--json'], { root: CLI_ROOT }); + + expect(migrateStoredMetadata).toHaveBeenCalledTimes(1); + const request = migrateStoredMetadata.mock.calls[0][0]; + expect(request.apply).toBe(true); + expect(request).not.toHaveProperty('actor'); + }, RUN_TIMEOUT); + + it('the preview hands it no actor either', async () => { + await MigrateMeta.run(['--stored', '--json'], { root: CLI_ROOT }); + + expect(migrateStoredMetadata).toHaveBeenCalledTimes(1); + const request = migrateStoredMetadata.mock.calls[0][0]; + expect(request.apply).toBe(false); + expect(request).not.toHaveProperty('actor'); + }, RUN_TIMEOUT); +}); diff --git a/packages/cli/src/commands/migrate/meta.stored-flow-resolution.integration.test.ts b/packages/cli/src/commands/migrate/meta.stored-flow-resolution.integration.test.ts index ede7c30417d..f85252ee504 100644 --- a/packages/cli/src/commands/migrate/meta.stored-flow-resolution.integration.test.ts +++ b/packages/cli/src/commands/migrate/meta.stored-flow-resolution.integration.test.ts @@ -110,11 +110,11 @@ describe('os migrate meta --stored — the protocol resolves the engine itself ( const protocol: any = stack.kernel.getService('protocol'); - // The command's exact call since #4498 — no `canonicalizeFlow`. + // The command's exact call since #4498 — no `canonicalizeFlow` — and, + // since #23068, no `actor` either (the command has no user to name). const report = await protocol.migrateStoredMetadata({ apply: true, types: ['flow'], - actor: 'os migrate meta --stored', }); // Before the resolver this row came back `skipped` with "no automation @@ -195,7 +195,8 @@ describe('os migrate meta --stored — the protocol resolves the engine itself ( name: 'sfs_purge', item: { ...item, label: 'Purge Stale Leads (edited)' }, parentVersion: null, - actor: 'studio-roundtrip-probe', + // A user id, as a Studio save passes one — never a label (#23068). + actor: 'usr_studio_probe', }); const [row] = await ql.find('sys_metadata', { diff --git a/packages/cli/src/commands/migrate/meta.ts b/packages/cli/src/commands/migrate/meta.ts index f026976c08c..85d0272451b 100644 --- a/packages/cli/src/commands/migrate/meta.ts +++ b/packages/cli/src/commands/migrate/meta.ts @@ -1555,7 +1555,12 @@ export default class MigrateMeta extends Command { : await protocol.migrateStoredMetadata({ apply, ...(flags.type && flags.type.length > 0 ? { types: flags.type } : {}), - actor: 'os migrate meta --stored', + // [#23068] No `actor`: this command opens the database directly, + // with no session, so it has no user id to give — and the actor + // lands in `recorded_by` / `updated_by`, `lookup('sys_user')` + // columns, where only a user id or `null` belongs. Omitted, the + // rows record `null` ("system-initiated", #4556); the pass names + // itself on `source: 'migrate-stored'`. }); const clean = storedMigrationClean(report); if (!clean) exitCode = 1; diff --git a/packages/metadata-core/src/types.ts b/packages/metadata-core/src/types.ts index 68342c47bae..4f88e04aa6a 100644 --- a/packages/metadata-core/src/types.ts +++ b/packages/metadata-core/src/types.ts @@ -99,7 +99,7 @@ export const MetadataItemSchema = z.object({ body: z.record(z.string(), z.unknown()).describe('Canonical Zod-normalised spec'), hash: z.string().regex(/^sha256:[0-9a-f]{64}$/).describe('sha256(canonicalize(body, ref.type))'), parentHash: z.string().nullable().describe('Hash this version was derived from; null for first version'), - authoredBy: z.string().nullable().describe('Identity of the writer (user id, "cli", "ai:claude", …); null = system-initiated, no actor'), + authoredBy: z.string().nullable().describe('Identity of the writer: the acting user id, as PutOptions.actor records it; null = system-initiated, no actor — never a label such as "system" or "cli"'), authoredAt: z.string().describe('ISO-8601 timestamp'), message: z.string().optional().describe('Optional commit message'), seq: z.number().int().nonnegative().describe('Sequence number this write produced in the org log'), diff --git a/packages/metadata-protocol/src/protocol.stored-migration.test.ts b/packages/metadata-protocol/src/protocol.stored-migration.test.ts index 7e0563798a4..54230ae504b 100644 --- a/packages/metadata-protocol/src/protocol.stored-migration.test.ts +++ b/packages/metadata-protocol/src/protocol.stored-migration.test.ts @@ -330,6 +330,41 @@ describe('migrateStoredMetadata — apply (#4327)', () => { }); }); +/** + * [#23068] Who a rewritten row records. The pass's `actor` becomes + * `PutOptions.actor`, which lands in `sys_metadata_history.recorded_by` and in + * the row's own `updated_by` — both `lookup('sys_user')` columns. So it is the + * acting user's id, or nothing (`null`, #4556's "system-initiated"). The pass + * names ITSELF on `source: 'migrate-stored'`, the column that exists for an + * origin label, and never in the actor: a label there is a reference to no + * user, and no engine check stops it (`put` writes under `isSystem`). + */ +describe('migrateStoredMetadata — a rewrite records the user who ran it, or nobody (#23068)', () => { + it('a pass run by a user records that user id in recorded_by and in the row\'s updated_by', async () => { + const { engine, tables } = makeStubEngine([legacyObjectRow]); + const protocol = new ObjectStackProtocolImplementation(engine); + + const report = await protocol.migrateStoredMetadata({ apply: true, actor: 'usr_operator' }); + + expect(report.rewritten).toBe(1); + expect(historyRows(tables).map((h) => h.recorded_by)).toEqual(['usr_operator']); + expect(metaRows(tables).map((r) => r.updated_by)).toEqual(['usr_operator']); + }); + + it('a pass with no user records null in both columns — no label reaches either one', async () => { + const { engine, tables } = makeStubEngine([legacyObjectRow, legacyActionRow]); + const protocol = new ObjectStackProtocolImplementation(engine); + + const report = await protocol.migrateStoredMetadata({ apply: true }); + + expect(report.rewritten).toBe(2); + expect(historyRows(tables).map((h) => h.recorded_by)).toEqual([null, null]); + expect(metaRows(tables).map((r) => r.updated_by)).toEqual([null, null]); + // The pass's own name is still on every history row, where it belongs. + expect(historyRows(tables).map((h) => h.source)).toEqual(['migrate-stored', 'migrate-stored']); + }); +}); + describe('migrateStoredMetadata — flow rows via the canonicalizeFlow hook (#4454)', () => { // A body the write path's schema gate accepts — the hook canonicalizes the // shape, it does not exempt the row from validation. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 2abdde20037..799733c6b23 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -21389,7 +21389,9 @@ export class ObjectStackProtocolImplementation implements * are about to write (the same conversion). It is empty by construction, * and there is no author here for a confirmation prompt to reach. * - `source: 'migrate-stored'` — so a history diff distinguishes a - * canonicalization pass from an edit someone made. + * canonicalization pass from an edit someone made. This is the ONLY + * place the pass names itself: `actor` (below) is the user who ran it + * or nobody, never a label (#23068). * * ## What it lists and never writes * @@ -21483,7 +21485,16 @@ export class ObjectStackProtocolImplementation implements apply?: boolean; /** Restrict to these metadata types (singular or plural spelling). */ types?: string[]; - /** Recorded as the writer on the history + audit rows. */ + /** + * The id of the user running the pass — omit it when no user is + * (a CLI run against the database, an `isSystem` caller). It becomes + * each rewrite's `PutOptions.actor`, so it lands in + * `sys_metadata_history.recorded_by` and the row's `updated_by`, both + * `lookup('sys_user')`: omitted, they record `null`, #4556's + * "system-initiated". ⛔ Never a label naming the door or the pass — + * that is a reference to no user, and `source: 'migrate-stored'` + * already says where the write came from (#23068). + */ actor?: string; /** * Canonicalize a stored `flow` body (#4454). **Optional override** — @@ -21815,7 +21826,10 @@ export class ObjectStackProtocolImplementation implements packageId, force: true, source: 'migrate-stored', - actor: request.actor ?? 'migrate-stored', + // [#23068] The user who ran the pass, or none — and none + // reaches `PutOptions.actor` as `null` (`saveMetaItem`'s + // #4556 default). The pass's name is on `source` above. + ...(request.actor ? { actor: request.actor } : {}), }); record({ ...base, notices: flattened, todos: flattenedTodos, outcome: 'rewritten' }); } catch (e: any) { diff --git a/packages/rest/src/rest-meta-migrate-stored.test.ts b/packages/rest/src/rest-meta-migrate-stored.test.ts index 07533941886..bdbb984b717 100644 --- a/packages/rest/src/rest-meta-migrate-stored.test.ts +++ b/packages/rest/src/rest-meta-migrate-stored.test.ts @@ -174,10 +174,21 @@ describe('POST /meta/_migrate-stored — preview by default', () => { expect(migrateStoredMetadata.mock.calls[0][0]).not.toHaveProperty('canonicalizeFlow'); }); - it('attributes the run to the caller — history and audit rows answer "who ran it"', async () => { + // [#23068] The actor lands in `sys_metadata_history.recorded_by` and the + // rewritten row's `updated_by`, both `lookup('sys_user')`: the caller's user + // id exactly, or nothing — never the route's name, which the pass already + // records on `source: 'migrate-stored'`. + it('attributes the run to the caller\'s user id, and to nothing else — history rows answer "who ran it"', async () => { const { route, migrateStoredMetadata } = boot(admin); await run(route, {}); - expect(migrateStoredMetadata.mock.calls[0][0].actor).toContain('admin'); + expect(migrateStoredMetadata.mock.calls[0][0].actor).toBe('admin'); + }); + + it('a caller with no user (isSystem) runs it as nobody — no actor, never a label', async () => { + const { route, migrateStoredMetadata } = boot({ isSystem: true }); + const out = await run(route, { apply: true }); + expect(out.getStatus()).toBe(200); + expect(migrateStoredMetadata.mock.calls[0][0]).not.toHaveProperty('actor'); }); it('returns the report unwrapped', async () => { diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 42e0bc79bc1..712ca3eb781 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -2849,7 +2849,8 @@ export class RestServer { /** * [#7749] The acting identity recorded on a metadata WRITE — the single * producer for every `/meta` route that stamps an `actor` (save, delete, - * publish, rollback, compound save). + * publish, rollback, compound save, and — since #23068, which retired the + * route-name label it stamped inline — `POST /_migrate-stored`). * * ## What was wrong * @@ -6002,15 +6003,19 @@ export class RestServer { // Preview by default — `apply` must be explicitly true, // the same posture the CLI takes. A caller who sends an // empty body gets a report and no writes. + // Attributed to the caller: this writes history rows, + // and "who ran the migration" is the question they + // exist to answer. [#23068] By user id alone — the + // identity this request was authorized as, read by the + // producer every `/meta` write door shares — or by + // nobody. It lands in `recorded_by` / `updated_by`, + // `lookup('sys_user')` columns, so ⛔ never the route's + // name: the pass records its origin on `source`. + const actor = await this.resolveMetaWriteActor(environmentId, req); const report = await (p as any).migrateStoredMetadata({ apply: (req.body as any)?.apply === true, ...(types.length > 0 ? { types } : {}), - // Attributed to the caller: this writes history + - // audit rows, and "who ran the migration" is the - // question those rows exist to answer. - actor: ctx?.userId - ? `${ctx.userId} (POST ${metadata.prefix}/_migrate-stored)` - : `POST ${metadata.prefix}/_migrate-stored`, + ...(actor ? { actor } : {}), }); res.json(report); } catch (error: any) { diff --git a/packages/runtime/src/domains/meta-migrate-stored.test.ts b/packages/runtime/src/domains/meta-migrate-stored.test.ts index 1b7a4f55157..82801626ff8 100644 --- a/packages/runtime/src/domains/meta-migrate-stored.test.ts +++ b/packages/runtime/src/domains/meta-migrate-stored.test.ts @@ -132,10 +132,23 @@ describe('POST /meta/_migrate-stored — preview by default (#4327)', () => { expect(migrateStoredMetadata.mock.calls[0][0]).not.toHaveProperty('canonicalizeFlow'); }); - it('attributes the run to the caller — history and audit rows answer "who ran it"', async () => { + // [#23068] The actor lands in `sys_metadata_history.recorded_by` and the + // rewritten row's `updated_by`, both `lookup('sys_user')`: the caller's user + // id exactly, or nothing — never the route's name, which the pass already + // records on `source: 'migrate-stored'`. + it('attributes the run to the caller\'s user id, and to nothing else — history rows answer "who ran it"', async () => { const migrateStoredMetadata = vi.fn().mockResolvedValue(REPORT); await make({ migrateStoredMetadata }).handleMetadata('/_migrate-stored', admin(), 'POST', {}); - expect(migrateStoredMetadata.mock.calls[0][0].actor).toContain('admin'); + expect(migrateStoredMetadata.mock.calls[0][0].actor).toBe('admin'); + }); + + it('a caller with no user (isSystem) runs it as nobody — no actor, never a label', async () => { + const migrateStoredMetadata = vi.fn().mockResolvedValue(REPORT); + const res = await make({ migrateStoredMetadata }).handleMetadata( + '/_migrate-stored', ctx({ isSystem: true }), 'POST', { apply: true }, + ); + expect(res.response?.status).toBe(200); + expect(migrateStoredMetadata.mock.calls[0][0]).not.toHaveProperty('actor'); }); it('returns the report as-is', async () => { diff --git a/packages/runtime/src/domains/meta.ts b/packages/runtime/src/domains/meta.ts index ccf31fe3c4e..06c805fe043 100644 --- a/packages/runtime/src/domains/meta.ts +++ b/packages/runtime/src/domains/meta.ts @@ -1832,14 +1832,18 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin const types = Array.isArray(body?.types) ? body.types.filter((t: unknown): t is string => typeof t === 'string' && t.length > 0) : undefined; + // Attributed to the caller, not to the route: this writes history + // rows, and "who ran the migration" is the question they exist to + // answer. [#23068] By the caller's user id alone, or by nobody (an + // `isSystem` caller): it lands in `recorded_by` / `updated_by`, + // `lookup('sys_user')` columns, so ⛔ never the route's name — the + // pass records its origin on `source: 'migrate-stored'`. + const actor = typeof ec?.userId === 'string' && ec.userId ? ec.userId : undefined; try { const report = await protocol.migrateStoredMetadata({ apply: body?.apply === true, ...(types && types.length > 0 ? { types } : {}), - // Attributed to the caller, not to the route: this writes - // history + audit rows, and "who ran the migration" is the - // question those rows exist to answer. - actor: ec?.userId ? `${ec.userId} (POST /metadata/_migrate-stored)` : 'POST /metadata/_migrate-stored', + ...(actor ? { actor } : {}), }); return { handled: true, response: deps.success(report) }; } catch (e: any) {