Skip to content

Commit cd18c06

Browse files
fix webhook review file links
1 parent 52c6d2b commit cd18c06

2 files changed

Lines changed: 172 additions & 10 deletions

File tree

src/adapter.ts

Lines changed: 92 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,10 +1092,10 @@ async function publishResultIfConfigured(task: JsonObject, resultPath: string, t
10921092
}
10931093
}
10941094

1095-
function publicationCommentBody(task: JsonObject, result: JsonObject): string {
1095+
export function publicationCommentBody(task: JsonObject, result: JsonObject): string {
10961096
const status = result.status || "unknown";
1097-
const summary = String(result.summary || "No summary returned.");
1098-
const prBody = String(result.pr_body || "");
1097+
const summary = linkGithubFileMentions(task, String(result.summary || "No summary returned."));
1098+
const prBody = linkGithubFileMentions(task, String(result.pr_body || ""));
10991099
const filesChanged = Array.isArray(result.files_changed) ? result.files_changed : [];
11001100
const commits = Array.isArray(result.commits) ? result.commits : [];
11011101
const taskId = String(task.task_id || "");
@@ -1117,12 +1117,12 @@ function publicationCommentBody(task: JsonObject, result: JsonObject): string {
11171117
`- Review context SHA-256: \`${evidence.review_context_sha256}\``,
11181118
);
11191119
if (changedFiles.length) {
1120-
parts.push(`- Files: ${changedFiles.slice(0, 20).map((file) => `\`${file}\``).join(", ")}`);
1120+
parts.push(`- Files: ${changedFiles.slice(0, 20).map((file) => githubFileMarkdown(task, String(file))).join(", ")}`);
11211121
}
11221122
} else {
11231123
parts.push("- No PR review evidence was captured for this run.");
11241124
}
1125-
parts.push(...structuredReviewLines(review));
1125+
parts.push(...structuredReviewLines(review, task));
11261126
parts.push(
11271127
"",
11281128
`**Files changed:** ${filesChanged.length}`,
@@ -1133,6 +1133,87 @@ function publicationCommentBody(task: JsonObject, result: JsonObject): string {
11331133
return parts.join("\n");
11341134
}
11351135

1136+
function githubBlobBase(task: JsonObject): string | null {
1137+
const repository = String(task.repository || "").trim();
1138+
if (!/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repository)) {
1139+
return null;
1140+
}
1141+
const evidence = (task.review_evidence as JsonObject | undefined) || {};
1142+
const ref = String(evidence.head_sha || evidence.workspace_head_sha || task.default_branch || "").trim();
1143+
if (!ref) {
1144+
return null;
1145+
}
1146+
const encodedRef = encodeURIComponent(ref).replaceAll("%2F", "/");
1147+
return `https://github.com/${repository}/blob/${encodedRef}`;
1148+
}
1149+
1150+
function parseRepoRelativePath(rawPath: string): {display: string; path: string; line: number | null} | null {
1151+
const display = rawPath.trim();
1152+
if (!display || display !== rawPath || /[\s<>]/.test(display)) {
1153+
return null;
1154+
}
1155+
if (/^[\\/]/.test(display) || /^[A-Za-z]:[\\/]/.test(display)) {
1156+
return null;
1157+
}
1158+
1159+
let path = display.replaceAll("\\", "/");
1160+
let line: number | null = null;
1161+
const lineMatch = /^(.*):(\d+)$/.exec(path);
1162+
if (lineMatch) {
1163+
path = lineMatch[1];
1164+
line = Number(lineMatch[2]);
1165+
}
1166+
if (/^[A-Za-z][A-Za-z0-9+.-]*:/.test(path)) {
1167+
return null;
1168+
}
1169+
while (path.startsWith("./")) {
1170+
path = path.slice(2);
1171+
}
1172+
const segments = path.split("/");
1173+
const filename = segments.at(-1) || "";
1174+
if (
1175+
!path ||
1176+
path.includes("//") ||
1177+
segments.some((segment) => !segment || segment === "." || segment === "..") ||
1178+
!/\.[A-Za-z][A-Za-z0-9._-]{0,15}$/.test(filename)
1179+
) {
1180+
return null;
1181+
}
1182+
return {display, path, line};
1183+
}
1184+
1185+
function githubFileMarkdown(task: JsonObject, rawPath: string, lineOverride: number | null = null): string {
1186+
const target = parseRepoRelativePath(rawPath);
1187+
const base = githubBlobBase(task);
1188+
if (!target || !base) {
1189+
return `\`${rawPath}\``;
1190+
}
1191+
const encodedPath = target.path.split("/").map((segment) => encodeURIComponent(segment)).join("/");
1192+
const line = lineOverride ?? target.line;
1193+
const lineAnchor = line !== null && Number.isFinite(line) && line > 0 ? `#L${line}` : "";
1194+
return `[\`${target.display}\`](${base}/${encodedPath}${lineAnchor})`;
1195+
}
1196+
1197+
function linkGithubFileMentions(task: JsonObject, text: string): string {
1198+
const fencePattern = /(```[\s\S]*?```)/g;
1199+
return text
1200+
.split(fencePattern)
1201+
.map((segment, index) => {
1202+
if (index % 2 === 1) {
1203+
return segment;
1204+
}
1205+
return segment.replace(/`([^`\n]+)`/g, (match, rawPath: string, offset: number) => {
1206+
const alreadyLinkText = segment[offset - 1] === "[" && segment.slice(offset + match.length, offset + match.length + 2) === "](";
1207+
if (alreadyLinkText) {
1208+
return match;
1209+
}
1210+
const linked = githubFileMarkdown(task, rawPath);
1211+
return linked === `\`${rawPath}\`` ? match : linked;
1212+
});
1213+
})
1214+
.join("");
1215+
}
1216+
11361217
function reviewFixLoopLines(task: JsonObject): string[] {
11371218
const loops = Array.isArray(task.review_fix_loops) ? (task.review_fix_loops as JsonObject[]) : [];
11381219
if (!loops.length) {
@@ -1148,7 +1229,7 @@ function reviewFixLoopLines(task: JsonObject): string[] {
11481229
return lines;
11491230
}
11501231

1151-
function structuredReviewLines(review: JsonObject): string[] {
1232+
function structuredReviewLines(review: JsonObject, task: JsonObject): string[] {
11521233
if (!Object.keys(review).length) {
11531234
return ["", "### Structured review", "- No structured review result was emitted."];
11541235
}
@@ -1162,15 +1243,15 @@ function structuredReviewLines(review: JsonObject): string[] {
11621243
const reviewedFiles = Array.isArray(review.reviewed_files) ? review.reviewed_files : [];
11631244
lines.push(`- Reviewed files: ${reviewedFiles.length}`);
11641245
if (reviewedFiles.length) {
1165-
lines.push(`- Reviewed file list: ${reviewedFiles.slice(0, 20).map((path) => `\`${path}\``).join(", ")}`);
1246+
lines.push(`- Reviewed file list: ${reviewedFiles.slice(0, 20).map((path) => githubFileMarkdown(task, String(path))).join(", ")}`);
11661247
if (reviewedFiles.length > 20) {
11671248
lines.push("- Reviewed file list truncated after 20 entries.");
11681249
}
11691250
}
11701251
const supportingFiles = Array.isArray(review.supporting_files) ? review.supporting_files : [];
11711252
lines.push(`- Supporting files inspected: ${supportingFiles.length}`);
11721253
if (supportingFiles.length) {
1173-
lines.push(`- Supporting file list: ${supportingFiles.slice(0, 20).map((path) => `\`${path}\``).join(", ")}`);
1254+
lines.push(`- Supporting file list: ${supportingFiles.slice(0, 20).map((path) => githubFileMarkdown(task, String(path))).join(", ")}`);
11741255
if (supportingFiles.length > 20) {
11751256
lines.push("- Supporting file list truncated after 20 entries.");
11761257
}
@@ -1182,13 +1263,14 @@ function structuredReviewLines(review: JsonObject): string[] {
11821263
if (finding.line !== undefined && finding.line !== null) {
11831264
location = `${location}:${finding.line}`;
11841265
}
1185-
lines.push(` ${index + 1}. \`${finding.severity || "unknown"}\` ${location} - ${finding.title || "Untitled finding"}`);
1266+
const linkedLocation = githubFileMarkdown(task, location, Number(finding.line || 0) || null);
1267+
lines.push(` ${index + 1}. \`${finding.severity || "unknown"}\` ${linkedLocation} - ${finding.title || "Untitled finding"}`);
11861268
});
11871269
if (findings.length > 10) {
11881270
lines.push("- Findings truncated after 10 entries.");
11891271
}
11901272
if (review.no_findings_reason) {
1191-
lines.push(`- No-findings reason: ${review.no_findings_reason}`);
1273+
lines.push(`- No-findings reason: ${linkGithubFileMentions(task, String(review.no_findings_reason))}`);
11921274
}
11931275
const testsRun = Array.isArray(review.tests_run) ? (review.tests_run as JsonObject[]) : [];
11941276
lines.push(`- Tests reported by runtime: ${testsRun.length}`);

tests/webhook-adapter.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
buildTaskFromEvent,
1010
createConfig,
1111
handleRequest,
12+
publicationCommentBody,
1213
type JsonObject,
1314
} from "../src/adapter.js";
1415

@@ -349,6 +350,85 @@ test("example policy routes a labeled issue to the configured familiar", () => {
349350
});
350351
});
351352

