From bc944ce58beb31a1b78be15e8b0e206ff1aabbe0 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 11 Oct 2026 18:40:28 +0200 Subject: [PATCH 01/12] feat: Look Up Bitbucket Cloud Pull Requests --- packages/api/src/pulls/bitbucket.spec.ts | 416 +++++++++++++++++++++++ packages/api/src/pulls/bitbucket.ts | 398 ++++++++++++++++++++++ packages/api/src/pulls/index.ts | 1 + packages/api/src/pulls/sources.spec.ts | 24 +- packages/api/src/pulls/sources.ts | 2 + 5 files changed, 838 insertions(+), 3 deletions(-) create mode 100644 packages/api/src/pulls/bitbucket.spec.ts create mode 100644 packages/api/src/pulls/bitbucket.ts diff --git a/packages/api/src/pulls/bitbucket.spec.ts b/packages/api/src/pulls/bitbucket.spec.ts new file mode 100644 index 00000000000..488b4b9f228 --- /dev/null +++ b/packages/api/src/pulls/bitbucket.spec.ts @@ -0,0 +1,416 @@ +import { createBitbucketPullRequestSource, isSameCommit, summarizeStatuses } from './bitbucket'; + +const API = 'https://api.bitbucket.org/2.0/repositories/w/r/'; +const sha = 'a'.repeat(40); +const short = sha.slice(0, 12); +const newer = 'b'.repeat(12); +const pullRequest = (overrides: Record = {}) => ({ + id: 7, + title: 'Simplify single tool', + state: 'OPEN', + draft: false, + links: { html: { href: 'https://bitbucket.org/w/r/pull-requests/7' } }, + source: { + branch: { name: 'feat/x' }, + commit: { hash: short }, + repository: { full_name: 'W/R' }, + }, + ...overrides, +}); +const fork = (hash = short) => ({ + branch: { name: 'feat/x' }, + commit: { hash }, + repository: { full_name: 'someone/r' }, +}); +const page = (values: unknown[], next?: string) => ({ values, ...(next ? { next } : {}) }); +const listed = (overrides: Record = {}) => page([pullRequest(overrides)]); +const status = (state: string, hash = sha) => ({ state, key: state, commit: { hash } }); +const json = (body: unknown, init?: ResponseInit) => Response.json(body, init); + +function sourceFor(routes: Record Response>) { + const fetchFn = jest.fn(async (input: string, _init?: RequestInit) => { + const url = String(input); + /** The most specific fragment wins, so a page link can be routed apart from its first page. */ + const key = Object.keys(routes) + .filter((fragment) => url.includes(fragment)) + .sort((a, b) => b.length - a.length)[0]; + if (key == null) throw new Error(`unexpected ${url}`); + return routes[key](); + }); + return { fetchFn, source: createBitbucketPullRequestSource({ fetchFn }) }; +} + +const routes = (detail = pullRequest(), overrides: Record Response> = {}) => ({ + 'state=OPEN': () => json(listed()), + 'state=MERGED': () => json(page([])), + '/pullrequests/7/diffstat': () => json(page([{ lines_added: 12, lines_removed: 3 }])), + '/pullrequests/7/statuses': () => json(page([status('SUCCESSFUL')])), + '/pullrequests/7': () => json(detail), + ...overrides, +}); + +const find = ( + source: ReturnType, + input: Record = {}, +) => source.find({ repo: 'w/r', branch: 'feat/x', token: 't', ...input }); + +const urlsOf = (fetchFn: ReturnType['fetchFn']) => + fetchFn.mock.calls.map(([url]) => String(url)); + +describe('summarizeStatuses', () => { + it.each([ + [[], false, 'none'], + [['SUCCESSFUL'], false, 'passing'], + [['SUCCESSFUL', 'INPROGRESS'], false, 'running'], + [['INPROGRESS', 'FAILED'], false, 'failing'], + [['SUCCESSFUL', 'STOPPED'], false, 'failing'], + [['SUCCESSFUL'], true, 'running'], + [[], true, 'running'], + ])('reports %p (incomplete %p) as %s', (states, incomplete, expected) => { + expect(summarizeStatuses(states, incomplete)).toBe(expected); + }); +}); + +describe('isSameCommit', () => { + it('compares by the shorter hash, never by fewer than seven characters', () => { + expect(isSameCommit(sha, short)).toBe(true); + expect(isSameCommit(short, sha)).toBe(true); + expect(isSameCommit(sha, newer)).toBe(false); + expect(isSameCommit(sha, 'aaaaaa')).toBe(false); + }); +}); + +describe('createBitbucketPullRequestSource', () => { + it('maps an open pull request with passing statuses', async () => { + const { source } = sourceFor(routes()); + await expect(find(source)).resolves.toEqual({ + number: 7, + title: 'Simplify single tool', + url: 'https://bitbucket.org/w/r/pull-requests/7', + provider: 'bitbucket', + additions: 12, + deletions: 3, + state: 'open', + isDraft: false, + mergeable: 'unknown', + checks: 'passing', + }); + }); + + it('asks the repository on the API host only, with a bearer token', async () => { + const { source, fetchFn } = sourceFor(routes()); + await find(source); + const urls = urlsOf(fetchFn); + expect(urls.length).toBeGreaterThan(0); + expect(urls.every((url) => url.startsWith(API))).toBe(true); + for (const [, init] of fetchFn.mock.calls) { + const headers = (init?.headers ?? {}) as Record; + expect(headers.Authorization).toBe('Bearer t'); + expect(headers.Accept).toBe('application/json'); + } + }); + + it('lists open pull requests first, newest first, then the closed history', async () => { + const { source, fetchFn } = sourceFor( + routes(pullRequest({ state: 'MERGED' }), { + 'state=OPEN': () => json(page([])), + 'state=MERGED': () => json(listed({ state: 'MERGED' })), + }), + ); + await find(source); + const [open, closed] = urlsOf(fetchFn).map((url) => new URL(url).searchParams); + expect(open.get('q')).toBe('source.branch.name = "feat/x" AND (state = "OPEN")'); + expect(open.getAll('state')).toEqual(['OPEN']); + expect(open.get('sort')).toBe('-created_on'); + expect(open.get('pagelen')).toBe('10'); + expect(closed.get('q')).toBe( + 'source.branch.name = "feat/x" AND (state = "MERGED" OR state = "DECLINED" OR state = "SUPERSEDED")', + ); + expect(closed.getAll('state')).toEqual(['MERGED', 'DECLINED', 'SUPERSEDED']); + /** Without a recorded commit only the newest closed one counts. */ + expect(closed.get('pagelen')).toBe('1'); + }); + + it('escapes a quote and a backslash in the branch name', async () => { + const branch = 'a"b\\c'; + const { source, fetchFn } = sourceFor({ + 'state=OPEN': () => json(page([])), + 'state=MERGED': () => json(page([])), + }); + await find(source, { branch }); + const q = new URL(urlsOf(fetchFn)[0]).searchParams.get('q'); + expect(q).toBe('source.branch.name = "a\\"b\\\\c" AND (state = "OPEN")'); + expect(urlsOf(fetchFn)[0]).not.toContain('"'); + }); + + it.each([ + ['another host', 'https://evil.example/w/r/pull-requests/7'], + ['plain http', 'http://bitbucket.org/w/r/pull-requests/7'], + ['a javascript url', 'javascript:alert(1)'], + ])('rejects %s as the pull request page', async (_label, href) => { + const { source } = sourceFor(routes(pullRequest({ links: { html: { href } } }))); + await expect(find(source)).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + }); + + it.each([ + ['MERGED', 'merged'], + ['DECLINED', 'closed'], + ['SUPERSEDED', 'closed'], + ])('reports a %s pull request as %s', async (state, expected) => { + const { source } = sourceFor( + routes(pullRequest({ state }), { + 'state=OPEN': () => json(page([])), + 'state=MERGED': () => json(listed({ state })), + }), + ); + await expect(find(source)).resolves.toMatchObject({ state: expected }); + }); + + it('reports a draft', async () => { + const { source } = sourceFor(routes(pullRequest({ draft: true }))); + await expect(find(source)).resolves.toMatchObject({ isDraft: true }); + }); + + it.each([ + ['failing', [status('SUCCESSFUL'), status('FAILED')]], + ['failing', [status('STOPPED')]], + ['running', [status('SUCCESSFUL'), status('INPROGRESS')]], + ['none', []], + ])('reports %s statuses', async (expected, statuses) => { + const { source } = sourceFor( + routes(pullRequest(), { '/pullrequests/7/statuses': () => json(page(statuses)) }), + ); + await expect(find(source)).resolves.toMatchObject({ checks: expected }); + }); + + it('ignores a status reported for another commit', async () => { + const { source } = sourceFor( + routes(pullRequest(), { + '/pullrequests/7/statuses': () => + json(page([status('SUCCESSFUL'), status('FAILED', 'c'.repeat(40))])), + }), + ); + await expect(find(source)).resolves.toMatchObject({ checks: 'passing' }); + }); + + it('never reports statuses past the page bound as passing', async () => { + const { source } = sourceFor( + routes(pullRequest(), { + '/pullrequests/7/statuses': () => + json(page([status('SUCCESSFUL')], `${API}pullrequests/7/statuses?page=2`)), + }), + ); + await expect(find(source, { limits: { maxCheckRunPages: 1 } })).resolves.toMatchObject({ + checks: 'running', + }); + }); + + it('sums the diffstat across pages', async () => { + const { source } = sourceFor( + routes(pullRequest(), { + '/pullrequests/7/diffstat?page=2': () => + json( + page([ + { lines_added: 5, lines_removed: 1 }, + { lines_added: 1, lines_removed: null }, + ]), + ), + '/pullrequests/7/diffstat': () => + json( + page([{ lines_added: 12, lines_removed: 3 }], `${API}pullrequests/7/diffstat?page=2`), + ), + }), + ); + await expect(find(source)).resolves.toMatchObject({ additions: 18, deletions: 4 }); + }); + + it.each([ + ['another host', 'https://evil.example/2.0/repositories/w/r/pullrequests/7/diffstat?page=2'], + ['plain http', 'http://api.bitbucket.org/2.0/repositories/w/r/pullrequests/7/diffstat?page=2'], + ['the site', 'https://bitbucket.org/2.0/repositories/w/r/pullrequests/7/diffstat?page=2'], + ])('never follows a next link to %s', async (_label, next) => { + const { source, fetchFn } = sourceFor( + routes(pullRequest(), { + '/pullrequests/7/diffstat': () => json(page([{ lines_added: 1, lines_removed: 1 }], next)), + }), + ); + await expect(find(source)).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + expect(urlsOf(fetchFn).every((url) => url.startsWith(API))).toBe(true); + }); + + it('finds nothing for a repository the token cannot see', async () => { + const { source } = sourceFor({ 'state=OPEN': () => new Response('', { status: 404 }) }); + await expect(find(source)).resolves.toBeNull(); + }); + + it('reports a rate limit with the wait Bitbucket asked for', async () => { + const { source } = sourceFor({ + 'state=OPEN': () => new Response('', { status: 429, headers: { 'retry-after': '30' } }), + }); + await expect(find(source)).rejects.toMatchObject({ + code: 'RATE_LIMITED', + retryAfterMs: 30_000, + }); + }); + + it('fails without upstream text on any other error', async () => { + const { source } = sourceFor({ + 'state=OPEN': () => new Response('secret', { status: 500 }), + }); + const error = await find(source).catch((caught: Error) => caught); + expect(error).toMatchObject({ code: 'UPSTREAM_ERROR' }); + expect(String((error as Error).message)).not.toContain('secret'); + }); + + it('fails on a malformed pull request', async () => { + const { source } = sourceFor(routes(pullRequest({ source: { commit: { hash: 'zz' } } }))); + await expect(find(source)).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + }); + + it('finds nothing for a lane that has a branch but no commit yet', async () => { + const { source, fetchFn } = sourceFor(routes()); + await expect(find(source, { head: null })).resolves.toBeNull(); + expect(fetchFn).not.toHaveBeenCalled(); + }); + + it('finds nothing for an empty branch', async () => { + const { source, fetchFn } = sourceFor(routes()); + await expect(find(source, { branch: '' })).resolves.toBeNull(); + expect(fetchFn).not.toHaveBeenCalled(); + }); + + it.each(['w', 'w/sub/r', 'w/..', './r', 'w/r?x', 'w/r%2F', ''])( + 'asks nothing for the path %p', + async (repo) => { + const { source, fetchFn } = sourceFor(routes()); + await expect(find(source, { repo })).resolves.toBeNull(); + expect(fetchFn).not.toHaveBeenCalled(); + }, + ); + + describe('with a recorded commit', () => { + it('accepts a head that is the recorded commit, abbreviated', async () => { + const { source, fetchFn } = sourceFor(routes()); + await expect(find(source, { head: sha })).resolves.toMatchObject({ number: 7 }); + expect(urlsOf(fetchFn).some((url) => url.includes('merge-base'))).toBe(false); + }); + + it('accepts a head that builds on the recorded commit', async () => { + const newerPull = pullRequest({ + source: { ...pullRequest().source, commit: { hash: newer } }, + }); + const { source, fetchFn } = sourceFor( + routes(newerPull, { + 'state=OPEN': () => json(page([newerPull])), + '/pullrequests/7/statuses': () => json(page([status('SUCCESSFUL', newer)])), + '/merge-base/': () => json({ hash: sha }), + }), + ); + await expect(find(source, { head: sha })).resolves.toMatchObject({ + number: 7, + checks: 'passing', + }); + expect(urlsOf(fetchFn)).toContain(`${API}merge-base/${sha}..${newer}`); + }); + + it('rejects a head that does not contain the recorded commit', async () => { + const newerPull = pullRequest({ + source: { ...pullRequest().source, commit: { hash: newer } }, + }); + const { source } = sourceFor( + routes(newerPull, { + 'state=OPEN': () => json(page([newerPull])), + '/merge-base/': () => json({ hash: 'c'.repeat(40) }), + }), + ); + await expect(find(source, { head: sha })).resolves.toBeNull(); + }); + + it.each([404, 400])('treats a merge base answered %s as no match', async (code) => { + const newerPull = pullRequest({ + source: { ...pullRequest().source, commit: { hash: newer } }, + }); + const { source } = sourceFor( + routes(newerPull, { + 'state=OPEN': () => json(page([newerPull])), + '/merge-base/': () => new Response('', { status: code }), + }), + ); + await expect(find(source, { head: sha })).resolves.toBeNull(); + }); + + it('stops comparing once the configured budget is spent', async () => { + const candidates = [1, 2, 3].map((id) => + pullRequest({ id, source: { ...pullRequest().source, commit: { hash: newer } } }), + ); + const { source, fetchFn } = sourceFor( + routes(pullRequest(), { + 'state=OPEN': () => json(page(candidates)), + '/merge-base/': () => json({ hash: 'c'.repeat(40) }), + }), + ); + await find(source, { head: sha, limits: { maxHeadComparisons: 2 } }); + const comparisons = urlsOf(fetchFn).filter((url) => url.includes('merge-base')); + expect(comparisons).toHaveLength(2); + }); + }); + + describe('forks', () => { + it('never takes a fork on its branch name alone', async () => { + const { source } = sourceFor( + routes(pullRequest({ source: fork() }), { + 'state=OPEN': () => json(listed({ source: fork() })), + }), + ); + await expect(find(source)).resolves.toBeNull(); + }); + + it('never takes a pull request whose source repository is gone', async () => { + const gone = { ...fork(), repository: null }; + const { source } = sourceFor( + routes(pullRequest({ source: gone }), { + 'state=OPEN': () => json(listed({ source: gone })), + }), + ); + await expect(find(source)).resolves.toBeNull(); + }); + + it('takes a fork that carries the recorded commit', async () => { + const { source } = sourceFor( + routes(pullRequest({ source: fork() }), { + 'state=OPEN': () => json(listed({ source: fork() })), + }), + ); + await expect(find(source, { head: sha })).resolves.toMatchObject({ number: 7 }); + }); + }); + + describe('a linked pull request', () => { + it('is read directly when its source branch is the lane branch', async () => { + const { source, fetchFn } = sourceFor(routes()); + await expect(find(source, { pullRequest: 7 })).resolves.toMatchObject({ number: 7 }); + expect(urlsOf(fetchFn).some((url) => url.includes('/pullrequests?'))).toBe(false); + }); + + it('is ignored for another branch, and the branch is searched instead', async () => { + const other = pullRequest({ + id: 9, + source: { ...pullRequest().source, branch: { name: 'other' } }, + }); + const { source, fetchFn } = sourceFor( + routes(pullRequest(), { '/pullrequests/9': () => json(other) }), + ); + await expect(find(source, { pullRequest: 9 })).resolves.toMatchObject({ number: 7 }); + expect(urlsOf(fetchFn).some((url) => url.includes('state=OPEN'))).toBe(true); + }); + + it('from a fork counts only with the recorded commit', async () => { + const linked = pullRequest({ source: fork() }); + const { source } = sourceFor(routes(linked, { 'state=OPEN': () => json(page([])) })); + await expect(find(source, { pullRequest: 7 })).resolves.toBeNull(); + await expect(find(source, { pullRequest: 7, head: sha })).resolves.toMatchObject({ + number: 7, + }); + }); + }); +}); diff --git a/packages/api/src/pulls/bitbucket.ts b/packages/api/src/pulls/bitbucket.ts new file mode 100644 index 00000000000..e38cf469676 --- /dev/null +++ b/packages/api/src/pulls/bitbucket.ts @@ -0,0 +1,398 @@ +import type { + PullRequestChecks, + PullRequestState, + TConversationPullRequest, +} from 'librechat-data-provider'; +import type { PullRequestFetch } from './shared'; +import type { PullRequestSource } from './types'; +import { + isCount, + isRecord, + COMMIT_ID, + isPageUrl, + retryAfterMs, + isPathSegment, + MAX_TITLE_LENGTH, + DEFAULT_LOOKUP_TIMEOUT_MS, + DEFAULT_REQUEST_TIMEOUT_MS, + DEFAULT_MAX_CANDIDATE_PAGES, + DEFAULT_MAX_HEAD_COMPARISONS, + DEFAULT_MAX_CANDIDATE_PULL_REQUESTS, +} from './shared'; +import { PullRequestSourceError } from './types'; + +export const BITBUCKET_HOST = 'bitbucket.org'; +/** Every request goes under this prefix; a `next` link elsewhere is never followed. */ +const API_ROOT = 'https://api.bitbucket.org/2.0/'; +/** `workspace/repo_slug`: Bitbucket Cloud has no nested groups. */ +const REPOSITORY_PATH = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; +/** Bitbucket abbreviates commits in pull requests (12 characters today), so any length is taken. */ +const COMMIT_HASH = /^[a-f0-9]{7,64}$/; +/** The shortest prefix two commits are compared by. */ +const MIN_HASH_LENGTH = 7; +/** The most pull requests Bitbucket returns per page; a larger `pagelen` is not honored. */ +const MAX_PAGE_LENGTH = 50; +const DEFAULT_MAX_PAGES = 10; +const OPEN_STATES = ['OPEN'] as const; +const CLOSED_STATES = ['MERGED', 'DECLINED', 'SUPERSEDED'] as const; +const STATES = [...OPEN_STATES, ...CLOSED_STATES]; +const FAILING_STATUSES = new Set(['FAILED', 'STOPPED']); + +type BitbucketPullRequest = { + id: number; + title: string; + url: string; + state: PullRequestState; + draft: boolean; + /** The abbreviated head commit. */ + hash: string; + /** The branch the pull request merges from, in whichever repository it lives. */ + sourceBranch: string | null; + /** Whether the source branch lives in another repository (a fork, or one since deleted). */ + fromFork: boolean; +}; + +type ListItem = { id: number; state: string; hash: string; fromFork: boolean }; + +/** Two commit hashes, either possibly abbreviated, name the same commit. */ +export function isSameCommit(a: string, b: string): boolean { + const length = Math.min(a.length, b.length); + return length >= MIN_HASH_LENGTH && a.slice(0, length) === b.slice(0, length); +} + +/** + * A failed or stopped status outranks one in progress; only every status successful is passing. + * An `incomplete` read (more statuses than were read) is never reported as passing. + */ +export function summarizeStatuses( + states: readonly string[], + incomplete = false, +): PullRequestChecks { + if (states.length === 0 && !incomplete) return 'none'; + if (states.some((state) => FAILING_STATUSES.has(state))) return 'failing'; + if (incomplete || states.some((state) => state !== 'SUCCESSFUL')) return 'running'; + return 'passing'; +} + +function summarizeState(state: string): PullRequestState { + if (state === 'OPEN') return 'open'; + return state === 'MERGED' ? 'merged' : 'closed'; +} + +function parseCommon(value: unknown, repo: string) { + if ( + !isRecord(value) || + !isCount(value.id) || + typeof value.state !== 'string' || + !(STATES as readonly string[]).includes(value.state) || + !isRecord(value.source) + ) { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + const { source } = value; + const commit = isRecord(source.commit) ? source.commit.hash : null; + if (typeof commit !== 'string' || !COMMIT_HASH.test(commit)) { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + const branch = isRecord(source.branch) ? source.branch.name : null; + /** A deleted fork leaves no source repository, which is as unproven as any other fork. */ + const fullName = isRecord(source.repository) ? source.repository.full_name : null; + return { + value, + id: value.id, + state: value.state, + hash: commit, + sourceBranch: typeof branch === 'string' ? branch : null, + fromFork: typeof fullName !== 'string' || fullName.toLowerCase() !== repo.toLowerCase(), + }; +} + +function parsePullRequest(value: unknown, repo: string): BitbucketPullRequest { + const common = parseCommon(value, repo); + const links = common.value.links; + const html = isRecord(links) && isRecord(links.html) ? links.html.href : null; + if (typeof common.value.title !== 'string' || !isPageUrl(html, BITBUCKET_HOST)) { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + return { + id: common.id, + title: common.value.title, + url: html, + state: summarizeState(common.state), + draft: common.value.draft === true, + hash: common.hash, + sourceBranch: common.sourceBranch, + fromFork: common.fromFork, + }; +} + +function parseListItem(value: unknown, repo: string): ListItem { + const { id, state, hash, fromFork } = parseCommon(value, repo); + return { id, state, hash, fromFork }; +} + +/** One page of a paginated collection: its values and the validated next page, if any. */ +function parsePage(value: unknown): { values: unknown[]; next: string | null } { + if (!isRecord(value) || !Array.isArray(value.values)) { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + if (value.next == null) return { values: value.values, next: null }; + if (typeof value.next !== 'string') throw new PullRequestSourceError('UPSTREAM_ERROR'); + let next: URL; + try { + next = new URL(value.next); + } catch { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + /** The token goes with the request, so a link off the API is a failure, never a fetch. */ + if (!next.href.startsWith(API_ROOT)) throw new PullRequestSourceError('UPSTREAM_ERROR'); + return { values: value.values, next: next.href }; +} + +/** The commit a status was reported for, when the status says. */ +function statusCommit(value: Record): string | null { + if (isRecord(value.commit) && typeof value.commit.hash === 'string') return value.commit.hash; + const links = value.links; + const href = isRecord(links) && isRecord(links.commit) ? links.commit.href : null; + if (typeof href !== 'string') return null; + return /\/commit\/([a-f0-9]{7,64})$/.exec(href)?.[1] ?? null; +} + +/** BBQL string literal: a backslash or a double quote inside it is escaped with a backslash. */ +const bbqlString = (value: string): string => `"${value.replace(/[\\"]/g, '\\$&')}"`; + +/** + * Reads pull requests through Bitbucket Cloud's REST API (2.0) with a token the caller supplies: + * a repository, project or workspace access token, or an OAuth access token, all sent as a bearer + * token. Only the decision (absent, rate limited, failed) leaves this module; response bodies and + * error text never do. + */ +export function createBitbucketPullRequestSource({ + fetchFn, +}: { + /** The deployment's HTTP client, so proxying and instrumentation are decided by the caller. */ + fetchFn: PullRequestFetch; +}): PullRequestSource { + type Lookup = { + token: string; + requestTimeoutMs: number; + deadline: AbortSignal; + maxCheckRunPages: number; + maxCandidatePullRequests: number; + maxHeadComparisons: number; + maxCandidatePages: number; + }; + + async function request( + url: string, + lookup: Lookup, + /** Statuses that mean "nothing there" for this request, besides 404. */ + absent: readonly number[] = [], + ): Promise { + let response: Response; + try { + response = await fetchFn(url, { + headers: { + Accept: 'application/json', + Authorization: `Bearer ${lookup.token}`, + 'User-Agent': 'LibreChat-Pull-Requests', + }, + signal: AbortSignal.any([AbortSignal.timeout(lookup.requestTimeoutMs), lookup.deadline]), + }); + } catch { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + if (response.ok) { + try { + return await response.json(); + } catch { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + } + /** Bitbucket sends no reset time with a rate limit, at most a `retry-after`. */ + if (response.status === 429) { + throw new PullRequestSourceError('RATE_LIMITED', retryAfterMs(response, 'retry-after')); + } + /** A repository the token cannot see is indistinguishable from one without pull requests. */ + if (response.status === 404 || absent.includes(response.status)) return null; + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + + /** + * Reads every page of a collection up to a bound, so a failure on a later page still shows. + * The pull request was read a moment ago, so a 404 here is one that vanished, not an empty list. + */ + async function readPages( + url: string, + lookup: Lookup, + ): Promise<{ values: unknown[]; incomplete: boolean }> { + const values: unknown[] = []; + let next: string | null = url; + for (let page = 0; page < lookup.maxCheckRunPages && next != null; page++) { + const body = await request(next, lookup); + if (body == null) throw new PullRequestSourceError('UPSTREAM_ERROR'); + const parsed = parsePage(body); + values.push(...parsed.values); + next = parsed.next; + } + return { values, incomplete: next != null }; + } + + return { + async find({ repo, branch, head: recordedHead, pullRequest, token, limits }) { + /** As on GitHub: a lane with a branch and no commit yet has nothing to match against. */ + if (recordedHead === null) return null; + if (!REPOSITORY_PATH.test(repo) || !repo.split('/').every(isPathSegment)) return null; + if (branch.length === 0) return null; + const lookup: Lookup = { + token, + requestTimeoutMs: limits?.requestTimeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS, + deadline: AbortSignal.timeout(limits?.lookupTimeoutMs ?? DEFAULT_LOOKUP_TIMEOUT_MS), + maxCheckRunPages: limits?.maxCheckRunPages ?? DEFAULT_MAX_PAGES, + maxCandidatePullRequests: + limits?.maxCandidatePullRequests ?? DEFAULT_MAX_CANDIDATE_PULL_REQUESTS, + maxHeadComparisons: limits?.maxHeadComparisons ?? DEFAULT_MAX_HEAD_COMPARISONS, + maxCandidatePages: limits?.maxCandidatePages ?? DEFAULT_MAX_CANDIDATE_PAGES, + }; + const [workspace, slug] = repo.split('/'); + const base = `${API_ROOT}repositories/${encodeURIComponent(workspace)}/${encodeURIComponent(slug)}`; + const getJson = (pathname: string, absent: readonly number[] = []) => + request(`${base}${pathname}`, lookup, absent); + + const recorded = + typeof recordedHead === 'string' && COMMIT_ID.test(recordedHead) ? recordedHead : undefined; + let comparisons = 0; + /** + * With a recorded commit a pull request counts only when its head is that commit or builds + * on it: their merge base is the recorded commit. Every comparison spends the configured + * budget, and a commit Bitbucket does not know is no match. + */ + const contains = async (hash: string): Promise => { + if (recorded == null || recorded.startsWith(hash)) return true; + if (comparisons >= lookup.maxHeadComparisons) return false; + comparisons += 1; + const merged = await getJson(`/merge-base/${recorded}..${hash}`, [400]); + if (merged == null) return false; + if (!isRecord(merged) || typeof merged.hash !== 'string') { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + return isSameCommit(merged.hash, recorded); + }; + /** + * A branch name proves nothing for a fork (anyone can open `patch-1`), so a fork counts only + * when it carries the recorded commit. + */ + const matches = async (item: { hash: string; fromFork: boolean }): Promise => + item.fromFork ? recorded != null && contains(item.hash) : contains(item.hash); + + const report = async (pull: BitbucketPullRequest): Promise => { + const path = `/pullrequests/${pull.id}`; + const [diffstat, statuses] = await Promise.all([ + readPages(`${base}${path}/diffstat`, lookup), + readPages(`${base}${path}/statuses`, lookup), + ]); + /** + * Past the page bound the counts cover only the files read. They are still reported: a + * pull request that large is better shown with a lower bound than with no size at all. + */ + let additions = 0; + let deletions = 0; + for (const file of diffstat.values) { + if (!isRecord(file)) throw new PullRequestSourceError('UPSTREAM_ERROR'); + const added = file.lines_added ?? 0; + const removed = file.lines_removed ?? 0; + if (!isCount(added) || !isCount(removed)) { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + additions += added; + deletions += removed; + } + const states: string[] = []; + for (const status of statuses.values) { + if (!isRecord(status) || typeof status.state !== 'string') { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + /** A status of an earlier head says nothing about this one. */ + const commit = statusCommit(status); + if (commit != null && !isSameCommit(commit, pull.hash)) continue; + states.push(status.state); + } + return { + number: pull.id, + title: pull.title.slice(0, MAX_TITLE_LENGTH), + url: pull.url, + provider: 'bitbucket', + additions, + deletions, + state: pull.state, + isDraft: pull.draft, + /** Bitbucket's API reports no conflict state for a pull request, only a diff. */ + mergeable: 'unknown', + checks: summarizeStatuses(states, statuses.incomplete), + }; + }; + + /** A pull request the conversation's own output linked to is read directly. */ + if (pullRequest != null && Number.isSafeInteger(pullRequest) && pullRequest > 0) { + const linked = await getJson(`/pullrequests/${pullRequest}`); + if (linked != null) { + const pull = parsePullRequest(linked, repo); + if (pull.sourceBranch === branch && (await matches(pull))) return report(pull); + } + } + + /** `undefined`: the repository is not visible. An empty list: it is, and nothing matched. */ + const list = async (states: readonly string[], pageLength: number, page: number) => { + const filter = states.map((state) => `state = "${state}"`).join(' OR '); + const query = new URLSearchParams({ + q: `source.branch.name = ${bbqlString(branch)} AND (${filter})`, + sort: '-created_on', + pagelen: String(pageLength), + page: String(page), + }); + /** The default list is open pull requests only. The `state` parameters lift that default + * and `q` names the same states, so either filter alone lists the same pull requests. */ + for (const state of states) query.append('state', state); + const listed = await getJson(`/pullrequests?${query.toString()}`); + if (listed == null) return undefined; + return parsePage(listed).values.map((item) => parseListItem(item, repo)); + }; + /** Without a recorded commit only the newest closed one matters, as on GitHub. */ + const listAll = async (closed: boolean): Promise => { + const states = closed ? CLOSED_STATES : OPEN_STATES; + const newestOnly = closed && recorded == null; + const pageLength = newestOnly + ? 1 + : Math.min(Math.max(lookup.maxCandidatePullRequests, 1), MAX_PAGE_LENGTH); + const pages = newestOnly ? 1 : lookup.maxCandidatePages; + const all: ListItem[] = []; + for (let page = 1; page <= pages; page++) { + const items = await list(states, pageLength, page); + if (items === undefined) return page === 1 ? undefined : all; + all.push(...items); + if (items.length < pageLength) break; + } + return all; + }; + const choose = async (candidates: ListItem[]): Promise => { + for (const candidate of candidates) { + if (await matches(candidate)) return candidate; + } + return null; + }; + + /** Open first on its own, so closed history on a reused branch name cannot hide it. */ + const open = await listAll(false); + if (open === undefined) return null; + const chosen = (await choose(open)) ?? (await choose((await listAll(true)) ?? [])); + if (chosen == null) return null; + + const detail = await getJson(`/pullrequests/${chosen.id}`); + if (detail == null) return null; + const pull = parsePullRequest(detail, repo); + /** A push between the list and the detail can move the head off the recorded commit. */ + if (!isSameCommit(pull.hash, chosen.hash) && !(await contains(pull.hash))) return null; + return report(pull); + }, + }; +} diff --git a/packages/api/src/pulls/index.ts b/packages/api/src/pulls/index.ts index 122c4d868dd..fe3776e1daa 100644 --- a/packages/api/src/pulls/index.ts +++ b/packages/api/src/pulls/index.ts @@ -2,6 +2,7 @@ export * from './types'; export * from './links'; export * from './github'; export * from './gitlab'; +export * from './bitbucket'; export * from './sources'; export * from './lookup'; export * from './handler'; diff --git a/packages/api/src/pulls/sources.spec.ts b/packages/api/src/pulls/sources.spec.ts index 652eb5e2abb..21c138ec969 100644 --- a/packages/api/src/pulls/sources.spec.ts +++ b/packages/api/src/pulls/sources.spec.ts @@ -1,12 +1,13 @@ import type { PullRequestSource } from './types'; -import { routePullRequestSources } from './sources'; +import { createPullRequestSource, routePullRequestSources } from './sources'; const input = { repo: 'o/r', branch: 'feat/x', token: 't' }; describe('routePullRequestSources', () => { const github: PullRequestSource = { find: jest.fn().mockResolvedValue(null) }; const gitlab: PullRequestSource = { find: jest.fn().mockResolvedValue(null) }; - const source = routePullRequestSources({ github, gitlab }); + const bitbucket: PullRequestSource = { find: jest.fn().mockResolvedValue(null) }; + const source = routePullRequestSources({ github, gitlab, bitbucket }); beforeEach(() => jest.clearAllMocks()); @@ -20,11 +21,28 @@ describe('routePullRequestSources', () => { await source.find({ ...input, provider: 'gitlab' }); expect(gitlab.find).toHaveBeenCalledTimes(1); expect(github.find).not.toHaveBeenCalled(); + await source.find({ ...input, provider: 'bitbucket' }); + expect(bitbucket.find).toHaveBeenCalledTimes(1); + expect(gitlab.find).toHaveBeenCalledTimes(1); }); it('reports a provider without a source as not configured', async () => { - await expect(source.find({ ...input, provider: 'bitbucket' })).rejects.toMatchObject({ + const githubOnly = routePullRequestSources({ github }); + await expect(githubOnly.find({ ...input, provider: 'gitlab' })).rejects.toMatchObject({ code: 'NOT_CONFIGURED', }); }); }); + +describe('createPullRequestSource', () => { + it('routes Bitbucket to Bitbucket Cloud', async () => { + const fetchFn = jest.fn(async (_input: string, _init?: RequestInit) => + Response.json({ values: [] }), + ); + const source = createPullRequestSource({ fetchFn }); + await expect(source.find({ ...input, provider: 'bitbucket' })).resolves.toBeNull(); + expect(String(fetchFn.mock.calls[0][0])).toMatch( + /^https:\/\/api\.bitbucket\.org\/2\.0\/repositories\/o\/r\/pullrequests\?/, + ); + }); +}); diff --git a/packages/api/src/pulls/sources.ts b/packages/api/src/pulls/sources.ts index 1eb588d9c79..4dfe8eb2b05 100644 --- a/packages/api/src/pulls/sources.ts +++ b/packages/api/src/pulls/sources.ts @@ -1,6 +1,7 @@ import type { PullRequestProvider } from 'librechat-data-provider'; import type { PullRequestFetch } from './shared'; import type { PullRequestSource } from './types'; +import { createBitbucketPullRequestSource } from './bitbucket'; import { createGitHubPullRequestSource } from './github'; import { createGitLabPullRequestSource } from './gitlab'; import { PullRequestSourceError } from './types'; @@ -31,5 +32,6 @@ export function createPullRequestSource({ return routePullRequestSources({ github: createGitHubPullRequestSource({ fetchFn }), gitlab: createGitLabPullRequestSource({ fetchFn }), + bitbucket: createBitbucketPullRequestSource({ fetchFn }), }); } From 20d44a78fb23f5f4495b269b26dce4ed3f3f2109 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:03:47 +0200 Subject: [PATCH 02/12] fix: Keep the Pull Request Host Out of Overrides and Per-Host Keys A role, group, user or tenant override can no longer change or remove pullRequests.host, so feature gating and link recording see the operator's host. Links are recorded only from the configured host, and the batch share key and the rate limit cooldown are scoped by host. --- api/server/services/ToolService.js | 4 +-- packages/api/src/code/lane.spec.ts | 6 ++++ packages/api/src/code/lane.ts | 5 ++- packages/api/src/pulls/handler.spec.ts | 32 +++++++++++++++++++ packages/api/src/pulls/handler.ts | 1 + packages/api/src/pulls/links.spec.ts | 20 ++++++++++-- packages/api/src/pulls/links.ts | 23 +++++++++---- packages/api/src/pulls/lookup.spec.ts | 11 +++++++ packages/api/src/pulls/lookup.ts | 8 +++-- packages/api/src/pulls/settings.spec.ts | 29 +++++++++++++++++ packages/api/src/pulls/settings.ts | 13 ++++++++ packages/data-provider/src/config.ts | 4 +++ .../data-schemas/src/app/resolution.spec.ts | 32 +++++++++++++++++++ packages/data-schemas/src/app/resolution.ts | 26 ++++++++++++++- 14 files changed, 198 insertions(+), 16 deletions(-) diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index e6a6edce3b3..3cee887ce3a 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -51,7 +51,7 @@ const { isFatalAgentInitializationError, codeExecutionAuthHeaders, createLaneGitRecorder, - isPullRequestFeatureActive, + resolvePullRequestRecording, createAttachedWorkspaceBashTool, createRepositoryInstructionSource, createRepositoryInstructionLoader, @@ -2369,7 +2369,7 @@ async function loadToolsForExecution({ workspaceId: codeExecutionContext.codeWorkspace.workspaceId, workspaceInstanceId: codeExecutionContext.codeWorkspace.workspaceInstanceId, onLaneGit: await createLaneGitRecorder({ - enabled: isPullRequestFeatureActive( + ...resolvePullRequestRecording( req.config?.endpoints?.agents?.pullRequests, process.env, ), diff --git a/packages/api/src/code/lane.spec.ts b/packages/api/src/code/lane.spec.ts index fbf9786d54b..6e7d4533203 100644 --- a/packages/api/src/code/lane.spec.ts +++ b/packages/api/src/code/lane.spec.ts @@ -119,6 +119,12 @@ describe('createLaneGitRecorder', () => { ); }); + it('records only a link on the configured host', async () => { + const { record, setConvoLaneGit } = make({ repo: 'o/r', host: 'ghe.example.com' }); + await record?.(laneGit, 'https://ghe.example.com/o/r/pull/5 https://github.com/o/r/pull/9'); + expect(setConvoLaneGit).toHaveBeenCalledWith(expect.objectContaining({ pullRequest: 5 })); + }); + it.each([ ['output without a link', 'nothing to see', 'o/r', laneGit], ['a link to another repository', 'https://github.com/x/y/pull/42', 'o/r', laneGit], diff --git a/packages/api/src/code/lane.ts b/packages/api/src/code/lane.ts index 3a2c2bb88c5..1c697ea3a25 100644 --- a/packages/api/src/code/lane.ts +++ b/packages/api/src/code/lane.ts @@ -78,6 +78,7 @@ const reservationTails = new Map>(); */ export async function createLaneGitRecorder({ enabled, + host, user, conversationId, repo, @@ -89,6 +90,8 @@ export async function createLaneGitRecorder({ }: { /** Whether the deployment shows pull requests; when it does not, nothing is read or recorded. */ enabled: boolean; + /** The configured pull request host; a link to any other host is not recorded. */ + host?: string; user: string | undefined; conversationId: string | undefined; repo?: string; @@ -177,7 +180,7 @@ export async function createLaneGitRecorder({ return (laneGit, output) => { const pullRequest = safeRepo != null && output != null && laneGit.branch != null - ? findPullRequestLink(output, safeRepo) + ? findPullRequestLink(output, safeRepo, host) : null; const previous = reservationTails.get(queueKey) ?? Promise.resolve(); const reservation = previous.then(() => reserve()); diff --git a/packages/api/src/pulls/handler.spec.ts b/packages/api/src/pulls/handler.spec.ts index 9f3602220d7..ccec6dde27c 100644 --- a/packages/api/src/pulls/handler.spec.ts +++ b/packages/api/src/pulls/handler.spec.ts @@ -1083,6 +1083,38 @@ describe('createConversationPullRequestsHandler', () => { }); }); + it('does not join a lookup started for another host', async () => { + const hosts: Array = []; + const lookup = jest.fn( + (input: { host?: string }) => + new Promise(() => { + hosts.push(input.host); + }), + ); + const settings = { ...enabled, batchTimeoutSeconds: 1 }; + const getAppConfig = jest + .fn() + .mockResolvedValueOnce(configWith(settings)) + .mockResolvedValueOnce(configWith(settings)) + .mockResolvedValue(configWith({ ...settings, host: 'ghe.example.com' })); + const handler = createConversationPullRequestsHandler({ + getConvosLaneGit: jest.fn(async () => [lane('a', 'o/r', 'feat/shared')]), + getAppConfig, + lookup: lookup as never, + env: { GH_TOKEN: 'ghp_secret' }, + }); + const call = () => + handler( + { user: { id: 'u1' }, body: { conversationIds: ['a'] } } as unknown as ServerRequest, + { status: jest.fn().mockReturnThis(), json: jest.fn() } as unknown as Response, + ); + void call(); + await flush(); + void call(); + await flush(); + expect(hosts).toEqual(['github.com', 'ghe.example.com']); + }); + it('gives every conversation on a shared branch the same failure when that lookup fails', async () => { const lookup = jest .fn() diff --git a/packages/api/src/pulls/handler.ts b/packages/api/src/pulls/handler.ts index 84fa4ee8372..7037758129a 100644 --- a/packages/api/src/pulls/handler.ts +++ b/packages/api/src/pulls/handler.ts @@ -467,6 +467,7 @@ export function createConversationPullRequestsHandler(deps: { const input = lookupInput(settings, token, lane); /** The same lane under the same policy is one lookup for every request that wants it. */ const shareKey = `${scope}\0${JSON.stringify([ + input.host, input.repo, input.branch, input.head, diff --git a/packages/api/src/pulls/links.spec.ts b/packages/api/src/pulls/links.spec.ts index 93f9e0db7f3..d9941793469 100644 --- a/packages/api/src/pulls/links.spec.ts +++ b/packages/api/src/pulls/links.spec.ts @@ -16,9 +16,23 @@ describe('findPullRequestLink', () => { expect(findPullRequestLink(output, 'o/r')).toBe(8); }); - it('finds a link on a GitHub Enterprise host, which the lookup then checks', () => { - expect(findPullRequestLink('https://ghe.example.com/o/r/pull/9', 'o/r')).toBe(9); - expect(findPullRequestLink('https://ghe.local:8443/o/r/pull/9', 'o/r')).toBe(9); + it('finds a link only on the configured host', () => { + expect(findPullRequestLink('https://ghe.example.com/o/r/pull/9', 'o/r')).toBeNull(); + expect( + findPullRequestLink('https://ghe.example.com/o/r/pull/9', 'o/r', 'ghe.example.com'), + ).toBe(9); + expect(findPullRequestLink('https://GHE.local:8443/o/r/pull/9', 'o/r', 'ghe.local:8443')).toBe( + 9, + ); + expect(findPullRequestLink('https://ghe.local:443/o/r/pull/9', 'o/r', 'ghe.local')).toBe(9); + expect( + findPullRequestLink('https://github.com/o/r/pull/9', 'o/r', 'ghe.example.com'), + ).toBeNull(); + }); + + it('keeps the last link on the configured host when another installation is linked after it', () => { + const output = 'https://ghe.example.com/o/r/pull/4 and https://github.com/o/r/pull/7'; + expect(findPullRequestLink(output, 'o/r', 'ghe.example.com')).toBe(4); }); it('finds a GitLab merge request, in a nested group too', () => { diff --git a/packages/api/src/pulls/links.ts b/packages/api/src/pulls/links.ts index de218899668..c04f2728df5 100644 --- a/packages/api/src/pulls/links.ts +++ b/packages/api/src/pulls/links.ts @@ -1,25 +1,36 @@ +const DEFAULT_HOST = 'github.com'; + +/** A URL drops the https default port, as the configured host does. */ +const normalizeHost = (host: string): string => host.toLowerCase().replace(/:443$/, ''); + /** * A pull request page on any provider, ending where the number does: GitHub's * `/{owner}/{name}/pull/{n}`, GitLab's `/{group}/.../{name}/-/merge_requests/{n}` and Bitbucket's * `/{workspace}/{name}/pull-requests/{n}`. A `/pull/new/` page (what `git push` suggests) has no - * number and never matches. Any host matches: the number is only a candidate, which the lookup - * reads from the configured provider and accepts only when its head is the lane's branch. + * number and never matches. */ const PULL_REQUEST_URL = - /https:\/\/[A-Za-z0-9.-]+(?::\d{1,5})?\/((?:[A-Za-z0-9_.-]+\/)+?[A-Za-z0-9_.-]+)\/(?:pull|-\/merge_requests|pull-requests)\/(\d+)(?![\w-])/g; + /https:\/\/([A-Za-z0-9.-]+(?::\d{1,5})?)\/((?:[A-Za-z0-9_.-]+\/)+?[A-Za-z0-9_.-]+)\/(?:pull|-\/merge_requests|pull-requests)\/(\d+)(?![\w-])/g; const MARKERS = ['/pull/', '/merge_requests/', '/pull-requests/']; /** * The pull request of `repo` that command output links to, the last one when it links several, * since tools such as `gh pr create` print the link they made last. Links to any other repository - * are ignored, so output that mentions another project's pull request cannot claim the lane. + * or to any host but `host` (the configured one, as `resolvePullRequestHost` returns it) are + * ignored, so output that mentions another project's or another installation's pull request + * cannot claim the lane. */ -export function findPullRequestLink(output: string, repo: string): number | null { +export function findPullRequestLink( + output: string, + repo: string, + host: string = DEFAULT_HOST, +): number | null { if (!MARKERS.some((marker) => output.includes(marker))) return null; const wanted = repo.toLowerCase(); let found: number | null = null; for (const match of output.matchAll(PULL_REQUEST_URL)) { - const [, path, digits] = match; + const [, linkHost, path, digits] = match; + if (normalizeHost(linkHost) !== host) continue; if (path.toLowerCase() !== wanted) continue; const number = Number(digits); if (Number.isSafeInteger(number) && number > 0) found = number; diff --git a/packages/api/src/pulls/lookup.spec.ts b/packages/api/src/pulls/lookup.spec.ts index 8fc89025380..1e381a39b7c 100644 --- a/packages/api/src/pulls/lookup.spec.ts +++ b/packages/api/src/pulls/lookup.spec.ts @@ -192,6 +192,17 @@ describe('rate limit cooldown', () => { }); }); + it('does not block the same credential on another host', async () => { + const find = jest.fn().mockRejectedValueOnce(limited(60_000)).mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup(input); + await expect(lookup({ ...input, host: 'ghe.example.com' })).resolves.toEqual({ + ok: true, + value, + }); + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual(rateLimited); + }); + it('waits at least a short floor when GitHub gives no retry time', async () => { let clock = 0; const find = jest.fn().mockRejectedValueOnce(limited()).mockResolvedValue(value); diff --git a/packages/api/src/pulls/lookup.ts b/packages/api/src/pulls/lookup.ts index 3bf197a3081..b4226a4fd7f 100644 --- a/packages/api/src/pulls/lookup.ts +++ b/packages/api/src/pulls/lookup.ts @@ -141,10 +141,12 @@ export function createPullRequestLookup({ if (cached != null && cached.expiresAt > now()) return cached.result; const pending = inflight.get(key); if (pending != null) return pending; - const until = cooldowns.get(scope); + /** A rate limit belongs to the credential on one host; the same token elsewhere is not limited. */ + const limitScope = `${scope}\0${host ?? ''}`; + const until = cooldowns.get(limitScope); if (until != null) { if (until > now()) return rateLimited; - cooldowns.delete(scope); + cooldowns.delete(limitScope); } const run = (async (): Promise => { @@ -170,7 +172,7 @@ export function createPullRequestLookup({ } const code = error instanceof PullRequestSourceError ? error.code : 'UPSTREAM_ERROR'; if (error instanceof PullRequestSourceError && error.code === 'RATE_LIMITED') { - startCooldown(scope, error.retryAfterMs); + startCooldown(limitScope, error.retryAfterMs); } result = { ok: false, error: { code } }; } diff --git a/packages/api/src/pulls/settings.spec.ts b/packages/api/src/pulls/settings.spec.ts index 7f9b9cc94f6..2104436adf8 100644 --- a/packages/api/src/pulls/settings.spec.ts +++ b/packages/api/src/pulls/settings.spec.ts @@ -3,6 +3,7 @@ import { resolvePullRequestHost, resolvePullRequestToken, isPullRequestFeatureActive, + resolvePullRequestRecording, resolvePullRequestProvider, } from './settings'; @@ -174,3 +175,31 @@ describe('isPullRequestFeatureActive', () => { ); }); }); + +describe('resolvePullRequestRecording', () => { + const env = { GITHUB_TOKEN: 't' }; + it('records with the configured host when the feature is on', () => { + expect( + resolvePullRequestRecording({ host: 'GHE.example.com:443', allowAllRepositories: true }, env), + ).toEqual({ enabled: true, host: 'ghe.example.com' }); + expect(resolvePullRequestRecording({ allowAllRepositories: true }, env)).toEqual({ + enabled: true, + host: 'github.com', + }); + }); + + it('records links on the default host of the configured provider', () => { + expect( + resolvePullRequestRecording( + { provider: 'gitlab', allowAllRepositories: true }, + { GITLAB_TOKEN: 't' }, + ), + ).toEqual({ enabled: true, host: 'gitlab.com' }); + }); + + it('records nothing when the feature is off', () => { + expect(resolvePullRequestRecording({ allowAllRepositories: true }, {})).toEqual({ + enabled: false, + }); + }); +}); diff --git a/packages/api/src/pulls/settings.ts b/packages/api/src/pulls/settings.ts index 0f3e6c5e035..448fd78ad0d 100644 --- a/packages/api/src/pulls/settings.ts +++ b/packages/api/src/pulls/settings.ts @@ -98,3 +98,16 @@ export function isPullRequestFeatureActive( settings?.allowAllRepositories === true || (settings?.allowedRepositories?.length ?? 0) > 0 ); } + +/** + * What a lane recorder needs from the settings: whether to record at all, and the host whose pull + * request links it keeps. + */ +export function resolvePullRequestRecording( + settings: Partial | null | undefined, + env: Env, +): { enabled: boolean; host?: string } { + const enabled = isPullRequestFeatureActive(settings, env); + const host = enabled ? resolvePullRequestHost(settings?.host) : null; + return host != null ? { enabled, host } : { enabled }; +} diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 5ed9928616f..321e8ac3eae 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -112,6 +112,10 @@ export const BASE_ONLY_CONFIG_SECTIONS = ['filters', 'mcpAppSandbox'] as const; /** Sections that may be stored in the tenant's base config document but must * not be overridden or tombstoned by role, group, or user config documents. */ export const BASE_PRINCIPAL_CONFIG_SECTIONS = ['langfuse'] as const; +/** Fields inside overridable sections that only the operator's YAML sets: a role, group, user + * or tenant config document can neither change nor remove them. The pull request host decides + * where the deployment's token is sent. */ +export const BASE_ONLY_CONFIG_PATHS = ['endpoints.agents.pullRequests.host'] as const; export const defaultRetrievalModels = [ 'gpt-4o', diff --git a/packages/data-schemas/src/app/resolution.spec.ts b/packages/data-schemas/src/app/resolution.spec.ts index f1ed8a393c0..87a3a01bb90 100644 --- a/packages/data-schemas/src/app/resolution.spec.ts +++ b/packages/data-schemas/src/app/resolution.spec.ts @@ -141,6 +141,38 @@ describe('mergeConfigOverrides', () => { expect(mergeConfigOverrides(base, configs).mcpAppSandbox).toEqual(base.mcpAppSandbox); }); + it('keeps the pull request host from the base config whatever an override does', () => { + const pullRequests = { host: 'ghe.example.com', allowedRepositories: ['acme/*'] }; + const base = { endpoints: { agents: { pullRequests } } } as unknown as AppConfig; + const read = (configs: IConfig[]) => + ( + mergeConfigOverrides(base, configs) as unknown as { + endpoints: { agents: { pullRequests: Record } }; + } + ).endpoints.agents.pullRequests; + + const changed = read([ + fakeConfig( + { + endpoints: { + agents: { pullRequests: { host: 'evil.example.com', cacheTtlSeconds: 60 } }, + }, + }, + 10, + ), + ]); + expect(changed).toMatchObject({ host: 'ghe.example.com', cacheTtlSeconds: 60 }); + + const removed = read([fakeConfig({}, 10, ['endpoints.agents.pullRequests.host'])]); + expect(removed.host).toBe('ghe.example.com'); + + const unset = { endpoints: { agents: { pullRequests: {} } } } as unknown as AppConfig; + const added = mergeConfigOverrides(unset, [ + fakeConfig({ endpoints: { agents: { pullRequests: { host: 'evil.example.com' } } } }, 10), + ]) as unknown as { endpoints: { agents: { pullRequests: Record } } }; + expect(added.endpoints.agents.pullRequests.host).toBeUndefined(); + }); + it('applies tenant-wide Langfuse settings only from the base principal', () => { const configs = [ fakeConfig( diff --git a/packages/data-schemas/src/app/resolution.ts b/packages/data-schemas/src/app/resolution.ts index c15a7b247ce..49b22269922 100644 --- a/packages/data-schemas/src/app/resolution.ts +++ b/packages/data-schemas/src/app/resolution.ts @@ -1,6 +1,7 @@ import { BASE_PRINCIPAL_CONFIG_SECTIONS, BASE_ONLY_CONFIG_SECTIONS, + BASE_ONLY_CONFIG_PATHS, INTERFACE_PERMISSION_FIELDS, RUNTIME_CONFIG_INTERFACE_FIELDS, PERMISSION_SUB_KEYS, @@ -852,7 +853,30 @@ export function mergeConfigOverrides(baseConfig: AppConfig, configs: IConfig[]): } } - return preserveRuntimeStops(baseConfig, merged); + return preserveBaseOnlyPaths(baseConfig, preserveRuntimeStops(baseConfig, merged)); +} + +function getPath(target: unknown, segments: string[]): unknown { + let current = target; + for (const segment of segments) { + if (!isPlainObject(current)) return undefined; + current = current[segment]; + } + return current; +} + +/** + * Puts back the base value of every `BASE_ONLY_CONFIG_PATHS` field, whatever an override set or a + * tombstone removed on the way, so every reader of the merged config sees the operator's value. + */ +function preserveBaseOnlyPaths(baseConfig: AppConfig, merged: T): T { + let result: unknown = merged; + for (const path of BASE_ONLY_CONFIG_PATHS) { + const segments = path.split('.'); + const value = getPath(baseConfig, segments); + if (getPath(result, segments) !== value) result = setPath(result, segments, value); + } + return result as T; } /** Whether a runtime-config interface field reads as OFF in either of its two shapes: From 7c58691b96bbaeba53fc859cc28ff593cab655c5 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:15:03 +0200 Subject: [PATCH 03/12] fix: Compare Pull Request Link Hosts Through the Host Resolver --- packages/api/src/pulls/links.spec.ts | 3 +++ packages/api/src/pulls/links.ts | 7 +++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/api/src/pulls/links.spec.ts b/packages/api/src/pulls/links.spec.ts index d9941793469..681c44ed5fb 100644 --- a/packages/api/src/pulls/links.spec.ts +++ b/packages/api/src/pulls/links.spec.ts @@ -25,6 +25,9 @@ describe('findPullRequestLink', () => { 9, ); expect(findPullRequestLink('https://ghe.local:443/o/r/pull/9', 'o/r', 'ghe.local')).toBe(9); + expect(findPullRequestLink('https://ghe.local:08443/o/r/pull/9', 'o/r', 'ghe.local:8443')).toBe( + 9, + ); expect( findPullRequestLink('https://github.com/o/r/pull/9', 'o/r', 'ghe.example.com'), ).toBeNull(); diff --git a/packages/api/src/pulls/links.ts b/packages/api/src/pulls/links.ts index c04f2728df5..0fb084003ef 100644 --- a/packages/api/src/pulls/links.ts +++ b/packages/api/src/pulls/links.ts @@ -1,7 +1,6 @@ -const DEFAULT_HOST = 'github.com'; +import { resolvePullRequestHost } from './settings'; -/** A URL drops the https default port, as the configured host does. */ -const normalizeHost = (host: string): string => host.toLowerCase().replace(/:443$/, ''); +const DEFAULT_HOST = 'github.com'; /** * A pull request page on any provider, ending where the number does: GitHub's @@ -30,7 +29,7 @@ export function findPullRequestLink( let found: number | null = null; for (const match of output.matchAll(PULL_REQUEST_URL)) { const [, linkHost, path, digits] = match; - if (normalizeHost(linkHost) !== host) continue; + if (resolvePullRequestHost(linkHost) !== host) continue; if (path.toLowerCase() !== wanted) continue; const number = Number(digits); if (Number.isSafeInteger(number) && number > 0) found = number; From f8361a897662dfc5baa47c986daa1fcceb5654b1 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:57:06 +0200 Subject: [PATCH 04/12] test: Add Pull Request Scenarios Against a Fake GitHub Enterprise Host --- e2e/config/librechat.e2e.yaml | 8 + e2e/playwright.config.mock.ts | 64 +++++++ e2e/setup/fake-git-host-server.js | 126 +++++++++++++ .../pull-request-enterprise-host.spec.ts | 176 ++++++++++++++++++ 4 files changed, 374 insertions(+) create mode 100644 e2e/setup/fake-git-host-server.js create mode 100644 e2e/specs/mock/scenarios/pull-request-enterprise-host.spec.ts diff --git a/e2e/config/librechat.e2e.yaml b/e2e/config/librechat.e2e.yaml index 73d14844222..5e85f397d8c 100644 --- a/e2e/config/librechat.e2e.yaml +++ b/e2e/config/librechat.e2e.yaml @@ -146,6 +146,14 @@ endpoints: - run_in_background - stateful_code_sessions # __E2E_CODE_BRIDGE_CONFIG__ + # Pull request lookups reach the fake GitHub Enterprise host + # (e2e/setup/fake-git-host-server.js), and only for the repository the pull request + # scenarios seed, so no other conversation ever starts a lookup. + pullRequests: + host: localhost:8793 + token: '${E2E_GIT_HOST_TOKEN}' + allowedRepositories: + - e2e-org/e2e-app # Keep the shared mock profile non-interactive except for the dedicated # approval probe. This exercises real HITL pause/resume without wedging the # existing file-authoring, steering, background-tool, or MCP specs. diff --git a/e2e/playwright.config.mock.ts b/e2e/playwright.config.mock.ts index a94fda2745d..2fc346adc8c 100644 --- a/e2e/playwright.config.mock.ts +++ b/e2e/playwright.config.mock.ts @@ -1,4 +1,5 @@ import { defineConfig, devices } from '@playwright/test'; +import { execFileSync } from 'child_process'; import fs from 'fs'; import path from 'path'; import { getLocalE2EEnv, getE2EBaseURL } from './setup/env'; @@ -38,6 +39,12 @@ const ragServerPath = path.resolve(rootPath, 'e2e/setup/fake-rag-server.js'); * Kept clear of the MCP (8765/8766) and label (8889) fixtures. */ const CODE_API_PORT = process.env.E2E_CODE_API_PORT || '8790'; const RAG_API_PORT = process.env.E2E_RAG_API_PORT || '8791'; +/** The fake GitHub Enterprise host pull request lookups reach (`pullRequests.host`). The template + * hard-codes 8793; `writeRuntimeMockConfig` substitutes any override into the generated copy. */ +const gitHostServerPath = path.resolve(rootPath, 'e2e/setup/fake-git-host-server.js'); +const GIT_HOST_PORT = process.env.E2E_GIT_HOST_PORT || '8793'; +const gitHostKeyPath = path.resolve(rootPath, 'e2e/.generated/git-host.key'); +const gitHostCertPath = path.resolve(rootPath, 'e2e/.generated/git-host.crt'); const fakeModelHookPath = path.resolve(rootPath, 'e2e/setup/fake-model.js'); /** Model-fixture record mode: the run hook taps the REAL provider stream into a * replayable fixture instead of overriding the model (e2e/setup/model-replay.js). */ @@ -219,6 +226,9 @@ const baseEnv = { ...(externalCodeBaseUrl ? { LIBRECHAT_CODE_BASEURL_STATEFUL: externalCodeBaseUrl } : {}), ...codeApiKeyEnv, RAG_API_URL: `http://127.0.0.1:${RAG_API_PORT}`, + /** Pull request lookups go to the fake git host, which trusts any bearer token. */ + E2E_GIT_HOST_TOKEN: 'e2e-git-host-token', + NODE_EXTRA_CA_CERTS: gitHostCertPath, ...vanillaOverrides, }; @@ -383,6 +393,9 @@ function writeRuntimeMockConfig() { if (enableDynamicMcp && MCP_DYNAMIC_PORT !== '8766') { config = config.split('127.0.0.1:8766').join(`127.0.0.1:${MCP_DYNAMIC_PORT}`); } + if (GIT_HOST_PORT !== '8793') { + config = config.split('localhost:8793').join(`localhost:${GIT_HOST_PORT}`); + } if (MCP_OAUTH_PORT !== '8767') { config = config.split('127.0.0.1:8767').join(`127.0.0.1:${MCP_OAUTH_PORT}`); } @@ -394,6 +407,40 @@ function writeRuntimeMockConfig() { } } +/** + * The fake git host speaks TLS, as every pull request host must, with a certificate for + * `localhost` the app server trusts through `NODE_EXTRA_CA_CERTS`. Node reads that file once at + * startup, so it is made here, before any web server starts, and kept while it exists: the + * config module loads again in every worker, which must not swap the pair under a running server. + */ +function ensureGitHostCertificate() { + if (fs.existsSync(gitHostKeyPath) && fs.existsSync(gitHostCertPath)) { + return; + } + fs.mkdirSync(path.dirname(gitHostCertPath), { recursive: true }); + execFileSync( + 'openssl', + [ + 'req', + '-x509', + '-newkey', + 'rsa:2048', + '-nodes', + '-keyout', + gitHostKeyPath, + '-out', + gitHostCertPath, + '-days', + '30', + '-subj', + '/CN=localhost', + '-addext', + 'subjectAltName=DNS:localhost', + ], + { stdio: 'ignore' }, + ); +} + function neutralizeCredentialEnv(env: NodeJS.ProcessEnv, keep: Set) { for (const key of Object.keys(env)) { if (!keep.has(key) && SECRET_KEY_PATTERN.test(key)) { @@ -424,6 +471,7 @@ function neutralizeDotenvSecrets(envFile: string, keep: Set) { } writeRuntimeMockConfig(); +ensureGitHostCertificate(); neutralizeCredentialEnv(process.env, preservedCredentialEnvKeys); Object.assign(process.env, baseEnv); neutralizeDotenvSecrets(path.resolve(rootPath, '.env'), preservedCredentialEnvKeys); @@ -549,6 +597,22 @@ export default defineConfig({ timeout: 60_000, reuseExistingServer: false, }, + { + // Fake GitHub Enterprise host for the pull request scenarios (`pullRequests.host`). + command: `node ${gitHostServerPath}`, + cwd: rootPath, + env: { + ...process.env, + E2E_GIT_HOST_PORT: GIT_HOST_PORT, + E2E_GIT_HOST_KEY: gitHostKeyPath, + E2E_GIT_HOST_CERT: gitHostCertPath, + }, + url: `https://localhost:${GIT_HOST_PORT}/`, + ignoreHTTPSErrors: true, + stdout: 'pipe', + timeout: 60_000, + reuseExistingServer: false, + }, { // Fake RAG (vector DB) API for file-provisioning specs (RAG_API_URL). command: `node ${ragServerPath}`, diff --git a/e2e/setup/fake-git-host-server.js b/e2e/setup/fake-git-host-server.js new file mode 100644 index 00000000000..6b97666a1e0 --- /dev/null +++ b/e2e/setup/fake-git-host-server.js @@ -0,0 +1,126 @@ +/** + * HTTPS fixture standing in for a GitHub Enterprise Server host, for the pull request scenarios. + * + * The mock config points `endpoints.agents.pullRequests.host` at `localhost:`, so the + * server's GitHub source sends every request of a lookup to `/api/v3` here, over TLS with the + * self-signed certificate `playwright.config.mock.ts` generates and hands the app server through + * `NODE_EXTRA_CA_CERTS`. It serves one repository with one open pull request per branch named in + * `PULLS`, and records what it was asked so a scenario can prove which host a lookup reached. + * + * `GET /__requests` lists the recorded requests (method, path, whether a bearer token came with + * it); `POST /__reset` clears them. Neither is under `/api/v3`, so a lookup never reaches them. + */ +const fs = require('fs'); +const https = require('https'); + +const PORT = Number(process.env.E2E_GIT_HOST_PORT) || 8793; +const HOST = `localhost:${PORT}`; +const REPO = 'e2e-org/e2e-app'; +const API_PREFIX = '/api/v3'; + +/** One open pull request per branch; the scenarios record the same head on their lane. */ +const PULLS = { + 'feat/enterprise-header': { + number: 7, + title: 'Enterprise header pull request', + sha: 'a'.repeat(40), + }, + 'feat/override-ignored': { + number: 8, + title: 'Override ignored pull request', + sha: 'b'.repeat(40), + }, +}; + +const requests = []; + +const byNumber = (number) => + Object.entries(PULLS).find(([, pull]) => pull.number === number) ?? null; + +function listItem([, pull]) { + return { number: pull.number, state: 'open', head: { sha: pull.sha } }; +} + +function detail([branch, pull]) { + return { + number: pull.number, + title: pull.title, + html_url: `https://${HOST}/${REPO}/pull/${pull.number}`, + state: 'open', + merged: false, + draft: false, + additions: 12, + deletions: 3, + mergeable: true, + head: { sha: pull.sha, ref: branch, repo: { full_name: REPO } }, + }; +} + +function send(res, status, body) { + res.writeHead(status, { 'content-type': 'application/json' }); + res.end(JSON.stringify(body)); +} + +function route(url) { + const repoBase = `${API_PREFIX}/repos/${REPO}`; + if (!url.pathname.startsWith(repoBase)) return null; + const rest = url.pathname.slice(repoBase.length); + if (rest === '/pulls') { + const state = url.searchParams.get('state'); + const headFilter = url.searchParams.get('head') ?? ''; + const branch = headFilter.slice(headFilter.indexOf(':') + 1); + const pull = PULLS[branch]; + return [200, state === 'open' && pull != null ? [listItem([branch, pull])] : []]; + } + const pullMatch = /^\/pulls\/(\d+)$/.exec(rest); + if (pullMatch) { + const found = byNumber(Number(pullMatch[1])); + return found ? [200, detail(found)] : [404, { message: 'Not Found' }]; + } + if (/^\/commits\/[a-f0-9]{40}\/check-runs$/.test(rest)) { + return [200, { total_count: 1, check_runs: [{ status: 'completed', conclusion: 'success' }] }]; + } + if (/^\/commits\/[a-f0-9]{40}\/pulls$/.test(rest)) { + return [200, []]; + } + return null; +} + +const server = https.createServer( + { + key: fs.readFileSync(process.env.E2E_GIT_HOST_KEY), + cert: fs.readFileSync(process.env.E2E_GIT_HOST_CERT), + }, + (req, res) => { + const url = new URL(req.url ?? '/', `https://${HOST}`); + if (url.pathname === '/') { + send(res, 200, { ok: true }); + return; + } + if (url.pathname === '/__requests' && req.method === 'GET') { + send(res, 200, requests); + return; + } + if (url.pathname === '/__reset' && req.method === 'POST') { + requests.length = 0; + send(res, 200, { ok: true }); + return; + } + requests.push({ + method: req.method, + path: `${url.pathname}${url.search}`, + authorized: (req.headers.authorization ?? '').startsWith('Bearer '), + }); + const answer = req.method === 'GET' ? route(url) : null; + if (answer == null) { + send(res, 404, { message: 'Not Found' }); + return; + } + send(res, answer[0], answer[1]); + }, +); + +/** Every local address, since `localhost` may resolve to either family. */ +server.listen(PORT, () => { + console.log(`[fake-git-host] listening on https://${HOST}`); +}); diff --git a/e2e/specs/mock/scenarios/pull-request-enterprise-host.spec.ts b/e2e/specs/mock/scenarios/pull-request-enterprise-host.spec.ts new file mode 100644 index 00000000000..6cdbb3af1eb --- /dev/null +++ b/e2e/specs/mock/scenarios/pull-request-enterprise-host.spec.ts @@ -0,0 +1,176 @@ +import { randomUUID } from 'crypto'; +import { expect, test } from '@playwright/test'; +import type { APIRequestContext, Page } from '@playwright/test'; +import { deleteConversations, seedConversations, withMongo } from '../db'; +import { getPrimaryE2EUser } from '../../../setup/users.mock'; + +/** + * Pull request lookups against a GitHub Enterprise host. The mock config sets + * `endpoints.agents.pullRequests.host` to the fake host `e2e/setup/fake-git-host-server.js` + * serves over TLS, so a code conversation whose lane names a branch with an open pull request + * shows it in the chat header, read from that host and linking to that host's page. The host + * decides where the deployment's token goes, so a principal config override cannot move it. + */ + +const GIT_HOST = `localhost:${process.env.E2E_GIT_HOST_PORT || '8793'}`; +const REPO = 'e2e-org/e2e-app'; + +type Lane = { branch: string; head: string; title: string; number: number }; + +/** Branches and heads `fake-git-host-server.js` serves an open pull request for. */ +const HEADER_LANE: Lane = { + branch: 'feat/enterprise-header', + head: 'a'.repeat(40), + title: 'Enterprise header pull request', + number: 7, +}; +const OVERRIDE_LANE: Lane = { + branch: 'feat/override-ignored', + head: 'b'.repeat(40), + title: 'Override ignored pull request', + number: 8, +}; + +test.use({ viewport: { width: 1280, height: 800 } }); + +/** A conversation that ran in an attached workspace, with the lane the worker last reported. */ +async function seedCodeConversation(lane: Lane): Promise { + const conversationId = randomUUID(); + const { email } = getPrimaryE2EUser(); + await seedConversations(email, [ + { conversationId, title: `Lane ${lane.branch}`, updatedAt: new Date() }, + ]); + await withMongo(async (db) => { + await db.collection('conversations').updateOne( + { conversationId }, + { + $set: { + codeEnvironmentMode: 'attached', + laneGit: { branch: lane.branch, head: lane.head, repo: REPO, seq: 1 }, + }, + }, + ); + }); + return conversationId; +} + +type SeenRequest = { method: string; path: string; authorized: boolean }; + +async function gitHostRequests(request: APIRequestContext): Promise { + const res = await request.get(`https://${GIT_HOST}/__requests`, { ignoreHTTPSErrors: true }); + expect(res.ok()).toBeTruthy(); + return (await res.json()) as SeenRequest[]; +} + +/** Opens the conversation and the pull request card under its header chip. */ +async function openHeaderCard(page: Page, conversationId: string, lane: Lane) { + await page.goto(`/c/${conversationId}`); + const chip = page.getByTestId('header-pull-request-button'); + await expect(chip).toBeVisible({ timeout: 30000 }); + await expect(chip).toContainText(lane.title); + await chip.focus(); + const card = page.getByRole('dialog', { name: 'Pull request' }); + await expect(card).toBeVisible(); + await expect(card.getByText(`PR #${lane.number}`)).toBeVisible(); + return card; +} + +test.describe('Pull requests on a GitHub Enterprise host', () => { + test.describe.configure({ mode: 'serial' }); + + /** Waits for the config cache and a lookup, so a failure reports its assertion, not a timeout. */ + test.setTimeout(120_000); + + const seeded: string[] = []; + let overriddenUser: string | undefined; + + test.afterAll(async () => { + await deleteConversations(seeded); + if (overriddenUser != null) { + const principalId = overriddenUser; + await withMongo((db) => db.collection('configs').deleteMany({ principalId })); + } + }); + + test('a code conversation shows its pull request from the configured host @scenario:enterprise-host-pull-request-appears-in-header', async ({ + page, + request, + }) => { + const conversationId = await seedCodeConversation(HEADER_LANE); + seeded.push(conversationId); + + const card = await openHeaderCard(page, conversationId, HEADER_LANE); + await expect(card.getByRole('link', { name: 'Open in GitHub' })).toHaveAttribute( + 'href', + `https://${GIT_HOST}/${REPO}/pull/${HEADER_LANE.number}`, + ); + + const seen = await gitHostRequests(request); + const lookups = seen.filter((entry) => + entry.path.includes(encodeURIComponent(HEADER_LANE.branch)), + ); + expect(lookups.length).toBeGreaterThan(0); + expect(lookups.every((entry) => entry.path.startsWith(`/api/v3/repos/${REPO}/`))).toBe(true); + expect(seen.every((entry) => entry.authorized)).toBe(true); + }); + + test('a principal override naming another host does not move the lookup @scenario:role-override-cannot-move-pull-request-host', async ({ + page, + request, + }) => { + const user = getPrimaryE2EUser(); + const login = await request.post('/api/auth/login', { + data: { email: user.email, password: user.password }, + }); + expect(login.ok()).toBeTruthy(); + const { token, user: body } = (await login.json()) as { + token: string; + user: { id?: string; _id?: string }; + }; + const userId = (body.id ?? body._id) as string; + overriddenUser = userId; + const headers = { Authorization: `Bearer ${token}` }; + const overridePath = `/api/admin/config/user/${userId}`; + const marker = `pull-request-override-${Date.now()}`; + + const stored = await request.put(overridePath, { + headers, + data: { + overrides: { + interface: { customWelcome: marker }, + endpoints: { agents: { pullRequests: { host: 'pull-requests.e2e.invalid' } } }, + }, + }, + }); + expect(stored.ok()).toBeTruthy(); + try { + /** The override as a whole applies, so only the host is held back. */ + await expect + .poll( + async () => { + const res = await request.get('/api/config', { headers }); + const config = (await res.json()) as { interface?: { customWelcome?: string } }; + return config.interface?.customWelcome; + }, + { timeout: 30000, intervals: [500, 1000, 2000] }, + ) + .toBe(marker); + + const conversationId = await seedCodeConversation(OVERRIDE_LANE); + seeded.push(conversationId); + const card = await openHeaderCard(page, conversationId, OVERRIDE_LANE); + await expect(card.getByRole('link', { name: 'Open in GitHub' })).toHaveAttribute( + 'href', + `https://${GIT_HOST}/${REPO}/pull/${OVERRIDE_LANE.number}`, + ); + + const seen = await gitHostRequests(request); + expect( + seen.some((entry) => entry.path.includes(encodeURIComponent(OVERRIDE_LANE.branch))), + ).toBe(true); + } finally { + const cleared = await request.delete(overridePath, { headers }); + expect([200, 204, 404]).toContain(cleared.status()); + } + }); +}); From cbbdea8a850a706c7b3adcc1afa01a6dc4697e9b Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:07:21 +0200 Subject: [PATCH 05/12] fix: Keep the Pull Request Provider Out of Overrides and the Batch Share Key A role, group, user or tenant override can no longer change pullRequests.provider, so feature gating and link recording agree with the lookup, and a batch lookup for one provider is never joined by a request for another. --- packages/api/src/pulls/handler.spec.ts | 32 +++++++++++++++++++ packages/api/src/pulls/handler.ts | 1 + packages/api/src/pulls/links.spec.ts | 17 +++++++--- packages/api/src/pulls/settings.ts | 4 ++- packages/data-provider/src/config.ts | 9 ++++-- .../data-schemas/src/app/resolution.spec.ts | 5 +++ 6 files changed, 60 insertions(+), 8 deletions(-) diff --git a/packages/api/src/pulls/handler.spec.ts b/packages/api/src/pulls/handler.spec.ts index ccec6dde27c..80c28de9534 100644 --- a/packages/api/src/pulls/handler.spec.ts +++ b/packages/api/src/pulls/handler.spec.ts @@ -1115,6 +1115,38 @@ describe('createConversationPullRequestsHandler', () => { expect(hosts).toEqual(['github.com', 'ghe.example.com']); }); + it('does not join a lookup started for another provider on the same host', async () => { + const providers: Array = []; + const lookup = jest.fn( + (input: { provider?: string }) => + new Promise(() => { + providers.push(input.provider); + }), + ); + const settings = { ...enabled, host: 'git.example.com', batchTimeoutSeconds: 1 }; + const getAppConfig = jest + .fn() + .mockResolvedValueOnce(configWith(settings)) + .mockResolvedValueOnce(configWith(settings)) + .mockResolvedValue(configWith({ ...settings, provider: 'gitlab' as const })); + const handler = createConversationPullRequestsHandler({ + getConvosLaneGit: jest.fn(async () => [lane('a', 'o/r', 'feat/shared')]), + getAppConfig, + lookup: lookup as never, + env: { GH_TOKEN: 'ghp_secret' }, + }); + const call = () => + handler( + { user: { id: 'u1' }, body: { conversationIds: ['a'] } } as unknown as ServerRequest, + { status: jest.fn().mockReturnThis(), json: jest.fn() } as unknown as Response, + ); + void call(); + await flush(); + void call(); + await flush(); + expect(providers).toEqual(['github', 'gitlab']); + }); + it('gives every conversation on a shared branch the same failure when that lookup fails', async () => { const lookup = jest .fn() diff --git a/packages/api/src/pulls/handler.ts b/packages/api/src/pulls/handler.ts index 7037758129a..4f7a190dc26 100644 --- a/packages/api/src/pulls/handler.ts +++ b/packages/api/src/pulls/handler.ts @@ -467,6 +467,7 @@ export function createConversationPullRequestsHandler(deps: { const input = lookupInput(settings, token, lane); /** The same lane under the same policy is one lookup for every request that wants it. */ const shareKey = `${scope}\0${JSON.stringify([ + input.provider, input.host, input.repo, input.branch, diff --git a/packages/api/src/pulls/links.spec.ts b/packages/api/src/pulls/links.spec.ts index 681c44ed5fb..3798e208dc1 100644 --- a/packages/api/src/pulls/links.spec.ts +++ b/packages/api/src/pulls/links.spec.ts @@ -40,18 +40,26 @@ describe('findPullRequestLink', () => { it('finds a GitLab merge request, in a nested group too', () => { const output = 'View it on GitLab: https://gitlab.com/g/r/-/merge_requests/14\n'; - expect(findPullRequestLink(output, 'g/r')).toBe(14); + expect(findPullRequestLink(output, 'g/r', 'gitlab.com')).toBe(14); expect( - findPullRequestLink('https://gitlab.example.com/g/sub/r/-/merge_requests/3', 'g/sub/r'), + findPullRequestLink( + 'https://gitlab.example.com/g/sub/r/-/merge_requests/3', + 'g/sub/r', + 'gitlab.example.com', + ), ).toBe(3); }); it('does not take a merge request of a project nested under the lane one', () => { - expect(findPullRequestLink('https://gitlab.com/g/r/x/-/merge_requests/3', 'g/r')).toBeNull(); + expect( + findPullRequestLink('https://gitlab.com/g/r/x/-/merge_requests/3', 'g/r', 'gitlab.com'), + ).toBeNull(); }); it('finds a Bitbucket pull request', () => { - expect(findPullRequestLink('https://bitbucket.org/w/r/pull-requests/21', 'w/r')).toBe(21); + expect( + findPullRequestLink('https://bitbucket.org/w/r/pull-requests/21', 'w/r', 'bitbucket.org'), + ).toBe(21); }); it('accepts a link to a page of the pull request', () => { @@ -75,6 +83,7 @@ describe('findPullRequestLink', () => { ['a bare number', 'opened #4'], ])('ignores %s', (_, output) => { expect(findPullRequestLink(output, 'o/r')).toBeNull(); + expect(findPullRequestLink(output, 'o/r', 'gitlab.com')).toBeNull(); }); it('ignores a number too large to be a pull request', () => { diff --git a/packages/api/src/pulls/settings.ts b/packages/api/src/pulls/settings.ts index 448fd78ad0d..c1409fd63c4 100644 --- a/packages/api/src/pulls/settings.ts +++ b/packages/api/src/pulls/settings.ts @@ -108,6 +108,8 @@ export function resolvePullRequestRecording( env: Env, ): { enabled: boolean; host?: string } { const enabled = isPullRequestFeatureActive(settings, env); - const host = enabled ? resolvePullRequestHost(settings?.host) : null; + const provider = resolvePullRequestProvider(settings?.provider); + const host = + enabled && provider != null ? resolvePullRequestHost(settings?.host, provider) : null; return host != null ? { enabled, host } : { enabled }; } diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 321e8ac3eae..ecb9f888d02 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -113,9 +113,12 @@ export const BASE_ONLY_CONFIG_SECTIONS = ['filters', 'mcpAppSandbox'] as const; * not be overridden or tombstoned by role, group, or user config documents. */ export const BASE_PRINCIPAL_CONFIG_SECTIONS = ['langfuse'] as const; /** Fields inside overridable sections that only the operator's YAML sets: a role, group, user - * or tenant config document can neither change nor remove them. The pull request host decides - * where the deployment's token is sent. */ -export const BASE_ONLY_CONFIG_PATHS = ['endpoints.agents.pullRequests.host'] as const; + * or tenant config document can neither change nor remove them. The pull request provider and + * host decide where the deployment's token is sent. */ +export const BASE_ONLY_CONFIG_PATHS = [ + 'endpoints.agents.pullRequests.provider', + 'endpoints.agents.pullRequests.host', +] as const; export const defaultRetrievalModels = [ 'gpt-4o', diff --git a/packages/data-schemas/src/app/resolution.spec.ts b/packages/data-schemas/src/app/resolution.spec.ts index 87a3a01bb90..504bee0c095 100644 --- a/packages/data-schemas/src/app/resolution.spec.ts +++ b/packages/data-schemas/src/app/resolution.spec.ts @@ -163,6 +163,11 @@ describe('mergeConfigOverrides', () => { ]); expect(changed).toMatchObject({ host: 'ghe.example.com', cacheTtlSeconds: 60 }); + const provider = read([ + fakeConfig({ endpoints: { agents: { pullRequests: { provider: 'gitlab' } } } }, 10), + ]); + expect(provider.provider).toBeUndefined(); + const removed = read([fakeConfig({}, 10, ['endpoints.agents.pullRequests.host'])]); expect(removed.host).toBe('ghe.example.com'); From 6ecd13169c7ba7765fe7cfce6f1b36a964b4246b Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:16:49 +0200 Subject: [PATCH 06/12] fix: Read GitLab History Apart From Open Merge Requests Merged and closed merge requests are listed on their own, so open fork merge requests on the branch name cannot crowd them out. Rate limit cooldowns are scoped by provider, and a workspace repository with a dot segment is rejected. --- packages/api/src/pulls/gitlab.spec.ts | 41 +++++++++++++++---- packages/api/src/pulls/gitlab.ts | 22 ++++++---- packages/api/src/pulls/lookup.spec.ts | 1 + packages/api/src/pulls/lookup.ts | 4 +- .../src/code/environment.spec.ts | 13 ++++++ packages/data-provider/src/code/workspace.ts | 3 +- 6 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 packages/data-provider/src/code/environment.spec.ts diff --git a/packages/api/src/pulls/gitlab.spec.ts b/packages/api/src/pulls/gitlab.spec.ts index 347425a7681..b22c88eb963 100644 --- a/packages/api/src/pulls/gitlab.spec.ts +++ b/packages/api/src/pulls/gitlab.spec.ts @@ -37,7 +37,8 @@ function sourceFor(routes: Record Response>) { const routes = (detail = mergeRequest(), overrides: Record Response> = {}) => ({ 'state=opened': () => json(listed()), - 'state=all': () => json([]), + 'state=merged': () => json([]), + 'state=closed': () => json([]), '/merge_requests/7': () => json(detail), '/api/graphql': () => json(stats()), ...overrides, @@ -118,7 +119,8 @@ describe('createGitLabPullRequestSource', () => { ])('reports a %s merge request as %s, with no conflict state', async (state, expected) => { const { source } = sourceFor({ 'state=opened': () => json([]), - 'state=all': () => json(listed({ state })), + 'state=merged': () => json([]), + 'state=closed': () => json(listed({ state })), '/merge_requests/7': () => json(mergeRequest({ state, has_conflicts: true })), '/api/graphql': () => json(stats()), }); @@ -237,22 +239,47 @@ describe('createGitLabPullRequestSource', () => { }); }); - it('finds a closed merge request behind a newer open fork, without a recorded commit', async () => { - const fork = { iid: 9, state: 'opened', sha, source_project_id: 2, target_project_id: 1 }; + it('finds a merged merge request behind more open forks than one page holds', async () => { + const forks = Array.from({ length: 10 }, (_, index) => ({ + iid: 20 + index, + state: 'opened', + sha, + source_project_id: 2, + target_project_id: 1, + })); const merged = { iid: 7, state: 'merged', sha, source_project_id: 1, target_project_id: 1 }; const { source, fetchFn } = sourceFor({ - 'state=opened': () => json([]), - 'state=all': () => json([fork, merged]), + 'state=opened': () => json(forks), + 'state=merged': () => json([merged]), + 'state=closed': () => json([]), '/merge_requests/7': () => json(mergeRequest({ state: 'merged' })), '/api/graphql': () => json(stats()), }); await expect(find(source)).resolves.toMatchObject({ number: 7, state: 'merged' }); const history = fetchFn.mock.calls .map(([url]) => String(url)) - .find((url) => url.includes('state=all')); + .find((url) => url.includes('state=merged')); expect(new URL(history ?? '').searchParams.get('per_page')).toBe('10'); }); + it('takes the newest of the merged and closed merge requests', async () => { + const item = (iid: number, state: string) => ({ + iid, + state, + sha, + source_project_id: 1, + target_project_id: 1, + }); + const { source } = sourceFor({ + 'state=opened': () => json([]), + 'state=merged': () => json([item(3, 'merged')]), + 'state=closed': () => json([item(7, 'closed')]), + '/merge_requests/7': () => json(mergeRequest({ state: 'closed' })), + '/api/graphql': () => json(stats()), + }); + await expect(find(source)).resolves.toMatchObject({ number: 7, state: 'closed' }); + }); + it('drops a merge request whose source branch changed after it was listed', async () => { const { source } = sourceFor(routes(mergeRequest({ source_branch: 'other' }))); await expect(find(source)).resolves.toBeNull(); diff --git a/packages/api/src/pulls/gitlab.ts b/packages/api/src/pulls/gitlab.ts index 402a1d84cc1..0206461f17f 100644 --- a/packages/api/src/pulls/gitlab.ts +++ b/packages/api/src/pulls/gitlab.ts @@ -47,6 +47,7 @@ type GitLabMergeRequest = { }; type ListItem = { iid: number; state: string; sha: string; fromFork: boolean }; +type HistoryState = 'opened' | 'merged' | 'closed'; /** A failed or canceled pipeline outranks one still running; anything unfinished is running. */ export function summarizePipeline(status: string | null): PullRequestChecks { @@ -297,7 +298,7 @@ export function createGitLabPullRequestSource({ } /** `undefined`: the project is not visible. An empty list: it is, and nothing matched. */ - const list = async (state: 'opened' | 'all', perPage: number, page: number) => { + const list = async (state: HistoryState, perPage: number, page: number) => { const query = new URLSearchParams({ state, source_branch: branch, @@ -312,11 +313,11 @@ export function createGitLabPullRequestSource({ return listed.map(parseListItem); }; /** - * Reads pages of one state up to the configured budget. The history is read a full page at - * a time even without a recorded commit: it also lists open merge requests from forks, which - * are skipped, so its newest entry is not necessarily the one that matches. + * Reads pages of one state up to the configured budget, a full page at a time even without + * a recorded commit: a fork's merge request never matches without one, so the newest entry + * is not necessarily the one that does. */ - const listAll = async (state: 'opened' | 'all'): Promise => { + const listAll = async (state: HistoryState): Promise => { const perPage = lookup.maxCandidatePullRequests; const pages = lookup.maxCandidatePages; const all: ListItem[] = []; @@ -338,9 +339,14 @@ export function createGitLabPullRequestSource({ /** Open first on its own, so closed history on a reused branch name cannot hide it. */ const open = await listAll('opened'); if (open === undefined) return null; - const chosen = - (await choose(open)) ?? - (await choose(((await listAll('all')) ?? []).filter((item) => item.state !== 'opened'))); + /** Merged and closed are listed apart from open ones, so any number of open fork merge + * requests on the branch name cannot crowd the history out of its page budget. Newest + * first across both: a project's `iid` grows with every merge request it creates. */ + const history = async (): Promise => { + const [merged, closed] = await Promise.all([listAll('merged'), listAll('closed')]); + return [...(merged ?? []), ...(closed ?? [])].sort((a, b) => b.iid - a.iid); + }; + const chosen = (await choose(open)) ?? (await choose(await history())); if (chosen == null) return null; const detail = await getJson(`/merge_requests/${chosen.iid}`); diff --git a/packages/api/src/pulls/lookup.spec.ts b/packages/api/src/pulls/lookup.spec.ts index 1e381a39b7c..76bd6e409c5 100644 --- a/packages/api/src/pulls/lookup.spec.ts +++ b/packages/api/src/pulls/lookup.spec.ts @@ -200,6 +200,7 @@ describe('rate limit cooldown', () => { ok: true, value, }); + await expect(lookup({ ...input, provider: 'gitlab' })).resolves.toEqual({ ok: true, value }); await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual(rateLimited); }); diff --git a/packages/api/src/pulls/lookup.ts b/packages/api/src/pulls/lookup.ts index b4226a4fd7f..8e98f5622d8 100644 --- a/packages/api/src/pulls/lookup.ts +++ b/packages/api/src/pulls/lookup.ts @@ -141,8 +141,8 @@ export function createPullRequestLookup({ if (cached != null && cached.expiresAt > now()) return cached.result; const pending = inflight.get(key); if (pending != null) return pending; - /** A rate limit belongs to the credential on one host; the same token elsewhere is not limited. */ - const limitScope = `${scope}\0${host ?? ''}`; + /** A rate limit belongs to the credential on one provider host; elsewhere it is not limited. */ + const limitScope = `${scope}\0${provider ?? ''}\0${host ?? ''}`; const until = cooldowns.get(limitScope); if (until != null) { if (until > now()) return rateLimited; diff --git a/packages/data-provider/src/code/environment.spec.ts b/packages/data-provider/src/code/environment.spec.ts new file mode 100644 index 00000000000..420095cc12a --- /dev/null +++ b/packages/data-provider/src/code/environment.spec.ts @@ -0,0 +1,13 @@ +import { isCodeWorkspaceEnvironment } from './workspace'; + +const environment = (repo: string) => ({ fingerprint: 'a'.repeat(64), repo, actions: [] }); + +describe('isCodeWorkspaceEnvironment repository', () => { + it.each(['o/r', 'group/sub/project', 'a.b/c_d/e-f'])('accepts %s', (repo) => { + expect(isCodeWorkspaceEnvironment(environment(repo))).toBe(true); + }); + + it.each(['r', 'group/../project', './r', 'o/..', 'o//r'])('rejects %s', (repo) => { + expect(isCodeWorkspaceEnvironment(environment(repo))).toBe(false); + }); +}); diff --git a/packages/data-provider/src/code/workspace.ts b/packages/data-provider/src/code/workspace.ts index 586f1b46d6f..6b7da4bbce8 100644 --- a/packages/data-provider/src/code/workspace.ts +++ b/packages/data-provider/src/code/workspace.ts @@ -133,7 +133,8 @@ export function isCodeWorkspaceEnvironment( (environment.repo === undefined || (typeof environment.repo === 'string' && environment.repo.length <= 256 && - /^[A-Za-z0-9_.-]+(?:\/[A-Za-z0-9_.-]+){1,19}$/.test(environment.repo))) && + /^[A-Za-z0-9_.-]+(?:\/[A-Za-z0-9_.-]+){1,19}$/.test(environment.repo) && + environment.repo.split('/').every((segment) => segment !== '.' && segment !== '..'))) && (environment.ref === undefined || (typeof environment.ref === 'string' && environment.ref.trim().length > 0 && From 6a7820a81b499c9ad4108984e46d5ab4169f671f Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:59:24 +0200 Subject: [PATCH 07/12] fix: Revalidate Bitbucket Pull Requests and Skip Ones Without a Source Commit Read a full closed page so a newer closed fork cannot hide an older pull request from the repository, recheck the source branch and fork status on the detail, and treat a pull request without a source commit as unmatchable instead of failing the lookup. --- packages/api/src/pulls/bitbucket.spec.ts | 48 +++++++++++++++++++++++- packages/api/src/pulls/bitbucket.ts | 41 ++++++++++++-------- 2 files changed, 72 insertions(+), 17 deletions(-) diff --git a/packages/api/src/pulls/bitbucket.spec.ts b/packages/api/src/pulls/bitbucket.spec.ts index 488b4b9f228..b97c3dfd1ae 100644 --- a/packages/api/src/pulls/bitbucket.spec.ts +++ b/packages/api/src/pulls/bitbucket.spec.ts @@ -127,8 +127,46 @@ describe('createBitbucketPullRequestSource', () => { 'source.branch.name = "feat/x" AND (state = "MERGED" OR state = "DECLINED" OR state = "SUPERSEDED")', ); expect(closed.getAll('state')).toEqual(['MERGED', 'DECLINED', 'SUPERSEDED']); - /** Without a recorded commit only the newest closed one counts. */ - expect(closed.get('pagelen')).toBe('1'); + /** A full page even without a recorded commit, so a closed fork cannot hide the rest. */ + expect(closed.get('pagelen')).toBe('10'); + }); + + it('finds an older closed pull request behind a newer closed fork on the same branch', async () => { + const merged = pullRequest({ id: 3, state: 'MERGED' }); + const { source } = sourceFor( + routes(merged, { + 'state=OPEN': () => json(page([])), + 'state=MERGED': () => + json(page([pullRequest({ id: 8, state: 'DECLINED', source: fork() }), merged])), + '/pullrequests/3/diffstat': () => json(page([])), + '/pullrequests/3/statuses': () => json(page([])), + '/pullrequests/3': () => json(merged), + }), + ); + await expect(find(source)).resolves.toMatchObject({ number: 3, state: 'merged' }); + }); + + it('skips a pull request whose source commit is gone and keeps searching', async () => { + const gone = pullRequest({ id: 5, source: { ...pullRequest().source, commit: null } }); + const { source } = sourceFor( + routes(pullRequest(), { 'state=OPEN': () => json(page([gone, pullRequest()])) }), + ); + await expect(find(source)).resolves.toMatchObject({ number: 7 }); + }); + + it('finds nothing when the detail lost its source commit', async () => { + const gone = pullRequest({ source: { ...pullRequest().source, commit: {} } }); + const { source } = sourceFor(routes(gone)); + await expect(find(source)).resolves.toBeNull(); + }); + + it.each([ + ['moved to another source branch', { branch: { name: 'other' } }], + ['moved to a fork', { repository: { full_name: 'someone/r' } }], + ])('finds nothing when the pull request %s after it was listed', async (_case, change) => { + const moved = pullRequest({ source: { ...pullRequest().source, ...change } }); + const { source } = sourceFor(routes(moved)); + await expect(find(source, { head: sha })).resolves.toBeNull(); }); it('escapes a quote and a backslash in the branch name', async () => { @@ -392,6 +430,12 @@ describe('createBitbucketPullRequestSource', () => { expect(urlsOf(fetchFn).some((url) => url.includes('/pullrequests?'))).toBe(false); }); + it('is ignored without a source commit, and the branch is searched instead', async () => { + const gone = pullRequest({ id: 9, source: { ...pullRequest().source, commit: null } }); + const { source } = sourceFor(routes(pullRequest(), { '/pullrequests/9': () => json(gone) })); + await expect(find(source, { pullRequest: 9 })).resolves.toMatchObject({ number: 7 }); + }); + it('is ignored for another branch, and the branch is searched instead', async () => { const other = pullRequest({ id: 9, diff --git a/packages/api/src/pulls/bitbucket.ts b/packages/api/src/pulls/bitbucket.ts index e38cf469676..9335bb9d68e 100644 --- a/packages/api/src/pulls/bitbucket.ts +++ b/packages/api/src/pulls/bitbucket.ts @@ -54,6 +54,9 @@ type BitbucketPullRequest = { type ListItem = { id: number; state: string; hash: string; fromFork: boolean }; +/** A pull request whose source commit is gone (a deleted branch, a collected commit) proves nothing. */ +type Parsed = Omit & { hash: string | null }; + /** Two commit hashes, either possibly abbreviated, name the same commit. */ export function isSameCommit(a: string, b: string): boolean { const length = Math.min(a.length, b.length); @@ -91,7 +94,7 @@ function parseCommon(value: unknown, repo: string) { } const { source } = value; const commit = isRecord(source.commit) ? source.commit.hash : null; - if (typeof commit !== 'string' || !COMMIT_HASH.test(commit)) { + if (commit != null && (typeof commit !== 'string' || !COMMIT_HASH.test(commit))) { throw new PullRequestSourceError('UPSTREAM_ERROR'); } const branch = isRecord(source.branch) ? source.branch.name : null; @@ -101,13 +104,13 @@ function parseCommon(value: unknown, repo: string) { value, id: value.id, state: value.state, - hash: commit, + hash: typeof commit === 'string' ? commit : null, sourceBranch: typeof branch === 'string' ? branch : null, fromFork: typeof fullName !== 'string' || fullName.toLowerCase() !== repo.toLowerCase(), }; } -function parsePullRequest(value: unknown, repo: string): BitbucketPullRequest { +function parsePullRequest(value: unknown, repo: string): Parsed { const common = parseCommon(value, repo); const links = common.value.links; const html = isRecord(links) && isRecord(links.html) ? links.html.href : null; @@ -126,7 +129,7 @@ function parsePullRequest(value: unknown, repo: string): BitbucketPullRequest { }; } -function parseListItem(value: unknown, repo: string): ListItem { +function parseListItem(value: unknown, repo: string): Parsed { const { id, state, hash, fromFork } = parseCommon(value, repo); return { id, state, hash, fromFork }; } @@ -158,6 +161,9 @@ function statusCommit(value: Record): string | null { return /\/commit\/([a-f0-9]{7,64})$/.exec(href)?.[1] ?? null; } +const isComplete = (item: T): item is T & { hash: string } => + item.hash != null; + /** BBQL string literal: a backslash or a double quote inside it is escaped with a backslash. */ const bbqlString = (value: string): string => `"${value.replace(/[\\"]/g, '\\$&')}"`; @@ -337,7 +343,9 @@ export function createBitbucketPullRequestSource({ const linked = await getJson(`/pullrequests/${pullRequest}`); if (linked != null) { const pull = parsePullRequest(linked, repo); - if (pull.sourceBranch === branch && (await matches(pull))) return report(pull); + if (isComplete(pull) && pull.sourceBranch === branch && (await matches(pull))) { + return report(pull); + } } } @@ -357,19 +365,18 @@ export function createBitbucketPullRequestSource({ if (listed == null) return undefined; return parsePage(listed).values.map((item) => parseListItem(item, repo)); }; - /** Without a recorded commit only the newest closed one matters, as on GitHub. */ + /** + * A full page even without a recorded commit: the newest closed one may be a fork, which + * cannot match, and must not hide an older one from the repository itself. + */ const listAll = async (closed: boolean): Promise => { const states = closed ? CLOSED_STATES : OPEN_STATES; - const newestOnly = closed && recorded == null; - const pageLength = newestOnly - ? 1 - : Math.min(Math.max(lookup.maxCandidatePullRequests, 1), MAX_PAGE_LENGTH); - const pages = newestOnly ? 1 : lookup.maxCandidatePages; + const pageLength = Math.min(Math.max(lookup.maxCandidatePullRequests, 1), MAX_PAGE_LENGTH); const all: ListItem[] = []; - for (let page = 1; page <= pages; page++) { + for (let page = 1; page <= lookup.maxCandidatePages; page++) { const items = await list(states, pageLength, page); if (items === undefined) return page === 1 ? undefined : all; - all.push(...items); + all.push(...items.filter(isComplete)); if (items.length < pageLength) break; } return all; @@ -390,8 +397,12 @@ export function createBitbucketPullRequestSource({ const detail = await getJson(`/pullrequests/${chosen.id}`); if (detail == null) return null; const pull = parsePullRequest(detail, repo); - /** A push between the list and the detail can move the head off the recorded commit. */ - if (!isSameCommit(pull.hash, chosen.hash) && !(await contains(pull.hash))) return null; + /** The pull request may have changed between the list and the detail: its source branch, + * the repository it comes from, or a push that moved the head off the recorded commit. */ + if (!isComplete(pull) || pull.sourceBranch !== branch || pull.fromFork !== chosen.fromFork) { + return null; + } + if (!isSameCommit(pull.hash, chosen.hash) && !(await matches(pull))) return null; return report(pull); }, }; From b47fc73858e30beabacb70881ed4bf05cd5a72a7 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:12:16 +0200 Subject: [PATCH 08/12] fix: Follow Bitbucket's Next Link When Listing Pull Requests --- packages/api/src/pulls/bitbucket.spec.ts | 41 ++++++++++++++++++++++++ packages/api/src/pulls/bitbucket.ts | 35 +++++++++----------- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/packages/api/src/pulls/bitbucket.spec.ts b/packages/api/src/pulls/bitbucket.spec.ts index b97c3dfd1ae..5ab6fbd3984 100644 --- a/packages/api/src/pulls/bitbucket.spec.ts +++ b/packages/api/src/pulls/bitbucket.spec.ts @@ -146,6 +146,47 @@ describe('createBitbucketPullRequestSource', () => { await expect(find(source)).resolves.toMatchObject({ number: 3, state: 'merged' }); }); + describe('closed history across pages', () => { + const merged = pullRequest({ id: 3, state: 'MERGED' }); + const declinedFork = pullRequest({ id: 8, state: 'DECLINED', source: fork() }); + const history = (next: string) => + routes(merged, { + 'state=OPEN': () => json(page([])), + /** A short first page that still links on: only a missing `next` ends the list. */ + 'state=MERGED': () => json(page([declinedFork], next)), + 'cursor=opaque': () => json(page([merged])), + '/pullrequests/3/diffstat': () => json(page([])), + '/pullrequests/3/statuses': () => json(page([])), + '/pullrequests/3': () => json(merged), + }); + + it('follows the next link Bitbucket returns instead of building page numbers', async () => { + const { source, fetchFn } = sourceFor(history(`${API}pullrequests?cursor=opaque`)); + await expect(find(source, { limits: { maxCandidatePages: 2 } })).resolves.toMatchObject({ + number: 3, + state: 'merged', + }); + expect(urlsOf(fetchFn)).toContain(`${API}pullrequests?cursor=opaque`); + expect(urlsOf(fetchFn).some((url) => /[?&]page=/.test(url))).toBe(false); + }); + + it('reads no more pages than the configured bound', async () => { + const { source, fetchFn } = sourceFor(history(`${API}pullrequests?cursor=opaque`)); + await expect(find(source, { limits: { maxCandidatePages: 1 } })).resolves.toBeNull(); + expect(urlsOf(fetchFn)).not.toContain(`${API}pullrequests?cursor=opaque`); + }); + + it('never follows a next link off the repository API', async () => { + const { source, fetchFn } = sourceFor( + history('https://evil.example/2.0/repositories/w/r/pullrequests?cursor=opaque'), + ); + await expect(find(source, { limits: { maxCandidatePages: 2 } })).rejects.toMatchObject({ + code: 'UPSTREAM_ERROR', + }); + expect(urlsOf(fetchFn).every((url) => url.startsWith(API))).toBe(true); + }); + }); + it('skips a pull request whose source commit is gone and keeps searching', async () => { const gone = pullRequest({ id: 5, source: { ...pullRequest().source, commit: null } }); const { source } = sourceFor( diff --git a/packages/api/src/pulls/bitbucket.ts b/packages/api/src/pulls/bitbucket.ts index 9335bb9d68e..ad8743f9ec1 100644 --- a/packages/api/src/pulls/bitbucket.ts +++ b/packages/api/src/pulls/bitbucket.ts @@ -349,35 +349,32 @@ export function createBitbucketPullRequestSource({ } } - /** `undefined`: the repository is not visible. An empty list: it is, and nothing matched. */ - const list = async (states: readonly string[], pageLength: number, page: number) => { + /** + * A full page even without a recorded commit: the newest closed one may be a fork, which + * cannot match, and must not hide an older one from the repository itself. Later pages come + * from the `next` link Bitbucket returns, which is opaque, and only its absence ends the + * list. `undefined`: the repository is not visible. An empty list: it is, and nothing matched. + */ + const listAll = async (closed: boolean): Promise => { + const states = closed ? CLOSED_STATES : OPEN_STATES; + const pageLength = Math.min(Math.max(lookup.maxCandidatePullRequests, 1), MAX_PAGE_LENGTH); const filter = states.map((state) => `state = "${state}"`).join(' OR '); const query = new URLSearchParams({ q: `source.branch.name = ${bbqlString(branch)} AND (${filter})`, sort: '-created_on', pagelen: String(pageLength), - page: String(page), }); /** The default list is open pull requests only. The `state` parameters lift that default * and `q` names the same states, so either filter alone lists the same pull requests. */ for (const state of states) query.append('state', state); - const listed = await getJson(`/pullrequests?${query.toString()}`); - if (listed == null) return undefined; - return parsePage(listed).values.map((item) => parseListItem(item, repo)); - }; - /** - * A full page even without a recorded commit: the newest closed one may be a fork, which - * cannot match, and must not hide an older one from the repository itself. - */ - const listAll = async (closed: boolean): Promise => { - const states = closed ? CLOSED_STATES : OPEN_STATES; - const pageLength = Math.min(Math.max(lookup.maxCandidatePullRequests, 1), MAX_PAGE_LENGTH); const all: ListItem[] = []; - for (let page = 1; page <= lookup.maxCandidatePages; page++) { - const items = await list(states, pageLength, page); - if (items === undefined) return page === 1 ? undefined : all; - all.push(...items.filter(isComplete)); - if (items.length < pageLength) break; + let next: string | null = `${base}/pullrequests?${query.toString()}`; + for (let page = 1; page <= lookup.maxCandidatePages && next != null; page++) { + const listed = await request(next, lookup, []); + if (listed == null) return page === 1 ? undefined : all; + const parsed = parsePage(listed); + all.push(...parsed.values.map((item) => parseListItem(item, repo)).filter(isComplete)); + next = parsed.next; } return all; }; From f46254c40c5dfd21628d5bc5852964a59e4ac952 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:18:15 +0200 Subject: [PATCH 09/12] fix: Record the Lane Pull Request Without an Aggregation Pipeline Update --- .../data-schemas/src/methods/conversation.ts | 100 ++++++++++-------- 1 file changed, 56 insertions(+), 44 deletions(-) diff --git a/packages/data-schemas/src/methods/conversation.ts b/packages/data-schemas/src/methods/conversation.ts index afc058fce5c..e69d5180114 100644 --- a/packages/data-schemas/src/methods/conversation.ts +++ b/packages/data-schemas/src/methods/conversation.ts @@ -11,6 +11,7 @@ import type { Model, SortOrder, Types, + UpdateQuery, } from 'mongoose'; import type { SearchParams } from 'meilisearch'; import type { @@ -3176,29 +3177,6 @@ export function createConversationMethods( workspace?: { environmentId: string; workspaceId: string; required?: boolean; epoch?: number }; }): Promise { const Conversation = mongoose.models.Conversation as Model; - /** - * A pipeline update, so a report without a link keeps the one already recorded while the lane - * is still on that branch and repository. Values are literals: a branch may start with `$`. - */ - const keptPullRequest = { - $cond: [ - { - $and: [ - { $eq: ['$laneGit.branch', { $literal: laneGit.branch }] }, - { $eq: ['$laneGit.repo', repo ? { $literal: repo } : '$$REMOVE'] }, - ], - }, - '$laneGit.pullRequest', - '$$REMOVE', - ], - }; - const next = { - branch: { $literal: laneGit.branch }, - head: { $literal: laneGit.head }, - ...(repo ? { repo: { $literal: repo } } : {}), - pullRequest: pullRequest != null ? { $literal: pullRequest } : keptPullRequest, - seq: { $literal: seq }, - }; const epochFence = workspace?.epoch == null ? [] @@ -3237,28 +3215,62 @@ export function createConversationMethods( ], }, ]; - const result = await withoutMeiliIndexing( - Conversation.updateOne( - { - user, - conversationId, - ...activeExpirationFilter(), - $and: [ - { - $or: [ - { 'laneGit.seq': { $exists: false } }, - { 'laneGit.seq': null }, - { 'laneGit.seq': { $lt: seq } }, - ], - }, - ...workspaceFence, - ], + const fences: FilterQuery[] = [ + { + $or: [ + { 'laneGit.seq': { $exists: false } }, + { 'laneGit.seq': null }, + { 'laneGit.seq': { $lt: seq } }, + ], + }, + ...workspaceFence, + ]; + const write = async ( + lane: FilterQuery | null, + update: UpdateQuery, + ): Promise => { + const result = await withoutMeiliIndexing( + Conversation.updateOne( + { + user, + conversationId, + ...activeExpirationFilter(), + $and: lane == null ? fences : [...fences, lane], + }, + update, + { timestamps: false }, + ), + ); + return result.modifiedCount === 1; + }; + const replaced = { + branch: laneGit.branch, + head: laneGit.head, + ...(repo ? { repo } : {}), + ...(pullRequest != null ? { pullRequest } : {}), + seq, + }; + if (pullRequest != null) return write(null, { $set: { laneGit: replaced } }); + /** + * A report without a link keeps the one already recorded while the lane is still on that + * branch and repository, and drops it otherwise. Plain updates rather than an aggregation + * pipeline, which Amazon DocumentDB rejects: the first matches only the same lane and sets its + * fields one by one, the second only another lane and replaces the whole state. A concurrent + * write can move the stored lane between the two, so they are retried once. + */ + const sameLane = { 'laneGit.branch': laneGit.branch, 'laneGit.repo': repo ?? null }; + for (let attempt = 0; attempt < 2; attempt++) { + const kept = await write(sameLane, { + $set: { + 'laneGit.branch': laneGit.branch, + 'laneGit.head': laneGit.head, + 'laneGit.seq': seq, }, - [{ $set: { laneGit: next } }], - { timestamps: false }, - ), - ); - return result.modifiedCount === 1; + }); + if (kept) return true; + if (await write({ $nor: [sameLane] }, { $set: { laneGit: replaced } })) return true; + } + return false; } /** The last reported lane state, or null when the conversation has none or is not the owner's. */ From f9a2e473f6f7451f23392629dad17cb775c9e52c Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:25:12 +0200 Subject: [PATCH 10/12] fix: Match an Empty Lane Repository as Unknown --- packages/data-schemas/src/methods/conversation.spec.ts | 7 +++++++ packages/data-schemas/src/methods/conversation.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/data-schemas/src/methods/conversation.spec.ts b/packages/data-schemas/src/methods/conversation.spec.ts index 3741259decc..c971f1b4089 100644 --- a/packages/data-schemas/src/methods/conversation.spec.ts +++ b/packages/data-schemas/src/methods/conversation.spec.ts @@ -9504,6 +9504,13 @@ describe('laneGit', () => { await expect(read(conversationId)).resolves.not.toHaveProperty('pullRequest'); }); + it('treats an empty repository like an unknown one when keeping the link', async () => { + const conversationId = await seed(); + await record({ user, conversationId, laneGit: lane, pullRequest: 7 }); + await record({ user, conversationId, laneGit: lane, repo: '' }); + await expect(read(conversationId)).resolves.toEqual({ ...lane, pullRequest: 7 }); + }); + it('replaces the link with a newer one', async () => { const conversationId = await seed(); await record({ user, conversationId, laneGit: lane, repo: 'o/r', pullRequest: 7 }); diff --git a/packages/data-schemas/src/methods/conversation.ts b/packages/data-schemas/src/methods/conversation.ts index e69d5180114..65d8a07c3c1 100644 --- a/packages/data-schemas/src/methods/conversation.ts +++ b/packages/data-schemas/src/methods/conversation.ts @@ -3258,7 +3258,7 @@ export function createConversationMethods( * fields one by one, the second only another lane and replaces the whole state. A concurrent * write can move the stored lane between the two, so they are retried once. */ - const sameLane = { 'laneGit.branch': laneGit.branch, 'laneGit.repo': repo ?? null }; + const sameLane = { 'laneGit.branch': laneGit.branch, 'laneGit.repo': repo || null }; for (let attempt = 0; attempt < 2; attempt++) { const kept = await write(sameLane, { $set: { From 61926557eb50321d14987a9718c85580ffd78521 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:26:13 +0200 Subject: [PATCH 11/12] fix: List Locked GitLab Merge Requests With the Open Ones --- packages/api/src/pulls/gitlab.spec.ts | 25 +++++++++++++++++++++++++ packages/api/src/pulls/gitlab.ts | 23 ++++++++++++++--------- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/packages/api/src/pulls/gitlab.spec.ts b/packages/api/src/pulls/gitlab.spec.ts index b22c88eb963..178e698a14c 100644 --- a/packages/api/src/pulls/gitlab.spec.ts +++ b/packages/api/src/pulls/gitlab.spec.ts @@ -29,6 +29,8 @@ function sourceFor(routes: Record Response>) { const fetchFn = jest.fn(async (input: string, _init?: RequestInit) => { const url = String(input); const key = Object.keys(routes).find((fragment) => url.includes(fragment)); + /** Nothing is being merged unless a test says so. */ + if (key == null && url.includes('state=locked')) return json([]); if (key == null) throw new Error(`unexpected ${url}`); return routes[key](); }); @@ -262,6 +264,29 @@ describe('createGitLabPullRequestSource', () => { expect(new URL(history ?? '').searchParams.get('per_page')).toBe('10'); }); + it('finds a merge request while GitLab is merging it, ahead of older history', async () => { + const item = (iid: number, state: string) => ({ + iid, + state, + sha, + source_project_id: 1, + target_project_id: 1, + }); + const { source, fetchFn } = sourceFor({ + 'state=opened': () => json([]), + 'state=locked': () => json([item(9, 'locked')]), + 'state=merged': () => json([item(3, 'merged')]), + 'state=closed': () => json([]), + '/merge_requests/9': () => json(mergeRequest({ iid: 9, state: 'locked' })), + '/api/graphql': () => json(stats()), + }); + await expect(find(source)).resolves.toMatchObject({ number: 9 }); + const locked = fetchFn.mock.calls + .map(([url]) => new URL(String(url))) + .find((url) => url.searchParams.get('state') === 'locked'); + expect(locked?.searchParams.get('source_branch')).toBe('feat/x'); + }); + it('takes the newest of the merged and closed merge requests', async () => { const item = (iid: number, state: string) => ({ iid, diff --git a/packages/api/src/pulls/gitlab.ts b/packages/api/src/pulls/gitlab.ts index 0206461f17f..9f552628942 100644 --- a/packages/api/src/pulls/gitlab.ts +++ b/packages/api/src/pulls/gitlab.ts @@ -47,7 +47,7 @@ type GitLabMergeRequest = { }; type ListItem = { iid: number; state: string; sha: string; fromFork: boolean }; -type HistoryState = 'opened' | 'merged' | 'closed'; +type ListState = 'opened' | 'locked' | 'merged' | 'closed'; /** A failed or canceled pipeline outranks one still running; anything unfinished is running. */ export function summarizePipeline(status: string | null): PullRequestChecks { @@ -298,7 +298,7 @@ export function createGitLabPullRequestSource({ } /** `undefined`: the project is not visible. An empty list: it is, and nothing matched. */ - const list = async (state: HistoryState, perPage: number, page: number) => { + const list = async (state: ListState, perPage: number, page: number) => { const query = new URLSearchParams({ state, source_branch: branch, @@ -317,7 +317,7 @@ export function createGitLabPullRequestSource({ * a recorded commit: a fork's merge request never matches without one, so the newest entry * is not necessarily the one that does. */ - const listAll = async (state: HistoryState): Promise => { + const listAll = async (state: ListState): Promise => { const perPage = lookup.maxCandidatePullRequests; const pages = lookup.maxCandidatePages; const all: ListItem[] = []; @@ -336,15 +336,20 @@ export function createGitLabPullRequestSource({ return null; }; - /** Open first on its own, so closed history on a reused branch name cannot hide it. */ - const open = await listAll('opened'); - if (open === undefined) return null; + /** A project's `iid` grows with every merge request it creates, so it orders by age. */ + const newestFirst = (...lists: (ListItem[] | undefined)[]): ListItem[] => + lists.flatMap((items) => items ?? []).sort((a, b) => b.iid - a.iid); + /** Open first on its own, so closed history on a reused branch name cannot hide it. GitLab + * lists a merge request that is being merged as `locked`, under no other state filter, and + * it is still the branch's live merge request, so it is read with the open ones. */ + const [opened, locked] = await Promise.all([listAll('opened'), listAll('locked')]); + if (opened === undefined) return null; + const open = newestFirst(opened, locked); /** Merged and closed are listed apart from open ones, so any number of open fork merge - * requests on the branch name cannot crowd the history out of its page budget. Newest - * first across both: a project's `iid` grows with every merge request it creates. */ + * requests on the branch name cannot crowd the history out of its page budget. */ const history = async (): Promise => { const [merged, closed] = await Promise.all([listAll('merged'), listAll('closed')]); - return [...(merged ?? []), ...(closed ?? [])].sort((a, b) => b.iid - a.iid); + return newestFirst(merged, closed); }; const chosen = (await choose(open)) ?? (await choose(await history())); if (chosen == null) return null; From a0c0803495e8c6fea8d0e6b292252726bfa650eb Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:26:13 +0200 Subject: [PATCH 12/12] fix: Replace an Expiring Fake Git Host Certificate --- e2e/playwright.config.mock.ts | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/e2e/playwright.config.mock.ts b/e2e/playwright.config.mock.ts index 2fc346adc8c..a579fcabccd 100644 --- a/e2e/playwright.config.mock.ts +++ b/e2e/playwright.config.mock.ts @@ -407,14 +407,39 @@ function writeRuntimeMockConfig() { } } +/** A kept certificate is replaced when it expires within this window, so no run outlives it. */ +const GIT_HOST_CERT_MIN_SECONDS = 7 * 24 * 60 * 60; + +function hasFreshGitHostCertificate(): boolean { + if (!fs.existsSync(gitHostKeyPath) || !fs.existsSync(gitHostCertPath)) { + return false; + } + try { + /** Exits non-zero when the certificate expires within the window, or cannot be read. */ + execFileSync( + 'openssl', + ['x509', '-checkend', String(GIT_HOST_CERT_MIN_SECONDS), '-noout', '-in', gitHostCertPath], + { stdio: 'ignore' }, + ); + return true; + } catch { + return false; + } +} + /** * The fake git host speaks TLS, as every pull request host must, with a certificate for * `localhost` the app server trusts through `NODE_EXTRA_CA_CERTS`. Node reads that file once at - * startup, so it is made here, before any web server starts, and kept while it exists: the - * config module loads again in every worker, which must not swap the pair under a running server. + * startup, so it is made here, before any web server starts, and kept while it is valid for a + * week more. The config module loads again in every worker, which must not swap the pair under a + * running server, so only the main process ever replaces one. */ function ensureGitHostCertificate() { - if (fs.existsSync(gitHostKeyPath) && fs.existsSync(gitHostCertPath)) { + const exists = fs.existsSync(gitHostKeyPath) && fs.existsSync(gitHostCertPath); + if (exists && process.env.TEST_WORKER_INDEX != null) { + return; + } + if (hasFreshGitHostCertificate()) { return; } fs.mkdirSync(path.dirname(gitHostCertPath), { recursive: true });