Skip to content

Fix three ide browse follow-ups: cmux routing, duplicate GitHub link, dead dock icon#59

Merged
GordonBeeming merged 6 commits into
mainfrom
gb/browse-followup-fixes
Jul 13, 2026
Merged

Fix three ide browse follow-ups: cmux routing, duplicate GitHub link, dead dock icon#59
GordonBeeming merged 6 commits into
mainfrom
gb/browse-followup-fixes

Conversation

@GordonBeeming

@GordonBeeming GordonBeeming commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

Three bugs the ide browse feature (v0.11) surfaced, each with a confirmed root cause.

  • ide browse opened the system browser instead of cmux's embedded one. The generated CLI wrapper ended with open "$browse_url" >/dev/null 2>&1 & then exit 0. Under cmux, open is a shim that hands the URL to a cmux browser open child; backgrounding it and exiting immediately orphans that child, which loses its pane context so the shim falls back to the system browser. Foregrounding the open keeps the wrapper alive through the handoff. An orphan-vs-foreground probe confirmed it: the backgrounded form falls to the system browser, the foreground form routes to cmux.
  • Duplicate "Open in GitHub" button. The commit panel built one action per GitHub remote, so a repo with two GitHub remotes got two identical buttons, and the second linked a repo that doesn't contain the commit. It now resolves the branch's upstream remote (falling back to origin, then the first) and shows a single action.
  • Dock icon did nothing after a browse launch. A browse launch hides the main window, and the app had no RunEvent::Reopen handler, so clicking the dock icon reactivated an app with no visible window. A macOS Reopen handler now re-shows and focuses the main window when nothing is visible.

Test plan

  • From a cmux pane, ide browse . opens a cmux embedded tab. The orphan probe shows the foreground form routing to cmux and the backgrounded form falling to the system browser.
  • New pick_primary_template unit test: two GitHub remotes resolve to one action for the tracking remote, with origin and first-remote fallbacks.
  • Green locally: cargo test, cargo fmt --check, vitest (411), and both launcher-validation scripts.
  • Dock reopen after a browse launch re-shows the main window (behavioural, worth a click-through in the running app).

Summary by CodeRabbit

  • New Features

    • Reopening the app on macOS now restores and focuses the main window when it is hidden.
    • Git commit attribution links now consistently use the most relevant repository remote.
  • Bug Fixes

    • “Browse” now stays connected to the active pane/session by keeping the browser-launch wrapper alive.
    • Improved behavior when browsing from repositories with multiple configured remotes.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR keeps the ide browse wrapper in the foreground, restores hidden macOS main windows on reopen, and selects a single preferred GitHub remote for attribution links.

Changes

Browse flow

Layer / File(s) Summary
Foreground browse launcher validation
scripts/install-cli-command.sh, scripts/validate-launch-runners.mjs
The generated browse command now runs open without backgrounding it, and validation checks the updated command form.
macOS reopen handling
src-tauri/src/lib.rs
macOS Reopen events show and focus the main window when no visible windows are present.

Git attribution

Layer / File(s) Summary
Primary remote selection
src-tauri/src/git_attribution.rs
Attribution selects the upstream remote first, then origin, then the first available remote; unit tests cover these cases, including remote names containing slashes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the three main ide browse fixes: cmux routing, GitHub link selection, and macOS reopen handling.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch gb/browse-followup-fixes

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

@GordonBeeming

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@GordonBeeming

Copy link
Copy Markdown
Owner Author

/gemini review

@GordonBeeming

Copy link
Copy Markdown
Owner Author

@codex review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces several key improvements. First, it updates the CLI installation script to run the browser-opening command in the foreground, preventing orphaned child processes under cmux. Second, it refactors the Git attribution logic to select a single primary remote (preferring the tracking remote, then origin), avoiding duplicate or incorrect 'Open in GitHub' links. Finally, it adds macOS-specific handling to re-show and focus the main window when the application is reopened with no visible windows. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Copilot AI 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.

Pull request overview

Fixes three ide browse (v0.11) follow-ups by ensuring browse launches route correctly under cmux, preventing duplicate/incorrect “Open in GitHub” actions, and restoring expected macOS dock-icon reopen behavior after a headless browse launch.

Changes:

  • macOS: handle RunEvent::Reopen to re-show/focus the hidden main window when no windows are visible.
  • Git attribution: collapse multiple GitHub remotes down to a single primary remote based on upstream tracking (then origin, then fallback).
  • CLI wrapper: run open "$browse_url" in the foreground (and update validation) to preserve cmux pane context.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src-tauri/src/lib.rs Adds a macOS Reopen handler to re-show/focus the main window after browse launches hide it.
