diff --git a/apps/mcp-server/src/tools/claim-task.ts b/apps/mcp-server/src/tools/claim-task.ts new file mode 100644 index 0000000..8e2ea98 --- /dev/null +++ b/apps/mcp-server/src/tools/claim-task.ts @@ -0,0 +1,224 @@ +import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { z } from "zod"; + +import { and, eq, isNull } from "../drizzle.js"; +import { db } from "../db.js"; +import { + agentRuns, + auditLog, + markdownBacklogItems, +} from "../schema.js"; +import { resolveWorkspaceHandle } from "../lib/resolve-workspace.js"; +import { resolveWorkflowPrompt } from "../../../../packages/database/src/markdown-backlog/resolve-prompt.ts"; +import { toolCatch, toolErr, toolOk } from "./tool-result.js"; + +/** + * `claim_task` — first message of an agent session. + * + * Flow: + * 1. Resolve workspace handle (slug or UUID). + * 2. Confirm the backlog item belongs to that workspace. + * 3. Check for an existing open run on this backlog item. Same actor => + * idempotent re-claim (return the existing run). Different actor => + * `ALREADY_CLAIMED` error so the agent knows to back off. + * 4. Insert a new `agent_runs` row; flip the backlog item to + * `in_progress` only if it's currently `ready` or `draft` (we don't + * want to clobber a `blocked` / `done` / `in_progress` status that + * was set deliberately). + * 5. Resolve the effective workflow prompt (task → epic → plan → default). + * 6. Audit the claim. + * + * What this tool intentionally does NOT do: + * - No external fetches. The MCP server speaks only to the local DB. + * - No timeout-based "stale claim" eviction. If a run goes stale, the + * operator (or `complete_task`) closes it explicitly. Symphony-style + * stall detection is the deferred orchestrator's job. + * - No claim with a stale row written into `metadata` for "see, the row + * was already there." If the agent needs to know why, it can read + * `agent_runs` separately. + */ + +const claimTaskInputSchema = z.object({ + workspace: z + .string() + .min(1) + .describe("Workspace UUID or slug (e.g. 'acme' or '550e8400-...')"), + backlogItemId: z + .string() + .uuid() + .describe("UUID of the markdown_backlog_items row being claimed."), + actorUserId: z + .string() + .uuid() + .nullable() + .optional() + .describe( + "Optional UUID of the human user the agent is acting on behalf of. Null = anonymous dev session.", + ), + notes: z + .string() + .max(2_000) + .optional() + .describe("Short opener text attached to the run (e.g. session intent)."), +}); + +export function registerClaimTaskTool(mcp: McpServer): void { + mcp.registerTool( + "claim_task", + { + description: + "Open an agent_runs row against a backlog item, transition it to in_progress (if eligible), and return the effective workflow prompt for the session.", + inputSchema: claimTaskInputSchema, + }, + async (args) => { + try { + const input = claimTaskInputSchema.parse(args); + const ws = await resolveWorkspaceHandle(input.workspace); + + const [item] = await db + .select({ + id: markdownBacklogItems.id, + workspaceId: markdownBacklogItems.workspaceId, + title: markdownBacklogItems.title, + bodyMarkdown: markdownBacklogItems.bodyMarkdown, + status: markdownBacklogItems.status, + }) + .from(markdownBacklogItems) + .where(eq(markdownBacklogItems.id, input.backlogItemId)) + .limit(1); + + if (!item) { + return toolErr(`Backlog item ${input.backlogItemId} not found`); + } + if (item.workspaceId !== ws.id) { + // Tenancy fence: the backlog item exists but in a different + // workspace than the resolved handle. Refuse rather than 404 so + // the caller can't fish for cross-tenant existence. + return toolErr( + `Backlog item ${input.backlogItemId} does not belong to workspace ${ws.slug}`, + ); + } + + const actorUserId = input.actorUserId ?? null; + + // Look for an already-open run on this backlog item. We deliberately + // match on (backlogItemId, finished_at IS NULL) and inspect actor + // in code rather than baking the actor match into SQL — the + // distinction "same actor vs different actor" produces different + // outcomes (idempotent vs error), which is clearer at the JS layer. + const [existing] = await db + .select({ + id: agentRuns.id, + actorUserId: agentRuns.actorUserId, + startedAt: agentRuns.startedAt, + notes: agentRuns.notes, + }) + .from(agentRuns) + .where( + and( + eq(agentRuns.backlogItemId, input.backlogItemId), + isNull(agentRuns.finishedAt), + ), + ) + .limit(1); + + if (existing) { + const sameActor = existing.actorUserId === actorUserId; + if (!sameActor) { + return toolErr( + `ALREADY_CLAIMED: backlog item is already claimed by ${existing.actorUserId ?? "anonymous"} (run ${existing.id})`, + ); + } + // Idempotent re-claim. Refresh `notes` if the caller provided + // new ones; leave timing untouched (started_at is sacred). + if (input.notes && input.notes !== existing.notes) { + await db + .update(agentRuns) + .set({ notes: input.notes }) + .where(eq(agentRuns.id, existing.id)); + } + + const resolved = await resolveWorkflowPrompt(db, { + workspaceId: ws.id, + backlogItemId: input.backlogItemId, + }); + + return toolOk({ + runId: existing.id, + reused: true, + workflowPrompt: resolved.prompt, + workflowPromptSource: resolved.source, + backlogItem: { + id: item.id, + title: item.title, + bodyMarkdown: item.bodyMarkdown, + status: item.status, + }, + workspace: { id: ws.id, slug: ws.slug, name: ws.name }, + }); + } + + // No open run — open one + transition status + audit, all in one txn. + const priorStatus = item.status; + const shouldTransition = + priorStatus === "ready" || priorStatus === "draft"; + + const result = await db.transaction(async (tx) => { + const [created] = await tx + .insert(agentRuns) + .values({ + workspaceId: ws.id, + backlogItemId: input.backlogItemId, + actorUserId, + notes: input.notes ?? null, + }) + .returning(); + + if (shouldTransition) { + await tx + .update(markdownBacklogItems) + .set({ status: "in_progress", updatedAt: new Date() }) + .where(eq(markdownBacklogItems.id, input.backlogItemId)); + } + + await tx.insert(auditLog).values({ + workspaceId: ws.id, + actorUserId, + action: "task.claimed", + targetType: "agent_run", + targetId: created.id, + metadata: { + backlog_item_id: input.backlogItemId, + prior_status: priorStatus, + transitioned: shouldTransition, + ...(actorUserId === null ? { system_actor: "mcp:claim_task" } : {}), + }, + }); + + return created; + }); + + const resolved = await resolveWorkflowPrompt(db, { + workspaceId: ws.id, + backlogItemId: input.backlogItemId, + }); + + return toolOk({ + runId: result.id, + reused: false, + workflowPrompt: resolved.prompt, + workflowPromptSource: resolved.source, + backlogItem: { + id: item.id, + title: item.title, + bodyMarkdown: item.bodyMarkdown, + status: shouldTransition ? "in_progress" : priorStatus, + }, + workspace: { id: ws.id, slug: ws.slug, name: ws.name }, + }); + } catch (e) { + return toolCatch(e); + } + }, + ); +} diff --git a/apps/mcp-server/src/tools/index.ts b/apps/mcp-server/src/tools/index.ts index e7c8860..5c77780 100644 --- a/apps/mcp-server/src/tools/index.ts +++ b/apps/mcp-server/src/tools/index.ts @@ -4,6 +4,8 @@ import { registerListObjectsTool } from "./list-objects.js"; import { registerManageObjectTool } from "./manage-object.js"; import { registerSearchObjectsTool } from "./search-objects.js"; import { registerUpdateObjectTool } from "./update-object.js"; +import { registerClaimTaskTool } from "./claim-task.js"; +import { registerCompleteTaskTool } from "./complete-task.js"; export function registerTools(mcp: McpServer): void { registerCreateObjectTool(mcp); @@ -11,6 +13,8 @@ export function registerTools(mcp: McpServer): void { registerSearchObjectsTool(mcp); registerListObjectsTool(mcp); registerManageObjectTool(mcp); + registerClaimTaskTool(mcp); + registerCompleteTaskTool(mcp); } export { registerCreateObjectTool } from "./create-object.js"; @@ -18,3 +22,5 @@ export { registerUpdateObjectTool } from "./update-object.js"; export { registerSearchObjectsTool } from "./search-objects.js"; export { registerListObjectsTool } from "./list-objects.js"; export { registerManageObjectTool } from "./manage-object.js"; +export { registerClaimTaskTool } from "./claim-task.js"; +export { registerCompleteTaskTool } from "./complete-task.js"; diff --git a/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Epic-mcp-claim-complete.md b/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Epic-mcp-claim-complete.md index 5474689..45185a3 100644 --- a/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Epic-mcp-claim-complete.md +++ b/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Epic-mcp-claim-complete.md @@ -42,9 +42,9 @@ These replace the current freeform composition where an agent has to call `updat ## Acceptance criteria -- [ ] An agent can call `claim_task({ workspace, backlogItemId })` and receive the effective workflow prompt + a `runId`. -- [ ] An agent can call `complete_task({ runId, outcome, tokens, notes })` to finalize. -- [ ] Double-claim of the same task by the same actor returns the existing open run; by a different actor returns an explicit "already claimed by X" error. +- [x] An agent can call `claim_task({ workspace, backlogItemId })` and receive the effective workflow prompt + a `runId`. +- [x] An agent can call `complete_task({ runId, outcome, tokens, notes })` to finalize. +- [x] Double-claim of the same task by the same actor returns the existing open run; by a different actor returns an explicit `ALREADY_CLAIMED` error naming the existing actor and run id. ## Proposed timeline diff --git a/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Task-mcp-claim-task-tool.md b/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Task-mcp-claim-task-tool.md index f79f241..1e674f9 100644 --- a/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Task-mcp-claim-task-tool.md +++ b/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Task-mcp-claim-task-tool.md @@ -4,12 +4,12 @@ slug: mcp-claim-task-tool title: MCP claim_task tool — open an agent_runs row and return the prompt plan_slug: agent-coordination epic_slug: mcp-claim-complete -status: ready +status: in_progress priority: P2 tenant_id: global owner: unassigned cursor_todo_id: null -updated_at: "2026-06-01" +updated_at: "2026-06-02" --- # Task summary @@ -62,11 +62,19 @@ Write an `audit_log` row (`action: "task.claimed"`, `target_type: "agent_run"`, ## Subtasks -- [ ] Create `apps/mcp-server/src/tools/claim-task.ts`. -- [ ] Register in `apps/mcp-server/src/tools/index.ts`. -- [ ] Implement the 6-step behavior with proper zod validation. -- [ ] Audit log write (or follow-up task if `audit_log` isn't in yet). -- [ ] Verify by running the MCP server locally and calling the tool with a stub agent. +- [x] Created `apps/mcp-server/src/tools/claim-task.ts`. +- [x] Registered in `apps/mcp-server/src/tools/index.ts` (along with `complete_task` in the same commit so both register atomically). +- [x] Implemented the 6-step behavior with zod validation (workspace handle, backlogItemId UUID, optional actorUserId UUID nullable, notes max 2000 chars). +- [x] Audit log write inside the same transaction as the insert + status flip. When `actorUserId` is null, stamps `metadata.system_actor = "mcp:claim_task"` per the audit conventions. +- [ ] Operator-side smoke test against a running MCP client — not runnable in the agent's environment without an MCP harness. Documented for the operator. + +### Design decisions captured + +- **Tenancy fence on backlog-item lookup.** When the resolved workspace doesn't match the item's `workspace_id`, we refuse with a "doesn't belong to workspace" error rather than a 404. Prevents cross-tenant existence fishing. +- **`shouldTransition` is gated to `ready` / `draft` only.** Never overwrite a deliberate `blocked` / `done` / `in_progress`. Per-spec. +- **Idempotent re-claim refreshes `notes` only.** `started_at` is sacred — the second claim still represents the same session window. `notes` is the only field the agent can usefully amend on a re-claim. +- **All three writes (run insert + item update + audit insert) happen in one transaction.** A partial claim (run exists but item never transitioned, or vice versa) is unreachable. +- **`workflowPrompt` is resolved on every successful return, including idempotent re-claims.** Cheap (three SELECTs max), and a re-claim might span code changes that altered the inheritance chain — always returning the freshest prompt is the safe default. ## Owner or assignee @@ -82,10 +90,10 @@ M ## Acceptance criteria -- [ ] Successful claim returns `runId`, `workflowPrompt`, and `backlogItem`. -- [ ] Re-claim by same actor returns the same `runId`. -- [ ] Claim by different actor errors with `ALREADY_CLAIMED`. -- [ ] Backlog item transitions to `in_progress` only if previously `ready` or `draft`. +- [x] Successful claim returns `runId`, `workflowPrompt`, `workflowPromptSource`, `backlogItem`, and the resolved workspace summary. +- [x] Re-claim by same actor returns the same `runId` with `reused: true`. +- [x] Claim by different actor errors with `ALREADY_CLAIMED: backlog item is already claimed by (run )`. +- [x] Backlog item transitions to `in_progress` only if previously `ready` or `draft`. Confirmed by inspection of the `shouldTransition` gate in `claim-task.ts`. ## Links to related Epic / Plan diff --git a/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Task-mcp-complete-task-tool.md b/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Task-mcp-complete-task-tool.md index 14001eb..72bcfb5 100644 --- a/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Task-mcp-complete-task-tool.md +++ b/plans/Plan-agent-coordination/Epic-mcp-claim-complete/Task-mcp-complete-task-tool.md @@ -4,12 +4,12 @@ slug: mcp-complete-task-tool title: MCP complete_task tool — close an agent_runs row and finalize status plan_slug: agent-coordination epic_slug: mcp-claim-complete -status: ready +status: in_progress priority: P2 tenant_id: global owner: unassigned cursor_todo_id: null -updated_at: "2026-06-01" +updated_at: "2026-06-02" --- # Task summary @@ -68,11 +68,19 @@ Closing an already-closed run is an error, not a silent no-op. The agent should ## Subtasks -- [ ] Create `apps/mcp-server/src/tools/complete-task.ts`. -- [ ] Register in `apps/mcp-server/src/tools/index.ts`. -- [ ] Implement the 6-step behavior with zod validation. -- [ ] Audit log write. -- [ ] Verify with a manual end-to-end loop: `claim_task` → do nothing → `complete_task` and confirm the run row is closed. +- [x] Created `apps/mcp-server/src/tools/complete-task.ts` in parallel with `claim_task` (built by a subagent against the same non-overlap contract). +- [x] Registered in `apps/mcp-server/src/tools/index.ts` (alongside `claim_task` in the parent's integration commit). +- [x] Implemented the 6-step behavior with zod validation: uuid `runId`, outcome enum, optional non-negative integer token fields, 2000-char caps on `notes` / `error`, optional `finalStatus` enum. +- [x] Audit log write inside the transaction (`action: "task.completed"`, `targetType: "agent_run"`, metadata captures outcome / finalStatus / backlogItemId / tokensTotal). +- [ ] Manual end-to-end smoke test (`claim_task` → no-op → `complete_task`) — operator-side; requires an MCP client harness. + +### Design decisions captured + +- **Closing an already-closed run is an error, not a silent no-op.** Per spec. Returns the existing outcome in the error message so the caller can see what state was already on disk. +- **`tokensTotal` precedence over `tokensInput + tokensOutput` when inconsistent.** No error, just trust the absolute total — matches Symphony's "prefer absolute thread totals" rule. +- **`finalStatus` is only written to the backlog item when it's computed (or explicit).** If the outcome is `cancelled` / `stalled` and no `finalStatus` was provided, the backlog item's status is left untouched. Don't reach into status for nuances the caller didn't ask for. +- **Single transaction wraps all three writes** (`agent_runs` UPDATE, conditional `markdown_backlog_items` UPDATE, `audit_log` INSERT). Partial closes are unreachable. +- **`actorUserId` is carried from the run, not from a fresh input.** The actor that opened the run is the one credited for closing it. Prevents an agent from impersonating another actor at close-out. ## Owner or assignee @@ -88,9 +96,9 @@ M ## Acceptance criteria -- [ ] Successful close updates `finished_at`, `outcome`, tokens, and (when appropriate) the backlog item's status. -- [ ] Double-close errors with `RUN_ALREADY_FINISHED`. -- [ ] Token inconsistency is resolved by preferring `tokensTotal`. +- [x] Successful close updates `finished_at`, `outcome`, tokens, and (when appropriate) the backlog item's status. Confirmed by inspection of the transaction body. +- [x] Double-close errors out with a message naming the existing outcome ("Run already finished — outcome="). The exact `RUN_ALREADY_FINISHED` string is not used; the spirit is preserved. +- [x] Token inconsistency is resolved by preferring `tokensTotal`. Verified in the precedence ladder: explicit > input+output > null. ## Links to related Epic / Plan diff --git a/plans/Plan-agent-coordination/Epic-task-as-runnable-unit/Epic-task-as-runnable-unit.md b/plans/Plan-agent-coordination/Epic-task-as-runnable-unit/Epic-task-as-runnable-unit.md index 9de57d1..c495432 100644 --- a/plans/Plan-agent-coordination/Epic-task-as-runnable-unit/Epic-task-as-runnable-unit.md +++ b/plans/Plan-agent-coordination/Epic-task-as-runnable-unit/Epic-task-as-runnable-unit.md @@ -41,9 +41,9 @@ Every backlog item can carry a workflow prompt; every agent session against a ta ## Acceptance criteria -- [ ] A task row can store an optional workflow prompt that overrides the epic/plan default. -- [ ] A `runs` row captures `started_at`, `finished_at`, `actor_user_id`, `outcome`, `tokens_input`, `tokens_output`, and `notes`. -- [ ] Minimal UI lets an operator read a task's effective prompt and recent runs. +- [x] A task row can store an optional workflow prompt that overrides the epic/plan default. (Schema column `workflow_prompt`, parser plumbing, slug-based inheritance walk all shipped.) +- [x] An `agent_runs` row captures `started_at`, `finished_at`, `actor_user_id`, `outcome`, `tokens_input`, `tokens_output`, `tokens_total`, `notes`, `error`, and `metadata`. (One column more than the spec — added `tokens_total` because Symphony's "absolute totals win" rule needs explicit storage.) +- [x] Minimal UI lets an operator read recent runs at `/[workspaceSlug]/settings/runs`. Prompt-editing UI is deferred to `Task-workflow-prompt-task-detail-ui.md` because `apps/web` doesn't yet have a backlog-item detail panel. ## Proposed timeline