188f3ea5ec
Part of the docs-migration chore (issue #708). Remove GitHub issue numbers from inline comments, option descriptions, and rustdoc — these are contextless noise for anyone reading the code without access to the original discussions. Replace with prose that captures the same rationale directly. No functional change. Build still clean (cargo check passes).
430 lines
16 KiB
Rust
430 lines
16 KiB
Rust
//! Pre-apply validation for agent `flake.lock` files.
|
|
//!
|
|
//! Every `request_apply_commit` lands a `proposal/<id>` tag in the
|
|
//! agent's applied repo before the operator sees the approval. We
|
|
//! parse `flake.lock` from that tag's tree and reject the request if
|
|
//! two or more nodes share an identical `original` field — that
|
|
//! signals a missing `inputs.<X>.inputs.nixpkgs.follows = "nixpkgs"`
|
|
//! directive in `flake.nix` and would inflate meta's lock with
|
|
//! duplicates after deploy.
|
|
//!
|
|
//! The check runs on the agent repo, not meta, and catches *new*
|
|
//! violations only. Existing agents whose lock already has duplicates
|
|
//! are out of scope here and get a coordinated config-change pass via
|
|
//! the manager instead.
|
|
|
|
use std::collections::BTreeMap;
|
|
use std::fmt::Write as _;
|
|
use std::path::Path;
|
|
|
|
use anyhow::{Context, Result};
|
|
use serde_json::Value;
|
|
use tokio::process::Command;
|
|
|
|
use crate::lifecycle::git_command;
|
|
|
|
/// One group of `flake.lock` nodes that all share the same canonical
|
|
/// `original` reference. Surfaced in the rejection message so the
|
|
/// operator (and the manager that submitted the apply) can see
|
|
/// exactly which input pair needs a `follows` directive.
|
|
#[derive(Debug, Clone)]
|
|
pub struct DuplicateGroup {
|
|
/// One of the original `Value`s from the lock — used for
|
|
/// pretty-printing in the error message. Canonical equivalence
|
|
/// is enforced by the `BTreeMap` key in `duplicate_groups`, so
|
|
/// we don't need to keep the canonicalised form on the struct.
|
|
pub original: Value,
|
|
/// Names of the flake.lock nodes that share this `original`,
|
|
/// sorted for stable error output.
|
|
pub keys: Vec<String>,
|
|
}
|
|
|
|
/// Read `flake.lock` from `<tag>:flake.lock` in `repo`. Returns
|
|
/// `Ok(None)` when the file isn't tracked in that tag (no inputs ⇒
|
|
/// nothing to dedup); `Err` only on real git plumbing failures.
|
|
async fn read_lock_at_tag(repo: &Path, tag: &str) -> Result<Option<String>> {
|
|
let spec = format!("{tag}:flake.lock");
|
|
let out = git_command()
|
|
.current_dir(repo)
|
|
.args(["show", &spec])
|
|
.output()
|
|
.await
|
|
.with_context(|| format!("git show {spec} in {}", repo.display()))?;
|
|
if !out.status.success() {
|
|
let stderr = String::from_utf8_lossy(&out.stderr);
|
|
// git uses two different messages for "path not in tree"
|
|
// depending on whether the path also collides with an on-disk
|
|
// file. Both translate to "no flake.lock in this commit" —
|
|
// a legitimate, dedup-clean state for an agent with empty
|
|
// `inputs = { }`. Any other git failure (permission denied,
|
|
// ref-not-found, etc.) propagates as a hard error rather than
|
|
// being silently swallowed.
|
|
if stderr.contains("does not exist")
|
|
|| stderr.contains("exists on disk, but not in")
|
|
{
|
|
return Ok(None);
|
|
}
|
|
anyhow::bail!("git show {spec} failed: {}", stderr.trim());
|
|
}
|
|
Ok(Some(String::from_utf8_lossy(&out.stdout).into_owned()))
|
|
}
|
|
|
|
/// Recursively serialise `v` with object keys sorted, so two
|
|
/// JSON values that differ only in key insertion order produce the
|
|
/// same string. `serde_json::Value` preserves `IndexMap` order by
|
|
/// default, which is fine for parsing but breaks our group-by-key
|
|
/// idea — hence this hand-rolled canonicaliser.
|
|
fn canonical_json(v: &Value) -> String {
|
|
match v {
|
|
Value::Object(map) => {
|
|
let mut keys: Vec<&String> = map.keys().collect();
|
|
keys.sort();
|
|
let mut s = String::from("{");
|
|
for (i, k) in keys.iter().enumerate() {
|
|
if i > 0 {
|
|
s.push(',');
|
|
}
|
|
s.push_str(&serde_json::to_string(k).unwrap_or_default());
|
|
s.push(':');
|
|
s.push_str(&canonical_json(&map[*k]));
|
|
}
|
|
s.push('}');
|
|
s
|
|
}
|
|
Value::Array(arr) => {
|
|
let mut s = String::from("[");
|
|
for (i, x) in arr.iter().enumerate() {
|
|
if i > 0 {
|
|
s.push(',');
|
|
}
|
|
s.push_str(&canonical_json(x));
|
|
}
|
|
s.push(']');
|
|
s
|
|
}
|
|
_ => serde_json::to_string(v).unwrap_or_default(),
|
|
}
|
|
}
|
|
|
|
/// Parse `raw` (a `flake.lock` JSON document) and return every group
|
|
/// of nodes whose `original` field is identical. Nodes without an
|
|
/// `original` (the synthetic `root`, or anomalous entries) are
|
|
/// skipped. Groups with only one member are filtered out — only true
|
|
/// duplicates surface.
|
|
///
|
|
/// Pure function, no I/O — covered by the unit tests below.
|
|
pub fn duplicate_groups(raw: &str) -> Result<Vec<DuplicateGroup>> {
|
|
let json: Value = serde_json::from_str(raw).context("parse flake.lock")?;
|
|
let Some(nodes) = json.get("nodes").and_then(|v| v.as_object()) else {
|
|
return Ok(Vec::new());
|
|
};
|
|
let mut groups: BTreeMap<String, DuplicateGroup> = BTreeMap::new();
|
|
for (name, node) in nodes {
|
|
let Some(original) = node.get("original") else {
|
|
continue;
|
|
};
|
|
let key = canonical_json(original);
|
|
let entry = groups.entry(key).or_insert_with(|| DuplicateGroup {
|
|
original: original.clone(),
|
|
keys: Vec::new(),
|
|
});
|
|
entry.keys.push(name.clone());
|
|
}
|
|
let mut dups: Vec<DuplicateGroup> = groups
|
|
.into_values()
|
|
.filter(|g| g.keys.len() > 1)
|
|
.collect();
|
|
for g in &mut dups {
|
|
g.keys.sort();
|
|
}
|
|
Ok(dups)
|
|
}
|
|
|
|
/// Re-derive the agent's `flake.lock` from its `flake.nix` (in a
|
|
/// throw-away worktree at the proposal tag) and reject the apply when
|
|
/// the result differs from what's committed — that means the manager
|
|
/// edited `flake.nix` but didn't commit the regenerated lock, so the
|
|
/// shipped state lies about what nix will actually fetch.
|
|
///
|
|
/// Plain `nix flake lock` (no `--update-input` flags) only fills in
|
|
/// MISSING entries; it never refreshes existing ones. So a lock that
|
|
/// matches its `flake.nix` round-trips to a no-op, and any diff is a
|
|
/// real "stale lock" signal.
|
|
///
|
|
/// Materialises the proposal tag into a temp worktree under
|
|
/// `std::env::temp_dir()` to avoid touching `applied/<n>/main` while
|
|
/// the check runs. Cleanup is unconditional via the inner-fn pattern.
|
|
///
|
|
/// Returns `Ok(())` when in sync (or there's no `flake.nix` at all);
|
|
/// `Err` with a human-readable message on stale lock or nix tooling
|
|
/// failure.
|
|
pub async fn check_lock_in_sync(repo: &Path, tag: &str, approval_id: i64) -> Result<()> {
|
|
let suffix = std::time::SystemTime::now()
|
|
.duration_since(std::time::UNIX_EPOCH)
|
|
.map(|d| d.as_nanos())
|
|
.unwrap_or(0);
|
|
let tmp_dir = std::env::temp_dir().join(format!("hive-flake-check-{approval_id}-{suffix}"));
|
|
|
|
// Detached worktree at the proposal tag — gives us a clean, mutable
|
|
// copy of the agent's tree without disturbing whatever's currently
|
|
// checked out on `applied/<n>/main`.
|
|
let out = git_command()
|
|
.current_dir(repo)
|
|
.args([
|
|
"worktree",
|
|
"add",
|
|
"--detach",
|
|
&tmp_dir.to_string_lossy(),
|
|
tag,
|
|
])
|
|
.output()
|
|
.await
|
|
.with_context(|| format!("git worktree add {} {tag}", tmp_dir.display()))?;
|
|
if !out.status.success() {
|
|
anyhow::bail!(
|
|
"git worktree add failed: {}",
|
|
String::from_utf8_lossy(&out.stderr).trim()
|
|
);
|
|
}
|
|
|
|
let result = lock_in_sync_inner(&tmp_dir).await;
|
|
|
|
// Best-effort cleanup. `git worktree remove --force` handles the
|
|
// common case; `remove_dir_all` mops up if git decided the worktree
|
|
// is half-gone (or if the inner work bailed before nix touched the
|
|
// tree). Failures here are logged, not propagated — the check's
|
|
// result is what matters.
|
|
if let Err(e) = remove_worktree(repo, &tmp_dir).await {
|
|
tracing::warn!(
|
|
worktree = %tmp_dir.display(),
|
|
error = %format!("{e:#}"),
|
|
"flake_check: temp worktree cleanup failed"
|
|
);
|
|
}
|
|
|
|
result
|
|
}
|
|
|
|
async fn lock_in_sync_inner(worktree: &Path) -> Result<()> {
|
|
// No `flake.nix` means there's nothing for nix to lock — skip the
|
|
// check cleanly (the dedup check will likewise no-op).
|
|
if !worktree.join("flake.nix").exists() {
|
|
return Ok(());
|
|
}
|
|
|
|
let committed = tokio::fs::read_to_string(worktree.join("flake.lock"))
|
|
.await
|
|
.ok();
|
|
|
|
// `--extra-experimental-features` mirrors `meta::nix` for hosts
|
|
// that haven't already enabled flakes in `nix.conf`. Plain
|
|
// `nix flake lock` (no `--update-input`) fills missing entries but
|
|
// never refreshes existing ones — exactly the semantics we want.
|
|
let out = Command::new("nix")
|
|
.current_dir(worktree)
|
|
.args([
|
|
"--extra-experimental-features",
|
|
"nix-command flakes",
|
|
"flake",
|
|
"lock",
|
|
])
|
|
.output()
|
|
.await
|
|
.with_context(|| format!("nix flake lock in {}", worktree.display()))?;
|
|
if !out.status.success() {
|
|
anyhow::bail!(
|
|
"nix flake lock failed: {}",
|
|
String::from_utf8_lossy(&out.stderr).trim()
|
|
);
|
|
}
|
|
|
|
let regenerated = tokio::fs::read_to_string(worktree.join("flake.lock"))
|
|
.await
|
|
.ok();
|
|
|
|
// An agent that declares inputs in flake.nix but ships no
|
|
// flake.lock at all hits this branch (committed = None,
|
|
// regenerated = Some(...)). That's a deliberate reject: every
|
|
// agent with inputs MUST commit its lock, otherwise meta's
|
|
// dedup pass has nothing to introspect and the broken state
|
|
// leaks downstream. Treated identically to a stale lock.
|
|
if committed.as_deref() != regenerated.as_deref() {
|
|
anyhow::bail!(
|
|
"flake.lock is out of sync with flake.nix — `nix flake lock` produces a different lock. \
|
|
Run `nix flake lock` in your agent config, commit the result, and re-submit request_apply_commit."
|
|
);
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
async fn remove_worktree(repo: &Path, worktree: &Path) -> Result<()> {
|
|
let out = git_command()
|
|
.current_dir(repo)
|
|
.args([
|
|
"worktree",
|
|
"remove",
|
|
"--force",
|
|
&worktree.to_string_lossy(),
|
|
])
|
|
.output()
|
|
.await
|
|
.with_context(|| format!("git worktree remove {}", worktree.display()))?;
|
|
if !out.status.success() {
|
|
// `git worktree remove` already errored — still try the raw
|
|
// rmdir so we don't leak the dir on disk. Surface the original
|
|
// git stderr for context.
|
|
let _ = tokio::fs::remove_dir_all(worktree).await;
|
|
anyhow::bail!(
|
|
"git worktree remove failed: {}",
|
|
String::from_utf8_lossy(&out.stderr).trim()
|
|
);
|
|
}
|
|
// git removed the worktree's metadata but the dir itself may
|
|
// linger on stripped-down git versions — best-effort clean.
|
|
let _ = tokio::fs::remove_dir_all(worktree).await;
|
|
Ok(())
|
|
}
|
|
|
|
/// Run the dedup check against the agent's freshly-applied tree.
|
|
///
|
|
/// `Ok(())` means either the commit doesn't carry a `flake.lock` (no
|
|
/// inputs declared) or every node has a unique `original`. `Err`
|
|
/// carries a multi-line message listing every duplicate group with
|
|
/// the offending node names, suitable for surfacing on the failed
|
|
/// approval row.
|
|
pub async fn check_no_duplicate_inputs(repo: &Path, tag: &str) -> Result<()> {
|
|
let Some(raw) = read_lock_at_tag(repo, tag).await? else {
|
|
return Ok(());
|
|
};
|
|
let dups = duplicate_groups(&raw)?;
|
|
if dups.is_empty() {
|
|
return Ok(());
|
|
}
|
|
let mut msg = String::from(
|
|
"flake.lock has duplicate flake inputs — add a `follows` directive in flake.nix to collapse them:\n",
|
|
);
|
|
for g in &dups {
|
|
let original = serde_json::to_string(&g.original).unwrap_or_else(|_| "?".into());
|
|
let _ = writeln!(msg, " - {original} → nodes [{}]", g.keys.join(", "));
|
|
}
|
|
anyhow::bail!("{}", msg.trim_end());
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
const CLEAN_LOCK: &str = r#"{
|
|
"nodes": {
|
|
"nixpkgs": {
|
|
"locked": {"rev": "aaa"},
|
|
"original": {"owner": "NixOS", "repo": "nixpkgs", "ref": "nixos-26.05", "type": "github"}
|
|
},
|
|
"nixpkgs-unstable": {
|
|
"locked": {"rev": "bbb"},
|
|
"original": {"owner": "NixOS", "repo": "nixpkgs", "ref": "nixpkgs-unstable", "type": "github"}
|
|
},
|
|
"root": {"inputs": {"nixpkgs": "nixpkgs"}}
|
|
},
|
|
"root": "root",
|
|
"version": 7
|
|
}"#;
|
|
|
|
const DUPLICATE_LOCK: &str = r#"{
|
|
"nodes": {
|
|
"nixpkgs": {
|
|
"locked": {"rev": "aaa"},
|
|
"original": {"owner": "NixOS", "repo": "nixpkgs", "ref": "nixos-26.05", "type": "github"}
|
|
},
|
|
"nixpkgs_2": {
|
|
"locked": {"rev": "ccc"},
|
|
"original": {"owner": "NixOS", "repo": "nixpkgs", "ref": "nixos-26.05", "type": "github"}
|
|
},
|
|
"nixpkgs_3": {
|
|
"locked": {"rev": "ddd"},
|
|
"original": {"owner": "NixOS", "repo": "nixpkgs", "ref": "nixos-26.05", "type": "github"}
|
|
},
|
|
"treefmt-nix": {
|
|
"locked": {"rev": "eee"},
|
|
"original": {"owner": "numtide", "repo": "treefmt-nix", "type": "github"}
|
|
},
|
|
"treefmt-nix_2": {
|
|
"locked": {"rev": "fff"},
|
|
"original": {"type": "github", "owner": "numtide", "repo": "treefmt-nix"}
|
|
},
|
|
"root": {"inputs": {"nixpkgs": "nixpkgs"}}
|
|
},
|
|
"root": "root",
|
|
"version": 7
|
|
}"#;
|
|
|
|
#[test]
|
|
fn clean_lock_has_no_duplicates() {
|
|
let dups = duplicate_groups(CLEAN_LOCK).expect("parse");
|
|
assert!(dups.is_empty(), "expected no dups, got {dups:#?}");
|
|
}
|
|
|
|
#[test]
|
|
fn duplicate_lock_reports_groups() {
|
|
let dups = duplicate_groups(DUPLICATE_LOCK).expect("parse");
|
|
assert_eq!(dups.len(), 2, "expected nixpkgs + treefmt-nix groups");
|
|
|
|
// Order is BTreeMap-stable: sorted by canonical_json key. The
|
|
// numtide treefmt-nix key sorts before the NixOS nixpkgs one
|
|
// because `numtide` < `NixOS` lexicographically (case-sensitive,
|
|
// capitals come first... wait — capital N is 0x4e, lowercase n
|
|
// is 0x6e, so capitals come first). So nixpkgs group sorts
|
|
// first. Verify by content instead of position to avoid coupling
|
|
// to that subtlety.
|
|
let nixpkgs_group = dups
|
|
.iter()
|
|
.find(|g| g.original.get("ref").is_some())
|
|
.expect("nixpkgs group present");
|
|
assert_eq!(
|
|
nixpkgs_group.keys,
|
|
vec!["nixpkgs", "nixpkgs_2", "nixpkgs_3"]
|
|
);
|
|
|
|
let treefmt_group = dups
|
|
.iter()
|
|
.find(|g| g.original.get("ref").is_none())
|
|
.expect("treefmt-nix group present");
|
|
assert_eq!(treefmt_group.keys, vec!["treefmt-nix", "treefmt-nix_2"]);
|
|
}
|
|
|
|
#[test]
|
|
fn key_order_in_original_does_not_matter() {
|
|
// The treefmt-nix and treefmt-nix_2 entries above use different
|
|
// key orderings for `original` ({owner,repo,type} vs
|
|
// {type,owner,repo}); duplicate_groups should still merge them.
|
|
let dups = duplicate_groups(DUPLICATE_LOCK).expect("parse");
|
|
let treefmt = dups
|
|
.iter()
|
|
.find(|g| g.keys.iter().any(|k| k == "treefmt-nix"))
|
|
.expect("treefmt-nix group");
|
|
assert!(treefmt.keys.contains(&"treefmt-nix_2".to_owned()));
|
|
}
|
|
|
|
#[test]
|
|
fn nodes_without_original_are_ignored() {
|
|
// The synthetic `root` node has no `original` and must not be
|
|
// grouped against anything.
|
|
let dups = duplicate_groups(CLEAN_LOCK).expect("parse");
|
|
assert!(dups.iter().all(|g| !g.keys.iter().any(|k| k == "root")));
|
|
}
|
|
|
|
#[test]
|
|
fn missing_nodes_object_is_not_an_error() {
|
|
// A lock file that's syntactically JSON but lacks `nodes` (e.g.
|
|
// a partial test fixture) should fail open — no dups reported.
|
|
let raw = r#"{"root": "root", "version": 7}"#;
|
|
let dups = duplicate_groups(raw).expect("parse");
|
|
assert!(dups.is_empty());
|
|
}
|
|
|
|
#[test]
|
|
fn invalid_json_is_a_hard_error() {
|
|
let err = duplicate_groups("not-json-at-all").unwrap_err();
|
|
assert!(format!("{err:#}").contains("parse flake.lock"));
|
|
}
|
|
}
|