Skip to content

feat(review): add local ESLint checks to code review #1148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leoreisdias wants to merge 3 commits into
base: main
Choose a base branch
from
+1,849 −7
Open

feat(review): add local ESLint checks to code review #1148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc66edd
Add local ESLint checks to Pi code reviews
leoreisdias Jul 29, 2026
3044b18
Format ESLint review check code for consistency
leoreisdias Jul 29, 2026
ad64fef
Remove trailing blank line from ESLint types
leoreisdias Jul 29, 2026
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
95 changes: 95 additions & 0 deletions apps/pi-extension/server.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,32 @@ function makeMockSem(dir: string, options: {
return semPath;
}

function makeMockEslintProject(dir: string): void {
mkdirSync(join(dir, "src"), { recursive: true });
mkdirSync(join(dir, "node_modules", "eslint", "bin"), { recursive: true });
writeFileSync(join(dir, "src", "app.ts"), "export const value = 1;\n", "utf-8");
writeFileSync(join(dir, "eslint.config.js"), "export default [];\n", "utf-8");
writeFileSync(join(dir, "node_modules", "eslint", "package.json"), JSON.stringify({
version: "9.12.0",
bin: { eslint: "bin/eslint.cjs" },
}), "utf-8");
writeFileSync(join(dir, "node_modules", "eslint", "bin", "eslint.cjs"), [
'const { join } = require("node:path");',
"process.stdout.write(JSON.stringify([{",
' filePath: join(process.cwd(), "src", "app.ts"),',
" messages: [{",
' ruleId: "react-hooks/exhaustive-deps",',
" severity: 1,",
' message: "React Hook has a missing dependency.",',
" line: 1,",
" column: 1,",
" }],",
"}]));",
"process.exitCode = 1;",
"",
].join("\n"), "utf-8");
}

function makeBlockingSem(dir: string): { semPath: string; startedPath: string; releasePath: string } {
const semPath = join(dir, "sem-blocking");
const startedPath = join(dir, "started");
Expand Down Expand Up @@ -615,6 +641,75 @@ describe("pi review server", () => {
}
}, 10_000);

test("advertises and runs the reviewed project's local ESLint for the current snapshot", async () => {
const dir = makeTempDir("plannotator-pi-eslint-server-");
makeMockEslintProject(dir);
git(dir, ["init"]);
git(dir, ["branch", "-M", "main"]);
git(dir, ["config", "user.email", "pi-review@example.com"]);
git(dir, ["config", "user.name", "Pi Review"]);
writeFileSync(join(dir, "README.md"), "# Test\n", "utf-8");
git(dir, ["add", "README.md"]);
git(dir, ["commit", "-m", "initial"]);
const gitContext = await getVcsContext(dir, "git");
process.env.PLANNOTATOR_PORT = String(await reservePort());
const rawPatch = [
"diff --git a/src/app.ts b/src/app.ts",
"new file mode 100644",
"--- /dev/null",
"+++ b/src/app.ts",
"@@ -0,0 +1 @@",
"+export const value = 1;",
"",
].join("\n");
const server = await startReviewServer({
rawPatch,
gitRef: "test",
diffType: "uncommitted",
gitContext,
origin: "pi",
htmlContent: "<!doctype html><html><body>review</body></html>",
});

try {
const diffPayload = await fetch(`${server.url}/api/diff`).then((response) => response.json()) as {
snapshotId: string;
eslintCheck?: { available: boolean; fileCount?: number; projectCount?: number };
};
expect(diffPayload.eslintCheck).toEqual({ available: true, fileCount: 1, projectCount: 1 });

const response = await fetch(`${server.url}/api/eslint-check`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ snapshotId: diffPayload.snapshotId }),
});
expect(response.status).toBe(200);
await expect(response.json()).resolves.toMatchObject({
status: "ok",
eslintVersions: ["9.12.0"],
summary: { errors: 0, warnings: 1, changedLineWarnings: 1 },
diagnostics: [{ filePath: "src/app.ts", line: 1, onChangedLine: true }],
});

writeFileSync(join(dir, "src", "app.ts"), "export const value = 2;\n", "utf-8");
const changedResponse = await fetch(`${server.url}/api/eslint-check`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ snapshotId: diffPayload.snapshotId }),
});
expect(changedResponse.status).toBe(409);

const staleResponse = await fetch(`${server.url}/api/eslint-check`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ snapshotId: "stale" }),
});
expect(staleResponse.status).toBe(409);
} finally {
server.stop();
}
});

