review-checklist git:20260703.774344bA

Immutable. This exact content is served forever at /api/v1/blob/735f7f44e287379f.

---
name: review-checklist
description: Use during Claude Code reviews to classify API, IAM, cleanup, exception, concurrency, config, temp-file, and version-pin risks.
---

# Review Checklist

Prioritize findings that can break users, expand permissions, hide failures, or make parallel agent runs unsafe.

- API contract safety: are new endpoints backward compatible? Does removing a field break existing callers?
- Silent IAM expansion: any change that quietly adds new IAM permissions is a HIGH finding. It must be explicit and operator-approved.
- Exception handling: bare `except:` or overly broad `except Exception:` that discards traceback context is a MEDIUM finding. Prefer typed exceptions with `raise ... from e`.
- Cleanup safety: cleanup code must be best-effort. Use `try/finally`; never let cleanup raise and abort the cleanup sequence. `also_teardown_controller=False` is the established safe default for SkyPilot.
- Narrow exception paths: test coverage for failure paths such as submit failure, auth failure, and cleanup failure is required. Absence is a MEDIUM finding.
- Concurrent run safety: does the code handle parallel Codex runs safely? Check for file ownership assumptions and missing commit-lock patterns.
- Config injection consistency: config should resolve through one precedence-ordered path: explicit arg, then env var, then config file. Inconsistent resolution is a MEDIUM finding.
- Temp file leaks: `tempfile.mkdtemp` without cleanup on exception paths is a MEDIUM finding. Prefer `TemporaryDirectory` context manager.
- Version pins: required dependency versions should be asserted at runtime, not just documented.