fix(web): stablize merge button with merged chip - #6271

Closed
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state
Closed

fix(web): stablize merge button with merged chip#6271
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state

Conversation

@flamboh

@flambohflamboh commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5 on behalf of Oliver

What Changed

The Merge button now stays pending until refreshed PR data confirms the merge, then becomes a solid violet "Merged" marker that persists on every merged PR, so opening one shows the outcome where the action used to be (thanks @Bil0000!). The merge success toast only fires as a fallback on hosts that confirm merges asynchronously (e.g. Azure DevOps), where the marker has to yield before the host catches up.

The action lifecycle lives in packages/client-runtime/src/state/pullRequestActionState.ts with transitions tracked by per-PR atoms.

Why

The merge command finished before the detail refresh, so stale open-state data briefly made the Merge button clickable again.

UI Changes

Before

prb_before.mp4

After

prb_after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Built by GPT-5.6-sol (Codex CLI) and Claude Fable 5 (Claude Code) in T3 Code, with the merged-button presentation contributed by @Bil0000.

Note

Stabilize PR merge button with merged chip via shared action state

  • Adds a per-pull-request action state atom family in pullRequestActionState.ts that tracks pending action and a merge hold, reconciled by observePullRequestDetail from incoming detail reads.
  • Introduces usePullRequestActionState and pullRequestMergeButtonPresentation so PullRequestDetailPanel.tsx shows a disabled, violet-toned "Merged" chip during merge hold or once the host reports merged, and "Merging..." only while the merge is in-flight.
  • Defers the merge success toast until the merge hold clears for the same PR; non-merge action toasts stay immediate.
  • Risk: merge hold clears only when observePullRequestDetail sees detail state leave open or settle open after a pending read; if the host never delivers such a read, the hold persists — verify the reconciliation logic in observePullRequestDetail.

Macroscope summarized 4ae02b4.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0d1a64f8-9abb-4f83-9678-afda7307e213

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 12, 2026
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@macroscopeapp

macroscopeappBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4ae02b4

Macroscope's review found this PR approvable — This is a focused client-side fix that stabilizes the existing merge control and shows a confirmed merged state. Its finite per-PR state tracking and presentation logic are tested, while merge RPC behavior, schemas, and deployment remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from fd4ffaa to 3c20061CompareAugust 12, 2026 22:10
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 12, 2026 22:10

Dismissing prior approval to re-evaluate 3c20061

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 3c20061 to 592e3b9CompareAugust 14, 2026 01:13
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 14, 2026 01:13

Dismissing prior approval to re-evaluate 592e3b9

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 14, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 592e3b9 to 23f892fCompareAugust 15, 2026 18:56

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 15, 2026 19:31

Dismissing prior approval to re-evaluate 9fdf68d

@github-actionsgithub-actionsBot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the extended pending window introduced for merge. Everything else in the diff (per-pull-request keying of pendingAction, identity-guarded clears, the activePendingAction label read) is consistent with the panel's existing "only the pressed control speaks" contract.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the merge-completion suppression: the header's primary control is unmounted rather than shown in a resolved state, and the suppression only lifts on a non-open detail, so a host that keeps reporting the pull request as open leaves the slot permanently empty.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 15, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from e3d9363 to c2a2401CompareAugust 19, 2026 06:45
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 19, 2026 06:45

Dismissing prior approval to re-evaluate c2a2401

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 19, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from c2a2401 to 022268eCompareAugust 21, 2026 00:59
@maria-rcks

Copy link
Copy Markdown
Collaborator

pls never record the videos like that, NEVER; they make it hard to see what actually changed

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks yeah sorry you're definitely right, I'll put new ones in a sec

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks fixed, should be pretty clear now. Again, sorry. Got carried away with the Cap default edits on the vid.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 022268e to 237e0deCompareAugust 23, 2026 09:57
@github-actionsgithub-actionsBot added size:XXL 1,000+ changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). size:XXL 1,000+ changed lines (additions + deletions). labels Sep 1, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the new merged-button tone. The lifecycle rework itself reads consistently with the panel's contracts: the hold is per-pull-request, only the Merge control is suppressed while it is held, and detailQuery.isPending is the waiting flag of the AsyncResult (true on a refresh() refetch that already has data), so the post-merge read does arm and then clear the bound.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@Bil0000

Copy link
Copy Markdown
Contributor

@flamboh would be cool if you update your vid/img in the desc too 🫡

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 working on that already!

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reworked hold itself checks out: useEnvironmentQuery.isPending maps to AsyncResult.waiting, so the refreshDetail() a merge takes does flip it true and then false, and observePullRequestDetail bounds the hold on that read regardless of which refresh path ran — the earlier unbounded-hold and per-PR-overwrite findings are addressed. Two findings on how the completed state is presented once it clears, inline.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 is it intended that the "Merged" button is only shown briefly?
image
I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

@Bil0000

Copy link
Copy Markdown
Contributor

@Bil0000 is it intended that the "Merged" button is only shown briefly? image I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

Change it however you like, & add some after media, will take a look then & let you know my thoughts ;)

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@flambohflamboh changed the title fix(web): keep merge pending until refreshfix(web): stablize merge button with merged chipSep 1, 2026
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 see new after video, macroscope approved as well!

@maria-rcks

Copy link
Copy Markdown
Collaborator

closed because of #9188 (sorry)

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks LOL no it's all good. Glad it got fixed!!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flamboh@maria-rcks@Bil0000
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(web): stablize merge button with merged chip - #6271

Closed
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state
Closed

fix(web): stablize merge button with merged chip#6271
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state

Conversation

