diff options
Diffstat (limited to 'claude-rules')
| -rw-r--r-- | claude-rules/commits.md | 481 | ||||
| -rw-r--r-- | claude-rules/cross-project.md | 60 | ||||
| -rw-r--r-- | claude-rules/interaction.md | 31 | ||||
| -rw-r--r-- | claude-rules/subagents.md | 126 | ||||
| -rw-r--r-- | claude-rules/testing.md | 296 | ||||
| -rw-r--r-- | claude-rules/todo-format.md | 223 | ||||
| -rw-r--r-- | claude-rules/verification.md | 42 |
7 files changed, 1259 insertions, 0 deletions
diff --git a/claude-rules/commits.md b/claude-rules/commits.md new file mode 100644 index 0000000..6c1d275 --- /dev/null +++ b/claude-rules/commits.md @@ -0,0 +1,481 @@ +# Commit Rules + +Applies to: `**/*` + +## Author Identity + +All commits are authored as the user (repo owner / maintainer), never as +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: + +- Commit messages (subject or body) +- PR descriptions and titles +- Issue comments and reviews +- Code comments +- Commit trailers +- Release notes, changelogs, and any public-facing artifact + +This means: + +- **No** `Co-Authored-By: Claude β¦` (or Claude Code, or any AI) trailers +- **No** "Generated with Claude Code" footers or equivalents +- **No** π€ emojis or similar markers implying AI authorship +- **No** references to "Claude", "Anthropic", "LLM", "AI tool" as a credited contributor +- **No** attribution added via template defaults β strip them before committing + +If a tool, template, or default config inserts attribution, remove it. If +settings.json needs it, set `attribution.commit: ""` and `attribution.pr: ""` +to suppress the defaults. + +## 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 + +PR descriptions, Linear ticket bodies, and PR review comments are +visible to the team and to anyone with read access to the repo or +project. Don't mention: + +- Local file paths on the user's personal machine. +- Private repos by name (e.g. a personal notes repo, a career repo). +- Personal tooling or workflow the team doesn't share. +- Anything a teammate couldn't reproduce or act on from public sources. + +Rule of thumb: if a teammate couldn't find the referenced thing without +the user's help, don't reference it. + +**Personal-tooling files.** The rules and skills that drive my workflow are private. Treat the following as personal tooling and don't cite them as authority in any commit message, PR description, PR comment, Linear ticket, or other shared artifact: + +- Anything in `~/code/rulesets/claude-rules/` (`commits.md`, `testing.md`, `verification.md`, `subagents.md`, and any others added later). +- Any `CLAUDE.md`, `AGENTS.md`, or similar project-level rules file. +- Anything under `~/.claude/`, project `.claude/`, or project `.ai/`. +- Any skill definition (`SKILL.md`) under `~/code/rulesets/`. + +Don't write "per `testing.md`, integration tests must hit a real DB" or "the rule in `commits.md` saysβ¦". State the reason directly: "integration tests hit a real DB so the migration is exercised end-to-end." The personal rule doesn't matter to a teammate. The reason does. + +Edge case: when one of these files *is* the change (a commit in the rulesets repo, an edit to a project's `CLAUDE.md`), describe what changed and why without invoking the wider personal-rules layer around it. The commit can absolutely say "tighten testing rule for legacy code". It shouldn't say "per the personal-rules layer this file is loaded intoβ¦". + +Different artifact types carry different content. Don't duplicate. + +**Linear ticket bodies:** two 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 (or what's proposed). + +The causal "why" and the test verification belong in the PR, not the +ticket. Linear's GitHub integration auto-cross-links once the PR body +includes the `Linear:` line, so the ticket reader reaches the PR +without needing a body-level link. + +**PR descriptions:** four sections, in order. Same first two as the +ticket, plus: + +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. + +**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 mode and approval gate.** Before drafting, run this command to decide which mode applies: + +``` +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. **Personal-voice mode**: drafts run through `/voice personal` (39 patterns including the 8 personal-only ones), and the **approval gate applies**. Write to `/tmp`, run the voice pass, 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. The `.ai/` layer is shared with the team, so the personal voice patterns (first-person rewrite, contraction enforcement, semicolon swap, etc.) don't fit. **General-voice mode**: drafts run through `/voice` (general mode, 31 patterns; the 8 personal-only patterns are skipped), and the **approval gate is skipped**. Write to `/tmp`, run the voice pass, print inline, publish immediately. + +The subflows below describe the personal-voice path with the full gate. For the general-voice path: substitute `/voice` for `/voice personal` everywhere, and 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 39 patterns covering signs of AI writing, universal good-writing rules (Strunk & White, Orwell, Plain English, Garner), and the personal voice patterns (first-person rewrite, semicolons β periods/commas, contractions, sentence-split on conjunctions, felt-experience cut, sentence-fragment rewrite, terse cut for rhetorical padding, public-artifact scope flag). 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 β 39 patterns walked"). 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-<ticket-or-slug>.md`. **Title format:** `<conventional-commit subject> (<TICKET-ID>)` β the ticket ID goes at the end in parentheses (e.g. `refactor: remove dead if-count-is-not-None check in admin (SE-289)`). If there is no ticket, omit the parenthetical. The body must also include a `Linear: [<TICKET-ID>](<linear-url>)` line so Linear's GitHub integration auto-cross-links the PR to the ticket. If there is no ticket, state that explicitly ("Linear: n/a") so reviewers know it was considered. +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. After `gh pr create` returns a URL, post a comment on the linked Linear ticket with the PR URL (use the Linear MCP `save_comment` tool, or open the ticket manually if MCP is unavailable). This closes the ticketβPR direction of the cross-link. +8. Move the Linear ticket to the "Dev Review" status (use `save_issue` with the Dev Review state ID, or the Linear UI). The ticket should not remain "In Progress" once a PR is open against it. + +**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. Slack 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 Slack notification. +- **Shape 3 β Reply on an existing inline thread**. Responding to a specific prior reviewer comment. Threads under that comment. No Slack 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 all 39 patterns 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, exactly as it'll be posted, with its separator header. State that the skill ran (e.g. "/voice personal β 39 patterns walked 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 (e.g. `deepsat.ghe.com`). + +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. **Notify the PR author on Slack** (`APPROVE` or `REQUEST_CHANGES` verdicts only). After the review posts, send a one-line Slack message to the PR-review group DM `C0B1B0NH2N5` (the mpdm with Craig, Eric, Vrezh, Kostya β *not* `C0AM2MWHCJU`, which is a separate 3-person Craig/Vrezh/Kostya mpdm and is *not* an "#ai" channel despite older notes calling it that) via the `slack-deepsat` MCP. No `/voice personal` pass β the message is short and templated. Two cases: + - Approved: `<@author-slack-id> Approved PR #N.\n<pr-url>` + - Changes requested: `<@author-slack-id> Changes Requested PR #N\n<pr-url>` + Replace `#N` with the PR number and `<pr-url>` with the GHE URL of the PR. Always lead with an `<@author-slack-id>` mention so the engineer who owns the merge decision gets pinged β the message is useless without it. Resolve the Slack user ID via the slack-deepsat `users_search` tool using the PR author's name or email. The URL goes on its own line, immediately after the message. If the MCP doesn't have a post-message tool registered (for example, in a project where the Slack MCP is read-only), surface that to the user and skip the post β don't fall back to a different channel or asking the user to paste it. + + **Thread under the engineer's review-request post.** Before posting, scan the channel's recent history (`mcp__slack-deepsat__conversations_history`) for the engineer's original post containing the PR URL β typically a "New PR" message or just the PR link. Post the verdict notification with `thread_ts=<original-message-ts>` so it lands in the reply thread off that original. This keeps each PR's review state contained in one thread instead of fragmenting across top-level posts in a mixed feed. The same threading rule applies to both `APPROVE` and `REQUEST_CHANGES` verdicts. + + **No review-request post found?** Don't auto-pick a fallback. Ask the user which they prefer: + - Post the verdict at the top level of the channel. + - DM the engineer directly. + - Don't post at all. + + Skip Slack for `event=COMMENT` reviews (informal feedback, not a verdict) and for Shapes 2 and 3 below. Approve and request-changes are the only verdicts that warrant the notification. + +**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 Slack 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 Slack notification. + +**Approve does not authorize a merge.** The team's practice is approve-then-author-merges, not approve-and-merge. The Slack notification in Step 8 hands the merge decision to the PR author. 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. + +**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 the voice skill before printing the draft. In personal-voice mode it's `/voice personal` β 39 patterns covering AI-writing signs, universal good-writing rules, and the 8 personal-only patterns. In general-voice mode it's `/voice` β the same 31 patterns minus the 8 personal-only ones. The mode is determined by the **Voice mode and approval gate** preamble at the top of Step 2. 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 in the current mode β not a subset. Always state that the skill ran when announcing the printed draft (e.g. "/voice personal β 39 patterns walked" or "/voice β 31 patterns walked"). Skipping the pass without flagging it is a defect. + +### 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). +3. Review the diff β only intended changes staged; no unrelated files. +4. Run tests and linters (see `verification.md`). + +## If You Catch Yourself + +Typing any of the following β stop, delete, rewrite: + +- `Co-Authored-By: Claude` +- `π€ Generated with β¦` +- "Created with Claude Code" +- "Assisted by AI" + +Rewrite the commit as the user would write it: concise, focused on the +change, no mention of how the change was produced. diff --git a/claude-rules/cross-project.md b/claude-rules/cross-project.md new file mode 100644 index 0000000..50bc34e --- /dev/null +++ b/claude-rules/cross-project.md @@ -0,0 +1,60 @@ +# Cross-Project Boundaries + +Applies to: `**/*` + +How to handle requests that target files or tasks belonging to a different project's `.ai/` scope than the current session. + +## The Rule + +When a request points at a file or task living under a *different* project's `.ai/` scope, stop before doing the work. Surface the boundary crossing in one line and ask: "this looks like it belongs to `<other project>`'s session β confirm you want me to do it from here, or switch projects?" + +Each project's `.ai/` directory is the scope boundary. It carries that project's `protocols.org`, `session-context.org`, `sessions/`, `notes.org`, `todo.org`, `inbox/`, and its own memory dir under `~/.claude/projects/<encoded-cwd>/memory/`. Crossing the boundary without flagging it pollutes the current session's log with the other project's content, drops memories into the wrong dir, and skips the other project's protocols / CLAUDE.md / startup-extras that would otherwise apply. + +## When to Detect + +Trigger the check on any of these: + +- A skill or tool argument names a file under another known project (e.g. cwd is `~/.emacs.d/` and the path is `~/projects/work/todo.org`). +- A file read or write would cross into another project's `.ai/`. +- A user request names another project by topic ("the work todo", "the deepsat repo", "my emacs config") while we're not in that project. + +## How to Apply + +State the mismatch and offer the two acceptable answers. Inline numbered options per `interaction.md` β no popup. + +Two acceptable outcomes: + +1. **"Yes, do it from here"** β proceed. Record the cross-project artifact in a handoff file under the *other* project's `inbox/`, named `YYYY-MM-DD-handoff-from-<this-project>-<topic>.org`, with a top note explaining the crossover. The other project's startup workflow picks it up during inbox processing. + + Prefer the `inbox-send` script (`.ai/scripts/inbox-send.py`, auto-synced into every project) over a manual `Write`/`Edit` for the drop. It handles project discovery, source-project provenance in the filename, slug derivation, and timestamping in one call: + + ``` + inbox-send <target> --text "your message" # text β dated .org file + inbox-send <target> --file <path> # copy a file into target/inbox/ + inbox-send --list # see available projects + ``` + + Output filenames follow `YYYY-MM-DD-HHMM-from-<this-project>-<slug>.<ext>` automatically, so the target's next session sees the source + timestamp at a glance without you having to construct the name. Fall back to `Write`/`Edit` only when the script isn't available (e.g. a freshly-cloned project before the first startup-rsync). +2. **"Switch projects"** β stop. Let the user reopen Claude in the right cwd. + +Don't assume which one was meant. Either guess is wrong half the time and the cost of asking once is one short turn. + +## Recovery When It Goes Wrong + +If you do the work first and the boundary issue surfaces afterwards: + +1. Move the cross-project session-log entries out of the current session's `.ai/session-context.org` into `<other-project>/inbox/YYYY-MM-DD-handoff-from-<this-project>-<topic>.org`. Top of that file: a heads-up explaining the crossover so the other project's next session knows what happened. +2. Replace the moved content in `session-context.org` with a brief stub pointing at the handoff file. +3. Move any project-specific memories you saved into the right project's memory dir, or note them in the handoff file if you can't move them. + +## Why + +The user sometimes invokes a skill from whatever shell they happen to be in. The request may be accidental (they meant to be in the other project's terminal) or deliberate (knowing cross-project handoff). The model can't tell from the request alone, and assuming wrong both times costs more than asking once. + +The per-project scope of `.ai/` is the design β protocols, history, memory, inbox, and todo all coupled to one project. Cross-project work breaks every assumption the next session of each project will make. + +## Related + +- `subagents.md` β the per-agent context-isolation discipline. Same principle, smaller scope. +- `interaction.md` β inline numbered options for the "from here / switch?" prompt. +- Per-project `.ai/protocols.org` β the project-scoped instructions this rule protects. diff --git a/claude-rules/interaction.md b/claude-rules/interaction.md new file mode 100644 index 0000000..4d9279b --- /dev/null +++ b/claude-rules/interaction.md @@ -0,0 +1,31 @@ +# Interaction Style + +Applies to: `**/*` + +How Claude communicates with the user during a session β choice prompts, status updates, decision points. + +## No Popup Menus for Choices + +When Claude needs the user to pick between options, **do not** use the AskUserQuestion popup. Present the options inline in chat as a numbered list and ask the user to reply with a number. + +**Why:** The popup menu UI sits at the bottom of the chat window and obscures the chat content directly above it β exactly the area the user needs to read to make the choice. Inline numbered options keep the question, the surrounding context, and the proposed text all visible in the same scrollback. + +**How to apply:** + +For approve / changes / cancel flows (commit-message review, PR-description review, plan approval), draft inline: + +``` +1. Approve β commit now +2. Request changes β tell me what to adjust +3. Open in editor β emacsclient -n /tmp/... + +Pick a number. +``` + +For pick-one decisions, same shape: numbered list, one-line prompt at the end. + +For multi-select decisions, say so explicitly: "Pick any combination β reply with the numbers." + +Reserve `AskUserQuestion` only when the user explicitly asks for the popup form ("use the popup for this one") or for genuinely free-form input where numbered options don't fit. + +This rule applies to all three approval gates in the `commits.md` publish flow (commit message, PR description, PR review reply): print the draft inline, then offer numbered approve / changes / edit options inline. Do not switch to the popup form for the gate even though the prior protocol referenced it. diff --git a/claude-rules/subagents.md b/claude-rules/subagents.md new file mode 100644 index 0000000..310f979 --- /dev/null +++ b/claude-rules/subagents.md @@ -0,0 +1,126 @@ +# Subagents β When, How, and When Not To + +Applies to: `**/*` + +Subagents exist to protect the main thread's context and parallelize +independent work. They are not free β every spawn pays a prompt-construction +cost and breaks the chain of context from the current conversation. Use them +deliberately, not reflexively. + +## When to Spawn a Subagent + +### Parallel-safe (spawn multiple in parallel) + +- **Read-only investigation across independent domains** β "what uses + function X?", "what are the three logging libraries in this repo?", etc. + spread across different subsystems. +- **N independent test-file failures** where each is its own diagnosis. +- **Library/API research across unrelated topics** β doc fan-out. +- **Analysis scans over different parts of a codebase** β e.g. C4 diagram + generation scanning distinct services. + +### Sequential-with-review (one at a time, review between) + +- **Plan-task execution** where each task may depend on the last. +- **Coupled edits** β related files that must stay in sync (schema + migration + + seed script). +- **Anything where mid-course correction is likely** β the review gate is + where course correction happens. + +### Never Parallel + +- **Concurrent writes to the same files or directories** β race conditions, + conflicting edits, lost work. +- **Ordered edits where sequence matters** β e.g. add a config flag, then + read it in code; don't fan these out. + +### Don't Subagent At All + +- **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. + +## Prompt Contract + +Every Agent spawn must include four fields. Missing any one produces +shallow, generic work: + +1. **Scope** β one bounded task, named file or domain. Not "fix the bugs" + but "find the root cause of the NPE in `order_service.py:process_refund`." +2. **Context** β paste the relevant output, error, or prior finding. The + subagent cannot see this conversation. If you learned something from an + earlier turn, include it verbatim; don't paraphrase. +3. **Constraints** β explicit "do NOT" list. "Do not refactor surrounding + code." "Do not add tests." "Do not touch files outside `src/billing/`." +4. **Output format** β what to return and in what shape. "Report root cause + + file:line + proposed fix in under 200 words" beats "investigate this." + +If you can't fill all four fields, you don't yet understand the task well +enough to delegate it. Do it yourself, or think more before dispatching. + +## Context-Pollution Rule + +Subagents exist to absorb noise the main thread shouldn't carry. When one +fails or produces unexpected results: + +- **Do not retry the task manually in the orchestrator context.** That + re-imports the exact noise the subagent was meant to contain β failed + approaches, dead ends, irrelevant exploration. +- **Dispatch a fix subagent** with the failure report as its context + (paste the subagent's output verbatim). New scope, fresh context. +- **If two fix attempts fail**, stop and surface the problem to the user. + Don't keep spawning. + +The corollary: if you're tempted to "just quickly fix it myself after the +agent failed," you are about to pollute your context. Dispatch. + +## Review-Gate Cadence + +Subagent output is not verified work β it's a claim about what was done. +Review before moving on: + +- **Sequential execution** β review after each task completes. Read the + diff, run the relevant tests, confirm the claim matches reality (see + `verification.md`). Then spawn the next task. +- **Parallel execution** β review after every batch of ~3 tasks. Larger + batches compound bugs; smaller batches make review overhead dominate. +- **Never chain subagents past a failed review.** If the review finds a + problem, dispatch a fix subagent before continuing the plan. + +## Delegation vs Understanding + +Subagents execute; they do not understand *for you*. Never write prompts +like: + +- "Based on your findings, fix the bug" β synthesis pushed onto the agent +- "Investigate and implement" β scope too broad, no contract + +Do the understanding step yourself (read the agent's report, decide the +fix), then dispatch the fix with a specific contract. + +## Anti-Patterns + +- **Parallel implementation agents on overlapping files** β they will + conflict. Fan-out is for investigation, not concurrent writes. +- **Broad prompts** β "fix the failing tests" sends the agent exploring; + "fix the assertion at `test_cart.py:142`" gets a diff. +- **Timeout-tuning to quiet flaky tests** β the flake is usually a race + condition. Diagnose, don't mask. +- **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. +- **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" + produces sprawling, unfocused work. You decide scope; the agent + executes it. + +## Cross-References + +- Completion claims must be verified regardless of who produced them β + see `verification.md`. +- Testing discipline applies to subagent-produced tests too β see + `testing.md`. diff --git a/claude-rules/testing.md b/claude-rules/testing.md new file mode 100644 index 0000000..b2ff606 --- /dev/null +++ b/claude-rules/testing.md @@ -0,0 +1,296 @@ +# Testing Standards + +Applies to: `**/*` + +Core TDD discipline and test quality rules. Language-specific patterns +(frameworks, fixture idioms, mocking tools) live in per-language testing files +under `languages/<lang>/claude/rules/`. + +## Test-Driven Development (Default) + +TDD is the default workflow for all code, including demos and prototypes. **Write tests first, before any implementation code.** Tests are how you prove you understand the problem β if you can't write a failing test, you don't yet understand what needs to change. + +1. **Red**: Write a failing test that defines the desired behavior +2. **Green**: Write the minimal code to make the test pass +3. **Refactor**: Clean up while keeping tests green + +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: + +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. + +### Adding Tests to Existing Untested Code + +When working in a codebase without tests: + +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 + +## Test Categories (Required for All Code) + +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). + +## 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 β then throw away the spike, write the test, and implement properly. | + +If you catch yourself thinking any of these, stop and write the test. + +## 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 + +## Content scope + +Test code, fixtures, docstrings, and comments are checked into the repo and visible to the team. They must follow the *Content scope for public artifacts* rule in [`commits.md`](commits.md): no local paths, no private repo names, no personal tooling references. diff --git a/claude-rules/todo-format.md b/claude-rules/todo-format.md new file mode 100644 index 0000000..a8df76a --- /dev/null +++ b/claude-rules/todo-format.md @@ -0,0 +1,223 @@ +# Todo Entry Format + +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. + +## The Rule + +A todo entry has two parts: + +1. **Heading** β terse subject naming just the topic. No action verbs, no + sentence-shape, no dates. Tags belong on the heading line. +2. **Body** (optional) β fuller description: action verbs, context, + rationale, source/origin, links, deadlines. Used when the topic alone + isn't enough. + +When the topic alone is enough, skip the body entirely. + +## Format + + ** TODO [#A] Terse topic phrase :tag1:tag2: + Optional body β fuller description, action verbs, context, links. + + Multi-paragraph body is fine when context warrants it. + +## Examples + +Good: + + ** TODO [#A] Blacken + Prettier config from Vrezh + Ask Jason to implement the formatter config Vrezh sends over. + + ** TODO [#B] TAK-server plugin user scenarios :quick: + Develop with Eric, send to Nate Soule for review. + Out of the 2026-05-13 RTX<>DeepSat sync. + +Bad (sentence-shaped heading, details crammed in): + + ** TODO [#B] Develop TAK-server plugin user scenarios with Eric, send to Nate :quick: + +## Why + +The org agenda view shows the heading. A short heading is scannable; a +sentence-shaped one runs off the edge of the agenda buffer, and the +context that mattered ends up in the truncated tail. The body is always +reachable by visiting the entry β push everything beyond the topic there. + +## How to apply + +When adding a new task: + +1. Pick the smallest noun phrase that names the topic. +2. If anything else is worth saying, put it in the body. +3. Tags go on the heading line, not in the body. + +When restructuring an existing entry that's already sentence-shaped, split +it: keep the topic as the heading, move the rest to the body. + +## Completion β depth-based + +How a finished `TODO` / `DOING` task is closed depends on its depth in the outline. The rule is *depth-based*, not keyword-based, because the agenda truncates beyond level-2 and the visibility tradeoff flips at that boundary. + +### Top-level tasks (`*` and `**`) β stay task-shaped + +A completed `*` section or `**` task keeps its `TODO`/`DOING`-shape so it remains visible in the agenda as a record of what shipped: + +1. Change the keyword to `DONE` (or `CANCELLED` if the task was abandoned rather than completed). +2. Add a `CLOSED: [YYYY-MM-DD Day]` line directly under the heading. Use `date "+%Y-%m-%d %a"` to generate. +3. Leave the original heading text, priority cookie, and tags intact. +4. Optionally add a one-line resolution note in the body. + +The entry is then a candidate for `--archive-done` in the wrap-up cleanup, which moves level-2 `DONE`/`CANCELLED` subtrees from the project's "Open Work" section into "Resolved." + +**Example:** + + ** TODO [#B] Convert <cj structure template to universal yasnippet + +becomes + + ** DONE [#B] Convert <cj structure template to universal yasnippet + CLOSED: [2026-05-15 Fri] + +### Sub-tasks (`***` and deeper) β rewrite to a dated event-log entry + +A completed sub-task disappears as a task and becomes an in-place event-log entry under its parent. The parent's subtree organically grows into a chronological history of what landed without the agenda being cluttered by a long tail of nested `DONE` lines. + +1. Replace the heading with `<same depth> YYYY-MM-DD Day @ HH:MM:SS -ZZZZ <past-tense description>`. +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). + +**Example:** + + *** TODO [#B] Wire yasnippet for universal availability :refactor: + +becomes + + *** 2026-05-15 Fri @ 12:58:08 -0500 Wired yasnippet for universal availability + +### Why depth-based + +The agenda view (`org-agenda`) shows entries at the section + top-task level. Letting `**` tasks stay task-shaped preserves their visibility as "things that recently shipped." Letting `***+` sub-tasks flip to dated entries keeps the agenda from being clogged with a long list of completed sub-tasks at every depth β those become history within their parent instead. + +`VERIFY` is the documented exception: it follows the dated-rewrite rule at **all** depths (including `**`), because a resolved VERIFY is an answered question rather than a finished task. See the VERIFY section below. + +## VERIFY tasks + +`VERIFY` is the keyword for "waiting on Craig's input" β open questions, +pending decisions, drafts awaiting approval. The placement and completion +rules below are stricter than normal TODO tasks because VERIFYs are +open-question placeholders, not regular tasks. + +### Placement β top-level or first-level child only + +A VERIFY task lives at exactly one of two depths: + +1. **Top-level** under the relevant section (e.g., `** VERIFY ...` directly + under `* Work Open Work`). +2. **First-level child** of a parent task (e.g., `*** VERIFY ...` under a + `** TODO` / `** DOING` parent). + +Never deeper. A VERIFY at `****` or below is buried β the agenda view +collapses it under its grandparent and Craig stops seeing it. When you +catch a VERIFY at `****+`, flatten it up to one of the two allowed depths. + +The goal is a flat, scannable task list: the parent task is the topic; its +VERIFYs are the open threads under that topic; the dated history sits as +log entries (which *can* be deeper). + +### Creating a new VERIFY β sibling of its trigger + +When you add a new VERIFY task, place it as a **sibling of the heading +that triggered its creation** β not as a child of that heading. + +The trigger is whatever surfaced the open question: + +- A cj annotation that asks something or marks a pending decision β + trigger is the heading the annotation sits under. +- A task body or sub-task that uncovered an unanswered question while you + were working it β trigger is that task. +- A draft awaiting Craig's sign-off β trigger is the draft's parent task. + +Depth-wise, this means: + +- Trigger at `**` β new VERIFY at `**` (top-level sibling under the + section). +- Trigger at `***` β new VERIFY at `***` (first-level-child sibling under + the same `**` parent). +- Trigger at `****` or deeper β VERIFY can't follow it there (Placement + rule above). Climb the VERIFY up to `***` and surface the buried + trigger as a candidate to flatten. +- Trigger at `*` (a top-level section) β VERIFY goes at `**` (top-level + task under the section). + +File placement: insert the new VERIFY *after* the trigger's entire +sub-tree ends β i.e., after any sub-tasks, dated log headers, or draft +sub-headings the trigger already contains. This keeps everything under a +`**` parent flat: sub-tasks, dated logs, and VERIFYs all live as +siblings at `***`, never burrowing deeper. + +The sibling rule is the active force that keeps `todo.org` flat. Without +it, VERIFYs accumulate one level deeper than their trigger every time β +turning a clean parent tree into a long pole of nested sub-headings. + +### Completion β dated rewrite + content replacement + +When a VERIFY resolves, **rewrite the heading and body together** at the +same depth β regardless of whether the VERIFY is at `**` or `***`: + +1. **Replace the heading.** Drop the `VERIFY` keyword (and any priority + cookie / tags) and replace with a timestamp + short description: + + *** 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"`. + Match the original depth (a `**` VERIFY becomes `** YYYY-MM-DD ...`; + a `***` VERIFY becomes `*** YYYY-MM-DD ...`). + +2. **Replace the body.** Drop the original question/instruction prose and + replace with either: + - **The information Craig provided** (when the VERIFY was a question + β paste the answer, a quote, a link, whatever resolved it), or + - **A description of the action taken** (when the VERIFY was an + instruction or pending-decision marker β what was done, when, where + the artifact lives). + +The completed VERIFY becomes an in-place event log entry. The original +question is preserved by the dated heading + body shape; anyone scanning +the agenda or `git log` can see what was asked and what landed. + +**Note on the top-level case.** Regular `**` DONE tasks stay task-shaped +with a `DONE` keyword + `CLOSED:` line per *Completion β depth-based* +above. VERIFYs at `**` are the exception β they convert to dated log +entries on completion because a resolved VERIFY isn't a "done task," it's +an answered question. The dated-rewrite rule wins for VERIFYs at all +depths. + +### Don't leave stale placeholders + +A well-named VERIFY heading carries the question on its own. Don't append +`cj: <fill in>` placeholder lines under the heading β Craig adds his own +cj comment (a `#+begin_src cj: ... #+end_src` block) when he's ready to +answer, and that's the trigger to process the response. Empty placeholders +are noise that pollute his `cj:` greps. + +### Examples + +**Top-level VERIFY, before / after:** + + ** VERIFY What's our position on the BBN NDA reciprocal-term length? + + ** 2026-05-15 Fri @ 14:00:00 -0500 BBN NDA β standard 2-year reciprocal terms confirmed + Per Nerses 2026-05-15 (Slack DM D0AAAEW3BS4): DeepSat's standard NDA template uses 2-year reciprocal. Sent to BBN legal for sign-off. + +**First-level child VERIFY, before / after:** + + ** DOING [#A] Kostya's contract :admin:kostya: + *** VERIFY Confirm Kostya's basis β part-time hr/week or full-time? + + ** DOING [#A] Kostya's contract :admin:kostya: + *** 2026-05-15 Fri @ 14:00:00 -0500 Kostya basis β part-time, 20 hr/week + Nerses confirmed 5/15 13:30 CDT: Kostya runs at 20 hr/week part-time, mirroring Vrezh's structure. Plugged into Exhibit A Β§ 2 of the contract draft. diff --git a/claude-rules/verification.md b/claude-rules/verification.md new file mode 100644 index 0000000..8993736 --- /dev/null +++ b/claude-rules/verification.md @@ -0,0 +1,42 @@ +# Verification Before Completion + +Applies to: `**/*` + +## The Rule + +Do not claim work is done without fresh verification evidence. Run the command, read the output, confirm it matches the claim, then β and only then β declare success. + +This applies to every completion claim: +- "Tests pass" β Run the test suite. Read the output. Confirm all green. +- "Linter is clean" β Run the linter. Read the output. Confirm no warnings. +- "Build succeeds" β Run the build. Read the output. Confirm no errors. +- "Bug is fixed" β Run the reproduction steps. Confirm the bug is gone. +- "No regressions" β Run the full test suite, not just the tests you added. + +## What Fresh Means + +- Run the verification command **now**, in the current session +- Do not rely on a previous run from before your changes +- Do not assume your changes didn't break something unrelated +- Do not extrapolate from partial output β read the whole result + +## Red Flags + +If you find yourself using these words, you haven't verified: + +- "should" ("tests should pass") +- "probably" ("this probably works") +- "I believe" ("I believe the build is clean") +- "based on the changes" ("based on the changes, nothing should break") + +Replace beliefs with evidence. Run the command. + +## Before Committing + +Before any commit: +1. Run the test suite β confirm all tests pass +2. Run the linter β confirm no new warnings +3. Run the type checker β confirm no new errors +4. Review the diff β confirm only intended changes are staged + +Do not commit based on the assumption that nothing broke. Verify. |