* fix(session): sync provider change mid-session (#6703) * fix(session): full chat-start parity for /goal explicit-pick (#6703) Addresses review parity details on #6703: - stamp model_explicit_pick_signature after explicit goal resolution (both kickoff paths), matching /api/chat/start #5979, so a first /goal launch after a deliberate custom-provider pick survives a cold streaming catalog instead of reverting to the profile default - consume the one-shot pending session-model marker in cmdGoal, matching the chat/start path (messages.js), so a later /goal with an unchanged dropdown isn't treated as an explicit pick * fix(session): consume pending model-pick marker only after successful kickoff (#6705) * fix(session): node-driven behavioral tests for /goal explicit-pick marker (#6703) * changelog: #6705 — carry explicit_model_pick through /goal * ci: re-trigger tests on fresh runners (prior run's shards hung on GitHub-hosted runners) * ci: re-trigger (playwright browser-install hung on GH runners again; warm cache should restore) --------- Co-authored-by: webtecnica <webtecnica@gmail.com> Co-authored-by: n <a@n> Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
This commit is contained in:
5 files changed
+714
-2
No files matched your search
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
+42
-2
@@ -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);
|
||||
|
||||
@@ -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
|
||||
@@ -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)
|
||||
Reference in new issue
Block a user