refactor: share daemon command and teardown budgets - #2307
Conversation
thymikee
changed the title
Size Report
Startup median (7 runs, lower is better):
|
|
Sentinel review at 5c4b003, independently reviewed. The budget arithmetic and shutdown behavior are preserved, with tests moved alongside their owners and recording reservation covered. One blocker: the new static import from daemon-client.ts to request-timeout.ts grows the CLI eager closure from 381 to 382 modules; Coverage run 33984965827 fails the enforced gate on that exact path. Reuse the shared resolver through a lazy boundary in the asynchronous client path (or another owning construction that preserves the closure), then prove the eager-closure gate and exact-head Coverage pass. Do not raise the baseline for this extraction. No live device evidence is required for the behavior-preserving prerequisite itself. No readiness label applied. |
|
Addressed the blocker from the Sentinel review in 327c2e8. The shared resolver now loads inside the already-async sendToDaemon path. Its synchronous API and timeout policy are unchanged; no baseline was raised. The CLI eager closure is back to 381 modules. The failure was reproduced against the CI comparison commit a90d95a before the fix. After the fix, all 418 exact-base closure checks and the focused timing/client/shutdown tests pass (465 tests total). The required affected checks also pass (805 tests). Exact-head Coverage is still running: https://github.com/callstack/agent-device/actions/runs/33986043196/job/101359591847. The requested CI proof remains pending; this is not yet a merge-ready claim. |
|
Exact-head Coverage is green for |
|
Sentinel re-review at 327c2e8: code clean, independently confirmed. Moving the shared timeout resolver import inside sendToDaemon resolves the eager-closure finding; Coverage passes. Merge readiness is still blocked by Android Smoke (automation-alert-result selector missing) and iOS Smoke (truncated AX capture cannot prove absence). These logs do not establish a regression from the import-only delta; require successful exact-head device lanes before merging. No ready-for-human label while checks fail. |
327c2e8 to
9a83035
Compare
|
Sentinel at 9a83035: coordinator + independent Terra/high code review clean. Request-envelope arithmetic and existing-session teardown budgets are preserved; the unit fixture now intercepts prewarm at its owning seam. Coverage and Integration pass. NOT merge-ready: iOS job https://github.com/callstack/agent-device/actions/runs/33991468607/job/101374350976 fails before device smoke: shallow base fetch makes the changed-path diff report no merge base, then installed bridge preparation times out xcrun --sdk iphonesimulator --show-sdk-version at 10s. This is outside the modified request-timeout path, not proof of a source regression or a proven host flake. Next action: obtain a green exact-head iOS run; workflow guard repair belongs to its owner. No readiness label. |
|
Follow-up to the current Sentinel review: one focused rerun of the failed iOS job on unchanged head Attempt 2 again reports no merge base in the changed-path diff. Installed snapshot bridge preparation then times out The local exact-head gate remains green. Downstream restacking continues from this published prerequisite without waiting for CI; merge readiness still requires the missing exact-head iOS evidence. |
9a83035 to
97f4e70
Compare
|
Rebased the complete timing layer onto requested main snapshot All four commits are unchanged by range-diff; no conflicts or additional timing/native changes were introduced. Independent adversarial re-review is READY. The fresh exact-head affected gate passed 3,227 tests across 429 files, and 482 focused checks passed, including all 418 eager-closure cases against that exact main snapshot. Prior red/green margin and fixture evidence remains preserved in the unchanged commits. Replacement-head CI and iOS are running. Android failed during scenario setup: its screenshot and snapshot show a “System UI isn't responding” dialog over the correctly opened Automation lab. The explicit 10-second wait expired; orientation cleanup was refused by the same system dialog, while session close succeeded. This did not exhaust the daemon request envelope. One focused retry was dispatched; the underlying ANR cause is unproven. The verified published prerequisite is already handed downstream; CI does not delay the remaining restack. No merge-readiness claim. |
|
Sentinel: clean coordinator + independent Terra/high review at 97f4e70. All four commits are range-diff equivalent after the rebase; shared command/teardown budget ownership and existing behavior remain intact. Exact-head runnable checks now pass, including iOS/Android smoke and coverage/integration, resolving the earlier CI blocker. Ready for human review/merge; land before dependent stack layers. No merge performed. |
thymikee
added
the
ready-for-human
|
2 participants
Summary
Shares request timeout calculation through the existing command policy module. The client resolves registry policy and calculates its envelope synchronously from neutral inputs, making the same calculation available to managed admission.
Ordinary teardown requires an existing session; managed recording reservation belongs to #2308. Arithmetic, cleanup behavior, and ordinary/provider behavior are preserved.
Twelve files: eleven within timing/teardown, plus one fixture correction in a separate commit. That fixture intercepts session prewarm before its deferred runner import. Tests move beside their owners and reuse session fixtures. No standalone docs or skills changed for this internal refactor.
Validation
Rebased onto requested main snapshot
80997b6bf1dda30ca57d74dc42036f6b447e16ad. All four commits are unchanged by range-diff from the previously reviewed head; there were no conflicts. Tested head:97f4e7057f3c4d6dead9df39a6a7645063c877e4.pnpm check:affected --runpassed: 3,227 tests / 429 files. The focused suite passed 482 tests, including all 418 eager-closure checks against the exact main snapshot. Host-wide gate serialization was used.Prior iOS attempts failed before device smoke in toolchain preparation. Replacement-head Coverage, Integration, and native checks will be rechecked against this main snapshot; its smoke fixes are not assumed to pass.