[allenhutchison]

Requested by Allen · project thread

Human overview

In first-line mode, address-review now answers every unresolved final-reviewer thread around each push:

• A thread it fixes gets Fixed in <sha>.
• A thread it does not fix gets a reply starting with the dispute prefix, Declined:, or Deferred: with an issue reference.
• In this mode it never asks the final reviewer for a re-review.

This matches the first-line reviewer's new rule: it holds the hand-off until every final-reviewer finding is fixed and answered. This is phase G3 of docs/design/vr-greptile-followthrough.md in Vycari/vycari (Refs Vycari/vycari#187).

AI reviewer

What changed

• plugins/repo-ops/skills/address-review/SKILL.md, First-line mode, new subsection "Around every push: answer the final reviewer's threads" (before Stage 2):
  • One reply on each unresolved inline thread opened by a review.bots login: Fixed in <sha>, review.firstLine.disputePrefix (default Disputed:), Declined:, or Deferred: with #N / owner/repo#N / /issues/N. The issue is filed first.
  • Order:
    1. Disputed/Declined/Deferred replies are posted any time.
    2. The agent commits and runs pre-flight, then pushes.
    3. Straight after the push, inside the first-line reviewer's review.pushCooldownMinutes window, it posts each Fixed in <sha>. The sha is checked with git merge-base --is-ancestor against the pushed head. If the push fails, no Fixed in is posted.
    4. The filter is rerun and must print [].
  • A GraphQL thread read that the filter consumes. It paginates reviewThreads (--paginate, $endCursor), joins the pages, and stops on a failed read. For each thread it takes the opener (the id to reply to) and the latest 100 comments.
  • Two fenced jq blocks. The first is a filter listing threads that still owe an answer: an acceptable answer, from a non-bot, after the final reviewer's latest comment on the thread. The second is a predicate for an acceptable reply. Bot logins and the dispute prefix are inputs, not literals.
  • An explicit ban on posting review.rereviewCommand or any @-mention of the final reviewer.
• The stale-score @greptileai please re-review instruction is now gated "not in first-line mode". Non-first-line text is otherwise unchanged.
• plugins/core/references/config-schema.md: one sentence, synced into the six vendored copies with ./scripts/sync-references.sh.
• scripts/test-address-review-first-line.sh (new, 23 checks), wired into .github/workflows/validate.yml. It extracts both jq blocks from SKILL.md and runs them on fixtures, including long threads whose opener is outside the latest-100 window. It also checks the order, pagination and failed-read lines.

Declined: VR-5f6f903c. An author-only answer rule would hold threads answered by a maintainer driving someone else's PR. ws prs blockOnOpenThreads also counts any non-bot reply.

Limits. The tests check the filters and key phrases. They do not check an agent actually running the skill.

Verification (actual results)

