From cb9b3e82bc0bd89bb945a497a19ed76df85b0976 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Mon, 5 Oct 2026 10:27:17 +0100 Subject: [PATCH] Retire the same-subsystem test wildcard the guards never honoured staged_verification_files_for_requirement() matches allow_same_subsystem_tests against a subsystem's owned_prefixes/owned_files, but both commit guards passed it the impacted entry, which carried neither. Since the v6 import (778a2577b6) a same-subsystem test never satisfied a requirement, while subsystem_lookup.py (which passes the registry rule) and the guard's own BLOCKED message ("same-subsystem test/spec files are also accepted") said it would. Honouring the 12 flags would have loosened the guard well past their apparent intent. Every rule requires explicit path-policy coverage, so the five rule-level flags are never consulted, and on the seven path policies the flag reaches every test the subsystem owns: a mutation-registry change would accept any internal/ai or AI Chat test (~455) instead of its 4 listed proofs, and a frontend Actions change any Go test under internal/unifiedresources. Six of the seven already list their own directory in test_prefixes. So the strictness stays and the tools now agree on it: the registry sets the 12 flags to false (what has always been enforced), registry_audit rejects true and points at test_prefixes/exact_files, and the impacted entry carries the rule's ownership so the completion and commit-shape guards read the flag the same way subsystem_lookup does. No requirement accepts a proof it rejected before. subsystem_lookup stops reporting matches the guard refused: a new internal/hostagent test no longer claims three agent-lifecycle policies. --- .../v6/internal/subsystems/registry.json | 24 +++---- .../canonical_completion_guard.py | 7 ++ .../canonical_completion_guard_test.py | 53 +++++++++++++- scripts/release_control/registry_audit.py | 21 ++++++ .../release_control/registry_audit_test.py | 69 ++++++++++++++++--- 5 files changed, 153 insertions(+), 21 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index b3b25a1a4f..b8ddb8e624 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -1433,7 +1433,7 @@ "match_files": [ "cmd/pulse-agent/collector_lifecycle.go" ], - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [ "internal/collectorlifecycle/" ], @@ -1456,7 +1456,7 @@ "internal/hostagent/action_runner_health_persistence_unix.go", "internal/hostagent/action_runner_health_persistence_windows.go" ], - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [ "internal/actionrunner/" ], @@ -1475,7 +1475,7 @@ "match_files": [ "cmd/pulse-agent-helper/main.go" ], - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [ "internal/agenthelper/" ], @@ -2140,7 +2140,7 @@ "match_files": [ "docs/AI_PATROL_QUALIFICATION.md" ], - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [ "internal/ai/qualification/" ], @@ -2156,7 +2156,7 @@ "internal/mutationregistry/" ], "match_files": [], - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [ "internal/mutationregistry/" ], @@ -2465,7 +2465,7 @@ "tests/integration/scripts/run-alert-qualification.mjs" ], "verification": { - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [], "exact_files": [ "internal/alerts/migration_characterization_test.go", @@ -3650,7 +3650,7 @@ "pulse-pro:scripts/validate_stripe_catalog.py" ], "verification": { - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [], "exact_files": [ "internal/api/contract_test.go", @@ -6069,7 +6069,7 @@ "pkg/tlsutil/certificate.go" ], "verification": { - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [], "exact_files": [ "internal/unifiedresources/code_standards_test.go" @@ -7477,7 +7477,7 @@ "pulse-pro:relay-server/revocation_feed.go" ], "verification": { - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [], "exact_files": [ "internal/api/relay_mobile_capability_test.go", @@ -7817,7 +7817,7 @@ "match_files": [ "pkg/extensions/audit_admin.go" ], - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [], "exact_files": [ "internal/api/audit_handlers_test.go", @@ -8293,7 +8293,7 @@ "internal/platformsupport/manifest_generated.go" ], "verification": { - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [], "exact_files": [ "frontend-modern/src/stores/__tests__/websocket-unified.test.ts", @@ -8564,7 +8564,7 @@ "frontend-modern/src/routing/navigation.ts", "frontend-modern/src/routing/routePreload.ts" ], - "allow_same_subsystem_tests": true, + "allow_same_subsystem_tests": false, "test_prefixes": [ "frontend-modern/src/features/actions/__tests__/" ], diff --git a/scripts/release_control/canonical_completion_guard.py b/scripts/release_control/canonical_completion_guard.py index 3cca39bc13..241e379d9e 100644 --- a/scripts/release_control/canonical_completion_guard.py +++ b/scripts/release_control/canonical_completion_guard.py @@ -321,6 +321,13 @@ def infer_impacted_subsystems( "touched_runtime_files": [], "cycle_artifact_files": [], "verification": dict(rule.get("verification", {})), + # Ownership travels with the entry because the guards pass + # this entry, not the registry rule, as the `rule` argument + # of staged_verification_files_for_requirement; without it + # a requirement's allow_same_subsystem_tests could never + # match while subsystem_lookup reported that it did. + "owned_prefixes": list(rule.get("owned_prefixes", [])), + "owned_files": list(rule.get("owned_files", [])), }, ) entry["touched_runtime_files"].append(path) diff --git a/scripts/release_control/canonical_completion_guard_test.py b/scripts/release_control/canonical_completion_guard_test.py index 61c1bb19d2..0c6044ecb1 100644 --- a/scripts/release_control/canonical_completion_guard_test.py +++ b/scripts/release_control/canonical_completion_guard_test.py @@ -661,7 +661,7 @@ class CanonicalCompletionGuardTest(unittest.TestCase): "internal/actionrunner/runner.go", "internal/dockeragent/action_runtime.go", ], - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": ["internal/actionrunner/"], "exact_files": [ "cmd/pulse-agent-runner/main_test.go", @@ -2557,6 +2557,57 @@ None yet. ) self.assertEqual(matches, ["frontend-modern/src/api/__tests__/alerts.test.ts"]) + def test_check_staged_contracts_reads_same_subsystem_tests_from_ownership(self): + # The guards hand the impacted entry, not the registry rule, to + # staged_verification_files_for_requirement. Before the entry carried + # owned_prefixes/owned_files, a same-subsystem test never satisfied a + # requirement that allowed one, while subsystem_lookup and the BLOCKED + # message both said it would. The live registry keeps the flag false + # (registry_audit rejects true), so a synthetic rule pins the plumbing. + synthetic_rule = { + "id": "synthetic", + "contract": "docs/release-control/v6/internal/subsystems/synthetic.md", + "owned_prefixes": ["synthetic/"], + "owned_files": [], + "verification": { + "allow_same_subsystem_tests": False, + "test_prefixes": [], + "exact_files": [], + "require_explicit_path_policy_coverage": True, + "path_policies": [ + { + "id": "synthetic-runtime", + "label": "synthetic runtime proof", + "match_prefixes": ["synthetic/"], + "match_files": [], + "allow_same_subsystem_tests": True, + "test_prefixes": [], + "exact_files": ["synthetic/listed_test.go"], + } + ], + }, + } + stderr = io.StringIO() + with ( + patch.dict(os.environ, {CONTRACT_NEUTRAL_OVERRIDE_ENV: ""}, clear=False), + patch( + "canonical_completion_guard.load_subsystem_rules", + return_value=[synthetic_rule], + ), + patch("canonical_completion_guard.required_contract_updates", return_value={}), + redirect_stderr(stderr), + ): + accepted = check_staged_contracts( + ["synthetic/runtime.go", "synthetic/nested/other_test.go"] + ) + blocked = check_staged_contracts( + ["synthetic/runtime.go", "elsewhere/other_test.go"] + ) + + self.assertEqual(accepted, 0) + self.assertEqual(blocked, 1) + self.assertIn("same-subsystem test/spec files are also accepted", stderr.getvalue()) + def test_patrol_autopilot_runtime_accepts_only_dedicated_verification(self): rules = {rule["id"]: rule for rule in load_subsystem_rules()} cases = [ diff --git a/scripts/release_control/registry_audit.py b/scripts/release_control/registry_audit.py index 18e317db02..5d5bc5355e 100644 --- a/scripts/release_control/registry_audit.py +++ b/scripts/release_control/registry_audit.py @@ -26,6 +26,17 @@ from status_audit import load_status_payload REGISTRY_PATH = DEFAULT_CONTROL_PLANE["registry_path"] REGISTRY_SCHEMA_PATH = DEFAULT_CONTROL_PLANE["registry_schema_path"] LANE_RE = re.compile(r"^L[0-9]+$") +# Every subsystem requires explicit path-policy coverage, so the default +# policy is never consulted and a path policy's own proof routes are the whole +# requirement. allow_same_subsystem_tests would widen a policy to every test +# the subsystem owns (any internal/ai test for ai-runtime's mutation-registry +# policy), and its reach would grow with ownership rather than with the +# policy. The commit guards ignored the flag from the v6 import until it was +# retired here, so false is the behaviour every commit has been held to. +SAME_SUBSYSTEM_TESTS_RETIRED = ( + "it would accept any test the subsystem owns as proof for this policy; " + "list the accepted proofs in test_prefixes or exact_files instead" +) def load_registry_payload(*, staged: bool = False) -> dict[str, Any]: @@ -342,6 +353,11 @@ def audit_registry_payload( if not isinstance(verification.get("allow_same_subsystem_tests"), bool): errors.append(f"{context}.verification.allow_same_subsystem_tests must be a bool") + elif verification.get("allow_same_subsystem_tests") is True: + errors.append( + f"{context}.verification.allow_same_subsystem_tests must be false: " + + SAME_SUBSYSTEM_TESTS_RETIRED + ) if not isinstance(verification.get("require_explicit_path_policy_coverage"), bool): errors.append(f"{context}.verification.require_explicit_path_policy_coverage must be a bool") elif verification.get("require_explicit_path_policy_coverage") is not True: @@ -465,6 +481,11 @@ def audit_registry_payload( if not isinstance(raw_policy.get("allow_same_subsystem_tests"), bool): errors.append(f"{policy_context}.allow_same_subsystem_tests must be a bool") + elif raw_policy.get("allow_same_subsystem_tests") is True: + errors.append( + f"{policy_context}.allow_same_subsystem_tests must be false: " + + SAME_SUBSYSTEM_TESTS_RETIRED + ) policy_test_prefixes = raw_policy.get("test_prefixes") if not isinstance(policy_test_prefixes, list): diff --git a/scripts/release_control/registry_audit_test.py b/scripts/release_control/registry_audit_test.py index 803a075b08..619adc5178 100644 --- a/scripts/release_control/registry_audit_test.py +++ b/scripts/release_control/registry_audit_test.py @@ -571,7 +571,7 @@ class RegistryAuditTest(unittest.TestCase): "owned_prefixes": ["internal/monitoring/"], "owned_files": [], "verification": { - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": [], "exact_files": ["internal/monitoring/canonical_guardrails_test.go"], "require_explicit_path_policy_coverage": True, @@ -615,7 +615,7 @@ class RegistryAuditTest(unittest.TestCase): "owned_prefixes": ["pulse-mobile:src/relay/"], "owned_files": [], "verification": { - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": [], "exact_files": [ "pulse-mobile:src/relay/__tests__/client.test.ts", @@ -661,7 +661,7 @@ class RegistryAuditTest(unittest.TestCase): "owned_prefixes": ["pulse-mobile:src/relay/"], "owned_files": [], "verification": { - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": [], "exact_files": ["pulse-mobile:src/relay/__tests__/client.test.ts"], "require_explicit_path_policy_coverage": True, @@ -714,7 +714,7 @@ class RegistryAuditTest(unittest.TestCase): "owned_prefixes": ["internal/alerts/"], "owned_files": [], "verification": { - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": [], "exact_files": [], "require_explicit_path_policy_coverage": True, @@ -743,7 +743,7 @@ class RegistryAuditTest(unittest.TestCase): "owned_prefixes": ["pkg/licensing/"], "owned_files": [], "verification": { - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": [], "exact_files": ["pkg/licensing/cloud_paid_guardrails_test.go"], "require_explicit_path_policy_coverage": True, @@ -819,6 +819,59 @@ class RegistryAuditTest(unittest.TestCase): "\n".join(report["errors"]), ) + def test_audit_registry_payload_rejects_same_subsystem_tests_wildcard(self) -> None: + # Explicit path-policy coverage is mandatory, so the flag could only + # widen a policy to every test the subsystem owns. + payload = { + "version": 13, + "shared_ownerships": [], + "subsystems": [ + { + "id": "monitoring", + "lane": "L6", + "contract": "docs/release-control/v6/internal/subsystems/monitoring.md", + "owned_prefixes": ["internal/monitoring/"], + "owned_files": [], + "verification": { + "allow_same_subsystem_tests": True, + "test_prefixes": [], + "exact_files": ["internal/monitoring/canonical_guardrails_test.go"], + "require_explicit_path_policy_coverage": True, + "path_policies": [ + { + "id": "monitoring-runtime", + "label": "monitoring runtime proof", + "match_prefixes": ["internal/monitoring/"], + "match_files": [], + "allow_same_subsystem_tests": True, + "test_prefixes": [], + "exact_files": ["internal/monitoring/canonical_guardrails_test.go"], + } + ], + }, + } + ], + } + tracked_files = { + "docs/release-control/v6/internal/subsystems/monitoring.md", + "internal/monitoring/monitor.go", + "internal/monitoring/canonical_guardrails_test.go", + } + + report = audit_registry_payload(payload, tracked_files=tracked_files, status_lane_ids={"L6"}) + + same_subsystem_errors = [ + error for error in report["errors"] if "allow_same_subsystem_tests must be false" in error + ] + self.assertEqual( + [error.split(":", 1)[0] for error in same_subsystem_errors], + [ + "subsystems[0].verification.allow_same_subsystem_tests must be false", + "subsystems[0].verification.path_policies[0].allow_same_subsystem_tests must be false", + ], + ) + self.assertIn("test_prefixes or exact_files", same_subsystem_errors[0]) + def test_audit_registry_payload_rejects_uncanonical_ordering(self) -> None: payload = { "version": 13, @@ -831,7 +884,7 @@ class RegistryAuditTest(unittest.TestCase): "owned_prefixes": ["internal/monitoring/", "frontend/monitoring/"], "owned_files": ["internal/monitoring/b.go", "internal/monitoring/a.go"], "verification": { - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": ["internal/monitoring/tests/", "frontend/tests/"], "exact_files": [ "internal/monitoring/z_test.go", @@ -861,7 +914,7 @@ class RegistryAuditTest(unittest.TestCase): "owned_prefixes": ["internal/alerts/"], "owned_files": [], "verification": { - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": [], "exact_files": ["internal/alerts/guardrails_test.go"], "require_explicit_path_policy_coverage": True, @@ -931,7 +984,7 @@ class RegistryAuditTest(unittest.TestCase): "owned_prefixes": ["internal/monitoring/"], "owned_files": [], "verification": { - "allow_same_subsystem_tests": True, + "allow_same_subsystem_tests": False, "test_prefixes": [], "exact_files": ["internal/monitoring/guardrails_test.go"], "require_explicit_path_policy_coverage": True,