refactor: extract platform log runtime - #1701
Conversation
|
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
Review verdict: not ready at P1 — durable recovery leaves a stale marker that wedges Acceptance blocker — #1696 locks every bundle/package metric to ≤+3%. The exact same-runner size report is raw JS Evidence blocker — no exact-head live changed-route proof. The iOS/Android/macOS smoke jobs do not invoke No other source defect was confirmed in this review. |
|
Correctness and live-evidence follow-up at exact head 2d8ca8a: the stale owned-marker wedge is fixed by clearing the verified marker when cleanup-only reattachment proves the process missing. The planted test failed before the fix with stale marker still blocks replacement and is green after it. The mandatory affected gate passed 3,930 tests with 87.78% changed-line coverage, and every GitHub check is now green. Exact-head live proof also passed on Android emulator-5554 and iOS simulator 6044A251-23C2-4584-B7DB-87A16B787757: open, logs start, marker, app relaunch, second marker, logs stop, and close; both app.log files contained both markers plus relaunch output with the expected android and ios-simulator backends. The guarded Android takeover case verified daemon and log-child PID/start-time/command identities, killed only those two exact processes, observed startup recovery remove app-log.pid and terminalize app-log.resource.json, then successfully started, marked, stopped, and closed a replacement stream. The locked size threshold remains exceeded and is intentionally unresolved here; ADR 0019 remains revise and the packaging/budget decision is tracked by #1704. |
|
Re-review at exact head It remains not ready solely because #1696’s locked ≤+3% package-size condition is unresolved: raw is ~+3.40% and gzip ~+3.52%; #1704 is open and has not revised or satisfied that bound. No ready label applied. |
2d8ca8a to
14ce1ef
Compare
|
Follow-up adversarial review found two inherited tooling P1s and one gate defect, all fixed at 14ce1ef. Apple logs doctor/CoreDevice/start foreground probes now use the closed request-scoped |
|
Re-review at exact head This remains not ready for two reasons:
No ready label applied. |
Concrete shapes for the blocking findings — reuse-focusedFollow-up to the earlier review, after a focused audit of every app-log site at stack head 1. Canonical device identity →
|
14ce1ef to
e99da36
Compare
Revision package implemented at
|
| Metric | 44c298d7f |
e99da3680 |
Cumulative delta |
|---|---|---|---|
| JS raw | 2,036,067 | 2,120,117 | +84,050 (+4.128%) |
| JS gzip | 659,646 | 689,797 | +30,151 (+4.571%) |
| npm tarball | 797,027 | 823,385 | +26,358 (+3.307%) |
| npm unpacked | 2,781,186 | 2,866,441 | +85,255 (+3.065%) |
That is the honest cumulative checkpoint cost; the rebased per-PR report must not replace it. The accepted rationale is capture-pipeline reliability, durable cloud log streaming, and reusable durable-capture substrate—not a new observability layer.
Independent verification at
|
|
Exact-head re-review at Confirmed P2: daemon startup retains legacy app-log markers in Exact-head live logs evidence is also stale after the admission-ledger/identity/PID-runtime/lifecycle/teardown rewrite. Rerun Android recovery/replacement-start and iOS Finally, the cumulative checkpoint remains above the locked +3% bound in all four metrics: +4.128% raw, +4.571% gzip, +3.307% tarball, and +3.065% unpacked. #1704 remains open. Other targeted revision paths and all current checks are clean. No ready label applied. |
|
Exact-head re-review at Two readiness blockers remain: no exact-head Android recovery/replacement-start and iOS All checks are green; no ready label applied. |
|
Implemented and pushed the remaining exact-head logs correction at 6394ef4. What changed:
Exact-head Android live proof on emulator-5554 / com.callstack.agentdevicelab:
Crash-recovery / second-start proof:
Addendum bookkeeping: the temporary #1699 contracts implementations packages/contracts/src/android-adb-failure.ts and android-device.ts are absent at this #1701 head. Their resolved ownership is carried by this rebase; they are not intended to remain in contracts. The package-budget decision remains tracked separately in #1704 and must use the cumulative 44c298d baseline, not the post-#1699 moving base. |
6394ef4 to
1887953
Compare
|
Exact-head re-review at This remains not ready for two reasons. First, practical evidence is not exact-head complete: the Android rollover/recovery proof was captured at pre-rebase Second, #1704 now has reviewed same-host cumulative measurements and revised measured bounds, so the old “unreviewed +3% decision” wording is obsolete. But the checkpoint is not complete: #1704 remains open, ADR 0019 has not recorded the accepted bound, and checkpoint Status remains |
|
Final rebased-head live follow-up:
This refreshes the relaunch/stream-rollover evidence on the final rebased PR head. The earlier comment retains the detailed verified daemon-crash/dead-child recovery → completed manifest → successful second start proof. |
|
The architecture/budget portion of the remaining review is now resolved by the pushed checkpoint stack:
This follow-up closes the ADR/budget bookkeeping portion only. I am not relabeling the previously requested exact-head iOS live route as satisfied by unit or smoke coverage. |
|
Exact-head iOS simulator evidence is now complete on Target and controlled fixture:
Observed production route:
The Metro process was stopped afterward and the simulator was left running. This closes the requested exact-head iOS start → relaunch/PID rollover → output → stop evidence rather than relying on the generic native smoke job. |
|
Exact-head readiness update at The remaining blocker is HarmonyOS/HDC practical evidence. This head changes the shared Android/Harmony PID-scoped owner plus Harmony toolchain/cancellation/start behavior, but no live Harmony target has exercised it. On an available configured HDC target, provide |
Summary
Move the complete
logsdescriptor onto typed platform runtimes for all six families and Limrun while preserving the seven CLI plans.Add exact-owner durable handles, fencing, recovery, fail-closed provider transport composition, and remove daemon platform/tag/provider branches.
Stacked on #1699. Part of #1696.
Validation
pnpm check:affected --runpnpm check:layering