feat(sessions): honor the agent's worktree config default in /api/session/new

/api/session/new treated an absent 'worktree' key as false, so the
agent's config-level 'worktree: true' default was never consulted for
WebUI sessions even though the WebUI reuses the agent's own worktree
helper. Both callers should agree on same-agent/same-repo isolation.

Three-value model:
- body 'worktree' absent -> agent config default applies (profile-aware
  via get_config_for_profile_home, so a user with worktree:true in one
  profile but not another gets per-profile behavior)
- explicit true/false -> honored verbatim; explicit always wins

Non-git workspaces: a config-default request degrades to a plain
session and surfaces 'worktree_skipped' in the payload; an explicit
worktree:true keeps the hard 400.

Frontend (same change, load-bearing): newSession() forwards explicit
true/false and omits the key otherwise; the boot-time auto-bind
(_maybeBindFreshDefaultWorkspaceSession) and the onboarding session
send worktree:false explicitly so a config default can never mint a
worktree + branch from merely opening the UI. Deliberate 'New Chat'
sends nothing and inherits the default.

Tests: route matrix (config on/off x body absent/true/false), non-git
fallback vs explicit 400, explicit-false-beats-config-true, config
helper (ambient/named-profile/fail-soft), static UI assertions that
auto-bind and onboarding opt out and New Chat paths don't pin a value.

Fixes #6022
This commit is contained in:
Rui Xue authored and nesquena-hermes committed 2026-07-14 04:35:06 +00:00
1 parent f617b6bb00
commit 5f638a1694
8 files changed
+366 -11

No files matched your search

+60 -6
View File
@@ -7574,6 +7574,35 @@ def _rescale_threshold_tokens_for_context_window(
return max(1, int(threshold * new_window / old_window)) return max(1, int(threshold * new_window / old_window))
def _worktree_default_from_config(profile: str | None) -> bool:
"""Return the agent's config-level ``worktree:`` default for *profile*.
The agent CLI honors ``worktree: true`` in config.yaml for every session
it creates (``use_worktree = worktree or w or CLI_CONFIG.get("worktree",
False)``). /api/session/new consults this only when the request body has
no explicit ``worktree`` key, so both entry points to the same repo agree
on isolation (#6022). Explicit body values always win.
Profile-aware on purpose: the WebUI serves multiple profiles from one
process, and a user with ``worktree: true`` in one profile but not another
expects per-profile behavior. ``get_config_for_profile_home`` handles the
ambient/common case via the mtime-tracked cache and reads a diverging
profile's config.yaml directly off disk (see #3294).
"""
try:
if profile:
from api.profiles import get_hermes_home_for_profile
cfg_dict = get_config_for_profile_home(get_hermes_home_for_profile(profile))
else:
cfg_dict = get_config_for_profile_home(None)
return bool((cfg_dict or {}).get("worktree", False))
except Exception:
# Config resolution must never break session creation.
logger.warning("failed to read worktree config default", exc_info=True)
return False
def _session_model_state_from_request( def _session_model_state_from_request(
model: str | None, model: str | None,
requested_provider: str | None, requested_provider: str | None,
@@ -13877,10 +13906,22 @@ def handle_post(handler, parsed) -> bool:
except (TypeError, ValueError) as e: except (TypeError, ValueError) as e:
return bad(handler, str(e)) return bad(handler, str(e))
worktree_info = None worktree_info = None
worktree_requested = ( worktree_skipped = None
body.get("worktree") is True # Three-value worktree model (#6022): an explicit body value always
or str(body.get("worktree")).strip().lower() in {"1", "true", "yes", "on"} # wins; an ABSENT key falls back to the agent's config-level
) # ``worktree:`` default so WebUI sessions and CLI sessions agree on
# isolation for the same repo. Clients that must never create a
# worktree (e.g. the boot-time auto-bind) send ``worktree: false``
# explicitly.
raw_worktree = body.get("worktree")
worktree_explicit = raw_worktree is not None
if worktree_explicit:
worktree_requested = (
raw_worktree is True
or str(raw_worktree).strip().lower() in {"1", "true", "yes", "on"}
)
else:
worktree_requested = _worktree_default_from_config(body.get("profile") or None)
if worktree_requested: if worktree_requested:
try: try:
from api.worktrees import create_worktree_for_workspace from api.worktrees import create_worktree_for_workspace
@@ -13890,7 +13931,15 @@ def handle_post(handler, parsed) -> bool:
worktree_info = create_worktree_for_workspace(base_workspace) worktree_info = create_worktree_for_workspace(base_workspace)
workspace = worktree_info["path"] workspace = worktree_info["path"]
except (TypeError, ValueError) as e: except (TypeError, ValueError) as e:
return bad(handler, str(e), status=400) # Explicit requests keep the hard failure. A config-default
# request on a non-git workspace (create_worktree_for_workspace
# raises ValueError) degrades to a plain session instead —
# otherwise `worktree: true` in config.yaml would 400 every
# session in every non-git directory.
if worktree_explicit:
return bad(handler, str(e), status=400)
worktree_info = None
worktree_skipped = str(e)
except Exception as e: except Exception as e:
logger.exception("failed to create worktree-backed session") logger.exception("failed to create worktree-backed session")
return bad(handler, f"Failed to create worktree: {e}", status=500) return bad(handler, f"Failed to create worktree: {e}", status=500)
@@ -13973,7 +14022,12 @@ def handle_post(handler, parsed) -> bool:
profile=getattr(s, "profile", None), profile=getattr(s, "profile", None),
session_id=getattr(s, "session_id", None), session_id=getattr(s, "session_id", None),
) )
return j(handler, {"session": s.compact() | {"messages": s.messages}}) payload = {"session": s.compact() | {"messages": s.messages}}
if worktree_skipped:
# Config-default worktree was skipped (non-git workspace); tell the
# client the session is plain so the UI doesn't assume isolation.
payload["worktree_skipped"] = worktree_skipped
return j(handler, payload)
if parsed.path == "/api/session/compression-recovery/start": if parsed.path == "/api/session/compression-recovery/start":
return _handle_session_compression_recovery_start(handler, body) return _handle_session_compression_recovery_start(handler, body)
+4 -1
View File
@@ -303,7 +303,10 @@ async function _maybeBindFreshDefaultWorkspaceSession(prefillIntent=null){
if(_workspacePanelMode!=='browse') return false; if(_workspacePanelMode!=='browse') return false;
if(!S._profileDefaultWorkspace) return false; if(!S._profileDefaultWorkspace) return false;
try{ try{
await newSession(false, {awaitWorkspaceLoad: true}); // worktree:false is explicit and load-bearing — this auto-bind runs on
// page load, and a config-level worktree default must never leak a fresh
// worktree + branch from simply opening the UI (#6022).
await newSession(false, {awaitWorkspaceLoad: true, worktree: false});
return true; return true;
}catch(e){ }catch(e){
console.warn('[hermes] failed to bind fresh default workspace session', e); console.warn('[hermes] failed to bind fresh default workspace session', e);
+3 -1
View File
@@ -527,7 +527,9 @@ async function _finishOnboarding(){
await loadWorkspaceList(); await loadWorkspaceList();
if(typeof renderSessionList==='function') await renderSessionList(); if(typeof renderSessionList==='function') await renderSessionList();
if(!S.session && typeof newSession==='function'){ if(!S.session && typeof newSession==='function'){
await newSession(true); // System-minted session (no deliberate New Chat intent) — like the boot
// auto-bind, it must not inherit a config-level worktree default (#6022).
await newSession(true, {worktree: false});
await renderSessionList(); await renderSessionList();
} }
} }
+5 -1
View File
@@ -1362,7 +1362,11 @@ async function newSession(flash, options={}){
profile:S.activeProfile||'default', profile:S.activeProfile||'default',
}; };
if(S.session&&S.session.session_id) reqBody.prev_session_id=S.session.session_id; if(S.session&&S.session.session_id) reqBody.prev_session_id=S.session.session_id;
if(options&&options.worktree) reqBody.worktree=true; // Three-value worktree contract (#6022): explicit true/false is forwarded
// verbatim; an ABSENT key lets the server apply the agent's config-level
// `worktree:` default. Auto-bind paths pass worktree:false explicitly so a
// config default can never mint a worktree (+ branch) on mere page load.
if(options&&Object.prototype.hasOwnProperty.call(options,'worktree')) reqBody.worktree=!!options.worktree;
if(Object.prototype.hasOwnProperty.call(options,'project_id')){ if(Object.prototype.hasOwnProperty.call(options,'project_id')){
reqBody.project_id=options.project_id; reqBody.project_id=options.project_id;
} else if(_activeProject&&_activeProject!==NO_PROJECT_FILTER){ } else if(_activeProject&&_activeProject!==NO_PROJECT_FILTER){
+3 -1
View File
@@ -18,7 +18,9 @@ def test_session_new_route_accepts_worktree_flag_and_uses_worktree_info():
def test_new_session_request_can_include_worktree_flag(): def test_new_session_request_can_include_worktree_flag():
src = read("static/sessions.js") src = read("static/sessions.js")
assert "async function newSession(flash, options={})" in src assert "async function newSession(flash, options={})" in src
assert "reqBody.worktree=true" in src # Three-value contract (#6022): explicit true/false forwarded verbatim,
# absent key omitted so the server applies the config default.
assert "reqBody.worktree=!!options.worktree" in src
def test_workspace_dropdown_exposes_new_worktree_conversation_action(): def test_workspace_dropdown_exposes_new_worktree_conversation_action():
@@ -116,7 +116,10 @@ def test_blank_boot_open_panel_auto_binds_default_workspace_session():
result = _helper_driver("browse", "/default-workspace") result = _helper_driver("browse", "/default-workspace")
assert result["bound"] is True assert result["bound"] is True
assert result["calls"] == [{"arg0": False, "arg1": {"awaitWorkspaceLoad": True}}] # worktree:false is explicit and load-bearing (#6022): the boot auto-bind
# must opt out so a config-level worktree default can't mint a worktree
# from merely opening the page.
assert result["calls"] == [{"arg0": False, "arg1": {"awaitWorkspaceLoad": True, "worktree": False}}]
@node_test @node_test
@@ -0,0 +1,232 @@
"""Tests for the config-level worktree default in /api/session/new (Issue #6022).
Three-value contract:
- body ``worktree`` ABSENT -> agent config ``worktree:`` default applies
- body ``worktree`` explicit -> honored verbatim (explicit always wins)
Config-default requests degrade to a plain session (+ ``worktree_skipped`` in
the payload) on non-git workspaces; explicit requests keep the hard 400.
"""
from types import SimpleNamespace
import pytest
import api.models as models
import api.routes as routes
import api.worktrees as worktrees
from api.models import SESSIONS
@pytest.fixture(autouse=True)
def _isolate_sessions(tmp_path, monkeypatch):
session_dir = tmp_path / "sessions"
session_dir.mkdir()
monkeypatch.setattr(models, "SESSION_DIR", session_dir)
monkeypatch.setattr(models, "SESSION_INDEX_FILE", session_dir / "_index.json")
monkeypatch.setenv("GIT_CEILING_DIRECTORIES", str(tmp_path.parent.resolve()))
SESSIONS.clear()
yield session_dir
SESSIONS.clear()
def _post_session_new(tmp_path, monkeypatch, body, *, config_default, workspace_dir=None, fake_worktree=None):
"""Drive POST /api/session/new with a stubbed transport and worktree factory."""
monkeypatch.setattr(routes, "_check_csrf", lambda handler: True)
monkeypatch.setattr(routes, "read_body", lambda handler: body)
monkeypatch.setattr(
routes, "_worktree_default_from_config", lambda profile: config_default
)
if workspace_dir is not None:
monkeypatch.setattr(
routes, "resolve_trusted_workspace", lambda raw: workspace_dir
)
if fake_worktree is not None:
monkeypatch.setattr(
worktrees, "create_worktree_for_workspace", lambda workspace: fake_worktree
)
captured = {}
monkeypatch.setattr(
routes,
"j",
lambda handler, payload, status=200, extra_headers=None: captured.update(
payload=payload, status=status
)
or True,
)
import api.helpers as helpers
monkeypatch.setattr(
helpers,
"j",
lambda handler, payload, status=200, extra_headers=None: captured.update(
payload=payload, status=status
)
or True,
)
assert routes.handle_post(object(), SimpleNamespace(path="/api/session/new")) is True
return captured
def _fake_worktree_info(repo, worktree):
return {
"path": str(worktree),
"branch": "hermes/hermes-6022",
"repo_root": str(repo),
"created_at": 321.0,
}
def _mk_repo_dirs(tmp_path):
repo = tmp_path / "repo"
worktree = repo / ".worktrees" / "hermes-6022"
repo.mkdir()
worktree.mkdir(parents=True)
return repo, worktree
# ── Route matrix: absent key ─────────────────────────────────────────────────
def test_absent_key_with_config_default_off_creates_plain_session(tmp_path, monkeypatch):
repo, _ = _mk_repo_dirs(tmp_path)
captured = _post_session_new(
tmp_path,
monkeypatch,
{"workspace": str(repo), "profile": "default"},
config_default=False,
workspace_dir=repo,
)
assert captured["status"] == 200
session = captured["payload"]["session"]
assert session.get("worktree_path") in (None, "")
assert "worktree_skipped" not in captured["payload"]
def test_absent_key_with_config_default_on_creates_worktree_session(tmp_path, monkeypatch):
repo, worktree = _mk_repo_dirs(tmp_path)
captured = _post_session_new(
tmp_path,
monkeypatch,
{"workspace": str(repo), "profile": "default"},
config_default=True,
workspace_dir=repo,
fake_worktree=_fake_worktree_info(repo, worktree),
)
assert captured["status"] == 200
session = captured["payload"]["session"]
assert session["workspace"] == str(worktree.resolve())
assert session["worktree_path"] == str(worktree.resolve())
assert session["worktree_branch"] == "hermes/hermes-6022"
# ── Route matrix: explicit wins ──────────────────────────────────────────────
def test_explicit_false_beats_config_default_true(tmp_path, monkeypatch):
repo, worktree = _mk_repo_dirs(tmp_path)
def _fail(workspace):
pytest.fail("worktree must not be created when body says worktree=false")
monkeypatch.setattr(worktrees, "create_worktree_for_workspace", _fail)
captured = _post_session_new(
tmp_path,
monkeypatch,
{"workspace": str(repo), "profile": "default", "worktree": False},
config_default=True,
workspace_dir=repo,
)
assert captured["status"] == 200
session = captured["payload"]["session"]
assert session.get("worktree_path") in (None, "")
def test_explicit_true_with_config_default_off_creates_worktree(tmp_path, monkeypatch):
repo, worktree = _mk_repo_dirs(tmp_path)
captured = _post_session_new(
tmp_path,
monkeypatch,
{"workspace": str(repo), "profile": "default", "worktree": True},
config_default=False,
workspace_dir=repo,
fake_worktree=_fake_worktree_info(repo, worktree),
)
assert captured["status"] == 200
session = captured["payload"]["session"]
assert session["worktree_path"] == str(worktree.resolve())
# ── Route matrix: non-git workspace ──────────────────────────────────────────
def test_config_default_on_non_git_workspace_falls_back_to_plain_session(tmp_path, monkeypatch):
non_git = tmp_path / "plain"
non_git.mkdir()
captured = _post_session_new(
tmp_path,
monkeypatch,
{"workspace": str(non_git), "profile": "default"},
config_default=True,
workspace_dir=non_git,
# real create_worktree_for_workspace -> find_git_repo_root raises ValueError
)
assert captured["status"] == 200
session = captured["payload"]["session"]
assert session.get("worktree_path") in (None, "")
assert "not inside a git repository" in captured["payload"]["worktree_skipped"]
def test_explicit_true_on_non_git_workspace_still_hard_400(tmp_path, monkeypatch):
non_git = tmp_path / "plain"
non_git.mkdir()
captured = _post_session_new(
tmp_path,
monkeypatch,
{"workspace": str(non_git), "profile": "default", "worktree": True},
config_default=False,
workspace_dir=non_git,
)
assert captured["status"] == 400
assert "not inside a git repository" in captured["payload"].get("error", "")
# ── Config helper ────────────────────────────────────────────────────────────
def test_worktree_default_reads_ambient_config_when_no_profile(monkeypatch):
monkeypatch.setattr(
routes, "get_config_for_profile_home", lambda home: {"worktree": True}
)
assert routes._worktree_default_from_config(None) is True
monkeypatch.setattr(
routes, "get_config_for_profile_home", lambda home: {"worktree": False}
)
assert routes._worktree_default_from_config(None) is False
monkeypatch.setattr(routes, "get_config_for_profile_home", lambda home: {})
assert routes._worktree_default_from_config(None) is False
def test_worktree_default_resolves_named_profile_home(monkeypatch, tmp_path):
import api.profiles as profiles
seen = {}
monkeypatch.setattr(
profiles, "get_hermes_home_for_profile", lambda name: tmp_path / name
)
def fake_cfg(home):
seen["home"] = home
return {"worktree": True}
monkeypatch.setattr(routes, "get_config_for_profile_home", fake_cfg)
assert routes._worktree_default_from_config("work") is True
assert seen["home"] == tmp_path / "work"
def test_worktree_default_is_fail_soft_on_config_errors(monkeypatch):
def boom(home):
raise RuntimeError("config unavailable")
monkeypatch.setattr(routes, "get_config_for_profile_home", boom)
assert routes._worktree_default_from_config(None) is False
@@ -0,0 +1,55 @@
"""Static assertions for the frontend half of Issue #6022.
The three-value worktree contract only holds if system-minted sessions
(boot-time auto-bind, onboarding) explicitly opt OUT — otherwise a config
``worktree: true`` default would leak a fresh worktree + branch on every page
load, with nothing to reap it (#6023).
"""
from pathlib import Path
ROOT = Path(__file__).resolve().parents[1]
def read(path):
return (ROOT / path).read_text(encoding="utf-8")
def test_new_session_forwards_explicit_worktree_and_omits_absent():
src = read("static/sessions.js")
# Explicit true/false forwarded verbatim; absent key stays absent so the
# server can apply the config default.
assert (
"if(options&&Object.prototype.hasOwnProperty.call(options,'worktree')) "
"reqBody.worktree=!!options.worktree;" in src
)
# The old shape (only-true is ever sent) must be gone.
assert "if(options&&options.worktree) reqBody.worktree=true;" not in src
def test_boot_auto_bind_sends_explicit_worktree_false():
src = read("static/boot.js")
bind = src[src.index("async function _maybeBindFreshDefaultWorkspaceSession") :]
bind = bind[: bind.index("\n}\n")]
assert "worktree: false" in bind
def test_onboarding_session_sends_explicit_worktree_false():
src = read("static/onboarding.js")
finish = src[src.index("async function _finishOnboarding") :]
finish = finish[: finish.index("\n}\n")]
assert "worktree: false" in finish
def test_deliberate_new_chat_paths_do_not_pin_worktree():
# Sidebar "New Chat" and command paths must NOT pass an explicit worktree
# value — they inherit the server-side config default by design.
boot = read("static/boot.js")
for line_no in (
i
for i, line in enumerate(boot.splitlines(), 1)
if "await newSession();await renderSessionList();closeMobileSidebar();" in line
):
line = boot.splitlines()[line_no - 1]
assert "worktree" not in line