Skip to content

fix(workflows): restore interactive-loop cross-gate session continuity (reverts #1923) #2046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Wirasm merged 1 commit into from
Jun 27, 2026
Merged

fix(workflows): restore interactive-loop cross-gate session continuity (reverts #1923) #2046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 1 addition & 4 deletions packages/docs-web/src/content/docs/guides/loop-nodes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,7 @@ Controls session continuity between iterations:
| `true` | Each iteration starts a fresh AI session. No memory of prior iterations. | Work state lives on disk (files, git). Prevents context window exhaustion on long loops. |
| `false` (default) | Sessions thread — each iteration resumes the prior conversation. | Iterative refinement where the agent needs to remember what it tried before. |

The first iteration is always fresh regardless of this setting. The first
iteration executed after resuming from an interactive loop approval gate is
also always fresh — the stored session id may have expired during the human
review wait, and user feedback is carried via `$LOOP_USER_INPUT` instead.
The first iteration is always fresh regardless of this setting.

@coderabbitai coderabbitai Bot Jun 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify that this means the loop’s initial iteration.

“First iteration” is a bit ambiguous now that resumed interactive loops reuse the stored session. Calling out the initial loop iteration (or i === 1) would avoid readers interpreting this as “first iteration after resume.”

Suggested wording 
-The first iteration is always fresh regardless of this setting.
+The loop's initial iteration (`i === 1`) is always fresh regardless of this setting.
📝 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
The first iteration is always fresh regardless of this setting.
The loop's initial iteration (`i === 1`) is always fresh regardless of this setting.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/docs-web/src/content/docs/guides/loop-nodes.md` at line 135, Clarify
the wording in the loop-nodes guide so “first iteration” is explicitly the
loop’s initial iteration, not the first run after a resume. Update the relevant
sentence in the docs content to reference the initial loop iteration (for
example, by naming the first pass or `i === 1`) so readers don’t misinterpret
the behavior of resumed interactive loops.


### `until_bash`

Expand Down
83 changes: 2 additions & 81 deletions packages/workflows/src/dag-executor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4997,88 +4997,9 @@ describe('executeDagWorkflow -- resume with priorCompletedNodes', () => {
// Verify the prompt contains the user input
const promptArg = mockSendQueryDag.mock.calls[0][0] as string;
expect(promptArg).toContain('Add error handling');
// Should use a fresh session on the first resumed iteration: the stored
// gate session may have expired during the human review wait, so we
// start fresh (user feedback is carried via $LOOP_USER_INPUT). See
// packages/workflows/src/dag-executor.ts needsFreshSession (#1208).
// Should have resumed with stored session ID
const sessionArg = mockSendQueryDag.mock.calls[0][2] as string | undefined;
expect(sessionArg).toBeUndefined();
});

it('interactive loop resume does not crash with error_during_execution on iteration 2', async () => {
// Regression test for #1208: when resuming a paused interactive loop
// gate, the first resumed iteration must use a fresh session (not the
// potentially stale stored session id), so the SDK never receives an
// expired session id that would trigger error_during_execution.
mockSendQueryDag.mockImplementation(function* () {
yield {
type: 'assistant',
content: 'Updated plan with error handling. <promise>APPROVED</promise>',
};
yield { type: 'result', sessionId: 'fresh-session-1' };
});

const store = createMockStore();
const mockDeps = createMockDeps(store);
const platform = createMockPlatform();
// Simulate resumed run: metadata has loop gate state from iteration 1
// with a stale session id (the SDK would reject this if reused).
const workflowRun = makeWorkflowRun('resume-no-crash-id', {
metadata: {
approval: {
type: 'interactive_loop',
nodeId: 'refine',
iteration: 1,
sessionId: 'stale-session-from-hours-ago',
message: 'Review the plan.',
},
loop_user_input: 'Add error handling',
},
});

await executeDagWorkflow(
mockDeps,
platform,
'conv-dag',
testDir,
{
name: 'interactive-loop-no-crash',
nodes: [
{
id: 'refine',
loop: {
prompt: 'User said: $LOOP_USER_INPUT. Refine the plan.',
until: 'APPROVED',
max_iterations: 10,
interactive: true,
gate_message: 'Review the plan.',
},
},
],
},
workflowRun,
'claude',
undefined,
join(testDir, 'artifacts'),
join(testDir, 'logs'),
'main',
'docs/',
minimalConfig
);

// Exactly one iteration runs (it completes via APPROVED signal)
expect(mockSendQueryDag.mock.calls.length).toBe(1);
// Crucially: sessionArg must be undefined (fresh session), NOT the stale stored id.
const sessionArg = mockSendQueryDag.mock.calls[0][2] as string | undefined;
expect(sessionArg).toBeUndefined();

// No failure events were emitted (no loop_iteration_failed, no node_failed).
const eventCalls = (store.createWorkflowEvent as ReturnType<typeof mock>).mock.calls;
const failedEvents = eventCalls.filter((call: unknown[]) => {
const evt = (call[0] as Record<string, unknown>).event_type as string;
return evt === 'loop_iteration_failed' || evt === 'node_failed';
});
expect(failedEvents).toHaveLength(0);
expect(sessionArg).toBe('loop-session-1');
});

it('loop iteration fails loudly when SDK returns error_during_execution', async () => {
Expand Down
10 changes: 2 additions & 8 deletions packages/workflows/src/dag-executor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2305,14 +2305,8 @@ async function executeLoopNode(
logEventStoreError(err, i);
});

// Session threading. Force a fresh session on the first iteration of an
// interactive loop resume: the stored session id from the gate metadata
// may be stale after a human review wait — passing it to the SDK can
// trigger errors (e.g. error_during_execution on Claude SDK).
// User feedback is carried via $LOOP_USER_INPUT, so session continuity is
// not required for the first resumed iteration.
const needsFreshSession =
loop.fresh_context || i === 1 || (isLoopResume && i === startIteration);
// Session threading
const needsFreshSession = loop.fresh_context || i === 1;
const resumeSessionId = needsFreshSession ? undefined : currentSessionId;

// Stream AI response for this iteration
Expand Down
Loading