Agent & model refactor (#478)
* agent & model refactor with ASKPASS git auth, UI restructure, clerk v7 Made-with: Cursor * fix stale agent/effort refs, add tests for askpass + model resolution - reviewCleanup.ts: payload.agent -> payload.model, remove effort - selectMode.ts PlanEdit: remove delegation/subagent/effort references - pullfrog.yml.ts: update env vars (drop GOOGLE_API_KEY/CURSOR_API_KEY, add GOOGLE_GENERATIVE_AI_API_KEY/XAI_API_KEY/MOONSHOT_API_KEY/OPENCODE_API_KEY) - FlagsSettings/RepoInstructionsSection: remove stale effort/timeout copy - new: gitAuthServer.test.ts (10 tests — lifecycle, token delivery, tamper detection, script gen) - new: agent.test.ts (4 tests — default opentoad, AGENT_OVERRIDE, invalid override) - new: models.test.ts (19 tests — parseModel, resolution, registry invariants) - update models.dev snapshot Made-with: Cursor * fix changed-agents.sh to filter legacy agent files from CI matrix legacy agent files (claude.ts, codex.ts, etc.) are @ts-nocheck and not exported from index.ts. changed-agents.sh now reads index.ts imports to build the active agent set and treats changes to inactive files as non-agent changes (opentoad canary only). Made-with: Cursor * remove MCP file tools, old agent harnesses, and obsolete security tests ASKPASS-based git auth makes the old MCP file tool security layer unnecessary: - token never in subprocess env, so symlink/gitattributes/hook attacks can't exfiltrate it - agents now use native file tools (OpenCode builtin read/edit) deleted: - action/mcp/file.ts (file_read, file_write, file_edit, file_delete, list_directory) - action/mcp/index.ts (dead re-export) - agent harnesses: claude.ts, codex.ts, cursor.ts, gemini.ts, opencode.ts - opencode-runner.ts (inlined into opentoad.ts) - security tests that validated MCP file tool restrictions - commented-out three-step review flow (~300 lines) - sanitizeSchema/wrapSchema dead code from mcp/shared.ts - OPENCODE_MODEL_MINI/MAX env vars (effort-level model overrides removed) updated test prompts to use generic file ops instead of MCP tool names. restored pkg-json-scripts + requirements-txt-attack (test --ignore-scripts defense). Made-with: Cursor * bump actions/checkout v4 → v6 (node 24) node 20 actions deprecated june 2, 2026. Made-with: Cursor * temporarily disable fail-fast on agnostic tests to debug checkout@v6 Made-with: Cursor * re-enable fail-fast on agnostic tests Made-with: Cursor * fix test token mismatch: mint OIDC tokens scoped to target repo CI tests override GITHUB_REPOSITORY to pullfrog/test-repo but inherit the runner's GITHUB_TOKEN (scoped to pullfrog/app), causing 401s on every run-context fetch. Clear GITHUB_TOKEN in the test subprocess so ensureGitHubToken() mints a properly scoped token via OIDC. Also centralizes the default GITHUB_REPOSITORY in runAgentStreaming instead of repeating it in every test file, and fixes preview-cleanup to remove workers from all queues (not just name-matching ones). Made-with: Cursor * fix ensureGitHubToken to try OIDC when app credentials are absent ensureGitHubToken only attempted token minting when GITHUB_APP_ID and GITHUB_PRIVATE_KEY were set. In CI, OIDC is available but app creds aren't exposed — so the guard prevented minting entirely. Made-with: Cursor * dead code cleanup: remove remnants of deleted agents, file tools, effort system remove unused @anthropic-ai/claude-agent-sdk and @openai/codex-sdk deps, orphaned file-tool security tests, dead GEMINI_MODEL passthrough, stale opencode-runner wiki refs, deleted test file references, and MCP file tool docs. rename docs/effort → docs/models. fix vitest setup: move dotenv to globalSetup (runs once before forks instead of per-file, 19s → 200ms). Made-with: Cursor * address review feedback: remove dead code, update stale references - remove AGENT_OVERRIDE (only opentoad exists) - remove shellToolName plumbing (always restricted shell) - bump action version to 0.0.179 - remove CURSOR_API_KEY from all workflows/configs - remove OPENCODE_MODEL_MINI/MAX from workflows/docs - delete wiki/effort.md, rewrite docs/effort.mdx as "Models" - rewrite wiki/modes.md: orchestrator/subagent → single agent - simplify flag system: drop builtin flag extraction (debug, effort, timeout, agent), keep custom flag replacement only - reserve all legacy flag names to prevent custom flag conflicts Made-with: Cursor * regenerate lockfile after removing claude-agent-sdk and codex-sdk Made-with: Cursor * fix import ordering, add lockfile check to pre-push hook Made-with: Cursor * remove dead debug payload field, stale packageExtensions Made-with: Cursor * merge proc-sandbox and token-exfil into a single test proc-sandbox and token-exfil were duplicative — both tested that SANDBOX_TEST_TOKEN couldn't be exfiltrated. consolidated into token-exfil with shell:restricted (which actually exercises filterEnv) and the /proc attack vector hints from proc-sandbox. Made-with: Cursor * fix wiki adversarial.md to match actual tokenExfil validator Made-with: Cursor
This commit is contained in:
committed by
pullfrog[bot]
parent
5bcfae990a
commit
6d25adfd1a
+32
-307
@@ -4,9 +4,16 @@ import { apiFetch } from "../utils/apiFetch.ts";
|
||||
import { getApiUrl } from "../utils/apiUrl.ts";
|
||||
import { buildPullfrogFooter } from "../utils/buildPullfrogFooter.ts";
|
||||
import { log } from "../utils/cli.ts";
|
||||
import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts";
|
||||
import type { ToolContext } from "./server.ts";
|
||||
import { execute, tool } from "./shared.ts";
|
||||
|
||||
function isStatusError(err: unknown): err is { status: number; message?: string } {
|
||||
return (
|
||||
typeof err === "object" && err !== null && "status" in err && typeof err.status === "number"
|
||||
);
|
||||
}
|
||||
|
||||
// one-shot review tool
|
||||
export const CreatePullRequestReview = type({
|
||||
pull_number: type.number.describe("The pull request number to review"),
|
||||
@@ -70,6 +77,8 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
" Put feedback about code outside the diff in 'body' instead.",
|
||||
parameters: CreatePullRequestReview,
|
||||
execute: execute(async ({ pull_number, body, approved, commit_id, comments = [] }) => {
|
||||
if (body) body = fixDoubleEscapedString(body);
|
||||
|
||||
// set issue context (PRs are issues)
|
||||
ctx.toolState.issueNumber = pull_number;
|
||||
|
||||
@@ -99,7 +108,7 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
if (comments.length > 0) {
|
||||
type ReviewComment = (typeof params.comments & {})[number];
|
||||
params.comments = comments.map((comment) => {
|
||||
let commentBody = comment.body || "";
|
||||
let commentBody = fixDoubleEscapedString(comment.body || "");
|
||||
if (comment.suggestion !== undefined) {
|
||||
const suggestionBlock = "```suggestion\n" + comment.suggestion + "\n```";
|
||||
commentBody = commentBody ? commentBody + "\n\n" + suggestionBlock : suggestionBlock;
|
||||
@@ -120,14 +129,28 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
|
||||
// no body → single-step createReview (no footer needed)
|
||||
// has body → pending + submit so we can build footer with Fix links using review ID
|
||||
const result = body
|
||||
? await createAndSubmitWithFooter(ctx, params, {
|
||||
body,
|
||||
approved: approved ?? false,
|
||||
hasComments: comments.length > 0,
|
||||
})
|
||||
: await ctx.octokit.rest.pulls.createReview(params);
|
||||
|
||||
let result;
|
||||
try {
|
||||
result = body
|
||||
? await createAndSubmitWithFooter(ctx, params, {
|
||||
body,
|
||||
approved: approved ?? false,
|
||||
hasComments: comments.length > 0,
|
||||
})
|
||||
: await ctx.octokit.rest.pulls.createReview(params);
|
||||
} catch (err: unknown) {
|
||||
if (isStatusError(err) && err.status === 422 && params.comments?.length) {
|
||||
const paths = [...new Set(params.comments.map((comment) => comment.path))];
|
||||
throw new Error(
|
||||
`${err.message ?? "422 Unprocessable Entity"}. ` +
|
||||
`The review had ${params.comments.length} inline comment(s) targeting these paths: ${paths.join(", ")}. ` +
|
||||
`GitHub cannot resolve one or more of these paths in the PR diff (common when the PR has >100 changed files and some are truncated). ` +
|
||||
`Fix: remove the failing comment(s) and retry. Put their feedback in the review body instead.`
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
log.debug(`createReview response: ${JSON.stringify(result.data)}`);
|
||||
if (!result.data.id) {
|
||||
throw new Error(`createReview returned invalid data: ${JSON.stringify(result.data)}`);
|
||||
}
|
||||
@@ -266,301 +289,3 @@ export async function reportReviewNodeId(ctx: ToolContext, reviewNodeId: string)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// COMMENTED OUT: Three-step review flow (start_review, add_review_comment, submit_review)
|
||||
// This approach used GraphQL to add comments to a pending review one-by-one,
|
||||
// but GitHub's API was returning null for valid lines. Keeping for reference.
|
||||
// =============================================================================
|
||||
|
||||
/*
|
||||
// graphql mutation to add a comment thread to a pending review
|
||||
// note: REST API doesn't support adding comments to an existing pending review
|
||||
const ADD_PULL_REQUEST_REVIEW_THREAD = `
|
||||
mutation AddPullRequestReviewThread($pullRequestReviewId: ID!, $path: String!, $line: Int!, $body: String!, $side: DiffSide, $subjectType: PullRequestReviewThreadSubjectType) {
|
||||
addPullRequestReviewThread(input: {
|
||||
pullRequestReviewId: $pullRequestReviewId,
|
||||
path: $path,
|
||||
line: $line,
|
||||
body: $body,
|
||||
side: $side,
|
||||
subjectType: $subjectType
|
||||
}) {
|
||||
thread {
|
||||
id
|
||||
}
|
||||
}
|
||||
}
|
||||
`;
|
||||
|
||||
type AddPullRequestReviewThreadResponse = {
|
||||
addPullRequestReviewThread: {
|
||||
thread: {
|
||||
id: string;
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
// helper to find existing pending review for the authenticated user
|
||||
async function findPendingReview(
|
||||
ctx: ToolContext,
|
||||
pull_number: number
|
||||
): Promise<{ id: number; node_id: string } | null> {
|
||||
const reviews = await ctx.octokit.rest.pulls.listReviews({
|
||||
owner: ctx.repo.owner,
|
||||
repo: ctx.repo.name,
|
||||
pull_number,
|
||||
per_page: 100,
|
||||
});
|
||||
|
||||
// find a PENDING review from our bot
|
||||
// note: authenticated user is the GitHub App, reviews show as "pullfrog[bot]"
|
||||
const pendingReview = reviews.data.find((r) => r.state === "PENDING");
|
||||
if (pendingReview) {
|
||||
return { id: pendingReview.id, node_id: pendingReview.node_id };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// start_review tool
|
||||
export const StartReview = type({
|
||||
pull_number: type.number.describe("The pull request number to review"),
|
||||
});
|
||||
|
||||
export function StartReviewTool(ctx: ToolContext) {
|
||||
return tool({
|
||||
name: "start_review",
|
||||
description:
|
||||
"Start a new review session for a pull request. Creates a pending review on GitHub. Must be called before add_review_comment.",
|
||||
parameters: StartReview,
|
||||
execute: execute(async ({ pull_number }) => {
|
||||
// check if review already started in this session
|
||||
if (ctx.toolState.review) {
|
||||
throw new Error(
|
||||
`Review session already in progress. Call submit_review first to finish it.`
|
||||
);
|
||||
}
|
||||
|
||||
// get the PR to get head commit SHA
|
||||
const pr = await ctx.octokit.rest.pulls.get({
|
||||
owner: ctx.repo.owner,
|
||||
repo: ctx.repo.name,
|
||||
pull_number,
|
||||
});
|
||||
|
||||
let reviewId: number;
|
||||
let reviewNodeId: string;
|
||||
|
||||
// try to create a new pending review (omitting 'event' creates PENDING state)
|
||||
log.debug(`creating pending review for PR #${pull_number}...`);
|
||||
try {
|
||||
const result = await ctx.octokit.rest.pulls.createReview({
|
||||
owner: ctx.repo.owner,
|
||||
repo: ctx.repo.name,
|
||||
pull_number,
|
||||
commit_id: pr.data.head.sha,
|
||||
// no 'event' = PENDING review
|
||||
});
|
||||
log.debug(`createReview response: ${JSON.stringify(result.data)}`);
|
||||
if (!result.data.id || !result.data.node_id) {
|
||||
log.debug(result);
|
||||
throw new Error(
|
||||
`createReview returned invalid data: id=${result.data.id}, node_id=${result.data.node_id}`
|
||||
);
|
||||
}
|
||||
reviewId = result.data.id;
|
||||
reviewNodeId = result.data.node_id;
|
||||
log.debug(`created new pending review: id=${reviewId}`);
|
||||
} catch (error) {
|
||||
// check for "already has pending review" error
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
log.debug(`createReview failed: ${errorMessage}`);
|
||||
if (errorMessage.includes("pending review")) {
|
||||
// find the existing pending review
|
||||
log.debug(`pending review already exists, fetching existing review...`);
|
||||
const existing = await findPendingReview(ctx, pull_number);
|
||||
if (!existing) {
|
||||
throw new Error(
|
||||
"GitHub says a pending review exists but we couldn't find it. Try again or check the PR reviews."
|
||||
);
|
||||
}
|
||||
reviewId = existing.id;
|
||||
reviewNodeId = existing.node_id;
|
||||
log.debug(`reusing existing pending review: id=${reviewId}`);
|
||||
} else {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
// set issue context (PRs are issues) and review state
|
||||
ctx.toolState.issueNumber = pull_number;
|
||||
ctx.toolState.review = {
|
||||
nodeId: reviewNodeId,
|
||||
id: reviewId,
|
||||
};
|
||||
|
||||
log.debug(`review session started: id=${reviewId}, nodeId=${reviewNodeId}`);
|
||||
|
||||
return {
|
||||
message: `Review session started for PR #${pull_number}. Add comments with add_review_comment, then submit with submit_review.`,
|
||||
};
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
// add_review_comment tool
|
||||
export const AddReviewComment = type({
|
||||
path: type.string.describe("The file path to comment on (relative to repo root)"),
|
||||
line: type.number.describe(
|
||||
"The line number in the file (use line numbers from the diff - the NEW file line number)"
|
||||
),
|
||||
body: type.string.describe("The comment text for this specific line"),
|
||||
side: type
|
||||
.enumerated("LEFT", "RIGHT")
|
||||
.describe("Side of the diff: LEFT (old code) or RIGHT (new code). Defaults to RIGHT.")
|
||||
.optional(),
|
||||
});
|
||||
|
||||
export function AddReviewCommentTool(ctx: ToolContext) {
|
||||
return tool({
|
||||
name: "add_review_comment",
|
||||
description:
|
||||
"Add a comment to the current review session. Must call start_review first. Comments are stored in draft state until submit_review is called.",
|
||||
parameters: AddReviewComment,
|
||||
execute: execute(async ({ path, line, body, side }) => {
|
||||
// check if review started
|
||||
if (!ctx.toolState.review) {
|
||||
throw new Error("No review session started. Call start_review first.");
|
||||
}
|
||||
|
||||
const reviewNodeId = ctx.toolState.review.nodeId;
|
||||
log.debug(
|
||||
`adding review comment: reviewNodeId=${reviewNodeId}, path=${path}, line=${line}, side=${side || "RIGHT"}`
|
||||
);
|
||||
|
||||
// add comment thread via GraphQL (REST doesn't support adding to existing pending review)
|
||||
let result: AddPullRequestReviewThreadResponse;
|
||||
try {
|
||||
result = await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>(
|
||||
ADD_PULL_REQUEST_REVIEW_THREAD,
|
||||
{
|
||||
pullRequestReviewId: reviewNodeId,
|
||||
path,
|
||||
line,
|
||||
body,
|
||||
side: side || "RIGHT",
|
||||
subjectType: "LINE",
|
||||
}
|
||||
);
|
||||
log.debug(`addPullRequestReviewThread response: ${JSON.stringify(result)}`);
|
||||
} catch (error) {
|
||||
const errorMsg = error instanceof Error ? error.message : String(error);
|
||||
log.debug(`addPullRequestReviewThread error: ${errorMsg}`);
|
||||
throw new Error(
|
||||
`Failed to add comment to ${path}:${line}. GraphQL error: ${errorMsg}. ` +
|
||||
`Ensure the line is part of the diff and the path is correct.`
|
||||
);
|
||||
}
|
||||
|
||||
// check if the mutation succeeded - null means the line is not in the diff
|
||||
if (!result) {
|
||||
throw new Error(
|
||||
`Failed to add comment to ${path}:${line}. GraphQL returned null response.`
|
||||
);
|
||||
}
|
||||
if (!result.addPullRequestReviewThread) {
|
||||
throw new Error(
|
||||
`Failed to add comment to ${path}:${line}. addPullRequestReviewThread is null. Response: ${JSON.stringify(result)}`
|
||||
);
|
||||
}
|
||||
if (!result.addPullRequestReviewThread.thread) {
|
||||
throw new Error(
|
||||
`Failed to add comment to ${path}:${line}. thread is null. The line must be part of the diff. Response: ${JSON.stringify(result)}`
|
||||
);
|
||||
}
|
||||
|
||||
const threadId = result.addPullRequestReviewThread.thread.id;
|
||||
log.debug(`review comment added: threadId=${threadId}`);
|
||||
|
||||
return {
|
||||
success: true,
|
||||
message: `Comment added to ${path}:${line}`,
|
||||
threadId,
|
||||
};
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
// submit_review tool
|
||||
export const SubmitReview = type({
|
||||
body: type.string
|
||||
.describe(
|
||||
"Review body text. Typically 1-3 sentences with high-level overview and urgency level. Action links are auto-appended."
|
||||
)
|
||||
.optional(),
|
||||
});
|
||||
|
||||
export function SubmitReviewTool(ctx: ToolContext) {
|
||||
return tool({
|
||||
name: "submit_review",
|
||||
description:
|
||||
"Submit the current review session. All comments added via add_review_comment will be published. Must call start_review first.",
|
||||
parameters: SubmitReview,
|
||||
execute: execute(async ({ body }) => {
|
||||
// check if review started
|
||||
if (!ctx.toolState.review) {
|
||||
throw new Error("No review session started. Call start_review first.");
|
||||
}
|
||||
if (ctx.toolState.issueNumber === undefined) {
|
||||
throw new Error("No PR context. Call checkout_pr or start_review first.");
|
||||
}
|
||||
|
||||
const reviewId = ctx.toolState.review.id;
|
||||
log.debug(
|
||||
`submitting review: id=${reviewId}, nodeId=${ctx.toolState.review.nodeId}, issueNumber=${ctx.toolState.issueNumber}`
|
||||
);
|
||||
|
||||
// build quick links footer
|
||||
const apiUrl = getApiUrl();
|
||||
const fixAllUrl = `${apiUrl}/trigger/${ctx.repo.owner}/${ctx.repo.name}/${ctx.toolState.issueNumber}?action=fix&review_id=${reviewId}`;
|
||||
const fixApprovedUrl = `${apiUrl}/trigger/${ctx.repo.owner}/${ctx.repo.name}/${ctx.toolState.issueNumber}?action=fix-approved&review_id=${reviewId}`;
|
||||
|
||||
const footer = buildPullfrogFooter({
|
||||
workflowRun: { owner: ctx.repo.owner, repo: ctx.repo.name, runId: ctx.runId, jobId: ctx.jobId },
|
||||
customParts: [`[Fix all ➔](${fixAllUrl})`, `[Fix 👍s ➔](${fixApprovedUrl})`],
|
||||
});
|
||||
|
||||
const bodyWithFooter = (body || "") + footer;
|
||||
|
||||
// submit the pending review via REST
|
||||
const result = await ctx.octokit.rest.pulls.submitReview({
|
||||
owner: ctx.repo.owner,
|
||||
repo: ctx.repo.name,
|
||||
pull_number: ctx.toolState.issueNumber,
|
||||
review_id: reviewId,
|
||||
event: "COMMENT",
|
||||
body: bodyWithFooter,
|
||||
});
|
||||
|
||||
log.debug(`submitReview response: ${JSON.stringify(result.data)}`);
|
||||
if (!result.data.id) {
|
||||
throw new Error(`submitReview returned invalid data: ${JSON.stringify(result.data)}`);
|
||||
}
|
||||
log.debug(`review submitted: reviewId=${result.data.id}, state=${result.data.state}`);
|
||||
|
||||
// clear review state
|
||||
delete ctx.toolState.review;
|
||||
|
||||
// delete progress comment
|
||||
await deleteProgressComment(ctx);
|
||||
|
||||
return {
|
||||
success: true,
|
||||
reviewId: result.data.id,
|
||||
html_url: result.data.html_url,
|
||||
state: result.data.state,
|
||||
};
|
||||
}),
|
||||
});
|
||||
}
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user