Skip to content

refactor: extract platform network runtime - #1702

Merged
thymikee merged 3 commits into
agent/platform-runtime-logsfrom
agent/platform-runtime-network
Aug 10, 2026
Merged

refactor: extract platform network runtime#1702
thymikee merged 3 commits into
agent/platform-runtime-logsfrom
agent/platform-runtime-network

Conversation

@thymikee

@thymikee thymikee commented Aug 10, 2026

Copy link
Copy Markdown
Member

Summary

Extract the network command into the combined platform runtime.

  • add contract-owned network plans, traffic parsing, facts, and one networkDump operation
  • compose lazy Apple, Android, Web, Limrun, WebDriver, and unavailable-family implementations without provider-to-local fallback
  • route all eight dump|log × include cells through one runtime-backed daemon path and remove the legacy daemon parser/recovery modules
  • add R15 structural gates for command atomicity, admission, narrowing, and retired-route absence
  • keep stacked changed-line coverage working for diffs larger than Node's default synchronous process buffer
  • preserve absolute app-log line numbers across bounded tail selection (Claude P1)

Validation

  • pnpm check:affected --run
  • 3,968 affected tests passed
  • changed-line coverage: 87.70%
  • package, layering, fallow, integration-progress, and replay-compat gates passed
  • adversarial claude -p review: confirmed P1 fixed; no remaining P0/P1

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.12 MB 2.13 MB +6.9 kB
JS gzip 691.3 kB 695.1 kB +3.9 kB
npm tarball 824.9 kB 826.8 kB +1.8 kB
npm unpacked 2.87 MB 2.88 MB +7.0 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 25.5 ms 25.7 ms +0.1 ms
CLI --help 62.2 ms 61.3 ms -0.9 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/internal/daemon.js -14.9 kB -5.0 kB
dist/src/session.js -11.1 kB -3.7 kB
dist/src/sdk-batch-runner.js +562 B +177 B
dist/src/src3.js +253 B +88 B
dist/src/screenshot-result.js -1 B +12 B

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-1702/

Built to branch gh-pages at 2026-08-10 15:08 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@thymikee

Copy link
Copy Markdown
Member Author

Review: changes requested

Reviewed at exact head 99f5af1b7bb8fa62977a6a88b7aed6b934c8a1f3.

  • P1 — iOS simulator recovery bypasses the request-scoped Apple tool provider. The prior route used runXcrun, which resolves the scoped Apple tool provider. The extracted implementation calls host.commands.run({ executable: "xcrun", ... }) from packages/platform-apple/src/network/runtime.ts; the host maps that directly to local runCmd, and withAppleToolProvider does not install a generic runCmd override. A scoped-provider recovery therefore silently becomes local xcrun, violating refactor(daemon): implement ADR 0019 through the adoption checkpoint #1696’s narrow-provider injection/fail-closed contract. Please add a neutral request-bound Apple-tool host operation and a scoped-provider production-route regression that proves local xcrun does not run, with red-before evidence.

  • P2 — Android optional logcat-recovery errors now escape. The previous captureAndroidLogcatWithAdb(...).catch(() => "") treated recovery as best-effort. recoverPackageTraffic now awaits the host command without a recovery-local fallback. A logcat rejection can therefore fail network dump even when the canonical log has valid entries. Preserve cancellation, but fall back for ordinary optional recovery failure/timeout; add a red-before regression using a valid canonical entry, inactive stream, and rejecting logcat.

The stack is not merge-ready independently: base #1701 remains blocked (stale-PID P1 plus size/evidence), and #1696’s checkpoint is revise, with #1704 owning the cumulative >3% package-budget decision. I also found no exact-head live network-recovery route proof. Please provide iOS-simulator and Android-emulator open → logs clear --restart → generate traffic → network dump evidence, including a scoped-provider or recovery case.

At review time the PR was mergeable; all observed checks except iOS Smoke were green, and iOS Smoke was pending. No label applied.

