Skip to content

Commit 82bd7e0

Browse files
committed
fix(cli): S1 merge round — the lock is asked before any write of a resolution; the follow-up door save sends the row as written; history duplicates are named (C7b S1)
- The door's lock verdict is asked once, before the history fold, the promotion and the in-place write: a refusal leaves nothing written. Pinned under a wall with a history table. - The door save after the in-place write sends the row as written, its stored lock family kept, so a rerun meets the same body. - history-renumber's refusal names the table, each item holding the duplicated number and the remedy; both messages pinned. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH
1 parent 97408a0 commit 82bd7e0

2 files changed

Lines changed: 95 additions & 22 deletions

File tree

‎packages/cli/src/utils/organization-ownership-apply.test.ts‎

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,12 +1365,54 @@ describe('ADR-0131 D10 --apply — a customization conflict resolved by --metada
13651365
});
13661366
expect(asked).toEqual([['view', 'deal_list', null]]);
13671367
expect(step(report).status).toBe('blocked');
1368-
expect(step(report).blockers.join('\n')).toMatch(/is locked \(_lock=full: shipped locked\).*Nothing was deleted/s);
1368+
expect(step(report).blockers.join('\n')).toMatch(/is locked \(_lock=full: shipped locked\).*Nothing was written or deleted/s);
13691369
expect(labelOf('md_env')).toBe('Env list');
13701370
expect(deletes()).toEqual([]);
13711371
});
13721372
});
13731373

