From 37b9ae2417073e11adb39ef50cdd10dcaf95e32c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 3 Oct 2026 09:17:48 +0200 Subject: [PATCH 1/2] fix: retain session and registration ownership through review controls --- .../src/durable-capture/transitions.test.ts | 9 ++++- .../daemon-registration-owner.test.ts | 21 +++++----- src/__tests__/test-utils/store-factory.ts | 10 +++-- .../__tests__/session-capture-binding.test.ts | 5 ++- .../__tests__/session-store-lifetime.test.ts | 40 ++++++++++++++++++- .../screen-recording-session-binding.ts | 3 +- src/daemon/session-store.ts | 7 ++-- src/session-repair-tombstone.ts | 26 ++++++++---- 8 files changed, 90 insertions(+), 31 deletions(-) diff --git a/packages/capture-kit/src/durable-capture/transitions.test.ts b/packages/capture-kit/src/durable-capture/transitions.test.ts index 0ff01c1062..d50c0e3c02 100644 --- a/packages/capture-kit/src/durable-capture/transitions.test.ts +++ b/packages/capture-kit/src/durable-capture/transitions.test.ts @@ -220,7 +220,7 @@ test('a disposal finish disposes a preserving kind’s material too', async () = }); }); -test.each(['rebuild', 'retire', 'token', 'generation'] as const)( +test.each(['rebuild', 'retire', 'handle', 'token', 'generation'] as const)( 'a held finish after %s clears only its matching lifetime, handle and fence', async (change) => { const context = makeDurableCaptureContext(); @@ -270,7 +270,12 @@ test.each(['rebuild', 'retire', 'token', 'generation'] as const)( context.sessionStore.update(ref, (current) => ({ ...current, name: 'updated', - capture: { ...active, envelope: { ...active.envelope, fence } }, + capture: { + ...active, + handle: + change === 'handle' ? makeDurableCaptureStartResult(context).handle : active.handle, + envelope: { ...active.envelope, fence }, + }, })); } const before = context.sessionStore.get(context.sessionName)!; diff --git a/src/__tests__/daemon-registration-owner.test.ts b/src/__tests__/daemon-registration-owner.test.ts index 5426a07764..8af5ffc729 100644 --- a/src/__tests__/daemon-registration-owner.test.ts +++ b/src/__tests__/daemon-registration-owner.test.ts @@ -484,12 +484,15 @@ async function waitForCutoverFixture(ready: () => boolean): Promise { assert.fail('cutover fixture did not reach its barrier'); } -function legacyDisposition(paths: DaemonPaths): boolean { - return ( - JSON.parse(fs.readFileSync(path.join(paths.baseDir, 'legacy-disposition.json'), 'utf8')) as { - acquired: boolean; - } - ).acquired; +function legacyDisposition(paths: DaemonPaths): boolean | undefined { + try { + const parsed = JSON.parse( + fs.readFileSync(path.join(paths.baseDir, 'legacy-disposition.json'), 'utf8'), + ) as { acquired?: unknown }; + return typeof parsed.acquired === 'boolean' ? parsed.acquired : undefined; + } catch { + return undefined; + } } test('cutover refuses an already-running legacy daemon before signaling or changing registration', async () => { @@ -542,9 +545,7 @@ test('a legacy contender cannot unlink a hardened owner while it delays metadata try { await waitForCutoverFixture(() => fs.existsSync(path.join(paths.baseDir, 'registration-held'))); legacy = spawnLegacyDaemonFixture(paths); - await waitForCutoverFixture(() => - fs.existsSync(path.join(paths.baseDir, 'legacy-disposition.json')), - ); + await waitForCutoverFixture(() => legacyDisposition(paths) !== undefined); assert.equal(legacyDisposition(paths), false); await legacy.exited; const claim = inspectProcessLock(paths.lockPath); @@ -580,7 +581,7 @@ test('concurrent old and new daemon startup has one owner at the shared lock pat fs.writeFileSync(barrier, 'start'); await waitForCutoverFixture( () => - fs.existsSync(path.join(paths.baseDir, 'legacy-disposition.json')) && + legacyDisposition(paths) !== undefined && (currentExited || fs.existsSync(path.join(paths.baseDir, 'registration-held'))), ); const oldAcquired = legacyDisposition(paths); diff --git a/src/__tests__/test-utils/store-factory.ts b/src/__tests__/test-utils/store-factory.ts index cc5a128879..c646a9b861 100644 --- a/src/__tests__/test-utils/store-factory.ts +++ b/src/__tests__/test-utils/store-factory.ts @@ -12,9 +12,13 @@ export function makeStoredSessionRef(session: SessionState, address = session.na return makeSessionStore().publish(address, session); } -export function storeSessionForTest(store: SessionStore, session: SessionState): SessionRef { - const ref = store.lookup(session.name); - if (!ref) return store.publish(session.name, session); +export function storeSessionForTest( + store: SessionStore, + session: SessionState, + address = session.name, +): SessionRef { + const ref = store.lookup(address); + if (!ref) return store.publish(address, session); if (ref.session !== session) throw new Error('A different test session occupies this address'); return ref; } diff --git a/src/daemon/__tests__/session-capture-binding.test.ts b/src/daemon/__tests__/session-capture-binding.test.ts index 1058b0f354..a4a0d5039a 100644 --- a/src/daemon/__tests__/session-capture-binding.test.ts +++ b/src/daemon/__tests__/session-capture-binding.test.ts @@ -73,9 +73,10 @@ test('clearing an older handle or fence leaves a replacement capture intact', () sessionStore: store, finish: async () => ({ status: 'cleanup-pending', reason: 'cleanup-unconfirmed' }), }).screenRecording!; - store.update(ref, { screenRecording: replacement }); + const differentHandle = { ...active, handle: replacement.handle }; + store.update(ref, { screenRecording: differentHandle }); expect(binding.clear(active)).toBe('resource-changed'); - expect(binding.read()).toBe(replacement); + expect(binding.read()).toBe(differentHandle); for (const fence of [ { ...active.envelope.fence, token: 'next' }, { ...active.envelope.fence, generation: active.envelope.fence.generation + 1 }, diff --git a/src/daemon/__tests__/session-store-lifetime.test.ts b/src/daemon/__tests__/session-store-lifetime.test.ts index f834cc2f96..ef1315fa6d 100644 --- a/src/daemon/__tests__/session-store-lifetime.test.ts +++ b/src/daemon/__tests__/session-store-lifetime.test.ts @@ -8,7 +8,8 @@ import { makeRepairArmedSession, authoringPublication, } from '../../__tests__/test-utils/session-factories.ts'; -import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { makeSessionStore, storeSessionForTest } from '../../__tests__/test-utils/store-factory.ts'; +import { resolveRepairTombstonePath } from '../../session-repair-tombstone.ts'; const ADDRESS = 'cwd:worktree:default'; @@ -171,3 +172,40 @@ test('repair tombstones follow the scoped address and cannot be written by a ret assert.equal(store.readRepairTombstone(ADDRESS), undefined); assert.equal(store.requireCurrent(successor), successor.session); }); + +test('test session publication uses its explicit scoped address', () => { + const store = makeSessionStore(); + const session = makeSession('default'); + const ref = store.publish(ADDRESS, session); + const stored = storeSessionForTest(store, session, ADDRESS); + assert.equal(stored.lifetime, ref.lifetime); + assert.equal(stored.session, session); + assert.equal(store.get('default'), undefined); + assert.equal(store.listRefs().length, 1); +}); + +test('colliding artifact directories cannot share or clear another address\u2019s repair tombstone', () => { + const store = makeSessionStore(); + const collision = 'cwd_worktree_default'; + const ref = store.publish(ADDRESS, makeRepairArmedSession('default')); + assert.equal(store.resolveSessionDir(ADDRESS), store.resolveSessionDir(collision)); + store.writeRepairTombstone(ref); + assert.equal(store.readRepairTombstone(ADDRESS)?.owner, ADDRESS); + assert.equal(store.readRepairTombstone(collision), undefined); + store.clearRepairTombstone(collision); + assert.equal(store.readRepairTombstone(ADDRESS)?.owner, ADDRESS); + store.clearRepairTombstone(ADDRESS); + assert.equal(store.readRepairTombstone(ADDRESS), undefined); +}); + +test('tombstone cleanup retains malformed evidence and removes an expired owned marker', () => { + const store = makeSessionStore(); + const tombstonePath = resolveRepairTombstonePath(store.resolveSessionDir(ADDRESS)); + fs.mkdirSync(store.resolveSessionDir(ADDRESS), { recursive: true }); + fs.writeFileSync(tombstonePath, '{'); + store.clearRepairTombstone(ADDRESS); + assert.equal(fs.readFileSync(tombstonePath, 'utf8'), '{'); + fs.writeFileSync(tombstonePath, JSON.stringify({ owner: ADDRESS, expiresAt: 0, reapedAt: 0 })); + store.clearRepairTombstone(ADDRESS); + assert.equal(fs.existsSync(tombstonePath), false); +}); diff --git a/src/daemon/screen-recording-session-binding.ts b/src/daemon/screen-recording-session-binding.ts index b5d56813d0..4c4505b3ee 100644 --- a/src/daemon/screen-recording-session-binding.ts +++ b/src/daemon/screen-recording-session-binding.ts @@ -17,8 +17,7 @@ export function bindRecordOnlyScreenRecording( canPersist: () => !published && sessionStore.lookup(address) === undefined, adopt: (screenRecording) => { sessionStore.assertPublishable(address); - draft.screenRecording = screenRecording; - published = sessionStore.publish(address, draft); + published = sessionStore.publish(address, { ...draft, screenRecording }); }, clear: (expected) => published ? bindSessionScreenRecording(sessionStore, published).clear(expected) : 'retired', diff --git a/src/daemon/session-store.ts b/src/daemon/session-store.ts index 00baf86df3..e6818d4071 100644 --- a/src/daemon/session-store.ts +++ b/src/daemon/session-store.ts @@ -12,6 +12,7 @@ import { } from './session-artifact-paths.ts'; import { readRepairTombstoneFile, + clearRepairTombstoneFile, resolveRepairTombstonePath, type RepairSessionTombstone, } from '../session-repair-tombstone.ts'; @@ -357,14 +358,12 @@ export class SessionStore { /** Returns a non-expired repair tombstone for `sessionName`, or `undefined`. */ readRepairTombstone(sessionName: string): RepairSessionTombstone | undefined { - return readRepairTombstoneFile(this.repairTombstonePath(sessionName)); + return readRepairTombstoneFile(this.repairTombstonePath(sessionName), sessionName); } /** ADR 0012 R7 (C5a): a fresh `replay --save-script` on this key clears the tombstone. */ clearRepairTombstone(sessionName: string): void { - try { - fs.rmSync(this.repairTombstonePath(sessionName), { force: true }); - } catch {} + clearRepairTombstoneFile(this.repairTombstonePath(sessionName), sessionName); } /** diff --git a/src/session-repair-tombstone.ts b/src/session-repair-tombstone.ts index a0fae0461b..dedf854736 100644 --- a/src/session-repair-tombstone.ts +++ b/src/session-repair-tombstone.ts @@ -31,15 +31,28 @@ export function resolveRepairTombstonePath(sessionDir: string): string { } /** Parses/validates a tombstone file at `tombstonePath`; `undefined` if missing, malformed, or expired. */ -export function readRepairTombstoneFile(tombstonePath: string): RepairSessionTombstone | undefined { +export function readRepairTombstoneFile( + tombstonePath: string, + owner: string, +): RepairSessionTombstone | undefined { try { - return readRepairTombstoneForCleanup(tombstonePath); + const tombstone = readRepairTombstone(tombstonePath); + return tombstone?.owner === owner && tombstone.expiresAt > Date.now() ? tombstone : undefined; } catch { return undefined; } } -function readRepairTombstoneForCleanup(tombstonePath: string): RepairSessionTombstone | undefined { +/** Removes only a parseable marker belonging to the requested session, including expired markers. */ +export function clearRepairTombstoneFile(tombstonePath: string, owner: string): void { + try { + if (readRepairTombstone(tombstonePath)?.owner === owner) { + fs.rmSync(tombstonePath, { force: true }); + } + } catch {} +} + +function readRepairTombstone(tombstonePath: string): RepairSessionTombstone | undefined { let raw: string; try { raw = fs.readFileSync(tombstonePath, 'utf8'); @@ -47,8 +60,7 @@ function readRepairTombstoneForCleanup(tombstonePath: string): RepairSessionTomb if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined; throw error; } - const parsed = parseRepairTombstone(raw, tombstonePath); - return parsed.expiresAt > Date.now() ? parsed : undefined; + return parseRepairTombstone(raw, tombstonePath); } function parseRepairTombstone(raw: string, tombstonePath: string): RepairSessionTombstone { @@ -110,10 +122,10 @@ export function findUnrecoveredRepairCommitFailure(sessionsDir: string): } for (const entry of entries) { if (!entry.isDirectory()) continue; - const tombstone = readRepairTombstoneForCleanup( + const tombstone = readRepairTombstone( resolveRepairTombstonePath(path.join(sessionsDir, entry.name)), ); - if (tombstone?.commitFailure) { + if (tombstone?.commitFailure && tombstone.expiresAt > Date.now()) { return { sessionName: entry.name, tombstone: { ...tombstone, commitFailure: tombstone.commitFailure }, From 6c3e88b95d175bc78f8691e86618e99351d30907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 3 Oct 2026 09:17:48 +0200 Subject: [PATCH 2/2] chore(gates): declare the record-only capture publication owner --- scripts/layering/session-resource-ownership.test.ts | 4 ++++ scripts/layering/session-resource-ownership.ts | 1 + 2 files changed, 5 insertions(+) diff --git a/scripts/layering/session-resource-ownership.test.ts b/scripts/layering/session-resource-ownership.test.ts index 8a1453e6cb..a85922db6c 100644 --- a/scripts/layering/session-resource-ownership.test.ts +++ b/scripts/layering/session-resource-ownership.test.ts @@ -32,6 +32,10 @@ test('session resources are constructed only by their durable domain owners', () sessionStore.update(ref, { perfCapture: perf }); sessionStore.update(ref, { screenRecording: recording });`, ], + [ + 'src/daemon/screen-recording-session-binding.ts', + `sessionStore.publish(address, { ...draft, screenRecording });`, + ], [ 'packages/capture-kit/src/capture-admission/audio-probe-session-resource.ts', `sessionStore.set(name, { ...session, audioProbe: audio });`, diff --git a/scripts/layering/session-resource-ownership.ts b/scripts/layering/session-resource-ownership.ts index 323d8af3be..77aa399444 100644 --- a/scripts/layering/session-resource-ownership.ts +++ b/scripts/layering/session-resource-ownership.ts @@ -30,6 +30,7 @@ const RESOURCE_OWNERS: Readonly>> = { audioProbe: new Set(['src/daemon/session-capture-binding.ts', 'src/daemon/session-state.ts']), screenRecording: new Set([ 'src/daemon/session-capture-binding.ts', + 'src/daemon/screen-recording-session-binding.ts', 'src/daemon/session-state.ts', ]), perfCapture: new Set(['src/daemon/session-capture-binding.ts', 'src/daemon/session-state.ts']),