@thymikee
thymikee force-pushed the agent/platform-runtime-network branch from 99f5af1 to d73bdb4 Compare August 10, 2026 06:23
@thymikee

Copy link
Copy Markdown
Member Author

Addressed both correctness findings at exact head d73bdb4. Apple simulator recovery now goes through a required narrow runSimctl host port backed at invocation time by the request-scoped Apple provider; the planted old-path run failed because the generic command runner was still reached, and the production-route provider regression failed until the scoped path was restored. Android inactive recovery now treats only optional pidof/logcat transport failures as unavailable, preserves cancellation exactly, and leaves marker/artifact/canonical parsing failures fail-closed; the planted old path failed with adb transport unavailable while a canonical entry existed. The mandatory affected gate passed with 3,973 tests and 87.73% changed-line coverage. Exact-head public CLI verification also passed on emulator-5554 and iOS simulator 6044A251-23C2-4584-B7DB-87A16B787757: logs start, a deterministic GET canary, network dump with the expected app-log backend/URL/status, logs stop, and close on both targets. All 27 current checks are green, including iOS, Android, macOS, Linux, coverage, layering, and package checks.

@thymikee

Copy link
Copy Markdown
Member Author

Re-review at exact head d73bdb4: both findings are fixed and no new source defect was found. Apple simulator recovery now uses the narrow request-bound runSimctl port backed by scoped runXcrun; the provider-route regression would fail on the old generic command path. Android pidof/logcat recovery now falls back only for ordinary optional transport failure while preserving exact cancellation, with a real red-before canonical-entry regression. Exact-head iOS simulator and Android emulator network evidence is present; all 27 checks are green and the PR is CLEAN/mergeable. Residual stack blocker: #1704’s package-budget decision remains open and ADR 0019 stays revise, so no ready label is applied until the aggregate ≤+3% acceptance question is resolved.

@thymikee

Copy link
Copy Markdown
Member Author

Rebase expectation: parser placement + a known ratchet hole

When this PR rebases onto the revised #1701 head (e99da368), the network-traffic* parsers must land in @agent-device/capture-kit, not @agent-device/contracts (where they lived in the pre-revision stack). Contracts keeps only the network operation/outcome types and plan models, per the placement decision recorded on #1704.

Heads-up on why this needs saying explicitly: the new contracts-implementation ratchet bans filesystem/process/timer mechanics, but the network-traffic parsers are pure string processing and would pass that gate while re-bloating contracts. Either extend the policy to catch it (parser/LOC/byte ceiling on contracts modules) or treat this comment as the enforcement for this cutover and add the policy extension alongside.

Also carried over from the #1701 review: Apple simulator network recovery must keep using the request-scoped runSimctl port (the d73bdb4 fix), and the android/apple network runtimes stay hand-written — the ~70–80 shareable skeleton lines are not worth a factory.

@thymikee

Copy link
Copy Markdown
Member Author

Exact head d73bdb4 remains source-clean: the prior scoped Apple-provider and Android optional-recovery findings are fixed with valid regressions and live evidence. It is not ready because stacked base #1701 moved to e99da368 and GitHub now reports DIRTY/conflicting. The base rewrite touches the same Apple tooling/app-log contracts, so rebase and resolve, then rerun focused regressions, authoritative CI, and exact-head iOS/Android changed-route evidence before re-review. No ready label applied.

@thymikee
thymikee force-pushed the agent/platform-runtime-network branch from d73bdb4 to ae5d9e7 Compare August 10, 2026 12:47
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto #1701 at e99da3680; current #1702 head is ae5d9e737.

The rebase preserves the combined runtime cutover while applying the package-placement correction:

  • contracts retains only network types/plans;
  • the network traffic parser, Android enrichment, value parsing, and their tests live in private @agent-device/capture-kit;
  • platform/provider consumers import those mechanics from capture-kit;
  • the obsolete network-only AppleToolHost.runSimctl shape is gone; simulator recovery uses the shared closed AppleToolHost.run({ tool: 'simctl', ... });
  • provider runtime registrations remain separate from the public provider runtime object, preserving the refactor: extract platform log runtime #1701 declaration-surface split;
  • R11 and the contracts-implementation ratchet include the new package/dependency ownership.

