Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions docs/adr/0016-active-session-script-publication.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,20 @@ work, so the caller can retry with plain `close`. Plain `close` tears down ABORT
writing; closing an unpublished ARMED recording retains the existing close-time publication behavior. A
fresh session is the only re-arming boundary.

> **Amendment (2026-08-02, shipped).** A session that was never armed — no recorded
> `open --save-script` at all — has no fourth pre-ARMED state name here, but it previously fell
> through the same close-time write path as an ARMED recording: `close --save-script` on it folded
> the request into the authoring lifecycle at record time and published anyway. Live evidence
> showed this produces a script whose actions carry selector fallback chains but no recording-time
> `target-v1` evidence, with no signal to the caller that evidence capture never ran — degraded
> replay verification is worse than a loud refusal. `close --save-script` on a never-armed session
> is now rejected before any teardown or filesystem work, the same way ABORTED/PUBLISHED are,
> naming `open --save-script` as the recovery; a plain `close` still tears the session down
> without writing. This is distinct from
> [#1533](https://github.com/callstack/agent-device/issues/1533), which is about an
> already-ARMED-then-ABORTED session whose flag ingress re-enables `recordSession` and lets a
> *bare* `close` (no `--save-script` on the close itself) publish; that case is unresolved here.

This lifecycle is distinct from ADR 0012's repair transaction. `session save-script` rejects a session
with `saveScriptBoundary` set and directs the caller to finish or abort the repair through its existing
`replay --from` and teardown commit protocol. Active-session publication never marks a repair COMPLETE,
Expand Down Expand Up @@ -264,6 +278,9 @@ executing that script, not the artifact being saved.
- In ABORTED/PUBLISHED, `close --save-script[=<other>]` is rejected before platform close and plain
`close` tears down without writing; closing an unpublished ARMED recording preserves current
close-time publication behavior.
- On a never-armed session (2026-08-02 amendment), `close --save-script` is likewise rejected before
platform close or filesystem work, naming `open --save-script` as the recovery; plain `close` still
tears down without writing, and the session is not deleted by the rejected request.
- Descriptor completeness tests classify every recordable request's mutation effect, including
request-sensitive read-only/mutating subcommands, and destination-guard ordering consumes only that
trait.
Expand Down
4 changes: 4 additions & 0 deletions src/cli-schema/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ export type CommandSchema = {
defaults?: Partial<CliFlags>;
usageOverride?: string;
listUsageOverride?: string;
// Swaps a shared flag's usageDescription for this command only, when the flag's generic
// documentation (flag-definitions-*.ts) does not fit every command it is allowed on — for
// example `--save-script` arms authoring on open/close but a repair transaction on replay.
flagDescriptionOverrides?: Partial<Record<FlagKey, string>>;
};

export type CommandSchemaOverride = Partial<CommandSchema>;
32 changes: 32 additions & 0 deletions src/cli/parser/__tests__/cli-help-command-usage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,3 +407,35 @@ test('removed trigger aliases are no longer documented as commands', async () =>
const help = await usageForCommand('trigger-screenshot-notification');
assert.equal(help, null);
});

// #1558 follow-up: replay's --save-script arms an ADR 0012 repair transaction, not the
// open/close authoring lifecycle (arm-on-open, publish-on-close) the shared flag describes.
// `flagDescriptionOverrides` (CommandSchema) swaps the text for replay only; open/close keep the
// shared FlagDefinition description unchanged.
test('replay --save-script help describes arming a repair transaction, not open/close authoring', async () => {
const help = await usageForCommand('replay');
if (help === null) throw new Error('Expected replay help text');
assert.match(help, /Arm a repair transaction from this replay \(ADR 0012\)/);
assert.match(help, /commits when the repair-armed session tears down/);
assert.doesNotMatch(help, /Arm evidence capture on open, publish the armed recording on close/);
});

test('open --save-script help keeps the shared open\\/close authoring description', async () => {
const help = await usageForCommand('open');
if (help === null) throw new Error('Expected open help text');
assert.match(
help,
/Arm evidence capture on open, publish the armed recording on close; close --save-script alone \(without an armed open\) is rejected/,
);
assert.doesNotMatch(help, /Arm a repair transaction from this replay/);
});

