diff --git a/CHANGELOG.md b/CHANGELOG.md index d736035d..82c9e68c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +## [v0.51.516] — 2026-06-19 — Release SA (stage per-session toolsets before the first message) + +### Fixed + +- **The composer Tools chip can now stage per-session toolsets before the first conversation exists (#4490).** Applying a custom toolset from the empty composer stores it for the next new session instead of silently no-oping (the documented per-session override #493 was unreachable on a brand-new session), `/api/session/new` now accepts the staged `enabled_toolsets` value with the same structural validation as `/api/session/toolsets`, and staged values are cleared on workspace/profile context switches and when a real session loads so they cannot leak into an unrelated later New Chat. Thanks @franksong2702. + ## [v0.51.515] — 2026-06-19 — Release RZ (remote-gateway approvals work again + conversation history threaded) ### Fixed diff --git a/api/models.py b/api/models.py index 88db56b6..80ff7eec 100644 --- a/api/models.py +++ b/api/models.py @@ -2459,7 +2459,7 @@ def _profile_default_model_state(profile=None): return default_model or get_effective_default_model(), default_provider -def new_session(workspace=None, model=None, profile=None, model_provider=None, project_id=None, worktree_info=None): +def new_session(workspace=None, model=None, profile=None, model_provider=None, project_id=None, worktree_info=None, enabled_toolsets=None): """Create a new in-memory session. The session lives in the SESSIONS dict only — no disk write happens until @@ -2512,6 +2512,7 @@ def new_session(workspace=None, model=None, profile=None, model_provider=None, p worktree_branch=wt.get('branch') if wt else None, worktree_repo_root=wt.get('repo_root') if wt else None, worktree_created_at=wt.get('created_at') if wt else None, + enabled_toolsets=enabled_toolsets, ) with LOCK: SESSIONS[s.session_id] = s diff --git a/api/routes.py b/api/routes.py index db793e54..afcfdff6 100644 --- a/api/routes.py +++ b/api/routes.py @@ -8603,6 +8603,16 @@ def _require_passkey_registration_auth(handler) -> tuple[bool, str, int]: return False, "Authentication required", 401 return True, "", 200 +def _validate_session_toolsets_shape(toolsets): + """Validate per-session toolset override shape without catalog lookup.""" + if toolsets is None: + return None + if not isinstance(toolsets, list) or not toolsets: + raise ValueError("toolsets must be a non-empty list or null") + if not all(isinstance(t, str) and t for t in toolsets): + raise ValueError("each toolset must be a non-empty string") + return toolsets + 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) @@ -8768,6 +8778,10 @@ def handle_post(handler, parsed) -> bool: body.get("model"), body.get("model_provider"), ) + try: + enabled_toolsets = _validate_session_toolsets_shape(body.get("enabled_toolsets")) + except ValueError as e: + return bad(handler, str(e), status=400) # Use the profile sent by the client tab (if any) so that two tabs on # different profiles never clobber each other via the process-level global. # ── Memory lifecycle: commit the previous session before starting a new one ── @@ -8791,6 +8805,7 @@ def handle_post(handler, parsed) -> bool: profile=body.get("profile") or None, project_id=body.get("project_id") or None, worktree_info=worktree_info, + enabled_toolsets=enabled_toolsets, ) if worktree_info: publish_session_list_changed( @@ -9110,12 +9125,10 @@ def handle_post(handler, parsed) -> bool: return bad(handler, str(e)) sid = body["session_id"] toolsets = body.get("toolsets") - # Validate: if not None, must be a non-empty list of strings - if toolsets is not None: - if not isinstance(toolsets, list) or not toolsets: - return bad(handler, "toolsets must be a non-empty list or null") - if not all(isinstance(t, str) and t for t in toolsets): - return bad(handler, "each toolset must be a non-empty string") + try: + toolsets = _validate_session_toolsets_shape(toolsets) + except ValueError as e: + return bad(handler, str(e), status=400) try: s = get_session(sid) except KeyError: diff --git a/static/panels.js b/static/panels.js index 4497e077..9aa3e35e 100644 --- a/static/panels.js +++ b/static/panels.js @@ -5448,7 +5448,7 @@ async function promptWorkspacePath(){ if(!ws)return; try{ const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws})}); - if(r&&r.session){S.session=r.session;S.messages=[];if(typeof syncTopbar==='function')syncTopbar();if(typeof renderMessages==='function')renderMessages();if(typeof renderSessionList==='function')await renderSessionList();} + if(r&&r.session){S._pendingSessionToolsets=null;S.session=r.session;S.messages=[];if(typeof syncTopbar==='function')syncTopbar();if(typeof renderMessages==='function')renderMessages();if(typeof renderSessionList==='function')await renderSessionList();} }catch(e){showToast(t('workspace_switch_failed')+e.message);return;} if(!S.session)return; } @@ -5484,7 +5484,7 @@ async function switchToWorkspace(path,name){ if(!ws){showToast(t('no_workspace'));return;} try{ const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws})}); - if(r&&r.session){S.session=r.session;S.messages=[];if(typeof syncTopbar==='function')syncTopbar();if(typeof renderMessages==='function')renderMessages();if(typeof renderSessionList==='function')await renderSessionList();} + if(r&&r.session){S._pendingSessionToolsets=null;S.session=r.session;S.messages=[];if(typeof syncTopbar==='function')syncTopbar();if(typeof renderMessages==='function')renderMessages();if(typeof renderSessionList==='function')await renderSessionList();} }catch(e){if(typeof setStatus==='function')setStatus(t('switch_failed')+e.message);return;} if(!S.session)return; } @@ -5512,6 +5512,7 @@ async function switchToWorkspace(path,name){ // Explicit workspace switch = user overriding any pending profile-switch default. // Clear the one-shot flag so a subsequent newSession() inherits this choice instead. S._profileSwitchWorkspace=null; + S._pendingSessionToolsets=null; syncTopbar(); await loadDir('.'); if (_currentPanel === 'memory') await loadMemory(true); @@ -5811,6 +5812,7 @@ window.addEventListener('resize',()=>{ }); async function switchToProfile(name) { + S._pendingSessionToolsets=null; // Profile switches are per-client cookie/TLS scoped, so a running stream in // the current session can safely continue while this tab moves to another // profile. The in-flight session stays attached to its original profile. diff --git a/static/sessions.js b/static/sessions.js index 568293fc..631e9253 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -692,6 +692,8 @@ async function newSession(flash, options={}){ if(S.session&&S.session.session_id) reqBody.prev_session_id=S.session.session_id; if(options&&options.worktree) reqBody.worktree=true; if(_activeProject&&_activeProject!==NO_PROJECT_FILTER) reqBody.project_id=_activeProject; + // Forward a pre-session toolset override only from the empty composer (#4490). + if(!S.session && Array.isArray(S._pendingSessionToolsets)) reqBody.enabled_toolsets=S._pendingSessionToolsets; // Carry the visible picker selection into the new session. Without this, // /api/session/new falls back to config.yaml defaults (e.g. gpt-5.5) even // when the user already chose cursor/composer-2.5 in the composer chip. @@ -747,6 +749,7 @@ async function newSession(flash, options={}){ } const data=await api('/api/session/new',{method:'POST',body:JSON.stringify(reqBody)}); S.session=data.session;S.messages=data.session.messages||[]; + S._pendingSessionToolsets=null; if(_sessionSourceFilter==='cli') _sessionSourceFilter='webui'; if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); S.lastUsage={...(data.session.last_usage||{})}; @@ -1067,6 +1070,10 @@ async function loadSession(sid){ return; } S.session=data.session; + // Loading a real existing session abandons any pre-session toolset override + // staged on the empty composer — clear it so it can't leak into a later New + // Chat started from this session (#4490 follow-up). + S._pendingSessionToolsets=null; if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); S.session._modelResolutionDeferred=true; S.lastUsage={...(data.session.last_usage||{})}; diff --git a/static/ui.js b/static/ui.js index 63b631b3..c580c552 100644 --- a/static/ui.js +++ b/static/ui.js @@ -5,7 +5,7 @@ // legacy reverse-scan over S.messages — that keeps new clients working // against old servers (Phase 1 may not yet be deployed everywhere). // See api/todo_state.py for the wire contract. -const S={session:null,messages:[],entries:[],busy:false,pendingFiles:[],toolCalls:[],activeStreamId:null,currentDir:'.',activeProfile:'default',activeProfileIsDefault:true,showHiddenWorkspaceFiles:false,todos:[],todoStateMeta:null}; +const S={session:null,messages:[],entries:[],busy:false,pendingFiles:[],toolCalls:[],activeStreamId:null,currentDir:'.',activeProfile:'default',activeProfileIsDefault:true,showHiddenWorkspaceFiles:false,todos:[],todoStateMeta:null,_pendingSessionToolsets:null}; function assistantDisplayName(){ if(S.activeProfile&&S.activeProfile!=='default') return S.activeProfile.charAt(0).toUpperCase()+S.activeProfile.slice(1); @@ -3092,10 +3092,16 @@ function _applyToolsetsChip(toolsets) { // callers regardless of UI visibility. (#1431) wrap.style.display = ''; const hasCustom = Array.isArray(toolsets) && toolsets.length > 0; + const isStaged = hasCustom + && typeof S !== 'undefined' + && S + && !S.session + && Array.isArray(S._pendingSessionToolsets); if (hasCustom) { - label.textContent = toolsets.join(', '); + const stagedSuffix = isStaged ? ' (staged)' : ''; + label.textContent = toolsets.join(', ') + stagedSuffix; chip.classList.add('has-custom'); - chip.title = t('session_toolsets') + ': ' + toolsets.join(', '); + chip.title = t('session_toolsets') + ': ' + toolsets.join(', ') + stagedSuffix; } else { label.textContent = t('session_toolsets_profile_defaults'); chip.classList.remove('has-custom'); @@ -3105,7 +3111,10 @@ function _applyToolsetsChip(toolsets) { function _syncToolsetsChip() { if (typeof S === 'undefined' || !S || !S.session) { - _applyToolsetsChip(null); + const stagedToolsets = (typeof S !== 'undefined' && S && Array.isArray(S._pendingSessionToolsets)) + ? S._pendingSessionToolsets + : null; + _applyToolsetsChip(stagedToolsets); return; } _applyToolsetsChip(S.session.enabled_toolsets || null); @@ -3268,7 +3277,6 @@ function toggleToolsetsDropdown() { const dd = $('composerToolsetsDropdown'); const chip = $('composerToolsetsChip'); if (!dd || !chip) return; - if (typeof S === 'undefined' || !S || !S.session) return; // Don't open when the chip itself is hidden by responsive CSS (#1431). // offsetParent === null catches display:none on the element or any ancestor. if (chip.offsetParent === null) return; @@ -3303,7 +3311,17 @@ function closeToolsetsDropdown() { } function _applySessionToolsets(toolsets) { - if (typeof S === 'undefined' || !S || !S.session) return; + if (typeof S === 'undefined' || !S) return; + if (!S.session) { + S._pendingSessionToolsets = toolsets; + _applyToolsetsChip(toolsets); + if (Array.isArray(toolsets) && toolsets.length) { + showToast('🔧 ' + t('session_toolsets_applied') + ': ' + toolsets.join(', ')); + } else { + showToast('🌍 ' + t('session_toolsets_cleared')); + } + return; + } const sid = S.session.session_id; api('/api/session/toolsets', { method: 'POST', @@ -13806,7 +13824,7 @@ async function promptNewFile(targetDir = S.currentDir || '.'){ if(!ws) return; try{ const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws})}); - if(r&&r.session){S.session=r.session;S.messages=[];syncTopbar();renderMessages();await renderSessionList();} + if(r&&r.session){S._pendingSessionToolsets=null;S.session=r.session;S.messages=[];syncTopbar();renderMessages();await renderSessionList();} }catch(e){setStatus(t('create_failed')+e.message);return;} } if(!S.session)return; @@ -13833,7 +13851,7 @@ async function promptNewFolder(targetDir = S.currentDir || '.'){ if(!ws) return; try{ const r=await api('/api/session/new',{method:'POST',body:JSON.stringify({workspace:ws})}); - if(r&&r.session){S.session=r.session;S.messages=[];syncTopbar();renderMessages();await renderSessionList();} + if(r&&r.session){S._pendingSessionToolsets=null;S.session=r.session;S.messages=[];syncTopbar();renderMessages();await renderSessionList();} }catch(e){setStatus(t('folder_create_failed')+e.message);return;} } if(!S.session)return; diff --git a/tests/test_issue2518_active_provider_fallback.py b/tests/test_issue2518_active_provider_fallback.py index 15bf0fba..2763484b 100644 --- a/tests/test_issue2518_active_provider_fallback.py +++ b/tests/test_issue2518_active_provider_fallback.py @@ -89,7 +89,10 @@ class TestClientFallbackSourceShape: """Future readers should be able to trace this back to the issue.""" src = _read("static/sessions.js") idx = src.find("async function newSession(flash, options={}){") - body = src[idx:idx + 4000] + # Window covers the model-fallback region of newSession(); the function + # has grown over time (e.g. pre-session toolset staging #4490), so keep + # the window comfortably larger than the fallback block it guards. + body = src[idx:idx + 5000] assert "#2518" in body, ( "newSession()'s fallback comment should reference #2518 so the " "follow-up provenance survives future refactors." diff --git a/tests/test_issue4490_presession_toolsets.py b/tests/test_issue4490_presession_toolsets.py new file mode 100644 index 00000000..b88b5875 --- /dev/null +++ b/tests/test_issue4490_presession_toolsets.py @@ -0,0 +1,188 @@ +"""Regression tests for #4490 pre-session toolset staging.""" + +from __future__ import annotations + +import json +import tempfile +from pathlib import Path +from urllib.parse import urlparse +from unittest.mock import patch + +from api.models import new_session +from api.routes import handle_post + +REPO = Path(__file__).resolve().parents[1] +UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") +SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8") +PANELS_JS = (REPO / "static" / "panels.js").read_text(encoding="utf-8") + + +def _function_body(src: str, signature: str) -> str: + start = src.index(signature) + brace = src.index("{", start) + depth = 0 + for i in range(brace, len(src)): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + if depth == 0: + return src[start : i + 1] + raise AssertionError(f"function body not found: {signature}") + + +class _DummyHandler: + command = "POST" + + def __init__(self, body: dict): + raw = json.dumps(body).encode("utf-8") + self.headers = {"Content-Length": str(len(raw))} + self.rfile = tempfile.SpooledTemporaryFile() + self.rfile.write(raw) + self.rfile.seek(0) + self.status = None + self.response = {} + self.wfile = tempfile.SpooledTemporaryFile() + self.client_address = ("127.0.0.1", 12345) + + def send_response(self, code: int): + self.status = code + + def send_header(self, key: str, value: str): + self.response.setdefault("headers", {})[key] = value + + def end_headers(self): + pass + + def payload(self) -> dict: + self.wfile.seek(0) + return json.loads(self.wfile.read().decode("utf-8")) + + +def test_toggle_toolsets_dropdown_opens_without_session_guard(): + toggle = _function_body(UI_JS, "function toggleToolsetsDropdown") + + assert "!S.session" not in toggle + assert "chip.offsetParent === null" in toggle + assert "_populateToolsetsDropdown();" in toggle + + +def test_apply_toolsets_stages_without_session_and_skips_post(): + apply_body = _function_body(UI_JS, "function _applySessionToolsets") + compact = apply_body.replace(" ", "") + + assert "S._pendingSessionToolsets=toolsets" in compact + pre_session_branch = apply_body[: apply_body.index("const sid = S.session.session_id;")] + assert "api('/api/session/toolsets'" not in pre_session_branch + assert "_applyToolsetsChip(toolsets)" in pre_session_branch + + +def test_staged_toolsets_are_visibly_marked_on_chip(): + apply_chip = _function_body(UI_JS, "function _applyToolsetsChip") + + assert "(staged)" in apply_chip + assert "!S.session" in apply_chip + assert "S._pendingSessionToolsets" in apply_chip + + +def test_new_session_request_consumes_pending_toolsets_once(): + compact = SESSIONS_JS.replace(" ", "") + + post_start = compact.index("api('/api/session/new'") + before_post = compact[:post_start] + after_assignment = compact[compact.index("S.session=data.session") :] + + assert "Array.isArray(S._pendingSessionToolsets)" in before_post + assert "reqBody.enabled_toolsets=S._pendingSessionToolsets" in before_post + assert "S._pendingSessionToolsets=null" in after_assignment + + +def test_pending_toolsets_only_forwarded_from_empty_composer(): + """The staged override must only be forwarded when there is no active session. + + Without the `!S.session` guard, a toolset staged on the empty composer would + leak into a later New Chat started from an existing session (#4490 follow-up). + """ + compact = SESSIONS_JS.replace(" ", "") + post_start = compact.index("api('/api/session/new'") + before_post = compact[:post_start] + # The forwarding line must be gated on the no-session (empty composer) state. + assert "!S.session&&Array.isArray(S._pendingSessionToolsets)" in before_post + + +def test_load_existing_session_clears_staged_toolsets(): + """Loading a real existing session must clear any abandoned staged override. + + loadSession() assigns S.session=data.session on the success path; the staged + value is cleared there so a subsequent New Chat does not inherit it (#4490). + """ + body = _function_body(SESSIONS_JS, "async function loadSession") + compact = body.replace(" ", "") + assign = compact.index("S.session=data.session") + # The clear must accompany the real-session assignment, not only the create path. + assert "S._pendingSessionToolsets=null" in compact[assign : assign + 400] + + +def test_workspace_and_profile_switches_clear_pending_toolsets(): + for marker in ( + "function promptWorkspacePath", + "function switchToWorkspace", + "function switchToProfile", + ): + body = _function_body(PANELS_JS, marker) + assert "S._pendingSessionToolsets=null" in body.replace(" ", "") + + assert "S._pendingSessionToolsets=null" in UI_JS.replace(" ", "") + + +def test_context_switch_auto_sessions_do_not_forward_staged_toolsets(): + """Workspace/profile context switches intentionally reset staged values. + + The staged toolset belongs to the empty composer context. Auto-creating a + workspace/file session from another context should not silently carry it + forward; those paths clear the staged value after creating their session. + """ + for src, marker in ( + (PANELS_JS, "function promptWorkspacePath"), + (PANELS_JS, "function switchToWorkspace"), + (UI_JS, "function promptNewFile"), + (UI_JS, "function promptNewFolder"), + ): + body = _function_body(src, marker) + assert "enabled_toolsets" not in body + assert "S._pendingSessionToolsets=null" in body.replace(" ", "") + + +def test_backend_new_session_accepts_enabled_toolsets(): + with tempfile.TemporaryDirectory() as tmp: + session = new_session(workspace=tmp, enabled_toolsets=["filesystem", "shell"]) + + assert session.enabled_toolsets == ["filesystem", "shell"] + + +def test_api_session_new_accepts_enabled_toolsets(): + with tempfile.TemporaryDirectory() as tmp, patch( + "api.routes.get_last_workspace", return_value=tmp + ): + handler = _DummyHandler({"enabled_toolsets": ["filesystem", "shell"]}) + handle_post(handler, urlparse("/api/session/new")) + + payload = handler.payload() + assert handler.status == 200 + assert payload["session"]["enabled_toolsets"] == ["filesystem", "shell"] + + +def test_api_session_new_rejects_malformed_enabled_toolsets(): + cases = [ + {"enabled_toolsets": []}, + {"enabled_toolsets": "filesystem"}, + {"enabled_toolsets": ["filesystem", ""]}, + {"enabled_toolsets": ["filesystem", 42]}, + ] + with tempfile.TemporaryDirectory() as tmp, patch( + "api.routes.get_last_workspace", return_value=tmp + ): + for body in cases: + handler = _DummyHandler(body) + handle_post(handler, urlparse("/api/session/new")) + assert handler.status == 400