refactor(apple-runner): move command exchange out of session owner

[thymikee]

Summary

Second review layer for #2967, based on #3011. Move readiness decisions, command send, response decoding, diagnostics, and #2965 charge settlement into runner-exchange.ts. runner-session.ts keeps the registry, lease, startup/disposal, and awaited fatal invalidation; it falls below 1,000 lines. The exchange loads when a command uses it, preserving facade import budgets. No second state owner or transport retry policy is added.

Rebased on #3011 at fb7244388, final 4e77d99be: production +512/−480 (net +32); tests +3/−3; fixtures/docs 0; 6 files. The old mixed command path is removed; net growth is module imports and the explicit invalidation boundary. Rename-aware gross diff: 998 lines.

Validation

pnpm check:affected --run passed at 4e77d99be (format, lint, typecheck, layering, fallow, build, 2,538 related tests). The inherited #2965 accounting and recovery suites remained green. CI coverage, integration, and native lanes are pending; live device verification belongs to the completed top layer.

[cubic-dev-ai]

2 issues found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/platform-apple/src/runner/runner-exchange.ts">

<violation number="1" location="packages/platform-apple/src/runner/runner-exchange.ts:320">
P2: The readiness preflight drops the runner’s `runnerMainThreadBusy` stamp. Copy the parsed stamp into `session.runnerMainThreadBusy` while preserving the existing value when the field is absent, otherwise a lost command response can make an occupied runner look safe to hand off.</violation>

<violation number="2" location="packages/platform-apple/src/runner/runner-exchange.ts:399">
P1: The exemption bypasses startup readiness, so a first `activate`, `terminate`, or `targetReset` on a newly launched session uses one direct request before the listener is ready and can fail with connection refused. Apply the exemption only when `session.state === 'ready'`; starting sessions must use the startup preflight.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P1: The exemption bypasses startup readiness, so a first activate, terminate, or targetReset on a newly launched session uses one direct request before the listener is ready and can fail with connection refused. Apply the exemption only when session.state === 'ready'; starting sessions must use the startup preflight.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform-apple/src/runner/runner-exchange.ts, line 399:

<comment>The exemption bypasses startup readiness, so a first `activate`, `terminate`, or `targetReset` on a newly launched session uses one direct request before the listener is ready and can fail with connection refused. Apply the exemption only when `session.state === 'ready'`; starting sessions must use the startup preflight.</comment>

<file context>
@@ -0,0 +1,493 @@
+  command: RunnerCommand,
+): RunnerReadinessPreflightDecision {
+  const readOnlyCommand = isReadOnlyRunnerCommand(command);
+  if (isRunnerReadinessPreflightExempt(command)) {
+    return { action: 'skip', reason: 'preflight_exempt_command' };
+  }
</file context>

[cubic-dev-ai]

P2: The readiness preflight drops the runner’s runnerMainThreadBusy stamp. Copy the parsed stamp into session.runnerMainThreadBusy while preserving the existing value when the field is absent, otherwise a lost command response can make an occupied runner look safe to hand off.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform-apple/src/runner/runner-exchange.ts, line 320:

<comment>The readiness preflight drops the runner’s `runnerMainThreadBusy` stamp. Copy the parsed stamp into `session.runnerMainThreadBusy` while preserving the existing value when the field is absent, otherwise a lost command response can make an occupied runner look safe to hand off.</comment>

<file context>
@@ -0,0 +1,493 @@
+        timeoutMs: readinessTimeoutMs,
+      },
+    );
+    await parseRunnerResponse(readinessResponse, session, logAttempt);
+  } catch (error) {
+    throw markRunnerReadinessPreflightError(error);
</file context>

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.88 MB	4.88 MB	+1.0 kB
Package (unpacked)	4.88 MB	4.88 MB	+1.0 kB
Package (download)	1.46 MB	1.46 MB	+426 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	31.7 ms	31.1 ms	-0.6 ms
CLI --help	93.6 ms	90.1 ms	-3.5 ms

[thymikee]

Reviewed at 7be94ce. The removed lines match the new runner-exchange.ts, except for one new seam: the injected invalidateFatalSession callback, which keeps awaited fatal invalidation with the session owner without an import cycle. Is that callback the smallest way to do this, or did you consider and reject another split?

Not blocking: readRunnerStartupTimeoutMs only reads launchDeadline, so it could live in runner-start-budget.ts instead of behind a new dynamic import in runner-lifecycle.ts line 297.

CI: checks are still queued. This PR stacks on #3011, which needs to land first.

[thymikee]

The injected invalidateFatalSession callback keeps process disposal with the session owner and lets the exchange await it without importing the owner back into the exchange module. I kept that seam rather than moving disposal into the transport layer, which would create a second resource owner. The busy-stamp and startup-exemption findings are valid; both are fixed in child #3006 at a7a7301 with focused regressions. They were present in the pre-extraction code, and this layer is already at the 1,000-line review budget (998 gross), so the behavior corrections live in the next layer of the stack. Exact-head affected gate passed on #3006; its new CI run is pending.

[thymikee]

Thanks, that answers it. Keeping disposal with the session owner avoids a second resource owner, so the injected callback is the right seam. The code at 7be94ce is ready for human review; it still stacks on #3011, and the busy-stamp and startup-exemption fixes land in #3006.

[thymikee]

Reviewed at 4e77d99. The code still looks clean: after the restack onto #3011, the patch is byte-identical to the dfa9b15 version I reviewed before, so it is still a pure move of the command exchange into runner-exchange.ts with no behavior change.

The cancelled CI jobs were superseded by the restack, not failed. iOS Smoke Tests exercises this runner route, so its rerun on 4e77d99 is the result to wait for. #3011 needs to land first, since this PR is stacked on it.