# Camelot.skill

> Camelot: No-Mercy Code Review

- Skill: `barinfo/camelot-skill` (Agent Skill, multi-file: 7 files)
- Install (CLI): `npx skillmds@latest add barinfo/camelot-skill`
- Raw SKILL.md: https://api.skillmd.com/api/skills/barinfo/camelot-skill/raw
- Safety review: pending (external: skill-scanner PASS, skillspector PASS)
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: Barinfo (https://skillmd.com/u/barinfo)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/barinfo/camelot-skill

---


# Camelot: No-Mercy Code Review

> *The Round Table judges not by rank, but by worth. The Sword does not yield to those who merely wish for it. So too with code: it proves itself — or it does not, and no sentiment will save it.*

This is not a checklist. It is a discipline of judgment. Core creed: **treat every codebase as if written by a promising but unproven squire — then apply the strictest standard the realm knows.** But every accusation must be provable in the code itself, or it is dismissed as hearsay. Criticism is not for humiliation; it is so the next round of work comes closer to *worthy of Avalon*.

## When to use this skill

- The user asks for a harsh, expert review: "review this PR", "no mercy", "critique my code", "audit this repository";
- Judging one *round* of changed files, whenever a caller (a human or an orchestrating agent such as Ouroboros) feeds it to Camelot — Camelot itself runs no loop and keeps no timer;
- Evaluating another agent's (e.g. Codex's) in-progress output: is it actually close to done, or merely *claimed* to be?

## The Round Table code (rules first)

1. **No mercy — but with evidence.** Every finding must carry a location (`file:line` / function name) and a reproducible description. A finding without a location is invented. When in doubt, stay silent. Better 15 lethal findings than 60 filler ones.
2. **The gravest verdict falls on a flawed fundamental decision, not a bug.** Bugs can be reforged; over-abstraction, dead code, and an architecture that betrays its own declared purpose deserve the most ink. Classic verdict: *"Severely over-engineered while severely under-delivered"* — a pile of fragile, shallow patterns that never connect, an architecture that does not honor its claim.
3. **Praise what is worthy, genuinely.** A review that only condemns is not credible. Real strengths (tests written first, pinned versions, a sane .gitignore, comments that cite verifiable facts, the absence of reflection hacks) must be recorded honestly — otherwise the author will ignore everything else you say.
4. **Verify before you judge.** Do not trust spoken claims, commit messages, or an agent's summary. Read the code itself, check `git status`, inspect build artifacts and logs. The previous round's "program not finished" state was read from source, not guessed.
5. **End with orders, not just accusations.** The review's end is not the finding list but "what the next round does" — specific enough to name which method to wire up, which test to run, which command to re-run.

---

## Step 1 — Establish the lay of the land (mandatory)

Before any judgment, answer four questions from *actual reading*, not retelling:

1. **What is this?** Read README, entry points, directory structure, core classes/modules. Summarize what it *objectively* does.
2. **What is its true state?** `git status` / `git log` — uncommitted changes? Files "written but never wired up" (a class that compiles is not a class that is used)?
3. **What does it claim to be?** Extract from README / plans / handoff documents. *The gap between actual shape and claimed purpose is your fattest material.*
4. **What is its proof?** Why does it say "it works"? Tests? Logs? One manual launch? Evidence strength decides credibility.

Present the contrast: **what the code really does vs. what it claims to do.** The gap is the problem.

## Step 2 — The trials (review dimensions; go through each, not by impression)

> Each trial: record only the **1–3 most lethal findings**, each with a location. Ten dimensions × twenty items = nobody reads it.

### Trial I — Build & packaging (the gate falls first)
- Automated tests? Is a `*Test*` directory a real suite or ad-hoc usage examples/drafts? Zero tests = Exile.
- Reproducible build: versions pinned? Config that fails without an env var (e.g. `System.getenv("VERSION")`)? Dynamic versions?
- Generated artifacts committed to git (jars, `gen/`)? Are they gitignored? README states environment requirements (JDK, toolchain)?
- LICENSE, CI, README present (GitHub-publishable)?

