From 9fa79b4e05a3b2d98a921fbee34349ad15831ac1 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 19 Jul 2026 13:14:14 +0200 Subject: [PATCH 1/8] fix: prevent broker from re-registering torn-down reviewers as orphans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The observed dispatch failure sequence — issue admitted, implementer worker_ready, reviewer agent_spawned but never ready, Factory tears down the workers, the broker retries an orphan, and the dashboard only shows dispatch_failed — has a concrete cause in defaultRestartPolicy. Factory owns durable resume/respawn for its dispatch agents and neutralizes the broker's default restart policy with { maxRestarts: 0 } so a broker-level retry can't re-register a name before Factory resumes it (relay#1116-family). That opt-out was applied to implementer and babysitter but NOT reviewer, even though routeReviewerSpec sets no restartPolicy and the reviewer shares the implementer's dispatch lifecycle (spawned in the same batch, torn down by the same dispatch-failure/#releaseAndTerminateAgents paths, resumed via the same #resumeDurableDispatch flow). So a torn-down reviewer fell through to the broker default, which re-registered its name as an orphan while the dashboard only reported dispatch_failed. Extend the maxRestarts:0 opt-out to the reviewer role. Add a regression test asserting both the implementer and reviewer dispatch spawns carry { maxRestarts: 0 } (the reviewer's restartPolicy was previously undefined). Co-Authored-By: Claude Opus 4.8 --- src/orchestrator/factory.test.ts | 30 ++++++++++++++++++++++++++++++ src/orchestrator/factory.ts | 8 +++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index b84b640..2bc3998 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -1699,6 +1699,36 @@ describe('FactoryLoop', () => { expect(mergeGate.merges).toEqual([]) }) + it('spawns the reviewer with maxRestarts:0 so a torn-down reviewer is not re-registered as a broker orphan', async () => { + // Regression: without an explicit restart policy the reviewer fell through + // to the broker's default, which re-registers a name on exit. When Factory + // tore the reviewer down (dispatch-failure teardown / release) the broker + // respawned it as an orphan while the dashboard only reported + // dispatch_failed. Every Factory-owned dispatch role must opt out of + // broker-level restarts because Factory owns durable resume/respawn. + const path = githubIssuePath('AgentWorkforce', 'pear', 49) + const mount = new FakeMountClient({ + [path]: githubIssueFile(49, { labels: ['factory'] }), + }) + const fleet = new FakeFleetClient() + const factory = createFactory(config({ issueSource: 'github' }), { + mount, + fleet, + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + }) + + await factory.runOnce() + + const bySuffix = (suffix: string) => fleet.spawns.find((spawn) => spawn.name.includes(suffix)) + expect(bySuffix('-impl-')?.name).toBe('ar-49-impl-pear') + expect(bySuffix('-review-')?.name).toBe('ar-49-review-pear') + // Both dispatch roles must carry the maxRestarts:0 opt-out; before the fix + // the reviewer's restartPolicy was undefined. + expect(bySuffix('-impl-')?.restartPolicy).toEqual({ maxRestarts: 0 }) + expect(bySuffix('-review-')?.restartPolicy).toEqual({ maxRestarts: 0 }) + }) + it('deduplicates compact and nested Relayfile aliases for the same GitHub issue', async () => { const byIdPath = githubIssueCompactPath('AgentWorkforce', 'pear', 47) const nestedPath = githubIssueNestedMetaPath('AgentWorkforce', 'pear', 47) diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 2cbcb68..b7b4261 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -11435,7 +11435,13 @@ const isAgentAlreadyExistsError = (error: unknown): boolean => { const defaultRestartPolicy = (spec: AgentSpec): AgentSpec['restartPolicy'] | undefined => // Factory owns durable resume/respawn decisions. Broker-level retries race // that lifecycle and can re-register the same name before Factory resumes it. - spec.role === 'implementer' || spec.role === 'babysitter' + // The reviewer shares the implementer's dispatch lifecycle: it is spawned in + // the same batch, torn down by the same dispatch-failure/teardown paths, and + // resumed through the same durable #resumeDurableDispatch flow. Without this + // opt-out the broker's default restart policy re-registers a torn-down + // reviewer's name as an orphan (relay#1116-family) while the dashboard only + // reports dispatch_failed — the exact orphan/restart sequence being audited. + spec.role === 'implementer' || spec.role === 'reviewer' || spec.role === 'babysitter' ? { maxRestarts: 0 } as AgentSpec['restartPolicy'] : spec.restartPolicy From d0f61b0721c8f4c8f33f81c0a9288a26a8153e2e Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 19 Jul 2026 13:15:04 +0200 Subject: [PATCH 2/8] fix: classify dispatch cancellation telemetry --- src/index.ts | 2 ++ src/observability/events.test.ts | 20 +++++++++++ src/observability/events.ts | 15 +++++++++ src/orchestrator/factory.test.ts | 58 ++++++++++++++++++++++++++++++-- src/orchestrator/factory.ts | 57 +++++++++++++++++++++++++++---- 5 files changed, 142 insertions(+), 10 deletions(-) diff --git a/src/index.ts b/src/index.ts index eeaa07b..976bef4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -257,6 +257,7 @@ export { FACTORY_CLOUD_EVENT_MAX_BATCH_SIZE, FACTORY_CLOUD_EVENT_MAX_PAYLOAD_BYTES, FACTORY_CLOUD_EVENT_TYPES, + FACTORY_CLOUD_CANCELLATION_REASONS_V1, FACTORY_CLOUD_RELEASE_REASONS_V1, FactoryCloudEventAttributesV1Schema, FactoryCloudEventBatchV1Schema, @@ -284,6 +285,7 @@ export type { } from './observability/cloud-reporter' export type { CreateFactoryCloudEventV1Options, + FactoryCloudCancellationReasonV1, FactoryCloudEventAttributesV1, FactoryCloudEventBatchV1, FactoryCloudEventInputV1, diff --git a/src/observability/events.test.ts b/src/observability/events.test.ts index 13d689e..4e08820 100644 --- a/src/observability/events.test.ts +++ b/src/observability/events.test.ts @@ -87,7 +87,9 @@ describe('Factory cloud event v1 contract', () => { it('maps raw fleet exit reasons to a closed privacy-safe category', () => { expect(factoryCloudReleaseReasonV1('node-offline')).toBe('node_offline') + expect(factoryCloudReleaseReasonV1('worker_exited')).toBe('exited') expect(factoryCloudReleaseReasonV1('max delivery retries exceeded')).toBe('delivery_failed') + expect(factoryCloudReleaseReasonV1('live dispatch state changed')).toBe('source_state_changed') expect(factoryCloudReleaseReasonV1('customer path /private/repo failed')).toBe('other') expect(factoryCloudReleaseReasonV1(undefined)).toBeUndefined() @@ -99,6 +101,24 @@ describe('Factory cloud event v1 contract', () => { })).toThrow() }) + it('accepts only closed cancellation reasons', () => { + const cancelled = createFactoryCloudEventV1({ + type: 'run.cancelled', + runId: 'run-cancelled', + status: 'cancelled', + attributes: { cancellationReason: 'agent_delivery_failed' }, + }) + expect(cancelled.attributes?.cancellationReason).toBe('agent_delivery_failed') + + expect(() => FactoryCloudEventInputV1Schema.parse({ + id: 'event-private-cancellation', + occurredAt: '2026-07-19T12:00:00.000Z', + type: 'run.cancelled', + runId: 'run-private-cancellation', + attributes: { cancellationReason: 'recipient unavailable: /private/customer/repo' }, + })).toThrow() + }) + it('rejects fields that could carry prompts, messages, paths, URLs, or raw errors', () => { const base = { id: 'event-private', diff --git a/src/observability/events.ts b/src/observability/events.ts index 57b1384..2bd93b5 100644 --- a/src/observability/events.ts +++ b/src/observability/events.ts @@ -19,11 +19,21 @@ export const FACTORY_CLOUD_RELEASE_REASONS_V1 = [ 'reconciled_missing', 'delivery_failed', 'dispatch_failed', + 'source_state_changed', 'other', ] as const export type FactoryCloudReleaseReasonV1 = (typeof FACTORY_CLOUD_RELEASE_REASONS_V1)[number] +export const FACTORY_CLOUD_CANCELLATION_REASONS_V1 = [ + 'source_state_changed', + 'agent_spawn_failed', + 'agent_delivery_failed', + 'dispatch_failed', +] as const + +export type FactoryCloudCancellationReasonV1 = (typeof FACTORY_CLOUD_CANCELLATION_REASONS_V1)[number] + export const FACTORY_CLOUD_EVENT_TYPES = [ 'instance.started', 'instance.heartbeat', @@ -98,6 +108,7 @@ export const FactoryCloudEventAttributesV1Schema = z.object({ nodeId: opaqueString.optional(), previousPhase: categoryString.optional(), releaseReason: z.enum(FACTORY_CLOUD_RELEASE_REASONS_V1).optional(), + cancellationReason: z.enum(FACTORY_CLOUD_CANCELLATION_REASONS_V1).optional(), dryRun: z.boolean().optional(), count: boundedCount.optional(), activeRuns: boundedCount.optional(), @@ -234,6 +245,7 @@ export function factoryCloudReleaseReasonV1(value: string | undefined): FactoryC case 'waiting-for-human': return 'waiting_for_human' case 'task_exit': + case 'worker_exited': case 'exited': return 'exited' case 'offline': @@ -252,6 +264,9 @@ export function factoryCloudReleaseReasonV1(value: string | undefined): FactoryC return 'delivery_failed' case 'dispatch failed': return 'dispatch_failed' + case 'live dispatch state changed': + case 'source state changed': + return 'source_state_changed' default: return 'other' } diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 2bc3998..2c18ac7 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -544,6 +544,15 @@ class SpawnFailingFleetClient extends FakeFleetClient { } } +class SpawnDeliveryFailingFleetClient extends FakeFleetClient { + override readonly durableOwnership = true + + override async spawn(input: SpawnInput): Promise { + this.spawns.push(input) + throw new Error(`recipient unavailable: ${input.name}`) + } +} + // Mimics the broker rejecting a resume because it never released the agent's // name on exit (relay#1116-family): http 500 "agent '' already exists". class ResumeNameCollisionFleetClient extends FakeFleetClient { @@ -638,6 +647,10 @@ class LocalLifecycleFleetClient extends FakeFleetClient { readonly durableOwnership = true } +class DurableSpawnFailingFleetClient extends SpawnFailingFleetClient { + override readonly durableOwnership = true +} + class TransientRemoteReleaseFleetClient extends RemoteLifecycleFleetClient { failReleaseFor?: string releaseFailures = 0 @@ -2068,11 +2081,13 @@ describe('FactoryLoop', () => { [racedPath]: racedReady, [freshPath]: githubIssueFile(60, { labels: ['factory', 'pear'] }), }) - const fleet = new FakeFleetClient() + const fleet = new LocalLifecycleFleetClient() const worktrees = new RecordingWorktreeManager() + const reporter = new RecordingFactoryEventReporter() const factory = createFactory(config({ issueSource: 'github', batchSize: 4 }), { mount, fleet, + reporter, triage: new StaticTriage(), githubWriteback: new RecordingGithubWriteback(), ...(withWorktrees ? { worktrees } : {}), @@ -2097,6 +2112,15 @@ describe('FactoryLoop', () => { ]) expect(factory.status().counters.dispatchLiveStateRaces).toBe(1) expect(factory.status().counters.errors ?? 0).toBe(0) + expect(reporter.events + .filter((event) => event.type === 'agent.released') + .map((event) => event.attributes?.releaseReason)) + .toEqual(['source_state_changed', 'source_state_changed']) + expect(reporter.events.find((event) => event.type === 'run.cancelled')).toMatchObject({ + phase: 'abandoned', + status: 'cancelled', + attributes: { cancellationReason: 'source_state_changed' }, + }) }, ) @@ -14257,13 +14281,15 @@ describe('FactoryLoop PR babysitter', () => { it('releases a planned agent and cleans its worktree when the spawn ack fails', async () => { const issue = realIssueFile(409, ready, { title: '[factory-e2e] Failed spawn worktree cleanup' }) const mount = new FakeMountClient({ [issuePath(409)]: issue }) - const fleet = new SpawnFailingFleetClient() + const fleet = new DurableSpawnFailingFleetClient() const worktrees = new RecordingWorktreeManager() + const reporter = new RecordingFactoryEventReporter() const cleanupReleaseCounts: number[] = [] worktrees.onCleanup = () => cleanupReleaseCounts.push(fleet.releases.length) - const factory = createFactory(config(), { + const factory = createFactory(config({ dispatch: { errorCooldownMs: 0, maxAttempts: 1 } }), { mount, fleet, + reporter, triage: new StaticTriage(), worktrees, }) @@ -14275,6 +14301,32 @@ describe('FactoryLoop PR babysitter', () => { expect(worktrees.cleaned).toHaveLength(1) expect(cleanupReleaseCounts).toEqual([1]) expect(factory.status().inFlight).toEqual([]) + expect(reporter.events.find((event) => event.type === 'run.cancelled')).toMatchObject({ + status: 'cancelled', + attributes: { cancellationReason: 'agent_spawn_failed' }, + }) + }) + + it('categorizes dispatch delivery failures without forwarding provider messages', async () => { + const issue = realIssueFile(411, ready, { title: '[factory-e2e] Failed spawn delivery' }) + const mount = new FakeMountClient({ [issuePath(411)]: issue }) + const fleet = new SpawnDeliveryFailingFleetClient() + const reporter = new RecordingFactoryEventReporter() + const factory = createFactory(config({ dispatch: { errorCooldownMs: 0, maxAttempts: 1 } }), { + mount, + fleet, + reporter, + triage: new StaticTriage(), + }) + + await expect(factory.dispatch(await factory.triageIssue(parseLinearIssue(issuePath(411), issue)))) + .rejects.toThrow('recipient unavailable') + + expect(reporter.events.find((event) => event.type === 'run.cancelled')).toMatchObject({ + status: 'cancelled', + attributes: { cancellationReason: 'agent_delivery_failed' }, + }) + expect(JSON.stringify(reporter.events)).not.toContain('recipient unavailable') }) it('retries transient local worktree cleanup without re-releasing completed agents', async () => { diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index b7b4261..5821473 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -84,6 +84,7 @@ import { readFactoryInFlightRegistry, terminatePids } from './reaper' import { createFactoryCloudEventV1, factoryCloudReleaseReasonV1, + type FactoryCloudCancellationReasonV1, type FactoryCloudEventInputV1, } from '../observability/events' @@ -2321,8 +2322,10 @@ export class FactoryLoop implements Factory { // acknowledged spawns, so cleanup never races a name-only survivor. const failureHandoffs = this.#dispatchFailureHandoffs(record, spawnedForReaperHandoff) await this.#persistDispatchFailureReaperHandoff(record, failureHandoffs) - let worktreesTornDown = await this.#teardownFailedDispatchWorktrees(failureHandoffs) const liveStateChanged = error instanceof LiveDispatchStateChangedError + const cancellationReason = factoryCloudDispatchCancellationReason(error) + const cleanupReason = liveStateChanged ? 'live dispatch state changed' : 'dispatch failed' + let worktreesTornDown = await this.#teardownFailedDispatchWorktrees(failureHandoffs, cleanupReason) if (liveStateChanged && !failureHandoffs.some((handoff) => handoff.worktree)) { const failed = await this.#releaseAndTerminateAgents( failureHandoffs.map((handoff) => [handoff.name, handoff.tracked]), @@ -2342,12 +2345,26 @@ export class FactoryLoop implements Factory { let failedState: { terminal: boolean } | undefined if (liveStateChanged) { await this.#clearDispatchInFlight(decision.issue) - await this.#saveDispatchLifecycle(record, 'abandoned') + await this.#saveDispatchLifecycle( + record, + 'abandoned', + undefined, + undefined, + new Set(), + { cancellationReason }, + ) this.#increment('dispatchLiveStateRaces') } else { await this.#recordDispatchFailure(decision.issue) failedState = await this.#state.getDispatchAttempts(this.#workspaceId, decision.issue.key) - await this.#saveDispatchLifecycle(record, failedState?.terminal ? 'abandoned' : 'retryable') + await this.#saveDispatchLifecycle( + record, + failedState?.terminal ? 'abandoned' : 'retryable', + undefined, + undefined, + new Set(), + { cancellationReason: failedState?.terminal ? cancellationReason : undefined }, + ) } batch.abandon(decision.issue) if (!liveStateChanged && !failedState?.terminal) this.#scheduleDispatchLifecycleRetry(record) @@ -2606,6 +2623,7 @@ export class FactoryLoop implements Factory { level?: FactoryCloudEventInputV1['level'] previousPhase?: DispatchLifecyclePhase errorCode?: string + cancellationReason?: FactoryCloudCancellationReasonV1 } = {}, ): Promise { await this.#report({ @@ -2626,6 +2644,7 @@ export class FactoryLoop implements Factory { operation: 'save_lifecycle', previousPhase: options.previousPhase, errorCode: options.errorCode, + cancellationReason: options.cancellationReason, dryRun: lifecycle.dryRun, trackedAgents: lifecycle.agents.length, }, @@ -2671,6 +2690,7 @@ export class FactoryLoop implements Factory { pullRequest?: GithubPublishPullRequestResult, releaseReason?: string, releasedAgentNames: ReadonlySet = new Set(), + telemetry: { cancellationReason?: FactoryCloudCancellationReasonV1 } = {}, ): Promise { if (record.dryRun || !this.#usesDurableDispatchLifecycle()) return true const key = issueKey(record.issue) @@ -2719,7 +2739,7 @@ export class FactoryLoop implements Factory { : lifecycle.phase === 'abandoned' ? 'run.cancelled' : 'run.phase_changed', - { previousPhase: previous?.phase }, + { previousPhase: previous?.phase, cancellationReason: telemetry.cancellationReason }, ) } if (isTerminalDispatchLifecycle(lifecycle)) { @@ -3917,11 +3937,14 @@ export class FactoryLoop implements Factory { return [...handoffs.values()] } - async #teardownFailedDispatchWorktrees(handoffs: RegistryHandoffAgent[]): Promise { + async #teardownFailedDispatchWorktrees( + handoffs: RegistryHandoffAgent[], + releaseReason = 'dispatch failed', + ): Promise { if (!this.#worktrees || !handoffs.some((handoff) => handoff.worktree)) return false const failed = await this.#releaseAndTerminateAgents( handoffs.map((handoff) => [handoff.name, handoff.tracked]), - 'dispatch failed', + releaseReason, 'completion', ) if (failed.length > 0) return false @@ -4083,10 +4106,15 @@ export class FactoryLoop implements Factory { channel: spec.channel, }) } catch (error) { - throw contextualError( + const wrapped = contextualError( `Dispatch spawn failed for ${record.issue.key}/${spec.name} (${spec.capability}) cwd=${spec.clonePath ?? 'default'}`, error, ) + throw Object.assign(wrapped, { + factoryCancellationReason: isDispatchDeliveryError(error) + ? 'agent_delivery_failed' as const + : 'agent_spawn_failed' as const, + }) } batch.recordSpawn(record, spec, invocationId, result) if (!await this.#saveDispatchLifecycle(record, 'dispatching')) { @@ -11514,6 +11542,21 @@ const isRegistrationLagInjectionError = (error: unknown): boolean => { .test(errorMessage) } +const isDispatchDeliveryError = (error: unknown): boolean => { + if (isRegistrationLagInjectionError(error)) return true + const { errorMessage } = describeError(error) + return /delivery[_ -]failed|dead-lettered|max delivery retries exceeded/iu.test(errorMessage) +} + +const factoryCloudDispatchCancellationReason = (error: unknown): FactoryCloudCancellationReasonV1 => { + if (error instanceof LiveDispatchStateChangedError) return 'source_state_changed' + if (error && typeof error === 'object' && 'factoryCancellationReason' in error) { + const reason = error.factoryCancellationReason + if (reason === 'agent_spawn_failed' || reason === 'agent_delivery_failed') return reason + } + return 'dispatch_failed' +} + const telemetryRunStatus = ( phase: DispatchLifecyclePhase, ): NonNullable => { From 8f675b4499201e5a25d42cc622b2532f71d4f748 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 19 Jul 2026 13:24:25 +0200 Subject: [PATCH 3/8] feat: report factory instance names --- src/cli/fleet.ts | 4 ++- src/config/schema.test.ts | 18 +++++++++++ src/config/schema.ts | 2 ++ src/observability/events.test.ts | 34 ++++++++++++++++++++- src/observability/events.ts | 1 + src/observability/instance-identity.test.ts | 28 ++++++++++++++++- src/observability/instance-identity.ts | 24 +++++++++++++++ 7 files changed, 108 insertions(+), 3 deletions(-) diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index d7c22fe..2f3f398 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -53,7 +53,7 @@ import { } from '../index' import { FakeFleetClient, FakeMountClient } from '../testing' import { GitAgentWorktreeManager } from '../git/agent-worktree' -import { loadOrCreateFactoryInstanceId } from '../observability/instance-identity' +import { loadOrCreateFactoryInstanceId, resolveFactoryInstanceName } from '../observability/instance-identity' interface FleetCliDeps { fleet?: FleetClient @@ -890,6 +890,7 @@ async function buildFactoryCloudReporter(input: { const outboxPath = input.config.reporting.outboxPath ?? join(dirname(input.config.loop.registryPath), 'factory-cloud-events.json') const instanceId = await loadOrCreateFactoryInstanceId(`${outboxPath}.instance-id`) + const instanceName = resolveFactoryInstanceName(input.config) const cloudFetch: typeof fetch = async (_request, init) => session.client.fetch('/api/v1/factory/events', init) return new FactoryCloudReporter({ @@ -899,6 +900,7 @@ async function buildFactoryCloudReporter(input: { bootId: randomUUID(), version: await readFactoryVersion(), metadata: { + ...(instanceName !== undefined ? { name: instanceName } : {}), backend: input.backend, mode: input.mode, runtime: 'node', diff --git a/src/config/schema.test.ts b/src/config/schema.test.ts index 75d8353..a47df44 100644 --- a/src/config/schema.test.ts +++ b/src/config/schema.test.ts @@ -98,6 +98,23 @@ describe('FactoryConfigSchema', () => { }) }) + it('trims and validates an explicit reporting instance name', () => { + const parsed = FactoryConfigSchema.parse({ + repos: {}, + reporting: { instanceName: ' Oslo Factory ' }, + }) + + expect(parsed.reporting.instanceName).toBe('Oslo Factory') + expect(() => FactoryConfigSchema.parse({ + repos: {}, + reporting: { instanceName: ' ' }, + })).toThrow() + expect(() => FactoryConfigSchema.parse({ + repos: {}, + reporting: { instanceName: 'x'.repeat(257) }, + })).toThrow() + }) + it('honors an explicit per-role agent CLI override and rejects unwired capabilities', () => { const parsed = FactoryConfigSchema.parse({ workspaceId: 'ws_123', @@ -201,6 +218,7 @@ describe('FactoryConfigSchema', () => { expect(parsed.subscription.labels).toEqual(['pear', 'cloud', 'agentswarm']) expect(parsed.repos.default).toBe('pear') expect(parsed.repos.org).toBe('AgentWorkforce') + expect(parsed.repos.names).toEqual(['pear', 'cloud', 'agentswarm']) }) it('lets explicit byLabel/clonePaths/labels override the derived ones', () => { diff --git a/src/config/schema.ts b/src/config/schema.ts index 6992066..6cefe69 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -122,6 +122,7 @@ const babysitterSchema = z.object({ const reportingSchema = z.object({ enabled: z.boolean().default(true), + instanceName: z.string().trim().min(1).max(256).optional(), outboxPath: z.string().min(1).optional(), batchSize: z.number().int().min(1).max(100).default(100), requestTimeoutMs: z.number().int().min(100).max(60_000).default(15_000), @@ -297,6 +298,7 @@ function normalizeFactoryConfig(cfg: z.infer) }, repos: { ...(cfg.repos.org !== undefined ? { org: cfg.repos.org } : {}), + ...(cfg.repos.names !== undefined ? { names: cfg.repos.names } : {}), byLabel: resolved.byLabel, byProject: resolved.byProject, keywordRules: resolved.keywordRules, diff --git a/src/observability/events.test.ts b/src/observability/events.test.ts index 4e08820..df93c81 100644 --- a/src/observability/events.test.ts +++ b/src/observability/events.test.ts @@ -138,7 +138,12 @@ describe('Factory cloud event v1 contract', () => { it('validates the exact authenticated ingestion batch shape', () => { const parsed = FactoryCloudEventBatchV1Schema.parse({ contract: 'factory.telemetry.v1', - instance: { id: 'instance-1', bootId: 'boot-1', version: '0.1.32' }, + instance: { + id: 'instance-1', + bootId: 'boot-1', + version: '0.1.32', + metadata: { name: ' hoopsheet ' }, + }, events: [{ id: 'event-1', sequence: 1, @@ -147,6 +152,33 @@ describe('Factory cloud event v1 contract', () => { }], }) expect(parsed.events).toHaveLength(1) + expect(parsed.instance.metadata?.name).toBe('hoopsheet') expect(parsed).not.toHaveProperty('workspaceId') }) + + it('keeps instance metadata.name optional and validates its bounded display value', () => { + const instance = { id: 'instance-1', bootId: 'boot-1', version: '0.1.32' } + const event = { + id: 'event-1', + sequence: 1, + occurredAt: '2026-07-19T12:00:00.000Z', + type: 'instance.heartbeat', + } + + expect(FactoryCloudEventBatchV1Schema.parse({ + contract: 'factory.telemetry.v1', + instance, + events: [event], + }).instance).toEqual(instance) + expect(() => FactoryCloudEventBatchV1Schema.parse({ + contract: 'factory.telemetry.v1', + instance: { ...instance, metadata: { name: ' ' } }, + events: [event], + })).toThrow() + expect(() => FactoryCloudEventBatchV1Schema.parse({ + contract: 'factory.telemetry.v1', + instance: { ...instance, metadata: { name: 'x'.repeat(257) } }, + events: [event], + })).toThrow() + }) }) diff --git a/src/observability/events.ts b/src/observability/events.ts index 2bd93b5..e0a453b 100644 --- a/src/observability/events.ts +++ b/src/observability/events.ts @@ -129,6 +129,7 @@ export const FactoryCloudInstanceV1Schema = z.object({ bootId: opaqueString, version: z.string().trim().min(1).max(64), metadata: z.object({ + name: opaqueString.optional(), backend: categoryString.optional(), mode: categoryString.optional(), platform: categoryString.optional(), diff --git a/src/observability/instance-identity.test.ts b/src/observability/instance-identity.test.ts index 9b89edf..266bbd3 100644 --- a/src/observability/instance-identity.test.ts +++ b/src/observability/instance-identity.test.ts @@ -4,7 +4,33 @@ import { join } from 'node:path' import { describe, expect, it } from 'vitest' -import { loadOrCreateFactoryInstanceId } from './instance-identity' +import { loadOrCreateFactoryInstanceId, resolveFactoryInstanceName } from './instance-identity' + +describe('resolveFactoryInstanceName', () => { + it('prefers an explicit reporting name over the sole configured repository', () => { + expect(resolveFactoryInstanceName({ + reporting: { instanceName: ' Production Factory ' }, + repos: { names: ['hoopsheet'] }, + })).toBe('Production Factory') + }) + + it('uses the trimmed sole repository name for HoopSheet', () => { + expect(resolveFactoryInstanceName({ + reporting: {}, + repos: { names: [' hoopsheet '] }, + })).toBe('hoopsheet') + }) + + it.each([ + { names: undefined, description: 'no configured repositories' }, + { names: [], description: 'an empty repository list' }, + { names: ['pear', 'hoopsheet'], description: 'multiple repositories' }, + { names: [' '], description: 'an empty trimmed repository name' }, + { names: ['x'.repeat(257)], description: 'an overlong repository name' }, + ])('omits the name for $description instead of deriving one from the hostname', ({ names }) => { + expect(resolveFactoryInstanceName({ reporting: {}, repos: { names } })).toBeUndefined() + }) +}) describe('loadOrCreateFactoryInstanceId', () => { it('persists one opaque identity across starts and concurrent creators', async () => { diff --git a/src/observability/instance-identity.ts b/src/observability/instance-identity.ts index 6d2af99..0391604 100644 --- a/src/observability/instance-identity.ts +++ b/src/observability/instance-identity.ts @@ -3,6 +3,25 @@ import { link, mkdir, open, readFile, unlink } from 'node:fs/promises' import { dirname } from 'node:path' const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/iu +const MAX_FACTORY_INSTANCE_NAME_LENGTH = 256 + +export interface FactoryInstanceNameConfig { + reporting: { instanceName?: string } + repos: { names?: readonly string[] } +} + +/** + * Resolve the operator-facing instance name without fingerprinting the host. + * An explicit reporting name wins; otherwise only an unambiguous, valid sole + * repository name is used. + */ +export function resolveFactoryInstanceName(config: FactoryInstanceNameConfig): string | undefined { + if (config.reporting.instanceName !== undefined) { + return normalizeFactoryInstanceName(config.reporting.instanceName) + } + if (config.repos.names?.length !== 1) return undefined + return normalizeFactoryInstanceName(config.repos.names[0]) +} /** * Return one opaque identity for this Factory installation. The create-only @@ -53,3 +72,8 @@ const isAlreadyExistsError = (error: unknown): boolean => const isMissingFileError = (error: unknown): boolean => Boolean(error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT') + +const normalizeFactoryInstanceName = (value: string): string | undefined => { + const name = value.trim() + return name.length >= 1 && name.length <= MAX_FACTORY_INSTANCE_NAME_LENGTH ? name : undefined +} From 77e04ca52e96775f292529469c00d4d1848f934c Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 19 Jul 2026 13:29:03 +0200 Subject: [PATCH 4/8] fix closed issue dispatch --- src/orchestrator/factory.test.ts | 174 +++++++++++++++++++++++++++++-- src/orchestrator/factory.ts | 147 ++++++++++++++++++++++++-- 2 files changed, 305 insertions(+), 16 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 2c18ac7..6de0f95 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -1538,6 +1538,15 @@ describe('FactoryLoop', () => { ...githubIssue, raw: { payload: { source: { provider: 'github', number: 2174 } } }, })).toBe(false) + for (const path of [ + githubIssueNestedMetaPath('AgentWorkforce', 'cloud', 2174), + githubIssuePath('AgentWorkforce', 'cloud', 2174), + ]) { + expect(isDispatchableIssue(parseGithubFactoryIssue(path, githubIssueFile(2174, { + repo: 'cloud', + state: 'closed', + })))).toBe(false) + } expect(isDispatchableIssue(parseLinearIssue(issuePath(2174), realIssueFile(2174, ready, { url: undefined })))).toBe(false) expect(isDispatchableIssue(parseLinearIssue(issuePath(2174), realIssueFile(2174)))).toBe(true) }) @@ -1760,13 +1769,58 @@ describe('FactoryLoop', () => { const report = await factory.runOnce() - expect(report.pulled).toEqual([{ uuid: 'AgentWorkforce/pear#47', key: '47', path: byIdPath }]) + expect(report.pulled).toEqual([{ uuid: 'AgentWorkforce/pear#47', key: '47', path: nestedPath }]) expect(report.triaged).toHaveLength(1) expect(report.dispatched).toHaveLength(1) expect(fleet.spawns.map((spawn) => spawn.name)).toEqual(['ar-47-impl-pear', 'ar-47-review-pear']) expect(factory.status().counters.githubIssueAliasPathsSuppressed).toBe(1) }) + it('trusts a closed canonical GitHub issue over a stale ready alias', async () => { + const canonicalPath = githubIssueNestedMetaPath('AgentWorkforce', 'pear', 46) + const aliasPath = githubIssueCompactPath('AgentWorkforce', 'pear', 46) + const mount = new FakeMountClient({ + [canonicalPath]: githubIssueFile(46, { state: 'closed', labels: ['factory'] }), + [aliasPath]: githubIssueFile(46, { state: 'open', labels: ['factory'] }), + }) + const fleet = new FakeFleetClient() + const factory = createFactory(config({ issueSource: 'github' }), { + mount, + fleet, + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + }) + + const report = await factory.runOnce() + + expect(report.pulled).toEqual([{ uuid: 'AgentWorkforce/pear#46', key: '46', path: canonicalPath }]) + expect(report.dispatched).toEqual([]) + expect(report.skipped).toEqual([{ + issue: { uuid: 'AgentWorkforce/pear#46', key: '46', path: canonicalPath }, + reason: 'live state is not ready-for-agent', + }]) + expect(fleet.spawns).toEqual([]) + }) + + it.each([ + ['canonical', githubIssueNestedMetaPath('AgentWorkforce', 'pear', 45)], + ['by-id alias', githubIssuePath('AgentWorkforce', 'pear', 45)], + ])('refuses explicit dispatch for a closed GitHub issue from its %s path before spawning', async (_kind, path) => { + const content = githubIssueFile(45, { state: 'closed', labels: ['factory'] }) + const mount = new FakeMountClient({ [path]: content }) + const fleet = new FakeFleetClient() + const factory = createFactory(config({ issueSource: 'github' }), { + mount, + fleet, + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + }) + const decision = await factory.triageIssue(parseGithubFactoryIssue(path, content)) + + await expect(factory.dispatch(decision)).rejects.toThrow(/Live state changed/) + expect(fleet.spawns).toEqual([]) + }) + it('isolates equal-number GitHub dispatch names, state, registry, resume, and completion across repos', async () => { const number = 26 const pearPath = githubIssuePath('AgentWorkforce', 'pear', number) @@ -2101,15 +2155,10 @@ describe('FactoryLoop', () => { }) expect(report.dispatched.map((result) => result.issue.key)).toEqual(['60']) expect(fleet.spawns.map((spawn) => spawn.name)).toEqual([ - 'ar-59-impl-pear', - 'ar-59-review-pear', 'ar-60-impl-pear', 'ar-60-review-pear', ]) - expect(fleet.releases.map((release) => release.name).sort()).toEqual([ - 'ar-59-impl-pear', - 'ar-59-review-pear', - ]) + expect(fleet.releases).toEqual([]) expect(factory.status().counters.dispatchLiveStateRaces).toBe(1) expect(factory.status().counters.errors ?? 0).toBe(0) expect(reporter.events @@ -4756,6 +4805,117 @@ describe('FactoryLoop', () => { } }) + it.each([ + ['canonical dispatching lifecycle', 586, false, 'dispatching'], + ['stale by-id alias retryable lifecycle', 587, true, 'retryable'], + ] as const)('abandons a durable GitHub dispatch when the %s resolves to a closed issue', async ( + _kind, + number, + staleAlias, + persistedPhase, + ) => { + class AckGapFleet extends RemoteLifecycleFleetClient { + failed = false + + override async spawn(input: SpawnInput): Promise { + const result = await super.spawn(input) + if (!this.failed) { + this.failed = true + throw new Error('owner crashed after remote spawn ack') + } + return result + } + } + + class CrashAfterLifecycleClaimStore extends FileStateStore { + crashed = false + + override async claimDispatchLifecycle(...args: Parameters) { + const claimed = await super.claimDispatchLifecycle(...args) + if (!this.crashed) { + this.crashed = true + throw new Error('owner crashed after durable lifecycle claim') + } + return claimed + } + } + + const root = await mkdtemp(join(tmpdir(), `factory-closed-durable-${number}-`)) + const watchStatePath = join(root, 'state.json') + const canonicalPath = githubIssueNestedMetaPath('AgentWorkforce', 'pear', number) + const aliasPath = githubIssuePath('AgentWorkforce', 'pear', number) + const dispatchPath = staleAlias ? aliasPath : canonicalPath + const openIssue = githubIssueFile(number, { state: 'open', labels: ['factory'] }) + const mount = new FakeMountClient({ + [dispatchPath]: openIssue, + ...(staleAlias ? { [canonicalPath]: openIssue } : {}), + }) + const fleet = persistedPhase === 'retryable' ? new AckGapFleet() : new RemoteLifecycleFleetClient() + const clock = new ManualClock() + const state = () => new FileStateStore({ batchSize: 2, watchStatePath }) + const first = createFactory(config({ issueSource: 'github' }), { + mount, + fleet, + stateStore: persistedPhase === 'dispatching' + ? new CrashAfterLifecycleClaimStore({ batchSize: 2, watchStatePath }) + : state(), + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + clock, + }) + let restarted: ReturnType | undefined + try { + const decision = await first.triageIssue(parseGithubFactoryIssue(dispatchPath, openIssue)) + await expect(first.dispatch(decision)).rejects.toThrow( + persistedPhase === 'dispatching' + ? 'owner crashed after durable lifecycle claim' + : 'owner crashed after remote spawn ack', + ) + await expect(state().getDispatchLifecycle('factory-test', issueKey(decision.issue))) + .resolves.toMatchObject({ + phase: persistedPhase, + agents: persistedPhase === 'retryable' + ? [expect.objectContaining({ name: `ar-${number}-impl-pear` })] + : [], + }) + await first.stop() + clock.advance(5 * 60_000 + 1) + + mount.files.set(canonicalPath, { + content: githubIssueFile(number, { state: 'closed', labels: ['factory'] }), + }) + restarted = createFactory(config({ issueSource: 'github' }), { + mount, + fleet, + stateStore: state(), + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + clock, + }) + await restarted.start({ mode: 'dispatch-owner' }) + + await vi.waitFor(async () => expect(await state().getDispatchLifecycle( + 'factory-test', + issueKey(decision.issue), + )).toMatchObject({ phase: 'abandoned' }), { timeout: 4_000 }) + await vi.waitFor(() => expect(restarted?.status().counters.dispatchLifecycleStaleIssuesAbandoned).toBe(1)) + expect(fleet.spawns.map((spawn) => spawn.name)).toEqual( + persistedPhase === 'retryable' ? [`ar-${number}-impl-pear`] : [], + ) + if (persistedPhase === 'retryable') { + expect(fleet.releases).toContainEqual({ + name: `ar-${number}-impl-pear`, + reason: 'live issue no longer ready', + }) + } + expect(restarted.status().inFlight).toEqual([]) + } finally { + await restarted?.stop() + await first.stop() + await rm(root, { recursive: true, force: true }) + } + }) + it('autonomously retries a transient remote PR publication without another exit event', async () => { const root = await mkdtemp(join(tmpdir(), 'factory-publish-retry-')) const watchStatePath = join(root, 'state.json') diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 5821473..263089e 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -311,6 +311,7 @@ export class FactoryLoop implements Factory { readonly #githubIssueCommentQueues = new Map>() readonly #githubIssueAuthors = new Map() readonly #githubIssueAuthorLookups = new Map>() + readonly #githubIssuePreferredPaths = new Map() readonly #slackReporterUserIds = new Map() readonly #slackReporterUserIdLookups = new Map>() readonly #reconciledGithubInProgress = new Set() @@ -2165,6 +2166,10 @@ export class FactoryLoop implements Factory { throw error } + if (!this.#isIssueReady(liveIssue)) { + throw new LiveDispatchStateChangedError(decision.issue.key) + } + if (!isDispatchableIssue(liveIssue)) { const error = new Error(`Refusing to dispatch ${decision.issue.key}: not reconciled real Linear issue`) this.#error(error, decision.issue) @@ -2262,6 +2267,12 @@ export class FactoryLoop implements Factory { const spawnedForReaperHandoff: RegistryHandoffAgent[] = [] try { + if (!dryRun) { + const issue = await this.#readIssue(dispatchDecision.issue.path) + if (!issue || !this.#isIssueReady(issue)) { + throw new LiveDispatchStateChangedError(dispatchDecision.issue.key) + } + } const specs = dispatchSpecs(dispatchDecision) const agents: DispatchResult['agents'] = [] for (const spec of specs) { @@ -2917,6 +2928,17 @@ export class FactoryLoop implements Factory { } async #resumeDurableDispatch(record: InFlightIssue): Promise { + if (!record.dryRun) { + const issue = await this.#readIssue(record.issue.path) + const resumable = Boolean(issue && ( + !isGithubIssue(issue) || + (isDispatchableIssue(issue) && this.#isGithubIssueResumable(issue)) + )) + if (!resumable) { + await this.#abandonDurableResume(record, 'live GitHub issue is closed or no longer ready-for-agent') + return + } + } const agents: DispatchResult['agents'] = [] const specs = dispatchSpecs(record.decision) const plannedNames = new Set(specs.map((spec) => spec.name)) @@ -2960,6 +2982,62 @@ export class FactoryLoop implements Factory { } } + #isGithubIssueResumable(issue: LinearIssue): boolean { + if (this.#isIssueReady(issue)) return true + if (githubFactoryIssueIsClosed(issue)) return false + const labels = new Set(issue.labels.map((label) => label.trim().toLowerCase())) + const required = this.#config.safety.requireLabel.trim().toLowerCase() + return Boolean(required) && + labels.has(required) && + labels.has('factory:in-progress') && + !labels.has('factory:human-review') + } + + async #abandonDurableResume(record: InFlightIssue, reason: string): Promise { + const handoffs = this.#dispatchFailureHandoffs(record, [...record.agents].map(([name, tracked]) => ({ + issue: record.issue, + name, + tracked: cloneTrackedAgent(tracked), + persistedAtMs: this.#clock.now(), + }))) + await this.#persistDispatchFailureReaperHandoff(record, handoffs) + if (!await this.#saveDispatchLifecycle(record, 'abandoned', undefined, reason)) return + + await this.#clearDispatchInFlight(record.issue) + const batch = await this.#batch() + batch.abandon(record.issue) + for (const [name] of record.agents) { + this.#fleet.markAgentTerminal?.(name, 'durable-dispatch-abandoned') + } + + if (handoffs.some((handoff) => handoff.worktree)) { + await this.#teardownFailedDispatchWorktrees(handoffs) + } else if (handoffs.length > 0) { + const failed = new Set(await this.#releaseAndTerminateAgents( + handoffs.map((handoff) => [handoff.name, handoff.tracked]), + 'live issue no longer ready', + 'completion', + )) + for (const handoff of handoffs) { + if (failed.has(handoff.name)) continue + await this.#state.clearFailureHandoff( + this.#workspaceId, + registryHandoffKey(handoff.issue, handoff.name), + ) + } + } + + await this.#stopSlackWatcher(record.issue) + await this.#stopGithubIssueCommentWatcherForIssue(record.issue) + await this.#writeInFlightRegistry() + this.#increment('dispatchLifecycleStaleIssuesAbandoned') + this.#resolveDispatchTerminalWaiters(record.issue) + this.#logger.info?.('[factory] abandoned durable dispatch whose live issue is no longer ready', { + issue: record.issue.key, + reason, + }) + } + async #finishDurableRelease(record: InFlightIssue, releaseReason?: string): Promise { const batch = await this.#batch() const reason = releaseReason ?? (this.#config.terminalState === 'human-review' ? 'issue-human-review' : 'issue-done') @@ -3140,8 +3218,7 @@ export class FactoryLoop implements Factory { if (!isGithubIssue(issue)) { return this.#states.isRole(issue.stateId, 'readyForAgent') } - const githubState = (issue.state?.name ?? stringValue(wrappedPayload(issue.raw).state) ?? '').trim().toLowerCase() - if (githubState === 'closed') { + if (githubFactoryIssueIsClosed(issue)) { return false } const labels = new Set(issue.labels.map((label) => label.trim().toLowerCase())) @@ -3280,6 +3357,9 @@ export class FactoryLoop implements Factory { } } } + for (const [identity, path] of issuePaths) { + this.#githubIssuePreferredPaths.set(identity, path) + } return [...issuePaths.values()].sort() } catch (error) { this.#increment('githubIssueListFailures') @@ -3361,7 +3441,11 @@ export class FactoryLoop implements Factory { } async #readGithubIssue(path: string): Promise { - const candidatePaths = githubIssueReadCandidatePaths(path) + const preferredPath = await this.#preferredGithubIssuePath(path) + const candidatePaths = [...new Set([ + ...githubIssueReadCandidatePaths(preferredPath), + ...githubIssueReadCandidatePaths(path), + ])] try { for (const candidatePath of candidatePaths) { try { @@ -3384,6 +3468,40 @@ export class FactoryLoop implements Factory { } } + async #preferredGithubIssuePath(path: string): Promise { + const parts = githubIssuePathParts(path) ?? githubIssueDirectoryPathParts(path) + if (!parts) return path + const identity = githubIssueIdentity(parts.owner, parts.repo, parts.number) + const cached = this.#githubIssuePreferredPaths.get(identity) + if (cached && githubIssuePathPreference(cached) <= githubIssuePathPreference(path)) return cached + if (githubIssuePathPreference(path) === 0) { + this.#githubIssuePreferredPaths.set(identity, path) + return path + } + + let preferred = cached ?? path + for (const root of githubIssueRepoRoots(parts.owner, parts.repo)) { + let paths: string[] + try { + paths = await this.#listRelayfileTree(root, 'GitHub canonical issue resolution') + } catch { + continue + } + for (const candidate of paths) { + const candidateParts = githubIssuePathParts(candidate) + if ( + !candidateParts || + githubIssueIdentity(candidateParts.owner, candidateParts.repo, candidateParts.number) !== identity + ) continue + if (githubIssuePathPreference(candidate) < githubIssuePathPreference(preferred)) { + preferred = candidate + } + } + } + this.#githubIssuePreferredPaths.set(identity, preferred) + return preferred + } + async #findGithubIssueMirror( ghIssue: GithubIssueSource, candidates?: MirrorCandidateCache, @@ -9627,6 +9745,9 @@ export function isDispatchableIssue(issue: LinearIssue): boolean { if (!isGithubIssue(issue)) { return false } + if (githubFactoryIssueIsClosed(issue)) { + return false + } const payload = wrappedPayload(issue.raw) const source = asRecord(payload.source) const sourceId = stringValue(source?.id) @@ -9643,6 +9764,9 @@ const isGithubIssue = (issue: LinearIssue): boolean => { (stringValue(wrapper?.provider)?.toLowerCase() === 'github' || isGithubIssueFilePath(issue.path)) } +const githubFactoryIssueIsClosed = (issue: LinearIssue): boolean => + (issue.state?.name ?? stringValue(wrappedPayload(issue.raw).state) ?? '').trim().toLowerCase() === 'closed' + const githubIssueSourceRef = (issue: LinearIssue): GithubIssueSourceRef | undefined => { const source = asRecord(wrappedPayload(issue.raw).source) if (stringValue(source?.provider)?.toLowerCase() !== 'github') { @@ -10182,10 +10306,11 @@ const githubIssueRefIdentity = (issue: IssueRef): string | undefined => { } const githubIssuePathPreference = (path: string): number => { - if (path.includes('/issues/by-id/')) return 0 - if (path.endsWith('/meta.json')) return 1 - if (path.endsWith('.json')) return 2 - return 3 + if (path.endsWith('/meta.json')) return 0 + if (path.endsWith('/metadata.json')) return 1 + if (path.includes('/issues/by-id/')) return 2 + if (path.endsWith('.json')) return 3 + return 4 } const githubAgentNameMatchesIssue = (name: string, issue: LinearIssue): boolean => { @@ -10566,12 +10691,16 @@ export const githubRepoSubscriptionGlobs = (config: FactoryConfig): string[] => const githubIssueScanRoots = (config: FactoryConfig): string[] => { const roots = new Set() for (const { owner, repo } of configuredGithubRepoParts(config)) { - roots.add(`${GITHUB_ISSUE_ROOT}/${owner}/${repo}/issues`) - roots.add(`${GITHUB_ISSUE_ROOT}/${owner}__${repo}/issues`) + for (const root of githubIssueRepoRoots(owner, repo)) roots.add(root) } return [...roots] } +const githubIssueRepoRoots = (owner: string, repo: string): string[] => [ + `${GITHUB_ISSUE_ROOT}/${owner}/${repo}/issues`, + `${GITHUB_ISSUE_ROOT}/${owner}__${repo}/issues`, +] + const githubRepoPathParts = (path: string): { owner: string; repo: string } | undefined => { const compactSegment = path.match(/^\/github\/repos\/([^/]+)\//u)?.[1] const separator = compactSegment?.indexOf('__') ?? -1 From ff4168d946c6d3ea88653a7089b5520d466b3b09 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 19 Jul 2026 13:33:05 +0200 Subject: [PATCH 5/8] fix: report stale dispatch abandonment --- src/orchestrator/factory.test.ts | 4 ++-- src/orchestrator/factory.ts | 13 ++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 6de0f95..3bcdf4f 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -2164,7 +2164,7 @@ describe('FactoryLoop', () => { expect(reporter.events .filter((event) => event.type === 'agent.released') .map((event) => event.attributes?.releaseReason)) - .toEqual(['source_state_changed', 'source_state_changed']) + .toEqual([]) expect(reporter.events.find((event) => event.type === 'run.cancelled')).toMatchObject({ phase: 'abandoned', status: 'cancelled', @@ -4905,7 +4905,7 @@ describe('FactoryLoop', () => { if (persistedPhase === 'retryable') { expect(fleet.releases).toContainEqual({ name: `ar-${number}-impl-pear`, - reason: 'live issue no longer ready', + reason: 'live dispatch state changed', }) } expect(restarted.status().inFlight).toEqual([]) diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 263089e..4cb272d 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -3001,7 +3001,14 @@ export class FactoryLoop implements Factory { persistedAtMs: this.#clock.now(), }))) await this.#persistDispatchFailureReaperHandoff(record, handoffs) - if (!await this.#saveDispatchLifecycle(record, 'abandoned', undefined, reason)) return + if (!await this.#saveDispatchLifecycle( + record, + 'abandoned', + undefined, + reason, + new Set(), + { cancellationReason: 'source_state_changed' }, + )) return await this.#clearDispatchInFlight(record.issue) const batch = await this.#batch() @@ -3011,11 +3018,11 @@ export class FactoryLoop implements Factory { } if (handoffs.some((handoff) => handoff.worktree)) { - await this.#teardownFailedDispatchWorktrees(handoffs) + await this.#teardownFailedDispatchWorktrees(handoffs, 'live dispatch state changed') } else if (handoffs.length > 0) { const failed = new Set(await this.#releaseAndTerminateAgents( handoffs.map((handoff) => [handoff.name, handoff.tracked]), - 'live issue no longer ready', + 'live dispatch state changed', 'completion', )) for (const handoff of handoffs) { From 1c057e0645a32831b601829d6a93efcf83dcff9d Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 19 Jul 2026 13:32:25 +0200 Subject: [PATCH 6/8] keep unreadable dispatches retryable --- src/orchestrator/factory.test.ts | 28 ++++++++++++++++++++++------ src/orchestrator/factory.ts | 9 ++++----- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 3bcdf4f..e5ac34f 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -4806,13 +4806,15 @@ describe('FactoryLoop', () => { }) it.each([ - ['canonical dispatching lifecycle', 586, false, 'dispatching'], - ['stale by-id alias retryable lifecycle', 587, true, 'retryable'], - ] as const)('abandons a durable GitHub dispatch when the %s resolves to a closed issue', async ( + ['closed canonical dispatching lifecycle', 586, false, 'dispatching', 'closed'], + ['closed canonical from a stale by-id alias retryable lifecycle', 587, true, 'retryable', 'closed'], + ['temporarily unreadable dispatching lifecycle', 588, false, 'dispatching', 'unreadable'], + ] as const)('revalidates a durable GitHub dispatch with a %s', async ( _kind, number, staleAlias, persistedPhase, + liveState, ) => { class AckGapFleet extends RemoteLifecycleFleetClient { failed = false @@ -4881,9 +4883,13 @@ describe('FactoryLoop', () => { await first.stop() clock.advance(5 * 60_000 + 1) - mount.files.set(canonicalPath, { - content: githubIssueFile(number, { state: 'closed', labels: ['factory'] }), - }) + if (liveState === 'closed') { + mount.files.set(canonicalPath, { + content: githubIssueFile(number, { state: 'closed', labels: ['factory'] }), + }) + } else { + mount.files.delete(dispatchPath) + } restarted = createFactory(config({ issueSource: 'github' }), { mount, fleet, @@ -4894,6 +4900,16 @@ describe('FactoryLoop', () => { }) await restarted.start({ mode: 'dispatch-owner' }) + if (liveState === 'unreadable') { + await new Promise((resolve) => setTimeout(resolve, 1_200)) + await expect(state().getDispatchLifecycle('factory-test', issueKey(decision.issue))) + .resolves.toMatchObject({ phase: persistedPhase }) + expect(fleet.spawns).toEqual([]) + expect(fleet.releases).toEqual([]) + expect(restarted.status().counters.dispatchLifecycleStaleIssuesAbandoned).toBeUndefined() + return + } + await vi.waitFor(async () => expect(await state().getDispatchLifecycle( 'factory-test', issueKey(decision.issue), diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 4cb272d..37a2db3 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -2930,11 +2930,10 @@ export class FactoryLoop implements Factory { async #resumeDurableDispatch(record: InFlightIssue): Promise { if (!record.dryRun) { const issue = await this.#readIssue(record.issue.path) - const resumable = Boolean(issue && ( - !isGithubIssue(issue) || - (isDispatchableIssue(issue) && this.#isGithubIssueResumable(issue)) - )) - if (!resumable) { + if (!issue) { + throw new Error(`Unable to recover durable dispatch ${record.issue.key}: issue is not currently readable`) + } + if (isGithubIssue(issue) && !this.#isGithubIssueResumable(issue)) { await this.#abandonDurableResume(record, 'live GitHub issue is closed or no longer ready-for-agent') return } From b45360820c4692ef7268c23e460fda0c750fef0a Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 19 Jul 2026 14:11:05 +0200 Subject: [PATCH 7/8] test: await GitHub clarification dispatch completion --- src/orchestrator/factory.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index e5ac34f..0dc2b02 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -11303,7 +11303,7 @@ describe('FactoryLoop', () => { expect(fleet.spawns.find((spawn) => spawn.name === 'ar-58-impl-pear')?.task) .toContain('Human clarification from GitHub:\nUse the shared retry helper and add regression coverage.') expect(fleet.inputs).toEqual([]) - expect(factory.status().counters.githubTriageAnswersDispatched).toBe(1) + await vi.waitFor(() => expect(factory.status().counters.githubTriageAnswersDispatched).toBe(1)) expect(factory.status().counters.githubTriageAnswersInjectedToAgents).toBeUndefined() fleet.emitAgentExit('ar-58-impl-pear', 'issue-done') From 02897ff42e01026211663bc024aef9db2597cd92 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 19 Jul 2026 14:22:56 +0200 Subject: [PATCH 8/8] fix: address dispatch recovery review feedback --- src/orchestrator/factory.test.ts | 45 ++++++++++++++++++++++++++++++++ src/orchestrator/factory.ts | 33 +++++++++-------------- 2 files changed, 58 insertions(+), 20 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 0dc2b02..caa8f0f 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -1802,6 +1802,48 @@ describe('FactoryLoop', () => { expect(fleet.spawns).toEqual([]) }) + it('builds one shared canonical GitHub path index for explicit alias dispatches', async () => { + class RecordingTreeMount extends FakeMountClient { + readonly treePrefixes: string[] = [] + + override async listTree(prefix: string): Promise { + this.treePrefixes.push(prefix) + return super.listTree(prefix) + } + } + + const firstAlias = githubIssuePath('AgentWorkforce', 'pear', 43) + const secondAlias = githubIssuePath('AgentWorkforce', 'pear', 44) + const firstCanonical = '/github/repos/AgentWorkforce/pear/issues/43__first-closed/meta.json' + const secondCanonical = '/github/repos/AgentWorkforce/pear/issues/44__second-closed/meta.json' + const firstOpen = githubIssueFile(43, { state: 'open', labels: ['factory'] }) + const secondOpen = githubIssueFile(44, { state: 'open', labels: ['factory'] }) + const mount = new RecordingTreeMount({ + [firstAlias]: firstOpen, + [secondAlias]: secondOpen, + [firstCanonical]: githubIssueFile(43, { state: 'closed', labels: ['factory'] }), + [secondCanonical]: githubIssueFile(44, { state: 'closed', labels: ['factory'] }), + }) + const fleet = new FakeFleetClient() + const factory = createFactory(config({ issueSource: 'github' }), { + mount, + fleet, + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + }) + + await expect(factory.dispatch(await factory.triageIssue(parseGithubFactoryIssue(firstAlias, firstOpen)))) + .rejects.toThrow(/Live state changed/) + await expect(factory.dispatch(await factory.triageIssue(parseGithubFactoryIssue(secondAlias, secondOpen)))) + .rejects.toThrow(/Live state changed/) + + expect(mount.treePrefixes).toEqual([ + '/github/repos/AgentWorkforce/pear/issues', + '/github/repos/AgentWorkforce__pear/issues', + ]) + expect(fleet.spawns).toEqual([]) + }) + it.each([ ['canonical', githubIssueNestedMetaPath('AgentWorkforce', 'pear', 45)], ['by-id alias', githubIssuePath('AgentWorkforce', 'pear', 45)], @@ -14423,11 +14465,13 @@ describe('FactoryLoop PR babysitter', () => { const mount = new FakeMountClient({ [issuePath(408)]: issue }) const fleet = new FakeFleetClient() const worktrees = new RecordingWorktreeManager() + const stateStore = new InMemoryStateStore({ batchSize: 2 }) const cleanupReleaseCounts: number[] = [] worktrees.onCleanup = () => cleanupReleaseCounts.push(fleet.releases.length) const factory = createFactory(config(), { mount, fleet, + stateStore, triage: new StaticTriage(), worktrees, linear: { @@ -14451,6 +14495,7 @@ describe('FactoryLoop PR babysitter', () => { .toEqual(['ar-408-impl-pear', 'ar-408-review']) expect(worktrees.cleaned).toHaveLength(1) expect(cleanupReleaseCounts).toEqual([2]) + await expect(stateStore.listFailureHandoffs('factory-test')).resolves.toEqual([]) expect(factory.status().inFlight).toEqual([]) }) diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 37a2db3..6a05600 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -312,6 +312,7 @@ export class FactoryLoop implements Factory { readonly #githubIssueAuthors = new Map() readonly #githubIssueAuthorLookups = new Map>() readonly #githubIssuePreferredPaths = new Map() + #githubIssuePathIndexReady = false readonly #slackReporterUserIds = new Map() readonly #slackReporterUserIdLookups = new Map>() readonly #reconciledGithubInProgress = new Set() @@ -3366,8 +3367,10 @@ export class FactoryLoop implements Factory { for (const [identity, path] of issuePaths) { this.#githubIssuePreferredPaths.set(identity, path) } + this.#githubIssuePathIndexReady = true return [...issuePaths.values()].sort() } catch (error) { + this.#githubIssuePathIndexReady = false this.#increment('githubIssueListFailures') this.#logger.warn?.('[factory] failed to list GitHub issue source tree', error) return [] @@ -3485,27 +3488,17 @@ export class FactoryLoop implements Factory { return path } - let preferred = cached ?? path - for (const root of githubIssueRepoRoots(parts.owner, parts.repo)) { - let paths: string[] - try { - paths = await this.#listRelayfileTree(root, 'GitHub canonical issue resolution') - } catch { - continue - } - for (const candidate of paths) { - const candidateParts = githubIssuePathParts(candidate) - if ( - !candidateParts || - githubIssueIdentity(candidateParts.owner, candidateParts.repo, candidateParts.number) !== identity - ) continue - if (githubIssuePathPreference(candidate) < githubIssuePathPreference(preferred)) { - preferred = candidate - } - } + // Normal discovery has already indexed every configured GitHub issue path. + // A dispatch-only replacement owner can reach this method before that + // backfill, so build the same shared index once instead of traversing both + // repository trees separately for every durable issue it recovers. + if (!this.#githubIssuePathIndexReady) { + await this.#githubIssuePaths() } - this.#githubIssuePreferredPaths.set(identity, preferred) - return preferred + const indexed = this.#githubIssuePreferredPaths.get(identity) + return indexed && githubIssuePathPreference(indexed) < githubIssuePathPreference(path) + ? indexed + : path } async #findGithubIssueMirror(