Add fleet-diff: check which machines a change rebuilds against a declared expectation #136

Merged
vnprc merged 4 commits from agent/fleet-diff into master 2026-09-04 00:43:22 +01:00
Member

Adds fleet-diff, which answers whether an unmerged change alters any machine you actually run, and fails when the answer differs from what the change claimed.

No machine changes. This adds one script and its tests to a tools repo, and the script is not on any PATH yet, so nothing can invoke it even by accident; the two changes that install it are open separately and blocked on this. What proves it: 135 assertions, including the sabotage cases where the tool is made to lie and has to fail, plus runs against real git and real Nix. If it is wrong, the tool reports a fleet as unaffected when it is not — so treat a clean report as evidence, never as a release. Nothing here writes a lock or touches a machine.

Run it from a composition root, naming the revision you want tested:

cd ~/work/allod/deploy
fleet-diff --override archetypes/vm=1a2b3c4 --expect-none

For each machine in the flake it evaluates config.system.build.toplevel.drvPath twice — once against the committed lock, once with that revision substituted — and prints unchanged or CHANGES. With --expect or --expect-none it compares that set against what you declared, and a mismatch fails in both directions, naming the unexpected and the missing machines as two separate lists. Each machine is evaluated on its own, so a run costs one machine's evaluation at a time rather than the whole fleet's at once.

Exit status is distinct per failure so it composes as a preflight inside another command: 0 match or no expectation, 1 usage or precondition error, 2 expectation mismatch, 3 evaluation failed.

What the review changed

Only the revision is typed now. The first draft made a caller paste a fully-qualified flake URL. The composition root's own flake.lock already records which repository every input was fetched from, so the tool reads it there and the caller supplies the one thing only they know. A value that is not a bare revision is still passed through as a whole flake URL — one option covers both, told apart by shape rather than by a second flag — and that escape hatch now serves one narrow case: a source flake.lock cannot turn back into a git remote.

Abbreviations, with the collision caught rather than assumed. Seven characters or more, which is git's own floor for core.abbrev=auto; rev-parse will not disambiguate below four, and the Linux kernel uses twelve because it is enormous. The floor is only a sanity check. The guarantee is that an abbreviation is expanded against the remote's refs, so one matching no commit there is refused, and one matching more than one is refused with both hashes named. A length rule alone cannot do that. The consequence worth knowing: an abbreviation has to be pushed and be some ref's tip, because that listing is the only place the rest of the hash exists. A commit deeper in history takes its whole 40-character revision, which needs no expanding and so triggers no lookup.

The run prints what your revision resolved toarchetypes/vm=1a2b3c4 → git+…?rev=1a2b3c4d… (refs/heads/agent/split) — so the pinned commit is on the receipt rather than taken on trust.

A point measurement is gone, from flake/README.md and from the --help text that repeated it. The per-machine design is stated as the property it is, which survives different hardware and a moving codebase; the number did neither.

One claim I had to retract

I first composed ?ref=<branch>&rev=<hash> and justified it as necessary — Nix, I asserted, refuses a rev it cannot reach from the ref it was given. That is what the original long-form example did by hand, so it looked settled.

It is not true on Nix 2.31. A bare ?rev= reaches a commit that is not on the default branch. I checked it twice, because the first run could have been a warm fetcher cache: nix flake metadata 'git+https://forge.anarch.diy/allod/tools.git?rev=1608d3f…' succeeded, and ~/.cache/nix/gitv3 did hold that repository, so I re-ran it against a URL spelling Nix had never seen — a genuinely cold cache — and it succeeded there too.

So the tool composes only ?rev=. Pinning a ref as well would have added a second thing that has to stay true for no gain, and would have refused a commit deeper in history that works. The ref is still looked up when an abbreviation is expanded, but only to print where the commit was found.

What I found wrong or underspecified in the spec

Nix treats a mistyped --override-input as a warning and exits 0. Verified against Nix 2.31: --override-input mid/depp <url> on a flake whose input is mid/dep prints warning: the flag … does not match any input, evaluates the baseline, and succeeds. Left unchecked, one typo in an override path makes every machine report unchanged and --expect-none pass while proving nothing — the gate would not merely be weak, it would be inverted, and no output would say so. The tool therefore resolves every override path through flake.lock before evaluating anything, and before contacting any remote, so a typo costs no network round trip either. architecture.md principle 11 is exactly the rule that a check must be able to fail.

Two smaller gaps, both closed the same way: a checkout with no committed flake.lock has no baseline and is refused rather than letting Nix silently re-resolve one, and a fleet that evaluates to zero machines exits 3 rather than vacuously matching --expect-none. An uncommitted flake.nix or flake.lock warns on stderr, since the baseline is then the working tree — a warning, not an error, because testing a modified lock is legitimate.

Two smaller decisions worth naming. An --expect name that is not in the fleet is a usage error, checked before the expensive evaluation, so a typo fails fast and never reads as "expected to change, did not". A failed evaluation aborts rather than continuing, because the cause is nearly always systemic — a bad override URL or an unreachable rev — and repeating it across N machines costs minutes to say the same thing.

Report-only exits 0 even when machines change, as #135 proposed. Exit status should answer the question that was asked, and without an expectation no question was asked; the composable use always names one. The cost is that a gate which forgets --expect-none passes silently, so the tool prints No expectation declared — report only, nothing asserted. on every such run.

Also worth recording, since the brainstorm's line 69 says the opposite: this implements #135's contract, not the brainstorm's "non-zero exit when anything changes". The issue supersedes it deliberately and gives the reason.

Risk

R2. New tool, no existing behaviour touched, and nothing else in the repo imports it. It is read-only by construction: both evaluations pass --no-write-lock-file, and the fixture's lock was byte-identical after every run. It is not on PATH yet, so nothing can invoke it accidentally.

The residual risk is that the tool is trusted to answer a question it answers only partly. It sees the derivation, not the world — provisioning behaviour, secrets contents, and external volume state change with zero diff — and a run over allod/deploy covers the public example fleet only, so a green public result is not fleet-wide proof. The tool prints a scope line to that effect on every run rather than letting a clean report read as more than it is.

The parts with real logic are the flake.lock walk and the ls-remote expansion. The walk resolves node keys and follows arrays; the expansion parses a ref listing, including peeled annotated-tag entries. Both are now exercised against real data rather than only against stubs — see below.

A revision that is abbreviated but not pushed is refused, which is the intended reading of "the remote has never heard of this commit". A whole revision that is not pushed is not refused, and surfaces later as a Nix fetch failure and exit 3.

One bounded limit on pinning the revision alone. On a strict vanilla smart-HTTP remote with want-by-SHA disabled, a commit reachable only through an annotated tag cannot be fetched by hash: Server does not allow request for unadvertised object. That is a loud Nix error and exits 3, so it cannot make a gate silently pass, and every git-type input in the real allod/deploy lock points at Forgejo, which serves it. Branch tips and non-tip ancestors work even there.

Validation

tests/flake/fleet-diff.sh — 135 assertions, all passing. It stubs nix and git into a temp PATH the way tests/flake/flake-status.sh stubs git, so no real evaluation happens. The four sibling suites still pass unchanged (flake-status 21, and the three flake-update-cascade suites).

