SFR-271: add manager-style v1 task surface to ClawConnect MCP#23
Merged
jaruesink merged 5 commits intoMay 19, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds TaskStatus and TaskSummary types; extends RunTaskResult. Implements listTasks and getSession in core tools and re-exports them. MCP server and chat app add tools/formatters for listing and inspecting tasks and sessions with agent-scoped handlers. ChangesTask Listing and Status Tracking
Sequence DiagramssequenceDiagram
participant Client
participant listTasks_fn
participant GatewayPool
participant SessionStore
Client->>listTasks_fn: listTasks(pool)
listTasks_fn->>GatewayPool: iterate entries & sessions
GatewayPool->>SessionStore: fetch latest job for session
SessionStore-->>GatewayPool: job object
listTasks_fn->>listTasks_fn: map job -> TaskSummary (mapTaskStatus)
listTasks_fn-->>Client: TaskSummary[]
sequenceDiagram
participant MCP_Client
participant list_tasks_tool
participant listTasks_fn
participant MCP_Server
participant checkTask_fn
MCP_Client->>list_tasks_tool: invoke list_tasks(view)
list_tasks_tool->>listTasks_fn: listTasks(pool)
listTasks_fn-->>list_tasks_tool: TaskSummary[]
list_tasks_tool->>list_tasks_tool: filter by view and format via fmtListTasks
list_tasks_tool-->>MCP_Client: McpToolResponse
MCP_Client->>MCP_Server: invoke get_task(taskId, include)
MCP_Server->>checkTask_fn: checkTask(pool, jobId)
checkTask_fn-->>MCP_Server: snapshot or null
MCP_Server->>MCP_Client: formatted McpToolResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/tools.ts`:
- Around line 29-33: The mapping in mapTaskStatus incorrectly collapses
intervention-required states into "failed"; update mapTaskStatus to explicitly
return "needs-human" when input is "needs-human" and "blocked" when input is
"blocked" (in addition to keeping "running" -> "running",
"completed"/"completed_no_summary" -> "done", and default -> "failed") so
manager workflows can distinguish human-intervention states; apply the same
explicit mappings to the other occurrence of mapTaskStatus in the file and
ensure TaskSummary["status"] types include "needs-human" and "blocked" if
necessary.
- Line 49: The code sets summary: job.summary ?? session.lastSummary which can
surface stale summaries from prior jobs; change this to avoid falling back to
session.lastSummary for per-task listings — instead set summary to job.summary
(or null/undefined) and only use session.lastSummary in contexts that are
explicitly session-level; update the assignment in the block that defines
summary (referencing job.summary and session.lastSummary) so per-task entries do
not inherit session.lastSummary.
In `@packages/mcp/src/server.ts`:
- Line 260: The summary field currently treats include: [] as "not provided" and
returns snapshot.summary; update the logic in the get_task response construction
(the expression that sets summary) to distinguish undefined include from an
explicit empty array — i.e. only return snapshot.summary when include is
strictly undefined, otherwise return snapshot.summary only if
include.includes("summary") is true; locate the expression with summary:
include?.length ? (include.includes("summary") ? snapshot.summary : undefined) :
snapshot.summary and change it to check include === undefined vs
include.includes("summary").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 68c4fd0b-d9fd-454e-9306-9f0532c1d3ab
📒 Files selected for processing (4)
packages/core/src/index.tspackages/core/src/tools.tspackages/core/src/types.tspackages/mcp/src/server.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/core/src/tools.ts (1)
31-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve explicit
"needs-human"/"blocked"statuses in fallback mapping.
deriveTaskStatuscan still fall back tomapTaskStatus(job.status)(Line 42), butmapTaskStatuscurrently downgrades unknown statuses to"failed". That still collapses explicit intervention states if they arrive viajob.status.Suggested minimal fix
function mapTaskStatus(status: string): TaskSummary["status"] { if (status === "running") return "running"; if (status === "completed" || status === "completed_no_summary") return "done"; + if (status === "needs-human") return "needs-human"; + if (status === "blocked") return "blocked"; return "failed"; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/tools.ts` around lines 31 - 43, mapTaskStatus currently downgrades any unrecognized status to "failed", which can overwrite explicit "needs-human" and "blocked" states when deriveTaskStatus falls back to it; update mapTaskStatus (and keep deriveTaskStatus as-is) so it preserves those intervention statuses by returning "needs-human" when status === "needs-human" and "blocked" when status === "blocked" (or more generally return the incoming status when it already matches a valid TaskSummary["status"]) instead of mapping them to "failed".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/core/src/tools.ts`:
- Around line 31-43: mapTaskStatus currently downgrades any unrecognized status
to "failed", which can overwrite explicit "needs-human" and "blocked" states
when deriveTaskStatus falls back to it; update mapTaskStatus (and keep
deriveTaskStatus as-is) so it preserves those intervention statuses by returning
"needs-human" when status === "needs-human" and "blocked" when status ===
"blocked" (or more generally return the incoming status when it already matches
a valid TaskSummary["status"]) instead of mapping them to "failed".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 34fdf1e5-d4cb-494a-a061-79aa1c0c61ec
📒 Files selected for processing (4)
packages/core/src/index.tspackages/core/src/tools.tspackages/core/src/types.tspackages/mcp/src/server.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/index.ts
- packages/core/src/types.ts
…aden active view - mapTaskStatus now preserves needs-human, blocked, and queued instead of collapsing them to failed. deriveTaskStatus already handled these before falling through, but the fallback path no longer loses semantics. - list_tasks(view='active') now includes all non-terminal statuses (queued, running, blocked, needs-human) instead of only running. - Add isActiveTaskStatus predicate with ACTIVE_STATUSES set for clarity. Review feedback addressed: - CodeRabbit: mapTaskStatus can collapse needs-human/blocked to failed - CodeRabbit: active view should mean non-terminal, not just running
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/mcp/src/server.ts`:
- Line 267: The not-found branch in the get_task handler currently returns
defaultFormatCheckTask(result) directly; change it to use the configured
provider formatter (call provider.formatCheckTask(result) or the equivalent
configured check formatter used by check_task) and only fall back to
defaultFormatCheckTask(result) when the provider/formatter is undefined; update
the get_task flow that references result and provider so get_task and check_task
use the same provider.formatCheckTask behavior for consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd75e6cc-0e76-40da-9472-5a207a32196e
📒 Files selected for processing (2)
packages/core/src/tools.tspackages/mcp/src/server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/tools.ts
The new manager-style tools (get_task, list_tasks, get_session) were only registered in packages/mcp/src/server.ts (MCP SDK server). The ChatGPT connector has its own hand-rolled JSON-RPC handler and was never updated. Root cause: ChatGPT connector's buildTools() and tools/call handler had no knowledge of get_task / list_tasks / get_session. Calls would hit the "Unknown tool" error path, or the tools were invisible via tools/list. Changes: - Import listTasks, getSession, TaskSummary from @clawconnect/core - Add list_tasks, get_task, get_session tool definitions to buildTools() - Add tools/call handlers with proper scope filtering - get_task.include is respected: undefined = summary only, [] = core only, specific includes add only those fields
Replace include?: ('summary'|'updates'|'artifacts'|'diagnostics')[]
with detail?: 'core'|'summary'|'updates'|'artifacts'|'diagnostics'|'full'|'fullWithDiagnostics'
Avoids non-empty array tool-bridge issues by using a single enum value
instead of an array. Omitted detail defaults to 'summary' (same behavior).
Applied in packages/mcp/src/server.ts and apps/chatgpt/src/index.ts.
pnpm ready passes cleanly.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/mcp/src/server.ts (1)
270-272:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the configured check formatter for
get_taskmisses.Line 272 still bypasses
provider.formatCheckTask, so custom providers get inconsistent not-found output betweencheck_taskandget_task.Suggested fix
async ({ taskId, detail, mode }) => { const result = await checkTask(pool, { jobId: taskId, mode: (mode as CheckMode) ?? defaultMode }); - if (!result.found) return defaultFormatCheckTask(result); + if (!result.found) return fmtCheck(result); const snapshot = result.snapshot;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mcp/src/server.ts` around lines 270 - 272, When a checkTask call returns not found in the get_task handler, don't always return defaultFormatCheckTask; instead call the configured provider's formatter (provider.formatCheckTask) so custom providers get consistent "not found" output. Update the async handler that calls checkTask (the block using { taskId, detail, mode }) to branch on result.found and, when false, invoke provider.formatCheckTask(result, { detail, mode }) (or the provider's expected signature) and return that output instead of defaultFormatCheckTask; keep defaultFormatCheckTask as the fallback if provider.formatCheckTask is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/mcp/src/server.ts`:
- Around line 270-272: When a checkTask call returns not found in the get_task
handler, don't always return defaultFormatCheckTask; instead call the configured
provider's formatter (provider.formatCheckTask) so custom providers get
consistent "not found" output. Update the async handler that calls checkTask
(the block using { taskId, detail, mode }) to branch on result.found and, when
false, invoke provider.formatCheckTask(result, { detail, mode }) (or the
provider's expected signature) and return that output instead of
defaultFormatCheckTask; keep defaultFormatCheckTask as the fallback if
provider.formatCheckTask is missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4cbe4472-e13d-4649-966a-39c623bc6bb5
📒 Files selected for processing (2)
apps/chatgpt/src/index.tspackages/mcp/src/server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/chatgpt/src/index.ts
jaruesink
deleted the
codex/linear-mention-sfr-271-implement-manager-style-multi-taski
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
TaskStatusandTaskSummarytypes and extendedRunTaskResultwith an optionaltaskIdinpackages/core/src/types.tsto represent manager-level tasks (v1 mapstaskIdtojobId).listTasks(pool)inpackages/core/src/tools.tsand exported it frompackages/core/src/index.tsto enumerate compact task summaries across agents.list_tasks(view?)for compact task listings withactiveandallviews.get_task(taskId, detail?, mode?)for task inspection using detail presets:core,summary,updates,artifacts,diagnostics,full, andfullWithDiagnostics.get_session(sessionId, mode?, limit?, after?, agent?)for session debugging with snapshot, events, and tail modes.run_task,check_task, andlist_sessionsbehavior for backward compatibility while introducing the new task-level surfaces.Testing
pnpm readypasses on the PR branch after the finaldetailenum update.list_tasks({ view: "all" })list_tasks({ view: "active" })get_task({ detail: "core" })get_task({ detail: "updates" })get_task({ detail: "full" })get_task({ detail: "fullWithDiagnostics" })get_sessionsnapshot/events/tail behavior from earlier testingCodex Task
Summary by CodeRabbit