-
-
Notifications
You must be signed in to change notification settings - Fork 315
refactor(move): relocate replay resume and failure responses #3030
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| import type { SessionAction } from '@agent-device/contracts/session'; | ||
| import assert from 'node:assert/strict'; | ||
| import { test } from 'vitest'; | ||
| import { buildReplayDivergenceResume } from '../session-replay-resume.ts'; | ||
|
|
||
| function action(overrides: Partial<SessionAction> = {}): SessionAction { | ||
| return { ts: 0, command: 'click', positionals: ['label="Save"'], flags: {}, ...overrides }; | ||
| } | ||
|
|
||
| test('buildReplayDivergenceResume reports a resumable generic .ad failure', () => { | ||
| const actions: SessionAction[] = [ | ||
| action({ command: 'open', positionals: ['Demo'] }), | ||
| action({ command: 'click', positionals: ['label="Save"'] }), | ||
| ]; | ||
| // `state-repair` carries no `alternateFrom` (no recorded-action alternate), | ||
| // so this stays a clean base-shape assertion — the caution/manual | ||
| // `alternateFrom` cases are covered by the dedicated tests below. | ||
| const resume = buildReplayDivergenceResume({ | ||
| failedIndex: 2, | ||
| actions, | ||
| planDigest: 'abc123', | ||
| repairHint: 'state-repair', | ||
| sessionExists: true, | ||
| }); | ||
| assert.deepEqual(resume, { allowed: true, from: 2, planDigest: 'abc123' }); | ||
| }); | ||
| // --- repairHint 'record-and-heal' shifts `from` to failedIndex + 1 (ADR 0012 | ||
| // decision 6, R2): the agent already performed the diverged step manually, so | ||
| // resuming AT it would re-diverge on the exact same step. --- | ||
|
|
||
| test('buildReplayDivergenceResume with repairHint record-and-heal resumes AFTER the failed step', () => { | ||
| const actions: SessionAction[] = [ | ||
| action({ command: 'open', positionals: ['Demo'] }), | ||
| action({ command: 'click', positionals: ['label="Save"'] }), | ||
| action({ command: 'click', positionals: ['label="Confirm"'] }), | ||
| ]; | ||
| const resume = buildReplayDivergenceResume({ | ||
| failedIndex: 2, | ||
| actions, | ||
| planDigest: 'abc123', | ||
| repairHint: 'record-and-heal', | ||
| sessionExists: true, | ||
| }); | ||
| assert.deepEqual(resume, { allowed: true, from: 3, planDigest: 'abc123' }); | ||
| }); | ||
|
|
||
| test('buildReplayDivergenceResume with repairHint record-and-heal on the LAST plan step is a legal empty-tail resume', () => { | ||
| const actions: SessionAction[] = [ | ||
| action({ command: 'open', positionals: ['Demo'] }), | ||
| action({ command: 'click', positionals: ['label="Save"'] }), | ||
| ]; | ||
| // failedIndex 2 (the last of 2 actions) shifts to from 3 = actions.length + | ||
| // 1 — there is no step 3 to run, but that is not an error: the runtime | ||
| // executes zero steps and reaches the normal end-of-plan completion path. | ||
| const resume = buildReplayDivergenceResume({ | ||
| failedIndex: 2, | ||
| actions, | ||
| planDigest: 'abc123', | ||
| repairHint: 'record-and-heal', | ||
| sessionExists: true, | ||
| }); | ||
| assert.deepEqual(resume, { allowed: true, from: 3, planDigest: 'abc123' }); | ||
| }); | ||
| // --- buildReplayDivergenceResume: `resume.alternateFrom` (#1262). The | ||
| // `caution`/`manual` dual-path's SECOND ordinal (`failedIndex + 1`), present | ||
| // ONLY when a `--from failedIndex + 1` request would actually be accepted. | ||
| // Generic `.ad` plans contain no runtime variable producers or control | ||
| // wrappers, so only the empty-tail session requirement can block it. --- | ||
|
|
||
| test('buildReplayDivergenceResume: caution mid-plan carries alternateFrom = failedIndex + 1', () => { | ||
| const actions: SessionAction[] = [ | ||
| action({ command: 'open' }), | ||
| action({ command: 'click' }), | ||
| action({ command: 'click' }), | ||
| ]; | ||
| const resume = buildReplayDivergenceResume({ | ||
| failedIndex: 2, | ||
| actions, | ||
| planDigest: 'abc123', | ||
| repairHint: 'caution', | ||
| sessionExists: true, | ||
| }); | ||
| assert.equal(resume.allowed, true); | ||
| assert.equal(resume.from, 2); // unshifted | ||
| if (!resume.allowed) return; | ||
| assert.equal(resume.alternateFrom, 3); | ||
| }); | ||
|
|
||
| test('buildReplayDivergenceResume: manual last-step carries alternateFrom = actions.length + 1', () => { | ||
| const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })]; | ||
| const resume = buildReplayDivergenceResume({ | ||
| failedIndex: 2, | ||
| actions, | ||
| planDigest: 'abc123', | ||
| repairHint: 'manual', | ||
| sessionExists: true, | ||
| }); | ||
| assert.equal(resume.allowed, true); | ||
| assert.equal(resume.from, 2); | ||
| if (!resume.allowed) return; | ||
| assert.equal(resume.alternateFrom, 3); // empty-tail ordinal | ||
| }); | ||
|
|
||
| test('buildReplayDivergenceResume: record-and-heal and state-repair never carry alternateFrom (no separate recorded-action alternate)', () => { | ||
| const actions: SessionAction[] = [ | ||
| action({ command: 'open' }), | ||
| action({ command: 'click' }), | ||
| action({ command: 'click' }), | ||
| ]; | ||
| for (const repairHint of ['record-and-heal', 'state-repair'] as const) { | ||
| const resume = buildReplayDivergenceResume({ | ||
| failedIndex: 2, | ||
| actions, | ||
| planDigest: 'abc123', | ||
| repairHint, | ||
| sessionExists: true, | ||
| }); | ||
| assert.equal(resume.allowed, true); | ||
| if (!resume.allowed) return; | ||
| assert.equal(resume.alternateFrom, undefined, `expected no alternateFrom for ${repairHint}`); | ||
| } | ||
| }); | ||
|
|
||
| // --- #1262 (re-review): the EMPTY-TAIL alternate (`failedIndex + 1 > | ||
| // actions.length`) is authorizable only via the `pendingRecordAndHeal` | ||
| // watermark, which can only be stamped on a LIVE session. With NO session — a | ||
| // one-step `open` failure, or a session closed mid-replay — advertising | ||
| // `--from actions.length + 1` would be rejected as out of range, so it must | ||
| // not be emitted. A MID-PLAN alternate (in range) needs no watermark and stays | ||
| // session-independent. --- | ||
|
|
||
| test('buildReplayDivergenceResume: LAST-step caution/manual with NO session carries NO alternateFrom (empty-tail needs a watermark, which needs a session)', () => { | ||
| const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })]; | ||
| for (const repairHint of ['caution', 'manual'] as const) { | ||
| const resume = buildReplayDivergenceResume({ | ||
| failedIndex: 2, // last step → alternate would be the one-past-end ordinal 3 | ||
| actions, | ||
| planDigest: 'abc123', | ||
| repairHint, | ||
| sessionExists: false, | ||
| }); | ||
| assert.equal(resume.allowed, true); // resuming AT the failed step (2) is still fine | ||
| if (!resume.allowed) return; | ||
| assert.equal( | ||
| resume.alternateFrom, | ||
| undefined, | ||
| `expected no empty-tail alternateFrom without a session for ${repairHint}`, | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| test('buildReplayDivergenceResume: MID-PLAN caution/manual with NO session STILL carries alternateFrom (in-range, no watermark needed)', () => { | ||
| // 3-step plan; failedIndex 2 → alternate 3 is IN RANGE (<= actions.length), | ||
| // so it needs no watermark and is emitted regardless of session existence. | ||
| const actions: SessionAction[] = [ | ||
| action({ command: 'open' }), | ||
| action({ command: 'click' }), | ||
| action({ command: 'click' }), | ||
| ]; | ||
| for (const repairHint of ['caution', 'manual'] as const) { | ||
| const resume = buildReplayDivergenceResume({ | ||
| failedIndex: 2, | ||
| actions, | ||
| planDigest: 'abc123', | ||
| repairHint, | ||
| sessionExists: false, | ||
| }); | ||
| assert.equal(resume.allowed, true); | ||
| if (!resume.allowed) return; | ||
| assert.equal(resume.alternateFrom, 3, `expected mid-plan alternateFrom for ${repairHint}`); | ||
| } | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,48 @@ | ||||||||||||
| import { test, expect } from 'vitest'; | ||||||||||||
| import { buildReplayDivergenceFailureResponseFromDescriptor } from '../session-replay-runtime-failure-response.ts'; | ||||||||||||
|
|
||||||||||||
| test('native replay failure metadata keeps machine fields and daemon-owned paths intact', () => { | ||||||||||||
| const replayPath = '/tmp/flows/ios-login.ad'; | ||||||||||||
| const artifactPath = '/tmp/sessions/default/screenshot-1.png'; | ||||||||||||
| const response = buildReplayDivergenceFailureResponseFromDescriptor({ | ||||||||||||
| error: { | ||||||||||||
| code: 'COMMAND_FAILED', | ||||||||||||
| message: 'Could not tap Continue on ios', | ||||||||||||
| hint: 'Retry Continue on ios', | ||||||||||||
| details: { | ||||||||||||
| reason: 'not_found', | ||||||||||||
| retriable: false, | ||||||||||||
| supportedOn: 'ios', | ||||||||||||
| }, | ||||||||||||
| retriable: false, | ||||||||||||
| supportedOn: 'ios', | ||||||||||||
| }, | ||||||||||||
| actionLabel: 'press Continue', | ||||||||||||
| action: 'press', | ||||||||||||
| positionals: ['Continue'], | ||||||||||||
| step: 2, | ||||||||||||
| replayPath, | ||||||||||||
| artifactPaths: [artifactPath], | ||||||||||||
| divergence: {}, | ||||||||||||
| scrubVars: [ | ||||||||||||
| { name: 'MODE', value: 'on' }, | ||||||||||||
| { name: 'PLATFORM', value: 'ios' }, | ||||||||||||
| { name: 'SESSION', value: 'default' }, | ||||||||||||
| ], | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| expect(response.ok).toBe(false); | ||||||||||||
| if (response.ok) return; | ||||||||||||
| expect(response.error.retriable).toBe(false); | ||||||||||||
| expect(response.error.supportedOn).toBe('ios'); | ||||||||||||
| expect(response.error.details).toMatchObject({ | ||||||||||||
| reason: 'not_found', | ||||||||||||
| retriable: false, | ||||||||||||
| supportedOn: 'ios', | ||||||||||||
| replayPath, | ||||||||||||
| positionals: ['Continue'], | ||||||||||||
| artifactPaths: [artifactPath], | ||||||||||||
| }); | ||||||||||||
| expect(response.error.details).toHaveProperty('reason'); | ||||||||||||
| expect(response.error.details).not.toHaveProperty('reas<var:MODE>'); | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The Prompt for AI agents
Suggested change
|
||||||||||||
| }); | ||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,11 +32,11 @@ const RUNTIME_FILE = 'src/daemon/handlers/session-replay-command.ts'; | |
|
|
||
| /** The divergence-report chain: never a second `ReplayCoordinator`, never a bare `SessionStore`. */ | ||
| const DIVERGENCE_CHAIN_FILES = [ | ||
| 'src/daemon/replay/internal/session-replay-resume.ts', | ||
| 'packages/replay-port/src/daemon-port/session-replay-resume.ts', | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: These two chain files moved out of Prompt for AI agentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Prompt for AI agents |
||
| 'src/daemon/replay/internal/session-replay-divergence.ts', | ||
| 'src/daemon/replay/internal/session-replay-target-verification.ts', | ||
| 'src/daemon/replay/internal/session-replay-runtime-failure.ts', | ||
| 'src/daemon/replay/internal/session-replay-runtime-failure-response.ts', | ||
| 'packages/replay-port/src/daemon-port/session-replay-runtime-failure-response.ts', | ||
| ] as const; | ||
|
|
||
| type ImportSite = { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: These mid-file comment blocks narrate implementation decisions and review history (#1262 "re-review", ADR 0012 rationales) between tests, which AGENTS.md's comment rule explicitly discourages ("No control-flow narration or review history in comments; encode invariants in names/types/boundaries/tests"). The tests already encode each boundary case in its name and assertions; keep at most a one-line note where the boundary is non-obvious (e.g., the empty-tail-without-session rule).
Prompt for AI agents