Skip to content

ci: install cargo-insta and cargo-nextest into the tend sandbox - #6251

Open
prql-bot wants to merge 3 commits into
mainfrom
daily/review-runs-33241784791
Open

ci: install cargo-insta and cargo-nextest into the tend sandbox#6251
prql-bot wants to merge 3 commits into
mainfrom
daily/review-runs-33241784791

Conversation

@prql-bot

@prql-bot prql-bot commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

.github/actions/tend-setup installed cargo-insta and cargo-nextest via baptiste0928/cargo-install, which puts them under the runner user's home — which tend deliberately keeps off the sandbox PATH, since that home can hold credentials. Since tend 0.1.20 the agent gets a stub that exits 127 and names the remedy: reinstall under the sandbox user's home via sandbox_setup:. This adds that block and drops the two now-dead cargo-install steps, so the tools move rather than being installed twice; both crates ship prebuilt binaries, so the pair downloads in ~2s rather than building.

This restores task prqlc:test, the inner loop CLAUDE.md documents. It does not restore task prqlc:pull-request: that reaches cargo llvm-cov (third command of prqlc:test-all) and pre-commit (first command of :lint), and unlike the two crates here neither is a blocked stub — both are absent from the runner's PATH too.

Verified from inside a tend session on this runner: starting from the blocked-stub state, the exact command block resolves cargo-nextest 0.9.143 and cargo-insta 1.48.0 in ~2s, both to ~/.local/bin rather than the ~/.tend-blocked/bin stub, with no dependence on the runner-home copies. task prqlc:test then runs to completion — cargo insta test --accept --test-runner=nextest over 704 tests, all passed, "no snapshots to review", doctests and clippy --fix clean, exit 0, working tree unchanged.

Two earlier attempts at this were closed, so please close this one too if you'd rather own it. #6144 symlinked into /usr/local/bin from the composite action — that reached past tend's supported lever. #6235 documented an export PATH workaround in the running-tend skill and was closed with "we need to fix the underlying problem, am working on it". This is neither: it is the residual repo-side config that the underlying fix expects. 0.1.20 landed here in #6242 and did fix the underlying problem — cargo, rustc and task all reach the sandbox now, where none of them did before. What is left is the class tend refuses on purpose: tools installed under the runner's home. sandbox_setup: is the first-class knob for exactly that case, and it is what the stub asks for by name.

Why the `cargo-install` steps go away

tend-setup is used only by the eight tend-*.yaml workflows, and no runner-side step in any of them invokes cargo or task — the agent is the only consumer of both crates. Keeping the cargo-install steps after this change would install them into a location nothing reads, and rebuild both from source on a cache miss in every tend job (Swatinem/rust-cache runs with cache-bin: "false"). The set_version.sh and cache steps around them are still doing real work and stay. baptiste0928/cargo-install remains in use across tests.yaml, release.yaml and others, so Dependabot still tracks it.

What the gap cost in last night's runs

Two hits in the nightly session (33238864253):

  1. Regenerating a book snapshot for #6248, cargo insta test --accept -p mdbook-prql --test documentation was unavailable, so the session fell back to INSTA_UPDATE=always INSTA_FORCE_UPDATE=1 cargo test. That is broader than --accept: it also rewrote documentation__book__project__target__version__0.snap and one other snapshot the change did not touch, which the session had to git checkout -- back by hand. It caught it; a session that did not would have shipped unrelated snapshot churn in a docs PR.
  2. The session then declined to file anything about the gap, correctly, on the strength of docs: record how to run the documented inner loop from a tend session #6235's close.

task prqlc:pull-request also aborted in that session, so #6248 and #6250 were opened without the pre-return gate having run — but as noted above, this PR does not fix that one.

This has been recorded in #6123 and its predecessors 15+ times since 2026-08-05, across roughly twelve review-runs windows.

Why ~/.local/bin and not the cargo default

The sandbox PATH is, in order: ~/.local/bin, ~/.tend-blocked/bin, /opt/hostedtoolcache/task/3.53.1/x64/bin, /opt/pipx_bin, ~/.cargo/bin, then system paths. ~/.local/bin is first, ahead of both ~/.tend-blocked/bin (where the 127 stubs live) and ~/.cargo/bin. Hence CARGO_HOME=~/.local on the cargo-dist installer, which places the binary at $CARGO_HOME/bin. The rationale is in the config comment at .config/tend.yaml so it is not re-derived.

Each command ends by running the tool, per tend's own guidance, so a broken install fails before the agent starts rather than silently reinstating today's degradation. Versions are unpinned; see the review thread for the one caveat there (a transient blip at either download URL fails the job for all eight workflows, and a bare curl --retry is the wrong remedy on a pipe).

`.github/actions/tend-setup` installs both under the runner user's home,
which tend keeps off the sandbox PATH; the agent gets a stub that exits
127. Reinstall them under the sandbox user's home with `sandbox_setup:`,
which is the remedy that stub names.

Both ship prebuilt binaries, so the pair downloads in ~1.2s. Verified
from inside a tend session: from the blocked-stub state the block
resolves both tools, and `task prqlc:test` then runs to completion —
704 tests passed, no snapshots to review, exit 0.
@prql-bot prql-bot added the review-runs Findings from tend-review-runs daily analysis label Aug 29, 2026

@prql-bot prql-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review — I authored this, so treating it with extra skepticism per the review skill. I re-ran the exact sandbox_setup: block from inside a live tend sandbox on this runner: it resolves cargo-nextest 0.9.143 and cargo-insta 1.48.0 in 1.4s, and both then resolve to ~/.local/bin rather than the ~/.tend-blocked/bin stub. I also confirmed the rendered block in all eight tend-*.yaml files is byte-identical to the joined .config/tend.yaml list, so regeneration is faithful. Three things I'd still change or flag.

