Skip to content

fix(review): resync bot re-entry before skipping#5151

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-bot-author-re-entry-skip-issue
Jul 12, 2026
Merged

fix(review): resync bot re-entry before skipping#5151
JSONbored merged 1 commit into
mainfrom
codex/fix-bot-author-re-entry-skip-issue

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent a security bypass where actorless re-entry paths could skip review solely because the stored PR author looked like a trusted automation bot, which allowed human-modified bot branches to evade review/gate work.

Description

  • Defer the actorless automation-bot early-return in reReviewStoredPullRequest until after a live PR resync and preserve the pre-resync stored head SHA for comparison by introducing storedHeadShaBeforeResync and automationBotSkipEnabled checks in src/queue/processors.ts.
  • Only skip actorless re-entry when the live fetch succeeds and live.head.sha === storedHeadShaBeforeResync, so a failed fetch or a drifted head falls through to the full review and gate path.
  • Update isTrustedAutomationBotAuthor doc comment in src/settings/automation-bot-skip.ts to clarify that stored authorship is a necessary precondition (not sufficient proof) for actorless skips.
  • Add regression coverage in test/unit/queue-lifecycle-guards.test.ts and adjust test/unit/automation-bot-skip.test.ts to exercise the unchanged-bot-head and head-drift cases, and regenerate runtime Worker types (worker-configuration.d.ts) produced by the typegen step.

Testing

  • Ran targeted unit tests with npx vitest run test/unit/automation-bot-skip.test.ts test/unit/queue-lifecycle-guards.test.ts -t "automation-bot-skip|isTrustedAutomationBotAuthor|isTrustedAutomationBotWebhookActor", which passed for the focused regressions.
  • Verified git diff --check returned clean results and regenerated worker types with npm run cf-typegen, which succeeded and updated worker-configuration.d.ts.
  • Ran the full gate with npm run test:ci; early gate steps (typecheck, cf-typegen, etc.) completed but the full unsharded coverage run reported unrelated queue-suite timeouts/failures in the broader test/unit/queue.* suites, so the consolidated test:ci run did not finish green in this environment.
  • npm audit --audit-level=moderate was attempted but failed due to the registry audit endpoint returning 403 Forbidden in this environment.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored force-pushed the codex/fix-bot-author-re-entry-skip-issue branch from b763aaa to 04d513e Compare July 12, 2026 09:03
@JSONbored JSONbored self-assigned this Jul 12, 2026
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.34%. Comparing base (448dd3e) to head (04d513e).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5151   +/-   ##
=======================================
  Coverage   94.34%   94.34%           
=======================================
  Files         473      473           
  Lines       39965    39967    +2     
  Branches    14570    14570           
=======================================
+ Hits        37704    37706    +2     
  Misses       1585     1585           
  Partials      676      676           
Flag Coverage Δ
shard-1 46.33% <40.00%> (-0.01%) ⬇️
shard-2 34.41% <0.00%> (-0.26%) ⬇️
shard-3 31.20% <0.00%> (-0.81%) ⬇️
shard-4 32.73% <100.00%> (+0.75%) ⬆️
shard-5 33.66% <0.00%> (-0.10%) ⬇️
shard-6 45.05% <40.00%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/queue/processors.ts 95.51% <100.00%> (+<0.01%) ⬆️
src/settings/automation-bot-skip.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit dc7a19b into main Jul 12, 2026
17 checks passed
@JSONbored
JSONbored deleted the codex/fix-bot-author-re-entry-skip-issue branch July 12, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant