-
Notifications
You must be signed in to change notification settings - Fork 60
fix(broker): honor spawn_mode/exit_after_task on engine-dispatched spawns #1277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -374,6 +374,22 @@ impl BrokerRuntime { | |
| let channel = action_invoke_string(&invoke.input, &["channel"]); | ||
| let model = action_invoke_string(&invoke.input, &["model"]); | ||
|
|
||
| // Honor the task-exit lifecycle exactly like the local HTTP spawn API: | ||
| // `spawn_mode: task_exit` / `exit_after_task: true` make the agent exit | ||
| // once its task is done instead of idling. Reject an unknown spawn_mode | ||
| // loudly rather than silently defaulting to interactive. | ||
| let spawn_mode = action_invoke_string(&invoke.input, &["spawn_mode", "spawnMode"]); | ||
| let explicit_exit_after_task = | ||
| action_invoke_bool(&invoke.input, &["exit_after_task", "exitAfterTask"]); | ||
|
Comment on lines
+381
to
+383
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In the fleet Useful? React with 👍 / 👎. |
||
| let exit_after_task = | ||
| match resolve_exit_after_task(spawn_mode.as_deref(), explicit_exit_after_task) { | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| Ok(value) => value, | ||
| Err(error) => { | ||
| self.reply_action_error(&invoke.invocation_id, &error).await; | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| // Reuse the action input as the `ws_value` the spawn fn reads | ||
| // harnessConfig / supplied tokens from, mirroring the firehose payload | ||
| // shape (top-level and nested-`agent` lookups both work). | ||
|
|
@@ -406,6 +422,7 @@ impl BrokerRuntime { | |
| task, | ||
| channel, | ||
| model, | ||
| exit_after_task, | ||
| &ws_value, | ||
| &workspace_id, | ||
| None, | ||
|
|
@@ -940,6 +957,25 @@ fn action_invoke_string(input: &Value, keys: &[&str]) -> Option<String> { | |
| None | ||
| } | ||
|
|
||
| /// Read the first boolean at any of the given top-level keys of an | ||
| /// `action.invoke` input object (also checks under a nested `agent` object), | ||
| /// mirroring [`action_invoke_string`]'s lookup order for the flattened-vs-nested | ||
| /// spawn payload shape. | ||
| fn action_invoke_bool(input: &Value, keys: &[&str]) -> Option<bool> { | ||
| for key in keys { | ||
| if let Some(value) = input.get(key).and_then(Value::as_bool) { | ||
| return Some(value); | ||
| } | ||
| } | ||
| let agent = input.get("agent")?; | ||
| for key in keys { | ||
| if let Some(value) = agent.get(key).and_then(Value::as_bool) { | ||
| return Some(value); | ||
| } | ||
| } | ||
| None | ||
| } | ||
|
|
||
| /// Message fields extracted from a node `deliver` payload, ready to build a | ||
| /// [`RelayDelivery`]. | ||
| struct FleetDeliveryFields { | ||
|
|
@@ -1279,6 +1315,55 @@ mod tests { | |
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn action_invoke_bool_reads_top_level_and_nested_agent() { | ||
| // Top-level (flattened) and nested-`agent` shapes both resolve, matching | ||
| // the fleet TS layer that flattens `{...spawn.agent, task, ...}`. | ||
| assert_eq!( | ||
| action_invoke_bool(&json!({"exit_after_task": true}), &["exit_after_task"]), | ||
| Some(true) | ||
| ); | ||
| assert_eq!( | ||
| action_invoke_bool( | ||
| &json!({"agent": {"exitAfterTask": false}}), | ||
| &["exit_after_task", "exitAfterTask"] | ||
| ), | ||
| Some(false) | ||
| ); | ||
| // Absent on both levels yields None so the caller can default. | ||
| assert_eq!( | ||
| action_invoke_bool(&json!({"cli": "codex"}), &["exit_after_task"]), | ||
| None | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn action_invoke_spawn_input_resolves_task_exit_lifecycle() { | ||
| // The engine-dispatched spawn reads spawn_mode/exit_after_task from the | ||
| // invoke input exactly like the local HTTP spawn, from either the | ||
| // flattened top level or the nested `agent` object. | ||
| let top_level = json!({"cli": "codex", "spawn_mode": "task_exit"}); | ||
| assert!(resolve_exit_after_task( | ||
| action_invoke_string(&top_level, &["spawn_mode", "spawnMode"]).as_deref(), | ||
| action_invoke_bool(&top_level, &["exit_after_task", "exitAfterTask"]), | ||
| ) | ||
| .expect("valid spawn_mode")); | ||
|
|
||
| let nested = json!({"agent": {"cli": "codex", "spawnMode": "interactive"}}); | ||
| assert!(!resolve_exit_after_task( | ||
| action_invoke_string(&nested, &["spawn_mode", "spawnMode"]).as_deref(), | ||
| action_invoke_bool(&nested, &["exit_after_task", "exitAfterTask"]), | ||
| ) | ||
| .expect("valid spawn_mode")); | ||
|
|
||
| let explicit = json!({"cli": "codex", "exit_after_task": true}); | ||
| assert!(resolve_exit_after_task( | ||
| action_invoke_string(&explicit, &["spawn_mode", "spawnMode"]).as_deref(), | ||
| action_invoke_bool(&explicit, &["exit_after_task", "exitAfterTask"]), | ||
| ) | ||
| .expect("valid explicit flag")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn fleet_initial_session_ref_prefers_explicit_spec_session() { | ||
| let spec = test_agent_spec(Some("session-spec"), Some("session-harness")); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,6 +106,31 @@ pub(crate) fn apply_exit_after_task_instruction(task: Option<String>) -> String | |
| } | ||
| } | ||
|
|
||
| /// Resolve a spawn request's effective `exit_after_task` lifecycle flag from its | ||
| /// `spawn_mode` selector and any explicit `exit_after_task` boolean. | ||
| /// | ||
| /// Shared by the local HTTP spawn API and the engine-dispatched node spawn so | ||
| /// task-exit semantics are identical on both paths: a `spawn_mode` of | ||
| /// `task_exit`/`single_shot` — or an explicit `exit_after_task: true` — makes | ||
| /// the agent exit once its task is done; `interactive`/absent keeps it running. | ||
| /// An unrecognized `spawn_mode` is rejected with a caller-facing message. | ||
| pub(crate) fn resolve_exit_after_task( | ||
| spawn_mode: Option<&str>, | ||
| exit_after_task: Option<bool>, | ||
| ) -> Result<bool, String> { | ||
| let normalized = spawn_mode.map(|value| value.trim().to_ascii_lowercase()); | ||
| let spawn_mode_exit_after_task = match normalized.as_deref() { | ||
| None | Some("") | Some("interactive") => false, | ||
| Some("task_exit" | "task-exit" | "single_shot" | "single-shot") => true, | ||
| Some(other) => { | ||
| return Err(format!( | ||
| "unsupported spawnMode '{other}' (expected 'interactive' or 'task_exit')" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Error message lists only Prompt for AI agents |
||
| )); | ||
| } | ||
| }; | ||
| Ok(exit_after_task.unwrap_or(false) || spawn_mode_exit_after_task) | ||
| } | ||
|
|
||
| pub(crate) struct RelaySessionOptions<'a> { | ||
| pub(crate) paths: &'a RuntimePaths, | ||
| pub(crate) requested_name: &'a str, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.