diff --git a/CHANGELOG.md b/CHANGELOG.md index f6af485a6c..51db6cb258 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Server startup no longer marks actively-running workflows as failed.** The `failOrphanedRuns()` call has been removed from `packages/server/src/index.ts` to match the CLI precedent (`packages/cli/src/cli.ts:256-258`). Per the new CLAUDE.md principle "No Autonomous Lifecycle Mutation Across Process Boundaries", a stuck `running` row is now transitioned explicitly by the user: via the per-row Cancel/Abandon buttons on the dashboard workflow card, or `archon workflow abandon ` from the CLI. (`archon workflow cleanup` is a separate command that deletes OLD terminal runs for disk hygiene — it does not handle stuck `running` rows.) Closes #1216. - **`MCP server connection failed: ` noise no longer surfaces in workflow runs.** The dag-executor now loads the workflow node's `mcp:` config file once and filters the SDK's failure message to only the servers the workflow actually configured. User-level Claude plugin MCPs (e.g. `telegram` inherited from `~/.claude/`) that fail to connect in the headless subprocess are debug-logged as `dag.mcp_plugin_connection_suppressed` instead of being forwarded to the conversation. Other provider warnings (⚠️) surface unchanged. Credits @MrFadiAi for reporting the issue in #1134 (that PR was 9 days stale and conflicting; this is a fresh re-do on current `dev`). +- **Web UI approval gates now auto-resume.** Previously, clicking Approve or Reject on a paused workflow from the Web UI only recorded the decision — the workflow never continued, and the user had to send a follow-up chat message (or use the CLI) to resume. Three fixes: (1) orchestrator-agent now threads `parentConversationId` through `executeWorkflow` for every web dispatch, (2) the `POST /approve` and `POST /reject` API handlers dispatch `/workflow run ` back through the orchestrator when `parent_conversation_id` is set and points at a web-platform parent (mirrors `workflowApproveCommand`/`workflowRejectCommand` on the CLI; non-web parents skip the auto-resume to prevent cross-adapter misrouting), and (3) the during-streaming status check in the DAG executor tolerates the `paused` state so a concurrent AI node in the same topological layer finishes its own stream rather than being aborted when a sibling approval node pauses the run. The Web UI reject button uses the proper `ConfirmRunActionDialog` with an optional reason textarea (was `window.confirm` in the chat card, and lacked a reason input on the dashboard) — the trimmed reason propagates to `$REJECTION_REASON` in the workflow's `on_reject` prompt. Credits @jonasvanderhaegen for surfacing and diagnosing the bug in #1147 (that PR was 87 commits stale on a dev that had since refactored the reject UX; this is a fresh re-do on current `dev`). Closes #1131. ### Changed diff --git a/packages/core/src/orchestrator/orchestrator-agent.test.ts b/packages/core/src/orchestrator/orchestrator-agent.test.ts index ab8165ca7e..3a4a1299c9 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.test.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.test.ts @@ -1099,6 +1099,42 @@ describe('workflow dispatch routing — interactive flag', () => { expect(mockExecuteWorkflow).toHaveBeenCalled(); expect(mockDispatchBackgroundWorkflow).not.toHaveBeenCalled(); + // Regression for the auto-resume plumbing: the interactive web dispatch + // must pass the caller conversation's DB id as parentConversationId + // (11th positional arg) so the approve/reject API handlers can dispatch + // resume back through the orchestrator. + const callArgs = mockExecuteWorkflow.mock.calls[0] as unknown[]; + expect(callArgs[10]).toBe('conv-1'); // parentConversationId = conversation.id + }); + + test('foreground_resume_detected: passes parentConversationId to executeWorkflow when a resumable run exists', async () => { + // Regression for the foreground-resume branch added as part of the + // auto-resume fix: when `findResumableRunByParentConversation` returns a + // paused run, the orchestrator picks the working_path from that run and + // must still carry parentConversationId forward so the API helpers can + // keep dispatching resume on subsequent approvals. + mockGetOrCreateConversation.mockReturnValueOnce(Promise.resolve(makeDispatchConversation())); + mockGetCodebase.mockReturnValueOnce(Promise.resolve(makeDispatchCodebase())); + mockHandleCommand.mockReturnValueOnce(Promise.resolve(makeWorkflowResult(true))); + mockFindResumableRunByParentConversation.mockReturnValueOnce( + Promise.resolve({ + id: 'resumable-run-1', + workflow_name: 'test-workflow', + working_path: '/repos/test-repo/worktrees/feature', + parent_conversation_id: 'conv-1', + status: 'failed', + }) + ); + + const platform = makePlatform(); // getPlatformType returns 'web' + await handleMessage(platform, 'conv-1', '/workflow run test-workflow'); + + expect(mockExecuteWorkflow).toHaveBeenCalled(); + const callArgs = mockExecuteWorkflow.mock.calls[0] as unknown[]; + // cwd (position 3) should come from the resumable run's working_path + expect(callArgs[3]).toBe('/repos/test-repo/worktrees/feature'); + // parentConversationId (position 10) should still be the caller conversation id + expect(callArgs[10]).toBe('conv-1'); }); test('calls dispatchBackgroundWorkflow for non-interactive workflow on web', async () => { diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index ba24331b69..27b9964835 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -293,7 +293,10 @@ async function dispatchOrchestratorWorkflow( workflow, userMessage, conversation.id, - codebase.id + codebase.id, + undefined, // issueContext + undefined, // isolationContext + conversation.id // parentConversationId — enables approve/reject auto-resume ); } else if (workflow.interactive) { // Interactive workflows run in foreground so output stays in the user's conversation @@ -305,7 +308,10 @@ async function dispatchOrchestratorWorkflow( workflow, userMessage, conversation.id, - codebase.id + codebase.id, + undefined, // issueContext + undefined, // isolationContext + conversation.id // parentConversationId — enables approve/reject auto-resume ); } else { await dispatchBackgroundWorkflow( @@ -331,7 +337,10 @@ async function dispatchOrchestratorWorkflow( workflow, userMessage, conversation.id, - codebase.id + codebase.id, + undefined, // issueContext + undefined, // isolationContext + conversation.id // parentConversationId — enables approve/reject auto-resume ); } } diff --git a/packages/core/src/orchestrator/orchestrator.test.ts b/packages/core/src/orchestrator/orchestrator.test.ts index b46523b153..58e5ac304e 100644 --- a/packages/core/src/orchestrator/orchestrator.test.ts +++ b/packages/core/src/orchestrator/orchestrator.test.ts @@ -1081,7 +1081,10 @@ describe('orchestrator-agent handleMessage', () => { expect.anything(), // workflow synthesized, // synthesizedPrompt, not original message expect.anything(), // conversation.id - expect.anything() // codebase.id + expect.anything(), // codebase.id + undefined, // issueContext + undefined, // isolationContext + expect.anything() // parentConversationId — web approval auto-resume ); }); @@ -1106,7 +1109,10 @@ describe('orchestrator-agent handleMessage', () => { expect.anything(), 'fix the login bug', // original message used as fallback expect.anything(), - expect.anything() + expect.anything(), + undefined, // issueContext + undefined, // isolationContext + expect.anything() // parentConversationId — web approval auto-resume ); }); diff --git a/packages/docs-web/src/content/docs/guides/approval-nodes.md b/packages/docs-web/src/content/docs/guides/approval-nodes.md index 42ebc48fec..c48f8c4856 100644 --- a/packages/docs-web/src/content/docs/guides/approval-nodes.md +++ b/packages/docs-web/src/content/docs/guides/approval-nodes.md @@ -55,9 +55,9 @@ to the user on whatever platform they're using (CLI, Slack, GitHub, etc.). On th block the worktree path guard (no other workflow can start on the same path). 4. **Approve**: The user approves, which writes a `node_completed` event for the approval node and transitions the run to resumable. Natural-language - messages (recommended) and the CLI auto-resume immediately. The explicit - `/workflow approve` command records the approval; send a follow-up message - to resume. + messages, the CLI, and the Web UI approve button all auto-resume the + workflow from the paused gate. (The explicit `/workflow approve ` + slash command also auto-resumes when issued in the originating conversation.) 5. **Reject**: The user rejects. - **Without `on_reject`**: The workflow is cancelled immediately. - **With `on_reject`**: The executor runs the `on_reject.prompt` via AI (with @@ -140,7 +140,19 @@ bun run cli workflow reject --reason "Plan needs more test coverage" ### Web UI Paused workflows show an amber pulsing badge on the dashboard. Click **Approve** -or **Reject** directly on the workflow card. +or **Reject** directly on the workflow card. Both actions auto-resume the +workflow from the paused gate — no follow-up message required. + +**Reject with reason**: the Reject dialog includes an optional free-text +reason field. The trimmed value (empty after trim → omitted) is passed to +the workflow as `$REJECTION_REASON`, available in the `on_reject.prompt`. +Rejects on web and chat cards use the same confirmation dialog. + +**Cross-platform caveat**: auto-resume via the Web UI only applies when the +run was originally dispatched from the Web UI (parent conversation is a web +conversation). If you approve a Slack / Telegram / GitHub-dispatched run +from the dashboard, the decision is recorded, but the resume flow has to +happen in the originating platform (re-run the workflow there). ### REST API diff --git a/packages/docs-web/src/content/docs/guides/authoring-workflows.md b/packages/docs-web/src/content/docs/guides/authoring-workflows.md index 5caea999f0..0fbc282640 100644 --- a/packages/docs-web/src/content/docs/guides/authoring-workflows.md +++ b/packages/docs-web/src/content/docs/guides/authoring-workflows.md @@ -1021,12 +1021,12 @@ nodes: When the workflow reaches `review-gate`, it pauses and notifies you. Approve or reject via: - **Natural language** (recommended): Just type your response in the conversation — the system detects the paused workflow and auto-resumes -- **CLI**: `bun run cli workflow approve ` or `bun run cli workflow reject ` -- **Explicit command**: `/workflow approve ` or `/workflow reject ` (records approval; send a follow-up message to resume) -- **Web UI**: Click the Approve/Reject buttons on the dashboard card +- **CLI**: `bun run cli workflow approve ` or `bun run cli workflow reject ` — auto-resumes +- **Explicit command**: `/workflow approve ` or `/workflow reject ` — auto-resumes when issued in the originating conversation +- **Web UI**: Click the Approve/Reject buttons on the dashboard card — auto-resumes for Web-UI-dispatched runs; the Reject dialog includes an optional reason field that flows to `$REJECTION_REASON` - **API**: `POST /api/workflows/runs//approve` or `/reject` -After approval via natural language or CLI, the workflow auto-resumes from the next node. The user's approval comment is available as `$review-gate.output` in downstream nodes only when `capture_response: true` is set on the approval node. +All four paths auto-resume the workflow from the next node. The user's approval comment is available as `$review-gate.output` in downstream nodes only when `capture_response: true` is set on the approval node. Cross-platform caveat: Web-UI approvals on Slack / Telegram / GitHub-dispatched runs record the decision but do not auto-resume — re-run from the originating platform to continue. Without `on_reject`: rejecting cancels the workflow. With `on_reject`: rejecting triggers an AI rework prompt and re-pauses for re-review. diff --git a/packages/server/src/routes/api.ts b/packages/server/src/routes/api.ts index 8adf5e836d..928a8f35cd 100644 --- a/packages/server/src/routes/api.ts +++ b/packages/server/src/routes/api.ts @@ -52,7 +52,7 @@ import { RESUMABLE_WORKFLOW_STATUSES, TERMINAL_WORKFLOW_STATUSES, } from '@archon/workflows/schemas/workflow-run'; -import type { ApprovalContext } from '@archon/workflows/schemas/workflow-run'; +import type { ApprovalContext, WorkflowRun } from '@archon/workflows/schemas/workflow-run'; import { findMarkdownFilesRecursive } from '@archon/core/utils/commands'; /** Lazy-initialized logger (deferred so test mocks can intercept createLogger) */ @@ -1035,6 +1035,95 @@ export function registerApiRoutes( return { accepted: true, status: result.status }; } + /** + * Re-enter the orchestrator after a paused approval gate is resolved, so a + * web-dispatched workflow continues (approve) or runs its on_reject prompt + * (reject) without the user having to re-run the workflow command. The CLI's + * `workflowApproveCommand` / `workflowRejectCommand` already auto-resume via + * `workflowRunCommand({ resume: true })`; this is the web-side equivalent. + * + * Returns `true` when a resume dispatch was initiated, `false` otherwise (no + * parent conversation on the run, parent conversation deleted, parent was on + * a non-web platform, or dispatch threw). Failures are non-fatal: the gate + * decision is recorded regardless; when this returns `false` the response + * text instructs the user to re-run the workflow command. + * + * **Cross-adapter guard**: only web-sourced parents qualify. + * `dispatchToOrchestrator` is wired to the web adapter + its lock manager, + * so a Slack / Telegram / GitHub / Discord run being approved from the + * dashboard must not route through it — the Slack thread would never see + * the resumed output. Non-web parents skip auto-resume and the originating + * platform's own re-run flow applies. + */ + async function tryAutoResumeAfterGate( + run: WorkflowRun, + action: 'approve' | 'reject' + ): Promise { + if (!run.parent_conversation_id) return false; + // Literal event names per action — greppable for ops tooling. Keeping the + // branch explicit rather than templating avoids the earlier 3-segment + // `api.workflow_*.dispatched` shape that broke `{domain}.{action}_{state}`. + const events = + action === 'approve' + ? { + dispatched: 'api.workflow_approve_auto_resume_dispatched' as const, + skippedNoPlatformConv: + 'api.workflow_approve_auto_resume_skipped_no_platform_conv' as const, + skippedNonWebParent: 'api.workflow_approve_auto_resume_skipped_non_web_parent' as const, + failed: 'api.workflow_approve_auto_resume_failed' as const, + } + : { + dispatched: 'api.workflow_reject_auto_resume_dispatched' as const, + skippedNoPlatformConv: + 'api.workflow_reject_auto_resume_skipped_no_platform_conv' as const, + skippedNonWebParent: 'api.workflow_reject_auto_resume_skipped_non_web_parent' as const, + failed: 'api.workflow_reject_auto_resume_failed' as const, + }; + try { + const parentConv = await conversationDb.getConversationById(run.parent_conversation_id); + const platformConvId = parentConv?.platform_conversation_id; + if (!platformConvId) { + // parentConv === null is a data-integrity signal (the parent + // conversation was deleted while the run was paused) — worth + // surfacing at info level so operators notice. Missing + // platform_conversation_id on an existing row shouldn't happen and + // stays at debug. + const logFn = + parentConv === null ? getLog().info.bind(getLog()) : getLog().debug.bind(getLog()); + logFn( + { + runId: run.id, + parentConversationId: run.parent_conversation_id, + parentDeleted: parentConv === null, + }, + events.skippedNoPlatformConv + ); + return false; + } + if (parentConv.platform_type !== 'web') { + getLog().debug( + { + runId: run.id, + parentConversationId: run.parent_conversation_id, + platformType: parentConv.platform_type, + }, + events.skippedNonWebParent + ); + return false; + } + const resumeMessage = `/workflow run ${run.workflow_name} ${run.user_message ?? ''}`.trim(); + await dispatchToOrchestrator(platformConvId, resumeMessage); + getLog().info( + { runId: run.id, workflowName: run.workflow_name, platformConvId }, + events.dispatched + ); + return true; + } catch (err) { + getLog().warn({ err: err as Error, runId: run.id }, events.failed); + return false; + } + } + // GET /api/conversations - List conversations registerOpenApiRoute(getConversationsRoute, async c => { try { @@ -1894,9 +1983,20 @@ export function registerApiRoutes( status: 'failed', metadata: metadataUpdate, }); + + // Auto-resume: dispatch to the orchestrator so the workflow continues + // without requiring the user to re-run the workflow command. Mirrors + // what `workflowApproveCommand` does in the CLI. Requires + // `parent_conversation_id` on the run (set by orchestrator-agent for any + // web-dispatched workflow — foreground, interactive, and background via + // the pre-created run) and a web-platform parent (guarded in the helper). + const autoResumed = await tryAutoResumeAfterGate(run, 'approve'); + return c.json({ success: true, - message: `Workflow approved: ${run.workflow_name}. Send a message to continue the workflow.`, + message: autoResumed + ? `Workflow approved: ${run.workflow_name}. Resuming workflow.` + : `Workflow approved: ${run.workflow_name}. Send a message to continue.`, }); } catch (error) { getLog().error({ err: error, runId }, 'api.workflow_run_approve_failed'); @@ -1940,9 +2040,18 @@ export function registerApiRoutes( status: 'failed', metadata: { rejection_reason: reason, rejection_count: currentCount + 1 }, }); + + // Auto-resume: dispatch to the orchestrator so the on_reject prompt runs + // without requiring the user to re-run the workflow command. Mirrors + // what `workflowRejectCommand` does in the CLI. Same cross-adapter + // guard as approve — only web parents auto-resume. + const autoResumed = await tryAutoResumeAfterGate(run, 'reject'); + return c.json({ success: true, - message: `Workflow rejected: ${run.workflow_name}. On-reject prompt will run on resume.`, + message: autoResumed + ? `Workflow rejected: ${run.workflow_name}. Running on-reject prompt.` + : `Workflow rejected: ${run.workflow_name}. On-reject prompt will run on resume.`, }); } diff --git a/packages/server/src/routes/api.workflow-runs.test.ts b/packages/server/src/routes/api.workflow-runs.test.ts index 41bee85003..8d837d3623 100644 --- a/packages/server/src/routes/api.workflow-runs.test.ts +++ b/packages/server/src/routes/api.workflow-runs.test.ts @@ -22,7 +22,8 @@ const mockGetWorkflowRunByWorkerPlatformId = mock( ); const mockListWorkflowEvents = mock(async (_runId: string) => [] as MockWorkflowEvent[]); const mockGetConversationById = mock( - async (_id: string) => null as null | { id: string; platform_conversation_id: string } + async (_id: string) => + null as null | { id: string; platform_conversation_id: string; platform_type: string } ); const mockFindConversationByPlatformId = mock( async (_id: string) => @@ -1362,3 +1363,186 @@ describe('POST /api/workflows/runs/:runId/reject', () => { expect(mockUpdateWorkflowRun).not.toHaveBeenCalled(); }); }); + +// --------------------------------------------------------------------------- +// Auto-resume: approve/reject endpoints dispatch to orchestrator when the run +// has parent_conversation_id set (web-dispatched foreground/interactive +// workflows). Mirrors what the CLI does in workflowApproveCommand/RejectCommand. +// --------------------------------------------------------------------------- + +describe('approve/reject auto-resume', () => { + beforeEach(() => { + mockGetWorkflowRun.mockReset(); + mockUpdateWorkflowRun.mockReset(); + mockCreateWorkflowEvent.mockReset(); + mockGetConversationById.mockReset(); + mockHandleMessage.mockReset(); + mockCancelWorkflowRun.mockReset(); + }); + + test('approve: dispatches resume when parent_conversation_id is set', async () => { + mockGetWorkflowRun.mockResolvedValueOnce({ + ...MOCK_PAUSED_RUN, + id: 'run-auto-resume-approve', + parent_conversation_id: 'parent-conv-uuid', + user_message: 'Deploy feature X', + }); + mockGetConversationById.mockResolvedValueOnce({ + id: 'parent-conv-uuid', + platform_conversation_id: 'web-plat-abc', + platform_type: 'web', + }); + + const { app } = makeApp(); + const response = await app.request('/api/workflows/runs/run-auto-resume-approve/approve', { + method: 'POST', + body: JSON.stringify({ comment: 'LGTM' }), + headers: { 'Content-Type': 'application/json' }, + }); + + expect(response.status).toBe(200); + const body = (await response.json()) as { message: string }; + expect(body.message).toContain('Resuming workflow'); + + // dispatchToOrchestrator → lockManager → handleMessage + expect(mockHandleMessage).toHaveBeenCalled(); + const [, platformConvId, dispatchedMessage] = mockHandleMessage.mock.calls[0] as [ + unknown, + string, + string, + ]; + expect(platformConvId).toBe('web-plat-abc'); + expect(dispatchedMessage).toBe('/workflow run deploy Deploy feature X'); + }); + + test('approve: skips dispatch when parent_conversation_id is null (CLI-dispatched run)', async () => { + mockGetWorkflowRun.mockResolvedValueOnce({ + ...MOCK_PAUSED_RUN, + parent_conversation_id: null, + }); + + const { app } = makeApp(); + const response = await app.request('/api/workflows/runs/run-paused-1/approve', { + method: 'POST', + body: JSON.stringify({ comment: 'LGTM' }), + headers: { 'Content-Type': 'application/json' }, + }); + + expect(response.status).toBe(200); + const body = (await response.json()) as { message: string }; + expect(body.message).toContain('Send a message to continue'); + expect(mockHandleMessage).not.toHaveBeenCalled(); + expect(mockGetConversationById).not.toHaveBeenCalled(); + }); + + test('approve: skips dispatch when parent conversation no longer exists', async () => { + mockGetWorkflowRun.mockResolvedValueOnce({ + ...MOCK_PAUSED_RUN, + parent_conversation_id: 'deleted-conv-uuid', + }); + mockGetConversationById.mockResolvedValueOnce(null); // conversation deleted + + const { app } = makeApp(); + const response = await app.request('/api/workflows/runs/run-paused-1/approve', { + method: 'POST', + body: JSON.stringify({}), + headers: { 'Content-Type': 'application/json' }, + }); + + expect(response.status).toBe(200); + const body = (await response.json()) as { message: string }; + expect(body.message).toContain('Send a message to continue'); + expect(mockHandleMessage).not.toHaveBeenCalled(); + }); + + test('approve: skips dispatch when parent conversation is on a non-web platform', async () => { + // A Slack/Telegram/GitHub-sourced run being approved via the dashboard + // must not route through dispatchToOrchestrator — that helper is wired + // to the web adapter + lock manager, so dispatching a Slack thread_ts + // or Telegram chat_id would misroute through the wrong adapter. + mockGetWorkflowRun.mockResolvedValueOnce({ + ...MOCK_PAUSED_RUN, + parent_conversation_id: 'slack-parent-conv-uuid', + }); + mockGetConversationById.mockResolvedValueOnce({ + id: 'slack-parent-conv-uuid', + platform_conversation_id: '1234567890.123456', // a Slack thread_ts + platform_type: 'slack', + }); + + const { app } = makeApp(); + const response = await app.request('/api/workflows/runs/run-paused-1/approve', { + method: 'POST', + body: JSON.stringify({ comment: 'LGTM' }), + headers: { 'Content-Type': 'application/json' }, + }); + + expect(response.status).toBe(200); + const body = (await response.json()) as { message: string }; + // Same fallback text as no-parent case — user re-runs from the originating platform. + expect(body.message).toContain('Send a message to continue'); + expect(mockHandleMessage).not.toHaveBeenCalled(); + }); + + test('reject: dispatches resume for on_reject flows when parent is set', async () => { + mockGetWorkflowRun.mockResolvedValueOnce({ + ...MOCK_PAUSED_RUN, + id: 'run-auto-resume-reject', + parent_conversation_id: 'parent-conv-uuid', + user_message: 'Review PR', + metadata: { + approval: { + type: 'approval', + nodeId: 'review-gate', + message: 'Approve?', + onRejectPrompt: 'Fix: $REJECTION_REASON', + onRejectMaxAttempts: 3, + }, + rejection_count: 0, + }, + }); + mockGetConversationById.mockResolvedValueOnce({ + id: 'parent-conv-uuid', + platform_conversation_id: 'web-plat-xyz', + platform_type: 'web', + }); + + const { app } = makeApp(); + const response = await app.request('/api/workflows/runs/run-auto-resume-reject/reject', { + method: 'POST', + body: JSON.stringify({ reason: 'tests missing' }), + headers: { 'Content-Type': 'application/json' }, + }); + + expect(response.status).toBe(200); + const body = (await response.json()) as { message: string }; + expect(body.message).toContain('Running on-reject prompt'); + expect(mockHandleMessage).toHaveBeenCalled(); + const [, platformConvId, dispatchedMessage] = mockHandleMessage.mock.calls[0] as [ + unknown, + string, + string, + ]; + expect(platformConvId).toBe('web-plat-xyz'); + expect(dispatchedMessage).toBe('/workflow run deploy Review PR'); + }); + + test('reject: does NOT dispatch when the run is being cancelled (no on_reject configured)', async () => { + mockGetWorkflowRun.mockResolvedValueOnce({ + ...MOCK_PAUSED_RUN, + parent_conversation_id: 'parent-conv-uuid', // set, but doesn't matter — reject cancels + }); + + const { app } = makeApp(); + const response = await app.request('/api/workflows/runs/run-paused-1/reject', { + method: 'POST', + body: JSON.stringify({ reason: 'no' }), + headers: { 'Content-Type': 'application/json' }, + }); + + expect(response.status).toBe(200); + // Cancellation path doesn't auto-resume — nothing to resume to. + expect(mockHandleMessage).not.toHaveBeenCalled(); + expect(mockCancelWorkflowRun).toHaveBeenCalledWith('run-paused-1'); + }); +}); diff --git a/packages/web/src/components/chat/WorkflowProgressCard.tsx b/packages/web/src/components/chat/WorkflowProgressCard.tsx index bb65471f3b..44eb70af74 100644 --- a/packages/web/src/components/chat/WorkflowProgressCard.tsx +++ b/packages/web/src/components/chat/WorkflowProgressCard.tsx @@ -5,6 +5,7 @@ import { CheckCircle, ChevronRight, Loader2, Pause, XCircle } from 'lucide-react import { cn } from '@/lib/utils'; import { approveWorkflowRun, getWorkflowRunByWorker, rejectWorkflowRun } from '@/lib/api'; import { useWorkflowStore } from '@/stores/workflow-store'; +import { ConfirmRunActionDialog } from '@/components/dashboard/ConfirmRunActionDialog'; import { StatusIcon } from '@/components/workflows/StatusIcon'; import { formatDurationMs } from '@/lib/format'; import { isTerminalStatus } from '@/lib/workflow-utils'; @@ -87,7 +88,7 @@ export function WorkflowProgressCard({ mutationFn: () => approveWorkflowRun(runId ?? ''), }); const rejectMutation = useMutation({ - mutationFn: () => rejectWorkflowRun(runId ?? ''), + mutationFn: (reason?: string) => rejectWorkflowRun(runId ?? '', reason), }); const mutationError = approveMutation.error ?? rejectMutation.error; @@ -220,18 +221,33 @@ export function WorkflowProgressCard({ Approve - + } + title="Reject workflow?" + description={ + <> + Reject the paused workflow {workflowName}. If the approval + node defines an on_reject prompt, it runs with your reason as{' '} + $REJECTION_REASON; otherwise the run is cancelled. + + } + confirmLabel="Reject" + reasonInput={{ + label: 'Reason (optional)', + placeholder: 'Why are you rejecting? Visible to the on_reject prompt.', }} - disabled={!runId || approveMutation.isPending || rejectMutation.isPending} - className="flex items-center gap-1 rounded-md px-2 py-1 text-xs text-error/80 hover:bg-error/10 hover:text-error transition-colors disabled:opacity-50" - > - - Reject - + onConfirm={(reason): void => { + rejectMutation.mutate(reason); + }} + /> {(approveMutation.isError || rejectMutation.isError) && (

diff --git a/packages/web/src/components/dashboard/ConfirmRunActionDialog.tsx b/packages/web/src/components/dashboard/ConfirmRunActionDialog.tsx index 2292aef3ce..4de85ce2bf 100644 --- a/packages/web/src/components/dashboard/ConfirmRunActionDialog.tsx +++ b/packages/web/src/components/dashboard/ConfirmRunActionDialog.tsx @@ -1,4 +1,4 @@ -import type { ReactNode } from 'react'; +import { useId, useState, type ReactNode } from 'react'; import { AlertDialog, AlertDialogAction, @@ -11,6 +11,16 @@ import { AlertDialogTrigger, } from '@/components/ui/alert-dialog'; +/** + * Optional free-text input rendered below the description. Used for the + * reject flow so reviewers can attach a reason that propagates to the + * workflow's `on_reject` prompt as `$REJECTION_REASON`. + */ +interface ReasonInputConfig { + label: string; + placeholder?: string; +} + interface Props { /** The element that opens the dialog when clicked (typically a button). */ trigger: ReactNode; @@ -20,11 +30,17 @@ interface Props { description: ReactNode; /** Confirm-button label (e.g. "Abandon", "Delete"). */ confirmLabel: string; - /** Invoked when the user confirms. The current callsites are all - * fire-and-forget wrappers around React Query mutations whose error - * handling lives at the page level (`runAction` in `DashboardPage.tsx`). - * Widen to `Promise` only if a caller needs to await the action. */ - onConfirm: () => void; + /** + * When provided, renders a textarea below the description. The trimmed + * value is passed to `onConfirm` — empty after trim becomes `undefined` + * so callers can distinguish "no reason given" from "empty string given". + */ + reasonInput?: ReasonInputConfig; + /** Invoked when the user confirms. Fire-and-forget; callers own error + * surfacing. Widen to `Promise` only if a future caller needs to + * await the action. `reason` is only non-`undefined` when `reasonInput` + * is supplied and the user typed something after trimming. */ + onConfirm: (reason?: string) => void; } /** @@ -36,6 +52,10 @@ interface Props { * `@/components/ui/alert-dialog`), which is appropriate for every workflow * lifecycle action this is used for (Abandon, Cancel, Delete, Reject). * + * For reject flows, pass `reasonInput` to collect a trimmed free-text reason + * that propagates to `$REJECTION_REASON` inside the workflow's `on_reject` + * prompt. + * * Replaces previous use of `window.confirm()` for these actions to match the * codebase-delete UX in `sidebar/ProjectSelector.tsx`. */ @@ -44,10 +64,22 @@ export function ConfirmRunActionDialog({ title, description, confirmLabel, + reasonInput, onConfirm, }: Props): React.ReactElement { + const [reason, setReason] = useState(''); + // useId() so multiple dialog instances on the same page (e.g. side-by-side + // run cards) don't collide on a shared DOM id. + const reasonInputId = useId(); + return ( - + { + // Reset the textarea every time the dialog closes so a previous + // reason doesn't bleed into the next reject action on the same card. + if (!open) setReason(''); + }} + > {trigger} @@ -56,6 +88,23 @@ export function ConfirmRunActionDialog({

{description}
+ {reasonInput && ( +
+ +