diff --git a/tools/renovate/bot-config.json5 b/tools/renovate/bot-config.json5 index ab0fb6bc..27213769 100644 --- a/tools/renovate/bot-config.json5 +++ b/tools/renovate/bot-config.json5 @@ -99,11 +99,14 @@ // 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), + // 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 374b8c6b..84cde305 100644 --- a/tools/renovate/config.json5 +++ b/tools/renovate/config.json5 @@ -614,31 +614,39 @@ // 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 (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 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", }, @@ -698,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. // - // 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. + // 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 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 + // 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", }, }, @@ -773,19 +810,47 @@ // 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: 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. // - // 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 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 + // `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 c6df3a45..5451da6a 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 + @@ -69,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[] => { @@ -190,11 +201,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 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 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 @@ -208,21 +219,32 @@ 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. - const fod = "bun tools/renovate/refresh-fod-hashes.ts"; - expect(commands.filter((c) => c === fod)).toHaveLength(2); + 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'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", () => { @@ -275,8 +297,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 both sites' fileFilters. - const FOD = "bun tools/renovate/refresh-fod-hashes.ts"; + // 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 topLevel = cfg.postUpgradeTasks; const catalogRule = cfg.packageRules.find( (r) => @@ -284,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"); }); @@ -304,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"); }); @@ -506,20 +530,29 @@ 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. - 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: 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"); expect(task?.fileFilters).toEqual([ @@ -528,6 +561,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 — @@ -537,17 +571,11 @@ 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?.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).toEqual([ + "bun tools/renovate/refresh-devenv-nixpkgs.ts", "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 @@ -587,6 +615,13 @@ describe("tools/renovate devenv fork currency (RIG-2815, RIG-2546 T7)", () => { 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", @@ -594,6 +629,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_COMMAND], + taskFileFilters: ["devenv.lock", "agent-image/entrypoint.nix"], }, { label: "agent-image", @@ -601,6 +638,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) => @@ -697,18 +736,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); }, ); @@ -838,18 +880,38 @@ 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. 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"); - 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", ]); }); @@ -1314,3 +1376,165 @@ 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 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 + // 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. + + // 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 + // 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) => covers(filters, trigger)), + ).map((entry) => ({ label, task, entry })); + }); + + 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); + }); + + // 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)); + } + } + 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.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..eaf25309 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,23 @@ // // 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 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), 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 @@ -113,7 +126,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"], }, ];