Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/cli/commands/add.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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!();

Expand Down
6 changes: 5 additions & 1 deletion src/cli/commands/commit.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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!();
Expand Down
8 changes: 7 additions & 1 deletion src/cli/commands/push.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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...");
Expand Down
101 changes: 101 additions & 0 deletions tests/cli_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<prefix>-<name>` checked out at `./<name>`, where
// the operator naturally types `--repo <name>`. 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:?}"
);
}