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
This commit is contained in:
committed by
pullfrog[bot]
parent
f82f08dff6
commit
36cc5cde14
+9
-3
@@ -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<string> {
|
||||
return await installFromNpmTarball({
|
||||
@@ -311,7 +317,7 @@ async function runClaude(params: RunParams): Promise<AgentResult> {
|
||||
};
|
||||
|
||||
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<AgentResult> {
|
||||
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();
|
||||
|
||||
+18
-6
@@ -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<string> {
|
||||
return await installFromNpmTarball({
|
||||
@@ -255,7 +261,7 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
||||
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<string, number>();
|
||||
let currentStepId: string | null = null;
|
||||
@@ -263,11 +269,15 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
||||
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<AgentResult> {
|
||||
);
|
||||
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<AgentResult> {
|
||||
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<AgentResult> {
|
||||
};
|
||||
|
||||
const recentStderr: string[] = [];
|
||||
const MAX_STDERR_LINES = 20;
|
||||
|
||||
let lastProviderError: string | null = null;
|
||||
|
||||
let output = "";
|
||||
|
||||
@@ -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
|
||||
*/
|
||||
|
||||
@@ -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 <support@pullfrog.com>",
|
||||
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 = "";
|
||||
|
||||
+10
-1
@@ -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`);
|
||||
|
||||
+2
-2
@@ -157,13 +157,13 @@ type FetchPrDiffParams = {
|
||||
* this is the core diff formatting logic, extracted for testability.
|
||||
*/
|
||||
export async function fetchAndFormatPrDiff(params: FetchPrDiffParams): Promise<FormatFilesResult> {
|
||||
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";
|
||||
|
||||
+16
-2
@@ -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<string, ParsedHunk[]>();
|
||||
for (const file of prFilesResponse.data) {
|
||||
for (const file of prFiles) {
|
||||
if (file.patch) {
|
||||
filePatchMap.set(file.filename, parseFilePatches(file.patch));
|
||||
}
|
||||
|
||||
+6
-2
@@ -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 <support@pullfrog.com>",
|
||||
"license": "MIT",
|
||||
"bugs": {
|
||||
"url": "https://github.com/pullfrog/pullfrog/issues"
|
||||
|
||||
@@ -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 <support@pullfrog.com>",
|
||||
license: "MIT",
|
||||
bugs: {
|
||||
url: "https://github.com/pullfrog/pullfrog/issues"
|
||||
|
||||
Reference in New Issue
Block a user