Review generated lifecycle artifacts #2

Merged

vnprc merged 1 commit from agent/review-generated-lifecycle-artifacts into master

2026-06-27 14:12:39 +01:00

vnprc-agent commented

2026-06-27 05:02:20 +01:00

Contributor

Closes Allod/memory#1

Summary

Update shared Allod review and provisioning memory so future plan reviews do not stop at source-level Nix evaluation when a change affects NixOS, Home Manager, provisioning, systemd, or wrapper-script lifecycle behavior.

This is a direct process fix for the allod-dev first-build failure. The failure was not a Nix evaluation problem: the allod-dev toplevel built successfully. The bug was in a generated runtime artifact. profiles/modules/netrc.nix used exit 0 when /root/.git-credentials was absent. In a NixOS module, system.activationScripts.*.text snippets are concatenated into one generated activate script, so that exit 0 returned from the whole activation script. Activation stopped before the final /run/current-system symlink was created, and nixos-anywhere later tried to run /run/current-system/bin/switch-to-configuration, which did not exist. The VM disk was left partially installed and UEFI had no bootable system.

Several reviews missed this because the review and acceptance-test guidance pushed reviewers toward structural consistency, closure scans, credential inventory checks, and nix build success. Those checks were useful, but they did not require inspection of generated activation scripts or absent-resource paths such as a tokenless VM with no /root/.git-credentials. The rubric already classified the outcome as a blocker because it caused provisioning to fail and left the system nonfunctional; the missing piece was explicit coverage guidance.

Changes

templates/plan-review-prompt.md: add a review guideline requiring generated lifecycle artifact inspection for NixOS, Home Manager, provisioning, systemd, and wrapper-script changes. The guideline explicitly calls out generated activation scripts, units, wrappers, install/boot phases, and negative paths for missing optional secrets, credentials, files, or host state.
templates/plan-review-prompt.md: extend the blocker rubric so first boot, activation, provisioning, rebuild, and rollback lifecycle breakage is classified as [BLOCKER] instead of being softened into a generic test gap.
dev-plans.md: strengthen Acceptance Tests guidance so plans must include generated-artifact and absent-resource checks when lifecycle-generating systems are involved, not only source evaluation or happy-path builds.
dev-plans.md: add Generated lifecycle behavior as a standing review focus area alongside consistency, sequencing, acceptance coverage, and rollback fidelity.
vm-provisioning.md: replace the ambiguous activation-script "early exit" guidance with a NixOS-specific rule: use a conditional no-op for missing optional resources and do not exit 0 from activation snippets, because snippets are concatenated and exiting aborts the whole activation before /run/current-system is linked.

Validation

git diff --check
rg -n "early exit|exit 0|generated lifecycle|absent-resource|Activation scripts must tolerate" . -g '*.md'

Closes Allod/memory#1 ## Summary Update shared Allod review and provisioning memory so future plan reviews do not stop at source-level Nix evaluation when a change affects NixOS, Home Manager, provisioning, systemd, or wrapper-script lifecycle behavior. This is a direct process fix for the allod-dev first-build failure. The failure was not a Nix evaluation problem: the allod-dev toplevel built successfully. The bug was in a generated runtime artifact. `profiles/modules/netrc.nix` used `exit 0` when `/root/.git-credentials` was absent. In a NixOS module, `system.activationScripts.*.text` snippets are concatenated into one generated `activate` script, so that `exit 0` returned from the whole activation script. Activation stopped before the final `/run/current-system` symlink was created, and `nixos-anywhere` later tried to run `/run/current-system/bin/switch-to-configuration`, which did not exist. The VM disk was left partially installed and UEFI had no bootable system. Several reviews missed this because the review and acceptance-test guidance pushed reviewers toward structural consistency, closure scans, credential inventory checks, and `nix build` success. Those checks were useful, but they did not require inspection of generated activation scripts or absent-resource paths such as a tokenless VM with no `/root/.git-credentials`. The rubric already classified the outcome as a blocker because it caused provisioning to fail and left the system nonfunctional; the missing piece was explicit coverage guidance. ## Changes - `templates/plan-review-prompt.md`: add a review guideline requiring generated lifecycle artifact inspection for NixOS, Home Manager, provisioning, systemd, and wrapper-script changes. The guideline explicitly calls out generated activation scripts, units, wrappers, install/boot phases, and negative paths for missing optional secrets, credentials, files, or host state. - `templates/plan-review-prompt.md`: extend the blocker rubric so first boot, activation, provisioning, rebuild, and rollback lifecycle breakage is classified as `[BLOCKER]` instead of being softened into a generic test gap. - `dev-plans.md`: strengthen Acceptance Tests guidance so plans must include generated-artifact and absent-resource checks when lifecycle-generating systems are involved, not only source evaluation or happy-path builds. - `dev-plans.md`: add `Generated lifecycle behavior` as a standing review focus area alongside consistency, sequencing, acceptance coverage, and rollback fidelity. - `vm-provisioning.md`: replace the ambiguous activation-script "early exit" guidance with a NixOS-specific rule: use a conditional no-op for missing optional resources and do not `exit 0` from activation snippets, because snippets are concatenated and exiting aborts the whole activation before `/run/current-system` is linked. ## Validation ```bash git diff --check rg -n "early exit|exit 0|generated lifecycle|absent-resource|Activation scripts must tolerate" . -g '*.md' ```

vnprc-agent added 1 commit

2026-06-27 05:02:21 +01:00

Review generated lifecycle artifacts c51d6f1130

vnprc-agent commented

2026-06-27 05:02:45 +01:00

Author

Contributor

Read-only review pass: no findings.

Checked the final diff against the allod-dev provisioning failure mode. The template now forces reviewers to inspect generated lifecycle artifacts and negative paths, the dev-plan guidance makes those checks part of acceptance testing, and the provisioning memory no longer recommends an ambiguous early-exit pattern for NixOS activation snippets.

Validation run locally:

git diff --check
rg -n "early exit|exit 0|generated lifecycle|absent-resource|Activation scripts must tolerate" . -g '*.md'

Read-only review pass: no findings. Checked the final diff against the allod-dev provisioning failure mode. The template now forces reviewers to inspect generated lifecycle artifacts and negative paths, the dev-plan guidance makes those checks part of acceptance testing, and the provisioning memory no longer recommends an ambiguous early-exit pattern for NixOS activation snippets. Validation run locally: ```bash git diff --check rg -n "early exit|exit 0|generated lifecycle|absent-resource|Activation scripts must tolerate" . -g '*.md' ```