diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 2b7fd1ab..87c822be 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -14,9 +14,9 @@ import type { FactoryIntegrationConnections, FactoryIntegrationProvider, FactoryPorts, - createFactory, } from '../index' import { + createFactory, FactoryConfigSchema, FileFactoryCloudEventOutbox, LiveDispatchStateChangedError, @@ -25,10 +25,11 @@ import { import { MountAuthScopeError, mountAuthRemediation } from '../mount/mount-auth-error' import { DocumentStateStore, FileStateStore } from '../state/file-state-store' import { FakeFleetClient, FakeMountClient, withDeadline } from '../testing' -import type { GithubConnectionRead, GithubConnectionWrite, GithubIssueLookup, LocalMountOptions, SpawnInput, SpawnResult } from '../ports' +import type { GithubConnectionRead, GithubConnectionWrite, GithubIssueLookup, GithubWriteback, LocalMountOptions, SpawnInput, SpawnResult } from '../ports' import type { HarnessDriverClientLike } from '../fleet/internal-fleet-client' import type { RelayMessaging } from '@agent-relay/sdk' import { factoryGithubIssueCommentDraftName } from '../github/writeback-paths' +import { GhCliGithubWriteback } from '../writeback/github' import { ensureLocalMount as runLocalMountPreflight } from '../mount/local-mount-preflight' import { formatLogArgs, installFactoryStopSignalHandlers, parseFleetCommand, parseGithubIssueSelector, parseGlobalOptions, reportFactoryVersionDrift, resolveBrokerConnectionPath, resolveFactoryBrokerConnectionPath, runFleetCli } from './fleet' @@ -1981,10 +1982,656 @@ describe('fleet CLI runtime', () => { } }) - // factory#319 regression guard. The self-race itself is exercised by - // "keeps relay dispatch ownership …" above, which failed ~50% of runs on - // main and is deterministic with the fix; this control pins the other half - // of the contract — that a park this dispatch did NOT make still aborts. + // factory#319. The state write becomes locally visible before provider + // confirmation returns. Pin the post-spawn read in that interval and test + // both outcomes: confirmation lets the completed dispatch converge; a + // rejected confirmation MUST NOT excuse the visible park. + it.each([ + { confirmation: 'acked' as const, expectedExit: 0 }, + { confirmation: 'failed' as const, expectedExit: 3 }, + ])('waits for its in-flight park confirmation ($confirmation)', async ({ confirmation, expectedExit }) => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-park-inflight-')) + try { + const configPath = await writeConfig(root, { + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + heartbeatStaleMs: 10_000, + }, + }) + const githubWrite: GithubConnectionWrite = { + publishPullRequest: async (input) => ({ + repo: input.repo, + number: 79, + url: 'https://github.com/AgentWorkforce/pear/pull/79', + headRef: input.headRef ?? 'unexpected-local-head', + }), + closePullRequest: async () => undefined, + } + class ControlledCompletingRemoteFleetClient extends FakeFleetClient { + override readonly placementLocality = 'remote' as const + implementerName?: string + exitEmitted = false + + override async spawn(input: SpawnInput): Promise { + const result = await super.spawn(input) + if (input.name.includes('-impl-')) this.implementerName = input.name + return { ...result, node: 'sf-mini', locality: 'remote' } + } + } + const fleet = new ControlledCompletingRemoteFleetClient() + // `setState` writes `stateId` at the top level of the record; the initial + // fixture carries it under `payload`. Read either. + const stateOf = (entry: { content: unknown } | undefined): string | undefined => { + const content = entry?.content as { stateId?: string; payload?: { stateId?: string } } | undefined + return content?.stateId ?? content?.payload?.stateId + } + + class ParkConfirmBlockingMount extends FakeMountClient { + parkWritten = false + releaseConfirm?: () => void + reReadSeen = false + + override async writeFile(path: string, content: unknown, opts?: { guarded?: boolean }): Promise { + await super.writeFile(path, content, opts) + if (path === issuePath && stateOf(this.files.get(path)) === TEST_STATE_IDS.humanReview) { + this.parkWritten = true + } + } + + override async readFile(path: string): Promise<{ content: unknown; revision?: string }> { + if (path === issuePath && fleet.implementerName && !fleet.exitEmitted) { + // This is the dispatch's post-spawn re-read. Start completion here + // instead of hoping a timer wins the race, then hold this read + // until completion has made the parked state visible. Completion's + // own reads pass through because `exitEmitted` is already true. + fleet.exitEmitted = true + fleet.emitAgentExit(fleet.implementerName, 'exited') + await vi.waitFor(() => expect(this.parkWritten).toBe(true), { timeout: 1_000 }) + this.reReadSeen = true + queueMicrotask(() => this.releaseConfirm?.()) + } + if (path === issuePath && this.parkWritten && confirmation === 'acked') { + if (!this.releaseConfirm) { + // First issue read after the park is `setState`'s own readback + // confirmation. Hold it, so `setState` cannot return. Bounded so + // a wrong assumption about ordering fails loudly rather than + // hanging the suite. + await new Promise((resolve) => { + this.releaseConfirm = resolve + setTimeout(resolve, 400) + }) + } + } + return super.readFile(path) + } + + override async confirmWrite(path: string): Promise<'acked' | 'pending' | 'failed' | 'timeout'> { + if (path === issuePath && this.parkWritten) return confirmation + return super.confirmWrite(path) + } + } + + const mount = new ParkConfirmBlockingMount({ + [issuePath]: issueFile, + '/github/repos/AgentWorkforce/pear/meta.json': { default_branch: 'main' }, + }, githubWrite) + const output = buffer() + + const code = await runFleetCli([ + 'dispatch', + 'AR-77', + '--backend', + 'relay', + '--config', + configPath, + ], { + fleet, + mount, + stdout: output, + stderr: buffer(), + probePrGhRunner: async () => ({ stdout: '[]' }), + }) + + // Both must hold or the test is not exercising the window at all. + expect(mount.parkWritten).toBe(true) + expect(mount.reReadSeen).toBe(true) + expect(code).toBe(expectedExit) + const releaseReasons = fleet.releases.map((release) => release.reason) + if (confirmation === 'acked') { + expect(releaseReasons).not.toContain('live dispatch state changed') + } else { + expect(releaseReasons).toContain('live dispatch state changed') + } + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it.each([ + { receipt: 'explicit already-matched' as const, legacyVoid: false }, + { receipt: 'legacy void' as const, legacyVoid: true }, + ])('does not attribute a third-party GitHub park from an in-flight $receipt receipt', async ({ legacyVoid }) => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-foreign-park-inflight-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + heartbeatStaleMs: 10_000, + }, + }) + const githubPath = '/github/repos/AgentWorkforce__pear/issues/by-id/48.json' + const githubWrite: GithubConnectionWrite = { + publishPullRequest: async (input) => ({ + repo: input.repo, + number: 80, + url: 'https://github.com/AgentWorkforce/pear/pull/80', + headRef: input.headRef ?? 'unexpected-local-head', + }), + closePullRequest: async () => undefined, + } + class ControlledCompletingGithubFleetClient extends FakeFleetClient { + override readonly placementLocality = 'remote' as const + implementerName?: string + exitEmitted = false + + override async spawn(input: SpawnInput): Promise { + const result = await super.spawn(input) + if (input.name.includes('-impl-')) this.implementerName = input.name + return { ...result, node: 'sf-mini', locality: 'remote' } + } + } + const fleet = new ControlledCompletingGithubFleetClient() + let thirdPartyParked = false + let terminalLabelProvisioned = false + let terminalViews = 0 + let terminalEdits = 0 + let releaseConfirmation: (() => void) | undefined + + class ThirdPartyGithubParkMount extends FakeMountClient { + postSpawnReadSawPark = false + + parkAsThirdParty(): void { + const existing = this.files.get(githubPath) + const content = existing?.content as ReturnType + const labels = content.payload.labels + .filter((label) => label.name !== 'factory:in-progress') + labels.push({ name: 'factory:human-review' }) + this.files.set(githubPath, { + ...existing, + content: { + ...content, + payload: { ...content.payload, labels }, + }, + }) + thirdPartyParked = true + } + + override async readFile(path: string): Promise<{ content: unknown; revision?: string }> { + if (path === githubPath && fleet.implementerName && !fleet.exitEmitted) { + // This is dispatch's post-spawn read. Completion starts here and + // registers its fence before the CLI adapter's first label read. + fleet.exitEmitted = true + fleet.emitAgentExit(fleet.implementerName, 'exited') + await vi.waitFor(() => expect(thirdPartyParked).toBe(true), { timeout: 1_000 }) + const result = await super.readFile(path) + this.postSpawnReadSawPark = true + // Let the adapter's provider-confirmation read finish only after + // the dispatch has observed the foreign state while the fence is + // still registered. + setTimeout(() => releaseConfirmation?.(), 0) + return result + } + return super.readFile(path) + } + } + + const githubContent = githubIssueFile('pear') + githubContent.payload.title = '[factory-e2e] preserve foreign GitHub lifecycle ownership' + githubContent.payload.body = [ + 'Fix the completion race without attributing a third-party lifecycle change to Factory.', + '', + 'Acceptance criteria:', + '- A skipped GitHub CLI label edit never stamps Factory authorship.', + '- The live dispatch state-change guard still releases the remote agent.', + '- Add a deterministic integration regression for the provider-read ordering.', + ].join('\n') + const mount = new ThirdPartyGithubParkMount({ + [githubPath]: githubContent, + '/github/repos/AgentWorkforce/pear/meta.json': { default_branch: 'main' }, + }, githubWrite) + const providerLabels = new Set(['factory', 'pear']) + const githubWriteback = new GhCliGithubWriteback({ + runner: async (args) => { + if (args[0] === 'label' && args[1] === 'create') { + terminalLabelProvisioned = args[2] === 'factory:human-review' + return { stdout: '' } + } + if (args[0] === 'issue' && args[1] === 'view') { + if (terminalLabelProvisioned) { + terminalViews += 1 + if (terminalViews === 1) { + // The third party wins before the adapter's first read. The + // target is present and the previous label absent, so the CLI + // must skip `gh issue edit` and report already-matched. + providerLabels.delete('factory:in-progress') + providerLabels.add('factory:human-review') + mount.parkAsThirdParty() + } else if (terminalViews === 2 && !releaseConfirmation) { + await new Promise((resolve) => { + releaseConfirmation = resolve + setTimeout(resolve, 400) + }) + } + } + return { stdout: JSON.stringify({ labels: [...providerLabels].map((name) => ({ name })) }) } + } + if (args[0] === 'issue' && args[1] === 'edit') { + if (terminalLabelProvisioned) terminalEdits += 1 + const added = args[args.indexOf('--add-label') + 1] + const removed = args[args.indexOf('--remove-label') + 1] + if (args.includes('--add-label') && added) providerLabels.add(added) + if (args.includes('--remove-label') && removed) providerLabels.delete(removed) + } + return { stdout: '' } + }, + }) + const effectiveGithubWriteback: GithubWriteback = legacyVoid + ? { + getIssueStatus: async (target) => await githubWriteback.getIssueStatus(target), + postComment: async (target, body) => await githubWriteback.postComment(target, body), + setStatus: async (target, status) => { + await githubWriteback.setStatus(target, status) + }, + closeIssue: async (target, body) => await githubWriteback.closeIssue(target, body), + } + : githubWriteback + const output = buffer() + const errors = buffer() + + const code = await runFleetCli([ + 'dispatch', + '48', + '--backend', + 'relay', + '--config', + configPath, + ], { + fleet, + mount, + createFactory: (factoryConfig, ports) => createFactory(factoryConfig, { + ...ports, + githubWriteback: effectiveGithubWriteback, + }), + stdout: output, + stderr: errors, + probePrGhRunner: async () => ({ stdout: '[]' }), + }) + + expect(thirdPartyParked).toBe(true) + expect(mount.postSpawnReadSawPark).toBe(true) + expect(terminalEdits).toBe(0) + expect(code).toBe(3) + expect(errors.text()).toContain('Live state changed before writeback for 48') + expect(fleet.releases.map((release) => release.reason)).toContain('live dispatch state changed') + if (legacyVoid) { + expect(errors.text()).toContain('GitHub writeback returned no ownership receipt') + expect(errors.text()).toContain('"method":"setStatus"') + } else { + expect(errors.text()).not.toContain('GitHub writeback returned no ownership receipt') + } + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it.each([ + { receipt: 'explicit acknowledged' as const, legacyVoid: false }, + { receipt: 'legacy void' as const, legacyVoid: true }, + ])('does not attribute a third-party GitHub close from an in-flight $receipt receipt', async ({ legacyVoid }) => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-foreign-close-inflight-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + terminalState: 'done', + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + heartbeatStaleMs: 10_000, + }, + }) + const githubPath = '/github/repos/AgentWorkforce__pear/issues/by-id/48.json' + const githubWrite: GithubConnectionWrite = { + publishPullRequest: async (input) => ({ + repo: input.repo, + number: 80, + url: 'https://github.com/AgentWorkforce/pear/pull/80', + headRef: input.headRef ?? 'unexpected-local-head', + }), + closePullRequest: async () => undefined, + } + class ControlledCompletingGithubFleetClient extends FakeFleetClient { + override readonly placementLocality = 'remote' as const + implementerName?: string + exitEmitted = false + + override async spawn(input: SpawnInput): Promise { + const result = await super.spawn(input) + if (input.name.includes('-impl-')) this.implementerName = input.name + return { ...result, node: 'sf-mini', locality: 'remote' } + } + } + const fleet = new ControlledCompletingGithubFleetClient() + let thirdPartyClosed = false + + class ThirdPartyGithubCloseMount extends FakeMountClient { + postSpawnReadSawClose = false + + closeAsThirdParty(): void { + const existing = this.files.get(githubPath) + const content = existing?.content as ReturnType + this.files.set(githubPath, { + ...existing, + content: { + ...content, + payload: { ...content.payload, state: 'closed' }, + }, + }) + thirdPartyClosed = true + } + + override async readFile(path: string): Promise<{ content: unknown; revision?: string }> { + if (path === githubPath && fleet.implementerName && !fleet.exitEmitted) { + // Pin the dispatch's post-spawn read behind completion. The + // provider close command returns idempotently after another actor + // creates the terminal event, so only the audit receipt can tell + // the dispatch that it must not claim ownership. + fleet.exitEmitted = true + fleet.emitAgentExit(fleet.implementerName, 'exited') + await vi.waitFor(() => expect(thirdPartyClosed).toBe(true), { timeout: 1_000 }) + this.postSpawnReadSawClose = true + } + return super.readFile(path) + } + } + + const githubContent = githubIssueFile('pear') + githubContent.payload.title = '[factory-e2e] preserve foreign GitHub close ownership' + githubContent.payload.body = [ + 'Keep terminal lifecycle ownership provider-authoritative during concurrent close operations.', + '', + 'Acceptance criteria:', + '- An idempotent close must not claim Factory ownership when another actor creates the close event.', + '- The post-spawn live-state guard must abort the dispatch.', + '- Remote agents must be released with the live-state-change reason.', + ].join('\n') + const mount = new ThirdPartyGithubCloseMount({ + [githubPath]: githubContent, + '/github/repos/AgentWorkforce/pear/meta.json': { default_branch: 'main' }, + '/github/repos/AgentWorkforce/pear/pulls/80/metadata.json': { + provider: 'github', + objectType: 'pull_request', + objectId: '80', + payload: { + number: 80, + title: '[factory-e2e] preserve foreign GitHub close ownership', + body: 'Fixes #48', + head_ref: 'factory/48-agentworkforce-pear', + isDraft: false, + state: 'MERGED', + merged: true, + }, + }, + }, githubWrite) + let providerState = 'OPEN' + const stateEvents: string[] = [] + let closeCommands = 0 + const githubWriteback = new GhCliGithubWriteback({ + runner: async (args) => { + if (args[0] === 'issue' && args[1] === 'view' && args.includes('state')) { + return { stdout: JSON.stringify({ state: providerState }) } + } + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') { + return { stdout: stateEvents.join('\n') } + } + if (args[0] === 'issue' && args[1] === 'close') { + closeCommands += 1 + providerState = 'CLOSED' + stateEvents.push('1\tclosed\tother-user') + mount.closeAsThirdParty() + } + return { stdout: '' } + }, + }) + const effectiveGithubWriteback: GithubWriteback = legacyVoid + ? { + getIssueStatus: async (target) => await githubWriteback.getIssueStatus(target), + postComment: async (target, body) => await githubWriteback.postComment(target, body), + setStatus: async (target, status) => await githubWriteback.setStatus(target, status), + closeIssue: async (target, body) => { + await githubWriteback.closeIssue(target, body) + }, + } + : githubWriteback + const output = buffer() + const errors = buffer() + + const code = await runFleetCli([ + 'dispatch', + '48', + '--backend', + 'relay', + '--config', + configPath, + ], { + fleet, + mount, + createFactory: (factoryConfig, ports) => createFactory(factoryConfig, { + ...ports, + githubWriteback: effectiveGithubWriteback, + }), + stdout: output, + stderr: errors, + probePrGhRunner: async () => ({ + stdout: JSON.stringify([{ + number: 80, + title: '[factory-e2e] preserve foreign GitHub close ownership', + body: 'Fixes #48', + headRefName: 'factory/48-agentworkforce-pear', + isDraft: false, + state: 'MERGED', + }]), + }), + }) + + expect(thirdPartyClosed, JSON.stringify({ + code, + output: output.text(), + errors: errors.text(), + releases: fleet.releases, + })).toBe(true) + expect(mount.postSpawnReadSawClose).toBe(true) + expect(closeCommands).toBe(1) + expect(code).toBe(3) + expect(errors.text()).toContain('Live state changed before writeback for 48') + expect(fleet.releases.map((release) => release.reason)).toContain('live dispatch state changed') + if (legacyVoid) { + expect(errors.text()).toContain('GitHub writeback returned no ownership receipt') + expect(errors.text()).toContain('"method":"closeIssue"') + } else { + expect(errors.text()).not.toContain('GitHub writeback returned no ownership receipt') + } + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('serializes terminal completion that starts after the ready read against the dispatch claim', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-claim-completion-fence-')) + let releaseClaim: (() => void) | undefined + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + terminalState: 'human-review', + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + heartbeatStaleMs: 10_000, + }, + }) + const githubPath = '/github/repos/AgentWorkforce__pear/issues/by-id/48.json' + class CompletingDuringClaimFleetClient extends FakeFleetClient { + override readonly placementLocality = 'remote' as const + implementerName?: string + exitEmitted = false + + override async spawn(input: SpawnInput): Promise { + const result = await super.spawn(input) + if (input.name.includes('-impl-')) this.implementerName = input.name + return { ...result, node: 'sf-mini', locality: 'remote' } + } + } + const fleet = new CompletingDuringClaimFleetClient() + let resolveCompletionRead!: () => void + const completionRead = new Promise((resolve) => { + resolveCompletionRead = resolve + }) + class CompletionObservedMount extends FakeMountClient { + completionReadSeen = false + + override async readFile(path: string): Promise<{ content: unknown; revision?: string }> { + if (path === githubPath && fleet.exitEmitted && !this.completionReadSeen) { + this.completionReadSeen = true + resolveCompletionRead() + } + return await super.readFile(path) + } + } + const githubWrite: GithubConnectionWrite = { + publishPullRequest: async (input) => ({ + repo: input.repo, + number: 80, + url: 'https://github.com/AgentWorkforce/pear/pull/80', + headRef: input.headRef ?? 'unexpected-local-head', + }), + closePullRequest: async () => undefined, + } + const githubContent = githubIssueFile('pear') + githubContent.payload.title = '[factory-e2e] serialize dispatch claim with terminal completion' + githubContent.payload.body = [ + 'Prevent a stale post-spawn ready snapshot from overwriting a concurrent terminal writeback.', + '', + 'Acceptance criteria:', + '- Completion that starts after the ready read waits for the dispatch claim write.', + '- The final provider state remains human review.', + '- The agent is released with the terminal completion reason.', + ].join('\n') + const mount = new CompletionObservedMount({ + [githubPath]: githubContent, + '/github/repos/AgentWorkforce/pear/meta.json': { default_branch: 'main' }, + }, githubWrite) + let resolveClaimStarted!: () => void + const claimStarted = new Promise((resolve) => { + resolveClaimStarted = resolve + }) + const claimRelease = new Promise((resolve) => { + releaseClaim = resolve + }) + const statusCalls: string[] = [] + let terminalStarted = false + const output = buffer() + const errors = buffer() + const setMountedStatus = (status: 'in-progress' | 'human-review'): void => { + const existing = mount.files.get(githubPath) + const content = existing?.content as ReturnType + const labels = content.payload.labels.filter((label) => + label.name !== 'factory:in-progress' && label.name !== 'factory:human-review') + labels.push({ name: `factory:${status}` }) + mount.files.set(githubPath, { + ...existing, + content: { ...content, payload: { ...content.payload, labels } }, + }) + } + const githubWriteback: GithubWriteback = { + getIssueStatus: async () => undefined, + postComment: async () => undefined, + setStatus: async (_issue, status) => { + statusCalls.push(`start:${status}`) + if (status === 'in-progress') { + resolveClaimStarted() + if (!fleet.implementerName) throw new Error('implementer was not spawned before the dispatch claim') + fleet.exitEmitted = true + fleet.emitAgentExit(fleet.implementerName, 'exited') + await claimRelease + } else if (status === 'human-review') { + terminalStarted = true + } + setMountedStatus(status) + statusCalls.push(`applied:${status}`) + return 'applied' + }, + closeIssue: async () => 'acknowledged', + } + + const run = runFleetCli([ + 'dispatch', + '48', + '--backend', + 'relay', + '--config', + configPath, + ], { + fleet, + mount, + createFactory: (factoryConfig, ports) => createFactory(factoryConfig, { + ...ports, + githubWriteback, + }), + stdout: output, + stderr: errors, + probePrGhRunner: async () => ({ stdout: '[]' }), + }) + + try { + await withDeadline(claimStarted, 1_000, 'dispatch claim did not start') + } catch (error) { + throw new Error(JSON.stringify({ + error: error instanceof Error ? error.message : String(error), + output: output.text(), + errors: errors.text(), + statusCalls, + spawns: fleet.spawns, + releases: fleet.releases, + })) + } + await withDeadline(completionRead, 1_000, 'completion did not reach its issue read') + await new Promise((resolve) => setTimeout(resolve, 10)) + const terminalStartedBeforeClaimReleased = terminalStarted + releaseClaim() + const code = await run + + expect(fleet.exitEmitted).toBe(true) + expect(mount.completionReadSeen).toBe(true) + expect(terminalStartedBeforeClaimReleased).toBe(false) + expect(statusCalls).toEqual([ + 'start:in-progress', + 'applied:in-progress', + 'start:human-review', + 'applied:human-review', + ]) + expect(code).toBe(0) + expect((mount.files.get(githubPath)?.content as ReturnType).payload.labels) + .toContainEqual({ name: 'factory:human-review' }) + expect(fleet.releases.map((release) => release.reason)).toContain('issue-human-review') + } finally { + releaseClaim?.() + await rm(root, { recursive: true, force: true }) + } + }) + it('still aborts when a third party parks the issue and this dispatch did not', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-foreign-park-')) try { diff --git a/src/index.ts b/src/index.ts index 2f7e931d..0ecff7cf 100644 --- a/src/index.ts +++ b/src/index.ts @@ -329,6 +329,7 @@ export type { Clock, EventPage, GithubConnectionWrite, + GithubConnectionMutationReceipt, GithubConnectionIssue, GithubConnectionRead, GithubIssueLookup, @@ -352,7 +353,11 @@ export type { SpawnResult, AgentUsage, GithubRead, + GithubIssueCloseWriteResult, GithubIssueStatus, + GithubStatusWriteResult, + GithubStatusClaimReceipt, + GithubStatusRollbackResult, GithubWriteback, LinearWriteback, Logger, diff --git a/src/mount/relayfile-cloud-mount-client.test.ts b/src/mount/relayfile-cloud-mount-client.test.ts index aa15968b..1b5ec8b2 100644 --- a/src/mount/relayfile-cloud-mount-client.test.ts +++ b/src/mount/relayfile-cloud-mount-client.test.ts @@ -1511,6 +1511,25 @@ describe('RelayfileCloudMountClient', () => { } } + class HangingReadFileClient extends FakeRelayFileClient { + seenSignal?: AbortSignal + + override async readFile( + workspaceId: string, + path: string, + _correlationId?: string, + signal?: AbortSignal, + ): Promise { + this.readFileCalls.push({ workspaceId, path }) + this.seenSignal = signal + return await new Promise((_resolve, reject) => { + signal?.addEventListener('abort', () => { + reject((signal as AbortSignal & { reason?: unknown }).reason) + }) + }) + } + } + it('cancels a read that stops answering and names the operation', async () => { const client = new HangingListTreeClient() const mount = new RelayfileCloudMountClient({ @@ -1546,6 +1565,38 @@ describe('RelayfileCloudMountClient', () => { }) }) + it('cancels the revision read before an unguarded write', async () => { + const client = new HangingReadFileClient() + const mount = new RelayfileCloudMountClient({ + workspaceId: 'rw_test', + client, + operationTimeoutMs: 25, + }) + + await expect(mount.writeFile('/tmp/draft.json', { draft: true })).rejects.toMatchObject({ + name: 'RelayfileOperationTimeoutError', + operation: 'writeFile.readRevision', + }) + expect(client.seenSignal?.aborted).toBe(true) + expect(client.writeFileCalls).toEqual([]) + }) + + it('cancels the current-revision read before a delete', async () => { + const client = new HangingReadFileClient() + const mount = new RelayfileCloudMountClient({ + workspaceId: 'rw_test', + client, + operationTimeoutMs: 25, + }) + + await expect(mount.deleteFile('/tmp/draft.json')).rejects.toMatchObject({ + name: 'RelayfileOperationTimeoutError', + operation: 'deleteFile.readCurrent', + }) + expect(client.seenSignal?.aborted).toBe(true) + expect(client.deleteFileCalls).toEqual([]) + }) + it('caps an explicit ensureSubRoot timeout at the tighter client-wide budget', async () => { const client = new HangingListTreeClient() const mount = new RelayfileCloudMountClient({ @@ -1766,7 +1817,8 @@ describe('RelayfileCloudMountClient', () => { }) const mount = new RelayfileCloudMountClient({ workspaceId: 'rw_test', client: fake, isAllowedDraft: () => true }) - await mount.writeFile('/linear/issues/AR-1.json', { stateId: 'new' }) + await expect(mount.writeFile('/linear/issues/AR-1.json', { stateId: 'new' })) + .resolves.toEqual({ targetRevision: 'next' }) expect(fake.writeFileCalls).toEqual([{ workspaceId: 'rw_test', @@ -1777,6 +1829,33 @@ describe('RelayfileCloudMountClient', () => { }]) }) + it('does not refresh or retry an explicit baseRevision after a conflict', async () => { + const fake = new FakeRelayFileClient() + const conflict = Object.assign(new Error('revision conflict'), { status: 409 }) + const write = vi.spyOn(fake, 'writeFile').mockRejectedValue(conflict) + const mount = new RelayfileCloudMountClient({ + workspaceId: 'rw_test', + client: fake, + isAllowedDraft: () => true, + }) + + await expect(mount.writeFile( + '/linear/issues/AR-1.json', + { stateId: 'ready' }, + { baseRevision: '7' }, + )).rejects.toBe(conflict) + + expect(write).toHaveBeenCalledTimes(1) + expect(write).toHaveBeenCalledWith({ + workspaceId: 'rw_test', + path: '/linear/issues/AR-1.json', + baseRevision: '7', + content: '{"stateId":"ready"}', + contentType: 'application/json', + }) + expect(fake.readFileCalls).toEqual([]) + }) + it('uses baseRevision 0 for creates and confirms the queued operation', async () => { const fake = new FakeRelayFileClient() const mount = new RelayfileCloudMountClient({ diff --git a/src/mount/relayfile-cloud-mount-client.ts b/src/mount/relayfile-cloud-mount-client.ts index f12c4e48..05921b14 100644 --- a/src/mount/relayfile-cloud-mount-client.ts +++ b/src/mount/relayfile-cloud-mount-client.ts @@ -681,7 +681,11 @@ export class RelayfileCloudMountClient implements MountClient { } } - async writeFile(path: string, content: unknown, opts?: { guarded?: boolean }): Promise { + async writeFile( + path: string, + content: unknown, + opts?: { guarded?: boolean; baseRevision?: string }, + ): Promise<{ targetRevision: string }> { if (isProviderWritebackPath(path) && await this.#isAllowedDraft?.(path, content, opts) !== true) { throw new Error(`Refusing provider writeback draft for ${path}: draft predicate rejected or is unset`) } @@ -691,9 +695,19 @@ export class RelayfileCloudMountClient implements MountClient { this.#confirmedFailureReasonByPath.delete(path) const writeAtCurrentRevision = async (): Promise => { - let baseRevision = '0' + let baseRevision = opts?.baseRevision ?? '0' + if (opts?.baseRevision !== undefined) { + return this.#client.writeFile({ + workspaceId: this.workspaceId, + path, + baseRevision, + content: serialized.content, + contentType: serialized.contentType, + }) + } try { - baseRevision = (await this.#client.readFile(this.workspaceId, path)).revision + baseRevision = (await this.#bounded('writeFile.readRevision', this.#operationTimeoutMs, (signal) => + this.#client.readFile(this.workspaceId, path, undefined, signal))).revision } catch (error) { if (!isHttpStatus(error, 404)) throw error } @@ -707,18 +721,22 @@ export class RelayfileCloudMountClient implements MountClient { }) } + let queued: WriteQueuedResponse try { - this.#lastOpByPath.set(path, (await writeAtCurrentRevision()).opId) + queued = await writeAtCurrentRevision() } catch (error) { - if (!isHttpStatus(error, 409)) throw error - this.#lastOpByPath.set(path, (await writeAtCurrentRevision()).opId) + if (opts?.baseRevision !== undefined || !isHttpStatus(error, 409)) throw error + queued = await writeAtCurrentRevision() } + this.#lastOpByPath.set(path, queued.opId) + return { targetRevision: queued.targetRevision } } async deleteFile(path: string): Promise { this.#confirmedExternalIdByPath.delete(path) this.#confirmedFailureReasonByPath.delete(path) - const current = await this.#client.readFile(this.workspaceId, path) + const current = await this.#bounded('deleteFile.readCurrent', this.#operationTimeoutMs, (signal) => + this.#client.readFile(this.workspaceId, path, undefined, signal)) const currentContent = parseRemoteContent(current) if (isProviderPath(path)) { await this.#assertProviderDeleteAllowed(path, currentContent) diff --git a/src/mount/relayfile-github-connection-write.test.ts b/src/mount/relayfile-github-connection-write.test.ts index 045b7fee..6984ca80 100644 --- a/src/mount/relayfile-github-connection-write.test.ts +++ b/src/mount/relayfile-github-connection-write.test.ts @@ -16,6 +16,19 @@ const gitRunnerForBranch = (branch: string): GitCommandRunner => vi.fn(async (ar }) describe('RelayfileGithubConnectionWrite', () => { + it('reads a private issue through the authenticated connected projection', async () => { + const path = '/github/repos/PrivateOrg__private-repo/issues/by-id/42.json' + const content = { payload: { number: 42, labels: [{ name: 'factory:human-review' }] } } + const write = new RelayfileGithubConnectionWrite({ + mount: new FakeMountClient({ [path]: content }), + }) + + await expect(write.getIssue('PrivateOrg/private-repo', 42)).resolves.toEqual({ + outcome: 'found', + issue: { repo: 'PrivateOrg/private-repo', number: 42, path, content }, + }) + }) + it('publishes an already-pushed remote branch without reading an orchestrator-local clone', async () => { const pullRequestPath = '/github/repos/AgentWorkforce/factory/pull-requests/factory-factory-ar-85-agentworkforce-factory-pushed.json' class ReceiptMount extends FakeMountClient { @@ -234,14 +247,14 @@ describe('RelayfileGithubConnectionWrite', () => { description: 'Factory agents are working on this issue.', author: 'app', }) - await write.mutateIssueLabel({ + const addReceipt = await write.mutateIssueLabel({ repo: 'AgentWorkforce/factory', number: 221, operation: 'add', label: 'factory:in-progress', author: 'app', }) - await write.mutateIssueLabel({ + const removeReceipt = await write.mutateIssueLabel({ repo: 'AgentWorkforce/factory', number: 221, operation: 'remove', @@ -249,6 +262,9 @@ describe('RelayfileGithubConnectionWrite', () => { author: 'app', }) + expect(addReceipt).toBe('acknowledged') + expect(removeReceipt).toBe('acknowledged') + expect(mount.writes).toEqual([ { path: '/github/repos/AgentWorkforce/factory/labels/factory-11111111-1111-4111-8111-111111111111.json', diff --git a/src/mount/relayfile-github-connection-write.ts b/src/mount/relayfile-github-connection-write.ts index 5cc6d89d..865a0146 100644 --- a/src/mount/relayfile-github-connection-write.ts +++ b/src/mount/relayfile-github-connection-write.ts @@ -8,6 +8,8 @@ import { } from '../github/writeback-paths' import type { GithubConnectionIssueUpdateInput, + GithubIssueLookup, + GithubConnectionMutationReceipt, GithubConnectionWrite, GithubPublishPullRequestInput, GithubPublishPullRequestResult, @@ -50,6 +52,35 @@ export class RelayfileGithubConnectionWrite implements GithubConnectionWrite { this.#operationIdFactory = config.operationIdFactory ?? randomUUID } + async getIssue(repo: string, number: number): Promise { + const { owner, repo: name } = githubRepoParts(repo) + assertPositiveGithubNumber(number, 'issue') + // Provider projections use the encoded owner__repo canonical tree, while + // connected write paths use the nested owner/repo tree. Accept both so the + // authenticated workspace connection remains authoritative across mount + // layouts and migrations. + const paths = [ + `/github/repos/${encodeURIComponent(owner)}__${encodeURIComponent(name)}/issues/by-id/${number}.json`, + `/github/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/issues/by-id/${number}.json`, + ] + for (const path of paths) { + try { + const { content } = await this.#mount.readFile(path) + return { + outcome: 'found', + issue: { repo: `${owner}/${name}`, number, path, content }, + } + } catch { + // Try the alternate canonical layout. If neither is readable, absence + // and transient sync failure are intentionally indistinguishable. + } + } + return { + outcome: 'indeterminate', + reason: `connected GitHub projection did not expose ${owner}/${name}#${number}`, + } + } + async publishPullRequest(input: GithubPublishPullRequestInput): Promise { const { owner, repo } = githubRepoParts(input.repo) const headRef = input.headRef ?? (input.clonePath @@ -188,7 +219,7 @@ export class RelayfileGithubConnectionWrite implements GithubConnectionWrite { operation: 'add' | 'remove' label: string author: 'app' - }): Promise { + }): Promise { const repoRoot = githubRepoRoot(input.repo) assertPositiveGithubNumber(input.number, 'issue') assertAppAuthor(input.author, 'issue label mutations') @@ -199,6 +230,10 @@ export class RelayfileGithubConnectionWrite implements GithubConnectionWrite { ? { operation: 'add', labels: [label] } : { operation: 'remove', label }, ) + // The durable operation proves App authorship and provider success, but + // the current adapter receipt does not distinguish a created mutation from + // an idempotent no-op. Callers must not infer ownership from it. + return 'acknowledged' } async updateIssue(input: GithubConnectionIssueUpdateInput): Promise { diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 3d165078..00711b3a 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -6,6 +6,7 @@ import { join } from 'node:path' import type { BrokerEvent, SendMessageInput, SpawnPtyInput } from '@agent-relay/harness-driver' import { + AppGithubWriteback, FactoryConfigSchema, checkFactoryLoopLiveness, closeProbePr, @@ -31,7 +32,7 @@ import { import { LatePlacementReleasedError, changeEventPath } from './factory' import { RelaySpawnAckTimeoutError } from '../fleet/relay-fleet-client' import { RelayfileOperationTimeoutError } from '../mount/relayfile-operation-timeout' -import type { AgentWorktree, AgentWorktreeCleanupInspection, AgentWorktreeManager, AgentWorktreeRepository, ChangeEvent, EventPage, GithubConnectionRead, GithubConnectionWrite, GithubIssueStatus, GithubPublishPullRequestInput, GithubWriteback, LinearWriteback, PreviewReference, PreviewStartInput, ProviderSyncStatus, RosterEntry, SlackWriteback, SpawnInput, SpawnResult } from '../ports' +import type { AgentWorktree, AgentWorktreeCleanupInspection, AgentWorktreeManager, AgentWorktreeRepository, ChangeEvent, EventPage, GithubConnectionRead, GithubConnectionWrite, GithubIssueStatus, GithubPublishPullRequestInput, GithubStatusClaimReceipt, GithubStatusWriteResult, GithubWriteback, LinearWriteback, PreviewReference, PreviewStartInput, ProviderSyncStatus, RosterEntry, SlackWriteback, SpawnInput, SpawnResult } from '../ports' import { FakeFleetClient, FakeMountClient, withDeadline } from '../testing' import type { CloseProbePrInput, GithubMergeGatePort, GithubMergeGateVerdict, GithubMergeInput, LinearIssue, VerificationGate, VerificationGateInput, VerificationVerdict } from '../index' import { dispatchIssueIdentity } from '../dispatch/work-unit-identity' @@ -542,7 +543,10 @@ class RecordingGithubWriteback implements GithubWriteback { this.comments.push({ key: issue.key, body }) } - async setStatus(issue: LinearIssue, status: GithubIssueStatus): Promise { + async setStatus( + issue: LinearIssue, + status: GithubIssueStatus, + ): Promise { this.statuses.push({ key: issue.key, status }) } @@ -994,6 +998,21 @@ class ManualClock { } } +class BlockingDispatchWritebackRetryClock extends ManualClock { + readonly retrySleepStarted = Promise.withResolvers() + readonly retrySleepGate = Promise.withResolvers() + #blocked = false + + override async sleep(ms: number): Promise { + await super.sleep(ms) + if (ms === 250 && !this.#blocked) { + this.#blocked = true + this.retrySleepStarted.resolve() + await this.retrySleepGate.promise + } + } +} + class RemoteLifecycleFleetClient extends FakeFleetClient { override readonly placementLocality = 'remote' as const override readonly lifecycleActionName = 'factory.lifecycle' @@ -1021,6 +1040,297 @@ class RemoteLifecycleFleetClient extends FakeFleetClient { } } +class LaterSpawnHangingFleetClient extends RemoteLifecycleFleetClient { + readonly laterSpawnGate = Promise.withResolvers() + terminalExitEmitted = false + + override async spawn(input: SpawnInput): Promise { + if (input.name.includes('-review') && !this.terminalExitEmitted) { + const implementer = this.spawns.find((spawn) => spawn.name.includes('-impl-')) + if (!implementer) throw new Error('reviewer spawn started before the implementer placement') + this.terminalExitEmitted = true + this.emitAgentExit(implementer.name, 'issue-done') + await this.laterSpawnGate.promise + } + return await super.spawn(input) + } +} + +class BlockingDispatchClaimGithubWriteback extends RecordingGithubWriteback { + readonly claimWriteStarted = Promise.withResolvers() + readonly claimWriteGate = Promise.withResolvers() + #blocked = false + #currentStatus: GithubIssueStatus = 'ready' + #currentClaimToken: string | undefined + #claimSequence = 0 + + override async getIssueStatus(): Promise { + return this.#currentStatus + } + + override async setStatus( + issue: LinearIssue, + status: GithubIssueStatus, + ): Promise { + if (status === 'in-progress' && !this.#blocked) { + this.#blocked = true + this.claimWriteStarted.resolve() + await this.claimWriteGate.promise + } + await super.setStatus(issue, status) + this.#currentStatus = status + return 'applied' + } + + async claimStatus( + issue: LinearIssue, + status: GithubIssueStatus, + ): Promise { + const result = await this.setStatus(issue, status) + const claimToken = `github-claim-${++this.#claimSequence}` + this.#currentClaimToken = claimToken + return { result, claimToken } + } + + async rollbackStatusClaim( + issue: LinearIssue, + status: GithubIssueStatus, + claimToken: string, + ): Promise<'reverted' | 'superseded' | 'unproven'> { + if (claimToken !== this.#currentClaimToken) return 'superseded' + if (this.#currentStatus !== status) return 'superseded' + await super.setStatus(issue, 'ready') + this.#currentStatus = 'ready' + return 'reverted' + } +} + +class BlockingDispatchCommentGithubWriteback extends BlockingDispatchClaimGithubWriteback { + readonly commentWriteStarted = Promise.withResolvers() + readonly commentWriteGate = Promise.withResolvers() + #blockedComment = false + + override async postComment(issue: LinearIssue, body: string): Promise { + if (!this.#blockedComment) { + this.#blockedComment = true + this.commentWriteStarted.resolve() + await this.commentWriteGate.promise + } + await super.postComment(issue, body) + } +} + +class RejectingBlockingDispatchCommentGithubWriteback extends BlockingDispatchClaimGithubWriteback { + readonly commentWriteStarted = Promise.withResolvers() + readonly commentWriteGate = Promise.withResolvers() + #blockedComment = false + + override async postComment(): Promise { + if (!this.#blockedComment) { + this.#blockedComment = true + this.commentWriteStarted.resolve() + await this.commentWriteGate.promise + } + throw new Error('provider rejected the in-flight dispatch comment') + } +} + +class FailOnceDispatchCommentGithubWriteback extends BlockingDispatchClaimGithubWriteback { + commentAttempts = 0 + + override async postComment(issue: LinearIssue, body: string): Promise { + this.commentAttempts += 1 + if (this.commentAttempts === 1) throw new Error('first dispatch comment attempt failed') + await super.postComment(issue, body) + } +} + +class RejectingBlockingDispatchClaimGithubWriteback extends BlockingDispatchClaimGithubWriteback { + override async setStatus( + issue: LinearIssue, + status: GithubIssueStatus, + ): Promise { + const result = await super.setStatus(issue, status) + if (status === 'in-progress') throw new Error('provider rejected the in-flight dispatch claim') + return result + } +} + +class UnprovenRollbackDispatchClaimGithubWriteback extends RejectingBlockingDispatchCommentGithubWriteback { + rollbackAttempts = 0 + readonly statusReadOptions: Array<{ requireFresh?: boolean; freshAfterMs?: number }> = [] + + override async getIssueStatus( + issue: LinearIssue, + opts?: { requireFresh?: boolean; freshAfterMs?: number }, + ): Promise { + this.statusReadOptions.push(opts ?? {}) + return await super.getIssueStatus(issue) + } + + override async rollbackStatusClaim(): Promise<'unproven'> { + this.rollbackAttempts += 1 + return 'unproven' + } +} + +class SupersededRollbackDispatchClaimGithubWriteback extends BlockingDispatchClaimGithubWriteback { + override async rollbackStatusClaim( + issue: LinearIssue, + ): Promise<'superseded'> { + await super.setStatus(issue, 'human-review') + return 'superseded' + } +} + +class BlockingDispatchLifecycleReleaseStateStore extends FileStateStore { + readonly releaseStarted = Promise.withResolvers() + readonly releaseGate = Promise.withResolvers() + #blocked = false + + override async releaseDispatchLifecycleLease( + workspaceId: string, + key: string, + owner: string, + epoch: number, + ): Promise { + if (!this.#blocked) { + this.#blocked = true + this.releaseStarted.resolve() + await this.releaseGate.promise + } + await super.releaseDispatchLifecycleLease(workspaceId, key, owner, epoch) + } +} + +class BlockingDispatchLifecycleRunningStateStore extends FileStateStore { + readonly runningSaveStarted = Promise.withResolvers() + readonly runningSaveGate = Promise.withResolvers() + #blocked = false + + override async saveDispatchLifecycle( + ...args: Parameters + ): Promise { + const lifecycle = args[5] + if (lifecycle.phase === 'running' && !this.#blocked) { + this.#blocked = true + this.runningSaveStarted.resolve() + await this.runningSaveGate.promise + } + return await super.saveDispatchLifecycle(...args) + } +} + +class RejectingPendingClaimFenceStateStore extends FileStateStore { + override async saveDispatchLifecycle( + ...args: Parameters + ): Promise { + if (args[5].dispatchClaim?.cancellationPending === true) return false + return await super.saveDispatchLifecycle(...args) + } +} + +class ThrowOncePendingClaimFenceStateStore extends FileStateStore { + pendingFailures = 0 + + override async saveDispatchLifecycle( + ...args: Parameters + ): Promise { + if (args[5].dispatchClaim?.cancellationPending === true && this.pendingFailures === 0) { + this.pendingFailures += 1 + throw new Error('transient cancellation fence persistence failure') + } + return await super.saveDispatchLifecycle(...args) + } +} + +class BlockingDispatchClaimLinearWriteback implements LinearWriteback { + readonly claimWriteStarted = Promise.withResolvers() + readonly claimWriteGate = Promise.withResolvers() + readonly states: Array<{ key: string; stateId: string }> = [] + readonly comments: Array<{ key: string; body: string }> = [] + #blocked = false + #currentStateId = ready + + async getIssueStateId(): Promise { + return this.#currentStateId + } + + setProviderState(stateId: string): void { + this.#currentStateId = stateId + } + + async setState(issue: LinearIssue, stateId: string): Promise<{ claimToken: string }> { + if (stateId === implementing && !this.#blocked) { + this.#blocked = true + this.claimWriteStarted.resolve() + await this.claimWriteGate.promise + } + this.states.push({ key: issue.key, stateId }) + this.#currentStateId = stateId + issue.stateId = stateId + return { claimToken: `linear-claim-${this.states.length}` } + } + + async compareAndSetState( + issue: LinearIssue, + expectedStateId: string, + claimToken: string, + stateId: string, + ): Promise<'applied' | 'superseded'> { + if (claimToken !== `linear-claim-${this.states.length}`) return 'superseded' + if (this.#currentStateId !== expectedStateId) return 'superseded' + await this.setState(issue, stateId) + return 'applied' + } + + async postComment(issue: LinearIssue, body: string): Promise { + this.comments.push({ key: issue.key, body }) + } + + async createIssue(): Promise<{ path: string }> { + throw new Error('not used') + } + + async verify(): Promise { + return true + } +} + +class UnprovenRollbackDispatchClaimLinearWriteback extends BlockingDispatchClaimLinearWriteback { + #claimIssue?: LinearIssue + #stateReadAvailable = true + + override async getIssueStateId(issue: LinearIssue): Promise { + if (!this.#stateReadAvailable) return undefined + return await super.getIssueStateId(issue) + } + + override async compareAndSetState(issue: LinearIssue): Promise<'unproven'> { + this.#claimIssue = issue + return 'unproven' + } + + setSparseDispatchSnapshotState(stateId: string): void { + if (!this.#claimIssue) throw new Error('dispatch claim issue was not captured') + this.#claimIssue.stateId = stateId + } + + setStateReadAvailable(available: boolean): void { + this.#stateReadAvailable = available + } +} + +class FailOnceDispatchCommentLinearWriteback extends BlockingDispatchClaimLinearWriteback { + commentAttempts = 0 + + override async postComment(issue: LinearIssue, body: string): Promise { + this.commentAttempts += 1 + if (this.commentAttempts === 1) throw new Error('first Linear dispatch comment attempt failed') + await super.postComment(issue, body) + } +} + class DurableRemoteLifecycleFleetClient extends RemoteLifecycleFleetClient { override readonly durableOwnership = true } @@ -11265,6 +11575,1396 @@ describe('FactoryLoop', () => { } }) + it('settles post-spawn completion waits when a later spawn reaches the held-agent deadline', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-deadline-')) + const number = 1252 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LaterSpawnHangingFleetClient() + const githubWriteback = new RecordingGithubWriteback() + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }), + triage: new StaticTriage(), + githubWriteback, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await vi.waitFor(() => expect(fleet.terminalExitEmitted).toBe(true), { timeout: 4_000 }) + await vi.waitFor(() => expect(githubWriteback.statuses).toContainEqual({ + key: String(number), + status: 'human-review', + }), { timeout: 4_000 }) + // Completion has written provider terminal state and is now waiting for + // the post-spawn observation, while the reviewer spawn never returns. + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + await vi.waitFor(() => expect(fleet.releases).toContainEqual({ + name: `ar-${number}-impl-pear`, + reason: 'held-past-deadline', + }), { timeout: 4_000 }) + + // The deadline settlement must have drained the completion exit before + // shutdown supplies its independent settlement backstop. + await withDeadline(factory.stop(), 2_000, 'stop remained blocked after held-agent abandonment') + stopped = true + expect(factory.status().counters.postSpawnWaitsSettledByStop).toBeUndefined() + + fleet.laterSpawnGate.resolve() + await withDeadline(run, 8_000, 'late reviewer spawn did not unwind after abandonment') + } finally { + fleet.laterSpawnGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('settles post-spawn completion waits before shutdown drains agent exits', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-stop-')) + const number = 1253 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LaterSpawnHangingFleetClient() + const githubWriteback = new RecordingGithubWriteback() + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }), + triage: new StaticTriage(), + githubWriteback, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await vi.waitFor(() => expect(fleet.terminalExitEmitted).toBe(true), { timeout: 4_000 }) + await vi.waitFor(() => expect(githubWriteback.statuses).toContainEqual({ + key: String(number), + status: 'human-review', + }), { timeout: 4_000 }) + expect(factory.status().counters.postSpawnWaitsSettledByAbandonment).toBeUndefined() + + await withDeadline(factory.stop(), 2_000, 'stop remained blocked on a post-spawn completion wait') + stopped = true + expect(factory.status().counters.postSpawnWaitsSettledByStop).toBe(1) + + fleet.laterSpawnGate.resolve() + await withDeadline(run, 8_000, 'late reviewer spawn did not unwind after stop') + } finally { + fleet.laterSpawnGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('retains held agents until an unresolved dispatch claim conclusively compensates', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-claim-deadline-')) + const number = 1254 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new RemoteLifecycleFleetClient() + const githubWriteback = new BlockingDispatchClaimGithubWriteback() + const stateStore = new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + // Both planned agents have returned and dispatch has synchronously + // published claimStarted before entering this blocked provider write. + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + + // Prove completion reached the claim fence itself, not merely an earlier + // read or the independent post-spawn observation wait. + await vi.waitFor(() => expect(factory.status().counters.postSpawnDispatchClaimFenceWaits) + .toBe(1), { timeout: 4_000 }) + expect(githubWriteback.statuses).toEqual([]) + expect(fleet.releases).toEqual([]) + + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + await vi.waitFor(() => expect(factory.status().counters.postSpawnDispatchClaimRecoveryRetentions) + .toBe(1), { timeout: 5_000 }) + expect(fleet.releases).toEqual([]) + await vi.waitFor(async () => expect(await stateStore.getDispatchLifecycle( + 'factory-test', + dispatchIssueIdentity({ + uuid: `AgentWorkforce/pear#${number}`, + key: String(number), + path, + }), + )).toMatchObject({ + dispatchClaim: { + cancellationBlocked: true, + cancellationPending: true, + }, + }), { timeout: 4_000 }) + expect(githubWriteback.statuses).not.toContainEqual({ key: String(number), status: 'human-review' }) + expect(fleet.releases).not.toContainEqual(expect.objectContaining({ reason: 'issue-human-review' })) + + // Only the original caller can turn the unresolved handoff into a + // conclusively compensated claim. A pre-claim Ready observation cannot + // release the agents while this write may still land. + githubWriteback.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'blocked dispatch claim did not unwind after abandonment') + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + { key: String(number), status: 'ready' }, + ]) + expect(githubWriteback.comments).toEqual([]) + await vi.waitFor(() => expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'held-past-deadline' }, + { name: `ar-${number}-review-pear`, reason: 'held-past-deadline' }, + ]), { timeout: 5_000 }) + expect(factory.status().inFlight).toEqual([]) + + await factory.stop() + stopped = true + expect(factory.status().counters.postSpawnWaitsSettledByStop).toBeUndefined() + } finally { + githubWriteback.claimWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('re-arms abandonment after an early cancellation-fence persistence failure', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-claim-retry-arm-')) + const number = 1270 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new RemoteLifecycleFleetClient() + const githubWriteback = new BlockingDispatchClaimGithubWriteback() + const stateStore = new ThrowOncePendingClaimFenceStateStore({ + batchSize: 2, + watchStatePath: join(root, 'state.json'), + }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + await vi.waitFor(() => expect(stateStore.pendingFailures).toBe(1), { timeout: 6_000 }) + expect(fleet.releases).toEqual([]) + + githubWriteback.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'dispatch did not settle after the persistence failure') + await vi.waitFor(() => expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'held-past-deadline' }, + { name: `ar-${number}-review-pear`, reason: 'held-past-deadline' }, + ]), { timeout: 5_000 }) + expect(factory.status().inFlight).toEqual([]) + + await factory.stop() + stopped = true + } finally { + githubWriteback.claimWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('keeps a claim cancellable while its dispatch comment is still pending', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-comment-deadline-')) + const number = 1257 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new RemoteLifecycleFleetClient() + const githubWriteback = new BlockingDispatchCommentGithubWriteback() + const stateStore = new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + githubWriteback.claimWriteGate.resolve() + await withDeadline(githubWriteback.commentWriteStarted.promise, 4_000, 'dispatch comment did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + await vi.waitFor(() => expect(factory.status().counters.postSpawnDispatchClaimFenceWaits) + .toBe(1), { timeout: 4_000 }) + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + + githubWriteback.commentWriteGate.resolve() + await withDeadline(run, 8_000, 'late dispatch comment did not unwind after abandonment') + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + { key: String(number), status: 'ready' }, + ]) + expect(githubWriteback.comments).toHaveLength(1) + expect(factory.status().counters.dispatched).toBeUndefined() + await vi.waitFor(async () => expect(await stateStore.getDispatchLifecycle( + 'factory-test', + dispatchIssueIdentity({ uuid: String(number), key: String(number), path }), + )).toMatchObject({ phase: 'abandoned' }), { timeout: 4_000 }) + + await factory.stop() + stopped = true + } finally { + githubWriteback.claimWriteGate.resolve() + githubWriteback.commentWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('does not abandon held agents when a rejected GitHub claim rollback is unproven', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-unproven-rollback-deadline-')) + const number = 1266 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new RemoteLifecycleFleetClient() + const githubWriteback = new UnprovenRollbackDispatchClaimGithubWriteback() + const stateStore = new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + githubWriteback.claimWriteGate.resolve() + await withDeadline(githubWriteback.commentWriteStarted.promise, 4_000, 'dispatch comment did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + githubWriteback.commentWriteGate.resolve() + await withDeadline(run, 8_000, 'unproven claim rollback did not unwind dispatch') + + expect(githubWriteback.rollbackAttempts).toBe(3) + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + ]) + expect(githubWriteback.comments).toEqual([]) + expect(factory.status().counters.postSpawnDispatchClaimCompensationFailures).toBe(1) + expect(factory.status().counters.postSpawnDispatchClaimRecoveryRetentions).toBe(1) + expect(factory.status().inFlight).toHaveLength(1) + expect(fleet.releases).toEqual([]) + await expect(stateStore.getDispatchLifecycle('factory-test', dispatchIssueIdentity({ + uuid: `AgentWorkforce/pear#${number}`, + key: String(number), + path, + }))).resolves.toMatchObject({ + phase: 'running', + dispatchClaim: { + state: 'degraded', + claimStartedAtMs: expect.any(Number), + cancellationBlocked: true, + write: 'rejected dispatch claim rollback', + deadLettered: true, + }, + }) + await vi.waitFor(() => expect(githubWriteback.statusReadOptions).toContainEqual({ + requireFresh: true, + freshAfterMs: expect.any(Number), + }), { timeout: 4_000 }) + + await withDeadline(factory.stop(), 4_000, 'stop did not retain the uncompensated held dispatch') + stopped = true + expect(factory.status().inFlight).toHaveLength(1) + expect(fleet.releases).toEqual([]) + } finally { + githubWriteback.claimWriteGate.resolve() + githubWriteback.commentWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('refuses a durable shutdown handoff when the unresolved claim fence is not saved', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-claim-handoff-rejected-')) + const number = 1269 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LocalLifecycleFleetClient() + const githubWriteback = new BlockingDispatchClaimGithubWriteback() + const stateStore = new RejectingPendingClaimFenceStateStore({ + batchSize: 2, + watchStatePath: join(root, 'state.json'), + }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + + await expect(withDeadline( + factory.stop(), + 6_000, + 'shutdown did not report the rejected claim-fence handoff', + )).rejects.toThrow(/refusing shutdown handoff/u) + expect(fleet.releases).toEqual([]) + expect(factory.status().inFlight).toHaveLength(1) + + githubWriteback.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'dispatch did not compensate after the rejected handoff') + await withDeadline(factory.stop(), 4_000, 'shutdown did not recover after compensation settled') + stopped = true + expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'factory-stopped' }, + { name: `ar-${number}-review-pear`, reason: 'factory-stopped' }, + ]) + } finally { + githubWriteback.claimWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('recovers an App-backed cancellation block after provider status supersession', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-app-status-recovery-')) + const number = 1268 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new RemoteLifecycleFleetClient() + const stateStore = new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }) + const commentWriteStarted = Promise.withResolvers() + const commentWriteGate = Promise.withResolvers() + let providerStatus: GithubIssueStatus = 'ready' + const githubWrite: GithubConnectionWrite = { + publishPullRequest: async () => { throw new Error('not used') }, + closePullRequest: async () => undefined, + postIssueComment: async () => { + commentWriteStarted.resolve() + await commentWriteGate.promise + }, + ensureRepositoryLabel: async () => undefined, + mutateIssueLabel: async ({ operation, label }) => { + if (operation === 'add' && label === 'factory:in-progress') providerStatus = 'in-progress' + if (operation === 'add' && label === 'factory:human-review') providerStatus = 'human-review' + return operation === 'add' ? 'applied' : 'already-matched' + }, + updateIssue: async () => undefined, + } + const githubRead: GithubConnectionRead = { + getIssue: async () => ({ + outcome: 'found', + issue: { + repo: 'AgentWorkforce/pear', + number, + path, + content: { + payload: { + labels: providerStatus === 'ready' + ? [{ name: 'factory' }] + : [{ name: 'factory' }, { name: `factory:${providerStatus}` }], + }, + }, + }, + }), + } + const factory = createFactory(config({ + issueSource: 'github', + github: { identity: 'app' }, + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback: new AppGithubWriteback(githubWrite, githubRead), + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(commentWriteStarted.promise, 4_000, 'App dispatch comment did not start') + expect(providerStatus).toBe('in-progress') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + + commentWriteGate.resolve() + await withDeadline(run, 8_000, 'App-backed cancellation did not unwind dispatch') + expect(factory.status().counters.postSpawnDispatchClaimCompensationFailures).toBe(1) + expect(factory.status().counters.postSpawnDispatchClaimRecoveryRetentions).toBe(1) + expect(factory.status().inFlight).toHaveLength(1) + expect(fleet.releases).toEqual([]) + + providerStatus = 'human-review' + await vi.waitFor(() => expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'held-past-deadline' }, + { name: `ar-${number}-review-pear`, reason: 'held-past-deadline' }, + ]), { timeout: 5_000 }) + await vi.waitFor(async () => expect(await stateStore.getDispatchLifecycle( + 'factory-test', + dispatchIssueIdentity({ uuid: `AgentWorkforce/pear#${number}`, key: String(number), path }), + )).toMatchObject({ + phase: 'abandoned', + dispatchClaim: expect.not.objectContaining({ cancellationBlocked: true }), + }), { timeout: 4_000 }) + expect(factory.status().inFlight).toEqual([]) + + await factory.stop() + stopped = true + } finally { + commentWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('keeps a claim cancellable while the running lifecycle publication is pending', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-running-save-deadline-')) + const number = 1260 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new RemoteLifecycleFleetClient() + const githubWriteback = new BlockingDispatchClaimGithubWriteback() + const stateStore = new BlockingDispatchLifecycleRunningStateStore({ + batchSize: 2, + watchStatePath: join(root, 'state.json'), + }) + const notifications: string[] = [] + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + hooks: { + onTicketDispatch: { + notify: [{ surface: 'slack', channel: 'C123' }], + }, + }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + ticketDispatchDelivery: { + async slack({ text }) { + notifications.push(text) + }, + async telegram() {}, + }, + }) + githubWriteback.claimWriteGate.resolve() + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(stateStore.runningSaveStarted.promise, 4_000, 'running lifecycle save did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + await vi.waitFor(() => expect(factory.status().counters.postSpawnDispatchClaimFenceWaits) + .toBe(1), { timeout: 4_000 }) + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + + stateStore.runningSaveGate.resolve() + const runResult = await withDeadline(run, 8_000, 'late running lifecycle save did not unwind') + expect(runResult).toMatchObject({ + dispatched: [], + skipped: [expect.objectContaining({ reason: 'dispatch terminated during post-spawn claim' })], + }) + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + { key: String(number), status: 'ready' }, + ]) + expect(notifications).toEqual([]) + expect(factory.status().counters.dispatched).toBeUndefined() + await vi.waitFor(async () => expect(await stateStore.getDispatchLifecycle( + 'factory-test', + dispatchIssueIdentity({ uuid: String(number), key: String(number), path }), + )).toMatchObject({ phase: 'abandoned' }), { timeout: 4_000 }) + + await factory.stop() + stopped = true + } finally { + githubWriteback.claimWriteGate.resolve() + stateStore.runningSaveGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('preserves a newer GitHub status instead of unconditionally rolling it back', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-superseded-rollback-')) + const number = 1258 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new RemoteLifecycleFleetClient() + const githubWriteback = new SupersededRollbackDispatchClaimGithubWriteback() + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }), + triage: new StaticTriage(), + githubWriteback, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + + githubWriteback.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'superseded dispatch claim did not unwind') + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + { key: String(number), status: 'human-review' }, + ]) + expect(githubWriteback.comments).toEqual([]) + expect(factory.status().counters.postSpawnDispatchClaimCompensationFailures).toBeUndefined() + + await factory.stop() + stopped = true + } finally { + githubWriteback.claimWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('rolls back a late Linear dispatch claim without posting its comment after abandonment', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-linear-claim-deadline-')) + const number = 1256 + const fleet = new RemoteLifecycleFleetClient() + const linear = new BlockingDispatchClaimLinearWriteback() + const factory = createFactory(config({ + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ [issuePath(number)]: issueFile(number) }), + fleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }), + triage: new StaticTriage(), + linear, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(linear.claimWriteStarted.promise, 4_000, 'Linear dispatch claim did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + await vi.waitFor(() => expect(factory.status().counters.postSpawnDispatchClaimFenceWaits) + .toBe(1), { timeout: 4_000 }) + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + + linear.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'late Linear dispatch claim did not unwind after abandonment') + expect(factory.status().inFlight).toEqual([]) + expect(linear.states).toEqual([ + { key: `AR-${number}`, stateId: implementing }, + { key: `AR-${number}`, stateId: ready }, + ]) + expect(linear.comments).toEqual([]) + + await factory.stop() + stopped = true + } finally { + linear.claimWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('releases a blocked Linear claim only after the provider state is superseded', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-linear-unproven-recovery-')) + const number = 1267 + const path = issuePath(number) + const fleet = new RemoteLifecycleFleetClient() + const linear = new UnprovenRollbackDispatchClaimLinearWriteback() + const stateStore = new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }) + const factory = createFactory(config({ + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 1_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ [path]: issueFile(number) }), + fleet, + stateStore, + triage: new StaticTriage(), + linear, + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(linear.claimWriteStarted.promise, 4_000, 'Linear dispatch claim did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + await vi.waitFor(() => expect(factory.status().counters.postSpawnWaitsSettledByAbandonment) + .toBe(1), { timeout: 8_000 }) + + linear.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'unproven Linear rollback did not unwind dispatch') + expect(factory.status().counters.postSpawnDispatchClaimCompensationFailures).toBe(1) + expect(factory.status().counters.postSpawnDispatchClaimRecoveryRetentions).toBe(1) + expect(factory.status().inFlight).toHaveLength(1) + expect(fleet.releases).toEqual([]) + await expect(stateStore.getDispatchLifecycle('factory-test', dispatchIssueIdentity({ + uuid: `uuid-${number}`, + key: `AR-${number}`, + path, + }))).resolves.toMatchObject({ + phase: 'running', + dispatchClaim: { + state: 'degraded', + cancellationBlocked: true, + write: 'rejected dispatch claim rollback', + deadLettered: true, + }, + }) + + // A sparse dispatch snapshot cannot substitute for an unavailable + // canonical read, even when that stale snapshot appears superseded. The + // first abandonment retry must keep every placement retained. + linear.setProviderState(humanReview) + linear.setSparseDispatchSnapshotState(humanReview) + linear.setStateReadAvailable(false) + await new Promise((resolve) => { setTimeout(resolve, 1_200) }) + expect(factory.status().inFlight).toHaveLength(1) + expect(fleet.releases).toEqual([]) + + linear.setStateReadAvailable(true) + await vi.waitFor(() => expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'held-past-deadline' }, + { name: `ar-${number}-review`, reason: 'held-past-deadline' }, + ]), { timeout: 5_000 }) + await vi.waitFor(async () => expect(await stateStore.getDispatchLifecycle( + 'factory-test', + dispatchIssueIdentity({ uuid: `uuid-${number}`, key: `AR-${number}`, path }), + )).toMatchObject({ + phase: 'abandoned', + dispatchClaim: expect.not.objectContaining({ cancellationBlocked: true }), + }), { timeout: 4_000 }) + expect(factory.status().inFlight).toEqual([]) + + await factory.stop() + stopped = true + } finally { + linear.claimWriteGate.resolve() + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('does not retry a dispatch comment after shutdown rejects its claim fence', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-comment-retry-stop-')) + const number = 1263 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LocalLifecycleFleetClient() + const githubWriteback = new FailOnceDispatchCommentGithubWriteback() + const clock = new BlockingDispatchWritebackRetryClock() + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }), + triage: new StaticTriage(), + githubWriteback, + clock, + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stop: Promise | undefined + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + githubWriteback.claimWriteGate.resolve() + await withDeadline(clock.retrySleepStarted.promise, 4_000, 'dispatch comment did not enter retry delay') + + stop = factory.stop() + clock.retrySleepGate.resolve() + const runResult = await withDeadline(run, 8_000, 'cancelled comment retry did not unwind') + expect(runResult).toMatchObject({ name: 'PostSpawnDispatchWaitRejectedError' }) + await withDeadline(stop, 4_000, 'stop did not drain the rejected dispatch') + stopped = true + + expect(githubWriteback.commentAttempts).toBe(1) + expect(githubWriteback.comments).toEqual([]) + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + { key: String(number), status: 'ready' }, + ]) + expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'factory-stopped' }, + { name: `ar-${number}-review-pear`, reason: 'factory-stopped' }, + ]) + expect(factory.status().inFlight).toEqual([]) + } finally { + githubWriteback.claimWriteGate.resolve() + clock.retrySleepGate.resolve() + if (!stopped) await (stop ?? factory.stop()) + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('does not retry a Linear dispatch comment after shutdown rejects its claim fence', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-linear-comment-retry-stop-')) + const number = 1264 + const fleet = new LocalLifecycleFleetClient() + const linear = new FailOnceDispatchCommentLinearWriteback() + const clock = new BlockingDispatchWritebackRetryClock() + const factory = createFactory(config({ + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ [issuePath(number)]: issueFile(number) }), + fleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }), + triage: new StaticTriage(), + linear, + clock, + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stop: Promise | undefined + let stopped = false + try { + await withDeadline(linear.claimWriteStarted.promise, 4_000, 'Linear dispatch claim did not start') + linear.claimWriteGate.resolve() + await withDeadline(clock.retrySleepStarted.promise, 4_000, 'Linear comment did not enter retry delay') + + stop = factory.stop() + clock.retrySleepGate.resolve() + const runResult = await withDeadline(run, 8_000, 'cancelled Linear comment retry did not unwind') + expect(runResult).toMatchObject({ name: 'PostSpawnDispatchWaitRejectedError' }) + await withDeadline(stop, 4_000, 'stop did not drain the rejected Linear dispatch') + stopped = true + + expect(linear.commentAttempts).toBe(1) + expect(linear.comments).toEqual([]) + expect(linear.states).toEqual([ + { key: `AR-${number}`, stateId: implementing }, + { key: `AR-${number}`, stateId: ready }, + ]) + expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'factory-stopped' }, + { name: `ar-${number}-review`, reason: 'factory-stopped' }, + ]) + expect(factory.status().inFlight).toEqual([]) + } finally { + linear.claimWriteGate.resolve() + clock.retrySleepGate.resolve() + if (!stopped) await (stop ?? factory.stop()) + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('rolls back an applied GitHub claim when a later cancelled dispatch comment rejects', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-comment-failure-stop-')) + const number = 1262 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LocalLifecycleFleetClient() + const githubWriteback = new RejectingBlockingDispatchCommentGithubWriteback() + const stateStore = new BlockingDispatchLifecycleReleaseStateStore({ + batchSize: 2, + watchStatePath: join(root, 'state.json'), + }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + clock: new ManualClock(), + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stop: Promise | undefined + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + githubWriteback.claimWriteGate.resolve() + await withDeadline(githubWriteback.commentWriteStarted.promise, 4_000, 'dispatch comment did not start') + + stop = factory.stop() + githubWriteback.commentWriteGate.resolve() + const runResult = await withDeadline(run, 8_000, 'comment failure did not compensate the claim') + expect(runResult).toMatchObject({ name: 'PostSpawnDispatchWaitRejectedError' }) + if (!(runResult instanceof Error) || !('compensationError' in runResult)) { + throw new Error('expected a rejected post-spawn dispatch with compensation detail') + } + expect(runResult.compensationError).toBeUndefined() + expect(factory.status().counters.postSpawnDispatchClaimCompensationFailures).toBeUndefined() + await withDeadline(stateStore.releaseStarted.promise, 2_000, 'stop did not drain before lifecycle release') + expect(factory.status().inFlight).toHaveLength(1) + + stateStore.releaseGate.resolve() + await withDeadline(stop, 2_000, 'stop did not release agents after claim compensation') + stopped = true + expect(factory.status().inFlight).toEqual([]) + expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'factory-stopped' }, + { name: `ar-${number}-review-pear`, reason: 'factory-stopped' }, + ]) + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + { key: String(number), status: 'ready' }, + ]) + expect(githubWriteback.comments).toEqual([]) + } finally { + githubWriteback.claimWriteGate.resolve() + githubWriteback.commentWriteGate.resolve() + stateStore.releaseGate.resolve() + if (!stopped) await (stop ?? factory.stop()) + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('retains the lifecycle and local placements when conditional claim rollback is unproven', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-rollback-failure-stop-')) + const number = 1259 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LocalLifecycleFleetClient() + const githubWriteback = new UnprovenRollbackDispatchClaimGithubWriteback() + const registryPath = join(root, 'registry.json') + const stateStore = new BlockingDispatchLifecycleReleaseStateStore({ + batchSize: 2, + watchStatePath: join(root, 'state.json'), + }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath, + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + clock: new ManualClock(), + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stop: Promise | undefined + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + githubWriteback.claimWriteGate.resolve() + await withDeadline(githubWriteback.commentWriteStarted.promise, 4_000, 'dispatch comment did not start') + + stop = factory.stop() + githubWriteback.commentWriteGate.resolve() + const runResult = await withDeadline(run, 8_000, 'failed rollback did not preserve cancellation') + expect(runResult).toMatchObject({ name: 'PostSpawnDispatchWaitRejectedError' }) + expect(runResult).toHaveProperty('compensationError') + if (!(runResult instanceof Error) || !('compensationError' in runResult)) { + throw new Error('expected a rejected post-spawn dispatch with compensation detail') + } + expect(runResult.compensationError).toBeInstanceOf(AggregateError) + expect((runResult.compensationError as AggregateError).errors).toEqual([ + expect.objectContaining({ message: 'provider rejected the in-flight dispatch comment' }), + expect.objectContaining({ message: 'GitHub rejected dispatch claim rollback could not prove ownership' }), + ]) + expect(githubWriteback.rollbackAttempts).toBe(3) + expect(factory.status().counters.postSpawnDispatchClaimCompensationFailures).toBe(1) + expect(factory.status().counters.postSpawnDispatchClaimRecoveryRetentions).toBe(1) + await withDeadline(stateStore.releaseStarted.promise, 2_000, 'stop did not drain before lifecycle release') + expect(factory.status().inFlight).toHaveLength(1) + + stateStore.releaseGate.resolve() + await withDeadline(stop, 2_000, 'stop did not retain agents after unproven rollback') + stopped = true + expect(factory.status().inFlight).toHaveLength(1) + expect(fleet.releases).toEqual([]) + await expect(stateStore.getDispatchLifecycle('factory-test', dispatchIssueIdentity({ + uuid: `AgentWorkforce/pear#${number}`, + key: String(number), + path, + }))).resolves.toMatchObject({ + phase: 'running', + dispatchClaim: { + state: 'degraded', + cancellationBlocked: true, + write: 'rejected dispatch claim rollback', + deadLettered: true, + }, + }) + expect((await readFactoryInFlightRegistry(registryPath))?.agents).toEqual([ + expect.objectContaining({ + name: `ar-${number}-impl-pear`, + dispatchClaim: expect.objectContaining({ cancellationBlocked: true }), + }), + expect.objectContaining({ + name: `ar-${number}-review-pear`, + dispatchClaim: expect.objectContaining({ cancellationBlocked: true }), + }), + ]) + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + ]) + expect(githubWriteback.comments).toEqual([]) + } finally { + githubWriteback.claimWriteGate.resolve() + githubWriteback.commentWriteGate.resolve() + stateStore.releaseGate.resolve() + if (!stopped) await (stop ?? factory.stop()) + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('retains local placements when a cancelled in-flight claim rejects with a provider error', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-provider-failure-stop-')) + const number = 1261 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LocalLifecycleFleetClient() + const githubWriteback = new RejectingBlockingDispatchClaimGithubWriteback() + const stateStore = new BlockingDispatchLifecycleReleaseStateStore({ + batchSize: 2, + watchStatePath: join(root, 'state.json'), + }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + clock: new ManualClock(), + logger: { error: () => {}, warn: () => {} }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stop: Promise | undefined + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + + stop = factory.stop() + githubWriteback.claimWriteGate.resolve() + const runResult = await withDeadline(run, 8_000, 'provider failure bypassed post-spawn cancellation') + expect(runResult).toMatchObject({ name: 'PostSpawnDispatchWaitRejectedError' }) + expect(factory.status().counters.postSpawnDispatchClaimCompensationFailures).toBe(1) + await withDeadline(stateStore.releaseStarted.promise, 2_000, 'stop did not drain before lifecycle release') + expect(factory.status().inFlight).toHaveLength(1) + + stateStore.releaseGate.resolve() + await withDeadline(stop, 2_000, 'stop did not retain agents after the ambiguous provider failure') + stopped = true + expect(factory.status().inFlight).toHaveLength(1) + expect(fleet.releases).toEqual([]) + await expect(stateStore.getDispatchLifecycle('factory-test', dispatchIssueIdentity({ + uuid: `AgentWorkforce/pear#${number}`, + key: String(number), + path, + }))).resolves.toMatchObject({ + phase: 'running', + dispatchClaim: { + state: 'degraded', + cancellationBlocked: true, + deadLettered: true, + }, + }) + expect(githubWriteback.statuses).toEqual(Array.from( + { length: 3 }, + () => ({ key: String(number), status: 'in-progress' as const }), + )) + expect(githubWriteback.comments).toEqual([]) + } finally { + githubWriteback.claimWriteGate.resolve() + stateStore.releaseGate.resolve() + if (!stopped) await (stop ?? factory.stop()) + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('settles a completion blocked on the dispatch-claim fence before shutdown drains exits', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-claim-stop-')) + const number = 1255 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LocalLifecycleFleetClient() + const githubWriteback = new BlockingDispatchClaimGithubWriteback() + const stateStore = new BlockingDispatchLifecycleReleaseStateStore({ + batchSize: 2, + watchStatePath: join(root, 'state.json'), + }) + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stop: Promise | undefined + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + fleet.emitAgentExit(`ar-${number}-impl-pear`, 'issue-done') + await vi.waitFor(() => expect(factory.status().counters.postSpawnDispatchClaimFenceWaits) + .toBe(1), { timeout: 4_000 }) + expect(githubWriteback.statuses).toEqual([]) + expect(factory.status().counters.postSpawnWaitsSettledByAbandonment).toBeUndefined() + + stop = factory.stop() + expect(factory.status().counters.postSpawnWaitsSettledByStop).toBe(1) + expect(githubWriteback.statuses).not.toContainEqual({ key: String(number), status: 'human-review' }) + expect(fleet.releases).not.toContainEqual(expect.objectContaining({ reason: 'issue-human-review' })) + + // Shutdown must drain the rejected provider claim before relinquishing + // the lifecycle lease or taking its in-flight release snapshot. + githubWriteback.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'blocked dispatch claim did not unwind after stop') + await withDeadline(stateStore.releaseStarted.promise, 2_000, 'stop did not drain before lifecycle release') + expect(fleet.releases).toEqual([]) + expect(factory.status().inFlight).toHaveLength(1) + + stateStore.releaseGate.resolve() + await withDeadline(stop, 2_000, 'stop did not release the retained local placements') + stopped = true + expect(factory.status().inFlight).toEqual([]) + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + { key: String(number), status: 'ready' }, + ]) + expect(githubWriteback.comments).toEqual([]) + expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'factory-stopped' }, + { name: `ar-${number}-review-pear`, reason: 'factory-stopped' }, + ]) + } finally { + githubWriteback.claimWriteGate.resolve() + stateStore.releaseGate.resolve() + if (!stopped) await (stop ?? factory.stop()) + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('bounds a rejected dispatch drain when the provider claim never settles', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-claim-stop-timeout-')) + const number = 1264 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new LocalLifecycleFleetClient() + const githubWriteback = new BlockingDispatchClaimGithubWriteback() + const registryPath = join(root, 'registry.json') + const stateStore = new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }) + const warnings: unknown[][] = [] + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath, + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + logger: { warn: (...args: unknown[]) => warnings.push(args) }, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stopped = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + + await withDeadline(factory.stop(), 6_000, 'stop remained blocked on the provider claim') + stopped = true + + expect(factory.status().counters.postSpawnDispatchClaimDrainTimeouts).toBe(1) + expect(factory.status().counters.postSpawnDispatchClaimRecoveryRetentions).toBe(1) + expect(warnings).toContainEqual([ + '[factory] rejected post-spawn dispatch compensation timed out; continuing shutdown', + { dispatches: 1, timeoutMs: 2_500 }, + ]) + expect(githubWriteback.statuses).toEqual([]) + expect(factory.status().inFlight).toEqual([ + expect.objectContaining({ uuid: `AgentWorkforce/pear#${number}` }), + ]) + expect(fleet.releases).toEqual([]) + await expect(stateStore.getDispatchLifecycle('factory-test', dispatchIssueIdentity({ + uuid: `AgentWorkforce/pear#${number}`, + key: String(number), + path, + }))).resolves.toMatchObject({ + phase: 'running', + lease: { + owner: expect.any(String), + epoch: expect.any(Number), + }, + dispatchClaim: { + state: 'degraded', + cancellationBlocked: true, + cancellationPending: true, + write: 'rejected dispatch claim rollback', + deadLettered: true, + }, + }) + expect((await readFactoryInFlightRegistry(registryPath))?.agents).toEqual([ + expect.objectContaining({ + name: `ar-${number}-impl-pear`, + dispatchClaim: expect.objectContaining({ cancellationBlocked: true }), + }), + expect.objectContaining({ + name: `ar-${number}-review-pear`, + dispatchClaim: expect.objectContaining({ cancellationBlocked: true }), + }), + ]) + } finally { + githubWriteback.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'timed-out dispatch did not unwind after its provider recovered') + // Shutdown's release snapshot deliberately skipped this ambiguous + // claim. Even if compensation proves safe after stop has returned, keep + // the lifecycle and placements for successor adoption rather than + // silently abandoning agents that were never released. + expect(factory.status().inFlight).toHaveLength(1) + await vi.waitFor(async () => { + const lifecycle = await stateStore.getDispatchLifecycle('factory-test', dispatchIssueIdentity({ + uuid: `AgentWorkforce/pear#${number}`, + key: String(number), + path, + })) + expect(lifecycle).toBeDefined() + expect(lifecycle?.dispatchClaim?.cancellationPending).toBeUndefined() + }, { timeout: 4_000 }) + if (!stopped) await factory.stop() + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + + it('keeps draining a rejected claim for a non-durable fleet until compensation settles', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-post-spawn-claim-stop-nondurable-')) + const number = 1265 + const path = githubIssuePath('AgentWorkforce', 'pear', number) + const fleet = new FakeFleetClient() + const githubWriteback = new BlockingDispatchClaimGithubWriteback() + const factory = createFactory(config({ + issueSource: 'github', + mergePolicy: 'never', + terminalState: 'human-review', + dispatch: { agentHoldTimeoutMs: 60_000, agentlessHoldTimeoutMs: 60_000 }, + loop: { + heartbeatPath: join(root, 'heartbeat.json'), + registryPath: join(root, 'registry.json'), + }, + }), { + mount: new FakeMountClient({ + [path]: githubIssueFile(number, { labels: ['factory', 'pear'] }), + }), + fleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath: join(root, 'state.json') }), + triage: new StaticTriage(), + githubWriteback, + }) + const run = factory.runOnce().catch((error: unknown) => error) + let stop: Promise | undefined + let stopSettled = false + try { + await withDeadline(githubWriteback.claimWriteStarted.promise, 4_000, 'dispatch claim did not start') + + stop = factory.stop().then(() => { stopSettled = true }) + await new Promise((resolve) => { setTimeout(resolve, 2_700) }) + + expect(stopSettled).toBe(false) + expect(factory.status().counters.postSpawnDispatchClaimDrainTimeouts).toBeUndefined() + expect(fleet.releases).toEqual([]) + + githubWriteback.claimWriteGate.resolve() + await withDeadline(run, 8_000, 'non-durable dispatch did not compensate after its provider recovered') + await withDeadline(stop, 4_000, 'stop did not finish after non-durable compensation settled') + + expect(githubWriteback.statuses).toEqual([ + { key: String(number), status: 'in-progress' }, + { key: String(number), status: 'ready' }, + ]) + expect(fleet.releases).toEqual([ + { name: `ar-${number}-impl-pear`, reason: 'factory-stopped' }, + { name: `ar-${number}-review-pear`, reason: 'factory-stopped' }, + ]) + expect(factory.status().inFlight).toEqual([]) + } finally { + githubWriteback.claimWriteGate.resolve() + await (stop ?? factory.stop()) + await rm(root, { recursive: true, force: true }) + } + }, 30_000) + // #303: a lifecycle that reached a slot-occupying phase and never had a // live agent placement has no `heldSinceAtMs`, so both halves of the // held-agent reaper skipped it. With batchSize 1 that one row held the only diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index efa118fe..9d285392 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -423,6 +423,12 @@ const CLARIFICATION_ESCALATION_LEASE_MS = 2 * 60_000 const CLARIFICATION_ESCALATION_RETRY_MS = 5_000 const CLARIFICATION_STALE_WARN_MS = 7 * 24 * 60 * 60_000 const STOP_TEARDOWN_TIMEOUT_MS = 2_500 +// A rejected post-spawn dispatch normally gets a final opportunity to undo an +// external claim before shutdown relinquishes its lifecycle lease. Provider +// writes are not guaranteed to settle, so that opportunity must stay inside a +// bounded shutdown budget; a successor can recover from the retained durable +// lifecycle after this process releases its local agents. +const STOP_REJECTED_DISPATCH_DRAIN_TIMEOUT_MS = 2_500 const DISPATCH_LIFECYCLE_LEASE_MS = 5 * 60_000 const DISPATCH_LIFECYCLE_RENEW_MS = 60_000 const DISPATCH_LIFECYCLE_RETRY_MS = 1_000 @@ -633,6 +639,36 @@ class ReadinessReconcileTimeoutError extends Error { } } +class PostSpawnDispatchWaitRejectedError extends Error { + constructor( + readonly issueKey: string, + readonly compensationError?: unknown, + ) { + super(`Post-spawn dispatch waits were rejected for ${issueKey}`) + this.name = 'PostSpawnDispatchWaitRejectedError' + } +} + +interface PostSpawnDispatchClaimFence { + completionAtWriteBoundary: boolean + claimStarted: boolean + accepted?: boolean + settled: Promise + rejectionSettled: Promise + settle(accepted: boolean): void + settleRejection(compensated: boolean): void +} + +interface AppliedDispatchClaim { + implementingStateId?: string + reject(): Promise +} + +interface PostSpawnIssueObservation { + settled: Promise + settle(accepted: boolean): void +} + const realClock: Clock = { now: () => Date.now(), sleep: (ms) => new Promise((resolve) => setTimeout(resolve, ms)), @@ -857,6 +893,29 @@ export class FactoryLoop implements Factory { #previewSweepTimer?: ReturnType #previewSweepInFlight?: Promise readonly #completionInFlight = new Set() + readonly #issueWritebackInFlight = new Map>() + // A fast completion can make terminal issue state visible while dispatch is + // still performing its post-spawn readiness read. Completion publishes its + // provider receipt, then waits for that read to classify the state as owned + // or foreign before it releases agents under a terminal success reason. + readonly #postSpawnIssueObservations = new Map() + // The post-spawn ready read and following dispatch claim form one local + // write boundary. A completion arriving first makes dispatch wait and + // re-read; one arriving after claim entry waits for the claim to finish. + readonly #postSpawnDispatchClaimFences = new Map() + // Shutdown must not lose a local placement merely because a rejected + // provider claim unwinds before #releaseInFlightAgents snapshots the batch. + // The capture set makes both interleavings explicit: the dispatch catch may + // remove a record only after shutdown has copied its agents, otherwise the + // release sweep removes it after the copy is safe. + readonly #shutdownReleaseCapturedDispatches = new Set() + readonly #postSpawnDispatchesRejectedDuringStop = new Set() + // A rejected provider claim is unsafe to release until its conditional + // rollback succeeds or a newer provider status supersedes it. This runtime + // set closes the race before the durable dispatchClaim flag is persisted. + readonly #uncompensatedDispatchClaims = new Set() + readonly #pendingDispatchClaims = new Set() + readonly #dispatchClaimSettlementsInFlight = new Set() readonly #agentExitsInFlight = new Map>() #reconciledAgentExitsActive = 0 readonly #reconciledAgentExitWaiters: Array<() => void> = [] @@ -1248,6 +1307,15 @@ export class FactoryLoop implements Factory { } async #start(opts: FactoryStartOptions): Promise { + // These sets coordinate one stop snapshot with dispatches unwinding during + // that same stop. They are not durable lifecycle state: retaining their + // keys across a later start would make the next stop mistake a fresh + // dispatch for one already captured by the previous shutdown. + this.#shutdownReleaseCapturedDispatches.clear() + this.#postSpawnDispatchesRejectedDuringStop.clear() + this.#uncompensatedDispatchClaims.clear() + this.#pendingDispatchClaims.clear() + this.#dispatchClaimSettlementsInFlight.clear() this.#stopping = false this.#startMode = opts.mode ?? 'live' const issueSource = await this.#issueSource() @@ -1362,12 +1430,27 @@ export class FactoryLoop implements Factory { async stop(): Promise { this.#started = false this.#stopping = true + // These waits are armed before every planned spawn returns. Resolve them + // before the first shutdown await: a fast first agent may be completing + // while a later spawn is hung, and #drainAgentExitsInFlight would otherwise + // wait forever on a resolver stranded inside that dispatch stack. + const postSpawnWaitKeys = new Set([ + ...this.#postSpawnIssueObservations.keys(), + ...this.#postSpawnDispatchClaimFences.keys(), + ]) + const rejectedClaimDispatches = new Set>() + for (const [key, fence] of this.#postSpawnDispatchClaimFences) { + if (!fence.claimStarted) continue + const dispatchKeyPrefix = `${key}:live:` + for (const [dispatchKey, dispatch] of this.#dispatchInFlight) { + if (dispatchKey.startsWith(dispatchKeyPrefix)) rejectedClaimDispatches.add(dispatch) + } + } + for (const key of postSpawnWaitKeys) this.#settlePostSpawnDispatchWaits(key, false, 'stop') if (this.#babysitterResourceDeliveryRetryTimer) clearTimeout(this.#babysitterResourceDeliveryRetryTimer) this.#babysitterResourceDeliveryRetryTimer = undefined if (this.#babysitterResourceSubscriptionRenewTimer) clearTimeout(this.#babysitterResourceSubscriptionRenewTimer) this.#babysitterResourceSubscriptionRenewTimer = undefined - if (this.#dispatchLifecycleRenewTimer) clearInterval(this.#dispatchLifecycleRenewTimer) - this.#dispatchLifecycleRenewTimer = undefined if (this.#heldAgentDeadlineTimer) clearTimeout(this.#heldAgentDeadlineTimer) this.#heldAgentDeadlineTimer = undefined this.#heldAgentDeadlineDueAtMs = undefined @@ -1395,12 +1478,26 @@ export class FactoryLoop implements Factory { await this.#previewSweepInFlight this.#stoppingHeartbeatRefreshActive = await this.#stopLiveHeartbeat('stopping') try { + // Only dispatches that had entered the provider-claim boundary can leave + // an external claim behind. Drain those rejected promises before + // relinquishing their lifecycle leases or tearing down the fleet/mount. + // A pre-claim spawn hang is deliberately absent from this set, so it + // cannot hold shutdown open. + await this.#drainRejectedClaimDispatchesForStop(rejectedClaimDispatches) + await this.#persistOutstandingClaimCancellationBlocks() + // Keep renewing ownership through the potentially slow provider drain + // and cancellation-fence handoff. Stopping renewal earlier can let the + // lease expire before the fence is saved, allowing a successor to reap + // agents while the old provider write can still land. // Relinquish durable ownership before waiting on mount-backed lifecycle // drives. A slow Relayfile scan must not consume the shutdown deadline // while every issue remains fenced to a publisher that is already - // stopping. The owner/epoch fence makes any late completion from those - // drives harmless; a second sweep below catches claims racing this one. - await this.#releaseOwnedDispatchLifecycleLeases() + // stopping. Preserve leases for provider claims whose bounded drain + // expired: the original caller still needs its epoch to persist a late + // conclusive settlement. A second sweep below catches claims racing this + // one. + await this.#releaseOwnedDispatchLifecycleLeases((key) => + this.#pendingDispatchClaims.has(key) || this.#dispatchClaimSettlementsInFlight.has(key)) await Promise.allSettled([...this.#dispatchLifecycleDrives]) // Fence every source of new clarification work before touching the fleet. // A wake already past the fence is allowed to unwind, and is awaited @@ -1423,12 +1520,17 @@ export class FactoryLoop implements Factory { // non-durable (local/internal) records; terminal completion performs the // normal remote release before clearing the lifecycle. await this.#releaseInFlightAgents('factory-stopped', { preserveDurable: true }) - await this.#releaseOwnedDispatchLifecycleLeases() + if (this.#dispatchLifecycleRenewTimer) clearInterval(this.#dispatchLifecycleRenewTimer) + this.#dispatchLifecycleRenewTimer = undefined + await this.#releaseOwnedDispatchLifecycleLeases((key) => + this.#pendingDispatchClaims.has(key) || this.#dispatchClaimSettlementsInFlight.has(key)) if (this.#livePollTimer) clearTimeout(this.#livePollTimer) this.#livePollTimer = undefined this.#livePollInFlight = false this.#liveEventQueue.length = 0 this.#completionInFlight.clear() + this.#postSpawnIssueObservations.clear() + this.#postSpawnDispatchClaimFences.clear() this.#babysitterSpawned.clear() this.#babysitterPr.clear() this.#babysitterIssueRefs.clear() @@ -1472,11 +1574,13 @@ export class FactoryLoop implements Factory { this.#offUnpricedModel = undefined await this.#fleet.dispose() } finally { + if (this.#dispatchLifecycleRenewTimer) clearInterval(this.#dispatchLifecycleRenewTimer) + this.#dispatchLifecycleRenewTimer = undefined this.#stoppingHeartbeatRefreshActive = false } } - async #releaseOwnedDispatchLifecycleLeases(): Promise { + async #releaseOwnedDispatchLifecycleLeases(preserve: (key: string) => boolean = () => false): Promise { // The epoch cache is an execution optimization, not the durable ownership // authority. Error/fence paths may evict a cached epoch while its persisted // lease is still ours, so enumerate state before shutdown relinquishment. @@ -1487,6 +1591,7 @@ export class FactoryLoop implements Factory { } } for (const [key, epoch] of owned) { + if (preserve(key)) continue await this.#state.releaseDispatchLifecycleLease( this.#workspaceId, key, @@ -1518,6 +1623,175 @@ export class FactoryLoop implements Factory { } } + async #drainRejectedClaimDispatchesForStop( + dispatches: ReadonlySet>, + ): Promise { + if (dispatches.size === 0) return + + let timer: ReturnType | undefined + const drained = Promise.allSettled([...dispatches]).then(() => true) + // A durable fleet can recover ownership after this process exits. Local + // and other non-durable fleets cannot: releasing their placements before + // the provider claim has compensated would leave no successor able to + // finish the lifecycle, and a late write could strand the issue in its + // claimed state. Keep draining those fleets without the shutdown escape + // hatch. + if (this.#fleet.durableOwnership !== true) { + await drained + return + } + const timedOut = new Promise((resolve) => { + timer = setTimeout(() => resolve(false), STOP_REJECTED_DISPATCH_DRAIN_TIMEOUT_MS) + timer.unref?.() + }) + const completed = await Promise.race([drained, timedOut]) + if (timer) clearTimeout(timer) + if (completed) return + + this.#increment('postSpawnDispatchClaimDrainTimeouts') + this.#logger.warn?.('[factory] rejected post-spawn dispatch compensation timed out; continuing shutdown', { + dispatches: dispatches.size, + timeoutMs: STOP_REJECTED_DISPATCH_DRAIN_TIMEOUT_MS, + }) + } + + async #persistOutstandingClaimCancellationBlocks(): Promise { + const batch = await this.#batch() + for (const record of batch.inFlight) { + const key = dispatchLifecycleKey(record.issue) + if (!this.#pendingDispatchClaims.has(key)) continue + // The bounded durable drain expired before the provider operation could + // report whether rollback was safe. Persist the provisional release + // fence while this publisher still owns the lifecycle lease, so a + // successor cannot mistake the unresolved claim for abandonable work. + const persisted = await this.#retainUncompensatedDispatchClaim( + record, + new Error('Rejected dispatch claim compensation did not settle before shutdown'), + { preferExistingError: true, pending: true }, + ) + if (!persisted) { + const durable = await this.#state.getDispatchLifecycle(this.#workspaceId, key) + if (durable?.dispatchClaim?.cancellationPending !== true) { + throw new Error(`Unable to persist unresolved dispatch-claim fence for ${record.issue.key}; refusing shutdown handoff`) + } + } + } + } + + async #awaitRejectedClaimCompensation(settled: Promise): Promise { + let timer: ReturnType | undefined + const timedOut = new Promise((resolve) => { + timer = setTimeout(() => resolve(false), STOP_REJECTED_DISPATCH_DRAIN_TIMEOUT_MS) + timer.unref?.() + }) + const compensated = await Promise.race([settled, timedOut]) + if (timer) clearTimeout(timer) + return compensated + } + + async #retainUncompensatedDispatchClaim( + record: InFlightIssue, + error: unknown, + opts: { preferExistingError?: boolean; pending?: boolean } = {}, + ): Promise { + const key = dispatchLifecycleKey(record.issue) + const alreadyBlocked = record.dispatchClaim?.cancellationBlocked === true + const errorMessage = describeError(error).errorMessage + this.#uncompensatedDispatchClaims.add(key) + const { cancellationPending: _pending, ...existingClaim } = record.dispatchClaim ?? { state: 'degraded' as const } + record.dispatchClaim = { + ...existingClaim, + state: 'degraded', + cancellationBlocked: true, + ...(opts.pending ? { cancellationPending: true } : {}), + write: 'rejected dispatch claim rollback', + error: opts.preferExistingError ? record.dispatchClaim?.error ?? errorMessage : errorMessage, + deadLettered: true, + updatedAtMs: this.#clock.now(), + } + this.#dispatchClaimStatuses.set(key, record.dispatchClaim) + if (!alreadyBlocked) { + this.#increment('postSpawnDispatchClaimRecoveryRetentions') + this.#logger.warn?.('[factory] retaining dispatch lifecycle because rejected claim compensation is unproven', { + issue: record.issue.key, + }) + } + if (this.#usesDurableDispatchLifecycle()) { + return await this.#saveDispatchLifecycle(record, 'running') + } else { + await this.#writeDispatchClaimRegistry(record.issue) + return true + } + } + + async #clearDispatchCancellationBlock(record: InFlightIssue): Promise { + const key = dispatchLifecycleKey(record.issue) + if (record.dispatchClaim?.cancellationBlocked !== true) { + this.#uncompensatedDispatchClaims.delete(key) + return true + } + const previousClaim = record.dispatchClaim + const { + cancellationBlocked: _blocked, + cancellationPending: _pending, + ...claim + } = previousClaim + record.dispatchClaim = claim + this.#dispatchClaimStatuses.set(key, claim) + try { + const persisted = this.#usesDurableDispatchLifecycle() + ? await this.#saveDispatchLifecycle(record, 'running') + : await this.#writeDispatchClaimRegistry(record.issue).then(() => true) + if (!persisted) { + record.dispatchClaim = previousClaim + this.#dispatchClaimStatuses.set(key, previousClaim) + return false + } + this.#uncompensatedDispatchClaims.delete(key) + return true + } catch (error) { + record.dispatchClaim = previousClaim + this.#dispatchClaimStatuses.set(key, previousClaim) + throw error + } + } + + async #dispatchClaimBlocksAbandonment(record: InFlightIssue): Promise { + const key = dispatchLifecycleKey(record.issue) + if (!this.#uncompensatedDispatchClaims.has(key) && record.dispatchClaim?.cancellationBlocked !== true) { + return false + } + // A provider request that was still unresolved at handoff can land after + // any current-state read. No observation is proof of supersession until + // the original caller records that the operation conclusively settled. + if (record.dispatchClaim?.cancellationPending === true) return true + try { + const issue = await this.#readIssue(record.issue.path) + if (!issue) return true + if (isGithubIssue(issue)) { + if (!this.#githubWriteback.getIssueStatus) return true + const status = await this.#githubWriteback.getIssueStatus(issue, { + requireFresh: true, + freshAfterMs: record.dispatchClaim?.claimStartedAtMs, + }) + if (!status || status === 'in-progress') return true + } else { + if (!this.#linear.getIssueStateId) return true + const implementingStateId = this.#states.idFor(issue.team, 'agentImplementing') + const currentStateId = await this.#linear.getIssueStateId(issue) + if (!implementingStateId || !currentStateId || currentStateId === implementingStateId) return true + } + if (!await this.#clearDispatchCancellationBlock(record)) return true + return false + } catch (error) { + this.#logger.warn?.('[factory] unable to verify whether a blocked dispatch claim was superseded', { + issue: record.issue.key, + error: describeError(error).errorMessage, + }) + return true + } + } + async #boundedStopTeardown(label: string, teardown: () => Promise | void | undefined): Promise { let timer: ReturnType | undefined const action = Promise.resolve() @@ -4719,6 +4993,63 @@ export class FactoryLoop implements Factory { if (!dryRun) await this.#ensureGithubAgentQuestionWatch(record, liveIssue) const spawnedForReaperHandoff: RegistryHandoffAgent[] = [] + // These waits belong to the durable work unit, not the ingestion surface. + // Completion and lifecycle ownership use the same identity (#211); using + // issueKey here would make every GitHub-native dispatch miss its own fence. + const postSpawnKey = dispatchLifecycleKey(record.issue) + let resolvePostSpawnIssueObservation!: (accepted: boolean) => void + let postSpawnIssueObservationDidSettle = false + const postSpawnIssueObservationSettled = new Promise((resolve) => { + resolvePostSpawnIssueObservation = resolve + }) + let postSpawnIssueObservation!: PostSpawnIssueObservation + postSpawnIssueObservation = { + settled: postSpawnIssueObservationSettled, + settle: (accepted: boolean): void => { + if (postSpawnIssueObservationDidSettle) return + postSpawnIssueObservationDidSettle = true + resolvePostSpawnIssueObservation(accepted) + if (this.#postSpawnIssueObservations.get(postSpawnKey) === postSpawnIssueObservation) { + this.#postSpawnIssueObservations.delete(postSpawnKey) + } + }, + } + this.#postSpawnIssueObservations.set(postSpawnKey, postSpawnIssueObservation) + let resolvePostSpawnDispatchClaim!: (accepted: boolean) => void + let postSpawnDispatchClaimSettled = false + const postSpawnDispatchClaim = new Promise((resolve) => { + resolvePostSpawnDispatchClaim = resolve + }) + let resolveRejectedClaimCompensation!: (compensated: boolean) => void + let rejectedClaimCompensationSettled = false + const rejectedClaimCompensation = new Promise((resolve) => { + resolveRejectedClaimCompensation = resolve + }) + let postSpawnDispatchClaimFence!: PostSpawnDispatchClaimFence + postSpawnDispatchClaimFence = { + completionAtWriteBoundary: false, + claimStarted: false, + settled: postSpawnDispatchClaim, + rejectionSettled: rejectedClaimCompensation, + settle: (accepted: boolean): void => { + if (postSpawnDispatchClaimSettled) return + postSpawnDispatchClaimSettled = true + postSpawnDispatchClaimFence.accepted = accepted + resolvePostSpawnDispatchClaim(accepted) + if (this.#postSpawnDispatchClaimFences.get(postSpawnKey) === postSpawnDispatchClaimFence) { + this.#postSpawnDispatchClaimFences.delete(postSpawnKey) + } + }, + settleRejection: (compensated: boolean): void => { + if (rejectedClaimCompensationSettled) return + rejectedClaimCompensationSettled = true + resolveRejectedClaimCompensation(compensated) + }, + } + this.#postSpawnDispatchClaimFences.set(postSpawnKey, postSpawnDispatchClaimFence) + const settlePostSpawnIssueObservation = postSpawnIssueObservation.settle + const settlePostSpawnDispatchClaim = postSpawnDispatchClaimFence.settle + let rejectDispatchClaim: (() => Promise) | undefined try { if (!dryRun) { const issue = await this.#readIssue(dispatchDecision.issue.path) @@ -4743,9 +5074,11 @@ export class FactoryLoop implements Factory { agents.push({ name: spawned.name, role: spec.role }) } if (!dryRun) { + const claimStartedAtMs = this.#clock.now() record.dispatchClaim = { state: 'pending', - updatedAtMs: this.#clock.now(), + claimStartedAtMs, + updatedAtMs: claimStartedAtMs, } this.#dispatchClaimStatuses.set(dispatchLifecycleKey(record.issue), record.dispatchClaim) } @@ -4753,37 +5086,66 @@ export class FactoryLoop implements Factory { const comment = dispatchComment(dispatchDecision, agents) let implementingStateId: string | undefined + const rejectDispatchClaimIfCancelled = async (): Promise => { + if (postSpawnDispatchClaimFence.accepted !== false) return + if (rejectDispatchClaim) await rejectDispatchClaim() + throw new PostSpawnDispatchWaitRejectedError(dispatchDecision.issue.key) + } if (!dryRun) { - const issue = await this.#readIssue(dispatchDecision.issue.path) - if (!issue || !this.#isIssueReady(issue)) { - // The agents spawned a few lines above can reach terminal before we - // get here. Their completion writeback parks the issue — Linear - // `humanReview`, or the GitHub human-review label — and stamps - // *this same record's* lifecycle on the way past. Re-reading the - // issue then shows "not ready", but the writer was us. - // - // Treating that as a foreign change is not a cosmetic misreport: the - // catch below classifies LiveDispatchStateChangedError as terminal - // and calls #releaseAndTerminateAgents, so a dispatch whose agents - // finished quickly tore down its own completed work and reported - // RETRYABLE to its supervisor (factory#319). + let issue = await this.#readIssue(dispatchDecision.issue.path) + if (postSpawnDispatchClaimFence.completionAtWriteBoundary) { + // Completion reached its terminal provider-write boundary before + // dispatch entered the claim boundary. Always re-read after its + // receipt, even when the first snapshot was ready: the terminal + // write may have started immediately after that snapshot. + const issueWriteback = this.#issueWritebackInFlight.get(postSpawnKey) + if (issueWriteback) await issueWriteback + issue = await this.#readIssue(dispatchDecision.issue.path) + } else if ((!issue || !this.#isIssueReady(issue)) && record.issueWritebackConfirmedAtMs === undefined) { + // A very fast agent can finish while its own dispatch is still + // performing this post-spawn read. Do not guess authorship from the + // first visible state: an idempotent provider mutation cannot tell us + // whether Factory or another actor won the race. Instead, wait for + // this record's concurrent completion writeback to settle, then use + // only its provider-confirmed outcome (factory#319). // - // Only THIS dispatch's own confirmed issue writeback may excuse the - // change. Lifecycle phases are not enough: `publishing` is entered - // before the PR is published and `parking` before anything touches - // the issue, so a phase records local progress, not authorship — and - // a foreign park landing during those awaits would be misread as - // ours (codex review on #321). Losing the row to another owner is a - // different condition and is still caught where it always was, by - // #saveDispatchLifecycle returning false. + // The completion promise settles as soon as terminal issue writeback + // succeeds or fails, before Slack/release cleanup. If there is no + // completion in flight, this is an ordinary foreign state change and + // the existing abort remains immediate. + const issueWriteback = this.#issueWritebackInFlight.get(postSpawnKey) + if (issueWriteback) { + await issueWriteback + issue = await this.#readIssue(dispatchDecision.issue.path) + } + } + if (!issue || !this.#isIssueReady(issue)) { + // Only a terminal writeback applied and confirmed by the completion + // already in flight for this exact record may excuse the change. The + // marker is stamped after provider acknowledgement/readback, never at + // the earlier locally-visible write boundary. A confirmed no-op does + // not establish authorship: another actor may have parked the issue + // before our adapter's first provider read. if (record.issueWritebackConfirmedAtMs === undefined) { throw new LiveDispatchStateChangedError(dispatchDecision.issue.key) } // The claim is moot and would be wrong to write: it would drag an // issue our own lifecycle has already parked back to `implementing`. } else { - implementingStateId = await this.#applyDispatchClaim(record, issue, comment) - record.issueWritebackConfirmedAtMs = this.#clock.now() + // This assignment is synchronous with the preceding completion flag + // check. Completion either arrived first (the branch above) or will + // now observe claimStarted and wait; no await-sized gap remains. + postSpawnDispatchClaimFence.claimStarted = true + const appliedClaim = await this.#applyDispatchClaim( + record, + issue, + comment, + postSpawnDispatchClaimFence, + ) + implementingStateId = appliedClaim.implementingStateId + rejectDispatchClaim = appliedClaim.reject + await rejectDispatchClaimIfCancelled() + record.issueWritebackConfirmedAtMs ??= this.#clock.now() this.#emit('writeback-verified', { issue: dispatchDecision.issue, path: issue.path }) } } @@ -4799,17 +5161,110 @@ export class FactoryLoop implements Factory { dryRun, } record.result = result - if (!await this.#saveDispatchLifecycle(record, 'running')) return result - this.#increment('dispatched') - this.#emit('dispatched', { issue: dispatchDecision.issue, result }) + const runningSaved = await this.#saveDispatchLifecycle(record, 'running') + await rejectDispatchClaimIfCancelled() + if (!runningSaved) { + settlePostSpawnDispatchClaim(true) + settlePostSpawnIssueObservation(true) + return result + } if (!dryRun && this.#config.hooks?.onTicketDispatch) { await this.#notifyTicketDispatch(dispatchDecision, liveIssue, record, result) + await rejectDispatchClaimIfCancelled() } if (!dryRun) { await this.#ensureSlackDispatchThread(record, result, liveIssue) + await rejectDispatchClaimIfCancelled() } + // Accept without an intervening await only after every durable + // publication and post-dispatch side effect has completed. Stop or held + // abandonment can therefore reject any still-publishing dispatch. + settlePostSpawnDispatchClaim(true) + settlePostSpawnIssueObservation(true) + this.#increment('dispatched') + this.#emit('dispatched', { issue: dispatchDecision.issue, result }) return result - } catch (error) { + } catch (caughtError) { + // Stop/deadline may reject the fence while any awaited provider or + // lifecycle operation is still in flight. If that operation then fails, + // its raw error must not fall into ordinary dispatch cleanup: shutdown + // still owns the already-spawned placements and needs them retained + // until its release snapshot. If the claim receipt is already available, + // conditionally reject it before releasing those placements. Without a + // receipt the provider outcome is ambiguous, so preserve the original + // failure as the compensation detail after the claim boundary began. + let error = caughtError + if (postSpawnDispatchClaimFence.accepted === false + && !(caughtError instanceof PostSpawnDispatchWaitRejectedError)) { + if (rejectDispatchClaim) { + try { + await this.#rejectDispatchClaimAfterOperationError( + dispatchDecision.issue.key, + caughtError, + rejectDispatchClaim, + ) + } catch (rejectionError) { + error = rejectionError + } + } else { + error = new PostSpawnDispatchWaitRejectedError( + dispatchDecision.issue.key, + postSpawnDispatchClaimFence.claimStarted ? caughtError : undefined, + ) + } + } + settlePostSpawnDispatchClaim(false) + if (error instanceof PostSpawnDispatchWaitRejectedError) { + settlePostSpawnIssueObservation(false) + const compensated = error.compensationError === undefined + // The original provider call has conclusively returned. Stop the + // timeout handoff from writing a provisional pending fence, but keep a + // separate lease fence until the definitive result is durable. Without + // that second set, shutdown can relinquish this epoch in the await + // below and make the late save fail after the provider compensated. + this.#dispatchClaimSettlementsInFlight.add(postSpawnKey) + this.#pendingDispatchClaims.delete(postSpawnKey) + let settlementPersisted = false + try { + settlementPersisted = compensated + ? await this.#clearDispatchCancellationBlock(record) + : await this.#retainUncompensatedDispatchClaim(record, error.compensationError) + if (!settlementPersisted) { + throw new Error(`Unable to persist settled dispatch-claim compensation for ${decision.issue.key}`) + } + postSpawnDispatchClaimFence.settleRejection(compensated) + if (error.compensationError !== undefined) { + this.#increment('postSpawnDispatchClaimCompensationFailures') + this.#logger.error?.('[factory] rejected dispatch claim could not be safely compensated', { + issue: decision.issue.key, + error: describeError(error.compensationError).errorMessage, + }) + } + // External stop/abandonment owns agent and durable-lifecycle cleanup. + // Remove only the process-local slot here; running the ordinary + // dispatch-failure path would race or duplicate that cleanup. During + // stop, however, retain local placements until the shutdown release + // sweep has captured them. If it already has, removal is safe now. + if (!compensated) { + // The provider still exposes a claim this lifecycle authored. Keep + // the process-local slot and durable row recoverable; releasing its + // agents would leave the claimed issue with no successor lifecycle. + } else if (this.#stopping) { + this.#postSpawnDispatchesRejectedDuringStop.add(postSpawnKey) + if (this.#shutdownReleaseCapturedDispatches.has(postSpawnKey)) { + batch.abandon(decision.issue) + this.#postSpawnDispatchesRejectedDuringStop.delete(postSpawnKey) + } + } else { + batch.abandon(decision.issue) + } + } finally { + if (!settlementPersisted) this.#pendingDispatchClaims.add(postSpawnKey) + this.#dispatchClaimSettlementsInFlight.delete(postSpawnKey) + } + throw error + } + settlePostSpawnIssueObservation(!(error instanceof LiveDispatchStateChangedError)) // A spawn can fail after the broker accepted it but before its ack // reached Factory. Include every planned worktree agent, not only the // acknowledged spawns, so cleanup never races a name-only survivor. @@ -6747,9 +7202,11 @@ export class FactoryLoop implements Factory { const comment = dispatchComment(record.decision, agents) let implementingStateId: string | undefined if (!record.dryRun) { + const claimStartedAtMs = this.#clock.now() record.dispatchClaim = { state: 'pending', - updatedAtMs: this.#clock.now(), + claimStartedAtMs, + updatedAtMs: claimStartedAtMs, } this.#dispatchClaimStatuses.set(dispatchLifecycleKey(record.issue), record.dispatchClaim) } @@ -6758,7 +7215,7 @@ export class FactoryLoop implements Factory { const issue = liveIssue ?? await this.#readIssue(record.issue.path) if (!issue) throw new Error(`Unable to recover durable dispatch ${record.issue.key}: issue is no longer readable`) await this.#ensureGithubAgentQuestionWatch(record, issue) - implementingStateId = await this.#applyDispatchClaim(record, issue, comment) + implementingStateId = (await this.#applyDispatchClaim(record, issue, comment)).implementingStateId } const recoveredPreviews = uniquePreviewReferences([ ...dispatchSpecs(record.decision).map((spec) => spec.preview), @@ -7100,31 +7557,86 @@ export class FactoryLoop implements Factory { record: InFlightIssue, issue: LinearIssue, comment: string, - ): Promise { + claimFence?: PostSpawnDispatchClaimFence, + ): Promise { + const claimWasRejected = (): boolean => claimFence?.accepted === false + // stop() can reject this fence after the post-spawn readiness read but + // before this method starts. Never begin a provider claim in that state; + // shutdown intentionally does not drain dispatches that never crossed the + // claim boundary. + if (claimWasRejected()) { + throw new PostSpawnDispatchWaitRejectedError(record.issue.key) + } let implementingStateId: string | undefined + let rejectClaim: (() => Promise) | undefined if (isGithubIssue(issue)) { + const statusClaim: { value: Awaited>> } = { + value: { result: undefined }, + } await this.#retryDispatchWriteback(record, issue, 'GitHub label factory:in-progress', async () => { - await this.#githubWriteback.setStatus(issue, 'in-progress') + statusClaim.value = this.#githubWriteback.claimStatus + ? await this.#githubWriteback.claimStatus(issue, 'in-progress') + : { result: await this.#githubWriteback.setStatus(issue, 'in-progress') } }) + rejectClaim = async (): Promise => await this.#rejectGithubDispatchClaim( + record, + issue, + statusClaim.value, + ) + if (claimWasRejected()) { + await rejectClaim!() + } const commentApplied = this.#githubWriteback.hasCommentMarker ? async (): Promise => this.#githubWriteback.hasCommentMarker!(issue, comment) : undefined - await this.#retryDispatchWriteback( - record, - issue, - 'GitHub dispatch comment', - async () => this.#githubWriteback.postComment(issue, comment), - commentApplied, - ) + try { + await this.#retryDispatchWriteback( + record, + issue, + 'GitHub dispatch comment', + async () => { + if (claimWasRejected()) await rejectClaim!() + await this.#githubWriteback.postComment(issue, comment) + }, + commentApplied, + ) + } catch (error) { + if (claimWasRejected() && !(error instanceof PostSpawnDispatchWaitRejectedError)) { + await this.#rejectDispatchClaimAfterOperationError(record.issue.key, error, rejectClaim!) + } + throw error + } + if (claimWasRejected()) await rejectClaim!() } else { implementingStateId = this.#states.idFor(issue.team, 'agentImplementing') + const readyStateId = issue.stateId + let claimToken: string | undefined await this.#retryDispatchWriteback(record, issue, `Linear state ${implementingStateId}`, async () => { - await this.#linear.setState(issue, implementingStateId!) - }) - await this.#retryDispatchWriteback(record, issue, 'Linear dispatch comment', async () => { - await this.#linear.postComment(issue, comment) + claimToken = (await this.#linear.setState(issue, implementingStateId!))?.claimToken }) + rejectClaim = async (): Promise => await this.#rejectLinearDispatchClaim( + record, + issue, + implementingStateId!, + readyStateId, + claimToken, + ) + if (claimWasRejected()) { + await rejectClaim!() + } + try { + await this.#retryDispatchWriteback(record, issue, 'Linear dispatch comment', async () => { + if (claimWasRejected()) await rejectClaim!() + await this.#linear.postComment(issue, comment) + }) + } catch (error) { + if (claimWasRejected() && !(error instanceof PostSpawnDispatchWaitRejectedError)) { + await this.#rejectDispatchClaimAfterOperationError(record.issue.key, error, rejectClaim!) + } + throw error + } + if (claimWasRejected()) await rejectClaim!() } record.dispatchClaim = { @@ -7133,7 +7645,98 @@ export class FactoryLoop implements Factory { } this.#dispatchClaimStatuses.set(dispatchLifecycleKey(record.issue), record.dispatchClaim) await this.#writeDispatchClaimRegistry(record.issue) - return implementingStateId + if (claimWasRejected()) await rejectClaim?.() + return { + ...(implementingStateId ? { implementingStateId } : {}), + reject: rejectClaim!, + } + } + + async #rejectGithubDispatchClaim( + record: InFlightIssue, + issue: LinearIssue, + claim: Awaited>>, + ): Promise { + let compensationError: unknown + if (claim.result !== 'already-matched') { + try { + const claimToken = claim.claimToken + if (!claimToken) { + throw new Error('GitHub rejected dispatch claim has no immutable ownership token') + } + const rollbackStatusClaim = this.#githubWriteback.rollbackStatusClaim?.bind(this.#githubWriteback) + if (!rollbackStatusClaim) { + throw new Error('GitHub writeback cannot conditionally roll back a rejected dispatch claim') + } + let outcome: Awaited>> = 'unproven' + await this.#retryDispatchWriteback(record, issue, 'GitHub rejected dispatch claim rollback', async () => { + outcome = await rollbackStatusClaim(issue, 'in-progress', claimToken) + if (outcome === 'unproven') { + throw new Error('GitHub rejected dispatch claim rollback could not prove ownership') + } + }) + } catch (error) { + compensationError = error + } + } + throw new PostSpawnDispatchWaitRejectedError(record.issue.key, compensationError) + } + + async #rejectDispatchClaimAfterOperationError( + issueKey: string, + operationError: unknown, + rejectClaim: () => Promise, + ): Promise { + try { + await rejectClaim() + } catch (rejectionError) { + if (rejectionError instanceof PostSpawnDispatchWaitRejectedError) { + if (rejectionError.compensationError === undefined) throw rejectionError + throw new PostSpawnDispatchWaitRejectedError( + issueKey, + new AggregateError( + [operationError, rejectionError.compensationError], + 'Cancelled dispatch operation failed and its claim could not be safely compensated', + ), + ) + } + throw new PostSpawnDispatchWaitRejectedError( + issueKey, + new AggregateError( + [operationError, rejectionError], + 'Cancelled dispatch operation failed and its claim rejection threw unexpectedly', + ), + ) + } + throw new PostSpawnDispatchWaitRejectedError(issueKey) + } + + async #rejectLinearDispatchClaim( + record: InFlightIssue, + issue: LinearIssue, + implementingStateId: string, + readyStateId: string, + claimToken: string | undefined, + ): Promise { + let compensationError: unknown + try { + const compareAndSetState = this.#linear.compareAndSetState?.bind(this.#linear) + if (!compareAndSetState) { + throw new Error('Linear writeback cannot conditionally roll back a rejected dispatch claim') + } + if (!claimToken) { + throw new Error('Linear rejected dispatch claim has no immutable ownership token') + } + await this.#retryDispatchWriteback(record, issue, `Linear rejected dispatch claim rollback to ${readyStateId}`, async () => { + const outcome = await compareAndSetState(issue, implementingStateId, claimToken, readyStateId) + if (outcome === 'unproven') { + throw new Error('Linear rejected dispatch claim rollback could not prove an atomic provider transition') + } + }) + } catch (error) { + compensationError = error + } + throw new PostSpawnDispatchWaitRejectedError(record.issue.key, compensationError) } async #retryDispatchWriteback( @@ -7153,11 +7756,29 @@ export class FactoryLoop implements Factory { } return } catch (error) { + // Claim rejection is terminal for this dispatch. In particular, do not + // turn a cancellation discovered between comment attempts into another + // provider retry that can publish after shutdown began. + if (error instanceof PostSpawnDispatchWaitRejectedError) { + if (lastError !== undefined && error.compensationError !== undefined) { + throw new PostSpawnDispatchWaitRejectedError( + record.issue.key, + new AggregateError( + [lastError, error.compensationError], + 'Dispatch writeback failed before cancellation and its claim could not be safely compensated', + ), + ) + } + throw error + } lastError = error const deadLettered = attempt === DISPATCH_WRITEBACK_MAX_ATTEMPTS this.#increment('dispatchWritebackFailures') record.dispatchClaim = { state: 'degraded', + ...(record.dispatchClaim?.claimStartedAtMs === undefined + ? {} + : { claimStartedAtMs: record.dispatchClaim.claimStartedAtMs }), write, attempts: attempt, maxAttempts: DISPATCH_WRITEBACK_MAX_ATTEMPTS, @@ -8342,10 +8963,20 @@ export class FactoryLoop implements Factory { async #releaseInFlightAgents(reason: string, opts: { preserveDurable?: boolean } = {}): Promise { const agents = new Map() - for (const record of (await this.#batch()).inFlight) { + const blockedDispatches = new Set() + const batch = await this.#batch() + const records = [...batch.inFlight] + for (const record of records) { + const key = dispatchLifecycleKey(record.issue) if (record.dryRun) { + this.#shutdownReleaseCapturedDispatches.add(key) + continue + } + if (this.#uncompensatedDispatchClaims.has(key) || record.dispatchClaim?.cancellationBlocked === true) { + blockedDispatches.add(key) continue } + this.#shutdownReleaseCapturedDispatches.add(key) if (opts.preserveDurable && [...record.agents.values()].some((tracked) => tracked.result?.locality === 'remote')) { continue } @@ -8355,7 +8986,13 @@ export class FactoryLoop implements Factory { } await this.#releaseAndTerminateAgents([...agents], reason, 'stop') - await this.#writeInFlightRegistry(undefined, undefined, true) + for (const record of records) { + const key = dispatchLifecycleKey(record.issue) + if (blockedDispatches.has(key)) continue + if (!this.#postSpawnDispatchesRejectedDuringStop.delete(key)) continue + batch.abandon(record.issue) + } + await this.#writeInFlightRegistry(undefined, undefined, true, blockedDispatches) } async #releaseAndTerminateAgents( @@ -8600,6 +9237,7 @@ export class FactoryLoop implements Factory { path = this.#config.loop.registryPath, heartbeatPath = this.#config.loop.heartbeatPath, empty = false, + retainIssueKeys?: ReadonlySet, ): Promise { const updatedAtMs = this.#clock.now() const agents: FactoryInFlightRegistryAgent[] = [] @@ -8644,9 +9282,10 @@ export class FactoryLoop implements Factory { }) } - if (!empty) { + if (!empty || retainIssueKeys?.size) { for (const record of (await this.#batch()).inFlight) { if (record.dryRun) continue + if (empty && !retainIssueKeys?.has(dispatchLifecycleKey(record.issue))) continue if (record.dispatchClaim) { this.#dispatchClaimStatuses.set(dispatchLifecycleKey(record.issue), record.dispatchClaim) } @@ -9900,10 +10539,67 @@ export class FactoryLoop implements Factory { // the release-driven exit event so it cannot re-trigger a resume before the // record leaves the batch. async #abandonStuckDispatch(record: InFlightIssue, reason: string): Promise { - const key = issueKey(record.issue) + const key = dispatchLifecycleKey(record.issue) + // Fence the periodic held sweep before any await below. A blocked claim is + // retried by the single keyed abandonment timer; letting the held sweep + // enter concurrently can release the same placements twice when provider + // supersession becomes visible. + this.#abandonedDispatchReasons.set(key, reason) + try { + await this.#abandonStuckDispatchFenced(record, reason, key) + } catch (error) { + // The early fence above suppresses the periodic held sweep. Any + // exceptional exit must therefore install the keyed retry before + // propagating, or one transient persistence/read failure becomes a + // permanent absorbing state. + this.#increment('abandonedDispatchReleaseRetries') + this.#logger.warn?.('[factory] abandoned dispatch cleanup failed before retry was armed', { + issue: record.issue.key, + error: describeError(error).errorMessage, + }) + this.#scheduleAbandonedDispatchRetry(record, reason) + throw error + } + } + + async #abandonStuckDispatchFenced( + record: InFlightIssue, + reason: string, + key: string, + ): Promise { + const claimFence = this.#postSpawnDispatchClaimFences.get(key) + const rejectedClaimCompensation = claimFence?.claimStarted + ? claimFence.rejectionSettled + : undefined + // A dispatch can be stuck inside a later spawn after an earlier agent has + // already started completion. Release both post-spawn waits before the + // first abandonment await so terminal processing cannot remain an + // absorbing promise after this lifecycle is reaped. + this.#settlePostSpawnDispatchWaits(key, false, 'abandonment') + if (rejectedClaimCompensation) { + const compensated = await this.#awaitRejectedClaimCompensation(rejectedClaimCompensation) + if (!compensated) { + if (!this.#pendingDispatchClaims.has(key)) { + // The provider call settled while this bounded wait expired; its + // caller is recording the definitive compensated/unproven result. + this.#scheduleAbandonedDispatchRetry(record, reason) + return + } + await this.#retainUncompensatedDispatchClaim( + record, + new Error('Rejected dispatch claim compensation did not complete before abandonment'), + { preferExistingError: true, pending: true }, + ) + this.#scheduleAbandonedDispatchRetry(record, reason) + return + } + } + if (await this.#dispatchClaimBlocksAbandonment(record)) { + this.#scheduleAbandonedDispatchRetry(record, reason) + return + } const heldPastDeadline = reason === HELD_PAST_DEADLINE_RELEASE_REASON const agentReleaseReason = heldPastDeadline ? HELD_PAST_DEADLINE_RELEASE_REASON : 'issue-abandoned' - this.#abandonedDispatchReasons.set(key, reason) if (!await this.#saveDispatchLifecycle( record, 'abandoning', @@ -14291,6 +14987,21 @@ export class FactoryLoop implements Factory { return } this.#completionInFlight.add(completionKey) + const postSpawnIssueObservation = this.#postSpawnIssueObservations.get(completionKey) + const postSpawnDispatchClaimFence = this.#postSpawnDispatchClaimFences.get(completionKey) + let settleIssueWriteback!: () => void + const issueWritebackSettled = new Promise((resolve) => { + settleIssueWriteback = resolve + }) + let issueWritebackDidSettle = false + const settleIssueWritebackOnce = () => { + if (issueWritebackDidSettle) return + issueWritebackDidSettle = true + settleIssueWriteback() + if (this.#issueWritebackInFlight.get(completionKey) === issueWritebackSettled) { + this.#issueWritebackInFlight.delete(completionKey) + } + } let releaseReasonForRetry: string | undefined try { if (!await this.#assertDispatchLifecycleOwner(record)) return @@ -14340,29 +15051,85 @@ export class FactoryLoop implements Factory { const humanReview = configuredHumanReview || (githubIssue && !githubMerged) const statusLabel = humanReview ? 'In Human Review' : 'Done' if (issue) { + if (postSpawnDispatchClaimFence) { + // Publish boundary arrival before any await. Dispatch performs the + // paired synchronous claimStarted assignment after its ready read, + // so exactly one side wins: an earlier completion is awaited and + // re-read; a claim already entering its provider write finishes + // before this terminal write begins. + postSpawnDispatchClaimFence.completionAtWriteBoundary = true + if (postSpawnDispatchClaimFence.claimStarted || postSpawnDispatchClaimFence.accepted !== undefined) { + let claimAccepted = postSpawnDispatchClaimFence.accepted + if (claimAccepted === undefined) { + // This counter makes the otherwise internal promise boundary + // observable in deterministic race regressions and in incident + // telemetry. External abandonment/stop settlement must release + // every completion counted here. + this.#increment('postSpawnDispatchClaimFenceWaits') + claimAccepted = await postSpawnDispatchClaimFence.settled + } + if (!claimAccepted) return + } + } + // Register only at the provider-write boundary. Work before this point + // (PR discovery/merge gating) has not changed the issue, so a foreign + // park during it must still abort immediately rather than waiting on a + // possibly long completion path. + this.#issueWritebackInFlight.set(completionKey, issueWritebackSettled) if (githubIssue) { if (humanReview) { - await this.#githubWriteback.setStatus(issue, 'human-review') + const statusWrite = await this.#githubWriteback.setStatus(issue, 'human-review') + if (statusWrite === undefined) this.#recordMissingGithubWritebackReceipt('setStatus') + // Only an explicit provider-proven transition establishes that + // this dispatch owns the visible park. Legacy void adapters and + // App acknowledgements remain deliberately untrusted. + if (statusWrite === 'applied') { + record.issueWritebackConfirmedAtMs ??= this.#clock.now() + } + // The lifecycle-state outcome is now known. Unblock the concurrent + // post-spawn read before the separate completion comment write. + settleIssueWritebackOnce() await this.#githubWriteback.postComment( issue, `Factory agents completed; this issue is awaiting human review. The pull request remains open.\n\nMerge policy: ${this.#config.mergePolicy}`, ) } else { - await this.#githubWriteback.closeIssue( + const closeWrite = await this.#githubWriteback.closeIssue( issue, 'Factory observed the linked pull request merge and completed this issue.', ) + if (closeWrite === undefined) this.#recordMissingGithubWritebackReceipt('closeIssue') + // A provider-confirmed, actor-attributed close is the only safe + // proof that this dispatch owns the visible terminal state. An + // idempotent no-op, legacy void adapter, or App acknowledgement + // may instead mean a third party won the close race. + if (closeWrite === 'applied') { + record.issueWritebackConfirmedAtMs ??= this.#clock.now() + } } } else { const targetState = humanReview ? this.#states.idFor(issueTeam, 'humanReview') : this.#states.idFor(issueTeam, 'done') await this.#linear.setState(issue, targetState) + record.issueWritebackConfirmedAtMs ??= this.#clock.now() await this.#recordCanonicalIssueState({ ...record.issue, stateId: targetState }) } - record.issueWritebackConfirmedAtMs = this.#clock.now() - this.#emit('writeback-verified', { issue: record.issue, path: issue.path }) + if (record.issueWritebackConfirmedAtMs !== undefined) { + this.#emit('writeback-verified', { issue: record.issue, path: issue.path }) + } + // Unblock a concurrent post-spawn read as soon as the issue writeback + // outcome is known. Completion still has dependency, Slack and release + // work to do; none determines whether the observed issue state is safe. + settleIssueWritebackOnce() + // If completion raced dispatch's post-spawn read, let that read consume + // the receipt before claiming terminal success. An unproven provider + // transition is a foreign live-state change: dispatch owns the + // abandonment and must release agents with that reason, not issue-done. + if (postSpawnIssueObservation && !await postSpawnIssueObservation.settled) return if (!humanReview) await this.#markDependencyTerminalAndReconcile(issue) + } else { + settleIssueWritebackOnce() } if (!await this.#saveDispatchLifecycle(record, 'writeback-applied')) return @@ -14425,6 +15192,10 @@ export class FactoryLoop implements Factory { if (releaseReasonForRetry) this.#scheduleReleaseRetry(record, releaseReasonForRetry) else this.#scheduleDispatchLifecycleRetry(record) } finally { + // Errors before or during provider confirmation leave the marker unset; + // the waiting dispatch will re-read and preserve the foreign-change + // abort. Always settle so a failed write cannot strand that dispatch. + settleIssueWritebackOnce() this.#completionInFlight.delete(completionKey) const stateKey = issueStateKey(record.issue) this.#probePrGhBackoffUntilMs.delete(stateKey) @@ -14441,6 +15212,38 @@ export class FactoryLoop implements Factory { } } + #recordMissingGithubWritebackReceipt(method: 'setStatus' | 'closeIssue'): void { + const implementation = this.#githubWriteback.constructor.name || 'anonymous GithubWriteback' + this.#increment('githubWritebackReceiptMissing') + this.#logger.warn?.('[factory] GitHub writeback returned no ownership receipt', { + implementation, + method, + }) + } + + #settlePostSpawnDispatchWaits( + key: string, + accepted: boolean, + source?: 'abandonment' | 'stop', + ): boolean { + const observation = this.#postSpawnIssueObservations.get(key) + const claimFence = this.#postSpawnDispatchClaimFences.get(key) + if (!observation && !claimFence) return false + // Capture both before either settlement deletes its own map entry. + if (!accepted && claimFence?.claimStarted) { + // The rejection handler clears this provisional block only after a + // conditional provider rollback succeeds. Until then, shutdown and held + // abandonment must not release the lifecycle's agents. + this.#uncompensatedDispatchClaims.add(key) + this.#pendingDispatchClaims.add(key) + } + observation?.settle(accepted) + claimFence?.settle(accepted) + if (source === 'abandonment') this.#increment('postSpawnWaitsSettledByAbandonment') + if (source === 'stop') this.#increment('postSpawnWaitsSettledByStop') + return true + } + #emit(event: FactoryEvent, payload: FactoryEventPayload): void { for (const listener of this.#listeners.get(event) ?? []) { try { @@ -17448,7 +18251,7 @@ const defaultGithubWriteback = (config: FactoryConfig, mount: MountClient): Gith 'GitHub identity "app" requires a connected workspace GitHub App lifecycle write path; refusing to fall back to the local gh user', ) } - return new AppGithubWriteback(mount.githubWrite) + return new AppGithubWriteback(mount.githubWrite, mount.githubRead) } export function parseLinearIssue(path: string, content: unknown): LinearIssue { @@ -20753,6 +21556,10 @@ const UNCLASSIFIED_DISPATCH_FAILURE_LIMIT = 5 const isClassifiedPerItemDispatchFailure = (error: unknown): boolean => error instanceof LiveDispatchStateChangedError || error instanceof DispatchLifecycleClaimRefusedError || + // An external stop/held-agent deadline rejected the post-spawn boundary + // while a provider claim was already in flight. That lifecycle's cleanup is + // owned by the rejecting path, so the late claim unwind is an expected skip. + error instanceof PostSpawnDispatchWaitRejectedError || // #303: the never-placed deadline released this dispatch while its spawn was // still in flight. Named, expected and self-healing — the issue goes back to // the queue — and it recurs under exactly the slow-spawn conditions the @@ -20779,7 +21586,8 @@ const isClassifiedPerItemDispatchFailure = (error: unknown): boolean => */ const mayHaveSpawnedBeforeFailing = (error: unknown): boolean => !(error instanceof LiveDispatchStateChangedError) && - !(error instanceof DispatchLifecycleClaimRefusedError) + !(error instanceof DispatchLifecycleClaimRefusedError) && + !(error instanceof PostSpawnDispatchWaitRejectedError) /** * The run-report reason recorded for a work unit the pass could not dispatch. @@ -20795,6 +21603,7 @@ const perItemDispatchSkipReason = (error: unknown): string => { if (overload) return `relayfile overloaded (${relayfileOverloadReasonLabel(overload.reason)})` if (error instanceof LiveDispatchStateChangedError) return 'live state changed during dispatch' if (error instanceof LatePlacementReleasedError) return 'dispatch released while its agent was still spawning' + if (error instanceof PostSpawnDispatchWaitRejectedError) return 'dispatch terminated during post-spawn claim' if (error instanceof DispatchLifecycleClaimRefusedError) { return error.refusal === 'terminal' ? 'dispatch lifecycle already terminal' diff --git a/src/ports/index.ts b/src/ports/index.ts index d4836146..0f3278d4 100644 --- a/src/ports/index.ts +++ b/src/ports/index.ts @@ -7,6 +7,7 @@ export type { FactoryIntegrationProvider, GithubConnectionIssue, GithubConnectionIssueUpdateInput, + GithubConnectionMutationReceipt, GithubConnectionRead, GithubConnectionWrite, GithubIssueLookup, @@ -41,7 +42,11 @@ export type { } from './fleet' export type { GithubRead, + GithubIssueCloseWriteResult, GithubIssueStatus, + GithubStatusWriteResult, + GithubStatusClaimReceipt, + GithubStatusRollbackResult, GithubWriteback, LinearWriteback, SlackWriteback, diff --git a/src/ports/mount.ts b/src/ports/mount.ts index 5414d3dc..2028d821 100644 --- a/src/ports/mount.ts +++ b/src/ports/mount.ts @@ -137,6 +137,8 @@ export interface FactoryIntegrationConnections { * writeback drafts interpreted by the server-side GitHub adapter. */ export interface GithubConnectionWrite { + /** Authenticated issue read through the same connected GitHub App, when supported. */ + getIssue?(repo: string, number: number): Promise publishPullRequest(input: GithubPublishPullRequestInput): Promise closePullRequest(input: { repo: string; number: number }): Promise /** App-authored issue comment through the workspace GitHub connection. */ @@ -161,11 +163,14 @@ export interface GithubConnectionWrite { operation: 'add' | 'remove' label: string author: 'app' - }): Promise + }): Promise /** App-authored partial issue update through the workspace GitHub connection. */ updateIssue?(input: GithubConnectionIssueUpdateInput): Promise } +/** Whether a connected App mutation proves it created the visible change. */ +export type GithubConnectionMutationReceipt = 'applied' | 'already-matched' | 'acknowledged' + type GithubConnectionIssueUpdateTarget = { repo: string number: number @@ -206,7 +211,11 @@ export interface MountClient { /** Stop SDK-owned local mount processes created by this client. */ dispose?(): Promise readFile(path: string): Promise<{ content: unknown; revision?: string }> - writeFile(path: string, content: unknown, opts?: { guarded?: boolean }): Promise + writeFile(path: string, content: unknown, opts?: { + guarded?: boolean + /** Require this exact source revision and do not retry a conflict. */ + baseRevision?: string + }): Promise<{ targetRevision: string } | void> deleteFile(path: string): Promise setDefaultAllowedDraftPredicate?( predicate: (path: string, content: unknown, opts?: { guarded?: boolean }) => boolean | Promise, diff --git a/src/ports/writeback.ts b/src/ports/writeback.ts index d28a27a9..d413633d 100644 --- a/src/ports/writeback.ts +++ b/src/ports/writeback.ts @@ -2,7 +2,17 @@ import type { GithubPublishPullRequestInput, GithubPublishPullRequestResult } fr import type { LinearIssue, PrSummary } from '../types' export interface LinearWriteback { - setState(issue: LinearIssue, stateId: string): Promise + /** The immutable provider revision is the ownership token for this exact state write. */ + setState(issue: LinearIssue, stateId: string): Promise<{ claimToken: string } | void> + /** Read the provider's current state rather than relying on the dispatch snapshot. */ + getIssueStateId?(issue: LinearIssue): Promise + /** Atomically restore a state only while the provider still matches this exact claim write. */ + compareAndSetState?( + issue: LinearIssue, + expectedStateId: string, + claimToken: string, + stateId: string, + ): Promise<'applied' | 'superseded' | 'unproven'> postComment(issue: LinearIssue, body: string): Promise createIssue(payload: Record): Promise<{ path: string }> verify(issue: LinearIssue, expect: { stateId?: string; commentName?: string }): Promise @@ -19,16 +29,56 @@ export interface GithubRead { export type GithubIssueStatus = 'ready' | 'in-progress' | 'human-review' +/** + * Whether a lifecycle call provably changed the provider's effective status. + * `applied` requires a provider audit event attributed to the writer's actor. + * `acknowledged` means the provider accepted an idempotent operation but its + * API did not prove who created the visible transition. `undefined` preserves + * compatibility with caller-supplied writebacks that predate this receipt. + */ +export type GithubStatusWriteResult = 'applied' | 'already-matched' | 'acknowledged' +export type GithubStatusRollbackResult = 'reverted' | 'superseded' | 'unproven' +export interface GithubStatusClaimReceipt { + result: GithubStatusWriteResult | void + /** Immutable provider event that created this exact effective status. */ + claimToken?: string +} + +/** + * Whether closing an issue provably created the provider's visible transition. + * The same conservative receipt semantics as status writes apply: only an + * `applied` result may establish Factory ownership of the closed state. + */ +export type GithubIssueCloseWriteResult = 'applied' | 'already-matched' | 'acknowledged' + export interface GithubWriteback { /** Optional local-user PR publisher, implemented by the default `gh` writeback. */ publishPullRequest?(input: GithubPublishPullRequestInput): Promise /** Provider-authoritative fallback when the mounted issue record omits its reporter. */ getIssueAuthor?(issue: LinearIssue): Promise /** Provider-authoritative lifecycle lookup used before recovering stale mounted labels. */ - getIssueStatus?(issue: LinearIssue): Promise + getIssueStatus?(issue: LinearIssue, opts?: { + /** Require mutation-relative evidence before trusting a connected non-in-progress projection. */ + requireFresh?: boolean + /** Local instant immediately before the ambiguous provider claim began. */ + freshAfterMs?: number + }): Promise postComment(issue: LinearIssue, body: string): Promise /** Provider-authoritative lookup used to reconcile ambiguous comment writes. */ hasCommentMarker?(issue: LinearIssue, marker: string): Promise - setStatus(issue: LinearIssue, status: GithubIssueStatus): Promise - closeIssue(issue: LinearIssue, body: string): Promise + setStatus(issue: LinearIssue, status: GithubIssueStatus): Promise + /** Claim a status and return immutable provider evidence when the adapter can prove authorship. */ + claimStatus?(issue: LinearIssue, status: GithubIssueStatus): Promise + /** + * Undo one provider status claim without replacing a newer status. The + * adapter must atomically qualify the operation with the immutable claim + * token. Providers without such a primitive must return `unproven` without + * mutating the visible status. + */ + rollbackStatusClaim?( + issue: LinearIssue, + status: GithubIssueStatus, + claimToken: string, + ): Promise + closeIssue(issue: LinearIssue, body: string): Promise } diff --git a/src/state/watch-state-document.ts b/src/state/watch-state-document.ts index 5dfb3f02..706814c3 100644 --- a/src/state/watch-state-document.ts +++ b/src/state/watch-state-document.ts @@ -530,8 +530,11 @@ const validGithubConnection = (value: unknown): boolean => isRecord(value) && const validDispatchClaimStatus = (value: unknown): boolean => isRecord(value) && (value.state === 'pending' || value.state === 'verified' || value.state === 'degraded') && validNumber(value.updatedAtMs) && validOptionalString(value.write) && validOptionalString(value.error) && + validOptionalNumber(value.claimStartedAtMs) && validOptionalNumber(value.attempts) && validOptionalNumber(value.maxAttempts) && - validOptionalBoolean(value.deadLettered) + validOptionalBoolean(value.deadLettered) && + validOptionalBoolean(value.cancellationBlocked) && + validOptionalBoolean(value.cancellationPending) const validPullRequest = (value: unknown): boolean => isRecord(value) && typeof value.repo === 'string' && Number.isSafeInteger(value.number) && (value.number as number) > 0 && diff --git a/src/testing/fakes.ts b/src/testing/fakes.ts index 4fdde641..11401ee6 100644 --- a/src/testing/fakes.ts +++ b/src/testing/fakes.ts @@ -63,12 +63,20 @@ export class FakeMountClient implements MountClient { return { ...entry } } - async writeFile(path: string, content: unknown, _opts?: { guarded?: boolean }): Promise { + async writeFile( + path: string, + content: unknown, + opts?: { guarded?: boolean; baseRevision?: string }, + ): Promise<{ targetRevision: string } | void> { + if (opts?.baseRevision !== undefined && this.files.get(path)?.revision !== opts.baseRevision) { + throw Object.assign(new Error(`Revision conflict for ${path}`), { status: 409 }) + } const revision = String((Number(this.files.get(path)?.revision ?? 0) || 0) + 1) const existing = this.files.get(path)?.content const storedContent = mergedLinearIssueContent(existing, content) ?? content this.files.set(path, { content: storedContent, revision }) this.writes.push({ path, content }) + return { targetRevision: revision } } async deleteFile(path: string): Promise { diff --git a/src/types.ts b/src/types.ts index cdb05eb1..16020806 100644 --- a/src/types.ts +++ b/src/types.ts @@ -433,6 +433,12 @@ export interface FactoryInFlightRegistryAgent { export interface FactoryDispatchClaimStatus { state: 'pending' | 'verified' | 'degraded' + /** Local instant immediately before the provider claim began; recovery requires a newer connected projection. */ + claimStartedAtMs?: number + /** Cancellation could not safely undo the provider claim; keep its lifecycle and placements recoverable. */ + cancellationBlocked?: boolean + /** The provider claim call itself had not settled when ownership was handed off. */ + cancellationPending?: boolean write?: string attempts?: number maxAttempts?: number diff --git a/src/writeback/github.ts b/src/writeback/github.ts index b1bb91f4..34ced957 100644 --- a/src/writeback/github.ts +++ b/src/writeback/github.ts @@ -1,9 +1,16 @@ import { execFile } from 'node:child_process' import { promisify } from 'node:util' -import type { GithubConnectionWrite, MountClient } from '../ports' +import type { GithubConnectionRead, GithubConnectionWrite, MountClient } from '../ports' import type { GithubPublishPullRequestInput, GithubPublishPullRequestResult } from '../ports/mount' -import type { GithubIssueStatus, GithubWriteback } from '../ports/writeback' +import type { + GithubIssueCloseWriteResult, + GithubIssueStatus, + GithubStatusClaimReceipt, + GithubStatusRollbackResult, + GithubStatusWriteResult, + GithubWriteback, +} from '../ports/writeback' import { defaultGhRunner, type GhRunner } from '../github/merge-gate' import type { LinearIssue, PrSummary } from '../types' import { asRecord, wrappedPayload } from './shared' @@ -63,6 +70,30 @@ export interface GhCliGithubWritebackConfig { gitRunner?: GhRunner } +interface GithubLabelEvent { + id: string + event: 'labeled' | 'unlabeled' + label: string + actor: string +} + +interface GithubLabelReceiptBaseline { + actor: string + eventIds: Set + statusLabels: Set +} + +interface GithubIssueStateEvent { + id: string + event: 'closed' | 'reopened' + actor: string +} + +interface GithubIssueCloseReceiptBaseline { + actor: string + eventIds: Set +} + type AppIssueConnectionWrite = GithubConnectionWrite & Required { @@ -99,7 +139,34 @@ export class AppGithubWriteback implements GithubWriteback { }) } - async setStatus(issue: LinearIssue, status: GithubIssueStatus): Promise { + async getIssueStatus( + issue: LinearIssue, + opts: { requireFresh?: boolean; freshAfterMs?: number } = {}, + ): Promise { + const ref = githubIssueRef(issue) + if (this.#connectedRead) { + const connected = await this.#connectedRead.getIssue(ref.repo, ref.number) + if (connected.outcome === 'not-found') return undefined + if (connected.outcome === 'found') { + const status = githubStatusFromLabels(githubLabelsFromContent(connected.issue.content)) + const updatedAtMs = githubIssueUpdatedAtMs(connected.issue.content) + if ( + status === 'in-progress' || + !opts.requireFresh || + (opts.freshAfterMs !== undefined && updatedAtMs !== undefined && updatedAtMs > opts.freshAfterMs) + ) return status + // The canonical projection is readable but predates the ambiguous + // mutation. A public direct read can still supply a live provider + // observation; a private repository fails closed below. + } + } + if (!this.#fallbackRead) return undefined + const fallback = await this.#fallbackRead.getIssue(ref.repo, ref.number) + if (fallback.outcome !== 'found') return undefined + return githubStatusFromLabels(githubLabelsFromContent(fallback.issue.content)) + } + + async setStatus(issue: LinearIssue, status: GithubIssueStatus): Promise { const ref = githubIssueRef(issue) if (status === 'ready') { for (const label of Object.values(FACTORY_GITHUB_STATUS_LABELS)) { @@ -111,7 +178,7 @@ export class AppGithubWriteback implements GithubWriteback { author: 'app', }) } - return + return 'acknowledged' } const target = FACTORY_GITHUB_STATUS_LABELS[status] const previous = FACTORY_GITHUB_STATUS_LABELS[status === 'in-progress' ? 'human-review' : 'in-progress'] @@ -120,7 +187,7 @@ export class AppGithubWriteback implements GithubWriteback { ...target, author: 'app', }) - await this.#write.mutateIssueLabel({ + const addReceipt = await this.#write.mutateIssueLabel({ repo: ref.repo, number: ref.number, operation: 'add', @@ -134,9 +201,31 @@ export class AppGithubWriteback implements GithubWriteback { label: previous.name, author: 'app', }) + // Only the target-label add can prove ownership of the claim that rollback + // may later remove. Mutating the obsolete label is not such a receipt. + return addReceipt ?? 'acknowledged' } - async closeIssue(issue: LinearIssue, body: string): Promise { + async claimStatus(issue: LinearIssue, status: GithubIssueStatus): Promise { + // Relayfile's current GitHub label mutation acknowledges provider success + // but does not expose an immutable label-event identity. Preserve that + // uncertainty explicitly so rejected dispatches fail closed. + return { result: await this.setStatus(issue, status) } + } + + async rollbackStatusClaim( + _issue: LinearIssue, + _status: GithubIssueStatus, + _claimToken: string, + ): Promise { + // GitHub label mutation has no compare-and-set precondition. A read here + // cannot authorize a later remove: an identical newer claim may land in + // between. Until the connection exposes an atomic ownership primitive, + // preserve the status and make the compensation failure observable. + return 'unproven' + } + + async closeIssue(issue: LinearIssue, body: string): Promise { const ref = githubIssueRef(issue) await this.postComment(issue, body) await this.#write.updateIssue({ @@ -145,6 +234,9 @@ export class AppGithubWriteback implements GithubWriteback { state: 'closed', author: 'app', }) + // Relayfile confirms acknowledgement, but this writer has no provider + // audit receipt that distinguishes our close from a concurrent actor's. + return 'acknowledged' } } @@ -298,24 +390,41 @@ export class GhCliGithubWriteback implements GithubWriteback { return result.stdout.includes(marker) } - async setStatus(issue: LinearIssue, status: GithubIssueStatus): Promise { + async setStatus(issue: LinearIssue, status: GithubIssueStatus): Promise { + return (await this.#setStatusWithClaim(issue, status)).result ?? 'acknowledged' + } + + async claimStatus(issue: LinearIssue, status: GithubIssueStatus): Promise { + return await this.#setStatusWithClaim(issue, status) + } + + async #setStatusWithClaim(issue: LinearIssue, status: GithubIssueStatus): Promise { const ref = githubIssueRef(issue) if (status === 'ready') { const labels = await this.#issueLabels(ref) + const statusBefore = githubStatusFromLabels(labels) const editArgs = ['issue', 'edit', String(ref.number), '--repo', ref.repo] for (const label of Object.values(FACTORY_GITHUB_STATUS_LABELS)) { if (labels.has(label.name.toLowerCase())) { editArgs.push('--remove-label', label.name) } } - if (editArgs.length > 5) { + const editRequired = editArgs.length > 5 + const receiptBaseline = editRequired + ? await this.#labelReceiptBaseline(ref, labels).catch(() => undefined) + : undefined + if (editRequired) { await this.#run(editArgs) } const confirmed = await this.#issueLabels(ref) if (Object.values(FACTORY_GITHUB_STATUS_LABELS).some((label) => confirmed.has(label.name.toLowerCase()))) { throw new Error(`GitHub writeback did not confirm removal of Factory status labels on ${ref.repo}#${ref.number}`) } - return + if (!editRequired) return { result: 'already-matched' } + const claimToken = await this.#authoredStatusTransitionToken(ref, receiptBaseline, statusBefore, status) + return claimToken + ? { result: 'applied', claimToken } + : { result: 'acknowledged' } } const target = FACTORY_GITHUB_STATUS_LABELS[status] const previous = FACTORY_GITHUB_STATUS_LABELS[status === 'in-progress' ? 'human-review' : 'in-progress'] @@ -332,6 +441,7 @@ export class GhCliGithubWriteback implements GithubWriteback { '--force', ]) const labels = await this.#issueLabels(ref) + const statusBefore = githubStatusFromLabels(labels) const editArgs = [ 'issue', 'edit', @@ -345,16 +455,49 @@ export class GhCliGithubWriteback implements GithubWriteback { if (labels.has(previous.name.toLowerCase())) { editArgs.push('--remove-label', previous.name) } - if (editArgs.length > 5) { + const editRequired = editArgs.length > 5 + const receiptBaseline = editRequired + ? await this.#labelReceiptBaseline(ref, labels).catch(() => undefined) + : undefined + if (editRequired) { await this.#run(editArgs) } const confirmed = await this.#issueLabels(ref) if (confirmed.has(target.name.toLowerCase()) && !confirmed.has(previous.name.toLowerCase())) { - return + // Removing an obsolete label is a provider mutation, but it does not + // establish ownership when the requested effective status already won + // before our first read (notably human-review over in-progress). + if (statusBefore === status) return { result: 'already-matched' } + const claimToken = await this.#authoredStatusTransitionToken(ref, receiptBaseline, statusBefore, status) + return claimToken + ? { result: 'applied', claimToken } + : { result: 'acknowledged' } } throw new Error(`GitHub writeback did not confirm ${target.name} on ${ref.repo}#${ref.number}`) } + async rollbackStatusClaim( + issue: LinearIssue, + status: GithubIssueStatus, + claimToken: string, + ): Promise { + if (status !== 'in-progress') return 'unproven' + const ref = githubIssueRef(issue) + if (githubStatusFromLabels(await this.#issueLabels(ref)) !== status) return 'superseded' + const events = await this.#issueLabelEvents(ref).catch(() => undefined) + if (!events) return 'unproven' + const claimIndex = events.findIndex((event) => event.id === claimToken) + const claimEvent = claimIndex >= 0 ? events[claimIndex] : undefined + if (claimEvent?.event !== 'labeled' + || claimEvent.label !== FACTORY_GITHUB_STATUS_LABELS['in-progress'].name) return 'unproven' + const statusLabels = new Set(Object.values(FACTORY_GITHUB_STATUS_LABELS).map((label) => label.name)) + if (events.slice(claimIndex + 1).some((event) => statusLabels.has(event.label))) return 'superseded' + // `gh issue edit` has no atomic label-event precondition. Even this exact + // provider token can become stale after the read, so never turn it into an + // unsafe read-then-remove operation. + return 'unproven' + } + async #issueLabels(ref: { repo: string; number: number }): Promise> { const result = await this.#run([ 'issue', @@ -376,9 +519,78 @@ export class GhCliGithubWriteback implements GithubWriteback { ) } - async closeIssue(issue: LinearIssue, body: string): Promise { + async #labelReceiptBaseline( + ref: { repo: string; number: number }, + labels: ReadonlySet, + ): Promise { + const actor = (await this.#run(['api', 'user', '--jq', '.login'])).stdout.trim().toLowerCase() + if (!actor) throw new Error('GitHub lifecycle receipt could not resolve the authenticated actor') + const events = await this.#issueLabelEvents(ref) + const statusLabelNames = new Set(Object.values(FACTORY_GITHUB_STATUS_LABELS).map((label) => label.name)) + return { + actor, + eventIds: new Set(events.map((event) => event.id)), + statusLabels: new Set([...labels].filter((label) => statusLabelNames.has(label))), + } + } + + async #authoredStatusTransitionToken( + ref: { repo: string; number: number }, + baseline: GithubLabelReceiptBaseline | undefined, + from: GithubIssueStatus, + to: GithubIssueStatus, + ): Promise { + if (!baseline) return undefined + const expected = githubStatusTransitionEvent(from, to) + if (!expected) return undefined + const events = await this.#issueLabelEvents(ref).catch(() => []) + const statusLabels = new Set(Object.values(FACTORY_GITHUB_STATUS_LABELS).map((label) => label.name)) + const newStatusEvents = events.filter((event) => + !baseline.eventIds.has(event.id) && statusLabels.has(event.label), + ) + const effectiveLabels = new Set(baseline.statusLabels) + let definingEvent: GithubLabelEvent | undefined + for (const event of newStatusEvents) { + const before = githubStatusFromLabels(effectiveLabels) + if (event.event === 'labeled') effectiveLabels.add(event.label) + else effectiveLabels.delete(event.label) + const after = githubStatusFromLabels(effectiveLabels) + if (before !== to && after === to) definingEvent = event + } + if (githubStatusFromLabels(effectiveLabels) !== to) return undefined + // Attribute the event that last made the confirmed effective status true, + // not later status-label cleanup that leaves the effective status intact. + return definingEvent?.actor === baseline.actor + && definingEvent.event === expected.event + && definingEvent.label === expected.label + ? definingEvent.id + : undefined + } + + async #issueLabelEvents(ref: { repo: string; number: number }): Promise { + const result = await this.#run([ + 'api', + '--paginate', + `repos/${ref.repo}/issues/${ref.number}/events`, + '--jq', + '.[] | select(.event == "labeled" or .event == "unlabeled") | [.id, .event, .label.name, .actor.login] | @tsv', + ]) + return result.stdout + .split('\n') + .map((line) => line.trim()) + .filter(Boolean) + .flatMap((line): GithubLabelEvent[] => { + const [id, event, label, actor] = line.split('\t') + if (!id || (event !== 'labeled' && event !== 'unlabeled') || !label || !actor) return [] + return [{ id, event, label: label.toLowerCase(), actor: actor.toLowerCase() }] + }) + } + + async closeIssue(issue: LinearIssue, body: string): Promise { const ref = githubIssueRef(issue) await this.postComment(issue, body) + if (await this.#issueState(ref) === 'closed') return 'already-matched' + const receiptBaseline = await this.#issueCloseReceiptBaseline(ref).catch(() => undefined) await this.#run([ 'issue', 'close', @@ -388,6 +600,77 @@ export class GhCliGithubWriteback implements GithubWriteback { '--reason', 'completed', ]) + if (await this.#issueState(ref) !== 'closed') { + throw new Error(`GitHub writeback did not confirm closed state on ${ref.repo}#${ref.number}`) + } + return await this.#hasAuthoredIssueClose(ref, receiptBaseline) + ? 'applied' + : 'acknowledged' + } + + async #issueState(ref: { repo: string; number: number }): Promise<'open' | 'closed'> { + const result = await this.#run([ + 'issue', + 'view', + String(ref.number), + '--repo', + ref.repo, + '--json', + 'state', + ]) + const parsed = JSON.parse(result.stdout) as { state?: unknown } + const state = stringValue(parsed.state)?.toLowerCase() + if (state === 'open' || state === 'closed') return state + throw new Error(`GitHub lifecycle read returned an unknown issue state on ${ref.repo}#${ref.number}`) + } + + async #issueCloseReceiptBaseline( + ref: { repo: string; number: number }, + ): Promise { + const actor = (await this.#run(['api', 'user', '--jq', '.login'])).stdout.trim().toLowerCase() + if (!actor) throw new Error('GitHub close receipt could not resolve the authenticated actor') + const events = await this.#issueStateEvents(ref) + return { actor, eventIds: new Set(events.map((event) => event.id)) } + } + + async #hasAuthoredIssueClose( + ref: { repo: string; number: number }, + baseline: GithubIssueCloseReceiptBaseline | undefined, + ): Promise { + if (!baseline) return false + const events = await this.#issueStateEvents(ref).catch(() => []) + const newEvents = events.filter((event) => !baseline.eventIds.has(event.id)) + let state: 'open' | 'closed' = 'open' + let definingClose: GithubIssueStateEvent | undefined + for (const event of newEvents) { + if (event.event === 'closed') { + if (state === 'open') definingClose = event + state = 'closed' + } else { + state = 'open' + definingClose = undefined + } + } + return state === 'closed' && definingClose?.actor === baseline.actor + } + + async #issueStateEvents(ref: { repo: string; number: number }): Promise { + const result = await this.#run([ + 'api', + '--paginate', + `repos/${ref.repo}/issues/${ref.number}/events`, + '--jq', + '.[] | select(.event == "closed" or .event == "reopened") | [.id, .event, .actor.login] | @tsv', + ]) + return result.stdout + .split('\n') + .map((line) => line.trim()) + .filter(Boolean) + .flatMap((line): GithubIssueStateEvent[] => { + const [id, event, actor] = line.split('\t') + if (!id || (event !== 'closed' && event !== 'reopened') || !actor) return [] + return [{ id, event, actor: actor.toLowerCase() }] + }) } async #gitValue(args: string[], description: string): Promise { @@ -401,6 +684,49 @@ export class GhCliGithubWriteback implements GithubWriteback { } } +const githubStatusFromLabels = (labels: Set): GithubIssueStatus => { + if (labels.has(FACTORY_GITHUB_STATUS_LABELS['human-review'].name.toLowerCase())) return 'human-review' + if (labels.has(FACTORY_GITHUB_STATUS_LABELS['in-progress'].name.toLowerCase())) return 'in-progress' + return 'ready' +} + +const githubLabelsFromContent = (content: unknown): Set => { + const payload = wrappedPayload(content) + const labels = Array.isArray(payload.labels) ? payload.labels : [] + return new Set(labels.flatMap((label) => { + if (typeof label === 'string' && label.trim()) return [label.trim().toLowerCase()] + const name = stringValue(asRecord(label)?.name)?.trim().toLowerCase() + return name ? [name] : [] + })) +} + +const githubIssueUpdatedAtMs = (content: unknown): number | undefined => { + const payload = wrappedPayload(content) + const raw = payload.updatedAt ?? payload.updated_at + if (typeof raw === 'number' && Number.isFinite(raw)) return raw + if (typeof raw !== 'string' || !raw.trim()) return undefined + const parsed = Date.parse(raw) + return Number.isFinite(parsed) ? parsed : undefined +} + +const githubStatusTransitionEvent = ( + from: GithubIssueStatus, + to: GithubIssueStatus, +): Pick | undefined => { + if (from === to) return undefined + if (to === 'human-review') { + return { event: 'labeled', label: FACTORY_GITHUB_STATUS_LABELS['human-review'].name } + } + if (to === 'in-progress') { + return from === 'human-review' + ? { event: 'unlabeled', label: FACTORY_GITHUB_STATUS_LABELS['human-review'].name } + : { event: 'labeled', label: FACTORY_GITHUB_STATUS_LABELS['in-progress'].name } + } + return from === 'human-review' + ? { event: 'unlabeled', label: FACTORY_GITHUB_STATUS_LABELS['human-review'].name } + : { event: 'unlabeled', label: FACTORY_GITHUB_STATUS_LABELS['in-progress'].name } +} + const defaultGitRunner: GhRunner = async (args) => { const { stdout, stderr } = await execFileAsync('git', args, { maxBuffer: 1024 * 1024 }) return { stdout, stderr } diff --git a/src/writeback/linear.ts b/src/writeback/linear.ts index 610c951f..19f426f1 100644 --- a/src/writeback/linear.ts +++ b/src/writeback/linear.ts @@ -257,16 +257,94 @@ export const MountLinearWriteback = ( } const adapter = { - async setState(issue: LinearIssue, stateId: string): Promise { + async getIssueStateId(issue: LinearIssue): Promise { + // The primary issue path may be only a sparse change-event stub. Reuse + // the guarded canonical lookup so recovery observes the effective + // by-id/by-uuid record rather than a stale or state-less alias. + const current = await readIssuePayloadForGuard(mount, issue) + // A state-only primary was not a canonical issue read. Without guard + // fields from the full record, treating its state as authoritative could + // release a cancellation fence on stale sparse-sync data. + if (!hasGuardFields(current)) return undefined + if (typeof current.stateId === 'string' && current.stateId) return current.stateId + const state = asRecord(current.state) + if (typeof state?.id === 'string' && state.id) return state.id + // The orchestrator already resolves name-only canonical records through + // its state catalog. Reuse that resolved id only when the live canonical + // name still matches the issue projection it supplied. + if ( + typeof state?.name === 'string' && + state.name && + state.name === issue.state?.name && + issue.stateId + ) return issue.stateId + return undefined + }, + + async setState(issue: LinearIssue, stateId: string): Promise<{ claimToken: string } | void> { const path = issuePath(issue) const canonical = await canonicalForIssue(issue) assertInFactoryScope(scopeIssueFromPayload(canonical.payload, issue.key), safety) - await mount.writeFile(path, { + const receipt = await mount.writeFile(path, { ...canonical.writable, stateId, }, { guarded: true }) + await confirmWriteback(mount, path, () => verifyStateReadback(mount, issue, stateId), logger, readbackConfirm) updateCanonicalState(path, issue, canonical, stateId) + return receipt?.targetRevision + ? { claimToken: receipt.targetRevision } + : undefined + }, + + async compareAndSetState( + issue: LinearIssue, + expectedStateId: string, + claimToken: string, + stateId: string, + ): Promise<'applied' | 'superseded' | 'unproven'> { + const path = issuePath(issue) + const canonical = await canonicalForIssue(issue) + assertInFactoryScope(scopeIssueFromPayload(canonical.payload, issue.key), safety) + const current = await mount.readFile(path) + const currentPayload = wrappedPayload(current.content) + if (currentPayload.stateId !== expectedStateId) return 'superseded' + if (current.revision === undefined) return 'unproven' + // Matching only the effective state is insufficient: another actor may + // have restored the same state after this dispatch claim. The exact + // target revision returned by the original write identifies ownership. + if (current.revision !== claimToken) return 'unproven' + // A revision only protects the exact mounted resource. If that resource + // is a sparse/state-only projection, Factory cannot prove that a full + // rewrite would preserve concurrent provider fields, so fail closed. + if (!payloadInFactoryScope(currentPayload, safety)) return 'unproven' + const currentCanonical: CachedIssuePayload = { + payload: { ...currentPayload }, + writable: createIssueWritePayload(currentPayload), + } + try { + await mount.writeFile(path, { + ...currentCanonical.writable, + stateId, + }, { guarded: true, baseRevision: claimToken }) + } catch (error) { + if (isRevisionConflict(error)) { + // A conflict invalidates the immutable claim token. Re-read only to + // distinguish a real state transition (`superseded`) from a same- + // state edit whose ownership is now ambiguous (`unproven`). Never + // retry with the newer revision: it could belong to another claim + // with the same effective state. + try { + const latest = wrappedPayload((await mount.readFile(path)).content) + return latest.stateId === expectedStateId ? 'unproven' : 'superseded' + } catch { + return 'unproven' + } + } + throw error + } await confirmWriteback(mount, path, () => verifyStateReadback(mount, issue, stateId), logger, readbackConfirm) + updateCanonicalState(path, issue, currentCanonical, stateId) + return 'applied' }, async postComment(issue: LinearIssue, body: string): Promise { @@ -317,6 +395,12 @@ export const MountLinearWriteback = ( return adapter } +const isRevisionConflict = (error: unknown): boolean => + Boolean(error && typeof error === 'object' && ( + ('status' in error && error.status === 409) + || ('statusCode' in error && error.statusCode === 409) + )) + const isStateOnlyDraft = (payload: Record): boolean => { const keys = Object.keys(payload) return keys.length === 1 && keys[0] === 'stateId' && typeof payload.stateId === 'string' diff --git a/src/writeback/writeback.test.ts b/src/writeback/writeback.test.ts index 1e1d472d..8ebe3317 100644 --- a/src/writeback/writeback.test.ts +++ b/src/writeback/writeback.test.ts @@ -1,10 +1,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { FactoryConfigSchema } from '../config/schema' -import { linearCommentPath } from '../constants/linear' +import { linearByIdPath, linearCommentPath } from '../constants/linear' import { slackReplyPath } from '../constants/slack' import { AppGithubWriteback, createFactory, GhCliGithubWriteback, linearCommentName, MountGithubRead, MountLinearWriteback, MountSlackWriteback } from '../index' -import type { GithubConnectionWrite, GithubWriteback, MountClient } from '../ports' +import type { GithubConnectionRead, GithubConnectionWrite, GithubWriteback, MountClient } from '../ports' import type { LinearIssue } from '../types' import { FakeFleetClient, FakeMountClient } from '../testing' @@ -70,7 +70,7 @@ describe('MountLinearWriteback', () => { }) const linear = MountLinearWriteback(mount) - await linear.setState(issue, 'implementing-state') + await expect(linear.setState(issue, 'implementing-state')).resolves.toEqual({ claimToken: '1' }) expect(mount.writes).toEqual([ { @@ -86,6 +86,184 @@ describe('MountLinearWriteback', () => { expect(await linear.verify(issue, { stateId: 'implementing-state' })).toBe(true) }) + it('reads the current mounted Linear state instead of the dispatch snapshot', async () => { + const mount = new FakeMountClient({ + [issuePath]: wrappedIssueRecord({ stateId: 'human-review-state' }), + }) + const linear = MountLinearWriteback(mount) + + await expect(linear.getIssueStateId(issue)).resolves.toBe('human-review-state') + expect(issue.stateId).toBe('ready-state') + }) + + it('reads a name-only current state from the canonical record behind a sparse primary alias', async () => { + const currentIssue: LinearIssue = { + ...issue, + stateId: 'human-review-state', + state: { name: 'In Human Review' }, + } + const mount = new FakeMountClient({ + [issuePath]: { payload: { stateId: 'ready-state' } }, + [linearByIdPath(issueKey)]: wrappedIssueRecord({ + stateId: undefined, + state: { name: 'In Human Review' }, + }), + }) + const linear = MountLinearWriteback(mount) + + await expect(linear.getIssueStateId(currentIssue)).resolves.toBe('human-review-state') + }) + + it('fails closed when only a sparse Linear state alias is readable', async () => { + const mount = new FakeMountClient({ + [issuePath]: { payload: { stateId: 'human-review-state' } }, + }) + + await expect(MountLinearWriteback(mount).getIssueStateId(issue)).resolves.toBeUndefined() + }) + + it('conditionally restores a Linear state at the exact mounted revision', async () => { + const mount = new FakeMountClient() + mount.files.set(issuePath, { content: wrappedIssueRecord({ stateId: 'implementing-state' }), revision: '7' }) + const linear = MountLinearWriteback(mount) + + await expect(linear.compareAndSetState?.(issue, 'implementing-state', '7', 'ready-state')) + .resolves.toBe('applied') + expect(mount.writes).toEqual([expect.objectContaining({ + path: issuePath, + content: expect.objectContaining({ stateId: 'ready-state' }), + })]) + }) + + it('preserves a newer Linear state when the conditional rollback revision loses', async () => { + class RacingMountClient extends FakeMountClient { + override async writeFile( + path: string, + content: unknown, + opts?: { guarded?: boolean; baseRevision?: string }, + ): Promise { + if (opts?.baseRevision !== undefined) { + this.files.set(path, { + content: wrappedIssueRecord({ stateId: 'human-review-state' }), + revision: String(Number(opts.baseRevision) + 1), + }) + } + await super.writeFile(path, content, opts) + } + } + const mount = new RacingMountClient() + mount.files.set(issuePath, { content: wrappedIssueRecord({ stateId: 'implementing-state' }), revision: '7' }) + const linear = MountLinearWriteback(mount) + + await expect(linear.compareAndSetState?.(issue, 'implementing-state', '7', 'ready-state')) + .resolves.toBe('superseded') + expect(mount.writes).toEqual([]) + expect((mount.files.get(issuePath)?.content as { payload: { stateId: string } }).payload.stateId) + .toBe('human-review-state') + }) + + it('recognizes a statusCode-shaped Linear revision conflict', async () => { + class StatusCodeConflictMountClient extends FakeMountClient { + override async writeFile( + path: string, + content: unknown, + opts?: { guarded?: boolean; baseRevision?: string }, + ): Promise { + if (opts?.baseRevision !== undefined) { + this.files.set(path, { + content: wrappedIssueRecord({ stateId: 'human-review-state' }), + revision: String(Number(opts.baseRevision) + 1), + }) + throw Object.assign(new Error(`Revision conflict for ${path}`), { statusCode: 409 }) + } + await super.writeFile(path, content, opts) + } + } + const mount = new StatusCodeConflictMountClient() + mount.files.set(issuePath, { content: wrappedIssueRecord({ stateId: 'implementing-state' }), revision: '7' }) + const linear = MountLinearWriteback(mount) + + await expect(linear.compareAndSetState?.(issue, 'implementing-state', '7', 'ready-state')) + .resolves.toBe('superseded') + expect(mount.writes).toEqual([]) + }) + + it('preserves current Linear fields when conditionally restoring the state', async () => { + const mount = new FakeMountClient() + mount.files.set(issuePath, { + content: wrappedIssueRecord({ + stateId: 'implementing-state', + description: 'A newer operator-authored description', + priority: 1, + }), + revision: '7', + }) + const linear = MountLinearWriteback(mount) + + await expect(linear.compareAndSetState?.(issue, 'implementing-state', '7', 'ready-state')) + .resolves.toBe('applied') + expect(mount.writes.at(-1)?.content).toEqual(expect.objectContaining({ + stateId: 'ready-state', + description: 'A newer operator-authored description', + priority: 1, + })) + }) + + it('fails closed when the exact Linear revision is only a sparse state projection', async () => { + const mount = new FakeMountClient() + mount.files.set(issuePath, { + content: { payload: { stateId: 'implementing-state' } }, + revision: '7', + }) + // Seed the scope-bearing canonical record through the issue object, while + // keeping the mounted compare-and-set target deliberately sparse. + const linear = MountLinearWriteback(mount) + + await expect(linear.compareAndSetState?.(issue, 'implementing-state', '7', 'ready-state')) + .resolves.toBe('unproven') + expect(mount.writes).toEqual([]) + }) + + it('fails closed for an identical newer Linear state whose revision is not the claim token', async () => { + const mount = new FakeMountClient() + mount.files.set(issuePath, { content: wrappedIssueRecord({ stateId: 'implementing-state' }), revision: '8' }) + const linear = MountLinearWriteback(mount) + + await expect(linear.compareAndSetState?.(issue, 'implementing-state', '7', 'ready-state')) + .resolves.toBe('unproven') + expect(mount.writes).toEqual([]) + }) + + it('fails closed when an unrelated Linear edit wins the rollback CAS', async () => { + class UnrelatedEditRacingMountClient extends FakeMountClient { + override async writeFile( + path: string, + content: unknown, + opts?: { guarded?: boolean; baseRevision?: string }, + ): Promise { + if (opts?.baseRevision !== undefined) { + this.files.set(path, { + content: wrappedIssueRecord({ + stateId: 'implementing-state', + description: 'A concurrent description edit', + }), + revision: String(Number(opts.baseRevision) + 1), + }) + } + await super.writeFile(path, content, opts) + } + } + const mount = new UnrelatedEditRacingMountClient() + mount.files.set(issuePath, { content: wrappedIssueRecord({ stateId: 'implementing-state' }), revision: '7' }) + const linear = MountLinearWriteback(mount) + + await expect(linear.compareAndSetState?.(issue, 'implementing-state', '7', 'ready-state')) + .resolves.toBe('unproven') + expect(mount.writes).toEqual([]) + expect((mount.files.get(issuePath)?.content as { payload: { description: string } }).payload.description) + .toBe('A concurrent description edit') + }) + it('builds setState from in-memory writable fields instead of the live mount read', async () => { const richIssue: LinearIssue = { ...issue, @@ -305,6 +483,8 @@ describe('MountLinearWriteback', () => { }) await expect(linear.setState(issue, 'implementing-state')).rejects.toThrow(/read-back never confirmed it landed/u) + expect(issue.stateId).toBe('ready-state') + expect((issue.raw.payload as Record).stateId).toBe('ready-state') await expect(linear.postComment(issue, 'Agent dispatched after stale mirror')).rejects.toThrow(/read-back never confirmed it landed/u) await expect(linear.createIssue({ id: 'uuid-stale-create', @@ -362,7 +542,7 @@ describe('MountLinearWriteback', () => { }) await expect(MountLinearWriteback(mount).setState(issue, 'implementing-state')) - .resolves.toBeUndefined() + .resolves.toEqual({ claimToken: '1' }) expect(mount.writes).toEqual([ { path: issuePath, @@ -826,7 +1006,7 @@ describe('AppGithubWriteback', () => { }, } - it('delegates PRs and lifecycle writes to the app connection without exposing read methods', async () => { + it('delegates PRs and lifecycle writes to the app connection and fails closed without a reader', async () => { const publishPullRequest: GithubConnectionWrite['publishPullRequest'] = vi.fn(async (input) => ({ repo: input.repo, number: 322, @@ -856,9 +1036,9 @@ describe('AppGithubWriteback', () => { body: 'Fixes #221', })).resolves.toMatchObject({ number: 322, author: 'app' }) await app.postComment(appIssue, 'Factory dispatch for 221') - await app.setStatus(appIssue, 'human-review') - await app.setStatus(appIssue, 'ready') - await app.closeIssue(appIssue, 'Factory observed the linked PR merge.') + await expect(app.setStatus(appIssue, 'human-review')).resolves.toBe('acknowledged') + await expect(app.setStatus(appIssue, 'ready')).resolves.toBe('acknowledged') + await expect(app.closeIssue(appIssue, 'Factory observed the linked PR merge.')).resolves.toBe('acknowledged') expect(postIssueComment).toHaveBeenNthCalledWith(1, { repo: 'AgentWorkforce/factory', @@ -894,7 +1074,7 @@ describe('AppGithubWriteback', () => { }) const writeback: GithubWriteback = app expect(writeback.getIssueAuthor).toBeUndefined() - expect(writeback.getIssueStatus).toBeUndefined() + await expect(writeback.getIssueStatus?.(appIssue)).resolves.toBeUndefined() expect(writeback.hasCommentMarker).toBeUndefined() }) @@ -904,6 +1084,214 @@ describe('AppGithubWriteback', () => { closePullRequest: async () => undefined, })).toThrow('requires connected comment, label, and issue-update capabilities') }) + + it('propagates an actor-qualified App label receipt to the status claim', async () => { + const mutateIssueLabel: NonNullable = vi.fn(async (input) => + input.operation === 'add' ? 'applied' : 'already-matched') + const app = new AppGithubWriteback({ + publishPullRequest: async () => { throw new Error('not used') }, + closePullRequest: async () => undefined, + postIssueComment: async () => undefined, + ensureRepositoryLabel: async () => undefined, + mutateIssueLabel, + updateIssue: async () => undefined, + }) + + await expect(app.setStatus(appIssue, 'in-progress')).resolves.toBe('applied') + }) + + it('prefers the connected App issue reader over an unauthenticated fallback', async () => { + const connectedGetIssue = vi.fn(async () => ({ + outcome: 'found' as const, + issue: { + repo: 'AgentWorkforce/factory', + number: 221, + path: appIssue.path, + content: { payload: { labels: [{ name: 'factory:human-review' }] } }, + }, + })) + const fallbackGetIssue = vi.fn(async () => ({ + outcome: 'indeterminate' as const, + reason: 'repository is private', + })) + const app = new AppGithubWriteback({ + getIssue: connectedGetIssue, + publishPullRequest: async () => { throw new Error('not used') }, + closePullRequest: async () => undefined, + postIssueComment: async () => undefined, + ensureRepositoryLabel: async () => undefined, + mutateIssueLabel: async () => undefined, + updateIssue: async () => undefined, + }, { getIssue: fallbackGetIssue }) + + await expect(app.getIssueStatus(appIssue)).resolves.toBe('human-review') + expect(connectedGetIssue).toHaveBeenCalledWith('AgentWorkforce/factory', 221) + expect(fallbackGetIssue).not.toHaveBeenCalled() + }) + + it('falls back to the direct issue reader after an indeterminate connected projection', async () => { + const connectedGetIssue = vi.fn(async () => ({ + outcome: 'indeterminate' as const, + reason: 'connected projection is still migrating', + })) + const fallbackGetIssue = vi.fn(async () => ({ + outcome: 'found' as const, + issue: { + repo: 'AgentWorkforce/factory', + number: 221, + path: appIssue.path, + content: { payload: { labels: [{ name: 'factory:human-review' }] } }, + }, + })) + const app = new AppGithubWriteback({ + getIssue: connectedGetIssue, + publishPullRequest: async () => { throw new Error('not used') }, + closePullRequest: async () => undefined, + postIssueComment: async () => undefined, + ensureRepositoryLabel: async () => undefined, + mutateIssueLabel: async () => undefined, + updateIssue: async () => undefined, + }, { getIssue: fallbackGetIssue }) + + await expect(app.getIssueStatus(appIssue)).resolves.toBe('human-review') + expect(connectedGetIssue).toHaveBeenCalledWith('AgentWorkforce/factory', 221) + expect(fallbackGetIssue).toHaveBeenCalledWith('AgentWorkforce/factory', 221) + }) + + it('requires a connected non-in-progress projection to postdate an ambiguous claim', async () => { + let content: unknown = { + payload: { + updated_at: '2026-08-24T05:00:00.000Z', + labels: [{ name: 'factory' }], + }, + } + const connectedGetIssue = vi.fn(async () => ({ + outcome: 'found' as const, + issue: { repo: 'PrivateOrg/private-repo', number: 221, path: appIssue.path, content }, + })) + const fallbackGetIssue = vi.fn(async () => ({ + outcome: 'indeterminate' as const, + reason: 'repository is private', + })) + const app = new AppGithubWriteback({ + getIssue: connectedGetIssue, + publishPullRequest: async () => { throw new Error('not used') }, + closePullRequest: async () => undefined, + postIssueComment: async () => undefined, + ensureRepositoryLabel: async () => undefined, + mutateIssueLabel: async () => undefined, + updateIssue: async () => undefined, + }, { getIssue: fallbackGetIssue }) + const privateIssue: LinearIssue = { + ...appIssue, + path: '/github/repos/PrivateOrg/private-repo/issues/by-id/221.json', + raw: { + payload: { + source: { + provider: 'github', + id: 'github-221', + owner: 'PrivateOrg', + repo: 'private-repo', + number: 221, + url: 'https://github.com/PrivateOrg/private-repo/issues/221', + }, + }, + }, + } + const opts = { requireFresh: true, freshAfterMs: Date.parse('2026-08-24T05:30:00.000Z') } + + await expect(app.getIssueStatus(privateIssue, opts)).resolves.toBeUndefined() + expect(fallbackGetIssue).toHaveBeenCalledTimes(1) + + content = { + payload: { + updated_at: '2026-08-24T06:00:00.000Z', + labels: [{ name: 'factory:human-review' }], + }, + } + await expect(app.getIssueStatus(privateIssue, opts)).resolves.toBe('human-review') + expect(fallbackGetIssue).toHaveBeenCalledTimes(1) + }) + + it('refuses to roll back an App claim from acknowledgement alone', async () => { + const mutateIssueLabel = vi.fn(async () => undefined) + const getIssue = vi.fn() + const app = new AppGithubWriteback({ + publishPullRequest: async () => { throw new Error('not used') }, + closePullRequest: async () => undefined, + postIssueComment: async () => undefined, + ensureRepositoryLabel: async () => undefined, + mutateIssueLabel, + updateIssue: async () => undefined, + }, { getIssue }) + + await expect(app.rollbackStatusClaim(appIssue, 'in-progress', 'unavailable-token')) + .resolves.toBe('unproven') + expect(getIssue).not.toHaveBeenCalled() + expect(mutateIssueLabel).not.toHaveBeenCalled() + }) + + it('fails closed instead of read-then-removing an App-backed in-progress claim', async () => { + const mutateIssueLabel = vi.fn(async () => undefined) + const connection: GithubConnectionWrite = { + publishPullRequest: async () => { throw new Error('not used') }, + closePullRequest: async () => undefined, + postIssueComment: async () => undefined, + ensureRepositoryLabel: async () => undefined, + mutateIssueLabel, + updateIssue: async () => undefined, + } + const read: GithubConnectionRead = { + getIssue: async () => ({ + outcome: 'found', + issue: { + repo: 'AgentWorkforce/factory', + number: 221, + path: appIssue.path, + content: { payload: { labels: [{ name: 'factory' }, { name: 'factory:in-progress' }] } }, + }, + }), + } + const app = new AppGithubWriteback(connection, read) + + await expect(app.rollbackStatusClaim(appIssue, 'in-progress', 'provider-event-1')) + .resolves.toBe('unproven') + await expect(app.getIssueStatus(appIssue)).resolves.toBe('in-progress') + expect(mutateIssueLabel).not.toHaveBeenCalled() + }) + + it('preserves a newer App-backed human-review status during claim rollback', async () => { + const mutateIssueLabel = vi.fn(async () => undefined) + const connection: GithubConnectionWrite = { + publishPullRequest: async () => { throw new Error('not used') }, + closePullRequest: async () => undefined, + postIssueComment: async () => undefined, + ensureRepositoryLabel: async () => undefined, + mutateIssueLabel, + updateIssue: async () => undefined, + } + const read: GithubConnectionRead = { + getIssue: async () => ({ + outcome: 'found', + issue: { + repo: 'AgentWorkforce/factory', + number: 221, + path: appIssue.path, + content: { + payload: { + labels: [{ name: 'factory:in-progress' }, { name: 'factory:human-review' }], + }, + }, + }, + }), + } + const app = new AppGithubWriteback(connection, read) + + await expect(app.rollbackStatusClaim(appIssue, 'in-progress', 'provider-event-1')) + .resolves.toBe('unproven') + await expect(app.getIssueStatus(appIssue)).resolves.toBe('human-review') + expect(mutateIssueLabel).not.toHaveBeenCalled() + }) }) describe('GhCliGithubWriteback', () => { @@ -929,6 +1317,21 @@ describe('GhCliGithubWriteback', () => { }, }, } + const authenticatedActorCall = ['api', 'user', '--jq', '.login'] + const issueLabelEventsCall = [ + 'api', + '--paginate', + 'repos/AgentWorkforce/factory/issues/48/events', + '--jq', + '.[] | select(.event == "labeled" or .event == "unlabeled") | [.id, .event, .label.name, .actor.login] | @tsv', + ] + const issueStateEventsCall = [ + 'api', + '--paginate', + 'repos/AgentWorkforce/factory/issues/48/events', + '--jq', + '.[] | select(.event == "closed" or .event == "reopened") | [.id, .event, .actor.login] | @tsv', + ] it('pushes a local branch and returns the gh-authenticated PR author', async () => { const ghCalls: string[][] = [] @@ -1042,45 +1445,65 @@ describe('GhCliGithubWriteback', () => { it('sets the first lifecycle status without removing an absent label, then transitions statuses', async () => { const calls: string[][] = [] const labels = new Set() + const events: string[] = [] + let nextEventId = 1 const github = new GhCliGithubWriteback({ runner: async (args) => { calls.push(args) + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } if (args[0] === 'issue' && args[1] === 'view') { return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } } if (args[0] === 'issue' && args[1] === 'edit') { const added = args[args.indexOf('--add-label') + 1] const removed = args[args.indexOf('--remove-label') + 1] - if (args.includes('--add-label') && added) labels.add(added) - if (args.includes('--remove-label') && removed) labels.delete(removed) + if (args.includes('--add-label') && added && !labels.has(added)) { + labels.add(added) + events.push(`${nextEventId++}\tlabeled\t${added}\tfactory-bot`) + } + if (args.includes('--remove-label') && removed && labels.has(removed)) { + labels.delete(removed) + events.push(`${nextEventId++}\tunlabeled\t${removed}\tfactory-bot`) + } } return { stdout: '' } }, }) await github.postComment(githubIssue, 'Factory dispatch for 48') - await github.setStatus(githubIssue, 'in-progress') - await github.setStatus(githubIssue, 'human-review') + await expect(github.setStatus(githubIssue, 'in-progress')).resolves.toBe('applied') + await expect(github.setStatus(githubIssue, 'human-review')).resolves.toBe('applied') expect(calls).toEqual([ ['issue', 'comment', '48', '--repo', 'AgentWorkforce/factory', '--body', 'Factory dispatch for 48'], ['label', 'create', 'factory:in-progress', '--repo', 'AgentWorkforce/factory', '--color', '1d76db', '--description', 'Factory agents are working on this issue.', '--force'], ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'labels'], + authenticatedActorCall, + issueLabelEventsCall, ['issue', 'edit', '48', '--repo', 'AgentWorkforce/factory', '--add-label', 'factory:in-progress'], ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'labels'], + issueLabelEventsCall, ['label', 'create', 'factory:human-review', '--repo', 'AgentWorkforce/factory', '--color', 'fbca04', '--description', 'Factory work is ready for human review.', '--force'], ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'labels'], + authenticatedActorCall, + issueLabelEventsCall, ['issue', 'edit', '48', '--repo', 'AgentWorkforce/factory', '--add-label', 'factory:human-review', '--remove-label', 'factory:in-progress'], ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'labels'], + issueLabelEventsCall, ]) }) it('clears stale lifecycle labels when returning an orphaned issue to ready', async () => { const calls: string[][] = [] const labels = new Set(['factory-ready', 'factory:in-progress', 'factory:human-review']) + const events: string[] = [] + let nextEventId = 1 const github = new GhCliGithubWriteback({ runner: async (args) => { calls.push(args) + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } if (args[0] === 'issue' && args[1] === 'view') { return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }), @@ -1088,17 +1511,22 @@ describe('GhCliGithubWriteback', () => { } if (args[0] === 'issue' && args[1] === 'edit' && args.includes('--remove-label')) { args.forEach((arg, index) => { - if (arg === '--remove-label') labels.delete(args[index + 1]!) + const removed = args[index + 1]! + if (arg === '--remove-label' && labels.delete(removed)) { + events.push(`${nextEventId++}\tunlabeled\t${removed}\tfactory-bot`) + } }) } return { stdout: '' } }, }) - await github.setStatus(githubIssue, 'ready') + await expect(github.setStatus(githubIssue, 'ready')).resolves.toBe('applied') expect(calls).toEqual([ ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'labels'], + authenticatedActorCall, + issueLabelEventsCall, [ 'issue', 'edit', @@ -1111,9 +1539,235 @@ describe('GhCliGithubWriteback', () => { 'factory:human-review', ], ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'labels'], + issueLabelEventsCall, ]) }) + it('does not attribute a label add won by another actor between read and edit', async () => { + const labels = new Set(['factory:in-progress']) + const events: string[] = [] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + if (args[0] === 'issue' && args[1] === 'edit') { + labels.delete('factory:in-progress') + labels.add('factory:human-review') + events.push('1\tunlabeled\tfactory:in-progress\tother-user') + events.push('2\tlabeled\tfactory:human-review\tother-user') + } + return { stdout: '' } + }, + }) + + await expect(github.setStatus(githubIssue, 'human-review')).resolves.toBe('acknowledged') + }) + + it('does not attribute a label removal won by another actor between read and edit', async () => { + const labels = new Set(['factory:human-review']) + const events: string[] = [] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + if (args[0] === 'issue' && args[1] === 'edit') { + labels.delete('factory:human-review') + events.push('1\tunlabeled\tfactory:human-review\tother-user') + } + return { stdout: '' } + }, + }) + + await expect(github.setStatus(githubIssue, 'ready')).resolves.toBe('acknowledged') + }) + + it('does not attribute a park that another actor removes and recreates after the edit', async () => { + const labels = new Set(['factory:in-progress']) + const events: string[] = [] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + if (args[0] === 'issue' && args[1] === 'edit') { + labels.delete('factory:in-progress') + labels.add('factory:human-review') + events.push('1\tlabeled\tfactory:human-review\tfactory-bot') + events.push('2\tunlabeled\tfactory:in-progress\tfactory-bot') + labels.delete('factory:human-review') + labels.add('factory:human-review') + events.push('3\tunlabeled\tfactory:human-review\tother-user') + events.push('4\tlabeled\tfactory:human-review\tother-user') + } + return { stdout: '' } + }, + }) + + await expect(github.setStatus(githubIssue, 'human-review')).resolves.toBe('acknowledged') + }) + + it('returns the immutable defining event when another actor only removes a stale label', async () => { + const labels = new Set(['factory:in-progress']) + const events: string[] = [] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + if (args[0] === 'issue' && args[1] === 'edit') { + labels.add('factory:human-review') + events.push('1\tlabeled\tfactory:human-review\tfactory-bot') + labels.delete('factory:in-progress') + events.push('2\tunlabeled\tfactory:in-progress\tother-user') + } + return { stdout: '' } + }, + }) + + await expect(github.claimStatus(githubIssue, 'human-review')).resolves.toEqual({ + result: 'applied', + claimToken: '1', + }) + }) + + it('does not attribute ready when another actor recreates the final removal', async () => { + const labels = new Set(['factory:human-review']) + const events: string[] = [] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + if (args[0] === 'issue' && args[1] === 'edit') { + labels.delete('factory:human-review') + events.push('1\tunlabeled\tfactory:human-review\tfactory-bot') + labels.add('factory:human-review') + labels.delete('factory:human-review') + events.push('2\tlabeled\tfactory:human-review\tother-user') + events.push('3\tunlabeled\tfactory:human-review\tother-user') + } + return { stdout: '' } + }, + }) + + await expect(github.setStatus(githubIssue, 'ready')).resolves.toBe('acknowledged') + }) + + it.each([ + { status: 'ready' as const, labels: [] }, + { status: 'in-progress' as const, labels: ['factory:in-progress'] }, + ])('does not issue a $status lifecycle edit when the state already matches', async ({ status, labels: initialLabels }) => { + const calls: string[][] = [] + const labels = new Set(initialLabels) + const github = new GhCliGithubWriteback({ + runner: async (args) => { + calls.push(args) + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + return { stdout: '' } + }, + }) + await expect(github.setStatus(githubIssue, status)).resolves.toBe('already-matched') + + expect(calls.some((args) => args[0] === 'issue' && args[1] === 'edit')).toBe(false) + }) + + it('does not claim a status transition for cleanup after human-review already won', async () => { + const calls: string[][] = [] + const labels = new Set(['factory:in-progress', 'factory:human-review']) + const github = new GhCliGithubWriteback({ + runner: async (args) => { + calls.push(args) + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + if (args[0] === 'issue' && args[1] === 'edit' && args.includes('--remove-label')) { + labels.delete('factory:in-progress') + } + return { stdout: '' } + }, + }) + + await expect(github.setStatus(githubIssue, 'human-review')).resolves.toBe('already-matched') + expect(calls.some((args) => args[0] === 'issue' && args[1] === 'edit')).toBe(true) + }) + + it('fails closed when GitHub cannot atomically qualify label removal by the claim event', async () => { + const calls: string[][] = [] + const labels = new Set(['factory', 'factory:in-progress']) + const events = ['claim-1\tlabeled\tfactory:in-progress\tfactory-bot'] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + calls.push(args) + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + return { stdout: '' } + }, + }) + + await expect(github.rollbackStatusClaim(githubIssue, 'in-progress', 'claim-1')) + .resolves.toBe('unproven') + expect(calls.some((args) => args[0] === 'issue' && args[1] === 'edit')).toBe(false) + expect(labels).toEqual(new Set(['factory', 'factory:in-progress'])) + }) + + it('preserves an identical newer GitHub claim with a different defining event', async () => { + const calls: string[][] = [] + const labels = new Set(['factory:in-progress']) + const events = [ + 'claim-1\tlabeled\tfactory:in-progress\tfactory-bot', + '2\tunlabeled\tfactory:in-progress\tother-user', + 'claim-2\tlabeled\tfactory:in-progress\tother-user', + ] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + calls.push(args) + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + return { stdout: '' } + }, + }) + + await expect(github.rollbackStatusClaim(githubIssue, 'in-progress', 'claim-1')) + .resolves.toBe('superseded') + expect(calls.some((args) => args[0] === 'issue' && args[1] === 'edit')).toBe(false) + }) + + it('does not roll back a GitHub claim after human review supersedes it', async () => { + const calls: string[][] = [] + const labels = new Set(['factory:in-progress', 'factory:human-review']) + const github = new GhCliGithubWriteback({ + runner: async (args) => { + calls.push(args) + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ labels: [...labels].map((name) => ({ name })) }) } + } + return { stdout: '' } + }, + }) + + await expect(github.rollbackStatusClaim(githubIssue, 'in-progress', 'claim-1')) + .resolves.toBe('superseded') + expect(calls.some((args) => args[0] === 'issue' && args[1] === 'edit')).toBe(false) + }) + it('rejects an acknowledged lifecycle edit when provider read-back never shows the label', async () => { let edits = 0 const github = new GhCliGithubWriteback({ @@ -1146,18 +1800,76 @@ describe('GhCliGithubWriteback', () => { it('comments and closes the GitHub issue after merge', async () => { const calls: string[][] = [] + let state = 'OPEN' + const events: string[] = [] const github = new GhCliGithubWriteback({ runner: async (args) => { calls.push(args) + if (args[0] === 'issue' && args[1] === 'view' && args.includes('state')) { + return { stdout: JSON.stringify({ state }) } + } + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'close') { + state = 'CLOSED' + events.push('1\tclosed\tfactory-bot') + } return { stdout: '' } }, }) - await github.closeIssue(githubIssue, 'Factory observed the linked pull request merge.') + await expect( + github.closeIssue(githubIssue, 'Factory observed the linked pull request merge.'), + ).resolves.toBe('applied') expect(calls).toEqual([ ['issue', 'comment', '48', '--repo', 'AgentWorkforce/factory', '--body', 'Factory observed the linked pull request merge.'], + ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'state'], + authenticatedActorCall, + issueStateEventsCall, ['issue', 'close', '48', '--repo', 'AgentWorkforce/factory', '--reason', 'completed'], + ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'state'], + issueStateEventsCall, + ]) + }) + + it('does not attribute an issue close won by another actor between read and command', async () => { + let state = 'OPEN' + const events: string[] = [] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + if (args[0] === 'issue' && args[1] === 'view' && args.includes('state')) { + return { stdout: JSON.stringify({ state }) } + } + if (args[0] === 'api' && args[1] === 'user') return { stdout: 'factory-bot\n' } + if (args[0] === 'api' && args[1] === '--paginate') return { stdout: events.join('\n') } + if (args[0] === 'issue' && args[1] === 'close') { + state = 'CLOSED' + events.push('1\tclosed\tother-user') + } + return { stdout: '' } + }, + }) + + await expect(github.closeIssue(githubIssue, 'Factory completion.')).resolves.toBe('acknowledged') + }) + + it('reports an already-closed issue without issuing a close command', async () => { + const calls: string[][] = [] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + calls.push(args) + if (args[0] === 'issue' && args[1] === 'view') { + return { stdout: JSON.stringify({ state: 'CLOSED' }) } + } + return { stdout: '' } + }, + }) + + await expect(github.closeIssue(githubIssue, 'Factory completion.')).resolves.toBe('already-matched') + expect(calls).toEqual([ + ['issue', 'comment', '48', '--repo', 'AgentWorkforce/factory', '--body', 'Factory completion.'], + ['issue', 'view', '48', '--repo', 'AgentWorkforce/factory', '--json', 'state'], ]) })