diff --git a/.archon/workflows/defaults/archon-adversarial-dev.yaml b/.archon/workflows/defaults/archon-adversarial-dev.yaml index 2ab207dc03..68722c8b1a 100644 --- a/.archon/workflows/defaults/archon-adversarial-dev.yaml +++ b/.archon/workflows/defaults/archon-adversarial-dev.yaml @@ -101,7 +101,9 @@ nodes: "status": "running" } STATEEOF - sed -i "s/SPRINT_COUNT_PLACEHOLDER/$SPRINT_COUNT/" "$ARTIFACTS/state.json" + STATE_TMP="$ARTIFACTS/state.json.tmp" + sed "s/SPRINT_COUNT_PLACEHOLDER/$SPRINT_COUNT/" "$ARTIFACTS/state.json" > "$STATE_TMP" + mv "$STATE_TMP" "$ARTIFACTS/state.json" echo "{\"totalSprints\": $SPRINT_COUNT, \"appDir\": \"$ARTIFACTS/app\", \"artifactsDir\": \"$ARTIFACTS\"}" timeout: 30000 diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a5efeb66b..6d541d13da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Bumped transitive `axios` to `^1.15.0` via root `overrides` to clear CVE-2025-62718** (NO_PROXY bypass via hostname normalization → potential SSRF). Archon pulls `axios` transitively through `@slack/bolt` and `@slack/web-api`; both semver ranges (`^1.12.0` and `^1.13.5`) accept the override cleanly, so no API surface changes. Credits @stefans71 for identifying and reporting the vulnerability in #1153. Closes #1053. +- **Stale workspace symlink no longer reported as "not in a git repository" by the CLI.** When `archon workflow run` (or `--resume`) is invoked from a valid git repo whose `~/.archon/workspaces///source` symlink points somewhere else (common after moving/renaming the checkout), auto-registration fails but the repo is fine. Previously both the worktree-creation and resume paths fell through to the generic `Cannot create worktree: not in a git repository` / `Cannot resume: Not in a git repository` errors — a lie that sent users down the wrong diagnostic path. Both sites now preserve the registration error and throw `Cannot {create worktree,resume}: repository registration failed.` with the original cause and a concrete cleanup hint (`Remove the stale workspace entry at and retry`) when the failure matches the `createProjectSourceSymlink()` shape. Credits @Bortlesboat for identifying the root cause and the parser approach in #1157. Closes #1146. - **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 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/bun.lock b/bun.lock index 8599602c73..8f1fcec74e 100644 --- a/bun.lock +++ b/bun.lock @@ -1,6 +1,5 @@ { "lockfileVersion": 1, - "configVersion": 1, "workspaces": { "": { "name": "archon", @@ -23,7 +22,7 @@ }, "packages/adapters": { "name": "@archon/adapters", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "@archon/core": "workspace:*", "@archon/git": "workspace:*", @@ -41,7 +40,7 @@ }, "packages/cli": { "name": "@archon/cli", - "version": "0.3.6", + "version": "0.4.0", "bin": { "archon": "./src/cli.ts", }, @@ -63,7 +62,7 @@ }, "packages/core": { "name": "@archon/core", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "@archon/git": "workspace:*", "@archon/isolation": "workspace:*", @@ -83,7 +82,7 @@ }, "packages/docs-web": { "name": "@archon/docs-web", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "@astrojs/starlight": "^0.38.0", "astro": "^6.1.0", @@ -92,7 +91,7 @@ }, "packages/git": { "name": "@archon/git", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "@archon/paths": "workspace:*", }, @@ -102,7 +101,7 @@ }, "packages/isolation": { "name": "@archon/isolation", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "@archon/git": "workspace:*", "@archon/paths": "workspace:*", @@ -113,7 +112,7 @@ }, "packages/paths": { "name": "@archon/paths", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "dotenv": "^17", "pino": "^9", @@ -141,7 +140,7 @@ }, "packages/server": { "name": "@archon/server", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "@archon/adapters": "workspace:*", "@archon/core": "workspace:*", @@ -160,7 +159,7 @@ }, "packages/web": { "name": "@archon/web", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "@dagrejs/dagre": "^2.0.4", "@radix-ui/react-alert-dialog": "^1.1.15", @@ -212,7 +211,7 @@ }, "packages/workflows": { "name": "@archon/workflows", - "version": "0.3.6", + "version": "0.4.0", "dependencies": { "@archon/git": "workspace:*", "@archon/paths": "workspace:*", @@ -226,6 +225,7 @@ }, }, "overrides": { + "axios": "^1.15.0", "test-exclude": "^7.0.1", }, "packages": { @@ -1043,7 +1043,7 @@ "atomic-sleep": ["atomic-sleep@1.0.0", "", {}, "sha512-kNOjDqAh7px0XWNI+4QbzoiR/nTkHAWNud2uvnJquD1/x5a7EQZMJT0AczqK0Qn67oY/TTQ1LbUKajZpp3I9tQ=="], - "axios": ["axios@1.13.6", "", { "dependencies": { "follow-redirects": "^1.15.11", "form-data": "^4.0.5", "proxy-from-env": "^1.1.0" } }, "sha512-ChTCHMouEe2kn713WHbQGcuYrr6fXTBiu460OTwWrWob16g1bXn4vtz07Ope7ewMozJAnEquLk5lWQWtBig9DQ=="], + "axios": ["axios@1.15.1", "", { "dependencies": { "follow-redirects": "^1.15.11", "form-data": "^4.0.5", "proxy-from-env": "^2.1.0" } }, "sha512-WOG+Jj8ZOvR0a3rAn+Tuf1UQJRxw5venr6DgdbJzngJE3qG7X0kL83CZGpdHMxEm+ZK3seAbvFsw4FfOfP9vxg=="], "axobject-query": ["axobject-query@4.1.0", "", {}, "sha512-qIj0G9wZbMGNLjLmg1PT6v2mE9AH2zlnADJD/2tC6E00hgmhUOfEB6greHPAfLRSufHqROIUTkw6E+M3lH0PTQ=="], @@ -2033,7 +2033,7 @@ "proxy-addr": ["proxy-addr@2.0.7", "", { "dependencies": { "forwarded": "0.2.0", "ipaddr.js": "1.9.1" } }, "sha512-llQsMLSUDUPT44jdrU/O37qlnifitDP+ZwrmmZcoSKyLKvtZxpyV0n2/bD/N4tBAAZ/gJEdZU7KMraoK1+XYAg=="], - "proxy-from-env": ["proxy-from-env@1.1.0", "", {}, "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg=="], + "proxy-from-env": ["proxy-from-env@2.1.0", "", {}, "sha512-cJ+oHTW1VAEa8cJslgmUZrc+sjRKgAKl3Zyse6+PV38hZe/V6Z14TbCuXcan9F9ghlz4QrFr2c92TNF82UkYHA=="], "pump": ["pump@3.0.4", "", { "dependencies": { "end-of-stream": "^1.1.0", "once": "^1.3.1" } }, "sha512-VS7sjc6KR7e1ukRFhQSY5LM2uBWAUPiOPa/A3mkKmiMwSmRFUITt0xuj+/lesgnCv+dPIEYlkzrcyXgquIHMcA=="], diff --git a/package.json b/package.json index d20a7583dd..2fceb51a72 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,8 @@ "bun": "^1.3.0" }, "overrides": { - "test-exclude": "^7.0.1" + "test-exclude": "^7.0.1", + "axios": "^1.15.0" }, "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.2.74" diff --git a/packages/cli/src/commands/workflow.test.ts b/packages/cli/src/commands/workflow.test.ts index d7a4030684..c6e08e8cd2 100644 --- a/packages/cli/src/commands/workflow.test.ts +++ b/packages/cli/src/commands/workflow.test.ts @@ -867,6 +867,114 @@ describe('workflowRunCommand', () => { expect(createCallsAfter).toBe(createCallsBefore); }); + // ------------------------------------------------------------------------- + // Stale workspace source-symlink → truthful CLI error + // ------------------------------------------------------------------------- + + it('surfaces auto-registration failures instead of claiming the repo is invalid', async () => { + const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery'); + const { registerRepository } = await import('@archon/core'); + const conversationDb = await import('@archon/core/db/conversations'); + const codebaseDb = await import('@archon/core/db/codebases'); + const gitModule = await import('@archon/git'); + + (discoverWorkflowsWithConfig as ReturnType).mockResolvedValueOnce({ + workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })], + errors: [], + }); + (conversationDb.getOrCreateConversation as ReturnType).mockResolvedValueOnce({ + id: 'conv-123', + }); + (codebaseDb.findCodebaseByDefaultCwd as ReturnType).mockResolvedValueOnce(null); + (gitModule.findRepoRoot as ReturnType).mockResolvedValueOnce('/test/path'); + (registerRepository as ReturnType).mockRejectedValueOnce( + new Error( + 'Source symlink at /home/test/.archon/workspaces/acme/widget/source already points to ' + + '/home/test/.archon/workspaces/widget, expected /test/path' + ) + ); + + const error = await workflowRunCommand('/test/path', 'assist', 'hello', {}).catch( + err => err as Error + ); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toContain('Cannot create worktree: repository registration failed.'); + expect(error.message).toContain( + 'Remove the stale workspace entry at /home/test/.archon/workspaces/acme/widget and retry' + ); + expect(error.message).not.toContain('not in a git repository'); + }); + + it('surfaces auto-registration failures on --resume instead of claiming the repo is invalid', async () => { + const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery'); + const { registerRepository } = await import('@archon/core'); + const conversationDb = await import('@archon/core/db/conversations'); + const codebaseDb = await import('@archon/core/db/codebases'); + const gitModule = await import('@archon/git'); + + (discoverWorkflowsWithConfig as ReturnType).mockResolvedValueOnce({ + workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })], + errors: [], + }); + (conversationDb.getOrCreateConversation as ReturnType).mockResolvedValueOnce({ + id: 'conv-123', + }); + (codebaseDb.findCodebaseByDefaultCwd as ReturnType).mockResolvedValueOnce(null); + (gitModule.findRepoRoot as ReturnType).mockResolvedValueOnce('/test/path'); + (registerRepository as ReturnType).mockRejectedValueOnce( + new Error( + 'Source symlink at /home/test/.archon/workspaces/acme/widget/source already points to ' + + '/home/test/.archon/workspaces/widget, expected /test/path' + ) + ); + + const error = await workflowRunCommand('/test/path', 'assist', 'hello', { + resume: true, + }).catch(err => err as Error); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toContain('Cannot resume: repository registration failed.'); + expect(error.message).toContain( + 'Remove the stale workspace entry at /home/test/.archon/workspaces/acme/widget and retry' + ); + expect(error.message).not.toContain('Not in a git repository'); + }); + + it('falls back to generic workspace hint when registration error has an unrecognized shape', async () => { + const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery'); + const { registerRepository } = await import('@archon/core'); + const conversationDb = await import('@archon/core/db/conversations'); + const codebaseDb = await import('@archon/core/db/codebases'); + const gitModule = await import('@archon/git'); + + (discoverWorkflowsWithConfig as ReturnType).mockResolvedValueOnce({ + workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })], + errors: [], + }); + (conversationDb.getOrCreateConversation as ReturnType).mockResolvedValueOnce({ + id: 'conv-123', + }); + (codebaseDb.findCodebaseByDefaultCwd as ReturnType).mockResolvedValueOnce(null); + (gitModule.findRepoRoot as ReturnType).mockResolvedValueOnce('/test/path'); + (registerRepository as ReturnType).mockRejectedValueOnce( + new Error("EACCES: permission denied, mkdir '/home/test/.archon/workspaces/acme'") + ); + + const error = await workflowRunCommand('/test/path', 'assist', 'hello', {}).catch( + err => err as Error + ); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toContain('Cannot create worktree: repository registration failed.'); + expect(error.message).toContain('EACCES: permission denied'); + // Path-separator-agnostic check: on Windows path.join normalizes to `\`, + // on POSIX to `/`. Assert the hint prefix + the final segment separately. + expect(error.message).toContain('Check your Archon workspace registration under'); + expect(error.message).toMatch(/workspaces\b/); + expect(error.message).not.toContain('Remove the stale workspace entry'); + }); + it('throws when isolation cannot be created due to missing codebase', async () => { const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery'); const conversationDb = await import('@archon/core/db/conversations'); @@ -2272,3 +2380,51 @@ describe('workflowRunCommand — progress rendering', () => { expect(stderrSpy).toHaveBeenCalledWith('[slow] Completed (1m30s)\n'); }); }); + +// --------------------------------------------------------------------------- +// extractStaleWorkspaceEntry — parser edge cases +// --------------------------------------------------------------------------- + +describe('extractStaleWorkspaceEntry', () => { + it('extracts the workspace dir from a POSIX source-symlink error', async () => { + const { extractStaleWorkspaceEntry } = await import('./workflow'); + expect( + extractStaleWorkspaceEntry( + 'Source symlink at /home/user/.archon/workspaces/acme/widget/source already points to /other, expected /here' + ) + ).toBe('/home/user/.archon/workspaces/acme/widget'); + }); + + it('extracts the workspace dir from a Windows source-symlink error (backslash sep)', async () => { + const { extractStaleWorkspaceEntry } = await import('./workflow'); + expect( + extractStaleWorkspaceEntry( + 'Source symlink at C:\\Users\\me\\.archon\\workspaces\\acme\\widget\\source already points to D:\\x, expected D:\\y' + ) + ).toBe('C:\\Users\\me\\.archon\\workspaces\\acme\\widget'); + }); + + it('returns null when the prefix does not match (unrelated error)', async () => { + const { extractStaleWorkspaceEntry } = await import('./workflow'); + expect(extractStaleWorkspaceEntry('ENOENT: no such file or directory')).toBeNull(); + }); + + it('returns null when the prefix matches but the delimiter is missing', async () => { + const { extractStaleWorkspaceEntry } = await import('./workflow'); + expect( + extractStaleWorkspaceEntry('Source symlink at /some/path (truncated message)') + ).toBeNull(); + }); + + it('returns null when the source path has no path separator at all', async () => { + const { extractStaleWorkspaceEntry } = await import('./workflow'); + expect( + extractStaleWorkspaceEntry('Source symlink at bareword already points to /x, expected /y') + ).toBeNull(); + }); + + it('returns null on an empty input', async () => { + const { extractStaleWorkspaceEntry } = await import('./workflow'); + expect(extractStaleWorkspaceEntry('')).toBeNull(); + }); +}); diff --git a/packages/cli/src/commands/workflow.ts b/packages/cli/src/commands/workflow.ts index 4c28edcb65..4eb90e8731 100644 --- a/packages/cli/src/commands/workflow.ts +++ b/packages/cli/src/commands/workflow.ts @@ -11,6 +11,7 @@ import { import { WORKFLOW_EVENT_TYPES, type WorkflowEventType } from '@archon/workflows/store'; import { configureIsolation, getIsolationProvider } from '@archon/isolation'; import { createLogger, getArchonHome } from '@archon/paths'; +import { join } from 'node:path'; import { createWorkflowDeps } from '@archon/core/workflows/store-adapter'; import { discoverWorkflowsWithConfig } from '@archon/workflows/workflow-discovery'; import { resolveWorkflowName } from '@archon/workflows/router'; @@ -77,6 +78,57 @@ function generateConversationId(): string { return `cli-${String(timestamp)}-${random}`; } +/** + * Parses the "Source symlink at X already points to Y, expected Z" error + * thrown by `createProjectSourceSymlink` in @archon/paths. Cross-package + * string contract — if that throw site changes wording, this parser silently + * stops matching. Returns the workspace dir (parent of the `source` link) so + * the caller can emit an exact cleanup path, or null if unrecognized. + */ +export function extractStaleWorkspaceEntry(message: string): string | null { + const prefix = 'Source symlink at '; + const delimiter = ' already points to '; + if (!message.startsWith(prefix)) return null; + + const remainder = message.slice(prefix.length); + const delimiterIndex = remainder.indexOf(delimiter); + if (delimiterIndex === -1) return null; + + const sourcePath = remainder.slice(0, delimiterIndex).trim(); + const lastSeparator = Math.max(sourcePath.lastIndexOf('/'), sourcePath.lastIndexOf('\\')); + return lastSeparator === -1 ? null : sourcePath.slice(0, lastSeparator); +} + +/** + * Wraps a codebase auto-registration failure for either the worktree-create or + * resume path. Preserves the original error message and delegates hint detail + * to `extractStaleWorkspaceEntry`; falls back to a workspace-root pointer when + * the error shape is unrecognized. + */ +function buildRegistrationFailureError(action: string, error: Error): Error { + const staleWorkspaceEntry = extractStaleWorkspaceEntry(error.message); + let hint: string; + if (staleWorkspaceEntry) { + hint = `Hint: Remove the stale workspace entry at ${staleWorkspaceEntry} and retry, or use --no-worktree to skip isolation.`; + } else { + // Guard against a throwing getArchonHome() (misconfigured env vars, etc.): + // the registration error we're wrapping is the load-bearing one — we'd + // rather lose the exact path in the hint than replace it with a secondary + // home-resolution error that masks the root cause. + try { + const workspacesPath = join(getArchonHome(), 'workspaces'); + hint = `Hint: Check your Archon workspace registration under ${workspacesPath} and retry, or use --no-worktree to skip isolation.`; + } catch { + hint = + 'Hint: Check your Archon workspace registration and retry, or use --no-worktree to skip isolation.'; + } + } + + return new Error( + `Cannot ${action}: repository registration failed.\nError: ${error.message}\n${hint}` + ); +} + /** Render a workflow event to stderr as a progress line. Called only when --quiet is not set. */ function renderWorkflowEvent(event: WorkflowEmitterEvent, verbose: boolean): void { switch (event.type) { @@ -285,6 +337,7 @@ export async function workflowRunCommand( // Try to find a codebase for this directory let codebase = null; let codebaseLookupError: Error | null = null; + let codebaseRegistrationError: Error | null = null; try { codebase = await codebaseDb.findCodebaseByDefaultCwd(cwd); } catch (error) { @@ -330,6 +383,7 @@ export async function workflowRunCommand( } } catch (error) { const err = error as Error; + codebaseRegistrationError = err; getLog().warn( { err, errorType: err.constructor.name, repoRoot }, 'cli.codebase_auto_registration_failed' @@ -354,6 +408,9 @@ export async function workflowRunCommand( 'Hint: Check your database connection before using --resume.' ); } + if (codebaseRegistrationError) { + throw buildRegistrationFailureError('resume', codebaseRegistrationError); + } throw new Error( 'Cannot resume: Not in a git repository.\n' + 'Either run from a git repo or use /clone first.' @@ -507,6 +564,9 @@ export async function workflowRunCommand( 'Hint: Check your database connection, or use --no-worktree to skip isolation.' ); } + if (codebaseRegistrationError) { + throw buildRegistrationFailureError('create worktree', codebaseRegistrationError); + } throw new Error( 'Cannot create worktree: not in a git repository.\n' + 'Run from within a git repo, or use --no-worktree to skip isolation.' diff --git a/packages/core/src/db/codebases.test.ts b/packages/core/src/db/codebases.test.ts index 26c269a085..b9bdbb6f1f 100644 --- a/packages/core/src/db/codebases.test.ts +++ b/packages/core/src/db/codebases.test.ts @@ -189,6 +189,22 @@ describe('codebases', () => { // Original frozen object should be unchanged expect(frozenCommands).not.toHaveProperty('new-command'); }); + + test('throws on corrupt JSON string (SQLite TEXT column)', async () => { + mockQuery.mockResolvedValueOnce(createQueryResult([{ commands: '{not valid json' }])); + + await expect(getCodebaseCommands('codebase-123')).rejects.toThrow( + /Corrupt commands JSON for codebase codebase-123/ + ); + }); + + test('parses valid JSON string from SQLite TEXT column', async () => { + const commands = { plan: { path: 'plan.md', description: 'Plan' } }; + mockQuery.mockResolvedValueOnce(createQueryResult([{ commands: JSON.stringify(commands) }])); + + const result = await getCodebaseCommands('codebase-123'); + expect(result).toEqual(commands); + }); }); describe('registerCommand', () => { diff --git a/packages/core/src/db/codebases.ts b/packages/core/src/db/codebases.ts index f3947fb6c1..27adc91557 100644 --- a/packages/core/src/db/codebases.ts +++ b/packages/core/src/db/codebases.ts @@ -59,9 +59,12 @@ export async function getCodebaseCommands( if (typeof raw === 'string') { try { parsed = JSON.parse(raw); - } catch { - getLog().error({ codebaseId: id, raw }, 'db.codebase_commands_json_parse_failed'); - return {}; + } catch (err) { + getLog().error({ codebaseId: id, raw, err }, 'db.codebase_commands_json_parse_failed'); + throw new Error( + `Corrupt commands JSON for codebase ${id}: unable to parse stored data. ` + + `Run UPDATE remote_agent_codebases SET commands = '{}' WHERE id = '${id}' to reset.` + ); } } else { parsed = raw ?? {}; 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 d5eb9397b3..292f0e0ad8 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -281,7 +281,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 @@ -293,7 +296,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( @@ -319,7 +325,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 f8f199a5de..bd0caf3bf8 100644 --- a/packages/core/src/orchestrator/orchestrator.test.ts +++ b/packages/core/src/orchestrator/orchestrator.test.ts @@ -1078,7 +1078,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 ); }); @@ -1103,7 +1106,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/adapters/community/discord.md b/packages/docs-web/src/content/docs/adapters/community/discord.md index 0f3e59082c..b719d719ce 100644 --- a/packages/docs-web/src/content/docs/adapters/community/discord.md +++ b/packages/docs-web/src/content/docs/adapters/community/discord.md @@ -40,6 +40,14 @@ Connect Archon to Discord so you can interact with your AI coding assistant from 2. Enable **"Message Content Intent"** (required for the bot to read messages) 3. Save changes +:::caution +Skipping this step causes Discord to reject the bot's connection with +`Used disallowed intents`. Archon will log +`discord.start_failed_continuing_without_adapter` and keep the rest of +the server running, but the Discord adapter will be unavailable until +the intent is enabled and the server is restarted. +::: + ## Invite Bot to Your Server 1. Go to "OAuth2" > "URL Generator" in the left sidebar diff --git a/packages/docs-web/src/content/docs/adapters/web.md b/packages/docs-web/src/content/docs/adapters/web.md index 7a3aeebb86..0025ca0219 100644 --- a/packages/docs-web/src/content/docs/adapters/web.md +++ b/packages/docs-web/src/content/docs/adapters/web.md @@ -172,6 +172,7 @@ The Workflow Builder at `/workflows/builder` provides a visual editor for creati - **Command picker** -- Browse available commands when configuring command nodes - **Validation panel** -- Real-time validation feedback as you build - **Undo/redo** -- Full undo/redo stack with keyboard shortcuts +- **Delete node** -- Remove a selected node with `Delete` or `Backspace`, the Delete button in the inspector header, or the right-click context menu on any node - **Save** -- Saves the workflow YAML to your project's `.archon/workflows/` directory You can also browse existing workflows on the `/workflows` page and open any of them in the builder to edit. diff --git a/packages/docs-web/src/content/docs/getting-started/overview.md b/packages/docs-web/src/content/docs/getting-started/overview.md index cee57df09d..ca3690937d 100644 --- a/packages/docs-web/src/content/docs/getting-started/overview.md +++ b/packages/docs-web/src/content/docs/getting-started/overview.md @@ -482,17 +482,19 @@ The CLI is standalone, but if you also want to interact via Telegram, Slack, Dis ## Troubleshooting -### "Cannot create worktree: not in a git repository" (but the repo exists) +### "Cannot create worktree: repository registration failed" (stale workspace symlink) -The real cause is usually a stale symlink from a previous Archon run with a different path. Look for this in the error output: +This happens when `~/.archon/workspaces///source` is a symlink pointing at a previous checkout (common after moving or renaming the repo). The error message includes the exact cleanup path to follow: ``` -Source symlink at ~/.archon/workspaces/.../source already points to , expected +Cannot create worktree: repository registration failed. +Error: Source symlink at ~/.archon/workspaces///source already points to , expected +Hint: Remove the stale workspace entry at ~/.archon/workspaces// and retry, or use --no-worktree to skip isolation. ``` -Fix it by manually deleting the stale workspace folder at `~/.archon/workspaces//` and retrying the command. +Follow the hint — delete the stale workspace folder and re-run, or pass `--no-worktree` to skip isolation for one run. -> In the future, `archon isolation cleanup` will handle this automatically. +> On Archon versions before this fix, the same root cause surfaced as the misleading "Cannot create worktree: not in a git repository" (even though the repo was valid). If you see that string, upgrade and you'll get the actionable message above. --- 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 c4fdfc7830..4fcb6d5238 100644 --- a/packages/docs-web/src/content/docs/guides/authoring-workflows.md +++ b/packages/docs-web/src/content/docs/guides/authoring-workflows.md @@ -977,12 +977,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/isolation/src/providers/worktree.ts b/packages/isolation/src/providers/worktree.ts index aad76ad6c4..9d15196f7f 100644 --- a/packages/isolation/src/providers/worktree.ts +++ b/packages/isolation/src/providers/worktree.ts @@ -49,6 +49,13 @@ function getLog(): ReturnType { return cachedLog; } +/** + * Ceiling for a single git subprocess in worktree operations (create/fetch/checkout/remove/branch-delete). + * Generous enough for repos with heavy post-checkout hooks (lint/install) while still catching genuine + * hangs (e.g. credential prompts in non-TTY, stalled network fetches). See #1119, #1029. + */ +const GIT_OPERATION_TIMEOUT_MS = 5 * 60 * 1000; + export class WorktreeProvider implements IIsolationProvider { readonly providerType = 'worktree'; @@ -150,7 +157,7 @@ export class WorktreeProvider implements IIsolationProvider { gitArgs.push(worktreePath); try { - await execFileAsync('git', gitArgs, { timeout: 30000 }); + await execFileAsync('git', gitArgs, { timeout: GIT_OPERATION_TIMEOUT_MS }); result.worktreeRemoved = true; } catch (error) { if (!this.isWorktreeMissingError(error)) { @@ -266,7 +273,9 @@ export class WorktreeProvider implements IIsolationProvider { result: DestroyResult ): Promise { try { - await execFileAsync('git', ['-C', repoPath, 'branch', '-D', branchName], { timeout: 30000 }); + await execFileAsync('git', ['-C', repoPath, 'branch', '-D', branchName], { + timeout: GIT_OPERATION_TIMEOUT_MS, + }); getLog().debug({ repoPath, branchName }, 'branch_deleted'); return true; } catch (error) { @@ -301,7 +310,7 @@ export class WorktreeProvider implements IIsolationProvider { ): Promise { try { await execFileAsync('git', ['-C', repoPath, 'push', 'origin', '--delete', branchName], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }); getLog().debug({ repoPath, branchName }, 'remote_branch_deleted'); return true; @@ -850,7 +859,7 @@ export class WorktreeProvider implements IIsolationProvider { ): Promise { // Fetch the PR's actual branch await execFileAsync('git', ['-C', repoPath, 'fetch', 'origin', prBranch], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }); // Try to create worktree with the branch @@ -859,14 +868,14 @@ export class WorktreeProvider implements IIsolationProvider { await execFileAsync( 'git', ['-C', repoPath, 'worktree', 'add', worktreePath, '-b', prBranch, `origin/${prBranch}`], - { timeout: 30000 } + { timeout: GIT_OPERATION_TIMEOUT_MS } ); } catch (error) { const err = error as Error & { stderr?: string }; // Branch already exists locally - use it directly if (err.stderr?.includes('already exists')) { await execFileAsync('git', ['-C', repoPath, 'worktree', 'add', worktreePath, prBranch], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }); } else { throw error; @@ -878,7 +887,7 @@ export class WorktreeProvider implements IIsolationProvider { await execFileAsync( 'git', ['-C', worktreePath, 'branch', '--set-upstream-to', `origin/${prBranch}`], - { timeout: 30000 } + { timeout: GIT_OPERATION_TIMEOUT_MS } ); } catch (trackingError) { getLog().warn({ err: trackingError, worktreePath, prBranch }, 'upstream_tracking_failed'); @@ -903,11 +912,11 @@ export class WorktreeProvider implements IIsolationProvider { if (prSha) { // SHA provided: create at specific commit for reproducible reviews await execFileAsync('git', ['-C', repoPath, 'fetch', 'origin', `pull/${prNumber}/head`], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }); await execFileAsync('git', ['-C', repoPath, 'worktree', 'add', worktreePath, prSha], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }); // Create a local tracking branch so it's not detached HEAD @@ -915,7 +924,7 @@ export class WorktreeProvider implements IIsolationProvider { repoPath, () => execFileAsync('git', ['-C', worktreePath, 'checkout', '-b', reviewBranch, prSha], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }), reviewBranch ); @@ -927,13 +936,13 @@ export class WorktreeProvider implements IIsolationProvider { execFileAsync( 'git', ['-C', repoPath, 'fetch', 'origin', `pull/${prNumber}/head:${reviewBranch}`], - { timeout: 30000 } + { timeout: GIT_OPERATION_TIMEOUT_MS } ), reviewBranch ); await execFileAsync('git', ['-C', repoPath, 'worktree', 'add', worktreePath, reviewBranch], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }); } } @@ -954,7 +963,7 @@ export class WorktreeProvider implements IIsolationProvider { if (err.stderr?.includes('already exists')) { getLog().debug({ repoPath, branchName }, 'stale_branch_retry'); await execFileAsync('git', ['-C', repoPath, 'branch', '-D', branchName], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }); await createCommand(); } else { @@ -988,7 +997,7 @@ export class WorktreeProvider implements IIsolationProvider { 'git', ['-C', repoPath, 'worktree', 'add', worktreePath, '-b', branchName, startPoint], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, } ); } catch (error) { @@ -1016,7 +1025,7 @@ export class WorktreeProvider implements IIsolationProvider { timeout: 10000, }); await execFileAsync('git', ['-C', repoPath, 'worktree', 'add', worktreePath, branchName], { - timeout: 30000, + timeout: GIT_OPERATION_TIMEOUT_MS, }); } else { throw error; diff --git a/packages/providers/src/claude/binary-resolver.test.ts b/packages/providers/src/claude/binary-resolver.test.ts index f87e78f36d..c5c407a531 100644 --- a/packages/providers/src/claude/binary-resolver.test.ts +++ b/packages/providers/src/claude/binary-resolver.test.ts @@ -5,6 +5,8 @@ * with BUNDLED_IS_BINARY=true, which conflicts with other test files. */ import { describe, test, expect, mock, beforeEach, afterAll, spyOn } from 'bun:test'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; import { createMockLogger } from '../test/mocks/logger'; const mockLogger = createMockLogger(); @@ -76,7 +78,55 @@ describe('resolveClaudeBinaryPath (binary mode)', () => { expect(result).toBe('/env/cli.js'); }); - test('throws with install instructions when nothing configured', async () => { + test('autodetects native installer path when env and config are unset', async () => { + // Mirror the implementation: use os.homedir() + node:path.join so the + // expected path matches the platform's actual home dir and separator. + const expected = join( + homedir(), + '.local', + 'bin', + process.platform === 'win32' ? 'claude.exe' : 'claude' + ); + // File exists only at the native-installer path. + fileExistsSpy = spyOn(resolver, 'fileExists').mockImplementation( + (path: string) => path === expected + ); + + const result = await resolver.resolveClaudeBinaryPath(); + expect(result).toBe(expected); + // Log must mark this as autodetect, not 'env' or 'config' — the source + // string is load-bearing for debug triage. + expect(mockLogger.info).toHaveBeenCalledWith( + { binaryPath: expected, source: 'autodetect' }, + 'claude.binary_resolved' + ); + }); + + test('env var takes precedence over autodetect when both would match', async () => { + process.env.CLAUDE_BIN_PATH = '/custom/env/claude'; + fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true); + + const result = await resolver.resolveClaudeBinaryPath(); + expect(result).toBe('/custom/env/claude'); + expect(mockLogger.info).toHaveBeenCalledWith( + { binaryPath: '/custom/env/claude', source: 'env' }, + 'claude.binary_resolved' + ); + }); + + test('config takes precedence over autodetect when both would match', async () => { + fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true); + + const result = await resolver.resolveClaudeBinaryPath('/custom/config/claude'); + expect(result).toBe('/custom/config/claude'); + expect(mockLogger.info).toHaveBeenCalledWith( + { binaryPath: '/custom/config/claude', source: 'config' }, + 'claude.binary_resolved' + ); + }); + + test('throws with install instructions when nothing is configured and autodetect misses', async () => { + // Every probe returns false — env unset, config unset, native path absent. fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(false); const promise = resolver.resolveClaudeBinaryPath(); diff --git a/packages/providers/src/claude/binary-resolver.ts b/packages/providers/src/claude/binary-resolver.ts index f236acb277..c2273d85d2 100644 --- a/packages/providers/src/claude/binary-resolver.ts +++ b/packages/providers/src/claude/binary-resolver.ts @@ -9,13 +9,16 @@ * Resolution order (binary mode only): * 1. `CLAUDE_BIN_PATH` environment variable * 2. `assistants.claude.claudeBinaryPath` in config - * 3. Throw with install instructions + * 3. Autodetect canonical install path (native installer default) + * 4. Throw with install instructions * * In dev mode (BUNDLED_IS_BINARY=false), returns undefined so the caller * omits `pathToClaudeCodeExecutable` entirely and the SDK resolves via its * normal node_modules lookup. */ import { existsSync as _existsSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; import { BUNDLED_IS_BINARY, createLogger } from '@archon/paths'; /** Wrapper for existsSync — enables spyOn in tests (direct imports can't be spied on). */ @@ -89,6 +92,25 @@ export async function resolveClaudeBinaryPath( return configClaudeBinaryPath; } - // 3. Not found — throw with install instructions + // 3. Autodetect — the Anthropic native installer + // (`curl -fsSL https://claude.ai/install.sh | bash` on macOS/Linux, + // `irm https://claude.ai/install.ps1 | iex` on Windows) writes the + // executable to a fixed location relative to $HOME. Users who follow + // the recommended install path don't need any env var or config entry; + // users who deviate (npm global, custom path, etc.) still set one of + // the higher-priority sources above. + const nativeInstallerPath = + process.platform === 'win32' + ? join(homedir(), '.local', 'bin', 'claude.exe') + : join(homedir(), '.local', 'bin', 'claude'); + if (fileExists(nativeInstallerPath)) { + getLog().info( + { binaryPath: nativeInstallerPath, source: 'autodetect' }, + 'claude.binary_resolved' + ); + return nativeInstallerPath; + } + + // 4. Not found — throw with install instructions throw new Error(INSTALL_INSTRUCTIONS); } diff --git a/packages/providers/src/claude/provider.ts b/packages/providers/src/claude/provider.ts index 26935bf373..7202f4e19e 100644 --- a/packages/providers/src/claude/provider.ts +++ b/packages/providers/src/claude/provider.ts @@ -381,6 +381,9 @@ async function applyNodeConfig( if (Object.keys(builtHooks).length > 0) { // Merge with existing hooks (PostToolUse capture hook) const existingHooks = options.hooks as SDKHooksMap | undefined; + if (!options.hooks) { + (options as Record).hooks = {}; + } for (const [event, matchers] of Object.entries(builtHooks)) { if (!matchers) continue; const existing = existingHooks?.[event] as HookCallbackMatcher[] | undefined; @@ -740,6 +743,7 @@ async function* streamClaudeMessages( total_cost_usd?: number; stop_reason?: string | null; num_turns?: number; + errors?: string[]; model_usage?: Record< string, { @@ -751,9 +755,15 @@ async function* streamClaudeMessages( >; }; const tokens = normalizeClaudeUsage(resultMsg.usage); + const sdkErrors = Array.isArray(resultMsg.errors) ? resultMsg.errors : undefined; if (resultMsg.is_error) { getLog().error( - { sessionId: resultMsg.session_id, errorSubtype: resultMsg.subtype }, + { + sessionId: resultMsg.session_id, + errorSubtype: resultMsg.subtype, + stopReason: resultMsg.stop_reason, + errors: sdkErrors, + }, 'claude.result_is_error' ); } @@ -765,6 +775,7 @@ async function* streamClaudeMessages( ? { structuredOutput: resultMsg.structured_output } : {}), ...(resultMsg.is_error ? { isError: true, errorSubtype: resultMsg.subtype } : {}), + ...(resultMsg.is_error && sdkErrors?.length ? { errors: sdkErrors } : {}), ...(resultMsg.total_cost_usd !== undefined ? { cost: resultMsg.total_cost_usd } : {}), ...(resultMsg.stop_reason != null ? { stopReason: resultMsg.stop_reason } : {}), ...(resultMsg.num_turns !== undefined ? { numTurns: resultMsg.num_turns } : {}), diff --git a/packages/providers/src/codex/binary-resolver.test.ts b/packages/providers/src/codex/binary-resolver.test.ts index 1df4e7c6f6..a121e4c204 100644 --- a/packages/providers/src/codex/binary-resolver.test.ts +++ b/packages/providers/src/codex/binary-resolver.test.ts @@ -87,7 +87,70 @@ describe('resolveCodexBinaryPath (binary mode)', () => { expect(normalized).toContain('/tmp/test-archon-home/vendor/codex/'); }); + test('autodetects npm global install at ~/.npm-global/bin/codex (POSIX)', async () => { + if (process.platform === 'win32') return; // POSIX-only probe + const home = process.env.HOME ?? '/Users/test'; + const expected = `${home}/.npm-global/bin/codex`; + fileExistsSpy = spyOn(resolver, 'fileExists').mockImplementation( + (path: string) => path === expected + ); + + const result = await resolver.resolveCodexBinaryPath(); + expect(result).toBe(expected); + expect(mockLogger.info).toHaveBeenCalledWith( + { binaryPath: expected, source: 'autodetect' }, + 'codex.binary_resolved' + ); + }); + + test('autodetects homebrew install on Apple Silicon', async () => { + if (process.platform !== 'darwin' || process.arch !== 'arm64') { + // `/opt/homebrew/bin/codex` is only probed on darwin-arm64; on other + // hosts this test has nothing to assert (the probe list excludes it). + return; + } + fileExistsSpy = spyOn(resolver, 'fileExists').mockImplementation( + (path: string) => path === '/opt/homebrew/bin/codex' + ); + + const result = await resolver.resolveCodexBinaryPath(); + expect(result).toBe('/opt/homebrew/bin/codex'); + expect(mockLogger.info).toHaveBeenCalledWith( + { binaryPath: '/opt/homebrew/bin/codex', source: 'autodetect' }, + 'codex.binary_resolved' + ); + }); + + test('autodetects system install at /usr/local/bin/codex', async () => { + if (process.platform === 'win32') { + // /usr/local/bin is not probed on Windows. + return; + } + fileExistsSpy = spyOn(resolver, 'fileExists').mockImplementation( + (path: string) => path === '/usr/local/bin/codex' + ); + + const result = await resolver.resolveCodexBinaryPath(); + expect(result).toBe('/usr/local/bin/codex'); + }); + + test('vendor directory takes precedence over autodetect', async () => { + // Both vendor and npm-global would match; vendor must win (lower tier #). + fileExistsSpy = spyOn(resolver, 'fileExists').mockImplementation((path: string) => { + const normalized = path.replace(/\\/g, '/'); + return normalized.includes('vendor/codex') || normalized.includes('.npm-global'); + }); + + const result = await resolver.resolveCodexBinaryPath(); + expect(result!.replace(/\\/g, '/')).toContain('/vendor/codex/'); + expect(mockLogger.info).toHaveBeenCalledWith( + expect.objectContaining({ source: 'vendor' }), + 'codex.binary_resolved' + ); + }); + test('throws with install instructions when binary not found anywhere', async () => { + // Env unset, config unset, vendor dir empty, every autodetect path missing. fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(false); await expect(resolver.resolveCodexBinaryPath()).rejects.toThrow('Codex CLI binary not found'); diff --git a/packages/providers/src/codex/binary-resolver.ts b/packages/providers/src/codex/binary-resolver.ts index a1e0f01a5b..1ac8e57cfb 100644 --- a/packages/providers/src/codex/binary-resolver.ts +++ b/packages/providers/src/codex/binary-resolver.ts @@ -9,12 +9,14 @@ * 1. `CODEX_BIN_PATH` environment variable * 2. `assistants.codex.codexBinaryPath` in config * 3. `~/.archon/vendor/codex/` (user-placed) - * 4. Throw with install instructions + * 4. Autodetect canonical install paths (npm prefix defaults per platform) + * 5. Throw with install instructions * * In dev mode (BUNDLED_IS_BINARY=false), returns undefined so the SDK * uses its normal node_modules-based resolution. */ import { existsSync as _existsSync } from 'node:fs'; +import { homedir } from 'node:os'; import { join } from 'node:path'; import { BUNDLED_IS_BINARY, getArchonHome, createLogger } from '@archon/paths'; @@ -89,7 +91,19 @@ export async function resolveCodexBinaryPath( } } - // 4. Not found — throw with install instructions + // 4. Autodetect — probe the handful of paths Codex typically lands at + // when installed via the documented package managers. Users who install + // somewhere else (custom npm prefix, etc.) still set one of the higher- + // priority sources above. Order: most specific → least specific. + const autodetectPaths = getAutodetectPaths(); + for (const probePath of autodetectPaths) { + if (fileExists(probePath)) { + getLog().info({ binaryPath: probePath, source: 'autodetect' }, 'codex.binary_resolved'); + return probePath; + } + } + + // 5. Not found — throw with install instructions const vendorPath = `~/.archon/${CODEX_VENDOR_DIR}/`; throw new Error( 'Codex CLI binary not found. The Codex provider requires a native binary\n' + @@ -105,3 +119,47 @@ export async function resolveCodexBinaryPath( ' codexBinaryPath: /path/to/codex\n' ); } + +/** + * Canonical install locations probed by tier 4 autodetect. Grounded in + * the official @openai/codex README and the npm global-install contract + * (npm writes the binary to `{npm_prefix}/bin/` on POSIX and + * `{npm_prefix}\.cmd` on Windows). The probes cover the npm prefix + * a default install lands at on each platform: + * + * - `$HOME/.npm-global/bin/codex` — common when the user ran + * `npm config set prefix ~/.npm-global` to avoid root writes + * - `/opt/homebrew/bin/codex` — mac Apple Silicon with homebrew-node + * (homebrew sets npm prefix to /opt/homebrew) + * - `/usr/local/bin/codex` — mac Intel with homebrew-node, or linux + * with system-installed node (npm prefix defaults to /usr/local) + * - `%AppData%\npm\codex.cmd` — Windows npm global default + * + * Not covered (explicit override required via CODEX_BIN_PATH or config): + * - users with other custom npm prefixes — `npm root -g` would spawn + * a subprocess per resolve, too heavy for a probe helper + * - Homebrew cask install (`brew install --cask codex`) — cask layout + * isn't a PATH binary; users should symlink or set the path + * - manual GitHub Releases extract — placement is user-determined + */ +function getAutodetectPaths(): string[] { + const paths: string[] = []; + + if (process.platform === 'win32') { + const appData = process.env.APPDATA; + if (appData) paths.push(join(appData, 'npm', 'codex.cmd')); + paths.push(join(homedir(), '.npm-global', 'codex.cmd')); + return paths; + } + + // POSIX (macOS + Linux) + paths.push(join(homedir(), '.npm-global', 'bin', 'codex')); + + if (process.platform === 'darwin' && process.arch === 'arm64') { + paths.push('/opt/homebrew/bin/codex'); + } + + paths.push('/usr/local/bin/codex'); + + return paths; +} diff --git a/packages/providers/src/types.ts b/packages/providers/src/types.ts index 330669e0c5..5fdf48de17 100644 --- a/packages/providers/src/types.ts +++ b/packages/providers/src/types.ts @@ -62,6 +62,8 @@ export type MessageChunk = structuredOutput?: unknown; isError?: boolean; errorSubtype?: string; + /** SDK-provided error detail strings. Populated when isError is true. */ + errors?: string[]; cost?: number; stopReason?: string; numTurns?: number; diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 18c173cc66..76f8d67690 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -262,6 +262,11 @@ export async function startServer(opts: ServerOptions = {}): Promise { await webAdapter.start(); persistence.startPeriodicFlush(); + // Mutable — pushed to as each adapter starts, read by the /api/health endpoint. + // Must be a live reference because Telegram starts after the HTTP listener begins + // accepting requests, so a snapshot taken at registration time would miss it. + const activePlatforms: string[] = ['Web']; + // Platform adapters (skipped in CLI serve mode or when not configured) let github: GitHubAdapter | null = null; let gitea: GiteaAdapter | null = null; @@ -294,6 +299,7 @@ export async function startServer(opts: ServerOptions = {}): Promise { botMention ); await github.start(); + activePlatforms.push('GitHub'); } else { getLog().info('github_adapter_skipped'); } @@ -310,6 +316,7 @@ export async function startServer(opts: ServerOptions = {}): Promise { giteaBotMention ); await gitea.start(); + activePlatforms.push('Gitea'); } else { getLog().info('gitea_adapter_skipped'); } @@ -326,6 +333,7 @@ export async function startServer(opts: ServerOptions = {}): Promise { gitlabBotMention ); await gitlab.start(); + activePlatforms.push('GitLab'); } else { getLog().info('gitlab_adapter_skipped'); } @@ -387,7 +395,24 @@ export async function startServer(opts: ServerOptions = {}): Promise { .catch(createMessageErrorHandler('Discord', discordAdapter, conversationId)); }); - await discord.start(); + // Don't let a Discord login failure (bad token, missing privileged + // intents, etc.) bring down the whole server — users running + // `archon serve` for the web UI shouldn't lose it because of an + // unrelated bot misconfiguration. See #1365. + try { + await discord.start(); + activePlatforms.push('Discord'); + } catch (error) { + const err = error as Error; + const isPrivilegedIntentError = err.message?.includes('disallowed intents'); + const hint = isPrivilegedIntentError + ? 'Enable "Message Content Intent" in the Discord Developer Portal ' + + '(your application > Bot > Privileged Gateway Intents) and restart, ' + + 'or unset DISCORD_BOT_TOKEN if you do not want the Discord adapter.' + : 'Verify DISCORD_BOT_TOKEN is valid, or unset it to disable the Discord adapter.'; + getLog().error({ err, hint }, 'discord.start_failed_continuing_without_adapter'); + discord = null; + } } else { getLog().info('discord_adapter_skipped'); } @@ -443,6 +468,7 @@ export async function startServer(opts: ServerOptions = {}): Promise { }); await slack.start(); + activePlatforms.push('Slack'); } else { getLog().info('slack_adapter_skipped'); } @@ -461,7 +487,7 @@ export async function startServer(opts: ServerOptions = {}): Promise { }); // Register Web UI API routes - registerApiRoutes(app, webAdapter, lockManager); + registerApiRoutes(app, webAdapter, lockManager, activePlatforms); // GitHub webhook endpoint if (github) { @@ -617,6 +643,7 @@ export async function startServer(opts: ServerOptions = {}): Promise { try { await telegramAdapter.start(); + activePlatforms.push('Telegram'); } catch (err) { const error = err instanceof Error ? err : new Error(String(err)); getLog().error({ err: error, errorType: error.constructor.name }, 'telegram.start_failed'); @@ -679,15 +706,6 @@ export async function startServer(opts: ServerOptions = {}): Promise { // the try/catch in claude.ts). These are SDK cleanup races, not fatal app errors. process.on('unhandledRejection', handleUnhandledRejection); - // Show active platforms - const activePlatforms = ['Web']; - if (telegram) activePlatforms.push('Telegram'); - if (discord) activePlatforms.push('Discord'); - if (slack) activePlatforms.push('Slack'); - if (github) activePlatforms.push('GitHub'); - if (gitea) activePlatforms.push('Gitea'); - if (gitlab) activePlatforms.push('GitLab'); - getLog().info({ activePlatforms, port }, 'server_ready'); // Non-blocking: warn at startup if gh CLI auth is unavailable diff --git a/packages/server/src/routes/api.ts b/packages/server/src/routes/api.ts index 7ac7c60474..4832e06b61 100644 --- a/packages/server/src/routes/api.ts +++ b/packages/server/src/routes/api.ts @@ -51,7 +51,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) */ @@ -821,6 +821,7 @@ const getHealthRoute = createRoute({ runningWorkflows: z.number(), version: z.string().optional(), is_docker: z.boolean(), + activePlatforms: z.array(z.string()).optional(), }) .openapi('HealthResponse'), }, @@ -868,7 +869,8 @@ const getCostAnalyticsRoute = createRoute({ export function registerApiRoutes( app: OpenAPIHono, webAdapter: WebAdapter, - lockManager: ConversationLockManager + lockManager: ConversationLockManager, + activePlatforms?: readonly string[] ): void { function apiError( c: Context, @@ -1049,6 +1051,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 { @@ -1908,9 +1999,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'); @@ -1954,9 +2056,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.`, }); } @@ -2675,6 +2786,7 @@ export function registerApiRoutes( runningWorkflows: runningWorkflowRows.length, version: appVersion, is_docker: isDocker(), + activePlatforms: activePlatforms ? [...activePlatforms] : ['Web'], }); }); 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/package.json b/packages/web/package.json index 8deb2ed573..ad976cff54 100644 --- a/packages/web/package.json +++ b/packages/web/package.json @@ -8,7 +8,7 @@ "build": "tsc --noEmit && vite build", "preview": "vite preview", "type-check": "tsc --noEmit", - "test": "bun test src/lib/ && bun test src/stores/", + "test": "bun test src/lib/ && bun test src/stores/ && bun test src/hooks/", "generate:types": "openapi-typescript http://localhost:3090/api/openapi.json -o src/lib/api.generated.d.ts" }, "dependencies": { 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 && ( +
+ +