Skip to content
Merged
243 changes: 243 additions & 0 deletions packages/cli/src/commands/workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -975,6 +975,249 @@ describe('workflowRunCommand', () => {
consoleWarnSpy.mockRestore();
}
});

it('sends dispatch message before executeWorkflow with correct metadata', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const messagesDb = await import('@archon/core/db/messages');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce(null);
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);

// Track call order for assistant messages only (user message is added first via addMessage directly)
const callOrder: string[] = [];
(messagesDb.addMessage as ReturnType<typeof mock>).mockImplementation(
async (_dbId: unknown, role: unknown, content: unknown) => {
if (role === 'assistant') {
callOrder.push(`addMessage:${String(content)}`);
}
}
);
(executeWorkflow as ReturnType<typeof mock>).mockImplementation(async () => {
callOrder.push('executeWorkflow');
return { success: true, workflowRunId: 'run-1' };
});
Comment on lines +998 to +1008
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the test file around the specified lines
cd packages/cli/src/commands
wc -l workflow.test.ts

Repository: coleam00/Archon

Length of output: 80


🏁 Script executed:

# Read the specific lines mentioned in the review
sed -n '980,1020p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 1988


🏁 Script executed:

# Look for how these mocks are initially set up - search for messagesDb and executeWorkflow mocking
rg -n "mock.module.*messagesDb|spyOn.*messagesDb" packages/cli/src/commands/workflow.test.ts | head -20

Repository: coleam00/Archon

Length of output: 41


🏁 Script executed:

# Check for executeWorkflow mock setup
rg -n "mock.module.*executeWorkflow|spyOn.*executeWorkflow" packages/cli/src/commands/workflow.test.ts | head -20

Repository: coleam00/Archon

Length of output: 41


🏁 Script executed:

# Look at the broader context of the test to understand test structure and reset patterns
sed -n '950,1050p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 4585


🏁 Script executed:

# Look at the top of the test file to see how mocking is set up
head -100 packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 3235


🏁 Script executed:

# Search for mock.module() calls in the file
rg -n "mock\.module\(" packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 746


🏁 Script executed:

# Look for beforeEach/afterEach patterns
rg -n "beforeEach|afterEach" packages/cli/src/commands/workflow.test.ts | head -10

Repository: coleam00/Archon

Length of output: 371


🏁 Script executed:

# Look at the beforeEach/afterEach patterns
sed -n '148,160p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 503


🏁 Script executed:

# Also check other beforeEach/afterEach blocks
sed -n '344,355p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 441


🏁 Script executed:

# Look at the full mock.module for messages to understand the initial state
sed -n '124,128p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 208


🏁 Script executed:

# Check more context around lines 980-1050 to see all tests in this block
sed -n '920,1120p' packages/cli/src/commands/workflow.test.ts | head -150

Repository: coleam00/Archon

Length of output: 6924


🏁 Script executed:

# Check if there's any reset/clearing of these mocks between tests
sed -n '1060,1110p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 2506


🏁 Script executed:

# Look for any patterns where tests depend on mock state
rg -n "mockClear|mockRestore" packages/cli/src/commands/workflow.test.ts | grep -A 2 -B 2 "messagesDb\|executeWorkflow"

Repository: coleam00/Archon

Length of output: 354


🏁 Script executed:

# Look at the full test structure around line 998 to see the complete test case
sed -n '987,1045p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 2780


🏁 Script executed:

# Check if the next test properly resets or has its own implementations
sed -n '1045,1120p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 3524


🏁 Script executed:

# Verify exactly what mockClear does vs what's needed - check if there's any reset of executeWorkflow in test 2
sed -n '1049,1075p' packages/cli/src/commands/workflow.test.ts

Repository: coleam00/Archon

Length of output: 1279


Use mockImplementationOnce() to prevent test isolation violations.

The mockImplementation() calls at lines 998–1008 persist beyond this test case. Since mockClear() in subsequent tests only clears call history but preserves the implementation, later tests inherit these persistent behaviors and become order-dependent. Replace both with mockImplementationOnce() to ensure each mock applies only to the next call.

