From 6bfacea6d5177cda6b18437ed7c5eae5915cb609 Mon Sep 17 00:00:00 2001 From: Jordon Date: Thu, 4 Jun 2026 10:49:33 +0100 Subject: [PATCH 1/7] Add more merge options --- Backend/src/core/mod.rs | 42 ++++++++++++++- Backend/src/core/models.rs | 16 ++++++ Backend/src/lib.rs | 1 + .../src/plugin_runtime/node_instance/vcs.rs | 15 +++++- Backend/src/plugin_runtime/vcs_proxy.rs | 26 +++++++-- Backend/src/tauri_commands/branches.rs | 46 +++++++++++++++- Backend/tests/core/mod.rs | 6 +-- .../tests/plugin_runtime/node_instance/vcs.rs | 4 +- Backend/tests/plugin_runtime/vcs_proxy.rs | 2 +- Backend/tests/tauri_commands/branches.rs | 41 ++++++++++++++ Frontend/src/modals/merge-strategy.html | 29 ++++++++++ Frontend/src/scripts/features/branches.ts | 8 +-- .../src/scripts/features/mergeStrategy.ts | 53 +++++++++++++++++++ Frontend/src/scripts/ui/modals.ts | 2 + Frontend/src/styles/index.css | 1 + Frontend/src/styles/modal/merge-strategy.css | 44 +++++++++++++++ .../tests/scripts/features/branches.test.ts | 25 ++++++--- Frontend/tests/scripts/ui/modals.test.ts | 5 ++ 18 files changed, 341 insertions(+), 25 deletions(-) create mode 100644 Frontend/src/modals/merge-strategy.html create mode 100644 Frontend/src/scripts/features/mergeStrategy.ts create mode 100644 Frontend/src/styles/modal/merge-strategy.css diff --git a/Backend/src/core/mod.rs b/Backend/src/core/mod.rs index 7b901f35..40b1dd42 100644 --- a/Backend/src/core/mod.rs +++ b/Backend/src/core/mod.rs @@ -13,6 +13,8 @@ pub use self::models::OnEvent; use std::path::{Path, PathBuf}; +use crate::core::models::VcsCaps; + /// Error type returned by VCS backend operations. #[derive(thiserror::Error, Debug)] pub enum VcsError { @@ -33,6 +35,14 @@ pub enum VcsError { /// Backend-provided error message. msg: String, }, + /// The backend does not support the requested merge strategy. + #[error("unsupported merge strategy '{strategy}' for backend {backend}")] + UnsupportedStrategy { + /// Backend identifier. + backend: BackendId, + /// The strategy that was requested. + strategy: String, + }, } impl VcsError { @@ -47,6 +57,17 @@ impl VcsError { VcsError::Unsupported(backend) => format!("unsupported backend: {backend}"), VcsError::Io(e) => e.to_string(), VcsError::Backend { msg, .. } => msg.clone(), + VcsError::UnsupportedStrategy { strategy, .. } => { + format!("unsupported merge strategy '{strategy}'") + } + } + } + + /// Builds an unsupported strategy error for the given backend and strategy. + pub fn unsupported_strategy(backend: &BackendId, strategy: &str) -> Self { + VcsError::UnsupportedStrategy { + backend: backend.clone(), + strategy: strategy.to_string(), } } } @@ -87,6 +108,11 @@ pub trait Vcs: Send + Sync { /// Creates a commit from the provided paths and returns its id. fn commit(&self, message: &str, name: &str, email: &str, paths: &[PathBuf]) -> Result; + /// Returns the capabilities advertised by this backend. + fn caps(&self) -> Result { + Ok(VcsCaps::default()) + } + /// Creates a commit from the current index and returns its id. fn commit_index(&self, message: &str, name: &str, email: &str) -> Result; /// Returns status details for files and ahead/behind counts. @@ -139,8 +165,20 @@ pub trait Vcs: Send + Sync { fn rename_branch(&self, old: &str, new: &str) -> Result<()>; /// Merges a branch into the current branch. fn merge_into_current(&self, name: &str) -> Result<()>; - /// Merges a branch into the current branch with an optional message. - fn merge_into_current_with_message(&self, name: &str, message: Option<&str>) -> Result<()> { + /// Merges a branch into the current branch with an optional message and strategy. + fn merge_into_current_with_message( + &self, + name: &str, + message: Option<&str>, + strategy: Option<&str>, + ) -> Result<()> { + // Reject non-merge strategies by default — backends that support them + // override this method and advertise via caps. + if let Some(s) = strategy { + if s != "merge" { + return Err(VcsError::unsupported_strategy(&self.id(), s)); + } + } let _ = message; self.merge_into_current(name) } diff --git a/Backend/src/core/models.rs b/Backend/src/core/models.rs index 836bf745..ff8f6374 100644 --- a/Backend/src/core/models.rs +++ b/Backend/src/core/models.rs @@ -236,6 +236,22 @@ pub struct HunkSelection { pub partial_hunks: std::collections::HashMap>, } +/// Describes the capabilities advertised by a VCS backend. +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct VcsCaps { + /// Whether the backend supports merge strategy selection (merge/squash/rebase). + #[serde(default)] + pub merge_strategies: bool, +} + +impl Default for VcsCaps { + fn default() -> Self { + Self { + merge_strategies: false, + } + } +} + /// Callback function type for handling VCS events. pub type OnEvent = Arc; diff --git a/Backend/src/lib.rs b/Backend/src/lib.rs index e46c3b6a..053c7345 100644 --- a/Backend/src/lib.rs +++ b/Backend/src/lib.rs @@ -419,6 +419,7 @@ fn build_invoke_handler() tauri_commands::vcs_launch_merge_tool, tauri_commands::vcs_delete_branch, tauri_commands::vcs_merge_branch, + tauri_commands::vcs_merge_strategy_supported, tauri_commands::vcs_merge_context, tauri_commands::vcs_merge_abort, tauri_commands::vcs_merge_continue, diff --git a/Backend/src/plugin_runtime/node_instance/vcs.rs b/Backend/src/plugin_runtime/node_instance/vcs.rs index da52a43e..389cfe95 100644 --- a/Backend/src/plugin_runtime/node_instance/vcs.rs +++ b/Backend/src/plugin_runtime/node_instance/vcs.rs @@ -280,9 +280,20 @@ impl NodePluginRuntimeInstance { self.rpc_call_unit(Methods::VCS_RENAME_BRANCH, params) } + /// Calls `vcs.get_caps` and returns the raw JSON response. + pub fn vcs_get_caps(&self) -> Result { + let params = self.session_params(Value::Object(serde_json::Map::new()))?; + self.rpc_call(Methods::VCS_GET_CAPS, params) + } + /// Calls `vcs.merge-into-current`. - pub fn vcs_merge_into_current(&self, name: &str, message: Option<&str>) -> Result<(), String> { - let params = self.session_params(json!({ "name": name, "message": message }))?; + pub fn vcs_merge_into_current( + &self, + name: &str, + message: Option<&str>, + strategy: Option<&str>, + ) -> Result<(), String> { + let params = self.session_params(json!({ "name": name, "message": message, "strategy": strategy }))?; self.rpc_call_unit(Methods::VCS_MERGE_INTO_CURRENT, params) } diff --git a/Backend/src/plugin_runtime/vcs_proxy.rs b/Backend/src/plugin_runtime/vcs_proxy.rs index d8a70731..c8bdb101 100644 --- a/Backend/src/plugin_runtime/vcs_proxy.rs +++ b/Backend/src/plugin_runtime/vcs_proxy.rs @@ -5,6 +5,7 @@ use crate::core::models::{ BranchItem, CommitItem, ConflictDetails, ConflictSide, DiffFileResult, LogQuery, OnEvent, StashItem, StatusPayload, }; +use crate::core::models::VcsCaps; use crate::core::{BackendId, Result as VcsResult, Vcs, VcsError}; use crate::logging::LogTimer; use crate::plugin_runtime::instance::PluginRuntimeInstance; @@ -285,15 +286,34 @@ impl Vcs for PluginVcsProxy { .map_err(|e| self.map_runtime_error(e)) } + fn caps(&self) -> VcsResult { + let response = self + .runtime + .vcs_get_caps() + .map_err(|e| VcsError::Backend { + backend: self.backend_id.clone(), + msg: e, + })?; + serde_json::from_value(response).map_err(|e| VcsError::Backend { + backend: self.backend_id.clone(), + msg: format!("failed to parse caps: {e}"), + }) + } + fn merge_into_current(&self, name: &str) -> VcsResult<()> { self.runtime - .vcs_merge_into_current(name, None) + .vcs_merge_into_current(name, None, None) .map_err(|e| self.map_runtime_error(e)) } - fn merge_into_current_with_message(&self, name: &str, message: Option<&str>) -> VcsResult<()> { + fn merge_into_current_with_message( + &self, + name: &str, + message: Option<&str>, + strategy: Option<&str>, + ) -> VcsResult<()> { self.runtime - .vcs_merge_into_current(name, message) + .vcs_merge_into_current(name, message, strategy) .map_err(|e| self.map_runtime_error(e)) } diff --git a/Backend/src/tauri_commands/branches.rs b/Backend/src/tauri_commands/branches.rs index 0880276f..c07a7794 100644 --- a/Backend/src/tauri_commands/branches.rs +++ b/Backend/src/tauri_commands/branches.rs @@ -377,25 +377,67 @@ pub async fn vcs_rename_branch( .await } +#[tauri::command] +/// Returns whether the current VCS backend supports merge strategy selection. +/// +/// # Parameters +/// - `state`: Shared application state. +/// +/// # Returns +/// - `Ok(bool)` indicating whether merge strategies are supported. +pub async fn vcs_merge_strategy_supported(state: State<'_, AppState>) -> Result { + let repo = current_repo_or_err(&state)?; + run_repo_task("vcs_merge_strategy_supported", repo, move |repo| { + let caps = repo.inner().caps().map_err(|e| e.to_string())?; + Ok(caps.merge_strategies) + }) + .await +} + #[tauri::command] /// Merges a source branch into the current branch. /// /// # Parameters /// - `state`: Shared application state. /// - `name`: Source branch to merge. +/// - `strategy`: Optional merge strategy (`merge`, `squash`, or `rebase`). /// /// # Returns /// - `Ok(())` when merge succeeds. /// - `Err(String)` when validation or merge fails. -pub async fn vcs_merge_branch(state: State<'_, AppState>, name: String) -> Result<(), String> { +pub async fn vcs_merge_branch( + state: State<'_, AppState>, + name: String, + strategy: Option, +) -> Result<(), String> { let name = name.trim(); if name.is_empty() { return Err("Branch name cannot be empty".to_string()); } + // Validate strategy before dispatching + let validated_strategy = match strategy.as_deref() { + None | Some("merge") => None, + Some(s) if s.trim().is_empty() => { + return Err("Merge strategy cannot be empty".to_string()); + } + Some("squash") | Some("rebase") => strategy, + Some(other) => return Err(format!("Unknown merge strategy '{other}'. Must be 'merge', 'squash', or 'rebase'.")), + }; + let needs_caps_check = validated_strategy.is_some(); + let repo = current_repo_or_err(&state)?; let branch = name.to_string(); let template = backend_merge_message_template(&repo.id()); + let strategy_clone = validated_strategy.clone(); run_repo_task("vcs_merge_branch", repo, move |repo| { + // Backend-side capability guard: reject squash/rebase if unsupported + if needs_caps_check { + let caps = repo.inner().caps().map_err(|e| e.to_string())?; + if !caps.merge_strategies { + return Err("Backend does not support merge strategies".to_string()); + } + } + let vcs = repo.inner(); let target_branch = vcs .current_branch() @@ -437,7 +479,7 @@ pub async fn vcs_merge_branch(state: State<'_, AppState>, name: String) -> Resul )) }; - vcs.merge_into_current_with_message(&branch, message.as_deref()) + vcs.merge_into_current_with_message(&branch, message.as_deref(), strategy_clone.as_deref()) .map_err(|e| e.to_string()) }) .await diff --git a/Backend/tests/core/mod.rs b/Backend/tests/core/mod.rs index 4eb9e160..73554c66 100644 --- a/Backend/tests/core/mod.rs +++ b/Backend/tests/core/mod.rs @@ -298,7 +298,7 @@ fn vcs_revert_commit_returns_unsupported_by_default() { fn vcs_merge_into_current_with_message_delegates_to_merge_into_current() { let vcs = DummyVcs::new("test"); // Should not error even with a message - assert!(vcs.merge_into_current_with_message("feature", Some("auto-merge")).is_ok()); - // Should work with None message too - assert!(vcs.merge_into_current_with_message("feature", None).is_ok()); + assert!(vcs.merge_into_current_with_message("feature", Some("auto-merge"), None).is_ok()); + // Should work with None message and strategy too + assert!(vcs.merge_into_current_with_message("feature", None, None).is_ok()); } diff --git a/Backend/tests/plugin_runtime/node_instance/vcs.rs b/Backend/tests/plugin_runtime/node_instance/vcs.rs index ff13c412..19f80c40 100644 --- a/Backend/tests/plugin_runtime/node_instance/vcs.rs +++ b/Backend/tests/plugin_runtime/node_instance/vcs.rs @@ -176,7 +176,7 @@ fn vcs_merge_into_current_works() { let rt = test_runtime(); *rt.vcs_session_id.lock() = Some("s".into()); mock_response(&rt, Value::Null); - rt.vcs_merge_into_current("feature", Some("merge msg")).unwrap(); + rt.vcs_merge_into_current("feature", Some("merge msg"), None).unwrap(); } #[test] @@ -184,7 +184,7 @@ fn vcs_merge_into_current_without_message_works() { let rt = test_runtime(); *rt.vcs_session_id.lock() = Some("s".into()); mock_response(&rt, Value::Null); - rt.vcs_merge_into_current("feature", None::<&str>).unwrap(); + rt.vcs_merge_into_current("feature", None::<&str>, None).unwrap(); } #[test] diff --git a/Backend/tests/plugin_runtime/vcs_proxy.rs b/Backend/tests/plugin_runtime/vcs_proxy.rs index 143bfe50..4810864a 100644 --- a/Backend/tests/plugin_runtime/vcs_proxy.rs +++ b/Backend/tests/plugin_runtime/vcs_proxy.rs @@ -212,7 +212,7 @@ fn proxy_merge_into_current_with_message_delegates() { let (proxy, rt) = mock_proxy(); rt.set_session_id(Some("s".into())); set_unit_response(&rt); - assert!(proxy.merge_into_current_with_message("feature", Some("msg")).is_ok()); + assert!(proxy.merge_into_current_with_message("feature", Some("msg"), None).is_ok()); } #[test] diff --git a/Backend/tests/tauri_commands/branches.rs b/Backend/tests/tauri_commands/branches.rs index 430746c8..3aa6e344 100644 --- a/Backend/tests/tauri_commands/branches.rs +++ b/Backend/tests/tauri_commands/branches.rs @@ -170,6 +170,7 @@ fn build_vcs_branches_app() -> (tauri::App, Arc().expect("should deserialize bool"); + assert!(!data, "test backend should not support merge strategies"); +} diff --git a/Frontend/src/modals/merge-strategy.html b/Frontend/src/modals/merge-strategy.html new file mode 100644 index 00000000..3b38c600 --- /dev/null +++ b/Frontend/src/modals/merge-strategy.html @@ -0,0 +1,29 @@ + diff --git a/Frontend/src/scripts/features/branches.ts b/Frontend/src/scripts/features/branches.ts index f902d38b..5e6556d6 100644 --- a/Frontend/src/scripts/features/branches.ts +++ b/Frontend/src/scripts/features/branches.ts @@ -12,6 +12,7 @@ import { openRenameBranch } from './renameBranch'; import { openSetUpstream } from './setUpstream'; import { confirmDeleteBranch } from './deleteBranchConfirm'; import { buildCtxMenu, CtxItem } from '../lib/menu'; +import { promptMergeStrategy } from './mergeStrategy'; import { renderList, hydrateStatus } from './repo'; import { setTab } from '../ui/layout'; import type { ConflictDetails, FileStatus } from '../types'; @@ -212,8 +213,9 @@ export function bindBranchUI() { }}); items.push({ label: 'Merge into current…', action: async () => { if (name === cur) { notify('Cannot merge a branch into itself'); return; } - const ok = await confirmBool(`Merge '${name}' into '${cur}'?`); - if (!ok) return; + const supported = await TAURI.invoke('vcs_merge_strategy_supported').catch(() => false); + const strategy = supported ? await promptMergeStrategy(name, cur) : (await confirmBool(`Merge '${name}' into '${cur}'?`) ? 'merge' : null); + if (!strategy) return; const statusEl = document.getElementById('status'); const setBusy = (msg: string) => { @@ -225,7 +227,7 @@ export function bindBranchUI() { try { setBusy('Merging…'); - await TAURI.invoke('vcs_merge_branch', { name }); + await TAURI.invoke('vcs_merge_branch', { name, strategy }); clearBusy(); notify(`Merged branch '${name}' into '${cur}'`); await Promise.allSettled([renderList(), loadBranches()]); diff --git a/Frontend/src/scripts/features/mergeStrategy.ts b/Frontend/src/scripts/features/mergeStrategy.ts new file mode 100644 index 00000000..860f1750 --- /dev/null +++ b/Frontend/src/scripts/features/mergeStrategy.ts @@ -0,0 +1,53 @@ +// Copyright © 2025-2026 OpenVCS Contributors +// SPDX-License-Identifier: GPL-3.0-or-later +import { closeModal, hydrate, openModal } from '../ui/modals'; + +let wired = false; +let pendingResolve: ((strategy: string | null) => void) | null = null; + +function resolvePending(strategy: string | null) { + if (!pendingResolve) return; + const resolve = pendingResolve; + pendingResolve = null; + resolve(strategy); +} + +function wireMergeStrategyModal() { + if (wired) return; + wired = true; + const modal = document.getElementById('merge-strategy-modal'); + if (!modal) return; + + const options = modal.querySelectorAll('.merge-strategy-option'); + for (const opt of options) { + const handler = () => { + const strategy = opt.getAttribute('data-strategy'); + resolvePending(strategy); + closeModal('merge-strategy-modal'); + }; + opt.addEventListener('click', handler); + opt.addEventListener('keydown', (e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + handler(); + } + }); + } +} + +export function promptMergeStrategy(branchName: string, targetBranch: string): Promise { + hydrate('merge-strategy-modal'); + wireMergeStrategyModal(); + + const hintEl = document.getElementById('merge-strategy-hint'); + if (hintEl) { + hintEl.textContent = `Choose how to merge '${branchName}' into '${targetBranch}'.`; + } + + return new Promise((resolve) => { + const prev = pendingResolve; + pendingResolve = resolve; + if (prev) prev(null); + openModal('merge-strategy-modal'); + }); +} diff --git a/Frontend/src/scripts/ui/modals.ts b/Frontend/src/scripts/ui/modals.ts index 34066d29..1384376f 100644 --- a/Frontend/src/scripts/ui/modals.ts +++ b/Frontend/src/scripts/ui/modals.ts @@ -29,6 +29,7 @@ import { wireUpdate } from "../features/update"; import stashConfirmHtml from "@modals/stash-confirm.html?raw"; import { wireStashConfirm } from "../features/stashConfirm"; import mergeHtml from "@modals/merge.html?raw"; +import mergeStrategyHtml from "@modals/merge-strategy.html?raw"; import conflictsSummaryHtml from "@modals/conflicts-summary.html?raw"; import { wireSshKeys } from "../features/sshKeys"; import repoSwitchDrawerHtml from "@modals/repoSwitchDrawer.html?raw"; @@ -53,6 +54,7 @@ const FRAGMENTS: Record = { "update-modal": updateHtml, "stash-confirm-modal": stashConfirmHtml, "merge-modal": mergeHtml, + "merge-strategy-modal": mergeStrategyHtml, "conflicts-summary-modal": conflictsSummaryHtml, "error-modal": errorHtml, }; diff --git a/Frontend/src/styles/index.css b/Frontend/src/styles/index.css index ddcc187c..ac2975bf 100644 --- a/Frontend/src/styles/index.css +++ b/Frontend/src/styles/index.css @@ -17,6 +17,7 @@ @import "./modal/confirm.css"; @import "./modal/stash-confirm.css"; @import "./modal/merge.css"; +@import "./modal/merge-strategy.css"; @import "./modal/ssh-keys.css"; @import "./modal/delete-branch.css"; @import "./modal/repo-switch-drawer.css"; diff --git a/Frontend/src/styles/modal/merge-strategy.css b/Frontend/src/styles/modal/merge-strategy.css new file mode 100644 index 00000000..dce88fbf --- /dev/null +++ b/Frontend/src/styles/modal/merge-strategy.css @@ -0,0 +1,44 @@ +/* src/styles/modal/merge-strategy.css */ + +#merge-strategy-modal .dialog.sheet { + width: clamp(380px, 88vw, 480px); + max-height: min(80vh, 400px); +} + +#merge-strategy-body { + display: flex; + flex-direction: column; + gap: 8px; + padding: 12px 16px; +} + +.merge-strategy-option { + cursor: pointer; + border: 1px solid var(--border, #ddd); + border-radius: 8px; + padding: 12px 16px; + transition: background-color 0.15s, border-color 0.15s; + user-select: none; +} + +.merge-strategy-option:hover { + background-color: var(--hover-bg, rgba(0,0,0,0.04)); + border-color: var(--accent, #4a90d9); +} + +.merge-strategy-option:focus-visible { + outline: 2px solid var(--accent, #4a90d9); + outline-offset: 2px; +} + +.merge-strategy-option strong { + display: block; + font-size: 14px; + margin-bottom: 2px; +} + +.merge-strategy-option .hint { + margin: 0; + font-size: 12px; + opacity: 0.75; +} diff --git a/Frontend/tests/scripts/features/branches.test.ts b/Frontend/tests/scripts/features/branches.test.ts index 2bf6d13d..15e85867 100644 --- a/Frontend/tests/scripts/features/branches.test.ts +++ b/Frontend/tests/scripts/features/branches.test.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mockInvoke = vi.fn(); const mockConfirmBool = vi.fn(); +const mockPromptMergeStrategy = vi.fn(); const mockNotify = vi.fn(); const mockRefreshOverlayScrollbarsFor = vi.fn(); const mockOpenModal = vi.fn(); @@ -49,6 +50,12 @@ vi.mock('@scripts/state/state', () => ({ vi.mock('@scripts/ui/modals', () => ({ openModal: mockOpenModal, + closeModal: vi.fn(), + hydrate: vi.fn(), +})); + +vi.mock('@scripts/features/mergeStrategy', () => ({ + promptMergeStrategy: mockPromptMergeStrategy, })); vi.mock('@scripts/features/renameBranch', () => ({ @@ -387,10 +394,11 @@ describe('bindBranchUI', () => { }); it('merge into current', async () => { - mockConfirmBool.mockResolvedValue(true); + mockPromptMergeStrategy.mockResolvedValue('merge'); mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockInvoke.mockResolvedValueOnce(undefined); + mockInvoke.mockResolvedValueOnce(true); // vcs_merge_strategy_supported + mockInvoke.mockResolvedValueOnce(undefined); // vcs_merge_branch const { bindBranchUI } = await import('@scripts/features/branches'); bindBranchUI(); @@ -399,15 +407,16 @@ describe('bindBranchUI', () => { const items = await triggerContextMenu(); await items[1].action(); - expect(mockConfirmBool).toHaveBeenCalledWith("Merge 'feature' into 'main'?"); - expect(mockInvoke).toHaveBeenCalledWith('vcs_merge_branch', { name: 'feature' }); + expect(mockPromptMergeStrategy).toHaveBeenCalledWith('feature', 'main'); + expect(mockInvoke).toHaveBeenCalledWith('vcs_merge_branch', { name: 'feature', strategy: 'merge' }); expect(mockNotify).toHaveBeenCalledWith("Merged branch 'feature' into 'main'"); }); it('merge cancelled by user', async () => { - mockConfirmBool.mockResolvedValue(false); + mockPromptMergeStrategy.mockResolvedValue(null); mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockInvoke.mockResolvedValueOnce(true); // vcs_merge_strategy_supported const { bindBranchUI } = await import('@scripts/features/branches'); bindBranchUI(); @@ -435,9 +444,10 @@ describe('bindBranchUI', () => { }); it('merge detects conflicts', async () => { - mockConfirmBool.mockResolvedValue(true); + mockPromptMergeStrategy.mockResolvedValue('merge'); mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockInvoke.mockResolvedValueOnce(true); // vcs_merge_strategy_supported mockInvoke.mockRejectedValueOnce(new Error('Automatic merge failed; fix conflicts and then commit')); mockState.files = [{ path: 'file.txt' }]; @@ -453,9 +463,10 @@ describe('bindBranchUI', () => { }); it('merge shows generic error', async () => { - mockConfirmBool.mockResolvedValue(true); + mockPromptMergeStrategy.mockResolvedValue('merge'); mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockInvoke.mockResolvedValueOnce(true); // vcs_merge_strategy_supported mockInvoke.mockRejectedValueOnce(new Error('some other error')); const { bindBranchUI } = await import('@scripts/features/branches'); diff --git a/Frontend/tests/scripts/ui/modals.test.ts b/Frontend/tests/scripts/ui/modals.test.ts index a3d165ae..9c43db63 100644 --- a/Frontend/tests/scripts/ui/modals.test.ts +++ b/Frontend/tests/scripts/ui/modals.test.ts @@ -722,6 +722,11 @@ describe('hydrate wiring for specific modals', () => { expect(() => hydrate('merge-modal')).not.toThrow(); }); + it('hydrates merge-strategy-modal without throwing', async () => { + const { hydrate } = await import('@scripts/ui/modals'); + expect(() => hydrate('merge-strategy-modal')).not.toThrow(); + }); + it('hydrates conflicts-summary-modal without throwing', async () => { const { hydrate } = await import('@scripts/ui/modals'); expect(() => hydrate('conflicts-summary-modal')).not.toThrow(); From 9f9072b2865288933a29b6524523288355fd5bdb Mon Sep 17 00:00:00 2001 From: Jordon Date: Thu, 4 Jun 2026 11:13:44 +0100 Subject: [PATCH 2/7] Fix tests --- Backend/src/core/mod.rs | 5 ++--- Backend/src/core/models.rs | 8 +------- Backend/src/plugin_runtime/node_instance/vcs.rs | 3 ++- Backend/src/plugin_runtime/vcs_proxy.rs | 13 +++++-------- Backend/src/tauri_commands/branches.rs | 6 +++++- Backend/src/tauri_commands/plugins.rs | 5 ++--- 6 files changed, 17 insertions(+), 23 deletions(-) diff --git a/Backend/src/core/mod.rs b/Backend/src/core/mod.rs index 40b1dd42..f96c9ce9 100644 --- a/Backend/src/core/mod.rs +++ b/Backend/src/core/mod.rs @@ -174,11 +174,10 @@ pub trait Vcs: Send + Sync { ) -> Result<()> { // Reject non-merge strategies by default — backends that support them // override this method and advertise via caps. - if let Some(s) = strategy { - if s != "merge" { + if let Some(s) = strategy + && s != "merge" { return Err(VcsError::unsupported_strategy(&self.id(), s)); } - } let _ = message; self.merge_into_current(name) } diff --git a/Backend/src/core/models.rs b/Backend/src/core/models.rs index ff8f6374..5f1f6bb5 100644 --- a/Backend/src/core/models.rs +++ b/Backend/src/core/models.rs @@ -238,19 +238,13 @@ pub struct HunkSelection { /// Describes the capabilities advertised by a VCS backend. #[derive(Serialize, Deserialize, Clone, Debug)] +#[derive(Default)] pub struct VcsCaps { /// Whether the backend supports merge strategy selection (merge/squash/rebase). #[serde(default)] pub merge_strategies: bool, } -impl Default for VcsCaps { - fn default() -> Self { - Self { - merge_strategies: false, - } - } -} /// Callback function type for handling VCS events. pub type OnEvent = Arc; diff --git a/Backend/src/plugin_runtime/node_instance/vcs.rs b/Backend/src/plugin_runtime/node_instance/vcs.rs index 389cfe95..7499964f 100644 --- a/Backend/src/plugin_runtime/node_instance/vcs.rs +++ b/Backend/src/plugin_runtime/node_instance/vcs.rs @@ -293,7 +293,8 @@ impl NodePluginRuntimeInstance { message: Option<&str>, strategy: Option<&str>, ) -> Result<(), String> { - let params = self.session_params(json!({ "name": name, "message": message, "strategy": strategy }))?; + let params = + self.session_params(json!({ "name": name, "message": message, "strategy": strategy }))?; self.rpc_call_unit(Methods::VCS_MERGE_INTO_CURRENT, params) } diff --git a/Backend/src/plugin_runtime/vcs_proxy.rs b/Backend/src/plugin_runtime/vcs_proxy.rs index c8bdb101..3f2a2390 100644 --- a/Backend/src/plugin_runtime/vcs_proxy.rs +++ b/Backend/src/plugin_runtime/vcs_proxy.rs @@ -1,11 +1,11 @@ // Copyright © 2025-2026 OpenVCS Contributors // SPDX-License-Identifier: GPL-3.0-or-later +use crate::core::models::VcsCaps; use crate::core::models::{ BranchItem, CommitItem, ConflictDetails, ConflictSide, DiffFileResult, LogQuery, OnEvent, StashItem, StatusPayload, }; -use crate::core::models::VcsCaps; use crate::core::{BackendId, Result as VcsResult, Vcs, VcsError}; use crate::logging::LogTimer; use crate::plugin_runtime::instance::PluginRuntimeInstance; @@ -287,13 +287,10 @@ impl Vcs for PluginVcsProxy { } fn caps(&self) -> VcsResult { - let response = self - .runtime - .vcs_get_caps() - .map_err(|e| VcsError::Backend { - backend: self.backend_id.clone(), - msg: e, - })?; + let response = self.runtime.vcs_get_caps().map_err(|e| VcsError::Backend { + backend: self.backend_id.clone(), + msg: e, + })?; serde_json::from_value(response).map_err(|e| VcsError::Backend { backend: self.backend_id.clone(), msg: format!("failed to parse caps: {e}"), diff --git a/Backend/src/tauri_commands/branches.rs b/Backend/src/tauri_commands/branches.rs index c07a7794..9f9f2e69 100644 --- a/Backend/src/tauri_commands/branches.rs +++ b/Backend/src/tauri_commands/branches.rs @@ -421,7 +421,11 @@ pub async fn vcs_merge_branch( return Err("Merge strategy cannot be empty".to_string()); } Some("squash") | Some("rebase") => strategy, - Some(other) => return Err(format!("Unknown merge strategy '{other}'. Must be 'merge', 'squash', or 'rebase'.")), + Some(other) => { + return Err(format!( + "Unknown merge strategy '{other}'. Must be 'merge', 'squash', or 'rebase'." + )); + } }; let needs_caps_check = validated_strategy.is_some(); diff --git a/Backend/src/tauri_commands/plugins.rs b/Backend/src/tauri_commands/plugins.rs index 850d5a1e..011ea570 100644 --- a/Backend/src/tauri_commands/plugins.rs +++ b/Backend/src/tauri_commands/plugins.rs @@ -630,11 +630,10 @@ pub fn list_plugins_with_settings(state: State<'_, AppState>) -> Result Date: Thu, 4 Jun 2026 11:20:45 +0100 Subject: [PATCH 3/7] Fixed issues --- Backend/src/core/models.rs | 14 +- Backend/src/lib.rs | 2 +- Backend/src/tauri_commands/branches.rs | 20 +-- Backend/tests/tauri_commands/branches.rs | 12 +- Frontend/src/modals/merge-strategy.html | 2 +- Frontend/src/scripts/features/branches.ts | 5 +- .../src/scripts/features/mergeStrategy.ts | 9 ++ .../tests/scripts/features/branches.test.ts | 134 +++++++++--------- 8 files changed, 110 insertions(+), 88 deletions(-) diff --git a/Backend/src/core/models.rs b/Backend/src/core/models.rs index 5f1f6bb5..5e8c1b61 100644 --- a/Backend/src/core/models.rs +++ b/Backend/src/core/models.rs @@ -238,11 +238,19 @@ pub struct HunkSelection { /// Describes the capabilities advertised by a VCS backend. #[derive(Serialize, Deserialize, Clone, Debug)] -#[derive(Default)] pub struct VcsCaps { - /// Whether the backend supports merge strategy selection (merge/squash/rebase). + /// Merge strategies the backend supports (values like "merge", "squash", "rebase"). + /// Empty means only default merge is supported. #[serde(default)] - pub merge_strategies: bool, + pub merge_strategies: Vec, +} + +impl Default for VcsCaps { + fn default() -> Self { + Self { + merge_strategies: Vec::new(), + } + } } diff --git a/Backend/src/lib.rs b/Backend/src/lib.rs index 053c7345..5ec69de2 100644 --- a/Backend/src/lib.rs +++ b/Backend/src/lib.rs @@ -419,7 +419,7 @@ fn build_invoke_handler() tauri_commands::vcs_launch_merge_tool, tauri_commands::vcs_delete_branch, tauri_commands::vcs_merge_branch, - tauri_commands::vcs_merge_strategy_supported, + tauri_commands::vcs_merge_strategies, tauri_commands::vcs_merge_context, tauri_commands::vcs_merge_abort, tauri_commands::vcs_merge_continue, diff --git a/Backend/src/tauri_commands/branches.rs b/Backend/src/tauri_commands/branches.rs index 9f9f2e69..41b4f0ee 100644 --- a/Backend/src/tauri_commands/branches.rs +++ b/Backend/src/tauri_commands/branches.rs @@ -385,9 +385,9 @@ pub async fn vcs_rename_branch( /// /// # Returns /// - `Ok(bool)` indicating whether merge strategies are supported. -pub async fn vcs_merge_strategy_supported(state: State<'_, AppState>) -> Result { +pub async fn vcs_merge_strategies(state: State<'_, AppState>) -> Result, String> { let repo = current_repo_or_err(&state)?; - run_repo_task("vcs_merge_strategy_supported", repo, move |repo| { + run_repo_task("vcs_merge_strategies", repo, move |repo| { let caps = repo.inner().caps().map_err(|e| e.to_string())?; Ok(caps.merge_strategies) }) @@ -415,7 +415,7 @@ pub async fn vcs_merge_branch( return Err("Branch name cannot be empty".to_string()); } // Validate strategy before dispatching - let validated_strategy = match strategy.as_deref() { + let validated_strategy: Option = match strategy.as_deref() { None | Some("merge") => None, Some(s) if s.trim().is_empty() => { return Err("Merge strategy cannot be empty".to_string()); @@ -427,18 +427,18 @@ pub async fn vcs_merge_branch( )); } }; - let needs_caps_check = validated_strategy.is_some(); let repo = current_repo_or_err(&state)?; let branch = name.to_string(); let template = backend_merge_message_template(&repo.id()); - let strategy_clone = validated_strategy.clone(); run_repo_task("vcs_merge_branch", repo, move |repo| { - // Backend-side capability guard: reject squash/rebase if unsupported - if needs_caps_check { + // Backend-side capability guard: reject strategy if not in supported list + if let Some(ref strat) = validated_strategy { let caps = repo.inner().caps().map_err(|e| e.to_string())?; - if !caps.merge_strategies { - return Err("Backend does not support merge strategies".to_string()); + if !caps.merge_strategies.iter().any(|s| s == strat) { + return Err(format!( + "Backend does not support merge strategy '{strat}'" + )); } } @@ -483,7 +483,7 @@ pub async fn vcs_merge_branch( )) }; - vcs.merge_into_current_with_message(&branch, message.as_deref(), strategy_clone.as_deref()) + vcs.merge_into_current_with_message(&branch, message.as_deref(), validated_strategy.as_deref()) .map_err(|e| e.to_string()) }) .await diff --git a/Backend/tests/tauri_commands/branches.rs b/Backend/tests/tauri_commands/branches.rs index 3aa6e344..d3c871dc 100644 --- a/Backend/tests/tauri_commands/branches.rs +++ b/Backend/tests/tauri_commands/branches.rs @@ -170,7 +170,7 @@ fn build_vcs_branches_app() -> (tauri::App, Arc().expect("should deserialize bool"); - assert!(!data, "test backend should not support merge strategies"); + let data = value.deserialize::>().expect("should deserialize string list"); + assert!(data.is_empty(), "test backend should have no merge strategies"); } diff --git a/Frontend/src/modals/merge-strategy.html b/Frontend/src/modals/merge-strategy.html index 3b38c600..dbea6a12 100644 --- a/Frontend/src/modals/merge-strategy.html +++ b/Frontend/src/modals/merge-strategy.html @@ -19,7 +19,7 @@

Merge strategy

Rebase -

Rebase the current branch onto the selected branch.

+

Rebase commits from the selected branch onto the current branch and fast-forward.

diff --git a/Frontend/src/scripts/features/branches.ts b/Frontend/src/scripts/features/branches.ts index 5e6556d6..12ee71ef 100644 --- a/Frontend/src/scripts/features/branches.ts +++ b/Frontend/src/scripts/features/branches.ts @@ -213,8 +213,9 @@ export function bindBranchUI() { }}); items.push({ label: 'Merge into current…', action: async () => { if (name === cur) { notify('Cannot merge a branch into itself'); return; } - const supported = await TAURI.invoke('vcs_merge_strategy_supported').catch(() => false); - const strategy = supported ? await promptMergeStrategy(name, cur) : (await confirmBool(`Merge '${name}' into '${cur}'?`) ? 'merge' : null); + const strategies = await TAURI.invoke('vcs_merge_strategies').catch(() => []); + const hasAdvanced = strategies.some(s => s === 'squash' || s === 'rebase'); + const strategy = hasAdvanced ? await promptMergeStrategy(name, cur) : (await confirmBool(`Merge '${name}' into '${cur}'?`) ? 'merge' : null); if (!strategy) return; const statusEl = document.getElementById('status'); diff --git a/Frontend/src/scripts/features/mergeStrategy.ts b/Frontend/src/scripts/features/mergeStrategy.ts index 860f1750..ab4e8955 100644 --- a/Frontend/src/scripts/features/mergeStrategy.ts +++ b/Frontend/src/scripts/features/mergeStrategy.ts @@ -39,6 +39,7 @@ export function promptMergeStrategy(branchName: string, targetBranch: string): P hydrate('merge-strategy-modal'); wireMergeStrategyModal(); + const modal = document.getElementById('merge-strategy-modal'); const hintEl = document.getElementById('merge-strategy-hint'); if (hintEl) { hintEl.textContent = `Choose how to merge '${branchName}' into '${targetBranch}'.`; @@ -48,6 +49,14 @@ export function promptMergeStrategy(branchName: string, targetBranch: string): P const prev = pendingResolve; pendingResolve = resolve; if (prev) prev(null); + + // Resolve with null on any dismiss path (backdrop click, cancel, Escape) + const onClosed = () => { + resolvePending(null); + modal?.removeEventListener('modal:closed', onClosed); + }; + modal?.addEventListener('modal:closed', onClosed); + openModal('merge-strategy-modal'); }); } diff --git a/Frontend/tests/scripts/features/branches.test.ts b/Frontend/tests/scripts/features/branches.test.ts index 15e85867..b0cb550d 100644 --- a/Frontend/tests/scripts/features/branches.test.ts +++ b/Frontend/tests/scripts/features/branches.test.ts @@ -393,92 +393,96 @@ describe('bindBranchUI', () => { expect(items[0].label).toBe('Checkout'); }); - it('merge into current', async () => { - mockPromptMergeStrategy.mockResolvedValue('merge'); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockInvoke.mockResolvedValueOnce(true); // vcs_merge_strategy_supported - mockInvoke.mockResolvedValueOnce(undefined); // vcs_merge_branch - const { bindBranchUI } = await import('@scripts/features/branches'); - bindBranchUI(); - await openPopover(1); - const items = await triggerContextMenu(); - await items[1].action(); +it('merge into current', async () => { + mockPromptMergeStrategy.mockResolvedValue('merge'); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockInvoke.mockResolvedValueOnce(['merge', 'squash', 'rebase']); // vcs_merge_strategies + mockInvoke.mockResolvedValueOnce(undefined); // vcs_merge_branch - expect(mockPromptMergeStrategy).toHaveBeenCalledWith('feature', 'main'); - expect(mockInvoke).toHaveBeenCalledWith('vcs_merge_branch', { name: 'feature', strategy: 'merge' }); - expect(mockNotify).toHaveBeenCalledWith("Merged branch 'feature' into 'main'"); - }); + const { bindBranchUI } = await import('@scripts/features/branches'); + bindBranchUI(); + await openPopover(1); - it('merge cancelled by user', async () => { - mockPromptMergeStrategy.mockResolvedValue(null); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockInvoke.mockResolvedValueOnce(true); // vcs_merge_strategy_supported + const items = await triggerContextMenu(); + await items[1].action(); - const { bindBranchUI } = await import('@scripts/features/branches'); - bindBranchUI(); - await openPopover(1); + expect(mockPromptMergeStrategy).toHaveBeenCalledWith('feature', 'main'); + expect(mockInvoke).toHaveBeenCalledWith('vcs_merge_branch', { name: 'feature', strategy: 'merge' }); + expect(mockNotify).toHaveBeenCalledWith("Merged branch 'feature' into 'main'"); +}); - const items = await triggerContextMenu(); - await items[1].action(); +it('merge cancelled by user', async () => { + mockPromptMergeStrategy.mockResolvedValue(null); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockInvoke.mockResolvedValueOnce(['merge', 'squash', 'rebase']); // vcs_merge_strategies - expect(mockInvoke).not.toHaveBeenCalledWith('vcs_merge_branch', expect.anything()); - }); + const { bindBranchUI } = await import('@scripts/features/branches'); + bindBranchUI(); + await openPopover(1); - it('merge into self shows notify', async () => { - mockState.branch = 'feature'; - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + const items = await triggerContextMenu(); + await items[1].action(); - const { bindBranchUI } = await import('@scripts/features/branches'); - bindBranchUI(); - await openPopover(1); + expect(mockInvoke).not.toHaveBeenCalledWith('vcs_merge_branch', expect.anything()); +}); - const items = await triggerContextMenu(); - await items[1].action(); +it('merge into self shows notify', async () => { + mockState.branch = 'feature'; + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - expect(mockNotify).toHaveBeenCalledWith('Cannot merge a branch into itself'); - }); + const { bindBranchUI } = await import('@scripts/features/branches'); + bindBranchUI(); + await openPopover(1); - it('merge detects conflicts', async () => { - mockPromptMergeStrategy.mockResolvedValue('merge'); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockInvoke.mockResolvedValueOnce(true); // vcs_merge_strategy_supported - mockInvoke.mockRejectedValueOnce(new Error('Automatic merge failed; fix conflicts and then commit')); - mockState.files = [{ path: 'file.txt' }]; + const items = await triggerContextMenu(); + await items[1].action(); - const { bindBranchUI } = await import('@scripts/features/branches'); - bindBranchUI(); - await openPopover(1); + expect(mockNotify).toHaveBeenCalledWith('Cannot merge a branch into itself'); +}); - const items = await triggerContextMenu(); - await items[1].action(); +it('merge detects conflicts', async () => { + mockPromptMergeStrategy.mockResolvedValue('merge'); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockInvoke.mockResolvedValueOnce(['merge', 'squash', 'rebase']); // vcs_merge_strategies + mockInvoke.mockRejectedValueOnce(new Error('Automatic merge failed; fix conflicts and then commit')); + mockState.files = [{ path: 'file.txt' }]; - expect(mockNotify).toHaveBeenCalledWith('Merge conflict detected'); - expect(mockSetTab).toHaveBeenCalledWith('changes'); - }); + const { bindBranchUI } = await import('@scripts/features/branches'); + bindBranchUI(); + await openPopover(1); - it('merge shows generic error', async () => { - mockPromptMergeStrategy.mockResolvedValue('merge'); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); - mockInvoke.mockResolvedValueOnce(true); // vcs_merge_strategy_supported - mockInvoke.mockRejectedValueOnce(new Error('some other error')); + const items = await triggerContextMenu(); + await items[1].action(); - const { bindBranchUI } = await import('@scripts/features/branches'); - bindBranchUI(); - await openPopover(1); + expect(mockNotify).toHaveBeenCalledWith('Merge conflict detected'); + expect(mockSetTab).toHaveBeenCalledWith('changes'); +}); - const items = await triggerContextMenu(); - await items[1].action(); +it('merge shows generic error', async () => { + mockPromptMergeStrategy.mockResolvedValue('merge'); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockLoadBranches([{ name: 'feature', kind: { type: 'local' } }]); + mockInvoke.mockResolvedValueOnce(['merge', 'squash', 'rebase']); // vcs_merge_strategies + mockInvoke.mockRejectedValueOnce(new Error('some other error')); + + const { bindBranchUI } = await import('@scripts/features/branches'); + bindBranchUI(); + await openPopover(1); + + const items = await triggerContextMenu(); + await items[1].action(); - expect(mockNotify).toHaveBeenCalledWith('Merge failed: Error: some other error'); + expect(mockNotify).toHaveBeenCalledWith('Merge failed: Error: some other error'); }); + + it('set upstream for local branch', async () => { mockLoadBranches([ { name: 'feature', kind: { type: 'local' } }, From c061032603fcb053bcc945ff5070ca22703ccf68 Mon Sep 17 00:00:00 2001 From: Jordon Date: Thu, 4 Jun 2026 11:26:14 +0100 Subject: [PATCH 4/7] Fixed merge issues --- Backend/src/core/mod.rs | 7 ++++--- Backend/src/core/models.rs | 9 +-------- Backend/src/tauri_commands/branches.rs | 12 +++++++----- Backend/src/tauri_commands/plugins.rs | 7 ++++--- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/Backend/src/core/mod.rs b/Backend/src/core/mod.rs index f96c9ce9..235a3663 100644 --- a/Backend/src/core/mod.rs +++ b/Backend/src/core/mod.rs @@ -175,9 +175,10 @@ pub trait Vcs: Send + Sync { // Reject non-merge strategies by default — backends that support them // override this method and advertise via caps. if let Some(s) = strategy - && s != "merge" { - return Err(VcsError::unsupported_strategy(&self.id(), s)); - } + && s != "merge" + { + return Err(VcsError::unsupported_strategy(&self.id(), s)); + } let _ = message; self.merge_into_current(name) } diff --git a/Backend/src/core/models.rs b/Backend/src/core/models.rs index 5e8c1b61..f1b4ad66 100644 --- a/Backend/src/core/models.rs +++ b/Backend/src/core/models.rs @@ -238,6 +238,7 @@ pub struct HunkSelection { /// Describes the capabilities advertised by a VCS backend. #[derive(Serialize, Deserialize, Clone, Debug)] +#[derive(Default)] pub struct VcsCaps { /// Merge strategies the backend supports (values like "merge", "squash", "rebase"). /// Empty means only default merge is supported. @@ -245,14 +246,6 @@ pub struct VcsCaps { pub merge_strategies: Vec, } -impl Default for VcsCaps { - fn default() -> Self { - Self { - merge_strategies: Vec::new(), - } - } -} - /// Callback function type for handling VCS events. pub type OnEvent = Arc; diff --git a/Backend/src/tauri_commands/branches.rs b/Backend/src/tauri_commands/branches.rs index 41b4f0ee..a9fb91c3 100644 --- a/Backend/src/tauri_commands/branches.rs +++ b/Backend/src/tauri_commands/branches.rs @@ -436,9 +436,7 @@ pub async fn vcs_merge_branch( if let Some(ref strat) = validated_strategy { let caps = repo.inner().caps().map_err(|e| e.to_string())?; if !caps.merge_strategies.iter().any(|s| s == strat) { - return Err(format!( - "Backend does not support merge strategy '{strat}'" - )); + return Err(format!("Backend does not support merge strategy '{strat}'")); } } @@ -483,8 +481,12 @@ pub async fn vcs_merge_branch( )) }; - vcs.merge_into_current_with_message(&branch, message.as_deref(), validated_strategy.as_deref()) - .map_err(|e| e.to_string()) + vcs.merge_into_current_with_message( + &branch, + message.as_deref(), + validated_strategy.as_deref(), + ) + .map_err(|e| e.to_string()) }) .await } diff --git a/Backend/src/tauri_commands/plugins.rs b/Backend/src/tauri_commands/plugins.rs index 011ea570..7b0b219b 100644 --- a/Backend/src/tauri_commands/plugins.rs +++ b/Backend/src/tauri_commands/plugins.rs @@ -631,9 +631,10 @@ pub fn list_plugins_with_settings(state: State<'_, AppState>) -> Result Date: Thu, 4 Jun 2026 11:29:21 +0100 Subject: [PATCH 5/7] Fix comment --- Backend/src/tauri_commands/branches.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Backend/src/tauri_commands/branches.rs b/Backend/src/tauri_commands/branches.rs index a9fb91c3..a10e9e02 100644 --- a/Backend/src/tauri_commands/branches.rs +++ b/Backend/src/tauri_commands/branches.rs @@ -378,13 +378,15 @@ pub async fn vcs_rename_branch( } #[tauri::command] -/// Returns whether the current VCS backend supports merge strategy selection. +/// Returns the merge strategies advertised by the current VCS backend. +/// +/// An empty list means only the default merge strategy is available. /// /// # Parameters /// - `state`: Shared application state. /// /// # Returns -/// - `Ok(bool)` indicating whether merge strategies are supported. +/// - `Ok(Vec)` — supported strategy names (e.g. `"squash"`, `"rebase"`). pub async fn vcs_merge_strategies(state: State<'_, AppState>) -> Result, String> { let repo = current_repo_or_err(&state)?; run_repo_task("vcs_merge_strategies", repo, move |repo| { From 3ac6620d85465502a2116d44171d3352afcb00af Mon Sep 17 00:00:00 2001 From: Jordon Date: Thu, 4 Jun 2026 11:30:06 +0100 Subject: [PATCH 6/7] Update models.rs --- Backend/src/core/models.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Backend/src/core/models.rs b/Backend/src/core/models.rs index f1b4ad66..8d66294e 100644 --- a/Backend/src/core/models.rs +++ b/Backend/src/core/models.rs @@ -237,8 +237,7 @@ pub struct HunkSelection { } /// Describes the capabilities advertised by a VCS backend. -#[derive(Serialize, Deserialize, Clone, Debug)] -#[derive(Default)] +#[derive(Serialize, Deserialize, Clone, Debug, Default)] pub struct VcsCaps { /// Merge strategies the backend supports (values like "merge", "squash", "rebase"). /// Empty means only default merge is supported. @@ -246,7 +245,6 @@ pub struct VcsCaps { pub merge_strategies: Vec, } - /// Callback function type for handling VCS events. pub type OnEvent = Arc; From ac9501be3668658845deace2f4ba3d026d058133 Mon Sep 17 00:00:00 2001 From: Jordon Date: Thu, 4 Jun 2026 11:41:14 +0100 Subject: [PATCH 7/7] Fixed identified issues --- Backend/tests/tauri_commands/branches.rs | 9 ++++-- Frontend/src/scripts/features/branches.ts | 2 +- .../src/scripts/features/mergeStrategy.ts | 28 +++++++++++++------ .../tests/scripts/features/branches.test.ts | 2 +- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/Backend/tests/tauri_commands/branches.rs b/Backend/tests/tauri_commands/branches.rs index d3c871dc..52b667d9 100644 --- a/Backend/tests/tauri_commands/branches.rs +++ b/Backend/tests/tauri_commands/branches.rs @@ -279,7 +279,7 @@ fn vcs_create_branch_propagates_error() { let (app, _) = build_vcs_branches_app(); let wv = test_webview(&app); - let body = tauri::ipc::InvokeBody::Json(serde_json::json!({"name": "new-branch", "base": "main"})); + let body = tauri::ipc::InvokeBody::Json(serde_json::json!({"name": "new-branch", "from": "main"})); let res = invoke_cmd(&wv, "vcs_create_branch", body); assert!(res.is_err(), "create should fail (unsupported)"); } @@ -311,7 +311,10 @@ fn vcs_merge_context_fails_silently_when_not_in_progress() { let wv = test_webview(&app); let res = invoke_cmd(&wv, "vcs_merge_context", tauri::ipc::InvokeBody::default()); - let _ = res; + assert!(res.is_ok(), "merge context should succeed even when not in progress"); + let value = res.unwrap(); + let ctx = value.deserialize::().expect("should deserialize"); + assert_eq!(ctx.get("in_progress").and_then(|v| v.as_bool()), Some(false), "should report not in progress"); } #[test] @@ -320,7 +323,7 @@ fn vcs_set_upstream_propagates_error() { let (app, _) = build_vcs_branches_app(); let wv = test_webview(&app); - let body = tauri::ipc::InvokeBody::Json(serde_json::json!({"name": "main", "upstream": "origin/main"})); + let body = tauri::ipc::InvokeBody::Json(serde_json::json!({"branch": "main", "upstream": "origin/main"})); let res = invoke_cmd(&wv, "vcs_set_upstream", body); assert!(res.is_err(), "set upstream should fail (unsupported)"); } diff --git a/Frontend/src/scripts/features/branches.ts b/Frontend/src/scripts/features/branches.ts index 12ee71ef..ba7d49e4 100644 --- a/Frontend/src/scripts/features/branches.ts +++ b/Frontend/src/scripts/features/branches.ts @@ -215,7 +215,7 @@ export function bindBranchUI() { if (name === cur) { notify('Cannot merge a branch into itself'); return; } const strategies = await TAURI.invoke('vcs_merge_strategies').catch(() => []); const hasAdvanced = strategies.some(s => s === 'squash' || s === 'rebase'); - const strategy = hasAdvanced ? await promptMergeStrategy(name, cur) : (await confirmBool(`Merge '${name}' into '${cur}'?`) ? 'merge' : null); + const strategy = hasAdvanced ? await promptMergeStrategy(name, cur, strategies) : (await confirmBool(`Merge '${name}' into '${cur}'?`) ? 'merge' : null); if (!strategy) return; const statusEl = document.getElementById('status'); diff --git a/Frontend/src/scripts/features/mergeStrategy.ts b/Frontend/src/scripts/features/mergeStrategy.ts index ab4e8955..f929d570 100644 --- a/Frontend/src/scripts/features/mergeStrategy.ts +++ b/Frontend/src/scripts/features/mergeStrategy.ts @@ -33,9 +33,16 @@ function wireMergeStrategyModal() { } }); } + + // Single persistent listener for all dismiss paths — never leaks + modal.addEventListener('modal:closed', () => resolvePending(null)); } -export function promptMergeStrategy(branchName: string, targetBranch: string): Promise { +export function promptMergeStrategy( + branchName: string, + targetBranch: string, + supported: string[], +): Promise { hydrate('merge-strategy-modal'); wireMergeStrategyModal(); @@ -45,18 +52,21 @@ export function promptMergeStrategy(branchName: string, targetBranch: string): P hintEl.textContent = `Choose how to merge '${branchName}' into '${targetBranch}'.`; } + // Show only options for strategies the backend supports + const options = modal?.querySelectorAll('.merge-strategy-option'); + if (options) { + const supportedSet = new Set(supported); + for (const opt of options) { + const strat = opt.getAttribute('data-strategy'); + const visible = strat ? supportedSet.has(strat) : false; + opt.style.display = visible ? '' : 'none'; + } + } + return new Promise((resolve) => { const prev = pendingResolve; pendingResolve = resolve; if (prev) prev(null); - - // Resolve with null on any dismiss path (backdrop click, cancel, Escape) - const onClosed = () => { - resolvePending(null); - modal?.removeEventListener('modal:closed', onClosed); - }; - modal?.addEventListener('modal:closed', onClosed); - openModal('merge-strategy-modal'); }); } diff --git a/Frontend/tests/scripts/features/branches.test.ts b/Frontend/tests/scripts/features/branches.test.ts index b0cb550d..fbfdb1d3 100644 --- a/Frontend/tests/scripts/features/branches.test.ts +++ b/Frontend/tests/scripts/features/branches.test.ts @@ -409,7 +409,7 @@ it('merge into current', async () => { const items = await triggerContextMenu(); await items[1].action(); - expect(mockPromptMergeStrategy).toHaveBeenCalledWith('feature', 'main'); + expect(mockPromptMergeStrategy).toHaveBeenCalledWith('feature', 'main', ['merge', 'squash', 'rebase']); expect(mockInvoke).toHaveBeenCalledWith('vcs_merge_branch', { name: 'feature', strategy: 'merge' }); expect(mockNotify).toHaveBeenCalledWith("Merged branch 'feature' into 'main'"); });