Merge pull request #7794 from nesquena/stage/7779-cron-provider-pin

Release: #7779 cron keeps its provider-only pin when the model picker is Default (@cushingw)

CHANGELOG.md

@@ -28,6 +28,10 @@
 
 ### 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)
 - **A consumed mid-turn `/steer` no longer leaves its out-of-band wrapper in the settled chat (#7600).**
   A `/steer` reaches the agent as an `[OUT-OF-BAND USER MESSAGE …]` block appended to the turn's
   last tool result. After the turn settled, that wrapper stayed visible in the chat transcript. The

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
       }

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