Mandatory pre-push gate passed:

  • pnpm check:affected --run && git push --force-with-lease origin agent/platform-runtime-network
  • 796 test files / 6,409 tests passed
  • changed-line coverage: 941/1047 = 89.88%
  • changed-branch coverage: 488/685 = 71.24%
  • format, lint, typecheck, 130/130 layering policy tests + full layering scan, fallow, declarations/build, clean-install package verification, Node integration, provider integration, replay compatibility, and integration-progress all green.

CI has started for this exact head. The cumulative budget denominator remains the original 44c298d7f checkpoint baseline per #1704; the rebase onto merged devices must not reset that measurement.

@thymikee
thymikee force-pushed the agent/platform-runtime-network branch from ae5d9e7 to 8fd0d11 Compare August 10, 2026 13:07
@thymikee

Copy link
Copy Markdown
Member Author

Exact-head re-review at 8fd0d111: not ready. Both prior source findings are fixed: Apple simulator recovery now uses the request-bound scoped Apple-tool port with a provider-route regression, and Android optional pidof/logcat recovery falls back on ordinary transport errors while preserving the exact cancellation reason.

P2 remains: the contracts implementation ratchet bans only fs/process/timer mechanics. A pure network parser could move back into packages/contracts undetected, despite the rebase placement requirement. Add a parser/placement or bounded contracts-size rule, with a planted-red regression.

All current checks are green, but no exact-head iOS/Android changed-route live evidence exists after this rebase. Provide clean-daemon open → logs clear --restart → generate traffic → network dump → stop/close runs, with recovery actually activated on each applicable platform. The stacked #1701 base still lacks its exact-head logs-lifecycle evidence, and #1704 remains open with ADR 0019 checkpoint/package-budget unresolved. No ready label applied.

@thymikee
thymikee force-pushed the agent/platform-runtime-network branch from 8fd0d11 to 7b339cd Compare August 10, 2026 14:11
@thymikee
thymikee force-pushed the agent/platform-runtime-network branch from 7b339cd to 1bdd382 Compare August 10, 2026 14:30
@thymikee

Copy link
Copy Markdown
Member Author

Final stacked rebase is complete at 1bdd382, directly on the final #1701 head 1887953. #1701 itself is now rebased onto current main, including the merged #1699 squash and subsequent main commits.

The required pnpm check:affected --run && git push --force-with-lease chain passed before the branch update:

  • 799 test files / 6,424 tests green;
  • changed-line coverage 2,097 / 2,401 (87.34%);
  • format, lint, workspace/root/examples typecheck, layering, fallow, build/declarations, published-package clean install, Node integration, provider integration, replay compatibility, and integration-progress gates green.

The checkpoint denominator remains the original 44c298d baseline. Rebasing onto merged #1699/current main does not reset it; the final cumulative measurement is recorded in #1704 and will be copied into the rewritten #1703 ADR evidence.

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head re-review at 1bdd38236: not ready. The Apple scoped-provider and Android optional-recovery fixes survive the final rebase unchanged; their relevant route/test blobs match the prior reviewed 8fd0d111 head, and no new source-route defect was found.

P2 remains: the parser is physically in @agent-device/capture-kit, but the contracts implementation ratchet only rejects fs/process/timer mechanics. A pure network parser can drift back into packages/contracts undetected. Add an explicit parser/placement or bounded-contracts-size rule with a planted-red regression.

Practical evidence is stale: the only live network dump proof predates this final rebase, while the stacked base rewrote app-log lifecycle/rotation. Provide exact-head iOS-simulator and Android-emulator clean-daemon open → logs clear --restart → generate traffic → network dump → stop/close evidence, activating recovery where applicable. Coverage and iOS/Android Smoke are still in progress, and #1704 remains open while ADR 0019 still records a pending checkpoint. No ready label applied.