@flamboh

@flambohflamboh commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5 on behalf of Oliver

What Changed

The Merge button now stays pending until refreshed PR data confirms the merge, then becomes a solid violet "Merged" marker that persists on every merged PR, so opening one shows the outcome where the action used to be (thanks @Bil0000!). The merge success toast only fires as a fallback on hosts that confirm merges asynchronously (e.g. Azure DevOps), where the marker has to yield before the host catches up.

The action lifecycle lives in packages/client-runtime/src/state/pullRequestActionState.ts with transitions tracked by per-PR atoms.

Why

The merge command finished before the detail refresh, so stale open-state data briefly made the Merge button clickable again.

UI Changes

Before

prb_before.mp4

After

prb_after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Built by GPT-5.6-sol (Codex CLI) and Claude Fable 5 (Claude Code) in T3 Code, with the merged-button presentation contributed by @Bil0000.

Note

Stabilize PR merge button with merged chip via shared action state

  • Adds a per-pull-request action state atom family in pullRequestActionState.ts that tracks pending action and a merge hold, reconciled by observePullRequestDetail from incoming detail reads.
  • Introduces usePullRequestActionState and pullRequestMergeButtonPresentation so PullRequestDetailPanel.tsx shows a disabled, violet-toned "Merged" chip during merge hold or once the host reports merged, and "Merging..." only while the merge is in-flight.
  • Defers the merge success toast until the merge hold clears for the same PR; non-merge action toasts stay immediate.
  • Risk: merge hold clears only when observePullRequestDetail sees detail state leave open or settle open after a pending read; if the host never delivers such a read, the hold persists — verify the reconciliation logic in observePullRequestDetail.

Macroscope summarized 4ae02b4.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0d1a64f8-9abb-4f83-9678-afda7307e213

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 12, 2026
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@macroscopeapp

macroscopeappBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4ae02b4

Macroscope's review found this PR approvable — This is a focused client-side fix that stabilizes the existing merge control and shows a confirmed merged state. Its finite per-PR state tracking and presentation logic are tested, while merge RPC behavior, schemas, and deployment remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from fd4ffaa to 3c20061CompareAugust 12, 2026 22:10
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 12, 2026 22:10

Dismissing prior approval to re-evaluate 3c20061

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 3c20061 to 592e3b9CompareAugust 14, 2026 01:13
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 14, 2026 01:13

Dismissing prior approval to re-evaluate 592e3b9

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 14, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 592e3b9 to 23f892fCompareAugust 15, 2026 18:56

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 15, 2026 19:31

Dismissing prior approval to re-evaluate 9fdf68d

@github-actionsgithub-actionsBot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the extended pending window introduced for merge. Everything else in the diff (per-pull-request keying of pendingAction, identity-guarded clears, the activePendingAction label read) is consistent with the panel's existing "only the pressed control speaks" contract.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the merge-completion suppression: the header's primary control is unmounted rather than shown in a resolved state, and the suppression only lifts on a non-open detail, so a host that keeps reporting the pull request as open leaves the slot permanently empty.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 15, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from e3d9363 to c2a2401CompareAugust 19, 2026 06:45
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 19, 2026 06:45

Dismissing prior approval to re-evaluate c2a2401

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 19, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from c2a2401 to 022268eCompareAugust 21, 2026 00:59
@maria-rcks

Copy link
Copy Markdown
Collaborator

pls never record the videos like that, NEVER; they make it hard to see what actually changed

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks yeah sorry you're definitely right, I'll put new ones in a sec

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks fixed, should be pretty clear now. Again, sorry. Got carried away with the Cap default edits on the vid.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 022268e to 237e0deCompareAugust 23, 2026 09:57
@github-actionsgithub-actionsBot added size:XXL 1,000+ changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). size:XXL 1,000+ changed lines (additions + deletions). labels Sep 1, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the new merged-button tone. The lifecycle rework itself reads consistently with the panel's contracts: the hold is per-pull-request, only the Merge control is suppressed while it is held, and detailQuery.isPending is the waiting flag of the AsyncResult (true on a refresh() refetch that already has data), so the post-merge read does arm and then clear the bound.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@Bil0000

Copy link
Copy Markdown
Contributor

@flamboh would be cool if you update your vid/img in the desc too 🫡

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 working on that already!

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reworked hold itself checks out: useEnvironmentQuery.isPending maps to AsyncResult.waiting, so the refreshDetail() a merge takes does flip it true and then false, and observePullRequestDetail bounds the hold on that read regardless of which refresh path ran — the earlier unbounded-hold and per-PR-overwrite findings are addressed. Two findings on how the completed state is presented once it clears, inline.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 is it intended that the "Merged" button is only shown briefly?
image
I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

@Bil0000

Copy link
Copy Markdown
Contributor

@Bil0000 is it intended that the "Merged" button is only shown briefly? image I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

Change it however you like, & add some after media, will take a look then & let you know my thoughts ;)

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@flambohflamboh changed the title fix(web): keep merge pending until refreshfix(web): stablize merge button with merged chipSep 1, 2026
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 see new after video, macroscope approved as well!

@maria-rcks

Copy link
Copy Markdown
Collaborator

closed because of #9188 (sorry)

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks LOL no it's all good. Glad it got fixed!!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flamboh@maria-rcks@Bil0000
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(web): stablize merge button with merged chip - #6271

Closed
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state
Closed

fix(web): stablize merge button with merged chip#6271
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state

Conversation

@flamboh

@flambohflamboh commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5 on behalf of Oliver

What Changed

