Repository navigation
🗂️ feat: Record Lane Git State and Serve Conversation Pull Requests - #16793
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: e5bf939561
ℹ️ 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".
| subagentControlHandler, | ||
| ); | ||
| router.get('/:parentConversationId/subagents', parentSubagentIndexHandler); | ||
| router.get('/:conversationId/pull-request', conversationPullRequestHandler); |
There was a problem hiding this comment.
This registers the backend capability, but a repo-wide search of client and packages/client finds no caller of getConversationPullRequest/conversationPullRequest and no pull-request header rendering. Consequently enabling the documented setting cannot show anything to users; add the header query and its loading, empty, success, and failure states before exposing the route.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR is the second of three stacked ones on purpose. The header consumer is #16794, which is based on this branch: it adds the query hook, the chip and card, and the loading, empty, success and failure states, and uses getConversationPullRequest. The route is inert until endpoints.agents.pullRequests is enabled, which defaults to off, so nothing is exposed before the consumer lands. Leaving this open for you to decide whether the stack is acceptable or should be one PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24f1295ceb
ℹ️ 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".
A conversation now stores the branch, head and repository its code lane last reported, written only by the server after a finished bash command and excluded from ordinary reads, generic saves, unset fields and imports. GET /api/convos/:conversationId/pull-request finds the pull request for that branch through an injected GitHub source with a bounded cache, answers null for an absence, and maps failures to stable codes without upstream text. The feature is off unless endpoints.agents.pullRequests is enabled with a token reference.
Scope the lookup cache, shared in-flight request and rate limit cooldown to the credential, so one tenant's token never serves another. Ask for open pull requests on their own before falling back to the newest closed one, read every page of check runs up to a bound and never report an incomplete rollup as passing, order lane writes per conversation so an older write cannot land last, and name both required token permissions in the example config.
24f1295 to
2415224
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2415224389
ℹ️ 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".
Clear the stored lane when the conversation moves to another workspace or leaves attached execution, so the previous workspace's pull request is never served. Fence the lane write on the time it was reported, so an older report from another replica cannot replace a newer one. Honor GitHub's reset window up to an hour instead of ten minutes, and expose the request timeout, whole lookup timeout and check run page limit as endpoints.agents.pullRequests settings with the previous values as defaults.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d831aa3b01
ℹ️ 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".
Require an administrator allowlist of repositories before the server token is used, since a worker reports its own repository. Match a pull request to the commit the conversation last ran at, so a reused branch name does not return another pull request. Fence the lane write on the workspace the command ran in, so a write queued before a move cannot bring the old lane back, and record a subagent thread's lane on the visible root conversation. Key the lookup cache and shared request by the recorded commit and the lookup policy, and document the worker flag in the example config.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c305a0c633
ℹ️ 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".
| const chosen = | ||
| (await choose(open)) ?? | ||
| (await choose((await listPulls('closed', recorded == null ? 1 : 10)) ?? [])); |
There was a problem hiding this comment.
When a restored conversation refers to an older incarnation of a branch that has since produced more than ten closed PRs, the associated PR is never fetched; even among fetched non-exact heads, MAX_HEAD_COMPARISONS stops checking after three. The fresh evidence after the earlier pagination thread is that recorded-head matching now depends on searching closed history while both search bounds remain hard-coded, so the endpoint can return null despite a PR whose head contains the recorded commit. Paginate within a configured lookup budget and expose the new search bound in config.
AGENTS.md reference: AGENTS.md:L90-L92
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partly fixed in ab87dd9, and I want to be exact about the limit. The candidates listed per state (maxCandidatePullRequests, default 10, up to 100) and the candidates compared (maxHeadComparisons, default 3, up to 20) are now endpoints.agents.pullRequests settings, are part of the cache key, and are documented in the example config. I did not add unbounded pagination: a branch name reused more times than the configured bounds can still return no pull request, because the whole lookup is held to lookupTimeoutSeconds and each comparison is a request. Raising those settings is the operator's trade. Leaving this open if you want pagination added instead.
| export function createGitHubPullRequestSource({ | ||
| fetchFn = fetch, | ||
| apiBase = GITHUB_API_BASE, | ||
| }: { fetchFn?: PullRequestFetch; apiBase?: string } = {}): PullRequestSource { |
There was a problem hiding this comment.
This new integration silently falls back to the process-global fetch, and the production route constructs it without supplying a transport, so the package owns an undeclared outbound client instead of receiving the deployment's HTTP implementation at composition time. Make fetchFn required and inject it from the route so proxying, instrumentation, and alternate transports do not require changing the integration itself.
AGENTS.md reference: AGENTS.md:L93-L99
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partly fixed in ab87dd9: fetchFn is now a required argument of createGitHubPullRequestSource, so the package no longer reaches for a global, and the route supplies it. The route passes the global fetch, so this does not add proxy support; routing GitHub calls through the deployment's proxy dispatcher would be a separate change to that one argument. Leaving this open for you to say whether that is wanted here.
Replace wall clock ordering of lane reports with a per conversation counter reserved in the database when the command settles, so replica clock skew cannot reject a newer report. Treat a pull request whose head is behind the recorded commit as no match, and expose the candidate and comparison bounds of the history search as settings. Read the app config and the owner scoped lane together, inject the HTTP transport from the route instead of falling back to a global, skip a report identical to the last one written, and name the Contents permission that commit comparison needs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab87dd99b5
ℹ️ 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".
…or the Current Head Order lane report reservations by the conversation the write lands on, so two subagent threads of one conversation share a chain, and write every report instead of skipping a repeat, since another recorder may have changed the lane in between. Read a pull request's detail before its check runs and use the head it reports, so line counts, mergeability and checks describe one commit. Treat an expired temporary chat as gone when reserving, writing and reading the lane, and make the shared cache capacity a setting.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15cdb7af61
ℹ️ 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".
…e Reporting Strip dotted descendants of lane fields from saves and imports, partition the pull request cache per credential, resolve the lane write target when the tool is created, and fence reports with a codeAttachmentEpoch that advances only on an owner move or detach.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 517f4ec964
ℹ️ 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".
…d Moved Heads Read the visible root's attachment epoch for a subagent report, gate lane recording on pullRequests.enabled, make the credential partition bound a config field, revalidate the detail head against the recorded commit, and find fork pull requests through the recorded commit.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d35c5cf663
ℹ️ 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".
| subagentControlHandler, | ||
| ); | ||
| router.get('/:parentConversationId/subagents', parentSubagentIndexHandler); | ||
| router.get('/:conversationId/pull-request', conversationPullRequestHandler); |
There was a problem hiding this comment.
Enabling endpoints.agents.pullRequests still cannot show the advertised pull-request chip: a repo-wide search finds the route and data-provider endpoint/service definitions, but no client hook or component calls conversationPullRequest or renders its response. This leaves the new backend capability without the promised frontend entry point, so operators can complete the documented setup and users will observe no change.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
…Revalidation Apply the largest configured credential bound to the shared cache, count cache hits as use, spend the comparison budget on a moved head, and fix the data-provider config default test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30f3ce83cd
ℹ️ 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".
A 403 whose message names a secondary rate limit now starts the credential cooldown for at least a minute instead of being cached as a ten second failure.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 062c52f364
ℹ️ 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".
|
|
||
| function isRateLimited(response: Response): boolean { | ||
| return ( | ||
| response.status === 429 || |
There was a problem hiding this comment.
When GitHub returns a secondary-limit 429 without Retry-After or quota headers, this unconditional branch classifies it before the body-based secondary-limit detector can assign SECONDARY_LIMIT_WAIT_MS; the lookup therefore falls back to only a 10-second cooldown. Fresh evidence beyond the prior headerless-403 finding is that GitHub documents secondary limits as either 403 or 429 and requires waiting at least one minute when neither limiting header applies, so polling can resume too early and risk an integration ban. GitHub rate-limit documentation
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and a gap in the previous fix: the 429 branch runs before the body check. Not changed in this PR; tracked in #16807 (item 6).
| while (entries.size > capacity) { | ||
| const oldest = entries.keys().next(); | ||
| if (oldest.done) break; | ||
| entries.delete(oldest.value); |
There was a problem hiding this comment.
When two principal-scoped configurations reference the same GitHub token but specify different cacheMaxEntries values, both use the same scopeOf(token) partition, so one cache miss from the principal configured for 10 entries trims that shared partition to 10 and evicts fresh entries belonging to the principal configured for 1,000. Fresh evidence after the credential-partition fix is that the partition key is still only the credential while this loop applies each caller's capacity; retain the largest capacity seen for that token or add the configuration scope to the partition.
AGENTS.md reference: AGENTS.md:L90-L92
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid for principals that share one token with different capacities. Tracked in #16807 (item 9).
| (await choose( | ||
| (await listPulls('closed', recorded == null ? 1 : lookup.maxCandidatePullRequests)) ?? [], | ||
| )) ?? | ||
| (await choose(await forkCandidates())); |
There was a problem hiding this comment.
When the recorded commit is associated with both an open fork PR and a matching closed PR from the base owner's same branch name, the closed candidate is selected before forkCandidates() is queried, so the endpoint returns the closed PR despite its open-first contract. Fresh evidence beyond the earlier fork-matching fix is that fork candidates of both states were added only after the closed base-repository search; split them by state or search open fork candidates before any closed candidates.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid for a commit that has both an open fork PR and a closed base PR. Tracked in #16807 (item 10).
| `${pathname}?per_page=${CHECK_RUN_PAGE_SIZE}&page=${page}`, | ||
| lookup, | ||
| ); | ||
| if (body == null) return { runs, incomplete: false }; |
There was a problem hiding this comment.
If a check-run page returns 404—for example because the PR head becomes unavailable between the detail and checks requests—getJson converts it to null and this branch returns the runs collected so far as a complete success. On the first page that reports checks: none; on a later page it can report passing, and either false result is cached even though GitHub never returned a check-run result. The GitHub endpoint documents success as a 200 response and requires Checks read permission, so handle its 404 as an operational lookup failure rather than documented absence.
AGENTS.md reference: AGENTS.md:L116-L121
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid: a check-run 404 should fail the lookup instead of reading as no checks. Tracked in #16807 (item 7).
| /** Every comparison, the first match and the revalidation of a moved head alike, spends the | ||
| * same configured budget. */ | ||
| const contains = async (headSha: string): Promise<boolean> => { | ||
| if (recorded == null || headSha === recorded) return true; |
There was a problem hiding this comment.
When the worker reports a named but unborn lane such as { branch: 'feature', head: null }, the WorkspaceLaneGit contract says the lane has no commit yet, but this condition treats every listed candidate as containing the recorded head. If that branch name was used previously, the subsequent closed-history lookup can therefore display an old PR from the prior incarnation even though the current lane cannot have a pull request; return null when the reported head is null instead of falling back to branch-only matching.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid: an unborn lane should return no pull request. Tracked in #16807 (item 8).
Summary
A conversation has no record of which branch its code workspace is on, so there is nothing to look a pull request up by. This stores the branch, head commit and
owner/namerepository a lane last reported on the conversation, and serves the matching pull request atGET /api/convos/:conversationId/pull-request.After each finished
bash_toolcommand, the recorder writes the reportedlaneGitfor the requesting user and the resolved conversation. The write is conditional on a change, so a lane that keeps reporting the same state costs no write, and it never delays or fails the command. The field is server-written only: it is excluded from ordinary reads and stripped from generic saves,unsetFieldsand bulk imports, the same waytoolApprovalAllowsis. The repository is stored only when it is a plainowner/namewithout dot segments.The route reads the stored branch owner-scoped, so another user's conversation looks the same as one with no pull request. It finds the pull request through an injected GitHub source: the open pull request wins over a newer closed one, a repository the token cannot see answers null, and results are cached per branch with a short lifetime for failures. A failure answers 503 with only a stable code (
NOT_CONFIGURED,RATE_LIMITED,UPSTREAM_ERROR); upstream text, the token and the exception never reach the response. The pull request page link is accepted only as an httpsgithub.comURL. The feature does nothing unlessendpoints.agents.pullRequestsis enabled, andlibrechat.example.yamldocuments it.Based on #16792; GitHub will retarget this to
devonce that merges. Nothing populateslaneGituntil a worker that reports it is deployed (LibreChat-AI/code-interpreter#311) andCODEAPI_BRIDGE_LANE_GIT=trueis set, so this is inert until then. Programmatic bash goes through the SDK and does not record a lane yet.How it works
Type of change
Testing
Tested environments/configuration:
mongodb-memory-serverfor the storage testsendpoints.agents.pullRequestsset in handler testsAutomated tests:
packages/data-schemas: 10laneGittests (owner scoping, no write when unchanged, repository change, nulls versus unknown, and four protection tests). I removed each strip in turn (generic save,unsetFields, bulk import) and the matching test failed.packages/api/src/pulls: GitHub mapping (draft, conflicts only while open, checks rollup, open preferred over closed, 404 as null, rate limit, bad link hosts, encoded branch), cache and shared in-flight request, and handler failure contract (no token or exception text in any response). Breaking each of the link host check, the dot-segment check, the owner scope, the failure cache cap and the error text made the matching test fail.packages/api/src/code/lane.spec.ts, plus route andToolServicetests.ToolService.spec.jsandconvos.spec.jspass with the new cases.packages/apirelated suites pass (3727 tests); the one failing suite,hitl/modes.integration.spec.ts(192 tests), fails identically onorigin/devbecause the Mongo memory server will not start in this sandbox.scheduleConsent.spec.ts(17 tests) fails the same way onorigin/dev.npx tsc --noEmitexits 0 indata-schemas,data-providerandapi.npm run static-checks:full -- --against origin/devpasses except config migration tests, which fail for the same Mongo reason.Screenshots / recordings
No user-facing change.
Risk / compatibility
Adds an optional field to the conversation schema and one route; existing documents are untouched and the field is not selected by default.
/apigains wiring only (8 lines inToolService.js, 10 inconvos.js). The lookup cache is per process, not shared across replicas. A chat not saved yet has no document, so its first command records nothing and the next one does. Not verified against a real worker or a real GitHub token.Checklist