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 AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ During normal plan review, an Archive sidebar tab provides the same browsing via
| `/api/external-annotations` | PATCH | Update fields on a single annotation (`?id=`) |
| `/api/external-annotations` | DELETE | Remove by `?id=`, `?source=`, or clear all |
| `/api/agents/capabilities` | GET | Check available agent providers (claude, codex, tour) |
| `/api/agents/review-profiles` | GET | List launchable review profiles (user dir + builtins) |
| `/api/agents/jobs/stream` | GET | SSE stream for real-time agent job status updates |
| `/api/agents/jobs` | GET | Snapshot of agent jobs (polling fallback, `?since=N` for version gating) |
| `/api/agents/jobs` | POST | Launch an agent job (body: `{ provider, command, label }`) |
Expand Down
31 changes: 30 additions & 1 deletion apps/pi-extension/server/agent-jobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export interface AgentJobHandlerOptions {
diffScope?: string;
/** Diff context snapshot at launch (stored on AgentJobInfo for per-job "Copy All"). */
diffContext?: AgentJobInfo["diffContext"];
/** Resolved review profile id at launch time. Stored on AgentJobInfo. */
reviewProfileId?: string;
/** Resolved review profile label at launch time. Stored on AgentJobInfo. */
reviewProfileLabel?: string;
} | null>;
/** Called when a job completes successfully — parse results and push annotations. */
onJobComplete?: (job: AgentJobInfo, meta: { outputPath?: string; stdout?: string; cwd?: string }) => void | Promise<void>;
Expand Down Expand Up @@ -124,7 +128,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {
command: string[],
label: string,
outputPath?: string,
spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean; prUrl?: string; diffScope?: string; diffContext?: AgentJobInfo["diffContext"] },
spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean; prUrl?: string; diffScope?: string; diffContext?: AgentJobInfo["diffContext"]; reviewProfileId?: string; reviewProfileLabel?: string },
): AgentJobInfo {
const id = crypto.randomUUID();
const source = jobSource(id);
Expand All @@ -146,6 +150,8 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {
...(spawnOptions?.prUrl && { prUrl: spawnOptions.prUrl }),
...(spawnOptions?.diffScope && { diffScope: spawnOptions.diffScope }),
...(spawnOptions?.diffContext && { diffContext: spawnOptions.diffContext }),
...(spawnOptions?.reviewProfileId && { reviewProfileId: spawnOptions.reviewProfileId }),
...(spawnOptions?.reviewProfileLabel && { reviewProfileLabel: spawnOptions.reviewProfileLabel }),
};

let proc: ChildProcess | null = null;
Expand Down Expand Up @@ -405,6 +411,22 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {
if (url.pathname === JOBS && req.method === "POST") {
try {
const body = await parseBody(req);

// Reject unknown fields rather than silently ignoring them (per the
// custom-reviews spec — a typo'd field should fail loud, not no-op).
const KNOWN_JOB_FIELDS = new Set([
"provider", "command", "label",
"engine", "model", "reasoningEffort", "effort", "fastMode",
"reviewProfileId",
]);
if (body && typeof body === "object") {
const unknown = Object.keys(body).filter((k) => !KNOWN_JOB_FIELDS.has(k));
if (unknown.length > 0) {
json(res, { error: `Unknown field(s): ${unknown.join(", ")}` }, 400);
return true;
}
}

const provider = typeof body.provider === "string" ? body.provider : "";
let rawCommand = Array.isArray(body.command) ? body.command : [];
let command = rawCommand.filter((c: unknown): c is string => typeof c === "string");
Expand All @@ -431,6 +453,8 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {
let jobPrUrl: string | undefined;
let jobDiffScope: string | undefined;
let jobDiffContext: AgentJobInfo["diffContext"] | undefined;
let jobReviewProfileId: string | undefined;
let jobReviewProfileLabel: string | undefined;
if (options.buildCommand) {
// Thread config from POST body to buildCommand
const config: Record<string, unknown> = {};
Expand All @@ -439,6 +463,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {
if (typeof body.reasoningEffort === "string") config.reasoningEffort = body.reasoningEffort;
if (typeof body.effort === "string") config.effort = body.effort;
if (body.fastMode === true) config.fastMode = true;
if (typeof body.reviewProfileId === "string") config.reviewProfileId = body.reviewProfileId;
const built = await options.buildCommand(provider, Object.keys(config).length > 0 ? config : undefined);
if (built) {
command = built.command;
Expand All @@ -456,6 +481,8 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {
jobPrUrl = built.prUrl;
jobDiffScope = built.diffScope;
jobDiffContext = built.diffContext;
jobReviewProfileId = built.reviewProfileId;
jobReviewProfileLabel = built.reviewProfileLabel;
}
}

Expand All @@ -477,6 +504,8 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {
prUrl: jobPrUrl,
diffScope: jobDiffScope,
diffContext: jobDiffContext,
reviewProfileId: jobReviewProfileId,
reviewProfileLabel: jobReviewProfileLabel,
});
json(res, { job }, 201);
} catch (err) {
Expand Down
116 changes: 88 additions & 28 deletions apps/pi-extension/server/serverReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import type { WorktreePool } from "../generated/worktree-pool.js";

import { createEditorAnnotationHandler } from "./annotations.js";
import { createAgentJobHandler } from "./agent-jobs.js";
import type { AgentJobInfo } from "../generated/agent-jobs.js";
import { type AgentJobInfo, REVIEW_OUTPUT_FAILED, markJobReviewFailed } from "../generated/agent-jobs.js";
import { createExternalAnnotationHandler } from "./external-annotations.js";
import {
handleDraftRequest,
Expand All @@ -72,15 +72,15 @@ import {
} from "./pr.js";
import { getRepoInfo } from "./project.js";
import {
CODEX_REVIEW_SYSTEM_PROMPT,
composeCodexReviewPrompt,
buildCodexCommand,
generateOutputPath,
parseCodexOutput,
transformReviewFindings,
} from "../generated/codex-review.js";
import { buildAgentReviewUserMessage, buildAgentReviewUserMessageForTarget, type WorkspaceReviewPromptContext } from "../generated/agent-review-message.js";
import {
CLAUDE_REVIEW_PROMPT,
composeClaudeReviewPrompt,
buildClaudeCommand,
parseClaudeStreamOutput,
transformClaudeFindings,
Expand All @@ -107,6 +107,12 @@ import {
SemanticDiffResponseCache,
} from "../generated/semantic-diff.js";
import type { SemanticDiffAvailability, SemanticDiffResponse } from "../generated/semantic-diff-types.js";
import { loadReviewProfiles } from "../generated/review-profile-loader.js";
import {
BUILTIN_DEFAULT_PROFILE,
type ResolvedReviewProfile,
type ReviewProfilesResponse,
} from "../generated/review-profiles.js";
import {
canStageFiles,
detectRemoteDefaultCompareTarget,
Expand Down Expand Up @@ -151,6 +157,9 @@ const piCodeNavRuntime: CodeNavRuntime = {
},
};

// Review ingestion completion semantics (REVIEW_OUTPUT_FAILED,
// markJobReviewFailed) now live in the shared agent-jobs module.

/** Detect if running inside WSL (Windows Subsystem for Linux) */
function detectWSL(): boolean {
if (process.platform !== "linux") return false;
Expand Down Expand Up @@ -491,6 +500,17 @@ export async function startReviewServer(options: {
: null;
const launchPrUrl = prMeta?.url;
const launchDiffScope = isPRMode ? currentPRDiffScope : undefined;

// Resolve the requested review profile. Profiles come from the user
// dir plus builtins. Absent or unknown id → builtin:default, so
// today's "Run Review" stays byte-identical.
const requestedProfileId =
typeof config?.reviewProfileId === "string" ? config.reviewProfileId : undefined;
const reviewProfile: ResolvedReviewProfile = requestedProfileId
? loadReviewProfiles().find((p) => p.id === requestedProfileId) ??
BUILTIN_DEFAULT_PROFILE
: BUILTIN_DEFAULT_PROFILE;

const diffContext: AgentJobInfo["diffContext"] | undefined = workspacePrompt
? { mode: String(currentDiffType), worktreePath: null }
: prMeta
Expand All @@ -510,7 +530,7 @@ export async function startReviewServer(options: {
prMetadata: prMeta,
config,
});
return built ? { ...built, prUrl: launchPrUrl, diffScope: launchDiffScope, diffContext } : built;
return built ? { ...built, prUrl: launchPrUrl, diffScope: launchDiffScope, diffContext, reviewProfileId: reviewProfile.id, reviewProfileLabel: reviewProfile.label } : built;
}

const userMessage = workspacePrompt
Expand All @@ -527,17 +547,17 @@ export async function startReviewServer(options: {
const reasoningEffort = typeof config?.reasoningEffort === "string" && config.reasoningEffort ? config.reasoningEffort : undefined;
const fastMode = config?.fastMode === true;
const outputPath = generateOutputPath();
const prompt = CODEX_REVIEW_SYSTEM_PROMPT + "\n\n---\n\n" + userMessage;
const prompt = composeCodexReviewPrompt(userMessage, reviewProfile);
const command = await buildCodexCommand({ cwd, outputPath, prompt, model, reasoningEffort, fastMode });
return { command, outputPath, prompt, cwd, label: jobLabel, model, reasoningEffort, fastMode: fastMode || undefined, prUrl: launchPrUrl, diffScope: launchDiffScope, diffContext };
return { command, outputPath, prompt, cwd, label: jobLabel, model, reasoningEffort, fastMode: fastMode || undefined, prUrl: launchPrUrl, diffScope: launchDiffScope, diffContext, reviewProfileId: reviewProfile.id, reviewProfileLabel: reviewProfile.label };
}

if (provider === "claude") {
const model = typeof config?.model === "string" && config.model ? config.model : undefined;
const effort = typeof config?.effort === "string" && config.effort ? config.effort : undefined;
const prompt = CLAUDE_REVIEW_PROMPT + "\n\n---\n\n" + userMessage;
const prompt = composeClaudeReviewPrompt(userMessage, reviewProfile);
const { command, stdinPrompt } = buildClaudeCommand(prompt, model, effort);
return { command, stdinPrompt, prompt, cwd, label: jobLabel, captureStdout: true, model, effort, prUrl: launchPrUrl, diffScope: launchDiffScope, diffContext };
return { command, stdinPrompt, prompt, cwd, label: jobLabel, captureStdout: true, model, effort, prUrl: launchPrUrl, diffScope: launchDiffScope, diffContext, reviewProfileId: reviewProfile.id, reviewProfileLabel: reviewProfile.label };
}

return null;
Expand All @@ -555,9 +575,33 @@ export async function startReviewServer(options: {
prRepo: getDisplayRepo(jobPrMeta),
} : jobPrUrl ? { prUrl: jobPrUrl } : {};

if (job.provider === "codex" && meta.outputPath) {
const output = await parseCodexOutput(meta.outputPath);
if (!output) return;
// Only tag annotations with a *custom* profile — the default review needs no tag.
const profileLabel =
job.reviewProfileId && job.reviewProfileId !== BUILTIN_DEFAULT_PROFILE.id
? job.reviewProfileLabel
: undefined;

// Map findings onto annotations and ingest. Shared by both engine branches;
// no-ops on an empty set so a clean (zero-finding) review stays "done".
const ingest = <T extends object>(transformed: readonly T[], logTag: string) => {
if (transformed.length === 0) return;
const annotations = transformed.map((a) => ({
...a,
...jobPrContext,
...(jobDiffScope && { diffScope: jobDiffScope }),
...(profileLabel && { reviewProfileLabel: profileLabel }),
}));
const result = externalAnnotations.addAnnotations({ annotations });
if ("error" in result) console.error(`[${logTag}] addAnnotations error:`, result.error);
};

if (job.provider === "codex") {
const output = meta.outputPath ? await parseCodexOutput(meta.outputPath) : null;
if (!output) {
// Process exited 0 but output is missing/unparseable — not a green run.
markJobReviewFailed(job, REVIEW_OUTPUT_FAILED);
return;
}

const hasBlockingFindings = output.findings.some(f => f.priority !== null && f.priority <= 1);
job.summary = {
Expand All @@ -566,25 +610,25 @@ export async function startReviewServer(options: {
confidence: output.overall_confidence_score,
};

if (output.findings.length > 0) {
const annotations = transformReviewFindings(
ingest(
transformReviewFindings(
output.findings,
job.source,
cwd,
"Codex",
workspace ? (filePath) => workspace.normalizeAnnotationPath(filePath) : undefined,
)
.map(a => ({ ...a, ...jobPrContext, ...(jobDiffScope && { diffScope: jobDiffScope }) }));
const result = externalAnnotations.addAnnotations({ annotations });
if ("error" in result) console.error(`[codex-review] addAnnotations error:`, result.error);
}
),
"codex-review",
);
return;
}

if (job.provider === "claude" && meta.stdout) {
const output = parseClaudeStreamOutput(meta.stdout);
if (job.provider === "claude") {
const stdout = meta.stdout ?? "";
const output = parseClaudeStreamOutput(stdout);
if (!output) {
console.error(`[claude-review] Failed to parse output (${meta.stdout.length} bytes, last 200: ${meta.stdout.slice(-200)})`);
console.error(`[claude-review] Failed to parse output (${stdout.length} bytes, last 200: ${stdout.slice(-200)})`);
markJobReviewFailed(job, REVIEW_OUTPUT_FAILED);
return;
}

Expand All @@ -595,17 +639,15 @@ export async function startReviewServer(options: {
confidence: total === 0 ? 1.0 : Math.max(0, 1.0 - (output.summary.important * 0.2)),
};

if (output.findings.length > 0) {
const annotations = transformClaudeFindings(
ingest(
transformClaudeFindings(
output.findings,
job.source,
cwd,
workspace ? (filePath) => workspace.normalizeAnnotationPath(filePath) : undefined,
)
.map(a => ({ ...a, ...jobPrContext, ...(jobDiffScope && { diffScope: jobDiffScope }) }));
const result = externalAnnotations.addAnnotations({ annotations });
if ("error" in result) console.error(`[claude-review] addAnnotations error:`, result.error);
}
),
"claude-review",
);
return;
}

Expand Down Expand Up @@ -1299,6 +1341,24 @@ export async function startReviewServer(options: {
await handleUploadRequest(req, res);
} else if (url.pathname === "/api/agents" && req.method === "GET") {
json(res, { agents: [] });
} else if (
url.pathname === "/api/agents/review-profiles" &&
req.method === "GET"
) {
// Custom reviews discovery. Reloaded per request, no file watching.
// Profiles come from the user dir plus builtins.
const profiles = loadReviewProfiles();
const body: ReviewProfilesResponse = {
profiles: profiles.map((p) => ({
id: p.id,
label: p.label,
description: p.description,
source: p.source,
sourcePath: p.sourcePath,
default: p.default,
})),
};
json(res, body);
} else if (url.pathname === "/api/git-add" && req.method === "POST") {
try {
const body = await parseBody(req);
Expand Down
5 changes: 3 additions & 2 deletions apps/pi-extension/vendor.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,20 @@ cd "$(dirname "$0")"
rm -rf generated
mkdir -p generated generated/ai/providers

for f in feedback-templates prompts review-core diff-paths cli-pagination jj-core vcs-core review-args storage draft project pr-types pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common favicon code-file resolve-file config external-annotation agent-jobs worktree worktree-pool html-to-markdown url-to-markdown tour annotate-args at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff; do
for f in feedback-templates prompts review-core diff-paths cli-pagination jj-core vcs-core review-args storage draft project pr-types pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common favicon code-file resolve-file config external-annotation agent-jobs worktree worktree-pool html-to-markdown url-to-markdown tour annotate-args at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff review-profiles; do
src="../../packages/shared/$f.ts"
printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts"
done

# Vendor review agent modules from packages/server/ — rewrite imports for generated/ layout
for f in agent-review-message codex-review claude-review path-utils; do
for f in agent-review-message codex-review claude-review path-utils review-profile-loader; do
src="../../packages/server/$f.ts"
printf '// @generated — DO NOT EDIT. Source: packages/server/%s.ts\n' "$f" | cat - "$src" \
| sed 's|from "./vcs"|from "./review-core.js"|' \
| sed 's|from "./pr"|from "./pr-provider.js"|' \
| sed 's|from "./path-utils"|from "./path-utils.js"|' \
| sed 's|from "@plannotator/shared/review-workspace"|from "./review-workspace.js"|' \
| sed 's|from "@plannotator/shared/review-profiles"|from "./review-profiles.js"|' \
| sed 's|from "@plannotator/shared/data-dir"|from "./data-dir"|' \
> "generated/$f.ts"
done
Expand Down
Loading
Loading