The Merge button now stays pending until refreshed PR data confirms the merge, then becomes a solid violet "Merged" marker that persists on every merged PR, so opening one shows the outcome where the action used to be (thanks @Bil0000!). The merge success toast only fires as a fallback on hosts that confirm merges asynchronously (e.g. Azure DevOps), where the marker has to yield before the host catches up.

The action lifecycle lives in packages/client-runtime/src/state/pullRequestActionState.ts with transitions tracked by per-PR atoms.

Why

The merge command finished before the detail refresh, so stale open-state data briefly made the Merge button clickable again.

UI Changes

Before

prb_before.mp4

After

prb_after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Built by GPT-5.6-sol (Codex CLI) and Claude Fable 5 (Claude Code) in T3 Code, with the merged-button presentation contributed by @Bil0000.

Note

Stabilize PR merge button with merged chip via shared action state

  • Adds a per-pull-request action state atom family in pullRequestActionState.ts that tracks pending action and a merge hold, reconciled by observePullRequestDetail from incoming detail reads.
  • Introduces usePullRequestActionState and pullRequestMergeButtonPresentation so PullRequestDetailPanel.tsx shows a disabled, violet-toned "Merged" chip during merge hold or once the host reports merged, and "Merging..." only while the merge is in-flight.
  • Defers the merge success toast until the merge hold clears for the same PR; non-merge action toasts stay immediate.
  • Risk: merge hold clears only when observePullRequestDetail sees detail state leave open or settle open after a pending read; if the host never delivers such a read, the hold persists — verify the reconciliation logic in observePullRequestDetail.

Macroscope summarized 4ae02b4.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0d1a64f8-9abb-4f83-9678-afda7307e213

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 12, 2026
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@macroscopeapp

macroscopeappBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4ae02b4

Macroscope's review found this PR approvable — This is a focused client-side fix that stabilizes the existing merge control and shows a confirmed merged state. Its finite per-PR state tracking and presentation logic are tested, while merge RPC behavior, schemas, and deployment remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from fd4ffaa to 3c20061CompareAugust 12, 2026 22:10
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 12, 2026 22:10

Dismissing prior approval to re-evaluate 3c20061

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 3c20061 to 592e3b9CompareAugust 14, 2026 01:13
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 14, 2026 01:13

Dismissing prior approval to re-evaluate 592e3b9

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 14, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 592e3b9 to 23f892fCompareAugust 15, 2026 18:56

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 15, 2026 19:31

Dismissing prior approval to re-evaluate 9fdf68d

@github-actionsgithub-actionsBot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the extended pending window introduced for merge. Everything else in the diff (per-pull-request keying of pendingAction, identity-guarded clears, the activePendingAction label read) is consistent with the panel's existing "only the pressed control speaks" contract.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the merge-completion suppression: the header's primary control is unmounted rather than shown in a resolved state, and the suppression only lifts on a non-open detail, so a host that keeps reporting the pull request as open leaves the slot permanently empty.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 15, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from e3d9363 to c2a2401CompareAugust 19, 2026 06:45
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 19, 2026 06:45

Dismissing prior approval to re-evaluate c2a2401

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 19, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from c2a2401 to 022268eCompareAugust 21, 2026 00:59
@maria-rcks

Copy link
Copy Markdown
Collaborator

pls never record the videos like that, NEVER; they make it hard to see what actually changed

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks yeah sorry you're definitely right, I'll put new ones in a sec

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks fixed, should be pretty clear now. Again, sorry. Got carried away with the Cap default edits on the vid.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 022268e to 237e0deCompareAugust 23, 2026 09:57
@github-actionsgithub-actionsBot added size:XXL 1,000+ changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). size:XXL 1,000+ changed lines (additions + deletions). labels Sep 1, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the new merged-button tone. The lifecycle rework itself reads consistently with the panel's contracts: the hold is per-pull-request, only the Merge control is suppressed while it is held, and detailQuery.isPending is the waiting flag of the AsyncResult (true on a refresh() refetch that already has data), so the post-merge read does arm and then clear the bound.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@Bil0000

Copy link
Copy Markdown
Contributor

@flamboh would be cool if you update your vid/img in the desc too 🫡

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 working on that already!

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reworked hold itself checks out: useEnvironmentQuery.isPending maps to AsyncResult.waiting, so the refreshDetail() a merge takes does flip it true and then false, and observePullRequestDetail bounds the hold on that read regardless of which refresh path ran — the earlier unbounded-hold and per-PR-overwrite findings are addressed. Two findings on how the completed state is presented once it clears, inline.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 is it intended that the "Merged" button is only shown briefly?
image
I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

@Bil0000

Copy link
Copy Markdown
Contributor

@Bil0000 is it intended that the "Merged" button is only shown briefly? image I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

Change it however you like, & add some after media, will take a look then & let you know my thoughts ;)

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@flambohflamboh changed the title fix(web): keep merge pending until refreshfix(web): stablize merge button with merged chipSep 1, 2026
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 see new after video, macroscope approved as well!

@maria-rcks

Copy link
Copy Markdown
Collaborator

closed because of #9188 (sorry)

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks LOL no it's all good. Glad it got fixed!!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flamboh@maria-rcks@Bil0000
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(web): stablize merge button with merged chip - #6271

Closed
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state
Closed

fix(web): stablize merge button with merged chip#6271
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state

Conversation

@flamboh

@flambohflamboh commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5 on behalf of Oliver

What Changed

The Merge button now stays pending until refreshed PR data confirms the merge, then becomes a solid violet "Merged" marker that persists on every merged PR, so opening one shows the outcome where the action used to be (thanks @Bil0000!). The merge success toast only fires as a fallback on hosts that confirm merges asynchronously (e.g. Azure DevOps), where the marker has to yield before the host catches up.

