Skip to content

worker-protocol: add cancelled-and-hash-mismatch-status feature#15090

Open
amaanq wants to merge 1 commit intoNixOS:masterfrom
obsidiansystems:hash-mismatch-fix
Open

worker-protocol: add cancelled-and-hash-mismatch-status feature#15090
amaanq wants to merge 1 commit intoNixOS:masterfrom
obsidiansystems:hash-mismatch-fix

Conversation

@amaanq
Copy link
Member

@amaanq amaanq commented Jan 26, 2026

Motivation

This commit adds a cancelled-and-hash-mismatch-status protocol feature that gates two new BuildResult failure statuses: HashMismatch (a specific type of OutputRejected for hash mismatches) and Cancelled (for goals never attempted because another goal failed without --keep-going). Goals now default to Cancelled status instead of MiscFailure. When communicating with older remotes, HashMismatch falls back to OutputRejected and Cancelled falls back to MiscFailure.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority store Issues and pull requests concerning the Nix store c api Nix as a C library with a stable interface labels Jan 26, 2026
@amaanq amaanq force-pushed the hash-mismatch-fix branch 2 times, most recently from 7c26fed to 145c623 Compare January 29, 2026 01:24
@Ericson2314 Ericson2314 marked this pull request as ready for review January 29, 2026 14:36
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Please make a first PR that just introduces ExitStatusFlags (and does the things I mention below). Then we'll return to this one for the actual wire protocol changes using it.

@dpulls
Copy link

dpulls bot commented Jan 30, 2026

🎉 All dependencies have been resolved !

@Ericson2314 Ericson2314 force-pushed the hash-mismatch-fix branch 2 times, most recently from d2623b4 to 0c5cd3b Compare January 30, 2026 21:22
@amaanq amaanq force-pushed the hash-mismatch-fix branch from 0c5cd3b to 5956f56 Compare February 5, 2026 15:42
@amaanq amaanq force-pushed the hash-mismatch-fix branch 5 times, most recently from 556da11 to e4dbc59 Compare February 6, 2026 06:30
@amaanq amaanq force-pushed the hash-mismatch-fix branch 2 times, most recently from f48542e to 9b369ca Compare February 6, 2026 15:26
@amaanq amaanq force-pushed the hash-mismatch-fix branch 2 times, most recently from ce832f0 to 433911c Compare February 6, 2026 20:46
@Ericson2314
Copy link
Member

@amaanq amaanq force-pushed the hash-mismatch-fix branch 3 times, most recently from f2dfbe7 to b8b6a14 Compare February 6, 2026 21:24
@amaanq amaanq force-pushed the hash-mismatch-fix branch from b8b6a14 to 377a372 Compare February 6, 2026 21:34
* Build result.
*/
BuildResult buildResult;
BuildResult buildResult = {.inner = BuildResult::Failure{BuildResult::Failure::Cancelled, ""}};
Copy link
Member

Choose a reason for hiding this comment

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

N.B. tempted to just change the default status on BuildResult itself, rather than here.

@amaanq amaanq force-pushed the hash-mismatch-fix branch from 377a372 to e7bf1b1 Compare February 6, 2026 21:52
@Ericson2314 Ericson2314 requested a review from roberth February 6, 2026 21:52
@amaanq amaanq marked this pull request as ready for review February 6, 2026 21:53
Comment on lines +53 to +57
/// Goal was never attempted because another goal failed (and
/// `--keep-going` wasn't used). Requires the
/// `cancelled-and-hash-mismatch-status` feature; falls back to
/// `MiscFailure` when communicating with older remotes.
Cancelled,
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also handle Interrupted cases right?

Copy link
Member

@Ericson2314 Ericson2314 Feb 6, 2026

Choose a reason for hiding this comment

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

Honestly I am not sure, I asked @amaanq to roll in @edolstra's PR because I didn't want us to do two separate feature flags back to back.

Comment might just be wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added the comment based on Eelco's PR description, I don't think handling Interrupted would make sense since it's not a BuildResult status but rather its own error that occurs on SIGINT, SIGTERM, and SIGHUP.

@amaanq amaanq force-pushed the hash-mismatch-fix branch 3 times, most recently from 939aa89 to c948def Compare February 6, 2026 22:57
@amaanq amaanq force-pushed the hash-mismatch-fix branch 2 times, most recently from 0f6f3a6 to abd4ec8 Compare February 11, 2026 20:51
@amaanq amaanq changed the title worker-protocol: Add hash-mismatch-status feature for proper exit codes worker-protocol: add cancelled-and-hash-mismatch-status feature Feb 11, 2026
This commit adds a `cancelled-and-hash-mismatch-status` protocol feature that gates two new `BuildResult` failure statuses: `HashMismatch` (a specific type of `OutputRejected` for hash mismatches) and `Cancelled` (for goals never attempted because another goal failed without `--keep-going`). Goals now default to `Cancelled` status instead of `MiscFailure`. When communicating with older remotes, `HashMismatch` falls back to `OutputRejected` and `Cancelled` falls back to `MiscFailure`.

Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c api Nix as a C library with a stable interface new-cli Relating to the "nix" command store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants