Skip to content

Commit 8e0690e

Browse files
committed
fix: tighten agent governance diff scope
1 parent c0dc966 commit 8e0690e

3 files changed

Lines changed: 103 additions & 14 deletions

File tree

.github/workflows/agent_governance.yml

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,21 @@ jobs:
2222
id: governance
2323
run: |
2424
set +e
25+
base_sha="${{ github.event.pull_request.base.sha }}"
26+
head_sha="${{ github.event.pull_request.head.sha }}"
27+
merge_base="$(git merge-base "$base_sha" "$head_sha")"
28+
merge_base_status=$?
29+
if [ "$merge_base_status" -ne 0 ]; then
30+
{
31+
echo "## Agent Governance Check"
32+
echo
33+
echo "Failed to compute the pull request merge base."
34+
} > agent_governance_summary.md
35+
exit "$merge_base_status"
36+
fi
2537
python3 tools/03_code_analysis/agent_governance_check.py \
26-
--base "${{ github.event.pull_request.base.sha }}" \
27-
--head "${{ github.event.pull_request.head.sha }}" \
38+
--base "$merge_base" \
39+
--head "$head_sha" \
2840
--event-path "$GITHUB_EVENT_PATH" \
2941
--format markdown | tee agent_governance_summary.md
3042
status=${PIPESTATUS[0]}

tools/03_code_analysis/agent_governance_check.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,9 @@ def parse_added_lines(diff_text: str) -> List[DiffLine]:
150150

151151
def added_lines(root: Path, args: argparse.Namespace) -> List[DiffLine]:
152152
if args.staged:
153-
output = git(["diff", "--cached", "-U0"], root).stdout
153+
output = git(["diff", "--cached", "--ignore-cr-at-eol", "-U0"], root).stdout
154154
elif args.base and args.head:
155-
output = git(["diff", "-U0", args.base, args.head], root).stdout
155+
output = git(["diff", "--ignore-cr-at-eol", "-U0", args.base, args.head], root).stdout
156156
else:
157157
output = ""
158158
return parse_added_lines(output)
@@ -436,16 +436,20 @@ def check_input_parameter_docs(
436436
)
437437

438438

439-
def read_pr_body(event_path: Optional[str]) -> str:
439+
def read_pr_body(event_path: Optional[str]) -> Optional[str]:
440440
if not event_path:
441-
return ""
441+
return None
442442
try:
443443
with open(event_path, "r", encoding="utf-8") as handle:
444444
payload = json.load(handle)
445445
except (OSError, json.JSONDecodeError):
446+
return None
447+
if "pull_request" not in payload or not isinstance(payload["pull_request"], dict):
448+
return None
449+
body = payload["pull_request"].get("body")
450+
if body is None:
446451
return ""
447-
pr = payload.get("pull_request") or {}
448-
return pr.get("body") or ""
452+
return str(body)
449453

450454

451455
def pr_sections(body: str) -> Dict[str, str]:
@@ -482,8 +486,8 @@ def section_is_placeholder(content: str) -> bool:
482486
return not meaningful
483487

484488

