Skip to content

Commit cd01cdc

Browse files
committed
Merge remote-tracking branch 'origin/main' into codex/aiauditbridge-single-reviewer-cleanup-20260729
Co-Authored-By: Codex <noreply@openai.com> # Conflicts: # .github/codex_auto_merge_policy.json
2 parents 816d00e + 068a034 commit cd01cdc

3 files changed

Lines changed: 153 additions & 2 deletions

File tree

scripts/gate_codex_app_review_static.py

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import json
99
import re
10-
from pathlib import Path
10+
from pathlib import Path, PurePosixPath
1111
from typing import Any
1212

1313
DEFAULT_POLICY_PATH = Path(".github/codex_auto_merge_policy.json")
@@ -70,16 +70,72 @@ def scan_diff(diff_text: str, path_patterns: list[re.Pattern[str]]) -> list[str]
7070
return list(dict.fromkeys(violations))
7171

7272

73+
def _safe_exact_paths(values: Any) -> set[str] | None:
74+
if not isinstance(values, list) or not values:
75+
return None
76+
paths: set[str] = set()
77+
for value in values:
78+
if not isinstance(value, str) or value != value.strip():
79+
return None
80+
path = PurePosixPath(value)
81+
if (
82+
not value
83+
or path.is_absolute()
84+
or path.as_posix() != value
85+
or ".." in path.parts
86+
or "\\" in value
87+
or any(character in value for character in "*?[]")
88+
or value in paths
89+
):
90+
return None
91+
paths.add(value)
92+
return paths
93+
94+
7395
def check_metadata(files: list[dict[str, Any]], policy: dict[str, Any]) -> list[str]:
7496
issues: list[str] = []
97+
approved_deleted_paths: set[str] = set()
7598
max_files = policy.get("max_changed_files", 50)
7699
max_lines = policy.get("max_changed_lines", 5000)
100+
changed_paths = {
101+
filename
102+
for f in files
103+
if isinstance((filename := f.get("filename")), str)
104+
}
105+
removed_paths = {
106+
f["filename"]
107+
for f in files
108+
if isinstance(f.get("filename"), str)
109+
and (f.get("status") or "").lower().strip() == "removed"
110+
}
111+
configured_bundles = policy.get("approved_change_bundles", [])
112+
if isinstance(configured_bundles, list):
113+
for bundle in configured_bundles:
114+
if not isinstance(bundle, dict):
115+
continue
116+
exact_changed_paths = _safe_exact_paths(bundle.get("exact_changed_paths"))
117+
exact_deleted_paths = _safe_exact_paths(bundle.get("exact_deleted_paths"))
118+
bundle_max_lines = bundle.get("max_changed_lines")
119+
if (
120+
exact_changed_paths is None
121+
or exact_deleted_paths is None
122+
or not exact_deleted_paths.issubset(exact_changed_paths)
123+
or type(bundle_max_lines) is not int
124+
or bundle_max_lines < max_lines
125+
or changed_paths != exact_changed_paths
126+
or removed_paths != exact_deleted_paths
127+
):
128+
continue
129+
approved_deleted_paths = exact_deleted_paths
130+
max_lines = bundle_max_lines
131+
break
132+
77133
total_added = sum(f.get("additions", 0) or 0 for f in files)
78134
total_deleted = sum(f.get("deletions", 0) or 0 for f in files)
79135
for f in files:
80136
filename = f.get("filename", "?")
81137
status = (f.get("status") or "").lower().strip()
82-
if status == "removed":
138+
if status == "removed" and filename not in approved_deleted_paths:
83139
issues.append(f"**File deleted**: `{filename}` — verify intentional")
84140
elif status == "renamed":
85141
issues.append(f"**File renamed**: `{f.get('previous_filename', '?')}` → `{filename}`")

