Skip to content

feat: implement issue #2247 — Stale-disposition retry cap does not count a fix-reviews pass that dies before posting any marker (follow-up to the edited-comment deadlock fix) - #2290

Open
don-petry wants to merge 4 commits into
mainfrom
dev-lead/issue-2247-20261011-1510
Open

don-petry wants to merge 4 commits into
mainfrom
dev-lead/issue-2247-20261011-1510

Conversation

@don-petry

@don-petry don-petry commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Stale-disposition retry cap does not count a fix-reviews pass that dies before posting any marker (follow-up to the edited-comment deadlock fix)

From the issue: The stale-disposition retry cap added for #2166 (PR #2230) bounds how many fix-reviews passes the sweep dispatches for a bot comment that was edited after its disposition. It counts passes by their markers on the PR: any <!-- dev-lead-fix-reviews pr=<n> … intent=fix-reviews status=… --> comment newer than the edit.

Risk

Low — changes automation shell logic under scripts/, covered by shellcheck (--severity=warning) and the bats suite.

Test plan

Tests added/updated: tests/dev-lead/unit/test_dev_lead_retry.bats,tests/dev-lead/unit/test_fix_reviews.bats tests/dev-lead/unit/test_maintainer_comment_gate.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

This PR's Lint (shellcheck) and bats checks show pass/fail; watch subsequent dev-lead / pr-review runs for behavioral regressions.

Closes #2247

View guided diff Turn on auto-fix

@don-petry
don-petry requested a review from a team as a code owner October 11, 2026 15:45
@don-petry don-petry added the auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label) label Oct 11, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

This comment has been minimized.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to post a start marker (<!-- dev-lead-fix-reviews-start ... -->) before a fix-reviews pass runs. This ensures that passes that fail or die prematurely (before posting a terminal marker) are still counted towards the stale disposition retry cap. The retry logic is updated to count these start markers, grouping them with terminal markers by their read_at timestamp to avoid double-counting. Unit tests are added to verify the new behavior and ensure the start marker is treated as a registered automation marker. I have no feedback to provide as there are no review comments.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@codeant-ai

codeant-ai Bot commented Oct 11, 2026

Copy link
Copy Markdown

CodeAnt Nitpicks

1 code suggestion

1. If separate passes start in the same second, they reuse read_at; the sweep counts them as one attempt, allowing failures to exceed the retry cap.

Logic error · scripts/dev-lead-fix-reviews.sh:936

@codeant-ai

This comment has been minimized.

@don-petry