test("advertises semantic diff availability and serves parsed sem output", async () => {
const dir = makeTempDir("plannotator-pi-sem-server-");
const dataDir = makeTempDir("plannotator-pi-sem-data-");
Expand Down
169 changes: 169 additions & 0 deletions apps/pi-extension/server/serverReview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ import {
SemanticDiffResponseCache,
} from "../generated/semantic-diff.js";
import type { SemanticDiffAvailability, SemanticDiffResponse } from "../generated/semantic-diff-types.js";
import {
buildEslintCheckInput,
getEslintCheckAvailability,
isEslintCheckCompatibleReviewView,
runEslintCheck,
} from "../generated/eslint-check.js";
import type { EslintCheckAdvert, EslintCheckResponse } from "../generated/eslint-check-types.js";
import { discoverCuratedSkills, resolveRequestedReviewProfile, listAllSkills, enableReviewSkill } from "../generated/review-skill-loader.js";
import {
BUILTIN_DEFAULT_PROFILE,
Expand Down Expand Up @@ -843,6 +850,157 @@ export async function startReviewServer(options: {
return result;
}

function eslintCheckCompatibleView(): boolean {
return isEslintCheckCompatibleReviewView({
isPRMode,
isWorkspaceMode: !!workspace,
diffType: currentDiffType as string,
});
}

function eslintCheckUnavailableReason(): string {
if (isPRMode) return "pr-review-unsupported";

return eslintCheckCompatibleView() ? "local-checkout-unavailable" : "snapshot-not-working-tree";
}

function resolveEslintCheckInput() {
if (!eslintCheckCompatibleView()) return null;

const cwd = workspace?.root
?? (isPRMode
? resolvePRLocalCwd()
: resolveAgentCwd());

if (!cwd) return null;

return buildEslintCheckInput(
currentPatch,
cwd,
workspace?.repos.map((repo) => ({ label: repo.label, cwd: repo.cwd })),
);
}

function getEslintCheckAdvert(): EslintCheckAdvert {
const input = resolveEslintCheckInput();

if (!input) {
return {
available: false,
reason: eslintCheckUnavailableReason(),
};
}

return getEslintCheckAvailability(input);
}

interface EslintCheckBaseline {
snapshotId: string;
fingerprintGeneration: number;
fingerprint: string | null;
}

async function resolveEslintCheckBaseline(
requestedSnapshotId: string | undefined,
): Promise<EslintCheckBaseline | null> {
if (requestedSnapshotId !== currentSnapshotId()) return null;

const baselineGeneration = fingerprintGeneration;
let baseline = currentFingerprint;
const pendingCapture = pendingFingerprintCapture;

if (baseline == null && pendingCapture) {
baseline = await pendingCapture;
}

if (
requestedSnapshotId !== currentSnapshotId()
|| baselineGeneration !== fingerprintGeneration
) {
return null;
}

if (baseline != null) {
const probe = await fileContentFingerprintProbes.run(
`${requestedSnapshotId}:${baselineGeneration}`,
computeDiffFingerprint,
);

if (
requestedSnapshotId !== currentSnapshotId()
|| currentFingerprint !== baseline
|| (probe != null && probe !== baseline)
) {
return null;
}
}

return {
snapshotId: requestedSnapshotId,
fingerprintGeneration: baselineGeneration,
fingerprint: baseline,
};
}

function sameEslintCheckBaseline(
left: EslintCheckBaseline,
right: EslintCheckBaseline,
): boolean {
return left.snapshotId === right.snapshotId
&& left.fingerprintGeneration === right.fingerprintGeneration
&& left.fingerprint === right.fingerprint;
}

let eslintCheckCache: {
baseline: EslintCheckBaseline;
response: EslintCheckResponse;
} | null = null;

async function getEslintCheck(requestedSnapshotId: string | undefined): Promise<EslintCheckResponse> {
const baseline = await resolveEslintCheckBaseline(requestedSnapshotId);

if (!baseline) {
return {
status: "error",
reason: "stale-snapshot",
message: "The reviewed diff changed before ESLint started. Run the check again.",
};
}

if (eslintCheckCache && sameEslintCheckBaseline(eslintCheckCache.baseline, baseline)) {
return eslintCheckCache.response;
}

const input = resolveEslintCheckInput();

if (!input) {
return {
status: "unavailable",
reason: eslintCheckUnavailableReason(),
message: isPRMode
? "ESLint is currently available only for local code reviews."
: eslintCheckCompatibleView()
? "ESLint requires a local checkout of the code under review."
: "ESLint is available only when the review's new side is the current working tree.",
};
}

const response = await runEslintCheck(input);
const completedBaseline = await resolveEslintCheckBaseline(requestedSnapshotId);

if (!completedBaseline || !sameEslintCheckBaseline(baseline, completedBaseline)) {
return {
status: "error",
reason: "stale-snapshot",
message: "The reviewed diff changed while ESLint was running. Run the check again.",
};
}

if (response.status === "ok") eslintCheckCache = { baseline, response };

return response;
}

const agentJobs = createAgentJobHandler({
mode: "review",
getServerUrl: () => serverUrl,
Expand Down Expand Up @@ -1498,6 +1656,7 @@ export async function startReviewServer(options: {
...(baseBehindRemote && { baseBehindRemote: true }),
...(servedError && { error: servedError }),
semanticDiff: await getSemanticDiffAdvert(servedDiffType as DiffType),
eslintCheck: getEslintCheckAdvert(),
serverConfig: getServerConfig(gitUser),
});
} else if (url.pathname === "/api/fetch-base" && req.method === "POST") {
Expand Down Expand Up @@ -1581,6 +1740,11 @@ export async function startReviewServer(options: {
});
} else if (url.pathname === "/api/semantic-diff" && req.method === "GET") {
json(res, await getSemanticDiff(url));
} else if (url.pathname === "/api/eslint-check" && req.method === "POST") {
const body = await parseBody(req) as { snapshotId?: unknown };
const requestedSnapshotId = typeof body.snapshotId === "string" ? body.snapshotId : undefined;
const result = await getEslintCheck(requestedSnapshotId);
json(res, result, result.status === "error" && result.reason === "stale-snapshot" ? 409 : 200);
} else if (url.pathname === "/api/commits" && req.method === "GET") {
// Linear commit history for the Commits panel (mirrors Bun review.ts).
// Git-local sessions only — PR/workspace/jj/p4 don't offer the view.
Expand Down Expand Up @@ -1662,6 +1826,7 @@ export async function startReviewServer(options: {
hideWhitespace: currentHideWhitespace,
...(currentError ? { error: currentError } : {}),
semanticDiff: await getSemanticDiffAdvert(),
eslintCheck: getEslintCheckAdvert(),
});
return;
}
Expand Down Expand Up @@ -1776,6 +1941,7 @@ export async function startReviewServer(options: {
...(updatedContext ? { gitContext: updatedContext } : {}),
...(currentError ? { error: currentError } : {}),
semanticDiff: switchSemanticDiff,
eslintCheck: getEslintCheckAdvert(),
});
} catch (err) {
const message = err instanceof Error ? err.message : "Failed to switch diff";
Expand Down Expand Up @@ -1872,6 +2038,7 @@ export async function startReviewServer(options: {
...(layerPatchIncomplete ? { prPatchIncomplete: true, prPatchUpgradeAvailable: layerUpgradeAvailable } : {}),
...((currentError ?? upgradeError) ? { error: currentError ?? upgradeError } : {}),
semanticDiff: await getSemanticDiffAdvert(),
eslintCheck: getEslintCheckAdvert(),
});
return;
}
Expand Down Expand Up @@ -1908,6 +2075,7 @@ export async function startReviewServer(options: {
snapshotId: currentSnapshotId(),
prDiffScope: currentPRDiffScope,
semanticDiff: await getSemanticDiffAdvert(),
eslintCheck: getEslintCheckAdvert(),
});
} catch (err) {
const message = err instanceof Error ? err.message : "Failed to switch PR diff scope";
Expand Down Expand Up @@ -2002,6 +2170,7 @@ export async function startReviewServer(options: {
...(switchedViewedFiles.length > 0 && { viewedFiles: switchedViewedFiles }),
...(currentError ? { error: currentError } : {}),
semanticDiff: await getSemanticDiffAdvert(),
eslintCheck: getEslintCheckAdvert(),
});
} catch (err) {
return json(res, { error: err instanceof Error ? err.message : "Failed to switch PR" }, 500);
Expand Down
2 changes: 1 addition & 1 deletion apps/pi-extension/vendor.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ for f in config-types storage-types workspace-status-types; do
done

# Everything else in the original flat list stays sourced from packages/shared.
for f in prompts review-core diff-paths cli-pagination jj-core gitbutler-core vcs-core review-args draft pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common resolve-file annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node 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 single-flight source-save-node review-profiles guide commit-avatars commit-history port-range; do
for f in prompts review-core diff-paths cli-pagination jj-core gitbutler-core vcs-core review-args draft pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common resolve-file annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node 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 eslint-check-types eslint-check single-flight source-save-node review-profiles guide commit-avatars commit-history port-range; do
src="../../packages/shared/$f.ts"
printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts"
done
Expand Down
Loading