fix(missions): give each phase its own task and its own capture base
ci / gates (push) Failing after 5s
ci / rust (push) Skipped
ci / frontend (push) Skipped
ci / e2e (push) Skipped
ci / publish (push) Skipped

The first push run against a scratch repo (mission 019fc42b) delivered
two branches correctly but exposed two bugs behind them.

Per-phase instructions were inert. `phase_task_text` took only
(kind, title, description), so `mission_phases.config.task` was accepted
by the API, stored, and read by nothing. Every phase of a mission
received byte-identical text differing only by the kind directive —
so both coding phases did the whole mission instead of their slice,
producing the same two files. The task now reaches the agent as a
trailing THIS PHASE'S TASK block, scoped against the shared brief.

The capture base never advanced. `.git/clawmates-base` is written once
at clone time, so phase two diffed against the original clone point and
reported the union of both phases' files as its own. It now moves to
each phase's committed head after the patch is on disk; the pushed
branch stays cumulative because it is built from HEAD.

Both regression tests were confirmed to fail with their fix disabled.

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
Omar Sobh
2026-08-02 13:39:40 -07:00
co-authored by Claude Opus 5
parent e2871c4361
commit 8bad869248
4 changed files with 163 additions and 4 deletions
+8
View File
@@ -247,6 +247,14 @@ pub async fn capture_phase_diff_at(
} }
}; };
// Hand the next phase a base that excludes this one's work. Done here
// rather than inside `commit_phase_work` so the patch on disk is already
// written: if the process dies between the two, the worst case is a phase
// that re-reports work, not a phase whose work is invisible.
if let Some(c) = committed.as_ref() {
mission_workspace::advance_base_commit(&repo, &c.sha);
}
// Gate, then publish. Both are best-effort on top of an artifact that has // Gate, then publish. Both are best-effort on top of an artifact that has
// already landed: a phase whose tests fail, or whose push is rejected, // already landed: a phase whose tests fail, or whose push is rejected,
// still has its patch on disk and its work on a local branch. // still has its patch on disk and its work on a local branch.
+25
View File
@@ -165,6 +165,31 @@ pub(crate) fn record_base_commit(path: &std::path::Path) {
} }
} }
/// Move the capture base forward to a commit a phase just produced.
///
/// The base is recorded once at clone time, which is right for the mission's
/// first phase and wrong for every phase after it: a later phase would diff
/// against the original clone point and claim its predecessors' commits as its
/// own work. Mission `019fc42b` showed this plainly — two coding phases, and
/// the second phase's artifact reported the *union* of both phases' files.
///
/// Advancing after each successful commit makes each artifact the incremental
/// work of one phase. The pushed branch stays cumulative, because it is built
/// from `HEAD` and therefore still carries the earlier commits.
pub(crate) fn advance_base_commit(path: &std::path::Path, sha: &str) {
let sha = sha.trim();
if sha.is_empty() {
return;
}
if let Err(e) = std::fs::write(path.join(".git/clawmates-base"), format!("{sha}\n")) {
eprintln!(
"mission_workspace: could not advance base commit for {} ({e}) — the next \
phase will re-report this phase's work as its own",
path.display()
);
}
}
/// The commit this mission's checkout started from, if it was recorded. /// The commit this mission's checkout started from, if it was recorded.
pub(crate) fn base_commit(path: &std::path::Path) -> Option<String> { pub(crate) fn base_commit(path: &std::path::Path) -> Option<String> {
std::fs::read_to_string(path.join(".git/clawmates-base")) std::fs::read_to_string(path.join(".git/clawmates-base"))
+60 -4
View File
@@ -147,6 +147,7 @@ async fn start_pending_phases(pool: &PgPool) -> Result<(), String> {
// handles order 0 (no prior rows) + skipped phases naturally. // handles order 0 (no prior rows) + skipped phases naturally.
let rows = sqlx::query( let rows = sqlx::query(
"SELECT mp.id, mp.mission_id, mp.kind, mp.order_idx, mp.iteration, "SELECT mp.id, mp.mission_id, mp.kind, mp.order_idx, mp.iteration,
mp.config->>'task' AS phase_task,
m.workspace_id, m.title, m.description m.workspace_id, m.title, m.description
FROM mission_phases mp FROM mission_phases mp
JOIN missions m ON m.id = mp.mission_id JOIN missions m ON m.id = mp.mission_id
@@ -171,6 +172,7 @@ async fn start_pending_phases(pool: &PgPool) -> Result<(), String> {
let workspace_id: Uuid = row.get("workspace_id"); let workspace_id: Uuid = row.get("workspace_id");
let title: String = row.get("title"); let title: String = row.get("title");
let description: Option<String> = row.get("description"); let description: Option<String> = row.get("description");
let phase_task: Option<String> = row.get("phase_task");
let iteration: i32 = row.get("iteration"); let iteration: i32 = row.get("iteration");
if let Err(e) = launch_phase( if let Err(e) = launch_phase(
@@ -182,6 +184,7 @@ async fn start_pending_phases(pool: &PgPool) -> Result<(), String> {
workspace_id, workspace_id,
title: &title, title: &title,
description: description.as_deref(), description: description.as_deref(),
phase_task: phase_task.as_deref(),
iteration, iteration,
}, },
) )
@@ -202,6 +205,14 @@ struct PhaseLaunch<'a> {
workspace_id: Uuid, workspace_id: Uuid,
title: &'a str, title: &'a str,
description: Option<&'a str>, description: Option<&'a str>,
/// This phase's own instructions, from `mission_phases.config.task`.
///
/// Without it every phase of a mission receives byte-identical task text
/// and differs only by the kind directive — so a two-phase mission has
/// both phases do the same work. Mission `019fc42b` demonstrated it: two
/// coding phases with distinct `task` values both produced the same two
/// files, because neither phase ever saw its own instructions.
phase_task: Option<&'a str>,
/// Which pass this is, 0-based. Stamped onto the runs so the completion /// Which pass this is, 0-based. Stamped onto the runs so the completion
/// check can tell this pass's work from the previous one's. /// check can tell this pass's work from the previous one's.
iteration: i32, iteration: i32,
@@ -215,6 +226,7 @@ async fn launch_phase(pool: &PgPool, p: PhaseLaunch<'_>) -> Result<(), String> {
workspace_id, workspace_id,
title, title,
description, description,
phase_task,
iteration, iteration,
} = p; } = p;
// Which team purposes should execute this phase. // Which team purposes should execute this phase.
@@ -314,7 +326,7 @@ async fn launch_phase(pool: &PgPool, p: PhaseLaunch<'_>) -> Result<(), String> {
let prior = crate::evaluator::latest(pool, phase_id) let prior = crate::evaluator::latest(pool, phase_id)
.await .await
.unwrap_or(None); .unwrap_or(None);
let task = phase_task_text(kind, title, description); let task = phase_task_text(kind, title, description, phase_task);
let task = match prior { let task = match prior {
Some((iter, false, guidance)) => format!( Some((iter, false, guidance)) => format!(
"{task}\n\nPASS {} DID NOT SATISFY THE COMPLETION CONDITION. What is \ "{task}\n\nPASS {} DID NOT SATISFY THE COMPLETION CONDITION. What is \
@@ -389,7 +401,12 @@ async fn launch_phase(pool: &PgPool, p: PhaseLaunch<'_>) -> Result<(), String> {
Ok(()) Ok(())
} }
fn phase_task_text(kind: &str, title: &str, description: Option<&str>) -> String { fn phase_task_text(
kind: &str,
title: &str,
description: Option<&str>,
phase_task: Option<&str>,
) -> String {
let base = description.unwrap_or("").trim(); let base = description.unwrap_or("").trim();
// The prior template-derived system prompts trained agents to look // The prior template-derived system prompts trained agents to look
// for `file_read`/`file_write` — tools that no longer exist under // for `file_read`/`file_write` — tools that no longer exist under
@@ -471,7 +488,20 @@ fn phase_task_text(kind: &str, title: &str, description: Option<&str>) -> String
} }
_ => "Execute this mission phase according to the mission brief.", _ => "Execute this mission phase according to the mission brief.",
}; };
format!("MISSION: {title}\n\n{tool_preamble}\n{marker_protocol}\n{directive}\n\nBRIEF:\n{base}") // The mission brief is shared by every phase; this block is not. It goes
// last and says so explicitly, because the failure it fixes was agents
// re-doing the whole mission in each phase rather than their slice of it.
let scope = match phase_task.map(str::trim).filter(|t| !t.is_empty()) {
Some(t) => format!(
"\n\nTHIS PHASE'S TASK — do this and only this. The brief above is \
the mission's full scope across all phases; the following is your \
share of it:\n{t}"
),
None => String::new(),
};
format!(
"MISSION: {title}\n\n{tool_preamble}\n{marker_protocol}\n{directive}\n\nBRIEF:\n{base}{scope}"
)
} }
/// Close phases whose topology_runs are all terminal. /// Close phases whose topology_runs are all terminal.
@@ -727,6 +757,32 @@ mod tests {
pairs.iter().map(|(k, v)| (k.to_string(), *v)).collect() pairs.iter().map(|(k, v)| (k.to_string(), *v)).collect()
} }
/// A phase's own task must reach the agent, and two phases of one mission
/// must not receive identical text.
///
/// This is the regression from mission `019fc42b`: `config.task` was
/// accepted by the API, stored in the DB, and read by nothing. Both coding
/// phases got byte-identical instructions and both produced the same two
/// files. Asserting the texts *differ* is the part that matters — asserting
/// only that the task appears would still pass if the brief carried it.
#[test]
fn phase_task_reaches_the_agent_and_distinguishes_phases() {
let brief = Some("Add two marker files.");
let alpha = phase_task_text("coding", "Demo", brief, Some("Create ALPHA.md"));
let beta = phase_task_text("coding", "Demo", brief, Some("Create BETA.md"));
assert!(alpha.contains("Create ALPHA.md"), "phase task must be injected");
assert!(beta.contains("Create BETA.md"));
assert!(!alpha.contains("BETA.md"), "a phase must not see its sibling's task");
assert_ne!(alpha, beta, "sibling phases received identical instructions");
// A phase with no task of its own is unchanged from before the fix.
let bare = phase_task_text("coding", "Demo", brief, None);
assert!(!bare.contains("THIS PHASE'S TASK"));
// Empty and whitespace-only configs take the same path as absent.
assert_eq!(bare, phase_task_text("coding", "Demo", brief, Some(" ")));
}
/// The marker syntax we hand the agent must be the syntax we parse back. /// The marker syntax we hand the agent must be the syntax we parse back.
/// ///
/// These two sides used to live far apart — the rules were in team-template /// These two sides used to live far apart — the rules were in team-template
@@ -735,7 +791,7 @@ mod tests {
/// Every example line in the prompt is fed through the real parser here. /// Every example line in the prompt is fed through the real parser here.
#[test] #[test]
fn task_text_marker_examples_parse() { fn task_text_marker_examples_parse() {
let text = phase_task_text("coding", "Demo", Some("brief")); let text = phase_task_text("coding", "Demo", Some("brief"), None);
let examples: Vec<&str> = text let examples: Vec<&str> = text
.lines() .lines()
+70
View File
@@ -270,6 +270,22 @@ async fn seed_mission_phase(pool: &sqlx::PgPool, mission: Uuid) -> (Uuid, Uuid)
(ws, phase) (ws, phase)
} }
/// Add a second phase to a mission `seed_mission_phase` already created.
async fn seed_extra_phase(pool: &sqlx::PgPool, mission: Uuid, order_idx: i32) -> Uuid {
let phase = Uuid::now_v7();
sqlx::query(
"INSERT INTO mission_phases (id, mission_id, kind, order_idx, status, config)
VALUES ($1,$2,'coding',$3,'completed','{}'::jsonb)",
)
.bind(phase)
.bind(mission)
.bind(order_idx)
.execute(pool)
.await
.unwrap();
phase
}
/// The production failure this pairs with. Mission 019fc372's agent created /// The production failure this pairs with. Mission 019fc372's agent created
/// the file it was asked for and *committed* it — `rust_sdlc` has a committer /// the file it was asked for and *committed* it — `rust_sdlc` has a committer
/// role, so that is the intended path — leaving a clean working tree. Capture /// role, so that is the intended path — leaving a clean working tree. Capture
@@ -585,3 +601,57 @@ async fn an_unreachable_remote_does_not_lose_the_work() {
.unwrap(); .unwrap();
assert!(String::from_utf8_lossy(&show.stdout).contains("fn kept()")); assert!(String::from_utf8_lossy(&show.stdout).contains("fn kept()"));
} }
/// A later phase must report its own work, not its predecessor's.
///
/// The capture base is recorded once at clone time. Left there, phase 2 diffs
/// against the original clone point and claims phase 1's commits as its own —
/// which is exactly what mission `019fc42b` produced: two coding phases, two
/// artifacts, and the second one reporting the union of both.
#[tokio::test]
async fn a_later_phase_reports_only_its_own_work() {
let pool = cm_testkit::test_pool().await;
let tmp = tempfile::tempdir().unwrap();
let mission = Uuid::now_v7();
let repo = seed_repo(tmp.path(), mission);
let (_, phase_one) = seed_mission_phase(&pool, mission).await;
let phase_two = seed_extra_phase(&pool, mission, 1).await;
std::fs::write(repo.join("ALPHA.md"), "ALPHA-DELIVERED\n").unwrap();
let first = capture(&pool, tmp.path(), mission, phase_one)
.await
.unwrap()
.unwrap();
assert_eq!(first.files_changed, 1, "phase one wrote one file");
std::fs::write(repo.join("BETA.md"), "BETA-DELIVERED\n").unwrap();
let second = capture(&pool, tmp.path(), mission, phase_two)
.await
.unwrap()
.unwrap();
assert_eq!(
second.files_changed, 1,
"phase two must report only BETA.md, not ALPHA.md as well"
);
let patch = std::fs::read_to_string(&second.patch_path).unwrap();
assert!(patch.contains("BETA-DELIVERED"), "phase two's own work");
assert!(
!patch.contains("ALPHA-DELIVERED"),
"phase one's work must not reappear in phase two's patch"
);
// The branch, unlike the patch, stays cumulative: it is built from HEAD,
// so it still carries phase one's commit underneath phase two's.
let branch = second.committed.expect("phase two committed").branch;
let files = Command::new("git")
.arg("-C")
.arg(&repo)
.args(["ls-tree", "--name-only", "-r", &branch])
.output()
.unwrap();
let listed = String::from_utf8_lossy(&files.stdout);
assert!(listed.contains("ALPHA.md"), "branch is cumulative: {listed}");
assert!(listed.contains("BETA.md"), "branch is cumulative: {listed}");
}