Skip to content

Add dynamic PR toast actions to Git actions flow - #41

Closed
juliusmarminge wants to merge 1 commit into
mainfrom
codething/3a47e5fc
Closed

Add dynamic PR toast actions to Git actions flow#41
juliusmarminge wants to merge 1 commit into
mainfrom
codething/3a47e5fc

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • refactored Git action execution into a shared executeGitAction path for both modal and immediate actions
  • added dynamic progress toast CTA logic via getGitProgressToastAction to show either Open PR or Create PR based on post-run state
  • extracted reusable PR-eligibility checks (canCreatePrFromStatus) and reused them in menu-item gating
  • removed the separate immediate-action mutation and aligned run-state handling with the modal action runner
  • added unit coverage for toast-action selection behavior in apps/web/src/git-actions-control.test.ts

Testing

  • Added unit tests for getGitProgressToastAction covering:
  • returns Open PR when openPrUrl exists
  • returns Create PR after commit_push when branch status is PR-eligible
  • returns null for non-push actions
  • returns null while running or when an error is present
  • Lint: Not run
  • Full app/integration tests: Not run

Open with Devin

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced Git workflow with improved multi-step progress tracking for commits, pushes, and pull requests
    • Added toast notifications with quick actions to open existing pull requests or create new ones
    • Improved pull request eligibility detection and handling
  • Tests

    • Added comprehensive unit tests for Git action workflows

- Refactor Git action execution into a shared flow for modal and quick actions
- Show `Open PR` when URL exists, otherwise `Create PR` after `commit_push` when eligible
- Add unit tests for `getGitProgressToastAction` decision logic
@coderabbitai

coderabbitai Bot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Refactors Git action control flow in the web component by introducing a generalized executeGitAction function that orchestrates commit, push, and PR steps with progress tracking. Adds new getGitProgressToastAction function to determine toast actions based on run state and PR eligibility, and includes comprehensive unit tests.

Changes

Cohort / File(s) Summary
Git Actions Control Component
apps/web/src/components/GitActionsControl.tsx
Introduces new executeGitAction function orchestrating multi-step Git operations (commit, push, PR) with progress updates and error handling. Adds getGitProgressToastAction function to determine toast action type (open/create PR). Refactors PR eligibility logic with canCreatePrFromStatus helper. Updates Git progress initialization with optional generate step flag. Removes previous mutation-based immediate action path and adjusts UI wiring for new action flow.
Git Actions Control Tests
apps/web/src/git-actions-control.test.ts
New test file covering getGitProgressToastAction function with test cases for PR URL availability, push eligibility, running/error states, and non-push actions. Includes helper builders for mocking Git status and action results.

Sequence Diagram(s)

sequenceDiagram
    participant User as User Action
    participant Component as GitActionsControl Component
    participant Executor as executeGitAction
    participant Git as Git Operations
    participant Toast as Progress Toast

    User->>Component: Trigger runGitActionImmediately()
    Component->>Executor: executeGitAction() with steps
    Executor->>Toast: Show progress (starting)
    
    Executor->>Git: Commit changes
    Git-->>Executor: Commit result
    Executor->>Toast: Update progress
    
    Executor->>Git: Push to remote
    Git-->>Executor: Push result
    Executor->>Toast: Update progress
    
    Executor->>Git: Create/Open PR
    Git-->>Executor: PR result
    Executor->>Toast: Update final status
    
    Toast->>Component: getGitProgressToastAction()
    Component-->>Toast: Determine action (open_pr/create_pr/null)
    Toast-->>User: Display with action
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add dynamic PR toast actions to Git actions flow' accurately summarizes the main change: introducing dynamic toast CTA logic to show contextual PR actions.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/3a47e5fc

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

@macroscopeapp

macroscopeapp Bot commented Feb 14, 2026

Copy link
Copy Markdown
Contributor

Add dynamic progress toast actions to Git actions flow and update GitActionsControl in GitActionsControl.tsx

Introduce GitProgressToastAction for toast buttons, add getGitProgressToastAction and canCreatePrFromStatus helpers, refactor GitActionsControl to use a unified executeGitAction, and adjust initialGitProgressSteps to take a boolean. Tests cover toast action selection in git-actions-control.test.ts.

📍Where to Start

Start with executeGitAction and toast action calculation in GitActionsControl.tsx.


Macroscope summarized 84ee10d.

@greptile-apps