This comment has been minimized.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
- CodeAnt neutral overview (PR Risk: Low Risk): No actionable findings — dispositioned informational
Files changed: none
Skipped (informational): 1
```
The bot comment is a neutral overview describing the PR as safe to merge with good test coverage. Per the maintainer-comment gate requirements, I posted an `informational` disposition reply on the original comment (ID: `IC_kwDOR9SdIs8AAAABbDtF2A`) to clear the gate blocker. No code changes were needed.

@don-petry
don-petry enabled auto-merge October 11, 2026 15:56
@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@don-petry
don-petry disabled auto-merge October 11, 2026 16:10
@don-petry

This comment has been minimized.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
## Summary
**Bot:** coderabbitai[bot]
**Issues addressed:** 0 (no code findings)
**Disposition:** Rate-limit notice → posted `informational` disposition
The CodeRabbit comment is a **pure rate-limit notice** with no code review findings, no Security Architecture Review findings, and no actionable suggestions. The code review was throttled due to free OSS review limits being reached. No changes to the diff are needed.
**Reply posted:** https://github.com/petry-projects/.github-private/pull/2290#issuecomment-6110988068
The maintainer-comment gate will recognize the `disposition=informational` marker and clear this as a non-actionable operational notice, allowing the PR to proceed.

@don-petry
don-petry enabled auto-merge October 11, 2026 16:11
@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@don-petry
don-petry disabled auto-merge October 11, 2026 16:23
@don-petry

Copy link
Copy Markdown
Collaborator Author

Fixed in scripts/dev-lead-fix-reviews.sh: updated PASS_STARTED_AT to use millisecond precision (line 89) and updated regex patterns (lines 487, 935) to accept milliseconds, ensuring each pass gets a unique read_at timestamp even when starting in the same second.

@don-petry
don-petry enabled auto-merge October 11, 2026 16:33
Comment thread scripts/dev-lead-retry.sh Outdated
@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-11T17:35:26Z.

@don-petry
don-petry disabled auto-merge October 11, 2026 16:35
@don-petry

This comment has been minimized.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (applied)

Changes committed and pushed. Requested items addressed:

  • scripts/dev-lead-retry.sh:517 — applied

@don-petry
don-petry enabled auto-merge October 11, 2026 16:47
@don-petry

Copy link
Copy Markdown
Collaborator Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@don-petry
don-petry disabled auto-merge October 11, 2026 18:10
@don-petry

Copy link
Copy Markdown
Collaborator Author

CodeAnt nitpick (same-second passes share a read_at, so the sweep counts them as one attempt): addressed on this branch. afbbd5b made PASS_STARTED_AT millisecond-precision so passes get distinct markers, and 3a70996 relaxed the stale-disposition sweep read_at capture so it accepts the fractional-second value.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Codex usage-limit notice: no review was produced and it contains no findings, so there is nothing to change.

@don-petry

Copy link
Copy Markdown
Collaborator Author

SonarCloud Quality Gate passed, with 0 new issues and 0 hotspots. There are no findings to act on.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (no-changes)

Agent reasoning
Addressed 0 threads. There were no open review threads on this PR, so I made no code changes and no commits.
- **Reviews:** nothing is in `CHANGES_REQUESTED`. The Gemini, Graphite and `don-petry` reviews are all `COMMENTED`, which is neutral.
- **CI:** three checks show `cancelled`: `dev-lead / ci-relay`, `dev-lead / dispatch` and `dev-lead / resume`. They are workflow-run cancellations (usually a newer run superseding an older one), not code failures. There is nothing in the repo to fix for them, and a re-run is the only remedy. Every other check passed or was skipped.
- **Issue comments:** I posted three dispositions on comments that had none:
  - **CodeAnt nitpick (same-second `read_at`):** `fixed`, citing `3a709963538384ebac3d4cfc0e8390867a493c4f`. Commit afbbd5b0 made `PASS_STARTED_AT` millisecond-precision, and 3a709963 relaxed the sweep's `read_at` capture to accept fractional seconds. I cited 3a709963 because it is the one whose diff removes a `read_at` line, which the harness checks.
  - **Codex usage-limit notice:** `informational`.
  - **SonarCloud Quality Gate passed:** `informational`.
- **Already dispositioned:** the CodeRabbit and CodeAnt risk comments already had dispositions. The auto-rebase-conflict and dispatch-guard comments are our own automation comments, so I left them alone.
Test verification: not run, since nothing changed.
Files changed: none.

…11-1510

Updates this branch onto the renamed main and rewrites its petry-projects references to combsmith (GitHub Actions does not redirect uses: after an org rename). Rename-aware merge: base and branch were rewritten first, so rename-only differences do not conflict.
@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@sonarqubecloud

Copy link
Copy Markdown

@donpetry-bot

donpetry-bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@don-petry
don-petry disabled auto-merge October 14, 2026 15:17
@don-petry

Copy link
Copy Markdown
Collaborator Author

Re-read the current CodeRabbit summary body. It holds only the rate-limit notice ("Review limit reached", next review available in about 59 minutes), the Autofix opt-in checkbox and the share/tips footer. It has no architecture-review section, no actionable comments, no outside-diff or nitpick findings, so nothing needs a code change. Acknowledged as informational.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (no-changes)

Agent reasoning
Addressed 0 threads. There were no open review threads, and I made no code changes or commits.
- **Stale disposition `IC_kwDOR9SdIs8AAAABbDq-jg` (coderabbitai):** I re-read the current body and posted a fresh `informational` disposition. The comment holds only a rate-limit notice, the Autofix checkbox and the footer. It has no architecture-review section, actionable comments, outside-diff items or nitpicks.
- **Tier 1 blockers:** none. No CI check failed, and no review has `CHANGES_REQUESTED`. `bats` and `unit` were still `in_progress` at the time of the snapshot.
- **Test verification:** I didn't run the suite, since nothing changed.
- **Files changed:** none.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stale-disposition retry cap does not count a fix-reviews pass that dies before posting any marker (follow-up to the edited-comment deadlock fix)

2 participants