Skip to content

[codex] fix git remote parsing, PR resolution, and sidebar PR state#1619

Merged
juliusmarminge merged 19 commits intomainfrom
t3code/gitcore-race-fix
Apr 1, 2026
Merged

[codex] fix git remote parsing, PR resolution, and sidebar PR state#1619
juliusmarminge merged 19 commits intomainfrom
t3code/gitcore-race-fix

Conversation

@juliusmarminge
Copy link
Copy Markdown
Member

@juliusmarminge juliusmarminge commented Apr 1, 2026

What changed

This expands the original upstream parsing fix into a broader cleanup of how git remotes, upstream refs, and PR heads are resolved across the app.

The backend now uses shared remote-ref parsing in remoteRefs.ts so GitCore and GitManager agree on how to interpret upstream refs, including remotes whose names contain slashes and remotes where one name is a prefix of another.

It also fixes PR lookup/creation to avoid treating synthetic local aliases like upstream/effect-atom as GitHub PR head selectors, while still preferring owner-qualified selectors for cross-repo PRs.

On the UI side, stacked git actions now sync the thread's stored branch metadata after creating a branch, so the sidebar PR badge and the git action menu stay aligned.

The git tests were expanded and stabilized to cover:

  • lazy upstream refresh after checkout
  • slash-containing remote names in status refreshes and pushes
  • prefix-overlapping remote names
  • slash-remote PR lookup for same-repo branches
  • cross-repo PR selector ordering and post-create rediscovery
  • sidebar branch metadata sync logic after branch creation

Why

A tracked upstream like my-org/upstream/feature was previously vulnerable to being split at the wrong /, and prefix-overlapping remote names could also be misparsed depending on which parser path was used.

That inconsistency leaked into PR resolution: synthetic local branch aliases could be queried as GitHub --head selectors, which explains missing PR labels in the sidebar and cases where Show PR opened an unrelated PR.

Separately, the sidebar relies on stored thread branch metadata, and branch-creating git actions were not updating that metadata immediately, so the sidebar and action menu could disagree even when the backend had the right PR.

Impact

  • upstream refs for slash-containing and prefix-overlapping remotes now resolve consistently across GitCore and GitManager
  • status refresh, push, and PR base/head resolution now use the correct remote/branch pairing
  • cross-repo PR lookup and creation now prefer the right owner-qualified selectors without drifting to unrelated PRs
  • sidebar PR badges stay in sync with the git action menu after branch-creating actions
  • slash-remote and cross-repo tests are more stable across Git versions and selector ordering changes

Validation

  • bun run test -- --run src/git/Layers/GitCore.test.ts -t "statusDetails remains successful when upstream refresh fails after checkout"
  • bun run test -- --run src/git/Layers/GitCore.test.ts -t "defers upstream refresh until statusDetails is requested"
  • bun run test -- --run src/git/Layers/GitCore.test.ts -t "checks out a remote tracking branch when remote name contains slashes"
  • bun run test -- --run src/git/Layers/GitCore.test.ts -t "pushes to the tracked upstream when the remote name contains slashes"
  • bun run test -- --run src/git/Layers/GitManager.test.ts -t "status ignores synthetic local branch aliases when the upstream remote name contains slashes"
  • bun run test -- --run src/git/Layers/GitManager.test.ts -t "returns the correct existing PR when a slash remote checks out to a synthetic local alias"
  • bun run test -- --run src/git/Layers/GitManager.test.ts -t "returns existing cross-repo PR metadata using the fork owner selector"
  • bun run test -- --run src/git/Layers/GitManager.test.ts -t "prefers owner-qualified selectors before bare branch names for cross-repo PRs"
  • bun run test -- --run src/git/Layers/GitManager.test.ts -t "creates cross-repo PRs with the fork owner selector and default base branch"
  • bun run test -- --run src/components/GitActionsControl.logic.test.ts -t "resolveThreadBranchUpdate"
  • bun fmt
  • bun lint
  • bun typecheck

Note

