mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-08-14 15:19:26 +00:00
c8a012d4d2
Some checks failed
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript-typescript) (push) Has been cancelled
CodeQL / Analyze (python) (push) Has been cancelled
ci / docker publish / build (amd64) (push) Has been cancelled
ci / docker publish / build (arm64) (push) Has been cancelled
ci / docker publish / merge manifest + tag (push) Has been cancelled
* fix(memory): don't let an unreadable store get overwritten with an empty one
load_all() answered a failed read the same way it answered an empty store:
with []. Every mutation path is a read-modify-write (load the whole file,
change it, save it back), so a failed read became
load_all() -> [] -> [].append(new) -> save([new])
and save() is atomic, so the replacement stuck.
The case that actually destroys data is a store that is READABLE but not
parseable - a truncated file, or one holding {} instead of []. Nothing
obstructs the write, so adding a memory returns HTTP 200 and every memory
already stored is gone. Verified end-to-end against a running instance: on the
current code a truncated memory.json plus one add leaves the file holding only
the new entry. Truncation is reachable - core/database.py rewrites memory.json
during migration with a plain open(.., "w") + json.dump, which is not atomic.
A live exclusive lock is not the dangerous case: it blocks the read and the
os.replace alike, so the save fails too and the store survives. That path
currently 500s and loses nothing.
_read_entries() now returns [] only when the file genuinely does not exist and
raises MemoryStoreUnreadable for every other failure, including a store that
parses but is not a JSON array. load_all() keeps the old lenient behaviour so
display, search and context injection still degrade quietly instead of
breaking chat. The read-modify-write callers switch to load_all_for_update(),
which propagates the error: the memory routes turn it into a 503 and change
nothing, backup import refuses rather than saving only the incoming rows, and
auto-extraction and the audit merge skip the write. The audit merge mattered
most - it rebuilds the whole file from one owner's slice plus everyone else's
rows, so an empty read there dropped every other tenant's memories.
The corrupt-JSON path still gets its one shot at the legacy memory.txt
migration before raising, so that recovery is unchanged.
The two updated fakes gained load_all_for_update because the real class has it;
MagicMock would otherwise hand the import path a Mock instead of the seeded list.
Fixes #5673
* fix(memory): fail closed on the remaining read-modify-write add paths
The strict loader landed with the routes, the backup import and the extractor
converted, but three read-modify-write sinks still called load_all(), which
degrades an unreadable store to []. Two of them are the paths users actually
reach, so the data loss in #5673 stayed reproducible:
- src/ai_interaction.py do_manage_memory, action "add" — reached from ordinary
chat via src/tool_execution.py:793 -> dispatch_ai_tool. "Remember that I
prefer X" against an unreadable store wrote a one-entry file over it and
reported success.
- mcp_servers/memory_server.py, action "add" — the same shape through
_scope_entries(), registered as a built-in in src/builtin_mcp.py.
- src/memory_provider.py NativeMemoryProvider.remember and .delete — wired
into app state in src/app_initializer.py but not consumed outside tests yet,
converted here so the pattern is uniform before it goes live.
The MCP server takes _scope_entries(for_update=True) so list keeps the lenient
read. The edit and delete branches on both tool paths were already fail-closed
by accident — an empty view matches nothing and returns before the save — so
they are left alone.
The three new tests drive the real entry points rather than replaying the
shape, and use a truncated store, which is the case that reads back fine so
nothing stops the save. Each asserts memory.json is byte-identical afterwards;
all three fail on the previous commit with the store overwritten.
|
||
|---|---|---|
| .. | ||
| agent_tools | ||
| model_capability_readers | ||
| search | ||
| tools | ||
| action_intents.py | ||
| agent_loop.py | ||
| agent_runs.py | ||
| ai_interaction.py | ||
| api_key_manager.py | ||
| app_helpers.py | ||
| app_initializer.py | ||
| assistant_log.py | ||
| attachment_refs.py | ||
| auth_helpers.py | ||
| bg_jobs.py | ||
| bg_monitor.py | ||
| builtin_actions.py | ||
| builtin_mcp.py | ||
| caldav_sync.py | ||
| caldav_writeback.py | ||
| chat_handler.py | ||
| chat_helpers.py | ||
| chat_processor.py | ||
| chatgpt_subscription.py | ||
| chroma_client.py | ||
| cleanup_service.py | ||
| config.py | ||
| constants.py | ||
| context_budget.py | ||
| context_compactor.py | ||
| cookbook_serve_lifecycle.py | ||
| copilot.py | ||
| database.py | ||
| deep_research.py | ||
| document_actions.py | ||
| document_processor.py | ||
| email_thread_parser.py | ||
| embedding_lanes.py | ||
| embeddings.py | ||
| endpoint_resolver.py | ||
| event_bus.py | ||
| exceptions.py | ||
| generated_images.py | ||
| goal_based_extractor.py | ||
| host_docker_access.py | ||
| image_model_ids.py | ||
| index_walk.py | ||
| integrations.py | ||
| interactive_gate.py | ||
| llm_core.py | ||
| markitdown_runtime.py | ||
| mcp_manager.py | ||
| mcp_oauth.py | ||
| memory.py | ||
| memory_provider.py | ||
| memory_vector.py | ||
| model_capabilities.py | ||
| model_context.py | ||
| model_discovery.py | ||
| office_doc.py | ||
| optional_deps.py | ||
| pdf_form_doc.py | ||
| pdf_forms.py | ||
| pdf_runtime.py | ||
| personal_docs.py | ||
| preset_manager.py | ||
| prompt_security.py | ||
| rag_manager.py | ||
| rag_singleton.py | ||
| rag_vector.py | ||
| rate_limiter.py | ||
| readiness.py | ||
| reminder_personas.py | ||
| request_models.py | ||
| research_handler.py | ||
| research_utils.py | ||
| runtime_paths.py | ||
| secret_storage.py | ||
| service_health.py | ||
| session_actions.py | ||
| session_image_cleanup.py | ||
| session_search.py | ||
| settings.py | ||
| settings_scrub.py | ||
| task_action_policy.py | ||
| task_endpoint.py | ||
| task_scheduler.py | ||
| teacher_escalation.py | ||
| text_helpers.py | ||
| tls_overrides.py | ||
| tool_execution.py | ||
| tool_implementations.py | ||
| tool_index.py | ||
| tool_parsing.py | ||
| tool_policy.py | ||
| tool_schemas.py | ||
| tool_security.py | ||
| tool_utils.py | ||
| topic_analyzer.py | ||
| upload_handler.py | ||
| upload_limits.py | ||
| url_safety.py | ||
| url_security.py | ||
| user_time.py | ||
| visual_report.py | ||
| webhook_manager.py | ||
| youtube_handler.py | ||