The action lifecycle lives in packages/client-runtime/src/state/pullRequestActionState.ts with transitions tracked by per-PR atoms.

Why

The merge command finished before the detail refresh, so stale open-state data briefly made the Merge button clickable again.

UI Changes

Before

prb_before.mp4

After

prb_after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Built by GPT-5.6-sol (Codex CLI) and Claude Fable 5 (Claude Code) in T3 Code, with the merged-button presentation contributed by @Bil0000.

Note

Stabilize PR merge button with merged chip via shared action state

  • Adds a per-pull-request action state atom family in pullRequestActionState.ts that tracks pending action and a merge hold, reconciled by observePullRequestDetail from incoming detail reads.
  • Introduces usePullRequestActionState and pullRequestMergeButtonPresentation so PullRequestDetailPanel.tsx shows a disabled, violet-toned "Merged" chip during merge hold or once the host reports merged, and "Merging..." only while the merge is in-flight.
  • Defers the merge success toast until the merge hold clears for the same PR; non-merge action toasts stay immediate.
  • Risk: merge hold clears only when observePullRequestDetail sees detail state leave open or settle open after a pending read; if the host never delivers such a read, the hold persists — verify the reconciliation logic in observePullRequestDetail.

Macroscope summarized 4ae02b4.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0d1a64f8-9abb-4f83-9678-afda7307e213

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 12, 2026
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@macroscopeapp

macroscopeappBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4ae02b4

Macroscope's review found this PR approvable — This is a focused client-side fix that stabilizes the existing merge control and shows a confirmed merged state. Its finite per-PR state tracking and presentation logic are tested, while merge RPC behavior, schemas, and deployment remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from fd4ffaa to 3c20061CompareAugust 12, 2026 22:10
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 12, 2026 22:10

Dismissing prior approval to re-evaluate 3c20061

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 3c20061 to 592e3b9CompareAugust 14, 2026 01:13
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 14, 2026 01:13

Dismissing prior approval to re-evaluate 592e3b9

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 14, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 592e3b9 to 23f892fCompareAugust 15, 2026 18:56

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 15, 2026 19:31

Dismissing prior approval to re-evaluate 9fdf68d

@github-actionsgithub-actionsBot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the extended pending window introduced for merge. Everything else in the diff (per-pull-request keying of pendingAction, identity-guarded clears, the activePendingAction label read) is consistent with the panel's existing "only the pressed control speaks" contract.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the merge-completion suppression: the header's primary control is unmounted rather than shown in a resolved state, and the suppression only lifts on a non-open detail, so a host that keeps reporting the pull request as open leaves the slot permanently empty.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 15, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from e3d9363 to c2a2401CompareAugust 19, 2026 06:45
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 19, 2026 06:45

Dismissing prior approval to re-evaluate c2a2401

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 19, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from c2a2401 to 022268eCompareAugust 21, 2026 00:59
@maria-rcks

Copy link
Copy Markdown
Collaborator

pls never record the videos like that, NEVER; they make it hard to see what actually changed

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks yeah sorry you're definitely right, I'll put new ones in a sec

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks fixed, should be pretty clear now. Again, sorry. Got carried away with the Cap default edits on the vid.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 022268e to 237e0deCompareAugust 23, 2026 09:57
@github-actionsgithub-actionsBot added size:XXL 1,000+ changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). size:XXL 1,000+ changed lines (additions + deletions). labels Sep 1, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the new merged-button tone. The lifecycle rework itself reads consistently with the panel's contracts: the hold is per-pull-request, only the Merge control is suppressed while it is held, and detailQuery.isPending is the waiting flag of the AsyncResult (true on a refresh() refetch that already has data), so the post-merge read does arm and then clear the bound.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@Bil0000

Copy link
Copy Markdown
Contributor

@flamboh would be cool if you update your vid/img in the desc too 🫡

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 working on that already!

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reworked hold itself checks out: useEnvironmentQuery.isPending maps to AsyncResult.waiting, so the refreshDetail() a merge takes does flip it true and then false, and observePullRequestDetail bounds the hold on that read regardless of which refresh path ran — the earlier unbounded-hold and per-PR-overwrite findings are addressed. Two findings on how the completed state is presented once it clears, inline.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 is it intended that the "Merged" button is only shown briefly?
image
I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

@Bil0000

Copy link
Copy Markdown
Contributor

@Bil0000 is it intended that the "Merged" button is only shown briefly? image I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

Change it however you like, & add some after media, will take a look then & let you know my thoughts ;)

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@flambohflamboh changed the title fix(web): keep merge pending until refreshfix(web): stablize merge button with merged chipSep 1, 2026
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 see new after video, macroscope approved as well!

@maria-rcks

Copy link
Copy Markdown
Collaborator

closed because of #9188 (sorry)

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks LOL no it's all good. Glad it got fixed!!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flamboh@maria-rcks@Bil0000
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(web): stablize merge button with merged chip - #6271

Closed
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state
Closed

fix(web): stablize merge button with merged chip#6271
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state

Conversation

@flamboh

@flambohflamboh commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5 on behalf of Oliver

What Changed

The Merge button now stays pending until refreshed PR data confirms the merge, then becomes a solid violet "Merged" marker that persists on every merged PR, so opening one shows the outcome where the action used to be (thanks @Bil0000!). The merge success toast only fires as a fallback on hosts that confirm merges asynchronously (e.g. Azure DevOps), where the marker has to yield before the host catches up.

