Skip to content

fix(review): hold PRs whose review cannot complete, never "safe to merge" - #2008

Merged
JSONbored merged 1 commit into
mainfrom
claude/disposition-hold-on-blockers
Jul 1, 2026
Merged

fix(review): hold PRs whose review cannot complete, never "safe to merge"#2008
JSONbored merged 1 commit into
mainfrom
claude/disposition-hold-on-blockers

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

What

When preflight cannot complete the review (the repo's review lane is unknown/inactive, so the PR isn't eligible for a confident automated recommendation), the unified disposition previously still rendered an otherwise-ready PR as Approve/Merge. The preflight hold only fed the advisory readiness score, never the merge/close disposition — so an incomplete review could read as "safe to merge."

This threads preflightHeld (preflight.status === "hold") through the comment bridge into deriveUnifiedStatus, which downgrades an otherwise-ready status to a manual-review hold.

Why it's safe

Also corrects the preflight-hold validation-component message to describe the real cause (review lane unavailable) instead of a nonexistent "blocker."

Tests

  • unified-comment.test.ts — a preflight hold downgrades a gate merge verdict to held; a clean merge with no hold still renders ready (downgrade-only invariant); a gate merge with advisory blockers stays authoritative-ready.
  • unified-comment-bridge.test.ts — a passing PR with preflightHeld renders > [!WARNING] / Manual Review, never > [!TIP] / Approve-Merge.
  • signals-coverage.test.ts — updated to the corrected preflight-hold message.

Full unsharded test:coverage green; typecheck green.

Advances #1936. Refs #2002.

… merge"

When preflight cannot complete the review (the repo's review lane is
unknown/inactive), the disposition previously still rendered an otherwise-ready
PR as approve/merge — the preflight hold only fed the advisory readiness score,
never the merge/close disposition. An incomplete review must never read as
"safe to merge".

Thread `preflightHeld` (preflight.status === "hold") through the comment bridge
into `deriveUnifiedStatus`, which downgrades an otherwise-`ready` status to a
manual-review hold. Applied last and only to a would-be-`ready` status, so it
can only ever downgrade, never approve. A gate `merge` verdict carrying advisory
blockers stays authoritative-ready by design — the gate already weighed those;
tightening that lives in the gate's confidence/approval bars, not this renderer.

Also correct the preflight-hold validation-component message to describe the
real cause (review lane unavailable) rather than a nonexistent blocker.

Advances #1936. Refs #2002.
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.69%. Comparing base (5e82d8e) to head (af0c162).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2008   +/-   ##
=======================================
  Coverage   95.69%   95.69%           
=======================================
  Files         222      222           
  Lines       24584    24586    +2     
  Branches     8923     8925    +2     
=======================================
+ Hits        23526    23528    +2     
  Misses        433      433           
  Partials      625      625           
Files with missing lines Coverage Δ
src/queue/processors.ts 90.02% <ø> (ø)
src/review/unified-comment-bridge.ts 96.15% <100.00%> (+0.03%) ⬆️
src/review/unified-comment.ts 99.44% <100.00%> (+<0.01%) ⬆️
src/signals/engine.ts 97.36% <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 0667d48 into main Jul 1, 2026
11 checks passed
@JSONbored
JSONbored deleted the claude/disposition-hold-on-blockers branch July 1, 2026 08:40
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant