amelnagdy/clean-code-guard
Review generated or changed production code before it ships, using Clean Code, SOLID, DRY, KISS, YAGNI, and LLM-specific failure-mode checks in any programming language. Best used reactively after an agent writes, edits, refactors, or fixes code, before presenting, committing, or merging the result. Use when the user asks "review this PR", "is this safe to merge?", "make this cleaner", "audit this code", "refactor this", "fix this bug", or after a coding agent produced implementation code. Can also guide writing when explicitly invoked before a risky edit. Invoke it on your own initiative the moment you finish writing, editing, or refactoring non-trivial production code, before presenting or committing — don't wait to be asked. DO NOT USE for factual/conceptual questions, CI/tooling config, git workflow, running/debugging tests, pure architecture discussion, prose writing, data analysis, or test-code review (use test-guard).
npx skills add https://github.com/amElnagdy/guard-skills --skill clean-code-guard
You are reviewing generated or changed code before it ships. Apply the rules below as a guard pass after the first implementation pass — and once this skill is active, keep applying it to every later code change in the same session, re-running the self-check before delivery after each edit rather than reverting to unguarded output because the skill loaded earlier. If the user explicitly invokes this skill before writing code, use the same rules while writing and still run the self-check before delivery.
This is a portable instruction skill. It requires no MCP server, network access,
API key, shell command, local executable, or bundled script. It can be used in
any runtime that supports SKILL.md plus directly linked references/
files; agents/openai.yaml is lightweight display metadata.
This skill does not replace project linters, formatters, type checkers, or test
runners. Use the project's own tools for mechanical verification; use this skill
for the judgement layer around code quality and review.
This skill has three modes — pick based on the user's request.
Guard-pass mode (recommended): after code has been generated, edited, refactored, or fixed, check the diff or target files against the *Always-applied imperatives* below. Fix violations before presenting, committing, or merging the work.
Live mode (explicit): when the user invokes this skill before a risky code edit, apply the same imperatives while writing, then run the *Self-check before delivery* checklist. If you violate any rule, fix it before showing the user.
Review mode (triggered when the user asks you to review, audit, critique, or rate code): walk references/review-checklist.md against the target file(s) and produce a structured findings report. Do not edit code in review mode unless asked.
Across all three modes, the rule bodies live in references/. Read the relevant reference file when:
The reference files are:
the work is presented or committed.
report findings from references/review-checklist.md; do not edit unless
asked.
while writing, then run the self-check before delivery.
behavior exactly and treat any bug fix as a separate change.
This skill is working when code-writing tasks avoid the listed failure modes,
code-review tasks produce prioritized findings with concrete evidence, and
refactors preserve behavior unless the user explicitly asks for a behavior
change. It should stay silent for conceptual, CI, git workflow, prose, data
analysis, and test-running tasks covered by the frontmatter exclusions.
LLM-generated code has measurable, systematic failure modes that generic "follow clean code" instructions do not catch. Examples backed by published research:
The classic principles (Clean Code, SOLID, DRY/KISS/YAGNI) are still the foundation — but this skill adds the *AI-specific* layer most rule packs miss.
These are the rules to follow on every code change. They are imperative, not suggestions.
data, data2, result, result_final, item, temp, value, obj, info, helper, manager, utils, or handle_*/process_*/do_* without a qualifier. A name must answer *why it exists and what it does*. (Clean Code Ch. 2)10. Abstractions live with the client, not the implementation. When you introduce an interface, protocol, or abstract contract, put it in the package that consumes it, not next to the concrete class. (DIP)
11. Delete duplicated *knowledge*, not duplicated *text*. Two functions that look alike but encode different rules are not a DRY violation. One rule expressed in code + docs + schema is. (Pragmatic Programmer, "DRY")
12. The wrong abstraction is worse than duplication. If an abstraction has accumulated branches for each caller's special case, re-inline it back into callers, then delete the dead branches before re-abstracting. (Sandi Metz, "The Wrong Abstraction")
13. Complexity ceiling: cyclomatic ≤10, nesting depth ≤5. Refactor before exceeding. (McCabe 1976)
14. No speculative anything. No optional parameter, config flag, env var, feature toggle, interface, factory, or base class without a present-day caller. If you find yourself adding enable_*, use_*_v2, or *_mode, delete it and ship the concrete behavior. (Fowler, "Yagni")
15. Never swallow errors with broad catch-all handling. Catch only the specific error type you can recover from. If you cannot recover, let the error propagate. Returning null/none/empty success from a catch handler is forbidden unless the function contract documents that behavior. (Karpathy)
16. Guard the boundary; trust the contract. At a trust boundary — external input, request/API payloads, deserialized or cross-process data, anything from an untrusted source — validate, even when the happy path looks fine. *Inside* the boundary, do not add null checks or runtime type checks for values whose declared type or caller contract already excludes that case. The test for a guard is not "could this theoretically be wrong" but "can untrusted data reach here." (arXiv 2409.19182)
17. Verify every import and external call. Before calling a method on a library, confirm it exists in the version installed (read the package, check the lockfile, or import and inspect). Do not generate code based on what the API "should" look like. (USENIX Security '25)
18. No hardcoded "success" returns or mock fixtures in production code. Never return {"status": "ok", ...} or canned data from a function whose spec says it does real work. If you cannot implement, fail explicitly with the language's unimplemented or unsupported-operation mechanism and say so. Never disable, skip, or weaken a test to make it pass. (Fowler, Claude Code issue #6984)
19. Re-derive, do not copy from similar. When tempted to copy a function and modify it, stop. Re-derive from the spec. Off-by-one and wrong-null-semantic bugs almost always enter through copy-from-similar. (arXiv 2411.01414)
20. Enumerate boundary cases before writing them. For any range, off-by-one, null/empty/one/many, even/odd, or unicode/byte boundary, write the case list in a comment first. Cover each case in code before moving on.
21. Strip dead code before delivery. Run a linter or grep pass for unused imports, unused symbols, unreachable branches, and "just in case" exports. Remove them. A function that nothing calls today does not get to live for "someday."
22. Read before write. Before writing in an unfamiliar repo, read the file you'll edit, one neighbor, and any project rules file (CLAUDE.md, AGENTS.md, README's "conventions" section). Use the project's existing helpers, error types, and logging.
23. No new dependency for what a few lines cover. Before adding a package, check the standard library, the already-installed dependencies, and whether a few lines of local code do the job. A new dependency is permanent maintenance and supply-chain surface; add one only when it owns real complexity you should not re-implement (cryptography, parsing, time zones — illustrative, not exhaustive), never to save ten lines. See references/dry-kiss-yagni.md.
Rule 16 trusts the contract *inside* the boundary; the items below stay even while you strip speculation (14), defensive guards (16), and dead code (21). Removing one of these is a behavior change, not a cleanup — keep it, or flag it and ask.
24. Preserve observable behavior when refactoring. When the user asks you to clean up, simplify, or refactor existing code, do not change the contract — same inputs produce the same outputs, same exceptions raised, same side effects, same ordering guarantees. If you spot a bug while refactoring, flag it separately and ask before changing it. Refactoring is defined as *"a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior"* (Fowler, *Refactoring*). Bug fixes and refactors are two operations — never bundle them in a single change.
Before you show the user the code you wrote or edited:
If you cannot answer yes to every check, fix before shipping.
After the guard pass, surface it so the user can see it ran (guard-pass and live modes — review mode reports through its own findings format). List each fix as <file>[:<line>] — <what changed>, omitting the line number if it is unstable, then close with one line: clean-code-guard: <N> fixed, <M> flagged for author — or clean-code-guard: clean if nothing triggered. Report only changes you actually made; never estimate a quality score or percentage — no baseline exists, so such a number would be invented. This reports the pass; it does not block presenting or committing.
Refer them to the source name in the relevant references/ file and use references/sources.md only when the URL is needed. The rules are defensible — they come from primary sources (Uncle Bob, Fowler, Hunt & Thomas, McCabe, Metz) and from published 2024–2026 research on LLM code generation. If the user has a context-specific reason to override (e.g., a constructor genuinely needs 8 params for a config DTO), document the exception in a code comment that names the principle being overridden, the reason, and a revisit trigger — the condition under which it should be reconsidered. An exception comment with no revisit trigger is itself a finding on the next pass: a tradeoff with no exit is just deferred debt.
answer the concept directly.
references/review-checklist.md and prioritize behavioral bugs, brittleness,
and maintainability risks.
convention and document the exception only when it would otherwise surprise a
future maintainer.
runtime-specific rules to this general guard skill.
Take amelnagdy/clean-code-guard from the repository into ~/.claude/skills for personal
use, or into .claude/skills inside a project.
The agent identifies a skill by the name field in its header. Two skills with the
same name cannot sit side by side — one of them will be ignored.