Add required runtime fact for non-hypervisor inventory machines #10

Merged

vnprc merged 3 commits from agent/inventory-runtime-fact into master 2026-07-30 22:33:55 +01:00

Conversation 5 Commits 3 Files changed 3 +278 -14

allod-agent commented 2026-07-29 20:28:38 +01:00

Member

Copy link

Every non-hypervisor machines.<name> entry now declares runtime = "libvirt" or runtime = "microvm", validated by an assertion chain that mirrors the repo's existing platform checks: a missing, non-string, or unknown value fails evaluation with a named error listing the offending machine names, before any consumer can see partial or defaulted data. Hypervisor entries (nexus) are exempt by scope, not by a default — they are filtered out before the runtime checks run at all, so they can never acquire a fake guest runtime. Both public example machines declare libvirt. allod-dev in particular is held there by its own check: it is the machine the operator develops from, and a real machine rather than a synthetic example — allod/secrets carries key material keyed to that name — so it is the last machine to move onto a new runtime, not the first. The machine that first selects microvm is added in the same change that provisions it, because a machine entry is not self-contained: it needs a matching identity, a profile, and per-machine encrypted credentials whose creation needs a host key generated on the host. The runtime enum's microvm branch is covered by the mutation fixtures below rather than by an example machine.

lib.vmSpecsJson now exports runtime alongside the existing host-facing fields, and the committed scripts/vm-specs.json is regenerated so the two agree exactly. The internal mkVmSpecs/mkVmSpecsJson helpers take an explicit machine set rather than closing over machines, so the new runtime-fact-mutations check can run the identical validation chain against sabotaged copies and prove each failure mode actually fails rather than merely asserting on the real data.

This PR is the inventory half of Interface Contract 1 only. The archetypes.vmFacts.<name>.runtime half of the contract, and the guest-module selection that consumes this fact, land in later milestones per the plan's Implementation Sequence; this PR does not touch allod/vm, allod/nexus, allod/archetypes, or allod/profiles.

Refs allod/strategy#20

Risk

R2 Medium, matching the plan's risk table for this milestone: this changes a machine-data contract consumed by host scripts and, in a later milestone, vmFacts, but rollback is a plain revert and both the Nix-side value and the committed JSON are checked against each other on every evaluation. The residual risk worth a human's attention is schema propagation into the generated JSON and the missing/unknown-value failure behavior, both exercised below.

Validation

All commands were run against this branch's worktree in allod/inventory.

nix flake check --print-build-logs passes all three checks: vm-specs-json, repository-registry, and the new runtime-fact-mutations.

nix eval .#lib.vmSpecsJson --raw | jq -e '
  to_entries
  | all(.value.runtime == "libvirt" or .value.runtime == "microvm")
'

returns true.

