Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
70f0084
feat: implement issue #2045 — dev-lead: no deferred/out-of-scope outc…
Oct 3, 2026
2152ac5
fix(reviews): address review comments [skip ci-relay]
don-petry Oct 3, 2026
566862c
fix(reviews): address review comments [skip ci-relay]
don-petry Oct 3, 2026
2c67484
fix(reviews): address review comments [skip ci-relay]
don-petry Oct 3, 2026
f153197
fix: handle command substitution failures under set -euo pipefail in …
Oct 4, 2026
4dfa863
fix(reviews): guard command substitutions under set -e, revalidate tr…
Oct 4, 2026
98a7d94
fix(reviews): treat explicit hasNextPage=false as single page in defe…
Oct 4, 2026
866072f
fix(reviews): anchor GitHub hostname in deferred-thread discussion UR…
don-petry Oct 4, 2026
4042045
fix(reviews): resolve deferrals on guard aborts, propagate enumeratio…
Oct 4, 2026
08a0853
fix(bot): correct scoped URL regex in deferred-thread-verify.sh
don-petry Oct 6, 2026
19dbbed
fix(bot): correct scoped URL regex in deferred-thread-verify.sh
don-petry Oct 6, 2026
1ebb0be
fix(reviews): fail closed on tracker-comment pagination errors, split…
Oct 7, 2026
7e8d33a
fix(reviews): verify resolve response, escape repo dots, dedupe stub …
Oct 8, 2026
509cca7
fix(reviews): fail closed on non-404 tracker-issue fetch errors; requ…
don-petry Oct 8, 2026
2810779
fix(reviews): detect 404 from gh stdout body as well as stderr
Oct 8, 2026
402a772
fix(reviews): resolve deferrals before terminal markers, strict 404 d…
Oct 8, 2026
ce96c58
fix(reviews): handle finalize_review_application failures with retry …
don-petry Oct 8, 2026
fc6a9ff
Merge branch 'main' into dev-lead/issue-2045-20261003-1350
don-petry Oct 9, 2026
b670c10
fix(reviews): make deferred-thread resolution work and recover on fai…
Oct 9, 2026
3a93825
fix(reviews): count an unreadable edit time as a failure; require a c…
Oct 9, 2026
bb38d3b
fix(reviews): record resolve-failed as a non-quota hold; a malformed …
Oct 9, 2026
38adcb1
fix(reviews): reason-aware hold dedup and an accurate resolve-failed …
Oct 9, 2026
83fcfd5
Merge branch 'main' into dev-lead/issue-2045-20261003-1350
don-petry Oct 9, 2026
73d32a1
Merge branch 'main' into dev-lead/issue-2045-20261003-1350
don-petry Oct 9, 2026
edb92ae
fix(dev-lead): fail closed on partial deferral snapshots; retry fix-b…
don-petry Oct 9, 2026
d9f47e7
Merge branch 'main' into dev-lead/issue-2045-20261003-1350
don-petry Oct 9, 2026
22cb533
fix(dev-lead): key resolve-failed retry markers to the pushed head (#…
don-petry Oct 9, 2026
1dbe01a
Merge remote-tracking branch 'origin/dev-lead/issue-2045-20261003-135…
don-petry Oct 9, 2026
642ebc0
fix(dev-lead): retry deferral resolution failures on failed passes to…
don-petry Oct 9, 2026
7bf83ec
fix(dev-lead): stop on a non-advancing thread cursor; never retitle h…
don-petry Oct 9, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@
tests/dev-lead/unit/test_claim_landing.bats \
tests/dev-lead/unit/test_test_tamper_guard.bats \
tests/dev-lead/unit/test_test_regression_guard.bats \
tests/dev-lead/unit/test_deferred_thread_verify.bats \
tests/dev-lead/unit/test_fewshot_assemble.bats \
tests/dev-lead/unit/test_net_diff_guard.bats \
tests/dev-lead/unit/test_git_history.bats \
Expand Down Expand Up @@ -309,7 +310,7 @@
timeout-minutes: 5
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
# NOTE: persist-credentials is intentionally left at the default (true) —

Check warning on line 313 in .github/workflows/lint.yml

View workflow job for this annotation

GitHub Actions / Lint

313:9 [comments-indentation] comment not indented like content
# vci_resolve_reusable needs `git fetch origin` to authenticate against this
# private repo when resolving same-repo channel tags. Setting
# persist-credentials: false would silently push every such caller into the
Expand Down
16 changes: 16 additions & 0 deletions prompts/dev-lead/fix-bot-comment.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,22 @@ The claim payload is schema `v1` — one comment per reply, with a full 40-char

**Do not resolve the thread yourself.** You must not call the `resolveReviewThread` (or `unresolveReviewThread`) GraphQL mutation under any circumstance — resolution is done **only** by the harness (`dev-lead-fix-reviews.sh`), whose deterministic guards are the authoritative merge gate. The harness resolves every bot thread you addressed with an our-account addressed-marker reply, plus any thread from this bot marked `isOutdated: true`. Your reply and its marker are your only lever on resolution.

## Deferring a valid finding that is out of scope (#2045)

When a thread from this bot is a real finding that does not belong in this PR, do not leave a bare skip note: that thread can never be resolved and blocks merge. Defer it to the repo's **single** `dev-lead: deferred review findings` tracking issue (never one issue per finding):

1. Find the open issue with that exact title (`gh issue list --repo ${REPO} --state open --search 'in:title "dev-lead: deferred review findings"' --json number,title`) and use a result only when its title is **exactly** `dev-lead: deferred review findings` (the search also matches titles like `… (legacy)`). If none exists, reuse the tracker this repo already cites, renaming it to that title only when our own account opened it and its body holds nothing but deferred-finding entries (never retitle a human-authored or general-purpose issue); otherwise create one. If more than one is open, use the lowest-numbered.
2. Get the originating comment URL by querying the thread node: `gh api graphql -f query='query($id:ID!){node(id:$id){... on PullRequestReviewThread{comments(first:1){nodes{url}}}}}' -f id=<thread id>`. Append that URL plus a one-line summary to the issue: `gh issue comment <n> --repo ${REPO} --body "…"`.
3. Reply to the thread with the reason and the tracker, ending with **exactly one** marker and **no** addressed-marker or claim:

```
Valid, but deferring — out of scope for this PR: <reason>. Tracked in #<n>.

<!-- dev-lead:deferred ref=#<n> -->
```

The harness resolves the thread, on commit and no-commit passes alike, only when your latest reply carries exactly one such marker and `#<n>` is an **open** issue whose body or comments link the thread. Deferral is for **bot** threads only; never defer a marker-less maintainer thread, and if a maintainer marked the finding required, fix it instead.

## SonarQube / SonarCloud comments

If `${ACTOR}` is `sonarqubecloud[bot]` and the comment reports security hotspots or ratings **without referencing specific files or line numbers**, the SonarCloud dashboard link is not browsable — you must infer the hotspot from the PR's changed files:
Expand Down
26 changes: 22 additions & 4 deletions prompts/dev-lead/fix-reviews.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,29 @@ So: edit, **commit locally**, then reply citing that commit's SHA. If you end up

For a thread that is `isOutdated: true` with no code change, a reply is optional — a one-line note that the referenced code no longer exists is helpful but not required.

#### Deferring a valid bot finding that is out of scope (#2045)

When a **bot** thread's finding is real but does not belong in this PR, do not leave a bare skip note: that thread can never be resolved and blocks merge. Defer it to the repo's **single** deferred-findings tracking issue instead (AC6 — the same issue you use for issue-comment `out-of-scope` dispositions; never one issue per finding):

1. Find that issue: `gh issue list --repo ${REPO} --state open --search 'in:title "dev-lead: deferred review findings"' --json number,title`, and use a result only when its title is **exactly** `dev-lead: deferred review findings` (the search also matches titles like `… (legacy)`). If there is no exact-title result, look for the tracker this repo already uses — an open issue cited by an earlier `disposition=out-of-scope ref=#<n>` reply or by a prior `dev-lead:deferred` marker — and, if it is titled differently, **rename it** to exactly that title (`gh issue edit <n> --repo ${REPO} --title "dev-lead: deferred review findings"`) rather than opening a second tracker — but **only** when our own account opened that issue and its body holds nothing but deferred-finding entries. Never retitle a human-authored or general-purpose issue (an `out-of-scope` ref can point at an ordinary feature issue); in that case create a new tracker instead. Only if no such issue exists, create one with exactly that title. Concurrent runs can race here, so after creating it search again and, if more than one open issue carries the title, use the lowest-numbered one (and note the duplicate on it).
2. Fetch the originating comment's URL — the supplied thread JSON omits it — by querying the thread node by its `id`: `gh api graphql -f query='query($id:ID!){node(id:$id){... on PullRequestReviewThread{comments(first:1){nodes{url}}}}}' -f id=<thread id>`. Append the finding to the issue with that URL (`…/pull/${PR_NUMBER}#discussion_r<id>`) plus a one-line summary: `gh issue comment <n> --repo ${REPO} --body "…"`.
3. Reply to the thread saying why it is deferred and where it is tracked, ending with **exactly one** deferral marker, and **no** addressed-marker or claim:

```
Valid, but deferring — out of scope for this PR: <reason>. Tracked in #<n>.

<!-- dev-lead:deferred ref=#<n> -->
```

The harness resolves the thread on its own, on commit and no-commit passes alike, but only when your latest reply on the thread carries exactly one such marker and `#<n>` is an **open issue** whose body or comments link the thread. A missing, closed or non-linking issue, or a second marker, leaves the thread open. Deferral is for **bot** threads only. Never defer a marker-less maintainer thread. Reply without a marker and leave it for the maintainer. If a maintainer marked the finding required, fix it instead.
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.

#### Resolution is the harness's responsibility — never call `resolveReviewThread`

**Do not resolve review threads yourself.** You have a shell, but resolution is not yours to perform: you must not call the `resolveReviewThread` (or `unresolveReviewThread`) GraphQL mutation under any circumstance. Thread resolution is done **only** by the harness (`dev-lead-fix-reviews.sh`), whose deterministic guards are the authoritative merge gate (`required_review_thread_resolution`). Your contract is: **reply with the addressed-marker on the threads you genuinely fixed; the harness resolves them once this pass commits your fix.** A pass that advances the PR head triggers resolution; a no-commit pass resolves nothing (the #1617 resolution gate — a pass that produced no commit resolves zero threads).
**Do not resolve review threads yourself.** You have a shell, but resolution is not yours to perform: you must not call the `resolveReviewThread` (or `unresolveReviewThread`) GraphQL mutation under any circumstance. Thread resolution is done **only** by the harness (`dev-lead-fix-reviews.sh`), whose deterministic guards are the authoritative merge gate (`required_review_thread_resolution`). Your contract is: **reply with the addressed-marker on the threads you genuinely fixed; the harness resolves them once this pass commits your fix.** A pass that advances the PR head triggers resolution; a no-commit pass resolves no *addressed* threads (the #1617 resolution gate — a pass that produced no commit resolves zero addressed/outdated threads). The one exception is a bot thread carrying a verified deferral marker (see above), which the harness resolves regardless.

Your reply and its marker are the *only* lever you have on resolution — which is why the reply above is mandatory. The harness resolves by exactly this scope (outdated status never overrides marker ownership for human threads):

- **Bot threads** (`comments.nodes[0].author.__typename` is `"Bot"` — the GitHub GraphQL API sets this for all bot accounts; note that GraphQL omits the `[bot]` suffix from `comments.nodes[0].author.login` for bots, so the login field alone is not a reliable bot indicator): the harness resolves every bot thread you addressed with an our-account `<!-- dev-lead:addressed -->` reply **and** every thread with `isOutdated: true`, **regardless of which reviewer triggered this run**. So stamp the addressed-marker on your reply whenever you genuinely fixed a bot thread — a thread you addressed must not be left open just because a different bot's comment triggered the run.
- **Bot threads** (`comments.nodes[0].author.__typename` is `"Bot"` — the GitHub GraphQL API sets this for all bot accounts; note that GraphQL omits the `[bot]` suffix from `comments.nodes[0].author.login` for bots, so the login field alone is not a reliable bot indicator): the harness resolves every bot thread you addressed with an our-account `<!-- dev-lead:addressed -->` reply, every bot thread you deferred with a verified `<!-- dev-lead:deferred ref=#<n> -->` reply (see above — this one also on no-commit passes), **and** every thread with `isOutdated: true`, **regardless of which reviewer triggered this run**. So stamp the addressed-marker on your reply whenever you genuinely fixed a bot thread — a thread you addressed must not be left open just because a different bot's comment triggered the run.
- **Human threads** (`comments.nodes[0].author.__typename` is `"User"`): **a maintainer's review thread is never resolved** — not by you, not by the harness — unless its originating comment carries one of our automation markers. You run as the owner account `don-petry` — the *same* account a human maintainer uses — so `comments.nodes[0].author.login` (even when it matches `${TRIGGERING_REVIEWER}`) **cannot** tell your own thread apart from the maintainer's. The harness discriminates by the **automation marker** in the thread's originating comment (`comments.nodes[0].body`):
- If the originating comment carries one of **our** markers — `<!-- pr-review-agent … -->`, `<!-- persona:… -->`, `<!-- dev-lead … -->`, `<!-- dependency-advisory -->` — the thread is ours and the harness may resolve it once addressed.
- If it carries **no** marker, it is a **maintainer finding**: post your fix reply, **but the thread stays open** for the maintainer to resolve. Resolving it would clear the maintainer's own review gate — exactly the PR #1413 defect this rule closes (#1415). A marker that cannot be determined is treated as a maintainer finding and left open (fail closed).
Expand Down Expand Up @@ -165,7 +181,7 @@ For **every other** comment (bot or human alike — a bot conflict report or tri
| Disposition | Use when | Required evidence (harness-verified) |
|---|---|---|
| `fixed` | you changed code to address the finding | `sha=` the **full 40-char** SHA of the commit **whose diff fixes the finding**. Cite the commit you made in this pass (`git rev-parse HEAD` *after* committing the fix locally), or the earlier commit on this branch that already fixed it. Never cite the commit that introduced the problem. The harness checks the commit is on the PR's pushed head and not on the base branch, has a non-empty diff, and is dated at or after the finding. Its diff must also **remove** (a `-` line) a token the finding names (a backticked span, quoted identifier or `--flag`) and must not re-add it. A finding with no such token (generic spans like `local`, `true`, numbers and paths do not count) fails closed unless the cited sha is a commit produced by this pass (`tokenless-not-this-pass`) (#2004) |
| `out-of-scope` | the finding is real but belongs elsewhere | `ref=#<n>` a tracking issue that **exists**; open one first if needed |
| `out-of-scope` | the finding is real but belongs elsewhere | `ref=#<n>` a tracking issue that **exists**; open one first if needed (reuse the single `dev-lead: deferred review findings` issue) |
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
| `invalid` | the finding is wrong / a false positive | a reply body with concrete reasoning (non-empty beyond the marker) |
| `answered` | the comment asked a question you answer in the reply | a reply body that actually answers it |
| `informational` | a notice with no action needed (e.g. a trial-ended notice), and **no** finding-bearing section anywhere in the comment | a reply body noting the acknowledgement; route any follow-up to **one** tracking issue per repo (see below) |
Expand All @@ -174,7 +190,7 @@ For **every other** comment (bot or human alike — a bot conflict report or tri

**AC5 — human maintainer comments.** A comment whose `author.__typename` is `"User"` (a person, using an account like a human maintainer) is auto-resolved by the harness **only** on a verified `fixed`. For any other disposition you still post your reply, but the comment stays open for the human to resolve — you can never dismiss a person's finding by arguing it away. Bot comments (`__typename` = `"Bot"`) resolve on any verified disposition.

**AC6 — one tracking issue per repo.** When you defer findings (`out-of-scope`) or route `informational` follow-ups, funnel them into a **single** tracking issue per repository rather than opening one per comment; reference that issue's number in `ref=`.
**AC6 — one tracking issue per repo.** When you defer findings (`out-of-scope`) or route `informational` follow-ups, funnel them into a **single** tracking issue per repository rather than opening one per comment; reference that issue's number in `ref=`. It is the same `dev-lead: deferred review findings` issue that review-thread deferrals use (see *Deferring a valid bot finding*).

**Never minimize a comment yourself** (no `minimizeComment` mutation). As with review threads, resolution is the harness's job: it verifies your disposition reply and minimizes the original comment RESOLVED. Your reply + its marker are your only lever.

Expand Down Expand Up @@ -209,6 +225,7 @@ Read every changed line as if you are the reviewer seeing the response:
- For every thread you fix, post a reply naming the specific change — never reply-less. End the reply with the addressed-marker `<!-- dev-lead:addressed -->` **only** on bot threads and marker-carrying human threads; on a marker-less human (maintainer) thread, reply **without** the marker so it stays open for the maintainer
- **Never resolve a thread yourself**: do not call the `resolveReviewThread` (or `unresolveReviewThread`) mutation. Resolution is the harness's job — it resolves every bot thread you addressed (via your our-account addressed-marker reply) and every outdated bot thread, and it leaves every marker-less (maintainer) thread open (#1415). Your only lever is the addressed-marker on your reply
- For a thread you are skipping due to ambiguity, post a skip note **without** the addressed-marker and leave it in your output
- For a bot thread whose finding is valid but out of scope, defer it with a `<!-- dev-lead:deferred ref=#<n> -->` reply pointing at the repo's single deferred-findings tracking issue, which must link the thread (see "Deferring a valid bot finding")
- Do not make changes beyond what the review threads request, except that fixing Tier-1 blockers (failure/timed_out/cancelled/action_required/stale/startup_failure CI checks and CHANGES_REQUESTED reviews) is always in-scope
- Never revert or undo the PR's own committed changes to satisfy a neutral `COMMENTED`/overview review — that produces a net-zero diff that silently cancels the fix (#1340)
- If a review thread is ambiguous, apply the most conservative interpretation
Expand All @@ -225,6 +242,7 @@ Addressed N threads:
- Thread <id>: <brief description of fix> — maintainer thread [reply without marker, left open]
- Thread <id>: outdated — replied (harness resolves) # bot thread
- Thread <id>: outdated — maintainer thread — replied [reply without marker, left open]
- Thread <id>: deferred — <reason> [replied + deferred ref=#<n>] # bot thread
- Thread <id>: skipped — <reason> [reply without marker]
Test verification: <pass/fail — paste output if relevant>
Files changed: <list of files>
Expand Down
Loading
Loading