@thymikee

Copy link
Copy Markdown
Member Author

Addressed the remaining contracts-placement review item at 457fafe6399a95a4ddbfac57f02b3a7fe4157a54.

  • Added an OXC-AST ownership ratchet for the neutral network vocabulary in packages/contracts/src/network-traffic.ts: only type imports and type/interface exports are permitted there.
  • The gate rejects runtime parser declarations in that file and rejects sibling network-traffic-* implementation modules under contracts.
  • The parser implementation in @agent-device/capture-kit remains the allowed owner.
  • Planted cases cover both prohibited shapes; the focused policy suite is 3/3 green and full typecheck is green.

The branch is now the parent of the corrected checkpoint PR. Exact-head device evidence is tracked separately; this comment closes the concrete package-placement/code-review finding only.

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head live network evidence is now refreshed on 457fafe6399a95a4ddbfac57f02b3a7fe4157a54 for both local mobile platforms.

iOS simulator — iPhone 17 Pro 6044A251-23C2-4584-B7DB-87A16B787757, session pr1702-ios-final:

  • loaded the real Agent Device Tester surface, started the canonical app-log capture with logs clear --restart, opened the WebView accessibility lab, pressed Load Callstack, and observed Loaded Callstack;
  • placed adjacent canary GET https://www.callstack.com/ status=200 canary=pr1702-ios-final;
  • network dump 25 headers returned source via the canonical session app.log, backend ios-simulator, active: true, one entry with URL https://www.callstack.com/, status 200, and the absolute source line 22488;
  • after stopping/clearing the stream and relaunching, --debug network dump exercised simulator recovery: it reported 23 lines recovered through simctl log show and honestly returned zero HTTP entries because Unified Logging contained no request URL in that window.

Android emulator — Pixel 9 Pro XL emulator-5554, session pr1702-android-final:

  • loaded the exact-head bundle, started canonical logcat capture, opened the WebView lab, and pressed Load Callstack;
  • placed adjacent canary GET https://www.callstack.com/ status=200 canary=pr1702-android-final;
  • network dump 25 headers returned backend android, active: true, one entry with URL https://www.callstack.com/, status 200, and absolute line 1977.

The Android post-press wait text hit the known wait_capture_stalled shape (0 readable captures) even though the press itself succeeded; I did not convert that into claimed WebView-text evidence or change a timeout. The active runtime-owned dump itself succeeded. An inactive Android dump after clearing/relaunching returned an honest empty success; unlike iOS, it emitted no direct diagnostic proving a bounded recovery command, so I am not overclaiming that sub-route from this live run—its planted package tests remain the evidence for that branch.

Both sessions were closed, Metro was stopped, and the devices were left running.

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head re-review at 457fafe6: the parser-placement P2 is fixed. The R11 production scan now permits only type vocabulary in packages/contracts/src/network-traffic.ts and rejects runtime declarations there or a network-traffic-* parser sidecar; both planted-red cases are valid. No source finding in the runtime route delta. It remains not ready: exact-head iOS and Android logs clear --restart → generate traffic → network dump → stop/close evidence is still absent. The initial iOS Smoke failure was a runner alert main-thread timeout and a same-SHA retry passed, so it is a confirmed flake rather than a network defect, but the original check remains red. The stacked ADR checkpoint remains gated. No ready label applied.

@thymikee

Copy link
Copy Markdown
Member Author

CI follow-up at exact head 457fafe:

Neither failure touched the network runtime or the new contracts-implementation AST ratchet, and no production code or timeout budget was changed for the reruns. With the exact-head iOS/Android network evidence already attached at #1702 (comment), all current checks are green and there are no unresolved review threads.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 10, 2026
@thymikee
thymikee merged commit b15c502 into main Aug 10, 2026
32 of 34 checks passed
@thymikee
thymikee deleted the agent/platform-runtime-network branch August 10, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant