diff --git a/packages/core/src/orchestrator/orchestrator-agent.test.ts b/packages/core/src/orchestrator/orchestrator-agent.test.ts index ee2abede..8d120e46 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.test.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.test.ts @@ -1102,7 +1102,8 @@ describe('workflow dispatch routing — interactive flag', () => { // Verify parentConversationId is passed so resume-after-approval works const callArgs = mockExecuteWorkflow.mock.calls[0] as unknown[]; - // param 11 (index 10) is parentConversationId — should be conversation.id + // executeWorkflow is called with 11 positional args; index 10 is parentConversationId + expect(callArgs).toHaveLength(11); expect(callArgs[10]).toBe('conv-1'); }); 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 42ebc48f..e6c02aee 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,10 @@ 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 (recommended), the CLI, and the Web UI all auto-resume immediately. + The explicit `/workflow approve` slash command records the approval and also + auto-resumes on the Web UI; on other platforms it requires a follow-up + message to trigger resume. 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 @@ -227,3 +228,7 @@ PR #871). When approved, the run transitions through `failed` status briefly so that `findResumableRun` picks it up — this avoids duplicating resume logic. The `metadata.approval_response` field distinguishes approved-then-resumed from genuinely-failed runs. + +Interactive loop gates follow a different path: the run stays `paused`, the +approve endpoint auto-dispatches to the orchestrator, and the natural-language +resume path (`getPausedWorkflowRun`) handles the transition. 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 c4fdfc78..1141b886 100644 --- a/packages/docs-web/src/content/docs/guides/authoring-workflows.md +++ b/packages/docs-web/src/content/docs/guides/authoring-workflows.md @@ -978,7 +978,7 @@ When the workflow reaches `review-gate`, it pauses and notifies you. Approve or - **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) +- **Explicit command**: `/workflow approve ` or `/workflow reject ` (auto-resumes on Web UI; on other platforms, send a follow-up message after approving) - **Web UI**: Click the Approve/Reject buttons on the dashboard card - **API**: `POST /api/workflows/runs//approve` or `/reject` diff --git a/packages/server/src/routes/api.ts b/packages/server/src/routes/api.ts index 2bac2c1d..a5161a40 100644 --- a/packages/server/src/routes/api.ts +++ b/packages/server/src/routes/api.ts @@ -1902,9 +1902,22 @@ export function registerApiRoutes( // dispatches the resumed workflow. const parentConvDbId = run.parent_conversation_id ?? run.conversation_id; const parentConv = await conversationDb.getConversationById(parentConvDbId); - if (parentConv?.platform_conversation_id) { - void dispatchToOrchestrator(parentConv.platform_conversation_id, comment); + if (!parentConv?.platform_conversation_id) { + // Can't auto-dispatch — surface the failure so the user can resume manually. + getLog().error( + { runId, parentConvDbId, workflowName: run.workflow_name }, + 'api.workflow_run_approve_interactive_loop_no_parent_conv' + ); + return apiError( + c, + 500, + 'Workflow approved but could not auto-resume: parent conversation not found. ' + + 'Send a message to continue the workflow.' + ); } + void dispatchToOrchestrator(parentConv.platform_conversation_id, comment).catch(err => { + getLog().error({ err, runId }, 'api.workflow_run_approve_interactive_loop_dispatch_failed'); + }); return c.json({ success: true, message: `Workflow approved and resuming: ${run.workflow_name}.`, diff --git a/packages/server/src/routes/api.workflow-runs.test.ts b/packages/server/src/routes/api.workflow-runs.test.ts index 41bee850..f7ad11dc 100644 --- a/packages/server/src/routes/api.workflow-runs.test.ts +++ b/packages/server/src/routes/api.workflow-runs.test.ts @@ -1251,6 +1251,136 @@ describe('POST /api/workflows/runs/:runId/approve', () => { data: { node_output: '', approval_decision: 'approved' }, }); }); + + test('transitions standard approval run to failed status with cleared rejection metadata', async () => { + mockGetWorkflowRun.mockResolvedValueOnce(MOCK_PAUSED_RUN); + const { app } = makeApp(); + await app.request('/api/workflows/runs/run-paused-1/approve', { + method: 'POST', + body: JSON.stringify({ comment: 'LGTM' }), + headers: { 'Content-Type': 'application/json' }, + }); + expect(mockUpdateWorkflowRun).toHaveBeenCalledWith('run-paused-1', { + status: 'failed', + metadata: { approval_response: 'approved', rejection_reason: '', rejection_count: 0 }, + }); + }); +}); + +// --------------------------------------------------------------------------- +// Tests: POST /api/workflows/runs/:runId/approve — interactive_loop branch +// --------------------------------------------------------------------------- + +const MOCK_LOOP_RUN: MockWorkflowRun = { + ...MOCK_RUNNING_RUN, + id: 'run-loop-1', + status: 'paused', + conversation_id: 'worker-conv-uuid', + parent_conversation_id: 'parent-conv-uuid', + metadata: { + approval: { + type: 'interactive_loop', + nodeId: 'loop-gate', + message: 'Please provide feedback', + }, + }, +}; + +describe('POST /api/workflows/runs/:runId/approve — interactive_loop branch', () => { + beforeEach(() => { + mockGetWorkflowRun.mockReset(); + mockUpdateWorkflowRun.mockReset(); + mockCreateWorkflowEvent.mockReset(); + mockGetConversationById.mockReset(); + mockHandleMessage.mockReset(); + }); + + test('keeps run status paused and stores loop_user_input in metadata', async () => { + mockGetWorkflowRun.mockResolvedValueOnce(MOCK_LOOP_RUN); + mockGetConversationById.mockResolvedValueOnce({ + id: 'parent-conv-uuid', + platform_conversation_id: 'web-parent-abc', + }); + const { app } = makeApp(); + const response = await app.request('/api/workflows/runs/run-loop-1/approve', { + method: 'POST', + body: JSON.stringify({ comment: 'Looks great, continue' }), + headers: { 'Content-Type': 'application/json' }, + }); + expect(response.status).toBe(200); + // Must NOT call node_completed — executor writes that on actual loop exit + const nodeCompletedCall = mockCreateWorkflowEvent.mock.calls.find( + (c: unknown[]) => (c[0] as Record).event_type === 'node_completed' + ); + expect(nodeCompletedCall).toBeUndefined(); + // Status must stay paused — not transition to 'failed' + expect(mockUpdateWorkflowRun).toHaveBeenCalledWith('run-loop-1', { + metadata: { loop_user_input: 'Looks great, continue' }, + }); + const callArg = mockUpdateWorkflowRun.mock.calls[0][1] as Record; + expect(callArg).not.toHaveProperty('status'); + // Message must indicate auto-resuming + const body = (await response.json()) as { success: boolean; message: string }; + expect(body.message).toContain('resuming'); + expect(body.message).not.toContain('Send a message'); + }); + + test('dispatches to parent conversation when parent_conversation_id is set', async () => { + mockGetWorkflowRun.mockResolvedValueOnce(MOCK_LOOP_RUN); + mockGetConversationById.mockResolvedValueOnce({ + id: 'parent-conv-uuid', + platform_conversation_id: 'web-parent-abc', + }); + const { app } = makeApp(); + await app.request('/api/workflows/runs/run-loop-1/approve', { + method: 'POST', + body: JSON.stringify({ comment: 'proceed' }), + headers: { 'Content-Type': 'application/json' }, + }); + // Allow fire-and-forget microtask to flush + await new Promise(resolve => setTimeout(resolve, 0)); + expect(mockHandleMessage).toHaveBeenCalledWith( + expect.anything(), + 'web-parent-abc', + 'proceed', + expect.anything() + ); + }); + + test('falls back to conversation_id when parent_conversation_id is null', async () => { + const runNullParent = { + ...MOCK_LOOP_RUN, + parent_conversation_id: null, + conversation_id: 'worker-conv-uuid', + }; + mockGetWorkflowRun.mockResolvedValueOnce(runNullParent); + mockGetConversationById.mockResolvedValueOnce({ + id: 'worker-conv-uuid', + platform_conversation_id: 'web-worker-abc', + }); + const { app } = makeApp(); + await app.request('/api/workflows/runs/run-loop-1/approve', { + method: 'POST', + body: JSON.stringify({ comment: 'go' }), + headers: { 'Content-Type': 'application/json' }, + }); + expect(mockGetConversationById).toHaveBeenCalledWith('worker-conv-uuid'); + }); + + test('returns 500 when parent conversation cannot be resolved', async () => { + mockGetWorkflowRun.mockResolvedValueOnce(MOCK_LOOP_RUN); + mockGetConversationById.mockResolvedValueOnce(null); + const { app } = makeApp(); + const response = await app.request('/api/workflows/runs/run-loop-1/approve', { + method: 'POST', + body: JSON.stringify({ comment: 'proceed' }), + headers: { 'Content-Type': 'application/json' }, + }); + expect(response.status).toBe(500); + const body = (await response.json()) as { error: string }; + expect(body.error).toContain('could not auto-resume'); + expect(mockHandleMessage).not.toHaveBeenCalled(); + }); }); // ---------------------------------------------------------------------------