feat: add task-scoped CorPilot for task steering#408
feat: add task-scoped CorPilot for task steering#408deanfluencebot wants to merge 4 commits intospacedriveapp:mainfrom
Conversation
WalkthroughAdds a task-scoped CorPilot feature: frontend CorPilot tab with configurable Cortex chat panel, client and hook changes to pass optional taskNumber, backend prompt and cortex chat plumbing to load/insert task context, and TaskUpdateTool validations to restrict in-progress task core edits. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/tools/spawn_worker.rs (1)
291-297:⚠️ Potential issue | 🟠 MajorAdd
task_numberfield toTrackedWorkerand capture it at spawn time.When a detached worker completes and auto-triggers a follow-up turn, the retrigger call at
src/agent/cortex_chat.rs:531passestask_number: Noneinstead of the task context captured at spawn time. SinceTrackedWorker(defined atsrc/agent/cortex_chat.rs:412) currently stores onlythread_idandchannel_context, the task number is lost. If the user switches tasks between spawn and completion, the follow-up turn will rebuild with incorrect (or missing) task context.Add a
task_number: Option<i64>field toTrackedWorker, capturecurrent_task_numberwhen registering the worker atsrc/tools/spawn_worker.rs:499, and pass the stored value when retriggering atsrc/agent/cortex_chat.rs:531.🤖 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 291 - 297, The TrackedWorker struct is missing a task_number field so the task context is lost between spawn and completion; add a new field pub task_number: Option<i64> to TrackedWorker, capture the current_task_number (Arc<RwLock<Option<i64>>>) value when registering the worker in the spawn path (the code around register/register_tracked_worker in spawn_worker.rs where thread_id and channel_context are captured), store that value into the new task_number field, and then update the retrigger call in cortex_chat.rs (the retrigger at the site around the current retrigger invocation) to pass the stored tracked_worker.task_number instead of None so the follow-up turn uses the original task context.interface/src/hooks/useCortexChat.ts (1)
60-94:⚠️ Potential issue | 🟠 MajorReload fixed-thread state when the active thread changes, and don't make it writable before history arrives.
threadIdis seeded fromoptions.fixedThreadIdat initialization (line 60), enabling sends viasendMessage()before the history fetch completes. This creates a race where optimistic messages sent during the fetch window can be overwritten by the latersetMessages(data.messages)call. Additionally, theloadedRefguard prevents the effect from re-running even whenagentIdorfixedThreadIdchange—onceloadedRef.currentis set to true, the early return at line 68 blocks reloads. This leaves the hook locked to the original thread's messages whileoptions.taskNumbercan still change, mixing task contexts into the wrong conversation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/hooks/useCortexChat.ts` around lines 60 - 94, The effect currently seeds threadId from options.fixedThreadId and uses loadedRef to run only once, causing sendMessage race conditions and preventing reloads when agentId/fixedThreadId change; change the logic so threadId is not made writable until history loads (initialize threadId to null and only call setThreadId after successful api.cortexChatMessages or generateThreadId), remove or reset the loadedRef guard so the useEffect (the effect that calls api.cortexChatMessages) re-runs when agentId or options.fixedThreadId/options.freshThread change, and in the fixed-thread branch fetch history first (api.cortexChatMessages(agentId, options.fixedThreadId).then(...).catch(...)) before calling setThreadId, ensuring setMessages is applied after the fetch and preventing optimistic sends from being overwritten.
🧹 Nitpick comments (4)
src/tools/task_update.rs (1)
331-381: Assert that the blocked path leaves the task unchanged.This test currently proves only that
call()returns an error. The contract you care about here is also “no in-place mutation”, so it should reload the task afterexpect_err()and verify the title, description, and subtasks are still intact.🧪 Missing assertion
assert!( error .to_string() .contains("cannot rewrite the core spec of an in-progress task"), "unexpected error: {error}" ); + + let task = store + .get_by_number(agent_id.as_ref(), created.task_number) + .await + .expect("task fetch should succeed") + .expect("task should exist"); + assert_eq!(task.title, "in-progress task"); + assert_eq!(task.description.as_deref(), Some("original")); + assert!(task.subtasks.is_empty()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/task_update.rs` around lines 331 - 381, The test corpilot_blocks_core_rewrite_for_in_progress_task must also assert that the task was not mutated when TaskUpdateTool::call(...) returns an error; after the expect_err() reload the task from the store (using the same store and created.task_number) and assert the title, description, and subtasks remain equal to the original values ("in-progress task", "original", and an empty subtasks list) so the blocked path leaves the task unchanged.src/agent/cortex_chat.rs (1)
892-965: Renameoutto a descriptive local.
outmakes this formatter harder to scan;task_contextorrendered_task_contextwould read much more clearly here. As per coding guidelines, "Don't abbreviate variable names. Usequeuenotq,messagenotmsg. Common abbreviations likeconfigare fine".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/cortex_chat.rs` around lines 892 - 965, The local variable named out is ambiguous—rename it (e.g., task_context or rendered_task_context) by changing the binding let mut out = ... to let mut task_context = ... and update every usage (all out.push_str / out.push / &format! calls and the final Some(out)) to use the new name; ensure the match block that appends worker details (using ProcessRunLogger and get_worker_detail) also writes into task_context so behavior is unchanged.interface/src/routes/AgentTasks.tsx (2)
720-739: Don’t offer blocked rewrite prompts onin_progresstasks.
corpilotStarterPrompts()always exposes “Refine” and “Split Subtasks”, but CorPilot now rejects title/description/subtask rewrites forin_progresstasks insrc/tools/task_update.rs, Lines 175-182. On an active task, those empty-state actions lead straight to an avoidable tool error.✍️ Suggested change
function corpilotStarterPrompts(task: TaskItem) { + if (task.status === "in_progress") { + return [ + { + label: "Inspect", + prompt: `Inspect task #${task.task_number}, check worker state or blockers, and tell me the next best steering action.`, + }, + { + label: "Add Context", + prompt: `Help me add missing context to task #${task.task_number}. Ask for only the highest-value missing details, then update the task without rewriting the in-progress spec in place.`, + }, + { + label: "Unblock", + prompt: `Inspect task #${task.task_number}, tell me whether it should move out of in_progress before any spec rewrite, and recommend the next action.`, + }, + ]; + } + return [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/AgentTasks.tsx` around lines 720 - 739, corpilotStarterPrompts currently always returns "Refine" and "Split Subtasks" options which trigger CorPilot tool errors for active tasks; update corpilotStarterPrompts(TaskItem) to check the task state and omit the rewrite prompts when the task is in progress (e.g., task.state === "in_progress" or equivalent enum value), returning only safe actions like "Add Context" and "Inspect" for active tasks so the UI never offers actions that will be rejected by src/tools/task_update.rs.
521-547: Give the Overview/CorPilot switch real tab semantics.Right now assistive tech just sees two unrelated buttons. Adding
role="tablist"/role="tab",aria-selected,aria-controls, and a labelledtabpanelwill make the active pane discoverable and the dialog easier to navigate from the keyboard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/AgentTasks.tsx` around lines 521 - 547, Wrap the two tab buttons container with role="tablist" and convert each button into an accessible tab by adding role="tab", aria-selected={tab === "..."} and aria-controls pointing to the corresponding panel id (e.g., "overview-tab" -> controls "overview-panel", "corpilot-tab" -> "corpilot-panel"); ensure each button still calls setTab("overview"/"corpilot") onClick. Add matching role="tabpanel" elements for the content panes with id="overview-panel"/id="corpilot-panel" and aria-labelledby pointing back to the tab ids; keep the existing tab state variable tab and handler setTab to toggle active state. Ensure the CorPilotMark usage remains inside the CorPilot tab and update attributes on the existing divs/buttons (no logic changes needed other than attributes and panel ids).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@prompts/en/tools/task_update_description.md.j2`:
- Line 1: The shared prompt text for task_update currently contains a
CorPilot-specific rule about not rewriting in_progress tasks, which incorrectly
applies to branch, worker, and cortex callers; update the prompt so the
CorPilot-only guideline is removed from the shared template
(prompts/en/tools/task_update_description.md.j2) and instead append that
sentence into the CorPilot-specific tool description or cortex-specific tool
definition where the guardrail belongs (reference symbols: task_update,
CorPilot, branch, worker, cortex) so non-CorPilot callers aren’t mistakenly
constrained.
In `@src/agent/cortex_chat.rs`:
- Around line 527-532: The call to send_message_blocking currently passes None
for the task id which clears current_task_number and strips task context used by
build_system_prompt and the in-progress rewrite guard; change the call in
cortex_chat.rs to pass the active task id (e.g., Some(current_task_number) or
the tracked.task_number/ task_number field) so the background turn keeps
CorPilot task scope, and also ensure when inserting into tracked_workers in
spawn_worker.rs you populate the task_number field for that tracked entry so the
worker-completion synthesis retains the same task context and the guard in
task_update.rs continues to work.
---
Outside diff comments:
In `@interface/src/hooks/useCortexChat.ts`:
- Around line 60-94: The effect currently seeds threadId from
options.fixedThreadId and uses loadedRef to run only once, causing sendMessage
race conditions and preventing reloads when agentId/fixedThreadId change; change
the logic so threadId is not made writable until history loads (initialize
threadId to null and only call setThreadId after successful
api.cortexChatMessages or generateThreadId), remove or reset the loadedRef guard
so the useEffect (the effect that calls api.cortexChatMessages) re-runs when
agentId or options.fixedThreadId/options.freshThread change, and in the
fixed-thread branch fetch history first (api.cortexChatMessages(agentId,
options.fixedThreadId).then(...).catch(...)) before calling setThreadId,
ensuring setMessages is applied after the fetch and preventing optimistic sends
from being overwritten.
In `@src/tools/spawn_worker.rs`:
- Around line 291-297: The TrackedWorker struct is missing a task_number field
so the task context is lost between spawn and completion; add a new field pub
task_number: Option<i64> to TrackedWorker, capture the current_task_number
(Arc<RwLock<Option<i64>>>) value when registering the worker in the spawn path
(the code around register/register_tracked_worker in spawn_worker.rs where
thread_id and channel_context are captured), store that value into the new
task_number field, and then update the retrigger call in cortex_chat.rs (the
retrigger at the site around the current retrigger invocation) to pass the
stored tracked_worker.task_number instead of None so the follow-up turn uses the
original task context.
---
Nitpick comments:
In `@interface/src/routes/AgentTasks.tsx`:
- Around line 720-739: corpilotStarterPrompts currently always returns "Refine"
and "Split Subtasks" options which trigger CorPilot tool errors for active
tasks; update corpilotStarterPrompts(TaskItem) to check the task state and omit
the rewrite prompts when the task is in progress (e.g., task.state ===
"in_progress" or equivalent enum value), returning only safe actions like "Add
Context" and "Inspect" for active tasks so the UI never offers actions that will
be rejected by src/tools/task_update.rs.
- Around line 521-547: Wrap the two tab buttons container with role="tablist"
and convert each button into an accessible tab by adding role="tab",
aria-selected={tab === "..."} and aria-controls pointing to the corresponding
panel id (e.g., "overview-tab" -> controls "overview-panel", "corpilot-tab" ->
"corpilot-panel"); ensure each button still calls setTab("overview"/"corpilot")
onClick. Add matching role="tabpanel" elements for the content panes with
id="overview-panel"/id="corpilot-panel" and aria-labelledby pointing back to the
tab ids; keep the existing tab state variable tab and handler setTab to toggle
active state. Ensure the CorPilotMark usage remains inside the CorPilot tab and
update attributes on the existing divs/buttons (no logic changes needed other
than attributes and panel ids).
In `@src/agent/cortex_chat.rs`:
- Around line 892-965: The local variable named out is ambiguous—rename it
(e.g., task_context or rendered_task_context) by changing the binding let mut
out = ... to let mut task_context = ... and update every usage (all out.push_str
/ out.push / &format! calls and the final Some(out)) to use the new name; ensure
the match block that appends worker details (using ProcessRunLogger and
get_worker_detail) also writes into task_context so behavior is unchanged.
In `@src/tools/task_update.rs`:
- Around line 331-381: The test
corpilot_blocks_core_rewrite_for_in_progress_task must also assert that the task
was not mutated when TaskUpdateTool::call(...) returns an error; after the
expect_err() reload the task from the store (using the same store and
created.task_number) and assert the title, description, and subtasks remain
equal to the original values ("in-progress task", "original", and an empty
subtasks list) so the blocked path leaves the task unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1230c22b-402b-4877-80df-711f69f9fea4
📒 Files selected for processing (13)
docs/design-docs/corpilot.mdinterface/src/api/client.tsinterface/src/components/CortexChatPanel.tsxinterface/src/hooks/useCortexChat.tsinterface/src/routes/AgentTasks.tsxprompts/en/cortex_chat.md.j2prompts/en/tools/task_update_description.md.j2src/agent/cortex_chat.rssrc/api/cortex.rssrc/prompts/engine.rssrc/tools.rssrc/tools/spawn_worker.rssrc/tools/task_update.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/agent/cortex_chat.rs (1)
430-440: Consider usingfloor_char_boundarydirectly.This helper manually implements what
str::floor_char_boundarydoes (stabilized in Rust 1.80). Line 964 already usesfloor_char_boundarydirectly, so consolidating on the standard library method would improve consistency.♻️ Suggested simplification
fn truncate_task_context_text(value: &str, max_bytes: usize) -> &str { if value.len() <= max_bytes { return value; } - - let mut end = max_bytes; - while end > 0 && !value.is_char_boundary(end) { - end -= 1; - } - &value[..end] + &value[..value.floor_char_boundary(max_bytes)] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/cortex_chat.rs` around lines 430 - 440, The function truncate_task_context_text duplicates manual UTF-8 boundary logic; replace its loop with the standard str::floor_char_boundary call to match other uses (e.g., line where floor_char_boundary is already used). Specifically, in truncate_task_context_text, compute the end index with value.floor_char_boundary(max_bytes) and return &value[..end] (and keep the initial length check), removing the manual while loop so the function delegates to the std method for correctness and consistency.
🤖 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/task_update.rs`:
- Around line 165-191: Collapse the nested `if let` by using `let ... else`
guards to simplify control flow: first do `let Some(cortex_ctx) =
&self.cortex_ctx else { /* early return */ };` then `let
Some(current_task_number) = *cortex_ctx.current_task_number.read().await else {
/* early return */ };` and keep the existing logic that compares
`current_task_number` to `task_number`, calls
`self.task_store.get_by_number(&self.agent_id, task_number).await`, and checks
`args.title/description/subtasks` and `task.status == TaskStatus::InProgress`
before returning `TaskUpdateError`; this removes the nested `if let` blocks
while preserving the checks around `cortex_ctx`, `current_task_number`,
`task_store.get_by_number`, and the `is_rewriting_core_spec` logic.
---
Nitpick comments:
In `@src/agent/cortex_chat.rs`:
- Around line 430-440: The function truncate_task_context_text duplicates manual
UTF-8 boundary logic; replace its loop with the standard
str::floor_char_boundary call to match other uses (e.g., line where
floor_char_boundary is already used). Specifically, in
truncate_task_context_text, compute the end index with
value.floor_char_boundary(max_bytes) and return &value[..end] (and keep the
initial length check), removing the manual while loop so the function delegates
to the std method for correctness and consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26490664-c6c0-455f-a256-2bc9582725ba
📒 Files selected for processing (3)
prompts/en/cortex_chat.md.j2src/agent/cortex_chat.rssrc/tools/task_update.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- prompts/en/cortex_chat.md.j2
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/agent/cortex_chat.rs (1)
964-966: Prefer the helper method for consistency.The worker result truncation uses
result.floor_char_boundary(800)directly, while description truncation usesSelf::truncate_task_context_text. Consider using the helper for consistency:♻️ Suggested refactor
if let Some(result) = worker.result.as_deref() && !result.trim().is_empty() { - let preview = if result.len() > 800 { - let boundary = result.floor_char_boundary(800); - format!("{}...", &result[..boundary]) + let preview = if result.len() > 800 { + let truncated = Self::truncate_task_context_text(result, 800); + format!("{}...", truncated) } else { result.to_string() };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/cortex_chat.rs` around lines 964 - 966, The worker result truncation builds preview by calling result.floor_char_boundary(800) directly; replace that logic to use the existing helper Self::truncate_task_context_text(result, 800) for consistency and reuse. Update the code that sets preview (in the block creating preview from result) to call Self::truncate_task_context_text and append the "..." suffix if the helper does not already include it, ensuring the same truncation behavior as description handling.src/tools/task_update.rs (1)
155-160: Consider moving CorPilot guidance to the prompt file.The inline string appended here is guidance text that could live in
prompts/per coding guidelines. However, since it's a short conditional suffix rather than a full system prompt, this is a minor deviation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/task_update.rs` around lines 155 - 160, The conditional CorPilot guidance is hardcoded into the description built from prompts::text::get("tools/task_update")—move that suffix into the prompts system so the prompt file owns all user-facing text; update the builder in the task_update flow to fetch a single prompt (e.g., add a new key like "tools/task_update_corpilot_suffix" or combine into "tools/task_update" and remove the inline string), then when building description use crate::prompts::text::get(...) (and conditionally append the prompt text via self.cortex_ctx.is_some() without any hardcoded literals). Ensure you update any prompt keys used by get(...) and remove the inline string literal near the variable description to keep all guidance in prompts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/agent/cortex_chat.rs`:
- Around line 964-966: The worker result truncation builds preview by calling
result.floor_char_boundary(800) directly; replace that logic to use the existing
helper Self::truncate_task_context_text(result, 800) for consistency and reuse.
Update the code that sets preview (in the block creating preview from result) to
call Self::truncate_task_context_text and append the "..." suffix if the helper
does not already include it, ensuring the same truncation behavior as
description handling.
In `@src/tools/task_update.rs`:
- Around line 155-160: The conditional CorPilot guidance is hardcoded into the
description built from prompts::text::get("tools/task_update")—move that suffix
into the prompts system so the prompt file owns all user-facing text; update the
builder in the task_update flow to fetch a single prompt (e.g., add a new key
like "tools/task_update_corpilot_suffix" or combine into "tools/task_update" and
remove the inline string), then when building description use
crate::prompts::text::get(...) (and conditionally append the prompt text via
self.cortex_ctx.is_some() without any hardcoded literals). Ensure you update any
prompt keys used by get(...) and remove the inline string literal near the
variable description to keep all guidance in prompts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b84dd1a-f5a8-4aa4-b2ff-9d9aae11cce8
📒 Files selected for processing (4)
prompts/en/tools/task_update_description.md.j2src/agent/cortex_chat.rssrc/tools/spawn_worker.rssrc/tools/task_update.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- prompts/en/tools/task_update_description.md.j2
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/tools/send_message_to_another_channel.rs (1)
193-212: Extract the direct-target send flow into a helper.This branch now repeats the same
broadcast/ tracing /SendMessageOutputconstruction used in the explicit Signal and explicit email paths. Pulling that into one helper would make future routing changes much less likely to drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/send_message_to_another_channel.rs` around lines 193 - 212, The implicit Signal branch duplicates the broadcast + tracing + SendMessageOutput construction used elsewhere; extract that logic into a helper (e.g., a private function like forward_via_broadcast or send_via_messaging_manager) that accepts (&self.messaging_manager, adapter: AdapterType, target: String, message: String) and performs self.messaging_manager.broadcast(...).await, the tracing::info call, and returns Result<SendMessageOutput, SendMessageError>; replace the repeated code in the implicit Signal branch (the self.messaging_manager.broadcast call, tracing::info, and Ok(SendMessageOutput { ... })) with a call to this new helper to keep routing logic DRY and consistent with the explicit Signal/email paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/tools/send_message_to_another_channel.rs`:
- Around line 193-212: The implicit Signal branch duplicates the broadcast +
tracing + SendMessageOutput construction used elsewhere; extract that logic into
a helper (e.g., a private function like forward_via_broadcast or
send_via_messaging_manager) that accepts (&self.messaging_manager, adapter:
AdapterType, target: String, message: String) and performs
self.messaging_manager.broadcast(...).await, the tracing::info call, and returns
Result<SendMessageOutput, SendMessageError>; replace the repeated code in the
implicit Signal branch (the self.messaging_manager.broadcast call,
tracing::info, and Ok(SendMessageOutput { ... })) with a call to this new helper
to keep routing logic DRY and consistent with the explicit Signal/email paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 745e5f9f-905f-4d12-9f44-2b42f5b4a0b4
📒 Files selected for processing (1)
src/tools/send_message_to_another_channel.rs
|
This is epic, but I'm going to Vito the name CorPilot lmao, lets just keep it as cortex so its consistent haha |
Summary
What changed
task_numberthrough cortex chat APIs and prompt renderingtask_updateguidanceValidation
Note
Task-scoped CorPilot interface for controlled task steering. Adds embedded chat panel to task detail that injects task context into Cortex prompts, with guardrails preventing core task mutations (title, description, subtasks) on in-progress work. UI: 787 additions across 13 files including new design doc, API endpoint parameters, and component props. Backend constraints enforce task-update policy—backlog tasks accept full rewrites, in-progress tasks only allow status updates and steering. Tests validate both allowed and blocked mutations.
Written by Tembo for commit 6358e27. This will update automatically on new commits.