5 Commits

Author SHA1 Message Date
Kenny Van de Maele de12d4734a fix(ui): route tasks.js + skills.js dropdowns through topPortalZ() (#4768)
Fixes #4767. #4724 routed 16 body-portaled dropdowns through the shared
topPortalZ() helper so they always render just above the currently-raised tool
modal, but two were missed and still used a hardcoded z-index, so they hit the
same #4720 bug once a modal's bring-to-front counter climbed past the literal:

  - tasks.js _showTaskDropdown(): inline z-index:100000 on .task-dropdown
  - skills.js kebab menu (.skill-kebab-menu): z-index:100002 in style.css

Both now set zIndex from topPortalZ() after they are appended to the body,
matching the other migrated sites. The dead CSS z-index on .skill-kebab-menu is
removed (the inline value always wins). test_portal_dropdown_z_js.py gains a
source guard asserting both files use topPortalZ() and that no hardcoded
100000/100002 portal literal survives in either file or style.css.
2026-06-24 22:29:36 +02:00
Samy 5d23495eb2 fix(cookbook): only block model launch on real port collisions (#4760)
* Fix #4507: only block model launch on real port collisions

Quick-run hardcoded port 8000 and never called _nextAvailablePort(), so
every launch collided. Both pre-launch guards (serve panel + quick-run)
were count-based and fired regardless of port.

- quick-run now auto-assigns a free port (8080 for llama.cpp)
- both guards parse the new port and only prompt on a real overlap,
  stopping only the colliding serve
- dialog reports the actual port instead of a hardcoded 8000

* refactor(cookbook): share _taskPort for port parsing; auto-assign llama.cpp port

Addresses review on #4760:
- _taskPort regex now matches --port= as well as --port (space)
- _nextAvailablePort and both launch guards reuse _taskPort instead of inline regex
- quick-run llama.cpp no longer pins 8080, so two can run concurrently

* fix(cookbook): _taskPort also parses -p; add port-parsing tests

Addresses review on #4760:
- _taskPort now matches -p <n> too, so it's the complete single reader
  (was missing the short flag that other readers already handle)
- add tests/test_cookbook_port_parsing_js.py covering the port forms,
  shared-reader reuse, and llama.cpp auto-assign

* test(cookbook): extract pure port helpers and test behavior

Addresses review on #4760: the prior tests only asserted source strings.
- extract portOf() and nextFreePort() into static/js/cookbookPorts.js
- cookbookRunning.js imports them; _taskPort and _nextAvailablePort delegate
- tests run the helpers via node and assert real behavior: all port forms
  (--port, --port=, -p, -p=), next-free-port skipping taken ports, and the
  same-port-clash / different-port-coexist outcome

---------

Co-authored-by: samy <samy@odysseus.boukouro.com>
2026-06-24 19:44:09 +02:00
Solanki Sumit 22379fe736 fix(model-routes): harden _probe_endpoint against malformed model-list responses (#4789)
* fix(model-routes): harden _probe_endpoint against malformed model-list responses

_probe_endpoint parsed model lists with data.get(...) at four sites without
checking that data is a dict, and built the list with a truthiness-only
filter. A /models (or /api/tags) endpoint returning HTTP 200 with valid but
non-dict JSON ([], "x", null, 123) made data.get(...) raise AttributeError,
and a non-string id like 123 passed the filter and then hit .startswith() /
.lower() in the Z.AI/Kimi curated merge and _is_chat_model(). Both errors are
swallowed by the broad except Exception, but the comprehension dies mid-list
so the ENTIRE probed model list is discarded and the endpoint silently
degrades — masking a misconfigured/non-compliant upstream as "no models".

- Guard each data.get(...) with isinstance(data, dict) so a non-dict body
  falls through the existing `or []` default.
- Restrict the OpenAI and Ollama model-list comprehensions to non-empty str
  values, protecting the .startswith() merges and both _is_chat_model calls.
- Add an isinstance guard at the top of _is_chat_model (defense in depth for
  all four call sites).

No behavior change for well-formed {"data":[...]} / {"models":[...]}
responses. Adds regression tests (non-dict body via caplog, mixed/all
non-string ids, _is_chat_model boundary) that fail before the fix and pass
after.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(model-routes): extract _openai_model_ids / _ollama_model_names helpers

Per review on #4789: the malformed-response guards were inlined four times in
_probe_endpoint (two OpenAI-id comprehensions, two Ollama-name comprehensions).
Pull each into a small, directly-testable helper so the security-relevant
parsing lives in one place and a future malformed-shape fix doesn't have to be
applied in four spots (CONTRIBUTING flags repeated logic for this reason).

Behavior is unchanged. Adds direct unit tests for both helpers (non-dict body,
non-string ids, non-dict entries, name>model precedence).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-24 19:05:31 +02:00
Magiomakes 4e46e415ea fix(tasks): normalize task endpoint URL to /chat/completions before model call (#4619)
Upstream bug (present in pewdiepie-archdaemon/odysseus main): the task
executor passes task.endpoint_url VERBATIM to the model HTTP call, unlike
the chat path which stores build_chat_url(normalize_base(base)) on the
session. A task carrying an explicit bare OpenAI-compatible base such as
"http://host:11434/v1" therefore POSTs to a 404 ("page not found"); the
agent loop swallows the empty body into "The model returned an empty
response" and marks the run success, so nothing surfaces the failure.

Tasks that omit an endpoint dodge this only because _resolve_defaults()
cribs an already-full URL from a recent chat session. The API/token path
(e.g. an external client that POSTs /api/tasks with endpoint_url=".../v1")
hits it every time.

Fix: route every resolved task endpoint through _normalize_chat_endpoint()
at the three resolution sites (_execute_llm_task, the persona/research
session path, and _execute_research_task). The helper is idempotent
(strips any existing chat suffix, re-appends the correct one) and leaves
native-Ollama (/api...) and already-concrete URLs untouched, so other
providers are unaffected. Proven via isolated repro: ".../v1" -> 404 ->
empty; ".../v1/chat/completions" -> 200 -> real gemma4:31b output.

Regression test asserts the bare-/v1 -> full-chat-URL mapping, idempotency,
and the native-Ollama/empty passthroughs.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-24 18:02:31 +02:00
Solanki Sumit 6a2a39f892 refactor(exceptions): dedupe src/exceptions via core re-export (#4785)
src/exceptions.py was a byte-for-byte duplicate of the canonical
core/exceptions.py. Replace its class bodies with a re-export shim
(mirroring the core/constants.py -> src/constants.py pattern) so the
exception classes are defined in exactly one place. Also fix the stale
"# src/exceptions.py" header comment in core/exceptions.py.

No behavior change: both import paths resolve to the same class objects
(verified by identity), so `except SessionNotFoundError` works regardless
of which module it was imported from. Ran py_compile and
pytest tests/test_app.py (12 passed).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-24 16:50:07 +02:00
16 changed files with 374 additions and 89 deletions
+1 -1
View File
@@ -1,4 +1,4 @@
# src/exceptions.py
# core/exceptions.py
"""Custom exceptions for the application."""
class SessionNotFoundError(Exception):
+36 -4
View File
@@ -523,6 +523,10 @@ _NON_CHAT_EXACT_PREFIXES = (
def _is_chat_model(model_id: str) -> bool:
"""Return True if the model ID looks like a chat/completions-capable model."""
if not isinstance(model_id, str):
# Non-compliant upstreams can return non-string IDs (e.g. int/None);
# treat them as chat-capable rather than crashing on .lower().
return True
mid = model_id.lower()
for prefix in _NON_CHAT_PREFIXES:
if mid.startswith(prefix):
@@ -726,6 +730,34 @@ def _is_loading_model_response(resp: Any) -> bool:
def _openai_model_ids(data: Any) -> List[str]:
"""Extract OpenAI-style model IDs (``{"data": [{"id": ...}]}``).
Tolerates a non-dict body and non-string IDs from non-compliant upstreams,
returning only non-empty string IDs.
"""
items = data.get("data") if isinstance(data, dict) else None
return [m["id"] for m in (items or [])
if isinstance(m, dict) and isinstance(m.get("id"), str) and m["id"]]
def _ollama_model_names(data: Any) -> List[str]:
"""Extract native-Ollama model names (``{"models": [{"name"|"model": ...}]}``).
Same tolerance as :func:`_openai_model_ids`: a non-dict body or non-string
value is skipped rather than crashing, preserving name-then-model precedence.
"""
items = data.get("models") if isinstance(data, dict) else None
out: List[str] = []
for m in (items or []):
if not isinstance(m, dict):
continue
v = m.get("name") or m.get("model")
if isinstance(v, str) and v:
out.append(v)
return out
def _probe_endpoint(base_url: str, api_key: str = None, timeout: int = 5) -> List[str]:
"""Probe a base URL's /models endpoint and return list of model IDs.
For Anthropic, queries their /v1/models API, falling back to hardcoded list."""
@@ -748,7 +780,7 @@ def _probe_endpoint(base_url: str, api_key: str = None, timeout: int = 5) -> Lis
r = httpx.get(url, headers=headers, timeout=timeout, verify=llm_verify())
r.raise_for_status()
data = r.json()
models = [m.get("id") for m in (data.get("data") or []) if m.get("id")]
models = _openai_model_ids(data)
if models:
return models
except httpx.HTTPStatusError as e:
@@ -770,10 +802,10 @@ def _probe_endpoint(base_url: str, api_key: str = None, timeout: int = 5) -> Lis
r.raise_for_status()
data = r.json()
# OpenAI format: {"data": [{"id": "model-name"}]}
models = [m.get("id") for m in (data.get("data") or []) if m.get("id")]
models = _openai_model_ids(data)
# Ollama format: {"models": [{"name": "model-name"}]}
if not models:
models = [m.get("name") or m.get("model") for m in (data.get("models") or []) if m.get("name") or m.get("model")]
models = _ollama_model_names(data)
if models:
# Z.AI coding plan omits some working models from /models;
# append curated-only entries for that endpoint only.
@@ -812,7 +844,7 @@ def _probe_endpoint(base_url: str, api_key: str = None, timeout: int = 5) -> Lis
r = httpx.get(root + "/api/tags", timeout=timeout, verify=llm_verify())
r.raise_for_status()
data = r.json()
models = [m.get("name") or m.get("model") for m in (data.get("models") or []) if m.get("name") or m.get("model")]
models = _ollama_model_names(data)
if models:
return [m for m in models if _is_chat_model(m)]
except Exception as e:
+19 -26
View File
@@ -1,29 +1,22 @@
# src/exceptions.py
"""Custom exceptions for the application."""
"""Backward-compatible shim — the single source of truth is core/exceptions.py.
class SessionNotFoundError(Exception):
"""Raised when a requested session is not found."""
def __init__(self, session_id: str):
self.session_id = session_id
super().__init__(f"Session '{session_id}' not found")
Historically this module was a byte-for-byte duplicate of core/exceptions.py,
which is the canonical definition (imported by app.py, core/__init__.py, and
routes/chat_routes.py). To kill the drift, this now simply re-exports the
exception classes from core.exceptions so there is exactly one place that
defines them. Existing `from src.exceptions import ...` callers keep working.
"""
from core.exceptions import ( # noqa: F401
SessionNotFoundError,
InvalidFileUploadError,
LLMServiceError,
WebSearchError,
)
class InvalidFileUploadError(Exception):
"""Raised when a file upload fails validation."""
def __init__(self, message: str, filename: str = None):
self.filename = filename
self.message = message
super().__init__(message)
class LLMServiceError(Exception):
"""Raised when there is an error communicating with the LLM service."""
def __init__(self, message: str, endpoint: str = None):
self.endpoint = endpoint
self.message = message
super().__init__(message)
class WebSearchError(Exception):
"""Raised when there is an error with web search functionality."""
def __init__(self, message: str, query: str = None):
self.query = query
self.message = message
super().__init__(message)
__all__ = [
"SessionNotFoundError",
"InvalidFileUploadError",
"LLMServiceError",
"WebSearchError",
]
+40
View File
@@ -289,6 +289,42 @@ def _checkin_calendar_events(db, owner, start, end):
)
def _normalize_chat_endpoint(url: str) -> str:
"""Repair a resolved task endpoint to a full chat-completions URL.
Unlike the chat path — which stores ``build_chat_url(normalize_base(base))``
on the session — the task executor passes ``task.endpoint_url`` verbatim to
the model HTTP call. A bare OpenAI-compatible base such as
``http://host:11434/v1`` therefore POSTs to a 404 ("page not found") and the
model silently appears to "return an empty response".
Repair only bare OpenAI-compatible bases. Native-Ollama URLs (``/api...``)
and URLs that already point at a concrete endpoint are returned untouched, so
their own downstream normalizers keep working. Idempotent: a URL already
ending in ``/chat/completions`` is left as-is.
"""
if not url:
return url
# Imports kept function-local (endpoint_resolver pulls in heavy deps) but
# OUTSIDE the try: an import failure is a real bug that should surface, not
# be silently swallowed into the un-normalized URL this function exists to
# repair.
from urllib.parse import urlparse
from src.endpoint_resolver import normalize_base, build_chat_url
path = (urlparse(url).path or "").rstrip("/")
if path == "/api" or path.startswith("/api/"):
return url # native Ollama — handled by the native path downstream
if path.endswith(("/chat/completions", "/messages", "/responses", "/completions")):
return url # already a concrete endpoint
try:
return build_chat_url(normalize_base(url))
except Exception:
# Guard only the actual normalization. Returning the URL un-normalized
# reverts to the 404 this fixes, so make the silent revert visible.
logger.debug("task endpoint normalization failed for %r; using as-is", url, exc_info=True)
return url
class TaskScheduler:
def __init__(self, session_manager):
self._session_manager = session_manager
@@ -1357,6 +1393,7 @@ class TaskScheduler:
endpoint_url, model = self._resolve_defaults(db, task.owner)
if not endpoint_url or not model:
raise RuntimeError("No model/endpoint configured")
endpoint_url = _normalize_chat_endpoint(endpoint_url)
# Record the resolved model so _execute_task_locked can persist it on
# the run (tasks rarely pin a model, so this is the only record of
# which model actually produced the output).
@@ -1548,6 +1585,8 @@ class TaskScheduler:
except Exception:
pass
endpoint_url = _normalize_chat_endpoint(endpoint_url)
session_id = task.session_id
if not session_id:
session_id = str(uuid.uuid4())
@@ -1821,6 +1860,7 @@ class TaskScheduler:
endpoint_url, model = self._resolve_defaults(db, task.owner)
if not endpoint_url or not model:
raise RuntimeError("No model/endpoint configured for research")
endpoint_url = _normalize_chat_endpoint(endpoint_url)
# Record the resolved model for the run record (see _execute_task_locked).
self._last_run_model = model
+19 -22
View File
@@ -31,7 +31,7 @@ import {
} from './cookbook.js';
import uiModule from './ui.js';
import spinnerModule from './spinner.js';
import { _loadTasks, _tmuxGracefulKill } from './cookbookRunning.js';
import { _loadTasks, _tmuxGracefulKill, _nextAvailablePort, _taskPort } from './cookbookRunning.js';
import { openCookbookDependencies } from './cookbook-diagnosis.js';
// Map a serve-backend code (vllm / sglang / llamacpp) → the package name
@@ -1493,36 +1493,34 @@ export function _expandModelRow(row, modelData) {
}
return;
}
// Detect backend and port now — the pre-launch guard below needs them.
const _qrBackendDetect = _detectBackend(modelData);
const _qrRunBackend = _qrBackendDetect.backend || 'vllm';
const _qrPort = _nextAvailablePort();
// ─── Pre-launch: stop the model already serving on this host ───────
// Two servers can't share port 8000. Without this, the new launch
// silently collided and the user saw no feedback. We surface the
// conflict and offer to kill the running one first as the default
// action (it's almost always what the user wants).
// ─── Pre-launch: stop colliding serves on the same port ───────
// Different ports coexist fine (e.g. vLLM on 8000 + Qwen VL on
// 8001). Only block when the new model's port genuinely collides
// with a running serve. (Issue #4507)
try {
const _qrHostStr = _envState.remoteHost || '';
const _activeServes = _loadTasks().filter(t =>
const _allServes = _loadTasks().filter(t =>
t && t.type === 'serve'
&& (t.remoteHost || '') === _qrHostStr
&& (t.status === 'running' || t.status === 'ready' || t._serveReady)
);
if (_activeServes.length) {
const _names = _activeServes.map(t => t.payload?.repo_id || t.repo || t.name || '?').filter(Boolean);
const _clashing = _allServes.filter(t => _taskPort(t) === _qrPort);
if (_clashing.length) {
const _names = _clashing.map(t => t.payload?.repo_id || t.repo || t.name || '?').filter(Boolean);
const _ok = await window.styledConfirm?.(
`${_names.length} model${_names.length === 1 ? '' : 's'} already serving on ${_qrHostStr || 'local'} (${_names.join(', ')}). Port 8000 will collide. Stop the running model and launch this one?`,
`${_clashing.length} model${_clashing.length === 1 ? '' : 's'} on port ${_qrPort} (${_names.join(', ')}). Stop it and launch this one?`,
{ confirmText: 'Stop & launch', cancelText: 'Cancel' }
);
if (!_ok) return;
// Mark + kill each running serve, then wait briefly for the
// tmux session to actually go down before we kick off the new
// launch. Otherwise vLLM still races against the dying socket.
quickRunBtn.disabled = true;
quickRunBtn.textContent = 'Stopping…';
for (const t of _activeServes) {
for (const t of _clashing) {
try {
// Use that task's own Stop button if it's rendered (handles
// endpoint cleanup, Ollama unload, fade-out). Falls back to
// a direct tmux kill if the Active tab isn't in the DOM yet.
const _taskEl = document.querySelector(`.cookbook-task[data-task-id="${t.sessionId}"]`);
const _stopBtn = _taskEl?.querySelector('.cookbook-task-action-stop');
if (_stopBtn) {
@@ -1537,11 +1535,12 @@ export function _expandModelRow(row, modelData) {
}
} catch (_killErr) { /* best-effort */ }
}
// Give the OS a beat to release port 8000.
await new Promise(r => setTimeout(r, 2500));
}
} catch (_e) { /* best-effort */ }
// -- Launch ───────────────────────────────────────────────────
// ─── Pre-launch driver check ─────────────────────────────────────
// vLLM/SGLang need a working CUDA/ROCm driver. nvidia-smi failures
// surface as system.gpu_error from our hardware probe; "no GPU
@@ -1550,8 +1549,6 @@ export function _expandModelRow(row, modelData) {
// user watches `pip install vllm` finish, then sees a cryptic CUDA
// error 10 minutes later. (llama.cpp / Ollama have CPU fallbacks
// so they skip this gate.)
const _qrBackendDetect = _detectBackend(modelData);
const _qrRunBackend = _qrBackendDetect.backend || 'vllm';
if (_qrRunBackend === 'vllm' || _qrRunBackend === 'sglang') {
const _sys = _hwfitCache?.system || {};
if (_sys.gpu_error) {
@@ -1658,7 +1655,7 @@ export function _expandModelRow(row, modelData) {
const host = _envState.remoteHost || '';
const hostIp = host.includes('@') ? host.split('@').pop() : host;
const port = '8000';
const port = _qrPort;
const detected = _detectBackend(modelData);
const runBackend = detected.backend || 'vllm';
@@ -1673,7 +1670,7 @@ export function _expandModelRow(row, modelData) {
} else if (runBackend === 'llamacpp') {
const dir = `"$HOME/.cache/huggingface/hub/models--${modelData.name.replace(/\//g, '--')}/snapshots"`;
const ggufPath = `$({ find ${dir} -name '*-00001-of-*.gguf' 2>/dev/null | sort; find ${dir} -name '*.gguf' 2>/dev/null | sort; } | head -1)`;
cmd = `llama-server --model "${ggufPath}" --host 0.0.0.0 --port 8080 -ngl 99 -c ${maxCtx} --flash-attn auto`;
cmd = `llama-server --model "${ggufPath}" --host 0.0.0.0 --port ${port} -ngl 99 -c ${maxCtx} --flash-attn auto`;
} else {
cmd = `vllm serve ${modelData.name} --host 0.0.0.0 --port ${port}`;
cmd += ` --tensor-parallel-size ${tp}`;
+19
View File
@@ -0,0 +1,19 @@
// Pure port helpers extracted so they're unit-testable without the
// browser-bound rest of cookbookRunning.js (issue #4507 follow-up).
// Read the port out of a serve launch command. Handles --port 8000,
// --port=8000, -p 8000, and -p=8000. Returns '' when none is present.
export function portOf(cmd) {
const s = cmd || '';
const m = s.match(/--port[=\s]+(\d+)/) || s.match(/(?:^|\s)-p[=\s]+(\d+)/);
return m ? m[1] : '';
}
// Lowest free port >= start that isn't in usedPorts (array or Set of
// numbers/strings). Returns a string to match the serve command format.
export function nextFreePort(usedPorts, start = 8000) {
const used = new Set([...usedPorts].map(p => parseInt(p, 10)));
let port = start;
while (used.has(port)) port++;
return String(port);
}
+6 -9
View File
@@ -8,6 +8,7 @@ import uiModule from './ui.js';
import { _diagnose, _showDiagnosis, _clearDiagnosis } from './cookbook-diagnosis.js';
import { registerMenuDismiss } from './escMenuStack.js';
import { computeProgressSignal } from './cookbookProgressSignal.js';
import { portOf, nextFreePort } from './cookbookPorts.js';
// Human-friendly badge label for a task's internal status. Avoids surfacing
// the word "error" in the sidebar — a server the user stopped or one that
@@ -266,9 +267,7 @@ function _taskHostLabel(task) {
}
function _taskPort(task) {
const cmd = task?.payload?._cmd || '';
const match = cmd.match(/--port\s+(\d+)/);
return match ? match[1] : '';
return portOf(task?.payload?._cmd || '');
}
function _buildCrashReport(task, outputText) {
@@ -455,16 +454,14 @@ function _nextAvailablePort() {
const usedPorts = new Set();
tasks.forEach(t => {
if (t.type === 'serve' && (t.status === 'running' || t.status === 'queued')) {
const m = t.payload?._cmd?.match(/--port\s+(\d+)/);
if (m) usedPorts.add(parseInt(m[1]));
const p = _taskPort(t);
if (p) usedPorts.add(parseInt(p));
}
});
presets.forEach(p => {
if (p.port) usedPorts.add(parseInt(p.port));
});
let port = 8000;
while (usedPorts.has(port)) port++;
return String(port);
return nextFreePort(usedPorts);
}
// ── Endpoint cleanup ──
@@ -3987,4 +3984,4 @@ export function initRunning(shared) {
}
// Also export _retryDownload and _nextAvailablePort for use by other modules
export { _retryDownload, _nextAvailablePort, _processQueue };
export { _retryDownload, _nextAvailablePort, _processQueue, _taskPort };
+33 -25
View File
@@ -2958,33 +2958,41 @@ function _rerenderCachedModels() {
&& ((t.remoteHost || '') === _hostStr || (t.remoteServerKey || '') === _serverKeyStr)
&& (t.status === 'running' || t.status === 'ready' || t._serveReady)
);
// Only block when the new model's port genuinely collides with
// a running serve. Different ports coexist fine (issue #4507).
if (_active.length) {
const _names = _active.map(t => t.payload?.repo_id || t.repo || t.name || '?').filter(Boolean);
const _ok = await window.styledConfirm(
`${_active.length} model${_active.length === 1 ? '' : 's'} already serving on ${_hostStr || 'local'} (${_names.join(', ')}). Port 8000 will collide. Stop the running model and launch this one?`,
{ title: 'Server already running', confirmText: 'Stop & launch', cancelText: 'Cancel' },
);
if (!_ok) { _restoreLaunchBtn(); return; }
// Kill each active serve; prefer the rendered Stop button so
// endpoint cleanup + Ollama unload run normally. Fall back to
// a raw tmux kill when the Active tab isn't in the DOM.
for (const t of _active) {
try {
const _el = document.querySelector(`.cookbook-task[data-task-id="${t.sessionId}"]`);
const _btn = _el?.querySelector('.cookbook-task-action-stop');
if (_btn) {
_btn.click();
} else if (_runningMod._tmuxGracefulKill) {
await fetch('/api/shell/exec', {
method: 'POST', credentials: 'same-origin',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ command: _runningMod._tmuxGracefulKill(t) }),
});
}
} catch (_killErr) { /* best-effort */ }
const _newPort = (launchCmd.match(/--port[=\s]+(\d+)/) || [])[1] || '';
const _clashing = _newPort
? _active.filter(t => _runningMod._taskPort(t) === _newPort)
: _active;
if (_clashing.length) {
const _names = _clashing.map(t => t.payload?.repo_id || t.repo || t.name || '?').filter(Boolean);
const _portNote = _newPort ? ` on port ${_newPort}` : '';
const _ok = await window.styledConfirm(
`${_clashing.length} model${_clashing.length === 1 ? '' : 's'} already serving on ${_hostStr || 'local'} (${_names.join(', ')})${_portNote}. Stop it and launch this one?`,
{ title: _newPort ? `Port ${_newPort} in use` : 'Server already running', confirmText: 'Stop & launch', cancelText: 'Cancel' },
);
if (!_ok) { _restoreLaunchBtn(); return; }
// Kill each clashing serve; prefer the rendered Stop button so
// endpoint cleanup + Ollama unload run normally. Fall back to
// a raw tmux kill when the Active tab isn't in the DOM.
for (const t of _clashing) {
try {
const _el = document.querySelector(`.cookbook-task[data-task-id="${t.sessionId}"]`);
const _btn = _el?.querySelector('.cookbook-task-action-stop');
if (_btn) {
_btn.click();
} else if (_runningMod._tmuxGracefulKill) {
await fetch('/api/shell/exec', {
method: 'POST', credentials: 'same-origin',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ command: _runningMod._tmuxGracefulKill(t) }),
});
}
} catch (_killErr) { /* best-effort */ }
}
await new Promise(r => setTimeout(r, 2500));
}
// Give the OS a beat to release port 8000.
await new Promise(r => setTimeout(r, 2500));
}
} catch (_e) { /* best-effort */ }
+5
View File
@@ -8,6 +8,7 @@
import uiModule from './ui.js';
import * as spinnerModule from './spinner.js';
import { bindMenuDismiss, dismissOrRemove } from './escMenuStack.js';
import { topPortalZ } from './toolWindowZOrder.js';
const API = window.location.origin;
let skills = [];
@@ -437,6 +438,10 @@ function _openSkillMenu(btn, card, sk, name, isPublished) {
menu.appendChild(cancelItem);
document.body.appendChild(menu);
// Override the CSS z-index (100002) with a value derived from the live
// tool-window stack so the kebab menu stays above its modal even after the
// bring-to-front counter climbs past the static value (#4720).
menu.style.zIndex = String(topPortalZ());
const r = btn.getBoundingClientRect();
menu.style.top = (r.bottom + 4) + 'px';
menu.style.right = Math.max(6, window.innerWidth - r.right) + 'px';
+6 -1
View File
@@ -6,6 +6,7 @@ import uiModule from './ui.js';
import markdownModule from './markdown.js';
import * as spinnerModule from './spinner.js';
import { makeWindowDraggable } from './windowDrag.js';
import { topPortalZ } from './toolWindowZOrder.js';
import { sortModelIds } from './modelSort.js';
import { ordinalSuffix } from './util/ordinal.js';
import { bindMenuDismiss, dismissOrRemove } from './escMenuStack.js';
@@ -903,7 +904,7 @@ function _showTaskDropdown(anchor, items) {
document.querySelectorAll('.task-dropdown').forEach(dismissOrRemove);
const dd = document.createElement('div');
dd.className = 'task-dropdown';
dd.style.cssText = 'position:fixed;z-index:100000;background:var(--panel);border:1px solid var(--border);border-radius:6px;box-shadow:0 4px 12px rgba(0,0,0,0.3);padding:4px;min-width:120px;';
dd.style.cssText = 'position:fixed;background:var(--panel);border:1px solid var(--border);border-radius:6px;box-shadow:0 4px 12px rgba(0,0,0,0.3);padding:4px;min-width:120px;';
items.forEach(item => {
const btn = document.createElement('button');
btn.style.cssText = 'display:flex;align-items:center;gap:8px;width:100%;text-align:left;padding:6px 10px;border:none;background:none;color:var(--fg);font-size:11px;font-family:inherit;cursor:pointer;border-radius:4px;transition:background 0.1s;';
@@ -919,6 +920,10 @@ function _showTaskDropdown(anchor, items) {
dd.appendChild(btn);
});
document.body.appendChild(dd);
// Sit above the currently-raised tool modal at any stack depth (#4720): the
// modal bring-to-front counter climbs unbounded, so a hardcoded z eventually
// loses. topPortalZ() derives the value from the live tool-window stack.
dd.style.zIndex = String(topPortalZ());
const rect = anchor.getBoundingClientRect();
let top = rect.bottom + 4;
let left = rect.right - dd.offsetWidth;
+2 -1
View File
@@ -16960,7 +16960,8 @@ body:not(.email-doc-split-active) #email-lib-modal.email-lib-fullscreen:not(.mod
/* Kebab dropdown */
.skill-kebab-menu {
position: fixed;
z-index: 100002;
/* z-index is set inline via topPortalZ() at open time (#4720); a static
value here loses once the modal bring-to-front counter climbs past it. */
min-width: 150px;
padding: 4px;
background: var(--panel, var(--bg));
+53
View File
@@ -0,0 +1,53 @@
"""Behavioral tests for Cookbook port parsing / picking (#4507 follow-up).
Driven through `node --input-type=module` (same approach as the other
*_js.py tests); skips when `node` is not installed.
"""
import json
import shutil
import subprocess
from pathlib import Path
import pytest
_REPO = Path(__file__).resolve().parent.parent
_HELPER = _REPO / "static" / "js" / "cookbookPorts.js"
_HAS_NODE = shutil.which("node") is not None
def _run(expr):
js = (
f"import {{ portOf, nextFreePort }} from '{_HELPER.as_posix()}';"
f"console.log(JSON.stringify({expr}));"
)
proc = subprocess.run(
["node", "--input-type=module"],
input=js, capture_output=True, text=True, cwd=str(_REPO), timeout=30,
)
assert proc.returncode == 0, proc.stderr
return json.loads(proc.stdout.strip())
@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
def test_port_of_handles_all_forms():
assert _run("portOf('vllm serve m --host 0.0.0.0 --port 8000')") == "8000"
assert _run("portOf('x --port=8001')") == "8001"
assert _run("portOf('llama-server -p 8002')") == "8002"
assert _run("portOf('llama-server -p=8003')") == "8003"
assert _run("portOf('serve with no port flag')") == ""
@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
def test_next_free_port_skips_taken_including_eq_and_short_flag():
# a --port= serve and a -p serve are both 'taken'; picker skips them
taken = "[portOf('a --port=8000'), portOf('b -p 8001')]"
assert _run(f"nextFreePort({taken})") == "8002"
assert _run("nextFreePort([])") == "8000"
assert _run("nextFreePort(['8000', '8002'])") == "8001"
@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
def test_clash_outcome_same_port_flagged_different_ignored():
# the guard's predicate is portOf(cmd) === target
assert _run("portOf('m --port 8000') === '8000'") is True
assert _run("portOf('m --port 8001') === '8000'") is False
+66
View File
@@ -53,6 +53,8 @@ with preserve_import_state("core.database", "src.database", "core.session_manage
_resolve_probe_key,
_classify_endpoint,
_rewrite_loopback_for_docker,
_openai_model_ids,
_ollama_model_names,
_PROVIDER_CURATED,
)
@@ -74,6 +76,33 @@ def _resp(status, *, json=None, headers=None, url="https://api.example.com/v1/mo
return httpx.Response(status, **kwargs)
# ── _openai_model_ids / _ollama_model_names: parsing helpers ──
class TestModelListHelpers:
@pytest.mark.parametrize("data,expected", [
({"data": [{"id": "gpt-4o"}, {"id": "gpt-4o-mini"}]}, ["gpt-4o", "gpt-4o-mini"]),
({"data": [{"id": None}, {"id": 123}, {"id": "gpt-4o"}]}, ["gpt-4o"]), # non-string ids dropped
({"data": ["x", {"id": "ok"}]}, ["ok"]), # non-dict entries dropped
({"data": []}, []),
({"data": "oops"}, []), # non-list "data"
([], []), ("nope", []), (None, []), (123, []), # non-dict body
])
def test_openai_model_ids(self, data, expected):
assert _openai_model_ids(data) == expected
@pytest.mark.parametrize("data,expected", [
({"models": [{"name": "llama3:8b"}, {"model": "qwen3:4b"}]}, ["llama3:8b", "qwen3:4b"]),
({"models": [{"name": "a", "model": "b"}]}, ["a"]), # name precedence over model
({"models": [{"name": 123}, {"model": None}, {"name": "ok"}]}, ["ok"]), # non-string values dropped
({"models": ["x", {"name": "ok"}]}, ["ok"]), # non-dict entries dropped
({"models": []}, []),
({"models": "oops"}, []),
([], []), (None, []), (42, []), # non-dict body
])
def test_ollama_model_names(self, data, expected):
assert _ollama_model_names(data) == expected
# ── _probe_endpoint: model-list parsing ──
class TestProbeEndpointParsing:
@@ -121,6 +150,43 @@ class TestProbeEndpointParsing:
)
assert _probe_endpoint("https://api.example.com/v1") == []
@pytest.mark.parametrize("body", [[], "invalid", 123, True])
def test_non_dict_json_body_degrades_to_empty(self, monkeypatch, caplog, body):
# HTTP 200 with valid-but-non-dict JSON must not crash the probe with an
# AttributeError (data.get(...) on a list/str/int); it should fall through
# to the empty/curated path. caplog gives this test teeth: pre-fix the
# swallowed AttributeError logs "Failed to probe"; post-fix it does not.
_patch_resolve(monkeypatch)
monkeypatch.setattr(
model_routes.httpx, "get",
lambda url, headers=None, timeout=None, verify=None, **kwargs: _resp(200, json=body),
)
with caplog.at_level("WARNING", logger="routes.model_routes"):
assert _probe_endpoint("https://api.example.com/v1") == []
assert "Failed to probe" not in caplog.text
def test_skips_non_string_model_ids(self, monkeypatch):
# A non-compliant upstream returns int/None IDs alongside a valid one.
# The probe must not crash on .lower()/.startswith and must still surface
# the valid string model.
_patch_resolve(monkeypatch)
monkeypatch.setattr(
model_routes.httpx, "get",
lambda url, headers=None, timeout=None, verify=None, **kwargs: _resp(
200, json={"data": [{"id": None}, {"id": 123}, {"id": "gpt-4o"}]}),
)
assert _probe_endpoint("https://api.example.com/v1", "key") == ["gpt-4o"]
def test_all_non_string_ids_returns_empty(self, monkeypatch):
# Every id is non-string -> empty result, no exception, no curated leak.
_patch_resolve(monkeypatch)
monkeypatch.setattr(
model_routes.httpx, "get",
lambda url, headers=None, timeout=None, verify=None, **kwargs: _resp(
200, json={"data": [{"id": 123}, {"id": None}]}),
)
assert _probe_endpoint("https://api.example.com/v1") == []
def test_chatgpt_subscription_probe_uses_discovery_only(self, monkeypatch):
_patch_resolve(monkeypatch)
calls = []
+6
View File
@@ -403,6 +403,12 @@ class TestIsChatModel:
def test_legacy_openai_instruct_is_not_chat(self):
assert _is_chat_model("gpt-3.5-turbo-instruct") is False
@pytest.mark.parametrize("bad", [None, 123, 4.5, ["x"], {"a": 1}])
def test_non_string_id_is_treated_as_chat(self, bad):
# Defensive boundary: a non-compliant upstream can yield a non-string
# model id; it must not crash on .lower() (treated as chat-capable).
assert _is_chat_model(bad) is True
# ── _classify_endpoint ──
+20
View File
@@ -9,6 +9,7 @@ and the dock-chip floor, without importing the browser-heavy UI modules.
"""
import json
import re
import shutil
import subprocess
import textwrap
@@ -87,3 +88,22 @@ def test_portal_z_uses_chip_floor_when_the_open_modal_sits_below_it():
)
assert values == {"z": 10031}
# tasks.js and skills.js were not in #4724's batch; #4767 routes their portaled
# dropdowns through the same helper. Pin that they use topPortalZ() and carry no
# hardcoded portal z-index, so they cannot regress to the #4720 bug.
@pytest.mark.parametrize("rel", ["static/js/tasks.js", "static/js/skills.js"])
def test_late_routed_dropdowns_use_top_portal_z(rel):
src = (ROOT / rel).read_text()
assert "topPortalZ" in src, f"{rel} must import/use topPortalZ()"
assert "topPortalZ()" in src, f"{rel} must call topPortalZ() for its dropdown z"
@pytest.mark.parametrize("rel", ["static/js/tasks.js", "static/js/skills.js", "static/style.css"])
def test_no_hardcoded_portal_z_literals_remain(rel):
src = (ROOT / rel).read_text()
# Match the exact 100000/100002 these dropdowns used; the trailing-digit
# guard avoids false-matching an unrelated 1000000 elsewhere.
hits = re.findall(r"z-index:\s*10000[02](?!\d)", src)
assert not hits, f"{rel} still has hardcoded portal z: {hits}"
+43
View File
@@ -0,0 +1,43 @@
"""Regression test for the task-path endpoint-URL normalization fix.
Bug: the task executor passed ``task.endpoint_url`` verbatim to the model HTTP
call (unlike the chat path, which normalizes via ``build_chat_url``). A bare
OpenAI-compatible base such as ``http://host:11434/v1`` POSTed to a 404 and the
run silently reported "The model returned an empty response".
The fix routes every resolved task endpoint through ``_normalize_chat_endpoint``.
"""
from src.task_scheduler import _normalize_chat_endpoint
def test_bare_v1_base_gets_chat_completions_suffix():
# The exact failure case: a bare /v1 base must become a full chat URL.
assert (
_normalize_chat_endpoint("http://localhost:11434/v1")
== "http://localhost:11434/v1/chat/completions"
)
def test_full_chat_url_is_unchanged_idempotent():
full = "http://localhost:11434/v1/chat/completions"
assert _normalize_chat_endpoint(full) == full
# Idempotent under repeated application.
assert _normalize_chat_endpoint(_normalize_chat_endpoint(full)) == full
def test_native_ollama_url_left_alone():
# Native Ollama (/api...) has its own downstream normalizer — don't touch it.
assert _normalize_chat_endpoint("http://localhost:11434/api") == "http://localhost:11434/api"
assert _normalize_chat_endpoint("http://localhost:11434/api/chat") == "http://localhost:11434/api/chat"
def test_empty_and_none_are_passthrough():
assert _normalize_chat_endpoint("") == ""
assert _normalize_chat_endpoint(None) is None
def test_trailing_slash_base_normalized():
assert (
_normalize_chat_endpoint("http://localhost:11434/v1/")
== "http://localhost:11434/v1/chat/completions"
)