Skip to content

refactor(runner): declare journal retention in command traits #3094

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.

Sign up for GitHub

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

Merged
thymikee merged 1 commit into from
Oct 1, 2026
Merged

refactor(runner): declare journal retention in command traits #3094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 1 addition & 14 deletions apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandJournal.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,22 +125,9 @@ final class RunnerCommandJournal {
}

private func encodeResponseJson(command: Command, response: Response) -> String? {
guard shouldRetainResponseJson(command: command) else { return nil }
guard command.traits.retainsJournalResponseJson else { return nil }
guard let data = try? JSONEncoder().encode(response) else { return nil }
guard data.count <= maxResponseJsonBytes else { return nil }
return String(data: data, encoding: .utf8)
}

private func shouldRetainResponseJson(command: Command) -> Bool {
switch command.command {
case .snapshot, .screenshot:
return false
case .tap, .mouseClick, .longPress, .drag,
.remotePress, .type, .swipe, .scroll, .desktopScroll, .findText, .querySelector, .readText,
.backInApp, .backSystem, .home, .rotate, .appSwitcher, .actionButton, .keyboardDismiss, .keyboardReturn,
.alert, .sequence, .gesture, .gestureViewport, .recordStart, .recordStop,
.status, .uptime, .appState, .pasteboardWrite, .activate, .terminate, .targetReset, .shutdown:
return true
}
}
}
62 changes: 44 additions & 18 deletions apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,25 +93,29 @@ struct CommandTraits {
let isInteraction: Bool
/// Whether the command is eligible for the session-invalidating retry.
let retryOnSessionLoss: Bool
/// What the runner may do when the command's app is not running. The one fact with no default: no
/// command inherits a launch answer from how it was classified for anything else.
/// What the runner may do when the command's app is not running. No command inherits a launch
/// answer from how it was classified for anything else.
let launchPolicy: CommandLaunchPolicy
/// Whether an XCTest-recorded failure during this command turns its own healthy response into a
/// failure and invalidates the session. That conversion is the only evidence a mutation with no
/// settle and no post-action observation ever landed, while a command that reports the runner's own
/// state or drives its lifecycle has no user-visible mutation to prove.
let convertsRecordedFailure: Bool
/// Whether the journal may store this command's response JSON, subject to its byte limit.
let retainsJournalResponseJson: Bool

init(
isInteraction: Bool = false,
retryOnSessionLoss: Bool = false,
launchPolicy: CommandLaunchPolicy,
convertsRecordedFailure: Bool = false
convertsRecordedFailure: Bool = false,
retainsJournalResponseJson: Bool
) {
self.isInteraction = isInteraction
self.retryOnSessionLoss = retryOnSessionLoss
self.launchPolicy = launchPolicy
self.convertsRecordedFailure = convertsRecordedFailure
self.retainsJournalResponseJson = retainsJournalResponseJson
}
}

