From 898e15a899f027db9a802506cad92b6306ff61b2 Mon Sep 17 00:00:00 2001 From: stocky789 Date: Wed, 20 May 2026 08:14:30 +0000 Subject: [PATCH] fix(workspace): restore branch changes on switch --- api/routes.py | 4 ++ api/workspace_git.py | 80 ++++++++++++++++++++++++++++++--- docs/workspace-git.md | 89 +++++++++++++++++++++++++++++++++++++ tests/test_workspace_git.py | 63 +++++++++++++++++++++++++- 4 files changed, 229 insertions(+), 7 deletions(-) create mode 100644 docs/workspace-git.md diff --git a/api/routes.py b/api/routes.py index fcbae668..517e1aaf 100644 --- a/api/routes.py +++ b/api/routes.py @@ -9190,6 +9190,10 @@ def _handle_git_stash_checkout(handler, body): "message": result.get("message", ""), "stash_name": result.get("stash_name", ""), "stashed": bool(result.get("stashed")), + "restored_stash": result.get("restored_stash"), + "restore_failed": bool(result.get("restore_failed")), + "restore_error": result.get("restore_error", ""), + "restore_stash": result.get("restore_stash"), }, ) except ValueError as e: diff --git a/api/workspace_git.py b/api/workspace_git.py index 705bb81c..9631056e 100644 --- a/api/workspace_git.py +++ b/api/workspace_git.py @@ -28,6 +28,7 @@ DIFF_SIZE_LIMIT = 512 * 1024 COMMIT_MESSAGE_DIFF_LIMIT = 64 * 1024 WORKSPACE_GIT_DESTRUCTIVE_ENV = "HERMES_WEBUI_WORKSPACE_GIT_DESTRUCTIVE" _GIT_ENV_SCRUB_KEYS = ("GIT_DIR", "GIT_WORK_TREE", "GIT_CONFIG_GLOBAL") +_HERMES_BRANCH_SWITCH_STASH_PREFIX = "hermes-webui branch switch" def workspace_git_destructive_enabled() -> bool: @@ -622,6 +623,60 @@ def _dirty_worktree(ctx: GitContext) -> bool: return bool(result.stdout.strip()) +def _current_checkout_label(ctx: GitContext) -> str: + branch = _run_git(ctx, ["branch", "--show-current"], check=False).stdout.strip() + if branch: + return branch + return _run_git(ctx, ["rev-parse", "--short", "HEAD"], check=True).stdout.strip() or "HEAD" + + +def _stash_subject_parts(subject: str) -> tuple[str, str] | None: + subject = str(subject or "").strip() + if not subject.startswith("On ") or ": " not in subject: + return None + branch, message = subject[3:].split(": ", 1) + branch = branch.strip() + message = message.strip() + if not branch or not message.startswith(_HERMES_BRANCH_SWITCH_STASH_PREFIX): + return None + return branch, message + + +def _hermes_branch_switch_stashes(ctx: GitContext) -> list[dict]: + result = _run_git(ctx, ["stash", "list", "--format=%gd%x00%gs"], check=False) + if result.returncode != 0: + return [] + stashes = [] + for line in result.stdout.splitlines(): + try: + ref, subject = line.split("\0", 1) + except ValueError: + continue + parts = _stash_subject_parts(subject) + if not parts: + continue + branch, message = parts + stashes.append({"ref": ref, "branch": branch, "message": message}) + return stashes + + +def _restore_branch_switch_stash_locked(ctx: GitContext, branch: str) -> dict: + if _dirty_worktree(ctx): + return {} + for item in _hermes_branch_switch_stashes(ctx): + if item.get("branch") != branch: + continue + result = _run_git(ctx, ["stash", "pop", "--index", item["ref"]], check=False) + if result.returncode == 0: + return {"restored_stash": item} + return { + "restore_failed": True, + "restore_error": (result.stderr or result.stdout or "Git stash restore failed").strip(), + "restore_stash": item, + } + return {} + + def _validate_checkout_request_locked( ctx: GitContext, ref: str, @@ -729,15 +784,24 @@ def git_stash_and_checkout( if ctx is None: raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") mode = str(mode or "local").strip().lower() - stash_name = f"hermes-webui branch switch {ref}".strip() + target_label = str(new_branch or ref or "HEAD").strip() or "HEAD" + stash_name = f"{_HERMES_BRANCH_SWITCH_STASH_PREFIX} to {target_label}".strip() + restored: dict = {} with _git_mutation_lock(ctx): _validate_checkout_request_locked(ctx, ref, mode, new_branch) stashed = False - stash_result = _run_git(ctx, ["stash", "push", "-u", "-m", stash_name], check=True) - stash_text = _remote_message(stash_result) - if "No local changes to save" not in stash_text: - stashed = True - result = _perform_checkout_locked(ctx, workspace, ref, mode, new_branch, track) + if _dirty_worktree(ctx): + stash_result = _run_git(ctx, ["stash", "push", "-u", "-m", stash_name], check=True) + stash_text = _remote_message(stash_result) + stashed = "No local changes to save" not in stash_text + try: + result = _perform_checkout_locked(ctx, workspace, ref, mode, new_branch, track) + except Exception: + if stashed: + _run_git(ctx, ["stash", "pop", "--index", "stash@{0}"], check=False) + raise + current_branch = _current_checkout_label(ctx) + restored = _restore_branch_switch_stash_locked(ctx, current_branch) status = git_status(workspace) branches = git_branches(workspace) return { @@ -745,6 +809,10 @@ def git_stash_and_checkout( "message": _remote_message(result), "stash_name": stash_name if stashed else "", "stashed": stashed, + "restored_stash": restored.get("restored_stash"), + "restore_failed": bool(restored.get("restore_failed")), + "restore_error": restored.get("restore_error", ""), + "restore_stash": restored.get("restore_stash"), "current_branch": branches.get("current"), "status": status, "branches": branches, diff --git a/docs/workspace-git.md b/docs/workspace-git.md new file mode 100644 index 00000000..9d551d76 --- /dev/null +++ b/docs/workspace-git.md @@ -0,0 +1,89 @@ +# Workspace Git controls + +Workspace Git controls let the browser inspect Git state for the active session workspace. By default, +WebUI can read status, list branches, show diffs, fetch remote refs, and generate commit-message +suggestions. Actions that modify the repository, index, or worktree are disabled unless the WebUI +process is started with `HERMES_WEBUI_WORKSPACE_GIT_DESTRUCTIVE=1`. + +> **Trust model - read this first.** Once mutating Git actions are enabled, a browser action can run +> Git commands inside a mounted workspace. Some Git commands can also run repository hook code from +> `.git/hooks/` or a configured `core.hooksPath`. That hook code runs as the WebUI process user, with +> the WebUI process permissions and environment. + +## What works by default + +Without `HERMES_WEBUI_WORKSPACE_GIT_DESTRUCTIVE=1`, WebUI can: + +- show repository status +- list branches +- show file diffs +- fetch from the configured remote +- generate commit-message suggestions + +Fetch may update remote-tracking refs, but it does not change the worktree, merge branches, create +commits, or push changes. + +Commit-message generation may send staged or selected diff context to the configured model provider. +The UI labels this before generation. + +Diffs for untracked files are size checked before WebUI reads file contents. Large or binary files +return metadata instead of inline diff text. + +## What requires explicit enablement + +These actions are blocked unless `HERMES_WEBUI_WORKSPACE_GIT_DESTRUCTIVE=1` is set: + +- stage and unstage +- discard changes +- commit and selected-file commit +- pull and push +- checkout +- branch switching that parks and restores local changes + +Leave the flag unset for deployments where WebUI should only inspect mounted workspaces. Set it only +when browser users are trusted to modify those repositories from WebUI. + +When the flag is enabled, browser branch switching is designed to be uninterrupted. If the branch +being left has local changes, WebUI parks those changes in a WebUI-owned Git stash, switches branches, +and then restores any WebUI-owned stash for the branch being entered. If Git cannot restore the stash +cleanly, WebUI leaves the stash in place and reports the restore failure instead of dropping it. + +## Workspace and path scope + +The browser does not send an arbitrary repository path. Git requests carry a session id and, when +needed, workspace-relative file paths. The server resolves the session workspace, checks each path +against that workspace, and then builds Git pathspecs from the checked paths. + +Git commands run through `subprocess.run` with `shell=False`. Local status and diff commands use a 5 +second timeout. Remote operations such as fetch, pull, and push use a 60 second timeout. + +Before any Git subprocess starts, WebUI removes inherited `GIT_DIR`, `GIT_WORK_TREE`, and +`GIT_CONFIG_GLOBAL` values from the environment. Those variables can redirect Git to a different +repository or config file, so WebUI does not trust them from the parent process. + +`GIT_INDEX_FILE` is the intentional exception. Selected-file commits use a temporary index so WebUI +can commit only the requested files, then remove the temporary index afterward. + +## Coordination with active runs + +Mutating Git actions are rejected while the same session has a live stream. The API returns +`active_stream` instead of racing a running agent that may be writing files in the same workspace. + +Mutating Git actions also take a per-repository lock. If another Git mutation is already running for +that repository, the API returns `operation_in_progress`. + +## Hook and remote behavior + +WebUI does not bypass Git hooks. Hook code may come from `.git/hooks/` or from a configured +`core.hooksPath`. + +Commit actions run normal commit hooks such as `pre-commit`, `commit-msg`, and `post-commit`. Push can +run `pre-push`. Pull uses Git's normal behavior too; with `--ff-only` it does not create a merge +commit, but it is still a Git operation running under the WebUI process. + +Pull uses `--ff-only`. Push keeps Git's normal non-fast-forward protection and reports +non-fast-forward rejection separately from general Git failures. + +If a hook fails, the API returns a structured Git error instead of hiding the failure. Other classified +failures include authentication errors, missing upstream branches, conflicts, dirty worktrees, invalid +refs, missing Git binaries, and timeouts. diff --git a/tests/test_workspace_git.py b/tests/test_workspace_git.py index d0dea59b..531f605a 100644 --- a/tests/test_workspace_git.py +++ b/tests/test_workspace_git.py @@ -615,7 +615,68 @@ def test_git_stash_and_checkout_is_explicit(tmp_path): assert result["stash_name"].startswith("hermes-webui branch switch") assert result["current_branch"] == "target" assert git_status(repo)["totals"]["changed"] == 0 - assert "hermes-webui branch switch target" in _git(repo, "stash", "list") + assert "hermes-webui branch switch to target" in _git(repo, "stash", "list") + + +def test_git_stash_and_checkout_restores_branch_changes_when_returning(tmp_path): + from api.workspace_git import git_stash_and_checkout, git_status + + repo = _init_repo(tmp_path / "repo") + _git(repo, "branch", "-M", "main") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + _git(repo, "checkout", "-b", "feature") + _git(repo, "checkout", "main") + + (repo / "tracked.txt").write_text("main dirty\n", encoding="utf-8") + (repo / "main-only.txt").write_text("untracked on main\n", encoding="utf-8") + + to_feature = git_stash_and_checkout(repo, "feature", "local") + assert to_feature["ok"] is True + assert to_feature["stashed"] is True + assert to_feature["current_branch"] == "feature" + assert git_status(repo)["totals"]["changed"] == 0 + assert not (repo / "main-only.txt").exists() + + (repo / "feature-only.txt").write_text("untracked on feature\n", encoding="utf-8") + to_main = git_stash_and_checkout(repo, "main", "local") + + assert to_main["ok"] is True + assert to_main["stashed"] is True + assert to_main["current_branch"] == "main" + assert to_main["restored_stash"]["branch"] == "main" + assert (repo / "tracked.txt").read_text(encoding="utf-8") == "main dirty\n" + assert (repo / "main-only.txt").read_text(encoding="utf-8") == "untracked on main\n" + assert not (repo / "feature-only.txt").exists() + stash_list = _git(repo, "stash", "list") + assert "On main: hermes-webui branch switch" not in stash_list + assert "On feature: hermes-webui branch switch" in stash_list + + +def test_git_stash_and_checkout_reports_restore_conflicts_without_dropping_stash(tmp_path): + from api.workspace_git import git_stash_and_checkout + + repo = _init_repo(tmp_path / "repo") + _git(repo, "branch", "-M", "main") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + _git(repo, "checkout", "-b", "feature") + _git(repo, "checkout", "main") + (repo / "tracked.txt").write_text("main dirty\n", encoding="utf-8") + + git_stash_and_checkout(repo, "feature", "local") + _git(repo, "checkout", "main") + (repo / "tracked.txt").write_text("main changed while parked\n", encoding="utf-8") + _commit_all(repo, "advance main") + _git(repo, "checkout", "feature") + + result = git_stash_and_checkout(repo, "main", "local") + + assert result["ok"] is True + assert result["current_branch"] == "main" + assert result["restore_failed"] is True + assert result["restore_stash"]["branch"] == "main" + assert "On main: hermes-webui branch switch" in _git(repo, "stash", "list") def test_git_stash_checkout_validates_before_stashing(tmp_path):