The required cases, with the sabotage ones demonstrated failing rather than asserted in prose:

  • no machine changes; one machine changes; --expect-none matches; --expect matches exactly
  • expectation misses a machine that changed--expect-none with one machine changing exits 2 and names it under changed, not expected; likewise --expect a when a and b both change
  • expectation names a machine that did not change — exits 2, named under expected, did not change
  • both directions at once, reported as two separate lists
  • a mistyped override path exits 1 instead of passing --expect-none, and evaluates nothing and asks no remote
  • an abbreviation naming two commits exits 1 with both hashes named, rather than pinning one
  • an abbreviation on no ref of the remote exits 1; a whole revision on no ref is accepted, since it needs no expanding
  • an unreachable remote exits 1, not 3, and passes git's own diagnostic through
  • an input whose lock entry is not a git source refuses a revision and names the type
  • an evaluation failure exits 3, distinct from 1 and 2; an empty fleet exits 3
  • per-machine sequential evaluation: exactly two evaluations per machine plus one enumeration, and nix flake check never invoked
  • argument handling: unknown option, missing --override, extra positional, missing directory, no flake.nix, no flake.lock, --override without =, empty value, sub-minimum revision, invalid input name, flags that consume nothing, empty --expect list, empty element, --expect with --expect-none, unknown machine, --, --flag=value, repeated --override, --help

Every new assertion was checked against a deliberately broken tool, because a test that cannot fail is not evidence. Thirteen mutations, each reverted: ambiguity check disabled, ref preference inverted, case-folding removed, peeled-tag handling removed, ref= dropped, the minimum-length floor removed, the whole-revision shortcut removed, multi-ref reporting reduced to the first, the lock-derived URL replaced by a fixed one, and the github flakeref form removed, plus the HEAD skip, the over-long-revision diagnostic, and the repeated-input guard. Each was caught by the assertion that guards it, and by that one first.

Three things a stub cannot catch, so they were run for real:

  1. Real git ls-remote parsing. A throwaway local repository with a branch, a master, and an annotated tag. Real git output, real awk. The branch tip resolved to (refs/heads/agent/split); the annotated tag's commit resolved through the peeled refs/tags/v1^{} entry, which is where that hash actually appears; an abbreviation the remote does not have was refused.
  2. The composed URL against real Nix. A root flake with that repository as a real input and a committed lock. Baseline evaluates to "one"; overriding with exactly the URL the tool composes evaluates to "two". That is the whole chain closed — lock to listing to flakeref to a different answer.
  3. The real allod/deploy lock. With nix stubbed so nothing was evaluated, but the lock and the remotes real: archetypes/vm resolved through the transitive path, secrets as a direct input, and nixpkgs through a follows chain to a github-locked node, which came back as github:NixOS/nixpkgs/b6018f87… rather than a git+ URL. All three URLs were derived from the lock, none typed.

A read-only adversarial review pass ran over this branch on a different model, rebuilding the rev-only claim from scratch against file://, git://, and a deliberately strict smart-HTTP server, and auditing the harness for vacuous assertions. It found the two swapped file modes, the over-long-revision diagnostic, the accepted duplicate override, and one mutation the suite could not see. All fixed; the thread records it in full.

The script passes shellcheck -s bash, and builds as a writeShellApplication with excludeShellChecks = [ "SC1091" ], which is how both PATH changes install it.

Not run: any real evaluation of allod/deploy itself. It is minutes of cold evaluation and network fetching for no additional signal about this tool.

Follow-up

Putting fleet-diff on PATH is open as allod/archetypes#68 (dev boxes) and allod/nexus#38 (host). Both are blocked on this merging and then on advancing the allod-tools input, which allod/archetypes holds for both — allod/nexus does not pin it, it receives it through extraSpecialArgs. That single lock advance is the integration change, and it is where Closes for those two issues belongs.

The limit recorded in this thread — that any input whose store path is embedded in a machine's configuration makes CHANGES unconditional, which is secrets for every credential-holding machine and allod-tools for every dev machine — is caused elsewhere and tracked elsewhere: allod/archetypes#21 for the secrets exposure, and allod/archetypes#66 for the mkDevVm token-argument type that lets the same leak back in. The tools-source reference at modules/agent-hooks.nix:11 still has no home; whether it folds into #21 or gets its own issue is an open decision.

Closes allod/tools#135

