Skip to content

feat: implement issue #508 — Compliance: stub-surface-drift-pr-auto-review.yml-concurrency - #521

Merged
don-petry merged 29 commits into
mainfrom
dev-lead/issue-508-20260925-1750
Oct 11, 2026
Merged

don-petry merged 29 commits into
mainfrom
dev-lead/issue-508-20260925-1750

Conversation

@don-petry

@don-petry don-petry commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Compliance: stub-surface-drift-pr-auto-review.yml-concurrency

From the issue: Category: ci-workflows Severity: warning Check: stub-surface-drift-pr-auto-review.yml-concurrency

Risk

Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.

Test plan

No test files were added or updated. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the existing CI (bats + lint) guards the change.

Rollback

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

Monitoring

Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.

Closes #508

Review in cubic

Summary by CodeRabbit

  • Chores
    • Runs triggered by check_suite or workflow_run events associated with a pull request are grouped by pull request number, and newer runs in the same group cancel in-progress runs. Other events use unique run groups and do not cancel in-progress runs.
  • Tests
    • Added regression checks for workflow run grouping, cancellation rules, and unique run ID fallbacks.
    • Included the regression checks in the continuous integration workflow.

@don-petry
don-petry requested a review from a team as a code owner September 25, 2026 17:54
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@gemini-code-assist

This comment has been minimized.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #521
No changes were committed, but the PR still can't be marked done: required check SonarCloud is still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-09-25T18:25:29Z

@don-petry

Copy link
Copy Markdown
Contributor Author

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required check SonarCloud is still pending. I'll re-check automatically.
Next attempt after: 2026-09-25T18:25:29Z

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 17:55
@codeant-ai

codeant-ai Bot commented Sep 25, 2026

Copy link
Copy Markdown

CodeAnt Nitpicks

2 code suggestions

1. When one commit belongs to multiple PRs, both events use pull_requests[0], so runs for other PRs share the first PR's cancellable group and can be cancelled.

Logic error · .github/workflows/pr-auto-review.yml:64-67


2. A delayed completion event for an older commit can enter the same PR group after a newer run starts and cancel that newer readiness check, leaving the current commit unprocessed.

Race condition · .github/workflows/pr-auto-review.yml:70

@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 1 file

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread .github/workflows/pr-auto-review.yml
Comment thread .github/workflows/pr-auto-review.yml
Comment thread .github/workflows/pr-auto-review.yml
@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-09-25T19:02:04Z.

@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@don-petry
don-petry disabled auto-merge September 25, 2026 18:36
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (applied)

Changes committed and pushed. Requested items addressed:

  • .github/workflows/pr-auto-review.yml:67 — applied
  • .github/workflows/pr-auto-review.yml:70 — applied
  • .github/workflows/pr-auto-review.yml:64 — applied

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 18:42
@don-petry
don-petry disabled auto-merge September 25, 2026 18:43
@don-petry

Copy link
Copy Markdown
Contributor Author

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

