Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed spurious infinite loads with explore panel, file tree, and file search command. [#617](https://github.com/sourcebot-dev/sourcebot/pull/617)
- Wipe search context on init if entitlement no longer exists [#618](https://github.com/sourcebot-dev/sourcebot/pull/618)
- Fixed Bitbucket repository exclusions not supporting glob patterns. [#620](https://github.com/sourcebot-dev/sourcebot/pull/620)
- Fixed review agent so that it works with GHES instances [#611](https://github.com/sourcebot-dev/sourcebot/pull/611)

## [4.9.2] - 2025-11-13

Expand Down
74 changes: 62 additions & 12 deletions packages/web/src/app/api/(server)/webhook/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,42 +6,88 @@ import { WebhookEventDefinition} from "@octokit/webhooks/types";
import { EndpointDefaults } from "@octokit/types";
import { env } from "@sourcebot/shared";
import { processGitHubPullRequest } from "@/features/agents/review-agent/app";
import { throttling } from "@octokit/plugin-throttling";
import { throttling, type ThrottlingOptions } from "@octokit/plugin-throttling";
import fs from "fs";
import { GitHubPullRequest } from "@/features/agents/review-agent/types";
import { createLogger } from "@sourcebot/shared";

const logger = createLogger('github-webhook');

let githubApp: App | undefined;
const DEFAULT_GITHUB_API_BASE_URL = "https://api.github.com";
type GitHubAppBaseOptions = Omit<ConstructorParameters<typeof App>[0], "Octokit"> & { throttle: ThrottlingOptions };

let githubAppBaseOptions: GitHubAppBaseOptions | undefined;
const githubAppCache = new Map<string, App>();

if (env.GITHUB_REVIEW_AGENT_APP_ID && env.GITHUB_REVIEW_AGENT_APP_WEBHOOK_SECRET && env.GITHUB_REVIEW_AGENT_APP_PRIVATE_KEY_PATH) {
try {
const privateKey = fs.readFileSync(env.GITHUB_REVIEW_AGENT_APP_PRIVATE_KEY_PATH, "utf8");

const throttledOctokit = Octokit.plugin(throttling);
githubApp = new App({
githubAppBaseOptions = {
appId: env.GITHUB_REVIEW_AGENT_APP_ID,
privateKey: privateKey,
privateKey,
webhooks: {
secret: env.GITHUB_REVIEW_AGENT_APP_WEBHOOK_SECRET,
},
Octokit: throttledOctokit,
throttle: {
onRateLimit: (retryAfter: number, options: Required<EndpointDefaults>, octokit: Octokit, retryCount: number) => {
enabled: true,
onRateLimit: (retryAfter, _options, _octokit, retryCount) => {
if (retryCount > 3) {
logger.warn(`Rate limit exceeded: ${retryAfter} seconds`);
return false;
}

return true;
},
}
});
onSecondaryRateLimit: (_retryAfter, options) => {
// no retries on secondary rate limits
logger.warn(`SecondaryRateLimit detected for ${options.method} ${options.url}`);
}
},
};
Comment on lines +26 to +47
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Throttle configuration is defined but not actually applied to Octokit

githubAppBaseOptions includes a throttle config, but getGithubAppForBaseUrl only passes baseUrl into Octokit.plugin(throttling).defaults(...). App’s constructor doesn’t accept a throttle option; throttle must be passed to the underlying Octokit constructor (e.g., via defaults({ throttle: ... })) for @octokit/plugin-throttling to use it.

Right now, you’re likely getting Octokit’s default throttling behavior instead of the custom retryCount > 3 logic and the secondary-rate-limit “no retry” behavior.

Consider wiring the throttle options into the Octokit defaults, e.g.:

-const getGithubAppForBaseUrl = (baseUrl: string) => {
-    if (!githubAppBaseOptions) {
-        return undefined;
-    }
-
-    const normalizedBaseUrl = normalizeGithubApiBaseUrl(baseUrl);
-    const cachedApp = githubAppCache.get(normalizedBaseUrl);
-    if (cachedApp) {
-        return cachedApp;
-    }
-
-    const OctokitWithBaseUrl = Octokit.plugin(throttling).defaults({ baseUrl: normalizedBaseUrl });
-    const app = new App({
-        ...githubAppBaseOptions,
-        Octokit: OctokitWithBaseUrl,
-    });
+const getGithubAppForBaseUrl = (baseUrl: string) => {
+    if (!githubAppBaseOptions) {
+        return undefined;
+    }
+
+    const normalizedBaseUrl = normalizeGithubApiBaseUrl(baseUrl);
+    const cachedApp = githubAppCache.get(normalizedBaseUrl);
+    if (cachedApp) {
+        return cachedApp;
+    }
+
+    const { throttle, ...appOptions } = githubAppBaseOptions;
+    const OctokitWithBaseUrl = Octokit
+        .plugin(throttling)
+        .defaults({
+            baseUrl: normalizedBaseUrl,
+            ...(throttle ? { throttle } : {}),
+        });
+
+    const app = new App({
+        ...appOptions,
+        Octokit: OctokitWithBaseUrl,
+    });

This keeps your per-base-URL behavior while ensuring the throttle callbacks are honored.

Also applies to: 70-89


I'll verify the review comment by inspecting the actual code and confirming the behavior of the Octokit throttling plugin.


---

🏁 Script executed:

```shell
#!/bin/bash
# View the webhook route file to inspect the getGithubAppForBaseUrl function
cat packages/web/src/app/api/\(server\)/webhook/route.ts | head -100

Length of output: 3944



---

🌐 Web query:

```web
@octokit/plugin-throttling how to pass throttle option Octokit constructor

💡 Result:

Use the throttle option when constructing the Octokit instance (after applying the plugin). Example:

JavaScript (Node)

  • import and apply plugin:
    const { Octokit } = require("@octokit/rest");
    const { throttling } = require("@octokit/plugin-throttling");
    const MyOctokit = Octokit.plugin(throttling);

  • pass throttle in the constructor:
    const octokit = new MyOctokit({
    auth: "token " + process.env.TOKEN,
    throttle: {
    onRateLimit: (retryAfter, options, octokit) => {
    octokit.log.warn(Rate limit for ${options.method} ${options.url});
    if (options.request.retryCount === 0) return true; // retry once
    },
    onSecondaryRateLimit: (retryAfter, options, octokit) => {
    octokit.log.warn(Secondary rate limit for ${options.method} ${options.url});
    },
    // or disable: enabled: false
    },
    });

Notes: the option key is throttle (not “throttling”), and onRateLimit / onSecondaryRateLimit (or onAbuseLimit on older versions) should return true to retry. [1][2]

Sources:
[1] Octokit docs — Throttling examples and constructor options.
[2] octokit/octokit.js README (throttle option description).


Throttle configuration is defined but not passed to the Octokit instance

At line 81, Octokit.plugin(throttling).defaults() only receives baseUrl, not the throttle config. The throttle callbacks defined in lines 31-47 are passed to the App constructor, which doesn't accept a throttle option—they're ignored.

Per the @octokit/plugin-throttling API, the throttle option must be passed directly to the Octokit instance via defaults() or the constructor. Without this, the custom retryCount > 3 logic and secondary rate-limit "no retry" behavior won't execute.

Update getGithubAppForBaseUrl to wire throttle into Octokit.defaults():

const OctokitWithBaseUrl = Octokit.plugin(throttling).defaults({ baseUrl: normalizedBaseUrl });
const app = new App({
    ...githubAppBaseOptions,
    Octokit: OctokitWithBaseUrl,
});

to:

const { throttle, ...appOptions } = githubAppBaseOptions;
const OctokitWithBaseUrl = Octokit.plugin(throttling).defaults({
    baseUrl: normalizedBaseUrl,
    ...(throttle ? { throttle } : {}),
});
const app = new App({
    ...appOptions,
    Octokit: OctokitWithBaseUrl,
});

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/web/src/app/api/(server)/webhook/route.ts around lines 26 to 47, the
throttle config is defined but passed to the App constructor (which ignores it)
instead of being provided to the Octokit instance; update getGithubAppForBaseUrl
so Octokit.plugin(throttling).defaults(...) receives both baseUrl and the
throttle object (or pass throttle into the Octokit constructor) and remove
throttle from the App constructor args so your onRateLimit/onSecondaryRateLimit
callbacks are honored by @octokit/plugin-throttling.

} catch (error) {
logger.error(`Error initializing GitHub app: ${error}`);
}
}

const normalizeGithubApiBaseUrl = (baseUrl?: string) => {
if (!baseUrl) {
return DEFAULT_GITHUB_API_BASE_URL;
}

return baseUrl.replace(/\/+$/, "");
};

const resolveGithubApiBaseUrl = (headers: Record<string, string>) => {
const enterpriseHost = headers["x-github-enterprise-host"];
if (enterpriseHost) {
return normalizeGithubApiBaseUrl(`https://${enterpriseHost}/api/v3`);
}

return DEFAULT_GITHUB_API_BASE_URL;
};

const getGithubAppForBaseUrl = (baseUrl: string) => {
if (!githubAppBaseOptions) {
return undefined;
}

const normalizedBaseUrl = normalizeGithubApiBaseUrl(baseUrl);
const cachedApp = githubAppCache.get(normalizedBaseUrl);
if (cachedApp) {
return cachedApp;
}

const OctokitWithBaseUrl = Octokit.plugin(throttling).defaults({ baseUrl: normalizedBaseUrl });
const app = new App({
...githubAppBaseOptions,
Octokit: OctokitWithBaseUrl,
});

githubAppCache.set(normalizedBaseUrl, app);
return app;
};

function isPullRequestEvent(eventHeader: string, payload: unknown): payload is WebhookEventDefinition<"pull-request-opened"> | WebhookEventDefinition<"pull-request-synchronize"> {
return eventHeader === "pull_request" && typeof payload === "object" && payload !== null && "action" in payload && typeof payload.action === "string" && (payload.action === "opened" || payload.action === "synchronize");
}
Expand All @@ -52,12 +98,16 @@ function isIssueCommentEvent(eventHeader: string, payload: unknown): payload is

export const POST = async (request: NextRequest) => {
const body = await request.json();
const headers = Object.fromEntries(request.headers.entries());
const headers = Object.fromEntries(Array.from(request.headers.entries(), ([key, value]) => [key.toLowerCase(), value]));

const githubEvent = headers['x-github-event'] || headers['X-GitHub-Event'];
const githubEvent = headers['x-github-event'];
if (githubEvent) {
logger.info('GitHub event received:', githubEvent);

const githubApiBaseUrl = resolveGithubApiBaseUrl(headers);
logger.debug('Using GitHub API base URL for event', { githubApiBaseUrl });
const githubApp = getGithubAppForBaseUrl(githubApiBaseUrl);

if (!githubApp) {
logger.warn('Received GitHub webhook event but GitHub app env vars are not set');
return Response.json({ status: 'ok' });
Expand Down Expand Up @@ -113,4 +163,4 @@ export const POST = async (request: NextRequest) => {
}

return Response.json({ status: 'ok' });
}
}
Loading