The action lifecycle lives in packages/client-runtime/src/state/pullRequestActionState.ts with transitions tracked by per-PR atoms.

Why

The merge command finished before the detail refresh, so stale open-state data briefly made the Merge button clickable again.

UI Changes

Before

prb_before.mp4

After

prb_after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Built by GPT-5.6-sol (Codex CLI) and Claude Fable 5 (Claude Code) in T3 Code, with the merged-button presentation contributed by @Bil0000.

Note

Stabilize PR merge button with merged chip via shared action state

  • Adds a per-pull-request action state atom family in pullRequestActionState.ts that tracks pending action and a merge hold, reconciled by observePullRequestDetail from incoming detail reads.
  • Introduces usePullRequestActionState and pullRequestMergeButtonPresentation so PullRequestDetailPanel.tsx shows a disabled, violet-toned "Merged" chip during merge hold or once the host reports merged, and "Merging..." only while the merge is in-flight.
  • Defers the merge success toast until the merge hold clears for the same PR; non-merge action toasts stay immediate.
  • Risk: merge hold clears only when observePullRequestDetail sees detail state leave open or settle open after a pending read; if the host never delivers such a read, the hold persists — verify the reconciliation logic in observePullRequestDetail.

Macroscope summarized 4ae02b4.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0d1a64f8-9abb-4f83-9678-afda7307e213

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 12, 2026
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@macroscopeapp

macroscopeappBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4ae02b4

Macroscope's review found this PR approvable — This is a focused client-side fix that stabilizes the existing merge control and shows a confirmed merged state. Its finite per-PR state tracking and presentation logic are tested, while merge RPC behavior, schemas, and deployment remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from fd4ffaa to 3c20061CompareAugust 12, 2026 22:10
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 12, 2026 22:10

Dismissing prior approval to re-evaluate 3c20061

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 3c20061 to 592e3b9CompareAugust 14, 2026 01:13
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 14, 2026 01:13

Dismissing prior approval to re-evaluate 592e3b9

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 14, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 592e3b9 to 23f892fCompareAugust 15, 2026 18:56

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 15, 2026 19:31

Dismissing prior approval to re-evaluate 9fdf68d

@github-actionsgithub-actionsBot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the extended pending window introduced for merge. Everything else in the diff (per-pull-request keying of pendingAction, identity-guarded clears, the activePendingAction label read) is consistent with the panel's existing "only the pressed control speaks" contract.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the merge-completion suppression: the header's primary control is unmounted rather than shown in a resolved state, and the suppression only lifts on a non-open detail, so a host that keeps reporting the pull request as open leaves the slot permanently empty.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 15, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from e3d9363 to c2a2401CompareAugust 19, 2026 06:45
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 19, 2026 06:45

Dismissing prior approval to re-evaluate c2a2401

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 19, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from c2a2401 to 022268eCompareAugust 21, 2026 00:59
@maria-rcks

Copy link
Copy Markdown
Collaborator

pls never record the videos like that, NEVER; they make it hard to see what actually changed

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks yeah sorry you're definitely right, I'll put new ones in a sec

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks fixed, should be pretty clear now. Again, sorry. Got carried away with the Cap default edits on the vid.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 022268e to 237e0deCompareAugust 23, 2026 09:57
@github-actionsgithub-actionsBot added size:XXL 1,000+ changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). size:XXL 1,000+ changed lines (additions + deletions). labels Sep 1, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the new merged-button tone. The lifecycle rework itself reads consistently with the panel's contracts: the hold is per-pull-request, only the Merge control is suppressed while it is held, and detailQuery.isPending is the waiting flag of the AsyncResult (true on a refresh() refetch that already has data), so the post-merge read does arm and then clear the bound.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@Bil0000

Copy link
Copy Markdown
Contributor

@flamboh would be cool if you update your vid/img in the desc too 🫡

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 working on that already!

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reworked hold itself checks out: useEnvironmentQuery.isPending maps to AsyncResult.waiting, so the refreshDetail() a merge takes does flip it true and then false, and observePullRequestDetail bounds the hold on that read regardless of which refresh path ran — the earlier unbounded-hold and per-PR-overwrite findings are addressed. Two findings on how the completed state is presented once it clears, inline.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 is it intended that the "Merged" button is only shown briefly?
image
I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

@Bil0000

Copy link
Copy Markdown
Contributor

@Bil0000 is it intended that the "Merged" button is only shown briefly? image I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

Change it however you like, & add some after media, will take a look then & let you know my thoughts ;)

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@flambohflamboh changed the title fix(web): keep merge pending until refreshfix(web): stablize merge button with merged chipSep 1, 2026
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 see new after video, macroscope approved as well!

@maria-rcks

Copy link
Copy Markdown
Collaborator

closed because of #9188 (sorry)

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks LOL no it's all good. Glad it got fixed!!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flamboh@maria-rcks@Bil0000
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(web): stablize merge button with merged chip - #6271

Closed
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state
Closed

fix(web): stablize merge button with merged chip#6271
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state

Conversation

@flamboh

@flambohflamboh commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5 on behalf of Oliver

What Changed

The Merge button now stays pending until refreshed PR data confirms the merge, then becomes a solid violet "Merged" marker that persists on every merged PR, so opening one shows the outcome where the action used to be (thanks @Bil0000!). The merge success toast only fires as a fallback on hosts that confirm merges asynchronously (e.g. Azure DevOps), where the marker has to yield before the host catches up.

The action lifecycle lives in packages/client-runtime/src/state/pullRequestActionState.ts with transitions tracked by per-PR atoms.

Why

The merge command finished before the detail refresh, so stale open-state data briefly made the Merge button clickable again.

UI Changes

Before

prb_before.mp4

After

prb_after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Built by GPT-5.6-sol (Codex CLI) and Claude Fable 5 (Claude Code) in T3 Code, with the merged-button presentation contributed by @Bil0000.

Note

Stabilize PR merge button with merged chip via shared action state

  • Adds a per-pull-request action state atom family in pullRequestActionState.ts that tracks pending action and a merge hold, reconciled by observePullRequestDetail from incoming detail reads.
  • Introduces usePullRequestActionState and pullRequestMergeButtonPresentation so PullRequestDetailPanel.tsx shows a disabled, violet-toned "Merged" chip during merge hold or once the host reports merged, and "Merging..." only while the merge is in-flight.
  • Defers the merge success toast until the merge hold clears for the same PR; non-merge action toasts stay immediate.
  • Risk: merge hold clears only when observePullRequestDetail sees detail state leave open or settle open after a pending read; if the host never delivers such a read, the hold persists — verify the reconciliation logic in observePullRequestDetail.

Macroscope summarized 4ae02b4.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0d1a64f8-9abb-4f83-9678-afda7307e213

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 12, 2026
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@macroscopeapp

macroscopeappBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4ae02b4

Macroscope's review found this PR approvable — This is a focused client-side fix that stabilizes the existing merge control and shows a confirmed merged state. Its finite per-PR state tracking and presentation logic are tested, while merge RPC behavior, schemas, and deployment remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from fd4ffaa to 3c20061CompareAugust 12, 2026 22:10
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 12, 2026 22:10

Dismissing prior approval to re-evaluate 3c20061

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 3c20061 to 592e3b9CompareAugust 14, 2026 01:13
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 14, 2026 01:13

Dismissing prior approval to re-evaluate 592e3b9

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 14, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 592e3b9 to 23f892fCompareAugust 15, 2026 18:56

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 15, 2026 19:31

Dismissing prior approval to re-evaluate 9fdf68d

@github-actionsgithub-actionsBot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the extended pending window introduced for merge. Everything else in the diff (per-pull-request keying of pendingAction, identity-guarded clears, the activePendingAction label read) is consistent with the panel's existing "only the pressed control speaks" contract.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the merge-completion suppression: the header's primary control is unmounted rather than shown in a resolved state, and the suppression only lifts on a non-open detail, so a host that keeps reporting the pull request as open leaves the slot permanently empty.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 15, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from e3d9363 to c2a2401CompareAugust 19, 2026 06:45
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 19, 2026 06:45

Dismissing prior approval to re-evaluate c2a2401

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 19, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from c2a2401 to 022268eCompareAugust 21, 2026 00:59
@maria-rcks

Copy link
Copy Markdown
Collaborator

pls never record the videos like that, NEVER; they make it hard to see what actually changed

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks yeah sorry you're definitely right, I'll put new ones in a sec

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks fixed, should be pretty clear now. Again, sorry. Got carried away with the Cap default edits on the vid.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 022268e to 237e0deCompareAugust 23, 2026 09:57
@github-actionsgithub-actionsBot added size:XXL 1,000+ changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). size:XXL 1,000+ changed lines (additions + deletions). labels Sep 1, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the new merged-button tone. The lifecycle rework itself reads consistently with the panel's contracts: the hold is per-pull-request, only the Merge control is suppressed while it is held, and detailQuery.isPending is the waiting flag of the AsyncResult (true on a refresh() refetch that already has data), so the post-merge read does arm and then clear the bound.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@Bil0000

Copy link
Copy Markdown
Contributor

@flamboh would be cool if you update your vid/img in the desc too 🫡

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 working on that already!

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reworked hold itself checks out: useEnvironmentQuery.isPending maps to AsyncResult.waiting, so the refreshDetail() a merge takes does flip it true and then false, and observePullRequestDetail bounds the hold on that read regardless of which refresh path ran — the earlier unbounded-hold and per-PR-overwrite findings are addressed. Two findings on how the completed state is presented once it clears, inline.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 is it intended that the "Merged" button is only shown briefly?
image
I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

@Bil0000

Copy link
Copy Markdown
Contributor

@Bil0000 is it intended that the "Merged" button is only shown briefly? image I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

Change it however you like, & add some after media, will take a look then & let you know my thoughts ;)

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@flambohflamboh changed the title fix(web): keep merge pending until refreshfix(web): stablize merge button with merged chipSep 1, 2026
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 see new after video, macroscope approved as well!

@maria-rcks

Copy link
Copy Markdown
Collaborator

closed because of #9188 (sorry)

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks LOL no it's all good. Glad it got fixed!!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flamboh@maria-rcks@Bil0000
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(web): stablize merge button with merged chip - #6271

Closed
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state
Closed

fix(web): stablize merge button with merged chip#6271
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state

Conversation

@flamboh

@flambohflamboh commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5 on behalf of Oliver

What Changed

The Merge button now stays pending until refreshed PR data confirms the merge, then becomes a solid violet "Merged" marker that persists on every merged PR, so opening one shows the outcome where the action used to be (thanks @Bil0000!). The merge success toast only fires as a fallback on hosts that confirm merges asynchronously (e.g. Azure DevOps), where the marker has to yield before the host catches up.

The action lifecycle lives in packages/client-runtime/src/state/pullRequestActionState.ts with transitions tracked by per-PR atoms.

Why

The merge command finished before the detail refresh, so stale open-state data briefly made the Merge button clickable again.

UI Changes

Before

prb_before.mp4

After