Expand All @@ -121,52 +125,68 @@ fileprivate extension CommandTraits {
static let interaction = CommandTraits(
isInteraction: true,
launchPolicy: .mayLaunch,
convertsRecordedFailure: true
convertsRecordedFailure: true,
retainsJournalResponseJson: true
)

/// Mutations the runner performs without the element-interaction preflight. NOTE: `mouseClick`
/// stays non-interaction for now — it is macOS-only and the foreground guard interacts with
/// bespoke macOS activation, so classifying it needs a macOS smoke check first (tracked as a
/// follow-up).
static let appMutation = CommandTraits(launchPolicy: .mayLaunch, convertsRecordedFailure: true)
static let appMutation = CommandTraits(
launchPolicy: .mayLaunch, convertsRecordedFailure: true, retainsJournalResponseJson: true
)

/// Reads of the session app: replayable after session invalidation, and refused rather than
/// answered by starting the app.
static let appRead = CommandTraits(retryOnSessionLoss: true, launchPolicy: .existingApp)
static func appRead(retainsJournalResponseJson: Bool) -> CommandTraits {
CommandTraits(
retryOnSessionLoss: true, launchPolicy: .existingApp,
retainsJournalResponseJson: retainsJournalResponseJson
)
}

/// Selector resolution is an observation: it refuses a stopped app instead of bare-launching it,
/// and the runner still must not replay it after session invalidation. Those are two facts about
/// one command, which is why they are two declarations (#2890). The refusal is the iOS-enforced
/// half: off iOS a selector read of a stopped app still activates it, as it did before this axis.
static let selectorResolution = CommandTraits(
launchPolicy: .existingApp,
convertsRecordedFailure: true
convertsRecordedFailure: true,
retainsJournalResponseJson: true
)

/// Reads the runner answers from its own capture and state, so preparation never brings an app
/// forward; a capture aimed at an app still observes that app while it executes.
static let runnerCaptureRead = CommandTraits(retryOnSessionLoss: true, launchPolicy: .noApp)
static func runnerCaptureRead(retainsJournalResponseJson: Bool) -> CommandTraits {
CommandTraits(
retryOnSessionLoss: true, launchPolicy: .noApp,
retainsJournalResponseJson: retainsJournalResponseJson
)
}

/// The runner's own lifecycle: no session app is brought forward, and no mutation is proven.
static let runnerLifecycle = CommandTraits(launchPolicy: .noApp)
static let runnerLifecycle = CommandTraits(launchPolicy: .noApp, retainsJournalResponseJson: true)

/// Device state the runner sets from its own process, such as the pasteboard: no app is brought
/// forward, since the state belongs to the device rather than to the session app, and no UI
/// mutation is proven.
static let deviceState = CommandTraits(launchPolicy: .noApp)
static let deviceState = CommandTraits(launchPolicy: .noApp, retainsJournalResponseJson: true)

/// Commands hosted by the surface that already has focus, which no activation may cancel. A
/// hardware press belongs to the system rather than to the session app, and an alert answers from
/// the modal where it sits; both mutate.
static let presentedSurfaceMutation = CommandTraits(
launchPolicy: .presentedSurface,
convertsRecordedFailure: true
convertsRecordedFailure: true,
retainsJournalResponseJson: true
)

/// `alert get` changes nothing, so it is the one alert action that may be replayed.
static let presentedSurfaceQuery = CommandTraits(
retryOnSessionLoss: true,
launchPolicy: .presentedSurface
launchPolicy: .presentedSurface,
retainsJournalResponseJson: true
)
}

