| name | review |
| description | Review changes against feature/integration before opening a PR or when the user asks for review. Checks correctness, the kernel/userland boundary, edge cases, regressions, test gaps, and resource-kind checklist compliance. |
Review
Review the current changes before they leave the branch. Catch problems here,
not in PR comments.
What to review
Both uncommitted changes and commits not on the base:
git status -sb
git diff --stat
git branch --show-current
git diff feature/integration...HEAD --stat
git log feature/integration..HEAD --oneline 2>/dev/null
If the base is main, you're on the wrong line — Hades stacks on
feature/integration. Fetch first if local main is stale.
If there's nothing to review, say so and stop.
Review focus
The kernel/userland boundary (Hades-specific, highest weight)
- Does any application logic leak into
src/? A fetch, GitHub client, or
browser in the kernel is wrong — it belongs in a deployed userland image the
kernel routes to. See the hades-conventions skill.
- Does a new capability respect the three kernel jobs (govern / discover /
route) without interpreting the body? HTTP is the unifying standard.
- Is logic put in the right layer? Controller reasons about resources + ports;
only adapters touch k8s.
New resource kinds
If a kind was added, verify the full checklist (see hades-conventions):
KINDS updated; CRD in both infra/k8s/crds/ and charts/hades/templates/crds.yamlbuildX pure fn + reconcileX + wired into the loop in the right orderHADES_KINDS updated; finalizer cleanup if it owns native objects- a
system.<kind> event; a capability-gated syscall + API route if runtime-created
- a test against
FakeKubeClient
Correctness
- Does it do what it claims? Edge cases: empty inputs, single elements, missing
fields, concurrent reconcile of two Hands targeting one pod.
- Are error paths handled with the right message + status?
Regressions
npm run build && npm test green? (expect 229 tests, 2 Postgres skip)- Does a public port signature change without updating callers/adapters?
FakeKubeClient + KubeClientNode both updated for a new KubeClient method?
Testing
- New public functions / resource kinds tested?
- Edge cases, not just the happy path?
- Does a test rely on a stale dist? Rebuild (
npm run build) if unsure.
Code quality
- Leftover debug prints, commented-out code, TODO without an issue?
- 0 lint errors? (warnings for
Record<string, any> on open specs are fine)
- Docstrings factual, not rotting implementation detail?
Report findings
Classify each:
| Severity | Meaning |
|---|
| Critical | Broken behaviour, data loss, security hole, kernel-boundary violation. Must fix. |
| High | Likely bug, missing test for an important path, skipped checklist step. Should fix. |
| Medium | Minor bug, style inconsistency, fragile pattern. Fix if convenient. |
| Low | Nit, opinion. Mention, don't block. |
Summary:
## Review: [branch name]
**Files changed:** N
**Base:** feature/integration
**Verdict:** ready / needs fixes / blocked
### Findings
| Severity | File | Issue |
|----------|------|-------|
| critical | path | description |
| ... | ... | ... |
### No significant issues
(If clean, say so explicitly — don't make the user wonder if you skipped it.)
After review
- Critical/High → offer to fix before opening the PR.
- Medium/Low only → note in the PR body, proceed.
- Clean → proceed to open the PR (see make-a-pr skill).
- A durable record is sometimes wanted — post as an issue comment, not a repo
file, unless Willow asks for one.
Gotchas
- Compare against
feature/integration, not main.
- A very large branch gives a broad, less precise review — say so if so.
- "Cannot access X before initialization" in a CLI test = a
const referenced
above its definition (TDZ) — a recurring Hades footgun.
- Two Hands → one
hands-<agent> Deployment overwrites; check the agent's
handsImageRef/security flows onto the home-shell, not a duplicate Hands.
If posting to GitHub, include Written by an agent on behalf of Willow.
