From 5fc7aee78187a71283be22577c3b977a88fb0916 Mon Sep 17 00:00:00 2001 From: stocky789 Date: Wed, 20 May 2026 01:20:46 +0000 Subject: [PATCH 01/10] feat(workspace): add backend Git operations --- api/routes.py | 549 +++++++++++- api/workspace.py | 41 +- api/workspace_git.py | 1158 +++++++++++++++++++++++++ tests/conftest.py | 1 + tests/test_workspace_dir_signature.py | 26 + tests/test_workspace_git.py | 796 +++++++++++++++++ 6 files changed, 2566 insertions(+), 5 deletions(-) create mode 100644 api/workspace_git.py create mode 100644 tests/test_workspace_dir_signature.py create mode 100644 tests/test_workspace_git.py diff --git a/api/routes.py b/api/routes.py index 063226bf..fcbae668 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2221,6 +2221,7 @@ from api.workspace import ( get_last_workspace, set_last_workspace, list_dir, + dir_signature, list_workspace_suggestions, read_file_content, safe_resolve_ws, @@ -4139,6 +4140,15 @@ def handle_get(handler, parsed) -> bool: if parsed.path == "/api/list": return _handle_list_dir(handler, parsed) + if parsed.path == "/api/git/status": + return _handle_git_status(handler, parsed) + + if parsed.path == "/api/git/branches": + return _handle_git_branches(handler, parsed) + + if parsed.path == "/api/git/diff": + return _handle_git_diff(handler, parsed) + if parsed.path == "/api/personalities": # Read personalities from config.yaml agent.personalities section # (matches hermes-agent CLI behavior, not filesystem SOUL.md approach) @@ -4170,9 +4180,22 @@ def handle_get(handler, parsed) -> bool: s = get_session(sid) except KeyError: return bad(handler, "Session not found", 404) - from api.workspace import git_info_for_workspace + from api.workspace_git import GitWorkspaceError, git_status - info = git_info_for_workspace(Path(s.workspace)) + try: + status = git_status(Path(s.workspace)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + totals = status.get("totals") or {} + info = None if not status.get("is_git") else { + "branch": status.get("branch"), + "dirty": totals.get("changed", 0), + "modified": (totals.get("staged", 0) or 0) + (totals.get("unstaged", 0) or 0), + "untracked": totals.get("untracked", 0), + "ahead": status.get("ahead", 0), + "behind": status.get("behind", 0), + "is_git": True, + } return j(handler, {"git": info}) if parsed.path == "/api/commands": @@ -5311,6 +5334,43 @@ def handle_post(handler, parsed) -> bool: with cron_profile_context(): return _handle_cron_resume(handler, body) + # ── Git workspace ops (POST) ── + if parsed.path == "/api/git/stage": + return _handle_git_stage(handler, body) + + if parsed.path == "/api/git/unstage": + return _handle_git_unstage(handler, body) + + if parsed.path == "/api/git/discard": + return _handle_git_discard(handler, body) + + if parsed.path == "/api/git/commit-message": + return _handle_git_commit_message(handler, body) + + if parsed.path == "/api/git/commit-message-selected": + return _handle_git_commit_message_selected(handler, body) + + if parsed.path == "/api/git/commit": + return _handle_git_commit(handler, body) + + if parsed.path == "/api/git/commit-selected": + return _handle_git_commit_selected(handler, body) + + if parsed.path == "/api/git/fetch": + return _handle_git_remote_action(handler, body, "fetch") + + if parsed.path == "/api/git/pull": + return _handle_git_remote_action(handler, body, "pull") + + if parsed.path == "/api/git/push": + return _handle_git_remote_action(handler, body, "push") + + if parsed.path == "/api/git/checkout": + return _handle_git_checkout(handler, body) + + if parsed.path == "/api/git/stash-checkout": + return _handle_git_stash_checkout(handler, body) + # ── File ops (POST) ── if parsed.path == "/api/file/delete": return _handle_file_delete(handler, body) @@ -6185,11 +6245,14 @@ def _handle_list_dir(handler, parsed): except Exception: return bad(handler, "Session not found", 404) try: + rel_path = qs.get("path", ["."])[0] + entries = list_dir(Path(workspace), rel_path) return j( handler, { - "entries": list_dir(Path(workspace), qs.get("path", ["."])[0]), - "path": qs.get("path", ["."])[0], + "entries": entries, + "signature": dir_signature(Path(workspace), rel_path, entries), + "path": rel_path, }, ) except (FileNotFoundError, ValueError) as e: @@ -8657,6 +8720,484 @@ def _handle_cron_resume(handler, body): return bad(handler, "Job not found", 404) +def _git_session(handler, session_id: str): + if not session_id: + bad(handler, "session_id required") + return None + try: + return get_session(session_id) + except KeyError: + bad(handler, "Session not found", 404) + return None + + +def _git_session_workspace(handler, session_id: str): + session = _git_session(handler, session_id) + if session is None: + return None + return Path(session.workspace) + + +def _git_session_and_workspace(handler, session_id: str): + session = _git_session(handler, session_id) + if session is None: + return None, None + return session, Path(session.workspace) + + +def _git_locked_by_active_stream(session) -> bool: + stream_id = getattr(session, "active_stream_id", None) + if not stream_id: + return False + try: + from api.config import STREAMS, STREAMS_LOCK + + with STREAMS_LOCK: + return stream_id in STREAMS + except Exception: + return False + + +def _git_reject_destructive_if_unsafe(handler, session) -> bool: + from api.workspace_git import ( + GitWorkspaceError, + WORKSPACE_GIT_DESTRUCTIVE_ENV, + workspace_git_destructive_enabled, + ) + + if not workspace_git_destructive_enabled(): + _git_bad( + handler, + GitWorkspaceError( + f"Destructive workspace Git operations are disabled. Set {WORKSPACE_GIT_DESTRUCTIVE_ENV}=1 to enable them.", + "destructive_git_disabled", + ), + status=403, + ) + return True + if _git_locked_by_active_stream(session): + _git_bad( + handler, + GitWorkspaceError( + "A session run is active. Wait for it to finish before running this Git operation.", + "active_stream", + ), + status=409, + ) + return True + return False + + +def _handle_git_status(handler, parsed): + qs = parse_qs(parsed.query) + workspace = _git_session_workspace(handler, qs.get("session_id", [""])[0]) + if workspace is None: + return True + try: + from api.workspace_git import GitWorkspaceError, git_status + + return j(handler, {"git": git_status(workspace)}) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _handle_git_branches(handler, parsed): + qs = parse_qs(parsed.query) + workspace = _git_session_workspace(handler, qs.get("session_id", [""])[0]) + if workspace is None: + return True + try: + from api.workspace_git import GitWorkspaceError, git_branches + + return j(handler, {"branches": git_branches(workspace)}) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _handle_git_diff(handler, parsed): + qs = parse_qs(parsed.query) + workspace = _git_session_workspace(handler, qs.get("session_id", [""])[0]) + if workspace is None: + return True + path = qs.get("path", [""])[0] + kind = qs.get("kind", ["unstaged"])[0] + if not path: + return bad(handler, "path required") + try: + from api.workspace_git import GitWorkspaceError, git_diff + + return j(handler, {"diff": git_diff(workspace, path, kind)}) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _git_bad(handler, err, status: int = 400): + return j( + handler, + { + "error": _sanitize_error(err), + "code": getattr(err, "code", "git_failed") or "git_failed", + }, + status=status, + ) + + +def _git_paths_from_body(body) -> list[str]: + raw_paths = body.get("paths") + if raw_paths is None and body.get("path"): + raw_paths = [body.get("path")] + if isinstance(raw_paths, str): + raw_paths = [raw_paths] + if not isinstance(raw_paths, list): + raise ValueError("paths must be a list") + return [str(path) for path in raw_paths] + + +def _handle_git_stage(handler, body): + try: + require(body, "session_id") + paths = _git_paths_from_body(body) + session, workspace = _git_session_and_workspace(handler, body["session_id"]) + if workspace is None: + return True + if _git_reject_destructive_if_unsafe(handler, session): + return True + from api.workspace_git import GitWorkspaceError, git_stage + + return j(handler, {"ok": True, "git": git_stage(workspace, paths)}) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _handle_git_unstage(handler, body): + try: + require(body, "session_id") + paths = _git_paths_from_body(body) + session, workspace = _git_session_and_workspace(handler, body["session_id"]) + if workspace is None: + return True + if _git_reject_destructive_if_unsafe(handler, session): + return True + from api.workspace_git import GitWorkspaceError, git_unstage + + return j(handler, {"ok": True, "git": git_unstage(workspace, paths)}) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _handle_git_discard(handler, body): + try: + require(body, "session_id") + paths = _git_paths_from_body(body) + session, workspace = _git_session_and_workspace(handler, body["session_id"]) + if workspace is None: + return True + if _git_reject_destructive_if_unsafe(handler, session): + return True + from api.workspace_git import GitWorkspaceError, git_discard + + return j( + handler, + { + "ok": True, + "git": git_discard( + workspace, + paths, + delete_untracked=bool(body.get("delete_untracked")), + ), + }, + ) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _llm_git_commit_message(system_prompt: str, user_prompt: str, session=None) -> str: + from api import profiles as profiles_api + + active_profile = profiles_api.get_active_profile_name() or "default" + with profiles_api.profile_env_for_background_worker( + active_profile, + "git commit message", + logger_override=logger, + ): + from api.config import ( + get_effective_default_model, + model_with_provider_context, + resolve_custom_provider_connection, + resolve_model_provider, + ) + + session_model = str(getattr(session, "model", "") or "").strip() + session_provider = str(getattr(session, "model_provider", "") or "").strip() or None + model_for_resolution = ( + model_with_provider_context(session_model, session_provider) + if session_model + else get_effective_default_model() + ) + _main_model, _main_provider, _main_base_url = resolve_model_provider(model_for_resolution) + _main_api_key = None + try: + from api.oauth import resolve_runtime_provider_with_anthropic_env_lock + from hermes_cli.runtime_provider import resolve_runtime_provider + + _rt = resolve_runtime_provider_with_anthropic_env_lock( + resolve_runtime_provider, + requested=_main_provider, + ) + _main_api_key = _rt.get("api_key") + if not _main_provider: + _main_provider = _rt.get("provider") + if not _main_base_url: + _main_base_url = _rt.get("base_url") + except Exception as _e: + logger.debug("git commit message runtime provider resolution failed: %s", _e) + if isinstance(_main_provider, str) and _main_provider.startswith("custom:"): + _cp_key, _cp_base = resolve_custom_provider_connection(_main_provider) + if not _main_api_key and _cp_key: + _main_api_key = _cp_key + if not _main_base_url and _cp_base: + _main_base_url = _cp_base + + messages = [ + {"role": "system", "content": system_prompt}, + {"role": "user", "content": user_prompt}, + ] + main_runtime = { + "provider": _main_provider, + "model": _main_model, + "base_url": _main_base_url, + "api_key": _main_api_key, + } + try: + from agent.auxiliary_client import get_text_auxiliary_client + + aux_client, aux_model = get_text_auxiliary_client( + "compression", + main_runtime=main_runtime, + ) + if aux_client is not None and aux_model: + response = aux_client.chat.completions.create( + model=aux_model, + messages=messages, + ) + return str(response.choices[0].message.content or "").strip() + except Exception as _e: + logger.debug("git commit message auxiliary model failed; falling back to main model: %s", _e) + + from run_agent import AIAgent + + agent = AIAgent( + model=_main_model, + provider=_main_provider, + base_url=_main_base_url, + api_key=_main_api_key, + platform="webui", + quiet_mode=True, + enabled_toolsets=[], + session_id=f"git-commit-message-{uuid.uuid4().hex[:8]}", + ) + result = agent.run_conversation( + user_message=user_prompt, + system_message=system_prompt, + conversation_history=[], + task_id=f"git-commit-message-{uuid.uuid4().hex[:8]}", + ) + return str(result.get("final_response") or "").strip() + + +def _handle_git_commit_message(handler, body): + from api.workspace_git import ( + GitWorkspaceError, + clean_generated_commit_message, + staged_commit_message_prompt, + ) + + try: + require(body, "session_id") + session = get_session(body["session_id"]) + workspace = Path(session.workspace) + + prompt = staged_commit_message_prompt(workspace) + message = clean_generated_commit_message( + _llm_git_commit_message(prompt["system_prompt"], prompt["user_prompt"], session=session) + ) + if not message: + raise GitWorkspaceError("No commit message was generated") + return j(handler, {"ok": True, "message": message, "truncated": bool(prompt.get("truncated"))}) + except KeyError: + return bad(handler, "Session not found", 404) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + except Exception as e: + logger.exception("git commit message generation failed") + return bad(handler, _sanitize_error(e), 500) + + +def _handle_git_commit_message_selected(handler, body): + from api.workspace_git import ( + GitWorkspaceError, + clean_generated_commit_message, + selected_commit_message_prompt, + ) + + try: + require(body, "session_id") + paths = _git_paths_from_body(body) + session = get_session(body["session_id"]) + workspace = Path(session.workspace) + + prompt = selected_commit_message_prompt(workspace, paths) + message = clean_generated_commit_message( + _llm_git_commit_message(prompt["system_prompt"], prompt["user_prompt"], session=session) + ) + if not message: + raise GitWorkspaceError("No commit message was generated") + return j(handler, {"ok": True, "message": message, "truncated": bool(prompt.get("truncated"))}) + except KeyError: + return bad(handler, "Session not found", 404) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + except Exception as e: + logger.exception("selected git commit message generation failed") + return bad(handler, _sanitize_error(e), 500) + + +def _handle_git_commit(handler, body): + try: + require(body, "session_id", "message") + session, workspace = _git_session_and_workspace(handler, body["session_id"]) + if workspace is None: + return True + if _git_reject_destructive_if_unsafe(handler, session): + return True + from api.workspace_git import GitWorkspaceError, git_commit + + return j(handler, git_commit(workspace, body.get("message", ""))) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _handle_git_commit_selected(handler, body): + try: + require(body, "session_id", "message") + paths = _git_paths_from_body(body) + session, workspace = _git_session_and_workspace(handler, body["session_id"]) + if workspace is None: + return True + if _git_reject_destructive_if_unsafe(handler, session): + return True + from api.workspace_git import GitWorkspaceError, git_commit_selected + + return j(handler, git_commit_selected(workspace, body.get("message", ""), paths)) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _handle_git_remote_action(handler, body, action: str): + try: + require(body, "session_id") + session, workspace = _git_session_and_workspace(handler, body["session_id"]) + if workspace is None: + return True + if action in {"pull", "push"} and _git_reject_destructive_if_unsafe(handler, session): + return True + from api.workspace_git import GitWorkspaceError, git_fetch, git_pull, git_push + + actions = { + "fetch": git_fetch, + "pull": git_pull, + "push": git_push, + } + return j(handler, actions[action](workspace)) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _handle_git_checkout(handler, body): + try: + require(body, "session_id", "ref", "mode") + session, workspace = _git_session_and_workspace(handler, body["session_id"]) + if workspace is None: + return True + if _git_reject_destructive_if_unsafe(handler, session): + return True + from api.workspace_git import GitWorkspaceError, git_checkout + + result = git_checkout( + workspace, + str(body.get("ref", "")), + str(body.get("mode", "local")), + new_branch=body.get("new_branch"), + track=bool(body.get("track")), + dirty_mode=str(body.get("dirty_mode", "block")), + ) + return j( + handler, + { + "ok": True, + "git": result.get("status"), + "branches": result.get("branches"), + "current_branch": result.get("current_branch"), + "message": result.get("message", ""), + }, + ) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + +def _handle_git_stash_checkout(handler, body): + try: + require(body, "session_id", "ref", "mode") + session, workspace = _git_session_and_workspace(handler, body["session_id"]) + if workspace is None: + return True + if _git_reject_destructive_if_unsafe(handler, session): + return True + from api.workspace_git import GitWorkspaceError, git_stash_and_checkout + + result = git_stash_and_checkout( + workspace, + str(body.get("ref", "")), + str(body.get("mode", "local")), + new_branch=body.get("new_branch"), + track=bool(body.get("track")), + ) + return j( + handler, + { + "ok": True, + "git": result.get("status"), + "branches": result.get("branches"), + "current_branch": result.get("current_branch"), + "message": result.get("message", ""), + "stash_name": result.get("stash_name", ""), + "stashed": bool(result.get("stashed")), + }, + ) + except ValueError as e: + return bad(handler, str(e)) + except GitWorkspaceError as e: + return _git_bad(handler, e) + + def _handle_file_delete(handler, body): try: require(body, "session_id", "path") diff --git a/api/workspace.py b/api/workspace.py index 97c768f6..362b9e87 100644 --- a/api/workspace.py +++ b/api/workspace.py @@ -7,6 +7,7 @@ profile has its own workspace configuration. State files live at ``{profile_home}/webui_state/last_workspace.txt``. The global STATE_DIR paths are used as fallback when no profile module is available. """ +import hashlib import json import logging import os @@ -714,12 +715,18 @@ def list_dir(workspace: Path, rel: str='.'): display_path = str(Path(item.name)) if rel and rel != '.': display_path = rel + '/' + display_path + try: + item_stat = item.lstat() + mtime_ns = item_stat.st_mtime_ns + except OSError: + mtime_ns = None entry = { 'name': item.name, 'path': display_path, 'type': 'symlink', 'target': str(link_target), 'is_dir': is_dir, + 'mtime_ns': mtime_ns, } if not is_dir: try: @@ -733,17 +740,49 @@ def list_dir(workspace: Path, rel: str='.'): entry_path = item.name if rel and rel != '.': entry_path = rel + '/' + item.name + try: + item_stat = item.stat() + size = item_stat.st_size if item.is_file() else None + mtime_ns = item_stat.st_mtime_ns + except OSError: + size = None + mtime_ns = None entries.append({ 'name': item.name, 'path': entry_path, 'type': 'dir' if item.is_dir() else 'file', - 'size': item.stat().st_size if item.is_file() else None, + 'size': size, + 'mtime_ns': mtime_ns, }) if len(entries) >= 200: break return entries +def dir_signature(workspace: Path, rel: str = '.', entries: list[dict] | None = None) -> str: + """Return a cheap, stable signature for a listed workspace directory. + + The signature is based only on bounded directory-entry metadata already used + by the workspace tree: names, displayed paths, entry type, file sizes, + mtimes, and symlink targets. It intentionally does not read file contents. + """ + if entries is None: + entries = list_dir(workspace, rel) + payload = [] + for entry in entries: + payload.append({ + 'name': entry.get('name'), + 'path': entry.get('path'), + 'type': entry.get('type'), + 'is_dir': entry.get('is_dir'), + 'size': entry.get('size'), + 'mtime_ns': entry.get('mtime_ns'), + 'target': entry.get('target'), + }) + raw = json.dumps(payload, sort_keys=True, separators=(',', ':'), ensure_ascii=False) + return hashlib.sha256(raw.encode('utf-8')).hexdigest() + + def read_file_content(workspace: Path, rel: str) -> dict: target = safe_resolve_ws(workspace, rel) if not target.is_file(): diff --git a/api/workspace_git.py b/api/workspace_git.py new file mode 100644 index 00000000..fbef17f6 --- /dev/null +++ b/api/workspace_git.py @@ -0,0 +1,1158 @@ +"""Git helpers for the workspace panel. + +The browser only sends session ids and workspace-relative paths. This module +resolves the active workspace server-side, scopes paths before they become Git +pathspecs, and keeps all Git subprocess calls shell-free and bounded. +""" + +from __future__ import annotations + +import difflib +import os +import shutil +import subprocess +import tempfile +import threading +from contextlib import contextmanager +from dataclasses import dataclass +from pathlib import Path +from typing import Iterable + +from api.workspace import safe_resolve_ws + + +GIT_TIMEOUT = 5 +GIT_REMOTE_TIMEOUT = 60 +STATUS_FILE_LIMIT = 500 +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") + + +def workspace_git_destructive_enabled() -> bool: + return os.getenv(WORKSPACE_GIT_DESTRUCTIVE_ENV, "").strip().lower() in { + "1", + "true", + "yes", + "on", + } + + +def _clean_git_env(extra: dict[str, str] | None = None) -> dict[str, str]: + env = os.environ.copy() + if extra: + env.update(extra) + for key in _GIT_ENV_SCRUB_KEYS: + env.pop(key, None) + return env + + +class GitWorkspaceError(RuntimeError): + """User-facing Git operation error.""" + + def __init__(self, message: str, code: str = "git_failed"): + super().__init__(message) + self.code = code + + +@dataclass(frozen=True) +class GitContext: + workspace: Path + repo_root: Path + workspace_prefix: str + + +_LOCKS_GUARD = threading.Lock() +_OP_LOCKS: dict[str, threading.Lock] = {} + + +@contextmanager +def _git_mutation_lock(ctx: GitContext): + key = str(ctx.repo_root) + with _LOCKS_GUARD: + lock = _OP_LOCKS.setdefault(key, threading.Lock()) + if not lock.acquire(timeout=GIT_REMOTE_TIMEOUT): + raise GitWorkspaceError("Another Git operation is still running", "operation_in_progress") + try: + yield + finally: + lock.release() + + +def _classify_git_error(message: str, args: list[str] | None = None) -> str: + text = (message or "").lower() + joined = " ".join(args or []).lower() + if "timed out" in text: + return "timeout" + if "not installed" in text or "no such file or directory: 'git'" in text: + return "missing_git" + if "not a git repository" in text: + return "not_a_repo" + if "outside the workspace" in text or "outside the git repository" in text: + return "path_outside_workspace" + if "authentication failed" in text or "permission denied" in text or "could not read username" in text: + return "auth_failed" + if "no upstream" in text or "no configured push destination" in text or "has no upstream branch" in text: + return "no_upstream" + if ( + "non-fast-forward" in text + or "fetch first" in text + or ("rejected" in text and "push" in joined) + ): + return "non_fast_forward" + if "conflict" in text or "unmerged" in text or ("merge" in text and "needs" in text): + return "conflict" + if "working tree" in text and ("clean" in text or "dirty" in text): + return "dirty_worktree" + if "local changes" in text or "would be overwritten by checkout" in text: + return "dirty_worktree" + if "invalid reference" in text or "not a valid" in text or "unknown revision" in text: + return "invalid_ref" + if "hook" in text: + return "hook_failed" + return "git_failed" + + +def _run_git( + ctx_or_cwd: GitContext | Path, + args: list[str], + *, + timeout: int = GIT_TIMEOUT, + check: bool = False, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + cwd = ctx_or_cwd.repo_root if isinstance(ctx_or_cwd, GitContext) else ctx_or_cwd + run_env = _clean_git_env(env) + try: + result = subprocess.run( + ["git", *args], + cwd=str(cwd), + shell=False, + capture_output=True, + text=True, + timeout=timeout, + env=run_env, + ) + except subprocess.TimeoutExpired as exc: + raise GitWorkspaceError("Git command timed out", "timeout") from exc + except FileNotFoundError as exc: + raise GitWorkspaceError("Git is not installed or not available on PATH", "missing_git") from exc + except OSError as exc: + raise GitWorkspaceError(str(exc), _classify_git_error(str(exc), args)) from exc + if check and result.returncode != 0: + message = (result.stderr or result.stdout or "Git command failed").strip() + raise GitWorkspaceError(message, _classify_git_error(message, args)) + return result + + +def resolve_git_context(workspace: str | Path) -> GitContext | None: + ws = Path(workspace).expanduser().resolve() + result = _run_git(ws, ["rev-parse", "--show-toplevel"], check=False) + if result.returncode != 0: + return None + repo_root = Path(result.stdout.strip()).resolve() + try: + prefix = ws.relative_to(repo_root).as_posix() + except ValueError: + return None + return GitContext(workspace=ws, repo_root=repo_root, workspace_prefix="" if prefix == "." else prefix) + + +def _workspace_pathspec(ctx: GitContext) -> str: + return ctx.workspace_prefix or "." + + +def _repo_rel(ctx: GitContext, workspace_rel: str) -> str: + try: + target = safe_resolve_ws(ctx.workspace, workspace_rel or ".") + except ValueError as exc: + raise GitWorkspaceError(str(exc), "path_outside_workspace") from exc + try: + repo_rel = target.relative_to(ctx.repo_root).as_posix() + except ValueError as exc: + raise GitWorkspaceError("Path is outside the Git repository", "path_outside_workspace") from exc + if ctx.workspace_prefix: + try: + target.relative_to(ctx.workspace) + except ValueError as exc: + raise GitWorkspaceError("Path is outside the workspace", "path_outside_workspace") from exc + return repo_rel + + +def _workspace_rel(ctx: GitContext, repo_rel: str) -> str | None: + repo_rel = repo_rel.replace("\\", "/") + if not ctx.workspace_prefix: + return repo_rel + prefix = ctx.workspace_prefix.rstrip("/") + "/" + if repo_rel == ctx.workspace_prefix: + return "." + if repo_rel.startswith(prefix): + return repo_rel[len(prefix) :] + return None + + +def _empty_status() -> dict: + return { + "changed": 0, + "staged": 0, + "unstaged": 0, + "untracked": 0, + "conflicts": 0, + } + + +def _status_code(xy: str, *, untracked: bool = False, renamed: bool = False) -> str: + if untracked: + return "??" + if xy in {"DD", "AU", "UD", "UA", "DU", "AA", "UU"}: + return xy + if renamed: + return "R" + for ch in xy: + if ch in "MADRCUT": + return ch + return xy.strip(".") or "M" + + +def _parse_numstat(text: str, ctx: GitContext) -> dict[str, tuple[int, int, bool]]: + stats: dict[str, tuple[int, int, bool]] = {} + for line in text.splitlines(): + parts = line.split("\t", 2) + if len(parts) < 3: + continue + raw_add, raw_del, raw_path = parts + binary = raw_add == "-" or raw_del == "-" + additions = 0 if binary else int(raw_add or "0") + deletions = 0 if binary else int(raw_del or "0") + workspace_path = _workspace_rel(ctx, raw_path) + if workspace_path is None: + continue + stats[workspace_path] = (additions, deletions, binary) + return stats + + +def _parse_path_list(text: str, ctx: GitContext) -> set[str]: + paths: set[str] = set() + for raw_path in text.split("\0"): + if not raw_path: + continue + workspace_path = _workspace_rel(ctx, raw_path) + if workspace_path is not None: + paths.add(workspace_path) + return paths + + +def _collect_diff_paths(ctx: GitContext, cached: bool) -> set[str] | None: + args = ["diff", "--name-only", "-z", "--ignore-cr-at-eol"] + if cached: + args.append("--cached") + args.extend(["--", _workspace_pathspec(ctx)]) + result = _run_git(ctx, args, check=False) + if result.returncode != 0: + return None + return _parse_path_list(result.stdout, ctx) + + +def _collect_numstat(ctx: GitContext, cached: bool) -> dict[str, tuple[int, int, bool]]: + args = ["diff", "--numstat", "--ignore-cr-at-eol"] + if cached: + args.append("--cached") + args.extend(["--", _workspace_pathspec(ctx)]) + result = _run_git(ctx, args, check=False) + if result.returncode != 0: + return {} + return _parse_numstat(result.stdout, ctx) + + +def _count_untracked_file(path: Path) -> tuple[int, int, bool]: + try: + if not path.is_file() or path.stat().st_size > DIFF_SIZE_LIMIT: + return 0, 0, False + except OSError: + return 0, 0, False + try: + data = path.read_bytes() + except OSError: + return 0, 0, False + if b"\0" in data: + return 0, 0, True + try: + text = data.decode("utf-8") + except UnicodeDecodeError: + return 0, 0, True + return len(text.splitlines()) or (1 if text else 0), 0, False + + +def git_status(workspace: str | Path) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + return {"is_git": False} + + result = _run_git( + ctx, + [ + "status", + "--porcelain=v2", + "-z", + "--branch", + "--ignored=matching", + "--untracked-files=all", + "--", + _workspace_pathspec(ctx), + ], + check=True, + ) + staged_stats = _collect_numstat(ctx, cached=True) + unstaged_stats = _collect_numstat(ctx, cached=False) + staged_diff_paths = _collect_diff_paths(ctx, cached=True) + unstaged_diff_paths = _collect_diff_paths(ctx, cached=False) + + branch = "" + upstream = "" + ahead = 0 + behind = 0 + files: dict[str, dict] = {} + filtered_noise = {"filemode_only": 0, "crlf_only": 0} + tokens = result.stdout.split("\0") + i = 0 + truncated = False + while i < len(tokens): + rec = tokens[i] + i += 1 + if not rec: + continue + if rec.startswith("# "): + parts = rec.split(" ", 2) + if len(parts) >= 3 and parts[1] == "branch.head": + branch = "" if parts[2] == "(detached)" else parts[2] + elif len(parts) >= 3 and parts[1] == "branch.upstream": + upstream = parts[2] + elif len(parts) >= 3 and parts[1] == "branch.ab": + for bit in parts[2].split(): + if bit.startswith("+") and bit[1:].isdigit(): + ahead = int(bit[1:]) + elif bit.startswith("-") and bit[1:].isdigit(): + behind = int(bit[1:]) + continue + + old_path = None + renamed = False + if rec.startswith("? "): + xy = "??" + repo_path = rec[2:] + untracked = True + ignored = False + elif rec.startswith("! "): + xy = "!!" + repo_path = rec[2:] + untracked = False + ignored = True + elif rec.startswith("1 "): + parts = rec.split(" ", 8) + if len(parts) < 9: + continue + xy = parts[1] + repo_path = parts[8] + untracked = False + ignored = False + elif rec.startswith("2 "): + parts = rec.split(" ", 9) + if len(parts) < 10: + continue + xy = parts[1] + repo_path = parts[9] + if i < len(tokens): + old_path = tokens[i] + i += 1 + renamed = True + untracked = False + ignored = False + elif rec.startswith("u "): + parts = rec.split(" ", 10) + if len(parts) < 11: + continue + xy = parts[1] + repo_path = parts[10] + untracked = False + ignored = False + else: + continue + + workspace_path = _workspace_rel(ctx, repo_path) + if workspace_path is None: + continue + old_workspace_path = _workspace_rel(ctx, old_path) if old_path else None + x = xy[0] if xy else "." + y = xy[1] if len(xy) > 1 else "." + conflict = xy in {"DD", "AU", "UD", "UA", "DU", "AA", "UU"} or rec.startswith("u ") + additions, deletions, binary = 0, 0, False + for source in (staged_stats, unstaged_stats): + if workspace_path in source: + a, d, b = source[workspace_path] + additions += a + deletions += d + binary = binary or b + if untracked: + additions, deletions, binary = _count_untracked_file(ctx.workspace / workspace_path) + + staged = (x not in {".", "?"}) and not untracked + unstaged = (y not in {".", " "}) and not untracked + if staged and staged_diff_paths is not None and not renamed: + raw_staged = staged + staged = workspace_path in staged_diff_paths or ( + old_workspace_path is not None and old_workspace_path in staged_diff_paths + ) + if raw_staged and not staged: + filtered_noise["filemode_only"] += 1 + if unstaged and unstaged_diff_paths is not None and not renamed: + raw_unstaged = unstaged + unstaged = workspace_path in unstaged_diff_paths or ( + old_workspace_path is not None and old_workspace_path in unstaged_diff_paths + ) + if raw_unstaged and not unstaged: + filtered_noise["filemode_only"] += 1 + if ignored: + files[workspace_path] = { + "path": workspace_path, + "old_path": None, + "workspace_path": workspace_path, + "status": "Ignored", + "staged": False, + "unstaged": False, + "untracked": False, + "ignored": True, + "conflict": False, + "additions": 0, + "deletions": 0, + "binary": False, + } + if len(files) >= STATUS_FILE_LIMIT: + truncated = True + break + continue + + if not (staged or unstaged or untracked or conflict or renamed): + continue + if not (untracked or conflict or renamed or binary) and additions == 0 and deletions == 0: + filtered_noise["crlf_only"] += 1 + continue + + files[workspace_path] = { + "path": workspace_path, + "old_path": old_workspace_path, + "workspace_path": workspace_path, + "status": _status_code(xy, untracked=untracked, renamed=renamed), + "staged": staged, + "unstaged": unstaged, + "untracked": untracked, + "ignored": False, + "conflict": conflict, + "additions": additions, + "deletions": deletions, + "binary": binary, + } + if len(files) >= STATUS_FILE_LIMIT: + truncated = True + break + + file_list = sorted(files.values(), key=lambda f: (f["path"].lower())) + totals = _empty_status() + for item in file_list: + if item.get("ignored"): + continue + if item["staged"]: + totals["staged"] += 1 + if item["unstaged"]: + totals["unstaged"] += 1 + if item["untracked"]: + totals["untracked"] += 1 + if item["conflict"]: + totals["conflicts"] += 1 + totals["changed"] = sum(1 for item in file_list if not item.get("ignored")) + + if not branch: + branch = (_run_git(ctx, ["rev-parse", "--short", "HEAD"], check=False).stdout or "").strip() + return { + "is_git": True, + "branch": branch or "HEAD", + "upstream": upstream, + "ahead": ahead, + "behind": behind, + "totals": totals, + "files": file_list, + "truncated": truncated, + "noise_filtering": { + **filtered_noise, + "active": any(filtered_noise.values()), + }, + } + + +def _branch_ahead_behind(ctx: GitContext, branch: str, upstream: str) -> tuple[int, int]: + if not upstream: + return 0, 0 + result = _run_git(ctx, ["rev-list", "--left-right", "--count", f"{branch}...{upstream}"], check=False) + if result.returncode != 0: + return 0, 0 + parts = result.stdout.strip().split() + if len(parts) != 2: + return 0, 0 + try: + return int(parts[0]), int(parts[1]) + except ValueError: + return 0, 0 + + +def _for_each_ref(ctx: GitContext, ref_prefix: str) -> list[dict]: + fmt = ( + "%(refname)%00%(refname:short)%00%(upstream:short)%00%(objectname:short)%00" + "%(committerdate:unix)%00%(committerdate:relative)%00%(authorname)%00%(subject)" + ) + result = _run_git(ctx, ["for-each-ref", f"--format={fmt}", ref_prefix], check=True) + refs = [] + for line in result.stdout.splitlines(): + full_name, name, upstream, sha, updated, updated_relative, author, subject = ( + line.split("\0") + ["", "", "", "", "", "", "", ""] + )[:8] + if not name or full_name.endswith("/HEAD") or name.endswith("/HEAD"): + continue + if ref_prefix == "refs/remotes" and "/" not in name: + continue + item = { + "name": name, + "sha": sha, + "updated": int(updated) if str(updated).isdigit() else 0, + "updated_relative": updated_relative, + "author": author, + "subject": subject, + } + if upstream: + ahead, behind = _branch_ahead_behind(ctx, name, upstream) + item.update({"upstream": upstream, "ahead": ahead, "behind": behind}) + else: + item.update({"upstream": "", "ahead": 0, "behind": 0}) + refs.append(item) + return sorted(refs, key=lambda item: item["name"].lower()) + + +def git_branches(workspace: str | Path) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + head_name = _run_git(ctx, ["branch", "--show-current"], check=True).stdout.strip() + detached = not bool(head_name) + head_sha = _run_git(ctx, ["rev-parse", "--short", "HEAD"], check=True).stdout.strip() + status = git_status(workspace) + local = _for_each_ref(ctx, "refs/heads") + remote = _for_each_ref(ctx, "refs/remotes") + return { + "is_git": True, + "current": head_name or head_sha or "HEAD", + "detached": detached, + "head": head_sha, + "local": local, + "remote": remote, + "upstream": status.get("upstream", ""), + "ahead": status.get("ahead", 0), + "behind": status.get("behind", 0), + } + + +def _validate_local_branch(ctx: GitContext, ref: str) -> str: + ref = str(ref or "").strip() + if not ref: + raise GitWorkspaceError("Branch name is required", "invalid_ref") + _run_git(ctx, ["show-ref", "--verify", f"refs/heads/{ref}"], check=True) + return ref + + +def _validate_remote_branch(ctx: GitContext, ref: str) -> str: + ref = str(ref or "").strip() + if not ref: + raise GitWorkspaceError("Remote branch name is required", "invalid_ref") + _run_git(ctx, ["show-ref", "--verify", f"refs/remotes/{ref}"], check=True) + return ref + + +def _validate_checkout_start(ctx: GitContext, ref: str) -> str: + ref = str(ref or "HEAD").strip() or "HEAD" + result = _run_git(ctx, ["rev-parse", "--verify", f"{ref}^{{commit}}"], check=False) + if result.returncode != 0: + raise GitWorkspaceError("Invalid checkout reference", "invalid_ref") + return ref + + +def _validate_new_branch_name(ctx: GitContext, name: str) -> str: + name = str(name or "").strip() + if not name: + raise GitWorkspaceError("New branch name is required", "invalid_ref") + result = _run_git(ctx, ["check-ref-format", "--branch", name], check=False) + if result.returncode != 0: + raise GitWorkspaceError("Invalid branch name", "invalid_ref") + exists = _run_git(ctx, ["show-ref", "--verify", f"refs/heads/{name}"], check=False) + if exists.returncode == 0: + raise GitWorkspaceError("A local branch with that name already exists", "invalid_ref") + return name + + +def _dirty_worktree(ctx: GitContext) -> bool: + result = _run_git(ctx, ["status", "--porcelain=v2", "--untracked-files=all"], check=True) + return bool(result.stdout.strip()) + + +def _validate_checkout_request_locked( + ctx: GitContext, + ref: str, + mode: str, + new_branch: str | None, +) -> None: + if mode == "local": + _validate_local_branch(ctx, ref) + return + if mode in {"new", "create"}: + _validate_new_branch_name(ctx, new_branch or ref) + _validate_checkout_start(ctx, ref if (new_branch and ref and ref != new_branch) else "HEAD") + return + if mode == "remote": + remote_ref = _validate_remote_branch(ctx, ref) + branch_name = str(new_branch or remote_ref.split("/", 1)[-1]).strip() + exists = _run_git(ctx, ["show-ref", "--verify", f"refs/heads/{branch_name}"], check=False) + if exists.returncode != 0: + _validate_new_branch_name(ctx, branch_name) + return + if mode in {"detached", "detach"}: + _validate_checkout_start(ctx, ref) + return + raise GitWorkspaceError("Unsupported checkout mode", "invalid_ref") + + +def _perform_checkout_locked( + ctx: GitContext, + workspace: str | Path, + ref: str, + mode: str, + new_branch: str | None, + track: bool, +) -> subprocess.CompletedProcess[str]: + if mode == "local": + target = _validate_local_branch(ctx, ref) + return _run_git(ctx, ["switch", target], check=True) + if mode in {"new", "create"}: + branch = _validate_new_branch_name(ctx, new_branch or ref) + start_ref = _validate_checkout_start(ctx, ref if (new_branch and ref and ref != new_branch) else "HEAD") + return _run_git(ctx, ["switch", "-c", branch, start_ref], check=True) + if mode == "remote": + remote_ref = _validate_remote_branch(ctx, ref) + branch_name = str(new_branch or remote_ref.split("/", 1)[-1]).strip() + exists = _run_git(ctx, ["show-ref", "--verify", f"refs/heads/{branch_name}"], check=False) + if exists.returncode == 0: + result = _run_git(ctx, ["switch", branch_name], check=True) + if track: + _run_git(ctx, ["branch", "--set-upstream-to", remote_ref, branch_name], check=False) + return result + branch = _validate_new_branch_name(ctx, branch_name) + args = ["switch", "-c", branch] + if track: + args.append("--track") + args.append(remote_ref) + return _run_git(ctx, args, check=True) + if mode in {"detached", "detach"}: + target = _validate_checkout_start(ctx, ref) + return _run_git(ctx, ["switch", "--detach", target], check=True) + raise GitWorkspaceError("Unsupported checkout mode", "invalid_ref") + + +def git_checkout( + workspace: str | Path, + ref: str, + mode: str, + new_branch: str | None = None, + track: bool = False, + dirty_mode: str = "block", +) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + mode = str(mode or "local").strip().lower() + dirty_mode = str(dirty_mode or "block").strip().lower() + if dirty_mode != "block": + raise GitWorkspaceError("Only dirty_mode=block is supported for branch checkout", "dirty_worktree") + with _git_mutation_lock(ctx): + _validate_checkout_request_locked(ctx, ref, mode, new_branch) + if _dirty_worktree(ctx): + raise GitWorkspaceError( + "Checkout blocked because the Git worktree has uncommitted changes", + "dirty_worktree", + ) + result = _perform_checkout_locked(ctx, workspace, ref, mode, new_branch, track) + status = git_status(workspace) + branches = git_branches(workspace) + return { + "ok": True, + "message": _remote_message(result), + "current_branch": branches.get("current"), + "status": status, + "branches": branches, + } + + +def git_stash_and_checkout( + workspace: str | Path, + ref: str, + mode: str, + new_branch: str | None = None, + track: bool = False, +) -> dict: + ctx = resolve_git_context(workspace) + 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() + 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) + status = git_status(workspace) + branches = git_branches(workspace) + return { + "ok": True, + "message": _remote_message(result), + "stash_name": stash_name if stashed else "", + "stashed": stashed, + "current_branch": branches.get("current"), + "status": status, + "branches": branches, + } + + +def _diff_stats(diff_text: str) -> tuple[int, int]: + additions = deletions = 0 + for line in diff_text.splitlines(): + if line.startswith("+++") or line.startswith("---"): + continue + if line.startswith("+"): + additions += 1 + elif line.startswith("-"): + deletions += 1 + return additions, deletions + + +def _synthetic_untracked_diff(path: Path, label: str) -> dict: + try: + if not path.is_file(): + raise GitWorkspaceError("Path is not a file") + if path.stat().st_size > DIFF_SIZE_LIMIT: + return { + "binary": False, + "too_large": True, + "diff": "", + "additions": 0, + "deletions": 0, + } + except OSError as exc: + raise GitWorkspaceError(str(exc)) from exc + try: + data = path.read_bytes() + except OSError as exc: + raise GitWorkspaceError(str(exc)) from exc + if b"\0" in data: + return {"binary": True, "too_large": False, "diff": "", "additions": 0, "deletions": 0} + try: + text = data.decode("utf-8") + except UnicodeDecodeError: + return {"binary": True, "too_large": False, "diff": "", "additions": 0, "deletions": 0} + lines = text.splitlines() + diff_lines = list( + difflib.unified_diff([], lines, fromfile="/dev/null", tofile=f"b/{label}", lineterm="") + ) + diff = "\n".join(diff_lines) + ("\n" if diff_lines else "") + too_large = len(diff.encode("utf-8", errors="replace")) > DIFF_SIZE_LIMIT + if too_large: + diff = diff[:DIFF_SIZE_LIMIT] + additions, deletions = _diff_stats(diff) + return { + "binary": False, + "too_large": too_large, + "diff": diff, + "additions": additions, + "deletions": deletions, + } + + +def git_diff(workspace: str | Path, path: str, kind: str = "unstaged") -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository") + if kind not in {"unstaged", "staged"}: + raise GitWorkspaceError("kind must be staged or unstaged") + repo_rel = _repo_rel(ctx, path) + workspace_rel = _workspace_rel(ctx, repo_rel) or path + + status = git_status(workspace) + file_state = next((f for f in status.get("files", []) if f.get("path") == workspace_rel), None) + if kind == "unstaged" and file_state and file_state.get("untracked"): + payload = _synthetic_untracked_diff(ctx.workspace / workspace_rel, workspace_rel) + return {"path": workspace_rel, "kind": kind, **payload} + + args = ["diff", "--no-ext-diff", "--unified=3"] + if kind == "staged": + args.append("--cached") + args.extend(["--", repo_rel]) + result = _run_git(ctx, args, check=True) + diff = result.stdout + binary = "Binary files " in diff or "GIT binary patch" in diff + too_large = len(diff.encode("utf-8", errors="replace")) > DIFF_SIZE_LIMIT + if too_large: + diff = diff[:DIFF_SIZE_LIMIT] + additions, deletions = _diff_stats(diff) + return { + "path": workspace_rel, + "kind": kind, + "binary": binary, + "too_large": too_large, + "additions": additions, + "deletions": deletions, + "diff": "" if binary else diff, + } + + +def _clean_paths(paths: Iterable[str]) -> list[str]: + cleaned = [] + for path in paths: + value = str(path or "").strip() + if value and value not in cleaned: + cleaned.append(value) + if not cleaned: + raise GitWorkspaceError("At least one path is required") + return cleaned + + +def _pathspecs(ctx: GitContext, paths: Iterable[str]) -> list[str]: + return [_repo_rel(ctx, path) for path in _clean_paths(paths)] + + +def git_stage(workspace: str | Path, paths: Iterable[str]) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + with _git_mutation_lock(ctx): + _run_git(ctx, ["add", "--", *_pathspecs(ctx, paths)], check=True) + return git_status(workspace) + + +def git_unstage(workspace: str | Path, paths: Iterable[str]) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + specs = _pathspecs(ctx, paths) + with _git_mutation_lock(ctx): + result = _run_git(ctx, ["restore", "--staged", "--", *specs], check=False) + if result.returncode != 0: + _run_git(ctx, ["reset", "HEAD", "--", *specs], check=True) + return git_status(workspace) + + +def git_discard(workspace: str | Path, paths: Iterable[str], *, delete_untracked: bool = False) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + with _git_mutation_lock(ctx): + status = git_status(workspace) + by_path = {f["path"]: f for f in status.get("files", [])} + for path in _clean_paths(paths): + repo_rel = _repo_rel(ctx, path) + workspace_rel = _workspace_rel(ctx, repo_rel) or path + state = by_path.get(workspace_rel) or by_path.get(workspace_rel.rstrip("/") + "/") + if state and state.get("conflict"): + raise GitWorkspaceError("Conflicted files cannot be discarded from this panel", "conflict") + if state and state.get("untracked"): + if not delete_untracked: + raise GitWorkspaceError("Untracked files require delete_untracked=true") + target = safe_resolve_ws(ctx.workspace, workspace_rel) + if target.is_dir(): + shutil.rmtree(target) + else: + target.unlink(missing_ok=True) + continue + _run_git(ctx, ["restore", "--worktree", "--", repo_rel], check=True) + return git_status(workspace) + + +COMMIT_MESSAGE_SYSTEM_PROMPT = """When writing commit messages, PR titles, or PR descriptions: + +- Inspect the staged diff before suggesting a commit message. +- Do not use vague subjects like "update", "improve", "refine", "misc changes", "fix stuff", or "various changes". +- For large commits, write a concise subject plus a short body with 2-5 bullets summarizing the main areas changed. +- The subject should describe the actual user-facing result or bug fixed, not just broad implementation activity. +- Keep wording short, clear, and natural. +- Never mention AI, Cursor, Zed, agents, or similar tooling in commits, branch names, PR titles, or PR descriptions. +- Never add your own thoughts or questions into the commit message, the commit message is definitive in nature. + +Return only the commit message text. Do not wrap it in Markdown fences. +""".strip() + + +def _staged_diff_text(ctx: GitContext) -> tuple[str, bool]: + result = _run_git( + ctx, + [ + "diff", + "--cached", + "--no-ext-diff", + "--unified=3", + "--", + _workspace_pathspec(ctx), + ], + check=True, + ) + diff = result.stdout or "" + encoded = diff.encode("utf-8", errors="replace") + if len(encoded) <= COMMIT_MESSAGE_DIFF_LIMIT: + return diff, False + return encoded[:COMMIT_MESSAGE_DIFF_LIMIT].decode("utf-8", errors="replace"), True + + +def _selected_temp_index_env(ctx: GitContext, specs: list[str]) -> tuple[dict[str, str], str]: + fd, index_path = tempfile.mkstemp(prefix="hermes-webui-git-index-") + os.close(fd) + Path(index_path).unlink(missing_ok=True) + env = {"GIT_INDEX_FILE": index_path} + try: + head = _run_git(ctx, ["rev-parse", "--verify", "HEAD"], check=False, env=env) + if head.returncode == 0: + _run_git(ctx, ["read-tree", "HEAD"], check=True, env=env) + else: + _run_git(ctx, ["read-tree", "--empty"], check=True, env=env) + _run_git(ctx, ["add", "-A", "--", *specs], check=True, env=env) + return env, index_path + except Exception: + Path(index_path).unlink(missing_ok=True) + raise + + +def _selected_files(ctx: GitContext, paths: Iterable[str]) -> tuple[list[str], list[str], list[dict]]: + requested = _clean_paths(paths) + requested_specs = [_repo_rel(ctx, path) for path in requested] + workspace_paths = [_workspace_rel(ctx, spec) or path for spec, path in zip(requested_specs, requested)] + status = git_status(ctx.workspace) + by_path = {f["path"]: f for f in status.get("files", [])} + specs: list[str] = [] + selected = [] + for path, repo_rel in zip(workspace_paths, requested_specs): + state = by_path.get(path) + if not state: + continue + if state.get("conflict"): + raise GitWorkspaceError("Resolve conflicts before committing selected files", "conflict") + if state.get("staged") or state.get("unstaged") or state.get("untracked"): + selected.append(state) + for spec in (repo_rel, _repo_rel(ctx, state["old_path"]) if state.get("old_path") else ""): + if spec and spec not in specs: + specs.append(spec) + if len(selected) != len(workspace_paths): + raise GitWorkspaceError("Selected paths have no committable changes") + return specs, workspace_paths, selected + + +def _selected_diff_text(ctx: GitContext, specs: list[str]) -> tuple[str, bool]: + env, index_path = _selected_temp_index_env(ctx, specs) + try: + result = _run_git( + ctx, + ["diff", "--cached", "--no-ext-diff", "--unified=3", "--", *specs], + check=True, + env=env, + ) + diff = result.stdout or "" + encoded = diff.encode("utf-8", errors="replace") + if len(encoded) <= COMMIT_MESSAGE_DIFF_LIMIT: + return diff, False + return encoded[:COMMIT_MESSAGE_DIFF_LIMIT].decode("utf-8", errors="replace"), True + finally: + Path(index_path).unlink(missing_ok=True) + + +def selected_commit_message_prompt(workspace: str | Path, paths: Iterable[str]) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + specs, _workspace_paths, selected_files = _selected_files(ctx, paths) + diff, truncated = _selected_diff_text(ctx, specs) + if not diff.strip(): + raise GitWorkspaceError("No selected diff is available") + status = git_status(workspace) + file_lines = [] + for item in selected_files[:80]: + stats = ( + "binary" + if item.get("binary") + else f"+{item.get('additions') or 0} -{item.get('deletions') or 0}" + ) + file_lines.append(f"- {item.get('status') or 'M'} {item.get('path')} ({stats})") + if len(selected_files) > 80: + file_lines.append(f"- ... {len(selected_files) - 80} more selected file(s)") + user_prompt = ( + "Write a commit message for the selected Git diff below.\n\n" + f"Branch: {status.get('branch') or 'HEAD'}\n" + f"Selected files ({len(selected_files)}):\n" + + "\n".join(file_lines) + + ( + "\n\nDiff was truncated for size; summarize only what is visible.\n" + if truncated + else "\n" + ) + + "\nSelected diff:\n```diff\n" + + diff + + "\n```" + ) + return { + "system_prompt": COMMIT_MESSAGE_SYSTEM_PROMPT, + "user_prompt": user_prompt, + "truncated": truncated, + "status": status, + } + + +def staged_commit_message_prompt(workspace: str | Path) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository") + status = git_status(workspace) + if int((status.get("totals") or {}).get("staged") or 0) <= 0: + raise GitWorkspaceError("Stage changes before generating a commit message") + diff, truncated = _staged_diff_text(ctx) + if not diff.strip(): + raise GitWorkspaceError("No staged diff is available") + staged_files = [f for f in status.get("files", []) if f.get("staged")] + file_lines = [] + for item in staged_files[:80]: + stats = ( + "binary" + if item.get("binary") + else f"+{item.get('additions') or 0} -{item.get('deletions') or 0}" + ) + file_lines.append(f"- {item.get('status') or 'M'} {item.get('path')} ({stats})") + if len(staged_files) > 80: + file_lines.append(f"- ... {len(staged_files) - 80} more staged file(s)") + user_prompt = ( + "Write a commit message for the staged Git diff below.\n\n" + f"Branch: {status.get('branch') or 'HEAD'}\n" + f"Staged files ({len(staged_files)}):\n" + + "\n".join(file_lines) + + ( + "\n\nDiff was truncated for size; summarize only what is visible.\n" + if truncated + else "\n" + ) + + "\nStaged diff:\n```diff\n" + + diff + + "\n```" + ) + return { + "system_prompt": COMMIT_MESSAGE_SYSTEM_PROMPT, + "user_prompt": user_prompt, + "truncated": truncated, + "status": status, + } + + +def clean_generated_commit_message(message: str) -> str: + text = str(message or "").strip() + if text.startswith("```"): + lines = text.splitlines() + if lines and lines[0].startswith("```"): + lines = lines[1:] + if lines and lines[-1].strip() == "```": + lines = lines[:-1] + text = "\n".join(lines).strip() + if (text.startswith('"') and text.endswith('"')) or ( + text.startswith("'") and text.endswith("'") + ): + text = text[1:-1].strip() + return text + + +def git_commit(workspace: str | Path, message: str) -> dict: + msg = str(message or "").strip() + if not msg: + raise GitWorkspaceError("Commit message is required") + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + with _git_mutation_lock(ctx): + _run_git(ctx, ["commit", "-m", msg], timeout=10, check=True) + sha = _run_git(ctx, ["rev-parse", "--short", "HEAD"], check=True).stdout.strip() + return {"ok": True, "commit": sha, "status": git_status(workspace)} + + +def git_commit_selected(workspace: str | Path, message: str, paths: Iterable[str]) -> dict: + msg = str(message or "").strip() + if not msg: + raise GitWorkspaceError("Commit message is required") + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + with _git_mutation_lock(ctx): + specs, workspace_paths, _selected_files_list = _selected_files(ctx, paths) + env, index_path = _selected_temp_index_env(ctx, specs) + try: + quiet = _run_git(ctx, ["diff", "--cached", "--quiet", "--", *specs], check=False, env=env) + if quiet.returncode == 0: + raise GitWorkspaceError("Selected paths have no committable changes") + _run_git(ctx, ["commit", "-m", msg], timeout=10, check=True, env=env) + _run_git(ctx, ["reset", "-q", "HEAD", "--", *specs], check=True) + finally: + Path(index_path).unlink(missing_ok=True) + sha = _run_git(ctx, ["rev-parse", "--short", "HEAD"], check=True).stdout.strip() + return {"ok": True, "commit": sha, "paths": workspace_paths, "status": git_status(workspace)} + + +def _branch_name(ctx: GitContext) -> str: + branch = _run_git(ctx, ["branch", "--show-current"], check=True).stdout.strip() + if not branch: + raise GitWorkspaceError("Cannot push from a detached HEAD") + return branch + + +def _remote_message(result: subprocess.CompletedProcess[str]) -> str: + return (result.stdout or result.stderr or "").strip() + + +def git_fetch(workspace: str | Path) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + with _git_mutation_lock(ctx): + result = _run_git(ctx, ["fetch", "--prune"], timeout=GIT_REMOTE_TIMEOUT, check=True) + return {"ok": True, "message": _remote_message(result), "status": git_status(workspace)} + + +def git_pull(workspace: str | Path) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + with _git_mutation_lock(ctx): + result = _run_git(ctx, ["pull", "--ff-only"], timeout=GIT_REMOTE_TIMEOUT, check=True) + return {"ok": True, "message": _remote_message(result), "status": git_status(workspace)} + + +def git_push(workspace: str | Path) -> dict: + ctx = resolve_git_context(workspace) + if ctx is None: + raise GitWorkspaceError("Workspace is not a Git repository", "not_a_repo") + with _git_mutation_lock(ctx): + status = git_status(workspace) + args = ["push"] + if not status.get("upstream"): + branch = _branch_name(ctx) + remotes = _run_git(ctx, ["remote"], check=True).stdout.split() + if "origin" not in remotes: + raise GitWorkspaceError("No upstream branch or origin remote is configured", "no_upstream") + args.extend(["-u", "origin", branch]) + result = _run_git(ctx, args, timeout=GIT_REMOTE_TIMEOUT, check=True) + return {"ok": True, "message": _remote_message(result), "status": git_status(workspace)} diff --git a/tests/conftest.py b/tests/conftest.py index 66cf0102..39ab0c1d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -536,6 +536,7 @@ def test_server(): # pytest-side block can't see. env["HERMES_WEBUI_TEST_NETWORK_BLOCK"] = "1" env.update({ + "HERMES_WEBUI_WORKSPACE_GIT_DESTRUCTIVE": "1", "HERMES_WEBUI_PORT": str(TEST_PORT), "HERMES_WEBUI_HOST": "127.0.0.1", "HERMES_WEBUI_STATE_DIR": str(TEST_STATE_DIR), diff --git a/tests/test_workspace_dir_signature.py b/tests/test_workspace_dir_signature.py new file mode 100644 index 00000000..696acf75 --- /dev/null +++ b/tests/test_workspace_dir_signature.py @@ -0,0 +1,26 @@ +from api.workspace import dir_signature, list_dir + + +def test_directory_signature_is_metadata_only_and_changes_with_entries(tmp_path): + (tmp_path / "alpha.txt").write_text("one", encoding="utf-8") + + entries = list_dir(tmp_path, ".") + sig1 = dir_signature(tmp_path, ".", entries) + + assert isinstance(sig1, str) + assert len(sig1) == 64 + assert all("mtime_ns" in entry for entry in entries) + + (tmp_path / "beta.txt").write_text("two", encoding="utf-8") + entries2 = list_dir(tmp_path, ".") + sig2 = dir_signature(tmp_path, ".", entries2) + + assert sig2 != sig1 + + +def test_directory_signature_can_be_computed_from_supplied_entries(tmp_path): + (tmp_path / "alpha.txt").write_text("one", encoding="utf-8") + + entries = list_dir(tmp_path, ".") + + assert dir_signature(tmp_path, ".", entries) == dir_signature(tmp_path, ".", entries) diff --git a/tests/test_workspace_git.py b/tests/test_workspace_git.py new file mode 100644 index 00000000..d0dea59b --- /dev/null +++ b/tests/test_workspace_git.py @@ -0,0 +1,796 @@ +import json +import pathlib +import subprocess +import uuid +import urllib.error +import urllib.parse +import urllib.request + +import pytest + +from tests._pytest_port import BASE + + +ROOT = pathlib.Path(__file__).parent.parent + + +def _git(cwd, *args): + result = subprocess.run( + ["git", *args], + cwd=str(cwd), + shell=False, + text=True, + capture_output=True, + timeout=20, + ) + assert result.returncode == 0, result.stderr or result.stdout + return result.stdout + + +def _init_repo(path): + path.mkdir(parents=True, exist_ok=True) + _git(path, "init") + _git(path, "config", "user.email", "hermes-tests@example.invalid") + _git(path, "config", "user.name", "Hermes Tests") + return path + + +def _commit_all(path, message="initial"): + _git(path, "add", ".") + _git(path, "commit", "-m", message) + + +def _get(path): + try: + with urllib.request.urlopen(BASE + path, timeout=10) as r: + return json.loads(r.read()), r.status + except urllib.error.HTTPError as e: + return json.loads(e.read()), e.code + + +def _post(path, body=None): + data = json.dumps(body or {}).encode() + req = urllib.request.Request( + BASE + path, + data=data, + headers={"Content-Type": "application/json"}, + ) + try: + with urllib.request.urlopen(req, timeout=10) as r: + return json.loads(r.read()), r.status + except urllib.error.HTTPError as e: + return json.loads(e.read()), e.code + + +def _make_session(created_list, ws=None): + body = {} + if ws: + body["workspace"] = str(ws) + data, status = _post("/api/session/new", body) + assert status == 200 + sid = data["session"]["session_id"] + created_list.append(sid) + return sid, pathlib.Path(data["session"]["workspace"]) + + +def test_git_status_non_git_workspace(tmp_path): + from api.workspace_git import git_status + + ws = tmp_path / "plain" + ws.mkdir() + assert git_status(ws) == {"is_git": False} + + +def test_git_status_handles_staged_unstaged_untracked_deleted_and_renamed(tmp_path): + from api.workspace_git import git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + (repo / "delete-me.txt").write_text("bye\n", encoding="utf-8") + (repo / "old name.txt").write_text("move\n", encoding="utf-8") + _commit_all(repo) + + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8") + (repo / "staged.txt").write_text("staged\n", encoding="utf-8") + _git(repo, "add", "staged.txt") + (repo / "delete-me.txt").unlink() + _git(repo, "mv", "old name.txt", "new name.txt") + (repo / "untracked space.txt").write_text("new\nfile\n", encoding="utf-8") + + status = git_status(repo) + by_path = {item["path"]: item for item in status["files"]} + + assert status["is_git"] is True + assert by_path["tracked.txt"]["unstaged"] is True + assert by_path["staged.txt"]["staged"] is True + assert by_path["delete-me.txt"]["status"] == "D" + assert by_path["new name.txt"]["old_path"] == "old name.txt" + assert by_path["untracked space.txt"]["untracked"] is True + assert by_path["untracked space.txt"]["additions"] == 2 + assert status["totals"]["changed"] >= 5 + + +def test_git_status_reports_ignored_files_without_counting_them_as_changes(tmp_path): + from api.workspace_git import git_status + + repo = _init_repo(tmp_path / "repo") + (repo / ".gitignore").write_text("*.log\nbuild/\n", encoding="utf-8") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8") + (repo / "debug.log").write_text("ignored log\n", encoding="utf-8") + build = repo / "build" + build.mkdir() + (build / "artifact.txt").write_text("ignored artifact\n", encoding="utf-8") + + status = git_status(repo) + by_path = {item["path"]: item for item in status["files"]} + + assert by_path["tracked.txt"]["unstaged"] is True + assert by_path["debug.log"]["ignored"] is True + assert by_path["debug.log"]["status"] == "Ignored" + assert by_path["build/"]["ignored"] is True + assert by_path["build/"]["staged"] is False + assert by_path["build/"]["untracked"] is False + assert status["totals"]["changed"] == 1 + assert status["totals"]["untracked"] == 0 + + +def test_git_status_ignores_crlf_only_worktree_noise(tmp_path): + from api.workspace_git import git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8", newline="\n") + _commit_all(repo) + + (repo / "tracked.txt").write_text("one\r\ntwo\r\n", encoding="utf-8", newline="") + + raw = _git(repo, "status", "--porcelain", "--", "tracked.txt") + assert raw.startswith(" M") + + status = git_status(repo) + assert status["totals"]["changed"] == 0 + assert status["files"] == [] + assert status["noise_filtering"]["active"] is True + assert status["noise_filtering"]["crlf_only"] == 1 + + +def test_git_status_keeps_real_edit_with_crlf_endings(tmp_path): + from api.workspace_git import git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8", newline="\n") + _commit_all(repo) + + (repo / "tracked.txt").write_text("one\r\ntwo\r\nthree\r\n", encoding="utf-8", newline="") + + status = git_status(repo) + by_path = {item["path"]: item for item in status["files"]} + assert status["totals"]["changed"] == 1 + assert by_path["tracked.txt"]["unstaged"] is True + assert by_path["tracked.txt"]["additions"] == 1 + assert by_path["tracked.txt"]["deletions"] == 0 + + +def test_git_status_ignores_filemode_only_noise(tmp_path): + from api.workspace_git import git_status + + repo = _init_repo(tmp_path / "repo") + script = repo / "script.sh" + script.write_text("#!/bin/sh\necho hi\n", encoding="utf-8") + _commit_all(repo) + + _git(repo, "update-index", "--chmod=+x", "script.sh") + + raw = _git(repo, "status", "--porcelain", "--", "script.sh") + assert "script.sh" in raw + + status = git_status(repo) + assert status["totals"]["changed"] == 0 + assert status["files"] == [] + assert status["noise_filtering"]["active"] is True + + +def test_git_status_scopes_nested_workspace_to_that_directory(tmp_path): + from api.workspace_git import git_status + + repo = _init_repo(tmp_path / "repo") + nested = repo / "app" + nested.mkdir() + (nested / "inside.txt").write_text("inside\n", encoding="utf-8") + (repo / "outside.txt").write_text("outside\n", encoding="utf-8") + _commit_all(repo) + + (nested / "inside.txt").write_text("inside\nchanged\n", encoding="utf-8") + (repo / "outside.txt").write_text("outside\nchanged\n", encoding="utf-8") + + status = git_status(nested) + paths = {item["path"] for item in status["files"]} + assert paths == {"inside.txt"} + + +def test_git_diff_generates_untracked_text_diff_and_blocks_escape(tmp_path): + from api.workspace_git import GitWorkspaceError, git_diff + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + (repo / "new file.txt").write_text("hello\nworld\n", encoding="utf-8") + + diff = git_diff(repo, "new file.txt", "unstaged") + assert diff["binary"] is False + assert "+++ b/new file.txt" in diff["diff"] + assert "+hello" in diff["diff"] + + with pytest.raises(GitWorkspaceError): + git_diff(repo, "../outside.txt", "unstaged") + + +def test_git_status_reports_untracked_files_inside_directories(tmp_path): + from api.workspace_git import git_discard, git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + nested = repo / "newdir" + nested.mkdir() + (nested / "a.txt").write_text("hello\n", encoding="utf-8") + + status = git_status(repo) + paths = {item["path"] for item in status["files"]} + assert "newdir/a.txt" in paths + assert "newdir/" not in paths + + git_discard(repo, ["newdir/a.txt"], delete_untracked=True) + assert not (nested / "a.txt").exists() + + +def test_git_status_reports_ignored_files_without_counting_them_as_changed(tmp_path): + from api.workspace_git import git_status + + repo = _init_repo(tmp_path / "repo") + (repo / ".gitignore").write_text("*.log\nbuild/\n", encoding="utf-8") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8") + (repo / "debug.log").write_text("ignored log\n", encoding="utf-8") + build = repo / "build" + build.mkdir() + (build / "artifact.txt").write_text("ignored artifact\n", encoding="utf-8") + + status = git_status(repo) + by_path = {item["path"]: item for item in status["files"]} + + assert by_path["tracked.txt"]["unstaged"] is True + assert by_path["debug.log"]["ignored"] is True + assert by_path["debug.log"]["status"] == "Ignored" + assert by_path["debug.log"]["staged"] is False + assert by_path["debug.log"]["unstaged"] is False + assert by_path["debug.log"]["untracked"] is False + assert any(item["ignored"] and item["path"].startswith("build") for item in status["files"]) + assert status["totals"]["changed"] == 1 + assert status["totals"]["untracked"] == 0 + + +def test_git_diff_large_untracked_file_is_bounded(tmp_path): + from api.workspace_git import DIFF_SIZE_LIMIT, git_diff, git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + large = repo / "large.txt" + large.write_text("x" * (DIFF_SIZE_LIMIT + 1), encoding="utf-8") + + status = git_status(repo) + by_path = {item["path"]: item for item in status["files"]} + assert by_path["large.txt"]["untracked"] is True + assert by_path["large.txt"]["additions"] == 0 + + diff = git_diff(repo, "large.txt", "unstaged") + assert diff["too_large"] is True + assert diff["diff"] == "" + + +def test_git_stage_unstage_discard_and_commit(tmp_path): + from api.workspace_git import git_commit, git_discard, git_stage, git_status, git_unstage + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8") + staged = git_stage(repo, ["tracked.txt"]) + assert staged["totals"]["staged"] == 1 + + unstaged = git_unstage(repo, ["tracked.txt"]) + assert unstaged["totals"]["staged"] == 0 + assert unstaged["totals"]["unstaged"] == 1 + + git_discard(repo, ["tracked.txt"]) + assert git_status(repo)["totals"]["changed"] == 0 + + (repo / "tracked.txt").write_text("one\nthree\n", encoding="utf-8") + git_stage(repo, ["tracked.txt"]) + committed = git_commit(repo, "Update tracked file") + assert committed["ok"] is True + assert committed["commit"] + assert committed["status"]["totals"]["changed"] == 0 + + +def test_git_commit_selected_ignores_unrelated_real_index(tmp_path): + from api.workspace_git import git_commit_selected, git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "selected.txt").write_text("one\n", encoding="utf-8") + (repo / "staged.txt").write_text("alpha\n", encoding="utf-8") + _commit_all(repo) + + (repo / "selected.txt").write_text("one\ntwo\n", encoding="utf-8") + (repo / "staged.txt").write_text("alpha\nbeta\n", encoding="utf-8") + _git(repo, "add", "staged.txt") + + committed = git_commit_selected(repo, "Commit selected only", ["selected.txt"]) + assert committed["ok"] is True + assert committed["paths"] == ["selected.txt"] + assert _git(repo, "show", "--name-only", "--format=", "HEAD").splitlines() == ["selected.txt"] + + by_path = {item["path"]: item for item in git_status(repo)["files"]} + assert "selected.txt" not in by_path + assert by_path["staged.txt"]["staged"] is True + + +def test_git_commit_selected_supports_initial_commit(tmp_path): + from api.workspace_git import git_commit_selected, git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "first.txt").write_text("first\n", encoding="utf-8") + + committed = git_commit_selected(repo, "Initial selected commit", ["first.txt"]) + assert committed["ok"] is True + assert _git(repo, "show", "--name-only", "--format=", "HEAD").splitlines() == ["first.txt"] + assert git_status(repo)["totals"]["changed"] == 0 + + +def test_git_commit_selected_preserves_rename_semantics(tmp_path): + from api.workspace_git import git_commit_selected, git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "old.txt").write_text("old\n", encoding="utf-8") + _commit_all(repo) + + _git(repo, "mv", "old.txt", "new.txt") + + committed = git_commit_selected(repo, "Rename selected file", ["new.txt"]) + assert committed["ok"] is True + assert _git(repo, "ls-tree", "--name-only", "HEAD").splitlines() == ["new.txt"] + assert "old.txt" not in _git(repo, "status", "--porcelain=v2") + assert git_status(repo)["totals"]["changed"] == 0 + + +def test_git_commit_selected_handles_untracked_and_mixed_paths(tmp_path): + from api.workspace_git import git_commit_selected + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8") + (repo / "new.txt").write_text("new\n", encoding="utf-8") + + committed = git_commit_selected(repo, "Commit mixed selected files", ["tracked.txt", "new.txt"]) + assert committed["ok"] is True + assert set(_git(repo, "show", "--name-only", "--format=", "HEAD").splitlines()) == { + "tracked.txt", + "new.txt", + } + + +def test_git_commit_selected_respects_nested_workspace_scope(tmp_path): + from api.workspace_git import GitWorkspaceError, git_commit_selected + + repo = _init_repo(tmp_path / "repo") + nested = repo / "app" + nested.mkdir() + (nested / "inside.txt").write_text("inside\n", encoding="utf-8") + (repo / "outside.txt").write_text("outside\n", encoding="utf-8") + _commit_all(repo) + + (nested / "inside.txt").write_text("inside\nchanged\n", encoding="utf-8") + (repo / "outside.txt").write_text("outside\nchanged\n", encoding="utf-8") + + committed = git_commit_selected(nested, "Nested selected commit", ["inside.txt"]) + assert committed["paths"] == ["inside.txt"] + assert _git(repo, "show", "--name-only", "--format=", "HEAD").splitlines() == ["app/inside.txt"] + + with pytest.raises(GitWorkspaceError) as outside: + git_commit_selected(nested, "Outside", ["../outside.txt"]) + assert outside.value.code == "path_outside_workspace" + + +def test_git_commit_selected_rejects_conflicts_and_path_traversal(tmp_path): + from api.workspace_git import GitWorkspaceError, git_commit_selected + + repo = _init_repo(tmp_path / "repo") + (repo / "conflict.txt").write_text("base\n", encoding="utf-8") + _commit_all(repo) + _git(repo, "checkout", "-b", "side") + (repo / "conflict.txt").write_text("side\n", encoding="utf-8") + _commit_all(repo, "side") + _git(repo, "checkout", "master") + (repo / "conflict.txt").write_text("main\n", encoding="utf-8") + _commit_all(repo, "main") + subprocess.run(["git", "merge", "side"], cwd=repo, shell=False, text=True, capture_output=True, timeout=20) + + with pytest.raises(GitWorkspaceError) as conflict: + git_commit_selected(repo, "Nope", ["conflict.txt"]) + assert conflict.value.code == "conflict" + + with pytest.raises(GitWorkspaceError) as traversal: + git_commit_selected(repo, "Nope", ["../outside.txt"]) + assert traversal.value.code == "path_outside_workspace" + + +def test_selected_commit_message_prompt_uses_selected_diff(tmp_path): + from api.workspace_git import selected_commit_message_prompt + + repo = _init_repo(tmp_path / "repo") + (repo / "selected.txt").write_text("one\n", encoding="utf-8") + (repo / "other.txt").write_text("alpha\n", encoding="utf-8") + _commit_all(repo) + (repo / "selected.txt").write_text("one\ntwo\n", encoding="utf-8") + (repo / "other.txt").write_text("alpha\nbeta\n", encoding="utf-8") + + prompt = selected_commit_message_prompt(repo, ["selected.txt"]) + assert "selected.txt" in prompt["user_prompt"] + assert "+two" in prompt["user_prompt"] + assert "other.txt" not in prompt["user_prompt"] + assert "beta" not in prompt["user_prompt"] + + +def test_staged_commit_message_prompt_uses_only_staged_diff(tmp_path): + from api.workspace_git import ( + GitWorkspaceError, + clean_generated_commit_message, + staged_commit_message_prompt, + ) + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + + (repo / "tracked.txt").write_text("one\nstaged\n", encoding="utf-8") + _git(repo, "add", "tracked.txt") + (repo / "tracked.txt").write_text("one\nstaged\nunstaged\n", encoding="utf-8") + + prompt = staged_commit_message_prompt(repo) + assert prompt["truncated"] is False + assert "tracked.txt" in prompt["user_prompt"] + assert "+staged" in prompt["user_prompt"] + assert "unstaged" not in prompt["user_prompt"] + assert "Never mention AI, Cursor, Zed, agents" in prompt["system_prompt"] + + _git(repo, "restore", "--staged", "tracked.txt") + with pytest.raises(GitWorkspaceError): + staged_commit_message_prompt(repo) + + assert clean_generated_commit_message("```text\nSubject\n\n- Body\n```") == "Subject\n\n- Body" + + +def test_git_fetch_pull_and_push_with_upstream(tmp_path): + from api.workspace_git import git_fetch, git_pull, git_push, git_status + + remote = tmp_path / "remote.git" + _git(tmp_path, "init", "--bare", str(remote)) + + origin = _init_repo(tmp_path / "origin") + (origin / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(origin) + _git(origin, "remote", "add", "origin", str(remote)) + _git(origin, "push", "-u", "origin", "HEAD") + + clone = tmp_path / "clone" + _git(tmp_path, "clone", str(remote), str(clone)) + _git(clone, "config", "user.email", "hermes-tests@example.invalid") + _git(clone, "config", "user.name", "Hermes Tests") + + (origin / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8") + _commit_all(origin, "Remote update") + _git(origin, "push") + + fetched = git_fetch(clone) + assert fetched["status"]["behind"] == 1 + + pulled = git_pull(clone) + assert pulled["status"]["behind"] == 0 + assert (clone / "tracked.txt").read_text(encoding="utf-8") == "one\ntwo\n" + + (clone / "tracked.txt").write_text("one\ntwo\nthree\n", encoding="utf-8") + _git(clone, "add", "tracked.txt") + _git(clone, "commit", "-m", "Local update") + assert git_status(clone)["ahead"] == 1 + + pushed = git_push(clone) + assert pushed["status"]["ahead"] == 0 + + +def test_git_branches_lists_local_remote_and_upstream(tmp_path): + from api.workspace_git import git_branches + + remote = tmp_path / "remote.git" + _git(tmp_path, "init", "--bare", str(remote)) + origin = _init_repo(tmp_path / "origin") + (origin / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(origin) + _git(origin, "branch", "-M", "main") + _git(origin, "remote", "add", "origin", str(remote)) + _git(origin, "push", "-u", "origin", "main") + _git(remote, "symbolic-ref", "HEAD", "refs/heads/main") + + clone = tmp_path / "clone" + _git(tmp_path, "clone", str(remote), str(clone)) + branches = git_branches(clone) + assert branches["current"] == "main" + assert branches["detached"] is False + assert any(item["name"] == "main" and item["upstream"] == "origin/main" for item in branches["local"]) + main = next(item for item in branches["local"] if item["name"] == "main") + assert "updated_relative" in main and "author" in main and "subject" in main + assert any(item["name"] == "origin/main" for item in branches["remote"]) + assert not any(item["name"] == "origin" for item in branches["remote"]) + + +def test_git_checkout_local_new_remote_dirty_and_invalid_refs(tmp_path): + from api.workspace_git import GitWorkspaceError, git_branches, git_checkout + + remote = tmp_path / "remote.git" + _git(tmp_path, "init", "--bare", str(remote)) + origin = _init_repo(tmp_path / "origin") + (origin / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(origin) + _git(origin, "branch", "-M", "main") + _git(origin, "remote", "add", "origin", str(remote)) + _git(origin, "push", "-u", "origin", "main") + _git(remote, "symbolic-ref", "HEAD", "refs/heads/main") + _git(origin, "checkout", "-b", "remote-feature") + (origin / "remote.txt").write_text("remote\n", encoding="utf-8") + _commit_all(origin, "remote feature") + _git(origin, "push", "-u", "origin", "remote-feature") + + clone = tmp_path / "clone" + _git(tmp_path, "clone", str(remote), str(clone)) + _git(clone, "config", "user.email", "hermes-tests@example.invalid") + _git(clone, "config", "user.name", "Hermes Tests") + + created = git_checkout(clone, "main", "new", new_branch="local-work") + assert created["current_branch"] == "local-work" + assert git_branches(clone)["current"] == "local-work" + + switched = git_checkout(clone, "main", "local") + assert switched["current_branch"] == "main" + + tracked = git_checkout(clone, "origin/remote-feature", "remote", new_branch="remote-feature", track=True) + assert tracked["current_branch"] == "remote-feature" + assert git_branches(clone)["upstream"] == "origin/remote-feature" + + (clone / "tracked.txt").write_text("dirty\n", encoding="utf-8") + with pytest.raises(GitWorkspaceError) as dirty: + git_checkout(clone, "main", "local") + assert dirty.value.code == "dirty_worktree" + _git(clone, "restore", "tracked.txt") + + with pytest.raises(GitWorkspaceError) as invalid: + git_checkout(clone, "does-not-exist", "local") + assert invalid.value.code in {"invalid_ref", "git_failed"} + + +def test_git_checkout_detached_requires_explicit_mode(tmp_path): + from api.workspace_git import git_branches, git_checkout + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + sha = _git(repo, "rev-parse", "--short", "HEAD").strip() + + result = git_checkout(repo, sha, "detached") + assert result["ok"] is True + branches = git_branches(repo) + assert branches["detached"] is True + assert branches["current"] == sha + + +def test_git_stash_and_checkout_is_explicit(tmp_path): + from api.workspace_git import git_stash_and_checkout, git_status + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + _git(repo, "checkout", "-b", "target") + _git(repo, "checkout", "master") + (repo / "tracked.txt").write_text("dirty\n", encoding="utf-8") + + result = git_stash_and_checkout(repo, "target", "local") + assert result["ok"] is True + assert result["stashed"] is True + 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") + + +def test_git_stash_checkout_validates_before_stashing(tmp_path): + from api.workspace_git import GitWorkspaceError, git_stash_and_checkout + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + (repo / "tracked.txt").write_text("dirty\n", encoding="utf-8") + + with pytest.raises(GitWorkspaceError) as invalid: + git_stash_and_checkout(repo, "missing-branch", "local") + + assert invalid.value.code == "invalid_ref" + assert "M tracked.txt" in _git(repo, "status", "--porcelain") + assert _git(repo, "stash", "list") == "" + + +def test_git_routes_status_diff_stage_unstage_discard_commit(cleanup_test_sessions): + sid, base_ws = _make_session(cleanup_test_sessions) + repo = base_ws / f"git-route-{uuid.uuid4().hex[:8]}" + _init_repo(repo) + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + + _post("/api/session/update", {"session_id": sid, "workspace": str(repo), "model": "openai/gpt-5.4-mini"}) + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8") + + status, code = _get(f"/api/git/status?session_id={sid}") + assert code == 200 + assert status["git"]["totals"]["unstaged"] == 1 + + diff, code = _get( + f"/api/git/diff?session_id={sid}&path={urllib.parse.quote('tracked.txt')}&kind=unstaged" + ) + assert code == 200 + assert "+two" in diff["diff"]["diff"] + + staged, code = _post("/api/git/stage", {"session_id": sid, "paths": ["tracked.txt"]}) + assert code == 200 and staged["git"]["totals"]["staged"] == 1 + + unstaged, code = _post("/api/git/unstage", {"session_id": sid, "paths": ["tracked.txt"]}) + assert code == 200 and unstaged["git"]["totals"]["unstaged"] == 1 + + discarded, code = _post("/api/git/discard", {"session_id": sid, "paths": ["tracked.txt"]}) + assert code == 200 and discarded["git"]["totals"]["changed"] == 0 + + (repo / "tracked.txt").write_text("one\nthree\n", encoding="utf-8") + _post("/api/git/stage", {"session_id": sid, "paths": ["tracked.txt"]}) + committed, code = _post("/api/git/commit", {"session_id": sid, "message": "Route commit"}) + assert code == 200 + assert committed["ok"] is True + assert committed["status"]["totals"]["changed"] == 0 + + +def test_git_routes_branches_and_checkout(cleanup_test_sessions): + sid, base_ws = _make_session(cleanup_test_sessions) + repo = base_ws / f"git-branch-route-{uuid.uuid4().hex[:8]}" + _init_repo(repo) + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + _git(repo, "branch", "-M", "main") + _git(repo, "checkout", "-b", "feature") + _git(repo, "checkout", "main") + + _post("/api/session/update", {"session_id": sid, "workspace": str(repo), "model": "openai/gpt-5.4-mini"}) + branches, code = _get(f"/api/git/branches?session_id={sid}") + assert code == 200 + assert branches["branches"]["current"] == "main" + assert any(item["name"] == "feature" for item in branches["branches"]["local"]) + + checked, code = _post( + "/api/git/checkout", + {"session_id": sid, "ref": "feature", "mode": "local", "dirty_mode": "block"}, + ) + assert code == 200 + assert checked["ok"] is True + assert checked["current_branch"] == "feature" + assert checked["git"]["branch"] == "feature" + + +def test_git_routes_selected_commit_and_structured_error(cleanup_test_sessions): + sid, base_ws = _make_session(cleanup_test_sessions) + repo = base_ws / f"git-selected-route-{uuid.uuid4().hex[:8]}" + _init_repo(repo) + (repo / "selected.txt").write_text("one\n", encoding="utf-8") + (repo / "other.txt").write_text("alpha\n", encoding="utf-8") + _commit_all(repo) + + _post("/api/session/update", {"session_id": sid, "workspace": str(repo), "model": "openai/gpt-5.4-mini"}) + (repo / "selected.txt").write_text("one\ntwo\n", encoding="utf-8") + (repo / "other.txt").write_text("alpha\nbeta\n", encoding="utf-8") + _git(repo, "add", "other.txt") + + bad, code = _post("/api/git/commit-selected", {"session_id": sid, "message": "Bad", "paths": ["../x"]}) + assert code == 400 + assert bad["code"] == "path_outside_workspace" + + committed, code = _post( + "/api/git/commit-selected", + {"session_id": sid, "message": "Selected route commit", "paths": ["selected.txt"]}, + ) + assert code == 200 + assert committed["ok"] is True + assert committed["paths"] == ["selected.txt"] + assert _git(repo, "show", "--name-only", "--format=", "HEAD").splitlines() == ["selected.txt"] + + +def test_git_env_scrub_removes_redirecting_vars_and_preserves_temp_index(monkeypatch): + from api.workspace_git import _clean_git_env + + monkeypatch.setenv("GIT_DIR", "/tmp/evil-git-dir") + monkeypatch.setenv("GIT_WORK_TREE", "/tmp/evil-work-tree") + monkeypatch.setenv("GIT_CONFIG_GLOBAL", "/tmp/evil-config") + + env = _clean_git_env({"GIT_INDEX_FILE": "/tmp/hermes-index"}) + + assert "GIT_DIR" not in env + assert "GIT_WORK_TREE" not in env + assert "GIT_CONFIG_GLOBAL" not in env + assert env["GIT_INDEX_FILE"] == "/tmp/hermes-index" + + +def test_git_error_classifier_identifies_non_fast_forward_push(): + from api.workspace_git import _classify_git_error + + assert _classify_git_error("Updates were rejected", ["push"]) == "non_fast_forward" + assert _classify_git_error("non-fast-forward", ["push"]) == "non_fast_forward" + assert _classify_git_error("fetch first", ["push"]) == "non_fast_forward" + + +def test_git_commit_hook_failure_returns_hook_failed_code(tmp_path): + from api.workspace_git import GitWorkspaceError, git_commit, git_stage + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + hook = repo / ".git" / "hooks" / "pre-commit" + hook.write_text("#!/bin/sh\necho hook blocked >&2\nexit 1\n", encoding="utf-8") + hook.chmod(0o755) + + (repo / "tracked.txt").write_text("one\ntwo\n", encoding="utf-8") + git_stage(repo, ["tracked.txt"]) + + with pytest.raises(GitWorkspaceError) as exc: + git_commit(repo, "Hook should fail") + assert exc.value.code == "hook_failed" + + +def test_destructive_workspace_git_flag_defaults_off_and_accepts_truthy(monkeypatch): + from api.workspace_git import WORKSPACE_GIT_DESTRUCTIVE_ENV, workspace_git_destructive_enabled + + monkeypatch.delenv(WORKSPACE_GIT_DESTRUCTIVE_ENV, raising=False) + assert workspace_git_destructive_enabled() is False + + monkeypatch.setenv(WORKSPACE_GIT_DESTRUCTIVE_ENV, "1") + assert workspace_git_destructive_enabled() is True + + monkeypatch.setenv(WORKSPACE_GIT_DESTRUCTIVE_ENV, "true") + assert workspace_git_destructive_enabled() is True + + +def test_git_active_stream_lock_detection(monkeypatch): + import types + + from api import routes + from api.config import STREAMS, STREAMS_LOCK + + session = types.SimpleNamespace(active_stream_id="stream-git-lock-test") + with STREAMS_LOCK: + STREAMS[session.active_stream_id] = object() + try: + assert routes._git_locked_by_active_stream(session) is True + finally: + with STREAMS_LOCK: + STREAMS.pop(session.active_stream_id, None) + + assert routes._git_locked_by_active_stream(session) is False From 0f9c64b7807ec75a9b072342458f0ad5995dc304 Mon Sep 17 00:00:00 2001 From: stocky789 Date: Wed, 20 May 2026 05:43:17 +0000 Subject: [PATCH 02/10] fix: classify CRLF-only git status noise Distinguish CRLF-only working tree changes from filemode-only noise when the ignored-CR diff path set is empty on GitHub Actions. --- api/workspace_git.py | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/api/workspace_git.py b/api/workspace_git.py index fbef17f6..705bb81c 100644 --- a/api/workspace_git.py +++ b/api/workspace_git.py @@ -243,8 +243,10 @@ def _parse_path_list(text: str, ctx: GitContext) -> set[str]: return paths -def _collect_diff_paths(ctx: GitContext, cached: bool) -> set[str] | None: - args = ["diff", "--name-only", "-z", "--ignore-cr-at-eol"] +def _collect_diff_paths(ctx: GitContext, cached: bool, *, ignore_cr_at_eol: bool = True) -> set[str] | None: + args = ["diff", "--name-only", "-z"] + if ignore_cr_at_eol: + args.append("--ignore-cr-at-eol") if cached: args.append("--cached") args.extend(["--", _workspace_pathspec(ctx)]) @@ -254,8 +256,15 @@ def _collect_diff_paths(ctx: GitContext, cached: bool) -> set[str] | None: return _parse_path_list(result.stdout, ctx) -def _collect_numstat(ctx: GitContext, cached: bool) -> dict[str, tuple[int, int, bool]]: - args = ["diff", "--numstat", "--ignore-cr-at-eol"] +def _collect_numstat( + ctx: GitContext, + cached: bool, + *, + ignore_cr_at_eol: bool = True, +) -> dict[str, tuple[int, int, bool]]: + args = ["diff", "--numstat"] + if ignore_cr_at_eol: + args.append("--ignore-cr-at-eol") if cached: args.append("--cached") args.extend(["--", _workspace_pathspec(ctx)]) @@ -305,6 +314,8 @@ def git_status(workspace: str | Path) -> dict: ) staged_stats = _collect_numstat(ctx, cached=True) unstaged_stats = _collect_numstat(ctx, cached=False) + staged_raw_stats = _collect_numstat(ctx, cached=True, ignore_cr_at_eol=False) + unstaged_raw_stats = _collect_numstat(ctx, cached=False, ignore_cr_at_eol=False) staged_diff_paths = _collect_diff_paths(ctx, cached=True) unstaged_diff_paths = _collect_diff_paths(ctx, cached=False) @@ -404,14 +415,24 @@ def git_status(workspace: str | Path) -> dict: old_workspace_path is not None and old_workspace_path in staged_diff_paths ) if raw_staged and not staged: - filtered_noise["filemode_only"] += 1 + if workspace_path in staged_raw_stats or ( + old_workspace_path is not None and old_workspace_path in staged_raw_stats + ): + filtered_noise["crlf_only"] += 1 + else: + filtered_noise["filemode_only"] += 1 if unstaged and unstaged_diff_paths is not None and not renamed: raw_unstaged = unstaged unstaged = workspace_path in unstaged_diff_paths or ( old_workspace_path is not None and old_workspace_path in unstaged_diff_paths ) if raw_unstaged and not unstaged: - filtered_noise["filemode_only"] += 1 + if workspace_path in unstaged_raw_stats or ( + old_workspace_path is not None and old_workspace_path in unstaged_raw_stats + ): + filtered_noise["crlf_only"] += 1 + else: + filtered_noise["filemode_only"] += 1 if ignored: files[workspace_path] = { "path": workspace_path, From 898e15a899f027db9a802506cad92b6306ff61b2 Mon Sep 17 00:00:00 2001 From: stocky789 Date: Wed, 20 May 2026 08:14:30 +0000 Subject: [PATCH 03/10] 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): From 9ac94d3ef6c3861518f637df5f0ef781f27f70c7 Mon Sep 17 00:00:00 2001 From: stocky789 Date: Wed, 20 May 2026 11:02:45 +0000 Subject: [PATCH 04/10] fix(workspace): tighten git subprocess trust boundary --- api/workspace_git.py | 16 ++++++++- docs/workspace-git.md | 27 ++++++++------- tests/test_workspace_git.py | 67 +++++++++++++++++++++++++++++++++++-- 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/api/workspace_git.py b/api/workspace_git.py index 9631056e..23ac79c9 100644 --- a/api/workspace_git.py +++ b/api/workspace_git.py @@ -27,7 +27,15 @@ STATUS_FILE_LIMIT = 500 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") +_GIT_ENV_SCRUB_KEYS = ( + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + "GIT_CONFIG_COUNT", + "GIT_CONFIG_PARAMETERS", +) +_GIT_ENV_SCRUB_PREFIXES = ("GIT_CONFIG_KEY_", "GIT_CONFIG_VALUE_") _HERMES_BRANCH_SWITCH_STASH_PREFIX = "hermes-webui branch switch" @@ -46,6 +54,9 @@ def _clean_git_env(extra: dict[str, str] | None = None) -> dict[str, str]: env.update(extra) for key in _GIT_ENV_SCRUB_KEYS: env.pop(key, None) + for key in list(env): + if key.startswith(_GIT_ENV_SCRUB_PREFIXES): + env.pop(key, None) return env @@ -70,6 +81,9 @@ _OP_LOCKS: dict[str, threading.Lock] = {} @contextmanager def _git_mutation_lock(ctx: GitContext): + # Key by repo root so sessions in the same repository serialize mutations. + # Separate worktrees get separate locks; Git still protects shared metadata + # with its own locks. key = str(ctx.repo_root) with _LOCKS_GUARD: lock = _OP_LOCKS.setdefault(key, threading.Lock()) diff --git a/docs/workspace-git.md b/docs/workspace-git.md index 9d551d76..e075b835 100644 --- a/docs/workspace-git.md +++ b/docs/workspace-git.md @@ -8,7 +8,8 @@ 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. +> the WebUI process permissions and environment. Treat hooks that download or execute code as code +> execution by the WebUI process user. ## What works by default @@ -43,10 +44,11 @@ These actions are blocked unless `HERMES_WEBUI_WORKSPACE_GIT_DESTRUCTIVE=1` is s 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. +When the flag is enabled, branch switching parks and restores local changes automatically. 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 @@ -57,9 +59,10 @@ 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. +Before any Git subprocess starts, WebUI removes inherited `GIT_DIR`, `GIT_WORK_TREE`, +`GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM`, `GIT_CONFIG_COUNT`, `GIT_CONFIG_PARAMETERS`, and injected +`GIT_CONFIG_KEY_*` / `GIT_CONFIG_VALUE_*` values from the environment. Those variables can redirect +Git to a different repository or inject config, 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. @@ -78,11 +81,11 @@ WebUI does not bypass Git hooks. Hook code may come from `.git/hooks/` or from a `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. +run `pre-push`. Pull uses `--ff-only`, so 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. +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 diff --git a/tests/test_workspace_git.py b/tests/test_workspace_git.py index 531f605a..7c8a667b 100644 --- a/tests/test_workspace_git.py +++ b/tests/test_workspace_git.py @@ -1,10 +1,12 @@ import json import pathlib import subprocess +import types import uuid import urllib.error import urllib.parse import urllib.request +from io import BytesIO import pytest @@ -73,6 +75,26 @@ def _make_session(created_list, ws=None): return sid, pathlib.Path(data["session"]["workspace"]) +class _CaptureHandler: + def __init__(self): + self.status = None + self.headers = {} + self.response_headers = [] + self.wfile = BytesIO() + + def send_response(self, status): + self.status = status + + def send_header(self, key, value): + self.response_headers.append((key, value)) + + def end_headers(self): + pass + + def payload(self): + return json.loads(self.wfile.getvalue().decode("utf-8")) + + def test_git_status_non_git_workspace(tmp_path): from api.workspace_git import git_status @@ -791,12 +813,22 @@ def test_git_env_scrub_removes_redirecting_vars_and_preserves_temp_index(monkeyp monkeypatch.setenv("GIT_DIR", "/tmp/evil-git-dir") monkeypatch.setenv("GIT_WORK_TREE", "/tmp/evil-work-tree") monkeypatch.setenv("GIT_CONFIG_GLOBAL", "/tmp/evil-config") + monkeypatch.setenv("GIT_CONFIG_SYSTEM", "/tmp/evil-system-config") + monkeypatch.setenv("GIT_CONFIG_COUNT", "1") + monkeypatch.setenv("GIT_CONFIG_KEY_0", "core.sshCommand") + monkeypatch.setenv("GIT_CONFIG_VALUE_0", "ssh -i /tmp/evil-key") + monkeypatch.setenv("GIT_CONFIG_PARAMETERS", "'core.sshCommand=ssh -i /tmp/evil-key'") env = _clean_git_env({"GIT_INDEX_FILE": "/tmp/hermes-index"}) assert "GIT_DIR" not in env assert "GIT_WORK_TREE" not in env assert "GIT_CONFIG_GLOBAL" not in env + assert "GIT_CONFIG_SYSTEM" not in env + assert "GIT_CONFIG_COUNT" not in env + assert "GIT_CONFIG_KEY_0" not in env + assert "GIT_CONFIG_VALUE_0" not in env + assert "GIT_CONFIG_PARAMETERS" not in env assert env["GIT_INDEX_FILE"] == "/tmp/hermes-index" @@ -840,8 +872,6 @@ def test_destructive_workspace_git_flag_defaults_off_and_accepts_truthy(monkeypa def test_git_active_stream_lock_detection(monkeypatch): - import types - from api import routes from api.config import STREAMS, STREAMS_LOCK @@ -855,3 +885,36 @@ def test_git_active_stream_lock_detection(monkeypatch): STREAMS.pop(session.active_stream_id, None) assert routes._git_locked_by_active_stream(session) is False + + +def test_git_commit_route_rejects_active_stream(monkeypatch, tmp_path): + from api import routes + from api.config import STREAMS, STREAMS_LOCK + + repo = _init_repo(tmp_path / "repo") + (repo / "tracked.txt").write_text("one\n", encoding="utf-8") + _commit_all(repo) + _git(repo, "add", "tracked.txt") + session = types.SimpleNamespace( + session_id="sid-active-git", + workspace=str(repo), + active_stream_id="stream-active-git", + ) + + monkeypatch.setattr(routes, "get_session", lambda sid: session) + handler = _CaptureHandler() + with STREAMS_LOCK: + STREAMS[session.active_stream_id] = object() + try: + assert routes._handle_git_commit( + handler, + {"session_id": session.session_id, "message": "Should be blocked"}, + ) is True + finally: + with STREAMS_LOCK: + STREAMS.pop(session.active_stream_id, None) + + assert handler.status == 409 + payload = handler.payload() + assert payload["code"] == "active_stream" + assert "active" in payload["error"].lower() From 7f1feca3fe6fe9257833845549e5dfff966de880 Mon Sep 17 00:00:00 2001 From: Francesco Farinola Date: Wed, 20 May 2026 15:41:42 +0200 Subject: [PATCH 05/10] feat: sidebar tab visibility toggle in Settings > Appearance Add chip row in Settings > Appearance that lets users toggle individual sidebar/rail tabs on or off. Chat and Settings are always visible. - Backend: hidden_tabs list setting with validation (no bool coerce) - Frontend: pill chips that scan rail buttons, autosave via appearance - Boot: _restoreTabVisibility IIFE applies hidden tabs before first paint - i18n: 11 locales (label + description) - Tests: 5 regression tests covering backend, frontend contracts, boot restore, i18n coverage, and settings session tracking --- api/config.py | 6 ++ static/boot.js | 18 ++++ static/i18n.js | 33 ++++++++ static/index.html | 5 ++ static/panels.js | 90 ++++++++++++++++++++ static/style.css | 10 +++ tests/test_sidebar_tab_visibility.py | 119 +++++++++++++++++++++++++++ 7 files changed, 281 insertions(+) create mode 100644 tests/test_sidebar_tab_visibility.py diff --git a/api/config.py b/api/config.py index 067db69e..e0961d59 100644 --- a/api/config.py +++ b/api/config.py @@ -4323,6 +4323,7 @@ _SETTINGS_DEFAULTS = { "font_size": "default", # small | default | large | xlarge "session_jump_buttons": False, # show Start/End transcript jump pills "session_endless_scroll": False, # auto-load older transcript pages while scrolling upward + "hidden_tabs": [], # sidebar tab panel names hidden by user (e.g. ["tasks","kanban"]); chat and settings are always visible "language": "en", # UI locale code; must match a key in static/i18n.js LOCALES "bot_name": os.getenv( "HERMES_WEBUI_BOT_NAME", "Hermes" @@ -4514,6 +4515,11 @@ def save_settings(settings: dict) -> dict: not isinstance(v, str) or not _SETTINGS_LANG_RE.match(v) ): continue + # Validate hidden_tabs: must be a list of non-empty strings + if k == "hidden_tabs": + if not isinstance(v, list): + continue + v = [s for s in v if isinstance(s, str) and s.strip()] # Coerce bool keys if k in _SETTINGS_BOOL_KEYS: v = bool(v) diff --git a/static/boot.js b/static/boot.js index ae24cd5a..da34596b 100644 --- a/static/boot.js +++ b/static/boot.js @@ -283,6 +283,24 @@ function expandSidebar(){ }catch(_){} _syncSidebarAria(); })(); +// ── Boot-time tab visibility ──────────────────────────────────────────────── +// Apply hidden tabs from localStorage before first paint. If the active tab +// is hidden, switch focus to chat. Runs after DOM is ready (unlike theme/skin +// flash-prevention, tab elements aren't available in ). +(function _restoreTabVisibility(){ + try{ + if(typeof _applyTabVisibility==='function'&&typeof _getHiddenTabs==='function'){ + _applyTabVisibility(_getHiddenTabs()); + } + var active=document.querySelector('.rail .rail-btn.nav-tab.active[data-panel]') + ||document.querySelector('.sidebar-nav .nav-tab.active[data-panel]'); + if(active&&active.classList.contains('nav-tab-hidden')){ + var chatBtn=document.querySelector('.rail .rail-btn.nav-tab[data-panel="chat"]'); + if(chatBtn)chatBtn.classList.add('active'); + if(active)active.classList.remove('active'); + } + }catch(_){} +})(); function toggleMobileFiles(){ toggleWorkspacePanel(); } diff --git a/static/i18n.js b/static/i18n.js index 8a5f77ff..36213a3f 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -511,6 +511,9 @@ const LOCALES = { settings_label_session_endless_scroll: 'Load older messages while scrolling up', settings_desc_session_endless_scroll: 'When enabled, older messages load automatically as you scroll upward. When disabled, use the older-messages button.', + + settings_label_tab_visibility: 'Sidebar tabs', + settings_desc_tab_visibility: 'Choose which tabs appear in the sidebar and rail. Chat and Settings are always visible.', open_in_browser: 'Open in browser', settings_dropdown_conversation: 'Conversation', settings_dropdown_appearance: 'Appearance', @@ -1735,6 +1738,9 @@ const LOCALES = { settings_label_session_endless_scroll: 'Carica messaggi precedenti scorrendo in alto', settings_desc_session_endless_scroll: 'Se abilitato, i messaggi precedenti si caricano automaticamente scorrendo in alto. Se disabilitato, usa il pulsante messaggi precedenti.', + + settings_label_tab_visibility: 'Schede della barra laterale', + settings_desc_tab_visibility: 'Scegli quali schede mostrare nella barra laterale e nel rail. Chat e Impostazioni sono sempre visibili.', open_in_browser: 'Apri nel browser', settings_dropdown_conversation: 'Conversazione', settings_dropdown_appearance: 'Aspetto', @@ -2951,6 +2957,9 @@ const LOCALES = { settings_label_session_endless_scroll: '上スクロールで古いメッセージを読み込む', settings_desc_session_endless_scroll: '有効にすると、上にスクロールしたとき古いメッセージを自動で読み込みます。無効の場合は古いメッセージボタンを使います。', + + settings_label_tab_visibility: 'サイドバータブ', + settings_desc_tab_visibility: 'サイドバーとレールに表示するタブを選択します。チャットと設定は常に表示されます。', open_in_browser: 'ブラウザで開く', settings_dropdown_conversation: '会話', settings_dropdown_appearance: '外観', @@ -4688,6 +4697,9 @@ const LOCALES = { settings_label_session_endless_scroll: 'Загружать старые сообщения при прокрутке вверх', settings_desc_session_endless_scroll: 'Если включено, старые сообщения загружаются автоматически при прокрутке вверх. Если выключено, используйте кнопку загрузки старых сообщений.', + + settings_label_tab_visibility: 'Вкладки боковой панели', + settings_desc_tab_visibility: 'Выберите, какие вкладки отображаются на боковой панели и в рейле. Чат и настройки всегда видны.', open_in_browser: 'Открыть в браузере', settings_section_system_title: 'System', settings_tab_appearance: 'Appearance', @@ -5831,6 +5843,9 @@ const LOCALES = { settings_label_session_endless_scroll: 'Cargar mensajes antiguos al desplazarse hacia arriba', settings_desc_session_endless_scroll: 'Si está activado, los mensajes antiguos se cargan automáticamente al desplazarte hacia arriba. Si está desactivado, usa el botón de mensajes antiguos.', + + settings_label_tab_visibility: 'Pestañas de la barra lateral', + settings_desc_tab_visibility: 'Elige qué pestañas aparecen en la barra lateral y el rail. Chat y Configuración siempre están visibles.', open_in_browser: 'Abrir en el navegador', settings_section_system_title: 'System', settings_tab_appearance: 'Appearance', @@ -6706,6 +6721,9 @@ const LOCALES = { settings_desc_session_endless_scroll: 'Wenn aktiviert, werden ältere Nachrichten beim Hochscrollen automatisch geladen. Wenn deaktiviert, nutzt du den Button für ältere Nachrichten.', + settings_label_tab_visibility: 'Seitenleiste-Tabs', + settings_desc_tab_visibility: 'Wähle, welche Tabs in der Seitenleiste und im Rail angezeigt werden. Chat und Einstellungen sind immer sichtbar.', + workspace_drag_hint: 'Ziehen zum Neuordnen', workspace_reorder_failed: 'Neuordnen fehlgeschlagen', open_in_browser: 'Im Browser öffnen', @@ -8147,6 +8165,9 @@ const LOCALES = { settings_label_session_endless_scroll: '向上滚动时加载更早的消息', settings_desc_session_endless_scroll: '启用后，向上滚动时会自动加载更早的消息。禁用时请使用加载更早消息按钮。', + + settings_label_tab_visibility: '侧边栏标签', + settings_desc_tab_visibility: '选择在侧边栏和导航栏中显示哪些标签。聊天和设置始终可见。', open_in_browser: '在浏览器中打开', settings_section_system_title: '系统', settings_tab_appearance: '外观', @@ -8613,6 +8634,9 @@ const LOCALES = { settings_label_session_endless_scroll: '向上捲動時載入較早訊息', settings_desc_session_endless_scroll: '啟用後，向上捲動時會自動載入較早訊息。停用時請使用載入較早訊息按鈕。', + + settings_label_tab_visibility: '側邊欄標籤', + settings_desc_tab_visibility: '選擇在側邊欄和導航列中顯示哪些標籤。聊天和設定始終可見。', open_in_browser: '在瀏覽器中開啓', settings_dropdown_conversation: '對話', settings_dropdown_appearance: '外觀', @@ -9915,6 +9939,9 @@ const LOCALES = { settings_label_session_endless_scroll: 'Carregar mensagens antigas ao rolar para cima', settings_desc_session_endless_scroll: 'Quando ativado, mensagens antigas carregam automaticamente ao rolar para cima. Quando desativado, use o botão de mensagens antigas.', + + settings_label_tab_visibility: 'Abas da barra lateral', + settings_desc_tab_visibility: 'Escolha quais abas aparecem na barra lateral e no rail. Chat e Configurações estão sempre visíveis.', open_in_browser: 'Abrir no navegador', settings_dropdown_conversation: 'Conversa', settings_dropdown_appearance: 'Aparência', @@ -11034,6 +11061,9 @@ const LOCALES = { settings_label_session_endless_scroll: '위로 스크롤할 때 이전 메시지 불러오기', settings_desc_session_endless_scroll: '활성화하면 위로 스크롤할 때 이전 메시지를 자동으로 불러옵니다. 비활성화하면 이전 메시지 버튼을 사용합니다.', + + settings_label_tab_visibility: '사이드바 탭', + settings_desc_tab_visibility: '사이드바와 레일에 표시할 탭을 선택하세요. 채팅과 설정은 항상 표시됩니다.', open_in_browser: '브라우저에서 열기', settings_dropdown_conversation: '대화', settings_dropdown_appearance: '외형', @@ -12168,6 +12198,9 @@ const LOCALES = { settings_desc_session_jump_buttons: 'Affichez les boutons flottants de début et de fin lors de la lecture de longs historiques de session.', settings_label_session_endless_scroll: 'Charger les anciens messages en faisant défiler vers le haut', settings_desc_session_endless_scroll: 'Lorsqu\'ils sont activés, les anciens messages se chargent automatiquement lorsque vous faites défiler vers le haut. Lorsqu\'il est désactivé, utilisez le bouton des messages plus anciens.', + + settings_label_tab_visibility: 'Onglets de la barre latérale', + settings_desc_tab_visibility: 'Choisissez quels onglets apparaissent dans la barre latérale et le rail. Chat et Paramètres sont toujours visibles.', open_in_browser: 'Ouvrir dans le navigateur', settings_dropdown_conversation: 'Conversation', settings_dropdown_appearance: 'Apparence', diff --git a/static/index.html b/static/index.html index 6fd160b3..046089a7 100644 --- a/static/index.html +++ b/static/index.html @@ -971,6 +971,11 @@
When enabled, older messages load automatically as you scroll upward. When disabled, use the older-messages button.
+
+ +
+
Choose which tabs appear in the sidebar and rail. Chat and Settings are always visible.
+
diff --git a/static/panels.js b/static/panels.js index 472a6002..e30da475 100644 --- a/static/panels.js +++ b/static/panels.js @@ -152,6 +152,7 @@ function _beginSettingsPanelSession() { _settingsThemeOnOpen = localStorage.getItem('hermes-theme') || 'dark'; _settingsSkinOnOpen = localStorage.getItem('hermes-skin') || 'default'; _settingsFontSizeOnOpen = localStorage.getItem('hermes-font-size') || 'default'; + _settingsHiddenTabsOnOpen = _getHiddenTabs().slice(); _pendingSettingsTargetPanel = null; if (_settingsAppearanceAutosaveTimer) { clearTimeout(_settingsAppearanceAutosaveTimer); @@ -5109,10 +5110,84 @@ let _settingsHermesDefaultModelOnOpen = ''; let _settingsSection = 'conversation'; let _currentSettingsSection = 'conversation'; let _settingsAppearanceAutosaveTimer = null; +let _settingsHiddenTabsOnOpen = null; // track hidden_tabs at open time for discard revert let _settingsAppearanceAutosaveRetryPayload = null; let _settingsPreferencesAutosaveTimer = null; let _settingsPreferencesAutosaveRetryPayload = null; +// ── Sidebar tab visibility ───────────────────────────────────────────────── +const _ALWAYS_VISIBLE_TABS = new Set(['chat','settings']); +const _HIDDEN_TABS_LS_KEY = 'hermes-webui-hidden-tabs'; + +function _getHiddenTabs(){ + try{var h=localStorage.getItem(_HIDDEN_TABS_LS_KEY);if(h){var p=JSON.parse(h);if(Array.isArray(p))return p;}}catch(e){} + return[]; +} + +function _setHiddenTabs(panels){ + try{localStorage.setItem(_HIDDEN_TABS_LS_KEY,JSON.stringify(panels));}catch(e){} +} + +function _applyTabVisibility(hidden){ + if(!Array.isArray(hidden)) hidden=[]; + // Hide/unhide all [data-panel] elements (sidebar-nav buttons + rail buttons) + document.querySelectorAll('[data-panel]').forEach(function(el){ + var panel=el.dataset.panel; + if(!panel)return; + var shouldHide=hidden.indexOf(panel)!==-1; + el.classList.toggle('nav-tab-hidden',shouldHide); + }); + // If the currently active tab is hidden, switch to chat + var activeRail=document.querySelector('.rail .rail-btn.nav-tab.active[data-panel]'); + var activeSidebar=document.querySelector('.sidebar-nav .nav-tab.active[data-panel]'); + var activeEl=activeRail||activeSidebar; + if(activeEl&&activeEl.classList.contains('nav-tab-hidden')){ + if(typeof switchPanel==='function') switchPanel('chat'); + } +} + +function _renderTabVisibilityChips(){ + var container=$('tabVisibilityChips'); + if(!container)return; + var hidden=_getHiddenTabs(); + // Scan rail buttons to discover all available panels (skip always-visible + dashboard-link) + var tabs=document.querySelectorAll('.rail .rail-btn.nav-tab[data-panel]'); + container.innerHTML=''; + tabs.forEach(function(tab){ + var panel=tab.dataset.panel; + if(!panel||_ALWAYS_VISIBLE_TABS.has(panel))return; + if(tab.classList.contains('dashboard-link'))return; + var label=tab.dataset.tooltip||tab.dataset.label||panel; + // Capitalize first letter + label=label.charAt(0).toUpperCase()+label.slice(1); + var chip=document.createElement('button'); + chip.type='button'; + chip.className='tab-visibility-chip'; + var isOff=hidden.indexOf(panel)!==-1; + if(isOff)chip.classList.add('chip-off'); + chip.textContent=label; + chip.setAttribute('data-tab-panel',panel); + chip.setAttribute('aria-pressed',isOff?'false':'true'); + chip.onclick=function(){_toggleTabVisibilityChip(panel);}; + container.appendChild(chip); + }); +} + +function _toggleTabVisibilityChip(panel){ + if(_ALWAYS_VISIBLE_TABS.has(panel))return; + var hidden=_getHiddenTabs(); + var idx=hidden.indexOf(panel); + if(idx!==-1){ + hidden.splice(idx,1); + }else{ + hidden.push(panel); + } + _setHiddenTabs(hidden); + _applyTabVisibility(hidden); + _renderTabVisibilityChips(); + _scheduleAppearanceAutosave(); +} + function switchSettingsSection(name){ const section=(name==='appearance'||name==='preferences'||name==='providers'||name==='plugins'||name==='system')?name:'conversation'; _settingsSection=section; @@ -5240,6 +5315,7 @@ function _appearancePayloadFromUi(){ font_size: ($('settingsFontSize')||{}).value || localStorage.getItem('hermes-font-size') || 'default', session_jump_buttons: !!($('settingsSessionJumpButtons')||{}).checked, session_endless_scroll: !!($('settingsSessionEndlessScroll')||{}).checked, + hidden_tabs: _getHiddenTabs(), }; } @@ -5266,6 +5342,7 @@ function _rememberAppearanceSaved(payload){ _settingsThemeOnOpen=payload.theme||localStorage.getItem('hermes-theme')||'dark'; _settingsSkinOnOpen=payload.skin||localStorage.getItem('hermes-skin')||'default'; _settingsFontSizeOnOpen=payload.font_size||localStorage.getItem('hermes-font-size')||'default'; + _settingsHiddenTabsOnOpen=Array.isArray(payload.hidden_tabs)?payload.hidden_tabs:[]; } function _scheduleAppearanceAutosave(){ @@ -5495,6 +5572,18 @@ async function loadSettingsPanel(){ _scheduleAppearanceAutosave(); }; } + // Tab visibility chips (dynamically populated from DOM) + var hiddenTabs=[]; + if(Array.isArray(settings.hidden_tabs)){ + // Server value takes priority — even an empty array means "no tabs hidden" + hiddenTabs=settings.hidden_tabs.filter(function(s){return typeof s==='string'&&s.trim();}); + }else{ + // Server has no hidden_tabs key — fall back to localStorage + hiddenTabs=_getHiddenTabs(); + } + _setHiddenTabs(hiddenTabs); + _applyTabVisibility(hiddenTabs); + _renderTabVisibilityChips(); const resolvedLanguage=(typeof resolvePreferredLocale==='function') ? resolvePreferredLocale(settings.language, localStorage.getItem('hermes-lang')) : (settings.language || localStorage.getItem('hermes-lang') || 'en'); @@ -6388,6 +6477,7 @@ function _applySavedSettingsUi(saved, body, opts){ _settingsHermesDefaultModelOnOpen=body.default_model||_settingsHermesDefaultModelOnOpen||''; // Sync window._defaultModel so newSession() uses the just-saved default without a reload (#908). if(body.default_model) window._defaultModel=body.default_model; + _settingsHiddenTabsOnOpen=_getHiddenTabs().slice(); if(typeof clearMessageRenderCache==='function') clearMessageRenderCache(); renderMessages(); if(typeof syncTopbar==='function') syncTopbar(); diff --git a/static/style.css b/static/style.css index 063cf730..38e92017 100644 --- a/static/style.css +++ b/static/style.css @@ -2774,6 +2774,16 @@ main.main.showing-logs > #mainLogs{display:flex;} #mainSettings .settings-field > div[style*="color:var(--muted)"], #mainSettings .settings-field > div[style*="color: var(--muted)"]{font-size:12px;color:var(--muted);line-height:1.5;} +/* Sidebar tab visibility — hide nav-tab and matching rail-btn when toggled off by user */ +.nav-tab-hidden{display:none!important;} + +/* Tab visibility chips in Settings > Appearance */ +.tab-visibility-chips{display:flex;flex-wrap:wrap;gap:6px;margin-top:4px;} +.tab-visibility-chip{font-size:12px;line-height:1;padding:5px 10px;border-radius:var(--radius-pill,999px);border:1px solid var(--border);background:var(--accent-bg);color:var(--accent-text);cursor:pointer;transition:background .15s,border-color .15s,color .15s;white-space:nowrap;user-select:none;} +.tab-visibility-chip:hover{border-color:var(--accent);background:var(--accent-bg-strong);} +.tab-visibility-chip.chip-off{background:var(--input-bg,var(--surface));color:var(--muted);border-color:var(--border);} +.tab-visibility-chip.chip-off:hover{border-color:var(--muted);} + /* Selects & text/password inputs — uniform. Uses !important to win over legacy inline styles that were written for the modal era. */ #mainSettings select, diff --git a/tests/test_sidebar_tab_visibility.py b/tests/test_sidebar_tab_visibility.py new file mode 100644 index 00000000..bd385fe7 --- /dev/null +++ b/tests/test_sidebar_tab_visibility.py @@ -0,0 +1,119 @@ +"""Regression tests for sidebar tab visibility feature. + +Covers backend validation round-trip, frontend static contracts, +i18n coverage, and the key integration points that have broken before. +""" +import json +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +CONFIG_PY = (ROOT / "api" / "config.py").read_text(encoding="utf-8") +PANELS_JS = (ROOT / "static" / "panels.js").read_text(encoding="utf-8") +BOOT_JS = (ROOT / "static" / "boot.js").read_text(encoding="utf-8") +INDEX_HTML = (ROOT / "static" / "index.html").read_text(encoding="utf-8") +STYLE_CSS = (ROOT / "static" / "style.css").read_text(encoding="utf-8") +I18N_JS = (ROOT / "static" / "i18n.js").read_text(encoding="utf-8") + + +def test_backend_round_trip_and_validation(monkeypatch, tmp_path): + """hidden_tabs defaults to [], saves/reloads, rejects non-list, filters empty strings.""" + import api.config as config + settings_path = tmp_path / "settings.json" + monkeypatch.setattr(config, "SETTINGS_FILE", settings_path) + + loaded = config.load_settings() + assert loaded["hidden_tabs"] == [], "default must be empty list" + + saved = config.save_settings({"hidden_tabs": ["kanban", "insights"]}) + assert saved["hidden_tabs"] == ["kanban", "insights"] + assert config.load_settings()["hidden_tabs"] == ["kanban", "insights"] + + # Non-list is rejected, default preserved + bad = config.save_settings({"hidden_tabs": "not-a-list"}) + assert bad["hidden_tabs"] == ["kanban", "insights"] + + # Empty strings filtered, empty list clears + saved = config.save_settings({"hidden_tabs": ["kanban", "", " ", "logs"]}) + assert saved["hidden_tabs"] == ["kanban", "logs"] + cleared = config.save_settings({"hidden_tabs": []}) + assert cleared["hidden_tabs"] == [] + + # Must NOT be in bool keys (would corrupt the list) + assert "hidden_tabs" not in config._SETTINGS_BOOL_KEYS + assert "hidden_tabs" in config._SETTINGS_ALLOWED_KEYS + + +def test_frontend_static_contracts(): + """All required HTML, JS, CSS, and boot elements exist with correct wiring.""" + # HTML: container in Appearance pane + assert 'id="tabVisibilityChips"' in INDEX_HTML + assert 'data-i18n="settings_label_tab_visibility"' in INDEX_HTML + assert 'data-i18n="settings_desc_tab_visibility"' in INDEX_HTML + appearance_start = INDEX_HTML.find('id="settingsPaneAppearance"') + prefs_start = INDEX_HTML.find('id="settingsPanePreferences"', appearance_start + 1) + chips_pos = INDEX_HTML.find('id="tabVisibilityChips"') + assert appearance_start < chips_pos < prefs_start, \ + "tabVisibilityChips must be inside Appearance pane" + + # JS: constants, functions, and wiring + assert "_ALWAYS_VISIBLE_TABS" in PANELS_JS + assert "'chat'" in PANELS_JS.split("_ALWAYS_VISIBLE_TABS")[1][:80] + assert "'settings'" in PANELS_JS.split("_ALWAYS_VISIBLE_TABS")[1][:80] + assert "_HIDDEN_TABS_LS_KEY" in PANELS_JS + assert "hermes-webui-hidden-tabs" in PANELS_JS + for fn in ("_getHiddenTabs", "_setHiddenTabs", "_applyTabVisibility", + "_renderTabVisibilityChips", "_toggleTabVisibilityChip"): + assert f"function {fn}(" in PANELS_JS, f"panels.js must define {fn}()" + + # Toggle must autosave and respect always-visible tabs + toggle_block = PANELS_JS[PANELS_JS.find("function _toggleTabVisibilityChip"):] + toggle_body = toggle_block[:toggle_block.find("\nfunction ", 1) or 2000] + assert "_scheduleAppearanceAutosave" in toggle_body + assert "_ALWAYS_VISIBLE_TABS" in toggle_body + + # Appearance payload must include hidden_tabs + payload_block = PANELS_JS[PANELS_JS.find("function _appearancePayloadFromUi"):] + payload_body = payload_block[:payload_block.find("\nfunction ", 1) or 2000] + assert "hidden_tabs" in payload_body + assert "_getHiddenTabs" in payload_body + + # CSS: hidden class and chip styles + assert ".nav-tab-hidden" in STYLE_CSS + assert "display:none" in STYLE_CSS.split(".nav-tab-hidden")[1][:80].replace(" ", "") + assert ".tab-visibility-chip" in STYLE_CSS + + # No flash-prevention script in (DOM elements don't exist at that point) + head_end = INDEX_HTML.find("") + assert "hermes-webui-hidden-tabs" not in INDEX_HTML[:head_end] + + +def test_boot_restores_visibility_from_localstorage(): + """boot.js must call _applyTabVisibility at boot time so hidden tabs take effect.""" + assert "_restoreTabVisibility" in BOOT_JS + block = BOOT_JS[BOOT_JS.find("_restoreTabVisibility"):][:1500] + assert "_applyTabVisibility" in block, \ + "boot.js must call _applyTabVisibility so tabs are hidden before first paint" + + +def test_i18n_coverage(): + """Label and description keys must exist in all locales with matching counts.""" + label_count = I18N_JS.count("settings_label_tab_visibility") + desc_count = I18N_JS.count("settings_desc_tab_visibility") + assert label_count >= 11, f"Expected ≥11 locales, found {label_count}" + assert desc_count >= 11, f"Expected ≥11 locales, found {desc_count}" + assert label_count == desc_count, \ + f"Label ({label_count}) and desc ({desc_count}) counts must match" + + +def test_settings_session_tracking(): + """Settings open/close lifecycle must track hidden_tabs for discard revert.""" + # _beginSettingsPanelSession snapshots hidden_tabs + begin_block = PANELS_JS[PANELS_JS.find("function _beginSettingsPanelSession"):] + begin_body = begin_block[:begin_block.find("\nfunction ", 1) or 2000] + assert "_settingsHiddenTabsOnOpen" in begin_body + + # _applySavedSettingsUi updates the baseline on full save + apply_block = PANELS_JS[PANELS_JS.find("function _applySavedSettingsUi"):] + apply_body = apply_block[:apply_block.find("\nfunction ", 1) or 5000] + assert "_settingsHiddenTabsOnOpen" in apply_body, \ + "_applySavedSettingsUi must update _settingsHiddenTabsOnOpen after full save" \ No newline at end of file From 5491a542855113c06bdaf92a8f3822805ba050c6 Mon Sep 17 00:00:00 2001 From: Francesco Farinola Date: Wed, 20 May 2026 23:17:00 +0200 Subject: [PATCH 06/10] fix: address PR review feedback on sidebar tab visibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three tweaks from reviewer: 1. Harden _applyTabVisibility to skip always-visible panels even if they appear in hidden_tabs (localStorage tampering, stale server data). Forces shouldHide=false so stale nav-tab-hidden classes on chat/settings get removed, not just skipped. 2. Add synchronous inline
diff --git a/static/panels.js b/static/panels.js index e30da475..9372cbd6 100644 --- a/static/panels.js +++ b/static/panels.js @@ -152,7 +152,6 @@ function _beginSettingsPanelSession() { _settingsThemeOnOpen = localStorage.getItem('hermes-theme') || 'dark'; _settingsSkinOnOpen = localStorage.getItem('hermes-skin') || 'default'; _settingsFontSizeOnOpen = localStorage.getItem('hermes-font-size') || 'default'; - _settingsHiddenTabsOnOpen = _getHiddenTabs().slice(); _pendingSettingsTargetPanel = null; if (_settingsAppearanceAutosaveTimer) { clearTimeout(_settingsAppearanceAutosaveTimer); @@ -5110,7 +5109,6 @@ let _settingsHermesDefaultModelOnOpen = ''; let _settingsSection = 'conversation'; let _currentSettingsSection = 'conversation'; let _settingsAppearanceAutosaveTimer = null; -let _settingsHiddenTabsOnOpen = null; // track hidden_tabs at open time for discard revert let _settingsAppearanceAutosaveRetryPayload = null; let _settingsPreferencesAutosaveTimer = null; let _settingsPreferencesAutosaveRetryPayload = null; @@ -5135,6 +5133,8 @@ function _applyTabVisibility(hidden){ var panel=el.dataset.panel; if(!panel)return; var shouldHide=hidden.indexOf(panel)!==-1; + // Never hide always-visible panels (chat, settings) even if present in hidden_tabs + if(_ALWAYS_VISIBLE_TABS.has(panel)) shouldHide=false; el.classList.toggle('nav-tab-hidden',shouldHide); }); // If the currently active tab is hidden, switch to chat @@ -5342,7 +5342,6 @@ function _rememberAppearanceSaved(payload){ _settingsThemeOnOpen=payload.theme||localStorage.getItem('hermes-theme')||'dark'; _settingsSkinOnOpen=payload.skin||localStorage.getItem('hermes-skin')||'default'; _settingsFontSizeOnOpen=payload.font_size||localStorage.getItem('hermes-font-size')||'default'; - _settingsHiddenTabsOnOpen=Array.isArray(payload.hidden_tabs)?payload.hidden_tabs:[]; } function _scheduleAppearanceAutosave(){ @@ -6477,7 +6476,6 @@ function _applySavedSettingsUi(saved, body, opts){ _settingsHermesDefaultModelOnOpen=body.default_model||_settingsHermesDefaultModelOnOpen||''; // Sync window._defaultModel so newSession() uses the just-saved default without a reload (#908). if(body.default_model) window._defaultModel=body.default_model; - _settingsHiddenTabsOnOpen=_getHiddenTabs().slice(); if(typeof clearMessageRenderCache==='function') clearMessageRenderCache(); renderMessages(); if(typeof syncTopbar==='function') syncTopbar(); diff --git a/tests/test_sidebar_tab_visibility.py b/tests/test_sidebar_tab_visibility.py index bd385fe7..3a936e18 100644 --- a/tests/test_sidebar_tab_visibility.py +++ b/tests/test_sidebar_tab_visibility.py @@ -102,18 +102,4 @@ def test_i18n_coverage(): assert label_count >= 11, f"Expected ≥11 locales, found {label_count}" assert desc_count >= 11, f"Expected ≥11 locales, found {desc_count}" assert label_count == desc_count, \ - f"Label ({label_count}) and desc ({desc_count}) counts must match" - - -def test_settings_session_tracking(): - """Settings open/close lifecycle must track hidden_tabs for discard revert.""" - # _beginSettingsPanelSession snapshots hidden_tabs - begin_block = PANELS_JS[PANELS_JS.find("function _beginSettingsPanelSession"):] - begin_body = begin_block[:begin_block.find("\nfunction ", 1) or 2000] - assert "_settingsHiddenTabsOnOpen" in begin_body - - # _applySavedSettingsUi updates the baseline on full save - apply_block = PANELS_JS[PANELS_JS.find("function _applySavedSettingsUi"):] - apply_body = apply_block[:apply_block.find("\nfunction ", 1) or 5000] - assert "_settingsHiddenTabsOnOpen" in apply_body, \ - "_applySavedSettingsUi must update _settingsHiddenTabsOnOpen after full save" \ No newline at end of file + f"Label ({label_count}) and desc ({desc_count}) counts must match" \ No newline at end of file From 38933b288da3aa202b719bb0a8367c042674f665 Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Wed, 20 May 2026 23:05:19 +0000 Subject: [PATCH 07/10] Stage-394 follow-up: profile-switch reconciliation + a11y switch role + server-side chat/settings filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per deep-review verdict SHIP-WITH-FIXES on PR #2636: 1. Profile-switch reconciliation: _refreshProfileSwitchBackground now re-fetches /api/settings and re-applies hidden_tabs for the new profile. Without this, Profile A's hidden-tabs choice stayed in effect under Profile B until the user opened Settings → Appearance. 2. A11y: switched chips from role=button + aria-pressed to role=switch + aria-checked. The pressed/not-pressed wording confused screen-reader users because chip-off looks like the off state. Added role=group + aria-labelledby on the container, and a :focus-visible style on the chips. 3. Server-side belt-and-suspenders: api/config.py now strips 'chat' and 'settings' from hidden_tabs at validation time, matching the client's apply- time filter. A tampered POST can no longer persist the forbidden values. 3 new regression tests added (chat/settings rejection, profile-switch wiring, chip a11y attributes). Co-authored-by: FrancescoFarinola --- api/config.py | 11 ++++- static/index.html | 4 +- static/panels.js | 18 ++++++++- static/style.css | 1 + tests/test_sidebar_tab_visibility.py | 60 +++++++++++++++++++++++++++- 5 files changed, 88 insertions(+), 6 deletions(-) diff --git a/api/config.py b/api/config.py index e0961d59..480ea2b6 100644 --- a/api/config.py +++ b/api/config.py @@ -4515,11 +4515,18 @@ def save_settings(settings: dict) -> dict: not isinstance(v, str) or not _SETTINGS_LANG_RE.match(v) ): continue - # Validate hidden_tabs: must be a list of non-empty strings + # Validate hidden_tabs: must be a list of non-empty strings. + # Belt-and-suspenders strip of "chat" and "settings" so a + # malicious POST cannot lock the user out of the always-visible + # nav tabs even though the client also filters them at apply time. + # Stage-394 follow-up to #2636 deep review. if k == "hidden_tabs": if not isinstance(v, list): continue - v = [s for s in v if isinstance(s, str) and s.strip()] + v = [ + s for s in v + if isinstance(s, str) and s.strip() and s not in {"chat", "settings"} + ] # Coerce bool keys if k in _SETTINGS_BOOL_KEYS: v = bool(v) diff --git a/static/index.html b/static/index.html index 99d427a7..fa579ac9 100644 --- a/static/index.html +++ b/static/index.html @@ -977,8 +977,8 @@
When enabled, older messages load automatically as you scroll upward. When disabled, use the older-messages button.
- -
+ +
Choose which tabs appear in the sidebar and rail. Chat and Settings are always visible.
diff --git a/static/panels.js b/static/panels.js index 9372cbd6..14bdab9c 100644 --- a/static/panels.js +++ b/static/panels.js @@ -4508,6 +4508,17 @@ function _refreshProfileSwitchBackground(gen){ if (gen !== _profileSwitchGeneration) return; if (S.session && typeof syncTopbar === 'function') syncTopbar(); }).catch(()=>{}); + // Reconcile per-profile sidebar tab visibility. hidden_tabs is a per-profile + // appearance setting; without this fetch, Profile A's hidden-tabs choice + // would remain in effect under Profile B until the user opens Settings. + // Stage-394 follow-up to #2636 deep review. + Promise.resolve(api('/api/settings')).then(function(s){ + if (gen !== _profileSwitchGeneration) return; + var hidden = (s && Array.isArray(s.hidden_tabs)) ? s.hidden_tabs : []; + hidden = hidden.filter(function(x){ return typeof x === 'string' && x.trim(); }); + if (typeof _setHiddenTabs === 'function') _setHiddenTabs(hidden); + if (typeof _applyTabVisibility === 'function') _applyTabVisibility(hidden); + }).catch(function(){}); } async function loadProfilesPanel() { @@ -5167,7 +5178,12 @@ function _renderTabVisibilityChips(){ if(isOff)chip.classList.add('chip-off'); chip.textContent=label; chip.setAttribute('data-tab-panel',panel); - chip.setAttribute('aria-pressed',isOff?'false':'true'); + // Use role="switch" + aria-checked instead of aria-pressed so screen + // readers narrate "Tasks switch on/off" (matches user mental model) rather + // than "Tasks toggle button pressed/not-pressed" (where the polarity is + // confusing because chip-off looks like the "off" state). + chip.setAttribute('role','switch'); + chip.setAttribute('aria-checked',isOff?'false':'true'); chip.onclick=function(){_toggleTabVisibilityChip(panel);}; container.appendChild(chip); }); diff --git a/static/style.css b/static/style.css index 38e92017..fb4a7475 100644 --- a/static/style.css +++ b/static/style.css @@ -2781,6 +2781,7 @@ main.main.showing-logs > #mainLogs{display:flex;} .tab-visibility-chips{display:flex;flex-wrap:wrap;gap:6px;margin-top:4px;} .tab-visibility-chip{font-size:12px;line-height:1;padding:5px 10px;border-radius:var(--radius-pill,999px);border:1px solid var(--border);background:var(--accent-bg);color:var(--accent-text);cursor:pointer;transition:background .15s,border-color .15s,color .15s;white-space:nowrap;user-select:none;} .tab-visibility-chip:hover{border-color:var(--accent);background:var(--accent-bg-strong);} +.tab-visibility-chip:focus-visible{outline:2px solid var(--link,var(--accent));outline-offset:2px;} .tab-visibility-chip.chip-off{background:var(--input-bg,var(--surface));color:var(--muted);border-color:var(--border);} .tab-visibility-chip.chip-off:hover{border-color:var(--muted);} diff --git a/tests/test_sidebar_tab_visibility.py b/tests/test_sidebar_tab_visibility.py index 3a936e18..43ce2f66 100644 --- a/tests/test_sidebar_tab_visibility.py +++ b/tests/test_sidebar_tab_visibility.py @@ -102,4 +102,62 @@ def test_i18n_coverage(): assert label_count >= 11, f"Expected ≥11 locales, found {label_count}" assert desc_count >= 11, f"Expected ≥11 locales, found {desc_count}" assert label_count == desc_count, \ - f"Label ({label_count}) and desc ({desc_count}) counts must match" \ No newline at end of file + f"Label ({label_count}) and desc ({desc_count}) counts must match" + + +def test_backend_rejects_chat_and_settings_in_hidden_tabs(monkeypatch, tmp_path): + """Server-side belt-and-suspenders: a malicious POST that tries to hide + `chat` or `settings` (the always-visible nav tabs) must be filtered out + server-side, not just client-side. The client already applies the same + filter at apply time, but the server should not let a tampered payload + persist the forbidden values.""" + import api.config as config + settings_path = tmp_path / "settings.json" + monkeypatch.setattr(config, "SETTINGS_FILE", settings_path) + + saved = config.save_settings({"hidden_tabs": ["chat", "kanban", "settings", "logs"]}) + assert saved["hidden_tabs"] == ["kanban", "logs"], \ + "chat and settings must be stripped server-side" + + # Even an all-forbidden payload reduces to empty (not rejected — empty is fine) + saved = config.save_settings({"hidden_tabs": ["chat", "settings"]}) + assert saved["hidden_tabs"] == [] + + +def test_profile_switch_reconciles_hidden_tabs(): + """When a user switches profiles, the new profile's hidden_tabs value + must be applied — the per-profile settings.json is the source of truth, + not the previous profile's localStorage value. Stage-394 added a + /api/settings refetch in _refreshProfileSwitchBackground; verify it stays + wired (the API call + the _applyTabVisibility call).""" + bg_start = PANELS_JS.find("function _refreshProfileSwitchBackground") + assert bg_start >= 0, "_refreshProfileSwitchBackground not found" + bg_end = PANELS_JS.find("\nfunction ", bg_start + 1) + if bg_end < 0: + bg_end = bg_start + 4000 + bg_body = PANELS_JS[bg_start:bg_end] + assert "/api/settings" in bg_body, \ + "profile-switch background refresh must re-fetch settings for the new profile" + assert "_applyTabVisibility" in bg_body, \ + "profile-switch background refresh must re-apply tab visibility" + assert "hidden_tabs" in bg_body, \ + "profile-switch background refresh must read hidden_tabs from server response" + + +def test_chip_a11y_uses_switch_role_with_aria_checked(): + """Chips should use role=switch + aria-checked instead of plain + aria-pressed. The pressed/not-pressed wording is confusing for a toggle + that visually represents an on/off switch; role=switch + aria-checked + matches user mental model.""" + render_block = PANELS_JS[PANELS_JS.find("function _renderTabVisibilityChips"):] + body = render_block[:render_block.find("\nfunction ", 1) or 3000] + assert "role" in body and "'switch'" in body, \ + "chip should declare role='switch' for clearer screen-reader narration" + assert "aria-checked" in body, "chip should use aria-checked to match role=switch" + # Group container also has role=group + aria-labelledby + assert 'role="group"' in INDEX_HTML, "chip container needs role=group" + assert 'aria-labelledby="tabVisibilityLabel"' in INDEX_HTML, \ + "chip container needs aria-labelledby pointing at the label" + # Focus-visible style exists + assert ".tab-visibility-chip:focus-visible" in STYLE_CSS, \ + "chip needs a :focus-visible style for keyboard nav" \ No newline at end of file From ea8305d5e2080b38fb23a0cd26873484bb39b436 Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Wed, 20 May 2026 23:38:51 +0000 Subject: [PATCH 08/10] Stage-394 chip CSS contrast: dark text on filled chips for light-theme readability Light-theme review revealed white text on gold chips (color: var(--bg-page)) was washed out and hard to read. Switched to fixed dark text #1a1a1a with font-weight 600 so the on-state reads clearly on the gold accent in both light and dark themes. Off-state unchanged (muted text on transparent). --- static/style.css | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/static/style.css b/static/style.css index fb4a7475..9ddc7cc0 100644 --- a/static/style.css +++ b/static/style.css @@ -2779,11 +2779,11 @@ main.main.showing-logs > #mainLogs{display:flex;} /* Tab visibility chips in Settings > Appearance */ .tab-visibility-chips{display:flex;flex-wrap:wrap;gap:6px;margin-top:4px;} -.tab-visibility-chip{font-size:12px;line-height:1;padding:5px 10px;border-radius:var(--radius-pill,999px);border:1px solid var(--border);background:var(--accent-bg);color:var(--accent-text);cursor:pointer;transition:background .15s,border-color .15s,color .15s;white-space:nowrap;user-select:none;} -.tab-visibility-chip:hover{border-color:var(--accent);background:var(--accent-bg-strong);} +.tab-visibility-chip{font-size:12px;line-height:1;padding:5px 10px;border-radius:var(--radius-pill,999px);border:1px solid var(--accent,var(--link));background:var(--accent,var(--link));color:#1a1a1a;font-weight:600;cursor:pointer;transition:background .15s,border-color .15s,color .15s,opacity .15s;white-space:nowrap;user-select:none;} +.tab-visibility-chip:hover{filter:brightness(0.92);} .tab-visibility-chip:focus-visible{outline:2px solid var(--link,var(--accent));outline-offset:2px;} -.tab-visibility-chip.chip-off{background:var(--input-bg,var(--surface));color:var(--muted);border-color:var(--border);} -.tab-visibility-chip.chip-off:hover{border-color:var(--muted);} +.tab-visibility-chip.chip-off{background:transparent;color:var(--muted);border-color:var(--border);font-weight:400;} +.tab-visibility-chip.chip-off:hover{border-color:var(--muted);color:var(--text);} /* Selects & text/password inputs — uniform. Uses !important to win over legacy inline styles that were written for the modal era. */ From 0774235987bae61ba04a44dcaad22e71ca33ec70 Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Wed, 20 May 2026 23:54:00 +0000 Subject: [PATCH 09/10] Stamp CHANGELOG for v0.51.101 (Release BY / stage-394 / 2-PR deep-review batch) --- CHANGELOG.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e44f6f6..05b81fd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,25 @@ ## [Unreleased] +## [v0.51.101] — 2026-05-20 — Release BY (stage-394 — 2-PR deep-review batch — workspace Git backend + sidebar tab visibility toggle) + +### Added + +- **PR #2625** by @stocky789 — Add backend Git operations for the workspace panel. New `api/workspace_git.py` module exposes read-only ops (`/api/git/status`, `/api/git/branches`, `/api/git/diff`, `/api/git/commit-message[-selected]`) unconditionally and mutating ops (`stage`, `unstage`, `discard`, `commit`, `commit-selected`, `checkout`, `stash-checkout`, `pull`, `push`) only when `HERMES_WEBUI_WORKSPACE_GIT_DESTRUCTIVE=1` is set in the environment — default OFF so existing deployments are unaffected. All subprocess calls use `["git", *args]` with `shell=False`, all branch/ref names go through `git check-ref-format --branch` validation before flowing to `git switch -c`, and `subprocess.env` is scrubbed of `GIT_DIR`/`GIT_WORK_TREE`/`GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM`/`GIT_CONFIG_COUNT`/`GIT_CONFIG_PARAMETERS` plus the full `GIT_CONFIG_KEY_*`/`GIT_CONFIG_VALUE_*` namespace before every invocation. `GIT_INDEX_FILE` is intentionally preserved to drive selected-file commits through a private temporary index. Paths are bound to the workspace root via `safe_resolve_ws()` + `Path.relative_to()` enforcement (rejects `..` traversal and symlinked escapes); active-stream gate prevents mutations during a running agent turn. Documented in `docs/workspace-git.md` with the full trust model (hooks-as-RCE warning, default-allowed vs gated lists, env-scrub enumeration). Frontend UI ships in a follow-up PR. +- **PR #2636** by @FrancescoFarinola — Per-tab sidebar visibility toggle in Settings → Appearance. Power users can hide unused rail tabs (Tasks, Kanban, Skills, Memory, Spaces, Profiles, Todos, Insights, Logs) while keeping Chat and Settings always reachable. Settings is per-profile so each profile can have its own hidden-tabs preference; an inline `