Adds `fleet-diff`, which answers whether an unmerged change alters any machine you actually run, and fails when the answer differs from what the change claimed. No machine changes. This adds one script and its tests to a tools repo, and the script is not on any PATH yet, so nothing can invoke it even by accident; the two changes that install it are open separately and blocked on this. What proves it: 135 assertions, including the sabotage cases where the tool is made to lie and has to fail, plus runs against real git and real Nix. If it is wrong, the tool reports a fleet as unaffected when it is not — so treat a clean report as evidence, never as a release. Nothing here writes a lock or touches a machine. Run it from a composition root, naming the revision you want tested: ```bash cd ~/work/allod/deploy fleet-diff --override archetypes/vm=1a2b3c4 --expect-none ``` For each machine in the flake it evaluates `config.system.build.toplevel.drvPath` twice — once against the committed lock, once with that revision substituted — and prints `unchanged` or `CHANGES`. With `--expect` or `--expect-none` it compares that set against what you declared, and a mismatch fails in both directions, naming the unexpected and the missing machines as two separate lists. Each machine is evaluated on its own, so a run costs one machine's evaluation at a time rather than the whole fleet's at once. Exit status is distinct per failure so it composes as a preflight inside another command: `0` match or no expectation, `1` usage or precondition error, `2` expectation mismatch, `3` evaluation failed. ## What the review changed **Only the revision is typed now.** The first draft made a caller paste a fully-qualified flake URL. The composition root's own `flake.lock` already records which repository every input was fetched from, so the tool reads it there and the caller supplies the one thing only they know. A value that is not a bare revision is still passed through as a whole flake URL — one option covers both, told apart by shape rather than by a second flag — and that escape hatch now serves one narrow case: a source `flake.lock` cannot turn back into a git remote. **Abbreviations, with the collision caught rather than assumed.** Seven characters or more, which is git's own floor for `core.abbrev=auto`; `rev-parse` will not disambiguate below four, and the Linux kernel uses twelve because it is enormous. The floor is only a sanity check. The guarantee is that an abbreviation is expanded against the remote's refs, so one matching no commit there is refused, and one matching more than one is refused with both hashes named. A length rule alone cannot do that. The consequence worth knowing: an abbreviation has to be pushed and be some ref's tip, because that listing is the only place the rest of the hash exists. A commit deeper in history takes its whole 40-character revision, which needs no expanding and so triggers no lookup. **The run prints what your revision resolved to** — `archetypes/vm=1a2b3c4 → git+…?rev=1a2b3c4d… (refs/heads/agent/split)` — so the pinned commit is on the receipt rather than taken on trust. **A point measurement is gone**, from `flake/README.md` and from the `--help` text that repeated it. The per-machine design is stated as the property it is, which survives different hardware and a moving codebase; the number did neither. ## One claim I had to retract I first composed `?ref=<branch>&rev=<hash>` and justified it as necessary — Nix, I asserted, refuses a rev it cannot reach from the ref it was given. That is what the original long-form example did by hand, so it looked settled. It is not true on Nix 2.31. A bare `?rev=` reaches a commit that is not on the default branch. I checked it twice, because the first run could have been a warm fetcher cache: `nix flake metadata 'git+https://forge.anarch.diy/allod/tools.git?rev=1608d3f…'` succeeded, and `~/.cache/nix/gitv3` did hold that repository, so I re-ran it against a URL spelling Nix had never seen — a genuinely cold cache — and it succeeded there too. So the tool composes only `?rev=`. Pinning a ref as well would have added a second thing that has to stay true for no gain, and would have refused a commit deeper in history that works. The ref is still looked up when an abbreviation is expanded, but only to print where the commit was found. ## What I found wrong or underspecified in the spec **Nix treats a mistyped `--override-input` as a warning and exits 0.** Verified against Nix 2.31: `--override-input mid/depp <url>` on a flake whose input is `mid/dep` prints `warning: the flag … does not match any input`, evaluates the *baseline*, and succeeds. Left unchecked, one typo in an override path makes every machine report `unchanged` and `--expect-none` pass while proving nothing — the gate would not merely be weak, it would be inverted, and no output would say so. The tool therefore resolves every override path through `flake.lock` before evaluating anything, and before contacting any remote, so a typo costs no network round trip either. `architecture.md` principle 11 is exactly the rule that a check must be able to fail. Two smaller gaps, both closed the same way: a checkout with no committed `flake.lock` has no baseline and is refused rather than letting Nix silently re-resolve one, and a fleet that evaluates to zero machines exits 3 rather than vacuously matching `--expect-none`. An uncommitted `flake.nix` or `flake.lock` warns on stderr, since the baseline is then the working tree — a warning, not an error, because testing a modified lock is legitimate. Two smaller decisions worth naming. An `--expect` name that is not in the fleet is a usage error, checked before the expensive evaluation, so a typo fails fast and never reads as "expected to change, did not". A failed evaluation aborts rather than continuing, because the cause is nearly always systemic — a bad override URL or an unreachable rev — and repeating it across N machines costs minutes to say the same thing. **Report-only exits 0 even when machines change**, as `#135` proposed. Exit status should answer the question that was asked, and without an expectation no question was asked; the composable use always names one. The cost is that a gate which forgets `--expect-none` passes silently, so the tool prints `No expectation declared — report only, nothing asserted.` on every such run. Also worth recording, since the brainstorm's line 69 says the opposite: this implements `#135`'s contract, not the brainstorm's "non-zero exit when anything changes". The issue supersedes it deliberately and gives the reason. ## Risk R2. New tool, no existing behaviour touched, and nothing else in the repo imports it. It is read-only by construction: both evaluations pass `--no-write-lock-file`, and the fixture's lock was byte-identical after every run. It is not on PATH yet, so nothing can invoke it accidentally. The residual risk is that the tool is trusted to answer a question it answers only partly. It sees the derivation, not the world — provisioning behaviour, secrets contents, and external volume state change with zero diff — and a run over `allod/deploy` covers the public example fleet only, so a green public result is not fleet-wide proof. The tool prints a scope line to that effect on every run rather than letting a clean report read as more than it is. The parts with real logic are the `flake.lock` walk and the `ls-remote` expansion. The walk resolves node keys and `follows` arrays; the expansion parses a ref listing, including peeled annotated-tag entries. Both are now exercised against real data rather than only against stubs — see below. A revision that is abbreviated but not pushed is refused, which is the intended reading of "the remote has never heard of this commit". A whole revision that is not pushed is not refused, and surfaces later as a Nix fetch failure and exit 3. One bounded limit on pinning the revision alone. On a strict vanilla smart-HTTP remote with want-by-SHA disabled, a commit reachable only through an annotated tag cannot be fetched by hash: `Server does not allow request for unadvertised object`. That is a loud Nix error and exits 3, so it cannot make a gate silently pass, and every git-type input in the real `allod/deploy` lock points at Forgejo, which serves it. Branch tips and non-tip ancestors work even there. ## Validation `tests/flake/fleet-diff.sh` — 135 assertions, all passing. It stubs `nix` and `git` into a temp `PATH` the way `tests/flake/flake-status.sh` stubs `git`, so no real evaluation happens. The four sibling suites still pass unchanged (`flake-status` 21, and the three `flake-update-cascade` suites). The required cases, with the sabotage ones demonstrated failing rather than asserted in prose: - no machine changes; one machine changes; `--expect-none` matches; `--expect` matches exactly - **expectation misses a machine that changed** — `--expect-none` with one machine changing exits 2 and names it under `changed, not expected`; likewise `--expect a` when `a` and `b` both change - **expectation names a machine that did not change** — exits 2, named under `expected, did not change` - both directions at once, reported as two separate lists - a mistyped override path exits 1 instead of passing `--expect-none`, and evaluates nothing and asks no remote - **an abbreviation naming two commits** exits 1 with both hashes named, rather than pinning one - an abbreviation on no ref of the remote exits 1; a whole revision on no ref is accepted, since it needs no expanding - an unreachable remote exits 1, not 3, and passes git's own diagnostic through - an input whose lock entry is not a git source refuses a revision and names the type - an evaluation failure exits 3, distinct from 1 and 2; an empty fleet exits 3 - per-machine sequential evaluation: exactly two evaluations per machine plus one enumeration, and `nix flake check` never invoked - argument handling: unknown option, missing `--override`, extra positional, missing directory, no `flake.nix`, no `flake.lock`, `--override` without `=`, empty value, sub-minimum revision, invalid input name, flags that consume nothing, empty `--expect` list, empty element, `--expect` with `--expect-none`, unknown machine, `--`, `--flag=value`, repeated `--override`, `--help` **Every new assertion was checked against a deliberately broken tool**, because a test that cannot fail is not evidence. Thirteen mutations, each reverted: ambiguity check disabled, ref preference inverted, case-folding removed, peeled-tag handling removed, `ref=` dropped, the minimum-length floor removed, the whole-revision shortcut removed, multi-ref reporting reduced to the first, the lock-derived URL replaced by a fixed one, and the github flakeref form removed, plus the HEAD skip, the over-long-revision diagnostic, and the repeated-input guard. Each was caught by the assertion that guards it, and by that one first. Three things a stub cannot catch, so they were run for real: 1. **Real `git ls-remote` parsing.** A throwaway local repository with a branch, a master, and an annotated tag. Real git output, real awk. The branch tip resolved to `(refs/heads/agent/split)`; the annotated tag's commit resolved through the peeled `refs/tags/v1^{}` entry, which is where that hash actually appears; an abbreviation the remote does not have was refused. 2. **The composed URL against real Nix.** A root flake with that repository as a real input and a committed lock. Baseline evaluates to `"one"`; overriding with exactly the URL the tool composes evaluates to `"two"`. That is the whole chain closed — lock to listing to flakeref to a different answer. 3. **The real `allod/deploy` lock.** With `nix` stubbed so nothing was evaluated, but the lock and the remotes real: `archetypes/vm` resolved through the transitive path, `secrets` as a direct input, and `nixpkgs` through a `follows` chain to a github-locked node, which came back as `github:NixOS/nixpkgs/b6018f87…` rather than a `git+` URL. All three URLs were derived from the lock, none typed. A read-only adversarial review pass ran over this branch on a different model, rebuilding the rev-only claim from scratch against `file://`, `git://`, and a deliberately strict smart-HTTP server, and auditing the harness for vacuous assertions. It found the two swapped file modes, the over-long-revision diagnostic, the accepted duplicate override, and one mutation the suite could not see. All fixed; the thread records it in full. The script passes `shellcheck -s bash`, and builds as a `writeShellApplication` with `excludeShellChecks = [ "SC1091" ]`, which is how both PATH changes install it. Not run: any real evaluation of `allod/deploy` itself. It is minutes of cold evaluation and network fetching for no additional signal about this tool. ## Follow-up Putting `fleet-diff` on PATH is open as allod/archetypes#68 (dev boxes) and allod/nexus#38 (host). Both are blocked on this merging and then on advancing the `allod-tools` input, which `allod/archetypes` holds for both — `allod/nexus` does not pin it, it receives it through `extraSpecialArgs`. That single lock advance is the integration change, and it is where `Closes` for those two issues belongs. The limit recorded in this thread — that any input whose store path is embedded in a machine's configuration makes `CHANGES` unconditional, which is `secrets` for every credential-holding machine and `allod-tools` for every dev machine — is caused elsewhere and tracked elsewhere: allod/archetypes#21 for the secrets exposure, and allod/archetypes#66 for the `mkDevVm` token-argument type that lets the same leak back in. The tools-source reference at `modules/agent-hooks.nix:11` still has no home; whether it folds into #21 or gets its own issue is an open decision. Closes allod/tools#135
For each machine in a composition-root flake, evaluate the committed lock
against the same lock plus the change's revisions and report unchanged or
CHANGES, then compare that set against a declared expectation. Mismatch
fails in both directions and names the unexpected and the missing sets
separately.

