From bee81f1e3413b9f792e462c81943518dde66d99a Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 8 Aug 2026 22:36:51 +0200 Subject: [PATCH 01/19] docs: checkpoint dispatch fallback discovery --- .agent-notes/factory-dispatch-unblock.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .agent-notes/factory-dispatch-unblock.md diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md new file mode 100644 index 0000000..75a5427 --- /dev/null +++ b/.agent-notes/factory-dispatch-unblock.md @@ -0,0 +1,17 @@ +# Factory dispatch API fallback lane + +## 2026-08-08 discovery + +- Branch: `agent/factory-dispatch-api-fallback`, created from `origin/main` at `33cda42` (Factory PR #220 merge). +- The shared Factory checkout was not edited; it contains unrelated untracked files. +- Veto MCP methods requested by the workspace instructions are not exposed in this session. +- Current targeted resolution is projection-only: `runFactoryCommand` calls `readIssueArg`, which calls `findIssuePath`; GitHub resolution tries canonical repo-scoped paths, then lists configured Relayfile GitHub issue roots, and throws on zero matches before triage can run. +- The resolved mounted record is parsed by `parseGithubFactoryIssue`; `Factory.dispatch` then re-reads the same projection path and applies the existing scope, readiness, dispatchability, and repo-label routing gates. +- PR #220's `localMountDegraded` and daemon `eventListener` values are currently only assembled for `factory status`; targeted issue resolution does not consult or report them. +- Planned seam: a read-only, Relayfile-workspace-token GitHub API client on `MountClient`, projection-first targeted lookup, an authoritative provider lookup only after zero projection matches, explicit source/health metadata on triage and dispatch records, and provider re-read during dispatch safety validation. + +## Constraints retained + +- No queue, Cloudflare, mount, daemon, or launchd mutations. +- No `gh` process for the fallback. +- No merge and no default-branch push. From b9bb4542ebd81090fe7519db0190c8c21e70370e Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 8 Aug 2026 22:43:52 +0200 Subject: [PATCH 02/19] fix: fall back to GitHub API for stale issue projection --- .agent-notes/factory-dispatch-unblock.md | 9 + src/cli/fleet.test.ts | 222 +++++++++++++++++- src/cli/fleet.ts | 122 +++++++++- src/index.ts | 4 + .../relayfile-cloud-mount-client.test.ts | 1 + src/mount/relayfile-cloud-mount-client.ts | 15 ++ .../relayfile-github-connection-read.test.ts | 75 ++++++ src/mount/relayfile-github-connection-read.ts | 131 +++++++++++ src/orchestrator/factory.ts | 23 +- src/ports/index.ts | 2 + src/ports/mount.ts | 19 ++ src/triage/schema.ts | 14 ++ src/types.ts | 14 ++ 13 files changed, 639 insertions(+), 12 deletions(-) create mode 100644 src/mount/relayfile-github-connection-read.test.ts create mode 100644 src/mount/relayfile-github-connection-read.ts diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md index 75a5427..c0aca11 100644 --- a/.agent-notes/factory-dispatch-unblock.md +++ b/.agent-notes/factory-dispatch-unblock.md @@ -15,3 +15,12 @@ - No queue, Cloudflare, mount, daemon, or launchd mutations. - No `gh` process for the fallback. - No merge and no default-branch push. + +## 2026-08-08 implementation checkpoint + +- Added `RelayfileGithubConnectionRead`, which calls the Cloud GitHub GraphQL read route through `WorkspaceHandle.requestJson`; the SDK supplies the Relayfile workspace token and Factory never handles a GitHub token. +- Added `integration:github:read` to Factory's requested Relayfile workspace scopes. +- Targeted GitHub resolution now checks the Relayfile projection first, calls the API only after zero matches, and treats an authoritative empty API result as not found. +- Triage and dispatch results carry `issueResolution`; fallback records also include PR #220's `localMountDegraded`, `localMountDegradedReason`, and `eventListener` state. +- Dispatch re-reads a fallback issue through the provider before applying the existing scope/readiness/dispatchability/repo-label gates. +- Focused build/tests: exit 0; 153 tests passed across the new connection reader, mount client, and CLI suites. diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 0525462..c2438ce 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -17,7 +17,7 @@ import type { import { stateResolutionFromIds } from '../index' import { FileStateStore } from '../state/file-state-store' import { FakeFleetClient, FakeMountClient } from '../testing' -import type { GithubConnectionWrite, LocalMountOptions, SpawnInput, SpawnResult } from '../ports' +import type { GithubConnectionRead, GithubConnectionWrite, LocalMountOptions, SpawnInput, SpawnResult } from '../ports' import type { HarnessDriverClientLike } from '../fleet/internal-fleet-client' import { ensureLocalMount as runLocalMountPreflight } from '../mount/local-mount-preflight' import { formatLogArgs, installFactoryStopSignalHandlers, parseFleetCommand, parseGlobalOptions, resolveBrokerConnectionPath, runFleetCli } from './fleet' @@ -117,6 +117,21 @@ const githubIssueFile = (repo: string, number = 48) => ({ }, }) +const githubConnectionIssue = ( + repo: string, + number: number, + content: unknown = githubIssueFile(repo, number), +) => ({ + repo: `AgentWorkforce/${repo}`, + number, + path: `/github/repos/AgentWorkforce__${repo}/issues/by-id/${number}.json`, + content, +}) + +const fakeGithubConnectionRead = ( + resolveIssue: (repo: string, number: number) => ReturnType, +): GithubConnectionRead => ({ getIssue: vi.fn(resolveIssue) }) + const mountWithIntegrationConnections = ( files: Record, integrationConnections: FactoryIntegrationConnections, @@ -1560,6 +1575,211 @@ describe('fleet CLI runtime', () => { } }) + it('uses the GitHub API fallback after an empty projection and reports the existing health signals', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-api-fallback-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + loop: { heartbeatPath: join(root, 'missing-heartbeat.json'), heartbeatStaleMs: 10_000 }, + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }) + const githubRead = fakeGithubConnectionRead(async (repo, number) => + repo === 'AgentWorkforce/pear' && number === 222 + ? githubConnectionIssue('pear', number) + : undefined, + ) + const mount = Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ + degraded: true, + reason: 'last reconcile is stale', + localDir: join(root, '.integrations'), + }), + }) + const output = buffer() + + const code = await runFleetCli(['triage', '222', '--config', configPath], { + fleet: new FakeFleetClient(), + mount, + stdout: output, + stderr: buffer(), + }) + + expect(code).toBe(0) + expect(JSON.parse(output.text())).toMatchObject({ + issue: { key: '222' }, + routes: [{ repo: 'AgentWorkforce/pear' }], + issueResolution: { + source: 'github-api-fallback', + repo: 'AgentWorkforce/pear', + projection: { + outcome: 'no-match', + localMountDegraded: true, + localMountDegradedReason: 'last reconcile is stale', + eventListener: { state: 'not-listening' }, + }, + }, + }) + expect(githubRead.getIssue).toHaveBeenCalledWith('AgentWorkforce/pear', 222) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('keeps a populated Relayfile projection preferred and reports that source', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-projection-source-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }) + const issuePath = '/github/repos/AgentWorkforce__pear/issues/by-id/222.json' + const githubRead = fakeGithubConnectionRead(async () => { + throw new Error('GitHub API must not run when the projection has a match') + }) + const mount = Object.assign(new FakeMountClient({ + [issuePath]: githubIssueFile('pear', 222), + }), { githubRead }) + const output = buffer() + + const code = await runFleetCli(['triage', '222', '--config', configPath], { + fleet: new FakeFleetClient(), + mount, + stdout: output, + stderr: buffer(), + }) + + expect(code).toBe(0) + expect(JSON.parse(output.text())).toMatchObject({ + issue: { key: '222', path: issuePath }, + issueResolution: { + source: 'relayfile-projection', + projection: { outcome: 'matched' }, + }, + }) + expect(githubRead.getIssue).not.toHaveBeenCalled() + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('does not manufacture a match when GitHub authoritatively has no issue', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-api-not-found-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }) + const githubRead = fakeGithubConnectionRead(async () => undefined) + const errors = buffer() + + const code = await runFleetCli(['triage', '999999', '--config', configPath], { + fleet: new FakeFleetClient(), + mount: Object.assign(new FakeMountClient(), { githubRead }), + stdout: buffer(), + stderr: errors, + }) + + expect(code).toBe(1) + expect(errors.text()).toContain('found 0 matches in the projection and GitHub API') + expect(githubRead.getIssue).toHaveBeenCalledWith('AgentWorkforce/pear', 999999) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('re-reads an API fallback issue during dry-run dispatch and records the source', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-api-dispatch-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }) + const content = githubIssueFile('pear', 222) + content.payload.body = [ + 'Restore dispatch through the GitHub API fallback without changing routing.', + '', + 'Acceptance criteria:', + '- Projection misses use the workspace connection.', + '- Projection hits remain preferred.', + '- Run the focused CLI regression checks.', + ].join('\n') + const githubRead = fakeGithubConnectionRead(async (_repo, number) => + githubConnectionIssue('pear', number, content), + ) + const output = buffer() + + const code = await runFleetCli(['dispatch', '222', '--dry-run', '--config', configPath], { + fleet: new FakeFleetClient(), + mount: Object.assign(new FakeMountClient(), { githubRead }), + stdout: output, + stderr: buffer(), + }) + + expect(code).toBe(0) + expect(JSON.parse(output.text())).toMatchObject({ + issue: { key: '222' }, + issueResolution: { source: 'github-api-fallback' }, + comments: [expect.stringContaining('Issue resolution: github-api-fallback')], + }) + expect(githubRead.getIssue).toHaveBeenCalledTimes(2) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('does not let the API fallback bypass the existing GitHub safety label gate', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-api-safety-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + safety: { requireLabel: 'factory', requireTitlePrefix: '[factory]', requireTeamKey: 'AR' }, + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }) + const unsafe = githubIssueFile('pear', 222) + unsafe.payload.labels = [{ name: 'pear' }] + unsafe.payload.title = 'Missing both configured safety markers' + const githubRead = fakeGithubConnectionRead(async (_repo, number) => + githubConnectionIssue('pear', number, unsafe), + ) + const errors = buffer() + const fleet = new FakeFleetClient() + + const code = await runFleetCli(['dispatch', '222', '--dry-run', '--config', configPath], { + fleet, + mount: Object.assign(new FakeMountClient(), { githubRead }), + stdout: buffer(), + stderr: errors, + }) + + expect(code).toBe(1) + expect(errors.text()).toContain('not factory-e2e scope') + expect(fleet.spawns).toEqual([]) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('rejects non-positive GitHub issue numbers with source context', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-invalid-number-')) try { diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index 63fd480..56c3270 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -52,6 +52,8 @@ import { type LocalMountHealthEvent, type LocalMountOptions, type MountClient, + type IssueResolution, + type LinearIssue, type ProbeCloser, type RelayfileCloudMountClientConfig, type ResolvedFactoryWorkspace, @@ -753,8 +755,13 @@ async function runFactoryCommand( return result.ok ? 0 : 1 } - const issue = await readIssueArg(mount, command.issue, config) - const decision = await factory.triageIssue(issue) + const resolved = await resolveIssueArg(mount, command.issue, config, async () => + issueProjectionStatus(factory, mount, config), + ) + const decision = { + ...await factory.triageIssue(resolved.issue), + issueResolution: resolved.resolution, + } if (command.kind === 'factory-triage') { writeJson(out, decision) return 0 @@ -1637,15 +1644,54 @@ const scopeIssueFromDraftContent = (content: unknown) => ({ raw: asRecord(content), }) -async function readIssueArg(mount: MountClient, issueArg: string, config: FactoryConfig) { - const path = issueArg.startsWith('/') ? issueArg : await findIssuePath(mount, issueArg, config) - if (githubIssuePathParts(path)) { - return parseGithubFactoryIssue(path, (await mount.readFile(path)).content) +async function readIssueArg(mount: MountClient, issueArg: string, config: FactoryConfig): Promise { + return (await resolveIssueArg(mount, issueArg, config)).issue +} + +type ResolvedIssueArg = { issue: LinearIssue; resolution: IssueResolution } + +async function resolveIssueArg( + mount: MountClient, + issueArg: string, + config: FactoryConfig, + projectionStatus?: () => Promise, +): Promise { + const explicitPath = issueArg.startsWith('/') + const path = explicitPath ? issueArg : await findIssuePath(mount, issueArg, config) + if (path) { + const issue = githubIssuePathParts(path) + ? parseGithubFactoryIssue(path, (await mount.readFile(path)).content) + : await readLinearIssueWithCanonicalFallback(mount, path) + return { + issue, + resolution: { + source: 'relayfile-projection', + detail: 'Resolved from the preferred Relayfile projection.', + projection: { outcome: 'matched' }, + }, + } + } + + const number = Number(issueArg.replace(/^#/, '')) + const projection = projectionStatus + ? await projectionStatus() + : projectionStatusFromMount(mount) + const fallback = await findGithubIssueThroughConnection(mount, number, config) + if (!fallback) { + throw new Error(`${githubIssueResolutionError(config, issueArg)}: found 0 matches in the projection and GitHub API`) + } + return { + issue: parseGithubFactoryIssue(fallback.path, fallback.content), + resolution: { + source: 'github-api-fallback', + repo: fallback.repo, + detail: 'Relayfile projection returned no match; resolved authoritatively through the workspace GitHub API connection.', + projection, + }, } - return readLinearIssueWithCanonicalFallback(mount, path) } -async function findIssuePath(mount: MountClient, key: string, config: FactoryConfig): Promise { +async function findIssuePath(mount: MountClient, key: string, config: FactoryConfig): Promise { if (config.issueSource === 'github') { const number = Number(key.replace(/^#/, '')) if (!Number.isInteger(number) || number <= 0) { @@ -1678,7 +1724,7 @@ async function findIssuePath(mount: MountClient, key: string, config: FactoryCon }) .sort((left, right) => githubIssuePathPreference(left) - githubIssuePathPreference(right) || left.localeCompare(right)) if (matches.length === 0) { - throw new Error(`${githubIssueResolutionError(config, key)}: found 0 matches`) + return undefined } const matchesByRepo = new Map() for (const path of matches) { @@ -1709,6 +1755,64 @@ async function findIssuePath(mount: MountClient, key: string, config: FactoryCon return matches[0] } +async function findGithubIssueThroughConnection( + mount: MountClient, + number: number, + config: FactoryConfig, +) { + const github = mount.githubRead + if (!github) { + throw new Error( + `${githubIssueResolutionError(config, String(number))}: projection returned no match and the GitHub API fallback is unavailable`, + ) + } + const repos = configuredGithubIssueRepos(config) + if (repos.length === 0) { + throw new Error( + `${githubIssueResolutionError(config, String(number))}: projection returned no match and no configured owner/repo is available for the GitHub API fallback`, + ) + } + const matches = (await Promise.all(repos.map((repo) => github.getIssue(repo, number)))) + .filter((issue): issue is NonNullable => Boolean(issue)) + if (matches.length === 0) return undefined + if (!config.repos.default && matches.length > 1) { + const matchedRepos = matches.map((match) => match.repo).sort((left, right) => left.localeCompare(right)) + throw new Error( + `${githubIssueResolutionError(config, String(number))}: GitHub API matches multiple repositories (${matchedRepos.join(', ')}); ` + + 'set repos.default or pass a repo-qualified argument', + ) + } + return matches[0] +} + +async function issueProjectionStatus( + factory: Factory, + mount: MountClient, + config: FactoryConfig, +): Promise { + const status = await factoryStatusWithMountHealth( + factory, + mount, + config.loop.heartbeatPath, + config.loop.heartbeatStaleMs, + ) + return { + outcome: 'no-match', + ...(status.localMountDegraded !== undefined ? { localMountDegraded: status.localMountDegraded } : {}), + ...(status.localMountDegradedReason ? { localMountDegradedReason: status.localMountDegradedReason } : {}), + ...(status.eventListener ? { eventListener: status.eventListener } : {}), + } +} + +function projectionStatusFromMount(mount: MountClient): IssueResolution['projection'] { + const health = mount.getLocalMountHealth?.() + return { + outcome: 'no-match', + ...(health ? { localMountDegraded: health.degraded } : {}), + ...(health?.reason ? { localMountDegradedReason: health.reason } : {}), + } +} + function configuredGithubIssueRepos(config: FactoryConfig): string[] { const candidates = config.repos.default ? [config.repos.default] diff --git a/src/index.ts b/src/index.ts index b5a4e9b..0dac27a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -80,6 +80,7 @@ export { resolveFactoryWorkspace, } from './mount/relayfile-cloud-mount-client' export { RelayfileGithubConnectionWrite } from './mount/relayfile-github-connection-write' +export { RelayfileGithubConnectionRead } from './mount/relayfile-github-connection-read' export { ensureFactoryIntegrations, inspectFactoryIntegration, @@ -295,6 +296,8 @@ export type { Clock, EventPage, GithubConnectionWrite, + GithubConnectionIssue, + GithubConnectionRead, FactoryIntegrationConnectionStatus, FactoryIntegrationConnections, FactoryIntegrationConnectResult, @@ -438,6 +441,7 @@ export type { FactoryStartOptions, FactoryStatus, IssueRef, + IssueResolution, IterationReport, LinearIssue, ProbeCloser, diff --git a/src/mount/relayfile-cloud-mount-client.test.ts b/src/mount/relayfile-cloud-mount-client.test.ts index 4dcfda8..7d97914 100644 --- a/src/mount/relayfile-cloud-mount-client.test.ts +++ b/src/mount/relayfile-cloud-mount-client.test.ts @@ -994,6 +994,7 @@ describe('RelayfileCloudMountClient', () => { // github uses the provider root `/github/**`; `/github/repos/**` is rejected // by RelayAuth's path-token validator and would fail the whole batch mint. expect(joinOptions.scopes).toContain('relayfile:fs:write:/github/**') + expect(joinOptions.scopes).toContain('integration:github:read') expect(joinOptions.scopes).not.toContain('relayfile:fs:write:/github/repos/**') expect(joinOptions.scopes).toContain('relayfile:fs:write:/factory/observability/**') expect(joinOptions.scopes).toContain('relayfile:fs:read:/slack/users/**') diff --git a/src/mount/relayfile-cloud-mount-client.ts b/src/mount/relayfile-cloud-mount-client.ts index 0d61125..9bf5489 100644 --- a/src/mount/relayfile-cloud-mount-client.ts +++ b/src/mount/relayfile-cloud-mount-client.ts @@ -28,6 +28,7 @@ import type { EventPage, FactoryIntegrationConnections, FactoryIntegrationProvider, + GithubConnectionRead, GithubConnectionWrite, LocalMountOptions, LocalMountHealth, @@ -46,6 +47,7 @@ import { } from '../subscriptions' import type { ResourceSubscriptionsClient } from '../subscriptions' import { RelayfileGithubConnectionWrite } from './relayfile-github-connection-write' +import { RelayfileGithubConnectionRead } from './relayfile-github-connection-read' import { ensureLocalMount as runLocalMountPreflight, type EnsureLocalMountOptions, @@ -71,6 +73,7 @@ export const FACTORY_RELAYFILE_SCOPES = [ // cleanly. Do NOT narrow this back to `/github/repos/**`. 'relayfile:fs:read:/github/**', 'relayfile:fs:write:/github/**', + 'integration:github:read', 'relayfile:fs:read:/slack/channels/**', 'relayfile:fs:write:/slack/channels/**', 'relayfile:fs:read:/slack/users/**', @@ -125,6 +128,13 @@ export interface RelayfileWorkspaceHandleLike { info: { relayfileUrl: string } client(): RelayFileClientLike getToken(): Promise | string + requestJson?(options: { + operation: string + method: string + path: string + body?: unknown + timeoutMs?: number + }): Promise getConnectionStatus?(provider: FactoryIntegrationProvider, connectionId: string): Promise<{ ready: boolean state?: string @@ -257,6 +267,7 @@ export function relayfileWorkspaceTokenProvider( export class RelayfileCloudMountClient implements MountClient { readonly workspaceId: string readonly writebackTransport = 'relayfile-cloud' + readonly githubRead?: GithubConnectionRead readonly githubWrite: GithubConnectionWrite readonly resourceSubscriptions?: ResourceSubscriptionsClient readonly integrationConnections?: FactoryIntegrationConnections @@ -337,6 +348,10 @@ export class RelayfileCloudMountClient implements MountClient { resolveRegisteredWorkspaceMirror(workspaceIds)?.localDir) this.#isAllowedDraft = config.isAllowedDraft this.#isAllowedDelete = config.isAllowedDelete + const githubConnectionRequest = config.relayfileWorkspace?.requestJson?.bind(config.relayfileWorkspace) + this.githubRead = githubConnectionRequest + ? new RelayfileGithubConnectionRead({ workspace: { requestJson: githubConnectionRequest } }) + : undefined this.githubWrite = new RelayfileGithubConnectionWrite({ mount: this }) this.integrationConnections = relayfileIntegrationConnections( config.relayfileWorkspace, diff --git a/src/mount/relayfile-github-connection-read.test.ts b/src/mount/relayfile-github-connection-read.test.ts new file mode 100644 index 0000000..98b65c8 --- /dev/null +++ b/src/mount/relayfile-github-connection-read.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from 'vitest' + +import { RelayfileGithubConnectionRead } from './relayfile-github-connection-read' + +describe('RelayfileGithubConnectionRead', () => { + it('reads an issue through the Relayfile workspace SDK request surface', async () => { + const requestJson = vi.fn(async () => ({ + data: { + repository: { + issue: { + id: 'I_222', + number: 222, + title: '[factory] Restore dispatch', + body: 'Use the connected GitHub API as a fallback.', + state: 'OPEN', + url: 'https://github.example/AgentWorkforce/factory/issues/222', + updatedAt: '2026-08-08T12:00:00Z', + author: { login: 'factory-app' }, + labels: { nodes: [{ name: 'factory' }, { name: 'factory-repo' }] }, + }, + }, + }, + })) + const reader = new RelayfileGithubConnectionRead({ workspace: { requestJson } }) + + await expect(reader.getIssue('AgentWorkforce/factory', 222)).resolves.toEqual({ + repo: 'AgentWorkforce/factory', + number: 222, + path: '/github/repos/AgentWorkforce__factory/issues/by-id/222.json', + content: expect.objectContaining({ + provider: 'github', + objectType: 'issue', + objectId: 'I_222', + payload: expect.objectContaining({ + number: 222, + title: '[factory] Restore dispatch', + state: 'open', + labels: [{ name: 'factory' }, { name: 'factory-repo' }], + repository: { name: 'factory', owner: { login: 'AgentWorkforce' } }, + }), + }), + }) + expect(requestJson).toHaveBeenCalledWith(expect.objectContaining({ + operation: 'getGithubIssue', + method: 'POST', + path: 'api/v1/github/graphql', + body: expect.objectContaining({ + variables: { owner: 'AgentWorkforce', repo: 'factory', number: 222 }, + }), + })) + }) + + it('returns undefined only for an authoritative empty issue result', async () => { + const reader = new RelayfileGithubConnectionRead({ + workspace: { requestJson: vi.fn(async () => ({ data: { repository: { issue: null } } })) }, + }) + + await expect(reader.getIssue('AgentWorkforce/factory', 999_999)).resolves.toBeUndefined() + }) + + it('surfaces GraphQL failures instead of manufacturing absence', async () => { + const reader = new RelayfileGithubConnectionRead({ + workspace: { + requestJson: vi.fn(async () => ({ + data: { repository: { issue: null } }, + errors: [{ extensions: { type: 'FORBIDDEN' } }], + })), + }, + }) + + await expect(reader.getIssue('AgentWorkforce/factory', 222)).rejects.toThrow( + 'GitHub API issue lookup failed (FORBIDDEN)', + ) + }) +}) diff --git a/src/mount/relayfile-github-connection-read.ts b/src/mount/relayfile-github-connection-read.ts new file mode 100644 index 0000000..80fc698 --- /dev/null +++ b/src/mount/relayfile-github-connection-read.ts @@ -0,0 +1,131 @@ +import type { GithubConnectionIssue, GithubConnectionRead } from '../ports' + +const GITHUB_ISSUE_QUERY = ` + query FactoryIssue($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + issue(number: $number) { + id + number + title + body + state + url + updatedAt + author { login } + labels(first: 100) { nodes { name } } + } + } + } +` + +export interface RelayfileGithubConnectionRequest { + requestJson(options: { + operation: string + method: string + path: string + body?: unknown + timeoutMs?: number + }): Promise +} + +export interface RelayfileGithubConnectionReadConfig { + workspace: RelayfileGithubConnectionRequest +} + +/** + * Read-only GitHub issue lookup through the authenticated Relayfile workspace + * connection. WorkspaceHandle.requestJson supplies the Relayfile workspace + * token; Factory never receives or shells out with a provider credential. + */ +export class RelayfileGithubConnectionRead implements GithubConnectionRead { + readonly #workspace: RelayfileGithubConnectionRequest + + constructor(config: RelayfileGithubConnectionReadConfig) { + this.#workspace = config.workspace + } + + async getIssue(repo: string, number: number): Promise { + const { owner, name } = githubRepoParts(repo) + if (!Number.isSafeInteger(number) || number <= 0) { + throw new Error(`GitHub issue number must be a positive integer: ${number}`) + } + + const response = record(await this.#workspace.requestJson({ + operation: 'getGithubIssue', + method: 'POST', + path: 'api/v1/github/graphql', + body: { + query: GITHUB_ISSUE_QUERY, + variables: { owner, repo: name, number }, + }, + timeoutMs: 30_000, + })) + const errors = Array.isArray(response?.errors) ? response.errors : [] + if (errors.length > 0) { + const codes = errors + .map((error) => stringValue(record(record(error)?.extensions)?.type) ?? stringValue(record(record(error)?.extensions)?.code)) + .filter((code): code is string => Boolean(code)) + throw new Error(`GitHub API issue lookup failed${codes.length > 0 ? ` (${[...new Set(codes)].join(', ')})` : ''}`) + } + + const issue = record(record(record(response?.data)?.repository)?.issue) + if (!issue) return undefined + const resolvedNumber = positiveInteger(issue.number) + const title = stringValue(issue.title) + const url = stringValue(issue.url) + if (resolvedNumber !== number || !title || !url) { + throw new Error(`GitHub API issue lookup returned an incomplete record for ${repo}#${number}`) + } + + const labels = record(issue.labels)?.nodes + const author = stringValue(record(issue.author)?.login) + const path = githubIssuePath(owner, name, number) + return { + repo: `${owner}/${name}`, + number, + path, + content: { + provider: 'github', + objectType: 'issue', + objectId: stringValue(issue.id) ?? `${owner}/${name}#${number}`, + payload: { + id: stringValue(issue.id), + number, + title, + body: stringValue(issue.body) ?? '', + state: (stringValue(issue.state) ?? '').toLowerCase(), + url, + html_url: url, + updated_at: stringValue(issue.updatedAt), + labels: Array.isArray(labels) + ? labels.map((label) => ({ name: stringValue(record(label)?.name) })).filter((label) => Boolean(label.name)) + : [], + ...(author ? { user: { login: author }, author: { login: author } } : {}), + repository: { name, owner: { login: owner } }, + }, + }, + } + } +} + +const githubRepoParts = (repo: string): { owner: string; name: string } => { + const [owner, name, ...extra] = repo.split('/') + if (!owner || !name || extra.length > 0) { + throw new Error(`GitHub repo must be owner/repo: ${repo}`) + } + return { owner, name } +} + +const githubIssuePath = (owner: string, repo: string, number: number): string => + `/github/repos/${encodeURIComponent(owner)}__${encodeURIComponent(repo)}/issues/by-id/${number}.json` + +const record = (value: unknown): Record | undefined => + value !== null && typeof value === 'object' && !Array.isArray(value) + ? value as Record + : undefined + +const stringValue = (value: unknown): string | undefined => + typeof value === 'string' && value.length > 0 ? value : undefined + +const positiveInteger = (value: unknown): number | undefined => + typeof value === 'number' && Number.isSafeInteger(value) && value > 0 ? value : undefined diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 927ec21..ff7defc 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -2388,7 +2388,9 @@ export class FactoryLoop implements Factory { const dispatched = this.#dispatchUnlocked(decision, opts) this.#dispatchInFlight.set(key, dispatched) try { - return await dispatched + const result = await dispatched + if (decision.issueResolution) result.issueResolution = structuredClone(decision.issueResolution) + return result } finally { if (this.#dispatchInFlight.get(key) === dispatched) { this.#dispatchInFlight.delete(key) @@ -4669,8 +4671,22 @@ export class FactoryLoop implements Factory { } } catch (error) { if (isMissingIssueFileError(error)) { + const parts = githubIssuePathParts(path) ?? githubIssueDirectoryPathParts(path) + if (parts && this.#mount.githubRead) { + const fallback = await this.#mount.githubRead.getIssue(`${parts.owner}/${parts.repo}`, parts.number) + if (fallback) { + const githubIssue = parseGithubIssue(fallback.path, fallback.content) + this.#indexDependencyIssue(githubIssueAsFactoryIssue(githubIssue)) + this.#logger.warn?.('[factory] Relayfile projection missed GitHub issue; using GitHub API fallback', { + repo: fallback.repo, + number: fallback.number, + source: 'github-api-fallback', + }) + return githubIssue + } + } this.#increment('githubIssuePhantomSkipped') - this.#logger.debug?.('[factory] skipped missing GitHub issue file discovered from issue tree', { path }) + this.#logger.debug?.('[factory] skipped missing GitHub issue after projection and provider lookup', { path }) return undefined } throw error @@ -13433,6 +13449,9 @@ const pidsFromSpawnResult = (result: { pid?: number; pids?: number[] } | undefin const dispatchComment = (decision: TriageDecision, agents: DispatchResult['agents']): string => [ `Factory dispatch for ${decision.issue.key}`, + decision.issueResolution + ? `Issue resolution: ${decision.issueResolution.source} — ${decision.issueResolution.detail}` + : undefined, `Implementers: ${agents.filter((agent) => agent.role === 'implementer').map((agent) => agent.name).join(', ') || 'none'}`, decision.scope === 'workflow' ? `Workflow: ${agents.find((agent) => agent.role === 'workflow')?.name ?? 'none'}` : undefined, `Reviewer: ${agents.find((agent) => agent.role === 'reviewer')?.name ?? 'none'}`, diff --git a/src/ports/index.ts b/src/ports/index.ts index 90fb19c..9377759 100644 --- a/src/ports/index.ts +++ b/src/ports/index.ts @@ -5,6 +5,8 @@ export type { FactoryIntegrationConnections, FactoryIntegrationConnectResult, FactoryIntegrationProvider, + GithubConnectionIssue, + GithubConnectionRead, GithubConnectionWrite, GithubPublishPullRequestInput, GithubPublishPullRequestResult, diff --git a/src/ports/mount.ts b/src/ports/mount.ts index ab142a7..f24e7d0 100644 --- a/src/ports/mount.ts +++ b/src/ports/mount.ts @@ -71,6 +71,24 @@ export interface GithubPublishPullRequestResult { author?: string } +/** + * Provider-authoritative GitHub issue returned through the workspace's + * connected GitHub integration. `content` intentionally uses the same + * provider record shape as the Relayfile projection so the existing parser + * and safety gates stay authoritative. + */ +export interface GithubConnectionIssue { + repo: string + number: number + path: string + content: unknown +} + +export interface GithubConnectionRead { + /** Returns undefined only when GitHub authoritatively reports no issue. */ + getIssue(repo: string, number: number): Promise +} + export type FactoryIntegrationProvider = 'github' | 'linear' export interface FactoryIntegrationConnectionStatus { @@ -104,6 +122,7 @@ export interface GithubConnectionWrite { export interface MountClient { readonly writebackTransport?: 'relayfile-cloud' | 'test' + readonly githubRead?: GithubConnectionRead readonly githubWrite?: GithubConnectionWrite /** * Optional durable Relayfile resource-subscription API. Its absence means diff --git a/src/triage/schema.ts b/src/triage/schema.ts index b0467c1..f6d5c3a 100644 --- a/src/triage/schema.ts +++ b/src/triage/schema.ts @@ -23,6 +23,20 @@ export const TriageDecisionSchema = z.object({ key: z.string(), path: z.string(), }), + issueResolution: z.object({ + source: z.enum(['relayfile-projection', 'github-api-fallback']), + repo: z.string().optional(), + detail: z.string(), + projection: z.object({ + outcome: z.enum(['matched', 'no-match']), + localMountDegraded: z.boolean().optional(), + localMountDegradedReason: z.string().optional(), + eventListener: z.object({ + state: z.enum(['starting', 'subscribed', 'polling', 'not-listening', 'unknown']), + reason: z.string().optional(), + }).optional(), + }), + }).optional(), routes: z.array(z.object({ repo: z.string(), clonePath: z.string().optional(), diff --git a/src/types.ts b/src/types.ts index 1821891..34a414f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -192,6 +192,7 @@ export interface IterationReport { export interface DispatchResult { issue: IssueRef + issueResolution?: IssueResolution agents: Array<{ name: string; role: AgentSpec['role'] }> comments?: string[] stateId?: string @@ -244,6 +245,7 @@ export interface RepoMapEntry { export interface TriageDecision { issue: IssueRef + issueResolution?: IssueResolution routes: Array<{ repo: string; clonePath?: string; rationale: string }> scope: 'single' | 'workflow' | 'team' implementers: AgentSpec[] @@ -254,6 +256,18 @@ export interface TriageDecision { rationale: string } +export interface IssueResolution { + source: 'relayfile-projection' | 'github-api-fallback' + repo?: string + detail: string + projection: { + outcome: 'matched' | 'no-match' + localMountDegraded?: boolean + localMountDegradedReason?: string + eventListener?: FactoryEventListenerStatus + } +} + export interface PrSummary { repo: string number: number From 848b210805e2db731a3fa1c0f9c569f502e02972 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 8 Aug 2026 22:48:16 +0200 Subject: [PATCH 03/19] fix: allow targeted fallback past stale projection preflight --- .agent-notes/factory-dispatch-unblock.md | 2 ++ src/cli/fleet.test.ts | 38 ++++++++++++++++++++++++ src/cli/fleet.ts | 24 ++++++++++++++- 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md index c0aca11..422d3cb 100644 --- a/.agent-notes/factory-dispatch-unblock.md +++ b/.agent-notes/factory-dispatch-unblock.md @@ -24,3 +24,5 @@ - Triage and dispatch results carry `issueResolution`; fallback records also include PR #220's `localMountDegraded`, `localMountDegradedReason`, and `eventListener` state. - Dispatch re-reads a fallback issue through the provider before applying the existing scope/readiness/dispatchability/repo-label gates. - Focused build/tests: exit 0; 153 tests passed across the new connection reader, mount client, and CLI suites. +- Live preflight exposed a necessary unblock: the connected GitHub projection currently reports `degraded, complete`, and the old preflight rejected the command before resolution. Targeted triage/dispatch now proceeds only when the SDK GitHub read seam exists; missing connections and run-loop/canary flows retain the existing preflight. The command prints a warning and still checks the projection first. +- Updated focused build/tests: exit 0; 154 tests passed. diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index c2438ce..d83dab9 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -758,6 +758,44 @@ describe('fleet CLI runtime', () => { } }) + it('allows targeted GitHub resolution through the API seam when the connected projection is not ready', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-integration-github-fallback-')) + try { + const configPath = await writeConfig(root, { issueSource: 'github' }) + const issue = '/github/repos/AgentWorkforce__pear/issues/48/meta.json' + const integrations = fakeIntegrationConnections(async () => ({ + ready: false, + state: 'degraded', + initialSyncState: 'complete', + })) + const githubRead = fakeGithubConnectionRead(async () => { + throw new Error('populated projection must remain preferred') + }) + const mount = Object.assign( + mountWithIntegrationConnections({ [issue]: githubIssueFile('pear') }, integrations), + { githubRead }, + ) + const output = buffer() + const errors = buffer() + + const code = await runFleetCli(['triage', '48', '--config', configPath], { + fleet: new FakeFleetClient(), + mount, + stdout: output, + stderr: errors, + }) + + expect(code).toBe(0) + expect(errors.text()).toContain('GitHub projection is not ready (degraded, complete)') + expect(JSON.parse(output.text())).toMatchObject({ + issueResolution: { source: 'relayfile-projection' }, + }) + expect(githubRead.getIssue).not.toHaveBeenCalled() + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('does not prompt or connect when a missing integration is checked without a TTY', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-integration-headless-')) try { diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index 56c3270..b9c7f6e 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -1446,9 +1446,31 @@ async function prepareFactoryIntegrations( const providers = requiredIntegrationsForCommand(command, config) if (providers.length === 0) return const dryRun = globals.dryRun || command.kind === 'factory-canary' + const providersToEnsure: FactoryIntegrationProvider[] = [] + for (const provider of providers) { + const observation = observed.get(provider) ?? await inspectFactoryIntegration(connections, provider) + observed.set(provider, observation) + if ( + provider === 'github' && + observation.kind === 'connected-not-ready' && + mount.githubRead && + (command.kind === 'factory-triage' || command.kind === 'factory-dispatch') + ) { + const details = [observation.state, observation.initialSyncState] + .filter((value): value is string => Boolean(value)) + .join(', ') + err.write( + `[factory] warning: GitHub projection is not ready${details ? ` (${details})` : ''}; ` + + 'targeted resolution remains projection-first and will use the GitHub API fallback only for a miss.\n', + ) + continue + } + providersToEnsure.push(provider) + } + if (providersToEnsure.length === 0) return await ensureFactoryIntegrations({ connections, - providers, + providers: providersToEnsure, workspaceId, interactive: !dryRun && (deps.isInteractive?.() ?? Boolean(process.stdin.isTTY && process.stderr.isTTY)), dryRun, From 886327a1321f7923f27e6fe53d95c1898b487deb Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 8 Aug 2026 23:07:28 +0200 Subject: [PATCH 04/19] fix: use direct GitHub API for stale projection --- .agent-notes/factory-dispatch-unblock.md | 7 + src/cli/fleet.test.ts | 103 +++++++++++++- src/cli/fleet.ts | 77 ++++++++-- src/index.ts | 2 +- src/mount/github-api-issue-read.test.ts | 77 ++++++++++ src/mount/github-api-issue-read.ts | 111 +++++++++++++++ .../relayfile-cloud-mount-client.test.ts | 1 - src/mount/relayfile-cloud-mount-client.ts | 8 +- .../relayfile-github-connection-read.test.ts | 75 ---------- src/mount/relayfile-github-connection-read.ts | 131 ------------------ src/ports/mount.ts | 8 +- 11 files changed, 364 insertions(+), 236 deletions(-) create mode 100644 src/mount/github-api-issue-read.test.ts create mode 100644 src/mount/github-api-issue-read.ts delete mode 100644 src/mount/relayfile-github-connection-read.test.ts delete mode 100644 src/mount/relayfile-github-connection-read.ts diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md index 422d3cb..17c5588 100644 --- a/.agent-notes/factory-dispatch-unblock.md +++ b/.agent-notes/factory-dispatch-unblock.md @@ -26,3 +26,10 @@ - Focused build/tests: exit 0; 153 tests passed across the new connection reader, mount client, and CLI suites. - Live preflight exposed a necessary unblock: the connected GitHub projection currently reports `degraded, complete`, and the old preflight rejected the command before resolution. Targeted triage/dispatch now proceeds only when the SDK GitHub read seam exists; missing connections and run-loop/canary flows retain the existing preflight. The command prints a warning and still checks the projection first. - Updated focused build/tests: exit 0; 154 tests passed. + +## 2026-08-08 live correction + +- The connection-backed GraphQL attempt reached Cloud but exited 1 `Forbidden`: that route additionally requires a deployed sponsor persona, which the local Factory workspace join is not. Replaced it with a read-only direct GitHub REST client; GitHub writes remain on Relayfile app-authored writeback. +- Fallback eligibility now uses PR #220's health facts: a degraded local mount or listener state other than `subscribed`/`polling` means the projection cannot answer. A healthy projection miss fails without calling GitHub. +- Added configured `repo#number` and `owner/repo#number` selectors so a targeted fallback performs one authoritative lookup and avoids ambiguous org-wide probes. +- Live `factory triage factory#222` reached the empty projection, reported the listener `unknown`, resolved via `github-api-fallback`, and routed to `AgentWorkforce/factory`. It emitted its successful decision; the existing one-shot shutdown path remained open until SIGINT, then returned 0. The shutdown hang is separate from issue resolution. diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index d83dab9..4fda930 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -1725,7 +1725,10 @@ describe('fleet CLI runtime', () => { const code = await runFleetCli(['triage', '999999', '--config', configPath], { fleet: new FakeFleetClient(), - mount: Object.assign(new FakeMountClient(), { githubRead }), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), stdout: buffer(), stderr: errors, }) @@ -1765,7 +1768,10 @@ describe('fleet CLI runtime', () => { const code = await runFleetCli(['dispatch', '222', '--dry-run', '--config', configPath], { fleet: new FakeFleetClient(), - mount: Object.assign(new FakeMountClient(), { githubRead }), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), stdout: output, stderr: buffer(), }) @@ -1805,7 +1811,10 @@ describe('fleet CLI runtime', () => { const code = await runFleetCli(['dispatch', '222', '--dry-run', '--config', configPath], { fleet, - mount: Object.assign(new FakeMountClient(), { githubRead }), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), stdout: buffer(), stderr: errors, }) @@ -1818,6 +1827,94 @@ describe('fleet CLI runtime', () => { } }) + it('treats a healthy projection miss as authoritative and does not call the fallback', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-healthy-miss-')) + try { + const heartbeatPath = join(root, 'heartbeat.json') + const configPath = await writeConfig(root, { + issueSource: 'github', + loop: { heartbeatPath, heartbeatStaleMs: 10_000 }, + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }) + const githubRead = fakeGithubConnectionRead(async () => githubConnectionIssue('pear', 222)) + const errors = buffer() + const now = Date.now() + await writeFile(heartbeatPath, JSON.stringify({ + pid: process.pid, + status: 'running', + iteration: 0, + maxIterations: 0, + updatedAt: new Date(now).toISOString(), + updatedAtMs: now, + eventListener: { state: 'subscribed' }, + })) + + const code = await runFleetCli(['triage', '222', '--config', configPath], { + fleet: new FakeFleetClient(), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: false }), + }), + stdout: buffer(), + stderr: errors, + }) + + expect(code).toBe(1) + expect(errors.text()).toContain('found 0 matches in the healthy Relayfile projection') + expect(errors.text()).toContain('fallback was not used') + expect(githubRead.getIssue).not.toHaveBeenCalled() + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('uses a configured repo-qualified reference to make one fallback lookup', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-qualified-fallback-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + org: 'AgentWorkforce', + byLabel: { + factory: 'AgentWorkforce/factory', + workforce: 'AgentWorkforce/workforce', + }, + clonePaths: { + 'AgentWorkforce/factory': '/work/factory', + 'AgentWorkforce/workforce': '/work/workforce', + }, + }, + }) + const githubRead = fakeGithubConnectionRead(async (repo, number) => + repo === 'AgentWorkforce/factory' ? githubConnectionIssue('factory', number) : undefined, + ) + const output = buffer() + + const code = await runFleetCli(['triage', 'factory#222', '--config', configPath], { + fleet: new FakeFleetClient(), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), + stdout: output, + stderr: buffer(), + }) + + expect(code).toBe(0) + expect(JSON.parse(output.text())).toMatchObject({ + issueResolution: { source: 'github-api-fallback', repo: 'AgentWorkforce/factory' }, + }) + expect(githubRead.getIssue).toHaveBeenCalledTimes(1) + expect(githubRead.getIssue).toHaveBeenCalledWith('AgentWorkforce/factory', 222) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('rejects non-positive GitHub issue numbers with source context', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-invalid-number-')) try { diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index b9c7f6e..153f9bb 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -1694,11 +1694,18 @@ async function resolveIssueArg( } } - const number = Number(issueArg.replace(/^#/, '')) + const selector = parseGithubIssueSelector(issueArg, config) const projection = projectionStatus ? await projectionStatus() : projectionStatusFromMount(mount) - const fallback = await findGithubIssueThroughConnection(mount, number, config) + const unavailableReason = githubProjectionUnavailableReason(projection) + if (!unavailableReason) { + throw new Error( + `${githubIssueResolutionError(config, issueArg)}: found 0 matches in the healthy Relayfile projection; ` + + 'the GitHub API fallback was not used', + ) + } + const fallback = await findGithubIssueThroughConnection(mount, selector, config, issueArg) if (!fallback) { throw new Error(`${githubIssueResolutionError(config, issueArg)}: found 0 matches in the projection and GitHub API`) } @@ -1707,7 +1714,7 @@ async function resolveIssueArg( resolution: { source: 'github-api-fallback', repo: fallback.repo, - detail: 'Relayfile projection returned no match; resolved authoritatively through the workspace GitHub API connection.', + detail: `Relayfile projection could not answer (${unavailableReason}); resolved authoritatively through the GitHub API fallback.`, projection, }, } @@ -1715,11 +1722,9 @@ async function resolveIssueArg( async function findIssuePath(mount: MountClient, key: string, config: FactoryConfig): Promise { if (config.issueSource === 'github') { - const number = Number(key.replace(/^#/, '')) - if (!Number.isInteger(number) || number <= 0) { - throw new Error(`${githubIssueResolutionError(config, key)}: expected a positive issue number`) - } - const configuredRepos = configuredGithubIssueRepos(config) + const selector = parseGithubIssueSelector(key, config) + const number = selector.number + const configuredRepos = selector.repo ? [selector.repo] : configuredGithubIssueRepos(config) if (configuredRepos.length === 0 && hasConfiguredGithubIssueRoutes(config)) { throw new Error( `${githubIssueResolutionError(config, key)}: configured repository routes do not resolve to owner/repo; ` + @@ -1779,28 +1784,29 @@ async function findIssuePath(mount: MountClient, key: string, config: FactoryCon async function findGithubIssueThroughConnection( mount: MountClient, - number: number, + selector: GithubIssueSelector, config: FactoryConfig, + issueArg: string, ) { const github = mount.githubRead if (!github) { throw new Error( - `${githubIssueResolutionError(config, String(number))}: projection returned no match and the GitHub API fallback is unavailable`, + `${githubIssueResolutionError(config, issueArg)}: projection cannot answer and the GitHub API fallback is unavailable`, ) } - const repos = configuredGithubIssueRepos(config) + const repos = selector.repo ? [selector.repo] : configuredGithubIssueRepos(config) if (repos.length === 0) { throw new Error( - `${githubIssueResolutionError(config, String(number))}: projection returned no match and no configured owner/repo is available for the GitHub API fallback`, + `${githubIssueResolutionError(config, issueArg)}: projection cannot answer and no configured owner/repo is available for the GitHub API fallback`, ) } - const matches = (await Promise.all(repos.map((repo) => github.getIssue(repo, number)))) + const matches = (await Promise.all(repos.map((repo) => github.getIssue(repo, selector.number)))) .filter((issue): issue is NonNullable => Boolean(issue)) if (matches.length === 0) return undefined - if (!config.repos.default && matches.length > 1) { + if (!selector.repo && !config.repos.default && matches.length > 1) { const matchedRepos = matches.map((match) => match.repo).sort((left, right) => left.localeCompare(right)) throw new Error( - `${githubIssueResolutionError(config, String(number))}: GitHub API matches multiple repositories (${matchedRepos.join(', ')}); ` + + `${githubIssueResolutionError(config, issueArg)}: GitHub API matches multiple repositories (${matchedRepos.join(', ')}); ` + 'set repos.default or pass a repo-qualified argument', ) } @@ -1835,6 +1841,47 @@ function projectionStatusFromMount(mount: MountClient): IssueResolution['project } } +function githubProjectionUnavailableReason(projection: IssueResolution['projection']): string | undefined { + if (projection.localMountDegraded) { + return projection.localMountDegradedReason ?? 'local mount is degraded' + } + if (projection.eventListener && !['subscribed', 'polling'].includes(projection.eventListener.state)) { + return projection.eventListener.reason ?? `event listener is ${projection.eventListener.state}` + } + return undefined +} + +type GithubIssueSelector = { number: number; repo?: string } + +function parseGithubIssueSelector(key: string, config: FactoryConfig): GithubIssueSelector { + const qualified = key.match(/^([^#]+)#([1-9]\d*)$/u) + const bare = key.match(/^#?([1-9]\d*)$/u) + const number = Number(qualified?.[2] ?? bare?.[1]) + if (!Number.isSafeInteger(number) || number <= 0) { + throw new Error( + `${githubIssueResolutionError(config, key)}: expected a positive issue number or repo-qualified reference (repo#number)`, + ) + } + if (!qualified) return { number } + + const requested = qualified[1]! + const configured = configuredGithubIssueRepos(config) + const expanded = requested.includes('/') + ? requested + : config.repos.org + ? `${config.repos.org}/${requested}` + : Object.entries(config.repos.byLabel).find(([label]) => label.toLowerCase() === requested.toLowerCase())?.[1] + const repo = expanded + ? configured.find((candidate) => candidate.toLowerCase() === expanded.toLowerCase()) + : undefined + if (!repo) { + throw new Error( + `${githubIssueResolutionError(config, key)}: repository ${requested} is not one of the configured Factory routes`, + ) + } + return { number, repo } +} + function configuredGithubIssueRepos(config: FactoryConfig): string[] { const candidates = config.repos.default ? [config.repos.default] diff --git a/src/index.ts b/src/index.ts index 0dac27a..fe61f10 100644 --- a/src/index.ts +++ b/src/index.ts @@ -80,7 +80,7 @@ export { resolveFactoryWorkspace, } from './mount/relayfile-cloud-mount-client' export { RelayfileGithubConnectionWrite } from './mount/relayfile-github-connection-write' -export { RelayfileGithubConnectionRead } from './mount/relayfile-github-connection-read' +export { GithubApiIssueRead } from './mount/github-api-issue-read' export { ensureFactoryIntegrations, inspectFactoryIntegration, diff --git a/src/mount/github-api-issue-read.test.ts b/src/mount/github-api-issue-read.test.ts new file mode 100644 index 0000000..189dde7 --- /dev/null +++ b/src/mount/github-api-issue-read.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it, vi } from 'vitest' + +import { GithubApiIssueRead } from './github-api-issue-read' + +describe('GithubApiIssueRead', () => { + it('reads an issue directly through the GitHub REST API', async () => { + const request = vi.fn(async () => new Response(JSON.stringify({ + id: 222, + node_id: 'I_222', + number: 222, + title: '[factory] Restore dispatch', + body: 'Use the GitHub API as a fallback.', + state: 'open', + html_url: 'https://github.example/AgentWorkforce/factory/issues/222', + updated_at: '2026-08-08T12:00:00Z', + user: { login: 'factory-app' }, + labels: [{ name: 'factory' }, { name: 'factory-repo' }], + }), { status: 200 })) + const reader = new GithubApiIssueRead({ fetch: request }) + + await expect(reader.getIssue('AgentWorkforce/factory', 222)).resolves.toEqual({ + repo: 'AgentWorkforce/factory', + number: 222, + path: '/github/repos/AgentWorkforce__factory/issues/by-id/222.json', + content: expect.objectContaining({ + provider: 'github', + objectType: 'issue', + objectId: 'I_222', + payload: expect.objectContaining({ + number: 222, + title: '[factory] Restore dispatch', + state: 'open', + labels: [{ name: 'factory' }, { name: 'factory-repo' }], + repository: { name: 'factory', owner: { login: 'AgentWorkforce' } }, + }), + }), + }) + expect(request).toHaveBeenCalledWith( + 'https://api.github.com/repos/AgentWorkforce/factory/issues/222', + expect.objectContaining({ method: 'GET' }), + ) + }) + + it('returns undefined only for an authoritative 404', async () => { + const reader = new GithubApiIssueRead({ + fetch: vi.fn(async () => new Response('{}', { status: 404 })), + }) + + await expect(reader.getIssue('AgentWorkforce/factory', 999_999)).resolves.toBeUndefined() + }) + + it('surfaces API failures instead of manufacturing absence', async () => { + const reader = new GithubApiIssueRead({ + fetch: vi.fn(async () => new Response('{}', { status: 403 })), + }) + + await expect(reader.getIssue('AgentWorkforce/factory', 222)).rejects.toThrow( + 'GitHub API issue lookup failed (HTTP 403)', + ) + }) + + it('does not treat a pull request returned by the issues endpoint as an issue', async () => { + const reader = new GithubApiIssueRead({ + fetch: vi.fn(async () => new Response(JSON.stringify({ + id: 222, + number: 222, + title: '[factory] PR', + html_url: 'https://github.example/AgentWorkforce/factory/pull/222', + pull_request: {}, + }), { status: 200 })), + }) + + await expect(reader.getIssue('AgentWorkforce/factory', 222)).rejects.toThrow( + 'returned an incomplete issue record', + ) + }) +}) diff --git a/src/mount/github-api-issue-read.ts b/src/mount/github-api-issue-read.ts new file mode 100644 index 0000000..e3ead20 --- /dev/null +++ b/src/mount/github-api-issue-read.ts @@ -0,0 +1,111 @@ +import type { GithubConnectionIssue, GithubConnectionRead } from '../ports' + +const GITHUB_API_BASE_URL = 'https://api.github.com' + +type FetchLike = (input: string | URL | Request, init?: RequestInit) => Promise + +export interface GithubApiIssueReadConfig { + /** Internal request override for deterministic tests. */ + fetch?: FetchLike +} + +/** + * Provider-authoritative, read-only lookup against GitHub's REST API. + * + * Factory never receives or invokes a GitHub CLI credential here. GitHub + * mutations continue to use the Relayfile connection writeback path with the + * app author; this deliberately separate reader only recovers public issue + * facts when the preferred Relayfile projection cannot answer. + */ +export class GithubApiIssueRead implements GithubConnectionRead { + readonly #fetch: FetchLike + + constructor(config: GithubApiIssueReadConfig = {}) { + this.#fetch = config.fetch ?? fetch + } + + async getIssue(repo: string, number: number): Promise { + const { owner, name } = githubRepoParts(repo) + if (!Number.isSafeInteger(number) || number <= 0) { + throw new Error(`GitHub issue number must be a positive integer: ${number}`) + } + + const response = await this.#fetch( + `${GITHUB_API_BASE_URL}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/issues/${number}`, + { + method: 'GET', + headers: { + accept: 'application/vnd.github+json', + 'user-agent': '@agent-relay/factory', + }, + }, + ) + if (response.status === 404) return undefined + if (!response.ok) { + throw new Error(`GitHub API issue lookup failed (HTTP ${response.status})`) + } + + const issue = record(await response.json()) + if (!issue) { + throw new Error(`GitHub API issue lookup returned an incomplete issue record for ${repo}#${number}`) + } + const resolvedNumber = positiveInteger(issue.number) + const title = stringValue(issue.title) + const url = stringValue(issue.html_url) + if (resolvedNumber !== number || !title || !url || issue.pull_request !== undefined) { + throw new Error(`GitHub API issue lookup returned an incomplete issue record for ${repo}#${number}`) + } + + const labels = Array.isArray(issue.labels) ? issue.labels : [] + const author = stringValue(record(issue.user)?.login) + const path = githubIssuePath(owner, name, number) + return { + repo: `${owner}/${name}`, + number, + path, + content: { + provider: 'github', + objectType: 'issue', + objectId: stringValue(issue.node_id) ?? String(issue.id ?? `${owner}/${name}#${number}`), + payload: { + id: issue.id, + node_id: issue.node_id, + number, + title, + body: stringValue(issue.body) ?? '', + state: (stringValue(issue.state) ?? '').toLowerCase(), + url, + html_url: url, + updated_at: stringValue(issue.updated_at), + labels: labels + .map((label) => typeof label === 'string' ? { name: label } : { name: stringValue(record(label)?.name) }) + .filter((label): label is { name: string } => Boolean(label.name)), + ...(author ? { user: { login: author }, author: { login: author } } : {}), + repository: { name, owner: { login: owner } }, + }, + }, + } + } +} + +const githubRepoParts = (repo: string): { owner: string; name: string } => { + const [owner, name, ...extra] = repo.split('/') + if (!owner || !name || extra.length > 0) { + throw new Error(`GitHub repo must be owner/repo: ${repo}`) + } + return { owner, name } +} + +const githubIssuePath = (owner: string, repo: string, number: number): string => + `/github/repos/${encodeURIComponent(owner)}__${encodeURIComponent(repo)}/issues/by-id/${number}.json` + +const record = (value: unknown): Record | undefined => + value !== null && typeof value === 'object' && !Array.isArray(value) + ? value as Record + : undefined + +const stringValue = (value: unknown): string | undefined => + typeof value === 'string' && value.length > 0 ? value : undefined + +const positiveInteger = (value: unknown): number | undefined => + typeof value === 'number' && Number.isSafeInteger(value) && value > 0 ? value : undefined diff --git a/src/mount/relayfile-cloud-mount-client.test.ts b/src/mount/relayfile-cloud-mount-client.test.ts index 7d97914..4dcfda8 100644 --- a/src/mount/relayfile-cloud-mount-client.test.ts +++ b/src/mount/relayfile-cloud-mount-client.test.ts @@ -994,7 +994,6 @@ describe('RelayfileCloudMountClient', () => { // github uses the provider root `/github/**`; `/github/repos/**` is rejected // by RelayAuth's path-token validator and would fail the whole batch mint. expect(joinOptions.scopes).toContain('relayfile:fs:write:/github/**') - expect(joinOptions.scopes).toContain('integration:github:read') expect(joinOptions.scopes).not.toContain('relayfile:fs:write:/github/repos/**') expect(joinOptions.scopes).toContain('relayfile:fs:write:/factory/observability/**') expect(joinOptions.scopes).toContain('relayfile:fs:read:/slack/users/**') diff --git a/src/mount/relayfile-cloud-mount-client.ts b/src/mount/relayfile-cloud-mount-client.ts index 9bf5489..3a3d8a0 100644 --- a/src/mount/relayfile-cloud-mount-client.ts +++ b/src/mount/relayfile-cloud-mount-client.ts @@ -47,7 +47,7 @@ import { } from '../subscriptions' import type { ResourceSubscriptionsClient } from '../subscriptions' import { RelayfileGithubConnectionWrite } from './relayfile-github-connection-write' -import { RelayfileGithubConnectionRead } from './relayfile-github-connection-read' +import { GithubApiIssueRead } from './github-api-issue-read' import { ensureLocalMount as runLocalMountPreflight, type EnsureLocalMountOptions, @@ -73,7 +73,6 @@ export const FACTORY_RELAYFILE_SCOPES = [ // cleanly. Do NOT narrow this back to `/github/repos/**`. 'relayfile:fs:read:/github/**', 'relayfile:fs:write:/github/**', - 'integration:github:read', 'relayfile:fs:read:/slack/channels/**', 'relayfile:fs:write:/slack/channels/**', 'relayfile:fs:read:/slack/users/**', @@ -348,10 +347,7 @@ export class RelayfileCloudMountClient implements MountClient { resolveRegisteredWorkspaceMirror(workspaceIds)?.localDir) this.#isAllowedDraft = config.isAllowedDraft this.#isAllowedDelete = config.isAllowedDelete - const githubConnectionRequest = config.relayfileWorkspace?.requestJson?.bind(config.relayfileWorkspace) - this.githubRead = githubConnectionRequest - ? new RelayfileGithubConnectionRead({ workspace: { requestJson: githubConnectionRequest } }) - : undefined + this.githubRead = new GithubApiIssueRead() this.githubWrite = new RelayfileGithubConnectionWrite({ mount: this }) this.integrationConnections = relayfileIntegrationConnections( config.relayfileWorkspace, diff --git a/src/mount/relayfile-github-connection-read.test.ts b/src/mount/relayfile-github-connection-read.test.ts deleted file mode 100644 index 98b65c8..0000000 --- a/src/mount/relayfile-github-connection-read.test.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' - -import { RelayfileGithubConnectionRead } from './relayfile-github-connection-read' - -describe('RelayfileGithubConnectionRead', () => { - it('reads an issue through the Relayfile workspace SDK request surface', async () => { - const requestJson = vi.fn(async () => ({ - data: { - repository: { - issue: { - id: 'I_222', - number: 222, - title: '[factory] Restore dispatch', - body: 'Use the connected GitHub API as a fallback.', - state: 'OPEN', - url: 'https://github.example/AgentWorkforce/factory/issues/222', - updatedAt: '2026-08-08T12:00:00Z', - author: { login: 'factory-app' }, - labels: { nodes: [{ name: 'factory' }, { name: 'factory-repo' }] }, - }, - }, - }, - })) - const reader = new RelayfileGithubConnectionRead({ workspace: { requestJson } }) - - await expect(reader.getIssue('AgentWorkforce/factory', 222)).resolves.toEqual({ - repo: 'AgentWorkforce/factory', - number: 222, - path: '/github/repos/AgentWorkforce__factory/issues/by-id/222.json', - content: expect.objectContaining({ - provider: 'github', - objectType: 'issue', - objectId: 'I_222', - payload: expect.objectContaining({ - number: 222, - title: '[factory] Restore dispatch', - state: 'open', - labels: [{ name: 'factory' }, { name: 'factory-repo' }], - repository: { name: 'factory', owner: { login: 'AgentWorkforce' } }, - }), - }), - }) - expect(requestJson).toHaveBeenCalledWith(expect.objectContaining({ - operation: 'getGithubIssue', - method: 'POST', - path: 'api/v1/github/graphql', - body: expect.objectContaining({ - variables: { owner: 'AgentWorkforce', repo: 'factory', number: 222 }, - }), - })) - }) - - it('returns undefined only for an authoritative empty issue result', async () => { - const reader = new RelayfileGithubConnectionRead({ - workspace: { requestJson: vi.fn(async () => ({ data: { repository: { issue: null } } })) }, - }) - - await expect(reader.getIssue('AgentWorkforce/factory', 999_999)).resolves.toBeUndefined() - }) - - it('surfaces GraphQL failures instead of manufacturing absence', async () => { - const reader = new RelayfileGithubConnectionRead({ - workspace: { - requestJson: vi.fn(async () => ({ - data: { repository: { issue: null } }, - errors: [{ extensions: { type: 'FORBIDDEN' } }], - })), - }, - }) - - await expect(reader.getIssue('AgentWorkforce/factory', 222)).rejects.toThrow( - 'GitHub API issue lookup failed (FORBIDDEN)', - ) - }) -}) diff --git a/src/mount/relayfile-github-connection-read.ts b/src/mount/relayfile-github-connection-read.ts deleted file mode 100644 index 80fc698..0000000 --- a/src/mount/relayfile-github-connection-read.ts +++ /dev/null @@ -1,131 +0,0 @@ -import type { GithubConnectionIssue, GithubConnectionRead } from '../ports' - -const GITHUB_ISSUE_QUERY = ` - query FactoryIssue($owner: String!, $repo: String!, $number: Int!) { - repository(owner: $owner, name: $repo) { - issue(number: $number) { - id - number - title - body - state - url - updatedAt - author { login } - labels(first: 100) { nodes { name } } - } - } - } -` - -export interface RelayfileGithubConnectionRequest { - requestJson(options: { - operation: string - method: string - path: string - body?: unknown - timeoutMs?: number - }): Promise -} - -export interface RelayfileGithubConnectionReadConfig { - workspace: RelayfileGithubConnectionRequest -} - -/** - * Read-only GitHub issue lookup through the authenticated Relayfile workspace - * connection. WorkspaceHandle.requestJson supplies the Relayfile workspace - * token; Factory never receives or shells out with a provider credential. - */ -export class RelayfileGithubConnectionRead implements GithubConnectionRead { - readonly #workspace: RelayfileGithubConnectionRequest - - constructor(config: RelayfileGithubConnectionReadConfig) { - this.#workspace = config.workspace - } - - async getIssue(repo: string, number: number): Promise { - const { owner, name } = githubRepoParts(repo) - if (!Number.isSafeInteger(number) || number <= 0) { - throw new Error(`GitHub issue number must be a positive integer: ${number}`) - } - - const response = record(await this.#workspace.requestJson({ - operation: 'getGithubIssue', - method: 'POST', - path: 'api/v1/github/graphql', - body: { - query: GITHUB_ISSUE_QUERY, - variables: { owner, repo: name, number }, - }, - timeoutMs: 30_000, - })) - const errors = Array.isArray(response?.errors) ? response.errors : [] - if (errors.length > 0) { - const codes = errors - .map((error) => stringValue(record(record(error)?.extensions)?.type) ?? stringValue(record(record(error)?.extensions)?.code)) - .filter((code): code is string => Boolean(code)) - throw new Error(`GitHub API issue lookup failed${codes.length > 0 ? ` (${[...new Set(codes)].join(', ')})` : ''}`) - } - - const issue = record(record(record(response?.data)?.repository)?.issue) - if (!issue) return undefined - const resolvedNumber = positiveInteger(issue.number) - const title = stringValue(issue.title) - const url = stringValue(issue.url) - if (resolvedNumber !== number || !title || !url) { - throw new Error(`GitHub API issue lookup returned an incomplete record for ${repo}#${number}`) - } - - const labels = record(issue.labels)?.nodes - const author = stringValue(record(issue.author)?.login) - const path = githubIssuePath(owner, name, number) - return { - repo: `${owner}/${name}`, - number, - path, - content: { - provider: 'github', - objectType: 'issue', - objectId: stringValue(issue.id) ?? `${owner}/${name}#${number}`, - payload: { - id: stringValue(issue.id), - number, - title, - body: stringValue(issue.body) ?? '', - state: (stringValue(issue.state) ?? '').toLowerCase(), - url, - html_url: url, - updated_at: stringValue(issue.updatedAt), - labels: Array.isArray(labels) - ? labels.map((label) => ({ name: stringValue(record(label)?.name) })).filter((label) => Boolean(label.name)) - : [], - ...(author ? { user: { login: author }, author: { login: author } } : {}), - repository: { name, owner: { login: owner } }, - }, - }, - } - } -} - -const githubRepoParts = (repo: string): { owner: string; name: string } => { - const [owner, name, ...extra] = repo.split('/') - if (!owner || !name || extra.length > 0) { - throw new Error(`GitHub repo must be owner/repo: ${repo}`) - } - return { owner, name } -} - -const githubIssuePath = (owner: string, repo: string, number: number): string => - `/github/repos/${encodeURIComponent(owner)}__${encodeURIComponent(repo)}/issues/by-id/${number}.json` - -const record = (value: unknown): Record | undefined => - value !== null && typeof value === 'object' && !Array.isArray(value) - ? value as Record - : undefined - -const stringValue = (value: unknown): string | undefined => - typeof value === 'string' && value.length > 0 ? value : undefined - -const positiveInteger = (value: unknown): number | undefined => - typeof value === 'number' && Number.isSafeInteger(value) && value > 0 ? value : undefined diff --git a/src/ports/mount.ts b/src/ports/mount.ts index f24e7d0..8193f79 100644 --- a/src/ports/mount.ts +++ b/src/ports/mount.ts @@ -72,10 +72,10 @@ export interface GithubPublishPullRequestResult { } /** - * Provider-authoritative GitHub issue returned through the workspace's - * connected GitHub integration. `content` intentionally uses the same - * provider record shape as the Relayfile projection so the existing parser - * and safety gates stay authoritative. + * Provider-authoritative GitHub issue returned by the direct API fallback. + * `content` intentionally uses the same provider record shape as the + * Relayfile projection so the existing parser and safety gates stay + * authoritative. */ export interface GithubConnectionIssue { repo: string From 733e63f60a2b71b888e9427cb0a9b6c2030c21a0 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 8 Aug 2026 23:19:59 +0200 Subject: [PATCH 05/19] docs: record fallback red checks --- .agent-notes/factory-dispatch-unblock.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md index 17c5588..146c22b 100644 --- a/.agent-notes/factory-dispatch-unblock.md +++ b/.agent-notes/factory-dispatch-unblock.md @@ -33,3 +33,11 @@ - Fallback eligibility now uses PR #220's health facts: a degraded local mount or listener state other than `subscribed`/`polling` means the projection cannot answer. A healthy projection miss fails without calling GitHub. - Added configured `repo#number` and `owner/repo#number` selectors so a targeted fallback performs one authoritative lookup and avoids ambiguous org-wide probes. - Live `factory triage factory#222` reached the empty projection, reported the listener `unknown`, resolved via `github-api-fallback`, and routed to `AgentWorkforce/factory`. It emitted its successful decision; the existing one-shot shutdown path remained open until SIGINT, then returned 0. The shutdown hang is separate from issue resolution. + +## 2026-08-08 verification + +- Live CLI through `runFleetCli` with a no-op reporter: `factory#222` exit 0, source `github-api-fallback`, projection `no-match`, routed only to `AgentWorkforce/factory`; `factory#999999` exit 1, no decision emitted. +- Focused build and three-suite check: exit 0; 157 tests passed. +- Projection-preference mutation check: forced projection hits past the preferred branch; targeted test exited 1. Restored source; same check exited 0. +- Safety mutation check: made the intentionally unsafe fallback fixture satisfy the existing GitHub label/title markers; targeted rejection test exited 1. Restored unsafe fixture; same check exited 0. +- Default-timeout full suite: exit 1; 1,461 passed and six timing-sensitive tests failed. Rerun of the three affected non-orchestrator files with a 20-second ceiling exited 0 (65 tests). One pre-existing heartbeat timing assertion still exits 1 even in isolation (`600 < 500`); the other affected orchestrator test passed in isolation. From ef88320c6f43f06132b70b3946f9b29f885b49c0 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 8 Aug 2026 23:24:06 +0200 Subject: [PATCH 06/19] fix: scope provider fallback to stale targeted issues --- .agent-notes/factory-dispatch-unblock.md | 2 + src/cli/fleet.test.ts | 49 +++++++++++++++++++++++ src/cli/fleet.ts | 10 +++++ src/mount/github-api-issue-read.ts | 1 + src/mount/relayfile-cloud-mount-client.ts | 7 ---- src/orchestrator/factory.ts | 8 +++- src/triage/schema.ts | 5 +++ src/types.ts | 5 +++ 8 files changed, 79 insertions(+), 8 deletions(-) diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md index 146c22b..b32f5b7 100644 --- a/.agent-notes/factory-dispatch-unblock.md +++ b/.agent-notes/factory-dispatch-unblock.md @@ -41,3 +41,5 @@ - Projection-preference mutation check: forced projection hits past the preferred branch; targeted test exited 1. Restored source; same check exited 0. - Safety mutation check: made the intentionally unsafe fallback fixture satisfy the existing GitHub label/title markers; targeted rejection test exited 1. Restored unsafe fixture; same check exited 0. - Default-timeout full suite: exit 1; 1,461 passed and six timing-sensitive tests failed. Rerun of the three affected non-orchestrator files with a 20-second ceiling exited 0 (65 tests). One pre-existing heartbeat timing assertion still exits 1 even in isolation (`600 < 500`); the other affected orchestrator test passed in isolation. +- Manual diff review caught an over-broad orchestrator read fallback. It is now scoped to issue identities explicitly resolved by the targeted fallback, so ordinary healthy ingestion misses never call GitHub. The existing integration connection status is also included in the projection-health record and can independently prove a connected-but-not-ready projection cannot answer. +- Final focused build/check after that correction: exit 0; 158 tests passed. Live `factory#222` remained exit 0 through the fallback and routed only to Factory. diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 4fda930..55746fe 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -796,6 +796,55 @@ describe('fleet CLI runtime', () => { } }) + it('uses connected-not-ready status as the reported reason an empty projection cannot answer', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-integration-github-empty-fallback-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }) + const integrations = fakeIntegrationConnections(async () => ({ + ready: false, + state: 'degraded', + initialSyncState: 'complete', + })) + const githubRead = fakeGithubConnectionRead(async (_repo, number) => githubConnectionIssue('pear', number)) + const mount = Object.assign( + mountWithIntegrationConnections({}, integrations), + { + githubRead, + getLocalMountHealth: () => ({ degraded: false }), + }, + ) + const output = buffer() + + const code = await runFleetCli(['triage', '222', '--config', configPath], { + fleet: new FakeFleetClient(), + mount, + stdout: output, + stderr: buffer(), + }) + + expect(code).toBe(0) + expect(JSON.parse(output.text())).toMatchObject({ + issueResolution: { + source: 'github-api-fallback', + detail: expect.stringContaining('GitHub projection connection is not ready (degraded, complete)'), + projection: { + githubConnection: { ready: false, state: 'degraded', initialSyncState: 'complete' }, + }, + }, + }) + expect(githubRead.getIssue).toHaveBeenCalledWith('AgentWorkforce/pear', 222) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('does not prompt or connect when a missing integration is checked without a TTY', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-integration-headless-')) try { diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index 153f9bb..b4313dd 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -1824,11 +1824,15 @@ async function issueProjectionStatus( config.loop.heartbeatPath, config.loop.heartbeatStaleMs, ) + const githubConnection = mount.integrationConnections + ? await mount.integrationConnections.getStatus('github') + : undefined return { outcome: 'no-match', ...(status.localMountDegraded !== undefined ? { localMountDegraded: status.localMountDegraded } : {}), ...(status.localMountDegradedReason ? { localMountDegradedReason: status.localMountDegradedReason } : {}), ...(status.eventListener ? { eventListener: status.eventListener } : {}), + ...(githubConnection ? { githubConnection } : {}), } } @@ -1842,6 +1846,12 @@ function projectionStatusFromMount(mount: MountClient): IssueResolution['project } function githubProjectionUnavailableReason(projection: IssueResolution['projection']): string | undefined { + if (projection.githubConnection && !projection.githubConnection.ready) { + const detail = [projection.githubConnection.state, projection.githubConnection.initialSyncState] + .filter((value): value is string => Boolean(value)) + .join(', ') + return `GitHub projection connection is not ready${detail ? ` (${detail})` : ''}` + } if (projection.localMountDegraded) { return projection.localMountDegradedReason ?? 'local mount is degraded' } diff --git a/src/mount/github-api-issue-read.ts b/src/mount/github-api-issue-read.ts index e3ead20..714fca3 100644 --- a/src/mount/github-api-issue-read.ts +++ b/src/mount/github-api-issue-read.ts @@ -34,6 +34,7 @@ export class GithubApiIssueRead implements GithubConnectionRead { `${GITHUB_API_BASE_URL}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/issues/${number}`, { method: 'GET', + signal: AbortSignal.timeout(30_000), headers: { accept: 'application/vnd.github+json', 'user-agent': '@agent-relay/factory', diff --git a/src/mount/relayfile-cloud-mount-client.ts b/src/mount/relayfile-cloud-mount-client.ts index 3a3d8a0..2e04912 100644 --- a/src/mount/relayfile-cloud-mount-client.ts +++ b/src/mount/relayfile-cloud-mount-client.ts @@ -127,13 +127,6 @@ export interface RelayfileWorkspaceHandleLike { info: { relayfileUrl: string } client(): RelayFileClientLike getToken(): Promise | string - requestJson?(options: { - operation: string - method: string - path: string - body?: unknown - timeoutMs?: number - }): Promise getConnectionStatus?(provider: FactoryIntegrationProvider, connectionId: string): Promise<{ ready: boolean state?: string diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index ff7defc..fbbcee1 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -412,6 +412,7 @@ export class FactoryLoop implements Factory { readonly #githubIssueAuthors = new Map() readonly #githubIssueAuthorLookups = new Map>() readonly #githubIssuePreferredPaths = new Map() + readonly #githubApiFallbackIssues = new Set() #githubIssuePathIndexReady = false readonly #slackReporterUserIds = new Map() readonly #slackReporterUserIdLookups = new Map>() @@ -2426,6 +2427,10 @@ export class FactoryLoop implements Factory { throw error } + if (decision.issueResolution?.source === 'github-api-fallback') { + const parts = githubIssuePathParts(decision.issue.path) ?? githubIssueDirectoryPathParts(decision.issue.path) + if (parts) this.#githubApiFallbackIssues.add(githubIssueIdentity(parts.owner, parts.repo, parts.number)) + } const liveIssue = await this.#readIssue(decision.issue.path) if (!liveIssue || !isInFactoryScope(liveIssue, this.#config.safety)) { const error = new Error(`Refusing to dispatch ${decision.issue.key}: not factory-e2e scope`) @@ -4672,7 +4677,8 @@ export class FactoryLoop implements Factory { } catch (error) { if (isMissingIssueFileError(error)) { const parts = githubIssuePathParts(path) ?? githubIssueDirectoryPathParts(path) - if (parts && this.#mount.githubRead) { + const identity = parts ? githubIssueIdentity(parts.owner, parts.repo, parts.number) : undefined + if (parts && identity && this.#githubApiFallbackIssues.has(identity) && this.#mount.githubRead) { const fallback = await this.#mount.githubRead.getIssue(`${parts.owner}/${parts.repo}`, parts.number) if (fallback) { const githubIssue = parseGithubIssue(fallback.path, fallback.content) diff --git a/src/triage/schema.ts b/src/triage/schema.ts index f6d5c3a..887aaca 100644 --- a/src/triage/schema.ts +++ b/src/triage/schema.ts @@ -35,6 +35,11 @@ export const TriageDecisionSchema = z.object({ state: z.enum(['starting', 'subscribed', 'polling', 'not-listening', 'unknown']), reason: z.string().optional(), }).optional(), + githubConnection: z.object({ + ready: z.boolean(), + state: z.string().optional(), + initialSyncState: z.string().optional(), + }).optional(), }), }).optional(), routes: z.array(z.object({ diff --git a/src/types.ts b/src/types.ts index 34a414f..fc6be8f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -265,6 +265,11 @@ export interface IssueResolution { localMountDegraded?: boolean localMountDegradedReason?: string eventListener?: FactoryEventListenerStatus + githubConnection?: { + ready: boolean + state?: string + initialSyncState?: string + } } } From cff7303c6afc77912ff8cff5a32b35a0b1c4723c Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 8 Aug 2026 23:29:03 +0200 Subject: [PATCH 07/19] fix: ignore pull requests during issue fallback --- .agent-notes/factory-dispatch-unblock.md | 2 ++ src/mount/github-api-issue-read.test.ts | 6 ++---- src/mount/github-api-issue-read.ts | 6 +++++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md index b32f5b7..1934335 100644 --- a/.agent-notes/factory-dispatch-unblock.md +++ b/.agent-notes/factory-dispatch-unblock.md @@ -43,3 +43,5 @@ - Default-timeout full suite: exit 1; 1,461 passed and six timing-sensitive tests failed. Rerun of the three affected non-orchestrator files with a 20-second ceiling exited 0 (65 tests). One pre-existing heartbeat timing assertion still exits 1 even in isolation (`600 < 500`); the other affected orchestrator test passed in isolation. - Manual diff review caught an over-broad orchestrator read fallback. It is now scoped to issue identities explicitly resolved by the targeted fallback, so ordinary healthy ingestion misses never call GitHub. The existing integration connection status is also included in the projection-health record and can independently prove a connected-but-not-ready projection cannot answer. - Final focused build/check after that correction: exit 0; 158 tests passed. Live `factory#222` remained exit 0 through the fallback and routed only to Factory. +- A bare-number live check exposed GitHub's REST issues endpoint returning PRs. Those are now authoritative issue misses instead of malformed records. After the correction, live `factory triage 222` exited 0 and selected/routed `AgentWorkforce/factory` through the fallback. +- Live safety and dispatch checks: dry-run dispatch of an existing issue missing both Factory markers exited 1 with no dispatch; dry-run dispatch of `factory#222` exited 0 and carried `github-api-fallback` in both the result and its dispatch comment. diff --git a/src/mount/github-api-issue-read.test.ts b/src/mount/github-api-issue-read.test.ts index 189dde7..9789301 100644 --- a/src/mount/github-api-issue-read.test.ts +++ b/src/mount/github-api-issue-read.test.ts @@ -59,7 +59,7 @@ describe('GithubApiIssueRead', () => { ) }) - it('does not treat a pull request returned by the issues endpoint as an issue', async () => { + it('treats a pull request returned by the issues endpoint as an authoritative issue miss', async () => { const reader = new GithubApiIssueRead({ fetch: vi.fn(async () => new Response(JSON.stringify({ id: 222, @@ -70,8 +70,6 @@ describe('GithubApiIssueRead', () => { }), { status: 200 })), }) - await expect(reader.getIssue('AgentWorkforce/factory', 222)).rejects.toThrow( - 'returned an incomplete issue record', - ) + await expect(reader.getIssue('AgentWorkforce/factory', 222)).resolves.toBeUndefined() }) }) diff --git a/src/mount/github-api-issue-read.ts b/src/mount/github-api-issue-read.ts index 714fca3..8d6625a 100644 --- a/src/mount/github-api-issue-read.ts +++ b/src/mount/github-api-issue-read.ts @@ -50,10 +50,14 @@ export class GithubApiIssueRead implements GithubConnectionRead { if (!issue) { throw new Error(`GitHub API issue lookup returned an incomplete issue record for ${repo}#${number}`) } + // GitHub's REST "issues" endpoint also returns pull requests. For Factory + // issue dispatch a PR occupying the same repository number is an + // authoritative non-match, not a malformed issue. + if (issue.pull_request !== undefined) return undefined const resolvedNumber = positiveInteger(issue.number) const title = stringValue(issue.title) const url = stringValue(issue.html_url) - if (resolvedNumber !== number || !title || !url || issue.pull_request !== undefined) { + if (resolvedNumber !== number || !title || !url) { throw new Error(`GitHub API issue lookup returned an incomplete issue record for ${repo}#${number}`) } From e350588ef7223a3d4fc1669b7f09d987ea3dd5f4 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 8 Aug 2026 23:36:48 +0200 Subject: [PATCH 08/19] docs: record Relayfile PR permission blocker --- .agent-notes/factory-dispatch-unblock.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md index 1934335..111819c 100644 --- a/.agent-notes/factory-dispatch-unblock.md +++ b/.agent-notes/factory-dispatch-unblock.md @@ -45,3 +45,10 @@ - Final focused build/check after that correction: exit 0; 158 tests passed. Live `factory#222` remained exit 0 through the fallback and routed only to Factory. - A bare-number live check exposed GitHub's REST issues endpoint returning PRs. Those are now authoritative issue misses instead of malformed records. After the correction, live `factory triage 222` exited 0 and selected/routed `AgentWorkforce/factory` through the fallback. - Live safety and dispatch checks: dry-run dispatch of an existing issue missing both Factory markers exited 1 with no dispatch; dry-run dispatch of `factory#222` exited 0 and carried `github-api-fallback` in both the result and its dispatch comment. + +## 2026-08-08 PR publication blocker + +- The implementation branch is pushed to `origin/agent/factory-dispatch-api-fallback`; no default branch was pushed or merged. +- PR publication was attempted only through `RelayfileGithubConnectionWrite`, whose draft explicitly requests `author: 'app'`. No `gh` command or alternate GitHub identity was used. +- The guarded Relayfile draft writes were accepted, but all three durable operations failed before a provider attempt (`attemptCount: 0`) with GitHub HTTP 403 resource-access semantics. A direct readback of the minted Relayfile token confirmed that its normalized grants include scoped write access to `/github/**`, so the remaining denial is at the GitHub App repository/permission boundary. +- A read-only GitHub API check confirmed that zero open pull requests exist for the pushed head branch. Repeating the same write cannot satisfy the gate until the app permission/install state changes. From 1cef8c377f46a0c6e9f27dc68358b71743fc5772 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 13:06:13 +0200 Subject: [PATCH 09/19] fix: validate qualified GitHub issue selectors against every configured route configuredGithubIssueRepos collapsed to repos.default whenever it was set, so parseGithubIssueSelector rejected a valid repo-qualified selector (e.g. cloud#222) reachable only through byLabel, byProject, or keywordRules. Bare-number resolution intentionally stays default-only; qualified selectors now validate against the full configured route set via a new allConfiguredGithubIssueRepos. --- src/cli/fleet.test.ts | 42 ++++++++++++++++++++++++++++++++++++++++++ src/cli/fleet.ts | 37 ++++++++++++++++++++++++++++--------- 2 files changed, 70 insertions(+), 9 deletions(-) diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 55746fe..18ef2e9 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -1964,6 +1964,48 @@ describe('fleet CLI runtime', () => { } }) + it('validates a qualified selector against every configured route, not just repos.default', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-qualified-non-default-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + default: 'AgentWorkforce/factory', + byLabel: { + factory: 'AgentWorkforce/factory', + cloud: 'AgentWorkforce/cloud', + }, + clonePaths: { + 'AgentWorkforce/factory': '/work/factory', + 'AgentWorkforce/cloud': '/work/cloud', + }, + }, + }) + const githubRead = fakeGithubConnectionRead(async (repo, number) => + repo === 'AgentWorkforce/cloud' ? githubConnectionIssue('cloud', number) : undefined, + ) + const output = buffer() + + const code = await runFleetCli(['triage', 'cloud#222', '--config', configPath], { + fleet: new FakeFleetClient(), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), + stdout: output, + stderr: buffer(), + }) + + expect(code).toBe(0) + expect(JSON.parse(output.text())).toMatchObject({ + issueResolution: { source: 'github-api-fallback', repo: 'AgentWorkforce/cloud' }, + }) + expect(githubRead.getIssue).toHaveBeenCalledWith('AgentWorkforce/cloud', 222) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('rejects non-positive GitHub issue numbers with source context', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-invalid-number-')) try { diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index b4313dd..c9bc206 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -1875,7 +1875,7 @@ function parseGithubIssueSelector(key: string, config: FactoryConfig): GithubIss if (!qualified) return { number } const requested = qualified[1]! - const configured = configuredGithubIssueRepos(config) + const configured = allConfiguredGithubIssueRepos(config) const expanded = requested.includes('/') ? requested : config.repos.org @@ -1892,14 +1892,7 @@ function parseGithubIssueSelector(key: string, config: FactoryConfig): GithubIss return { number, repo } } -function configuredGithubIssueRepos(config: FactoryConfig): string[] { - const candidates = config.repos.default - ? [config.repos.default] - : [ - ...Object.values(config.repos.byLabel), - ...Object.values(config.repos.byProject), - ...config.repos.keywordRules.map((rule) => rule.repo), - ] +function resolveGithubIssueRepoCandidates(config: FactoryConfig, candidates: string[]): string[] { const repos = new Map() const routedRepos = [ ...Object.values(config.repos.byLabel), @@ -1924,6 +1917,32 @@ function configuredGithubIssueRepos(config: FactoryConfig): string[] { return [...repos.values()] } +/** Bare-number resolution stays default-only: when repos.default is set, a + * bare issue number resolves against that single repo, not every route. */ +function configuredGithubIssueRepos(config: FactoryConfig): string[] { + const candidates = config.repos.default + ? [config.repos.default] + : [ + ...Object.values(config.repos.byLabel), + ...Object.values(config.repos.byProject), + ...config.repos.keywordRules.map((rule) => rule.repo), + ] + return resolveGithubIssueRepoCandidates(config, candidates) +} + +/** Qualified `repo#number` selectors must validate against every configured + * route, not just repos.default — a route reachable only through byLabel, + * byProject, or keywordRules is still a valid dispatch target. */ +function allConfiguredGithubIssueRepos(config: FactoryConfig): string[] { + const candidates = [ + ...(config.repos.default ? [config.repos.default] : []), + ...Object.values(config.repos.byLabel), + ...Object.values(config.repos.byProject), + ...config.repos.keywordRules.map((rule) => rule.repo), + ] + return resolveGithubIssueRepoCandidates(config, candidates) +} + function hasConfiguredGithubIssueRoutes(config: FactoryConfig): boolean { return Boolean( config.repos.default || From 3eb01ca42834eb3202a6e60a14921bd91f5de384 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 13:18:13 +0200 Subject: [PATCH 10/19] fix: stop treating unauthenticated 404s as authoritative GitHub issue misses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GithubApiIssueRead made every lookup unauthenticated, then treated a 404 as a confirmed absence. GitHub returns the same 404 for "issue does not exist" and "repository is private, existence hidden from an unauthenticated caller" — so a real issue in a private repo was misreported as gone, and the reader's own doc comment ("provider- authoritative", "returns undefined only when GitHub authoritatively reports no issue") was already false for anything it couldn't prove public. GithubConnectionRead.getIssue now returns a three-outcome GithubIssueLookup (found / not-found / indeterminate) instead of GithubConnectionIssue | undefined. The reader confirms repository visibility with one unauthenticated, cached repo-level GET before trusting an issue-level 404 as not-found; a 404 against a repo it cannot confirm public, or a rate-limited/ambiguous probe response, degrades to indeterminate rather than throwing or manufacturing absence. Both real consumers of the port are updated to stop collapsing indeterminate into absence: - orchestrator/factory.ts #readGithubIssue no longer folds an indeterminate result into the phantomSkipped ("confirmed gone") counter; it now counts and warn-logs it separately. - cli/fleet.ts findGithubIssueThroughConnection throws a distinct "could not determine" error instead of a false "found 0 matches", and — the sharper bug — refuses to dispatch to a single found match when another configured repo in the same org-wide probe came back indeterminate, since a same-numbered issue could exist there too and silently picking the one repo that answered would misroute dispatch. --- src/cli/fleet.test.ts | 103 ++++++++++++-- src/cli/fleet.ts | 45 ++++-- src/index.ts | 1 + src/mount/github-api-issue-read.test.ts | 176 ++++++++++++++++++------ src/mount/github-api-issue-read.ts | 131 ++++++++++++++---- src/orchestrator/factory.ts | 25 +++- src/ports/index.ts | 1 + src/ports/mount.ts | 17 ++- 8 files changed, 401 insertions(+), 98 deletions(-) diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 18ef2e9..5c7e46c 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -17,7 +17,7 @@ import type { import { stateResolutionFromIds } from '../index' import { FileStateStore } from '../state/file-state-store' import { FakeFleetClient, FakeMountClient } from '../testing' -import type { GithubConnectionRead, GithubConnectionWrite, LocalMountOptions, SpawnInput, SpawnResult } from '../ports' +import type { GithubConnectionRead, GithubConnectionWrite, GithubIssueLookup, LocalMountOptions, SpawnInput, SpawnResult } from '../ports' import type { HarnessDriverClientLike } from '../fleet/internal-fleet-client' import { ensureLocalMount as runLocalMountPreflight } from '../mount/local-mount-preflight' import { formatLogArgs, installFactoryStopSignalHandlers, parseFleetCommand, parseGlobalOptions, resolveBrokerConnectionPath, runFleetCli } from './fleet' @@ -128,6 +128,15 @@ const githubConnectionIssue = ( content, }) +const githubIssueFound = (repo: string, number: number, content?: unknown): GithubIssueLookup => + ({ outcome: 'found', issue: githubConnectionIssue(repo, number, content) }) + +const githubIssueNotFound = (): GithubIssueLookup => ({ outcome: 'not-found' }) + +const githubIssueIndeterminate = ( + reason = 'repository not visible without authentication', +): GithubIssueLookup => ({ outcome: 'indeterminate', reason }) + const fakeGithubConnectionRead = ( resolveIssue: (repo: string, number: number) => ReturnType, ): GithubConnectionRead => ({ getIssue: vi.fn(resolveIssue) }) @@ -812,7 +821,7 @@ describe('fleet CLI runtime', () => { state: 'degraded', initialSyncState: 'complete', })) - const githubRead = fakeGithubConnectionRead(async (_repo, number) => githubConnectionIssue('pear', number)) + const githubRead = fakeGithubConnectionRead(async (_repo, number) => githubIssueFound('pear', number)) const mount = Object.assign( mountWithIntegrationConnections({}, integrations), { @@ -1676,8 +1685,8 @@ describe('fleet CLI runtime', () => { }) const githubRead = fakeGithubConnectionRead(async (repo, number) => repo === 'AgentWorkforce/pear' && number === 222 - ? githubConnectionIssue('pear', number) - : undefined, + ? githubIssueFound('pear', number) + : githubIssueNotFound(), ) const mount = Object.assign(new FakeMountClient(), { githubRead, @@ -1769,7 +1778,7 @@ describe('fleet CLI runtime', () => { default: 'AgentWorkforce/pear', }, }) - const githubRead = fakeGithubConnectionRead(async () => undefined) + const githubRead = fakeGithubConnectionRead(async () => githubIssueNotFound()) const errors = buffer() const code = await runFleetCli(['triage', '999999', '--config', configPath], { @@ -1790,6 +1799,80 @@ describe('fleet CLI runtime', () => { } }) + it('reports that GitHub could not determine existence instead of a confident 0 matches', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-api-indeterminate-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }) + const githubRead = fakeGithubConnectionRead(async () => githubIssueIndeterminate()) + const errors = buffer() + + const code = await runFleetCli(['triage', '222', '--config', configPath], { + fleet: new FakeFleetClient(), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), + stdout: buffer(), + stderr: errors, + }) + + expect(code).toBe(1) + expect(errors.text()).toContain('could not determine whether the issue exists') + expect(errors.text()).not.toContain('found 0 matches') + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('refuses to dispatch to a found match when another configured repo could not be checked (no silent misroute)', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-api-unconfirmed-unique-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + byLabel: { + factory: 'AgentWorkforce/factory', + cloud: 'AgentWorkforce/cloud', + }, + clonePaths: { + 'AgentWorkforce/factory': '/work/factory', + 'AgentWorkforce/cloud': '/work/cloud', + }, + }, + }) + // A public repo (factory) answers with a match; a repo it cannot see + // (cloud) is indeterminate. A same-numbered issue could exist there + // too — dispatching to the found match alone would be a silent misroute. + const githubRead = fakeGithubConnectionRead(async (repo, number) => + repo === 'AgentWorkforce/factory' ? githubIssueFound('factory', number) : githubIssueIndeterminate(), + ) + const errors = buffer() + + const code = await runFleetCli(['triage', '222', '--config', configPath], { + fleet: new FakeFleetClient(), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), + stdout: buffer(), + stderr: errors, + }) + + expect(code).toBe(1) + expect(errors.text()).toContain('could not confirm it is unique') + expect(errors.text()).toContain('AgentWorkforce/factory') + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('re-reads an API fallback issue during dry-run dispatch and records the source', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-api-dispatch-')) try { @@ -1811,7 +1894,7 @@ describe('fleet CLI runtime', () => { '- Run the focused CLI regression checks.', ].join('\n') const githubRead = fakeGithubConnectionRead(async (_repo, number) => - githubConnectionIssue('pear', number, content), + githubIssueFound('pear', number, content), ) const output = buffer() @@ -1853,7 +1936,7 @@ describe('fleet CLI runtime', () => { unsafe.payload.labels = [{ name: 'pear' }] unsafe.payload.title = 'Missing both configured safety markers' const githubRead = fakeGithubConnectionRead(async (_repo, number) => - githubConnectionIssue('pear', number, unsafe), + githubIssueFound('pear', number, unsafe), ) const errors = buffer() const fleet = new FakeFleetClient() @@ -1889,7 +1972,7 @@ describe('fleet CLI runtime', () => { default: 'AgentWorkforce/pear', }, }) - const githubRead = fakeGithubConnectionRead(async () => githubConnectionIssue('pear', 222)) + const githubRead = fakeGithubConnectionRead(async () => githubIssueFound('pear', 222)) const errors = buffer() const now = Date.now() await writeFile(heartbeatPath, JSON.stringify({ @@ -1939,7 +2022,7 @@ describe('fleet CLI runtime', () => { }, }) const githubRead = fakeGithubConnectionRead(async (repo, number) => - repo === 'AgentWorkforce/factory' ? githubConnectionIssue('factory', number) : undefined, + repo === 'AgentWorkforce/factory' ? githubIssueFound('factory', number) : githubIssueNotFound(), ) const output = buffer() @@ -1982,7 +2065,7 @@ describe('fleet CLI runtime', () => { }, }) const githubRead = fakeGithubConnectionRead(async (repo, number) => - repo === 'AgentWorkforce/cloud' ? githubConnectionIssue('cloud', number) : undefined, + repo === 'AgentWorkforce/cloud' ? githubIssueFound('cloud', number) : githubIssueNotFound(), ) const output = buffer() diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index c9bc206..46707c3 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -1800,17 +1800,42 @@ async function findGithubIssueThroughConnection( `${githubIssueResolutionError(config, issueArg)}: projection cannot answer and no configured owner/repo is available for the GitHub API fallback`, ) } - const matches = (await Promise.all(repos.map((repo) => github.getIssue(repo, selector.number)))) - .filter((issue): issue is NonNullable => Boolean(issue)) - if (matches.length === 0) return undefined - if (!selector.repo && !config.repos.default && matches.length > 1) { - const matchedRepos = matches.map((match) => match.repo).sort((left, right) => left.localeCompare(right)) - throw new Error( - `${githubIssueResolutionError(config, issueArg)}: GitHub API matches multiple repositories (${matchedRepos.join(', ')}); ` + - 'set repos.default or pass a repo-qualified argument', - ) + const lookups = await Promise.all(repos.map((repo) => github.getIssue(repo, selector.number))) + const found = lookups.filter((lookup): lookup is Extract => + lookup.outcome === 'found', + ) + const indeterminate = lookups.some((lookup) => lookup.outcome === 'indeterminate') + + if (found.length === 0) { + if (indeterminate) { + throw new Error( + `${githubIssueResolutionError(config, issueArg)}: the projection could not answer and the GitHub API fallback ` + + 'could not determine whether the issue exists (one or more configured repositories are not visible without authentication)', + ) + } + return undefined } - return matches[0] + if (!selector.repo && !config.repos.default) { + if (found.length > 1) { + const matchedRepos = found.map((match) => match.issue.repo).sort((left, right) => left.localeCompare(right)) + throw new Error( + `${githubIssueResolutionError(config, issueArg)}: GitHub API matches multiple repositories (${matchedRepos.join(', ')}); ` + + 'set repos.default or pass a repo-qualified argument', + ) + } + if (indeterminate) { + // A single confirmed match is not the same as a unique one: at least + // one other configured repository could not be checked, so a same- + // numbered issue could exist there too. Refuse rather than silently + // dispatch to whichever repo happened to answer. + throw new Error( + `${githubIssueResolutionError(config, issueArg)}: GitHub API found a match in ${found[0]!.issue.repo} but could not ` + + 'confirm it is unique because one or more other configured repositories could not be checked without authentication; ' + + 'set repos.default or pass a repo-qualified argument', + ) + } + } + return found[0]!.issue } async function issueProjectionStatus( diff --git a/src/index.ts b/src/index.ts index fe61f10..17ea22a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -298,6 +298,7 @@ export type { GithubConnectionWrite, GithubConnectionIssue, GithubConnectionRead, + GithubIssueLookup, FactoryIntegrationConnectionStatus, FactoryIntegrationConnections, FactoryIntegrationConnectResult, diff --git a/src/mount/github-api-issue-read.test.ts b/src/mount/github-api-issue-read.test.ts index 9789301..f1d165b 100644 --- a/src/mount/github-api-issue-read.test.ts +++ b/src/mount/github-api-issue-read.test.ts @@ -2,38 +2,53 @@ import { describe, expect, it, vi } from 'vitest' import { GithubApiIssueRead } from './github-api-issue-read' +const fetchByPath = (byPath: Record Response>) => + vi.fn(async (input: string | URL | Request) => { + const url = String(input) + const path = url.replace('https://api.github.com', '') + const handler = byPath[path] + if (!handler) throw new Error(`unexpected request: ${url}`) + return handler() + }) + describe('GithubApiIssueRead', () => { - it('reads an issue directly through the GitHub REST API', async () => { - const request = vi.fn(async () => new Response(JSON.stringify({ - id: 222, - node_id: 'I_222', - number: 222, - title: '[factory] Restore dispatch', - body: 'Use the GitHub API as a fallback.', - state: 'open', - html_url: 'https://github.example/AgentWorkforce/factory/issues/222', - updated_at: '2026-08-08T12:00:00Z', - user: { login: 'factory-app' }, - labels: [{ name: 'factory' }, { name: 'factory-repo' }], - }), { status: 200 })) + it('reads an issue directly through the GitHub REST API for a confirmed-public repo', async () => { + const request = fetchByPath({ + '/repos/AgentWorkforce/factory': () => new Response('{}', { status: 200 }), + '/repos/AgentWorkforce/factory/issues/222': () => new Response(JSON.stringify({ + id: 222, + node_id: 'I_222', + number: 222, + title: '[factory] Restore dispatch', + body: 'Use the GitHub API as a fallback.', + state: 'open', + html_url: 'https://github.example/AgentWorkforce/factory/issues/222', + updated_at: '2026-08-08T12:00:00Z', + user: { login: 'factory-app' }, + labels: [{ name: 'factory' }, { name: 'factory-repo' }], + }), { status: 200 }), + }) const reader = new GithubApiIssueRead({ fetch: request }) await expect(reader.getIssue('AgentWorkforce/factory', 222)).resolves.toEqual({ - repo: 'AgentWorkforce/factory', - number: 222, - path: '/github/repos/AgentWorkforce__factory/issues/by-id/222.json', - content: expect.objectContaining({ - provider: 'github', - objectType: 'issue', - objectId: 'I_222', - payload: expect.objectContaining({ - number: 222, - title: '[factory] Restore dispatch', - state: 'open', - labels: [{ name: 'factory' }, { name: 'factory-repo' }], - repository: { name: 'factory', owner: { login: 'AgentWorkforce' } }, + outcome: 'found', + issue: { + repo: 'AgentWorkforce/factory', + number: 222, + path: '/github/repos/AgentWorkforce__factory/issues/by-id/222.json', + content: expect.objectContaining({ + provider: 'github', + objectType: 'issue', + objectId: 'I_222', + payload: expect.objectContaining({ + number: 222, + title: '[factory] Restore dispatch', + state: 'open', + labels: [{ name: 'factory' }, { name: 'factory-repo' }], + repository: { name: 'factory', owner: { login: 'AgentWorkforce' } }, + }), }), - }), + }, }) expect(request).toHaveBeenCalledWith( 'https://api.github.com/repos/AgentWorkforce/factory/issues/222', @@ -41,35 +56,114 @@ describe('GithubApiIssueRead', () => { ) }) - it('returns undefined only for an authoritative 404', async () => { + it('returns not-found for a 404 against a repo it has confirmed is public', async () => { const reader = new GithubApiIssueRead({ - fetch: vi.fn(async () => new Response('{}', { status: 404 })), + fetch: fetchByPath({ + '/repos/AgentWorkforce/factory': () => new Response('{}', { status: 200 }), + '/repos/AgentWorkforce/factory/issues/999999': () => new Response('{}', { status: 404 }), + }), }) - await expect(reader.getIssue('AgentWorkforce/factory', 999_999)).resolves.toBeUndefined() + await expect(reader.getIssue('AgentWorkforce/factory', 999_999)).resolves.toEqual({ outcome: 'not-found' }) }) - it('surfaces API failures instead of manufacturing absence', async () => { + it('returns indeterminate — never not-found — for a 404 against a repo it cannot confirm is public', async () => { + // This is the private-repo case: GitHub returns the same 404 for "issue + // does not exist" and "repository is private, existence hidden from an + // unauthenticated caller". Before this reader probed repo visibility, a + // 404 here collapsed straight to `undefined`, indistinguishable from a + // confirmed miss on a public repo — RED without the repo-visibility check. const reader = new GithubApiIssueRead({ - fetch: vi.fn(async () => new Response('{}', { status: 403 })), + fetch: fetchByPath({ + '/repos/AgentWorkforce/cloud': () => new Response('{}', { status: 404 }), + '/repos/AgentWorkforce/cloud/issues/222': () => new Response('{}', { status: 404 }), + }), + }) + + const result = await reader.getIssue('AgentWorkforce/cloud', 222) + expect(result.outcome).toBe('indeterminate') + expect(result).not.toEqual({ outcome: 'not-found' }) + }) + + it('caches a confirmed repo-visibility verdict across repeated lookups', async () => { + const request = fetchByPath({ + '/repos/AgentWorkforce/factory': () => new Response('{}', { status: 200 }), + '/repos/AgentWorkforce/factory/issues/1': () => new Response('{}', { status: 404 }), + '/repos/AgentWorkforce/factory/issues/2': () => new Response('{}', { status: 404 }), + }) + const reader = new GithubApiIssueRead({ fetch: request }) + + await reader.getIssue('AgentWorkforce/factory', 1) + await reader.getIssue('AgentWorkforce/factory', 2) + + const repoProbeCalls = request.mock.calls.filter(([input]) => + String(input) === 'https://api.github.com/repos/AgentWorkforce/factory') + expect(repoProbeCalls).toHaveLength(1) + }) + + it('does not cache a rate-limited repo-visibility probe, so a later clean answer is trusted', async () => { + let call = 0 + const request = vi.fn(async (input: string | URL | Request) => { + const url = String(input) + if (url === 'https://api.github.com/repos/AgentWorkforce/factory') { + call += 1 + return call === 1 ? new Response('{}', { status: 429 }) : new Response('{}', { status: 200 }) + } + return new Response('{}', { status: 404 }) + }) + const reader = new GithubApiIssueRead({ fetch: request }) + + const first = await reader.getIssue('AgentWorkforce/factory', 1) + expect(first).toEqual({ + outcome: 'indeterminate', + reason: 'could not confirm AgentWorkforce/factory is publicly visible without authentication', + }) + + const second = await reader.getIssue('AgentWorkforce/factory', 2) + expect(second).toEqual({ outcome: 'not-found' }) + }) + + it('degrades a rate-limited issue lookup to indeterminate instead of throwing', async () => { + const reader = new GithubApiIssueRead({ + fetch: fetchByPath({ + '/repos/AgentWorkforce/factory': () => new Response('{}', { status: 200 }), + '/repos/AgentWorkforce/factory/issues/222': () => new Response('{}', { status: 403 }), + }), + }) + + await expect(reader.getIssue('AgentWorkforce/factory', 222)).resolves.toEqual({ + outcome: 'indeterminate', + reason: 'GitHub API issue lookup returned HTTP 403', + }) + }) + + it('surfaces a genuine API failure instead of manufacturing absence', async () => { + const reader = new GithubApiIssueRead({ + fetch: fetchByPath({ + '/repos/AgentWorkforce/factory': () => new Response('{}', { status: 200 }), + '/repos/AgentWorkforce/factory/issues/222': () => new Response('{}', { status: 500 }), + }), }) await expect(reader.getIssue('AgentWorkforce/factory', 222)).rejects.toThrow( - 'GitHub API issue lookup failed (HTTP 403)', + 'GitHub API issue lookup failed (HTTP 500)', ) }) it('treats a pull request returned by the issues endpoint as an authoritative issue miss', async () => { const reader = new GithubApiIssueRead({ - fetch: vi.fn(async () => new Response(JSON.stringify({ - id: 222, - number: 222, - title: '[factory] PR', - html_url: 'https://github.example/AgentWorkforce/factory/pull/222', - pull_request: {}, - }), { status: 200 })), + fetch: fetchByPath({ + '/repos/AgentWorkforce/factory': () => new Response('{}', { status: 200 }), + '/repos/AgentWorkforce/factory/issues/222': () => new Response(JSON.stringify({ + id: 222, + number: 222, + title: '[factory] PR', + html_url: 'https://github.example/AgentWorkforce/factory/pull/222', + pull_request: {}, + }), { status: 200 }), + }), }) - await expect(reader.getIssue('AgentWorkforce/factory', 222)).resolves.toBeUndefined() + await expect(reader.getIssue('AgentWorkforce/factory', 222)).resolves.toEqual({ outcome: 'not-found' }) }) }) diff --git a/src/mount/github-api-issue-read.ts b/src/mount/github-api-issue-read.ts index 8d6625a..dc9b61c 100644 --- a/src/mount/github-api-issue-read.ts +++ b/src/mount/github-api-issue-read.ts @@ -1,4 +1,4 @@ -import type { GithubConnectionIssue, GithubConnectionRead } from '../ports' +import type { GithubIssueLookup, GithubConnectionRead } from '../ports' const GITHUB_API_BASE_URL = 'https://api.github.com' @@ -10,26 +10,42 @@ export interface GithubApiIssueReadConfig { } /** - * Provider-authoritative, read-only lookup against GitHub's REST API. + * Read-only lookup against GitHub's REST API, unauthenticated. * - * Factory never receives or invokes a GitHub CLI credential here. GitHub + * Factory never receives or invokes a GitHub credential here. GitHub * mutations continue to use the Relayfile connection writeback path with the - * app author; this deliberately separate reader only recovers public issue - * facts when the preferred Relayfile projection cannot answer. + * app author; this deliberately separate reader only recovers issue facts + * when the preferred Relayfile projection cannot answer. + * + * Because every request is unauthenticated, this reader can only be + * authoritative about repositories it can independently confirm are public + * (see #isRepoPublic). GitHub returns an identical 404 for "issue does not + * exist" and "repository is private, existence hidden from you" — without + * that independent confirmation a 404 cannot be trusted as absence, so it is + * reported as `indeterminate` instead of a false `not-found`. */ export class GithubApiIssueRead implements GithubConnectionRead { readonly #fetch: FetchLike + readonly #repoIsPublic = new Map() constructor(config: GithubApiIssueReadConfig = {}) { this.#fetch = config.fetch ?? fetch } - async getIssue(repo: string, number: number): Promise { + async getIssue(repo: string, number: number): Promise { const { owner, name } = githubRepoParts(repo) if (!Number.isSafeInteger(number) || number <= 0) { throw new Error(`GitHub issue number must be a positive integer: ${number}`) } + const isPublic = await this.#isRepoPublic(owner, name) + if (isPublic === undefined) { + return { + outcome: 'indeterminate', + reason: `could not confirm ${owner}/${name} is publicly visible without authentication`, + } + } + const response = await this.#fetch( `${GITHUB_API_BASE_URL}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/issues/${number}`, { @@ -41,7 +57,17 @@ export class GithubApiIssueRead implements GithubConnectionRead { }, }, ) - if (response.status === 404) return undefined + if (response.status === 404) { + // A confirmed-public repo's 404 is a trustworthy miss. Otherwise this + // 404 is the same one GitHub returns to hide a private repo's + // existence, and cannot be told apart from a real absence. + return isPublic + ? { outcome: 'not-found' } + : { outcome: 'indeterminate', reason: `${owner}/${name} is not visible without authentication` } + } + if (isRateLimitOrAuthStatus(response.status)) { + return { outcome: 'indeterminate', reason: `GitHub API issue lookup returned HTTP ${response.status}` } + } if (!response.ok) { throw new Error(`GitHub API issue lookup failed (HTTP ${response.status})`) } @@ -53,7 +79,7 @@ export class GithubApiIssueRead implements GithubConnectionRead { // GitHub's REST "issues" endpoint also returns pull requests. For Factory // issue dispatch a PR occupying the same repository number is an // authoritative non-match, not a malformed issue. - if (issue.pull_request !== undefined) return undefined + if (issue.pull_request !== undefined) return { outcome: 'not-found' } const resolvedNumber = positiveInteger(issue.number) const title = stringValue(issue.title) const url = stringValue(issue.html_url) @@ -65,34 +91,79 @@ export class GithubApiIssueRead implements GithubConnectionRead { const author = stringValue(record(issue.user)?.login) const path = githubIssuePath(owner, name, number) return { - repo: `${owner}/${name}`, - number, - path, - content: { - provider: 'github', - objectType: 'issue', - objectId: stringValue(issue.node_id) ?? String(issue.id ?? `${owner}/${name}#${number}`), - payload: { - id: issue.id, - node_id: issue.node_id, - number, - title, - body: stringValue(issue.body) ?? '', - state: (stringValue(issue.state) ?? '').toLowerCase(), - url, - html_url: url, - updated_at: stringValue(issue.updated_at), - labels: labels - .map((label) => typeof label === 'string' ? { name: label } : { name: stringValue(record(label)?.name) }) - .filter((label): label is { name: string } => Boolean(label.name)), - ...(author ? { user: { login: author }, author: { login: author } } : {}), - repository: { name, owner: { login: owner } }, + outcome: 'found', + issue: { + repo: `${owner}/${name}`, + number, + path, + content: { + provider: 'github', + objectType: 'issue', + objectId: stringValue(issue.node_id) ?? String(issue.id ?? `${owner}/${name}#${number}`), + payload: { + id: issue.id, + node_id: issue.node_id, + number, + title, + body: stringValue(issue.body) ?? '', + state: (stringValue(issue.state) ?? '').toLowerCase(), + url, + html_url: url, + updated_at: stringValue(issue.updated_at), + labels: labels + .map((label) => typeof label === 'string' ? { name: label } : { name: stringValue(record(label)?.name) }) + .filter((label): label is { name: string } => Boolean(label.name)), + ...(author ? { user: { login: author }, author: { login: author } } : {}), + repository: { name, owner: { login: owner } }, + }, }, }, } } + + /** + * Confirms repository visibility with one unauthenticated GET, cached for + * the life of this reader. Returns `undefined` — never throws, never + * caches — for any response that is not a clean 200 or 404, so a rate + * limit or transient failure on this probe degrades a lookup to + * `indeterminate` instead of turning it into a hard error or pinning a + * wrong verdict. + */ + async #isRepoPublic(owner: string, name: string): Promise { + const key = `${owner}/${name}`.toLowerCase() + const cached = this.#repoIsPublic.get(key) + if (cached !== undefined) return cached + + let response: Response + try { + response = await this.#fetch( + `${GITHUB_API_BASE_URL}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}`, + { + method: 'GET', + signal: AbortSignal.timeout(30_000), + headers: { + accept: 'application/vnd.github+json', + 'user-agent': '@agent-relay/factory', + }, + }, + ) + } catch { + return undefined + } + if (response.status === 200) { + this.#repoIsPublic.set(key, true) + return true + } + if (response.status === 404) { + this.#repoIsPublic.set(key, false) + return false + } + return undefined + } } +const isRateLimitOrAuthStatus = (status: number): boolean => status === 403 || status === 429 + const githubRepoParts = (repo: string): { owner: string; name: string } => { const [owner, name, ...extra] = repo.split('/') if (!owner || !name || extra.length > 0) { diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index fbbcee1..9287a78 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -4679,17 +4679,32 @@ export class FactoryLoop implements Factory { const parts = githubIssuePathParts(path) ?? githubIssueDirectoryPathParts(path) const identity = parts ? githubIssueIdentity(parts.owner, parts.repo, parts.number) : undefined if (parts && identity && this.#githubApiFallbackIssues.has(identity) && this.#mount.githubRead) { - const fallback = await this.#mount.githubRead.getIssue(`${parts.owner}/${parts.repo}`, parts.number) - if (fallback) { - const githubIssue = parseGithubIssue(fallback.path, fallback.content) + const lookup = await this.#mount.githubRead.getIssue(`${parts.owner}/${parts.repo}`, parts.number) + if (lookup.outcome === 'found') { + const githubIssue = parseGithubIssue(lookup.issue.path, lookup.issue.content) this.#indexDependencyIssue(githubIssueAsFactoryIssue(githubIssue)) this.#logger.warn?.('[factory] Relayfile projection missed GitHub issue; using GitHub API fallback', { - repo: fallback.repo, - number: fallback.number, + repo: lookup.issue.repo, + number: lookup.issue.number, source: 'github-api-fallback', }) return githubIssue } + if (lookup.outcome === 'indeterminate') { + // The provider lookup could not confirm absence (e.g. an + // unauthenticated 404 against a repo it cannot prove is public). + // That is not the same claim as "confirmed gone" — count and log + // it separately so it stays visible instead of being folded into + // the phantom-skip metric. + this.#increment('githubIssueUnverifiable') + this.#logger.warn?.('[factory] GitHub API fallback could not determine issue existence', { + path, + repo: `${parts.owner}/${parts.repo}`, + number: parts.number, + reason: lookup.reason, + }) + return undefined + } } this.#increment('githubIssuePhantomSkipped') this.#logger.debug?.('[factory] skipped missing GitHub issue after projection and provider lookup', { path }) diff --git a/src/ports/index.ts b/src/ports/index.ts index 9377759..cf9315f 100644 --- a/src/ports/index.ts +++ b/src/ports/index.ts @@ -8,6 +8,7 @@ export type { GithubConnectionIssue, GithubConnectionRead, GithubConnectionWrite, + GithubIssueLookup, GithubPublishPullRequestInput, GithubPublishPullRequestResult, LocalMountHealth, diff --git a/src/ports/mount.ts b/src/ports/mount.ts index 8193f79..919684a 100644 --- a/src/ports/mount.ts +++ b/src/ports/mount.ts @@ -84,9 +84,22 @@ export interface GithubConnectionIssue { content: unknown } +/** + * Outcome of a GitHub issue lookup. A reader that cannot authenticate can + * only prove absence within what it can see: `not-found` means the reader + * confirmed the repository is visible and the issue is not in it; + * `indeterminate` means the reader could not establish that (for example an + * unauthenticated 404 against a repository it also cannot confirm is + * public — GitHub returns the same 404 for "does not exist" and "private, + * hidden from you"). Callers must not treat `indeterminate` as absence. + */ +export type GithubIssueLookup = + | { outcome: 'found'; issue: GithubConnectionIssue } + | { outcome: 'not-found' } + | { outcome: 'indeterminate'; reason: string } + export interface GithubConnectionRead { - /** Returns undefined only when GitHub authoritatively reports no issue. */ - getIssue(repo: string, number: number): Promise + getIssue(repo: string, number: number): Promise } export type FactoryIntegrationProvider = 'github' | 'linear' From 0d3bda6d012dbfd22fa99aa94d75210c0d33a00d Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 13:24:29 +0200 Subject: [PATCH 11/19] fix: short-circuit the issue-level GET when a repo is confirmed private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the repo-visibility probe cleanly confirms a repo is not visible unauthenticated (a 404, not an ambiguous/rate-limited response), the issue-level GET was still being made and only then discarded as indeterminate at its own 404. That second call can never inform the result — an invisible repo's issues are invisible too — so it only burned rate-limit budget (60 req/hr, unauthenticated) for nothing, and in this org private is the common case, not the edge. isPublic === false now returns indeterminate immediately, before the issue fetch. Added a test asserting the issue endpoint is never called when the repo probe cleanly 404s, checked on fetch call count rather than outcome alone. --- src/mount/github-api-issue-read.test.ts | 14 +++++++++----- src/mount/github-api-issue-read.ts | 18 ++++++++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/mount/github-api-issue-read.test.ts b/src/mount/github-api-issue-read.test.ts index f1d165b..991d68b 100644 --- a/src/mount/github-api-issue-read.test.ts +++ b/src/mount/github-api-issue-read.test.ts @@ -73,16 +73,20 @@ describe('GithubApiIssueRead', () => { // unauthenticated caller". Before this reader probed repo visibility, a // 404 here collapsed straight to `undefined`, indistinguishable from a // confirmed miss on a public repo — RED without the repo-visibility check. - const reader = new GithubApiIssueRead({ - fetch: fetchByPath({ - '/repos/AgentWorkforce/cloud': () => new Response('{}', { status: 404 }), - '/repos/AgentWorkforce/cloud/issues/222': () => new Response('{}', { status: 404 }), - }), + const request = vi.fn(async (input: string | URL | Request) => { + expect(String(input)).toBe('https://api.github.com/repos/AgentWorkforce/cloud') + return new Response('{}', { status: 404 }) }) + const reader = new GithubApiIssueRead({ fetch: request }) const result = await reader.getIssue('AgentWorkforce/cloud', 222) expect(result.outcome).toBe('indeterminate') expect(result).not.toEqual({ outcome: 'not-found' }) + + // A confirmed-invisible repo can't inform the issue lookup either way — + // the issue-level GET would only spend a second request (of 60/hr, + // unauthenticated) on the same unresolvable 404. Assert it is never made. + expect(request).toHaveBeenCalledTimes(1) }) it('caches a confirmed repo-visibility verdict across repeated lookups', async () => { diff --git a/src/mount/github-api-issue-read.ts b/src/mount/github-api-issue-read.ts index dc9b61c..f3e4b41 100644 --- a/src/mount/github-api-issue-read.ts +++ b/src/mount/github-api-issue-read.ts @@ -45,6 +45,15 @@ export class GithubApiIssueRead implements GithubConnectionRead { reason: `could not confirm ${owner}/${name} is publicly visible without authentication`, } } + if (!isPublic) { + // If the repo itself isn't visible unauthenticated, its issues can't + // be either — the issue-level GET would only return the same 404 for + // a different reason we still can't tell apart. Skip it: on this + // reader's rate limit (60 req/hr, unauthenticated), a private repo is + // the common case in this org, not the edge, so this second call is + // never wasted by accident. + return { outcome: 'indeterminate', reason: `${owner}/${name} is not visible without authentication` } + } const response = await this.#fetch( `${GITHUB_API_BASE_URL}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/issues/${number}`, @@ -58,12 +67,9 @@ export class GithubApiIssueRead implements GithubConnectionRead { }, ) if (response.status === 404) { - // A confirmed-public repo's 404 is a trustworthy miss. Otherwise this - // 404 is the same one GitHub returns to hide a private repo's - // existence, and cannot be told apart from a real absence. - return isPublic - ? { outcome: 'not-found' } - : { outcome: 'indeterminate', reason: `${owner}/${name} is not visible without authentication` } + // Reached only when isPublic === true: a confirmed-public repo's 404 + // is a trustworthy miss. + return { outcome: 'not-found' } } if (isRateLimitOrAuthStatus(response.status)) { return { outcome: 'indeterminate', reason: `GitHub API issue lookup returned HTTP ${response.status}` } From fccdfe624ad0439848efa40588433e7dee193fce Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 13:25:50 +0200 Subject: [PATCH 12/19] fix: resolve a qualified label selector routed outside repos.org allConfiguredGithubIssueRepos (added in 1cef8c3) includes every byLabel/byProject/keywordRules route regardless of owner, but parseGithubIssueSelector's expansion only ever tried `${repos.org}/${requested}` when repos.org was set, and fell back to the label's own route only when repos.org was unset. A label routed to a different owner than repos.org was still rejected as unconfigured even though it is in the validated set. Try the org expansion first, then the label's own route, and keep the first that matches a configured repo. --- src/cli/fleet.test.ts | 52 +++++++++++++++++++++++++++++++++++++++++++ src/cli/fleet.ts | 23 ++++++++++++------- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 5c7e46c..6ccb9ca 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -2089,6 +2089,58 @@ describe('fleet CLI runtime', () => { } }) + it('validates a qualified label selector routed outside repos.org', async () => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-qualified-cross-owner-')) + try { + const configPath = await writeConfig(root, { + issueSource: 'github', + repos: { + org: 'AgentWorkforce', + byLabel: { + // Routed to a different owner than repos.org — the org expansion + // alone (`${org}/${requested}`) cannot resolve this label. + partner: 'OtherOrg/partner-repo', + }, + clonePaths: { + 'OtherOrg/partner-repo': '/work/partner-repo', + }, + }, + }) + const githubRead = fakeGithubConnectionRead(async (repo, number) => + repo === 'OtherOrg/partner-repo' + ? { + outcome: 'found', + issue: { + repo: 'OtherOrg/partner-repo', + number, + path: `/github/repos/OtherOrg__partner-repo/issues/by-id/${number}.json`, + content: githubIssueFile('partner-repo', number), + }, + } + : githubIssueNotFound(), + ) + const output = buffer() + + const code = await runFleetCli(['triage', 'partner#5', '--config', configPath], { + fleet: new FakeFleetClient(), + mount: Object.assign(new FakeMountClient(), { + githubRead, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), + stdout: output, + stderr: buffer(), + }) + + expect(code).toBe(0) + expect(JSON.parse(output.text())).toMatchObject({ + issueResolution: { source: 'github-api-fallback', repo: 'OtherOrg/partner-repo' }, + }) + expect(githubRead.getIssue).toHaveBeenCalledWith('OtherOrg/partner-repo', 5) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('rejects non-positive GitHub issue numbers with source context', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-github-invalid-number-')) try { diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index 46707c3..afddcd5 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -1901,14 +1901,21 @@ function parseGithubIssueSelector(key: string, config: FactoryConfig): GithubIss const requested = qualified[1]! const configured = allConfiguredGithubIssueRepos(config) - const expanded = requested.includes('/') - ? requested - : config.repos.org - ? `${config.repos.org}/${requested}` - : Object.entries(config.repos.byLabel).find(([label]) => label.toLowerCase() === requested.toLowerCase())?.[1] - const repo = expanded - ? configured.find((candidate) => candidate.toLowerCase() === expanded.toLowerCase()) - : undefined + const labelRoute = Object.entries(config.repos.byLabel) + .find(([label]) => label.toLowerCase() === requested.toLowerCase())?.[1] + // A label can route to a repo outside repos.org. Try the org expansion + // first (the common case), but fall back to the label's own route rather + // than only ever trying one — otherwise a cross-owner label is rejected as + // unconfigured even though allConfiguredGithubIssueRepos does include it. + const candidates = requested.includes('/') + ? [requested] + : [ + ...(config.repos.org ? [`${config.repos.org}/${requested}`] : []), + ...(labelRoute ? [labelRoute] : []), + ] + const repo = candidates + .map((expanded) => configured.find((candidate) => candidate.toLowerCase() === expanded.toLowerCase())) + .find((match): match is string => Boolean(match)) if (!repo) { throw new Error( `${githubIssueResolutionError(config, key)}: repository ${requested} is not one of the configured Factory routes`, From 39a7d330545df8ee210640a2fbdc1a158981d5c5 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 13:34:57 +0200 Subject: [PATCH 13/19] fix: stop posting issueResolution.detail to public GitHub issue comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit issueResolution.detail (and the localMountDegradedReason it can embed) is free text describing local operator state — e.g. "mount state is missing at /Users//.relayfile//.relay/state.json". dispatchComment interpolated it directly into the comment body posted to the issue via #postIssueComment, and the same string was mirrored into DispatchResult.comments. factory is a public repo; this PR introduced issueResolution, so this was a regression the PR added. Audited every consumer of issueResolution.detail and localMountDegradedReason: dispatchComment (this PR's only construction site of a comment string containing either) is the sole render path that reaches a posted comment. Other #postIssueComment callers in this file (label-dispatch-failure and dependency-park notices) don't touch issueResolution at all. Emit only issueResolution.source in the comment — a closed two-value enum, so an allowlist rather than trying to scrub paths out of free text. detail remains untouched in the JSON issueResolution field and in logs, per the existing dispatch/triage JSON output and #logger calls, which read the field directly rather than through this function. Exported dispatchComment for a direct unit test (matching this file's existing pattern for testing pure helpers, e.g. githubIssuePathParts) asserting the rendered comment string never contains an absolute path for a degraded-mount fallback resolution. --- src/orchestrator/factory.test.ts | 65 +++++++++++++++++++++++++++++++- src/orchestrator/factory.ts | 10 ++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 6ff9061..01410fa 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -33,7 +33,7 @@ import type { CloseProbePrInput, GithubMergeGatePort, GithubMergeGateVerdict, Gi import { BatchTracker, issueKey } from './batch-tracker' import { InMemoryStateStore } from '../state/in-memory-state-store' import { FileStateStore } from '../state/file-state-store' -import { githubIssuePathParts, githubRepoSubscriptionGlobs, keyFromPath } from './factory' +import { dispatchComment, githubIssuePathParts, githubRepoSubscriptionGlobs, keyFromPath } from './factory' import { globMatchesPath } from '../subscriptions/globs' import { ResourceSubscriptionsUnavailableError, @@ -1776,6 +1776,69 @@ class RecoveringSlackRootMountClient extends CloudWritebackFakeMountClient { } } +describe('dispatchComment', () => { + const baseDecision: TriageDecision = { + issue: { uuid: 'uuid-222', key: 'factory#222', path: '/github/repos/AgentWorkforce__factory/issues/by-id/222.json' }, + routes: [{ repo: 'AgentWorkforce/factory', clonePath: '/work/factory', rationale: 'test route' }], + scope: 'single', + implementers: [], + reviewer: { + name: 'ar-222-review', + role: 'reviewer', + capability: 'spawn:claude', + model: 'claude', + task: 'Review factory#222', + repo: 'AgentWorkforce/factory', + clonePath: '/work/factory', + node: 'self', + }, + thin: false, + confidence: 'high', + rationale: 'test decision', + } + + it('never posts a local filesystem path from a degraded-mount fallback resolution', () => { + // This is the actual shape a degraded local mount produces: the Relayfile + // health check embeds its own state file path in the reason string, and + // that reason flows into issueResolution.detail unmodified. + const decision: TriageDecision = { + ...baseDecision, + issueResolution: { + source: 'github-api-fallback', + repo: 'AgentWorkforce/factory', + detail: 'Relayfile projection could not answer (mount state is missing at /Users/khaliqgant/.relayfile/rw_7ccfea89/.relay/state.json); resolved authoritatively through the GitHub API fallback.', + projection: { + outcome: 'no-match', + localMountDegraded: true, + localMountDegradedReason: 'mount state is missing at /Users/khaliqgant/.relayfile/rw_7ccfea89/.relay/state.json', + }, + }, + } + + const comment = dispatchComment(decision, [{ name: 'ar-222-review', role: 'reviewer' }]) + + expect(comment).toContain('Issue resolution: github-api-fallback') + expect(comment).not.toMatch(/\/Users\//) + expect(comment).not.toContain('/.relay/state.json') + expect(comment).not.toContain(decision.issueResolution!.detail) + }) + + it('still names the resolution source for a healthy projection match', () => { + const decision: TriageDecision = { + ...baseDecision, + issueResolution: { + source: 'relayfile-projection', + detail: 'Resolved from the preferred Relayfile projection.', + projection: { outcome: 'matched' }, + }, + } + + const comment = dispatchComment(decision, [{ name: 'ar-222-review', role: 'reviewer' }]) + + expect(comment).toContain('Issue resolution: relayfile-projection') + }) +}) + describe('FactoryLoop', () => { it('sweeps preview orphans on daemon startup using durable active issue owners', async () => { const mount = new FakeMountClient() diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 9287a78..d93a04b 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -13468,10 +13468,16 @@ const pidsFromSpawnResult = (result: { pid?: number; pids?: number[] } | undefin return [...pids].sort((a, b) => a - b) } -const dispatchComment = (decision: TriageDecision, agents: DispatchResult['agents']): string => [ +// `issueResolution.detail` and `projection.localMountDegradedReason` are +// free text that can carry an absolute local filesystem path (e.g. "mount +// state is missing at /Users//...") — safe in the JSON result and in +// logs, but this comment is posted to a public GitHub issue. Emit only +// `source`, a closed enum, rather than trying to scrub paths out of free +// text; free text is not a safe thing to scrub, only a safe thing to omit. +export const dispatchComment = (decision: TriageDecision, agents: DispatchResult['agents']): string => [ `Factory dispatch for ${decision.issue.key}`, decision.issueResolution - ? `Issue resolution: ${decision.issueResolution.source} — ${decision.issueResolution.detail}` + ? `Issue resolution: ${decision.issueResolution.source}` : undefined, `Implementers: ${agents.filter((agent) => agent.role === 'implementer').map((agent) => agent.name).join(', ') || 'none'}`, decision.scope === 'workflow' ? `Workflow: ${agents.find((agent) => agent.role === 'workflow')?.name ?? 'none'}` : undefined, From 6836c2323320a8d4e845ce9e914ca7f9dbfa82de Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 13:49:29 +0200 Subject: [PATCH 14/19] fix: restore restart-lost fallback eligibility and bound the eligibility set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unit 2 — restart recovery. #githubApiFallbackIssues is process-local, so after a restart it starts empty even though a durably-persisted decision still records issueResolution.source === 'github-api-fallback'. Durable dispatch resume (#resumeDurableDispatch) re-reads the issue before continuing and, without eligibility restored, throws "issue is not currently readable" on every retry — the lifecycle never leaves 'retryable'. Extracted the existing #dispatchUnlocked registration logic into #registerGithubApiFallbackEligibility(decision) and call it from both #dispatchUnlocked and #resumeDurableDispatch, so eligibility is restored from the decision itself rather than assumed lost. Unit 4 — bound the set. #githubApiFallbackIssues was append-only for the life of the process, growing without bound in a long-running `factory start --mode live` daemon. Bounded it at 2,000 entries, evicting the least-recently-registered identity at capacity. Bounding a set unit 2 depends on being complete risks quietly re-breaking unit 2: eviction and "never was eligible" look identical to a reader unless kept distinguishable. Evicted identities move into a second, separately-bounded record (200 entries) purely for that distinction; #readGithubIssue now checks it on a miss and counts/logs "eligibility evicted" (githubIssueApiFallbackEligibilityEvicted) separately from a confirmed-gone phantom skip (githubIssuePhantomSkipped), so an operator investigating a false phantom-skip has a way to tell the two apart instead of both collapsing into the same silent signal. The bounding/eviction/distinguishability mechanics are exported as a pure function (rememberBoundedFallbackEligibility, no class state) and tested directly against small max sizes — bounding, eviction, recency refresh on re-registration, eviction-record restoration, and bounding of the eviction record itself. Testing the full wiring at the production-sized cap (2,000 entries) through real dispatch cycles would need thousands of spawns per test run; the pure function is the same code path #rememberGithubApiFallbackEligible calls, so this is exhaustive on the mechanics without that cost. Flagging that tradeoff rather than presenting it as full end-to-end coverage. --- src/orchestrator/factory.test.ts | 154 ++++++++++++++++++++++++++++++- src/orchestrator/factory.ts | 83 ++++++++++++++++- 2 files changed, 231 insertions(+), 6 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 01410fa..6bd3d62 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -27,13 +27,13 @@ import { type WorkflowRunnerInput, } from '../index' import { changeEventPath } from './factory' -import type { AgentWorktree, AgentWorktreeCleanupInspection, AgentWorktreeManager, AgentWorktreeRepository, ChangeEvent, EventPage, GithubConnectionWrite, GithubIssueStatus, GithubPublishPullRequestInput, GithubWriteback, LinearWriteback, PreviewReference, PreviewStartInput, ProviderSyncStatus, SlackWriteback, SpawnInput, SpawnResult } from '../ports' +import type { AgentWorktree, AgentWorktreeCleanupInspection, AgentWorktreeManager, AgentWorktreeRepository, ChangeEvent, EventPage, GithubConnectionRead, GithubConnectionWrite, GithubIssueStatus, GithubPublishPullRequestInput, GithubWriteback, LinearWriteback, PreviewReference, PreviewStartInput, ProviderSyncStatus, SlackWriteback, SpawnInput, SpawnResult } from '../ports' import { FakeFleetClient, FakeMountClient } from '../testing' import type { CloseProbePrInput, GithubMergeGatePort, GithubMergeGateVerdict, GithubMergeInput, LinearIssue, VerificationGate, VerificationGateInput, VerificationVerdict } from '../index' import { BatchTracker, issueKey } from './batch-tracker' import { InMemoryStateStore } from '../state/in-memory-state-store' import { FileStateStore } from '../state/file-state-store' -import { dispatchComment, githubIssuePathParts, githubRepoSubscriptionGlobs, keyFromPath } from './factory' +import { dispatchComment, githubIssuePathParts, githubRepoSubscriptionGlobs, keyFromPath, rememberBoundedFallbackEligibility } from './factory' import { globMatchesPath } from '../subscriptions/globs' import { ResourceSubscriptionsUnavailableError, @@ -1839,6 +1839,69 @@ describe('dispatchComment', () => { }) }) +describe('rememberBoundedFallbackEligibility', () => { + it('bounds the eligible set, evicting the least-recently-registered identity', () => { + const eligible = new Set() + const evicted = new Set() + + rememberBoundedFallbackEligibility(eligible, evicted, 'a', 2, 2) + rememberBoundedFallbackEligibility(eligible, evicted, 'b', 2, 2) + expect(eligible).toEqual(new Set(['a', 'b'])) + + // Over capacity: 'a' is the least-recently-registered and is evicted. + rememberBoundedFallbackEligibility(eligible, evicted, 'c', 2, 2) + expect(eligible).toEqual(new Set(['b', 'c'])) + expect(eligible.has('a')).toBe(false) + expect(evicted.has('a')).toBe(true) + }) + + it('re-registering an identity refreshes its recency instead of leaving it due for eviction', () => { + const eligible = new Set() + const evicted = new Set() + + rememberBoundedFallbackEligibility(eligible, evicted, 'a', 2, 2) + rememberBoundedFallbackEligibility(eligible, evicted, 'b', 2, 2) + // Without the delete+add refresh, 'a' would still be positioned first + // (oldest) and would be evicted next, even though it was just reused. + rememberBoundedFallbackEligibility(eligible, evicted, 'a', 2, 2) + rememberBoundedFallbackEligibility(eligible, evicted, 'c', 2, 2) + + expect(eligible).toEqual(new Set(['a', 'c'])) + expect(eligible.has('b')).toBe(false) + }) + + it('re-registering a previously evicted identity restores it and clears the eviction record', () => { + const eligible = new Set() + const evicted = new Set() + + rememberBoundedFallbackEligibility(eligible, evicted, 'a', 2, 2) + rememberBoundedFallbackEligibility(eligible, evicted, 'b', 2, 2) + rememberBoundedFallbackEligibility(eligible, evicted, 'c', 2, 2) + expect(evicted.has('a')).toBe(true) + + rememberBoundedFallbackEligibility(eligible, evicted, 'a', 2, 2) + + expect(eligible.has('a')).toBe(true) + expect(evicted.has('a')).toBe(false) + }) + + it('bounds the evicted record itself, so it cannot grow without bound either', () => { + const eligible = new Set() + const evicted = new Set() + + // maxEligible=1 forces an eviction on every insert after the first. + for (const identity of ['a', 'b', 'c', 'd', 'e']) { + rememberBoundedFallbackEligibility(eligible, evicted, identity, 1, 2) + } + + expect(eligible).toEqual(new Set(['e'])) + expect(evicted.size).toBeLessThanOrEqual(2) + // The most recently evicted identities survive; the earliest do not. + expect(evicted.has('d')).toBe(true) + expect(evicted.has('a')).toBe(false) + }) +}) + describe('FactoryLoop', () => { it('sweeps preview orphans on daemon startup using durable active issue owners', async () => { const mount = new FakeMountClient() @@ -6458,6 +6521,93 @@ describe('FactoryLoop', () => { } }) + it('recovers a durable GitHub dispatch through the API fallback after a restart loses process-local eligibility', async () => { + 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 + } + } + + const number = 700 + const path = githubIssueNestedMetaPath('AgentWorkforce', 'pear', number) + const openIssue = githubIssueFile(number, { state: 'open', labels: ['factory'] }) + // The Relayfile projection never has this issue at all — only the direct + // GitHub API fallback can serve it, exactly like a CLI-resolved + // repo#number target the projection could never answer. + const githubRead: GithubConnectionRead = { + getIssue: vi.fn(async (repo: string, num: number) => + repo === 'AgentWorkforce/pear' && num === number + ? { outcome: 'found' as const, issue: { repo, number: num, path, content: openIssue } } + : { outcome: 'not-found' as const }), + } + const mount = Object.assign(new FakeMountClient({}), { githubRead }) + const fleet = new AckGapFleet() + const root = await mkdtemp(join(tmpdir(), `factory-github-api-fallback-restart-${number}-`)) + const watchStatePath = join(root, 'state.json') + const state = () => new FileStateStore({ batchSize: 2, watchStatePath }) + const first = createFactory(config({ issueSource: 'github' }), { + mount, + fleet, + stateStore: state(), + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + }) + let restarted: ReturnType | undefined + try { + const triaged = await first.triageIssue(parseGithubFactoryIssue(path, openIssue)) + const decision: TriageDecision = { + ...triaged, + issueResolution: { + source: 'github-api-fallback', + repo: 'AgentWorkforce/pear', + detail: 'Relayfile projection could not answer; resolved authoritatively through the GitHub API fallback.', + projection: { outcome: 'no-match' }, + }, + } + + await expect(first.dispatch(decision)).rejects.toThrow('owner crashed after remote spawn ack') + expect(githubRead.getIssue).toHaveBeenCalledWith('AgentWorkforce/pear', number) + await expect(state().getDispatchLifecycle('factory-test', issueKey(decision.issue))) + .resolves.toMatchObject({ phase: 'retryable' }) + await first.stop() + + // A restarted process's #githubApiFallbackIssues is empty — it is + // process-local. Only decision.issueResolution, persisted in the + // durable lifecycle record, can restore fallback eligibility here. + // Without that restoration this resume throws "issue is not currently + // readable" on every retry and the lifecycle never leaves 'retryable'. + restarted = createFactory(config({ issueSource: 'github' }), { + mount, + fleet, + stateStore: state(), + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + }) + await restarted.start({ mode: 'dispatch-owner' }) + + await vi.waitFor(async () => expect(await state().getDispatchLifecycle( + 'factory-test', + issueKey(decision.issue), + )).toMatchObject({ phase: 'running' }), { timeout: 4_000 }) + + expect(fleet.spawns.map((spawn) => spawn.name)).toEqual([ + `ar-${number}-impl-pear`, + `ar-${number}-review-pear`, + ]) + } 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 d93a04b..55fe184 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -345,6 +345,47 @@ const STALE_LOCAL_AGENT_RECLAIM_MAX_ATTEMPTS = 3 const STALE_LOCAL_AGENT_RECLAIM_BACKOFF_MS = 500 export const DEFAULT_FACTORY_LOOP_HEARTBEAT_PATH = '/tmp/factory-run/factory-loop-heartbeat.json' export const DEFAULT_FACTORY_LOOP_REGISTRY_PATH = '/tmp/factory-run/factory-loop-registry.json' +// #githubApiFallbackIssues is append-only in memory for the life of the +// process; bound it so a long-running `factory start --mode live` daemon +// doesn't accumulate one entry per fallback dispatch forever. +const GITHUB_API_FALLBACK_ISSUES_MAX = 2_000 +// A small, separately-bounded record of recently evicted identities, so a +// later miss can be reported as "eligibility was evicted" rather than +// silently folded into the same signal as "never was eligible" — an +// operator investigating a false phantom-skip needs to tell those apart. +const GITHUB_API_FALLBACK_ISSUES_EVICTED_MAX = 200 + +/** + * Adds `identity` to `eligible`, evicting the least-recently-registered + * entry once at capacity (Sets preserve insertion order; delete+add + * refreshes an existing identity's recency instead of leaving it at its + * original position). An evicted identity moves into `evicted` — itself + * bounded — so a later caller can tell "eligibility was evicted" apart from + * "never was eligible" instead of collapsing both into the same signal. + * Exported standalone (pure, no class state) so the bounding and eviction + * behavior is directly testable without driving thousands of dispatches + * through a full FactoryLoop to fill the production-sized set. + */ +export function rememberBoundedFallbackEligibility( + eligible: Set, + evicted: Set, + identity: string, + maxEligible: number, + maxEvicted: number, +): void { + evicted.delete(identity) + eligible.delete(identity) + eligible.add(identity) + if (eligible.size <= maxEligible) return + const oldest = eligible.values().next().value + if (oldest === undefined) return + eligible.delete(oldest) + evicted.delete(oldest) + evicted.add(oldest) + if (evicted.size <= maxEvicted) return + const oldestEvicted = evicted.values().next().value + if (oldestEvicted !== undefined) evicted.delete(oldestEvicted) +} class DispatchLifecycleCapacityError extends Error {} class DispatchLifecycleOwnedElsewhereError extends Error { @@ -413,6 +454,7 @@ export class FactoryLoop implements Factory { readonly #githubIssueAuthorLookups = new Map>() readonly #githubIssuePreferredPaths = new Map() readonly #githubApiFallbackIssues = new Set() + readonly #githubApiFallbackIssuesEvicted = new Set() #githubIssuePathIndexReady = false readonly #slackReporterUserIds = new Map() readonly #slackReporterUserIdLookups = new Map>() @@ -2427,10 +2469,7 @@ export class FactoryLoop implements Factory { throw error } - if (decision.issueResolution?.source === 'github-api-fallback') { - const parts = githubIssuePathParts(decision.issue.path) ?? githubIssueDirectoryPathParts(decision.issue.path) - if (parts) this.#githubApiFallbackIssues.add(githubIssueIdentity(parts.owner, parts.repo, parts.number)) - } + this.#registerGithubApiFallbackEligibility(decision) const liveIssue = await this.#readIssue(decision.issue.path) if (!liveIssue || !isInFactoryScope(liveIssue, this.#config.safety)) { const error = new Error(`Refusing to dispatch ${decision.issue.key}: not factory-e2e scope`) @@ -3994,6 +4033,7 @@ export class FactoryLoop implements Factory { async #resumeDurableDispatch(record: InFlightIssue): Promise { let liveIssue: LinearIssue | undefined if (!record.dryRun) { + this.#registerGithubApiFallbackEligibility(record.decision) liveIssue = await this.#readIssue(record.issue.path) if (!liveIssue) { throw new Error(`Unable to recover durable dispatch ${record.issue.key}: issue is not currently readable`) @@ -4654,6 +4694,30 @@ export class FactoryLoop implements Factory { } } + /** + * `#githubApiFallbackIssues` is process-local, so a restart loses it while + * a durably-persisted decision still records `issueResolution.source === + * 'github-api-fallback'`. Call this before any `#readIssue`/ + * `#readGithubIssue` that must be able to recover such an issue (initial + * dispatch and durable-dispatch resume alike) so eligibility is restored + * from the decision itself rather than assumed lost. + */ + #registerGithubApiFallbackEligibility(decision: TriageDecision): void { + if (decision.issueResolution?.source !== 'github-api-fallback') return + const parts = githubIssuePathParts(decision.issue.path) ?? githubIssueDirectoryPathParts(decision.issue.path) + if (parts) this.#rememberGithubApiFallbackEligible(githubIssueIdentity(parts.owner, parts.repo, parts.number)) + } + + #rememberGithubApiFallbackEligible(identity: string): void { + rememberBoundedFallbackEligibility( + this.#githubApiFallbackIssues, + this.#githubApiFallbackIssuesEvicted, + identity, + GITHUB_API_FALLBACK_ISSUES_MAX, + GITHUB_API_FALLBACK_ISSUES_EVICTED_MAX, + ) + } + async #readGithubIssue(path: string): Promise { const preferredPath = await this.#preferredGithubIssuePath(path) const candidatePaths = [...new Set([ @@ -4706,6 +4770,17 @@ export class FactoryLoop implements Factory { return undefined } } + if (identity && this.#githubApiFallbackIssuesEvicted.has(identity)) { + // identity was eligible for the API fallback but aged out of the + // bounded set — a different claim from "never was eligible", and + // one this signal must not silently collapse into a confirmed miss. + this.#increment('githubIssueApiFallbackEligibilityEvicted') + this.#logger.warn?.('[factory] GitHub API fallback eligibility was evicted from the bounded cache before this read', { + path, + identity, + }) + return undefined + } this.#increment('githubIssuePhantomSkipped') this.#logger.debug?.('[factory] skipped missing GitHub issue after projection and provider lookup', { path }) return undefined From 07b2a254f80a34803e57dc26581c72a9d68b14cc Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 13:50:26 +0200 Subject: [PATCH 15/19] fix: rename a test whose title contradicted its own assertions Titled as if the GitHub API fallback resolves the issue, but the assertions verify the opposite: the projection stays preferred (source: relayfile-projection) and githubRead.getIssue is never called. Applied cubic's exact suggested title. --- src/cli/fleet.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 6ccb9ca..428fb07 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -767,7 +767,7 @@ describe('fleet CLI runtime', () => { } }) - it('allows targeted GitHub resolution through the API seam when the connected projection is not ready', async () => { + it('keeps a populated projection preferred over the API fallback when the connection is not ready', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-integration-github-fallback-')) try { const configPath = await writeConfig(root, { issueSource: 'github' }) From 48def31831ad8fcc47ddb63a54a4a3f39b7ee87d Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 13:55:51 +0200 Subject: [PATCH 16/19] fix: normalize qualified GitHub issue selectors through one canonicalization path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three consecutive review rounds found a different normalization gap in parseGithubIssueSelector's qualified-selector resolution: round 1 collapsed to repos.default; round 2's org-expansion never consulted byLabel for a cross-owner route; round 3 compared a bare byLabel route against a normalized configured entry and never matched. Each round patched the specific case found rather than the shared cause: an ad-hoc, hand-rolled candidate/comparison list that only handled whichever combination of org/label/qualification it happened to be built against. Replaced it with the invariant every configured entry already satisfies: resolve the requested selector through the same resolveGithubIssueRepoCandidates canonicalization (label mapping, org prefixing, canonical-route lookup) used to build the configured route set, then compare only normalized against normalized. Deleted the separate ad-hoc expansion entirely. Tested as a 3-dimensional matrix (bare vs qualified selector x repos.org set vs unset x label route written bare vs owner/repo, plus case-insensitivity and an already-qualified selector) via a direct unit test of the exported parseGithubIssueSelector, rather than one regression case per bug a reviewer happened to name. Confirmed the two rows matching round 3's exact bug shape fail against the prior (round 2) resolution logic and pass with this one; the other seven rows already passed under round 2's logic, isolating exactly the case that was actually broken. Also fixed an internally inconsistent fixture in the round-2 cross- owner test: its mock issue content hardcoded owner "AgentWorkforce" while the test's own scenario is a repo owned by "OtherOrg" — an inconsistent fixture can pass for the wrong reason. githubIssueFile now takes an optional owner parameter (default unchanged) instead of hardcoding one. --- src/cli/fleet.test.ts | 100 +++++++++++++++++++++++++++++++++++++++--- src/cli/fleet.ts | 29 +++++------- 2 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 428fb07..a8463b6 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -8,19 +8,20 @@ import type { CloseProbePrInput, Factory, FactoryCloudEventInputV1, + FactoryConfig, FactoryEventReporter, FactoryIntegrationConnections, FactoryIntegrationProvider, FactoryPorts, createFactory, } from '../index' -import { stateResolutionFromIds } from '../index' +import { FactoryConfigSchema, stateResolutionFromIds } from '../index' import { FileStateStore } from '../state/file-state-store' import { FakeFleetClient, FakeMountClient } from '../testing' import type { GithubConnectionRead, GithubConnectionWrite, GithubIssueLookup, LocalMountOptions, SpawnInput, SpawnResult } from '../ports' import type { HarnessDriverClientLike } from '../fleet/internal-fleet-client' import { ensureLocalMount as runLocalMountPreflight } from '../mount/local-mount-preflight' -import { formatLogArgs, installFactoryStopSignalHandlers, parseFleetCommand, parseGlobalOptions, resolveBrokerConnectionPath, runFleetCli } from './fleet' +import { formatLogArgs, installFactoryStopSignalHandlers, parseFleetCommand, parseGithubIssueSelector, parseGlobalOptions, resolveBrokerConnectionPath, runFleetCli } from './fleet' const issuePath = '/linear/issues/AR-77__uuid-77.json' @@ -102,7 +103,7 @@ class CompletingRemoteFleetClient extends FakeFleetClient { } } -const githubIssueFile = (repo: string, number = 48) => ({ +const githubIssueFile = (repo: string, number = 48, owner = 'AgentWorkforce') => ({ provider: 'github', objectType: 'issue', objectId: `${repo}-${number}`, @@ -112,8 +113,8 @@ const githubIssueFile = (repo: string, number = 48) => ({ body: 'Dispatch the repository-qualified GitHub issue.', state: 'open', labels: [{ name: 'factory' }, { name: repo }], - url: `https://github.com/AgentWorkforce/${repo}/issues/${number}`, - repository: { name: repo, owner: { login: 'AgentWorkforce' } }, + url: `https://github.com/${owner}/${repo}/issues/${number}`, + repository: { name: repo, owner: { login: owner } }, }, }) @@ -478,6 +479,93 @@ describe('fleet CLI parsing', () => { }) }) +describe('parseGithubIssueSelector normalization matrix', () => { + // Three review rounds each found a different normalization gap in this + // resolution (default-only collapse, org-only expansion, bare-label vs + // normalized comparison). Every candidate and every configured entry must + // pass through the same canonicalization before any comparison — this + // table exercises the combinations that produced each of those bugs, + // not just the one case each round happened to name. + const buildConfig = (overrides: { + org?: string + byLabel?: Record + default?: string + }): FactoryConfig => FactoryConfigSchema.parse({ + workspaceId: 'factory-cli-test', + repos: { + byLabel: overrides.byLabel ?? {}, + clonePaths: {}, + ...(overrides.org !== undefined ? { org: overrides.org } : {}), + ...(overrides.default !== undefined ? { default: overrides.default } : {}), + }, + stateIds: TEST_STATE_IDS, + }) + + it.each([ + [ + 'qualified selector, bare label route, org set: resolves via org-prefixing', + 'work#5', + { org: 'AgentWorkforce', byLabel: { work: 'factory' } }, + { number: 5, repo: 'AgentWorkforce/factory' }, + ], + [ + 'qualified selector, bare label route, org unset: cannot resolve without an owner', + 'work#5', + { byLabel: { work: 'factory' } }, + undefined, + ], + [ + 'qualified selector, cross-owner qualified label route, org set to a different owner: label route wins over org', + 'work#5', + { org: 'AgentWorkforce', byLabel: { work: 'OtherOrg/partner-repo' } }, + { number: 5, repo: 'OtherOrg/partner-repo' }, + ], + [ + 'qualified selector, cross-owner qualified label route, org unset: resolves without needing org', + 'work#5', + { byLabel: { work: 'OtherOrg/partner-repo' } }, + { number: 5, repo: 'OtherOrg/partner-repo' }, + ], + [ + 'qualified selector, already fully owner/repo-qualified: resolves directly regardless of byLabel', + 'AgentWorkforce/factory#5', + { org: 'AgentWorkforce', byLabel: { work: 'factory' } }, + { number: 5, repo: 'AgentWorkforce/factory' }, + ], + [ + 'qualified selector, label match is case-insensitive', + 'WORK#5', + { org: 'AgentWorkforce', byLabel: { work: 'factory' } }, + { number: 5, repo: 'AgentWorkforce/factory' }, + ], + [ + 'qualified selector, unconfigured label: rejected regardless of org', + 'unknown#5', + { org: 'AgentWorkforce', byLabel: { work: 'factory' } }, + undefined, + ], + [ + 'bare selector: never carries a repo, independent of org/byLabel shape', + '5', + { org: 'AgentWorkforce', byLabel: { work: 'factory' }, default: 'AgentWorkforce/factory' }, + { number: 5 }, + ], + [ + 'bare selector with no default and no org: still just the number', + '5', + { byLabel: { work: 'factory' } }, + { number: 5 }, + ], + ] as const)('%s', (_name, key, overrides, expected) => { + const config = buildConfig(overrides) + if (expected === undefined) { + expect(() => parseGithubIssueSelector(key, config)).toThrow(/is not one of the configured Factory routes/) + } else { + expect(parseGithubIssueSelector(key, config)).toEqual(expected) + } + }) +}) + describe('fleet CLI runtime', () => { it.each([ { name: 'warns after spawning without a discovered connection', started: true, connection: false, warns: true }, @@ -2114,7 +2202,7 @@ describe('fleet CLI runtime', () => { repo: 'OtherOrg/partner-repo', number, path: `/github/repos/OtherOrg__partner-repo/issues/by-id/${number}.json`, - content: githubIssueFile('partner-repo', number), + content: githubIssueFile('partner-repo', number, 'OtherOrg'), }, } : githubIssueNotFound(), diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index afddcd5..3033dbf 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -1886,9 +1886,9 @@ function githubProjectionUnavailableReason(projection: IssueResolution['projecti return undefined } -type GithubIssueSelector = { number: number; repo?: string } +export type GithubIssueSelector = { number: number; repo?: string } -function parseGithubIssueSelector(key: string, config: FactoryConfig): GithubIssueSelector { +export function parseGithubIssueSelector(key: string, config: FactoryConfig): GithubIssueSelector { const qualified = key.match(/^([^#]+)#([1-9]\d*)$/u) const bare = key.match(/^#?([1-9]\d*)$/u) const number = Number(qualified?.[2] ?? bare?.[1]) @@ -1901,21 +1901,16 @@ function parseGithubIssueSelector(key: string, config: FactoryConfig): GithubIss const requested = qualified[1]! const configured = allConfiguredGithubIssueRepos(config) - const labelRoute = Object.entries(config.repos.byLabel) - .find(([label]) => label.toLowerCase() === requested.toLowerCase())?.[1] - // A label can route to a repo outside repos.org. Try the org expansion - // first (the common case), but fall back to the label's own route rather - // than only ever trying one — otherwise a cross-owner label is rejected as - // unconfigured even though allConfiguredGithubIssueRepos does include it. - const candidates = requested.includes('/') - ? [requested] - : [ - ...(config.repos.org ? [`${config.repos.org}/${requested}`] : []), - ...(labelRoute ? [labelRoute] : []), - ] - const repo = candidates - .map((expanded) => configured.find((candidate) => candidate.toLowerCase() === expanded.toLowerCase())) - .find((match): match is string => Boolean(match)) + // Resolve what the caller typed through the exact same canonicalization + // every configured route already goes through (label mapping, org + // prefixing, canonical-route lookup) instead of a second, ad-hoc + // expansion — three review rounds each found a different normalization + // gap (default-only, org-only, bare-label-vs-normalized) in a hand-rolled + // comparison here. Comparison is only ever normalized against normalized. + const [resolved] = resolveGithubIssueRepoCandidates(config, [requested]) + const repo = resolved + ? configured.find((candidate) => candidate.toLowerCase() === resolved.toLowerCase()) + : undefined if (!repo) { throw new Error( `${githubIssueResolutionError(config, key)}: repository ${requested} is not one of the configured Factory routes`, From 793f252a31ab15ead4fe715220012e353c774043 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 14:24:36 +0200 Subject: [PATCH 17/19] fix: derive GitHub API fallback eligibility instead of registering it per call site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registering eligibility at exactly two sites (#dispatchUnlocked, #resumeDurableDispatch) against ~21 #readIssue/#readGithubIssue call sites is the same shape that produced three rounds of selector bugs: a fallback-backed record that reached 'publishing' (PR publication) after a restart could not reread its issue, because nothing on that path ever registered it — #publishImplementerPullRequest reads the issue directly and had no idea the eligibility cache existed. Adding a third registration call at that one site would only repeat the mistake for whichever site is found next. Eligibility is now derived, not remembered. #readGithubIssue resolves it itself on a cache miss via #deriveGithubApiFallbackEligibility, which checks (a) an optional decisionHint, for the one read that happens before its own record exists in the batch (the initial live dispatch, which validates scope before it is tracked at all), or (b) the currently in-flight batch, scanned for a record whose decision was resolved through the fallback. BatchTracker.restore/start insert a durably-resumed or freshly-dispatched record into the batch before any phase handler runs, so every other call site — publishing, parking, question-handling, completion, and any future one — resolves correctly with no changes and nothing to remember. A successful derived lookup warms the existing bounded cache so repeated reads of the same issue stay cheap. Derivation intentionally scans the in-memory batch rather than looking up the durable record directly by its composite key (uuid/key/path): the real uuid is built from GitHub's node_id/id when content is available, which a path alone cannot reconstruct, so a direct durable-store lookup keyed that way would not reliably match. The in-memory batch instead matches by GitHub identity (owner/repo/number) against a repository's own issue path, sidestepping that problem entirely, at the cost of only ever finding an issue that is currently tracked in this process (a durably-restored record always is by the time any phase handler reads it). Exported the derivation as a pure function (isGithubApiFallbackEligible) and tested it directly: eligible from a tracked record alone with no hint and nothing registered (the guard against a future call site reintroducing this by omission), not eligible with no match, not eligible for a projection-sourced (non-fallback) record, not eligible for a different identity, eligible from decisionHint alone before any record is tracked, and decisionHint does not leak to a different identity. Also added a full FactoryLoop-level red check: a fallback-backed GitHub issue whose PR publication is deliberately parked at 'publishing' pre-restart (publisher fails until reactivated), then resumed by a fresh createFactory instance with an empty in-memory cache. Confirmed failing before this fix — the restarted process's publish retries never reach the publisher a second time, stuck throwing "issue is no longer readable" on every attempt — and passing after it. --- src/orchestrator/factory.test.ts | 163 ++++++++++++++++++++++++++++++- src/orchestrator/factory.ts | 93 +++++++++++++----- 2 files changed, 232 insertions(+), 24 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 6bd3d62..be66a7d 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -33,7 +33,7 @@ import type { CloseProbePrInput, GithubMergeGatePort, GithubMergeGateVerdict, Gi import { BatchTracker, issueKey } from './batch-tracker' import { InMemoryStateStore } from '../state/in-memory-state-store' import { FileStateStore } from '../state/file-state-store' -import { dispatchComment, githubIssuePathParts, githubRepoSubscriptionGlobs, keyFromPath, rememberBoundedFallbackEligibility } from './factory' +import { dispatchComment, githubIssueIdentity, githubIssuePathParts, githubRepoSubscriptionGlobs, isGithubApiFallbackEligible, keyFromPath, rememberBoundedFallbackEligibility } from './factory' import { globMatchesPath } from '../subscriptions/globs' import { ResourceSubscriptionsUnavailableError, @@ -1902,6 +1902,69 @@ describe('rememberBoundedFallbackEligibility', () => { }) }) +describe('isGithubApiFallbackEligible', () => { + // Registering eligibility at exactly two sites (dispatch, resume) against + // ~21 read call sites is the shape that produced the 'publishing'-phase + // bug (round 4): the next entry path added without also registering + // silently reintroduces it. This is the guard against that: a record + // present in `inFlight` with a fallback-sourced issueResolution must be + // found eligible by identity alone, with no hint and nothing having + // "registered" it — proving no caller has to remember anything for a + // currently-tracked issue. + const path = githubIssueNestedMetaPath('AgentWorkforce', 'pear', 700) + const identity = githubIssueIdentity('AgentWorkforce', 'pear', 700) + const issue = { uuid: 'u-700', key: '700', path } + const fallbackDecision: TriageDecision = { + issue, + routes: [], + scope: 'single', + implementers: [], + reviewer: { name: 'r', role: 'reviewer', capability: 'spawn:claude', model: 'claude', task: 't', repo: 'AgentWorkforce/pear', clonePath: '/work/pear', node: 'self' }, + thin: false, + confidence: 'high', + rationale: 'test', + issueResolution: { + source: 'github-api-fallback', + repo: 'AgentWorkforce/pear', + detail: 'test', + projection: { outcome: 'no-match' }, + }, + } + + it('is eligible from a tracked in-flight record alone — no hint, nothing registered', () => { + expect(isGithubApiFallbackEligible([{ issue, decision: fallbackDecision }], identity)).toBe(true) + }) + + it('is not eligible when no tracked record and no hint matches the identity', () => { + expect(isGithubApiFallbackEligible([], identity)).toBe(false) + }) + + it('is not eligible for a tracked record resolved through the projection, not the fallback', () => { + const projectionDecision: TriageDecision = { + ...fallbackDecision, + issueResolution: { source: 'relayfile-projection', detail: 'test', projection: { outcome: 'matched' } }, + } + expect(isGithubApiFallbackEligible([{ issue, decision: projectionDecision }], identity)).toBe(false) + }) + + it('is not eligible for a tracked record with a different identity', () => { + const otherIssue = { uuid: 'u-701', key: '701', path: githubIssueNestedMetaPath('AgentWorkforce', 'pear', 701) } + expect(isGithubApiFallbackEligible( + [{ issue: otherIssue, decision: { ...fallbackDecision, issue: otherIssue } }], + identity, + )).toBe(false) + }) + + it('is eligible from decisionHint alone, before any record is tracked', () => { + expect(isGithubApiFallbackEligible([], identity, fallbackDecision)).toBe(true) + }) + + it('does not use decisionHint for a different identity', () => { + const otherIdentity = githubIssueIdentity('AgentWorkforce', 'pear', 701) + expect(isGithubApiFallbackEligible([], otherIdentity, fallbackDecision)).toBe(false) + }) +}) + describe('FactoryLoop', () => { it('sweeps preview orphans on daemon startup using durable active issue owners', async () => { const mount = new FakeMountClient() @@ -6608,6 +6671,104 @@ describe('FactoryLoop', () => { } }) + it('recovers a fallback-backed issue during PR publication after a restart, not just during dispatch resume', async () => { + // Registering eligibility only at #dispatchUnlocked/#resumeDurableDispatch + // covers the 'dispatching'/'retryable' phases but not 'publishing' (or + // any other phase/call site that reads the issue) — this drives a + // fallback-backed record through a restart while parked at 'publishing' + // specifically, where nothing ever registered anything for it. + const number = 701 + const path = githubIssueNestedMetaPath('AgentWorkforce', 'pear', number) + const openIssue = githubIssueFile(number, { state: 'open', labels: ['factory'] }) + const githubRead: GithubConnectionRead = { + getIssue: vi.fn(async (repo: string, num: number) => + repo === 'AgentWorkforce/pear' && num === number + ? { outcome: 'found' as const, issue: { repo, number: num, path, content: openIssue } } + : { outcome: 'not-found' as const }), + } + let providerReady = false + let attempts = 0 + const githubWrite: GithubConnectionWrite = { + publishPullRequest: async (input) => { + attempts += 1 + if (!providerReady) throw new Error('publisher unavailable') + return { + repo: input.repo, + number: 900, + url: 'https://github.com/AgentWorkforce/pear/pull/900', + headRef: input.headRef!, + } + }, + closePullRequest: async () => undefined, + } + const mount = Object.assign(new FakeMountClient({ + '/github/repos/AgentWorkforce/pear/meta.json': { default_branch: 'main' }, + }, githubWrite), { githubRead }) + const firstFleet = new RemoteLifecycleFleetClient() + const root = await mkdtemp(join(tmpdir(), `factory-github-api-fallback-publishing-${number}-`)) + const watchStatePath = join(root, 'state.json') + const state = () => new FileStateStore({ batchSize: 2, watchStatePath }) + const first = createFactory(config({ issueSource: 'github' }), { + mount, + fleet: firstFleet, + stateStore: state(), + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + probePrResolver: async () => undefined, + }) + let restarted: ReturnType | undefined + try { + const triaged = await first.triageIssue(parseGithubFactoryIssue(path, openIssue)) + const decision: TriageDecision = { + ...triaged, + issueResolution: { + source: 'github-api-fallback', + repo: 'AgentWorkforce/pear', + detail: 'Relayfile projection could not answer; resolved authoritatively through the GitHub API fallback.', + projection: { outcome: 'no-match' }, + }, + } + + await first.dispatch(decision) + firstFleet.emitAgentExit(`ar-${number}-impl-pear`, 'exited') + // The read inside #publishImplementerPullRequest succeeds here too — + // still the same process, the cache warmed during dispatch covers it. + await vi.waitFor(() => expect(attempts).toBe(1)) + await vi.waitFor(async () => expect(await state().getDispatchLifecycle('factory-test', issueKey(decision.issue))) + .toMatchObject({ phase: 'publishing' })) + await first.stop() + + providerReady = true + const restartedFleet = new RemoteLifecycleFleetClient() + restarted = createFactory(config({ issueSource: 'github' }), { + mount, + fleet: restartedFleet, + stateStore: state(), + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + probePrResolver: async () => undefined, + }) + await restarted.start({ mode: 'dispatch-owner' }) + + // Before the fix: the restarted process's eligibility set is empty and + // nothing registers it for the 'publishing' phase, so the read inside + // #publishImplementerPullRequest throws "issue is no longer readable" + // before ever calling the publisher — attempts never reaches 2, and + // this waitFor times out. + await vi.waitFor(() => expect(attempts).toBe(2), { timeout: 4_000 }) + // The lifecycle can race straight past 'published' to 'complete' by + // the time this polls — either proves the read (and publish) succeeded. + await vi.waitFor(async () => { + const lifecycle = await state().getDispatchLifecycle('factory-test', issueKey(decision.issue)) + expect(['published', 'complete']).toContain(lifecycle?.phase) + }, { timeout: 4_000 }) + } finally { + await restarted?.stop() + await first.stop() + await rm(root, { recursive: true, force: true }) + } + }, 15_000) + 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 55fe184..cf8540b 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -2469,8 +2469,11 @@ export class FactoryLoop implements Factory { throw error } - this.#registerGithubApiFallbackEligibility(decision) - const liveIssue = await this.#readIssue(decision.issue.path) + // This read runs before `decision` has any tracked record for + // #deriveGithubApiFallbackEligibility to find (batch insertion happens + // later, once scope/readiness are validated), so pass it directly + // rather than registering it into the cache first. + const liveIssue = await this.#readIssue(decision.issue.path, decision) if (!liveIssue || !isInFactoryScope(liveIssue, this.#config.safety)) { const error = new Error(`Refusing to dispatch ${decision.issue.key}: not factory-e2e scope`) this.#error(error, decision.issue) @@ -4033,8 +4036,11 @@ export class FactoryLoop implements Factory { async #resumeDurableDispatch(record: InFlightIssue): Promise { let liveIssue: LinearIssue | undefined if (!record.dryRun) { - this.#registerGithubApiFallbackEligibility(record.decision) - liveIssue = await this.#readIssue(record.issue.path) + // record is already inserted into the batch by the time any phase + // handler runs (BatchTracker.restore, called before this), so + // #deriveGithubApiFallbackEligibility would find it there too — the + // explicit hint is defense in depth, not a requirement. + liveIssue = await this.#readIssue(record.issue.path, record.decision) if (!liveIssue) { throw new Error(`Unable to recover durable dispatch ${record.issue.key}: issue is not currently readable`) } @@ -4694,20 +4700,6 @@ export class FactoryLoop implements Factory { } } - /** - * `#githubApiFallbackIssues` is process-local, so a restart loses it while - * a durably-persisted decision still records `issueResolution.source === - * 'github-api-fallback'`. Call this before any `#readIssue`/ - * `#readGithubIssue` that must be able to recover such an issue (initial - * dispatch and durable-dispatch resume alike) so eligibility is restored - * from the decision itself rather than assumed lost. - */ - #registerGithubApiFallbackEligibility(decision: TriageDecision): void { - if (decision.issueResolution?.source !== 'github-api-fallback') return - const parts = githubIssuePathParts(decision.issue.path) ?? githubIssueDirectoryPathParts(decision.issue.path) - if (parts) this.#rememberGithubApiFallbackEligible(githubIssueIdentity(parts.owner, parts.repo, parts.number)) - } - #rememberGithubApiFallbackEligible(identity: string): void { rememberBoundedFallbackEligibility( this.#githubApiFallbackIssues, @@ -4718,7 +4710,30 @@ export class FactoryLoop implements Factory { ) } - async #readGithubIssue(path: string): Promise { + /** + * `#githubApiFallbackIssues` is a cache, not a source of truth: eligibility + * is derived here, not remembered by every caller. A restart (or any read + * path added later that never registered anything) still resolves + * correctly because this checks the actual current state instead of + * requiring every entry point to have called a registration method first. + * + * `decisionHint` covers the one read that happens before its own record + * exists in the batch (the initial live dispatch, which reads the issue + * to validate scope before it is tracked at all) — everywhere else, the + * durably-restored or live-dispatched record is already in + * `batch.inFlight` (`BatchTracker.restore`/`start` insert it before any + * phase handler runs), so scanning it finds the same `issueResolution` + * a durable-state lookup would, without needing to reconstruct the + * `IssueRef` composite key (uuid/key/path) that a direct durable-store + * lookup would require and that this path alone cannot reliably rebuild + * (the real uuid is built from GitHub's node_id/id when content is + * available; from just a path, only the issue number is derivable). + */ + async #deriveGithubApiFallbackEligibility(identity: string, decisionHint?: TriageDecision): Promise { + return isGithubApiFallbackEligible((await this.#batch()).inFlight, identity, decisionHint) + } + + async #readGithubIssue(path: string, decisionHint?: TriageDecision): Promise { const preferredPath = await this.#preferredGithubIssuePath(path) const candidatePaths = [...new Set([ ...githubIssueReadCandidatePaths(preferredPath), @@ -4742,7 +4757,12 @@ export class FactoryLoop implements Factory { if (isMissingIssueFileError(error)) { const parts = githubIssuePathParts(path) ?? githubIssueDirectoryPathParts(path) const identity = parts ? githubIssueIdentity(parts.owner, parts.repo, parts.number) : undefined - if (parts && identity && this.#githubApiFallbackIssues.has(identity) && this.#mount.githubRead) { + const eligible = identity + ? this.#githubApiFallbackIssues.has(identity) || + await this.#deriveGithubApiFallbackEligibility(identity, decisionHint) + : false + if (eligible && identity) this.#rememberGithubApiFallbackEligible(identity) + if (parts && identity && eligible && this.#mount.githubRead) { const lookup = await this.#mount.githubRead.getIssue(`${parts.owner}/${parts.repo}`, parts.number) if (lookup.outcome === 'found') { const githubIssue = parseGithubIssue(lookup.issue.path, lookup.issue.content) @@ -5384,10 +5404,10 @@ export class FactoryLoop implements Factory { return [...pathsByKey.values()].sort() } - async #readIssue(path: string): Promise { + async #readIssue(path: string, decisionHint?: TriageDecision): Promise { try { if (isGithubIssueFilePath(path)) { - const githubIssue = await this.#readGithubIssue(path) + const githubIssue = await this.#readGithubIssue(path, decisionHint) return githubIssue ? githubIssueAsFactoryIssue(githubIssue) : undefined } // Newly-synced issues land as a change-event STUB at the primary @@ -14177,7 +14197,7 @@ export const githubIssuePathParts = (path: string): { owner: string; repo: strin } } -const githubIssueIdentity = (owner: string, repo: string, number: number): string => +export const githubIssueIdentity = (owner: string, repo: string, number: number): string => `${owner.toLowerCase()}/${repo.toLowerCase()}#${number}` const githubIssueRefIdentity = (issue: IssueRef): string | undefined => { @@ -14185,6 +14205,33 @@ const githubIssueRefIdentity = (issue: IssueRef): string | undefined => { return parts ? githubIssueIdentity(parts.owner, parts.repo, parts.number) : undefined } +/** + * Whether a GitHub API fallback read is eligible for `identity`, derived + * from currently-tracked state rather than a separately-remembered flag: + * `decisionHint` (the one read that happens before its own record is + * tracked) or a currently in-flight record whose decision was resolved + * through the fallback. Exported standalone so this is directly testable — + * no caller has to have registered anything for this to return true, which + * is the property that keeps a future read call site from silently + * reintroducing the restart-eligibility bug by omission. + */ +export function isGithubApiFallbackEligible( + inFlight: readonly { issue: IssueRef; decision: TriageDecision }[], + identity: string, + decisionHint?: TriageDecision, +): boolean { + if ( + decisionHint?.issueResolution?.source === 'github-api-fallback' && + githubIssueRefIdentity(decisionHint.issue) === identity + ) { + return true + } + return inFlight.some((record) => + record.decision.issueResolution?.source === 'github-api-fallback' && + githubIssueRefIdentity(record.issue) === identity, + ) +} + const githubIssuePathPreference = (path: string): number => { if (path.endsWith('/meta.json')) return 0 if (path.endsWith('/metadata.json')) return 1 From f6512a1ec59bc11841b5f1caf6979db0db8db6e8 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 14:36:27 +0200 Subject: [PATCH 18/19] fix: stop the round-4 red check from passing on a pre-restart retry race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit attempts incremented before the providerReady gate in both the round-4 API-fallback-publishing red check and its pre-existing Linear sibling ("takes over a persisted publishing phase after the owner stops"), so a failed publish attempt counted regardless of why it failed. The still-running first process's 1s dispatch-lifecycle retry timer (DISPATCH_LIFECYCLE_RETRY_MS) can fire another failing attempt in the window between the 'publishing' phase check and first.stop() resolving, which could push the API-fallback test's attempts to 2 before the restarted process ever read the issue — satisfying the exact absolute count the test asserted without exercising the behavior it exists to prove. The sibling's primary gate (restarted.status().counters.done) is race-immune, but its trailing exact-count assertion was vulnerable to the same race in the opposite direction (a spurious failure if an extra pre-restart retry occurred). Both now baseline attempts immediately after first.stop() and assert the restarted process strictly increased it, rather than asserting an absolute count — correct regardless of how many retries occurred before the restart. Confirmed the corrected API-fallback test still red-checks: reverted #readGithubIssue to the pre-derivation (registration-only) design and reran — fails on attempts never exceeding the pre-restart baseline, timing out as intended. Restored the fix; passes, stable across repeated runs. --- src/orchestrator/factory.test.ts | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index be66a7d..7fb0b02 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -6738,6 +6738,15 @@ describe('FactoryLoop', () => { .toMatchObject({ phase: 'publishing' })) await first.stop() + // attempts increments before the providerReady gate, so a failed + // publish counts too — the still-running first process's 1s retry + // timer (DISPATCH_LIFECYCLE_RETRY_MS) can fire another failing + // attempt in the window between the phase check above and this stop() + // resolving. Baseline after stop() rather than asserting an absolute + // count, so this can't pass on a pre-restart retry that has nothing + // to do with the restarted process. + const attemptsBeforeRestart = attempts + providerReady = true const restartedFleet = new RemoteLifecycleFleetClient() restarted = createFactory(config({ issueSource: 'github' }), { @@ -6753,9 +6762,9 @@ describe('FactoryLoop', () => { // Before the fix: the restarted process's eligibility set is empty and // nothing registers it for the 'publishing' phase, so the read inside // #publishImplementerPullRequest throws "issue is no longer readable" - // before ever calling the publisher — attempts never reaches 2, and - // this waitFor times out. - await vi.waitFor(() => expect(attempts).toBe(2), { timeout: 4_000 }) + // before ever calling the publisher — attempts never increases past + // attemptsBeforeRestart, and this waitFor times out. + await vi.waitFor(() => expect(attempts).toBeGreaterThan(attemptsBeforeRestart), { timeout: 4_000 }) // The lifecycle can race straight past 'published' to 'complete' by // the time this polls — either proves the read (and publish) succeeded. await vi.waitFor(async () => { @@ -7074,6 +7083,13 @@ describe('FactoryLoop', () => { .toMatchObject({ phase: 'publishing' })) await first.stop() + // attempts increments before the providerReady gate, so the + // still-running first process's retry timer could in principle push + // it past 1 in the window before stop() resolves — baseline here + // rather than asserting the post-restart total is exactly 2, so an + // extra pre-restart retry can't turn this into a flaky failure. + const attemptsBeforeRestart = attempts + providerReady = true const restartedFleet = new RemoteLifecycleFleetClient() const restarted = createFactory(config(), { @@ -7085,7 +7101,7 @@ describe('FactoryLoop', () => { }) await restarted.start({ mode: 'dispatch-owner' }) await vi.waitFor(() => expect(restarted.status().counters.done).toBe(1), { timeout: 4_000 }) - expect(attempts).toBe(2) + expect(attempts).toBeGreaterThan(attemptsBeforeRestart) expect(restartedFleet.releases.map((release) => release.name).sort()).toEqual(['ar-485-impl-pear', 'ar-485-review']) await restarted.stop() } finally { From 91add5f1d9f721fc571d809f4a7c77f6ce9c1404 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 9 Aug 2026 15:11:28 +0200 Subject: [PATCH 19/19] fix: widen GitHub API fallback eligibility derivation to durable clarifications and lifecycles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A restarted fallback-backed clarification cancelled a valid human reply instead of resuming: #clarificationIssueStillActive (which gates whether a wake resumes or is cancelled) reads the issue via #readIssue with no hint, and #deriveGithubApiFallbackEligibility only scanned batch.inFlight. The clarification restore path (#restoreClarifications-adjacent code near #resumeWaitingClarification) rebuilds an InFlightIssue locally from the persisted decision purely to re-arm the Slack watcher — it never inserts it into the batch, so the in-flight scan alone missed it, the read failed, the issue was treated as having left factory scope, and the wake was cancelled (clarificationWakesCancelledStaleIssue) — silently discarding the human's answer. Confirmed the stated limitation from 793f252 ("a durably-restored record is always in the batch") is false specifically for this path: a decision can durably outlive the original dispatch call in (at least) three distinct shapes, not one. Widened #deriveGithubApiFallbackEligibility to gather candidates from all three: batch.inFlight (existing), #state.listWaitingClarifications (new — the clarification-parked case), and #state.listDispatchLifecycles filtered to non-terminal (new — a durable dispatch not currently reflected in the in-memory batch at all, e.g. a non-durable-fleet dispatch never reaches the batch either). decisionHint remains a pure optimization ahead of the scan, now also passed at the one clarification call site that already holds its decision (#clarificationIssueStillActive, from #resumeWaitingClarification) — cheap, but not required for correctness, since 20-odd other read sites still pass none and rely entirely on the scan. isGithubApiFallbackEligible's candidate parameter was already source-agnostic (generalized from round 4's "inFlight" naming); added two small pure mapping functions (githubApiFallbackCandidatesFromWaitingClarifications, githubApiFallbackCandidatesFromDispatchLifecycles, the latter excluding terminal lifecycles) so the gathering logic for the two new sources is directly unit-tested in isolation, not only through the full-process red check. Red check: parks a fallback-backed GitHub clarification, restarts with an empty in-memory cache, delivers a human reply directly through the durable state store, and asserts the team resumes (clarificationTeamsWoken reaches 1) rather than the wake being cancelled. Confirmed failing before this fix (the counter never increments, hand-reverted to the round-5 in-flight-only derivation) and passing after, stable across 5 repeated runs. Guard tests: a fallback-backed decision reachable from ONLY the waiting-clarifications source, and from ONLY a non-terminal dispatch lifecycle, both resolve eligible; a terminal (complete/abandoned) lifecycle is excluded from candidates entirely, individually and alongside a non-terminal one for a different issue. --- src/orchestrator/factory.test.ts | 204 ++++++++++++++++++++++++++++++- src/orchestrator/factory.ts | 106 ++++++++++++---- 2 files changed, 286 insertions(+), 24 deletions(-) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 7fb0b02..3bb9a36 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -33,7 +33,8 @@ import type { CloseProbePrInput, GithubMergeGatePort, GithubMergeGateVerdict, Gi import { BatchTracker, issueKey } from './batch-tracker' import { InMemoryStateStore } from '../state/in-memory-state-store' import { FileStateStore } from '../state/file-state-store' -import { dispatchComment, githubIssueIdentity, githubIssuePathParts, githubRepoSubscriptionGlobs, isGithubApiFallbackEligible, keyFromPath, rememberBoundedFallbackEligibility } from './factory' +import { dispatchComment, githubApiFallbackCandidatesFromDispatchLifecycles, githubApiFallbackCandidatesFromWaitingClarifications, githubIssueIdentity, githubIssuePathParts, githubRepoSubscriptionGlobs, isGithubApiFallbackEligible, keyFromPath, rememberBoundedFallbackEligibility } from './factory' +import type { WaitingClarification } from '../ports/state' import { globMatchesPath } from '../subscriptions/globs' import { ResourceSubscriptionsUnavailableError, @@ -1965,6 +1966,95 @@ describe('isGithubApiFallbackEligible', () => { }) }) +describe('github API fallback eligibility candidate gathering', () => { + // Round 6: a fallback-backed decision parked as a waiting clarification is + // reachable from #state.listWaitingClarifications but never inserted into + // the batch, so isGithubApiFallbackEligible alone (scanning only + // batch.inFlight) missed it — #clarificationIssueStillActive treated a + // still-valid issue as gone and cancelled a human's reply instead of + // resuming. The guard: a fallback-backed decision reachable from EACH of + // the three durable sources #deriveGithubApiFallbackEligibility gathers + // resolves eligible once mapped into candidates, so a future path relying + // on any of them is covered by construction, not by having been found. + const path = githubIssueNestedMetaPath('AgentWorkforce', 'pear', 704) + const identity = githubIssueIdentity('AgentWorkforce', 'pear', 704) + const issue = { uuid: 'u-704', key: '704', path } + const fallbackDecision: TriageDecision = { + issue, + routes: [], + scope: 'single', + implementers: [], + reviewer: { name: 'r', role: 'reviewer', capability: 'spawn:claude', model: 'claude', task: 't', repo: 'AgentWorkforce/pear', clonePath: '/work/pear', node: 'self' }, + thin: false, + confidence: 'high', + rationale: 'test', + issueResolution: { + source: 'github-api-fallback', + repo: 'AgentWorkforce/pear', + detail: 'test', + projection: { outcome: 'no-match' }, + }, + } + + it('a fallback-backed decision reachable only as a waiting clarification resolves eligible', () => { + const waiting: WaitingClarification = { + issue, + decision: fallbackDecision, + dryRun: false, + askerName: 'ar-704-impl-pear', + question: 'test question', + askedAtMs: 0, + agents: [], + } + const candidates = githubApiFallbackCandidatesFromWaitingClarifications([['704', waiting]]) + expect(isGithubApiFallbackEligible(candidates, identity)).toBe(true) + }) + + it('a fallback-backed decision reachable only as a non-terminal dispatch lifecycle resolves eligible', () => { + const lifecycle: DispatchLifecycle = { + runId: 'run-704', + issue, + decision: fallbackDecision, + dryRun: false, + phase: 'running', + agents: [], + invocationIds: [], + updatedAtMs: 0, + } + const candidates = githubApiFallbackCandidatesFromDispatchLifecycles([['704', lifecycle]]) + expect(isGithubApiFallbackEligible(candidates, identity)).toBe(true) + }) + + it.each(['complete', 'abandoned'] as const)( + 'excludes a %s dispatch lifecycle — a finished dispatch is not a reason to trust a live read', + (phase) => { + const lifecycle: DispatchLifecycle = { + runId: 'run-704', + issue, + decision: fallbackDecision, + dryRun: false, + phase, + agents: [], + invocationIds: [], + updatedAtMs: 0, + } + const candidates = githubApiFallbackCandidatesFromDispatchLifecycles([['704', lifecycle]]) + expect(candidates).toEqual([]) + expect(isGithubApiFallbackEligible(candidates, identity)).toBe(false) + }, + ) + + it('keeps a non-terminal dispatch lifecycle alongside an excluded terminal one', () => { + const other = { uuid: 'u-705', key: '705', path: githubIssueNestedMetaPath('AgentWorkforce', 'pear', 705) } + const lifecycles: Array<[string, DispatchLifecycle]> = [ + ['704', { runId: 'run-704', issue, decision: fallbackDecision, dryRun: false, phase: 'complete', agents: [], invocationIds: [], updatedAtMs: 0 }], + ['705', { runId: 'run-705', issue: other, decision: { ...fallbackDecision, issue: other }, dryRun: false, phase: 'running', agents: [], invocationIds: [], updatedAtMs: 0 }], + ] + const candidates = githubApiFallbackCandidatesFromDispatchLifecycles(lifecycles) + expect(candidates).toEqual([{ issue: other, decision: { ...fallbackDecision, issue: other } }]) + }) +}) + describe('FactoryLoop', () => { it('sweeps preview orphans on daemon startup using durable active issue owners', async () => { const mount = new FakeMountClient() @@ -15633,6 +15723,118 @@ describe('FactoryLoop', () => { } }) + it('resumes a fallback-backed clarification after a restart instead of cancelling the wake', async () => { + // A parked clarification is rebuilt as a local InFlightIssue purely to + // re-arm its watcher (factory.ts, near #resumeWaitingClarification) — + // it is never inserted into the batch. Before widening derivation to + // consult durable waiting clarifications too, #clarificationIssueStillActive + // (which gates the wake) could not read a fallback-backed issue after a + // restart, treated it as "left factory scope", and cancelled the wake — + // silently discarding a human's answer instead of resuming the team. + const root = await mkdtemp(join(tmpdir(), 'factory-github-clarification-fallback-restart-')) + try { + const watchStatePath = join(root, 'factory-state.json') + const number = 703 + const path = githubIssueNestedMetaPath('AgentWorkforce', 'pear', number) + // #isIssueReady requires 'factory:in-progress' to be ABSENT for + // dispatch to accept the issue as ready, but + // #clarificationIssueStillActive's GitHub "still active" check + // requires it to be PRESENT for the post-answer wake — mirror the + // real dispatch-then-implementing transition (as the Linear restart + // template does via mount.files.set) so a failure here can only be + // the eligibility bug, not a label mismatch. + let currentLabels = ['factory'] + const githubRead: GithubConnectionRead = { + getIssue: vi.fn(async (repo: string, num: number) => + repo === 'AgentWorkforce/pear' && num === number + ? { + outcome: 'found' as const, + issue: { + repo, + number: num, + path, + content: githubIssueFile(number, { state: 'open', labels: currentLabels }), + }, + } + : { outcome: 'not-found' as const }), + } + const mount = Object.assign(new ConfirmRecordingSlackMountClient({}), { githubRead }) + const firstFleet = new FakeFleetClient() + firstFleet.setSessionRef(`ar-${number}-impl-pear`, `session-ar-${number}-impl-pear`) + firstFleet.setSessionRef(`ar-${number}-review-pear`, `session-ar-${number}-review-pear`) + const factoryConfig = config({ issueSource: 'github', slack: slackConfig() }) + const firstFactory = createFactory(factoryConfig, { + mount, + fleet: firstFleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath }), + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + probePrResolver: async () => undefined, + }) + + const triaged = await firstFactory.triageIssue( + parseGithubFactoryIssue(path, githubIssueFile(number, { state: 'open', labels: currentLabels })), + ) + const decision: TriageDecision = { + ...triaged, + issueResolution: { + source: 'github-api-fallback', + repo: 'AgentWorkforce/pear', + detail: 'Relayfile projection could not answer; resolved authoritatively through the GitHub API fallback.', + projection: { outcome: 'no-match' }, + }, + } + await firstFactory.dispatch(decision) + currentLabels = ['factory', 'factory:in-progress'] + firstFleet.emitAgentMessage({ + from: `ar-${number}-impl-pear`, + target: 'factory', + body: `[factory-needs-input] Issue: ${number}\nQuestion: Which durable path?`, + eventId: `agent-question-${number}`, + }) + await vi.waitFor(() => expect(firstFactory.status().counters.agentQuestionTeamsReleased).toBe(1)) + // agentQuestionTeamsReleased alone isn't sufficient synchronization — + // the durable clarification record isn't reliably queryable by a + // fresh FileStateStore instance until delivery also completes. + await vi.waitFor(() => expect(firstFactory.status().counters.clarificationQuestionsDelivered).toBe(1)) + await firstFactory.stop() + + const persisted = new FileStateStore({ batchSize: 2, watchStatePath }) + const [key] = (await persisted.listWaitingClarifications('factory-test'))[0]! + const claimed = await persisted.claimClarificationReply('factory-test', key, { + id: `persisted-answer-${number}`, + text: 'Resume through the stored session refs.', + receivedAtMs: 500, + }) + expect(claimed?.reply?.id).toBe(`persisted-answer-${number}`) + + const restartedFleet = new FakeFleetClient() + const restartedFactory = createFactory(factoryConfig, { + mount, + fleet: restartedFleet, + stateStore: new FileStateStore({ batchSize: 2, watchStatePath }), + triage: new StaticTriage(), + githubWriteback: new RecordingGithubWriteback(), + probePrResolver: async () => undefined, + }) + await restartedFactory.start({ mode: 'live', liveSubscription: { transport: 'subscribe' } }) + + // Before the fix: #clarificationIssueStillActive can't read the issue + // (the restarted process's eligibility set is empty and nothing + // registered this identity), treats it as gone, and cancels the wake + // (clarificationWakesCancelledStaleIssue) instead of resuming — this + // counter never reaches 1 and the waitFor times out. + await vi.waitFor(() => expect(restartedFactory.status().counters.clarificationTeamsWoken).toBe(1), { timeout: 4_000 }) + expect(restartedFleet.resumes.map((resume) => resume.sessionRef).sort()).toEqual([ + `session-ar-${number}-impl-pear`, + `session-ar-${number}-review-pear`, + ]) + await restartedFactory.stop() + } finally { + await rm(root, { recursive: true, force: true }) + } + }, 15_000) + it('dispatch-owner recovers a remote team parked for clarification and preserves its prior dispatch result', async () => { const root = await mkdtemp(join(tmpdir(), 'factory-clarification-dispatch-owner-')) try { diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index cf8540b..221fb05 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -4717,20 +4717,52 @@ export class FactoryLoop implements Factory { * correctly because this checks the actual current state instead of * requiring every entry point to have called a registration method first. * - * `decisionHint` covers the one read that happens before its own record - * exists in the batch (the initial live dispatch, which reads the issue - * to validate scope before it is tracked at all) — everywhere else, the - * durably-restored or live-dispatched record is already in - * `batch.inFlight` (`BatchTracker.restore`/`start` insert it before any - * phase handler runs), so scanning it finds the same `issueResolution` - * a durable-state lookup would, without needing to reconstruct the - * `IssueRef` composite key (uuid/key/path) that a direct durable-store - * lookup would require and that this path alone cannot reliably rebuild - * (the real uuid is built from GitHub's node_id/id when content is - * available; from just a path, only the issue number is derivable). + * A decision can durably outlive the original dispatch call in three + * shapes, and this checks all three — a fourth found later means this + * enumeration is incomplete, not that the approach is wrong: + * 1. `batch.inFlight` — an active live-dispatched or durably-restored + * record (`BatchTracker.restore`/`start` insert it before any phase + * handler runs). + * 2. `#state.listWaitingClarifications` — an issue parked awaiting a + * human reply. Restoring it (`#restoreClarifications` et al.) + * rebuilds an `InFlightIssue`-shaped record locally to re-arm the + * watcher and does not insert it into the batch, so (1) alone misses + * it. + * 3. `#state.listDispatchLifecycles`, non-terminal — a durably-tracked + * dispatch that is not currently in-memory at all yet (e.g. a + * non-durable-fleet dispatch never reaches the batch either). + * + * `decisionHint` stays a pure optimization ahead of all three: the one + * read that happens before its own record exists anywhere (the initial + * live dispatch, validating scope before it is tracked) can skip the + * scan entirely, but no read site is required to pass it — omitting it + * only costs the scan, never correctness. + * + * Deliberately scans by GitHub identity (owner/repo/number) against each + * candidate's own issue path rather than looking up the durable record by + * its composite key (uuid/key/path): the real uuid is built from GitHub's + * node_id/id when content is available, which a bare path cannot + * reconstruct, so a keyed lookup would not reliably match. */ async #deriveGithubApiFallbackEligibility(identity: string, decisionHint?: TriageDecision): Promise { - return isGithubApiFallbackEligible((await this.#batch()).inFlight, identity, decisionHint) + if ( + decisionHint?.issueResolution?.source === 'github-api-fallback' && + githubIssueRefIdentity(decisionHint.issue) === identity + ) { + return true + } + const inFlight = (await this.#batch()).inFlight + if (isGithubApiFallbackEligible(inFlight, identity)) return true + + const [waitingClarifications, dispatchLifecycles] = await Promise.all([ + this.#state.listWaitingClarifications(this.#workspaceId), + this.#state.listDispatchLifecycles(this.#workspaceId), + ]) + const durable = [ + ...githubApiFallbackCandidatesFromWaitingClarifications(waitingClarifications), + ...githubApiFallbackCandidatesFromDispatchLifecycles(dispatchLifecycles), + ] + return isGithubApiFallbackEligible(durable, identity) } async #readGithubIssue(path: string, decisionHint?: TriageDecision): Promise { @@ -12631,7 +12663,7 @@ export class FactoryLoop implements Factory { heartbeat.unref?.() try { - if (!await this.#clarificationIssueStillActive(waiting.issue)) { + if (!await this.#clarificationIssueStillActive(waiting.issue, waiting.decision)) { this.#assertClarificationWakeRunning() await renewLease() const completed = await this.#state.completeClarificationWake(this.#workspaceId, key, this.#clarificationWakeOwner) @@ -12815,8 +12847,8 @@ export class FactoryLoop implements Factory { if (this.#stopping) throw new ClarificationWakeStoppedError('factory is stopping') } - async #clarificationIssueStillActive(issueRef: IssueRef): Promise { - const issue = await this.#readIssue(issueRef.path) + async #clarificationIssueStillActive(issueRef: IssueRef, decisionHint?: TriageDecision): Promise { + const issue = await this.#readIssue(issueRef.path, decisionHint) if (!issue || !isInFactoryScope(issue, this.#config.safety) || !isDispatchableIssue(issue)) { this.#logger.info?.('[factory] clarification wake cancelled because issue left factory scope', { issue: issueRef.key, @@ -14209,14 +14241,22 @@ const githubIssueRefIdentity = (issue: IssueRef): string | undefined => { * Whether a GitHub API fallback read is eligible for `identity`, derived * from currently-tracked state rather than a separately-remembered flag: * `decisionHint` (the one read that happens before its own record is - * tracked) or a currently in-flight record whose decision was resolved - * through the fallback. Exported standalone so this is directly testable — - * no caller has to have registered anything for this to return true, which - * is the property that keeps a future read call site from silently + * tracked anywhere) or any `{issue, decision}` candidate — drawn from + * whichever durable or in-memory store the caller has gathered — whose + * decision was resolved through the fallback. Exported standalone so this + * is directly testable — no caller has to have registered anything for + * this to return true for a candidate that is genuinely present, which is + * the property that keeps a future read call site from silently * reintroducing the restart-eligibility bug by omission. + * + * A decision durably outlives the in-memory batch in more than one shape + * (in-flight dispatch, a parked clarification, a durable dispatch + * lifecycle — see #deriveGithubApiFallbackEligibility for the full list + * this class gathers); this function does not care which shape a + * candidate came from, only whether one matches. */ export function isGithubApiFallbackEligible( - inFlight: readonly { issue: IssueRef; decision: TriageDecision }[], + candidates: readonly { issue: IssueRef; decision: TriageDecision }[], identity: string, decisionHint?: TriageDecision, ): boolean { @@ -14226,12 +14266,32 @@ export function isGithubApiFallbackEligible( ) { return true } - return inFlight.some((record) => - record.decision.issueResolution?.source === 'github-api-fallback' && - githubIssueRefIdentity(record.issue) === identity, + return candidates.some((candidate) => + candidate.decision.issueResolution?.source === 'github-api-fallback' && + githubIssueRefIdentity(candidate.issue) === identity, ) } +/** Maps durable waiting-clarification records to isGithubApiFallbackEligible candidates. */ +export function githubApiFallbackCandidatesFromWaitingClarifications( + waitingClarifications: ReadonlyArray, +): Array<{ issue: IssueRef; decision: TriageDecision }> { + return waitingClarifications.map(([, waiting]) => ({ issue: waiting.issue, decision: waiting.decision })) +} + +/** + * Maps durable dispatch-lifecycle records to isGithubApiFallbackEligible + * candidates, excluding terminal ones — a completed or abandoned dispatch's + * decision is no longer a reason to trust a live read for that issue. + */ +export function githubApiFallbackCandidatesFromDispatchLifecycles( + dispatchLifecycles: ReadonlyArray, +): Array<{ issue: IssueRef; decision: TriageDecision }> { + return dispatchLifecycles + .filter(([, lifecycle]) => !isTerminalDispatchLifecycle(lifecycle)) + .map(([, lifecycle]) => ({ issue: lifecycle.issue, decision: lifecycle.decision })) +} + const githubIssuePathPreference = (path: string): number => { if (path.endsWith('/meta.json')) return 0 if (path.endsWith('/metadata.json')) return 1