diff --git a/CHANGELOG.md b/CHANGELOG.md index c6fe66f3..22f82fa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **A `/goal` launch no longer silently switches providers mid-session after a deliberate model pick.** When a session had a deliberate cross-provider model/provider choice, running `/goal` omitted the `explicit_model_pick` marker that `/api/chat/start` sends, so the model resolver treated the persisted pick as stale and "repaired" it back to the profile default — silently changing providers while the goal ran. Both the frontend `/goal` command and the backend `_handle_goal_command` (initial kickoff and kickoff-prompt fallback paths) now carry the explicit-pick marker and stamp the pick signature, matching `/api/chat/start`; ordinary default sessions are unaffected (they still omit the marker, so a default pick stays repairable). Thanks @webtecnica. (#6705) + - **A completed assistant turn no longer renders twice in the feed.** In `renderMessages()`, the live-turn preserve guard kept the in-flight assistant node across the DOM rebuild whenever it held any rendered content, so when a stream ended but its inflight bookkeeping was not yet cleared the just-completed turn was re-attached over the rebuilt settled transcript (a first copy without the model label, then a second with it). The stored data was always clean (state.db, sidecar, and `/api/session` each hold one row) — the duplicate existed only in the DOM. Preservation now requires a provable live owner (`S.activeStreamId` or explicit live-projection markers) instead of bare DOM content; the mid-stream flicker guard and blank-turn guard are preserved. Thanks @webtecnica. (#7109) - **Visible inline chat videos no longer appear stalled after metadata.** Every inline video stayed at `preload="metadata"`, so a currently-visible mobile player could look frozen because the browser stops loading after metadata. A shared IntersectionObserver now promotes a near-viewport video to `preload="auto"` (and immediately on play), keeps off-screen history videos at metadata to avoid buffering every historical video, and unobserves nodes removed before they intersect. No media URL, caching, or controls change. Thanks @allenliang2022. (#7076) diff --git a/api/routes.py b/api/routes.py index 5c91ec9d..492b522d 100644 --- a/api/routes.py +++ b/api/routes.py @@ -22592,6 +22592,7 @@ def _handle_goal_command(handler, body): and not stream_running ) workspace = model = model_provider = normalized_model = None + explicit_model_pick = bool(body.get("explicit_model_pick")) previous_goal_state = None if will_kickoff: try: @@ -22604,6 +22605,11 @@ def _handle_goal_command(handler, body): if "model_provider" in body else getattr(s, "model_provider", None) ) + # #6703: carry the explicit-pick marker through goal kickoffs. The + # frontend marks a session-level provider/model choice as explicit (same + # signal /api/chat/start receives); without it the model resolver treats + # a persisted cross-provider pick as stale and "repairs" it back to the + # profile default, silently switching providers mid-session. _pp_provider, _pp_default, _pp_cfg = _read_profile_model_config(s, requested_provider) model, model_provider, normalized_model = _resolve_compatible_session_model_state( requested_model, @@ -22611,7 +22617,20 @@ def _handle_goal_command(handler, body): profile_provider=_pp_provider, profile_default_model=_pp_default, profile_config=_pp_cfg, + explicit_model_pick=explicit_model_pick, ) + # #5979/#6703 parity with chat-start: record a SIGNATURE of the + # deliberately-picked model+provider so the streaming resolver can + # preserve a custom-proxy vendor namespace on a cold catalog. A first + # /goal launch after a deliberate custom-provider pick must survive a + # cold streaming catalog exactly like /api/chat/start does; otherwise the + # provider reverts to the profile default mid-session. + try: + if explicit_model_pick: + from api.models import model_explicit_pick_signature as _mk_sig + s.model_explicit_pick_signature = _mk_sig(model, model_provider) + except Exception: + pass previous_goal_state = goal_state_snapshot(s.session_id, profile_home=profile_home) from api.runtime_adapter import LegacyJournalRuntimeAdapter, runtime_adapter_enabled @@ -22663,7 +22682,16 @@ def _handle_goal_command(handler, body): profile_provider=_pp_provider, profile_default_model=_pp_default, profile_config=_pp_cfg, + explicit_model_pick=explicit_model_pick, ) + # #6703 parity: same explicit-pick signature stamping on the + # kickoff-prompt fallback resolution path as /api/chat/start. + try: + if explicit_model_pick: + from api.models import model_explicit_pick_signature as _mk_sig + s.model_explicit_pick_signature = _mk_sig(model, model_provider) + except Exception: + pass stream_response = _start_chat_stream_for_session( s, msg=kickoff_prompt, diff --git a/static/commands.js b/static/commands.js index 8bef376d..74e5eb49 100644 --- a/static/commands.js +++ b/static/commands.js @@ -1232,12 +1232,39 @@ async function cmdGoal(args){ if(!S.session||!S.session.session_id){showToast(t('no_active_session'));return;} const activeSid=S.session.session_id; try{ + // #6703: re-assert the explicit-pick marker on /api/goal the same way + // /api/chat/start does. Without it the server's model resolver treats a + // persisted cross-provider pick as stale and silently reverts the session + // to the profile default mid-session (e.g. while /goal is running). + const _goalModel=S.session.model||($('modelSelect')&&$('modelSelect').value)||''; + const _goalProvider=S.session.model_provider||null; + const _pendingPick=(typeof _readPendingSessionModel==='function') + ? _readPendingSessionModel(activeSid) + : null; + const _pendingPickMatch=_pendingPick + && _pendingPick.model===_goalModel + && String(_pendingPick.model_provider||'')===String(_goalProvider||''); + const _defaultModel=(typeof window!=='undefined' && window._defaultModel)||''; + const _activeProvider=(typeof window!=='undefined' && window._activeProvider)||null; + const _isCrossProviderPick=_goalModel + && _goalProvider + && _defaultModel + && _activeProvider + && _goalModel !== _defaultModel + && String(_goalProvider||'') !== String(_activeProvider||''); + const _explicitPick=(_pendingPickMatch||_isCrossProviderPick)||undefined; + // Do NOT consume the pending explicit-pick marker here: a control-only + // invocation (e.g. /goal status) skips server-side model resolution, so a + // pre-request clear would drop the pick without using it. Consume it below, + // only after a successful kickoff (r.stream_id), re-checking that the stored + // marker still matches the model/provider captured for this kickoff (#6705). const r=await api('/api/goal',{method:'POST',body:JSON.stringify({ session_id:activeSid, args:args||'', workspace:S.session.workspace, - model:S.session.model||($('modelSelect')&&$('modelSelect').value)||'', - model_provider:S.session.model_provider||null, + model:_goalModel, + model_provider:_goalProvider, + explicit_model_pick:_explicitPick, profile:S.activeProfile||S.session.profile||'default', })}); const msg = (() => { @@ -1257,6 +1284,19 @@ async function cmdGoal(args){ showToast(msg.split('\n')[0],2600); } if(!r||!r.stream_id)return; + // #6705: consume the one-shot pending explicit-pick marker only after a + // successful kickoff. Re-read the stored marker and clear it only if it + // still matches the model/provider captured above — a control command (no + // stream_id) must leave the marker intact for the next real send, and a + // marker re-recorded mid-flight (newer onchange) must not be clobbered. + if(_pendingPickMatch && typeof _readPendingSessionModel==='function' && typeof _clearPendingSessionModel==='function'){ + const _stillPending=_readPendingSessionModel(activeSid); + if(_stillPending + && _stillPending.model===_goalModel + && String(_stillPending.model_provider||'')===String(_goalProvider||'')){ + _clearPendingSessionModel(activeSid); + } + } S.toolCalls=[]; if(typeof clearLiveToolCards==='function')clearLiveToolCards(); appendThinking();setBusy(true); diff --git a/tests/test_goal_command_js_behaviour.py b/tests/test_goal_command_js_behaviour.py new file mode 100644 index 00000000..80bb1d17 --- /dev/null +++ b/tests/test_goal_command_js_behaviour.py @@ -0,0 +1,260 @@ +"""Behavioural tests that drive the ACTUAL cmdGoal() from static/commands.js via node. + +The source-inspection regressions in test_goal_command_webui.py assert that +certain expressions/order exist inside cmdGoal's source text. They can stay +green when a refactor preserves those strings but sends the wrong +explicit_model_pick or consumes the pending session-model marker at the wrong +time (#6705, greptile-apps P2). This file closes that gap by spawning node on +the real static/commands.js, extracting cmdGoal, and driving it against a +mocked browser environment (sessionStorage, S, window, api) — asserting the +OBSERVABLE effects: the explicit_model_pick field on the /api/goal payload and +the pending marker's survival/consumption in sessionStorage. + +Mirrors the approach of test_renderer_js_behaviour.py. +""" +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] +COMMANDS_JS_PATH = REPO_ROOT / "static" / "commands.js" + +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +_DRIVER_SRC = r""" +const fs = require('fs'); +const src = fs.readFileSync(process.argv[2], 'utf8'); +const scenario = process.argv[3] || ''; + +// ---- mocked browser environment ---- +const _store = new Map(); +global.sessionStorage = { + getItem: k => (_store.has(k) ? _store.get(k) : null), + setItem: (k, v) => { _store.set(k, String(v)); }, + removeItem: k => { _store.delete(k); }, +}; +global.window = {}; + +// Pending session-model marker helpers mirroring static/ui.js +// (PENDING_SESSION_MODEL_PREFIX / _readPendingSessionModel / _clearPendingSessionModel). +const PENDING_PREFIX = 'hermes-webui-pending-session-model:'; +const MAX_AGE_MS = 10 * 60 * 1000; +const _key = sid => PENDING_PREFIX + String(sid || ''); +function rememberPending(sid, model, provider) { + const s = String(sid || '').trim(); + const value = String(model || '').trim(); + if (!s || !value) return; + try { + sessionStorage.setItem(_key(s), JSON.stringify({ + model: value, + model_provider: provider ? String(provider).trim() : null, + saved_at: Date.now(), + })); + } catch (_) {} +} +function readPending(sid) { + const s = String(sid || '').trim(); + if (!s) return null; + try { + const raw = sessionStorage.getItem(_key(s)); + if (!raw) return null; + const parsed = JSON.parse(raw); + const model = String(parsed && parsed.model || '').trim(); + if (!model) { sessionStorage.removeItem(_key(s)); return null; } + const savedAt = Number(parsed.saved_at || 0); + if (savedAt && Date.now() - savedAt > MAX_AGE_MS) { sessionStorage.removeItem(_key(s)); return null; } + return { + model, + model_provider: parsed && parsed.model_provider ? String(parsed.model_provider) : null, + }; + } catch (_) { return null; } +} +function clearPending(sid) { + const s = String(sid || '').trim(); + if (!s) return; + try { sessionStorage.removeItem(_key(s)); } catch (_) {} +} +const _readPendingSessionModel = readPending; +const _clearPendingSessionModel = clearPending; + +// ---- command helpers the extracted cmdGoal references ---- +const t = k => k; +const showToast = () => {}; +const renderMessages = () => {}; +const clearLiveToolCards = () => {}; +const appendThinking = () => {}; +const setBusy = () => {}; +const setComposerStatus = () => {}; +const markInflight = () => {}; +const saveInflightState = () => {}; +const startApprovalPolling = () => {}; +const startClarifyPolling = () => {}; +const _fetchYoloState = () => {}; +const attachLiveStream = () => {}; +const renderSessionList = () => {}; +const newSession = async () => {}; +const $ = () => null; +const INFLIGHT = {}; + +// ---- api mock: record every /api/goal payload, respond per scenario ---- +const _apiCalls = []; +let _nextResponse = () => ({}); +async function api(url, opts) { + _apiCalls.push({ url, body: JSON.parse(opts.body) }); + return _nextResponse(); +} + +// ---- extract cmdGoal from the real file and evaluate it ---- +function extractFunc(name) { + // Preserve a leading `async` keyword — dropping it would make the + // extracted `await` statements a SyntaxError. + const re = new RegExp('(?:async\\s+)?function\\s+' + name + '\\s*\\('); + const m = re.exec(src); + if (!m) throw new Error(name + ' not found'); + const start = m.index; + let i = src.indexOf('{', start); + let depth = 1; i++; + while (depth > 0 && i < src.length) { + if (src[i] === '{') depth++; + else if (src[i] === '}') depth--; + i++; + } + return src.slice(start, i); +} +eval(extractFunc('cmdGoal')); + +// ---- scenario state ---- +const SID = 'sid-6705-behaviour'; +const S = { + session: { + session_id: SID, + workspace: '/tmp/ws', + model: 'openai/gpt-5.4', + model_provider: 'openai', + profile: 'default', + active_stream_id: null, + }, + activeProfile: 'default', + messages: [], + toolCalls: [], + activeStreamId: null, +}; + +(async () => { + const out = {}; + if (scenario === 'kickoff_consumes') { + // Pending pick matches the session model; server returns a real kickoff. + rememberPending(SID, 'openai/gpt-5.4', 'openai'); + window._defaultModel = 'gpt-4o'; window._activeProvider = 'openai'; + _nextResponse = () => ({ stream_id: 's1', pending_started_at: 1, + effective_model: 'openai/gpt-5.4', effective_model_provider: 'openai' }); + await cmdGoal('ship it'); + out.payload = _apiCalls[0].body; + out.markerAfter = readPending(SID); + } else if (scenario === 'control_then_kickoff') { + // Control-only /goal status: server responds WITHOUT stream_id. + rememberPending(SID, 'openai/gpt-5.4', 'openai'); + window._defaultModel = 'gpt-4o'; window._activeProvider = 'openai'; + _nextResponse = () => ({ message: 'no active goal', message_key: 'goal_no_active' }); + await cmdGoal('status'); + out.controlPayload = _apiCalls[0].body; + out.markerAfterControl = readPending(SID); + // Next real send must still carry the marker and consume it on kickoff. + _nextResponse = () => ({ stream_id: 's2', pending_started_at: 1 }); + await cmdGoal('ship it'); + out.kickoffPayload = _apiCalls[1].body; + out.markerAfterKickoff = readPending(SID); + } else if (scenario === 'midflight_newer_marker_kept') { + // A newer dropdown selection is recorded WHILE the request is in flight. + rememberPending(SID, 'openai/gpt-5.4', 'openai'); + window._defaultModel = 'gpt-4o'; window._activeProvider = 'openai'; + _nextResponse = () => { + rememberPending(SID, 'openai/gpt-6', 'openai'); + return { stream_id: 's3', pending_started_at: 1 }; + }; + await cmdGoal('ship it'); + out.payload = _apiCalls[0].body; + out.markerAfter = readPending(SID); + } else if (scenario === 'no_marker_no_pick') { + // Untouched default session: no pending marker, no cross-provider pick. + S.session.model = 'gpt-4o'; S.session.model_provider = 'openai'; + window._defaultModel = 'gpt-4o'; window._activeProvider = 'openai'; + _nextResponse = () => ({ stream_id: 's4', pending_started_at: 1 }); + await cmdGoal('ship it'); + out.payload = _apiCalls[0].body; + out.markerAfter = readPending(SID); + } else { + throw new Error('unknown scenario: ' + scenario); + } + process.stdout.write(JSON.stringify(out)); +})().catch(e => { + process.stderr.write(String((e && e.stack) || e)); + process.exit(1); +}); +""" + + +@pytest.fixture(scope="module") +def driver_path(tmp_path_factory): + """Write the node driver to a tmp file (works around `node -e` arg quirks).""" + p = tmp_path_factory.mktemp("goal_driver") / "driver.js" + p.write_text(_DRIVER_SRC, encoding="utf-8") + return str(p) + + +def _run_scenario(driver_path, scenario): + """Run cmdGoal against the real commands.js with mocked browser state.""" + result = subprocess.run( + [NODE, driver_path, str(COMMANDS_JS_PATH), scenario], + capture_output=True, + text=True, + timeout=30, + ) + if result.returncode != 0: + raise RuntimeError(f"node driver failed for {scenario}: {result.stderr}") + return json.loads(result.stdout) + + +def test_goal_kickoff_consumes_pending_marker_after_success(driver_path): + """#6703/#6705: a real kickoff carries explicit_model_pick and consumes the + one-shot pending marker AFTER the successful response (r.stream_id).""" + out = _run_scenario(driver_path, "kickoff_consumes") + assert out["payload"].get("explicit_model_pick") is True + assert out["markerAfter"] is None + + +def test_goal_control_command_keeps_marker_and_next_send_still_picks(driver_path): + """#6705: a control-only /goal (e.g. `/goal status`, no stream_id) must NOT + consume the pending explicit-pick marker; the next real send still carries + explicit_model_pick and only then consumes the marker.""" + out = _run_scenario(driver_path, "control_then_kickoff") + # Control round-trip: marker read for the payload but left intact. + assert out["controlPayload"].get("explicit_model_pick") is True + assert out["markerAfterControl"] is not None + assert out["markerAfterControl"]["model"] == "openai/gpt-5.4" + # Next real send: still carries the pick, then consumes the marker. + assert out["kickoffPayload"].get("explicit_model_pick") is True + assert out["markerAfterKickoff"] is None + + +def test_goal_kickoff_keeps_newer_midflight_marker(driver_path): + """#6705: a marker re-recorded while the request is in flight (newer + dropdown selection) must not be clobbered by the stale consume-clear.""" + out = _run_scenario(driver_path, "midflight_newer_marker_kept") + assert out["payload"].get("explicit_model_pick") is True + assert out["markerAfter"] is not None + assert out["markerAfter"]["model"] == "openai/gpt-6" + + +def test_goal_kickoff_without_marker_sends_no_explicit_pick(driver_path): + """#6703: untouched default sessions (no pending marker, no cross-provider + pick) must not send the marker at all.""" + out = _run_scenario(driver_path, "no_marker_no_pick") + assert "explicit_model_pick" not in out["payload"] + assert out["markerAfter"] is None diff --git a/tests/test_goal_command_webui.py b/tests/test_goal_command_webui.py index 32283c88..57033f28 100644 --- a/tests/test_goal_command_webui.py +++ b/tests/test_goal_command_webui.py @@ -2,6 +2,7 @@ import io import json +import re import threading from pathlib import Path from types import SimpleNamespace @@ -673,3 +674,384 @@ def test_frontend_goal_evaluating_state_uses_calm_composer_indicator(): assert "if(goalState==='evaluating')" in MESSAGES_JS assert "setComposerStatus(goalEvaluatingMessage);" in MESSAGES_JS assert "return;" in MESSAGES_JS + + +def test_goal_kickoff_forwards_explicit_model_pick_to_resolver(monkeypatch, tmp_path): + """#6703: /api/goal must carry explicit_model_pick to the model resolver. + + Without it a persisted cross-provider pick (e.g. provider1:model1 chosen + for the session) is treated as stale by _resolve_compatible_session_model_state + and "repaired" back to the profile default mid-session, switching the + provider while /goal runs. + """ + from api import goals as webui_goals + from api import routes + + class FakeState: + goal = "ship the feature" + status = "active" + turns_used = 0 + max_turns = 20 + last_verdict = None + last_reason = None + paused_reason = None + + class FakeGoalManager: + def __init__(self, session_id, default_max_turns=20): + self.session_id = session_id + self.default_max_turns = default_max_turns + + def set(self, goal): + state = FakeState() + state.goal = goal + return state + + class FakeSession: + session_id = "sid-goal-explicit" + profile = "default" + workspace = str(tmp_path) + model = "deepseek/deepseek-v4-flash" + model_provider = "deepseek" + messages = [] + context_messages = [] + pending_user_message = None + active_stream_id = None + + resolver_kwargs = {} + + def fake_resolve(model, provider, **kwargs): + resolver_kwargs.update(kwargs) + return model, provider, False + + monkeypatch.setattr(webui_goals, "GoalManager", FakeGoalManager) + monkeypatch.setattr(routes, "get_session", lambda sid: FakeSession()) + monkeypatch.setattr(routes, "resolve_trusted_workspace", lambda workspace: tmp_path) + monkeypatch.setattr(routes, "webui_gateway_chat_enabled", lambda _cfg: False) + monkeypatch.setattr(routes, "get_config", lambda: {}) + monkeypatch.setattr(routes, "_resolve_compatible_session_model_state", fake_resolve) + + started = [] + + def fake_start(session, **kwargs): + started.append(kwargs) + return {"stream_id": "goal-stream", "session_id": session.session_id, "pending_started_at": 123.0} + + monkeypatch.setattr(routes, "_start_chat_stream_for_session", fake_start) + monkeypatch.setattr(routes, "j", lambda handler, payload, status=200, **kwargs: {"status": status, "payload": payload}) + + result = routes._handle_goal_command( + object(), + { + "session_id": "sid-goal-explicit", + "args": "ship the feature", + "workspace": str(tmp_path), + "model": "deepseek/deepseek-v4-flash", + "model_provider": "deepseek", + "explicit_model_pick": True, + }, + ) + + assert result["status"] == 200 + assert started and started[0]["model"] == "deepseek/deepseek-v4-flash" + assert started[0]["model_provider"] == "deepseek" + # The explicit-pick marker must reach the resolver so the persisted + # cross-provider model is honored instead of reverted to the default. + assert resolver_kwargs.get("explicit_model_pick") is True + + +def test_goal_kickoff_defaults_explicit_model_pick_false(monkeypatch, tmp_path): + """#6703: without the marker the resolver receives explicit_model_pick=False.""" + from api import goals as webui_goals + from api import routes + + class FakeState: + goal = "ship the feature" + status = "active" + turns_used = 0 + max_turns = 20 + last_verdict = None + last_reason = None + paused_reason = None + + class FakeGoalManager: + def __init__(self, session_id, default_max_turns=20): + self.session_id = session_id + self.default_max_turns = default_max_turns + + def set(self, goal): + state = FakeState() + state.goal = goal + return state + + class FakeSession: + session_id = "sid-goal-default" + profile = "default" + workspace = str(tmp_path) + model = "gpt-5.5" + model_provider = "openai-codex" + messages = [] + context_messages = [] + pending_user_message = None + active_stream_id = None + + resolver_kwargs = {} + + def fake_resolve(model, provider, **kwargs): + resolver_kwargs.update(kwargs) + return model, provider, False + + monkeypatch.setattr(webui_goals, "GoalManager", FakeGoalManager) + monkeypatch.setattr(routes, "get_session", lambda sid: FakeSession()) + monkeypatch.setattr(routes, "resolve_trusted_workspace", lambda workspace: tmp_path) + monkeypatch.setattr(routes, "webui_gateway_chat_enabled", lambda _cfg: False) + monkeypatch.setattr(routes, "get_config", lambda: {}) + monkeypatch.setattr(routes, "_resolve_compatible_session_model_state", fake_resolve) + + started = [] + + def fake_start(session, **kwargs): + started.append(kwargs) + return {"stream_id": "goal-stream", "session_id": session.session_id, "pending_started_at": 123.0} + + monkeypatch.setattr(routes, "_start_chat_stream_for_session", fake_start) + monkeypatch.setattr(routes, "j", lambda handler, payload, status=200, **kwargs: {"status": status, "payload": payload}) + + result = routes._handle_goal_command( + object(), + { + "session_id": "sid-goal-default", + "args": "ship the feature", + "workspace": str(tmp_path), + "model": "gpt-5.5", + "model_provider": "openai-codex", + }, + ) + + assert result["status"] == 200 + assert resolver_kwargs.get("explicit_model_pick") is False + + +def test_frontend_goal_sends_explicit_model_pick(): + """#6703: cmdGoal must include explicit_model_pick in the /api/goal payload. + + Mirrors the chat/start marker (messages.js) so the server honors a + persisted cross-provider session pick instead of reverting to the default. + """ + goal_idx = COMMANDS_JS.find("function cmdGoal") + assert goal_idx != -1 + goal_fn = COMMANDS_JS[goal_idx : COMMANDS_JS.find("\n}", goal_idx)] + assert "explicit_model_pick:_explicitPick" in goal_fn + assert "api('/api/goal'" in goal_fn + assert "_isCrossProviderPick" in goal_fn + assert "_readPendingSessionModel" in goal_fn + + +def test_goal_kickoff_stamps_explicit_pick_signature(monkeypatch, tmp_path): + """#6703: /api/goal must stamp model_explicit_pick_signature on explicit picks. + + Parity with /api/chat/start (#5979): the streaming resolver preserves a + custom-proxy vendor namespace on a cold catalog only when the persisted + signature matches the current routing context. Without the stamp, a first + /goal launch after a deliberate custom-provider pick loses the vendor + namespace and the provider reverts to the profile default mid-session. + """ + from api import goals as webui_goals + from api import routes + + class FakeState: + goal = "ship the feature" + status = "active" + turns_used = 0 + max_turns = 20 + last_verdict = None + last_reason = None + paused_reason = None + + class FakeGoalManager: + def __init__(self, session_id, default_max_turns=20): + self.session_id = session_id + self.default_max_turns = default_max_turns + + def set(self, goal): + state = FakeState() + state.goal = goal + return state + + class FakeSession: + session_id = "sid-goal-sig" + profile = "default" + workspace = str(tmp_path) + model = "deepseek/deepseek-v4-flash" + model_provider = "deepseek" + messages = [] + context_messages = [] + pending_user_message = None + active_stream_id = None + model_explicit_pick_signature = None + + def fake_resolve(model, provider, **kwargs): + return model, provider, False + + monkeypatch.setattr(webui_goals, "GoalManager", FakeGoalManager) + monkeypatch.setattr(routes, "get_session", lambda sid: FakeSession()) + monkeypatch.setattr(routes, "resolve_trusted_workspace", lambda workspace: tmp_path) + monkeypatch.setattr(routes, "webui_gateway_chat_enabled", lambda _cfg: False) + monkeypatch.setattr(routes, "get_config", lambda: {}) + monkeypatch.setattr(routes, "_resolve_compatible_session_model_state", fake_resolve) + + def fake_start(session, **kwargs): + return {"stream_id": "goal-stream", "session_id": session.session_id, "pending_started_at": 123.0} + + monkeypatch.setattr(routes, "_start_chat_stream_for_session", fake_start) + monkeypatch.setattr(routes, "j", lambda handler, payload, status=200, **kwargs: {"status": status, "payload": payload}) + + session = FakeSession() + monkeypatch.setattr(routes, "get_session", lambda sid: session) + + result = routes._handle_goal_command( + object(), + { + "session_id": "sid-goal-sig", + "args": "ship the feature", + "workspace": str(tmp_path), + "model": "deepseek/deepseek-v4-flash", + "model_provider": "deepseek", + "explicit_model_pick": True, + }, + ) + + assert result["status"] == 200 + from api.models import model_explicit_pick_signature as _mk_sig + + assert session.model_explicit_pick_signature == _mk_sig("deepseek/deepseek-v4-flash", "deepseek") + + +def test_goal_kickoff_does_not_stamp_signature_without_explicit_pick(monkeypatch, tmp_path): + """#6703: without the marker no signature is stamped (parity with chat/start).""" + from api import goals as webui_goals + from api import routes + + class FakeState: + goal = "ship the feature" + status = "active" + turns_used = 0 + max_turns = 20 + last_verdict = None + last_reason = None + paused_reason = None + + class FakeGoalManager: + def __init__(self, session_id, default_max_turns=20): + self.session_id = session_id + self.default_max_turns = default_max_turns + + def set(self, goal): + state = FakeState() + state.goal = goal + return state + + class FakeSession: + session_id = "sid-goal-nosig" + profile = "default" + workspace = str(tmp_path) + model = "deepseek/deepseek-v4-flash" + model_provider = "deepseek" + messages = [] + context_messages = [] + pending_user_message = None + active_stream_id = None + model_explicit_pick_signature = "stale-previous-sig" + + def fake_resolve(model, provider, **kwargs): + return model, provider, False + + monkeypatch.setattr(webui_goals, "GoalManager", FakeGoalManager) + monkeypatch.setattr(routes, "resolve_trusted_workspace", lambda workspace: tmp_path) + monkeypatch.setattr(routes, "webui_gateway_chat_enabled", lambda _cfg: False) + monkeypatch.setattr(routes, "get_config", lambda: {}) + monkeypatch.setattr(routes, "_resolve_compatible_session_model_state", fake_resolve) + + def fake_start(session, **kwargs): + return {"stream_id": "goal-stream", "session_id": session.session_id, "pending_started_at": 123.0} + + monkeypatch.setattr(routes, "_start_chat_stream_for_session", fake_start) + monkeypatch.setattr(routes, "j", lambda handler, payload, status=200, **kwargs: {"status": status, "payload": payload}) + + session = FakeSession() + monkeypatch.setattr(routes, "get_session", lambda sid: session) + + result = routes._handle_goal_command( + object(), + { + "session_id": "sid-goal-nosig", + "args": "ship the feature", + "workspace": str(tmp_path), + "model": "deepseek/deepseek-v4-flash", + "model_provider": "deepseek", + }, + ) + + assert result["status"] == 200 + # A non-explicit kickoff leaves any prior signature untouched (chat-start parity). + assert session.model_explicit_pick_signature == "stale-previous-sig" + + +def test_frontend_goal_consumes_pending_model_marker(): + """#6703/#6705: cmdGoal must consume the one-shot pending session-model marker. + + Mirrors the chat/start path (messages.js) so a later /goal with an unchanged + dropdown isn't treated as an explicit pick. The consume-clear is deferred to + AFTER a successful kickoff (r.stream_id) and gated on a re-read of the stored + marker that still matches the captured model/provider (#6705), so a control + command (e.g. /goal status, which never produces a stream_id) leaves the + marker intact for the next real send. + """ + goal_idx = COMMANDS_JS.find("function cmdGoal") + assert goal_idx != -1 + goal_fn = COMMANDS_JS[goal_idx : COMMANDS_JS.find("\n}", goal_idx)] + assert "_clearPendingSessionModel(activeSid)" in goal_fn + assert "_pendingPickMatch && typeof _readPendingSessionModel==='function' && typeof _clearPendingSessionModel==='function'" in goal_fn + # The consume-clear must come AFTER the kickoff guard: a control command + # (no stream_id) returns before reaching it. + assert goal_fn.index("_clearPendingSessionModel(activeSid)") > goal_fn.index("if(!r||!r.stream_id)return;") + # Re-check: the marker is re-read and only cleared while it still matches + # the captured model/provider. + assert "_stillPending.model===_goalModel" in goal_fn + assert "_stillPending.model_provider" in goal_fn + + +def test_frontend_goal_control_command_keeps_pending_marker(): + """#6705: a control-only /goal (e.g. `/goal status`) must NOT consume the + pending explicit-pick marker. + + The server skips model resolution for control commands (api/routes.py + will_kickoff excludes status/pause/resume/clear/stop/done), so clearing the + marker before the request would drop the explicit pick without using it and + the next real send would revert to the default model. The marker must + survive the control round-trip and still be read for the next kickoff. + """ + goal_idx = COMMANDS_JS.find("function cmdGoal") + assert goal_idx != -1 + goal_fn = COMMANDS_JS[goal_idx : COMMANDS_JS.find("\n}", goal_idx)] + + # 1) No clear may run before the /api/goal request: the marker must + # survive a control command. + pre_request = goal_fn.split("const r=await api('/api/goal'")[0] + assert "_clearPendingSessionModel" not in pre_request, ( + "the pending explicit-pick marker must NOT be cleared before /api/goal — " + "a control command like /goal status never kicks off, so a pre-request " + "clear would drop the pick without using it (#6705)" + ) + + # 2) The consume-clear sits after the kickoff guard (r.stream_id present). + post_request = goal_fn.split("const r=await api('/api/goal'")[1] + assert "if(!r||!r.stream_id)return;" in post_request + assert "_clearPendingSessionModel(activeSid)" in post_request + assert post_request.index("if(!r||!r.stream_id)return;") < post_request.index("_clearPendingSessionModel(activeSid)") + + # 3) The marker is re-read and only cleared while it still matches the + # captured model/provider. + assert "_stillPending=_readPendingSessionModel(activeSid)" in post_request + assert re.search(r"_stillPending\.model\s*===\s*_goalModel", post_request) + assert re.search(r"_stillPending\.model_provider", post_request) + assert re.search(r"_clearPendingSessionModel\(activeSid\)", post_request)