# Trace Analysis: Builder/Reviewer Approval Gate Failures

> **Date:** 2026-10-10
> **Analyzed:** `20261009T143045Z` (11,261 chars) + `20261010T143005Z` (18,721 chars) trace reports
> **Scope:** 24+ blocked builder/reviewer sessions, 20+ finding files, 100+ evidence instances
> **Status:** Diagnosis complete, fix pending Tony's approval

---

## 1. Root Cause

**The builder and reviewer profiles run headless (`-q` / `--oneshot` mode) with `approvals.mode` unset (defaults to `manual`).** Every `terminal` or `execute_code` call that matches a "dangerous pattern" triggers an approval prompt. In headless mode there is no user to approve, so the command is **denied by timeout** (default 60s) and the agent receives a `BLOCKED` result.

The builder profile's `config.yaml` currently has:

```yaml
approvals:
  destructive_slash_confirm: false
```

But **no `mode` key**, so the default `manual` mode applies. The `destructive_slash_confirm` flag only controls the `/` slash-command confirmation, not the terminal approval gate.

### Why the agent's workaround also fails

When a `terminal` call is BLOCKED, the agent (correctly) falls back to `execute_code` to accomplish the same work. But the `execute_code` tool is **also gated** — it runs Python in a subprocess, which is flagged as a dangerous pattern (`python -e` / script execution). So the fallback gets blocked too. This creates the repeated pattern:

```
BLOCKED: terminal (dangerous pattern) → retry with execute_code → BLOCKED: execute_code (approval)
```

This is the single largest source of the 24+ blocked sessions and ~120 `BLOCKED:` occurrences across 24+ sessions.

### The dangerous patterns being triggered

From the trace evidence, the specific patterns matching:

| Pattern | Why it matches |
|---|---|
| `bash -c` / `sh -c` | Builder runs `bash -c 'cd /path && python script.py'` for inline scripts |
| `python -e` / `python -c` | Agent uses `execute_code` which runs `python -c '<code>'` |
| `rm -r` / recursive delete | Builder cleans up temp files, old workspaces, test artifacts |
| `rm ... /` (root path) | Cleanup touches `/tmp/` or `/mnt/c/Users/...` paths |
| `kill -9` / `pkill` | Process cleanup for stuck background processes |
| `> /etc/` or `>> /etc/` | Config file writes (rare, 1-2 occurrences) |

### Secondary issues (from trace reports)

1. **write_file pagination paradox** (10 occurrences): `write_file` refuses to overwrite without a prior `read_file`. The builder writes to files it created in the same session but hasn't read back. This is a tool-protocol issue, not an approval issue.

2. **Terminal timeout 180s**: The builder's `terminal.timeout: 180` is too low for long-running test suites or large file operations. The reviewer has no explicit timeout (defaults to 300s).

3. **Tool loop guardrail not configured**: Both profiles have `tool_loop_guardrails: warnings_enabled: true, hard_stop_enabled: false` with empty `warn_after` / `hard_stop_after`. The trace shows 22 tool-loop warnings (30+ identical failures) that never escalate to a hard stop, so the agent keeps retrying the same blocked command.

4. **No model fallback for builder**: The trace shows the builder's model (DeepSeek via OpenRouter) hit `502 upstream_service_errored` on 2026-10-09. The fallback to `custom:brother-5090/Qwen3.6-27B-MTP-pi-reasoning-Q4_K_M` worked, but the session already had accumulated context that made recovery slow.

---

## 2. Recommended Fix

### Fix A: Set approval mode to `smart` for builder/reviewer (PRIMARY)

In `builder/config.yaml` and `reviewer/config.yaml`:

```yaml
approvals:
  mode: smart
  timeout: 60
  destructive_slash_confirm: false
```

**Why `smart` and not `off`:**

- `smart` uses an auxiliary LLM to assess risk. Low-risk commands (running Python scripts, reading files, `rm -rf /tmp/...`) are **auto-approved**. Genuinely dangerous commands (rm -rf /, fork bombs, block device writes) are **auto-denied**. Uncertain cases escalate to a manual prompt.
- `off` disables ALL approval checks. The docs explicitly warn: "Use only in trusted environments (CI/CD, containers, etc.)" and it still respects the hardline blocklist.
- The builder/reviewer work on Tony's local machine with real projects. `smart` gives the right balance: it unblocks the 95% of commands that are actually safe (running tests, cleaning temp files, reading code) while still catching anything genuinely catastrophic.

**If Tony wants zero prompts:** `mode: off` is the alternative. The hardline blocklist still protects against `rm -rf /`, fork bombs, `mkfs`, etc.

### Fix B: Configure tool loop guardrails (SECONDARY)

In both `builder/config.yaml` and `reviewer/config.yaml`:

```yaml
tool_loop_guardrails:
  warnings_enabled: true
  hard_stop_enabled: true
  warn_after: 3
  hard_stop_after: 5
```

**Why:** The traces show agents hitting the same BLOCKED command 10-30 times before giving up. With `hard_stop_after: 5`, the agent is forced to change strategy after 5 identical failures instead of burning context tokens on retries.

### Fix C: Increase terminal timeout for builder (TERTIARY)

```yaml
terminal:
  timeout: 300
```

**Why:** The builder runs test suites, builds, and large file operations. 180s is too aggressive — the trace shows `[Command timed out after 180s]` findings. 300s matches the reviewer's effective timeout.

### Fix D: Add `command_allowlist` for known-safe patterns (OPTIONAL)

If Tony wants to skip even the `smart` LLM assessment for the most common patterns:

```yaml
command_allowlist:
  - rm
  - python
  - bash
  - sh
  - pkill
  - kill
```

**Why:** These patterns are the most common triggers. Adding them to the allowlist means they're silently approved without even consulting the smart-mode LLM. **Risk:** this is broader than ideal — `rm` and `bash` allowlisted means any `rm` or `bash` command passes. The `smart` mode in Fix A already handles this more precisely. Only use Fix D if Fix A still shows blocking.

---

## 3. What This Does NOT Fix

- **write_file pagination paradox**: This is a tool-protocol issue. The builder needs to `read_file` before `write_file` on files it created. This is a prompt/skill issue, not a config issue. The fix is in the builder's system prompt or a skill that teaches the read-before-write pattern.
- **Model 502 errors**: The fallback worked. If this becomes frequent, consider adding a second fallback or pinning to a local model for reliability.
- **The kanban launch pattern**: The builder/reviewer are launched via `work kanban task t_XXX`. If the kanban launcher passes `--yolo` or sets `HERMES_YOLO_MODE=1` in the environment, that would also solve the approval issue without config changes. But this requires modifying the launcher, which is a code change, not a config change.

---

## 4. Application Steps

1. **Back up** both config files before editing
2. **Edit** `builder/config.yaml`: add `mode: smart` under `approvals:`, add `tool_loop_guardrails` block, change `terminal.timeout: 300`
3. **Edit** `reviewer/config.yaml`: same changes
4. **Restart** any running builder/reviewer sessions (or wait for next kanban dispatch)
5. **Verify** on next trace-review run: `BLOCKED:` count should drop from ~120 to near 0

---

## 5. Risk Assessment

| Change | Risk | Mitigation |
|---|---|---|
| `approvals.mode: smart` | Low — LLM assesses risk, auto-denies genuinely dangerous | Hardline blocklist still active as floor |
| `approvals.mode: off` | Medium — all prompts disabled | Hardline blocklist still active; use only if smart still blocks |
| `tool_loop_guardrails.hard_stop_after: 5` | Low — forces strategy change after 5 retries | Agent may give up too early on legitimately slow operations |
| `terminal.timeout: 300` | Low — longer wait | Commands that hang will take longer to detect |
| `command_allowlist` (optional) | Medium — broad patterns | Only apply if `smart` mode still blocks; start with specific patterns |