Robustness review: agt-policies fail-open & contract-leak defects (9 files, 36 verified findings)

> **Suggested labels** (issue author lacks label-write access on this repo; maintainers/triage please apply): `bug` · `security` · `Priority: HIGH` · `agent-governance` · `python` · `triage`

# agt-policies — Robustness & Maintainability Review

This review covers the nine reviewed source files of the AGT v5 `agt-policies` package (`policies/bridge.py`, `policies/runtime.py`, `policies/snapshot.py`, `manifest_resolution/merge.py`, `manifest_resolution/build.py`, `manifest_resolution/discover.py`, `manifest_resolution/scope.py`, `_harness/opa_runner.py`, and `cli/migrate.py`; `policies/result.py` was explicitly excluded). Each file was reviewed independently and every finding was then re-confirmed through a two-lens adversarial pass (fail-open/fail-closed semantics against ADR-0013, and determinism/contract semantics against ADR-0004/ADR-0014). Findings tagged **high confidence** were confirmed by both lenses; **medium confidence** by one. No findings were invented beyond that verification set. The dominant theme is *fail-open and contract-leak risk*: many error and edge paths resolve to `allow` (or to an uncaught non-`ResolutionError` exception that bypasses the host's fail-closed translation) rather than to a deterministic deny.

## Summary

| File | High | Medium | Headline issue |
|---|---|---|---|
| `policies/bridge.py` | 1 | 0 | GLOB deny patterns emit Python-`fnmatch` regex that RE2/OPA cannot match → blocked pattern fails open |
| `policies/runtime.py` | 0 | 2 | `_run_sync(timeout=None)` inside a running loop spawns an unbounded daemon thread and `join(None)` blocks forever |
| `policies/snapshot.py` | 1 | 1 | Budget validation accepts `NaN`/`Infinity` → non-round-trippable bytes and a deny rule that never trips |
| `manifest_resolution/merge.py` | 0 | 2 | Parent deny with an "unsatisfiable"-classified condition is silently neutralized (fail-open, ADR-0014) |
| `manifest_resolution/build.py` | 2 | 2 | Invalid/unvalidated regex operator renders a normal branch → OPA eval error → default `allow` |
| `manifest_resolution/discover.py` | 0 | 2 | `Path.resolve()`/`is_file()` `OSError` escapes as non-`ResolutionError`, bypassing fail-closed translation |
| `manifest_resolution/scope.py` | 1 | 1 | `fnmatch` (vs `fnmatchcase`) makes scope matching case/separator platform-dependent → non-deterministic doc selection |
| `_harness/opa_runner.py` | 0 | 2 | Missing `decision` key defaults to `allow`; empty `result` list crashes with `IndexError` |
| `cli/migrate.py` | 1 | 2 | `--write` backs up parent governance files outside `chain_root` (ADR-0014) and migration writes are non-atomic |

Lower-severity type-safety and resource-leak findings are documented under each file below.

---

## `policies/bridge.py`

### GLOB patterns rendered with `fnmatch.translate` produce non-RE2 regex (fail-open) — severity **high**, confidence **high**

**Location:** `_pattern_to_regex`, lines 121–124.

**What's wrong:** For a v4 `GLOB` blocked-pattern entry the bridge returns `fnmatch.translate(value)`. `fnmatch.translate('*.txt')` yields `(?s:.*\.txt)\Z` — an inline-flag group `(?s:...)` plus the `\Z`/`\z` end anchor. Go RE2 (used by OPA's `agt.patterns.deny_if_pattern`) does not support `\Z`/`\z` and rejects or ignores these constructs, directly contradicting the `_pattern_to_regex` docstring promise that "the bridge emits a Go RE2 regex literal in every case."

**Why it matters:** At best OPA raises a bundle compile error; at worst the pattern silently fails to match, so a `GLOB` `blocked_pattern` that should DENY falls through to `default verdict := {"decision": "allow"}`. That is a fail-open against the v4 blocked-pattern contract and **ADR-0013**.

**Proposed fix:** Translate the glob to an RE2-safe pattern explicitly rather than reusing `fnmatch.translate`.

```python
# Before
if kind_name == "GLOB":
import fnmatch
return fnmatch.translate(value) # -> '(?s:.*\\.txt)\\Z' (Python regex, not RE2)

# After
if kind_name == "GLOB":
out = ["^"]
for ch in value:
if ch == "*":
out.append(".*")
elif ch == "?":
out.append(".")
else:
out.append(re.escape(ch))
out.append("$")
return "".join(out) # RE2-safe; prepend "(?s)" as a leading flag if dot-all is needed
```

Add a unit test asserting the output compiles under RE2/OPA.

### Orphaned temp bundle dir on render failure — severity **low**, confidence **high**

**Location:** `governance_to_acs_manifest`, lines 304–346.

**What's wrong:** The function `mkdtemp`s a fresh bundle dir (307), copies stock `.rego` files in (312–315), then calls `_pattern_to_regex` per blocked pattern (317) and writes the generated module (346). `_pattern_to_regex` raises `ValueError` on bad input and `write_text` can raise `OSError`. None of this is wrapped in `try/finally`, so any failure after 307 leaks an `agt_bridge_*` dir containing a half-built bundle (stock libs present, generated `.rego` missing).

**Why it matters:** Repeated calls accumulate orphaned temp dirs, and a leftover dir *looks* like a valid bundle but lacks the policy module, so a later consumer fails OPA load rather than failing closed cleanly.

**Proposed fix:** Only clean up a self-created temp dir, then re-raise.

```python
created = bundle_dir is None
bundle_dir = (
Path(bundle_dir).resolve()
if not created
else Path(tempfile.mkdtemp(prefix="agt_bridge_")).resolve()
)
try:
# copy stock libs, translate blocked_patterns, write generated module
...
except Exception:
if created:
shutil.rmtree(bundle_dir, ignore_errors=True)
raise
```

Alternatively, validate/translate all `blocked_patterns` *before* creating the temp dir so bad input never produces an artifact.

### Non-finite `confidence_threshold` (`inf`) renders invalid Rego/JSON — severity **low**, confidence **high**

**Location:** `_render_rego`, lines 186–189; `governance_to_acs_manifest`, line 338.

**What's wrong:** `confidence_threshold` flows into `json.dumps(confidence_threshold)` (188). With the default `allow_nan=True`, `json.dumps(float('inf'))` emits the bare token `Infinity`. `NaN` is filtered by the `> 0` guards, but `float('inf') > 0` is `True`, so `inf` reaches the renderer and produces `confidence.deny_if_low_confidence(Infinity)` — invalid Rego and invalid JSON. The same applies to budget thresholds at line 183.

**Why it matters:** The bundle fails to compile, surfacing as a load error rather than a clean deny.

**Proposed fix:** Reject non-finite values at generation time and disable `allow_nan`.

```python
import math

if confidence_threshold is not None and not math.isfinite(confidence_threshold):
raise ValueError(f"confidence_threshold must be finite, got {confidence_threshold!r}")

branches.append(
f"v := confidence.deny_if_low_confidence({json.dumps(confidence_threshold, allow_nan=False)})"
)
```

---

## `policies/runtime.py`

### `_run_sync(timeout=None)` inside a running loop spawns an unbounded thread and blocks forever — severity **medium**, confidence **medium**

**Location:** `_run_sync`, lines 604–657 (esp. 613–617, 657).

**What's wrong:** When `timeout is None` *and* a loop is already running, `asyncio.get_running_loop()` does **not** raise, so the `if timeout is None` block falls through without returning. Execution reaches the worker-thread path; because `timeout is None`, the slot/guard block (620–627) is skipped (`slot_acquired` stays `False`), and at 657 `thread.join(timeout)` becomes `thread.join(None)` — blocking the caller forever while a daemon worker drives a second loop. The primary `_run_sync(_evaluate())` call at line 309 is untimed, so any host calling `evaluate_intervention_point` from within an existing asyncio loop hits this.

**Why it matters:** An unbounded daemon thread with no semaphore accounting plus a potentially indefinite block bypasses the `_TIMED_RUN_SYNC_MAX_WORKERS` cap the docstring promises. An indefinite hang is not a defined fail-closed outcome under **ADR-0013**.

**Proposed fix:** Make the `timeout=None` branch total. Take the worker-thread path only for finite timeouts; for `timeout=None` with a running loop, run the coro on a fresh loop in a slot-guarded worker with a finite default timeout, or raise a defined error mapped to a fail-closed deny. Never call `thread.join(None)`.

```python
if timeout is None:
try:
asyncio.get_running_loop()
except RuntimeError:
return asyncio.run(coro) # no running loop: safe
# running loop present: do NOT fall through to an untimed join
timeout = _DEFAULT_INLOOP_TIMEOUT # finite, slot-guarded below
```

### Resolver wrapper awaits a never-set `Event` on event-loop binding error — severity **medium**, confidence **high**

**Location:** `_make_acs_resolver._resolve`, lines 563–566.

**What's wrong:** When the host callback raises a `RuntimeError` matching the event-loop-binding heuristic, the code runs `await asyncio.Event().wait()` on an `Event` that is never set — intentionally blocking forever before an unreachable `raise`. Only the outer `_run_sync` timeout unblocks it. With the default `approval_timeout_seconds` (300s) the worker is pinned for up to that long; reached via the untimed `_run_sync` (e.g. the `_evaluate` path) it blocks indefinitely. The original `RuntimeError` context is also discarded.

**Why it matters:** Under **ADR-0013** an event-loop binding error should resolve to a deterministic closed/deny outcome, not an unbounded await that depends on an external timeout. `evaluate_intervention_point` already detects this condition (`_is_event_loop_binding_error` → deny at 336–338), but only because `_enforce` is timed; the wrapper itself encodes the hang.

**Proposed fix:** Surface the binding condition deterministically; do not block.

```python
# Before
except RuntimeError as exc:
if _is_event_loop_binding_error(exc):
await asyncio.Event().wait() # blocks until outer timeout
raise

# After
except RuntimeError as exc:
# Let evaluate_intervention_point's _is_event_loop_binding_error
# handling map this to a fail-closed deny immediately.
raise
```

### Dead helper `_approval_settings_from_manifest_text` duplicates divergent parsing — severity **low**, confidence **high**

**Location:** `_approval_settings_from_manifest_text`, lines 407–411 (also `_parse_and_sanitize_manifest_text`, 397–404).

**What's wrong:** `__init__` never calls `_approval_settings_from_manifest_text`; it parses via `_parse_and_sanitize_manifest_text` then `_approval_settings_from_manifest(parsed)`. The standalone helper re-implements the same `yaml.safe_load(...) or {}` + `Mapping` handling — dead/divergent code that can drift from the real path and is uncovered by the evaluation flow. Separately, `yaml.safe_load` can raise `yaml.YAMLError`; neither parser wraps it, so a malformed manifest surfaces a raw `YAMLError` out of `__init__` rather than a governed error.

**Why it matters:** Maintainability (silent divergence) plus an ungoverned exception surface on construction.

**Proposed fix:** Delete the unused helper (or route `__init__` through it) and wrap the load in a typed/governed error.

```python
def _parse_and_sanitize_manifest_text(manifest_text: str) -> Mapping[str, Any]:
try:
parsed = yaml.safe_load(manifest_text) or {}
except yaml.YAMLError as exc:
raise ManifestError(f"invalid manifest YAML: {exc}") from exc
...
```

### `TemporaryDirectory` cleanup only via explicit `close()` — severity **low**, confidence **medium**

**Location:** `AgtRuntime.__init__` (232–233), `AgtRuntime.close` (389–394).

**What's wrong:** When `resolution_root` is set, `__init__` creates a `tempfile.TemporaryDirectory` stored on `self._resolution_bundle_dir`, cleaned up only in `close()`. `AgtRuntime` is not a context manager and has no `__del__`/`weakref.finalize`, so any host that constructs it and never calls `close()` (caller exception, forgotten cleanup) leaks the bundle dir for the process lifetime. Construction-time failures are already handled (try/except at 234–253); successful construction without `close()` is not.

**Why it matters:** Long-lived governance hosts accumulate temp dirs.

**Proposed fix:** Add context-manager support and/or a finalizer.

```python
bundle_dir = tempfile.TemporaryDirectory(prefix="agt_runtime_bundle_")
self._resolution_bundle_dir = bundle_dir
weakref.finalize(self, bundle_dir.cleanup)

def __enter__(self): return self
def __exit__(self, *exc): self.close()
```

---

## `policies/snapshot.py`

### Budget validation accepts `NaN`/`Infinity` — severity **high**, confidence **high**

**Location:** `_validate_budget_counter` (50–56); also `_envelope` (76–82), `record_cost` (362–366), `record_elapsed` (368–372).

**What's wrong:** For the float counters (`elapsed_seconds`, `cost_usd`) the only numeric guard is `value < 0`. `float('nan') < 0` and `float('inf') < 0` are both `False`, so `NaN`/`+Inf` pass validation into `envelope.budgets`. The same hole exists in `record_cost`/`record_elapsed`: `isinstance(usd,(int,float)) and usd < 0` lets `NaN` through, then `self.cost_usd += float(nan)` poisons the running budget irrecoverably. (`-Inf` does trip the `< 0` check; `+Inf` and `NaN` do not.)

**Why it matters:** AGT-SNAPSHOT-1.0 §3 requires the snapshot to round-trip through JSON without loss with ECMA-262 float serialization, but `json.dumps(float('nan'))` emits the non-standard `NaN` token, which is invalid JSON and breaks any canonical-bytes/action-identity SHA-256 (**ADR-0004**). Worse, the engine reads budgets at evaluation start, and every `>=` comparison in `agt.budgets.deny_if_budget_exceeded` returns `False` against a `NaN` budget — the deny rule silently stops firing (fail-open, **ADR-0013**).

**Proposed fix:** Reject non-finite values explicitly, and guard the mutators before `+=`.

```python
import math

# _validate_budget_counter (float branch)
if (isinstance(value, bool)
or not isinstance(value, (int, float))
or not math.isfinite(value)
or value < 0):
raise ValueError(f"{name} must be a non-negative finite number, got {value!r}")

# record_cost / record_elapsed
if isinstance(usd, bool) or not isinstance(usd, (int, float)) or not math.isfinite(usd) or usd < 0:
raise ValueError(...)
self.cost_usd += float(usd)
```

### Caller-supplied mutable bodies are aliased, not copied — severity **medium**, confidence **medium**

**Location:** `pre_model_call_snapshot` (156–157), `post_model_call_snapshot` (179–180), `pre_tool_call_snapshot` (194), `post_tool_call_snapshot` (221–222), `output_snapshot` (238/241), `input_snapshot` (132, dict body).

**What's wrong:** These helpers store the caller's mutable objects by reference while sibling fields in the *same* function are defensively copied (`dict(model_params or {})` at 155, `dict(headers or {})` at 132). The intra-function asymmetry shows the aliasing is unintentional: `"messages": messages`, `"tools": tools or []`, `"args": args`, `"tool_result": {"value": result, ...}` are all stored live.

**Why it matters:** AGT-SNAPSHOT-1.0 §3 mandates that the same logical state produce the same snapshot bytes (the basis for the action-identity SHA-256). The host pattern is to build a `pre_*` snapshot, then run the model/tool — which commonly appends to `messages`, mutates `args`, or fills `response`. The already-emitted snapshot's bytes then change after the fact, so the same logical pre-call state can hash to two different action identities and an audit record can be retroactively altered (**ADR-0004**).

**Proposed fix:** Deep-copy mutable inputs at build time, consistent with the existing `dict(... or {})` pattern. A shallow copy is insufficient for nested message/arg structures.

```python
import copy
# Before
"messages": messages,
"tools": tools or [],
# After
"messages": copy.deepcopy(messages),
"tools": copy.deepcopy(tools or []),
# likewise: args, response, body/content/result/message_chain when containers
```

### Mutators accept `bool` where the constructor rejects it — severity **low**, confidence **high**

**Location:** `record_tool_call` (343–354), `record_tokens` (356–360), `record_cost` (362–366), `record_elapsed` (368–372).

**What's wrong:** `_validate_budget_counter` (used by `__post_init__`/`_envelope`) explicitly rejects `bool` for every counter. The four mutators do not: `isinstance(count, int)` is `True` for `True`/`False` (bool subclasses int), and `isinstance(usd,(int,float))` likewise. So `record_tokens(True)` silently adds 1, `record_cost(True)` adds 1.0 — values the same builder would reject at construction.

**Why it matters:** It violates the internal contract established by `_validate_budget_counter` and the `count: int`/`usd: float` annotations, letting an accidental truthy flag corrupt a running budget without error.

**Proposed fix:** Route the mutators through `_validate_budget_counter`, or add the same `bool` rejection.

```python
def record_tokens(self, tokens: int) -> None:
_validate_budget_counter("token_count", tokens) # rejects bool + negatives uniformly
self.token_count += tokens
```

### Module-level helpers accept empty/non-string `agent_id` — severity **low**, confidence **medium**

**Location:** `_envelope` (59–112); reachable from every `*_snapshot` helper (e.g. `input_snapshot`, line 129).

**What's wrong:** `_envelope` validates the four budget counters but never validates `agent_id`/`session_id`, unlike `SnapshotBuilder.__post_init__` (332–335) which requires a non-empty string. `input_snapshot(agent_id="", ...)` yields `envelope.agent.id == ""` and, via `agent_name or agent_id` (88), `envelope.agent.name == ""`.

**Why it matters:** AGT-SNAPSHOT-1.0 §1 marks `envelope.agent.id` and `envelope.session.id` as required stable identifiers. An empty id is a structurally-present-but-meaningless identifier that defeats per-agent/per-session policy scoping and audit correlation, inconsistent with the builder path.

**Proposed fix:** Hoist the non-empty-string check into `_envelope`.

```python
for name, val in (("agent_id", agent_id), ("session_id", session_id),
("intervention_point", intervention_point)):
if not isinstance(val, str) or not val:
raise ValueError(f"{name} must be a non-empty string")
```

---

## `manifest_resolution/merge.py`

### Parent deny with "unsatisfiable"-classified condition is silently neutralized (fail-open) — severity **medium**, confidence **medium**

**Location:** `merge_documents` blocking_deny check (323–342) via `_conditions_overlap` (270–273), `_conditions_disjoint` (244–246), `_condition_unsatisfiable` (225–241).

**What's wrong:** `_conditions_disjoint()` short-circuits to `True` (disjoint) whenever *either* side is deemed unsatisfiable: `if _condition_unsatisfiable(left) or _condition_unsatisfiable(right): return True` (245). In the blocking_deny path the **parent deny's** condition is `left`. `_condition_unsatisfiable` misclassifies real deny conditions — most clearly an empty `or` list: line 237 `return not or_items or all(...)` returns `True` for `{'or': []}`. A deny judged unsatisfiable is treated as non-overlapping with every child allow, so `_conditions_overlap` returns `False` and the child allow is **not** dropped.

**Why it matters:** The parent-level deny is neutralized by a more-specific manifest — exactly the immutability **ADR-0014** forbids — and evaluation can resolve to `allow` on a deny path (**ADR-0013**). Reproduced end-to-end: `merge_documents([{rules:[{name:'org-deny',action:'deny',condition:{'or':[]},priority:100}]}, {rules:[{name:'child-allow',action:'allow',condition:{field:'tool',operator:'eq',value:'shell'},priority:1}]}])` returns *both* rules; the child allow survives. Any false-positive in the heuristic on a deny condition becomes a fail-open hole, since its errors are not biased toward "overlap."

**Proposed fix:** Never let unsatisfiability bias the security-critical path toward "not overlapping." In `_conditions_disjoint`, only honor the unsatisfiable short-circuit for the **child** side, never the parent-deny side; and make the analyzer conservative — return `True` (overlap) on any structural form it cannot fully reason about.

```python
# Before
def _conditions_disjoint(left, right):
if _condition_unsatisfiable(left) or _condition_unsatisfiable(right):
return True
...

# After: parent-deny (left) unsatisfiability must NOT declare it harmless
def _conditions_disjoint(left, right, *, child_side):
if _condition_unsatisfiable(child_side): # only the child allow may be pruned this way
return True
...
# and: _condition_unsatisfiable({'or': []}) -> False unless the spec truly defines empty-or
# as a contradiction; default unknown shapes to satisfiable so overlap is assumed.
```

### Unhandled `TypeError` sorting rules when `priority` is null/non-numeric — severity **medium**, confidence **high**

**Location:** `merge_documents`, line 310 (single-doc) and 373 (multi-doc): `key=lambda r: r.get('priority', 0)`.

**What's wrong:** `r.get('priority', 0)` only substitutes `0` when the key is *absent*. An explicit `priority: null` in YAML yields Python `None`; a typo `priority: high` yields `str`. Sorting then raises `TypeError: '<' not supported between instances of 'NoneType'/'str' and 'int'`. Reproduced on both single- and multi-doc paths.

**Why it matters:** The `TypeError` is not a `ResolutionError`, so it is neither documented in the `Raises` section nor guaranteed to be translated by the host's fail-closed mapping. Per **ADR-0013** a malformed rule must fail closed via `ResolutionError.invalid_governance`; an arbitrary `TypeError` may surface as a 500/crash instead.

**Proposed fix:** Validate priority up front (preferred) or make the sort key total-order safe.

```python
# Preferred: validate in the upfront loop (297-306)
prio = rule.get("priority", 0)
if isinstance(prio, bool) or not isinstance(prio, (int, float)):
raise ResolutionError.invalid_governance(f"rule {rule.get('name')!r} has non-numeric priority {prio!r}")

# Or make the sort total-order safe:
def _prio(r):
p = r.get("priority", 0)
return p if isinstance(p, (int, float)) and not isinstance(p, bool) else 0
rules.sort(key=_prio, reverse=True)
```

---

## `manifest_resolution/build.py`

### `matches`/`regex` operator emits an unvalidated pattern → OPA eval error → default `allow` (fail-open) — severity **high**, confidence **medium**

**Location:** `_rego_op_clause`, lines 364–369.

**What's wrong:** For operators `matches`/`regex` the rendered body is `regex.match({literal}, _v)`, where `literal` is `json.dumps` of the policy author's value. The pattern is never validated. If it is not valid Go RE2, OPA's `regex.match` raises an evaluation error; the conditional branch becomes undefined-or-errored and falls through to `default verdict := {"decision":"allow"}` (215). Unlike the unsupported-operator path (233–258), which deliberately renders an always-matching deny to fail closed, the malformed-regex path is a normal branch and therefore fails **open**.

**Why it matters:** A policy author — or an attacker who controls a `governance.yaml` value — can disable a deny rule by supplying a deliberately invalid regex. This contradicts the in-code "last line of defense" fail-closed comment (234–238) and **ADR-0013**.

**Proposed fix:** Validate at render time; on failure, route through the same fail-closed always-matching deny used for unsupported operators. Require `value` to be `str`.

```python
if operator in {"matches", "regex"}:
if not isinstance(value, str):
return None # -> unsupported-operator fail-closed deny branch
try:
re.compile(value) # plus an RE2-compatibility check where feasible
except re.error:
return None # -> fail-closed deny branch, not a normal branch
return (
f"{indent}_v := {accessor}\n"
f"{indent}_v != null\n"
f"{indent}regex.match({literal}, _v)"
)
```

### `json.dumps(value)` on YAML-sourced condition values raises `TypeError` for dates/binary — severity **high**, confidence **high**

**Location:** `_rego_op_clause`, line 326 (`literal = json.dumps(value)`); also `_render_rego` lines 253, 270–272.

**What's wrong:** `value` comes straight from `yaml.safe_load` (38 → `cond.get('value')` at 226). YAML `safe_load` natively produces `datetime.date`/`datetime` for timestamp scalars (e.g. `value: 2026-06-23`) and `bytes` for `!!binary`. `json.dumps` of a `datetime`/`date`/`bytes` raises `TypeError`, uncaught here, propagating out of `resolve_manifest` as a raw `TypeError`. The same exposure exists for `action`/`name`/`message` via `json.dumps` at 270–272.

**Why it matters:** It breaks the documented `Raises: ResolutionError` contract and risks bypassing the host's fail-closed translation (**ADR-0013**).

**Proposed fix:** Coerce non-JSON scalars deterministically and/or validate condition values to JSON primitives before rendering.

```python
# Minimal: coerce deterministically
literal = json.dumps(value, default=str)

# Better: validate at the merge/validation layer that condition values are
# str/int/float/bool/None, else raise ResolutionError.invalid_governance(...)
```

### `_load_yaml` only catches `YAMLError`; `OSError` escapes the contract — severity **medium**, confidence **high**

**Location:** `_load_yaml`, lines 35–49 (try/except at 36–42).

**What's wrong:** The `except` catches only `yaml.YAMLError`. `path.open()`/read can raise `OSError` (file removed between discovery and load, permission denied, ENOENT on a dangling symlink) and decoding raises `UnicodeDecodeError` (a `ValueError`, not `YAMLError`). These propagate raw out of `resolve_manifest`.

**Why it matters:** `resolve_manifest`'s docstring (82–84) promises `Raises: ResolutionError`, and `errors.py` documents that the host translates `ResolutionError` into a fail-closed deny. A non-`ResolutionError` exception bypasses that translation, making fail-closed behavior depend on a broad `except` in the untrusted host wrapper — an **ADR-0013** gap for a parse/IO failure.

**Proposed fix:** Widen the `except`.

```python
try:
with path.open("r", encoding="utf-8") as fh:
data = yaml.safe_load(fh)
except (yaml.YAMLError, OSError, UnicodeDecodeError) as exc:
raise ResolutionError.invalid_governance(f"failed to read/parse {path}: {exc}") from exc
```

### `_materialize_rego_bundle`: `OSError` escapes and writes are non-atomic / can desync — severity **medium**, confidence **high**

**Location:** `_materialize_rego_bundle`, lines 188–199.

**What's wrong:** `mkdir(parents=True)` and the two `write_text` calls can raise `OSError` (read-only fs, ENOSPC, permission), propagating raw out of `resolve_manifest`. Separately, `write_text` is not atomic: a crash/ENOSPC mid-write at 194 leaves a truncated `agt_legacy.rego`; and if the `.rego` write (194) succeeds but the `.sha256` write (197) fails, the bundle has a rego file with a missing/stale integrity sidecar, so a downstream integrity check may reject a valid bundle or accept a partially written one. `policy_dir` is returned and referenced by the manifest even when the second write failed.

**Why it matters:** Contract leak (`Raises: ResolutionError`) plus an integrity-desync hazard against **ADR-0013**.

