diff --git a/CLAUDE.md b/CLAUDE.md index 2331ac0..ac60ea1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,6 +63,18 @@ The `/commit` skill lives in `.claude/skills/commit/` with a preprocessing scrip The `/test` skill lives in `.claude/skills/test/` with a TAP-parsing test runner. Other commands (worktree, pr, pgxntool-update) remain in `.claude/commands/`. +## Skill and Process Doc Quality + +These rules apply to `SKILL.md` files, command markdown, and any other process doc under `.claude/`. They describe required properties of the doc itself — check them by reading the resulting file/diff, not by trusting that a rewrite process was followed. + +**CRITICAL**: A step's instructions must never contradict another section of the same file (e.g., one section pushing repo A before repo B, another section saying B must go first). When reviewing a diff that rewrites or restructures part of a skill or process doc, read the whole file, not just the changed hunk — large, late-stage rewrites of a single section are the most common place for this kind of self-contradiction to hide, because editing section-by-section doesn't surface whether the doc still agrees with itself end-to-end. + +**CRITICAL**: A skill or process doc must never instruct an action that contradicts this repo's own `CLAUDE.md` policies (e.g., committing or pushing without the required confirmation gate, pushing without triggering the required CI-monitoring step, or skipping a required audit step before a branch is deleted). A diff to a skill/process doc must be checked against `CLAUDE.md`'s actual current text, not against a paraphrase of it. + +**RULE**: Every point where a skill or process doc asks the user a yes/no or multiple-choice question must state the follow-up for every answer, not just the common-path one. A branch that just says "continue" without defining what continuing does is incomplete. + +**RULE**: Markdown fenced code blocks must have a blank line before and after the fence, and every fence must carry a language tag (` ```bash `, not a bare ` ``` `). + ## What This Repo Is **pgxntool-test** is the test harness for validating **../pgxntool/** (a PostgreSQL extension build framework). @@ -162,6 +174,10 @@ Note: "state modifications" here means changes to an already-initialized test en 3. **Never use `skip` for "already done" state modifications.** Make them idempotent with conditionals that simply don't re-run when unnecessary (no skip, no `@test`). Rebuilding state every time is too expensive, so reuse is important - but that logic belongs in non-test code. 4. **Abort early on environment setup failures.** Since we never commit with failing tests, it's better to abort the suite immediately when environment setup fails rather than continuing and collecting potentially many false failures. A failed state modification can invalidate all downstream tests, and the false failures just obscure the real problem. +### Reuse Canonical Test Helpers + +**RULE**: A `.bats` test must resolve a path or piece of state (e.g., the pgxntool checkout location) via the existing canonical helper/variable (e.g. `PGXNREPO` from `load_test_env`), not by deriving it independently (e.g. via `$TOPDIR/../pgxntool`). A configured non-default checkout should not silently cause the wrong thing to be tested. Before introducing new path-resolution logic, grep sibling `.bats` files for how they already solve it. + ### Template Design Principles Tests should generally avoid making changes to template environments. Writing test code to modify the test environment is more complex than having the correct files in the template to begin with. Tests that depend on running `make test` inside a template should strongly consider having the template itself contain the necessary test SQL and expected output files.