nimmo/nixos-config

Watch

Fork 0

feat(nix): generalise remote deployment across the fleet (#237) #248

Merged

nimmo merged 2 commits from feat/issue-237-generalise-remote-deployment into main

2026-09-21 20:25:00 +01:00

nimmo commented

2026-09-21 17:46:47 +01:00

Owner

Summary

Generalises the proven Electra → Nova → target controller workflow from #236 into a fleet-capable remote deployment interface that supports test, boot, and switch actions.

Relates to #237.

Changes

Renamed and extended scripts/nixos-controller-test.sh → scripts/nixos-remote-deploy.sh.
- Accepts a positional target host and an optional activation action.
- Supports test (default), boot, and switch.
- For boot/switch, sets the target's /nix/var/nix/profiles/system profile before activation.
- Still supports --host and --action flags for direct invocation.
- Keeps the working-tree snapshot semantics from #236 (tracked + untracked files, excluding Git metadata and build/editor state).
- Keeps the security separation: builder credential never reaches the target; controller signing key never reaches Nova.
Updated just test-remote recipe.
- Target is now positional: just test-remote vega instead of just test-remote --host vega.
- Action is optional: just test-remote lyra switch.
Updated documentation.
- Fixed stale just test-controller references in README.md and docs/nova-remote-builder.md.
- Added fleet eligibility and exclusions section.
- Added target-side recovery procedures (Nova unavailable, controller SSH unavailable).

Validation

nix flake check passes.
Existing evidence:
- Vega proven in #236.
- Lyra and Lena proven with the generalised workflow (recorded in #237 comments).

Remaining before closing #237

The target-side recovery scenarios (Nova outage, controller SSH unavailable after a switch) are documented but not yet exercised live. Once they are demonstrated, #237 can be closed.

## Summary Generalises the proven Electra → Nova → target controller workflow from #236 into a fleet-capable remote deployment interface that supports `test`, `boot`, and `switch` actions. Relates to #237. ## Changes - **Renamed and extended `scripts/nixos-controller-test.sh` → `scripts/nixos-remote-deploy.sh`.** - Accepts a positional target host and an optional activation action. - Supports `test` (default), `boot`, and `switch`. - For `boot`/`switch`, sets the target's `/nix/var/nix/profiles/system` profile before activation. - Still supports `--host` and `--action` flags for direct invocation. - Keeps the working-tree snapshot semantics from #236 (tracked + untracked files, excluding Git metadata and build/editor state). - Keeps the security separation: builder credential never reaches the target; controller signing key never reaches Nova. - **Updated `just test-remote` recipe.** - Target is now positional: `just test-remote vega` instead of `just test-remote --host vega`. - Action is optional: `just test-remote lyra switch`. - **Updated documentation.** - Fixed stale `just test-controller` references in `README.md` and `docs/nova-remote-builder.md`. - Added fleet eligibility and exclusions section. - Added target-side recovery procedures (Nova unavailable, controller SSH unavailable). ## Validation - `nix flake check` passes. - Existing evidence: - **Vega** proven in #236. - **Lyra** and **Lena** proven with the generalised workflow (recorded in #237 comments). ## Remaining before closing #237 The target-side recovery scenarios (Nova outage, controller SSH unavailable after a switch) are documented but not yet exercised live. Once they are demonstrated, #237 can be closed.

nimmo added 1 commit

2026-09-21 17:46:48 +01:00

feat(nix): generalise controller remote deployment with boot/switch actions

AI pull-request review / review (pull_request) Successful in 1m28s

Details

6a14d28e6a

nimmo commented

2026-09-21 17:48:14 +01:00

Author

Owner

Automated code review

Reviewed commit: a65a656427794b6a38196906818801fb67d55c73

Verdict: Ready to merge

No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory.

Overall assessment

The PR refactors the remote deployment workflow by renaming nixos-controller-test.sh to nixos-remote-deploy.sh, adding support for boot and switch activation actions, and updating documentation. The human reviewer confirmed that previous findings regarding unsafe variable interpolation and argument parsing were fixed in a subsequent commit (a65a656). The provided diff reflects the state including these fixes (e.g., safe argument passing via $1 to sh -c, explicit positional argument handling). No new security or logic defects are evident in the supplied changes.

Blocking findings

None.

Important findings

None.

Suggestions

None.

Tests and validation

No configured deterministic checks.

Questions

The documentation mentions just test-remote vega as a command. The Justfile defines test-remote TARGET ACTION="test". Does the Justfile syntax correctly pass the single positional argument when only the target is provided, or does it require explicit empty action handling in the shell wrapper?

Review limitations

The review is based solely on the provided diff and human replies. I cannot verify the runtime behavior of the nixos-remote-deploy.sh script against actual NixOS targets.
I cannot confirm if the commit a65a656 fully resolves all edge cases in argument parsing without seeing its specific diff, although the current diff appears to implement the described fixes.

Diff coverage

README.md: reviewed — included in a context-limited batch
docs/nova-remote-builder.md: reviewed — included in a context-limited batch
justfiles/deploy.just: reviewed — included in a context-limited batch
scripts/nixos-controller-test.sh: reviewed — included in a context-limited batch
scripts/nixos-remote-deploy.sh: reviewed — included in a context-limited batch

Review metadata

Reviewer: forgejo-ai-review v1
Model: qwen3.6:35b-a3b-q8_0
Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, docs/auto-update-remediation.md, docs/nova-remote-builder.md, README.md
Commits considered: 2
Previous automated review: updated
Findings: 0

## Automated code review Reviewed commit: `a65a656427794b6a38196906818801fb67d55c73` ## Verdict: Ready to merge No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory. ### Overall assessment The PR refactors the remote deployment workflow by renaming `nixos-controller-test.sh` to `nixos-remote-deploy.sh`, adding support for `boot` and `switch` activation actions, and updating documentation. The human reviewer confirmed that previous findings regarding unsafe variable interpolation and argument parsing were fixed in a subsequent commit (`a65a656`). The provided diff reflects the state including these fixes (e.g., safe argument passing via `$1` to `sh -c`, explicit positional argument handling). No new security or logic defects are evident in the supplied changes. ### Blocking findings None. ### Important findings None. ### Suggestions None. ### Tests and validation - No configured deterministic checks. ### Questions - The documentation mentions `just test-remote vega` as a command. The Justfile defines `test-remote TARGET ACTION="test"`. Does the Justfile syntax correctly pass the single positional argument when only the target is provided, or does it require explicit empty action handling in the shell wrapper? ### Review limitations - The review is based solely on the provided diff and human replies. I cannot verify the runtime behavior of the `nixos-remote-deploy.sh` script against actual NixOS targets. - I cannot confirm if the commit `a65a656` fully resolves all edge cases in argument parsing without seeing its specific diff, although the current diff appears to implement the described fixes. ### Diff coverage - `README.md`: **reviewed** — included in a context-limited batch - `docs/nova-remote-builder.md`: **reviewed** — included in a context-limited batch - `justfiles/deploy.just`: **reviewed** — included in a context-limited batch - `scripts/nixos-controller-test.sh`: **reviewed** — included in a context-limited batch - `scripts/nixos-remote-deploy.sh`: **reviewed** — included in a context-limited batch <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, docs/auto-update-remediation.md, docs/nova-remote-builder.md, README.md - Commits considered: 2 - Previous automated review: updated - Findings: 0 </details>

nimmo added 1 commit

2026-09-21 18:11:36 +01:00

fix(nix): safer remote shell invocation and argument parsing

AI pull-request review / review (pull_request) Successful in 1m31s

Details

a65a656427

nimmo commented

2026-09-21 18:11:39 +01:00

Author

Owner

Thanks for the automated review. Both findings were relevant:

Unsafe interpolation in boot/switch SSH commands — fixed. $built_toplevel is now passed as an argument to the remote sh -c script and referenced via "$1", so it is not interpolated into the command string.
Fragile positional argument parsing — fixed. The script now collects positional arguments into an array after flag processing and assigns target/action explicitly, instead of relying on the default action value.

Pushed as commit a65a656. nix flake check still passes.

Thanks for the automated review. Both findings were relevant: 1. **Unsafe interpolation in `boot`/`switch` SSH commands** — fixed. `$built_toplevel` is now passed as an argument to the remote `sh -c` script and referenced via `"$1"`, so it is not interpolated into the command string. 2. **Fragile positional argument parsing** — fixed. The script now collects positional arguments into an array after flag processing and assigns target/action explicitly, instead of relying on the default action value. Pushed as commit `a65a656`. `nix flake check` still passes.

nimmo merged commit 87a543342e into main

2026-09-21 20:25:00 +01:00

nimmo referenced this pull request from a commit

2026-09-21 20:25:00 +01:00

Merge pull request 'feat(nix): generalise remote deployment across the fleet (#237)' (#248) from feat/issue-237-generalise-remote-deployment into main

nimmo deleted branch feat/issue-237-generalise-remote-deployment

2026-09-21 20:25:01 +01:00

nimmo referenced this pull request

2026-09-21 20:49:09 +01:00

06 – Generalise Remote Deployment Across the NixOS Fleet #237