Fix git remote parsing, PR resolution, and sidebar PR state for remotes with slashes

  • Adds push and create_pr as explicit GitStackedAction values, separating push-only and PR-creation flows from the combined commit_push/commit_push_pr actions throughout the client and server.
  • Fixes upstream ref parsing and PR lookup when remote names contain slashes by using known remote names (via parseRemoteRefWithRemoteNames) instead of splitting at the first slash.
  • Keys upstream refresh caching by git common directory instead of working directory, so sibling worktrees share a single fetch with a 5-second failure cooldown.
  • Adds server-authored GitRunStackedActionToast to GitRunStackedActionResult, providing a structured title, description, and context-sensitive CTA (Push, Create PR, or View PR) after each action.
  • Sidebar new-thread creation now inherits branch, worktree path, and env mode from the active thread via resolveSidebarNewThreadSeedContext.
  • GitManager.status results are cached for 1 second per cwd to reduce redundant PR lookups; the cache is invalidated after preparePullRequestThread and runStackedAction.
  • Risk: GitRunStackedActionResult now requires a toast field — any existing producers that omit it will fail schema validation.

Macroscope summarized c0c8c5c.


Note

Medium Risk
Touches core git workflow orchestration and upstream/PR resolution (including new action types and shared caches), which can affect push/PR creation behavior across repos and worktrees. Changes are well-covered by expanded tests but still impact critical developer workflows.

Overview
Fixes git remote/upstream parsing and status refresh behavior, especially for remotes whose names contain slashes or overlap by prefix, by centralizing remote-ref parsing in remoteRefs.ts and updating GitCore to key upstream-refresh caching by --git-common-dir (with a short failure cooldown) and to defer upstream refresh until statusDetails is called.

Refactors GitManager PR discovery/creation to request richer gh pr list metadata and filter matches by head repo/owner + cross-repo state, avoiding synthetic local alias branches being treated as GitHub --head selectors; also adds a 1s in-memory cache for status() results.

Expands the stacked git action API to include explicit push/create_pr actions and returns a typed, server-authored toast (title/description/CTA) in GitRunStackedActionResult; the web UI consumes this toast, adjusts progress stage logic, and syncs thread branch metadata after branch-creating actions and when live status diverges. Sidebar new-thread creation now seeds branch/worktree/env-mode from the active thread/draft when creating within the same project.

Written by Cursor Bugbot for commit c0c8c5c. This will update automatically on new commits. Configure here.

Co-authored-by: codex <codex@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

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: Pro

Run ID: 7d0fbb00-6c9f-4d45-9e89-ad724924b7c9

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/gitcore-race-fix

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

@github-actions github-actions bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Apr 1, 2026
@juliusmarminge juliusmarminge marked this pull request as ready for review April 1, 2026 16:16
Co-authored-by: codex <codex@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Wrong remote name ordering causes ambiguous upstream misparsing
    • Removed .toReversed() from listRemoteNames so it returns remotes in longest-first order (matching parseRemoteNames), ensuring parseUpstreamRefWithRemoteNames correctly disambiguates prefix-overlapping remote names.

Create PR

Or push these changes by commenting:

