Skip to content

fix(stella-cli): forward parallel_safe_names through every tool-stack decorator - #1836

Open
macanderson wants to merge 3 commits into
mainfrom
fix/parallel-safe-forwarding
Open

fix(stella-cli): forward parallel_safe_names through every tool-stack decorator#1836
macanderson wants to merge 3 commits into
mainfrom
fix/parallel-safe-forwarding

Conversation

@macanderson

@macanderson macanderson commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Problem

#1776 added ToolExecutor::parallel_safe_names() with an empty default, implemented only by ToolRegistry. But no production session hands the engine a bare registry — the CLI wraps it as DiscoveryToolSet(PolicyToolSet(InteractiveToolSet(CustomToolSet(registry)))), the deck adds TaskTap, and stella-mcp/stella-serve/fleet each have their own decorators. None of them forwarded the method, so the empty default won at the outermost layer and sibling task calls still serialized in every real session — the exact defect #1776 claims to fix, alive one layer up. (Its witness couldn't see this: it implements the trait directly on a bare fake executor.)

Every decorator already forwards the sibling method drain_sub_agent_spend_usd for the same reason; this brings parallel_safe_names to parity in all eleven:

discovery.rs, tool_policy.rs, interactive.rs, custom.rs, command_deck.rs (TaskTap, split into command_deck/task_tap.rs to respect the god-file ceiling), claims.rs, fleet_commits.rs, hunk_review.rs, stella-mcp/toolset.rs (both toolsets), stella-serve/subagents.rs.

PolicyToolSet intersects instead of delegating blindly: a tool the policy withholds is refused by execute, so advertising it as parallel-safe would be an empty promise — forwarded names are filtered through policy.allows.

Also in the same seam:

  • ToolRegistry::parallel_safe_names now recovers a poisoned late_tools lock like every sibling read path (unwrap_or_else(|p| p.into_inner())) instead of silently returning an incomplete set.
  • The sibling-carve overshoot doc in subagent.rs is corrected for the concurrent world: the bound is one child's cap per concurrently-running sibling (dispatch cap 8), not "one child's cap".

Witness

  • subagent::tests::the_production_tool_stack_forwards_parallel_safe_names (stella-cli) — builds the real decorator stack and asserts the registry's claim survives to the outermost layer. Checked the artisanal way: with origin/main's discovery.rs restored it fails (empty set at the top of the stack); with this change it passes.
  • tool_policy: parallel_safe_names_are_forwarded_when_the_policy_allows + a_disabled_tool_is_not_advertised_as_parallel_safe (the two-sided contract).
  • stella-mcp: parallel_safe_names_forward_from_the_native_layer.
  • registry/tests.rs: poisoned-lock recovery.

cargo test: stella-cli 1422 passed, stella-tools 718+119, stella-mcp all green, stella-serve 165+ green · clippy --all-targets -- -D warnings clean on all four crates · cargo fmt --check clean · check-file-size OK (command_deck.rs shrank to 4,696).

Notes

Found by a resilience audit of the #1776 seam. Follow-up defects in the same seam (sibling agent_id collision, the unreachable sub-agent spend-pool ceiling, child-panic spend loss) are being filed as separate issues.

Summary by Sourcery

Ensure tool executor decorators correctly propagate parallel execution capability and harden registry handling of poisoned late tool overlays.

Bug Fixes:

  • Fix parallel_safe_names not being forwarded through tool executor decorator stacks, which caused sibling tool calls to be serialized despite registry claims.
  • Handle poisoned late_tools locks in ToolRegistry::parallel_safe_names so late-enabled tools remain included in concurrency claims instead of being silently dropped.
  • Correct sub-agent spend overshoot documentation to reflect concurrent sibling execution bounds.

Enhancements:

  • Forward parallel_safe_names through all relevant ToolExecutor wrappers across CLI, tools, MCP, serve, and fleet components to align with existing sub-agent spend forwarding.
  • Split the TaskTap implementation into its own module from command_deck.rs to keep the deck driver maintainable.

