Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions packages/workflows/src/dag-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4674,6 +4674,76 @@ describe('executeDagWorkflow -- approval node', () => {
expect(pauseCalls.length).toBe(1);
});

it('on_reject does not write node_completed for the approval gate node ID', async () => {
mockSendQueryDag.mockImplementation(function* () {
yield { type: 'assistant', content: 'Fixed based on feedback' };
yield { type: 'result', sessionId: 'reject-no-poison-session' };
});

const store = createMockStore();
const mockDeps = createMockDeps(store);
const platform = createMockPlatform();

const workflowRun = makeWorkflowRun('reject-no-poison-run', {
metadata: {
approval: {
type: 'approval',
nodeId: 'review',
message: 'Approve this plan?',
onRejectPrompt: 'Fix based on: $REJECTION_REASON',
onRejectMaxAttempts: 3,
},
rejection_reason: 'Missing edge case handling',
rejection_count: 1,
},
});

await executeDagWorkflow(
mockDeps,
platform,
'conv-approval',
testDir,
{
name: 'approval-no-poison',
nodes: [
{
id: 'review',
approval: {
message: 'Approve this plan?',
on_reject: { prompt: 'Fix based on: $REJECTION_REASON', max_attempts: 3 },
},
},
],
},
workflowRun,
'claude',
undefined,
join(testDir, 'artifacts'),
join(testDir, 'logs'),
'main',
'docs/',
minimalConfig
);

// The on_reject synthetic node must NOT produce a node_completed event with
// step_name equal to the approval gate's own ID ('review'). If it did, a
// subsequent resume would find the event via getCompletedDagNodeOutputs and
// skip the approval gate entirely, bypassing the human gate.
const eventCalls = (store.createWorkflowEvent as ReturnType<typeof mock>).mock.calls;
const nodeCompletedEvents = eventCalls.filter(
(call: unknown[]) => (call[0] as Record<string, unknown>).event_type === 'node_completed'
);
const completedStepNames = nodeCompletedEvents.map(
(call: unknown[]) => (call[0] as Record<string, unknown>).step_name
);
expect(completedStepNames).not.toContain('review');

// The synthetic on_reject node MUST produce a node_completed event with the
// distinct ID 'review:on_reject'. This ensures the synthetic node itself is
// recorded as completed so it is not re-run on a subsequent resume.
expect(completedStepNames.filter((n: unknown) => n === 'review:on_reject').length).toBe(1);
});

it('on_reject cancels when max_attempts exhausted', async () => {
const store = createMockStore();
const mockDeps = createMockDeps(store);
Expand Down
16 changes: 14 additions & 2 deletions packages/workflows/src/dag-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2249,9 +2249,21 @@ async function executeApprovalNode(
rejectionReason
);

// Build a synthetic PromptNode to reuse executeNodeInternal
// Build a synthetic PromptNode to reuse executeNodeInternal.
// Use a distinct ID so the node_completed event written by executeNodeInternal
// does not collide with the approval gate's own ID in getCompletedDagNodeOutputs.
// If we used node.id here, a resumed run would find the event and treat the
// approval gate as already completed, bypassing the human gate entirely.
//
// Note: executeNodeInternal also emits node_started/node_completed WorkflowEmitterEvents
// with nodeId = `${node.id}:on_reject`. These flow through SSE into the web UI, where
// WorkflowExecution.tsx builds its nodeMap from all node_* events unconditionally.
// This means a transient `${node.id}:on_reject` phantom entry may appear in the UI's
// execution view during an on_reject cycle. This is cosmetic-only — the approval gate
// still re-presents correctly and the human gate contract is preserved. A follow-up can
// filter synthetic `:on_reject` IDs from the UI's nodeMap if needed.
const syntheticNode: PromptNode = {
id: node.id,
id: `${node.id}:on_reject`,
prompt: substituteNodeOutputRefs(substitutedPrompt, nodeOutputs),
...(node.depends_on ? { depends_on: node.depends_on } : {}),
...(node.idle_timeout ? { idle_timeout: node.idle_timeout } : {}),
Expand Down
Loading