From a4ab14f4970c82fee87f42b400d86cfa0bfe5fc8 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:08:38 +0800 Subject: [PATCH 1/2] feat(gate): require base-approved exact deletions Co-Authored-By: Codex --- .github/codex_auto_merge_policy.json | 8 +++- scripts/gate_codex_app_review_static.py | 22 ++++++++++- tests/test_gate_codex_app_review.py | 50 +++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/.github/codex_auto_merge_policy.json b/.github/codex_auto_merge_policy.json index 50d2bfd4..084baa70 100644 --- a/.github/codex_auto_merge_policy.json +++ b/.github/codex_auto_merge_policy.json @@ -23,7 +23,13 @@ } }, "max_changed_files": 30, - "max_changed_lines": 2000, + "max_changed_lines": 3000, + "approved_deleted_paths": [ + ".github/workflows/codex_pr_review.yml", + "prompts/pr_review.md", + "scripts/run_codex_pr_review.py", + "tests/test_run_codex_pr_review.py" + ], "pr_review": { "enabled": true, "block_severities": [ diff --git a/scripts/gate_codex_app_review_static.py b/scripts/gate_codex_app_review_static.py index 8ac817fb..cf630f91 100644 --- a/scripts/gate_codex_app_review_static.py +++ b/scripts/gate_codex_app_review_static.py @@ -7,7 +7,7 @@ import json import re -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Any DEFAULT_POLICY_PATH = Path(".github/codex_auto_merge_policy.json") @@ -72,6 +72,24 @@ def scan_diff(diff_text: str, path_patterns: list[re.Pattern[str]]) -> list[str] def check_metadata(files: list[dict[str, Any]], policy: dict[str, Any]) -> list[str]: issues: list[str] = [] + approved_deleted_paths: set[str] = set() + configured_approvals = policy.get("approved_deleted_paths", []) + if isinstance(configured_approvals, list): + for value in configured_approvals: + if not isinstance(value, str) or value != value.strip(): + continue + path = PurePosixPath(value) + if ( + not value + or path.is_absolute() + or path.as_posix() != value + or ".." in path.parts + or "\\" in value + or any(character in value for character in "*?[]") + ): + continue + approved_deleted_paths.add(value) + max_files = policy.get("max_changed_files", 50) max_lines = policy.get("max_changed_lines", 5000) total_added = sum(f.get("additions", 0) or 0 for f in files) @@ -79,7 +97,7 @@ def check_metadata(files: list[dict[str, Any]], policy: dict[str, Any]) -> list[ for f in files: filename = f.get("filename", "?") status = (f.get("status") or "").lower().strip() - if status == "removed": + if status == "removed" and filename not in approved_deleted_paths: issues.append(f"**File deleted**: `{filename}` — verify intentional") elif status == "renamed": issues.append(f"**File renamed**: `{f.get('previous_filename', '?')}` → `{filename}`") diff --git a/tests/test_gate_codex_app_review.py b/tests/test_gate_codex_app_review.py index 923403c1..7a554ed0 100644 --- a/tests/test_gate_codex_app_review.py +++ b/tests/test_gate_codex_app_review.py @@ -107,6 +107,56 @@ def test_collect_static_gate_issues_aggregates_metadata_and_diff(self) -> None: self.assertTrue(any("Hardcoded secret" in issue for issue in issues)) self.assertTrue(any("Too many lines" in issue for issue in issues)) + def test_check_metadata_allows_only_exact_base_approved_deletions(self) -> None: + files = [ + { + "filename": "scripts/retired.py", + "status": "removed", + "additions": 0, + "deletions": 10, + }, + { + "filename": "scripts/unapproved.py", + "status": "removed", + "additions": 0, + "deletions": 10, + }, + ] + policy = { + "approved_deleted_paths": ["scripts/retired.py"], + "max_changed_files": 10, + "max_changed_lines": 100, + } + + issues = gate_codex_app_review_static.check_metadata(files, policy) + + self.assertFalse(any("scripts/retired.py" in issue for issue in issues)) + self.assertTrue(any("scripts/unapproved.py" in issue for issue in issues)) + + def test_check_metadata_rejects_unsafe_delete_approvals(self) -> None: + files = [ + { + "filename": "scripts/retired.py", + "status": "removed", + "additions": 0, + "deletions": 10, + }, + ] + for approval in ( + "scripts/*.py", + "../scripts/retired.py", + "/scripts/retired.py", + "scripts\\retired.py", + ): + with self.subTest(approval=approval): + policy = { + "approved_deleted_paths": [approval], + "max_changed_files": 10, + "max_changed_lines": 100, + } + issues = gate_codex_app_review_static.check_metadata(files, policy) + self.assertTrue(any("scripts/retired.py" in issue for issue in issues)) + if __name__ == "__main__": unittest.main() From 994c296d0a3bc7a5206385eb281149146eb674e1 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:21:28 +0800 Subject: [PATCH 2/2] fix(gate): scope deletion approval to exact cleanup Co-Authored-By: Codex --- .github/codex_auto_merge_policy.json | 35 +++++++++-- scripts/gate_codex_app_review_static.py | 66 ++++++++++++++++----- tests/test_gate_codex_app_review.py | 79 +++++++++++++++++++------ 3 files changed, 142 insertions(+), 38 deletions(-) diff --git a/.github/codex_auto_merge_policy.json b/.github/codex_auto_merge_policy.json index 084baa70..73144359 100644 --- a/.github/codex_auto_merge_policy.json +++ b/.github/codex_auto_merge_policy.json @@ -23,12 +23,35 @@ } }, "max_changed_files": 30, - "max_changed_lines": 3000, - "approved_deleted_paths": [ - ".github/workflows/codex_pr_review.yml", - "prompts/pr_review.md", - "scripts/run_codex_pr_review.py", - "tests/test_run_codex_pr_review.py" + "max_changed_lines": 2000, + "approved_change_bundles": [ + { + "name": "retire-duplicate-aiaudit-pr-reviewer", + "exact_changed_paths": [ + ".github/actionlint.yaml", + ".github/codex_auto_merge_policy.json", + ".github/workflows/codex_pr_review.yml", + "README.md", + "README.zh-CN.md", + "client/gateway_client.py", + "docs/ai_autonomy_architecture.md", + "prompts/pr_review.md", + "scripts/run_codex_pr_review.py", + "service/dual_review_primary.py", + "service/org_health.py", + "tests/test_codex_audit_service_complexity.py", + "tests/test_dual_review_primary.py", + "tests/test_org_health.py", + "tests/test_run_codex_pr_review.py", + "tests/test_single_pr_reviewer_contract.py" + ], + "exact_deleted_paths": [ + "prompts/pr_review.md", + "scripts/run_codex_pr_review.py", + "tests/test_run_codex_pr_review.py" + ], + "max_changed_lines": 3000 + } ], "pr_review": { "enabled": true, diff --git a/scripts/gate_codex_app_review_static.py b/scripts/gate_codex_app_review_static.py index cf630f91..b0b4ba12 100644 --- a/scripts/gate_codex_app_review_static.py +++ b/scripts/gate_codex_app_review_static.py @@ -70,28 +70,66 @@ def scan_diff(diff_text: str, path_patterns: list[re.Pattern[str]]) -> list[str] return list(dict.fromkeys(violations)) +def _safe_exact_paths(values: Any) -> set[str] | None: + if not isinstance(values, list) or not values: + return None + paths: set[str] = set() + for value in values: + if not isinstance(value, str) or value != value.strip(): + return None + path = PurePosixPath(value) + if ( + not value + or path.is_absolute() + or path.as_posix() != value + or ".." in path.parts + or "\\" in value + or any(character in value for character in "*?[]") + or value in paths + ): + return None + paths.add(value) + return paths + + def check_metadata(files: list[dict[str, Any]], policy: dict[str, Any]) -> list[str]: issues: list[str] = [] approved_deleted_paths: set[str] = set() - configured_approvals = policy.get("approved_deleted_paths", []) - if isinstance(configured_approvals, list): - for value in configured_approvals: - if not isinstance(value, str) or value != value.strip(): + max_files = policy.get("max_changed_files", 50) + max_lines = policy.get("max_changed_lines", 5000) + changed_paths = { + filename + for f in files + if isinstance((filename := f.get("filename")), str) + } + removed_paths = { + f["filename"] + for f in files + if isinstance(f.get("filename"), str) + and (f.get("status") or "").lower().strip() == "removed" + } + configured_bundles = policy.get("approved_change_bundles", []) + if isinstance(configured_bundles, list): + for bundle in configured_bundles: + if not isinstance(bundle, dict): continue - path = PurePosixPath(value) + exact_changed_paths = _safe_exact_paths(bundle.get("exact_changed_paths")) + exact_deleted_paths = _safe_exact_paths(bundle.get("exact_deleted_paths")) + bundle_max_lines = bundle.get("max_changed_lines") if ( - not value - or path.is_absolute() - or path.as_posix() != value - or ".." in path.parts - or "\\" in value - or any(character in value for character in "*?[]") + exact_changed_paths is None + or exact_deleted_paths is None + or not exact_deleted_paths.issubset(exact_changed_paths) + or type(bundle_max_lines) is not int + or bundle_max_lines < max_lines + or changed_paths != exact_changed_paths + or removed_paths != exact_deleted_paths ): continue - approved_deleted_paths.add(value) + approved_deleted_paths = exact_deleted_paths + max_lines = bundle_max_lines + break - max_files = policy.get("max_changed_files", 50) - max_lines = policy.get("max_changed_lines", 5000) total_added = sum(f.get("additions", 0) or 0 for f in files) total_deleted = sum(f.get("deletions", 0) or 0 for f in files) for f in files: diff --git a/tests/test_gate_codex_app_review.py b/tests/test_gate_codex_app_review.py index 7a554ed0..e73f542e 100644 --- a/tests/test_gate_codex_app_review.py +++ b/tests/test_gate_codex_app_review.py @@ -107,33 +107,69 @@ def test_collect_static_gate_issues_aggregates_metadata_and_diff(self) -> None: self.assertTrue(any("Hardcoded secret" in issue for issue in issues)) self.assertTrue(any("Too many lines" in issue for issue in issues)) - def test_check_metadata_allows_only_exact_base_approved_deletions(self) -> None: + def test_check_metadata_allows_only_an_exact_base_approved_change_bundle(self) -> None: files = [ { "filename": "scripts/retired.py", "status": "removed", "additions": 0, - "deletions": 10, + "deletions": 150, }, { - "filename": "scripts/unapproved.py", - "status": "removed", - "additions": 0, - "deletions": 10, + "filename": "service/caller.py", + "status": "modified", + "additions": 2_400, + "deletions": 0, + }, + ] + policy = { + "max_changed_files": 10, + "max_changed_lines": 2_000, + "approved_change_bundles": [ + { + "exact_changed_paths": [ + "scripts/retired.py", + "service/caller.py", + ], + "exact_deleted_paths": ["scripts/retired.py"], + "max_changed_lines": 3_000, + } + ], + } + + issues = gate_codex_app_review_static.check_metadata(files, policy) + + self.assertEqual(issues, []) + + def test_check_metadata_does_not_expand_line_budget_for_an_unrelated_change(self) -> None: + files = [ + { + "filename": "service/unrelated.py", + "status": "modified", + "additions": 2_500, + "deletions": 0, }, ] policy = { - "approved_deleted_paths": ["scripts/retired.py"], "max_changed_files": 10, - "max_changed_lines": 100, + "max_changed_lines": 2_000, + "approved_change_bundles": [ + { + "exact_changed_paths": [ + "scripts/retired.py", + "service/caller.py", + ], + "exact_deleted_paths": ["scripts/retired.py"], + "max_changed_lines": 3_000, + } + ], } issues = gate_codex_app_review_static.check_metadata(files, policy) - self.assertFalse(any("scripts/retired.py" in issue for issue in issues)) - self.assertTrue(any("scripts/unapproved.py" in issue for issue in issues)) + self.assertTrue(any("Too many lines" in issue for issue in issues)) - def test_check_metadata_rejects_unsafe_delete_approvals(self) -> None: + def test_check_metadata_rejects_partial_or_unsafe_change_bundles(self) -> None: files = [ { "filename": "scripts/retired.py", @@ -142,17 +178,24 @@ def test_check_metadata_rejects_unsafe_delete_approvals(self) -> None: "deletions": 10, }, ] - for approval in ( - "scripts/*.py", - "../scripts/retired.py", - "/scripts/retired.py", - "scripts\\retired.py", + for exact_changed_paths in ( + ["scripts/retired.py", "service/caller.py"], + ["scripts/*.py"], + ["../scripts/retired.py"], + ["/scripts/retired.py"], + ["scripts\\retired.py"], ): - with self.subTest(approval=approval): + with self.subTest(exact_changed_paths=exact_changed_paths): policy = { - "approved_deleted_paths": [approval], "max_changed_files": 10, "max_changed_lines": 100, + "approved_change_bundles": [ + { + "exact_changed_paths": exact_changed_paths, + "exact_deleted_paths": ["scripts/retired.py"], + "max_changed_lines": 200, + } + ], } issues = gate_codex_app_review_static.check_metadata(files, policy) self.assertTrue(any("scripts/retired.py" in issue for issue in issues))