fix(missions): stop resetting a checkout that holds mission work
Mission 019fc444 ran two coding phases. Phase 0 created ALPHA.md and
delivery committed it; phase 1 then started and ALPHA.md was gone from
the working tree, so the second phase never saw the first's output.
`ensure_checkout` is called at every phase launch, not once per mission,
and its reuse path runs `git reset --hard origin/<branch>`. That is right
for a checkout picked up cold and destructive for one mid-mission.
Delivery is what made this reachable. Before the mission branch existed,
agent output stayed untracked and a hard reset left it alone. Committing
it makes it tracked, and tracked files absent from origin/<branch> are
exactly what a hard reset removes — so the slice written to stop work
being destroyed is what put it in reach of the thing destroying it. The
flagship shape is the casualty: in research_and_code, the coding phase
never sees the research brief.
`has_local_work` now gates the refresh. It checks both a dirty tree and a
HEAD that has moved off the recorded base, because the two failure shapes
differ: an agent that committed leaves a CLEAN tree at a new HEAD, which
a dirty-tree check alone would miss — and that is precisely the shape
being destroyed. With no recorded base it preserves, since wrongly
skipping a refresh costs staleness while wrongly resetting costs a phase.
This also makes the base-advance fix in 8bad869 live. It was inert in
production: fetch_and_reset calls record_base_commit, overwriting the
advanced base at every phase launch, so both artifacts of 019fc444
recorded origin/main. Their correct per-phase attribution came from the
reset having deleted the earlier work, not from the fix. The two only
compose now that the reset is skipped.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5b53705c97
commit
08b2adae23
@@ -70,7 +70,18 @@ pub async fn ensure_checkout(
|
||||
// Checkouts cloned before this setting existed get it on reuse. It
|
||||
// governs objects created from now on, which is what delivery needs.
|
||||
share_repository_across_uids(&path);
|
||||
fetch_and_reset(&path, default_branch, &auth_url).await?;
|
||||
// `ensure_checkout` runs at every phase launch, not once per mission.
|
||||
// Freshening a pristine checkout is right; freshening one that already
|
||||
// holds this mission's work destroys it. See `has_local_work`.
|
||||
if has_local_work(&path) {
|
||||
eprintln!(
|
||||
"mission_workspace: {} already holds mission work — skipping \
|
||||
fetch/reset so earlier phases' output survives",
|
||||
path.display()
|
||||
);
|
||||
} else {
|
||||
fetch_and_reset(&path, default_branch, &auth_url).await?;
|
||||
}
|
||||
} else {
|
||||
clone(&path, &auth_url).await?;
|
||||
}
|
||||
@@ -129,6 +140,57 @@ async fn clone(path: &std::path::Path, url: &str) -> Result<(), String> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Has anything happened in this checkout since it was created?
|
||||
///
|
||||
/// `ensure_checkout` is called once per *phase launch*, not once per mission,
|
||||
/// and its reuse path runs `git reset --hard origin/<branch>`. That is correct
|
||||
/// for a checkout being picked up cold and destructive for one mid-mission:
|
||||
/// mission `019fc444` had its phase-0 file deleted from the working tree when
|
||||
/// phase 1 started, so the second phase never saw the first's output.
|
||||
///
|
||||
/// Delivery is what made this reachable. Before the mission branch existed,
|
||||
/// agent output stayed *untracked* and `reset --hard` left it alone. Committing
|
||||
/// it — the whole point of the delivery slice — makes it tracked, and tracked
|
||||
/// files that are absent from `origin/<branch>` are exactly what a hard reset
|
||||
/// removes. The feature that preserves work is what put it in reach of the
|
||||
/// reset.
|
||||
///
|
||||
/// "Local work" is either a commit that is not on the fetched tip, or a dirty
|
||||
/// tree. Both are checked because the two phases of the failure look different:
|
||||
/// an agent that committed leaves a clean tree at a new HEAD, and one that did
|
||||
/// not leaves a dirty tree at the old HEAD.
|
||||
fn has_local_work(path: &std::path::Path) -> bool {
|
||||
let git = |args: &[&str]| -> Option<String> {
|
||||
let out = std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(path)
|
||||
.args(["-c", &format!("safe.directory={}", path.display())])
|
||||
.args(args)
|
||||
.output()
|
||||
.ok()?;
|
||||
out.status
|
||||
.success()
|
||||
.then(|| String::from_utf8_lossy(&out.stdout).trim().to_string())
|
||||
};
|
||||
|
||||
// A dirty tree is unambiguous: someone is mid-work here.
|
||||
if let Some(status) = git(&["status", "--porcelain"]) {
|
||||
if !status.is_empty() {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// Otherwise compare HEAD against the base recorded when the checkout was
|
||||
// made. A HEAD that has moved means a phase committed. If the base was
|
||||
// never recorded we cannot tell, and the safe answer is to preserve:
|
||||
// wrongly skipping a refresh costs staleness, wrongly resetting costs a
|
||||
// phase's output.
|
||||
match (base_commit(path), git(&["rev-parse", "HEAD"])) {
|
||||
(Some(base), Some(head)) => base != head,
|
||||
_ => true,
|
||||
}
|
||||
}
|
||||
|
||||
/// Let the server and the agent container both write to this checkout.
|
||||
///
|
||||
/// The checkout is one directory bind-mounted into two processes running as
|
||||
@@ -544,4 +606,73 @@ mod tests {
|
||||
);
|
||||
assert!(body.contains("/AGENTS.md"));
|
||||
}
|
||||
|
||||
fn seed(dir: &std::path::Path) {
|
||||
let g = |args: &[&str]| {
|
||||
std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(dir)
|
||||
.args(args)
|
||||
.output()
|
||||
.unwrap();
|
||||
};
|
||||
g(&["init", "--quiet"]);
|
||||
g(&["config", "user.email", "[email protected]"]);
|
||||
g(&["config", "user.name", "T"]);
|
||||
std::fs::write(dir.join("README.md"), "# base\n").unwrap();
|
||||
g(&["add", "."]);
|
||||
g(&["commit", "--quiet", "-m", "base"]);
|
||||
record_base_commit(dir);
|
||||
}
|
||||
|
||||
/// A checkout mid-mission must not be mistaken for a cold one.
|
||||
///
|
||||
/// `ensure_checkout` runs per phase launch and resets on the reuse path.
|
||||
/// Mission `019fc444` lost phase 0's committed file that way — phase 1
|
||||
/// started from `origin/main` and never saw it. These are the three states
|
||||
/// that path has to tell apart.
|
||||
#[test]
|
||||
fn local_work_is_recognized_before_a_checkout_is_reset() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let repo = tmp.path();
|
||||
seed(repo);
|
||||
|
||||
assert!(
|
||||
!has_local_work(repo),
|
||||
"a freshly cloned checkout has no work and may be refreshed"
|
||||
);
|
||||
|
||||
// An agent that wrote files and did not commit: dirty tree, HEAD put.
|
||||
std::fs::write(repo.join("ALPHA.md"), "ALPHA\n").unwrap();
|
||||
assert!(has_local_work(repo), "uncommitted agent output is work");
|
||||
|
||||
// An agent (or delivery) that committed: clean tree, HEAD moved. This
|
||||
// is the shape that was actually being destroyed, because a hard reset
|
||||
// leaves untracked files alone but removes tracked ones.
|
||||
for args in [
|
||||
vec!["add", "ALPHA.md"],
|
||||
vec!["commit", "--quiet", "-m", "phase 0"],
|
||||
] {
|
||||
std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo)
|
||||
.args(&args)
|
||||
.output()
|
||||
.unwrap();
|
||||
}
|
||||
let status = std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo)
|
||||
.args(["status", "--porcelain"])
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(
|
||||
String::from_utf8_lossy(&status.stdout).trim().is_empty(),
|
||||
"the commit left a clean tree — the case a dirty-tree check misses"
|
||||
);
|
||||
assert!(
|
||||
has_local_work(repo),
|
||||
"committed phase output must not be reset away"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user