From 23ab52244550bffc8d56aa323ef1f52c63d09a74 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 05:32:08 +0000 Subject: [PATCH] agent: propose_patch ergonomics from the phase2-comrak-004 audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Standalone-proposal semantics: workspace resets to pristine before every apply, so probe patches can't accumulate into a tested tree the ledger's patch field doesn't describe. - Friendly errors: explicit trailing-newline check, and git apply's stderr relayed on refusal instead of a generic message. - Tool docs + task prompt updated to match. - phase2-comrak-004 audited: honest rejected-bench (median 1.0124, CI [1.0011, 1.0267] — real but under the 2% bar), Mutex pre-flagged for human review, cumulative-probe state disclosed in the hypothesis. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EcKPWQ3MQD4NdEHwar96u2 --- agent/hotpath_agent/loop.py | 4 +- agent/hotpath_agent/mcp_server.py | 9 +++-- crates/harnessd/src/main.rs | 64 ++++++++++++++++++++++++------ results/ledger.sqlite | Bin 53248 -> 57344 bytes 4 files changed, 61 insertions(+), 16 deletions(-) diff --git a/agent/hotpath_agent/loop.py b/agent/hotpath_agent/loop.py index b03bf0f..a431a3d 100644 --- a/agent/hotpath_agent/loop.py +++ b/agent/hotpath_agent/loop.py @@ -86,7 +86,9 @@ async def run_attempt(target: str, run_id: str, max_turns: int, repo_root: str) f"3. Read the relevant source with read_target_source — for large files pass " f"offset/limit and page through in windows (do NOT attempt any shell). State your " f"hypothesis, then propose_patch (unified diff, a/ b/ prefixes, paths relative to the " - f"target workspace root).\n" + f"target workspace root, trailing newline required). Each proposal is STANDALONE — " + f"the workspace resets to pristine before every apply, so always submit the complete " + f"patch, never an increment on a prior proposal.\n" f"4. run_verdict with run_id '{run_id}', the class number, and the hotspot string. " f"It launches a multi-minute pipeline and returns immediately.\n" f"5. Poll read_verdict('{run_id}') until it returns a verdict (not a 'running' status). " diff --git a/agent/hotpath_agent/mcp_server.py b/agent/hotpath_agent/mcp_server.py index a7b04f7..dbb51b6 100644 --- a/agent/hotpath_agent/mcp_server.py +++ b/agent/hotpath_agent/mcp_server.py @@ -50,9 +50,12 @@ def read_target_source(target: str, path: str, offset: int = 0, limit: int = 400 @mcp.tool() def propose_patch(target: str, diff: str, hypothesis: str) -> str: - """Submit a unified git diff (a/ b/ prefixes, paths relative to the target workspace) - plus the hypothesis it tests. The harness applies it only after a path-allowlist - check. Returns a patch_id, or an error string if refused.""" + """Submit a unified git diff (a/ b/ prefixes, paths relative to the target + workspace, MUST end with a trailing newline) plus the hypothesis it tests. + Each proposal is STANDALONE: the workspace is reset to pristine before the + diff is applied, so always submit the complete patch — never an increment + on top of an earlier proposal. The harness applies it only after a + path-allowlist check. Returns a patch_id, or an error string if refused.""" return harness.propose_patch(target, diff, hypothesis) diff --git a/crates/harnessd/src/main.rs b/crates/harnessd/src/main.rs index 580222f..a914edb 100644 --- a/crates/harnessd/src/main.rs +++ b/crates/harnessd/src/main.rs @@ -222,6 +222,13 @@ fn handle(root: &Path, line: &str) -> Result { let diff = field(&req, "diff")?; let hypothesis = field(&req, "hypothesis")?; check_target(t)?; + // git rejects a diff without a final newline with an opaque + // "corrupt patch" — say it plainly (phase2-comrak-004 burned + // several turns rediscovering this). + ensure!( + diff.ends_with('\n'), + "diff must end with a trailing newline (git would report 'corrupt patch' at EOF)" + ); // Allowlist: every path named by the diff must be a safe // relative path (the git -C below roots them in the target // workspace; nothing outside it is reachable). @@ -238,18 +245,51 @@ fn handle(root: &Path, line: &str) -> Result { ); } let ws = root.join(format!("targets/{t}/workspace")); - let mut check = Command::new("git") - .args(["-C", ws.to_str().unwrap(), "apply", "--check", "-"]) - .stdin(std::process::Stdio::piped()) - .spawn()?; - check.stdin.take().unwrap().write_all(diff.as_bytes())?; - ensure!(check.wait()?.success(), "diff does not apply cleanly"); - let mut apply = Command::new("git") - .args(["-C", ws.to_str().unwrap(), "apply", "-"]) - .stdin(std::process::Stdio::piped()) - .spawn()?; - apply.stdin.take().unwrap().write_all(diff.as_bytes())?; - ensure!(apply.wait()?.success(), "git apply failed"); + // Every proposal stands alone: reset the workspace to HEAD + // first so probe/iteration patches cannot accumulate into a + // tested tree the ledger's patch field doesn't describe + // (phase2-comrak-004 audit finding). + ensure!( + Command::new("git") + .args(["-C", ws.to_str().unwrap(), "checkout", "--", "."]) + .status()? + .success(), + "workspace reset failed" + ); + ensure!( + Command::new("git") + .args(["-C", ws.to_str().unwrap(), "clean", "-fdq"]) + .status()? + .success(), + "workspace clean failed" + ); + let run_git_apply = |check: bool| -> Result { + let mut args = vec!["-C", ws.to_str().unwrap(), "apply"]; + if check { + args.push("--check"); + } + args.push("-"); + let mut child = Command::new("git") + .args(&args) + .stdin(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn()?; + child.stdin.take().unwrap().write_all(diff.as_bytes())?; + Ok(child.wait_with_output()?) + }; + let checked = run_git_apply(true)?; + ensure!( + checked.status.success(), + "diff does not apply cleanly against the pristine workspace \ + (proposals are standalone; prior proposals were reset): {}", + String::from_utf8_lossy(&checked.stderr).trim() + ); + let applied = run_git_apply(false)?; + ensure!( + applied.status.success(), + "git apply failed: {}", + String::from_utf8_lossy(&applied.stderr).trim() + ); let patch_id = format!("{:x}", Sha256::digest(diff.as_bytes()))[..12].to_string(); let pending = root.join("results/pending"); std::fs::create_dir_all(&pending)?; diff --git a/results/ledger.sqlite b/results/ledger.sqlite index 0c1d7affa0c35fb79aae7563138de7653d70dc17..d2fbb0e82b0143afc1e95c25e0c4000c2b39f0e3 100644 GIT binary patch delta 5324 zcmbVQO>Z056_xB5vDL_|VdcYZ(A)^HLc&98_$hK^%T67)Nde_cLV+T6$>GeSIMr}w zJo84fV!LC*MS*@S+Ds4>0gARjfdEcYz;%FbvPin@56G^YY`SPRU9{_-_udT2&`w%Z zfGv{my*u}vd(OF+y7{}qH$OW3%da0fF*i4N{8w~-{>0thAD;Tg;_*-B3jZj4RQROu zZsD!M#lk`1`-O7h@xuJ^PmX_f?`itrcsjqjbf~d-@3YDi^T!tpY_9OH!ruzNF8sW} z?tN9?Dl9(o(5d-(%O8d2?vcfN;rTai9zAsE(A>huhYsDn_3!!hyN$)A)#j!;4CCFqI|EZKEh_P$GrNyvomz@58JZc8-}qTI-GTf<0YFu zz4;~uEJRi|EB9UB+NH#GJZ6TZ;%qTa+}~v$MV9GNv0kk-TBpe$&~sG^nQ7C=i6Rzh zRBcvTZMpJIYg9V&yw9V+=LVgq4IpV-1=YD8<)=*p~!EGvZ(KVxpnp8C19Urg_ zM~)@YVy-iCJd-orK4AE5ljH4~p<{ZS_Wf|Tq|pc;;{(?{Adl@EWIKZa3mG;H6)Lns zBW4Xv&xuB~7tu5~l3kFaMny|ypi$sQJP3V@A))Uw>p6BMW_ZXXsM`4@Aw|S$9` zKJL@C%)qo9en7rQeCR|ph&_ute&Rh7kstE_2=`-=c&S30dA0@(9MA4Xl4RZfL06Ka z=$hV6i4er3AP!j`%M(Q5Lq^VM6mzrhvME}Lf`~P7R&h+D1I7~(CS>I>_5{IA#6mF@ zDFqk~v;e|FO^&ytkp)$ip%Ugr4tMsL;;Fv}dHiTi*R$un%47k6Ar1VH7?uxcz&s1( z6Wk}o{i5T!3Ft3s%6+~(JVX`NQO1$)yp9WUk#rQhTxV)bxC55Nk&_thwAUz2G;!E48i8Vwv!d!$9UEkl01B%!P8gVSt6b!*3 zwj<>Opv5I&aDsUIi8p2sTm?7IxQGY!4>)RVLr9Kgx_FDUya<&Ly!9&d+P2_vU~)hN zV+>rzQScyiP;w}!4{YRG4W7|%nFGDkX+aG`2m4LiM#{KyLcKTwjkqn1*D?%Q!1W$T z9QYw$m$)E&UlK(~9(618kl|7gy3}*%L|ovCbfmV_b7AP>y~(psmCJPLYJiI~+o0T7 z5~M7VkKNr|KDA0~^Fe-Qk~NGyxlI__r$2dvYMF5EYcv_F;$2j>^LrOelwJ&tdye&5jw`u5dS)MIRA|nUbJ( zNNFp1`O-!D%R6layn23axWLyM`Vc=3TBfKQZX`ixxx<1fpk-tZ7MGOW`LV0~?$<_$P7nGpLafRpG zaHViZY$&BietiCDt**7iRC4d*zn(vm;ME+R!Dm&SE?ih3@sqi0#Lzysq|>4eYibSD z7_qKg+7>S-D^?VBWh>|+rGazRpv+@do{6|D1f;T|(dufhH^UeveB>I2>~>rHz-5>p z&=%OsG6DpVVIWi_V#0EOG)+fbI3rR<&`o895%p$#Mz)AvR@BHqdkmzn3rOJh-oU4x zXaYTDXybr*mJot6!>p0OV|{lIi7AY*NWBSM74=fMWltkN9Ea+?=z9zbR^3PqG$5xn zOc2Ub24r5XsQp0*W={C~4=6z9eQEeKt^vSbi-KBZk-xNw2W^@i4=GxfICXpZX) z+!6uag9~mLLY7^d4NSC^sEbiTcs1^pQ9&NXe-EI<%-Q>njJcAG$s!Qa6aabP2LNFh zNTZlFGxY|INl{ZA0}9!%9Q#%J7AmMbO$8Jr%H1g8)Dyj>Cx;S@PLf1W&*brkx8g*#2JqzMJLGzlTywoNJ}<0LA_~OYI5cj?ROGSc#E2P)O<2Ee5es4 zyD(TP!XflJ?K9OM#mKO5M%I57>X{lIBt3DjZIQu@mL@&BH{G=Jo;^uTJa01D$PSJk z>v8ndaTmw+639+9a&%fC_PJh;jgx??sb>-yph=^u*&JA0iI_Xk4Si-%=O#}tcEqaylT zu~dn@{m=}GrF9isG7V7@--+i($ohuHeSKj?vN^d{(l_{snKTFd)r?7S` z?S6P9{o|cuPej+%6cX^zFxZu%7>5qLQYk;ssxaAVi_6N;vW65g9*3SFf5yH+=#RiZ zR2La6m&DcPZ=`p3mqaGZ3PCYJhYFes_q!+|Gzi;}$Yfg&3zUBF-qY!C-+Lqh#uJnk z!ZHg{!o0Y70k-@RE8=qrwd6`(K7pmmQux|y*mHTtgYt?9ASVx`^cQ!IE~t~D8K$@H zoP27gC=;n*TCvf5u_sX!>H1-pxe+T~2gFls66s7w30%|hx)|TN7{4cvzoA~@m$Ey^ zAvZMM9ipY?ikPTC){ZA8_&uuyDUes?mxuA!gG*uPhv4DFSrx_#J&1e6rWXa$5@op} zaaL`oQ`M>sI#aDzwKepeWD-vt=?bprvwtS$X)|9(Pb-U!;zy_(8*NY#nG2`i{tIg?n*j~(y6aT74tu^XRT~A*bEj`+(HmlW6 zyWZ6F3ec*yYVCR_z4iX%$7=P?8a8(H^!M*Cb!zpsX06()cI3WVt<&h}waeOa6yUo} z9CSy_#@~XLjcP@&*6QtAr(Ua8>v~mB|9Ru&GJZE(YkIR&uh!d*wuW7`R;!6UYYqHe asN1idCzW&oUAAaqX!>9c}fA2pS4Cu80 delta 105 zcmV-v0G9uNzypAw1CSd95C8xG4Ur%}0T8iZq#qg#4(tFA>kq&W)(@)>mJdS@c@JC< zEe{tD0ka?=whoby7_;my7y%3n4{`tx^$*<-m=BAy5-?K_kx&n_azB!Q1_27e01Cme Lfwu~?!OA}cE}9@*