fix(review): allow fresh verdicts after thread responses

[piekstra]

Summary

Allow a normal cr review to produce a fresh verdict after a newer, empty COMMENTED event from thread replies. GitHub records these reply-side events separately from the earlier approval; they are discussion activity, not a new approval.

Both live-review fast paths now use the latest approvals, change requests or comments from the posting identity. Tied timestamps keep all candidate verdicts: opaque review IDs never establish chronology, and any tied non-approval prevents the approval fast path. An unmarked comment or stale-revision comment starts a fresh review. A legitimate COMMENTED verdict with the current head/base submit-review marker remains complete and idempotent unless a tied empty event makes completion ambiguous.

This changes review recovery, not approval policy. It does not stamp a reply as an approval, ignore newer discussion at a downstream merge gate, or suppress GitHub's empty reply-side review events. A new real review is still required.

Evidence

The existing patch's marked-comment control reproduced a failure: TestEvaluateMarkedCommentedVerdictRemainsComplete returned continue/fresh instead of a completed review. After the correction it passes, alongside the newer-empty-comment reproduction and stale-base control. Review feedback also reproduced false approval for tied numeric IDs 99 and 100 in both input orders. Those controls now pass for change requests and empty comments, with additional coverage for a marked COMMENTED verdict tied with an empty event.

Final recovery controls also verify that a newer marked COMMENTED verdict ends recovery after an earlier empty event, and that unanimous tied approvals remain idempotent. These are test-only additions after the live run below; runtime gate code is unchanged.

go test ./internal/gateio -run '^TestEvaluateMarkedCommentedVerdictRemainsComplete$' -count=1
go test -race ./internal/gateio ./internal/app ./internal/cmd/reviewcmd
golangci-lint run
make build

Focused packages pass and lint reports 0 issues. The locally built command's help describes the COMMENTED recovery path. These fixture-backed tests prove gate selection and idempotency, not a live GitHub reply write.

make check reaches an unrelated installed-Pi integration failure: the locally installed Pi rejects --no-builtin-tools and --no-approve. That local live-adapter check remains incomplete; no test was disabled or gate weakened.

Live GitHub verification on head 99d5211e66190fe39b74bca5a441f28248d902ae, base 8949797d3729b90bcf93d7b2a407bf878f35ef93 used the binary built from this head:

• The real finding response posted and resolved the thread (response run b0e0ea10-1aba-4f3f-acbf-f96616eacc17). GitHub recorded an empty COMMENTED event on this head.
• A normal review, without --rerun, selected continue/fresh and posted an exact-head/base approval (run 03fbdf43-3336-432f-8321-3bf5381453ba).
• A subsequent normal review exits as already approved without another review post.

This proves real reply-to-review recovery and approval idempotency on this head. The tied-timestamp and marked-COMMENTED edge cases above remain fixture-backed tests, not manufactured GitHub verdict events. CI's tests, builds, static smoke and lint pass; the local installed-Pi adapter gap remains as noted above.

[monit-reviewer]

Automated PR Review

Reviewed commit: 23fb501a0267
Profile: codex-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	1
policies:conventions	0
structure:repo-health	0
security:code-auditor	0

go:implementation-tests (1 finding)

Major - internal/gateio/gateio.go:1025

Lexicographic ReviewID comparison does not establish review chronology and replaces the existing conservative changes-requested tie handling. GitHub IDs are decimal strings: with equal SubmittedAt values, approval ID "99" beats newer changes-requested or COMMENTED ID "100", causing "review already approved" to skip the required fresh review. Replacing only the IDs in TestEvaluateSameTimestampChangesRequestedPreventsActiveApprovalExit with these values reproduces failure in both input orders; its current alphabetic IDs accidentally mask the regression. Preserve conservative verdict precedence for ambiguous timestamps, or expose trustworthy ordering at the provider boundary, and add tied numeric-ID coverage for both changes requests and empty comments.

Reviewer Coverage

