Skip to content

Commit 1eaec72

Browse files
committed
fix: make replay session handoff reliable
1 parent 5dacea4 commit 1eaec72

6 files changed

Lines changed: 215 additions & 64 deletions

File tree

packages/contracts/src/replay.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ export type ReplayCommandResult = {
77
session: string;
88
/**
99
* True iff `session` still exists in the daemon's session store when the
10-
* response is built — i.e. the replayed script had no terminal `close`
11-
* (ADR 0016's consumption contract). The client uses this, not script
12-
* parsing, to decide whether an owned one-shot daemon must stay alive so
13-
* the caller can keep addressing this session.
10+
* response is built. This remains true when replay suppresses an authored
11+
* terminal `close` for an explicit live-session handoff. The client uses
12+
* this, not script parsing, to decide whether an owned one-shot daemon must
13+
* stay alive so the caller can keep addressing this session.
1414
*/
1515
sessionActive: boolean;
1616
artifactPaths: string[];

src/daemon/handlers/__tests__/session-replay-runtime.test.ts

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,60 @@ test('a close-less replay reports sessionActive: true (real producer, session st
7474
expect(response.ok).toBe(true);
7575
if (!response.ok) return;
7676
expect(sessionStore.get(sessionName)).toBeDefined();
77-
expect((response.data as { sessionActive: boolean }).sessionActive).toBe(true);
77+
expect(response.data).toMatchObject({ sessionActive: true, replayed: 2 });
78+
});
79+
80+
test('--keep-session suppresses a close that is terminal among executable actions', async () => {
81+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-keep-marker-tail-'));
82+
const sessionStore = new SessionStore(path.join(root, 'sessions'));
83+
const sessionName = 'default';
84+
sessionStore.set(sessionName, makeIosSession(sessionName));
85+
const filePath = writeReplayFile(root, ['open "Demo"', 'close', 'replay "./nested-flow.ad"']);
86+
const commands: string[] = [];
87+
88+
const response = await runReplayScriptFile({
89+
req: baseReq({ positionals: [filePath], flags: { replayKeepSession: true } }),
90+
sessionName,
91+
logPath: path.join(root, 'daemon.log'),
92+
sessionStore,
93+
invoke: async (req) => {
94+
commands.push(req.command);
95+
if (req.command === 'close') sessionStore.delete(sessionName);
96+
return { ok: true, data: {} };
97+
},
98+
});
99+
100+
expect(response.ok).toBe(true);
101+
expect(commands).toEqual(['open']);
102+
if (!response.ok) return;
103+
expect(response.data).toMatchObject({ sessionActive: true, replayed: 1 });
104+
});
105+
106+
test('--keep-session fails explicitly when the completed replay has no live session', async () => {
107+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-keep-postcondition-'));
108+
const sessionStore = new SessionStore(path.join(root, 'sessions'));
109+
const sessionName = 'default';
110+
sessionStore.set(sessionName, makeIosSession(sessionName));
111+
const filePath = writeReplayFile(root, ['open "Demo"', 'click "Log out"']);
112+
113+
const response = await runReplayScriptFile({
114+
req: baseReq({ positionals: [filePath], flags: { replayKeepSession: true } }),
115+
sessionName,
116+
logPath: path.join(root, 'daemon.log'),
117+
sessionStore,
118+
invoke: async (req) => {
119+
if (req.command === 'click') sessionStore.delete(sessionName);
120+
return { ok: true, data: {} };
121+
},
122+
});
123+
124+
expect(response).toMatchObject({
125+
ok: false,
126+
error: {
127+
code: 'COMMAND_FAILED',
128+
message: expect.stringContaining('--keep-session could not preserve session'),
129+
},
130+
});
78131
});
79132

