Skip to content

Replace ineffective PR-preview flake retention guidance #301

Description

@davidvornholt

New infrastructure adopters can reproduce the same garbage-collection outage that disabled every FESK PR preview: the canonical guide tells them to retain a source path that may differ from the path embedded in the forced command. After Nix collects the embedded path, every host-local preview reconcile fails before convergence. This affects every consumer that copies the canonical PR-preview pattern.

Background

The forced preview SSH command rebuilds a host from a flake path embedded in its generated shell library. Nix keeps that path alive only when the generated derivation retains the path’s string context. The current guide in .agents/skills/declarative-infra/references/pr-previews.md instead recommends system.extraDependencies = [ ../.. ]. That can produce a second store copy and does not make the command own the exact path it reads.

Evidence

In fes-kirchheim/fesk-infra PR #48, the old command embedded /nix/store/…-source/infra, while system.extraDependencies retained /nix/store/…-infra. A disposable negative-control build kept the new closure test but restored toString plus system.extraDependencies; it failed with outside the command closure: /nix/store/…-source. The fixed command uses context-preserving interpolation and directly retains both /nix/store/…-infra and /nix/store/…-images.json.

Concrete failure scenario

A consumer follows the guide, deploys successfully, and later runs Nix garbage collection. The flake path named by /run/current-system/sw/bin/<repo>-pr-preview-deploy is not in that command’s closure and is collected. The next scheduled reconcile reaches nixos-rebuild --flake <collected-path>#<host> or reads a file below that path and fails. Preview environments remain unavailable until another production activation replaces the command.

Suggested change

Update the canonical pattern to require context-preserving interpolation for every runtime store path embedded in the forced command. Recommend passing separately read files as explicit path substitutions. Add a flake check that extracts store paths named by the generated command and its sourced libraries, then requires each path to appear in the command’s exported reference graph. Remove the system.extraDependencies recommendation for this purpose.

Suggested verification

  • A negative control using toString fails the closure-reference check.
  • The corrected forced command’s nix-store -qR output contains the exact embedded flake and image-file paths.
  • The consumer host toplevel builds without system.extraDependencies.
  • A representative consumer syncs the canonical change and passes nix flake check.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    deferred-findingDeferred review finding filed by a review-fix cycle

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions