Aif Fix

# Project Rules for /aif-fix

> Auto-generated by `/aif-evolve`. Do not edit manually.
> Last updated: 2026-09-04
> Based on: 2 analyzed patches + /aif-evolve 107-patch batch (2026-06-17, cursor 2026-05-30 → 2026-06-16)
> + /aif-evolve 114-patch batch (2026-07-11, cursor 2026-06-16 → 2026-07-07)
> + /aif-evolve 74-patch batch (2026-09-04, cursor 2026-07-07 → 2026-09-04)

## Rules

### Sweep sibling fields when fixing a boundary-validation bug
**Source**: 2026-05-26-12.45.md, 2026-05-26-12.53.md
**Rule**: When fixing a single Zod field whose value flows into `path.resolve` / `path.join` / `fs.*` / `child_process.spawn` argv / SQL / HTML, audit every sibling field in the same schema in the same pass. Apply the equivalent regex / `.refine()` / length cap to every sibling whose value reaches the same class of sink. This does NOT violate the general "minimal changes / no scope creep" rule — siblings that touch the same sink are part of the same security-fix scope, not "while you're there" cleanup. Reason: in patch 12.45 a regex was added to `runId` / `stepId` but the equally-affected `worktreePath` (consumed by `child_process.spawn` `cwd`) was missed; a follow-up round (patch 12.53) had to land the same fix on the sibling.

**Generalize the sweep far beyond Zod fields.** When fixing ONE instance of a defect class, grep and fix EVERY sibling in the SAME pass: every reader of a leaked field (board / inbox / run-detail all read permission `schema`), every recorder of a value (runner ×2 / takeover / `recordDefaultArtifacts` — the highest-volume recorder is the one usually missed), every route using a shared wrapper, every OpenAPI operation, secret-ref validators (`headers` vs `signing_secret_ref`), and token-revocation sites (detach vs `updateAgentLink({enabled:false})`). Fix the **class**, not the cited line, and RECORD the grep in the fix close-out — a prior patch prescribed exactly this and the next patch records it still didn't happen. A `// mirrors X:line` / `// Task N reconciles both` comment is the author confessing known duplication that will drift — treat it as a must-fix, not a note.
**Source**: 2026-06-02-03.12.md, 2026-06-03-02.56.md, 2026-06-05-16.51.md, 2026-06-08-19.02.md, 2026-06-11-11.54.md, 2026-06-13-20.02.md

