diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index e3a048d1d4..0e4c7c8066 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -875,6 +875,17 @@ works" and "healed scripts are always valid": prevent keep-alive, the armed replay must **fail-fast before step 1** with actionable guidance, never proceed and then fail a later `--from` with `SESSION_NOT_FOUND`. + **Interaction with the CLI client's one-shot teardown (issue #1384).** A repair-armed replay that + completes cleanly (no divergence, transaction reaches `COMPLETE`) skips its terminal source `close` by + design (above), so the session always survives the run. Once the client keeps a one-shot `replay`'s + owning daemon alive whenever the session survives (`ReplayCommandResult.sessionActive`, independent of + repair state), that keep-alive now applies here too: the healed `.ad` commit — gated on teardown, not on + the replay response — is deferred past the request that completed it, landing only when the agent issues + an explicit `close`/`close --save-script` or ordinary idle-reap tears the session down. This is the + correct shape of "commit at teardown" (the session stays addressable for inspection immediately after a + completed repair, per R7), but it changes observable timing for a scripted caller that previously saw + the healed sibling appear the instant the one-shot client process exited. + **Terminal lifecycle steps during a repair-armed `--from` resume.** Verification is scoped, per decision 3, to *annotated resolved targets* — so a non-target step (a source `close`, or any step carrying no `target-v1` `targetEvidence`) is already exempt from target-binding divergence: it may still surface an diff --git a/docs/adr/0016-active-session-script-publication.md b/docs/adr/0016-active-session-script-publication.md index fc14468fe3..6da9f14584 100644 --- a/docs/adr/0016-active-session-script-publication.md +++ b/docs/adr/0016-active-session-script-publication.md @@ -107,7 +107,16 @@ enough for this boundary. On consumption, a script without `close` preserves the existing replay behavior: the named session stays active and the successful `ReplayCommandResult` returns its `session` id. The caller binds subsequent commands to that returned id. Replay reports success only after the destination guard completes; the -absence of `close` changes neither action dispatch nor the success response shape. +absence of `close` changes neither action dispatch nor when replay reports success. + +**Response shape (issue #1384).** `ReplayCommandResult` carries a required `sessionActive: boolean`, +computed from whether `session` still exists in the daemon's own store when the response is built — never +by re-parsing the script for `close`. This is what lets the real CLI/IPC client (not just the in-process +handler) actually honor "the session stays active": without an explicit signal in the response, the +client's one-shot `replay`/`test` teardown had no way to distinguish a still-active handoff from a +finished one, and tore the owning daemon down regardless (`src/daemon/client/daemon-client-lifecycle.ts`). +`sessionActive` is `true` for every close-less run (including a `--from` resume) and `false` once the +script's terminal `close` — or a repair-armed run's deferred equivalent — has executed. ### Sensitive inputs @@ -169,6 +178,13 @@ executing that script, not the artifact being saved. fragment pinning remain entirely under #1336. - Secret-bearing authoring remains unsafe until #1348; the initial workflow is limited to journeys that do not enter secrets. Arbitrary history ranges remain out of scope. +- On consumption (issue #1384), a `replay` whose script has no terminal `close` leaves a live daemon and + app session behind — including the ordinary one-shot `replay` invocation's own owned/ephemeral daemon, + which the client keeps alive and reports a `--state-dir` address hint for instead of tearing down. An + unattended close-less replay (e.g. in CI) therefore leaks a session exactly like one opened + interactively: bounded by ordinary idle-reap or an explicit `close`, never by the one-shot command's own + process lifetime. This is the intended shape of "the caller owns close," not an accident, and recorded + test flows already end with `close` so `test` runs are unaffected. ## Alternatives Considered diff --git a/src/commands/replay/index.ts b/src/commands/replay/index.ts index 31a564c4e7..7177d9b507 100644 --- a/src/commands/replay/index.ts +++ b/src/commands/replay/index.ts @@ -93,7 +93,7 @@ export const testCommandDefinition = defineExecutableCommand(testCommandMetadata const replayCliSchema = { usageOverride: 'replay | replay export [--out ]', helpDescription: - 'Replay a recorded session. For Maestro YAML compatibility flows, use replay --maestro and keep the target binding such as --platform ios on the replay command.', + 'Replay a recorded session. For Maestro YAML compatibility flows, use replay --maestro and keep the target binding such as --platform ios on the replay command. A script with no terminal close leaves its session (and daemon) running until you close it or it idle-reaps — no different from a session opened interactively.', summary: replayCommandDescription, positionalArgs: ['path'], allowsExtraPositionals: true, diff --git a/src/contracts/replay.ts b/src/contracts/replay.ts index 4544cd1d3e..901eb4c42a 100644 --- a/src/contracts/replay.ts +++ b/src/contracts/replay.ts @@ -5,6 +5,14 @@ export type ReplayCommandResult = { replayed: number; healed: number; session: string; + /** + * True iff `session` still exists in the daemon's session store when the + * response is built — i.e. the replayed script had no terminal `close` + * (ADR 0016's consumption contract). The client uses this, not script + * parsing, to decide whether an owned one-shot daemon must stay alive so + * the caller can keep addressing this session. + */ + sessionActive: boolean; artifactPaths: string[]; warnings?: string[]; snapshotDiagnostics?: SnapshotDiagnosticsSummary; diff --git a/src/daemon/client/daemon-client-lifecycle.ts b/src/daemon/client/daemon-client-lifecycle.ts index 101014f9a0..07804b5faf 100644 --- a/src/daemon/client/daemon-client-lifecycle.ts +++ b/src/daemon/client/daemon-client-lifecycle.ts @@ -18,6 +18,7 @@ import { } from '../config.ts'; import { computeDaemonCodeSignature } from '../code-signature.ts'; import { PUBLIC_COMMANDS } from '../../command-catalog.ts'; +import { shellQuoteIfNeeded } from '../../utils/shell-quote.ts'; import { sleep } from '../../utils/timeouts.ts'; import { cleanupFailedDaemonStartupMetadata, @@ -342,7 +343,19 @@ export async function cleanupDaemonAfterRequest( // `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) + isHeldRepairDivergence(response) || + // ADR 0016: a `replay` whose script had no terminal `close` reports its + // session as still active by design (the consumption contract this ADR + // defines) — tearing down its owning daemon here would make that contract + // unaddressable over the real CLI path the instant the response is sent. + // Keyed off the session surviving the run (`sessionActive`), never off + // parsing the script for `close`, so a `--from` resume is covered too. + // Unlike the repair case, this session has no bounded reap of its own: an + // unattended close-less replay leaves a live daemon+app session until + // ordinary idle-reap or an explicit `close` ends it — the same lifetime an + // interactively opened session already has, and exactly what the ADR's + // "caller owns close" contract asks for. + isActiveReplaySessionResponse(req, response) ) { return response; } @@ -476,6 +489,72 @@ function isOneShotReplayCommand(command: string | undefined): boolean { return command === PUBLIC_COMMANDS.replay || command === PUBLIC_COMMANDS.test; } +/** + * ADR 0016: true when a successful `replay` response reports its session as + * still active (`ReplayCommandResult.sessionActive`, set by the daemon from + * whether the session survived in its own store — never derived here by + * re-parsing the script). Restricted to `replay` itself, never `test`: a + * `test` run's own per-file runner already closes each session before the + * suite summary is built, and its `ReplaySuiteResult` carries no such field + * anyway, but the explicit command check keeps that carve-out a decision + * rather than an accident of the response shape. + */ +export function isActiveReplaySessionResponse( + req: Omit, + response: DaemonResponse | undefined, +): boolean { + if (req.command !== PUBLIC_COMMANDS.replay) return false; + if (!response || !response.ok) return false; + return response.data?.sessionActive === true; +} + +/** + * ADR 0016 counterpart to `attachRepairSessionAddressHint`: a still-active + * replay session is only unaddressable by `--state-dir` when it lives on an + * OWNED, randomly generated one (`stateDir` undefined otherwise — an explicit + * `--state-dir`/`AGENT_DEVICE_STATE_DIR` caller already knows it). But the + * SESSION name is always cwd-qualified (`cwd::default`) and, per #1394, + * `session list` cannot rediscover it either — so `--session` is always + * emitted when a name is available, explicit state dir or not. Attached to + * both a structured `hint` field (for `--json` consumers) and appended to + * `message` — the only field the default text renderer surfaces + * (`src/utils/success-text.ts`) — so the hint reaches a caller in either mode. + * + * `data.session` is used verbatim, never reconstructed as `default`: an + * EXPLICIT `--session ` is used as-is by `resolveEffectiveSessionName`, + * skipping cwd-scoping entirely (`hasExplicitSessionFlag`), so passing the + * qualified name back unchanged is what actually reaches the same session + * from any cwd — a bare `--session default` would only match by coincidence + * (an implicit, no-`--session` follow-up run from the identical cwd). Both + * the state dir and the session name are shell-quoted (only when needed) so + * the hint stays literally copy-pasteable even if either contains spaces or + * shell metacharacters. + */ +export function attachActiveSessionAddressHint( + response: Extract, + stateDir: string | undefined, +): Extract { + const data = response.data ?? {}; + const sessionName = typeof data.session === 'string' ? data.session : undefined; + const addressFlags = [ + ...(stateDir ? [`--state-dir ${shellQuoteIfNeeded(stateDir)}`] : []), + ...(sessionName ? [`--session ${shellQuoteIfNeeded(sessionName)}`] : []), + ]; + if (addressFlags.length === 0) return response; + const addressHint = + `This session's daemon was kept alive because its script left the session active; ` + + `pass ${addressFlags.join(' ')} on your next command to reach it.`; + const existingMessage = typeof data.message === 'string' ? data.message : undefined; + return { + ...response, + data: { + ...data, + hint: addressHint, + message: existingMessage ? `${existingMessage} ${addressHint}` : addressHint, + }, + }; +} + async function waitForDaemonStartup( timeoutMs: number, settings: DaemonClientSettings, diff --git a/src/daemon/client/daemon-client.ts b/src/daemon/client/daemon-client.ts index 9d3277992e..fb46efd7b0 100644 --- a/src/daemon/client/daemon-client.ts +++ b/src/daemon/client/daemon-client.ts @@ -8,9 +8,11 @@ import { createRequestId, emitDiagnostic, withDiagnosticTimer } from '../../util import { INTERNAL_COMMANDS, PUBLIC_COMMANDS } from '../../command-catalog.ts'; import { prepareRemoteRequestArtifacts } from '../../remote/daemon-artifacts.ts'; import { + attachActiveSessionAddressHint, attachRepairSessionAddressHint, cleanupDaemonAfterRequest, ensureDaemon, + isActiveReplaySessionResponse, isHeldRepairDivergence, resolveClientSettings, type DaemonClientSettings, @@ -94,7 +96,11 @@ export async function sendToDaemon( ), { requestId, command: req.command }, ); - return withRepairSessionAddressHintIfOwned(response, settings); + return withActiveSessionAddressHint( + withRepairSessionAddressHintIfOwned(response, settings), + req, + settings, + ); }); } @@ -153,6 +159,32 @@ function withRepairSessionAddressHintIfOwned( return attachRepairSessionAddressHint(response, settings.paths.baseDir); } +/** + * ADR 0016 counterpart to `withRepairSessionAddressHintIfOwned` — but unlike + * that one, NOT gated on `settings.ownedStateDir`. An owned ephemeral state + * dir is unaddressable by a later invocation either way, so it's included + * when owned; an explicit `--state-dir`/`AGENT_DEVICE_STATE_DIR` caller + * already knows their own dir, so it's omitted then. But the session's own + * name is cwd-qualified and, per #1394, `session list` cannot rediscover it + * either — so `--session` is still worth hinting even at an explicit state + * dir, which is why this runs for every active-session response regardless + * of `ownedStateDir` (`attachActiveSessionAddressHint` itself decides what, + * if anything, is worth attaching). + */ +function withActiveSessionAddressHint( + response: DaemonResponse, + req: Omit, + settings: DaemonClientSettings, +): DaemonResponse { + if (!response.ok || !isActiveReplaySessionResponse(req, response)) { + return response; + } + return attachActiveSessionAddressHint( + response, + settings.ownedStateDir ? settings.paths.baseDir : undefined, + ); +} + 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-runtime.test.ts b/src/daemon/handlers/__tests__/session-replay-runtime.test.ts index 74c3cbaaa6..b404c21e11 100644 --- a/src/daemon/handlers/__tests__/session-replay-runtime.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-runtime.test.ts @@ -45,6 +45,61 @@ test('a successful replay prints one line with the step count and wall time', as expect(data.message).toMatch(/^Replayed 2 steps in \d+\.\ds$/); }); +// --- ADR 0016 / issue #1384: `sessionActive` is derived from the REAL +// producer (`completeReplayRun`'s `sessionStore.get(sessionName)` check), not +// asserted against a hand-crafted fixture — deleting that line would fail +// these, unlike the client-lifecycle tests in +// `src/utils/__tests__/daemon-client-lifecycle.test.ts`, which only prove the +// CLIENT'S reaction to a `sessionActive` value it is handed. --- + +test('a close-less replay reports sessionActive: true (real producer, session still in the store)', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-session-active-')); + const sessionStore = new SessionStore(path.join(root, 'sessions')); + const sessionName = 'default'; + sessionStore.set(sessionName, makeIosSession(sessionName)); + const filePath = writeReplayFile(root, ['open "Demo"', 'click "Save"']); + + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async () => ({ ok: true, data: {} }), + }); + + expect(response.ok).toBe(true); + if (!response.ok) return; + expect(sessionStore.get(sessionName)).toBeDefined(); + expect((response.data as { sessionActive: boolean }).sessionActive).toBe(true); +}); + +test('a replay whose terminal close removes the session reports sessionActive: false', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-session-closed-')); + const sessionStore = new SessionStore(path.join(root, 'sessions')); + const sessionName = 'default'; + sessionStore.set(sessionName, makeIosSession(sessionName)); + const filePath = writeReplayFile(root, ['open "Demo"', 'close']); + + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + // Mirrors the one real effect of `handleCloseCommand` this test needs: + // removing the session from the store. Every other command is a no-op, + // same as the rest of this file's `invoke` stubs. + invoke: async (req) => { + if (req.command === 'close') sessionStore.delete(sessionName); + return { ok: true, data: {} }; + }, + }); + + expect(response.ok).toBe(true); + if (!response.ok) return; + expect(sessionStore.get(sessionName)).toBeUndefined(); + expect((response.data as { sessionActive: boolean }).sessionActive).toBe(false); +}); + test('Maestro YAML uses the typed engine while .ad remains generic', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-typed-maestro-route-')); const sessionStore = new SessionStore(path.join(root, 'sessions')); @@ -94,6 +149,31 @@ test('Maestro YAML uses the typed engine while .ad remains generic', async () => expect(commands).toEqual(['open']); }); +test('ADR 0016 / #1384: a Maestro replay reports sessionActive: true (real producer, no fake HTTP)', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-maestro-session-active-')); + const sessionStore = new SessionStore(path.join(root, 'sessions')); + const sessionName = 'default'; + sessionStore.set(sessionName, makeIosSession(sessionName)); + const yamlPath = path.join(root, 'flow.yaml'); + fs.writeFileSync(yamlPath, 'appId: com.example.app\n---\n- launchApp\n'); + + const response = await runReplayScriptFile({ + req: baseReq({ + positionals: [yamlPath], + flags: { replayBackend: 'maestro', platform: 'ios' }, + }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async () => ({ ok: true, data: {} }), + }); + + expect(response.ok).toBe(true); + if (!response.ok) return; + expect(sessionStore.get(sessionName)).toBeDefined(); + expect((response.data as { sessionActive: boolean }).sessionActive).toBe(true); +}); + test('typed Maestro nested commands receive the runtime hints bound into the plan', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-maestro-runtime-envelope-')); const sessionStore = new SessionStore(path.join(root, 'sessions')); diff --git a/src/daemon/handlers/session-replay-maestro-response.ts b/src/daemon/handlers/session-replay-maestro-response.ts index a4c35f40a6..5eeb49aa19 100644 --- a/src/daemon/handlers/session-replay-maestro-response.ts +++ b/src/daemon/handlers/session-replay-maestro-response.ts @@ -28,6 +28,7 @@ export function buildTypedMaestroSuccessResponse(params: { replayed, healed: 0, session: sessionName, + sessionActive: sessionStore.get(sessionName) !== undefined, artifactPaths: result.artifactPaths, ...(result.warnings ? { warnings: result.warnings } : {}), ...(snapshotDiagnostics ? { snapshotDiagnostics } : {}), diff --git a/src/daemon/handlers/session-replay-runtime.ts b/src/daemon/handlers/session-replay-runtime.ts index 62e5576da5..c7403002c7 100644 --- a/src/daemon/handlers/session-replay-runtime.ts +++ b/src/daemon/handlers/session-replay-runtime.ts @@ -383,6 +383,7 @@ function completeReplayRun(params: { replayed: replayedCount, healed: 0, session: sessionName, + sessionActive: completedSession !== undefined, artifactPaths: [...artifactPaths], ...(snapshotDiagnosticsSummary ? { snapshotDiagnostics: snapshotDiagnosticsSummary } : {}), message: formatReplaySuccessMessage(replayedCount, Date.now() - startedAt), diff --git a/src/mcp/command-output-schemas.ts b/src/mcp/command-output-schemas.ts index a46b085149..802c343ccc 100644 --- a/src/mcp/command-output-schemas.ts +++ b/src/mcp/command-output-schemas.ts @@ -607,11 +607,14 @@ export const COMMAND_OUTPUT_SCHEMAS = { replayed: numberSchema(), healed: numberSchema(), session: stringSchema(), + sessionActive: booleanSchema( + 'True iff the session is still active — the script had no terminal close.', + ), artifactPaths: stringArraySchema, snapshotDiagnostics: looseObjectSchema(), message: stringSchema(), }, - ['replayed', 'healed', 'session', 'artifactPaths', 'message'], + ['replayed', 'healed', 'session', 'sessionActive', 'artifactPaths', 'message'], ), test: objectSchema( { diff --git a/src/utils/__tests__/daemon-client-lifecycle.test.ts b/src/utils/__tests__/daemon-client-lifecycle.test.ts index c44d041620..ed214d9fa7 100644 --- a/src/utils/__tests__/daemon-client-lifecycle.test.ts +++ b/src/utils/__tests__/daemon-client-lifecycle.test.ts @@ -23,7 +23,12 @@ vi.mock('../timeouts.ts', async (importOriginal) => { }); import { resolveDaemonPaths, type DaemonPaths } from '../../daemon/config.ts'; -import { sendToDaemon, type DaemonRequest } from '../../daemon/client/daemon-client.ts'; +import { + sendToDaemon, + type DaemonRequest, + type DaemonResponse, +} from '../../daemon/client/daemon-client.ts'; +import { attachActiveSessionAddressHint } from '../../daemon/client/daemon-client-lifecycle.ts'; import { computeDaemonCodeSignature } from '../../daemon/code-signature.ts'; import { sendRequest } from '../../daemon/client/daemon-client-transport.ts'; import { @@ -34,6 +39,7 @@ import { import { AppError } from '../../kernel/errors.ts'; import { runCmdDetachedMonitored, runCmdSync } from '../exec.ts'; import { readProcessStartTime } from '../host-process.ts'; +import { shellQuoteIfNeeded } from '../shell-quote.ts'; import { sleep } from '../timeouts.ts'; import { findProjectRoot, readVersion } from '../version.ts'; @@ -1081,3 +1087,328 @@ test('continuation: sendToDaemon keeps the daemon alive on a held divergence eve if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); } }); + +// --- ADR 0016: on consumption, a `replay` whose script had no terminal +// `close` reports its session as still active (`ReplayCommandResult. +// sessionActive: true`) by design — the named session stays active and the +// caller binds subsequent commands to the returned id. Tearing down the +// owning daemon here (issue #1384) makes that contract unaddressable the +// instant the response is sent. This mirrors the repair-divergence keep-alive +// above, but for a SUCCESSFUL response instead of a held divergence, and has +// no bounded reap of its own (unlike a held repair): an unattended close-less +// replay leaves a live daemon+app session until ordinary idle-reap or an +// explicit `close` ends it — the same lifetime an interactively opened +// session already has. --- + +function activeReplaySuccessData(overrides: Record = {}): Record { + return { + replayed: 1, + healed: 0, + session: 'default', + sessionActive: true, + artifactPaths: [], + message: 'Replayed 1 step in 0.1s', + ...overrides, + }; +} + +test('attachActiveSessionAddressHint shell-quotes a --state-dir/--session value containing spaces or shell metacharacters', () => { + const unsafeStateDir = '/tmp/state dir with $(danger)'; + const unsafeSession = 'cwd:abc123:my session; rm -rf /'; + const response: Extract = { + ok: true, + data: activeReplaySuccessData({ session: unsafeSession }), + }; + + const hinted = attachActiveSessionAddressHint(response, unsafeStateDir); + + assert.equal( + hinted.data?.hint, + "This session's daemon was kept alive because its script left the session active; " + + `pass --state-dir ${shellQuoteIfNeeded(unsafeStateDir)} ` + + `--session ${shellQuoteIfNeeded(unsafeSession)} on your next command to reach it.`, + ); + // Both values actually needed quoting — this test would pass vacuously + // (raw interpolation indistinguishable from quoted) if they didn't. + assert.notEqual(shellQuoteIfNeeded(unsafeStateDir), unsafeStateDir); + assert.notEqual(shellQuoteIfNeeded(unsafeSession), unsafeSession); +}); + +test('attachActiveSessionAddressHint omits --state-dir but still quotes an unsafe --session-only value', () => { + const unsafeSession = "cwd:abc123:it's mine"; + const response: Extract = { + ok: true, + data: activeReplaySuccessData({ session: unsafeSession }), + }; + + const hinted = attachActiveSessionAddressHint(response, undefined); + + assert.equal( + hinted.data?.hint, + "This session's daemon was kept alive because its script left the session active; " + + `pass --session ${shellQuoteIfNeeded(unsafeSession)} on your next command to reach it.`, + ); + assert.doesNotMatch(String(hinted.data?.hint), /--state-dir/); +}); + +/** Issues a close-less `replay` against an owned ephemeral daemon spawned at `daemonPort`. */ +async function replayLeavingSessionActive( + daemonPort: number, + requestId: string, +): Promise<{ response: Awaited>; ownedStateDir: string }> { + let ownedStateDir = ''; + installSpawnedHttpDaemonAtOwnedStateDir(daemonPort, (dir) => { + ownedStateDir = dir; + }); + const response = await sendToDaemon({ + session: 'default', + command: 'replay', + positionals: ['open-only.ad'], + flags: { daemonTransport: 'http' }, + meta: { requestId }, + }); + return { response, ownedStateDir }; +} + +/** Parses a `--state-dir --session ` pair out of an address hint. */ +function parseAddressHint(hint: string): { stateDir: string; session: string } { + const match = hint.match(/--state-dir (\S+) --session (\S+)/); + assert.ok(match, `hint did not contain a parseable --state-dir/--session pair: ${hint}`); + return { stateDir: match[1] ?? '', session: match[2] ?? '' }; +} + +test('sendToDaemon keeps an owned ephemeral daemon alive and hints its --state-dir when a close-less replay leaves the session active', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + + const daemon = await startHttpDaemonFixture(activeReplaySuccessData()); + let ownedStateDir = ''; + try { + const result = await replayLeavingSessionActive(daemon.port, 'req-active-session-keep-alive'); + ownedStateDir = result.ownedStateDir; + assert.equal(result.response.ok, true); + if (!result.response.ok) return; + const data = result.response.data; + assert.ok(data); + assert.equal(data.session, 'default'); + assert.equal(data.sessionActive, true); + assert.ok(ownedStateDir.length > 0); + const hint = String(data.hint); + assert.match(hint, /--state-dir/); + assert.ok(hint.includes(ownedStateDir)); + // Names the session verbatim (not just --state-dir): an explicit + // --session is used as-is by resolveEffectiveSessionName, so the + // hint must be copy-pasteable into a follow-up command from any cwd. + assert.match(hint, /--session default/); + assert.match(String(data.message), /--state-dir/); + + // The daemon was NOT torn down: metadata and the owned state dir itself + // are still on disk, addressable by a follow-up command's --state-dir. + const ownedPaths = resolveDaemonPaths(ownedStateDir); + assert.equal(fs.existsSync(ownedPaths.infoPath), true); + assert.equal(fs.existsSync(ownedPaths.lockPath), true); + assert.equal(fs.existsSync(ownedStateDir), true); + } finally { + await closeLoopbackServer(daemon.server); + if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + } +}); + +test('closes the loop: a follow-up sendToDaemon using the hinted --state-dir/--session reaches the SAME kept-alive daemon, without spawning a new one', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + + const daemon = await startHttpDaemonFixture(activeReplaySuccessData()); + let ownedStateDir = ''; + try { + const result = await replayLeavingSessionActive(daemon.port, 'req-active-session-hint-source'); + ownedStateDir = result.ownedStateDir; + assert.equal(result.response.ok, true); + if (!result.response.ok) return; + const hinted = parseAddressHint(String(result.response.data?.hint)); + assert.equal(hinted.stateDir, ownedStateDir); + + const spawnCallsBeforeFollowUp = mockRunCmdDetached.mock.calls.length; + const followUp = await sendToDaemon({ + session: hinted.session, + command: 'press', + positionals: [], + flags: { stateDir: hinted.stateDir, daemonTransport: 'http' }, + meta: { requestId: 'req-active-session-followup', sessionExplicit: true }, + }); + + // Reached the SAME kept-alive daemon fixture — no new one was spawned to + // serve the follow-up — and the request actually carried the hinted + // session name, closing the loop on the promise the hint makes. + assert.equal(mockRunCmdDetached.mock.calls.length, spawnCallsBeforeFollowUp); + assert.equal(followUp.ok, true); + assert.equal(daemon.rpcRequests.length, 2); + assert.equal(daemon.rpcRequests[1]?.params?.session, hinted.session); + assert.equal(daemon.rpcRequests[1]?.params?.command, 'press'); + } finally { + await closeLoopbackServer(daemon.server); + if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + } +}); + +test('sendToDaemon tears down an owned ephemeral daemon when replay reports the session was closed (sessionActive: false)', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + + const daemon = await startHttpDaemonFixture(activeReplaySuccessData({ sessionActive: false })); + let ownedStateDir = ''; + installSpawnedHttpDaemonAtOwnedStateDir(daemon.port, (dir) => { + ownedStateDir = dir; + }); + + try { + const response = await sendToDaemon({ + session: 'default', + command: 'replay', + positionals: ['closed.ad'], + flags: { daemonTransport: 'http' }, + meta: { requestId: 'req-closed-session-teardown' }, + }); + + assert.equal(response.ok, true); + if (!response.ok) return; + assert.equal(response.data?.hint, undefined); + assert.ok(ownedStateDir.length > 0); + assert.equal(fs.existsSync(ownedStateDir), false); + } finally { + await closeLoopbackServer(daemon.server); + if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + } +}); + +test('ADR 0012 R7 x ADR 0016: a completed --save-script repair also keeps its owning daemon alive (its terminal source close is always skipped)', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + + // A repair-armed replay that completes with NO divergence always skips its + // terminal source `close` (ADR 0012, "the terminal source close ... is + // SKIPPED — not dispatched"), so the session survives every such run and + // `sessionActive` is always true here — this falls out of the SAME guard as + // the plain #1384 case above, but is pinned separately because it changes + // when the healed `.ad` commit (gated on teardown, not on this response) + // actually lands: previously immediate (the one-shot client tore the daemon + // down right after this response), now deferred to an explicit close or + // idle-reap. + const daemon = await startHttpDaemonFixture(activeReplaySuccessData({ healed: 1 })); + let ownedStateDir = ''; + installSpawnedHttpDaemonAtOwnedStateDir(daemon.port, (dir) => { + ownedStateDir = dir; + }); + + try { + const response = await sendToDaemon({ + session: 'default', + command: 'replay', + positionals: ['flow.ad'], + flags: { saveScript: true, daemonTransport: 'http' }, + meta: { requestId: 'req-repair-complete-keep-alive' }, + }); + + assert.equal(response.ok, true); + if (!response.ok) return; + assert.equal(response.data?.sessionActive, true); + assert.match(String(response.data?.hint), /--state-dir/); + + // No immediate teardown means no immediate commit trigger either: the + // owned daemon and its state dir are still on disk, exactly like the + // plain (non-repair) active-session case. + assert.ok(ownedStateDir.length > 0); + assert.equal(fs.existsSync(ownedStateDir), true); + } finally { + await closeLoopbackServer(daemon.server); + if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + } +}); + +test('issue #1384: sendToDaemon does not stop a client-started daemon at an explicit --state-dir when replay leaves the session active', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + + // The literal #1384 repro: an explicit AGENT_DEVICE_STATE_DIR/--state-dir + // means `ownedStateDir` is false, but this client still STARTS the daemon + // fresh at that fixed dir (`daemon.startedByClient`) since nothing was + // running there yet — the teardown branch this guards is reached + // regardless of `ownedStateDir`. + const stateDir = makeTempStateDir('agent-device-active-session-explicit-dir-'); + const paths = resolveDaemonPaths(stateDir); + const daemon = await startHttpDaemonFixture(activeReplaySuccessData()); + installSpawnedHttpDaemon(paths, daemon.port); + + try { + const response = await sendToDaemon({ + session: 'default', + command: 'replay', + positionals: ['open-only.ad'], + flags: { stateDir, daemonTransport: 'http' }, + meta: { requestId: 'req-explicit-dir-active-session' }, + }); + + assert.equal(response.ok, true); + if (!response.ok) return; + assert.equal(response.data?.sessionActive, true); + // The hint still names --session (the caller can't rediscover a + // cwd-qualified session name any other way, per #1394) but omits + // --state-dir — the caller already knows this explicit dir, it passed it + // itself, so only an OWNED (randomly generated) state dir needs one. + assert.equal( + response.data?.hint, + "This session's daemon was kept alive because its script left the session active; " + + 'pass --session default on your next command to reach it.', + ); + assert.doesNotMatch(String(response.data?.hint), /--state-dir/); + assert.equal(fs.existsSync(paths.infoPath), true); + assert.equal(fs.existsSync(paths.lockPath), true); + } finally { + await closeLoopbackServer(daemon.server); + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +test('sendToDaemon still tears down a `test` command owned ephemeral daemon even if its response data carried sessionActive:true', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + + // `test` never actually sets this field in practice (its own per-file + // runner always closes each session before the suite summary is built) — + // this proves the carve-out is an explicit command check, not an accident + // of the response shape, should a future response ever carry it by mistake. + const daemon = await startHttpDaemonFixture(activeReplaySuccessData({ total: 1 })); + let ownedStateDir = ''; + installSpawnedHttpDaemonAtOwnedStateDir(daemon.port, (dir) => { + ownedStateDir = dir; + }); + + try { + const response = await sendToDaemon({ + session: 'default', + command: 'test', + positionals: ['suite.ad'], + flags: { daemonTransport: 'http' }, + meta: { requestId: 'req-test-command-teardown' }, + }); + + assert.equal(response.ok, true); + assert.ok(ownedStateDir.length > 0); + assert.equal(fs.existsSync(ownedStateDir), false); + } finally { + await closeLoopbackServer(daemon.server); + if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + } +}); diff --git a/test/integration/provider-scenarios/active-session-script-publication.test.ts b/test/integration/provider-scenarios/active-session-script-publication.test.ts index ee5a68c7ef..0720bb5ff7 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -34,7 +34,15 @@ test('provider route publishes and replays an open-to-destination script with a await client.sessions.close(); const replay = await world.daemon.callCommand('replay', [scriptPath], world.selection); - assert.equal(assertRpcOk<{ session?: string }>(replay).session, 'default'); + const replayData = assertRpcOk<{ session?: string; sessionActive?: boolean }>(replay); + assert.equal(replayData.session, 'default'); + // ADR 0016 / #1384: the published script has no terminal `close`, so the + // real (not test-fixture-injected) daemon response reports the session + // still active — this is the exact producer line the client-lifecycle + // tests in daemon-client-lifecycle.test.ts assume but cannot themselves + // exercise, since they inject sessionActive via a fake HTTP fixture. + assert.equal(replayData.sessionActive, true); + assert.ok(world.daemon.session(), 'a close-less replay must not tear down the session'); const replaySnapshot = await client.capture.snapshot({ interactiveOnly: true }); assert.ok(replaySnapshot.nodes.some((node) => node.label === 'Search')); await client.sessions.close();