Agent reasoning
Issues addressed: 0
- Quality gate passed with zero new issues — no actionable findings
Files changed: None
Skipped (informational): Quality gate summary
```
No fixes required. The PR is ready for merge.

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 18:43
@donpetry-bot donpetry-bot added the needs-human-review Requires human review label Sep 25, 2026
@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-09-25T19:52:14Z.

@don-petry

don-petry commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor Author
Resolved — the `needs-human-review` hold was lifted; dev-lead has picked this item up. Click to expand the prior hold notice.

dev-lead is withholding action on this item.

It is labeled needs-human-review (flagged for human review — this label is applied by automation as well as by people, so an item can become held without anyone noticing), so dev-lead will not pick it up while that label is present. This notice is posted once so the withhold is visible rather than looking like a stalled run.

To re-enable automated pickup: remove the needs-human-review label.

@don-petry
don-petry disabled auto-merge October 11, 2026 16:44
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@don-petry

Copy link
Copy Markdown
Contributor Author

Re-read the current body of this CodeRabbit comment. No code review findings; the Security Architecture Review is Low risk with no retained concerns, and two Hardening Proposals:

  1. Validate the reusable workflow's interruption/repetition contract — that reusable lives in petry-projects/.github, outside this repo; cancellation here only applies to check_suite/workflow_run runs (default-branch context), which are re-triggered by later events, so a superseded run is safely re-evaluated.
  2. Strengthen the regression control — the guard (scripts/test-pr-auto-review-workflow.sh) now checks event-gating, pull_requests[0].number keying, the run_id fallback and cancel gating, and the fixture suite covers mutations of each (7f9a376 aligns the workflow with it). The commit-scoped group the review describes no longer exists.
    No further change.

@don-petry

Copy link
Copy Markdown
Contributor Author

Re-read the current CodeAnt comment: it is an informational risk summary (Low Risk) with no findings. Acknowledged.

@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.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-reviews (partial)

A commit was pushed, but not every requested change was applied. Per requested item:

  • scripts/test-pr-auto-review-workflow.sh:248 — not applied
  • .github/workflows/pr-auto-review.yml:69 — applied
  • scripts/test-pr-auto-review-workflow.sh:107 — not applied

The unaddressed items above still need work.

@don-petry
don-petry enabled auto-merge (squash) October 11, 2026 16:45
@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

Issue #508 asks for the concurrency surface to be re-synced verbatim from
petry-projects/.github standards/workflows/pr-auto-review.yml. Earlier review
passes added a pull_requests[1] guard to the check_suite and workflow_run
branches of the group expression, which the compliance audit's
stub_surface_drift comparison still reports as drift. Drop the two guards so
the block matches the canonical template; a multi-PR guard, if wanted, belongs
in the template.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U1bsmUpeti2XUHnhQxWHU9
@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.

@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

…text

The self-test's "check_suite branch not gated on its event name" mutation
matched the pull_requests[1] guard that the previous commit removed, so the
sed no longer applied and the case failed with "mutation did not apply".
Match the canonical line instead; the guard still rejects the mutant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U1bsmUpeti2XUHnhQxWHU9
@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.

@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry
don-petry disabled auto-merge October 11, 2026 18:01
Strengthen the concurrency control by adding !pull_requests[1] checks to
ensure that events affecting multiple PRs fall back to the run-unique group
instead of mixing work across PRs. Update the regression guard to enforce
this constraint and add a test case verifying rejection of configurations
without the multi-PR guard.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@don-petry

Copy link
Copy Markdown
Contributor Author

Fixed in .github/workflows/pr-auto-review.yml and scripts/test-pr-auto-review-workflow.sh: added !pull_requests[1] guard to both per-PR concurrency branches. This prevents multi-PR events from incorrectly sharing a cancelable group. Updated the regression guard to enforce this constraint and added a test case verifying rejection of configurations without the multi-PR check.

@don-petry

Copy link
Copy Markdown
Contributor Author

Addressed hardening proposals:

  1. Validate the reusable workflow's interruption and repetition contract — This is outside the scope of this PR. The reusable workflow lives in petry-projects/.github and should be hardened separately. Valid point for future work.

  2. Strengthen the concurrency regression control — Now addressed with the addition of !pull_requests[1] guards to the per-PR branches. This prevents multi-PR events from colliding into a single cancelable group. The regression guard has been strengthened to enforce this constraint, and test coverage added to verify rejection of vulnerable configurations.

The regression check now prevents future drift by requiring both the PR-number check AND the multi-PR guard in the canonical policy.

@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.

@don-petry
don-petry enabled auto-merge (squash) October 11, 2026 18:04
@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@don-petry don-petry added the dev-lead:hands-off Exclude from dev-lead agent automation label Oct 11, 2026
…again)

Reverts ec9ff01, which re-added the pull_requests[1] guard to the group
expression after a reviewer-bot comment. Issue #508 requires the concurrency
surface to match petry-projects/.github standards/workflows/pr-auto-review.yml
verbatim; with the guard the compliance audit's stub_surface_drift comparison
still reports drift. The tree is identical to 28d9085, on which all required
checks and the workflow regression guards passed.

The maintainer has chosen to merge this PR by hand; it carries
dev-lead:hands-off so the guard is not added back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U1bsmUpeti2XUHnhQxWHU9
@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.

@don-petry

Copy link
Copy Markdown
Contributor Author

dev-lead is withholding action on this item.

It is labeled dev-lead:hands-off (deliberately kept off-limits to dev-lead (e.g. changes to the dev-lead automation itself)), so dev-lead will not pick it up while that label is present. This notice is posted once so the withhold is visible rather than looking like a stalled run.

To re-enable automated pickup: remove the dev-lead:hands-off label.

@sonarqubecloud

Copy link
Copy Markdown

@don-petry
don-petry disabled auto-merge October 11, 2026 19:50
@don-petry
don-petry merged commit 1ec7eae into main Oct 11, 2026
23 checks passed
@don-petry
don-petry deleted the dev-lead/issue-508-20260925-1750 branch October 11, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev-lead:hands-off Exclude from dev-lead agent automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compliance: stub-surface-drift-pr-auto-review.yml-concurrency

2 participants