code-review

---
name: code-review
description: Comprehensive code review agent skill for letitloop pull requests and diffs. Enforces deterministic contract contracts, typed exceptions, zero blind except Exception blocks, Bandit SAST security checks, and triple-OS test verification.
metadata:
  author: sdageltc (@sdageltc)
  version: "1.0.0"
compatibility: Cross-platform (Python 3.11+, pytest, ruff, bandit)
---

# Code Review Protocol for letitloop

Use this skill when reviewing pull requests, diffs, or architecture changes in the `letitloop` repository.

## 1. Non-Negotiable Invariants

Every code contribution to `letitloop` MUST adhere to these 5 invariants:

1. **Typed Domain Exceptions (Zero Blind Catches)**:
   - Prohibited: `except Exception:`, `except: pass`, or unlogged exception swallowing.
   - Required: Explicit typed exceptions (`StateError`, `IllegalTransitionError`, `TimeoutError`, `OSError`, `json.JSONDecodeError`, `KeyError`, `ValueError`, `TypeError`).
   - If an error is caught during recovery, it must output diagnostic logging or be explicitly raised.

2. **Decoupled Architecture & Minimal Coupling**:
   - Subsystems (Budget, MemoryBridge, Scope, Replanner, Doctor) must avoid circular imports.
   - Core modules must import defaults from `orchestrator.config`, not from `orchestrator.cli`.
   - APIs must accept optional path/config overrides to allow hermetic unit testing without host directory side-effects.

3. **Hermetic Test Isolation**:
   - Tests must NOT pollute the host filesystem. Use `@pytest.fixture(autouse=True)` with `tmp_path` and `monkeypatch.setenv("LIL_RUN_DIR", ...)` or isolated temp paths.
   - Mock all external network calls (`urllib.request.urlopen`, socket connections) with unit test fixtures.

4. **Security & SAST Invariants**:
   - Never use weak MD5 hashing for approach or state fingerprints; use SHA-256 (`hashlib.sha256`).
   - Always whitelist URL schemes (`http://` or `https://`) before invoking `urllib.request.urlopen`.
   - When subprocess execution with `shell=True` is intentionally required for user-configured script workers, annotate the call with `# nosec B602`.

5. **Cross-Platform Parity**:
   - Code must execute reliably across macOS (Darwin / Apple Silicon), Linux, and Windows.
   - Never assume `/proc/{pid}/stat` exists; use platform-aware fallbacks for PID inspection (e.g. `ps -p <pid> -o lstart=` on macOS).
   - Handle Windows file lock semantics `(FileExistsError, PermissionError)` on file creation collisions.

## 2. Review Checklist

When reviewing a PR, evaluate:
- [ ] Are all new CLI flags covered by unit tests in `tests/`?
- [ ] Does `ruff check .` and `ruff format --check .` pass with 0 warnings?
- [ ] Does `bandit -r orchestrator/ -ll -ii` pass with 0 Medium/High findings?
- [ ] Do all 1,120+ unit and integration tests pass cleanly via `python fast_test_runner.py`?
- [ ] Is backward compatibility preserved for existing state files and contract JSON schemas?