Tests:

  • Add end-to-end tests in stella-cli to verify the production tool stack and various decorators forward parallel_safe_names correctly.
  • Add tests for PolicyToolSet and MCP toolsets to assert that concurrency claims are forwarded and filtered according to policy and native layers.
  • Add a registry test to validate that parallel_safe_names remains correct when the late tool overlay lock is poisoned.
  • Add decorator-specific tests for TaskTap, ClaimTap, CommitObserver, HunkGate, and DelegatingTools to ensure they preserve inner executors' parallel_safe_names.

Stella Test added 2 commits August 6, 2026 03:17
… decorator

PR #1776 added ToolExecutor::parallel_safe_names with an empty default,
implemented only by ToolRegistry — but every production session wraps the
registry in decorators, and none forwarded it, so the empty default won
and sibling task calls still serialized in every real session.

Forward it through DiscoveryToolSet, PolicyToolSet (intersected with the
policy, mirroring its two-sided schemas/execute shape), InteractiveToolSet,
CustomToolSet, TaskTap, ClaimTap, CommitObserver, HunkGate, McpToolSet and
CandidateMcpView (native layer only — MCP tools never carry the claim),
and serve's DelegatingTools. TaskTap moves to command_deck/task_tap.rs
because command_deck.rs sat at exactly its file-size ceiling (the
driver/settlement.rs split pattern).

Also make ToolRegistry::parallel_safe_names poison-tolerant on late_tools
like every sibling read path — 'if let Ok' silently dropped late-enabled
claims after a panic elsewhere.

Witnesses: the_production_tool_stack_forwards_parallel_safe_names asserts
through the shipped deck composition; per-decorator tests cover the taps
outside that stack; a PolicyToolSet test pins that a disabled tool is not
advertised as parallel-safe; parallel_safe_names_survive_a_poisoned_late_overlay
pins the lock repair.
…rrent dispatch

The 'bounded by one child's cap' claim was written for the serialized
world; with sibling spawns dispatched concurrently the bound is one cap
per concurrently-running sibling, up to the engine's dispatch cap of 8.
@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
stella-cli-docs Ready Ready Preview Aug 6, 2026 11:01am

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Ensures ToolExecutor::parallel_safe_names is correctly forwarded through all tool-stack decorators (so parallel-safe tools like task remain concurrent in real sessions), fixes poisoned-lock handling in ToolRegistry::parallel_safe_names, extracts the CLI TaskTap into its own module, and adds targeted tests across crates to witness the behavior and concurrency guarantees.

Sequence diagram for parallel_safe_names forwarding through the CLI tool stack

sequenceDiagram
    title parallel_safe_names forwarding through CLI tool stack
    participant Engine
    participant TaskTap
    participant DiscoveryToolSet
    participant PolicyToolSet
    participant InteractiveToolSet
    participant CustomToolSet
    participant ToolRegistry
    participant ToolPolicy

    Engine->>TaskTap: parallel_safe_names()
    TaskTap->>DiscoveryToolSet: parallel_safe_names()
    DiscoveryToolSet->>PolicyToolSet: parallel_safe_names()
    PolicyToolSet->>InteractiveToolSet: parallel_safe_names()
    InteractiveToolSet->>CustomToolSet: parallel_safe_names()
    CustomToolSet->>ToolRegistry: parallel_safe_names()

    ToolRegistry->>ToolRegistry: parallel_safe_names()
    ToolRegistry-->>CustomToolSet: HashSet<String>
    CustomToolSet-->>InteractiveToolSet: HashSet<String>
    InteractiveToolSet-->>PolicyToolSet: HashSet<String>

    loop filter by policy
        PolicyToolSet->>ToolPolicy: allows(name)
        ToolPolicy-->>PolicyToolSet: bool
    end

    PolicyToolSet-->>DiscoveryToolSet: filtered HashSet<String>
    DiscoveryToolSet-->>TaskTap: filtered HashSet<String>
    TaskTap-->>Engine: filtered HashSet<String>
