#26 feat(mcp,ui): add per-client global install status and update-safe MCP launcher

[Anonymous]

Originally created by: theBGuy
Originally owned by: theBGuy

This change adds per-client global install status for Claude Code and Copilot in Settings, letting users see, reinstall, or remove each client entry, and switches all MCP configuration and command-line entrypoints to use an update-safe gitdesktop-mcp launcher. The new launcher solves update-file-lock and upgrade-kill issues on Windows by running MCP from a managed copy outside the install directory, ensuring updates are always safe and live status is never stale.

MCP launcher: update safety and platform integration

• Adds src-tauri/src/mcp_launcher.rs providing a managed gitdesktop-mcp launcher copy, with atomic copy & version tracking, only on Windows release builds (env override for dev/tests).
• Updates src-tauri/src/lib.rs to refresh the managed launcher after each app update.
• All MCP config now refers to gitdesktop-mcp (not gitdesktop), switching config emission logic in all clusters.

Global install status and management

• Implements new API in src-tauri/src/mcp.rs:
• Adds mcp_global_status and mcp_global_remove Tauri commands to enumerate/read and remove per-client user-config MCP installs via the relevant CLI.
• Adds tolerant config parsing and normalization to robustly detect command paths and installation status.
• Frontend (src/features/settings/mcp/GitDesktopAsServer.tsx, src/lib/git/api.ts):
• Surfaces per-client global install status, showing live state, and new Reinstall/Remove controls.
• All config emits respect the new launcher and path/command conventions.
• Adds global install/remove logic for both Claude and Copilot.
• Documentation: Updates help and onboarding copy (README.md, src/features/help/content.ts) to refer to the new MCP launcher name, global install management flow, and launcher path expectations.

Command-line launcher and PATH handling

• Updates src-tauri/src/path_launcher.rs to operate on gitdesktop-mcp consistently:
• Windows: puts managed bin dir on the user PATH, automatically migrates old install-dir entries, and only removes what was previously created.
• macOS/Linux: symlinks gitdesktop-mcp into ~/.local/bin (auto-migrates previous gitdesktop symlinks).
• Frontend UI and logic to match (src/lib/git/api.ts, src/features/settings/mcp/GitDesktopAsServer.tsx): new launcher path accessors and copy/install flows, migrated everywhere relevant.

Changelog and support artifacts

• Adds changelog.d/added-mcp-global-install-status.md covering the new global install status view and management.
• Adds changelog.d/fixed-mcp-installer-file-lock.md documenting the fix for file locks and update safety.

Miscellaneous/internal

• Removes dead or obsolete code in src-tauri/src/fsops.rs (the previous app_exe_path).
• Refactors/exports constants in src-tauri/src/local_prs.rs as needed for cross-module coordination.

[Anonymous]

• assigned_to: theBGuy

[Anonymous]

Originally posted by: cloudflare-workers-and-pages[bot]

Deploying gitdesktop with    Cloudflare Pages

Latest commit:	970314e
Status:	✅  Deploy successful!
Preview URL:	https://95485184.gitdesktop.pages.dev
Branch Preview URL:	https://feat-mcp-global-install-stat.gitdesktop.pages.dev

View logs

[Anonymous]

Originally posted by: theBGuy

🤖 GitDesktop AI review · sonnet · automated

---

