🔑 feat: Let Users Connect Their Own Account for Pull Request Lookups

api/server/experimental.js

@@ -641,6 +641,7 @@ if (cluster.isMaster) {
     app.use('/api/search', routes.search);
     app.use('/api/messages', routes.messages);
     app.use('/api/convos', routes.convos);
+    app.use('/api/pull-requests', preAuthTenantMiddleware, routes.pullRequests);
     app.use('/api/presets', routes.presets);
     app.use('/api/projects', routes.projects);
     app.use('/api/prompts', routes.prompts);

api/server/index.js

@@ -421,6 +421,7 @@ const startServer = async () => {
   app.use('/api/search', routes.search);
   app.use('/api/messages', routes.messages);
   app.use('/api/convos', routes.convos);
+  app.use('/api/pull-requests', preAuthTenantMiddleware, routes.pullRequests);
   app.use('/api/traces', routes.traces);
   app.use('/api/presets', routes.presets);
   app.use('/api/projects', routes.projects);

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

@@ -151,6 +151,7 @@ module.exports = {
     createProxyAwareFetch: jest.fn(() => jest.fn()),
     createPullRequestLookup: jest.fn(() => jest.fn()),
     createPullRequestLookupStore: jest.fn(() => undefined),
+    createPullRequestUserTokenFromEnv: jest.fn(() => undefined),
     createConversationPullRequestsHandler: jest.fn(() => jest.fn()),
     createConversationPullRequestHandler: jest.fn(
       () => (_req, res) => res.status(200).json({ pullRequest: null }),

api/server/routes/admin/users.js

@@ -40,6 +40,7 @@ const handlers = createAdminUsersHandlers({
   deleteConfig: db.deleteConfig,
   deleteAclEntries: db.deleteAclEntries,
   deletePasskeysByUser: db.deletePasskeysByUser,
+  deleteTokens: db.deleteTokens,
 });
 
 router.use(requireJwtAuth, requireAdminAccess);

api/server/routes/convos.js

@@ -26,6 +26,7 @@ const {
   createPullRequestLookupStore,
   keyvRedisClient,
   cacheConfig,
+  createPullRequestUserTokenFromEnv,
   createConversationPullRequestsHandler,
   backgroundTaskRegistry,
   createSubagentThreadViewHandler,
@@ -180,17 +181,24 @@ const pullRequestLookup = createPullRequestLookup({
   source: createPullRequestSource({ fetchFn: createProxyAwareFetch() }),
   store: createPullRequestLookupStore({ client: keyvRedisClient, config: cacheConfig }),
 });
+const pullRequestUserToken = createPullRequestUserTokenFromEnv({
+  store: db,
+  fetchFn: createProxyAwareFetch(),
+  env: process.env,
+});
 const conversationPullRequestHandler = createConversationPullRequestHandler({
   getConvoLaneGit: db.getConvoLaneGit,
   getAppConfig,
   lookup: pullRequestLookup,
   env: process.env,
+  userToken: pullRequestUserToken,
 });
 const conversationPullRequestsHandler = createConversationPullRequestsHandler({
   getConvosLaneGit: db.getConvosLaneGit,
   getAppConfig,
   lookup: pullRequestLookup,
   env: process.env,
+  userToken: pullRequestUserToken,
 });
 router.use(requireJwtAuth);
 

api/server/routes/index.js

@@ -29,6 +29,7 @@ const openapi = require('./openapi');
 const search = require('./search');
 const models = require('./models');
 const convos = require('./convos');
+const pullRequests = require('./pullRequests');
 const traces = require('./traces');
 const config = require('./config');
 const agents = require('./agents');
@@ -45,6 +46,7 @@ const rum = require('./rum');
 const insights = require('./insights');
 
 module.exports = {
+  pullRequests,
   insights,
   rum,
   mcp,

api/server/routes/pullRequests.js

@@ -0,0 +1,17 @@
+const { createProxyAwareFetch, createPullRequestConnectionRouter } = require('@librechat/api');
+const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
+const { markOAuthNavigation } = require('~/server/middleware/oauthNavigation');
+const { loginLimiter } = require('~/server/middleware/limiters');
+const { getAppConfig } = require('~/server/services/Config');
+const db = require('~/models');
+
+module.exports = createPullRequestConnectionRouter({
+  store: db,
+  fetchFn: createProxyAwareFetch(),
+  env: process.env,
+  getAppConfig,
+  getUserById: db.getUserById,
+  requireJwtAuth,
+  markNavigation: markOAuthNavigation,
+  callbackLimiter: loginLimiter,
+});

librechat.example.yaml

@@ -950,6 +950,19 @@ endpoints:
   #     # host: "github.example.com"
   #     # Environment variable reference only, never the token itself. Optional, see above.
   #     token: "${GITHUB_PULL_REQUEST_TOKEN}"
+  #     # Let each user connect their own account, so lookups use their token, rate limit and
+  #     # repository access instead of the server's (read from this file only). Register
+  #     # <DOMAIN_SERVER>/api/pull-requests/oauth/callback as the app's callback URL. On GitHub a
+  #     # GitHub App with read-only Pull requests, Checks and Contents permissions is recommended.
+  #     # Changing clientId, provider or host asks users to connect again; a changed scope only
+  #     # applies to new connections. Disconnecting removes every grant of the user.
+  #     # oauth:
+  #     #   clientId: "${GITHUB_PR_OAUTH_CLIENT_ID}"
+  #     #   clientSecret: "${GITHUB_PR_OAUTH_CLIENT_SECRET}"
+  #     #   scope: "repo" # only for a classic OAuth app; GitLab defaults to read_api
+  #     #   flowTimeoutMinutes: 10 # time to approve at the provider
+  #     #   requestTimeoutSeconds: 10 # per code exchange or token refresh
+  #     #   retentionDays: 180 # an unused connection is removed after this; each use extends it
   #     # Repositories the token may be used for, as owner/name or owner/*. A worker reports its
   #     # own repository, and only these are ever looked up with the server's token.
   #     allowedRepositories: ["LibreChat-AI/LibreChat"]

packages/api/src/admin/users.spec.ts

@@ -68,6 +68,7 @@ function createDeps(overrides: Partial<AdminUsersDeps> = {}): AdminUsersDeps {
     deleteConfig: jest.fn().mockResolvedValue(null),
     deleteAclEntries: jest.fn().mockResolvedValue(undefined),
     deletePasskeysByUser: jest.fn().mockResolvedValue(0),
+    deleteTokens: jest.fn().mockResolvedValue({ deletedCount: 0 }),
     ...overrides,
   };
 }
@@ -461,6 +462,34 @@ describe('createAdminUsersHandlers', () => {
       expect(deps.deletePasskeysByUser).toHaveBeenCalledWith(validUserId);
     });
 
+    it("removes the user's stored credentials before the user", async () => {
+      const deps = createDeps();
+      const handlers = createAdminUsersHandlers(deps);
+      const { req, res, status } = createReqRes({ params: { id: validUserId } });
+
+      await handlers.deleteUser(req, res);
+
+      expect(status).toHaveBeenCalledWith(200);
+      expect(deps.deleteTokens).toHaveBeenCalledWith({ userId: validUserId });
+      expect(jest.mocked(deps.deleteTokens).mock.invocationCallOrder[0]).toBeLessThan(
+        jest.mocked(deps.deleteUserById).mock.invocationCallOrder[0],
+      );
+    });
+
+    it('keeps the user, and releases the fence, when its credentials cannot be removed', async () => {
+      const deps = createDeps({
+        deleteTokens: jest.fn().mockRejectedValue(new Error('mongo down')),
+      });
+      const handlers = createAdminUsersHandlers(deps);
+      const { req, res, status } = createReqRes({ params: { id: validUserId } });
+
+      await handlers.deleteUser(req, res);
+
+      expect(status).toHaveBeenCalledWith(500);
+      expect(deps.deleteUserById).not.toHaveBeenCalled();
+      expect(deps.cancelAgentTriggerUserDeletion).toHaveBeenCalled();
+    });
+
     it('returns success even when cascade cleanup partially fails', async () => {
       const result: UserDeleteResult = {
         deletedCount: 1,

packages/api/src/admin/users.ts

@@ -62,6 +62,12 @@ export interface AdminUsersDeps {
    * keep authentication material and can collide with a later registration.
    */
   deletePasskeysByUser: (userId: string) => Promise<number>;
+  /**
+   * Stored credentials the user holds, such as a connected account's OAuth grant. Removed before
+   * the user is, as the self-delete flow does, so a failure leaves the account to retry rather
+   * than its credentials behind.
+   */
+  deleteTokens: (query: { userId: string }) => Promise<unknown>;
 }
 
 export function createAdminUsersHandlers(deps: AdminUsersDeps): {
@@ -85,6 +91,7 @@ export function createAdminUsersHandlers(deps: AdminUsersDeps): {
     deleteConfig,
     deleteAclEntries,
     deletePasskeysByUser,
+    deleteTokens,
   } = deps;
 
   async function listUsersHandler(req: ServerRequest, res: Response) {
@@ -202,6 +209,7 @@ export function createAdminUsersHandlers(deps: AdminUsersDeps): {
       await prepareAgentTriggerUserPurge(id, triggerDeletionFence, targetUser?.tenantId);
       await drainAgentTriggerDeliveriesForUser(id);
 
+      await deleteTokens({ userId: id });
       const result = await deleteUserById(id);
 
       if (result.deletedCount === 0) {

packages/api/src/oauth/csrf.ts

@@ -148,13 +148,19 @@ function getOAuthCookiePath(cookiePath: string): string {
   }
 }
 
-/** Sets a SameSite=Lax CSRF cookie bound to a specific OAuth flow */
-export function setOAuthCsrfCookie(res: Response, flowId: string, cookiePath: string): void {
+/** Sets a SameSite=Lax CSRF cookie bound to a specific OAuth flow. `maxAge` follows the flow's own
+ *  approval window when it has one, so the cookie lives as long as the state it binds. */
+export function setOAuthCsrfCookie(
+  res: Response,
+  flowId: string,
+  cookiePath: string,
+  maxAge: number = OAUTH_CSRF_MAX_AGE,
+): void {
   res.cookie(OAUTH_CSRF_COOKIE, generateOAuthCsrfToken(flowId), {
     httpOnly: true,
     secure: shouldUseSecureCookie(),
     sameSite: 'lax',
-    maxAge: OAUTH_CSRF_MAX_AGE,
+    maxAge,
     path: getOAuthCookiePath(cookiePath),
   });
 }

packages/api/src/pulls/bitbucket.spec.ts

@@ -222,6 +222,11 @@ describe('createBitbucketPullRequestSource', () => {
     expect(urlsOf(fetchFn)[0]).not.toContain('"');
   });
 
+  it('maps a 401 to CREDENTIAL_REJECTED, a refusal of the token itself', async () => {
+    const { source } = sourceFor({ 'state=OPEN': () => new Response('', { status: 401 }) });
+    await expect(find(source)).rejects.toMatchObject({ code: 'CREDENTIAL_REJECTED' });
+  });
+
   it.each([
     ['another host', 'https://evil.example/w/r/pull-requests/7'],
     ['plain http', 'http://bitbucket.org/w/r/pull-requests/7'],

packages/api/src/pulls/bitbucket.ts

@@ -221,6 +221,8 @@ export function createBitbucketPullRequestSource({
     }
     /** A repository the token cannot see is indistinguishable from one without pull requests. */
     if (response.status === 404 || absent.includes(response.status)) return null;
+    /** The provider no longer accepts the token: revoked, or expired without a refresh. */
+    if (response.status === 401) throw new PullRequestSourceError('CREDENTIAL_REJECTED');
     throw new PullRequestSourceError('UPSTREAM_ERROR');
   }
 

packages/api/src/pulls/capabilities.spec.ts

@@ -17,6 +17,19 @@ describe('resolvePullRequestCapabilities', () => {
     });
   });
 
+  it('advertises the provider users can connect to, only when an app is configured', () => {
+    const oauth = { clientId: '${ID}', clientSecret: '${SECRET}' };
+    expect(
+      resolvePullRequestCapabilities(configWith({ provider: 'gitlab', oauth }), {
+        ID: 'i',
+        SECRET: 's',
+      }),
+    ).toMatchObject({ pullRequestsEnabled: true, pullRequestsConnect: 'gitlab' });
+    expect(resolvePullRequestCapabilities(configWith(scoped), env)).not.toHaveProperty(
+      'pullRequestsConnect',
+    );
+  });
+
   it('advertises the configured lookup limit', () => {
     expect(
       resolvePullRequestCapabilities(configWith({ ...scoped, maxConcurrentLookups: 2 }), env),

packages/api/src/pulls/capabilities.ts

@@ -1,7 +1,7 @@
 import { EModelEndpoint, PULL_REQUEST_BATCH_VERSION } from 'librechat-data-provider';
-import type { TAgentsEndpoint } from 'librechat-data-provider';
+import type { PullRequestProvider, TAgentsEndpoint } from 'librechat-data-provider';
 import type { AppConfig } from '@librechat/data-schemas';
-import { isPullRequestFeatureActive } from './settings';
+import { isPullRequestFeatureActive, resolvePullRequestOAuth } from './settings';
 
 export type PullRequestCapabilities = {
   pullRequestsEnabled: boolean;
@@ -10,6 +10,8 @@ export type PullRequestCapabilities = {
   /** The lookups one batch runs at once, so a client that falls back to the single route can
    *  keep to the same limit while a replica without the batch route is still serving it. */
   pullRequestsMaxConcurrentLookups?: number;
+  /** The provider users can connect their own account to, when the operator configured one. */
+  pullRequestsConnect?: PullRequestProvider;
 };
 
 /**
@@ -23,12 +25,16 @@ export function resolvePullRequestCapabilities(
 ): PullRequestCapabilities {
   const agents = appConfig?.endpoints?.[EModelEndpoint.agents] as TAgentsEndpoint | undefined;
   const enabled = isPullRequestFeatureActive(agents?.pullRequests, env);
+  const connect = enabled
+    ? resolvePullRequestOAuth(agents?.pullRequests, env)?.provider
+    : undefined;
   return {
     pullRequestsEnabled: enabled,
     ...(enabled
       ? {
           pullRequestsBatchVersion: PULL_REQUEST_BATCH_VERSION,
           pullRequestsMaxConcurrentLookups: agents?.pullRequests?.maxConcurrentLookups ?? 4,
+          ...(connect != null ? { pullRequestsConnect: connect } : {}),
         }
       : {}),
   };