greptile-apps Bot commented Feb 14, 2026

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Refactored Git action execution to use a unified executeGitAction function that replaces the previous mutation-based approach for immediate actions. The key improvements include:

  • Extracted canCreatePrFromStatus helper for reusable PR-eligibility checks
  • Added getGitProgressToastAction to dynamically determine toast button actions based on post-run state (Open PR vs Create PR)
  • Unified executeGitAction handles both modal and immediate actions with configurable modal closing
  • Removed runImmediateGitActionMutation mutation and replaced with direct executeGitAction calls
  • Changed initialGitProgressSteps parameter from commitMessage string to includeGenerateStep boolean for clarity
  • Removed "Running..." text from Git actions button to keep it static
  • Removed unnecessary void from onClick handler since runGitAction already handles the promise

The refactor improves code organization by consolidating Git action logic into a single execution path while maintaining all existing functionality.

Confidence Score: 4/5

  • Safe to merge with minor considerations around edge case handling
  • The refactoring consolidates Git action execution into a cleaner architecture with proper test coverage. The logic is sound and well-tested, though there's a subtle behavioral difference in the canCreatePrFromStatus check that now prevents PR creation when there are working tree changes, which may or may not be intentional.
  • No files require special attention

Important Files Changed

Filename Overview
apps/web/src/components/GitActionsControl.tsx Refactored Git action execution into unified executeGitAction path with dynamic toast actions. Removed mutation-based immediate actions in favor of shared execution flow.
apps/web/src/git-actions-control.test.ts Added comprehensive unit tests for getGitProgressToastAction covering all branches including open PR, create PR, and null return conditions.

Flowchart

flowchart TD
    Start[User triggers Git action] --> CheckModal{Modal or<br/>Immediate?}
    
    CheckModal -->|Modal| RunGitAction[runGitAction<br/>with user inputs]
    CheckModal -->|Immediate| RunImmediate[runGitActionImmediately<br/>no commit message]
    
    RunGitAction --> ExecuteGitAction[executeGitAction]
    RunImmediate --> ExecuteGitAction
    
    ExecuteGitAction --> CloseModal{closeModal<br/>param?}
    CloseModal -->|true| CloseIt[Close modal]
    CloseModal -->|false| ShowToast[Show progress toast]
    CloseIt --> ShowToast
    
    ShowToast --> RunCommit[Run commit action]
    RunCommit --> CheckAction{Action type?}
    
    CheckAction -->|commit| Done[Complete]
    CheckAction -->|commit_push| RunPush[Run push action]
    CheckAction -->|commit_push_pr| RunPush
    
    RunPush --> CheckPR{commit_push_pr?}
    CheckPR -->|yes| RunPRAction[Run PR action]
    CheckPR -->|no| RefreshStatus[Refresh git status]
    RunPRAction --> RefreshStatus
    
    RefreshStatus --> ComputeToastAction[getGitProgressToastAction]
    ComputeToastAction --> CheckToastResult{Toast action?}
    
    CheckToastResult -->|openPrUrl exists| ShowOpenPR[Show 'Open PR' button]
    CheckToastResult -->|commit_push + eligible| ShowCreatePR[Show 'Create PR' button]
    CheckToastResult -->|otherwise| NoButton[No button shown]
    
    ShowOpenPR --> Done
    ShowCreatePR --> Done
    NoButton --> Done
Loading

