Skip to content

Commit 1d7ebac

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

2 files changed

Lines changed: 223 additions & 12 deletions

File tree

src/adapter.ts

Lines changed: 125 additions & 12 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,117 @@ 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 inlineCodeWithGithubFileLinks(task: JsonObject, rawText: string): string {
1198+
const direct = githubFileMarkdown(task, rawText);
1199+
if (direct !== `\`${rawText}\``) {
1200+
return direct;
1201+
}
1202+
1203+
let linkedAny = false;
1204+
const parts = rawText.split(/(\s+)/).map((part) => {
1205+
if (!part || /^\s+$/.test(part)) {
1206+
return part;
1207+
}
1208+
const linked = githubFileMarkdown(task, part);
1209+
if (linked !== `\`${part}\``) {
1210+
linkedAny = true;
1211+
return linked;
1212+
}
1213+
return `\`${part}\``;
1214+
});
1215+
1216+
return linkedAny ? parts.join("") : `\`${rawText}\``;
1217+
}
1218+
1219+
function linkGithubFileMentions(task: JsonObject, text: string): string {
1220+
const fencePattern = /(```[\s\S]*?```)/g;
1221+
return text
1222+
.split(fencePattern)
1223+
.map((segment, index) => {
1224+
if (index % 2 === 1) {
1225+
return segment;
1226+
}
1227+
return segment.replace(/`([^`\n]+)`/g, (match, rawPath: string, offset: number) => {
1228+
const alreadyLinkText = segment[offset - 1] === "[" && segment.slice(offset + match.length, offset + match.length + 2) === "](";
1229+
if (alreadyLinkText) {
1230+
return match;
1231+
}
1232+
const linked = inlineCodeWithGithubFileLinks(task, rawPath);
1233+
return linked === `\`${rawPath}\`` ? match : linked;
1234+
});
1235+
})
1236+
.join("");
1237+
}
1238+
1239+
function reviewTestCommandMarkdown(task: JsonObject, rawCommand: string): string {
1240+
const command = rawCommand.trim();
1241+
if (!command) {
1242+
return "`unknown command`";
1243+
}
1244+
return inlineCodeWithGithubFileLinks(task, command);
1245+
}
1246+
11361247
function reviewFixLoopLines(task: JsonObject): string[] {
11371248
const loops = Array.isArray(task.review_fix_loops) ? (task.review_fix_loops as JsonObject[]) : [];
11381249
if (!loops.length) {
@@ -1148,7 +1259,7 @@ function reviewFixLoopLines(task: JsonObject): string[] {
11481259
return lines;
11491260
}
11501261

1151-
function structuredReviewLines(review: JsonObject): string[] {
1262+
function structuredReviewLines(review: JsonObject, task: JsonObject): string[] {
11521263
if (!Object.keys(review).length) {
11531264
return ["", "### Structured review", "- No structured review result was emitted."];
11541265
}
@@ -1162,15 +1273,15 @@ function structuredReviewLines(review: JsonObject): string[] {
11621273
const reviewedFiles = Array.isArray(review.reviewed_files) ? review.reviewed_files : [];
11631274
lines.push(`- Reviewed files: ${reviewedFiles.length}`);
11641275
if (reviewedFiles.length) {
1165-
lines.push(`- Reviewed file list: ${reviewedFiles.slice(0, 20).map((path) => `\`${path}\``).join(", ")}`);
1276+
lines.push(`- Reviewed file list: ${reviewedFiles.slice(0, 20).map((path) => githubFileMarkdown(task, String(path))).join(", ")}`);
11661277
if (reviewedFiles.length > 20) {
11671278
lines.push("- Reviewed file list truncated after 20 entries.");
11681279
}
11691280
}
11701281
const supportingFiles = Array.isArray(review.supporting_files) ? review.supporting_files : [];
11711282
lines.push(`- Supporting files inspected: ${supportingFiles.length}`);
11721283
if (supportingFiles.length) {
1173-
lines.push(`- Supporting file list: ${supportingFiles.slice(0, 20).map((path) => `\`${path}\``).join(", ")}`);
1284+
lines.push(`- Supporting file list: ${supportingFiles.slice(0, 20).map((path) => githubFileMarkdown(task, String(path))).join(", ")}`);
11741285
if (supportingFiles.length > 20) {
11751286
lines.push("- Supporting file list truncated after 20 entries.");
11761287
}
@@ -1182,19 +1293,21 @@ function structuredReviewLines(review: JsonObject): string[] {
11821293
if (finding.line !== undefined && finding.line !== null) {
11831294
location = `${location}:${finding.line}`;
11841295
}
1185-
lines.push(` ${index + 1}. \`${finding.severity || "unknown"}\` ${location} - ${finding.title || "Untitled finding"}`);
1296+
const linkedLocation = githubFileMarkdown(task, location, Number(finding.line || 0) || null);
1297+
lines.push(` ${index + 1}. \`${finding.severity || "unknown"}\` ${linkedLocation} - ${finding.title || "Untitled finding"}`);
11861298
});
11871299
if (findings.length > 10) {
11881300
lines.push("- Findings truncated after 10 entries.");
11891301
}
11901302
if (review.no_findings_reason) {
1191-
lines.push(`- No-findings reason: ${review.no_findings_reason}`);
1303+
lines.push(`- No-findings reason: ${linkGithubFileMentions(task, String(review.no_findings_reason))}`);
11921304
}
11931305
const testsRun = Array.isArray(review.tests_run) ? (review.tests_run as JsonObject[]) : [];
11941306
lines.push(`- Tests reported by runtime: ${testsRun.length}`);
11951307
testsRun.slice(0, 10).forEach((item) => {
1196-
const summary = item.output_summary ? ` - ${item.output_summary}` : "";
1197-
lines.push(` - \`${item.command || "unknown command"}\`: \`${item.status || "unknown"}\`${summary}`);
1308+
const command = reviewTestCommandMarkdown(task, String(item.command || "unknown command"));
1309+
const summary = item.output_summary ? ` - ${linkGithubFileMentions(task, String(item.output_summary))}` : "";
1310+
lines.push(` - ${command}: \`${item.status || "unknown"}\`${summary}`);
11981311
});
11991312
if (testsRun.length > 10) {
12001313
lines.push("- Test list truncated after 10 entries.");

tests/webhook-adapter.test.ts

Lines changed: 98 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,103 @@ 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+
"- `Read src/lib/server/skill-scan.ts`",
370+
"- `README.md:12`",
371+
"- `pnpm test`",
372+
"",
373+
"```ts",
374+
"`src/not-linked-inside-fence.ts`",
375+
"```",
376+
].join("\n"),
377+
review: {},
378+
},
379+
);
380+
381+
assert.match(
382+
body,
383+
/\[`src\/lib\/server\/skills-directory\.ts`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/abc123def456\/src\/lib\/server\/skills-directory\.ts\)/,
384+
);
385+
assert.match(
386+
body,
387+
/`Read` \[`src\/lib\/server\/skill-scan\.ts`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/abc123def456\/src\/lib\/server\/skill-scan\.ts\)/,
388+
);
389+
assert.match(
390+
body,
391+
/\[`README\.md:12`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/abc123def456\/README\.md#L12\)/,
392+
);
393+
assert.match(body, /- `pnpm test`/);
394+
assert.match(body, /`src\/not-linked-inside-fence\.ts`/);
395+
assert.doesNotMatch(body, /\[`src\/not-linked-inside-fence\.ts`\]/);
396+
});
397+
398+
test("publication body links structured review file lists and findings", () => {
399+
const body = publicationCommentBody(
400+
{
401+
task_id: "task-structured-links",
402+
repository: "OpenCoven/coven-github-webhook",
403+
default_branch: "main",
404+
review_evidence: {
405+
head_sha: "feedface",
406+
changed_files: ["src/app.ts"],
407+
changed_file_count: 1,
408+
},
409+
},
410+
{
411+
status: "success",
412+
summary: "Done.",
413+
review: {
414+
mode: "review",
415+
evidence_status: "complete",
416+
reviewed_files: ["src/app.ts"],
417+
supporting_files: ["tests/app.test.ts"],
418+
findings: [
419+
{
420+
severity: "medium",
421+
file: "src/app.ts",
422+
line: 7,
423+
title: "Example finding",
424+
},
425+
],
426+
no_findings_reason: "Checked `tests/app.test.ts` with `npm test`.",
427+
tests_run: [
428+
{
429+
command: "Read src/app.ts",
430+
status: "passed",
431+
output_summary: "inspected `tests/app.test.ts` coverage.",
432+
},
433+
{
434+
command: "npm test",
435+
status: "passed",
436+
},
437+
],
438+
},
439+
},
440+
);
441+
442+
assert.match(body, /\[`src\/app\.ts`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/feedface\/src\/app\.ts\)/);
443+
assert.match(body, /\[`tests\/app\.test\.ts`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/feedface\/tests\/app\.test\.ts\)/);
444+
assert.match(body, /\[`src\/app\.ts:7`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/feedface\/src\/app\.ts#L7\)/);
445+
assert.match(body, /`Read` \[`src\/app\.ts`\]\(https:\/\/github\.com\/OpenCoven\/coven-github-webhook\/blob\/feedface\/src\/app\.ts\): `passed`/);
446+
assert.match(body, /with `npm test`/);
447+
assert.match(body, /- `npm test`: `passed`/);
448+
});
449+
352450
test("demo mode handles a signed labeled issue without external GitHub calls", async () => {
353451
const secret = "demo-route-secret";
354452
const stateDir = tempStateDir();

0 commit comments

Comments
 (0)