From 27f93cabdd806871379527cb0b14e68e39606fa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Tue, 29 Sep 2026 19:20:22 +0000 Subject: [PATCH 1/2] =?UTF-8?q?docs:=20make=20CodeRabbit=20best-effort=20?= =?UTF-8?q?=E2=80=94=20request,=20wait,=20address=20only=20if=20it=20revie?= =?UTF-8?q?ws?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A CodeRabbit review is no longer a hard merge requirement. Agents must still request one (the coderabbit-review label, or the trusted workflow dispatch when the label run does not fire) and wait about 15 minutes. A review that arrives blocks merge until its feedback is resolved. If nothing arrives (silence, a "Review skipped" status, a rate limit, or a failed request workflow), agents record the observed outcome in the PR and merge on the remaining gates. The old wording told agents to add needs-human-review whenever CodeRabbit was silent. That label fails check-specialist-review-evidence, and CodeRabbit has not reviewed an agent-authored PR since #2115 (2026-09-21), so finished PRs stalled waiting for per-PR waivers. Updated every surface that stated the gate: rule 25 (restructured so the local-reviewer and CodeRabbit rules each own their sections), rule 14's state-file tracker, /do Phase 7, the Codex do skill and its prompt, the PR template's CodeRabbit evidence section, and the AGENTS.md guardrail. Also formats rule 14, rule 25 and AGENTS.md (format debt 2118 -> 2115). Co-Authored-By: Claude Opus 5.5 --- .agents/skills/do/SKILL.md | 4 +- .agents/skills/do/agents/openai.yaml | 2 +- .claude/commands/do.md | 19 +++-- .claude/rules/14-do-workflow-persistence.md | 8 +-- .claude/rules/25-review-merge-gate.md | 77 +++++++++++++-------- .github/pull_request_template.md | 13 ++-- AGENTS.md | 4 +- 7 files changed, 72 insertions(+), 55 deletions(-) diff --git a/.agents/skills/do/SKILL.md b/.agents/skills/do/SKILL.md index 78b738cc8b..ad9a2aad25 100644 --- a/.agents/skills/do/SKILL.md +++ b/.agents/skills/do/SKILL.md @@ -1,6 +1,6 @@ --- name: do -description: 'End-to-end autonomous task executor. Takes a task description and handles the full lifecycle: research, plan, implement, review with specialist skills, iterative CodeRabbit review, and merge via PR. Use when given a task to execute end-to-end.' +description: 'End-to-end autonomous task executor. Takes a task description and handles the full lifecycle: research, plan, implement, review with specialist skills, best-effort CodeRabbit review, and merge via PR. Use when given a task to execute end-to-end.' --- # End-to-End Task Executor @@ -17,7 +17,7 @@ Read the full workflow from `.claude/commands/do.md` and execute it. Use `.claud 5. **Validate** — full quality suite: lint, typecheck, test, build 6. **Review** — invoke local specialist skills / local subagents ($go-specialist, $cloudflare-specialist, etc.) 7. **Staging** — check for existing staging deploys (wait 5min if active), trigger manual deployment via `gh workflow run deploy-staging.yml --ref `. **Use `$CF_TOKEN` to query D1/KV/DNS directly** (see `.claude/rules/32-cf-api-debugging.md`) to verify migrations, data state, and feature flags — this is faster and more precise than UI-based checks. Then verify changed behavior end-to-end via Playwright. **For infrastructure changes** (cloud-init, VM agent, DNS, TLS, scripts/deploy): MUST provision a real VM and verify heartbeat arrives. -8. **PR** — create with `gh pr create`, wait for CI, then request CodeRabbit through the trusted GitHub Actions path: apply the `coderabbit-review` label with `gh pr edit --add-label coderabbit-review`; if an explicit retry is needed for the current ready state, run `gh workflow run coderabbit-bot-review.yml --ref main -f pr_number=`. Do not post `@coderabbitai review` directly with an agent token. Treat CodeRabbit as merge-blocking unless the user explicitly waives it: implement or review/close all CodeRabbit feedback, rely on incremental CodeRabbit reviews after pushed fixes, and repeat until the agent and CodeRabbit agree there is no unresolved feedback. If the user requested draft PR / do-not-merge, stop at the draft PR and do not merge. +8. **PR** — create with `gh pr create`, wait for CI, then request CodeRabbit through the trusted GitHub Actions path: apply the `coderabbit-review` label with `gh pr edit --add-label coderabbit-review`; if the label run did not fire (it never fires on a draft PR), run `gh workflow run coderabbit-bot-review.yml --ref main -f pr_number=`. Do not post `@coderabbitai review` directly with an agent token. Then wait about 15 minutes. A CodeRabbit review is not required, but one that arrives blocks merge: implement or resolve every finding with a reason, and give incremental reviews after pushed fixes the same wait. If no review arrives (silence, `Review skipped`, rate limit), record that in the PR's CodeRabbit Review Evidence and continue. Never add `needs-human-review` or wait for a waiver because CodeRabbit is silent (see `.claude/rules/25-review-merge-gate.md`). If the user requested draft PR / do-not-merge, stop at the draft PR and do not merge. 9. **Cleanup** — remove worktree, pull main ## ⚠️ Anti-Compaction: State File diff --git a/.agents/skills/do/agents/openai.yaml b/.agents/skills/do/agents/openai.yaml index fda0fce85f..a96d88e3b4 100644 --- a/.agents/skills/do/agents/openai.yaml +++ b/.agents/skills/do/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Do (Task Executor)" short_description: "End-to-end autonomous task execution" - default_prompt: "Use $do to execute a task end-to-end: research, implement, review, and complete a CodeRabbit-gated PR." + default_prompt: "Use $do to execute a task end-to-end: research, implement, review, request CodeRabbit, and merge via PR." diff --git a/.claude/commands/do.md b/.claude/commands/do.md index cd637b21de..fcf7117c22 100644 --- a/.claude/commands/do.md +++ b/.claude/commands/do.md @@ -25,7 +25,7 @@ TodoWrite([ { content: "Phase 4: Pre-PR validation (lint, typecheck, test, build)", status: "pending", activeForm: "Running full quality suite" }, { content: "Phase 5: Review (local specialist subagents)", status: "pending", activeForm: "Running local reviewer subagents" }, { content: "Phase 6: Staging verification (deploy + Playwright)", status: "pending", activeForm: "Verifying on staging" }, - { content: "Phase 7: Create PR, wait for CI and CodeRabbit, merge", status: "pending", activeForm: "Creating PR and completing review gates" }, + { content: "Phase 7: Create PR, wait for CI, request CodeRabbit and wait, merge", status: "pending", activeForm: "Creating PR and completing review gates" }, ]) ``` @@ -274,13 +274,13 @@ You made a mistake. Close the PR, complete staging verification, then re-open. D 3. **If CI fails:** inspect logs, fix issues, commit, push, repeat. -4. **Once CI is fully green and every non-CodeRabbit gate is satisfied**, request CodeRabbit review through the repository's trusted GitHub Actions path, not by posting `@coderabbitai review` yourself. Apply the opt-in label first: +4. **Once CI is fully green, every other gate is satisfied, and the PR is not a draft**, request CodeRabbit review through the repository's trusted GitHub Actions path, not by posting `@coderabbitai review` yourself. Apply the opt-in label first: ``` gh pr edit --add-label coderabbit-review ``` - If the label-triggered run needs an explicit retry for the current ready state, dispatch the same workflow from `main`: + If the label-triggered run did not fire (the label path never fires on a draft PR), dispatch the same workflow from `main`: ``` gh workflow run coderabbit-bot-review.yml --ref main -f pr_number= @@ -288,16 +288,13 @@ You made a mistake. Close the PR, complete staging verification, then re-open. D Agents MUST NOT post `@coderabbitai review` directly with their own GitHub App token; CodeRabbit ignores bot-authored review commands. The workflow is the human-identity bridge. -5. **Complete the iterative CodeRabbit review loop before merge.** The PR is NOT good to go until the agent and CodeRabbit are in agreement: - - Read every CodeRabbit review comment, thread, and summary. - - Implement valid feedback, push fixes, and re-run the affected validation/CI checks. - - For feedback you believe is not applicable, explicitly review it, document the reason, and close/resolve the thread when GitHub supports that state. - - Keep the `coderabbit-review` label on the PR so CodeRabbit performs incremental reviews for subsequent commits. - - Repeat this loop until the latest CodeRabbit review after the final pushed fixes has no unresolved feedback and you independently agree the PR is ready. +5. **Wait about 15 minutes for CodeRabbit, then follow whichever case applies.** A CodeRabbit review is not required; requesting one and waiting is. See `.claude/rules/25-review-merge-gate.md`. + - **A review arrived: it blocks merge until its feedback is resolved.** Read every CodeRabbit review comment, thread, and summary. Implement valid feedback, push fixes, and re-run the affected validation/CI checks. For feedback you believe is not applicable, explicitly review it, document the reason, and resolve the thread. Keep the `coderabbit-review` label on the PR so pushed fixes get incremental reviews, and give each one the same wait. You are done when no CodeRabbit feedback is unresolved and you independently agree the PR is ready. CodeRabbit not re-reviewing your fixes within the wait does not block. + - **No review arrived: skip CodeRabbit and continue.** This covers silence after the wait, a `Review skipped` status (for example `bot user not eligible for review`), a rate-limit or quota notice, and a failed request workflow. Record what you observed in the PR's "CodeRabbit Review Evidence" section. **Do NOT add `needs-human-review`, call `request_human_input`, or wait for a waiver because CodeRabbit is silent.** - **If CodeRabbit is unavailable, does not respond, or its feedback state cannot be inspected, the PR is not self-mergeable. Add `needs-human-review`, document the blocker, and do NOT merge.** + Do not re-trigger CodeRabbit in a loop; request again only for a materially new ready state. A review that lands after you stopped waiting but before merge is binding. One that lands after merge goes through a follow-up PR. -6. **Once CI is fully green and the CodeRabbit loop is complete**, merge the PR: +6. **Once CI is fully green and the CodeRabbit step is complete** (its feedback resolved, or no review arrived within the wait), merge the PR: ``` gh pr merge --squash --delete-branch diff --git a/.claude/rules/14-do-workflow-persistence.md b/.claude/rules/14-do-workflow-persistence.md index ca61e6cc2e..cb3d9501a7 100644 --- a/.claude/rules/14-do-workflow-persistence.md +++ b/.claude/rules/14-do-workflow-persistence.md @@ -52,8 +52,8 @@ Phase 1: Research & Task Creation ## Phase 7: CodeRabbit Review Tracker - - + + ## Implementation Progress @@ -117,13 +117,13 @@ fallback bounded and record it in the workflow state file. | Repeating already-done work | Checked items + notes show what's been accomplished | | Jumping to PR creation early | Phase checklist enforces ordering | | Merging before reviewers finish | Review Tracker blocks Phase 5 completion until all reviewers report back | -| Forgetting unresolved CodeRabbit feedback | Phase 7 CodeRabbit Review Tracker records label trigger, fix commits, incremental reviews, and final agreement | +| Forgetting unresolved CodeRabbit feedback | Phase 7 CodeRabbit Review Tracker records the request, the wait outcome, and any findings with fix commits | | Silently failing production deploy | Phase 7 checklist includes deploy monitoring — task is not complete until deploy succeeds or user is alerted | | Harness poller disappears after ACP prompt completion | Durable wait subscription wakes the parent through SAM-owned delivery | ## Cleanup -Delete `.do-state.md` at the end of Phase 7 (after CodeRabbit review completion, PR merge, deploy monitoring, and worktree cleanup). It's gitignored, so even if you forget, it won't pollute the repo. +Delete `.do-state.md` at the end of Phase 7 (after the CodeRabbit request-and-wait step, PR merge, deploy monitoring, and worktree cleanup). It's gitignored, so even if you forget, it won't pollute the repo. ## Phase 5 → Phase 6 Transition Guard diff --git a/.claude/rules/25-review-merge-gate.md b/.claude/rules/25-review-merge-gate.md index 18594c96c0..23702e724d 100644 --- a/.claude/rules/25-review-merge-gate.md +++ b/.claude/rules/25-review-merge-gate.md @@ -4,20 +4,6 @@ If you run specialist local subagents during Phase 5 of the `/do` workflow, **every single reviewer must return results and have its findings addressed before you may merge the PR.** There are no exceptions. Filing findings as backlog tasks does not satisfy this requirement for CRITICAL or HIGH severity issues. -## Rule: CodeRabbit Must Agree Before Agent Merge - -For `/do` workflow PRs, once CI and every non-CodeRabbit gate are green, the agent MUST apply the `coderabbit-review` label with `gh pr edit --add-label coderabbit-review`. The label invokes `.github/workflows/coderabbit-bot-review.yml`, which posts the CodeRabbit command through the repository's human-scoped `CODERABBIT_REVIEW_PAT`. The PR is not merge-ready until all CodeRabbit feedback is implemented or explicitly reviewed and closed/resolved, and the latest CodeRabbit review has no unresolved feedback. - -If the label-triggered workflow did not run, needs to be retried, or a fresh explicit review is required after fixes, agents MAY dispatch the same trusted workflow directly: - -```bash -gh workflow run coderabbit-bot-review.yml --ref main -f pr_number= -``` - -Always dispatch the workflow from `main`; do not execute a workflow definition from the PR branch. Agents MUST NOT post `@coderabbitai review` directly with their own GitHub App token: CodeRabbit ignores bot-authored review commands. The workflow is the human-identity bridge. - -This is an iterative gate: keep the `coderabbit-review` label on the PR so CodeRabbit can perform incremental reviews for subsequent commits, and manually dispatch the workflow when an explicit fresh review is needed. Repeat until the agent and CodeRabbit agree there is no unresolved feedback. If CodeRabbit is unavailable, does not respond, or the agent cannot inspect whether feedback remains unresolved, add `needs-human-review` and do not self-merge. - ### Why This Rule Exists PR #568 (Neko Browser Streaming Sidecar) was merged while the go-specialist and security-auditor were still running. Context compaction caused the agent to lose track of outstanding reviewers. The agent merged the PR, then processed the late-arriving reviews and filed 5 backlog tasks for CRITICAL findings — including JWT tokens exposed in URL query parameters and mutex held during Docker I/O. See the retained incident lesson in this rule. @@ -44,35 +30,70 @@ Add this label and stop (do NOT merge) when ANY of: - A reviewer errored or timed out - You cannot confirm whether all reviewers completed (e.g., after context compaction you've lost track) - A reviewer raised CRITICAL findings you cannot fix within the current session -- CodeRabbit is unavailable, does not respond, or its unresolved-feedback state cannot be inspected - You are approaching timeout (75% of max execution time per rule 21) and reviews are incomplete +CodeRabbit not reviewing is **not** a reason to add this label. CodeRabbit is not a local reviewer: silence, a `Review skipped` status, or a rate limit is recorded and then ignored (see the CodeRabbit rule below). Do not list CodeRabbit in the Specialist Review Evidence table either, because a `PENDING` or `FAILED` row there fails the same CI check the label does. + ### The `needs-human-review` Label This label is a **safety valve**, not a failure. It means: "I did the work, but I cannot fully self-verify. A human needs to look before this ships." Creating this label and stopping is the correct action — it is infinitely better than merging with incomplete reviews. If the label doesn't exist yet in the repository, create it: + ```bash gh label create needs-human-review --description "Agent could not complete all review gates — human must approve before merge" --color "D93F0B" ``` -### Quick Compliance Check +## Rule: Request CodeRabbit and Wait — a Review Is Not Required + +A CodeRabbit review is **not** a hard merge requirement. The requirement is to **request one and wait to see whether it arrives**. A review that arrives is binding. A review that never arrives is skipped. + +1. **Request it once the PR is otherwise ready.** When CI and every other gate are green and the PR is not a draft, apply the label with `gh pr edit --add-label coderabbit-review`. The label invokes `.github/workflows/coderabbit-bot-review.yml`, which posts the CodeRabbit command through the repository's human-scoped `CODERABBIT_REVIEW_PAT`. If the label run did not fire (the label path never fires on a draft PR), dispatch the same trusted workflow directly: + + ```bash + gh workflow run coderabbit-bot-review.yml --ref main -f pr_number= + ``` + + Always dispatch the workflow from `main`; do not execute a workflow definition from the PR branch. Agents MUST NOT post `@coderabbitai review` directly with their own GitHub App token: CodeRabbit ignores bot-authored review commands. The workflow is the human-identity bridge. + +2. **Wait about 15 minutes for a response.** Watch the PR's reviews, review comments, and CodeRabbit's check and status comment. If CodeRabbit has visibly started a review that is still in progress, wait for it to finish. + +3. **If a review arrives, it blocks merge until its feedback is resolved.** Implement valid feedback, push, and re-run the affected checks. For feedback you judge inapplicable, reply with the reason and resolve the thread. Keep the `coderabbit-review` label on the PR so pushed fixes get incremental reviews, and give each one the same wait. The PR is ready when no CodeRabbit feedback is unresolved. CodeRabbit not re-reviewing your fixes within the wait does not block. + +4. **If nothing arrives, skip CodeRabbit and continue.** Any of these means CodeRabbit did not review: no response within the wait, a `Review skipped` status (for example `bot user not eligible for review` or `automatic reviews are disabled`), a rate-limit or quota notice, or a failed request workflow. Record what you observed in the PR's "CodeRabbit Review Evidence" section and merge on the remaining gates. Do **not** add `needs-human-review`, call `request_human_input`, or wait for a human waiver because CodeRabbit is silent. + +5. **Do not re-trigger in a loop.** Request again only for a materially new ready state, such as a large rework. Re-triggering an unresponsive or rate-limited CodeRabbit burns the shared review quota and cannot change a `Review skipped` outcome. + +6. **Late reviews still count.** A review that lands after you stopped waiting but before merge is binding under step 3. One that lands after merge is handled through a follow-up PR, never a direct commit to `main` (hard requirement 6 above). + +### Why CodeRabbit Is Best-Effort + +In mid-September 2026 CodeRabbit's free OSS quota began rate-limiting agent PRs. After 2026-09-21 it stopped reviewing agent-authored PRs altogether, reporting `Review skipped: bot user not eligible for review` or `automatic reviews are disabled`. The old wording treated that silence as "cannot verify, escalate", so agents added `needs-human-review` and waited for per-PR waivers, and finished PRs stalled for hours or days. On 2026-09-29 Raphaël changed the rule: requesting a review and waiting for it is mandatory, receiving one is not. + +## Quick Compliance Check Before merging any agent-authored PR: +Local reviewers: + - [ ] PR description has "Specialist Review Evidence" table - [ ] Every local reviewer has a row in the table - [ ] Every row shows `PASS` or `ADDRESSED` (not `PENDING` or `FAILED`) - [ ] All CRITICAL/HIGH findings are fixed in the branch (not deferred to backlog) -- [ ] PR description has "CodeRabbit Review Evidence" filled out -- [ ] Latest CodeRabbit review has no unresolved feedback -- [ ] If any of the above are false: `needs-human-review` label added and merge deferred - -### What This Rule Prevents - -| Without this rule | With this rule | -|---|---| -| Agent merges with outstanding reviewers after context compaction | Agent must populate PR table — compaction doesn't affect the PR | -| CRITICAL findings filed as backlog tasks post-merge | CRITICAL findings block merge; human decides on deferrals | -| No visibility into which reviewers actually ran | PR table is auditable by humans | -| Agent self-approves all quality gates | `needs-human-review` creates a human checkpoint for uncertain cases | +- [ ] If any local-reviewer item above is false: `needs-human-review` label added and merge deferred + +CodeRabbit: + +- [ ] Requested once the PR was otherwise ready, and the wait was observed +- [ ] If it reviewed: every finding is implemented or resolved with a reason, and no CodeRabbit feedback is unresolved +- [ ] If it did not review: the observed outcome is recorded in "CodeRabbit Review Evidence", with no `needs-human-review` label and no waiver request + +## What This Rule Prevents + +| Without this rule | With this rule | +| ------------------------------------------------------------------ | --------------------------------------------------------------------- | +| Agent merges with outstanding reviewers after context compaction | Agent must populate PR table — compaction doesn't affect the PR | +| CRITICAL findings filed as backlog tasks post-merge | CRITICAL findings block merge; human decides on deferrals | +| No visibility into which reviewers actually ran | PR table is auditable by humans | +| Agent self-approves all quality gates | `needs-human-review` creates a human checkpoint for uncertain cases | +| A silent CodeRabbit parks finished PRs behind `needs-human-review` | The non-response is recorded and the PR merges on its remaining gates | diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 9525794201..dec84066dd 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -102,7 +102,7 @@ All checkboxes below are mandatory for any PR that changes runtime code (`.ts`, ## Specialist Review Evidence (Required for agent-authored PRs) -If local subagents were used during Phase 5, list every reviewer below. **Do NOT merge until every row shows PASS or ADDRESSED.** If any reviewer could not complete (timeout, workspace killed, error), you MUST add the `needs-human-review` label and stop — do not self-merge. See `.claude/rules/25-review-merge-gate.md`. +If local subagents were used during Phase 5, list every reviewer below. **Do NOT merge until every row shows PASS or ADDRESSED.** If any reviewer could not complete (timeout, workspace killed, error), you MUST add the `needs-human-review` label and stop — do not self-merge. CodeRabbit is not a local reviewer: record it in the CodeRabbit section below, not in this table. See `.claude/rules/25-review-merge-gate.md`. - [ ] **All local reviewers completed and findings addressed before merge** - [ ] **If any reviewer did NOT complete: `needs-human-review` label added and merge deferred to human** @@ -125,16 +125,15 @@ If this is not an agent-authored PR, write `N/A: human-authored PR`. ## CodeRabbit Review Evidence (Required for agent-authored PRs) -When every non-CodeRabbit gate is satisfied, the agent must apply the `coderabbit-review` label to this PR. Do **not** merge until all CodeRabbit feedback is either implemented or explicitly reviewed and closed/resolved. Keep the label on the PR so pushed fixes receive incremental CodeRabbit reviews; repeat until the latest review has no unresolved feedback and the agent agrees the PR is ready. +Once every other gate is satisfied and the PR is not a draft, the agent applies the `coderabbit-review` label (or dispatches the trusted workflow) and waits about 15 minutes. A CodeRabbit review is **not** required. **If CodeRabbit reviews, its feedback blocks merge** until every finding is implemented or explicitly resolved with a reason. **If it does not review** (silence, `Review skipped`, rate limit), record what you observed below and merge on the remaining gates. A missing review never calls for `needs-human-review` or a waiver. See `.claude/rules/25-review-merge-gate.md`. -- [ ] `coderabbit-review` label applied after local review, staging if applicable, and CI gates passed -- [ ] All CodeRabbit findings implemented or explicitly reviewed and closed/resolved -- [ ] Incremental CodeRabbit review completed after final pushed fixes, or no fixes were needed -- [ ] Latest CodeRabbit review has no unresolved feedback +- [ ] CodeRabbit requested after local review, staging if applicable, and CI gates passed +- [ ] Waited about 15 minutes, or longer while a review CodeRabbit had started was still in progress +- [ ] Either CodeRabbit reviewed and no CodeRabbit feedback is unresolved, or it did not review and the observed outcome is recorded below ### CodeRabbit Notes - + ## Exceptions (If any) diff --git a/AGENTS.md b/AGENTS.md index dd5c4b72a7..23ac43053e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,9 +84,9 @@ These are Codex-facing reminders for recurring SAM workflow failures. The durabl | Dispatching a SAM task | Verify the task started, the title matches, the requested profile/agent is observable, and critical constraints such as `/do`, branch, `draft PR`, or `do not merge` survived. Do not start dispatched task descriptions with `/do` or any slash command; write prose such as `Execute this task using the /do skill.` so Codex does not reject the prompt before SAM bootstrap instructions are processed. | | Sizing a dispatched SAM task (`resourceRequirements` / `vmSize`) | Prefer omitting sizing so the pool packs the task onto an existing node. Otherwise look for prior signals (previous task notes, observed OOM kills, comparable tasks), cite the source, and cap at 2 vCPU and a 4 GB machine (`minMemoryGb` at most 3.5, because the 512 MB host reserve is subtracted before matching) unless an OOM was observed at that size for the same kind of work. See `.claude/rules/09-task-tracking.md`. | | Retrying or redispatching after a failed SAM task | First inspect the failed task/session and check for active duplicate work with the same prompt, output branch, branch, or title. Do not blindly submit the same prompt again after no-workspace/startup failures or transient provider failures. | -| Reviewing previous sessions or conversation history | Search tools keep long normal multi-word queries searchable by matching retained terms separately, while generous byte/term guardrails disclose any trimming through `queryTruncated`, the effective `query`, and `queryLimits`. Refine a truncated query when exact coverage matters; see `.claude/rules/09-task-tracking.md`. | +| Reviewing previous sessions or conversation history | Search tools keep long normal multi-word queries searchable by matching retained terms separately, while generous byte/term guardrails disclose any trimming through `queryTruncated`, the effective `query`, and `queryLimits`. Refine a truncated query when exact coverage matters; see `.claude/rules/09-task-tracking.md`. | | Draft PR / do-not-merge request | Preserve the constraint in task state and PR wording. Stop at the draft/open PR unless Raphaël later authorizes readiness or merge. | -| CodeRabbit review gate | Request CodeRabbit through the repository GitHub Actions path: apply the `coderabbit-review` label, or dispatch `gh workflow run coderabbit-bot-review.yml --ref main -f pr_number=` for a needed explicit retry. Do not post `@coderabbitai review` directly with an agent token; the workflow uses the trusted human-scoped token that CodeRabbit accepts. | +| CodeRabbit review gate | Apply the `coderabbit-review` label, or dispatch `gh workflow run coderabbit-bot-review.yml --ref main -f pr_number=` if it did not fire; never post `@coderabbitai review` with an agent token. Wait about 15 minutes. A review that arrives blocks merge until resolved; if none arrives (silence, `Review skipped`, rate limit), record that and continue without `needs-human-review` or a waiver. See `.claude/rules/25-review-merge-gate.md`. | | Deployment setup/config changes | Prefer generated Pulumi-managed platform secrets with explicit override paths. Do not add manual GitHub Environment prerequisites for deployment-owned keys or values SAM can safely create during deployment. | | Profile/default-profile work | Fresh installs should not seed multiple provider-specific built-in agent profiles. Prefer a setup wizard, templates, or at most one conversational default so users learn profiles intentionally instead of inheriting clutter. | | Incidental bug found | If it is not blocking and not a small adjacent fix, file a backlog task with reproduction/evidence and continue the assigned work. | From fdb96cb4c8f2a322f2807d62b4f314d8b7254836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Tue, 29 Sep 2026 19:28:08 +0000 Subject: [PATCH 2/2] docs: bound the in-progress CodeRabbit wait and align summaries Address doc-sync-validator findings on the CodeRabbit best-effort rule: - /do Phase 7 now carries the "let a started review finish" clause that rule 25 and the PR template already had. - Cap that extension at about 45 minutes in total, after which the review counts as not arriving, so a stuck review cannot block forever. - Reword rule 25's checklist item ("the ~15-minute wait completed"). - Add "do not re-trigger in a loop" to the AGENTS.md row and the Codex do skill summary. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/do/SKILL.md | 2 +- .claude/commands/do.md | 2 +- .claude/rules/25-review-merge-gate.md | 4 ++-- .github/pull_request_template.md | 2 +- AGENTS.md | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.agents/skills/do/SKILL.md b/.agents/skills/do/SKILL.md index ad9a2aad25..ee4c8dea40 100644 --- a/.agents/skills/do/SKILL.md +++ b/.agents/skills/do/SKILL.md @@ -17,7 +17,7 @@ Read the full workflow from `.claude/commands/do.md` and execute it. Use `.claud 5. **Validate** — full quality suite: lint, typecheck, test, build 6. **Review** — invoke local specialist skills / local subagents ($go-specialist, $cloudflare-specialist, etc.) 7. **Staging** — check for existing staging deploys (wait 5min if active), trigger manual deployment via `gh workflow run deploy-staging.yml --ref `. **Use `$CF_TOKEN` to query D1/KV/DNS directly** (see `.claude/rules/32-cf-api-debugging.md`) to verify migrations, data state, and feature flags — this is faster and more precise than UI-based checks. Then verify changed behavior end-to-end via Playwright. **For infrastructure changes** (cloud-init, VM agent, DNS, TLS, scripts/deploy): MUST provision a real VM and verify heartbeat arrives. -8. **PR** — create with `gh pr create`, wait for CI, then request CodeRabbit through the trusted GitHub Actions path: apply the `coderabbit-review` label with `gh pr edit --add-label coderabbit-review`; if the label run did not fire (it never fires on a draft PR), run `gh workflow run coderabbit-bot-review.yml --ref main -f pr_number=`. Do not post `@coderabbitai review` directly with an agent token. Then wait about 15 minutes. A CodeRabbit review is not required, but one that arrives blocks merge: implement or resolve every finding with a reason, and give incremental reviews after pushed fixes the same wait. If no review arrives (silence, `Review skipped`, rate limit), record that in the PR's CodeRabbit Review Evidence and continue. Never add `needs-human-review` or wait for a waiver because CodeRabbit is silent (see `.claude/rules/25-review-merge-gate.md`). If the user requested draft PR / do-not-merge, stop at the draft PR and do not merge. +8. **PR** — create with `gh pr create`, wait for CI, then request CodeRabbit through the trusted GitHub Actions path: apply the `coderabbit-review` label with `gh pr edit --add-label coderabbit-review`; if the label run did not fire (it never fires on a draft PR), run `gh workflow run coderabbit-bot-review.yml --ref main -f pr_number=`. Do not post `@coderabbitai review` directly with an agent token. Then wait about 15 minutes. A CodeRabbit review is not required, but one that arrives blocks merge: implement or resolve every finding with a reason, and give incremental reviews after pushed fixes the same wait. If no review arrives (silence, `Review skipped`, rate limit), record that in the PR's CodeRabbit Review Evidence and continue. Do not re-trigger in a loop, and never add `needs-human-review` or wait for a waiver because CodeRabbit is silent (see `.claude/rules/25-review-merge-gate.md`). If the user requested draft PR / do-not-merge, stop at the draft PR and do not merge. 9. **Cleanup** — remove worktree, pull main ## ⚠️ Anti-Compaction: State File diff --git a/.claude/commands/do.md b/.claude/commands/do.md index fcf7117c22..13dd9a2bf4 100644 --- a/.claude/commands/do.md +++ b/.claude/commands/do.md @@ -288,7 +288,7 @@ You made a mistake. Close the PR, complete staging verification, then re-open. D Agents MUST NOT post `@coderabbitai review` directly with their own GitHub App token; CodeRabbit ignores bot-authored review commands. The workflow is the human-identity bridge. -5. **Wait about 15 minutes for CodeRabbit, then follow whichever case applies.** A CodeRabbit review is not required; requesting one and waiting is. See `.claude/rules/25-review-merge-gate.md`. +5. **Wait about 15 minutes for CodeRabbit, then follow whichever case applies.** If CodeRabbit has visibly started a review that is still in progress, let it finish, up to about 45 minutes in total. A CodeRabbit review is not required; requesting one and waiting is. See `.claude/rules/25-review-merge-gate.md`. - **A review arrived: it blocks merge until its feedback is resolved.** Read every CodeRabbit review comment, thread, and summary. Implement valid feedback, push fixes, and re-run the affected validation/CI checks. For feedback you believe is not applicable, explicitly review it, document the reason, and resolve the thread. Keep the `coderabbit-review` label on the PR so pushed fixes get incremental reviews, and give each one the same wait. You are done when no CodeRabbit feedback is unresolved and you independently agree the PR is ready. CodeRabbit not re-reviewing your fixes within the wait does not block. - **No review arrived: skip CodeRabbit and continue.** This covers silence after the wait, a `Review skipped` status (for example `bot user not eligible for review`), a rate-limit or quota notice, and a failed request workflow. Record what you observed in the PR's "CodeRabbit Review Evidence" section. **Do NOT add `needs-human-review`, call `request_human_input`, or wait for a waiver because CodeRabbit is silent.** diff --git a/.claude/rules/25-review-merge-gate.md b/.claude/rules/25-review-merge-gate.md index 23702e724d..fae025f841 100644 --- a/.claude/rules/25-review-merge-gate.md +++ b/.claude/rules/25-review-merge-gate.md @@ -56,7 +56,7 @@ A CodeRabbit review is **not** a hard merge requirement. The requirement is to * Always dispatch the workflow from `main`; do not execute a workflow definition from the PR branch. Agents MUST NOT post `@coderabbitai review` directly with their own GitHub App token: CodeRabbit ignores bot-authored review commands. The workflow is the human-identity bridge. -2. **Wait about 15 minutes for a response.** Watch the PR's reviews, review comments, and CodeRabbit's check and status comment. If CodeRabbit has visibly started a review that is still in progress, wait for it to finish. +2. **Wait about 15 minutes for a response.** Watch the PR's reviews, review comments, and CodeRabbit's check and status comment. If CodeRabbit has visibly started a review that is still in progress, wait for it to finish, up to about 45 minutes in total. Past that, treat it as no review. 3. **If a review arrives, it blocks merge until its feedback is resolved.** Implement valid feedback, push, and re-run the affected checks. For feedback you judge inapplicable, reply with the reason and resolve the thread. Keep the `coderabbit-review` label on the PR so pushed fixes get incremental reviews, and give each one the same wait. The PR is ready when no CodeRabbit feedback is unresolved. CodeRabbit not re-reviewing your fixes within the wait does not block. @@ -84,7 +84,7 @@ Local reviewers: CodeRabbit: -- [ ] Requested once the PR was otherwise ready, and the wait was observed +- [ ] Requested once the PR was otherwise ready, and the ~15-minute wait completed - [ ] If it reviewed: every finding is implemented or resolved with a reason, and no CodeRabbit feedback is unresolved - [ ] If it did not review: the observed outcome is recorded in "CodeRabbit Review Evidence", with no `needs-human-review` label and no waiver request diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index dec84066dd..4422c8b5e5 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -128,7 +128,7 @@ If this is not an agent-authored PR, write `N/A: human-authored PR`. Once every other gate is satisfied and the PR is not a draft, the agent applies the `coderabbit-review` label (or dispatches the trusted workflow) and waits about 15 minutes. A CodeRabbit review is **not** required. **If CodeRabbit reviews, its feedback blocks merge** until every finding is implemented or explicitly resolved with a reason. **If it does not review** (silence, `Review skipped`, rate limit), record what you observed below and merge on the remaining gates. A missing review never calls for `needs-human-review` or a waiver. See `.claude/rules/25-review-merge-gate.md`. - [ ] CodeRabbit requested after local review, staging if applicable, and CI gates passed -- [ ] Waited about 15 minutes, or longer while a review CodeRabbit had started was still in progress +- [ ] Waited about 15 minutes, or up to about 45 minutes in total while a review CodeRabbit had started was still in progress - [ ] Either CodeRabbit reviewed and no CodeRabbit feedback is unresolved, or it did not review and the observed outcome is recorded below ### CodeRabbit Notes diff --git a/AGENTS.md b/AGENTS.md index 23ac43053e..750d0eaafe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -86,7 +86,7 @@ These are Codex-facing reminders for recurring SAM workflow failures. The durabl | Retrying or redispatching after a failed SAM task | First inspect the failed task/session and check for active duplicate work with the same prompt, output branch, branch, or title. Do not blindly submit the same prompt again after no-workspace/startup failures or transient provider failures. | | Reviewing previous sessions or conversation history | Search tools keep long normal multi-word queries searchable by matching retained terms separately, while generous byte/term guardrails disclose any trimming through `queryTruncated`, the effective `query`, and `queryLimits`. Refine a truncated query when exact coverage matters; see `.claude/rules/09-task-tracking.md`. | | Draft PR / do-not-merge request | Preserve the constraint in task state and PR wording. Stop at the draft/open PR unless Raphaël later authorizes readiness or merge. | -| CodeRabbit review gate | Apply the `coderabbit-review` label, or dispatch `gh workflow run coderabbit-bot-review.yml --ref main -f pr_number=` if it did not fire; never post `@coderabbitai review` with an agent token. Wait about 15 minutes. A review that arrives blocks merge until resolved; if none arrives (silence, `Review skipped`, rate limit), record that and continue without `needs-human-review` or a waiver. See `.claude/rules/25-review-merge-gate.md`. | +| CodeRabbit review gate | Apply the `coderabbit-review` label, or dispatch `gh workflow run coderabbit-bot-review.yml --ref main -f pr_number=` if it did not fire; never post `@coderabbitai review` with an agent token. Wait about 15 minutes; never re-trigger in a loop. A review that arrives blocks merge until resolved; if none arrives (silence, `Review skipped`, rate limit), record that and continue without `needs-human-review` or a waiver. See `.claude/rules/25-review-merge-gate.md`. | | Deployment setup/config changes | Prefer generated Pulumi-managed platform secrets with explicit override paths. Do not add manual GitHub Environment prerequisites for deployment-owned keys or values SAM can safely create during deployment. | | Profile/default-profile work | Fresh installs should not seed multiple provider-specific built-in agent profiles. Prefer a setup wizard, templates, or at most one conversational default so users learn profiles intentionally instead of inheriting clutter. | | Incidental bug found | If it is not blocking and not a small adjacent fix, file a backlog task with reproduction/evidence and continue the assigned work. |