Loading

File-Level Changes

Change Details Files
Forward ToolExecutor::parallel_safe_names through all relevant decorators so the registry’s parallel-safe claims survive to the engine in real, stacked executors.
  • Add parallel_safe_names forwarding methods that delegate to the wrapped executor in DiscoveryToolSet, InteractiveToolSet, CustomToolSet, ClaimTap, CommitObserver, HunkGate, DelegatingTools, McpToolSet, CandidateMcpView, PolicyToolSet, and the CLI TaskTap.
  • In PolicyToolSet, intersect the forwarded name set with the active policy (policy.allows) instead of blindly delegating, mirroring the schemas/execute contract.
  • Document the rationale for forwarding in each decorator (empty default would silently serialize sibling spawns) and, where relevant, note which tools are or are not included in the claim.
crates/stella-cli/src/discovery.rs
crates/stella-cli/src/interactive.rs
crates/stella-tools/src/custom.rs
crates/stella-cli/src/claims.rs
crates/stella-cli/src/fleet_commits.rs
crates/stella-tools/src/hunk_review.rs
crates/stella-serve/src/subagents.rs
crates/stella-mcp/src/toolset.rs
crates/stella-cli/src/tool_policy.rs
crates/stella-cli/src/command_deck.rs
crates/stella-cli/src/command_deck/task_tap.rs
Harden ToolRegistry::parallel_safe_names against poisoned late_tools locks and add a focused test witness.
  • Change ToolRegistry::parallel_safe_names to recover from a poisoned late_tools RwLock via unwrap_or_else(
p
Split the deck’s TaskTap ToolExecutor decorator into its own module and extend it to forward parallel_safe_names with a unit test.
  • Move TaskTap definition and its ToolExecutor impl (schemas, execute, drain_sub_agent_spend_usd) from command_deck.rs into a new command_deck/task_tap.rs module.
  • Wire the new module into command_deck.rs via a new mod task_tap; and use task_tap::TaskTap re-export, reducing command_deck.rs size.
  • Extend TaskTap to forward parallel_safe_names from its inner executor and add a dedicated test that asserts the forwarded set includes "task".
crates/stella-cli/src/command_deck.rs
crates/stella-cli/src/command_deck/task_tap.rs
Add integration-style witnesses that the production tool stack and various wrappers preserve parallel_safe_names end-to-end.
  • In stella-cli subagent tests, extend the LedgerBase stub ToolExecutor to report parallel_safe_names containing "task" and add the_production_tool_stack_forwards_parallel_safe_names to assert that the full decorator stack (CustomToolSet, InteractiveToolSet, PolicyToolSet, DiscoveryToolSet) preserves the claim.
  • Add unit tests for PolicyToolSet (forwarding when allowed; empty when disabled), ClaimTap, CommitObserver, HunkGate, DelegatingTools, McpToolSet, and CandidateMcpView, each asserting that their parallel_safe_names includes a known claimed tool name when wrapped.
  • In stella-mcp tests, add end-to-end coverage that both McpToolSet and CandidateMcpView forward the native layer’s parallel-safe claim, and that a set with no native layer yields an empty claim set.
crates/stella-cli/src/subagent/tests.rs
crates/stella-cli/src/tool_policy.rs
crates/stella-cli/src/claims.rs
crates/stella-cli/src/fleet_commits.rs
crates/stella-tools/src/hunk_review.rs
crates/stella-serve/src/subagents.rs
crates/stella-mcp/src/toolset.rs
crates/stella-cli/src/command_deck/task_tap.rs
Clarify subagent overshoot documentation for the concurrent execution model.
  • Update the subagent.rs docs to explain that overshoot is bounded by one child’s cap per concurrently running sibling (up to the dispatch cap of 8), rather than just one child’s cap.
crates/stella-cli/src/subagent.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

# Conflicts:
#	crates/stella-cli/src/command_deck.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant