refactor: extract platform network runtime - #1702
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
24aa884 to
99f5af1
Compare
Review: changes requestedReviewed at exact head
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 At review time the PR was mergeable; all observed checks except iOS Smoke were green, and iOS Smoke was pending. No label applied. |
99f5af1 to
d73bdb4
Compare
|
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. |
|
Re-review at exact head |
Rebase expectation: parser placement + a known ratchet holeWhen this PR rebases onto the revised #1701 head ( 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 |
|
Exact head |
d73bdb4 to
ae5d9e7
Compare
|
Rebased onto #1701 at The rebase preserves the combined runtime cutover while applying the package-placement correction:
Mandatory pre-push gate passed:
CI has started for this exact head. The cumulative budget denominator remains the original |
ae5d9e7 to
8fd0d11
Compare
|
Exact-head re-review at P2 remains: the contracts implementation ratchet bans only fs/process/timer mechanics. A pure network parser could move back into All current checks are green, but no exact-head iOS/Android changed-route live evidence exists after this rebase. Provide clean-daemon open → |
8fd0d11 to
7b339cd
Compare
7b339cd to
1bdd382
Compare
|
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:
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. |
|
Exact-head re-review at P2 remains: the parser is physically in Practical evidence is stale: the only live |
|
Addressed the remaining contracts-placement review item at
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. |
|
Exact-head live network evidence is now refreshed on iOS simulator — iPhone 17 Pro
Android emulator — Pixel 9 Pro XL
The Android post-press Both sessions were closed, Metro was stopped, and the devices were left running. |
|
Exact-head re-review at |
|
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. |
Summary
Extract the
networkcommand into the combined platform runtime.networkDumpoperationdump|log × includecells through one runtime-backed daemon path and remove the legacy daemon parser/recovery modulesValidation
pnpm check:affected --runclaude -preview: confirmed P1 fixed; no remaining P0/P1