From 8ebe10f5a47d6b873b5df6daa644349925381ca4 Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Thu, 16 Jul 2026 12:00:11 -0300 Subject: [PATCH 1/4] fix(harness): release held calls with hook-mutated arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A call held by a pre_trigger hook and released with harness::function::resolve {action: "execute"} ran the target with the model's ORIGINAL arguments recovered from the transcript — mutations from hooks that ran before the holder were dropped, and the remaining chain never resumed, breaking the harness.md § function::resolve promise that the pre_trigger chain resumes after the holding hook. - PreTriggerOutcome::Hold now carries the arguments as mutated up to the hold; the loop persists them on the call's checkpoint (held_arguments), so they survive the park durably. Post-trigger holds checkpoint the fully pre-mutated args for the same reason. - The execute release path prefers the checkpointed arguments (transcript recovery remains the fallback for records written before the field existed), resumes the pre_trigger chain AFTER the holder — hooks up to and including it already ran, so re-running would double-apply mutations and re-hold — and handles a resumed Deny/Hold like the loop. The filesystem scope stamp is still re-applied after the resumed chain. - A holder no longer bound at release runs no further hooks: the chain shape changed under the hold and the safe resume point is unknowable. Fixes iii-hq/workers#506 --- harness/src/deferred.rs | 109 +++++++++++++++++++--- harness/src/functions/function_trigger.rs | 1 + harness/src/hooks/runner.rs | 94 ++++++++++++++++++- harness/src/subagent.rs | 1 + harness/src/trigger.rs | 3 + harness/src/turn_loop.rs | 12 ++- harness/src/types/turn.rs | 7 ++ 7 files changed, 210 insertions(+), 17 deletions(-) diff --git a/harness/src/deferred.rs b/harness/src/deferred.rs index d069feede..059edd454 100644 --- a/harness/src/deferred.rs +++ b/harness/src/deferred.rs @@ -81,17 +81,98 @@ pub async fn resolve( }) } "execute" => { - // `execute` releases a `pre_trigger` hook-held call: invoke the - // target (the holding hook already decided to allow), run the - // post_trigger chain, append the result, and resume — bypassing - // the approver's own re-evaluation (harness.md § `function::resolve`). - if checkpoint.held_by.is_none() { + // `execute` releases a `pre_trigger` hook-held call: resume the + // pre_trigger chain AFTER the holding hook (earlier hooks and the + // holder's own decision already ran — harness.md § + // `function::resolve`), invoke the target, run the post_trigger + // chain, append the result, and resume the turn. + let Some(held_by) = checkpoint.held_by.clone() else { return Ok(not_resolved()); - } + }; let function_id = checkpoint.function_id.clone().unwrap_or_default(); - let arguments = find_call_arguments(deps, &record, &req.function_call_id) + // The checkpoint carries the arguments as mutated by the chain up + // to the hold (issue #506); transcript recovery — the model's + // ORIGINAL arguments — is only the fallback for records written + // before held arguments were persisted. + let arguments = match checkpoint.held_arguments.clone() { + Some(args) => args, + None => find_call_arguments(deps, &record, &req.function_call_id) + .await + .unwrap_or(Value::Null), + }; + let (arguments, pre_annotations) = match deps + .hooks + .run_pre_trigger( + &record, + record.step, + &req.function_call_id, + &function_id, + &arguments, + Some(&held_by), + ) .await - .unwrap_or(Value::Null); + { + crate::hooks::runner::PreTriggerOutcome::Continue { + arguments, + annotations, + } => (arguments, annotations), + crate::hooks::runner::PreTriggerOutcome::Deny(reason) => { + let data = crate::trigger::ResultData { + content: vec![ContentBlock::text(reason.clone())], + is_error: true, + details: json!({ "error": "hook_denied", "message": reason }), + }; + let entry_id = + ids::function_result_entry_id(&record.turn_id, &req.function_call_id); + let message = AgentMessage::FunctionResult(FunctionResultMessage { + role: FunctionResultRoleTag::FunctionResult, + function_call_id: req.function_call_id.clone(), + function_id, + content: data.content, + details: data.details, + is_error: data.is_error, + timestamp: AgentMessage::now_ms(), + }); + session + .append( + &record.session_id, + &message, + Some(&entry_id), + None, + Some(&json!({ "turn_id": record.turn_id })), + ) + .await?; + if let Some(cp) = record.calls.get_mut(&req.function_call_id) { + cp.state = CallState::Done; + cp.entry_id = Some(entry_id); + } + let turn_resumed = persist_and_maybe_resume(deps, &cfg, &mut record).await?; + return Ok(FunctionResolveResponse { + resolved: true, + turn_resumed, + }); + } + crate::hooks::runner::PreTriggerOutcome::Hold { + held_by, + arguments, + annotations: _, + } => { + // A later hook held the released call: re-park under the + // new holder, keeping the chain's mutations so far. + if let Some(cp) = record.calls.get_mut(&req.function_call_id) { + cp.held_by = Some(held_by); + cp.held_arguments = Some(arguments); + cp.pending_at = Some(AgentMessage::now_ms()); + } + record.status = TurnStatus::AwaitingFunctions; + record.updated_at = AgentMessage::now_ms(); + crate::state::put_turn(&deps.iii, &record, cfg.session_timeout_ms).await?; + return Ok(FunctionResolveResponse { + resolved: true, + turn_resumed: false, + }); + } + }; // The release path runs OUTSIDE the turn loop, so re-apply the // filesystem scope stamp the loop would have added: without it an // approved shell/coder call runs un-scoped (the session's picked @@ -163,6 +244,7 @@ pub async fn resolve( if let Some(cp) = record.calls.get_mut(&req.function_call_id) { cp.state = CallState::Pending; cp.held_by = Some(held_by); + cp.held_arguments = Some(arguments.clone()); cp.pending_at = Some(AgentMessage::now_ms()); } record.status = TurnStatus::AwaitingFunctions; @@ -178,6 +260,9 @@ pub async fn resolve( let entry_id = ids::function_result_entry_id(&record.turn_id, &req.function_call_id); let mut origin = serde_json::Map::new(); origin.insert("turn_id".into(), json!(record.turn_id)); + for (k, v) in pre_annotations { + origin.insert(k, v); + } for (k, v) in annotations { origin.insert(k, v); } @@ -244,9 +329,10 @@ async fn persist_and_maybe_resume( Ok(true) } -/// Recover a held call's (unwrapped) arguments from the transcript so -/// `execute` can invoke the target. Scans assistant messages for the -/// function_call block with this id. +/// Recover a held call's (unwrapped) arguments from the transcript — the +/// model's ORIGINAL arguments, without any hook-chain mutations. Fallback for +/// checkpoints written before `held_arguments` was persisted; scans assistant +/// messages for the function_call block with this id. async fn find_call_arguments( deps: &Deps, record: &crate::types::turn::TurnRecord, @@ -428,6 +514,7 @@ mod tests { None }, held_by: held_by.map(str::to_string), + held_arguments: None, pending_timeout_ms: timeout_ms, pending_at: Some(pending_at), } diff --git a/harness/src/functions/function_trigger.rs b/harness/src/functions/function_trigger.rs index fc14c6a92..108105704 100644 --- a/harness/src/functions/function_trigger.rs +++ b/harness/src/functions/function_trigger.rs @@ -123,6 +123,7 @@ pub async fn handle( &req.call.id, &req.call.function_id, &arguments, + None, ) .await { diff --git a/harness/src/hooks/runner.rs b/harness/src/hooks/runner.rs index a32b5ca40..3a0a9fb42 100644 --- a/harness/src/hooks/runner.rs +++ b/harness/src/hooks/runner.rs @@ -51,6 +51,9 @@ pub enum PreTriggerOutcome { Deny(String), Hold { held_by: String, + /// Arguments as mutated by the chain up to (not including) the holder, + /// checkpointed so a release executes the mutated call (issue #506). + arguments: Value, annotations: Map, }, } @@ -145,7 +148,10 @@ impl HookRegistry { } /// `pre_trigger`: deny / hold / rewrite arguments. Only bindings whose - /// `functions` globs match the target are consulted. + /// `functions` globs match the target are consulted. `resume_after` is + /// the hook-held release path (harness.md § `function::resolve`): the + /// chain resumes AFTER the named holder (see [`chain_slice`]), over the + /// checkpointed arguments it already mutated. pub async fn run_pre_trigger( &self, record: &TurnRecord, @@ -153,13 +159,11 @@ impl HookRegistry { call_id: &str, function_id: &str, arguments: &Value, + resume_after: Option<&str>, ) -> PreTriggerOutcome { let mut args = arguments.clone(); let mut annotations = Map::new(); - for binding in self.pre_trigger.ordered() { - if !functions_match(&binding, function_id) { - continue; - } + for binding in chain_slice(self.pre_trigger.ordered(), function_id, resume_after) { let mut input = self.envelope(HookPoint::PreTrigger, record, step); input["call"] = serde_json::json!({ "id": call_id, @@ -177,6 +181,7 @@ impl HookRegistry { HookOutcome::Hold => { return PreTriggerOutcome::Hold { held_by: binding.function_id.clone(), + arguments: args, annotations, } } @@ -325,6 +330,30 @@ fn merge(into: &mut Map, from: Map) { } } +/// The bindings a `pre_trigger` run consults for this target: the glob-matched +/// chain, cut down to everything AFTER `resume_after` when resuming a released +/// hold — hooks up to and including the holder already ran and mutated the +/// checkpointed arguments, so re-running them would double-apply mutations +/// (and re-hold). If the holder is no longer bound, nothing runs: the chain +/// shape changed under the hold and the safe resume point is unknowable. +fn chain_slice( + bindings: Vec, + function_id: &str, + resume_after: Option<&str>, +) -> Vec { + let mut matched: Vec = bindings + .into_iter() + .filter(|b| functions_match(b, function_id)) + .collect(); + let Some(holder) = resume_after else { + return matched; + }; + match matched.iter().position(|b| b.function_id == holder) { + Some(i) => matched.split_off(i + 1), + None => Vec::new(), + } +} + /// Whether a pre/post_trigger binding's `functions` globs match the target /// (no `functions` filter → all calls). pub(super) fn functions_match(binding: &HookBinding, function_id: &str) -> bool { @@ -402,6 +431,61 @@ mod tests { } } + fn binding(function_id: &str, priority: i64) -> HookBinding { + HookBinding { + function_id: function_id.into(), + functions: Some(vec!["shell::*".into()]), + priority, + timeout_ms: 5000, + fail_closed: true, + } + } + + fn ids(bindings: &[HookBinding]) -> Vec<&str> { + bindings.iter().map(|b| b.function_id.as_str()).collect() + } + + #[test] + fn chain_slice_without_resume_runs_the_matched_chain() { + let chain = vec![binding("mutate", 10), binding("gate", 20)]; + assert_eq!( + ids(&chain_slice(chain, "shell::run", None)), + vec!["mutate", "gate"] + ); + } + + #[test] + fn chain_slice_resumes_after_the_holder() { + let chain = vec![ + binding("mutate", 10), + binding("gate", 20), + binding("audit", 30), + ]; + // The mutator and the holder already ran — only `audit` remains. + assert_eq!( + ids(&chain_slice(chain.clone(), "shell::run", Some("gate"))), + vec!["audit"] + ); + // Holder last in the chain → nothing remains. + assert!(chain_slice(chain, "shell::run", Some("audit")).is_empty()); + } + + #[test] + fn chain_slice_runs_nothing_when_the_holder_was_unbound() { + let chain = vec![binding("mutate", 10), binding("audit", 30)]; + assert!(chain_slice(chain, "shell::run", Some("gone")).is_empty()); + } + + #[test] + fn chain_slice_positions_by_the_glob_matched_chain() { + // A holder bound to a different target never matches; the released + // call's matched chain decides the resume point. + let mut other = binding("gate", 20); + other.functions = Some(vec!["fs::*".into()]); + let chain = vec![binding("mutate", 10), other, binding("audit", 30)]; + assert!(chain_slice(chain, "shell::run", Some("gate")).is_empty()); + } + #[test] fn functions_filter_matches_globs() { let binding = HookBinding { diff --git a/harness/src/subagent.rs b/harness/src/subagent.rs index 64d92294f..f77ddb7db 100644 --- a/harness/src/subagent.rs +++ b/harness/src/subagent.rs @@ -88,6 +88,7 @@ pub async fn spawn_pending( Ok(PendingInfo { pending_timeout_ms: Some(pending_timeout_ms), held_by: None, + held_arguments: None, child_session_id: Some(child.session_id), child_turn_id: Some(child.turn_id), }) diff --git a/harness/src/trigger.rs b/harness/src/trigger.rs index 78429c6f0..e5866ee9e 100644 --- a/harness/src/trigger.rs +++ b/harness/src/trigger.rs @@ -35,6 +35,9 @@ pub enum TriggerResult { pub struct PendingInfo { pub pending_timeout_ms: Option, pub held_by: Option, + /// Hook holds only: the arguments as mutated by the chain up to the hold, + /// checkpointed so a release executes the mutated call (issue #506). + pub held_arguments: Option, pub child_session_id: Option, pub child_turn_id: Option, } diff --git a/harness/src/turn_loop.rs b/harness/src/turn_loop.rs index 184fd888f..4ab28da0d 100644 --- a/harness/src/turn_loop.rs +++ b/harness/src/turn_loop.rs @@ -833,6 +833,7 @@ pub async fn run_step( &call.id, &call.function_id, &trusted_call_args, + None, ) .await { @@ -869,10 +870,13 @@ pub async fn run_step( crate::state::put_turn(&deps.iii, &record, cfg.session_timeout_ms).await?; continue; } - crate::hooks::runner::PreTriggerOutcome::Hold { held_by, .. } => { + crate::hooks::runner::PreTriggerOutcome::Hold { + held_by, arguments, .. + } => { let info = trigger::PendingInfo { pending_timeout_ms: None, held_by: Some(held_by), + held_arguments: Some(arguments), child_session_id: None, child_turn_id: None, }; @@ -918,6 +922,7 @@ pub async fn run_step( child_session_id: None, child_turn_id: None, held_by: None, + held_arguments: None, pending_timeout_ms: None, pending_at: None, }, @@ -963,6 +968,9 @@ pub async fn run_step( let info = trigger::PendingInfo { pending_timeout_ms: None, held_by: Some(held_by), + // A post-trigger release re-invokes the target: keep + // the fully pre-mutated args, not the model originals. + held_arguments: Some(eff_args.clone()), child_session_id: None, child_turn_id: None, }; @@ -1537,6 +1545,7 @@ fn checkpoint_pending( child_session_id: info.child_session_id.clone(), child_turn_id: info.child_turn_id.clone(), held_by: info.held_by.clone(), + held_arguments: info.held_arguments.clone(), pending_timeout_ms: info.pending_timeout_ms, pending_at: Some(AgentMessage::now_ms()), }, @@ -1556,6 +1565,7 @@ fn mark_done(record: &mut TurnRecord, call_id: &str, entry_id: &str) { child_session_id: None, child_turn_id: None, held_by: None, + held_arguments: None, pending_timeout_ms: None, pending_at: None, }, diff --git a/harness/src/types/turn.rs b/harness/src/types/turn.rs index 13927cbb2..1c6c090ed 100644 --- a/harness/src/types/turn.rs +++ b/harness/src/types/turn.rs @@ -172,6 +172,12 @@ pub struct CallCheckpoint { pub child_turn_id: Option, #[serde(skip_serializing_if = "Option::is_none")] pub held_by: Option, + /// The call's arguments as mutated by the hook chain up to the hold, so a + /// release executes the mutated call — not the model's original arguments + /// re-read from the transcript. Absent on records written before this + /// field existed; release falls back to transcript recovery then. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub held_arguments: Option, #[serde(skip_serializing_if = "Option::is_none")] pub pending_timeout_ms: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -335,6 +341,7 @@ mod tests { child_session_id: child.map(|s| s.to_string()), child_turn_id: child.map(|_| "t_child".to_string()), held_by: None, + held_arguments: None, pending_timeout_ms: None, pending_at: None, } From adf8cce9f63626545d331bd26900ce47dc2ac07d Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Thu, 16 Jul 2026 15:48:10 -0300 Subject: [PATCH 2/4] =?UTF-8?q?test(conformance):=20unquarantine=20C-E2E-5?= =?UTF-8?q?06=20=E2=80=94=20regression=20gate=20for=20#506?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix on this branch makes the scenario pass; --scenario all (and the per-PR CI job) now executes it. Verified locally: C-E2E-001, C-E2E-002, C-E2E-506 all pass. --- .../integration/scenarios/hook-held-release-506/scenario.yaml | 1 - .../tests/snapshots/hook-held-release-506.compiled.json | 1 - 2 files changed, 2 deletions(-) diff --git a/harness/evals/integration/scenarios/hook-held-release-506/scenario.yaml b/harness/evals/integration/scenarios/hook-held-release-506/scenario.yaml index bf5c98cee..47efe66d8 100644 --- a/harness/evals/integration/scenarios/hook-held-release-506/scenario.yaml +++ b/harness/evals/integration/scenarios/hook-held-release-506/scenario.yaml @@ -3,7 +3,6 @@ schema_version: "1" id: C-E2E-506 description: A held call released for execution must run with the arguments produced by earlier hooks. -quarantine: true send: message: Call the recorder once. diff --git a/harness/evals/integration/tests/snapshots/hook-held-release-506.compiled.json b/harness/evals/integration/tests/snapshots/hook-held-release-506.compiled.json index 56a1760f1..854afb7f5 100644 --- a/harness/evals/integration/tests/snapshots/hook-held-release-506.compiled.json +++ b/harness/evals/integration/tests/snapshots/hook-held-release-506.compiled.json @@ -327,7 +327,6 @@ } } ], - "quarantine": true, "recorder": { "extra_functions": [ { From 476ed93d5b55aa595d29e8704e4fb8d235d860cf Mon Sep 17 00:00:00 2001 From: Ytallo Date: Thu, 16 Jul 2026 22:27:43 -0300 Subject: [PATCH 3/4] fix(harness): pre-trigger hold keeps its own argument mutations (#519) A pre-trigger hook returning decision:"hold" WITH mutations had them discarded at parse (parse_output mapped "hold" to a data-less outcome), so the approval-gate pattern (hold AND stamp validated context) was impossible. HookOutcome::Hold now carries HookMutations via a shared parse_mutations helper, and run_pre_trigger folds the holding hook's argument rewrite into the effective arguments before parking. Those args ride the call checkpoint added for #506 (fix/506-release-runs-original-args, which this is based on), so the resolve(execute) release runs with the gate's mutation. Unquarantines C-E2E-505, now a regression gate. Fixes iii-hq/workers#505 --- .../scenarios/hold-mutation-505/scenario.yaml | 1 - .../snapshots/hold-mutation-505.compiled.json | 1 - harness/src/hooks/runner.rs | 88 ++++++++++++------- 3 files changed, 58 insertions(+), 32 deletions(-) diff --git a/harness/evals/integration/scenarios/hold-mutation-505/scenario.yaml b/harness/evals/integration/scenarios/hold-mutation-505/scenario.yaml index 39ebc0f61..b24a0daec 100644 --- a/harness/evals/integration/scenarios/hold-mutation-505/scenario.yaml +++ b/harness/evals/integration/scenarios/hold-mutation-505/scenario.yaml @@ -3,7 +3,6 @@ schema_version: "1" id: C-E2E-505 description: A pre-trigger hook that holds and mutates must apply its mutation to the released call. -quarantine: true send: message: Call the recorder once. diff --git a/harness/evals/integration/tests/snapshots/hold-mutation-505.compiled.json b/harness/evals/integration/tests/snapshots/hold-mutation-505.compiled.json index 832de97de..cd7313d5f 100644 --- a/harness/evals/integration/tests/snapshots/hold-mutation-505.compiled.json +++ b/harness/evals/integration/tests/snapshots/hold-mutation-505.compiled.json @@ -300,7 +300,6 @@ } } ], - "quarantine": true, "recorder": { "extra_functions": [ { diff --git a/harness/src/hooks/runner.rs b/harness/src/hooks/runner.rs index 3a0a9fb42..12be606d2 100644 --- a/harness/src/hooks/runner.rs +++ b/harness/src/hooks/runner.rs @@ -29,7 +29,9 @@ struct HookMutations { enum HookOutcome { Continue(HookMutations), Deny(String), - Hold, + // A holding hook may mutate as it holds (approval-gate: park the call AND + // stamp validated context onto it) — its mutations must not be dropped. + Hold(HookMutations), } /// Outcome of the `pre_generate` chain. @@ -91,7 +93,7 @@ impl HookRegistry { match self.invoke(&binding, input).await { HookOutcome::Continue(_) => {} HookOutcome::Deny(reason) => return Err(reason), - HookOutcome::Hold => {} + HookOutcome::Hold(_) => {} } } Ok(()) @@ -128,7 +130,7 @@ impl HookRegistry { merge(&mut annotations, m.annotations); } HookOutcome::Deny(reason) => return PreGenerateOutcome::Deny(reason), - HookOutcome::Hold => {} + HookOutcome::Hold(_) => {} } } PreGenerateOutcome::Continue { @@ -178,12 +180,18 @@ impl HookRegistry { merge(&mut annotations, m.annotations); } HookOutcome::Deny(reason) => return PreTriggerOutcome::Deny(reason), - HookOutcome::Hold => { + HookOutcome::Hold(m) => { + // Apply the holding hook's own rewrite before parking, so + // the release runs with the full chain's effective args. + if let Some(a) = m.arguments { + args = a; + } + merge(&mut annotations, m.annotations); return PreTriggerOutcome::Hold { held_by: binding.function_id.clone(), arguments: args, annotations, - } + }; } } } @@ -238,7 +246,7 @@ impl HookRegistry { // Deny is not valid at post_trigger; fail-open like existing // post-trigger error handling. HookOutcome::Deny(_) => {} - HookOutcome::Hold => { + HookOutcome::Hold(_) => { return PostTriggerOutcome::Hold { held_by: binding.function_id.clone(), annotations, @@ -298,30 +306,34 @@ fn parse_output(value: Value) -> HookOutcome { .unwrap_or("denied by hook") .to_string(), ), - Some("hold") => HookOutcome::Hold, - _ => { - let mut muts = HookMutations::default(); - if let Some(m) = value.get("mutations") { - muts.system_prompt = m - .get("system_prompt") - .and_then(Value::as_str) - .map(str::to_string); - if let Some(arr) = m.get("append_messages").and_then(Value::as_array) { - muts.append_messages = arr.clone(); - } - muts.arguments = m.get("arguments").cloned(); - if let Some(c) = m.get("content") { - muts.content = serde_json::from_value(c.clone()).ok(); - } - muts.details = m.get("details").cloned(); - muts.is_error = m.get("is_error").and_then(Value::as_bool); - } - if let Some(Value::Object(ann)) = value.get("annotations") { - muts.annotations = ann.clone(); - } - HookOutcome::Continue(muts) + Some("hold") => HookOutcome::Hold(parse_mutations(&value)), + _ => HookOutcome::Continue(parse_mutations(&value)), + } +} + +/// Parse a hook's `mutations`/`annotations` — shared by the continue and hold +/// branches so a holding hook's rewrites are honored, not dropped. +fn parse_mutations(value: &Value) -> HookMutations { + let mut muts = HookMutations::default(); + if let Some(m) = value.get("mutations") { + muts.system_prompt = m + .get("system_prompt") + .and_then(Value::as_str) + .map(str::to_string); + if let Some(arr) = m.get("append_messages").and_then(Value::as_array) { + muts.append_messages = arr.clone(); + } + muts.arguments = m.get("arguments").cloned(); + if let Some(c) = m.get("content") { + muts.content = serde_json::from_value(c.clone()).ok(); } + muts.details = m.get("details").cloned(); + muts.is_error = m.get("is_error").and_then(Value::as_bool); + } + if let Some(Value::Object(ann)) = value.get("annotations") { + muts.annotations = ann.clone(); } + muts } fn merge(into: &mut Map, from: Map) { @@ -399,16 +411,32 @@ mod tests { _ => panic!("expected deny"), } match parse_output(json!({ "decision": "hold" })) { - HookOutcome::Hold => {} + HookOutcome::Hold(m) => assert!(m.arguments.is_none()), _ => panic!("expected hold"), } // Legacy hooks may still send pending_timeout_ms; it is ignored. assert!(matches!( parse_output(json!({ "decision": "hold", "pending_timeout_ms": 1000 })), - HookOutcome::Hold + HookOutcome::Hold(_) )); } + #[test] + fn hold_keeps_mutations_and_annotations() { + let out = parse_output(json!({ + "decision": "hold", + "mutations": { "arguments": { "value": "expected+approved" } }, + "annotations": { "approved_by": "gate" } + })); + match out { + HookOutcome::Hold(m) => { + assert_eq!(m.arguments, Some(json!({ "value": "expected+approved" }))); + assert_eq!(m.annotations.get("approved_by"), Some(&json!("gate"))); + } + _ => panic!("expected hold"), + } + } + #[test] fn continue_mutations_parse() { let out = parse_output(json!({ From 75a236b80e7d0afb2f849daa940561a19cd5d520 Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Mon, 20 Jul 2026 11:22:53 -0300 Subject: [PATCH 4/4] test(integration): retry empty descendant PID reads --- harness/evals/integration/tests/supervisor.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/harness/evals/integration/tests/supervisor.rs b/harness/evals/integration/tests/supervisor.rs index 3df1b465f..afa01cc61 100644 --- a/harness/evals/integration/tests/supervisor.rs +++ b/harness/evals/integration/tests/supervisor.rs @@ -143,11 +143,13 @@ async fn children_see_only_the_environment_allowlist() { async fn wait_for_pid(path: &std::path::Path) -> u32 { for _ in 0..50 { if let Ok(raw) = std::fs::read_to_string(path) { - return raw.trim().parse().expect("numeric descendant pid"); + if let Ok(pid) = raw.trim().parse() { + return pid; + } } tokio::time::sleep(Duration::from_millis(20)).await; } - panic!("descendant pid was not written"); + panic!("numeric descendant pid was not written"); } async fn wait_until_not_running(pid: u32) -> bool {