diff --git a/CHANGELOG.md b/CHANGELOG.md index 34e4d9ff..00347f57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **CLI and server no longer silently lose repo-local env vars.** Previously, env vars in `/.env` were parsed, deleted from `process.env` by `stripCwdEnv()`, and the only output operators saw was `[dotenv@17.3.1] injecting env (0) from .env` — which read as "file was empty." Workflows that needed `SLACK_WEBHOOK` or similar had no way to recover without knowing to use `~/.archon/.env`. The new `/.archon/.env` path + archon-owned log lines make the load state observable and recoverable. (#1302) - **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. +- **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 foreground/interactive web dispatch, (2) the `POST /approve` and `POST /reject` API handlers dispatch `/workflow run ` back through the orchestrator when `parent_conversation_id` is set — `findResumableRunByParentConversation` then picks up the paused run and resumes it (mirrors `workflowApproveCommand`/`workflowRejectCommand` on the CLI), and (3) the during-streaming status check in the DAG executor now tolerates the `paused` state so a concurrent AI node finishes its own stream rather than being aborted when a sibling approval node pauses the run. The Web UI reject button now also 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.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index ba24331b..27b99648 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 b46523b1..58e5ac30 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/server/src/routes/api.ts b/packages/server/src/routes/api.ts index 8adf5e83..2891a4cc 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,43 @@ 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 type a follow-up message. 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 + * (missing parent conversation, parent conversation not found, or dispatch + * threw). Failures are non-fatal: the gate was still recorded; the user can + * resume manually by sending any message in the conversation. + */ + async function tryAutoResumeAfterGate(run: WorkflowRun, logPrefix: string): Promise { + if (!run.parent_conversation_id) return false; + try { + const parentConv = await conversationDb.getConversationById(run.parent_conversation_id); + const platformConvId = parentConv?.platform_conversation_id; + if (!platformConvId) { + getLog().debug( + { runId: run.id, parentConversationId: run.parent_conversation_id }, + `${logPrefix}.skipped_no_platform_conv` + ); + return false; + } + const resumeMessage = `/workflow run ${run.workflow_name} ${run.user_message ?? ''}`.trim(); + void dispatchToOrchestrator(platformConvId, resumeMessage); + getLog().info( + { runId: run.id, workflowName: run.workflow_name, platformConvId }, + `${logPrefix}.dispatched` + ); + return true; + } catch (err) { + getLog().warn({ err: err as Error, runId: run.id }, `${logPrefix}.failed`); + return false; + } + } + // GET /api/conversations - List conversations registerOpenApiRoute(getConversationsRoute, async c => { try { @@ -1894,9 +1931,19 @@ export function registerApiRoutes( status: 'failed', metadata: metadataUpdate, }); + + // Auto-resume: dispatch to the orchestrator so the workflow continues + // without requiring the user to type a follow-up message. Mirrors what + // `workflowApproveCommand` does in the CLI. Requires + // `parent_conversation_id` to be set on the run — which orchestrator-agent + // now passes for every web-dispatched foreground/interactive workflow. + const autoResumed = await tryAutoResumeAfterGate(run, 'api.workflow_approve_auto_resume'); + 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 +1987,17 @@ 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 type a follow-up message. Mirrors what + // `workflowRejectCommand` does in the CLI. + const autoResumed = await tryAutoResumeAfterGate(run, 'api.workflow_reject_auto_resume'); + 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 41bee850..76e001e8 100644 --- a/packages/server/src/routes/api.workflow-runs.test.ts +++ b/packages/server/src/routes/api.workflow-runs.test.ts @@ -1362,3 +1362,155 @@ 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', + }); + + 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('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', + }); + + 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 bb65471f..44eb70af 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 2292aef3..f2a2f1e8 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 { 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; + /** + * 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. 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; + 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,19 @@ export function ConfirmRunActionDialog({ title, description, confirmLabel, + reasonInput, onConfirm, }: Props): React.ReactElement { + const [reason, setReason] = useState(''); + 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 +85,26 @@ export function ConfirmRunActionDialog({

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