From a2ce4e81b51e76284e8c74adac8f796487a69238 Mon Sep 17 00:00:00 2001 From: Mark Baker Date: Wed, 20 May 2026 00:01:02 -0400 Subject: [PATCH 01/81] fix(plugins): distinguish exclusive/provider activation in Settings panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Settings → Plugins panel keyed off `loaded.enabled` and the four agentic visibility hooks, both of which are False/empty for exclusive plugins (memory.provider, model-provider, etc.). Those plugins were mislabeled as "Disabled" with "No registered lifecycle hooks" even when fully functional as the active provider for their category. Surface `manifest.kind` and a derived `activation` field ("enabled" | "disabled" | "exclusive" | "provider") in /api/plugins. The card render picks a third badge state ("Active (provider)") and a dedicated empty-hooks line for those rows. `enabled` is preserved in the payload so older clients still work; new clients should prefer `activation`. Fixes #2659 --- api/routes.py | 21 ++++++++- static/i18n.js | 2 + static/panels.js | 28 ++++++++++-- tests/test_plugins_panel.py | 89 ++++++++++++++++++++++++++++++++++++- 4 files changed, 134 insertions(+), 6 deletions(-) diff --git a/api/routes.py b/api/routes.py index 228eb9f1..d3995580 100644 --- a/api/routes.py +++ b/api/routes.py @@ -3299,6 +3299,13 @@ def _plugin_visibility_payload(manager=None) -> dict: four lifecycle hook names called out by the Settings visibility MVP. It never includes plugin source paths, callback names, callback reprs, or raw load errors because those can contain private filesystem details. + + Exclusive plugins (e.g. memory providers) are activated through their + category's ``.provider`` config, not through ``plugins.enabled``. + Their ``loaded.enabled`` stays False by design and they register hooks + outside the four visibility hooks below. The payload surfaces ``kind`` + and ``activation`` so the panel can render them distinctly instead of + mislabeling them as "Disabled" with no hooks (issue #2659). """ manager = manager or _get_plugin_manager_for_visibility() manager.discover_and_load(force=False) @@ -3316,6 +3323,14 @@ def _plugin_visibility_payload(manager=None) -> dict: name = _clean_plugin_visibility_text(getattr(manifest, "name", "") or plugin_key, limit=120) version = _clean_plugin_visibility_text(getattr(manifest, "version", ""), limit=80) description = _clean_plugin_visibility_text(getattr(manifest, "description", ""), limit=280) + kind = _clean_plugin_visibility_text(getattr(manifest, "kind", "") or "standalone", limit=40) + enabled_flag = bool(getattr(loaded, "enabled", False)) + if kind == "exclusive": + activation = "exclusive" + elif kind == "model-provider" and enabled_flag: + activation = "provider" + else: + activation = "enabled" if enabled_flag else "disabled" registered = [] for hook in list(getattr(manifest, "provides_hooks", []) or []) + list(getattr(loaded, "hooks_registered", []) or []): hook_name = str(hook or "").strip() @@ -3327,7 +3342,11 @@ def _plugin_visibility_payload(manager=None) -> dict: "key": plugin_key or name, "version": version, "description": description, - "enabled": bool(getattr(loaded, "enabled", False)), + # `enabled` is preserved for back-compat with older WebUI clients + # that key off it directly. New clients should prefer `activation`. + "enabled": enabled_flag, + "kind": kind, + "activation": activation, "hooks": registered, }) diff --git a/static/i18n.js b/static/i18n.js index 8a5f77ff..dd415774 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -530,6 +530,8 @@ const LOCALES = { plugins_registered_hooks: 'Registered hooks', plugins_enabled: 'Enabled', plugins_disabled: 'Disabled', + plugins_active_provider: 'Active (provider)', + plugins_provider_no_hooks: 'Provider plugin — no agent-visibility hooks', plugins_load_failed: 'Failed to load plugins: ', settings_tab_system: 'System', settings_title: 'Settings', diff --git a/static/panels.js b/static/panels.js index 472a6002..b8c54da5 100644 --- a/static/panels.js +++ b/static/panels.js @@ -5757,20 +5757,42 @@ function _buildPluginCard(plugin){ const card=document.createElement('div'); card.className='provider-card plugin-card'; card.dataset.plugin=(plugin&&plugin.key)||''; + // `activation` is the canonical state from /api/plugins (added in #2659). + // Fall back to the older `enabled` boolean when the field is missing so + // the panel still works against older backends. + const activation=(plugin&&typeof plugin.activation==='string') + ? plugin.activation + : (plugin&&plugin.enabled===false ? 'disabled' : 'enabled'); + const isProvider=activation==='exclusive'||activation==='provider'; const hooks=Array.isArray(plugin&&plugin.hooks)?plugin.hooks:[]; + // Provider plugins (memory/web/browser/etc.) register hooks on their + // category's dispatcher, not the four agent-wide visibility hooks the + // payload filters to. Show an explanatory line instead of the generic + // "No registered lifecycle hooks" when the visibility-hook list is empty. const hookHtml=hooks.length ? hooks.map(h=>`${esc(h)}`).join('') - : ''+t('plugins_no_hooks')+''; + : ''+t(isProvider?'plugins_provider_no_hooks':'plugins_no_hooks')+''; const version=(plugin&&plugin.version)?' · v'+esc(plugin.version):''; const desc=(plugin&&plugin.description)?esc(plugin.description):t('plugins_no_description'); - const enabled=plugin&&plugin.enabled!==false; + let badgeText; + let badgeClass; + if(isProvider){ + badgeText=t('plugins_active_provider'); + badgeClass='plugin-card-badge-provider'; + }else if(activation==='enabled'){ + badgeText=t('plugins_enabled'); + badgeClass=''; + }else{ + badgeText=t('plugins_disabled'); + badgeClass='plugin-card-badge-disabled'; + } card.innerHTML=`
${esc((plugin&&plugin.name)||t('plugins_unnamed'))}
${esc((plugin&&plugin.key)||'plugin')}${version}
- ${enabled?t('plugins_enabled'):t('plugins_disabled')} + ${badgeText}
${desc}
diff --git a/tests/test_plugins_panel.py b/tests/test_plugins_panel.py index 3022a184..6ca3bce3 100644 --- a/tests/test_plugins_panel.py +++ b/tests/test_plugins_panel.py @@ -10,7 +10,7 @@ def read(path: str) -> str: class _FakeManifest: - def __init__(self, *, name, key, version="", description="", provides_hooks=None, path=None): + def __init__(self, *, name, key, version="", description="", provides_hooks=None, path=None, kind="standalone"): self.name = name self.key = key self.version = version @@ -18,7 +18,7 @@ class _FakeManifest: self.provides_hooks = provides_hooks or [] self.path = path self.source = "user" - self.kind = "standalone" + self.kind = kind class _FakeLoadedPlugin: @@ -86,6 +86,8 @@ class TestPluginsApi: "version": "1.2.3", "description": "Blocks unsafe tool calls", "enabled": True, + "kind": "standalone", + "activation": "enabled", "hooks": ["pre_tool_call", "post_tool_call"], }] serialized = repr(payload) @@ -128,9 +130,70 @@ class TestPluginsApi: plugin = payload["plugins"][0] assert plugin["hooks"] == ["pre_llm_call", "post_llm_call"] assert plugin["enabled"] is False + assert plugin["kind"] == "standalone" + assert plugin["activation"] == "disabled" assert "/tmp/not-a-hook" not in repr(payload) assert "/secret" not in repr(payload) + def test_api_plugins_marks_exclusive_plugins_as_active_provider(self): + # Exclusive plugins (e.g. memory.provider: noema) leave loaded.enabled + # False by design — the bundled discovery scanner records the manifest + # but defers loading to the category's own activation path. Without + # the `activation`/`kind` fields, the panel rendered them as + # "Disabled + no hooks" indistinguishable from broken plugins (#2659). + manager = _FakePluginManager({ + "noema": _FakeLoadedPlugin( + _FakeManifest( + name="noema", + key="noema", + version="0.1.0", + description="Structured memory backed by a Noema Cortex", + kind="exclusive", + ), + enabled=False, + hooks_registered=[], + error="exclusive plugin — activate via .provider config", + ) + }) + + payload = self._capture_plugins_response(manager) + + plugin = payload["plugins"][0] + assert plugin["kind"] == "exclusive" + assert plugin["activation"] == "exclusive" + # `enabled` stays False for back-compat with older WebUI clients that + # key off it directly; new clients must read `activation`. + assert plugin["enabled"] is False + assert plugin["hooks"] == [] + # The raw error string is intentionally NOT surfaced — it can contain + # filesystem paths or other internals on other plugin kinds. + assert "exclusive plugin" not in repr(payload) + + def test_api_plugins_marks_active_model_provider(self): + # model-provider plugins that loaded successfully (loaded.enabled True) + # get the "provider" activation badge so the panel can distinguish + # them from standalone hook plugins. + manager = _FakePluginManager({ + "openrouter": _FakeLoadedPlugin( + _FakeManifest( + name="openrouter", + key="openrouter", + version="1.0.0", + description="OpenRouter model provider", + kind="model-provider", + ), + enabled=True, + hooks_registered=[], + ) + }) + + payload = self._capture_plugins_response(manager) + + plugin = payload["plugins"][0] + assert plugin["kind"] == "model-provider" + assert plugin["activation"] == "provider" + assert plugin["enabled"] is True + class TestPluginsSettingsUi: def test_settings_sidebar_has_plugins_section(self): @@ -159,3 +222,25 @@ class TestPluginsSettingsUi: segment = js[js.find("function _buildPluginCard"):js.find("// ── Providers panel")] assert ".path" not in segment assert ".callback" not in segment + + def test_plugins_panel_renders_active_provider_badge(self): + # The card must distinguish exclusive/provider activation from a plain + # "Enabled" state and use the dedicated empty-hooks message for + # provider plugins instead of "No registered lifecycle hooks" (#2659). + js = read("static/panels.js") + segment = js[js.find("function _buildPluginCard"):js.find("// ── Providers panel")] + + assert "plugin.activation" in segment + assert "'exclusive'" in segment + assert "'provider'" in segment + assert "plugins_active_provider" in segment + assert "plugins_provider_no_hooks" in segment + # Graceful fallback when the older payload shape (no `activation` field) + # is returned — the card should still resolve a badge from `enabled`. + assert "plugin.enabled===false" in segment + + def test_plugins_panel_i18n_strings_present(self): + i18n = read("static/i18n.js") + + assert "plugins_active_provider:" in i18n + assert "plugins_provider_no_hooks:" in i18n From 5fc7aee78187a71283be22577c3b977a88fb0916 Mon Sep 17 00:00:00 2001 From: stocky789 Date: Wed, 20 May 2026 01:20:46 +0000 Subject: [PATCH 02/81] 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 03/81] 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 04/81] 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 05/81] 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 f17d4e204f9377f37b90d29d35ba6cfff309217d Mon Sep 17 00:00:00 2001 From: Michael Lam Date: Wed, 20 May 2026 15:54:44 -0700 Subject: [PATCH 06/81] fix: keep resumed CLI sessions in sidebar cap --- CHANGELOG.md | 3 ++ api/agent_sessions.py | 15 +++++++--- tests/test_issue2628_cli_sessions_perf.py | 34 +++++++++++++++++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e44f6f6..e1741871 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ ## [Unreleased] +### Fixed + +- Keep capped CLI/agent sidebar scans ordered by latest message activity as well as session start time, so long-lived CLI sessions that were resumed recently stay visible in the sidebar window. ## [v0.51.100] — 2026-05-20 — Release BX (stage-393 — 3-PR deep-review batch — lazy journal recovery retry + faster profile-switch + cross-tab session list SSE sync) diff --git a/api/agent_sessions.py b/api/agent_sessions.py index 059fef3e..1b235f7c 100644 --- a/api/agent_sessions.py +++ b/api/agent_sessions.py @@ -433,10 +433,13 @@ def read_importable_agent_session_rows( if result_limit == 0: return [] # The sidebar only needs a small visible window. Bound the expensive - # messages join to a recent-session candidate set instead of + # messages join to a recent-activity candidate set instead of # aggregating every historical Hermes state.db session before - # slicing in Python. Oversampling preserves room for hidden - # compression segments or other rows filtered after projection. + # slicing in Python. The candidate ordering must include the latest + # message timestamp, not only ``started_at``: long-lived CLI sessions + # can be resumed days later and should still surface at the top. + # Oversampling preserves room for hidden compression segments or + # other rows filtered after projection. candidate_limit = max(result_limit * 8, result_limit) cur.execute( f""" @@ -444,7 +447,11 @@ def read_importable_agent_session_rows( SELECT s.id FROM sessions s WHERE {' AND '.join(where_clauses)} - ORDER BY s.started_at DESC + ORDER BY COALESCE( + (SELECT MAX(mx.timestamp) FROM messages mx WHERE mx.session_id = s.id), + s.started_at + ) DESC, + s.started_at DESC LIMIT ? ) {select_sql} diff --git a/tests/test_issue2628_cli_sessions_perf.py b/tests/test_issue2628_cli_sessions_perf.py index c97b5591..1c19d552 100644 --- a/tests/test_issue2628_cli_sessions_perf.py +++ b/tests/test_issue2628_cli_sessions_perf.py @@ -71,9 +71,43 @@ def test_importable_agent_rows_push_sidebar_limit_into_sql(tmp_path): src = (REPO_ROOT / "api" / "agent_sessions.py").read_text() assert "WITH candidates AS" in src assert "JOIN candidates c ON c.id = s.id" in src + assert "SELECT MAX(mx.timestamp) FROM messages mx WHERE mx.session_id = s.id" in src assert "candidate_limit = max(result_limit * 8, result_limit)" in src +def test_importable_agent_rows_limit_includes_resumed_old_session(tmp_path): + """The capped candidate window must not hide old sessions resumed recently.""" + db = tmp_path / "state.db" + _make_state_db(db, sessions=200, messages_per_session=1) + + old_started = time.time() - 60 * 60 * 24 * 30 + recent_activity = time.time() + 60 + conn = sqlite3.connect(str(db)) + conn.execute( + """ + INSERT INTO sessions + (id, source, session_source, title, model, started_at, message_count, parent_session_id, ended_at, end_reason) + VALUES ('cli_resumed_old', 'cli', 'cli', 'Old resumed session', 'openai/gpt-5', ?, 2, NULL, NULL, NULL) + """, + (old_started,), + ) + conn.execute( + "INSERT INTO messages (id, session_id, role, content, timestamp) VALUES ('old_msg_1', 'cli_resumed_old', 'user', 'old hello', ?)", + (old_started,), + ) + conn.execute( + "INSERT INTO messages (id, session_id, role, content, timestamp) VALUES ('old_msg_2', 'cli_resumed_old', 'assistant', 'recent reply', ?)", + (recent_activity,), + ) + conn.commit() + conn.close() + + rows = agent_sessions.read_importable_agent_session_rows(db, limit=20, exclude_sources=("webui",)) + + assert rows[0]["id"] == "cli_resumed_old" + assert rows[0]["actual_message_count"] == 2 + + def test_importable_agent_rows_zero_limit_skips_query_work(tmp_path): db = tmp_path / "state.db" _make_state_db(db, sessions=5, messages_per_session=1) From 7f1feca3fe6fe9257833845549e5dfff966de880 Mon Sep 17 00:00:00 2001 From: Francesco Farinola Date: Wed, 20 May 2026 15:41:42 +0200 Subject: [PATCH 07/81] 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 08/81] 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 09/81] 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 10/81] 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 11/81] 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 `