**Proposed fix:** Wrap in `try/except OSError → ResolutionError`, and write atomically (temp file + `os.replace`).

```python
try:
policy_dir.mkdir(parents=True, exist_ok=True)
body = _render_rego(rules)
digest = hashlib.sha256(body.encode("utf-8")).hexdigest()

def _atomic_write(target: Path, text: str) -> None:
tmp = target.with_suffix(target.suffix + ".tmp")
tmp.write_text(text, encoding="utf-8")
os.replace(tmp, target)

_atomic_write(policy_dir / "agt_legacy.rego.sha256", digest) # sidecar first
_atomic_write(policy_dir / "agt_legacy.rego", body)
except OSError as exc:
raise ResolutionError.invalid_governance(f"failed to materialize bundle in {policy_dir}: {exc}") from exc
```

### Always-matching fail-closed deny matcher makes every lower-priority rule dead code — severity **low**, confidence **high**

**Location:** `_render_rego`, lines 244–258 (matcher `_match_{idx} if { true }`) and negation chains at 255, 265–267.

**What's wrong:** For an unsupported-operator / invalid-field rule the matcher renders as `_match_{idx} if { true }` (always true). Every later rule `j>idx` carries `not _match_{idx}` in its negation chain (265–267), which is always false, so no rule after the first invalid rule can ever match. The first invalid rule swallows the entire lower-priority tail.

**Why it matters:** The branch verdict is still `deny` (so not a fail-open), but the reported reason/message is wrong (it names the invalid rule, not the actual matching deny), and valid downstream rules become unreachable dead code. The verdict reason then depends on the priority position of an *unrelated* invalid rule — a determinism/correctness defect (**ADR-0004**).

**Proposed fix:** Don't let the invalid rule swallow the tail. Scope-bound it to its own field/condition, or — preferably — reject the manifest up front.

```python
# Preferred: fail at resolution time rather than emitting a catch-all
if operator not in SUPPORTED_OPERATORS or field is None:
raise ResolutionError.invalid_governance(
f"rule {name!r} uses unsupported operator/field; cannot render fail-closed"
)
```

### `tempfile.mkdtemp` bundle directory is never cleaned up and leaks on error paths — severity **low**, confidence **medium**

**Location:** `resolve_manifest`, line 109 (`bundle_path = ... mkdtemp(...)`); `_materialize_rego_bundle` call at 110.

**What's wrong:** When `bundle_dir is None`, a fresh temp dir is `mkdtemp`'d on every call and never removed. A long-running host leaks unbounded `agt_resolved_bundle_*` dirs. If `_materialize_rego_bundle` (110) or the binding check (112–116) raises after `mkdtemp`, the just-created dir is orphaned — no `try/finally`.

**Why it matters:** Disk leak; orphaned partial bundles.

**Proposed fix:** Track ownership and clean up on failure; defer materialization until after the binding check so a validation failure creates no dir.

```python
created = bundle_dir is None
bundle_path = bundle_dir or Path(tempfile.mkdtemp(prefix="agt_resolved_bundle_"))
try:
intervention_points = _collect_intervention_points(docs_only)
if merged_rules and not _binds_legacy_rules(intervention_points):
raise ResolutionError.invalid_governance(...)
rego_path = _materialize_rego_bundle(bundle_path, merged_rules)
except Exception:
if created:
shutil.rmtree(bundle_path, ignore_errors=True)
raise
```

---

## `manifest_resolution/discover.py`

### `OSError` from `Path.resolve()` escapes as non-`ResolutionError` — severity **medium**, confidence **high**

**Location:** `discover_policies`, lines 50–51.

**What's wrong:** `root.resolve()` and `action_path.resolve()` perform filesystem IO with no error handling and can raise `OSError` (`PermissionError`, `ELOOP` on symlink cycles, `ENAMETOOLONG`) — and `RuntimeError` on infinite symlink loops in some CPython versions — for pathological/attacker-controlled `action_path`.

**Why it matters:** The docstring (44–48) declares the only raised exception is `ResolutionError(PATH_TRAVERSAL)`, and `errors.py` (3–9) requires the host to translate `ResolutionError` into a deny. A raw `OSError`/`RuntimeError` bypasses that translation — an **ADR-0013** fail-closed-bypass that also contradicts the documented contract.

**Proposed fix:** Wrap the resolves and re-raise as `ResolutionError`.

```python
try:
root = root.resolve()
action_path = action_path.resolve()
except OSError as exc:
raise ResolutionError.path_traversal(f"failed to resolve action_path under {root}: {exc}") from exc
```

### `OSError` from `is_file()` stat calls during the walk escapes — severity **medium**, confidence **high**

**Location:** `discover_policies`, lines 53 and 67 (and `candidate.resolve()` at 68).

**What's wrong:** `action_path.is_file()` (53) and `candidate.is_file()` (67) each `stat()` and can raise `OSError` (`PermissionError`, `ELOOP`, Windows `ERROR_ACCESS_DENIED`) rather than returning `False`. These are unguarded inside the walk; `candidate.resolve()` (68) has the same exposure.

**Why it matters:** Same contract/fail-closed violation as above — a non-`ResolutionError` escapes the documented `Raises` and the host's `ResolutionError`-keyed translation (**ADR-0013**).

**Proposed fix:** Convert IO errors to `ResolutionError`, ideally wrapping the post-validation walk body.

```python
try:
if action_path.is_file():
action_path = action_path.parent
for ...:
if candidate.is_file():
resolved_candidate = candidate.resolve()
...
except OSError as exc:
raise ResolutionError.path_traversal(f"failed to walk governance chain under {root}: {exc}") from exc
```

### Non-existent / non-file-non-dir `action_path` is silently walked as a directory — severity **low**, confidence **medium**

**Location:** `discover_policies`, lines 51–54.

**What's wrong:** After `action_path.resolve()` (with default `strict=False`, which does not require existence), the code only branches on `is_file()`. A non-existent path, a broken symlink, or a special file (socket/FIFO) all yield `is_file()==False` and are used as a starting directory. For a non-existent path, discovery walks the ancestors and may return a non-empty chain (governance files in real ancestor dirs), masking a caller bug.

**Why it matters:** Silent misbehavior — discovery succeeds against a path that does not exist, contrary to the docstring framing (36–37) that `action_path` is where the agent action originates.

**Proposed fix:** Validate existence / branch on `is_dir()` vs `is_file()`.

```python
if not action_path.exists():
raise ResolutionError.path_traversal(f"action_path {action_path} does not exist")
if action_path.is_file():
action_path = action_path.parent
elif not action_path.is_dir():
raise ResolutionError.path_traversal(f"action_path {action_path} is neither a file nor a directory")
```

---

## `manifest_resolution/scope.py`

### `fnmatch` makes scope matching platform-dependent (case + separator) — severity **high**, confidence **high**