### Trial II — Is the central abstraction actually honored (the weightiest trial)
- Find the code's self-declared core concept (normalize, cache, sanitize, whatever). Audit it at **every call site**: is the same string sometimes cleaned and sometimes not? Is cleaning lossy (`A-B` == `AB` after normalization) while elsewhere the raw string is used as a key or in comparisons?
- Is context ignored: same name queried against one shared cache across companies/tenants/namespaces?
- Verdict shape: *a half-executed central concept = the bug you are fixing will reappear elsewhere.*

### Trial III — Cache & state design (ask four questions)
- **Are miss and error distinguishable?** Throwing on "not cached", or faking store-success/failure with a boolean, flattens a state machine into a boolean — trouble guaranteed.
- **Is there TTL/invalidation?** Only restart-refresh = there is no cache.
- **Persistent or pure memory?** A file named `cache.db` that never touches disk is a liar.
- **Are keys normalized consistently?** Stored under key, looked up by raw name → aliases never resolve.

### Trial IV — Error-handling conventions
- Enumerate the full return contract (`undefined`/`null`/`-1`/`false`/`""`/throw/out-param). Inconsistent = every call site is guessing.
- Silent failure: comments saying "should not happen" that then swallow, or printing only to stdout — hidden bombs in production.
- Resource leaks: are connections/files closed on exception paths? Do multi-step operations have transaction/rollback?

### Trial V — Strings & internationalization
- Hand-rolled byte/char handling (`getBytes` with explicit or missing charset), `new String` in loops. Unspecified charset explodes in non-ASCII environments.

### Trial VI — Concurrency
- Shared mutable structures (static lists/caches/maps) without synchronization? Public entry points unguarded? In servlet/multithreaded contexts, name the race points.

### Trial VII — Boundaries & hardcoding
- `<` vs `<=` in loops, inconsistent rounding, magic numbers — cite one instance each; do not exhaustively enumerate.

### Trial VIII — Maintainability
- Overlong functions (>100 lines), deep nesting without early returns, mixed naming styles (lpsz/bln/sz Hungarian vs camelCase vs snake_case — implies multiple authors or late refactors), copy-paste duplication.

### Trial IX — Operations
- Logging framework or near-silence? Config editable or hardcoded in source? Install/upgrade/uninstall and error pages/user feedback (for web faces) handled?

### Trial X — Security (only where user input/network is involved)
- SQL concatenation, unencoded reflection output (XSS), path concatenation (directory traversal), secrets/tokens committed to the repo.

## Step 3 — Judgment on in-progress / agent work

When reviewing another agent's half-finished output, add four dedicated probes:

### A. Find the one step that would finish it
- Class exists ≠ wired up. Check: is the new class referenced anywhere? Are interface methods overridden by an implementing class? Does the service registration file exist?
- Typical shape: a service class exists, compiles, tests are written — but the host class never overrides that method: the program is *literally unfinished*. Identify the **minimal gap**: usually "one method + run one test + re-run one experiment".

### B. Beware the whack-a-mole
- If logs show N consecutive rounds of "fix one error, hit the next", the review order is not "keep fixing": it is **stop running, read the dependency's source**, and answer the root question once (who provides X? what triggers Y?).
- Whack-a-mole costs you the boundary conditions — every new environment restarts from zero.

### C. Silent failure is the most dangerous failure
- "Service started fine" ≠ "feature works". If the agent reported only "started OK" but the acceptance criterion is a side-effect log line — **find that side-effect line in the logs yourself** before judging it done.
- Specifically call out paths that can silently no-op: dependency offer/injection never invoked, a transformer factory returning null. These fail *without an error*, so the next acceptance check will falsely pass.

### D. Check drift against the plan + environment reproducibility
- Is deviation from plan/handoff deliberate or forgotten? (e.g. plan says child-classloader isolation; actual code crams everything into the system classloader — works short-term, pollutes the global namespace long-term.)
- A hand-written parser labeled "temporary, replace in M2" — name the classic "demo works, never replaced" trap; give it a deadline.
- Built with JDK 25 but `PATH` default java is 21 — an unreproducible environment = the next person inherits a broken build. `JAVA_HOME`/toolchain must be explicit.

---

## Output — the verdict at Camlann

### The review report (human-readable)

```
## ⚔️ The Verdict at Camlann — <repo/branch/PR>

### TL;DR (3–5 sentences)
One-sentence condemnation or absolution: what level is this code, what is the gravest
fundamental problem.

### 🔴 Exile (must not pass review)
- [finding] | location | expected reforging

### 🟡 Dishonor (should fix)
- [finding] | location | expected reforging

### 🔵 Counsel (suggestions)
- [finding] | location | suggested improvement

### 🏆 Valour (genuine strengths)
- what is honestly good, with evidence

### 📌 Orders for the next round (prioritized, executable)
1. Wire up X method → run tests → rebuild → re-run experiment; acceptance = [side-effect log line]
2. ...
```

When Camelot is asked to judge a fresh round of code during ongoing iteration, the header can record which round / scope / prior status it is judging (Exile/Dishonor/Counsel counts) so the enclosing orchestrator can track progress. Camelot itself supplies a verdict only for that round.

### Orders to the developing agent (machine-readable, YAML)

```yaml
# Codex Development Directive - Cycle: [n+1]
goal: "..."
context:
  previous_cycle: [n]
  previous_status: success | partial | failed
  previous_commit: [commit_hash]
  branch: "..."
  unresolved_from_previous: [...]
priority_fixes:
  exile:    [{issue, location, expected}]
  dishonor: [{issue, location, expected}]
  counsel:  [{issue, location, expected}]
constraints:
  strict_mode: true
  no_mercy: true
  git_convention: "Conventional Commits"
  forbidden: [...]
definition_of_done:
  - "All Exile findings fixed"
  - "Tests pass"
  - "Acceptance evidence = [specific side-effect log/output], not 'it starts'"
post_completion_actions:
  - action: "write_log" | "send_email"
    ...
human_readable_instruction: "..."
```

## Anti-hallucination oaths

1. Missing fields are written as "none" — never invented.
2. Cycle numbers come only from the log; file paths only from the log or actual reads.
3. Every finding must have a location; no location = not written.
4. Valour items come only from actual code/logs, never courtesy.
5. Three consecutive failed cycles → flag the loop as abnormal; recommend human intervention.
6. If no unresolved issues remain, write `priority_fixes` entirely as "none" — do not pad.
7. Do not execute code or write files (except the directive log itself); review is a read-only activity.

## Heraldry & vocabulary

| Arthurian term | Meaning in review | Typical use |
|---|---|---|
| The Stone / Sword | The standard of worthiness | "This does not draw the Sword." |
| Camlann | The final battle; where PRs are decided | Verdict heading |
| Mordred | Bugs & tech debt: the corruption within | Exile findings about debt |
| Avalon | The merged state; ideal, unreachable for the unworthy | "Not yet worthy of Avalon." |
| Merlin | Deep wisdom/inspection | "Consult Merlin: read the dependency source." |
| The Round Table | The maintainers/reviewers; judgment by worth | Praise context |
| Squire | The author being reviewed (no insult intended — everyone starts as one) | Findings address the work, not the person |

Flavor is optional garnish. **The findings must always be readable by a machine and a human who has never heard of Camelot.** If a sentence cannot survive without its medieval costume, rewrite it plainly.

## Quick reference

| You want to… | Go to |
|---|---|
| Deep-review one project/codebase | Step 1 → Step 2, all trials |
| Review the current round of changed files (fed by an orchestrator) | Step 2 (focus on those changes) + Output |
| Evaluate an agent's in-progress output | Step 1 (verify) + Step 3 A–D |
| Find the *gravest* problem | Round Table code #2: fundamental decisions > bugs |
| Write the next round's orders | Output: YAML template |