Evaluation is per-machine and sequential: nix flake check over a whole
composition root peaks near 7 GiB and gets OOM-killed on an 8 GiB VM.

Exit status is distinct per failure so the tool composes as a preflight:
1 usage, 2 expectation mismatch, 3 evaluation failure. Report-only mode,
with neither expectation flag, exits 0 even when machines change.

An override path that names no input in flake.lock is refused. Nix answers
a mistyped --override-input with a warning and evaluates the baseline
anyway, exit 0, so an unchecked typo would report every machine unchanged
and pass --expect-none while proving nothing.

Refs allod/tools#135

Model: claude-opus-5
The previous commit carried only the documentation: record stages tracked
files, so the two new files were skipped.

Refs allod/tools#135

Model: claude-opus-5
Author
Member

The criticism applies, and I reproduced it with your tool rather than reasoning about it. For changes to the secrets input the gate always fires, so it cannot tell a real effect from a no-op — which is the one failure mode that makes a gate worth ignoring. This is not a defect in the tool's logic; the tool is reporting the truth. It is a limit worth naming in the output, because a reader cannot tell it from a real result.

Run against allod/deploy with the open allod/secrets PR #11 substituted:

allod-dev              CHANGES
installer              unchanged
nexus                  unchanged
privacy-1              unchanged
1 of 4 machines change.

That change is semantically inert. It nulls agentTokenFile for machines that set forgeAccess = false, and no machine sets it — the review of that PR verified allod-dev's generated age.secrets are byte-identical between secrets master and the branch. The machine does not change. Its derivation does.

The cause is that a machine holding an encrypted secret embeds the secrets input's own store path through age.secrets.<name>.file. Any commit to that repo produces a new store path, so every machine holding a secret gets a new derivation. A comment-only commit does it; that was verified independently during the archetypes review with two copies differing by one comment line, which shifted allod-dev's toplevel from 71appaya… to kzrdwxf2….

Why this matters more than an ordinary caveat: the tool's value comes from CHANGES being rare and therefore meaningful. For a whole class of changes it is unconditional, so --expect allod-dev becomes a ritual the author writes without reading, and the next real change to that machine is hidden inside an expectation that was already there. That is the rubber-stamping failure your own PR body worries about, arriving through the input rather than through the human.

It generalises past secrets: any input whose store path is referenced from a machine's configuration behaves this way. secrets is simply the one that does it for every machine that holds a credential.

Two ways out, and I do not think the choice is obvious.

Normalise before comparing — compare the two derivations with references to the overridden input's store path elided, so only genuine differences survive. It makes the gate mean the same thing for every input, at the cost of real complexity in the one part of the tool that has logic, and of a way to mask a change that genuinely is only a store path.

Or name it and let the human carry it — detect that an override names an input whose path is embedded in the fleet's configurations, and print that the result for that input cannot distinguish a real change from a no-op. Cheap, honest, and it keeps the tool reporting what Nix says. The cost is that the expectation for a secrets change is always the full list of credential-holding machines, which is exactly the ritual above, only labelled.

Either way it wants to be visible in the output. The tool already prints a scope line and a report-only line for the same reason: the reader cannot see what the tool cannot see.

For context on where this came from: it was found while reviewing allod/archetypes#17, and recorded against the brainstorm as allod/strategy#25 before this tool existed. If the brainstorm is superseded by the tool, that finding should move here rather than be dropped with it.

The criticism applies, and I reproduced it with your tool rather than reasoning about it. For changes to the `secrets` input the gate always fires, so it cannot tell a real effect from a no-op — which is the one failure mode that makes a gate worth ignoring. This is not a defect in the tool's logic; the tool is reporting the truth. It is a limit worth naming in the output, because a reader cannot tell it from a real result. Run against `allod/deploy` with the open `allod/secrets` PR #11 substituted: ``` allod-dev CHANGES installer unchanged nexus unchanged privacy-1 unchanged 1 of 4 machines change. ``` That change is semantically inert. It nulls `agentTokenFile` for machines that set `forgeAccess = false`, and no machine sets it — the review of that PR verified `allod-dev`'s generated `age.secrets` are byte-identical between `secrets` master and the branch. The machine does not change. Its derivation does. The cause is that a machine holding an encrypted secret embeds the `secrets` input's own store path through `age.secrets.<name>.file`. Any commit to that repo produces a new store path, so every machine holding a secret gets a new derivation. A comment-only commit does it; that was verified independently during the archetypes review with two copies differing by one comment line, which shifted `allod-dev`'s toplevel from `71appaya…` to `kzrdwxf2…`. Why this matters more than an ordinary caveat: the tool's value comes from `CHANGES` being rare and therefore meaningful. For a whole class of changes it is unconditional, so `--expect allod-dev` becomes a ritual the author writes without reading, and the next real change to that machine is hidden inside an expectation that was already there. That is the rubber-stamping failure your own PR body worries about, arriving through the input rather than through the human. It generalises past `secrets`: any input whose store path is referenced from a machine's configuration behaves this way. `secrets` is simply the one that does it for every machine that holds a credential. Two ways out, and I do not think the choice is obvious. Normalise before comparing — compare the two derivations with references to the overridden input's store path elided, so only genuine differences survive. It makes the gate mean the same thing for every input, at the cost of real complexity in the one part of the tool that has logic, and of a way to mask a change that genuinely is only a store path. Or name it and let the human carry it — detect that an override names an input whose path is embedded in the fleet's configurations, and print that the result for that input cannot distinguish a real change from a no-op. Cheap, honest, and it keeps the tool reporting what Nix says. The cost is that the expectation for a secrets change is always the full list of credential-holding machines, which is exactly the ritual above, only labelled. Either way it wants to be visible in the output. The tool already prints a scope line and a report-only line for the same reason: the reader cannot see what the tool cannot see. For context on where this came from: it was found while reviewing allod/archetypes#17, and recorded against the brainstorm as allod/strategy#25 before this tool existed. If the brainstorm is superseded by the tool, that finding should move here rather than be dropped with it.
Owner

if we're not comparing hashes then i don't think the juice is worth the squeeze. we should reevaluate whether this tool is worth building.

For changes to the secrets input the gate always fires, so it cannot tell a real effect from a no-op

is this a code smell or a valid architectural decision? will it happen often enough to make this check fire too often?

if we're not comparing hashes then i don't think the juice is worth the squeeze. we should reevaluate whether this tool is worth building. > For changes to the secrets input the gate always fires, so it cannot tell a real effect from a no-op is this a code smell or a valid architectural decision? will it happen often enough to make this check fire too often?
Author
Member

