feature/persona-fk-and-guard #90
Reference in New Issue
Block a user
Delete Branch "feature/persona-fk-and-guard"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two persona-infrastructure correctness fixes that go together because the second one (FK with CASCADE) requires the first (preventing the persona row from being mutated out from under its facts). 1. update_persona handler refuses name/systemPrompt edits to built-ins (409). includeAllMemories stays editable — that's a per-user preference, not the persona's identity. Mirrors the existing delete_persona guard. The DAO is intentionally permissive so the guard sits at the HTTP layer; persona_dao test pins that contract. 2. Migration 2026-05-10 adds user_id to entity_facts and a composite FK (user_id, persona_id) -> personas(user_id, persona_id) ON DELETE CASCADE. This closes two issues at once: - Persona orphans: deleting a custom persona used to leave its facts dangling forever, readable only via PersonaFilter::All. CASCADE now wipes them with the persona row. - Multi-user fact leakage: PersonaFilter::Single("default") used to surface every user's default-scoped facts. PersonaFilter is now { user_id, persona_id } and all read paths (get_facts_for_entity, list_facts, get_recent_activity) filter on user_id first. upsert_fact's dedup key extends to user_id so identical claims under shared persona names from different users no longer corroborate-bump each other's confidence. - user_id threads from Claims.sub.parse::<i32>().unwrap_or(1) at the chat / insight handlers through ChatTurnRequest, the streaming agentic loop, execute_tool, and into the leaf tools (tool_store_fact, tool_recall_facts_for_photo). The ".unwrap_or(1)" accommodates Apollo's service token whose sub is non-numeric on legacy mints. - Backfill picks the smallest user_id matching each legacy fact's persona_id so the FK holds for already-stored rows. Five new knowledge_dao tests with FK-on connection: persona scoping isolation, All-variant union per-user, dedup not crossing users, CASCADE delete, FK rejection of unknown personas. Plus dao_update_does_not_block_built_ins documenting where the HTTP-layer guard lives. Apollo coordinates separately — the matching changes there add the /api/personas proxy and start sending persona_id on photo-chat turns. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>When the LLM calls search_messages with { date, limit } and no query, it's making the predictable mistake of conflating the two "messages"-shaped tools. The previous behaviour returned an error that pointed it at get_sms_messages — correct, but burning a turn on the misroute. Long photo-chat threads where the user asks "what was happening that weekend?" hit this on small models roughly half the time. Now the date-string-without-query case transparently dispatches to get_sms_messages with the same args (date / limit / days_radius / contact name all pass through unchanged) and prepends a short "(Note: routed to get_sms_messages — prefer it directly next time)" to the result. The model sees real data on its first try while still learning the right tool for next time. Cases that don't have a get_sms_messages equivalent (numeric contact_id, or start_ts / end_ts windows) keep the original error so the model knows to either supply a query or restructure its call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>When a photo exists in more than one library and the user regenerates its insight from library A's chat, the regenerate streams cleanly, store_insight flips library A's old row to is_current=false, and inserts a new is_current=true row tagged (library A, rel_path). On the next history fetch the user sees their old transcript — the regenerate appears to vanish. The cause: get_insight(file_path) filters on rel_path + is_current only, so library B's untouched is_current=true row for the same rel_path satisfies the query and gets returned by SQLite's .first() ahead of A's new row. Because get_insight is also what chat_turn_stream uses to decide bootstrap vs. continuation, the next chat turn after the shadow hit also routes against the wrong insight, so update_training_messages corrupts library B's transcript with library A's chat. Fix: add get_current_insight_for_library(library_id, file_path) filtered on (library_id, rel_path, is_current=true) and route the chat surface (load_history, chat_turn{,_stream}, rewind_history) through it. load_history falls back to the cross-library get_insight when the scoped lookup misses — preserves the "scalar data merges across libraries" intent for the case where the active library has no insight but another does. The path-only get_insight stays for callers that don't have library context (populate_knowledge, the photo-grid metadata fetch). chat_history_handler stops dropping the parsed library on the floor and threads it through. Single-library deploys see no behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>