fix(ci): promote workflow comment-spam hardening to main - Hotfix - #1265
Merged
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces pull-request comment-based gate revalidation with trusted CodeRabbit status events and maintainer-controlled labels. It adds SHA-based PR resolution, fail-closed handling, issue-comment filtering, waiver messaging, regression coverage, documentation, and automated validation. ChangesWorkflow hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CodeRabbit
participant PRGateWorkflow
participant GitHubAPI
participant PullRequest
CodeRabbit->>PRGateWorkflow: Emit successful status with commit SHA
PRGateWorkflow->>GitHubAPI: List pull requests associated with SHA
GitHubAPI-->>PRGateWorkflow: Return open PR candidates
PRGateWorkflow->>PullRequest: Match current head SHA
PRGateWorkflow->>PullRequest: Revalidate and update gate state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
Wibias
changed the title
Wibias
marked this pull request as ready for review
August 8, 2026 06:00
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/enforce-pr-target.yml:
- Line 59: Update the explanatory comments in
.github/workflows/enforce-pr-target.yml at lines 53-58 and 560-564: replace the
stale issue_comment references with equivalent status-event wording, including
the statements about the default branch, missing pull_request.head.sha, and
readiness promotion. Keep the workflow logic unchanged and align both comments
with the status conditions at lines 59 and 567.
- Around line 499-513: Authorize GUI_SCREENSHOT_WAIVER_LABEL before filtering
missing_ui_screenshot failures: validate its actor against the same
maintainer-controlled authorization used by hasGuiOverride/readMaintainerLogins,
and only treat the label as an effective waiver when authorized. Update
tests/ci-workflows.test.ts lines 2658-2696 to cover an unauthorized label actor
and assert the screenshot failure remains; the workflow site requires the
authorization change, and the test site requires the regression case.
In @.github/workflows/pr-hygiene.yml:
- Around line 14-18: Restore PR-level concurrency in the workflow group used by
the shared gate comment by including github.event.pull_request.number together
with the head SHA, matching the corresponding enforce-pr-target workflow so
different heads on the same PR serialize read-modify-write updates. Also obtain
the explicit security review required by MAINTAINERS.md for this
release-adjacent workflow change.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 98ad8341-3000-47e2-8f7c-93a42ef52502
📒 Files selected for processing (10)
.github/scripts/enforce-pr-target.test.cjs.github/workflows/enforce-issue-quality.yml.github/workflows/enforce-pr-target.yml.github/workflows/pr-hygiene.ymldevlog/_plan/260808_workflow_comment_spam_hardening/000_plan.mddocs-site/src/content/docs/contributing/pr-quality.mdstructure/06_docs-and-release.mdtests/ci-workflows.test.tstests/helpers/enforce-pr-target-harness.tstests/zz-pr-coderabbit-readiness-revalidation.test.ts
Contributor
Fixes Applied SuccessfullyFixed 2 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken:
Lines 51–57 uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
# `pull_request_target` runs from the PR base revision, so use that
- # immutable base SHA for the trusted scripts. `issue_comment` runs the
+ # immutable base SHA for the trusted scripts. `status` runs the
# privileged workflow from the repository default branch; source its
# scripts and MAINTAINERS.md from that same promoted trust boundary.
# This prevents unpromoted `dev` script changes from executing withLines 496–508 // the GUI waives the screenshot gate. The flag is what tells the
// author the screenshot is not required, even though the failure
// itself is gone from `failures`.
- const screenshotWaivedByLabel = (pr.labels ?? []).some(
+ //
+ // The gui-screenshot-waived label also waives the gate, but ONLY
+ // when applied by an authorized maintainer (defense-in-depth: even
+ // though GitHub restricts label additions to users with triage+
+ // permission, explicitly verify against MAINTAINERS.md for
+ // consistency with hasGuiOverride's authorization pattern).
+ const labelPresent = (pr.labels ?? []).some(
label => label.name === GUI_SCREENSHOT_WAIVER_LABEL
);
- if (screenshotWaivedByLabel) {
- failures = failures.filter(
- failure => failure.code !== "missing_ui_screenshot"
- );
+ let screenshotWaivedByLabel = false;
+ if (labelPresent) {
+ const labelActor = context.payload.sender?.login;
+ const maintainers = readMaintainerLogins();
+ const actorIsMaintainer = labelActor && maintainers.includes(labelActor);
+ if (actorIsMaintainer) {
+ screenshotWaivedByLabel = true;
+ failures = failures.filter(
+ failure => failure.code !== "missing_ui_screenshot"
+ );
+ } else if (labelActor) {
+ core.info(
+ `The ${GUI_SCREENSHOT_WAIVER_LABEL} label was applied by ${labelActor}, who is not in MAINTAINERS.md; the waiver is ignored.`
+ );
+ }
}
const screenshotWaived =
screenshotWaivedByLabel || hasGuiOverride({ comments });Lines 557–567 // (see `completionIsStale`). When it is stale the gate resets the
// boxes and the notification state, re-drafts, and tells the
// author to re-test and re-tick against the latest code.
- // `issue_comment` events carry no `pull_request.head.sha`, so the
- // fallback to the live head would let a completed checklist with
- // no recorded completion head pass as if it attested the current
- // head. A comment-triggered run must not promote readiness: pass
- // the live head only when the event actually delivered it.
+ // `status` events carry no `pull_request.head.sha` (only
+ // `github.event.sha`), so the fallback to the live head would let
+ // a completed checklist with no recorded completion head pass as if
+ // it attested the current head. A status-triggered run must not
+ // promote readiness: pass the live head only when the event
+ // actually delivered it.
const eventHeadSha =
context.payload.pull_request?.head?.sha ??
(context.eventName === "status" |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 784f6f6b26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/_agent_patch_1265_codex.py:
- Around line 83-87: Replace eventSenderLogin-based authorization in
.github/scripts/_agent_patch_1265_codex.py#L83-L87 with durable provenance
showing that a listed maintainer applied the current
GUI_SCREENSHOT_WAIVER_LABEL, and clear that authorization when the label is
removed; do not infer it from later PR, status, or wake-up event senders. Update
tests/ci-workflows.test.ts#L2701-L2726 to model a labeled event with its label
actor and add regressions for an unauthorized label followed by a maintainer
event and an authorized label followed by a CodeRabbit status event.
In @.github/workflows/_agent_fix_1265_codex_v2.yml:
- Around line 27-34: Retain
.github/workflows/_agent_fix_1265_codex_v2.yml#L27-L34 as the sole workflow
owning the patch scripts and cleanup; no direct change is required there. Remove
the competing .github/workflows/_agent_fix_1265_codex.yml#L27-L31 workflow
entirely so both workflows cannot delete each other’s required inputs.
- Around line 7-30: Update both
.github/workflows/_agent_fix_1265_codex_v2.yml#L7-L30 and
.github/workflows/_agent_fix_1265_codex.yml#L7-L29 to add a trusted admission
step before the hotfix checkout and patch-script execution: check out
github.event.repository.default_branch first, then run
.github/scripts/issue-quality.cjs with
rejectsWorkflowDispatchNonDefaultBranch(...). Ensure the validation uses only
the trusted default-branch script and rejects non-default manual dispatches
before any write-capable scripts run.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 15f82368-6cad-4b70-9206-72d9b66287e9
📒 Files selected for processing (6)
.github/scripts/_agent_patch_1265_codex.py.github/scripts/_agent_patch_1265_codex_followup.py.github/workflows/_agent_fix_1265_codex.yml.github/workflows/_agent_fix_1265_codex_v2.ymltests/ci-workflows.test.tstests/helpers/enforce-pr-target-harness.ts
Wibias
force-pushed
the
hotfix/workflow-comment-spam-hardening-main
branch
from
August 8, 2026 06:37
f45d3cd to
e851c79
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/_agent_blobs_1265_codex.yml:
- Around line 18-60: Update .github/workflows/_agent_blobs_1265_codex.yml lines
18-60 to check out the triggering push SHA, apply the patches, run the required
validation contract on the patched files, and create blobs only after
validation; use an immutable approved SHA for manual dispatches and add
concurrency keyed to the target revision. Apply the same provenance, validation,
immutable-dispatch-SHA, and target-revision concurrency changes to
.github/workflows/_agent_export_1265_codex.yml lines 18-37 before artifact
upload, and to .github/workflows/_agent_export_1265_codex_v2.yml lines 18-34
before uploading its patched workflow files.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2bd57f82-59f6-4d75-9966-a50fd8d84788
📒 Files selected for processing (3)
.github/workflows/_agent_blobs_1265_codex.yml.github/workflows/_agent_export_1265_codex.yml.github/workflows/_agent_export_1265_codex_v2.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/helpers/enforce-pr-target-harness.ts (1)
905-931: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe
statuspayload fabricates anactionthat a real status webhook never delivers.Line 881 sets
actionfor every event exceptissue_comment, so astatusrun in this harness reportsaction: "opened". A realstatuswebhook payload carries noactionfield at all;context.payload.actionisundefinedon the runner.This matters for the exact code path this PR changed.
.github/workflows/enforce-pr-target.ymlLine 653 passeseventAction: context.payload.actionintocompletionIsStale, immediately below the new status-specificeventHeadShafallback at Lines 639-643. The harness therefore exercises the staleness decision witheventAction: "opened"while production exercises it withundefined. IfcompletionIsStalebranches on the action at all, the new status tests intests/ci-workflows.test.tsLines 2775-2791 validate a branch that never executes on a real status event.The same applies to
numberat Line 882, which a status payload also omits.🐛 Proposed fix: model the real status payload shape
payload = { - action: options.eventAction ?? (options.eventName === "issue_comment" ? "created" : "opened"), - number: eventPr.number, + // A real `status` webhook carries neither `action` nor `number`. Keeping + // them here would let the gate read an event field that is always + // `undefined` on the runner. + ...(options.eventName === "status" + ? (options.eventAction === undefined ? {} : { action: options.eventAction }) + : { + action: + options.eventAction ?? + (options.eventName === "issue_comment" ? "created" : "opened"), + number: eventPr.number, + }),The
sendersplit at Lines 921-931 and thesha/context/statefields at Lines 907-909 are correct and match the payload the workflow reads at Lines 204-212.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/helpers/enforce-pr-target-harness.ts` around lines 905 - 931, Update the event payload construction around the status branch so status webhooks omit both action and number, matching the real payload shape and preserving undefined values passed to completionIsStale. Keep the existing action/number fields for event types that provide them, and leave the sender plus status sha, context, and state fields unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/enforce-pr-target.yml:
- Around line 560-587: Update the screenshot-waiver authorization around
screenshotWaivedByLabel to resolve the actor who applied
GUI_SCREENSHOT_WAIVER_LABEL via issues.listEvents, authorize that login against
readMaintainerLogins(), and fail closed if the event list cannot be read instead
of using context.payload.sender.login. In tests/ci-workflows.test.ts at lines
2661-2682 and 2685-2702, retain the label and maintainer fixtures but use a
pull_request_target synchronize event from a contributor, asserting the waiver
persists and the ready output still includes the waiver message and “## ✅
READY”.
- Around line 194-238: Update the write job to consume
needs.resolve-pr.outputs.pull-number via an environment variable, and remove the
duplicated status-event PR resolution and pagination around pull_number. Keep
only the fail-closed guard for a missing or invalid resolved number before
mutation, and update the enforce-pr-target harness fixture and related CI
workflow tests to provide and validate the env-based input.
In `@tests/ci-workflows.test.ts`:
- Around line 909-922: Update the workflow assertion for the enforce-target job
to require an explicit permissions block containing only the scopes that job
uses, rather than allowing it to inherit workflow-level write permissions.
Adjust the expected job keys and add an assertion for enforce-target.permissions
alongside the existing runs-on, needs, if, and concurrency checks.
In `@tests/zz-pr-coderabbit-readiness-revalidation.test.ts`:
- Around line 44-56: Replace the exact normalized-expression comparison in the
`resolve-pr` guard test with individual `toContain` assertions, matching the
guard-by-guard approach used in `tests/ci-workflows.test.ts`. Assert the
required event, context, state, sender identity, sender ID, and pull-request
label conditions while allowing harmless predicate reordering, additional
trusted producers, or formatting changes.
---
Outside diff comments:
In `@tests/helpers/enforce-pr-target-harness.ts`:
- Around line 905-931: Update the event payload construction around the status
branch so status webhooks omit both action and number, matching the real payload
shape and preserving undefined values passed to completionIsStale. Keep the
existing action/number fields for event types that provide them, and leave the
sender plus status sha, context, and state fields unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d605e82-104f-45cf-893b-d44aca85131f
📒 Files selected for processing (5)
.github/workflows/enforce-pr-target.yml.github/workflows/pr-hygiene.ymltests/ci-workflows.test.tstests/helpers/enforce-pr-target-harness.tstests/zz-pr-coderabbit-readiness-revalidation.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/_agent_fix_1265_coderabbit.yml:
- Line 42: Construct the GitHub expression literal in Python without writing
`${{` directly in the agent workflow source, then reuse that symbol at
.github/workflows/_agent_fix_1265_coderabbit.yml lines 42-42, 104-104, and
148-148. Apply it to the generated env.RESOLVED_PULL_NUMBER value and both
generated/readiness assertions so the target workflow contains the resolver
expression while this workflow remains actionlint-valid.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5952dd3f-671f-4ea6-a628-4c57c217b52f
📒 Files selected for processing (1)
.github/workflows/_agent_fix_1265_coderabbit.yml
Wibias
force-pushed
the
hotfix/workflow-comment-spam-hardening-main
branch
from
August 8, 2026 07:15
01f41cb to
ac2c446
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mainmain's release-only changes intact; the two commits unique tomaintouch onlypackage.jsonissue_commentgate reruns with trusted CodeRabbitstatusrevalidation and label-based GUI waiver rerunsWhy this is separate
devandmainhave diverged substantially, so mergingdevintomainwould promote many unrelated commits. This PR is exactly one commit on top of currentmainand contains only the ten files changed by #1255.Validation
The exact promoted file versions are the versions merged in #1255, where the final validation was:
bun test tests/zz-pr-coderabbit-readiness-revalidation.test.ts tests/ci-workflows.test.ts— 129 pass, 0 failnode --test .github/scripts/*.test.cjs— 451 pass, 0 failbun run typecheck— passbun run privacy:scan— passgit diff --check— passRollout
After this PR is merged to
main, the default-branch-controlledpull_request_target/statusbehavior becomes live. Only then should the currently disabled workflows be re-enabled and observed for a reduction inissue_comment-driven runs.Summary by CodeRabbit
New Features
gui-screenshot-waivedlabel support for bypassing screenshot requirements.Bug Fixes
Documentation
Tests