test('close --save-script help keeps the shared open\\/close authoring description', async () => {
const help = await usageForCommand('close');
if (help === null) throw new Error('Expected close help text');
assert.match(
help,
/Arm evidence capture on open, publish the armed recording on close; close --save-script alone \(without an armed open\) is rejected/,
);
assert.doesNotMatch(help, /Arm a repair transaction from this replay/);
});
20 changes: 19 additions & 1 deletion src/cli/parser/cli-help.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1243,6 +1243,21 @@ function listHelpFlags(keys: ReadonlySet<FlagKey>): FlagDefinition[] {
);
}

// Command-specific override for a shared flag's help text (see CommandSchema.flagDescriptionOverrides):
// keeps the FlagDefinition registry as one shared row per flag while letting a command whose
// semantics genuinely differ (e.g. `replay --save-script` arms a repair transaction, not the
// open/close authoring lifecycle) show its own description without duplicating the flag entry.
function applyFlagDescriptionOverrides(
definitions: FlagDefinition[],
overrides: Partial<Record<FlagKey, string>> | undefined,
): FlagDefinition[] {
if (!overrides) return definitions;
return definitions.map((definition) => {
const override = overrides[definition.key];
return override === undefined ? definition : { ...definition, usageDescription: override };
});
}

function renderFlagSection(title: string, definitions: FlagDefinition[]): string {
return renderAlignedSection(
title,
Expand Down Expand Up @@ -1302,7 +1317,10 @@ export function buildCommandUsageText(commandName: string): string | null {
const schema = getCommandSchema(commandName);
if (!schema) return null;
const usage = buildCommandUsage(commandName, schema);
const commandFlags = listHelpFlags(new Set<FlagKey>(schema.allowedFlags ?? []));
const commandFlags = applyFlagDescriptionOverrides(
listHelpFlags(new Set<FlagKey>(schema.allowedFlags ?? [])),
schema.flagDescriptionOverrides,
);
const sections: string[] = [];
if (commandFlags.length > 0) {
sections.push(renderFlagSection('Command flags:', commandFlags));
Expand Down
3 changes: 2 additions & 1 deletion src/commands/cli-grammar/flag-definitions-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ export const ACTION_FLAG_DEFINITIONS: readonly FlagDefinition[] = [
names: ['--save-script'],
type: 'booleanOrString',
usageLabel: '--save-script [path]',
usageDescription: 'Save session script (.ad) on close; optional custom output path',
usageDescription:
'Arm evidence capture on open, publish the armed recording on close; close --save-script alone (without an armed open) is rejected — start with open --save-script, or use session save-script mid-session. Optional custom output path.',
},
{
key: 'networkInclude',
Expand Down
12 changes: 8 additions & 4 deletions src/commands/command-explain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,22 +280,26 @@ function describeCliSurface(command: string, schema: CommandSchema): CommandExpl
return {
usage: buildCommandUsage(command, schema),
positionalArgs: schema.positionalArgs ?? [],
commandFlags: describeFlags(schema.allowedFlags ?? []),
supportedFlags: describeFlags(schema.supportedFlags ?? []),
commandFlags: describeFlags(schema.allowedFlags ?? [], schema.flagDescriptionOverrides),
supportedFlags: describeFlags(schema.supportedFlags ?? [], schema.flagDescriptionOverrides),
globalFlags: describeFlags([...GLOBAL_FLAG_KEYS]),
};
}

function describeFlags(keys: readonly FlagKey[]): CommandFlagExplanation[] {
function describeFlags(
keys: readonly FlagKey[],
overrides?: Partial<Record<FlagKey, string>>,
): CommandFlagExplanation[] {
return [...new Set(keys)].sort().map((key) => {
const definitions = flagDefinitionsByKey.get(key) ?? [];
const preferred = definitions.find((definition) => definition.usageLabel) ?? definitions[0];
const description = overrides?.[key] ?? preferred?.usageDescription;
return {
key,
syntax:
preferred?.usageLabel ??
[...new Set(definitions.flatMap((definition) => definition.names))].join('/'),
...(preferred?.usageDescription ? { description: preferred.usageDescription } : {}),
...(description ? { description } : {}),
};
});
}
Expand Down
7 changes: 7 additions & 0 deletions src/commands/replay/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@ const replayCliSchema = {
'saveScript',
'force',
],
// ADR 0012 decision 6: on replay, --save-script arms a repair transaction from step 1 (not the
// open/close authoring lifecycle the shared flag description documents) and the healed script
// commits on that transaction's own teardown, not on a plain close.
flagDescriptionOverrides: {
saveScript:
'Arm a repair transaction from this replay (ADR 0012): recording starts at step 1, and the healed script commits when the repair-armed session tears down (close, close --save-script, or idle-reap). Independent of the open/close authoring arm-on-open. Optional custom output path.',
},
} as const satisfies CommandSchemaOverride;

const testCliSchema = {
Expand Down
120 changes: 119 additions & 1 deletion src/daemon/handlers/__tests__/session-close-shutdown.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,11 @@ test('targeted close skips platform dispatch and preserves the error when the re
reason: 'runner_stop_failed',
hint: 'Retry once the runner is reachable.',
});
mockStopIosRunnerSession.mockRejectedValue(preCloseError);
// Scoped to this test's own two internal calls (pre-close stop, then the later
// independent-cleanup retry) — a persistent `mockRejectedValue` here would leak into every
// later test that exercises an Apple-platform runner stop, since `vi.clearAllMocks()` in
// `beforeEach` clears call history but not a mock's implementation.
mockStopIosRunnerSession.mockRejectedValueOnce(preCloseError);

await expect(
handleSessionCommands({
Expand Down Expand Up @@ -1131,3 +1135,117 @@ test('targeted close skips platform dispatch and preserves the error when the re
expect(mockStopIosRunnerSession.mock.calls.length).toBeGreaterThan(1);
expect(sessionStore.get(sessionName)).toBeUndefined();
});

// Live evidence (2026-08-02): a plain `open` followed by `close --save-script` used to fold the
// never-armed session into the authoring lifecycle and publish anyway, producing a script with
// selector fallback chains but no recording-time `target-v1` evidence. These two tests prove the
// daemon-seam fix: the rejection fires before ANY teardown work (no dispatch mock needed — a
// no-target close on Android never reaches `dispatchCommand`), the session survives so the agent
// can retry, and no script file is written.
test('close --save-script on a never-armed session is rejected before teardown, with no script written', async () => {
const sessionStore = makeSessionStore();
const sessionName = 'ios-unarmed-close-save-script-session';
const scriptPath = path.join(os.tmpdir(), `agent-device-unarmed-close-${Date.now()}.ad`);
// The fixture must carry real cleanup-bearing state (here: an active recording, like
// `makeIosSimulatorRecordingSession`'s other consumers) so this test can actually prove the
// guard runs *before* `stopBestEffortSessionResources` — not just that the response rejects.
// Without it, moving the guard after teardown would still pass: there would be nothing for
// teardown to observably touch.
const session = makeIosSimulatorRecordingSession(sessionName);
const kill = recordingKillMock(session);
sessionStore.set(sessionName, session);

await expect(
handleSessionCommands({
req: {
token: 't',
session: sessionName,
command: 'close',
positionals: [],
flags: { saveScript: scriptPath },
},
sessionName,
logPath: path.join(os.tmpdir(), 'daemon.log'),
sessionStore,
invoke: noopInvoke,
}),
).rejects.toMatchObject({
code: 'INVALID_ARGS',
message: expect.stringMatching(/not armed/),
details: expect.objectContaining({
hint: expect.stringMatching(/open <app> --save-script/),
}),
});

// The rejection does not tear down the session — it stays retryable/recoverable.
expect(sessionStore.get(sessionName)).toBeDefined();
expect(fs.existsSync(scriptPath)).toBe(false);
// No teardown hook ran: the recording is still live (recorder never signaled) and the runner
// was never told to stop. This is the assertion that goes red if the guard moves after
// `stopBestEffortSessionResources` — see the counterfactual in the PR description.
expect(kill).not.toHaveBeenCalled();
expect(session.recording).toBeDefined();
expect(mockStopIosRunnerSession).not.toHaveBeenCalled();

// A plain close (no --save-script) still closes the same session cleanly afterward, and now
// teardown genuinely does run: the recorder is signaled and the session deleted.
const plainClose = await handleSessionCommands({
req: {
token: 't',
session: sessionName,
command: 'close',
positionals: [],
flags: {},
},
sessionName,
logPath: path.join(os.tmpdir(), 'daemon.log'),
sessionStore,
invoke: noopInvoke,
});
expect(plainClose?.ok).toBe(true);
expect(kill).toHaveBeenCalledWith('SIGINT');
expect(session.recording).toBeUndefined();
expect(mockStopIosRunnerSession).toHaveBeenCalledWith(session.device.id);
expect(sessionStore.get(sessionName)).toBeUndefined();
});

test('close --save-script on a session with an active .ad repair transaction is unaffected by the unarmed-authoring guard', async () => {
const sessionStore = makeSessionStore();
const sessionName = 'android-repair-close-save-script-session';
const session = {
...makeSession(sessionName, {
platform: 'android',
id: 'emulator-5554',
name: 'Pixel_9_API_35',
kind: 'emulator',
booted: true,
}),
recordSession: true,
scriptPublication: {
kind: 'repair' as const,
status: 'complete' as const,
target: { kind: 'default' as const, force: false },
boundary: 0,
},
};
sessionStore.set(sessionName, session);

const response = await handleSessionCommands({
req: {
token: 't',
session: sessionName,
command: 'close',
positionals: [],
flags: { saveScript: true },
},
sessionName,
logPath: path.join(os.tmpdir(), 'daemon.log'),
sessionStore,
invoke: noopInvoke,
});

// Repair transactions are a disjoint lifecycle (ADR 0012) with their own arming and close-time
// commit protocol; the new unarmed-authoring guard must not intercept them.
expect(response?.ok).toBe(true);
expect(sessionStore.get(sessionName)).toBeUndefined();
});
20 changes: 19 additions & 1 deletion src/daemon/handlers/session-close.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,29 @@ async function stopOrRetainAppleRunnerAfterClose(
scheduleIosRunnerIdleStop(session.device.id);
}

// Live evidence (2026-08-02): a plain `open` followed by `close --save-script` used to fold into
// the authoring lifecycle at close time (`applyRecordedSaveScriptFlags`'s `none -> authoring`
// branch) and publish anyway. That silently produces a script whose actions carry selector
// fallback chains but no `target-v1` recording-time evidence — degraded replay verification with
// no signal to the caller. Recording-time evidence can only be captured from action zero
// (`armAuthoringOnOpen`), so an unarmed session has nothing to retroactively arm; the only
// correct response is refusal, before any teardown or publication work runs. This intentionally
// does not resolve #1533 (aborted-mid-recording close --save-script); that is a distinct,
// already-armed case with its own resolution.
function assertTerminalRecordingCloseAllowed(req: DaemonRequest, session: SessionState): void {
if (!req.flags?.saveScript) return;
if (isAuthoringArmedSession(session)) return;
const state = session.scriptPublication;
if (state?.kind !== 'authoring') return;
if (state?.kind === 'repair') return;
if (state === undefined || state.kind === 'none') {
throw new AppError(
'INVALID_ARGS',
'close --save-script cannot publish this session: recording was not armed before this journey began, so there is no recording-time target evidence to publish.',
{
hint: 'Retry with plain close (it tears down without writing). To capture a publishable recording, start a fresh session with open <app> --save-script[=<path>].',
},
);
}
throw new AppError(
'INVALID_ARGS',
`close --save-script cannot ${state.status === 'published' ? 're-publish' : 'publish'} this terminal recording. Retry with plain close; it will tear down the session without writing.`,
Expand Down
13 changes: 8 additions & 5 deletions src/daemon/session-script-publication-capability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,14 @@ export function abortAuthoringOnSecondOpen(session: SessionState): void {
* other command's raw flag closed at the router). Arms recording and applies target/force to
* whichever lifecycle the session is in:
*
* - `none` -> ordinary authoring armed. This is how a never-armed `close --save-script`
* publishes the whole log: the close request arms at record time and publishes moments later
* in the same request, folding the former "third mode" into the authoring lifecycle. The
* session is deleted by every close path that gets this far, so the transient armed state is
* unobservable to `session save-script` eligibility.
* - `none` -> ordinary authoring armed. This branch is UNREACHABLE for `close`: live evidence
* (2026-08-02) showed it used to let a never-armed `close --save-script` fold into the
* authoring lifecycle and publish moments later in the same request — a script with selector
* fallback chains but no recording-time `target-v1` evidence, and no signal to the caller.
* `session-close.ts`'s `assertTerminalRecordingCloseAllowed` now rejects an unarmed
* `close --save-script` before any action recording runs, so this arm only fires for a
* future non-close caller of the shared ingress; it is kept as that caller's safety net, not
* as a documented close-time behavior.
* - `authoring` -> retarget under the #1258 per-target force rule (`resolveScriptTarget`).
* - `repair` -> retarget the repair target the same way (a replayed step may carry the flag).
*/
Expand Down
Loading
Loading