diff --git a/.agent-notes/factory-dispatch-unblock.md b/.agent-notes/factory-dispatch-unblock.md new file mode 100644 index 0000000..111819c --- /dev/null +++ b/.agent-notes/factory-dispatch-unblock.md @@ -0,0 +1,54 @@ +# 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. + +## 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. +- 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. + +## 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. +- 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. + +## 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. diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 0525462..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 { 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' +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,11 +113,35 @@ 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 } }, }, }) +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 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) }) + const mountWithIntegrationConnections = ( files: Record, integrationConnections: FactoryIntegrationConnections, @@ -454,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 }, @@ -743,6 +855,93 @@ describe('fleet CLI runtime', () => { } }) + 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' }) + 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('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) => githubIssueFound('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 { @@ -1560,6 +1759,476 @@ 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 + ? githubIssueFound('pear', number) + : githubIssueNotFound(), + ) + 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 () => githubIssueNotFound()) + const errors = buffer() + + const code = await runFleetCli(['triage', '999999', '--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('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('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 { + 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) => + githubIssueFound('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, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), + 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) => + githubIssueFound('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, + getLocalMountHealth: () => ({ degraded: true, reason: 'projection reconcile is stale' }), + }), + 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('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 () => githubIssueFound('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' ? githubIssueFound('factory', number) : githubIssueNotFound(), + ) + 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('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' ? githubIssueFound('cloud', number) : githubIssueNotFound(), + ) + 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('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, 'OtherOrg'), + }, + } + : 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 63fd480..3033dbf 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 @@ -1439,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, @@ -1637,21 +1666,65 @@ 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 selector = parseGithubIssueSelector(issueArg, config) + const projection = projectionStatus + ? await projectionStatus() + : projectionStatusFromMount(mount) + 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`) + } + return { + issue: parseGithubFactoryIssue(fallback.path, fallback.content), + resolution: { + source: 'github-api-fallback', + repo: fallback.repo, + detail: `Relayfile projection could not answer (${unavailableReason}); resolved authoritatively through the GitHub API fallback.`, + 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) { - 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; ` + @@ -1678,7 +1751,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,14 +1782,144 @@ async function findIssuePath(mount: MountClient, key: string, config: FactoryCon return matches[0] } -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), - ] +async function findGithubIssueThroughConnection( + mount: MountClient, + selector: GithubIssueSelector, + config: FactoryConfig, + issueArg: string, +) { + const github = mount.githubRead + if (!github) { + throw new Error( + `${githubIssueResolutionError(config, issueArg)}: projection cannot answer and the GitHub API fallback is unavailable`, + ) + } + const repos = selector.repo ? [selector.repo] : configuredGithubIssueRepos(config) + if (repos.length === 0) { + throw new Error( + `${githubIssueResolutionError(config, issueArg)}: projection cannot answer and no configured owner/repo is available for the GitHub API fallback`, + ) + } + 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 + } + 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( + factory: Factory, + mount: MountClient, + config: FactoryConfig, +): Promise { + const status = await factoryStatusWithMountHealth( + factory, + mount, + 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 } : {}), + } +} + +function projectionStatusFromMount(mount: MountClient): IssueResolution['projection'] { + const health = mount.getLocalMountHealth?.() + return { + outcome: 'no-match', + ...(health ? { localMountDegraded: health.degraded } : {}), + ...(health?.reason ? { localMountDegradedReason: health.reason } : {}), + } +} + +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' + } + if (projection.eventListener && !['subscribed', 'polling'].includes(projection.eventListener.state)) { + return projection.eventListener.reason ?? `event listener is ${projection.eventListener.state}` + } + return undefined +} + +export type GithubIssueSelector = { number: number; repo?: string } + +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]) + 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 = allConfiguredGithubIssueRepos(config) + // 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`, + ) + } + return { number, repo } +} + +function resolveGithubIssueRepoCandidates(config: FactoryConfig, candidates: string[]): string[] { const repos = new Map() const routedRepos = [ ...Object.values(config.repos.byLabel), @@ -1741,6 +1944,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 || diff --git a/src/index.ts b/src/index.ts index b5a4e9b..17ea22a 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 { GithubApiIssueRead } from './mount/github-api-issue-read' export { ensureFactoryIntegrations, inspectFactoryIntegration, @@ -295,6 +296,9 @@ export type { Clock, EventPage, GithubConnectionWrite, + GithubConnectionIssue, + GithubConnectionRead, + GithubIssueLookup, FactoryIntegrationConnectionStatus, FactoryIntegrationConnections, FactoryIntegrationConnectResult, @@ -438,6 +442,7 @@ export type { FactoryStartOptions, FactoryStatus, IssueRef, + IssueResolution, IterationReport, LinearIssue, ProbeCloser, 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..991d68b --- /dev/null +++ b/src/mount/github-api-issue-read.test.ts @@ -0,0 +1,173 @@ +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 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({ + 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', + expect.objectContaining({ method: 'GET' }), + ) + }) + + it('returns not-found for a 404 against a repo it has confirmed is public', async () => { + const reader = new GithubApiIssueRead({ + 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.toEqual({ outcome: 'not-found' }) + }) + + 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 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 () => { + 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 500)', + ) + }) + + it('treats a pull request returned by the issues endpoint as an authoritative issue miss', async () => { + const reader = new GithubApiIssueRead({ + 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.toEqual({ outcome: 'not-found' }) + }) +}) diff --git a/src/mount/github-api-issue-read.ts b/src/mount/github-api-issue-read.ts new file mode 100644 index 0000000..f3e4b41 --- /dev/null +++ b/src/mount/github-api-issue-read.ts @@ -0,0 +1,193 @@ +import type { GithubIssueLookup, 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 +} + +/** + * Read-only lookup against GitHub's REST API, unauthenticated. + * + * 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 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 { + 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`, + } + } + 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}`, + { + method: 'GET', + signal: AbortSignal.timeout(30_000), + headers: { + accept: 'application/vnd.github+json', + 'user-agent': '@agent-relay/factory', + }, + }, + ) + if (response.status === 404) { + // 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}` } + } + 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}`) + } + // 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 { outcome: 'not-found' } + const resolvedNumber = positiveInteger(issue.number) + const title = stringValue(issue.title) + const url = stringValue(issue.html_url) + if (resolvedNumber !== number || !title || !url) { + 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 { + 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) { + 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.ts b/src/mount/relayfile-cloud-mount-client.ts index 0d61125..2e04912 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 { GithubApiIssueRead } from './github-api-issue-read' import { ensureLocalMount as runLocalMountPreflight, type EnsureLocalMountOptions, @@ -257,6 +259,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 +340,7 @@ export class RelayfileCloudMountClient implements MountClient { resolveRegisteredWorkspaceMirror(workspaceIds)?.localDir) this.#isAllowedDraft = config.isAllowedDraft this.#isAllowedDelete = config.isAllowedDelete + this.githubRead = new GithubApiIssueRead() this.githubWrite = new RelayfileGithubConnectionWrite({ mount: this }) this.integrationConnections = relayfileIntegrationConnections( config.relayfileWorkspace, diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 6ff9061..3bb9a36 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -27,13 +27,14 @@ 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 { githubIssuePathParts, githubRepoSubscriptionGlobs, keyFromPath } 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, @@ -1776,6 +1777,284 @@ 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('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('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('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() @@ -6395,6 +6674,200 @@ 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('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() + + // 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' }), { + 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 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 () => { + 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') @@ -6700,6 +7173,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(), { @@ -6711,7 +7191,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 { @@ -15243,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 927ec21..221fb05 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 { @@ -412,6 +453,8 @@ export class FactoryLoop implements Factory { readonly #githubIssueAuthors = new Map() 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>() @@ -2388,7 +2431,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) @@ -2424,7 +2469,11 @@ export class FactoryLoop implements Factory { throw error } - 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) @@ -3987,7 +4036,11 @@ export class FactoryLoop implements Factory { async #resumeDurableDispatch(record: InFlightIssue): Promise { let liveIssue: LinearIssue | undefined if (!record.dryRun) { - 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`) } @@ -4647,7 +4700,72 @@ export class FactoryLoop implements Factory { } } - async #readGithubIssue(path: string): Promise { + #rememberGithubApiFallbackEligible(identity: string): void { + rememberBoundedFallbackEligibility( + this.#githubApiFallbackIssues, + this.#githubApiFallbackIssuesEvicted, + identity, + GITHUB_API_FALLBACK_ISSUES_MAX, + GITHUB_API_FALLBACK_ISSUES_EVICTED_MAX, + ) + } + + /** + * `#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. + * + * 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 { + 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 { const preferredPath = await this.#preferredGithubIssuePath(path) const candidatePaths = [...new Set([ ...githubIssueReadCandidatePaths(preferredPath), @@ -4669,8 +4787,54 @@ export class FactoryLoop implements Factory { } } catch (error) { if (isMissingIssueFileError(error)) { + const parts = githubIssuePathParts(path) ?? githubIssueDirectoryPathParts(path) + const identity = parts ? githubIssueIdentity(parts.owner, parts.repo, parts.number) : undefined + 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) + this.#indexDependencyIssue(githubIssueAsFactoryIssue(githubIssue)) + this.#logger.warn?.('[factory] Relayfile projection missed GitHub issue; using GitHub API fallback', { + 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 + } + } + 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 file discovered from issue tree', { path }) + this.#logger.debug?.('[factory] skipped missing GitHub issue after projection and provider lookup', { path }) return undefined } throw error @@ -5272,10 +5436,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 @@ -12499,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) @@ -12683,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, @@ -13431,8 +13595,17 @@ 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}` + : 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'}`, @@ -14056,7 +14229,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 => { @@ -14064,6 +14237,61 @@ 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 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( + candidates: readonly { issue: IssueRef; decision: TriageDecision }[], + identity: string, + decisionHint?: TriageDecision, +): boolean { + if ( + decisionHint?.issueResolution?.source === 'github-api-fallback' && + githubIssueRefIdentity(decisionHint.issue) === identity + ) { + return true + } + 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 diff --git a/src/ports/index.ts b/src/ports/index.ts index 90fb19c..cf9315f 100644 --- a/src/ports/index.ts +++ b/src/ports/index.ts @@ -5,7 +5,10 @@ export type { FactoryIntegrationConnections, FactoryIntegrationConnectResult, FactoryIntegrationProvider, + GithubConnectionIssue, + GithubConnectionRead, GithubConnectionWrite, + GithubIssueLookup, GithubPublishPullRequestInput, GithubPublishPullRequestResult, LocalMountHealth, diff --git a/src/ports/mount.ts b/src/ports/mount.ts index ab142a7..919684a 100644 --- a/src/ports/mount.ts +++ b/src/ports/mount.ts @@ -71,6 +71,37 @@ export interface GithubPublishPullRequestResult { author?: string } +/** + * 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 + number: number + path: string + 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 { + getIssue(repo: string, number: number): Promise +} + export type FactoryIntegrationProvider = 'github' | 'linear' export interface FactoryIntegrationConnectionStatus { @@ -104,6 +135,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..887aaca 100644 --- a/src/triage/schema.ts +++ b/src/triage/schema.ts @@ -23,6 +23,25 @@ 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(), + githubConnection: z.object({ + ready: z.boolean(), + state: z.string().optional(), + initialSyncState: 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..fc6be8f 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,23 @@ 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 + githubConnection?: { + ready: boolean + state?: string + initialSyncState?: string + } + } +} + export interface PrSummary { repo: string number: number