storkit: merge 379_bug_start_agent_ignores_story_front_matter_agent_assignment
This commit is contained in:
@@ -253,6 +253,24 @@ impl AgentPool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Read the preferred agent from the story's front matter before acquiring
|
||||||
|
// the lock. When no explicit agent_name is given, this lets start_agent
|
||||||
|
// honour `agent: coder-opus` written by the `assign` command — mirroring
|
||||||
|
// the auto_assign path (bug 379).
|
||||||
|
let front_matter_agent: Option<String> = if agent_name.is_none() {
|
||||||
|
find_active_story_stage(project_root, story_id).and_then(|stage_dir| {
|
||||||
|
let path = project_root
|
||||||
|
.join(".storkit")
|
||||||
|
.join("work")
|
||||||
|
.join(stage_dir)
|
||||||
|
.join(format!("{story_id}.md"));
|
||||||
|
let contents = std::fs::read_to_string(path).ok()?;
|
||||||
|
crate::io::story_metadata::parse_front_matter(&contents).ok()?.agent
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
|
||||||
// Atomically resolve agent name, check availability, and register as
|
// Atomically resolve agent name, check availability, and register as
|
||||||
// Pending. When `agent_name` is `None` the first idle coder is
|
// Pending. When `agent_name` is `None` the first idle coder is
|
||||||
// selected inside the lock so no TOCTOU race can occur between the
|
// selected inside the lock so no TOCTOU race can occur between the
|
||||||
@@ -268,24 +286,75 @@ impl AgentPool {
|
|||||||
|
|
||||||
resolved_name = match agent_name {
|
resolved_name = match agent_name {
|
||||||
Some(name) => name.to_string(),
|
Some(name) => name.to_string(),
|
||||||
None => auto_assign::find_free_agent_for_stage(&config, &agents, &PipelineStage::Coder)
|
None => {
|
||||||
.map(|s| s.to_string())
|
// Honour the `agent:` field in the story's front matter so that
|
||||||
.ok_or_else(|| {
|
// `start 368` after `assign 368 opus` picks the right agent
|
||||||
if config
|
// (bug 379). Mirrors the auto_assign selection logic.
|
||||||
.agent
|
if let Some(ref pref) = front_matter_agent {
|
||||||
.iter()
|
let stage_matches = config
|
||||||
.any(|a| agent_config_stage(a) == PipelineStage::Coder)
|
.find_agent(pref)
|
||||||
{
|
.map(|cfg| agent_config_stage(cfg) == PipelineStage::Coder)
|
||||||
format!(
|
.unwrap_or(false);
|
||||||
"All coder agents are busy; story '{story_id}' has been \
|
if stage_matches {
|
||||||
queued in work/2_current/ and will be auto-assigned when \
|
if auto_assign::is_agent_free(&agents, pref) {
|
||||||
one becomes available"
|
pref.clone()
|
||||||
)
|
} else {
|
||||||
|
return Err(format!(
|
||||||
|
"Preferred agent '{pref}' from story front matter is busy; \
|
||||||
|
story '{story_id}' has been queued in work/2_current/ and will \
|
||||||
|
be auto-assigned when it becomes available"
|
||||||
|
));
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
"No coder agent configured. Specify an agent_name explicitly."
|
// Stage mismatch — fall back to any free coder.
|
||||||
.to_string()
|
auto_assign::find_free_agent_for_stage(
|
||||||
|
&config,
|
||||||
|
&agents,
|
||||||
|
&PipelineStage::Coder,
|
||||||
|
)
|
||||||
|
.map(|s| s.to_string())
|
||||||
|
.ok_or_else(|| {
|
||||||
|
if config
|
||||||
|
.agent
|
||||||
|
.iter()
|
||||||
|
.any(|a| agent_config_stage(a) == PipelineStage::Coder)
|
||||||
|
{
|
||||||
|
format!(
|
||||||
|
"All coder agents are busy; story '{story_id}' has been \
|
||||||
|
queued in work/2_current/ and will be auto-assigned when \
|
||||||
|
one becomes available"
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
"No coder agent configured. Specify an agent_name explicitly."
|
||||||
|
.to_string()
|
||||||
|
}
|
||||||
|
})?
|
||||||
}
|
}
|
||||||
})?,
|
} else {
|
||||||
|
auto_assign::find_free_agent_for_stage(
|
||||||
|
&config,
|
||||||
|
&agents,
|
||||||
|
&PipelineStage::Coder,
|
||||||
|
)
|
||||||
|
.map(|s| s.to_string())
|
||||||
|
.ok_or_else(|| {
|
||||||
|
if config
|
||||||
|
.agent
|
||||||
|
.iter()
|
||||||
|
.any(|a| agent_config_stage(a) == PipelineStage::Coder)
|
||||||
|
{
|
||||||
|
format!(
|
||||||
|
"All coder agents are busy; story '{story_id}' has been \
|
||||||
|
queued in work/2_current/ and will be auto-assigned when \
|
||||||
|
one becomes available"
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
"No coder agent configured. Specify an agent_name explicitly."
|
||||||
|
.to_string()
|
||||||
|
}
|
||||||
|
})?
|
||||||
|
}
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
key = composite_key(story_id, &resolved_name);
|
key = composite_key(story_id, &resolved_name);
|
||||||
@@ -2196,6 +2265,108 @@ stage = "coder"
|
|||||||
assert_eq!(agents.len(), 1, "existing agents should not be affected");
|
assert_eq!(agents.len(), 1, "existing agents should not be affected");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── front matter agent preference (bug 379) ──────────────────────────────
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn start_agent_honours_front_matter_agent_when_idle() {
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let sk = tmp.path().join(".storkit");
|
||||||
|
let backlog = sk.join("work/1_backlog");
|
||||||
|
std::fs::create_dir_all(&backlog).unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
sk.join("project.toml"),
|
||||||
|
r#"
|
||||||
|
[[agent]]
|
||||||
|
name = "coder-sonnet"
|
||||||
|
stage = "coder"
|
||||||
|
|
||||||
|
[[agent]]
|
||||||
|
name = "coder-opus"
|
||||||
|
stage = "coder"
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
// Story file with agent preference in front matter.
|
||||||
|
std::fs::write(
|
||||||
|
backlog.join("368_story_test.md"),
|
||||||
|
"---\nname: Test Story\nagent: coder-opus\n---\n# Story 368\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let pool = AgentPool::new_test(3010);
|
||||||
|
// coder-sonnet is busy so without front matter the auto-selection
|
||||||
|
// would skip coder-opus and try something else.
|
||||||
|
pool.inject_test_agent("other-story", "coder-sonnet", AgentStatus::Running);
|
||||||
|
|
||||||
|
let result = pool
|
||||||
|
.start_agent(tmp.path(), "368_story_test", None, None)
|
||||||
|
.await;
|
||||||
|
match result {
|
||||||
|
Ok(info) => {
|
||||||
|
assert_eq!(
|
||||||
|
info.agent_name, "coder-opus",
|
||||||
|
"should pick the front-matter preferred agent"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
Err(err) => {
|
||||||
|
// Allowed to fail for infrastructure reasons (no git repo),
|
||||||
|
// but NOT due to agent selection ignoring the preference.
|
||||||
|
assert!(
|
||||||
|
!err.contains("All coder agents are busy"),
|
||||||
|
"should not report busy when coder-opus is idle: {err}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!err.contains("coder-sonnet"),
|
||||||
|
"should not have picked coder-sonnet: {err}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn start_agent_returns_error_when_front_matter_agent_busy() {
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let sk = tmp.path().join(".storkit");
|
||||||
|
let backlog = sk.join("work/1_backlog");
|
||||||
|
std::fs::create_dir_all(&backlog).unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
sk.join("project.toml"),
|
||||||
|
r#"
|
||||||
|
[[agent]]
|
||||||
|
name = "coder-sonnet"
|
||||||
|
stage = "coder"
|
||||||
|
|
||||||
|
[[agent]]
|
||||||
|
name = "coder-opus"
|
||||||
|
stage = "coder"
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
backlog.join("368_story_test.md"),
|
||||||
|
"---\nname: Test Story\nagent: coder-opus\n---\n# Story 368\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let pool = AgentPool::new_test(3011);
|
||||||
|
// Preferred agent is busy — should NOT fall back to coder-sonnet.
|
||||||
|
pool.inject_test_agent("other-story", "coder-opus", AgentStatus::Running);
|
||||||
|
|
||||||
|
let result = pool
|
||||||
|
.start_agent(tmp.path(), "368_story_test", None, None)
|
||||||
|
.await;
|
||||||
|
assert!(result.is_err(), "expected error when preferred agent is busy");
|
||||||
|
let err = result.unwrap_err();
|
||||||
|
assert!(
|
||||||
|
err.contains("coder-opus"),
|
||||||
|
"error should mention the preferred agent: {err}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
err.contains("busy") || err.contains("queued"),
|
||||||
|
"error should say agent is busy or story is queued: {err}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// ── archive + cleanup integration test ───────────────────────────────────
|
// ── archive + cleanup integration test ───────────────────────────────────
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
Reference in New Issue
Block a user