Skip to content

Commit 88c2b97

Browse files
committed
fix(runtime): restore deterministic CLI contracts [AI: OpenAI gpt-5.6-sol via OpenCode]
1 parent 195fccd commit 88c2b97

16 files changed

Lines changed: 329 additions & 99 deletions

File tree

crates/homeboy-cli/src/commands/infra/route/local_detach_fanout.rs

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,8 @@ fn detached_fanout_child_args(
339339
fanout_id: &str,
340340
pin_fanout_id: bool,
341341
) -> Vec<String> {
342-
let mut args: Vec<String> = normalized_args
342+
let owned_args = crate::command_capability::homeboy_owned_args(normalized_args);
343+
let mut args: Vec<String> = owned_args
343344
.iter()
344345
.skip(1)
345346
.filter(|arg| {
@@ -351,6 +352,7 @@ fn detached_fanout_child_args(
351352
args.push("--fanout-id".to_string());
352353
args.push(fanout_id.to_string());
353354
}
355+
args.extend(normalized_args.iter().skip(owned_args.len()).cloned());
354356
args
355357
}
356358

@@ -414,6 +416,7 @@ fn materialize_plan_from(
414416
/// Both spellings clap accepts are covered: a separated `--input -` names the
415417
/// following element, an attached `--input=-` names itself.
416418
fn stdin_input_index(args: &[String]) -> Option<usize> {
419+
let args = crate::command_capability::homeboy_owned_args(args);
417420
args.iter().enumerate().find_map(|(index, arg)| {
418421
if arg == "--input" && args.get(index + 1).is_some_and(|value| value == "-") {
419422
Some(index + 1)
@@ -868,6 +871,36 @@ mod tests {
868871
assert_eq!(args[args.len() - 1], "wave-9");
869872
}
870873

874+
#[test]
875+
fn the_child_argv_preserves_forwarded_detach_named_arguments() {
876+
let normalized = [
877+
"homeboy",
878+
"--detach-after-handoff",
879+
"agent-task",
880+
"fanout",
881+
"run-plan",
882+
"--input",
883+
"@plan.json",
884+
"--",
885+
"--detach-after-handoff",
886+
]
887+
.map(str::to_string);
888+
let args = detached_fanout_child_args(&normalized, "wave-9", false);
889+
890+
assert_eq!(
891+
args,
892+
[
893+
"agent-task",
894+
"fanout",
895+
"run-plan",
896+
"--input",
897+
"@plan.json",
898+
"--",
899+
"--detach-after-handoff",
900+
]
901+
);
902+
}
903+
871904
/// An operator who named the wave must not get a second, contradictory id.
872905
#[test]
873906
fn an_already_named_wave_is_not_re_pinned() {
@@ -919,6 +952,27 @@ mod tests {
919952
}
920953
}
921954

955+
#[test]
956+
fn a_forwarded_input_flag_is_not_materialized() {
957+
let session = tempfile::tempdir().expect("temp session root");
958+
let mut args = vec![
959+
"fanout".to_string(),
960+
"--input".to_string(),
961+
"@plan.json".to_string(),
962+
"--".to_string(),
963+
"--input".to_string(),
964+
"-".to_string(),
965+
];
966+
let mut source = std::io::Cursor::new(br#"{"schema":"x"}"#.to_vec());
967+
968+
assert_eq!(
969+
materialize_plan_from(&mut args, session.path(), &mut source)
970+
.expect("forwarded input does not need materialization"),
971+
None
972+
);
973+
assert_eq!(args[4..], ["--input", "-"]);
974+
}
975+
922976
/// An argv with no stdin plan must be left exactly alone.
923977
#[test]
924978
fn a_file_plan_is_left_untouched() {

crates/homeboy-cli/src/commands/infra/route/tests/dispatch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1803,7 +1803,7 @@ fn lab_cook_plan_preserves_actual_cli_provenance_through_handoff_serialization()
18031803
"--max-attempts",
18041804
"3",
18051805
"--max-provider-executions",
1806-
"1",
1806+
"3",
18071807
"--protected-branch",
18081808
"main",
18091809
"--protected-branch",

crates/homeboy-cli/src/commands/json_output/ops.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,18 @@ mod tests {
3838
macro_rules! assert_registered_as_ops {
3939
($(($module:ident, $variant:ident, $handler:path),)*) => {
4040
$({
41-
// Module names are the command names, except `self_cmd`,
42-
// which exists only because `self` is a Rust keyword.
41+
// Module names are command names with Rust identifier
42+
// separators normalized; `self_cmd` exists only because
43+
// `self` is a Rust keyword.
4344
let module = stringify!($module);
44-
let name = if module == "self_cmd" { "self" } else { module };
45+
let name = if module == "self_cmd" {
46+
"self".to_string()
47+
} else {
48+
module.replace('_', "-")
49+
};
4550

4651
assert_eq!(
47-
registered_command_json_family(name),
52+
registered_command_json_family(&name),
4853
Some(CommandJsonFamily::Ops),
4954
"ops descriptor `{name}` is missing from COMMAND_SPECS or is \
5055
registered outside the Ops JSON family, so it would never \

crates/homeboy-cli/src/commands/triage.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -366,22 +366,24 @@ mod tests {
366366
assert_eq!(default.args.poll_interval, "60s");
367367
}
368368

369-
/// The two spellings are one argument, not two: supplying both is a
370-
/// duplicate value for the same id, and the last one wins rather than
371-
/// silently producing two independent intervals.
369+
/// The published alias is one argument: the canonical spelling takes the
370+
/// value, while a duplicate spelling is rejected by Clap.
372371
#[test]
373372
fn watch_interval_spellings_are_one_argument() {
374-
let cli = TestCli::try_parse_from([
373+
let result = TestCli::try_parse_from([
375374
"triage",
376375
"--watch",
377376
"Extra-Chill/homeboy#2238",
378377
"--poll-interval",
379378
"30s",
380379
"--interval",
381380
"45s",
382-
])
383-
.expect("both spellings parse");
384-
assert_eq!(cli.args.poll_interval, "45s");
381+
]);
382+
assert!(result.is_err(), "duplicate spellings must be rejected");
383+
assert_eq!(
384+
result.err().expect("duplicate spelling error").kind(),
385+
clap::error::ErrorKind::ArgumentConflict
386+
);
385387
}
386388

387389
#[test]

crates/homeboy-cli/src/commands/utils/resource_policy/mod.rs

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -834,25 +834,20 @@ mod tests {
834834
}
835835

836836
#[test]
837-
fn agent_task_mutating_recovery_commands_remain_resource_managed() {
838-
// Bounded readers are exempt above; replay and promotion still execute
839-
// provider or worktree operations and retain resource admission.
840-
for args in [
841-
[
842-
"homeboy",
843-
"agent-task",
844-
"replay-provider-boundary",
845-
"agent-task-123",
846-
]
847-
.as_slice(),
848-
["homeboy", "agent-task", "review", "agent-task-123"].as_slice(),
849-
] {
850-
let cli = Cli::parse_from(args);
851-
assert!(
852-
hot_command(&cli.command).is_some(),
853-
"{args:?} must retain resource admission"
854-
);
855-
}
837+
fn agent_task_provider_replay_remains_resource_managed() {
838+
// `review` is a bounded metadata read. Provider replay executes work
839+
// against the selected runner and retains resource admission.
840+
let args = [
841+
"homeboy",
842+
"agent-task",
843+
"replay-provider-boundary",
844+
"agent-task-123",
845+
];
846+
let cli = Cli::parse_from(args);
847+
assert!(
848+
hot_command(&cli.command).is_some(),
849+
"{args:?} must retain resource admission"
850+
);
856851
}
857852

858853
#[test]

crates/homeboy-core/src/daemon/mod.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2784,6 +2784,17 @@ fn enqueue_exec_job(
27842784
request.capture_patch,
27852785
)
27862786
})?;
2787+
// Resolve this before runner-specific preparation so the opaque process plan
2788+
// can retain the same authority the daemon uses for durable lifecycle state.
2789+
let canonical_durable_run_id = resolve_exec_idempotency_key(
2790+
request.idempotency_key.as_deref(),
2791+
exec_request_run_ref_metadata(
2792+
request.lifecycle.as_ref(),
2793+
request.lab_runner_workload.as_ref(),
2794+
request.metadata.as_ref(),
2795+
)
2796+
.as_ref(),
2797+
);
27872798
let plan = runner_exec_driver::prepare_exec(runner_exec_driver::RunnerExecPrepareRequest {
27882799
runner_id: request.runner_id,
27892800
runner: request.runner,
@@ -2800,6 +2811,7 @@ fn enqueue_exec_job(
28002811
source_snapshot: request.source_snapshot,
28012812
require_paths: request.require_paths,
28022813
extension_env_providers: request.extension_env_providers,
2814+
authoritative_run_id: canonical_durable_run_id.clone(),
28032815
validate_require_paths_on_host: true,
28042816
})?;
28052817
let source_snapshot = Some(plan.source_snapshot.clone());
@@ -2811,15 +2823,6 @@ fn enqueue_exec_job(
28112823
// controllers that predate the explicit field. Either way the resolved key
28122824
// is folded into the lifecycle's `durable_run_id` below, which is what the
28132825
// job-store dedup guard keys on.
2814-
let canonical_durable_run_id = resolve_exec_idempotency_key(
2815-
request.idempotency_key.as_deref(),
2816-
exec_request_run_ref_metadata(
2817-
lifecycle.as_ref(),
2818-
request.lab_runner_workload.as_ref(),
2819-
request.metadata.as_ref(),
2820-
)
2821-
.as_ref(),
2822-
);
28232826
if let Some(durable_run_id) = canonical_durable_run_id {
28242827
lifecycle
28252828
.get_or_insert_with(RunnerJobLifecycleMetadata::default)
@@ -3110,6 +3113,7 @@ fn enqueue_exec_job(
31103113
let metrics = process_output.metrics.clone();
31113114
let capture = process_output.capture.clone();
31123115
let extension_env_provenance = process_output.extension_env_provenance.clone();
3116+
let diagnostic_hints = process_output.diagnostic_hints.clone();
31133117
if cancellation_requested.load(Ordering::SeqCst) {
31143118
let evidence = stall_evidence
31153119
.lock()
@@ -3169,7 +3173,7 @@ fn enqueue_exec_job(
31693173
} else {
31703174
None
31713175
};
3172-
let result = json!({
3176+
let mut result = json!({
31733177
"runner_id": plan.runner_id,
31743178
"cwd": plan.cwd,
31753179
"command": plan.command,
@@ -3183,6 +3187,9 @@ fn enqueue_exec_job(
31833187
"capture": capture,
31843188
"extension_env_providers": extension_env_provenance,
31853189
});
3190+
if !diagnostic_hints.is_empty() {
3191+
result["diagnostic_hints"] = json!(diagnostic_hints);
3192+
}
31863193
if exit_code != 0 {
31873194
job.result(result.clone())?;
31883195
return Err(Error::remote_command_failed(RemoteCommandFailedDetails {

crates/homeboy-core/src/daemon/runner_exec_driver.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ pub struct RunnerExecPrepareRequest {
4646
pub require_paths: Vec<String>,
4747
/// Ordered installed extension IDs whose runner-local scripts contribute env.
4848
pub extension_env_providers: Vec<String>,
49+
/// Canonical durable run identity resolved by the daemon from the explicit
50+
/// idempotency key or its compatible lifecycle/metadata fallback.
51+
pub authoritative_run_id: Option<String>,
4952
pub validate_require_paths_on_host: bool,
5053
}
5154

@@ -132,6 +135,8 @@ pub struct DaemonExecOutput {
132135
pub stderr: String,
133136
pub exit_code: i32,
134137
pub extension_env_provenance: Value,
138+
/// Non-secret runner diagnostics persisted with the terminal result.
139+
pub diagnostic_hints: Vec<String>,
135140
/// Serialized `RunnerResourceMetrics`, if any.
136141
pub metrics: Option<Value>,
137142
/// Serialized command-capture metadata, if any.

crates/homeboy-extension/src/env_provider.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,27 @@ fn parse_env_provider_output(stdout: &str) -> Result<Vec<(String, String)>> {
261261
mod tests {
262262
use super::*;
263263

264+
#[cfg(unix)]
265+
fn install_provider_fixture(root: &Path, id: &str, output: &str) {
266+
use std::os::unix::fs::PermissionsExt;
267+
268+
let source = root.join(id);
269+
std::fs::create_dir_all(&source).expect("provider source directory");
270+
std::fs::write(
271+
source.join(format!("{id}.json")),
272+
format!(
273+
r#"{{"id":"{id}","name":"{id}","version":"1.0.0","env_provider":{{"script":"env.sh"}}}}"#
274+
),
275+
)
276+
.expect("provider manifest");
277+
let script = source.join("env.sh");
278+
std::fs::write(&script, format!("#!/bin/sh\nprintf '%s\\n' '{output}'\n"))
279+
.expect("provider script");
280+
std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755))
281+
.expect("provider executable");
282+
crate::install(&source.display().to_string(), Some(id)).expect("install provider fixture");
283+
}
284+
264285
#[test]
265286
fn parses_blank_output_as_no_env() {
266287
assert!(parse_env_provider_output("\n").unwrap().is_empty());
@@ -309,4 +330,24 @@ mod tests {
309330
assert!(!payload.contains(secret));
310331
assert!(payload.contains("claim_ref"));
311332
}
333+
334+
#[cfg(unix)]
335+
#[test]
336+
fn providers_cannot_contribute_the_same_public_environment_key() {
337+
homeboy_core::test_support::with_isolated_home(|_| {
338+
let root = tempfile::tempdir().expect("provider fixtures");
339+
install_provider_fixture(root.path(), "first", r#"{"SHARED_KEY":"first"}"#);
340+
install_provider_fixture(root.path(), "second", r#"{"SHARED_KEY":"second"}"#);
341+
342+
let error = resolve_installed_all(
343+
&RunnerJobExecutionContext::local("homeboy"),
344+
&["first".to_string(), "second".to_string()],
345+
root.path(),
346+
&[],
347+
)
348+
.expect_err("provider-provider public env collision must fail");
349+
350+
assert!(error.message.contains("SHARED_KEY"));
351+
});
352+
}
312353
}

crates/homeboy-lab-runner/src/daemon_exec_driver.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use super::Runner;
2626
struct DaemonPreparedPlan {
2727
plan: PreparedRunnerProcess,
2828
extension_env_providers: Vec<String>,
29+
authoritative_run_id: Option<String>,
2930
}
3031

3132
/// The runner layer's `RunnerExecDriver`. Registered with core at startup.
@@ -71,6 +72,7 @@ impl RunnerExecDriver for RunnerDaemonExecDriver {
7172
&request.env,
7273
secret_env_plan,
7374
);
75+
let authoritative_run_id = request.authoritative_run_id;
7476
let plan = prepare_daemon_local_process(RunnerProcessRequest {
7577
runner_id: request.runner_id,
7678
runner,
@@ -103,6 +105,7 @@ impl RunnerExecDriver for RunnerDaemonExecDriver {
103105
plan_token: Arc::new(DaemonPreparedPlan {
104106
plan,
105107
extension_env_providers: request.extension_env_providers,
108+
authoritative_run_id,
106109
}),
107110
}))
108111
}
@@ -153,17 +156,20 @@ impl RunnerExecDriver for RunnerDaemonExecDriver {
153156
execution_context,
154157
&base.extension_env_providers,
155158
std::path::Path::new(&plan.cwd),
156-
&plan
157-
.env
158-
.iter()
159-
.map(|(key, value)| (key.clone(), value.clone()))
160-
.collect::<Vec<_>>(),
159+
&plan.env,
160+
base.authoritative_run_id.as_deref(),
161161
)?;
162162
for contribution in &contributions {
163163
for (key, value) in &contribution.public_env {
164164
plan.env.insert(key.clone(), value.clone());
165165
}
166166
}
167+
let diagnostic_hints = super::execution::apply_explicit_runner_exec_run_id_env(
168+
&mut plan.env,
169+
base.authoritative_run_id.as_deref(),
170+
)
171+
.into_iter()
172+
.collect();
167173
let extension_env_provenance = serde_json::to_value(&contributions).map_err(|err| {
168174
homeboy_core::error::Error::internal_json(
169175
err.to_string(),
@@ -196,6 +202,7 @@ impl RunnerExecDriver for RunnerDaemonExecDriver {
196202
.capture
197203
.and_then(|capture| serde_json::to_value(capture).ok()),
198204
extension_env_provenance,
205+
diagnostic_hints,
199206
})
200207
}
201208
}

0 commit comments

Comments
 (0)