fix: tune iOS runner response retention

[thymikee]

Summary

Tune the iOS runner command journal retention policy so small lifecycle responses remain recoverable from lifecycleResponseJson, while snapshot and screenshot commands finish without retained response JSON.

The retention fixture now documents the policy for scalar responses, small object/node responses, snapshot trees, screenshots, oversized JSON, and snapshot error metadata. Error code/message/hint fields remain available even when response JSON is intentionally dropped.

Touched-file count: 1. Scope stayed within the iOS runner command journal.

Validation

pnpm format passed.
pnpm typecheck passed.
pnpm exec vitest run src/platforms/ios/__tests__/runner-session.test.ts src/platforms/ios/__tests__/runner-client.test.ts passed: 58 tests.
pnpm build:xcuitest passed for iOS and macOS runner targets.
pnpm build passed before runtime validation.
pnpm clean:daemon ran before and after runtime validation.

Simulator validation on iPhone 17 Pro (C25DBB5B-9254-4293-A8D5-2785C78DE03A): direct runner status recovered a small uptime response from lifecycleResponseJson; a snapshot command reported completed and lifecycleResponseOk=true with no retained response JSON; three repeated snapshots completed; a final uptime confirmed the runner stayed responsive. A stale validation xcodebuild PID was found and stopped, and the final process check only matched the check command itself.

Known gap: the public CLI does not expose the internal runner status command, so retained-response evidence used the runner client directly rather than a named daemon session.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.1 MB	1.1 MB	0 B
JS gzip	360.8 kB	360.8 kB	0 B
npm tarball	463.7 kB	464.8 kB	+1.1 kB
npm unpacked	1.5 MB	1.5 MB	+5.7 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	25.8 ms	25.9 ms	+0.1 ms
CLI --help	40.6 ms	41.2 ms	+0.6 ms

Top changed chunks: no changes in the largest emitted chunks.

[thymikee]

Addressed the retention-default finding in d243f1b06: shouldRetainResponseJson now uses an exhaustive CommandType switch with no default, so future runner commands must make an explicit retention decision at compile time.

Validation rerun: pnpm build:xcuitest passed.

[thymikee]

Review

Good change. Replacing the heuristic guard response.data?.nodes == nil with an explicit per-command shouldRetainResponseJson switch makes the retention policy intentional rather than incidental, and leaving the switch default-less means adding a new Command case forces a compile-time decision about retention — that's the right call.

Notes:

1. Behavioral change worth confirming. Under the old rule, any response carrying nodes was dropped. Now querySelector/findText/readText are in the retain list, so their node-bearing results will be retained (still bounded by maxResponseJsonBytes). The new test covers the small-querySelector-with-one-node case explicitly, so this looks intended — just flagging that it's a real semantics change from "no nodes ever retained" to "snapshot/screenshot never retained, others retained up to the size cap."

2. The error-metadata test (testCommandJournalKeepsErrorMetadataWhenResponseJsonIsDropped) is valuable — confirming lifecycleErrorCode/Message/Hint survive even when lifecycleResponseJson is intentionally nil is exactly the recovery contract the TS side relies on.

3. Minor: the retain branch lists ~30 commands. If the Command enum is large this is fine, but consider whether the inverse (only snapshot/screenshot → false, default → true) would be less churn to maintain — at the cost of losing the compile-time "decide for new commands" guarantee. Your current form is the safer one; just a tradeoff to be aware of.

LGTM pending CI.

---

Generated by Claude Code

[thymikee]

Reviewed current head d243f1b06. No blocking findings.

The previous retention-policy concern is addressed: shouldRetainResponseJson is now exhaustive over CommandType, so future runner commands must make an explicit retention decision at compile time. The 16 KB cap and snapshot/screenshot exclusions remain intact.

[thymikee]

Confirmed the behavioral-change note: retaining small targeted results such as querySelector is intentional. The old nodes == nil heuristic was too broad for recovery because it dropped useful small command results; the new policy is command-based instead:

• snapshot and screenshot are never retained because they represent full-tree or binary/artifact-heavy paths.
• targeted scalar/object responses, including small querySelector results, are retained only while they fit under maxResponseJsonBytes.
• error code/message/hint metadata is preserved independently of retained response JSON.

Keeping the switch exhaustive is deliberate despite the longer retain list, since it forces every future runner command to make an explicit retention decision.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-06-02 03:24 UTC