* fix(mcp): stop assuming http://localhost:7000 for the OAuth callback
The MCP OAuth callback origin is wrong on any install not reached at
http://localhost:7000, and on Docker it cannot be corrected at all.
Three sites, one assumption:
- The redirect base fell back to a fixed port 7000. The app binds APP_PORT
natively (app.py, launcher.py) and the macOS launcher defaults to 7860,
where 7000 is AirPlay Receiver, so the callback lands on another service
entirely. The fallback now follows APP_PORT. The hostname stays localhost
rather than internal_api_base()'s 127.0.0.1: this URI is registered with
the authorization server, so changing the host would invalidate the
registrations that already exist.
- The paste-back form hardcoded an http:// action. Serving the page over
HTTPS, Chrome raises its insecure-form interstitial, and overriding that
posts plain HTTP at a TLS port, which fails too. Either way the
authorization code never reaches Odysseus. The action now carries the
scheme the request arrived on.
- OAUTH_REDIRECT_BASE_URL is the only fix available to a Docker install,
because the container listens on 7000 and cannot see the host port map,
but compose never forwarded it and nothing documented it. Both fixed.
* fix(mcp): make the paste-back form action relative and export APP_PORT
Answers the review on #6032. Three of the fixes did not survive contact with
the deployments they targeted.
- The form action derived its scheme from request.url.scheme. uvicorn only
honours X-Forwarded-Proto from a peer inside --forwarded-allow-ips, which
defaults to 127.0.0.1; the Dockerfile CMD sets no override, so a proxy
arriving over the Docker bridge is untrusted and the scheme stays http.
That is mixed content on exactly the HTTPS installs paste-back exists for.
A relative action is resolved by the browser against the origin the page
came from, which is right under every proxy setup, and it drops the Host
header from the page entirely.
- The APP_PORT fallback never fired for the shipped launchers. start-macos.sh,
the generated .app launcher and launch-windows.ps1 all pass --port to
uvicorn without putting the value in the environment, so the motivating
case, macOS on 7860, still registered localhost:7000. Each now exports it.
internal_api_base() and companion pairing read APP_PORT too and were wrong
in the same way.
- .env.example pointed Google MCP servers at OAUTH_REDIRECT_BASE_URL.
add_server writes Desktop App credentials, and Google only accepts loopback
redirects for that client type, so a public origin comes back as
redirect_uri_mismatch. The variable is for the DCR flow; Google stays on the
loopback default and finishes remotely through paste-back.
The Host header is no longer reflected into the page, so the escaping
regression test asserts its absence instead of its escaping.
* fix(docker): migrate retained SearXNG settings
Retained nonempty SearXNG settings can miss defaults required by newer pinned images while bypassing the entrypoint's narrow regeneration checks.
Add an atomic PyYAML-aware migration to all Compose variants. Preserve existing inheritance choices, custom content, secrets, ownership, and mode while inserting only the missing top-level default-inheritance key.
Validated with 39 focused and adjacent tests, compile checks, and fresh and retained pinned-image HTTP 200 gates. Full repository CI remains for the PR.
* fix(docker): chmod the settings temp file before chowning it
The Compose cap set is `cap_drop: ALL` plus CHOWN/SETGID/SETUID/DAC_OVERRIDE
and carries no FOWNER, and searxng's own entrypoint chowns /etc/searxng to
searxng:searxng, so every retained settings file belongs to that user by the
second boot. Chowning the temporary file first left root unable to chmod it,
so the migration exited 1 and `set -eu` killed the container before
`exec /usr/local/searxng/entrypoint.sh` — SearXNG never started and odysseus
blocked on its healthcheck.
Swap the two calls so the chmod lands while the temporary file is still
root-owned, and cover the ordering with a test that refuses the chmod once
the chown has happened, the way the kernel does.
* fix(docker): let searxng boot when the settings migration fails
The migration runs under `set -eu`, so any settings file it cannot parse or
rewrite took the container down instead of merely going unmigrated. A symlinked
/etc/searxng/settings.yml is enough: the migration refuses a non-regular file
and searxng, which reads through the symlink perfectly well, never got to start.
Guard the call with `|| true` in all three Compose variants. The failure still
prints its reason on stderr, and searxng is left to report anything genuinely
wrong with the file.
---------
Co-authored-by: Léo <leograndcontact@gmail.com>
The timeline's terminating dot used a single left offset (-17px) at both
breakpoints, but the thread's padding-left differs (22px desktop, 18px
mobile) and the step dots already carry a per-breakpoint offset. The 6px
dot therefore landed 2px right of the 2px rail on desktop and 2px left of
it on mobile, which is the visible kink under an expanded last step.
Derive each offset from the rail's centre instead: the rail sits at
left:5px and is 2px wide, so the dot's left edge belongs at 3px, giving
3px - padding-left per breakpoint.
* fix(personal): run directory indexing off the event loop (#5558)
POST /api/personal/add_directory called rag.index_personal_documents
inline from an async handler, so the whole indexing job (os.walk, file
reads, per-chunk embedding, Chroma inserts) ran on the event loop and
every other request queued behind it. Indexing a real directory froze
the UI and API for 25+ minutes with no sign of life.
Move the blocking section into the threadpool via run_in_threadpool.
personal_docs_manager.add_directory stays inside it because its
refresh_index() re-extracts text across tracked directories, which is
also blocking work. A module-level lock serializes index jobs so the
threadpool move does not introduce parallel jobs racing
PersonalDocsManager's unsynchronized list mutations and file writes;
they previously serialized on the blocked loop, so one-at-a-time is
behavior parity.
* fix(personal): serialize add/remove/reload on an async job lock
The #5558 fix took the job lock INSIDE the threadpool worker and only on the
add path, so (1) remove_directory and /reload mutated PersonalDocsManager's
unsynchronized list/index concurrently with an in-flight add — the inconsistent
state the PR claimed to prevent — and (2) a queued add blocked on the lock while
holding an AnyIO threadpool token, starving the shared pool.
Move the lock to an asyncio.Lock acquired in the async handler BEFORE offloading,
and route add, remove and reload through it. A waiting request now parks on the
event loop instead of pinning a worker, and all three mutators are serialized so
the 'add/remove are serialized and cannot leave inconsistent state' guarantee
holds. remove and reload also run their blocking work off the event loop. The
lock is per-router so each app binds it to its own loop; single-process scope.
Tests: add-vs-remove and add-vs-reload serialization regressions (async via
ASGITransport, since asyncio.Lock deadlocks starlette TestClient's portal); the
existing add-vs-add test converted to the same driver.
* fix(personal): route upload and delete through the index job lock
/api/personal/upload and DELETE /api/personal/file mutated the same
vector and tracking state add/remove/reload serialize on, outside
_index_job_lock and inline on the event loop.
Both now stage async work on the loop, then run the complete transition
(vector writes, disk change, personal_docs_manager update) in one
offloaded critical section under the shared lock, acquired before the
offload so queued requests park on the loop rather than pinning a
threadpool worker.
Adds add-vs-upload and add-vs-file ordering regressions.
* fix(personal): bound multi-file upload memory
---------
Co-authored-by: RaresKeY <158580472+RaresKeY@users.noreply.github.com>
Classify built-in tool effects in a server-owned registry and carry run-local external-context integrity state through the agent loop and dispatcher. Block high-impact and unknown actions after successful external results, including same-batch calls, without relying on model compliance.
* docs(setup): document the HTTP/2 reverse-proxy setup
The "private or proxied deployments" section named Caddy, nginx and Traefik
but gave no runnable config, and never mentioned the main reason to bother:
the frontend is unbundled ES modules, so a page load is a few hundred small
same-origin requests. Over HTTP/1.1 the 6-connection cap serialises those
into dozens of round trips, which is invisible on localhost and dominates
load time over a LAN or VPN.
Adds a five-step setup you can paste: a Caddyfile for each of the three ways
people reach these boxes (public domain, Tailscale, own certificate), how to
run the proxy in the foreground and then as a service, the .env keys that
have to follow the origin, and a curl one-liner to confirm HTTP/2 actually
negotiated.
Also covers what bites when moving an existing install behind TLS:
SECURE_COOKIES applying regardless of the scheme the request arrived on,
OAUTH_REDIRECT_BASE_URL still defaulting to localhost because the MCP
redirect is registered up front rather than derived per request, and HSTS
being host-wide and port-agnostic. Notes that a custom HTTPS port does not
stop Caddy binding port 80 for the redirect, which is the failure I hit
first.
Docs only — no code change is needed to run behind HTTP/2 today.
* docs(setup): clarify HTTP/2 and origin migration
---------
Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
* fix(skill-importer): validate URL scheme and improve skills.sh handling
* fix(skill-importer): enhance DNS resolution and SSRF protection in fetch URL handling
* fix(url-safety): add allowed_dist parameter to check_outbound_url for flexible private blocking
* test(skill-importer): add comprehensive tests for URL parsing and outbound checks
* ensure newline at end of file in test_check_outbound_url_allows_public_ip
* fix(skill-importer): improve TLS certificate handling in _get_checked function
* fix(skill-importer): enhance _check_fetch_url to handle both hostnames and full URLs
* fix(skill-importer): enhance parse_skill_source to support skills.sh URLs in path and netloc
* fix(skill-importer): simplify skills.sh hostname check in parse_skill_source
* fix(skill-importer): enhance parse_skill_source to identify skills.sh URLs in path and handle localhost/IP addresses
* fix(skill-importer): enhance _resolve_and_check_url to validate all resolved IP addresses and prevent TOCTOU vulnerabilities
* fix(skill-importer): enhance parse_skill_source to support schemeless GitHub and skills.sh URLs
* fix(memory): resolve CodeQL URL sanitization warning and restore _check_fetch_url test alias
* fix(memory): pin skill fetch sockets without rewriting URLs
* fix(memory): reject unsupported skill wrapper hosts
* refactor(url-safety): remove unused importer exception
* test(memory): keep redirect regression hermetic
* test(dns-rebinding): add test for _PinnedTransport to ensure connection to pinned IP
* fix(skill-importer): enhance skills.sh support to extract GitHub links from page content
* fix(skill-importer): improve URL scheme validation for GitHub and skills.sh links
* fix(skills): reject unusable skill URLs instead of guessing
Resolving a skills.sh link by scraping the first github.com URL out of
the page body cannot work. Skill pages only ever link the repository
root, never the skill's subdirectory, so every skill in a repo resolved
to the same bundle: importing skills.sh/anthropics/skills/pdf walked the
whole monorepo, saturated the 64-file cap, and installed algorithmic-art
behind an ok:true response. Restore the redirect-target unwrap and fail
with a message that says what to do instead.
Also report the real reason a URL is rejected. The scheme check keyed off
"://" appearing anywhere in the string, so a supplied-but-unusable URL
came back as "URL is required", and a schemeless URL carrying "://" in
its query was reported as an unsupported scheme. Key off the parsed
scheme and let opaque schemes (mailto:, javascript:) and a schemeless
host:port fall through to the host check.
* test(skills): tighten the real-socket pinning regression
The handler swallowed its own exceptions, so a failure inside it
surfaced as a confusing assertion on the captured client address.
Record the exception and assert on it, run the thread as a daemon, and
close the listening socket from the test so a hang cannot outlive the
run. Also drop the duplicate ipaddress import and the missing newline.
* fix(skills): require exact GitHub skill URLs
* test(skills): read complete pinned request headers
---------
Co-authored-by: RaresKeY <158580472+RaresKeY@users.noreply.github.com>
Co-authored-by: Léo <leograndcontact@gmail.com>
* fix: stop polling GET /api/tasks/runs/recent from cancelling running tasks
Two paths caused the scheduler to interrupt a running background task
when the frontend Activity view polled for status:
1. GET /api/tasks/runs/recent was not in _PASSIVE_EXACT_PATHS, so
_InteractiveActivityMiddleware treated it as a foreground request
and called stop_background_tasks_for_foreground, cancelling any
in-flight scheduled task. Add it to _PASSIVE_EXACT_PATHS alongside
the other read-only polling endpoints.
2. The /api/activity/heartbeat handler called
stop_background_tasks_for_foreground unconditionally, ignoring
BACKGROUND_TASK_FOREGROUND_GATE=false. Wrap the call in a
_gate_enabled() guard so the env var fully disables heartbeat-
triggered cancellations.
Fixes#5782
Signed-off-by: Christian Sidak <christian@sentineltech.eu>
Signed-off-by: Christian-Sidak <61099993+Christian-Sidak@users.noreply.github.com>
* fix(scheduler): respect foreground gate for heartbeat
---------
Signed-off-by: Christian Sidak <christian@sentineltech.eu>
Signed-off-by: Christian-Sidak <61099993+Christian-Sidak@users.noreply.github.com>
Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
Background scheduled agent runs were aborted as "Stopped by user" when
the web UI was merely open, because the idle /api/email/unread-state
poll was counted as foreground activity while its sibling
/api/email/urgency-state was already excluded.
Fixes#5981
Co-authored-by: michaelxer <michaelxer@users.noreply.github.com>
* refactor(model-routing): centralize explicit foreground fallback policy
Make foreground fallback an explicit per-user, availability-only policy shared by streaming Chat, non-stream Chat, and Agent runs.
Preserve strict defaults, owner/model and credential boundaries, pinned Agent routes, and truthful per-round provenance/accounting. Carry provider-reported model identifiers through native streaming adapters, non-stream responses, and caches, and keep legacy default_model_fallbacks as tombstoned raw storage that generic settings APIs and agent tools cannot expose or mutate.
* fix(agent-loop): restore rebase-dropped qwen routing, workspace prompt, and temperature clamp
* fix(model-routing): thread selected endpoint identity, fix cost classification and fallback eligibility
* fix(chat): restore stream helpers and harden run stop lifecycle
* fix(model-routing): let numeric provider codes win over symbolic rate-limit statuses
* fix(agent-loop): apply qwen temperature and notes-tool clamps per fallback candidate
* fix(chat): honor queued stop across resend and reload canonical terminal on EOF
* fix(chat): track stop queue and cleanup ownership by per-send generation
* fix(agent-loop): preserve requested temperature for non-qwen fallback candidates
* fix(chat): reserve send ownership before any await and scope stop to the current send
* fix(chat): clear the previous run identity at send reservation
---------
Co-authored-by: RaresKeY <158580472+RaresKeY@users.noreply.github.com>
Co-authored-by: StressTestor <212606152+StressTestor@users.noreply.github.com>
* fix(cookbook): record real Windows pid for local serve so Stop kills the model
The Windows-local serve runner recorded Git Bash's `$$`, which is the
MSYS/Cygwin pid, not the Windows pid. Win32 tooling (taskkill,
Get-CimInstance ParentProcessId, Stop-Process) can't match an MSYS pid, so
the frontend Stop-Tree walk found nothing and the llama-server child survived
after Stop, leaving the model loaded and the GPU pinned.
Record the serving shell's true Win32 pid via `/proc/$$/winpid`, falling back
to the outer proc.pid already written from Python when the map is unavailable.
The existing pid-tracking test asserted the buggy `$$` literal at the source
level, so it passed while the feature was broken; update it to the winpid
behavior and add a focused regression test.
* fix(cookbook): make Windows serve pid handoff deterministic
---------
Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
* docs(setup): document supports_tools opt-in for manual Ollama /v1 endpoints
Manually-added Ollama /v1 endpoints default to the conservative
fenced-block tool-calling path, and there's currently no UI control to
opt a specific endpoint into native tool calling (#5192). The
supports_tools PATCH flag already exists and works, it just wasn't
documented anywhere a user could find it without reading source.
Adds a short section next to the existing "Ollama with Docker" notes
explaining when to use it and the exact API call, framed as an
advanced/opt-in setting per the maintainer's stated preference against
a casual UI toggle (#3195/#3438).
* docs(setup): clarify supports_tools false semantics
---------
Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
* fix(thinking): add deepseek-v4 to thinking model patterns
deepseek-v4-flash emits reasoning_content via the API but was not
recognized in _THINKING_MODEL_PATTERNS (only deepseek-r1 and
deepseek-reasoner were listed). Add the deepseek-v4 prefix so
the model is recognized as thinking-capable.
The between-round _thinkOpen leakage was separately fixed by
PR #5931 (perf(chat): batch live thinking rendering).
Related: #3998, #5931
* test(thinking): cover DeepSeek v4 detection
---------
Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
parse_tool_blocks fed <tool_call> wrapper bodies only to the XML
iterators (_iter_xml_invoke/_iter_xml_direct), so the canonical
Qwen/Hermes text-mode form — a bare JSON object like
{"name": "bash", "arguments": {"command": "..."}} inside the
wrapper — parsed to zero tool blocks and the agent never executed
anything. Pattern 4d only matches OpenAI-style blobs with a literal
"function" key, which the Hermes format lacks.
Wrapper bodies are now classified first: a JSON-looking body ({ or [)
is parsed by the new _parse_json_tool_call_body, which requires an
object with a string "name" and rejects a non-object "arguments"
instead of coercing it, then converts through the same
function_call_to_tool_block used by the XML paths so aliases and
per-tool argument formatting stay uniform. JSON-looking bodies fail
closed — they are never rescanned by the XML iterators (including the
unclosed-wrapper and bare-invoke fallbacks), so XML-like text inside
JSON argument values stays data instead of selecting a different tool.
Non-JSON bodies keep the existing XML path unchanged.
Fixes#5187
Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
gpt-oss (harmony) ships BUILT-IN tools named python/browser, invoked
with the raw body as the argument (to=python + bare source), while
custom functions use to=functions.NAME + JSON. Exposing our own tools
under those names makes the model answer with the built-in convention:
it emits raw code, the server parses it as JSON, and the request dies
with 'error parsing tool call: raw=import sys, ...'. In streaming mode
Ollama does not report it at all — it truncates the stream, so the turn
arrives as an empty response and the agent loop reads it as a model
stall. bash collides the same way in practice.
Measured on gpt-oss:20b via Ollama /v1, fixed agentic prompt, 12 runs
per arm: python+bash as-is 2/12, python renamed 10/12, both renamed
12/12. 74 HTTP 500s were logged server-side during investigation with
zero surfaced to the client.
Rename the colliding tools on the outbound payload and map the names
back on responses. Transport-only and gated on gpt-oss: every other
model's schemas pass through untouched (asserted in tests).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
* fix: stop stripping the word 'assistant' from rendered text
The QWEN_BARE_MARKER_RE regex in both the Python backend (tool_parsing.py)
and JS frontend (chatRenderer.js) was matching any standalone occurrence of
the word 'assistant' separated by any whitespace, then replacing it with a
space. This caused normal English uses like 'Home assistant' to render as
'Home '.
Fixed by narrowing the word-boundary check from [\t\r\n ] (any whitespace)
to [\r\n] (line boundaries only), so only Qwen-format role-token leaks
(where 'assistant' appears alone on a line) are stripped.
* fix(tests): update bare-marker test expectations for #5971
Move 'x assistant y' from STRIPPED to KEPT (mid-sentence must survive).
Add 'Before\nassistant\nAfter' to STRIPPED (bare-marker on own line).
* fix(ui): strip whitespace-padded assistant role markers
---------
Co-authored-by: samy <samy@users.noreply.github.com>
Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
* fix(sidebar): keep minimized icon rail in sync with per-tab visibility
Per-tab visibility (Customize UI / Appearance checkboxes, stored in
localStorage under `odysseus-ui-visibility`) was only applied to the full
sidebar elements — `UI_VIS_MAP` never targeted the collapsed `#icon-rail`
launchers. So a user who turned a tab off (e.g. Email) in the full view saw
every tab reappear when minimizing the sidebar to the icon rail.
Pair each tool/section selector with its `#rail-*` counterpart (mapping
mirrors `_railToolMap`), so `applyUIVis()` hides the rail launcher too.
Admin feature-flag handling is unaffected: the features-fetch reconcile at
app.js already re-applies `applyUIVis()`, so rail launchers now track admin
disables exactly like their sidebar buttons.
Adds a static regression test asserting every customizable tab pairs its
rail button.
Co-Authored-By: Claude <noreply@anthropic.com>
* refactor(sidebar): extract UI visibility into testable module
Move UI_VIS_MAP, UI_VIS_DEFAULT_OFF, and a pure resolveVisibility() into
static/js/ui_visibility.js so the icon-rail visibility rules are unit
testable without a DOM. app.js applies resolveVisibility() to the document,
replacing the ad-hoc tools-section override with an inline parent rule
(tools-section off hides every tool rail launcher). Add edge-case tests
covering per-tool off, the tools-section parent rule, parent+child combos,
email-section, and the tool-library <-> #rail-archive mapping.
Refs #5985
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
_emit_scalar quotes a frontmatter scalar with json.dumps when it holds
punctuation that would change how the line reads back. _parse_scalar undid that
with a bare raw[1:-1]: it stripped the quotes but never decoded the escapes. So
a description containing ü was written as the escape sequence \u00fc, read
back with that escape still sitting literally in the value, and re-escaped on
the next save. The backslash run doubles every save, so a non-English skill
description degrades into backslash noise after a few edits, and the escapes are
shown verbatim in the skills list and the /skills catalog.
This is not limited to non-ASCII. Any description containing a quote takes the
same path, since the quote is itself what forces the quoted form.
Make the two halves symmetric: emit with ensure_ascii=False, since SKILL.md is
UTF-8 at both ends (skills.py reads it, atomic_write_text writes it) and the
ASCII-escaped form bought nothing; and parse double-quoted scalars with
json.loads, falling back to the previous literal reading when the value is not
valid JSON. Files already corrupted heal one level per load.
ensure_ascii=False on its own would open a smaller hole. json.dumps escapes
every C0 control character but passes NEL, LINE SEPARATOR and PARAGRAPH
SEPARATOR through literally, and parse_frontmatter reads one scalar per line via
str.splitlines(), which breaks on all three. Re-escape those three, and add them
plus the remaining splitlines characters to the set that forces a quoted scalar,
so none of them can reach the file bare.
Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
The app font faces are declared in static/style.css, so the browser only
discovers FiraCode-Regular.woff2 and FiraCode-SemiBold.woff2 once the
stylesheet has parsed. On a cold load they start about 145 ms in, behind
the module graph. font-display: swap keeps that from blocking render, so
the cost is a visible swap rather than a stall, but the fetch can start
immediately instead.
Two preload hints move the request into the head. Measured cold on a
scratch instance with an empty cache, three runs per arm: request start
142-203 ms becomes 15-19 ms, response end 174-248 ms becomes 46-63 ms.
The total request count is unchanged and each face is still fetched
exactly once.
crossorigin is required even though these are same-origin: fonts are
always fetched in CORS mode, and without it the preload is discarded and
the font fetched again. Dropping the attribute produces four font entries
in the Resource Timing list instead of two.
Only Fira Code 400 and 600 are preloaded. They are the only faces first
paint uses. Inter, OpenDyslexic and Fira Code 300 stay unloaded on both
desktop and mobile, with or without a saved font preference.
llm_call_async returned raw list content for Mistral thinking models,
breaking callers that expect a str (e.g. auto-title). Match the sync
and streaming parsers by running list content through
_normalize_mistral_content.
Fixes#5435
Co-authored-by: michaelxer <michaelxer@users.noreply.github.com>
Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
_drawWhirlpool re-armed requestAnimationFrame forever whenever its element
had never been connected to the document. The grace period is there so a
spinner can keep drawing between start() and the caller appending the
element, but it had no deadline: while the element has never been connected
_wpWasConnected stays false, so the guard stays true and the else branch is
unreachable. Any caller that starts a spinner and then takes an early return,
such as an aborted request or a panel that resolved from cache, leaves a loop
redrawing an 84-segment spiral into a detached canvas at one frame per
displayed frame until the tab closes.
Put a 2 second deadline on the grace period. Callers append in the same task
as start(), so that is far more slack than any of them need. A spinner that
is actually in the document is unaffected.
Two supporting changes in the same file:
- Both self-terminate paths now call stop() instead of setting isRunning
directly, so termination always runs one cancelAnimationFrame and never
depends solely on inferring DOM connectivity. Both draw functions bail at
the top when they are no longer running, and _requestFrame() clears rafId
as the callback enters so it is a truthful "a frame is pending" flag.
- start() arms a visibilitychange listener and stop() removes it. A hidden
tab cancels the pending frame, a re-shown tab re-arms it. Chrome throttles
background rAF but does not reliably stop the canvas work, and owning the
listener from start/stop means a dead spinner never leaves one behind.
Adds tests/test_spinner_stops_when_never_attached_js.py, which drives the
real module under node with a fake clock and a manual frame pump. It covers
all four exits and, importantly, the converse: a spinner that is attached
keeps running well past the grace window.
Léo