fix(github): pull requests short of required approvals no longer show as approved #25725 · pingdotgg/t3code

PR	GitHub `reviewDecision` / latest reviews	Row label on main (becf853def)	Row label with fix (dcbaace321)
nodejs/node#67028	`REVIEW_REQUIRED` / 1 × APPROVED	Approved (green)	Awaiting review (muted)
juliusmarminge/turbo-respect-pnpm-configs#1 (control, no review rule)	`null` / 1 × APPROVED	Approved	Approved

MmacroscopeappBotapproved these changes3d ago

Approvability

Verdict: Approved at 0402298

Macroscope's review found this PR approvable — This narrowly scoped fix prevents an individual approval from making a pull request appear approved when GitHub reports that required branch approvals are still missing. A focused regression test covers the behavior, with no product-default or static-analysis configuration changes.

^{You can add or adjust custom eligibility rules. Learn more.}

Ggithub-actionsBot3d ago

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	20.9 KiB	20.9 KiB	0 B (0.0%)	29.3 KiB	✅
Codex	Live turn messages	2	2	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	−7 B (−0.1%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	−7 B (−0.6%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	0 B (0.0%)	29.3 KiB	✅
Claude	Live turn messages	2	2	0 (0.0%)	8	✅

Baseline: daa720a · PR result: 0402298 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

Codex decoded thread snapshot: 108.5 KiB
Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

CcoderabbitaiBot3d ago

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: fix/issue-25717
File: .coderabbit.config.ts
Commit: 0402298
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: 8d7276ec-c0c2-46a1-bb87-08bfba1599f8

External image Commits

Reviewing files that changed from the base of the PR and between daa720a986ca40e59b063206bb03ccef87f008de and 0402298415772c8fbd6fb157a66c71afb9d5b0db.

External image Files selected for processing: 2

packages/source-control-github/src/server/gitHubPullRequestJson.test.ts
packages/source-control-github/src/server/gitHubPullRequestJson.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

The decoder now uses an approved latest review as a fallback only when GitHub’s summarized review decision is null. A regression test verifies that REVIEW_REQUIRED remains review-required when the latest review is approved.

Changes: Review decision decoding

Layer / File(s)	Summary
Review decision fallback and regression test `packages/source-control-github/src/server/gitHubPullRequestJson.ts`, `packages/source-control-github/src/server/gitHubPullRequestJson.test.ts`	An approved latest review sets the decoded decision to `approved` only when the summarized decision is null. The test checks that a summarized `REVIEW_REQUIRED` decision remains `review-required`.

Priority: External image Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: maria-rcks

Merge Risk: External image Minimal · up to 04022

The change correctly preserves required-review status, with no actionable merge risk established.

Pre-merge checks: ^Passed 3 · ^Inconclusive 1

^Failed Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Description check	Inconclusive	The description follows the repository template closely. It has a clear Problem section with a reproduction path and environment (main at becf853def, a nodejs/node PR linked to a thread). The Change s…	Add a link to the triaged issue `#25717` together with the maintainer approval comment or discussion that approved the direction and scope, or explain why the fix qualifies as a very small, focused bug fix without prior approval. Because this…

^Passed Passed checks: 3 passed

Check name	Status	Explanation
Title check	Passed	The title clearly and concisely describes the fix: pull requests with insufficient required approvals no longer appear approved.
Linked Issues check	Passed	Issue `#25717` requires T3 Code to avoid implying full approval when required approvals are incomplete. The decoder now preserves GitHub's `REVIEW_REQUIRED` result when an approval exists, and the regre…
Out of Scope Changes check	Passed	The pull request changes only the shared review-decision decoder and its focused regression test. These changes directly address required-review display behavior. The diff does not add changes for che…

Full details: Linked Issues check

Explanation

Issue #25717 requires T3 Code to avoid implying full approval when required approvals are incomplete. The decoder now preserves GitHub's REVIEW_REQUIRED result when an approval exists, and the regression test verifies review-required for that case. The fallback still maps an approval to approved when GitHub reports no decision. This meets the issue's equivalent-status requirement without requiring an approval count.

Full details: Out of Scope Changes check

Explanation

The pull request changes only the shared review-decision decoder and its focused regression test. These changes directly address required-review display behavior. The diff does not add changes for checks, merge queues, or unrelated functionality.

Full details: Description check

Explanation

The description follows the repository template closely. It has a clear Problem section with a reproduction path and environment (main at becf853def, a nodejs/node PR linked to a thread). The Change section explains the fix in toReviewDecisionWithReviews and why the shared decoder reaches the thread panel, Pull requests page, and mobile. The Scope and approval section links the issue (#25717) but does not link a maintainer approval comment or discussion. It only gives a bare issue reference, so the template's approval requirement is not clearly met. The Verification section is strong: it names the focused test and its fail-then-pass result, the full check run, and a live before/after table of row labels. It also states what was not checked (screenshots, mobile). Remaining gaps: no explicit maintainer approval link or comment, and no before/after screenshots, which the template requires for UI changes. The author did note screenshots were not attached and that the table records rendered labels instead.

Resolution

Add a link to the triaged issue #25717 together with the maintainer approval comment or discussion that approved the direction and scope, or explain why the fix qualifies as a very small, focused bug fix without prior approval. Because this changes a user-visible UI label, attach before/after screenshots of the thread panel row (main versus fix) and embed or link them in the Verification section. Mobile was not run, so either run it or keep the note that it was not checked.

Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

fix(github): pull requests short of required approvals no longer show as approved#25725

Problem

Change

Scope and approval

Verification

Checks

Approvability

Thread transfer impact