Align allod-dev plan with tokenless provisioning #7

Merged

vnprc merged 2 commits from agent/align-agent-isolation-tokenless-allod-dev into master

2026-06-27 14:23:35 +01:00

vnprc-agent commented

2026-06-27 05:09:36 +01:00

Contributor

Closes #6

Related fixes:

vnprc/profiles#74 fixed the actual activation bug by preventing profiles/modules/netrc.nix from exiting the combined NixOS activation script when /root/.git-credentials is absent.
allod/memory#2 updates the shared review template and memory guidance so future plans require generated lifecycle-artifact checks.
Allod/strategy#5 tracks the broader inventory-owned platform cleanup so platform literals do not spread back into consumer code.

Problem

The first allod-dev provisioning run failed after agenix activation with:

netrc activation: /root/.git-credentials missing or empty, skipping
/run/current-system/bin/switch-to-configuration: No such file or directory

The root implementation bug was that netrc.nix used exit 0 for a missing optional root git credential file. In a NixOS activation snippet that does not exit only the local snippet: the snippets are concatenated into one activation script, so it exited before /run/current-system was linked. That made the first install appear to complete far enough to write secrets but left the VM unbootable.

allod-dev exposed this because it is intentionally tokenless for root HTTPS git credentials. It clones public repos over SSH with the allod-agent key and uses only the raw API token for the Forge CLI. The active agent-isolation plan was stale: it still described forgejo-https-token-allod-dev.age as an allod-dev credential and treated both forgeTokenFile and agentTokenFile as regular token files.

Review Classification

The root bug should be graded [BLOCKER] under the review rubric because following the implementation path made first provisioning/activation leave the VM nonfunctional. The missing acceptance test that would have caught it is a [GAP]: source evaluation and profile builds were not enough because the failure was visible only in the generated activation lifecycle path for an absent optional credential.

Previous review passes missed it because they focused on repo boundaries, closure leaks, secret file shape, and source-level Nix evaluation. They did not require reviewers to inspect generated activation scripts or negative paths where optional host state is intentionally absent. The active review prompt also had stale current-state context saying allod-dev and the public template repos did not exist, which made it less useful for post-implementation regression review.

Changes

This PR updates dev-plans/agent-isolation-dev-plan.md to:

make forgeTokenFile = null the explicit allod-dev contract;
remove the obsolete allod-dev HTTPS credential-store token and gate;
keep only the raw agent-pr-token-allod-dev API token in the plan;
require mkDevVm to accept httpsTokenFile and only declare the root HTTPS token secret when it is non-null;
add an allod-dev generated activation check that scans the built activation script for unsafe exit 0 behavior and verifies the expected skip message and /run/current-system lifecycle path are present;
derive profile check systems from the target configuration instead of hardcoding checks.x86_64-linux;
derive allowed platform overlaps from inventory.lib.supportedPlatforms instead of embedding the platform in consumer checks;
clarify that platform strings belong in inventory data and lib.supportedPlatforms, while consumers derive them.

This PR also updates review-plans/agent-isolation-dev-plan-review.md to:

require generated lifecycle-artifact inspection in the active review prompt;
classify first boot, activation, provisioning, rebuild, or rollback breakage as [BLOCKER] severity;
refresh current-state context so future reviews treat this as a follow-up/regression prompt for the implemented allod-dev plan, not a pre-implementation prompt.

Validation

git diff --check

rg -n "does not exist yet|do not exist yet|Four prerequisites|forgejo-https-token-allod-dev|gates? 6-9|gates? 15-18|gate 18|checks\\.x86_64|\\.x86_64-linux" \
  dev-plans/agent-isolation-dev-plan.md review-plans/agent-isolation-dev-plan-review.md

The stale-pattern scan above returns no matches. The remaining x86_64-linux examples in the plan are inside inventory snippets, which is the intended source of truth.

rg -n "forgeTokenFile|httpsTokenFile|generated activation|supportedPlatforms|/root/.git-credentials|exit 0|Platform strings belong|Inspect generated lifecycle artifacts|first boot, activation|Five prerequisites|follow-up corrections" \
  dev-plans/agent-isolation-dev-plan.md review-plans/agent-isolation-dev-plan-review.md

The positive scan confirms the new tokenless contract, inventory-derived platform guidance, generated activation acceptance test, review severity language, and refreshed prompt context are present.