Code smell, and yes — about once a working day. But I don't think either answer means the tool isn't worth building, because there is a third option neither of us named: keep comparing hashes exactly as the tool does now, and fix the input that makes the hash lie. I verified that fix works end to end before writing this.

Code smell, and a locatable one

archetypes/modules/github-credentials.nix:29 reads file = "${secrets}/${consumer.secret}". That interpolates the whole secrets repo's store path, so every machine holding any credential depends on every byte of that repo — including a comment in an unrelated .nix file. Nothing about agenix requires this; it needs a readable path to the encrypted blob and nothing more.

Importing each secret as its own content-addressed path makes the dependency what it should have been all along: this machine depends on the contents of the secrets it actually holds. Verified through a real flake input under pure evaluation, two revisions differing only in an unrelated file:

construction revision 1 revision 2 (only an unrelated file differs)
"${secrets}/x.age" — current gb7f0ghs…-source/x.age a7bbli5r…-source/x.age
builtins.path per file 2jsyhl4j…-x.age 2jsyhl4j…-x.age

Rotation still propagates — changing the blob itself moves it to 71x7hjsk…-x.age. So the change drops exactly the noise and keeps exactly the signal.

Frequency: about once a working day

allod/secrets has 27 commits since 26 June. For the merge gate that means every secrets PR. For the deploy lock advance — the gate that actually stands between you and a rebuild — secrets will essentially always have moved since the last advance, so it would fire every time, always with the full list of credential-holding machines. That is precisely the frequency at which a gate becomes something a person scrolls past, so I read the instinct behind the question as right rather than impatient.

On "if we're not comparing hashes"

Agreed, and I would go further: normalising the comparison is the worst of the three options. It puts real logic into the one part of the tool that has any, and the power to mask a difference that is only a store path is the same power as masking a real one — the tool cannot tell them apart either, which is the entire problem. Naming the limit in the output is honest but leaves the ritual in place. Only fixing the input makes CHANGES mean the same thing for every input.

What the fix costs

Not a one-liner. The interpolated shape is asserted on elsewhere: archetypes/flake.nix:716 builds expected="${secrets}/secrets/vm-host-keys/''${vm}-ssh.age" and fails when the real path differs, and vmFacts.<vm>.hostKeySecretFile publishes that shape downstream. So it is a focused change in archetypes — the path constructions plus the checks asserting their shape — and it moves every machine's derivation once, deliberately. It deserves its own issue and its own risk call rather than being folded in here.

Suggestion

Merge this PR as it stands. Its logic does not change under the fix, and what it reports today is true. Then take the archetypes change as the actual remedy, and let this tool be what verifies it: the first honest use of --expect naming exactly the credential-holding machines, once, on purpose.

I would skip the "name the limit in the output" option entirely. If the fix lands it is code written to be deleted, and if it does not land, we will have learned the tool is not trusted anyway.

The allod/strategy#25 finding should move onto the archetypes issue rather than onto this one — the limit belongs recorded with the thing that causes it, not with the thing that observes it.

Code smell, and yes — about once a working day. But I don't think either answer means the tool isn't worth building, because there is a third option neither of us named: keep comparing hashes exactly as the tool does now, and fix the input that makes the hash lie. I verified that fix works end to end before writing this. ## Code smell, and a locatable one `archetypes/modules/github-credentials.nix:29` reads `file = "${secrets}/${consumer.secret}"`. That interpolates the whole secrets repo's store path, so every machine holding any credential depends on every byte of that repo — including a comment in an unrelated `.nix` file. Nothing about agenix requires this; it needs a readable path to the encrypted blob and nothing more. Importing each secret as its own content-addressed path makes the dependency what it should have been all along: this machine depends on the contents of the secrets it actually holds. Verified through a real flake input under pure evaluation, two revisions differing only in an unrelated file: | construction | revision 1 | revision 2 (only an unrelated file differs) | | --- | --- | --- | | `"${secrets}/x.age"` — current | `gb7f0ghs…-source/x.age` | `a7bbli5r…-source/x.age` | | `builtins.path` per file | `2jsyhl4j…-x.age` | `2jsyhl4j…-x.age` | Rotation still propagates — changing the blob itself moves it to `71x7hjsk…-x.age`. So the change drops exactly the noise and keeps exactly the signal. ## Frequency: about once a working day `allod/secrets` has 27 commits since 26 June. For the merge gate that means every secrets PR. For the deploy lock advance — the gate that actually stands between you and a rebuild — secrets will essentially always have moved since the last advance, so it would fire every time, always with the full list of credential-holding machines. That is precisely the frequency at which a gate becomes something a person scrolls past, so I read the instinct behind the question as right rather than impatient. ## On "if we're not comparing hashes" Agreed, and I would go further: normalising the comparison is the worst of the three options. It puts real logic into the one part of the tool that has any, and the power to mask a difference that is only a store path is the same power as masking a real one — the tool cannot tell them apart either, which is the entire problem. Naming the limit in the output is honest but leaves the ritual in place. Only fixing the input makes `CHANGES` mean the same thing for every input. ## What the fix costs Not a one-liner. The interpolated shape is asserted on elsewhere: `archetypes/flake.nix:716` builds `expected="${secrets}/secrets/vm-host-keys/''${vm}-ssh.age"` and fails when the real path differs, and `vmFacts.<vm>.hostKeySecretFile` publishes that shape downstream. So it is a focused change in `archetypes` — the path constructions plus the checks asserting their shape — and it moves every machine's derivation once, deliberately. It deserves its own issue and its own risk call rather than being folded in here. ## Suggestion Merge this PR as it stands. Its logic does not change under the fix, and what it reports today is true. Then take the archetypes change as the actual remedy, and let this tool be what verifies it: the first honest use of `--expect` naming exactly the credential-holding machines, once, on purpose. I would skip the "name the limit in the output" option entirely. If the fix lands it is code written to be deleted, and if it does not land, we will have learned the tool is not trusted anyway. The `allod/strategy#25` finding should move onto the archetypes issue rather than onto this one — the limit belongs recorded with the thing that causes it, not with the thing that observes it.
Author
Member

Correcting my previous comment: the fix already has an issue, and I understated the problem.

allod/archetypes#21 covers it and is better scoped than what I proposed. It names the same construction and the same remedy — builtins.path { path = "${gitPolicySource}/git/${name}"; name = name; }, recorded there as verified — and it names a second site I missed, modules/agent-hooks.nix:14,16,18,20, alongside modules/github-credentials.nix:29. It also has the motivation the right way round: the primary harm is that every machine carries every ciphertext in the deployment, so one guest compromise walks off with the lot to attack offline. The rebuild noise this PR cares about is its third goal, not its reason.

Two corrections to what I wrote above.

I said the fix would need archetypes/flake.nix:716 updated because it asserts on the interpolated path shape. allod/archetypes#21 deliberately scopes the check-derivation references at flake.nix:717 and :723 out — a check built on demand that inspects the repo is supposed to see the repo, and it never lands on a machine. So the cost is lower than I made it sound, and confined to the two consuming modules.

I also said "it deserves its own issue," which was wrong — it has one, open since before this PR.

The correction that goes the other way: the noise is not secrets-specific. modules/agent-hooks.nix:11 interpolates "${allod-tools}/git-hooks/protected-refs-policy", the same construction against the tools input, and allod/tools has 39 commits since 26 June against secrets' 27. So every commit to the tools repo also moves every dev machine's derivation. That makes the answer to "will it fire too often" worse than the figure I gave, and it means allod/archetypes#21 alone does not fully quiet the gate, since its scope is the secrets exposure rather than the tools reference. Worth deciding whether the tools site is folded into that issue or tracked separately.

