From 7290be3f83b313ab549699bbd3882c33e0994a3d Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:44:41 +0200 Subject: [PATCH 1/3] docs: codify branch/merge-discipline lessons from the WS-recovery sprint Persists three lessons learned during the PROMPT-WSS-v1.24.x recovery session for future sessions/contributors: - Never commit directly to main -- always branch + PR, even for trivial single-file changes (corrected once this session after a direct-to-main commit slip). - Wait for the full CI suite -- including non-required/advisory jobs like E2E -- to go green before merging, not just branch-protection-required checks. - Group related small workstreams into the fewest reviewable PRs (by natural/documented boundaries) instead of one PR per tiny item, while keeping one commit per logical concern. Also documents two GitHub merge-gate quirks hit repeatedly this session: the mergeable-state cache-lag false block, and stacked-PR auto-close (not retarget) when a squash-merge deletes a branch other PRs are based on, plus the recovery procedure for each. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index a89858f38..fbed70d44 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -55,6 +55,12 @@ pnpm run token:audit # audit-tokens.mjs — design-token usage gate (CI b **PR-size limit — keep every PR under ~100 changed files so CodeAnt actually reviews it.** CodeAnt does **not** post inline review comments on PRs that exceed ~100 changed files (the >100-file check hangs/skips). Since any i18n-touching change fans out across 19 locale source files + 19 rebuilt `bundle.json` per module, a multi-feature branch crosses 100 fast. **Before pushing, run `git diff --name-only ...HEAD | wc -l`.** If it is over ~100, split the work into the **fewest** stacked PRs that each stay clearly under the limit — group by which locale module-files they touch so the per-PR fan-out stays small (e.g. P0 batch touching `writer.json`; P1/P2 batch touching `common.json`/`dashboard.json`). Stack them (PR2 base = PR1's branch) so each PR's incremental diff — what CodeAnt sees — is small; when PR1 merges, PR2 auto-retargets to `main`. **Do not** make more PRs than needed: if everything fits under ~100 in one (or two) PRs, use that. Keep commits atomic per concern regardless of how they are bundled into PRs. +**Branching & merge discipline (every change, no exceptions):** Never commit directly to `main` — always create a feature branch, push, and open a PR, even for a single-file doc/config/chore edit. Before merging, wait for the **full CI suite to go green, including non-required/advisory jobs** (`E2E Tests`, `E2E Deep Coverage`, `Storybook`, `Lighthouse`, `Visual Regression`) — not just the branch-protection-required checks (Security Audit, Build, Quality Gate ×2). When doing a structured multi-step sprint (an audit, a migration broken into workstreams), group related small workstreams into the fewest PRs that stay reviewable — by natural/documented boundaries, not one PR per tiny item — while keeping one commit per logical concern inside each PR. + +**Known merge-gate quirks (GitHub, this repo):** +- **Mergeable-state cache lag:** `gh pr merge` can fail with "base branch policy prohibits the merge" even when `mergeable: MERGEABLE`, every required check is `success`, and 0 review threads are unresolved — `mergeStateStatus` can also just be stuck on a still-pending advisory job (the merge button blocks on *any* running check, required or not). Re-poll a few times at ~60s spacing; if a check is still `pending`, wait for it instead of retrying blindly. Never use `--admin` to route around this without a maintainer's fresh, explicit authorization for that specific merge. +- **Stacked-PR auto-close on squash-merge:** squash-merging a PR with `--delete-branch` can cause GitHub to **auto-close (not retarget)** a downstream PR whose base was the just-deleted branch, instead of the usual automatic retarget-to-`main`. Recovery: `git push origin :refs/heads/` to temporarily restore the ref, `gh pr reopen `, `gh pr edit --base main`, then delete the temp branch once `gh pr list --state open --json baseRefName` shows nothing still depends on it. + **E2E notes:** Do NOT use `networkidle` waits (HMR keeps WebSocket open). Scope sidebar navigation via `#sidebar`. Shared helpers: `tests/e2e/helpers.ts`. Mobile E2E: set `RUN_MOBILE_E2E=1` locally (off by default). **Feature-flag E2E coverage (anti-pattern guard):** Every test that relies on a specific flag state MUST use `setFeatureFlags(page, {...})` from `helpers.ts` to make that dependency explicit and guard against future default changes. Call it BEFORE `page.goto()` — it uses `addInitScript` so it runs before app JS. From f0db397526114612adf0b2b5ede10f56f32708b6 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:47:37 +0200 Subject: [PATCH 2/3] docs: explicitly call out CodeRabbit nitpick + outside-diff-range comments The existing "ALL inline comments" wording technically already covered these, but they're collapsed by default in the review UI and easy to miss in practice -- reinforced twice this session, so making it explicit in the policy text rather than relying on it being implied. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index fbed70d44..8f01ba4bd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ pnpm run token:audit # audit-tokens.mjs — design-token usage gate (CI b - **OSV vulnerabilities**: Run `pnpm audit` or check the security CI job. Add `pnpm.overrides` with pinned exact versions. - Correction loop: fix → commit → verify CI → fix until all jobs green. -**PR review-comment policy — the CodeAnt Correction Loop (proactive, automatic, every PR):** Fix ALL inline comments (CodeAnt AI + any bot/human) on every PR, **without being asked**. Validate findings against the *current* code (anchors may be stale); implement real **root-cause** fixes (code **+ tests + i18n + docs** in lockstep) or reply with evidence if a false positive. **Never add a new `biome-ignore`** — the suppression ratchet (`scripts/check-suppressions.mjs`) fails the quality gate; refactor so the rule passes honestly. Reply to each thread citing the resolving commit (`POST .../comments//replies`), resolve it (GraphQL `resolveReviewThread`), leave **0 unresolved**. Then commit, push, and **re-trigger** (`gh pr comment --body "@codeant-ai review"`). **Iron rule — loop until quiescent:** a push triggers a fresh review that often raises NEW findings (a "wave"); repeat the full cycle until **BOTH** a fresh review yields **0 new comments** AND **0 threads unresolved**. Only then merge (auto-squash; admin-squash only after CI is green + loop quiescent). Full canonical procedure: [`docs/CODEANT-REVIEW-LOOP.md`](docs/CODEANT-REVIEW-LOOP.md). +**PR review-comment policy — the CodeAnt Correction Loop (proactive, automatic, every PR):** Fix ALL inline comments (CodeAnt AI + any bot/human) on every PR, **without being asked** — this explicitly includes CodeRabbit's collapsed **nitpick** sections and **outside-diff-range** findings (both easy to miss since they're collapsed by default in the review UI), not just its top-level actionable comments. Validate findings against the *current* code (anchors may be stale); implement real **root-cause** fixes (code **+ tests + i18n + docs** in lockstep) or reply with evidence if a false positive. **Never add a new `biome-ignore`** — the suppression ratchet (`scripts/check-suppressions.mjs`) fails the quality gate; refactor so the rule passes honestly. Reply to each thread citing the resolving commit (`POST .../comments//replies`), resolve it (GraphQL `resolveReviewThread`), leave **0 unresolved**. Then commit, push, and **re-trigger** (`gh pr comment --body "@codeant-ai review"`). **Iron rule — loop until quiescent:** a push triggers a fresh review that often raises NEW findings (a "wave"); repeat the full cycle until **BOTH** a fresh review yields **0 new comments** AND **0 threads unresolved**. Only then merge (auto-squash; admin-squash only after CI is green + loop quiescent). Full canonical procedure: [`docs/CODEANT-REVIEW-LOOP.md`](docs/CODEANT-REVIEW-LOOP.md). **PR-size limit — keep every PR under ~100 changed files so CodeAnt actually reviews it.** CodeAnt does **not** post inline review comments on PRs that exceed ~100 changed files (the >100-file check hangs/skips). Since any i18n-touching change fans out across 19 locale source files + 19 rebuilt `bundle.json` per module, a multi-feature branch crosses 100 fast. **Before pushing, run `git diff --name-only ...HEAD | wc -l`.** If it is over ~100, split the work into the **fewest** stacked PRs that each stay clearly under the limit — group by which locale module-files they touch so the per-PR fan-out stays small (e.g. P0 batch touching `writer.json`; P1/P2 batch touching `common.json`/`dashboard.json`). Stack them (PR2 base = PR1's branch) so each PR's incremental diff — what CodeAnt sees — is small; when PR1 merges, PR2 auto-retargets to `main`. **Do not** make more PRs than needed: if everything fits under ~100 in one (or two) PRs, use that. Keep commits atomic per concern regardless of how they are bundled into PRs. From d868a112843951b59b2195434ec2eb7e741272dd Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Thu, 30 Jul 2026 10:01:04 +0200 Subject: [PATCH 3/3] fix(docs): distinguish GitHub's pending-check block from this repo's wait-for-pass policy Addresses a CodeRabbit finding on PR #294: the cache-lag paragraph read as if GitHub only ever blocks on required checks, conflating two distinct things -- GitHub blocking the merge button while ANY check (required or not) is still pending, versus this repo's own stricter policy of waiting for advisory jobs to actually pass, not just stop being pending, before merging. Spelled both out separately so the cache-lag description isn't mistaken for either policy. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8f01ba4bd..eb373b523 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -58,7 +58,7 @@ pnpm run token:audit # audit-tokens.mjs — design-token usage gate (CI b **Branching & merge discipline (every change, no exceptions):** Never commit directly to `main` — always create a feature branch, push, and open a PR, even for a single-file doc/config/chore edit. Before merging, wait for the **full CI suite to go green, including non-required/advisory jobs** (`E2E Tests`, `E2E Deep Coverage`, `Storybook`, `Lighthouse`, `Visual Regression`) — not just the branch-protection-required checks (Security Audit, Build, Quality Gate ×2). When doing a structured multi-step sprint (an audit, a migration broken into workstreams), group related small workstreams into the fewest PRs that stay reviewable — by natural/documented boundaries, not one PR per tiny item — while keeping one commit per logical concern inside each PR. **Known merge-gate quirks (GitHub, this repo):** -- **Mergeable-state cache lag:** `gh pr merge` can fail with "base branch policy prohibits the merge" even when `mergeable: MERGEABLE`, every required check is `success`, and 0 review threads are unresolved — `mergeStateStatus` can also just be stuck on a still-pending advisory job (the merge button blocks on *any* running check, required or not). Re-poll a few times at ~60s spacing; if a check is still `pending`, wait for it instead of retrying blindly. Never use `--admin` to route around this without a maintainer's fresh, explicit authorization for that specific merge. +- **Mergeable-state cache lag vs. this repo's own wait-for-everything policy — two different things:** (1) GitHub itself blocks the merge button while *any* check is still `pending`, required or not (a real, observed technical constraint — it clears on its own once every check concludes, pass or fail); separately, once all checks have actually concluded, GitHub's branch protection only re-blocks on a *failing required* check. (2) This repo's own policy above is stricter than that floor: wait for the advisory jobs to *pass*, not just stop being `pending`, before merging. If `gh pr merge` still fails with "base branch policy prohibits the merge" after every job (required and advisory) shows a concluded `success`, and `mergeable: MERGEABLE`, and 0 review threads are unresolved, that's the mergeable-state *cache* lagging behind reality, not either policy above. Re-poll a few times at ~60s spacing. Never use `--admin` to route around any of this without a maintainer's fresh, explicit authorization for that specific merge. - **Stacked-PR auto-close on squash-merge:** squash-merging a PR with `--delete-branch` can cause GitHub to **auto-close (not retarget)** a downstream PR whose base was the just-deleted branch, instead of the usual automatic retarget-to-`main`. Recovery: `git push origin :refs/heads/` to temporarily restore the ref, `gh pr reopen `, `gh pr edit --base main`, then delete the temp branch once `gh pr list --state open --json baseRefName` shows nothing still depends on it. **E2E notes:** Do NOT use `networkidle` waits (HMR keeps WebSocket open). Scope sidebar navigation via `#sidebar`. Shared helpers: `tests/e2e/helpers.ts`. Mobile E2E: set `RUN_MOBILE_E2E=1` locally (off by default).