fix(agent): enforce spawn-hierarchy tier gate at runtime (#4098) (#4102)

This commit is contained in:
oxoxDev
2026-06-25 09:31:21 -07:00
committed by GitHub
parent 4f27b99516
commit 69b36b936c
7 changed files with 294 additions and 17 deletions
+51
View File
@@ -312,6 +312,57 @@ impl AgentTier {
}
}
impl std::fmt::Display for AgentTier {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(self.as_str())
}
}
/// Single source of truth for the spawn-hierarchy rule: is a `parent`-tier
/// agent allowed to delegate to a `child`-tier agent?
///
/// Returns `Ok(())` for the legal handoffs and `Err(reason)` for the three
/// forbidden shapes, where `reason` is a tier-only human-readable explanation
/// (no agent ids — callers prepend their own context):
///
/// - `Worker → *` — workers are leaf executors and must not spawn anything.
/// - `Chat → Chat` — the chat tier is a leaf in its own dimension; cloning it
/// defeats the fast-path and risks unbounded `chat → chat → …` chains.
/// - `Reasoning → Reasoning` — reasoning agents compose downward into workers,
/// not into each other (a depth-blowing recursion of slow models).
///
/// Note this forbids same-tier and worker-as-parent hops, **not** upward hops:
/// `reasoning → chat` is a real, intentional builtin edge (the `subconscious`
/// reasoner can hand a follow-up back to the `orchestrator` chat agent), so it
/// must stay legal. The harness'es `MAX_SPAWN_DEPTH` cap bounds chain length
/// independently of tier direction.
///
/// This is the static authoring rule the loader walks over declared `subagents`
/// pairs at boot (see
/// [`crate::openhuman::agent_registry::agents::validate_tier_hierarchy`]). The
/// runtime spawn gate (`run_subagent`) reuses it as defense-in-depth, but
/// deliberately exempts worker *parents* — at runtime a worker only reaches the
/// spawn chokepoint via the documented collapsed `delegate_to_integrations_agent`
/// path (→ `integrations_agent`, itself a worker), which the loader intentionally
/// leaves untouched.
pub fn validate_tier_transition(parent: AgentTier, child: AgentTier) -> Result<(), String> {
match (parent, child) {
(AgentTier::Worker, _) => Err(format!(
"a `worker` tier agent must not spawn `{}` — workers are leaf executors",
child.as_str()
)),
(AgentTier::Chat, AgentTier::Chat) => Err(
"the chat tier is a leaf in its own dimension — hand off to a `reasoning` or \
`worker` agent instead"
.to_string(),
),
(AgentTier::Reasoning, AgentTier::Reasoning) => {
Err("reasoning agents compose downward into workers, not into each other".to_string())
}
_ => Ok(()),
}
}
// ─────────────────────────────────────────────────────────────────────────────
// Subagent delegation entries
// ─────────────────────────────────────────────────────────────────────────────
@@ -281,3 +281,61 @@ max_iterations = 1
assert_eq!(def.iteration_policy, IterationPolicy::Strict);
assert_eq!(def.effective_max_iterations(), 1);
}
// ── Spawn-hierarchy tier rule (validate_tier_transition) ────────────────────
// Single source of truth shared by the boot loader walk and the runtime
// spawn gate in `run_subagent` (issue #4098).
#[test]
fn tier_transition_allows_legal_descending_handoffs() {
// chat → reasoning, chat → worker, reasoning → worker are the only
// legal hops; each strictly descends the hierarchy.
assert!(validate_tier_transition(AgentTier::Chat, AgentTier::Reasoning).is_ok());
assert!(validate_tier_transition(AgentTier::Chat, AgentTier::Worker).is_ok());
assert!(validate_tier_transition(AgentTier::Reasoning, AgentTier::Worker).is_ok());
}
#[test]
fn tier_transition_rejects_chat_to_chat() {
let err = validate_tier_transition(AgentTier::Chat, AgentTier::Chat)
.expect_err("chat→chat must be rejected");
// Wording must carry the substrings the loader diagnostics rely on.
assert!(err.contains("chat") && err.contains("leaf"), "got: {err}");
}
#[test]
fn tier_transition_rejects_reasoning_to_reasoning() {
let err = validate_tier_transition(AgentTier::Reasoning, AgentTier::Reasoning)
.expect_err("reasoning→reasoning must be rejected");
assert!(err.contains("reasoning"), "got: {err}");
}
#[test]
fn tier_transition_allows_upward_reasoning_to_chat() {
// Upward delegation is intentionally legal: the `subconscious` reasoner
// hands follow-ups back to the `orchestrator` chat agent. Only same-tier
// and worker-as-parent hops are forbidden.
assert!(validate_tier_transition(AgentTier::Reasoning, AgentTier::Chat).is_ok());
}
#[test]
fn tier_transition_rejects_worker_as_parent() {
// A worker is a leaf executor — it may not spawn any tier, including
// another worker.
for child in [AgentTier::Chat, AgentTier::Reasoning, AgentTier::Worker] {
let err = validate_tier_transition(AgentTier::Worker, child)
.expect_err("worker must not spawn any tier");
assert!(
err.contains("worker") && err.contains("leaf"),
"worker→{} reason should call out the worker-leaf rule, got: {err}",
child.as_str()
);
}
}
#[test]
fn tier_display_matches_as_str() {
assert_eq!(AgentTier::Chat.to_string(), "chat");
assert_eq!(AgentTier::Reasoning.to_string(), "reasoning");
assert_eq!(AgentTier::Worker.to_string(), "worker");
}
@@ -11,7 +11,8 @@ use std::sync::Arc;
use std::time::Instant;
use crate::openhuman::agent::harness::definition::{
AgentDefinition, IterationPolicy, PromptSource,
validate_tier_transition, AgentDefinition, AgentDefinitionRegistry, AgentTier, IterationPolicy,
PromptSource,
};
use crate::openhuman::agent::harness::fork_context::{current_parent, ParentExecutionContext};
use crate::openhuman::agent::harness::subagent_runner::extract_tool::ExtractFromResultTool;
@@ -37,6 +38,58 @@ use super::provider::{
resolve_subagent_provider, user_is_signed_in_to_composio, LazyToolkitResolver,
};
/// Runtime spawn-hierarchy gate decision for one delegation hop.
///
/// `parent_def` is the resolved parent agent definition (looked up from the
/// global registry by its definition id) or `None` when the parent can't be
/// resolved — e.g. a dynamically-named agent (model-council juror) or a custom
/// agent absent from the registry, or any context where the registry isn't
/// initialised. A `None` parent yields `Ok(())`: we skip rather than mask, the
/// same defensive posture the loader takes for unknown child ids.
///
/// A **worker** parent is also exempted. At runtime a worker only reaches the
/// spawn chokepoint via the documented collapsed `delegate_to_integrations_agent`
/// path (→ `integrations_agent`, itself a worker) — a shape the loader
/// intentionally leaves untouched. Re-denying it here would turn valid custom
/// worker agents that use `{ skills = "*" }` into runtime failures. The
/// worker-leaf authoring rule stays enforced statically at boot, and the
/// per-parent allowlist gate blocks any other worker spawn.
///
/// For chat / reasoning parents the hop is checked against
/// [`validate_tier_transition`] (the single source of truth shared with the
/// boot loader walk); a forbidden hop is logged and becomes a
/// [`SubagentRunError::TierViolation`]. Logging lives here (rather than at the
/// call site) so the deny path is exercised by this fn's unit tests.
pub(crate) fn tier_gate_decision(
parent_def: Option<&AgentDefinition>,
child: &AgentDefinition,
parent_agent_id: &str,
task_id: &str,
) -> Result<(), SubagentRunError> {
let Some(parent_def) = parent_def else {
return Ok(());
};
if parent_def.agent_tier == AgentTier::Worker {
return Ok(());
}
if let Err(reason) = validate_tier_transition(parent_def.agent_tier, child.agent_tier) {
tracing::warn!(
parent_agent = %parent_agent_id,
parent_tier = %parent_def.agent_tier,
child_agent = %child.id,
child_tier = %child.agent_tier,
task_id = %task_id,
"[subagent_runner] blocked tier-violating delegation: {reason}"
);
return Err(SubagentRunError::TierViolation {
parent_tier: parent_def.agent_tier,
child_tier: child.agent_tier,
reason,
});
}
Ok(())
}
/// Run a sub-agent based on its definition and a task prompt.
///
/// This is the primary entry point for agent delegation. It performs the following:
@@ -90,6 +143,17 @@ pub async fn run_subagent(
});
}
// Runtime spawn-hierarchy (tier) gate — defense-in-depth alongside the
// depth gate above. The loader validates *declared* `subagents` pairs
// statically at boot (`validate_tier_hierarchy`), but dynamic, custom,
// or model-chosen spawns reach this chokepoint without ever passing
// through that walk. Resolve the parent's tier from the registry by its
// definition id; `tier_gate_decision` rejects (and logs) any forbidden
// chat/reasoning hop while exempting unresolved + worker parents.
let parent_def =
AgentDefinitionRegistry::global().and_then(|reg| reg.get(&parent.agent_definition_id));
tier_gate_decision(parent_def, definition, &parent.agent_definition_id, &task_id)?;
tracing::info!(
agent_id = %definition.id,
task_id = %task_id,
@@ -1542,3 +1542,99 @@ fn prefix_tier_refuses_ambiguous_and_short_slugs() {
// Short slug below the length gate never engages the prefix tier.
assert!(resolver.resolve("NOTION").is_none());
}
// ── Runtime spawn-hierarchy (tier) gate (issue #4098) ───────────────────────
// `tier_gate_decision` is the pure decision the runtime gate in `run_subagent`
// applies to each delegation hop. Tested directly so the deny/allow/skip
// table is covered without standing up a global registry or a live spawn.
// Thin wrapper to call the gate with throwaway log-context ids.
fn gate(parent: Option<&AgentDefinition>, child: &AgentDefinition) -> Result<(), SubagentRunError> {
super::runner::tier_gate_decision(parent, child, "parent-agent", "task-1")
}
#[test]
fn tier_gate_skips_when_parent_unresolved() {
use crate::openhuman::agent::harness::definition::AgentTier;
// No resolvable parent definition (e.g. registry uninitialised, or a
// dynamically-named model-council juror / custom agent absent from it) →
// skip rather than mask. Even a would-be-illegal child tier passes, because
// we have no parent tier to judge against.
let mut child = make_def_named_tools(&[]);
child.agent_tier = AgentTier::Chat;
assert!(gate(None, &child).is_ok());
}
#[test]
fn tier_gate_allows_legal_descending_hops() {
use crate::openhuman::agent::harness::definition::AgentTier;
let mut parent = make_def_named_tools(&[]);
let mut child = make_def_named_tools(&[]);
// chat → worker
parent.agent_tier = AgentTier::Chat;
child.agent_tier = AgentTier::Worker;
assert!(gate(Some(&parent), &child).is_ok());
// chat → reasoning
child.agent_tier = AgentTier::Reasoning;
assert!(gate(Some(&parent), &child).is_ok());
// reasoning → worker
parent.agent_tier = AgentTier::Reasoning;
child.agent_tier = AgentTier::Worker;
assert!(gate(Some(&parent), &child).is_ok());
}
#[test]
fn tier_gate_allows_worker_parent_for_collapsed_integration() {
use crate::openhuman::agent::harness::definition::AgentTier;
// A worker only reaches the runtime spawn chokepoint via the documented
// collapsed `delegate_to_integrations_agent` path (→ `integrations_agent`,
// itself a worker). The gate must NOT re-deny that — the worker-leaf rule
// is a static boot-time authoring constraint, not a runtime one. Regression
// for the wildcard-integration case (CodeRabbit P2 on PR #4102).
let mut parent = make_def_named_tools(&[]);
let child = make_def_named_tools(&[]); // worker by default
parent.agent_tier = AgentTier::Worker;
assert!(gate(Some(&parent), &child).is_ok());
}
#[test]
fn tier_gate_denies_chat_to_chat() {
use crate::openhuman::agent::harness::definition::AgentTier;
let mut parent = make_def_named_tools(&[]);
let mut child = make_def_named_tools(&[]);
parent.agent_tier = AgentTier::Chat;
child.agent_tier = AgentTier::Chat;
let err =
gate(Some(&parent), &child).expect_err("chat→chat must be denied at the runtime gate");
match err {
SubagentRunError::TierViolation {
parent_tier,
child_tier,
reason,
} => {
assert_eq!(parent_tier, AgentTier::Chat);
assert_eq!(child_tier, AgentTier::Chat);
assert!(
reason.contains("chat") && reason.contains("leaf"),
"got: {reason}"
);
}
other => panic!("expected TierViolation, got: {other:?}"),
}
}
#[test]
fn tier_gate_allows_upward_reasoning_to_chat() {
use crate::openhuman::agent::harness::definition::AgentTier;
// Upward delegation is intentionally legal (subconscious reasoner →
// orchestrator chat). The gate must not deny it.
let mut parent = make_def_named_tools(&[]);
let mut child = make_def_named_tools(&[]);
parent.agent_tier = AgentTier::Reasoning;
child.agent_tier = AgentTier::Chat;
assert!(gate(Some(&parent), &child).is_ok());
}
@@ -7,6 +7,7 @@ use std::path::PathBuf;
use std::time::Duration;
use thiserror::Error;
use crate::openhuman::agent::harness::definition::AgentTier;
use crate::openhuman::inference::provider::ChatMessage;
/// Per-spawn options that override or augment what the
@@ -178,6 +179,16 @@ pub enum SubagentRunError {
max_depth: usize,
},
#[error(
"delegation blocked by the spawn-hierarchy gate: a `{parent_tier}` agent may not \
delegate to a `{child_tier}` agent — {reason}"
)]
TierViolation {
parent_tier: AgentTier,
child_tier: AgentTier,
reason: String,
},
#[error("sub-agent exceeded maximum iterations ({0})")]
MaxIterationsExceeded(usize),
}
+12 -15
View File
@@ -35,7 +35,8 @@
//! collision.
use crate::openhuman::agent::harness::definition::{
AgentDefinition, AgentTier, DefinitionSource, PromptBuilder, PromptSource, SubagentEntry,
validate_tier_transition, AgentDefinition, AgentTier, DefinitionSource, PromptBuilder,
PromptSource, SubagentEntry,
};
use anyhow::{Context, Result};
use std::collections::HashMap;
@@ -305,22 +306,18 @@ pub fn validate_tier_hierarchy(defs: &[AgentDefinition]) -> Result<()> {
// Same-tier delegation is forbidden for chat and reasoning.
// (Chat→Chat would defeat the whole point of the fast tier;
// Reasoning→Reasoning produces a depth-blowing recursion of
// slow models.)
match (def.agent_tier, child_tier) {
(AgentTier::Chat, AgentTier::Chat) => anyhow::bail!(
"agent `{parent}` (chat) lists `{child}` (chat) in subagents — the chat tier \
is a leaf in its own dimension. Hand off to a `reasoning` or `worker` agent \
instead.",
// slow models.) The pair-rule lives in `validate_tier_transition`
// (the single source of truth shared with the runtime spawn gate
// in `run_subagent`); here we wrap its reason with the offending
// agent ids + tiers for a boot-time-friendly diagnostic.
if let Err(reason) = validate_tier_transition(def.agent_tier, child_tier) {
anyhow::bail!(
"agent `{parent}` ({ptier}) lists `{child}` ({ctier}) in subagents — {reason}",
parent = def.id,
ptier = def.agent_tier.as_str(),
child = child_id,
),
(AgentTier::Reasoning, AgentTier::Reasoning) => anyhow::bail!(
"agent `{parent}` (reasoning) lists `{child}` (reasoning) in subagents — \
reasoning agents compose downward into workers, not into each other.",
parent = def.id,
child = child_id,
),
_ => {}
ctier = child_tier.as_str(),
);
}
}
}
@@ -59,7 +59,7 @@ You can open and operate native apps on this machine, but you do it by **delegat
- **You are the chat tier.** You run on a fast UX-focused model (TTFT > deep reasoning). When a task needs sustained multi-step thinking — planning across many steps, comparing several non-obvious options, untangling ambiguous requirements — **delegate to the reasoning tier (`delegate_plan`)** rather than reasoning through it yourself. Your job at that point is to brief the planner well and synthesise its output back to the user.
- **Never spawn yourself** — You cannot delegate to another chat-tier agent (Orchestrator or otherwise). The chat tier is a leaf in its own dimension.
- **Spawn hierarchy (hard rule).** Allowed handoffs from here: `chat → worker` (fast path) or `chat → reasoning → worker` (deep path). Never `chat → chat` and never `chat → reasoning → reasoning`. The loader rejects same-tier delegation at boot; a runtime depth gate capping chains at 3 hops is a planned follow-up — until it lands, this rule is enforced by you, by the planner's matching rule, and by the static loader check.
- **Spawn hierarchy (hard rule).** Allowed handoffs from here: `chat → worker` (fast path) or `chat → reasoning → worker` (deep path). Never `chat → chat` and never `chat → reasoning → reasoning`. This is enforced in depth: the loader rejects same-tier delegation at boot, and the spawn chokepoint denies any tier-violating or over-deep spawn at runtime (a depth gate caps chains at 3 hops and a tier gate rejects the forbidden hops). Those gates are a safety net, not a license to mis-route — still follow the hierarchy yourself, as does the planner's matching rule.
- **Minimise sub-agents** — Use the fewest agents necessary. Simple questions don't need a DAG.
- **Direct-first always** — First try direct reply or direct tools; delegate only when required by task complexity/capability gaps.
- **Context is expensive** — Pass only relevant context to sub-agents, not everything.