Skip to content

Commit 216b18a

Browse files
fix(cli): the ownership ceremony re-runs its not-null step on a resumed apply, so tables it skipped receive NOT NULL (#31409)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X2kM3DQTy5XByDHKN47ZrVA)_ domain:server#5 · os-project-manager · session_01FEkQ5mbAyTfp7gbjowRA48 Fixes #31374 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:所有权仪式(`os migrate organization-ownership --apply`)续跑时,加 NOT NULL 那一步不再因为「已打过检查点」直接跳过,而是重新评估一遍:当时因组合没声明(notDeclared)或因遗留 NULL 行被挡住的表,这次能加就加上;已经是 NOT NULL 的列从数据库目录读出来,报 `already-not-null`,不会再重建一次表。 - 为什么改:原来这一步跳过表后照样记为完成,换成声明了这些表的组合重跑、或围墙部署「apply → 修数据 → plan → apply」恢复后,数据库照样认证通过,但 `organization_id` 仍可为空;这一步自己的提示「重跑即可补上」实际做不到(#31374 的 A,p1,cloud ee-v4.3.1 依赖这条恢复路径)。 - 风险与回滚:只动 `@objectstack/cli` 的仪式代码;首次运行行为不变,续跑且无事可做时仍显示 `checkpointed`。续跑时加 NOT NULL 失败会挡住认证并撤掉这一步的检查点,与首次运行失败一致。回滚即还原本 PR。B(`composeServedPlatform` 不跟随 `serve` 的 AI 自动注入)不在本 PR,作为待决问题交席位。 - 先看:`packages/cli/src/utils/organization-ownership-apply.ts` 续跑循环里新增的 `not-null` 分支;`packages/cli/src/utils/organization-ownership-not-null.ts` 先读 NOT NULL 再判声明的顺序;`packages/cli/src/commands/migrate/organization-ownership.ts` 的目录读缓存。 - 两分钟验证:`pnpm --filter @objectstack/cli exec vitest run --project integration src/utils/organization-ownership-apply.not-null-resume.test.ts`,预期 4 个用例全过(在 main 上 3 个红)。 - 需要你做的:无,席位落地。 ## What changes `os migrate organization-ownership --apply` checkpoints its `not-null` step (18.x step 5d, ADR-0131 8e) when the step finishes, also when it left tables nullable for one of two reasons: - the composition the run booted declares no object for the table (`not-declared`). The step's note says "re-run the ceremony where the deployment's composition is declared to apply it"; - the table still holds a NULL row the operator must resolve by hand. `UNATTRIBUTABLE_REMEDY` says "then re-run --apply". On `main`, a later apply reads the checkpoint and skips the step. So both re-runs certify the database with `organization_id` still nullable on those tables. **Choice: re-evaluate on resume, not "leave the step un-checkpointed while anything is pending".** Leaving it un-checkpointed would make `complete` false, so a database with a `not-declared` table could no longer certify at all. Today it certifies with the table reported, and the card's own acceptance (1) certifies with N tables `not-declared`. That would change what certification means. Re-evaluating keeps the checkpoint contract unchanged: a checkpoint still means the step ran to `done` once. It follows the pattern the loop already uses for `attribution`, `mirror-deletion` and `history-renumber`, and it makes both advice lines literally true. 1. `utils/organization-ownership-apply.ts`: on a resumed apply that has a schema door, a checkpointed `not-null` step runs again. - Something applied: the step reports `done` and the checkpoint takes the new summary. - Nothing left to tighten: the step reports `checkpointed` with the recorded summary, exactly as before, and keeps the re-run's notes, so a table that is still `not-declared` keeps its advice line. - A NOT NULL fails: the step blocks and its checkpoint entry is removed, so the run does not certify. A first run does the same. - The module docblock's "Idempotent and resumable" paragraph names the step. 2. `utils/organization-ownership-not-null.ts`: the port asks `isNotNull` of every column first and answers `already-not-null` for a column that is NOT NULL already. Before this, only a rotation shard was asked. Without it, the re-run would rebuild, on SQLite, every table an earlier run tightened, on every resume. 3. `commands/migrate/organization-ownership.ts`: the command's `isNotNull` reads the column catalog once per database per port call. It used to read once per table, and every table is now asked. Each table is read before it is tightened, so the cached read is never stale for the table it answers. ## Tests New `src/utils/organization-ownership-apply.not-null-resume.test.ts` (integration tier: real `SqlDriver` on SQLite, NOT NULL through the driver's drift apply door, read back from `pragma_table_info`): - ⭐ (1) certified with 2 of 3 tables `not-declared`, re-run under a composition declaring all 3 → NOT NULL on all 3. The table the first run tightened is not rebuilt (`rebuilt` = 3 tables, not 4). - ⭐ (2) walled recovery path (`isolated` posture): apply → certification held by the NULL row → fix the row → plan (now `willReceive: true`) → apply → NOT NULL on the fixed table, certified. - (3) CONTROL: a first run with nothing pending writes the same step report and checkpoint as before. Its re-run stays `checkpointed` with the same summary and rebuilds nothing. - (4) a re-run whose NOT NULL fails blocks, does not certify, and drops the step's checkpoint entry. `src/utils/organization-ownership-not-null.test.ts`: ⭐ a declared table an earlier run tightened is `already-not-null` and is never handed to the door again. <details><summary>Red on main (base 7189fb2, test file only): 3 failed, 1 passed (the control)</summary> ``` × ⭐ (1) certified with tables not-declared, re-run under a composition that declares them: they receive NOT NULL × ⭐ (2) the walled recovery path — apply, fix the held rows, plan, apply — ends with NOT NULL on the fixed table × (4) a re-run whose NOT NULL fails blocks and does not certify, as the same failure does on a first run - "status": "done", + "status": "checkpointed", "summary": { - "alreadyNotNull": 1, - "applied": 2, + "alreadyNotNull": 0, + "applied": 1, "due": 3, - "notDeclared": 0, + "notDeclared": 2, ❯ src/utils/organization-ownership-apply.not-null-resume.test.ts:199:25 Tests 3 failed | 1 passed (4) ``` Each red is at the re-run assertion. Every first-run assertion (including certification with `notDeclared: 2`, and the held walled first run) passes on main. </details> <details><summary>Ablations (scripts/ablation-replace.mjs, on head 9bd2dbc; each restored, blob == HEAD, git diff HEAD empty)</summary> - **ABL1**, the resume branch disabled (`… && opts.notNull && String("ABL1").length < 0`): the same three pins red, `Tests 3 failed | 8 passed (11)`, with the `"status": "checkpointed"` / `"notDeclared": 2` diff above. - **ABL2**, the port's first-in-order `isNotNull` check disabled: 5 red, `Tests 5 failed | 6 passed (11)`. The shard and declared-table `already-not-null` port pins, (1), (2), and the CONTROL (3), whose re-run rebuilds again: `- "status": "checkpointed" + "status": "done"`. The test imports `src` directly (no `dist` in the resolution path), so no rebuild leg was owed. </details> ## Acceptance notes - **Dialect.** Measured on SQLite (the table rebuild). The defect and the fix are in the checkpoint loop and the port's ordering, which no dialect reaches. The PostgreSQL / MySQL `ALTER COLUMN … SET NOT NULL` path is the existing drift door, unchanged here. - **B (p2) is not in this PR.** Composing what `serve` composes needs a host import of `@objectstack/service-ai`, a cloud-only package outside this workspace (`CAPABILITY_PROVIDERS` has no `ai` entry). It also needs the Studio chain and its required/auto/off rule, which today live inline in `serve.ts`. Sharing them means a new seam in `serve.ts`, which open PR #31377 also touches, plus a fake host-package fixture. The other route, refusing the plan when `ai` is only auto-injected, narrows what `os migrate` accepts for such hosts. Per the order, B goes to the report's `open_questions` (needs_decision) instead of widening this PR. cloud mounts the AI plugin explicitly since cloud#3744, so no cloud host depends on it. - **Re-run steps that block keep their checkpoint (seen, not changed).** `attribution` and `mirror-deletion` keep their checkpoint entry when their re-run blocks, so such a run still computes `complete` from the checkpoint. This PR changes that only for `not-null`, the step it owns. - `node scripts/pm/check-governed-merges.mjs --branch HEAD`: "0 of 6 path(s) hit the register … NOT governed — ordinary queue landing applies". `dispatch-gates --tier`: no path-derived mandate. ## Verification All on head `85776d2b` (the branch merged `origin/main` at `eff90af6a` first). Every exit code was captured before any pipe. - **Derived gate union.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` listed 69 families for this diff. All 69 exited 0. `--ran`: "69 derived famil(ies) accounted for — 69 run, 0 NOT-MEASURED". The full record is in the seat's order folder (`31374-gates-ran.txt`). - Round 1, on `c4eec5e3`, found one real red: `pnpm check:test-source-alias` exit 1. The new test file reached seven dynamic imports through the plan's module graph. `85776d2b` loads them at module top. Round 2 re-ran all 69. - Round 1's `check-plugin-teardown-shape.mjs --self-test` exit 3 was a shallow-clone prerequisite. After `git fetch --depth=1 origin 621a487…` it exited 0, and it exited 0 again in round 2. - **cli unit tier**: `pnpm --filter @objectstack/cli exec vitest run --project unit`, 401/401 files. - The first run's two reds were `test/published-subpath-{console,hook-body}.pin.test.ts`: "packages/cli is not built", a prerequisite. After the build both passed, 29/29. - **cli integration, ceremony suites** (VERDICT command-exit 0): `Test Files 7 passed | 1 skipped`, `Tests 150 passed | 1 skipped`. - The suites: the new resume pins, `organization-ownership-not-null`, `organization-ownership-apply`, `…apply.generation-1`, `organization-ownership-not-declared.integration`, `organization-ownership.no-organization.integration` and `organization-ownership.declared-rule-mirrors.integration`. - The skip is `organization-ownership.remote-face.integration` (`describe.skipIf(!SQLD)`, no sqld binary here): NOT MEASURED locally, so CI's run is the reading. - The rest of the integration tier is declared to CI. - **`pnpm --filter @objectstack/cli typecheck`** (`tsc --noEmit` and `check:test-typecheck`, heap 6144): exit 0 on `85776d2b`. - **eslint** (`pnpm exec eslint --no-inline-config`) on the five touched `.ts` files: exit 0. - **Builds**, through `scripts/pm/os-verify-lock.sh`: `@objectstack/cli^...`, then `turbo run build --filter=./packages/**`, both VERDICT command-exit 0. - The families CI runs outside the derived total are read from this PR's check-runs, not re-run here. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. (eslint on the five touched files: exit 0) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. (`.changeset/31374-ownership-not-null-resume.md`, `@objectstack/cli` patch) - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. (the 维护者速读 block is the lane's required Chinese summary) - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5mbAyTfp7gbjowRA48 --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5mbAyTfp7gbjowRA48)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cd6618e commit 216b18a

6 files changed

Lines changed: 356 additions & 13 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
fix(cli): the organization-ownership ceremony's not-null step runs again on a resumed apply, so a table it left nullable receives NOT NULL once that is possible
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`os migrate organization-ownership --apply` recorded its not-null step as done even when it left tables nullable: tables the booted composition declared no object for (`notDeclared`), and tables still holding a NULL row the operator had to resolve by hand. A later apply then read the checkpoint and skipped the step. Re-running under a composition that declares those tables, or after fixing the rows (apply, fix rows, plan, apply), certified the database with `organization_id` still nullable on them, although the step's own advice said such a re-run applies it.
11+
12+
On a resumed apply that has a schema door, the step now runs again. A table that is declared and has no NULL row receives NOT NULL. A column that is already NOT NULL is answered `already-not-null`, read from the database's catalog, and is never rebuilt. A re-run with nothing left to tighten stays `checkpointed`, exactly as before. A re-run whose NOT NULL fails blocks the run and is no longer recorded as done, the same as on a first run. A first run is unchanged.

‎packages/cli/src/commands/migrate/organization-ownership.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -881,15 +881,24 @@ export default class OrganizationOwnershipCeremony extends Command {
881881
.filter((name): name is string => typeof name === 'string' && name.length > 0)
882882
.map((name) => StorageNameMapping.resolveTableName({ name })),
883883
);
884+
const nullability = new Map<string, ReturnType<typeof readColumnNullability>>();
884885
return applyNotNullThroughDrift(
885886
{
886887
isDeclared: (table) => declared.has(rotationBaseOf(table) ?? table),
887888
shardBaseOf: (table) => rotationBaseOf(table) ?? null,
888-
// [ADR-0131 D1] A shard the driver created NOT NULL is reported
889-
// so, never as one the ceremony could not tighten.
889+
// [ADR-0131 D1] A column already NOT NULL — a shard the driver
890+
// created so, or a table an earlier run tightened — is reported
891+
// so, never handed to the door again. One catalog read per
892+
// database per call: each table is read before it is tightened.
890893
isNotNull: async (table, column, database) => {
891-
const read = await readColumnNullability(driverFor(database), [column], normalizeRows);
892-
return 'detail' in read ? null : read.tables.get(table)?.get(column) ?? null;
894+
const key = `${database ?? ''}|${column}`;
895+
let read = nullability.get(key);
896+
if (!read) {
897+
read = readColumnNullability(driverFor(database), [column], normalizeRows);
898+
nullability.set(key, read);
899+
}
900+
const answer = await read;
901+
return 'detail' in answer ? null : answer.tables.get(table)?.get(column) ?? null;
893902
},
894903
driverFor: (database) => driverFor(database) as never,
895904
},
Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The ceremony's `not-null` step (18.x step 5d, ADR-0131 8e) on a run that
5+
* resumes from its checkpoint, over a REAL SQLite driver: the NOT NULL is the
6+
* driver's own drift apply door (the table rebuild), read back from the
7+
* database's own catalog.
8+
*
9+
* The step leaves a table nullable for one of two reasons it states: the
10+
* composition the run booted declares no object for it (`not-declared`, "re-run
11+
* the ceremony where the deployment's composition is declared to apply it"), or
12+
* the table still holds a NULL row the operator must resolve by hand
13+
* (`UNATTRIBUTABLE_REMEDY`, "then re-run --apply"). Either way the step itself
14+
* finished, so it is checkpointed. The re-run each advice line names must then
15+
* apply the NOT NULL it left, not read the checkpoint and skip the step:
16+
*
17+
* - (1) certified with two tables `not-declared`, re-run under a composition
18+
* that declares them: both end NOT NULL;
19+
* - (2) the walled recovery path — apply, fix the held rows by hand, plan,
20+
* apply — ends with NOT NULL on the fixed table, and certifies;
21+
* - (3) control: a first run with nothing pending checkpoints exactly as
22+
* before, and a re-run of it stays `checkpointed` and rebuilds nothing;
23+
* - (4) a re-run whose NOT NULL fails blocks and does not certify, as the
24+
* same failure does on a first run.
25+
*/
26+
27+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
28+
import Database from 'better-sqlite3';
29+
import { mkdtempSync, rmSync } from 'node:fs';
30+
import { tmpdir } from 'node:os';
31+
import { join } from 'node:path';
32+
import { SqlDriver } from '@objectstack/driver-sql';
33+
import { buildOrganizationOwnershipPlan, inventoryDigest } from './organization-ownership-plan.js';
34+
import {
35+
applyOrganizationOwnership,
36+
reviewedConflictsOf,
37+
type ApplyExecutor,
38+
type CeremonyCheckpoint,
39+
type CeremonyLedgerPort,
40+
type NotNullPort,
41+
type OrganizationOwnershipApplyReport,
42+
} from './organization-ownership-apply.js';
43+
import { applyNotNullThroughDrift } from './organization-ownership-not-null.js';
44+
45+
// The dynamic imports the plan's module graph reaches, loaded at collection,
46+
// never inside a case's clocked window (`check:test-source-alias`).
47+
import '@objectstack/driver-turso';
48+
import '@objectstack/metadata-protocol';
49+
import '@objectstack/platform-objects/plugin';
50+
import '@objectstack/plugin-security';
51+
import '@objectstack/rest';
52+
import '@objectstack/runtime';
53+
import '@objectstack/service-datasource';
54+
55+
const ORG = 'org_default';
56+
const TABLES = ['crm_deal', 'crm_note', 'crm_task'] as const;
57+
58+
let dir: string;
59+
let file: string;
60+
let db: Database.Database;
61+
let driver: SqlDriver;
62+
/** Tables the drift apply door rebuilt, in order. */
63+
let rebuilt: string[];
64+
65+
function executor(): ApplyExecutor {
66+
return {
67+
dialect: 'sqlite',
68+
client: 'better-sqlite3',
69+
async query(sql, params = []) {
70+
return db.prepare(sql).all(...(params as unknown[])) as Record<string, unknown>[];
71+
},
72+
async execute(sql, params = []) {
73+
db.prepare(sql).run(...(params as unknown[]));
74+
},
75+
};
76+
}
77+
78+
function ledger(): CeremonyLedgerPort & { stored: () => CeremonyCheckpoint | null } {
79+
let stored: CeremonyCheckpoint | null = null;
80+
return {
81+
stored: () => stored,
82+
async read() {
83+
return stored ? (JSON.parse(JSON.stringify(stored)) as CeremonyCheckpoint) : null;
84+
},
85+
async write(checkpoint) {
86+
stored = JSON.parse(JSON.stringify(checkpoint)) as CeremonyCheckpoint;
87+
},
88+
};
89+
}
90+
91+
/** Whether the table's tenant column is NOT NULL, read from the database's own catalog. */
92+
const isNotNull = (table: string): boolean =>
93+
(db.prepare(`SELECT "notnull" AS nn FROM pragma_table_info('${table}') WHERE name = 'organization_id'`).get() as { nn: number }).nn === 1;
94+
95+
/**
96+
* The command's `notNull` port, as `os migrate organization-ownership` wires
97+
* it: the drift apply door of the driver, for the tables `declared` (the
98+
* composition this run booted), the column's nullability read from the catalog.
99+
*/
100+
function schemaDoor(declared: ReadonlySet<string>, opts: { failOn?: string } = {}): NotNullPort {
101+
return {
102+
apply: (columns) =>
103+
applyNotNullThroughDrift(
104+
{
105+
isDeclared: (table) => declared.has(table),
106+
isNotNull: async (table) => isNotNull(table),
107+
driverFor: () => ({
108+
applyMigrationEntries: async (entries: never[], options: never) => {
109+
const table = (entries[0] as { table: string }).table;
110+
if (table === opts.failOn) throw new Error(`SQLITE_BUSY: ${table} is locked`);
111+
rebuilt.push(table);
112+
return driver.applyMigrationEntries(entries, options);
113+
},
114+
}) as never,
115+
},
116+
columns,
117+
),
118+
};
119+
}
120+
121+
async function apply(
122+
l: CeremonyLedgerPort,
123+
notNull: NotNullPort,
124+
posture = 'single',
125+
): Promise<OrganizationOwnershipApplyReport> {
126+
const reviewed = await buildOrganizationOwnershipPlan({ reader: executor(), posture, database: file });
127+
return applyOrganizationOwnership({
128+
executor: executor(),
129+
posture,
130+
database: file,
131+
reviewedPlan: {
132+
file: 'plan.json',
133+
generatedAt: reviewed.generatedAt,
134+
database: file,
135+
inventoryDigest: inventoryDigest(),
136+
posture,
137+
conflicts: reviewedConflictsOf(reviewed),
138+
},
139+
catalog: { async resolve() { return undefined; } },
140+
notNull,
141+
door: {
142+
async save(type, name) {
143+
throw new Error(`no environment save is expected here (${type} ${name})`);
144+
},
145+
},
146+
templates: { async find() { return undefined; } },
147+
views: { async list() { return []; } },
148+
lock: { async verdict() { return { editable: true }; } },
149+
columnDrops: { async drop(columns) { return columns.map((c) => ({ ...c, outcome: 'dropped' as const })); } },
150+
ledger: l,
151+
now: () => new Date('2026-11-08T00:00:00Z'),
152+
});
153+
}
154+
155+
const step = (report: OrganizationOwnershipApplyReport) => report.steps.find((s) => s.step === 'not-null')!;
156+
const certified = (report: OrganizationOwnershipApplyReport): boolean => report.complete && report.blocking === 0;
157+
158+
beforeEach(async () => {
159+
dir = mkdtempSync(join(tmpdir(), 'os-d10-not-null-resume-'));
160+
file = join(dir, 'db.sqlite');
161+
rebuilt = [];
162+
driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: file }, useNullAsDefault: true });
163+
(driver as any).logger = { warn: () => {}, info: () => {}, error: () => {}, debug: () => {} };
164+
await driver.initObjects(
165+
TABLES.map((name) => ({
166+
name,
167+
fields: { name: { type: 'text' }, organization_id: { type: 'lookup', reference: 'sys_organization', required: false } },
168+
})) as never,
169+
);
170+
// The tables an earlier release built: their tenant column nullable, which a
171+
// table this release creates never is (ADR-0131 D1). Relaxed through the
172+
// driver's own drift apply door, the shape of that store.
173+
for (const table of TABLES) {
174+
const { applied } = await driver.applyMigrationEntries([
175+
{
176+
kind: 'nullability_mismatch', remoteName: table, table, column: 'organization_id', category: 'needs_confirm',
177+
op: { type: 'relax_not_null', table, column: 'organization_id' },
178+
} as never,
179+
]);
180+
if (applied.length !== 1) throw new Error(`${table} was not returned to its pre-18 shape`);
181+
}
182+
db = new Database(file);
183+
db.exec(`
184+
CREATE TABLE sys_organization (id TEXT PRIMARY KEY, slug TEXT);
185+
INSERT INTO sys_organization VALUES ('${ORG}', 'default');
186+
INSERT INTO crm_deal (id, name, organization_id) VALUES ('deal_1', 'Acme', '${ORG}');
187+
INSERT INTO crm_note (id, name, organization_id) VALUES ('note_1', 'Call back', '${ORG}');
188+
INSERT INTO crm_task (id, name, organization_id) VALUES ('task_1', 'Send quote', '${ORG}');
189+
`);
190+
for (const table of TABLES) expect(isNotNull(table)).toBe(false);
191+
});
192+
193+
afterEach(async () => {
194+
db.close();
195+
await driver.disconnect?.();
196+
rmSync(dir, { recursive: true, force: true });
197+
});
198+
199+
describe('the not-null step on a resumed ceremony', () => {
200+
it('⭐ (1) certified with tables not-declared, re-run under a composition that declares them: they receive NOT NULL', async () => {
201+
const l = ledger();
202+
const first = await apply(l, schemaDoor(new Set(['crm_deal'])));
203+
expect(step(first)).toMatchObject({ status: 'done', summary: { due: 3, applied: 1, notDeclared: 2 } });
204+
expect(step(first).notes.filter((n) => /re-run the ceremony where the deployment's composition is declared/.test(n))).toHaveLength(2);
205+
expect(certified(first)).toBe(true);
206+
expect(TABLES.map(isNotNull)).toEqual([true, false, false]);
207+
208+
const again = await apply(l, schemaDoor(new Set(TABLES)));
209+
expect(step(again)).toMatchObject({ status: 'done', summary: { due: 3, applied: 2, alreadyNotNull: 1, notDeclared: 0 } });
210+
expect(TABLES.map(isNotNull)).toEqual([true, true, true]);
211+
// The table tightened by the first run is not rebuilt again.
212+
expect(rebuilt).toEqual(['crm_deal', 'crm_note', 'crm_task']);
213+
expect(l.stored()!.completed['not-null']!.summary).toMatchObject({ applied: 2, alreadyNotNull: 1, notDeclared: 0 });
214+
expect(certified(again)).toBe(true);
215+
});
216+
217+
it('⭐ (2) the walled recovery path — apply, fix the held rows, plan, apply — ends with NOT NULL on the fixed table', async () => {
218+
db.exec(`INSERT INTO crm_note (id, name, organization_id) VALUES ('note_orphan', 'nobody', NULL)`);
219+
const l = ledger();
220+
const door = schemaDoor(new Set(TABLES));
221+
const first = await apply(l, door, 'isolated');
222+
expect(step(first)).toMatchObject({ status: 'done', summary: { due: 2, applied: 2 } });
223+
expect(TABLES.map(isNotNull)).toEqual([true, false, true]);
224+
// The held row stops certification, and names the remedy: resolve it by hand, re-run --apply.
225+
expect(certified(first)).toBe(false);
226+
expect(first.tables.find((t) => t.object === 'crm_note')!.held).toMatchObject({ rows: 1 });
227+
228+
db.exec(`UPDATE crm_note SET organization_id = '${ORG}' WHERE id = 'note_orphan'`);
229+
const plan = await buildOrganizationOwnershipPlan({ reader: executor(), posture: 'isolated', database: file });
230+
expect(plan.tables.find((t) => t.object === 'crm_note')!.notNull.willReceive).toBe(true);
231+
232+
const again = await apply(l, door, 'isolated');
233+
expect(step(again)).toMatchObject({ status: 'done', summary: { due: 3, applied: 1, alreadyNotNull: 2 } });
234+
expect(TABLES.map(isNotNull)).toEqual([true, true, true]);
235+
expect(certified(again)).toBe(true);
236+
});
237+
238+
it('(3) control — a first run with nothing pending checkpoints as before; its re-run stays checkpointed and rebuilds nothing', async () => {
239+
const l = ledger();
240+
const door = schemaDoor(new Set(TABLES));
241+
const first = await apply(l, door);
242+
const summary = { applied: 3, alreadyNotNull: 0, due: 3, notDeclared: 0, unsupported: 0 };
243+
expect(step(first)).toEqual({ step: 'not-null', status: 'done', summary, blockers: [], notes: [] });
244+
expect(l.stored()!.completed['not-null']).toEqual({ at: '2026-11-08T00:00:00.000Z', summary });
245+
expect(certified(first)).toBe(true);
246+
expect(rebuilt).toEqual([...TABLES]);
247+
248+
const again = await apply(l, door);
249+
expect(step(again)).toEqual({ step: 'not-null', status: 'checkpointed', summary, blockers: [], notes: [] });
250+
expect(l.stored()!.completed['not-null']).toEqual({ at: '2026-11-08T00:00:00.000Z', summary });
251+
expect(rebuilt).toEqual([...TABLES]);
252+
expect(certified(again)).toBe(true);
253+
});
254+
255+
it('(4) a re-run whose NOT NULL fails blocks and does not certify, as the same failure does on a first run', async () => {
256+
const l = ledger();
257+
await apply(l, schemaDoor(new Set(['crm_deal'])));
258+
const again = await apply(l, schemaDoor(new Set(TABLES), { failOn: 'crm_task' }));
259+
expect(step(again)).toMatchObject({ status: 'blocked', blockers: [expect.stringMatching(/NOT NULL on crm_task\.organization_id failed: SQLITE_BUSY/)] });
260+
expect(TABLES.map(isNotNull)).toEqual([true, true, false]);
261+
expect(certified(again)).toBe(false);
262+
expect(l.stored()!.completed['not-null']).toBeUndefined();
263+
});
264+
});

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

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,14 @@ import type { DeclaredSharingRuleSkip } from '@objectstack/plugin-sharing';
182182
* empty, a delete selects only mirrors — so a second run writes nothing the
183183
* first one wrote. On top of that, each step that finishes is recorded in the
184184
* ceremony's checkpoint ({@link CeremonyLedgerPort}) and is not run again: a
185-
* run interrupted inside step N resumes at step N. Two steps run again when
186-
* checkpointed: `history-renumber` (below), and `attribution` whenever the
185+
* run interrupted inside step N resumes at step N. Steps that run again when
186+
* checkpointed include `history-renumber` (below); `attribution` whenever the
187187
* plan derives an owner for rows still NULL — a row the operator made
188188
* attributable since, or one an earlier run found no Default Organization
189-
* for. A step that stops on a
189+
* for; and `not-null` whenever the run has a schema door, so a table it left
190+
* nullable (not declared by that run's composition, or held by a NULL row
191+
* since resolved) receives NOT NULL, while a column already NOT NULL is never
192+
* rebuilt. A step that stops on a
190193
* blocker stops the ceremony there — fate order is the contract — and the
191194
* report names the blocker and its remedy.
192195
*
@@ -4154,6 +4157,33 @@ export async function applyOrganizationOwnership(opts: ApplyOptions): Promise<Or
41544157
run.columns.clear();
41554158
continue;
41564159
}
4160+
// `not-null` runs again even when checkpointed, whenever the run has a
4161+
// schema door: a table the step left nullable — the composition an earlier
4162+
// run booted declared no object for it (`not-declared`), or a NULL row held
4163+
// it until the operator resolved it by hand — receives NOT NULL on the
4164+
// re-run each of those advice lines names. The door answers
4165+
// `already-not-null` for a column an earlier run tightened, so a run with
4166+
// nothing left to tighten stays `checkpointed`, exactly as before; one
4167+
// whose NOT NULL fails is no longer recorded as done, as on a first run.
4168+
if (done && step === 'not-null' && opts.notNull) {
4169+
const again = await RUNNERS[step](run);
4170+
if (again.status !== 'done') {
4171+
steps.push(again);
4172+
delete checkpoint.completed[step];
4173+
await opts.ledger.write(checkpoint, { complete: false, blocking: 0, advisory: 0 });
4174+
stopped = true;
4175+
continue;
4176+
}
4177+
if (Number(again.summary.applied ?? 0) > 0) {
4178+
steps.push(again);
4179+
checkpoint.completed[step] = { at: now().toISOString(), summary: again.summary };
4180+
await opts.ledger.write(checkpoint, { complete: false, blocking: 0, advisory: 0 });
4181+
run.columns.clear();
4182+
continue;
4183+
}
4184+
steps.push({ step, status: 'checkpointed', summary: done.summary, blockers: [], notes: again.notes });
4185+
continue;
4186+
}
41574187
if (done) {
41584188
// `history-renumber` runs again even when checkpointed: it is
41594189
// idempotent, and an environment write made since the earlier run must

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,28 @@ describe('organization_id NOT NULL through the drift apply door', () => {
158158
expect(attempted).toBe(0);
159159
});
160160

161+
it('⭐ a declared table an earlier run tightened is already-not-null, never rebuilt again', async () => {
162+
const target = {
163+
isDeclared: (t: string) => t === 'crm_deal',
164+
isNotNull: async (table: string, column: string) => !(await nullable(table, column)),
165+
driverFor: () => driver as never,
166+
};
167+
expect(await applyNotNullThroughDrift(target, [{ table: 'crm_deal', column: 'organization_id' }])).toEqual([
168+
{ table: 'crm_deal', column: 'organization_id', outcome: 'applied' },
169+
]);
170+
let attempted = 0;
171+
const outcomes = await applyNotNullThroughDrift(
172+
{
173+
...target,
174+
driverFor: () => ({ applyMigrationEntries: async () => { attempted++; return { applied: [], skipped: [] }; } }) as never,
175+
},
176+
[{ table: 'crm_deal', column: 'organization_id' }],
177+
);
178+
expect(outcomes).toEqual([{ table: 'crm_deal', column: 'organization_id', outcome: 'already-not-null' }]);
179+
expect(attempted).toBe(0);
180+
expect(await nullable('crm_deal', 'organization_id')).toBe(false);
181+
});
182+
161183
it('no driver for the table\'s database fails, naming the database', async () => {
162184
const [outcome] = await applyNotNullThroughDrift(
163185
{ isDeclared: () => true, driverFor: () => undefined },

0 commit comments

Comments
 (0)