From e466d9732949e244ba3d5665dd689515d6e7ea6e Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 14:19:35 -0400 Subject: [PATCH 1/5] fix(renovate): refresh the FOD hash on devenv-nixpkgs channel bumps (RIG-3296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The devenv-nixpkgs digest packageRule's `postUpgradeTasks` ran only `refresh-devenv-nixpkgs.ts`, with a comment asserting the FOD refresh was "intentionally absent" because a channel bump supposedly cannot move `agent-image/entrypoint.nix`'s `outputHash` (premise: biome is a root-only devDependency, absent from the `--filter '@compass/agent'` install). That premise is empirically false. PR #580 (digest → 256551e) failed CI `e2e` with `hash mismatch in fixed-output derivation compass-agent-node-modules` (specified `sha256-GrsX8VmqoHIg…`, got `sha256-+ogECRp2…`). The channel bump re-resolved the `@compass/agent` closure in `bun.lock` (`@opentelemetry/sdk-metrics` 2.9→2.10 and an `@oh-my-pi/pi-coding-agent` re-nesting) **and** moved `pkgs.bun`, the nixpkgs-versioned builder the FOD installs with — both move the recursive `outputHash`. So every channel bump reds `e2e`. Fix: add `bun tools/renovate/refresh-fod-hashes.ts` to the rule's `postUpgradeTasks.commands` (after the relock, so it self-gates on the now-changed `bun.lock` and realises the new tree) and `agent-image/entrypoint.nix` to `fileFilters` (Renovate commits only listed files — without it the recomputed hash edit is silently dropped). The command is already allowlisted in `bot-config.json5`. The exemption test in `config.test.ts` is flipped to pin the corrected wiring, and the global FOD site-count guard goes 2→3 (the FOD refresh now rides three task sites). Verified: `bun test tools/renovate/config.test.ts` 68/68, biome clean. Spec-impact: none. Refs RIG-3296 Co-authored-by: Matt Wilkinson --- tools/renovate/config.json5 | 37 ++++++++++++++------------ tools/renovate/config.test.ts | 49 ++++++++++++++++------------------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/tools/renovate/config.json5 b/tools/renovate/config.json5 index 374b8c6b..53f46f8d 100644 --- a/tools/renovate/config.json5 +++ b/tools/renovate/config.json5 @@ -614,31 +614,34 @@ // in config.test.ts documents). compass has no committed inner-rev guard // file, so no such entry. // - // FOD-REFRESH IS INTENTIONALLY ABSENT here, though this branch rewrites - // bun.lock + a workspace manifest (the biome catalog pin) — the two inputs - // agent-image/entrypoint.nix names as moving its bun `outputHash`. It is - // safe because biome never enters the hashed tree: the FOD installs with - // `bun install --frozen-lockfile --ignore-scripts --filter '@compass/agent'` - // (entrypoint.nix), and biome is a ROOT-ONLY devDependency (package.json - // `devDependencies`, absent from @compass/agent's manifest). Verified: that - // exact filtered install yields biome in neither `node_modules` nor the - // `.bun` virtual store, so a channel/biome bump cannot move outputHash. If - // biome ever becomes a compass-agent dependency, this rule must append - // `bun tools/renovate/refresh-fod-hashes.ts` + `agent-image/entrypoint.nix` - // to its task (it runs after the bun.lock rewrite, so it would self-gate and - // refresh in the same branch). config.test.ts pins this exemption. + // A channel bump re-resolves the agent-closure bun.lock (including + // opentelemetry transitives) and moves pkgs.bun, the nixpkgs-versioned + // builder used by the FOD. Both inputs move agent-image/entrypoint.nix's + // bun `outputHash`, so the FOD refresh MUST ride this rule. It self-gates + // on bun.lock changing and runs after refresh-devenv-nixpkgs.ts rewrites + // the lockfile; when the trigger is unchanged it safely no-ops. The + // command and output file must both be listed here because fileFilters is + // an INCLUDE allowlist: omitting agent-image/entrypoint.nix would silently + // drop the recomputed hash edit and leave every channel bump red. The + // #580 digest bump empirically hit exactly that failure (`hash mismatch + // in fixed-output derivation compass-agent-node-modules`). // - // Self-hosted bot config MUST allow the command: - // allowedCommands: [… , "^bun tools/renovate/refresh-devenv-nixpkgs\\.ts$"] - // see tools/renovate/bot-config.json5 (config.test.ts pins the two together). + // Self-hosted bot config MUST allow both commands: + // allowedCommands: [… , "^bun tools/renovate/refresh-devenv-nixpkgs\\.ts$", + // "^bun tools/renovate/refresh-fod-hashes\\.ts$"] + // see tools/renovate/bot-config.json5 (config.test.ts pins the wiring). postUpgradeTasks: { - commands: ["bun tools/renovate/refresh-devenv-nixpkgs.ts"], + commands: [ + "bun tools/renovate/refresh-devenv-nixpkgs.ts", + "bun tools/renovate/refresh-fod-hashes.ts", + ], fileFilters: [ "devenv.lock", "package.json", "bun.lock", "flake.nix", "flake.lock", + "agent-image/entrypoint.nix", ], executionMode: "branch", }, diff --git a/tools/renovate/config.test.ts b/tools/renovate/config.test.ts index c6df3a45..ce20321a 100644 --- a/tools/renovate/config.test.ts +++ b/tools/renovate/config.test.ts @@ -208,14 +208,14 @@ describe("tools/renovate postUpgradeTasks ↔ allowedCommands (RIG-2432)", () => expect(allowed).toHaveLength(6); }); - test("the fod-hash refresh is declared at BOTH task sites (top-level + catalog)", () => { - // The command must ride two sites so it fires on every branch shape: the - // top-level branch-mode slot (gomod + bun/npm-first branches) AND the catalog - // rule's update-mode pass (catalog-first branches evict the top-level slot). - // If it collapses to one site, one of those branch shapes ships a stale FOD - // pin — PR #579's failure. So the raw (non-deduped) list carries it twice. + test("the fod-hash refresh is declared at all three task sites", () => { + // The command must ride every task shape that can own a dependency bump: + // the top-level branch-mode slot, the catalog rule's update-mode pass, and + // the devenv-nixpkgs branch-mode lockstep task. Catalog-first branches evict + // the top-level slot, while channel branches use the devenv-nixpkgs slot; the + // raw command list therefore carries the FOD refresh once at each site. const fod = "bun tools/renovate/refresh-fod-hashes.ts"; - expect(commands.filter((c) => c === fod)).toHaveLength(2); + expect(commands.filter((c) => c === fod)).toHaveLength(3); const topLevel = cfg.postUpgradeTasks?.commands ?? []; expect(topLevel).toContain(fod); const catalogRule = cfg.packageRules.find( @@ -506,19 +506,18 @@ describe("tools/renovate devenv nixpkgs lockstep", () => { expect(devenvRule?.minimumReleaseAge).toBeNull(); }); - // Branch-mode lockstep task over exactly the five files the script writes: + // Branch-mode lockstep task over the files the script writes: // devenv.lock + package.json (biome catalog pin) + bun.lock (steps 2/4/5), - // and flake.nix + flake.lock (step 6's flake-parity lockstep). compass has NO - // committed inner-rev guard file, unlike the internal monorepo's guard entry. + // flake.nix + flake.lock (step 6's flake-parity lockstep), and + // agent-image/entrypoint.nix (the FOD outputHash). compass has NO committed + // inner-rev guard file, unlike the internal monorepo's guard entry. // - // The `every(... refresh-devenv-nixpkgs ...)` assertion also PINS the verified - // FOD-refresh exemption: this branch rewrites bun.lock + the biome catalog pin, - // but biome is a root-only devDependency absent from the FOD's filtered - // `--filter '@compass/agent'` install (verified: not in node_modules nor .bun), - // so a channel bump cannot move agent-image/entrypoint.nix's outputHash and the - // FOD refresh is intentionally NOT wired here. If biome ever enters the - // compass-agent closure, adding the FOD command is what makes this test fail — - // forcing a conscious revisit of the exemption rather than a silent red build. + // The FOD refresh is required here: a channel bump re-resolves the + // compass-agent bun.lock closure (including opentelemetry transitives) and + // moves pkgs.bun, the nixpkgs-versioned FOD builder. Both move the outputHash; + // PR #580 empirically failed with a compass-agent-node-modules hash mismatch. + // refresh-fod-hashes.ts runs after the relock, self-gates on bun.lock changes, + // and fileFilters must include its output or Renovate silently drops the edit. test("the lockstep postUpgradeTask is branch-mode over the written files", () => { const task = devenvRule?.postUpgradeTasks; expect(task?.executionMode).toBe("branch"); @@ -528,6 +527,7 @@ describe("tools/renovate devenv nixpkgs lockstep", () => { "bun.lock", "flake.nix", "flake.lock", + "agent-image/entrypoint.nix", ]); // Silent-drop guard (mirrors the top-level rule's flake.nix guard): step 6 // writes flake.nix + flake.lock, and fileFilters is an INCLUDE allowlist — @@ -538,16 +538,13 @@ describe("tools/renovate devenv nixpkgs lockstep", () => { expect(task?.fileFilters).toContain("flake.nix"); expect(task?.fileFilters).toContain("flake.lock"); expect(task?.commands?.length).toBeGreaterThan(0); - expect( - task?.commands?.every((c) => - /^bun tools\/renovate\/refresh-devenv-nixpkgs\.ts$/.test(c), - ), - ).toBe(true); - // Explicit: the FOD refresh is NOT on this rule (the verified exemption). - expect(task?.commands).not.toContain( + expect(task?.commands).toContain( + "bun tools/renovate/refresh-devenv-nixpkgs.ts", + ); + expect(task?.commands).toContain( "bun tools/renovate/refresh-fod-hashes.ts", ); - expect(task?.fileFilters).not.toContain("agent-image/entrypoint.nix"); + expect(task?.fileFilters).toContain("agent-image/entrypoint.nix"); }); // The digest-excludes-rollup seam: the TS rollup ALSO matches custom.regex, so From f607649c535e1bcd3522519cf57424fd5996d694 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 15:24:38 -0400 Subject: [PATCH 2/5] fix(renovate): gate node-modules FOD on devenv.lock + pin command order (RIG-3296 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review of #897. **high** — the node-modules FOD refresh gated only on `bun.lock`, but `refresh-devenv-nixpkgs.ts` writes `bun.lock` only when the biome catalog pin moves; a biome-static channel bump left `bun.lock` untouched, so the FOD refresh no-op'd even though the channel moved `pkgs.bun` (the FOD's builder), shipping a stale `outputHash`. Add `devenv.lock` to the node-modules entry's triggers so the refresh fires on every channel bump and reconciles the pin idempotently. Regression test added (red-green verified). **medium** — order-insensitive `.toContain` command asserts replaced with an order-pinning `.toEqual` (relock-then-FOD is load-bearing; mutation-verified reversal now reds). Stale two-site FOD counts corrected to three sites across config.test.ts, bot-config.json5, and the script header. **low** — flipped test renamed to cover the command+order half; `config.json5` no-op comment reworded to state the gate's real scope. Spec-impact: none. Refs RIG-3296 --- tools/renovate/bot-config.json5 | 12 ++++--- tools/renovate/config.json5 | 27 ++++++++------- tools/renovate/config.test.ts | 40 +++++++++++++---------- tools/renovate/refresh-fod-hashes.test.ts | 22 +++++++++++++ tools/renovate/refresh-fod-hashes.ts | 32 +++++++++++------- 5 files changed, 88 insertions(+), 45 deletions(-) diff --git a/tools/renovate/bot-config.json5 b/tools/renovate/bot-config.json5 index ab0fb6bc..1a989850 100644 --- a/tools/renovate/bot-config.json5 +++ b/tools/renovate/bot-config.json5 @@ -99,11 +99,13 @@ // 4. the FOD-hash refresh, which recomputes the pinned Nix fixed-output- // derivation hashes a dep bump invalidates — the Go `vendorHash` // (guest-image/default.nix) on a gomod bump, the bun `outputHash` - // (agent-image/entrypoint.nix) on a bun/catalog bump. Left stale the image - // build fails `hash mismatch in fixed-output derivation` (PR #579). Wired at - // both the top-level (branch mode) and catalog-rule (update mode) - // postUpgradeTasks in config.json5 — one command string, so ONE allowlist - // entry covers both sites. + // (agent-image/entrypoint.nix) on a bun/catalog bump OR a devenv-nixpkgs + // channel bump (which moves pkgs.bun, the FOD's builder). Left stale the + // image build fails `hash mismatch in fixed-output derivation` (PR #579). + // Wired at the top-level (branch mode), catalog-rule (update mode), and + // devenv-nixpkgs channel rule (branch mode) postUpgradeTasks in + // config.json5 — one command string, so ONE allowlist entry covers all + // three sites. // 5. the go↔go-overlay lockstep, which on a go.nix toolchain bump advances // the go-overlay input in devenv.lock to a rev that provides the new go // release and validates it resolves through gate-tools.nix's `langs.go` diff --git a/tools/renovate/config.json5 b/tools/renovate/config.json5 index 53f46f8d..f4d45256 100644 --- a/tools/renovate/config.json5 +++ b/tools/renovate/config.json5 @@ -614,17 +614,22 @@ // in config.test.ts documents). compass has no committed inner-rev guard // file, so no such entry. // - // A channel bump re-resolves the agent-closure bun.lock (including - // opentelemetry transitives) and moves pkgs.bun, the nixpkgs-versioned - // builder used by the FOD. Both inputs move agent-image/entrypoint.nix's - // bun `outputHash`, so the FOD refresh MUST ride this rule. It self-gates - // on bun.lock changing and runs after refresh-devenv-nixpkgs.ts rewrites - // the lockfile; when the trigger is unchanged it safely no-ops. The - // command and output file must both be listed here because fileFilters is - // an INCLUDE allowlist: omitting agent-image/entrypoint.nix would silently - // drop the recomputed hash edit and leave every channel bump red. The - // #580 digest bump empirically hit exactly that failure (`hash mismatch - // in fixed-output derivation compass-agent-node-modules`). + // A channel bump re-resolves the agent-closure bun.lock (opentelemetry + // transitives) ONLY when the biome catalog pin moves — refresh-devenv- + // nixpkgs.ts skips the relock on a biome-static bump — and independently + // moves pkgs.bun, the nixpkgs-versioned builder the FOD realises. Either + // can move agent-image/entrypoint.nix's bun `outputHash`, so the FOD + // refresh MUST ride this rule. It gates on bun.lock OR devenv.lock (the + // channel rev this bump always rewrites), so it fires on EVERY channel + // bump regardless of whether the relock ran, and reconciles the pin + // idempotently — a no-op write when the realised tree is unchanged. It runs + // after refresh-devenv-nixpkgs.ts so the relock's bun.lock write is already + // in the working tree. The command and output file must both be listed here + // because fileFilters is an INCLUDE allowlist: omitting + // agent-image/entrypoint.nix would silently drop the recomputed hash edit + // and leave a channel bump red. The #580 digest bump empirically hit + // exactly that failure (`hash mismatch in fixed-output derivation + // compass-agent-node-modules`). // // Self-hosted bot config MUST allow both commands: // allowedCommands: [… , "^bun tools/renovate/refresh-devenv-nixpkgs\\.ts$", diff --git a/tools/renovate/config.test.ts b/tools/renovate/config.test.ts index ce20321a..7aad472a 100644 --- a/tools/renovate/config.test.ts +++ b/tools/renovate/config.test.ts @@ -190,11 +190,12 @@ describe("tools/renovate postUpgradeTasks ↔ allowedCommands (RIG-2432)", () => // `allowedCommands` allowlist (a repo config cannot self-authorize a command), // which Renovate matches UNANCHORED via regEx(pattern).test(cmd). So each // entry's `^…$` IS the security property. Compass declares six DISTINCT - // commands across the task sites (the FOD-hash refresh rides two sites — the - // top-level branch-mode task and the catalog rule's update-mode task — so it - // appears twice in the declared list but needs only one allowlist entry; the - // devenv-fork relock likewise rides BOTH devenv-fork rules under one command - // string, since the script self-gates on which lock changed); every + // commands across the task sites (the FOD-hash refresh rides three sites — the + // top-level branch-mode task, the catalog rule's update-mode task, and the + // devenv-nixpkgs branch-mode lockstep task — so it appears three times in the + // declared list but needs only one allowlist entry; the devenv-fork relock + // likewise rides BOTH devenv-fork rules under one command string, since the + // script self-gates on which lock changed); every // distinct command must be permitted, every entry must be used, and no entry may // be an unanchored substring rule. RIG-3100 added the fifth: the go↔go-overlay // lockstep on the go pin's solo branch. RIG-2815 added the sixth: the @@ -275,7 +276,7 @@ describe("tools/renovate FOD-hash refresh wiring (PR #579)", () => { // image build fails `hash mismatch in fixed-output derivation`. refresh-fod- // hashes.ts recomputes it, but Renovate only COMMITS files a task's fileFilters // name — so a task that rewrites a FOD file without listing it silently drops - // the fix and the bump PR still goes red. Guard both sites' fileFilters. + // the fix and the bump PR still goes red. Guard all three sites' fileFilters. const FOD = "bun tools/renovate/refresh-fod-hashes.ts"; const topLevel = cfg.postUpgradeTasks; const catalogRule = cfg.packageRules.find( @@ -512,13 +513,19 @@ describe("tools/renovate devenv nixpkgs lockstep", () => { // agent-image/entrypoint.nix (the FOD outputHash). compass has NO committed // inner-rev guard file, unlike the internal monorepo's guard entry. // - // The FOD refresh is required here: a channel bump re-resolves the - // compass-agent bun.lock closure (including opentelemetry transitives) and - // moves pkgs.bun, the nixpkgs-versioned FOD builder. Both move the outputHash; - // PR #580 empirically failed with a compass-agent-node-modules hash mismatch. - // refresh-fod-hashes.ts runs after the relock, self-gates on bun.lock changes, - // and fileFilters must include its output or Renovate silently drops the edit. - test("the lockstep postUpgradeTask is branch-mode over the written files", () => { + // The FOD refresh is required here: a channel bump moves pkgs.bun, the + // nixpkgs-versioned builder the FOD realises, and — when the biome catalog pin + // also moves — re-resolves the compass-agent bun.lock closure (opentelemetry + // transitives). Either can move the outputHash; PR #580 empirically failed + // with a compass-agent-node-modules hash mismatch. refresh-fod-hashes.ts runs + // AFTER the relock (it reads the relock's bun.lock write from the working + // tree) and gates on bun.lock OR devenv.lock so it fires on every channel bump + // regardless of whether the relock ran; fileFilters must include its output or + // Renovate silently drops the edit. The order is load-bearing and silent when + // wrong: reversed, the FOD refresh runs before the relock writes bun.lock, the + // gate reads clean, and it no-ops — the pinned toEqual below turns that into a + // red test. + test("the lockstep postUpgradeTask is branch-mode, runs relock-then-FOD, and commits every written file", () => { const task = devenvRule?.postUpgradeTasks; expect(task?.executionMode).toBe("branch"); expect(task?.fileFilters).toEqual([ @@ -537,13 +544,10 @@ describe("tools/renovate devenv nixpkgs lockstep", () => { // asserts turn that silent drop into a red test. expect(task?.fileFilters).toContain("flake.nix"); expect(task?.fileFilters).toContain("flake.lock"); - expect(task?.commands?.length).toBeGreaterThan(0); - expect(task?.commands).toContain( + expect(task?.commands).toEqual([ "bun tools/renovate/refresh-devenv-nixpkgs.ts", - ); - expect(task?.commands).toContain( "bun tools/renovate/refresh-fod-hashes.ts", - ); + ]); expect(task?.fileFilters).toContain("agent-image/entrypoint.nix"); }); diff --git a/tools/renovate/refresh-fod-hashes.test.ts b/tools/renovate/refresh-fod-hashes.test.ts index 2e38dc4a..2658c46e 100644 --- a/tools/renovate/refresh-fod-hashes.test.ts +++ b/tools/renovate/refresh-fod-hashes.test.ts @@ -268,6 +268,28 @@ describe("tools/renovate/refresh-fod-hashes.ts gate (PR #579)", () => { expect(await readFile(join(repo, GO_ENTRY.file), "utf8")).toBe(goBefore); }); + // The HIGH-finding regression (RIG-3296 review): a devenv-nixpkgs channel bump + // that does NOT move biome leaves bun.lock untouched (refresh-devenv-nixpkgs + // skips the relock when the catalog pin is static), yet the channel moves + // pkgs.bun — the FOD's builder — which may move the recursive outputHash. So + // the node-modules entry gates on devenv.lock too: a devenv.lock-only diff + // must still refresh the bun outputHash, or a biome-static channel bump ships + // the exact `compass-agent-node-modules` hash mismatch this task exists to + // prevent. The Go pin is left untouched (its trigger did not change). + test("a devenv.lock-only bump refreshes the bun outputHash (channel pkgs.bun move)", async () => { + const goBefore = await readFile(join(repo, GO_ENTRY.file), "utf8"); + await Bun.write(join(repo, "devenv.lock"), "bumped\n"); + + const res = await runRefresh(repo); + expect(res.exitCode).toBe(0); + + const bunNix = await readFile(join(repo, BUN_ENTRY.file), "utf8"); + expect(hashOnMarker(bunNix, BUN_ENTRY.marker)).toBe( + stubSriForFragment("node-modules"), + ); + expect(await readFile(join(repo, GO_ENTRY.file), "utf8")).toBe(goBefore); + }); + // Restore-on-completion: the script fakes the pin to force the mismatch, then // writes the REAL value — never leaving the fake all-A hash in the tree. test("never leaves the fake all-A SRI in a refreshed pin file", async () => { diff --git a/tools/renovate/refresh-fod-hashes.ts b/tools/renovate/refresh-fod-hashes.ts index fe02ad1b..3b2ff6b3 100755 --- a/tools/renovate/refresh-fod-hashes.ts +++ b/tools/renovate/refresh-fod-hashes.ts @@ -19,7 +19,13 @@ // realise — see FodEntry.mirrorFiles. // agent-image/entrypoint.nix outputHash compass-agent's installed node_modules // tree (recursive FOD of `bun install`) — -// invalidated by a bun.lock bump +// invalidated by a bun.lock bump, and +// refreshed on a devenv-nixpkgs channel +// bump too (devenv.lock): the channel +// moves pkgs.bun, the FOD's builder, +// which MAY move the recursive tree — so +// the entry gates on BOTH and the refresh +// reconciles the pin whichever moved. // // Neither is a URL hash a `nix store prefetch-file` can recompute (that is // refresh-toolchain-hashes.ts's job for the vendored-binary pins). A vendorHash / @@ -41,16 +47,20 @@ // // Self-gating: for each entry, act only when one of its trigger manifests differs // from the base branch (mirrors refresh-toolchain-hashes.ts's versions/*.nix -// gate). So it is a cheap no-op on every branch that touches neither manifest, -// a gomod bump refreshes only the Go vendorHash, and a bun bump only the bun -// outputHash. Idempotent: re-running rewrites the same SRI. +// gate). So it is a cheap no-op on every branch that touches no trigger manifest, +// a gomod bump refreshes only the Go vendorHash, and a bun.lock bump OR a +// devenv-nixpkgs channel bump (devenv.lock) refreshes the bun outputHash. +// Idempotent: re-running rewrites the same SRI — a no-op write when the realised +// tree is unchanged (so gating on devenv.lock costs at most one extra realise). // -// Wired from config.json5 both at top-level postUpgradeTasks (branch mode — -// covers gomod branches and pure-bun-first TypeScript-rollup branches) and on the -// catalog packageRule (update mode — covers catalog-first rollup branches, where -// the collapsed branch config evicts the top-level branch task; see that rule's -// note). Both are the same `bun tools/renovate/refresh-fod-hashes.ts` command, -// allowlisted once in bot-config.json5 (config.test.ts pins the two together). +// Wired from config.json5 at THREE sites, all the same +// `bun tools/renovate/refresh-fod-hashes.ts` command (allowlisted once in +// bot-config.json5; config.test.ts pins them together): top-level +// postUpgradeTasks (branch mode — gomod branches and pure-bun-first +// TypeScript-rollup branches), the catalog packageRule (update mode — +// catalog-first rollup branches, where the collapsed branch config evicts the +// top-level branch task), and the devenv-nixpkgs channel rule (branch mode — a +// channel bump moves pkgs.bun; see that rule's note). // // Requires `nix` (nix-command) + `bun` + `git` on PATH and network. The build is // self-contained: `nix build` fetches the Go/bun toolchains it needs into the @@ -113,7 +123,7 @@ export const FOD_ENTRIES: FodEntry[] = [ file: "agent-image/entrypoint.nix", marker: 'outputHash = "sha256-', drvFragment: "node-modules", - triggers: ["bun.lock"], + triggers: ["bun.lock", "devenv.lock"], }, ]; From 6d7ecb9d65689a940986056f259122d691169fb8 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 16:16:43 -0400 Subject: [PATCH 3/5] docs(renovate): correct stale order-pin rationale in FOD lockstep test (RIG-3296 review R2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The order-pinning test's rationale described the pre-fix failure mode (gate reads clean and no-ops on reversed order), which is false against the shipped triggers: ["bun.lock", "devenv.lock"]. The devenv.lock trigger makes the FOD entry gate in either order, so a reversed order is worse than a no-op — it realises the FOD against the still-at-base bun.lock, then step 5 rewrites bun.lock underneath it, committing a pin for the OLD closure beside the NEW lockfile (the exact hash mismatch this task prevents). Comment-only; the toEqual pin already defends the order. Co-authored-by: Matt Wilkinson --- tools/renovate/config.test.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tools/renovate/config.test.ts b/tools/renovate/config.test.ts index 7aad472a..3282cbd8 100644 --- a/tools/renovate/config.test.ts +++ b/tools/renovate/config.test.ts @@ -521,10 +521,14 @@ describe("tools/renovate devenv nixpkgs lockstep", () => { // AFTER the relock (it reads the relock's bun.lock write from the working // tree) and gates on bun.lock OR devenv.lock so it fires on every channel bump // regardless of whether the relock ran; fileFilters must include its output or - // Renovate silently drops the edit. The order is load-bearing and silent when - // wrong: reversed, the FOD refresh runs before the relock writes bun.lock, the - // gate reads clean, and it no-ops — the pinned toEqual below turns that into a - // red test. + // Renovate silently drops the edit. The order is load-bearing and silent + // when wrong: the devenv.lock trigger makes the FOD entry gate in either + // order, so a reversed order does NOT read clean and no-op — it realises the + // FOD against the still-at-base bun.lock, then refresh-devenv-nixpkgs.ts step + // 5 rewrites bun.lock underneath it, committing a pin that content-addresses + // the OLD closure beside the NEW lockfile (the `hash mismatch in fixed-output + // derivation compass-agent-node-modules` this task exists to prevent). The + // pinned toEqual below turns that reversal into a red test. test("the lockstep postUpgradeTask is branch-mode, runs relock-then-FOD, and commits every written file", () => { const task = devenvRule?.postUpgradeTasks; expect(task?.executionMode).toBe("branch"); From 285a19632d7e29b66036f937bc166c913bf7bbd5 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sun, 6 Sep 2026 02:09:17 -0400 Subject: [PATCH 4/5] fix(renovate): close FOD-refresh gap on the two devenv relock sites (RIG-3360) `packageRules[13]` (devenv fork root) and `packageRules[15]` (go <-> go-overlay lockstep) each declare `devenv.lock` in `fileFilters` -- a declared trigger of the `agent-image/entrypoint.nix` FOD entry -- while running no hash refresh. A rule-level `postUpgradeTasks` REPLACES the top-level one on its branch, so both evict the top-level FOD leg and can commit a lock change beside a pin that still content-addresses the old closure. That is the failure mode which kept Renovate PR #580 red: `hash mismatch in fixed-output derivation`. Both sites now append the refresh AFTER the command that writes the lock (the pin must be realised against the written file, not the still-at-base one) and name `agent-image/entrypoint.nix` in `fileFilters` so the recomputed pin can actually be committed -- `fileFilters` is an include allowlist, so omitting it recomputes the pin and then silently drops it. This is fail-safe rather than a live fix: `devenv update devenv` and `devenv update go-overlay` each relock one non-nixpkgs input, so neither moves the nixpkgs rev supplying `pkgs.bun`, the FOD's builder. But the coupling is declared at file granularity, and an input-graph refactor would falsify that causal argument silently. The refresh self-gates per entry, so the price is a no-op. Adds a generalized guard deriving the requirement from `FOD_ENTRIES` itself rather than a hand-copied list: every task site naming a declared trigger must run the refresh and name that entry's FOD file plus its mirrorFiles. Includes a non-vacuity assertion and documents the detection boundary -- a trigger written by a Renovate manager rather than the task never appears in `fileFilters`, which is why the `guest-image/default.nix` vendorHash leg (triggers `go/go.mod`, `go/go.sum`) stays covered by assertion on the top-level site instead. Reconciles the moved counts: the refresh now rides five task sites, not three, and the per-scope task pins for the fork and lockstep rules assert their whole command lists so a reordering reds a test instead of shipping a stale pin. Co-authored-by: Matt Wilkinson --- tools/renovate/config.json5 | 104 +++++++++++++---- tools/renovate/config.test.ts | 209 +++++++++++++++++++++++++++++----- 2 files changed, 258 insertions(+), 55 deletions(-) diff --git a/tools/renovate/config.json5 b/tools/renovate/config.json5 index f4d45256..6d814686 100644 --- a/tools/renovate/config.json5 +++ b/tools/renovate/config.json5 @@ -706,23 +706,52 @@ // `executionMode: "branch"` runs it once after the digest update is // applied; safe because this dep never shares a branch (its own groupName // above), so it owns the single branch-mode slot. Rule-level - // postUpgradeTasks REPLACE the top-level one for matching branches; the - // evicted top-level task self-gates on the versions/*.nix pins + - // go.mod/bun.lock and would be a no-op here anyway. fileFilters is an - // INCLUDE allowlist — Renovate commits ONLY matching files — and the - // relock writes exactly this one lock, so it is the sole entry. Listing - // the agent-image lock here would be dead surface (that lock is the - // sibling rule's) and listing LESS would silent-drop the relock, shipping - // a rev bump whose narHash never moved. + // postUpgradeTasks REPLACE the top-level one for matching branches, which + // evicts the top-level task's FOD-hash refresh too — and that refresh's + // agent-image/entrypoint.nix entry gates on devenv.lock (a declared trigger + // in refresh-fod-hashes.ts's FOD_ENTRIES), the very file this relock + // rewrites. So the refresh is re-declared below rather than left to the + // evicted slot; the toolchain-hash leg really is lost-free, since it + // self-gates on the versions/*.nix pins this branch never touches. + // + // Order is LOAD-BEARING: relock FIRST, refresh SECOND. The refresh realises + // the pin against the lock as it stands on disk, so the reverse order would + // realise against the still-at-base lock and then have that lock rewritten + // underneath it — shipping a pin that content-addresses the OLD closure, + // i.e. the silent form of the `hash mismatch in fixed-output derivation` + // build break these tasks exist to prevent. + // + // Carrying the refresh here is FAIL-SAFE, not a fix for a live break: + // `devenv update devenv` re-locks ONE non-nixpkgs input, so it does not + // move the nixpkgs rev that supplies pkgs.bun — the FOD's builder — and the + // installed tree, hence today's pin, is unchanged. But the coupling is + // declared at FILE granularity (devenv.lock IS a trigger of that entry), + // and a fork bump that altered how `pkgs` is constructed would falsify the + // causal argument silently. The refresh self-gates per entry, so the price + // of being safe is one no-op. // - // Self-hosted bot config MUST allow the command: - // allowedCommands: [… , "^bun tools/renovate/refresh-devenv-lock\\.ts$"] - // see tools/renovate/bot-config.json5 (config.test.ts pins the two - // together). ONE command string serves both locks — the script self-gates - // on WHICH lock changed — so one allowlist entry covers both rules. + // fileFilters is an INCLUDE allowlist — Renovate commits ONLY matching + // files — and it names exhaustively the files this task can write: the lock + // the relock rewrites, plus the FOD file whose recomputed pin would + // otherwise be silently dropped. Listing the agent-image lock would be dead + // surface (that lock is the sibling rule's), as would guest-image/default.nix + // and flake.nix (the Go entry's triggers are go/go.mod and go/go.sum, which + // this task never writes). Listing LESS silent-drops the relock — shipping a + // rev bump whose narHash never moved — or the refreshed pin. + // + // Self-hosted bot config MUST allow both commands: + // allowedCommands: [… , "^bun tools/renovate/refresh-devenv-lock\\.ts$", + // "^bun tools/renovate/refresh-fod-hashes\\.ts$"] + // see tools/renovate/bot-config.json5 (config.test.ts pins them together). + // ONE relock command string serves both locks — the script self-gates on + // WHICH lock changed — so one allowlist entry covers both rules, and the + // FOD refresh's entry is the same one the other refresh sites already use. postUpgradeTasks: { - commands: ["bun tools/renovate/refresh-devenv-lock.ts"], - fileFilters: ["devenv.lock"], + commands: [ + "bun tools/renovate/refresh-devenv-lock.ts", + "bun tools/renovate/refresh-fod-hashes.ts", + ], + fileFilters: ["devenv.lock", "agent-image/entrypoint.nix"], executionMode: "branch", }, }, @@ -781,19 +810,46 @@ // matching branches; the evicted top-level task's refresh-toolchain-hashes // leg self-gates on the vendored-binary versions/*.nix pins and no-ops on // go.nix anyway (go's hashes come from the overlay, not a sha256 pin), so - // nothing is lost. fileFilters is an INCLUDE allowlist — Renovate commits - // ONLY files matching it — and the refresh writes exactly devenv.lock (the - // `devenv update go-overlay` re-lock); it does NOT rewrite go.nix (the go - // manager already did) nor any hash pin, so devenv.lock is the sole entry. + // nothing is lost there — but the eviction ALSO drops the top-level FOD-hash + // refresh, whose agent-image/entrypoint.nix entry gates on devenv.lock (a + // declared trigger in refresh-fod-hashes.ts's FOD_ENTRIES), the very file + // this task rewrites. So the refresh is re-declared below, AFTER the overlay + // re-lock. Order is LOAD-BEARING: the refresh realises the pin against the + // lock as it stands on disk, so the reverse order would realise against the + // still-at-base lock and then have that lock rewritten underneath it, + // shipping a pin that content-addresses the OLD closure — the silent form of + // the `hash mismatch in fixed-output derivation` break it exists to prevent. // - // Self-hosted bot config MUST allow the command: - // allowedCommands: [… , "^bun tools/renovate/refresh-go-overlay\\.ts$"] - // see tools/renovate/bot-config.json5 (config.test.ts pins the two together). + // Carrying it here is FAIL-SAFE, not a fix for a live break: `devenv update + // go-overlay` re-locks ONE non-nixpkgs input, so it does not move the + // nixpkgs rev that supplies pkgs.bun — the FOD's builder — and today's pin + // stays valid. But the coupling is declared at FILE granularity (devenv.lock + // IS a trigger of that entry) and the causal argument is one input-graph + // refactor away from being wrong. The refresh self-gates per entry, so the + // price of being safe is one no-op. + // + // fileFilters is an INCLUDE allowlist — Renovate commits ONLY files matching + // it — and it names exhaustively what this task writes: devenv.lock (the + // `devenv update go-overlay` re-lock) and the FOD file carrying the + // recomputed pin. It does NOT rewrite go.nix (the go manager already did) + // nor any hash pin beyond that FOD; guest-image/default.nix and flake.nix + // would be dead surface, since the Go entry's triggers are go/go.mod and + // go/go.sum, which this task never writes. Listing LESS silent-drops the + // re-lock or the refreshed pin. + // + // Self-hosted bot config MUST allow both commands: + // allowedCommands: [… , "^bun tools/renovate/refresh-go-overlay\\.ts$", + // "^bun tools/renovate/refresh-fod-hashes\\.ts$"] + // see tools/renovate/bot-config.json5 (config.test.ts pins them together); + // the FOD refresh's entry is the same one its other sites already use. matchManagers: ["custom.regex"], matchDepNames: ["go"], postUpgradeTasks: { - commands: ["bun tools/renovate/refresh-go-overlay.ts"], - fileFilters: ["devenv.lock"], + commands: [ + "bun tools/renovate/refresh-go-overlay.ts", + "bun tools/renovate/refresh-fod-hashes.ts", + ], + fileFilters: ["devenv.lock", "agent-image/entrypoint.nix"], executionMode: "branch", }, }, diff --git a/tools/renovate/config.test.ts b/tools/renovate/config.test.ts index 3282cbd8..c212932b 100644 --- a/tools/renovate/config.test.ts +++ b/tools/renovate/config.test.ts @@ -3,6 +3,12 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import botConfig from "./bot-config.json5"; import config from "./config.json5"; +// The shipped FOD table. Imported (not restated) so the coverage guard at the +// end of this file re-derives its requirement from the declaration itself. +// Side-effect-safe: that module does no I/O at import (its main() is behind an +// `import.meta.main` guard); the only load-time work is a fragment-disjointness +// assertion over this same table. +import { FOD_ENTRIES } from "./refresh-fod-hashes.ts"; // Guard suite for compass's self-hosted Renovate config (RIG-2432). Ported from // the internal monorepo's ci/renovate config.test.ts and adapted to compass's config + @@ -190,12 +196,11 @@ describe("tools/renovate postUpgradeTasks ↔ allowedCommands (RIG-2432)", () => // `allowedCommands` allowlist (a repo config cannot self-authorize a command), // which Renovate matches UNANCHORED via regEx(pattern).test(cmd). So each // entry's `^…$` IS the security property. Compass declares six DISTINCT - // commands across the task sites (the FOD-hash refresh rides three sites — the - // top-level branch-mode task, the catalog rule's update-mode task, and the - // devenv-nixpkgs branch-mode lockstep task — so it appears three times in the - // declared list but needs only one allowlist entry; the devenv-fork relock - // likewise rides BOTH devenv-fork rules under one command string, since the - // script self-gates on which lock changed); every + // commands across the task sites (the FOD-hash refresh rides FIVE sites — see + // the per-site enumeration on the count test below — so it appears five times + // in the declared list but needs only one allowlist entry; the devenv-fork + // relock likewise rides BOTH devenv-fork rules under one command string, since + // the script self-gates on which lock changed); every // distinct command must be permitted, every entry must be used, and no entry may // be an unanchored substring rule. RIG-3100 added the fifth: the go↔go-overlay // lockstep on the go pin's solo branch. RIG-2815 added the sixth: the @@ -209,14 +214,22 @@ describe("tools/renovate postUpgradeTasks ↔ allowedCommands (RIG-2432)", () => expect(allowed).toHaveLength(6); }); - test("the fod-hash refresh is declared at all three task sites", () => { - // The command must ride every task shape that can own a dependency bump: - // the top-level branch-mode slot, the catalog rule's update-mode pass, and - // the devenv-nixpkgs branch-mode lockstep task. Catalog-first branches evict - // the top-level slot, while channel branches use the devenv-nixpkgs slot; the - // raw command list therefore carries the FOD refresh once at each site. + test("the fod-hash refresh is declared at all five task sites", () => { + // The command must ride every task shape that can own a bump able to move a + // pinned FOD, because a rule-level task REPLACES the top-level one on its + // branch. The five sites, all carrying the refresh: + // 1. top-level (branch mode) — gomod + bun/TypeScript-first branches + // 2. devenv-nixpkgs channel rule — the channel moves pkgs.bun + // 3. devenv fork (root) rule — relocks devenv.lock, a declared trigger + // 4. go ↔ go-overlay lockstep — relocks devenv.lock likewise + // 5. workspaces.catalog rule — update mode, eviction-proof + // Sites 3 and 4 carry it fail-safe: each relocks ONE non-nixpkgs input, so + // neither moves pkgs.bun today — but each writes a declared trigger of the + // entrypoint.nix entry, so the coupling holds at file granularity and the + // refresh self-gates to a no-op when nothing moved. The generalized guard at + // the end of this file is what keeps that property true for future sites. const fod = "bun tools/renovate/refresh-fod-hashes.ts"; - expect(commands.filter((c) => c === fod)).toHaveLength(3); + expect(commands.filter((c) => c === fod)).toHaveLength(5); const topLevel = cfg.postUpgradeTasks?.commands ?? []; expect(topLevel).toContain(fod); const catalogRule = cfg.packageRules.find( @@ -276,7 +289,10 @@ describe("tools/renovate FOD-hash refresh wiring (PR #579)", () => { // image build fails `hash mismatch in fixed-output derivation`. refresh-fod- // hashes.ts recomputes it, but Renovate only COMMITS files a task's fileFilters // name — so a task that rewrites a FOD file without listing it silently drops - // the fix and the bump PR still goes red. Guard all three sites' fileFilters. + // the fix and the bump PR still goes red. This describe pins the two + // eviction-critical sites named below site-by-site; the generalized guard at + // the end of this file covers the property across EVERY task site, present and + // future, deriving what each must declare from FOD_ENTRIES itself. const FOD = "bun tools/renovate/refresh-fod-hashes.ts"; const topLevel = cfg.postUpgradeTasks; const catalogRule = cfg.packageRules.find( @@ -586,12 +602,20 @@ describe("tools/renovate devenv fork currency (RIG-2815, RIG-2546 T7)", () => { // substring (the devenv-nixpkgs channel manager pattern-matches the same root // lock). const RELOCK = "bun tools/renovate/refresh-devenv-lock.ts"; + const FOD = "bun tools/renovate/refresh-fod-hashes.ts"; const forkScopes: { label: string; depName: string; lock: string; groupName: string; patternLiteral: string; + // The rule's WHOLE declared task, pinned literally per scope. The two + // diverge: the root lock (devenv.lock) is a declared trigger of the + // entrypoint.nix FOD entry, so that rule must also carry the FOD refresh + // and name the FOD file; the agent-image lock is not a declared trigger + // (a separately-filed gap), so its task stays relock-only. + taskCommands: string[]; + taskFileFilters: string[]; }[] = [ { label: "root", @@ -599,6 +623,8 @@ describe("tools/renovate devenv fork currency (RIG-2815, RIG-2546 T7)", () => { lock: "devenv.lock", groupName: "devenv fork (root)", patternLiteral: "/^devenv\\.lock$/", + taskCommands: [RELOCK, FOD], + taskFileFilters: ["devenv.lock", "agent-image/entrypoint.nix"], }, { label: "agent-image", @@ -606,6 +632,8 @@ describe("tools/renovate devenv fork currency (RIG-2815, RIG-2546 T7)", () => { lock: "agent-image/devenv.lock", groupName: "devenv fork (agent-image)", patternLiteral: "/^agent-image\\/devenv\\.lock$/", + taskCommands: [RELOCK], + taskFileFilters: ["agent-image/devenv.lock"], }, ]; const managerFor = (depName: string) => @@ -702,18 +730,21 @@ describe("tools/renovate devenv fork currency (RIG-2815, RIG-2546 T7)", () => { }, ); - // Branch-mode relock over exactly the ONE lock the rule governs. fileFilters - // is an INCLUDE allowlist — Renovate commits ONLY listed files — so naming - // the sibling lock would be dead surface and naming LESS would silent-drop - // the relock, shipping a rev bump whose narHash/lastModified never moved (the - // same silent-drop mode the FOD guard above documents). + // Branch-mode relock over exactly the files the rule writes. fileFilters is an + // INCLUDE allowlist — Renovate commits ONLY listed files — so naming the + // sibling lock would be dead surface and naming LESS would silent-drop the + // relock, shipping a rev bump whose narHash/lastModified never moved (the same + // silent-drop mode the FOD guard above documents). The root scope additionally + // carries the FOD refresh, relock-FIRST: its lock is a declared trigger of the + // entrypoint.nix pin, and the reverse order would realise that pin against the + // still-at-base lock. The literal pins below turn a reordering into a red test. test.each(forkScopes)( - "the $label relock postUpgradeTask is branch-mode over its lock alone", - ({ depName, lock }) => { + "the $label relock postUpgradeTask is branch-mode over the files it writes", + ({ depName, taskCommands, taskFileFilters }) => { const task = ruleFor(depName)?.postUpgradeTasks; expect(task?.executionMode).toBe("branch"); - expect(task?.fileFilters).toEqual([lock]); - expect(task?.commands).toEqual([RELOCK]); + expect(task?.fileFilters).toEqual(taskFileFilters); + expect(task?.commands).toEqual(taskCommands); }, ); @@ -843,18 +874,28 @@ describe("tools/renovate go ↔ go-overlay lockstep (RIG-3100)", () => { expect(goOverlayRule?.matchDepNames).toContain("go"); }); - // Branch-mode task over exactly devenv.lock — the sole file the refresh - // writes (the `devenv update go-overlay` re-lock). It must NOT rewrite go.nix - // (the go manager already did) nor any hash pin, so listing anything else - // would be dead filter surface; listing LESS would silent-drop the re-lock, - // shipping a go bump the overlay can't resolve → the exact CI red this task - // exists to prevent. fileFilters is an INCLUDE allowlist, so this pins it. - test("the lockstep postUpgradeTask is branch-mode over devenv.lock alone", () => { + // Branch-mode task over exactly the two files it writes: devenv.lock (the + // `devenv update go-overlay` re-lock) and the bun outputHash pin that re-lock + // invalidates by declaration — devenv.lock is a trigger of the entrypoint.nix + // FOD entry, so the refresh rides here and its recomputed pin needs a filter + // slot to be committed. It must NOT rewrite go.nix (the go manager already + // did) nor any other hash pin, so listing anything else — guest-image/ + // default.nix or flake.nix, whose entry triggers on go/go.mod|go.sum — would be + // dead filter surface; listing LESS would silent-drop the re-lock (shipping a + // go bump the overlay can't resolve → the exact CI red this task exists to + // prevent) or the refreshed pin. fileFilters is an INCLUDE allowlist, so this + // pins it. The command order is load-bearing: re-lock FIRST, so the pin is + // realised against the written lock and not the still-at-base one. + test("the lockstep postUpgradeTask is branch-mode over the files it writes", () => { const task = goOverlayRule?.postUpgradeTasks; expect(task?.executionMode).toBe("branch"); - expect(task?.fileFilters).toEqual(["devenv.lock"]); + expect(task?.fileFilters).toEqual([ + "devenv.lock", + "agent-image/entrypoint.nix", + ]); expect(task?.commands).toEqual([ "bun tools/renovate/refresh-go-overlay.ts", + "bun tools/renovate/refresh-fod-hashes.ts", ]); }); @@ -1319,3 +1360,109 @@ describe("tools/renovate self-pin workflow (exact Renovate version)", () => { expect(/^\s*RENOVATE_TOKEN:\s*\S/m.test(stepBody)).toBe(true); }); }); + +describe("tools/renovate FOD trigger coverage (every task site, derived from FOD_ENTRIES)", () => { + // THE CLASS THIS GUARDS. A pinned Nix fixed-output-derivation hash content- + // addresses a fetched dependency set, so it is invalidated by a change to any + // of the manifests refresh-fod-hashes.ts declares as that entry's `triggers`. + // A postUpgradeTask that names such a trigger in its fileFilters is declaring + // "this task may commit a change to that manifest" — and Renovate commits ONLY + // files a task's fileFilters names, so such a task ships the lock change while + // the pin beside it still addresses the OLD closure. The image build then fails + // `hash mismatch in fixed-output derivation` and the bump PR goes red — the + // failure that kept PR #580 red for weeks. + // + // So the requirement is structural, not per-site: any site that can write a + // declared trigger must ALSO run the refresh (to recompute the pin) and name + // that entry's FOD file plus every mirrorFile (or Renovate drops the recomputed + // pin on the floor — the RIG-2852 Gap 1 silent-drop shape). The refresh + // self-gates per entry, so a site where the trigger did not actually move pays + // only a no-op; there is no cost that justifies leaving a site uncovered. + // + // The trigger sets are READ from FOD_ENTRIES rather than restated here, so + // adding a trigger or a new pinned FOD re-derives the requirement over every + // site automatically instead of needing a matching test edit. + // + // DETECTION BOUNDARY. What this reads is a site's fileFilters — i.e. the files + // the TASK declares it may commit. That misses the case where the trigger is + // written by Renovate's own manager update rather than by the task, because a + // manager's writes are never declared in fileFilters. The live instance is the + // guest-image/default.nix vendorHash: its triggers are go/go.mod and go/go.sum, + // which the gomod manager writes, so NO site names them and the pair below is + // empty for that entry — yet the pin still moves and the top-level task still + // has to refresh it. That leg is covered by assertion instead of derivation + // (the top-level site is pinned to carry the refresh, and gomod branches fall + // to the top-level slot because no rule-level task matches the gomod manager). + // A future rule-level task that matched gomod deps would evict that slot and + // this guard would NOT flag it; such a rule must carry the refresh, the FOD + // file, and its mirrorFiles by hand. + const FOD = "bun tools/renovate/refresh-fod-hashes.ts"; + + // Every declared task site, enumerated exactly as allDeclaredCommands does: + // the top-level task plus every packageRule-level one. Labelled so a failure + // names the offending site rather than an index alone. + const taskSites: { label: string; task: PostUpgradeTasks }[] = []; + if (cfg.postUpgradeTasks) { + taskSites.push({ label: "top-level", task: cfg.postUpgradeTasks }); + } + cfg.packageRules.forEach((rule, i) => { + const task = rule.postUpgradeTasks; + if (!task) return; + const group = rule.groupName; + taskSites.push({ + label: + group != null ? `packageRules[${i}] (${group})` : `packageRules[${i}]`, + task, + }); + }); + + // The (site, entry) pairs the coupling actually binds: a site whose declared + // write-set names at least one of that entry's triggers. + const coupled = taskSites.flatMap(({ label, task }) => { + const filters = task.fileFilters ?? []; + return FOD_ENTRIES.filter((entry) => + entry.triggers.some((trigger) => filters.includes(trigger)), + ).map((entry) => ({ label, task, entry })); + }); + + test("at least one task site is coupled to a FOD entry (guard is not vacuous)", () => { + // Without this, a refactor that renamed a trigger — or dropped every + // trigger from every fileFilters — would leave the guard below silently + // passing over an empty set while the real coupling went unchecked. + expect(coupled.length).toBeGreaterThan(0); + expect(taskSites.length).toBeGreaterThan(0); + }); + + test("every task site naming a FOD trigger runs the refresh and commits the pin", () => { + const violations: string[] = []; + for (const { label, task, entry } of coupled) { + const filters = task.fileFilters ?? []; + const named = entry.triggers.filter((t) => filters.includes(t)); + const context = + `${label} declares FOD trigger(s) ${named.join(", ")} for the pin in ` + + `${entry.file}`; + if (!(task.commands ?? []).includes(FOD)) { + violations.push( + `${context}, but does NOT run '${FOD}'. A task that commits a ` + + `trigger change without recomputing the pin ships a lockfile ` + + `change beside a hash addressing the OLD closure — the image ` + + `build then fails 'hash mismatch in fixed-output derivation'. ` + + `Append the refresh AFTER the command that writes the trigger ` + + `(the pin must be realised against the written file, not the ` + + `still-at-base one), or drop the trigger from fileFilters.`, + ); + } + for (const required of [entry.file, ...(entry.mirrorFiles ?? [])]) { + if (!filters.includes(required)) { + violations.push( + `${context}, but its fileFilters omit '${required}'. ` + + `fileFilters is an INCLUDE allowlist, so the refreshed pin is ` + + `recomputed and then silently DROPPED, and the bump lands with ` + + `the stale pin exactly as if no refresh ran.`, + ); + } + } + } + expect(violations).toEqual([]); + }); +}); From fcc000728452a2c75d26d571a95b6046e15c8698 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sun, 6 Sep 2026 03:14:30 -0400 Subject: [PATCH 5/5] fix(renovate): glob-match FOD trigger coupling, bind refresh ordering (RIG-3360) The generalized coverage guard bound membership but not order, matched fileFilters by string equality where Renovate specifies globs, and its non-vacuity floor could not see a partial trigger rename thin the coupled set. Correct all three, hoist the single FOD command literal, and re-ground the realise-cost and go-site reversed-order prose. Co-authored-by: Matt Wilkinson --- tools/renovate/bot-config.json5 | 9 +- tools/renovate/config.json5 | 19 +-- tools/renovate/config.test.ts | 172 +++++++++++++++++++-------- tools/renovate/refresh-fod-hashes.ts | 9 +- 4 files changed, 143 insertions(+), 66 deletions(-) diff --git a/tools/renovate/bot-config.json5 b/tools/renovate/bot-config.json5 index 1a989850..27213769 100644 --- a/tools/renovate/bot-config.json5 +++ b/tools/renovate/bot-config.json5 @@ -102,10 +102,11 @@ // (agent-image/entrypoint.nix) on a bun/catalog bump OR a devenv-nixpkgs // channel bump (which moves pkgs.bun, the FOD's builder). Left stale the // image build fails `hash mismatch in fixed-output derivation` (PR #579). - // Wired at the top-level (branch mode), catalog-rule (update mode), and - // devenv-nixpkgs channel rule (branch mode) postUpgradeTasks in - // config.json5 — one command string, so ONE allowlist entry covers all - // three sites. + // Wired at the top-level (branch mode), catalog-rule (update mode), + // devenv-nixpkgs channel rule (branch mode), devenv fork (root) rule + // (branch mode), and go↔go-overlay lockstep rule (branch mode) + // postUpgradeTasks in config.json5 — one command string, so ONE + // allowlist entry covers all five sites. // 5. the go↔go-overlay lockstep, which on a go.nix toolchain bump advances // the go-overlay input in devenv.lock to a rev that provides the new go // release and validates it resolves through gate-tools.nix's `langs.go` diff --git a/tools/renovate/config.json5 b/tools/renovate/config.json5 index 6d814686..84cde305 100644 --- a/tools/renovate/config.json5 +++ b/tools/renovate/config.json5 @@ -727,8 +727,8 @@ // installed tree, hence today's pin, is unchanged. But the coupling is // declared at FILE granularity (devenv.lock IS a trigger of that entry), // and a fork bump that altered how `pkgs` is constructed would falsify the - // causal argument silently. The refresh self-gates per entry, so the price - // of being safe is one no-op. + // causal argument silently. The gate fires, so the price of being safe is + // one extra rootfs realise per branch whose write is then a no-op. // // fileFilters is an INCLUDE allowlist — Renovate commits ONLY matching // files — and it names exhaustively the files this task can write: the lock @@ -814,19 +814,20 @@ // refresh, whose agent-image/entrypoint.nix entry gates on devenv.lock (a // declared trigger in refresh-fod-hashes.ts's FOD_ENTRIES), the very file // this task rewrites. So the refresh is re-declared below, AFTER the overlay - // re-lock. Order is LOAD-BEARING: the refresh realises the pin against the - // lock as it stands on disk, so the reverse order would realise against the - // still-at-base lock and then have that lock rewritten underneath it, - // shipping a pin that content-addresses the OLD closure — the silent form of - // the `hash mismatch in fixed-output derivation` break it exists to prevent. + // re-lock. Order is LOAD-BEARING: relock FIRST, refresh SECOND. The go + // manager changes go.nix, not devenv.lock, so reversing the order leaves + // devenv.lock byte-identical to base; the refresh is silently skipped by + // its clean self-gate, then `devenv update go-overlay` rewrites the lock. + // The conclusion is unchanged: relock first, refresh second, and the + // literal command pin above enforces that order. // // Carrying it here is FAIL-SAFE, not a fix for a live break: `devenv update // go-overlay` re-locks ONE non-nixpkgs input, so it does not move the // nixpkgs rev that supplies pkgs.bun — the FOD's builder — and today's pin // stays valid. But the coupling is declared at FILE granularity (devenv.lock // IS a trigger of that entry) and the causal argument is one input-graph - // refactor away from being wrong. The refresh self-gates per entry, so the - // price of being safe is one no-op. + // refactor away from being wrong. The gate fires, so the price of being + // safe is one extra rootfs realise per branch whose write is then a no-op. // // fileFilters is an INCLUDE allowlist — Renovate commits ONLY files matching // it — and it names exhaustively what this task writes: devenv.lock (the diff --git a/tools/renovate/config.test.ts b/tools/renovate/config.test.ts index c212932b..5451da6a 100644 --- a/tools/renovate/config.test.ts +++ b/tools/renovate/config.test.ts @@ -75,6 +75,11 @@ const bot = botConfig as { // tools/renovate/ → repo root is two levels up. const repoRoot = join(import.meta.dir, "..", ".."); +// The FOD-hash refresh command, declared once. It rides FIVE task sites in +// config.json5 and is asserted from several describes below; a rename must be a +// single edit here, not one per assertion (a missed copy degrades quietly). +const FOD_COMMAND = "bun tools/renovate/refresh-fod-hashes.ts"; + // Every postUpgradeTasks.commands entry declared anywhere in config.json5: the // top-level task plus every packageRule-level task. const allDeclaredCommands = (): string[] => { @@ -226,17 +231,20 @@ describe("tools/renovate postUpgradeTasks ↔ allowedCommands (RIG-2432)", () => // Sites 3 and 4 carry it fail-safe: each relocks ONE non-nixpkgs input, so // neither moves pkgs.bun today — but each writes a declared trigger of the // entrypoint.nix entry, so the coupling holds at file granularity and the - // refresh self-gates to a no-op when nothing moved. The generalized guard at - // the end of this file is what keeps that property true for future sites. - const fod = "bun tools/renovate/refresh-fod-hashes.ts"; - expect(commands.filter((c) => c === fod)).toHaveLength(5); + // refresh's write is a no-op when nothing moved (the gate itself fires on + // those branches — the relocked devenv.lock IS the trigger — so the price + // is one extra realise, not nothing). The generalized guard at the end of + // this file is what keeps that coverage property true for future sites: it + // derives from FOD_ENTRIES that any site declaring a trigger must run the + // refresh LAST and name the pin's file. + expect(commands.filter((c) => c === FOD_COMMAND)).toHaveLength(5); const topLevel = cfg.postUpgradeTasks?.commands ?? []; - expect(topLevel).toContain(fod); + expect(topLevel).toContain(FOD_COMMAND); const catalogRule = cfg.packageRules.find( (r) => r.matchDepTypes?.includes("workspaces.catalog") && r.postUpgradeTasks, ); - expect(catalogRule?.postUpgradeTasks?.commands).toContain(fod); + expect(catalogRule?.postUpgradeTasks?.commands).toContain(FOD_COMMAND); }); test("every declared command is permitted by an anchored allowlist entry", () => { @@ -293,7 +301,6 @@ describe("tools/renovate FOD-hash refresh wiring (PR #579)", () => { // eviction-critical sites named below site-by-site; the generalized guard at // the end of this file covers the property across EVERY task site, present and // future, deriving what each must declare from FOD_ENTRIES itself. - const FOD = "bun tools/renovate/refresh-fod-hashes.ts"; const topLevel = cfg.postUpgradeTasks; const catalogRule = cfg.packageRules.find( (r) => @@ -301,7 +308,7 @@ describe("tools/renovate FOD-hash refresh wiring (PR #579)", () => { ); test("the top-level branch-mode task runs the fod refresh and is branch mode", () => { - expect(topLevel?.commands).toContain(FOD); + expect(topLevel?.commands).toContain(FOD_COMMAND); expect(topLevel?.executionMode).toBe("branch"); }); @@ -321,7 +328,7 @@ describe("tools/renovate FOD-hash refresh wiring (PR #579)", () => { test("the catalog rule runs the fod refresh in UPDATE mode (eviction-proof)", () => { // A catalog-first rollup branch evicts the top-level branch task, so the // refresh must also ride the catalog rule's per-upgrade update pass. - expect(catalogRule?.postUpgradeTasks?.commands).toContain(FOD); + expect(catalogRule?.postUpgradeTasks?.commands).toContain(FOD_COMMAND); expect(catalogRule?.postUpgradeTasks?.executionMode).toBe("update"); }); @@ -602,7 +609,6 @@ describe("tools/renovate devenv fork currency (RIG-2815, RIG-2546 T7)", () => { // substring (the devenv-nixpkgs channel manager pattern-matches the same root // lock). const RELOCK = "bun tools/renovate/refresh-devenv-lock.ts"; - const FOD = "bun tools/renovate/refresh-fod-hashes.ts"; const forkScopes: { label: string; depName: string; @@ -623,7 +629,7 @@ describe("tools/renovate devenv fork currency (RIG-2815, RIG-2546 T7)", () => { lock: "devenv.lock", groupName: "devenv fork (root)", patternLiteral: "/^devenv\\.lock$/", - taskCommands: [RELOCK, FOD], + taskCommands: [RELOCK, FOD_COMMAND], taskFileFilters: ["devenv.lock", "agent-image/entrypoint.nix"], }, { @@ -885,7 +891,17 @@ describe("tools/renovate go ↔ go-overlay lockstep (RIG-3100)", () => { // go bump the overlay can't resolve → the exact CI red this task exists to // prevent) or the refreshed pin. fileFilters is an INCLUDE allowlist, so this // pins it. The command order is load-bearing: re-lock FIRST, so the pin is - // realised against the written lock and not the still-at-base one. + // realised against the written lock. The failure mode under a REVERSED order + // differs here from the devenv-nixpkgs and devenv-fork sites: the go + // customManager's file scope is /^tools/toolchain/versions/go\.nix$/ + // (config.json5:319), which never touches devenv.lock — so at the point a + // reversed refresh ran, the lock would still be byte-identical to base, its + // self-gate (`git diff --quiet -- devenv.lock`, + // refresh-fod-hashes.ts:265-274) would read CLEAN, and the refresh would + // no-op without realising anything; only THEN would `devenv update + // go-overlay` rewrite the lock. So the refresh is silently SKIPPED rather + // than realised against a stale lock — a different mechanism, the same end + // state: the bump ships the stale pin. Hence the literal pin below. test("the lockstep postUpgradeTask is branch-mode over the files it writes", () => { const task = goOverlayRule?.postUpgradeTasks; expect(task?.executionMode).toBe("branch"); @@ -1376,8 +1392,15 @@ describe("tools/renovate FOD trigger coverage (every task site, derived from FOD // declared trigger must ALSO run the refresh (to recompute the pin) and name // that entry's FOD file plus every mirrorFile (or Renovate drops the recomputed // pin on the floor — the RIG-2852 Gap 1 silent-drop shape). The refresh - // self-gates per entry, so a site where the trigger did not actually move pays - // only a no-op; there is no cost that justifies leaving a site uncovered. + // self-gates per entry on TRIGGER CHANGE, and devenv.lock is exactly what + // these tasks rewrite — so on those branches the gate FIRES rather than + // passing over: firing means writing a deliberately-fake SRI and running a + // full `nix build` of compass-guest-rootfs, a guaranteed fixed-output cache + // miss that forces a networked `bun install`. What is a no-op is the + // resulting WRITE (the same SRI), not the work. So covering a site whose + // installed tree did not actually move costs one extra rootfs realise per + // such branch (refresh-fod-hashes.ts:54: "costs at most one extra realise"), + // and no site's coverage gap is worth that price. // // The trigger sets are READ from FOD_ENTRIES rather than restated here, so // adding a trigger or a new pinned FOD re-derives the requirement over every @@ -1396,7 +1419,15 @@ describe("tools/renovate FOD trigger coverage (every task site, derived from FOD // A future rule-level task that matched gomod deps would evict that slot and // this guard would NOT flag it; such a rule must carry the refresh, the FOD // file, and its mirrorFiles by hand. - const FOD = "bun tools/renovate/refresh-fod-hashes.ts"; + + // fileFilters entries are GLOB patterns (Renovate matches them as globs, e.g. + // "**/*.js"), so coupling must be decided by glob match, not string equality — + // otherwise a site that names a trigger under any non-literal spelling is + // invisible here and the exact failure class this guard exists to catch ships + // silently. Every entry is a literal path today, for which a glob match and an + // equality test coincide. + const covers = (filters: string[], path: string): boolean => + filters.some((filter) => new Bun.Glob(filter).match(path)); // Every declared task site, enumerated exactly as allDeclaredCommands does: // the top-level task plus every packageRule-level one. Labelled so a failure @@ -1421,48 +1452,89 @@ describe("tools/renovate FOD trigger coverage (every task site, derived from FOD const coupled = taskSites.flatMap(({ label, task }) => { const filters = task.fileFilters ?? []; return FOD_ENTRIES.filter((entry) => - entry.triggers.some((trigger) => filters.includes(trigger)), + entry.triggers.some((trigger) => covers(filters, trigger)), ).map((entry) => ({ label, task, entry })); }); - test("at least one task site is coupled to a FOD entry (guard is not vacuous)", () => { - // Without this, a refactor that renamed a trigger — or dropped every - // trigger from every fileFilters — would leave the guard below silently - // passing over an empty set while the real coupling went unchecked. - expect(coupled.length).toBeGreaterThan(0); + test("the coupled (site, entry) set has its expected shape (guard is not vacuous)", () => { + // The guard below iterates `coupled`, so a set that emptied or thinned out + // would leave it passing while checking nothing. A bare `> 0` cannot see + // the thinning: all four pairs today bind the SAME entry + // (agent-image/entrypoint.nix, whose triggers are bun.lock and + // devenv.lock), so renaming just ONE of that entry's two triggers + // dissolves the coupling at three of the four sites while the fourth keeps + // the count positive. Pinning the exact count catches a partial trigger + // rename, or a fileFilters edit, that dissolves any single pairing. It does + // NOT check WHICH sites are coupled — the per-site describes above pin + // that — only that the population has not shrunk or grown. A newly coupled + // site is a deliberate edit: update this number in the same change. + expect(coupled.length).toBe(4); expect(taskSites.length).toBeGreaterThan(0); }); - test("every task site naming a FOD trigger runs the refresh and commits the pin", () => { - const violations: string[] = []; - for (const { label, task, entry } of coupled) { - const filters = task.fileFilters ?? []; - const named = entry.triggers.filter((t) => filters.includes(t)); - const context = - `${label} declares FOD trigger(s) ${named.join(", ")} for the pin in ` + - `${entry.file}`; - if (!(task.commands ?? []).includes(FOD)) { - violations.push( - `${context}, but does NOT run '${FOD}'. A task that commits a ` + - `trigger change without recomputing the pin ships a lockfile ` + - `change beside a hash addressing the OLD closure — the image ` + - `build then fails 'hash mismatch in fixed-output derivation'. ` + - `Append the refresh AFTER the command that writes the trigger ` + - `(the pin must be realised against the written file, not the ` + - `still-at-base one), or drop the trigger from fileFilters.`, - ); - } - for (const required of [entry.file, ...(entry.mirrorFiles ?? [])]) { - if (!filters.includes(required)) { - violations.push( - `${context}, but its fileFilters omit '${required}'. ` + - `fileFilters is an INCLUDE allowlist, so the refreshed pin is ` + - `recomputed and then silently DROPPED, and the bump lands with ` + - `the stale pin exactly as if no refresh ran.`, - ); - } + // Violation messages, built outside the scan loop: each states the failure + // mechanism a maintainer needs, and keeping them here leaves the loop below + // as the three predicates it checks. + const missingRefresh = (context: string): string => + `${context}, but does NOT run '${FOD_COMMAND}'. A task that commits ` + + `a trigger change without recomputing the pin ships a lockfile ` + + `change beside a hash addressing the OLD closure — the image ` + + `build then fails 'hash mismatch in fixed-output derivation'. ` + + `Append the refresh AFTER the command that writes the trigger ` + + `(the pin must be realised against the written file, not the ` + + `still-at-base one), or drop the trigger from fileFilters.`; + + const refreshNotLast = ( + context: string, + position: number, + total: number, + ): string => + `${context}, but runs '${FOD_COMMAND}' at position ${position} ` + + `of ${total} instead of LAST. The refresh realises the ` + + `pin against the trigger files as they stand on disk, so it must ` + + `run after EVERY command that writes a trigger. A refresh sitting ` + + `before a relock either realises against the still-at-base file ` + + `and then has that file rewritten underneath it, or — when no ` + + `earlier command has moved a trigger yet — self-gates clean and ` + + `no-ops entirely without realising anything. Either way the bump ` + + `ships the stale pin. Move the refresh to the end of commands.`; + + const pinDropped = (context: string, required: string): string => + `${context}, but its fileFilters omit '${required}'. ` + + `fileFilters is an INCLUDE allowlist, so the refreshed pin is ` + + `recomputed and then silently DROPPED, and the bump lands with ` + + `the stale pin exactly as if no refresh ran.`; + + // Every way a single (site, entry) pairing can fail the coupling: the refresh + // missing, the refresh not last, or a file the pin needs missing from the + // include allowlist. + const violationsFor = ({ + label, + task, + entry, + }: (typeof coupled)[number]): string[] => { + const filters = task.fileFilters ?? []; + const commands = task.commands ?? []; + const named = entry.triggers.filter((t) => covers(filters, t)); + const context = + `${label} declares FOD trigger(s) ${named.join(", ")} for the pin in ` + + `${entry.file}`; + const fodIndex = commands.indexOf(FOD_COMMAND); + const found: string[] = []; + if (fodIndex === -1) { + found.push(missingRefresh(context)); + } else if (fodIndex !== commands.length - 1) { + found.push(refreshNotLast(context, fodIndex + 1, commands.length)); + } + for (const required of [entry.file, ...(entry.mirrorFiles ?? [])]) { + if (!covers(filters, required)) { + found.push(pinDropped(context, required)); } } - expect(violations).toEqual([]); + return found; + }; + + test("every task site naming a FOD trigger runs the refresh LAST and commits the pin", () => { + expect(coupled.flatMap(violationsFor)).toEqual([]); }); }); diff --git a/tools/renovate/refresh-fod-hashes.ts b/tools/renovate/refresh-fod-hashes.ts index 3b2ff6b3..eaf25309 100755 --- a/tools/renovate/refresh-fod-hashes.ts +++ b/tools/renovate/refresh-fod-hashes.ts @@ -53,14 +53,17 @@ // Idempotent: re-running rewrites the same SRI — a no-op write when the realised // tree is unchanged (so gating on devenv.lock costs at most one extra realise). // -// Wired from config.json5 at THREE sites, all the same +// Wired from config.json5 at FIVE sites, all the same // `bun tools/renovate/refresh-fod-hashes.ts` command (allowlisted once in // bot-config.json5; config.test.ts pins them together): top-level // postUpgradeTasks (branch mode — gomod branches and pure-bun-first // TypeScript-rollup branches), the catalog packageRule (update mode — // catalog-first rollup branches, where the collapsed branch config evicts the -// top-level branch task), and the devenv-nixpkgs channel rule (branch mode — a -// channel bump moves pkgs.bun; see that rule's note). +// top-level branch task), the devenv-nixpkgs channel rule (branch mode — a +// channel bump moves pkgs.bun; see that rule's note), the devenv fork (root) +// rule (branch mode — relocks devenv.lock; a declared trigger), and the go ↔ +// go-overlay lockstep rule (branch mode — relocks devenv.lock via +// `devenv update go-overlay`; a declared trigger). // // Requires `nix` (nix-command) + `bun` + `git` on PATH and network. The build is // self-contained: `nix build` fetches the Go/bun toolchains it needs into the