fix: WorkflowBuilder preset field + reviewer double-booking

[khaliqgant]

Summary

• WorkflowBuilder drops preset field: WorkflowBuilder.agent() never mapped options.preset to the AgentDefinition. The AgentOptions interface was missing the preset field entirely. This meant preset: 'worker' via the builder API had no effect — resolveAgentDef never saw the preset, so interactive: false was never applied, and agents were spawned as interactive PTY with the full owner-supervisor-reviewer pipeline.
• Reviewer double-booking: resolveAutoReviewAgent independently assigned the same agent as reviewer for multiple concurrent steps with no coordination. When e.g. worker-a was assigned to review both task-b and task-c simultaneously, one review could starve and hit the safety backstop timeout (observed in fan-out workflows).

Changes

1. Added preset?: AgentPreset to AgentOptions interface and the agent() method in builder.ts
2. Added activeReviewers Set to WorkflowRunner to track agents currently assigned as reviewers
3. Updated resolveAutoReviewAgent to prefer agents not currently reviewing, falling back to busy agents only when no alternative exists
4. Cleanup of activeReviewers in the finally block after review completes and in the run cleanup

Test plan

• Workflow with preset: 'worker' via builder API should run agents as non-interactive (no PTY, no owner/reviewer pipeline)
• Fan-out workflow with 3+ parallel steps should not assign the same reviewer to multiple concurrent reviews when alternatives exist
• Existing review gate behavior unchanged when only one eligible reviewer exists

🤖 Generated with Claude Code

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

[devin-ai-integration]

Devin Review found 2 new potential issues.

View 12 additional findings in Devin Review.

[devin-ai-integration]

🔴 Grace period exit-timeout check doesn't distinguish real exits from timeouts, causing non-deterministic behavior

When an agent goes idle but output_contains verification hasn't passed, the code enters a 15-second grace period at packages/sdk/src/workflows/runner.ts:5367. Both waitForExit and waitForIdle are raced with the same 15s timeout. If the agent is permanently idle, both resolve with 'timeout' near-simultaneously. Which promise wins Promise.race is non-deterministic:

• If exit wins: graceResult = { kind: 'exit', result: 'timeout' } → the if (graceResult.kind === 'exit') check at line 5375 matches → returns 'timeout' without releasing the agent. Upstream spawnAndWait then treats this as a step timeout (packages/sdk/src/workflows/runner.ts:5146-5174), running verification and potentially throwing a timeout error.
• If idle wins: graceResult = { kind: 'idle', result: 'timeout' } → falls through to line 5380-5383 → agent is released, returns 'released'. Upstream proceeds normally with output collection.

The same scenario (permanently idle agent, grace period expired, verification not passed) produces different outcomes depending on which promise resolves first. The comment at line 5378 ("Grace period timed out — agent is permanently idle") indicates the intended behavior is always the release path, but the overly broad graceResult.kind === 'exit' check intercepts exit timeouts along with real exits.

Fix: filter out timeout results from the exit check

if (graceResult.kind === 'exit' && graceResult.result !== 'timeout') {
  return graceResult.result as 'exited' | 'released';
}

Suggested change

	if (graceResult.kind === 'exit') {
	return graceResult.result as 'exited' | 'timeout' | 'released';
	}
	if (graceResult.kind === 'exit' && graceResult.result !== 'timeout') {
	return graceResult.result as 'exited' | 'released';
	}

---

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

[devin-ai-integration]

🟡 Idle verification grace period ignores remaining step timeout, potentially exceeding it by up to 15 seconds

At packages/sdk/src/workflows/runner.ts:5366-5369, the grace period uses a hardcoded idleGraceSecs = 15 without accounting for the remaining step timeout. The outer loop correctly computes remaining from timeoutMs at line 5334 and passes it to the main waitForExit/waitForIdle race. But once idle fires and verification fails, the grace period races with idleGraceSecs * 1000 (15000ms) regardless of how much timeout is left.

For example, if only 2 seconds of timeout remained when idle was detected, the main race resolves within 2s, but the grace period then waits up to 15s — exceeding the step timeout by 13 seconds. The timeout check at the top of the while loop (remaining <= 0) only runs on the next iteration, after the grace period has already blocked.

Suggested change

	const idleGraceSecs = 15;
	const graceResult = await Promise.race([
	agent.waitForExit(idleGraceSecs * 1000).then((r) => ({ kind: 'exit' as const, result: r })),
	agent.waitForIdle(idleGraceSecs * 1000).then((r) => ({ kind: 'idle' as const, result: r })),
	const idleGraceSecs = 15;
	const elapsedSinceLoopStart = Date.now() - idleLoopStart;
	const graceMs = timeoutMs != null
	? Math.min(idleGraceSecs * 1000, Math.max(0, timeoutMs - elapsedSinceLoopStart))
	: idleGraceSecs * 1000;
	const graceResult = await Promise.race([
	agent.waitForExit(graceMs).then((r) => ({ kind: 'exit' as const, result: r })),
	agent.waitForIdle(graceMs).then((r) => ({ kind: 'idle' as const, result: r })),
	]);

---

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

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 12 additional findings in Devin Review.

[devin-ai-integration]

🔴 JIT reviewer resolution drops review for self-owned steps in hub patterns

The deferred reviewer resolution at line 3448 requires usesDedicatedOwner in addition to usesAutoHardening, but the old eager resolution only required usesAutoHardening. This means hub-pattern steps where no dedicated lead/coordinator agent was found (i.e., ownerDef.name === specialistDef.name, so usesDedicatedOwner is false) no longer get a reviewer resolved. Previously, self-owned steps in hub patterns still received auto-review because resolveAutoReviewAgent was called unconditionally when usesAutoHardening was true (packages/sdk/src/workflows/runner.ts:3172). Now the review gate at line 3453 (if (usesOwnerFlow && reviewDef)) silently skips because reviewDef remains undefined — the JIT block at line 3448 never fires.

Example scenario where review is lost

A hub-spoke workflow with two agents where neither has an owner-ish name/role:

workflow('test')
  .pattern('hub-spoke')
  .agent('alpha', { cli: 'claude', role: 'Engineer' })
  .agent('beta',  { cli: 'claude', role: 'Tester' })
  .step('build', { agent: 'alpha', task: 'Build feature' })

resolveAutoStepOwner finds no dedicated owner → ownerDef === specialistDef → usesDedicatedOwner = false. Old code: reviewer resolved and review runs. New code: reviewer never resolved, review silently skipped.

Suggested change

	if (usesAutoHardening && usesDedicatedOwner && !reviewDef) {
	reviewDef = this.resolveAutoReviewAgent(ownerDef, agentMap);
	supervised.reviewer = reviewDef;
	}
	if (usesAutoHardening && !reviewDef) {
	reviewDef = this.resolveAutoReviewAgent(ownerDef, agentMap);
	supervised.reviewer = reviewDef;
	}

---

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