Last reviewed commit: 84ee10d

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@apps/web/src/components/GitActionsControl.tsx`:
- Around line 875-889: Replace the button's disabled prop with an
accessibility-preserving pattern: remove disabled={isGitModalActionRunning}, add
aria-disabled={isGitModalActionRunning} and keep the visual disabled styling
(opacity/disabled classes); guard the onClick handler so it returns early when
isGitModalActionRunning is true (i.e., do nothing if isGitModalActionRunning)
before calling openPrFromToast() or runGitActionImmediately("commit_push_pr");
also ensure keyboard activation is blocked when aria-disabled is true by
checking isGitModalActionRunning at the top of any click/key handlers related to
the GitActionsControl button using the existing symbols isGitModalActionRunning,
gitProgressToastAction, openPrFromToast, and runGitActionImmediately.
🧹 Nitpick comments (1)
apps/web/src/components/GitActionsControl.tsx (1)

422-576: Consider swapping manual running state for useTransition.
This keeps pending UI state aligned with project guidance; if you still need explicit async lifecycle tracking, add a short comment explaining why.

Based on learnings: Use useTransition for async actions instead of manually managing pending state. Replace manual pending state management with startTransition callback pattern.

Also applies to: 844-849

Comment on lines +875 to 889
{!isGitModalActionRunning && gitProgressToastAction && (
<button
type="button"
className="rounded-md bg-foreground px-2.5 py-1 text-xs font-medium text-background transition-colors duration-150 hover:bg-foreground/90 disabled:cursor-not-allowed disabled:opacity-60"
onClick={openPrFromToast}
onClick={() => {
if (gitProgressToastAction.kind === "open_pr") {
openPrFromToast();
return;
}
runGitActionImmediately("commit_push_pr");
}}
disabled={isGitModalActionRunning}
>
Open PR
{gitProgressToastAction.label}
</button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid disabled on the toast action button.
Use aria-disabled + click-guard to preserve accessibility/tooltips while still preventing action.

🔧 Suggested adjustment
-              <button
+              <button
                 type="button"
-                className="rounded-md bg-foreground px-2.5 py-1 text-xs font-medium text-background transition-colors duration-150 hover:bg-foreground/90 disabled:cursor-not-allowed disabled:opacity-60"
+                className={`rounded-md bg-foreground px-2.5 py-1 text-xs font-medium text-background transition-colors duration-150 hover:bg-foreground/90 ${
+                  isGitModalActionRunning ? "cursor-not-allowed opacity-60" : ""
+                }`}
                 onClick={() => {
+                  if (isGitModalActionRunning) return;
                   if (gitProgressToastAction.kind === "open_pr") {
                     openPrFromToast();
                     return;
                   }
                   runGitActionImmediately("commit_push_pr");
                 }}
-                disabled={isGitModalActionRunning}
+                aria-disabled={isGitModalActionRunning}
               >
Based on learnings: Avoid using disabled props on buttons as they harm accessibility and break tooltips. Instead, use styling (e.g., opacity, hover states) and handle the disabled state through click handlers.
🤖 Prompt for AI Agents
In `@apps/web/src/components/GitActionsControl.tsx` around lines 875 - 889,
Replace the button's disabled prop with an accessibility-preserving pattern:
remove disabled={isGitModalActionRunning}, add
aria-disabled={isGitModalActionRunning} and keep the visual disabled styling
(opacity/disabled classes); guard the onClick handler so it returns early when
isGitModalActionRunning is true (i.e., do nothing if isGitModalActionRunning)
before calling openPrFromToast() or runGitActionImmediately("commit_push_pr");
also ensure keyboard activation is blocked when aria-disabled is true by
checking isGitModalActionRunning at the top of any click/key handlers related to
the GitActionsControl button using the existing symbols isGitModalActionRunning,
gitProgressToastAction, openPrFromToast, and runGitActionImmediately.

aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
…gdotgg#41)

* feat: add a split-screen-horizontally toggle for the right panel

Adds a third toggle beside the terminal-drawer and right-panel buttons
that moves the right panel under the chat column instead of beside it,
splitting the screen into top and bottom the way the terminal drawer
already does.

Splitting is a placement change rather than an open/close one, so every
press has to leave a panel on screen: pressing from a closed panel opens
it at the bottom, and pressing while maximized un-maximizes first, since
maximizing hides the chat entirely — the opposite of a split.

The panel gains a row-resize handle on its top edge. Height persists
under its own key, separate from width, so flipping orientation restores
the size that orientation last had. Orientation itself is a per-browser
preference like the panel's width, not thread state.

Which surface owns the window's top-right follows the panel: its tab bar
when it sits beside the chat, the chat header when it is split off the
bottom or closed.

Supporting changes:
- useResizableWidth -> useResizableSize, now driving height as well via
  an "top" edge, with the pointer math extracted as a pure, tested
  function.
- ChatHeader's rightPanelOpen prop -> hostsLayoutControls, which is all
  it ever meant; the split makes the old name wrong.
- New rebindable command rightPanel.toggleHorizontalSplit, mod+alt+0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(fork): record the split-screen delta, and give the inventory an upstream to check against

The split-screen toggle adds a delta to ten upstream files, so it needs a
Fork Inventory row, a delta section, Path Policy rows and a convergence
watch entry. Eight of the ten were byte-identical to upstream before it.

The fork owns no file in that row except one: useResizableWidth.ts is
renamed to useResizableSize.ts, so upstream's edits to the hook will
arrive as a modify/delete conflict rather than a mergeable hunk. Recorded
rather than reversed, with the cheaper shape written beside it.

Two corrections found while verifying ownership against upstream, which
a sandbox can now do — the fork's clone is shallow and has no upstream
remote, but adding pingdotgg/t3code and fetching --depth=1 takes seconds,
so both commands go in Path Policy:

- .plans/** was marked ours while upstream owns 32 of its 35 files. That
  blanket ours would have discarded every upstream plan edit, which is
  the failure the paragraph above the table describes. Row split.
- messageOrigin.ts is confirmed absent upstream, clearing the caveat the
  previous log entry left on it. The other twelve ours paths verify
  clean too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(web): keep the split-screen delta additive, per AGENTS.md

AGENTS.md asks for fork changes that upstream merges cheaply: put the code
on a path upstream does not have, reach it from upstream files through the
smallest hook, and prefer a branch above upstream's to a rewrite of it.
Three parts of this branch did the opposite.

useResizableWidth.ts was renamed to useResizableSize.ts, which deletes a
path upstream owns and edits — every future upstream change to that hook
would have landed as a modify/delete conflict. Upstream's hook is restored
byte for byte, and the vertical axis moves to a fork-only sibling,
apps/web/src/fork/useResizablePanelHeight.ts, which also owns the max-height
helper and its tests.

The orientation preference was appended to upstream's rightPanelLayout.ts;
it moves to apps/web/src/fork/rightPanelOrientation.ts, leaving that file
byte-identical to upstream again.

ChatHeader's rightPanelOpen prop was renamed to hostsLayoutControls. It
keeps upstream's name and meaning now, with an optional
layoutControlsOverHeader beside it for the case the split introduces.

PreviewPanelShell keeps every upstream line as upstream wrote it: the
stacked classes are appended after upstream's own and resolved by twMerge,
and the width path is untouched. Net effect on merges: eight upstream
files with additive hunks, three fork-only modules, no renamed or deleted
upstream paths. Behaviour is unchanged.

Verification scoped to the change, per AGENTS.md: web typecheck, lint on
the touched files, fmt, and vp test run over apps/web/src/fork,
apps/web/src/components/preview and rightPanelStore — 24 files, 128 tests.

Model: Claude Opus 5. Harness: Claude Code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: fork code lives where the code it works with lives

apps/web/src/fork/ was reading as a ghetto for anything fork-authored,
which makes the tree look like a fork rather than like the app it is.
AGENTS.md said to put fork code there; it now says to write a new file and
put that file where it belongs, named for what it does. A path upstream
does not have merges for free wherever it sits, so the merge argument was
never a reason for an odd home. fork/ keeps only what is about being a
fork: the surface-gating registry and the mobile modules.

Two rules go with it. Mark fork code where someone will look — a line in
the module's doc comment, a short // Fork: on a hunk inside an upstream
file — so a merge can see what it is holding without diffing against
upstream. And never rename, move or delete an upstream file for a fork
reason, which is the lesson the useResizableWidth rename in this branch
taught.

The split-screen modules move accordingly: rightPanelOrientation.ts next
to upstream's rightPanelLayout.ts, useResizablePanelHeight.ts next to
upstream's useResizableWidth.ts. Imports and the fork inventory follow.
Behaviour is unchanged.

Verification: web typecheck, lint on the touched files, fmt --check, and
vp test run over apps/web/src/hooks and apps/web/src/components/preview —
25 files, 87 tests.

Model: Claude Opus 5. Harness: Claude Code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: trim the fork-change rules back to what they need to say

The placement rule arrived with two extra bullets and a paragraph of
justification aimed at the mistake that prompted it. Folded back into the
three bullets that were already there: where a new file goes and how it
says it is ours, the additive hook, and upstream's file keeping its name
and shape. The inventory and the log entry lose the same padding.

Model: Claude Opus 5. Harness: Claude Code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: fork code says so in the file

Dropped the placement bullet from AGENTS.md — where a file sits is a
judgement call, not a rule worth spending lines on. What replaces it is
the part that pays off at merge time: a fork-only module says it is ours
in its doc comment, and a fork hunk inside an upstream file carries a
short // Fork: above it, so a merge can see what it is holding without
diffing against upstream first.

Applied to this branch's own hunks: ChatView's two split derivations,
RightPanelTabs' orientation prop, PanelLayoutControls' third toggle, and
the two keybindings entries. The two fork-only modules already said it.

Model: Claude Opus 5. Harness: Claude Code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"

This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.

Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.

Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.

Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.

Model: Claude Opus 5. Harness: Claude Code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* feat(web): drop the terminal-drawer toggle from the chat header

The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.

Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.

Model: Claude Opus 5. Harness: Claude Code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(fork): the log keeps what is known, not what was undone

The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.

Model: Claude Opus 5. Harness: Claude Code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant