Skip to content

fix(agents): Phase 3.5 review C2 + M4 — blocked_on_qwen + read_paths runtime gating#30

Merged
AVADSA25 merged 1 commit intomainfrom
fix/phase35-step9-review-c2-m4
May 3, 2026
Merged

fix(agents): Phase 3.5 review C2 + M4 — blocked_on_qwen + read_paths runtime gating#30
AVADSA25 merged 1 commit intomainfrom
fix/phase35-step9-review-c2-m4

Conversation

@AVADSA25
Copy link
Copy Markdown
Owner

@AVADSA25 AVADSA25 commented May 3, 2026

Summary

Resolves the two Phase 3 Step 9 review deferrals (C2 + M4). Both are small backend cleanups; together ~150 LOC + 6 tests.

C2 — dedicated blocked_on_qwen status

Before: Qwen-3.6 unavailability mapped to blocked_on_permission. Semantic mismatch — no permission to grant; service is just down. User would see a "grant permission" prompt with nothing to grant.

After:

  • New status blocked_on_qwen in state machine (running → blocked_on_qwen → running | aborted)
  • _run_agent Qwen exception handler uses the new status
  • Daemon's _daemon_one_tick adds a branch: when an agent is blocked_on_qwen, probe Qwen with a tiny call. If alive → auto-resume to running. No user click required.

When Qwen flickers (PM2 restart, OOM, network blip), agents auto-recover within one daemon tick (5 s). Before this fix, you'd have to manually /resume.

M4 — read_paths runtime enforcement

Before: PermissionManifest.read_paths was declared at approval time for transparency but never enforced. An agent could ask Qwen for a skill that reads /etc/passwd and permission_gate wouldn't catch it (only touches_path = write was checked).

After:

  • Action dataclass: NEW fields reads_path: bool + read_path: str
  • permission_gate checks action.read_path against read_paths ∪ global.read_paths
  • LLM next-action prompt extended: Qwen emits reads_path + read_path fields. A single action can both read AND write (e.g. read template.md, write output.md).
  • _qwen_next_action parser populates the new fields.

Side fix: permission_gate now expands ~ on BOTH sides of the path comparison. Was a latent bug for write_paths (action with ~/Documents/x wouldn't match grant glob ~/Documents/** because the action wasn't expanded). Symmetric improvement that affects both read and write enforcement.

Tests

6 new tests in tests/test_agent_runner.py:

Test What it locks
test_blocked_on_qwen_in_state_machine Status registered with proper transitions
test_run_agent_qwen_failure_uses_blocked_on_qwen _run_agent uses correct status on Qwen failure
test_daemon_resumes_blocked_on_qwen_when_qwen_alive Auto-resume probe success path
test_action_dataclass_supports_reads_path New Action fields exist
test_permission_gate_blocks_read_path_outside_grants Read enforcement actually bites
test_permission_gate_allows_read_path_in_grants Happy path with ~/Documents/** glob

Full suite

944 passed / 20 failed / 73 skipped — same 20/73 baseline, +6 from this PR.

Compat

  • Action field additions are non-breaking (defaults preserve old behavior)
  • New status blocked_on_qwen only fires from new code paths; no migration needed
  • LLM prompt is additive; older Qwen responses without reads_path parse fine (default False)
  • Plan schema unchanged

Test plan

  • 🧪 tests/test_agent_runner.py → 42 passed
  • 🧪 Full suite — 944/20/73
  • Symmetric tilde expansion in permission_gate (write side benefits too)
  • Post-merge deploy: pm2 restart codec-agent-runner codec-dashboard

🤖 Generated with Claude Code

…gating

Resolves the two Phase 3 Step 9 review deferrals.

## C2 — dedicated `blocked_on_qwen` status

Previously, Qwen-3.6 unavailability was mapped to `blocked_on_permission`.
Semantic mismatch: there's no permission to grant; the LLM service is
just down. Now:

- New status `blocked_on_qwen` in `_VALID_TRANSITIONS`
  (running → blocked_on_qwen → running | aborted)
- `_run_agent` Qwen exception handler transitions to `blocked_on_qwen`
- Daemon's `_daemon_one_tick` adds a new branch: when status=blocked_on_qwen,
  probe Qwen with a tiny call. If alive → transition to running and
  respawn. No user interaction needed.

User benefit: when Qwen flickers (PM2 restart, OOM kill, etc.), agents
auto-recover within one daemon tick without needing manual /resume.

## M4 — read_paths runtime enforcement

Previously, `PermissionManifest.read_paths` was declared at approval time
but never enforced at runtime. Now:

- `Action` dataclass: NEW fields `reads_path: bool` + `read_path: str`
- `permission_gate` checks `action.read_path` against
  `agent_grants.read_paths ∪ global_grants.read_paths`
  (parallel to write_paths logic)
- `_NEXT_ACTION_SYSTEM_PROMPT` updated: LLM now emits `reads_path` +
  `read_path` fields. Single action can read AND write
  (e.g. read template.md, write output.md)
- `_qwen_next_action` parser populates the new Action fields

Side fix: `permission_gate` now expands `~` on BOTH sides (action path
and grant glob) so `~/Documents/x` matches `~/Documents/**` correctly.
Was a latent bug for write_paths too — symmetric improvement.

## Tests

6 new tests in `tests/test_agent_runner.py`:
- `test_blocked_on_qwen_in_state_machine` — state machine has the new entry
- `test_run_agent_qwen_failure_uses_blocked_on_qwen` — _run_agent uses correct status
- `test_daemon_resumes_blocked_on_qwen_when_qwen_alive` — auto-resume on probe success
- `test_action_dataclass_supports_reads_path` — Action fields exist
- `test_permission_gate_blocks_read_path_outside_grants` — read enforcement bites
- `test_permission_gate_allows_read_path_in_grants` — happy path

Full suite: 944 / 20 / 73 (same baseline, +6 from this PR).

## Compat

- `Action` field additions are non-breaking — defaults preserve old behavior
- New status `blocked_on_qwen` only fires from new code paths; old data
  migration not needed
- LLM prompt extension is additive; older Qwen responses (no reads_path
  field) parse correctly with the default `False`
@AVADSA25 AVADSA25 merged commit 0f212b8 into main May 3, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants