ci: avoid full Rust provisioning on cold browser runners - #389
Conversation
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
Head: aac8690f5be2d324d3571a07a20402def6b3d1a2
Base: 0c3a601bfee641d58bf3d8806730ffa398398464
One actionable public-material finding; no source-demonstrated functional defect in the three-file provisioning change.
P2 — Remove internal coordination and deployment identifiers from the public PR
The repository is public. The description’s Originating conversation reference publishes an internal channel/event locator. In addition, commit aac8690f exposes the internal deployment hostname and a stable agent identifier in its author/committer email fields and Signed-off-by trailer. These details are unnecessary for reviewing this CI change and remain publicly visible independently of the source patch.
Please remove the internal conversation reference and repair commit attribution with verified public-safe addresses, preserving truthful authorship and valid DCO certification. The DCO trailer exists and its check passed; that does not establish publication privacy. I have not repeated the sensitive strings here. This finding belongs in the review body because neither surface is a diff line.
Source assessment
.github/workflows/ci.yml:151–168selects the existing repository Rust pin, rejects missing/ambiguous/non-symlink pins, propagates installation failure, and verifies both compiler and Cargo executables before exporting the toolchain. It runs before the shared Hermit setup. The pinned activation action writeshermit env --rawinto the job environment; Hermit’s documented finalHERMIT_PREPEND_PATHoperation supports preserving this precedence through package subprocesses. The actual browser caller usescargothrough inherited PATH (tests/browser/conversation.spec.mjs:54–63;run-command.mjs:8–9). No second Rust-version constant was added.- Native formatting/Clippy/tests, all six functional shards, both engines, isolated measurements, timeouts, zero retries, and the fail-closed
CI requiredaggregate remain unchanged. Browser cases added/removed: 0/0. The single new Node integration case exercises the workflow’s actual shell using a stub installer and covers success, missing/ambiguous pins and install failure. It does not itself exercise real Hermit subprocess resolution or a cold hosted runner. - Error/cancellation handling was assessed separately: setup errors stop the job; unsuccessful/cancelled/skipped required lanes still fail aggregation. There is no changed user-interface success/cancel or error/retry focus path in this CI-only patch.
- Minimalness 9/10, elegance 9/10. Overall correctness/publication readiness 8/10 until the concrete public-material disclosure above is removed. No unrelated hardening requested.
Evidence and limits
One read-only snapshot of run 36510738150, associated with this head:
- All six browser jobs completed minimal Rust installation in 7–8s, Rust-cache setup in 2–4s, and fixture build in 17–21s. The previous base run’s WebKit 1/3 job spent 233s in Rust-cache setup and ultimately cancelled. These are observed job-step timings, not controlled cold-store before/after measurements or proof that the full cold-run budget is met.
- Rust/tool integration, browser measurements, DCO and security checks were successful; JavaScript and all six functional browser jobs were still running; Windows validation was skipped. No status polling.
- The measurement artifact reports 7/7 passed, 181.721s elapsed wall time, 170.103s summed test execution. Slowest test: Chromium cursor paging 85.747s; slowest file:
scroll.spec.mjs129.436s summed. Artifact state records clean GitHub merge checkout19c38eb946c400c00618051694a8c672ea0086e8, not the standalone PR head. Functional-shard final counts, summed execution and slowest-test/file evidence were not yet available in this snapshot; no claim of complete performance acceptance. A raw native-job log fetch was unavailable; its success is the hosted job status, not a locally verified test-count claim.
The current description/discussion contains no attached images or videos to inspect. Added workflow/test/doc lines were checked for additional internal-location disclosure; the concrete leaks are in the description and commit metadata above. Source review used 64 hashed Git-object extracts, not dirty checkout inputs. No PR code, tests, builds, installs, app launch or live workflow was executed locally. Full hosted completion and cold Ubuntu end-to-end acceptance remain unverified; the author’s macOS timing/probe claims are not Ubuntu acceptance. This non-blocking COMMENT is not approval or merge authorization.
Authored by Brain on behalf of Wes.
Summary
HERMIT_PREPEND_PATHbefore activation so pnpm/Node subprocesses keep the selected compiler. Native jobs retain their existing Hermit Rust, rustfmt, Clippy and complete tests.Why
The last green main run initialized Rust cache in 4 seconds. Main at 0c3a601b missed the Hermit cache after #361 changed
bin/hermit.hcl; WebKit 1/3 spent 233 seconds before cache restoration, exhausting its job budget. The pinned rust-cache action silently invokesrustc -vVfirst, triggering the full Hermit Rust installation.This fixes cold provisioning rather than adding retries, increasing timeouts or relying on a warm-cache rerun. It is independent of #384.
Validation
node --test tests/integration/browser-ci.test.mjs: 7 checks passed ataac8690f5be2d324d3571a07a20402def6b3d1a2; covers actual install shell, missing/ambiguous pins, install failure, exact shard discovery and fail-closed required gate.rustc -vV159s cold / 0.06s warm; minimal rustup install of the same pin 10s. This is not Ubuntu acceptance.bin/nodeandbin/pnpm exec nodesubprocess probes selected the isolated rustup Rust/Cargo 1.97.1 toolchain; command-path evidence is macOS, not Ubuntu.aac8690f; normal Ubuntu CI is running.aac8690f: blocker resolved; command-path design and failure guards reviewed.Originating conversation: buzz://message?channel=3428ec3f-a58b-429b-afbc-dc6a77918ce8&id=db263ce7de047b523873e93c6c788de75864a9a7069cd6bfbad3ed60478dcc8d