Expand All @@ -179,8 +199,8 @@ extension CommandTraits {

extension Command {
/// Whether arriving at the prepared command path invalidates a remembered text-entry tap. Not a
/// fifth trait: everywhere but the two owner commands, it is having a mutation to prove that makes
/// the witness stale, so this reads `convertsRecordedFailure` and that set rather than declaring a
/// separate trait: everywhere but the two owner commands, having a mutation to prove makes the
/// witness stale, so this reads `convertsRecordedFailure` and that set rather than declaring a
/// fact no command would answer for itself (#2890 review). `executeOnMainPrepared` is its only
/// consumer, and the exhaustive table test pins the answer for every command.
var invalidatesRememberedTextEntryTap: Bool {
Expand Down Expand Up @@ -243,13 +263,19 @@ extension Command {
.keyboardDismiss, .keyboardReturn, .sequence, .gesture:
return .interaction

case .findText, .readText, .snapshot, .gestureViewport:
return .appRead
case .findText, .readText, .gestureViewport:
return .appRead(retainsJournalResponseJson: true)

case .snapshot:
return .appRead(retainsJournalResponseJson: false)

// appState reads the session app's XCUIApplication.state; bringing no app forward is what makes
// its answer the state the app is in, not the one a repair leaves.
case .screenshot, .status, .appState:
return .runnerCaptureRead
case .status, .appState:
return .runnerCaptureRead(retainsJournalResponseJson: true)

case .screenshot:
return .runnerCaptureRead(retainsJournalResponseJson: false)

case .alert:
return (action ?? "get").lowercased() == "get"
Expand Down
92 changes: 50 additions & 42 deletions ...runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,22 @@ extension RunnerTests {
let retryOnSessionLoss: Bool
let launchPolicy: CommandLaunchPolicy
let convertsRecordedFailure: Bool
let retainsJournalResponseJson: Bool
}

private func expectation(
interaction: Bool,
retry: Bool,
launch: CommandLaunchPolicy,
converts: Bool
converts: Bool,
retains: Bool
) -> ExpectedTraits {
ExpectedTraits(
isInteraction: interaction,
retryOnSessionLoss: retry,
launchPolicy: launch,
convertsRecordedFailure: converts
convertsRecordedFailure: converts,
retainsJournalResponseJson: retains
)
}

Expand All @@ -186,6 +189,11 @@ extension RunnerTests {
expectation.convertsRecordedFailure,
"\(request) convertsRecordedFailure"
)
XCTAssertEqual(
traits.retainsJournalResponseJson,
expectation.retainsJournalResponseJson,
"\(request) retainsJournalResponseJson"
)
}

/// The commands the merge-base classified read-only, copied from its `CommandType.traits`
Expand Down Expand Up @@ -244,54 +252,54 @@ extension RunnerTests {
/// a concrete launch case, so re-pointing a command at another policy fails that row.
func testEveryCommandDeclaresEveryRunnerSideDecisionTogether() throws {
let table: [(CommandType, ExpectedTraits)] = [
(.tap, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.mouseClick, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true)),
(.longPress, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.drag, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.remotePress, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.type, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.swipe, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.scroll, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.desktopScroll, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.findText, expectation(interaction: false, retry: true, launch: .existingApp, converts: false)),
(.tap, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.mouseClick, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.longPress, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.drag, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.remotePress, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.type, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.swipe, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.scroll, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.desktopScroll, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.findText, expectation(interaction: false, retry: true, launch: .existingApp, converts: false, retains: true)),
(
.querySelector,
expectation(interaction: false, retry: false, launch: .existingApp, converts: true)
expectation(interaction: false, retry: false, launch: .existingApp, converts: true, retains: true)
),
(.readText, expectation(interaction: false, retry: true, launch: .existingApp, converts: false)),
(.snapshot, expectation(interaction: false, retry: true, launch: .existingApp, converts: false)),
(.screenshot, expectation(interaction: false, retry: true, launch: .noApp, converts: false)),
(.backInApp, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.backSystem, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.home, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true)),
(.rotate, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.appSwitcher, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.readText, expectation(interaction: false, retry: true, launch: .existingApp, converts: false, retains: true)),
(.snapshot, expectation(interaction: false, retry: true, launch: .existingApp, converts: false, retains: false)),
(.screenshot, expectation(interaction: false, retry: true, launch: .noApp, converts: false, retains: false)),
(.backInApp, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.backSystem, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.home, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.rotate, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.appSwitcher, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(
.actionButton,
expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true)
expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true, retains: true)
),
(.keyboardDismiss, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.keyboardReturn, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.keyboardDismiss, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.keyboardReturn, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(
.alert,
expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false)
expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false, retains: true)
),
(.sequence, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.gesture, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)),
(.sequence, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.gesture, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(
.gestureViewport,
expectation(interaction: false, retry: true, launch: .existingApp, converts: false)
expectation(interaction: false, retry: true, launch: .existingApp, converts: false, retains: true)
),
(.recordStart, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true)),
(.recordStop, expectation(interaction: false, retry: false, launch: .noApp, converts: false)),
(.status, expectation(interaction: false, retry: true, launch: .noApp, converts: false)),
(.uptime, expectation(interaction: false, retry: false, launch: .noApp, converts: false)),
(.appState, expectation(interaction: false, retry: true, launch: .noApp, converts: false)),
(.pasteboardWrite, expectation(interaction: false, retry: false, launch: .noApp, converts: false)),
(.activate, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true)),
(.terminate, expectation(interaction: false, retry: false, launch: .noApp, converts: false)),
(.targetReset, expectation(interaction: false, retry: false, launch: .noApp, converts: false)),
(.shutdown, expectation(interaction: false, retry: false, launch: .noApp, converts: false))
(.recordStart, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.recordStop, expectation(interaction: false, retry: false, launch: .noApp, converts: false, retains: true)),
(.status, expectation(interaction: false, retry: true, launch: .noApp, converts: false, retains: true)),
(.uptime, expectation(interaction: false, retry: false, launch: .noApp, converts: false, retains: true)),
(.appState, expectation(interaction: false, retry: true, launch: .noApp, converts: false, retains: true)),
(.pasteboardWrite, expectation(interaction: false, retry: false, launch: .noApp, converts: false, retains: true)),
(.activate, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true, retains: true)),
(.terminate, expectation(interaction: false, retry: false, launch: .noApp, converts: false, retains: true)),
(.targetReset, expectation(interaction: false, retry: false, launch: .noApp, converts: false, retains: true)),
(.shutdown, expectation(interaction: false, retry: false, launch: .noApp, converts: false, retains: true))
]
for (type, rowExpectation) in table {
let request = #"{"command":"\#(type.rawValue)"}"#
Expand All @@ -314,15 +322,15 @@ extension RunnerTests {
// The one payload-dependent command settles each fact per action: `get` changes nothing and may
// be replayed, while `accept` and `dismiss` mutate and must not be.
let alertCases: [(action: String?, expectation: ExpectedTraits)] = [
(nil, expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false)),
("get", expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false)),
(nil, expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false, retains: true)),
("get", expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false, retains: true)),
(
"accept",
expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true)
expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true, retains: true)
),
(
"dismiss",
expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true)
expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true, retains: true)
)
]
for alertCase in alertCases {
Expand Down
Loading