home-shared: migrate programs.ssh.matchBlocks to programs.ssh.settings #80

Closed
allod-agent wants to merge 2 commits from agent/ssh-settings into master
Member

Every VM user profile and the hypervisor's user profile stop printing the programs.ssh.matchBlocks deprecation warnings: modules/home-shared.nix now defines the operator's SSH hosts through programs.ssh.settings, the option Home Manager 26.05 replaced the alias with. The rendered ~/.ssh/config is unchanged, and the built VM system is byte-identical: evaluating the deploy template's allod-dev with this change gives the same derivation path as without it. identity.sshHosts keeps its shape, so the private secrets fork needs no edit. A new flake check builds the module against a fixture identity and resolves every host with a real ssh -G. If it is wrong, revert this PR; nothing else depends on it yet.

Merge order with allod/profiles#9 (issue allod/profiles#8). That profile defines the same forge-host block. This PR must land, and the deploy lock must take it, at or before the profiles change: with this module still on matchBlocks and the profile already on settings, Home Manager's legacy conversion drops this module's mkDefault and retypes the identity file, and allod-dev fails to evaluate with a conflicting-type error. Measured, not assumed. The reverse order is safe and is what the new check pins.

What changes

  • modules/home-shared.nix: a small translation turns each identity.sshHosts entry (hostname, user, port, identityFile, identitiesOnly) into the upstream directive names (HostName, User, Port, IdentityFile, IdentitiesOnly) and merges extraOptions into the block, since its keys already are directive names and settings.*.extraOptions is rejected by Home Manager. Any other key fails evaluation naming the host and the key rather than being dropped. The forge host's IdentityFile stays under lib.mkDefault so a profile's own definition wins.
  • checks/ssh-settings.nix, registered as ssh-settings: two fixture home configurations through the real module, one with a second module defining the forge host through legacy matchBlocks (an unmigrated profile), one without. ssh -G -F <generated config> <host> resolves a plain host, a host with extraOptions, and the forge host, and the check asserts hostname, user, port, identity file, IdentitiesOnly and the extra directive, that the legacy profile's identity file wins when present, that the module's default wins when it is absent, and that the module defines no matchBlocks.

Risk

R3: a shared interface consumed by every VM and by the host user profile. The worst credible failure after validation is a deployment's sshHosts entry carrying a key the translation does not know, which is a loud evaluation error at the next lock bump, not a silent config change. The authoritative data is untouched. Rollback is a revert.

Validation

Run by the integrator at 549eca4:

  • nix build .#checks.x86_64-linux.ssh-settings --rebuild exits 0.
  • nix flake check --no-build reports all checks passed.
  • Against the deploy template with --override-input archetypes <this worktree>: allod-dev evaluates to the same toplevel derivation as master (is37d2i7…), and the only matchBlocks warning left names hosts/dev/allod-dev/home.nix in profiles. With --override-input profiles set to the allod/profiles#8 branch as well: no warnings, and the rendered ~/.ssh/config is byte-identical to the current one.

Cold cross-family review: gpt-5.6-sol via codex exec, reasoning effort high, read the diff at 52e9e08 (a Claude-family worker authored it). Three findings. One marked blocker, that the check does not pin the profile-first order: that order fails in Home Manager itself and cannot be made to pass by this module, so it is the merge-order constraint above rather than a code change; the issue text that claimed order would not matter is corrected in a comment on allod/archetypes#79. Two should-fix, both real and fixed in 549eca4: an extraOptions key colliding with a translated directive silently replaced the typed value, now an evaluation error with a sabotage fixture in the check; and the identity-file assertions were substring matches, now exact full-line equality over the whole ssh -G result. The integrator also fixed a throw whose message was truncated by operator precedence. No second reading follows the fix round; the fixes are settled by the rebuilt check and the deploy measurements above.

Closes allod/archetypes#79

