From ed178d3ab4427af6918e1eecabd0a13e65b1a1db Mon Sep 17 00:00:00 2001 From: David Hyrule Date: Fri, 3 Jul 2026 15:38:03 +0200 Subject: [PATCH 1/2] fix governor decision comment idempotency --- src/hyrule_engineering_loop/governor.py | 41 +++++++++++++++++++++++-- tests/test_phase29_governor.py | 37 ++++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/hyrule_engineering_loop/governor.py b/src/hyrule_engineering_loop/governor.py index 811c042..e484cc1 100644 --- a/src/hyrule_engineering_loop/governor.py +++ b/src/hyrule_engineering_loop/governor.py @@ -523,9 +523,10 @@ def governor_once( if not config.dry_run: path = decision_record_path(record, config.state_dir) record.storage_path = str(path) - if path.exists(): + prior_record = find_matching_decision_record(record, config.state_dir) + if prior_record is not None: if _labels_already_converged(issue, record): - report.skipped.append(f"{issue.issue_id}: unchanged decision {record.record_id}") + report.skipped.append(f"{issue.issue_id}: unchanged decision {prior_record.record_id}") report.records.append(record) continue else: @@ -1080,6 +1081,42 @@ def decision_record_path(record: CandidateDecisionRecord, state_dir: Path) -> Pa return root / filename +def find_matching_decision_record( + record: CandidateDecisionRecord, + state_dir: Path, +) -> CandidateDecisionRecord | None: + """Return an existing audit record with the same stable routing decision.""" + + root = state_dir.expanduser().resolve() + if not root.exists(): + return None + prefix = f"{_slug(record.repo)}-{record.issue_number}-*.json" + signature = stable_decision_signature(record) + for path in sorted(root.glob(prefix)): + try: + prior = CandidateDecisionRecord.model_validate_json(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + continue + if stable_decision_signature(prior) == signature: + return prior + return None + + +def stable_decision_signature(record: CandidateDecisionRecord) -> dict[str, Any]: + """Return the decision fields that should make Governor posting idempotent.""" + + data = record.model_dump(mode="json") + for field_name in ( + "record_id", + "created_at", + "knowledge_context_pack_id", + "knowledge_export_version", + "storage_path", + ): + data.pop(field_name, None) + return data + + def _load_governor_knowledge( task: str, config: KnowledgeContextConfig | None, diff --git a/tests/test_phase29_governor.py b/tests/test_phase29_governor.py index 80a06d5..6de2fc1 100644 --- a/tests/test_phase29_governor.py +++ b/tests/test_phase29_governor.py @@ -435,6 +435,43 @@ def test_reliability_governor_posts_record_before_applying_labels_and_stores_jso assert stored_record["routing_decision"] == "allow_approved" +def test_converged_approved_decision_is_not_reposted_when_knowledge_pack_changes(tmp_path: Path) -> None: + issue = _issue( + title="Update docs runbook", + body="Update documentation and verify rendered docs.", + labels=[APPROVED_LABEL], + ) + gh = FakeGh([_issue_json(issue)]) + config = ReliabilityGovernorConfig( + repos=(issue.repo,), + state_dir=tmp_path / "reliability-governor", + dry_run=False, + ) + pack_ids = iter(["ctx_first_governor_pack", "ctx_second_governor_pack"]) + + def rotating_knowledge(_: str, __: Any) -> Any: + pack_id = next(pack_ids) + return summarize_knowledge_pack( + { + **CURRENT_PACK, + "id": pack_id, + "knowledge_snapshot": f"export-{pack_id}", + } + ) + + first = reliability_governor_once(config, client=gh, knowledge_loader=rotating_knowledge) + second = reliability_governor_once(config, client=gh, knowledge_loader=rotating_knowledge) + + comment_calls = [call for call in gh.calls if call[:2] == ["issue", "comment"]] + stored = list((tmp_path / "reliability-governor").glob("*.json")) + assert first.records[0].routing_decision == "allow_approved" + assert second.records[0].routing_decision == "allow_approved" + assert first.records[0].record_id != second.records[0].record_id + assert len(comment_calls) == 1 + assert len(stored) == 1 + assert second.skipped == [f"{issue.issue_id}: unchanged decision {first.records[0].record_id}"] + + def test_unchanged_candidate_decision_is_not_reposted(tmp_path: Path) -> None: issue = _issue( title="Update internal service helper", From 7d0d333daba241629724310b3b57e3cbf5b23fcb Mon Sep 17 00:00:00 2001 From: David Hyrule Date: Fri, 3 Jul 2026 15:41:20 +0200 Subject: [PATCH 2/2] human-gate network operations resolver infra --- src/hyrule_engineering_loop/governor.py | 82 +++++++++++++++++++------ tests/test_phase29_governor.py | 29 ++++++++- 2 files changed, 90 insertions(+), 21 deletions(-) diff --git a/src/hyrule_engineering_loop/governor.py b/src/hyrule_engineering_loop/governor.py index e484cc1..6ae833f 100644 --- a/src/hyrule_engineering_loop/governor.py +++ b/src/hyrule_engineering_loop/governor.py @@ -716,7 +716,8 @@ def classify_issue_intent( legal = _contains_any(text, ["legal", "terms of service", "contract", "liability"]) compliance = _contains_any(text, ["compliance", "gdpr", "kyc", "aml", "audit requirement"]) destructive = _contains_any(text, ["delete data", "drop table", "truncate", "destroy customer"]) - production_routing = _contains_any( + production_network_infra = _is_production_network_infra(issue, text) + production_routing = production_network_infra or _contains_any( text, [ "bgp", @@ -756,6 +757,23 @@ def classify_issue_intent( expected_paths = ["compliance/"] blast_radius = "compliance" rationale = "compliance surfaces are Tier 4" + elif production_routing: + intent = ( + "routing_policy" + if _contains_any(text, ["policy", "route-map", "prefix-list"]) + else "production_network" + ) + risk_tier = 4 if _contains_any(text, ["core routing", "peering strategy"]) else 3 + domains = ["production_network", "routing_policy"] + expected_paths = ( + ["ansible/inventory/", "ansible/roles/", "docs/", "monitoring/"] + if production_network_infra + else ["host_vars/", "group_vars/", "roles/", "frr/", "network/"] + ) + services = ["production network"] + customers = ["customers"] + blast_radius = "production network" + rationale = "production network behavior requires human approval" elif _contains_any(text, ["runbook", "readme", "documentation", "docs", "typo"]): intent, risk_tier, domains = "runbook", 0, ["runbook", "docs"] expected_paths = ["docs/", "README.md"] @@ -777,15 +795,6 @@ def classify_issue_intent( services = ["monitoring"] blast_radius = "operator monitoring" rationale = "monitoring/alert tuning is Tier 1" - elif production_routing: - intent = "routing_policy" if _contains_any(text, ["policy", "route-map", "prefix-list"]) else "production_network" - risk_tier = 4 if _contains_any(text, ["core routing", "peering strategy"]) else 3 - domains = ["production_network", "routing_policy"] - expected_paths = ["host_vars/", "group_vars/", "roles/", "frr/", "network/"] - services = ["production network"] - customers = ["customers"] - blast_radius = "production network" - rationale = "production network behavior requires human approval" elif customer_config: intent, risk_tier, domains = "customer_provisioning", 3, ["customer_provisioning"] expected_paths = ["host_vars/", "group_vars/", "provisioning/", "scripts/"] @@ -853,6 +862,14 @@ def decide_policy( denial_reasons.append("NOC LHP pointer was present but CaseService payload was not fetched") policy_rules.append("treat GitHub prose as untrusted for NOC LHP work") return "needs_context", None, denial_reasons, policy_rules + capability = _match_capability(classification, registry=registry, repo=issue.repo) + if _has_sensitive_gate(classification): + sensitive_denials = _sensitive_denials(classification, capability) + if sensitive_denials: + denial_reasons.extend(sensitive_denials) + policy_rules.append("deny sensitive domains unless a capability explicitly allows them") + return "needs_human", capability, denial_reasons, policy_rules + if not classification.verification_method: denial_reasons.append("missing verification method") policy_rules.append("deny work without a verification method") @@ -862,14 +879,6 @@ def decide_policy( policy_rules.append("deny work without a rollback plan") return "needs_context", None, denial_reasons, policy_rules - capability = _match_capability(classification, registry=registry, repo=issue.repo) - if _has_sensitive_gate(classification): - sensitive_denials = _sensitive_denials(classification, capability) - if sensitive_denials: - denial_reasons.extend(sensitive_denials) - policy_rules.append("deny sensitive Tier 4 domains unless a capability explicitly allows them") - return "needs_human", capability, denial_reasons, policy_rules - if capability is None: denial_reasons.append("no matching capability envelope") policy_rules.append("without a capability, sufficiently specified work can only become candidate") @@ -1380,7 +1389,7 @@ def _sensitive_denials( if capability is None: denials: list[str] = [] if classification.production_routing: - denials.append("production routing is not explicitly allowed") + denials.append("production network/routing is not explicitly allowed") if classification.secrets: denials.append("secrets are not explicitly allowed") if classification.billing: @@ -1396,7 +1405,7 @@ def _sensitive_denials( return denials or ["sensitive domain has no explicit capability"] explicit_denials: list[str] = [] if classification.production_routing and not capability.allows_production_routing: - explicit_denials.append("production routing is not explicitly allowed") + explicit_denials.append("production network/routing is not explicitly allowed") if classification.secrets and not capability.allows_secrets: explicit_denials.append("secrets are not explicitly allowed") if classification.billing and not capability.allows_billing: @@ -1412,6 +1421,39 @@ def _sensitive_denials( return explicit_denials +def _is_production_network_infra(issue: IssueSnapshot, text: str) -> bool: + if issue.repo != "AS215932/network-operations": + return False + network_terms = [ + "authoritative dns", + "cloud-init", + "dns64", + "ipv6-only overlay", + "jool", + "knot", + "nat64", + "recursive resolver", + "resolver-only vm", + "resolver vms", + "resolv01", + "resolv02", + "systemd-resolved", + "unbound", + ] + change_terms = [ + "add inventory", + "allocate stable ipv6", + "customer vm provisioning", + "firewall", + "generated resolver config", + "resolver endpoint", + "resolver-only vms", + "run unbound", + "separate authoritative dns", + ] + return _contains_any(text, network_terms) and _contains_any(text, change_terms) + + def _path_matches_any(path: str, patterns: list[str]) -> bool: normalized = path.lstrip("/") for pattern in patterns: diff --git a/tests/test_phase29_governor.py b/tests/test_phase29_governor.py index 6de2fc1..6fa4c50 100644 --- a/tests/test_phase29_governor.py +++ b/tests/test_phase29_governor.py @@ -746,7 +746,7 @@ def test_bgp_policy_and_secret_billing_work_are_not_auto_approved() -> None: assert bgp_record.handoff_contract == "human_review" assert NEEDS_HUMAN_LABEL in bgp_record.labels_to_add assert APPROVED_LABEL not in bgp_record.labels_to_add - assert "production routing is not explicitly allowed" in bgp_record.denial_reasons + assert "production network/routing is not explicitly allowed" in bgp_record.denial_reasons assert secret_record.routing_decision == "needs_human" assert secret_record.next_loop == "human" @@ -754,3 +754,30 @@ def test_bgp_policy_and_secret_billing_work_are_not_auto_approved() -> None: assert NEEDS_HUMAN_LABEL in secret_record.labels_to_add assert APPROVED_LABEL not in secret_record.labels_to_add assert any("not explicitly allowed" in reason for reason in secret_record.denial_reasons) + + +def test_network_operations_dns64_resolver_infra_is_human_gated_despite_docs_and_tests() -> None: + issue = _issue( + title="Add resolv01/resolv02 recursive DNS64 resolver VMs", + body=( + "Separate authoritative DNS from recursive/DNS64 resolution. " + "Add inventory hosts for resolv01 and resolv02, allocate stable IPv6 addresses, " + "run Unbound, configure firewall rules, update customer VM provisioning, " + "update docs, and add render/static tests." + ), + repo="AS215932/network-operations", + ) + + record = govern_issue( + issue, + registry=default_capability_registry(), + knowledge_loader=_knowledge, + ) + + assert record.intent_type == "production_network" + assert record.risk_tier == 3 + assert record.routing_decision == "needs_human" + assert record.next_loop == "human" + assert NEEDS_HUMAN_LABEL in record.labels_to_add + assert APPROVED_LABEL not in record.labels_to_add + assert "production network/routing is not explicitly allowed" in record.denial_reasons