prb_after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Built by GPT-5.6-sol (Codex CLI) and Claude Fable 5 (Claude Code) in T3 Code, with the merged-button presentation contributed by @Bil0000.

Note

Stabilize PR merge button with merged chip via shared action state

  • Adds a per-pull-request action state atom family in pullRequestActionState.ts that tracks pending action and a merge hold, reconciled by observePullRequestDetail from incoming detail reads.
  • Introduces usePullRequestActionState and pullRequestMergeButtonPresentation so PullRequestDetailPanel.tsx shows a disabled, violet-toned "Merged" chip during merge hold or once the host reports merged, and "Merging..." only while the merge is in-flight.
  • Defers the merge success toast until the merge hold clears for the same PR; non-merge action toasts stay immediate.
  • Risk: merge hold clears only when observePullRequestDetail sees detail state leave open or settle open after a pending read; if the host never delivers such a read, the hold persists — verify the reconciliation logic in observePullRequestDetail.

Macroscope summarized 4ae02b4.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0d1a64f8-9abb-4f83-9678-afda7307e213

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 12, 2026
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@macroscopeapp

macroscopeappBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4ae02b4

Macroscope's review found this PR approvable — This is a focused client-side fix that stabilizes the existing merge control and shows a confirmed merged state. Its finite per-PR state tracking and presentation logic are tested, while merge RPC behavior, schemas, and deployment remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from fd4ffaa to 3c20061CompareAugust 12, 2026 22:10
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 12, 2026 22:10

Dismissing prior approval to re-evaluate 3c20061

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 3c20061 to 592e3b9CompareAugust 14, 2026 01:13
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 14, 2026 01:13

Dismissing prior approval to re-evaluate 592e3b9

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 14, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 592e3b9 to 23f892fCompareAugust 15, 2026 18:56

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 15, 2026 19:31

Dismissing prior approval to re-evaluate 9fdf68d

@github-actionsgithub-actionsBot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the extended pending window introduced for merge. Everything else in the diff (per-pull-request keying of pendingAction, identity-guarded clears, the activePendingAction label read) is consistent with the panel's existing "only the pressed control speaks" contract.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the merge-completion suppression: the header's primary control is unmounted rather than shown in a resolved state, and the suppression only lifts on a non-open detail, so a host that keeps reporting the pull request as open leaves the slot permanently empty.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 15, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from e3d9363 to c2a2401CompareAugust 19, 2026 06:45
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 19, 2026 06:45

Dismissing prior approval to re-evaluate c2a2401

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 19, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from c2a2401 to 022268eCompareAugust 21, 2026 00:59
@maria-rcks

Copy link
Copy Markdown
Collaborator

pls never record the videos like that, NEVER; they make it hard to see what actually changed

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks yeah sorry you're definitely right, I'll put new ones in a sec

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks fixed, should be pretty clear now. Again, sorry. Got carried away with the Cap default edits on the vid.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 022268e to 237e0deCompareAugust 23, 2026 09:57
@github-actionsgithub-actionsBot added size:XXL 1,000+ changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). size:XXL 1,000+ changed lines (additions + deletions). labels Sep 1, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the new merged-button tone. The lifecycle rework itself reads consistently with the panel's contracts: the hold is per-pull-request, only the Merge control is suppressed while it is held, and detailQuery.isPending is the waiting flag of the AsyncResult (true on a refresh() refetch that already has data), so the post-merge read does arm and then clear the bound.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@Bil0000

Copy link
Copy Markdown
Contributor

@flamboh would be cool if you update your vid/img in the desc too 🫡

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 working on that already!

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reworked hold itself checks out: useEnvironmentQuery.isPending maps to AsyncResult.waiting, so the refreshDetail() a merge takes does flip it true and then false, and observePullRequestDetail bounds the hold on that read regardless of which refresh path ran — the earlier unbounded-hold and per-PR-overwrite findings are addressed. Two findings on how the completed state is presented once it clears, inline.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 is it intended that the "Merged" button is only shown briefly?
image
I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

@Bil0000

Copy link
Copy Markdown
Contributor

@Bil0000 is it intended that the "Merged" button is only shown briefly? image I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

Change it however you like, & add some after media, will take a look then & let you know my thoughts ;)

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@flambohflamboh changed the title fix(web): keep merge pending until refreshfix(web): stablize merge button with merged chipSep 1, 2026
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 see new after video, macroscope approved as well!

@maria-rcks

Copy link
Copy Markdown
Collaborator

closed because of #9188 (sorry)

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks LOL no it's all good. Glad it got fixed!!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flamboh@maria-rcks@Bil0000
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(web): stablize merge button with merged chip - #6271

Closed
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state
Closed

fix(web): stablize merge button with merged chip#6271
flamboh wants to merge 14 commits into
pingdotgg:mainfrom
flamboh:t3code/fix-merge-button-completion-state

Conversation

@flamboh

@flambohflamboh commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5 on behalf of Oliver

What Changed

The Merge button now stays pending until refreshed PR data confirms the merge, then becomes a solid violet "Merged" marker that persists on every merged PR, so opening one shows the outcome where the action used to be (thanks @Bil0000!). The merge success toast only fires as a fallback on hosts that confirm merges asynchronously (e.g. Azure DevOps), where the marker has to yield before the host catches up.

The action lifecycle lives in packages/client-runtime/src/state/pullRequestActionState.ts with transitions tracked by per-PR atoms.

Why

The merge command finished before the detail refresh, so stale open-state data briefly made the Merge button clickable again.

UI Changes

Before

prb_before.mp4

After

