Skip to content

feat: worker context injection and duplicate worker spawn prevention#358

Open
jamiepine wants to merge 5 commits intomainfrom
feat/worker-context-injection
Open

feat: worker context injection and duplicate worker spawn prevention#358
jamiepine wants to merge 5 commits intomainfrom
feat/worker-context-injection

Conversation

@jamiepine
Copy link
Member

@jamiepine jamiepine commented Mar 8, 2026

Summary

  • Add context injection mechanism that lets channels deliver addendum messages to running workers mid-task, using a terminate-and-resume pattern through SpacebotHook.on_completion_call — no Rig framework changes needed
  • Extend the route tool to support both interactive follow-up (worker_inputs) and context injection for running workers (worker_injections), so a single tool handles both WaitingForInput and Running workers
  • Add duplicate worker task guard that checks the status block for exact task description matches before spawning, preventing redundant workers when users send rapid-fire messages

How it works

  1. Every worker now returns an inject_tx channel from its constructor, stored in ChannelState.worker_injections
  2. The inject_rx is wired into SpacebotHook via with_inject_rx() at the start of Worker::run()
  3. Before each LLM call, on_completion_call drains pending injected messages and returns HookAction::Terminate with reason spacebot_context_injection
  4. prompt_with_tool_nudge_retry catches this termination (before the nudge arm), appends injected messages as [Context update from the user] entries in history, and re-prompts with a continuation hint
  5. Context injection does not count against the nudge attempt budget

Changes

File What
src/agent/channel.rs Add worker_injections field to ChannelState, cleanup in WorkerComplete and cancel_worker_with_reason
src/agent/channel_dispatch.rs Store inject_tx on worker spawn, add dedup guard via find_duplicate_worker_task
src/agent/cortex.rs Drop inject_tx for detached workers (no parent channel)
src/agent/status.rs Add find_duplicate_worker_task() with [opencode] prefix normalization
src/agent/worker.rs Add inject_rx field, return inject_tx from all constructors, wire into hook in run()
src/error.rs Add DuplicateWorkerTask error variant
src/hooks/spacebot.rs Add injection fields, CONTEXT_INJECTION_REASON, drain logic in on_completion_call, injection handler in retry loop
src/tools/route.rs Try worker_inputs first, fall back to worker_injections
src/tools/spawn_worker.rs Add dedup guard check before spawning
prompts/en/tools/route_description.md.j2 Describe both follow-up and injection capabilities
tests/context_dump.rs Add worker_injections field to test constructors

Testing

  • 6 new injection tests: terminates on pending messages, continues when empty, drains multiple, clears buffer, reason detection, no interference with nudge
  • 5 new dedup guard tests: exact match, no match, opencode prefix stripping (both directions), empty status block
  • just gate-pr passes (468 tests, clippy clean, fmt clean, integration tests compile)

Race/terminal-state reasoning

  • Injection vs. completion race: on_completion_call uses try_recv (non-blocking) so it only picks up messages already in the channel buffer. If a message arrives mid-LLM-call, it gets picked up on the next turn boundary — no lost messages.
  • Injection vs. worker completion race: If the worker completes before the injection is consumed, the inject_tx is cleaned up in the WorkerComplete handler and cancel_worker_with_reason. The channel's send() will fail with a closed channel error, which the route tool surfaces as "worker has stopped running."
  • Injection vs. nudge independence: The injection match arm runs before the nudge arm and does not modify the nudge attempt counter, so both systems operate independently without interfering.

Note

This PR implements worker context injection—allowing channels to send context updates to running workers mid-task without spawning duplicates. Uses a terminate-and-resume pattern through the hook system. 11 files modified, 11 tests added, all delivery gates pass.

Written by Tembo for commit c4e56d5. This will update automatically on new commits.

Add the ability for channels to deliver addendum context to running workers
mid-task via a terminate-and-resume pattern. When a user sends a follow-up
message while a worker is already running, the channel can route it as
injected context rather than spawning a duplicate worker.

