You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Follow-ups from #2004 / PR #2051 and the markets ingress pilot (verifier region check, phantom Fixed replies, dispatch cancellation, dry-run slot) #2068
Follow-ups left over from #2004 / PR #2051 (content-verified fixed dispositions) and the ADR-0007 markets pilot. None blocks those PRs; each was deliberately deferred. Unlabelled on purpose: triage before applying dev-lead.
1. Anchored findings are verified against the whole commit diff (from cubic on #2051)
cdv_diff_token_verdict matches finding tokens across the ENTIRE cited commit diff. For a finding anchored to a file/line (a review-thread finding), an unrelated file that removes the same identifier can verify the disposition while the anchored code is unchanged. Intersect the token match with the anchored file (and, where cheap, the region) when the finding carries an anchor; keep the looser whole-diff check for un-anchored issue comments. Test: an unrelated file removing the token does not verify an anchored finding.
2. dev-lead replied "Fixed in " for commits that were never pushed, twice, on #2051
Cited 6f60c6f (does not exist in the repo) and rdc_live_comment_body (absent from the head) in thread replies. #2013's retract_unlanded_claims guard did not catch either. Find out why the guard missed them (the claim was a thread reply for a commit made locally and lost to a merge/rebase?) and close the gap.
3. A dev-lead dispatch is cancelled by unrelated bot-comment retries
On #2051 five manual dev-lead-reviews-retry dispatches in one hour were cancelled by newer runs triggered by bot-comment retries on the same PR. Document the concurrency behavior, or key the concurrency group so a human-requested fix-reviews is not cancelled by a bot-comment retry.
4. markets pilot: dry-run versus live pr-review sweeps cancel each other
From ADR-0010 / package §4b: a workflow_dispatch with no pr_url shares the single pr-review-enumerate slot with cancel-in-progress: true, so a dry-run can cancel a live sweep. Include github.event.inputs.dry_run in the enumerate group (an event-payload read, within ADR-0010's bounds) so they stop colliding.
Acceptance criteria
Each item is separately closable. Filing one dev-lead issue per item when triaged is fine.
Thanks for writing up these follow-ups so clearly from #2004 / PR #2051 and the markets pilot. Items 1 and 2 look like correctness gaps in the verification and claim-retraction guards, and items 3 and 4 look like concurrency-group improvements. For item 2, which is the only one that needs investigation, can you share the thread reply or workflow run where the unpushed 6f60c6f claim was posted? We'd like to see whether retract_unlanded_claims ran at all and what it checked.
Pilot observation to watch for AC7 (from markets#513 review, cubic): the deleted markets pr-auto-review.yml stub documented (#1126) that check_suite/workflow_run events must NOT be grouped across PRs, because pull_requests[0] is indeterminate when PRs share a commit and one PR's event cancels another's. The ratified collapse (package §4b, ADR-0010) instead keys those events on the head SHA (pr-auto-review-ready-check-sha-<head_sha>, cancel-in-progress: true). That is plausibly equivalent (events for one commit carry the same PR list) but is a documented hazard replaced by design, not a proven-safe one. During the AC7 observed run, check that a commit shared by two PRs still yields a readiness check for each PR.
Correction: during the markets pilot review I told reviewers that the ci-failure-analyst loop-guard weakness and no-PR check-run handling were tracked here. They were not in this issue's list. They are now tracked in #2175. The pr-review/stable pin is already tracked in combsmith/.github#1125/#1106, and the ci-failure-analyst bare-SHA pin is #2176. The dry-run versus live pr-review sweep slot (item 4 above) and the same-SHA pr-auto-review note stand as written; the pr-auto-review group has since changed (see #2171), so that hazard note is partly superseded by the per-event slot now deployed.
Activity
github-actions commentedon Oct 7, 2026
Thanks for writing up these follow-ups so clearly from #2004 / PR #2051 and the markets pilot. Items 1 and 2 look like correctness gaps in the verification and claim-retraction guards, and items 3 and 4 look like concurrency-group improvements. For item 2, which is the only one that needs investigation, can you share the thread reply or workflow run where the unpushed
6f60c6fclaim was posted? We'd like to see whetherretract_unlanded_claimsran at all and what it checked.don-petry commentedon Oct 9, 2026
Pilot observation to watch for AC7 (from markets#513 review, cubic): the deleted markets
pr-auto-review.ymlstub documented (#1126) thatcheck_suite/workflow_runevents must NOT be grouped across PRs, becausepull_requests[0]is indeterminate when PRs share a commit and one PR's event cancels another's. The ratified collapse (package §4b, ADR-0010) instead keys those events on the head SHA (pr-auto-review-ready-check-sha-<head_sha>,cancel-in-progress: true). That is plausibly equivalent (events for one commit carry the same PR list) but is a documented hazard replaced by design, not a proven-safe one. During the AC7 observed run, check that a commit shared by two PRs still yields a readiness check for each PR.don-petry commentedon Oct 10, 2026
Correction: during the markets pilot review I told reviewers that the
ci-failure-analystloop-guard weakness and no-PR check-run handling were tracked here. They were not in this issue's list. They are now tracked in #2175. Thepr-review/stablepin is already tracked in combsmith/.github#1125/#1106, and theci-failure-analystbare-SHA pin is #2176. The dry-run versus livepr-reviewsweep slot (item 4 above) and the same-SHApr-auto-reviewnote stand as written; thepr-auto-reviewgroup has since changed (see #2171), so that hazard note is partly superseded by the per-event slot now deployed.