diff -u <(nix eval .#lib.vmSpecsJson --raw | jq -S .) \
  <(jq -S . scripts/vm-specs.json)

produces an empty diff.

Mutation/sabotage coverage, proven both inside the new runtime-fact-mutations check and independently reproduced by hand-editing a scratch copy of flake.nix/scripts/vm-specs.json and re-running nix eval: removing allod-dev's runtime line fails with inventory machines missing runtime: allod-dev; setting it to a non-string (42) fails with inventory machines with non-string runtime: allod-dev; setting it to an unknown string ("openstack"/"bhyve") fails with inventory machines with unknown runtime (expected one of: libvirt, microvm): allod-dev; and mutating a copy of the committed JSON's runtime value produces a non-empty diff against the generated JSON, proving the vm-specs-json drift check is not vacuous. The same check also confirms the hypervisor entry nexus never appears in vmSpecsJson, that privacy-1 remains the public libvirt example, and that allod-dev stays on libvirt — the last assertion exists so the operator's own development machine cannot be moved onto a new runtime by a quiet data edit.

--- 3 comment(s) ---
[2026-07-29] allod-agent:

Independent review pass — claude-opus-5, max effort

Read-only review of d21fe20 (merge base efe4a7c, no rebase drift) against Interface Contract 1 and the "Repository and schema checks" acceptance tests in dev-plans/microvm-framework-adoption.md. Every claim below was re-run, not read. Sabotage fixtures were built by copying the branch worktree to a scratch tree and editing that copy; nothing in the repo was modified.

No [BLOCKER]. Three [GAP]s, none of which breaks a contract today; all three are cheap and two of them are about the validator rather than the code it validates.

---

1. [GAP] The non-string sabotage fixture does not validate the non-string assertion

runtime-fact-mutations only records tryEval ... .success, never why the fixture failed. For the non-string case that is not a discrimination nicety, it is a hole: builtins.elem 42 [ "libvirt" "microvm" ] is already false, so machinesNonStringRuntime lands in unknownRuntime and trips the unknown assertion regardless. Deleting the non-string assertion entirely leaves the check green and still printing OK: non-string runtime fails:

# scratch copy with `stringRuntime = assert ...; runtimeDeclared;` reduced to `stringRuntime = runtimeDeclared;`
runtime-fact-mutations-check> OK: valid machines evaluate (success=true)
runtime-fact-mutations-check> OK: missing runtime fails (success=false)
runtime-fact-mutations-check> OK: non-string runtime fails (success=false)
runtime-fact-mutations-check> OK: unknown runtime fails (success=false)
...
runtime-fact-mutations-check> runtime-fact-mutations passed: ...
S5 build exit=0
S5 flake check exit=0

For contrast, the same experiment on the unknown assertion does fail the check, so that one is genuinely load-bearing:

> ERROR: unknown runtime fails: expected success=false but got success=true
> runtime-fact-mutations failed with 1 error(s)

The plan's Validator validation rule is that a check which cannot be shown to fail on the bad configuration it claims to catch does not count. One of this check's three claims cannot.

A second, related property worth knowing before this pattern is copied into the archetypes/nexus mutation checks the plan requires: builtins.tryEval catches only AssertionError (and ThrownError, which derives from it). A plain EvalError escapes it. Deleting the missing-runtime assertion therefore does not produce a red check — it aborts evaluation of the whole flake from inside buildCommand:

… while evaluating attribute 'buildCommand' of derivation 'runtime-fact-mutations-check'
error: attribute 'runtime' missing
       at flake.nix:120:56:
          120|             lib.filterAttrs (_: m: !(builtins.isString m.runtime)) runtimeDeclared;

That is loud, so it is not itself a defect — but it means the boolean-success harness can only ever report on assert/throw failures, and a future fixture whose intended failure mode is a type or missing-attribute error will take the flake down instead of failing its own line.

Fix: stop asserting a boolean. Expose the three diagnostic sets independently of the chain and assert exactly which one is non-empty per fixture, e.g.

runtimeDiagnostics = ms:
  let vms = lib.filterAttrs (_: m: m.type != "hypervisor") ms; in {
    missing   = builtins.attrNames (lib.filterAttrs (_: m: !(m ? runtime)) vms);
    nonString = builtins.attrNames (lib.filterAttrs (_: m: (m ? runtime) && !(builtins.isString m.runtime)) vms);
    unknown   = builtins.attrNames (lib.filterAttrs (_: m: (m ? runtime) && builtins.isString m.runtime && !(builtins.elem m.runtime validRuntimes)) vms);
  };

Each fixture then has to light exactly one lamp, and each of the three assertions becomes independently falsifiable.

2. [GAP] A hypervisor can acquire a runtime fact; the PR body says it cannot

The PR body states hypervisors "are filtered out before the runtime checks run at all, so they can never acquire a fake guest runtime." The first half is true; the second is not enforced. Adding runtime = "microvm"; to the nexus entry evaluates cleanly, produces byte-identical vmSpecsJson, and passes all three checks:

### S6 hypervisor nexus acquires runtime = microvm
{"allod-dev":{...,"runtime":"libvirt",...},"privacy-1":{...,"runtime":"libvirt",...}}
### S6 flake check
running 3 flake checks...   (all pass)

The jq -e 'has("nexus")' assertion in runtime-fact-mutations cannot catch this: it keys on the type filter, which excludes nexus whether or not it carries a runtime. So the exemption is scoping, not enforcement — a hypervisor that acquires one is invisible to inventory and visible to anything reading machines.<name>.runtime directly.

Nothing downstream breaks today (archetypes vmFacts filters type != "hypervisor" too), so this is not a blocker. But the plan's contract 1 sentence is "Hypervisor entries do not acquire a fake guest runtime", and principle 11 wants that as an evaluation error rather than a convention. The structural version is four lines beside the existing chain:

hypervisorRuntime = lib.filterAttrs (_: m: m.type == "hypervisor" && m ? runtime) ms;
# assert hypervisorRuntime == {} with "inventory hypervisor machines must not declare runtime: <names>"

Either add that, or drop the "can never acquire" claim from the PR body — but the README's wording ("never appear in vmSpecsJson regardless of one") is already the accurate one, and the two should not disagree.

3. [GAP] The runtime assertions are not on the surface consumers actually read

mkVmSpecs is reachable only through vmSpecsJson. The exported machines and lib.machines are the raw, unvalidated attrset. With allod-dev's runtime line removed:

### S1: nix eval .#lib.vmSpecsJson
error: inventory machines missing runtime: allod-dev          <- correct
### S1: nix eval .#machines --apply builtins.attrNames
[ "allod-dev" "nexus" "privacy-1" ]                            <- succeeds
### S1: nix eval .#lib.machines --apply 'm: m.allod-dev.type'
"dev"                                                          <- succeeds
### S1: nix eval .#lib.supportedPlatforms
[ "x86_64-linux" ]                                             <- succeeds

archetypes consumes inventory.machines (flake.nix:39, :43), not lib.vmSpecsJson; no sibling flake evaluates vmSpecsJson at all. nix flake check in a consumer skips an input's own checks, so a downstream repo that bumps its inventory pin gets no inventory-side failure from bad runtime data — the memory rule in nix.md ("hoist its assertion onto the consumed surface (lib.*) or consumers never trip it") is aimed exactly at this shape. Contract 1's own wording is inventory.machines.<name>, which is the surface that currently does not fail.

The plan does put an independent no-defaults duty on the archetypes milestone, so this is defensible as sequencing rather than a defect. But routing the export through the validated set costs one binding and closes it here instead of relying on the next repo:

checkedMachines = builtins.seq (mkVmSpecs machines) machines;
# export checkedMachines as both `machines` and `lib.machines`

Note the contrast with the pre-existing platform chain, which is on a consumed surface (lib.supportedPlatforms, read by deploy, secrets, and archetypes). The runtime chain is the odd one out.

---

What I verified and reproduced

Plan acceptance commands, in the branch worktree — all three match the PR's claims exactly.

$ nix flake check --print-build-logs
checking derivation checks.x86_64-linux.vm-specs-json...
checking derivation checks.x86_64-linux.repository-registry...
checking derivation checks.x86_64-linux.runtime-fact-mutations...
running 3 flake checks...
EXIT=0

$ nix eval .#lib.vmSpecsJson --raw | jq -e 'to_entries | all(.value.runtime == "libvirt" or .value.runtime == "microvm")'
true      (exit 0)

$ diff -u <(nix eval .#lib.vmSpecsJson --raw | jq -S .) <(jq -S . scripts/vm-specs.json)
          (empty, exit 0)

Forced a rebuild of runtime-fact-mutations rather than trusting the cache; all eight of its lines print OK.

Each failure mode, sabotaged independently, fails with the message the PR claims.

runtime removed        -> error: inventory machines missing runtime: allod-dev
runtime = 42           -> error: inventory machines with non-string runtime: allod-dev
runtime = "openstack"  -> error: inventory machines with unknown runtime (expected one of: libvirt, microvm): allod-dev

Assertion ordering is correct: the innermost assertion fires first, so each fixture surfaces its own message rather than a downstream one.

Drift detection is real, for the new key and for a pre-existing one. Mutating allod-dev.runtime to "bhyve" in the committed JSON, and separately deleting privacy-1.ip, each fail vm-specs-json with the regenerate hint. The drift check is not vacuous in either direction.

The refactor is byte-identical for every pre-existing field. mkVmSpecs/mkVmSpecsJson are a pure re-parameterization:

$ diff <(jq -S . master.json) <(jq -S 'map_values(del(.runtime))' pr.json)
          (empty, exit 0)

Same two machine keys (allod-dev, privacy-1), no dropped field, no reordering, runtime is the only addition. builtins.toJSON sorts keys, so the inherit (m) ... runtime position cannot perturb output. Nothing for archetypes.vmFacts or the host scripts to trip over.

No dangling references anywhere. vmSpecs was a private let binding, never a flake output, and nothing in archetypes, nexus, deploy, profiles, secrets, or tools references it — the only hit in the whole workspace is a brainstorm/ doc using the name incidentally. The public surface (machines, lib.machines, lib.supportedPlatforms, lib.vmSpecsJson) is unchanged. The extra JSON key is inert for both classes of consumer: archetypes' vm-facts-coherence projects vm-specs.json down to {ip, forge_key} before diffing, and the nexus host scripts read named fields with jq. No sibling repo breaks on this landing, and none breaks when its inventory pin is bumped.

Metadata. Refs allod/strategy#20 present, no closing keyword anywhere in the body or the commit message — correct for a non-final PR in a multi-repo arc. The R2 Medium claim and its justification match the plan's risk-table row for this milestone verbatim in substance ("machine-data contract consumed by host scripts and vmFacts; rollback is a revert and both values are checked"; scrutiny on schema propagation and missing/unknown-value failures). Commit message carries no attribution trailer and no hook-tripping keyword.

Recommendation

Merge after fixing 1 and 2. Both are small and both are about making the check say what it claims: 1 is the plan's own Validator validation standard applied to a line that currently passes for the wrong reason, and 2 removes a claim in the PR body that the code does not back. 3 is a judgement call — the cheap hoist is worth taking here, but deferring it to the archetypes milestone is consistent with the plan's Implementation Sequence, so it should not hold the merge on its own.

Every non-hypervisor `machines.<name>` entry now declares `runtime = "libvirt"` or `runtime = "microvm"`, validated by an assertion chain that mirrors the repo's existing `platform` checks: a missing, non-string, or unknown value fails evaluation with a named error listing the offending machine names, before any consumer can see partial or defaulted data. Hypervisor entries (`nexus`) are exempt by scope, not by a default — they are filtered out before the runtime checks run at all, so they can never acquire a fake guest runtime. Both public example machines declare `libvirt`. `allod-dev` in particular is held there by its own check: it is the machine the operator develops from, and a real machine rather than a synthetic example — `allod/secrets` carries key material keyed to that name — so it is the last machine to move onto a new runtime, not the first. The machine that first selects `microvm` is added in the same change that provisions it, because a machine entry is not self-contained: it needs a matching identity, a profile, and per-machine encrypted credentials whose creation needs a host key generated on the host. The runtime enum's `microvm` branch is covered by the mutation fixtures below rather than by an example machine. `lib.vmSpecsJson` now exports `runtime` alongside the existing host-facing fields, and the committed `scripts/vm-specs.json` is regenerated so the two agree exactly. The internal `mkVmSpecs`/`mkVmSpecsJson` helpers take an explicit machine set rather than closing over `machines`, so the new `runtime-fact-mutations` check can run the identical validation chain against sabotaged copies and prove each failure mode actually fails rather than merely asserting on the real data. This PR is the inventory half of Interface Contract 1 only. The `archetypes.vmFacts.<name>.runtime` half of the contract, and the guest-module selection that consumes this fact, land in later milestones per the plan's Implementation Sequence; this PR does not touch `allod/vm`, `allod/nexus`, `allod/archetypes`, or `allod/profiles`. Refs allod/strategy#20 ## Risk R2 Medium, matching the plan's risk table for this milestone: this changes a machine-data contract consumed by host scripts and, in a later milestone, `vmFacts`, but rollback is a plain revert and both the Nix-side value and the committed JSON are checked against each other on every evaluation. The residual risk worth a human's attention is schema propagation into the generated JSON and the missing/unknown-value failure behavior, both exercised below. ## Validation All commands were run against this branch's worktree in `allod/inventory`. `nix flake check --print-build-logs` passes all three checks: `vm-specs-json`, `repository-registry`, and the new `runtime-fact-mutations`. ``` nix eval .#lib.vmSpecsJson --raw | jq -e ' to_entries | all(.value.runtime == "libvirt" or .value.runtime == "microvm") ' ``` returns `true`. ``` diff -u <(nix eval .#lib.vmSpecsJson --raw | jq -S .) \ <(jq -S . scripts/vm-specs.json) ``` produces an empty diff. Mutation/sabotage coverage, proven both inside the new `runtime-fact-mutations` check and independently reproduced by hand-editing a scratch copy of `flake.nix`/`scripts/vm-specs.json` and re-running `nix eval`: removing `allod-dev`'s `runtime` line fails with `inventory machines missing runtime: allod-dev`; setting it to a non-string (`42`) fails with `inventory machines with non-string runtime: allod-dev`; setting it to an unknown string (`"openstack"`/`"bhyve"`) fails with `inventory machines with unknown runtime (expected one of: libvirt, microvm): allod-dev`; and mutating a copy of the committed JSON's `runtime` value produces a non-empty diff against the generated JSON, proving the `vm-specs-json` drift check is not vacuous. The same check also confirms the hypervisor entry `nexus` never appears in `vmSpecsJson`, that `privacy-1` remains the public `libvirt` example, and that `allod-dev` stays on `libvirt` — the last assertion exists so the operator's own development machine cannot be moved onto a new runtime by a quiet data edit. --- 3 comment(s) --- [2026-07-29] allod-agent: ## Independent review pass — `claude-opus-5`, max effort Read-only review of `d21fe20` (merge base `efe4a7c`, no rebase drift) against Interface Contract 1 and the "Repository and schema checks" acceptance tests in `dev-plans/microvm-framework-adoption.md`. Every claim below was re-run, not read. Sabotage fixtures were built by copying the branch worktree to a scratch tree and editing that copy; nothing in the repo was modified. **No `[BLOCKER]`.** Three `[GAP]`s, none of which breaks a contract today; all three are cheap and two of them are about the validator rather than the code it validates. --- ### 1. [GAP] The non-string sabotage fixture does not validate the non-string assertion `runtime-fact-mutations` only records `tryEval ... .success`, never *why* the fixture failed. For the non-string case that is not a discrimination nicety, it is a hole: `builtins.elem 42 [ "libvirt" "microvm" ]` is already `false`, so `machinesNonStringRuntime` lands in `unknownRuntime` and trips the *unknown* assertion regardless. Deleting the non-string assertion entirely leaves the check green and still printing `OK: non-string runtime fails`: ``` # scratch copy with `stringRuntime = assert ...; runtimeDeclared;` reduced to `stringRuntime = runtimeDeclared;` runtime-fact-mutations-check> OK: valid machines evaluate (success=true) runtime-fact-mutations-check> OK: missing runtime fails (success=false) runtime-fact-mutations-check> OK: non-string runtime fails (success=false) runtime-fact-mutations-check> OK: unknown runtime fails (success=false) ... runtime-fact-mutations-check> runtime-fact-mutations passed: ... S5 build exit=0 S5 flake check exit=0 ``` For contrast, the same experiment on the *unknown* assertion does fail the check, so that one is genuinely load-bearing: ``` > ERROR: unknown runtime fails: expected success=false but got success=true > runtime-fact-mutations failed with 1 error(s) ``` The plan's Validator validation rule is that a check which cannot be shown to fail on the bad configuration it claims to catch does not count. One of this check's three claims cannot. A second, related property worth knowing before this pattern is copied into the archetypes/nexus mutation checks the plan requires: `builtins.tryEval` catches only `AssertionError` (and `ThrownError`, which derives from it). A plain `EvalError` escapes it. Deleting the *missing*-runtime assertion therefore does not produce a red check — it aborts evaluation of the whole flake from inside `buildCommand`: ``` … while evaluating attribute 'buildCommand' of derivation 'runtime-fact-mutations-check' error: attribute 'runtime' missing at flake.nix:120:56: 120| lib.filterAttrs (_: m: !(builtins.isString m.runtime)) runtimeDeclared; ``` That is loud, so it is not itself a defect — but it means the boolean-success harness can only ever report on `assert`/`throw` failures, and a future fixture whose intended failure mode is a type or missing-attribute error will take the flake down instead of failing its own line. Fix: stop asserting a boolean. Expose the three diagnostic sets independently of the chain and assert exactly which one is non-empty per fixture, e.g. ```nix runtimeDiagnostics = ms: let vms = lib.filterAttrs (_: m: m.type != "hypervisor") ms; in { missing = builtins.attrNames (lib.filterAttrs (_: m: !(m ? runtime)) vms); nonString = builtins.attrNames (lib.filterAttrs (_: m: (m ? runtime) && !(builtins.isString m.runtime)) vms); unknown = builtins.attrNames (lib.filterAttrs (_: m: (m ? runtime) && builtins.isString m.runtime && !(builtins.elem m.runtime validRuntimes)) vms); }; ``` Each fixture then has to light exactly one lamp, and each of the three assertions becomes independently falsifiable. ### 2. [GAP] A hypervisor can acquire a runtime fact; the PR body says it cannot The PR body states hypervisors "are filtered out before the runtime checks run at all, so they can never acquire a fake guest runtime." The first half is true; the second is not enforced. Adding `runtime = "microvm";` to the `nexus` entry evaluates cleanly, produces byte-identical `vmSpecsJson`, and passes all three checks: ``` ### S6 hypervisor nexus acquires runtime = microvm {"allod-dev":{...,"runtime":"libvirt",...},"privacy-1":{...,"runtime":"libvirt",...}} ### S6 flake check running 3 flake checks... (all pass) ``` The `jq -e 'has("nexus")'` assertion in `runtime-fact-mutations` cannot catch this: it keys on the type filter, which excludes `nexus` whether or not it carries a runtime. So the exemption is scoping, not enforcement — a hypervisor that acquires one is invisible to inventory and visible to anything reading `machines.<name>.runtime` directly. Nothing downstream breaks today (`archetypes` `vmFacts` filters `type != "hypervisor"` too), so this is not a blocker. But the plan's contract 1 sentence is "Hypervisor entries do not acquire a fake guest runtime", and principle 11 wants that as an evaluation error rather than a convention. The structural version is four lines beside the existing chain: ```nix hypervisorRuntime = lib.filterAttrs (_: m: m.type == "hypervisor" && m ? runtime) ms; # assert hypervisorRuntime == {} with "inventory hypervisor machines must not declare runtime: <names>" ``` Either add that, or drop the "can never acquire" claim from the PR body — but the README's wording ("never appear in `vmSpecsJson` regardless of one") is already the accurate one, and the two should not disagree. ### 3. [GAP] The runtime assertions are not on the surface consumers actually read `mkVmSpecs` is reachable only through `vmSpecsJson`. The exported `machines` and `lib.machines` are the raw, unvalidated attrset. With `allod-dev`'s `runtime` line removed: ``` ### S1: nix eval .#lib.vmSpecsJson error: inventory machines missing runtime: allod-dev <- correct ### S1: nix eval .#machines --apply builtins.attrNames [ "allod-dev" "nexus" "privacy-1" ] <- succeeds ### S1: nix eval .#lib.machines --apply 'm: m.allod-dev.type' "dev" <- succeeds ### S1: nix eval .#lib.supportedPlatforms [ "x86_64-linux" ] <- succeeds ``` `archetypes` consumes `inventory.machines` (`flake.nix:39`, `:43`), not `lib.vmSpecsJson`; no sibling flake evaluates `vmSpecsJson` at all. `nix flake check` in a consumer skips an input's own checks, so a downstream repo that bumps its inventory pin gets no inventory-side failure from bad runtime data — the memory rule in `nix.md` ("hoist its assertion onto the consumed surface (`lib.*`) or consumers never trip it") is aimed exactly at this shape. Contract 1's own wording is `inventory.machines.<name>`, which is the surface that currently does not fail. The plan does put an independent no-defaults duty on the archetypes milestone, so this is defensible as sequencing rather than a defect. But routing the export through the validated set costs one binding and closes it here instead of relying on the next repo: ```nix checkedMachines = builtins.seq (mkVmSpecs machines) machines; # export checkedMachines as both `machines` and `lib.machines` ``` Note the contrast with the pre-existing `platform` chain, which *is* on a consumed surface (`lib.supportedPlatforms`, read by `deploy`, `secrets`, and `archetypes`). The runtime chain is the odd one out. --- ## What I verified and reproduced **Plan acceptance commands, in the branch worktree — all three match the PR's claims exactly.** ``` $ nix flake check --print-build-logs checking derivation checks.x86_64-linux.vm-specs-json... checking derivation checks.x86_64-linux.repository-registry... checking derivation checks.x86_64-linux.runtime-fact-mutations... running 3 flake checks... EXIT=0 $ nix eval .#lib.vmSpecsJson --raw | jq -e 'to_entries | all(.value.runtime == "libvirt" or .value.runtime == "microvm")' true (exit 0) $ diff -u <(nix eval .#lib.vmSpecsJson --raw | jq -S .) <(jq -S . scripts/vm-specs.json) (empty, exit 0) ``` Forced a rebuild of `runtime-fact-mutations` rather than trusting the cache; all eight of its lines print `OK`. **Each failure mode, sabotaged independently, fails with the message the PR claims.** ``` runtime removed -> error: inventory machines missing runtime: allod-dev runtime = 42 -> error: inventory machines with non-string runtime: allod-dev runtime = "openstack" -> error: inventory machines with unknown runtime (expected one of: libvirt, microvm): allod-dev ``` Assertion ordering is correct: the innermost assertion fires first, so each fixture surfaces its own message rather than a downstream one. **Drift detection is real, for the new key and for a pre-existing one.** Mutating `allod-dev.runtime` to `"bhyve"` in the committed JSON, and separately deleting `privacy-1.ip`, each fail `vm-specs-json` with the regenerate hint. The drift check is not vacuous in either direction. **The refactor is byte-identical for every pre-existing field.** `mkVmSpecs`/`mkVmSpecsJson` are a pure re-parameterization: ``` $ diff <(jq -S . master.json) <(jq -S 'map_values(del(.runtime))' pr.json) (empty, exit 0) ``` Same two machine keys (`allod-dev`, `privacy-1`), no dropped field, no reordering, `runtime` is the only addition. `builtins.toJSON` sorts keys, so the `inherit (m) ... runtime` position cannot perturb output. Nothing for `archetypes.vmFacts` or the host scripts to trip over. **No dangling references anywhere.** `vmSpecs` was a private `let` binding, never a flake output, and nothing in `archetypes`, `nexus`, `deploy`, `profiles`, `secrets`, or `tools` references it — the only hit in the whole workspace is a `brainstorm/` doc using the name incidentally. The public surface (`machines`, `lib.machines`, `lib.supportedPlatforms`, `lib.vmSpecsJson`) is unchanged. The extra JSON key is inert for both classes of consumer: `archetypes`' `vm-facts-coherence` projects `vm-specs.json` down to `{ip, forge_key}` before diffing, and the `nexus` host scripts read named fields with `jq`. No sibling repo breaks on this landing, and none breaks when its inventory pin is bumped. **Metadata.** `Refs allod/strategy#20` present, no closing keyword anywhere in the body or the commit message — correct for a non-final PR in a multi-repo arc. The R2 Medium claim and its justification match the plan's risk-table row for this milestone verbatim in substance ("machine-data contract consumed by host scripts and `vmFacts`; rollback is a revert and both values are checked"; scrutiny on schema propagation and missing/unknown-value failures). Commit message carries no attribution trailer and no hook-tripping keyword. ## Recommendation Merge after fixing 1 and 2. Both are small and both are about making the check say what it claims: 1 is the plan's own Validator validation standard applied to a line that currently passes for the wrong reason, and 2 removes a claim in the PR body that the code does not back. 3 is a judgement call — the cheap hoist is worth taking here, but deferring it to the archetypes milestone is consistent with the plan's Implementation Sequence, so it should not hold the merge on its own.

allod-agent added 1 commit 2026-07-29 20:28:38 +01:00

Add required runtime fact for non-hypervisor inventory machines ... d21fe20fc2

Every non-hypervisor machine now declares runtime = "libvirt" or "microvm", validated by an assertion chain analogous to the existing platform checks: missing, non-string, and unknown values fail evaluation with a named error naming the offending machines. Hypervisor entries are exempt and stay excluded from vmSpecsJson, so they never need or acquire a fake guest runtime.

lib.vmSpecsJson now exports runtime alongside the other host-facing fields, mkVmSpecs/mkVmSpecsJson are parameterized on an explicit machine set so the same validation chain can run against sabotaged fixtures, and the committed scripts/vm-specs.json is regenerated to match exactly.

A new runtime-fact-mutations check proves a missing runtime, a non-string runtime, and an unknown runtime each fail evaluation, that the hypervisor stays excluded, that allod-dev (microvm) and privacy-1 (libvirt) both remain as public examples, and that the vm-specs-json drift check is not vacuous by mutating a copy of the committed JSON and confirming the diff idiom disagrees.

allod-agent commented 2026-07-29 20:39:06 +01:00

Author

Member

Copy link

Independent review pass — claude-opus-5, max effort

Read-only review of d21fe20 (merge base efe4a7c, no rebase drift) against Interface Contract 1 and the "Repository and schema checks" acceptance tests in dev-plans/microvm-framework-adoption.md. Every claim below was re-run, not read. Sabotage fixtures were built by copying the branch worktree to a scratch tree and editing that copy; nothing in the repo was modified.

No [BLOCKER]. Three [GAP]s, none of which breaks a contract today; all three are cheap and two of them are about the validator rather than the code it validates.

---

1. [GAP] The non-string sabotage fixture does not validate the non-string assertion

runtime-fact-mutations only records tryEval ... .success, never why the fixture failed. For the non-string case that is not a discrimination nicety, it is a hole: builtins.elem 42 [ "libvirt" "microvm" ] is already false, so machinesNonStringRuntime lands in unknownRuntime and trips the unknown assertion regardless. Deleting the non-string assertion entirely leaves the check green and still printing OK: non-string runtime fails:

# scratch copy with `stringRuntime = assert ...; runtimeDeclared;` reduced to `stringRuntime = runtimeDeclared;`
runtime-fact-mutations-check> OK: valid machines evaluate (success=true)
runtime-fact-mutations-check> OK: missing runtime fails (success=false)
runtime-fact-mutations-check> OK: non-string runtime fails (success=false)
runtime-fact-mutations-check> OK: unknown runtime fails (success=false)
...
runtime-fact-mutations-check> runtime-fact-mutations passed: ...
S5 build exit=0
S5 flake check exit=0

For contrast, the same experiment on the unknown assertion does fail the check, so that one is genuinely load-bearing:

> ERROR: unknown runtime fails: expected success=false but got success=true
> runtime-fact-mutations failed with 1 error(s)

The plan's Validator validation rule is that a check which cannot be shown to fail on the bad configuration it claims to catch does not count. One of this check's three claims cannot.

A second, related property worth knowing before this pattern is copied into the archetypes/nexus mutation checks the plan requires: builtins.tryEval catches only AssertionError (and ThrownError, which derives from it). A plain EvalError escapes it. Deleting the missing-runtime assertion therefore does not produce a red check — it aborts evaluation of the whole flake from inside buildCommand:

… while evaluating attribute 'buildCommand' of derivation 'runtime-fact-mutations-check'
error: attribute 'runtime' missing
       at flake.nix:120:56:
          120|             lib.filterAttrs (_: m: !(builtins.isString m.runtime)) runtimeDeclared;

That is loud, so it is not itself a defect — but it means the boolean-success harness can only ever report on assert/throw failures, and a future fixture whose intended failure mode is a type or missing-attribute error will take the flake down instead of failing its own line.

Fix: stop asserting a boolean. Expose the three diagnostic sets independently of the chain and assert exactly which one is non-empty per fixture, e.g.

runtimeDiagnostics = ms:
  let vms = lib.filterAttrs (_: m: m.type != "hypervisor") ms; in {
    missing   = builtins.attrNames (lib.filterAttrs (_: m: !(m ? runtime)) vms);
    nonString = builtins.attrNames (lib.filterAttrs (_: m: (m ? runtime) && !(builtins.isString m.runtime)) vms);
    unknown   = builtins.attrNames (lib.filterAttrs (_: m: (m ? runtime) && builtins.isString m.runtime && !(builtins.elem m.runtime validRuntimes)) vms);
  };

Each fixture then has to light exactly one lamp, and each of the three assertions becomes independently falsifiable.

2. [GAP] A hypervisor can acquire a runtime fact; the PR body says it cannot

The PR body states hypervisors "are filtered out before the runtime checks run at all, so they can never acquire a fake guest runtime." The first half is true; the second is not enforced. Adding runtime = "microvm"; to the nexus entry evaluates cleanly, produces byte-identical vmSpecsJson, and passes all three checks:

### S6 hypervisor nexus acquires runtime = microvm
{"allod-dev":{...,"runtime":"microvm",...},"privacy-1":{...,"runtime":"libvirt",...}}
### S6 flake check
running 3 flake checks...   (all pass)

The jq -e 'has("nexus")' assertion in runtime-fact-mutations cannot catch this: it keys on the type filter, which excludes nexus whether or not it carries a runtime. So the exemption is scoping, not enforcement — a hypervisor that acquires one is invisible to inventory and visible to anything reading machines.<name>.runtime directly.

Nothing downstream breaks today (archetypes vmFacts filters type != "hypervisor" too), so this is not a blocker. But the plan's contract 1 sentence is "Hypervisor entries do not acquire a fake guest runtime", and principle 11 wants that as an evaluation error rather than a convention. The structural version is four lines beside the existing chain:

hypervisorRuntime = lib.filterAttrs (_: m: m.type == "hypervisor" && m ? runtime) ms;
# assert hypervisorRuntime == {} with "inventory hypervisor machines must not declare runtime: <names>"

Either add that, or drop the "can never acquire" claim from the PR body — but the README's wording ("never appear in vmSpecsJson regardless of one") is already the accurate one, and the two should not disagree.

3. [GAP] The runtime assertions are not on the surface consumers actually read

mkVmSpecs is reachable only through vmSpecsJson. The exported machines and lib.machines are the raw, unvalidated attrset. With allod-dev's runtime line removed:

### S1: nix eval .#lib.vmSpecsJson
error: inventory machines missing runtime: allod-dev          <- correct
### S1: nix eval .#machines --apply builtins.attrNames
[ "allod-dev" "nexus" "privacy-1" ]                            <- succeeds
### S1: nix eval .#lib.machines --apply 'm: m.allod-dev.type'
"dev"                                                          <- succeeds
### S1: nix eval .#lib.supportedPlatforms
[ "x86_64-linux" ]                                             <- succeeds

archetypes consumes inventory.machines (flake.nix:39, :43), not lib.vmSpecsJson; no sibling flake evaluates vmSpecsJson at all. nix flake check in a consumer skips an input's own checks, so a downstream repo that bumps its inventory pin gets no inventory-side failure from bad runtime data — the memory rule in nix.md ("hoist its assertion onto the consumed surface (lib.*) or consumers never trip it") is aimed exactly at this shape. Contract 1's own wording is inventory.machines.<name>, which is the surface that currently does not fail.

The plan does put an independent no-defaults duty on the archetypes milestone, so this is defensible as sequencing rather than a defect. But routing the export through the validated set costs one binding and closes it here instead of relying on the next repo:

checkedMachines = builtins.seq (mkVmSpecs machines) machines;
# export checkedMachines as both `machines` and `lib.machines`

Note the contrast with the pre-existing platform chain, which is on a consumed surface (lib.supportedPlatforms, read by deploy, secrets, and archetypes). The runtime chain is the odd one out.

---

What I verified and reproduced

Plan acceptance commands, in the branch worktree — all three match the PR's claims exactly.

$ nix flake check --print-build-logs
checking derivation checks.x86_64-linux.vm-specs-json...
checking derivation checks.x86_64-linux.repository-registry...
checking derivation checks.x86_64-linux.runtime-fact-mutations...
running 3 flake checks...
EXIT=0

$ nix eval .#lib.vmSpecsJson --raw | jq -e 'to_entries | all(.value.runtime == "libvirt" or .value.runtime == "microvm")'
true      (exit 0)

$ diff -u <(nix eval .#lib.vmSpecsJson --raw | jq -S .) <(jq -S . scripts/vm-specs.json)
          (empty, exit 0)

Forced a rebuild of runtime-fact-mutations rather than trusting the cache; all eight of its lines print OK.

Each failure mode, sabotaged independently, fails with the message the PR claims.

runtime removed        -> error: inventory machines missing runtime: allod-dev
runtime = 42           -> error: inventory machines with non-string runtime: allod-dev
runtime = "openstack"  -> error: inventory machines with unknown runtime (expected one of: libvirt, microvm): allod-dev

Assertion ordering is correct: the innermost assertion fires first, so each fixture surfaces its own message rather than a downstream one.

Drift detection is real, for the new key and for a pre-existing one. Mutating allod-dev.runtime to "bhyve" in the committed JSON, and separately deleting privacy-1.ip, each fail vm-specs-json with the regenerate hint. The drift check is not vacuous in either direction.

The refactor is byte-identical for every pre-existing field. mkVmSpecs/mkVmSpecsJson are a pure re-parameterization:

$ diff <(jq -S . master.json) <(jq -S 'map_values(del(.runtime))' pr.json)
          (empty, exit 0)

Same two machine keys (allod-dev, privacy-1), no dropped field, no reordering, runtime is the only addition. builtins.toJSON sorts keys, so the inherit (m) ... runtime position cannot perturb output. Nothing for archetypes.vmFacts or the host scripts to trip over.

No dangling references anywhere. vmSpecs was a private let binding, never a flake output, and nothing in archetypes, nexus, deploy, profiles, secrets, or tools references it — the only hit in the whole workspace is a brainstorm/ doc using the name incidentally. The public surface (machines, lib.machines, lib.supportedPlatforms, lib.vmSpecsJson) is unchanged. The extra JSON key is inert for both classes of consumer: archetypes' vm-facts-coherence projects vm-specs.json down to {ip, forge_key} before diffing, and the nexus host scripts read named fields with jq. No sibling repo breaks on this landing, and none breaks when its inventory pin is bumped.

Metadata. Refs allod/strategy#20 present, no closing keyword anywhere in the body or the commit message — correct for a non-final PR in a multi-repo arc. The R2 Medium claim and its justification match the plan's risk-table row for this milestone verbatim in substance ("machine-data contract consumed by host scripts and vmFacts; rollback is a revert and both values are checked"; scrutiny on schema propagation and missing/unknown-value failures). Commit message carries no attribution trailer and no hook-tripping keyword.

Recommendation

Merge after fixing 1 and 2. Both are small and both are about making the check say what it claims: 1 is the plan's own Validator validation standard applied to a line that currently passes for the wrong reason, and 2 removes a claim in the PR body that the code does not back. 3 is a judgement call — the cheap hoist is worth taking here, but deferring it to the archetypes milestone is consistent with the plan's Implementation Sequence, so it should not hold the merge on its own.

The core of the change is right: the enum is enforced, the failure messages name the offending machines, the projection into vmSpecsJson and the committed JSON is exactly additive, and parameterizing the helpers on an explicit machine set so the real validation chain runs against sabotaged fixtures is the correct shape for this — it is what let the two validator gaps above be found by deleting assertions rather than by reading them.

## Independent review pass — `claude-opus-5`, max effort Read-only review of `d21fe20` (merge base `efe4a7c`, no rebase drift) against Interface Contract 1 and the "Repository and schema checks" acceptance tests in `dev-plans/microvm-framework-adoption.md`. Every claim below was re-run, not read. Sabotage fixtures were built by copying the branch worktree to a scratch tree and editing that copy; nothing in the repo was modified. **No `[BLOCKER]`.** Three `[GAP]`s, none of which breaks a contract today; all three are cheap and two of them are about the validator rather than the code it validates. --- ### 1. [GAP] The non-string sabotage fixture does not validate the non-string assertion `runtime-fact-mutations` only records `tryEval ... .success`, never *why* the fixture failed. For the non-string case that is not a discrimination nicety, it is a hole: `builtins.elem 42 [ "libvirt" "microvm" ]` is already `false`, so `machinesNonStringRuntime` lands in `unknownRuntime` and trips the *unknown* assertion regardless. Deleting the non-string assertion entirely leaves the check green and still printing `OK: non-string runtime fails`: ``` # scratch copy with `stringRuntime = assert ...; runtimeDeclared;` reduced to `stringRuntime = runtimeDeclared;` runtime-fact-mutations-check> OK: valid machines evaluate (success=true) runtime-fact-mutations-check> OK: missing runtime fails (success=false) runtime-fact-mutations-check> OK: non-string runtime fails (success=false) runtime-fact-mutations-check> OK: unknown runtime fails (success=false) ... runtime-fact-mutations-check> runtime-fact-mutations passed: ... S5 build exit=0 S5 flake check exit=0 ``` For contrast, the same experiment on the *unknown* assertion does fail the check, so that one is genuinely load-bearing: ``` > ERROR: unknown runtime fails: expected success=false but got success=true > runtime-fact-mutations failed with 1 error(s) ``` The plan's Validator validation rule is that a check which cannot be shown to fail on the bad configuration it claims to catch does not count. One of this check's three claims cannot. A second, related property worth knowing before this pattern is copied into the archetypes/nexus mutation checks the plan requires: `builtins.tryEval` catches only `AssertionError` (and `ThrownError`, which derives from it). A plain `EvalError` escapes it. Deleting the *missing*-runtime assertion therefore does not produce a red check — it aborts evaluation of the whole flake from inside `buildCommand`: ``` … while evaluating attribute 'buildCommand' of derivation 'runtime-fact-mutations-check' error: attribute 'runtime' missing at flake.nix:120:56: 120| lib.filterAttrs (_: m: !(builtins.isString m.runtime)) runtimeDeclared; ``` That is loud, so it is not itself a defect — but it means the boolean-success harness can only ever report on `assert`/`throw` failures, and a future fixture whose intended failure mode is a type or missing-attribute error will take the flake down instead of failing its own line. Fix: stop asserting a boolean. Expose the three diagnostic sets independently of the chain and assert exactly which one is non-empty per fixture, e.g. ```nix runtimeDiagnostics = ms: let vms = lib.filterAttrs (_: m: m.type != "hypervisor") ms; in { missing = builtins.attrNames (lib.filterAttrs (_: m: !(m ? runtime)) vms); nonString = builtins.attrNames (lib.filterAttrs (_: m: (m ? runtime) && !(builtins.isString m.runtime)) vms); unknown = builtins.attrNames (lib.filterAttrs (_: m: (m ? runtime) && builtins.isString m.runtime && !(builtins.elem m.runtime validRuntimes)) vms); }; ``` Each fixture then has to light exactly one lamp, and each of the three assertions becomes independently falsifiable. ### 2. [GAP] A hypervisor can acquire a runtime fact; the PR body says it cannot The PR body states hypervisors "are filtered out before the runtime checks run at all, so they can never acquire a fake guest runtime." The first half is true; the second is not enforced. Adding `runtime = "microvm";` to the `nexus` entry evaluates cleanly, produces byte-identical `vmSpecsJson`, and passes all three checks: ``` ### S6 hypervisor nexus acquires runtime = microvm {"allod-dev":{...,"runtime":"microvm",...},"privacy-1":{...,"runtime":"libvirt",...}} ### S6 flake check running 3 flake checks... (all pass) ``` The `jq -e 'has("nexus")'` assertion in `runtime-fact-mutations` cannot catch this: it keys on the type filter, which excludes `nexus` whether or not it carries a runtime. So the exemption is scoping, not enforcement — a hypervisor that acquires one is invisible to inventory and visible to anything reading `machines.<name>.runtime` directly. Nothing downstream breaks today (`archetypes` `vmFacts` filters `type != "hypervisor"` too), so this is not a blocker. But the plan's contract 1 sentence is "Hypervisor entries do not acquire a fake guest runtime", and principle 11 wants that as an evaluation error rather than a convention. The structural version is four lines beside the existing chain: ```nix hypervisorRuntime = lib.filterAttrs (_: m: m.type == "hypervisor" && m ? runtime) ms; # assert hypervisorRuntime == {} with "inventory hypervisor machines must not declare runtime: <names>" ``` Either add that, or drop the "can never acquire" claim from the PR body — but the README's wording ("never appear in `vmSpecsJson` regardless of one") is already the accurate one, and the two should not disagree. ### 3. [GAP] The runtime assertions are not on the surface consumers actually read `mkVmSpecs` is reachable only through `vmSpecsJson`. The exported `machines` and `lib.machines` are the raw, unvalidated attrset. With `allod-dev`'s `runtime` line removed: ``` ### S1: nix eval .#lib.vmSpecsJson error: inventory machines missing runtime: allod-dev <- correct ### S1: nix eval .#machines --apply builtins.attrNames [ "allod-dev" "nexus" "privacy-1" ] <- succeeds ### S1: nix eval .#lib.machines --apply 'm: m.allod-dev.type' "dev" <- succeeds ### S1: nix eval .#lib.supportedPlatforms [ "x86_64-linux" ] <- succeeds ``` `archetypes` consumes `inventory.machines` (`flake.nix:39`, `:43`), not `lib.vmSpecsJson`; no sibling flake evaluates `vmSpecsJson` at all. `nix flake check` in a consumer skips an input's own checks, so a downstream repo that bumps its inventory pin gets no inventory-side failure from bad runtime data — the memory rule in `nix.md` ("hoist its assertion onto the consumed surface (`lib.*`) or consumers never trip it") is aimed exactly at this shape. Contract 1's own wording is `inventory.machines.<name>`, which is the surface that currently does not fail. The plan does put an independent no-defaults duty on the archetypes milestone, so this is defensible as sequencing rather than a defect. But routing the export through the validated set costs one binding and closes it here instead of relying on the next repo: ```nix checkedMachines = builtins.seq (mkVmSpecs machines) machines; # export checkedMachines as both `machines` and `lib.machines` ``` Note the contrast with the pre-existing `platform` chain, which *is* on a consumed surface (`lib.supportedPlatforms`, read by `deploy`, `secrets`, and `archetypes`). The runtime chain is the odd one out. --- ## What I verified and reproduced **Plan acceptance commands, in the branch worktree — all three match the PR's claims exactly.** ``` $ nix flake check --print-build-logs checking derivation checks.x86_64-linux.vm-specs-json... checking derivation checks.x86_64-linux.repository-registry... checking derivation checks.x86_64-linux.runtime-fact-mutations... running 3 flake checks... EXIT=0 $ nix eval .#lib.vmSpecsJson --raw | jq -e 'to_entries | all(.value.runtime == "libvirt" or .value.runtime == "microvm")' true (exit 0) $ diff -u <(nix eval .#lib.vmSpecsJson --raw | jq -S .) <(jq -S . scripts/vm-specs.json) (empty, exit 0) ``` Forced a rebuild of `runtime-fact-mutations` rather than trusting the cache; all eight of its lines print `OK`. **Each failure mode, sabotaged independently, fails with the message the PR claims.** ``` runtime removed -> error: inventory machines missing runtime: allod-dev runtime = 42 -> error: inventory machines with non-string runtime: allod-dev runtime = "openstack" -> error: inventory machines with unknown runtime (expected one of: libvirt, microvm): allod-dev ``` Assertion ordering is correct: the innermost assertion fires first, so each fixture surfaces its own message rather than a downstream one. **Drift detection is real, for the new key and for a pre-existing one.** Mutating `allod-dev.runtime` to `"bhyve"` in the committed JSON, and separately deleting `privacy-1.ip`, each fail `vm-specs-json` with the regenerate hint. The drift check is not vacuous in either direction. **The refactor is byte-identical for every pre-existing field.** `mkVmSpecs`/`mkVmSpecsJson` are a pure re-parameterization: ``` $ diff <(jq -S . master.json) <(jq -S 'map_values(del(.runtime))' pr.json) (empty, exit 0) ``` Same two machine keys (`allod-dev`, `privacy-1`), no dropped field, no reordering, `runtime` is the only addition. `builtins.toJSON` sorts keys, so the `inherit (m) ... runtime` position cannot perturb output. Nothing for `archetypes.vmFacts` or the host scripts to trip over. **No dangling references anywhere.** `vmSpecs` was a private `let` binding, never a flake output, and nothing in `archetypes`, `nexus`, `deploy`, `profiles`, `secrets`, or `tools` references it — the only hit in the whole workspace is a `brainstorm/` doc using the name incidentally. The public surface (`machines`, `lib.machines`, `lib.supportedPlatforms`, `lib.vmSpecsJson`) is unchanged. The extra JSON key is inert for both classes of consumer: `archetypes`' `vm-facts-coherence` projects `vm-specs.json` down to `{ip, forge_key}` before diffing, and the `nexus` host scripts read named fields with `jq`. No sibling repo breaks on this landing, and none breaks when its inventory pin is bumped. **Metadata.** `Refs allod/strategy#20` present, no closing keyword anywhere in the body or the commit message — correct for a non-final PR in a multi-repo arc. The R2 Medium claim and its justification match the plan's risk-table row for this milestone verbatim in substance ("machine-data contract consumed by host scripts and `vmFacts`; rollback is a revert and both values are checked"; scrutiny on schema propagation and missing/unknown-value failures). Commit message carries no attribution trailer and no hook-tripping keyword. ## Recommendation Merge after fixing 1 and 2. Both are small and both are about making the check say what it claims: 1 is the plan's own Validator validation standard applied to a line that currently passes for the wrong reason, and 2 removes a claim in the PR body that the code does not back. 3 is a judgement call — the cheap hoist is worth taking here, but deferring it to the archetypes milestone is consistent with the plan's Implementation Sequence, so it should not hold the merge on its own. The core of the change is right: the enum is enforced, the failure messages name the offending machines, the projection into `vmSpecsJson` and the committed JSON is exactly additive, and parameterizing the helpers on an explicit machine set so the real validation chain runs against sabotaged fixtures is the correct shape for this — it is what let the two validator gaps above be found by deleting assertions rather than by reading them.

allod-agent added 1 commit 2026-07-29 20:55:49 +01:00

Pin runtime mutation fixtures to their own diagnostics ... 530073c599

The mutation check recorded only whether evaluation failed, never which
diagnostic fired. Because builtins.elem 42 [ "libvirt" "microvm" ] is already
false, the runtime = 42 fixture tripped the unknown-value assertion rather than
the non-string one, and deleting the non-string assertion outright left the
check green. Classification now happens in one runtimeDiagnostics function whose
four predicates are each guarded on the previous condition, so the sets are
disjoint and a fixture can be pinned to exactly the diagnostic it targets. Each
fixture additionally asserts that the real mkVmSpecsJson path rejects it, which
catches a diagnostic that is computed correctly but never asserted on.

A hypervisor declaring a runtime fact is now rejected too. It previously
evaluated cleanly and produced identical JSON, because the surviving assertion
keyed on the non-hypervisor filter and structurally could not see it.

The chain is also forced on the machines surface consumers actually read.
archetypes consumes inventory.machines rather than lib.vmSpecsJson, and a
downstream nix flake check does not evaluate an input's own checks, so bad
runtime data on that path went unvalidated.

Each of the four assertions was proven load-bearing by neutering its condition
in turn on a scratch copy. Three then fail through the check's own diagnostic;
the missing-runtime case instead aborts evaluation with attribute 'runtime'
missing, because tryEval catches only AssertionError and ThrownError and a raw
EvalError escapes it. That is loud rather than silent, but it bounds what this
harness can report, and the same pattern is being copied into the archetypes
and nexus mutation checks.

Refs allod/strategy#20

allod-agent commented 2026-07-29 20:56:14 +01:00

Author

Member

Copy link

Review findings addressed in 530073c. All three are fixed; here is what changed and what was proven.

Finding 1, the non-string fixture proving nothing. Confirmed as described. Classification now happens in one runtimeDiagnostics function whose four predicates are each guarded on the previous condition (type first, then m ? runtime, then isString, then elem), so the four diagnostic sets are disjoint by construction and a machine with one problem trips exactly one diagnostic regardless of assertion order. Each fixture now asserts two separate things: pinnedTo proves the fixture hits exactly its own diagnostic and none of the other three, and rejects proves the real consumed mkVmSpecsJson path actually throws for it. The second half matters independently — pinnedTo alone would pass for a diagnostic that is computed correctly but never asserted on.

Finding 2, a hypervisor silently acquiring a runtime. Fixed in code rather than by weakening the PR body, since contract 1 says hypervisors do not acquire a fake guest runtime. hypervisorWithRuntime is now a real diagnostic with its own assertion and its own fixture, and the previously overclaiming comment on the nexus entry now describes enforcement that exists.

Finding 3, assertions not on the surface consumers read. Taken. It was one binding as predicted: checkedMachines = builtins.seq (mkVmSpecs machines) machines, exported as both machines and lib.machines. builtins.seq forces the assertions and returns the original value unchanged, so consumers see the same shape. No recursion.

Per-assertion deletion proof. Each of the four assertion conditions was neutered to (true) in turn on a scratch copy and the check re-run. All four are load-bearing:

Assertion neutered	Result
hypervisorWithRuntime	RED — hypervisor-with-runtime: fails mkVmSpecsJson: expected true but got false
missingRuntime	RED — evaluation aborts with error: attribute 'runtime' missing
nonStringRuntime	RED — non-string runtime: fails mkVmSpecsJson: expected true but got false
unknownRuntime	RED — unknown runtime: fails mkVmSpecsJson: expected true but got false

The missingRuntime row is the harness limitation the review identified, now characterized precisely. With that assertion gone, the JSON projection's inherit (m) ... runtime raises a raw EvalError, and tryEval catches only AssertionError and ThrownError, so it escapes and takes down the whole flake evaluation instead of failing the check through its own diagnostic. That is loud rather than silent, so the assertion is genuinely load-bearing, but it bounds what this harness can report: a tryEval-based fixture cannot distinguish "my assertion fired" from "evaluation died for an unrelated reason" unless it also checks which diagnostic set is non-empty. This is called out here because the same harness pattern is being copied into the allod/vm, allod/nexus, and allod/archetypes mutation checks, and the blind spot should not travel with it.

Full acceptance set re-run after the fix, all green:

runtime-fact-mutations-check> OK: valid machines have no runtime diagnostics (true)
runtime-fact-mutations-check> OK: valid machines evaluate (true)
runtime-fact-mutations-check> OK: hypervisor-with-runtime: pinned to its own diagnostic (true)
runtime-fact-mutations-check> OK: hypervisor-with-runtime: fails mkVmSpecsJson (true)
runtime-fact-mutations-check> OK: missing runtime: pinned to its own diagnostic (true)
runtime-fact-mutations-check> OK: missing runtime: fails mkVmSpecsJson (true)
runtime-fact-mutations-check> OK: non-string runtime: pinned to its own diagnostic (true)
runtime-fact-mutations-check> OK: non-string runtime: fails mkVmSpecsJson (true)
runtime-fact-mutations-check> OK: unknown runtime: pinned to its own diagnostic (true)
runtime-fact-mutations-check> OK: unknown runtime: fails mkVmSpecsJson (true)
runtime-fact-mutations-check> OK: hypervisor entry 'nexus' absent from vmSpecsJson
runtime-fact-mutations-check> OK: allod-dev is the public microvm example
runtime-fact-mutations-check> OK: privacy-1 is the public libvirt example
runtime-fact-mutations-check> OK: sabotaged JSON diverges from generated JSON (drift detection proven)
repository-registry-check> Registry validation passed: 10 repositories, all checks OK

nix flake check --print-build-logs exits 0 across all three checks. The jq -e enum assertion returns true, and diff -u between nix eval .#lib.vmSpecsJson and committed scripts/vm-specs.json is empty.

One correction to my own process, recorded so the next reviewer does not repeat it: my first sabotage sweep classified the missingRuntime case as leaving the check green, because it grepped for the check's own ERROR: lines and an eval abort produces none. The assertion was load-bearing all along; the detection was wrong. Sabotage sweeps on this pattern need to distinguish three outcomes, not two — green, red via diagnostic, and red via eval abort.

Review findings addressed in `530073c`. All three are fixed; here is what changed and what was proven. **Finding 1, the non-string fixture proving nothing.** Confirmed as described. Classification now happens in one `runtimeDiagnostics` function whose four predicates are each guarded on the previous condition (`type` first, then `m ? runtime`, then `isString`, then `elem`), so the four diagnostic sets are disjoint by construction and a machine with one problem trips exactly one diagnostic regardless of assertion order. Each fixture now asserts two separate things: `pinnedTo` proves the fixture hits exactly its own diagnostic and none of the other three, and `rejects` proves the real consumed `mkVmSpecsJson` path actually throws for it. The second half matters independently — `pinnedTo` alone would pass for a diagnostic that is computed correctly but never asserted on. **Finding 2, a hypervisor silently acquiring a runtime.** Fixed in code rather than by weakening the PR body, since contract 1 says hypervisors do not acquire a fake guest runtime. `hypervisorWithRuntime` is now a real diagnostic with its own assertion and its own fixture, and the previously overclaiming comment on the `nexus` entry now describes enforcement that exists. **Finding 3, assertions not on the surface consumers read.** Taken. It was one binding as predicted: `checkedMachines = builtins.seq (mkVmSpecs machines) machines`, exported as both `machines` and `lib.machines`. `builtins.seq` forces the assertions and returns the original value unchanged, so consumers see the same shape. No recursion. **Per-assertion deletion proof.** Each of the four assertion conditions was neutered to `(true)` in turn on a scratch copy and the check re-run. All four are load-bearing: | Assertion neutered | Result | |---|---| | `hypervisorWithRuntime` | RED — `hypervisor-with-runtime: fails mkVmSpecsJson: expected true but got false` | | `missingRuntime` | RED — evaluation aborts with `error: attribute 'runtime' missing` | | `nonStringRuntime` | RED — `non-string runtime: fails mkVmSpecsJson: expected true but got false` | | `unknownRuntime` | RED — `unknown runtime: fails mkVmSpecsJson: expected true but got false` | The `missingRuntime` row is the harness limitation the review identified, now characterized precisely. With that assertion gone, the JSON projection's `inherit (m) ... runtime` raises a raw `EvalError`, and `tryEval` catches only `AssertionError` and `ThrownError`, so it escapes and takes down the whole flake evaluation instead of failing the check through its own diagnostic. That is loud rather than silent, so the assertion is genuinely load-bearing, but it bounds what this harness can report: a `tryEval`-based fixture cannot distinguish "my assertion fired" from "evaluation died for an unrelated reason" unless it also checks which diagnostic set is non-empty. This is called out here because the same harness pattern is being copied into the `allod/vm`, `allod/nexus`, and `allod/archetypes` mutation checks, and the blind spot should not travel with it. **Full acceptance set re-run after the fix**, all green: ``` runtime-fact-mutations-check> OK: valid machines have no runtime diagnostics (true) runtime-fact-mutations-check> OK: valid machines evaluate (true) runtime-fact-mutations-check> OK: hypervisor-with-runtime: pinned to its own diagnostic (true) runtime-fact-mutations-check> OK: hypervisor-with-runtime: fails mkVmSpecsJson (true) runtime-fact-mutations-check> OK: missing runtime: pinned to its own diagnostic (true) runtime-fact-mutations-check> OK: missing runtime: fails mkVmSpecsJson (true) runtime-fact-mutations-check> OK: non-string runtime: pinned to its own diagnostic (true) runtime-fact-mutations-check> OK: non-string runtime: fails mkVmSpecsJson (true) runtime-fact-mutations-check> OK: unknown runtime: pinned to its own diagnostic (true) runtime-fact-mutations-check> OK: unknown runtime: fails mkVmSpecsJson (true) runtime-fact-mutations-check> OK: hypervisor entry 'nexus' absent from vmSpecsJson runtime-fact-mutations-check> OK: allod-dev is the public microvm example runtime-fact-mutations-check> OK: privacy-1 is the public libvirt example runtime-fact-mutations-check> OK: sabotaged JSON diverges from generated JSON (drift detection proven) repository-registry-check> Registry validation passed: 10 repositories, all checks OK ``` `nix flake check --print-build-logs` exits 0 across all three checks. The `jq -e` enum assertion returns `true`, and `diff -u` between `nix eval .#lib.vmSpecsJson` and committed `scripts/vm-specs.json` is empty. One correction to my own process, recorded so the next reviewer does not repeat it: my first sabotage sweep classified the `missingRuntime` case as leaving the check green, because it grepped for the check's own `ERROR:` lines and an eval abort produces none. The assertion was load-bearing all along; the detection was wrong. Sabotage sweeps on this pattern need to distinguish three outcomes, not two — green, red via diagnostic, and red via eval abort.

allod-agent added 1 commit 2026-07-30 19:45:40 +01:00

Keep the operator's dev machine on libvirt 15ad5528ed

allod-agent commented 2026-07-30 19:46:37 +01:00

Author

Member

Copy link

15ad552 takes allod-dev off the microvm runtime and adds a check that keeps it off.

allod-dev is the machine the operator develops from, and it is a real machine, not a synthetic example: allod/secrets carries allod-dev-forge-key.age, forgejo-https-token-allod-dev.age and a host key entry keyed to that name. Marking it runtime = "microvm" was therefore pointed at the working environment. Inert today, because nothing reads the fact until the archetypes milestone — but that milestone acts on it by name, and a dev guest that fails to boot takes its own repair environment with it.

Renaming was considered and rejected. Per-machine encrypted secret filenames are keyed to the machine name, and re-keying them is a human-only host action, so a rename is not a text edit.

Verified: with this branch overridden into the composition root, allod-dev still evaluates to i2bbywbv08bfbn6j9cr9awm1gd399pyj, byte-identical to the current lock. Nothing about that machine changes.

The check that asserted allod-dev is the public microvm example is replaced by one asserting it stays on libvirt, so the mistake cannot come back quietly.

No microvm example machine in this PR, deliberately

The obvious follow-through — add a purpose-made machine to carry the microvm runtime — does not fit in this PR, and it is worth recording why rather than leaving a gap.

A machine entry is not self-contained. Adding one requires a matching identity in secrets (archetypes asserts the machine and identity key sets are equal), a profile in profiles, and per-machine encrypted credentials. I built all of that and evaluated it end to end; it fails at the last step with path '.../secrets/forgejo-https-token-dev-2.age' does not exist. Creating that file needs a real token encrypted to the machine's own host key, and generating a host key is a host-only action behind the agent gates. So the machine cannot be completed from here, and landing the data half alone would leave an entry that the composition root cannot build.

The machine that first selects the microvm runtime should therefore be added in the same change that provisions it. The runtime enum's microvm branch stays covered by this PR's mutation fixtures, which do not need an example machine.

nix flake check green.

`15ad552` takes `allod-dev` off the microvm runtime and adds a check that keeps it off. `allod-dev` is the machine the operator develops from, and it is a real machine, not a synthetic example: `allod/secrets` carries `allod-dev-forge-key.age`, `forgejo-https-token-allod-dev.age` and a host key entry keyed to that name. Marking it `runtime = "microvm"` was therefore pointed at the working environment. Inert today, because nothing reads the fact until the archetypes milestone — but that milestone acts on it by name, and a dev guest that fails to boot takes its own repair environment with it. Renaming was considered and rejected. Per-machine encrypted secret filenames are keyed to the machine name, and re-keying them is a human-only host action, so a rename is not a text edit. Verified: with this branch overridden into the composition root, `allod-dev` still evaluates to `i2bbywbv08bfbn6j9cr9awm1gd399pyj`, byte-identical to the current lock. Nothing about that machine changes. The check that asserted `allod-dev` is the public microvm example is replaced by one asserting it stays on libvirt, so the mistake cannot come back quietly. ## No microvm example machine in this PR, deliberately The obvious follow-through — add a purpose-made machine to carry the microvm runtime — does not fit in this PR, and it is worth recording why rather than leaving a gap. A machine entry is not self-contained. Adding one requires a matching identity in `secrets` (archetypes asserts the machine and identity key sets are equal), a profile in `profiles`, and per-machine encrypted credentials. I built all of that and evaluated it end to end; it fails at the last step with `path '.../secrets/forgejo-https-token-dev-2.age' does not exist`. Creating that file needs a real token encrypted to the machine's own host key, and generating a host key is a host-only action behind the agent gates. So the machine cannot be completed from here, and landing the data half alone would leave an entry that the composition root cannot build. The machine that first selects the microvm runtime should therefore be added in the same change that provisions it. The runtime enum's microvm branch stays covered by this PR's mutation fixtures, which do not need an example machine. `nix flake check` green.

allod-agent referenced this pull request 2026-07-30 20:19:54 +01:00

Define the microvm-test machine in the private forks #11

allod-agent commented 2026-07-30 20:20:06 +01:00

Author

Member

Copy link

The machine that carries the microvm runtime is now specified and mostly built, but it is not in this PR and this PR stays inert.

It is called microvm-test, and the work that could be done without a human is pushed on branches: the identity and host key in allod/secrets (agent/microvm-test-machine), and the profile in allod/profiles (same branch name). The inventory entry is written but held back, because adding it here would make this PR change a machine, and it cannot evaluate anyway until the machine's Forgejo token exists — minting that is a human action.

allod/inventory#11 carries the whole handover: why the machine exists, what is done, the exact blocker, the steps to finish on the host, the inventory entry to paste, and what has to be repeated privately with real values.

So this PR still does exactly what it says: it adds the runtime fact, keeps both public example machines on libvirt, and keeps allod-dev there by assertion.

The machine that carries the microvm runtime is now specified and mostly built, but it is not in this PR and this PR stays inert. It is called `microvm-test`, and the work that could be done without a human is pushed on branches: the identity and host key in `allod/secrets` (`agent/microvm-test-machine`), and the profile in `allod/profiles` (same branch name). The inventory entry is written but held back, because adding it here would make this PR change a machine, and it cannot evaluate anyway until the machine's Forgejo token exists — minting that is a human action. allod/inventory#11 carries the whole handover: why the machine exists, what is done, the exact blocker, the steps to finish on the host, the inventory entry to paste, and what has to be repeated privately with real values. So this PR still does exactly what it says: it adds the runtime fact, keeps both public example machines on libvirt, and keeps `allod-dev` there by assertion.

vnprc approved these changes 2026-07-30 22:33:50 +01:00

vnprc merged commit 15ad5528ed into master 2026-07-30 22:33:55 +01:00

vnprc deleted branch agent/inventory-runtime-fact 2026-07-30 22:33:56 +01:00

Sign in to join this conversation.

Reviewers

No reviewers

vnprc

Labels

Clear labels   

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

wontfix

This won't be fixed

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

wontfix

This won't be fixed

No labels

bug

duplicate

enhancement

help wanted

invalid

question

wontfix

bug

duplicate

enhancement

help wanted

invalid

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

allod/inventory!10

No description provided.