• go:implementation-tests — complete (broad); skipped: none; constraints: Review limited to the assigned Go implementation and behavioral tests; no live GitHub verification performed. With CGO_ENABLED=0, gateio tests passed. Reviewcmd tests were blocked by sandbox-denied writes to the user cache; the default CGO build also failed on a cache path containing spaces.
• policies:conventions — complete (broad); skipped: none; constraints: Review limited to the assigned diff and visible repository guidance; optional sibling copies of shared standards and automation were unavailable. Tests and live GitHub recovery were not independently run; supplied evidence explicitly identifies the remaining live-verification gap.
• structure:repo-health — complete (broad); skipped: none; constraints: Gateio tests passed with CGO_ENABLED=0. The default build failed because the environment's clang cache path contains spaces. Review limited to the assigned diff and relevant local ownership, lifecycle, and provider contracts; no live GitHub verification performed.
• security:code-auditor — complete (broad); inspected 2 assigned files (3 inspected across reviewers): internal/gateio/gateio.go, internal/gateio/gateio_test.go; skipped: none; constraints: Review was limited to the two assigned gateio files; the canonical Monit security reference was inaccessible via web.

Inspected files (3)

• internal/cmd/reviewcmd/reviewcmd.go
• internal/gateio/gateio.go
• internal/gateio/gateio_test.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 3m 50s | gpt-6.1-sol, gpt-6-luna | cr 0.10.319

Field	Value
Model	gpt-6.1-sol, gpt-6-luna
Reviewers	go:implementation-tests, policies:conventions, structure:repo-health, security:code-auditor
Engine	codex_cli · gpt-6.1-sol, gpt-6-luna
Reviewed by	cr · monit-reviewer
Duration	3m 50s wall · 7m 03s compute
Cost	unavailable
Tokens	1.3M in / 11.9k out

Per-workstream usage

• orchestrator-selection — gpt-6.1-sol
  • In: 77.1k
  • Out: 673
  • Cache read: 60.0k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 57s
• go:implementation-tests — gpt-6.1-sol
  • In: 316.5k
  • Out: 1.5k
  • Cache read: 279.7k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 1m 33s
• policies:conventions — gpt-6.1-sol
  • In: 133.7k
  • Out: 474
  • Cache read: 100.5k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 35s
• structure:repo-health — gpt-6.1-sol
  • In: 338.7k
  • Out: 1.1k
  • Cache read: 306.3k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 1m 20s
• security:code-auditor — gpt-6-luna
  • In: 327.1k
  • Out: 7.3k
  • Cache read: 265.5k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 2m 29s
• orchestrator-rollup — gpt-6.1-sol
  • In: 108.3k
  • Out: 759
  • Cache read: 88.8k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 8s

[monit-reviewer]

Automated PR Review

Reviewed commit: 99d5211e6619
Profile: codex-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	0
policies:conventions	0
structure:repo-health	0
security:code-auditor	0

Reviewer Coverage

• go:implementation-tests — complete (constrained); skipped: none; constraints: Read-only sandbox prevented running Go tests; test adequacy was assessed by inspecting assertions and execution paths. No live GitHub verification performed. Review limited to the assigned Go implementation and behavioral test coverage.
• policies:conventions — complete (constrained); skipped: none; constraints: Read-only review; tests and live GitHub recovery were not independently run. Supplied evidence explicitly identifies the remaining live-verification gap. Review limited to the assigned diff and visible repository guidance; optional sibling copies of shared standards and automation were unavailable.
• structure:repo-health — complete (constrained); skipped: none; constraints: Static review of the assigned diff and relevant local lifecycle and ownership guidance. Tests were not executed in the read-only sandbox; live GitHub recovery remains unverified.
• security:code-auditor — complete (constrained); inspected 2 assigned files (3 inspected across reviewers): internal/gateio/gateio.go, internal/gateio/gateio_test.go; skipped: none; constraints: Canonical Monit security reference was inaccessible; review was limited to the assigned gateio files.

Inspected files (3)

• internal/cmd/reviewcmd/reviewcmd.go
• internal/gateio/gateio.go
• internal/gateio/gateio_test.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 3m 10s | gpt-6.1-sol, gpt-6-luna | cr dev

Field	Value
Model	gpt-6.1-sol, gpt-6-luna
Reviewers	go:implementation-tests, policies:conventions, structure:repo-health, security:code-auditor
Engine	codex_cli · gpt-6.1-sol, gpt-6-luna
Reviewed by	cr · monit-reviewer
Duration	3m 10s wall · 4m 08s compute
Cost	unavailable
Tokens	2.1M in / 17.2k out

Per-workstream usage

• go:implementation-tests — gpt-6.1-sol
  • In: 507.1k
  • Out: 2.0k
  • Cache read: 445.6k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 29s
