Skip to content

Fix Governor idempotency and #350 human-gating #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Svaag merged 2 commits into from
Jul 3, 2026
Merged

Fix Governor idempotency and #350 human-gating #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed178d3
fix governor decision comment idempotency
Svaag Jul 3, 2026
7d0d333
human-gate network operations resolver infra
Svaag Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 101 additions & 22 deletions src/hyrule_engineering_loop/governor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -715,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",
Expand Down Expand Up @@ -755,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"]
Expand All @@ -776,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/"]
Expand Down Expand Up @@ -852,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")
Expand All @@ -861,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")
Expand Down Expand Up @@ -1080,6 +1090,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)):

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use only the latest prior decision for idempotency

When an issue has an earlier approved record, is later edited to a different decision, and then is reverted while the Knowledge pack id/export has rotated, this scan can return the old matching JSON file even though a newer conflicting Reliability Decision Record comment exists. governor_once then skips reposting or just reapplies labels, but the daemon consumes the latest trusted Decision Record comment, so the issue can be labeled loop:approved while the daemon still sees the later needs_human/stale record and refuses to run it. Treat a match as idempotent only if it is the newest decision for that issue, or post a fresh record when newer conflicting records exist.

Useful? React with 👍 / 👎.

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,
Expand Down Expand Up @@ -1343,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:
Expand All @@ -1359,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:
Expand All @@ -1375,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:
Expand Down
66 changes: 65 additions & 1 deletion tests/test_phase29_governor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -709,11 +746,38 @@ 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"
assert secret_record.handoff_contract == "human_review"
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