Fix Cookbook dependency install completion state
* Fix Cookbook dependency install completion state Mark Cookbook dependency installs as complete when the background runner exits successfully, even when HuggingFace-specific download markers are absent. * Add focused regression coverage for cookbook dependency completion. Keep the fix narrowly scoped while carrying env_path through dependency tasks and locking the completion reconciliation behavior with targeted tests.
This commit is contained in:
committed by
GitHub
parent
acfdcf346c
commit
eda99360d1
@@ -1993,11 +1993,17 @@ def setup_cookbook_routes() -> APIRouter:
|
||||
status = "unknown"
|
||||
if is_alive or (local_win_task and full_snapshot):
|
||||
lower = full_snapshot.lower()
|
||||
has_exit = "=== process exited with code" in lower
|
||||
exit_match = re.search(r"=== process exited with code\s+(-?\d+)", full_snapshot, re.I)
|
||||
has_exit = exit_match is not None
|
||||
exit_code = int(exit_match.group(1)) if exit_match else None
|
||||
has_error = "error" in lower or "failed" in lower or "traceback" in lower
|
||||
if has_exit and task_type == "serve":
|
||||
# Serve tasks that exit are always errors — they should run indefinitely
|
||||
status = "error"
|
||||
elif has_exit and task_type == "download":
|
||||
# Dependency installs are tracked as download tasks but only
|
||||
# emit the generic runner exit marker, not HF download markers.
|
||||
status = "completed" if exit_code == 0 else "error"
|
||||
elif has_exit and "unrecognized arguments" in lower:
|
||||
status = "error"
|
||||
elif has_error and not ("application startup complete" in lower):
|
||||
|
||||
@@ -839,8 +839,15 @@ def setup_shell_routes() -> APIRouter:
|
||||
"""
|
||||
_require_admin(request)
|
||||
_reject_cross_site(request)
|
||||
import importlib, importlib.metadata as importlib_metadata, shlex, json as _json
|
||||
import importlib, importlib.metadata as importlib_metadata, shlex, json as _json, site, sys
|
||||
_prepend_user_install_bins_to_path()
|
||||
importlib.invalidate_caches()
|
||||
try:
|
||||
user_site = site.getusersitepackages()
|
||||
if user_site and os.path.isdir(user_site) and user_site not in sys.path:
|
||||
sys.path.append(user_site)
|
||||
except Exception:
|
||||
pass
|
||||
if ssh_port and str(ssh_port).strip() not in ("", "22"):
|
||||
_port = str(ssh_port).strip()
|
||||
if not _SSH_PORT_RE.match(_port) or not (1 <= int(_port) <= 65535):
|
||||
|
||||
@@ -716,7 +716,7 @@ async function _fetchDependencies() {
|
||||
}
|
||||
// _dep flags this as a pip dependency/driver install (not a servable
|
||||
// model) so the running-task card doesn't offer a "Serve →" button.
|
||||
const payload = { repo_id: pipName, _cmd: cmd, remote_host: _envState.remoteHost || '', _dep: true };
|
||||
const payload = { repo_id: pipName, _cmd: cmd, remote_host: _envState.remoteHost || '', _dep: true, env_path: _envState.envPath || '' };
|
||||
_addTask(data.session_id, 'pip ' + pkgName, 'download', payload);
|
||||
if (statusEl) { statusEl.textContent = upgrade ? 'Updating...' : 'Installing...'; statusEl.disabled = true; }
|
||||
uiModule.showToast(`${upgrade ? 'Updating' : 'Installing'} ${pkgName} on ${targetHost}...`);
|
||||
|
||||
@@ -1367,6 +1367,8 @@ export function _renderRunningTab() {
|
||||
|
||||
const tasks = _loadTasks();
|
||||
const hasContent = tasks.length > 0;
|
||||
const activeCount = tasks.filter(t => t.status === 'running' || t.status === 'queued').length;
|
||||
const activeCountHtml = activeCount ? ` <span class="cookbook-tab-count">${activeCount}</span>` : '';
|
||||
|
||||
let tabBar = body.querySelector('.cookbook-tabs');
|
||||
if (!tabBar) return;
|
||||
@@ -1376,7 +1378,7 @@ export function _renderRunningTab() {
|
||||
runTab.className = 'cookbook-tab';
|
||||
runTab.dataset.backend = 'Running';
|
||||
const _errCount = tasks.filter(t => t.status === 'error' || t.status === 'crashed').length;
|
||||
runTab.innerHTML = `Running <span class="cookbook-tab-count">${tasks.length}</span>${_errCount ? `<span class="cookbook-tab-error-dot"></span>` : ''}`;
|
||||
runTab.innerHTML = `Running${activeCountHtml}${_errCount ? `<span class="cookbook-tab-error-dot"></span>` : ''}`;
|
||||
tabBar.insertBefore(runTab, tabBar.firstChild);
|
||||
runTab.addEventListener('click', () => {
|
||||
tabBar.querySelectorAll('.cookbook-tab').forEach(t => t.classList.remove('active'));
|
||||
@@ -1387,7 +1389,7 @@ export function _renderRunningTab() {
|
||||
});
|
||||
} else if (runTab) {
|
||||
const _errCount2 = tasks.filter(t => t.status === 'error' || t.status === 'crashed').length;
|
||||
runTab.innerHTML = tasks.length ? `Running <span class="cookbook-tab-count">${tasks.length}</span>${_errCount2 ? '<span class="cookbook-tab-error-dot"></span>' : ''}` : 'Running';
|
||||
runTab.innerHTML = tasks.length ? `Running${activeCountHtml}${_errCount2 ? '<span class="cookbook-tab-error-dot"></span>' : ''}` : 'Running';
|
||||
if (!hasContent) {
|
||||
if (runTab.classList.contains('active')) {
|
||||
const wfTab = tabBar.querySelector('.cookbook-tab[data-backend="Search"]');
|
||||
@@ -1404,7 +1406,7 @@ export function _renderRunningTab() {
|
||||
group.dataset.backendGroup = 'Running';
|
||||
group.innerHTML = '<div class="admin-card" style="flex:1;display:flex;flex-direction:column;overflow:hidden;">' +
|
||||
'<div style="display:flex;align-items:baseline;gap:8px;margin-bottom:2px;">' +
|
||||
'<h2 style="margin:0;padding:0;line-height:1;">Running <span id="running-count" class="memory-count" style="font-size:0.6em;opacity:0.6;font-weight:normal">' + tasks.length + '</span></h2>' +
|
||||
'<h2 style="margin:0;padding:0;line-height:1;">Running <span id="running-count" class="memory-count" style="font-size:0.6em;opacity:0.6;font-weight:normal">' + activeCount + '</span></h2>' +
|
||||
'</div>' +
|
||||
'<p class="memory-desc doclib-desc" style="margin-top:6px;">Active downloads and serving processes.</p>' +
|
||||
'</div>';
|
||||
@@ -1416,7 +1418,7 @@ export function _renderRunningTab() {
|
||||
if (!group) return;
|
||||
|
||||
const countEl = group.querySelector('#running-count');
|
||||
if (countEl) countEl.textContent = tasks.length;
|
||||
if (countEl) countEl.textContent = activeCount;
|
||||
|
||||
if (!hasContent) {
|
||||
group.remove();
|
||||
@@ -2786,6 +2788,41 @@ async function _pollBackgroundStatus() {
|
||||
const data = await res.json();
|
||||
const tasks = data.tasks || [];
|
||||
|
||||
// Reconcile the authoritative tmux/process status back into the persisted
|
||||
// client task list. The Running-tab reconnect loop also does this, but it
|
||||
// only exists while cards are rendered; after a page refresh or closed modal
|
||||
// dependency installs could finish server-side while localStorage stayed
|
||||
// stuck at "running".
|
||||
try {
|
||||
const statusById = new Map(tasks.map(t => [t.session_id, t]));
|
||||
const localTasks = _loadTasks();
|
||||
let changed = false;
|
||||
const completedDeps = [];
|
||||
for (const task of localTasks) {
|
||||
const live = statusById.get(task.sessionId);
|
||||
if (!live) continue;
|
||||
const updates = {};
|
||||
const nextStatus = live.status === 'completed'
|
||||
? 'done'
|
||||
: (live.status === 'error' ? 'error' : null);
|
||||
if (nextStatus && task.status !== nextStatus) {
|
||||
updates.status = nextStatus;
|
||||
if (nextStatus === 'done' && task.payload?._dep) completedDeps.push(task);
|
||||
}
|
||||
if (live.progress && live.progress !== task.progress) updates.progress = live.progress;
|
||||
if (live.output_tail && live.output_tail !== task.output) updates.output = live.output_tail;
|
||||
if (Object.keys(updates).length) {
|
||||
Object.assign(task, updates);
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
if (changed) {
|
||||
_saveTasks(localTasks);
|
||||
_renderRunningTab();
|
||||
completedDeps.forEach(t => _refreshDepsAfterInstall(t));
|
||||
}
|
||||
} catch (_) { /* non-fatal: background status should never break polling */ }
|
||||
|
||||
const statusEl = document.getElementById('cookbook-bg-status');
|
||||
const activeTasks = tasks.filter(t => t.status === 'running' || t.status === 'ready');
|
||||
const errorTasks = tasks.filter(t => t.status === 'error');
|
||||
|
||||
41
tests/test_cookbook_dependency_completion_regression.py
Normal file
41
tests/test_cookbook_dependency_completion_regression.py
Normal file
@@ -0,0 +1,41 @@
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
def _read(rel_path: str) -> str:
|
||||
return (ROOT / rel_path).read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_backend_status_treats_download_exit_zero_as_completed():
|
||||
source = _read("routes/cookbook_routes.py")
|
||||
|
||||
assert "exit_match = re.search(r\"=== process exited with code\\s+(-?\\d+)\"" in source
|
||||
assert "elif has_exit and task_type == \"download\":" in source
|
||||
assert "status = \"completed\" if exit_code == 0 else \"error\"" in source
|
||||
|
||||
|
||||
def test_background_status_poll_reconciles_into_local_tasks():
|
||||
source = _read("static/js/cookbookRunning.js")
|
||||
|
||||
assert "const statusById = new Map(tasks.map(t => [t.session_id, t]));" in source
|
||||
assert "const nextStatus = live.status === 'completed'" in source
|
||||
assert "? 'done'" in source
|
||||
assert ": (live.status === 'error' ? 'error' : null);" in source
|
||||
assert "_saveTasks(localTasks);" in source
|
||||
assert "completedDeps.forEach(t => _refreshDepsAfterInstall(t));" in source
|
||||
|
||||
|
||||
def test_dependency_install_payload_keeps_env_path_for_refresh():
|
||||
source = _read("static/js/cookbook.js")
|
||||
|
||||
assert "env_path: _envState.envPath || ''" in source
|
||||
|
||||
|
||||
def test_local_dependency_probe_refreshes_user_site_visibility():
|
||||
source = _read("routes/shell_routes.py")
|
||||
|
||||
assert "importlib.invalidate_caches()" in source
|
||||
assert "user_site = site.getusersitepackages()" in source
|
||||
assert "if user_site and os.path.isdir(user_site) and user_site not in sys.path:" in source
|
||||
Reference in New Issue
Block a user