Suggested change
-    (messagesDb.addMessage as ReturnType<typeof mock>).mockImplementation(
+    (messagesDb.addMessage as ReturnType<typeof mock>).mockImplementationOnce(
       async (_dbId: unknown, role: unknown, content: unknown) => {
         if (role === 'assistant') {
           callOrder.push(`addMessage:${String(content)}`);
         }
       }
     );
-    (executeWorkflow as ReturnType<typeof mock>).mockImplementation(async () => {
+    (executeWorkflow as ReturnType<typeof mock>).mockImplementationOnce(async () => {
       callOrder.push('executeWorkflow');
       return { success: true, workflowRunId: 'run-1' };
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(messagesDb.addMessage as ReturnType<typeof mock>).mockImplementation(
async (_dbId: unknown, role: unknown, content: unknown) => {
if (role === 'assistant') {
callOrder.push(`addMessage:${String(content)}`);
}
}
);
(executeWorkflow as ReturnType<typeof mock>).mockImplementation(async () => {
callOrder.push('executeWorkflow');
return { success: true, workflowRunId: 'run-1' };
});
(messagesDb.addMessage as ReturnType<typeof mock>).mockImplementationOnce(
async (_dbId: unknown, role: unknown, content: unknown) => {
if (role === 'assistant') {
callOrder.push(`addMessage:${String(content)}`);
}
}
);
(executeWorkflow as ReturnType<typeof mock>).mockImplementationOnce(async () => {
callOrder.push('executeWorkflow');
return { success: true, workflowRunId: 'run-1' };
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/workflow.test.ts` around lines 998 - 1008, The test
sets persistent mock implementations on messagesDb.addMessage and
executeWorkflow using mockImplementation(), causing subsequent tests to inherit
behavior; change both to use mockImplementationOnce() so the custom behaviors
apply only to the next call (i.e., replace the mockImplementation(...) on
messagesDb.addMessage and the one on executeWorkflow with
mockImplementationOnce(...) to avoid cross-test pollution).


await workflowRunCommand('/test/path', 'assist', 'hello', { noWorktree: true });

// Dispatch assistant message fires before executeWorkflow
expect(callOrder[0]).toContain('Dispatching workflow');
expect(callOrder[1]).toBe('executeWorkflow');

// Correct metadata shape
expect(messagesDb.addMessage).toHaveBeenCalledWith(
expect.any(String),
'assistant',
'Dispatching workflow: **assist**',
expect.objectContaining({
category: 'workflow_dispatch_status',
workflowDispatch: expect.objectContaining({
workflowName: 'assist',
workerConversationId: expect.stringMatching(/^cli-/),
}),
})
);
});

it('sends result card when executeWorkflow returns a summary', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const messagesDb = await import('@archon/core/db/messages');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce(null);
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-42',
summary: 'All steps completed. Branch pushed.',
});
(messagesDb.addMessage as ReturnType<typeof mock>).mockClear();

await workflowRunCommand('/test/path', 'assist', 'hello', { noWorktree: true });

expect(messagesDb.addMessage).toHaveBeenCalledWith(
expect.any(String),
'assistant',
'All steps completed. Branch pushed.',
expect.objectContaining({
category: 'workflow_result',
workflowResult: { workflowName: 'assist', runId: 'run-42' },
})
);
});

it('does not send result card when executeWorkflow has no summary', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const messagesDb = await import('@archon/core/db/messages');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce(null);
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-1',
// no summary field
});
(messagesDb.addMessage as ReturnType<typeof mock>).mockClear();

await workflowRunCommand('/test/path', 'assist', 'hello', { noWorktree: true });

// Only dispatch addMessage call, no result card
const resultCalls = (messagesDb.addMessage as ReturnType<typeof mock>).mock.calls.filter(
(args: unknown[]) => {
const meta = args[3] as Record<string, unknown> | undefined;
return meta?.category === 'workflow_result';
}
);
expect(resultCalls).toHaveLength(0);
});

it('does not throw and logs warn when result message DB persist fails', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const messagesDb = await import('@archon/core/db/messages');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce(null);
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-1',
summary: 'Done.',
});
// addMessage is called three times: user message persist, dispatch, result
// CLIAdapter internally catches DB errors — it logs 'cli_message_persist_failed' and does not throw.
// Verify workflowRunCommand does not throw even when the result DB write fails.
(messagesDb.addMessage as ReturnType<typeof mock>)
.mockResolvedValueOnce(undefined) // user message persist succeeds
.mockResolvedValueOnce(undefined) // dispatch succeeds
.mockRejectedValueOnce(new Error('DB gone')); // result fails (caught inside CLIAdapter)

// Should not throw — the CLIAdapter swallows the DB error and logs a warn
await expect(
workflowRunCommand('/test/path', 'assist', 'hello', { noWorktree: true })
).resolves.toBeUndefined();

// CLIAdapter logs 'cli_message_persist_failed' when addMessage throws internally
expect(mockLogger.warn).toHaveBeenCalledWith(
expect.objectContaining({ err: expect.any(Error) }),
'cli_message_persist_failed'
);
});

it('does not throw and continues to executeWorkflow when dispatch sendMessage fails', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const messagesDb = await import('@archon/core/db/messages');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce(null);
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockClear();
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-1',
});
// First addMessage (user message persist) succeeds, second (dispatch) fails
(messagesDb.addMessage as ReturnType<typeof mock>)
.mockResolvedValueOnce(undefined) // user message persist succeeds
.mockRejectedValueOnce(new Error('DB gone')); // dispatch fails (caught inside CLIAdapter)

// Should not throw — dispatch failure must not block workflow execution
await expect(
workflowRunCommand('/test/path', 'assist', 'hello', { noWorktree: true })
).resolves.toBeUndefined();

// executeWorkflow was still called despite dispatch failure
expect(executeWorkflow).toHaveBeenCalledTimes(1);
});

it('does not send result card when workflow is paused even with summary', async () => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
const { executeWorkflow } = await import('@archon/workflows/executor');
const conversationDb = await import('@archon/core/db/conversations');
const codebaseDb = await import('@archon/core/db/codebases');
const messagesDb = await import('@archon/core/db/messages');

(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist', description: 'Help' })],
errors: [],
});
(conversationDb.getOrCreateConversation as ReturnType<typeof mock>).mockResolvedValueOnce({
id: 'conv-123',
});
(codebaseDb.findCodebaseByDefaultCwd as ReturnType<typeof mock>).mockResolvedValueOnce(null);
(conversationDb.updateConversation as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);
(executeWorkflow as ReturnType<typeof mock>).mockResolvedValueOnce({
success: true,
workflowRunId: 'run-paused',
paused: true,
summary: 'Steps completed so far.',
});
(messagesDb.addMessage as ReturnType<typeof mock>).mockClear();

const consoleSpy = spyOn(console, 'log').mockImplementation(() => {});
try {
await workflowRunCommand('/test/path', 'assist', 'hello', { noWorktree: true });

// Paused guard fires before summary check — no result card despite having a summary
const resultCalls = (messagesDb.addMessage as ReturnType<typeof mock>).mock.calls.filter(
(args: unknown[]) => {
const meta = args[3] as Record<string, unknown> | undefined;
return meta?.category === 'workflow_result';
}
);
expect(resultCalls).toHaveLength(0);

// Confirm paused message was printed
expect(consoleSpy).toHaveBeenCalledWith('\nWorkflow paused — waiting for approval.');
} finally {
consoleSpy.mockRestore();
}
});
});

describe('workflowStatusCommand', () => {
Expand Down
68 changes: 49 additions & 19 deletions packages/cli/src/commands/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export async function workflowListCommand(cwd: string, json?: boolean): Promise<
}

if (workflowEntries.length > 0) {
console.log(`\nFound ${String(workflowEntries.length)} workflow(s):\n`);
console.log(`\nFound ${workflowEntries.length} workflow(s):\n`);

for (const { workflow } of workflowEntries) {
console.log(` ${workflow.name}`);
Expand All @@ -193,7 +193,7 @@ export async function workflowListCommand(cwd: string, json?: boolean): Promise<
}

if (errors.length > 0) {
console.log(`\n${String(errors.length)} workflow(s) failed to load:\n`);
console.log(`\n${errors.length} workflow(s) failed to load:\n`);
for (const e of errors) {
console.log(` ${e.filename}: ${e.error}`);
}
Expand Down Expand Up @@ -591,6 +591,24 @@ export async function workflowRunCommand(
renderWorkflowEvent(event, verbose ?? false);
});

// Notify Web UI that a workflow is dispatching.
// Mirrors the orchestrator dispatch message structure (category/segment/workflowDispatch),
// but omits the rocket emoji and "(background)" qualifier since the CLI runs synchronously.
// In the CLI path there is no separate worker conversation — the CLI itself
// is both the dispatcher and the executor, so workerConversationId === conversationId.
try {
await adapter.sendMessage(conversationId, `Dispatching workflow: **${workflow.name}**`, {
category: 'workflow_dispatch_status',
segment: 'new',
workflowDispatch: { workerConversationId: conversationId, workflowName: workflow.name },
});
} catch (dispatchError) {
getLog().warn(
{ err: dispatchError as Error, conversationId },
'cli.workflow_dispatch_surface_failed'
);
}

// Execute workflow with workingCwd (may be worktree path)
let result: Awaited<ReturnType<typeof executeWorkflow>>;
try {
Expand All @@ -612,6 +630,22 @@ export async function workflowRunCommand(
if (result.success && 'paused' in result && result.paused) {
console.log('\nWorkflow paused — waiting for approval.');
} else if (result.success) {
// Surface workflow result to Web UI as a result card (mirrors orchestrator.ts result message).
// Paused workflows are handled in the branch above and intentionally do not get a result card.
if ('summary' in result && result.summary) {
try {
await adapter.sendMessage(conversationId, result.summary, {
category: 'workflow_result',
segment: 'new',
workflowResult: { workflowName: workflow.name, runId: result.workflowRunId },
});
} catch (surfaceError) {
getLog().warn(
{ err: surfaceError as Error, conversationId },
'cli.workflow_result_surface_failed'
);
}
}
console.log('\nWorkflow completed successfully.');
} else {
throw new Error(`Workflow failed: ${result.error}`);
Expand All @@ -630,25 +664,25 @@ function formatAge(startedAt: Date | string): string {
if (Number.isNaN(date.getTime())) return 'unknown';
const ms = Date.now() - date.getTime();
const secs = Math.floor(ms / 1000);
if (secs < 60) return `${String(secs)}s`;
if (secs < 60) return `${secs}s`;
const mins = Math.floor(secs / 60);
if (mins < 60) return `${String(mins)}m`;
if (mins < 60) return `${mins}m`;
const hours = Math.floor(mins / 60);
if (hours < 24) return `${String(hours)}h ${String(mins % 60)}m`;
if (hours < 24) return `${hours}h ${mins % 60}m`;
const days = Math.floor(hours / 24);
return `${String(days)}d ${String(hours % 24)}h`;
return `${days}d ${hours % 24}h`;
}

/**
* Format a duration in milliseconds as a compact string.
*/
function formatDuration(ms: number): string {
if (ms < 1000) return `${String(ms)}ms`;
if (ms < 1000) return `${ms}ms`;
const secs = Math.round(ms / 100) / 10;
if (secs < 60) return `${String(secs)}s`;
if (secs < 60) return `${secs}s`;
const mins = Math.floor(secs / 60);
const remSecs = Math.round(secs % 60);
return `${String(mins)}m${String(remSecs)}s`;
return `${mins}m${remSecs}s`;
}

interface NodeSummary {
Expand Down Expand Up @@ -732,20 +766,16 @@ export async function workflowStatusCommand(json?: boolean, verbose?: boolean):
}

if (json) {
let runsOutput: unknown[] = runs;
if (verbose) {
const eventsPerRun = await Promise.all(
runs.map(run =>
workflowEventsDb.listWorkflowEvents(run.id).catch(() => [] as WorkflowEventRow[])
)
);
const runsWithEvents = runs.map((run, i) => ({
...run,
events: eventsPerRun[i],
}));
console.log(JSON.stringify({ runs: runsWithEvents }, null, 2));
} else {
console.log(JSON.stringify({ runs }, null, 2));
runsOutput = runs.map((run, i) => ({ ...run, events: eventsPerRun[i] }));
}
console.log(JSON.stringify({ runs: runsOutput }, null, 2));
return;
}

Expand All @@ -754,7 +784,7 @@ export async function workflowStatusCommand(json?: boolean, verbose?: boolean):
return;
}

console.log(`\nActive workflows (${String(runs.length)}):\n`);
console.log(`\nActive workflows (${runs.length}):\n`);
for (const run of runs) {
const age = formatAge(run.started_at);
console.log(` ID: ${run.id}`);
Expand Down Expand Up @@ -968,9 +998,9 @@ export async function workflowCleanupCommand(days: number): Promise<void> {
try {
const { count } = await workflowDb.deleteOldWorkflowRuns(days);
if (count === 0) {
console.log(`No workflow runs older than ${String(days)} days to clean up.`);
console.log(`No workflow runs older than ${days} days to clean up.`);
} else {
console.log(`Deleted ${String(count)} workflow run(s) older than ${String(days)} days.`);
console.log(`Deleted ${count} workflow run(s) older than ${days} days.`);
}
} catch (error) {
const err = error as Error;
Expand Down
Loading
Loading