353+
test("publication body links screenshot-style file mentions to GitHub blobs", () => {
354+
const body = publicationCommentBody(
355+
{
356+
task_id: "task-file-links",
357+
repository: "OpenCoven/coven-github-webhook",
358+
default_branch: "main",
359+
review_evidence: {
360+
head_sha: "abc123def456",
361+
},
362+
},
363+
{
364+
status: "success",
365+
summary: [
366+
"### Files inspected",
367+
"",
368+
"- `src/lib/server/skills-directory.ts`",
369+
"- `README.md:12`",
370+
"- `pnpm test`",
371+
"",
372+
"```ts",
373+
"`src/not-linked-inside-fence.ts`",
374+
"```",
375+
].join("\n"),
376+
review: {},
377+
},
378+
);
379+
380+
assert.match(
381+
body,
382+
/\[`src\/lib\/server\/skills-directory\.ts`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/abc123def456\/src\/lib\/server\/skills-directory\.ts\)/,
383+
);
384+
assert.match(
385+
body,
386+
/\[`README\.md:12`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/abc123def456\/README\.md#L12\)/,
387+
);
388+
assert.match(body, /- `pnpm test`/);
389+
assert.match(body, /`src\/not-linked-inside-fence\.ts`/);
390+
assert.doesNotMatch(body, /\[`src\/not-linked-inside-fence\.ts`\]/);
391+
});
392+
393+
test("publication body links structured review file lists and findings", () => {
394+
const body = publicationCommentBody(
395+
{
396+
task_id: "task-structured-links",
397+
repository: "OpenCoven/coven-github-webhook",
398+
default_branch: "main",
399+
review_evidence: {
400+
head_sha: "feedface",
401+
changed_files: ["src/app.ts"],
402+
changed_file_count: 1,
403+
},
404+
},
405+
{
406+
status: "success",
407+
summary: "Done.",
408+
review: {
409+
mode: "review",
410+
evidence_status: "complete",
411+
reviewed_files: ["src/app.ts"],
412+
supporting_files: ["tests/app.test.ts"],
413+
findings: [
414+
{
415+
severity: "medium",
416+
file: "src/app.ts",
417+
line: 7,
418+
title: "Example finding",
419+
},
420+
],
421+
no_findings_reason: "Checked `tests/app.test.ts` with `npm test`.",
422+
},
423+
},
424+
);
425+
426+
assert.match(body, /\[`src\/app\.ts`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/feedface\/src\/app\.ts\)/);
427+
assert.match(body, /\[`tests\/app\.test\.ts`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/feedface\/tests\/app\.test\.ts\)/);
428+
assert.match(body, /\[`src\/app\.ts:7`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/feedface\/src\/app\.ts#L7\)/);
429+
assert.match(body, /with `npm test`/);
430+
});
431+
352432
test("demo mode handles a signed labeled issue without external GitHub calls", async () => {
353433
const secret = "demo-route-secret";
354434
const stateDir = tempStateDir();

0 commit comments

Comments
 (0)