80133
test('a replay whose terminal close removes the session reports sessionActive: false', async () => {
@@ -128,7 +181,7 @@ test('--keep-session suppresses only the authored terminal close and reports the
128181
expect(response.ok).toBe(true);
129182
if (!response.ok) return;
130183
expect(sessionStore.get(sessionName)).toBeDefined();
131-
expect((response.data as { sessionActive: boolean }).sessionActive).toBe(true);
184+
expect(response.data).toMatchObject({ sessionActive: true, replayed: 2 });
132185
});
133186

134187
test('--keep-session preserves an interior close instead of broad command filtering', async () => {
@@ -225,7 +278,7 @@ test('Maestro YAML uses the typed engine while .ad remains generic', async () =>
225278
const yamlResponse = await runReplayScriptFile({
226279
req: baseReq({
227280
positionals: [yamlPath],
228-
flags: { replayBackend: 'maestro', platform: 'ios' },
281+
flags: { replayBackend: 'maestro', platform: 'ios', replayKeepSession: false },
229282
}),
230283
sessionName,
231284
logPath: path.join(root, 'daemon.log'),

src/daemon/handlers/__tests__/session-replay.test.ts

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { LeaseRegistry } from '../../lease-registry.ts';
88
import type { DaemonRequest, DaemonResponse } from '../../types.ts';
99
import { makeIosSession } from '../../../__tests__/test-utils/index.ts';
1010
import { buildNestedReplayFlags, handleSessionReplayCommands } from '../session-replay.ts';
11+
import { REPLAY_ONLY_TEST_FLAG_REJECTIONS } from '../session-replay-test-policy.ts';
12+
import { replayCommandFamily } from '../../../commands/replay/index.ts';
1113

1214
const recordTraceMocks = vi.hoisted(() => ({
1315
handleRecordCommand: vi.fn(),
@@ -209,7 +211,7 @@ test('buildNestedReplayFlags strips test-only recordVideo before replay actions
209211
assert.deepEqual(result, { platform: 'ios' });
210212
});
211213

212-
test('test --record-video records each replay attempt on the generated test session', async () => {
214+
test('test normalizes false replay-only booleans while recording each replay attempt', async () => {
213215
vi.useFakeTimers({ now: 1_000 });
214216
const { root, replayPath, sessionStore, nestedRequests, events } = createRecordVideoFixture();
215217
installMockRecordingHandler(sessionStore, { recordingPath: '', events });
@@ -220,7 +222,13 @@ test('test --record-video records each replay attempt on the generated test sess
220222
session: 'default',
221223
command: 'test',
222224
positionals: [replayPath],
223-
flags: { recordVideo: true, artifactsDir: path.join(root, 'artifacts') },
225+
flags: {
226+
recordVideo: true,
227+
replayKeepSession: false,
228+
saveScript: false,
229+
force: false,
230+
artifactsDir: path.join(root, 'artifacts'),
231+
},
224232
meta: { cwd: root, requestId: 'record-video-suite' },
225233
},
226234
sessionName: 'default',
@@ -272,6 +280,20 @@ test('test --record-video records each replay attempt on the generated test sess
272280

273281
// --- ADR 0012 decision 4 / migration step 5: `--from` is replay-only ---
274282

283+
test('raw test-request guards enumerate every daemon-visible replay-only CLI flag', () => {
284+
const replayFlags = replayCommandFamily.cliSchemas.replay?.allowedFlags ?? [];
285+
const testFlags = new Set(replayCommandFamily.cliSchemas.test?.allowedFlags ?? []);
286+
const clientOnlyReplayFlags = new Set(['out']);
287+
const expectedDaemonFlags = replayFlags
288+
.filter((flag) => !testFlags.has(flag) && !clientOnlyReplayFlags.has(flag))
289+
.sort();
290+
291+
const guardedDaemonFlags = REPLAY_ONLY_TEST_FLAG_REJECTIONS.flatMap(
292+
(rejection) => rejection.keys,
293+
).sort();
294+
assert.deepEqual(guardedDaemonFlags, expectedDaemonFlags);
295+
});
296+
275297
test('test rejects raw --keep-session with INVALID_ARGS before running the suite', async () => {
276298
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-test-keep-session-rejected-'));
277299
const replayPath = path.join(root, 'flow.ad');
@@ -396,3 +418,34 @@ test('test rejects --save-script with INVALID_ARGS before running the suite', as
396418
assert.equal(response.error.code, 'INVALID_ARGS');
397419
assert.match(response.error.message, /--save-script/);
398420
});
421+
422+
test('test rejects raw --force without --save-script before running the suite', async () => {
423+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-test-force-rejected-'));
424+
const replayPath = path.join(root, 'flow.ad');
425+
fs.writeFileSync(replayPath, 'open "Demo"\n');
426+
const sessionStore = new SessionStore(path.join(root, 'sessions'));
427+
const invoke = vi.fn(async () => ({ ok: true as const, data: {} }));
428+
429+
const response = await handleSessionReplayCommands({
430+
req: {
431+
token: 'token',
432+
session: 'default',
433+
command: 'test',
434+
positionals: [replayPath],
435+
flags: { force: true },
436+
meta: { cwd: root },
437+
},
438+
sessionName: 'default',
439+
logPath: path.join(root, 'daemon.log'),
440+
sessionStore,
441+
leaseRegistry: new LeaseRegistry(),
442+
invoke,
443+
});
444+
445+
if (!response) throw new Error('Expected response');
446+
assert.equal(response.ok, false);
447+
if (response.ok) return;
448+
assert.equal(response.error.code, 'INVALID_ARGS');
449+
assert.match(response.error.message, /--force/);
450+
assert.equal(invoke.mock.calls.length, 0);
451+
});

src/daemon/handlers/session-replay-runtime.ts

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ export async function runReplayScriptFile(params: {
220220
}
221221

222222
const startedAt = Date.now();
223+
const keepSession = req.flags?.replayKeepSession === true;
223224
let resolved = '';
224225
const artifactPaths = new Set<string>();
225226
// #1478 P4b: the one locked coordinator this request reaches the repair
@@ -231,7 +232,7 @@ export async function runReplayScriptFile(params: {
231232
return errorResponse('INVALID_ARGS', maestroBackendRequiredMessage('replay', filePath));
232233
}
233234
if (resolveReplayFormat(resolved, req.flags?.replayBackend) === 'maestro') {
234-
if (req.flags?.replayKeepSession === true) {
235+
if (keepSession) {
235236
return errorResponse(
236237
'INVALID_ARGS',
237238
'--keep-session is supported only for native .ad replay; Maestro YAML owns its lifecycle.',
@@ -252,6 +253,7 @@ export async function runReplayScriptFile(params: {
252253
tracePath,
253254
resolved,
254255
coordinator,
256+
keepSession,
255257
});
256258
if (!planPreparation.ok) return planPreparation.response;
257259
const {
@@ -264,6 +266,7 @@ export async function runReplayScriptFile(params: {
264266
scope,
265267
actionTracePath,
266268
snapshotDiagnosticSamples,
269+
suppressedTerminalCloseIndex,
267270
} = planPreparation.value;
268271
const sessionPreparation = prepareReplaySession({
269272
req,
@@ -308,6 +311,7 @@ export async function runReplayScriptFile(params: {
308311
snapshotDiagnosticSamples,
309312
onStep,
310313
armSaveScript: sessionPreparation.armSaveScript,
314+
suppressedTerminalCloseIndex,
311315
});
312316
if (failure) return failure;
313317
return completeReplayRun({
@@ -320,6 +324,8 @@ export async function runReplayScriptFile(params: {
320324
snapshotDiagnosticSamples,
321325
armSaveScript: sessionPreparation.armSaveScript,
322326
coordinator,
327+
keepSession,
328+
suppressedTerminalCloseIndex,
323329
});
324330
} catch (err) {
325331
const appErr = asAppError(err);
@@ -348,6 +354,7 @@ type ReplayActionExecution = {
348354
snapshotDiagnosticSamples: SnapshotTimingSample[];
349355
onStep: ReplayTestAttemptStepSink | undefined;
350356
armSaveScript: () => void;
357+
suppressedTerminalCloseIndex: number | undefined;
351358
};
352359

353360
async function executeReplayActions(
@@ -363,24 +370,15 @@ async function executeReplayActions(
363370
snapshotDiagnosticSamples,
364371
onStep,
365372
armSaveScript,
373+
suppressedTerminalCloseIndex,
366374
} = params;
367375
for (let index = entryIndex; index < actions.length; index += 1) {
368376
const action = actions[index];
369377
if (!isExecutableReplayAction(action)) continue;
370378
// Arm before checking terminal close so `[open, close]` records the
371379
// session created by `open` before treating `close` as lifecycle.
372380
armSaveScript();
373-
if (
374-
shouldSkipTerminalClose({
375-
action,
376-
index,
377-
totalActions: actions.length,
378-
keepSession: params.req.flags?.replayKeepSession === true,
379-
coordinator: stepContext.coordinator,
380-
})
381-
) {
382-
continue;
383-
}
381+
if (index === suppressedTerminalCloseIndex) continue;
384382
onStep?.(replayActionStep(index, actions.length, action));
385383
const sampleStart = readSessionSnapshotSampleCount(sessionStore, sessionName);
386384
const response = await resolveReplayStepResponse(stepContext, action, index, [
@@ -441,6 +439,8 @@ function completeReplayRun(params: {
441439
snapshotDiagnosticSamples: SnapshotTimingSample[];
442440
armSaveScript: () => void;
443441
coordinator: ReplayCoordinator;
442+
keepSession: boolean;
443+
suppressedTerminalCloseIndex: number | undefined;
444444
}): DaemonResponse {
445445
const {
446446
startedAt,
@@ -452,11 +452,24 @@ function completeReplayRun(params: {
452452
snapshotDiagnosticSamples,
453453
armSaveScript,
454454
coordinator,
455+
keepSession,
456+
suppressedTerminalCloseIndex,
455457
} = params;
456458
armSaveScript();
457459
coordinator.markCompleteIfArmed();
458460
const completedSession = sessionStore.get(sessionName);
459-
const replayedCount = actions.length - entryIndex;
461+
if (keepSession && !completedSession) {
462+
return errorResponse(
463+
'COMMAND_FAILED',
464+
`Replay completed but --keep-session could not preserve session "${sessionName}". Run the script again after checking which action closed the session.`,
465+
artifactPaths.size > 0 ? { artifactPaths: [...artifactPaths] } : undefined,
466+
);
467+
}
468+
const replayedCount = countExecutedReplayActions({
469+
actions,
470+
entryIndex,
471+
suppressedTerminalCloseIndex,
472+
});
460473
const snapshotDiagnosticsSummary = summarizeSnapshotTimingSamples(snapshotDiagnosticSamples);
461474
return {
462475
ok: true,
@@ -521,6 +534,7 @@ type PreparedReplayPlan = {
521534
scope: ReplayVarScope;
522535
actionTracePath: string | undefined;
523536
snapshotDiagnosticSamples: SnapshotTimingSample[];
537+
suppressedTerminalCloseIndex: number | undefined;
524538
};
525539

526540
type ParsedReplayInput = ReturnType<typeof parseReplayInput>;
@@ -532,8 +546,9 @@ function prepareReplayPlan(params: {
532546
tracePath: string | undefined;
533547
resolved: string;
534548
coordinator: ReplayCoordinator;
549+
keepSession: boolean;
535550
}): { ok: true; value: PreparedReplayPlan } | { ok: false; response: DaemonResponse } {
536-
const { req, sessionName, sessionStore, tracePath, resolved, coordinator } = params;
551+
const { req, sessionName, sessionStore, tracePath, resolved, coordinator, keepSession } = params;
537552
const parsedResult = parseReplayScript(resolved, req);
538553
if (!parsedResult.ok) return parsedResult;
539554
const parsed = parsedResult.value;
@@ -571,6 +586,12 @@ function prepareReplayPlan(params: {
571586
scope: buildPreparedReplayScope({ req, replayReq, sessionName, resolved, metadata }),
572587
actionTracePath: tracePath ?? preEntrySession?.trace?.outPath,
573588
snapshotDiagnosticSamples: [],
589+
suppressedTerminalCloseIndex: resolveSuppressedTerminalCloseIndex({
590+
actions,
591+
keepSession,
592+
saveScript: req.flags?.saveScript,
593+
repairActive: coordinator.view()?.repairBoundary !== undefined,
594+
}),
574595
},
575596
};
576597
}
@@ -794,24 +815,37 @@ function preflightSaveScriptTarget(params: {
794815
}
795816

796817
/**
797-
* The one native replay lifecycle seam for an authored terminal `close`.
798-
* `--keep-session` suppresses it so callers can take over the live session;
799-
* ADR 0012 repair suppresses it so the agent can finalize through
800-
* `close --save-script`. Interior closes retain authored semantics. Repair is
801-
* checked against session state (not only this leg's flags), preserving R2
802-
* across separate `--from` continuations.
818+
* Resolves the one native replay lifecycle seam once per plan. Terminal means
819+
* the last executable action, because nested `replay` markers are plan
820+
* metadata and never dispatch. The suppressed close is therefore neither
821+
* divergence-checked nor included in the successful `replayed` count.
803822
*/
804-
function shouldSkipTerminalClose(params: {
805-
action: SessionAction;
806-
index: number;
807-
totalActions: number;
823+
function resolveSuppressedTerminalCloseIndex(params: {
824+
actions: SessionAction[];
808825
keepSession: boolean;
809-
coordinator: ReplayCoordinator;
810-
}): boolean {
811-
const { action, index, totalActions, keepSession, coordinator } = params;
812-
if (action.command !== 'close') return false;
813-
if (index !== totalActions - 1) return false;
814-
return keepSession || coordinator.view()?.repairBoundary !== undefined;
826+
saveScript: boolean | string | undefined;
827+
repairActive: boolean;
828+
}): number | undefined {
829+
if (!params.keepSession && !params.saveScript && !params.repairActive) return undefined;
830+
for (let index = params.actions.length - 1; index >= 0; index -= 1) {
831+
const action = params.actions[index];
832+
if (!isExecutableReplayAction(action)) continue;
833+
return action.command === 'close' ? index : undefined;
834+
}
835+
return undefined;
836+
}
837+
838+
function countExecutedReplayActions(params: {
839+
actions: SessionAction[];
840+
entryIndex: number;
841+
suppressedTerminalCloseIndex: number | undefined;
842+
}): number {
843+
let count = 0;
844+
for (let index = params.entryIndex; index < params.actions.length; index += 1) {
845+
if (index === params.suppressedTerminalCloseIndex) continue;
846+
if (isExecutableReplayAction(params.actions[index])) count += 1;
847+
}
848+
return count;
815849
}
816850

817851
/**

0 commit comments

Comments
 (0)