One loose end for the record: allod/strategy#25, which recorded this limit in the brainstorm, was closed without merging, so the text never landed and the finding currently lives only in that closed PR and in this thread. allod/archetypes#21 is the durable home for the cause; if the observed limit is worth recording anywhere, this thread is now the better place for it than the brainstorm.

Correcting my previous comment: the fix already has an issue, and I understated the problem. `allod/archetypes#21` covers it and is better scoped than what I proposed. It names the same construction and the same remedy — `builtins.path { path = "${gitPolicySource}/git/${name}"; name = name; }`, recorded there as verified — and it names a second site I missed, `modules/agent-hooks.nix:14,16,18,20`, alongside `modules/github-credentials.nix:29`. It also has the motivation the right way round: the primary harm is that every machine carries every ciphertext in the deployment, so one guest compromise walks off with the lot to attack offline. The rebuild noise this PR cares about is its third goal, not its reason. Two corrections to what I wrote above. I said the fix would need `archetypes/flake.nix:716` updated because it asserts on the interpolated path shape. `allod/archetypes#21` deliberately scopes the check-derivation references at `flake.nix:717` and `:723` out — a check built on demand that inspects the repo is supposed to see the repo, and it never lands on a machine. So the cost is lower than I made it sound, and confined to the two consuming modules. I also said "it deserves its own issue," which was wrong — it has one, open since before this PR. The correction that goes the other way: the noise is not secrets-specific. `modules/agent-hooks.nix:11` interpolates `"${allod-tools}/git-hooks/protected-refs-policy"`, the same construction against the tools input, and `allod/tools` has 39 commits since 26 June against secrets' 27. So every commit to the tools repo also moves every dev machine's derivation. That makes the answer to "will it fire too often" worse than the figure I gave, and it means `allod/archetypes#21` alone does not fully quiet the gate, since its scope is the secrets exposure rather than the tools reference. Worth deciding whether the tools site is folded into that issue or tracked separately. One loose end for the record: `allod/strategy#25`, which recorded this limit in the brainstorm, was closed without merging, so the text never landed and the finding currently lives only in that closed PR and in this thread. `allod/archetypes#21` is the durable home for the cause; if the observed limit is worth recording anywhere, this thread is now the better place for it than the brainstorm.
Contributor

Follow-up work tracked from this review. This PR is 37 commits behind master; the fleet-diff content must land here before the consumers below can work.

New issues:

  • allod/tools#161 — Rebase allod/tools#136 (fleet-diff) onto current master. The branch (agent/fleet-diff) is 37 commits behind; an equivalent single-commit rebase already exists locally on the current base, tests green, ready to apply.
  • allod/archetypes#66 — Assert tokenFile / httpsTokenFile are paths in mkDevVm. The Fable finding: accepting a string here lets the full secrets repo into the closure, and nothing catches it (the carve-out sweeps home.file only; the Forgejo matcher accepts both spellings). Complements archetypes#21.
  • allod/archetypes#67 — Put fleet-diff on the dev-box PATH (modules/dev-home-shared.nix). Blocked on this PR landing the flake/fleet-diff file.
  • allod/nexus#37 — Put fleet-diff on the host PATH (nix/home.nix). Blocked on this PR landing the flake/fleet-diff file.
Follow-up work tracked from this review. This PR is 37 commits behind `master`; the fleet-diff content must land here before the consumers below can work. New issues: - **allod/tools#161** — Rebase allod/tools#136 (fleet-diff) onto current `master`. The branch (`agent/fleet-diff`) is 37 commits behind; an equivalent single-commit rebase already exists locally on the current base, tests green, ready to apply. - **allod/archetypes#66** — Assert `tokenFile` / `httpsTokenFile` are paths in `mkDevVm`. The Fable finding: accepting a string here lets the full secrets repo into the closure, and nothing catches it (the carve-out sweeps `home.file` only; the Forgejo matcher accepts both spellings). Complements archetypes#21. - **allod/archetypes#67** — Put `fleet-diff` on the dev-box PATH (`modules/dev-home-shared.nix`). Blocked on this PR landing the `flake/fleet-diff` file. - **allod/nexus#37** — Put `fleet-diff` on the host PATH (`nix/home.nix`). Blocked on this PR landing the `flake/fleet-diff` file.
vnprc force-pushed agent/fleet-diff from 19979ff20f to 1608d3f333 2026-09-03 22:16:17 +01:00 Compare
vnprc requested changes 2026-09-03 23:10:35 +01:00
Dismissed
README.md Outdated
@ -147,0 +154,4 @@
```bash
cd ~/work/allod/deploy
fleet-diff --override 'archetypes/vm=git+https://forge.anarch.diy/allod/vm.git?rev=<40-char-rev>' \
Owner

this usage is really long. is it necessary to fully qualify the git checkout? can the tool just get the git repo manually from the local checkout? this would not pass muster for human use in it's current form. i think it's best to build tools a human would use and train the agents to use them. less complex usage leaves less room for tool calling errors. this makes the stack more resilient because a human can jump in and easily accomplish the same tasks agents can do. if the long for is necessary for some use cases maybe make accessible via a flag and keep the short form the default.

this usage is really long. is it necessary to fully qualify the git checkout? can the tool just get the git repo manually from the local checkout? this would not pass muster for human use in it's current form. i think it's best to build tools a human would use and train the agents to use them. less complex usage leaves less room for tool calling errors. this makes the stack more resilient because a human can jump in and easily accomplish the same tasks agents can do. if the long for is necessary for some use cases maybe make accessible via a flag and keep the short form the default.
Owner

I would want to use it like this: fleet-diff --override 'archetypes/vm=<6-or-7-char-rev>' --expect-none

only require the minimum number of rev chars to avoid collisions (i think 6 or 7 is standard? check on this to make sure)

I would want to use it like this: `fleet-diff --override 'archetypes/vm=<6-or-7-char-rev>' --expect-none` only require the minimum number of rev chars to avoid collisions (i think 6 or 7 is standard? check on this to make sure)
Author
Member

Done, and no second flag was needed — the short form and the long one are told apart by their shape, so one option covers both.

The URL never had to be typed. The composition root's own flake.lock already records which repository every input was fetched from, so the tool reads it there. What is left is the part only you know:

cd ~/work/allod/deploy
fleet-diff --override archetypes/vm=1a2b3c4 --expect-none

A value that is not a bare revision is still passed through as a whole flake URL, so nothing that worked before stops working. That escape hatch is now for one narrow case — a source flake.lock cannot turn back into a git remote — rather than the everyday path.

On "build tools a human would use and train the agents to use them": the run now prints what your revision resolved to, archetypes/vm=1a2b3c4 → git+…?rev=1a2b3c4d… (refs/heads/agent/split). A human reading a gate's output can see which commit was actually pinned and which branch it came from without reconstructing it from the command line.

Done, and no second flag was needed — the short form and the long one are told apart by their shape, so one option covers both. The URL never had to be typed. The composition root's own `flake.lock` already records which repository every input was fetched from, so the tool reads it there. What is left is the part only you know: ```bash cd ~/work/allod/deploy fleet-diff --override archetypes/vm=1a2b3c4 --expect-none ``` A value that is not a bare revision is still passed through as a whole flake URL, so nothing that worked before stops working. That escape hatch is now for one narrow case — a source `flake.lock` cannot turn back into a git remote — rather than the everyday path. On "build tools a human would use and train the agents to use them": the run now prints what your revision resolved to, `archetypes/vm=1a2b3c4 → git+…?rev=1a2b3c4d… (refs/heads/agent/split)`. A human reading a gate's output can see which commit was actually pinned and which branch it came from without reconstructing it from the command line.
Author
Member

That is now the usage, and 7 is the floor.

Checked rather than assumed. Git's core.abbrev defaults to auto, which starts at 7 characters and grows with a repository's object count; rev-parse will not disambiguate below 4; the Linux kernel's convention is 12 because it is enormous. So 7 is the standard you were reaching for, and 6 is below git's own floor.

The length is only a sanity check, though — it is not what makes this safe. The abbreviation is expanded against the remote's refs, so the tool refuses one that matches no commit there, and refuses one that matches more than one, naming both:

fleet-diff: abc1234 names 2 commits on https://forge.anarch.diy/allod/vm.git: abc1234000… abc1234fff…
  give more characters

That is the real collision guarantee: a collision fails loudly instead of pinning the wrong commit quietly, which is the failure a length rule alone cannot prevent.

Two consequences worth knowing. An abbreviation has to be pushed and be the tip of a branch, tag, or pull request, because the remote's ref listing is the only place the rest of the hash exists. A commit deeper in history takes its whole 40-character revision, which needs no expanding and so needs no lookup.

That is now the usage, and 7 is the floor. Checked rather than assumed. Git's `core.abbrev` defaults to `auto`, which starts at **7** characters and grows with a repository's object count; `rev-parse` will not disambiguate below 4; the Linux kernel's convention is 12 because it is enormous. So 7 is the standard you were reaching for, and 6 is below git's own floor. The length is only a sanity check, though — it is not what makes this safe. The abbreviation is expanded against the remote's refs, so the tool refuses one that matches no commit there, and refuses one that matches more than one, naming both: ``` fleet-diff: abc1234 names 2 commits on https://forge.anarch.diy/allod/vm.git: abc1234000… abc1234fff… give more characters ``` That is the real collision guarantee: a collision fails loudly instead of pinning the wrong commit quietly, which is the failure a length rule alone cannot prevent. Two consequences worth knowing. An abbreviation has to be pushed and be the tip of a branch, tag, or pull request, because the remote's ref listing is the only place the rest of the hash exists. A commit deeper in history takes its whole 40-character revision, which needs no expanding and so needs no lookup.
flake/README.md Outdated
@ -95,0 +110,4 @@
once with the given revisions substituted — and reports `unchanged` or
`CHANGES`. The checkout defaults to the current directory.
Evaluation is per-machine and sequential on purpose. `nix flake check` over a
Owner