Closes #6 Related fixes: - vnprc/profiles#74 fixed the actual activation bug by preventing `profiles/modules/netrc.nix` from exiting the combined NixOS activation script when `/root/.git-credentials` is absent. - allod/memory#2 updates the shared review template and memory guidance so future plans require generated lifecycle-artifact checks. - Allod/strategy#5 tracks the broader inventory-owned platform cleanup so platform literals do not spread back into consumer code. ## Problem The first allod-dev provisioning run failed after agenix activation with: ```text netrc activation: /root/.git-credentials missing or empty, skipping /run/current-system/bin/switch-to-configuration: No such file or directory ``` The root implementation bug was that `netrc.nix` used `exit 0` for a missing optional root git credential file. In a NixOS activation snippet that does not exit only the local snippet: the snippets are concatenated into one activation script, so it exited before `/run/current-system` was linked. That made the first install appear to complete far enough to write secrets but left the VM unbootable. allod-dev exposed this because it is intentionally tokenless for root HTTPS git credentials. It clones public repos over SSH with the allod-agent key and uses only the raw API token for the Forge CLI. The active agent-isolation plan was stale: it still described `forgejo-https-token-allod-dev.age` as an allod-dev credential and treated both `forgeTokenFile` and `agentTokenFile` as regular token files. ## Review Classification The root bug should be graded `[BLOCKER]` under the review rubric because following the implementation path made first provisioning/activation leave the VM nonfunctional. The missing acceptance test that would have caught it is a `[GAP]`: source evaluation and profile builds were not enough because the failure was visible only in the generated activation lifecycle path for an absent optional credential. Previous review passes missed it because they focused on repo boundaries, closure leaks, secret file shape, and source-level Nix evaluation. They did not require reviewers to inspect generated activation scripts or negative paths where optional host state is intentionally absent. The active review prompt also had stale current-state context saying allod-dev and the public template repos did not exist, which made it less useful for post-implementation regression review. ## Changes This PR updates `dev-plans/agent-isolation-dev-plan.md` to: - make `forgeTokenFile = null` the explicit allod-dev contract; - remove the obsolete allod-dev HTTPS credential-store token and gate; - keep only the raw `agent-pr-token-allod-dev` API token in the plan; - require `mkDevVm` to accept `httpsTokenFile` and only declare the root HTTPS token secret when it is non-null; - add an allod-dev generated activation check that scans the built activation script for unsafe `exit 0` behavior and verifies the expected skip message and `/run/current-system` lifecycle path are present; - derive profile check systems from the target configuration instead of hardcoding `checks.x86_64-linux`; - derive allowed platform overlaps from `inventory.lib.supportedPlatforms` instead of embedding the platform in consumer checks; - clarify that platform strings belong in inventory data and `lib.supportedPlatforms`, while consumers derive them. This PR also updates `review-plans/agent-isolation-dev-plan-review.md` to: - require generated lifecycle-artifact inspection in the active review prompt; - classify first boot, activation, provisioning, rebuild, or rollback breakage as `[BLOCKER]` severity; - refresh current-state context so future reviews treat this as a follow-up/regression prompt for the implemented allod-dev plan, not a pre-implementation prompt. ## Validation ```bash git diff --check ``` ```bash rg -n "does not exist yet|do not exist yet|Four prerequisites|forgejo-https-token-allod-dev|gates? 6-9|gates? 15-18|gate 18|checks\\.x86_64|\\.x86_64-linux" \ dev-plans/agent-isolation-dev-plan.md review-plans/agent-isolation-dev-plan-review.md ``` The stale-pattern scan above returns no matches. The remaining `x86_64-linux` examples in the plan are inside inventory snippets, which is the intended source of truth. ```bash rg -n "forgeTokenFile|httpsTokenFile|generated activation|supportedPlatforms|/root/.git-credentials|exit 0|Platform strings belong|Inspect generated lifecycle artifacts|first boot, activation|Five prerequisites|follow-up corrections" \ dev-plans/agent-isolation-dev-plan.md review-plans/agent-isolation-dev-plan-review.md ``` The positive scan confirms the new tokenless contract, inventory-derived platform guidance, generated activation acceptance test, review severity language, and refreshed prompt context are present.

vnprc-agent added 1 commit

2026-06-27 05:09:36 +01:00

Align allod-dev plan with tokenless provisioning ac36b16e03

vnprc-agent added 1 commit

2026-06-27 05:10:48 +01:00

Refresh allod-dev review context 5781418b3f

vnprc-agent commented

2026-06-27 05:11:40 +01:00

Author

Contributor

Read-only review pass after commits ac36b16 and 5781418: no BLOCKER, GAP, SIMPLIFY, or QUESTION findings.

Checked:

branch is clean and pushed;
git diff --check origin/master...HEAD passes;
stale allod-dev HTTPS token, stale gate ranges, stale pre-implementation review context, and hardcoded checks.x86_64 patterns are absent;
the plan now encodes tokenless allod-dev provisioning, generated activation-script inspection, and inventory-derived platform guidance;
the active review prompt now treats generated lifecycle artifacts as review scope and first boot/activation/provisioning/rebuild/rollback breakage as [BLOCKER] severity.

Read-only review pass after commits `ac36b16` and `5781418`: no BLOCKER, GAP, SIMPLIFY, or QUESTION findings. Checked: - branch is clean and pushed; - `git diff --check origin/master...HEAD` passes; - stale allod-dev HTTPS token, stale gate ranges, stale pre-implementation review context, and hardcoded `checks.x86_64` patterns are absent; - the plan now encodes tokenless allod-dev provisioning, generated activation-script inspection, and inventory-derived platform guidance; - the active review prompt now treats generated lifecycle artifacts as review scope and first boot/activation/provisioning/rebuild/rollback breakage as `[BLOCKER]` severity.