485-
def check_pr_metadata(findings: List[Finding], body: str) -> None:
486-
if not body:
489+
def check_pr_metadata(findings: List[Finding], body: Optional[str]) -> None:
490+
if body is None:
487491
return
488492
required_sections = [
489493
"Linked Issue",
@@ -615,18 +619,19 @@ def collect_findings(root: Path, args: argparse.Namespace) -> List[Finding]:
615619
statuses, changed = changed_paths(root, args)
616620
lines = added_lines(root, args)
617621
body = read_pr_body(args.event_path)
622+
body_text = body or ""
618623

619624
check_line_endings(findings, root, changed, statuses, args)
620625
check_global_dependencies(findings, lines)
621626
check_default_parameters(findings, lines)
622627
check_hpp_warnings(findings, statuses, lines)
623628
check_header_include_warnings(findings, lines)
624629
check_cmake_linkage(findings, statuses, changed)
625-
check_input_parameter_docs(findings, changed, statuses, lines, body)
630+
check_input_parameter_docs(findings, changed, statuses, lines, body_text)
626631
check_pr_metadata(findings, body)
627-
check_test_evidence_warning(findings, changed, body)
628-
check_heterogeneous_test_warning(findings, changed, body)
629-
check_documentation_warning(findings, changed, body)
632+
check_test_evidence_warning(findings, changed, body_text)
633+
check_heterogeneous_test_warning(findings, changed, body_text)
634+
check_documentation_warning(findings, changed, body_text)
630635
return findings
631636

632637

tools/03_code_analysis/test_agent_governance_check.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,31 @@ def test_blocks_default_parameters_added_to_headers(self):
118118

119119
self.assert_blocked_by(result, "No new default parameters")
120120

121+
def test_ignores_crlf_to_lf_only_changes_for_semantic_added_lines(self):
122+
self.write("source/source_base/defaults.h", b"void update_solver(int step = 0);\r\n", mode="wb")
123+
self.git("add", ".")
124+
self.git("commit", "-m", "add crlf header")
125+
base = self.git("rev-parse", "HEAD").stdout.strip()
126+
self.write("source/source_base/defaults.h", "void update_solver(int step = 0);\n")
127+
head = self.commit_change()
128+
129+
result = self.run_checker("--base", base, "--head", head)
130+
131+
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
132+
self.assertNotIn("No new default parameters", result.stdout)
133+
134+
def test_staged_mode_ignores_crlf_to_lf_only_semantic_added_lines(self):
135+
self.write("source/source_base/defaults.h", b"void update_solver(int step = 0);\r\n", mode="wb")
136+
self.git("add", ".")
137+
self.git("commit", "-m", "add crlf header")
138+
self.write("source/source_base/defaults.h", "void update_solver(int step = 0);\n")
139+
self.git("add", ".")
140+
141+
result = self.run_checker("--staged")
142+
143+
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
144+
self.assertNotIn("No new default parameters", result.stdout)
145+
121146
def test_allows_for_loop_initializer_in_header(self):
122147
self.write(
123148
"source/source_base/loop_header.h",
@@ -232,6 +257,33 @@ def test_blocks_unfilled_pr_template_fields_from_event_payload(self):
232257

233258
self.assert_blocked_by(result, "PR metadata completeness")
234259

260+
def test_blocks_empty_pr_template_from_event_payload(self):
261+
for body in ("", None):
262+
with self.subTest(body=body):
263+
event = self.repo / "event.json"
264+
event.write_text(json.dumps({"pull_request": {"body": body}}))
265+
266+
result = self.run_checker("--event-path", str(event))
267+
268+
self.assert_blocked_by(result, "PR metadata completeness")
269+
270+
def test_blocks_missing_pr_body_from_event_payload(self):
271+
event = self.repo / "event.json"
272+
event.write_text(json.dumps({"pull_request": {}}))
273+
274+
result = self.run_checker("--event-path", str(event))
275+
276+
self.assert_blocked_by(result, "PR metadata completeness")
277+
278+
def test_skips_pr_metadata_when_event_payload_is_not_a_pull_request(self):
279+
event = self.repo / "event.json"
280+
event.write_text(json.dumps({"workflow_run": {"name": "Agent Governance"}}))
281+
282+
result = self.run_checker("--event-path", str(event))
283+
284+
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
285+
self.assertNotIn("PR metadata completeness", result.stdout)
286+
235287
def test_accepts_filled_pr_template_fields_from_event_payload(self):
236288
event = self.repo / "event.json"
237289
event.write_text(
@@ -469,6 +521,26 @@ def test_warns_for_new_header_include(self):
469521

470522
self.assert_warns_with_success(result, "Header dependency review")
471523

524+
def test_merge_base_scoped_comparison_excludes_base_branch_only_changes(self):
525+
self.write("source/source_base/api.h", "void update_solver(int step = 0);\n")
526+
self.git("add", ".")
527+
self.git("commit", "-m", "add legacy default")
528+
base_branch = self.git("branch", "--show-current").stdout.strip()
529+
merge_base = self.git("rev-parse", "HEAD").stdout.strip()
530+
self.git("checkout", "-b", "feature")
531+
self.write("docs/feature.md", "feature docs\n")
532+
head = self.commit_change()
533+
self.git("checkout", base_branch)
534+
self.write("source/source_base/api.h", "void update_solver(int step);\n")
535+
base_tip = self.commit_change()
536+
537+
base_tip_result = self.run_checker("--base", base_tip, "--head", head)
538+
merge_base_result = self.run_checker("--base", merge_base, "--head", head)
539+
540+
self.assertIn("No new default parameters", base_tip_result.stdout)
541+
self.assertNotIn("No new default parameters", merge_base_result.stdout)
542+
self.assertEqual(merge_base_result.returncode, 0, merge_base_result.stdout + merge_base_result.stderr)
543+
472544
def test_blocks_new_heterogeneous_file_without_cmake_linkage(self):
473545
self.write("source/module_hamilt/kernels/new_kernel.cu", "__global__ void k() {}\n")
474546
head = self.commit_change()

0 commit comments

Comments
 (0)