Fix 21 safety and reliability defects found by audit - #131
Merged
Conversation
Avoid re-hashing the ~100MB llama.cpp runtime directory on every /api/v1/system poll (every 2s while idle) by memoizing the verification result against a cheap (path, size, mtime_ns) fingerprint, recomputing the full SHA-256 walk only when that fingerprint changes.
…n-ASCII input Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sweep orphaned .cortex-frontend-build-* staging directories left by a killed or crashed build before starting a new one, logging what was reclaimed. Also move the npm-install cache marker and node_modules out of the disposable per-build staging directory into a stable cache keyed on the package-lock.json hash, so an unchanged lockfile can actually skip npm ci instead of always missing the cache.
An empty local-model inventory (Ollama down, a failed refresh) is a routine, recoverable state, but saveDraft unconditionally wrote null over modelSettings.chat whenever the picker had nothing to offer, silently erasing a still-valid configured model on any unrelated settings save.
Overrides tuned before the first message were stored under the "new chat" placeholder key and never migrated once a real thread id existed, so they reverted after one message and leaked into whatever new chat the user started next.
Virtuoso's components.Footer was a fresh arrow function on every render, and a changed function identity there makes Virtuoso remount the Footer subtree -- tearing down and rebuilding the streaming bubble on every token in a long transcript. Keep Footer's own identity stable via useCallback and read the latest content through a ref, so React updates the existing instance instead.
…arving attachments fit_history_to_context broke out of its greedy walk the moment the single newest exchange alone exceeded the budget, discarding every older exchange too -- right after the model produces one long answer, the next message was sent with zero history and no indication it was gone. Candidate sizes are not monotonic once a newly-unpaired trailing assistant message is dropped, so the walk now keeps going instead of stopping at the first miss. Separately, attachments were fit against whatever the history fitter left behind, and history claimed the whole budget first -- a document attached mid-conversation could be cut to a few percent of its content with no signal to the user. Attachments are now fit first against a placeholder history to reserve their own room, and history is sized around that reservation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…batch fix: reliability quick-fixes batch (10 findings)
fix: context-window budget allocator (history wipe + attachment starvation)
JobRegistry.shutdown() awaited every pending worker with no bound at all, including one stuck inside a synchronous call that never polls cancel_event -- a model HTTP request with no read deadline, for example. That could hang app shutdown, and the llama-server child process behind it, for as long as that call took. Workers that have already begun committing their result still get an unbounded wait (a commit must finish, or persisted state and the event stream diverge); everything else is now bounded by a grace period and re-checked afterward, so a worker that started committing during the grace period still gets its due wait. Separately, the lifespan's runtime teardown (the only thing that actually terminates the llama-server child) ran strictly after job shutdown returned, so an exception there skipped it entirely. It now runs regardless. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: bound generation shutdown so llama-server is never orphaned
…tops Cancellation was only checked between phases -- once the single blocking model HTTP call started, nothing aborted it, so Stop left the composer locked and the generation slot occupied for the rest of that call (up to the read timeout), burning GPU on output that would be discarded regardless. Both chat clients now accept an optional cancellation_event. When one is given (only the real chat turn passes one -- title and translation calls don't), the call switches to a streamed request and checks the event between chunks, closing the response as soon as it fires instead of only after the model finishes on its own. Callers that never pass a cancellation_event keep today's exact single-shot request, so nothing else changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: thread cancellation into the chat-client call so Stop actually stops
llama-server was spawned with no Windows Job Object, so any hard exit of Cortex -- Task Manager, a crash, the launcher supervisor's own shutdown timeout -- left it running and holding the entire model resident in RAM/VRAM with no owner. The default launcher now creates a kill-on-close Job Object and assigns each launched process to it, reusing the same job across restarts; sandboxed execution workers already get this exact policy, it was simply missing here. The crash-loop guard tore the process down while still holding the state lock the class documents as held for microseconds only, so the runtime-status endpoint froze for the whole termination grace wait right as the guard fired. It now computes the verdict under the lock, releases it, and terminates outside -- the same pattern _terminate_and_reset already used six lines away. The known-bad GPU backend marker was a single global string, so one oversized model failing on vulkan permanently pushed every other model -- and every other context size -- to cpu too. It is now scoped to the exact (model, num_ctx, runtime release) that failed, and expires after 24 hours so a driver update or freed VRAM gets a retry rather than needing a manual reset. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: harden llama-server process lifecycle (orphans, locking, GPU bans)
expires_at was fixed at issuance, so the desktop app hard-locked after exactly one hour of continuous use -- the frontend has no way to reach a fresh bootstrap token once its only credential is destroyed after the initial handoff, so the only recovery was a full quit and relaunch. Every successful authenticate() now extends expires_at by the full TTL again, capped at issued_at + a 24-hour absolute maximum so a session cannot renew itself forever. A session that genuinely goes idle for longer than the TTL still expires normally, since nothing calls authenticate() to renew it while idle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: slide the session expiry forward on every authenticated request
initialMountRef is a per-instance ref, so it was true on every mount -- including a return from /settings, which unmounts ChatPage but leaves the module-level generation store populated with its accumulated text. The cursor was forced back to 0 while beginGeneration (the only thing that would clear the buffers) was skipped precisely because the job id still matched, so the backend replayed every event onto text that was already there and the answer rendered concatenated with itself. The rewind now happens only when the store is genuinely cold for that job -- a real page reload, which is the case the ref was written for. A route remount resumes from the persisted cursor instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…k guard _validate_network_url resolved the hostname, rejected the URL if any returned address was private/loopback/link-local, then threw that resolution away and handed the hostname back to urllib -- which resolved it a second time when opening the socket. A nameserver answering differently across those two lookups passed the check and then connected to whatever it wanted: the local Ollama API on 127.0.0.1:11434, other localhost dev servers, router and NAS admin pages. The redirect handler had the same gap one hop later. Validation now returns the address it approved, and the request is made through connection classes that dial exactly that address. The hostname still travels in the Host header and in TLS SNI, and server_hostname is still passed to wrap_socket, so certificate validation is unchanged -- only the address dialed is forced. Redirects re-validate and re-pin rather than only re-validating. The connection classes are built per connection, not once up front, so each redirect hop dials what its own validation approved instead of the first hop's address. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every settings write takes a full-file backup copy before touching the database. Settings lived inside the chat database, so each save byte-copied the entire transcript store: toggling the theme stalled on a full-database copy, permanently doubled the history's disk footprint, and on a constrained disk failed outright -- an unrelated settings save rejected because the chat database was large. The produced .bak could also be a torn, unopenable SQLite file. Settings now live in cortex_settings.sqlite, following the existing execution_database precedent for keeping a concern out of the chat store. A one-time adoption copies an existing colocated settings row into the new file on first run; without it every existing install would silently revert to defaults on upgrade. The source row is left in place so downgrading to a previous build stays a non-event. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes all 21 defects confirmed by a safety/reliability audit of this repo. Every fix has a regression test that was verified to fail before the fix and pass after.
mainis untouched until this merges.High severity
fit_history_to_contextstopped walking the moment the newest exchange alone exceeded the budget, discarding every older exchange too and returning"No history available."— so right after the model wrote a long answer, the next message went out with no context at all. Reproduced: a 10-exchange thread ending in a 35k-character answer kept 0 exchanges; after the fix, 10.Medium severity
npm cibecause the cache marker could never hitTest plan
python -m pytest tests/ -q— 547 passed, 1 skipped (baseline before this work: 514)npx vitest run— 168 passed (baseline: 165)tsc --noEmit,eslint,ruff— clean🤖 Generated with Claude Code