kill this paragraph. point measurements aren't generalizeable across hardware profiles and they don't stand the test of time against a moving codebase.

kill this paragraph. point measurements aren't generalizeable across hardware profiles and they don't stand the test of time against a moving codebase.
Author
Member

Killed, and in the --help text that repeated the same figure.

What replaced it is the property rather than one machine's reading of it: each machine is evaluated on its own, so a run costs one machine's evaluation at a time rather than the whole fleet's at once. That stays true on any hardware and survives the codebase moving; the number did neither.

Killed, and in the `--help` text that repeated the same figure. What replaced it is the property rather than one machine's reading of it: each machine is evaluated on its own, so a run costs one machine's evaluation at a time rather than the whole fleet's at once. That stays true on any hardware and survives the codebase moving; the number did neither.
Author
Member

Taking the three review comments. Recording what I read them as asking before I implement, so the record is on the PR rather than only in a session.

  1. Short form is the default; the long URL becomes the escape hatch. --override 'archetypes/vm=<rev>' — the repo URL comes from the checkout's own flake.lock (the input is already locked to a URL there; making the human retype it was the tool asking for something it already knew). A value that is not a bare revision is still passed through as a full flake URL, so the long form keeps working without a second flag.
  2. Short revisions. <rev> may be an abbreviated hash. On the minimum: git's own core.abbrev=auto floors at 7 characters and grows with object count, and rev-parse refuses fewer than 4; 7 is the convention worth adopting. The length is only a sanity floor though — the real guarantee is that the tool resolves the abbreviation against the actual repository and refuses an ambiguous or unknown prefix, so a collision fails loudly instead of silently pinning the wrong commit.
  3. The 7 GiB paragraph goes, in flake/README.md and in the --help text that repeats the same figure. The design stays per-machine and sequential; the reason it gives will be the durable one rather than one machine's measurement.

One thing the short form has to do that the long form was doing by hand: the original example carries ?ref=refs/heads/agent/...&rev=..., and that ref= is load-bearing — Nix refuses a rev it cannot reach from the default branch. So resolving a revision also means finding which ref carries it, which is the same lookup that expands an abbreviation. The short form therefore composes both, and a revision that is on no ref of the remote is refused with that reason rather than surfacing later as an opaque fetch failure.

Taking the three review comments. Recording what I read them as asking before I implement, so the record is on the PR rather than only in a session. 1. **Short form is the default; the long URL becomes the escape hatch.** `--override 'archetypes/vm=<rev>'` — the repo URL comes from the checkout's own `flake.lock` (the input is already locked to a URL there; making the human retype it was the tool asking for something it already knew). A value that is not a bare revision is still passed through as a full flake URL, so the long form keeps working without a second flag. 2. **Short revisions.** `<rev>` may be an abbreviated hash. On the minimum: git's own `core.abbrev=auto` floors at 7 characters and grows with object count, and `rev-parse` refuses fewer than 4; 7 is the convention worth adopting. The length is only a sanity floor though — the real guarantee is that the tool resolves the abbreviation against the actual repository and refuses an ambiguous or unknown prefix, so a collision fails loudly instead of silently pinning the wrong commit. 3. **The 7 GiB paragraph goes**, in `flake/README.md` and in the `--help` text that repeats the same figure. The design stays per-machine and sequential; the reason it gives will be the durable one rather than one machine's measurement. One thing the short form has to do that the long form was doing by hand: the original example carries `?ref=refs/heads/agent/...&rev=...`, and that `ref=` is load-bearing — Nix refuses a `rev` it cannot reach from the default branch. So resolving a revision also means finding which ref carries it, which is the same lookup that expands an abbreviation. The short form therefore composes both, and a revision that is on no ref of the remote is refused with that reason rather than surfacing later as an opaque fetch failure.
The checkout's own lock already records which repository each input came
from, so a revision is all a caller has to type. An abbreviation of seven
characters or more is expanded against the remote's refs, which is also
what refuses one naming no commit there, or more than one. A whole flake
URL is still accepted in the same place, for a source the lock cannot name.

Only the revision is pinned, never a ref. Nix reaches a commit without
being told which branch carries it, checked against a cold fetcher cache
on a commit that is not on the default branch, so naming a ref would add
a second thing that has to stay true for no gain.

Drops the per-machine evaluation rationale's memory figure, which measured
one machine on one host rather than the design.
Restores two file modes this branch had swapped: flake/fleet-diff back to
644 like its siblings, tests/flake/fleet-diff.sh back to executable so it
can be run directly the way every sibling suite can.

An over-long hex string reported itself as too short, because a single
predicate covered both ends of the range and only one message existed.
Naming the same input twice is now refused instead of printing a receipt
for an override nix would discard, keeping only the last. ls-remote runs
with GIT_TERMINAL_PROMPT=0, so a gate cannot stop waiting for a credential
prompt, and a github lock entry's own host is honoured rather than assumed
to be github.com.

