refactor: carry session lifetimes through close and script finalization

Author: thymikee

src/__tests__/test-utils/store-factory.ts

@@ -11,3 +11,10 @@ export function makeSessionStore(prefix = 'agent-device-test-'): SessionStore {
 export function makeStoredSessionRef(session: SessionState, address = session.name): SessionRef {
   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);
+  if (ref.session !== session) throw new Error('A different test session occupies this address');
+  return ref;
+}

src/daemon/__tests__/filesystem-boundary-faults.test.ts

@@ -1,3 +1,4 @@
+import { storeSessionForTest } from '../../__tests__/test-utils/store-factory.ts';
 import assert from 'node:assert/strict';
 import crypto from 'node:crypto';
 import path from 'node:path';
@@ -158,7 +159,7 @@ function createSessionStoreFixture(root: string): FilesystemBoundaryFixture {
 
   return {
     targetPath,
-    run: async () => store.finalizeRepairTeardown(session),
+    run: async () => store.finalizeRepairTeardown(storeSessionForTest(store, session)),
     expected: 'return',
     verifyReturn: (_value, errno) => {
       const tombstone = store.readRepairTombstone(session.name);

src/daemon/__tests__/replay-repair/session-replay-repair-acceptance.test.ts

@@ -1,3 +1,4 @@
+import { storeSessionForTest } from '../../../__tests__/test-utils/store-factory.ts';
 /**
  * ADR 0012 decision 6 acceptance test: a healed sibling `.ad` produced by the
  * repair loop must replay end-to-end in a FRESH session, with every selector
@@ -140,7 +141,7 @@ test('a healed script survives repair + fresh-session replay: self-contained ope
   // repair-armed write on the same explicit finalize signal `close
   // --save-script` sets). ---
   markRepairTransactionComplete(session);
-  sessionStore.writeSessionLog(session);
+  sessionStore.writeSessionLog(storeSessionForTest(sessionStore, session));
   const healedPath = path.join(root, 'flow.healed.ad');
   expect(fs.existsSync(healedPath)).toBe(true);
   const healedScript = fs.readFileSync(healedPath, 'utf8');

src/daemon/__tests__/replay-repair/session-replay-repair-empty-tail.test.ts

@@ -1,3 +1,4 @@
+import { storeSessionForTest } from '../../../__tests__/test-utils/store-factory.ts';
 /**
  * ADR 0012 decision 6, R2/R3, extended per #1262: behaviors introduced
  * alongside the `resume.from` / `repairHint` agreement fix
@@ -188,7 +189,7 @@ test('a record-and-heal divergence on the LAST step resumes with an empty tail a
   // --- Commit: the transaction is COMPLETE, so the healed script actually
   // publishes — the corrective press survives, "click" (never recorded) does
   // not. Proves the empty-tail resume did not lead to a discarded repair. ---
-  const writeResult = sessionStore.writeSessionLog(session);
+  const writeResult = sessionStore.writeSessionLog(storeSessionForTest(sessionStore, session));
   expect(writeResult.written).toBe(true);
   const healedPath = path.join(root, 'flow.healed.ad');
   expect(fs.existsSync(healedPath)).toBe(true);
@@ -312,7 +313,7 @@ test('a manual divergence (unannotated action-failure) on the LAST step resumes
   // since a `manual` divergence never dispatched it) does not. Proves the
   // empty-tail resume did not lead to a discarded repair (the #1260
   // discard-at-close trap, now also closed for `manual`). ---
-  const writeResult = sessionStore.writeSessionLog(session);
+  const writeResult = sessionStore.writeSessionLog(storeSessionForTest(sessionStore, session));
   expect(writeResult.written).toBe(true);
   const healedPath = path.join(root, 'flow.healed.ad');
   expect(fs.existsSync(healedPath)).toBe(true);
@@ -436,7 +437,7 @@ test('a caution (identity-mismatch) divergence on the LAST step resumes with an
 
   // --- Commit: COMPLETE, so the healed script publishes the corrective
   // press; the pre-action "click" (never dispatched) does not appear. ---
-  const writeResult = sessionStore.writeSessionLog(session);
+  const writeResult = sessionStore.writeSessionLog(storeSessionForTest(sessionStore, session));
   expect(writeResult.written).toBe(true);
   const healedPath = path.join(root, 'flow.healed.ad');
   expect(fs.existsSync(healedPath)).toBe(true);

src/daemon/__tests__/replay-repair/session-replay-repair-transaction-close-ordering.test.ts

@@ -1,3 +1,4 @@
+import { storeSessionForTest } from '../../../__tests__/test-utils/store-factory.ts';
 /**
  * ADR 0012 decision 6 repair-transaction close-ordering guarantees (BLOCKER 2/3 sequencing): the
  * platform close must run and succeed BEFORE the healed `.ad` commits (never claim a successful
@@ -244,7 +245,7 @@ test('BLOCKER 3: a competing second writer never overwrites a COMPLETE artifact
 
   // Writer 1 commits a complete artifact at the default healed path.
   const first = makeCompleteRepairSession(sessionStore, `${sessionName}-1`, root);
-  const r1 = sessionStore.writeSessionLog(first);
+  const r1 = sessionStore.writeSessionLog(storeSessionForTest(sessionStore, first));
   expect(r1.written).toBe(true);
   const committed = fs.readFileSync(healedPath, 'utf8');
   expect(committed).toContain(HEAL_COMPLETE_SENTINEL);
@@ -261,7 +262,7 @@ test('BLOCKER 3: a competing second writer never overwrites a COMPLETE artifact
     result: { selectorChain: ['id="different"'] },
     targetEvidence: freshEvidence('different', 'Different'),
   };
-  const r2 = sessionStore.writeSessionLog(second);
+  const r2 = sessionStore.writeSessionLog(storeSessionForTest(sessionStore, second));
   expect(r2.written).toBe(false);
   expect(r2.written === false && r2.error?.message).toMatch(/already exists/);
   // The first writer's complete artifact is byte-for-byte intact.

src/daemon/__tests__/replay-repair/session-replay-repair-transaction.test.ts

@@ -1,3 +1,4 @@
+import { storeSessionForTest } from '../../../__tests__/test-utils/store-factory.ts';
 /**
  * ADR 0012 decision 6 "repair transaction" lifecycle fixes (Q1/Q2a/Q2b/Q2c):
  * proves the WHOLE chain end to end, at the layer these fixes actually live —
@@ -304,7 +305,7 @@ test('C5a: an incomplete repair reaped by idle-reap leaves a tombstone (no heale
   // Idle-reap tears the still-incomplete repair session down: the writer commits
   // nothing (not complete) and a tombstone is left behind (the exact teardown
   // step daemon-runtime.ts's teardownDaemonSession runs).
-  sessionStore.finalizeRepairTeardown(session);
+  sessionStore.finalizeRepairTeardown(storeSessionForTest(sessionStore, session));
   sessionStore.delete(sessionName);
   expect(fs.existsSync(path.join(root, 'flow.healed.ad'))).toBe(false);
 
@@ -353,7 +354,7 @@ test('C5a/BLOCKER 3: teardown of a COMPLETE repair auto-commits a self-contained
 
   // Teardown (e.g. the client tearing down the ephemeral daemon after a clean
   // repair) auto-commits the completed transaction and leaves no tombstone.
-  sessionStore.finalizeRepairTeardown(session);
+  sessionStore.finalizeRepairTeardown(storeSessionForTest(sessionStore, session));
   expect(fs.existsSync(path.join(root, 'flow.healed.ad'))).toBe(true);
   const healedScript = fs.readFileSync(path.join(root, 'flow.healed.ad'), 'utf8');
   expect(healedScript).toContain(HEAL_COMPLETE_SENTINEL);
@@ -398,7 +399,7 @@ test('BLOCKER 1: a --from continuation on a reaped session returns SESSION_NOT_F
   const digest = leg1Divergence.resume.planDigest;
 
   // Idle-reap tears the incomplete repair down, leaving a tombstone.
-  sessionStore.finalizeRepairTeardown(sessionStore.get(sessionName)!);
+  sessionStore.finalizeRepairTeardown(sessionStore.lookup(sessionName)!);
   sessionStore.delete(sessionName);
   expect(sessionStore.readRepairTombstone(sessionName)).toBeDefined();
 

src/daemon/__tests__/request-router-idle-expired.test.ts

@@ -95,8 +95,7 @@ test('an expired marker that has aged out stops explaining the absence', async (
 
 test('an abandoned repair transaction outranks the idle-expiry marker', async () => {
   const { sessionStore, handler } = makeHandler('agent-device-router-idle-vs-repair-');
-  writeIdleMarker(sessionStore, 'repair-x');
-  sessionStore.writeRepairTombstone({
+  const ref = sessionStore.publish('repair-x', {
     name: 'repair-x',
     device: { platform: 'apple', id: 'sim-1', name: 'iPhone', kind: 'simulator', booted: true },
     createdAt: Date.now(),
@@ -110,6 +109,10 @@ test('an abandoned repair transaction outranks the idle-expiry marker', async ()
     },
   });
 
+  sessionStore.writeRepairTombstone(ref);
+  sessionStore.retire(ref);
+  writeIdleMarker(sessionStore, 'repair-x');
+
   const response = await handler(closeRequest('repair-x'));
 
   expect(response.ok).toBe(false);

src/daemon/__tests__/request-router-repair-expired.test.ts

@@ -62,7 +62,9 @@ test('a command that finds no session but hits a live repair tombstone gets REPA
   const { sessionStore, handler } = makeHandler('agent-device-router-repair-expired-');
   // The repair session was reaped (idle-reap) leaving a tombstone; the store
   // has no live session by that name.
-  sessionStore.writeRepairTombstone(tombstonedSession('repair-x'));
+  const ref = sessionStore.publish('repair-x', tombstonedSession('repair-x'));
+  sessionStore.writeRepairTombstone(ref);
+  sessionStore.retire(ref);
 
   const response = await handler(closeRequest('repair-x'));
 
@@ -89,10 +91,12 @@ test('without a tombstone, a missing session still returns a plain SESSION_NOT_F
 // never completed at all.
 test('a command hitting a commit-failure tombstone gets REPAIR_COMMIT_FAILED with the real cause, not a generic REPAIR_SESSION_EXPIRED', async () => {
   const { sessionStore, handler } = makeHandler('agent-device-router-commit-failed-');
-  sessionStore.writeRepairTombstone(tombstonedSession('repair-commit-fail'), undefined, {
+  const ref = sessionStore.publish('repair-commit-fail', tombstonedSession('repair-commit-fail'));
+  sessionStore.writeRepairTombstone(ref, undefined, {
     code: 'COMMAND_FAILED',
     message: 'A prior healed script already exists at /flows/login.healed.ad; ...',
   });
+  sessionStore.retire(ref);
 
   const response = await handler(closeRequest('repair-commit-fail'));
 
@@ -108,7 +112,9 @@ test('a command hitting a commit-failure tombstone gets REPAIR_COMMIT_FAILED wit
 test('an expired tombstone does not shadow a missing session', async () => {
   const { sessionStore, handler } = makeHandler('agent-device-router-expired-tombstone-');
   // TTL 0 => already stale.
-  sessionStore.writeRepairTombstone(tombstonedSession('repair-y'), 0);
+  const ref = sessionStore.publish('repair-y', tombstonedSession('repair-y'));
+  sessionStore.writeRepairTombstone(ref, 0);
+  sessionStore.retire(ref);
 
   const response = await handler(closeRequest('repair-y'));
 
@@ -144,7 +150,9 @@ test('a replay --from continuation on a reaped repair session gets REPAIR_SESSIO
   }).planDigest;
 
   // The repair session was reaped, leaving a tombstone; no live session exists.
-  sessionStore.writeRepairTombstone(tombstonedSession('repair-from'));
+  const ref = sessionStore.publish('repair-from', tombstonedSession('repair-from'));
+  sessionStore.writeRepairTombstone(ref);
+  sessionStore.retire(ref);
 
   const response = await handler({
     token: 'test-token',

src/daemon/__tests__/request-save-script-transports.test.ts

@@ -1,3 +1,4 @@
+import { storeSessionForTest } from '../../__tests__/test-utils/store-factory.ts';
 import { isSessionRecording } from '../session-script-publication-capability.ts';
 import { createTestDeviceInventoryGateways } from '../../__tests__/test-utils/device-inventory-gateways.ts';
 /**
@@ -204,7 +205,9 @@ for (const [transport, send] of TRANSPORTS) {
     expect(isSessionRecording(session)).toBe(false);
     expect(session.scriptPublication).toBe(undefined);
     // No artifact: the write a later close/teardown would attempt publishes nothing.
-    expect(sessionStore.writeSessionLog(session)).toEqual({ written: false });
+    expect(sessionStore.writeSessionLog(storeSessionForTest(sessionStore, session))).toEqual({
+      written: false,
+    });
     expect(listAdArtifacts(root)).toEqual([]);
     expect(fs.existsSync(path.join(root, 'forged.ad'))).toBe(false);
 
@@ -295,7 +298,7 @@ test('an owner-armed session still records its target and publishes its script',
   expect(isSessionRecording(session)).toBe(true);
   expect(scriptTargetPath(session.scriptPublication ?? NO_SCRIPT_PUBLICATION)).toBe(target);
 
-  const result = sessionStore.writeSessionLog(session);
+  const result = sessionStore.writeSessionLog(storeSessionForTest(sessionStore, session));
   expect(result).toEqual({ written: true, path: target, actionCount: 1 });
   expect(fs.readFileSync(target, 'utf8')).toMatch(/^open /m);
 });

src/daemon/__tests__/session-store-lifetime.test.ts

@@ -1,7 +1,13 @@
 import assert from 'node:assert/strict';
+import fs from 'node:fs';
 import { test } from 'vitest';
 import { AppError } from '@agent-device/kernel/errors';
-import { makeSession } from '../../__tests__/test-utils/session-factories.ts';
+import {
+  makeSession,
+  makeRepairCompleteSession,
+  makeRepairArmedSession,
+  authoringPublication,
+} from '../../__tests__/test-utils/session-factories.ts';
 import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts';
 
 const ADDRESS = 'cwd:worktree:default';
@@ -121,3 +127,47 @@ test('a ref from another store has no authority over the same address', () => {
   assert.equal(target.retire(foreign), false);
   assert.equal(target.requireCurrent(local), foreign.session);
 });
+
+test('script writes use the latest matching record and refuse a retired lifetime', () => {
+  const store = makeSessionStore();
+  const ref = store.publish(ADDRESS, makeSession('default'));
+  store.update(ref, {
+    scriptPublication: authoringPublication('armed'),
+    actions: [{ ts: 1, command: 'click', positionals: ['id="late-action"'], flags: {} }],
+  });
+  const result = store.writeSessionLog(ref);
+  assert.equal(result.written, true);
+  if (result.written) assert.match(fs.readFileSync(result.path, 'utf8'), /late-action/);
+  store.retire(ref);
+  const successor = store.publish(ADDRESS, makeRepairCompleteSession('default'));
+  assert.throws(() => store.writeSessionLog(ref), ended);
+  store.finalizeRepairTeardown(ref);
+  const state = store.requireCurrent(successor).scriptPublication;
+  assert.equal(state?.kind, 'repair');
+  if (state?.kind === 'repair') assert.equal(state.status, 'complete');
+  assert.equal(successor.session.actions.length, 0);
+});
+
+test('repair tombstones follow the scoped address and cannot be written by a retired ref', () => {
+  const store = makeSessionStore();
+  const ref = store.publish(ADDRESS, makeRepairArmedSession('default'));
+  store.update(ref, {
+    scriptPublication: {
+      kind: 'repair',
+      status: 'armed',
+      boundary: 0,
+      target: { kind: 'default', force: false },
+      sourcePath: '/latest.ad',
+    },
+  });
+  store.writeRepairTombstone(ref);
+  assert.equal(store.readRepairTombstone(ADDRESS)?.owner, ADDRESS);
+  assert.equal(store.readRepairTombstone(ADDRESS)?.sourcePath, '/latest.ad');
+  assert.equal(store.readRepairTombstone('default'), undefined);
+  store.retire(ref);
+  store.clearRepairTombstone(ADDRESS);
+  const successor = store.publish(ADDRESS, makeRepairArmedSession('default'));
+  store.writeRepairTombstone(ref);
+  assert.equal(store.readRepairTombstone(ADDRESS), undefined);
+  assert.equal(store.requireCurrent(successor), successor.session);
+});