mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
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.
This commit is contained in:
@@ -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__/"
|
||||
],
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 = [
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user