**Location:** `filter_by_scope`, line 53 (fnmatch branch); normalization at 46–47.

**What's wrong:** The function normalizes both the action path and the scope pattern to forward slashes (46–47) and the docstring promises cross-platform consistency. But line 53 calls `fnmatch`, not `fnmatchcase`. `fnmatch` runs both arguments through `os.path.normcase` first. On Windows, `normcase` lowercases *and* rewrites `/`→`\` (`ntpath.normcase('src/foo/bar') -> 'src\\foo\\bar'`), so it (a) silently undoes the forward-slash normalization and (b) makes matching case-insensitive on Windows but case-sensitive on POSIX. Verified: `fnmatch('Foo/bar','foo/*') == True` vs `fnmatchcase('Foo/bar','foo/*') == False`.

**Why it matters:** The same inputs produce different filter results by host OS. Scope filtering decides whether a governance document (possibly carrying deny rules) applies, so a document that should match can be dropped (or an unintended one kept) depending on platform/case — non-deterministic evaluation across hosts, violating **ADR-0004** and the docstring's cross-platform claim.

**Proposed fix:** Use the case-sensitive, `normcase`-free matcher.

```python
from fnmatch import fnmatchcase
# Before: return fnmatch(action_rel, normalized_scope)
return fnmatchcase(action_rel, normalized_scope)
```

### Empty-string scope pattern silently matches nothing, dropping the document — severity **medium**, confidence **high**

**Location:** `filter_by_scope`, lines 41–53.

**What's wrong:** Only `scope_pattern is None` triggers the always-applies short-circuit (41). An empty string `""` (or whitespace-only, e.g. `scope: ""` or a loosely-parsed `scope:`) is not `None`, so `"".replace(...) -> ""` doesn't end with `/`, and line 53 evaluates `fnmatch(action_rel, "")`. An empty glob matches nothing for any non-empty path (verified: `fnmatch('a/b','') == False`), so the document is silently filtered out of `scoped_docs` with no diagnostic.

**Why it matters:** For a document carrying parent deny rules, silently dropping it weakens enforcement — a malformed/empty scope should not be a path to discarding deny-bearing parent documents (**ADR-0014**). Intended behavior is unspecified in code, but silent never-matching is a latent bug.

**Proposed fix:** Treat blank scope as absent, or reject it explicitly.

```python
if scope_pattern is None or not scope_pattern.strip():
return True # blank/whitespace scope means "always applies"
# OR: raise ResolutionError.invalid_governance("scope must be a non-empty pattern")
```

### Unguarded `Path.relative_to` raises `ValueError` (wrong exception type) — severity **low**, confidence **medium**

**Location:** `filter_by_scope`, line 46.

**What's wrong:** `action_path.relative_to(root)` is called with no guard and raises `ValueError` when `action_path` is not under `root`. `filter_by_scope` is a public, exported API whose signature documents only a `bool` return with no exceptions, so a direct caller (or one whose `action_path`/`root` drift out of containment) gets an uncaught `ValueError`. `discover.py` treats the analogous out-of-root condition as a security event, wrapping it in `_is_relative_to` and raising `ResolutionError.path_traversal` ("v5 fails closed"). The resolved values passed here from `build.py` are not guaranteed identical to `discover_policies`' internally re-resolved values, so containment is not provably re-established.

**Why it matters:** A bare `ValueError` is not the reserved fail-closed signal; under **ADR-0013** the resolution layer must surface a fail-closed deny, and an over-broad upstream handler could fail open.

**Proposed fix:** Guard containment and raise the reserved error, mirroring `discover.py`.

```python
try:
rel = action_path.relative_to(root)
except ValueError:
raise ResolutionError.path_traversal(
f"action_path {action_path} is not under workspace root {root}"
)
action_rel = str(rel).replace("\\", "/")
```

---

## `_harness/opa_runner.py`

### Missing `decision` key defaults to `allow` (fail-open) — severity **medium**, confidence **high**

**Location:** `run_scenario`, line 270 (decode block 263–277).

**What's wrong:** After confirming `value` is a dict (266), the runner reads `str(value.get("decision", "allow"))`. A dict-shaped verdict lacking a `decision` field (malformed/partial/unexpected shape, or a future engine that renames the field) is silently treated as `allow` (`is_allow` true). The valid set is `allow|warn|deny|escalate|transform` (`policies/result.py` `Verdict`).

**Why it matters:** A dict without a recognized decision is an error condition. The generated Rego always carries `decision` in the happy path (`default verdict := {"decision": "allow"}`), so a dict missing `decision` is an anomaly that must fail **closed** under **ADR-0013** — not default to `allow`.

**Proposed fix:** Require an explicit, recognized decision; fail closed otherwise.

```python
decision = value.get("decision")
VALID = {"allow", "warn", "deny", "escalate", "transform"}
if decision not in VALID:
return ScenarioResult(
decision="deny",
reason="runtime_error:engine_invalid_verdict",
message=f"opa returned verdict without recognized decision: {value!r}",
raw=value,
)
```

### OPA empty `result` list crashes with unhandled `IndexError` — severity **medium**, confidence **high**

**Location:** `run_scenario`, lines 264–265.

**What's wrong:** `response.get("result", [{}])[0]` defaults only the *missing-key* case to `[{}]`. When OPA produces no bindings it returns `{"result": []}` (normal for an undefined/empty result set), and `[][0]` raises `IndexError`, unhandled and undocumented.

**Why it matters:** An empty `result` is "no result," which the docstring says is a `RuntimeError`; instead it crashes with `IndexError`. Under **ADR-0013** it must resolve to a closed (deny) verdict, not an arbitrary exception.

**Proposed fix:** Guard the list explicitly.

```python
results = response.get("result") or []
if not results:
raise RuntimeError("opa eval produced no result") # or return a deny ScenarioResult
expressions = results[0].get("expressions") or []
value = expressions[0].get("value") if expressions else None
```

### `json.loads(proc.stdout)` raises undocumented `JSONDecodeError` on non-JSON output — severity **low**, confidence **high**

**Location:** `run_scenario`, line 263.

**What's wrong:** `response = json.loads(proc.stdout)` runs whenever `returncode == 0`. OPA can exit 0 yet emit non-JSON stdout (partial output, a warning banner, or an empty string). `json.loads('')`/`json.loads('')` raise `json.JSONDecodeError`, propagating uncaught.

**Why it matters:** The docstring's `Raises` lists only `FileNotFoundError`/`RuntimeError`; `JSONDecodeError` is not a `RuntimeError` subclass, so callers catching `RuntimeError` to fail closed won't catch it — the error escapes the governance boundary (**ADR-0013**).

**Proposed fix:** Normalize to a closed outcome / `RuntimeError`.

```python
try:
response = json.loads(proc.stdout)
except json.JSONDecodeError as exc:
raise RuntimeError(f"opa produced non-JSON output: {proc.stdout[:200]!r}") from exc
```

### `_resolve_path` raises bare `KeyError`/`IndexError`/`ValueError`/`TypeError` — severity **low**, confidence **medium**

**Location:** `_resolve_path`, lines 149–155; called from `run_scenario` (222, 226).

**What's wrong:** The accessor loop does `obj = obj[idx]`/`obj = obj[part]` and `idx = int(part[1:-1])` with no handling, raising `KeyError` (absent key), `IndexError` (out of range), `TypeError` (subscripting a scalar/None), and `ValueError` (`int('abc')` or an unclosed `[` at line 140). All propagate raw out of `run_scenario`.

**Why it matters:** Undocumented (only `FileNotFoundError`/`RuntimeError` declared). Under **ADR-0013** a failed `policy_target` resolution must fail closed, not crash with an arbitrary exception type that `RuntimeError`-catching callers won't handle.

**Proposed fix:** Catch lookup failures and raise one typed error; translate to a deny in the caller.

```python
try:
obj = obj[idx] if is_index else obj[part]
except (KeyError, IndexError, TypeError):
raise RuntimeError(f"policy_target path {path!r} does not resolve in snapshot")
# wrap int(part[1:-1]) / rest.index("]", i) similarly;
# in run_scenario, convert a resolution failure to a deny ScenarioResult.
```

### Missing `tool_name_from` binding raises bare `KeyError` — severity **low**, confidence **high**

**Location:** `run_scenario`, line 226.

**What's wrong:** For `pre_tool_call`/`post_tool_call` the code does `ip_config["tool_name_from"]` (subscript, not `.get`). A manifest that binds a tool intervention point but omits `tool_name_from` (plausible since `_collect_intervention_points` does not schema-validate this key) raises a bare `KeyError("tool_name_from")`. The adjacent non-string case already raises a descriptive `RuntimeError` (228), so the missing-key case is an inconsistent gap.

**Why it matters:** Undocumented exception; under **ADR-0013** a malformed binding should fail closed with an actionable message.

**Proposed fix:** Validate the key first.

```python
tool_name_from = ip_config.get("tool_name_from")
if not isinstance(tool_name_from, str):
raise RuntimeError(f"intervention point {intervention_point!r} is missing a string 'tool_name_from'")
tool_name = _resolve_path(snapshot, tool_name_from)
```

### `result_labels` assigned `Any` without validating `list[str]` — severity **low**, confidence **medium**

**Location:** `run_scenario`, line 275 (field declared at `ScenarioResult` line 43).

**What's wrong:** `ScenarioResult.result_labels` is annotated `list[str] | None`, but line 275 assigns `value.get("result_labels")` — `Any` straight from decoded OPA JSON. A misbehaving/future policy could emit a string, dict, or list of non-strings, all stored despite the annotation. Unlike `decision` (coerced via `str(...)`), there is no coercion.

**Why it matters:** Downstream consumers that trust the annotation (e.g. iterate expecting strings) can break. No ADR, but a clear annotation-vs-actual mismatch.

**Proposed fix:** Validate or normalize before assignment.

```python
labels = value.get("result_labels")
if labels is not None and not (isinstance(labels, list) and all(isinstance(x, str) for x in labels)):
raise RuntimeError(f"opa returned non-list[str] result_labels: {labels!r}")
```

---

## `cli/migrate.py`

### `--write` backs up ALL chain governance files, including parents outside `chain_root` — severity **high**, confidence **medium**

**Location:** `_migrate_governance_chain`, lines 495–512.

**What's wrong:** `discovered` is built from `manifest['metadata']['resolved_from']['chain']`, which `resolve_manifest`/`discover_policies` populate by walking from `chain_root` UP TO `project_root` (root-first). It therefore includes parent governance files that live *above* `chain_root`. The `--write` block iterates `for gov_file in discovered` and calls `gov_file.replace(backup)` on every one, moving parent (and sibling-shared) governance files to `.governance.yaml.v4-backup`.

**Why it matters:** This mutates parent deny rules that **ADR-0014** declares immutable across merge, and breaks any sibling chain depending on the same parent file. The docstring (466–469) implies chain_root-local backups, but the discovered chain is workspace-wide.

**Proposed fix:** Only back up governance files located within `chain_root`.

```python
for gov_file in discovered:
if gov_file.parent.resolve() != chain_root.resolve():
continue # parent files are owned/migrated by their own chain root
backup = gov_file.with_name(f".{gov_file.name}.v4-backup")
if gov_file.exists():
gov_file.replace(backup)
finding.backups.append(backup)
```

### Non-atomic `--write` with no rollback leaves the project half-migrated — severity **medium**, confidence **high**

**Location:** `_migrate_governance_chain`, lines 503–512; contract in `migrate_project` docstring (803–808).

**What's wrong:** `manifest_path.write_text(...)` then the `gov_file.replace(backup)` loop run with no try/except and no atomicity. If `write_text` raises `OSError` (disk full, permission, read-only), it propagates out of `_migrate_governance_chain` and `migrate_project`, despite the docstring guarantee that "the function never raises for individual file errors." If the manifest write succeeds but `gov_file.replace` fails on the 2nd of N files, the project is left with the manifest written, some governance files renamed and some not, and no rollback. The earlier `ResolutionError` path (489–493) *is* caught; the IO write path is not — an asymmetry.

**Why it matters:** Violates the documented no-raise contract and leaves a partial, non-recoverable migration.

**Proposed fix:** Catch `OSError` into `finding.error`, write the manifest atomically, and roll back already-moved backups on failure.

```python
if write:
moved: list[tuple[Path, Path]] = []
try:
tmp = manifest_path.with_suffix(manifest_path.suffix + ".tmp")
tmp.write_text(yaml.safe_dump(manifest, sort_keys=False), encoding="utf-8")
os.replace(tmp, manifest_path) # manifest durable first
for gov_file in discovered:
if gov_file.parent.resolve() != chain_root.resolve() or not gov_file.exists():
continue
backup = gov_file.with_name(f".{gov_file.name}.v4-backup")
gov_file.replace(backup)
moved.append((backup, gov_file))
finding.backups.append(backup)
except OSError as exc:
for backup, original in reversed(moved): # roll back
backup.replace(original)
finding.error = f"write failed: {exc}"
```

### `_migrate_governance_policy` write/mkdir IO errors propagate — severity **medium**, confidence **high**

**Location:** `_migrate_governance_policy`, lines 568–579.

**What's wrong:** `policies_dir.mkdir(...)`, `governance_to_acs_manifest(...)`, and `manifest_path.write_text(...)` run with no exception handling (only the import is guarded at 562–566). Any `OSError` (permission denied, read-only, a name collision where `policies` is a file) or any bridge exception propagates through the `_migrate_governance_policy(gp, ...)` call (833) and out of `migrate_project`.

**Why it matters:** Same violation of the documented "never raises for individual file errors" guarantee. Per-file failures should land in `report.errors`, not abort the whole run.

**Proposed fix:** Wrap and record.

```python
try:
policies_dir.mkdir(parents=True, exist_ok=True)
bundle_dir = policies_dir / f"{base_name}_bundle"
manifest = governance_to_acs_manifest(
dataclasses.replace(inputs), bundle_dir=bundle_dir,
policy_id=base_name or "agt_governance_policy",
)
manifest_path.write_text(yaml.safe_dump(manifest, sort_keys=False), encoding="utf-8")
except (OSError, Exception) as exc:
return f"policy migration failed for {base_name}: {exc}" # threaded into report.errors
```

### `_coerce_bridge_inputs` accepts `bool` for int/float fields — severity **low**, confidence **high**

**Location:** `_coerce_bridge_inputs`, lines 415–431.

**What's wrong:** The `scalar_fields` guard uses `isinstance(value, expected_type)`. Because `bool` subclasses `int`, a literal-decoded `max_tokens=True` or `confidence_threshold=False` passes `isinstance(True, int)`/`isinstance(False, (int, float))` and is silently set on `_BridgeInputs` as a bool, flowing into `governance_to_acs_manifest` where int/float arithmetic is expected.

**Why it matters:** A v4 author's placeholder or typo'd flag yields a manifest with `True`/`False` where a number belongs, with no warning — and the "silently treated as default" docstring contract is violated (it is *set*, not defaulted).

**Proposed fix:** Reject `bool` for the numeric fields.

```python
if isinstance(value, bool) and expected_type is not bool:
continue # do not let True/False leak into numeric manifest fields
if not isinstance(value, expected_type):
continue
setattr(out, source_key, value)
```

### `--write-report` write is unguarded and can clobber/raise after a successful migration — severity **low**, confidence **high**

**Location:** `run_from_args`, lines 921–923.

**What's wrong:** After `migrate_project` has (in `--write` mode) already moved governance files and written manifests, `out_path.write_text(text, encoding="utf-8")` runs with no try/except and no parent-dir creation. An unwritable report path raises an uncaught `OSError`, and `run_from_args` propagates it as a traceback with a non-zero crash — even though the destructive migration already completed. The user sees a crash and may assume the migration failed. `--write-report subdir/MIGRATION.md` into a missing subdir also raises.

**Why it matters:** A report-write failure should not crash a run whose migration already succeeded.

**Proposed fix:** Create the parent dir and degrade gracefully.

```python
if args.write_report:
out_path = Path(args.write_report)
try:
out_path.parent.mkdir(parents=True, exist_ok=True)
out_path.write_text(text, encoding="utf-8")
except OSError as exc:
print(f"warning: failed to write report to {out_path}: {exc}", file=sys.stderr)
# report was already printed to stdout
```

---

## Recommended order of work

Prioritized by fail-open/security impact first, then contract leaks, then determinism, then hygiene.

**Tier 1 — fail-open holes (a deny can silently become allow):**
- [ ] `build.py` — validate `matches`/`regex` patterns and route invalid ones through the fail-closed deny branch (**high**).
- [ ] `bridge.py` — replace `fnmatch.translate` with an RE2-safe glob→regex so GLOB blocked patterns actually DENY (**high**).
- [ ] `snapshot.py` — reject `NaN`/`Infinity` in budget validation and the `record_*` mutators so `deny_if_budget_exceeded` keeps tripping (**high**).
- [ ] `merge.py` — stop unsatisfiability analysis from neutralizing a parent deny; default unknown shapes to "overlap" (**medium**, ADR-0014).
- [ ] `opa_runner.py` — fail closed when the verdict dict lacks a recognized `decision` instead of defaulting to `allow` (**medium**).
- [ ] `discover.py` — wrap `resolve()` (and the walk's `is_file()`/`resolve()`) so `OSError`/`RuntimeError` become `ResolutionError`, preserving fail-closed translation (**medium**).
- [ ] `scope.py` — guard `relative_to` and raise `ResolutionError.path_traversal` for out-of-root inputs (**low**, but on the security path).

**Tier 2 — `ResolutionError` contract leaks (raw exceptions bypass the host's fail-closed mapping):**
- [ ] `build.py` — widen `_load_yaml` except to `OSError`/`UnicodeDecodeError`; coerce/validate `json.dumps` condition values (dates/binary) (**high** + **medium**).
- [ ] `build.py` — wrap `_materialize_rego_bundle` IO and write atomically (rego/sidecar consistency) (**medium**).
- [ ] `merge.py` — validate/coerce `priority` before sorting so null/non-numeric fails as `ResolutionError`, not `TypeError` (**medium**).
- [ ] `opa_runner.py` — guard the empty-`result` `IndexError`, `json.loads` `JSONDecodeError`, `_resolve_path` lookups, and missing `tool_name_from` (**medium**/**low**).

