Skip to content

fix: treat force-released agent as step failure, not success #526

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
khaliqgant merged 3 commits into from
Mar 10, 2026
Merged

fix: treat force-released agent as step failure, not success #526

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79f417e
fix: treat force-released agent as step failure, not success
khaliqgant Mar 10, 2026
b9c9403
fix: distinguish force-released (nudge exhaustion) from released (idl…
khaliqgant Mar 10, 2026
f458d78
fix: use timeoutMs directly in nudge loop timeout guard
khaliqgant Mar 10, 2026
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
14 changes: 10 additions & 4 deletions packages/sdk/src/workflows/runner.ts
View file
Open in desktop
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -3448,6 +3448,12 @@ export class WorkflowRunner {
throw new Error(`Step "${step.name}" timed out after ${timeoutMs ?? 'unknown'}ms`);
}
}

if (exitResult === 'force-released') {
throw new Error(
`Step "${step.name}" failed — agent was force-released after exhausting idle nudges without completing`
);
}
Comment on lines +3452 to +3456

@devin-ai-integration devin-ai-integration Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Throwing on force-released breaks existing tests that expect step completion after nudge exhaustion

The new throw at lines 3452-3456 causes steps to fail when agents are force-released after exhausting idle nudges. However, three existing tests (force-releases after maxNudges is exceeded at packages/sdk/src/__tests__/idle-nudge.test.ts:234, force-releases after multiple nudges at line 253, and uses defaults when idleNudge is empty object at line 313) assert run.status === 'completed' for exactly this scenario. Since waitForExitWithIdleNudging now returns 'force-released' instead of 'released', the throw propagates up through spawnAndWait → executeAgentStep (caught at runner.ts:2431), and with maxRetries defaulting to 0 in those test configs, the step is immediately marked as failed, resulting in run.status === 'failed'. The tests were not updated to match the new intended semantics.

Prompt for agents 
The behavioral change from 'released' to 'force-released' (with an error throw) is intentional per the commit message, but the tests in packages/sdk/src/__tests__/idle-nudge.test.ts were not updated. Update the following tests to expect the new failure behavior:

1. Line 221-238 ('force-releases after maxNudges is exceeded'): Change expect(run.status).toBe('completed') to expect(run.status).toBe('failed') and add an assertion that run.error contains 'force-released'.

2. Line 240-256 ('force-releases after multiple nudges'): Change expect(run.status).toBe('completed') to expect(run.status).toBe('failed') and add an assertion that run.error contains 'force-released'.

3. Line 300-317 ('uses defaults when idleNudge is empty object'): Change expect(run.status).toBe('completed') to expect(run.status).toBe('failed') and add an assertion that run.error contains 'force-released'.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@khaliqgant khaliqgant Mar 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Addressed in b9c9403 — the released vs force-released distinction means existing tests that expect step completion after idle release still pass (exitResult === 'released' is SUCCESS). Only nudge-exhausted force-releases throw. Real e2e confirmed: nudge → force-release → step:failed, while normal idle release → step:completed.

} finally {
// Snapshot PTY chunks before cleanup — we need them for output reading below
ptyChunks = this.ptyOutputBuffers.get(agentName) ?? [];
Expand Down Expand Up @@ -3477,7 +3483,7 @@ export class WorkflowRunner {
: exitResult === 'timeout'
? 'Agent completed (released after idle timeout)'
: exitResult === 'released'
? 'Agent completed (force-released after idle nudging)'
? 'Agent completed (idle — treated as done)'
: `Agent exited (${exitResult})`;
}

Expand Down Expand Up @@ -3517,7 +3523,7 @@ export class WorkflowRunner {
agentDef: AgentDefinition,
step: WorkflowStep,
timeoutMs?: number
): Promise<'exited' | 'timeout' | 'released'> {
): Promise<'exited' | 'timeout' | 'released' | 'force-released'> {
const nudgeConfig = this.currentConfig?.swarm.idleNudge;
if (!nudgeConfig) {
// Idle = done: race exit against idle. Whichever fires first completes the step.
Expand Down Expand Up @@ -3568,7 +3574,7 @@ export class WorkflowRunner {
}

// Agent is still running after the window expired.
if (remaining !== undefined && Date.now() - startTime >= remaining) {
if (timeoutMs !== undefined && Date.now() - startTime >= timeoutMs) {
return 'timeout';
}

Expand All @@ -3587,7 +3593,7 @@ export class WorkflowRunner {
);
this.emit({ type: 'step:force-released', runId: this.currentRunId ?? '', stepName: step.name });
await agent.release();
return 'released';
return 'force-released';
}
}

Expand Down
Loading