fix(factory): fall back to direct GitHub API when the Relayfile issue projection is stale - #225
fix(factory): fall back to direct GitHub API when the Relayfile issue projection is stale#225khaliqgant wants to merge 8 commits into
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 58 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cff7303c6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| headers: { | ||
| accept: 'application/vnd.github+json', | ||
| 'user-agent': '@agent-relay/factory', | ||
| }, |
There was a problem hiding this comment.
When the configured repository is private, this request carries no GitHub authentication, so GitHub returns the same 404 used for nonexistent resources; line 44 then converts that response to undefined, and triage/dispatch incorrectly reports zero matches. Because every RelayfileCloudMountClient receives this reader even when its connected GitHub App can access private repositories, the new stale-projection recovery path cannot recover private issues.
Useful? React with 👍 / 👎.
| function configuredGithubIssueRepos(config: FactoryConfig): string[] { | ||
| const candidates = config.repos.default | ||
| ? [config.repos.default] |
There was a problem hiding this comment.
When a multi-repository config has repos.default, this reduces the configured repository set to only that default. parseGithubIssueSelector reuses this set to validate explicit repo#number selectors, so a configured non-default route such as cloud#222 is rejected as unconfigured even if it appears in byLabel, byProject, or keywordRules. Keep the default-only behavior for bare numbers, but validate qualified selectors against all configured routes.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/orchestrator/factory.ts">
<violation number="1" location="src/orchestrator/factory.ts:2432">
P1: Relay lifecycle recovery cannot re-read an API-fallback issue after Factory restarts, so it throws “issue is not currently readable” while the projection remains stale. Restore fallback eligibility from `record.decision.issueResolution` before recovery reads, rather than relying solely on this process-local set.</violation>
<violation number="2" location="src/orchestrator/factory.ts:2432">
P3: The new `#githubApiFallbackIssues` set is append-only and never pruned. Every GitHub-API-fallback dispatch permanently adds an issue identity for the lifetime of the FactoryLoop. In the long-running daemon (`factory start --mode live`) this set grows without bound and, because it is consulted on every `#readGithubIssue` missing-file path, it also keeps those identities eligible for re-fetch through `githubRead` indefinitely. Consider bounding it (e.g. track per-issue with an LRU, clear after terminal dispatch, or scope it to the current run) so stale fallback records don't accumulate for the process lifetime.</violation>
</file>
<file name="src/cli/fleet.test.ts">
<violation number="1" location="src/cli/fleet.test.ts:761">
P3: The test name contradicts its assertions: it is titled as if the GitHub API fallback resolves the issue, yet it verifies the projection is preferred (`source: relayfile-projection`, and `githubRead.getIssue` is not called). Rename it to reflect that a populated projection stays primary even when the connection is not ready (e.g. 'keeps a populated projection preferred over the API fallback when the connection is not ready').</violation>
</file>
<file name="src/mount/github-api-issue-read.ts">
<violation number="1" location="src/mount/github-api-issue-read.ts:44">
P2: This fallback makes unauthenticated GETs to api.github.com (no Authorization header/token is attached), and treats any HTTP 404 as an *authoritative* issue miss. For private repositories — which is precisely the org-wide Factory scenario this PR targets — GitHub deliberately returns 404 for unauthenticated access to hide existence, so a real issue in a private repo would be reported as 'not found' and the phantom-skip path would silently drop it. Since the object is described as 'provider-authoritative' / 'authoritative 404', this misclassification is misleading. Consider distinguishing a truly-missing issue from an inaccessible (private) repo, or degrading to a non-authoritative 'cannot determine' rather than a confident miss.</violation>
</file>
<file name="src/cli/fleet.ts">
<violation number="1" location="src/cli/fleet.ts:1878">
P2: parseGithubIssueSelector validates qualified `repo#number` selectors against `configuredGithubIssueRepos(config)`, which collapses to only `repos.default` when that option is set. This means a repo-qualified selector for any other configured route (present in `byLabel`, `byProject`, or `keywordRules`) is incorrectly rejected as unconfigured, even though it's a valid dispatch target. Qualified selectors should be validated against the full set of configured routes, not just the default.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| if (decision.issueResolution?.source === 'github-api-fallback') { | ||
| const parts = githubIssuePathParts(decision.issue.path) ?? githubIssueDirectoryPathParts(decision.issue.path) | ||
| if (parts) this.#githubApiFallbackIssues.add(githubIssueIdentity(parts.owner, parts.repo, parts.number)) |
There was a problem hiding this comment.
P1: Relay lifecycle recovery cannot re-read an API-fallback issue after Factory restarts, so it throws “issue is not currently readable” while the projection remains stale. Restore fallback eligibility from record.decision.issueResolution before recovery reads, rather than relying solely on this process-local set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/orchestrator/factory.ts, line 2432:
<comment>Relay lifecycle recovery cannot re-read an API-fallback issue after Factory restarts, so it throws “issue is not currently readable” while the projection remains stale. Restore fallback eligibility from `record.decision.issueResolution` before recovery reads, rather than relying solely on this process-local set.</comment>
<file context>
@@ -2424,6 +2427,10 @@ export class FactoryLoop implements Factory {
+ if (decision.issueResolution?.source === 'github-api-fallback') {
+ const parts = githubIssuePathParts(decision.issue.path) ?? githubIssueDirectoryPathParts(decision.issue.path)
+ if (parts) this.#githubApiFallbackIssues.add(githubIssueIdentity(parts.owner, parts.repo, parts.number))
+ }
const liveIssue = await this.#readIssue(decision.issue.path)
</file context>
| }, | ||
| }, | ||
| ) | ||
| if (response.status === 404) return undefined |
There was a problem hiding this comment.
P2: This fallback makes unauthenticated GETs to api.github.com (no Authorization header/token is attached), and treats any HTTP 404 as an authoritative issue miss. For private repositories — which is precisely the org-wide Factory scenario this PR targets — GitHub deliberately returns 404 for unauthenticated access to hide existence, so a real issue in a private repo would be reported as 'not found' and the phantom-skip path would silently drop it. Since the object is described as 'provider-authoritative' / 'authoritative 404', this misclassification is misleading. Consider distinguishing a truly-missing issue from an inaccessible (private) repo, or degrading to a non-authoritative 'cannot determine' rather than a confident miss.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mount/github-api-issue-read.ts, line 44:
<comment>This fallback makes unauthenticated GETs to api.github.com (no Authorization header/token is attached), and treats any HTTP 404 as an *authoritative* issue miss. For private repositories — which is precisely the org-wide Factory scenario this PR targets — GitHub deliberately returns 404 for unauthenticated access to hide existence, so a real issue in a private repo would be reported as 'not found' and the phantom-skip path would silently drop it. Since the object is described as 'provider-authoritative' / 'authoritative 404', this misclassification is misleading. Consider distinguishing a truly-missing issue from an inaccessible (private) repo, or degrading to a non-authoritative 'cannot determine' rather than a confident miss.</comment>
<file context>
@@ -0,0 +1,116 @@
+ },
+ },
+ )
+ if (response.status === 404) return undefined
+ if (!response.ok) {
+ throw new Error(`GitHub API issue lookup failed (HTTP ${response.status})`)
</file context>
| if (!qualified) return { number } | ||
|
|
||
| const requested = qualified[1]! | ||
| const configured = configuredGithubIssueRepos(config) |
There was a problem hiding this comment.
P2: parseGithubIssueSelector validates qualified repo#number selectors against configuredGithubIssueRepos(config), which collapses to only repos.default when that option is set. This means a repo-qualified selector for any other configured route (present in byLabel, byProject, or keywordRules) is incorrectly rejected as unconfigured, even though it's a valid dispatch target. Qualified selectors should be validated against the full set of configured routes, not just the default.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/fleet.ts, line 1878:
<comment>parseGithubIssueSelector validates qualified `repo#number` selectors against `configuredGithubIssueRepos(config)`, which collapses to only `repos.default` when that option is set. This means a repo-qualified selector for any other configured route (present in `byLabel`, `byProject`, or `keywordRules`) is incorrectly rejected as unconfigured, even though it's a valid dispatch target. Qualified selectors should be validated against the full set of configured routes, not just the default.</comment>
<file context>
@@ -1709,6 +1782,116 @@ async function findIssuePath(mount: MountClient, key: string, config: FactoryCon
+ if (!qualified) return { number }
+
+ const requested = qualified[1]!
+ const configured = configuredGithubIssueRepos(config)
+ const expanded = requested.includes('/')
+ ? requested
</file context>
|
|
||
| if (decision.issueResolution?.source === 'github-api-fallback') { | ||
| const parts = githubIssuePathParts(decision.issue.path) ?? githubIssueDirectoryPathParts(decision.issue.path) | ||
| if (parts) this.#githubApiFallbackIssues.add(githubIssueIdentity(parts.owner, parts.repo, parts.number)) |
There was a problem hiding this comment.
P3: The new #githubApiFallbackIssues set is append-only and never pruned. Every GitHub-API-fallback dispatch permanently adds an issue identity for the lifetime of the FactoryLoop. In the long-running daemon (factory start --mode live) this set grows without bound and, because it is consulted on every #readGithubIssue missing-file path, it also keeps those identities eligible for re-fetch through githubRead indefinitely. Consider bounding it (e.g. track per-issue with an LRU, clear after terminal dispatch, or scope it to the current run) so stale fallback records don't accumulate for the process lifetime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/orchestrator/factory.ts, line 2432:
<comment>The new `#githubApiFallbackIssues` set is append-only and never pruned. Every GitHub-API-fallback dispatch permanently adds an issue identity for the lifetime of the FactoryLoop. In the long-running daemon (`factory start --mode live`) this set grows without bound and, because it is consulted on every `#readGithubIssue` missing-file path, it also keeps those identities eligible for re-fetch through `githubRead` indefinitely. Consider bounding it (e.g. track per-issue with an LRU, clear after terminal dispatch, or scope it to the current run) so stale fallback records don't accumulate for the process lifetime.</comment>
<file context>
@@ -2424,6 +2427,10 @@ export class FactoryLoop implements Factory {
+ if (decision.issueResolution?.source === 'github-api-fallback') {
+ const parts = githubIssuePathParts(decision.issue.path) ?? githubIssueDirectoryPathParts(decision.issue.path)
+ if (parts) this.#githubApiFallbackIssues.add(githubIssueIdentity(parts.owner, parts.repo, parts.number))
+ }
const liveIssue = await this.#readIssue(decision.issue.path)
</file context>
| } | ||
| }) | ||
|
|
||
| it('allows targeted GitHub resolution through the API seam when the connected projection is not ready', async () => { |
There was a problem hiding this comment.
P3: The test name contradicts its assertions: it is titled as if the GitHub API fallback resolves the issue, yet it verifies the projection is preferred (source: relayfile-projection, and githubRead.getIssue is not called). Rename it to reflect that a populated projection stays primary even when the connection is not ready (e.g. 'keeps a populated projection preferred over the API fallback when the connection is not ready').
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/fleet.test.ts, line 761:
<comment>The test name contradicts its assertions: it is titled as if the GitHub API fallback resolves the issue, yet it verifies the projection is preferred (`source: relayfile-projection`, and `githubRead.getIssue` is not called). Rename it to reflect that a populated projection stays primary even when the connection is not ready (e.g. 'keeps a populated projection preferred over the API fallback when the connection is not ready').</comment>
<file context>
@@ -743,6 +758,93 @@ describe('fleet CLI runtime', () => {
}
})
+ it('allows targeted GitHub resolution through the API seam when the connected projection is not ready', async () => {
+ const root = await mkdtemp(join(tmpdir(), 'fleet-cli-integration-github-fallback-'))
+ try {
</file context>
| it('allows targeted GitHub resolution through the API seam when the connected projection is not ready', async () => { | |
| it('keeps a populated projection preferred over the API fallback when the connection is not ready', async () => { |
1 participant
Why
Factory currently dispatches nothing org-wide, and the cause is not in Factory's routing.
Targeted issue resolution is projection-only:
runFactoryCommand→readIssueArg→findIssuePathtries canonical repo-scoped paths, lists the configured Relayfile GitHub issue roots, and throws on zero matches before triage can run. When the Relayfile projection is stale, every lookup misses andfactory triagereportsfound 0 matches— which reads identically to "there is no such issue".The projection has been frozen since 2026-08-03T07:26Z. So the failure mode is silent: Factory is healthy, its routing is correct, and it dispatches nothing.
What changed
A read-only direct GitHub REST fallback, used only after the projection has demonstrably failed.
subscribed/polling.repo#numberandowner/repo#numberselectors make a targeted fallback exactly one authoritative lookup instead of an ambiguous org-wide probe.issueResolution; fallback records also carrylocalMountDegraded,localMountDegradedReason, andeventListener, so a decision states which source answered it.Verification
Live CLI through
runFleetCli:factory#222→ exit 0, sourcegithub-api-fallback, projectionno-match, routed only toAgentWorkforce/factory.factory#999999→ exit 1, no decision emitted. Dry-run dispatch of an issue missing both Factory markers → exit 1, no dispatch; dry-run dispatch offactory#222→ exit 0, carryinggithub-api-fallbackin both the result and its dispatch comment.Red-checked in both directions rather than assumed:
Focused build and suites: exit 0, 158 tests passed.
Known caveats, stated rather than buried
600 < 500) fails in isolation and is pre-existing — not introduced here.Forbidden— that route additionally requires a deployed sponsor persona, which a local Factory workspace join is not. Replaced with the read-only direct REST client. GitHub writes remain on Relayfile app-authored writeback; Factory never handles a GitHub token.Scope
No queue, Cloudflare, mount, daemon or launchd mutation. No default-branch push. Read-only on the GitHub side.
Authored by the
factory-dispatch-api-fallbacklane, which completed the work and red-checks but could not open a PR: sanctioned Relayfile app-authored write returned HTTP 403, and it correctly declined to shell out toghinstead. Opened on its behalf; that 403 is a real gap worth fixing separately — a lane that cannot publish its own work is blocked no matter how finished the work is.🤖 Generated with Claude Code
Summary by cubic
Restores Factory triage/dispatch by adding a read‑only GitHub REST fallback when the Relayfile issue projection is stale or unavailable. Projection remains preferred; healthy misses still fail without calling GitHub.
Bug Fixes
GithubApiIssueReadand wired it into the cloud mount for read-only lookups; all writes remain on Relayfile writeback; added focused tests across CLI, mount, and orchestrator.Migration
Written for commit cff7303. Summary will update on new commits.