diff --git a/src/dispatch/templates.test.ts b/src/dispatch/templates.test.ts index 90dbdf1..6d536db 100644 --- a/src/dispatch/templates.test.ts +++ b/src/dispatch/templates.test.ts @@ -35,6 +35,11 @@ describe('renderAgentTask', () => { expect(task).toContain('Do not run `gh pr create` or require local GitHub CLI authentication.') expect(task).toContain('Factory will hand the opened PR to reviewer `ar-123-review`.') expect(task).toContain('DM `factory` with `[factory-needs-input]`') + expect(task).toContain('source GitHub issue when available') + expect(task).toContain('operator-visible delivery error') + expect(task).toContain('No durable Slack dispatch thread was established') + expect(task).toContain('Keep the session available after asking') + expect(task).not.toContain('release the whole team and resume it from session memory') expect(task).toContain('DM `broker` when fully done.') expect(task).toContain('Do NOT auto-merge.') expect(task).toContain('Merge policy: never') @@ -257,6 +262,8 @@ describe('renderAgentTask', () => { expect(task).toContain('DM `factory` with `[factory-needs-input]`') expect(task).toContain('/work/.integrations/slack/channels/C123/messages/169_000/replies/question.json') + expect(task).toContain('source GitHub issue when available') + expect(task).toContain('operator-visible delivery error') expect(task).toContain('Do not wait or poll') expect(task).toContain('release the whole team and resume it from session memory only after the first human reply') expect(task).toContain('cold-start the team with the issue, question, reply, branch, and PR context') diff --git a/src/dispatch/templates.ts b/src/dispatch/templates.ts index 0045a8f..ad839e3 100644 --- a/src/dispatch/templates.ts +++ b/src/dispatch/templates.ts @@ -94,22 +94,26 @@ export function renderAgentTask(input: RenderAgentTaskInput): string { 'When implementation is complete, finish your session normally; Factory will open the PR targeting the repository default branch through the connected GitHub workspace.', 'Do not run `gh pr create` or require local GitHub CLI authentication.', `Factory will hand the opened PR to reviewer \`${input.reviewerName}\`.`, - 'If blocked and you need human input, DM `factory` with `[factory-needs-input]`, the issue key, and one concrete question. Factory will relay it to the issue conversation.', 'DM `broker` when fully done.', 'Do NOT auto-merge.', mergePolicyLine(input.config.mergePolicy), ] - const questionInstructions = input.slackDispatchThread - ? [ - '', - 'If you are blocked or need a human answer mid-task, finish any safe reversible work first, then DM `factory` with `[factory-needs-input]`, the issue key, and one concrete question.', + const questionInstructions = [ + '', + 'If you are blocked or need a human answer mid-task, finish any safe reversible work first, then DM `factory` with `[factory-needs-input]`, the issue key, and one concrete question.', + 'Factory will route the question through the issue Slack thread when available and healthy, otherwise through the source GitHub issue when available. If neither write path works, Factory will emit an operator-visible delivery error instead of silently discarding the question.', + ...(input.slackDispatchThread + ? [ // Absolute path: the agent runs in its repo clone, not the daemon cwd // where .integrations lives, so a relative path would be unreachable. - `Factory will durably post and, if necessary, retry the question to the Slack thread represented at ${input.slackDispatchThread.mountRoot}/slack/channels/${input.slackDispatchThread.channel}/messages/${input.slackDispatchThread.threadId.replaceAll('.', '_')}/replies/question.json.`, + `Factory will durably post and, if necessary, retry the question through that routing policy; the primary Slack thread is represented at ${input.slackDispatchThread.mountRoot}/slack/channels/${input.slackDispatchThread.channel}/messages/${input.slackDispatchThread.threadId.replaceAll('.', '_')}/replies/question.json.`, 'After sending the marker, stop work and finish your session normally. Do not wait or poll: Factory will release the whole team and resume it from session memory only after the first human reply.', 'If session resume is unavailable, Factory will cold-start the team with the issue, question, reply, branch, and PR context so work can be re-hydrated explicitly.', ] - : [] + : [ + 'No durable Slack dispatch thread was established for this task. Keep the session available after asking so Factory can inject a GitHub reply if that fallback is available.', + ]), + ] if (input.role === 'babysitter') { const prRef = input.pr diff --git a/src/mount/relayfile-cloud-mount-client.test.ts b/src/mount/relayfile-cloud-mount-client.test.ts index b001092..d0ae44f 100644 --- a/src/mount/relayfile-cloud-mount-client.test.ts +++ b/src/mount/relayfile-cloud-mount-client.test.ts @@ -429,6 +429,7 @@ describe('RelayfileCloudMountClient', () => { status: 'ready', lastEventAt: '2026-06-12T10:00:00.000Z', lagSeconds: 42, + webhookHealthy: true, }], })) const mount = new RelayfileCloudMountClient({ workspaceId: 'rw_test', client: fake }) @@ -440,10 +441,81 @@ describe('RelayfileCloudMountClient', () => { lastEventAtMs: Date.parse('2026-06-12T10:00:00.000Z'), watermarkTs: '2026-06-12T10:00:00.000Z', lagSeconds: 42, + webhookHealthy: true, }) expect(fake.getSyncStatus).toHaveBeenCalledWith('rw_test', { provider: 'slack' }) }) + it('normalizes snake-case webhook health as independent provider freshness', async () => { + const fake = new FakeRelayFileClient() + fake.getSyncStatus = vi.fn(async () => ({ + status: 'ready', + connections: [{ + provider: 'slack', + status: 'lagging', + last_event_at: '2026-06-06T12:05:00.000Z', + webhook_healthy: true, + }], + })) + const mount = new RelayfileCloudMountClient({ workspaceId: 'rw_test', client: fake }) + + await expect(mount.getSyncStatus?.('slack')).resolves.toEqual({ + provider: 'slack', + status: 'lagging', + lastEventAt: '2026-06-06T12:05:00.000Z', + lastEventAtMs: Date.parse('2026-06-06T12:05:00.000Z'), + watermarkTs: '2026-06-06T12:05:00.000Z', + lagSeconds: undefined, + webhookHealthy: true, + }) + }) + + it.each([ + { webhookHealthy: true, label: 'healthy' }, + { webhookHealthy: false, label: 'unhealthy' }, + ])('preserves nested sync freshness when wrapper webhook delivery is $label', async ({ webhookHealthy }) => { + const fake = new FakeRelayFileClient() + fake.getSyncStatus = vi.fn(async () => ({ + webhookHealthy, + connections: [{ + provider: 'slack', + status: 'lagging', + lastEventAt: '2026-06-06T12:05:00.000Z', + lagSeconds: 86_400, + }], + })) + const mount = new RelayfileCloudMountClient({ workspaceId: 'rw_test', client: fake }) + + await expect(mount.getSyncStatus?.('slack')).resolves.toEqual({ + provider: 'slack', + status: 'lagging', + lastEventAt: '2026-06-06T12:05:00.000Z', + lastEventAtMs: Date.parse('2026-06-06T12:05:00.000Z'), + watermarkTs: '2026-06-06T12:05:00.000Z', + lagSeconds: 86_400, + webhookHealthy, + }) + }) + + it('lets an explicit unhealthy signal win across split provider metadata', async () => { + const fake = new FakeRelayFileClient() + fake.getSyncStatus = vi.fn(async () => ({ + webhookHealthy: true, + connections: [{ + provider: 'slack', + status: 'ready', + last_event_at: '2026-06-12T10:00:00.000Z', + webhook_healthy: false, + }], + })) + const mount = new RelayfileCloudMountClient({ workspaceId: 'rw_test', client: fake }) + + await expect(mount.getSyncStatus?.('slack')).resolves.toMatchObject({ + lastEventAt: '2026-06-12T10:00:00.000Z', + webhookHealthy: false, + }) + }) + it('prefers nested provider freshness over wrapper status metadata', async () => { const fake = new FakeRelayFileClient() fake.getSyncStatus = vi.fn(async () => ({ @@ -463,6 +535,7 @@ describe('RelayfileCloudMountClient', () => { lastEventAtMs: 1_781_267_200_000, watermarkTs: undefined, lagSeconds: 12, + webhookHealthy: undefined, }) }) diff --git a/src/mount/relayfile-cloud-mount-client.ts b/src/mount/relayfile-cloud-mount-client.ts index 03fdc99..ca0effe 100644 --- a/src/mount/relayfile-cloud-mount-client.ts +++ b/src/mount/relayfile-cloud-mount-client.ts @@ -494,6 +494,15 @@ const normalizeProviderSyncStatus = (value: unknown, provider: string): Provider numberField(source, 'last_event_at_ms') ?? (lastEventAt ? Date.parse(lastEventAt) : undefined) const watermarkTs = stringField(source, 'watermarkTs') ?? stringField(source, 'watermark_ts') ?? lastEventAt + // Webhook health is independent of sync freshness and may be reported on a + // wrapper while the provider watermark lives on a nested connection. Keep + // freshness source selection focused on sync fields, then reconcile health + // across every applicable shape. A reported false wins conservatively: a + // false-positive healthy result would send questions to an unread Slack. + const webhookHealthySignals = [source, ...candidates] + .map((candidate) => + booleanField(candidate, 'webhookHealthy') ?? booleanField(candidate, 'webhook_healthy')) + .filter((signal): signal is boolean => signal !== undefined) return { provider: stringField(source, 'provider') ?? provider, status: stringField(source, 'status'), @@ -501,6 +510,7 @@ const normalizeProviderSyncStatus = (value: unknown, provider: string): Provider lastEventAtMs: Number.isFinite(lastEventAtMs) ? lastEventAtMs : undefined, watermarkTs, lagSeconds: numberField(source, 'lagSeconds') ?? numberField(source, 'lag_seconds'), + webhookHealthy: webhookHealthySignals.includes(false) ? false : webhookHealthySignals[0], } } @@ -523,6 +533,9 @@ const stringField = (record: Record, key: string): string | und const numberField = (record: Record, key: string): number | undefined => typeof record[key] === 'number' ? record[key] : undefined +const booleanField = (record: Record, key: string): boolean | undefined => + typeof record[key] === 'boolean' ? record[key] : undefined + const asRecord = (value: unknown): Record | undefined => value !== null && typeof value === 'object' && !Array.isArray(value) ? value as Record : undefined diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index bc0f76c..d5db5d9 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -19,6 +19,7 @@ import { readFactoryLoopHeartbeat, reapFactoryOrphansOnce, type FactoryConfig, + type FactoryEventPayload, type TriageDecision, type TriageEngine, type WorkflowRunnerInput, @@ -258,6 +259,91 @@ class FailingGithubCommentWriteback extends RecordingGithubWriteback { } } +class ImmediateGithubReplyWriteback extends RecordingGithubWriteback { + constructor( + private readonly mount: FakeMountClient, + private readonly issueNumber: number, + private readonly author: string, + ) { + super() + } + + override async postComment(issue: LinearIssue, body: string): Promise { + await super.postComment(issue, body) + if (!body.includes(' needs input.')) return + const prefix = githubReplyPrefixFromComment(body) + emitGithubIssueComment(this.mount, 'AgentWorkforce', 'pear', this.issueNumber, 9901, { + body: `${prefix} Preserve this fast fallback reply.`, + author: { login: this.author }, + }) + // Keep postComment unresolved long enough for the subscribed reply handler + // to claim the answer while the delivery lease is still held. + await flush() + await flush() + } +} + +class MirroringGithubQuestionWriteback extends RecordingGithubWriteback { + constructor( + private readonly mount: FakeMountClient, + private readonly issueNumber: number, + ) { + super() + } + + override async postComment(issue: LinearIssue, body: string): Promise { + await super.postComment(issue, body) + if (!body.includes(' needs input.')) return + emitGithubIssueComment(this.mount, 'AgentWorkforce', 'pear', this.issueNumber, 9701, { + body, + author: { login: 'factory[bot]', type: 'Bot' }, + }) + } +} + +class AmbiguousMirroringGithubQuestionWriteback extends MirroringGithubQuestionWriteback { + ambiguousQuestionPostsRemaining = 1 + + override async postComment(issue: LinearIssue, body: string): Promise { + await super.postComment(issue, body) + if (body.includes(' needs input.') && this.ambiguousQuestionPostsRemaining > 0) { + this.ambiguousQuestionPostsRemaining -= 1 + throw new Error('transport closed after GitHub accepted the comment') + } + } +} + +class RecordingQuestionDeliveryRenewalFileStateStore extends FileStateStore { + questionDeliveryRenewals = 0 + completedQuestionPostedAtMs?: number + + override async renewClarificationQuestionDelivery( + workspaceId: string, + issueKey: string, + owner: string, + nowMs: number, + ): Promise { + this.questionDeliveryRenewals += 1 + return await super.renewClarificationQuestionDelivery(workspaceId, issueKey, owner, nowMs) + } + + override async completeClarificationQuestionDelivery( + workspaceId: string, + issueKey: string, + owner: string, + postedAtMs: number, + ): Promise { + const completed = await super.completeClarificationQuestionDelivery( + workspaceId, + issueKey, + owner, + postedAtMs, + ) + if (completed) this.completedQuestionPostedAtMs = postedAtMs + return completed + } +} + class CountingTriage extends StaticTriage { count = 0 @@ -330,6 +416,28 @@ class TransientClarificationRenewalStateStore extends InMemoryStateStore { } } +class RecordingQuestionDeliveryStateStore extends InMemoryStateStore { + completedQuestionPostedAtMs?: number + + override async completeClarificationQuestionDelivery( + workspaceId: string, + issueKey: string, + owner: string, + postedAtMs: number, + ): Promise { + const completed = await super.completeClarificationQuestionDelivery( + workspaceId, + issueKey, + owner, + postedAtMs, + ) + this.completedQuestionPostedAtMs = ( + await this.getWaitingClarification(workspaceId, issueKey) + )?.questionPostedAtMs + return completed + } +} + class EscalatingTriage extends StaticTriage { readonly overrides: Partial> @@ -1145,6 +1253,20 @@ class ConfirmRecordingSlackMountClient extends CloudWritebackFakeMountClient { } } +class SlackStatusConfirmMountClient extends ConfirmRecordingSlackMountClient { + slackStatus: ProviderSyncStatus | undefined + eventsReadCount = 0 + + async getSyncStatus(provider: string): Promise { + return provider === 'slack' ? this.slackStatus : undefined + } + + override async getEvents(opts: { cursor?: string; limit?: number; provider?: string; last?: number }): Promise { + this.eventsReadCount += 1 + return await super.getEvents(opts) + } +} + class BlockingSlackQuestionMountClient extends ConfirmRecordingSlackMountClient { readonly questionWriteStarted: Promise #signalQuestionWriteStarted!: () => void @@ -1186,6 +1308,33 @@ class FailNextSlackReplyMountClient extends CloudWritebackFakeMountClient { } } +class FailingGithubCommentReconciliationMountClient extends FailNextSlackReplyMountClient { + failGithubCommentReconciliation: false | 'list' | 'read' | 'issue' = false + + override async listTree(prefix: string): Promise { + if ( + this.failGithubCommentReconciliation === 'list' && + ( + prefix === '/github/repos/AgentWorkforce/pear/issues' || + prefix === '/github/repos/AgentWorkforce__pear/issues' + ) + ) { + throw new Error('GitHub comment mirror is unavailable') + } + return await super.listTree(prefix) + } + + override async readFile(path: string): Promise<{ content: unknown; revision?: string }> { + if (this.failGithubCommentReconciliation === 'issue' && path.endsWith('/issues/by-id/70.json')) { + throw new Error('GitHub source issue is temporarily unavailable') + } + if (this.failGithubCommentReconciliation === 'read' && path.includes('/comments/')) { + throw new Error('GitHub comment mirror read is unavailable') + } + return await super.readFile(path) + } +} + class FailSlackRootMountClient extends CloudWritebackFakeMountClient { slackStatus: ProviderSyncStatus | undefined @@ -8599,12 +8748,95 @@ describe('FactoryLoop', () => { }) }) + it('uses independent webhook health to bootstrap Slack threads on cold start and restart', async () => { + const clock = new ManualClock() + const mount = new SlackStatusConfirmMountClient({ + [issuePath(152)]: issueFile(152), + [issuePath(153)]: issueFile(153), + }) + mount.slackStatus = { + provider: 'slack', + status: 'lagging', + lastEventAt: new Date(clock.now() - 24 * 60 * 60_000).toISOString(), + webhookHealthy: true, + } + + const first = createFactory(config({ slack: slackConfig() }), { + mount, + fleet: new FakeFleetClient(), + triage: new StaticTriage(), + clock, + }) + await first.dispatch(await first.triageIssue(parseLinearIssue(issuePath(152), issueFile(152)))) + expect(first.status().counters.slackGateBypassedByWebhookHealth).toBe(1) + expect(first.status().counters.slackEventWatermarkRefreshes).toBeUndefined() + await first.stop() + + const restarted = createFactory(config({ slack: slackConfig() }), { + mount, + fleet: new FakeFleetClient(), + triage: new StaticTriage(), + clock, + }) + await restarted.dispatch(await restarted.triageIssue(parseLinearIssue(issuePath(153), issueFile(153)))) + + expect(mount.writes.filter((write) => isSlackRootWritePath(write.path))).toHaveLength(2) + expect(restarted.status()).toMatchObject({ + slackDegraded: false, + counters: { slackGateBypassedByWebhookHealth: 1 }, + }) + expect(restarted.status().counters.slackEventWatermarkRefreshes).toBeUndefined() + await restarted.stop() + }) + + it('bypasses a false stale status from independently observed webhook arrival time', async () => { + const clock = new ManualClock() + const mount = new SlackStatusConfirmMountClient({ + [issuePath(150)]: issueFile(150), + [issuePath(151)]: issueFile(151), + }) + const factory = createFactory(config({ slack: slackConfig() }), { + mount, + fleet: new FakeFleetClient(), + triage: new StaticTriage(), + clock, + }) + + await factory.dispatch(await factory.triageIssue(parseLinearIssue(issuePath(150), issueFile(150)))) + mount.slackStatus = { + provider: 'slack', + status: 'lagging', + lastEventAt: new Date(clock.now() - 24 * 60 * 60_000).toISOString(), + } + const staleProviderTimestamp = changeEvent( + slackReplyFixturePath('C0FACTORY__factory-e2e', mount.threadTs, 'observed-webhook-150'), + 'observed-webhook-150', + new Date(clock.now() - 24 * 60 * 60_000).toISOString(), + ) + mount.emit({ + ...staleProviderTimestamp, + resource: { ...staleProviderTimestamp.resource, provider: 'slack' }, + }) + + await factory.dispatch(await factory.triageIssue(parseLinearIssue(issuePath(151), issueFile(151)))) + + expect(mount.writes.filter((write) => isSlackRootWritePath(write.path))).toHaveLength(2) + expect(factory.status()).toMatchObject({ + slackDegraded: false, + counters: { + slackWebhookEventsObserved: 1, + slackGateBypassedByObservedEvent: 1, + }, + }) + await factory.stop() + }) + it('keeps hard Slack sync failures blocking even when the Slack event watermark is fresh', async () => { for (const status of ['error', 'failed']) { const clock = new ManualClock() const issueNumber = status === 'error' ? 142 : 143 const mount = new SlackSyncStatusMount({ [issuePath(issueNumber)]: issueFile(issueNumber) }) - mount.slackStatus = { provider: 'slack', status } + mount.slackStatus = { provider: 'slack', status, webhookHealthy: true } const slackEvent = changeEvent( `/slack/channels/C0FACTORY__factory-e2e/messages/${status}/meta.json`, `slack-fresh-${status}`, @@ -9759,6 +9991,10 @@ describe('FactoryLoop', () => { await vi.waitFor(() => expect(firstFactory.status().counters.agentQuestionTeamsReleased).toBe(1)) await vi.waitFor(() => expect(firstFactory.status().counters.clarificationQuestionDeliveryFailures).toBe(1)) + await vi.waitFor(async () => { + const waiting = (await firstState.listWaitingClarifications('factory-test'))[0]?.[1] + expect(waiting?.questionDelivery?.owner).toBe('') + }) const pending = (await firstState.listWaitingClarifications('factory-test'))[0]?.[1] expect(pending).toMatchObject({ question: 'Preserve and retry this question.', @@ -10206,6 +10442,346 @@ describe('FactoryLoop', () => { expect(factory.status().counters.githubAnswersInjected).toBe(1) }) + it('falls back to the source GitHub issue when stale Slack prevented a dispatch thread', async () => { + const path = githubIssuePath('AgentWorkforce', 'pear', 67) + const issue = githubIssueFile(67, { labels: ['factory'], author: 'reporter' }) + const mount = new SlackStatusConfirmMountClient({ [path]: issue }) + mount.slackStatus = { provider: 'slack', status: 'stale' } + const fleet = new FakeFleetClient() + const githubWriteback = new RecordingGithubWriteback() + const factory = createFactory(config({ issueSource: 'github', slack: slackConfig() }), { + mount, + fleet, + triage: new StaticTriage(), + githubWriteback, + }) + + await factory.dispatch(await factory.triageIssue(parseGithubFactoryIssue(path, issue))) + expect(mount.writes.filter((write) => isSlackRootWritePath(write.path))).toEqual([]) + fleet.emitAgentMessage({ + from: 'ar-67-impl-pear', + target: 'factory', + body: '[factory-needs-input] Which fallback remains available?', + eventId: 'github-slack-fallback-question-67', + }) + + await vi.waitFor(() => expect(factoryGithubQuestionComments(githubWriteback)).toHaveLength(1)) + const comment = factoryGithubQuestionComments(githubWriteback)[0]?.body + expect(comment).toContain('Slack fallback reason: Slack writeback is degraded: slack sync status is stale.') + expect(factory.status().counters.agentQuestionsRoutedToGithubFallback).toBe(1) + expect(fleet.releases).toEqual([]) + + const replyPrefix = githubReplyPrefixFromComment(comment) + emitGithubIssueComment(mount, 'AgentWorkforce', 'pear', 67, 9801, { + body: `${replyPrefix} Use the GitHub fallback.`, + author: { login: 'reporter' }, + }) + await vi.waitFor(() => expect(fleet.inputs).toContainEqual({ + name: 'ar-67-impl-pear', + data: '\nHuman reply from @reporter on the GitHub issue:\nUse the GitHub fallback.\n\r', + })) + await factory.stop() + }) + + it('preserves the durable park and wake lifecycle when a Slack thread falls back to GitHub', async () => { + const path = githubIssuePath('AgentWorkforce', 'pear', 68) + const issue = githubIssueFile(68, { labels: ['factory'], author: 'reporter' }) + const mount = new SlackStatusConfirmMountClient({ [path]: issue }) + const fleet = new FakeFleetClient() + fleet.setSessionRef('ar-68-impl-pear', 'session-ar-68-impl-pear') + fleet.setSessionRef('ar-68-review-pear', 'session-ar-68-review-pear') + const stateStore = new InMemoryStateStore({ batchSize: 2 }) + const githubWriteback = new RecordingGithubWriteback() + const factory = createFactory(config({ issueSource: 'github', slack: slackConfig() }), { + mount, + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + }) + + await factory.dispatch(await factory.triageIssue(parseGithubFactoryIssue(path, issue))) + expect(mount.writes.filter((write) => isSlackRootWritePath(write.path))).toHaveLength(1) + mount.files.set(path, { content: githubIssueFile(68, { labels: ['factory', 'factory:in-progress'], author: 'reporter' }) }) + mount.slackStatus = { provider: 'slack', status: 'stale' } + fleet.emitAgentMessage({ + from: 'ar-68-impl-pear', + target: 'factory', + body: '[factory-needs-input] Keep the durable lifecycle through fallback.', + eventId: 'github-durable-fallback-question-68', + }) + + await vi.waitFor(() => expect(factory.status().counters.agentQuestionTeamsReleased).toBe(1)) + await vi.waitFor(() => expect(factory.status().counters.clarificationQuestionsDeliveredViaGithub).toBe(1)) + expect(fleet.releases).toEqual([ + { name: 'ar-68-impl-pear', reason: 'waiting-for-human' }, + { name: 'ar-68-review-pear', reason: 'waiting-for-human' }, + ]) + expect(slackReplyWrites(mount)).toEqual([]) + const waiting = (await stateStore.listWaitingClarifications('factory-test'))[0]?.[1] + expect(waiting?.questionPostedAtMs).toBeTypeOf('number') + const comment = factoryGithubQuestionComments(githubWriteback)[0]?.body + expect(comment).toContain('Slack fallback reason: Slack writeback is degraded: slack sync status is stale.') + + const replyPrefix = githubReplyPrefixFromComment(comment) + emitGithubIssueComment(mount, 'AgentWorkforce', 'pear', 68, 9802, { + body: `${replyPrefix} Resume from the saved sessions.`, + author: { login: 'reporter' }, + }) + + await vi.waitFor(() => expect(factory.status().counters.clarificationTeamsWoken).toBe(1)) + expect(fleet.resumes.map((resume) => resume.sessionRef)).toEqual([ + 'session-ar-68-impl-pear', + 'session-ar-68-review-pear', + ]) + expect(slackAnswerInputs(fleet).map((input) => input.data)).toEqual([ + '\nHuman reply from @reporter on the GitHub issue:\nResume from the saved sessions.\n\r', + '\nHuman reply from @reporter on the GitHub issue:\nResume from the saved sessions.\n\r', + ]) + expect(await stateStore.listWaitingClarifications('factory-test')).toEqual([]) + await factory.stop() + }) + + it('keeps the delivery lease through a confirmed Slack write failure and fast GitHub reply', async () => { + const path = githubIssuePath('AgentWorkforce', 'pear', 69) + const issue = githubIssueFile(69, { labels: ['factory'], author: 'reporter' }) + const mount = new FailNextSlackReplyMountClient({ [path]: issue }) + const fleet = new FakeFleetClient() + fleet.setSessionRef('ar-69-impl-pear', 'session-ar-69-impl-pear') + fleet.setSessionRef('ar-69-review-pear', 'session-ar-69-review-pear') + const stateStore = new RecordingQuestionDeliveryStateStore({ batchSize: 2 }) + const githubWriteback = new ImmediateGithubReplyWriteback(mount, 69, 'reporter') + const factory = createFactory(config({ issueSource: 'github', slack: slackConfig() }), { + mount, + fleet, + stateStore, + triage: new StaticTriage(), + githubWriteback, + }) + + await factory.dispatch(await factory.triageIssue(parseGithubFactoryIssue(path, issue))) + mount.files.set(path, { content: githubIssueFile(69, { labels: ['factory', 'factory:in-progress'], author: 'reporter' }) }) + mount.failNextReply = true + fleet.emitAgentMessage({ + from: 'ar-69-impl-pear', + target: 'factory', + body: '[factory-needs-input] Do not lose a reply during write failure fallback.', + eventId: 'github-write-failure-fallback-question-69', + }) + + await vi.waitFor(() => expect(factory.status().counters.clarificationTeamsWoken).toBe(1)) + expect(mount.failedReplies).toBe(1) + expect(factoryGithubQuestionComments(githubWriteback)).toHaveLength(1) + expect(stateStore.completedQuestionPostedAtMs).toBeTypeOf('number') + expect(factory.status()).toMatchObject({ + counters: { + clarificationQuestionDeliveryFailures: 1, + clarificationQuestionsDeliveredViaGithub: 1, + githubClarificationRepliesClaimed: 1, + }, + }) + expect(fleet.resumes.map((resume) => resume.sessionRef)).toEqual([ + 'session-ar-69-impl-pear', + 'session-ar-69-review-pear', + ]) + expect(slackAnswerInputs(fleet).map((input) => input.data)).toEqual([ + '\nHuman reply from @reporter on the GitHub issue:\nPreserve this fast fallback reply.\n\r', + '\nHuman reply from @reporter on the GitHub issue:\nPreserve this fast fallback reply.\n\r', + ]) + expect(await stateStore.listWaitingClarifications('factory-test')).toEqual([]) + await factory.stop() + }) + + it('reconciles an ambiguous accepted GitHub fallback through mirror failure and lease handoff', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-github-clarification-fallback-restart-')) + try { + const watchStatePath = join(root, 'factory-state.json') + const path = githubIssuePath('AgentWorkforce', 'pear', 70) + const issue = githubIssueFile(70, { labels: ['factory'], author: 'reporter' }) + const mount = new FailingGithubCommentReconciliationMountClient({ [path]: issue }) + const githubWriteback = new AmbiguousMirroringGithubQuestionWriteback(mount, 70) + const factoryConfig = config({ issueSource: 'github', slack: slackConfig() }) + const clock = new ManualClock() + const firstFleet = new FakeFleetClient() + firstFleet.setSessionRef('ar-70-impl-pear', 'session-ar-70-impl-pear') + firstFleet.setSessionRef('ar-70-review-pear', 'session-ar-70-review-pear') + const firstState = new RecordingQuestionDeliveryRenewalFileStateStore({ batchSize: 2, watchStatePath }) + const first = createFactory(factoryConfig, { + mount, + fleet: firstFleet, + stateStore: firstState, + triage: new StaticTriage(), + githubWriteback, + clock, + }) + + await first.dispatch(await first.triageIssue(parseGithubFactoryIssue(path, issue))) + mount.files.set(path, { content: githubIssueFile(70, { labels: ['factory', 'factory:in-progress'], author: 'reporter' }) }) + mount.failNextReply = true + firstFleet.emitAgentMessage({ + from: 'ar-70-impl-pear', + target: 'factory', + body: '[factory-needs-input] Persist this fallback across restart.', + eventId: 'github-write-failure-fallback-question-70', + }) + await vi.waitFor(() => expect(first.status().counters.agentQuestionGithubPostsAmbiguous).toBe(1)) + expect(factoryGithubQuestionComments(githubWriteback)).toHaveLength(1) + const replyPrefix = githubReplyPrefixFromComment(factoryGithubQuestionComments(githubWriteback)[0]?.body) + let stranded = (await firstState.listWaitingClarifications('factory-test'))[0]?.[1] + expect(stranded?.questionPostedAtMs).toBeUndefined() + expect(stranded?.questionDelivery).toMatchObject({ attempts: 1, owner: expect.any(String) }) + expect(stranded?.questionDelivery?.owner).not.toBe('') + expect(firstState.questionDeliveryRenewals).toBeGreaterThanOrEqual(2) + expect((await firstState.listGithubIssueCommentWatches('factory-test'))[0]?.[1].pending).toHaveLength(1) + + // The transport error is ambiguous: GitHub accepted the marker and the + // reporter can answer while durable completion is still stranded. The + // original pending watch must remain able to claim that answer. + emitGithubIssueComment(mount, 'AgentWorkforce', 'pear', 70, 9902, { + body: `${replyPrefix} Preserve the reply through ambiguous recovery.`, + author: { login: 'reporter' }, + }) + await vi.waitFor(async () => { + stranded = (await firstState.listWaitingClarifications('factory-test'))[0]?.[1] + expect(stranded?.reply).toMatchObject({ id: 'github:AgentWorkforce/pear#70:9902', source: 'github' }) + }) + const slackQuestionWritesBeforeRestart = slackReplyWrites(mount).length + await first.stop() + + // After the ambiguous owner's lease expires, an unavailable comment + // mirror must defer rather than treating the marker as absent and + // posting again. It also retains the watch, owner, and claimed reply. + clock.advance(3 * 60_000) + mount.failGithubCommentReconciliation = 'list' + const deferredFleet = new FakeFleetClient() + const deferredState = new FileStateStore({ batchSize: 2, watchStatePath }) + const deferred = createFactory(factoryConfig, { + mount, + fleet: deferredFleet, + stateStore: deferredState, + triage: new StaticTriage(), + githubWriteback, + clock, + }) + await deferred.start({ mode: 'live', liveSubscription: { transport: 'subscribe' } }) + await vi.waitFor(() => expect(deferred.status().counters.agentQuestionGithubReconciliationsDeferred).toBe(1)) + const deferredWaiting = (await deferredState.listWaitingClarifications('factory-test'))[0]?.[1] + expect(deferredWaiting?.questionPostedAtMs).toBeUndefined() + expect(deferredWaiting).toMatchObject({ + questionDelivery: { attempts: 2, owner: expect.any(String) }, + reply: { id: 'github:AgentWorkforce/pear#70:9902', source: 'github' }, + }) + expect(deferredWaiting?.questionDelivery?.owner).not.toBe('') + expect(factoryGithubQuestionComments(githubWriteback)).toHaveLength(1) + await deferred.stop() + + // A subsequent owner also fails closed when the comment paths list but + // the marker-bearing comment itself cannot be read. + clock.advance(3 * 60_000) + mount.failGithubCommentReconciliation = 'read' + const readDeferredState = new FileStateStore({ batchSize: 2, watchStatePath }) + const readDeferred = createFactory(factoryConfig, { + mount, + fleet: new FakeFleetClient(), + stateStore: readDeferredState, + triage: new StaticTriage(), + githubWriteback, + clock, + }) + await readDeferred.start({ mode: 'live', liveSubscription: { transport: 'subscribe' } }) + await vi.waitFor(() => expect(readDeferred.status().counters.agentQuestionGithubReconciliationsDeferred).toBe(1)) + const readDeferredWaiting = (await readDeferredState.listWaitingClarifications('factory-test'))[0]?.[1] + expect(readDeferredWaiting?.questionPostedAtMs).toBeUndefined() + expect(readDeferredWaiting?.questionDelivery).toMatchObject({ attempts: 3, owner: expect.any(String) }) + expect(readDeferredWaiting?.reply).toMatchObject({ source: 'github' }) + expect(factoryGithubQuestionComments(githubWriteback)).toHaveLength(1) + await readDeferred.stop() + + // Losing the source issue read must also defer before either provider is + // retried; the durable GitHub reply itself proves an external delivery + // may already exist even though its reply watch has been consumed. + clock.advance(3 * 60_000) + mount.failGithubCommentReconciliation = 'issue' + const issueDeferredState = new FileStateStore({ batchSize: 2, watchStatePath }) + const issueDeferred = createFactory(factoryConfig, { + mount, + fleet: new FakeFleetClient(), + stateStore: issueDeferredState, + triage: new StaticTriage(), + githubWriteback, + clock, + }) + await issueDeferred.start({ mode: 'live', liveSubscription: { transport: 'subscribe' } }) + await vi.waitFor(() => expect(issueDeferred.status().counters.agentQuestionGithubReconciliationsDeferred).toBe(1)) + const issueDeferredWaiting = (await issueDeferredState.listWaitingClarifications('factory-test'))[0]?.[1] + expect(issueDeferredWaiting?.questionPostedAtMs).toBeUndefined() + expect(issueDeferredWaiting?.questionDelivery).toMatchObject({ attempts: 4, owner: expect.any(String) }) + expect(issueDeferredWaiting?.reply).toMatchObject({ source: 'github' }) + expect(factoryGithubQuestionComments(githubWriteback)).toHaveLength(1) + expect(slackReplyWrites(mount)).toHaveLength(slackQuestionWritesBeforeRestart) + await issueDeferred.stop() + + // Once authoritative reconciliation recovers and the retained lease + // expires, race two owners through cold-start recovery. Exactly one can + // commit the existing marker and drain the already-claimed reply. + clock.advance(3 * 60_000) + mount.failGithubCommentReconciliation = false + const restartedFleetA = new FakeFleetClient() + const restartedFleetB = new FakeFleetClient() + const restartedStateA = new RecordingQuestionDeliveryRenewalFileStateStore({ batchSize: 2, watchStatePath }) + const restartedStateB = new RecordingQuestionDeliveryRenewalFileStateStore({ batchSize: 2, watchStatePath }) + const restartedA = createFactory(factoryConfig, { + mount, + fleet: restartedFleetA, + stateStore: restartedStateA, + triage: new StaticTriage(), + githubWriteback, + clock, + }) + const restartedB = createFactory(factoryConfig, { + mount, + fleet: restartedFleetB, + stateStore: restartedStateB, + triage: new StaticTriage(), + githubWriteback, + clock, + }) + await Promise.all([ + restartedA.start({ mode: 'live', liveSubscription: { transport: 'subscribe' } }), + restartedB.start({ mode: 'live', liveSubscription: { transport: 'subscribe' } }), + ]) + await vi.waitFor(() => expect([ + restartedStateA.completedQuestionPostedAtMs, + restartedStateB.completedQuestionPostedAtMs, + ].filter((value) => typeof value === 'number')).toHaveLength(1)) + expect(factoryGithubQuestionComments(githubWriteback)).toHaveLength(1) + expect(slackReplyWrites(mount)).toHaveLength(slackQuestionWritesBeforeRestart) + expect( + (restartedA.status().counters.agentQuestionGithubFallbacksReconciled ?? 0) + + (restartedB.status().counters.agentQuestionGithubFallbacksReconciled ?? 0), + ).toBe(1) + + await vi.waitFor(() => expect( + (restartedA.status().counters.clarificationTeamsWoken ?? 0) + + (restartedB.status().counters.clarificationTeamsWoken ?? 0), + ).toBe(1)) + expect(factoryGithubQuestionComments(githubWriteback)).toHaveLength(1) + expect([...restartedFleetA.resumes, ...restartedFleetB.resumes].map((resume) => resume.sessionRef).sort()).toEqual([ + 'session-ar-70-impl-pear', + 'session-ar-70-review-pear', + ].sort()) + expect([...slackAnswerInputs(restartedFleetA), ...slackAnswerInputs(restartedFleetB)]).toHaveLength(2) + expect([...slackAnswerInputs(restartedFleetA), ...slackAnswerInputs(restartedFleetB)].map((input) => input.data)).toEqual([ + '\nHuman reply from @reporter on the GitHub issue:\nPreserve the reply through ambiguous recovery.\n\r', + '\nHuman reply from @reporter on the GitHub issue:\nPreserve the reply through ambiguous recovery.\n\r', + ]) + expect(await restartedStateA.listWaitingClarifications('factory-test')).toEqual([]) + await Promise.all([restartedA.stop(), restartedB.stop()]) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('does not lose a human reply that arrives while the question post is completing', async () => { const mount = new HumanReplyDuringQuestionMountClient({ [issuePath(39)]: issueFile(39) }) const fleet = new FakeFleetClient() @@ -10704,10 +11280,12 @@ describe('FactoryLoop', () => { const issue = githubIssueFile(60, { labels: ['factory'] }) const mount = new FakeMountClient({ [path]: issue }) const fleet = new FakeFleetClient() + const stateStore = new InMemoryStateStore({ batchSize: 2 }) const errors: unknown[][] = [] const factory = createFactory(config({ issueSource: 'github' }), { mount, fleet, + stateStore, triage: new StaticTriage(), githubWriteback: new FailingGithubCommentWriteback(), logger: { error: (...args: unknown[]) => errors.push(args) }, @@ -10728,11 +11306,12 @@ describe('FactoryLoop', () => { expect.objectContaining({ kind: 'agent-question', correlationId: expect.stringMatching(/^factory-agent-question-/u), - reason: 'GitHub issue comment writeback failed', + reason: 'GitHub issue comment writeback returned an ambiguous result; the pending reply watch was retained for reconciliation', }), ]) expect(factory.status().counters.escalationDeliveryFailures).toBe(1) expect(factory.status().counters.errors).toBe(1) + expect((await stateStore.listGithubIssueCommentWatches('factory-test'))[0]?.[1].pending).toHaveLength(1) }) it('ignores marked agent questions that are not addressed to the factory', async () => { @@ -10793,30 +11372,53 @@ describe('FactoryLoop', () => { expect(factory.status().counters.escalationDeliveryFailures).toBe(1) }) - it('treats agent questions as no-ops while Slack sync is degraded without regressing dispatch', async () => { + it('keeps ask instructions and surfaces an observable failure when degraded Slack has no fallback', async () => { const mount = new SlackSyncStatusMount({ [issuePath(39)]: issueFile(39) }) mount.slackStatus = { provider: 'slack', status: 'stale' } const fleet = new FakeFleetClient() + const errors: unknown[][] = [] + const emittedErrors: FactoryEventPayload[] = [] const factory = createFactory(config({ slack: slackConfig() }), { mount, fleet, triage: new StaticTriage(), + logger: { error: (...args: unknown[]) => errors.push(args) }, }) + factory.on('error', (payload) => emittedErrors.push(payload)) const result = await factory.dispatch(await factory.triageIssue(parseLinearIssue(issuePath(39), issueFile(39)))) + expect(fleet.messages.find((message) => message.to === 'ar-39-impl-pear')?.text).toContain( + 'DM `factory` with `[factory-needs-input]`', + ) + expect(fleet.messages.find((message) => message.to === 'ar-39-impl-pear')?.text).toContain( + 'operator-visible delivery error', + ) fleet.emitAgentMessage({ from: 'ar-39-impl-pear', target: 'factory', body: '[factory-needs-input] Can anyone clarify the expected retry behavior?', eventId: 'agent-question-39', }) - await flush() - await flush() + await vi.waitFor(() => expect(factory.status().counters.escalationDeliveryFailures).toBe(1)) expect(result.agents.map((agent) => agent.name)).toEqual(['ar-39-impl-pear', 'ar-39-review']) expect(fleet.spawns.map((spawn) => spawn.name)).toEqual(['ar-39-impl-pear', 'ar-39-review']) expect(mount.writes.filter((write) => isSlackRootWritePath(write.path) || write.path.includes('/replies/'))).toEqual([]) expect(factory.status().counters.agentQuestionsSkippedSlackDegraded).toBe(1) + expect(errors).toContainEqual([ + '[factory] escalation delivery failed', + expect.objectContaining({ + kind: 'agent-question', + correlationId: expect.stringMatching(/^factory-agent-question-/u), + reason: 'Slack writeback is degraded: slack sync status is stale; no GitHub issue write path with an identifiable issue reporter is available', + }), + ]) + expect(emittedErrors).toEqual([ + expect.objectContaining({ + issue: expect.objectContaining({ key: 'AR-39' }), + errorMessage: expect.stringContaining('Slack writeback is degraded: slack sync status is stale'), + }), + ]) }) it('dedupes duplicate agent question events before posting to Slack', async () => { diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index c5a25a1..8c07b34 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -153,6 +153,11 @@ type GithubIssueSource = { raw: Record } class ClarificationWakeLeaseLostError extends Error {} +class ClarificationQuestionDeliveryLeaseLostError extends Error {} +class GithubEscalationReconciliationUnavailableError extends Error {} +class GithubEscalationPostAmbiguousError extends Error {} + +type GithubEscalationReconciliation = 'found' | 'absent' | 'unavailable' class ClarificationWakeStoppedError extends Error {} type SlackSyncStatusSeverity = 'soft' | 'hard' type SlackSyncStatusCheck = { known: boolean; degraded: boolean; reason?: string; severity?: SlackSyncStatusSeverity } @@ -312,6 +317,7 @@ export class FactoryLoop implements Factory { #slackWritebackFailureBackoffUntilMs = 0 #slackEventWatermarkCache?: { checkedAtMs: number; result: SlackEventWatermark } #slackEventWatermarkRefresh?: Promise + #lastObservedSlackEventAtMs?: number #subscription?: Subscription #livePollTimer?: ReturnType #livePollInFlight = false @@ -3969,13 +3975,16 @@ export class FactoryLoop implements Factory { async #postAgentQuestion(record: InFlightIssue, question: AgentQuestion): Promise { if (!this.#slack || !this.#config.slack) { - await this.#postAgentQuestionToGithub(record, question) - return false + return await this.#postAgentQuestionToGithub(record, question) } if (await this.#shouldSkipSlackWriteback('agent-question')) { this.#increment('agentQuestionsSkippedSlackDegraded') - return false + return await this.#postAgentQuestionToGithub( + record, + question, + `Slack writeback is degraded${this.#slackDegradedReason ? `: ${this.#slackDegradedReason}` : ''}`, + ) } const key = issueKey(record.issue) @@ -3992,7 +4001,7 @@ export class FactoryLoop implements Factory { issue: record.issue, from: question.agentName, }) - return false + return await this.#postAgentQuestionToGithub(record, question, 'no Slack dispatch thread exists') } try { @@ -4006,7 +4015,7 @@ export class FactoryLoop implements Factory { } catch (error) { this.#markSlackWritebackFailure('agent-question', error) this.#logger.warn?.(`[factory] failed to post agent question for ${record.issue.key}`, error) - return false + return await this.#postAgentQuestionToGithub(record, question, 'Slack question writeback failed') } } @@ -4044,16 +4053,105 @@ export class FactoryLoop implements Factory { return false } - if (this.#stopping || await this.#shouldSkipSlackWriteback('agent-question')) { + if (this.#stopping) { await this.#state.releaseClarificationQuestionDelivery( this.#workspaceId, key, this.#clarificationWakeOwner, ) - if (!this.#stopping) this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) return false } + // A previous owner may have crashed after GitHub accepted the fallback + // but before questionPostedAtMs was committed. Reconcile that external + // fact before choosing a currently healthy Slack route, otherwise restart + // could duplicate the same durable question across providers. + const claimedIssue = await this.#readIssue(claimed.issue.path) + const claimedSource = claimedIssue ? githubIssueSourceRef(claimedIssue) : undefined + const claimedCorrelationId = githubEscalationCorrelationId( + 'agent-question', + claimed.issue, + claimed.question, + ) + let githubDeliveryMayHaveStarted: boolean + try { + githubDeliveryMayHaveStarted = claimed.reply?.source === 'github' || + await this.#githubIssueCommentPending(claimedCorrelationId) + } catch (error) { + this.#increment('agentQuestionGithubReconciliationsDeferred') + this.#surfaceEscalationDeliveryFailure( + 'agent-question', + claimed.issue, + claimedCorrelationId, + 'GitHub reply-watch state is temporarily unreadable; the durable delivery lease was retained', + error, + ) + this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) + return false + } + if (githubDeliveryMayHaveStarted && (!claimedIssue || !claimedSource)) { + this.#increment('agentQuestionGithubReconciliationsDeferred') + this.#surfaceEscalationDeliveryFailure( + 'agent-question', + claimed.issue, + claimedCorrelationId, + 'GitHub source issue is temporarily unreadable; the durable delivery lease and reply state were retained', + ) + this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) + return false + } + if (githubDeliveryMayHaveStarted && claimedIssue && claimedSource) { + const reconciliation = await this.#reconcileGithubEscalationComment( + claimedIssue, + claimedSource, + claimedCorrelationId, + ) + if (reconciliation === 'unavailable') { + // A persisted pending watch means a prior owner may have crossed the + // external-write boundary. Keep its lease/watch and fail closed until + // an authoritative lookup can distinguish absence from success. + this.#increment('agentQuestionGithubReconciliationsDeferred') + this.#logger.warn?.('[factory] deferring clarification delivery until GitHub marker reconciliation is available', { + issue: claimed.issue.key, + correlationId: claimedCorrelationId, + }) + this.#surfaceEscalationDeliveryFailure( + 'agent-question', + claimed.issue, + claimedCorrelationId, + 'GitHub issue comment reconciliation is unavailable; the durable delivery lease and reply state were retained', + ) + this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) + return false + } + if (reconciliation === 'found') { + const completed = await this.#state.completeClarificationQuestionDelivery( + this.#workspaceId, + key, + this.#clarificationWakeOwner, + this.#clock.now(), + ) + if (!completed) { + this.#increment('clarificationQuestionDeliveryOwnershipLost') + this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) + return false + } + this.#increment('agentQuestionGithubFallbacksReconciled') + this.#increment('clarificationQuestionsDelivered') + this.#increment('clarificationQuestionsDeliveredViaGithub') + await this.#drainReadyClarificationWake() + return true + } + } + + if (await this.#shouldSkipSlackWriteback('agent-question')) { + return await this.#deliverClarificationQuestionToGithub( + key, + claimed, + `Slack writeback is degraded${this.#slackDegradedReason ? `: ${this.#slackDegradedReason}` : ''}`, + ) + } + try { await this.#slack.reply( claimed.threadId, @@ -4082,16 +4180,119 @@ export class FactoryLoop implements Factory { await this.#drainReadyClarificationWake() return true } catch (error) { + this.#markSlackWritebackFailure('agent-question', error) + this.#increment('clarificationQuestionDeliveryFailures') + this.#logger.warn?.(`[factory] failed to post agent question for ${claimed.issue.key}; trying GitHub fallback`, error) + return await this.#deliverClarificationQuestionToGithub(key, claimed, 'Slack question writeback failed') + } + } + + async #deliverClarificationQuestionToGithub( + key: string, + waiting: WaitingClarification, + fallbackReason: string, + ): Promise { + let leaseLost = false + let renewalInFlight = false + const renewLease = async (): Promise => { + if (leaseLost) { + throw new ClarificationQuestionDeliveryLeaseLostError('clarification question delivery lease lost') + } + const renewed = await this.#state.renewClarificationQuestionDelivery( + this.#workspaceId, + key, + this.#clarificationWakeOwner, + this.#clock.now(), + ) + if (!renewed) { + leaseLost = true + throw new ClarificationQuestionDeliveryLeaseLostError('clarification question delivery lease lost') + } + } + const heartbeat = setInterval(() => { + if (renewalInFlight || leaseLost) return + renewalInFlight = true + void renewLease() + .catch((error: unknown) => { + if (error instanceof ClarificationQuestionDeliveryLeaseLostError) { + leaseLost = true + return + } + this.#logger.warn?.('[factory] transient error renewing clarification question delivery lease; retrying', { + issue: waiting.issue.key, + error, + }) + }) + .finally(() => { renewalInFlight = false }) + }, Math.max(1_000, Math.floor(CLARIFICATION_QUESTION_DELIVERY_LEASE_MS / 3))) + heartbeat.unref?.() + + try { + await renewLease() + const posted = await this.#postAgentQuestionToGithub(waitingRecord(waiting), { + agentName: waiting.askerName, + question: waiting.question, + }, fallbackReason, renewLease) + if (!posted) { + await this.#state.releaseClarificationQuestionDelivery( + this.#workspaceId, + key, + this.#clarificationWakeOwner, + ) + this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) + return false + } + + // Fence completion against a lease handoff that happened while the + // external GitHub write was in flight. A successor can reconcile the + // deterministic marker and complete without posting again. + await renewLease() + const completed = await this.#state.completeClarificationQuestionDelivery( + this.#workspaceId, + key, + this.#clarificationWakeOwner, + this.#clock.now(), + ) + if (!completed) { + throw new ClarificationQuestionDeliveryLeaseLostError('clarification question delivery lease lost') + } + this.#increment('clarificationQuestionsDelivered') + this.#increment('clarificationQuestionsDeliveredViaGithub') + await this.#drainReadyClarificationWake() + return true + } catch (error) { + if (error instanceof ClarificationQuestionDeliveryLeaseLostError) { + this.#increment('clarificationQuestionDeliveryOwnershipLost') + this.#logger.warn?.('[factory] clarification question delivery ownership moved to another daemon', { + issue: waiting.issue.key, + }) + this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) + return false + } + if ( + error instanceof GithubEscalationReconciliationUnavailableError || + error instanceof GithubEscalationPostAmbiguousError + ) { + this.#increment(error instanceof GithubEscalationPostAmbiguousError + ? 'agentQuestionGithubPostsAmbiguous' + : 'agentQuestionGithubReconciliationsDeferred') + this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) + return false + } await this.#state.releaseClarificationQuestionDelivery( this.#workspaceId, key, this.#clarificationWakeOwner, ) - this.#markSlackWritebackFailure('agent-question', error) this.#increment('clarificationQuestionDeliveryFailures') - this.#logger.warn?.(`[factory] failed to post agent question for ${claimed.issue.key}; keeping it durable for retry`, error) + this.#logger.error?.('[factory] GitHub clarification fallback preparation failed; delivery remains durable for retry', { + issue: waiting.issue.key, + error, + }) this.#scheduleClarificationSweep(CLARIFICATION_QUESTION_DELIVERY_RETRY_MS) return false + } finally { + clearInterval(heartbeat) } } @@ -4233,7 +4434,12 @@ export class FactoryLoop implements Factory { } } - async #postAgentQuestionToGithub(record: InFlightIssue, question: AgentQuestion): Promise { + async #postAgentQuestionToGithub( + record: InFlightIssue, + question: AgentQuestion, + fallbackReason?: string, + ensureDeliveryLease?: () => Promise, + ): Promise { const correlationId = githubEscalationCorrelationId('agent-question', record.issue, question.question) const issue = await this.#readIssue(record.issue.path) const source = issue ? githubIssueSourceRef(issue) : undefined @@ -4243,9 +4449,11 @@ export class FactoryLoop implements Factory { 'agent-question', record.issue, correlationId, - 'no Slack channel or GitHub issue write path with an identifiable issue reporter is available', + fallbackReason + ? `${fallbackReason}; no GitHub issue write path with an identifiable issue reporter is available` + : 'no Slack channel or GitHub issue write path with an identifiable issue reporter is available', ) - return + return false } await this.#addGithubIssueCommentWatch(record.issue, source, { @@ -4253,26 +4461,125 @@ export class FactoryLoop implements Factory { kind: 'agent-question', authorizedAuthor, }) + const reconciliation = await this.#reconcileGithubEscalationComment(issue, source, correlationId) + if (reconciliation === 'unavailable') { + this.#surfaceEscalationDeliveryFailure( + 'agent-question', + record.issue, + correlationId, + 'GitHub issue comment reconciliation is unavailable; delivery was deferred to avoid a duplicate', + ) + if (ensureDeliveryLease) { + throw new GithubEscalationReconciliationUnavailableError('GitHub escalation reconciliation unavailable') + } + return false + } + if (reconciliation === 'found') { + this.#increment('agentQuestionGithubFallbacksReconciled') + return true + } + // Renew immediately before the irreversible external write. The caller + // also heartbeats long posts and fences durable completion afterward. + await ensureDeliveryLease?.() try { await this.#githubWriteback.postComment(issue, [ `${record.issue.key}: ${question.agentName} needs input.`, `Question: ${question.question}`, + ...(fallbackReason ? [`Slack fallback reason: ${fallbackReason}.`] : []), `Authorized responder: @${authorizedAuthor} (the issue reporter).`, `Reply with a comment that starts with \`${githubReplyPrefix(correlationId)}\`.`, '', githubEscalationMarker(correlationId), ].join('\n')) this.#increment('agentQuestionsPostedToGithub') + if (fallbackReason) this.#increment('agentQuestionsRoutedToGithubFallback') + return true } catch (error) { - await this.#removeGithubIssueCommentPending(source, correlationId) this.#surfaceEscalationDeliveryFailure( 'agent-question', record.issue, correlationId, - 'GitHub issue comment writeback failed', + 'GitHub issue comment writeback returned an ambiguous result; the pending reply watch was retained for reconciliation', error, ) + if (ensureDeliveryLease) { + throw new GithubEscalationPostAmbiguousError('GitHub escalation post outcome is ambiguous', { cause: error }) + } + return false + } + } + + async #reconcileGithubEscalationComment( + issue: LinearIssue, + source: GithubIssueSourceRef, + correlationId: string, + ): Promise { + const marker = githubEscalationMarker(correlationId) + if (this.#githubWriteback.hasCommentMarker) { + try { + return await this.#githubWriteback.hasCommentMarker(issue, marker) ? 'found' : 'absent' + } catch (error) { + this.#logger.warn?.('[factory] authoritative GitHub escalation marker lookup failed', { + issue: source.number, + correlationId, + error, + }) + return 'unavailable' + } + } + + const paths = new Set() + const owner = encodeURIComponent(source.owner) + const repo = encodeURIComponent(source.repo) + for (const prefix of [ + `${GITHUB_ISSUE_ROOT}/${owner}/${repo}/issues`, + `${GITHUB_ISSUE_ROOT}/${owner}__${repo}/issues`, + ]) { + try { + for (const path of await this.#mount.listTree(prefix)) { + const parts = githubIssueCommentPathParts(path) + if ( + parts && + parts.owner.toLowerCase() === source.owner.toLowerCase() && + parts.repo.toLowerCase() === source.repo.toLowerCase() && + parts.number === source.number + ) { + paths.add(path) + } + } + } catch (error) { + this.#logger.warn?.('[factory] GitHub escalation marker listing failed', { prefix, correlationId, error }) + return 'unavailable' + } } + + let unreadable = false + for (const path of paths) { + try { + const { content } = await this.#mount.readFile(path) + const comment = parseGithubIssueComment(path, content) + if (comment?.body.includes(marker)) return 'found' + } catch (error) { + unreadable = true + this.#logger.warn?.('[factory] GitHub escalation marker comment read failed', { + path, + correlationId, + error, + }) + } + } + return unreadable ? 'unavailable' : 'absent' + } + + async #githubIssueCommentPending( + correlationId: string, + ): Promise { + if ([...this.#githubIssueCommentWatchStates.values()] + .some((watch) => watch.pending.some((pending) => pending.correlationId === correlationId))) { + return true + } + return (await this.#state.listGithubIssueCommentWatches(this.#workspaceId)) + .some(([, watch]) => watch.pending.some((pending) => pending.correlationId === correlationId)) } async #addGithubIssueCommentWatch( @@ -4588,6 +4895,26 @@ export class FactoryLoop implements Factory { return await this.#handleTriageEscalationGithubAnswer(escalationWatchRecord(pending.decision), text) } + const clarificationKey = issueKey(watch.issue) + const waiting = await this.#state.getWaitingClarification(this.#workspaceId, clarificationKey) + if (waiting) { + const claimed = await this.#state.claimClarificationReply(this.#workspaceId, clarificationKey, { + id: `github:${watch.source.owner}/${watch.source.repo}#${watch.source.number}:${comment.commentId}`, + text, + receivedAtMs: this.#clock.now(), + source: 'github', + author: comment.author, + }) + if (!claimed) { + this.#increment('clarificationDuplicateWakesSuppressed') + return Boolean(waiting.reply) + } + this.#increment('clarificationRepliesClaimed') + this.#increment('githubClarificationRepliesClaimed') + await this.#wakeWaitingClarification(clarificationKey, claimed) + return true + } + const liveRecord = (await this.#batch()).getIssue(watch.issue) if (!liveRecord || liveRecord.dryRun) { this.#increment('githubAnswersIgnoredNoInFlight') @@ -5988,10 +6315,12 @@ export class FactoryLoop implements Factory { async #slackFreshness(): Promise<{ known: boolean; degraded: boolean; reason?: string; status?: ProviderSyncStatus }> { const staleAfterMs = this.#config.slack?.staleAfterMs ?? 10 * 60_000 let sawSlackStatus = false + let slackStatus: ProviderSyncStatus | undefined let softStatusResult: SlackSyncStatusCheck | undefined let softStatus: ProviderSyncStatus | undefined try { const status = await this.#mount.getSyncStatus?.('slack') + slackStatus = status?.provider === 'slack' ? status : undefined sawSlackStatus = status?.provider === 'slack' const statusResult = slackSyncStatusResult(status, this.#clock.now(), staleAfterMs) if (statusResult.known) { @@ -6005,6 +6334,33 @@ export class FactoryLoop implements Factory { this.#logger.warn?.('[factory] Slack sync freshness check failed; proceeding without degradation', error) } + if (slackStatus?.webhookHealthy === true) { + if (softStatusResult?.degraded) { + this.#increment('slackGateBypassedByWebhookHealth') + this.#logger.info?.('[factory] Slack sync soft-degraded but webhook delivery is healthy; continuing Slack writeback', { + reason: softStatusResult.reason, + status: slackStatus, + }) + } + return { known: true, degraded: false } + } + + const observedEventAgeMs = this.#lastObservedSlackEventAtMs === undefined + ? undefined + : this.#clock.now() - this.#lastObservedSlackEventAtMs + if (observedEventAgeMs !== undefined && observedEventAgeMs <= staleAfterMs) { + if (softStatusResult?.degraded) { + this.#increment('slackGateBypassedByObservedEvent') + this.#logger.info?.('[factory] Slack sync soft-degraded but a webhook event arrived recently; continuing Slack writeback', { + reason: softStatusResult.reason, + status: softStatus, + lastObservedSlackEventAtMs: this.#lastObservedSlackEventAtMs, + observedEventAgeMs, + }) + } + return { known: true, degraded: false } + } + try { const watermark = await this.#slackEventWatermark() if (watermark.lastEventAtMs === undefined) { @@ -6082,6 +6438,13 @@ export class FactoryLoop implements Factory { return result } + #recordObservedSlackEvent(event: ChangeEvent): void { + const path = changeEventPath(event) + if (eventProvider(event) !== 'slack' && !path?.startsWith('/slack/')) return + this.#lastObservedSlackEventAtMs = this.#clock.now() + this.#increment('slackWebhookEventsObserved') + } + async #ensureSlackDispatchThread(record: InFlightIssue, result: DispatchResult): Promise { if (!this.#slack || !this.#config.slack || result.dryRun) { return @@ -6349,6 +6712,10 @@ export class FactoryLoop implements Factory { let subscription: Subscription | undefined try { subscription = this.#mount.subscribe([`${messagesPrefix}**`], (event) => { + // Receipt time is independent of the provider-authored sync timestamp. + // A healthy webhook can therefore override a frozen advisory status + // without trusting the same field that declared the provider stale. + this.#recordObservedSlackEvent(event) void handle(event) }) } catch (error) { @@ -6669,6 +7036,7 @@ export class FactoryLoop implements Factory { id: `${reply.threadTs}:${reply.messageTs}`, text, receivedAtMs: this.#clock.now(), + source: 'slack', }) if (!claimed) { this.#increment('clarificationDuplicateWakesSuppressed') @@ -6884,7 +7252,9 @@ export class FactoryLoop implements Factory { await renewLease() } - const event = slackReplyEvent(waiting.issue, reply.text) + const event = reply.source === 'github' + ? githubReplyEvent(waiting.issue, reply.text, reply.author) + : slackReplyEvent(waiting.issue, reply.text) for (const [name] of resumed) { if (waiting.wake?.injectedAgents.includes(name)) continue this.#assertClarificationWakeRunning() @@ -9335,6 +9705,14 @@ const escalationWatchRecord = (decision: TriageDecision): InFlightIssue => ({ dryRun: false, }) +const waitingRecord = (waiting: WaitingClarification): InFlightIssue => ({ + issue: waiting.issue, + decision: waiting.decision, + agents: new Map(waiting.agents.map(({ name, tracked }) => [name, structuredClone(tracked)])), + invocationIds: new Set(), + dryRun: waiting.dryRun, +}) + const cloneTrackedAgent = (tracked: TrackedAgent): TrackedAgent => ({ spec: { ...tracked.spec, diff --git a/src/ports/mount.ts b/src/ports/mount.ts index bb32012..cb52e00 100644 --- a/src/ports/mount.ts +++ b/src/ports/mount.ts @@ -26,6 +26,8 @@ export interface ProviderSyncStatus { lastEventAtMs?: number watermarkTs?: string | null lagSeconds?: number + /** Independent health of provider webhook delivery, when reported by the mount. */ + webhookHealthy?: boolean } export interface GithubPublishPullRequestInput { diff --git a/src/ports/state.ts b/src/ports/state.ts index 755eacf..5eb8fd1 100644 --- a/src/ports/state.ts +++ b/src/ports/state.ts @@ -8,6 +8,8 @@ export type ClarificationReply = { id: string text: string receivedAtMs: number + source?: 'slack' | 'github' + author?: string } export type WaitingClarificationAgent = { @@ -231,6 +233,7 @@ export interface StateStore { getWaitingClarification(workspaceId: string, issueKey: string): Promise listWaitingClarifications(workspaceId: string): Promise> claimClarificationQuestionDelivery(workspaceId: string, issueKey: string, owner: string, nowMs: number, leaseMs: number): Promise + renewClarificationQuestionDelivery(workspaceId: string, issueKey: string, owner: string, nowMs: number): Promise completeClarificationQuestionDelivery(workspaceId: string, issueKey: string, owner: string, postedAtMs: number): Promise releaseClarificationQuestionDelivery(workspaceId: string, issueKey: string, owner: string): Promise claimClarificationReply(workspaceId: string, issueKey: string, reply: ClarificationReply): Promise diff --git a/src/ports/writeback.ts b/src/ports/writeback.ts index f46529a..d59fd37 100644 --- a/src/ports/writeback.ts +++ b/src/ports/writeback.ts @@ -20,6 +20,8 @@ export type GithubIssueStatus = 'in-progress' | 'human-review' export interface GithubWriteback { 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 } diff --git a/src/state/file-state-store.test.ts b/src/state/file-state-store.test.ts index f30c282..243b3c5 100644 --- a/src/state/file-state-store.test.ts +++ b/src/state/file-state-store.test.ts @@ -424,17 +424,19 @@ describe('FileStateStore', () => { expect(owner).toMatch(/^factory-[ab]$/u) expect(await first.claimClarificationQuestionDelivery('workspace-1', key, owner!, 201, 60_000)) .toBeUndefined() + expect(await first.renewClarificationQuestionDelivery('workspace-1', key, 'wrong-owner', 201)).toBe(false) + expect(await second.renewClarificationQuestionDelivery('workspace-1', key, owner!, 202)).toBe(true) expect(await second.claimClarificationReply('workspace-1', key, { - id: 'fast-answer', text: 'Visible in Slack before confirmation.', receivedAtMs: 202, + id: 'fast-answer', text: 'Visible in Slack before confirmation.', receivedAtMs: 203, })).toMatchObject({ reply: { id: 'fast-answer' } }) - expect(await first.claimClarificationWake('workspace-1', key, 'too-early', 202, 60_000)) + expect(await first.claimClarificationWake('workspace-1', key, 'too-early', 203, 60_000)) .toBeUndefined() await second.releaseClarificationQuestionDelivery('workspace-1', key, owner!) - const retry = await first.claimClarificationQuestionDelivery('workspace-1', key, 'factory-retry', 203, 60_000) + const retry = await first.claimClarificationQuestionDelivery('workspace-1', key, 'factory-retry', 204, 60_000) expect(retry?.questionDelivery).toMatchObject({ owner: 'factory-retry', attempts: 2 }) - expect(await second.completeClarificationQuestionDelivery('workspace-1', key, 'factory-retry', 204)).toBe(true) - expect(await first.claimClarificationWake('workspace-1', key, 'factory-wake', 205, 60_000)) + expect(await second.completeClarificationQuestionDelivery('workspace-1', key, 'factory-retry', 205)).toBe(true) + expect(await first.claimClarificationWake('workspace-1', key, 'factory-wake', 206, 60_000)) .toMatchObject({ reply: { id: 'fast-answer' }, wake: { owner: 'factory-wake' } }) } finally { await rm(root, { recursive: true, force: true }) diff --git a/src/state/file-state-store.ts b/src/state/file-state-store.ts index 1fc1f85..7efb645 100644 --- a/src/state/file-state-store.ts +++ b/src/state/file-state-store.ts @@ -328,6 +328,24 @@ export class FileStateStore extends InMemoryStateStore { }) } + override async renewClarificationQuestionDelivery( + workspaceId: string, + issueKey: string, + owner: string, + nowMs: number, + ): Promise { + return await this.#exclusive(async () => { + return await this.#withMutationLock(async () => { + const document = await this.#loadFromDisk() + const delivery = document.workspaces[workspaceId]?.waitingClarifications[issueKey]?.questionDelivery + if (delivery?.owner !== owner) return false + delivery.claimedAtMs = nowMs + await this.#persist(document) + return true + }) + }) + } + override async releaseClarificationQuestionDelivery(workspaceId: string, issueKey: string, owner: string): Promise { await this.#mutateClarification(workspaceId, issueKey, (record) => { if (record.questionDelivery?.owner !== owner) return diff --git a/src/state/in-memory-state-store.ts b/src/state/in-memory-state-store.ts index 61082fa..e7bab58 100644 --- a/src/state/in-memory-state-store.ts +++ b/src/state/in-memory-state-store.ts @@ -290,6 +290,18 @@ export class InMemoryStateStore implements StateStore { return true } + async renewClarificationQuestionDelivery( + workspaceId: string, + issueKey: string, + owner: string, + nowMs: number, + ): Promise { + const delivery = this.#workspace(workspaceId).waitingClarifications.get(issueKey)?.questionDelivery + if (delivery?.owner !== owner) return false + delivery.claimedAtMs = nowMs + return true + } + async releaseClarificationQuestionDelivery(workspaceId: string, issueKey: string, owner: string): Promise { const delivery = this.#workspace(workspaceId).waitingClarifications.get(issueKey)?.questionDelivery if (delivery?.owner !== owner) return diff --git a/src/writeback/github.ts b/src/writeback/github.ts index 40aecf8..58a9920 100644 --- a/src/writeback/github.ts +++ b/src/writeback/github.ts @@ -84,6 +84,18 @@ export class GhCliGithubWriteback implements GithubWriteback { ]) } + async hasCommentMarker(issue: LinearIssue, marker: string): Promise { + const ref = githubIssueRef(issue) + const result = await this.#run([ + 'api', + '--paginate', + `repos/${ref.repo}/issues/${ref.number}/comments`, + '--jq', + '.[].body', + ]) + return result.stdout.includes(marker) + } + async setStatus(issue: LinearIssue, status: GithubIssueStatus): Promise { const ref = githubIssueRef(issue) const target = STATUS_LABELS[status] diff --git a/src/writeback/writeback.test.ts b/src/writeback/writeback.test.ts index 43ec7fc..db0d75f 100644 --- a/src/writeback/writeback.test.ts +++ b/src/writeback/writeback.test.ts @@ -825,6 +825,28 @@ describe('GhCliGithubWriteback', () => { ]) }) + it('looks up an escalation marker from all provider comment pages', async () => { + const calls: string[][] = [] + const github = new GhCliGithubWriteback({ + runner: async (args) => { + calls.push(args) + return { stdout: 'ordinary comment\n\n' } + }, + }) + + await expect(github.hasCommentMarker( + githubIssue, + '', + )).resolves.toBe(true) + expect(calls).toEqual([[ + 'api', + '--paginate', + 'repos/AgentWorkforce/factory/issues/48/comments', + '--jq', + '.[].body', + ]]) + }) + it('refuses writeback when GitHub source identity is incomplete', async () => { const github = new GhCliGithubWriteback({ runner: async () => ({ stdout: '' }) }) await expect(github.postComment({