feat(browsers): install the Microsoft 365 web apps through firefoxpwa - #435
feat(browsers): install the Microsoft 365 web apps through firefoxpwa#435Bad3r wants to merge 71 commits into
Conversation
firefoxpwa keeps its site list in a profile database under $XDG_DATA_HOME/firefoxpwa, so a suite clicked together through the browser extension survives neither a reinstall nor a move to another host. packages/firefoxpwa-m365-install reuses the DMail installer's site bookkeeping (the ulid, origin and applied-URL records kept next to config.json, and the pending record that covers the kill window inside `firefoxpwa site install`) and adds a per-entry loop: an entry that cannot be installed is counted and reported at the end instead of aborting the run, so one refusal does not cost the remaining entries their install. Start URLs are the bare cloud.microsoft origins. The manifest scope is fixed at install time and `firefoxpwa site update` cannot rewrite it, so scope has to be the origin, and the landing paths Microsoft redirects to (/en-us/, /mail/, /tasks/) are locale or tenant dependent. Three subdomains are left out of the catalog after checking them on 2026-08-04: teams.cloud.microsoft answers Gecko with /v2/unsupported-browser, visio.cloud.microsoft redirects to m365.cloud.microsoft and would leave its own scope on first load, and clipchamp.cloud.microsoft does not resolve. programs.firefoxpwa.m365.enable defaults to false and no host sets it yet, so m365-check.nix drives the shipped catalog through a stub firefoxpwa: nothing else builds that installer or would notice a catalog entry that breaks it. Validation: nix fmt nix flake check path:. --accept-flake-config --no-build nix build path:.#checks.x86_64-linux."browsers/firefoxpwa-m365" nix eval path:.#checks.x86_64-linux."browsers/firefoxpwa-module-eval".drvPath nix develop path:. --fallback -c pre-commit run --files <changed files>
|
Claude finished @Bad3r's task in 12m 3s —— View job Review complete
Two findings, both
Nothing else surfaced. I traced the installer state machine in The |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e05b9197a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The leftover-temporary assertion added with the Microsoft 365 installer check reported a pass on every run while checking nothing: the bash a runCommand builder runs is built without programmable completion, so `compgen -G` resolved to no command at all, and a condition context turns that into a false, not an error, which took the else branch. Probed both interpreters to confirm the split: `type -t compgen` in a plain runCommand reports MISSING, while a bash from nativeBuildInputs reports HAVE, so nothing about the builtin is reliable across the two. tests/prune-old-stashes/run.sh depended on the same builtin for its defined-but-never-ran guard, the one thing that stops a test function from being added and silently never called. It passes today only because modules/meta/script-tests.nix puts pkgs.bash (resolved to bash-interactive) on PATH; `declare -F` is a plain builtin and holds whatever the harness runs under. m365-check.nix now proves the detector before it trusts it: a planted `.next` file has to be reported, and only then is an empty result read as a clean directory. checks.build-time-shell scans modules/, packages/ and tests/ for the builtin at a command position, and plants a fixture the scan has to hit before scanning the tree, so the guard cannot go quiet the same way the assertion it exists for did. scripts/ is out of scope: it runs under the user's own bash. Validation: nix build path:.#checks.x86_64-linux."browsers/firefoxpwa-m365" nix build path:.#checks.x86_64-linux.script-tests-prune-old-stashes (47 passed) nix build path:.#checks.x86_64-linux.build-time-shell, clean tree and with a planted tests/_scratch/probe.sh (exit 1) nix flake check path:. --accept-flake-config --no-build nix develop path:. --fallback -c pre-commit run --files <changed files>
statix reached only staged files. modules/meta/pre-commit.nix wires it through hook-statix, which pre-commit invokes with filenames, and nothing in .github/workflows/check.yml runs statix at all, so a lint in a file nobody edits again was reachable on main indefinitely. The repeated `options` assignment fixed in modules/browsers/firefoxpwa/apps.nix on this branch was caught only because that file happened to be staged. checks.statix-tree runs hook-statix with no arguments, its own whole-tree branch, so CI and pre-commit share one binary and one set of lints rather than drifting. passthru.runtimeCheck opts it into the workflow's runtime build step, which discovers checks by that marker, so no workflow edit is needed. The source is narrowed to .nix files through lib.fileset, keeping the check off the rebuild path of commits that touch nothing statix reads. The tree is clean under this check today, verified with `statix check .` over the repo before adding it. Validation: nix build path:.#checks.x86_64-linux.statix-tree nix flake check path:. --accept-flake-config --no-build nix develop path:. --fallback -c pre-commit run --files modules/meta/hooks/statix.nix
install_app ran as the left side of `|| failed=$((failed + 1))`, and bash ignores -e for the whole body of a function invoked that way, including every compound command inside it. A `record "$ulid_file" "$ulid"` that failed on a full or read-only filesystem was therefore silent: the next line removed the pending record, the entry was reported installed, and every later run took the "is not the site this unit installed" branch, which has no recovery short of `firefoxpwa site uninstall`. Reported independently by two reviewers on PR #435. The per-entry status is now collected by a `refuse` helper that counts the states the loop is meant to absorb, so install_app is called as a plain command and errexit covers the record writes, the pending write, the manifest jq and the base64. A refusal is still counted and does not cost the other entries their install; a failed write aborts the run with the pending record intact, which is what lets the next run adopt the registered site instead of refusing it. m365-check.nix plants a directory where the ulid record's temporary goes, so the redirection fails the way a full filesystem would, and asserts the run stops, leaves the pending record, and recovers on the next run. Restoring the `||` call site turns all three assertions red. Validation: nix build path:.#checks.x86_64-linux."browsers/firefoxpwa-m365" (51 assertions), nix flake check path:. --accept-flake-config --no-build --offline
The installer returns non-zero for a site that registered and then failed, on the stated expectation that the next run repairs it with `site update`. Nothing produced that run. sd-switch restarts a user unit only when the unit's own text changed, and firefoxpwa-m365.service's text changes only with the app table, dataDir or the firefoxpwa package, so a run that failed with an unchanged table stayed in `failed` (RemainAfterExit keeps it there) until the next login. ./dmail.nix does not have the gap because PartOf = sops-nix.service, restarted by sops-nix on every switch, reruns it; this unit is ordered against nothing and had no equivalent. Restart = "on-failure" with RestartSec = 120, bounded by StartLimitIntervalSec = 600 and StartLimitBurst = 3, so a transient failure or a half-installed site recovers in-session while a permanent refusal (an entry moved across origins, a foreign site under a managed name) stops after three tries. systemd rejects only `always` and `on-success` for Type=oneshot: `systemd-analyze verify --user` accepts the generated unit and rejects the same unit with Restart=always. programs.firefoxpwa.m365.enable claimed the service "reports failure on every switch and login", which was never true: a switch that leaves the app table alone does not rerun the unit at all. It now names the switch that moves the entry, the bounded restarts, and every later login. module-check.nix forces the whole unit rather than ExecStart and Environment, since no host enables the toggle and nothing else evaluates the restart policy. Validation: nix eval path:.#checks.x86_64-linux."browsers/firefoxpwa-module-eval".drvPath, systemd-analyze verify --user on the rendered unit, nix flake check path:. --accept-flake-config --no-build --offline
programs.firefoxpwa.m365.apps.*.key is already strMatching, but url was a bare str, so `m365.cloud.microsoft` or `htps://...` evaluated and built. The installer's url_origin requires `<scheme>://<authority>` to derive the manifest scope, so the typo surfaced only at runtime, as "cannot derive an origin from the start URL" in a user unit's journal after a switch. strMatching "https?://[^[:space:]]+" moves it to eval, matching the assertion m365-check.nix already makes about the shipped catalog. http is admitted because url_origin handles it, including the default-port fold. Validation: nix eval on the type's check against the catalog URLs and the rejected spellings, nix flake check path:. --accept-flake-config --no-build --offline
Two review rounds on PR #435 reported that `case "$out" in *"$want_text"*) [ "$rc" = "$want_rc" ] && ok=0 ;; esac` aborts the builder under `set -o errexit` when the message fragment matches but the status does not, losing the FAIL diagnostics and every later assertion. It does not: bash ignores -e for the left side of an && list, and the enclosing compound inherits that state, so the case returning 1 is not a trigger. Confirmed in a runCommand builder on bash 5.3.15, where the FAIL branch printed and the following statements ran. Recorded in both m365-check.nix and dmail-check.nix rather than rewritten, because the rule is the same one the m365 installer's `refuse` helper exists to work around, in the opposite direction: there the inherited suppression hid a failed write, here it keeps a deliberate status comparison from killing the run. Validation: nix build path:.#checks.x86_64-linux."browsers/firefoxpwa-m365", nix build path:.#checks.x86_64-linux."browsers/firefoxpwa-dmail"
Review round 1 addressedNine inline threads, five distinct findings (the two reviewers overlapped on four). All resolved; four commits pushed. Implemented
RejectedThe claim that It is the same bash rule as the real defect above, in the opposite direction, so Beyond the literal comments
ReliabilityThe distinction the fix rests on is refusal versus fault. A refusal is a state the installer decides it must not act on (a foreign site under a managed name, a move across origins, an install that never registered); it is counted, the other entries still install, and the entry stays describable. A fault is a failed write to a record or to
Validation
|
The alternation required compgen to follow the keyword, `;`, `&&`, `||` or `$(` immediately, so a leftover assertion written as `if ! compgen -G "$out"/*.next` matched none of the three branches and passed the tree scan. That is the same fail-open idiom the check exists to catch, in the spelling an assertion that wants to report a leftover is more likely to use, so the guard was blind to half its own defect class. Verified: the negated line is reported only after the change, and the tree stays clean under the widened pattern, including the prose mentions in this file, in m365-check.nix and in tests/prune-old-stashes/run.sh. Each alternative now admits an optional `!`, which keeps the command-position anchoring that stops prose from matching. The self-test plants both spellings and requires two hits rather than a non-empty result: a fixture for one of them proves only the alternative it happens to take, which is how this gap stayed green. Validation: nix build path:.#checks.x86_64-linux.build-time-shell, and the same build with a negated tests/_scratch/probe.sh planted, which fails with the file named
The m365 unit documents a StartLimitIntervalSec bound but the module check only forced the individual policy fields. Assert the interval covers StartLimitBurst multiplied by TimeoutStartSec plus RestartSec so a future policy edit cannot silently remove the retry bound.\n\nValidated with nix fmt, nix eval path:.#checks.x86_64-linux."browsers/firefoxpwa-module-eval".drvPath, pre-commit hooks for modules/browsers/firefoxpwa/module-check.nix, and git diff --check.
The build-time-shell regex now recognizes elif and single-pipe command positions, but its planted fixture still exercised only if branches. Add dedicated elif and pipe fixtures and raise the exact hit count so either alternative cannot regress silently.\n\nValidated with nix fmt, nix build path:.#checks.x86_64-linux.build-time-shell, pre-commit hooks for modules/meta/build-time-shell.nix, and git diff --check.
installed_manifest belongs to one m365 entry, but its assignment was global inside the installer loop. Declare it with the other per-entry locals so future paths cannot carry one site's manifest authentication state into another.\n\nValidated with nix fmt, the firefoxpwa-m365 check, pre-commit hooks for packages/firefoxpwa-m365-install/default.nix, and git diff --check.
Restart=on-failure consumes the configured start burst before a corrective start can run. Document the reset-failed recovery that apps.nix already exposes so the unit policy comment does not imply a reserved retry slot.
The build-time shell scan must catch compgen after case-arm, group, and subshell operators because each command position bypasses the completion-free builder when omitted. Plant and count all seven spellings so the detector cannot pass with an untested alternation.
The expect helpers validate both the installer status and a message fragment, but status-only diagnostics hid which text assertion failed. Include the wanted fragment in both M365 and DMail failure reports so message regressions are immediately actionable.
|
Review triage for this iteration is complete. Accepted and implemented: clarified the systemd retry-budget comment and reset-failed recovery; expanded the compgen detector to case-arm, group, and subshell positions with seven planted regression fixtures; and improved both installer test helpers to report the expected output fragment on status failures. No valid feedback was rejected in this iteration. Production reliability is improved by aligning operational documentation with actual systemd behavior, keeping the detector self-tested across each supported command position, and making assertion failures actionable. Targeted build-time-shell, Firefox PWA M365, and DMail checks plus formatter and pre-commit hooks pass. |
A config.json read race completes in milliseconds, but the flat 1080-second RestartSec delayed every retry equally. Use systemd RestartSteps with a 10-second first delay and 300-second ceiling, and size the start-limit invariant from the worst-case delay so retryable faults recover sooner without losing bounded failure behavior.
docs/drafts/chromium-webapps-plan.md carried 3181 lines covering three sequentially gated PRs, so executing one phase meant loading the other two. Each phase is now its own plan file, gated on the previous one merging: - chromium-webapps-plan-1-salvage.md: Task 1, rescue the compgen and statix fixes, close PR #435. - chromium-webapps-plan-2-remove-firefoxpwa.md: Tasks 2-6, delete and unwire the firefoxpwa subsystem. - chromium-webapps-plan-3-implement.md: Tasks 7-18 plus the decisions table, the verified facts, the browser-wide policy consequences and the self-review for the series. Phase 1 and phase 2 restate only the decision rows and the file-structure block that bind them, and point at phase 3 for the full record. Task bodies are byte-identical to the source ranges apart from seven rewritten cross-references: the three `docs/drafts/chromium-webapps-plan.md` mentions inside generated PR and issue text now name the phase 3 file, the two "do not start until PR N has merged" gates name the prerequisite file, the Task 5 Step 1 back-reference is qualified as phase 2, and the self-review coverage row for the firefoxpwa cleanup points at phase 2. Validation: nix run path:.#formatter.x86_64-linux -- on the three new files reports 0 changed; diffing each new task section against its source range in the deleted file shows only the seven intended edits.
Task 1 Step 6 ran `git add` then `git commit` on a tree the cherry-pick had already committed. Step 3's `git cherry-pick "$COMPGEN_SHA" "$STATIX_SHA"` commits both changes itself, and its conflict path ends in `git cherry-pick --continue`, which commits too. So `git add` staged nothing and `git commit` exited 1 with `nothing added to commit`. The block is pasted rather than run under `set -e`, so `git push -u origin` and `gh pr create` below it still ran: the PR opened with #435's two original commit messages instead of the single message this step writes, which is the message explaining why compgen fails in runCommand bash. `git reset --soft main` before the `git add` collapses both picked commits into the index, and picks up Step 5's `nix fmt` result at the same time. Step 1 now names `main` explicitly as the branch point rather than relying on the primary checkout's HEAD, since the reset depends on it. `m365-check.nix` is absent from `main`, so Step 3's `git rm -f` conflict resolution contributes nothing to the index. Validation: `git ls-tree -r main --name-only` confirms m365-check.nix is not on main; Step 3's cherry-pick and --continue paths read directly; nix run path:.#formatter.x86_64-linux -- docs/drafts/chromium-webapps-plan-1-salvage.md
…raft saw Task 1 pinned the rescue to $COMPGEN_SHA and $STATIX_SHA. Its own Step 2 query, `git log origin/main..HEAD -- tests/prune-old-stashes/run.sh modules/meta/build-time-shell.nix modules/meta/hooks/statix.nix`, now returns twelve commits on feat/firefoxpwa-m365, so following the plan literally drops ten and closes #435 unmerged with them still on a branch scheduled for deletion in phase 2. What the two-SHA pick loses is the correctness of the checks it rescues. 95971cf stops the compgen scan reading grep exit 2 as a clean tree, 3a7120e and 5c90c02 and a07d58f widen it to negated and compound command positions, 8636cf3 keeps checks.statix-tree off the vendored docs/nixos-manual mirror, and 2d32a0e and fc82fc4 declare the runtime utilities both checks call. 69b1b28 plants a lint so a clean $out cannot mean a run that walked nothing, which is the "Planted-lint proof" phase 1's own generated PR body already promised and no step delivered. Step 2 reads the list from the branch and writes it to /tmp/rescue-shas.txt with --reverse, because git log prints newest first and cherry-pick replays in argument order, which applies each fix before the commit it fixes. It also checks every rescued commit for content outside the three paths: exactly one line is expected, eb19699's m365-check.nix, which Step 3's conflict path already drops. The collapsed commit message and the PR body now describe what lands. Validation: nix run path:.#formatter.x86_64-linux -- docs/drafts/chromium-webapps-plan-1-salvage.md; git log --reverse over the three paths (12 commits); per-commit name-only scan (1 line outside the three paths)
Four statements did not survive checking against the tree they describe. Phase 1 credited the rescue with adding a whole-tree branch to modules/meta/hooks/statix.nix. That branch is on main already, from commit 66ac9ba (an unrelated lefthook migration); PR #435 adds checks.statix-tree, a header comment and widened perSystem arguments. An operator hand-implementing rather than cherry-picking would have read the file, found the described behavior present, and skipped the change that matters. Phase 1 Task 1 Step 4 promised no output from `rg -n 'compgen' tests/ modules/ packages/` minus one path exclusion. On main that prints modules/hm-apps/proton-drive.nix twice, from a comment predating the branch. The step now runs the detector's own command-position predicate from modules/meta/build-time-shell.nix, verified to match exactly the one site Step 3 replaces on main and nothing on the rescued branch. Phase 3's self-review, which already named this class of defect and claimed it fixed, is corrected to say so. Phase 2 justified freezing its task numbering on cross-references phase 3 makes "in Task 13, in its docs/reference/webapps.md block and in its self-review". `rg -n 'Phase 2 Task'` over phase 3 finds two: Task 17 Step 2 and the self-review. Neither named site has one. Phase 3's --load-extension bullet was the only entry under "Verified facts this plan depends on" with no reproduction, and it underpins the Decisions table row that is marked do-not-relitigate. It now carries one, run against the pinned brave-origin 1.95.47: load-extension and disable-extensions-except are both present in the binary, and the branding-gated refusal literal is absent, which is what GOOGLE_CHROME_BRANDING unset looks like. The Enhanced Safe Browsing half is a runtime gate no string probe reaches, so Task 18 Step 3 is named as its confirmation along with the fallback if it fails. Validation: nix develop path:. -c pre-commit run --all-files --hook-stage manual
…and fix the worktree command forms (#444) * docs(agents): give every instruction surface the linked-worktree flake command forms The branch workflow in these files mandates a linked worktree, and every flake command they prescribed fails there: Lix cannot fetch a clean linked worktree as a `git+file` flake because `.git` is a file, not a directory, so `nix eval .#formatter.x86_64-linux.name` exits with `opening file '<worktree>/.git/config': Not a directory` while the `path:.` form returns `"treefmt"`. A dirty worktree masks it, because Lix copies the working tree instead of fetching the revision, so the same command passes with uncommitted changes present and exits 1 once the tree is clean. That is why the rule survived: it only fails on the clean trees that matter. Two commands cannot be fixed by appending `path:.`, and both are now spelled out: - `nix fmt` hardcodes the `.` installable in `lix/nix/fmt.cc`, so `nix fmt path:.` passes `path:.` to treefmt as a path argument and still resolves `.` as the flake. Use `nix run path:.#formatter.x86_64-linux -- .`. - `nix flake update` reads positional arguments as input names, and `--flake` rejects a relative ref with `cannot fetch input 'path:.' because it uses a relative path`. Use `nix flake update --flake "path:$PWD"`. The `&&` chain in AGENTS.md's input-update recipe short-circuited on its first command and never reached the rest. Also covers the directory-scoped files, which inherited none of this: `docs/AGENTS.md` carried six bare invocations and told the reader to "Run commands from repo root (`/home/vx/nixos`)", naming the primary checkout as the working directory in direct contradiction of the branch workflow; it now points at the root of whichever checkout holds the work. `scripts/AGENTS.md` carried a seventh. Prose that names `nix flake check` as a concept rather than prescribing a run is unchanged. Validation: nix run path:.#formatter.x86_64-linux -- CLAUDE.md AGENTS.md docs/AGENTS.md scripts/AGENTS.md (0 changed) * feat(agents): state the linked-worktree flake forms in the generated baseline CLAUDE.md's header defers baseline agent behavior to this file, so an agent following the generated baseline got the bare `nix flake check` regardless of what the repo-local instruction files say. The two `path:.` sites already here sit inside a sentence scoped "In the nixos repo" and name a package unique to it, so they do not cover the generic ladder. The bare form stays the default rather than becoming `path:.` unconditionally. This renders to `~/.claude/CLAUDE.md` and `~/.config/codex/AGENTS.md`, which apply to every repo, and `path:.` is not inert: it forces the plain-path fetcher, dropping `self.rev`/`self.dirtyRev`, so a flake deriving `configurationRevision` from it evaluates differently. A caveat is correct wherever the bare form is, and adds the linked-worktree case the file never mentioned. Carries the two commands `path:.` cannot fix, `nix fmt` (Lix hardcodes the `.` installable in `lix/nix/fmt.cc`) and `nix flake update` (positional args are input names, and `--flake` rejects a relative ref), plus the reason the failure is intermittent: a dirty worktree makes Lix copy the working tree instead of fetching the revision. The rendered text reaches $HOME through Home Manager activation rather than `write-files`, so no managed artifact changes. Validation: nix-instantiate --parse; nix eval --offline --raw path:.#lib.agents.systemPrompt.sections.validation --apply 'f: f {}', confirming "path:$PWD" survives the indented string rather than interpolating * docs(architecture): give the canonical docs the linked-worktree command forms CLAUDE.md calls `docs/architecture/` the canonical architecture documentation, so it outranks the instruction files while prescribing the same invocations that cannot fetch a clean linked worktree as a `git+file` flake. 06-reference.md contradicted itself. Its Troubleshooting fix for "Git hooks fail in a new worktree" was to run bare `nix develop`, which is exactly the command that fails in that scenario, so an operator hitting the failure and consulting the canonical reference for the recovery got the command that reproduces it. That row now names the failure it used to cause. Rewrites 13 sites across the five files: `nix develop`, `nix flake check`, `nix flake show`, `nix eval`, `nix build` and `nix run .#generation-manager` take `path:.`; `nix fmt` becomes `nix run path:.#formatter.x86_64-linux -- .` because Lix hardcodes the `.` installable in `lix/nix/fmt.cc`. 06-reference.md's Validation section states the convention once for the page, including that dropping `path:.` gives the primary-checkout form. The `./build.sh` rows are unchanged: it resolves its own `path:` ref rather than taking an installable. Prose that names `nix flake check` as a concept rather than prescribing a run is also unchanged. Validation: nix run path:.#formatter.x86_64-linux -- <the five files> (0 changed) * chore(agents): delete the orphaned .agents/AGENT_PROMPT.md Surfaced while auditing which files prescribe flake commands: this one carried `nix fmt`, `nix flake check` and `nix flake update --update-input <name>`, all of which either fail in a linked worktree or no longer exist. Rewriting them would have made a dead file internally consistent on one axis while it stayed wrong on others. Nothing in the repo reads it. The only `AGENT_PROMPT` match outside the file is the commented-out `CLAUDE_CODE_ENABLE_APPEND_SUBAGENT_PROMPT` in `modules/agents/claude-code/_env.nix`, an unrelated substring. Its content describes an `inputs/*` git-submodule workflow and an `update-input-branches` helper that are both gone: `inputs/` is not in the tree and `update-input-branches` is defined nowhere else. The live agent surfaces are `AGENTS.md`, `docs/AGENTS.md`, `scripts/AGENTS.md`, `CLAUDE.md` and `modules/agents/system-prompt.nix`. `.agents/dendrite.md` is untouched. Removed with `rip`, so it stays recoverable through the graveyard. Validation: rg -n 'AGENT_PROMPT' --hidden -g '!.git' (no consumer); rg -l 'update-input-branches' (self only) * docs(webapps): split the Chromium web-app migration plan into one file per phase The draft reached 2594 lines covering three sequentially gated PRs, so executing any one phase meant loading the other two. Each phase is now its own file, gated on the previous one merging: - chromium-webapps-plan-1-salvage.md: Task 1, rescue the compgen and statix fixes - chromium-webapps-plan-2-remove-firefoxpwa.md: Tasks 2-6, delete and unwire the firefoxpwa subsystem - chromium-webapps-plan-3-implement.md: Tasks 7-18, plus the decisions table, verified facts, browser-wide-policy consequences and the self-review for the series Phases 1 and 2 restate only the decision rows and file-structure block that bind them and point at phase 3 for the full record. The monolith is deleted rather than renamed: at 39% similarity to the phase 3 file git does not report a rename below `-M40%`, so `git log --follow` needs an explicit lower threshold. The revisions carried in the same move, by class: - Build blockers: `lib.optionalString` inside a backslash-continued argument list renders a whitespace-only line and terminates the command, which `writeShellApplication`'s shellcheck rejects (SC2215, SC2287); `lib.getExe pkgs.diffutils` names a binary the package does not ship; `modules/browsers/webapps/apps.nix` would have failed `apps-catalog-sync` because that hook globs `modules/browsers/*/apps.nix` and demands a catalog entry the plan deliberately withholds; `policy-check.nix`'s `originOf` coerced a null `url` two tasks after the task that introduced it. - Unfalsifiable verification: nine steps asserted something their own command could not report. Sweeps grepped for a string the artifact created one step earlier necessarily contains, or for `firefoxpwa` in a tree holding these plan files and the `docs/index.md` rows linking them; a policy-name loop checked fifteen of the twenty names its bullet claimed; `jq ... 2>/dev/null` made a parse failure and an empty history print the same nothing in the step meant to tell them apart. - Scope corrections: the browser-wide policy reaches the whole Brave family, not `brave-origin` alone, so the collision assertion, the guard predicate and the generated PR body all widen; the capture claims were inverted, since the hardening set carries no capture key and dropping the secrets file un-denies capture. - Recoverability and lifecycle: worktrees cut from `origin/main`, an absolute restore trap, a secret edited and committed in the worktree that pushes it, the submodule gitlink proven moved against the remote ref rather than a `git status` field that is absent on a detached HEAD. - Every flake invocation now uses the form that runs where the plan sends the work, which is what the instruction and reference fixes in this branch's other commits make correct at the source. Validation: nix run path:.#formatter.x86_64-linux -- docs/drafts docs/index.md * fix(build): resolve the flake installable for linked worktrees Every default-path nix invocation passed a bare ${FLAKE_DIR} or an implicit '.', which Lix resolves as git+file. In a clean linked worktree .git is a file, not a directory, so all five sites failed with "opening file '<dir>/.git/config': Not a directory" and --allow-dirty was the only working mode. resolve_installable() already emitted path: for that case; it now also covers a linked worktree, and nix develop, nix flake metadata, nix flake update and nix flake check go through it instead of relying on cd plus an implicit '.'. Kept conditional rather than always path:, unlike scripts/cache-coverage.sh: Lix's path fetcher dumps with defaultPathFilter (lix/libutil/archive.cc), which filters nothing, so a primary checkout would copy all of .git into the store and lose self.rev along with system.configurationRevision. FLAKE_DIR is canonicalized because a relative -p argument would reach Lix as path:relative/dir, the one shape that really does hit the getAbsPath throw on lock write. Validation: bash -n build.sh; nix develop path:. -c shellcheck build.sh * fix(hooks): name a form that works where the remediation message fires Every one of these strings is printed or read inside a linked worktree and named a command that fails there. .githooks/post-checkout is the sharpest case: the hook runs on git worktree add, so its own refresh instruction was the first thing a new worktree showed and the first thing that would exit 1. managed-files-drift printed 'Run: nix develop -c write-files' on the verify-mode failure path, and sync-pre-commit-hooks.sh emitted "run 'nix develop' in this worktree" into the generated hook body plus the same text on its missing-config error. Each now names the path:. form and the primary-checkout form, matching the convention the docs already use. .githooks/post-checkout is regenerated from its Nix source rather than edited. Validation: nix develop path:. --accept-flake-config -c write-files --offline; nix develop path:. -c pre-commit run --all-files --hook-stage manual * feat(treefmt): expose the formatter as packages.treefmt nix fmt hardcodes the '.' installable in lix/nix/fmt.cc, so a linked worktree could only reach the formatter by naming its system: formatter.x86_64-linux appeared 22 times across CLAUDE.md, AGENTS.md, docs/AGENTS.md, docs/architecture/06-reference.md, docs/guides/host-onboarding.md and the three webapps plan drafts. config.treefmt.build.wrapper is already a derivation named treefmt with bin/treefmt, so binding it to packages.treefmt makes `nix run path:.#treefmt -- .` resolve on any system. Same drvPath as formatter.x86_64-linux and already inside packages.formatter-toolchain, so the closure does not grow; modules/meta/cache-roots.nix selects by curated name, so the alias cannot trip its unused-name throw. modules/agents/system-prompt.nix keeps a repo-independent form, because it generates the cross-repo baseline and cannot assume a packages.treefmt exists elsewhere. Its formatter.<system> placeholder, which nobody could paste, is replaced by a currentSystem substitution. Two further corrections in the same surfaces. The nix flake update justification claimed --flake "path:." is rejected outright; it fetches fine, and the throw comes from getAbsPath (lix/libfetchers/path.cc) only when putFile writes flake.lock back, which lockFlake reaches only if the lock actually changes, so a no-op update exits 0 and the claim looked fabricated. And system-prompt.nix attached the path:. caveat to the structural-changes bullet while the value-level bullet above it, the one that recommends formatting, carried none; the caveat is now a section-level paragraph ahead of both, with the causal clause 06-reference.md had dropped. Validation: nix run path:.#treefmt -- --version; nix flake check path:. --accept-flake-config --no-build --offline; nix develop path:. --accept-flake-config -c write-files --offline * docs(sops): give the runbooks the linked-worktree command forms Both runbooks start with a step that fails in the worktree the branch workflow puts the work in. secrets-act.md was untouched by the surrounding work and still opened on `nix develop -c write-files` to refresh .sops.yaml, then hit the same bare form again at the nix build and at all three gh-actions-run variants, so a host key rotation broke at step 1 and again at the local workflow dry run. sops/README.md had already been corrected at its CI `nix flake check` line while five sites below it were not, which is worse than uniformly stale: an operator debugging `Unknown recipient` follows a remediation two paragraphs under an already-correct command and hits the failure anyway. secrets-act.md gains the same one-paragraph note the other pages carry, so the forms are explained once rather than per command. Validation: nix develop path:. -c pre-commit run --all-files --hook-stage manual * fix(devshell): resolve the treefmt cache against the real git dir shellHook set treefmt_cache="$PWD/.git/treefmt-cache/cache". In a linked worktree .git is a file, so mkdir -p exits with "Not a directory", `|| true` swallowed it, and TREEFMT_CACHE_DB was still exported pointing at a path inside a file. Every treefmt run outside the primary checkout lost its cache, and nothing said so. git rev-parse --absolute-git-dir gives the per-worktree git dir, which git removes along with the worktree, so the cache does not outlive what it belongs to. The suppression is gone: a real mkdir failure now names the path it could not create and says the run is uncached, rather than exporting a variable that cannot work. Reproduced before the change in a clean linked worktree: ls: cannot access '<worktree>/.git/treefmt-cache': Not a directory Validation: nix develop path:. -c bash -c 'ls -la "$(dirname "$TREEFMT_CACHE_DB")"'; nix flake check path:. --accept-flake-config --no-build --offline * docs(webapps): correct the claims the plan drafts could not support Four statements did not survive checking against the tree they describe. Phase 1 credited the rescue with adding a whole-tree branch to modules/meta/hooks/statix.nix. That branch is on main already, from commit 66ac9bac (an unrelated lefthook migration); PR #435 adds checks.statix-tree, a header comment and widened perSystem arguments. An operator hand-implementing rather than cherry-picking would have read the file, found the described behavior present, and skipped the change that matters. Phase 1 Task 1 Step 4 promised no output from `rg -n 'compgen' tests/ modules/ packages/` minus one path exclusion. On main that prints modules/hm-apps/proton-drive.nix twice, from a comment predating the branch. The step now runs the detector's own command-position predicate from modules/meta/build-time-shell.nix, verified to match exactly the one site Step 3 replaces on main and nothing on the rescued branch. Phase 3's self-review, which already named this class of defect and claimed it fixed, is corrected to say so. Phase 2 justified freezing its task numbering on cross-references phase 3 makes "in Task 13, in its docs/reference/webapps.md block and in its self-review". `rg -n 'Phase 2 Task'` over phase 3 finds two: Task 17 Step 2 and the self-review. Neither named site has one. Phase 3's --load-extension bullet was the only entry under "Verified facts this plan depends on" with no reproduction, and it underpins the Decisions table row that is marked do-not-relitigate. It now carries one, run against the pinned brave-origin 1.95.47: load-extension and disable-extensions-except are both present in the binary, and the branding-gated refusal literal is absent, which is what GOOGLE_CHROME_BRANDING unset looks like. The Enhanced Safe Browsing half is a runtime gate no string probe reaches, so Task 18 Step 3 is named as its confirmation along with the fallback if it fails. Validation: nix develop path:. -c pre-commit run --all-files --hook-stage manual * docs(webapps): harden migration plan execution steps Pin the Phase 1 rescue range and squash reset to the SHA captured when the worktree is created, so a concurrent origin/main fetch cannot silently adopt a newer parent. Rebase the resulting single commit separately when the remote advances. Make the Phase 3 policy probe use a unique managed-policy filename and make keep-alive key generation stop on OpenSSL failures or empty decoded DER instead of producing an empty key and a matching invalid ID. Validation: git diff --check; bash -n on the edited shell blocks; executable-position compgen matcher check * docs(validation): drop the inert flake.lock formatting rung `treefmt.settings.global.excludes` in `modules/meta/treefmt.nix` carries `*.lock` and `**/*.lock`, so the third rung of the input-update ladder never reached a formatter. Verified against the pinned treefmt 2.5.0 wrapper: `nix run path:.#treefmt -- flake.lock` prints `traversed 1 files / emitted 0 files for processing / formatted 0 files` and exits 0. Biome is the only enabled formatter with a JSON include and it lists `*.json`, which does not match the literal name `flake.lock`, so the rung was dead twice over. `.github/workflows/update-flake.yml`, the only automation that writes `flake.lock`, never invokes treefmt on it either. Dropping it makes AGENTS.md's "the two exceptions above" wrong, since only the `nix flake update` exception is still exercised by the chain; the Why line now names that one and the short-circuit it guards against. Validation: nix run path:.#treefmt -- AGENTS.md CLAUDE.md (0 changed) * fix(agents): scope path:. to linked worktrees in the generic ladder The `validation` section renders to `~/.claude/CLAUDE.md` and `~/.config/codex/AGENTS.md` through `modules/agents/claude-code/home-manager.nix` and `modules/agents/codex/home-manager.nix`, neither of which passes a `sectionOverrides.validation`, so both files get the same text for every repository. The structural-changes bullet hardcoded `nix flake check path:.`, which contradicted the paragraph rendered directly above it scoping `path:.` to a linked worktree, and made the worktree form the unconditional default everywhere. `path:.` is not inert. Lix's `PathInputScheme::fetch` in `lix/libfetchers/path.cc` derives only `lastModified` and never a rev, so `nix flake metadata path:.` reports `rev`, `dirtyRev` and `revision` as null where the `git+file` fetcher reports the commit. `modules/hosts/common/imports.nix` builds `selfRevision` from `self.dirtyRev` then `self.rev` and gates `system.configurationRevision` on it being non-null, so the prescribed command silently drops the revision stamp. The path fetcher also has no git-aware exclusion, so a primary checkout copies its whole `.git` into the store. The value-level bullet had the same defect indirectly: "the form above" is the linked-worktree formatter form. It now names `nix fmt` as the default and points at that form only for the worktree case. Validation: nix-instantiate --parse modules/agents/system-prompt.nix; nix eval --offline --raw path:.#lib.agents.systemPrompt.sections.validation --apply 'f: f {}' (every rendered path:. is worktree-scoped); nix run path:.#treefmt -- modules/agents/system-prompt.nix (0 changed) * fix(devshell): drop the treefmt cache export treefmt never reads treefmt 2.5.0 reads no `TREEFMT_*` environment variable. `rg TREEFMT_CACHE_DB` over the numtide/treefmt mirror returns nothing at any tag from v2.0.0 to v2.5.0, and `strings` over the wrapper this flake builds (/nix/store/4jzk7p66a93hlm0js4s5577vrijbjmxs-treefmt) finds no `TREEFMT_` string at all. `Path(root)` in walk/cache/cache.go resolves the database unconditionally as `xdg.CacheFile("treefmt/eval-cache/" + sha256(root) + ".db")`, and `cmd/format/format.go` calls `cache.Open(cfg.TreeRoot)` with no env indirection. `config.go` binds `TREEFMT_`-prefixed variables through viper, but no `cache-db` field exists for one to attach to. So the export has done nothing since commit 425ab9fe introduced it, and the repo-local placement it advertised never happened: the caches have been landing in ~/.cache/treefmt/eval-cache the whole time. Hashing the absolute tree root into the filename already isolates every worktree, which is the property the block was added to provide, so removing it loses nothing and no `unset` on the failure branches is needed to protect a neighbouring repository. Validation: nix-instantiate --parse modules/devshell.nix; rg TREEFMT_CACHE_DB over the tree returns nothing; nix run path:.#treefmt -- modules/devshell.nix (0 changed) * docs(webapps): prove the phase 1 cherry-pick conflict before the pick Two reviewers read Step 3's conflict-gated removal as unreachable, both arguing that `modules/browsers/firefoxpwa/m365-check.nix` is absent from `origin/main` and so replays as a clean add. The path is absent, but the picked commit modifies it rather than adding it: the add lives in e05b9197, which touches none of the three rescue paths and is therefore excluded by Step 2's own filter, while the first picked commit eb196992 reports `M` for that path. Replaying a modify onto a branch that never received the add is a modify/delete conflict. Reproduced against the object database, no index or working tree touched: `git merge-tree --merge-base=eb196992^ origin/main eb196992` prints `CONFLICT (modify/delete): modules/browsers/firefoxpwa/m365-check.nix deleted in acb24249 and modified in eb196992`. A real pick in a scratch clone stops with `DU modules/browsers/firefoxpwa/m365-check.nix`, and the documented `git rm -f` plus `git cherry-pick --continue` resolves it to a tree matching origin/main's firefoxpwa listing. Step 3 and Step 6 stay as written. Step 2 gains the reason the first commit conflicts and a merge-tree probe, so the operator confirms the conflict exists before the pick instead of inferring it cannot from the file's absence. The probe also fails loudly if the source branch is ever rebased so that the add joins the picked range. Validation: git merge-tree probe run against the real objects, output matches the documented expectation; nix run path:.#treefmt -- docs/drafts/chromium-webapps-plan-1-salvage.md (0 changed) * fix(build): announce the path: reference that drops the revision stamp `resolve_installable` returns `path:${FLAKE_DIR}` for a linked worktree on the default path, not only under `--allow-dirty`, and that reference costs the revision stamp: Lix's `PathInputScheme::fetch` derives no rev, so `modules/hosts/common/imports.nix` sees `selfRevision == null`, its `lib.mkIf` guard goes false and `system.configurationRevision` is never set. A switch from a worktree therefore produced a system whose `nixos-version --json` reports no revision, with nothing in the output tying that to the fetcher. The notice fires for both reasons rather than the worktree case alone, because `--allow-dirty` selects the same `path:` reference and loses the same stamp. It is emitted once from the main flow instead of from `resolve_installable`: all six call sites are command substitutions, so a subshell there could neither print once nor carry a guard back to the caller. Validation: bash -n build.sh; shellcheck build.sh (clean); nix run path:.#treefmt -- build.sh (0 changed) * docs(troubleshooting): correct the missing-reference row for the path: form "Ensure the file is tracked by git" fixes a missing reference only under the bare `.` form, whose `git+file` fetcher cannot see untracked files. Both root instruction files now prescribe `path:.` for the linked worktree the branch workflow sends the work into, and that fetcher dumps the directory unfiltered, so the advice sends the reader to `git add` for a symptom `path:.` cannot produce. Confirmed in this worktree: an untracked `modules/zz-untracked-probe.nix` setting `flake.zzUntrackedProbeMarker` is copied into the `path:.` store source and `nix eval path:.#zzUntrackedProbeMarker` returns its value, so auto-discovery reaches it. The probe was removed with `rip` afterwards. The real hazard under `path:.` runs the other way, and both rows now state it: a stray untracked module makes a local check pass while CI, which fetches the pushed revision, does not see the file. `AGENTS.md` carried the identical row and gets the same correction. Validation: nix run path:.#treefmt -- CLAUDE.md AGENTS.md (0 changed); untracked-module discovery reproduced and reverted as described * fix(build): emit the path: notice after logging is redirected `setup_logging` installs `exec > >(tee ...)` as the first statement of `main()`, so anything printed at script load reaches the terminal and never the log file. The notice explaining a missing `system.configurationRevision` was the one `status_msg` in the script that never reached `${LOG_DIR}/build-<timestamp>-<pid>.log`, which is precisely the artifact someone consults after finding a host whose `nixos-version --json` reports no revision. `PATH_REF_REASON` still resolves at load, where `FLAKE_DIR` and `ALLOW_DIRTY` are final; only the printing moves, into `announce_path_ref` called from `main()` after `setup_logging`. Verified against a real run: the log now holds `==> Using path:/home/vx/trees/nixos/docs-webapps-plan-review (linked worktree); self.rev is unset there, ...` with ANSI stripped, on the line after `Logging to:`. Validation: bash -n build.sh; shellcheck build.sh (rc 0); ./build.sh run to the dirty-tree guard and the emitted log file read back * fix(docs): give nix flake metadata the absolute ref its lock write needs `nix flake metadata` locks the flake, so it writes `flake.lock` back when the lock is out of sync, and that write goes through the same Lix `getAbsPath` (`lix/libfetchers/path.cc`) that makes `nix flake update --flake path:.` throw. The input-update ladder ran the relative form on its first rung, which is the rung that fails, and in `AGENTS.md`'s `&&` chain the failure took the update with it: the exact short-circuit the chain was written to prevent. Reproduced on a scratch flake with no lock present: $ nix flake metadata --refresh path:. error: … while updating the lock file of flake 'path:.?…' error: cannot fetch input 'path:.' because it uses a relative path $ nix flake metadata --refresh "path:$PWD" Inputs: … # exits 0 and writes flake.lock Re-running the relative form once the lock is in sync exits 0, which is why the defect hid: it fails only when there is something to write, and that is the state the ladder is run in. `build.sh` already had it right, passing the absolute `${installable}` from `resolve_installable` to both commands, so the docs disagreed with the script. The worktree notes are widened from "`nix flake update`" to any lock-writing command, in `CLAUDE.md`, `AGENTS.md`, `docs/AGENTS.md` and the generic ladder in `modules/agents/system-prompt.nix`, and the stale "the two commands `path:.` does not fix" counts go with them. Validation: nix flake check path:. --accept-flake-config --no-build --offline (rc 0); nix-instantiate --parse modules/agents/system-prompt.nix; rendered validation section re-read for intact "path:$PWD"; nix run path:.#treefmt (0 changed) * fix(build): stop path: from copying gitignored secrets into the store The bare `.` form fetched through git, so `.gitignore` kept ignored files out of the store. `resolve_installable` now returns `path:${FLAKE_DIR}` for every linked worktree on the default path, and the path fetcher dumps the tree unfiltered, so the "Secrets safety (defense-in-depth)" block in `.gitignore` (`*.agekey`, `*.key`, `*.pem`, `*.p12`, `*.pfx`, `.env`, `.env.*`, `id_*`) stops protecting anything exactly when the ref switches. Reproduced before the fix: a probe `.env` and `id_ed25519_probe` in a worktree whose `git status --short` was empty both landed in `/nix/store/<hash>-source/` as `-r--r--r-- root root`. `ensure_clean_git_tree` cannot catch this. Its untracked scan is `git ls-files --others --exclude-standard`, which excludes precisely the ignored set, and under `--update` the guard is in the `else` branch and never runs at all. `ensure_no_ignored_secrets` therefore runs from `main` outside that branch, only when `PATH_REF_REASON` is set, since the bare form is already safe. Patterns are parsed out of `.gitignore` rather than restated, because that file is generated from `modules/files.nix` and a second copy would drift unseen; the `!id_*.pub` negation is honoured. `--allow-secret-copy` and `ALLOW_SECRET_COPY=1` override it, and the flag is mirrored into `modules/apps/build-sh-completion.nix` for `build-sh-completion-sync`. The `announce_path_ref` notice gains the non-secret half, `.direnv/`, `tmp/` and `*.log` copied on every build, where aborting would be wrong. Validation: probes `.env`, `test_probe.pem`, `id_probe_rsa` all reported and the build refused with exit 1; `id_probe.pub` correctly not reported; both override forms proceed past the guard; probes removed with rip. bash -n build.sh; shellcheck build.sh (rc 0); pre-commit run build-sh-completion-sync (Passed); nix flake check path:. --accept-flake-config --no-build --offline (rc 0) * docs(troubleshooting): record what path: does to ignored files Both root instruction files prescribe `path:.` as the default form for `nix develop`, `nix flake check`, `nix build` and `nix repl` in a linked worktree, and the missing-reference row covered only the evaluation half of "dumps the directory unfiltered". The other half is that the `.gitignore` secrets block protects nothing under that form: the files are copied into the world-readable store. `git status --short` does not surface them, because ignored files are not reported, so the row names `git status --porcelain --ignored=matching` as the check that does. It also records that `./build.sh` now aborts on a secrets-block match while a bare `nix` command run by hand has no such guard, so the reader knows which path is protected. Validation: nix run path:.#treefmt -- CLAUDE.md AGENTS.md (0 changed) * fix(build): scan submodules for ignored secrets and fail closed on parser drift Two holes in the guard added by 428d2ca8. `git ls-files --others --ignored` stops at a gitlink, while `path:` copies submodule working trees whole. `secrets/` ignores decrypted SOPS output through its own `.gitignore` (`**/decrypted_*`, `*.dec.*`), and those names match none of the superproject patterns, so the highest-value case in this repo went straight through. Reproduced: a probe `secrets/decrypted_probe.yaml` was absent from superproject `git status --short` and from `git ls-files --others --ignored --exclude-standard`, yet landed at `/nix/store/<hash>-source/secrets/` as `-r--r--r-- root root`. The submodule pass reports every ignored file it finds rather than filtering, since basename matching against the superproject block would not catch `decrypted_*.yaml`. The parser also failed open. It keys on the literal `# Secrets safety (defense-in-depth)` heading and on a blank line closing the block, both of which live in `modules/development/gitignore.nix`; a regenerated `.gitignore` with a renamed heading left `deny` empty and waved every secret through silently. `managed-files-drift` cannot catch that, because `.gitignore` would still agree with its source. An empty `deny` and a missing `.gitignore` now both abort. The status is read through `if ! hits="$(...)"`, which keeps `set -e` suspended so the cause prints instead of reaching `trap_error` as a bare `Command 'return 1' failed`. Validation: submodule probe reported and build refused (exit 1); heading rename produces the parser error plus "Refusing to build with the secrets guard inoperative" (exit 1); superproject `.env` still caught; clean tree passes to the next check; probes removed with rip and both trees verified clean. bash -n build.sh; shellcheck build.sh (rc 0, SC2016 disabled where git submodule foreach expands $displaypath itself) * docs(worktree): drop the stale one-command count for path: `de352727` widened the exception from `nix flake update` to any command that writes `flake.lock` back, and corrected the count in `docs/AGENTS.md`, but "the one command `path:.` cannot fix" survived in four more surfaces. Each now names both cases. `docs/architecture/06-reference.md` matters most: `CLAUDE.md` calls `docs/architecture/` the canonical architecture documentation, so it outranks both instruction files for a reader who consults it alone, and it named no lock-writing command at all. Its paragraph now carries `nix flake metadata --refresh` and `nix flake update` with the `"path:$PWD"` form. `README.md` is generated, so the fix goes into `modules/readme.nix` and the artifact is regenerated with `write-files` rather than edited. Validation: nix run path:.#treefmt (0 changed); write-files --offline regenerated README.md from the module; rg over the tree returns no remaining "one command `path:.`" phrasing; nix flake check path:. rc 0 * fix(build): scope the secrets guard to git worktrees and fail closed on scan errors Three defects in the guard from 94b937df. `PATH_REF_REASON` is set whenever `--allow-dirty` is passed, regardless of `-p`, so `./build.sh -p /tmp/otherflake --allow-dirty` reached the missing-.gitignore abort and exited 1 telling the operator to "Restore it with write-files" for a directory this repo does not generate. Reproduced against a scratch flake. The guard now gates on `git rev-parse --is-inside-work-tree` the way `ensure_clean_git_tree` does, and skips with a notice: no git worktree means no ignore set, so there is nothing for `path:` to smuggle past `.gitignore`. The submodule pass failed open in the same function whose parser had just been hardened to fail closed. `2>/dev/null || true` swallowed every `git submodule foreach` failure, leaving the list empty while `secrets/` stayed on disk for `path:` to copy whole, which reproduces the exact hole the pass was added to close. The per-submodule `ls-files` ran in a process substitution whose status nothing read. Both are captured and abort now; each returns 128 on failure, so the `if !` guards catch them. The error heading claimed every hit "matched the .gitignore secrets block", which is false for submodule hits by design: that pass reports all ignored files, and `--exclude-standard` there also honours the user's global `core.excludesFile`, so `secrets/.direnv/...` or an editor swap file printed under a heading naming a block it never matched. The heading now names both sets. Validation: -p at a non-git flake proceeds with the notice instead of aborting; submodule and superproject probes both reported under the new heading; clean tree passes through; git submodule foreach and git -C ls-files both return 128 on failure. bash -n build.sh; shellcheck build.sh (rc 0); nix flake check path:. (rc 0) * docs(troubleshooting): sweep submodules for ignored secrets too The row prescribed `git status --porcelain --ignored=matching`, which stops at the gitlink and so misses the file class the row exists to protect: `secrets/` ignores decrypted SOPS output through its own `.gitignore` (`**/decrypted_*`, `*.dec.*`). Confirmed against a probe `secrets/decrypted_probe.yaml`: the superproject form reports nothing while `git submodule foreach --recursive 'git status --porcelain --ignored=matching'` reports `!! decrypted_probe.yaml`. An operator running the prescribed command, seeing a clear tree, and then running the bare `nix develop path:.` these same files prescribe would copy the decrypted secret into the world-readable store having just been told there was nothing there. Both rows now name the second sweep, and record that `./build.sh` runs both. Validation: probe reproduced and removed with rip; nix run path:.#treefmt -- CLAUDE.md AGENTS.md (0 changed) * fix(build): fail closed on the superproject ignored-file scan The superproject `ls-files` still ran in a process substitution, so nothing read its status while the parser branch and the submodule pass beside it had already been hardened. An unreadable `core.excludesFile`, a locked index or a malformed nested `.gitignore` made git exit non-zero, the loop read nothing, and the guard reported no hits and built, copying `.env` and `id_*` into the store: the hole the submodule pass was captured to close. Both scans now write to a temp file whose status is checked, rather than into command substitution. Substitution would have forced dropping `-z`, and a path containing a newline then splits into two lines that match no pattern, so the fix for one fail-open would have opened another. Confirmed: a file named with an embedded newline is still reported. The temp file is removed on every exit path. The path-reference notice carried the framing corrected in the error heading by 5fdd8675. The submodule pass reports every ignored file, so `secrets/.direnv/` or an editor swap file under a submodule aborts the build while the notice was telling the operator `.direnv/` is merely copied. It now names both sets, matching the heading. Validation: superproject `.env` and `secrets/decrypted_probe.yaml` both reported; a file whose name contains a newline reported (proving -z survived); clean tree passes to the next check; no temp files left in /tmp. bash -n build.sh; shellcheck build.sh (rc 0); nix run path:.#treefmt -- build.sh (0 changed) * fix(build): abort on missing .gitignore only when it is tracked `5fdd8675` scoped the guard to git worktrees, which left the same wrong-advice regression live for any git repository that simply has no `.gitignore`, a common shape for small repos. `PATH_REF_REASON` is set whenever `--allow-dirty` is passed regardless of `-p`, so `./build.sh -p /tmp/nogitignore --allow-dirty` passed the `rev-parse --is-inside-work-tree` gate and aborted with "Restore it with write-files", naming a file this repo does not generate there. Drift is distinguishable from absence: `git ls-files --error-unmatch -- .gitignore` exits 0 for this repo, where the file is tracked and generated from modules/development/gitignore.nix, and 1 where it never existed. Only the tracked-but-absent case is drift, so only that one aborts. Validation: /tmp/nogitignore (git repo, no .gitignore) now proceeds with a notice; hiding this repo's own .gitignore still aborts with "tracked but absent" at exit 1, and the file was restored with git status clean. bash -n build.sh; shellcheck build.sh (rc 0) * docs(reference): add the ignored-files row to the canonical troubleshooting table `CLAUDE.md` names `docs/architecture/` the canonical architecture documentation, so this page outranks both root instruction files for a reader who consults it alone. It prescribes `path:.` across its command table while stating nothing about what that fetcher does to ignored files, so that reader got the prescription without the disclosure hazard. Same reasoning that moved the stale command count here in 79344a69. The row carries both sweeps, since the superproject form stops at the `secrets/` gitlink, and records that `./build.sh` is guarded while a bare `nix` command is not. Validation: nix run path:.#treefmt -- docs/architecture/06-reference.md (0 changed) * fix(completion): retry host lookup with path: when the bare ref finds nothing `_build_sh_hosts` defaults `flake_dir` to `.` and evaluates `nix eval --raw "$flake_dir#nixosConfigurations"`, the form that cannot fetch a clean linked worktree, which is where the branch workflow puts every change. `|| true` swallowed the failure, so `--host` completion degraded to the single local `hostname` with no diagnostic. Retried rather than replaced: `path:` unconditionally would copy the whole `.git` into the store from a primary checkout, which is the cost this PR documents elsewhere. The retry runs only when the first evaluation returns nothing, so the primary checkout keeps the git fetcher. Validation: nix-instantiate --parse modules/apps/build-sh-completion.nix; pre-commit run build-sh-completion-sync (Passed); nix flake check path:. --accept-flake-config --no-build --offline (rc 0) * fix(build): fail closed on a partial parse of the secrets block The empty-`deny` check closed the total-parse-failure hole and left the likelier one open. The awk block terminates at the first blank line, so a blank line inserted for readability, the natural spot being before the `# Common SSH/private key patterns` comment, leaves `deny` non-empty while dropping everything after it. Reproduced by inserting exactly that: the parser yielded seven patterns instead of eight, `id_*` was gone, the non-empty check passed, and a probe `id_probe_rsa` went unreported straight past the guard. `managed-files-drift` cannot see it either, because `.gitignore` still agrees with `modules/development/gitignore.nix`. The expected minimum set is now named in the script and asserted. That does not reintroduce the drift the runtime parse avoids: a legitimate change to the block fires loudly, naming the patterns it no longer finds, instead of thinning the deny list in silence. Fail closed is the point. Validation: blank line inserted mid-block aborts with `(missing: id_*)` at exit 1, and `.gitignore` was restored with git status clean; unmodified block still catches the superproject `.env` and `secrets/decrypted_probe.yaml` probes; clean tree passes to the next check. bash -n build.sh; shellcheck build.sh (rc 0) * fix(completion): gate the path: retry on the linked-worktree marker `e2c948f8` retried on empty output, which is not the same condition as "linked worktree". An evaluation error, a `nixosConfigurations` rename, or `nix` missing all fall through to `path:` as well, and in a primary checkout that copies the whole `.git` plus every `.gitignore`d file into the store, including the `*.key`, `*.pem`, `.env` and `id_*` set `ensure_no_ignored_secrets` aborts on. Pressing Tab is not where that should be paid, and nothing reported it had happened. Gated on the same discriminator `resolve_installable` uses: `.git` is a file in a linked worktree and a directory in a primary checkout. Validation: nix-instantiate --parse modules/apps/build-sh-completion.nix; pre-commit run build-sh-completion-sync (Passed); nix flake check path:. --accept-flake-config --no-build --offline (rc 0) * fix(build): run the secrets guard for --cache-coverage and name the .git cost The guard gated on `PATH_REF_REASON`, which assumes this script is the only thing that reaches a `path:` ref. `scripts/cache-coverage.sh` hardcodes `FLAKE_REF="path:${FLAKE_DIR}"` with no bare-ref branch, so in a primary checkout with a clean tree the guard returned early and `nix path-info --derivation "path:<dir>#..."` made the unfiltered copy anyway. `CLAUDE.md` and `AGENTS.md` now promise "`./build.sh` runs both sweeps and aborts", which was untrue on that invocation. The gate skips only when the bare ref is in play and `--cache-coverage` was not requested. The notice also enumerated the tolerated set without its largest member. `PATH_REF_REASON` is `allow-dirty` regardless of checkout kind, so `./build.sh --allow-dirty` in a primary checkout selects `path:` where `.git` is a directory and the copy takes the whole git directory, history and `.git/config` with any credential helper URLs included. `resolve_installable` names that cost as the reason the bare form is kept elsewhere; the operator overriding with `--allow-dirty` never saw it. Validation: --cache-coverage with a probe .env reports it and refuses; gate truth table checked for the primary-checkout combination this worktree cannot produce (empty PATH_REF_REASON plus CACHE_COVERAGE=true runs the guard); clean tree passes to the next check. bash -n build.sh; shellcheck build.sh (rc 0) * fix(completion): resolve hosts through the primary checkout, not path: `a9a590fc` gated the retry on the linked-worktree marker, which removed the `.git`-into-store cost in a primary checkout and left the other cost in the only case that still fires. A linked worktree is exactly where `ensure_no_ignored_secrets` refuses to build without `--allow-secret-copy`, and the retry performed the same unfiltered copy from a Tab press: `.env`, `*.key`, `*.pem`, `id_*` and `secrets/decrypted_*` into a world-readable store path, with no notice and no override, plus the whole worktree copied synchronously while the shell blocks on completion. No copy is needed. The primary checkout backing the worktree shares its object store and fetches as `git+file`, which filters ignored files. `git rev-parse --path-format=absolute --git-common-dir` resolves to `<primary>/.git` here, and `:h` gives the checkout. Host names differing between branches only costs the pre-existing `hostname` fallback. Validation: nix-instantiate --parse modules/apps/build-sh-completion.nix; git-common-dir resolution confirmed to give /home/vx/nixos with flake.nix present; pre-commit run build-sh-completion-sync (Passed); nix flake check path:. (rc 0) * docs(agents): state what path:. copies in the generic ladder The paragraph renders to `~/.claude/CLAUDE.md` and `~/.config/codex/AGENTS.md`, so it prescribes `path:.` in every linked worktree of every repository while stating only the fetch failure it fixes. The disclosure cost this PR treats as build-blocking in this repo, ignored private keys, `.env` files, decrypted secrets and whole submodule working trees landing world-readable in the store, was recorded in `CLAUDE.md`, `AGENTS.md` and `docs/architecture/06-reference.md` but not in the surface that outranks them for anything they do not restate. Outside this repository there is no `build.sh` guard at all, so the generic ladder is the only place a reader is warned. Both sweeps are named, since the superproject form stops at a gitlink, along with the `.git` copy on a primary checkout and the dropped `self.rev`/`self.dirtyRev`. Validation: nix-instantiate --parse modules/agents/system-prompt.nix; rendered validation section re-read with "path:$PWD" intact and the new paragraph present; nix flake check path:. (rc 0); treefmt 0 changed * fix(build): match deny patterns on every path component Matching only the basename failed open on an ignored directory. `git ls-files --others --ignored --exclude-standard` does not collapse ignored directories without `--directory`; it reports each file inside them with the directory prefix. So `id_backup/`, ignored by `id_*` and copied whole by `path:`, yields `id_backup/notes.txt`, whose basename matches nothing. Reproduced on a clean tree: `id_backup/` holding `notes.txt` and `inner_key` produced no guard output at all and the build proceeded to the pre-commit stage. The same holds for `.env.d/` under `.env.*`. The deny walk now tests every path component. The allow list still matches on the basename, since `!id_*.pub` un-ignores a file rather than a directory, and git omits such a file from `--ignored` output entirely. Validation: id_backup/notes.txt and .env.d/creds now reported and the build refuses; plain-file cases (.env, secrets/decrypted_probe.yaml) still reported; id_probe.pub still absent from the ignored set, so the negation is unaffected; clean tree passes. bash -n build.sh; shellcheck build.sh (rc 0) * fix(completion): guard the :h expansion against an empty git result zsh documents `:h` as working like `dirname`, so it turns an empty value into `.`. When `git rev-parse --git-common-dir` fails, which happens with `git` absent from `PATH` or a `.git` file pointing at a gitdir `git worktree prune` already removed, `main_checkout` became `.`, the `-n` test passed, and the `-f "${main_checkout}/flake.nix"` probe then read the completion shell's cwd instead of the directory `-p` named. Typing `build.sh -p /some/other/worktree` from inside a different flake would offer that flake's hosts as if they came from `-p`. Confirmed in zsh: `v=""; echo "${v:h}"` prints `.`. Validation: nix-instantiate --parse modules/apps/build-sh-completion.nix; pre-commit run build-sh-completion-sync (Passed); nix flake check path:. (rc 0) * docs(sops): correct the claim that path: bypasses submodules The page asserted "`path:` bypasses git and submodules entirely", which this branch disproves: `path:` copies what is on disk, submodule working trees included, which is why `ensure_no_ignored_secrets` exists. A probe `secrets/decrypted_probe.yaml` lands at `/nix/store/<hash>-source/secrets/` world-readable. Secret files are absent on a CI runner because the submodule is not checked out there, not because the fetcher skips it. That left this page as the only surface carrying both halves of the hazard and neither warning: its own workflows produce `secrets/decrypted_*`, and it prescribes `nix develop path:.` and `nix build "path:.#..."` with no pointer to the disclosure sweep `CLAUDE.md`, `AGENTS.md` and `docs/architecture/06-reference.md` all gained. Validation: nix run path:.#treefmt -- docs/sops/README.md (0 changed); git diff reviewed to confirm only the intended paragraph changed after an intermediate edit left stray code fences * fix(build): report the truncated secrets-guard hit list The list capped at 50 with nothing said, while the next line asks for all of them to be moved out of the worktree. At 51 or more hits the operator moves the 50 they were shown, reruns, and hits the same abort, with no way to tell a cap from a complete list. Validation: 60 ignored files under .env.d/ list 50 entries followed by "... and 10 more not shown."; bash -n build.sh; shellcheck build.sh (rc 0); nix run path:.#treefmt -- build.sh (0 changed) * fix(build): name the copier the --cache-coverage run actually uses The gate widened in 7c221afc runs ensure_no_ignored_secrets when CACHE_COVERAGE is true even with an empty PATH_REF_REASON, but both operator-facing messages still assumed build.sh had selected path: itself. In a clean primary checkout resolve_installable returns the bare ref, so the abort read "Ignored files that path:<dir> would copy" while naming a reference that run never uses, and scripts/cache-coverage.sh, whose hardcoded FLAKE_REF="path:${FLAKE_DIR}" is the only thing reaching that copy, went unmentioned. Empty PATH_REF_REASON inside the guard body is exact: PATH_REF_REASON is readonly and assigned once, CACHE_COVERAGE has no environment fallback, so the gate's own early return leaves --cache-coverage as the only way in. announce_path_ref had the same asymmetry and was silent instead of wrong: it gates on the same variable, so that run shape got no notice at all, even though cache-coverage.sh copies .git unconditionally with no --allow-dirty branch. It gains its own branch rather than a widened condition, because the existing text claims self.rev is unset and system.configurationRevision is dropped, and neither holds when the build keeps the bare ref and only the coverage probe copies. Validation: bash -n build.sh; nix develop path:. -c shellcheck build.sh (rc 0, no findings); nix run path:.#treefmt -- build.sh (0 changed); rendered announce_path_ref across all four run shapes and the abort heading in both ref shapes against extracted copies of the functions. * fix(cache-coverage): give the direct route the same secrets guard as build.sh scripts/cache-coverage.sh hardcodes FLAKE_REF="path:${FLAKE_DIR}" with no bare-ref branch, so every run copies the tree unfiltered into the world-readable store. 7c221afc made ./build.sh --cache-coverage reach ensure_no_ignored_secrets first, but the script run directly, and the nix run .#cache-coverage wrapper that readFile's it verbatim, still made that copy with nothing checking it. CLAUDE.md, AGENTS.md and docs/architecture/06-reference.md each promised "./build.sh runs both sweeps and aborts" against "a bare nix command does not", a split that left the route those same files prescribe in their validation ladder on the unguarded side. The scan and the abort move to scripts/lib/secrets-guard.sh rather than being restated, since the reason the patterns are parsed out of .gitignore instead of copied applies to the guard itself. secrets_guard_enforce returns 1 for hits and 2 for an inoperative guard so each caller keeps its own exit-code contract: build.sh exits 1, cache-coverage.sh exits 2, where neither outcome is a coverage result. Colours are read as ${YELLOW:-}/${RED:-}/${NC:-}, so build.sh keeps its formatting and cache-coverage.sh gets the same text unstyled. modules/packages/cache-coverage.nix prepends the library into the composed text, because nothing sits beside the script in the store; the script skips its own source when the functions are already defined, so the checkout and the wrapper run identical logic. --allow-secret-copy and ALLOW_SECRET_COPY=1 override it on both. No completion mirrors this script's flags, so nothing needs the new one. Validation: bash -n on both scripts; nix develop path:. -c shellcheck build.sh scripts/cache-coverage.sh scripts/lib/secrets-guard.sh (rc 0, no findings); nix run path:.#treefmt (0 changed); nix build path:$PWD#cache-coverage (builds, so the composed text passes writeShellApplication's shellcheck); nix flake check path:. --no-build --offline (rc 0). Fixtures: a repo carrying the secrets block plus a stray .env aborts at rc 2 through the script and through the built wrapper with byte-identical output, --allow-secret-copy passes it, and this worktree scans clear. * fix(build): stop the parser abort from naming a module foreign trees lack --flake-dir pointed at any other repository reached the secrets-block parser and aborted with "Realign this parser with modules/development/gitignore.nix", naming a file that tree does not contain. 5fdd8675 and 0d2d2768 fixed the same wrong-advice class for the non-git case and the untracked-.gitignore case, but both gates pass for the common shape: a foreign flake that is a git worktree and has a .gitignore. PATH_REF_REASON is set from --allow-dirty and from the linked-worktree marker without consulting --flake-dir, so `./build.sh -p /other/repo --allow-dirty` and any direct cache-coverage.sh run against another tree hit it. The discriminator 0d2d2768 already established extends one level down: the block is this repo's convention, emitted by modules/development/gitignore.nix. An absent heading in a tree that does not track that generator is a foreign .gitignore and the scan does not apply; an absent heading in a tree that does track it is the drift that must keep failing closed, so the parser still runs there. Validation: nix develop path:. -c shellcheck scripts/lib/secrets-guard.sh (rc 0); nix run path:.#treefmt (0 changed). Fixtures, both directions: a repo whose .gitignore has no secrets block and no generator is skipped with a notice instead of the write-files abort, and the same tree with modules/development/gitignore.nix tracked still fails closed naming every missing pattern. * docs(troubleshooting): record that every path: route now reaches the guard The ignored-files entries split the world into "./build.sh, guarded" and "a bare nix command, unguarded", which was already incomplete when written and is now wrong: scripts/cache-coverage.sh and the nix run path:.#cache-coverage app built from it share build.sh's guard through scripts/lib/secrets-guard.sh. CLAUDE.md is the sharpest case, since its own validation ladder prescribes running that script directly, so the file prescribed a command its troubleshooting section described as unprotected. docs/reference/cache-coverage.md gains the same statement in Manual Use, where all three invocations are listed as equal-weight options and none carried a warning; that page is where the CLAUDE.md ladder sends the reader, so leaving it silent would have kept the most direct instruction uncorrected. Validation: nix run path:.#treefmt (1 file reflowed); rg sweep for the "runs both sweeps" and "no such guard" phrasings across *.md and *.nix returns only the corrected rows. * fix(build): silence SC1091 on the guard source the way cache-coverage.sh does d49d79b6 moved the secrets guard into scripts/lib/secrets-guard.sh and gave build.sh a source line without the disable= that scripts/cache-coverage.sh carries on its own. The shellcheck pre-commit hook passes only the staged files, so any commit that stages build.sh without also staging the library fails with "SC1091 (info): Not following: scripts/lib/secrets-guard.sh was not specified as input". That is why the branch has not tripped it yet: d49d79b6 staged both files, and no commit since has touched build.sh alone. Reproduced: shellcheck build.sh exits 1, shellcheck build.sh scripts/lib/secrets-guard.sh exits 0. source-path=SCRIPTDIR stays, so a run naming both files still gets the cross-file check; only the unfollowable-source notice is suppressed. Validation: bash -n build.sh; shellcheck build.sh exits 0 alone and with the library; nix run path:.#treefmt -- build.sh reports 0 changed. * fix(build): forward --allow-secret-copy to the cache-coverage child ALLOW_SECRET_COPY is a plain shell variable that build.sh sets from the flag and never exports, and the --cache-coverage branch invoked scripts/cache-coverage.sh with only --flake-dir and --host. That script calls secrets_guard_enforce unconditionally, so ./build.sh --cache-coverage --allow-secret-copy cleared build.sh's own guard in process, then aborted inside the child with "refusing to evaluate ... with ignored files present" at exit 2, taking the run down through the ERR trap. ALLOW_SECRET_COPY=1 kept working, because a prefix assignment enters the process environment and is inherited, so the two spellings of one override disagreed on this route. docs/reference/cache-coverage.md documents the pair as working on exactly this invocation. Reproduced against a scratch flake whose scripts/cache-coverage.sh prints its argv: the previous build.sh passed "--flake-dir <dir> --host probehost" with ALLOW_SECRET_COPY unset in the child; it now appends --allow-secret-copy for both spellings and still omits it when neither is set, so an unoverridden run keeps aborting. The comparison matches secrets_guard_enforce's own "true" or "1" test, so an inherited ALLOW_SECRET_COPY=1 converts to the canonical flag. Validation: bash -n build.sh; shellcheck build.sh; nix run path:.#treefmt -- build.sh reports 0 changed; scratch-flake argv probe for all three override states; real scripts/cache-coverage.sh with a probe .env exits 2 without the flag and reaches the report with it (probe removed with rip). * fix(cache-coverage): declare the grep and sed the prepended guard runs modules/packages/cache-coverage.nix composes scripts/lib/secrets-guard.sh into the wrapper text, and that library is the only caller of two binaries runtimeInputs did not list: grep -qxF decides whether the scan applies at all, and sed -n '1,50p' prints the hit list. writeShellApplication emits export PATH="<runtimeInputs>:$PATH", so both resolved from whatever PATH the caller happened to carry. Reproduced against the wrapper built from HEAD, run under env -i: "grep: command not found" at the discriminator, then "sed: command not found", and the abort printed its heading and "Move them outside the worktree" with no list of what to move, so the operator is told to move files the run refuses to name. With gnugrep and gnused declared the same run lists the probe .env. The abort still fired in this tree only because it tracks modules/development/gitignore.nix; a tree that does not would have taken the skip branch on the failed grep. Validation: nix-instantiate --parse; nix run path:.#treefmt -- modules/packages/cache-coverage.nix reports 0 changed; nix build path:.#cache-coverage (writeShellApplication shellchecks the composed text); env -i comparison of the two wrappers with a probe .env (probe removed with rip). * fix(secrets-guard): fail closed when grep or sed is missing The guard is the only user of grep and sed in either caller, so declaring them in the wrapper's runtimeInputs covers `nix run path:.#cache-coverage` and leaves the two routes runtimeInputs cannot reach: a direct `./scripts/cache-coverage.sh` from a checkout, and `./build.sh`, which has no required-tool preflight at all. Checking them where they are used covers every caller instead of restating the list in each one. Both absences were silent in the guard's own terms. A grep that is not found exits 127, which negates to true, so the "tree does not own modules/development/gitignore.nix" branch skips the whole scan in any foreign tree carrying the heading; a sed that is not found drops the hit list between the abort heading and the instruction to move the files it no longer names. awk, mktemp and the git calls in secrets_guard_paths already fail closed through the parser's expected-pattern check and the status-checked scans. Reproduced with a PATH holding bash, lix, curl, jq, git-minimal, gawk and coreutils but no gnugrep or gnused: the previous guard printed "grep: command not found", then the abort heading, then "sed: command not found" in place of the hit list, and returned 1; it now returns 2 with "grep was not found, so the secrets guard cannot run". The check sits after the git-worktree and .gitignore gates, so a foreign non-git flake still gets its skip notice and 0, and after the ALLOW_SECRET_COPY return, so the override still wins. Validation: bash -n; shellcheck over build.sh, scripts/lib/secrets-guard.sh and scripts/cache-coverage.sh; nix run path:.#treefmt reports 0 changed; nix build path:.#cache-coverage (writeShellApplication shellchecks the composed text); the stripped-PATH probe above plus an unchanged full-PATH run that still lists a probe .env and exits 2 (probe removed with rip, superproject and submodule sweeps clean). * fix(secrets-guard): match submodule ignores against the secrets block The submodule pass reported every ignored file it found, because secrets/.gitignore names decrypted SOPS output (**/decrypted_*, *.dec.*) and those matched none of the superproject patterns the deny list is parsed from. That made build output, editor leftovers and any other ignored file under secrets/ abort a build or a coverage report as though they were keys. The secrets block now mirrors those two patterns, so the deny list covers them and both scans classify with one rule. Git does not apply superproject ignores across a gitlink, so this changes nothing about what git itself ignores in the submodule; it changes what the guard recognises. The matching moves into secrets_guard_is_hit, which both passes call, rather than a second copy of the component walk that could drift from the first. Reproduced on a scratch superproject whose submodule ignores decrypted_*, *.dec.* and junk/: the previous guard reported secrets/decrypted_probe.yaml and secrets/junk/artifact.bin, the new one reports only the decrypted probe. The superproject pass is unchanged, verified against the same tree: .env, test_probe.pem, id_probe_rsa, id_backup/notes.txt (the path-component case) and a new top-level decrypted_top.yaml are all reported, id_probe.pub still is not. The parser's expected-minimum set gains both patterns, so a regenerated .gitignore that drops them still fails closed rather than thinning the deny list in silence. The pattern lists in CLAUDE.md, AGENTS.md and docs/architecture/06-reference.md, the abort heading, the build.sh path-reference notice and docs/reference/cache-coverage.md all stated the old asymmetry and are corrected. Validation: bash -n; shellcheck over build.sh, scripts/lib/secrets-guard.sh and scripts/cache-coverage.sh; write-files regenerated .gitignore from modules/development/gitignore.nix; nix run path:.#treefmt over the seven touched files; scratch superproject and submodule probes above (removed with rip). * fix(cache-coverage): resolve the flake ref the way build.sh does The script hardcoded FLAKE_REF="path:${FLAKE_DIR}" with no bare-ref branch, so it made the unfiltered copy even in a primary checkout where the git+file ref works. Two things followed. The report measured a different tree than the build it gates: build.sh's resolve_installable takes path: only for a linked worktree or --allow-dirty and keeps the bare ref otherwise. And in the repo's ordinary state the guard turned a read-only report into a hard stop, because secrets/ holds decrypted SOPS output whenever sops has run, so `cache-cov…
The build-time shell guard recognized command substitution but missed process substitutions, where compgen follows <( or >( without whitespace. A future use under the scanned modules, packages, or tests could therefore pass the guard and fail in a completion-free runCommand builder. Extend the shared command-opening branch and add an eighth planted fixture so the hit count proves this coverage. Validation: nix-instantiate --parse modules/meta/build-time-shell.nix; nix build --accept-flake-config --option eval-cache false --no-link --print-build-logs --offline path:.#checks.x86_64-linux.build-time-shell; nix develop path:. --fallback -c pre-commit run --files modules/meta/build-time-shell.nix; nix flake check path:. --accept-flake-config --no-build --offline; bash tests/prune-old-stashes/run.sh; git diff --check.
The build-time shell detector covered dollar-parenthesis and process substitutions but missed legacy backtick command substitution. An assignment capture containing compgen could therefore pass the scan and fail in the completion-free runCommand builder. Add an assignment-capture anchor and a ninth planted fixture while preserving the command-position false-positive boundary. Validation: nix-instantiate --parse modules/meta/build-time-shell.nix; nix build --accept-flake-config --option eval-cache false --no-link --print-build-logs --offline path:.#checks.x86_64-linux.build-time-shell; nix develop path:. --fallback -c pre-commit run --files modules/meta/build-time-shell.nix; nix flake check path:. --accept-flake-config --no-build --offline; bash tests/prune-old-stashes/run.sh; git diff --check.
The assignment-anchored backtick branch still missed the original conditional assertion shape, where a quoted command substitution follows a shell opening. Extend the backtick branch to the realistic quote, assignment, grouping, pipeline, sequencing, and background openings, and add a tenth planted fixture so the widened alternative remains covered. Validation: nix-instantiate --parse modules/meta/build-time-shell.nix; nix build --accept-flake-config --option eval-cache false --no-link --print-build-logs --offline path:.#checks.x86_64-linux.build-time-shell; nix develop path:. --fallback -c pre-commit run --files modules/meta/build-time-shell.nix; nix flake check path:. --accept-flake-config --no-build --offline; bash tests/prune-old-stashes/run.sh; git diff --check.
| fileset = lib.fileset.unions [ | ||
| (lib.fileset.fileFilter (file: file.hasExt "nix") ../../modules) | ||
| (lib.fileset.fileFilter (file: file.hasExt "nix") ../../packages) | ||
| (lib.fileset.fileFilter (file: file.hasExt "sh") ../../tests) |
There was a problem hiding this comment.
[ENHANCEMENT] The header justifies excluding scripts/ with "scripts/ runs under the user's own bash, where the builtin exists". That is not true for the five scripts modules/meta/script-tests.nix copies into a runCommand and executes: scripts/prune-old-stashes.sh, scripts/prune-stale-worktrees.sh, scripts/git-worktree-remove-safe.sh, scripts/run-packages-updaters.sh and .github/scripts/upstream-tracker.sh (lines 13–75 there, run at line 144 via bash tests/${name}/run.sh after patchShebangs).
Those run under whatever pkgs.bash the suite happens to put on PATH — the exact dependency e87a1f6 removed from tests/prune-old-stashes/run.sh in this PR. tests/**/*.sh is scanned for that reason; the scripts those same builders execute are not, so a compgen -G guard added to scripts/prune-stale-worktrees.sh gets no coverage from this check while its failure mode is identical.
The replacements (declare -F, declare -p, a nullglob array) are plain builtins everywhere, so widening costs the user-facing invocation nothing. The header comment needs the matching correction.
| (lib.fileset.fileFilter (file: file.hasExt "sh") ../../tests) | |
| (lib.fileset.fileFilter (file: file.hasExt "sh") ../../tests) | |
| (lib.fileset.fileFilter (file: file.hasExt "sh") ../../scripts) |
| scan() { | ||
| local status=0 | ||
| grep -rnE \ | ||
| '^[[:space:]]*!?[[:space:]]*compgen\b|[$<>]\(!?[[:space:]]*compgen\b|["=(|;&]`!?[[:space:]]*compgen\b|(if|elif|while|until|then|else|do|;|&&|\|\|?|\{|\(|\))[[:space:]]+!?[[:space:]]*compgen\b' \ |
There was a problem hiding this comment.
[ENHANCEMENT] The operator alternatives require [[:space:]]+ after the token, so the unspaced spellings pass the scan: true;compgen -G "$out"/*.next, true|compgen -A function, true&&compgen -A function and (compgen -A function) are all valid bash and all matched by none of the four branches. ;, |, &&, {, (, ) do not need a following blank in bash; only the reserved words do.
The self-test cannot expose this, because every one of the ten planted fixtures (lines 80–89) writes the spaced form — 'true | %s -A function\n', '( %s -A function )\n', 'case x in *) %s -A function ;; esac\n' — so each proves only the branch it already takes. Splitting the operators out with [[:space:]]* while keeping [[:space:]]+ for the keywords closes it without loosening the keyword anchor:
| '^[[:space:]]*!?[[:space:]]*compgen\b|[$<>]\(!?[[:space:]]*compgen\b|["=(|;&]`!?[[:space:]]*compgen\b|(if|elif|while|until|then|else|do|;|&&|\|\|?|\{|\(|\))[[:space:]]+!?[[:space:]]*compgen\b' \ | |
| '^[[:space:]]*!?[[:space:]]*compgen\b|[$<>]\(!?[[:space:]]*compgen\b|["=(|;&]`!?[[:space:]]*compgen\b|[;&|(){}][[:space:]]*!?[[:space:]]*compgen\b|(if|elif|while|until|then|else|do)[[:space:]]+!?[[:space:]]*compgen\b' \ |
Verified against the tree as it stands: every existing compgen occurrence (this file's own message at line 99 and comment at line 8, m365-check.nix:640, tests/prune-old-stashes/run.sh:1584) is preceded by :, a space or a backtick, none of which is in the new class, so the widened branch adds no false positive. Plant the unspaced forms alongside the spaced ones and raise the expected count so the new branch is proven too.
Summary
The initial implementation, validation fixes, and review remediations are described below.
1.
feat(browsers): Microsoft 365 web apps through firefoxpwafirefoxpwa keeps its site list in a profile database under
$XDG_DATA_HOME/firefoxpwa, so a suite clicked together inthe browser extension survives neither a reinstall nor a move to another host.
packages/firefoxpwa-site-installer: the shared builder every firefoxpwa site installer is made with. It suppliesthe prelude they all need (
data_dir,config_file,FFPWA_USERDATA,XDG_DATA_HOME, the 0700 directory) and takesthe lock that keeps two of them off
config.jsonat once, which firefoxpwa rewrites whole throughFile::createwith no lock of its own. Both existing installers lose their duplicated preludes to it, and an installer that skips
the lock cannot exist, since that is how they are built.
packages/firefoxpwa-m365-install: oneshot installer built from the DMail installer's site bookkeeping (the ulid,origin and applied-URL records kept next to
config.json, plus the pending record covering the kill window insidefirefoxpwa site install), with a per-entry loop on top. An entry the installer decides it must not act on iscounted and reported at the end rather than aborting the run, so one refusal does not cost the remaining entries
their install. A failed write is not a refusal: it aborts under
errexit, because the records are whatauthenticate a site on the next run.
modules/browsers/firefoxpwa/m365.nix:firefoxpwa-m365user service, layered onto the existingbrowsers.firefoxpwaHome Manager key. No secret to wait for, so it is ordered against nothing: itsExecStartpathchanges with the app table and
systemd.user.startServices = "sd-switch"restarts it on the switch that changes it.That is the only switch it runs on, so transient installer faults retry themselves (
Restart=on-failure,RestartSec=1080, bounded byStartLimitIntervalSec=10800andStartLimitBurst=5) instead of sittingfaileduntil the next login. Permanent user-action refusals return
EX_CONFIG78, which the unit treats as successful andnon-restarting. Their starts still count toward that five-start window, which leaves room for corrective switches
while pacing fast retries across the same window. Ordered
Afterthe DMail unit, since firefoxpwa rewrites the whole ofconfig.jsonwith no lock and the two wouldotherwise start together at login. The lock is what actually keeps them apart; the ordering keeps the common case
from reaching it.
modules/browsers/firefoxpwa/apps.nix:programs.firefoxpwa.m365.{enable,apps}at NixOS scope, next to the existingdmail.enable, with assertions against duplicate keys and names.modules/browsers/firefoxpwa/_m365-apps.nix: the default catalog (Microsoft 365, Word, Excel, PowerPoint, Outlook,OneNote, OneDrive).
Start URLs are the bare
cloud.microsoftorigins: the manifest scope is fixed at install time andfirefoxpwa site updatecannot rewrite it, so scope has to be the origin, and the landing paths Microsoft redirects to(
/en-us/,/mail/,/tasks/) are locale or tenant dependent. Checked on 2026-08-04 and left out on purpose:teams.cloud.microsoftanswers Gecko with/v2/unsupported-browser,visio.cloud.microsoftredirects tom365.cloud.microsoftand would leave its own scope on first load, andclipchamp.cloud.microsoftdoes not resolve.programs.firefoxpwa.m365.enabledefaults to false and no host sets it yet.2.
fix(checks): stop trusting compgen in build-time shell textThe leftover-temporary assertion first written for the m365 check reported a pass on every run while checking nothing:
the bash a
runCommandbuilder runs is built without programmable completion, socompgen -Gresolved to no command,and a condition context turns that into a false rather than an error. Probed both interpreters to confirm the split:
type -t compgenin a plainrunCommandreports MISSING, a bash fromnativeBuildInputsreports HAVE.tests/prune-old-stashes/run.shused the same builtin for its defined-but-never-ran guard, the one thing stopping atest function from being added and silently never called. It passes today only because
modules/meta/script-tests.nixputspkgs.bash(resolved to bash-interactive) on PATH. Nowdeclare -F.m365-check.nixproves the detector before trusting it: a planted.nextfile has to be reported first.checks.build-time-shellscansmodules/,packages/andtests/for the builtin at a command position, plain ornegated, and plants both spellings the scan must hit before scanning the tree, so the guard cannot go quiet the way
the assertion did.
scripts/is out of scope: it runs under the user's own bash.3.
feat(meta): run statix across the whole tree as a flake checkstatix reached only staged files.
modules/meta/pre-commit.nixinvokeshook-statixwith filenames and nothing in.github/workflows/check.ymlruns statix at all, so a lint in a file nobody edits again was reachable on mainindefinitely. The repeated
optionsassignment fixed inapps.nixon this branch was caught only because that filehappened to be staged.
checks.statix-treerunshook-statixwith no arguments (its own whole-tree branch), so CI andpre-commit share one binary and one set of lints.
passthru.runtimeCheckopts it into the workflow's runtime buildstep, which discovers checks by that marker, so no workflow edit is needed. Source narrowed to
.nixfiles throughlib.fileset, minusdocs/nixos-manual/, the upstream mirror both the hook and treefmt already exclude. A lint isplanted and has to be reported before the tree is scanned, since nothing else calls
hook-statixwithout arguments.The tree is clean under it today.
4. Review round
fix(browsers):install_appran as the left side of|| failed=$((failed + 1)), and bash ignores-efor thewhole body of a function invoked that way, including every compound command inside it. A
recordthat failed on afull or read-only filesystem was silent, the pending record was then removed, the entry was reported installed, and
every later run refused it as foreign with no recovery short of
firefoxpwa site uninstall. The per-entry status isnow collected by a
refusehelper, soinstall_appis called as a plain command anderrexitcovers the recordwrites, the pending write, the manifest
jqand thebase64.fix(browsers): the installer returns non-zero for a site that registered and then failed, expecting the next run torepair it, but nothing produced that run: sd-switch restarts a unit only when its own text changed, and
RemainAfterExitkeeps the failed unit there until the next login.dmail.nixgets its rerun fromPartOf = sops-nix.service; this unit now gets it from a boundedRestart=on-failurefor retryable faults.Permanent refusals use
EX_CONFIG78, and the five-start window accounts for the refusal and corrective switcheswhile the retry delay paces fast faults across that window. The
programs.firefoxpwa.m365.enabletext that claimed failure "on every switch and login" is corrected with it.feat(browsers):apps.*.urlwas a barestr, som365.cloud.microsoftorhtps://...built fine and surfacedonly as a runtime refusal in a user unit's journal. Now
strMatching "https?://[^[:space:]]+".Round two, on the commits above:
fix(checks): the command-position alternation requiredcompgento follow the keyword or operator immediately, soif ! compgen -G ...matched none of its branches and passed the tree scan. Each alternative now admits an optional!, and the self-test plants both spellings and requires two hits, since a fixture for one of them proves only thebranch it happens to take.
test(browsers): nothing applied thekeyoption'sstrMatchingto the shipped catalog, because an option's typeis checked when its value is forced and no host forces this one. Asserted next to the duplicate-key and https
assertions already there for that reason.
test(browsers): the stub could failsite installtwo ways but neversite update, leaving untested that a failedupdate does not advance
applied_fileand so is retried rather than swallowed by the no-op fast path.Round three:
fix(checks): the scan helper ended in|| true, which collapsed grep's three-valued status. 1 is "matchednothing", but 2 and up mean grep could not read a path it was given, and both produced an empty result and a
reported pass, the same fail-open shape the check exists to catch. The status is now returned when it exceeds 1, so
both callers abort.
Round four:
fix(meta):checks.statix-treescanneddocs/nixos-manual/, the vendored upstream mirror that both the pre-commithook and treefmt exclude, so the check was strictly wider than the hook it shares a binary with and the next
update-nixos-manual.shsync could fail CI on code this repo cannot fix.test(meta): nothing else callshook-statixwithout arguments, so a clean$outwas indistinguishable from a runthat walked nothing. An empty let-in is planted and has to be reported first.
Round five:
fix(browsers): nothing orderedfirefoxpwa-m365.serviceagainstfirefoxpwa-dmail.service. Both areWantedBy = [ "default.target" ]and both drivefirefoxpwa site install, which rewritesconfig.jsonunlocked, soon a host with both toggles on they race at login and the later writer drops the other's site.
tpnixalready setsdmail.enable, so this was onem365.enableaway from being live.fix(browsers): editing an entry'snamewhile keeping itskeylanded in the fresh-install branch, registering asecond site and overwriting the only record of the first, leaving the original app and its launcher orphaned in
silence. Refused now, told apart from the documented uninstall by the recorded ulid still being present in
config.json. The stub's ulid allocation was derived from the site count, so it reused the id of a deleted site;a counter now, since a real ulid is unique for the life of the profile.
Round six:
fix(browsers): the ordering added in round five does not close the race.After=is enforced across transactions,not only within one, but Home Manager activates
reloadSystemdbefore the sops-nix step, so a switch starts the m365unit and only then restarts the dmail unit through
PartOf, the direction anAfter=on the m365 unit does notconstrain. The lock in the shared builder is the fix, and it is in the builder rather than in the two installers
because more site installers are coming and a lock only some of them take is not a lock.
test(browsers):deepSeqforced theAfter=value but asserted nothing about it, and systemd ignores an orderingdependency naming a unit that does not exist, so a typo would have been silent at switch and at login.
Round seven:
fix(browsers): the lock acquire blocked unbounded and silent, so a queued installer could wait past the usermanager's default
TimeoutStartSecof 90s and be killed with nothing in the journal naming the lock. It nowannounces the wait before blocking, and both units raise
TimeoutStartSecto 900, since being killed for waiting isnot a fault and the default is shorter than a legitimate wait.
test(browsers):site-lock-check.nixproves the builder serializes and deliberately names no installer, which leftnothing proving today's two go through it. Each installer's own check now greps its built text for the shared lock
path, so a rewrite back to a direct
writeShellApplicationfails.Round eight:
docs(browsers):keyreads as a naming detail but every record path ism365-<key>-*, so editing it while keepingnamemoves the records to a slug that has none and the entry hits the refusal that tells the user to runfirefoxpwa site uninstall, destroying a working PWA profile. Documented, and pinned by a check.fix(checks): the site-lock control's result was a race outcome decided by a fixed pause, so a busy builder couldfail it on a change that touched nothing. The two runs now rendezvous, which makes the outcome depend on whether the
peer can enter rather than on which process starts first.
Round nine:
fix(browsers): the duplicate-name assertion only saw the m365 list, but the launcher name is the idempotency key ina
config.jsonevery site installer shares, so an m365 entry namedDMailcollided with the DMail site and hit arefusal whose only remedy destroys that site's PWA profile.
programs.firefoxpwa.siteNamesis now an internalregistry each enabled site contributes to, with one assertion over the union, so a site added later joins by
contributing rather than by someone widening a comparison. New
checks.browsers/firefoxpwa-apps-evalforces theoption module, which no host does.
docs(browsers): the m365 module header still said the unit "is ordered against nothing" after theAfter=went inbelow it.
Round ten:
fix(browsers): promoting the DMail launcher name to an option in round nine made an edit reachable that the DMailinstaller had no guard for, and it is worse than the m365 case it mirrors: those records are keyed by the entry's
slug, so a name edit lands on a missing-record refusal, while these paths are fixed and the installer silently
registers a second site and overwrites the only records of the first. Same guard ported over, plus the option text
and a check case.
Round eleven, both on assertions that could not fail:
test(browsers): the m365 stub readmanifest.scopefrom--document-urlrather than from thedata:manifest theinstaller built, and every entry declared a start URL equal to its own origin, which normalizes identically either
way. Building the manifest with
scope: $urlkept the check green while the real firefoxpwa would install a scopethat excludes nothing. The stub decodes the manifest now, and a new entry installs below its own origin, which is
what discriminates.
test(browsers): the dmail rename assertions were invariant under a stub that writes every install to one hardcodedulid, so removing the guard overwrote the site rather than adding one and only the exit status went red. The site's
own name is now the observable.
Rounds twelve to fifteen, in the same vein:
fix(browsers): the rename guards readjq -efailure as "the recorded site is gone", so an unreadableconfig.jsonfell into the fresh-install branch and duplicated the site. Both scripts separate those two statuseseverywhere else they read that file.
test(browsers): the dmail reinstall cases matched"installed", which is also a substring of "already installedwith current URL", so they pinned neither the branch nor the name.
test(browsers): both m365 rerun cases passed an empty message fragment, so deleting the installer's no-op fast pathleft every assertion green. Silence is the observable now.
feat(browsers): theurltype admitted userinfo, so a start URL embedding credentials built fine and surfaced onlyas the installer's runtime refusal. These are static strings, unlike the DMail secret, so it is rejected at eval.
Rejected, with the reasoning recorded in both check files: the reviewed claim that
case "$out" in *"$want_text"*) [ "$rc" = "$want_rc" ] && ok=0 ;; esacaborts the builder undererrexitwhen thefragment matches but the status does not. It does not, for the same reason the installer defect above existed: bash
ignores
-efor the left side of an&&list and the enclosing compound inherits that state.Round twenty:
fix(meta): the build-time shell detector matched command substitution but missed process substitution,allowing
compgenimmediately after a process-substitution opening to pass the tree scan. The sharedcommand-opening branch now covers both process-substitution directions, and the planted self-test requires
all eight command-position fixtures.
Round twenty-one:
fix(meta): the detector also missed legacy backtick command substitution, so an assignment capture couldpass the tree scan. An assignment-capture anchor and a ninth planted fixture now cover this form without
matching the scanned prose that documents
compgen.Round twenty-two:
fix(meta): the assignment-anchored backtick branch missed the quoted conditional assertion shape. Thebacktick branch now covers the realistic shell openings for this form, and a tenth planted fixture proves
the widened alternative.
Test plan
nix fmtnix flake check path:. --accept-flake-config --no-build --offline(exit 0)nix build path:.#checks.x86_64-linux."browsers/firefoxpwa-m365": drives the real installer against a stub firefoxpwathrough a fresh install, an idempotent rerun, a same-origin move applied with
site update, a cross-origin moverefused, a start URL carrying credentials refused before any site lookup, a same-named site the unit did not install
refused rather than adopted, an entry that never registers not stopping the others, an install that registers and then
fails repaired on the next run rather than duplicated, a record write that fails stopping the run with the pending
record intact and recovering on the next run, an entry renamed onto its own live site refused while the same stale
record after an uninstall still reinstalls, the shipped catalog installed end to end, the leftover-temporary
detector planted before it is trusted, and a failed
site updatecounted as a refusal without advancing theapplied-URL record, and an entry installed below its own origin still scoped to the origin. 64 assertions. Restoring the
|| failed=$((failed + 1))call site turns thethree new ones red.
nix build path:.#checks.x86_64-linux."browsers/firefoxpwa-dmail"nix build path:.#checks.x86_64-linux."browsers/firefoxpwa-site-lock": two installers from the shared builderserialize on a read-rendezvous-write counter (ends at 2); the same pair built with
writeShellApplicationdirectlyloses an update (ends at 1), which is what shows the assertion can fail. Confirmed against a deliberate 3-second
startup skew
nix eval path:.#checks.x86_64-linux."browsers/firefoxpwa-apps-eval".drvPath: the launcher-name collision, theshipped catalog alongside DMail passing, a name claimed only while its site is enabled, and duplicate keys. Dropping
the DMail registration turns the first red
nix eval path:.#checks.x86_64-linux."browsers/firefoxpwa-module-eval".drvPath: extended with the m365 unit, theempty-app-list warning and the disabled cases, since the module's
mkIfblocks are unforced while no host enables thetoggle. Forces the whole m365 unit, so the restart policy is evaluated too.
systemd-analyze verify --useron the renderedfirefoxpwa-m365.service, ordering included: exit 0, and exit 1 withRestart=alwaysin its place, which is what makes the acceptance meaningful.nix build path:.#checks.x86_64-linux.build-time-shell: passes on the clean tree, and its ten plantedcommand-position fixtures include process substitution, backtick assignment capture, and a quoted
conditional backtick form. It fails with the expected message when a
tests/_scratch/probe.shusing thebuiltin is planted. Fails with exit code 2 and the path named when an unmaterialized directory is added to
the scan arguments.
nix build path:.#checks.x86_64-linux.statix-tree: the materialized source has nodocs/nixos-manual, and thecheck fails both with a
let in { }planted undermodules/and with the zero-argument branch stubbed toexit 0nix build path:.#checks.x86_64-linux.script-tests-prune-old-stashes(47 passed)nix develop path:. --fallback -c pre-commit run --files <changed files>(deadnix, nix-parse, shellcheck, statix,treefmt, typos)