diff --git a/packages/jinn/src/gateway/__tests__/control-plane-authority.test.ts b/packages/jinn/src/gateway/__tests__/control-plane-authority.test.ts index 1595b634..7a8f2d9d 100644 --- a/packages/jinn/src/gateway/__tests__/control-plane-authority.test.ts +++ b/packages/jinn/src/gateway/__tests__/control-plane-authority.test.ts @@ -538,7 +538,7 @@ describe("portal fallback is a virtual root, not employee authority", () => { expect(approval.approvalTarget).toBe(legacyRoot); expect(approval.approvalTargetKind).toBe("virtual"); - (await import("../../shared/db.js")).initDb().prepare("UPDATE work_items SET approval_target_kind = NULL WHERE id = ?").run(approval.id); + (await import("../../shared/db.js")).initDb().prepare("UPDATE work_item_approvals SET target_kind = NULL WHERE work_item_id = ?").run(approval.id); fs.writeFileSync( legacyFile, "name: Legacy Root\ndisplayName: Legacy Root\ndepartment: platform\nrank: employee\nreportsTo: platform-manager\nengine: codex\nmodel: gpt-5.5\npersona: Attempts to claim a legacy persisted approval target.\n", diff --git a/packages/jinn/src/gateway/__tests__/work-item-approval-parity.test.ts b/packages/jinn/src/gateway/__tests__/work-item-approval-parity.test.ts index 77371655..4056b22d 100644 --- a/packages/jinn/src/gateway/__tests__/work-item-approval-parity.test.ts +++ b/packages/jinn/src/gateway/__tests__/work-item-approval-parity.test.ts @@ -8,12 +8,12 @@ import type { ServerResponse } from "node:http"; /** * Todos v2 slice 4 — GOLDEN legacy byte-parity for the approval fields. * - * The approval_* columns are frozen and every payload sources the legacy - * `approval*` fields from `work_item_approvals`. These pins hold the hard - * compatibility bar: for databases carrying pre-slice column values (simulated - * here exactly as the backfill finds them), the compact AND detail payloads - * must emit the SAME values the pre-slice column-backed implementation emitted - * — plus the new additive `approvals` history on the detail payload only. + * Every payload sources the legacy `approval*` fields from `work_item_approvals`. + * These pins hold the hard compatibility bar: for databases whose approvals came + * from pre-slice columns (seeded here exactly as the backfill leaves them), the + * compact AND detail payloads must emit the SAME values the pre-slice + * column-backed implementation emitted — plus the new additive `approvals` + * history on the detail payload only. */ const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "jinn-wi-parity-")); @@ -23,11 +23,9 @@ const dbModule = await import("../../shared/db.js"); type Api = typeof import("../api.js"); type Store = typeof import("../../work-items/store.js"); type Approvals = typeof import("../../work-items/approvals.js"); -type Migrate = typeof import("../../work-items/migrate.js"); let api: Api; let store: Store; let approvals: Approvals; -let migrate: Migrate; function makeRes() { let status = 200; @@ -88,27 +86,18 @@ interface LegacyApprovalColumns { approvalDecidedAt: string | null; } -/** Simulate a pre-slice-4 database row: write the approval COLUMNS directly - * (the write path no longer does), exactly what the backfill later consumes. */ +/** Simulate a pre-slice-4 database's approvals: the exact `'legacy'` row the + * column backfill mints, so the payloads face what a migrated home carries. */ function seedLegacyColumns(id: string, legacy: LegacyApprovalColumns): void { - dbModule - .initDb() - .prepare( - `UPDATE work_items SET approval_state = ?, approval_request = ?, approval_ref = ?, approval_target = ?, - approval_target_kind = ?, approval_escalated_at = ?, approval_decided_by = ?, approval_decided_at = ? - WHERE id = ?`, - ) - .run( - legacy.approvalState, - legacy.approvalRequest, - legacy.approvalRef, - legacy.approvalTarget, - legacy.approvalTargetKind, - legacy.approvalEscalatedAt, - legacy.approvalDecidedBy, - legacy.approvalDecidedAt, - id, - ); + if (legacy.approvalState === null) return; + dbModule.initDb().prepare( + `INSERT INTO work_item_approvals (id, work_item_id, state, request, ref, target, target_kind, + requested_by, requested_at, escalated_at, decided_by, decided_at, note) + SELECT 'wap_' || lower(hex(randomblob(6))), w.id, @approvalState, COALESCE(@approvalRequest, ''), @approvalRef, + @approvalTarget, @approvalTargetKind, 'legacy', COALESCE(@approvalDecidedAt, w.updated_at), + @approvalEscalatedAt, @approvalDecidedBy, @approvalDecidedAt, NULL + FROM work_items w WHERE w.id = @id`, + ).run({ ...legacy, id }); } function legacySubset(payload: Record): LegacyApprovalColumns { @@ -202,17 +191,15 @@ beforeAll(async () => { api = await import("../api.js"); store = await import("../../work-items/store.js"); approvals = await import("../../work-items/approvals.js"); - migrate = await import("../../work-items/migrate.js"); - (await import("../../shared/db.js")).initDb(); + dbModule.initDb(); for (const fixture of FIXTURES) { const item = store.createWorkItem({ title: `parity ${fixture.name}`, department: "parity-fixture" }); seedLegacyColumns(item.id, fixture.legacy); itemIds.set(fixture.name, item.id); } - migrate.backfillWorkItemApprovals((await import("../../shared/db.js")).initDb()); }); -describe("legacy approval-field byte-parity across the dual-read window", () => { +describe("legacy approval-field byte-parity, sourced from work_item_approvals", () => { it.each(FIXTURES.map((fixture) => [fixture.name, fixture.legacy] as const))( "detail payload emits the exact pre-slice values for the %s fixture", async (name, legacy) => { diff --git a/packages/jinn/src/work-items/__tests__/approvals.test.ts b/packages/jinn/src/work-items/__tests__/approvals.test.ts index 1b7789c5..c57aa634 100644 --- a/packages/jinn/src/work-items/__tests__/approvals.test.ts +++ b/packages/jinn/src/work-items/__tests__/approvals.test.ts @@ -199,32 +199,12 @@ describe("decideWorkItemApproval — native consequence rules", () => { /* ── approvals off-row (Todos v2 slice 4) — the work_item_approvals table ───── */ -describe("approvals off-row — writes land in work_item_approvals, columns stay frozen", () => { - function rawColumns(id: string): Record { - return dbModule - .initDb() - .prepare( - `SELECT approval_state, approval_request, approval_ref, approval_target, - approval_target_kind, approval_escalated_at, approval_decided_by, approval_decided_at - FROM work_items WHERE id = ?`, - ) - .get(id) as Record; - } - - function expectColumnsFrozenNull(id: string): void { - const cols = rawColumns(id); - for (const [column, value] of Object.entries(cols)) { - expect(value, `${column} must stay frozen (never written post-slice-4)`).toBeNull(); - } - } - - it("request writes ONLY the new table; the legacy approval_* columns never change", () => { +describe("approvals off-row — writes land in work_item_approvals", () => { + it("request writes the approvals table and the returned item reads back from it", () => { const item = store.createWorkItem({ title: "Off-row request", status: "in_review", source: "human" }); const out = approvals.requestApproval(item.id, { request: "gate?", ref: "opaque-ref", target: null, actor: "session:sX" }); - // The returned WorkItem still reads as pending (dual-read), but the columns are untouched. expect(out.approvalState).toBe("pending"); expect(out.approvalRef).toBe("opaque-ref"); - expectColumnsFrozenNull(item.id); const row = approvals.currentApproval(item.id)!; expect(row.state).toBe("pending"); expect(row.request).toBe("gate?"); @@ -239,7 +219,7 @@ describe("approvals off-row — writes land in work_item_approvals, columns stay expect(out.version).toBe(item.version + 1); }); - it("decide + escalate write the pending row (note included); columns stay frozen", async () => { + it("decide + escalate write the pending row (note included)", async () => { const item = store.createWorkItem({ title: "Off-row decide", status: "backlog", source: "human" }); approvals.requestApproval(item.id, { request: "plan ok?", target: null }); const escalated = approvals.escalateApproval(item.id, "coo", "needs the operator"); @@ -252,7 +232,6 @@ describe("approvals off-row — writes land in work_item_approvals, columns stay expect(row.decidedBy).toBe("coo"); expect(row.decidedAt).toBeTruthy(); expect(row.note).toBe("fine"); - expectColumnsFrozenNull(item.id); }); it("keeps approval history: a fresh request after a decision is a NEW row; current = pending else latest decided", async () => { @@ -277,7 +256,7 @@ describe("approvals off-row — writes land in work_item_approvals, columns stay expect(current.request).toBe("round two"); expect(current.state).toBe("approved"); - // the dual-read WorkItem view tracks the current row + // the hydrated WorkItem view tracks the current row const roundTrip = store.getWorkItem(item.id)!; expect(roundTrip.approvalState).toBe("approved"); expect(roundTrip.approvalRequest).toBe("round two"); @@ -334,7 +313,6 @@ describe("approvals off-row — writes land in work_item_approvals, columns stay approvals.requestApproval(item.id, { request: "sign-off", target: "attention-target" }); const hits = store.listWorkItems({ needsAttentionFor: "attention-target" }); expect(hits.some((i) => i.id === item.id)).toBe(true); - expectColumnsFrozenNull(item.id); }); }); diff --git a/packages/jinn/src/work-items/__tests__/migrate-v2.test.ts b/packages/jinn/src/work-items/__tests__/migrate-v2.test.ts index ff6266d6..6deea61d 100644 --- a/packages/jinn/src/work-items/__tests__/migrate-v2.test.ts +++ b/packages/jinn/src/work-items/__tests__/migrate-v2.test.ts @@ -109,7 +109,7 @@ describe("v1 → v2 migration", () => { }); }); -/* ── Todos v2 slice 4: work_item_approvals backfill + self-heal + verifier ──── */ +/* ── Todos v2 slice 4 + PLA-48: work_item_approvals as the sole owner ───────── */ function freshV2(file: string): Database.Database { const fresh = new Database(file); @@ -118,10 +118,13 @@ function freshV2(file: string): Database.Database { return fresh; } -function seedItemWithColumns( - fresh: Database.Database, - approval: Record | null, -): string { +function approvalColumns(db: Database.Database): string[] { + return (db.prepare("PRAGMA table_info(work_items)").all() as Array<{ name: string }>) + .map((column) => column.name).filter((name) => name.startsWith("approval_")); +} + +/** Seed one item; `approval` (legacy-shape databases only) fills the shadow columns. */ +function seedItem(fresh: Database.Database, approval?: Record): string { const base = "2026-07-01T00:00:00.000Z"; const claim = migrate.allocateWorkItemId(fresh, base, "ACM"); migrate.useWorkItemAllocationClaim(fresh, claim, () => { @@ -134,65 +137,62 @@ function seedItemWithColumns( fresh.prepare( `UPDATE work_items SET approval_state = @state, approval_request = @request, approval_ref = @ref, approval_target = @target, approval_target_kind = @target_kind, approval_escalated_at = @escalated_at, - approval_decided_by = @decided_by, approval_decided_at = @decided_at - WHERE id = @id`, + approval_decided_by = @decided_by, approval_decided_at = @decided_at WHERE id = @id`, ).run({ - state: null, request: null, ref: null, target: null, target_kind: null, - escalated_at: null, decided_by: null, decided_at: null, - ...approval, - id: claim.id, + state: null, request: null, ref: null, target: null, target_kind: null, escalated_at: null, decided_by: null, decided_at: null, ...approval, id: claim.id, }); } return claim.id; } -describe("work_item_approvals backfill", () => { - it("copies pending/decided/escalated column values into exactly one row per item, skips NULL items, and is idempotent", () => { +describe("legacy approval columns heal into work_item_approvals", () => { + it("a fresh v2 database carries no approval_% column and verifies clean", () => { + const db = freshV2(path.join(tmp, "registry-no-approval-columns.db")); + expect(approvalColumns(db)).toEqual([]); + migrate.verifyCurrentWorkItemSchema(db); + db.close(); + }); + + it("classifies a legacy-shape database as current, moves the values off-row, drops the columns, and re-runs as a no-op", () => { const file = path.join(tmp, "registry-backfill.db"); - const db = freshV2(file); - const pendingId = seedItemWithColumns(db, { - state: "pending", request: "legacy pending gate", ref: "workflow-gate:old:run:g", - target: "coo", target_kind: "employee", - }); - const decidedId = seedItemWithColumns(db, { - state: "approved", request: "legacy decided gate", target: "coo", target_kind: "employee", - escalated_at: "2026-07-02T00:00:00.000Z", decided_by: "operator", decided_at: "2026-07-03T00:00:00.000Z", - }); - const plainId = seedItemWithColumns(db, null); + // A pre-PLA-48 home: work_items still carries the shadow approval_* columns, + // with the real indexes and triggers; the additive tables never shipped. + const legacy = new Database(file); + migrate.registerWorkItemIdentityFunctions(legacy); + for (const ddl of [migrate.WORK_ITEM_IDENTITY_TABLES_DDL, migrate.V2_APPROVAL_WORK_ITEMS_TABLE_DDL, + migrate.WORK_ITEMS_INDEX_DDL, migrate.WORK_ITEM_EVENTS_DDL, migrate.WORK_ITEM_EDIT_RECEIPTS_DDL, + migrate.WORK_ITEM_IDENTITY_TRIGGERS_DDL]) legacy.exec(ddl); + const pendingId = seedItem(legacy, { state: "pending", request: "legacy pending gate", ref: "workflow-gate:old:run:g", target: "coo", target_kind: "employee" }); + const decidedId = seedItem(legacy, { state: "approved", request: "legacy decided gate", target: "coo", target_kind: "employee", escalated_at: "2026-07-02T00:00:00.000Z", decided_by: "operator", decided_at: "2026-07-03T00:00:00.000Z" }); + const plainId = seedItem(legacy); + expect(approvalColumns(legacy).length).toBe(8); + legacy.close(); - const first = migrate.backfillWorkItemApprovals(db); - expect(first).toBe(2); + expect(migrate.preflightWorkItemsDatabase(file)).toBe("current"); + const db = new Database(file); + migrate.registerWorkItemIdentityFunctions(db); + expect(migrate.migrateWorkItemsSchema(db).rebuilt).toBe(false); + migrate.verifyCurrentWorkItemSchema(db); + expect(approvalColumns(db)).toEqual([]); - const rows = db - .prepare("SELECT * FROM work_item_approvals ORDER BY work_item_id") - .all() as Array>; + const rows = db.prepare("SELECT * FROM work_item_approvals ORDER BY work_item_id").all() as Array>; expect(rows.length).toBe(2); - const pendingRow = rows.find((r) => r.work_item_id === pendingId)!; - expect(pendingRow.state).toBe("pending"); - expect(pendingRow.request).toBe("legacy pending gate"); - expect(pendingRow.ref).toBe("workflow-gate:old:run:g"); - expect(pendingRow.target).toBe("coo"); - expect(pendingRow.target_kind).toBe("employee"); - expect(pendingRow.requested_by).toBe("legacy"); - expect(pendingRow.requested_at).toBeTruthy(); - expect(pendingRow.decided_by).toBeNull(); - const decidedRow = rows.find((r) => r.work_item_id === decidedId)!; - expect(decidedRow.state).toBe("approved"); - expect(decidedRow.escalated_at).toBe("2026-07-02T00:00:00.000Z"); - expect(decidedRow.decided_by).toBe("operator"); - expect(decidedRow.decided_at).toBe("2026-07-03T00:00:00.000Z"); + expect(rows.find((r) => r.work_item_id === pendingId)).toMatchObject({ + state: "pending", request: "legacy pending gate", ref: "workflow-gate:old:run:g", target: "coo", + target_kind: "employee", requested_by: "legacy", requested_at: "2026-07-01T00:00:00.000Z", decided_by: null, + }); + expect(rows.find((r) => r.work_item_id === decidedId)).toMatchObject({ + state: "approved", escalated_at: "2026-07-02T00:00:00.000Z", + decided_by: "operator", decided_at: "2026-07-03T00:00:00.000Z", + }); expect(rows.some((r) => r.work_item_id === plainId)).toBe(false); - // idempotent: a re-run inserts nothing and changes nothing + // a second boot inserts nothing and changes nothing const before = JSON.stringify(rows); - expect(migrate.backfillWorkItemApprovals(db)).toBe(0); - const after = JSON.stringify(db.prepare("SELECT * FROM work_item_approvals ORDER BY work_item_id").all()); - expect(after).toBe(before); - - // the frozen columns themselves are untouched by the backfill - const cols = db.prepare("SELECT approval_state FROM work_items WHERE id = ?").get(pendingId) as { approval_state: string }; - expect(cols.approval_state).toBe("pending"); + expect(migrate.migrateWorkItemsSchema(db).rebuilt).toBe(false); + expect(JSON.stringify(db.prepare("SELECT * FROM work_item_approvals ORDER BY work_item_id").all())).toBe(before); db.close(); + expect(migrate.preflightWorkItemsDatabase(file)).toBe("current"); }); it("backfills during the v1 → v2 rebuild", () => { @@ -213,15 +213,16 @@ describe("work_item_approvals backfill", () => { expect(row.state).toBe("pending"); expect(row.request).toBe("v1 pending"); expect(row.requested_by).toBe("legacy"); + expect(approvalColumns(db)).toEqual([]); db.close(); }); }); describe("work_item_approvals self-heal + verifier", () => { - it("boots a v2 DB missing the approvals table additively and backfills column carriers", () => { + it("boots a v2 DB missing the approvals table additively", () => { const file = path.join(tmp, "registry-heal-approvals.db"); const db = freshV2(file); - const carrierId = seedItemWithColumns(db, { state: "pending", request: "heal me", target: "coo", target_kind: "employee" }); + seedItem(db); db.exec("DROP TABLE work_item_approvals"); db.close(); @@ -231,15 +232,14 @@ describe("work_item_approvals self-heal + verifier", () => { migrate.registerWorkItemIdentityFunctions(reopened); expect(migrate.migrateWorkItemsSchema(reopened).rebuilt).toBe(false); migrate.verifyCurrentWorkItemSchema(reopened); - const row = reopened.prepare("SELECT state, request FROM work_item_approvals WHERE work_item_id = ?").get(carrierId) as Record; - expect(row).toEqual({ state: "pending", request: "heal me" }); + expect(reopened.prepare("SELECT count(*) AS n FROM work_item_approvals").get()).toEqual({ n: 0 }); reopened.close(); }); it("verifier refuses a dangling approval row (unknown work item)", () => { const file = path.join(tmp, "registry-verify-dangling.db"); const db = freshV2(file); - seedItemWithColumns(db, null); + seedItem(db); db.pragma("foreign_keys = OFF"); db.prepare( `INSERT INTO work_item_approvals (id, work_item_id, state, request, requested_by, requested_at) @@ -252,7 +252,7 @@ describe("work_item_approvals self-heal + verifier", () => { it("verifier refuses two pending rows for one item even if the unique index is gone (belt and suspenders)", () => { const file = path.join(tmp, "registry-verify-dup-pending.db"); const db = freshV2(file); - const id = seedItemWithColumns(db, null); + const id = seedItem(db); db.exec("DROP INDEX uq_wap_pending"); for (const rowId of ["wap_bbbbbbbbbbbb", "wap_cccccccccccc"]) { db.prepare( @@ -269,7 +269,7 @@ describe("work_item_approvals self-heal + verifier", () => { it("boots a v2 DB missing the operator-only table additively", () => { const file = path.join(tmp, "registry-heal-operator-only.db"); const db = freshV2(file); - seedItemWithColumns(db, { state: "pending", request: "reserve me", target: "coo", target_kind: "employee" }); + seedItem(db); db.exec("DROP TABLE work_item_approval_operator_only"); db.close(); diff --git a/packages/jinn/src/work-items/approvals.ts b/packages/jinn/src/work-items/approvals.ts index baddd6bd..cda5e43a 100644 --- a/packages/jinn/src/work-items/approvals.ts +++ b/packages/jinn/src/work-items/approvals.ts @@ -119,8 +119,8 @@ function classifyApprovalTarget(item: WorkItem, inputTarget: string | null | und /** * Attach a PENDING approval to an item (the native "any actor may REQUEST" path, - * design §1.3). Sets `approval_state='pending'` + the request text + optional ref, - * clears any prior decision stamps, and appends ONE `approval_requested` event — + * design §1.3). Writes a PENDING `work_item_approvals` row carrying the request + * text + optional ref, and appends ONE `approval_requested` event — * status is orthogonal and left untouched. Idempotent when the item is already * pending on the identical (request, ref): no write, no duplicate event (so a * workflow-park re-mirror on every sweep stays event-silent). Throws on an @@ -365,7 +365,7 @@ export type DecideWorkItemApprovalResult = * transaction (GRS-021b QA finding 2 — no half-applied approved+in_review). The * decision write, the `approval_decided` event, the status transition * (done / bounce+rounds / escalate), and the status event either ALL commit or - * NONE do. Guarded on `approval_state = 'pending'` re-read INSIDE the txn, so a + * NONE do. Guarded on a pending-approval re-read INSIDE the txn, so a * double-decide or a decide-after-resolved is a clean refusal, never a partial * apply. `decideApproval` and `transition` each open their own transaction; called * here they nest as SAVEPOINTs, so a throw from the status write (or a concurrent diff --git a/packages/jinn/src/work-items/migrate.ts b/packages/jinn/src/work-items/migrate.ts index a0a1f747..b672250b 100644 --- a/packages/jinn/src/work-items/migrate.ts +++ b/packages/jinn/src/work-items/migrate.ts @@ -109,6 +109,35 @@ ${V1_WORK_ITEM_ID_ISSUANCES_TABLE_DDL}; /** v2 (Todos v2 slice 1): per-department prefixes + sub-task tree columns. */ export const WORK_ITEMS_TABLE_DDL = ` +CREATE TABLE IF NOT EXISTS work_items ( + id TEXT PRIMARY KEY CHECK (${CANONICAL_ID_SQL}), + title TEXT NOT NULL, + body TEXT, + status TEXT NOT NULL DEFAULT 'backlog' CHECK (status IN ('backlog','assigned','executing','in_review','done','blocked','escalated','cancelled')), + department TEXT, + assignee TEXT, + created_by TEXT NOT NULL, + parent_id TEXT REFERENCES work_items(id), + root_id TEXT NOT NULL, + depth INTEGER NOT NULL DEFAULT 0 CHECK ((parent_id IS NULL AND depth = 0) OR (parent_id IS NOT NULL AND depth BETWEEN 1 AND 3)), + due_at TEXT, + priority INTEGER NOT NULL DEFAULT 2 CHECK (priority BETWEEN 0 AND 3), + rank REAL, + version INTEGER NOT NULL DEFAULT 1 CHECK (version >= 1), + source TEXT NOT NULL DEFAULT 'human' CHECK (source IN ('human','delegation','cron','workflow','session','connector','goal')), + source_ref TEXT, + acceptance TEXT, + verify_policy TEXT, + rounds INTEGER NOT NULL DEFAULT 0, + budget_usd REAL, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + closed_at TEXT +)`; + +/** Pre-PLA-48 v2 work_items shape, when approvals were also shadowed in eight + * columns here. Frozen recognizer: a match is healed at boot. */ +export const V2_APPROVAL_WORK_ITEMS_TABLE_DDL = ` CREATE TABLE IF NOT EXISTS work_items ( id TEXT PRIMARY KEY CHECK (${CANONICAL_ID_SQL}), title TEXT NOT NULL, @@ -143,6 +172,10 @@ CREATE TABLE IF NOT EXISTS work_items ( closed_at TEXT )`; +/** The shadow columns above, dropped once their values reach `work_item_approvals`. */ +const V2_APPROVAL_COLUMNS: readonly string[] = ["approval_state", "approval_request", "approval_ref", + "approval_target", "approval_target_kind", "approval_escalated_at", "approval_decided_by", "approval_decided_at"]; + export const WORK_ITEMS_INDEX_DDL = ` CREATE INDEX IF NOT EXISTS idx_work_items_status ON work_items(status); CREATE INDEX IF NOT EXISTS idx_work_items_department ON work_items(department); @@ -268,13 +301,11 @@ CREATE INDEX IF NOT EXISTS idx_wia_item ON work_item_attachments(work_item_id, c CREATE INDEX IF NOT EXISTS idx_wia_comment ON work_item_attachments(comment_id) WHERE comment_id IS NOT NULL; `; -/** Todos v2 slice 4: approvals leave the fat work_items row. Full history — - * one row per requested gate; the partial unique index makes "at most one - * PENDING approval per item" a DB guarantee. The legacy `approval_*` columns - * on work_items stay physically present but FROZEN (never written again after - * the backfill); every read goes through this table. `ref` is the opaque - * correlation reference the request contract has always carried (kept here so - * the legacy `approvalRef` payload field stays byte-identical). */ +/** Todos v2 slice 4: approvals leave the fat work_items row. The SOLE storage + * owner since PLA-48 — full history, one row per requested gate; the partial + * unique index makes "at most one PENDING approval per item" a DB guarantee. + * `ref` is the opaque correlation reference the request contract has always + * carried (kept here so the `approvalRef` payload field stays byte-identical). */ export const WORK_ITEM_APPROVALS_TABLE_DDL = ` CREATE TABLE IF NOT EXISTS work_item_approvals ( id TEXT PRIMARY KEY CHECK (id GLOB 'wap_[0-9a-f]*' AND length(id) = 16), @@ -685,15 +716,13 @@ const V2_ADDITIVE_TABLES: ReadonlyArray<{ name: string; ddl: string }> = [ ]; /** - * Copy the frozen legacy `approval_*` column values into `work_item_approvals` - * (Todos v2 slice 4, dual-read window). Exactly one row per item whose columns - * carry a state; idempotent — an item that already has ANY approval row is - * skipped, so re-runs (every boot) and post-slice items are no-ops. The columns - * themselves are read, never written. `requested_by`/`requested_at` are not - * recoverable from the columns: `'legacy'` and the best column-derived bound - * (decided_at when decided, else the row's updated_at) stand in. + * Copy a shadow-column table's `approval_*` values into `work_item_approvals`, + * one row per item carrying a state — a one-shot step inside each rebuild, and + * a no-op for an item that already has an approval row. `requested_by`/ + * `requested_at` are not recoverable from the columns: `'legacy'` and the best + * column-derived bound (decided_at when decided, else updated_at) stand in. */ -export function backfillWorkItemApprovals(db: DatabaseType): number { +function backfillWorkItemApprovals(db: DatabaseType, source: "work_items" | "work_items_v1_legacy"): number { return db .prepare( `INSERT INTO work_item_approvals @@ -701,7 +730,7 @@ export function backfillWorkItemApprovals(db: DatabaseType): number { SELECT 'wap_' || lower(hex(randomblob(6))), w.id, w.approval_state, COALESCE(w.approval_request, ''), w.approval_ref, w.approval_target, w.approval_target_kind, 'legacy', COALESCE(w.approval_decided_at, w.updated_at), w.approval_escalated_at, w.approval_decided_by, w.approval_decided_at, NULL - FROM work_items w + FROM ${source} w WHERE w.approval_state IS NOT NULL AND NOT EXISTS (SELECT 1 FROM work_item_approvals a WHERE a.work_item_id = w.id)`, ) @@ -712,8 +741,7 @@ export function backfillWorkItemApprovals(db: DatabaseType): number { * Register any department that holds Todos but is missing from the registry * (review F2). Department-changing writes now mint the row in their own * transaction; this reconciles rows written BEFORE that fix (move-only - * departments). Idempotent — runs on every boot next to the approvals - * backfill. + * departments). Idempotent — runs on every boot. */ export function reconcileDepartmentRegistry(db: DatabaseType): number { const missing = db @@ -787,15 +815,21 @@ function recognizedEmptyPrerelease(db: DatabaseType): boolean { return true; } -/** A v2 database created before some additive tables shipped (e.g. slice-1 - * pre-comments, slice-2 pre-relations/labels). Not a refusal and never a - * rebuild: `migrateWorkItemsSchema` creates the missing tables additively. - * Every table that IS present — additive or not — must shape-match exactly. */ -function recognizedV2MissingAdditiveTables(db: DatabaseType): boolean { +function hasShadowApprovalColumns(db: DatabaseType): boolean { + return sqlShape(currentTableSql(db, "work_items")) === sqlShape(V2_APPROVAL_WORK_ITEMS_TABLE_DDL); +} + +/** A v2 database whose only defects heal at boot: additive tables that shipped + * later are absent (e.g. slice-1 pre-comments), and/or work_items still carries + * the pre-PLA-48 approval columns. Never a refusal and never a rebuild. Every + * OTHER table that is present must shape-match exactly. */ +function recognizedHealableV2(db: DatabaseType): boolean { const additiveNames = new Set(V2_ADDITIVE_TABLES.map((table) => table.name)); + const shadowedApprovals = hasShadowApprovalColumns(db); const missing = V2_ADDITIVE_TABLES.filter((table) => !tableExists(db, table.name)); - if (missing.length === 0) return false; // nothing to heal — not this recognizer's case + if (missing.length === 0 && !shadowedApprovals) return false; // nothing to heal for (const [name, expected] of REQUIRED_TABLE_SQL) { + if (name === "work_items" && shadowedApprovals) continue; if (additiveNames.has(name) && !tableExists(db, name)) continue; if (sqlShape(currentTableSql(db, name)) !== sqlShape(expected)) return false; } @@ -978,7 +1012,7 @@ function classifyOpenWorkItemsDatabase(db: DatabaseType): WorkItemSchemaPrefligh } catch { // A v2 database missing additive tables (created before a later slice // shipped them) is "current": the migration creates them at boot. - if (recognizedV2MissingAdditiveTables(db)) return "current"; + if (recognizedHealableV2(db)) return "current"; if (recognizedV1(db)) return "v1"; if (todoTables.some((name) => name.startsWith("work_item_id_"))) refusal(); if (recognizedEmptyPrerelease(db)) return "empty-prerelease"; @@ -1023,17 +1057,22 @@ export function migrateWorkItemsSchema( ): WorkItemsMigrationResult { registerWorkItemIdentityFunctions(db); const migrate = db.transaction((): WorkItemsMigrationResult => { - // Additive self-heal, BEFORE classification: a v2 database created before a - // later slice shipped its additive tables gains them here so the exact-shape + // In-place self-heal, BEFORE classification: a v2 database created before a + // later slice shipped its additive tables (or before PLA-48 dropped the + // approval columns) is brought to the canonical shape here so the exact-shape // verifier below sees the complete v2 schema. IF NOT EXISTS makes this a // no-op everywhere else, and a refused classification rolls the creates back // with the transaction. Never a rebuild, never a refusal. (List order // matters: work_item_labels references labels.) - if (tableExists(db, "work_items") && sqlShape(currentTableSql(db, "work_items")) === sqlShape(WORK_ITEMS_TABLE_DDL)) { + const shadowedApprovals = hasShadowApprovalColumns(db); + if (shadowedApprovals || sqlShape(currentTableSql(db, "work_items")) === sqlShape(WORK_ITEMS_TABLE_DDL)) { for (const table of V2_ADDITIVE_TABLES) db.exec(table.ddl); - // Slice-4 dual-read: any item still carrying approval state ONLY in the - // frozen columns (a pre-slice-4 database) gains its history row here. - backfillWorkItemApprovals(db); + // PLA-48: a pre-drop database hands its shadowed approvals to their one + // owner and then loses the columns — same transaction, both or neither. + if (shadowedApprovals) { + backfillWorkItemApprovals(db, "work_items"); + for (const column of V2_APPROVAL_COLUMNS) db.exec(`ALTER TABLE work_items DROP COLUMN ${column}`); + } // Slice-5 review F2: departments that gained Todos through pre-fix // move-only writes get their registry rows. reconcileDepartmentRegistry(db); @@ -1097,19 +1136,16 @@ export function migrateWorkItemsSchema( id, title, body, status, department, assignee, created_by, parent_id, root_id, depth, due_at, priority, rank, version, source, source_ref, acceptance, verify_policy, rounds, budget_usd, - approval_state, approval_request, approval_ref, approval_target, approval_target_kind, - approval_escalated_at, approval_decided_by, approval_decided_at, created_at, updated_at, closed_at) SELECT id, title, body, status, department, assignee, CASE WHEN source = 'human' THEN 'operator' ELSE 'system' END, NULL, id, 0, NULL, priority, rank, version, source, source_ref, acceptance, verify_policy, rounds, budget_usd, - approval_state, approval_request, approval_ref, approval_target, approval_target_kind, - approval_escalated_at, approval_decided_by, approval_decided_at, created_at, updated_at, closed_at FROM work_items_v1_legacy`); - backfillWorkItemApprovals(db); + // Approvals come off the legacy row — read BEFORE the table is dropped. + backfillWorkItemApprovals(db, "work_items_v1_legacy"); reconcileDepartmentRegistry(db); // v1 rows carried departments with no registry const migratedRows = Number(db.prepare("SELECT COUNT(*) FROM work_items").pluck().get()); db.exec("DROP TABLE work_items_v1_legacy"); diff --git a/packages/jinn/src/work-items/store.ts b/packages/jinn/src/work-items/store.ts index 818127fd..485aa459 100644 --- a/packages/jinn/src/work-items/store.ts +++ b/packages/jinn/src/work-items/store.ts @@ -25,9 +25,9 @@ import { currentApproval, currentApprovalsByItem, type WorkItemApproval } from ' * always none — the §1.3 anti-bottleneck principle: creates cannot attach one), * and the append-only `work_item_events` audit. * - * Trust the DB, not just TS callers: status/priority/source/approval_state are - * enforced by CHECK constraints and machine-minted idempotency by a partial - * UNIQUE index (DDL in `migrate.ts`). + * Trust the DB, not just TS callers: status/priority/source are enforced by + * CHECK constraints and machine-minted idempotency by a partial UNIQUE index + * (DDL in `migrate.ts`). */ export type WorkItemStatus = @@ -230,7 +230,15 @@ function parseVerifyPolicy(raw: unknown): VerifyPolicy | null { } } -function rowToWorkItem(row: Record): WorkItem { +/** A work_items row as stored: everything on a WorkItem EXCEPT the approval + * facts, which live only in `work_item_approvals`. Producing a WorkItem + * therefore requires `overlayApproval` — a read path that skips hydration + * cannot silently serve all-null approvals, because it will not typecheck. */ +type WorkItemRowBase = Omit; + +function rowToWorkItem(row: Record): WorkItemRowBase { return { id: row.id as string, title: row.title as string, @@ -252,18 +260,6 @@ function rowToWorkItem(row: Record): WorkItem { verifyPolicy: parseVerifyPolicy(row.verify_policy), rounds: (row.rounds as number) ?? 0, budgetUsd: (row.budget_usd as number) ?? null, - approvalState: (row.approval_state as ApprovalState) ?? null, - approvalRequest: (row.approval_request as string) ?? null, - approvalRef: (row.approval_ref as string) ?? null, - // Frozen legacy columns never carried options; only the overlay sets them. - approvalOptions: null, - approvalChoice: null, - approvalOperatorOnly: false, - approvalTarget: (row.approval_target as string) ?? null, - approvalTargetKind: (row.approval_target_kind as ApprovalTargetKind) ?? null, - approvalEscalatedAt: (row.approval_escalated_at as string) ?? null, - approvalDecidedBy: (row.approval_decided_by as string) ?? null, - approvalDecidedAt: (row.approval_decided_at as string) ?? null, createdAt: row.created_at as string, updatedAt: row.updated_at as string, closedAt: (row.closed_at as string) ?? null, @@ -271,34 +267,31 @@ function rowToWorkItem(row: Record): WorkItem { } /** - * Slice-4 dual-read seam: the legacy `approval_*` columns are FROZEN (never - * written after the backfill), so every WorkItem leaving this module overlays - * them from the item's current `work_item_approvals` row. `rowToWorkItem` itself - * still maps the raw columns — the overlay is applied explicitly at the read - * functions (single reads hydrate per item; page/tree reads batch), which keeps - * EVERY consumer (payloads, authority checks, activity cards, transitions' - * returns) byte-identical to the pre-slice column-backed values. + * The ONLY producer of a WorkItem's approval fields: the item's current + * `work_item_approvals` row, or "no approval" when it has none. Applied + * explicitly at every read function (single reads hydrate per item; page/tree + * reads batch), which keeps EVERY consumer — payloads, authority checks, + * activity cards, transitions' returns — sourcing approvals from one place. */ -function overlayApproval(item: WorkItem, row: WorkItemApproval | undefined): WorkItem { - if (!row) return item; +function overlayApproval(base: WorkItemRowBase, row: WorkItemApproval | undefined): WorkItem { return { - ...item, - approvalState: row.state, - approvalRequest: row.request, - approvalRef: row.ref, - approvalOptions: row.options, - approvalChoice: row.choice, - approvalOperatorOnly: row.operatorOnly, - approvalTarget: row.target, - approvalTargetKind: row.targetKind, - approvalEscalatedAt: row.escalatedAt, - approvalDecidedBy: row.decidedBy, - approvalDecidedAt: row.decidedAt, + ...base, + approvalState: row?.state ?? null, + approvalRequest: row?.request ?? null, + approvalRef: row?.ref ?? null, + approvalOptions: row?.options ?? null, + approvalChoice: row?.choice ?? null, + approvalOperatorOnly: row?.operatorOnly ?? false, + approvalTarget: row?.target ?? null, + approvalTargetKind: row?.targetKind ?? null, + approvalEscalatedAt: row?.escalatedAt ?? null, + approvalDecidedBy: row?.decidedBy ?? null, + approvalDecidedAt: row?.decidedAt ?? null, }; } -function hydrateApprovals(items: WorkItem[]): WorkItem[] { - if (items.length === 0) return items; +function hydrateApprovals(items: WorkItemRowBase[]): WorkItem[] { + if (items.length === 0) return []; const currentByItem = currentApprovalsByItem(items.map((item) => item.id)); return items.map((item) => overlayApproval(item, currentByItem.get(item.id))); } @@ -550,7 +543,7 @@ export function createWorkItem(input: CreateWorkItemInput): WorkItem { .prepare('SELECT * FROM work_items WHERE source = ? AND source_ref = ?') .get(source, sourceRef) as Record | undefined; // Overlay like every other WorkItem-producing read: a retried machine mint - // can hit an item that has since gained an approval (dual-read seam). + // can hit an item that has since gained an approval. return row ? overlayApproval(rowToWorkItem(row), currentApproval(row.id as string)) : undefined; }; @@ -733,8 +726,8 @@ function workItemWhere(filter: ListWorkItemsFilter): { sql: string; values: unkn values.push(filter.label, filter.label); } if (filter.needsAttentionFor) { - // Pending approvals live in work_item_approvals (slice 4) — the frozen - // approval_* columns are no longer consulted anywhere. + // Approvals live in work_item_approvals — their sole storage owner since + // PLA-48 dropped the shadow columns from work_items. conditions.push( "(EXISTS (SELECT 1 FROM work_item_approvals wap WHERE wap.work_item_id = work_items.id AND wap.state = 'pending' AND wap.target = ?) OR (assignee = ? AND status IN ('blocked', 'escalated')))", );