Merge pull request #25 from Dark-Alex-17/feat/spawn-agent-variables

feat(agents): allow passing agent variables to agent__spawn and agent__task_create
This commit is contained in:
Alex Clarke authored and GitHub committed 2026-09-12 01:44:08 -06:00
commit fca086ba09
17 files changed
+641 -96

No files matched your search

+6 -1
View File
@@ -33,6 +33,7 @@ settings:
timeout: 5400
initial_state:
project_dir_in: ''
criteria: []
diff_spec: auto
plan_notes: ''
@@ -68,6 +69,8 @@ nodes:
- `diff_spec`: the git diff basis the caller named — a range, a ref/SHA,
"staged", "worktree", or "auto" when unnamed (a caller phrase like
"get_diff --base X" means diff_spec = "X").
- `project_dir_in`: the directory/worktree path the caller named as
holding the code under review; empty string when the caller named none.
- `plan_notes`: recorded decisions the plan states ("do X not Y because Z"),
verbatim — the holistic pass checks the diff didn't silently override them.
- `scope_notes`: what the plan says NOT to touch / explicit boundaries.
@@ -98,7 +101,9 @@ nodes:
type: string
extra_context:
type: string
required: [criteria, diff_spec, plan_notes, scope_notes, extra_context]
project_dir_in:
type: string
required: [criteria, diff_spec, plan_notes, scope_notes, extra_context, project_dir_in]
max_iterations: 3
max_attempts: 2
next: facts
@@ -22,7 +22,9 @@ def load_state():
state = load_state()
proj = state.get("project_dir") or "."
proj = os.path.expanduser(
(state.get("project_dir_in") or "").strip() or state.get("project_dir") or "."
)
spec = (state.get("diff_spec") or "auto").strip()
@@ -33,7 +35,7 @@ def git(*args):
return r.stdout
out = {"diff_note": ""}
out = {"diff_note": "", "project_dir": proj}
try:
if spec in ("auto", ""):
text = git("diff", "--cached")
+7 -1
View File
@@ -50,6 +50,7 @@ settings:
timeout: 10800
initial_state:
project_dir_in: ''
diff_spec: auto
intent: ''
rigor_in: ''
@@ -111,6 +112,9 @@ nodes:
- `rigor_in`: ONLY when the caller explicitly set a rigor (poc|prototype|production),
e.g. via a "Quality bar:" line; else empty string.
- `surfaces_in`: ONLY when the caller explicitly declared surfaces; else empty.
- `project_dir_in`: the directory/worktree path the caller named as
holding the code under review (e.g. "in /tmp/pr-review-x", "the repo
at ~/code/foo"); empty string when the caller named none.
- `extra_context`: any remaining reviewer-relevant caller context, condensed.
prompt: |
Extract the review-request facts from this request:
@@ -128,9 +132,11 @@ nodes:
type: string
surfaces_in:
type: string
project_dir_in:
type: string
extra_context:
type: string
required: [diff_spec, intent, rigor_in, surfaces_in, extra_context]
required: [diff_spec, intent, rigor_in, surfaces_in, project_dir_in, extra_context]
max_iterations: 3
max_attempts: 2
next: facts
+4 -2
View File
@@ -23,10 +23,12 @@ def load_state():
state = load_state()
proj = state.get("project_dir") or "."
proj = os.path.expanduser(
(state.get("project_dir_in") or "").strip() or state.get("project_dir") or "."
)
spec = (state.get("diff_spec") or "auto").strip()
notes = []
out = {}
out = {"project_dir": proj}
def git(*args, timeout=60):
+9 -2
View File
@@ -55,8 +55,12 @@ nodes:
- `path` and `lines` come from the finding verbatim; if a finding cites
multiple locations, emit one entry per location with the same id plus
a `-b`, `-c` suffix.
- `project_dir`: the repository directory named in the request (a
"Repository:" or "Project:" line). When none is named, echo the
default shown in the prompt.
prompt: |
Extract the findings from this verification request:
Extract the findings from this verification request (default repository
directory when the request names none: {{project_dir}}):
{{initial_prompt}}
tools: []
@@ -84,7 +88,10 @@ nodes:
type: string
description: Self-contained statement of what the finding asserts about the code.
required: [id, severity, path, lines, claim]
required: [findings_list]
project_dir:
type: string
description: Repository directory named in the request (echo the prompt default when none).
required: [findings_list, project_dir]
next: verify_each
verify_each:
+6 -1
View File
@@ -83,6 +83,9 @@ nodes:
range ("main...HEAD", "HEAD~3..HEAD"), a single ref, or the literal
"worktree" for uncommitted changes against HEAD. When the caller
did not name one, use "worktree".
- `project_dir_in`: the repository directory the caller named as holding
the change (absolute path preferred); empty string when the caller
named none.
- `plan_context`: the plan / spec / acceptance-criteria text the
implementation must satisfy, verbatim (or a file path the caller
pointed at). Empty string when the caller supplied none.
@@ -121,7 +124,9 @@ nodes:
type: string
extra_context:
type: string
required: [diff_spec, plan_context, rigor, security_posture, probe_context, forced_lanes, extra_context]
project_dir_in:
type: string
required: [diff_spec, plan_context, rigor, security_posture, probe_context, forced_lanes, extra_context, project_dir_in]
max_iterations: 3
max_attempts: 2
next: signals
@@ -21,7 +21,9 @@ def load_state():
state = load_state()
proj = state.get("project_dir") or "."
proj = os.path.expanduser(
(state.get("project_dir_in") or "").strip() or state.get("project_dir") or "."
)
spec = (state.get("diff_spec") or "worktree").strip()
@@ -34,7 +36,7 @@ def git(*args):
return r.stdout
out = {}
out = {"project_dir": proj}
try:
if spec in ("", "worktree"):
names = git("diff", "--name-only", "HEAD")
+3 -2
View File
@@ -329,6 +329,7 @@ impl Agent {
agent_variables: &[AgentVariable],
pre_set_variables: Option<&AgentVariables>,
no_interaction: bool,
interactive: bool,
) -> Result<AgentVariables> {
let mut output = IndexMap::new();
if agent_variables.is_empty() {
@@ -349,7 +350,7 @@ impl Agent {
if no_interaction {
continue;
}
if *IS_STDOUT_TERMINAL {
if interactive && *IS_STDOUT_TERMINAL {
if !printed {
println!("⚙ Init agent variables...");
printed = true;
@@ -1257,7 +1258,7 @@ fn load_agent_description(name: &str) -> String {
String::new()
}
fn load_agent_variables(name: &str) -> Vec<AgentVariable> {
pub fn load_agent_variables(name: &str) -> Vec<AgentVariable> {
if let Ok(config) = AgentConfig::load(&paths::agent_config_file(name)) {
return config.variables;
}
+1 -1
View File
@@ -28,7 +28,7 @@ pub(crate) use self::agent::AgentConfig;
pub(crate) use self::agent::default_max_agent_depth;
pub use self::agent::{
Agent, AgentVariable, AgentVariables, complete_agent_variables, list_agents,
list_agents_with_descriptions,
list_agents_with_descriptions, load_agent_variables,
};
#[allow(unused_imports)]
pub use self::app_config::AppConfig;
+3 -3
View File
@@ -84,13 +84,13 @@ pub(in crate::config) const DEFAULT_SPAWN_INSTRUCTIONS: &str = indoc! {"
| Tool | Purpose |
|------|----------|
| `agent__spawn` | Spawn a subagent in the background. Returns an `id` immediately. |
| `agent__spawn` | Spawn a subagent in the background. Returns an `id` immediately. Optional `variables` (string map) sets the target agent's declared variables, overriding inherited values. |
| `agent__check` | Non-blocking status probe: running or finished. Never returns/consumes the result — use `agent__collect`. |
| `agent__collect` | Blocking wait: wait for an agent to finish, return its output. |
| `agent__list_available` | List all agent types you can spawn (name + description). Use this to discover specialists before calling `agent__spawn`. |
| `agent__list_available` | List all agent types you can spawn (name + description + declared variables). Use this to discover specialists and the `variables` they accept before calling `agent__spawn`. |
| `agent__list_running` | List all subagents YOU have spawned, with their status. |
| `agent__cancel` | Cancel a running agent by ID. |
| `agent__task_create` | Create a task in the dependency-aware task queue. |
| `agent__task_create` | Create a task in the dependency-aware task queue. Optional `variables` (string map) is passed to the auto-dispatched agent. |
| `agent__task_list` | List all tasks and their status/dependencies. |
| `agent__task_complete` | Mark a task done; returns any newly unblocked tasks. Auto-dispatches agents for tasks with a designated agent. |
| `agent__task_fail` | Mark a task as failed. Dependents remain blocked. |
+62 -35
View File
@@ -1191,6 +1191,7 @@ impl RequestContext {
agent.defined_variables(),
self.agent_variables.as_ref(),
self.info_flag,
self.self_agent_id.is_none(),
)?;
agent.set_shared_variables(new_variables);
}
@@ -1213,6 +1214,7 @@ impl RequestContext {
agent.defined_variables(),
self.agent_variables.as_ref(),
self.info_flag,
self.self_agent_id.is_none(),
)?;
agent.set_shared_variables(new_variables.clone());
new_variables
@@ -8182,6 +8184,22 @@ mod tests {
.collect()
}
fn shadowed(names: &[&str]) -> Vec<String> {
names
.iter()
.map(|name| {
format!(
"-: declared variable '{name}' is shadowed by an explicit \
`initial_state` key: the `initial_state` value wins and the \
variable's resolved value (spawn-provided/CLI/default) is never \
seeded into graph state; rename one of them, or drop the \
`initial_state` key to let the variable seed it (script nodes' \
`LLM_AGENT_VAR_<NAME>` env is unaffected)"
)
})
.collect()
}
let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap();
@@ -8216,8 +8234,9 @@ mod tests {
);
}
// Warning parity with the pre-subgraph validator: the only warnings any
// bundled graph emits are the `_next`-routed unreachable nodes. A new
// Warning baseline for the shipped graphs: `_next`-routed unreachable
// nodes, plus variable-shadowing on `coder` and `step-runner` (they
// declare variables whose names are also `initial_state` keys). A new
// rule that fires on a shipped asset (or a new bundled graph) must be
// recorded here deliberately.
let expected_warnings = BTreeMap::from([
@@ -8225,19 +8244,23 @@ mod tests {
("code-reviewer".to_string(), Vec::new()),
(
"coder".to_string(),
unreachable(&[
"analyze_request",
"end_rejected",
"end_success",
"fix_loop_gate",
"gate_approval",
"implement",
"route_complexity",
"route_review_result",
"self_review",
"verify_build",
"verify_tests",
]),
[
shadowed(&["project_dir"]),
unreachable(&[
"analyze_request",
"end_rejected",
"end_success",
"fix_loop_gate",
"gate_approval",
"implement",
"route_complexity",
"route_review_result",
"self_review",
"verify_build",
"verify_tests",
]),
]
.concat(),
),
("deep-research".to_string(), unreachable(&["ask_topic"])),
("finding-verifier".to_string(), Vec::new()),
@@ -8245,26 +8268,30 @@ mod tests {
("review-gauntlet".to_string(), Vec::new()),
(
"step-runner".to_string(),
unreachable(&[
"check_handoff",
"edge_case_sweep",
"end_blocked",
"end_rejected",
"end_success",
"fix_loop_gate",
"gate_blocked",
"gate_deviation",
"gate_user_review",
"get_revision",
"independent_review",
"revise_from_choice",
"route_review",
"route_sweep",
"verify_build",
"verify_format_lint",
"verify_tests",
"write_handoff",
]),
[
shadowed(&["plans_dir", "project_dir"]),
unreachable(&[
"check_handoff",
"edge_case_sweep",
"end_blocked",
"end_rejected",
"end_success",
"fix_loop_gate",
"gate_blocked",
"gate_deviation",
"gate_user_review",
"get_revision",
"independent_review",
"revise_from_choice",
"route_review",
"route_sweep",
"verify_build",
"verify_format_lint",
"verify_tests",
"write_handoff",
]),
]
.concat(),
),
]);
assert_eq!(
+371 -15
View File
@@ -2,8 +2,9 @@ use super::todo::TODO_FUNCTION_PREFIX;
use super::{FunctionDeclaration, JsonSchema};
use crate::client::{Model, ModelType, call_chat_completions};
use crate::config::{
Agent, AppState, Input, RequestContext, Role, RoleLike, default_max_agent_depth,
effective_max_concurrent_jobs, jobs_enabled, list_agents_with_descriptions,
Agent, AgentVariable, AgentVariables, AppState, Input, RequestContext, Role, RoleLike,
default_max_agent_depth, effective_max_concurrent_jobs, jobs_enabled,
list_agents_with_descriptions, load_agent_variables,
};
use crate::supervisor::mailbox::{Envelope, EnvelopePayload, Inbox, PeerRegistry, graph_agent_id};
use crate::supervisor::notification::agent_notification;
@@ -39,6 +40,77 @@ fn agent_permitted(whitelist: Option<&[String]>, target: &str) -> bool {
}
}
fn parse_variables_arg(args: &Value) -> Result<Option<AgentVariables>, String> {
let Some(value) = args.get("variables") else {
return Ok(None);
};
let Some(obj) = value.as_object() else {
return Err(
"'variables' must be a JSON object mapping variable names to string values, e.g. {\"pr\": \"1234\"}".to_string(),
);
};
let mut variables = AgentVariables::new();
for (key, val) in obj {
let Some(val) = val.as_str() else {
return Err(format!("'variables.{key}' must be a string value"));
};
variables.insert(key.clone(), val.to_string());
}
Ok(Some(variables))
}
fn validate_spawn_variables(
agent_name: &str,
declared: &[AgentVariable],
provided: &AgentVariables,
) -> Result<(), String> {
let unknown: Vec<&str> = provided
.keys()
.filter(|key| !declared.iter().any(|v| v.name == **key))
.map(String::as_str)
.collect();
if unknown.is_empty() {
return Ok(());
}
if declared.is_empty() {
return Err(format!(
"Agent '{agent_name}' declares no variables; remove the 'variables' argument."
));
}
let declared_list = declared
.iter()
.map(|v| format!("{} ({})", v.name, v.description))
.collect::<Vec<_>>()
.join(", ");
Err(format!(
"Unknown variable(s) {unknown:?} for agent '{agent_name}'. Declared variables: {declared_list}"
))
}
fn merge_spawn_variables(inherited: &mut Option<AgentVariables>, provided: AgentVariables) {
if provided.is_empty() {
return;
}
inherited
.get_or_insert_with(AgentVariables::new)
.extend(provided);
}
fn dispatch_spawn_args(agent: &str, prompt: &str, variables: Option<&AgentVariables>) -> Value {
let mut spawn_args = json!({
"agent": agent,
"prompt": prompt,
});
if let Some(variables) = variables {
spawn_args["variables"] = json!(variables);
}
spawn_args
}
fn is_job_task(supervisor: Option<&Arc<RwLock<Supervisor>>>, id: &str) -> bool {
id.starts_with("job_") || supervisor.is_some_and(|sup| sup.read().has_job(id))
}
@@ -265,6 +337,14 @@ pub fn agent_function_declarations() -> Vec<FunctionDeclaration> {
..Default::default()
},
),
(
"variables".to_string(),
JsonSchema {
type_value: Some("object".to_string()),
description: Some("Values for the target agent's declared variables (its config/graph `variables:` list), e.g. {\"pr\": \"1234\"}. Overrides values inherited from the parent. String values only. Note: if the target agent resumes an existing shared session, the session's stored variable values take precedence and these are ignored. Discover an agent's declared variables via `agent__list_available`.".into()),
..Default::default()
},
),
])),
required: Some(vec!["agent".to_string(), "prompt".to_string()]),
..Default::default()
@@ -328,9 +408,11 @@ pub fn agent_function_declarations() -> Vec<FunctionDeclaration> {
},
FunctionDeclaration {
name: format!("{AGENT_FUNCTION_PREFIX}list_available"),
description: "List all agent types installed and available to spawn (name + description). Use this to \
discover what specialists exist before calling `agent__spawn` — especially when you're unsure \
which agent to delegate to. This is the discovery counterpart to `agent__list_running` \
description: "List all agent types installed and available to spawn (name + description + declared \
variables). Use this to discover what specialists exist before calling `agent__spawn` — \
especially when you're unsure which agent to delegate to. Each agent's `variables` entry \
describes the values to pass via the `variables` parameter of `agent__spawn` or \
`agent__task_create`. This is the discovery counterpart to `agent__list_running` \
(which reports agents you have already spawned).".to_string(),
parameters: JsonSchema {
type_value: Some("object".to_string()),
@@ -410,6 +492,14 @@ pub fn agent_function_declarations() -> Vec<FunctionDeclaration> {
..Default::default()
},
),
(
"variables".to_string(),
JsonSchema {
type_value: Some("object".to_string()),
description: Some("Values for the auto-spawned agent's declared variables, e.g. {\"pr\": \"1234\"}. Passed to `agent__spawn` at dispatch time. String values only. Discover an agent's declared variables via `agent__list_available`.".into()),
..Default::default()
},
),
])),
required: Some(vec!["subject".to_string()]),
..Default::default()
@@ -918,6 +1008,15 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
.ok_or_else(|| anyhow!("'prompt' is required"))?
.to_string();
let _task_id = args.get("task_id").and_then(Value::as_str);
let variables = match parse_variables_arg(args) {
Ok(v) => v,
Err(message) => {
return Ok(json!({
"status": "error",
"message": message,
}));
}
};
if let Some(parent) = ctx.agent.as_ref()
&& !agent_permitted(parent.spawnable_agents(), &agent_name)
@@ -995,6 +1094,16 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
)
.await?;
if let Some(provided) = variables.as_ref()
&& let Err(message) =
validate_spawn_variables(&agent_name, agent.defined_variables(), provided)
{
return Ok(json!({
"status": "error",
"message": message,
}));
}
let agent_mcp_servers = agent.mcp_server_names().to_vec();
let session = agent.agent_session().map(|v| v.to_string());
let child_jobs_enabled = jobs_enabled(Some(&agent), app_config.as_ref());
@@ -1020,6 +1129,9 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
Supervisor::new(max_concurrent_agents, max_depth).with_max_concurrent_jobs(max_jobs),
)));
}
if let Some(provided) = variables {
merge_spawn_variables(&mut child_ctx.agent_variables, provided);
}
if let Some(session) = session {
child_ctx
@@ -1337,6 +1449,27 @@ fn handle_list_running(ctx: &mut RequestContext) -> Result<Value> {
Ok(result)
}
fn variables_json(vars: &[AgentVariable]) -> Option<Value> {
if vars.is_empty() {
return None;
}
Some(Value::Array(
vars.iter()
.map(|v| {
let mut var = json!({
"name": v.name,
"description": v.description,
"required": v.default.is_none(),
});
if let Some(default) = &v.default {
var["default"] = json!(default);
}
var
})
.collect(),
))
}
fn handle_list_available(ctx: &RequestContext) -> Result<Value> {
let whitelist: Option<Vec<String>> = ctx
.agent
@@ -1352,11 +1485,15 @@ fn handle_list_available(ctx: &RequestContext) -> Result<Value> {
let agents: Vec<Value> = entries
.into_iter()
.map(|(name, description)| {
if description.is_empty() {
let mut agent = if description.is_empty() {
json!({ "name": name })
} else {
json!({ "name": name, "description": description })
};
if let Some(variables) = variables_json(&load_agent_variables(&name)) {
agent["variables"] = variables;
}
agent
})
.collect();
@@ -1605,6 +1742,16 @@ fn handle_task_create(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
bail!("'prompt' is required when 'agent' is set");
}
let variables = match parse_variables_arg(args) {
Ok(v) => v,
Err(message) => {
return Ok(json!({
"status": "error",
"message": message,
}));
}
};
let supervisor = ctx
.supervisor
.as_ref()
@@ -1617,6 +1764,7 @@ fn handle_task_create(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
description.to_string(),
dispatch_agent.clone(),
task_prompt,
variables,
);
let mut dep_errors = vec![];
@@ -1664,6 +1812,7 @@ fn handle_task_list(ctx: &mut RequestContext) -> Result<Value> {
"blocks": t.blocks.iter().collect::<Vec<_>>(),
"agent": t.dispatch_agent,
"prompt": t.prompt,
"variables": t.variables,
})
})
.collect();
@@ -1688,7 +1837,7 @@ async fn handle_task_complete(ctx: &mut RequestContext, args: &Value) -> Result<
let newly_runnable_ids = sup.task_queue_mut().complete(task_id);
let mut newly_runnable = Vec::new();
let mut to_dispatch: Vec<(String, String, String)> = Vec::new();
let mut to_dispatch: Vec<(String, String, String, Option<AgentVariables>)> = Vec::new();
for id in &newly_runnable_ids {
if let Some(t) = sup.task_queue().get(id) {
@@ -1700,15 +1849,20 @@ async fn handle_task_complete(ctx: &mut RequestContext, args: &Value) -> Result<
}));
if let (Some(agent), Some(prompt)) = (&t.dispatch_agent, &t.prompt) {
to_dispatch.push((id.clone(), agent.clone(), prompt.clone()));
to_dispatch.push((
id.clone(),
agent.clone(),
prompt.clone(),
t.variables.clone(),
));
}
}
}
let mut dispatchable = Vec::new();
for (tid, agent, prompt) in to_dispatch {
for (tid, agent, prompt, variables) in to_dispatch {
if sup.task_queue_mut().claim(&tid, &format!("auto:{agent}")) {
dispatchable.push((agent, prompt));
dispatchable.push((agent, prompt, variables));
}
}
@@ -1716,11 +1870,8 @@ async fn handle_task_complete(ctx: &mut RequestContext, args: &Value) -> Result<
};
let mut spawned = Vec::new();
for (agent, prompt) in &dispatchable {
let spawn_args = json!({
"agent": agent,
"prompt": prompt,
});
for (agent, prompt, variables) in &dispatchable {
let spawn_args = dispatch_spawn_args(agent, prompt, variables.as_ref());
match handle_spawn(ctx, &spawn_args).await {
Ok(result) => {
let agent_id = result
@@ -2515,6 +2666,53 @@ mod tests {
assert_eq!(full_count as usize, list_agents_with_descriptions().len());
}
#[test]
fn variables_json_maps_required_and_default() {
let vars = vec![
AgentVariable {
name: "pr".to_string(),
description: "PR number".to_string(),
default: None,
value: String::new(),
},
AgentVariable {
name: "branch".to_string(),
description: "Target branch".to_string(),
default: Some("x".to_string()),
value: String::new(),
},
];
let result = variables_json(&vars).unwrap();
assert_eq!(result[0]["name"], "pr");
assert_eq!(result[0]["description"], "PR number");
assert_eq!(result[0]["required"], true);
assert!(result[0].get("default").is_none());
assert_eq!(result[1]["name"], "branch");
assert_eq!(result[1]["required"], false);
assert_eq!(result[1]["default"], "x");
}
#[test]
fn variables_json_empty_slice_is_none() {
assert!(variables_json(&[]).is_none());
}
#[test]
fn variables_json_never_emits_value() {
let vars = vec![AgentVariable {
name: "pr".to_string(),
description: "PR number".to_string(),
default: None,
value: "resolved".to_string(),
}];
let result = variables_json(&vars).unwrap();
assert!(result[0].get("value").is_none());
}
#[test]
fn agent_permitted_none_whitelist_allows_all() {
assert!(agent_permitted(None, "explore"));
@@ -2935,6 +3133,32 @@ mod tests {
assert_eq!(result["auto_dispatch"], true);
}
#[test]
fn handle_task_create_stores_variables() {
let mut ctx = ctx_with_supervisor(4, 3);
let result = handle_task_create(
&mut ctx,
&json!({"subject": "Auto task", "agent": "coder", "prompt": "do it", "variables": {"pr": "1234"}}),
)
.unwrap();
assert_eq!(result["status"], "ok");
let list = handle_task_list(&mut ctx).unwrap();
let tasks = list["tasks"].as_array().unwrap();
assert_eq!(tasks[0]["variables"]["pr"], "1234");
}
#[test]
fn handle_task_create_rejects_non_object_variables() {
let mut ctx = ctx_with_supervisor(4, 3);
let result =
handle_task_create(&mut ctx, &json!({"subject": "Bad", "variables": ["pr"]})).unwrap();
assert_eq!(result["status"], "error");
assert!(result["message"].as_str().unwrap().contains("JSON object"));
}
#[test]
fn handle_task_create_agent_without_prompt_errors() {
let mut ctx = ctx_with_supervisor(4, 3);
@@ -3635,6 +3859,138 @@ mod tests {
);
}
#[test]
fn handle_spawn_non_object_variables_errors() {
let mut ctx = ctx_with_supervisor(4, 3);
let result = run_async(handle_spawn(
&mut ctx,
&json!({"agent": "x", "prompt": "p", "variables": "pr=1234"}),
))
.unwrap();
assert_eq!(result["status"], "error");
assert!(result["message"].as_str().unwrap().contains("JSON object"));
}
#[test]
fn handle_spawn_non_string_variable_value_errors() {
let mut ctx = ctx_with_supervisor(4, 3);
let result = run_async(handle_spawn(
&mut ctx,
&json!({"agent": "x", "prompt": "p", "variables": {"pr": 1234}}),
))
.unwrap();
assert_eq!(result["status"], "error");
assert!(
result["message"]
.as_str()
.unwrap()
.contains("'variables.pr'")
);
}
#[test]
fn parse_variables_arg_absent_returns_none() {
assert_eq!(parse_variables_arg(&json!({})).unwrap(), None);
}
#[test]
fn parse_variables_arg_accepts_object_of_strings() {
let parsed = parse_variables_arg(&json!({"variables": {"pr": "1234", "repo": "coyote"}}))
.unwrap()
.unwrap();
assert_eq!(parsed.get("pr").map(String::as_str), Some("1234"));
assert_eq!(parsed.get("repo").map(String::as_str), Some("coyote"));
}
#[test]
fn validate_spawn_variables_unknown_name_lists_declared() {
let declared = vec![AgentVariable {
name: "pr".into(),
description: "PR number".into(),
..Default::default()
}];
let provided = AgentVariables::from([("typo".to_string(), "x".to_string())]);
let err = validate_spawn_variables("coder", &declared, &provided).unwrap_err();
assert!(err.contains("typo"));
assert!(err.contains("pr (PR number)"));
}
#[test]
fn validate_spawn_variables_none_declared_errors() {
let provided = AgentVariables::from([("pr".to_string(), "1".to_string())]);
let err = validate_spawn_variables("coder", &[], &provided).unwrap_err();
assert!(err.contains("declares no variables"));
}
#[test]
fn validate_spawn_variables_known_names_ok() {
let declared = vec![AgentVariable {
name: "pr".into(),
description: "PR number".into(),
..Default::default()
}];
let provided = AgentVariables::from([("pr".to_string(), "1".to_string())]);
assert!(validate_spawn_variables("coder", &declared, &provided).is_ok());
}
#[test]
fn merge_spawn_variables_overrides_inherited_per_key() {
let mut inherited = Some(AgentVariables::from([
("pr".to_string(), "1".to_string()),
("repo".to_string(), "coyote".to_string()),
]));
merge_spawn_variables(
&mut inherited,
AgentVariables::from([("pr".to_string(), "2".to_string())]),
);
let merged = inherited.unwrap();
assert_eq!(merged.get("pr").map(String::as_str), Some("2"));
assert_eq!(merged.get("repo").map(String::as_str), Some("coyote"));
}
#[test]
fn merge_spawn_variables_seeds_when_nothing_inherited() {
let mut inherited = None;
merge_spawn_variables(
&mut inherited,
AgentVariables::from([("pr".to_string(), "2".to_string())]),
);
assert_eq!(inherited.unwrap().get("pr").map(String::as_str), Some("2"));
}
#[test]
fn dispatch_spawn_args_includes_variables() {
let variables = AgentVariables::from([("pr".to_string(), "1234".to_string())]);
let args = dispatch_spawn_args("coder", "do it", Some(&variables));
assert_eq!(args["agent"], "coder");
assert_eq!(args["prompt"], "do it");
assert_eq!(args["variables"]["pr"], "1234");
}
#[test]
fn dispatch_spawn_args_omits_absent_variables() {
let args = dispatch_spawn_args("coder", "do it", None);
assert!(args.get("variables").is_none());
}
#[test]
fn handle_spawn_at_capacity_errors() {
let mut ctx = ctx_with_supervisor(1, 3);
+86
View File
@@ -180,6 +180,7 @@ impl GraphValidator {
self.validate_max_concurrency_template(graph, &mut result);
self.validate_orchestration_limits(graph, &mut result);
self.validate_timeouts(graph, &mut result);
self.validate_variable_shadowing(graph, &mut result);
self.validate_map_subgraphs(graph, &mut result);
self.validate_parallel_user_interaction(graph, &mut result);
self.validate_parallel_writes(graph, &mut result);
@@ -613,6 +614,25 @@ impl GraphValidator {
}
}
/// Variable seeding never overwrites an existing state key, so a declared
/// variable whose name is also an `initial_state` key never reaches graph
/// state: the explicit `initial_state` value wins. Legal; the warning only
/// makes the shadowing visible.
fn validate_variable_shadowing(&self, graph: &Graph, result: &mut ValidationResult) {
for var in &graph.variables {
if graph.initial_state.contains_key(&var.name) {
result.warning(ValidationError::new(format!(
"declared variable '{}' is shadowed by an explicit `initial_state` key: \
the `initial_state` value wins and the variable's resolved value \
(spawn-provided/CLI/default) is never seeded into graph state; rename \
one of them, or drop the `initial_state` key to let the variable seed \
it (script nodes' `LLM_AGENT_VAR_<NAME>` env is unaffected)",
var.name
)));
}
}
}
/// Explicit `0` opt-outs on `settings` and node bounds: a `timeout: 0`
/// disables the wall-clock bound, `max_loop_iterations: 0` disables the
/// per-node visit cap, and an llm node's `max_iterations: 0` disables its
@@ -3272,6 +3292,72 @@ mod tests {
);
}
fn variable(name: &str) -> crate::config::AgentVariable {
crate::config::AgentVariable {
name: name.into(),
..Default::default()
}
}
fn shadowing_warnings(result: &ValidationResult) -> Vec<&ValidationError> {
result
.warnings
.iter()
.filter(|w| w.message.contains("shadowed"))
.collect()
}
#[test]
fn variable_shadowed_by_initial_state_key_warns() {
let mut graph = graph_with(vec![("e", end_node("e"))], "e");
graph.variables = vec![variable("region")];
graph
.initial_state
.insert("region".into(), serde_json::json!("us-east-1"));
let result = validator().validate(&graph);
assert!(result.is_valid());
let w = shadowing_warnings(&result);
assert_eq!(w.len(), 1, "{:?}", result.warnings);
assert_eq!(w[0].node_id, None);
assert!(w[0].message.contains("'region'"), "{}", w[0].message);
}
#[test]
fn disjoint_variables_and_initial_state_do_not_warn() {
let mut graph = graph_with(vec![("e", end_node("e"))], "e");
graph.variables = vec![variable("region")];
graph
.initial_state
.insert("zone".into(), serde_json::json!("a"));
let result = validator().validate(&graph);
assert!(
shadowing_warnings(&result).is_empty(),
"disjoint names should not warn: {:?}",
result.warnings
);
}
#[test]
fn each_shadowed_variable_warns_once() {
let mut graph = graph_with(vec![("e", end_node("e"))], "e");
graph.variables = vec![variable("region"), variable("tier")];
graph
.initial_state
.insert("region".into(), serde_json::json!("us"));
graph
.initial_state
.insert("tier".into(), serde_json::json!("gold"));
let result = validator().validate(&graph);
let w = shadowing_warnings(&result);
assert_eq!(w.len(), 2, "{:?}", result.warnings);
}
fn timeout_warnings(result: &ValidationResult) -> Vec<&ValidationError> {
result
.warnings
+4 -2
View File
@@ -367,9 +367,11 @@ mod tests {
file_name: &str,
parent: &Path,
) -> Result<Vec<FunctionDeclaration>> {
let pid = std::process::id();
let unique = PARSE_COUNTER.fetch_add(1, Ordering::Relaxed);
let path =
std::env::temp_dir().join(format!("coyote_python_parser_{file_name}_{unique}.py"));
let path = std::env::temp_dir().join(format!(
"coyote_python_parser_{file_name}_{pid}_{unique}.py"
));
fs::write(&path, source).expect("failed to write temp python source");
let file = File::open(&path).expect("failed to open temp python source");
let result = generate_python_declarations(file, file_name, Some(parent));
+7 -6
View File
@@ -414,18 +414,19 @@ mod tests {
use super::*;
use crate::function::JsonSchema;
use std::fs;
use std::time::{SystemTime, UNIX_EPOCH};
use std::sync::atomic::{AtomicU64, Ordering};
static PARSE_COUNTER: AtomicU64 = AtomicU64::new(0);
fn parse_ts_source(
source: &str,
file_name: &str,
parent: &Path,
) -> Result<Vec<FunctionDeclaration>> {
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("time")
.as_nanos();
let path = std::env::temp_dir().join(format!("coyote_ts_parser_{file_name}_{unique}.ts"));
let pid = std::process::id();
let unique = PARSE_COUNTER.fetch_add(1, Ordering::Relaxed);
let path =
std::env::temp_dir().join(format!("coyote_ts_parser_{file_name}_{pid}_{unique}.ts"));
fs::write(&path, source).expect("write");
let file = File::open(&path).expect("open");
let result = generate_typescript_declarations(file, file_name, Some(parent));
+1 -1
View File
@@ -504,7 +504,7 @@ mod tests {
let mut sup = Supervisor::new(4, 3);
let id = sup
.task_queue_mut()
.create("task".into(), "desc".into(), None, None);
.create("task".into(), "desc".into(), None, None, None);
assert!(!id.is_empty());
assert_eq!(sup.task_queue().list().len(), 1);
}
+63 -20
View File
@@ -1,3 +1,4 @@
use crate::config::AgentVariables;
use serde::{Deserialize, Serialize};
use std::collections::{HashMap, HashSet};
@@ -22,6 +23,7 @@ pub struct TaskNode {
pub blocks: HashSet<String>,
pub dispatch_agent: Option<String>,
pub prompt: Option<String>,
pub variables: Option<AgentVariables>,
}
impl TaskNode {
@@ -31,6 +33,7 @@ impl TaskNode {
description: String,
dispatch_agent: Option<String>,
prompt: Option<String>,
variables: Option<AgentVariables>,
) -> Self {
Self {
id,
@@ -42,6 +45,7 @@ impl TaskNode {
blocks: HashSet::new(),
dispatch_agent,
prompt,
variables,
}
}
@@ -70,10 +74,18 @@ impl TaskQueue {
description: String,
dispatch_agent: Option<String>,
prompt: Option<String>,
variables: Option<AgentVariables>,
) -> String {
let id = self.next_id.to_string();
self.next_id += 1;
let task = TaskNode::new(id.clone(), subject, description, dispatch_agent, prompt);
let task = TaskNode::new(
id.clone(),
subject,
description,
dispatch_agent,
prompt,
variables,
);
self.tasks.insert(id.clone(), task);
id
}
@@ -193,8 +205,15 @@ mod tests {
"Research auth patterns".into(),
None,
None,
None,
);
let id2 = queue.create(
"Implement".into(),
"Write the code".into(),
None,
None,
None,
);
let id2 = queue.create("Implement".into(), "Write the code".into(), None, None);
assert_eq!(id1, "1");
assert_eq!(id2, "2");
@@ -204,8 +223,8 @@ mod tests {
#[test]
fn test_dependency_and_completion() {
let mut queue = TaskQueue::new();
let id1 = queue.create("Step 1".into(), "".into(), None, None);
let id2 = queue.create("Step 2".into(), "".into(), None, None);
let id1 = queue.create("Step 1".into(), "".into(), None, None, None);
let id2 = queue.create("Step 2".into(), "".into(), None, None, None);
queue.add_dependency(&id2, &id1).unwrap();
@@ -221,9 +240,9 @@ mod tests {
#[test]
fn test_fan_in_dependency() {
let mut queue = TaskQueue::new();
let id1 = queue.create("A".into(), "".into(), None, None);
let id2 = queue.create("B".into(), "".into(), None, None);
let id3 = queue.create("C (needs A and B)".into(), "".into(), None, None);
let id1 = queue.create("A".into(), "".into(), None, None, None);
let id2 = queue.create("B".into(), "".into(), None, None, None);
let id3 = queue.create("C (needs A and B)".into(), "".into(), None, None, None);
queue.add_dependency(&id3, &id1).unwrap();
queue.add_dependency(&id3, &id2).unwrap();
@@ -242,8 +261,8 @@ mod tests {
#[test]
fn test_cycle_detection() {
let mut queue = TaskQueue::new();
let id1 = queue.create("A".into(), "".into(), None, None);
let id2 = queue.create("B".into(), "".into(), None, None);
let id1 = queue.create("A".into(), "".into(), None, None, None);
let id2 = queue.create("B".into(), "".into(), None, None, None);
queue.add_dependency(&id2, &id1).unwrap();
let result = queue.add_dependency(&id1, &id2);
@@ -254,7 +273,7 @@ mod tests {
#[test]
fn test_self_dependency_rejected() {
let mut queue = TaskQueue::new();
let id1 = queue.create("A".into(), "".into(), None, None);
let id1 = queue.create("A".into(), "".into(), None, None, None);
let result = queue.add_dependency(&id1, &id1);
assert!(result.is_err());
}
@@ -262,7 +281,7 @@ mod tests {
#[test]
fn test_claim() {
let mut queue = TaskQueue::new();
let id1 = queue.create("Task".into(), "".into(), None, None);
let id1 = queue.create("Task".into(), "".into(), None, None, None);
assert!(queue.claim(&id1, "worker-1"));
assert!(!queue.claim(&id1, "worker-2"));
@@ -272,7 +291,7 @@ mod tests {
#[test]
fn test_fail_sets_status() {
let mut queue = TaskQueue::new();
let id = queue.create("Task".into(), "".into(), None, None);
let id = queue.create("Task".into(), "".into(), None, None, None);
queue.fail(&id);
assert_eq!(queue.get(&id).unwrap().status, TaskStatus::Failed);
}
@@ -291,17 +310,41 @@ mod tests {
"desc".into(),
Some("coder".into()),
Some("implement feature".into()),
None,
);
let task = queue.get(&id).unwrap();
assert_eq!(task.dispatch_agent.as_deref(), Some("coder"));
assert_eq!(task.prompt.as_deref(), Some("implement feature"));
}
#[test]
fn test_variables_stored() {
let mut queue = TaskQueue::new();
let id = queue.create(
"Auto task".into(),
"desc".into(),
Some("coder".into()),
Some("implement feature".into()),
Some(AgentVariables::from([(
"pr".to_string(),
"1234".to_string(),
)])),
);
let task = queue.get(&id).unwrap();
assert_eq!(
task.variables
.as_ref()
.and_then(|v| v.get("pr"))
.map(String::as_str),
Some("1234")
);
}
#[test]
fn test_claim_blocked_task_fails() {
let mut queue = TaskQueue::new();
let id1 = queue.create("A".into(), "".into(), None, None);
let id2 = queue.create("B".into(), "".into(), None, None);
let id1 = queue.create("A".into(), "".into(), None, None, None);
let id2 = queue.create("B".into(), "".into(), None, None, None);
queue.add_dependency(&id2, &id1).unwrap();
assert!(!queue.claim(&id2, "worker"));
}
@@ -309,9 +352,9 @@ mod tests {
#[test]
fn test_list_sorted_by_id() {
let mut queue = TaskQueue::new();
queue.create("Third".into(), "".into(), None, None);
queue.create("First".into(), "".into(), None, None);
queue.create("Second".into(), "".into(), None, None);
queue.create("Third".into(), "".into(), None, None, None);
queue.create("First".into(), "".into(), None, None, None);
queue.create("Second".into(), "".into(), None, None, None);
let tasks = queue.list();
let ids: Vec<&str> = tasks.iter().map(|t| t.id.as_str()).collect();
assert_eq!(ids, vec!["1", "2", "3"]);
@@ -326,7 +369,7 @@ mod tests {
#[test]
fn test_dependency_on_nonexistent_task_errors() {
let mut queue = TaskQueue::new();
let id1 = queue.create("A".into(), "".into(), None, None);
let id1 = queue.create("A".into(), "".into(), None, None, None);
let result = queue.add_dependency(&id1, "nonexistent");
assert!(result.is_err());
}
@@ -340,13 +383,13 @@ mod tests {
#[test]
fn test_task_node_is_runnable() {
let node = TaskNode::new("1".into(), "t".into(), "d".into(), None, None);
let node = TaskNode::new("1".into(), "t".into(), "d".into(), None, None, None);
assert!(node.is_runnable());
}
#[test]
fn test_task_node_not_runnable_when_blocked() {
let mut node = TaskNode::new("1".into(), "t".into(), "d".into(), None, None);
let mut node = TaskNode::new("1".into(), "t".into(), "d".into(), None, None, None);
node.blocked_by.insert("2".into());
node.status = TaskStatus::Blocked;
assert!(!node.is_runnable());