@cursor push e51e1f5e74
Preview (e51e1f5e74)
diff --git a/apps/server/src/git/Layers/GitCore.ts b/apps/server/src/git/Layers/GitCore.ts
--- a/apps/server/src/git/Layers/GitCore.ts
+++ b/apps/server/src/git/Layers/GitCore.ts
@@ -927,7 +927,7 @@
 
   const listRemoteNames = (cwd: string): Effect.Effect<ReadonlyArray<string>, GitCommandError> =>
     runGitStdout("GitCore.listRemoteNames", cwd, ["remote"]).pipe(
-      Effect.map((stdout) => parseRemoteNames(stdout).toReversed()),
+      Effect.map((stdout) => parseRemoteNames(stdout)),
     );
 
   const resolvePrimaryRemoteName = Effect.fn("resolvePrimaryRemoteName")(function* (cwd: string) {

You can send follow-ups to this agent here.

juliusmarminge and others added 2 commits April 1, 2026 09:28
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp bot commented Apr 1, 2026

Approvability

Verdict: Needs human review

This PR introduces significant runtime behavior changes: new git action types ('push', 'create_pr'), modified upstream refresh timing and caching, changed PR resolution logic for fork handling, and server-side toast generation. While framed as fixes, the scope touches core git workflows and warrants human review.

You can customize Macroscope's approvability policy. Learn more.

juliusmarminge and others added 2 commits April 1, 2026 10:29
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@github-actions github-actions bot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Apr 1, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
macroscopeapp[bot]
macroscopeapp bot previously approved these changes Apr 1, 2026
@juliusmarminge juliusmarminge changed the title [codex] fix git upstream parsing and refresh timing [codex] fix git remote parsing, PR resolution, and sidebar PR state Apr 1, 2026
@macroscopeapp macroscopeapp bot dismissed their stale review April 1, 2026 18:34

Dismissing prior approval to re-evaluate 8e60ea7

- Include head repo metadata in `gh pr list`
- Ignore unrelated fork PRs with matching branch names
- Generate completion toasts from server-side git results
@github-actions github-actions bot added size:XL 500-999 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Apr 1, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Toast building adds redundant GitHub API calls to every action
    • Moved explicitResultPr computation before the findLatestPr call and skip the expensive GitHub API lookup when the PR URL is already known from the action result, limiting findLatestPr to bare commit_push actions only.
  • ✅ Fixed: Duplicate head-repository resolver functions with identical logic
    • Removed the duplicate resolvePullRequestHeadRepositoryNameWithOwner function and unified its logic into resolveHeadRepositoryNameWithOwner with signature PullRequestHeadRemoteInfo & { url: string }, updating all call sites.

Create PR

Or push these changes by commenting:

@cursor push 4fd9bc98f1
Preview (4fd9bc98f1)
diff --git a/apps/server/src/git/Layers/GitManager.ts b/apps/server/src/git/Layers/GitManager.ts
--- a/apps/server/src/git/Layers/GitManager.ts
+++ b/apps/server/src/git/Layers/GitManager.ts
@@ -82,20 +82,20 @@
 }
 
 function resolveHeadRepositoryNameWithOwner(
-  pullRequest: ResolvedPullRequest & PullRequestHeadRemoteInfo,
+  pr: PullRequestHeadRemoteInfo & { url: string },
 ): string | null {
-  const explicitRepository = pullRequest.headRepositoryNameWithOwner?.trim() ?? "";
-  if (explicitRepository.length > 0) {
+  const explicitRepository = normalizeOptionalString(pr.headRepositoryNameWithOwner);
+  if (explicitRepository) {
     return explicitRepository;
   }
 
-  if (!pullRequest.isCrossRepository) {
+  if (!pr.isCrossRepository) {
     return null;
   }
 
-  const ownerLogin = pullRequest.headRepositoryOwnerLogin?.trim() ?? "";
-  const repositoryName = parseRepositoryNameFromPullRequestUrl(pullRequest.url);
-  if (ownerLogin.length === 0 || !repositoryName) {
+  const ownerLogin = normalizeOptionalString(pr.headRepositoryOwnerLogin);
+  const repositoryName = parseRepositoryNameFromPullRequestUrl(pr.url);
+  if (!ownerLogin || !repositoryName) {
     return null;
   }
 
@@ -153,27 +153,6 @@
   return normalized ? normalized.toLowerCase() : null;
 }
 
-function resolvePullRequestHeadRepositoryNameWithOwner(
-  pr: PullRequestHeadRemoteInfo & { url: string },
-) {
-  const explicitRepository = normalizeOptionalString(pr.headRepositoryNameWithOwner);
-  if (explicitRepository) {
-    return explicitRepository;
-  }
-
-  if (!pr.isCrossRepository) {
-    return null;
-  }
-
-  const ownerLogin = normalizeOptionalString(pr.headRepositoryOwnerLogin);
-  const repositoryName = parseRepositoryNameFromPullRequestUrl(pr.url);
-  if (!ownerLogin || !repositoryName) {
-    return null;
-  }
-
-  return `${ownerLogin}/${repositoryName}`;
-}
-
 function matchesBranchHeadContext(
   pr: PullRequestInfo,
   headContext: Pick<
@@ -192,7 +171,7 @@
     normalizeOptionalOwnerLogin(headContext.headRepositoryOwnerLogin) ??
     parseRepositoryOwnerLogin(expectedHeadRepository);
   const prHeadRepository = normalizeOptionalRepositoryNameWithOwner(
-    resolvePullRequestHeadRepositoryNameWithOwner(pr),
+    resolveHeadRepositoryNameWithOwner(pr),
   );
   const prHeadOwner =
     normalizeOptionalOwnerLogin(pr.headRepositoryOwnerLogin) ??
@@ -857,10 +836,18 @@
     result: Pick<GitRunStackedActionResult, "action" | "branch" | "commit" | "push" | "pr">,
   ) {
     const summary = summarizeGitActionResult(result);
-    let latestOpenPr: PullRequestInfo | null = null;
     let currentBranchIsDefault = false;
 
-    if (result.action !== "commit") {
+    const explicitResultPr =
+      (result.pr.status === "created" || result.pr.status === "opened_existing") && result.pr.url
+        ? {
+            url: result.pr.url,
+            state: "open" as const,
+          }
+        : null;
+
+    let latestOpenPr: PullRequestInfo | null = null;
+    if (result.action !== "commit" && !explicitResultPr) {
       const finalStatus = yield* gitCore.statusDetails(cwd);
       if (finalStatus.branch) {
         latestOpenPr = yield* findLatestPr(cwd, {
@@ -878,14 +865,7 @@
       }
     }
 
-    const explicitResultPr =
-      (result.pr.status === "created" || result.pr.status === "opened_existing") && result.pr.url
-        ? {
-            url: result.pr.url,
-            state: "open" as const,
-          }
-        : null;
-    const openPr = latestOpenPr ?? explicitResultPr;
+    const openPr = explicitResultPr ?? latestOpenPr;
 
     const cta =
       result.action === "commit" && result.commit.status === "created"

You can send follow-ups to this agent here.

}

return `${ownerLogin}/${repositoryName}`;
}
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.

Duplicate head-repository resolver functions with identical logic

Low Severity

The new resolvePullRequestHeadRepositoryNameWithOwner duplicates the existing resolveHeadRepositoryNameWithOwner. Both check for an explicit headRepositoryNameWithOwner, fall back to constructing one from headRepositoryOwnerLogin and the PR URL when isCrossRepository is true, and return null otherwise. They differ only in input type and string normalization style (normalizeOptionalString vs ?.trim() ?? ""). A single generic implementation accepting PullRequestHeadRemoteInfo & { url: string } would cover both call sites and avoid the risk of the two copies drifting.

Additional Locations (1)
Fix in Cursor Fix in Web

- Detect PR-ready branches before running stacked actions
- Emit only PR progress and thread prOnlyIfReady through UI and contracts
- Reuse branch head context when a pushed branch may already have an open PR
- Add regression coverage for the extra `gh pr list` call
- Co-authored-by: codex <codex@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Redundant re-parsing of already-typed PR objects
    • Replaced the redundant parsePullRequestList call in findOpenPr with a new lightweight pullRequestSummaryToInfo function that directly maps already-validated GitHubPullRequestSummary objects to PullRequestInfo without re-doing type checks.

Create PR

Or push these changes by commenting:

@cursor push 3dbd6d7f9b
Preview (3dbd6d7f9b)
diff --git a/apps/server/src/git/Layers/GitManager.ts b/apps/server/src/git/Layers/GitManager.ts
--- a/apps/server/src/git/Layers/GitManager.ts
+++ b/apps/server/src/git/Layers/GitManager.ts
@@ -22,7 +22,7 @@
   type GitRunStackedActionOptions,
 } from "../Services/GitManager.ts";
 import { GitCore } from "../Services/GitCore.ts";
-import { GitHubCli } from "../Services/GitHubCli.ts";
+import { GitHubCli, type GitHubPullRequestSummary } from "../Services/GitHubCli.ts";
 import { TextGeneration } from "../Services/TextGeneration.ts";
 import { extractBranchNameFromRemoteRef } from "../remoteRefs.ts";
 import { ServerSettingsService } from "../../serverSettings.ts";
@@ -305,6 +305,25 @@
   return parsed;
 }
 
+function pullRequestSummaryToInfo(pr: GitHubPullRequestSummary): PullRequestInfo {
+  return {
+    number: pr.number,
+    title: pr.title,
+    url: pr.url,
+    baseRefName: pr.baseRefName,
+    headRefName: pr.headRefName,
+    state: pr.state ?? "open",
+    updatedAt: null,
+    ...(pr.isCrossRepository != null ? { isCrossRepository: pr.isCrossRepository } : {}),
+    ...(pr.headRepositoryNameWithOwner
+      ? { headRepositoryNameWithOwner: pr.headRepositoryNameWithOwner }
+      : {}),
+    ...(pr.headRepositoryOwnerLogin
+      ? { headRepositoryOwnerLogin: pr.headRepositoryOwnerLogin }
+      : {}),
+  };
+}
+
 function gitManagerError(operation: string, detail: string, cause?: unknown): GitManagerError {
   return new GitManagerError({
     operation,
@@ -767,21 +786,13 @@
         headSelector,
         limit: 1,
       });
-      const normalizedPullRequests = parsePullRequestList(pullRequests);
+      const normalizedPullRequests = pullRequests.map(pullRequestSummaryToInfo);
 
       const firstPullRequest = normalizedPullRequests.find((pullRequest) =>
         matchesBranchHeadContext(pullRequest, headContext),
       );
       if (firstPullRequest) {
-        return {
-          number: firstPullRequest.number,
-          title: firstPullRequest.title,
-          url: firstPullRequest.url,
-          baseRefName: firstPullRequest.baseRefName,
-          headRefName: firstPullRequest.headRefName,
-          state: "open",
-          updatedAt: null,
-        } satisfies PullRequestInfo;
+        return firstPullRequest;
       }
     }

You can send follow-ups to this agent here.

- Key status results by canonical cwd and invalidate on git actions
- Share upstream refreshes by git common dir with a brief failure cooldown
- Update tests for shared refresh and fetch argument handling
@github-actions github-actions bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:XL 500-999 changed lines (additions + deletions). labels Apr 1, 2026
- Map GitHub CLI PR summaries directly to `PullRequestInfo`
- Preserve cross-repo and head repository metadata
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Live branch sync clears thread metadata on detached HEAD
    • Added a guard !input.gitStatus.branch in resolveLiveThreadBranchUpdate to return null (skip sync) when in detached HEAD state, preventing the thread's branch metadata from being cleared during transient git states.

Create PR

Or push these changes by commenting:

@cursor push 29a02a8255
Preview (29a02a8255)
diff --git a/apps/web/src/components/GitActionsControl.logic.ts b/apps/web/src/components/GitActionsControl.logic.ts
--- a/apps/web/src/components/GitActionsControl.logic.ts
+++ b/apps/web/src/components/GitActionsControl.logic.ts
@@ -316,7 +316,7 @@
   threadBranch: string | null;
   gitStatus: GitStatusResult | null;
 }): { branch: string | null } | null {
-  if (!input.gitStatus) {
+  if (!input.gitStatus || !input.gitStatus.branch) {
     return null;
   }

You can send follow-ups to this agent here.

- Keep the existing thread branch when git status has no branch
- Add regression coverage for detached-HEAD branch updates
- Replace push/PR flags with explicit actions
- Update server, web, and contract schemas for clearer progress handling
- Co-authored-by: codex <codex@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Menu push on default branch skips confirmation dialog
    • Extended requiresDefaultBranchConfirmation and the action guard in runGitActionWithToast to also check for the new "push" and "create_pr" actions, restoring the default branch confirmation dialog for menu-triggered push and create PR actions.

Create PR

Or push these changes by commenting:

@cursor push c155060325
Preview (c155060325)
diff --git a/apps/web/src/components/GitActionsControl.logic.test.ts b/apps/web/src/components/GitActionsControl.logic.test.ts
--- a/apps/web/src/components/GitActionsControl.logic.test.ts
+++ b/apps/web/src/components/GitActionsControl.logic.test.ts
@@ -802,9 +802,12 @@
 describe("requiresDefaultBranchConfirmation", () => {
   it("requires confirmation for push actions on default branch", () => {
     assert.isFalse(requiresDefaultBranchConfirmation("commit", true));
+    assert.isTrue(requiresDefaultBranchConfirmation("push", true));
+    assert.isTrue(requiresDefaultBranchConfirmation("create_pr", true));
     assert.isTrue(requiresDefaultBranchConfirmation("commit_push", true));
     assert.isTrue(requiresDefaultBranchConfirmation("commit_push_pr", true));
     assert.isFalse(requiresDefaultBranchConfirmation("commit_push", false));
+    assert.isFalse(requiresDefaultBranchConfirmation("push", false));
   });
 });
 

diff --git a/apps/web/src/components/GitActionsControl.logic.ts b/apps/web/src/components/GitActionsControl.logic.ts
--- a/apps/web/src/components/GitActionsControl.logic.ts
+++ b/apps/web/src/components/GitActionsControl.logic.ts
@@ -31,7 +31,11 @@
   continueLabel: string;
 }
 
-export type DefaultBranchConfirmableAction = "commit_push" | "commit_push_pr";
+export type DefaultBranchConfirmableAction =
+  | "push"
+  | "create_pr"
+  | "commit_push"
+  | "commit_push_pr";
 
 export function buildGitActionProgressStages(input: {
   action: GitStackedAction;
@@ -272,7 +276,12 @@
   isDefaultBranch: boolean,
 ): boolean {
   if (!isDefaultBranch) return false;
-  return action === "commit_push" || action === "commit_push_pr";
+  return (
+    action === "push" ||
+    action === "create_pr" ||
+    action === "commit_push" ||
+    action === "commit_push_pr"
+  );
 }
 
 export function resolveDefaultBranchActionDialogCopy(input: {
@@ -283,7 +292,7 @@
   const branchLabel = input.branchName;
   const suffix = ` on "${branchLabel}". You can continue on this branch or create a feature branch and run the same action there.`;
 
-  if (input.action === "commit_push") {
+  if (input.action === "push" || input.action === "commit_push") {
     if (input.includesCommit) {
       return {
         title: "Commit & push to default branch?",

diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx
--- a/apps/web/src/components/GitActionsControl.tsx
+++ b/apps/web/src/components/GitActionsControl.tsx
@@ -425,7 +425,12 @@
         requiresDefaultBranchConfirmation(action, actionIsDefaultBranch) &&
         actionBranch
       ) {
-        if (action !== "commit_push" && action !== "commit_push_pr") {
+        if (
+          action !== "push" &&
+          action !== "create_pr" &&
+          action !== "commit_push" &&
+          action !== "commit_push_pr"
+        ) {
           return;
         }
         setPendingDefaultBranchAction({

You can send follow-ups to this agent here.

@juliusmarminge juliusmarminge force-pushed the t3code/gitcore-race-fix branch from 49293ef to 5dc3c21 Compare April 1, 2026 22:50
@juliusmarminge
Copy link
Copy Markdown
Member Author

@cursor push c155060

cursoragent and others added 2 commits April 1, 2026 22:50
…tions

The menu's push and create_pr actions bypassed the default branch
confirmation dialog because requiresDefaultBranchConfirmation only
checked for commit_push and commit_push_pr. Extend the check to also
cover the split push and create_pr actions, and update the action guard
and dialog copy resolver to handle them.

Applied via @cursor push command
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.

🟢 Low

import { Textarea } from "~/components/ui/textarea";

buildGitActionProgressStages returns early for push and create_pr actions without including branchStages, so when featureBranch is true the progress UI omits the "Preparing feature branch..." stage. This occurs when a user on the default branch triggers push or create_pr and clicks "Checkout feature branch & continue" — the checkoutFeatureBranchAndContinuePendingAction callback passes featureBranch: true with the original action, but the branch preparation stage is never shown.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/GitActionsControl.tsx around line 40:

`buildGitActionProgressStages` returns early for `push` and `create_pr` actions without including `branchStages`, so when `featureBranch` is true the progress UI omits the "Preparing feature branch..." stage. This occurs when a user on the default branch triggers push or create_pr and clicks "Checkout feature branch & continue" — the `checkoutFeatureBranchAndContinuePendingAction` callback passes `featureBranch: true` with the original action, but the branch preparation stage is never shown.

Evidence trail:
apps/web/src/components/GitActionsControl.logic.ts lines 40-68: `buildGitActionProgressStages` function shows that `branchStages` is calculated on line 48 but the early returns for `push` (line 51-52) and `create_pr` (line 53-55) do not include it, while `commit`, `commit_push`, and the default case do include it.

apps/web/src/components/GitActionsControl.tsx lines 625-637: `checkoutFeatureBranchAndContinuePendingAction` passes `featureBranch: true` with the original action from `pendingDefaultBranchAction`.

apps/web/src/components/GitActionsControl.logic.ts lines 33-37: `DefaultBranchConfirmableAction` type includes `push` and `create_pr`.

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Non-commit actions report misleading commit step status
    • Added 'skipped_not_requested' variant to GitCommitStepStatus schema and used it in the non-commit action fallback path, replacing the semantically incorrect 'skipped_no_changes' for push and create_pr actions.

Create PR

Or push these changes by commenting:

@cursor push 33d6c3c46f
Preview (33d6c3c46f)
diff --git a/apps/server/src/git/Layers/GitManager.test.ts b/apps/server/src/git/Layers/GitManager.test.ts
--- a/apps/server/src/git/Layers/GitManager.test.ts
+++ b/apps/server/src/git/Layers/GitManager.test.ts
@@ -1388,7 +1388,7 @@
         action: "push",
       });
 
-      expect(result.commit.status).toBe("skipped_no_changes");
+      expect(result.commit.status).toBe("skipped_not_requested");
       expect(result.push.status).toBe("pushed");
       expect(result.pr.status).toBe("skipped_not_requested");
       expect(
@@ -1432,7 +1432,7 @@
         action: "create_pr",
       });
 
-      expect(result.commit.status).toBe("skipped_no_changes");
+      expect(result.commit.status).toBe("skipped_not_requested");
       expect(result.push.status).toBe("pushed");
       expect(result.push.setUpstream).toBe(true);
       expect(result.pr.status).toBe("created");
@@ -2727,7 +2727,7 @@
         },
       );
 
-      expect(result.commit.status).toBe("skipped_no_changes");
+      expect(result.commit.status).toBe("skipped_not_requested");
       expect(result.push.status).toBe("skipped_not_requested");
       expect(result.pr.status).toBe("created");
       expect(

diff --git a/apps/server/src/git/Layers/GitManager.ts b/apps/server/src/git/Layers/GitManager.ts
--- a/apps/server/src/git/Layers/GitManager.ts
+++ b/apps/server/src/git/Layers/GitManager.ts
@@ -1592,7 +1592,7 @@
                 ),
               ),
             )
-          : { status: "skipped_no_changes" as const };
+          : { status: "skipped_not_requested" as const };
 
         const push = wantsPush
           ? yield* progress

diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts
--- a/packages/contracts/src/git.ts
+++ b/packages/contracts/src/git.ts
@@ -27,7 +27,11 @@
 export type GitActionProgressKind = typeof GitActionProgressKind.Type;
 export const GitActionProgressStream = Schema.Literals(["stdout", "stderr"]);
 export type GitActionProgressStream = typeof GitActionProgressStream.Type;
-const GitCommitStepStatus = Schema.Literals(["created", "skipped_no_changes"]);
+const GitCommitStepStatus = Schema.Literals([
+  "created",
+  "skipped_no_changes",
+  "skipped_not_requested",
+]);
 const GitPushStepStatus = Schema.Literals([
   "pushed",
   "skipped_not_requested",

You can send follow-ups to this agent here.

@juliusmarminge
Copy link
Copy Markdown
Member Author

@cursor push 33d6c3c

…t actions

When the action is 'push' or 'create_pr', the commit step was hardcoded
to 'skipped_no_changes' which incorrectly implies the system tried to
commit but found nothing. Added 'skipped_not_requested' variant to
GitCommitStepStatus (matching the pattern used by push, PR, and branch
step schemas) and use it when the commit phase is not part of the action.

Applied via @cursor push command
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Quick action uses commit_push instead of push on default branch
    • Changed both code paths in resolveQuickAction (no-upstream and has-upstream isAhead blocks) to return action "push" instead of "commit_push" when isDefaultBranch is true and the working tree is clean, and updated corresponding test expectations.

Create PR

Or push these changes by commenting:

@cursor push c272562090

You can send follow-ups to this agent here.

disabled: false,
kind: "run_action",
action: isDefaultBranch ? "commit_push" : "push",
};
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.

Quick action uses commit_push instead of push on default branch

Low Severity

When isDefaultBranch is true and the working tree is clean (no changes to commit), resolveQuickAction returns action: "commit_push" instead of the new "push" action. Since isCommitAction("commit_push") returns true, the server runs a full commit resolution step — staging, reading diff context, potentially invoking the AI commit message generator — only to discover there are no changes and skip. The new "push" action was introduced precisely for this case and requiresDefaultBranchConfirmation already handles "push", so "commit_push" is no longer needed here.

Additional Locations (1)
Fix in Cursor Fix in Web

@juliusmarminge juliusmarminge merged commit 801dfe5 into main Apr 1, 2026
13 checks passed
@juliusmarminge juliusmarminge deleted the t3code/gitcore-race-fix branch April 1, 2026 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants