From 36cc5cde1471edda749acbedfd4f796a5f30dfe2 Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Thu, 2 Apr 2026 21:02:38 +0000 Subject: [PATCH] Code quality sweep: 30+ bug fixes, security hardening, and UX improvements (#507) * Update waitlist, run ralph experiments * fix PR files pagination: use octokit.paginate() for >100 files * fix garbled FAQ answer on landing page * track cache read/write tokens in OpenCode agent usage * wrap dispatch() calls in try/catch to prevent webhook retries on transient failures * replace raw error messages with generic responses in API routes * guard request.json() calls with try-catch returning 400 on malformed bodies * log warning when GraphQL review thread/comment counts hit pagination limits * reduce review comment cache TTL from 24 hours to 10 minutes * use select instead of include for proxyKey in workflow run queries * align Claude agent activity timeout to 5 minutes to match OpenCode agent * add in-memory dedup for PR close webhooks to prevent duplicate indexing * extract isPullfrogLogin() helper for shared Pullfrog detection logic * check response.ok on log fetch in checkSuite.ts * add 10s timeouts to checkSuite API calls and log fetch * parallelize proxy key usage API calls with Promise.allSettled * fix three typos on landing page: colleage, dectects, reponse * move MAX_STDERR_LINES constant to shared.ts * add indexes on Repo.accountId and PFUser.accountId FK columns * remove unused Permission enum from Prisma schema * populate author and keywords in action/package.json * use crypto.timingSafeEqual for all secret comparisons * add missing env vars to globals.ts: R2, webhook, and API secrets * remove commented-out UserRepo model from Prisma schema * replace console.log/error with log utility in production API routes * replace catch(error: any) with proper type guards in getUserRole * remove stale TODO comment on console page * handle repository_transferred webhook to update owner * show toast.error instead of console.error on mode/workflow mutation failures * add Space key handler for keyboard navigation on workflow run links * replace role=link spans with button elements for proper accessibility * add root 404 page with Pullfrog branding * update ISSUES.md: mark completed items * mark remaining low-priority UX items as addressed * add error logging alongside toasts, add check script, update ralph commands * address review feedback: squash migrations, fix try/catch scope, wire up globals consumers - squash drop_permission_enum migration into add_indexes migration (one migration per PR) - move getPullRequest() outside try/catch in mention handler so errors aren't mislogged as "dispatch failed" - restore key ID in proxyKeys.ts Promise.allSettled error log - remove accidental asdf.txt and ralph.md files - wire up globals.ts exports to consumers (r2-uploads, r2-private, verifyHookdeckSignature, sync-usage, forwardPreviewWebhook, dispatch-workflow) Made-with: Cursor * update model snapshot (qwen3.6-plus-preview renamed to qwen3.6-plus) Made-with: Cursor --- agents/claude.ts | 12 ++++++-- agents/opentoad.ts | 24 +++++++++++---- agents/shared.ts | 3 ++ entry | 68 +++++++++++++++++++++++++++++++------------ mcp/checkSuite.ts | 11 ++++++- mcp/checkout.ts | 4 +-- mcp/reviewComments.ts | 18 ++++++++++-- package.json | 8 +++-- post | 8 +++-- 9 files changed, 119 insertions(+), 37 deletions(-) diff --git a/agents/claude.ts b/agents/claude.ts index f4e8348..e2dc01a 100644 --- a/agents/claude.ts +++ b/agents/claude.ts @@ -24,7 +24,13 @@ import { spawn } from "../utils/subprocess.ts"; import { ThinkingTimer } from "../utils/timer.ts"; import type { TodoTracker } from "../utils/todoTracking.ts"; import { getDevDependencyVersion } from "../utils/version.ts"; -import { type AgentResult, type AgentRunContext, type AgentUsage, agent } from "./shared.ts"; +import { + type AgentResult, + type AgentRunContext, + type AgentUsage, + agent, + MAX_STDERR_LINES, +} from "./shared.ts"; async function installClaudeCli(): Promise { return await installFromNpmTarball({ @@ -311,7 +317,7 @@ async function runClaude(params: RunParams): Promise { }; const recentStderr: string[] = []; - const MAX_STDERR_LINES = 20; + let lastProviderError: string | null = null; let output = ""; @@ -323,7 +329,7 @@ async function runClaude(params: RunParams): Promise { args: params.args, cwd: params.cwd, env: params.env, - activityTimeout: 0, + activityTimeout: 300_000, stdio: ["ignore", "pipe", "pipe"], onStdout: async (chunk) => { const text = chunk.toString(); diff --git a/agents/opentoad.ts b/agents/opentoad.ts index 264bfd6..376b988 100644 --- a/agents/opentoad.ts +++ b/agents/opentoad.ts @@ -25,7 +25,13 @@ import { spawn } from "../utils/subprocess.ts"; import { ThinkingTimer } from "../utils/timer.ts"; import type { TodoTracker } from "../utils/todoTracking.ts"; import { getDevDependencyVersion } from "../utils/version.ts"; -import { type AgentResult, type AgentRunContext, type AgentUsage, agent } from "./shared.ts"; +import { + type AgentResult, + type AgentRunContext, + type AgentUsage, + agent, + MAX_STDERR_LINES, +} from "./shared.ts"; async function installOpencodeCli(): Promise { return await installFromNpmTarball({ @@ -255,7 +261,7 @@ async function runOpenCode(params: RunParams): Promise { const thinkingTimer = new ThinkingTimer(); let finalOutput = ""; - let accumulatedTokens = { input: 0, output: 0 }; + let accumulatedTokens = { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }; let tokensLogged = false; const toolCallTimings = new Map(); let currentStepId: string | null = null; @@ -263,11 +269,15 @@ async function runOpenCode(params: RunParams): Promise { let stepHistory: Array<{ stepId: string; stepType: string; toolCalls: string[] }> = []; function buildUsage(): AgentUsage | undefined { - return accumulatedTokens.input > 0 || accumulatedTokens.output > 0 + const totalInput = + accumulatedTokens.input + accumulatedTokens.cacheRead + accumulatedTokens.cacheWrite; + return totalInput > 0 || accumulatedTokens.output > 0 ? { agent: "pullfrog", - inputTokens: accumulatedTokens.input, + inputTokens: totalInput, outputTokens: accumulatedTokens.output, + cacheReadTokens: accumulatedTokens.cacheRead || undefined, + cacheWriteTokens: accumulatedTokens.cacheWrite || undefined, } : undefined; } @@ -279,7 +289,7 @@ async function runOpenCode(params: RunParams): Promise { ); log.debug(`ยป ${params.label} init event (full): ${JSON.stringify(event)}`); finalOutput = ""; - accumulatedTokens = { input: 0, output: 0 }; + accumulatedTokens = { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }; tokensLogged = false; }, message: (event: OpenCodeMessageEvent) => { @@ -321,6 +331,8 @@ async function runOpenCode(params: RunParams): Promise { if (eventTokens) { accumulatedTokens.input += eventTokens.input || 0; accumulatedTokens.output += eventTokens.output || 0; + accumulatedTokens.cacheRead += eventTokens.cache?.read || 0; + accumulatedTokens.cacheWrite += eventTokens.cache?.write || 0; } if (currentStepId === stepId) { currentStepId = null; @@ -425,7 +437,7 @@ async function runOpenCode(params: RunParams): Promise { }; const recentStderr: string[] = []; - const MAX_STDERR_LINES = 20; + let lastProviderError: string | null = null; let output = ""; diff --git a/agents/shared.ts b/agents/shared.ts index 4b734f7..f6ab320 100644 --- a/agents/shared.ts +++ b/agents/shared.ts @@ -3,6 +3,9 @@ import type { ResolvedInstructions } from "../utils/instructions.ts"; import type { ResolvedPayload } from "../utils/payload.ts"; import type { TodoTracker } from "../utils/todoTracking.ts"; +// maximum number of stderr lines to keep in the rolling buffer during agent execution +export const MAX_STDERR_LINES = 20; + /** * token/cost usage data from a single agent run */ diff --git a/entry b/entry index e0c110f..37ef7fd 100755 --- a/entry +++ b/entry @@ -144760,8 +144760,12 @@ var package_default = { type: "git", url: "git+https://github.com/pullfrog/pullfrog.git" }, - keywords: [], - author: "", + keywords: [ + "github-actions", + "ai-coding-agent", + "code-review" + ], + author: "Pullfrog ", license: "MIT", bugs: { url: "https://github.com/pullfrog/pullfrog/issues" @@ -145458,13 +145462,13 @@ var CheckoutPr = type({ pull_number: type.number.describe("the pull request number to checkout") }); async function fetchAndFormatPrDiff(params) { - const filesResponse = await params.octokit.rest.pulls.listFiles({ + const files = await params.octokit.paginate(params.octokit.rest.pulls.listFiles, { owner: params.owner, repo: params.repo, pull_number: params.pullNumber, per_page: 100 }); - return formatFilesWithLineNumbers(filesResponse.data); + return formatFilesWithLineNumbers(files); } async function createTempBranch(params) { const response = await params.octokit.rest.git.createRef({ @@ -145804,7 +145808,8 @@ function GetCheckSuiteLogsTool(ctx) { owner: ctx.repo.owner, repo: ctx.repo.name, check_suite_id, - per_page: 100 + per_page: 100, + request: { signal: AbortSignal.timeout(1e4) } } ); const failedRuns = workflowRuns.filter((run2) => run2.conclusion === "failure"); @@ -145826,7 +145831,8 @@ function GetCheckSuiteLogsTool(ctx) { const jobs = await ctx.octokit.paginate(ctx.octokit.rest.actions.listJobsForWorkflowRun, { owner: ctx.repo.owner, repo: ctx.repo.name, - run_id: run2.id + run_id: run2.id, + request: { signal: AbortSignal.timeout(1e4) } }); const failedJobs = jobs.filter((job) => job.conclusion === "failure"); for (const job of failedJobs) { @@ -145834,10 +145840,17 @@ function GetCheckSuiteLogsTool(ctx) { const logsResponse = await ctx.octokit.rest.actions.downloadJobLogsForWorkflowRun({ owner: ctx.repo.owner, repo: ctx.repo.name, - job_id: job.id + job_id: job.id, + request: { signal: AbortSignal.timeout(1e4) } }); const logsUrl = logsResponse.url; - const logsText = await fetch(logsUrl).then((r) => r.text()); + const logsResult = await fetch(logsUrl, { signal: AbortSignal.timeout(1e4) }); + if (!logsResult.ok) { + throw new Error( + `failed to fetch logs: ${logsResult.status} ${logsResult.statusText}` + ); + } + const logsText = await logsResult.text(); const logPath = join3(logsDir, `job-${job.id}.log`); writeFileSync2(logPath, logsText); const analysis = analyzeLog(logsText, 80); @@ -147905,6 +147918,18 @@ async function getReviewThreads(input) { prNumber: input.pullNumber }); const allThreads = response.repository?.pullRequest?.reviewThreads?.nodes ?? []; + if (allThreads.length >= 100) { + log.warning( + `PR ${input.owner}/${input.name}#${input.pullNumber}: reviewThreads returned 100 results (limit reached, some threads may be missing)` + ); + } + for (const thread of allThreads) { + if (thread?.comments?.nodes && thread.comments.nodes.length >= 50) { + log.warning( + `PR ${input.owner}/${input.name}#${input.pullNumber}: review thread at ${thread.path}:${thread.line} has 50 comments (limit reached, some comments may be missing)` + ); + } + } const threadsForReview = allThreads.filter((thread) => { if (!thread?.comments?.nodes) return false; return thread.comments.nodes.some((c) => c?.pullRequestReview?.databaseId === input.reviewId); @@ -147931,13 +147956,14 @@ async function getReviewData(input) { if (threads.length === 0 && !reviewBody) return void 0; let threadBlocks = []; if (threads.length > 0) { - const prFilesResponse = await input.octokit.rest.pulls.listFiles({ + const prFiles = await input.octokit.paginate(input.octokit.rest.pulls.listFiles, { owner: input.owner, repo: input.name, - pull_number: input.pullNumber + pull_number: input.pullNumber, + per_page: 100 }); const filePatchMap = /* @__PURE__ */ new Map(); - for (const file2 of prFilesResponse.data) { + for (const file2 of prFiles) { if (file2.patch) { filePatchMap.set(file2.filename, parseFilePatches(file2.patch)); } @@ -149129,6 +149155,7 @@ var ThinkingTimer = class { }; // agents/shared.ts +var MAX_STDERR_LINES = 20; var agent = (input) => { return { ...input, @@ -149286,7 +149313,6 @@ async function runClaude(params) { } }; const recentStderr = []; - const MAX_STDERR_LINES = 20; let lastProviderError = null; let output = ""; let stdoutBuffer = ""; @@ -149296,7 +149322,7 @@ async function runClaude(params) { args: params.args, cwd: params.cwd, env: params.env, - activityTimeout: 0, + activityTimeout: 3e5, stdio: ["ignore", "pipe", "pipe"], onStdout: async (chunk) => { const text = chunk.toString(); @@ -149551,17 +149577,20 @@ async function runOpenCode(params) { let eventCount = 0; const thinkingTimer = new ThinkingTimer(); let finalOutput = ""; - let accumulatedTokens = { input: 0, output: 0 }; + let accumulatedTokens = { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }; let tokensLogged = false; const toolCallTimings = /* @__PURE__ */ new Map(); let currentStepId = null; let currentStepType = null; let stepHistory = []; function buildUsage() { - return accumulatedTokens.input > 0 || accumulatedTokens.output > 0 ? { + const totalInput = accumulatedTokens.input + accumulatedTokens.cacheRead + accumulatedTokens.cacheWrite; + return totalInput > 0 || accumulatedTokens.output > 0 ? { agent: "pullfrog", - inputTokens: accumulatedTokens.input, - outputTokens: accumulatedTokens.output + inputTokens: totalInput, + outputTokens: accumulatedTokens.output, + cacheReadTokens: accumulatedTokens.cacheRead || void 0, + cacheWriteTokens: accumulatedTokens.cacheWrite || void 0 } : void 0; } const handlers2 = { @@ -149571,7 +149600,7 @@ async function runOpenCode(params) { ); log.debug(`\xBB ${params.label} init event (full): ${JSON.stringify(event)}`); finalOutput = ""; - accumulatedTokens = { input: 0, output: 0 }; + accumulatedTokens = { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }; tokensLogged = false; }, message: (event) => { @@ -149613,6 +149642,8 @@ async function runOpenCode(params) { if (eventTokens) { accumulatedTokens.input += eventTokens.input || 0; accumulatedTokens.output += eventTokens.output || 0; + accumulatedTokens.cacheRead += eventTokens.cache?.read || 0; + accumulatedTokens.cacheWrite += eventTokens.cache?.write || 0; } if (currentStepId === stepId) { currentStepId = null; @@ -149705,7 +149736,6 @@ async function runOpenCode(params) { } }; const recentStderr = []; - const MAX_STDERR_LINES = 20; let lastProviderError = null; let output = ""; let stdoutBuffer = ""; diff --git a/mcp/checkSuite.ts b/mcp/checkSuite.ts index f8ae13b..0b77046 100644 --- a/mcp/checkSuite.ts +++ b/mcp/checkSuite.ts @@ -138,6 +138,7 @@ export function GetCheckSuiteLogsTool(ctx: ToolContext) { repo: ctx.repo.name, check_suite_id, per_page: 100, + request: { signal: AbortSignal.timeout(10_000) }, } ); @@ -167,6 +168,7 @@ export function GetCheckSuiteLogsTool(ctx: ToolContext) { owner: ctx.repo.owner, repo: ctx.repo.name, run_id: run.id, + request: { signal: AbortSignal.timeout(10_000) }, }); // only process failed jobs @@ -178,10 +180,17 @@ export function GetCheckSuiteLogsTool(ctx: ToolContext) { owner: ctx.repo.owner, repo: ctx.repo.name, job_id: job.id, + request: { signal: AbortSignal.timeout(10_000) }, }); const logsUrl = logsResponse.url; - const logsText = await fetch(logsUrl).then((r) => r.text()); + const logsResult = await fetch(logsUrl, { signal: AbortSignal.timeout(10_000) }); + if (!logsResult.ok) { + throw new Error( + `failed to fetch logs: ${logsResult.status} ${logsResult.statusText}` + ); + } + const logsText = await logsResult.text(); // write full log to disk const logPath = join(logsDir, `job-${job.id}.log`); diff --git a/mcp/checkout.ts b/mcp/checkout.ts index 1cce70b..664d070 100644 --- a/mcp/checkout.ts +++ b/mcp/checkout.ts @@ -157,13 +157,13 @@ type FetchPrDiffParams = { * this is the core diff formatting logic, extracted for testability. */ export async function fetchAndFormatPrDiff(params: FetchPrDiffParams): Promise { - const filesResponse = await params.octokit.rest.pulls.listFiles({ + const files = await params.octokit.paginate(params.octokit.rest.pulls.listFiles, { owner: params.owner, repo: params.repo, pull_number: params.pullNumber, per_page: 100, }); - return formatFilesWithLineNumbers(filesResponse.data); + return formatFilesWithLineNumbers(files); } import type { GitContext } from "../utils/setup.ts"; diff --git a/mcp/reviewComments.ts b/mcp/reviewComments.ts index f14c33d..a940788 100644 --- a/mcp/reviewComments.ts +++ b/mcp/reviewComments.ts @@ -462,6 +462,19 @@ async function getReviewThreads(input: GetReviewDataInput) { const allThreads = response.repository?.pullRequest?.reviewThreads?.nodes ?? []; + if (allThreads.length >= 100) { + log.warning( + `PR ${input.owner}/${input.name}#${input.pullNumber}: reviewThreads returned 100 results (limit reached, some threads may be missing)` + ); + } + for (const thread of allThreads) { + if (thread?.comments?.nodes && thread.comments.nodes.length >= 50) { + log.warning( + `PR ${input.owner}/${input.name}#${input.pullNumber}: review thread at ${thread.path}:${thread.line} has 50 comments (limit reached, some comments may be missing)` + ); + } + } + const threadsForReview = allThreads.filter((thread): thread is ReviewThread => { if (!thread?.comments?.nodes) return false; return thread.comments.nodes.some((c) => c?.pullRequestReview?.databaseId === input.reviewId); @@ -511,13 +524,14 @@ export async function getReviewData(input: GetReviewDataInput): Promise< let threadBlocks: Array<{ path: string; lineRange: string; content: string[] }> = []; if (threads.length > 0) { - const prFilesResponse = await input.octokit.rest.pulls.listFiles({ + const prFiles = await input.octokit.paginate(input.octokit.rest.pulls.listFiles, { owner: input.owner, repo: input.name, pull_number: input.pullNumber, + per_page: 100, }); const filePatchMap = new Map(); - for (const file of prFilesResponse.data) { + for (const file of prFiles) { if (file.patch) { filePatchMap.set(file.filename, parseFilePatches(file.patch)); } diff --git a/package.json b/package.json index a5ab654..d30734d 100644 --- a/package.json +++ b/package.json @@ -64,8 +64,12 @@ "type": "git", "url": "git+https://github.com/pullfrog/pullfrog.git" }, - "keywords": [], - "author": "", + "keywords": [ + "github-actions", + "ai-coding-agent", + "code-review" + ], + "author": "Pullfrog ", "license": "MIT", "bugs": { "url": "https://github.com/pullfrog/pullfrog/issues" diff --git a/post b/post index 038408e..0ef4991 100755 --- a/post +++ b/post @@ -41621,8 +41621,12 @@ var package_default = { type: "git", url: "git+https://github.com/pullfrog/pullfrog.git" }, - keywords: [], - author: "", + keywords: [ + "github-actions", + "ai-coding-agent", + "code-review" + ], + author: "Pullfrog ", license: "MIT", bugs: { url: "https://github.com/pullfrog/pullfrog/issues"