feat(missions): an operator button to merge a mission's branch into main
`MergePolicy::Never` — the default for anything touching code — has always meant
"do not merge on your own", deferring to a human. There was no way for that human
to say yes: `auto_merge` was reachable only from the paper-harvest path, no
workflow template declares `merge_policy`, and every mission ended at a branch.
`POST /api/missions/{id}/merge` is that yes, with a button on the artifacts tab.
The additive-only gate does NOT apply here, deliberately: an operator reading a
code change is exactly the judgement the policy was holding out for.
What is not waived:
- the branch comes from the artifact delivery RECORDED, not rebuilt from the
mission id, and must have `pushed: true`. A phase that never pushed shows no
button instead of one that cannot work.
- an empty branch is refused. A button reporting success for merging nothing
is worse than no button.
- a conflict refuses, aborts, and leaves the repo clean rather than forcing.
It works in a FRESH CLONE under `_merge/<mission>`, never the mission checkout:
that directory is reaped on a timer after a mission ends, so a merge using it
would succeed right after a run and fail inexplicably an hour later. The clone is
made by the server process, so nothing runs as root and ordinary cleanup works —
unlike the copies in `root_copy`.
`merge_and_push` is split out so the operator path and the automatic path run the
SAME git commands; only the gates differ. A test asserts both call it, that the
operator path does not re-apply the additive gate it exists to bypass, and that
it still refuses an empty branch.
Harness 43/43 across all five recipes before this change, with `_gate`, `_bench`
and `_verify` all at zero.
246 lib tests, 20 binaries, 89 frontend tests, clean build.
This commit is contained in:
@@ -166,11 +166,35 @@ pub async fn try_merge(
|
|||||||
return Ok(MergeOutcome::refused("branch adds nothing"));
|
return Ok(MergeOutcome::refused("branch adds nothing"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
merge_and_push(repo, push_url, branch, base, "auto-merge")
|
||||||
|
.await
|
||||||
|
.map(|o| match o.merged {
|
||||||
|
true => MergeOutcome {
|
||||||
|
merged: true,
|
||||||
|
reason: format!("additive-only and verified; merged into {base}"),
|
||||||
|
},
|
||||||
|
false => o,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The git half of a merge, with no policy in it.
|
||||||
|
///
|
||||||
|
/// Split out so an OPERATOR-approved merge runs exactly the same commands as an
|
||||||
|
/// automatic one — fetch the base as the remote has it, merge onto that, push.
|
||||||
|
/// The gates differ; the mechanics must not, or the rarely-taken path is the one
|
||||||
|
/// that breaks.
|
||||||
|
async fn merge_and_push(
|
||||||
|
repo: &Path,
|
||||||
|
push_url: &str,
|
||||||
|
branch: &str,
|
||||||
|
base: &str,
|
||||||
|
label: &str,
|
||||||
|
) -> Result<MergeOutcome, String> {
|
||||||
// Merge onto the freshly fetched base rather than a local branch.
|
// Merge onto the freshly fetched base rather than a local branch.
|
||||||
git(repo, &["checkout", "-B", base, "FETCH_HEAD"]).await?;
|
git(repo, &["checkout", "-B", base, "FETCH_HEAD"]).await?;
|
||||||
if let Err(e) = git(
|
if let Err(e) = git(
|
||||||
repo,
|
repo,
|
||||||
&["merge", "--no-ff", "-m", &format!("auto-merge {branch}"), branch],
|
&["merge", "--no-ff", "-m", &format!("{label} {branch}"), branch],
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
@@ -184,14 +208,88 @@ pub async fn try_merge(
|
|||||||
git(repo, &["push", push_url, &format!("HEAD:refs/heads/{base}")]).await?;
|
git(repo, &["push", push_url, &format!("HEAD:refs/heads/{base}")]).await?;
|
||||||
Ok(MergeOutcome {
|
Ok(MergeOutcome {
|
||||||
merged: true,
|
merged: true,
|
||||||
reason: format!("additive-only and verified; merged into {base}"),
|
reason: format!("merged into {base}"),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Merge a delivered branch because an OPERATOR asked for it.
|
||||||
|
///
|
||||||
|
/// `MergePolicy::Never` means "do not merge on your own" — it defers to a human,
|
||||||
|
/// and this is that human. So the additive-only test does not apply: an operator
|
||||||
|
/// looking at a code change is exactly the judgement the policy was holding out
|
||||||
|
/// for.
|
||||||
|
///
|
||||||
|
/// What is NOT waived:
|
||||||
|
///
|
||||||
|
/// - the branch must exist on the remote and differ from the base, so the button
|
||||||
|
/// cannot report success for a merge of nothing;
|
||||||
|
/// - a conflict refuses and leaves the repo clean, rather than forcing;
|
||||||
|
/// - the work happens in a FRESH CLONE, never the mission checkout — that
|
||||||
|
/// directory is reaped on a timer after the mission ends, so a merge that
|
||||||
|
/// depended on it would work right after a run and mysteriously fail later.
|
||||||
|
pub async fn merge_on_operator_approval(
|
||||||
|
workdir: &Path,
|
||||||
|
push_url: &str,
|
||||||
|
branch: &str,
|
||||||
|
base: &str,
|
||||||
|
) -> Result<MergeOutcome, String> {
|
||||||
|
git(workdir, &["fetch", push_url, base]).await?;
|
||||||
|
git(workdir, &["fetch", push_url, branch]).await?;
|
||||||
|
git(workdir, &["branch", "-f", branch, "FETCH_HEAD"]).await?;
|
||||||
|
git(workdir, &["fetch", push_url, base]).await?;
|
||||||
|
|
||||||
|
let diff = git(
|
||||||
|
workdir,
|
||||||
|
&["diff", "--name-status", &format!("FETCH_HEAD...{branch}")],
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
if diff.trim().is_empty() {
|
||||||
|
return Ok(MergeOutcome::refused(
|
||||||
|
"branch has nothing the base does not already have",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
merge_and_push(workdir, push_url, branch, base, "merge mission branch").await
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
/// An operator merge and an automatic one must run the SAME git commands.
|
||||||
|
///
|
||||||
|
/// The gates differ — that is the whole point — but if the mechanics
|
||||||
|
/// diverged, the rarely-taken path would be the untested one. Both go
|
||||||
|
/// through `merge_and_push`.
|
||||||
|
#[test]
|
||||||
|
fn both_merge_paths_share_the_same_mechanics() {
|
||||||
|
let src = include_str!("auto_merge.rs");
|
||||||
|
let calls = src.matches("merge_and_push(").count();
|
||||||
|
// one definition + one call from each path
|
||||||
|
assert!(
|
||||||
|
calls >= 3,
|
||||||
|
"expected try_merge and merge_on_operator_approval to both call \
|
||||||
|
merge_and_push, found {calls} mention(s)"
|
||||||
|
);
|
||||||
|
// And the operator path must NOT re-implement the policy gate it exists
|
||||||
|
// to bypass — if this string appears there, the button is a no-op.
|
||||||
|
let op = src
|
||||||
|
.split("pub async fn merge_on_operator_approval")
|
||||||
|
.nth(1)
|
||||||
|
.unwrap_or("");
|
||||||
|
let body = op.split("\n}").next().unwrap_or("");
|
||||||
|
assert!(
|
||||||
|
!body.contains("MergePolicy::AdditiveOnly"),
|
||||||
|
"the operator path must not apply the additive-only gate"
|
||||||
|
);
|
||||||
|
// It must still refuse an empty branch: a button that reports success
|
||||||
|
// for merging nothing is worse than no button.
|
||||||
|
assert!(
|
||||||
|
body.contains("nothing the base does not already have"),
|
||||||
|
"the operator path must refuse an empty branch"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn only_pure_additions_qualify() {
|
fn only_pure_additions_qualify() {
|
||||||
assert!(non_additive_changes("A\t60 Papers/a.md\nA\t60 Papers/b.md\n").is_empty());
|
assert!(non_additive_changes("A\t60 Papers/a.md\nA\t60 Papers/b.md\n").is_empty());
|
||||||
|
|||||||
@@ -491,6 +491,7 @@ pub fn router(state: AppState) -> Router {
|
|||||||
axum::routing::patch(routes::missions::set_status),
|
axum::routing::patch(routes::missions::set_status),
|
||||||
)
|
)
|
||||||
.route("/api/missions/{id}/refine", post(routes::missions::refine))
|
.route("/api/missions/{id}/refine", post(routes::missions::refine))
|
||||||
|
.route("/api/missions/{id}/merge", post(routes::missions::merge_branch))
|
||||||
.route(
|
.route(
|
||||||
"/api/missions/{id}/artifacts/{artifact_id}/content",
|
"/api/missions/{id}/artifacts/{artifact_id}/content",
|
||||||
get(routes::missions::artifact_content),
|
get(routes::missions::artifact_content),
|
||||||
|
|||||||
@@ -117,7 +117,7 @@ const FORGE_HOST: &str = "git.redclaw.dev";
|
|||||||
/// port, a different case in the host — silently came back unauthenticated, and
|
/// port, a different case in the host — silently came back unauthenticated, and
|
||||||
/// the first symptom was git opening `/dev/tty` several layers later. Tracing
|
/// the first symptom was git opening `/dev/tty` several layers later. Tracing
|
||||||
/// #55 cost hours to a failure whose cause was one unlogged early return.
|
/// #55 cost hours to a failure whose cause was one unlogged early return.
|
||||||
pub(crate) struct Authed {
|
pub struct Authed {
|
||||||
pub url: String,
|
pub url: String,
|
||||||
/// `None` when the token was applied; otherwise WHY it was not.
|
/// `None` when the token was applied; otherwise WHY it was not.
|
||||||
pub unauthenticated: Option<String>,
|
pub unauthenticated: Option<String>,
|
||||||
@@ -163,7 +163,7 @@ fn host_of(url: &str) -> Option<&str> {
|
|||||||
/// token, a host that is not ours, or a shape a token cannot be injected into.
|
/// token, a host that is not ours, or a shape a token cannot be injected into.
|
||||||
/// The token is never logged — only the rewritten URL is passed to git, via
|
/// The token is never logged — only the rewritten URL is passed to git, via
|
||||||
/// argv.
|
/// argv.
|
||||||
pub(crate) fn with_ambient_auth(url: &str) -> Authed {
|
pub fn with_ambient_auth(url: &str) -> Authed {
|
||||||
auth_with_token(url, std::env::var("GITEA_TOKEN").ok().as_deref())
|
auth_with_token(url, std::env::var("GITEA_TOKEN").ok().as_deref())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -397,6 +397,107 @@ pub async fn artifact_content(
|
|||||||
})))
|
})))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// POST /api/missions/{id}/merge — merge this mission's branch into the base.
|
||||||
|
///
|
||||||
|
/// The operator's button. `MergePolicy::Never` — the default for anything that
|
||||||
|
/// touches code — means "do not merge on your own", deferring to a human; this
|
||||||
|
/// endpoint is that human saying yes. So the additive-only test does not apply
|
||||||
|
/// here, and deliberately so.
|
||||||
|
///
|
||||||
|
/// It works in a FRESH CLONE under `_merge/<mission>`, never the mission
|
||||||
|
/// checkout: that directory is reaped on a timer after a mission ends, so a
|
||||||
|
/// merge that used it would succeed right after a run and fail inexplicably an
|
||||||
|
/// hour later. The clone is made by the server process, so nothing here runs as
|
||||||
|
/// root and the ordinary cleanup works — unlike the copies in `root_copy`.
|
||||||
|
pub async fn merge_branch(
|
||||||
|
State(state): State<AppState>,
|
||||||
|
Authed(user): Authed,
|
||||||
|
Path(id): Path<Uuid>,
|
||||||
|
) -> Result<Json<serde_json::Value>, ApiError> {
|
||||||
|
let mission = cm_db::repo::missions::get(&state.pool, id, user.workspace_id.as_uuid())
|
||||||
|
.await?
|
||||||
|
.ok_or(ApiError::NotFound)?;
|
||||||
|
let repo_id = mission.repo_id.ok_or(ApiError::BadRequest)?;
|
||||||
|
let repo = cm_db::repo::repos::get(&state.pool, repo_id, user.workspace_id)
|
||||||
|
.await
|
||||||
|
.map_err(|_| ApiError::NotFound)?;
|
||||||
|
let clone_url = repo.clone_url.as_deref().ok_or(ApiError::BadRequest)?;
|
||||||
|
let base = repo.default_branch.as_deref().unwrap_or("main");
|
||||||
|
|
||||||
|
// The branch is whatever delivery actually pushed — read from the artifact
|
||||||
|
// it recorded, not reconstructed from the mission id. A phase that never
|
||||||
|
// pushed has no branch, and that must be a refusal rather than a guess.
|
||||||
|
let artifacts = cm_db::repo::missions::artifacts_for(&state.pool, id).await?;
|
||||||
|
let delivered = artifacts.iter().rev().find_map(|a| {
|
||||||
|
let m = a.metadata.as_object()?;
|
||||||
|
let branch = m.get("branch")?.as_str()?.to_string();
|
||||||
|
(m.get("pushed").and_then(|v| v.as_bool()) == Some(true)).then_some(branch)
|
||||||
|
});
|
||||||
|
let Some(branch) = delivered else {
|
||||||
|
return Ok(Json(serde_json::json!({
|
||||||
|
"merged": false,
|
||||||
|
"reason": "this mission has no pushed branch to merge",
|
||||||
|
})));
|
||||||
|
};
|
||||||
|
|
||||||
|
let auth = crate::mission_workspace::with_ambient_auth(clone_url);
|
||||||
|
let workdir = crate::mission_workspace::missions_root()
|
||||||
|
.join("_merge")
|
||||||
|
.join(id.to_string());
|
||||||
|
let _ = tokio::fs::remove_dir_all(&workdir).await;
|
||||||
|
if let Some(parent) = workdir.parent() {
|
||||||
|
let _ = tokio::fs::create_dir_all(parent).await;
|
||||||
|
}
|
||||||
|
let clone = tokio::process::Command::new("git")
|
||||||
|
.args(["clone", "--quiet", &auth.url])
|
||||||
|
.arg(&workdir)
|
||||||
|
.env("GIT_TERMINAL_PROMPT", "0")
|
||||||
|
.output()
|
||||||
|
.await
|
||||||
|
.map_err(|_| ApiError::Internal)?;
|
||||||
|
if !clone.status.success() {
|
||||||
|
eprintln!(
|
||||||
|
"missions::merge_branch: clone for {id} failed: {}",
|
||||||
|
String::from_utf8_lossy(&clone.stderr)
|
||||||
|
.chars()
|
||||||
|
.take(300)
|
||||||
|
.collect::<String>()
|
||||||
|
);
|
||||||
|
return Ok(Json(serde_json::json!({
|
||||||
|
"merged": false,
|
||||||
|
"reason": "could not clone the repository to merge",
|
||||||
|
})));
|
||||||
|
}
|
||||||
|
|
||||||
|
let outcome =
|
||||||
|
crate::auto_merge::merge_on_operator_approval(&workdir, &auth.url, &branch, base).await;
|
||||||
|
let _ = tokio::fs::remove_dir_all(&workdir).await;
|
||||||
|
|
||||||
|
match outcome {
|
||||||
|
Ok(o) => {
|
||||||
|
eprintln!(
|
||||||
|
"missions::merge_branch: mission {id} branch {branch} -> {base}: {}",
|
||||||
|
o.reason
|
||||||
|
);
|
||||||
|
Ok(Json(serde_json::json!({
|
||||||
|
"merged": o.merged,
|
||||||
|
"reason": o.reason,
|
||||||
|
"branch": branch,
|
||||||
|
"base": base,
|
||||||
|
})))
|
||||||
|
}
|
||||||
|
Err(e) => {
|
||||||
|
eprintln!("missions::merge_branch: mission {id} failed: {e}");
|
||||||
|
Ok(Json(serde_json::json!({
|
||||||
|
"merged": false,
|
||||||
|
"reason": format!("merge failed: {e}"),
|
||||||
|
"branch": branch,
|
||||||
|
"base": base,
|
||||||
|
})))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// POST /api/missions/{id}/benchmark — run the benchmark harness
|
/// POST /api/missions/{id}/benchmark — run the benchmark harness
|
||||||
/// against a phase. Slot='baseline' records iteration 0's
|
/// against a phase. Slot='baseline' records iteration 0's
|
||||||
/// before_metrics; slot='after' with iteration=N records the
|
/// before_metrics; slot='after' with iteration=N records the
|
||||||
|
|||||||
@@ -11,11 +11,13 @@
|
|||||||
// research phase can leave dozens of documents.
|
// research phase can leave dozens of documents.
|
||||||
|
|
||||||
import { useCallback, useState } from "react";
|
import { useCallback, useState } from "react";
|
||||||
import { FileText } from "lucide-react";
|
import { FileText, GitMerge } from "lucide-react";
|
||||||
|
|
||||||
import {
|
import {
|
||||||
getArtifactContent,
|
getArtifactContent,
|
||||||
|
mergeMissionBranch,
|
||||||
type ArtifactContent,
|
type ArtifactContent,
|
||||||
|
type MergeResult,
|
||||||
type MissionDetail,
|
type MissionDetail,
|
||||||
} from "@/lib/api/missions";
|
} from "@/lib/api/missions";
|
||||||
import { MarkdownBlock } from "./MarkdownBlock";
|
import { MarkdownBlock } from "./MarkdownBlock";
|
||||||
@@ -32,6 +34,8 @@ export function MissionArtifacts({
|
|||||||
}) {
|
}) {
|
||||||
const missionId = mission.id;
|
const missionId = mission.id;
|
||||||
const artifacts = mission.artifacts;
|
const artifacts = mission.artifacts;
|
||||||
|
const [merging, setMerging] = useState(false);
|
||||||
|
const [merge, setMerge] = useState<MergeResult | null>(null);
|
||||||
const [openId, setOpenId] = useState<string | null>(null);
|
const [openId, setOpenId] = useState<string | null>(null);
|
||||||
const [text, setText] = useState<ArtifactContent | null>(null);
|
const [text, setText] = useState<ArtifactContent | null>(null);
|
||||||
const [loading, setLoading] = useState(false);
|
const [loading, setLoading] = useState(false);
|
||||||
@@ -65,6 +69,32 @@ export function MissionArtifacts({
|
|||||||
[missionId, openId],
|
[missionId, openId],
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// The branch delivery actually pushed, read from the artifact that recorded
|
||||||
|
// it — not rebuilt from the mission id, so a phase that never pushed shows no
|
||||||
|
// button rather than a button that cannot work.
|
||||||
|
const branch = artifacts
|
||||||
|
.map((a) => (a.metadata ?? {}) as Record<string, unknown>)
|
||||||
|
.filter((m) => m.pushed === true)
|
||||||
|
.map((m) => (typeof m.branch === "string" ? m.branch : null))
|
||||||
|
.filter(Boolean)
|
||||||
|
.pop() as string | null;
|
||||||
|
|
||||||
|
const doMerge = async () => {
|
||||||
|
if (merging) return;
|
||||||
|
setMerging(true);
|
||||||
|
setMerge(null);
|
||||||
|
try {
|
||||||
|
setMerge(await mergeMissionBranch(mission.id));
|
||||||
|
} catch (e) {
|
||||||
|
setMerge({
|
||||||
|
merged: false,
|
||||||
|
reason: e instanceof Error ? e.message : "merge request failed",
|
||||||
|
});
|
||||||
|
} finally {
|
||||||
|
setMerging(false);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
if (artifacts.length === 0) {
|
if (artifacts.length === 0) {
|
||||||
return (
|
return (
|
||||||
<div style={{ fontFamily: mono, fontSize: 11.5, color: "#8a8a92", padding: 12 }}>
|
<div style={{ fontFamily: mono, fontSize: 11.5, color: "#8a8a92", padding: 12 }}>
|
||||||
@@ -75,6 +105,42 @@ export function MissionArtifacts({
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<div style={{ display: "flex", flexDirection: "column", gap: 8 }}>
|
<div style={{ display: "flex", flexDirection: "column", gap: 8 }}>
|
||||||
|
{branch && (
|
||||||
|
<div
|
||||||
|
style={{
|
||||||
|
padding: 11,
|
||||||
|
borderRadius: 10,
|
||||||
|
border: "1px solid rgba(124,214,224,.22)",
|
||||||
|
background: "rgba(124,214,224,.05)",
|
||||||
|
display: "flex",
|
||||||
|
alignItems: "center",
|
||||||
|
gap: 10,
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
<GitMerge size={16} style={{ color: "#7cd6e0", flex: "none" }} />
|
||||||
|
<div style={{ flex: 1, minWidth: 0 }}>
|
||||||
|
<div style={{ fontSize: 13, color: "#f3f3f5", fontWeight: 500 }}>
|
||||||
|
Merge this mission’s work
|
||||||
|
</div>
|
||||||
|
<div style={{ fontFamily: mono, fontSize: 10.5, color: "#8a8a92" }}>
|
||||||
|
{merge ? merge.reason : `${branch} → default branch`}
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onClick={doMerge}
|
||||||
|
disabled={merging || merge?.merged === true}
|
||||||
|
style={{
|
||||||
|
...secondaryBtn,
|
||||||
|
padding: "5px 10px",
|
||||||
|
fontSize: 11,
|
||||||
|
opacity: merging || merge?.merged ? 0.6 : 1,
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
{merging ? "Merging…" : merge?.merged ? "Merged" : "Merge to main"}
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
{artifacts.map((a) => {
|
{artifacts.map((a) => {
|
||||||
const isOpen = openId === a.id;
|
const isOpen = openId === a.id;
|
||||||
return (
|
return (
|
||||||
|
|||||||
@@ -342,6 +342,19 @@ export const getMissionDocument = (
|
|||||||
`/api/missions/${id}/documents/${runId}/${index}`,
|
`/api/missions/${id}/documents/${runId}/${index}`,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
/** Result of an operator-approved merge of the mission branch into the base. */
|
||||||
|
export interface MergeResult {
|
||||||
|
merged: boolean;
|
||||||
|
reason: string;
|
||||||
|
branch?: string;
|
||||||
|
base?: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Merge this mission's delivered branch into the repo's default branch.
|
||||||
|
* Operator-triggered: the platform never does this on its own for code. */
|
||||||
|
export const mergeMissionBranch = (id: string) =>
|
||||||
|
api<MergeResult>(`/api/missions/${id}/merge`, { method: "POST" });
|
||||||
|
|
||||||
/** One artifact's text. Artifacts are FILES the phase left behind, which is a
|
/** One artifact's text. Artifacts are FILES the phase left behind, which is a
|
||||||
* different thing from `MissionDocument` — that is an agent's turn output, its
|
* different thing from `MissionDocument` — that is an agent's turn output, its
|
||||||
* account of the work rather than the work. Both are worth reading. */
|
* account of the work rather than the work. Both are worth reading. */
|
||||||
|
|||||||
Reference in New Issue
Block a user