Skip to content

Commit a5fa7cf

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

2 files changed

Lines changed: 188 additions & 9 deletions

File tree

deploy/coven-github/coven_github_adapter.py

Lines changed: 98 additions & 9 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,89 @@ 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+
line_match = re.match(r"^(.*):(\d+)$", path)
1346+
if line_match:
1347+
path = line_match.group(1)
1348+
line = int(line_match.group(2))
1349+
if re.match(r"^[A-Za-z][A-Za-z0-9+.-]*:", path):
1350+
return None
1351+
while path.startswith("./"):
1352+
path = path[2:]
1353+
1354+
segments = path.split("/")
1355+
filename = segments[-1] if segments else ""
1356+
if (
1357+
not path
1358+
or "//" in path
1359+
or any(not segment or segment in {".", ".."} for segment in segments)
1360+
or not re.search(r"\.[A-Za-z][A-Za-z0-9._-]{0,15}$", filename)
1361+
):
1362+
return None
1363+
return {"display": display, "path": path, "line": line}
1364+
1365+
1366+
def github_file_markdown(task, raw_path, line_override=None):
1367+
target = parse_repo_relative_path(raw_path)
1368+
base = github_blob_base(task)
1369+
if not target or not base:
1370+
return "`{}`".format(raw_path)
1371+
encoded_path = "/".join(quote(segment, safe="") for segment in target["path"].split("/"))
1372+
line = line_override if line_override is not None else target["line"]
1373+
line_anchor = "#L{}".format(line) if isinstance(line, int) and line > 0 else ""
1374+
return "[`{}`]({}/{}{})".format(target["display"], base, encoded_path, line_anchor)
1375+
1376+
1377+
def link_github_file_mentions(task, text):
1378+
fence_pattern = re.compile(r"(```[\s\S]*?```)")
1379+
segments = fence_pattern.split(str(text))
1380+
1381+
def replace_segment(segment):
1382+
def replace_match(match):
1383+
raw_path = match.group(1)
1384+
already_link_text = (
1385+
match.start() > 0
1386+
and segment[match.start() - 1] == "["
1387+
and segment[match.end() : match.end() + 2] == "]("
1388+
)
1389+
if already_link_text:
1390+
return match.group(0)
1391+
linked = github_file_markdown(task, raw_path)
1392+
return match.group(0) if linked == "`{}`".format(raw_path) else linked
1393+
1394+
return re.sub(r"`([^`\n]+)`", replace_match, segment)
1395+
1396+
return "".join(
1397+
segment if index % 2 == 1 else replace_segment(segment)
1398+
for index, segment in enumerate(segments)
1399+
)
1400+
1401+
13141402
def review_fix_loop_lines(task):
13151403
loops = task.get("review_fix_loops") or []
13161404
if not loops:
@@ -1335,7 +1423,7 @@ def review_fix_loop_lines(task):
13351423
return lines
13361424

13371425

1338-
def structured_review_lines(review):
1426+
def structured_review_lines(review, task):
13391427
if not review:
13401428
return ["", "### Structured review", "- No structured review result was emitted."]
13411429

@@ -1351,7 +1439,7 @@ def structured_review_lines(review):
13511439
if reviewed_files:
13521440
lines.append(
13531441
"- Reviewed file list: {}".format(
1354-
", ".join("`{}`".format(path) for path in reviewed_files[:20])
1442+
", ".join(github_file_markdown(task, path) for path in reviewed_files[:20])
13551443
)
13561444
)
13571445
if len(reviewed_files) > 20:
@@ -1362,7 +1450,7 @@ def structured_review_lines(review):
13621450
if supporting_files:
13631451
lines.append(
13641452
"- Supporting file list: {}".format(
1365-
", ".join("`{}`".format(path) for path in supporting_files[:20])
1453+
", ".join(github_file_markdown(task, path) for path in supporting_files[:20])
13661454
)
13671455
)
13681456
if len(supporting_files) > 20:
@@ -1374,11 +1462,12 @@ def structured_review_lines(review):
13741462
location = finding.get("file") or "unknown file"
13751463
if finding.get("line") is not None:
13761464
location = "{}:{}".format(location, finding.get("line"))
1465+
linked_location = github_file_markdown(task, location, finding.get("line"))
13771466
lines.append(
13781467
" {}. `{}` {} - {}".format(
13791468
index,
13801469
finding.get("severity") or "unknown",
1381-
location,
1470+
linked_location,
13821471
finding.get("title") or "Untitled finding",
13831472
)
13841473
)
@@ -1387,7 +1476,7 @@ def structured_review_lines(review):
13871476

