diff options
Diffstat (limited to 'claude-rules')
| -rw-r--r-- | claude-rules/commits.md | 445 | ||||
| -rw-r--r-- | claude-rules/cross-project.md | 8 | ||||
| -rw-r--r-- | claude-rules/desktop-capture.md | 9 | ||||
| -rw-r--r-- | claude-rules/emacs.md | 5 | ||||
| -rw-r--r-- | claude-rules/interaction.md | 45 | ||||
| -rw-r--r-- | claude-rules/knowledge-base.md | 25 | ||||
| -rw-r--r-- | claude-rules/org-tables.md | 5 | ||||
| -rw-r--r-- | claude-rules/subagents.md | 72 | ||||
| -rw-r--r-- | claude-rules/testing.md | 344 | ||||
| -rw-r--r-- | claude-rules/todo-format.md | 108 | ||||
| -rw-r--r-- | claude-rules/working-files.md | 22 |
11 files changed, 353 insertions, 735 deletions
diff --git a/claude-rules/commits.md b/claude-rules/commits.md index c4eb2cd..3283b0a 100644 --- a/claude-rules/commits.md +++ b/claude-rules/commits.md @@ -9,6 +9,7 @@ Claude, Claude Code, Anthropic, or any AI tool. Git uses the configured `user.name` and `user.email` — do not modify git config to attribute otherwise. + ## No AI Attribution — Anywhere Absolutely no AI/LLM/Claude/Anthropic attribution in: @@ -61,133 +62,6 @@ relabel a document another agent genuinely authored: if Codex wrote it, the byline stays Codex. The rule removes false co-authorship, not true authorship. -## Commit Message Format - -Commit messages follow the [Conventional Commits](https://www.conventionalcommits.org/) spec. - -### Structure - - <type>[optional scope]: <description> - - [optional body] - - [optional footer(s)] - -### Types - -- `feat:` — new feature (correlates with MINOR in SemVer) -- `fix:` — bug fix (correlates with PATCH in SemVer) -- `refactor:` — code restructuring, no behavior change -- `perf:` — performance improvement -- `test:` — adding or updating tests -- `docs:` — documentation only -- `style:` — formatting, whitespace, missing semicolons (no code-behavior change) -- `build:` — build system or external dependencies -- `ci:` — CI configuration and scripts -- `chore:` — anything else: tooling, meta, housekeeping - -The Conventional Commits spec doesn't mandate the type list. Add a new type only when the existing ones genuinely don't fit and the team will agree on what it means. - -### Scope - -A scope MAY follow the type, in parentheses, naming the affected area of the codebase: `feat(parser): add ability to parse arrays`. Use a single noun. - -### Breaking changes - -Either append `!` after the type or scope, or include a `BREAKING CHANGE:` footer (uppercase — required). Both at once is fine and adds detail. `!` alone is enough. - - feat!: drop support for Node 6 - - BREAKING CHANGE: uses JavaScript features not available in Node 6. - -### Subject line - -Imperative mood. ≤72 characters. No trailing period. The full subject is `<type>[scope]: <description>` — the 72-char limit covers the whole thing. - -### Body - -Optional. Begins one blank line after the subject. Free-form, multiple paragraphs allowed. Don't hard-wrap body lines — write each paragraph and each bullet as a single logical line and let the renderer (GitHub, Linear, `git log`) soft-wrap. Hard wraps shrink the visible render width in web UIs and cause awkward mid-sentence breaks. The same soft-wrap rule applies to PR bodies. - -Skip the body when the subject line covers the change. - -### Footers - -Optional. One blank line after the body. One per line. Format: `Token: value` or `Token #value` — the git trailer convention. The token uses `-` in place of whitespace (e.g. `Reviewed-by`, `Refs`, `Acked-by`). `BREAKING CHANGE:` is the one token allowed to contain a space, and `BREAKING-CHANGE:` is treated as a synonym. - -### How to write the message - -Write commit messages as if you're explaining the change to someone debugging a failure six months from now. Focus on what changed and why, not the play-by-play of how you typed it. Short imperative summaries like "Validate input before processing" age better than diary-style notes. - -The body, when you need it, is where context belongs — the constraint, bug, or tradeoff that forced the change. Over time the body becomes a lightweight decision log, which is more valuable than perfectly formatted messages. - -Commit messages describe what changed and why, not the process that produced the change. Don't reference code review, linting, test runs, or other workflow steps in the body (e.g. "from local review," "review surfaced," "flagged by reviewer"). Reviewers and future archaeologists want the what and the why. How you got there belongs in the PR discussion, not the commit. - -### Examples - -**Subject only:** - - docs: correct spelling of CHANGELOG - -**With scope:** - - feat(lang): add Polish language - -**With body and footer:** - - fix: prevent racing of requests - - Introduce a request id and a reference to the latest request. Dismiss incoming responses other than from the latest request. - - Remove timeouts which were used to mitigate the racing issue but are obsolete now. - - Refs: #123 - -**Breaking change with `!`:** - - feat(api)!: send an email to the customer when a product is shipped - -**Breaking change in footer:** - - feat: allow provided config object to extend other configs - - BREAKING CHANGE: `extends` key in config file is now used for extending other config files. - -## Voice and Focus - -Applies to commit bodies, PR descriptions, and PR comments (review replies, follow-up notes, thread responses). - -**Write as if to a colleague.** The reader is a teammate who'll see this in `git log`, a PR feed, or a Linear thread. "I" is allowed where natural. Don't sound abstract — name the file, the function, the constraint, the symptom. Press-release voice ("This change improves...") and committee voice ("It is recommended that...") both come out. The message has to read like one engineer talking to another, not like a generated artifact. - -**No felt-experience narration.** Don't tell the reader how the change will feel or how often you'll use it. Phrases like "I'll feel this every time I commit", "this will be a relief", "I'm excited about" — these read as performance, not communication. State what changed and let the reader decide what to do with it. - -**Don't noun-ify verbs.** "The ask", "a learn", "a reveal", "the spend", "a build" — use the real noun: "the request", "the lesson", "the finding", "the budget", "the system". Verb-as-noun reads as corporate-speak and makes the sentence feel performed. - -**No sentence fragments in prose.** Every prose sentence needs a subject and a verb. "Two changes." or "Fix incoming." or "Body as decision log." read as bullet-list shorthand even when they're standing alone in a paragraph. Bullets and headings can be fragments — prose sentences cannot. - -**"I" is the author, not the user.** First person is for what *I* did or decided in this commit ("I dropped the legacy fallback because..."). It's not for describing how the software or rule behaves for whoever uses it next. "The dialog only opens if I ask" is wrong when the rule is read by someone else — that "I" becomes ambiguous. Use third-person or passive for behavior: "opens on request", "opens when asked", "opens when the user invokes it". Code and systems are the actor; "I" stays for decisions. - -**First person where it fits.** When the subject is you or a decision you made, use "I" ("I added X", "I kept the parameter as `Any` because..."). When the subject is a team decision or shared rationale, "we" fits. When another author's prior work is the subject, name them ("Kostya's PR #116 did X"). Third-person constructions like "This PR introduces X" or "This change restores Y" read as press-release self-narration. The commit *is* the change, so don't announce it. Code and systems can stay third-person when they're the actor ("the guard rejects...", "the serializer returns...") — first person is for describing what you did or decided, not for narrating how the code behaves. - -**Brief. Terse is preferred.** A one-sentence body beats a paragraph saying the same thing. If the subject line covers it, skip the body entirely. Cut every clause that restates what the diff or the PR card already shows. Length is not a proxy for care. Rhetorical padding ("worth noting", "it's important to understand") always comes out; keep what a reader will actually use. - -**Follow-up approvals stay terse.** A re-review that just confirms prior CHANGES_REQUESTED feedback got addressed should be `Approved.` and nothing more. The fixes are visible in the diff and in the prior review thread, so restating them adds noise. The first round of substantive review gets a real comment. Subsequent sign-offs after fixes do not. Counts as a trivial one-liner under the Step 2 exception, so the draft-file flow can be skipped. - -**Kind.** PR comments and review replies are directed at a specific person. Acknowledge them when it fits ("thanks for the review") without pouring it on. When you disagree or push back, frame it as your read rather than a correction ("I think...", "my read was...", "did you mean X?"). Leave room for the other person to have seen something you didn't. A polite question beats a defensive explanation. Kindness is free and makes the next review cheaper. - -Focus on what was wrong and what was corrected. Not the mechanics. -Readers skimming `git log` or a PR want the before-state, the -after-state, and the reason. They don't need a TypeScript-variance -lesson, a compiler-inference walkthrough, or a trip through an API's -internals. Keep the "why" to one sentence unless a subtle invariant -genuinely needs more. - -Don't stack technical terms. A sentence that chains three or more type -signatures, API names, or compiler concepts reads as a jargon wall. -Break it into shorter sentences and translate to reader-facing -language. "The mock returns `Promise<Mission>`, so the resolver's -argument is `Mission`, not `unknown`" beats the full inference chain -that produces that signature. Keep the terms a reader will grep for, -drop the ones that name compiler internals. ## Content scope for public artifacts @@ -216,277 +90,54 @@ Edge case: when one of these files *is* the change (a commit in the rulesets rep **Tooling-path enumeration is the same leak.** Citing a rule as authority isn't the only way the tooling layer leaks into history. A commit whose *content* must name these paths — a `.gitignore` adding `.claude/`, `CLAUDE.md`, `.ai/` — has unavoidable, correct file content, but its *message prose* must not enumerate them ("chore: ignore .claude tooling, CLAUDE.md, and session files"). On a public or shared-remote repo that enumeration exposes the tooling layer's structure in the log just as a citation would. Name the category instead: "chore: extend gitignore for local tooling and build artifacts". The same holds for any incidental mention, not only `.gitignore` commits. Two exemptions: a commit whose change *is* one of these files (the edge case above), and private single-user repos with no shared remote, where the history is the project and there's no third party to leak to. -Different artifact types carry different content. Don't duplicate. - -**PR descriptions:** four sections, in order. - -1. **Problem** — what's wrong, with enough detail that a teammate can - recognize the same failure mode in their own work. -2. **Fix** — what changed. -3. **Why this fixes it** — causal link, one or two sentences. -4. **How it was tested** — skip for proposals, specs, or discussions; - required for shipped fixes. - -The PR is the technical artifact. It carries the detail. - -If the project's publishing overlay defines a ticket system, see it for -ticket-body conventions (a ticket body is typically just the Problem and -Fix, with the causal why and test verification left to the PR). - -**PR review comments** are conversational and don't follow this -structure — they follow the Voice and Focus rules above. - -Verbose preambles, motivational language, and context unrelated to the -problem belong out. Same conciseness pressure as commit-message bodies. - -## Review and Publish - -Commits and PRs are team-visible, permanent, and hard to amend once shared -(especially after push or after a reviewer has replied). Before executing -`git commit` or `gh pr create`, the change must pass a local code review -*and* the message must be reviewed by the user. The flow has three steps, in -order. - -### Step 0: pre-flight reconcile (mandatory) - -Before reviewing the diff, fetch from the remote and reconcile against the -upstream of the current branch. Reconciliation can change the working state -when a rebase brings in upstream commits that touch staged files, and that -would invalidate Step 1's review. Handling drift first means the review and -the commit message describe the post-reconcile state. - -1. Fetch all remotes: - - git fetch --all --prune - -2. If the current branch has no upstream (new branch, never pushed), skip - to Step 1 — there's nothing to reconcile against, and the first push - sets the upstream. - -3. Otherwise, check divergence against `@{u}`: - - git rev-list --left-right --count @{u}...HEAD - - Output is `<behind>\t<ahead>`. Decide based on the pair: - - - **0 behind, anything ahead** — no-op. Continue to Step 1. - - **Behind only, clean tree** — fast-forward: `git merge --ff-only @{u}`. - - **Behind only, dirty tree** — surface to the user. Don't auto-stash or - auto-merge. Offer to commit or stash first, or skip the reconcile and - proceed knowing the push may need attention later. - - **Diverged (behind AND ahead)** — surface to the user. Ask whether to - rebase the local commits onto upstream (default for feature branches), - merge the upstream branch in (rare; preserves both lines), or skip and - proceed with the divergence. Don't auto-rebase. - -4. **PR flow only.** Also fetch the base branch (usually `main`) and check - whether the feature branch's base is behind. Surface this informationally; - don't auto-rebase the feature branch without asking. The "X commits - behind base" badge on the PR is a follow-up decision, not a reason to - block publish. - -The startup workflow's `git fetch --all --prune` doesn't substitute for -Step 0. Upstream can advance during a long session, especially across -machines or with teammates pushing in parallel. Run Step 0 every time the -publish flow starts. - -### Step 1: local code review (mandatory) - -Run the `review-code` skill against the change: - -- Before a commit: `/review-code --staged` -- Before a PR: `/review-code` (branch diff against `main` merge-base) -- Before commenting on someone else's PR: `/review-code <PR#>` - -Surface **all** findings to the user: Critical, Important, and Minor. - -**Default block:** any Critical or Important finding stops the flow. Fix the -issues and re-run `/review-code` until the diff is clean. Minor findings are -shown but do not block. - -**Override:** the user can bypass the block with an explicit "proceed anyway" -(or equivalent wording). Without the explicit override, do not proceed to -Step 2. - -The `review-code` skill already has a Phase 0 eligibility gate that handles -trivial and ineligible diffs (whitespace-only, revert with obvious -justification, already-reviewed SHA). Trust that gate; there is no "trivial -enough to skip review" exemption on top of it. - -### Step 2: draft, review, publish - -**Voice patterns and the approval gate are two independent decisions.** Don't bundle them. - -*Voice patterns are always personal for publish artifacts.* Commit messages, PR titles + bodies, and PR review comments all go out under the user's name, so they always run through `/voice personal` (the full pattern walk — general + Craig's-voice + the artifact-mechanics patterns: first-person rewrite, public-artifact scope flag, praise/correction asymmetry, finding stems), regardless of whether `.ai/` is tracked. These three are personal-voice artifacts by definition — the skill's personal mode exists for exactly them. Pattern #39 (public-artifact scope flag) matters *most* on team-visible artifacts, so it must never be skipped on a PR comment or PR body. There is no "general-voice mode" for publish artifacts. - -*The approval gate is the only thing `.ai/`-tracking decides.* Before drafting, run this command: - -``` -git ls-files :/.ai/ 2>/dev/null | head -1 -``` - -The `:/` pathspec anchors the search to the repo root, so the command works from any subdirectory. Without it, running from a subdir returns no matches even when `.ai/` is tracked at the repo root, which silently misclassifies the project. - -- **No output** — `.ai/` is gitignored, missing, or empty (the user's personal repos). **Gate applies**: write to `/tmp`, run `/voice personal`, print inline, ask approve / request changes / open in editor, then publish only on explicit approval. -- **Any output** — one or more files under `.ai/` are tracked (a shared / team repo). **Gate skipped for velocity**: write to `/tmp`, run `/voice personal`, print inline, publish immediately. - -Either way the draft runs through `/voice personal` first. The subflows below describe the full gated path. For the gate-skipped path, run the same `/voice personal` pass, then collapse the "Ask: approve, request changes, or open in editor" step — the draft prints inline and the publish step runs immediately afterward. - -**For commit messages:** - -1. Write the proposed message to `/tmp/commit-<short-slug>.md`. -2. Run `/voice personal` on the file. Always. The skill walks its full pattern list covering signs of AI writing, universal good-writing rules (Strunk & White, Orwell, Plain English, Garner), and Craig's voice patterns (first-person rewrite, semicolons → periods/commas, contractions, sentence-split on conjunctions, felt-experience cut, sentence-fragment rewrite, terse cut for rhetorical padding, no-emphasis-formatting, public-artifact scope flag, praise/correction asymmetry, finding stems). The commit subject line stays imperative per Conventional Commits — `/voice personal` rewrites the body, not the subject. Skip the pass for purely mechanical commits (a chore version bump, a typo fix) where the subject alone carries the message. -3. Print the final draft inline in the terminal. Every line, exactly as it'll be committed. No truncation, no summary. State that the skill ran (e.g. "/voice personal — full pattern walk"). If pattern #39 (public-artifact scope) flagged anything, surface those warnings; the user resolves them manually. -4. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default — print first, edit only if asked. - - **Approve** → commit with `git commit -F /tmp/commit-<short-slug>.md`. - - **Request changes** → make them, re-run `/voice personal`, re-print inline, ask again. - - **Open in editor** → only if the user asks. `emacsclient -n /tmp/commit-<short-slug>.md`. After the editor closes, re-read the file, re-print the contents inline, and ask again. - -**For PR descriptions:** - -1. Write the title as line 1 and the body below it to `/tmp/pr-<slug>.md`. **Title format:** the conventional-commit subject (`refactor: remove dead if-count-is-not-None check in admin`). If the project defines a publishing overlay with a ticket system, follow it for the ticket suffix in the title and the cross-link line in the body (see the overlay). -2. Run `/voice personal` on the file. The PR title stays imperative per Conventional Commits — `/voice personal` rewrites the body, not the title. -3. Print the final draft inline in the terminal. Title on line 1, blank line, then body — exactly as it'll be posted. State that the skill ran. Surface any pattern #39 (public-artifact scope) warnings. -4. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default. - - **Approve** → continue to step 5. - - **Request changes** → make them, re-run `/voice personal`, re-print inline, ask again. - - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<ticket-or-slug>.md`. After the editor closes, re-read the file, re-print inline, ask again. -5. Split the file on the first blank line and pass the title and body to `gh pr create --title "..." --body "$(tail -n +3 <file>)"` (or a heredoc) so formatting is preserved. Add `--reviewer <user[,user...]>` in the same call when you already know who should review. -6. Request reviewers on the new PR if you didn't pass `--reviewer` at create time. Use `gh pr edit <N> --add-reviewer <user>`. If the repo has a `CODEOWNERS` file, GitHub auto-suggests based on touched paths. Still issue the explicit request so the reviewer gets notified. Pick reviewers per the team's convention for the area touched (often documented in the per-repo `CLAUDE.md`). For follow-up PRs, consider tagging the parent PR's author if their context would help. PRs without a human reviewer request stall — "checks passed" is not a substitute for review. -7. **Project publishing overlay (if present).** If the project defines a publishing overlay — a `publishing-<team>.md` rule loaded from its `.claude/rules/` — run its post-create steps now: ticket cross-linking, ticket-state moves, and any other tracker integration it specifies. A project with no overlay skips this; the PR is already open and reviewers are requested, which is the complete universal flow. - -**For PR review comments and replies (review verdicts, threaded discussion, follow-up notes on someone else's PR or your own):** - -Pick the shape first. Most reviews are Shape 1. - -- **Shape 1 — Single review** (verdict + summary body + 0+ inline pins). The default for any post that carries a verdict (`APPROVE`, `REQUEST_CHANGES`, `COMMENT`), even when the verdict has no line-specific findings. One `gh api` call posts the summary, every inline pin, and the verdict together. review notification fires once for `APPROVE` or `REQUEST_CHANGES`. -- **Shape 2 — Issue-thread comment** (no verdict). General PR discussion, not a review. No inline pins. No review notification. -- **Shape 3 — Reply on an existing inline thread**. Responding to a specific prior reviewer comment. Threads under that comment. No review notification. - -**Inline threshold for Shape 1.** Any finding that names a `path:line` belongs as an inline comment pinned to that line. Cross-cutting observations (verdict rationale, "third PR with the same pattern", overall test-coverage gaps that don't pin to one place) stay in the summary body. There's no "fold one inline into the summary" exception — a single line-specific finding still goes inline. - -**Shape 1: Single review (bundled summary + inline)** - -1. Identify findings, split into **inline-eligible** (each names a specific `path:line`) and **summary-only** (cross-cutting). Decide the verdict. - -2. Write one concatenated draft to `/tmp/pr-<N>-review.md` with explicit separators: - - ``` - === SUMMARY === - <verdict summary body> - - === INLINE path=frontend/src/foo.tsx line=440 === - <inline body 1> - - === INLINE path=frontend/src/bar.tsx line=137 === - <inline body 2> - ``` - - The separator format is exactly `=== SUMMARY ===` and `=== INLINE path=<path> line=<n> ===`. The summary block is mandatory even for verdict-only reviews. Inline blocks are zero-or-more. - -3. Run `/voice personal` on the file once. The skill walks its full pattern list across every block at the same time. The separators stay intact because they aren't prose. - -4. Print the final draft inline in the terminal. Every block — the summary body AND the full prose of every inline comment — exactly as it'll be posted, with its separator header. Print the inline in full; never describe it in place of printing it ("I'd pair it with one inline on…"). Craig approves the exact words that post under his name, so the exact words must be on screen. State that the skill ran (e.g. "/voice personal — full pattern walk across summary + 3 inline"). Surface any pattern #39 warnings. - -5. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default. - - **Approve** → continue to step 6. - - **Request changes** → make them, re-run `/voice personal` on the whole file, re-print inline, ask again. - - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<N>-review.md`. After the editor closes, re-read, re-print inline, ask again. - -6. Split the file on the separator lines and post in **a single** `gh api` call: - - ``` - gh api repos/<owner>/<repo>/pulls/<N>/reviews \ - --hostname <ghe-host-or-omit> \ - -F event=REQUEST_CHANGES \ - -F body="<summary block>" \ - -F "comments[][path]=<path1>" \ - -F "comments[][line]=<line1>" \ - -F "comments[][body]=<inline 1>" \ - -F "comments[][path]=<path2>" \ - -F "comments[][line]=<line2>" \ - -F "comments[][body]=<inline 2>" - ``` - - `event` is one of `APPROVE`, `REQUEST_CHANGES`, `COMMENT`. The `comments[]` array can be empty for verdicts with zero line-specific findings — the call still uses the same endpoint. Pass `--hostname` for non-`github.com` hosts (a project's publishing overlay names its host when it's a GitHub Enterprise instance). - -7. Verify the review landed. `gh api repos/<owner>/<repo>/pulls/<N>/reviews --hostname ...` returns the latest review with bundled inlines. Confirm `state` matches the verdict and the inline count matches what was posted. - -8. **Project review-notification overlay (if present).** If the project defines a publishing overlay with a review-notification step (e.g. a Slack ping to the PR author), run it now — but only for `APPROVE` and `REQUEST_CHANGES` verdicts. The overlay owns the channel, the message format, the author-mention lookup, and the threading. A project with no overlay skips notification entirely. `COMMENT` verdicts and Shapes 2-3 below never notify, overlay or not. - -**Shape 2: Issue-thread comment (no verdict)** - -Use when the post is informal discussion that shouldn't appear as a review verdict (e.g. "I'd like to discuss the X approach before you continue"). - -1. Write the proposed comment to `/tmp/pr-<N>-comment.md`. -2. Run `/voice personal`. -3. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5. -4. Post: `gh pr comment <N> --body-file /tmp/pr-<N>-comment.md`. -5. Verify: `gh api repos/<owner>/<repo>/issues/<N>/comments`. -6. No review notification. - -**Shape 3: Reply on an existing inline thread** - -Use when responding to a specific prior reviewer comment. - -1. Find the parent comment ID: `gh api repos/<owner>/<repo>/pulls/<N>/comments`. -2. Write the reply to `/tmp/pr-<N>-reply-<comment-id>.md`. -3. Run `/voice personal`. -4. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5. -5. Post: `gh api repos/<owner>/<repo>/pulls/<N>/comments -F in_reply_to=<comment-id> -F body="$(cat /tmp/pr-<N>-reply-<comment-id>.md)"`. -6. Verify in the same `comments` list. -7. No review notification. - -**Approve does not authorize a merge.** Reviewing a PR never authorizes merging it. Anything in `## Merge Strategy` below applies only to merges *you* are about to perform on your own branches — and even then, the merge needs its own explicit user confirmation per the rules there. A project's publishing overlay may add a team merge practice (e.g. approve-then-author-merges, where the review notification hands the merge decision to the PR author); that's an overlay concern, not a global one. - -**Exception:** trivial one-liners the user dictated verbatim in the -conversation (e.g. "commit this as `chore: bump version`", "reply just -'thanks for the review'") can skip the draft-file step in Step 2. -`/review-code` in Step 1 still runs when it applies; Phase 0 of that skill -handles trivial diffs, and acknowledgment-only replies don't need it at all. - -**Single-skill gate.** Each of the three subflows above runs `/voice personal` before printing the draft — the full pattern walk covering AI-writing signs, universal good-writing rules, Craig's voice patterns, and the artifact-mechanics patterns (first-person rewrite, public-artifact scope flag, praise/correction asymmetry, finding stems). Publish artifacts (commits, PR titles + bodies, PR review comments) always use personal mode; the `.ai/`-tracking check at the top of Step 2 decides only whether the approval gate fires, not which patterns run. Running the skill is mandatory; the printed draft must have been through it. When the user asks mid-flow for "the voice pass" on an in-progress draft, that means re-run the full pattern walk — not a subset. Always state that the skill ran when announcing the printed draft (e.g. "/voice personal — full pattern walk"). Skipping the pass without flagging it is a defect. The terse/omit-needless-words cut (pattern #38) is the *last* thing the skill does before the draft is printed: read each sentence and cut it in half, keeping only what changes meaning. The draft the user first sees must already be terse — if they have to ask for an Orwell pass after seeing it, the pass was skipped. - -**If `/voice` is unavailable.** The skill should be installed (it ships with rulesets), but a fresh or partial environment may not have it. Don't let that block the publish, and don't skip the discipline silently. Walk the same patterns inline — they're documented in the skill, and the publish flow already names which ones matter (first-person rewrite, semicolons → periods/commas, contractions, sentence-split, felt-experience cut, fragment rewrite, terse cut, the pattern #39 public-artifact scope flag, plus the AI-writing and good-writing passes). Then state that the skill was unavailable and the pass was applied by hand (e.g. "/voice unavailable — patterns walked inline"). The gate is the pattern walk, not the tooling; the skill is the convenient way to run it, not the only way. Flag the missing skill so it gets installed. - -### Hook-level authorization - -The Step 1 code review plus the Step 2 user approval together constitute the -authorization gate for the publish action. No separate hook-level approval -prompt is needed on `git commit`, `gh pr create`, `git push`, or their -variants once Step 2 has been approved. If a hook is configured, rely on the -flow above to be the source of truth; do not treat the hook as a second -independent gate. - -## Merge Strategy -- *Squash-merge is the default* for feature branches. It avoids carrying - WIP and fix-up commits into the target branch history and produces one - logical change per merge. -- State the planned merge approach (squash, rebase, or merge commit) and - the target branch *before* pushing or merging. Wait for explicit user - confirmation before `git push`, `gh pr merge`, or any equivalent. The - Review and Publish flow above approves the *content*; merge strategy is - a separate decision that needs its own confirmation. -- *Pre-push reconcile.* Right before `git push`, do one more - `git fetch <remote> <branch>` and verify the local branch is still - ahead-only against its upstream. If something landed between Step 0 and - push (review and draft together can take several minutes, and another - machine or teammate may push in that window), surface and resolve before - the push command runs. Catching drift here is cheaper than recovering - from a failed non-fast-forward push under publish-step pressure. -- Override the squash default only when there's a concrete reason: a - clean per-commit review history the user has explicitly asked for, a - multi-commit semantic narrative the team values, etc. Squash is the - safe default; document why when deviating. - -## Before Committing - -1. Check author identity: `git log -1 --format='%an <%ae>'` — should be the user. -2. Scan the message for AI-attribution language (including emojis and footers), and on a public or shared-remote repo for tooling-path enumeration — prose that lists `CLAUDE.md`, `.claude/`, `.ai/`, `todo.org`, `notes.org`, or `session-context`. Name the category, not the paths. Exempt: a commit whose change is one of those files, and private single-user repos. -3. Review the diff — only intended changes staged; no unrelated files. -4. Confirm staged files belong in the repo: nothing that the project's policy keeps untracked (the personal-tooling set in gitignore-mode projects), and in repos with a canonical/mirror split, the edit is on the canonical side — a mirror-only edit gets reverted by the next sync. -5. Run the full test suite and linters as their own step, read the result, and commit only on zero failures — never chain the run into the commit command (see `verification.md`). +## Write in the first person, as Craig + +Everything authored in or about this repo is first person: code comments, commit +messages, PR descriptions, PR review comments, and any note that lands in the +repo or its history. State a choice as a choice — "I swept the copies rather +than repairing them, because a drifted copy outranked the global rule" beats +"the copies are swept rather than repaired." + +**The "I" is Craig.** He is the author of record on every commit, comment, and +review in his repos, and these artifacts go out under his name. So the voice is +his, writing about his own work — not an agent narrating what it did on his +behalf. Never write the agent into the prose as a separate party: no "Craig +asked me to", no "I filed this for Craig", no "needs Craig's decision". Where a +decision is still open, it is *his* open decision, written as "I haven't decided +whether…" or "this needs a call I haven't made yet." + +The same holds for anyone else's work. Name them ("Kostya's PR #116 did X"), +because they are a third party. Craig is not. + +Third-person constructions like "This change introduces X" or "This PR restores +Y" read as press-release self-narration. The commit is the change, so it does +not need announcing. + +**The one carve-out: code is the actor when describing behavior.** A comment +saying *what the code does* stays third person, because the subject genuinely +is the code and not me — "the sweep only fires when the global rule exists", +"the guard rejects a malformed payload". First person is for the decision +behind it, third person for the behavior itself. Both often belong in the same +comment: what it does, then why I chose it. + +This is the rule the publish flow already applied to commit bodies. It lives +here because code comments get written constantly and the publish skill is not +loaded then. + +## The publish flow lives in the `publish` skill + +Everything about *how* a commit, PR, or review comment gets written, reviewed, +approved, and published — the pre-flight reconcile, the code-review gate, the +draft/voice/approval gate, conventional-commit format, Voice and Focus, PR +description structure, the three review shapes, merge strategy, and the +pre-commit checklist — is in the `publish` skill. Load it before drafting a +message, not after: the flow gates what gets written. + +What stays here is what must hold whether or not anything is being published, +and where a violation is permanent and reaches other people. If the skill fails +to load you will have to be told the flow, which is recoverable. The rules +below are not. ## If You Catch Yourself diff --git a/claude-rules/cross-project.md b/claude-rules/cross-project.md index 73c0e1b..c5de962 100644 --- a/claude-rules/cross-project.md +++ b/claude-rules/cross-project.md @@ -50,6 +50,14 @@ whose canonical home is `~/code/rulesets/`. When work in a downstream project needs one of these files to change, a local edit alone is a stopgap that the next sync reverts. The durable change happens only in the rulesets canonical. +Installed global paths such as `~/.claude/rules/`, `~/.claude/hooks/`, +`~/.claude/skills/`, and rulesets-owned entries in `~/.local/bin/` are +symlinks into the canonical repository, not downstream copies. Never edit +through those paths from another project's session: resolving the symlink +would dirty rulesets outside its own logging and wrap discipline. The +`rulesets-write-boundary.py` hook mechanically denies Edit/Write targets whose +real path lands there and directs the proposal through `inbox-send rulesets`. + The process, every time: 1. **Make the change locally** in the downstream project so it's usable diff --git a/claude-rules/desktop-capture.md b/claude-rules/desktop-capture.md index 0051c4d..c4a67f9 100644 --- a/claude-rules/desktop-capture.md +++ b/claude-rules/desktop-capture.md @@ -38,9 +38,12 @@ output isn't available; it needs the compositor installed. Open it on a *separate* real workspace and tell them which one, so it never grabs their active workspace. They switch when ready. Craig's viewer preference -is `imv`; launch it through the compositor (`hyprctl dispatch exec "imv -<files>"`) so it survives the agent's shell rather than a bare `&` job that gets -reaped. +is `imv`; launch it with `gui-open --image <file>` (dotfiles-shipped) rather +than a bare `&` job or `hyprctl dispatch exec`: it detaches through `systemd-run +--user` so the agent shell can't reap it, resolves the current Hyprland instance +after a compositor restart, and confirms the viewer is mapped and visible before +returning. An HTML render uses `gui-open <file>` (or `--browser`) the same way. +If `gui-open` isn't on PATH, the machine needs a dotfiles pull. ## Always clean up diff --git a/claude-rules/emacs.md b/claude-rules/emacs.md index 2c3b729..846888d 100644 --- a/claude-rules/emacs.md +++ b/claude-rules/emacs.md @@ -1,3 +1,8 @@ +--- +paths: + - "**/*.el" +--- + # Working With Craig's Running Emacs Applies to: `**/*.el` (and any task that edits Craig's Emacs configuration) diff --git a/claude-rules/interaction.md b/claude-rules/interaction.md index 8d65799..b5798bd 100644 --- a/claude-rules/interaction.md +++ b/claude-rules/interaction.md @@ -2,7 +2,46 @@ Applies to: `**/*` -How the agent communicates with the user during a session — choice prompts, status updates, decision points. +How the agent reasons with the user and communicates during a session — how interpretations are formed, then how choices, status, and decision points are presented. + +## Collaborative Peer Reasoning + +Treat the conversation as joint reasoning between peers. The user's words are evidence of intent, not merely a string to execute literally. Use the request, prior context, current state, and likely downstream consequences to form a working interpretation. + +### Infer first; clarify at material forks + +Infer the intended outcome and proceed when reasonable interpretations lead to the same action. When two plausible interpretations would produce materially different outcomes, strategies, or external effects, state the working interpretation and ask one focused question before crossing that fork. Do not interrupt for reversible implementation details that can be resolved with ordinary judgment. + +### Test conclusions before committing to them + +Do not promote the first plausible explanation or plan into a conclusion. Check the strongest reasonable alternative, test the assumptions that distinguish it, and weight the tradeoffs that decide between them. Separate verified facts, inferences, and recommendations when the distinction matters. Calibrate confidence instead of projecting certainty. + +For a consequential judgment, a useful response shape is: working interpretation, evidence, strongest alternative, recommendation, confidence, and the one clarification that would change the action. Do not force this scaffold onto simple tasks. + +### Corrections update the model + +Treat a user correction as new evidence that changes the working model. Reconcile its downstream implications immediately: assumptions, source selection, plans, scheduled actions, task state, and conclusions already reached. Do not reduce a substantive correction to a wording change or preserve stale premises silently. + +A correction is not automatically true merely because the user made it. If it conflicts with verified evidence, explain the conflict directly and identify what would resolve it. If the correction is supported, change course cleanly without defending the earlier answer. + +### Neither sycophantic nor adversarial + +Agreement follows evidence and reasoning, not deference. Disagreement serves the shared outcome, not the defense of a prior position. State a contrary view when it changes the decision, give the evidence behind it, and leave room for missing context. Once new evidence resolves the issue, stop arguing the old case. + +### Process serves the outcome + +Rules and workflows are constraints on the work, not substitutes for judgment. Apply them in service of the user's intended outcome. If a literal workflow interpretation produces a disproportionate, surprising, or strategically different result, surface that implication before proceeding. + +Failure signs: + +- Executing the narrow literal request while ignoring an evident intended outcome. +- Asking about a reversible detail while failing to clarify a consequential fork. +- Presenting one plausible account as settled without testing its strongest alternative. +- Treating a correction as local wording while leaving downstream assumptions unchanged. +- Agreeing to preserve rapport or resisting to preserve authority. +- Letting procedural completeness overwhelm the value of the task. + +This is a working collaboration contract, not immutable wording. Refine the section as real sessions expose better distinctions or failure modes; route the feedback to the canonical file rather than accumulating project-local exceptions. ## No Popup Menus for Choices @@ -54,7 +93,7 @@ In conversational output to the user, do not use Markdown bold (`**...**`) or in - Write command names, file paths, key chords, and code identifiers as plain text — `pearl-save-issue` becomes pearl-save-issue, `C-; L s s` becomes C-; L s s. - Use structure that doesn't invert colors: headers, numbered lists, dashes, parentheses, and double quotes for labels are all fine. -- Fenced code blocks (triple backtick) are acceptable when the user explicitly wants a block to copy — they don't invert the way inline spans do. Default to plain text otherwise. +- Fenced code blocks (triple backtick) are not an exception. Craig's direction is zero markup in chat output, always: "always always list it out without markup" (2026-05-30). Fences don't invert the way inline spans do, but they still read as markup he didn't ask for, and the carve-out kept reintroducing them. When he needs something to copy, give it as plain indented text, or write it to a file and name the path. This governs **chat output**, not the Markdown source of rule files, specs, or docs the user reads in an editor — those keep normal Markdown formatting. The constraint is the terminal rendering of the live conversation. @@ -64,7 +103,7 @@ Craig runs Claude Code inside Emacs EAT (through tmux). EAT renders SendUserFile Two display lanes, by what the visual is for: -- **Durable or interactive visuals** — HTML prototypes, full renders, anything Craig will study or click: open in the browser (`google-chrome-stable "file://<abs-path>" &>/dev/null &`) or imv, on a separate workspace per `desktop-capture.md`. +- **Durable or interactive visuals** — HTML prototypes, full renders, anything Craig will study or click: open with `gui-open <abs-path>` (dotfiles-shipped; detaches through `systemd-run --user` so the agent shell can't reap it, resolves the Hyprland instance after a compositor restart, and confirms the window is actually visible before returning). It auto-detects image vs HTML; `--image` / `--browser` force. Place it off Craig's active workspace and name it, per `desktop-capture.md`. - **Quick inline glances** — a chart, a did-it-render check: sixel in the terminal. Encode with ImageMagick (`magick <img> sixel:<out>`; img2sixel silently emits zero bytes on some builds) and display in a separate tmux window (`tmux new-window -d -n <name>`, then `tmux send-keys -t <name> "clear; cat <out>" Enter`, tell Craig the window name) rather than the Claude Code pane, whose TUI repaints over anything drawn into it. With native tmux sixel active, the image lives in tmux's grid and survives window switches, scrolling, and resizing. **Capability gate.** Native sixel needs two config pieces: EAT answering the XTWINOPS cell-size query (patched eat.el, owned by .emacs.d) and `terminal-features 'xterm*:sixel'` in tmux.conf (owned by dotfiles). Check before relying on it: `tmux display -p '#{client_cell_width}'` — nonzero means go; 0 means the chain is missing a piece, so fall back to the browser lane. (Diagnosed 2026-07-13: tmux won't transmit sixel until it knows the client's cell pixel size, and stock EAT 0.9.4 silently ignores the CSI 14 t query tmux uses to ask.) diff --git a/claude-rules/knowledge-base.md b/claude-rules/knowledge-base.md index d61ef03..146a5e4 100644 --- a/claude-rules/knowledge-base.md +++ b/claude-rules/knowledge-base.md @@ -22,13 +22,15 @@ Pull before querying (`git -C ~/org/roam pull --ff-only`); skip silently if offl Classify the project before any write. The source of truth is the work-root denylist below — never inference from remotes, names, or task content: - **Work** — project root is, or sits under, a denylisted root. No KB write, ever. Record durable facts per that project's own conventions. -- **Personal** — project root sits under `~/code/`, `~/projects/`, or `~/.emacs.d` and is not denylisted. KB writes allowed. +- **Personal** — project root sits under `~/code/`, `~/projects/`, `~/.emacs.d`, or `~/.dotfiles` and is not denylisted. KB writes allowed. - **Unknown** — anything else. No KB write. Work-root denylist (confirmed by Craig, 2026-06-10): `~/projects/work` **Refusal contract** (work and unknown alike): state the classification, name the durable fact in a one-line redacted summary, and say where it was or wasn't written — so Craig can re-route it deliberately instead of losing it silently. +**Scope of the denylist — durable KB-node writes only.** The work-denylist governs one thing: promoting a durable fact into a new `agents/` node. It is a confidentiality guard so work-confidential material doesn't land in the personal cross-machine store. It is *not* a general "don't touch roam from a work project" boundary. Roam is a *shared resource*, not another project's product scope. Reading it (any project) and *tidying the shared roam inbox* — processing, routing, and filing the capture items in `~/org/roam/inbox.org`, e.g. via inbox-zero — are allowed from any project session, work included; that is housekeeping on a shared resource, not a durable-fact write. Only the durable-node promotion stays work-denylisted. Do not park roam-inbox tidying as a cross-project boundary crossing (a sentry inbox-zero pass did exactly that on 2026-07-19 — the error this note closes). + A write is one node per fact, under `agents/`, roam-valid so Craig's org-roam indexes it: ``` @@ -43,7 +45,26 @@ A write is one node per fact, under `agents/`, roam-valid so Craig's org-roam in <the fact, with [[id:...]] links to related nodes> ``` -Pull before writing, commit and push after (`git -C ~/org/roam add -A && git commit && git push`) — same session discipline as any repo. Never edit Craig's hand-authored nodes; link to them. This write autonomy is scoped to the KB alone — it is not permission to send email, comment on tickets, or post to any public or external channel. +Pull before writing (`git -C ~/org/roam pull --ff-only`, read-only). Then acquire the roam-write lock, write the node, and trigger roam-sync to commit and push — roam-sync stays the roam repo's only committer (the 2026-06-24 one-git-owner rule). The tree is chronically dirty from live captures, so an agent's own `git add -A && commit` could sweep an in-flight capture into a stray commit; edit-plus-trigger avoids that. Never edit Craig's hand-authored nodes; link to them. This write autonomy is scoped to the KB alone — it is not permission to send email, comment on tickets, or post to any public or external channel. + +The write block, with the lock and the trigger: + +```sh +# Acquire the roam-write lock so a concurrent sentry pass or inbox writer can't +# race this write. Callers pass a name; agent-lock owns the path (tmpfs). +if [ -x .ai/scripts/agent-lock ]; then + if ! .ai/scripts/agent-lock acquire roam-write --wait; then + # Busy after the bounded wait — surface and stop, don't write unlocked. + echo "roam-write lock held by another writer; try again shortly" >&2 + exit 1 + fi +fi +# ... write ~/org/roam/agents/<ts>-<slug>.org ... +systemctl --user start roam-sync.service # roam-sync commits + pushes +[ -x .ai/scripts/agent-lock ] && .ai/scripts/agent-lock release roam-write +``` + +Degrade gracefully when `agent-lock` isn't installed (an older checkout mid-sync): the guard above is skipped and the write proceeds unlocked — today's behavior. Only a *present* helper reporting the lock busy after its bounded wait stops the write; an *absent* helper never blocks it. ## What goes in, what stays out diff --git a/claude-rules/org-tables.md b/claude-rules/org-tables.md index 1b70085..bc9d27d 100644 --- a/claude-rules/org-tables.md +++ b/claude-rules/org-tables.md @@ -1,3 +1,8 @@ +--- +paths: + - "**/*.org" +--- + # Org Table Standard Applies to: `**/*.org` diff --git a/claude-rules/subagents.md b/claude-rules/subagents.md index 8578dea..e52d906 100644 --- a/claude-rules/subagents.md +++ b/claude-rules/subagents.md @@ -34,6 +34,66 @@ This is the same boundary the "Don't Subagent At All" section and the "Subagenting trivial work" anti-pattern draw; treat it as an explicit gate at dispatch time. +Every size-based rule in this file — the cost gate here, "Don't Subagent At +All", the trivial-work anti-pattern — is subject to the isolation override +below. + +## Isolation Override — When Size Doesn't Gate + +Every size heuristic in this file rests on one assumption: that the main +thread could do the task itself just as well, so the only question is +whether delegating is worth the overhead. When that assumption fails, the +heuristics don't apply, and a five-line task can require a subagent that a +five-hundred-line one wouldn't. + +The assumption fails whenever **the main thread is structurally disqualified +from the task** — not slower at it, disqualified. The test: would the main +thread's own context make its answer *less* trustworthy? If holding the +context is what corrupts the judgment, then doing it inline doesn't save the +overhead, it destroys the result. The isolation *is* the deliverable, and +"it's only a small diff" is not an argument against it. + +**The standing instance is the pre-commit code review** (`publish` skill, +Step 1). The author cannot review their own change, because a self-review +checks the diff against the author's own model of it and cannot check the +model. Errors that survive a self-review are the ones that were never in the +diff — an inherited scope, an estimated blast radius, a fix correct for the +case in mind and wrong for the one never considered. So that review is +dispatched on *every* commit including a one-line one, and the ~10-tool-call +floor, the single-function rule, and the trivial-work anti-pattern are all +overridden there by design. + +Other cases with the same shape: verifying a claim the main thread already +committed to in conversation, and any second opinion where the first opinion +is already in context. If you find yourself reasoning "I already know the +answer, so a subagent is wasteful," check whether already knowing it is the +problem. + +This override widens *what* gets dispatched. Scope, constraints, and output +format are still required, and arguably matter more here, since an isolated +agent can't fall back on shared context to fill a gap. + +**Field 2 of the Prompt Contract inverts under this override, and the +inversion is the whole point.** Normally field 2 says to paste the relevant +output verbatim and include what you learned in earlier turns. Do that for an +isolation dispatch and you hand over the very model you spawned the agent to +escape — a reviewer given your findings reviews your findings. So for an +isolation dispatch, field 2 is *the artifact under test and the independent +record of what was asked, and nothing else*: the diff, a one-line claim of +what it does, and the ticket or plan where one exists. The conversation, the +rationale, and the dead ends are withheld on purpose. + +Keep the requirement source in. A ticket is not your model of the change; it +was written before the work, usually by someone else, and it is the only +thing that can contradict your claim about your own diff. + +**The output is a judgment, so the review gate resolves differently.** The +Review-Gate Cadence below says subagent output is a claim to be verified +before moving on, which is right when the deliverable is *work*. When the +deliverable is *a judgment about your work*, verifying it against your own +reading reinstates exactly the bias the dispatch removed. Disagreement goes +to the user to adjudicate, not back to the author's own judgment. + ## When to Spawn a Subagent ### Parallel-safe (spawn multiple in parallel) @@ -63,11 +123,15 @@ at dispatch time. ### Don't Subagent At All +Unless the Isolation Override applies — these are efficiency rules, and they +lapse when the main thread's own context is what makes its answer untrustworthy. + - **The target is already known** and the work fits in under ~10 tool calls. - **Single-function logic** — one Read + one Edit is faster than briefing an agent. - **You can see the answer from context** — don't spawn a researcher for - something already on screen. + something already on screen. (The inverse of this one is the override's + clearest case: when *having* seen it is the disqualification, dispatch.) ## Prompt Contract @@ -138,7 +202,11 @@ fix), then dispatch the fix with a specific contract. - **Retrying a failed subagent task in the orchestrator** — pollutes context. Dispatch a fix agent instead. - **Subagenting trivial work** — one Read + one Edit doesn't need an - agent; spawn overhead exceeds benefit. + agent; spawn overhead exceeds benefit. Except under the Isolation + Override, where a one-line diff still gets its own reviewer. +- **Reviewing your own change inline** — the mirror-image failure, and the + more expensive one. Skipping a dispatch to save overhead on a small diff + costs a review that could only have come from outside your context. - **Skipping review between tasks** — compounding bugs are much harder to unwind than any single bug. - **Letting the agent decide scope** — "figure out what needs changing" diff --git a/claude-rules/testing.md b/claude-rules/testing.md index b3fa5bf..dd15282 100644 --- a/claude-rules/testing.md +++ b/claude-rules/testing.md @@ -16,340 +16,30 @@ TDD is the default workflow for all code, including demos and prototypes. **Writ Do not skip TDD for demo code. Demos build muscle memory — the habit carries into production. -### Understand Before You Test -Before writing tests, invest time in understanding the code: +## Test Categories — required for all code -1. **Explore the codebase** — Read the module under test, its callers, and its dependencies. Understand the data flow end to end. -2. **Identify the root cause** — If fixing a bug, trace the problem to its origin. Don't test (or fix) surface symptoms when the real issue is deeper in the call chain. -3. **Reason through edge cases** — Consider boundary conditions, error states, concurrent access, and interactions with adjacent modules. Your tests should cover what could actually go wrong, not just the obvious happy path. +Every unit under test needs all three, not just the happy path: -### Adding Tests to Existing Untested Code +1. **Normal** — standard inputs, common workflows, typical volumes. +2. **Boundary** — zero, one, max, empty vs null, single-element collections, unicode, very long input, timezone and date edges. +3. **Error** — invalid input, type mismatches, network failure, missing parameters, permission denied, resource exhaustion, malformed data. -When working in a codebase without tests: +The negative and boundary cases are the ones that find bugs. A unit with only +Normal coverage is not tested, it is demonstrated. -1. Write a **characterization test** that captures current behavior before making changes -2. Use the characterization test as a safety net while refactoring -3. Then follow normal TDD for the new change +## The rest of the standard lives in the `testing-standards` skill -## Test Categories (Required for All Code) +Characterization tests for untested code, the per-category detail, combinatorial +and property-based and mutation testing, organization and the pyramid, +integration-test rules, naming, the test-quality rules (independence, +determinism, mocking boundaries, signs of overmocking), the +refactor-when-tests-are-hard principle, coverage targets, the spike exception, +and the anti-pattern list are all in the `testing-standards` skill. Load it when +writing tests. -Every unit under test requires coverage across three categories: - -### 1. Normal Cases (Happy Path) -- Standard inputs and expected use cases -- Common workflows and default configurations -- Typical data volumes - -### 2. Boundary Cases -- Minimum/maximum values (0, 1, -1, MAX_INT) -- Empty vs null vs undefined (language-appropriate) -- Single-element collections -- Unicode and internationalization (emoji, RTL text, combining characters) -- Very long strings, deeply nested structures -- Timezone boundaries (midnight, DST transitions) -- Date edge cases (leap years, month boundaries) - -### 3. Error Cases -- Invalid inputs and type mismatches -- Network failures and timeouts -- Missing required parameters -- Permission denied scenarios -- Resource exhaustion -- Malformed data - -## Combinatorial Coverage - -For functions with 3+ parameters that each take multiple values (feature-flag -combinations, config matrices, permission/role interactions, multi-field -form validation, API parameter spaces), the exhaustive test count explodes -(M^N) while 3-5 ad-hoc cases miss pair interactions. Use **pairwise / -combinatorial testing** — generate a minimal matrix that hits every 2-way -combination of parameter values. Empirically catches 60-90% of combinatorial -bugs with 80-99% fewer tests. - -Invoke `/pairwise-tests` on the offending function; continue using `/add-tests` -and the Normal/Boundary/Error discipline for the rest. The two approaches -complement: pairwise covers parameter *interactions*; category discipline -covers each parameter's individual edge space. - -Skip pairwise when: the function has 1-2 parameters (just write the cases), -the context requires *provably* exhaustive coverage (regulated systems — document -in an ADR), or the testing target is non-parametric (single happy path, -performance regression, a specific error). - -## Escalation Beyond Category and Pairwise - -The Normal/Boundary/Error categories and the pairwise matrix are the default -discipline. Two further techniques escalate beyond them — reach for them when -the default leaves a gap, not on every unit. - -### Property-Based Testing - -When an invariant holds across a broad input domain — round-trips -(`decode(encode(x)) == x`), idempotence (`f(f(x)) == f(x)`), ordering -invariants (output is always sorted), or any "output always satisfies X" — -generate inputs and assert the property instead of enumerating cases. The -generator explores corners you wouldn't think to write by hand, and a -failing case shrinks to a minimal reproducer. Use the standard tool for the -language (Hypothesis for Python, fast-check for JS, proptest for Rust). -State the property as the test name and let the framework supply the inputs. - -Reach for this when the behavior is a law over a domain rather than a fixed -set of examples. Keep category-discipline cases for the specific edges that -must always hold; the property test covers the space between them. - -### Mutation Testing - -When line coverage is high but you suspect the assertions are thin — tests -that execute the code without checking its output, or that pass with a -function body replaced by a stub — use mutation testing to measure whether -the suite actually kills injected faults. The tool flips conditionals, swaps -operators, and deletes statements, then reruns the suite; a surviving mutant -is a fault the tests didn't catch. Use mutmut or cosmic-ray for Python, -Stryker for JS. High line coverage with a low mutation score means weak -assertions, not a tested codebase. - -Reach for this on critical logic where coverage looks reassuring but you -want evidence the tests would fail on a regression. It's a diagnostic, not a -gate on every change — mutation runs are slow. - -## Test Organization - -Typical layout: - -``` -tests/ - unit/ # One test file per source file - integration/ # Multi-component workflows - e2e/ # Full system tests -``` - -Per-language files may adjust this (e.g. Elisp collates ERT tests into -`tests/test-<module>*.el` without subdirectories). - -### Testing Pyramid - -Rough proportions for most projects: -- Unit tests: 70-80% (fast, isolated, granular) -- Integration tests: 15-25% (component interactions, real dependencies) -- E2E tests: 5-10% (full system, slowest) - -Don't duplicate coverage: if unit tests fully exercise a function's logic, -integration tests should focus on *how* components interact — not repeat the -function's case coverage. - -## Integration Tests - -Integration tests exercise multiple components together. Two rules: - -**The docstring names every component integrated** and marks which are real vs -mocked. Integration failures are harder to pinpoint than unit failures; -enumerating the participants up front tells you where to start looking. - -Example: - -``` -def test_integration_refund_during_sync_updates_ledger_atomically(): - """Refund processed mid-sync updates order and ledger in one transaction. - - Components integrated: - - OrderService.refund (entry point) - - PaymentGateway.reverse (MOCKED — returns success) - - Ledger.credit (real) - - db.transaction (real) - - Validates: - - Refund rolls back if ledger write fails - - Both tables updated or neither - """ -``` - -**Write an integration test when** multiple components must work together, -state crosses function boundaries, or edge cases combine. **Don't** when -single-function behavior suffices, or when mocking would erase the interaction -you meant to test. - -## Naming Convention - -- Unit: `test_<module>_<function>_<scenario>_<expected>` -- Integration: `test_integration_<workflow>_<scenario>_<outcome>` - -Examples: -- `test_cart_apply_discount_expired_coupon_raises_error` -- `test_integration_order_sync_network_timeout_retries_three_times` - -Languages that prefer camelCase, kebab-case, or other conventions keep the -structure but use their idiom. Consistency within a project matters more than -the specific case choice. - -## Test Quality - -### Independence -- No shared mutable state between tests -- Each test runs successfully in isolation -- Explicit setup and teardown - -### Determinism -- Never hardcode dates or times — generate them relative to `now()` -- No reliance on test execution order -- No flaky network calls in unit tests -- Time/clock-mocking helpers must avoid two recurring failure modes: - - *Infinite recursion.* The helper must not call the primitive it's - replacing. If the mock for `now()` calls `now()`, the test stack - overflows. Compute the mock value from a fixed source (a captured - instant, an injected fake clock). - - *Scope-shadowing without reach.* A mock that only exists inside - the test function won't affect production code that reads the - symbol through its canonical path. Replace the symbol at its - definition site (monkey-patch the module attribute in Python, - redefine the global in Lisp, swap the package-level binding in - Go, replace the named export in JavaScript) — or inject a fake - via dependency-inversion. Don't lean on scope-shadowing - primitives (Lisp `let`, Python local rebind, JS shadowed `let`) - that fence the mock to the test's lexical scope; production code - won't see them and the test passes against the real clock. - -### Performance -- Unit tests: <100ms each -- Integration tests: <1s each -- E2E tests: <10s each -- Mark slow tests with appropriate decorators/tags - -### Mocking Boundaries -Mock external dependencies at the system boundary: -- Network calls (HTTP, gRPC, WebSocket) -- File I/O and cloud storage -- Time and dates -- Third-party service clients - -Never mock: -- The code under test -- Internal domain logic -- Framework behavior (ORM queries, middleware, hooks, buffer primitives) - -### Signs of Overmocking - -Ask yourself: - -- Would this test still pass if I replaced the function body with `raise NotImplementedError` (or equivalent)? If yes, the mocks are doing the work — you're testing mocks, not code. -- Is the mock more complex than the function being tested? Smell. -- Am I mocking internal string / parsing / decoding helpers? Those aren't boundaries — they're the work. -- Does the test break when I refactor without changing behavior? Good tests survive refactors; overmocked ones couple to implementation. - -When tests demand heavy internal mocking, the fix isn't better mocks — it's -restructuring the code (see *If Tests Are Hard to Write* below). - -### Testing Code That Uses Frameworks - -When a function mostly delegates to framework or library code, test *your* -integration logic: -- ✓ "I call the library with the right arguments in the right context" -- ✓ "I handle its return value correctly" -- ✗ "The library works in 50 scenarios" — trust it; it has its own tests - -For polyglot behavior (e.g., comment handling across C/Java/Go/JS), test 2-3 -representative modes thoroughly plus a minimal smoke test in the others. -Exhaustive permutations are diminishing returns. - -### Test Real Code, Not Copies - -Never inline or copy production code into test files. Always `require`/`import` -the module under test. Copied code passes even when production breaks — the -bug hides behind the duplicate. - -Mock dependencies at their boundary; exercise the real function body. - -### Error Behavior, Not Error Text - -Test that errors occur with the right type; don't assert exact wording: -- ✓ Right exception type (`pytest.raises(ValueError)`, `(should-error ... :type 'user-error)`) -- ✓ Regex on values the message *must* contain (e.g., the offending filename) -- ✗ `assert str(e) == "File 'foo' not found"` — breaks when prose changes even though behavior is unchanged - -Production code should emit clear, contextual errors. Tests verify the -behavior (raised, caught, returned nil) and values that must appear — not the -prose. - -## If Tests Are Hard to Write, Refactor the Code - -If a test needs extensive mocking of internal helpers, elaborate fixture -scaffolding, or mocks that recreate the function's own logic, the production -code needs restructuring — not the test. - -Signals: -- Deep nesting (callbacks inside callbacks) -- Long functions doing multiple things ("fetch AND parse AND decode AND save") -- Tests that mock internal string / parsing / I/O helpers -- Tests that break on refactors with no behavior change - -Fix: extract focused helpers (one responsibility each), test each in isolation -with real inputs, compose them in a thin outer function. Several small unit -tests plus one composition test beats one monster test behind a wall of mocks. - -## Coverage Targets - -- Business logic and domain services: **90%+** -- API endpoints and views: **80%+** -- UI components: **70%+** -- Utilities and helpers: **90%+** -- Overall project minimum: **80%+** - -New code must not decrease coverage. PRs that lower coverage require justification. - -## TDD Discipline - -TDD is non-negotiable. These are the rationalizations agents use to skip it — don't fall for them: - -| Excuse | Why It's Wrong | -|--------|----------------| -| "This is too simple to need a test" | Simple code breaks too. The test takes 30 seconds. Write it. | -| "I'll add tests after the implementation" | You won't, and even if you do, they'll test what you wrote rather than what was needed. Test-after validates implementation, not behavior. | -| "Let me just get it working first" | That's not TDD. If you can't write a failing test, you don't understand the requirement yet. | -| "This is just a refactor" | Refactors without tests are guesses. Write a characterization test first, then refactor while it stays green. | -| "I'm only changing one line" | One-line changes cause production outages. Write a test that covers the line you're changing. | -| "The existing code has no tests" | Start with a characterization test. Don't make the problem worse. | -| "This is demo/prototype code" | Demos build habits. Untested demo code becomes untested production code. | -| "I need to spike first" | Spikes are fine — under the protocol below. Throw the spike away, then write the first failing test before productionizing. | - -If you catch yourself thinking any of these, stop and write the test. - -### The Spike Exception (Disciplined) - -TDD stays the default. The one sanctioned way to write code before a test is -a spike — exploratory code that answers "is this approach even viable?" when -you can't yet write a meaningful failing test because the shape of the -solution is unknown. A spike is disciplined only when all three hold: - -1. **Timebox it.** Set a limit before starting (an hour, an afternoon) and - stop when it's up. An open-ended spike is just untested implementation - wearing a different name. -2. **Do not commit spike code.** The spike is a learning artifact, not a - deliverable. It never enters the branch history. Keep it in a scratch - file or a throwaway worktree. -3. **Throw the spike away, then start with a failing test.** Once the spike - has answered the viability question, delete it. Write the first failing - test against the now-understood behavior, then productionize under normal - Red/Green/Refactor. The production code is written test-first even though - the exploration wasn't — you don't promote the spike into production by - bolting tests on after. - -The spike buys understanding, not code. If you find yourself keeping the -spike because rewriting it feels wasteful, the timebox was too long or the -problem was tractable enough to TDD from the start. - -## Anti-Patterns (Do Not Do) - -- Hardcoded dates or timestamps (they rot) -- Testing implementation details instead of behavior -- Mocking the thing you're testing -- Mocking internal helpers (string ops, parsing, decoding) — those are the work -- Inlining production code into test files — always `require` / `import` the real module -- Asserting exact error-message text instead of type + key values -- Shared mutable state between tests -- Non-deterministic tests (random without seed, network in unit tests) -- Testing framework behavior instead of your code -- Ignoring or skipping failing tests without a tracking issue +What stays here is what has to be true before any code is written, which is when +no skill has been summoned yet: test first, and cover all three categories. ## Content scope diff --git a/claude-rules/todo-format.md b/claude-rules/todo-format.md index 2cdc76c..8038b98 100644 --- a/claude-rules/todo-format.md +++ b/claude-rules/todo-format.md @@ -1,3 +1,8 @@ +--- +paths: + - "**/*.org" +--- + # Todo Entry Format Applies to: `**/*.org` (org-mode todo and inbox files) @@ -5,6 +10,56 @@ Applies to: `**/*.org` (org-mode todo and inbox files) How task entries are structured in org-mode todo files (`todo.org`, `inbox.org`, any GTD-style org file). Same shape across every project. +## Stamp `:LAST_REVIEWED:` when you create the task, not a cycle later + +Every task filed at `**` with a priority cookie carries a `:LAST_REVIEWED:` +property from the moment it's written: + +``` +** TODO [#B] Terse topic phrase :tag: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-23 +:END: +Body. +``` + +Use today's date, from `date +%F`. The org-native `[YYYY-MM-DD Day]` form is +equally valid; both parse. + +**Why.** Writing a task *is* reviewing it. Whoever files it has just written +the body, chosen the wording, and graded the priority against the scheme — that +is the same judgment `task-review` applies, made with better context, because +the reason for the task is still in the room. Leaving the stamp off asserts the +opposite: `task-review-staleness.sh` sorts a missing property first, as +never-reviewed, so a task filed today arrives at the top of tomorrow's review +batch and gets "reviewed" by someone re-deriving what its author knew a day +earlier. That is ceremony, and ceremony teaches people to click through the +real thing. + +The concrete case: the 2026-07-23 sweep filed eight tasks in one night. Every +one landed unstamped, and the staleness count went 13 → 22 while the list got +*more* accurate, not less. The number stopped measuring drift and started +measuring recent activity. + +**This applies to every path that files a task**, not just the inbox: inbox +filing, triage intake, spec decomposition, task audit, a bug found mid-session, +a task you write by hand. If you wrote a task body today, stamp it today. + +**What it does not do.** The stamp never means "correct forever" — it means +"a person judged this on that date." A task filed today still enters the +review rotation on the normal cycle; it just enters it on the *next* cycle +rather than immediately. And it's a claim about a real event, so don't stamp +a task you didn't actually consider: a bulk import of someone else's list +is genuinely unreviewed, and stamping it would convert "nobody has read +these" into a false "reviewed today." + +**Enforcement.** `lint-org.el`'s `task-missing-last-reviewed` checker flags any +open `**` task with a priority cookie and no stamp, scoped to exactly the +headings `task-review-staleness.sh` selects, so the checker and the staleness +count never disagree. It's judgment-only and never auto-fixes: nothing can know +when an unstamped task was actually last considered, and writing today's date +onto an old one would destroy the very signal the property carries. + ## Priority and Tag Scheme Header Every project's `todo.org` opens with a top-level section named @@ -41,6 +96,9 @@ fixed definitions everywhere, because autonomous execution eligibility gate and trusts the author's tag rather than re-deriving autonomy at run time. +“Speedrunnable” is shorthand for `:solo:`. The `:quick:` tag plays no part +in that definition. + - **`:solo:` — autonomy.** The task can be completed *and verified* without Craig's involvement beyond at most one or two quick decisions that can be stated and answered before work starts. No open design question, no @@ -64,6 +122,49 @@ step** in the task-review and task-audit workflows, so the run-time gate can trust the tag. A review or audit that skips the `:solo:`/`:quick:` assessment is incomplete. +### Making an open-ended task measurable (so it can be `:solo:`) + +A task phrased as the *absence* of something — "find bugs until none are +visible," "refactor until no worthwhile opportunities remain," "clean this +up until it's good" — cannot be `:solo:`, because it fails the +*verifiable-by-the-agent* gate. Absence isn't falsifiable: an agent can +always look once more, so "done" is a judgment call, which is exactly what +`:solo:` forbids. The fix is not to drop the task but to give it an +objective completion criterion. Four moves convert a fuzzy goal into a +measurable one: + +1. **Bound the surface.** Enumerate the concrete units the task covers (the + N functions, the M files, the named code paths). The done-set is that + list, not the platonic set of all possible defects. Every claim is made + against the list, so "covered" is checkable where "found everything" + isn't. +2. **Net the behavior.** Bring the surface under characterization tests + (Normal/Boundary/Error per unit — see `testing.md`, and the + `testing-standards` skill for the characterization recipe) before changing + anything. This is the objective floor: writing a characterization test is + mechanical (record what the code does, not what it should), so it scales + across the surface, and it doubles as the safety net that makes any + later refactor falsifiable. +3. **Disposition every finding.** Run the relevant audits (a fixed + footgun/OWASP checklist, `/refactor`, `/review-code`) and give **every** + finding a verdict: fixed, filed as its own task, or declined with a + one-line reason. "Looked and it's fine" is not a disposition. The + measurement is zero undispositioned findings, not zero findings. +4. **Gate on an objective floor.** Static analysis clean (linter, + type-checker, `shellcheck`), the test suite green before and after, and + coverage of the enumerated surface (a per-unit test checklist, or a real + coverage number where the tooling exists). + +The **qualifying answer** is then a dispositioned report — surface split +covered/uncovered, tests before → after, static-analysis result, the audit +matrix fully dispositioned, all green — not a claim of perfection. The +honest limit stays honest ("no visible bugs" means "every enumerated path +passes its characterization set and clears the audit," never "zero bugs +exist"), but the criterion is now falsifiable, which is what lets the task +carry `:solo:`. Write these criteria into the task body at creation or +review time; a task that can't be given them stays non-`:solo:` until it +can. + ### Bug priority from severity × frequency (mandatory where a codebase exists) Some projects carry a codebase — source the project maintains under version @@ -217,6 +318,7 @@ A completed sub-task disappears as a task and becomes an in-place event-log entr 2. Generate the timestamp with `date "+%Y-%m-%d %a @ %H:%M:%S %z"`. 3. Reword the original imperative title into the past-tense action that landed. Trim or restate if the original wording doesn't fit the action. 4. Drop the `TODO`/`DOING` keyword, the priority cookie, and the tags. The body stays as the record of what was done (if useful). +5. Remove any `SCHEDULED:`/`DEADLINE:` planning line. The completion time lives in the heading now, so `CLOSED:` is redundant and an active planning date on a historical log entry is always wrong. Org renders any headline carrying an active `<...>` `SCHEDULED`/`DEADLINE` on the agenda, keyword or not, so a stale one pins the finished entry there as weeks-overdue forever. An interactive close (`org-log-done`) stamps `CLOSED:` but never strips a pre-existing planning line, which is exactly how the stale dates survive. **Example:** @@ -226,7 +328,7 @@ becomes *** 2026-05-15 Fri @ 12:58:08 -0500 Wired yasnippet for universal availability -**Enforcement.** This is applied at close time by whoever closes the task, but an interactive org close (`org-log-done` flips the keyword to `DONE` and stamps `CLOSED:`) never applies the dated rewrite, so level-3+ closes accumulate as `DONE` keywords. `todo-cleanup.el --convert-subtasks` (run in the `clean-todo` and wrap-up cleanup passes) normalizes them mechanically: it rewrites any level-3+ `DONE`/`CANCELLED`/`FAILED` heading into the dated form above, pulling the timestamp from the `CLOSED` cookie and keeping the heading text verbatim (a batch tool can't reliably past-tense a title — polish wording by hand where it matters). `lint-org.el` flags any that slip through (checker `subtask-done-not-dated`). So the depth rule holds even when tasks are closed interactively rather than by an agent applying this section. +**Enforcement.** This is applied at close time by whoever closes the task, but an interactive org close (`org-log-done` flips the keyword to `DONE` and stamps `CLOSED:`) never applies the dated rewrite, so level-3+ closes accumulate as `DONE` keywords. `todo-cleanup.el --convert-subtasks` (run in the `clean-todo` and wrap-up cleanup passes) normalizes them mechanically: it rewrites any level-3+ `DONE`/`CANCELLED`/`FAILED` heading into the dated form above, pulling the timestamp from the `CLOSED` cookie, dropping the whole planning line (`CLOSED`, `SCHEDULED`, and `DEADLINE` together — step 5), and keeping the heading text verbatim (a batch tool can't reliably past-tense a title — polish wording by hand where it matters). `lint-org.el` flags any that slip through: checker `subtask-done-not-dated` for a still-keyworded sub-task, and `dated-log-heading-active-timestamp` for a dated entry that kept an active `SCHEDULED`/`DEADLINE`. So the depth rule holds even when tasks are closed interactively rather than by an agent applying this section. ### Why depth-based @@ -309,6 +411,10 @@ tasks — dated entries at `***` and deeper, terminal keyword at `**`. *** 2026-05-15 Fri @ 14:00:00 -0500 <what was answered or done> Generate the timestamp with `date "+%Y-%m-%d %a @ %H:%M:%S %z"`. + Remove any `SCHEDULED:`/`DEADLINE:` planning line too, same as the + sub-task rule above — a dated event-log entry carries its date in the + heading, and an active planning date left on it pins the finished entry + to the agenda forever. - **At `**` — terminal keyword, like any top-level task.** Change `VERIFY` to `DONE` (answered / check passed) or `CANCELLED` (abandoned), diff --git a/claude-rules/working-files.md b/claude-rules/working-files.md index 2432268..b915579 100644 --- a/claude-rules/working-files.md +++ b/claude-rules/working-files.md @@ -26,6 +26,28 @@ hits a nested path instead of a single canonical name. Always rename the files individually with a shared prefix so they sort together but live as flat siblings in `assets/`. +## `working/` Is Version-Controlled From Creation + +`working/` holds the project work currently being developed, so it is +tracked in git the moment a task subdirectory and its artifacts are created — +not staged locally and excluded until it graduates. `working/` is the tracked +home of in-progress work, and it is never added to `.gitignore` (the +install/sweep tooling deliberately leaves it out). + +Filing on completion **reorganizes** durable artifacts into their permanent +homes; it does not mark the point at which they first become durable. The +artifacts were durable — and tracked — from creation. Graduation is a move, +not a promotion from throwaway to keep. + +The corollary: genuinely disposable work does not belong in `working/`. +Ephemeral, single-use, or regenerable artifacts — scratch output, a +throwaway conversion, intermediate data you will delete — go in a project-root +`temp/` directory (gitignored) or system `/tmp`, never in `working/`. +`working/` is for work that will graduate; `temp/` is for work that will be +thrown away. The install tooling ignores `temp/` in both track and +gitignore-mode projects, since ephemerality is independent of whether a +project tracks its `.ai/` tooling. + ## Directory Layout <project-root>/ |