Skip to content

Commit 71f4b39

Browse files
fix: publish native GitHub reviews
1 parent 52c6d2b commit 71f4b39

3 files changed

Lines changed: 378 additions & 17 deletions

File tree

README.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,17 @@ connection guide in
107107
GitHub or `coven-code`.
108108
- Captures PR checkout metadata and changed-file patches before invoking
109109
`coven-code`.
110-
- Publishes visible structured review evidence, including `reviewed_files`,
111-
`supporting_files`, findings, test evidence, no-findings rationale, and
112-
limitations.
110+
- Publishes PR results as native GitHub reviews: complete no-finding evidence
111+
approves, actionable findings request changes, and incomplete or
112+
contradictory evidence is published as a comment review. Findings are made
113+
inline only when their captured diff location is valid; all other findings
114+
remain in the review body.
115+
- Persists publication identities and review/comment IDs in the configured
116+
state directory so retries do not duplicate output and a newer review links
117+
to the prior covencat publication it supersedes.
118+
- Publishes non-PR task results and operational notices as issue comments,
119+
including structured `reviewed_files`, `supporting_files`, findings, test
120+
evidence, no-findings rationale, and limitations.
113121
- When `COVEN_REVIEW_FIX_LOOPS` is greater than `0`, reruns `coven-code` with
114122
prior structured review findings as explicit repair instructions until no
115123
findings remain or the configured loop count is exhausted.

src/adapter.ts

