Skip to content

Commit caeb786

Browse files
authored
🔀 feat: Show the Conversation Pull Request in the Sidebar List (#16815)
* feat: Show the Conversation Pull Request in the Sidebar List Add a batch lookup route and a sidebar row mark: the state icon with the CI dot, opening the same card beside the row on hover. The row keeps the conversation's own title. * fix: Address Review Findings on the Sidebar Pull Request Batch and Row Mark Cap the whole batch with a configurable deadline, make the row mark a keyboard-reachable button that opens the card on focus, refresh rows when the window regains focus through the shared batch, and show a retryable failure state instead of treating a failed first lookup as no pull request. * fix: Send Sidebar Pull Request Batches One at a Time Queue every batch request behind the one before it, so an oversized list or an overlapping flush cannot multiply the server's configured lookup concurrency, and bound how long one request may hold the queue so a request that never answers cannot stop the rest. * fix: Keep the Batch Request Timeout Above the Server Deadline Default the client's request timeout to just past the longest batchTimeoutSeconds a deployment may configure, so a queued request never starts while the previous request's server-side lookups are still running. * fix: Keep Batch Lookups Inside the Concurrency Limit and Per-Entry Failures Per Entry Share one lookup limiter across every batch request so lookups that outlive a request's deadline keep their slot, answer a missing token only for the conversations that needed it, and point the row at its pull request description only while that text is in the page. * fix: Bound Batch Reads, Scope Concurrency Per Credential, and Gate the Sidebar on the Batch Version Start the batch deadline when the request arrives and bound the config and lane reads, count lookup slots per credential, advertise a batch route version and ask only a server that has it, take no row space for an absent mark, show a failed refresh over a cached empty answer, keep a malformed response from stalling the request queue, and repair the lane recording tests that still used a mock removed on dev. * fix: Move Capability Resolution Into the Package and Harden the Batch Route and Queue Resolve the pull request startup capabilities in packages/api so the route stays wiring only, bound the config and lane reads by the configured deadline, merge identical lanes before taking limiter slots, fall back to the single route when a replica has no batch route, and dispose the batch queue when the signed-in user changes. * fix: Abort the Batch Fallback With Its Request So the Queue Never Overlaps It Give the batch fetcher the request's abort signal and pass it to every single-route call, abort a request that times out or is disposed, and stop the fallback workers from starting more calls once it has. * fix: Forward the Abort Signal to the Batch POST Pass the request's abort signal through the batch fetcher, the data-provider call and Axios, so a batch request that times out or is disposed is cancelled instead of continuing beside the next one, and settle a request promptly when it is aborted. * fix: Close the Pull Request Lookup Follow-Ups and Cancel Abandoned Sidebar Rows
1 parent cdf2e94 commit caeb786

47 files changed

Lines changed: 4260 additions & 128 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎api/server/routes/__test-utils__/convos-route-mocks.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,9 @@ module.exports = {
148148
),
149149
createBackgroundTaskPolicyMiddleware: jest.fn(() => (_req, _res, next) => next()),
150150
createGitHubPullRequestSource: jest.fn(() => ({ find: jest.fn() })),
151+
createProxyAwareFetch: jest.fn(() => jest.fn()),
151152
createPullRequestLookup: jest.fn(() => jest.fn()),
153+
createConversationPullRequestsHandler: jest.fn(() => jest.fn()),
152154
createConversationPullRequestHandler: jest.fn(
153155
() => (_req, res) => res.status(200).json({ pullRequest: null }),
154156
),

‎api/server/routes/__tests__/config.spec.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ describe('GET /api/config', () => {
181181
expect(response.body).not.toHaveProperty('conversationImportMaxFileSize');
182182
expect(response.body).not.toHaveProperty('insightsEnabled');
183183
expect(response.body).not.toHaveProperty('pullRequestsEnabled');
184+
expect(response.body).not.toHaveProperty('pullRequestsBatchVersion');
184185
expect(response.body).not.toHaveProperty('mcpApps');
185186
});
186187

@@ -683,9 +684,26 @@ describe('GET /api/config', () => {
683684
});
684685
const response = await request(createApp(mockUser)).get('/api/config');
685686
expect(response.body.pullRequestsEnabled).toBe(expected);
687+
/** The batch route's version rides with the flag, so a client never sees one without the other. */
688+
if (expected) {
689+
expect(response.body.pullRequestsBatchVersion).toBe(1);
690+
expect(response.body.pullRequestsMaxConcurrentLookups).toBe(4);
691+
} else {
692+
expect(response.body).not.toHaveProperty('pullRequestsBatchVersion');
693+
expect(response.body).not.toHaveProperty('pullRequestsMaxConcurrentLookups');
694+
}
686695
},
687696
);
688697

