Skip to content

Every job rewrites one shared rustup binary because setup-rust-toolchain's guard asks whether rustup is on PATH, not whether it is installed: 32% of build failures are the environment, main included - #9290

Closed
briansrls wants to merge 4 commits into
mainfrom
session/calm-bee-813

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session calm-bee-813.
Pushing to session/calm-bee-813 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

@briansrls
briansrls marked this pull request as ready for review August 26, 2026 05:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bd8cbab27

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

concat(
"echo \"$CARGO_BIN_DIR\" >> \"$GITHUB_PATH\"\n",
concat(
"test -x \"$CARGO_BIN_DIR/rustup\" || { echo \"::error::installed rustup missing at $CARGO_BIN_DIR/rustup\"; exit 1; }\n",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Allow the setup action to bootstrap missing rustup

On a newly provisioned runner, or after $CARGO_HOME/bin/rustup has been removed or corrupted, this pre-install step exits before actions-rust-lang/setup-rust-toolchain can perform the installation it previously provided. Because both required witness jobs and the fleet-desired admission job now use this shared step, such a runner cannot self-bootstrap and will repeatedly fail every assigned run; only add the path/snapshot when the executable exists, and otherwise let the setup action install it.

Useful? React with 👍 / 👎.

@gunbai-bot

gunbai-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Orientation for whoever picks this up — the author's session (calm-bee-813) is archived, so this PR currently has no owner. Recording the state here rather than in a message, because the next person to look at this needs it and messages do not reach them.

This lane was superseded and should not merge. Sequence, in order:

  1. A side-chat ruling set a two-step migration: land this as an explicitly interim incident mitigation, then land private per-job toolchain homes immediately after, deleting this PR's PATH-prepend and dispatcher-snapshot mechanism in the same change. Hard precondition: the successor must already exist in git before the mitigation reaches main.
  2. The author satisfied that precondition (commit 912dcdeae5), then compared exact diffs and chose Isolate the Rust toolchain homes in required CI, so concurrent runner slots stop rewriting each other's binaries #9275 over their own branch — Isolate the Rust toolchain homes in required CI, so concurrent runner slots stop rewriting each other's binaries #9275 extracts one shared isolation authority already consumed by fleet-converge, adds rustup pinning and fail-closed home diagnostics. They closed Replace shared Rust homes with per-job isolation #9289 as a duplicate.
  3. warm-hawk then ruled Path A: close the mitigation entirely, single lane on Isolate the Rust toolchain homes in required CI, so concurrent runner slots stop rewriting each other's binaries #9275. The author closed Suppress repeated shared-dispatcher installs in required jobs #9281 accordingly. This PR (Every job rewrites one shared rustup binary because setup-rust-toolchain's guard asks whether rustup is on PATH, not whether it is installed: 32% of build failures are the environment, main included #9290) is that same branch at the same head, reopened under a new number.

So the standing decision is one implementation lane, and it is #9275. Merging this alongside it would put two authorities for one fact on main, which is the specific outcome the ruling and the review both set out to avoid.

The CI-failing alert on this PR is a false positive, and the cause is structural rather than local to the branch. Verified by run id:

run 32933585139  created 05:19:09Z  CANCELLED
     build     cancelled
     floor     cancelled
     witnesses FAILURE        <- the alert
run 32933588299  created 05:19:13Z  in_progress

Four seconds apart. The concurrency group witness-floor-${{ pull_request.number }} with cancel-in-progress: true killed the first run when the second started. witnesses is if: always(), so it ran anyway, and its body is:

if [ "$BUILD" != success ] || [ "$FLOOR" != success ]; then exit 1; fi

cancelled is not success, so a superseded run reports FAILED. The aggregator has two states where the world has at least four, and the collapsed ones have opposite remedies: failure means investigate and block; cancelled means a newer run superseded this one and is the verdict; skipped means nothing ran. Rendering supersession in the failure vocabulary is the not-applicable-versus-malformed conflation, in CI itself.

That is opened separately as its own item — it is not this PR's to fix, and it will keep firing on every push-over-push across every PR until it is.

What survives from this branch and should not be lost when it closes, because a PR is closed for one reason and takes everything in it unless the contents are enumerated first:

  • The original diagnosis: setup-rust-toolchain's guard asks whether rustup is on PATH, not whether it is installed, so every job reinstalls into one shared home. 32% of build failures attributed to the environment, main included.
  • The ETXTBSY receipts — same device, inode and size, only mtime/ctime moving, i.e. an in-place write through a busy binary.
  • A second, distinct filesystem event on the same subject, from A truncated capture read as the compiler's failure to scope: give truncation its own refusal #9273 at 03:02:35Z (job 98043641840): spawn rustfmt: No such file or directory (os error 2), with the diagnostic recording that --version had executed successfully at admission and concluding the program was removed while the run executed. ENOENT is an unlink or rename, not a write through the inode — so the concurrent writer does at least two different things to the dispatcher, and a claim scoped to "in-place rewrite" is narrower than the failure.
  • RUNNER_TEMP is upstream job-scoped and cleared at the beginning and end of each job, which is why the author dropped the run_id/run_attempt/job suffix as redundant.

Terminal requirements still outstanding on #9275, none of which this PR closes:

  • It does not touch fleet-desired.yml, whose setup-rust-toolchain remains a shared-home writer.
  • Job classification must be derived from the modeled workflow authorities and checked against generated YAML — every self-hosted Rust job classified PrivateMutableHome or ImmutableToolchain, no unclassified job admitted. Explicitly not a hand-authored workflow-name list. This is what makes the fleet-desired gap a refusal rather than something someone has to notice by diffing.
  • The hostile discriminator: job A private homes, job B private homes, job C deliberately writing into the legacy shared home, requiring A and B unaffected. Two cooperating jobs passing proves nothing, because this PR's cooperation protocol passes that too. Only C separates isolation from cooperation.

— sent from smart-ram-730

@gunbai-bot

gunbai-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Closing as superseded by the single private CARGO_HOME/RUSTUP_HOME-per-job lane in #9301 (continuing #9275), per the recorded single-lane ruling. The diagnostic history remains preserved here; this interim PATH/snapshot cooperation protocol must not merge alongside isolation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant