Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions crates/stella-cli/src/claims.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,14 @@ impl ToolExecutor for ClaimTap<'_> {
fn drain_sub_agent_spend_usd(&self) -> f64 {
self.inner.drain_sub_agent_spend_usd()
}

/// Forwarded: letting the empty default stand would silently serialize the
/// inner executor's sibling spawns (see the port's contract). The spawn
/// tool names no mutating path and no transient lane, so concurrent
/// siblings pass through `execute` above without touching a claim.
fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
self.inner.parallel_safe_names()
}
}

#[cfg(test)]
Expand All @@ -372,12 +380,28 @@ mod tests {
content: "ok".into(),
}
}
fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
std::collections::HashSet::from(["task".to_string()])
}
}

fn store() -> Arc<Store> {
Arc::new(Store::in_memory().unwrap())
}

/// The tap sits directly under the engine in every deck lane, so a tap
/// that swallowed the claim (the default is empty) would serialize
/// sibling spawns no matter what the registry advertised.
#[test]
fn the_claim_tap_forwards_parallel_safe_names() {
let inner = Passthrough(Default::default());
let tap = ClaimTap::new(&inner, None, "ses-1/lead");
assert!(
tap.parallel_safe_names().contains("task"),
"the inner executor's concurrency claim must survive the tap"
);
}

#[tokio::test]
async fn first_write_claims_and_a_rival_is_refused_with_the_holder_named() {
let store = store();
Expand Down
50 changes: 3 additions & 47 deletions crates/stella-cli/src/command_deck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ use stella_pipeline::{
};
use stella_protocol::{
AgentEvent, CiStatus, CompletionMessage, CompletionRequest, ModelRef, PrStatus, TaskItem,
ToolOutput, ToolSchema,
ToolOutput,
};
use stella_store::Store;
use stella_tools::ToolRegistry;
Expand Down Expand Up @@ -118,6 +118,7 @@ mod scope_gate;
mod session_clear;
mod sessions_view;
mod settle;
mod task_tap;
mod theme_cmd;
use crate::memory::{SessionMemory, inject_recall_block};
use crate::runtime::{SystemClock, TokioSleeper};
Expand All @@ -126,6 +127,7 @@ use authoring::{agents_list_creating, agents_list_inbound, handle_agent_create};
pub(crate) use forwarder::spawn_forwarder;
use scope_gate::DeckApprovalGate;
use sessions_view::sessions_inbound;
use task_tap::TaskTap;

/// The lead agent's id — the one conversation this driver runs.
pub(crate) const LEAD: &str = "lead";
Expand Down Expand Up @@ -4641,51 +4643,5 @@ impl AskUserIo for DeckAskUserIo {
}
}

/// Mirrors the task board into the event stream: after any `task_*` tool
/// call the FULL board snapshot rides the turn's channel as
/// `AgentEvent::TaskUpdate` — persisted by the forwarder, so replay shows
/// the checklist exactly as it moved — and `task_assign`'s spawn requests
/// are handed to the driver's supervisor channel. `supervisor: None` is the
/// worker configuration (v1 delegation runs from the lead only; a worker's
/// stranded requests are reported on its lane by `crate::subsession`).
pub(crate) struct TaskTap<'a> {
pub(crate) inner: &'a dyn ToolExecutor,
pub(crate) events: UnboundedSender<AgentEvent>,
pub(crate) registry: &'a ToolRegistry,
pub(crate) supervisor: Option<UnboundedSender<SupervisorMsg>>,
}

#[async_trait]
impl ToolExecutor for TaskTap<'_> {
fn schemas(&self) -> Vec<ToolSchema> {
self.inner.schemas()
}

async fn execute(&self, name: &str, input: &Value) -> ToolOutput {
let output = self.inner.execute(name, input).await;
if name.starts_with("task_") {
let tasks: Vec<TaskItem> = {
let board = self.registry.task_board();
let guard = board.lock().unwrap_or_else(|p| p.into_inner());
guard.items().to_vec()
};
let _ = self.events.send(AgentEvent::TaskUpdate { tasks });
if let Some(sup) = &self.supervisor {
for request in self.registry.take_spawn_requests() {
let _ = sup.send(SupervisorMsg::SpawnTask(request));
}
}
}
output
}

/// Forwarded: this is a decorator, and a decorator that let the default
/// `0.0` stand would silently drop sub-agent spend out of the parent's
/// budget (see the port's contract).
fn drain_sub_agent_spend_usd(&self) -> f64 {
self.inner.drain_sub_agent_spend_usd()
}
}

#[cfg(test)]
mod tests;
107 changes: 107 additions & 0 deletions crates/stella-cli/src/command_deck/task_tap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
//! The deck's task-board tap — split out of `command_deck.rs` (a god file
//! closed to growth) when the tap grew its `parallel_safe_names` forward.

use async_trait::async_trait;
use serde_json::Value;
use stella_core::ports::ToolExecutor;
use stella_protocol::{AgentEvent, TaskItem, ToolOutput, ToolSchema};
use stella_tools::ToolRegistry;
use tokio::sync::mpsc::UnboundedSender;

use crate::subsession::SupervisorMsg;

/// Mirrors the task board into the event stream: after any `task_*` tool
/// call the FULL board snapshot rides the turn's channel as
/// `AgentEvent::TaskUpdate` — persisted by the forwarder, so replay shows
/// the checklist exactly as it moved — and `task_assign`'s spawn requests
/// are handed to the driver's supervisor channel. `supervisor: None` is the
/// worker configuration (v1 delegation runs from the lead only; a worker's
/// stranded requests are reported on its lane by `crate::subsession`).
pub(crate) struct TaskTap<'a> {
pub(crate) inner: &'a dyn ToolExecutor,
pub(crate) events: UnboundedSender<AgentEvent>,
pub(crate) registry: &'a ToolRegistry,
pub(crate) supervisor: Option<UnboundedSender<SupervisorMsg>>,
}

#[async_trait]
impl ToolExecutor for TaskTap<'_> {
fn schemas(&self) -> Vec<ToolSchema> {
self.inner.schemas()
}

async fn execute(&self, name: &str, input: &Value) -> ToolOutput {
let output = self.inner.execute(name, input).await;
if name.starts_with("task_") {
let tasks: Vec<TaskItem> = {
let board = self.registry.task_board();
let guard = board.lock().unwrap_or_else(|p| p.into_inner());
guard.items().to_vec()
};
let _ = self.events.send(AgentEvent::TaskUpdate { tasks });
if let Some(sup) = &self.supervisor {
for request in self.registry.take_spawn_requests() {
let _ = sup.send(SupervisorMsg::SpawnTask(request));
}
}
}
output
}

/// Forwarded: this is a decorator, and a decorator that let the default
/// `0.0` stand would silently drop sub-agent spend out of the parent's
/// budget (see the port's contract).
fn drain_sub_agent_spend_usd(&self) -> f64 {
self.inner.drain_sub_agent_spend_usd()
}

/// Forwarded: letting the empty default stand would silently serialize
/// the inner executor's sibling spawns (see the port's contract). The
/// spawn tool is `task`, not `task_*` — the tap never fires for it.
fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
self.inner.parallel_safe_names()
}
}

#[cfg(test)]
mod tests {
use super::*;

/// A leaf claiming one parallel-safe name, standing in for the registry.
struct Claiming;

#[async_trait]
impl ToolExecutor for Claiming {
fn schemas(&self) -> Vec<ToolSchema> {
Vec::new()
}
async fn execute(&self, _name: &str, _input: &Value) -> ToolOutput {
ToolOutput::Ok {
content: String::new(),
}
}
fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
std::collections::HashSet::from(["task".to_string()])
}
}

/// The deck's lead lane wraps the whole stack in this tap last, so a tap
/// that swallowed the claim would kill concurrent sibling spawns for
/// every deck session no matter what the layers below advertised.
#[test]
fn the_task_tap_forwards_parallel_safe_names() {
let inner = Claiming;
let registry = ToolRegistry::with_issue_backend(std::path::PathBuf::from("."), None);
let (events, _rx) = tokio::sync::mpsc::unbounded_channel();
let tap = TaskTap {
inner: &inner,
events,
registry: &registry,
supervisor: None,
};
assert!(
tap.parallel_safe_names().contains("task"),
"the tap must forward the inner executor's concurrency claims"
);
}
}
8 changes: 8 additions & 0 deletions crates/stella-cli/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,14 @@ impl ToolExecutor for DiscoveryToolSet<'_> {
fn drain_sub_agent_spend_usd(&self) -> f64 {
self.inner.drain_sub_agent_spend_usd()
}

/// Forwarded: letting the empty default stand would silently serialize the
/// inner executor's sibling spawns (see the port's contract). No lean-mode
/// intersection — hiding is a prompt-budget measure, and a hidden tool
/// called by name still executes above.
fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
self.inner.parallel_safe_names()
}
}

/// Split an advertised `mcp__server__tool` name into (server, tool).
Expand Down
26 changes: 26 additions & 0 deletions crates/stella-cli/src/fleet_commits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ impl ToolExecutor for CommitObserver<'_> {
fn drain_sub_agent_spend_usd(&self) -> f64 {
self.inner.drain_sub_agent_spend_usd()
}

/// Forwarded: letting the empty default stand would silently serialize
/// the inner executor's sibling spawns (see the port's contract) — the
/// spawn tool is not commit-lane routed, so nothing is observed for it.
fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
self.inner.parallel_safe_names()
}
}

/// One attempt's commits, oldest first — and the two ways of knowing which
Expand Down Expand Up @@ -317,6 +324,10 @@ mod tests {
Vec::new()
}

fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
std::collections::HashSet::from(["task".to_string()])
}

async fn execute(&self, name: &str, _input: &Value) -> ToolOutput {
if name == "repo_commit" {
let mut script = self.script.lock().unwrap();
Expand Down Expand Up @@ -349,6 +360,21 @@ mod tests {
commits.iter().map(|c| c.sha.as_str()).collect()
}

/// The observer wraps every shared-tree fleet worker's stack, so one that
/// swallowed the claim (the default is empty) would serialize sibling
/// spawns for the whole fleet no matter what the registry advertised.
#[test]
fn the_commit_observer_forwards_parallel_safe_names() {
let tree = Arc::new(SharedTree::default());
let tools = CommittingTools::new(tree.clone(), Vec::new());
assert!(
observer(&tools, &tree, "t1")
.parallel_safe_names()
.contains("task"),
"the inner executor's concurrency claim must survive the observer"
);
}

/// The witness for #1216's first half. Two shared-tree workers commit
/// into ONE tree, interleaved — `t1`, then `t2`, then `t1` again. Every
/// commit must appear under the task that made it and under no other.
Expand Down
7 changes: 7 additions & 0 deletions crates/stella-cli/src/interactive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,13 @@ impl ToolExecutor for InteractiveToolSet<'_> {
fn drain_sub_agent_spend_usd(&self) -> f64 {
self.inner.drain_sub_agent_spend_usd()
}

/// Forwarded: letting the empty default stand would silently serialize the
/// inner executor's sibling spawns (see the port's contract). The tools
/// added here (`ask_user`, skills) make no concurrency claim of their own.
fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
self.inner.parallel_safe_names()
}
}

#[cfg(test)]
Expand Down
10 changes: 6 additions & 4 deletions crates/stella-cli/src/subagent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@
//! `task` calls from one step execute concurrently — reachable since the
//! engine's dispatch scheduler groups the spawn tool with read-only calls
//! (`Tool::parallel_safe`; before that claim existed, every non-read-only
//! call was its own barrier and this concurrency was dead code). Two siblings
//! can therefore each carve against the same headroom before either settles —
//! an overshoot bounded by one child's cap, and caught by the parent's guard
//! at the next step boundary regardless.
//! call was its own barrier and this concurrency was dead code). Every
//! sibling in one dispatch group can therefore carve against the same
//! headroom before any settles — an overshoot bounded by one child's cap per
//! concurrently-running sibling (up to the engine's dispatch cap of 8, so
//! seven extra caps in the worst case, not one), and caught by the parent's
//! guard at the next step boundary regardless.
//!
//! ## Settling is the child's job
//!
Expand Down
38 changes: 38 additions & 0 deletions crates/stella-cli/src/subagent/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ impl ToolExecutor for LedgerBase {
fn drain_sub_agent_spend_usd(&self) -> f64 {
stella_core::subagent::drain_sub_agent_spend(&self.0)
}
fn parallel_safe_names(&self) -> std::collections::HashSet<String> {
// The registry claims exactly this for the `task` tool
// (`Tool::parallel_safe`); the stand-in reports it the same way.
std::collections::HashSet::from(["task".to_string()])
}
}

struct NeverProvider;
Expand Down Expand Up @@ -101,6 +106,39 @@ async fn the_production_tool_stack_forwards_sub_agent_spend() {
);
}

/// **The parallel-dispatch counterpart of the spend witness above.**
///
/// `parallel_safe_names` has an empty default, so any decorator that forgets
/// to forward it silently serializes sibling `task` calls in every real
/// session — the registry's claim never reaches the engine, and the feature
/// (#1776) is dead exactly where it shipped. Asserted through the shipped
/// composition for the same reason as the spend test: a future decorator
/// inserted into the real stack fails here, and nowhere else.
#[tokio::test]
async fn the_production_tool_stack_forwards_parallel_safe_names() {
let ledger: SubAgentSpendLedger = Arc::default();
let base = LedgerBase(ledger);

let customs =
stella_tools::custom::CustomToolSet::new(&base, Vec::new(), std::path::PathBuf::from("."));
let (stub_tx, _rx) = tokio::sync::mpsc::unbounded_channel();
let interactive = crate::interactive::InteractiveToolSet::new(
&customs,
stub_tx,
crate::interactive::default_ask_io(false),
);
let permitted = crate::agent::PolicyToolSet::new(&interactive, Default::default());
let discovery =
crate::discovery::DiscoveryToolSet::new(&permitted, std::path::PathBuf::from("."));

assert!(
discovery.parallel_safe_names().contains("task"),
"the registry's concurrency claim must survive every decorator \
between the engine and the registry — one that swallows it silently \
serializes sibling spawns"
);
}

/// A dispatcher whose registry has been dropped reports a refusal rather
/// than panicking a torn-down session.
#[tokio::test]
Expand Down
Loading
Loading