Skip to content

fix(cookbook): classify task status and build crash reports from the runner log, not the pane#5696

Closed
catagris wants to merge 1927 commits into
odysseus-dev:devfrom
catagris:fix/task-status-log-first
Closed

fix(cookbook): classify task status and build crash reports from the runner log, not the pane#5696
catagris wants to merge 1927 commits into
odysseus-dev:devfrom
catagris:fix/task-status-log-first

Conversation

@catagris

@catagris catagris commented Jul 22, 2026

Copy link
Copy Markdown

Summary

Task status classification and the copy actions read the tmux pane (or the on-screen output mirroring it), while the ground truth lives in the persisted runner log. Three symptoms of that one design flaw: a completed install on a collapsed card shows "running" forever (the DOWNLOAD_OK → done flip lives in _reconnectTask, which only runs while the Running tab is visible and the card expanded — completion detection required an audience); a completed task whose session later dies flips to "crashed" (session-gone classification reads the empty on-screen text, finds no markers); and "Copy crash report" / "Copy last 50 lines" capture the user's shell greeting — hostname, LAN IP, hardware inventory, a privacy footgun for reports meant to be posted publicly — because the runner's post-exit interactive shell overwrites the pane. And the inverse (#2709): a connection drop was indistinguishable from session-gone, so a still-running download could classify as finished from stale output.

This PR makes the on-disk runner state the source of truth, the pattern #2709 prescribes:

  • _fetchTaskLogTail() fetches /tmp/odysseus-tmux/<sid>.log over the existing shell-exec path (remote side wrapped as sh -c for non-POSIX login shells); the session-gone classification and both copy actions fall back to it whenever the on-screen output lacks the === Process exited with code N === sentinel.
  • A 45s background completion sweep (started idempotently from the Running-tab render) classifies running download-type tasks from their logs with no audience required — mirroring the live path's done behaviour (deps refresh + cleanup of the lingering inspection session).
  • Transport failure ≠ session gone: has-session exiting 255/124 (ssh failure, timeout) now shows "reconnecting" and keeps polling — a dropped connection is a UI-signal problem, not a job-state change. Classification only happens once tmux itself reports the session gone (exit 1), at which point the runner is provably dead and the log is the truthful record.

Serve tasks are excluded throughout: a live server has no exit sentinel and their lifecycle belongs to the serve poller.

Target branch

  • This PR targets dev, not main.

Linked Issue

Fixes #5695
Fixes #2709

Related: #3772 (fixed the local flavor with output markers; the remote/collapsed/pane-overwritten cases remained).

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app and verified the change works end-to-end: on a real deployment, a collapsed pip install flipped to done within one sweep cycle of finishing (previously stuck "running" for an hour); after killing the tmux server and reloading, a finished task classified done from its log instead of crashed; "Copy crash report" on a genuinely failed task produced the actual error tail instead of a fastfetch banner.

How to Test

  1. Install a small pip dependency on a remote server from Cookbook → Dependencies and leave the card collapsed. Without this fix it shows "running" indefinitely; with it, the card flips to done within ~45s of the install finishing (and the leftover inspection tmux session is cleaned up).
  2. Let an install finish, kill the tmux server on the target (tmux kill-server), reload the page. Without this fix the task flips to "crashed"; with it, it classifies done from the runner log.
  3. On a failed task whose session dropped to the interactive shell, use "Copy crash report" / "Copy last 50 lines" — the content is the runner log tail (real error included), not the shell greeting, and "No log content available yet" no longer appears while a log exists on disk.
  4. Cookbook download card shows "finished" when tmux connection drops, while download is still actually running #2709's repro: start a long download, then break the transport mid-run (e.g. sudo iptables -A OUTPUT -p tcp --dport 22 -d <target> -j DROP on the Odysseus host, or drop the VPN). The card shows "reconnecting…" instead of flipping to finished/crashed; restore the connection and polling resumes with the true state.

Visual / UI changes — REQUIRED if you touched anything that renders

None — behaviour only. Status changes render through the existing badge/label/toast components ("reconnecting…" uses the existing running badge style); no new elements, styles, or layout.

🤖 Generated with Claude Code

pewdiepie-archdaemon and others added 30 commits June 27, 2026 21:11
Keep an unhealthy MemoryVectorStore instance available for health reporting instead of discarding it as disabled. This lets health checks report a degraded/down vector-store state while preserving focused regression coverage for initializer behavior.
falabellamichael and others added 26 commits July 11, 2026 15:14
…ls (#5420)

* fix: harden stabilization attachment and agent guards

* fix(uploads): preserve durable references during cleanup

* fix(uploads): close cleanup and compaction races
…(#4411) (#5160)

* docs: update static/js/MODULE_SUMMARY.md to reflect current ES6 frontend

Rewrite the stale module summary to match the current no-build,
ES6-module frontend architecture. Adds coverage of app.js orchestration,
the chat/SSE pipeline (chat.js, chatStream.js, chatRenderer.js,
streamingRenderer.js), new subsystems (research/, compare/, document
streaming, cookbook*, skills.js), and removes the obsolete <script> load
order assumptions.

* cleanup: remove dead MEMORY_DOC / memory_doc paths (closes #4411)

Removes the unused MEMORY_DOC constant and the matching DataConfig
memory_doc field / set_data_paths entry. No runtime code imports or
references these paths, so this is a no-behavior-change dead-code
cleanup under the storage-architecture tracker #4377.
* fix(db): restrict data/app.db to 0600

app.db holds bearer-token hashes, bcrypt password hashes, and encrypted
provider keys but was created under the default umask (0644 -> world-readable),
unlike .app_key/vault/integrations which are already 0600 via safe_chmod.

init_db() now chmods the SQLite file to 0600 right after create_all (POSIX
only; no-op on Windows, skipped for Postgres / in-memory). Unconditional and
idempotent, so it also re-locks already-deployed 0644 installs on next
startup. The transient rollback journal inherits 0600 from the parent file at
creation - no sidecar handling needed; -wal/-shm don't exist until WAL is
enabled (#4409 C4) and inherit the same mode then.

Satisfies Rule B, unblocking #4413 and the vault/integration secret moves.
Mirrors src/secret_storage.py:43-45.

Verified: security + DB-permission suites pass; 6 pre-existing visual_report
failures (missing markdown/nh3 deps) are unrelated.

Closes #4407

* fix(db): harden SQLite path parsing and re-lock sidecars

Address review feedback on #4420.

P2: derive the file to chmod from engine.url (SQLAlchemy's parsed URL)
via _sqlite_db_path(), instead of DATABASE_URL.replace("sqlite:///", "").
A driver-qualified URL (sqlite+pysqlite://) or one carrying query args
(?cache=shared) previously slipped past the prefix check / string slice
and left the DB world-readable; the parsed path resolves correctly and
drops the query.

P3: re-lock stale -wal/-shm/-journal sidecars to 0o600 at startup. The
main file is chmod'd first, so any sidecar SQLite creates afterward
inherits 0o600, but a -wal/-shm left world-readable by an older 0o644
install (once WAL was enabled) could still expose DB pages. Absent
sidecars are the normal case, not an error.

Tests: unit-test _sqlite_db_path across driver/query/memory/postgres URL
forms, and a subprocess test asserting stale 0o644 -wal/-shm are
re-locked on startup.

* fix(db): handle sqlite file URI app db permissions

* fix(db): close remaining SQLite permission bypasses

---------

Co-authored-by: Ethan <23321960+0xLeathery@users.noreply.github.com>
Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
chat_stream() references `_explicit_web_intent` in three places
(disabled-tools gating, global-disabled web allowance, and the
per-turn tool filter) but the assignment was dropped during a
branch merge. Every chat request raised

    NameError: name '_explicit_web_intent' is not defined

at routes/chat_routes.py, surfacing to the client as a bare
"Internal Server Error" before any LLM call was made — chat was
fully broken on dev and main.

Restore the original definition, computed from the already-derived
tool intent, immediately before its first use:

    _explicit_web_intent = bool(_tool_intent and _tool_intent.category == "web")

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 2d81770)
…mporter (#5261)

* Harden skill importer against SSRF: block private targets + revalidate redirects per hop

The skill importer validated only the initial URL with the lenient SSRF guard
(block_private=False) and then fetched with follow_redirects=True, so a 3xx to
an internal/metadata address (169.254.169.254, 127.0.0.1, RFC-1918) was still
connected to — inconsistent with the hardened services/search/content.py
:_get_public_url path.

Add a _get_checked() helper that follows redirects manually and re-runs the
SSRF guard with block_private=True on every hop, and route all three fetch
sites (skills.sh unwrap, _fetch_bytes, _list_github_dir) through it. GitHub's
own redirects and the final-host _assert_github_url checks are preserved.

Adds hermetic regression tests (IP-literal hosts, faked HTTP layer) and updates
the existing mock signature for the new block_private kwarg.

Defense-in-depth: the endpoint is admin-gated (require_admin) and admins are
trusted per THREAT_MODEL.md, so this is not a cross-boundary vulnerability.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: enforce follow_redirects=False invariant in mock client

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Add Arch-specific package installation and NVIDIA runtime configuration
to the Docker setup guide. Cover passthrough verification, the NVIDIA
Compose overlay, and the distinction between GPU passthrough and
CUDA-backed model serving

Refs #831
chore: sync upstream changes from odysseus-dev/odysseus
fix(docker): bump Docker CLI to a patched release
fix(docker): bump Docker CLI to a patched release
* feat(models): define capability schema and readers

* fix(models): harden Google catalog probing

Restrict native catalog probing to the Gemini host, keep provider keys out of request URLs, filter non-chat model resources, and preserve the manual refresh default in the built-in Google add flow.
…(#5474)

* security(url-safety): reject RFC 6598 shared address space in strict mode

Strict mode (block_private=True) is a full SSRF lockdown, but it only
rejected is_private and is_loopback targets. CPython does not classify RFC
6598 shared/CGNAT space (100.64.0.0/10) as is_private (it is "shared", not
"private"), so a public redirect into 100.64.0.1 passed the per-hop guard
and still issued the request to a potentially internal CGNAT service.

not is_global would also exclude it, but only on CPython 3.11.10+/3.12.4+/
3.13+; the CI matrix runs 3.11/3.12, so reject the range explicitly to stay
correct across patch levels and the 3.14 runtime image. Default local-first
mode is unchanged. Adds strict-mode coverage for shared, non-global, and
public targets.

* docs(url-safety): correct CGNAT is_global rationale in strict-mode comment

The prior comment claimed `not is_global` catches 100.64.0.0/10 only on
CPython 3.11.10+/3.12.4+/3.13+. That is inaccurate for CGNAT: is_global
is False for 100.64.0.1 on every supported version (verified 3.10-3.14).
The version-fragility applies to other ranges gh-113171 touched, not CGNAT.
The explicit range reject is still the right choice; restate the reason as
is_private not covering shared space, and not coupling strict mode to
is_global's broader, cross-version definition. No behavior change.
…… (#5491)

* fix(llm): enhance fallback logic to handle empty completions and improve metadata handling

* fix(llm): stream tool call deltas immediately
Slice 2f of the route-domain reorganization (#4082/#4071, per
specs/architecture-runtime-inventory.md §6.3). Moves note_routes.py into
routes/note/, leaving a backward-compat sys.modules shim at the old path.
Pure file reorganization, no behavior change.

The shim uses sys.modules replacement (same pattern as the merged gallery
#4903, research #4975, memory #5007, history #5090, and contacts #5227
slices) so that `import routes.note_routes`, `from routes.note_routes import
X`, `importlib.import_module(...)`, and the `import ... as note_routes` +
`monkeypatch.setattr(note_routes, "SessionLocal", ...)` pattern used by
test_note_reminder_fire_scope.py / test_notes_fail_closed_auth.py all
operate on the same module object the application uses.

The canonical module does NOT depend on the shim — routes/note/note_routes.py
imports only from core/, src/, and stdlib. The outbound email cross-domain
imports (routes.email_routes._get_email_config, routes.email_helpers.
_send_smtp_message) are function-local lazy imports that keep resolving
through the email module's own path (email is not yet migrated).

One source-introspection test site repointed to the new canonical path:
- test_model_helper_owner_scope.py (shared with history; history entry
  already repointed in #5090, note entry repointed here)

Adds tests/test_note_routes_shim.py to pin the sys.modules shim contract
(legacy and canonical paths resolve to the same module object; monkeypatch
via legacy alias reaches the canonical module).

Verified: compileall clean; full suite 4487 passed, 3 skipped.
…runner log, not the pane

The runner intentionally drops into an interactive shell after the
process exits so the tmux session stays inspectable — but that means
the pane gets overwritten by the user's shell greeting, and after a
page reload with the tmux server gone there is no pane at all. Task
classification and the copy actions read only the on-screen/pane text,
so:

- a task that finished cleanly (exit-0 sentinel + DOWNLOAD_OK sitting
  in /tmp/odysseus-tmux/<sid>.log) got mislabeled "crashed" when its
  session disappeared before a poll saw the markers
- "Copy crash report" shipped the user's neofetch banner (plus
  hostname/IP/hardware details) instead of the actual error
- "Copy last 50 lines" said "No log content available yet" while a
  54KB log sat on disk

New _fetchTaskLogTail() fetches the persisted log tail over the same
shell-exec path (remote side wrapped as `sh -c` for non-POSIX login
shells). _reconnectTask's session-gone classification, Copy crash
report, and Copy last 50 lines all fall back to it whenever the
on-screen output lacks the exit sentinel. Windows sessions (different
log path) keep the existing pane behaviour.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e from their runner log

_reconnectTask only polls while the Running tab is visible AND the task
card is expanded, so a download/pip install that finished on a collapsed
card showed "running" indefinitely — the DOWNLOAD_OK marker and exit
sentinel sat unread in the runner log until someone expanded the card
(or the session died and the task got mislabeled crashed).

New 45s background sweep (started idempotently from _renderRunningTab)
checks running download-type tasks' persisted logs via _fetchTaskLogTail
and classifies from the exit sentinel: exit 0 without DOWNLOAD_FAILED →
done (+ deps refresh + best-effort kill of the lingering inspection
session, mirroring the live DOWNLOAD_OK branch), anything else →
crashed with the log tail as output. Serve tasks are excluded — a live
server has no exit sentinel and their semantics belong to the serve
poller.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Jul 22, 2026
…e classifying tasks

has-session exiting 0 means alive and 1 means no-such-session — but 255
(ssh transport failure) and 124 (timeout) were lumped in with 1, so a
dropped connection classified the task from stale output: false
"finished" while a download was still running (#2709), or false
"crashed" for a job that was fine. Transport failures now show
"reconnecting" and keep polling; classification only happens once tmux
itself confirms the session is gone — at which point the runner is
provably dead and the persisted log is the truthful record.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet