Preserve historical owners when retrying ledger writes

Author: mchwang

docs/implementation/persistent-review-store.md

@@ -33,3 +33,7 @@ Build the review screen under #3 next. The real-issue assignment, experiment pro
 ## Review round 1
 
 Copilot's summary identified two findings (no inline threads). Both were reproduced with failing regressions, then fixed: immutable ledger comparisons now compare the typed fields rather than JSON property order, and a checkpoint's item must be the last item in its completed prefix. No findings were declined. Final suite: 151 tests.
+
+## Review round 2
+
+Reproduced a historical-owner retry failure after an amendment removed the owning item. Normal history writes now apply current-item validation only to new ledger entries; existing SHAs still pass the immutable field comparison. The regression also checks that new commits cannot claim the removed owner and that rebases retain historical ownership. No findings declined. Final suite: 152 tests.

runner/store.ts

@@ -176,7 +176,8 @@ export class Store {
       this.#expect(key, expected);
       const plan = this.getPlan(identity);
       for (const entry of entries) {
-        if (entry.owner !== null && !plan.items.some(item => item.id === entry.owner)) throw new Error('Unknown ledger owner.');
+        const existing = this.#get('SELECT sha FROM ledger WHERE key=? AND sha=?', key, entry.sha);
+        if (!existing && entry.owner !== null && !plan.items.some(item => item.id === entry.owner)) throw new Error('Unknown ledger owner.');
         this.#entry(key, entry);
       }
       return this.#snapshot(key, base, head);

test/store.test.ts

@@ -176,3 +176,12 @@ it('requires the checkpoint item to be the last item in the completed prefix', (
   expect(() => store.recordCheckpoint(identity, state(store), { item: 'P1', completedItems: ['P1', 'P2'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries })).toThrow(/prefix/);
   expect(store.recordCheckpoint(identity, state(store), { item: 'P2', completedItems: ['P1', 'P2'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries }).item).toBe('P2');
 });
+it('allows historical ledger retries after the owning item is removed, but rejects new entries for it', () => {
+  const { store } = fixture(); const entry = { sha: oid(2), owner: 'P1', origin: 'owned' as const, sourceSha: null };
+  store.recordHistory(identity, state(store), oid(1), oid(2), [entry]);
+  const next = plan(); next.items[0]!.id = 'P2'; store.importRevision(JSON.stringify(next), 'json', context, 1);
+  expect(() => store.recordHistory(identity, state(store), oid(1), oid(2), [entry])).not.toThrow();
+  expect(() => store.recordHistory(identity, state(store), oid(1), oid(3), [{ ...entry, sha: oid(3) }])).toThrow(/Unknown ledger owner/);
+  store.recordRebase(identity, state(store), oid(4), oid(5), [{ oldSha: oid(2), newSha: oid(5) }]);
+  expect(store.ownership(identity).get(oid(5))).toBe('P1');
+});