diff --git a/CHANGELOG.md b/CHANGELOG.md index eca04ae..9f6aae0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.3.0] - 2026-08-25 + +**Scope.** This release promotes `v1.2.0..c095d5d`: 6 commits and 3 first-parent +units, measured with `git rev-list --count` and `git rev-list --first-parent --count` +over that exact range. All three units change the published Rust CLI. + +### Added +- **Codex startup prompts use the native developer boundary.** `gr spawn` loads each + configured `startup_prompt` and supplies it as `developer_instructions`, while recall + continuity remains a separate optional user payload. Missing configured prompts fail + closed. Generated launch scripts are constructed on mode-0600 inodes and atomically + replace prior files. + +### Fixed +- **Unknown repository filters are refused** by `gr add`, `gr commit`, and `gr push` + instead of matching zero repositories and reporting success. +- **Pruning protects the remote default branch** in addition to the current and target + branches. + ## [1.2.0] - 2026-08-21 **Scope, stated first because the range and the artifact are not the same thing.** diff --git a/Cargo.lock b/Cargo.lock index ad0c16a..911eb4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -814,7 +814,7 @@ dependencies = [ [[package]] name = "gitgrip" -version = "1.2.0" +version = "1.3.0" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index ff97c9e..f49cf92 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "gitgrip" -version = "1.2.0" +version = "1.3.0" edition = "2021" rust-version = "1.80" description = "Multi-repo workflow tool - manage multiple git repositories as one" @@ -99,11 +99,11 @@ urlencoding = "2" base64 = "0.22" futures = "0.3" which = "7" +tempfile = "3" clap_complete = "4.5.65" rusqlite = { version = "0.39.0", features = ["bundled"] } [dev-dependencies] -tempfile = "3" tokio-test = "0.4" wiremock = "0.6" assert_cmd = "2" diff --git a/docs/gr-spawn.md b/docs/gr-spawn.md index a2e2c7d..36a6167 100644 --- a/docs/gr-spawn.md +++ b/docs/gr-spawn.md @@ -66,7 +66,12 @@ Each agent can have a startup prompt file (Markdown) that defines its role, resp reviewer.md ``` -Prompts are injected via the `--prompt` flag when launching the agent's CLI tool. They should include: +Startup prompt delivery follows the target runtime's native instruction boundary. For +Codex, gitgrip passes the file as `developer_instructions`, keeping identity separate +from the first user turn. Other tools can consume the same file through their configured +launch arguments, such as Claude Code's `--append-system-prompt-file`. + +Prompts should include: - Role description - Responsibilities - Startup checklist (join channel, read journal, check unread, start loop) @@ -140,7 +145,7 @@ tmux select-window -t myproject:lead |-------|---------|-------------| | `session_name` | `"synapt"` | tmux session name | | `channel` | `"dev"` | Default channel all agents join | -| `auto_journal` | `true` | Agents read `recall_journal` on startup | +| `auto_journal` | `true` | Paste recall startup context into Codex as a compatibility fallback. Disable when a runtime SessionStart hook owns continuity. | | `mock_launch` | `false` | Use echo/sleep instead of real agent launch | ### [agents.*] section @@ -151,7 +156,7 @@ tmux select-window -t myproject:lead | `model` | no | `claude-sonnet-4-6` | Model ID | | `tool` | no | `claude` | CLI tool (`claude`, `codex`, `cursor`) | | `worktree` | no | `main` | Git worktree or `"new"` to auto-create | -| `startup_prompt` | no | — | Path to .md startup prompt file | +| `startup_prompt` | no | — | Path to an agent identity prompt. Codex receives it as developer instructions. | | `channel` | no | from `[spawn]` | Channel to auto-join | | `loop_interval` | no | `5m` | Channel read loop cadence | | `heartbeat_interval` | no | `60` | Seconds between heartbeat pings | @@ -174,4 +179,3 @@ tmux select-window -t myproject:lead | `SYNAPT_LOOP_INTERVAL` | `loop_interval` field | `2m` | Plus any custom vars from the `env` table in `agents.toml`. - diff --git a/src/cli/commands/add.rs b/src/cli/commands/add.rs index b05be19..d7649df 100644 --- a/src/cli/commands/add.rs +++ b/src/cli/commands/add.rs @@ -1,9 +1,10 @@ //! Add command implementation +use crate::cli::outcome::CliOutcomeError; use crate::cli::output::Output; use crate::core::manifest::Manifest; use crate::core::manifest_paths; -use crate::core::repo::{filter_repos, RepoInfo}; +use crate::core::repo::{filter_repos, validate_repo_filters_known, RepoInfo}; use crate::git::cache::invalidate_status_cache; use crate::git::{get_workdir, open_repo, path_exists}; use crate::util::log_cmd; @@ -19,6 +20,9 @@ pub fn run_add( repos_filter: Option<&[String]>, group_filter: Option<&[String]>, ) -> anyhow::Result<()> { + validate_repo_filters_known(manifest, repos_filter) + .map_err(|error| CliOutcomeError::refusal(error.to_string()))?; + Output::header("Checking repositories for changes to stage..."); println!(); diff --git a/src/cli/commands/commit.rs b/src/cli/commands/commit.rs index 20ad923..88f86b7 100644 --- a/src/cli/commands/commit.rs +++ b/src/cli/commands/commit.rs @@ -1,9 +1,10 @@ //! Commit command implementation +use crate::cli::outcome::CliOutcomeError; use crate::cli::output::Output; use crate::core::manifest::Manifest; use crate::core::manifest_paths; -use crate::core::repo::{filter_repos, RepoInfo}; +use crate::core::repo::{filter_repos, validate_repo_filters_known, RepoInfo}; use crate::git::cache::invalidate_status_cache; use crate::git::{get_workdir, open_repo, path_exists}; use crate::util::log_cmd; @@ -22,6 +23,9 @@ pub fn run_commit( repos_filter: Option<&[String]>, group_filter: Option<&[String]>, ) -> anyhow::Result<()> { + validate_repo_filters_known(manifest, repos_filter) + .map_err(|error| CliOutcomeError::refusal(error.to_string()))?; + if !json { Output::header("Committing changes..."); println!(); diff --git a/src/cli/commands/prune.rs b/src/cli/commands/prune.rs index 773d0e5..4d11834 100644 --- a/src/cli/commands/prune.rs +++ b/src/cli/commands/prune.rs @@ -1,14 +1,15 @@ //! Prune command implementation //! -//! Deletes local branches that have been merged into the default branch. +//! Deletes local branches that have been merged into the manifest target. //! Optionally prunes remote tracking refs. use crate::cli::output::Output; use crate::core::manifest::Manifest; use crate::core::repo::{filter_repos, RepoInfo}; use crate::git::branch::{delete_local_branch, is_branch_merged, list_local_branches}; -use crate::git::{get_current_branch, open_repo, path_exists}; +use crate::git::{get_current_branch, get_default_branch, open_repo, path_exists}; use crate::util::log_cmd; +use std::collections::BTreeSet; use std::path::Path; use std::process::Command; @@ -63,11 +64,37 @@ pub fn run_prune( } }; + // Branches that must never be pruned. Three members, not two: the branch + // checked out, the manifest target, and the remote's DEFAULT branch. The + // default is the only one that asserts "permanent" rather than "currently + // interesting" -- and until the target moved off main it was protected + // only by coincidence, because target and default were the same value. + let mut protected: BTreeSet = BTreeSet::new(); + protected.insert(current_branch.clone()); + protected.insert(repo.target_branch().to_string()); + match get_default_branch(&git_repo, &repo.sync_remote) { + Some(default_branch) => { + protected.insert(default_branch); + } + None => { + // Unknown default: protect MORE, never less. A resolution failure + // that silently shrinks this set reproduces the exact defect the + // set exists to prevent, one level up, inside its own fix -- and + // it would fail with nothing going red. Say so out loud, because + // a protected-more that is silent still reads as "resolved fine". + protected.insert("main".to_string()); + protected.insert("dev".to_string()); + Output::warning(&format!( + "{}: could not determine the default branch (no {}/HEAD); protecting 'main' and 'dev' by name", + repo.name, repo.sync_remote + )); + } + } + let mut merged_branches: Vec = Vec::new(); for branch in &branches { - // Skip current branch and default branch - if branch == ¤t_branch || branch == repo.target_branch() { + if protected.contains(branch) { continue; } diff --git a/src/cli/commands/push.rs b/src/cli/commands/push.rs index adf0e45..84ec0ce 100644 --- a/src/cli/commands/push.rs +++ b/src/cli/commands/push.rs @@ -1,8 +1,11 @@ //! Push command implementation +use crate::cli::outcome::CliOutcomeError; use crate::cli::output::Output; use crate::core::manifest::Manifest; -use crate::core::repo::{filter_repos, get_manifest_repo_info, RepoInfo}; +use crate::core::repo::{ + filter_repos, get_manifest_repo_info, validate_repo_filters_known, RepoInfo, +}; use crate::git::remote::{force_push_branch, push_branch}; use crate::git::{get_current_branch, open_repo, path_exists}; use git2::Repository; @@ -27,6 +30,9 @@ pub fn run_push( repos_filter: Option<&[String]>, group_filter: Option<&[String]>, ) -> anyhow::Result<()> { + validate_repo_filters_known(manifest, repos_filter) + .map_err(|error| CliOutcomeError::refusal(error.to_string()))?; + if !json { if force { Output::header("Force pushing changes..."); diff --git a/src/cli/commands/spawn.rs b/src/cli/commands/spawn.rs index 79cc501..bdbd28d 100644 --- a/src/cli/commands/spawn.rs +++ b/src/cli/commands/spawn.rs @@ -272,7 +272,17 @@ fn write_launch_script( let safe_name = agent_name.replace(['/', '\\', ':'], "-"); let script_path = script_dir.join(format!("{}-launch.sh", safe_name)); let content = build_launch_script_content(env, worktree_path, launch_cmd); - std::fs::write(&script_path, content)?; + + let mut file = tempfile::NamedTempFile::new_in(&script_dir)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + file.as_file() + .set_permissions(std::fs::Permissions::from_mode(0o600))?; + } + file.write_all(content.as_bytes())?; + file.as_file().sync_all()?; + file.persist(&script_path).map_err(|e| e.error)?; Ok(script_path) } @@ -523,6 +533,13 @@ pub fn run_spawn_up( launch_env.extend(agent.env.clone()); let worktree_path = resolve_worktree_path(&workspace_root, &agent.worktree); + let codex_startup_prompt = if !mock_mode && agent.tool == "codex" { + read_agent_startup_prompt(&workspace_root, agent).map_err(|e| { + anyhow::anyhow!("failed to load Codex startup prompt for {}: {}", name, e) + })? + } else { + String::new() + }; // Build and send launch command let (launch_cmd, expected_process) = if mock_mode { @@ -608,6 +625,7 @@ pub fn run_spawn_up( parts.extend(resolved_defaults.iter().cloned()); parts.extend(model_inject.iter().cloned()); parts.extend(resolved_args.iter().cloned()); + parts.extend(codex_developer_instruction_args(&codex_startup_prompt)); (shell_join(&parts), Some(expected_process_name(binary))) }; @@ -662,16 +680,6 @@ pub fn run_spawn_up( } if !mock_mode && agent.tool == "codex" { - let startup_prompt = match read_agent_startup_prompt(&workspace_root, agent) { - Ok(prompt) => prompt, - Err(e) => { - Output::warning(&format!( - "Failed to read Codex startup prompt for {}: {}", - name, e - )); - String::new() - } - }; let recall_context = if config.spawn.auto_journal { generate_synapt_startup_context( &worktree_path, @@ -687,8 +695,7 @@ pub fn run_spawn_up( } else { String::new() }; - if let Some(prompt) = build_codex_initial_prompt(name, &startup_prompt, &recall_context) - { + if let Some(prompt) = build_codex_initial_prompt(name, &recall_context) { if let Err(e) = send_codex_initial_prompt(&target, &prompt) { Output::warning(&format!( "Failed to inject Codex startup context for {}: {}", @@ -765,6 +772,18 @@ fn read_agent_startup_prompt(workspace_root: &Path, agent: &AgentConfig) -> anyh .map_err(|e| anyhow::anyhow!("failed to read {}: {}", path.display(), e)) } +fn codex_developer_instruction_args(startup_prompt: &str) -> Vec { + if startup_prompt.trim().is_empty() { + return Vec::new(); + } + + let value = toml::Value::String(startup_prompt.to_string()); + vec![ + "--config".to_string(), + format!("developer_instructions={value}"), + ] +} + fn generate_synapt_startup_context( worktree_path: &Path, agent_name: &str, @@ -802,14 +821,9 @@ fn generate_synapt_startup_context( } } -fn build_codex_initial_prompt( - agent_name: &str, - startup_prompt: &str, - recall_context: &str, -) -> Option { - let startup_prompt = startup_prompt.trim(); +fn build_codex_initial_prompt(agent_name: &str, recall_context: &str) -> Option { let recall_context = recall_context.trim(); - if startup_prompt.is_empty() && recall_context.is_empty() { + if recall_context.is_empty() { return None; } @@ -817,19 +831,11 @@ fn build_codex_initial_prompt( "Load this startup context for agent `{}` before doing any work.", agent_name )]; - if !startup_prompt.is_empty() { - sections.push(format!( - "\n{}\n", - startup_prompt - )); - } - if !recall_context.is_empty() { - let recall_context = wrap_long_lines(recall_context, CODEX_STARTUP_MAX_LINE_CHARS); - sections.push(format!( - "\n{}\n", - recall_context - )); - } + let recall_context = wrap_long_lines(recall_context, CODEX_STARTUP_MAX_LINE_CHARS); + sections.push(format!( + "\n{}\n", + recall_context + )); sections.push( "Use this context to choose the next action. Do not summarize it unless asked.".to_string(), ); @@ -863,7 +869,7 @@ fn wrap_long_lines(text: &str, max_chars: usize) -> String { wrapped.join("\n") } -fn send_codex_initial_prompt(target: &str, prompt: &str) -> anyhow::Result<()> { +fn tmux_load_buffer(prompt: &str) -> anyhow::Result<()> { let mut child = Command::new("tmux") .args(["load-buffer", "-"]) .stdin(Stdio::piped()) @@ -881,31 +887,52 @@ fn send_codex_initial_prompt(target: &str, prompt: &str) -> anyhow::Result<()> { anyhow::bail!("tmux load-buffer exited with {}", status); } - let status = Command::new("tmux") - .args(["paste-buffer", "-d", "-t", target]) - .status()?; - if !status.success() { - anyhow::bail!("tmux paste-buffer exited with {}", status); - } + Ok(()) +} - let status = Command::new("tmux") - .args(["send-keys", "-t", target, "Enter"]) - .status()?; +fn tmux_run(args: &[&str]) -> anyhow::Result<()> { + let status = Command::new("tmux").args(args).status()?; if !status.success() { - anyhow::bail!("tmux send-keys Enter exited with {}", status); + anyhow::bail!("tmux {} exited with {}", args.join(" "), status); } + Ok(()) +} - std::thread::sleep(std::time::Duration::from_millis(300)); - let status = Command::new("tmux") - .args(["send-keys", "-t", target, "Enter"]) - .status()?; - if !status.success() { - anyhow::bail!("tmux send-keys confirm Enter exited with {}", status); - } +fn send_codex_initial_prompt_with( + target: &str, + prompt: &str, + mut load_buffer: L, + mut run_tmux: R, + mut sleep: S, +) -> anyhow::Result<()> +where + L: FnMut(&str) -> anyhow::Result<()>, + R: FnMut(&[&str]) -> anyhow::Result<()>, + S: FnMut(std::time::Duration), +{ + load_buffer(prompt)?; + run_tmux(&["paste-buffer", "-d", "-t", target])?; + run_tmux(&["send-keys", "-t", target, "Enter"])?; + + sleep(std::time::Duration::from_millis(300)); + run_tmux(&["send-keys", "-t", target, "Enter"])?; + + sleep(std::time::Duration::from_millis(300)); + run_tmux(&["send-keys", "-t", target, "Enter"])?; Ok(()) } +fn send_codex_initial_prompt(target: &str, prompt: &str) -> anyhow::Result<()> { + send_codex_initial_prompt_with( + target, + prompt, + tmux_load_buffer, + tmux_run, + std::thread::sleep, + ) +} + /// Resolve a worktree identifier to an absolute path. fn resolve_worktree_path(workspace_root: &Path, worktree: &str) -> PathBuf { if worktree == "main" { @@ -1806,18 +1833,87 @@ mod tests { assert!(script.contains("exec 'codex' 'resume'")); } + #[cfg(unix)] #[test] - fn test_codex_initial_prompt_combines_agent_prompt_and_recall_context() { - let prompt = build_codex_initial_prompt( - "opus", - "You are Opus.", - "Last session: shipped recall startup injection.", + fn test_launch_script_is_owner_only() { + use std::io::Read; + use std::os::unix::fs::PermissionsExt; + + let workspace = tempfile::tempdir().unwrap(); + let script_dir = workspace.path().join(".gitgrip/spawn"); + std::fs::create_dir_all(&script_dir).unwrap(); + let existing_script = script_dir.join("sentinel-launch.sh"); + std::fs::write(&existing_script, "old public content").unwrap(); + std::fs::set_permissions(&existing_script, std::fs::Permissions::from_mode(0o644)).unwrap(); + let mut old_reader = std::fs::File::open(&existing_script).unwrap(); + + let script_path = write_launch_script( + workspace.path(), + "sentinel", + &HashMap::new(), + Path::new("/tmp/worktree"), + "'codex'", ) .unwrap(); + let mode = std::fs::metadata(script_path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + let mut old_content = String::new(); + old_reader.read_to_string(&mut old_content).unwrap(); + assert_eq!(old_content, "old public content"); + assert!(!std::fs::read_to_string(existing_script) + .unwrap() + .contains("old public content")); + } + + #[test] + fn test_codex_developer_instructions_preserve_startup_prompt() { + let startup_prompt = concat!( + "You are Stromus.\n", + "Quotes: \"truth\" and 'care', plus \"\"\" and ''' fences.\n", + "Path: C:\\work\\synapt\n", + "Shell-like text stays text: $(touch nope) and `touch nope`." + ); + let args = codex_developer_instruction_args(startup_prompt); + + assert_eq!(args[0], "--config"); + let parsed = args[1].parse::().unwrap(); + assert_eq!( + parsed + .get("developer_instructions") + .and_then(|v| v.as_str()), + Some(startup_prompt) + ); + } + + #[test] + fn test_codex_developer_instructions_preserve_large_prompt() { + let startup_prompt = format!("# Stromus\n\n{}", "identity substrate\n".repeat(4_000)); + let args = codex_developer_instruction_args(&startup_prompt); + let parsed = args[1].parse::().unwrap(); + + assert_eq!( + parsed + .get("developer_instructions") + .and_then(|v| v.as_str()), + Some(startup_prompt.as_str()) + ); + } + + #[test] + fn test_codex_developer_instructions_skip_empty_prompt() { + assert!(codex_developer_instruction_args("").is_empty()); + assert!(codex_developer_instruction_args(" \n").is_empty()); + } + + #[test] + fn test_codex_initial_prompt_contains_recall_context_only() { + let prompt = + build_codex_initial_prompt("opus", "Last session: shipped recall startup injection.") + .unwrap(); + assert!(prompt.contains("agent `opus`")); - assert!(prompt.contains("")); - assert!(prompt.contains("You are Opus.")); + assert!(!prompt.contains("")); assert!(prompt.contains("")); assert!(prompt.contains("Last session: shipped recall startup injection.")); assert!(prompt.contains("Do not summarize it unless asked.")); @@ -1825,9 +1921,9 @@ mod tests { #[test] fn test_codex_initial_prompt_skips_empty_context() { - assert!(build_codex_initial_prompt("opus", "", "").is_none()); + assert!(build_codex_initial_prompt("opus", "").is_none()); - let prompt = build_codex_initial_prompt("opus", "", "Recall context").unwrap(); + let prompt = build_codex_initial_prompt("opus", "Recall context").unwrap(); assert!(!prompt.contains("")); assert!(prompt.contains("")); } @@ -1835,10 +1931,73 @@ mod tests { #[test] fn test_codex_initial_prompt_wraps_long_recall_lines() { let recall = "x".repeat(CODEX_STARTUP_MAX_LINE_CHARS + 1); - let prompt = build_codex_initial_prompt("opus", "", &recall).unwrap(); + let prompt = build_codex_initial_prompt("opus", &recall).unwrap(); assert!(prompt.contains(&format!("{}\nx", "x".repeat(CODEX_STARTUP_MAX_LINE_CHARS)))); } + #[test] + fn test_codex_prompt_transport_pastes_then_sends_exactly_three_enters() { + let mut loaded = Vec::new(); + let mut commands = Vec::new(); + let mut sleeps = Vec::new(); + + send_codex_initial_prompt_with( + "synapt:atlas", + "recall context", + |prompt| { + loaded.push(prompt.to_string()); + Ok(()) + }, + |args| { + commands.push(args.iter().map(|arg| arg.to_string()).collect::>()); + Ok(()) + }, + |duration| sleeps.push(duration), + ) + .unwrap(); + + assert_eq!(loaded, ["recall context"]); + assert_eq!( + commands, + [ + ["paste-buffer", "-d", "-t", "synapt:atlas"], + ["send-keys", "-t", "synapt:atlas", "Enter"], + ["send-keys", "-t", "synapt:atlas", "Enter"], + ["send-keys", "-t", "synapt:atlas", "Enter"], + ] + ); + assert_eq!( + sleeps, + [ + std::time::Duration::from_millis(300), + std::time::Duration::from_millis(300), + ] + ); + } + + #[test] + fn test_codex_prompt_transport_stops_after_tmux_failure() { + let mut commands = Vec::new(); + let error = send_codex_initial_prompt_with( + "synapt:sentinel", + "recall context", + |_| Ok(()), + |args| { + commands.push(args.iter().map(|arg| arg.to_string()).collect::>()); + if commands.len() == 3 { + anyhow::bail!("simulated tmux failure"); + } + Ok(()) + }, + |_| {}, + ) + .unwrap_err(); + + assert_eq!(error.to_string(), "simulated tmux failure"); + assert_eq!(commands.len(), 3); + assert_eq!(commands[2], ["send-keys", "-t", "synapt:sentinel", "Enter"]); + } + #[test] fn test_resolve_startup_prompt_path_uses_workspace_for_relative_paths() { let root = PathBuf::from("/tmp/gripspace"); diff --git a/src/git/mod.rs b/src/git/mod.rs index cd7ebcc..4c49e84 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -198,6 +198,24 @@ pub fn get_current_branch(repo: &Repository) -> Result { } } +/// Resolve the remote's default branch from `refs/remotes//HEAD`. +/// +/// Local only: a cleanup verb must not acquire a network failure mode, so this +/// reads the ref that `clone` writes rather than asking the remote. Returns +/// `None` when the ref is absent, or present but not symbolic — both are normal +/// states for a clone that was never given one, and both mean "unknown" rather +/// than "no default exists". Callers must treat `None` as a reason to protect +/// more, never as a reason to protect less. +pub fn get_default_branch(repo: &Repository, remote: &str) -> Option { + let reference = repo + .find_reference(&format!("refs/remotes/{}/HEAD", remote)) + .ok()?; + let target = reference.symbolic_target()?; + target + .strip_prefix(&format!("refs/remotes/{}/", remote)) + .map(|name| name.to_string()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/tests/cli_tests.rs b/tests/cli_tests.rs index 5be90a7..a08253d 100644 --- a/tests/cli_tests.rs +++ b/tests/cli_tests.rs @@ -475,3 +475,104 @@ fn test_checkout_remove_rejects_extra_positional_args() { "unexpected extra arguments after checkout name", )); } + +// --- #196: repo-filter validation on the staging/commit/push verbs ------------- +// +// `validate_repo_filters_known` already existed and already produced the right +// message, including a basename suggestion for the common mistake that surfaced +// this: a manifest entry named `-` checked out at `./`, where +// the operator naturally types `--repo `. These three verbs did not call +// the validator, so an unknown `--repo` name matched zero repos and the command +// reported success. +// +// The failure direction is what makes it worth a test: another agent reads +// "pushed" or "staged" and acts on it. + +#[test] +fn test_add_unknown_repo_filter_is_refused_not_silently_empty() { + let ws = WorkspaceBuilder::new().add_repo("app").build(); + + let mut cmd = Command::cargo_bin("gr").unwrap(); + cmd.current_dir(&ws.workspace_root) + .arg("add") + .arg(".") + .arg("--repo") + .arg("missing") + .assert() + .code(2) + .stderr(predicate::str::contains( + "repo filter 'missing' not found in local manifest", + )); +} + +#[test] +fn test_commit_unknown_repo_filter_is_refused_not_silently_empty() { + let ws = WorkspaceBuilder::new().add_repo("app").build(); + + let mut cmd = Command::cargo_bin("gr").unwrap(); + cmd.current_dir(&ws.workspace_root) + .arg("commit") + .arg("-m") + .arg("msg") + .arg("--repo") + .arg("missing") + .assert() + .code(2) + .stderr(predicate::str::contains( + "repo filter 'missing' not found in local manifest", + )); +} + +#[test] +fn test_push_unknown_repo_filter_is_refused_not_silently_empty() { + let ws = WorkspaceBuilder::new().add_repo("app").build(); + + let mut cmd = Command::cargo_bin("gr").unwrap(); + cmd.current_dir(&ws.workspace_root) + .arg("push") + .arg("--repo") + .arg("missing") + .assert() + .code(2) + .stderr(predicate::str::contains( + "repo filter 'missing' not found in local manifest", + )); +} + +/// Control for the three tests above: a KNOWN repo name must still REACH THE +/// WORK, so the rejections cannot be passing merely because `gr add` fails for +/// some unrelated reason in this fixture. +/// +/// The control asserts the DESTINATION, not the absence of a message. An +/// earlier version of this test wrote no file and checked only that one +/// substring was missing from stderr — which passes identically whether `add` +/// stages the file or does nothing at all, and those are exactly the two +/// outcomes a control has to separate. The fixture commits its files before +/// cloning, so the worktree starts clean and the test must dirty it itself. +#[test] +fn test_add_known_repo_filter_reaches_the_work_and_stages() { + let ws = WorkspaceBuilder::new().add_repo("app").build(); + let repo = ws.repo_path("app"); + std::fs::write(repo.join("control.txt"), "dirty\n").unwrap(); + + let mut cmd = Command::cargo_bin("gr").unwrap(); + cmd.current_dir(&ws.workspace_root) + .arg("add") + .arg(".") + .arg("--repo") + .arg("app") + .assert() + .success() + .stderr(predicate::str::contains("not found in local manifest").not()); + + let staged = std::process::Command::new("git") + .args(["diff", "--cached", "--name-only"]) + .current_dir(&repo) + .output() + .unwrap(); + let staged = String::from_utf8_lossy(&staged.stdout); + assert!( + staged.contains("control.txt"), + "known --repo name must reach the work: expected control.txt in the index, got {staged:?}" + ); +} diff --git a/tests/test_prune.rs b/tests/test_prune.rs index 53001d0..a84733a 100644 --- a/tests/test_prune.rs +++ b/tests/test_prune.rs @@ -2,9 +2,41 @@ mod common; +use assert_cmd::Command as AssertCommand; +use predicates::prelude::*; + use common::fixtures::WorkspaceBuilder; use common::git_helpers; +/// Put a fixture into the shape production actually runs in: manifest target is +/// `dev`, `dev` is checked out, and `main` exists as the release branch. Before +/// 2026-08-01 target and default were both `main`, which protected `main` by +/// coincidence; every prune test still encodes that retired arrangement. +fn dev_target_workspace(repo: &str) -> common::fixtures::WorkspaceFixture { + let ws = WorkspaceBuilder::new().add_repo(repo).build(); + let manifest_path = ws + .workspace_root + .join(".gitgrip") + .join("spaces") + .join("main") + .join("gripspace.yml"); + let yaml = std::fs::read_to_string(&manifest_path).unwrap(); + assert!( + yaml.contains("default_branch: main"), + "fixture no longer declares a target this helper knows how to move: {yaml}" + ); + std::fs::write( + &manifest_path, + yaml.replace("default_branch: main", "default_branch: dev"), + ) + .unwrap(); + + let repo_path = ws.repo_path(repo); + git_helpers::create_branch(&repo_path, "dev"); + assert!(git_helpers::branch_exists(&repo_path, "main")); + ws +} + #[test] fn test_prune_dry_run_lists_merged_branches() { let ws = WorkspaceBuilder::new().add_repo("alpha").build(); @@ -131,3 +163,94 @@ fn test_prune_no_merged_branches() { // Unmerged branch should still exist assert!(git_helpers::branch_exists(&repo_path, "feat/unmerged")); } + +#[test] +fn test_prune_protects_main_when_target_is_dev() { + // The production shape: target `dev`, standing on `dev`, `main` present. + // `main` is neither current nor target here, which is exactly the state the + // old two-slot guard left unprotected. + let ws = dev_target_workspace("alpha"); + let repo_path = ws.repo_path("alpha"); + + // A genuinely merged branch, so this test also proves prune still WORKS. + // Without it, a fix that protected everything would pass just as happily. + git_helpers::create_branch(&repo_path, "feat/spent"); + git_helpers::commit_file(&repo_path, "spent.txt", "x", "spent work"); + git_helpers::checkout(&repo_path, "dev"); + std::process::Command::new("git") + .args(["merge", "feat/spent", "--no-ff", "-m", "merge spent"]) + .current_dir(&repo_path) + .output() + .unwrap(); + + AssertCommand::cargo_bin("gr") + .unwrap() + .current_dir(&ws.workspace_root) + .args(["prune", "--execute", "--repo", "alpha"]) + .assert() + .success(); + + assert!( + git_helpers::branch_exists(&repo_path, "main"), + "the release branch must survive a prune run from dev" + ); + assert!(git_helpers::branch_exists(&repo_path, "dev")); + assert!( + !git_helpers::branch_exists(&repo_path, "feat/spent"), + "positive control: prune must still delete a merged branch, or this test \ + would pass against a guard that simply protects everything" + ); +} + +#[test] +fn test_prune_protects_more_and_says_so_when_default_is_unresolvable() { + // The default is made GENUINELY unresolvable -- origin/HEAD is deleted from a + // real clone -- rather than stubbed. A stub would assert the code path against + // a fixture instead of against the condition, and the real failure (a clone + // that was never given an origin/HEAD) would go unexercised. + let ws = dev_target_workspace("alpha"); + let repo_path = ws.repo_path("alpha"); + + let before = std::process::Command::new("git") + .args(["symbolic-ref", "refs/remotes/origin/HEAD"]) + .current_dir(&repo_path) + .output() + .unwrap(); + assert!( + before.status.success(), + "control: the clone must HAVE an origin/HEAD before we remove it, or this \ + test proves nothing about removing it" + ); + + std::process::Command::new("git") + .args(["symbolic-ref", "--delete", "refs/remotes/origin/HEAD"]) + .current_dir(&repo_path) + .output() + .unwrap(); + let after = std::process::Command::new("git") + .args(["symbolic-ref", "refs/remotes/origin/HEAD"]) + .current_dir(&repo_path) + .output() + .unwrap(); + assert!( + !after.status.success(), + "the removal must actually make resolution fail" + ); + + AssertCommand::cargo_bin("gr") + .unwrap() + .current_dir(&ws.workspace_root) + .args(["prune", "--execute", "--repo", "alpha"]) + .assert() + .success() + // Protecting more must not be silent. A silent protected-more reads as + // "the default resolved fine" to the next person who runs this. + // NOTE: Output::warning writes to stdout, not stderr -- asserted against + // the stream the binary actually uses, verified by running it. + .stdout(predicate::str::contains( + "could not determine the default branch", + )); + + assert!(git_helpers::branch_exists(&repo_path, "main")); + assert!(git_helpers::branch_exists(&repo_path, "dev")); +}