Enable Create PR for pushed branches that are ahead of main#2081
Enable Create PR for pushed branches that are ahead of main#2081notkainoa wants to merge 8 commits intopingdotgg:mainfrom
Conversation
- Track `aheadOfBaseCount` through git status contracts and clients - Let Create PR use remote branch divergence instead of only local ahead count - Preserve push behavior for branches with unpushed commits
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR changes runtime behavior for when the 'Create PR' button appears, modifying UI logic conditions and priority ordering. Unresolved review comments identify a potential bug (missing default branch guard) and a performance issue (duplicate git process calls) that need attention. You can customize Macroscope's approvability policy. Learn more. |
Inspired by upstream t3code PRs: - pingdotgg/t3code#1 (resolveModelSlug-style normalization) - pingdotgg/t3code#2092 (code block copy for touch / position / clipboard fallback) - pingdotgg/t3code#2057 (completion chime when agent turn settles) - pingdotgg/t3code#2081 (surface ahead-of-base for clean pushed branches on mobile create PR) Adds resolveModelSlug with optional provider hint, chat code copy controls and non-secure clipboard fallback, optional Web Audio completion sound with settings, and commitsAheadOfBase on mobile PR create eligibility with iOS subtitle. Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
…gs (#163) * feat: port t3code UX — model slug, code copy, sounds, PR ahead hint Inspired by upstream t3code PRs: - pingdotgg/t3code#1 (resolveModelSlug-style normalization) - pingdotgg/t3code#2092 (code block copy for touch / position / clipboard fallback) - pingdotgg/t3code#2057 (completion chime when agent turn settles) - pingdotgg/t3code#2081 (surface ahead-of-base for clean pushed branches on mobile create PR) Adds resolveModelSlug with optional provider hint, chat code copy controls and non-secure clipboard fallback, optional Web Audio completion sound with settings, and commitsAheadOfBase on mobile PR create eligibility with iOS subtitle. Co-authored-by: Arul Sharma <arul28@users.noreply.github.com> * fix: banner dismiss controls, resilient completion sound, tests - Dismiss missing-AI and GitHub setup banners per session (inspired by pingdotgg/t3code#773) - Resume suspended AudioContext, defer AudioContext.close, use global setTimeout - Add vitest coverage for sound helper and banner dismiss; extend appStore prefs test Co-authored-by: Arul Sharma <arul28@users.noreply.github.com> * feat(settings): appearance tab with chat font preview (t3 #2174) - New Appearance settings: theme swatches, chat font size 12–24px with live ChatMarkdown preview, copy-button position + completion sound (moved from General) - chatFontSizePx in appStore; work chat scales via ChatSurfaceShell zoom - resolveModelSlug: exact getModelById before provider-hint lowercasing (review) - iOS Create PR lane subtitle when commitsAheadOfBase is 0 - A11y: label range input; fix preview sample template literal Co-authored-by: Arul Sharma <arul28@users.noreply.github.com> * fix: remove upstream PR link from UI; cross-browser chat scaling - Appearance: drop user-visible t3code/GitHub link; keep neutral copy - ChatSurfaceShell: scale header/body/footer via transform + inverse dimensions (Firefox-safe) instead of CSS zoom; contentScale prop - AgentChatPane: pass contentScale from chat font preference - agentTurnCompletionSound: module + function docstrings (CodeRabbit hint) - Add ChatSurfaceShell scale wrapper tests with cleanup between cases Co-authored-by: Arul Sharma <arul28@users.noreply.github.com> * fix: address CI failures and review feedback on PR branch Repair grepSearch JS-fallback tests by routing ripgrep exec through a test hook; relax TerminalView WebGL expectation for headless CI; play completion sound only on successful turns; preserve case-sensitive model refs with provider hints; improve Appearance preview layout, a11y, and clipboard cleanup; keep banner dismissals across project switches; make iOS commitsAheadOfBase backward compatible and show lane subtitles. Co-authored-by: Arul Sharma <arul28@users.noreply.github.com> * fix: safer repo-wide grep fallback, chat markdown code blocks, copy polish Skip hidden root-only dirs (e.g. .ade) in JS grep when searching the whole repo while still entering .github; add regression test. Route ChatMarkdown fenced blocks through HighlightedCode so copy placement applies in previews. Stub WebGL canvas in TerminalView tests instead of weakening assertions. Tighten Appearance section labels and helper copy to sentence case. Co-authored-by: Arul Sharma <arul28@users.noreply.github.com> * Normalize config paths, runpage & chat UI Normalize project config paths and improve run-page, chat UI, and related behavior. Summary of changes: - Project config: add path normalization helpers (normalizeConfigPath, projectRelativePath, normalizeProjectCwd/Command) and apply them when validating, saving, and returning snapshots so absolute project-root paths are converted to portable relative paths. Persist normalized shared/local in snapshots. - Tests: add unit test to verify project-root absolute paths are normalized and that saved ade.yaml contains portable paths. - RunPage refactor: introduce helpers to convert absolute paths to project-relative, build/update process config definitions, handle local vs shared placement (localOnly), and add upsert/remove helpers for processes and stack buttons. Adjust save logic and dependencies. - Grep search: surface descriptive "Invalid regex pattern" errors from JS fallback and improve glob handling to match bare filenames for patterns like **/*.ts; add tests for invalid regex and glob edge cases. - IPC: catch errors from resolveLaneOverlayContext during lane delete, log a warning, and continue. - App shell & store: move per-session banner-dismiss state into app store (avoid local-state leaks) and use store dismiss actions. - Chat & code UI: switch many inline styles to CSS variables, improve code copy button (auto/sticky mode), better diff coloring via styles, and style enhancements for code blocks, inline code, and tables. Wire code-copy position from store. - Agent sounds & settings: add volume and quiet-when-focused preferences, pass options to playAgentTurnCompletionSound, and update AppearanceSection UI (chat font-size swatches, sound controls, terminal options import). - Misc: make .ade/ade.yaml use project-relative script paths and update finalize.md to add a Mobile Parity agent checklist and instructions. Why: make config files portable across machines, provide clearer errors and more ergonomic UI/UX for chat/code blocks, and ensure consistent handling of local vs shared config changes when editing processes from the Run page. * Improve glob/path handling and small UI/accessibility fixes Enhance grep glob matching and path normalization, add tests, and apply minor UI/accessibility tweaks. - grepSearch: Normalize backslashes in file globs, detect when a glob includes directory components, and match against relative file paths when appropriate. Rewrote globToRegex to correctly handle **/, **, *, ?, and {a,b} patterns while preserving directory semantics. Added tests to exercise directory globs and JS fallback behavior. - projectConfigService: Add utilities to detect absolute paths across platforms, infer project-relative paths from foreign-platform absolute paths (e.g. Windows paths on POSIX), and return portable relative paths for config saving. Added a test to ensure foreign absolute process paths are normalized to portable relative paths in saved config. - CodeHighlighter: Adjust copy button rendering so the auto position renders consistently. - AppearanceSection: Add aria-pressed to theme swatch button and an aria-label to the custom terminal font input to improve accessibility. These changes improve cross-platform behavior for globs and config paths and address minor UX/accessibility issues. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
…into t3code/enable-create-pr-button
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bce2d06. Configure here.
| ); | ||
| } | ||
|
|
||
| const fallbackAheadCount = !upstreamRef && refName ? aheadOfBaseCount : null; |
There was a problem hiding this comment.
Duplicate git process spawn for same computation
Medium Severity
computeAheadCountAgainstBase(cwd, refName) is called unconditionally at line 1250 to compute aheadOfBaseCount, but the aheadOfDefaultCount fallback at line 1270 calls the same function with the same arguments again when fallbackAheadCount === null (i.e., when there IS an upstream). This spawns redundant git processes (resolveBaseBranchForNoUpstream + rev-list --count) on every status poll for non-default branches with an upstream — the exact scenario this PR targets. The value at line 1270 could simply reuse aheadOfBaseCount.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bce2d06. Configure here.
| !hasChanges && | ||
| !hasOpenPr && | ||
| hasDefaultBranchDelta && | ||
| isAheadOfBase && |
There was a problem hiding this comment.
Missing default branch guard in menu items
Low Severity
canCreatePr in buildMenuItems uses isAheadOfBase without checking !gitStatus.isDefaultRef. The old hasDefaultBranchDelta was implicitly zero on the default branch because aheadOfDefaultCount was only computed for non-default branches. The new aheadOfBaseCount is computed unconditionally and can be positive on the default branch if an alternative base branch candidate (like "master") exists, incorrectly enabling the "Create PR" menu item on the default branch.
Reviewed by Cursor Bugbot for commit bce2d06. Configure here.
|
Already doable |
1 similar comment
|
Already doable |
2 participants
What Changed
Makes Create PR button available when a branch has already been pushed and is ahead of the base branch, even if there are no local changes left to push.
Why
The button was incorrectly disabled for branches that were clean locally but still had changes on remote, which made the flow feel blocked and confusing.
Before
As you can see the "create pr" button is disabled even though this branch is ahead by 1 commit (the commit is already pushed to remote though).
After
Note
Enable Create PR for pushed branches ahead of main using
aheadOfBaseCountaheadOfBaseCount(commits ahead of the default/base branch) toGitStatusDetails,VcsStatusRemoteShape, and all downstream status objects throughout the stack.GitVcsDriverCorecomputesaheadOfBaseCountviacomputeAheadCountAgainstBasewhenever arefNameis present, independent of upstream tracking configuration.GitActionsControllogic now gates the Create PR quick action and menu item onaheadOfBaseCount > 0instead of the previousaheadOfDefaultCount ?? aheadCountfallback, so pushed branches without an upstream are correctly detected as ahead of base.VcsStatusRemoteShapenow requiresaheadOfBaseCountas aNonNegativeIntfield — any client or test constructing this shape must include the new field.Macroscope summarized bce2d06.
Note
Medium Risk
Extends the VCS status contract and recomputes branch-ahead metrics, which can affect git status behavior and UI action enablement across the app. Changes are localized but touch shared contracts and git parsing logic used broadly.
Overview
Enables PR creation for branches that are clean locally but still ahead of the base branch. Adds
aheadOfBaseCountto the sharedVcsStatusRemoteResult/VcsStatusResultcontract and plumbs it through server git status responses (including non-repo defaults).GitVcsDriverCorenow computesaheadOfBaseCountfor any checked-out ref (even with an upstream), using it as the fallbackaheadCountwhen no upstream is configured. The webGitActionsControllogic switches PR eligibility and disabled-copy fromaheadCount/aheadOfDefaultCounttoaheadOfBaseCount, so pushed-but-ahead branches show Create PR instead of “no action.”Test harness improvements add
preferRealGitOnPathForTests()and update many fixtures to include the new field; one web env test also increases its timeout.Reviewed by Cursor Bugbot for commit bce2d06. Bugbot is set up for automated code reviews on this repo. Configure here.