Skip to content

Commit f2c76d2

Browse files
fix deploy review file links
Signed-off-by: Timothy Wayne Gregg <Timothy.Gregg@complete.tech>
1 parent 63296ff commit f2c76d2

2 files changed

Lines changed: 258 additions & 12 deletions

File tree

deploy/coven-github/coven_github_adapter.py

Lines changed: 139 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from datetime import datetime, timezone
1212
from pathlib import Path
1313
from urllib.error import HTTPError
14+
from urllib.parse import quote
1415
from urllib.request import Request, urlopen
1516

1617

@@ -1263,8 +1264,8 @@ def publish_result_if_configured(task, result_path, token):
12631264

12641265
def publication_comment_body(task, result):
12651266
status = result.get("status") or "unknown"
1266-
summary = result.get("summary") or "No summary returned."
1267-
pr_body = result.get("pr_body") or ""
1267+
summary = link_github_file_mentions(task, result.get("summary") or "No summary returned.")
1268+
pr_body = link_github_file_mentions(task, result.get("pr_body") or "")
12681269
files_changed = result.get("files_changed") or []
12691270
commits = result.get("commits") or []
12701271
task_id = task.get("task_id") or ""
@@ -1295,10 +1296,14 @@ def publication_comment_body(task, result):
12951296
]
12961297
)
12971298
if changed_files:
1298-
parts.append("- Files: {}".format(", ".join("`{}`".format(f) for f in changed_files[:20])))
1299+
parts.append(
1300+
"- Files: {}".format(
1301+
", ".join(github_file_markdown(task, f) for f in changed_files[:20])
1302+
)
1303+
)
12991304
else:
13001305
parts.append("- No PR review evidence was captured for this run.")
1301-
parts.extend(structured_review_lines(review))
1306+
parts.extend(structured_review_lines(review, task))
13021307
parts.extend(
13031308
[
13041309
"",
@@ -1311,6 +1316,126 @@ def publication_comment_body(task, result):
13111316
return "\n".join(parts)
13121317

13131318

1319+
def github_blob_base(task):
1320+
repository = str(task.get("repository") or "").strip()
1321+
if not re.match(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$", repository):
1322+
return None
1323+
evidence = task.get("review_evidence") or {}
1324+
ref = str(
1325+
evidence.get("head_sha")
1326+
or evidence.get("workspace_head_sha")
1327+
or task.get("default_branch")
1328+
or ""
1329+
).strip()
1330+
if not ref:
1331+
return None
1332+
return "https://github.com/{}/blob/{}".format(repository, quote(ref, safe="/"))
1333+
1334+
1335+
def parse_repo_relative_path(raw_path):
1336+
raw = str(raw_path)
1337+
display = raw.strip()
1338+
if not display or display != raw or re.search(r"[\s<>\[\]]", display):
1339+
return None
1340+
if display.startswith(("/", "\\")) or re.match(r"^[A-Za-z]:[\\/]", display):
1341+
return None
1342+
1343+
path = display.replace("\\", "/")
1344+
line = None
1345+
end_line = None
1346+
line_match = re.match(r"^(.*):(\d+)(?:-(\d+))?$", path)
1347+
if line_match:
1348+
path = line_match.group(1)
1349+
line = int(line_match.group(2))
1350+
end_line = int(line_match.group(3)) if line_match.group(3) else None
1351+
if re.match(r"^[A-Za-z][A-Za-z0-9+.-]*:", path):
1352+
return None
1353+
while path.startswith("./"):
1354+
path = path[2:]
1355+
1356+
segments = path.split("/")
1357+
filename = segments[-1] if segments else ""
1358+
if (
1359+
not path
1360+
or "//" in path
1361+
or any(not segment or segment in {".", ".."} for segment in segments)
1362+
or not re.search(r"\.[A-Za-z][A-Za-z0-9._-]{0,15}$", filename)
1363+
):
1364+
return None
1365+
return {"display": display, "path": path, "line": line, "end_line": end_line}
1366+
1367+
1368+
def github_file_markdown(task, raw_path, line_override=None):
1369+
target = parse_repo_relative_path(raw_path)
1370+
base = github_blob_base(task)
1371+
if not target or not base:
1372+
return "`{}`".format(raw_path)
1373+
encoded_path = "/".join(quote(segment, safe="") for segment in target["path"].split("/"))
1374+
line = line_override if line_override is not None else target["line"]
1375+
end_line = None if line_override is not None else target["end_line"]
1376+
if isinstance(line, int) and line > 0:
1377+
line_anchor = "#L{}".format(line)
1378+
if isinstance(end_line, int) and end_line > line:
1379+
line_anchor = "{}-L{}".format(line_anchor, end_line)
1380+
else:
1381+
line_anchor = ""
1382+
return "[`{}`]({}/{}{})".format(target["display"], base, encoded_path, line_anchor)
1383+
1384+
1385+
def inline_code_with_github_file_links(task, raw_text):
1386+
raw_text = str(raw_text)
1387+
direct = github_file_markdown(task, raw_text)
1388+
if direct != "`{}`".format(raw_text):
1389+
return direct
1390+
1391+
linked_any = False
1392+
parts = []
1393+
for part in re.split(r"(\s+)", raw_text):
1394+
if not part or part.isspace():
1395+
parts.append(part)
1396+
continue
1397+
linked = github_file_markdown(task, part)
1398+
if linked != "`{}`".format(part):
1399+
linked_any = True
1400+
parts.append(linked)
1401+
else:
1402+
parts.append("`{}`".format(part))
1403+
1404+
return "".join(parts) if linked_any else "`{}`".format(raw_text)
1405+
1406+
1407+
def link_github_file_mentions(task, text):
1408+
fence_pattern = re.compile(r"(```[\s\S]*?```)")
1409+
segments = fence_pattern.split(str(text))
1410+
1411+
def replace_segment(segment):
1412+
def replace_match(match):
1413+
raw_path = match.group(1)
1414+
already_link_text = (
1415+
match.start() > 0
1416+
and segment[match.start() - 1] == "["
1417+
and segment[match.end() : match.end() + 2] == "]("
1418+
)
1419+
if already_link_text:
1420+
return match.group(0)
1421+
linked = inline_code_with_github_file_links(task, raw_path)
1422+
return match.group(0) if linked == "`{}`".format(raw_path) else linked
1423+
1424+
return re.sub(r"`([^`\n]+)`", replace_match, segment)
1425+
1426+
return "".join(
1427+
segment if index % 2 == 1 else replace_segment(segment)
1428+
for index, segment in enumerate(segments)
1429+
)
1430+
1431+
1432+
def review_test_command_markdown(task, raw_command):
1433+
command = str(raw_command).strip()
1434+
if not command:
1435+
return "`unknown command`"
1436+
return inline_code_with_github_file_links(task, command)
1437+
1438+
13141439
def review_fix_loop_lines(task):
13151440
loops = task.get("review_fix_loops") or []
13161441
if not loops:
@@ -1335,7 +1460,7 @@ def review_fix_loop_lines(task):
13351460
return lines
13361461

13371462

1338-
def structured_review_lines(review):
1463+
def structured_review_lines(review, task):
13391464
if not review:
13401465
return ["", "### Structured review", "- No structured review result was emitted."]
13411466

@@ -1351,7 +1476,7 @@ def structured_review_lines(review):
13511476
if reviewed_files:
13521477
lines.append(
13531478
"- Reviewed file list: {}".format(
1354-
", ".join("`{}`".format(path) for path in reviewed_files[:20])
1479+
", ".join(github_file_markdown(task, path) for path in reviewed_files[:20])
13551480
)
13561481
)
13571482
if len(reviewed_files) > 20:
@@ -1362,7 +1487,7 @@ def structured_review_lines(review):
13621487
if supporting_files:
13631488
lines.append(
13641489
"- Supporting file list: {}".format(
1365-
", ".join("`{}`".format(path) for path in supporting_files[:20])
1490+
", ".join(github_file_markdown(task, path) for path in supporting_files[:20])
13661491
)
13671492
)
13681493
if len(supporting_files) > 20:
@@ -1374,11 +1499,12 @@ def structured_review_lines(review):
13741499
location = finding.get("file") or "unknown file"
13751500
if finding.get("line") is not None:
13761501
location = "{}:{}".format(location, finding.get("line"))
1502+
linked_location = github_file_markdown(task, location, finding.get("line"))
13771503
lines.append(
13781504
" {}. `{}` {} - {}".format(
13791505
index,
13801506
finding.get("severity") or "unknown",
1381-
location,
1507+
linked_location,
13821508
finding.get("title") or "Untitled finding",
13831509
)
13841510
)
@@ -1387,16 +1513,17 @@ def structured_review_lines(review):
13871513

13881514
no_findings_reason = review.get("no_findings_reason")
13891515
if no_findings_reason:
1390-
lines.append("- No-findings reason: {}".format(no_findings_reason))
1516+
lines.append("- No-findings reason: {}".format(link_github_file_mentions(task, no_findings_reason)))
13911517

13921518
tests_run = review.get("tests_run") or []
13931519
lines.append("- Tests reported by runtime: {}".format(len(tests_run)))
13941520
for test in tests_run[:10]:
13951521
summary = test.get("output_summary")
1396-
suffix = " - {}".format(summary) if summary else ""
1522+
command = review_test_command_markdown(task, test.get("command") or "unknown command")
1523+
suffix = " - {}".format(link_github_file_mentions(task, summary)) if summary else ""
13971524
lines.append(
1398-
" - `{}`: `{}`{}".format(
1399-
test.get("command") or "unknown command",
1525+
" - {}: `{}`{}".format(
1526+
command,
14001527
test.get("status") or "unknown",
14011528
suffix,
14021529
)

deploy/coven-github/test_coven_github_adapter.py

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,125 @@ def fake_run_command(args, cwd=None, env=None, timeout=300):
7979
with self.assertRaisesRegex(RuntimeError, "does not match GitHub metadata head"):
8080
adapter.prepare_review_context(task, Path(tmp), "tok", {}, Path(tmp))
8181

82+
def test_publication_body_links_file_mentions(self):
83+
adapter = load_adapter()
84+
85+
body = adapter.publication_comment_body(
86+
{
87+
"task_id": "task-file-links",
88+
"repository": "OpenCoven/coven-github",
89+
"default_branch": "main",
90+
"review_evidence": {"head_sha": "abc123def456"},
91+
},
92+
{
93+
"status": "success",
94+
"summary": "\n".join(
95+
[
96+
"### Files inspected",
97+
"",
98+
"- `src/lib/server/skills-directory.ts`",
99+
"- `Read src/lib/server/skill-scan.ts`",
100+
"- `README.md:12`",
101+
"- `README.md:12-14`",
102+
"- `tests_run[].output_summary`",
103+
"- `npm test`",
104+
"",
105+
"```ts",
106+
"`src/not-linked-inside-fence.ts`",
107+
"```",
108+
]
109+
),
110+
"review": {},
111+
},
112+
)
113+
114+
self.assertIn(
115+
"[`src/lib/server/skills-directory.ts`](https://github.com/OpenCoven/coven-github/blob/abc123def456/src/lib/server/skills-directory.ts)",
116+
body,
117+
)
118+
self.assertIn(
119+
"`Read` [`src/lib/server/skill-scan.ts`](https://github.com/OpenCoven/coven-github/blob/abc123def456/src/lib/server/skill-scan.ts)",
120+
body,
121+
)
122+
self.assertIn(
123+
"[`README.md:12`](https://github.com/OpenCoven/coven-github/blob/abc123def456/README.md#L12)",
124+
body,
125+
)
126+
self.assertIn(
127+
"[`README.md:12-14`](https://github.com/OpenCoven/coven-github/blob/abc123def456/README.md#L12-L14)",
128+
body,
129+
)
130+
self.assertIn("- `tests_run[].output_summary`", body)
131+
self.assertNotIn("[`tests_run[].output_summary`]", body)
132+
self.assertIn("- `npm test`", body)
133+
self.assertIn("`src/not-linked-inside-fence.ts`", body)
134+
self.assertNotIn("[`src/not-linked-inside-fence.ts`]", body)
135+
136+
def test_publication_body_links_structured_review_files(self):
137+
adapter = load_adapter()
138+
139+
body = adapter.publication_comment_body(
140+
{
141+
"task_id": "task-structured-links",
142+
"repository": "OpenCoven/coven-github",
143+
"default_branch": "main",
144+
"review_evidence": {
145+
"head_sha": "feedface",
146+
"changed_files": ["src/app.ts"],
147+
"changed_file_count": 1,
148+
},
149+
},
150+
{
151+
"status": "success",
152+
"summary": "Done.",
153+
"review": {
154+
"mode": "pull_request",
155+
"evidence_status": "complete",
156+
"reviewed_files": ["src/app.ts"],
157+
"supporting_files": ["tests/app.test.ts"],
158+
"findings": [
159+
{
160+
"severity": "medium",
161+
"file": "src/app.ts",
162+
"line": 7,
163+
"title": "Example finding",
164+
}
165+
],
166+
"no_findings_reason": "Checked `tests/app.test.ts` with `npm test`.",
167+
"tests_run": [
168+
{
169+
"command": "Read src/app.ts",
170+
"status": "passed",
171+
"output_summary": "inspected `tests/app.test.ts` coverage.",
172+
},
173+
{
174+
"command": "npm test",
175+
"status": "passed",
176+
},
177+
],
178+
},
179+
},
180+
)
181+
182+
self.assertIn(
183+
"[`src/app.ts`](https://github.com/OpenCoven/coven-github/blob/feedface/src/app.ts)",
184+
body,
185+
)
186+
self.assertIn(
187+
"[`tests/app.test.ts`](https://github.com/OpenCoven/coven-github/blob/feedface/tests/app.test.ts)",
188+
body,
189+
)
190+
self.assertIn(
191+
"[`src/app.ts:7`](https://github.com/OpenCoven/coven-github/blob/feedface/src/app.ts#L7)",
192+
body,
193+
)
194+
self.assertIn(
195+
"`Read` [`src/app.ts`](https://github.com/OpenCoven/coven-github/blob/feedface/src/app.ts): `passed`",
196+
body,
197+
)
198+
self.assertIn("with `npm test`", body)
199+
self.assertIn("- `npm test`: `passed`", body)
200+
82201

83202
if __name__ == "__main__":
84203
unittest.main()

0 commit comments

Comments
 (0)