src-tauri/src/git_attribution.rs Chooses a single primary GitHub remote template (tracking → origin → first) and adds unit coverage.
scripts/validate-launch-runners.mjs Updates assertions to match the new foreground open "$browse_url" behavior.
scripts/install-cli-command.sh Stops backgrounding open "$browse_url" so cmux routing doesn’t fall back to the system browser.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src-tauri/src/lib.rs

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request prevents orphaned browser processes by running the open command in the foreground, and updates the validation tests accordingly. It also refactors Git attribution to prioritize a single primary remote (upstream tracking branch, then 'origin') to avoid duplicate buttons, and adds macOS-specific logic to restore the main window on reopen events. Feedback suggests using the existing focus_window helper in src-tauri/src/lib.rs to simplify window restoration and handle unminimizing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src-tauri/src/lib.rs

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c5f24f913e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src-tauri/src/git_attribution.rs Outdated
Comment thread scripts/validate-launch-runners.mjs
focus_window also restores a minimized window, unlike the raw show+set_focus pair.

@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: 2

🧹 Nitpick comments (1)
src-tauri/src/git_attribution.rs (1)

644-681: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover real upstream resolution in the tests.

This test supplies a synthetic tracking_remote, so it will pass even if tracking_remote_name fails against actual Git configuration. Add a repository-backed case with branch.main.remote = upstream and multiple GitHub remotes. Also, the “unknown upstream” case currently passes None; use Some("nope") to test that fallback explicitly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/src/git_attribution.rs` around lines 644 - 681, Extend
pick_primary_template tests with a repository-backed case configuring
branch.main.remote = upstream and multiple GitHub remotes, then verify the
resolved upstream is preferred. Update the unknown-upstream scenario in
pick_primary_template_prefers_tracking_then_origin to pass Some("nope") instead
of None, preserving its origin fallback assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/validate-launch-runners.mjs`:
- Line 115: Update the assertion for ideCommand in the launcher validation to
require the exact foreground open command and reject the same command when
followed by a background operator. Replace the broad substring check with an
assertion that distinguishes the intended command boundary while preserving the
existing redirection validation.

In `@src-tauri/src/git_attribution.rs`:
- Around line 494-515: Update tracking_remote_name to derive the upstream remote
with repo.branch_remote_name using the branch reference, instead of parsing the
tracking ref returned by branch_remote_tracking_ref_name. Preserve the existing
Option-based failure behavior and return the configured remote name directly.

---

Nitpick comments:
In `@src-tauri/src/git_attribution.rs`:
- Around line 644-681: Extend pick_primary_template tests with a
repository-backed case configuring branch.main.remote = upstream and multiple
GitHub remotes, then verify the resolved upstream is preferred. Update the
unknown-upstream scenario in pick_primary_template_prefers_tracking_then_origin
to pass Some("nope") instead of None, preserving its origin fallback assertion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 54b30cb2-a0bf-4b20-ba03-6130245ffd6f

📥 Commits

Reviewing files that changed from the base of the PR and between a802180 and c5f24f9.

📒 Files selected for processing (4)
  • scripts/install-cli-command.sh
  • scripts/validate-launch-runners.mjs
  • src-tauri/src/git_attribution.rs
  • src-tauri/src/lib.rs

Comment thread scripts/validate-launch-runners.mjs
Comment thread src-tauri/src/git_attribution.rs
A remote name containing a slash (e.g. team/fork) made refs/remotes/<remote>/<branch>
ambiguous to split on '/'. Read branch.<name>.remote via gix's branch_remote_name
instead, and add a regression test for a slashed remote name.
The prior assertion only checked a substring, so it still passed if the
launcher regressed to 'open "$browse_url" >/dev/null 2>&1 &' — exactly the
cmux-orphaning form this PR fixes.
@GordonBeeming

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@GordonBeeming

Copy link
Copy Markdown
Owner Author

/gemini review

@GordonBeeming

Copy link
Copy Markdown
Owner Author

@codex review

@GordonBeeming GordonBeeming requested a review from Copilot July 13, 2026 06:55
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces several improvements across the codebase. In scripts/install-cli-command.sh, the open command is now run in the foreground to prevent orphaned child processes under cmux, with corresponding updates made to the validation script. In src-tauri/src/git_attribution.rs, logic was added to resolve the branch's upstream remote or fall back to origin to avoid duplicate or incorrect GitHub links when multiple remotes exist. Finally, in src-tauri/src/lib.rs, a macOS-specific handler was added to re-show and focus the main window when the application is reopened without any visible windows. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@GordonBeeming GordonBeeming marked this pull request as ready for review July 13, 2026 07:00
@GordonBeeming GordonBeeming merged commit ca696e3 into main Jul 13, 2026
4 of 6 checks passed
@GordonBeeming GordonBeeming deleted the gb/browse-followup-fixes branch July 13, 2026 07:00
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.

2 participants