docs(skill+pitfalls): four new defect classes from the 2043/2042 review round (#2069)

Pitfalls: runtime state and key material never enter commits (16), new views
wire desktop AND mobile registries (17), retrofit columns on shipped stores
need guarded ALTERs because the migration runner baselines pre-existing DBs
without executing (18), scope deltas vs the issue are stated explicitly (19).
Extended pitfall 1 with the house project auth-guard pattern and pitfall 15
with the answer-in-thread requirement.

Skill: fold-first now requires in-thread replies per numbered item and bot
review freshness checks; new scope-honesty subsection covers deferral notes,
follow-up issues, and the git status runtime-artifact check.

Author: jaylfc

.claude/skills/taos-development-skill/SKILL.md

@@ -240,6 +240,15 @@ A finding folded within hours merges the same day; a finding left while you open
 new PRs stalls the whole train behind it (the maintainer will not merge past an
 open finding, ever).
 
+Folding means code AND a reply: answer every numbered item in the PR thread
+with the commit that addresses it or a concrete rebuttal. Code pushed without
+in-thread replies leaves the fold formally open and the verdict at HOLD.
+
+Bot review freshness is part of folding: check WHICH commit a bot actually
+reviewed. A rate-limited or stale "SUCCESS" on an older head is not a pass -
+after pushing fixes, re-trigger the review (for CodeRabbit: comment
+`@coderabbitai review`).
+
 ### Rebase cadence and the stale-base rule
 
 - dev moves fast. When your PR shows CONFLICTING, rebase onto current dev
@@ -265,6 +274,16 @@ A close without a successor link reads as lost work and forces the maintainer
 into git forensics (this happened with #1927/#1924 - both were legitimate
 "landed via" closures that looked like data loss for hours).
 
+### Scope honesty per slice
+
+- The PR body states exactly what it ships versus what its issue scopes. Any
+  deferred part gets an explicit deferral note AND a follow-up issue filed in
+  the same push; the parent issue never auto-closes on a partial slice.
+- Before committing, `git status` must show only intended source changes -
+  runtime artifacts (anything under `data/`) never enter a commit, and a new
+  component that writes under `data/` gitignores its directory in the same PR
+  (pitfall 16).
+
 ### One PR per slice
 
 - A fix and the test that proves it belong in ONE PR (pitfall 13 in

docs/contributor-pitfalls.md

@@ -12,6 +12,10 @@ session + CSRF, or the agent-token allowlist in `auth_middleware.py`) and a
 test asserting an unauthenticated or cross-principal caller gets 401/403.
 Example: PR #2036 added `GET /api/secrets/agent/{name}/github` with no guard,
 leaking installation IDs and repo names to any caller.
+For project-scoped routes, copy the house guard verbatim:
+`if not user.is_admin and user.user_id != p["user_id"]` followed by a masked
+404 (see routes/projects.py). A bare 403 without the admin bypass both locks
+out admins and leaks resource existence to other users (#2042).
 
 **2. Bind both directions on authenticated channels.**
 When a message or envelope arrives over an authenticated channel, verify BOTH
@@ -37,6 +41,16 @@ and strips it. Tokens we only VERIFY are stored hashed; tokens we must PRESENT
 outbound are the only ones stored recoverable. Example: PR #2009 moved the
 GitHub App RSA key out of plaintext config.yaml.
 
+**16. Never commit runtime state or key material.**
+Anything a store or subsystem writes under `data/` at runtime is state, not
+source. A PR that introduces a component writing under `data/` must add that
+directory to `.gitignore` in the SAME PR, and `git status` must be checked for
+runtime artifacts before every commit. Any private key that reaches a pushed
+commit is burned: regenerate it, and keep it out of the target branch's history
+(squash merge, or rewrite the branch). Example: `data/hub/identity.json` with
+live signing/encryption keys entered one branch's history and was then
+re-committed by a second PR (#2043 history, #2042).
+
 ## Correctness
 
 **5. Never take element `[0]` of a collection that can hold more than one.**
@@ -74,6 +88,12 @@ share flow), state it in the PR body and file a follow-up issue. Example: PR
 permissions. Wire the real value or do not add the parameter yet. Example:
 PR #2036 `handleSaveGrants`.
 
+**17. A new view must be wired into every surface it has: desktop AND mobile.**
+The desktop tab list and the mobile tab order are separate registries; updating
+one and not the other ships a view that is unreachable on phones (#2042:
+`TABS` updated, `mobileTabOrder` missed). Grep for every registry the sibling
+views appear in and update all of them.
+
 ## Store and schema
 
 **11. SCHEMA is the frozen v1; new columns and their indexes go in MIGRATIONS.**
@@ -87,6 +107,18 @@ Running a data migration twice must be a no-op (existence checks, INSERT OR
 IGNORE, migrated-from markers) and there must be a test proving the second run
 changes nothing. Example done right: PR #2028 `test_migrate_idempotent`.
 
+**18. Retrofitting a column onto an already-shipped store needs a guarded
+ALTER, not just a MIGRATIONS entry.**
+The migration runner baselines pre-existing databases at the latest version
+WITHOUT executing the migrations (FOOTGUN #2 in `db_migrations.py`'s own
+docstring), so a plain `MIGRATIONS = [(1, "ALTER TABLE ...")]` on a store that
+already shipped is a silent no-op on every upgraded install: fresh DBs work,
+upgraded DBs lose the feature at runtime. Use the guarded `_post_init` pattern
+(PRAGMA table_info, ALTER only when the column is absent - see
+`knowledge_store._migration_v1_add_user_id`) and add an upgrade test that
+builds the PRE-change schema first. Fresh-DB tests are structurally blind to
+this class (#2043: `peer_fingerprint`).
+
 ## Process
 
 **13. A fix and its test belong in one PR.**
@@ -110,3 +142,15 @@ Findings are gated on severity of content, not review state. Address each one
 (fix it or rebut it concretely in the thread); never merge past an open
 finding, and never let a "pass" verdict from a stale commit stand in for the
 current head.
+A finding is closed only when it is ANSWERED IN-THREAD: reply to each numbered
+item with the commit that addresses it or a concrete rebuttal. Pushing code
+without replies leaves the finding open - the reviewer re-verifies blind and
+the verdict stays HOLD (#2043 round two).
+
+**19. State scope deltas against the issue explicitly.**
+If a PR ships less than its issue scopes - a subfeature dropped, owner-only
+routes where the issue says peer-serving, a "live" view that fetches once -
+say so in the PR body and file the follow-up issue in the same push. Never let
+a partial slice close the parent issue. Silent shortfalls read as done, get
+caught in review anyway, and cost a full extra round (#2042: community chat
+and peer access absent with no deferral note).