tests/test_gate_codex_app_review.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,99 @@ def test_collect_static_gate_issues_aggregates_metadata_and_diff(self) -> None:
107107
self.assertTrue(any("Hardcoded secret" in issue for issue in issues))
108108
self.assertTrue(any("Too many lines" in issue for issue in issues))
109109

110+
def test_check_metadata_allows_only_an_exact_base_approved_change_bundle(self) -> None:
111+
files = [
112+
{
113+
"filename": "scripts/retired.py",
114+
"status": "removed",
115+
"additions": 0,
116+
"deletions": 150,
117+
},
118+
{
119+
"filename": "service/caller.py",
120+
"status": "modified",
121+
"additions": 2_400,
122+
"deletions": 0,
123+
},
124+
]
125+
policy = {
126+
"max_changed_files": 10,
127+
"max_changed_lines": 2_000,
128+
"approved_change_bundles": [
129+
{
130+
"exact_changed_paths": [
131+
"scripts/retired.py",
132+
"service/caller.py",
133+
],
134+
"exact_deleted_paths": ["scripts/retired.py"],
135+
"max_changed_lines": 3_000,
136+
}
137+
],
138+
}
139+
140+
issues = gate_codex_app_review_static.check_metadata(files, policy)
141+
142+
self.assertEqual(issues, [])
143+
144+
def test_check_metadata_does_not_expand_line_budget_for_an_unrelated_change(self) -> None:
145+
files = [
146+
{
147+
"filename": "service/unrelated.py",
148+
"status": "modified",
149+
"additions": 2_500,
150+
"deletions": 0,
151+
},
152+
]
153+
policy = {
154+
"max_changed_files": 10,
155+
"max_changed_lines": 2_000,
156+
"approved_change_bundles": [
157+
{
158+
"exact_changed_paths": [
159+
"scripts/retired.py",
160+
"service/caller.py",
161+
],
162+
"exact_deleted_paths": ["scripts/retired.py"],
163+
"max_changed_lines": 3_000,
164+
}
165+
],
166+
}
167+
168+
issues = gate_codex_app_review_static.check_metadata(files, policy)
169+
170+
self.assertTrue(any("Too many lines" in issue for issue in issues))
171+
172+
def test_check_metadata_rejects_partial_or_unsafe_change_bundles(self) -> None:
173+
files = [
174+
{
175+
"filename": "scripts/retired.py",
176+
"status": "removed",
177+
"additions": 0,
178+
"deletions": 10,
179+
},
180+
]
181+
for exact_changed_paths in (
182+
["scripts/retired.py", "service/caller.py"],
183+
["scripts/*.py"],
184+
["../scripts/retired.py"],
185+
["/scripts/retired.py"],
186+
["scripts\\retired.py"],
187+
):
188+
with self.subTest(exact_changed_paths=exact_changed_paths):
189+
policy = {
190+
"max_changed_files": 10,
191+
"max_changed_lines": 100,
192+
"approved_change_bundles": [
193+
{
194+
"exact_changed_paths": exact_changed_paths,
195+
"exact_deleted_paths": ["scripts/retired.py"],
196+
"max_changed_lines": 200,
197+
}
198+
],
199+
}
200+
issues = gate_codex_app_review_static.check_metadata(files, policy)
201+
self.assertTrue(any("scripts/retired.py" in issue for issue in issues))
202+
110203

111204
if __name__ == "__main__":
112205
unittest.main()

tests/test_single_pr_reviewer_contract.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ def test_retired_pr_reviewer_is_not_advertised_as_active() -> None:
4343
(ROOT / ".github/codex_auto_merge_policy.json").read_text(encoding="utf-8")
4444
)
4545
assert "pr_review" not in policy
46+
assert "approved_change_bundles" not in policy
47+
assert policy["max_changed_lines"] == 2_000
4648
assert "Codex PR Review" not in DEFAULT_WORKFLOW_ALLOWLIST
4749

4850
for relative_path in (

0 commit comments

Comments
 (0)