From 48f09d34af40abfc34171236ba733f2aa5210bd4 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Tue, 5 May 2026 13:59:27 -0400
Subject: [PATCH] docs(context): add recurring PR mistakes distilled from
 CodeRabbit reviews

---
 CONTEXT.md | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/CONTEXT.md b/CONTEXT.md
index 56446772e..08c061964 100644
--- a/CONTEXT.md
+++ b/CONTEXT.md
@@ -39,3 +39,36 @@ Module owning command resolution, policy projection (`mutation`, `output_mode`),
 
 ### Query Pre-Project Config Policy Module
 Module policy that defines query-time behavior when `.planning/config.json` is absent: use built-in defaults for parity-sensitive query Interfaces, and emit parity-aligned empty model ids for pre-project model resolution surfaces.
+
+---
+
+## Recurring PR mistakes (distilled from CodeRabbit reviews, 2026-05-05)
+
+### Tests — no source-grep
+- **Rule**: never bind `readFileSync` result to a var then call `.includes()` / `.match()` / `.startsWith()` on it. CI runs `scripts/lint-no-source-grep.cjs` and exits 1.
+- **Escape**: add `// allow-test-rule: <reason>` anywhere in the file to exempt the whole file. Use when reading product markdown or runtime output (not `.cjs` source).
+- **Pattern to reach for instead**: call the exported function, capture stdout/JSON, assert on typed fields.
+
+### Tests — no unescaped RegExp interpolation
+- `new RegExp(\`prefix${someVar}\`)` — if `someVar` can contain `.` or other metacharacters (e.g. phase id `5.1`), the pattern is wrong. Always `escapeRegex(someVar)`. The `escapeRegex` utility is in `core.cjs` and already imported in most modules.
+
+### Tests — no dead regex branches in `.includes()`
+- `src.includes('foo.*bar')` is always false — `.*` is a regex metacharacter, not a wildcard in `includes`. Either use `new RegExp('foo.*bar').test(src)` or delete the branch.
+
+### Tests — guard top-level `readFileSync` against ENOENT
+- Module-level `const src = fs.readFileSync(...)` throws before any `test()` registers, aborting the runner with an unhandled exception instead of a named failure. Wrap in try/catch and rethrow with a helpful message.
+
+### Changesets — `pr:` field must be the PR number, not the issue number
+- The `pr:` key in `.changeset/*.md` frontmatter must reference the PR introducing the fix (e.g. `3142`), not the issue it closes (e.g. `3120`). Changelog tooling links to GitHub PRs by this value.
+
+### Shell hooks — never interpolate `$VAR` into single-quoted JS strings
+- `node -e "require('$HOOK_DIR/lib/foo.js')"` breaks silently if `$HOOK_DIR` contains a single quote (POSIX-legal). Pass paths via env vars: `GIT_CMD_LIB="$HOOK_DIR/lib/foo.js" node -e "require(process.env.GIT_CMD_LIB)"`.
+
+### Shell guards — `[ -f .git ]` does not detect worktrees from main repo
+- In the main repo `.git` is a directory, so `[ -f .git ]` is false and the entire guard is skipped. Use `git rev-parse --git-dir` and match `*.git/worktrees/*` in a `case` statement instead.
+
+### Shell guards — absolute-path containment must use `root/` prefix, not glob
+- `[[ "$PATH" != "$ROOT"* ]]` matches sibling prefixes (`/repo-extra` passes when `ROOT=/repo`). Use `[[ "$P" != "$ROOT" && "$P" != "$ROOT/"* ]]`. Also: check `[ -z "$ROOT" ]` and exit 1 before the containment test. Warn → fail-closed for security-relevant path checks.
+
+### Docs — keep internal reference counts consistent
+- When a heading says `(N shipped)` and a footnote says `N-1 top-level references`, update the footnote. CodeRabbit catches this every time.