From 25ff38d7c60cf79fae7af96f7965f4da73ddc449 Mon Sep 17 00:00:00 2001 From: Doll Date: Mon, 18 May 2026 07:44:14 -0500 Subject: [PATCH] fix(kickoff): store agent keys in main repo so they survive worktree cleanup (GH#610) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, `crosslink agent init` generated each kickoff agent's ed25519 keypair at `/.crosslink/keys/_ed25519` and then wrote a worktree-scoped `user.signingkey` into the hub-cache's `.git/worktrees/-hub-cache/config.worktree` pointing at that path. When `crosslink kickoff cleanup --force` later removed the agent worktree, the key file vanished but the worktree-scoped reference on the hub-cache survived — and every subsequent `crosslink sync` failed to commit hub state with "Couldn't load public key … No such file or directory". GH#565's `repair_stale_signingkey` catches the residual breakage at commit time, but doesn't address the architectural cause: the worktree-scoped config legitimately needs to outlive the worktree, so the file it points at must too. Fix is the one option from #610 that addresses the cause rather than the symptom (option A in the issue): relocate new agent keys to the *main repo's* `.crosslink/keys/` via a new `host_crosslink_dir()` helper that walks back from any worktree's crosslink dir to the main repo (no-op when already in the main repo). The hub-cache's worktree-scoped `user.signingkey` then references a stable main-repo path that survives `git worktree remove`. A new `resolve_agent_key()` helper in `sync::trust` resolves `agent.json.ssh_key_path` against the host crosslink dir first and falls back to the worktree-local path so legacy agents created before this change keep signing until their worktree is cleaned up. Co-Authored-By: Claude Opus 4.7 (1M context) --- crosslink/src/commands/agent.rs | 26 +++++- crosslink/src/commands/init/mod.rs | 7 +- crosslink/src/identity.rs | 11 ++- crosslink/src/signing.rs | 130 +++++++++++++++++++++++++++++ crosslink/src/sync/tests.rs | 113 +++++++++++++++++++++++++ crosslink/src/sync/trust.rs | 22 ++++- 6 files changed, 301 insertions(+), 8 deletions(-) diff --git a/crosslink/src/commands/agent.rs b/crosslink/src/commands/agent.rs index eaf99bff7..5a82ce361 100644 --- a/crosslink/src/commands/agent.rs +++ b/crosslink/src/commands/agent.rs @@ -345,7 +345,12 @@ pub fn init( // Generate SSH key unless opted out if !no_key { - let keys_dir = crosslink_dir.join("keys"); + // GH#610: store keys under the main repo's `.crosslink/keys/` so + // they outlive `git worktree remove` of an agent worktree. The + // worktree-scoped `user.signingkey` on the hub-cache (which is + // also rooted in the main repo) then references a stable path. + let host_crosslink = signing::host_crosslink_dir(crosslink_dir); + let keys_dir = host_crosslink.join("keys"); match signing::generate_agent_key(&keys_dir, agent_id, &config.machine_id) { Ok(keypair) => { // Store relative path from .crosslink/ @@ -382,9 +387,20 @@ pub fn init( // On --force re-runs, key-gen errors with "already exists". // Reuse the on-disk key; otherwise agent.json loses // ssh_key_path and shared_writer silently disables signing. + // GH#610: prefer the host-side keys dir (where new agents + // store their keys) and fall back to the worktree-local + // path for legacy agents whose keys predate the relocation. let rel_path = format!("keys/{agent_id}_ed25519"); - let private_path = crosslink_dir.join(&rel_path); - let public_path = crosslink_dir.join(format!("keys/{agent_id}_ed25519.pub")); + let host_private = host_crosslink.join(&rel_path); + let host_public = host_crosslink.join(format!("keys/{agent_id}_ed25519.pub")); + let (private_path, public_path) = if host_private.exists() && host_public.exists() { + (host_private, host_public) + } else { + ( + crosslink_dir.join(&rel_path), + crosslink_dir.join(format!("keys/{agent_id}_ed25519.pub")), + ) + }; if private_path.exists() && public_path.exists() { if let (Ok(fp), Ok(pub_key)) = ( signing::get_key_fingerprint(&public_path), @@ -602,7 +618,9 @@ pub fn bootstrap( // Step 5: Generate SSH key unless opted out if !no_key && config.ssh_key_path.is_none() { - let keys_dir = crosslink_dir.join("keys"); + // GH#610: see `init` above — keys live under the main repo's + // `.crosslink/keys/` so they survive agent-worktree cleanup. + let keys_dir = signing::host_crosslink_dir(&crosslink_dir).join("keys"); match signing::generate_agent_key(&keys_dir, agent_id, &config.machine_id) { Ok(keypair) => { let rel_path = format!("keys/{agent_id}_ed25519"); diff --git a/crosslink/src/commands/init/mod.rs b/crosslink/src/commands/init/mod.rs index 1d2a2cf86..6ab46b57e 100644 --- a/crosslink/src/commands/init/mod.rs +++ b/crosslink/src/commands/init/mod.rs @@ -284,7 +284,12 @@ fn init_agent_identity(crosslink_dir: &Path, agent_id: &str) -> Result<()> { // AgentConfig::init defaults to role=driver. let mut config = crate::identity::AgentConfig::init(crosslink_dir, agent_id, None)?; - let keys_dir = crosslink_dir.join("keys"); + // GH#610: route through `host_crosslink_dir` for consistency with + // agent init/bootstrap. `crosslink init` always runs in the main + // repo, so this is a no-op in practice — but if any future + // refactor moves this into a worktree context, key storage stays + // pinned to the main repo and survives `git worktree remove`. + let keys_dir = crate::signing::host_crosslink_dir(crosslink_dir).join("keys"); match crate::signing::generate_agent_key(&keys_dir, agent_id, &config.machine_id) { Ok(keypair) => { config.ssh_key_path = Some(format!("keys/{agent_id}_ed25519")); diff --git a/crosslink/src/identity.rs b/crosslink/src/identity.rs index fa1ec91ac..d04e50ffb 100644 --- a/crosslink/src/identity.rs +++ b/crosslink/src/identity.rs @@ -36,7 +36,16 @@ pub struct AgentConfig { /// autonomous agent worktrees. Missing field defaults to `driver`. #[serde(default)] pub role: AgentRole, - /// Path to SSH private key, relative to .crosslink/ (e.g. "`keys/agent_ed25519`"). + /// Path to SSH private key, relative to the **main repo's** + /// `.crosslink/` (e.g. "`keys/agent_ed25519`"). + /// + /// GH#610: new agents store keys under the main repo's + /// `.crosslink/keys/` so they survive `git worktree remove` of a + /// kickoff agent worktree. Legacy agents (pre-#610) wrote this + /// path relative to the worktree's own `.crosslink/`; the + /// resolver in `sync::trust::resolve_agent_key` tries the host + /// path first and falls back to the worktree path for legacy + /// keys. #[serde(default, skip_serializing_if = "Option::is_none")] pub ssh_key_path: Option, /// SSH public key fingerprint (e.g. "SHA256:..."). diff --git a/crosslink/src/signing.rs b/crosslink/src/signing.rs index 4c2a6c7c6..437bd5dbb 100644 --- a/crosslink/src/signing.rs +++ b/crosslink/src/signing.rs @@ -42,6 +42,37 @@ pub enum SignatureVerification { // ── Key generation ────────────────────────────────────────────────── +/// Resolve the *host* (main-repo) crosslink directory for storing +/// agent signing keys. +/// +/// Agent keys live under the main repo's `.crosslink/keys/` rather +/// than each agent worktree's own `.crosslink/keys/` so that the +/// key file survives `git worktree remove` when a kickoff agent is +/// cleaned up (GH#610). Without this, the worktree-scoped +/// `user.signingkey` reference on the hub-cache outlives the file +/// it points at, breaking every subsequent `crosslink sync` until +/// a human re-points it (see also GH#565 for the C-path repair +/// that catches the residual breakage). +/// +/// Behaviour: +/// +/// - When `crosslink_dir` is inside a git worktree, walks up via +/// [`crate::utils::resolve_main_repo_root`] and returns +/// `/.crosslink`. +/// - When `crosslink_dir` is already the main repo's crosslink dir +/// (the driver case), returns it unchanged. +/// - When git can't resolve a repo (e.g. unit-test temp dirs with +/// no `git init`), returns `crosslink_dir` unchanged so callers +/// continue to behave as if there were no worktree. +#[must_use] +pub fn host_crosslink_dir(crosslink_dir: &Path) -> PathBuf { + let worktree_root = crosslink_dir.parent().unwrap_or(crosslink_dir); + crate::utils::resolve_main_repo_root(worktree_root).map_or_else( + || crosslink_dir.to_path_buf(), + |root| root.join(".crosslink"), + ) +} + /// Generate a new Ed25519 SSH key pair for an agent. /// /// Keys are stored at `{keys_dir}/{agent_id}_ed25519` (.pub for public). @@ -1127,6 +1158,105 @@ mod tests { assert!(read_public_key(&path).is_err()); } + // ── host_crosslink_dir (GH#610) ────────────────────────────────── + + /// In a temp dir with no git metadata, `resolve_main_repo_root` + /// returns `None`; `host_crosslink_dir` falls back to the input. + #[test] + fn test_host_crosslink_dir_no_git_returns_input() { + let dir = tempdir().unwrap(); + let crosslink_dir = dir.path().join(".crosslink"); + std::fs::create_dir_all(&crosslink_dir).unwrap(); + let resolved = host_crosslink_dir(&crosslink_dir); + assert_eq!(resolved, crosslink_dir); + } + + /// In a plain (non-worktree) git repo, `host_crosslink_dir` + /// returns the same `.crosslink/` it was given. + #[test] + fn test_host_crosslink_dir_main_repo_is_identity() { + let dir = tempdir().unwrap(); + Command::new("git") + .current_dir(dir.path()) + .args(["init", "-b", "main"]) + .output() + .unwrap(); + let crosslink_dir = dir.path().join(".crosslink"); + std::fs::create_dir_all(&crosslink_dir).unwrap(); + let resolved = host_crosslink_dir(&crosslink_dir); + // Canonicalize both sides — macOS resolves /tmp through /private/tmp, + // and `resolve_main_repo_root` canonicalises internally. + let expected = crosslink_dir.canonicalize().unwrap_or(crosslink_dir); + let actual = resolved.canonicalize().unwrap_or(resolved); + assert_eq!(actual, expected); + } + + /// In a linked git worktree, `host_crosslink_dir` walks back up to + /// the main repo's `.crosslink/` (the whole point of the helper). + #[test] + fn test_host_crosslink_dir_worktree_resolves_to_main() { + let main = tempdir().unwrap(); + let main_root = main.path(); + + // Build a main repo with one commit so `git worktree add` has + // something to branch from. + Command::new("git") + .current_dir(main_root) + .args(["init", "-b", "main"]) + .output() + .unwrap(); + Command::new("git") + .current_dir(main_root) + .args(["config", "user.email", "t@t.local"]) + .output() + .unwrap(); + Command::new("git") + .current_dir(main_root) + .args(["config", "user.name", "t"]) + .output() + .unwrap(); + std::fs::write(main_root.join("a"), "x").unwrap(); + Command::new("git") + .current_dir(main_root) + .args(["add", "."]) + .output() + .unwrap(); + Command::new("git") + .current_dir(main_root) + .args(["commit", "-m", "init", "--no-gpg-sign"]) + .output() + .unwrap(); + + // The main repo's crosslink dir — the answer we expect. + let main_crosslink = main_root.join(".crosslink"); + std::fs::create_dir_all(&main_crosslink).unwrap(); + + // Add a linked worktree under the main repo on a fresh branch. + let wt_root = main_root.join(".worktrees").join("agent-abcd"); + Command::new("git") + .current_dir(main_root) + .args([ + "worktree", + "add", + "-b", + "agent-test", + wt_root.to_str().unwrap(), + ]) + .output() + .unwrap(); + let wt_crosslink = wt_root.join(".crosslink"); + std::fs::create_dir_all(&wt_crosslink).unwrap(); + + let resolved = host_crosslink_dir(&wt_crosslink); + // Canonicalise so /tmp ↔ /private/tmp on macOS doesn't trip us. + let expected = main_crosslink.canonicalize().unwrap_or(main_crosslink); + let actual = resolved.canonicalize().unwrap_or(resolved); + assert_eq!( + actual, expected, + "worktree's host_crosslink_dir should resolve to the main repo's .crosslink/" + ); + } + // Integration tests requiring ssh-keygen on PATH #[test] fn test_generate_agent_key() { diff --git a/crosslink/src/sync/tests.rs b/crosslink/src/sync/tests.rs index 58a7cd650..35d8db12b 100644 --- a/crosslink/src/sync/tests.rs +++ b/crosslink/src/sync/tests.rs @@ -3570,6 +3570,119 @@ fn test_repair_stale_signingkey_cache_missing_is_noop() { assert!(!repaired); } +// ------------------------------------------------------------------ +// resolve_agent_key — GH#610: agent keys live in main repo, with +// a legacy fallback to the worktree-local path. +// ------------------------------------------------------------------ + +/// When no git metadata is present, `host_crosslink_dir` returns the input +/// unchanged. With the key only at the worktree-local path, the resolver +/// falls back to it — i.e. the legacy pre-#610 layout still works. +#[test] +fn test_resolve_agent_key_legacy_worktree_path_fallback() { + let dir = tempdir().unwrap(); + let wt_crosslink = dir.path().join(".crosslink"); + let keys_dir = wt_crosslink.join("keys"); + std::fs::create_dir_all(&keys_dir).unwrap(); + let key_file = keys_dir.join("legacy_ed25519"); + std::fs::write(&key_file, "legacy-key-bytes").unwrap(); + + let resolved = crate::sync::trust::resolve_agent_key(&wt_crosslink, "keys/legacy_ed25519"); + assert_eq!( + resolved, key_file, + "no host-side key + worktree-local key → resolver picks the worktree path" + ); +} + +/// When neither the host-side nor the worktree-local path has the file, +/// the resolver returns the worktree-relative path. The caller is then +/// responsible for checking `.exists()` — preserves the pre-fix +/// "key missing → caller disables signing" contract. +#[test] +fn test_resolve_agent_key_neither_path_exists_returns_worktree_path() { + let dir = tempdir().unwrap(); + let wt_crosslink = dir.path().join(".crosslink"); + std::fs::create_dir_all(&wt_crosslink).unwrap(); + + let resolved = crate::sync::trust::resolve_agent_key(&wt_crosslink, "keys/ghost_ed25519"); + assert_eq!(resolved, wt_crosslink.join("keys/ghost_ed25519")); + assert!(!resolved.exists(), "precondition for this test"); +} + +/// End-to-end #610: with a real main repo and a linked worktree, an +/// agent.json living in the worktree resolves `ssh_key_path` to the +/// main repo's `.crosslink/keys/`. This is the contract that lets +/// `configure_signing` (called from the worktree's context) bake an +/// absolute main-repo path into the hub-cache's worktree-scoped +/// `user.signingkey`, so the reference keeps pointing at a real file +/// after `git worktree remove` of the agent worktree. +#[test] +fn test_resolve_agent_key_worktree_picks_main_repo_keys() { + let main = tempdir().unwrap(); + let main_root = main.path(); + + // Bootstrap a real git repo so `resolve_main_repo_root` succeeds. + for args in [ + vec!["init", "-b", "main"], + vec!["config", "user.email", "t@t.local"], + vec!["config", "user.name", "t"], + ] { + Command::new("git") + .current_dir(main_root) + .args(&args) + .output() + .unwrap(); + } + std::fs::write(main_root.join("README.md"), "x").unwrap(); + Command::new("git") + .current_dir(main_root) + .args(["add", "."]) + .output() + .unwrap(); + Command::new("git") + .current_dir(main_root) + .args(["commit", "-m", "init", "--no-gpg-sign"]) + .output() + .unwrap(); + + // The post-#610 key lives in the MAIN repo's .crosslink/keys/. + let main_crosslink = main_root.join(".crosslink"); + let main_keys = main_crosslink.join("keys"); + std::fs::create_dir_all(&main_keys).unwrap(); + let main_key = main_keys.join("agent_ed25519"); + std::fs::write(&main_key, "real-key-bytes").unwrap(); + + // Linked worktree the agent would have run in. agent.json lives + // here and references `keys/agent_ed25519` — relative to *main* + // per the post-#610 semantics. The worktree's own .crosslink + // intentionally has no `keys/` directory. + let wt_root = main_root.join(".worktrees").join("agent-test"); + Command::new("git") + .current_dir(main_root) + .args([ + "worktree", + "add", + "-b", + "agent-test", + wt_root.to_str().unwrap(), + ]) + .output() + .unwrap(); + let wt_crosslink = wt_root.join(".crosslink"); + std::fs::create_dir_all(&wt_crosslink).unwrap(); + + let resolved = crate::sync::trust::resolve_agent_key(&wt_crosslink, "keys/agent_ed25519"); + let expected = main_key.canonicalize().unwrap_or(main_key); + let actual = resolved.canonicalize().unwrap_or(resolved); + assert_eq!( + actual, expected, + "with a main-repo key present, the resolver prefers it over the worktree path \ + — this is what lets `configure_signing` bake an absolute main-repo path into \ + the hub-cache's worktree-scoped `user.signingkey` so the reference survives \ + `git worktree remove` of the agent worktree (GH#610)" + ); +} + // ============================================================================ // GH#586: structured push-failure classifier // ============================================================================ diff --git a/crosslink/src/sync/trust.rs b/crosslink/src/sync/trust.rs index 8a193366c..b0b33b345 100644 --- a/crosslink/src/sync/trust.rs +++ b/crosslink/src/sync/trust.rs @@ -45,6 +45,24 @@ fn expand_tilde(path: &str) -> PathBuf { PathBuf::from(path) } +/// Resolve an agent's SSH private key path from the relative form stored in +/// `agent.json` (`ssh_key_path`). +/// +/// GH#610: new agents store their key under the *main repo's* +/// `.crosslink/keys/` so it survives `git worktree remove` of a kickoff +/// agent worktree. Legacy agents (created before that change) have their +/// key inside the worktree's own `.crosslink/keys/`. Try the host path +/// first; fall back to the worktree-local path so existing deployments +/// keep signing until the legacy worktree is cleaned up. +pub(super) fn resolve_agent_key(worktree_crosslink_dir: &Path, rel_key: &str) -> PathBuf { + let host = crate::signing::host_crosslink_dir(worktree_crosslink_dir); + let host_path = host.join(rel_key); + if host_path.exists() { + return host_path; + } + worktree_crosslink_dir.join(rel_key) +} + /// Best guess whether a `user.signingkey` value is a filesystem path rather /// than literal key material (e.g. an inline `ssh-ed25519 AAAA...` line). /// @@ -115,7 +133,7 @@ impl SyncManager { // attribution is distinct. if let Some(agent) = AgentConfig::load(crosslink_dir)? { if let (Some(rel_key), Some(_)) = (&agent.ssh_key_path, &agent.ssh_fingerprint) { - let private_key = self.crosslink_dir.join(rel_key); + let private_key = resolve_agent_key(&self.crosslink_dir, rel_key); if private_key.exists() { signing::configure_git_ssh_signing( &self.cache_dir, @@ -165,7 +183,7 @@ impl SyncManager { // hasn't set `user.signingkey` in the main repo. if let Some(agent) = AgentConfig::load(crosslink_dir)? { if let (Some(rel_key), Some(_)) = (&agent.ssh_key_path, &agent.ssh_fingerprint) { - let private_key = self.crosslink_dir.join(rel_key); + let private_key = resolve_agent_key(&self.crosslink_dir, rel_key); if private_key.exists() { signing::configure_git_ssh_signing( &self.cache_dir,