prb_after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Built by GPT-5.6-sol (Codex CLI) and Claude Fable 5 (Claude Code) in T3 Code, with the merged-button presentation contributed by @Bil0000.

Note

Stabilize PR merge button with merged chip via shared action state

  • Adds a per-pull-request action state atom family in pullRequestActionState.ts that tracks pending action and a merge hold, reconciled by observePullRequestDetail from incoming detail reads.
  • Introduces usePullRequestActionState and pullRequestMergeButtonPresentation so PullRequestDetailPanel.tsx shows a disabled, violet-toned "Merged" chip during merge hold or once the host reports merged, and "Merging..." only while the merge is in-flight.
  • Defers the merge success toast until the merge hold clears for the same PR; non-merge action toasts stay immediate.
  • Risk: merge hold clears only when observePullRequestDetail sees detail state leave open or settle open after a pending read; if the host never delivers such a read, the hold persists — verify the reconciliation logic in observePullRequestDetail.

Macroscope summarized 4ae02b4.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0d1a64f8-9abb-4f83-9678-afda7307e213

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 12, 2026
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@macroscopeapp

macroscopeappBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4ae02b4

Macroscope's review found this PR approvable — This is a focused client-side fix that stabilizes the existing merge control and shows a confirmed merged state. Its finite per-PR state tracking and presentation logic are tested, while merge RPC behavior, schemas, and deployment remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from fd4ffaa to 3c20061CompareAugust 12, 2026 22:10
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 12, 2026 22:10

Dismissing prior approval to re-evaluate 3c20061

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 12, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 3c20061 to 592e3b9CompareAugust 14, 2026 01:13
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 14, 2026 01:13

Dismissing prior approval to re-evaluate 592e3b9

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 14, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 592e3b9 to 23f892fCompareAugust 15, 2026 18:56

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings on the new pending lifecycle. The state-scoping and the awaiting-detail effect themselves look sound: pendingAction is keyed by pullRequestKey like the other panel scopes, each predicate is false in the state its action is offered from, and the manual Refresh menu item stays enabled, so the pending window cannot be wedged with no way out.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 15, 2026 19:31

Dismissing prior approval to re-evaluate 9fdf68d

@github-actionsgithub-actionsBot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the extended pending window introduced for merge. Everything else in the diff (per-pull-request keying of pendingAction, identity-guarded clears, the activePendingAction label read) is consistent with the panel's existing "only the pressed control speaks" contract.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Aug 15, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the merge-completion suppression: the header's primary control is unmounted rather than shown in a resolved state, and the suppression only lifts on a non-open detail, so a host that keeps reporting the pull request as open leaves the slot permanently empty.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 15, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from e3d9363 to c2a2401CompareAugust 19, 2026 06:45
@macroscopeapp
macroscopeappBot dismissed their stale reviewAugust 19, 2026 06:45

Dismissing prior approval to re-evaluate c2a2401

macroscopeapp[bot]
macroscopeappBot previously approved these changes Aug 19, 2026
@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from c2a2401 to 022268eCompareAugust 21, 2026 00:59
@maria-rcks

Copy link
Copy Markdown
Collaborator

pls never record the videos like that, NEVER; they make it hard to see what actually changed

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks yeah sorry you're definitely right, I'll put new ones in a sec

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks fixed, should be pretty clear now. Again, sorry. Got carried away with the Cap default edits on the vid.

@flamboh
flambohforce-pushed the t3code/fix-merge-button-completion-state branch from 022268e to 237e0deCompareAugust 23, 2026 09:57
@github-actionsgithub-actionsBot added size:XXL 1,000+ changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). size:XXL 1,000+ changed lines (additions + deletions). labels Sep 1, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the new merged-button tone. The lifecycle rework itself reads consistently with the panel's contracts: the hold is per-pull-request, only the Merge control is suppressed while it is held, and detailQuery.isPending is the waiting flag of the AsyncResult (true on a refresh() refetch that already has data), so the post-merge read does arm and then clear the bound.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@Bil0000

Copy link
Copy Markdown
Contributor

@flamboh would be cool if you update your vid/img in the desc too 🫡

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 working on that already!

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reworked hold itself checks out: useEnvironmentQuery.isPending maps to AsyncResult.waiting, so the refreshDetail() a merge takes does flip it true and then false, and observePullRequestDetail bounds the hold on that read regardless of which refresh path ran — the earlier unbounded-hold and per-PR-overwrite findings are addressed. Two findings on how the completed state is presented once it clears, inline.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
Comment threadapps/web/src/components/pullRequest/PullRequestDetailPanel.tsx Outdated
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 is it intended that the "Merged" button is only shown briefly?
image
I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

@Bil0000

Copy link
Copy Markdown
Contributor

@Bil0000 is it intended that the "Merged" button is only shown briefly? image I'd expect something similar to the above, so there's a bit more clarity that the PR is merged. Right now the only part of the UI showcasing the successful merge is the small merged icon, and the lack of the button to merge.

Change it however you like, & add some after media, will take a look then & let you know my thoughts ;)

Comment threadapps/web/src/components/pullRequest/pullRequestDetail.logic.ts Outdated
@flambohflamboh changed the title fix(web): keep merge pending until refreshfix(web): stablize merge button with merged chipSep 1, 2026
@flamboh

Copy link
Copy Markdown
ContributorAuthor

@Bil0000 see new after video, macroscope approved as well!

@maria-rcks

Copy link
Copy Markdown
Collaborator

closed because of #9188 (sorry)

@flamboh

Copy link
Copy Markdown
ContributorAuthor

@maria-rcks LOL no it's all good. Glad it got fixed!!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flamboh@maria-rcks@Bil0000