From 23c357ff8a6f80e14f7adef3bc4b21597e1f40cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 13 Jul 2026 17:08:42 +0200 Subject: [PATCH 01/16] fix(replay): repair-transaction lifecycle (ADR 0012 decision 6 / #1234) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-supervised re-record repair lifecycle, rebased onto #1225's failure-isolated close teardown. Consolidated from the earlier iterative rounds into the final teardown-commits model: - R7 keep-alive keyed off PERSISTED transaction state (repairSessionHeld signal), so a `replay --from` continuation without --save-script is still held on divergence. - Commit gated on transaction COMPLETION (saveScriptComplete/saveScriptCommitted), never on `close` alone — no prefix is ever published. - Single commit path: `commitRepairBeforeClose` runs before #1225's `runSessionCloseTeardown` destructive steps; a repair-armed session skips the teardown's ordinary writeSessionLog, non-repair keeps it. Idle-reap/shutdown commit-on-completion or tombstone via `finalizeRepairTeardown`. - BLOCKER fixes: reaped `replay --from` -> REPAIR_SESSION_EXPIRED; commit failures surfaced (not swallowed) and keep the session for retry (with healed-path reporting on success); race-safe atomic no-clobber publish; minimal `[open, close]` arms the transaction. Integrated with #1225: keeps runSessionCloseTeardown's failure-isolated cleanup + preserved platform-close error; repair commit happens first so a failed commit keeps the session addressable. --- .../request-router-repair-expired.test.ts | 140 +++++ .../__tests__/session-script-writer.test.ts | 199 ++++++- src/daemon/__tests__/session-store.test.ts | 38 ++ src/daemon/client/daemon-client-lifecycle.ts | 65 ++- src/daemon/client/daemon-client.ts | 26 +- .../session-replay-repair-acceptance.test.ts | 5 +- .../session-replay-repair-loop.test.ts | 240 ++++++++ .../session-replay-repair-transaction.test.ts | 520 ++++++++++++++++++ src/daemon/handlers/session-close.ts | 122 +++- src/daemon/handlers/session-replay-runtime.ts | 131 ++++- src/daemon/http-errors.ts | 3 + src/daemon/request-router.ts | 31 +- src/daemon/server/daemon-idle-reap.test.ts | 37 +- src/daemon/server/daemon-idle-reap.ts | 32 +- src/daemon/server/daemon-runtime.ts | 5 +- src/daemon/session-script-writer.ts | 192 ++++++- src/daemon/session-store.ts | 100 +++- src/daemon/types.ts | 26 + src/kernel/errors.ts | 3 + src/replay/divergence.ts | 12 +- .../__tests__/daemon-client-lifecycle.test.ts | 253 +++++++++ 21 files changed, 2096 insertions(+), 84 deletions(-) create mode 100644 src/daemon/__tests__/request-router-repair-expired.test.ts create mode 100644 src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts diff --git a/src/daemon/__tests__/request-router-repair-expired.test.ts b/src/daemon/__tests__/request-router-repair-expired.test.ts new file mode 100644 index 000000000..764a43e3e --- /dev/null +++ b/src/daemon/__tests__/request-router-repair-expired.test.ts @@ -0,0 +1,140 @@ +/** + * ADR 0012 decision 6, R7 (C5a): when a repair session was reaped before it was + * finalized, the request router rewrites the resulting `SESSION_NOT_FOUND` into + * a `REPAIR_SESSION_EXPIRED` recovery error with actionable re-run guidance, + * rather than leaking a bare SESSION_NOT_FOUND. Any other error, or the absence + * of a live tombstone, passes through unchanged. + */ +import { test, expect, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { getResolveTargetDeviceMock } from './request-router-dispatch-mocks.ts'; + +vi.mock('../device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}) })); + +import { createRequestHandler } from '../request-router.ts'; +import type { DaemonRequest, SessionState } from '../types.ts'; +import type { DeviceInfo } from '../../kernel/device.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { parseReplayInput } from '../../compat/replay-input.ts'; +import { computeReplayPlanDigest } from '../../replay/plan-digest.ts'; +import { readEffectiveReplayPlanDigestMetadata } from '../handlers/session-replay-runtime-plan.ts'; + +const mockResolveTargetDevice = vi.mocked(getResolveTargetDeviceMock()); + +function makeHandler(prefix: string) { + const sessionStore = makeSessionStore(prefix); + const handler = createRequestHandler({ + logPath: path.join(os.tmpdir(), 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + trackDownloadableArtifact: () => 'artifact-id', + }); + return { sessionStore, handler }; +} + +function tombstonedSession(name: string): SessionState { + return { + name, + device: { platform: 'apple', id: 'sim-1', name: 'iPhone', kind: 'simulator', booted: true }, + createdAt: Date.now(), + actions: [], + saveScriptBoundary: 0, + repairSourcePath: '/flows/login.ad', + }; +} + +function closeRequest(session: string): DaemonRequest { + return { token: 'test-token', session, command: 'close', positionals: [], flags: {} }; +} + +test('a command that finds no session but hits a live repair tombstone gets REPAIR_SESSION_EXPIRED', async () => { + 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 response = await handler(closeRequest('repair-x')); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('REPAIR_SESSION_EXPIRED'); + // Re-run guidance carries the original script path from the tombstone. + expect(response.error.message).toMatch(/replay \/flows\/login\.ad --save-script/); +}); + +test('without a tombstone, a missing session still returns a plain SESSION_NOT_FOUND', async () => { + const { handler } = makeHandler('agent-device-router-no-tombstone-'); + const response = await handler(closeRequest('never-existed')); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); +}); + +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 response = await handler(closeRequest('repair-y')); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); +}); + +// ADR 0012 decision 6, R7 (BLOCKER 1): the tombstone translation also covers the +// `replay --from` continuation path — a reaped repair session's continuation +// gets REPAIR_SESSION_EXPIRED, not a REPLAY_DIVERGENCE that slipped past it. +test('a replay --from continuation on a reaped repair session gets REPAIR_SESSION_EXPIRED (not REPLAY_DIVERGENCE)', async () => { + // Device resolution/readiness is mocked so the router reaches the replay + // handler's missing-session preflight fast and deterministically. + const iosDevice: DeviceInfo = { + platform: 'apple', + id: 'sim-1', + name: 'iPhone', + kind: 'simulator', + booted: true, + }; + mockResolveTargetDevice.mockResolvedValue(iosDevice); + const { sessionStore, handler } = makeHandler('agent-device-router-from-expired-'); + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-router-from-script-')); + const scriptPath = path.join(root, 'flow.ad'); + fs.writeFileSync(scriptPath, 'open "Demo"\nclick id="a"\n'); + + // Compute the plan digest exactly as runReplayScriptFile does (a real agent + // takes it from the divergence report's resume.planDigest). + const flags = { platform: 'ios' as const }; + const parsed = parseReplayInput(fs.readFileSync(scriptPath, 'utf8'), flags, { + sourcePath: scriptPath, + }); + const digest = computeReplayPlanDigest({ + actions: parsed.actions, + actionLines: parsed.actionLines, + actionSourcePaths: parsed.actionSourcePaths, + metadata: readEffectiveReplayPlanDigestMetadata(flags), + }); + + // The repair session was reaped, leaving a tombstone; no live session exists. + sessionStore.writeRepairTombstone(tombstonedSession('repair-from')); + + const response = await handler({ + token: 'test-token', + session: 'repair-from', + command: 'replay', + positionals: [scriptPath], + flags: { ...flags, replayFrom: 2, replayPlanDigest: digest }, + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('REPAIR_SESSION_EXPIRED'); + expect(response.error.code).not.toBe('REPLAY_DIVERGENCE'); + expect(response.error.message).toMatch(/replay \/flows\/login\.ad --save-script/); + + fs.rmSync(root, { recursive: true, force: true }); +}); diff --git a/src/daemon/__tests__/session-script-writer.test.ts b/src/daemon/__tests__/session-script-writer.test.ts index a28e01d19..b2de14415 100644 --- a/src/daemon/__tests__/session-script-writer.test.ts +++ b/src/daemon/__tests__/session-script-writer.test.ts @@ -2,7 +2,7 @@ import { test, expect } from 'vitest'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { SessionScriptWriter } from '../session-script-writer.ts'; +import { HEAL_COMPLETE_SENTINEL, SessionScriptWriter } from '../session-script-writer.ts'; import { recordActionEntry } from '../session-action-recorder.ts'; import { makeIosSession } from '../../__tests__/test-utils/session-factories.ts'; import { parseReplayScriptDetailed } from '../../replay/script.ts'; @@ -30,6 +30,10 @@ test('write() slices session.actions from saveScriptBoundary onward, excluding p const session = makeIosSession('default', { recordSession: true, saveScriptBoundary: 2, + // Fix 2: a repair-armed write only publishes once explicitly finalized + // (`close --save-script`) — set here to isolate THIS test's own concern + // (boundary slicing), covered separately below. + saveScriptComplete: true, actions: [ action({ command: 'open', positionals: ['Demo'] }), action({ command: 'click', positionals: ['label="Old"'] }), @@ -66,6 +70,7 @@ test('a boundary-sliced script still strips diagnostic snapshot actions', () => const session = makeIosSession('default', { recordSession: true, saveScriptBoundary: 1, + saveScriptComplete: true, actions: [ action({ command: 'open', positionals: ['Demo'] }), action({ command: 'snapshot', positionals: [] }), @@ -90,6 +95,7 @@ test('a recorded ref that resolved to a selectorChain writes a clean selector li const session = makeIosSession('default', { recordSession: true, saveScriptBoundary: 0, + saveScriptComplete: true, actions: [ action({ command: 'press', @@ -111,12 +117,17 @@ test('a recorded ref that never resolved to a selectorChain throws instead of em const session = makeIosSession('default', { recordSession: true, saveScriptBoundary: 0, + saveScriptComplete: true, actions: [action({ command: 'press', positionals: ['@e7'] })], }); const scriptPath = path.join(root, 'sessions', 'default', 'expected-not-written.ad'); - expect(() => writer.write(session)).toThrow(/never resolved to a selector/); - // Fail loud, not a swallowed { written: false } — no file was produced. + // BLOCKER 2: a repair commit failure is SURFACED via the result (never + // swallowed into a bare `{written:false}`), not thrown — so close/teardown + // can report it and keep the session for retry. + const result = writer.write(session); + expect(result.written).toBe(false); + expect(result.written === false && result.error?.message).toMatch(/never resolved to a selector/); expect(fs.existsSync(scriptPath)).toBe(false); expect(fs.readdirSync(path.join(root, 'sessions')).length).toBe(0); }); @@ -127,10 +138,13 @@ test('a bare-@ref fill action also fails loud, not just click-like commands', () const session = makeIosSession('default', { recordSession: true, saveScriptBoundary: 0, + saveScriptComplete: true, actions: [action({ command: 'fill', positionals: ['@e9', 'hello'] })], }); - expect(() => writer.write(session)).toThrow(/never resolved to a selector/); + const result = writer.write(session); + expect(result.written).toBe(false); + expect(result.written === false && result.error?.message).toMatch(/never resolved to a selector/); }); test('a bare @ref later in the same session (after a resolved earlier action) still fails loud, writing nothing', () => { @@ -139,6 +153,7 @@ test('a bare @ref later in the same session (after a resolved earlier action) st const session = makeIosSession('default', { recordSession: true, saveScriptBoundary: 0, + saveScriptComplete: true, actions: [ action({ command: 'open', positionals: ['Demo'] }), action({ @@ -150,7 +165,9 @@ test('a bare @ref later in the same session (after a resolved earlier action) st ], }); - expect(() => writer.write(session)).toThrow(/never resolved to a selector/); + const result = writer.write(session); + expect(result.written).toBe(false); + expect(result.written === false && result.error?.message).toMatch(/never resolved to a selector/); expect(fs.readdirSync(path.join(root, 'sessions')).length).toBe(0); }); @@ -176,28 +193,66 @@ test('an ordinary (non-repair-armed) recording keeps the existing bare-ref fallb // --- ADR 0012 decision 6 (P2): the default `.healed.ad` sibling is never // silently clobbered — a human must review each healed diff before promoting. --- -test('write() refuses to clobber an existing DEFAULT .healed.ad (no explicit --save-script=)', () => { +test('write() refuses to clobber an existing COMPLETE DEFAULT .healed.ad (no explicit --save-script=)', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-script-writer-clobber-')); const writer = new SessionScriptWriter(path.join(root, 'sessions')); const healedPath = path.join(root, 'flows', 'login.healed.ad'); fs.mkdirSync(path.dirname(healedPath), { recursive: true }); - // A prior, unreviewed healed script already sits at the default sibling path. - fs.writeFileSync(healedPath, 'context platform=ios device="x"\nclick id="old"\n'); + // A prior, unreviewed, COMPLETE healed script already sits at the default + // sibling path (Fix 4: only a file carrying the completeness sentinel is + // protected). + fs.writeFileSync( + healedPath, + `context platform=ios device="x"\nclick id="old"\n${HEAL_COMPLETE_SENTINEL}\n`, + ); const before = fs.readFileSync(healedPath, 'utf8'); const session = makeIosSession('default', { recordSession: true, saveScriptBoundary: 0, + saveScriptComplete: true, saveScriptPath: healedPath, saveScriptDefaultedHealedPath: true, actions: [action({ command: 'click', positionals: ['id="new"'] })], }); - expect(() => writer.write(session)).toThrow(/already exists/); - // Fail loud — the prior unreviewed diff is untouched. + // BLOCKER 2c: a no-clobber refusal is surfaced via the result's error (a + // distinct "already exists" message), not thrown; the prior complete diff is + // untouched. + const result = writer.write(session); + expect(result.written).toBe(false); + expect(result.written === false && result.error?.message).toMatch(/already exists/); expect(fs.readFileSync(healedPath, 'utf8')).toBe(before); }); +test('write() DOES overwrite a stale INCOMPLETE .healed.ad at the default path (Fix 4: partial is overwritable)', () => { + const root = fs.mkdtempSync( + path.join(os.tmpdir(), 'agent-device-script-writer-clobber-partial-'), + ); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const healedPath = path.join(root, 'flows', 'login.healed.ad'); + fs.mkdirSync(path.dirname(healedPath), { recursive: true }); + // A partial left over from a diverged-and-abandoned repair (pre-Fix-2 bug, + // or any other incomplete write) — no completeness sentinel. + fs.writeFileSync(healedPath, 'context platform=ios device="x"\nclick id="stale-partial"\n'); + + const session = makeIosSession('default', { + recordSession: true, + saveScriptBoundary: 0, + saveScriptComplete: true, + saveScriptPath: healedPath, + saveScriptDefaultedHealedPath: true, + actions: [action({ command: 'click', positionals: ['id="new"'] })], + }); + + const result = writer.write(session); + expect(result.written).toBe(true); + const script = fs.readFileSync(healedPath, 'utf8'); + expect(script).toContain(HEAL_COMPLETE_SENTINEL); + const parsed = parseReplayScriptDetailed(script); + expect(parsed.actions.map((a) => a.positionals[0])).toEqual(['id="new"']); +}); + test('write() DOES overwrite when the caller passed an explicit --save-script= (not defaulted)', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-script-writer-explicit-out-')); const writer = new SessionScriptWriter(path.join(root, 'sessions')); @@ -208,6 +263,7 @@ test('write() DOES overwrite when the caller passed an explicit --save-script=

clears the defaulted marker, so an exp }); expect(session.saveScriptDefaultedHealedPath).toBe(false); expect(session.saveScriptPath).toBe(explicitOut); + // `recordActionEntry` is the low-level action recorder `close`'s handler + // calls on its way to setting the finalize signal (Fix 2) — set here to + // isolate this test's own concern (defaulted-marker clearing). + session.saveScriptComplete = true; const result = writer.write(session); expect(result.written).toBe(true); @@ -253,3 +313,122 @@ test('close --save-script= clears the defaulted marker, so an exp const parsed = parseReplayScriptDetailed(fs.readFileSync(explicitOut, 'utf8')); expect(parsed.actions.some((a) => a.positionals[0] === 'id="new"')).toBe(true); }); + +// --- ADR 0012 decision 6, R7 + commit semantics (Fix 2, C2): a repair-armed +// write COMMITS only when the transaction is COMPLETE (ARMED -> COMPLETE -> +// COMMITTED); an incomplete transaction ABORTS (publishes no prefix), and a +// committed one is an idempotent no-op. --- + +test('C2 abort-before-complete: a repair-armed but NOT-complete write discards — no file, no prefix', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-script-writer-incomplete-')); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const session = makeIosSession('default', { + recordSession: true, + saveScriptBoundary: 0, + // No saveScriptComplete: the plan never ran to its last executable step + // (a `close`/`close --save-script` reached after a divergence, a daemon + // teardown, or an idle-reap of an in-flight repair). + actions: [action({ command: 'click', positionals: ['id="save"'] })], + }); + + const result = writer.write(session); + expect(result).toEqual({ written: false }); + expect(fs.existsSync(path.join(root, 'sessions'))).toBe(false); + // Not committed — teardown will tombstone it (C5a). + expect(session.saveScriptCommitted).toBeFalsy(); +}); + +test('C2 commit-when-complete: a repair-armed COMPLETE write publishes and marks the session COMMITTED', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-script-writer-complete-')); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const outPath = path.join(root, 'flows', 'flow.healed.ad'); + fs.mkdirSync(path.dirname(outPath), { recursive: true }); + const session = makeIosSession('default', { + recordSession: true, + saveScriptBoundary: 0, + saveScriptComplete: true, + saveScriptPath: outPath, + actions: [action({ command: 'click', positionals: ['id="save"'] })], + }); + + const result = writer.write(session); + expect(result.written).toBe(true); + expect(fs.readFileSync(outPath, 'utf8')).toContain(HEAL_COMPLETE_SENTINEL); + expect(session.saveScriptCommitted).toBe(true); +}); + +test('C2 idempotent post-commit: a second write on a COMMITTED session no-ops (no re-publish, no error)', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-script-writer-idempotent-')); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const outPath = path.join(root, 'flows', 'flow.healed.ad'); + fs.mkdirSync(path.dirname(outPath), { recursive: true }); + const session = makeIosSession('default', { + recordSession: true, + saveScriptBoundary: 0, + saveScriptComplete: true, + saveScriptPath: outPath, + actions: [action({ command: 'click', positionals: ['id="save"'] })], + }); + + expect(writer.write(session).written).toBe(true); + const firstContent = fs.readFileSync(outPath, 'utf8'); + const firstMtime = fs.statSync(outPath).mtimeMs; + + // Mutate actions to prove a re-publish WOULD change the file if it happened. + session.actions.push(action({ command: 'click', positionals: ['id="other"'] })); + const second = writer.write(session); + expect(second).toEqual({ written: false }); + // The published artifact is untouched — the committed transaction never + // re-writes (no duplicate, no corruption). + expect(fs.readFileSync(outPath, 'utf8')).toBe(firstContent); + expect(fs.statSync(outPath).mtimeMs).toBe(firstMtime); +}); + +test('write() still emits an ordinary (non-repair) recording on close without --save-script, unaffected by the commit gate', () => { + const root = fs.mkdtempSync( + path.join(os.tmpdir(), 'agent-device-script-writer-ordinary-unfinalized-'), + ); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const session = makeIosSession('default', { + recordSession: true, + // No saveScriptBoundary: an ordinary `open --save-script` recording, not + // a repair — the Fix 2 gate only applies to repair-armed sessions. + actions: [action({ command: 'click', positionals: ['id="save"'] })], + }); + + const { parsed } = writeAndParse(writer, session); + expect(parsed.actions.map((a) => a.command)).toEqual(['click']); +}); + +test('write() never appends the completeness sentinel to an ordinary (non-repair) recording', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-script-writer-no-sentinel-')); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const session = makeIosSession('default', { + recordSession: true, + actions: [action({ command: 'click', positionals: ['id="save"'] })], + }); + + const { script } = writeAndParse(writer, session); + expect(script).not.toContain(HEAL_COMPLETE_SENTINEL); +}); + +test('write() publishes atomically: no stray temp file survives a successful repair write', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-script-writer-atomic-')); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const outPath = path.join(root, 'flows', 'atomic.healed.ad'); + fs.mkdirSync(path.dirname(outPath), { recursive: true }); + const session = makeIosSession('default', { + recordSession: true, + saveScriptBoundary: 0, + saveScriptComplete: true, + saveScriptPath: outPath, + actions: [action({ command: 'click', positionals: ['id="save"'] })], + }); + + const result = writer.write(session); + expect(result.written).toBe(true); + // The only file left in the destination directory is the published script + // itself — the temp path was renamed into place, not left behind. + expect(fs.readdirSync(path.dirname(outPath))).toEqual([path.basename(outPath)]); + expect(fs.readFileSync(outPath, 'utf8')).toContain(HEAL_COMPLETE_SENTINEL); +}); diff --git a/src/daemon/__tests__/session-store.test.ts b/src/daemon/__tests__/session-store.test.ts index 149a30ff1..e5595d24f 100644 --- a/src/daemon/__tests__/session-store.test.ts +++ b/src/daemon/__tests__/session-store.test.ts @@ -656,3 +656,41 @@ test('writeSessionLog never fabricates a target-v1 annotation for actions record const script = writeScript(fixture); assert.equal(/agent-device:target-v1/.test(script), false); }); + +// --- ADR 0012 decision 6, R7 (C5a): repair tombstones turn a post-reap +// SESSION_NOT_FOUND into REPAIR_SESSION_EXPIRED with re-run guidance. --- + +test('writeRepairTombstone/readRepairTombstone round-trips owner + source path', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-tombstone-')); + const store = new SessionStore(path.join(root, 'sessions')); + const session = makeSession('default'); + session.saveScriptBoundary = 0; + session.repairSourcePath = '/flows/login.ad'; + + store.writeRepairTombstone(session); + const tombstone = store.readRepairTombstone('default'); + assert.ok(tombstone); + assert.equal(tombstone?.owner, 'default'); + assert.equal(tombstone?.sourcePath, '/flows/login.ad'); + assert.ok((tombstone?.expiresAt ?? 0) > Date.now()); +}); + +test('readRepairTombstone returns undefined once the tombstone has expired', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-tombstone-expiry-')); + const store = new SessionStore(path.join(root, 'sessions')); + const session = makeSession('default'); + // TTL 0 => expiresAt <= now => already stale. + store.writeRepairTombstone(session, 0); + assert.equal(store.readRepairTombstone('default'), undefined); +}); + +test('clearRepairTombstone removes a tombstone (a fresh replay --save-script clears the key)', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-tombstone-clear-')); + const store = new SessionStore(path.join(root, 'sessions')); + const session = makeSession('default'); + store.writeRepairTombstone(session); + assert.ok(store.readRepairTombstone('default')); + + store.clearRepairTombstone('default'); + assert.equal(store.readRepairTombstone('default'), undefined); +}); diff --git a/src/daemon/client/daemon-client-lifecycle.ts b/src/daemon/client/daemon-client-lifecycle.ts index ced977199..5d2a926a6 100644 --- a/src/daemon/client/daemon-client-lifecycle.ts +++ b/src/daemon/client/daemon-client-lifecycle.ts @@ -3,7 +3,7 @@ import net from 'node:net'; import os from 'node:os'; import path from 'node:path'; import { AppError } from '../../kernel/errors.ts'; -import type { DaemonRequest } from '../types.ts'; +import type { DaemonRequest, DaemonResponse } from '../types.ts'; import { runCmdDetachedMonitored, type ExecDetachedExit } from '../../utils/exec.ts'; import { findProjectRoot, readVersion } from '../../utils/version.ts'; import { emitDiagnostic } from '../../utils/diagnostics.ts'; @@ -309,11 +309,22 @@ export async function cleanupDaemonAfterRequest( req: Omit, daemon: EnsuredDaemon, settings: DaemonClientSettings, + response: DaemonResponse | undefined, ): Promise { if ( !isOneShotReplayCommand(req.command) || (!daemon.startedByClient && !settings.ownedStateDir) || - isRemoteDaemon(daemon.info) + isRemoteDaemon(daemon.info) || + // ADR 0012 decision 6, R7 (Fix 1, C1): a repair-armed `--save-script` + // replay that comes back as a HELD divergence must keep its owning daemon + // (and the session on it) addressable for the agent's corrective press + + // `replay --from`/`close` — tearing it down here is what turns a + // recoverable divergence into a later bare SESSION_NOT_FOUND. The daemon + // then bounds the held session's own lifetime via idle-reap (writing a + // `REPAIR_SESSION_EXPIRED` tombstone on reap), so an abandoned repair still + // cannot leak indefinitely; this only stops the ONE-SHOT-COMMAND teardown + // below from racing ahead of that window. + isHeldRepairDivergence(response) ) { return; } @@ -352,6 +363,56 @@ export async function cleanupDaemonAfterRequest( }); } +/** + * ADR 0012 decision 6, R7 (Fix 1, C1): true when this response must keep the + * owning daemon alive — a `REPLAY_DIVERGENCE` whose payload carries the + * daemon's `resume.repairSessionHeld` liveness signal. The daemon sets that + * signal from the PERSISTED repair-transaction state (the session is + * repair-armed and not yet committed), NOT from the current request's + * `--save-script` flag — so a `replay --from` continuation that does not + * repeat `--save-script` (R2) is still kept alive if it diverges. Keying the + * client purely off the signal (the daemon is the authority on transaction + * state) is what makes that continuation work; a plain, non-repair divergence + * carries no signal and gets no keep-alive. Also independent of + * `resume.allowed` (plan-resumability): a held divergence with `allowed: false` + * still holds the session so the agent can inspect and `close` cleanly. + */ +export function isHeldRepairDivergence(response: DaemonResponse | undefined): boolean { + if (!response || response.ok) return false; + if (response.error.code !== 'REPLAY_DIVERGENCE') return false; + const divergence = response.error.details?.divergence; + if (!divergence || typeof divergence !== 'object') return false; + const resume = (divergence as Record).resume; + if (!resume || typeof resume !== 'object') return false; + return (resume as Record).repairSessionHeld === true; +} + +/** + * ADR 0012 decision 6 (Fix 1): "keep it addressable" — an owned ephemeral + * daemon lives at a randomly generated `--state-dir` (`createOwnedReplayStateDir`) + * that no other invocation knows about, so keeping the process alive is not + * enough on its own. Appended (never overwriting an existing hint, e.g. a + * selector-miss's own guidance) so the agent's next command knows to target + * the SAME daemon instead of resolving to the default one. + */ +export function attachRepairSessionAddressHint( + response: Extract, + stateDir: string, +): Extract { + const addressHint = + `This repair session's daemon was kept alive to continue the repair; pass ` + + `--state-dir ${stateDir} on your next command (press, replay --from, or ` + + `close --save-script) to reach it.`; + const existingHint = response.error.hint; + return { + ...response, + error: { + ...response.error, + hint: existingHint ? `${existingHint} ${addressHint}` : addressHint, + }, + }; +} + function isOneShotReplayCommand(command: string | undefined): boolean { return command === PUBLIC_COMMANDS.replay || command === PUBLIC_COMMANDS.test; } diff --git a/src/daemon/client/daemon-client.ts b/src/daemon/client/daemon-client.ts index f417d78de..6c086899c 100644 --- a/src/daemon/client/daemon-client.ts +++ b/src/daemon/client/daemon-client.ts @@ -7,9 +7,12 @@ import { createRequestId, emitDiagnostic, withDiagnosticTimer } from '../../util import { INTERNAL_COMMANDS, PUBLIC_COMMANDS } from '../../command-catalog.ts'; import { prepareRemoteRequestArtifacts } from '../../remote/daemon-artifacts.ts'; import { + attachRepairSessionAddressHint, cleanupDaemonAfterRequest, ensureDaemon, + isHeldRepairDivergence, resolveClientSettings, + type DaemonClientSettings, } from './daemon-client-lifecycle.ts'; import { sendRequest } from './daemon-client-transport.ts'; import { resolveDaemonRequestTimeoutMs } from './daemon-client-timeout.ts'; @@ -75,8 +78,9 @@ export async function sendToDaemon( session: req.session, }, }); + let response: DaemonResponse | undefined; try { - return await withDiagnosticTimer( + response = await withDiagnosticTimer( 'daemon_request', async () => await sendRequest( @@ -89,11 +93,29 @@ export async function sendToDaemon( ), { requestId, command: req.command }, ); + response = withRepairSessionAddressHintIfOwned(response, settings); + return response; } finally { - await cleanupDaemonAfterRequest(req, daemon, settings); + await cleanupDaemonAfterRequest(req, daemon, settings, response); } } +/** + * ADR 0012 decision 6 (Fix 1): the owned ephemeral state dir this daemon was + * started at is otherwise unaddressable by a later invocation — hint it here, + * only when the daemon is actually being kept alive for it + * (`settings.ownedStateDir` means `daemon.startedByClient` is also true). + */ +function withRepairSessionAddressHintIfOwned( + response: DaemonResponse, + settings: DaemonClientSettings, +): DaemonResponse { + if (response.ok || !settings.ownedStateDir || !isHeldRepairDivergence(response)) { + return response; + } + return attachRepairSessionAddressHint(response, settings.paths.baseDir); +} + function writeInstallInProgressNotice(command: string | undefined): void { if (!isInstallLikeCommand(command) || process.stderr.isTTY !== true || process.env.CI) return; process.stderr.write( diff --git a/src/daemon/handlers/__tests__/session-replay-repair-acceptance.test.ts b/src/daemon/handlers/__tests__/session-replay-repair-acceptance.test.ts index b535a873c..24de8543f 100644 --- a/src/daemon/handlers/__tests__/session-replay-repair-acceptance.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-repair-acceptance.test.ts @@ -124,7 +124,10 @@ test('a healed script survives repair + fresh-session replay: self-contained ope expect(session.actions.map((a) => a.command)).toEqual(['open', 'press', 'click']); // --- End the repair: write the healed script (the `close --save-script` - // path reuses exactly this writer). --- + // path reuses exactly this writer; ADR 0012 decision 6 Fix 2 gates a + // repair-armed write on the same explicit finalize signal `close + // --save-script` sets). --- + session.saveScriptComplete = true; sessionStore.writeSessionLog(session); const healedPath = path.join(root, 'flow.healed.ad'); expect(fs.existsSync(healedPath)).toBe(true); diff --git a/src/daemon/handlers/__tests__/session-replay-repair-loop.test.ts b/src/daemon/handlers/__tests__/session-replay-repair-loop.test.ts index 9f170e843..a2dafd56b 100644 --- a/src/daemon/handlers/__tests__/session-replay-repair-loop.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-repair-loop.test.ts @@ -402,3 +402,243 @@ test('R1 bootstrap: a session created by step 1 (open) arms in time for step 2 t expect(session.saveScriptBoundary).toBe(0); expect(session.actions[1]?.targetEvidence).toBeDefined(); }); + +test('BLOCKER 4: a minimal [open, terminal close] cold-start script arms the transaction and skips the close', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-repair-open-close-')); + const sessionStore = new SessionStore(path.join(root, 'sessions')); + const sessionName = 'default'; + // No pre-existing session — step 1 (`open`) CREATES it, so pre-open arming is + // a no-op and the transaction can only arm once the session exists. + const filePath = writeReplayFile(root, ['open "Demo"', 'close']); + const spy: DaemonRequest[] = []; + const invoke = makeRecordingReplayInvoke({ sessionStore, sessionName, spy }); + + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke, + }); + + expect(response.ok).toBe(true); + const session = sessionStore.get(sessionName)!; + // ARMED-before-step-1 semantics satisfied: the transaction armed even though + // `open` created the session. + expect(session.recordSession).toBe(true); + expect(session.saveScriptBoundary).toBe(0); + // The terminal `close` was recognized as lifecycle and SKIPPED (never + // dispatched), leaving the session alive for finalize. + expect(spy.map((r) => r.command)).toEqual(['open']); + expect(sessionStore.get(sessionName)).toBeDefined(); + // The commit-state machine applies: a completed armed transaction. + expect(session.saveScriptComplete).toBe(true); +}); + +// --- ADR 0012 decision 6 (Fix 3): the source plan's own terminal `close` is +// lifecycle, not a script step, while a repair is armed — the agent +// finalizes with `close --save-script` instead. --- + +test("Fix 3: the source plan's terminal close is skipped (never dispatched, never recorded) while the repair is armed", async () => { + const { root, sessionStore, sessionName, logPath } = setup( + 'agent-device-replay-repair-close-skip-', + ); + const filePath = writeReplayFile(root, ['open "Demo"', 'click id="save"', 'close']); + const spy: DaemonRequest[] = []; + const invoke = makeRecordingReplayInvoke({ + sessionStore, + sessionName, + spy, + evidence: (req) => (req.command === 'click' ? freshEvidence('save', 'Save') : undefined), + }); + + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + + expect(response.ok).toBe(true); + // `close` never reached dispatch... + expect(spy.map((r) => r.command)).toEqual(['open', 'click']); + // ...so it never lands in session.actions (the healed script never carries + // it — the agent's own `close --save-script` supplies the real one). + const session = sessionStore.get(sessionName)!; + expect(session.actions.map((a) => a.command)).toEqual(['open', 'click']); + // C4: skipping the terminal close does NOT delete the session — it stays + // alive and COMPLETE so the agent can finalize it with `close --save-script`. + expect(sessionStore.get(sessionName)).toBeDefined(); + expect(session.saveScriptComplete).toBe(true); +}); + +test('Fix 3: an ordinary (non-repair) replay still dispatches its terminal close normally', async () => { + const { root, sessionStore, sessionName, logPath } = setup( + 'agent-device-replay-repair-close-ordinary-', + ); + const filePath = writeReplayFile(root, ['open "Demo"', 'click id="save"', 'close']); + const spy: DaemonRequest[] = []; + const invoke = makeRecordingReplayInvoke({ sessionStore, sessionName, spy }); + + // No --save-script: this is a plain deterministic replay, not a repair. + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath, + sessionStore, + invoke, + }); + + expect(response.ok).toBe(true); + expect(spy.map((r) => r.command)).toEqual(['open', 'click', 'close']); +}); + +test('Fix 3: only the TERMINAL close is skipped during a repair — a mid-plan close still dispatches', async () => { + const { root, sessionStore, sessionName, logPath } = setup( + 'agent-device-replay-repair-close-midplan-', + ); + const filePath = writeReplayFile(root, ['open "Demo"', 'close', 'open "Demo2"']); + const spy: DaemonRequest[] = []; + const invoke = makeRecordingReplayInvoke({ + sessionStore, + sessionName, + spy, + openReplacesSession: true, + }); + + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + + expect(response.ok).toBe(true); + // The mid-plan close (not the plan's last action) dispatches normally; only + // a close in the FINAL position is treated as repair lifecycle. + expect(spy.map((r) => r.command)).toEqual(['open', 'close', 'open']); +}); + +test('Fix 3: a --from resume that lands on the terminal close skips it too, letting the run complete', async () => { + const { root, sessionStore, sessionName, logPath } = setup( + 'agent-device-replay-repair-close-from-', + ); + const filePath = writeReplayFile(root, [ + 'open "Demo" --relaunch', + SAVE_ANNOTATION, + 'click id="save"', + 'close', + ]); + const spy: DaemonRequest[] = []; + const invoke = makeRecordingReplayInvoke({ + sessionStore, + sessionName, + spy, + failSteps: new Set(['click id="save"']), + }); + + const leg1 = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(leg1.ok).toBe(false); + if (leg1.ok) return; + const divergence = leg1.error.details?.divergence as { + resume: { allowed: boolean; from: number; planDigest: string }; + }; + expect(divergence.resume.allowed).toBe(true); + expect(divergence.resume.from).toBe(2); + + const session = sessionStore.get(sessionName)!; + sessionStore.recordAction(session, { + command: 'press', + positionals: ['@e9'], + flags: {}, + result: { selectorChain: ['id="save-v2"'] }, + targetEvidence: freshEvidence('save-v2', 'Save V2'), + }); + + spy.length = 0; + const leg2 = await runReplayScriptFile({ + req: baseReq({ + positionals: [filePath], + flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest }, + }), + sessionName, + logPath, + sessionStore, + invoke, + }); + + // The resume lands directly on the (skipped) terminal close and completes + // — not a REPLAY_DIVERGENCE with repairHint "manual". + expect(leg2.ok).toBe(true); + expect(spy).toHaveLength(0); + // "click id=save" failed and was never recorded; the corrective press was + // recorded live; the terminal close was skipped, never dispatched. + expect(session.actions.map((a) => a.command)).toEqual(['open', 'press']); +}); + +// --- ADR 0012 decision 6, R7 (continuation persisted-state): a `replay --from` +// continuation does NOT repeat --save-script; if it diverges, the session is +// still held alive keyed off the PERSISTED armed state, not the request flag. --- + +test('a --from continuation WITHOUT --save-script that diverges is still held alive (repairSessionHeld set from persisted state)', async () => { + const { root, sessionStore, sessionName, logPath } = setup( + 'agent-device-replay-repair-continuation-', + ); + // No annotations => action-failure path; both clicks fail so leg1 diverges at + // step 2 and the --from-3 continuation diverges at step 3. + const filePath = writeReplayFile(root, ['open "Demo"', 'click id="a"', 'click id="b"']); + const invoke = makeRecordingReplayInvoke({ + sessionStore, + sessionName, + failSteps: new Set(['click id="a"', 'click id="b"']), + }); + + // Leg 1: `replay --save-script` opens the transaction, arms the session, and + // diverges at step 2 — held alive. + const leg1 = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(leg1.ok).toBe(false); + if (leg1.ok) return; + const leg1Divergence = leg1.error.details?.divergence as { + resume: { planDigest: string; repairSessionHeld?: boolean }; + }; + expect(leg1Divergence.resume.repairSessionHeld).toBe(true); + const armed = sessionStore.get(sessionName)!; + expect(armed.saveScriptBoundary).not.toBeUndefined(); + + // Leg 2: the `--from 3` continuation carries NO --save-script. It re-diverges + // at step 3 — and must STILL be held alive, keyed off the persisted armed + // state, so the transaction survives. + const leg2 = await runReplayScriptFile({ + req: baseReq({ + positionals: [filePath], + flags: { replayFrom: 3, replayPlanDigest: leg1Divergence.resume.planDigest }, + }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(leg2.ok).toBe(false); + if (leg2.ok) return; + expect(leg2.error.code).toBe('REPLAY_DIVERGENCE'); + const leg2Divergence = leg2.error.details?.divergence as { + resume: { repairSessionHeld?: boolean }; + }; + // The key assertion: held alive despite no --save-script on this request. + expect(leg2Divergence.resume.repairSessionHeld).toBe(true); + expect(sessionStore.get(sessionName)).toBeDefined(); +}); diff --git a/src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts b/src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts new file mode 100644 index 000000000..eefc7452a --- /dev/null +++ b/src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts @@ -0,0 +1,520 @@ +/** + * 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 — + * `runReplayScriptFile` + `handleCloseCommand` sharing a live `SessionStore`, + * exactly like an agent's separate CLI invocations against the same daemon + * session would. `sendToDaemon`'s process-level keep-alive (Fix 1's daemon + * teardown guard) is a different architectural layer — a client-side process + * manager, not session/script state — and is covered separately in + * `src/utils/__tests__/daemon-client-lifecycle.test.ts` + * ("keeps an owned ephemeral daemon alive and hints its --state-dir..."). + * + * Fix 1 (session-side): a divergence never deletes the session — it stays in + * the SessionStore, addressable for the next call. + * Fix 2: SessionScriptWriter.write only publishes once `close --save-script` + * sets `saveScriptComplete` — never on a divergence-only exit or an + * abandoned close. + * Fix 3: the source plan's terminal `close` is skipped while repair-armed, so + * the resume completes instead of diverging on lifecycle. + * Fix 4: the publish is atomic (temp + rename) and carries the completeness + * sentinel, so a stale/partial file never blocks a later repair. + */ +import { test, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../../../platforms/apple/core/simulator.ts', async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, shutdownSimulator: vi.fn() }; +}); +vi.mock('../../../utils/exec.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, runCmd: vi.fn() }; +}); +vi.mock('../../../platforms/apple/core/runner/runner-client.ts', async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, stopIosRunnerSession: vi.fn(async () => {}) }; +}); +vi.mock('../../../platforms/apple/core/perf-xctrace.ts', async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, cleanupAppleXctracePerfCapture: vi.fn(async () => ({})) }; +}); +vi.mock('../../runtime-hints.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, clearRuntimeHintsFromApp: vi.fn(async () => {}) }; +}); +vi.mock('../../../core/dispatch.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, dispatchCommand: vi.fn(async () => ({})), resolveTargetDevice: vi.fn() }; +}); +vi.mock('../session-device-utils.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, settleIosSimulator: vi.fn(async () => {}) }; +}); + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { runReplayScriptFile } from '../session-replay-runtime.ts'; +import { handleCloseCommand } from '../session-close.ts'; +import { SessionStore } from '../../session-store.ts'; +import { LeaseRegistry } from '../../lease-registry.ts'; +import { dispatchCommand } from '../../../core/dispatch.ts'; +import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; +import { HEAL_COMPLETE_SENTINEL } from '../../session-script-writer.ts'; +import { parseReplayScriptDetailed } from '../../../replay/script.ts'; +import { + baseReplayRequest as baseReq, + writeReplayFile, +} from './session-replay-runtime.fixtures.ts'; +import { freshEvidence, makeRecordingReplayInvoke } from './session-replay-repair.fixtures.ts'; + +const mockDispatchCommand = vi.mocked(dispatchCommand); + +beforeEach(() => { + mockDispatchCommand.mockReset(); + // The "current" app state: "save" was renamed to "save-v2" (why step 2 + // diverges), matching the target verification the SAVE_ANNOTATION triggers. + mockDispatchCommand.mockResolvedValue({ + nodes: [ + { + index: 0, + depth: 0, + type: 'Button', + identifier: 'save-v2', + label: 'Save V2', + rect: { x: 10, y: 10, width: 40, height: 20 }, + }, + ], + truncated: false, + backend: 'xctest', + }); +}); + +const SAVE_ANNOTATION = + '# agent-device:target-v1 {"id":"save","role":"button","label":"Save","ancestry":[],"sibling":0,"viewportOrder":0,"verification":"verified"}'; + +function setup(prefix: string) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const sessionStore = new SessionStore(path.join(root, 'sessions')); + const sessionName = 'default'; + sessionStore.set(sessionName, makeIosSession(sessionName, { appBundleId: 'com.example.app' })); + return { + root, + sessionStore, + sessionName, + logPath: path.join(root, 'daemon.log'), + leaseRegistry: new LeaseRegistry(), + }; +} + +test('end-to-end repair transaction: cold divergence stays alive, corrective resume completes, close --save-script finalizes a COMPLETE healed .ad atomically, and an abandoned repair leaves no partial file', async () => { + // ============================================================ + // Part 1 — the repair chain that COMMITS. + // ============================================================ + const { root, sessionStore, sessionName, logPath, leaseRegistry } = setup( + 'agent-device-repair-transaction-commit-', + ); + const filePath = writeReplayFile(root, [ + 'open "Demo" --relaunch', + SAVE_ANNOTATION, + 'click id="save"', + 'click id="confirm"', + 'close', + ]); + const invoke = makeRecordingReplayInvoke({ + sessionStore, + sessionName, + evidence: (req) => (req.command === 'click' ? freshEvidence('confirm', 'Confirm') : undefined), + }); + + // --- Cold `replay drifted.ad --save-script` diverges on the renamed id. --- + const leg1 = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(leg1.ok).toBe(false); + if (leg1.ok) return; + expect(leg1.error.code).toBe('REPLAY_DIVERGENCE'); + const divergence = leg1.error.details?.divergence as { + kind: string; + resume: { allowed: boolean; from: number; planDigest: string; repairSessionHeld?: boolean }; + }; + expect(divergence.kind).toBe('selector-miss'); + expect(divergence.resume.allowed).toBe(true); + // C1: the daemon marks the repair-transaction liveness signal on the wire. + expect(divergence.resume.repairSessionHeld).toBe(true); + + // Fix 1 (session-side): the session stays alive — never torn down on a + // divergence-only exit. (The client-side daemon PROCESS keep-alive that + // makes this session reachable across separate CLI invocations is proven + // in daemon-client-lifecycle.test.ts.) + expect(sessionStore.get(sessionName)).toBeDefined(); + expect(sessionStore.get(sessionName)!.actions.map((a) => a.command)).toEqual(['open']); + // C2: the transaction is NOT complete yet — a `close` here would abort, not + // commit a prefix. + expect(sessionStore.get(sessionName)!.saveScriptComplete).toBeFalsy(); + + // --- Agent performs the corrective press (blessed @ref), recorded live. --- + const session = sessionStore.get(sessionName)!; + sessionStore.recordAction(session, { + command: 'press', + positionals: ['@e7'], + flags: {}, + result: { selectorChain: ['id="save-v2"'] }, + targetEvidence: freshEvidence('save-v2', 'Save V2'), + }); + + // --- `replay --from N+1 --plan-digest ` resumes to the end. The + // source plan's own terminal `close` (Fix 3) is skipped, so this completes + // instead of diverging on lifecycle. --- + const leg2 = await runReplayScriptFile({ + req: baseReq({ + positionals: [filePath], + flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest }, + }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(leg2.ok).toBe(true); + expect(session.actions.map((a) => a.command)).toEqual(['open', 'press', 'click']); + // The terminal close never dispatched or recorded. + expect(session.actions.some((a) => a.command === 'close')).toBe(false); + // C2: the resume reached the last executable step (terminal close skipped) — + // the transaction is now COMPLETE and commit-eligible. + expect(session.saveScriptComplete).toBe(true); + + // --- The agent finalizes: `close --save-script` (the real handler, not a + // direct writer call) commits the now-COMPLETE healed `.ad`. --- + const closeResponse = await handleCloseCommand({ + req: { + token: 't', + session: sessionName, + command: 'close', + positionals: [], + flags: { saveScript: true }, + }, + sessionName, + logPath, + sessionStore, + leaseRegistry, + }); + expect(closeResponse.ok).toBe(true); + // The session is gone (close's normal lifecycle) — but the healed script + // was written to disk before deletion. + expect(sessionStore.get(sessionName)).toBeUndefined(); + + const healedPath = path.join(root, 'flow.healed.ad'); + // BLOCKER 2a: the close response positively reports the committed healed path. + if (closeResponse.ok) expect(closeResponse.data?.savedScript).toBe(healedPath); + expect(fs.existsSync(healedPath)).toBe(true); + const healedScript = fs.readFileSync(healedPath, 'utf8'); + // Fix 4: complete + atomic — the sentinel is present, and the only file in + // the directory is the final published one (no stray temp file survived). + expect(healedScript).toContain(HEAL_COMPLETE_SENTINEL); + expect(fs.readdirSync(root).filter((entry) => entry.endsWith('.ad'))).toEqual([ + 'flow.ad', + 'flow.healed.ad', + ]); + const parsed = parseReplayScriptDetailed(healedScript); + // Exactly the repair run's own execution path: open, the corrective press, + // the surviving click, and the agent's own close — never the source + // plan's original (skipped) close, never a bare @ref. + expect(parsed.actions.map((a) => a.command)).toEqual(['open', 'press', 'click', 'close']); + const bareRefs = parsed.actions.flatMap((a) => a.positionals.filter((p) => p.startsWith('@'))); + expect(bareRefs).toEqual([]); + + // ============================================================ + // Part 2 — a diverged-and-abandoned repair leaves NO partial file. + // ============================================================ + const abandoned = setup('agent-device-repair-transaction-abandoned-'); + const abandonedFilePath = writeReplayFile(abandoned.root, [ + 'open "Demo" --relaunch', + SAVE_ANNOTATION, + 'click id="save"', + 'close', + ]); + const abandonedInvoke = makeRecordingReplayInvoke({ + sessionStore: abandoned.sessionStore, + sessionName: abandoned.sessionName, + }); + + const abandonedLeg1 = await runReplayScriptFile({ + req: baseReq({ positionals: [abandonedFilePath], flags: { saveScript: true } }), + sessionName: abandoned.sessionName, + logPath: abandoned.logPath, + sessionStore: abandoned.sessionStore, + invoke: abandonedInvoke, + }); + expect(abandonedLeg1.ok).toBe(false); + + // The agent walks away: a plain `close` (no --save-script) reaches the + // still repair-armed session — Fix 1/2's "abort/discard", not a commit. + const abandonedCloseResponse = await handleCloseCommand({ + req: { + token: 't', + session: abandoned.sessionName, + command: 'close', + positionals: [], + flags: {}, + }, + sessionName: abandoned.sessionName, + logPath: abandoned.logPath, + sessionStore: abandoned.sessionStore, + leaseRegistry: abandoned.leaseRegistry, + }); + expect(abandonedCloseResponse.ok).toBe(true); + + const abandonedHealedPath = path.join(abandoned.root, 'flow.healed.ad'); + expect(fs.existsSync(abandonedHealedPath)).toBe(false); + // No stray temp artifact either. + expect( + fs.existsSync(path.dirname(abandonedHealedPath)) + ? fs.readdirSync(path.dirname(abandonedHealedPath)) + : [], + ).toEqual(['flow.ad']); +}); + +test('C5a: an incomplete repair reaped by idle-reap leaves a tombstone (no healed file); a fresh replay --save-script clears it', async () => { + const { root, sessionStore, sessionName, logPath } = setup( + 'agent-device-repair-transaction-reap-', + ); + const filePath = writeReplayFile(root, [ + 'open "Demo" --relaunch', + SAVE_ANNOTATION, + 'click id="save"', + 'close', + ]); + const invoke = makeRecordingReplayInvoke({ sessionStore, sessionName }); + + const leg1 = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(leg1.ok).toBe(false); + const session = sessionStore.get(sessionName)!; + expect(session.saveScriptComplete).toBeFalsy(); + expect(session.repairSourcePath).toBe(filePath); + + // 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.delete(sessionName); + expect(fs.existsSync(path.join(root, 'flow.healed.ad'))).toBe(false); + + const tombstone = sessionStore.readRepairTombstone(sessionName); + expect(tombstone).toBeDefined(); + expect(tombstone?.sourcePath).toBe(filePath); + + // A fresh `replay --save-script` on the same key clears the tombstone. + await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke: makeRecordingReplayInvoke({ sessionStore, sessionName }), + }); + expect(sessionStore.readRepairTombstone(sessionName)).toBeUndefined(); +}); + +test('C5a: teardown of a COMPLETE repair auto-commits the healed file and writes NO tombstone', async () => { + const { root, sessionStore, sessionName, logPath } = setup( + 'agent-device-repair-transaction-autocommit-', + ); + // A clean (non-diverging) repair-armed replay completes the plan. + const filePath = writeReplayFile(root, ['open "Demo" --relaunch', 'click id="save-v2"']); + const invoke = makeRecordingReplayInvoke({ + sessionStore, + sessionName, + evidence: (req) => (req.command === 'click' ? freshEvidence('save-v2', 'Save V2') : undefined), + }); + + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(response.ok).toBe(true); + const session = sessionStore.get(sessionName)!; + expect(session.saveScriptComplete).toBe(true); + + // 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); + expect(fs.existsSync(path.join(root, 'flow.healed.ad'))).toBe(true); + expect(fs.readFileSync(path.join(root, 'flow.healed.ad'), 'utf8')).toContain( + HEAL_COMPLETE_SENTINEL, + ); + expect(sessionStore.readRepairTombstone(sessionName)).toBeUndefined(); +}); + +test('BLOCKER 1: a --from continuation on a reaped session returns SESSION_NOT_FOUND (translated to REPAIR_SESSION_EXPIRED), not a REPLAY_DIVERGENCE', async () => { + const { root, sessionStore, sessionName, logPath } = setup( + 'agent-device-repair-transaction-from-reaped-', + ); + const filePath = writeReplayFile(root, [ + 'open "Demo" --relaunch', + SAVE_ANNOTATION, + 'click id="save"', + 'click id="confirm"', + 'close', + ]); + const invoke = makeRecordingReplayInvoke({ sessionStore, sessionName }); + + // Leg 1 arms + diverges (save renamed to save-v2 in the mock tree). + const leg1 = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(leg1.ok).toBe(false); + if (leg1.ok) return; + const leg1Divergence = leg1.error.details?.divergence as { resume: { planDigest: string } }; + const digest = leg1Divergence.resume.planDigest; + + // Idle-reap tears the incomplete repair down, leaving a tombstone. + sessionStore.finalizeRepairTeardown(sessionStore.get(sessionName)!); + sessionStore.delete(sessionName); + expect(sessionStore.readRepairTombstone(sessionName)).toBeDefined(); + + // A `--from` continuation targeting the (now reaped) session must surface + // SESSION_NOT_FOUND — NOT a REPLAY_DIVERGENCE wrapping the first step's + // failure — so the router translates it to REPAIR_SESSION_EXPIRED. + const resumed = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath], flags: { replayFrom: 3, replayPlanDigest: digest } }), + sessionName, + logPath, + sessionStore, + invoke, + }); + expect(resumed.ok).toBe(false); + if (resumed.ok) return; + expect(resumed.error.code).toBe('SESSION_NOT_FOUND'); + expect(resumed.error.code).not.toBe('REPLAY_DIVERGENCE'); +}); + +/** A COMPLETE, committable repair-armed session at the default healed sibling path. */ +function makeCompleteRepairSession(sessionStore: SessionStore, sessionName: string, root: string) { + const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); + session.recordSession = true; + session.saveScriptBoundary = 0; + session.saveScriptComplete = true; + session.saveScriptPath = path.join(root, 'flow.healed.ad'); + session.saveScriptDefaultedHealedPath = true; + session.actions = [ + { ts: 1, command: 'open', positionals: ['Demo'], flags: {} }, + { + ts: 2, + command: 'press', + positionals: ['@e7'], + flags: {}, + result: { selectorChain: ['id="save-v2"'] }, + targetEvidence: freshEvidence('save-v2', 'Save V2'), + }, + ]; + sessionStore.set(sessionName, session); + return session; +} + +test('BLOCKER 2b/2c: a close whose commit FAILS (no-clobber) keeps the session for retry and surfaces a distinct error', async () => { + const { root, sessionStore, sessionName, logPath, leaseRegistry } = setup( + 'agent-device-repair-transaction-commit-fail-', + ); + makeCompleteRepairSession(sessionStore, sessionName, root); + // A prior COMPLETE (sentinel-marked) healed artifact already sits at the + // default path — the commit must refuse to clobber it. + fs.writeFileSync( + path.join(root, 'flow.healed.ad'), + `context platform=ios device="x"\nclick id="old"\n${HEAL_COMPLETE_SENTINEL}\n`, + ); + const before = fs.readFileSync(path.join(root, 'flow.healed.ad'), 'utf8'); + + const closeResponse = await handleCloseCommand({ + req: { token: 't', session: sessionName, command: 'close', positionals: [], flags: {} }, + sessionName, + logPath, + sessionStore, + leaseRegistry, + }); + + // BLOCKER 2b: the commit failed, so the session is NOT torn down — it stays + // addressable so the agent can retry (e.g. `close --save-script=`). + expect(closeResponse.ok).toBe(false); + expect(sessionStore.get(sessionName)).toBeDefined(); + // BLOCKER 2c: a no-clobber refusal is a distinct, surfaced error (not a silent + // success, not a swallowed skip), distinguishable from a filesystem failure. + if (!closeResponse.ok) { + expect(closeResponse.error.message).toMatch(/already exists/); + } + // The prior complete artifact is untouched. + expect(fs.readFileSync(path.join(root, 'flow.healed.ad'), 'utf8')).toBe(before); + // The rolled-back finalize `close` did not linger, so a retry does not + // accumulate a duplicate `close` in the healed slice. + expect(sessionStore.get(sessionName)!.actions.filter((a) => a.command === 'close')).toHaveLength( + 0, + ); + + // Retry with an explicit path commits cleanly — exactly ONE terminal close. + const retryPath = path.join(root, 'flow.promoted.ad'); + const retry = await handleCloseCommand({ + req: { + token: 't', + session: sessionName, + command: 'close', + positionals: [], + flags: { saveScript: retryPath }, + }, + sessionName, + logPath, + sessionStore, + leaseRegistry, + }); + expect(retry.ok).toBe(true); + if (retry.ok) expect(retry.data?.savedScript).toBe(retryPath); + const promoted = parseReplayScriptDetailed(fs.readFileSync(retryPath, 'utf8')); + expect(promoted.actions.filter((a) => a.command === 'close')).toHaveLength(1); +}); + +test('BLOCKER 3: a competing second writer never overwrites a COMPLETE artifact and gets a clear no-clobber error', async () => { + const { root, sessionStore, sessionName } = setup('agent-device-repair-transaction-competing-'); + const healedPath = path.join(root, 'flow.healed.ad'); + + // Writer 1 commits a complete artifact at the default healed path. + const first = makeCompleteRepairSession(sessionStore, `${sessionName}-1`, root); + const r1 = sessionStore.writeSessionLog(first); + expect(r1.written).toBe(true); + const committed = fs.readFileSync(healedPath, 'utf8'); + expect(committed).toContain(HEAL_COMPLETE_SENTINEL); + + // Writer 2 (a second repair on the same source → same default path) attempts + // to publish over it. The atomic create-exclusive publish must FAIL rather + // than overwrite the complete artifact. + const second = makeCompleteRepairSession(sessionStore, `${sessionName}-2`, root); + second.actions[1] = { + ts: 2, + command: 'press', + positionals: ['@e9'], + flags: {}, + result: { selectorChain: ['id="different"'] }, + targetEvidence: freshEvidence('different', 'Different'), + }; + const r2 = sessionStore.writeSessionLog(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. + expect(fs.readFileSync(healedPath, 'utf8')).toBe(committed); +}); diff --git a/src/daemon/handlers/session-close.ts b/src/daemon/handlers/session-close.ts index e4a6e6a42..44ab628aa 100644 --- a/src/daemon/handlers/session-close.ts +++ b/src/daemon/handlers/session-close.ts @@ -1,4 +1,5 @@ import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import { AppError } from '../../kernel/errors.ts'; import { scheduleIosRunnerIdleStop } from '../../platforms/apple/core/runner/runner-client.ts'; import { isApplePlatform, type DeviceInfo } from '../../kernel/device.ts'; import { dispatchCommand } from '../../core/dispatch.ts'; @@ -60,10 +61,74 @@ function shouldStopAppleRunnerBeforeTargetedClose(session: SessionState): boolea return isApplePlatform(session.device.platform) && !isIosSimulator(session.device); } -// Runs the failure-isolated resource teardown and the targeted platform close. -// Returns the preserved platform-close error (if any); best-effort cleanup -// failures are pushed into `cleanupFailures`. Never throws for a cleanup step so -// the caller's lease release and session deletion always run. +/** + * ADR 0012 decision 6 (BLOCKER 2): outcome of committing a repair transaction + * at `close` time, BEFORE any destructive teardown. `not-armed` = not a repair + * session (normal close flow); `committed` = the healed `.ad` was written + * (`path`) or the transaction was incomplete and intentionally discarded (no + * `path`) — either way close proceeds and tears the session down; `failed` = a + * COMPLETE transaction's commit failed (no-clobber / bare-`@ref` / fs error), + * so the session must be KEPT for retry and the failure surfaced. + */ +type RepairCloseOutcome = + | { kind: 'not-armed' } + | { kind: 'committed'; path?: string } + | { kind: 'failed'; error: AppError }; + +function commitRepairBeforeClose( + sessionStore: SessionStore, + session: SessionState, + req: DaemonRequest, +): RepairCloseOutcome { + if (session.saveScriptBoundary === undefined) return { kind: 'not-armed' }; + // Record the finalize `close` (so the committed healed slice ends with it), + // then COMMIT before any destructive teardown. A repair-armed session commits + // iff the transaction COMPLETED, regardless of `--save-script` on the close + // (C2); `recordSession` is already true from arming. + const actionsBeforeClose = session.actions.length; + recordSessionAction(sessionStore, session, req, 'close', { + session: session.name, + ...successText(`Closed: ${session.name}`), + }); + const result = sessionStore.writeSessionLog(session); + if (result.written) return { kind: 'committed', path: result.path }; + if (result.error) { + // The session is kept for retry (BLOCKER 2b): roll back the just-recorded + // finalize `close` so a subsequent `close --save-script=` retry does + // not accumulate duplicate `close` lines in the healed slice. + session.actions.length = actionsBeforeClose; + return { kind: 'failed', error: result.error }; + } + return { kind: 'committed' }; +} + +/** + * ADR 0012 decision 6 (BLOCKER 2b): a commit-failure close response. The session + * is intentionally NOT torn down (the caller returns before teardown), so the + * agent can fix the cause and retry `close --save-script`. + */ +function buildRepairCloseFailureResponse(session: SessionState, error: AppError): DaemonResponse { + const hint = typeof error.details?.hint === 'string' ? error.details.hint : undefined; + return { + ok: false, + error: { + code: error.code, + message: error.message, + ...(hint ? { hint } : {}), + details: { + session: session.name, + ...(session.saveScriptPath ? { savedScript: session.saveScriptPath } : {}), + // The session was preserved: retry after fixing the cause. + retriable: false, + }, + }, + }; +} + +// Runs the failure-isolated resource teardown and the targeted platform close +// (#1225). Returns the preserved platform-close error (if any); best-effort +// cleanup failures are pushed into `cleanupFailures`. Never throws for a cleanup +// step so the caller's lease release and session deletion always run. async function runSessionCloseTeardown(params: { req: DaemonRequest; session: SessionState; @@ -71,8 +136,9 @@ async function runSessionCloseTeardown(params: { logPath: string; sessionStore: SessionStore; cleanupFailures: SessionCleanupFailure[]; + repairArmed: boolean; }): Promise { - const { req, session, sessionName, logPath, sessionStore, cleanupFailures } = params; + const { req, session, sessionName, logPath, sessionStore, cleanupFailures, repairArmed } = params; const attemptCleanup = async (step: string, run: () => Promise): Promise => { try { await run(); @@ -85,23 +151,26 @@ async function runSessionCloseTeardown(params: { // its AppError (code/details/hint) is preserved and returned for the caller to // rethrow, and a failed close must not be recorded as `Closed`. Subsequent // resource cleanup still runs regardless. - const platformCloseError = await dispatchTargetedPlatformClose({ - req, - session, - logPath, - }); + const platformCloseError = await dispatchTargetedPlatformClose({ req, session, logPath }); await stopOrRetainAppleRunnerAfterClose(req, session, attemptCleanup); await clearSessionRuntimeHints(session, sessionStore, sessionName); - if (!platformCloseError) { - recordSessionAction(sessionStore, session, req, 'close', { - session: session.name, - ...successText(`Closed: ${session.name}`), - }); - } - if (req.flags?.saveScript) { - session.recordSession = true; + // ADR 0012 decision 6 (BLOCKER 2): a repair-armed session already recorded its + // finalize `close` and committed (or aborted) its healed `.ad` BEFORE this + // teardown (commit-state machine — the single commit path). Only an ordinary + // (non-repair) session records `close` + writes its session log here, and — + // per #1225 — a failed platform close is not recorded as `Closed`. + if (!repairArmed) { + if (!platformCloseError) { + recordSessionAction(sessionStore, session, req, 'close', { + session: session.name, + ...successText(`Closed: ${session.name}`), + }); + } + if (req.flags?.saveScript) { + session.recordSession = true; + } + sessionStore.writeSessionLog(session); } - sessionStore.writeSessionLog(session); await attemptCleanup('materialized_paths', () => cleanupRetainedMaterializedPathsForSession(sessionName), ); @@ -207,6 +276,14 @@ export async function handleCloseCommand(params: { if (!session) { return await closeWithoutSession(req, logPath); } + // ADR 0012 decision 6 (BLOCKER 2): commit the repair transaction BEFORE any + // destructive teardown. On failure, return without tearing the session down — + // it stays addressable so the agent can fix the cause and retry. + const repairCommit = commitRepairBeforeClose(sessionStore, session, req); + if (repairCommit.kind === 'failed') { + return buildRepairCloseFailureResponse(session, repairCommit.error); + } + const healedScriptPath = repairCommit.kind === 'committed' ? repairCommit.path : undefined; let providerData: Record | undefined; // Resource teardown is failure-isolated: a rejected step is collected instead of // short-circuiting the rest, so every subsequent resource (and the runner stop) @@ -222,6 +299,7 @@ export async function handleCloseCommand(params: { logPath, sessionStore, cleanupFailures, + repairArmed: repairCommit.kind !== 'not-armed', }); } finally { // Always drop the local session, even if provider-side release fails: @@ -246,6 +324,10 @@ export async function handleCloseCommand(params: { device: session.device, shutdownRequested: req.flags?.shutdown, }); + // ADR 0012 decision 6 (BLOCKER 2a): positively report the committed healed + // artifact path so the agent learns the repair published (and where) without + // an extra round-trip. + const savedScript = healedScriptPath ? { savedScript: healedScriptPath } : {}; if (shutdownResult) { return { ok: true, @@ -253,6 +335,7 @@ export async function handleCloseCommand(params: { { session: session.name, shutdown: shutdownResult, + ...savedScript, ...(providerData ? { provider: providerData } : {}), }, `Closed: ${session.name}`, @@ -264,6 +347,7 @@ export async function handleCloseCommand(params: { data: { session: session.name, ...successText(`Closed: ${session.name}`), + ...savedScript, ...(providerData ? { provider: providerData } : {}), }, }; diff --git a/src/daemon/handlers/session-replay-runtime.ts b/src/daemon/handlers/session-replay-runtime.ts index bc6603486..0886f61f1 100644 --- a/src/daemon/handlers/session-replay-runtime.ts +++ b/src/daemon/handlers/session-replay-runtime.ts @@ -12,7 +12,7 @@ import { SessionStore } from '../session-store.ts'; import { expandSessionPath } from '../session-paths.ts'; import { type ReplayScriptMetadata } from '../../replay/script.ts'; import { computeReplayPlanDigest } from '../../replay/plan-digest.ts'; -import { errorResponse } from './response.ts'; +import { errorResponse, noActiveSessionError } from './response.ts'; import { invokeReplayAction } from './session-replay-action-runtime.ts'; import { tryParseSelectorChain } from '../../selectors/index.ts'; import type { ResponseLevel } from '../../kernel/contracts.ts'; @@ -206,6 +206,26 @@ export async function runReplayScriptFile(params: { sessionName, }); if (repairRunPreflight) return repairRunPreflight; + // ADR 0012 decision 6, R7 (BLOCKER 1): a `--from` continuation (entryIndex > 0) + // resumes an EXISTING session — it never replays step 1 (`open`), so it + // cannot create one. If the session is gone (its repair was idle-reaped), + // surface SESSION_NOT_FOUND HERE so the router translates it to + // REPAIR_SESSION_EXPIRED via the tombstone — instead of the missing session + // surfacing later as a REPLAY_DIVERGENCE that wraps the first step's failure + // and slips past the tombstone translation. + if (entryIndex.value > 0 && !sessionStore.get(sessionName)) { + return noActiveSessionError(); + } + if (req.flags?.saveScript) { + // ADR 0012 decision 6, R7 (C5a): a fresh `replay --save-script` on this + // key clears any prior reap tombstone before starting a new transaction. + sessionStore.clearRepairTombstone(sessionName); + } + // ADR 0012 decision 6 (C2): a repair-armed resume re-opens the completion + // window — a leg that re-diverges must not inherit a prior leg's COMPLETE + // flag and let a later `close` commit a now-stale transaction. + const preRunSession = sessionStore.get(sessionName); + if (preRunSession?.saveScriptBoundary !== undefined) preRunSession.saveScriptComplete = false; const armReplaySaveScriptStep = createReplaySaveScriptArmer({ saveScript: req.flags?.saveScript, sessionStore, @@ -248,7 +268,26 @@ export async function runReplayScriptFile(params: { for (let index = entryIndex.value; index < actions.length; index += 1) { const action = actions[index]; if (!action || action.command === 'replay') continue; + // ADR 0012 decision 6, R1 (BLOCKER 4): arm BEFORE the terminal-close + // check. Pre-`open` arming is a no-op (no session yet), so for a script + // whose first step CREATES the session (`open`), the boundary is only set + // once a later iteration runs the armer on the now-existing session. + // Arming first means a minimal `[open, close]` transaction arms at the + // `close` step, so `isRepairArmedTerminalClose` then sees the boundary and + // treats the terminal `close` as lifecycle (skipped) rather than + // dispatching it and tearing the session down un-armed. armReplaySaveScriptStep(); + if ( + isRepairArmedTerminalClose({ + action, + index, + totalActions: actions.length, + sessionStore, + sessionName, + }) + ) { + continue; + } emitReplayTestActionProgress(resolved, index, actions.length, action); const sampleStart = readSessionSnapshotSampleCount(sessionStore, sessionName); const response = await resolveReplayStepResponse(stepContext, action, index, [ @@ -259,14 +298,34 @@ export async function runReplayScriptFile(params: { ); collectReplayActionArtifactPaths(response).forEach((entry) => artifactPaths.add(entry)); if (!response.ok) { + // ADR 0012 decision 6, R7 (C1): a divergence from a repair-armed run + // keeps its session live — mark the wire signal so the client keeps the + // owning daemon alive and the agent knows the session is addressable. + const held = (r: DaemonResponse): DaemonResponse => + markRepairSessionHeldIfArmed({ response: r, sessionStore, sessionName }); // A complete target-binding divergence must pass through unchanged — // failStep would rebuild it as a generic action-failure divergence // (double-capture + lost kind/targetBinding). - if (isCompleteTargetBindingDivergenceResponse(response)) return response; - return await failStep(response, action, index); + if (isCompleteTargetBindingDivergenceResponse(response)) return held(response); + return held(await failStep(response, action, index)); } } + // ADR 0012 decision 6, R1 (BLOCKER 4): a final arm so a repair whose LAST + // executable step created the session (e.g. a bare `[open]`, or `[open, + // close]` where the close is skipped) still arms the transaction before the + // completion/commit gate below evaluates it. + armReplaySaveScriptStep(); + + // ADR 0012 decision 6 (C2): the loop reached the last executable step with + // no outstanding divergence (the terminal source `close` was skipped, C4) — + // the repair transaction is now COMPLETE and commit-eligible. + const completedSession = sessionStore.get(sessionName); + if (completedSession?.saveScriptBoundary !== undefined) { + completedSession.saveScriptComplete = true; + sessionStore.set(sessionName, completedSession); + } + const replayedCount = actions.length - entryIndex.value; const snapshotDiagnosticsSummary = summarizeSnapshotTimingSamples(snapshotDiagnosticSamples); const wallClockMs = Date.now() - startedAt; @@ -420,6 +479,32 @@ function preflightReplayAgainstActiveRepair(params: { ); } +/** + * ADR 0012 decision 6 (Fix 3): the source plan's own terminal `close` is + * lifecycle, not a script step to replay, while a repair is armed — the agent + * finalizes the transaction with `close --save-script` instead + * (`session-close.ts`). Replaying the recorded `close` here would dispatch it + * as an ordinary step: it tears the session down (and, absent Fix 1/2, could + * even publish or diverge) before the agent gets that chance. Skipped exactly + * like the `replay` pseudo-command just above it in the loop — never + * dispatched, never divergence-checked, and (like that skip) not counted out + * of `replayedCount`. Checked against session state, not this invocation's + * own flags, matching R2: a repair stays armed across separate `--from` legs + * regardless of whether `--save-script` is repeated on each one. + */ +function isRepairArmedTerminalClose(params: { + action: SessionAction; + index: number; + totalActions: number; + sessionStore: SessionStore; + sessionName: string; +}): boolean { + const { action, index, totalActions, sessionStore, sessionName } = params; + if (action.command !== 'close') return false; + if (index !== totalActions - 1) return false; + return sessionStore.get(sessionName)?.saveScriptBoundary !== undefined; +} + /** * ADR 0012 decision 6, R1/R6: returns a per-step armer that sets * `recordSession` and stamps the repair-run boundary watermark ONCE. Absent @@ -475,9 +560,49 @@ function armReplaySaveScriptStep(params: { if (session.saveScriptBoundary === undefined) { session.saveScriptBoundary = firstArm ? session.actions.length : 0; } + // ADR 0012 decision 6, R7 (C5a): stash the original replay input so a reap + // tombstone can hand back an actionable `replay --save-script` re-run. + if (session.repairSourcePath === undefined) session.repairSourcePath = sourcePath; sessionStore.set(sessionName, session); } +/** + * ADR 0012 decision 6, R7 (C1): stamps the `resume.repairSessionHeld` liveness + * signal on a repair-armed divergence — the honest wire marker that the owning + * session was kept live (this daemon never tears it down on a divergence) and + * remains addressable for the corrective press + `replay --from`/`close`. Set + * only when the session is genuinely held (armed): a plain non-repair + * divergence, or one before step-1 `open` created/armed the session, gets no + * signal (and no keep-alive). Never `false` — absent when not held. + */ +function markRepairSessionHeldIfArmed(params: { + response: DaemonResponse; + sessionStore: SessionStore; + sessionName: string; +}): DaemonResponse { + const { response, sessionStore, sessionName } = params; + if (response.ok) return response; + // The transaction is active iff the session is repair-armed and not yet + // committed — the PERSISTED state, NOT this request's `--save-script` flag. + // A `replay --from` continuation (which does not repeat `--save-script`, per + // R2) is therefore still held on divergence and stays in the transaction. + const session = sessionStore.get(sessionName); + if (session?.saveScriptBoundary === undefined || session.saveScriptCommitted) return response; + const resume = readDivergenceResumeRecord(response); + if (resume) resume.repairSessionHeld = true; + return response; +} + +/** The mutable `details.divergence.resume` record on a failed response, or `undefined`. */ +function readDivergenceResumeRecord( + response: Extract, +): Record | undefined { + const divergence = response.error.details?.divergence; + if (!divergence || typeof divergence !== 'object') return undefined; + const resume = (divergence as Record).resume; + return resume && typeof resume === 'object' ? (resume as Record) : undefined; +} + /** `flows/login.ad` -> `flows/login.healed.ad`, beside the original (R6). */ function healedScriptSiblingPath(sourcePath: string): string { const dir = path.dirname(sourcePath); diff --git a/src/daemon/http-errors.ts b/src/daemon/http-errors.ts index 94c222a50..f119cacb0 100644 --- a/src/daemon/http-errors.ts +++ b/src/daemon/http-errors.ts @@ -10,6 +10,9 @@ export function statusCodeForNormalizedError(code: string): number { case 'UNAUTHORIZED': return 401; case 'SESSION_NOT_FOUND': + // ADR 0012 R7 (C5a): a reaped repair session is a gone-session state, like + // SESSION_NOT_FOUND — not an internal error. + case 'REPAIR_SESSION_EXPIRED': return 404; default: return 500; diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 3f59720f6..88594f1f6 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -114,7 +114,11 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { // wasted round-trip. Returned unchanged when neither applies, so the // default error wire shape is preserved. if (!response.ok) { - return { ok: false, error: enrichDaemonError(req.command, response.error) }; + // ADR 0012 decision 6, R7 (C5a): a command that finds no session but + // hits a live repair tombstone gets `REPAIR_SESSION_EXPIRED` with + // re-run guidance, never a bare SESSION_NOT_FOUND. + const error = repairExpiredIfTombstoned(req, response.error, sessionStore); + return { ok: false, error: enrichDaemonError(req.command, error) }; } // Phase 4 (agent-cost) grafts on the success path. Runs inside the // diagnostics scope so cost can read this request's runner-round-trip tally. @@ -336,6 +340,31 @@ function recordThrownRequestEvent( ); } +/** + * ADR 0012 decision 6, R7 (C5a): when a request finds no session (SESSION_NOT_FOUND) + * but a live repair tombstone exists for its session key, rewrite the error to + * `REPAIR_SESSION_EXPIRED` with an actionable re-run command. Any other error, or + * the absence of a (non-expired) tombstone, passes through untouched. + */ +function repairExpiredIfTombstoned( + req: DaemonRequest, + error: DaemonError, + sessionStore: SessionStore, +): DaemonError { + if (error.code !== 'SESSION_NOT_FOUND') return error; + const tombstone = sessionStore.readRepairTombstone(req.session); + if (!tombstone) return error; + const reRun = tombstone.sourcePath + ? `re-run: replay ${tombstone.sourcePath} --save-script` + : 're-run your replay