Expire segment assignments when their plan item is removed

Author: mchwang

docs/implementation/persistent-review-store.md

@@ -43,3 +43,7 @@ Reproduced a historical-owner retry failure after an amendment removed the ownin
 Reproduced continuation reapproval failing after a second plan amendment. Continuation records now include revision in their immutable primary key; the getter returns the latest approved revision, leaving earlier approvals intact. Repeating approval for the same revision is idempotent, with a separately reproduced regression. This changes the unreleased schema introduced by this PR, not a released database format.
 
 Declined the duplicate-source-mapping finding as a correctness issue: the rewrites primary key already rejects a repeated source within a snapshot, and the surrounding transaction rolls back its new snapshot and all ledger writes. A new regression passed before any implementation change and confirms the unchanged snapshot, empty ledger, and absence of mappings after rejection. Final suite: 154 tests.
+
+## Review round 4
+
+Reproduced a saved assignment throwing in `applyChoices` after its target item was removed by amendment. Revision commits now discard assignments whose targets no longer exist, in the same transaction as the revision and request invalidation. Unrelated assignments and standalone acceptances remain. Removed assignments cannot reactivate if an ID is later reintroduced. No finding declined this round. Final suite: 155 tests.

runner/store.ts

@@ -79,6 +79,12 @@ export class Store {
     if (this.#run('UPDATE plans SET revision=? WHERE key=? AND revision=?', plan.revision, key, expected).changes !== 1) throw new Error('Stale plan revision.');
     this.#run('INSERT INTO revisions VALUES (?,?,?)', key, plan.revision, encode(plan));
     this.#run("UPDATE requests SET state='invalidated' WHERE key=? AND state IN ('pending','ready')", key);
+    const items = new Set(plan.items.map(item => item.id));
+    for (const row of this.#db.prepare('SELECT choice_key,data FROM choices WHERE key=?').all(key)) {
+      const choice = decode<SegmentChoice>(row.data);
+      if (choice.action === 'assign' && !items.has(choice.item!))
+        this.#run('DELETE FROM choices WHERE key=? AND choice_key=?', key, row.choice_key!);
+    }
   }
   createPlan(source: string | Uint8Array, format: 'json' | 'yaml', context: PlanContext, base: string, head: string): Plan {
     sha(base); sha(head);

test/store.test.ts

@@ -6,7 +6,7 @@ import { once } from 'node:events';
 import { afterEach, expect, it } from 'vitest';
 import { Store, requireSupportedNode } from '../runner/store.ts';
 import type { Plan, PlanContext, EditReply } from '../core/plan.ts';
-import { approveItem, approvalStates, choiceKeys } from '../core/approvals.ts';
+import { approveItem, approvalStates, choiceKeys, applyChoices } from '../core/approvals.ts';
 import { linkHistory, type Segment } from '../core/linking.ts';
 import { readHistory } from '../git/history.ts';
 const identity = { repositoryId: 'repo', taskId: 'task', planId: 'plan' };
@@ -200,3 +200,13 @@ it('rejects duplicate source SHA mappings and rolls back every resulting ledger/
   expect(store.getSnapshot(identity)).toEqual(before); expect(store.getLedger(identity)).toEqual([]);
   expect(store.getRewrites(identity, before.id)).toEqual([]);
 });
+it('expires assignments to removed plan items atomically with the amendment', () => {
+  const { store } = fixture();
+  const segment: Segment = { path: 'a', oldPath: 'a', kind: 'text', operation: '+', content: 'after\n', context: '', owners: [null], row: 'Unplanned', scope: 'unplanned', oldLine: null, newLine: 1, hunk: 0, sharesHunkWith: [] };
+  store.saveReview(identity, state(store), [], [{ key: choiceKeys([segment], identity)[0]!, action: 'assign', item: 'P1' }]);
+  const next = plan(); next.items[0]!.id = 'P2'; store.importRevision(JSON.stringify(next), 'json', context, 1);
+  expect(() => applyChoices(store.getPlan(identity), [segment], store.getReview(identity).choices, identity)).not.toThrow();
+  expect(store.getReview(identity).choices).toEqual([]);
+  store.importRevision(JSON.stringify(plan()), 'json', context, 2);
+  expect(store.getReview(identity).choices).toEqual([]);
+});