This PR replaces the direct-app-binary MCP invocation with a managed gitdesktop-mcp copy on Windows, adds per-client global install status rows (Claude Code / Copilot) with Reinstall/Remove controls, and migrates the PATH launcher to point at the new copy. The architecture is sound — the marker-based staleness check, atomic copy recipe, and zero-write status probe are all well-designed. A few real issues below; nothing is a hard blocker, but finding [#1] will silently break the documented Linux test path.

---

Correctness

should-fix — mcp_launcher.rs, copy_into_place: std::fs::copy on Linux does not preserve file permissions (mode bits). On macOS it does (via copyfile); on Linux the destination is created with umask-restricted bits (typically 0o644), so the managed copy is not executable. Management is inactive on Linux in production (Windows-only), but the GD_MCP_LAUNCHER_DIR env override — which the module's own doc calls "how dev/live validation exercises the machinery" — makes management active on any platform. A developer validating this on Linux would get a copy that ensure() reports as fresh, mcp_launcher_path returns as a valid path, but gitdesktop-mcp mcp … fails with EACCES.

Fix: set the execute bit after the temp-copy step, before the rename, on Unix:

#[cfg(unix)]
{
    use std::os::unix::fs::PermissionsExt;
    let meta = std::fs::metadata(&tmp).map_err(AppError::Io)?;
    let mut perms = meta.permissions();
    perms.set_mode(perms.mode() | 0o111);
    std::fs::set_permissions(&tmp, perms).map_err(AppError::Io)?;
}

---

should-fix — mcp.rs, norm_launcher_path: the function unconditionally applies .to_ascii_lowercase() and replaces / with \\, making classify_global_entry perform a case-insensitive path comparison on every platform. On macOS (case-insensitive FS) this is correct. On Linux (case-sensitive FS), two genuinely distinct paths like /home/Alice/bin/gitdesktop-mcp and /home/alice/bin/gitdesktop-mcp would incorrectly compare equal, producing a false current: true. Low probability in practice (the app writes the exact current_exe() path), but becomes relevant for any manually-authored config or multi-user scenario.

Fix: guard the lowercase behind the same platforms that actually have case-insensitive paths:

fn norm_launcher_path(p: &str) -> String {
    let s = p.trim().trim_end_matches(['\\', '/']).replace('/', "\\");
    #[cfg(any(windows, target_os = "macos"))]
    return s.to_ascii_lowercase();
    #[cfg(not(any(windows, target_os = "macos")))]
    return s;
}

---

Nits

nit — mcp.rs, mcp_global_remove: _app: tauri::AppHandle is declared in the Tauri command signature and prefixed with _ to suppress the unused warning, but it genuinely isn't used. Tauri only injects AppHandle when the handler declares it; dropping it shrinks the noise.

nit — GitDesktopAsServer.tsx, globalRow: key={client} on the outermost <div> returned from a plain function call has no effect — React key is only meaningful when elements are part of an array (.map()). Since globalRow("claude") and globalRow("copilot") are rendered as two distinct JSX children, not an array, React silently ignores the prop. Remove it.

---

Posted by GitDesktop — AI output, verify before acting on it.

[Anonymous]

Originally posted by: theBGuy

🤖 GitDesktop AI security audit · sonnet · automated

---

No security issues found in these changes.

The key risk surfaces were all guarded: mcp_global_remove's client parameter is validated against an explicit allowlist ("claude" | "copilot") before dispatching; run_client_cli uses Command::new() + .args() (never a shell) so the frontend-supplied command/args in mcp_global_install cannot cause command injection; all file-operation paths in mcp_launcher.rs derive exclusively from the bundle identifier and dirs::data_local_dir() (no user-controlled input); the Windows registry PATH write uses the same app-controlled path; read_global_config reads only two hardcoded home-relative paths with a 16 MB size guard; and the command field from ~/.claude.json surfaced by mcp_global_status is used only for a boolean path-normalization comparison server-side — it is never rendered into the DOM (the UI shows only the installed/current booleans), so React's default escaping is not even needed.

---

Copilot's four findings are all documentation/copy inconsistencies (stale command names, comment inaccuracy, outdated UI description) — none introduce exploitable vulnerabilities.

The prior reviewer's Linux-chmod finding (non-executable managed copy under GD_MCP_LAUNCHER_DIR) is a correctness issue that's still present in copy_into_place, but it has no security impact: the worst case is EACCES on exec (a reliability failure, not a vulnerability), and it only affects the dev-override path on Linux.

---

Posted by GitDesktop — AI output, verify before acting on it.

[Anonymous]

Originally posted by: theBGuy

🤖 GitDesktop AI review · sonnet · automated

---

All four previous-review nits and the two Copilot-flagged copy/comment issues are resolved in this round. One genuine should-fix from the first review is still present.

Resolved since last review

• norm_launcher_path Linux case-sensitivity — the #[cfg(any(windows, target_os = "macos"))] gate is in the diff and the new platform-split tests confirm it.
• mcp_global_remove unused _app — signature is now client: String only.
• key={client} on non-array JSX — removed from <div className="space-y-1">.
• Copilot: success note "gitdesktop" → "gitdesktop-mcp" — fixed in path_launcher.rs:312.
• Copilot: TSX "shells out to each CLI" comment — corrected in GitDesktopAsServer.tsx:87.
• Copilot: help-guide "one-click button" description — rewritten to "per-client row" with accurate install/reinstall/remove language.
• Copilot: help-guide stale gitdesktop mcp ... command — removed; replaced with a platform-split description that avoids the old invocation form entirely.

---

Correctness

should-fix — mcp_launcher.rs, copy_into_place: the Linux execute-bit problem from the previous review is still unaddressed. The "changes since last review" diff contains no edits to mcp_launcher.rs. std::fs::copy on Linux does not preserve mode bits; the destination is created with umask-restricted permissions (0o644) and is therefore not executable. The GD_MCP_LAUNCHER_DIR env override explicitly enables the managed-copy path on any OS, so a developer live-validating this on Linux gets a copy that ensure() accepts as fresh but that exec rejects with EACCES.

Fix (after the temp-file write, before fs::rename):

#[cfg(unix)]
{
    use std::os::unix::fs::PermissionsExt;
    let meta = std::fs::metadata(&tmp).map_err(AppError::Io)?;
    let mut perms = meta.permissions();
    perms.set_mode(perms.mode() | 0o111);
    std::fs::set_permissions(&tmp, perms).map_err(AppError::Io)?;
}

---

Nits

nit — src/lib/git/api.ts, mcpGlobalStatus JSDoc: the new comment says "via each CLI" — that described the earlier CLI-spawn design. The current mcp_global_status backend reads each client's config file directly (no process spawn). The TSX comment was corrected; the api.ts JSDoc line wasn't. Change "via each CLI" to "by reading each client's config file directly" for accuracy.

---

Posted by GitDesktop — AI output, verify before acting on it.

[Anonymous]

Originally posted by: theBGuy

🤖 GitDesktop AI review · sonnet · automated

---

The diff addresses all four findings from the previous round (Linux case-fold guard in norm_launcher_path, dropped _app from mcp_global_remove, removed no-op key prop, and the gitdesktop → gitdesktop-mcp note text). One prior should-fix is still open; one new nit surfaced.

Resolved since last review

• norm_launcher_path Linux case-sensitivity — #[cfg(any(windows, target_os = "macos"))] guard is present and the test is correctly split.
• _app: tauri::AppHandle unused in mcp_global_remove — parameter dropped cleanly.
• key={client} on non-array <div> — removed from the globalRow return.
• Copilot's four doc/comment stale-text findings — all addressed (note text, comment on the useQuery, help content, "one-click button" wording).

---

Correctness

should-fix — mcp_launcher.rs, copy_into_place (unaddressed from prior review): std::fs::copy on Linux creates the destination with umask-restricted bits (typically 0o644), so the managed copy is not executable. The "changes since last review" diff doesn't touch mcp_launcher.rs, meaning the fix suggested in the previous round was never applied. In production this only matters under the GD_MCP_LAUNCHER_DIR override (management is Windows-only at runtime), but that override is the documented way to exercise the copy machinery on Linux/macOS dev machines — a developer following the module's own instructions will silently get an EACCES when the CLI tries to run the copy.

Fix (after the temp-write, before rename):

#[cfg(unix)]
{
    use std::os::unix::fs::PermissionsExt;
    let meta = std::fs::metadata(&tmp).map_err(AppError::Io)?;
    let mut perms = meta.permissions();
    perms.set_mode(perms.mode() | 0o111);
    std::fs::set_permissions(&tmp, perms).map_err(AppError::Io)?;
}

---

Nit

nit — src/lib/git/api.ts, mcpGlobalStatus doc comment: the JSDoc still says "via each CLI" (/** Read-only probe … via each CLI. …*/), but the implementation (and the updated GitDesktopAsServer.tsx comment) now correctly describes it as a direct config-file read — no CLI spawn. Low stakes, but misleads the next developer debugging a spawn timeout.

---

Posted by GitDesktop — AI output, verify before acting on it.

[Anonymous]

Originally posted by: theBGuy

🤖 GitDesktop AI review · sonnet · automated

---

The PR adds per-client (Claude Code / Copilot) global install status rows with Install/Reinstall/Remove controls, and replaces the direct-binary MCP invocation with a managed gitdesktop-mcp copy on Windows. The architecture is sound and the code is well-structured.

Resolved since last review

• mcp_launcher.rs copy_into_place — Linux execute-bit finding — Closed, and this finding was never valid. The previous rounds incorrectly claimed std::fs::copy on Linux creates the destination with umask-restricted bits. Rust's stdlib docs and implementation are explicit: fs::copy copies the source's permission bits on all Unix platforms (it calls set_permissions on the destination after the data copy). The new comment added in this round states this accurately. There is nothing to fix here, and there never was.
• api.ts mcpGlobalStatus JSDoc — corrected from "via each CLI" to "by reading each client's config file directly (no CLI spawn)". Accurate.

---

Correctness

nit — path_launcher.rs, path_launcher_install, _version on non-Windows: the underscore-prefix suppresses the "unused variable" lint, which is correct, but the way the cfg blocks are structured means _version is allocated and .to_string()-copied even on macOS/Linux where install_impl() never sees it. This is harmless (it's just a string alloc), but restructuring with #[cfg(windows)] let version = … would be cleaner and self-documenting.

---

The overall implementation — marker-based staleness check, same-volume atomic rename, zero-write status probe via resolved_launcher_path(), and UI gating on launcherDisabledReason before any path-embedding emission — is all well-designed. No new real issues.

---

Posted by GitDesktop — AI output, verify before acting on it.

[Anonymous]

Ticket changed by: theBGuy

• status: open --> closed