add_document() and add_documents_batch() derive the persistent ChromaDB
document id from Python's built-in hash():
doc_id = f"doc_{hash(text) % 10**16}"
str hashing is randomized per process (PYTHONHASHSEED is on by default), so
the same document text gets a different doc_id on every restart. The dedup
check right after — self._collection.get(ids=[doc_id]) — therefore misses
on restart, and identical documents are re-embedded and re-added as
duplicates each time the app restarts, bloating the vector store and
skewing retrieval.
Derive the id from a stable hashlib.sha256 of the text via a shared
_generate_doc_id() helper, used by both add paths so they agree.
tests/test_rag_vector_id_stability.py runs _generate_doc_id in subprocesses
under PYTHONHASHSEED=0/1/random and asserts the id is identical across all
of them (and differs for different text). Fails before this change.
analyze_topics() iterates session_manager.sessions and reads
session_data.get("history", []) directly. But SessionManager.load_sessions
seeds sessions metadata-only with empty history — messages are loaded
lazily, only when get_session(session_id) is called. So analyze_topics saw
empty history for every session that hadn't been individually opened this
process lifetime and reported total_topics: 0, even when the database held
plenty of matching messages.
Hydrate each candidate session via session_manager.get_session(session_id)
(the existing lazy-load path) before reading its history, after the
owner/archived filters so skipped sessions aren't loaded. Falls back to the
raw cached history when the manager has no get_session (test stubs).
tests/test_topic_analyzer.py: new test_topic_analyzer_hydrates_sessions
seeds a real SQLite DB with a session + message, runs the real
SessionManager (asserting cached history starts empty), then asserts
analyze_topics finds the topic. Fails before this change. The existing
keyword tests now pass an explicit owner to satisfy the owner-required
early return.
After a non-native tool round, the agent appends tool results as a {role:
'user'} message next to the user's original 'user' prompt, producing two
consecutive 'user' messages. Strict provider APIs (Anthropic/Claude) reject
consecutive same-role messages, so the follow-up generation request fails
silently — search returns sources, then nothing is generated.
_sanitize_llm_messages now merges consecutive 'user' messages (joining their
content). Only user/user is merged; normal chat and agent/tool turns already
alternate and are untouched.
Scoped down per maintainer review: the agent_loop 'output' source-extraction
change is already on main (#898/#901) and the broad-mocking web-sources test
was dropped. Added a focused test that runs consecutive-user messages through
the real _build_anthropic_payload and asserts the payload alternates correctly.
TTSService._put_cache writes .mp3 for MP3 audio (ID3/MPEG-framed bytes) and
.wav otherwise, and the rest of the class treats both as cache entries
(_get_cache iterates (".mp3", ".wav"); eviction globs "*.*"). But
get_stats() enumerated the cache with `glob("*.wav")` only, so both
cache_entries and cache_size_mb undercounted — reporting 0 whenever the
cache held MP3 files, which is the common case for most TTS providers.
Glob both extensions so the reported stats match what's actually cached.
tests/test_tts_cache_stats.py writes an MP3-headed blob via _put_cache and
asserts get_stats() reports one entry with non-zero size. Fails before this
change.
STTService._transcribe_local writes the audio to a NamedTemporaryFile
(delete=False) and only unlinks it on the success path, before the except.
If model.transcribe() raises (corrupt audio, model/runtime error, etc.) the
function logs, returns None, and leaves the .webm temp file behind — so
every failed local transcription leaks a file in the system temp dir.
Initialize tmp_path = None up front and move the unlink into a finally
block so the temp file is cleaned up whether transcription succeeds or
raises.
tests/test_stt_leak.py stubs the whisper model to raise during transcribe,
runs _transcribe_local, and asserts it returns None and leaves no new .webm
file in the temp dir. Fails before this change.
`strip_think` removes a dangling (unclosed) `<think>` block via
`_THINK_OPEN_RE`, but that pattern was anchored to the start of the string
(`^\s*<think>`). An unclosed `<think>` (or `<thinking>`) opener that
appears *after* any leading output was therefore only half-handled: the
stray tag itself was removed by `_THINK_TAG_RE`, but the reasoning content
following it leaked straight to the user.
strip_think("Hello! <think> I am thinking.") # -> "Hello! I am thinking." (leak)
strip_think("Sure.\n<think>\nLet me reconsider...") # -> leaks the reasoning
`strip_think` feeds user-facing output across research, email replies,
notes, and scheduled tasks, so this leaks chain-of-thought to end users.
Un-anchor `_THINK_OPEN_RE` so a dangling opener anywhere strips from the
opener to end of string, consistent with the existing start-of-string
behavior. Content before the opener, closed `<think>...</think>` blocks,
and tag-free text are all preserved.
tests/test_strip_think.py covers the mid-text leak (fails before this
change), start-anchored unclosed, closed blocks, no-tag passthrough,
content-before-opener, and mixed closed+unclosed. Full existing think
suite still passes.
* fix(db): enable SQLite foreign keys so ondelete cascades actually fire
core/database.py declares DB-level FK actions throughout
(ondelete="CASCADE" / "SET NULL"), but SQLite disables foreign-key
enforcement per connection by default and the engine had no connect-event
listener turning it on. So every one of those ondelete actions was dead.
Concrete impact: cleanup_old_sessions() in src/cleanup_service.py removes
old sessions with a bulk `query(Session).delete()`, which bypasses the
ORM-level relationship cascade and relies solely on the DB-level
ondelete="CASCADE" on ChatMessage.session_id. With foreign keys off, the
messages are never deleted — they pile up as orphaned rows on every
cleanup cycle.
Add the standard SQLAlchemy connect listener issuing `PRAGMA
foreign_keys=ON`, guarded by `isinstance(conn, sqlite3.Connection)` so it
only affects SQLite and leaves other backends untouched.
tests/test_sqlite_foreign_keys.py inserts a Session + ChatMessage, deletes
the Session via bulk `query().delete()`, and asserts the ChatMessage is
cascade-deleted. Fails before this change (orphan remains).
* docs(db): clarify FK pragma scope per review; trim test comments
Address review feedback on the foreign_keys PRAGMA change:
- Note that the class-level connect listener fires for every Engine in the
process and is a no-op on non-SQLite backends (isinstance guard).
- Warn near init_db() that FK enforcement is now global, so a migration
that temporarily violates FK constraints must disable foreign_keys around
that work.
- Drop the step-by-step narration comments from the regression test.
No behavior change.
The /api/admin/wipe/gallery branch deleted GalleryImage rows but left
every GalleryAlbum row behind (GalleryAlbum wasn't even imported). After
"wipe gallery" the user is left with orphaned, empty albums whose cover_id
points at now-deleted images — inconsistent with the other wipe branches,
which clear both parent and child tables.
Delete GalleryAlbum alongside GalleryImage and include both in the
returned count.
Adds tests/test_admin_wipe_gallery.py: seeds a real in-memory SQLite DB
with an album + image, runs the actual wipe handler, and asserts both
tables are emptied. Fails before this change (albums survive).
The image-edit endpoint lookup compared stored vs incoming base URLs with
`.rstrip("/v1")`. `str.rstrip(chars)` treats its argument as a character
set, not a suffix, so any URL ending in '/', 'v', or '1' is over-stripped
(e.g. `http://host1/v1` -> `http://host`). Two endpoints that are not the
same can then compare equal, or the real endpoint fails to match its own
stored record, leaving `api_key` unset and sending the upstream image call
unauthenticated.
Use `.removesuffix("/v1")` (exact-suffix removal) with surrounding
`.rstrip("/")` on both sides so only a genuine trailing `/v1` is dropped.
Adds a focused test that parses the actual comparison expression out of
gallery_routes.py via AST and evaluates it — it fails if the fix is
reverted and uses no mocking.
The DELETE /api/skills/{skill_id} handler resolves the caller, loads the
skill with skills_manager.load(owner=user), and verifies ownership with
_verify_owner(match, user) — but then calls
skills_manager.delete_skill(match.get("name")) without the owner.
SkillsManager.delete_skill filters candidates with
`(sk.owner or "") != (owner or "")`, so when owner is None an owner-scoped
skill is skipped and the method returns False. The route then raises a
spurious 404 "Skill not found" — meaning a logged-in user can never delete
their own skills through the API.
Pass the resolved owner through to delete_skill so the skill is matched and
removed.
tests/test_skills_delete_owner.py drops a real owner-scoped SKILL.md on disk
and (1) checks the manager directly: delete_skill without owner returns
False (regression lock) while delete_skill(owner="alice") returns True and
removes the dir; (2) drives the real DELETE route handler and asserts it
returns {"ok": True} and deletes the file. The route test fails before this
change (404). Real SkillsManager + real filesystem, no mocking.
APIKeyManager.load() decrypts every stored key with a dict comprehension
and no error handling. If the .key file no longer matches the ciphertext in
api_keys.json — key rotated, a partial/!mismatched data restore, or a
corrupted .key — Fernet.decrypt raises cryptography.fernet.InvalidToken.
app_initializer.py calls api_key_manager.load() during startup, so a single
undecryptable entry takes down the whole app at boot, and the user can't
reach the UI to fix it.
Decrypt each key in a loop and, on InvalidToken/ValueError, log a warning
and skip that one entry while still returning every key that decrypts
cleanly. One bad/stale key no longer blocks startup.
tests/test_api_key_manager_resilience.py saves a valid key, then injects an
entry encrypted under a different Fernet key (InvalidToken) and a malformed
token (ValueError), and asserts load() returns the good key and skips the
bad ones without raising. Fails before this change.
The webhook URL guard's _ip_is_private() only checks a hardcoded
_PRIVATE_NETWORKS list, which misses several addresses that route
internally. validate_webhook_url() therefore ALLOWED:
- http://[::]/ (IPv6 unspecified, reaches localhost)
- http://[::ffff:127.0.0.1]/ (IPv4-mapped IPv6 loopback = 127.0.0.1)
- http://[::ffff:169.254.169.254]/ (IPv4-mapped cloud metadata endpoint)
The last one is the dangerous case: a webhook pointed at the mapped
169.254.169.254 can pull cloud instance credentials (SSRF -> credential
theft).
Harden _ip_is_private(): first unwrap IPv4-mapped IPv6 to its embedded IPv4
(addr.ipv4_mapped), then reject via the stdlib address properties
(is_private, is_loopback, is_link_local, is_reserved, is_multicast,
is_unspecified) in addition to the existing network list. Public addresses
still pass.
tests/test_webhook_ssrf_resilience.py asserts validate_webhook_url raises
for the three IPv6 bypasses plus 127.0.0.1 and 0.0.0.0, and still accepts a
public IP literal. The IPv6 cases fail before this change.
Models (notably Gemini) emit a native 'google_search' function call, but the
agent loop had no mapping for it, so the call failed to convert, the round
produced 0 chars and 0 tool blocks, and generation died silently — the web
client hung on 'waiting for first token' with no error (also #443).
- Map google_search / google_search_retrieval / google_search_grounding to the
web_search tool, and read Gemini's 'queries' array (falling back to 'query').
- In stream_agent_loop, when a round yields no response text and no tool
events, emit a visible fallback message instead of leaving the user hanging.
- Give the unknown-tool execution branch an explicit exit_code=1 so the failure
is logged as an error rather than 'n/a'.
Unknown/unconvertible tool names still return None (unchanged) so they are
dropped safely rather than executed. Added tests covering the google_search
mapping, the queries array, and unknown/invalid-JSON returning None.
When the operator sets AUTH_ENABLED=false, three owner-scoped endpoints still
returned 401 (api/models, api/research/*, api/email/*), so the front-end
redirected the browser to /login and the app was unusable despite auth being
turned off. require_user() in src/auth_helpers.py already documents and honors
this contract (issue #622) via 'if _auth_disabled(): return ""', but these
endpoints did their own get_current_user/is_configured check without it.
Make _require_user (research), the /api/models anti-leak guard, and
email_helpers._require_auth consult _auth_disabled() and let anonymous through
(owner='') only when the operator explicitly disabled auth. The 401 protection
is fully intact when AUTH_ENABLED=true. Verified end-to-end: with
AUTH_ENABLED=false the SPA now loads instead of bouncing to /login.
The dependency-install fallback chain unconditionally ran
'pip install --user', which fails inside a virtualenv (and as root in
LXC/containers) with 'Can not perform a --user install. User site-packages
are not visible in this virtualenv.' — even though the function's docstring
already noted --user is invalid in venvs.
Guard the --user fallback with a venv check so it only runs outside a venv
(where --user is actually valid for PEP-668 system Pythons). Derive the venv
probe interpreter from the install command (python for 'pip', python3 for
'pip3'/'python3 -m pip') so the check runs in pip's own environment. System
PEP-668 installs keep the --user fallback; venv/LXC-root installs no longer
hit the --user error. Updated the unit test for the new chain.
Closes#388
When serving with the llama.cpp backend and no .gguf file exists on the host,
the GGUF launcher prelude exits with 'ERROR: No GGUF found on this host', but
_diagnose_serve_output had no matching pattern, so the UI showed a generic
crash instead of explaining the cause. Add a diagnosis pattern for the
no-GGUF case so users are told a .gguf is required and pointed at downloading
a GGUF build, instead of an opaque crash.
Closes#811
A Cookbook download whose repo/quant selector matched no files (e.g. a
':Q4_K_M' tag that does not exist) printed 'Fetching 0 files' and was still
reported as a successful '✓ Downloaded' / completed task. Detect the
zero-file signature in the download snapshot and mark the task as an error
with a clear diagnosis (no matching files — check the repo or quant/filename
pattern) so users know nothing was actually downloaded. Normal multi-file
and fully-cached downloads (which print 'Fetching N files', N>0) are
unaffected.
Closes#839
The Cookbook Scan/Download (hwfit) table gave the Fit column key:'score', so
clicking the Fit header sorted by score instead of by fit. Give the Fit column
its own 'fit' sort key, add a matching option to the #hwfit-sort select, and
rank fit_level (perfect > good > marginal > too_tight > no_fit) in the
client-side sort. Default puts the best fit first; clicking again reverses it.
Score still sorts by score.
Closes#842
Tatlatat