Lines changed: 201 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export interface AdapterConfig {
1212
stateDir: string;
1313
deliveriesDir: string;
1414
tasksDir: string;
15+
publicationsDir: string;
1516
workspacesDir: string;
1617
attemptsDir: string;
1718
policyPath: string;
@@ -69,6 +70,7 @@ export function createConfig(env: NodeJS.ProcessEnv = process.env, rootDir = pro
6970
stateDir,
7071
deliveriesDir: join(stateDir, "deliveries"),
7172
tasksDir: join(stateDir, "tasks"),
73+
publicationsDir: join(stateDir, "publications"),
7274
workspacesDir: join(stateDir, "workspaces"),
7375
attemptsDir: join(stateDir, "attempts"),
7476
policyPath: resolve(env.COVEN_GITHUB_POLICY_PATH || join(rootDir, "coven-github-policy.json")),
@@ -84,7 +86,7 @@ export function createConfig(env: NodeJS.ProcessEnv = process.env, rootDir = pro
8486
demoMode: ["1", "true", "yes"].includes((env.COVEN_GITHUB_DEMO_MODE || "").trim().toLowerCase()),
8587
};
8688

87-
for (const directory of [config.deliveriesDir, config.tasksDir, config.workspacesDir, config.attemptsDir]) {
89+
for (const directory of [config.deliveriesDir, config.tasksDir, config.publicationsDir, config.workspacesDir, config.attemptsDir]) {
8890
mkdirSync(directory, {recursive: true});
8991
}
9092
return config;
@@ -885,7 +887,7 @@ async function runTask(config: AdapterConfig, taskId: string): Promise<JsonObjec
885887
task.result_path = finalCycle.result_path;
886888
task.state = [0, 1, 3].includes(finalCycle.run.returncode) ? "completed" : "failed";
887889
task.updated_at = utcNow();
888-
await publishResultIfConfigured(task, String(finalCycle.result_path), token);
890+
await publishResultIfConfigured(config, task, String(finalCycle.result_path), token);
889891
writeJsonAtomic(path, task);
890892
return task;
891893
} catch (error) {
@@ -1054,16 +1056,145 @@ function reviewEvidence(reviewContext: JsonObject, reviewContextPath: string, ta
10541056
workspace_head_sha: checkout.workspace_head_sha,
10551057
changed_file_count: files.length,
10561058
changed_files: files.map((file) => file.filename).filter((file): file is JsonValue => file !== undefined),
1059+
changed_file_lines: files.map((file) => ({path: file.filename, right_lines: patchRightLines(String(file.patch || ""))})),
10571060
review_context_path: reviewContextPath,
10581061
review_context_sha256: fileSha256(reviewContextPath),
10591062
};
10601063
}
10611064

1065+
function patchRightLines(patch: string): number[] {
1066+
const lines: number[] = [];
1067+
let rightLine: number | null = null;
1068+
for (const line of patch.split("\n")) {
1069+
const hunk = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
1070+
if (hunk) {
1071+
rightLine = Number(hunk[1]);
1072+
continue;
1073+
}
1074+
if (rightLine === null || line.startsWith("\\")) continue;
1075+
if (line.startsWith("+")) {
1076+
lines.push(rightLine);
1077+
rightLine += 1;
1078+
} else if (!line.startsWith("-")) {
1079+
rightLine += 1;
1080+
}
1081+
}
1082+
return lines;
1083+
}
1084+
10621085
function fileSha256(path: string): string {
10631086
return sha256(readFileSync(path));
10641087
}
10651088

1066-
async function publishResultIfConfigured(task: JsonObject, resultPath: string, token: string): Promise<void> {
1089+
interface NormalizedReviewPublication {
1090+
review: JsonObject;
1091+
decision: "APPROVE" | "REQUEST_CHANGES" | "COMMENT";
1092+
inlineComments: JsonObject[];
1093+
validationIssues: string[];
1094+
evidenceComplete: boolean;
1095+
}
1096+
1097+
function publicationRecordPath(config: AdapterConfig, repo: string, prNumber: number): string {
1098+
return join(config.publicationsDir, `${sha256(`${repo}#${prNumber}`).slice(0, 24)}.json`);
1099+
}
1100+
1101+
function publicationIdentity(task: JsonObject, result: JsonObject): string {
1102+
const evidence = (task.review_evidence as JsonObject | undefined) || {};
1103+
return sha256(stableCompactStringify({
1104+
task_id: task.task_id,
1105+
head_sha: evidence.head_sha,
1106+
result,
1107+
}));
1108+
}
1109+
1110+
function repositoryPath(value: JsonValue | undefined): string | null {
1111+
const path = String(value || "").trim();
1112+
if (!path || path.startsWith("/") || path.includes("\\") || path.split("/").some((part) => !part || part === "." || part === "..")) {
1113+
return null;
1114+
}
1115+
return path;
1116+
}
1117+
1118+
function actionableFinding(finding: JsonObject): boolean {
1119+
return !["info", "informational", "nit", "note"].includes(String(finding.severity || "").toLowerCase()) && Boolean(finding.title || finding.body || finding.recommendation);
1120+
}
1121+
1122+
export function normalizeReviewPublication(task: JsonObject, result: JsonObject): NormalizedReviewPublication {
1123+
const review = {...((result.review as JsonObject | undefined) || {})};
1124+
const evidence = (task.review_evidence as JsonObject | undefined) || {};
1125+
const changedFiles = new Set((Array.isArray(evidence.changed_files) ? evidence.changed_files : [])
1126+
.map((path) => repositoryPath(path))
1127+
.filter((path): path is string => path !== null));
1128+
const changedLines = new Map<string, Set<number>>();
1129+
for (const item of (Array.isArray(evidence.changed_file_lines) ? evidence.changed_file_lines : [])) {
1130+
if (!item || typeof item !== "object" || Array.isArray(item)) continue;
1131+
const file = repositoryPath((item as JsonObject).path);
1132+
const lines = (item as JsonObject).right_lines;
1133+
if (file && Array.isArray(lines)) changedLines.set(file, new Set(lines.map(Number).filter((line) => Number.isInteger(line) && line > 0)));
1134+
}
1135+
const validationIssues: string[] = [];
1136+
const reviewedFiles = (Array.isArray(review.reviewed_files) ? review.reviewed_files : [])
1137+
.map((path) => repositoryPath(path));
1138+
const suppliedInspected = (Array.isArray(result.files_inspected) ? result.files_inspected : [])
1139+
.map((path) => repositoryPath(path));
1140+
const validReviewedFiles = reviewedFiles.filter((path): path is string => path !== null && changedFiles.has(path));
1141+
const invalidReviewedFiles = reviewedFiles.filter((path) => path === null || !changedFiles.has(path));
1142+
if (review.mode !== "pull_request") validationIssues.push("result did not declare pull_request review mode");
1143+
if (review.evidence_status !== "complete") validationIssues.push("review evidence was not marked complete");
1144+
if (!String(evidence.head_sha || "").trim()) validationIssues.push("PR head revision is missing");
1145+
if (!changedFiles.size) validationIssues.push("no changed-file evidence was captured");
1146+
if (!reviewedFiles.length) validationIssues.push("no reviewed files were reported");
1147+
if (invalidReviewedFiles.length) validationIssues.push("reviewed files are not all changed files in the captured PR revision");
1148+
if (suppliedInspected.some((path) => path === null || !changedFiles.has(path))) validationIssues.push("files_inspected contains paths outside the captured PR diff");
1149+
if (suppliedInspected.length && suppliedInspected.filter((path): path is string => path !== null).some((path) => !validReviewedFiles.includes(path))) {
1150+
validationIssues.push("files_inspected and reviewed_files disagree");
1151+
}
1152+
1153+
const testsRun = Array.isArray(review.tests_run) ? (review.tests_run as JsonObject[]) : [];
1154+
for (const test of testsRun) {
1155+
const command = String(test.command || "").trim();
1156+
const status = String(test.status || "").toLowerCase();
1157+
const output = String(test.output_summary || "").trim();
1158+
const narrative = `${String(result.summary || "")}\n${String(result.pr_body || "")}`.toLowerCase();
1159+
if (status === "passed" && (!command || !output || output.toLowerCase().includes("not run") || narrative.includes(`${command.toLowerCase()} - not run`))) {
1160+
validationIssues.push(`test evidence for ${command || "an unnamed command"} is contradictory or incomplete`);
1161+
}
1162+
}
1163+
1164+
const findings = Array.isArray(review.findings) ? (review.findings as JsonObject[]) : [];
1165+
const inlineComments: JsonObject[] = [];
1166+
for (const finding of findings) {
1167+
const path = repositoryPath(finding.file);
1168+
const line = Number(finding.line);
1169+
if (path && changedFiles.has(path) && changedLines.get(path)?.has(line) && Number.isInteger(line) && line > 0 && actionableFinding(finding)) {
1170+
inlineComments.push({
1171+
path,
1172+
line,
1173+
side: String(finding.side || "RIGHT").toUpperCase() === "LEFT" ? "LEFT" : "RIGHT",
1174+
body: findingCommentBody(finding),
1175+
});
1176+
}
1177+
}
1178+
1179+
const evidenceComplete = validationIssues.length === 0;
1180+
const actionable = findings.some(actionableFinding);
1181+
return {
1182+
review,
1183+
decision: evidenceComplete && !findings.length ? "APPROVE" : evidenceComplete && actionable ? "REQUEST_CHANGES" : "COMMENT",
1184+
inlineComments,
1185+
validationIssues,
1186+
evidenceComplete,
1187+
};
1188+
}
1189+
1190+
function findingCommentBody(finding: JsonObject): string {
1191+
const parts = [`**${String(finding.severity || "finding")}**: ${String(finding.title || "Untitled finding")}`];
1192+
if (finding.body) parts.push("", String(finding.body));
1193+
if (finding.recommendation) parts.push("", `Suggested resolution: ${String(finding.recommendation)}`);
1194+
return parts.join("\n").slice(0, 6000);
1195+
}
1196+
1197+
export async function publishResultIfConfigured(config: AdapterConfig, task: JsonObject, resultPath: string, token: string): Promise<void> {
10671198
const publication = (task.publication as JsonObject | undefined) || {};
10681199
const mode = publication.mode || "record_only";
10691200
if (mode !== "comment") {
@@ -1073,17 +1204,52 @@ async function publishResultIfConfigured(task: JsonObject, resultPath: string, t
10731204

10741205
const result = readJson<JsonObject>(resultPath, {});
10751206
const taskData = (task.task as JsonObject | undefined) || {};
1076-
const number = taskData.issue_number || taskData.pr_number;
1207+
const prNumber = prNumberForTask(task);
1208+
const number = taskData.issue_number || taskData.pr_number || prNumber;
10771209
if (!number) {
10781210
task.publication_state = "publication_skipped_no_issue_or_pr_number";
10791211
return;
10801212
}
10811213

1082-
const body = publicationCommentBody(task, result);
10831214
const repo = String(task.repository);
1215+
const identity = publicationIdentity(task, result);
10841216
try {
1085-
const response = (await githubRequest("POST", `https://api.github.com/repos/${repo}/issues/${Number(number)}/comments`, token, {body})) as JsonObject;
1086-
task.publication_state = "published_comment";
1217+
if (prNumber && Object.keys((result.review as JsonObject | undefined) || {}).length) {
1218+
if (task.publication_identity === identity && task.publication_review_id) {
1219+
task.publication_state = "publication_skipped_duplicate";
1220+
return;
1221+
}
1222+
const recordPath = publicationRecordPath(config, repo, prNumber);
1223+
const previous = readJson<JsonObject>(recordPath, {});
1224+
const normalized = normalizeReviewPublication(task, result);
1225+
const body = publicationReviewBody(task, result, normalized, previous);
1226+
const reviewPayload: JsonObject = {event: normalized.decision, body};
1227+
if (normalized.inlineComments.length) reviewPayload.comments = normalized.inlineComments;
1228+
let response: JsonObject;
1229+
try {
1230+
response = (await githubRequest("POST", `https://api.github.com/repos/${repo}/pulls/${prNumber}/reviews`, token, reviewPayload)) as JsonObject;
1231+
} catch (error) {
1232+
if (!normalized.inlineComments.length) throw error;
1233+
response = (await githubRequest("POST", `https://api.github.com/repos/${repo}/pulls/${prNumber}/reviews`, token, {event: normalized.decision, body: `${body}\n\n_Inline publication was unavailable; findings are included above._`})) as JsonObject;
1234+
normalized.validationIssues.push("inline finding locations were rejected by GitHub; findings were retained in the review body");
1235+
}
1236+
task.publication_state = "published_review";
1237+
task.publication_identity = identity;
1238+
task.publication_review_id = response.id;
1239+
task.publication_url = response.html_url;
1240+
task.publication_decision = normalized.decision;
1241+
writeJsonAtomic(recordPath, {identity, review_id: response.id, review_url: response.html_url, task_id: task.task_id, head_sha: ((task.review_evidence as JsonObject | undefined) || {}).head_sha, published_at: utcNow()});
1242+
return;
1243+
}
1244+
1245+
const body = publicationCommentBody(task, result, "Coven task result");
1246+
const method = task.publication_comment_id ? "PATCH" : "POST";
1247+
const url = task.publication_comment_id
1248+
? `https://api.github.com/repos/${repo}/issues/comments/${Number(task.publication_comment_id)}`
1249+
: `https://api.github.com/repos/${repo}/issues/${Number(number)}/comments`;
1250+
const response = (await githubRequest(method, url, token, {body})) as JsonObject;
1251+
task.publication_state = method === "PATCH" ? "updated_comment" : "published_comment";
1252+
task.publication_identity = identity;
10871253
task.publication_url = response.html_url;
10881254
task.publication_comment_id = response.id;
10891255
} catch (error) {
@@ -1092,7 +1258,16 @@ async function publishResultIfConfigured(task: JsonObject, resultPath: string, t
10921258
}
10931259
}
10941260

1095-
function publicationCommentBody(task: JsonObject, result: JsonObject): string {
1261+
function publicationReviewBody(task: JsonObject, result: JsonObject, normalized: NormalizedReviewPublication, previous: JsonObject): string {
1262+
const body = publicationCommentBody(task, result, "Coven review");
1263+
const additions: string[] = [];
1264+
if (previous.review_url && previous.identity !== task.publication_identity) additions.push(`This review supersedes [the prior covencat publication](${String(previous.review_url)}).`);
1265+
if (normalized.validationIssues.length) additions.push(`### Publication validation\n- ${normalized.validationIssues.join("\n- ")}\n\nEvidence was incomplete or contradictory, so this is a COMMENT review rather than an approval or change request.`);
1266+
if (normalized.review.findings && Array.isArray(normalized.review.findings) && normalized.inlineComments.length < normalized.review.findings.length) additions.push("### Findings without valid inline locations\nThe structured findings above remain part of this review body because their file/line locations could not be safely attached to the current diff.");
1267+
return [body, ...additions].join("\n\n");
1268+
}
1269+
1270+
function publicationCommentBody(task: JsonObject, result: JsonObject, heading = "Coven task result"): string {
10961271
const status = result.status || "unknown";
10971272
const summary = String(result.summary || "No summary returned.");
10981273
const prBody = String(result.pr_body || "");
@@ -1101,7 +1276,7 @@ function publicationCommentBody(task: JsonObject, result: JsonObject): string {
11011276
const taskId = String(task.task_id || "");
11021277
const evidence = (task.review_evidence as JsonObject | undefined) || {};
11031278
const review = (result.review as JsonObject | undefined) || {};
1104-
const parts = ["## Cody dogfood result", "", `**Status:** ${status}`, "", summary.trim()];
1279+
const parts = [`## ${heading}`, "", `**Status:** ${status}`, "", summary.trim()];
11051280
if (prBody.trim() && prBody.trim() !== summary.trim()) {
11061281
parts.push("", prBody.trim());
11071282
}
@@ -1122,13 +1297,13 @@ function publicationCommentBody(task: JsonObject, result: JsonObject): string {
11221297
} else {
11231298
parts.push("- No PR review evidence was captured for this run.");
11241299
}
1125-
parts.push(...structuredReviewLines(review));
1300+
parts.push(...structuredReviewLines(review, task));
11261301
parts.push(
11271302
"",
11281303
`**Files changed:** ${filesChanged.length}`,
11291304
`**Commits:** ${commits.length}`,
11301305
"",
1131-
`_Task \`${taskId}\`. Publication is enabled on the hosted test adapter only._`,
1306+
`_Task \`${taskId}\`._`,
11321307
);
11331308
return parts.join("\n");
11341309
}
@@ -1148,7 +1323,19 @@ function reviewFixLoopLines(task: JsonObject): string[] {
11481323
return lines;
11491324
}
11501325

1151-
function structuredReviewLines(review: JsonObject): string[] {
1326+
function githubFileMarkdown(task: JsonObject, raw: JsonValue): string {
1327+
const match = String(raw).match(/^(.*?)(?::(\d+))?$/);
1328+
const path = repositoryPath(match?.[1]);
1329+
const evidence = (task.review_evidence as JsonObject | undefined) || {};
1330+
const ref = String(evidence.head_sha || "").trim();
1331+
const repo = String(task.repository || "").trim();
1332+
if (!path || !ref || !repo || !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repo)) return `\`${String(raw)}\``;
1333+
const encodedPath = path.split("/").map(encodeURIComponent).join("/");
1334+
const anchor = match?.[2] ? `#L${match[2]}` : "";
1335+
return `[\`${String(raw)}\`](https://github.com/${repo}/blob/${encodeURIComponent(ref)}/${encodedPath}${anchor})`;
1336+
}
1337+
1338+
function structuredReviewLines(review: JsonObject, task?: JsonObject): string[] {
11521339
if (!Object.keys(review).length) {
11531340
return ["", "### Structured review", "- No structured review result was emitted."];
11541341
}
@@ -1162,15 +1349,15 @@ function structuredReviewLines(review: JsonObject): string[] {
11621349
const reviewedFiles = Array.isArray(review.reviewed_files) ? review.reviewed_files : [];
11631350
lines.push(`- Reviewed files: ${reviewedFiles.length}`);
11641351
if (reviewedFiles.length) {
1165-
lines.push(`- Reviewed file list: ${reviewedFiles.slice(0, 20).map((path) => `\`${path}\``).join(", ")}`);
1352+
lines.push(`- Reviewed file list: ${reviewedFiles.slice(0, 20).map((path) => task ? githubFileMarkdown(task, path) : `\`${path}\``).join(", ")}`);
11661353
if (reviewedFiles.length > 20) {
11671354
lines.push("- Reviewed file list truncated after 20 entries.");
11681355
}
11691356
}
11701357
const supportingFiles = Array.isArray(review.supporting_files) ? review.supporting_files : [];
11711358
lines.push(`- Supporting files inspected: ${supportingFiles.length}`);
11721359
if (supportingFiles.length) {
1173-
lines.push(`- Supporting file list: ${supportingFiles.slice(0, 20).map((path) => `\`${path}\``).join(", ")}`);
1360+
lines.push(`- Supporting file list: ${supportingFiles.slice(0, 20).map((path) => task ? githubFileMarkdown(task, path) : `\`${path}\``).join(", ")}`);
11741361
if (supportingFiles.length > 20) {
11751362
lines.push("- Supporting file list truncated after 20 entries.");
11761363
}

0 commit comments

Comments
 (0)