Sweep mechanics (2026-07 batch): fix the class at the CHOKE POINT all callers funnel through — grep the funnel (`rg "promoteWorkspaceRun|promoteChildRunForToken|promoteRun"` → all non-scratch promotes funnel through `promoteWorkspaceRun`) and place the guard there, covering every current AND future caller, not per-route. When fixing one branch of an if/else that guards an irreversible side-effect, fix the SIBLING branch in the same pass (the own-worktree `else` twin kept the unfenced CAS); when fixing inside a function with multiple dispatch branches, re-apply the fix above every `return <subpath>(...)` — an early return hides the sibling from the edit set (the `branch_base` twin took two rounds). Recurring sweep sets: every writer of a shared intent/state column (the four `launch_mode` writers); every `listSessions()` caller keyed on the lazily-persisted acp column; every launch path when adding a safety flag (`readOnlySession: true` leaked into the generic scratch launcher — pin the intended NEGATIVE contract too); every cap/validation sibling on the same sink INCLUDING paid-API fields; auth-first twins (the settings PATCH was fixed while its promotion-hold PUT twin kept parse-before-auth); response-handler families sharing a recovery invariant (`rg 'transition === "already-delivered"'` → 5 handlers, 3 laggards). RECORD the classification — fixed / already-safe / intentionally-excluded — and verify sibling REACHABILITY before expanding scope (hook_trip's flow runs never reach `NeedsInputIdle`, so its narrower self-heal was correct); flag same-exposure siblings that are out of scope in the patch rather than silently fixing or omitting them.
**Source (additions)**: 2026-06-22-13.40, 2026-06-22-14.06, 2026-06-24-18.55, 2026-06-25-19.13, 2026-06-27-17.49, 2026-06-25-17.16, 2026-07-02-17.52, 2026-07-03-14.58, 2026-07-01-12.55, 2026-06-24-14.22, 2026-06-22-22.14, 2026-06-23-23.16

2026-07..09 additions: "fix the class" must produce a COMMAND — write the grep, run it, and PASTE its output into the close-out; "I swept it" is not a sweep (the rule was quoted in a session's own patch and still not executed: `PromoteRunBody` was fixed while `ProjectSettingsPatchBody` — same `additionalProperties:false` class on a route the same branch changed — stayed broken). When a fix corrects a CONVENTION, grep the PREDICATE, not the symptom — after fixing one `rootRunId === id`, run `rg "rootRunId ===|root_run_id ="` (three readers shared one wrong belief; searching for "tree queries" found one). Prefer a REQUIRED parameter over a test when sweeping a new argument to N call sites — a `sed` matched two `verifySyncGate` callers and missed the third with a different expression; optionality made it silent, and the compiler surfaced two more sites a scoped grep would have missed (second consecutive round with an unswept sibling). Guarded and unguarded variants of one write coexisting (`failAttemptIfActive` at 1 of 9 sites) are fixed at the choke point. A bulk tag/number sweep reports the FORMS it covered, not a count (`| Designed |` table cells vs `(Designed — ADR-x)` prose). An "ordering violation" sweep is only evidence when it is in-function and CALL-SITE-AWARE (a whole-file grep flagged 44 routes; most were `parseJson` helpers whose call sites auth first) — and parse-before-auth is repo-wide, so record the classification instead of claiming the class fixed. Adding a precondition to a shared function breaks fixtures seeded for the looser contract — correct the SEED, or choose a signal that needs no new dependency (`agents.flow_ref`). Group same-shape sites by CONTROL FLOW before filing them as one class (two of four cascade-then-CAS sites had no exposure; one was misfiled); file same-exposure siblings you leave alone with per-caller reasons.
**Source (additions)**: 2026-07-16-15.20, 2026-09-03-17.20, 2026-07-15-18.10, 2026-07-16-11.20, 2026-07-17-00.00, 2026-08-06-19.10, 2026-08-06-17.03, 2026-07-27-20.55, 2026-09-03-15.37, 2026-09-03-15.06

### When fixing /aif-review findings, default to ALL findings unless explicitly scoped
**Source**: 2026-05-26-12.53.md
**Rule**: When `/aif-fix` is invoked to address output from a previous `/aif-review` (PR review / staged-changes review / commits-mode review), default to addressing every finding in one pass. Do NOT auto-narrow to "critical-only" or "the obvious ones" without the user explicitly scoping. If the user has not scoped, ask once before narrowing. Reason: in M3, round 1 of `/aif-fix` silently addressed 3 of 11 review findings; the gap surfaced only at PR-prep time. Two-pass review on review feedback is the contract.

Hold positions with evidence: when a review flags something a plan explicitly LOCKED with the owner, confirm it is a locked decision and keep it — fix only the genuinely-broken part (non-admin chips stayed non-focusable per Resolved-Q #3; only the name pollution was fixed). Conversely, never defer a KNOWN correctness gap on an auto-merge/promotion path as "out of scope, mention don't fix" — if it gates an irreversible action, it ships in the same pass.
**Source (additions)**: 2026-06-18-12.57, 2026-07-03-16.40

2026-07..09 additions: when a fix re-enables a DEAD guard, the fix is TWO tasks — re-enable it, then re-derive every semantic the spec never had to define (grep the spec for the feature and check its clauses cover the newly reachable states), and FIRST search for compensating workarounds (`rg` the feature name in comments: "X never fires, so we also do Y" is a load-bearing dependency on the bug — enabling X made Y an active killer). Before introducing a stricter helper, grep for the helper the NEIGHBOURING code already uses on the same input (`budgetFromSnapshot` was one grep away; `expandExecutionPolicy` 500'd on a run without a snapshot). Never edit a FROZEN spec as current guidance — cross-reference the canonical contract (`execution-policy.md`) and add a supersession banner naming the replacing ADRs instead.
**Source (additions)**: 2026-09-03-15.06, 2026-09-03-17.21

### Renumbering a shared ADR / migration namespace is grep-to-zero, never a global find-replace
**Source**: 2026-06-10-23.57.md, 2026-06-11-12.51.md, 2026-06-11-14.45.md, 2026-06-09-14.23.md, 2026-06-09-18.47.md
**Rule**: When a sibling branch must renumber its ADR/migration because main already claimed the number, NEVER run a global `sed s/ADR-071/ADR-072/`. Two sibling features each own an adjacent number and each mentions the OTHER's number in prose, so a global replace over-reaches (rewrites a legitimate sibling ref like `[ADR-072](#adr-071-…)`) and the separately-scoped ADR vs migration passes desync (under-reach). Procedure:
1. Enumerate every occurrence and classify each by the feature it names; only the renumbered feature's own references change.
2. Run **ONE** pass covering BOTH the ADR number and the migration number together.
3. Finish with **grep-to-zero** across ALL forms: `ADR-NNN`, the lowercase `adr-nnn` **anchor slug as a SECOND sed pass**, `NNNN_name`, AND bare `` `NNNN` ``. `CLAUDE.md` and `.ai-factory/ROADMAP.md` ARE part of the surface (not just `docs/`); exclude `.ai-factory/plans` / `patches` archives and generated `web/.next`.
4. Leave opaque PERSISTED id-segment literals frozen (`runs.…:adr071:rework-comments` is never parsed back to a number; "correcting" it orphans existing rows) — add a one-line WHY comment so a future reader doesn't "fix" it.
5. Verify the migration number against `migrations/meta/_journal.json`, not the prose (`0029` was documented in 5 places but shipped as `0030`).

Do the renumber/re-port as its OWN focused session, AFTER rebasing/merging onto the integration target — never reconcile docs against a branch baseline that is behind main (editing a stale pre-feature copy re-derives corrections main already shipped). macOS BSD-grep gotcha: for glob-unsafe paths like `app/api/[slug]/…`, `grep -rlZE … | xargs -0 perl` malfunctions — use `grep -rlE … | while IFS= read -r f; do perl -pi -e '…' "$f"; done` (quoted `"$f"` prevents `[slug]` shell-globbing; one perl per file).

Extensions (2026-07 batch): grep-to-zero includes PROSE number forms — `pre-NNNN`, `since NNNN`, `as of NNNN` — and prefers number-agnostic phrasing (`pre-ADR-118`) in long-lived comments so they survive the next renumber. Verify with BOTH single-line and whitespace-tolerant patterns (`grep -z` / multi-line) — a load-bearing claim wrapped across two lines evades a naive grep. Docs greps for columns try BOTH snake_case and the Drizzle camelCase field name (`database-schema.md` documents camelCase). Classify every residual hit as frozen-historical (pre-change ADR bodies, dated findings docs — they describe the past and STAY; annotate with a supersession pointer) vs current-contract (rewritten) — rewriting history falsifies it. A migration is a TRIPLE (SQL file + `_journal.json` entry + `meta/<NNNN>_snapshot.json`) — check the newest journal entry has a matching snapshot; frozen SDD/spec files count as migration-number surfaces (reconcile against the as-built file on disk).
**Source (additions)**: 2026-06-30-00.47, 2026-06-27-17.08, 2026-07-01-12.55, 2026-07-07-13.51, 2026-06-24-03.08

2026-07..09 additions: the grep-to-zero surface after a provenance renumber includes STATUS LABELS (`Designed`/`Accepted` → `Implemented`) and CODE COMMENTS, not only filenames and journal entries — an ADR's own Consequences paragraph is a cross-check against its body (they contradicted); classify each hit by feature so the twin's numbers stay intact. A migration that DROP+re-CREATEs a CHECK/enum on a SHARED table is re-derived from `schema.ts` at rebase, never carried verbatim (0111 dropped main's `run_pr_merged`). `schema.ts` is the FOURTH leg of the migration contract — run `drizzle-kit generate` and read the diff after any hand-authored migration; a proposed undo means the schema module was never updated. A cross-cutting constraint gets the NEXT migration number, never folded into a feature's migration. After a rebase grep consecutive duplicate statements (`awk 'prev==$0 && $0!=""{print FILENAME": "FNR": "$0}{prev=$0}'`).
**Source (additions)**: 2026-07-24-15.19, 2026-07-18-13.35, 2026-09-01-05.40, 2026-07-15-11.19-gate-chat-recovery, 2026-09-04-00.21

### Verify the finding before fixing it
**Source**: 2026-06-22-13.40, 2026-06-23-23.16, 2026-06-22-22.14, 2026-06-18-23.44, 2026-07-02-12.48, 2026-06-25-20.14
**Rule**: Confirm every finding in SOURCE before editing:
1. A reviewer's execution-order inference is checked against the actual lines (side-effect at 666-672 vs gate at 815 was confirmed in source before any edit).
2. A finding's PREMISE ("repo rule X exists") is verified — grep whether the claimed convention actually holds (a claimed "auth-first repo rule" was refuted: parse-before-auth was the universal convention across all 15 admin routes at that time; today auth-first IS the rule — the point is to verify, not assume).
3. A spec-conformant limitation is not a bug — grep the target's own doc comment for a best-effort/never-fail invariant before applying a "fail closed → throw" suggestion ("a write failure never fails the run" made degrade-to-skip the correct fix); document kind-based scope (Known-limitation note + Edge-case bullet) instead of mechanically applying a reviewer's suggestion.
4. When a review claims "X was deleted", establish WHERE X previously lived (`git show HEAD:<path>`, diff old-fixture vs old-deliverable) — the real defect may be that the shipped artifact NEVER had it.
5. A prior fix round's patch note is a CLAIM, not evidence — re-verify asserted guarantees ("worktree-add failures roll the pin back") against the code.

6. A plan's PRESCRIBED remedy is a hypothesis, not a spec — three fixes in one plan were wrong AS WRITTEN though the plan came from a verified review (routing the victim through the function that IS the thief; a `runId` source that exists only for Review runs while reopen targets Done runs); re-derive the defect from source, apply the source's answer, record the divergence.
7. Re-verify a finding's INFERENCE, not just its file:line — accurate line numbers carried rotten reasoning (which file, which mechanism, which DIRECTION: a spec `nullable` became MORE wrong after the cited change); verify the PREMISE of a prescription (7 of 9 ext routes put `runId` in the body); verify its stated CONSEQUENCES by reading the recovery code the reviewer did not open (a slot that "leaks" was freed by `promoteNextPending`); when a plan says "X becomes right after Y", re-derive after Y lands.
8. Rejecting and reversing are valid outcomes: a "broken anchors" finding was rejected after reading the validator (it sanctions the shorthand); a coverage-boundary finding was reversed by amending the spec's too-literal wording instead of deleting a useful test; a "deliberate non-goal" from a prior round is not evidence — re-derive it when challenged; a "duplicate" test may be the ONLY coverage of something (the journal-integrity test checks only the NEWEST snapshot) — check what it uniquely covers before deleting; a STYLE prescription is checked against repo convention (HeroUI `Checkbox` is used nowhere; raw `type="checkbox"` across 8+ components → refuted).
9. Findings against a NEWLY-ACTIVATED feature get provenance triage — "this commit caused it" vs "this commit made it reachable" — each checked against the spec (one asked to reverse a decision the spec locked as "not v1"); check whether a filed "race" is actually DETERMINISTIC (cheaper test, more convincing fix); "reproducible failure" ≠ "product bug" — check WHERE the test failed (a promote returned 200; only the assertion was wrong) before budgeting a fix; a baseline entry that says "real bug" is a claim.
   **Source (additions)**: 2026-07-16-14.05, 2026-07-16-16.40, 2026-07-16-16.05, 2026-07-17-00.00, 2026-07-21-23.50, 2026-07-27-21.50, 2026-09-03-15.06, 2026-09-03-15.37, 2026-09-03-16.10

### `[FIX]` logging yields to the project's logger boundary
**Source**: 2026-06-17-19.53, 2026-06-18-22.18, 2026-06-23-18.25, 2026-06-30-13.02, 2026-07-01-01.27, 2026-07-02-11.12
**Rule**: This repo bans `console.log` (`no-console` lint; pino at service boundaries; pure/client-safe modules carry a "no logging deps — callers log" contract). The fix skill's generic `[FIX]` console.log mandate is satisfied by: module-local pino (`log.warn`/`log.debug`) with structured fields (a `[FIX:...]` marker in the message text is fine), existing service-boundary logging that already covers the debugging surface (state transitions, error payloads embedding the decisive value, `/diagnostics`), typed `MaisterError` refusals, and — for tests / docs / declarative-contract changes (schemas, scope constants, tool specs, pure-const moves) — NO runtime logging at all, with behavior locked by tests instead. Never add `console.log` to satisfy the template; never add logging deps to modules whose contract forbids them.

### ACP/metric debugging heuristics
**Source**: 2026-06-28-19.44, 2026-06-29-20.55-cost-camelcase-usage, 2026-06-29-22.16
**Rule**: (1) "Assistant repeats its first reply verbatim regardless of input" ⇒ per-turn context re-injection dominating the user message — check the prompt builder, not the model. (2) A per-item metric recording only the first item (zeros for every later node) ⇒ locate the SINGLE writer of the artifact (`supervisor/src/cost.ts#extractCost` is the only `cost.jsonl` writer) and audit its event filter (fires only on `session.line`) + key-shape assumptions against REAL adapter output BEFORE touching aggregation code. (3) Replay the real on-disk artifact the bug came from (`.maister/<slug>/runs/<id>/<step>.log` raw JSON-RPC lines) before trusting any shape assumption — synthetic fixtures encode the author's assumption, not the wire.

2026-07..09 additions: (4) a 0-rows-selected query is debugged by replicating the WHERE PIECEWISE in the test (base predicates, then +1 guard at a time) — it localized a wrong-constant assumption (7 d vs `DEFAULT_GC_AGE_DAYS = 14`) in two runs. (5) A vitest worker OOMing before any test body runs ⇒ inspect render/effect identity feedback loops (a `next-intl` mock returning a fresh translator per render) before touching worker/heap settings. (6) A file that fails while each test passes ALONE ⇒ look for LEAKAGE (a parked promise surviving a failed assertion, a barrier not released in `finally`) before a product bug — run the failing tests individually first. (7) An order-dependent e2e flake that passes with `--workers=1` is reproduced by asking which INTERLEAVING opens the render→click window (shared target branch moved by a sibling scenario), not by re-running the default configuration. (8) A killed agent's work is AUDITED before redo: typecheck first (one compile error localizes the frontier), then its test files — failures are usually fixture bugs at the very end, not missing implementation; verdict per slice. (9) A fence "proven" by a mutation is checked for whether the mutation string actually matched.
**Source (additions)**: 2026-07-18-14.55, 2026-07-16-13.03, 2026-09-04-01.33, 2026-09-03-16.10, 2026-07-18-13.35, 2026-09-01-04.15

### Close-out gate battery for fix / review-fix sessions
**Source**: 2026-07-04-01.04, 2026-07-04-14.15-brain-review-followup, 2026-07-03-14.58, 2026-06-18-23.21
**Rule**: Before declaring a fix batch done: `CI=true pnpm validate:docs` + `CI=true pnpm validate:contracts` + `CI=true pnpm --filter maister-web typecheck` + the focused behavior tests + `git --no-pager diff --check`. When a fix adds a precondition to a SHARED function, re-run the FULL consumer suite of that function (a new promote precondition broke 44 files / 282 tests seeded with the old looser state — correct the seeds to the realistic precondition, never weaken the gate). Run the formatter on every NEW test file before committing (`pnpm test:unit` green does not imply `prettier/prettier` clean — lint and test are separate gates; scope `eslint --fix` to the new files only, never repo-wide). For bracket paths (`app/api/runs/[runId]/…`) eslint needs a `*` wildcard for the dynamic segment — the literal path or `\[`-escaping fails with "no files matching".

2026-07..09 additions: the battery is `CI=true pnpm validate:docs:all` (not only `validate:docs`) + `validate:contracts` + web typecheck + `supervisor tsc --noEmit` + `@maister/mcp` typecheck + `pnpm --filter @maister/mcp build` (dist is what runs) + focused tests + `git diff --check`, plus `node scripts/blast-maister-local-state.test.mjs` when local state stores changed. Judge integration regression on NAME SETS, never counts; BASELINE-PROVE before blaming the diff (stash → run on main); classify environment gaps (`supervisor unavailable (network)` = no :7777; `listen EPERM` = sandbox; no container runtime) instead of chasing them. Every Bash call `cd`s to the package dir explicitly. Adversarially re-review EVERY fix cycle before commit — the fix is a change too (two of five findings in a round were regressions from the previous fix; an adversarial pass on a just-shipped fix found the silent-substitution bug it introduced).
**Source (additions)**: 2026-07-11-22.35-m43-review-remediation, 2026-09-02-03.51, 2026-07-27-20.55, 2026-07-17-00.00, 2026-07-11-23.47-mcp-v2-review-fixes, 2026-07-11-12.42, 2026-07-18-13.35, 2026-09-03-17.21, 2026-07-25-12.09

### Fixing past the point of no return: widened cleanup is a regression, not a fix

**Source**: 2026-07-15-18.10, 2026-07-16-11.20, 2026-09-03-17.21, 2026-09-03-15.37
**Rule**: Before touching any `catch`/rollback/restore, list the side effects that may ALREADY have landed above it (force-push, PR create, publish, a committed cascade). When a review asks to "add cleanup" or "add the missing case to the cleanup path", check the sibling branch the widened compensation now ALSO covers — round N's fix (route the catch through `failResolver`) became round N+1's critical (it restored the worktree after a landed push); a phase past the point of no return needs a FORWARD-SETTLE branch (ask origin, settle `succeeded`), never a restore, and an unproven push is never restored. Model the point of no return with a flag every compensating path consults (`pushCommitted`). Before INVERTING an ordering to close a race (CAS-first vs cascade-first), enumerate the recovery for the NEW partial state — a window with a tested backstop beats a narrower window with none; a prescribed inversion was correctly rejected on that evidence. A refused request must leave no trace: if the handler can refuse, move the check BEFORE the first irreversible act rather than trusting the trailing CAS.