• ./scripts/test-address-review-first-line.sh: 23 passed, 0 failed (a mutation of the // -1 fallback fails it)
• ./scripts/test-repo-ops-hooks.sh, test-wait-tools.sh, test-coverage.sh, test-profile.sh and the rest of scripts/test-*.sh: pass
• ./scripts/sync-references.sh --check: all vendored references in sync
• python3 scripts/check-links.py: exit 0

Checklist (repo-specific)

• Plugin manifests validate against the marketplace and stay within the closed Agent Plugins schema: N/A, no plugin.json or marketplace.json changed
• Every skill has well-formed frontmatter: only the body of address-review/SKILL.md changed
• ./scripts/test-coverage.sh and ./scripts/test-profile.sh pass locally
• ./scripts/sync-references.sh --check passes
• python3 scripts/check-links.py passes

🤖 Generated with Claude Code

https://claude.ai/code/session_01QNDeh9EQwaMjc7xZXRRees

Confidence Score: 5/5

The PR appears safe to merge; no new actionable issues were found.

Summary

address-review now requires answers on unresolved final-reviewer threads in first-line mode.

• First-line review answers every unresolved final-review thread around each push.
• First-line mode leaves final-review requests to its reviewer.

_{Reviews (2) · Last reviewed commit: "address-review: gate the Phase 6 stale-s..." · Reviewed by Greptile}

[vycari-review]

P2 · VR-829f0b6c · A Fixed-in reply can name a sha that is not on the PR and still clear the gate

The final-gate filter accepts any hex sha after 'Fixed in', so a reply naming a commit that never reached the PR still clears the gate. The check that the commit is on the pushed head exists only as an instruction.

_{Reply Fixed in <sha> or Disputed: with a reason.}

Details for agents: evidence, failure scenario, verification, proving test

Step 4 requires each 'Fixed in <sha>' reply to use a short sha of a commit on the pushed head, checked with git merge-base --is-ancestor, because an amend or rebase changes the sha. That check is prose only. The mechanical gate, the thread filter (SKILL.md ~line 440-455), accepts any 7-40 hex string after 'Fixed in' (test("^Fixed in [0-9a-f]{7,40}\\b")) and never looks at git or the PR head. Step 5 then says the filter 'must print []' and treats that as the gate passing.

Evidence: SKILL.md:393-399 (step 4: merge-base check is only prose; 'never from memory'). SKILL.md filter 'answer' definition (~line 445-446) uses only the regex. scripts/test-address-review-first-line.sh:63 only greps for the prose 'merge-base --is-ancestor <sha> "$NEW_HEAD"', not for an enforced check. No git call appears in the filter or reply-check blocks.

Re-review (fix-regressed): a finding VR confirmed fixed is back in the change.

Failure scenario

The agent commits a fix, amends it after pre-flight (changing the sha), pushes, and replies 'Fixed in <pre-amend sha>' on three threads. The pre-amend sha is not an ancestor of the pushed head. The filter prints [] because the reply matches the regex, so the agent reports the answer gate cleared. The final reviewer and the first-line reviewer read 'Fixed in <sha>' pointing at a commit that is not on the PR, and the thread is closed on a claim of work that the head does not contain.

How this was verified (read-only-reasoning)

The filter regex accepts any 7-40 hex sha after 'Fixed in'. The is-ancestor check exists only in step 4 prose. Reachable from: address-review first-line step 4-5.

Proving test

Fixture: a final-bot thread whose only later non-bot comment is 'Fixed in deadbee1' where deadbee1 is not an ancestor of the head. Run the thread filter from SKILL.md with a head that does not contain deadbee1; assert the thread is listed (the reply is not an acceptable answer for a sha not on head). On the PR head the filter prints [] and the test fails. The filter has no git input, so the check needs either an ancestor test in the gate or a rule the test can verify.

_{lens spec-adversary · symbol Around every push: answer the final reviewer's threads}

[allenhutchison]

Fixed in 40f5fac. Fixed in replies now go up only after a push succeeds, and they are posted straight away. The sha is taken from the pushed history and checked with git merge-base --is-ancestor <sha> "$NEW_HEAD". If the push fails, no Fixed in reply is posted. The first-line push cooldown covers the gap between the push and the replies. Disputed, Declined and Deferred replies name no commit, so they can still go up at any point. The test now asserts both lines.

---

Generated by Claude Code

[vycari-review]

Fixed: confirmed @ 244e7ec. Thread resolved.

[vycari-review]

still open @ 39d4912 (fix-regressed)

[allenhutchison]

Declined: the filter decides whether a thread has an acceptable answer. Whether the cited sha is on the head is checked before the reply is posted, in step 4 with git merge-base --is-ancestor <sha> "$NEW_HEAD". Putting git into the jq filter would make it depend on a checkout it does not have. The first-line reviewer also confirms a fix from the hunk, not from the reply text (Fixed in with no fixing hunk stays open), so a wrong sha cannot close a finding.

---

Generated by Claude Code

[vycari-review]

still open @ 6e7a475

[vycari-review]

Vycari Review

Round 5 on 6e7a475. The verdict is the Vycari Review check run, and each open finding is an inline thread. Reply on every P0 and P1 thread: Fixed in <sha>, or Disputed: with a reason. P2s are optional.

Open: P0 0 · P1 0 · P2 2 · disputed 0 · confirmed fixed 1 · withdrawn 1

Hand-off: Greptile requested on 6e7a475.

Greptile follow-through

• P2 · GR-4429702215 · plugins/repo-ops/skills/address-review/SKILL.md:419 · answered on the thread · still-open: Greptile reported that comments(last: 100) misses an older acceptable answer that more than 100 later human comments push out of range. At this head the read still fetches only the latest 100 comments plus the opener, with no fallback fetch of older comments.

No open P0 or P1 findings this round.

Outside the change since 39d4912

• P2 · VR-85a7427d · A rejected push still lets 'Fixed in <sha>' replies name commits that are not on the PR · plugins/repo-ops/skills/address-review/SKILL.md lines 770–773 (no thread)

P2s, judge's notes and closed findings

Open P2 findings

• P2 · VR-5f6f903c · Any non-bot account's reply satisfies the gate, so a third party's 'Declined:' or 'Fixed in' silences an owed answer · plugins/repo-ops/skills/address-review/SKILL.md lines 401–405
• P2 · VR-829f0b6c · A Fixed-in reply can name a sha that is not on the PR and still clear the gate · plugins/repo-ops/skills/address-review/SKILL.md lines 393–399

Judge's notes

No P0 or P1 issues remain. The Phase 6 re-review gate is fixed, and the prior P1s about the thread read and the filter stay fixed. Four P2s are still open: a rejected push can still let a 'Fixed in' reply cite a commit not on the PR; the filter does not check the sha; any non-bot account's reply counts as an answer; and Greptile's 100-comment window issue.

Confirmed fixed

• VR-ce983ad9 · First-line mode still tells the agent to request the final re-review when a score is stale

Withdrawn

• VR-f3a0015c · The dispute holds. Step 4 posts the Fixed-in replies right after the push. The first-line reviewer waits pushCooldownMinutes (default 5) and merges pushes made inside that wait into one round, so the replies land seconds into a five-minute window. The replies cannot go up before the push, because t…

[vycari-review]

P2 · VR-5f6f903c · Any non-bot account's reply satisfies the gate, so a third party's 'Declined:' or 'Fixed in' silences an owed answer

Any account that is not a bot can clear a final-reviewer thread from the owed list by posting a reply that starts with an accepted prefix. A passer-by's reply can therefore stand in for the PR author's own answer.

_{Reply Fixed in <sha> or Disputed: with a reason.}

Details for agents: evidence, failure scenario, verification, proving test

The filter's 'answer' test accepts a reply from anyone outside $bots (the code selects 'from anyone outside $bots'), not from the PR author or a maintainer. A drive-by commenter's text starting 'Declined:' or 'Fixed in <7+ hex>' on a final-reviewer thread therefore removes that thread from the list the agent must answer. The rule says the agent must give each such thread one of its own replies, and the step-5 gate is meant to catch exactly the threads still owing one.

Evidence: SKILL.md filter: 'select(by($bots) | not)' then 'answer' test; prose: 'from anyone outside $bots'. The test script case 9 and case 2 use 'alice' as the human answerer, showing the intent is any non-bot. Did not find an author-identity check in the diff or in the section. The first-line reviewer's hold check is not in this diff, so whether it independently requires an author reply is unverified.

Failure scenario

A P1 final-reviewer thread on src/auth.go is unanswered by the PR author. A third-party user replies 'Declined: intentional'. The agent's filter excludes the thread, prints [] and pushes without answering. The first-line reviewer and the final reviewer see a P1 thread with no author reply, while the agent reported the answer gate cleared.

How this was verified (read-only-reasoning)

I read the filter in the diff. It excludes only $bots authors and does not check the PR author. I did not open any code that shows how the reviewer's hold check works. Reachable from: address-review first-line mode, thread filter.

Proving test

Thread fixture: final-bot opens a thread, a non-bot user 'outsider' replies 'Declined: intentional', no author reply. Run the thread filter with bots=[final-bot]: the expected output lists the thread's databaseId because the PR author has not answered; the current filter returns [] and the assertion fails on the head.

_{lens spec-adversary}

[allenhutchison]

Declined: the agent replies under whichever maintainer's login it runs as, and that is often not the PR author. Examples are foreman or auto-dev PRs opened by an App, or a teammate driving someone else's PR. An author-only rule would hold those threads forever. The merge-side thread check (ws prs blockOnOpenThreads) also counts any non-bot reply as answered, so this keeps the two gates consistent. A drive-by Declined: from an outsider is visible on the thread for the maintainer, who merges.

---

Generated by Claude Code

[vycari-review]

still open @ 62a0169

[vycari-review]

still open @ 39d4912

[vycari-review]

still open @ 6e7a475

[vycari-review]

@greptileai review

[vycari-review]

@greptileai review