Every VM user profile and the hypervisor's user profile stop printing the `programs.ssh.matchBlocks` deprecation warnings: `modules/home-shared.nix` now defines the operator's SSH hosts through `programs.ssh.settings`, the option Home Manager 26.05 replaced the alias with. The rendered `~/.ssh/config` is unchanged, and the built VM system is byte-identical: evaluating the deploy template's `allod-dev` with this change gives the same derivation path as without it. `identity.sshHosts` keeps its shape, so the private secrets fork needs no edit. A new flake check builds the module against a fixture identity and resolves every host with a real `ssh -G`. If it is wrong, revert this PR; nothing else depends on it yet. **Merge order with allod/profiles#9 (issue allod/profiles#8).** That profile defines the same forge-host block. This PR must land, and the deploy lock must take it, at or before the profiles change: with this module still on `matchBlocks` and the profile already on `settings`, Home Manager's legacy conversion drops this module's `mkDefault` and retypes the identity file, and `allod-dev` fails to evaluate with a conflicting-type error. Measured, not assumed. The reverse order is safe and is what the new check pins. ## What changes - `modules/home-shared.nix`: a small translation turns each `identity.sshHosts` entry (`hostname`, `user`, `port`, `identityFile`, `identitiesOnly`) into the upstream directive names (`HostName`, `User`, `Port`, `IdentityFile`, `IdentitiesOnly`) and merges `extraOptions` into the block, since its keys already are directive names and `settings.*.extraOptions` is rejected by Home Manager. Any other key fails evaluation naming the host and the key rather than being dropped. The forge host's `IdentityFile` stays under `lib.mkDefault` so a profile's own definition wins. - `checks/ssh-settings.nix`, registered as `ssh-settings`: two fixture home configurations through the real module, one with a second module defining the forge host through legacy `matchBlocks` (an unmigrated profile), one without. `ssh -G -F <generated config> <host>` resolves a plain host, a host with `extraOptions`, and the forge host, and the check asserts hostname, user, port, identity file, `IdentitiesOnly` and the extra directive, that the legacy profile's identity file wins when present, that the module's default wins when it is absent, and that the module defines no `matchBlocks`. ## Risk R3: a shared interface consumed by every VM and by the host user profile. The worst credible failure after validation is a deployment's `sshHosts` entry carrying a key the translation does not know, which is a loud evaluation error at the next lock bump, not a silent config change. The authoritative data is untouched. Rollback is a revert. ## Validation Run by the integrator at 549eca4: - `nix build .#checks.x86_64-linux.ssh-settings --rebuild` exits 0. - `nix flake check --no-build` reports all checks passed. - Against the deploy template with `--override-input archetypes <this worktree>`: `allod-dev` evaluates to the same toplevel derivation as master (`is37d2i7…`), and the only `matchBlocks` warning left names `hosts/dev/allod-dev/home.nix` in profiles. With `--override-input profiles` set to the allod/profiles#8 branch as well: no warnings, and the rendered `~/.ssh/config` is byte-identical to the current one. Cold cross-family review: `gpt-5.6-sol` via `codex exec`, reasoning effort `high`, read the diff at 52e9e08 (a Claude-family worker authored it). Three findings. One marked blocker, that the check does not pin the profile-first order: that order fails in Home Manager itself and cannot be made to pass by this module, so it is the merge-order constraint above rather than a code change; the issue text that claimed order would not matter is corrected in a comment on allod/archetypes#79. Two should-fix, both real and fixed in 549eca4: an `extraOptions` key colliding with a translated directive silently replaced the typed value, now an evaluation error with a sabotage fixture in the check; and the identity-file assertions were substring matches, now exact full-line equality over the whole `ssh -G` result. The integrator also fixed a `throw` whose message was truncated by operator precedence. No second reading follows the fix round; the fixes are settled by the rebuilt check and the deploy measurements above. Closes allod/archetypes#79
identity.sshHosts keeps its current shape; home-shared.nix now translates
its keys (hostname, user, port, identityFile, identitiesOnly) to upstream
ssh_config directive names and merges extraOptions directly into the same
block, since extraOptions already carries upstream directive names and
programs.ssh.settings.*.extraOptions itself is rejected. An unrecognized
key fails evaluation naming the host and the key. The forge host's
IdentityFile stays under lib.mkDefault so an unmigrated profile's own
definition still wins.

Adds checks/ssh-settings.nix, registered in flake.nix, which builds two
fixture home configurations through the real home-shared.nix (one beside
an unmigrated profile's matchBlocks block, one without) and runs ssh -G
against each generated config to pin the resolved directives, including
that the legacy profile's IdentityFile still wins when present.

Refs allod/archetypes#79
Reject an extraOptions key that collides with a translated directive
(e.g. port set alongside extraOptions.Port) as an evaluation error naming
the host and the colliding key, instead of letting `//` silently drop the
typed value. Before this migration matchBlocks rendered the typed
directive first and OpenSSH took the first occurrence, so the typed value
always won; silently reversing that on migration would be a behavior
change with no diagnostic.

Fix both throw call sites: `throw msg + "..."` parses as `(throw msg) +
"..."`, dropping the second half of the message. Build each full message
in a let-bound value first, then throw the complete string.

checks/ssh-settings.nix: set HOME explicitly in the runCommand sandbox and
replace the identityfile substring grep with an exact match on the whole
set of resolved identityfile lines, so a stray extra line or a
similarly-named file no longer passes. Add a sabotage fixture with a
colliding extraOptions.Port and assert, via builtins.tryEval, that it
fails evaluation.

Refs allod/archetypes#79
vnprc force-pushed agent/ssh-settings from 549eca4696 to c71704d834 2026-09-19 23:35:45 +01:00 Compare
First-time contributor

Hold, do not merge. Measured against a deployment whose sshHosts entries carry localForwards, this branch at c71704d fails to evaluate every machine that consumes that identity:

error: identity.sshHosts.<host> sets key(s) [localForwards] that programs.ssh.settings has no translation for

The body's claim that a deployment's secrets fork needs no edit does not hold; the validation covered the public template, whose entries have no forwarding fields. The failure is loud, so nothing wrong would deploy, but the translation must know every field the address book can carry, and the template should carry them so the check sees them. Separately, the body cites validation and a fix round at 549eca4, which is not an ancestor of the branch tip.

The direction has also changed: allod/archetypes#54 now removes the two-author seam instead of porting the mkDefault to the new option, which is what forced the landing order this PR documents. The translation function and the ssh -G check here are good and are harvested into that change; this PR closes when it lands. Tracking is allod/strategy#56.

Hold, do not merge. Measured against a deployment whose `sshHosts` entries carry `localForwards`, this branch at c71704d fails to evaluate every machine that consumes that identity: ``` error: identity.sshHosts.<host> sets key(s) [localForwards] that programs.ssh.settings has no translation for ``` The body's claim that a deployment's secrets fork needs no edit does not hold; the validation covered the public template, whose entries have no forwarding fields. The failure is loud, so nothing wrong would deploy, but the translation must know every field the address book can carry, and the template should carry them so the check sees them. Separately, the body cites validation and a fix round at 549eca4, which is not an ancestor of the branch tip. The direction has also changed: allod/archetypes#54 now removes the two-author seam instead of porting the `mkDefault` to the new option, which is what forced the landing order this PR documents. The translation function and the `ssh -G` check here are good and are harvested into that change; this PR closes when it lands. Tracking is allod/strategy#56.
Owner

closed in favor of allod/strategy#56

closed in favor of allod/strategy#56
vnprc closed this pull request 2026-09-20 01:41:01 +01:00

Pull request closed

Sign in to join this conversation.
No description provided.