**Tier 3 — determinism & correctness:**
- [ ] `scope.py` — switch `fnmatch` → `fnmatchcase` to remove platform-dependent scope matching (**high**, ADR-0004).
- [ ] `snapshot.py` — deep-copy caller-supplied mutable bodies so emitted snapshot bytes/action identity are stable (**medium**, ADR-0004).
- [ ] `build.py` — stop the always-true invalid-rule matcher from shadowing the lower-priority tail (**low**, ADR-0004).

**Tier 4 — runtime liveness & migration safety:**
- [ ] `runtime.py` — make `_run_sync(timeout=None)` total inside a running loop; never `thread.join(None)` (**medium**).
- [ ] `runtime.py` — remove the never-set-`Event` await in the resolver; surface the binding error for the existing deny mapping (**medium**).
- [ ] `migrate.py` — restrict `--write` backups to `chain_root`-local files (ADR-0014); make `--write` atomic with rollback; wrap per-policy and report writes (**high**/**medium**/**low**).

**Tier 5 — type-safety & resource hygiene:**
- [ ] `snapshot.py` — route `record_*` mutators through `_validate_budget_counter` (reject `bool`); validate `agent_id`/`session_id` in `_envelope`.
- [ ] `migrate.py` — reject `bool` for numeric fields in `_coerce_bridge_inputs`.
- [ ] `opa_runner.py` — validate `result_labels` as `list[str] | None`.
- [ ] `bridge.py` / `build.py` / `runtime.py` — clean up self-created temp bundle dirs on error (try/except + `rmtree`); add `weakref.finalize`/context-manager cleanup to `AgtRuntime`.
- [ ] `runtime.py` — remove (or wire up) the dead `_approval_settings_from_manifest_text` helper and wrap `yaml.safe_load` in a governed error.