From 6c466f2a4ccbffd82db18cdf9e7850fd4bd0de83 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sun, 14 Jun 2026 12:36:17 +0200 Subject: [PATCH 1/2] Stop resuming implementers after PR completion --- .../src/orchestrator/factory.test.ts | 78 +++++++++++++++++-- .../factory-sdk/src/orchestrator/factory.ts | 22 ++++++ 2 files changed, 94 insertions(+), 6 deletions(-) diff --git a/packages/factory-sdk/src/orchestrator/factory.test.ts b/packages/factory-sdk/src/orchestrator/factory.test.ts index fb5bb82d..3031f456 100644 --- a/packages/factory-sdk/src/orchestrator/factory.test.ts +++ b/packages/factory-sdk/src/orchestrator/factory.test.ts @@ -3058,6 +3058,74 @@ describe('FactoryLoop', () => { expect(fleet.spawns.map((spawn) => spawn.name)).toEqual(['ar-6-impl', 'ar-6-review']) }) + it('completes an implementer exit without resuming when a PR already exists', async () => { + const issue = realIssueFile(254, ready, { title: 'Real implementer PR exit terminal' }) + const mount = new FakeMountClient({ [issuePath(254)]: issue }) + const fleet = new FakeFleetClient() + fleet.setSessionRef('ar-254-impl', 'session-impl-254') + const probedIssues: string[] = [] + const factory = createFactory(config({ + safety: { requireTitlePrefix: 'Real', requireTeamKey: 'AR' }, + }), { + mount, + fleet, + triage: new StaticTriage(), + probePrResolver: async (issue) => { + probedIssues.push(issue.key) + return { repo: 'AgentWorkforce/pear', prNumber: 254 } + }, + }) + const decision = await factory.triageIssue(parseLinearIssue(issuePath(254), issue)) + + await factory.dispatch(decision) + fleet.emitAgentExit('ar-254-impl', 'worker_exited') + + await vi.waitFor(() => expect(factory.status().counters.done).toBe(1)) + + expect(probedIssues).toEqual(['AR-254']) + expect(fleet.resumes).toEqual([]) + expect(fleet.spawns.map((spawn) => spawn.name)).toEqual(['ar-254-impl', 'ar-254-review']) + expect(fleet.releases).toEqual([ + { name: 'ar-254-impl', reason: 'issue-done' }, + { name: 'ar-254-review', reason: 'issue-done' }, + ]) + expect(factory.status().inFlight).toEqual([]) + }) + + it('still resumes an abnormally exited implementer when no PR exists yet', async () => { + const issue = realIssueFile(255, ready, { title: 'Real implementer crash resumes' }) + const mount = new FakeMountClient({ [issuePath(255)]: issue }) + const fleet = new FakeFleetClient() + fleet.setSessionRef('ar-255-impl', 'session-impl-255') + const probedIssues: string[] = [] + const factory = createFactory(config({ + safety: { requireTitlePrefix: 'Real', requireTeamKey: 'AR' }, + }), { + mount, + fleet, + triage: new StaticTriage(), + probePrResolver: async (issue) => { + probedIssues.push(issue.key) + return undefined + }, + }) + const decision = await factory.triageIssue(parseLinearIssue(issuePath(255), issue)) + + await factory.dispatch(decision) + fleet.emitAgentExit('ar-255-impl', 'crash') + await flush() + + expect(probedIssues).toEqual(['AR-255']) + expect(fleet.resumes).toEqual([{ + name: 'ar-255-impl', + sessionRef: 'session-impl-255', + node: 'self', + capability: 'spawn:codex', + }]) + expect(fleet.releases).toEqual([]) + expect(factory.status().counters.done).toBeUndefined() + }) + it('coalesces duplicate exit callbacks for the same open issue, agent, and sessionRef', async () => { const mount = new FakeMountClient({ [issuePath(10)]: issueFile(10) }) const fleet = new FakeFleetClient() @@ -3509,12 +3577,10 @@ describe('FactoryLoop', () => { expect(factory.status().queued.map((issue) => issue.key)).toEqual(['AR-353']) for (const n of [351, 352]) { - for (const role of ['impl', 'review']) { - fleet.emitAgentExit(`ar-${n}-${role}`, 'worker_exited') - await flush() - fleet.emitAgentExit(`ar-${n}-${role}`, 'worker_exited') - await flush() - } + fleet.emitAgentExit(`ar-${n}-review`, 'worker_exited') + await flush() + fleet.emitAgentExit(`ar-${n}-review`, 'worker_exited') + await flush() } expect(factory.status().inFlight.map((issue) => issue.key)).toEqual(['AR-351', 'AR-352']) diff --git a/packages/factory-sdk/src/orchestrator/factory.ts b/packages/factory-sdk/src/orchestrator/factory.ts index 609a87b0..c90366f7 100644 --- a/packages/factory-sdk/src/orchestrator/factory.ts +++ b/packages/factory-sdk/src/orchestrator/factory.ts @@ -1581,6 +1581,11 @@ export class FactoryLoop implements Factory { } try { + if (tracked.spec.role === 'implementer' && await this.#issueHasCompletionPr(record)) { + await this.#completeIssue(record) + return + } + if (tracked.sessionRef) { const resumeKey = `${issueKey(record.issue)}:${name}:${tracked.sessionRef}` if (this.#resumedExitKeys.has(resumeKey)) { @@ -1622,6 +1627,23 @@ export class FactoryLoop implements Factory { } } + async #issueHasCompletionPr(record: InFlightIssue): Promise { + try { + const issue = await this.#readIssue(record.issue.path) + if (!issue) { + return false + } + return Boolean(await this.#completionPrForIssue(issue)) + } catch (error) { + this.#logger.warn?.('[factory] PR probe failed after implementer exit; preserving restart behavior', { + issue: record.issue.key, + error: describeError(error).errorMessage, + }) + this.#increment('exitPrProbeFailures') + return false + } + } + async #resumeTrackedAgent( record: InFlightIssue, name: string, From fcdc86f774544e4dd82f50c0a1aa600fa48603da Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sun, 14 Jun 2026 13:08:34 +0200 Subject: [PATCH 2/2] Exclude draft PRs from implementer-exit completion (#328 review fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #issueHasCompletionPr treated any resolved PR — including drafts — as a completion signal, so an implementer exiting while its PR was still a draft marked the issue done and released both agents prematurely, bypassing the #sweepPrStateCompletions draft guard. Flagged by gemini/codex/devin at factory.ts:1636. Gate on !pr.draft so only a ready PR counts as completion; a draft-PR exit falls through to the normal resume path. Adds a test for the draft case (no done, no release, resumes). Full factory-sdk suite green; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/orchestrator/factory.test.ts | 36 +++++++++++++++++++ .../factory-sdk/src/orchestrator/factory.ts | 7 +++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/factory-sdk/src/orchestrator/factory.test.ts b/packages/factory-sdk/src/orchestrator/factory.test.ts index 3031f456..9ee41738 100644 --- a/packages/factory-sdk/src/orchestrator/factory.test.ts +++ b/packages/factory-sdk/src/orchestrator/factory.test.ts @@ -3092,6 +3092,42 @@ describe('FactoryLoop', () => { expect(factory.status().inFlight).toEqual([]) }) + it('does not complete on an implementer exit when only a draft PR exists', async () => { + const issue = realIssueFile(256, ready, { title: 'Real implementer draft PR exit' }) + const mount = new FakeMountClient({ [issuePath(256)]: issue }) + const fleet = new FakeFleetClient() + fleet.setSessionRef('ar-256-impl', 'session-impl-256') + const probedIssues: string[] = [] + const factory = createFactory(config({ + safety: { requireTitlePrefix: 'Real', requireTeamKey: 'AR' }, + }), { + mount, + fleet, + triage: new StaticTriage(), + probePrResolver: async (issue) => { + probedIssues.push(issue.key) + return { repo: 'AgentWorkforce/pear', prNumber: 256, draft: true } + }, + }) + const decision = await factory.triageIssue(parseLinearIssue(issuePath(256), issue)) + + await factory.dispatch(decision) + fleet.emitAgentExit('ar-256-impl', 'worker_exited') + await flush() + + // A draft PR is not a completion signal: no done, no release; the exit falls + // through to the normal resume path (mirrors the no-PR case). + expect(probedIssues).toEqual(['AR-256']) + expect(factory.status().counters.done).toBeUndefined() + expect(fleet.releases).toEqual([]) + expect(fleet.resumes).toEqual([{ + name: 'ar-256-impl', + sessionRef: 'session-impl-256', + node: 'self', + capability: 'spawn:codex', + }]) + }) + it('still resumes an abnormally exited implementer when no PR exists yet', async () => { const issue = realIssueFile(255, ready, { title: 'Real implementer crash resumes' }) const mount = new FakeMountClient({ [issuePath(255)]: issue }) diff --git a/packages/factory-sdk/src/orchestrator/factory.ts b/packages/factory-sdk/src/orchestrator/factory.ts index c90366f7..70c4e3d3 100644 --- a/packages/factory-sdk/src/orchestrator/factory.ts +++ b/packages/factory-sdk/src/orchestrator/factory.ts @@ -1633,7 +1633,12 @@ export class FactoryLoop implements Factory { if (!issue) { return false } - return Boolean(await this.#completionPrForIssue(issue)) + // Only a NON-DRAFT (ready) PR counts as completion. A draft PR means the + // work isn't review-ready, so an implementer exiting with only a draft PR + // must NOT mark the issue done / release agents — mirror the + // #sweepPrStateCompletions draft guard, which keeps draft-PR issues in flight. + const pr = await this.#completionPrForIssue(issue) + return Boolean(pr && !pr.draft) } catch (error) { this.#logger.warn?.('[factory] PR probe failed after implementer exit; preserving restart behavior', { issue: record.issue.key,