1374+
it('⭐ the lock is asked before the history fold and the promotion: under a wall, a refusal leaves history and rows as they were', async () => {
1375+
metadataTable([
1376+
md('md_org', 'org_default', 'view', 'deal_list', { name: 'deal_list', label: 'Org list' }),
1377+
md('md_env', null, 'view', 'deal_list', { name: 'deal_list', label: 'Env list' }),
1378+
md('md_lone', 'org_default', 'dashboard', 'sales', { name: 'sales' }),
1379+
].join(', '));
1380+
db.exec(`
1381+
CREATE TABLE sys_metadata_history (id TEXT PRIMARY KEY, organization_id TEXT, type TEXT, name TEXT,
1382+
version INTEGER, event_seq INTEGER, checksum TEXT);
1383+
INSERT INTO sys_metadata_history VALUES
1384+
('h_env', NULL, 'view', 'deal_list', 1, 1, 'e1'),
1385+
('h_org1', 'org_default', 'view', 'deal_list', 1, 1, 'o1'), ('h_org2', 'org_default', 'view', 'deal_list', 2, 2, 'o2');
1386+
`);
1387+
const history = rows('SELECT * FROM sys_metadata_history ORDER BY id');
1388+
const report = await apply(storingDoor(catalog()), ledger(), {
1389+
posture: 'isolated',
1390+
metadataChoices: ['md_org'],
1391+
columnDrops: { drop: async (columns) => columns.map(({ table, column }) => ({ table, column, outcome: 'not-proposed' as const })) },
1392+
lock: { verdict: async (type) => (type === 'view' ? { editable: false, lock: 'full' } : { editable: true }) },
1393+
});
1394+
expect(step(report).status).toBe('blocked');
1395+
expect(step(report).blockers.join('\n')).toMatch(/view deal_list could not take the body .*Nothing was written or deleted/s);
1396+
// No fold, no promotion, no write: history and both rows of the key are as they were.
1397+
expect(writes.filter((w) => /sys_metadata/.test(w))).toEqual([]);
1398+
expect(rows('SELECT * FROM sys_metadata_history ORDER BY id')).toEqual(history);
1399+
expect(owners()).toEqual([['md_env', null], ['md_lone', 'org_default'], ['md_org', 'org_default']]);
1400+
expect(labelOf('md_env')).toBe('Env list');
1401+
});
1402+
1403+
it('the door save after the in-place write sends the row as written, its stored lock family kept', async () => {
1404+
metadataTable([
1405+
md('md_org', 'org_default', 'view', 'crm_lead.intake', formView(true, 'Org copy')),
1406+
md('md_other', 'org_b', 'view', 'crm_lead.intake', formView(false, 'Other copy')),
1407+
md('md_env', null, 'view', 'crm_lead.intake', { ...formView(true, 'Env copy'), _lock: 'customize' }),
1408+
].join(', '));
1409+
const c = storingDoor(catalog());
1410+
const report = await apply(c, ledger(), { views: servedViews(c, []), metadataChoices: ['md_org'] });
1411+
expect(step(report).status).toBe('done');
1412+
expect(c.viewSaves.at(-1)!.body).toMatchObject({ label: 'Org copy', _lock: 'customize' });
1413+
expect((c.viewSaves.at(-1)!.body.config as any).sharing.enabled).toBe(false);
1414+
});
1415+
13741416
it('two conflicts of one item that differ only by bundle member are two conflicts, each chosen for', async () => {
13751417
db.exec(`
13761418
CREATE TABLE sys_metadata (id TEXT PRIMARY KEY, organization_id TEXT, type TEXT, name TEXT, state TEXT,
@@ -1775,7 +1817,8 @@ describe('ADR-0131 D10 --apply — history renumbering (seat answers 6162538253,
17751817
// environment scope (NULL is distinct), and no layout can move them.
17761818
viewWithHistory(
17771819
`('md_env', NULL, 'object', 'deal', 'active', NULL, NULL, 1, '{"name":"deal"}')`,
1778-
`('h_e1', NULL, 'object', 'deal', 1, 7, 'e1'), ('h_e2', NULL, 'object', 'deal', 2, 7, 'e2')`,
1820+
`('h_e1', NULL, 'object', 'deal', 1, 7, 'e1'), ('h_e2', NULL, 'object', 'deal', 2, 7, 'e2'),
1821+
('h_v1', NULL, 'object', 'account', 3, 8, 'v1'), ('h_v2', NULL, 'object', 'account', 3, 9, 'v2')`,
17791822
);
17801823
const drops: string[][] = [];
17811824
const report = await apply(catalog(), ledger(), {
@@ -1788,7 +1831,16 @@ describe('ADR-0131 D10 --apply — history renumbering (seat answers 6162538253,
17881831
});
17891832
const renumber = report.steps.find((s) => s.step === 'history-renumber')!;
17901833
expect(renumber.status).toBe('blocked');
1791-
expect(renumber.blockers.join('\n')).toMatch(/event_seq is not unique environment-wide: 7 \(2 rows\)/);
1834+
// The refusal names the table, each item holding the number, and the remedy.
1835+
expect(renumber.blockers).toContainEqual(
1836+
'sys_metadata_history: event_seq is not unique environment-wide — event_seq 7: object deal (row h_e1), object deal ' +
1837+
'(row h_e2). The ceremony never renumbers history a database numbered twice: the organization_id column is not ' +
1838+
'dropped until every event has its own number. Remedy: give each of these rows a number no other row holds (or ' +
1839+
'remove the duplicate), then re-run --apply.',
1840+
);
1841+
expect(renumber.blockers).toContainEqual(expect.stringMatching(
1842+
/^sys_metadata_history: a version of an item is held more than once — object account version 3 \(2 rows\)\. .*Remedy: give each of these rows a version no other row of the item holds/,
1843+
));
17921844
expect(report.steps.find((s) => s.step === 'column-drops')!.status).toBe('not-reached');
17931845
expect(drops).toEqual([]);
17941846
});

‎packages/cli/src/utils/organization-ownership-apply.ts‎

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,12 +1049,24 @@ async function historyDuplicates(run: Run): Promise<string[]> {
10491049
const out: string[] = [];
10501050
if (hcols.has('event_seq')) {
10511051
const seq = sql.q('event_seq', HISTORY);
1052-
const dup = await sql.read(HISTORY, `SELECT ${seq} AS s, COUNT(*) AS n FROM ${h} GROUP BY ${seq} HAVING COUNT(*) > 1 ORDER BY ${seq}`);
1052+
const dup = await sql.read(HISTORY, `SELECT ${seq} AS s FROM ${h} GROUP BY ${seq} HAVING COUNT(*) > 1 ORDER BY ${seq}`);
10531053
if (dup.length) {
1054+
const shown = dup.slice(0, 10).map((d) => d.s);
1055+
const [ty, nm] = ['type', 'name'].map((c) => sql.q(c, HISTORY));
1056+
const holders = await sql.read(
1057+
HISTORY,
1058+
`SELECT ${seq} AS s, ${sql.q('id', HISTORY)} AS id, ${ty} AS t, ${nm} AS n FROM ${h} ` +
1059+
`WHERE ${seq} IN (${shown.map(() => '?').join(', ')}) ORDER BY ${seq}, ${sql.q('id', HISTORY)}`,
1060+
shown,
1061+
);
1062+
const byNumber = shown.map(
1063+
(n) => `event_seq ${String(n)}: ${holders.filter((r) => String(r.s) === String(n)).map((r) => `${String(r.t)} ${String(r.n)} (row ${String(r.id)})`).join(', ')}`,
1064+
);
10541065
out.push(
1055-
`${HISTORY}.event_seq is not unique environment-wide: ${dup.slice(0, 10).map((d) => `${String(d.s)} (${String(d.n)} rows)`).join(', ')}` +
1056-
`${dup.length > 10 ? `, and ${dup.length - 10} more` : ''}. The column is not dropped until every event has its own ` +
1057-
'number. Renumber or remove the duplicated events, then re-run --apply.',
1066+
`${HISTORY}: event_seq is not unique environment-wide — ${byNumber.join('; ')}` +
1067+
`${dup.length > 10 ? `; and ${dup.length - 10} more number(s)` : ''}. The ceremony never renumbers history ` +
1068+
'a database numbered twice: the organization_id column is not dropped until every event has its own number. ' +
1069+
'Remedy: give each of these rows a number no other row holds (or remove the duplicate), then re-run --apply.',
10581070
);
10591071
}
10601072
}
@@ -1066,10 +1078,11 @@ async function historyDuplicates(run: Run): Promise<string[]> {
10661078
);
10671079
if (dup.length) {
10681080
out.push(
1069-
`${HISTORY} holds one version of an item more than once: ` +
1070-
`${dup.slice(0, 10).map((d) => `${String(d.t)} ${String(d.n)} v${String(d.v)} (${String(d.c)} rows)`).join(', ')}` +
1071-
`${dup.length > 10 ? `, and ${dup.length - 10} more` : ''}. The column is not dropped until every version names one ` +
1072-
'snapshot. Renumber or remove the duplicated versions, then re-run --apply.',
1081+
`${HISTORY}: a version of an item is held more than once — ` +
1082+
`${dup.slice(0, 10).map((d) => `${String(d.t)} ${String(d.n)} version ${String(d.v)} (${String(d.c)} rows)`).join('; ')}` +
1083+
`${dup.length > 10 ? `; and ${dup.length - 10} more` : ''}. The ceremony never renumbers history a database ` +
1084+
'numbered twice: the organization_id column is not dropped until every version names one snapshot. Remedy: give ' +
1085+
'each of these rows a version no other row of the item holds (or remove the duplicate), then re-run --apply.',
10731086
);
10741087
}
10751088
}
@@ -1365,8 +1378,8 @@ async function environmentRowOf(run: Run, conflict: MetadataConflict): Promise<A
13651378

13661379
/**
13671380
* One `sys_metadata` row takes a body in place, by ONE statement, doing to the
1368-
* row what the metadata door does on a save: the door's lock gate first (a
1369-
* locked item stays locked: the write is refused, nothing written); the row's
1381+
* row what the metadata door does on a save (its caller has asked the door's
1382+
* lock gate first, before any write of the resolution); the row's
13701383
* stored `_lock` family kept (the door never sets, changes or removes it,
13711384
* ADR-0010 §3.5), whatever the body carries; and `checksum` restamped from the
13721385
* written body (`hashSpec`, the door's own), which the repository serves as
@@ -1380,14 +1393,6 @@ async function writeBody(
13801393
address: { type: string; name: string; packageId: string | null },
13811394
version?: number,
13821395
): Promise<Record<string, unknown>> {
1383-
const verdict = await run.opts.lock.verdict(address.type, address.name, address.packageId);
1384-
if (!verdict.editable) {
1385-
throw new Error(
1386-
`the ${address.type} ${address.name} is locked (_lock=${verdict.lock ?? 'unknown'}` +
1387-
`${verdict.lockReason ? `: ${verdict.lockReason}` : ''}), so the metadata door would refuse this write, and the ` +
1388-
'ceremony does not make it',
1389-
);
1390-
}
13911396
const written: Record<string, unknown> = Object.fromEntries(Object.entries(body).filter(([k]) => !k.startsWith('_lock')));
13921397
for (const [k, v] of Object.entries(row.stored ?? {})) if (k.startsWith('_lock')) written[k] = v;
13931398
const sql = run.sql;
@@ -1484,6 +1489,21 @@ async function resolveMetadataConflict(run: Run, r: MetadataResolution, notes: s
14841489
}
14851490
const env = environment[0];
14861491
const address = { type: conflict.type, name: conflict.name, packageId: conflict.packageId };
1492+
// The door's lock gate, asked ONCE and before any write of this resolution
1493+
// (the history fold, the promotion, the in-place write): a locked item stays
1494+
// locked, and a refusal leaves nothing written.
1495+
const writes = env ? !(env.body && sameBody(env.body, body)) : chosen.organizationId !== null;
1496+
if (writes) {
1497+
const verdict = await run.opts.lock.verdict(address.type, address.name, address.packageId);
1498+
if (!verdict.editable) {
1499+
return (
1500+
`${what} could not take the body of the chosen row ${chosen.id}: the item is locked (_lock=${verdict.lock ?? 'unknown'}` +
1501+
`${verdict.lockReason ? `: ${verdict.lockReason}` : ''}), so the metadata door would refuse the write, and the ` +
1502+
'ceremony does not make it. Nothing was written or deleted; unlock the item, or choose the row the environment ' +
1503+
'already holds, then re-run.'
1504+
);
1505+
}
1506+
}
14871507
// The body the key's environment row must read back with: the chosen body, as written.
14881508
let expected: Record<string, unknown> = env?.body && sameBody(env.body, body) ? env.body : body;
14891509
try {
@@ -1502,7 +1522,8 @@ async function resolveMetadataConflict(run: Run, r: MetadataResolution, notes: s
15021522
}
15031523
expected = await writeBody(run, { id: env.id, stored: env.body }, body, address, version);
15041524
}
1505-
if (closing.length) await run.opts.door.save(conflict.type as never, conflict.name, body, { packageId: conflict.packageId });
1525+
// The row as written (its stored lock family kept), so a rerun's door save meets the same body.
1526+
if (closing.length) await run.opts.door.save(conflict.type as never, conflict.name, expected, { packageId: conflict.packageId });
15061527
} catch (error) {
15071528
return (
15081529
`${what} could not take the body of the chosen row ${chosen.id}` +

0 commit comments

Comments
 (0)