• policies:conventions — gpt-6.1-sol
  • In: 310.5k
  • Out: 773
  • Cache read: 256.5k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 24s
• structure:repo-health — gpt-6.1-sol
  • In: 510.7k
  • Out: 1.5k
  • Cache read: 456.8k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 27s
• security:code-auditor — gpt-6-luna
  • In: 588.0k
  • Out: 12.1k
  • Cache read: 437.8k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 2m 40s
• orchestrator-rollup — gpt-6.1-sol
  • In: 141.1k
  • Out: 823
  • Cache read: 119.9k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 6s

[monit-reviewer]

Automated PR Review

Reviewed commit: 5ac31f474bce
Profile: codex-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	0
policies:conventions	0
structure:repo-health	0
security:code-auditor	0

Reviewer Coverage

• go:implementation-tests — complete (constrained); skipped: none; constraints: Read-only sandbox prevented running Go tests. Assertions and execution paths were inspected; reported live verification was not independently repeated. Review limited to the assigned Go implementation and behavioral test coverage.
• policies:conventions — complete (constrained); skipped: none; constraints: Read-only review; reported tests and live GitHub verification were not independently repeated. Review limited to the assigned diff and visible repository guidance; optional sibling copies of shared standards and automation were unavailable.
• structure:repo-health — complete (constrained); skipped: none; constraints: Static review of the assigned diff and relevant local gate and lifecycle guidance. Tests were not executed in the read-only sandbox; reported live verification was not independently reproduced.
• security:code-auditor — complete (constrained); inspected 2 assigned files (3 inspected across reviewers): internal/gateio/gateio.go, internal/gateio/gateio_test.go; skipped: none; constraints: Review limited to the assigned gateio files; live GitHub behavior was not reverified in this read-only worktree.

Inspected files (3)

• internal/cmd/reviewcmd/reviewcmd.go
• internal/gateio/gateio.go
• internal/gateio/gateio_test.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 1m 15s | gpt-6.1-sol, gpt-6-luna | cr dev

Field	Value
Model	gpt-6.1-sol, gpt-6-luna
Reviewers	go:implementation-tests, policies:conventions, structure:repo-health, security:code-auditor
Engine	codex_cli · gpt-6.1-sol, gpt-6-luna
Reviewed by	cr · monit-reviewer
Duration	1m 15s wall · 2m 14s compute
Cost	unavailable
Tokens	3.0M in / 19.6k out

Per-workstream usage

• go:implementation-tests — gpt-6.1-sol
  • In: 761.2k
  • Out: 2.4k
  • Cache read: 680.6k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 27s
• policies:conventions — gpt-6.1-sol
  • In: 540.6k
  • Out: 1.1k
  • Cache read: 469.6k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 27s
• structure:repo-health — gpt-6.1-sol
  • In: 740.2k
  • Out: 1.9k
  • Cache read: 667.5k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 30s
• security:code-auditor — gpt-6-luna
  • In: 806.8k
  • Out: 13.2k
  • Cache read: 640.0k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 41s
• orchestrator-rollup — gpt-6.1-sol
  • In: 175.5k
  • Out: 885
  • Cache read: 152.6k
  • Cache create: unavailable
  • Cost: unavailable
  • Duration: 6s

[piekstra]

Post-merge verification: v0.10.322 points to merge commit 015bba069b03c5117a54dd7236a65f3b73612730 and includes both #643 and #644.

• Main CI passed, including both test jobs.
• Release passed all five jobs: GoReleaser, Homebrew, Chocolatey, WinGet, and Linux dispatch. The real Windows archive resolver passed and both Windows jobs ran.
• Downloaded the Darwin ARM64 archive into an isolated temporary directory. SHA-256 matches the published checksum; cr --version reports 0.10.322 and the exact merge commit. Build metadata reports CGO_ENABLED=1 and vcs.modified=false. The Homebrew cask pins 0.10.322 with the same archive checksum. No shared installation was changed.
• Chocolatey accepted codereview-cli.0.10.322.nupkg; its API currently reports Submitted and IsApproved=false. Availability still awaits moderation.
• WinGet submitted microsoft/winget-pkgs #445652; its validation and merge remain pending.
• Downstream Linux repository publication is still running; the successful dispatch is not proof of completed repository publication.

Historical failed runs are unchanged. This verification uses the new merged commit and release. It does not claim a live Pi/provider integration result or resolve compatibility of older installed Pi versions.