Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/23068-migrate-actor.md
Original file line number Diff line number Diff line change
@@ -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'`, `'<user id> (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.
105 changes: 105 additions & 0 deletions packages/cli/src/commands/migrate/meta.stored-actor.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof vi.spyOn>;
let stderr: ReturnType<typeof vi.spyOn>;
let log: ReturnType<typeof vi.spyOn>;

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);
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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', {
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/src/commands/migrate/meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/metadata-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
35 changes: 35 additions & 0 deletions packages/metadata-protocol/src/protocol.stored-migration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
20 changes: 17 additions & 3 deletions packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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** —
Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 13 additions & 2 deletions packages/rest/src/rest-meta-migrate-stored.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
19 changes: 12 additions & 7 deletions packages/rest/src/rest-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 15 additions & 2 deletions packages/runtime/src/domains/meta-migrate-stored.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading
Loading