feat: conversational task proposals (propose_task → card → accept)#93
feat: conversational task proposals (propose_task → card → accept)#93Fullstop000 wants to merge 23 commits intomainfrom
Conversation
Pull the per-task insert block (sub-channel row, creator membership, task row) out of create_tasks' loop body into a tx-scoped helper shared with the upcoming accept_task_proposal path. The helper does not read MAX(task_number) — callers pass the number they've chosen under whatever serialization guarantee they hold. Event emission stays at the call site so accept_task_proposal can skip it (the proposal card already carries the creation signal). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror the sibling pattern in channels.rs and agents.rs: decode rows into TaskProposal via a named-field constructor instead of a positional 10-tuple where two Option<String> fields sat side-by-side and could be silently swapped. Invalid status or RFC 3339 timestamp now surface as rusqlite::Error::FromSqlConversionFailure with the offending proposal id embedded in the message so a live-DB debugger can locate the bad row. Strengthen the happy-path test to assert channel_id plus all four None- valued fields on a pending proposal, and add a created_at sanity bound. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds GET /api/task-proposals/{id}, POST /api/task-proposals/{id}/accept,
POST /api/task-proposals/{id}/dismiss. The accept handler explicitly
invokes deliver_message_to_agents on the newly created sub-channel after
the store transaction commits — that's how the proposer agent wakes up
and starts a run in the task sub-channel (Chorus does not auto-wake on
new messages). Introduces TaskProposalAlreadyResolved typed error code
(409 CONFLICT) so the UI can distinguish "already accepted/dismissed"
from generic errors. E2e tests cover the happy path for all three
endpoints.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a task_proposal render branch that sits before the task_event branch. Uses useTaskProposalLog's inverse-seq index for O(1) detection (no JSON re-parse), suppresses repeat snapshots, and anchors the card at the latest seq. TaskProposalCardBoundary wires accept/dismiss mutations and pulls currentUser off the store. Visibility-wrapper and firstUnreadAnchorRef preserved so unread anchors still work on suppressed rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a “conversational task proposals” flow where agents can propose tasks via a new MCP tool, the UI renders proposals as interactive cards, and user acceptance creates/claims a task and opens a task sub-channel (with an agent wake-up).
Changes:
- Added a persisted
task_proposalsstore table + store APIs to create/accept/dismiss proposals and emitkind="task_proposal"system-message snapshots. - Added HTTP endpoints for fetching/accepting/dismissing proposals and an internal agent endpoint used by the MCP bridge.
- Added UI parsing/folding (
useTaskProposalLog) and aTaskProposalMessagecard renderer, plus Playwright + Rust test coverage.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/hooks/useTaskProposalLog.ts | Folds proposal snapshot messages into a per-proposal state index for rendering. |
| ui/src/hooks/useTaskProposalLog.test.ts | Unit tests for folding/snapshot ordering behavior. |
| ui/src/data/taskProposals.ts | Adds proposal payload parser and React Query mutation hooks for accept/dismiss. |
| ui/src/data/taskProposals.test.ts | Unit tests for the proposal JSON parser. |
| ui/src/components/chat/TaskProposalMessage.tsx | Proposal card UI (pending actions + accepted/dismissed terminal states). |
| ui/src/components/chat/TaskProposalMessage.test.tsx | Rendering tests for the proposal card across lifecycle states. |
| ui/src/components/chat/TaskProposalMessage.css | Styling for the proposal card and its state variants. |
| ui/src/components/chat/MessageList.tsx | Integrates proposal card rendering into the chat message list and wires mutations. |
| ui/src/components/chat/MessageList.test.tsx | Ensures proposal snapshot suppression renders a single card per proposal. |
| tests/store_tests.rs | Store-level tests for task proposal lifecycle, DB schema, and acceptance side-effects. |
| tests/e2e_tests.rs | HTTP e2e tests for proposal endpoints and error cases. |
| tests/bridge_serve_tests.rs | Bridge e2e test for the new propose_task backend HTTP path. |
| src/utils/error.rs | Adds TASK_PROPOSAL_ALREADY_RESOLVED typed error code (409). |
| src/store/tasks/mod.rs | Extracts shared helper to insert task + sub-channel + membership within a transaction. |
| src/store/task_proposals.rs | New store module implementing task proposal persistence + accept/dismiss flows. |
| src/store/schema.sql | Adds the task_proposals table and index to the schema. |
| src/store/mod.rs | Exposes the new task_proposals module. |
| src/store/migrations.rs | Adds migration to create task_proposals table for existing DBs. |
| src/server/mod.rs | Registers internal + public HTTP routes for task proposals. |
| src/server/handlers/task_proposals.rs | Implements HTTP handlers for proposal CRUD/resolve and internal agent propose endpoint. |
| src/server/handlers/mod.rs | Exports the new task proposal handler module. |
| src/bridge/types.rs | Adds MCP tool params struct for propose_task. |
| src/bridge/mod.rs | Adds the propose_task MCP tool on ChatBridge. |
| src/bridge/backend.rs | Implements Backend::propose_task for ChorusBackend. |
| qa/cases/tasks.md | Adds QA catalog entry for the proposal accept flow (TSK-005). |
| qa/cases/playwright/TSK-005.spec.ts | Playwright smoke test covering propose → accept → deep-link → kickoff message. |
| docs/BACKEND.md | Documents the task proposal flow and the no-task_event emission scope constraint. |
| resolvedBy: typeof o.resolvedBy === 'string' ? o.resolvedBy : null, | ||
| resolvedAt: typeof o.resolvedAt === 'string' ? o.resolvedAt : null, |
There was a problem hiding this comment.
The docstring says this is a strict parser that rejects malformed payloads, but resolvedBy/resolvedAt currently coerce wrong types to null instead of returning null for the whole payload. To match the stated contract (and parseTaskEvent), treat present-but-wrong-type values as invalid and return null.
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(body), | ||
| }) | ||
| if (!r.ok) throw new Error(`accept failed: ${r.status}`) |
There was a problem hiding this comment.
The accept mutation uses raw fetch and throws Error('accept failed: <status>'), which drops the server-provided JSON { error, code } (including TASK_PROPOSAL_ALREADY_RESOLVED). Consider reusing ui/src/data/client.ts helpers so callers get a structured ApiError with message + typed code and consistent error handling across the app.
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(body), | ||
| }) | ||
| if (!r.ok) throw new Error(`dismiss failed: ${r.status}`) |
There was a problem hiding this comment.
Same issue as accept: the dismiss mutation throws a generic status-only error and ignores the server error payload/code. Using the shared post() helper from ui/src/data/client.ts would preserve the backend's error message and code for UI handling and keep API calls consistent.
| setCurrentTaskDetail({ | ||
| parentChannelId: conversationId, | ||
| parentSlug: "", | ||
| taskNumber: state.taskNumber, | ||
| returnToTab: "chat", |
There was a problem hiding this comment.
setCurrentTaskDetail is called with parentSlug: "", but parentSlug is used for breadcrumbs/back-navigation (and the task_event branch passes targetKey). Pass the channel slug/name into TaskProposalCardBoundary (e.g. targetKey) and use it here so the task detail header isn’t blank/inconsistent.
| // sub-channel who's an agent). Dispatch errors are reported to the | ||
| // caller because a proposal that creates a task but fails to wake the | ||
| // agent leaves the user staring at a silent sub-channel — worse than | ||
| // an error shown up-front. | ||
| crate::server::handlers::messages::deliver_message_to_agents( | ||
| &state, | ||
| &accepted.sub_channel_id, | ||
| "system", | ||
| &accepted.kickoff_message_id, | ||
| ) | ||
| .await | ||
| .map_err(|e| app_err!(StatusCode::INTERNAL_SERVER_ERROR, "wake dispatch: {e}"))?; |
There was a problem hiding this comment.
If deliver_message_to_agents fails, this handler returns a 500 even though accept_task_proposal has already committed (task + channels + snapshot messages). That makes the operation non-idempotent for clients (they’ll likely retry and then hit 409) and can show an error despite success. Consider logging/metricing the wake failure and still returning the successful accept response (or a 200 with a warning field) to reflect the committed state.
| // sub-channel who's an agent). Dispatch errors are reported to the | |
| // caller because a proposal that creates a task but fails to wake the | |
| // agent leaves the user staring at a silent sub-channel — worse than | |
| // an error shown up-front. | |
| crate::server::handlers::messages::deliver_message_to_agents( | |
| &state, | |
| &accepted.sub_channel_id, | |
| "system", | |
| &accepted.kickoff_message_id, | |
| ) | |
| .await | |
| .map_err(|e| app_err!(StatusCode::INTERNAL_SERVER_ERROR, "wake dispatch: {e}"))?; | |
| // sub-channel who's an agent). Because acceptance has already committed | |
| // by this point, wake dispatch is best-effort: failures are logged for | |
| // operators but do not change the successful response, which must | |
| // reflect the durable accepted state. | |
| if let Err(e) = crate::server::handlers::messages::deliver_message_to_agents( | |
| &state, | |
| &accepted.sub_channel_id, | |
| "system", | |
| &accepted.kickoff_message_id, | |
| ) | |
| .await | |
| { | |
| eprintln!( | |
| "failed to dispatch kickoff message to agents after accepting task proposal {id}: {}", | |
| e | |
| ); | |
| } |
| export function parseTaskProposal(content: string): TaskProposalPayload | null { | ||
| let v: unknown | ||
| try { | ||
| v = JSON.parse(content) | ||
| } catch { | ||
| return null | ||
| } |
There was a problem hiding this comment.
parseTaskProposal always attempts JSON.parse even when content is plain text (common for senderType='system' messages like kickoff text). This relies on exceptions for control flow and can be a hot path; consider adding the same fast-path guard as parseTaskEvent (e.g., return null when content[0] !== '{') before parsing.
|
Closing — superseded by the task-lifecycle unified design. Instead of landing v1 (proposal flow) + v2 (context snapshot) incrementally, we're building the full unified model as one cohesive PR: one `tasks` table with a six-state enum (`proposed / dismissed / todo / in_progress / in_review / done`), one evolving `TaskCard` message component replacing both the proposal card and the sub-channel task card. Every invariant this PR established survives on the new branch — snapshot CHECK constraint, RFC3339 normalization, membership preconditions, kickoff ordering contract. No work thrown away; the scaffolding just moves onto the merged surface.
|
Summary
Reshapes task origination from kanban-first to conversation-first. An agent mentioned with substantive work now calls a new
propose_taskMCP tool, which posts an interactive card in the channel. One click from the user creates the task, auto-claims to the proposer, opens the sub-channel, and wakes the agent to start working there.New surfaces:
propose_task(channel, title)onChatBridgetask_proposalstable withpending | accepted | dismissedlifecycle + atomic compare-and-set on resolveGET /api/task-proposals/:id,POST /api/task-proposals/:id/{accept,dismiss}, internalPOST /internal/agent/:agent/channels/:channel/task-proposalstask_proposal(pending + terminal-state snapshots, folded byuseTaskProposalLog)TaskProposalMessagecard — pending (two buttons) / accepted (deep-link) / dismissed (muted)Scope anchors (pinned at plan-review time)
accept_task_proposaldoes NOT emittask_eventcreated/claimed— the proposal card already carries that signal. Subsequentstatus_changedevents still emit normally.#[tool(description = ...)]docstring; no system-prompt carve-out. Agents usepropose_taskfor delegated work, reply inline for Q&A.Plan review loop
Plan went through 4 rounds of adversarial review with Gemini before execution:
CallToolResulttype, missingSenderTypeimport, harness mismatch, visibility drift, tuple size)ChorusBackendfield/method names, danglingtask_proposal.updatedstream-event references)ProposalViewnaming,useTaskProposalhook cleanup)Implementation review loop
Subagent-driven-development — 18 tasks, each with implementer + spec review + code quality review. 2 Important-severity findings surfaced during execution (both addressed in the same commit cycle):
TaskProposal::from_rowhelper (eliminated silent-swap risk between adjacentOption<String>positions)updated_at = datetime('now')on auto-claim UPDATE (matches every otherUPDATE tasksin the codebase)Test plan
cargo test— 542 passing across 17 binaries, 0 failing. Includes 341 lib, 64 store, 16 e2e, 14 bridge_serve.cargo clippy --all-targets -- -D warnings— cleancargo fmt --all --check— clean (one drift commitf265575from mechanical edits)cd ui && npx tsc --noEmit— cleancd ui && npm run test— 90 passing across 19 vitest filescargo run -- serve+npm run dev; create channel + agent, @mention with substantive work, click create on the proposal card, verify task opens in sub-channel and claude posts a first message there; then the same flow but click dismiss — card updates to muted tag, no task created./gstack-qaon the conversational-task flow before mergeKnown deferred items
anyhow::Result→ typedAcceptError/DismissErrorenum would eliminate themsg.contains("...")fragility at handler layer. Scope creep across multiple tasks; negative-path e2e tests inadb2be0pin the coupling at test level meanwhile.tracing::error!whendeliver_message_to_agentsfails after commit would aid on-call. Trivial follow-up.parseTaskProposalcoerces wrong-typeresolvedByto null; siblingparseTaskEventrejects. Worth a pass.🤖 Generated with Claude Code