crashlabs/huskies
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 37 Wiki Activity

huskies: merge 1176 bug base_branch fallback hardcodes master instead of auto-detecting

Browse Source
This commit is contained in:
Huskies Agent
2026-07-16 16:22:46 +00:00
parent a4e2d4bc25
commit efd0097f76
11 changed files with 301 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/agents/merge/squash/mod.rs
+9 -9
View File
@@ -17,6 +17,14 @@ use crate::config::ProjectConfig;
/// causing `git cherry-pick merge-queue/…` to fail with "bad revision".
static MERGE_LOCK: Mutex<()> = Mutex::new(());
/// Resolve the base branch for `project_root` from config, or auto-detect it.
fn resolve_base_branch(project_root: &Path) -> String {
let configured = crate::config::ProjectConfig::load(project_root)
.ok()
.and_then(|c| c.base_branch);
crate::worktree::resolve_base_branch(project_root, configured.as_deref())
}
pub(crate) fn run_squash_merge(
project_root: &Path,
branch: &str,
@@ -31,10 +39,7 @@ pub(crate) fn run_squash_merge(
// A zero-commit branch produces an empty squash and a silent "nothing to
// commit" failure. Catch it early with a grep-able error before any merge
// work starts.
let base_branch = crate::config::ProjectConfig::load(project_root)
.ok()
.and_then(|c| c.base_branch.clone())
.unwrap_or_else(|| "master".to_string());
let base_branch = resolve_base_branch(project_root);
let ahead_out = Command::new("git")
.args(["rev-list", "--count", &format!("{base_branch}..{branch}")])
@@ -316,11 +321,6 @@ pub(crate) fn run_squash_merge(
.map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string())
.unwrap_or_default();
let base_branch = crate::config::ProjectConfig::load(project_root)
.ok()
.and_then(|c| c.base_branch.clone())
.unwrap_or_else(|| "master".to_string());
if current_branch != base_branch {
all_output.push_str(&format!(
"=== VERIFICATION FAILED: expected branch '{base_branch}' but HEAD is on \
server/src/agents/merge/squash/tests_basic.rs
+73
View File
@@ -178,6 +178,79 @@ async fn squash_merge_clean_merge_succeeds() {
);
}
#[tokio::test]
async fn squash_merge_succeeds_on_main_based_repo_with_base_branch_unset() {
use std::fs;
use tempfile::tempdir;
let tmp = tempdir().unwrap();
let repo = tmp.path();
// Repo whose default branch is `main` — no `master` branch exists at all,
// and no `.huskies/project.toml` sets `base_branch`. run_squash_merge must
// auto-detect `main` instead of assuming `master` (bug 1176).
Command::new("git")
.args(["init", "-b", "main"])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["config", "user.email", "test@test.com"])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["config", "user.name", "Test"])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["commit", "--allow-empty", "-m", "init"])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["checkout", "-b", "feature/story-main_test"])
.current_dir(repo)
.output()
.unwrap();
fs::write(repo.join("new_file.txt"), "new content").unwrap();
Command::new("git")
.args(["add", "."])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["commit", "-m", "add new file"])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["checkout", "main"])
.current_dir(repo)
.output()
.unwrap();
let result = run_squash_merge(repo, "feature/story-main_test", "main_test").unwrap();
assert!(
matches!(
result,
super::MergeResult::Success {
conflicts_resolved: false,
..
}
),
"clean merge should succeed on a main-based repo; got: {result:?}"
);
assert!(
repo.join("new_file.txt").exists(),
"merged file should exist on main"
);
}
#[tokio::test]
async fn squash_merge_nonexistent_branch_fails() {
use tempfile::tempdir;
server/src/agents/pool/start/spawn.rs
+5 -4
View File
@@ -189,10 +189,10 @@ pub(super) async fn run_agent_spawn(
let wt_info = {
let wt_path = crate::worktree::worktree_path(&project_root_clone, &sid);
let branch = format!("feature/story-{sid}");
let base_branch = config_clone
.base_branch
.clone()
.unwrap_or_else(|| crate::worktree::detect_base_branch(&project_root_clone));
let base_branch = crate::worktree::resolve_base_branch(
&project_root_clone,
config_clone.base_branch.as_deref(),
);
let deadline =
tokio::time::Instant::now() + std::time::Duration::from_secs(worktree_wait_secs);
loop {
@@ -260,6 +260,7 @@ pub(super) async fn run_agent_spawn(
}
let (command, mut args, mut prompt) = match config_clone.render_agent_args(
&project_root_clone,
&wt_path_str,
&sid,
Some(&aname),
server/src/chat/commands/diff.rs
+3 -12
View File
@@ -90,19 +90,10 @@ fn find_story_id(num_str: &str) -> Option<String> {
/// Return the configured base branch, or auto-detect it from the project root HEAD.
fn resolve_base_branch(project_root: &Path) -> String {
crate::config::ProjectConfig::load(project_root)
let configured = crate::config::ProjectConfig::load(project_root)
.ok()
.and_then(|c| c.base_branch)
.unwrap_or_else(|| {
Command::new("git")
.args(["rev-parse", "--abbrev-ref", "HEAD"])
.current_dir(project_root)
.output()
.ok()
.filter(|o| o.status.success())
.map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string())
.unwrap_or_else(|| "master".to_string())
})
.and_then(|c| c.base_branch);
crate::worktree::resolve_base_branch(project_root, configured.as_deref())
}
/// Run a git command in `dir`, returning trimmed stdout (empty string on failure).
server/src/config/mod.rs
+6 -3
View File
@@ -593,6 +593,7 @@ impl ProjectConfig {
/// If `agent_name` is None, uses the first (default) agent.
pub fn render_agent_args(
&self,
project_root: &Path,
worktree_path: &str,
story_id: &str,
agent_name: Option<&str>,
@@ -607,9 +608,11 @@ impl ProjectConfig {
.ok_or_else(|| "No agents configured".to_string())?,
};
let bb = base_branch
.or(self.base_branch.as_deref())
.unwrap_or("master");
let bb_owned = crate::worktree::resolve_base_branch(
project_root,
base_branch.or(self.base_branch.as_deref()),
);
let bb = bb_owned.as_str();
let aname = agent.name.as_str();
let render = |s: &str| {
s.replace("{{worktree_path}}", worktree_path)
server/src/config/tests.rs
+49 -7
View File
@@ -129,7 +129,13 @@ max_turns = 0
fn render_agent_args_default() {
let config = ProjectConfig::default();
let (cmd, args, prompt) = config
.render_agent_args("/tmp/wt", "42_foo", None, None)
.render_agent_args(
std::path::Path::new("/tmp/wt"),
"/tmp/wt",
"42_foo",
None,
None,
)
.unwrap();
assert_eq!(cmd, "claude");
assert!(args.is_empty());
@@ -155,7 +161,13 @@ max_turns = 30
let config = ProjectConfig::parse(toml_str).unwrap();
let (cmd, args, prompt) = config
.render_agent_args("/tmp/wt", "42_foo", Some("supervisor"), Some("master"))
.render_agent_args(
std::path::Path::new("/tmp/wt"),
"/tmp/wt",
"42_foo",
Some("supervisor"),
Some("master"),
)
.unwrap();
assert_eq!(cmd, "claude");
assert!(args.contains(&"--model".to_string()));
@@ -173,7 +185,13 @@ max_turns = 30
// Render for coder
let (_, coder_args, _) = config
.render_agent_args("/tmp/wt", "42_foo", Some("coder"), Some("master"))
.render_agent_args(
std::path::Path::new("/tmp/wt"),
"/tmp/wt",
"42_foo",
Some("coder"),
Some("master"),
)
.unwrap();
assert!(coder_args.contains(&"sonnet".to_string()));
assert!(coder_args.contains(&"30".to_string()));
@@ -184,7 +202,13 @@ max_turns = 30
#[test]
fn render_agent_args_not_found() {
let config = ProjectConfig::default();
let result = config.render_agent_args("/tmp/wt", "42_foo", Some("nonexistent"), None);
let result = config.render_agent_args(
std::path::Path::new("/tmp/wt"),
"/tmp/wt",
"42_foo",
Some("nonexistent"),
None,
);
assert!(result.is_err());
assert!(result.unwrap_err().contains("No agent named 'nonexistent'"));
}
@@ -576,7 +600,13 @@ prompt = "git difftool {{base_branch}}...HEAD"
"#;
let config = ProjectConfig::parse(toml_str).unwrap();
let (_, _, prompt) = config
.render_agent_args("/tmp/wt", "42_foo", None, None)
.render_agent_args(
std::path::Path::new("/tmp/wt"),
"/tmp/wt",
"42_foo",
None,
None,
)
.unwrap();
assert!(
prompt.contains("develop"),
@@ -595,7 +625,13 @@ prompt = "git difftool {{base_branch}}...HEAD"
"#;
let config = ProjectConfig::parse(toml_str).unwrap();
let (_, _, prompt) = config
.render_agent_args("/tmp/wt", "42_foo", None, Some("feature-x"))
.render_agent_args(
std::path::Path::new("/tmp/wt"),
"/tmp/wt",
"42_foo",
None,
Some("feature-x"),
)
.unwrap();
assert!(
prompt.contains("feature-x"),
@@ -674,7 +710,13 @@ disallowed_tools = ["ScheduleWakeup", "SomeTool"]
let config = ProjectConfig::parse(toml_str).unwrap();
let (_, args, _) = config
.render_agent_args("/tmp/wt", "42_foo", None, None)
.render_agent_args(
std::path::Path::new("/tmp/wt"),
"/tmp/wt",
"42_foo",
None,
None,
)
.unwrap();
assert!(
args.contains(&"--disallowedTools".to_string()),
server/src/http/mcp/story_tools/criteria.rs
+138 -4
View File
@@ -185,10 +185,16 @@ fn validate_criterion_check(
workflow: &WorkflowState,
) -> Result<(), String> {
let branch = format!("feature/story-{story_id}");
let base_branch = {
let configured = crate::config::ProjectConfig::load(project_root)
.ok()
.and_then(|c| c.base_branch);
crate::worktree::resolve_base_branch(project_root, configured.as_deref())
};
// ── A: branch has commits vs master ──────────────────────────────────────
// ── A: branch has commits vs base branch ───────────────────────────────
let commits = Command::new("git")
.args(["log", &format!("master..{branch}"), "--oneline"])
.args(["log", &format!("{base_branch}..{branch}"), "--oneline"])
.current_dir(project_root)
.output()
.ok()
@@ -204,7 +210,7 @@ fn validate_criterion_check(
// ── B: AC text mentions a file touched by the branch ─────────────────────
let changed_files: Vec<String> = Command::new("git")
.args(["diff", &format!("master...{branch}"), "--name-only"])
.args(["diff", &format!("{base_branch}...{branch}"), "--name-only"])
.current_dir(project_root)
.output()
.ok()
@@ -254,7 +260,7 @@ fn validate_criterion_check(
Err(format!(
"No corroborating evidence for criterion '{ac_text}'. \
To proceed: commit your work to '{branch}' (currently has no commits vs master), \
To proceed: commit your work to '{branch}' (currently has no commits vs {base_branch}), \
add a passing test whose name matches the criterion, \
or change a file mentioned in the criterion text."
))
@@ -652,6 +658,134 @@ mod tests {
);
}
#[test]
fn tool_check_criterion_succeeds_on_main_based_repo_with_base_branch_unset() {
let tmp = tempfile::tempdir().unwrap();
// Repo whose default branch is `main` — no `master` branch exists at
// all, and no `.huskies/project.toml` sets `base_branch`. The evidence
// gate must diff against the auto-detected `main`, not a hardcoded
// `master` (bug 1176).
std::process::Command::new("git")
.args(["init", "-b", "main"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["config", "user.email", "test@test.com"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["config", "user.name", "Test"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["commit", "--allow-empty", "-m", "init"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["checkout", "-b", "feature/story-9998_main_branch"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["commit", "--allow-empty", "-m", "feature work"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["checkout", "main"])
.current_dir(tmp.path())
.output()
.unwrap();
crate::db::ensure_content_store();
crate::db::write_item_with_content(
"9998_main_branch",
"2_current",
"---\nname: Main Branch Test\n---\n## AC\n- [ ] Implement the feature\n",
crate::db::ItemMeta::named("Main Branch Test"),
);
let ctx = test_ctx(tmp.path());
let result = tool_check_criterion(
&json!({"story_id": "9998_main_branch", "criterion_index": 0}),
&ctx,
);
assert!(
result.is_ok(),
"Expected ok on main-based repo with commits ahead of main: {result:?}"
);
}
#[test]
fn tool_check_criterion_succeeds_on_main_based_repo() {
let tmp = tempfile::tempdir().unwrap();
// Repo whose default branch is `main` — no `master` branch exists at
// all. The evidence-gate diff must resolve against `main`, not a
// hardcoded `master` (bug 1176), or `git log master..branch` fails as
// an invalid revision range and blocks every check_criterion call.
std::process::Command::new("git")
.args(["init", "-b", "main"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["config", "user.email", "test@test.com"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["config", "user.name", "Test"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["commit", "--allow-empty", "-m", "init"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["checkout", "-b", "feature/story-9996_main_based"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["commit", "--allow-empty", "-m", "feature work"])
.current_dir(tmp.path())
.output()
.unwrap();
std::process::Command::new("git")
.args(["checkout", "main"])
.current_dir(tmp.path())
.output()
.unwrap();
crate::db::ensure_content_store();
crate::db::write_item_with_content(
"9996_main_based",
"2_current",
"---\nname: Main Based\n---\n## AC\n- [ ] Implement the feature\n",
crate::db::ItemMeta::named("Main Based"),
);
let ctx = test_ctx(tmp.path());
let result = tool_check_criterion(
&json!({"story_id": "9996_main_based", "criterion_index": 0}),
&ctx,
);
assert!(
result.is_ok(),
"Expected check_criterion to succeed on a main-based repo: {result:?}"
);
}
#[test]
fn tool_check_criterion_missing_story_id() {
let tmp = tempfile::tempdir().unwrap();
server/src/worktree/create.rs
+2 -5
View File
@@ -5,7 +5,7 @@ use std::path::Path;
use std::process::Command;
use super::git::{
branch_name, configure_sparse_checkout, create_worktree_sync, detect_base_branch,
branch_name, configure_sparse_checkout, create_worktree_sync, resolve_base_branch,
};
use super::{WorktreeInfo, worktree_path, write_mcp_json};
@@ -42,10 +42,7 @@ pub async fn create_worktree(
) -> Result<WorktreeInfo, String> {
let wt_path = worktree_path(project_root, story_id);
let branch = branch_name(story_id);
let base_branch = config
.base_branch
.clone()
.unwrap_or_else(|| detect_base_branch(project_root));
let base_branch = resolve_base_branch(project_root, config.base_branch.as_deref());
let root = project_root.to_path_buf();
// Already exists — reuse without re-running destructive setup commands.
server/src/worktree/git.rs
+13
View File
@@ -24,6 +24,19 @@ pub(crate) fn detect_base_branch(project_root: &Path) -> String {
.unwrap_or_else(|| "master".to_string())
}
/// Resolve the base branch to use for `project_root`.
///
/// Returns `configured` (typically `ProjectConfig.base_branch`) when set;
/// otherwise auto-detects the repository's default branch via
/// [`detect_base_branch`]. This is the single shared resolver for base-branch
/// fallback — production call sites must use it rather than hardcoding
/// `"master"` directly.
pub(crate) fn resolve_base_branch(project_root: &Path, configured: Option<&str>) -> String {
configured
.map(str::to_string)
.unwrap_or_else(|| detect_base_branch(project_root))
}
/// Placeholder for worktree isolation of `.huskies/work/`.
///
/// Previous approaches (sparse checkout, skip-worktree) all leaked state
server/src/worktree/mod.rs
+1 -1
View File
@@ -10,8 +10,8 @@ mod sweep;
pub use cleanup::{format_report, run_cleanup};
pub use create::create_worktree;
pub use create::install_pre_commit_hook;
pub(crate) use git::detect_base_branch;
pub use git::migrate_slug_paths;
pub(crate) use git::resolve_base_branch;
pub use remove::remove_worktree_by_story_id;
#[derive(Debug, Clone)]
server/src/worktree/remove.rs
+2 -5
View File
@@ -3,7 +3,7 @@ use crate::config::ProjectConfig;
use std::path::Path;
use super::create::run_teardown_commands;
use super::git::{branch_name, detect_base_branch, remove_worktree_sync};
use super::git::{branch_name, remove_worktree_sync, resolve_base_branch};
use super::{WorktreeInfo, worktree_path};
/// Remove a git worktree and its branch.
@@ -34,10 +34,7 @@ pub async fn remove_worktree_by_story_id(
return Err(format!("Worktree not found for story: {story_id}"));
}
let branch = branch_name(story_id);
let base_branch = config
.base_branch
.clone()
.unwrap_or_else(|| detect_base_branch(project_root));
let base_branch = resolve_base_branch(project_root, config.base_branch.as_deref());
let info = WorktreeInfo {
path,
branch,