698+
it('advertises the configured lookup limit so a fallback client can keep to it', async () => {
699+
mockGetAppConfig.mockResolvedValue({
700+
...baseAppConfig,
701+
endpoints: { agents: { pullRequests: { enabled: true, maxConcurrentLookups: 2 } } },
702+
});
703+
const response = await request(createApp(mockUser)).get('/api/config');
704+
expect(response.body.pullRequestsMaxConcurrentLookups).toBe(2);
705+
});
706+
689707
it('should not advertise pull requests for a config with no endpoints', async () => {
690708
mockGetAppConfig.mockResolvedValue(baseAppConfig);
691709
const response = await request(createApp(mockUser)).get('/api/config');

‎api/server/routes/config.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const {
2323
resolveCodeWorkspaceInheritanceCapability,
2424
resolveCodeEnvironmentTransitionVersion,
2525
loadConversationListLimits,
26+
resolvePullRequestCapabilities,
2627
} = require('@librechat/api');
2728
const {
2829
DEFAULT_MCP_APP_CSP_LIMITS,
@@ -331,8 +332,7 @@ router.get('/', async function (req, res) {
331332
langfuseConnectionAccess,
332333
insightsEnabled: isEnabled(process.env.ENABLE_INSIGHTS),
333334
/** Lets the client skip the pull request lookup entirely when the feature is off. */
334-
pullRequestsEnabled:
335-
appConfig?.endpoints?.[EModelEndpoint.agents]?.pullRequests?.enabled === true,
335+
...resolvePullRequestCapabilities(appConfig),
336336
compactionEnabled: appConfig?.summarization?.enabled !== false,
337337
...(codeEnvironmentDecisionVersion != null ? { codeEnvironmentDecisionVersion } : {}),
338338
mcpApps: resolveMCPAppsPolicy(

‎api/server/routes/convos.js‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ const {
2121
createBackgroundTaskPolicyMiddleware,
2222
createConversationPullRequestHandler,
2323
createGitHubPullRequestSource,
24+
createProxyAwareFetch,
2425
createPullRequestLookup,
26+
createConversationPullRequestsHandler,
2527
backgroundTaskRegistry,
2628
createSubagentThreadViewHandler,
2729
createGeneratedTitleHandler,
@@ -170,10 +172,20 @@ const backgroundTaskIndexHandler = createBackgroundTaskIndexHandler({
170172
const backgroundTaskCancelHandler = createBackgroundTaskCancelHandler({
171173
registry: backgroundTaskRegistry,
172174
});
175+
/** One lookup, so the header's single route and the sidebar's batch route share its cache. */
176+
const pullRequestLookup = createPullRequestLookup({
177+
source: createGitHubPullRequestSource({ fetchFn: createProxyAwareFetch() }),
178+
});
173179
const conversationPullRequestHandler = createConversationPullRequestHandler({
174180
getConvoLaneGit: db.getConvoLaneGit,
175181
getAppConfig,
176-
lookup: createPullRequestLookup({ source: createGitHubPullRequestSource({ fetchFn: fetch }) }),
182+
lookup: pullRequestLookup,
183+
env: process.env,
184+
});
185+
const conversationPullRequestsHandler = createConversationPullRequestsHandler({
186+
getConvosLaneGit: db.getConvosLaneGit,
187+
getAppConfig,
188+
lookup: pullRequestLookup,
177189
env: process.env,
178190
});
179191
router.use(requireJwtAuth);
@@ -248,6 +260,7 @@ router.post(
248260
subagentControlHandler,
249261
);
250262
router.get('/:parentConversationId/subagents', parentSubagentIndexHandler);
263+
router.post('/pull-requests', conversationPullRequestsHandler);
251264
router.get('/:conversationId/pull-request', conversationPullRequestHandler);
252265
router.get('/:conversationId/background-tasks', backgroundTaskPolicy, backgroundTaskIndexHandler);
253266
router.post(

‎api/server/services/ToolService.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2376,6 +2376,7 @@ async function loadToolsForExecution({
23762376
environmentId: codeExecutionContext.codeWorkspace.environmentId,
23772377
workspaceId: codeExecutionContext.codeWorkspace.workspaceId,
23782378
},
2379+
admittedEpoch: req.resolvedConversation?.codeAttachmentEpoch,
23792380
getConvoLaneContext,
23802381
reserveConvoLaneGitSeq,
23812382
setConvoLaneGit,

‎api/server/services/__tests__/ToolService.spec.js‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3274,6 +3274,55 @@ describe('ToolService - Action Capability Gating', () => {
32743274
expect(mockSetConvoLaneGit).toHaveBeenCalledWith(input);
32753275
});
32763276

3277+
it.each([
3278+
['a moved conversation', 3, 3],
3279+
['a conversation never moved', 0, 0],
3280+
['a conversation whose admitted read carried no epoch', undefined, undefined],
3281+
])(
3282+
'fences the lane report by the epoch of the admitted decision for %s',
3283+
async (_label, admitted, expected) => {
3284+
const capabilities = [
3285+
AgentCapabilities.tools,
3286+
AgentCapabilities.execute_code,
3287+
AgentCapabilities.stateful_code_sessions,
3288+
];
3289+
const req = createMockReq(capabilities);
3290+
req.config.endpoints[EModelEndpoint.agents].pullRequests = { enabled: true };
3291+
req.resolvedConversation = {
3292+
conversationId: 'resolved-convo',
3293+
codeWorkspaces: [{ environmentId: 'personal-machine', workspaceId: 'project-a' }],
3294+
...(admitted === undefined ? {} : { codeAttachmentEpoch: admitted }),
3295+
};
3296+
req.body = {
3297+
conversationId: 'resolved-convo',
3298+
codeWorkspaces: [{ environmentId: 'personal-machine', workspaceId: 'project-a' }],
3299+
};
3300+
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
3301+
req.config.endpoints[EModelEndpoint.agents].statefulCodeSessions = {
3302+
environments: [attachedEnvironment()],
3303+
};
3304+
mockCreateLaneGitRecorder.mockClear();
3305+
3306+
await loadToolsForExecution({
3307+
req,
3308+
res: {},
3309+
agent: {
3310+
id: 'attached-agent',
3311+
tools: [Tools.execute_code],
3312+
stateful_code_sessions: true,
3313+
stateful_code_environment: 'agent-user',
3314+
code_environment_id: 'personal-machine',
3315+
},
3316+
conversationId: 'resolved-convo',
3317+
toolNames: [AgentConstants.BASH_TOOL],
3318+
toolRegistry: new Map([[AgentConstants.BASH_TOOL, { name: AgentConstants.BASH_TOOL }]]),
3319+
actionsEnabled: false,
3320+
});
3321+
3322+
expect(mockCreateLaneGitRecorder.mock.calls[0][0].admittedEpoch).toBe(expected);
3323+
},
3324+
);
3325+
32773326
it.each([
32783327
['unset', undefined],
32793328
['disabled', { enabled: false }],

‎client/src/components/Chat/PullRequest/Chip.tsx‎

Lines changed: 8 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,10 @@ import { useAgentsMapContext, useChatContext } from '~/Providers';
55
import { TONE_DOT_CLASS, presentPullRequest } from './status';
66
import { URLIcon } from '~/components/Endpoints/URLIcon';
77
import { summarizePullRequest } from './summary';
8+
import PullRequestPanel from './Panel';
89
import { useLocalize } from '~/hooks';
910
import PullRequestIcon from './Icon';
10-
import PullRequestCard from './Card';
11-
import { cn } from '~/utils';
12-
13-
/** Slides in from the chip's side, left to right, and fades; reduced motion only fades. */
14-
const cardClass = cn(
15-
'border-border-light bg-surface-secondary text-text-primary z-[200] w-80 max-w-[calc(100vw-2rem)] rounded-xl border shadow-lg focus:outline-none',
16-
'origin-left -translate-x-3 opacity-0 transition duration-200 ease-out',
17-
'data-[enter]:translate-x-0 data-[enter]:opacity-100',
18-
'data-[leave]:-translate-x-3 data-[leave]:opacity-0',
19-
'motion-reduce:translate-x-0 motion-reduce:transition-opacity',
20-
);
21-
22-
function CiDot({ dotClass }: { dotClass: string }) {
23-
return (
24-
<span
25-
data-testid="pull-request-ci-dot"
26-
className={cn(
27-
'ring-presentation absolute -right-0.5 -bottom-0.5 size-2 rounded-full ring-2',
28-
dotClass,
29-
)}
30-
/>
31-
);
32-
}
11+
import CiDot from './CiDot';
3312

3413
/** The agent's picture with the CI dot on its corner. */
3514
function AgentAvatar({
@@ -72,7 +51,6 @@ function PullRequestChip({ conversationId }: { conversationId: string }) {
7251
hideTimeout: 150,
7352
});
7453
const open = Ariakit.useStoreState(hovercard, 'open');
75-
const cardElement = Ariakit.useStoreState(hovercard, 'contentElement');
7654
const { data, isError, refetch } = useConversationPullRequestQuery(conversationId);
7755
const pullRequest = data?.pullRequest;
7856

@@ -108,21 +86,12 @@ function PullRequestChip({ conversationId }: { conversationId: string }) {
10886
</Ariakit.Button>
10987
}
11088
/>
111-
<Ariakit.Hovercard
112-
gutter={8}
113-
portal
114-
unmountOnHide
115-
autoFocusOnShow={false}
116-
aria-label={localize('com_ui_pull_request')}
117-
className={cardClass}
118-
>
119-
<PullRequestCard
120-
pullRequest={pullRequest}
121-
refreshFailed={isError}
122-
onRetry={() => void refetch()}
123-
portalElement={cardElement}
124-
/>
125-
</Ariakit.Hovercard>
89+
<PullRequestPanel
90+
store={hovercard}
91+
pullRequest={pullRequest}
92+
refreshFailed={isError}
93+
onRetry={() => void refetch()}
94+
/>
12695
</Ariakit.HovercardProvider>
12796
);
12897
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { cn } from '~/utils';
2+
3+
/**
4+
* The CI status dot that sits on the corner of the picture or icon it describes. The ring is
5+
* painted in the surface the dot sits on, so the caller names that surface.
6+
*/
7+
export default function CiDot({
8+
dotClass,
9+
ringClassName = 'ring-presentation',
10+
}: {
11+
dotClass: string;
12+
ringClassName?: string;
13+
}) {
14+
return (
15+
<span
16+
data-testid="pull-request-ci-dot"
17+
aria-hidden="true"
18+
className={cn(
19+
'pointer-events-none absolute -right-0.5 -bottom-0.5 size-2 rounded-full ring-2',
20+
ringClassName,
21+
dotClass,
22+
)}
23+
/>
24+
);
25+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import * as Ariakit from '@ariakit/react';
2+
import type { TConversationPullRequest } from 'librechat-data-provider';
3+
import { useLocalize } from '~/hooks';
4+
import PullRequestCard from './Card';
5+
import { cn } from '~/utils';
6+
7+
/** Slides in from the anchor's side, left to right, and fades; reduced motion only fades. */
8+
export const panelClass = cn(
9+
'border-border-light bg-surface-secondary text-text-primary z-[200] w-80 max-w-[calc(100vw-2rem)] rounded-xl border shadow-lg focus:outline-none',
10+
'origin-left -translate-x-3 opacity-0 transition duration-200 ease-out',
11+
'data-[enter]:translate-x-0 data-[enter]:opacity-100',
12+
'data-[leave]:-translate-x-3 data-[leave]:opacity-0',
13+
'motion-reduce:translate-x-0 motion-reduce:transition-opacity',
14+
);
15+
16+
/**
17+
* The card as a hovercard beside its anchor. It is portaled, and a portal's events still bubble
18+
* to the React tree that rendered it, so anything that sits inside a clickable row stops them
19+
* here: a click on the card must not open the row it hangs from.
20+
*/
21+
export default function PullRequestPanel({
22+
store,
23+
pullRequest,
24+
refreshFailed,
25+
onRetry,
26+
}: {
27+
store: Ariakit.HovercardStore;
28+
pullRequest: TConversationPullRequest;
29+
refreshFailed: boolean;
30+
onRetry: () => void;
31+
}) {
32+
const localize = useLocalize();
33+
const panelElement = Ariakit.useStoreState(store, 'contentElement');
34+
const stop = (event: { stopPropagation: () => void }) => event.stopPropagation();
35+
36+
return (
37+
<Ariakit.Hovercard
38+
store={store}
39+
gutter={8}
40+
portal
41+
unmountOnHide
42+
autoFocusOnShow={false}
43+
aria-label={localize('com_ui_pull_request')}
44+
className={panelClass}
45+
onClick={stop}
46+
onDoubleClick={stop}
47+
onContextMenu={stop}
48+
onKeyDown={stop}
49+
>
50+
<PullRequestCard
51+
pullRequest={pullRequest}
52+
refreshFailed={refreshFailed}
53+
onRetry={onRetry}
54+
portalElement={panelElement}
55+
/>
56+
</Ariakit.Hovercard>
57+
);
58+
}

0 commit comments

Comments
 (0)