Adds the assertion for the HEAD skip, which nothing covered: the review
deleted that line and all 128 tests still passed.
Author
Member

Ran a read-only adversarial review pass over d919319 on a different model, since the short-form change put real logic — a lock walk and a ref listing — into the one part of this tool that had any. Findings and what changed, in 3e13fd8.

The load-bearing claim survived independent attack

The commit rests on Nix reaching a commit given only ?rev=, with no ?ref=. The review rebuilt that test rather than trusting my version of it: a local fixture with a branch tip, a non-tip ancestor, and a commit reachable only through an annotated tag, served three ways — file://, git:// via git-daemon, and smart HTTP through git http-backend with default upload-pack config, verified strict by confirming git fetch <url> <bare-sha> is refused there. Plus cold fetches against the real forge, including allod/archetypes, which was genuinely absent from ~/.cache/nix/gitv3.

Rev-only works in every case that matters here, including a non-tip ancestor six commits deep on a cold Forgejo fetch.

One bounded caveat, now in Risk: on a strict vanilla smart-HTTP remote with want-by-SHA disabled, a commit reachable only through an annotated tag fails with Server does not allow request for unadvertised object. That is a loud nix error and exits 3 — it cannot make a gate silently pass — and every git-type input in the real allod/deploy lock points at Forgejo, which serves it. Worth recording rather than discovering.

What was wrong

I had swapped two file modes. flake/fleet-diff had become 755 while its siblings flake-status and flake-update-cascade are 644, and tests/flake/fleet-diff.sh had lost the executable bit every other suite has, so ./tests/flake/fleet-diff.sh no longer ran. Both an artefact of writing through a temp file and moving it into place. Restored.

An over-long revision reported itself as too short. --override vm=<44 hex chars> answered a revision needs at least 7 characters, got 44, because one predicate covered both ends of the range and only one message existed. It now says the revision is at most 40.

The same input overridden twice was accepted. Both receipt lines printed, but nix keeps only the last --override-input — verified against a live flake in both orders. The assertion still ran against the surviving one, so the gate was not weakened, but a receipt that names an override which did not apply is exactly the kind of quiet lie this tool exists to prevent. Refused now.

One mutation the suite could not see. Deleting the HEAD skip from the ref parsing left all 128 assertions passing. The behaviour is only cosmetic — HEAD would have appeared in the receipt beside the branch it duplicates — but an untested line is an untested line. There is an assertion for it now, and I confirmed it fails when that line goes.

A garden-path sentence in flake/README.md: "A source flake.lock cannot turn back into a git remote takes a whole flake URL" was missing its "that".

Two smaller things the review raised as plausible, both taken: git ls-remote now runs with GIT_TERMINAL_PROMPT=0, so a gate cannot sit waiting for a credential prompt on a remote that wants auth; and a github lock entry's own host field is honoured rather than assumed to be github.com, which would have sent a GitHub Enterprise input to the wrong remote. No such input exists in any allod lock today.

What it could not break

No path was found where a bad override yields exit 0 or a wrong verdict. It checked the revision-versus-URL discriminator for misclassification in both directions, an empty ls-remote listing, exit called from inside a function that might have been running in a subshell, whether NIX_OVERRIDE_ARGS could end up empty while the run still asserted, and cross-contamination through the shared REV/REFS globals across mixed rev-and-URL overrides. It also audited the harness against shell.md and found no vacuous assertion: fail() exits, MOCK_LOG is truncated per invocation, and each grep -c … || true feeds an assert_equal rather than standing alone.

The suite is now 135 assertions. Every new one was checked against a deliberately broken tool, including the exact HEAD-skip mutation above.

Ran a read-only adversarial review pass over `d919319` on a different model, since the short-form change put real logic — a lock walk and a ref listing — into the one part of this tool that had any. Findings and what changed, in `3e13fd8`. ## The load-bearing claim survived independent attack The commit rests on Nix reaching a commit given only `?rev=`, with no `?ref=`. The review rebuilt that test rather than trusting my version of it: a local fixture with a branch tip, a non-tip ancestor, and a commit reachable only through an annotated tag, served three ways — `file://`, `git://` via git-daemon, and smart HTTP through `git http-backend` with **default** upload-pack config, verified strict by confirming `git fetch <url> <bare-sha>` is refused there. Plus cold fetches against the real forge, including `allod/archetypes`, which was genuinely absent from `~/.cache/nix/gitv3`. Rev-only works in every case that matters here, including a non-tip ancestor six commits deep on a cold Forgejo fetch. One bounded caveat, now in Risk: on a strict vanilla smart-HTTP remote with want-by-SHA disabled, a commit reachable **only** through an annotated tag fails with `Server does not allow request for unadvertised object`. That is a loud nix error and exits 3 — it cannot make a gate silently pass — and every git-type input in the real `allod/deploy` lock points at Forgejo, which serves it. Worth recording rather than discovering. ## What was wrong **I had swapped two file modes.** `flake/fleet-diff` had become 755 while its siblings `flake-status` and `flake-update-cascade` are 644, and `tests/flake/fleet-diff.sh` had lost the executable bit every other suite has, so `./tests/flake/fleet-diff.sh` no longer ran. Both an artefact of writing through a temp file and moving it into place. Restored. **An over-long revision reported itself as too short.** `--override vm=<44 hex chars>` answered `a revision needs at least 7 characters, got 44`, because one predicate covered both ends of the range and only one message existed. It now says the revision is at most 40. **The same input overridden twice was accepted.** Both receipt lines printed, but nix keeps only the last `--override-input` — verified against a live flake in both orders. The assertion still ran against the surviving one, so the gate was not weakened, but a receipt that names an override which did not apply is exactly the kind of quiet lie this tool exists to prevent. Refused now. **One mutation the suite could not see.** Deleting the `HEAD` skip from the ref parsing left all 128 assertions passing. The behaviour is only cosmetic — `HEAD` would have appeared in the receipt beside the branch it duplicates — but an untested line is an untested line. There is an assertion for it now, and I confirmed it fails when that line goes. **A garden-path sentence** in `flake/README.md`: "A source `flake.lock` cannot turn back into a git remote takes a whole flake URL" was missing its "that". Two smaller things the review raised as plausible, both taken: `git ls-remote` now runs with `GIT_TERMINAL_PROMPT=0`, so a gate cannot sit waiting for a credential prompt on a remote that wants auth; and a `github` lock entry's own `host` field is honoured rather than assumed to be github.com, which would have sent a GitHub Enterprise input to the wrong remote. No such input exists in any allod lock today. ## What it could not break No path was found where a bad override yields exit 0 or a wrong verdict. It checked the revision-versus-URL discriminator for misclassification in both directions, an empty `ls-remote` listing, `exit` called from inside a function that might have been running in a subshell, whether `NIX_OVERRIDE_ARGS` could end up empty while the run still asserted, and cross-contamination through the shared `REV`/`REFS` globals across mixed rev-and-URL overrides. It also audited the harness against `shell.md` and found no vacuous assertion: `fail()` exits, `MOCK_LOG` is truncated per invocation, and each `grep -c … || true` feeds an `assert_equal` rather than standing alone. The suite is now 135 assertions. Every new one was checked against a deliberately broken tool, including the exact HEAD-skip mutation above.
vnprc approved these changes 2026-09-04 00:43:08 +01:00
vnprc merged commit 3e13fd8c1a into master 2026-09-04 00:43:22 +01:00
vnprc deleted branch agent/fleet-diff 2026-09-04 00:43:22 +01:00
Sign in to join this conversation.
No description provided.