13881477
no_findings_reason = review.get("no_findings_reason")
13891478
if no_findings_reason:
1390-
lines.append("- No-findings reason: {}".format(no_findings_reason))
1479+
lines.append("- No-findings reason: {}".format(link_github_file_mentions(task, no_findings_reason)))
13911480

13921481
tests_run = review.get("tests_run") or []
13931482
lines.append("- Tests reported by runtime: {}".format(len(tests_run)))

deploy/coven-github/test_coven_github_adapter.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,96 @@ 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+
"- `README.md:12`",
100+
"- `npm test`",
101+
"",
102+
"```ts",
103+
"`src/not-linked-inside-fence.ts`",
104+
"```",
105+
]
106+
),
107+
"review": {},
108+
},
109+
)
110+
111+
self.assertIn(
112+
"[`src/lib/server/skills-directory.ts`](https://github.com/OpenCoven/coven-github/blob/abc123def456/src/lib/server/skills-directory.ts)",
113+
body,
114+
)
115+
self.assertIn(
116+
"[`README.md:12`](https://github.com/OpenCoven/coven-github/blob/abc123def456/README.md#L12)",
117+
body,
118+
)
119+
self.assertIn("- `npm test`", body)
120+
self.assertIn("`src/not-linked-inside-fence.ts`", body)
121+
self.assertNotIn("[`src/not-linked-inside-fence.ts`]", body)
122+
123+
def test_publication_body_links_structured_review_files(self):
124+
adapter = load_adapter()
125+
126+
body = adapter.publication_comment_body(
127+
{
128+
"task_id": "task-structured-links",
129+
"repository": "OpenCoven/coven-github",
130+
"default_branch": "main",
131+
"review_evidence": {
132+
"head_sha": "feedface",
133+
"changed_files": ["src/app.ts"],
134+
"changed_file_count": 1,
135+
},
136+
},
137+
{
138+
"status": "success",
139+
"summary": "Done.",
140+
"review": {
141+
"mode": "pull_request",
142+
"evidence_status": "complete",
143+
"reviewed_files": ["src/app.ts"],
144+
"supporting_files": ["tests/app.test.ts"],
145+
"findings": [
146+
{
147+
"severity": "medium",
148+
"file": "src/app.ts",
149+
"line": 7,
150+
"title": "Example finding",
151+
}
152+
],
153+
"no_findings_reason": "Checked `tests/app.test.ts` with `npm test`.",
154+
},
155+
},
156+
)
157+
158+
self.assertIn(
159+
"[`src/app.ts`](https://github.com/OpenCoven/coven-github/blob/feedface/src/app.ts)",
160+
body,
161+
)
162+
self.assertIn(
163+
"[`tests/app.test.ts`](https://github.com/OpenCoven/coven-github/blob/feedface/tests/app.test.ts)",
164+
body,
165+
)
166+
self.assertIn(
167+
"[`src/app.ts:7`](https://github.com/OpenCoven/coven-github/blob/feedface/src/app.ts#L7)",
168+
body,
169+
)
170+
self.assertIn("with `npm test`", body)
171+
82172

83173
if __name__ == "__main__":
84174
unittest.main()

0 commit comments

Comments
 (0)