fix(workspace): new chat inherits the current conversation's workspace (#7180)

* fix(workspace): new chat inherits the current conversation's workspace

`newSession()` resolved the workspace of a new chat as:

    switchWs || S._profileDefaultWorkspace || S.session.workspace

`S._profileDefaultWorkspace` is global to the profile, hydrated from
`/api/profile/active`. Preferring it over the current conversation meant a new
chat opened from a MES conversation landed on the profile-global value instead
of MES. With several conversations running in parallel on different workspaces,
the context the user was actually in was systematically lost.

The precedence is worse than it looks, because that "profile default" is not a
stable configured value: `get_profile_default_workspace()` reads
`last_workspace.txt` BEFORE the configured key (api/workspace.py ~419-427), and
for the `default` profile that file is the GLOBAL one. `set_last_workspace()`
rewrites it on every `/api/chat/start` and `/api/session/update` of ANY
conversation. So "profile default wins over the current session" degrades in
practice to "whatever workspace another conversation last used wins over the
conversation I am in" -- observed live: the pointer flipped mid-operation
because a concurrent turn wrote to it.

New precedence:

    switchWs || S.session.workspace || S._profileDefaultWorkspace

Preserved invariants:
- the one-shot profile-switch flag still wins first and is still consumed (#823);
- a blank page (no loaded session) still falls back to the profile default,
  which remains required for the empty-composer workspace chip (#804/#5169);
- `_profileDefaultWorkspace` is never nulled by `newSession()` (#823).

This deliberately diverges from upstream #4755, which made the profile default
win. That decision assumed a stable configured default; on a shared/global
`last_workspace.txt` it inverts its own intent. The upstream test is rewritten
with the rationale inline so a future rebase does not silently restore the bug,
plus a new test that evaluates the real expression under node and proves the
blank-page fallback still holds.

Pre-existing failures in tests/test_workspace_stale_recovery.py (3) were
verified identical with and without this change.

* fix(workspace): recover stale inherited workspace

---------

Co-authored-by: Anthony Ruiz <ruizanthony@users.noreply.github.com>
This commit is contained in:
ruizanthonyandAnthony Ruiz authored and GitHub committed 2026-08-20 16:36:39 -07:00
1 parent 1f5e45527a
commit eee72f942e
4 files changed
+276 -8

No files matched your search

+29 -1
View File
@@ -14196,6 +14196,29 @@ def _validate_session_toolsets_shape(toolsets):
raise ValueError("each toolset must be a non-empty string")
return toolsets
def _resolve_new_session_workspace(body, visible_prev_session_id):
"""Resolve a new-session workspace, recovering only verified inheritance."""
candidate = body.get("workspace")
if not candidate:
return None
if (
body.get("workspace_inherited_from_prev_session") is not True
or not visible_prev_session_id
):
return str(resolve_trusted_workspace(candidate))
try:
previous_session = get_session(visible_prev_session_id, metadata_only=True)
except KeyError:
return str(resolve_trusted_workspace(candidate))
if str(getattr(previous_session, "workspace", None) or "") != str(candidate):
return str(resolve_trusted_workspace(candidate))
workspace, _recovered = resolve_implicit_workspace_with_recovery(
candidate,
get_last_workspace,
)
return str(workspace)
def handle_post(handler, parsed) -> bool:
"""Handle all POST routes. Returns True if handled, False for 404."""
diag = RequestDiagnostics.maybe_start("POST", parsed.path, logger=logger, print_fn=getattr(handler, '_safe_webui_print', None))
@@ -14529,8 +14552,13 @@ def handle_post(handler, parsed) -> bool:
)
if parsed.path == "/api/session/new":
workspace_prev_session_id = body.get("prev_session_id")
if workspace_prev_session_id and not _session_id_visible_to_request_profile(
handler, workspace_prev_session_id, emit_error=False
):
workspace_prev_session_id = None
try:
workspace = str(resolve_trusted_workspace(body.get("workspace"))) if body.get("workspace") else None
workspace = _resolve_new_session_workspace(body, workspace_prev_session_id)
except (TypeError, ValueError) as e:
return bad(handler, str(e))
worktree_info = None
+9 -4
View File
@@ -1409,15 +1409,20 @@ async function newSession(flash, options={}){
_messagesTruncated=false;
_oldestIdx=0;
clearLiveToolCards();
// One-shot profile-switch workspace wins first; otherwise prefer the profile default.
// Explicit profile switch wins, then the current conversation, then the profile default.
// Provenance lets the server recover only a deleted inherited path; explicit paths stay strict.
const switchWs=S._profileSwitchWorkspace;
S._profileSwitchWorkspace=null;
const inheritWs=switchWs||(S._profileDefaultWorkspace||null)||(S.session?S.session.workspace:null);
const sessionWs=(!switchWs&&S.session)?S.session.workspace:null;
const inheritWs=switchWs||sessionWs||(S._profileDefaultWorkspace||null);
const reqBody={
workspace:inheritWs,
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(sessionWs) reqBody.workspace_inherited_from_prev_session=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
@@ -1453,7 +1458,7 @@ async function newSession(flash, options={}){
}
if(newModelState&&newModelState.model){
reqBody.model=newModelState.model;
// Cold-start / picker-without-provider fallback: when the dropdown option's
// Cold-start / picker-without-provider fallback (#2518): when the dropdown option's
// data-provider is empty/'default' or the persisted state predates provider
// tracking, newModelState.model_provider is null. POST /api/session/new's
// fast path in _resolve_compatible_session_model_state requires both model
@@ -120,7 +120,11 @@ def _new_session_driver(session_workspace: str, default_workspace: str, switch_w
function loadDir(){{return Promise.resolve();}}
{new_session}
newSession().then(()=>{{
process.stdout.write(JSON.stringify({{captured,switchWorkspace:S._profileSwitchWorkspace}}));
process.stdout.write(JSON.stringify({{
captured,
switchWorkspace:S._profileSwitchWorkspace,
profileDefaultWorkspace:S._profileDefaultWorkspace,
}}));
}}).catch(err=>{{
console.error(err && err.stack || err);
process.exit(1);
@@ -130,7 +134,33 @@ def _new_session_driver(session_workspace: str, default_workspace: str, switch_w
@node_test
def test_new_session_prefers_profile_default_over_current_session_workspace():
def test_new_session_prefers_current_session_workspace_over_profile_default():
"""Deliberate divergence from the precedence introduced in #4755.
#4755 made the "profile default" win over the current conversation's workspace.
That precedence assumes the profile default is a stable, configured value. It is
not, for a structural reason:
``api/workspace.py::get_profile_default_workspace()`` reads ``last_workspace.txt``
BEFORE the configured value, and for the ``default`` profile that file is the
GLOBAL one. ``set_last_workspace()`` rewrites it on every ``/api/chat/start`` and
``/api/session/update`` from ANY conversation.
So ``S._profileDefaultWorkspace`` — hydrated from ``/api/profile/active`` — is not a
configured default: it is a volatile shared pointer. "profile default wins over the
current conversation" therefore degrades into "the workspace last used by ANOTHER
conversation wins over the conversation I am actually in". With several
conversations open on different workspaces, opening a new chat from conversation A
created it on conversation B's workspace.
Precedence used here: one-shot profile switch -> current conversation -> profile
default. The profile default remains the blank-page fallback (#804/#5169) and is
never consumed (#823).
MAINTAINER NOTE: this intentionally diverges from #4755 and will conflict if that
precedence is restored. Please do not "fix" it by reinstating the previous order
without first addressing the volatility of ``get_profile_default_workspace()``.
"""
payload = _run_node(_new_session_driver(
session_workspace="/current-workspace",
default_workspace="/profile-default",
@@ -138,8 +168,29 @@ def test_new_session_prefers_profile_default_over_current_session_workspace():
))
assert payload["captured"]["path"] == "/api/session/new"
assert payload["captured"]["body"]["workspace"] == "/profile-default"
assert payload["captured"]["body"]["workspace"] == "/current-workspace"
assert payload["captured"]["body"]["prev_session_id"] == "previous-session"
assert payload["captured"]["body"]["workspace_inherited_from_prev_session"] is True
assert payload["profileDefaultWorkspace"] == "/profile-default"
@node_test
def test_new_session_blank_page_falls_back_to_profile_default():
"""Sans conversation chargée, le défaut de profil reste le repli (#804/#5169)."""
driver = _new_session_driver(
session_workspace="/current-workspace",
default_workspace="/profile-default",
switch_workspace=None,
).replace(
"session:{session_id:'previous-session',workspace:\"/current-workspace\"},",
"session:null,",
)
payload = _run_node(driver)
assert payload["captured"]["body"]["workspace"] == "/profile-default"
assert "workspace_inherited_from_prev_session" not in payload["captured"]["body"]
assert payload["profileDefaultWorkspace"] == "/profile-default"
@node_test
@@ -151,7 +202,28 @@ def test_new_session_one_shot_switch_workspace_still_wins_and_clears():
))
assert payload["captured"]["body"]["workspace"] == "/explicit-switch"
assert "workspace_inherited_from_prev_session" not in payload["captured"]["body"]
assert payload["switchWorkspace"] is None
assert payload["profileDefaultWorkspace"] == "/profile-default"
@node_test
def test_parallel_conversations_each_inherit_their_own_workspace():
first = _run_node(_new_session_driver(
session_workspace="/workspace-a",
default_workspace="/shared-profile-pointer",
switch_workspace=None,
))
second = _run_node(_new_session_driver(
session_workspace="/workspace-b",
default_workspace="/shared-profile-pointer",
switch_workspace=None,
))
assert first["captured"]["body"]["workspace"] == "/workspace-a"
assert second["captured"]["body"]["workspace"] == "/workspace-b"
assert first["captured"]["body"]["workspace_inherited_from_prev_session"] is True
assert second["captured"]["body"]["workspace_inherited_from_prev_session"] is True
@node_test
+163
View File
@@ -119,6 +119,169 @@ def test_resolve_chat_workspace_with_recovery_preserves_explicit_errors(monkeypa
assert saved["count"] == 0
def _post_new_session_with_workspace(
monkeypatch,
body,
previous_session,
fallback,
*,
previous_visible=True,
):
import api.session_lifecycle as session_lifecycle
captured = {"new": [], "fallback": 0}
class _NewSession:
session_id = "new-session"
profile = "default"
messages = []
def __init__(self, workspace):
self.workspace = workspace
def compact(self):
return {
"session_id": self.session_id,
"profile": self.profile,
"workspace": self.workspace,
}
def get_fallback():
captured["fallback"] += 1
return str(fallback)
def create_session(**kwargs):
captured["new"].append(kwargs)
return _NewSession(kwargs["workspace"])
monkeypatch.setattr(routes, "read_body", lambda _handler: body)
monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True)
monkeypatch.setattr(routes, "_csrf_exempt_path", lambda _path: False)
monkeypatch.setattr(routes, "_handle_extension_sidecar_proxy", lambda *_a, **_k: False)
monkeypatch.setattr(routes, "_guard_request_session_visibility", lambda *_a, **_k: True)
monkeypatch.setattr(
routes,
"_session_id_visible_to_request_profile",
lambda *_a, **_k: previous_visible,
)
monkeypatch.setattr(
routes,
"get_session",
lambda sid, metadata_only=False: previous_session
if sid == body.get("prev_session_id")
else (_ for _ in ()).throw(KeyError(sid)),
)
monkeypatch.setattr(routes, "get_last_workspace", get_fallback)
monkeypatch.setattr(routes, "_session_model_state_from_request", lambda *_a: (None, None))
monkeypatch.setattr(routes, "new_session", create_session)
monkeypatch.setattr(
session_lifecycle,
"_register_background_commit_thread",
lambda _thread: False,
)
monkeypatch.setattr(
routes,
"j",
lambda _handler, payload, status=200, **_kwargs: captured.setdefault(
"response", (status, payload)
),
)
monkeypatch.setattr(
routes,
"bad",
lambda _handler, message, status=400, **_kwargs: captured.setdefault(
"error", (status, message)
),
)
handler = SimpleNamespace(command="POST", headers={})
routes.handle_post(handler, urlparse("/api/session/new"))
return captured
def test_session_new_recovers_deleted_workspace_inherited_from_visible_previous_session(
monkeypatch, tmp_path
):
fallback = tmp_path / "fallback"
fallback.mkdir()
stale = tmp_path / "deleted-workspace"
previous = SimpleNamespace(
session_id="previous-session",
profile="default",
workspace=str(stale),
)
body = {
"workspace": str(stale),
"workspace_inherited_from_prev_session": True,
"prev_session_id": previous.session_id,
"profile": "default",
"worktree": False,
}
monkeypatch.setattr(workspace, "_home_path", lambda: tmp_path)
monkeypatch.setattr(workspace, "load_workspaces", lambda: [])
captured = _post_new_session_with_workspace(
monkeypatch,
body,
previous,
fallback,
)
assert "error" not in captured
assert captured["response"][0] == 200
assert captured["response"][1]["session"]["workspace"] == str(fallback.resolve())
assert captured["new"][0]["workspace"] == str(fallback.resolve())
assert captured["fallback"] == 1
@pytest.mark.parametrize(
("mark_inherited", "previous_visible", "stored_workspace"),
[
pytest.param(False, True, "requested", id="explicit-request"),
pytest.param(True, False, "requested", id="foreign-previous-session"),
pytest.param(True, True, "different", id="stored-workspace-mismatch"),
],
)
def test_session_new_keeps_deleted_unverified_workspace_strict(
monkeypatch,
tmp_path,
mark_inherited,
previous_visible,
stored_workspace,
):
fallback = tmp_path / "fallback"
fallback.mkdir()
stale = tmp_path / "deleted-workspace"
previous = SimpleNamespace(
session_id="previous-session",
profile="default",
workspace=(str(stale) if stored_workspace == "requested" else str(tmp_path / "other")),
)
body = {
"workspace": str(stale),
"prev_session_id": previous.session_id,
"profile": "default",
"worktree": False,
}
if mark_inherited:
body["workspace_inherited_from_prev_session"] = True
monkeypatch.setattr(workspace, "_home_path", lambda: tmp_path)
monkeypatch.setattr(workspace, "load_workspaces", lambda: [])
captured = _post_new_session_with_workspace(
monkeypatch,
body,
previous,
fallback,
previous_visible=previous_visible,
)
assert captured["error"][0] == 400
assert "does not exist" in captured["error"][1].lower()
assert captured["new"] == []
assert captured["fallback"] == 0
def test_chat_recovery_preserves_existing_implicit_trust_error(monkeypatch, tmp_path):
home = tmp_path / "home"
fallback = home / "fallback"