Skip to content

Commit 068a034

Browse files
PigbibiPigbibi
authored
feat(gate): require base-approved exact cleanup bundles (#101)
Scope deletion and temporary line-budget exceptions to an exact protected-base cleanup transaction.
1 parent 5eeea47 commit 068a034

3 files changed

Lines changed: 180 additions & 2 deletions

File tree

‎.github/codex_auto_merge_policy.json‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,35 @@
2424
},
2525
"max_changed_files": 30,
2626
"max_changed_lines": 2000,
27+
"approved_change_bundles": [
28+
{
29+
"name": "retire-duplicate-aiaudit-pr-reviewer",
30+
"exact_changed_paths": [
31+
".github/actionlint.yaml",
32+
".github/codex_auto_merge_policy.json",
33+
".github/workflows/codex_pr_review.yml",
34+
"README.md",
35+
"README.zh-CN.md",
36+
"client/gateway_client.py",
37+
"docs/ai_autonomy_architecture.md",
38+
"prompts/pr_review.md",
39+
"scripts/run_codex_pr_review.py",
40+
"service/dual_review_primary.py",
41+
"service/org_health.py",
42+
"tests/test_codex_audit_service_complexity.py",
43+
"tests/test_dual_review_primary.py",
44+
"tests/test_org_health.py",
45+
"tests/test_run_codex_pr_review.py",
46+
"tests/test_single_pr_reviewer_contract.py"
47+
],
48+
"exact_deleted_paths": [
49+
"prompts/pr_review.md",
50+
"scripts/run_codex_pr_review.py",
51+
"tests/test_run_codex_pr_review.py"
52+
],
53+
"max_changed_lines": 3000
54+
}
55+
],
2756
"pr_review": {
2857
"enabled": true,
2958
"block_severities": [

‎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()

0 commit comments

Comments
 (0)