The pull-request claim doesn't hold — inline suggestion below. task prqlc:pull-request runs prqlc:test-all, whose third command is cargo llvm-cov, then :lint, whose first command is pre-commit run --all-files. Neither cargo-llvm-cov nor pre-commit resolves in the sandbox — and unlike the two crates here, they aren't blocked stubs, they're absent from the runner's PATH too, so sandbox_setup: alone can't recover them. So the pre-return gate this PR's own evidence section says #6248/#6250 shipped without will still abort, just later and at a different command. That sentence is a durable comment a future session reads as ground truth, so it's worth narrowing rather than leaving it to be re-derived.

tend-setup now installs both crates for nobody. .github/actions/tend-setup/action.yaml still runs baptiste0928/cargo-install for cargo-insta and cargo-nextest, into the runner home. That action is used only by the eight tend workflows, and no runner-side step in any of them invokes cargo or task — the agent is the only consumer, and the runner home is exactly what the sandbox PATH excludes by design. After this PR those two steps install into a location nothing reads, and on a cache miss they build both crates from source in every tend job. Dropping them would make this change net-neutral in machinery instead of additive; the Swatinem/rust-cache and set_version.sh steps around them are still doing real work and should stay.

Minor: both installers are unpinned latest fetches behind bash -eo pipefail, so a transient blip at get.nexte.st or an upstream asset rename fails the whole job before the agent starts — on all eight workflows, including tend-notifications and tend-review-runs, which never touch Rust. Failing loudly is the right call over silently reinstating today's degradation, and unpinned does keep this in step with tend-setup's unpinned cargo-install; I mention it only because the blast radius is every tend session rather than just the ones that run tests. curl --retry 3 --retry-all-errors would cover the transient case without adding a version to maintain.

Comment thread .config/tend.yaml Outdated
Addresses self-review on #6251.

`tend-setup` installed cargo-insta and cargo-nextest with `cargo-install`
into the runner user's home. That action is used only by the eight tend
workflows, and no runner-side step in any of them invokes `cargo` or
`task` — the agent is the only consumer, and the runner home is exactly
what the sandbox PATH excludes by design. Now that `sandbox_setup:`
installs both under the sandbox home, those two steps install into a
location nothing reads, and build both crates from source on a cache
miss in every tend job.

Also narrows the config comment: `task prqlc:pull-request` reaches
`cargo llvm-cov` (via `prqlc:test-all`) and `pre-commit` (via `:lint`),
neither of which is on the sandbox PATH at all, so this change does not
restore that gate — only `task prqlc:test`.
@prql-bot

Copy link
Copy Markdown
Collaborator Author

Actioned the review as author. Two of the three are in 0369e91; the third I'm reporting rather than fixing, because the suggested remedy turns out to be unsafe.

1. pull-request claim — applied verbatim, replied inline. Verified against both Taskfiles and the live sandbox: cargo-llvm-cov and pre-commit are absent from the sandbox entirely, not stubbed.

2. tend-setup installing for nobody — agreed, and dropped both cargo-install steps. Verified the premise before removing: grep -rl tend-setup .github/ returns only the eight tend-*.yaml workflows (plus dependabot.yaml), and the only cargo/task invocations anywhere in those eight are inside the sandbox_setup: block this PR adds. I then re-ran that block in this session and both tools resolved to ~/.local/bin in 2.2s with the runner-home copies unused. baptiste0928/cargo-install is still used by tests.yaml, release.yaml, test-rust.yaml and others, so Dependabot keeps tracking it. This does make the PR net-neutral in machinery — the tools move rather than being installed in two places.

3. Unpinned installers — not changed, and curl --retry 3 --retry-all-errors would be the wrong fix here. curl restarts a retried transfer from byte 0, but bytes already written to a pipe can't be rewound, so a mid-stream failure feeds the consumer the partial body followed by the whole body. Both commands pipe straight into a consumer (tar zxf -, sh), so the retry converts a clean failure into corrupt input.

Repro for the retry hazard

Server returns Content-Length: 20, writes 5 bytes, closes; second request serves the full body:

$ curl -LsSf --retry 3 --retry-all-errors http://127.0.0.1:8731/x | od -c
curl: (18) transfer closed with 15 bytes remaining to read
0000000   A   A   A   A   A   B   B   B   B   B   B   B   B   B   B   B
0000020   B   B   B   B   B   B   B   B   B

25 bytes out of a 20-byte body. curl exits 0 — it did succeed, on the retry. tar would almost certainly reject that, but the | sh case is a shell script with a truncated prefix glued to a complete copy of itself.

The safe form is a download-to-tempfile then extract/run, which is four lines rather than one flag. Given no observed failure at either URL and a blast radius that is "job fails loudly before the agent starts" rather than a wrong outward action, I left it — the note is here so the next session doesn't reach for the one-flag version.

Nothing else from the review is left open. I'm polling the checks on 0369e91 and will follow up here only if they go red.

# Conflicts:
#	.github/workflows/tend-ci-fix.yaml
#	.github/workflows/tend-nightly.yaml
#	.github/workflows/tend-notifications.yaml
#	.github/workflows/tend-review-runs.yaml
#	.github/workflows/tend-review.yaml
#	.github/workflows/tend-triage.yaml
#	.github/workflows/tend-weekly.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-runs Findings from tend-review-runs daily analysis

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant