From e1f90aed09e3321dd3947aa1b6fabeeebf00dba1 Mon Sep 17 00:00:00 2001 From: Cushing Whitney Date: Wed, 23 Sep 2026 14:10:22 +0000 Subject: [PATCH 1/2] fix(cron): preserve provider-only pin on untouched save; honor explicit clear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Editing a cron job with the model picker at "Default" sent provider: null unconditionally, wiping provider-only pins (provider set, no model — the pattern for self-hosted LLM endpoints). The combined picker cannot express that state, so such jobs always rendered as "Default" and were silently re-resolved to the bare global default on any save, breaking dispatch. #4030 deliberately made clearing the picker mean "no overrides"; that contract is preserved for explicit clears and model+provider jobs, while provider-only pins survive saves where the picker was never touched. - _cronProviderForClear(prevDetail, pickerTouched) decides the provider submitted on clear: touched picker -> null; untouched + no model on the job -> prev.provider. - _cronModelPickerTouched is set by a change listener added when the picker populates, and reset at openCronEdit/openCronCreate/duplicateCurrentCron. - Tests: delegation + flag lifecycle locked in, and the helper driven through node for provider-only / model-pinned / no-override / no-snapshot inputs (6 passed). Refs: #4030 --- static/panels.js | 24 ++++++++- tests/test_cron_model_override.py | 88 +++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/static/panels.js b/static/panels.js index 9215cc77..4f708aae 100644 --- a/static/panels.js +++ b/static/panels.js @@ -25,6 +25,7 @@ let _currentCronDetail = null; // full cron job object let _currentCronDetailKey = ''; let _cronMode = 'empty'; // 'empty' | 'read' | 'create' | 'edit' let _cronPreFormDetail = null; // snapshot of prior selection when entering a form +let _cronModelPickerTouched = false; // true once the user changes the model picker in the current form let _showAllCronProfiles = false; let _cronOtherProfileCount = 0; let _currentWorkspaceDetail = null; // { path, name, is_default } @@ -1550,6 +1551,7 @@ function duplicateCurrentCron(){ const job = _currentCronDetail; if (typeof switchPanel === 'function' && _currentPanel !== 'tasks') switchPanel('tasks'); _cronPreFormDetail = { ...job }; + _cronModelPickerTouched = false; _editingCronId = null; _cronMode = 'create'; _cronIsDuplicate = true; @@ -1606,6 +1608,7 @@ let _cronDeliveryOptionsCache=null; function openCronCreate(){ if (typeof switchPanel === 'function' && _currentPanel !== 'tasks') switchPanel('tasks'); _cronPreFormDetail = _currentCronDetail ? { ..._currentCronDetail } : null; + _cronModelPickerTouched = false; _editingCronId = null; _cronMode = 'create'; _cronIsDuplicate = false; @@ -1623,6 +1626,7 @@ function openCronCreate(){ function openCronEdit(job){ if (!job) return; _cronPreFormDetail = { ...job }; + _cronModelPickerTouched = false; _editingCronId = job.id; _cronMode = 'edit'; _cronSelectedSkills = Array.isArray(job.skills) ? [...job.skills] : []; @@ -1855,6 +1859,9 @@ async function _populateCronFormModelSelect(selectedModel, selectedProvider, dis sel.appendChild(opt); } sel.dataset.loaded = '1'; + // Track deliberate picker changes so an untouched "Default" can keep a + // hidden provider-only pin on save (see _cronProviderForClear). + sel.addEventListener('change', () => { _cronModelPickerTouched = true; }); } catch (e) { console.warn('Failed to load cron model picker:', e.message); // Load failed: dataset.loaded stays unset so saveCronForm omits model/provider @@ -1940,6 +1947,18 @@ function _cronModelBareName(model, provider) { return _modelBareNameForProvider(model, provider); } +function _cronProviderForClear(prevDetail, pickerTouched) { + // Provider to submit when the picker shows "Default" (no model) on save. + // An explicit return to Default honors "Default = no overrides" (#4030) and + // clears the provider. An untouched picker must not: provider-only jobs + // (a pinned provider with no model) render as "Default" in the combined + // picker, so saving any other field would silently erase the pin the user + // never touched. + const prev = prevDetail || {}; + if (pickerTouched) return null; + return (prev.model == null) ? prev.provider : null; +} + async function saveCronForm(){ const nameEl=$('cronFormName'); const schEl=$('cronFormSchedule'); @@ -1977,8 +1996,11 @@ async function saveCronForm(){ updates.model = _cronModelBareName(modelState.model, modelState.model_provider) || null; updates.provider = modelState.model_provider || null; } else if (modelLoaded) { + // "Default" selected (no model). An untouched picker preserves a + // hidden provider-only pin; a deliberate return to Default clears + // it (#4030 "Default = no overrides"). updates.model = null; - updates.provider = null; + updates.provider = _cronProviderForClear(_cronPreFormDetail, _cronModelPickerTouched); } // else: select not yet populated — omit model/provider to preserve saved value } diff --git a/tests/test_cron_model_override.py b/tests/test_cron_model_override.py index bf3b4b89..6c9501c8 100644 --- a/tests/test_cron_model_override.py +++ b/tests/test_cron_model_override.py @@ -8,6 +8,8 @@ import sys import types from pathlib import Path +import pytest + REPO = Path(__file__).resolve().parents[1] PANELS_JS = (REPO / "static" / "panels.js").read_text(encoding="utf-8") @@ -170,3 +172,89 @@ def test_cron_model_picker_marks_loaded_only_after_successful_population(): catch_block = body.split("} catch (e)", 1)[1] assert "sel.dataset.loaded = '1'" in try_block assert "sel.dataset.loaded = '1'" not in catch_block + + +def test_cron_update_preserves_provider_only_pin_when_model_cleared(): + """Clearing the picker to "Default" must not wipe a provider-only pin. + + #4030 intentionally made "clearing the picker = no overrides", which is + correct for model+provider pins but silently destroyed provider-only + pins (jobs that pin a provider with no model, e.g. self-hosted LLM + endpoints). The combined picker cannot represent that state, so + saveCronForm must preserve prev.provider when the job carried no model + AND the user never changed the picker; a deliberate return to Default + must clear it. + """ + save_body = _function_body("saveCronForm") + helper_body = _function_body("_cronProviderForClear") + + # The clear branch delegates to the helper and records picker changes. + assert "_cronProviderForClear(_cronPreFormDetail, _cronModelPickerTouched)" in save_body + assert "preserveProvider" not in save_body + + # The helper distinguishes an explicit clear (pickerTouched) from an + # untouched provider-only pin. + assert "if (pickerTouched) return null" in helper_body + assert "prev.model == null" in helper_body + + # The form entry points reset the touched flag. + for fn in ("openCronCreate", "openCronEdit", "duplicateCurrentCron"): + assert "_cronModelPickerTouched = false" in _function_body(fn) + # The picker records deliberate changes. + pop_body = _function_body("_populateCronFormModelSelect") + assert "_cronModelPickerTouched = true" in pop_body + + +def test_cron_provider_for_clear_behavior(): + """Exercise _cronProviderForClear through node for both pin shapes.""" + import shutil + import subprocess + + node = shutil.which("node") + if node is None: + pytest.skip("node not on PATH") + + # Locate the full declaration (signature + body) of the helper. + src = PANELS_JS + start = src.find("function _cronProviderForClear(") + assert start != -1, "helper declaration not found" + brace = src.find("{", start) + depth = 0 + end = None + for i in range(brace, len(src)): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + if depth == 0: + end = i + 1 + break + decl = src[start:end] + + driver = f""" +{decl} +const cases = [ + ['provider_only_untouched', {{model: null, provider: 'custom:llama-swap'}}, false], + ['provider_only_touched', {{model: null, provider: 'custom:llama-swap'}}, true], + ['model_pin_touched', {{model: 'gpt-5', provider: 'openai'}}, true], + ['no_override_untouched', {{model: null, provider: null}}, false], + ['no_prev', null, false], +]; +const out = {{}}; +for (const [name, detail, touched] of cases) out[name] = _cronProviderForClear(detail, touched) ?? null; +process.stdout.write(JSON.stringify(out)); +""" + proc = subprocess.run([node, "-e", driver], capture_output=True, text=True, timeout=30) + assert proc.returncode == 0, f"node driver failed: {proc.stderr}" + r = json.loads(proc.stdout) + + # Untouched picker on a provider-only job: the pin survives. + assert r["provider_only_untouched"] == "custom:llama-swap" + # Deliberate return to Default: "Default = no overrides" wins. + assert r["provider_only_touched"] is None + # Model-pinned job, explicit clear: provider removed too. + assert r["model_pin_touched"] is None + # No overrides stored, untouched picker: nothing to preserve. + assert r["no_override_untouched"] is None + # No pre-form snapshot (create form): no provider. + assert r["no_prev"] is None From 9dfd9d4e9f67e072e58edca66a17606e8a60825b Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 23 Sep 2026 18:36:16 +0000 Subject: [PATCH 2/2] docs(changelog): #7779 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41bb356e..1eab9dfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,11 @@ ### Fixed +- **Saving a cron job with the model picker at "Default" keeps its provider-only pin.** Editing a job + pinned to a provider with no model (the usual shape for a self-hosted OpenAI-compatible router) + sent `provider: null` and wiped the pin. The editor now preserves a provider-only pin when the + model picker is left at Default. (#7779 by @cushingw) + - **The live stream reports which model actually served the turn.** A new additive `runtime_model` SSE event carries the model (and provider, when known) that the Agent reported while producing output, separately from the model that was requested. It is journaled for replay, is exposed as