The route tool now tries interactive input first (WaitingForInput workers),
then falls back to context injection (Running workers). The SpacebotHook
drains pending injections in on_completion_call and terminates the agent
loop so prompt_with_tool_nudge_retry can append the messages to history
and re-prompt — no Rig framework changes needed.

Also adds a duplicate worker task guard (from closed PR #281) that checks
the status block for exact task description matches before spawning.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 8, 2026

Walkthrough

Adds per-worker context-injection channels and plumbing to inject non-interactive messages into running workers at LLM turn boundaries, introduces duplicate-task reservation/detection to prevent double-spawn, updates routing to choose interactive follow-up or injection, and buffers/drains injected messages in SpacebotHook.

Changes

Cohort / File(s) Summary
Worker runtime & constructors
src/agent/worker.rs, src/agent/cortex.rs
Worker carries an inject_rx and constructors now return an inject_tx alongside input senders; detached worker creation drops the inject sender; run drains inject channel pre-LLM-turn.
Channel state & tests
src/agent/channel.rs, tests/context_dump.rs
Added worker_injections: Arc<RwLock<HashMap<WorkerId, mpsc::Sender<String>>>> and reserved_tasks: Arc<RwLock<HashSet<String>>> to ChannelState; initialize in tests; cleanup injection entries on cancel/complete.
Dispatch, spawn & reservation
src/agent/channel_dispatch.rs, src/tools/spawn_worker.rs
Added reserve_task_if_unique / release_task_reservation; spawn flows check/reserve tasks to prevent duplicates; store per-worker inject_tx on spawn/resume; spawn signatures adjusted to use task: impl Into<String>.
Status duplicate detection
src/agent/status.rs
Added StatusBlock::find_duplicate_worker_task(&self, task: &str) -> Option<WorkerId> which normalizes by stripping "[opencode] " prefix; includes unit tests.
Routing & route docs
src/tools/route.rs, prompts/en/tools/route_description.md.j2
Route now queries status to choose interactive input (follow-up) or context injection for running workers; updated route output messages and prompt description wording.
Spacebot hook injection handling
src/hooks/spacebot.rs
SpacebotHook gains optional inject_rx and injected_messages buffer; drains inject_rx before LLM prompt and returns CONTEXT_INJECTION_REASON to trigger re-prompting with appended injected context; adds helpers and tests.
Errors
src/error.rs
Added AgentError::DuplicateWorkerTask { channel_id, existing_worker_id } for duplicate spawn rejection.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: worker context injection and duplicate worker spawn prevention' directly and specifically summarizes the two main changes: adding context injection for workers and preventing duplicate spawns.
Description check ✅ Passed The description thoroughly explains the context injection mechanism, duplicate worker prevention, implementation details, and testing—all directly related to and supporting the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/worker-context-injection

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jamiepine jamiepine changed the base branch from refactor/browser-tool-rewrite to main March 8, 2026 02:22
.map_err(|e| RouteError(format!("Invalid worker ID: {e}")))?;

// Look up the input sender for this worker
// Try interactive input first (worker is WaitingForInput).
Copy link
Contributor

Choose a reason for hiding this comment

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

worker_inputs existing here only tells you the worker is interactive, not that it's actually WaitingForInput. Since worker_inputs is inserted at spawn-time and the worker only reads input_rx after it goes idle, a running interactive worker will always take this branch and never receive mid-flight injections (despite the prompt/docs saying it will).

Maybe gate the follow-up path on the status block (e.g. only treat as follow-up when the worker status is idle), otherwise prefer worker_injections so running interactive workers can incorporate context at the next turn boundary.

pub async fn run(mut self) -> Result<String> {
// Wire the injection receiver into the hook so `on_completion_call`
// can drain pending injected context before each LLM turn.
let inject_rx = std::mem::replace(
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit: std::mem::replace(&mut self.inject_rx, mpsc::channel(1).1) allocates a throwaway channel just to satisfy move semantics.

If you make inject_rx: Option<mpsc::Receiver<String>>, you can take() it here and avoid the placeholder channel (and make it clearer the receiver is single-use).

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/agent/channel_dispatch.rs`:
- Around line 332-344: The duplicate-task check in check_duplicate_task using
state.status_block.read() is racy; instead add an atomic reservation mechanism
(e.g. a reserved_tasks HashSet/Map guarded by the same async write lock or a new
RwLock/Mutex on ChannelState) and acquire the write lock to reserve the task
description before any await/spawn happens (use a method like reserve_task(&mut
self, task: &str) that returns an error if already reserved), then clear that
reservation on terminal cleanup paths (e.g. when handling WorkerComplete and
where worker_handles are registered/removed) so stale entries don’t cause false
positives; update all usages (including the analogous checks around the other
occurrences noted) to use the new reserve/release API rather than a read-only
find_duplicate_worker_task call.

In `@src/agent/channel.rs`:
- Around line 93-96: The field worker_injections (Arc<RwLock<HashMap<WorkerId,
tokio::sync::mpsc::Sender<String>>>) reintroduces channel-level context into
workers and must be removed: delete the worker_injections field and any code
that writes to or reads from it (search for worker_injections, WorkerId-based
injection sends, and the route tool paths that call into it) and instead
implement branching/respawn logic that creates a new task with the additional
context; ensure Worker structs and constructors (and any functions referenced by
the route tool) are updated to no longer expect or use worker_injections and
that new context is passed via task creation APIs rather than via in-flight
injection channels.

In `@src/agent/worker.rs`:
- Around line 266-272: The follow-up loop currently calls prompt_once and treats
any PromptCancelled as a generic error, so context-injection Terminate with
CONTEXT_INJECTION_REASON causes the worker to fail; update the follow-up error
handling (before the generic Err(error) arm) to detect PromptCancelled where
SpacebotHook::is_context_injection_reason(&reason) is true, call
follow_up_hook.take_injected_messages() to drain injected messages, append each
to history as Message::User, modify follow_up_prompt (e.g. append a short
"Context updated. Continue processing." hint) and let the loop continue to
re-prompt (mirroring how prompt_with_tool_nudge_retry handles
CONTEXT_INJECTION_REASON) so injected context is applied and retried instead of
propagating the error.

In `@src/tools/route.rs`:
- Around line 107-131: The current code path uses
self.state.worker_injections.read() and inject_tx.send(...) to append arbitrary
channel messages into a running worker's history (mentioned in inject_tx and
RouteOutput), which crosses the worker/branch boundary; instead of sending into
the running worker's injection channel, change the behavior to create a new
branch or spawn a new worker with the provided channel message as a fresh
prompt/task rewrite—i.e., remove the inject_tx.send(...) branch and call a
function like spawn_worker_with_context or create_branch_for_worker (implement
if missing on state) that takes worker_id and args.message, produces a new
worker/branch id and initial prompt, and return a RouteOutput indicating a new
branch/worker was created rather than mutating existing worker history.
- Around line 82-104: The routing code currently treats presence of an input
sender (self.state.worker_inputs) as indication the worker is idle, which
misroutes injections for interactive workers; modify the route logic in
src/tools/route.rs so you first check the worker's explicit state (e.g., compare
to WaitingForInput vs Running from the worker state machine) before using
inputs.get(&worker_id).cloned(); only send on input_tx when the worker state ==
WaitingForInput, otherwise leave inputs alone and let the injection path
(worker_injections) handle the update; if you don't have a worker-state map, add
or use the existing worker state lookup used by the worker loop and gate the
input branch with that explicit check to avoid treating registered senders as
the authoritative state.

In `@src/tools/spawn_worker.rs`:
- Around line 181-190: The duplicate-worker branch currently turns a detected
duplicate (found via status_block.read().await and
status.find_duplicate_worker_task(&args.task)) into a free-form
SpawnWorkerError(String); change it to return a structured tool result instead
of a prose error so callers/LLMs can recover deterministically. Replace the
Err(SpawnWorkerError(...)) path with a typed result variant or structured error
value (e.g., DuplicateWorker { existing_worker_id: existing_id,
recommended_action: "route" }) from the spawn worker API/enum you use, so the
duplicate condition is returned as data (referencing status_block,
find_duplicate_worker_task, existing_id, args.task and the intended "route"
action) rather than a string message.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0e4f1450-c7b0-45bd-918e-914012f5eb02

📥 Commits

Reviewing files that changed from the base of the PR and between e81b99b and f56f44c.

📒 Files selected for processing (11)
  • prompts/en/tools/route_description.md.j2
  • src/agent/channel.rs
  • src/agent/channel_dispatch.rs
  • src/agent/cortex.rs
  • src/agent/status.rs
  • src/agent/worker.rs
  • src/error.rs
  • src/hooks/spacebot.rs
  • src/tools/route.rs
  • src/tools/spawn_worker.rs
  • tests/context_dump.rs

Comment on lines +93 to +96
/// Injection senders for all workers, keyed by worker ID.
/// Used by the route tool to deliver addendum context to running workers
/// without requiring the worker to be interactive.
pub worker_injections: Arc<RwLock<HashMap<WorkerId, tokio::sync::mpsc::Sender<String>>>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This reintroduces channel context into workers.

worker_injections lets later channel messages mutate an in-flight worker's context. That breaks the repo invariant that workers run from a fresh prompt plus task, and it makes worker behavior depend on external conversation state instead of the task that spawned it. If extra context is needed, branch or respawn with an updated task instead.

Based on learnings "Don't give workers channel context. Workers get a fresh prompt and a task. If a worker needs conversation context, that's a branch, not a worker".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/agent/channel.rs` around lines 93 - 96, The field worker_injections
(Arc<RwLock<HashMap<WorkerId, tokio::sync::mpsc::Sender<String>>>) reintroduces
channel-level context into workers and must be removed: delete the
worker_injections field and any code that writes to or reads from it (search for
worker_injections, WorkerId-based injection sends, and the route tool paths that
call into it) and instead implement branching/respawn logic that creates a new
task with the additional context; ensure Worker structs and constructors (and
any functions referenced by the route tool) are updated to no longer expect or
use worker_injections and that new context is passed via task creation APIs
rather than via in-flight injection channels.

Comment on lines +107 to +131
// Fall back to context injection (worker is Running).
let injections = self.state.worker_injections.read().await;
if let Some(inject_tx) = injections.get(&worker_id).cloned() {
drop(injections);

inject_tx.send(args.message).await.map_err(|_| {
RouteError(format!(
"Worker {worker_id} has stopped running (injection channel closed)"
))
})?;

tracing::info!(
worker_id = %worker_id,
channel_id = %self.state.channel_id,
"context injected into running worker"
);

return Ok(RouteOutput {
routed: true,
worker_id,
message: format!(
"Context injected into running worker {worker_id}. \
The worker will incorporate this at its next turn boundary."
),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This crosses the worker/branch boundary.

This path appends arbitrary channel messages into a running worker's history. That gives workers channel context mid-task, which is exactly the case the repo guidance says should become a branch or a newly spawned worker with a rewritten task instead of mutating worker history in place. Based on learnings: Don't give workers channel context; workers must receive fresh prompt and task description, not conversation history.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/route.rs` around lines 107 - 131, The current code path uses
self.state.worker_injections.read() and inject_tx.send(...) to append arbitrary
channel messages into a running worker's history (mentioned in inject_tx and
RouteOutput), which crosses the worker/branch boundary; instead of sending into
the running worker's injection channel, change the behavior to create a new
branch or spawn a new worker with the provided channel message as a fresh
prompt/task rewrite—i.e., remove the inject_tx.send(...) branch and call a
function like spawn_worker_with_context or create_branch_for_worker (implement
if missing on state) that takes worker_id and args.message, produces a new
worker/branch id and initial prompt, and return a RouteOutput indicating a new
branch/worker was created rather than mutating existing worker history.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (2)
src/tools/spawn_worker.rs (1)

174-183: ⚠️ Potential issue | 🟠 Major

Return duplicate-worker conflicts as structured output.

This turns a deterministic, recoverable state into a prose SpawnWorkerError, so the model has to parse text instead of reading fields like the existing worker id and routing accordingly. Please surface this as structured SpawnWorkerOutput data (for example, duplicate status + existing_worker_id) and return Ok(...) here.

As per coding guidelines "Tool errors are returned as structured results, not panics. The LLM sees the error and can recover (error-as-result for tools pattern)".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/spawn_worker.rs` around lines 174 - 183, The code currently returns
an Err(SpawnWorkerError(...)) when find_duplicate_worker_task(&args.task) finds
an existing worker; change this to return a structured SpawnWorkerOutput
indicating a duplicate rather than an error. Inside the duplicate branch (where
you call status.find_duplicate_worker_task(&args.task)), construct and return
Ok(SpawnWorkerOutput::Duplicate { existing_worker_id: existing_id.clone(), /*
optional: reason or hint field if present */ }) or the equivalent variant/fields
your SpawnWorkerOutput defines, instead of returning Err(...); keep the read
lock usage (self.state.status_block.read().await) and preserve the existing_id
value so the caller/LLM can read the structured duplicate status. Ensure any
codepaths that expected Err(SpawnWorkerError) are updated to handle the
Ok(Duplicate) result accordingly.
src/agent/worker.rs (1)

286-293: ⚠️ Potential issue | 🟠 Major

Shared hook injection now breaks interactive follow-up retries.

Because self.hook is later cloned for the follow-up path, wiring inject_rx into the shared hook here also enables spacebot_context_injection cancellation during prompt_once(). The follow-up loop still treats that cancellation as a hard failure, so injecting context while an interactive worker is processing a follow-up now fails the worker instead of appending the messages and retrying. Either keep injection out of the follow-up hook, or mirror the main loop’s context-injection retry handling there.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/agent/worker.rs` around lines 286 - 293, The shared wiring of inject_rx
into self.hook via with_inject_rx causes injected-context cancellations to
propagate into the follow-up path and break prompt_once retries; fix by not
mutating the shared self.hook used for follow-ups. Concretely, stop replacing
self.hook with the with_inject_rx version (the std::mem::replace that sets
inject_rx into self.hook) and instead create a separate hook instance (e.g.,
clone self.hook and call with_inject_rx on that local/loop-specific clone) for
the main loop’s on_completion_call handling; alternatively, if you prefer to
keep injection on the shared hook, replicate the main loop’s context-injection
retry handling in the follow-up loop (the code that calls prompt_once) so
spacebot_context_injection cancellation is treated as a retry rather than a hard
failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/agent/worker.rs`:
- Around line 286-293: The shared wiring of inject_rx into self.hook via
with_inject_rx causes injected-context cancellations to propagate into the
follow-up path and break prompt_once retries; fix by not mutating the shared
self.hook used for follow-ups. Concretely, stop replacing self.hook with the
with_inject_rx version (the std::mem::replace that sets inject_rx into
self.hook) and instead create a separate hook instance (e.g., clone self.hook
and call with_inject_rx on that local/loop-specific clone) for the main loop’s
on_completion_call handling; alternatively, if you prefer to keep injection on
the shared hook, replicate the main loop’s context-injection retry handling in
the follow-up loop (the code that calls prompt_once) so
spacebot_context_injection cancellation is treated as a retry rather than a hard
failure.

In `@src/tools/spawn_worker.rs`:
- Around line 174-183: The code currently returns an Err(SpawnWorkerError(...))
when find_duplicate_worker_task(&args.task) finds an existing worker; change
this to return a structured SpawnWorkerOutput indicating a duplicate rather than
an error. Inside the duplicate branch (where you call
status.find_duplicate_worker_task(&args.task)), construct and return
Ok(SpawnWorkerOutput::Duplicate { existing_worker_id: existing_id.clone(), /*
optional: reason or hint field if present */ }) or the equivalent variant/fields
your SpawnWorkerOutput defines, instead of returning Err(...); keep the read
lock usage (self.state.status_block.read().await) and preserve the existing_id
value so the caller/LLM can read the structured duplicate status. Ensure any
codepaths that expected Err(SpawnWorkerError) are updated to handle the
Ok(Duplicate) result accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6e3ff9e-4345-48c0-b7ba-e2e1c6d99a6b

📥 Commits

Reviewing files that changed from the base of the PR and between f56f44c and 68f780a.

📒 Files selected for processing (2)
  • src/agent/worker.rs
  • src/tools/spawn_worker.rs

…ructured duplicate output, follow-up injection handling

- Route tool checks StatusBlock worker status instead of worker_inputs map presence
- inject_rx uses Option<Receiver> with .take() instead of dummy channel allocation
- Atomic task reservation via reserved_tasks to prevent TOCTOU race in duplicate detection
- Follow-up loop now handles CONTEXT_INJECTION_REASON, matching main task loop behavior
- Duplicate worker spawn returns structured SpawnWorkerOutput instead of error string
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/agent/channel_dispatch.rs (1)

451-490: ⚠️ Potential issue | 🟠 Major

Worker-limit enforcement still misses builtin workers.

check_worker_limit() counts state.active_workers, but this path only stores handles/input/injection senders and then moves the worker into the task. Since nothing here registers the builtin worker in active_workers, the concurrent-worker cap can still be exceeded. Count worker_handles/status_block instead, or store lightweight active-worker metadata separately from Worker.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/agent/channel_dispatch.rs` around lines 451 - 490, The builtin worker
paths (Worker::new_interactive and Worker::new) create workers and store
input/injection senders (state.worker_inputs, state.worker_injections) but never
register lightweight active-worker metadata, so check_worker_limit() that
inspects state.active_workers can be bypassed; fix by registering a minimal
active-worker entry (e.g., increment or insert into the same structure
check_worker_limit() reads) when creating any worker, or change
check_worker_limit() to count the existing worker_handles/status_block or
entries in state.worker_injections/worker_inputs instead; update the code paths
that call Worker::new_interactive and Worker::new to also add/remove the same
active-worker marker used by check_worker_limit() so builtin workers are
included in the concurrent-worker cap.
♻️ Duplicate comments (1)
src/agent/worker.rs (1)

559-579: ⚠️ Potential issue | 🟠 Major

This turns workers into branches.

These lines append later channel messages into the worker’s history mid-task, so the worker no longer runs from just its fresh prompt + task. If extra conversation context is needed, this repo’s model is to fork a branch or respawn with a rewritten task instead of mutating worker history in place.

Based on learnings: "Don't give workers channel context. Workers get a fresh prompt and a task. If a worker needs conversation context, that's a branch, not a worker"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/agent/worker.rs` around lines 559 - 579, The code is mutating the
worker's running history by appending injected channel messages (using
follow_up_hook.take_injected_messages() and pushing into history), which turns a
worker into a branch; instead, do NOT modify history in-place — drain injected
messages but hand them back to the supervisor/manager to spawn a new
branch/respawn a worker with a rewritten task. Concretely: remove the loop that
pushes into history and the in-place context injection; after detecting
SpacebotHook::is_context_injection_reason(reason) and calling
follow_up_hook.take_injected_messages(), call or signal the supervisor to create
a branch/respawn (e.g., return a variant or set a flag that the caller
understands) and set follow_up_prompt only to indicate that a branch should be
created; do not append to history or continue the current worker loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/tools/route.rs`:
- Around line 128-163: The Some(false) branch currently only looks up
self.state.worker_injections and treats missing senders as "not found", which
causes running OpenCode workers (which register in self.state.worker_inputs, not
worker_injections) to be misreported; update this branch to check
self.state.worker_inputs when no injection sender exists and either (a)
create/register an injection sender for that worker_id so inject_tx.send(...)
can be used for OpenCode workers, or (b) return a structured unsupported-state
RouteOutput (instead of RouteError) indicating injections are not supported for
OpenCode workers; reference the symbols worker_injections, worker_inputs,
inject_tx, RouteOutput and RouteError when making the change so the behavior for
running OpenCode workers is handled explicitly rather than falling through to
the final "not found" error.

---

Outside diff comments:
In `@src/agent/channel_dispatch.rs`:
- Around line 451-490: The builtin worker paths (Worker::new_interactive and
Worker::new) create workers and store input/injection senders
(state.worker_inputs, state.worker_injections) but never register lightweight
active-worker metadata, so check_worker_limit() that inspects
state.active_workers can be bypassed; fix by registering a minimal active-worker
entry (e.g., increment or insert into the same structure check_worker_limit()
reads) when creating any worker, or change check_worker_limit() to count the
existing worker_handles/status_block or entries in
state.worker_injections/worker_inputs instead; update the code paths that call
Worker::new_interactive and Worker::new to also add/remove the same
active-worker marker used by check_worker_limit() so builtin workers are
included in the concurrent-worker cap.

---

Duplicate comments:
In `@src/agent/worker.rs`:
- Around line 559-579: The code is mutating the worker's running history by
appending injected channel messages (using
follow_up_hook.take_injected_messages() and pushing into history), which turns a
worker into a branch; instead, do NOT modify history in-place — drain injected
messages but hand them back to the supervisor/manager to spawn a new
branch/respawn a worker with a rewritten task. Concretely: remove the loop that
pushes into history and the in-place context injection; after detecting
SpacebotHook::is_context_injection_reason(reason) and calling
follow_up_hook.take_injected_messages(), call or signal the supervisor to create
a branch/respawn (e.g., return a variant or set a flag that the caller
understands) and set follow_up_prompt only to indicate that a branch should be
created; do not append to history or continue the current worker loop.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 39d07c0b-20b4-484d-b900-49cbd9ffa06a

📥 Commits

Reviewing files that changed from the base of the PR and between 68f780a and 3519a01.

📒 Files selected for processing (6)
  • src/agent/channel.rs
  • src/agent/channel_dispatch.rs
  • src/agent/worker.rs
  • src/tools/route.rs
  • src/tools/spawn_worker.rs
  • tests/context_dump.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/tools/spawn_worker.rs

Comment on lines +128 to +163
// Worker is running — use context injection.
Some(false) => {
let injections = self.state.worker_injections.read().await;
if let Some(inject_tx) = injections.get(&worker_id).cloned() {
drop(injections);

inject_tx.send(args.message).await.map_err(|_| {
RouteError(format!(
"Worker {worker_id} has stopped running (injection channel closed)"
))
})?;

tracing::info!(
worker_id = %worker_id,
channel_id = %self.state.channel_id,
"context injected into running worker"
);

return Ok(RouteOutput {
routed: true,
worker_id,
message: format!(
"Context injected into running worker {worker_id}. \
The worker will incorporate this at its next turn boundary."
),
});
}
drop(injections);
}
// Worker not found in status block.
None => {}
}

Err(RouteError(format!(
"Worker {worker_id} not found. It may have already completed or been cancelled."
)))
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Running OpenCode workers are treated as missing.

This branch only succeeds when worker_injections has a sender, but the OpenCode spawn/resume paths only register worker_inputs. A running OpenCode worker will therefore fall through to the final “not found” error even though it is active. Either wire an injection sender for that worker type too, or return a structured unsupported-state result instead of reporting the worker as gone.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/route.rs` around lines 128 - 163, The Some(false) branch currently
only looks up self.state.worker_injections and treats missing senders as "not
found", which causes running OpenCode workers (which register in
self.state.worker_inputs, not worker_injections) to be misreported; update this
branch to check self.state.worker_inputs when no injection sender exists and
either (a) create/register an injection sender for that worker_id so
inject_tx.send(...) can be used for OpenCode workers, or (b) return a structured
unsupported-state RouteOutput (instead of RouteError) indicating injections are
not supported for OpenCode workers; reference the symbols worker_injections,
worker_inputs, inject_tx, RouteOutput and RouteError when making the change so
the behavior for running OpenCode workers is handled explicitly rather than
falling through to the final "not found" error.

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