From e70fe545cc85a6a3032481600793362f3a5d26e1 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 18:44:32 -0400 Subject: [PATCH 01/23] =?UTF-8?q?fix(trash):=20owner-scope=20all=20trash?= =?UTF-8?q?=20ops=20=E2=80=94=20close=20cross-tenant=20IDOR/disclosure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift-audit Group 1 (authz/IDOR). Multi-user is live, so these were exploitable ACL bypasses: - trash.py: add _owner_clause() and apply it to _exists_alive, restore, purge, list_trash, and purge_expired. A batch_id is a bearer token; without an owner predicate a leaked/guessed id let one tenant read (list_trash), restore, or PERMANENTLY purge another's content. Topics and rules carried no owner check at all (_OWNER mapped them to None) — ownership now derives through the parent rulebook (or owning project, for project-scoped rules). - purge_expired is now per-user; trash_scheduler iterates every user and applies that user's own trash_retention_days window, instead of applying user 1's window to everyone (early data loss for other users). - rulebooks subscribe/unsubscribe_project now assert project ownership, matching the suppression endpoints. - topic/rule DELETE routes return 404 when nothing owned was removed. Regression test locks in that every model — including topics/rules — gets a real owner clause. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/routes/rulebooks.py | 6 +- src/fabledassistant/services/rulebooks.py | 2 + src/fabledassistant/services/trash.py | 80 ++++++++++++++----- .../services/trash_scheduler.py | 28 ++++--- tests/test_services_trash.py | 27 ++++++- 5 files changed, 109 insertions(+), 34 deletions(-) diff --git a/src/fabledassistant/routes/rulebooks.py b/src/fabledassistant/routes/rulebooks.py index 4f63d76..5f7c394 100644 --- a/src/fabledassistant/routes/rulebooks.py +++ b/src/fabledassistant/routes/rulebooks.py @@ -118,7 +118,8 @@ async def update_topic(topic_id: int): @rulebooks_bp.delete("/rulebook-topics/") @login_required async def delete_topic(topic_id: int): - await trash_delete(_uid(), "topic", topic_id) + if await trash_delete(_uid(), "topic", topic_id) is None: + return jsonify({"error": "topic not found"}), 404 return "", 204 @@ -196,7 +197,8 @@ async def update_rule(rule_id: int): @rulebooks_bp.delete("/rules/") @login_required async def delete_rule(rule_id: int): - await trash_delete(_uid(), "rule", rule_id) + if await trash_delete(_uid(), "rule", rule_id) is None: + return jsonify({"error": "rule not found"}), 404 return "", 204 diff --git a/src/fabledassistant/services/rulebooks.py b/src/fabledassistant/services/rulebooks.py index 658f6f5..d8f1070 100644 --- a/src/fabledassistant/services/rulebooks.py +++ b/src/fabledassistant/services/rulebooks.py @@ -498,6 +498,7 @@ async def subscribe_project( from fabledassistant.models.rulebook import project_rulebook_subscriptions async with async_session() as session: + await _assert_project_owned(session, project_id, user_id) await _assert_rulebook_owned(session, rulebook_id, user_id) # ON CONFLICT DO NOTHING via try/except to keep dialect-agnostic. try: @@ -517,6 +518,7 @@ async def unsubscribe_project( from fabledassistant.models.rulebook import project_rulebook_subscriptions async with async_session() as session: + await _assert_project_owned(session, project_id, user_id) await _assert_rulebook_owned(session, rulebook_id, user_id) await session.execute( sql_delete(project_rulebook_subscriptions).where( diff --git a/src/fabledassistant/services/trash.py b/src/fabledassistant/services/trash.py index 92ab824..b7996f0 100644 --- a/src/fabledassistant/services/trash.py +++ b/src/fabledassistant/services/trash.py @@ -10,7 +10,7 @@ from __future__ import annotations import uuid from datetime import datetime, timezone -from sqlalchemy import select, update +from sqlalchemy import or_, select, update from fabledassistant.models import async_session from fabledassistant.models.note import Note @@ -19,19 +19,50 @@ from fabledassistant.models.project import Project from fabledassistant.models.milestone import Milestone from fabledassistant.models.rulebook import Rulebook, RulebookTopic, Rule -# entity_type -> (Model, owner_column_name or None). Used by the existence check. -_OWNER = { - "note": (Note, "user_id"), - "task": (Note, "user_id"), - "event": (Event, "user_id"), - "project": (Project, "user_id"), - "milestone": (Milestone, "user_id"), - "rulebook": (Rulebook, "owner_user_id"), - "topic": (RulebookTopic, None), - "rule": (Rule, None), +# entity_type -> Model. Used to resolve which table a trash op targets. +_MODEL_FOR = { + "note": Note, + "task": Note, + "event": Event, + "project": Project, + "milestone": Milestone, + "rulebook": Rulebook, + "topic": RulebookTopic, + "rule": Rule, } +def _owner_clause(model, user_id: int): + """Boolean expr scoping `model` rows to the ones `user_id` owns. + + EVERY trash query (exists-check, restore, purge, list, retention sweep) + must carry this — a batch_id is a bearer token, so without an owner + predicate a leaked/guessed id lets one tenant read, restore, or + permanently destroy another's content. Topics and rules carry no + user_id of their own; ownership is derived through the parent rulebook + (or, for project-scoped rules, the owning project). + """ + if model is Rulebook: + return Rulebook.owner_user_id == user_id + if model is RulebookTopic: + return RulebookTopic.rulebook_id.in_( + select(Rulebook.id).where(Rulebook.owner_user_id == user_id) + ) + if model is Rule: + return or_( + Rule.topic_id.in_( + select(RulebookTopic.id) + .join(Rulebook, RulebookTopic.rulebook_id == Rulebook.id) + .where(Rulebook.owner_user_id == user_id) + ), + Rule.project_id.in_( + select(Project.id).where(Project.user_id == user_id) + ), + ) + # Note, Event, Project, Milestone all carry user_id directly. + return model.user_id == user_id + + async def _set(session, model, where, batch, now) -> None: """Stamp deleted_at + batch on live rows matching `where`.""" await session.execute( @@ -42,10 +73,8 @@ async def _set(session, model, where, batch, now) -> None: async def _exists_alive(session, user_id: int, etype: str, eid: int) -> bool: - model, owner = _OWNER[etype] - where = [model.id == eid, model.deleted_at.is_(None)] - if owner: - where.append(getattr(model, owner) == user_id) + model = _MODEL_FOR[etype] + where = [model.id == eid, model.deleted_at.is_(None), _owner_clause(model, user_id)] return (await session.execute(select(model.id).where(*where))).first() is not None @@ -135,7 +164,7 @@ async def restore(user_id: int, batch_id: str) -> int: for model in _ALL: res = await session.execute( update(model) - .where(model.deleted_batch_id == batch_id) + .where(model.deleted_batch_id == batch_id, _owner_clause(model, user_id)) .values(deleted_at=None, deleted_batch_id=None) ) n += res.rowcount or 0 @@ -150,7 +179,9 @@ async def purge(user_id: int, batch_id: str) -> int: async with async_session() as session: for model in _ALL: res = await session.execute( - sql_delete(model).where(model.deleted_batch_id == batch_id) + sql_delete(model).where( + model.deleted_batch_id == batch_id, _owner_clause(model, user_id) + ) ) n += res.rowcount or 0 await session.commit() @@ -163,7 +194,9 @@ async def list_trash(user_id: int) -> list[dict]: async with async_session() as session: for model in _ALL: rows = (await session.execute( - select(model).where(model.deleted_at.isnot(None)) + select(model).where( + model.deleted_at.isnot(None), _owner_clause(model, user_id) + ) )).scalars().all() for r in rows: grp = batches.setdefault( @@ -185,9 +218,12 @@ async def list_trash(user_id: int) -> list[dict]: return out -async def purge_expired(retention_days: int) -> int: - """Cron entry: hard-delete rows trashed more than retention_days ago. +async def purge_expired(user_id: int, retention_days: int) -> int: + """Cron entry: hard-delete THIS user's rows trashed more than retention_days ago. + Scoped to one owner so the scheduler can apply each user's own + `trash_retention_days` window — a single global sweep would let one + user's short window prematurely destroy another's data. retention_days <= 0 disables auto-purge (returns 0 without touching anything). """ from datetime import timedelta @@ -200,7 +236,9 @@ async def purge_expired(retention_days: int) -> int: for model in _ALL: res = await session.execute( sql_delete(model).where( - model.deleted_at.isnot(None), model.deleted_at < cutoff + model.deleted_at.isnot(None), + model.deleted_at < cutoff, + _owner_clause(model, user_id), ) ) n += res.rowcount or 0 diff --git a/src/fabledassistant/services/trash_scheduler.py b/src/fabledassistant/services/trash_scheduler.py index 4273ca8..12a5ad8 100644 --- a/src/fabledassistant/services/trash_scheduler.py +++ b/src/fabledassistant/services/trash_scheduler.py @@ -1,9 +1,9 @@ """Daily APScheduler cron that purges expired trash. Mirrors version_pinning_scheduler.py: a single global BackgroundScheduler job -at 03:30 UTC bridges into the asyncio loop to run the async purge. Reads the -operator's `trash_retention_days` setting (single-tenant: user 1); 0 disables -auto-purge. +at 03:30 UTC bridges into the asyncio loop to run the async purge. Iterates +every user and applies that user's own `trash_retention_days` setting; 0 +disables auto-purge for that user. """ from __future__ import annotations @@ -30,12 +30,22 @@ def _run_purge_threadsafe() -> None: async def _runner(): try: - raw = await get_setting(1, "trash_retention_days", "90") - try: - days = int(raw) - except (TypeError, ValueError): - days = 90 - purged = await trash_svc.purge_expired(days) + from sqlalchemy import select + + from fabledassistant.models import async_session + from fabledassistant.models.user import User + + async with async_session() as session: + user_ids = (await session.execute(select(User.id))).scalars().all() + + purged = 0 + for uid in user_ids: + raw = await get_setting(uid, "trash_retention_days", "90") + try: + days = int(raw) + except (TypeError, ValueError): + days = 90 + purged += await trash_svc.purge_expired(uid, days) if purged: logger.info("trash purge: removed %d expired row(s)", purged) else: diff --git a/tests/test_services_trash.py b/tests/test_services_trash.py index 0586ec9..fd7f1f8 100644 --- a/tests/test_services_trash.py +++ b/tests/test_services_trash.py @@ -112,7 +112,7 @@ async def test_purge_expired_skips_when_retention_zero(): with patch("fabledassistant.services.trash.async_session") as cls: cls.return_value = session from fabledassistant.services.trash import purge_expired - n = await purge_expired(0) + n = await purge_expired(1, 0) assert n == 0 assert not session.execute.called # never opens a delete @@ -124,11 +124,34 @@ async def test_purge_expired_deletes_across_models_when_positive(): with patch("fabledassistant.services.trash.async_session") as cls: cls.return_value = session from fabledassistant.services.trash import purge_expired - n = await purge_expired(90) + n = await purge_expired(1, 90) assert n == 7 assert session.execute.await_count == 7 +def test_owner_clause_scopes_every_model(): + """Regression: every trash op must owner-scope, including topics/rules + which previously had NO owner check (IDOR across tenants).""" + from fabledassistant.services.trash import _owner_clause, _MODEL_FOR + from fabledassistant.models.note import Note + from fabledassistant.models.rulebook import Rulebook, RulebookTopic, Rule + + # Models with a direct owner column. + assert "user_id" in str(_owner_clause(Note, 7)) + assert "owner_user_id" in str(_owner_clause(Rulebook, 7)) + + # Topics/rules carry no user_id — ownership is derived through the + # parent rulebook (and, for rules, the owning project). + topic_sql = str(_owner_clause(RulebookTopic, 7)) + assert "rulebooks" in topic_sql and "owner_user_id" in topic_sql + rule_sql = str(_owner_clause(Rule, 7)) + assert "owner_user_id" in rule_sql and "projects" in rule_sql + + # Every entity type resolves to a model that yields a non-empty clause. + for model in set(_MODEL_FOR.values()): + assert _owner_clause(model, 1) is not None + + @pytest.mark.asyncio async def test_list_trash_groups_by_batch(): session = _make_mock_session() From 8b49ea896a21b65f89f8955126815097ce7a76c5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 18:47:54 -0400 Subject: [PATCH 02/23] fix(schedulers): wire recurring-task spawn + deliver event reminders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift-audit Group 2 (Phase-8 amputation — live wiring, no consumer): - Recurring tasks never recurred: spawn_recurring_tasks() had no caller. Register it as a 15-min interval job in the event scheduler (which app.py already starts/stops). Also add a deleted_at IS NULL guard to the spawn query in the same change, so a trashed recurring parent can never resurrect children once the sweep is live. - Event reminders were stamped reminder_sent_at but never delivered. _fire_reminders now creates an 'event_reminder' in-app notification before stamping, so a delivery failure stays retryable. Frontend NotificationsPanel renders the new type (⏰ + message); message logic pulled into a notifMessage() helper. - Remove the dead _fire_push_notif no-op stub (push left in Phase 8) and its three create_task call sites — no more throwaway tasks per share. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/components/NotificationsPanel.vue | 23 +++++-- .../services/event_scheduler.py | 61 +++++++++++++++---- src/fabledassistant/services/notifications.py | 9 --- src/fabledassistant/services/recurrence.py | 3 + 4 files changed, 71 insertions(+), 25 deletions(-) diff --git a/frontend/src/components/NotificationsPanel.vue b/frontend/src/components/NotificationsPanel.vue index 90fb9b5..ea5275b 100644 --- a/frontend/src/components/NotificationsPanel.vue +++ b/frontend/src/components/NotificationsPanel.vue @@ -13,6 +13,23 @@ const typeIcon: Record = { project_shared: '📁', note_shared: '📝', group_added: '👥', + event_reminder: '⏰', +} + +function notifMessage(n: { type: string; payload: Record }): string { + const p = n.payload + switch (n.type) { + case 'project_shared': + return ` shared "${p.project_title}" with you as ${p.permission}` + case 'note_shared': + return ` shared "${p.note_title}" with you as ${p.permission}` + case 'group_added': + return ` added you to "${p.group_name}" as ${p.role}` + case 'event_reminder': + return `Reminder: "${p.title}" is coming up` + default: + return '' + } } async function handleClick(notif: { id: number; payload: Record }) { @@ -49,11 +66,7 @@ onMounted(() => store.fetchAll())

{{ n.payload.invited_by }} - {{ n.type === 'project_shared' - ? ` shared "${n.payload.project_title}" with you as ${n.payload.permission}` - : n.type === 'note_shared' - ? ` shared "${n.payload.note_title}" with you as ${n.payload.permission}` - : ` added you to "${n.payload.group_name}" as ${n.payload.role}` }} + {{ notifMessage(n) }}

{{ relativeTime(n.created_at) }}
diff --git a/src/fabledassistant/services/event_scheduler.py b/src/fabledassistant/services/event_scheduler.py index b64bea3..25ef092 100644 --- a/src/fabledassistant/services/event_scheduler.py +++ b/src/fabledassistant/services/event_scheduler.py @@ -1,10 +1,12 @@ """Scheduler jobs for background maintenance tasks. -- Reminder notifications: checks every 5 minutes for due event reminders. +- Reminder notifications: checks every 5 minutes for due event reminders and + delivers them to the in-app notification feed. - CalDAV pull sync: runs every hour for all users with CalDAV configured. -- Chat retention cleanup: runs daily, deleting old conversations per user setting. +- Recurring-task spawn: every 15 minutes, creates the next occurrence of any + recurring task whose spawn time has arrived. -Uses the same BackgroundScheduler pattern as briefing_scheduler.py. +Uses the BackgroundScheduler pattern shared with the other *_scheduler modules. """ from __future__ import annotations @@ -55,19 +57,26 @@ async def _fire_reminders() -> None: if not to_notify: return + # Deliver via the in-app notification feed (push was removed in Phase 8). + from fabledassistant.services.notifications import create_in_app_notification + async with async_session() as session: for event in to_notify: - # Push delivery removed alongside the chat subsystem in Phase 8. - # Event reminders are still flagged via in-app notifications - # (see services/notifications.py). - - # Mark as sent regardless of push success to avoid re-firing result = await session.execute( select(Event).where(Event.id == event.id) ) ev = result.scalar_one_or_none() - if ev: - ev.reminder_sent_at = datetime.now(timezone.utc) + if ev is None or ev.reminder_sent_at is not None: + continue + await create_in_app_notification(ev.user_id, "event_reminder", { + "event_id": ev.id, + "title": ev.title, + "start_dt": ev.start_dt.isoformat() if ev.start_dt else None, + "url": "/calendar", + }) + # Stamp only after the notification is created, so a delivery + # failure leaves the reminder eligible to retry next sweep. + ev.reminder_sent_at = datetime.now(timezone.utc) await session.commit() @@ -91,6 +100,22 @@ def _run_caldav_sync_threadsafe(loop: asyncio.AbstractEventLoop) -> None: asyncio.run_coroutine_threadsafe(_run_caldav_sync(), loop) +# --------------------------------------------------------------------------- +# Recurring-task spawn job +# --------------------------------------------------------------------------- + +async def _run_recurrence_spawn() -> None: + from fabledassistant.services.recurrence import spawn_recurring_tasks # noqa: PLC0415 + try: + await spawn_recurring_tasks() + except Exception: + logger.warning("Recurring-task spawn job failed", exc_info=True) + + +def _run_recurrence_spawn_threadsafe(loop: asyncio.AbstractEventLoop) -> None: + asyncio.run_coroutine_threadsafe(_run_recurrence_spawn(), loop) + + # --------------------------------------------------------------------------- # Lifecycle # --------------------------------------------------------------------------- @@ -120,8 +145,22 @@ def start_event_scheduler(loop: asyncio.AbstractEventLoop) -> None: replace_existing=True, ) + # Spawn the next occurrence of due recurring tasks every 15 minutes. + # Without this job, recurrence_next_spawn_at is armed on completion but + # never drained, so recurring tasks never recur. + _scheduler.add_job( + _run_recurrence_spawn_threadsafe, + trigger=IntervalTrigger(minutes=15), + args=[loop], + id="recurrence_spawn", + replace_existing=True, + ) + _scheduler.start() - logger.info("Event scheduler started (reminders every 5m, CalDAV sync every 1h)") + logger.info( + "Event scheduler started (reminders every 5m, CalDAV sync every 1h, " + "recurring-task spawn every 15m)" + ) def stop_event_scheduler() -> None: diff --git a/src/fabledassistant/services/notifications.py b/src/fabledassistant/services/notifications.py index c8dec98..ac178ef 100644 --- a/src/fabledassistant/services/notifications.py +++ b/src/fabledassistant/services/notifications.py @@ -266,12 +266,6 @@ async def create_in_app_notification(user_id: int, notif_type: str, payload: dic return n -async def _fire_push_notif(user_id: int, title: str, body: str, url: str) -> None: - # Push delivery was removed alongside the chat subsystem (Phase 8). - # In-app notifications still flow through the bell-icon feed. - return None - - async def _fire_share_email(user_id: int, subject: str, body_text: str) -> None: try: if not await is_smtp_configured(): @@ -325,7 +319,6 @@ async def notify_project_shared( "invited_by": inviter.username, "url": url, }) - asyncio.create_task(_fire_push_notif(uid, "Project shared with you", msg, url)) asyncio.create_task(_fire_share_email(uid, f"[Fabled] {inviter.username} shared a project with you", msg)) @@ -361,7 +354,6 @@ async def notify_note_shared( "invited_by": inviter.username, "url": url, }) - asyncio.create_task(_fire_push_notif(uid, "Note shared with you", msg, url)) asyncio.create_task(_fire_share_email(uid, f"[Fabled] {inviter.username} shared a note with you", msg)) @@ -385,7 +377,6 @@ async def notify_group_added( "invited_by": inviter.username, "url": url, }) - asyncio.create_task(_fire_push_notif(target_user_id, "Added to a group", msg, url)) asyncio.create_task(_fire_share_email(target_user_id, "[Fabled] You've been added to a group", msg)) diff --git a/src/fabledassistant/services/recurrence.py b/src/fabledassistant/services/recurrence.py index b196df2..77f57b4 100644 --- a/src/fabledassistant/services/recurrence.py +++ b/src/fabledassistant/services/recurrence.py @@ -115,6 +115,9 @@ async def spawn_recurring_tasks() -> int: and_( Note.recurrence_rule.isnot(None), Note.recurrence_next_spawn_at <= now, + # Never spawn children off a trashed parent — that would + # resurrect work the user explicitly deleted. + Note.deleted_at.is_(None), ) ) ) From 5fe0fd126d626af29e0c8b59e51220294e92d544 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 18:51:40 -0400 Subject: [PATCH 03/23] fix(soft-delete): filter trashed rows across read/write paths Drift-audit Group 3 (soft-delete lifecycle gaps). Trashed rows were leaking into reads and being mutated/resurrected by writes: - update SELECTs now exclude trashed rows: update_milestone, update_project, update_event, and get_milestone_in_project (the latter backs all four milestone routes). Mutating a trashed row silently persisted and reappeared on restore. - MCP get_recent (notes/projects/events) and list_tags now filter deleted_at IS NULL, so trashed items stop surfacing in the agent's bootstrap context and tag counts. - convert_task_to_note clears recurrence_rule + recurrence_next_spawn_at so a demoted note can't spawn children via the (now-live) sweep. - caldav pull skips locally-trashed events (by caldav_uid) instead of resurrecting them via update or creating a duplicate live copy. - trash _cascade now stamps the FULL sub-task subtree (iterative descent), not just direct children, so deeply nested sub-tasks restore as one batch. Test updated for the new descent query. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/mcp/tools/recent.py | 9 ++++++--- src/fabledassistant/mcp/tools/tags.py | 2 +- src/fabledassistant/services/caldav_sync.py | 16 ++++++++++++---- src/fabledassistant/services/events.py | 5 ++++- src/fabledassistant/services/milestones.py | 6 +++++- src/fabledassistant/services/notes.py | 4 ++++ src/fabledassistant/services/projects.py | 5 ++++- src/fabledassistant/services/trash.py | 17 +++++++++++++++-- tests/test_services_trash.py | 9 ++++++--- 9 files changed, 57 insertions(+), 16 deletions(-) diff --git a/src/fabledassistant/mcp/tools/recent.py b/src/fabledassistant/mcp/tools/recent.py index 2d80aaf..dc2d1e9 100644 --- a/src/fabledassistant/mcp/tools/recent.py +++ b/src/fabledassistant/mcp/tools/recent.py @@ -38,7 +38,8 @@ async def get_recent(days: int = 7, limit: int = 25) -> dict: items: list[dict] = [] async with async_session() as session: notes = (await session.execute( - select(Note).where(Note.user_id == uid, Note.updated_at >= since) + select(Note).where(Note.user_id == uid, Note.updated_at >= since, + Note.deleted_at.is_(None)) .order_by(Note.updated_at.desc()).limit(limit) )).scalars().all() for n in notes: @@ -50,7 +51,8 @@ async def get_recent(days: int = 7, limit: int = 25) -> dict: }) projects = (await session.execute( select(Project).where(Project.user_id == uid, - Project.updated_at >= since) + Project.updated_at >= since, + Project.deleted_at.is_(None)) .order_by(Project.updated_at.desc()).limit(limit) )).scalars().all() for p in projects: @@ -62,7 +64,8 @@ async def get_recent(days: int = 7, limit: int = 25) -> dict: }) events = (await session.execute( select(Event).where(Event.user_id == uid, - Event.updated_at >= since) + Event.updated_at >= since, + Event.deleted_at.is_(None)) .order_by(Event.updated_at.desc()).limit(limit) )).scalars().all() for e in events: diff --git a/src/fabledassistant/mcp/tools/tags.py b/src/fabledassistant/mcp/tools/tags.py index 40f5f9b..1ea85f8 100644 --- a/src/fabledassistant/mcp/tools/tags.py +++ b/src/fabledassistant/mcp/tools/tags.py @@ -39,7 +39,7 @@ async def list_tags(limit: int = 50) -> dict: limit = max(1, min(limit, 200)) async with async_session() as session: result = await session.execute( - select(Note.tags).where(Note.user_id == uid) + select(Note.tags).where(Note.user_id == uid, Note.deleted_at.is_(None)) ) tag_lists = [row[0] for row in result.all()] counts = _aggregate_tag_counts(tag_lists) diff --git a/src/fabledassistant/services/caldav_sync.py b/src/fabledassistant/services/caldav_sync.py index f35ffee..60341fd 100644 --- a/src/fabledassistant/services/caldav_sync.py +++ b/src/fabledassistant/services/caldav_sync.py @@ -125,7 +125,7 @@ async def sync_user_events(user_id: int) -> dict: logger.warning("CalDAV pull sync failed for user %d", user_id, exc_info=True) return {"error": "CalDAV fetch failed"} - created = updated = unchanged = 0 + created = updated = unchanged = skipped = 0 async with async_session() as session: for ev in remote_events: @@ -149,6 +149,14 @@ async def sync_user_events(user_id: int) -> dict: ) existing = result.scalar_one_or_none() + if existing is not None and existing.deleted_at is not None: + # The user trashed this event locally. Don't resurrect it by + # updating, and don't create a duplicate live copy — leave it + # in the trash. (Propagating the delete to the remote server is + # tracked separately.) + skipped += 1 + continue + if existing is None: # Create new event new_ev = Event( @@ -180,10 +188,10 @@ async def sync_user_events(user_id: int) -> dict: await session.commit() logger.info( - "CalDAV sync user %d: %d created, %d updated, %d unchanged", - user_id, created, updated, unchanged, + "CalDAV sync user %d: %d created, %d updated, %d unchanged, %d skipped (trashed)", + user_id, created, updated, unchanged, skipped, ) - return {"created": created, "updated": updated, "unchanged": unchanged} + return {"created": created, "updated": updated, "unchanged": unchanged, "skipped": skipped} async def sync_all_users() -> None: diff --git a/src/fabledassistant/services/events.py b/src/fabledassistant/services/events.py index ce85545..590141a 100644 --- a/src/fabledassistant/services/events.py +++ b/src/fabledassistant/services/events.py @@ -261,7 +261,10 @@ async def update_event(user_id: int, event_id: int, **fields) -> Event | None: """ async with async_session() as session: result = await session.execute( - select(Event).where(Event.id == event_id, Event.user_id == user_id) + select(Event).where( + Event.id == event_id, Event.user_id == user_id, + Event.deleted_at.is_(None), + ) ) event = result.scalar_one_or_none() if event is None: diff --git a/src/fabledassistant/services/milestones.py b/src/fabledassistant/services/milestones.py index 3874a14..dd0c796 100644 --- a/src/fabledassistant/services/milestones.py +++ b/src/fabledassistant/services/milestones.py @@ -53,6 +53,7 @@ async def get_milestone_in_project(project_id: int, milestone_id: int) -> Milest select(Milestone).where( Milestone.id == milestone_id, Milestone.project_id == project_id, + Milestone.deleted_at.is_(None), ) ) return result.scalars().first() @@ -108,7 +109,10 @@ async def list_milestones( async def update_milestone(user_id: int, milestone_id: int, **fields: object) -> Milestone | None: async with async_session() as session: result = await session.execute( - select(Milestone).where(Milestone.id == milestone_id, Milestone.user_id == user_id) + select(Milestone).where( + Milestone.id == milestone_id, Milestone.user_id == user_id, + Milestone.deleted_at.is_(None), + ) ) milestone = result.scalars().first() if milestone is None: diff --git a/src/fabledassistant/services/notes.py b/src/fabledassistant/services/notes.py index 907d1da..c33b085 100644 --- a/src/fabledassistant/services/notes.py +++ b/src/fabledassistant/services/notes.py @@ -384,6 +384,10 @@ async def convert_task_to_note(user_id: int, note_id: int) -> Note: note.status = None note.priority = None note.due_date = None + # A plain note is not a task and must not recur — clear the rule and + # any armed spawn timestamp so the recurrence sweep never picks it up. + note.recurrence_rule = None + note.recurrence_next_spawn_at = None note.updated_at = datetime.now(timezone.utc) await session.commit() await session.refresh(note) diff --git a/src/fabledassistant/services/projects.py b/src/fabledassistant/services/projects.py index 74aabd9..477f65b 100644 --- a/src/fabledassistant/services/projects.py +++ b/src/fabledassistant/services/projects.py @@ -79,7 +79,10 @@ async def list_projects(user_id: int, status: str | None = None) -> list[Project async def update_project(user_id: int, project_id: int, **fields: object) -> Project | None: async with async_session() as session: result = await session.execute( - select(Project).where(Project.id == project_id, Project.user_id == user_id) + select(Project).where( + Project.id == project_id, Project.user_id == user_id, + Project.deleted_at.is_(None), + ) ) project = result.scalars().first() if project is None: diff --git a/src/fabledassistant/services/trash.py b/src/fabledassistant/services/trash.py index b7996f0..b0d068a 100644 --- a/src/fabledassistant/services/trash.py +++ b/src/fabledassistant/services/trash.py @@ -106,8 +106,21 @@ async def _cascade(session, user_id: int, etype: str, eid: int, batch: str, now) await _set(session, Note, [Note.user_id == user_id, Note.milestone_id == eid], batch, now) await _set(session, Milestone, [Milestone.user_id == user_id, Milestone.id == eid], batch, now) elif etype in ("note", "task"): - await _set(session, Note, [Note.user_id == user_id, Note.parent_id == eid], batch, now) # sub-tasks - await _set(session, Note, [Note.user_id == user_id, Note.id == eid], batch, now) + # Stamp the entire sub-task subtree (not just direct children) so a + # deeply nested task and all its descendants trash/restore as one batch. + ids = [eid] + frontier = [eid] + while frontier: + children = (await session.execute( + select(Note.id).where( + Note.user_id == user_id, + Note.parent_id.in_(frontier), + Note.deleted_at.is_(None), + ) + )).scalars().all() + frontier = [c for c in children if c not in ids] + ids.extend(frontier) + await _set(session, Note, [Note.user_id == user_id, Note.id.in_(ids)], batch, now) elif etype == "event": await _set(session, Event, [Event.user_id == user_id, Event.id == eid], batch, now) elif etype == "rulebook": diff --git a/tests/test_services_trash.py b/tests/test_services_trash.py index fd7f1f8..d218aa3 100644 --- a/tests/test_services_trash.py +++ b/tests/test_services_trash.py @@ -21,15 +21,18 @@ def _exists_result(found=True): @pytest.mark.asyncio async def test_delete_note_returns_batch_and_commits(): session = _make_mock_session() - # 1 execute for _exists_alive, then 2 for the note cascade (sub-tasks + self) - session.execute = AsyncMock(side_effect=[_exists_result(True), MagicMock(), MagicMock()]) + # exists-check, then the subtree descent: one child-lookup (no children + # here) + one _set stamping the whole subtree. + no_children = MagicMock() + no_children.scalars.return_value.all.return_value = [] + session.execute = AsyncMock(side_effect=[_exists_result(True), no_children, MagicMock()]) with patch("fabledassistant.services.trash.async_session") as cls: cls.return_value = session from fabledassistant.services.trash import delete batch = await delete(user_id=1, entity_type="note", entity_id=5) assert isinstance(batch, str) and len(batch) > 0 assert session.commit.called - # exists-check + 2 cascade updates + # exists-check + subtree-descent query + subtree _set assert session.execute.await_count == 3 From c363a5a6dfe58e20c869740f889809a000f60e0f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 18:55:23 -0400 Subject: [PATCH 04/23] fix(retention): add cleanup sweeps + CalDAV orphan reconciliation Drift-audit Group 4 (retention / unbounded growth): - CalDAV pull now reconciles deletions: a previously-synced event whose caldav_uid no longer appears remotely within the synced window is soft-deleted (one batch_id per run, restorable), so a remote delete propagates locally instead of orphaning forever. Guarded on a non-empty fetch so a spurious empty result can't wipe every local copy. Also wrap the blocking fetch in a 120s wait_for and log run duration. - Notifications: hourly loop now purges read notifications older than 30d (unread kept). Table no longer grows without bound. - Auth tokens: new daily sweep deletes password-reset / invitation tokens whose validity window ended >7d ago; wired via start_auth_token_retention_loop in app startup. Both tables previously only flipped used=True, never pruned. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/app.py | 2 + src/fabledassistant/services/auth.py | 43 ++++++++++++++++ src/fabledassistant/services/caldav_sync.py | 50 ++++++++++++++++--- src/fabledassistant/services/notifications.py | 28 +++++++++++ 4 files changed, 116 insertions(+), 7 deletions(-) diff --git a/src/fabledassistant/app.py b/src/fabledassistant/app.py index 8feb60c..40a7a15 100644 --- a/src/fabledassistant/app.py +++ b/src/fabledassistant/app.py @@ -148,12 +148,14 @@ def create_app() -> Quart: async def startup(): import asyncio + from fabledassistant.services.auth import start_auth_token_retention_loop from fabledassistant.services.embeddings import backfill_note_embeddings from fabledassistant.services.logging import start_log_retention_loop from fabledassistant.services.notifications import start_notification_loop start_log_retention_loop() start_notification_loop() + start_auth_token_retention_loop() # Backfill embeddings for any notes that don't have one. Runs in the # background so it never blocks the server from accepting requests. diff --git a/src/fabledassistant/services/auth.py b/src/fabledassistant/services/auth.py index 0605186..c94bb04 100644 --- a/src/fabledassistant/services/auth.py +++ b/src/fabledassistant/services/auth.py @@ -356,3 +356,46 @@ async def revoke_invitation(invitation_id: int) -> bool: await session.commit() logger.info("Invitation %d revoked", invitation_id) return True + + +# ── Token retention ───────────────────────────────────────────────────── +# Password-reset and invitation tokens are only ever flipped used=True and are +# never pruned, so on a long-lived instance both tables grow without bound. +# A daily sweep deletes any token whose validity window ended over grace_days +# ago (covers both used and naturally-expired rows once they're cold). + +_auth_retention_task = None + + +async def purge_expired_auth_tokens(grace_days: int = 7) -> int: + """Delete password-reset / invitation tokens that expired > grace_days ago.""" + from sqlalchemy import delete + cutoff = datetime.now(timezone.utc) - timedelta(days=grace_days) + removed = 0 + async with async_session() as session: + for model in (PasswordResetToken, InvitationToken): + result = await session.execute( + delete(model).where(model.expires_at < cutoff) + ) + removed += result.rowcount or 0 + await session.commit() + return removed + + +async def _auth_token_retention_loop() -> None: + import asyncio + while True: + await asyncio.sleep(86400) # daily + try: + removed = await purge_expired_auth_tokens() + if removed: + logger.info("Auth token retention: deleted %d expired token(s)", removed) + except Exception: + logger.exception("Error in auth token retention cleanup") + + +def start_auth_token_retention_loop() -> None: + global _auth_retention_task + import asyncio + if _auth_retention_task is None or _auth_retention_task.done(): + _auth_retention_task = asyncio.create_task(_auth_token_retention_loop()) diff --git a/src/fabledassistant/services/caldav_sync.py b/src/fabledassistant/services/caldav_sync.py index 60341fd..0c04919 100644 --- a/src/fabledassistant/services/caldav_sync.py +++ b/src/fabledassistant/services/caldav_sync.py @@ -11,7 +11,7 @@ import uuid from datetime import datetime, timedelta, timezone from typing import Any -from sqlalchemy import select +from sqlalchemy import select, update from fabledassistant.models import async_session from fabledassistant.models.event import Event @@ -20,6 +20,9 @@ logger = logging.getLogger(__name__) _SYNC_PAST_DAYS = 30 _SYNC_FUTURE_DAYS = 180 +# Wall-clock cap on the blocking CalDAV fetch so a hung/slow server can't +# wedge the hourly sweep indefinitely. +_SYNC_TIMEOUT_SECONDS = 120 def _parse_dt(val: Any) -> datetime | None: @@ -116,16 +119,24 @@ async def sync_user_events(user_id: int) -> dict: config = await get_caldav_config(user_id) + started = datetime.now(timezone.utc) + range_start = started - timedelta(days=_SYNC_PAST_DAYS) + range_end = started + timedelta(days=_SYNC_FUTURE_DAYS) + loop = asyncio.get_running_loop() try: - remote_events: list[dict] = await loop.run_in_executor( - None, _sync_one_user, config, user_id + remote_events: list[dict] = await asyncio.wait_for( + loop.run_in_executor(None, _sync_one_user, config, user_id), + timeout=_SYNC_TIMEOUT_SECONDS, ) + except asyncio.TimeoutError: + logger.warning("CalDAV pull sync timed out for user %d after %ds", user_id, _SYNC_TIMEOUT_SECONDS) + return {"error": "CalDAV fetch timed out"} except Exception: logger.warning("CalDAV pull sync failed for user %d", user_id, exc_info=True) return {"error": "CalDAV fetch failed"} - created = updated = unchanged = skipped = 0 + created = updated = unchanged = skipped = deleted = 0 async with async_session() as session: for ev in remote_events: @@ -185,13 +196,38 @@ async def sync_user_events(user_id: int) -> dict: else: unchanged += 1 + # Reconcile deletions: a previously-synced event (has a caldav_uid) + # that no longer appears remotely within the synced window is + # soft-deleted, so a delete on the remote propagates locally instead + # of orphaning forever. Guarded on a non-empty fetch so a spurious + # empty result can't wipe every local copy. + if remote_events: + remote_uids = {e["caldav_uid"] for e in remote_events} + orphan_batch = str(uuid.uuid4()) + orphan_res = await session.execute( + update(Event) + .where( + Event.user_id == user_id, + Event.caldav_uid.isnot(None), + Event.caldav_uid.notin_(remote_uids), + Event.deleted_at.is_(None), + Event.start_dt >= range_start, + Event.start_dt <= range_end, + ) + .values(deleted_at=datetime.now(timezone.utc), deleted_batch_id=orphan_batch) + ) + deleted = orphan_res.rowcount or 0 + await session.commit() + elapsed = (datetime.now(timezone.utc) - started).total_seconds() logger.info( - "CalDAV sync user %d: %d created, %d updated, %d unchanged, %d skipped (trashed)", - user_id, created, updated, unchanged, skipped, + "CalDAV sync user %d: %d created, %d updated, %d unchanged, %d skipped (trashed), " + "%d deleted (orphaned) in %.1fs", + user_id, created, updated, unchanged, skipped, deleted, elapsed, ) - return {"created": created, "updated": updated, "unchanged": unchanged, "skipped": skipped} + return {"created": created, "updated": updated, "unchanged": unchanged, + "skipped": skipped, "deleted": deleted} async def sync_all_users() -> None: diff --git a/src/fabledassistant/services/notifications.py b/src/fabledassistant/services/notifications.py index ac178ef..20a79c2 100644 --- a/src/fabledassistant/services/notifications.py +++ b/src/fabledassistant/services/notifications.py @@ -236,6 +236,28 @@ async def check_due_tasks() -> None: logger.exception("Failed to send task reminder for user %d", user_id) +# Read notifications are kept this long before the hourly sweep deletes them; +# unread are kept regardless. Without a sweep the table grows without bound. +_NOTIFICATION_RETENTION_DAYS = 30 + + +async def purge_old_read_notifications(retention_days: int = _NOTIFICATION_RETENTION_DAYS) -> int: + """Delete already-read in-app notifications older than retention_days.""" + from datetime import timedelta + from sqlalchemy import delete + from fabledassistant.models.notification import Notification + cutoff = datetime.now(timezone.utc) - timedelta(days=retention_days) + async with async_session() as session: + result = await session.execute( + delete(Notification).where( + Notification.read_at.isnot(None), + Notification.read_at < cutoff, + ) + ) + await session.commit() + return result.rowcount or 0 + + async def _notification_loop() -> None: while True: await asyncio.sleep(3600) # hourly @@ -243,6 +265,12 @@ async def _notification_loop() -> None: await check_due_tasks() except Exception: logger.exception("Error in notification loop") + try: + removed = await purge_old_read_notifications() + if removed: + logger.info("Notification retention: deleted %d read notification(s)", removed) + except Exception: + logger.exception("Error in notification retention cleanup") def start_notification_loop() -> None: From aef5009fc230a19b8e884b4532e8939f10775a4c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:02:19 -0400 Subject: [PATCH 05/23] fix(contract-drift): MCP read-only scope, shared-note writes, event TZ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift-audit Group 5 (high-severity contract drift): - MCP read-only keys could call every write tool: the Bearer resolver discarded api_key.scope and dispatch had no gate. Add resolve_bearer() (returns user_id + scope) and a scope gate in the /mcp ASGI wrapper that buffers the JSON-RPC body and rejects tools/call for any tool outside a read all-list when scope=='read' (default-deny for unknown/new tools). - Shared project notes/tasks panel was empty for non-owners: get_project_notes_route now queries notes/milestones with the project OWNER's uid (mirrors the already-fixed milestones route). - Shared editors couldn't save/delete shared NOTES (tasks worked): the three notes write routes now resolve via get_note_for_user, gate on can_write_note, and write as the owner — matching the tasks routes. - Event timezone drift: naive datetimes from the MCP date+time split are now localized to the user's tz at a single canonical service point (create_event /update_event), so MCP- and UI-created events agree. tz-aware inputs (REST/CalDAV) pass through untouched. - create_note validates status/priority (TaskStatus/TaskPriority), closing the MCP create_task path that let out-of-enum values persist (no DB CHECK). Tests cover resolve_bearer scope + the write-tool classifier. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/mcp/auth.py | 18 ++++++ src/fabledassistant/mcp/server.py | 87 +++++++++++++++++++++++++- src/fabledassistant/routes/notes.py | 33 ++++++++-- src/fabledassistant/routes/projects.py | 7 ++- src/fabledassistant/services/events.py | 25 ++++++++ src/fabledassistant/services/notes.py | 14 +++++ tests/test_mcp_auth.py | 44 +++++++++++++ 7 files changed, 218 insertions(+), 10 deletions(-) diff --git a/src/fabledassistant/mcp/auth.py b/src/fabledassistant/mcp/auth.py index 4cad787..ab1c3da 100644 --- a/src/fabledassistant/mcp/auth.py +++ b/src/fabledassistant/mcp/auth.py @@ -17,3 +17,21 @@ async def resolve_bearer_to_user_id(auth_header: str | None) -> int | None: return None api_key = await lookup_key(raw_token) return api_key.user_id if api_key else None + + +async def resolve_bearer(auth_header: str | None) -> tuple[int, str] | None: + """Resolve a Bearer token to (user_id, scope). + + scope is 'read' or 'write'. Returns None for a missing/malformed/invalid + token. The MCP dispatch layer uses scope to deny write-class tool calls + from read-only keys — the same read/write boundary the REST API enforces. + """ + if not auth_header or not auth_header.startswith("Bearer "): + return None + raw_token = auth_header[len("Bearer "):].strip() + if not raw_token: + return None + api_key = await lookup_key(raw_token) + if api_key is None: + return None + return api_key.user_id, (api_key.scope or "write") diff --git a/src/fabledassistant/mcp/server.py b/src/fabledassistant/mcp/server.py index 9b18199..388bbe1 100644 --- a/src/fabledassistant/mcp/server.py +++ b/src/fabledassistant/mcp/server.py @@ -81,6 +81,66 @@ auto-purges after the operator's retention window. """ +# Tools a read-only API key may call. Anything not listed is treated as a +# write for read keys (default-deny), so a newly-added tool is locked down +# until explicitly classified here. +_READ_ONLY_TOOLS = frozenset({ + "get_event", "get_note", "get_project", "get_rule", "get_rulebook", + "get_task", "get_recent", "enter_project", + "list_events", "list_lists", "list_milestones", "list_notes", + "list_persons", "list_places", "list_projects", "list_rulebooks", + "list_rules", "list_tags", "list_tasks", "list_topics", "list_trash", + "list_always_on_rules", "search", +}) + + +async def _buffer_request_body(receive): + """Drain the ASGI request body and return (body_bytes, replay_receive). + + The MCP sub-app still needs to read the body, so we return a fresh + `receive` that replays the buffered bytes. + """ + chunks: list[bytes] = [] + more = True + while more: + message = await receive() + if message["type"] == "http.request": + chunks.append(message.get("body", b"")) + more = message.get("more_body", False) + else: # http.disconnect + more = False + body = b"".join(chunks) + + sent = False + + async def replay(): + nonlocal sent + if not sent: + sent = True + return {"type": "http.request", "body": body, "more_body": False} + return {"type": "http.disconnect"} + + return body, replay + + +def _body_calls_write_tool(body: bytes) -> bool: + """True if the JSON-RPC body invokes a tool outside the read all-list.""" + import json + try: + payload = json.loads(body) + except Exception: + return False + items = payload if isinstance(payload, list) else [payload] + for item in items: + if not isinstance(item, dict): + continue + if item.get("method") == "tools/call": + name = (item.get("params") or {}).get("name", "") + if name and name not in _READ_ONLY_TOOLS: + return True + return False + + def build_mcp_server() -> FastMCP: """Build the FastMCP instance with all tools registered. @@ -128,7 +188,7 @@ def mount_mcp(app: Quart) -> None: inside Quart, we hook the session manager's `run()` async context manager into Quart's serving lifecycle (before_serving / after_serving). """ - from fabledassistant.mcp.auth import resolve_bearer_to_user_id + from fabledassistant.mcp.auth import resolve_bearer mcp = build_mcp_server() mcp_asgi = mcp.streamable_http_app() @@ -151,8 +211,8 @@ def mount_mcp(app: Quart) -> None: return await mcp_asgi(scope, receive, send) # ASGI headers are lowercase bytes per spec; lowercase explicitly to be safe. headers = {k.decode().lower(): v.decode() for k, v in scope.get("headers", [])} - user_id = await resolve_bearer_to_user_id(headers.get("authorization")) - if user_id is None: + resolved = await resolve_bearer(headers.get("authorization")) + if resolved is None: await send({ "type": "http.response.start", "status": 401, @@ -166,6 +226,27 @@ def mount_mcp(app: Quart) -> None: "body": b'{"error":"unauthorized"}', }) return + user_id, key_scope = resolved + + # Enforce read-only keys: REST blocks non-GET for scope='read', and the + # MCP surface must match or the read-only guarantee is void. A tool call + # arrives as a JSON-RPC POST; buffer the body, and if it invokes a tool + # outside the read all-list, reject before dispatch. (default-deny: any + # unknown/new tool is treated as a write for read keys.) + if key_scope == "read" and scope.get("method") == "POST": + body, receive = await _buffer_request_body(receive) + if _body_calls_write_tool(body): + await send({ + "type": "http.response.start", + "status": 403, + "headers": [(b"content-type", b"application/json")], + }) + await send({ + "type": "http.response.body", + "body": b'{"error":"read-only API key cannot call write tools"}', + }) + return + scope["scribe_user_id"] = user_id from fabledassistant.mcp._context import _user_id_ctx token = _user_id_ctx.set(user_id) diff --git a/src/fabledassistant/routes/notes.py b/src/fabledassistant/routes/notes.py index acac8c2..7b56d4d 100644 --- a/src/fabledassistant/routes/notes.py +++ b/src/fabledassistant/routes/notes.py @@ -8,6 +8,7 @@ from quart import Blueprint, jsonify, request from fabledassistant.auth import login_required, get_current_user_id from fabledassistant.routes.utils import not_found, parse_iso_date, parse_pagination +from fabledassistant.services.access import can_write_note from fabledassistant.services.notes import ( build_note_graph, convert_note_to_task, @@ -192,6 +193,15 @@ async def get_note_route(note_id: int): @login_required async def update_note_route(note_id: int): uid = get_current_user_id() + # Share-aware: resolve through the ACL and write as the OWNER, so a shared + # editor's save isn't rejected by the owner-scoped update service. + result = await get_note_for_user(uid, note_id) + if result is None: + return not_found("Note") + note_obj, _ = result + if not await can_write_note(uid, note_id): + return jsonify({"error": "Permission denied"}), 403 + owner_uid = note_obj.user_id data = await request.get_json() fields = {} for key in ("title", "body", "description", "parent_id", "project_id", "milestone_id", "status", "priority", "note_type"): @@ -212,14 +222,14 @@ async def update_note_route(note_id: int): if "tags" in data: fields["tags"] = data["tags"] try: - note = await update_note(uid, note_id, **fields) + note = await update_note(owner_uid, note_id, **fields) except ValueError as e: return jsonify({"error": str(e)}), 400 if note is None: return not_found("Note") text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "") if text: - asyncio.create_task(upsert_note_embedding(note.id, uid, text)) + asyncio.create_task(upsert_note_embedding(note.id, owner_uid, text)) return jsonify(note.to_dict()) @@ -227,6 +237,13 @@ async def update_note_route(note_id: int): @login_required async def patch_note_route(note_id: int): uid = get_current_user_id() + result = await get_note_for_user(uid, note_id) + if result is None: + return not_found("Note") + note_obj, _ = result + if not await can_write_note(uid, note_id): + return jsonify({"error": "Permission denied"}), 403 + owner_uid = note_obj.user_id data = await request.get_json() fields = {} for key in ("title", "body", "description", "parent_id", "project_id", "milestone_id", "status", "priority", "note_type"): @@ -245,14 +262,14 @@ async def patch_note_route(note_id: int): if "tags" in data: fields["tags"] = data["tags"] try: - note = await update_note(uid, note_id, **fields) + note = await update_note(owner_uid, note_id, **fields) except ValueError as e: return jsonify({"error": str(e)}), 400 if note is None: return not_found("Note") text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "") if text: - asyncio.create_task(upsert_note_embedding(note.id, uid, text)) + asyncio.create_task(upsert_note_embedding(note.id, owner_uid, text)) return jsonify(note.to_dict()) @@ -260,8 +277,14 @@ async def patch_note_route(note_id: int): @login_required async def delete_note_route(note_id: int): uid = get_current_user_id() + result = await get_note_for_user(uid, note_id) + if result is None: + return not_found("Note") + note_obj, _ = result + if not await can_write_note(uid, note_id): + return jsonify({"error": "Permission denied"}), 403 from fabledassistant.services.trash import delete as trash_delete - batch = await trash_delete(uid, "note", note_id) + batch = await trash_delete(note_obj.user_id, "note", note_id) if batch is None: return not_found("Note") return "", 204 diff --git a/src/fabledassistant/routes/projects.py b/src/fabledassistant/routes/projects.py index c90ec66..dd10f9d 100644 --- a/src/fabledassistant/routes/projects.py +++ b/src/fabledassistant/routes/projects.py @@ -116,6 +116,9 @@ async def get_project_notes_route(project_id: int): if result is None: return not_found("Project") project, _ = result + # Use the project owner's uid so the ownership filter on notes/milestones + # matches for shared collaborators (who'd otherwise see an empty panel). + owner_uid = project.user_id or uid # type filter: "note", "task", or None (both) type_filter = request.args.get("type") @@ -128,11 +131,11 @@ async def get_project_notes_route(project_id: int): elif type_filter == "note": is_task = False - ms_list = await list_milestones(uid, project_id) + ms_list = await list_milestones(owner_uid, project_id) milestone_ids = [m.id for m in ms_list] notes, total = await list_notes( - uid, + owner_uid, is_task=is_task, status=status_filter, project_id=project_id, diff --git a/src/fabledassistant/services/events.py b/src/fabledassistant/services/events.py index 590141a..ddd94c6 100644 --- a/src/fabledassistant/services/events.py +++ b/src/fabledassistant/services/events.py @@ -68,6 +68,19 @@ def _normalize_duration( return None +async def _localize_naive(user_id: int, dt: datetime | None) -> datetime | None: + """Anchor a naive datetime in the user's timezone; pass tz-aware through. + + Naive datetimes are the user's local wall-clock time (the MCP create/update + tools combine date+time without a zone). Attaching the user's tzinfo lets + asyncpg store the correct UTC instant, matching the REST/UI path. + """ + if dt is not None and dt.tzinfo is None: + from fabledassistant.services.tz import get_user_tz # noqa: PLC0415 + return dt.replace(tzinfo=await get_user_tz(user_id)) + return dt + + async def create_event( user_id: int, title: str, @@ -97,6 +110,13 @@ async def create_event( """ if duration is not None and duration_minutes is None: duration_minutes = duration + # Canonical localization point: a naive datetime (e.g. from the MCP tool's + # date+time split) is the user's wall-clock time, so anchor it in their + # timezone before storage. tz-aware inputs (REST, CalDAV pass-through) are + # left untouched. Without this, MCP-created events landed at the same + # wall-clock numerals in UTC and drifted from UI-created ones by the offset. + start_dt = await _localize_naive(user_id, start_dt) + end_dt = await _localize_naive(user_id, end_dt) duration_minutes = _normalize_duration( start_dt=start_dt, end_dt=end_dt, duration_minutes=duration_minutes, ) @@ -271,6 +291,11 @@ async def update_event(user_id: int, event_id: int, **fields) -> Event | None: return None old_title = event.title # capture before mutation for CalDAV lookup + # Localize a naive start_dt patch to the user's timezone (same canonical + # rule as create_event) before it's used or persisted. + if fields.get("start_dt") is not None: + fields["start_dt"] = await _localize_naive(user_id, fields["start_dt"]) + # Resolve any end_dt/duration_minutes inputs against the # post-update start_dt. If neither is in the patch, leave the # existing duration_minutes alone. diff --git a/src/fabledassistant/services/notes.py b/src/fabledassistant/services/notes.py index c33b085..90ee994 100644 --- a/src/fabledassistant/services/notes.py +++ b/src/fabledassistant/services/notes.py @@ -66,6 +66,20 @@ async def create_note( entity_meta: dict | None = None, task_kind: str = "work", ) -> Note: + # Validate status/priority here so the MCP create_task path (which passes + # them straight through) can't persist an out-of-enum value that the REST + # route would have rejected — there's no DB CHECK on notes.status. + if isinstance(status, str): + try: + status = TaskStatus(status).value + except ValueError: + raise ValueError(f"Invalid status: {status!r}. Must be one of: {[s.value for s in TaskStatus]}") + if isinstance(priority, str): + try: + priority = TaskPriority(priority).value + except ValueError: + raise ValueError(f"Invalid priority: {priority!r}. Must be one of: {[p.value for p in TaskPriority]}") + # Auto-populate project_id from milestone when not explicitly provided if milestone_id is not None and project_id is None: from fabledassistant.models.milestone import Milestone diff --git a/tests/test_mcp_auth.py b/tests/test_mcp_auth.py index 8579058..cf2493d 100644 --- a/tests/test_mcp_auth.py +++ b/tests/test_mcp_auth.py @@ -49,3 +49,47 @@ async def test_resolve_bearer_calls_lookup_with_stripped_token(): with patch("fabledassistant.mcp.auth.lookup_key", mock_lookup): await resolve_bearer_to_user_id("Bearer fmcp_abc123 ") mock_lookup.assert_awaited_once_with("fmcp_abc123") + + +# ── resolve_bearer (user_id + scope) ──────────────────────────────────── + +@pytest.mark.asyncio +async def test_resolve_bearer_returns_user_id_and_scope(): + from fabledassistant.mcp.auth import resolve_bearer + fake_key = MagicMock() + fake_key.user_id = 9 + fake_key.scope = "read" + with patch("fabledassistant.mcp.auth.lookup_key", AsyncMock(return_value=fake_key)): + assert await resolve_bearer("Bearer fmcp_x") == (9, "read") + + +@pytest.mark.asyncio +async def test_resolve_bearer_none_for_invalid(): + with patch("fabledassistant.mcp.auth.lookup_key", AsyncMock(return_value=None)): + assert await resolve_bearer("Bearer nope") is None + assert await resolve_bearer(None) is None + + +# ── read-only scope gate ──────────────────────────────────────────────── + +def test_body_calls_write_tool_classifies_correctly(): + import json + from fabledassistant.mcp.server import _body_calls_write_tool + + def call(name): + return json.dumps({"jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": {"name": name, "arguments": {}}}).encode() + + # Write-class tools are gated. + assert _body_calls_write_tool(call("create_note")) is True + assert _body_calls_write_tool(call("delete_project")) is True + assert _body_calls_write_tool(call("purge_trash")) is True + # An unknown/new tool defaults to write (default-deny for read keys). + assert _body_calls_write_tool(call("brand_new_tool")) is True + # Read tools and non-call methods pass. + assert _body_calls_write_tool(call("list_notes")) is False + assert _body_calls_write_tool(call("get_recent")) is False + assert _body_calls_write_tool( + json.dumps({"method": "tools/list"}).encode() + ) is False + assert _body_calls_write_tool(b"not json") is False From 4a220db513db184965fad051cf9e5fe81582cb3d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:12:14 -0400 Subject: [PATCH 06/23] test(mcp): import resolve_bearer at module level CI fix for aef5009: test_resolve_bearer_none_for_invalid referenced resolve_bearer but the import lived inside an earlier test only. Hoist it to the module import. Production code unaffected (1 failed / 284 passed). Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_mcp_auth.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_mcp_auth.py b/tests/test_mcp_auth.py index cf2493d..2f75b7c 100644 --- a/tests/test_mcp_auth.py +++ b/tests/test_mcp_auth.py @@ -3,7 +3,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest -from fabledassistant.mcp.auth import resolve_bearer_to_user_id +from fabledassistant.mcp.auth import resolve_bearer, resolve_bearer_to_user_id @pytest.mark.asyncio @@ -55,7 +55,6 @@ async def test_resolve_bearer_calls_lookup_with_stripped_token(): @pytest.mark.asyncio async def test_resolve_bearer_returns_user_id_and_scope(): - from fabledassistant.mcp.auth import resolve_bearer fake_key = MagicMock() fake_key.user_id = 9 fake_key.scope = "read" From c016bd664e5ecc0fd35d1adbcabb9343b99a87dc Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:16:45 -0400 Subject: [PATCH 07/23] fix(status-enum): add paused to ProjectStatus, validate, fix progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift-audit Group 6 + Group 5 #6 (enum-extension / status drift): - ProjectStatus gains 'paused' — routes and frontend already treated it as first-class, but the enum (the source of truth) omitted it and the error strings lied. A future CHECK derived from the enum would have rejected existing paused rows. - create_project/update_project now validate status via ProjectStatus at the service layer (canonical gate; notes.status has no DB CHECK), so the MCP create/update_project path can't persist a typo'd status. MCP docstrings realigned to the 4-value domain; route error strings corrected. - get_milestone_progress: cancelled tasks are excluded from the percent denominator (and now reported in status_counts), so a milestone whose only open task was cancelled reaches 100% instead of stalling below it. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/mcp/tools/projects.py | 6 +++--- src/fabledassistant/models/project.py | 1 + src/fabledassistant/routes/projects.py | 4 ++-- src/fabledassistant/services/milestones.py | 8 +++++++- src/fabledassistant/services/projects.py | 19 ++++++++++++++++++- 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/fabledassistant/mcp/tools/projects.py b/src/fabledassistant/mcp/tools/projects.py index 422a11a..73e1e28 100644 --- a/src/fabledassistant/mcp/tools/projects.py +++ b/src/fabledassistant/mcp/tools/projects.py @@ -27,7 +27,7 @@ from fabledassistant.services import trash as trash_svc async def list_projects() -> dict: """List all Scribe projects for the current user. - Returns id, title, description, goal, status (active/archived), color, + Returns id, title, description, goal, status (active/paused/completed/archived), color, and a short auto-generated summary for each project. """ uid = current_user_id() @@ -142,7 +142,7 @@ async def create_project( title: Project name (required). description: Short summary of what the project is. goal: The desired outcome or definition of done for the project. - status: active (default) or archived. + status: one of active (default), paused, completed, archived. color: Optional hex colour for the project card (e.g. "#6366f1"). """ uid = current_user_id() @@ -172,7 +172,7 @@ async def update_project( title: New title, or omit to leave unchanged. description: New description, or omit to leave unchanged. goal: New goal/definition-of-done, or omit to leave unchanged. - status: New status — active or archived. + status: New status — one of active, paused, completed, archived. color: New hex colour, or omit to leave unchanged. """ uid = current_user_id() diff --git a/src/fabledassistant/models/project.py b/src/fabledassistant/models/project.py index cfdf900..ccf4db5 100644 --- a/src/fabledassistant/models/project.py +++ b/src/fabledassistant/models/project.py @@ -8,6 +8,7 @@ from fabledassistant.models.base import TimestampMixin, SoftDeleteMixin class ProjectStatus(str, enum.Enum): active = "active" + paused = "paused" completed = "completed" archived = "archived" diff --git a/src/fabledassistant/routes/projects.py b/src/fabledassistant/routes/projects.py index dd10f9d..f1c65e5 100644 --- a/src/fabledassistant/routes/projects.py +++ b/src/fabledassistant/routes/projects.py @@ -53,7 +53,7 @@ async def create_project_route(): return jsonify({"error": "title is required"}), 400 status = data.get("status", "active") if status not in ("active", "paused", "completed", "archived"): - return jsonify({"error": "status must be 'active', 'completed', or 'archived'"}), 400 + return jsonify({"error": "status must be 'active', 'paused', 'completed', or 'archived'"}), 400 project = await create_project( uid, title=data["title"], @@ -90,7 +90,7 @@ async def update_project_route(project_id: int): allowed = {"title", "description", "goal", "status", "color"} fields = {k: (v if v is not None else "") for k, v in data.items() if k in allowed} if "status" in fields and fields["status"] not in ("active", "paused", "completed", "archived"): - return jsonify({"error": "status must be 'active', 'completed', or 'archived'"}), 400 + return jsonify({"error": "status must be 'active', 'paused', 'completed', or 'archived'"}), 400 project = await update_project(uid, project_id, **fields) if project is None: return not_found("Project") diff --git a/src/fabledassistant/services/milestones.py b/src/fabledassistant/services/milestones.py index dd0c796..fab1b98 100644 --- a/src/fabledassistant/services/milestones.py +++ b/src/fabledassistant/services/milestones.py @@ -155,8 +155,13 @@ async def get_milestone_progress(milestone_id: int) -> dict: status_counts[status] = count total = sum(status_counts.values()) + cancelled = status_counts.get("cancelled", 0) completed = status_counts.get("done", 0) - pct = round(completed / total * 100, 1) if total > 0 else 0.0 + # Cancelled tasks are resolved work, not pending — exclude them from the + # percent-complete denominator so a milestone whose only open task was + # cancelled still reaches 100% (and auto-collapses) instead of stalling. + active_total = total - cancelled + pct = round(completed / active_total * 100, 1) if active_total > 0 else 0.0 return { "total": total, @@ -166,6 +171,7 @@ async def get_milestone_progress(milestone_id: int) -> dict: "todo": status_counts.get("todo", 0), "in_progress": status_counts.get("in_progress", 0), "done": status_counts.get("done", 0), + "cancelled": cancelled, }, } diff --git a/src/fabledassistant/services/projects.py b/src/fabledassistant/services/projects.py index 477f65b..3b7ed74 100644 --- a/src/fabledassistant/services/projects.py +++ b/src/fabledassistant/services/projects.py @@ -6,11 +6,25 @@ from sqlalchemy import func, select from fabledassistant.models import async_session from fabledassistant.models.note import Note -from fabledassistant.models.project import Project +from fabledassistant.models.project import Project, ProjectStatus logger = logging.getLogger(__name__) +def _validate_status(status: str) -> str: + """Coerce/validate a project status against ProjectStatus. + + Canonical gate so the MCP create/update_project path (which passes status + straight through) can't persist an out-of-enum value — there's no DB CHECK. + """ + try: + return ProjectStatus(status).value + except ValueError: + raise ValueError( + f"Invalid status: {status!r}. Must be one of: {[s.value for s in ProjectStatus]}" + ) + + async def create_project( user_id: int, title: str, @@ -19,6 +33,7 @@ async def create_project( color: str | None = None, status: str = "active", ) -> Project: + status = _validate_status(status) async with async_session() as session: project = Project( user_id=user_id, @@ -87,6 +102,8 @@ async def update_project(user_id: int, project_id: int, **fields: object) -> Pro project = result.scalars().first() if project is None: return None + if "status" in fields and fields["status"] is not None: + fields["status"] = _validate_status(fields["status"]) for key, value in fields.items(): if hasattr(project, key): setattr(project, key, value) From 2fd9a2300a702038c890fd3a2502708e87088926 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:20:14 -0400 Subject: [PATCH 08/23] fix(caldav): point-event round-trip, recurrence push, delete propagation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift-audit Group 5 #7/#8 + Group 7 (CalDAV write-path): - Point events no longer fabricate a 60-min DTEND: caldav.create_event emits DTSTART-only when there's no end and no duration, so the next pull doesn't read it back as duration_minutes=60 and silently lengthen the event. - Recurrence edits now propagate: caldav.update_event gains a recurrence param (sentinel = leave unchanged; value/empty = set/clear RRULE), and _push_update passes the local event's rule so a changed/cleared RRULE isn't overwritten by the stale remote rule on the next pull. - Event deletions propagate to CalDAV: trash.delete captures an event's caldav_uid before soft-deleting and fires _push_delete, so a UI/MCP delete removes the remote copy instead of leaving it to linger. (delete_event the service primitive is kept — still tested/usable — rather than removed.) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/services/caldav.py | 42 ++++++++++++++++++++++---- src/fabledassistant/services/events.py | 3 ++ src/fabledassistant/services/trash.py | 17 +++++++++++ 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/fabledassistant/services/caldav.py b/src/fabledassistant/services/caldav.py index 12a0ec7..cb4c9a3 100644 --- a/src/fabledassistant/services/caldav.py +++ b/src/fabledassistant/services/caldav.py @@ -14,6 +14,10 @@ logger = logging.getLogger(__name__) CALDAV_SETTING_KEYS = ["caldav_url", "caldav_username", "caldav_password", "caldav_calendar_name", "caldav_timezone"] +# Sentinel: distinguishes "leave the RRULE untouched" from "clear it" (None/"") +# in update_event, since None is a meaningful value for recurrence. +_RECURRENCE_UNSET = object() + async def get_caldav_config(user_id: int) -> dict[str, str]: """Return the user's CalDAV config from their settings.""" @@ -214,14 +218,22 @@ async def create_event( result_end = d_end.isoformat() else: dt_start = _apply_timezone(datetime.fromisoformat(start), tz) + event.add("dtstart", dt_start) + result_start = dt_start.isoformat() if end: dt_end = _apply_timezone(datetime.fromisoformat(end), tz) + elif duration: + dt_end = dt_start + timedelta(minutes=duration) else: - dt_end = dt_start + timedelta(minutes=duration or 60) - event.add("dtstart", dt_start) - event.add("dtend", dt_end) - result_start = dt_start.isoformat() - result_end = dt_end.isoformat() + dt_end = None + if dt_end is not None: + event.add("dtend", dt_end) + result_end = dt_end.isoformat() + else: + # Point event (no end, no duration): emit DTSTART only. Fabricating + # a 60-min DTEND here would round-trip back on the next pull as + # duration_minutes=60, silently lengthening a point event. + result_end = None if description: event.add("description", description) @@ -325,8 +337,14 @@ async def update_event( location: str | None = None, timezone: str | None = None, calendar_name: str | None = None, + recurrence: str | None | object = _RECURRENCE_UNSET, ) -> dict: - """Update a calendar event matching the query.""" + """Update a calendar event matching the query. + + ``recurrence``: leave at the sentinel to keep the existing RRULE; pass an + RRULE string to set it, or None/"" to remove it. The push path passes the + local event's recurrence so RRULE edits propagate to the server. + """ config = await get_caldav_config(user_id) _check_config(config) tz = timezone or config.get("caldav_timezone") or None @@ -382,6 +400,18 @@ async def update_event( if "LOCATION" in component: del component["LOCATION"] component.add("location", location) + if recurrence is not _RECURRENCE_UNSET: + # Authoritatively sync the RRULE to the local event: drop the old + # rule, then re-add if a non-empty rule was provided (else clear it). + if "RRULE" in component: + del component["RRULE"] + if recurrence: + rrule_parts = {} + for part in str(recurrence).split(";"): + if "=" in part: + key, value = part.split("=", 1) + rrule_parts[key.strip().lower()] = value.strip() + component.add("rrule", rrule_parts) # Rebuild ical data and save cal_data = icalendar.Calendar() diff --git a/src/fabledassistant/services/events.py b/src/fabledassistant/services/events.py index ddd94c6..7bbe17d 100644 --- a/src/fabledassistant/services/events.py +++ b/src/fabledassistant/services/events.py @@ -450,6 +450,9 @@ async def _push_update(event: Event, user_id: int, old_title: str = "") -> None: end=derived_end.isoformat() if derived_end else None, description=event.description or None, location=event.location or None, + # Propagate the (possibly cleared) RRULE so a local recurrence edit + # isn't overwritten by the stale remote rule on the next pull. + recurrence=event.recurrence, ) except Exception: logger.warning("CalDAV push (update) failed for event %d", event.id, exc_info=True) diff --git a/src/fabledassistant/services/trash.py b/src/fabledassistant/services/trash.py index b0d068a..07da97c 100644 --- a/src/fabledassistant/services/trash.py +++ b/src/fabledassistant/services/trash.py @@ -149,11 +149,28 @@ async def delete(user_id: int, entity_type: str, entity_id: int) -> str | None: """ batch = str(uuid.uuid4()) now = datetime.now(timezone.utc) + caldav_event: tuple[str, str] | None = None async with async_session() as session: if not await _exists_alive(session, user_id, entity_type, entity_id): return None + # Capture CalDAV linkage before soft-deleting so we can propagate the + # deletion to the external server (the row stays present locally). + if entity_type == "event": + row = (await session.execute( + select(Event.caldav_uid, Event.title).where( + Event.id == entity_id, Event.user_id == user_id + ) + )).first() + if row and row[0]: + caldav_event = (row[0], row[1]) await _cascade(session, user_id, entity_type, entity_id, batch, now) await session.commit() + # Without this the soft-delete only hides the event locally and the remote + # copy lingers forever (and re-appears on any client syncing that server). + if caldav_event: + import asyncio + from fabledassistant.services.events import _push_delete + asyncio.create_task(_push_delete(caldav_event[0], caldav_event[1], user_id)) return batch From 7ce5bb8450fc5b728b298a3009d4d024cf7e0760 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:23:35 -0400 Subject: [PATCH 09/23] fix(lifecycle): OAuth pw 500, invite lockout, reminder re-arm, partial-unique MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift-audit Group 8 (lifecycle gaps): - change_password no longer 500s for OAuth-only users: short-circuit when password_hash is None (verify_password would crash on None) so the route returns a clean 4xx instead of a 500. - register_with_invitation no longer locks the invitee out on a username collision: create the user FIRST, then mark the token used, so a failed creation (409) leaves the single-use invite valid for retry. - update_event re-arms reminder_sent_at when start_dt/reminder_minutes change, so a rescheduled event fires again instead of being permanently suppressed. - Migration 0061: uq_topic_per_rulebook / uq_rule_per_topic become PARTIAL unique indexes (WHERE deleted_at IS NULL). Trashing 'X' then recreating it no longer 500s on the dead row's title. Model __table_args__ updated to match. Deferred: per-occurrence reminders for recurring events (event_scheduler) — needs a per-occurrence reminder-state design, not a one-line gate tweak. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0061_partial_unique_topic_rule_titles.py | 49 +++++++++++++++++++ src/fabledassistant/models/rulebook.py | 16 ++++-- src/fabledassistant/services/auth.py | 26 ++++++++-- src/fabledassistant/services/events.py | 5 ++ 4 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 alembic/versions/0061_partial_unique_topic_rule_titles.py diff --git a/alembic/versions/0061_partial_unique_topic_rule_titles.py b/alembic/versions/0061_partial_unique_topic_rule_titles.py new file mode 100644 index 0000000..fa8810c --- /dev/null +++ b/alembic/versions/0061_partial_unique_topic_rule_titles.py @@ -0,0 +1,49 @@ +"""partial-unique topic/rule titles (ignore soft-deleted rows) + +Revision ID: 0061 +Revises: 0060 +Create Date: 2026-06-02 + +Topics and rules are soft-deleted (SoftDeleteMixin), but uq_topic_per_rulebook +and uq_rule_per_topic were plain UNIQUE constraints. Trashing a topic/rule +named "X" then creating a new "X" — or restoring into a reused title slot — +collided with the dead row and raised an unhandled 500. Replace the full +UNIQUE constraints with partial unique indexes that only consider live +(deleted_at IS NULL) rows. +""" +from alembic import op +import sqlalchemy as sa + + +revision = "0061" +down_revision = "0060" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.drop_constraint("uq_topic_per_rulebook", "rulebook_topics", type_="unique") + op.create_index( + "uq_topic_per_rulebook", + "rulebook_topics", + ["rulebook_id", "title"], + unique=True, + postgresql_where=sa.text("deleted_at IS NULL"), + ) + op.drop_constraint("uq_rule_per_topic", "rules", type_="unique") + op.create_index( + "uq_rule_per_topic", + "rules", + ["topic_id", "title"], + unique=True, + postgresql_where=sa.text("deleted_at IS NULL"), + ) + + +def downgrade() -> None: + op.drop_index("uq_rule_per_topic", table_name="rules") + op.create_unique_constraint("uq_rule_per_topic", "rules", ["topic_id", "title"]) + op.drop_index("uq_topic_per_rulebook", table_name="rulebook_topics") + op.create_unique_constraint( + "uq_topic_per_rulebook", "rulebook_topics", ["rulebook_id", "title"] + ) diff --git a/src/fabledassistant/models/rulebook.py b/src/fabledassistant/models/rulebook.py index e65f9b9..67799aa 100644 --- a/src/fabledassistant/models/rulebook.py +++ b/src/fabledassistant/models/rulebook.py @@ -1,6 +1,6 @@ from datetime import datetime, timezone -from sqlalchemy import BigInteger, Boolean, Column, DateTime, ForeignKey, Integer, Table, Text, UniqueConstraint +from sqlalchemy import BigInteger, Boolean, Column, DateTime, ForeignKey, Index, Integer, Table, Text, text from sqlalchemy.orm import Mapped, mapped_column from fabledassistant.models import Base @@ -42,8 +42,13 @@ class Rulebook(Base, SoftDeleteMixin): class RulebookTopic(Base, SoftDeleteMixin): __tablename__ = "rulebook_topics" + # Partial unique: a title is unique among LIVE topics in a rulebook, so a + # trashed topic doesn't block recreating/restoring the same title. __table_args__ = ( - UniqueConstraint("rulebook_id", "title", name="uq_topic_per_rulebook"), + Index( + "uq_topic_per_rulebook", "rulebook_id", "title", + unique=True, postgresql_where=text("deleted_at IS NULL"), + ), ) id: Mapped[int] = mapped_column(BigInteger, primary_key=True) @@ -76,8 +81,13 @@ class RulebookTopic(Base, SoftDeleteMixin): class Rule(Base, SoftDeleteMixin): __tablename__ = "rules" + # Partial unique: title unique among LIVE rules in a topic (soft-deleted + # rules don't block recreating/restoring the same title). __table_args__ = ( - UniqueConstraint("topic_id", "title", name="uq_rule_per_topic"), + Index( + "uq_rule_per_topic", "topic_id", "title", + unique=True, postgresql_where=text("deleted_at IS NULL"), + ), ) id: Mapped[int] = mapped_column(BigInteger, primary_key=True) diff --git a/src/fabledassistant/services/auth.py b/src/fabledassistant/services/auth.py index c94bb04..ab1d005 100644 --- a/src/fabledassistant/services/auth.py +++ b/src/fabledassistant/services/auth.py @@ -106,6 +106,11 @@ async def change_password(user_id: int, current_password: str, new_password: str user = await session.get(User, user_id) if not user: return None + # OAuth-only accounts have no local password hash — verify_password + # would crash on None. Treat as "no local credential to change" (the + # route turns None into a clean 4xx, not a 500). + if user.password_hash is None: + return None if not verify_password(current_password, user.password_hash): return None user.password_hash = hash_password(new_password) @@ -325,12 +330,23 @@ async def register_with_invitation(raw_token: str, username: str, password: str) if not invitation or invitation.used or invitation.expires_at < datetime.now(timezone.utc): return None - invitation.used = True - await session.commit() + invitation_id = invitation.id + invite_email = invitation.email - # Create user outside the invitation session - user = await create_user(username, password, invitation.email) - logger.info("User '%s' registered via invitation for %s", username, invitation.email) + # Create the user FIRST, then consume the token. create_user can fail (e.g. + # a username collision raises and the route returns 409); marking the token + # used before that would burn this single-use invite and lock the invitee + # out of retrying. Ordering it after means a failed creation leaves the + # token valid. + user = await create_user(username, password, invite_email) + + async with async_session() as session: + invitation = await session.get(InvitationToken, invitation_id) + if invitation is not None: + invitation.used = True + await session.commit() + + logger.info("User '%s' registered via invitation for %s", username, invite_email) return user diff --git a/src/fabledassistant/services/events.py b/src/fabledassistant/services/events.py index 7bbe17d..66a82da 100644 --- a/src/fabledassistant/services/events.py +++ b/src/fabledassistant/services/events.py @@ -331,6 +331,11 @@ async def update_event(user_id: int, event_id: int, **fields) -> Event | None: for key, value in fields.items(): if key in allowed and (value is not None or key in nullable): setattr(event, key, value) + # Re-arm the reminder when the timing changes, so an event moved to a + # new (future) time — or given a new lead time — fires again instead of + # being permanently suppressed by a stale reminder_sent_at. + if "start_dt" in fields or "reminder_minutes" in fields: + event.reminder_sent_at = None await session.commit() await session.refresh(event) From 8d739c5da17fc72605219a189ff8905ee9d6860a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:24:57 -0400 Subject: [PATCH 10/23] perf(search): offload cosine scoring off event loop; document best-effort feed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift-audit Group 9 (param-cliff / unbounded search work): - semantic_search_notes: the O(rows) cosine-similarity scoring loop ran synchronously on the event loop, so every RAG injection / search stalled other requests proportional to the user's embedding count. Move the scoring into asyncio.to_thread (results unchanged). The deeper fix — bounding the candidate set via pgvector ORDER BY/LIMIT — is noted as separate infra work. - _semantic_knowledge_search: documented the best-effort top-N semantics — is the capped candidate-window size (not the true match count), matches beyond the cap aren't page-reachable, and each page recomputes the full merge. Prevents the silent-truncation trap; cached ranked-id paging / pgvector is the fix if exhaustive pagination is ever required. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/services/embeddings.py | 25 +++++++++++++--------- src/fabledassistant/services/knowledge.py | 8 +++++++ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/fabledassistant/services/embeddings.py b/src/fabledassistant/services/embeddings.py index 368b3ba..423917e 100644 --- a/src/fabledassistant/services/embeddings.py +++ b/src/fabledassistant/services/embeddings.py @@ -153,17 +153,22 @@ async def semantic_search_notes( if not rows: return [] - scored: list[tuple[float, Note]] = [] - for ne, note in rows: - try: - sim = _cosine_similarity(query_vec, ne.embedding) - except Exception: - continue - if sim >= threshold: - scored.append((sim, note)) + def _score() -> list[tuple[float, Note]]: + out: list[tuple[float, Note]] = [] + for ne, note in rows: + try: + sim = _cosine_similarity(query_vec, ne.embedding) + except Exception: + continue + if sim >= threshold: + out.append((sim, note)) + out.sort(key=lambda x: x[0], reverse=True) + return out[:limit] - scored.sort(key=lambda x: x[0], reverse=True) - return scored[:limit] + # Offload the O(rows) cosine scoring off the event loop so a large corpus + # doesn't stall other requests while ranking. Results are unchanged; the + # real scaling fix (ORDER BY / LIMIT in pgvector) is a separate effort. + return await asyncio.to_thread(_score) async def backfill_note_embeddings() -> None: diff --git a/src/fabledassistant/services/knowledge.py b/src/fabledassistant/services/knowledge.py index 1e0e48b..42b1141 100644 --- a/src/fabledassistant/services/knowledge.py +++ b/src/fabledassistant/services/knowledge.py @@ -140,6 +140,14 @@ async def _semantic_knowledge_search( Exact keyword matches always rank above semantic-only matches so that searching for a name like "Weston" surfaces the note with that title before conceptually related notes. + + BEST-EFFORT TOP-N, not exhaustive pagination: the ranked candidate set is + capped (keyword limit*2 + up to ~200 semantic), so `total` is the size of + that window, NOT the true match count, and matches beyond the cap are not + reachable by paging. Each page also recomputes the full merge (O(corpus) + per page). Acceptable for an interactive "best results" feed; a cached + ranked-id list or pgvector ORDER BY/LIMIT is the fix if exhaustive, + cheap pagination is ever needed. """ # 1. Keyword search — title and body ILIKE keyword_notes: list[Note] = [] From c39d7356ed0d07a00359107395259f1ef124a200 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:28:36 -0400 Subject: [PATCH 11/23] chore(dead-code): fix prod image, drop orphaned code, correct delete_rule doc Drift-audit Group 7 (renamed/removed lingers) + Group 5 #9: - docker-compose.prod.yml pulled fabledassistant:latest, a tag CI stopped publishing after the rename. Point it at fabledscribe:latest (the name CI and quickstart use). The internal DB name stays fabledassistant by design. - Remove the unused hard-delete delete_note imports from the notes and tasks route modules (they delete via trash; the import was an attractive nuisance that bypassed soft-delete). - delete_rule MCP tool: docstring/warning said 'permanently delete' but the body moves the rule to recoverable trash. Corrected to match. - Delete services/calendar_sync.py: fully orphaned (zero importers) and it read Config attrs that no longer exist, so any re-wiring would crash. - Remove dead services: notes.search_notes_for_context and logging.log_generation (zero callers; log_generation wrote a 'generation' category no stats/UI surface). Co-Authored-By: Claude Opus 4.8 (1M context) --- docker-compose.prod.yml | 2 +- src/fabledassistant/mcp/tools/rulebooks.py | 6 +- src/fabledassistant/routes/notes.py | 1 - src/fabledassistant/routes/tasks.py | 1 - src/fabledassistant/services/calendar_sync.py | 312 ------------------ src/fabledassistant/services/logging.py | 20 -- src/fabledassistant/services/notes.py | 31 -- 7 files changed, 4 insertions(+), 369 deletions(-) delete mode 100644 src/fabledassistant/services/calendar_sync.py diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 6150a66..5a89b8e 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -1,6 +1,6 @@ services: app: - image: git.fabledsword.com/bvandeusen/fabledassistant:latest + image: git.fabledsword.com/bvandeusen/fabledscribe:latest environment: DATABASE_URL: "postgresql+asyncpg://fabled:${DB_PASSWORD}@db:5432/fabledassistant" SECRET_KEY: "${SECRET_KEY}" diff --git a/src/fabledassistant/mcp/tools/rulebooks.py b/src/fabledassistant/mcp/tools/rulebooks.py index fabb55e..d95d81a 100644 --- a/src/fabledassistant/mcp/tools/rulebooks.py +++ b/src/fabledassistant/mcp/tools/rulebooks.py @@ -322,7 +322,7 @@ async def update_rule( async def delete_rule(rule_id: int, confirmed: bool = False) -> dict: - """Permanently delete a rule. Requires confirmed=True.""" + """Move a rule to the trash (recoverable). Requires confirmed=True.""" uid = current_user_id() rule = await rulebooks_svc.get_rule(rule_id, uid) if rule is None: @@ -330,8 +330,8 @@ async def delete_rule(rule_id: int, confirmed: bool = False) -> dict: if not confirmed: return { "warning": ( - f"Rule {rule_id} ('{rule.title}') will be permanently deleted. " - f"Pass confirmed=True to proceed." + f"Rule {rule_id} ('{rule.title}') will be moved to the trash " + f"(recoverable via restore). Pass confirmed=True to proceed." ), "confirmed_required": True, } diff --git a/src/fabledassistant/routes/notes.py b/src/fabledassistant/routes/notes.py index 7b56d4d..50a3f2b 100644 --- a/src/fabledassistant/routes/notes.py +++ b/src/fabledassistant/routes/notes.py @@ -14,7 +14,6 @@ from fabledassistant.services.notes import ( convert_note_to_task, convert_task_to_note, create_note, - delete_note, get_all_tags, get_backlinks, get_note, diff --git a/src/fabledassistant/routes/tasks.py b/src/fabledassistant/routes/tasks.py index c008453..87c8bd0 100644 --- a/src/fabledassistant/routes/tasks.py +++ b/src/fabledassistant/routes/tasks.py @@ -10,7 +10,6 @@ from fabledassistant.services.access import can_write_note from fabledassistant.services.embeddings import upsert_note_embedding from fabledassistant.services.notes import ( create_note, - delete_note, get_note, get_note_for_user, list_notes, diff --git a/src/fabledassistant/services/calendar_sync.py b/src/fabledassistant/services/calendar_sync.py deleted file mode 100644 index 0300556..0000000 --- a/src/fabledassistant/services/calendar_sync.py +++ /dev/null @@ -1,312 +0,0 @@ -"""Calendar sync service: bridges the DB Event table with local Radicale. - -Each user's calendars live at: http://radicale:5232/user_{user_id}/calendar/ -The app connects without auth (Radicale runs with auth.type = none inside Docker). - -External clients (iOS, macOS, Thunderbird) connect directly to Radicale on port 5232. -Their CalDAV URL is: http://:5232/user_{user_id}/calendar/ -""" - -import asyncio -import logging -import uuid -from datetime import datetime, timezone - -import httpx -import icalendar -from sqlalchemy import select - -from fabledassistant.config import Config -from fabledassistant.models import async_session -from fabledassistant.models.event import Event - -logger = logging.getLogger(__name__) - -# Internal Radicale URL (Docker service name) -_RADICALE_INTERNAL = "http://radicale:5232" - - -def _user_calendar_path(user_id: int) -> str: - return f"/user_{user_id}/calendar/" - - -def _user_calendar_url(user_id: int) -> str: - base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL - return f"{base}/user_{user_id}/calendar/" - - -async def setup_user_calendar(user_id: int) -> bool: - """Create Radicale collection for user if it doesn't exist. - - Returns True on success or if already exists, False on error. - """ - base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL - user_url = f"{base}/user_{user_id}/" - cal_url = f"{base}/user_{user_id}/calendar/" - - try: - async with httpx.AsyncClient(timeout=10.0) as client: - # Create user home collection - await client.request( - "MKCOL", - user_url, - headers={"Content-Type": "text/xml"}, - content=""" - - -""", - ) - # Create calendar collection - resp = await client.request( - "MKCOL", - cal_url, - headers={"Content-Type": "text/xml"}, - content=""" - - - - - Fabled Scribe - - -""", - ) - if resp.status_code in (201, 405): # 405 = already exists - logger.info("Calendar collection ready for user %d", user_id) - return True - logger.warning("Unexpected status %d creating calendar for user %d", resp.status_code, user_id) - return False - except Exception: - logger.warning("Failed to setup Radicale calendar for user %d", user_id, exc_info=True) - return False - - -def _make_vcalendar(event: Event) -> str: - """Build iCalendar string from DB Event.""" - cal = icalendar.Calendar() - cal.add("prodid", "-//FabledAssistant//EN") - cal.add("version", "2.0") - - vevent = icalendar.Event() - vevent.add("uid", event.uid) - vevent.add("summary", event.title) - - if event.all_day: - vevent.add("dtstart", event.start_dt.date()) - if event.end_dt: - vevent.add("dtend", event.end_dt.date()) - else: - vevent.add("dtstart", event.start_dt) - if event.end_dt: - vevent.add("dtend", event.end_dt) - - if event.description: - vevent.add("description", event.description) - if event.location: - vevent.add("location", event.location) - if event.recurrence: - rrule_parts: dict = {} - for part in event.recurrence.split(";"): - if "=" in part: - key, value = part.split("=", 1) - rrule_parts[key.strip().lower()] = value.strip() - if rrule_parts: - vevent.add("rrule", rrule_parts) - - vevent.add("dtstamp", datetime.now(timezone.utc)) - cal.add_component(vevent) - return cal.to_ical().decode("utf-8") - - -async def push_event_to_radicale(user_id: int, event: Event) -> bool: - """Write a DB Event to Radicale. Creates or updates the resource. - - Returns True on success. - """ - base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL - resource_url = f"{base}/user_{user_id}/calendar/{event.uid}.ics" - ical_data = _make_vcalendar(event) - - try: - async with httpx.AsyncClient(timeout=10.0) as client: - resp = await client.put( - resource_url, - content=ical_data.encode("utf-8"), - headers={"Content-Type": "text/calendar; charset=utf-8"}, - ) - if resp.status_code in (200, 201, 204): - return True - logger.warning("Radicale PUT returned %d for event %s", resp.status_code, event.uid) - return False - except Exception: - logger.warning("Failed to push event %s to Radicale", event.uid, exc_info=True) - return False - - -async def delete_event_from_radicale(user_id: int, uid: str) -> bool: - """Delete an event from Radicale by UID.""" - base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL - resource_url = f"{base}/user_{user_id}/calendar/{uid}.ics" - try: - async with httpx.AsyncClient(timeout=10.0) as client: - resp = await client.delete(resource_url) - return resp.status_code in (200, 204, 404) # 404 is ok (already gone) - except Exception: - logger.warning("Failed to delete event %s from Radicale", uid, exc_info=True) - return False - - -async def sync_event_to_db(user_id: int, ical_uid: str) -> Event | None: - """Fetch an event from Radicale and upsert into the DB Event table. - - Returns the upserted Event or None on error. - """ - base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL - resource_url = f"{base}/user_{user_id}/calendar/{ical_uid}.ics" - - try: - async with httpx.AsyncClient(timeout=10.0) as client: - resp = await client.get(resource_url) - if resp.status_code == 404: - return None - resp.raise_for_status() - ical_data = resp.text - except Exception: - logger.warning("Failed to fetch event %s from Radicale", ical_uid, exc_info=True) - return None - - try: - cal = icalendar.Calendar.from_ical(ical_data) - for component in cal.walk(): - if component.name != "VEVENT": - continue - title = str(component.get("SUMMARY", "")) - dtstart = component.get("DTSTART") - dtend = component.get("DTEND") - description = str(component.get("DESCRIPTION", "")) - location = str(component.get("LOCATION", "")) - rrule = component.get("RRULE") - recurrence = rrule.to_ical().decode("utf-8") if rrule else None - - all_day = False - if dtstart and isinstance(dtstart.dt, datetime): - start_dt = dtstart.dt if dtstart.dt.tzinfo else dtstart.dt.replace(tzinfo=timezone.utc) - elif dtstart: - from datetime import date - d = dtstart.dt - start_dt = datetime(d.year, d.month, d.day, tzinfo=timezone.utc) - all_day = True - else: - continue - - end_dt = None - if dtend and isinstance(dtend.dt, datetime): - end_dt = dtend.dt if dtend.dt.tzinfo else dtend.dt.replace(tzinfo=timezone.utc) - elif dtend: - from datetime import date - d = dtend.dt - end_dt = datetime(d.year, d.month, d.day, tzinfo=timezone.utc) - - # Storage uses duration, not end_dt. Convert iCal DTEND to a - # minute count anchored on DTSTART. Treat invalid (end <= start) - # incoming data as a point event rather than rejecting; we - # don't control external CalDAV writers. - duration_minutes = None - if end_dt is not None and end_dt > start_dt: - duration_minutes = int((end_dt - start_dt).total_seconds() // 60) - - async with async_session() as session: - result = await session.execute( - select(Event).where(Event.user_id == user_id, Event.uid == ical_uid) - ) - existing = result.scalars().first() - if existing: - existing.title = title - existing.start_dt = start_dt - existing.duration_minutes = duration_minutes - existing.all_day = all_day - existing.description = description - existing.location = location - existing.recurrence = recurrence - existing.updated_at = datetime.now(timezone.utc) - await session.commit() - await session.refresh(existing) - return existing - else: - event_obj = Event( - user_id=user_id, - uid=ical_uid, - title=title, - start_dt=start_dt, - duration_minutes=duration_minutes, - all_day=all_day, - description=description, - location=location, - recurrence=recurrence, - ) - session.add(event_obj) - await session.commit() - await session.refresh(event_obj) - return event_obj - except Exception: - logger.warning("Failed to parse/upsert Radicale event %s", ical_uid, exc_info=True) - return None - - -async def sync_all_events_from_radicale(user_id: int) -> int: - """Full sync: fetch all events from Radicale calendar and upsert into DB. - - Returns number of events synced. - """ - base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL - cal_url = f"{base}/user_{user_id}/calendar/" - - try: - async with httpx.AsyncClient(timeout=30.0) as client: - # PROPFIND to get all .ics resources - resp = await client.request( - "PROPFIND", - cal_url, - headers={"Depth": "1", "Content-Type": "text/xml"}, - content=""" -""", - ) - if resp.status_code == 404: - logger.info("No calendar found for user %d — setting up", user_id) - await setup_user_calendar(user_id) - return 0 - if resp.status_code != 207: - logger.warning("PROPFIND returned %d for user %d", resp.status_code, user_id) - return 0 - # Extract UIDs from href paths - import re - hrefs = re.findall(r"<[Dd]:[Hh]ref>([^<]+\.ics)", resp.text) - uids = [h.rstrip("/").split("/")[-1].removesuffix(".ics") for h in hrefs] - except Exception: - logger.warning("Failed to list Radicale events for user %d", user_id, exc_info=True) - return 0 - - synced = 0 - for uid in uids: - if uid: - event = await sync_event_to_db(user_id, uid) - if event: - synced += 1 - - logger.info("Synced %d events from Radicale for user %d", synced, user_id) - return synced - - -def get_external_caldav_url(user_id: int) -> str: - """Return the CalDAV URL for external clients (iOS, macOS, Thunderbird). - - Uses BASE_URL from Config with port 5232 substituted, or RADICALE_EXTERNAL_URL. - """ - external = Config.RADICALE_EXTERNAL_URL - if external: - return f"{external.rstrip('/')}/user_{user_id}/calendar/" - # Fall back to BASE_URL host with port 5232 - from urllib.parse import urlparse - parsed = urlparse(Config.BASE_URL) - host = parsed.hostname or "localhost" - return f"http://{host}:5232/user_{user_id}/calendar/" diff --git a/src/fabledassistant/services/logging.py b/src/fabledassistant/services/logging.py index 0c70f6e..df8dac3 100644 --- a/src/fabledassistant/services/logging.py +++ b/src/fabledassistant/services/logging.py @@ -59,26 +59,6 @@ async def log_usage( await session.commit() -async def log_generation( - user_id: int, - conv_id: int, - model: str, - timing: dict, -) -> None: - """Persist per-generation timing breakdown to app_logs for benchmarking.""" - async with async_session() as session: - log = AppLog( - category="generation", - user_id=user_id, - action="generation", - endpoint=f"/chat/conversations/{conv_id}", - duration_ms=timing.get("total_ms"), - details=json.dumps({"model": model, "conv_id": conv_id, **timing}), - ) - session.add(log) - await session.commit() - - async def log_error( user_id: int | None = None, username: str | None = None, diff --git a/src/fabledassistant/services/notes.py b/src/fabledassistant/services/notes.py index 90ee994..042cfff 100644 --- a/src/fabledassistant/services/notes.py +++ b/src/fabledassistant/services/notes.py @@ -409,37 +409,6 @@ async def convert_task_to_note(user_id: int, note_id: int) -> Note: return note -async def search_notes_for_context( - user_id: int, - keywords: list[str], - exclude_ids: set[int] | None = None, - limit: int = 3, - project_id: int | None = None, - orphan_only: bool = False, -) -> list[Note]: - """Search notes by keywords with OR logic. Optimized for context building — no count query.""" - async with async_session() as session: - keyword_filters = [] - for kw in keywords: - escaped = kw.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") - pattern = f"%{escaped}%" - keyword_filters.append(or_(Note.title.ilike(pattern), Note.body.ilike(pattern))) - - query = select(Note).where( - Note.user_id == user_id, or_(*keyword_filters), Note.deleted_at.is_(None) - ) - if orphan_only: - query = query.where(Note.project_id.is_(None)) - elif project_id is not None: - query = query.where(Note.project_id == project_id) - if exclude_ids: - query = query.where(Note.id.notin_(exclude_ids)) - query = query.order_by(Note.updated_at.desc()).limit(limit) - - result = await session.execute(query) - return list(result.scalars().all()) - - async def get_notes_by_ids(user_id: int, note_ids: list[int]) -> dict[int, Note]: """Batch fetch notes by ID list. Returns {note_id: Note}.""" if not note_ids: From 64bc50c7886f5e3e635242bb67641db2f0672d46 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:47:28 -0400 Subject: [PATCH 12/23] chore(frontend): drop dead settings toggle, types, and store list-surface Drift-audit Group 7 frontend cleanup (no behavioral change): - SettingsView: remove the 'auto-consolidate task bodies' toggle and its saveAutoConsolidate handler. The auto_consolidate_tasks setting has zero backend readers (curator removed in Phase 8); the control did nothing. - AppSettings type: drop the dead assistant_name / default_model hints (kept the open string index signature the store actually uses). Delete the fully orphaned types/chat.ts (zero importers). - notes/tasks Pinia stores: remove the list/filter/sort/pagination surface that backed the removed /notes and /tasks list views (verified no consumer uses the tasks/notes arrays, refresh, or any filter/sort/pagination method). Kept currentNote/currentTask, loading, fetch/create/update/delete, convert, patchStatus, startPlanning, backlinks, tags. Co-Authored-By: Claude Opus 4.8 (1M context) --- frontend/src/stores/notes.ts | 122 ++-------------------------- frontend/src/stores/tasks.ts | 120 +-------------------------- frontend/src/types/chat.ts | 67 --------------- frontend/src/types/settings.ts | 5 +- frontend/src/views/SettingsView.vue | 40 --------- 5 files changed, 12 insertions(+), 342 deletions(-) delete mode 100644 frontend/src/types/chat.ts diff --git a/frontend/src/stores/notes.ts b/frontend/src/stores/notes.ts index 1bf6a8a..2da439f 100644 --- a/frontend/src/stores/notes.ts +++ b/frontend/src/stores/notes.ts @@ -2,66 +2,16 @@ import { ref } from "vue"; import { defineStore } from "pinia"; import { apiGet, apiPost, apiPut, apiDelete } from "@/api/client"; import { useToastStore } from "@/stores/toast"; -import type { Note, NoteListResponse } from "@/types/note"; +import type { Note } from "@/types/note"; +// Single-note + mutation surface. The list/filter/sort/pagination surface that +// backed the removed /notes list view was dropped in the 2026-06-02 drift-audit +// cleanup (no consumers — KnowledgeView and the editors use single-entity and +// mutation methods only). export const useNotesStore = defineStore("notes", () => { - const notes = ref([]); const currentNote = ref(null); - const total = ref(0); const loading = ref(false); - // Filter / pagination / sort state - const activeTagFilters = ref([]); - const limit = ref(20); - const offset = ref(0); - const sortField = ref("updated_at"); - const sortOrder = ref<"asc" | "desc">("desc"); - const searchQuery = ref(""); - - async function refresh() { - loading.value = true; - try { - const searchParams = new URLSearchParams(); - if (searchQuery.value) searchParams.set("q", searchQuery.value); - for (const t of activeTagFilters.value) { - searchParams.append("tag", t); - } - searchParams.set("sort", sortField.value); - searchParams.set("order", sortOrder.value); - searchParams.set("limit", String(limit.value)); - searchParams.set("offset", String(offset.value)); - - const qs = searchParams.toString(); - const data = await apiGet( - `/api/notes${qs ? `?${qs}` : ""}` - ); - notes.value = data.notes; - total.value = data.total; - } catch (e) { - useToastStore().show("Failed to load notes", "error"); - throw e; - } finally { - loading.value = false; - } - } - - async function fetchNotes(params?: { - q?: string; - tag?: string[]; - sort?: string; - order?: string; - limit?: number; - offset?: number; - }) { - if (params?.q !== undefined) searchQuery.value = params.q || ""; - if (params?.tag) activeTagFilters.value = params.tag; - if (params?.sort) sortField.value = params.sort; - if (params?.order) sortOrder.value = params.order as "asc" | "desc"; - if (params?.limit) limit.value = params.limit; - if (params?.offset !== undefined) offset.value = params.offset; - await refresh(); - } - async function fetchNote(id: number) { loading.value = true; try { @@ -110,7 +60,6 @@ export const useNotesStore = defineStore("notes", () => { async function deleteNote(id: number) { try { await apiDelete(`/api/notes/${id}`); - notes.value = notes.value.filter((n) => n.id !== id); if (currentNote.value?.id === id) { currentNote.value = null; } @@ -120,50 +69,6 @@ export const useNotesStore = defineStore("notes", () => { } } - function addTagFilter(tag: string) { - if (!activeTagFilters.value.includes(tag)) { - activeTagFilters.value.push(tag); - offset.value = 0; - refresh(); - } - } - - function removeTagFilter(tag: string) { - activeTagFilters.value = activeTagFilters.value.filter((t) => t !== tag); - offset.value = 0; - refresh(); - } - - function clearTagFilters() { - activeTagFilters.value = []; - offset.value = 0; - refresh(); - } - - function setTagFilters(tags: string[]) { - activeTagFilters.value = [...tags]; - offset.value = 0; - refresh(); - } - - function setSort(field: string, order: "asc" | "desc") { - sortField.value = field; - sortOrder.value = order; - offset.value = 0; - refresh(); - } - - function setOffset(newOffset: number) { - offset.value = newOffset; - refresh(); - } - - function setSearch(q: string) { - searchQuery.value = q; - offset.value = 0; - refresh(); - } - async function resolveTitle(title: string): Promise { return await apiPost("/api/notes/resolve-title", { title }); } @@ -216,29 +121,12 @@ export const useNotesStore = defineStore("notes", () => { } return { - notes, currentNote, - total, loading, - activeTagFilters, - limit, - offset, - sortField, - sortOrder, - searchQuery, - fetchNotes, fetchNote, createNote, updateNote, deleteNote, - addTagFilter, - removeTagFilter, - clearTagFilters, - setTagFilters, - setSort, - setOffset, - setSearch, - refresh, resolveTitle, convertToTask, convertToNote, diff --git a/frontend/src/stores/tasks.ts b/frontend/src/stores/tasks.ts index e619a2a..f778d23 100644 --- a/frontend/src/stores/tasks.ts +++ b/frontend/src/stores/tasks.ts @@ -2,53 +2,15 @@ import { ref } from "vue"; import { defineStore } from "pinia"; import { apiGet, apiPost, apiPut, apiPatch, apiDelete } from "@/api/client"; import { useToastStore } from "@/stores/toast"; -import type { Task, TaskListResponse, TaskStatus, TaskPriority, StartPlanningResult } from "@/types/task"; +import type { Task, TaskStatus, TaskPriority, StartPlanningResult } from "@/types/task"; +// Single-task + mutation surface. The list/filter/sort/pagination surface that +// backed the removed /tasks list view was dropped in the 2026-06-02 drift-audit +// cleanup (no consumers — ProjectView's kanban keeps its own local task list). export const useTasksStore = defineStore("tasks", () => { - const tasks = ref([]); const currentTask = ref(null); - const total = ref(0); const loading = ref(false); - // Filter / pagination / sort state - const activeTagFilters = ref([]); - const statusFilter = ref([]); - const priorityFilter = ref([]); - const limit = ref(20); - const offset = ref(0); - const sortField = ref("updated_at"); - const sortOrder = ref<"asc" | "desc">("desc"); - const searchQuery = ref(""); - - async function refresh() { - loading.value = true; - try { - const searchParams = new URLSearchParams(); - if (searchQuery.value) searchParams.set("q", searchQuery.value); - for (const t of activeTagFilters.value) { - searchParams.append("tag", t); - } - for (const s of statusFilter.value) searchParams.append("status", s); - for (const p of priorityFilter.value) searchParams.append("priority", p); - searchParams.set("sort", sortField.value); - searchParams.set("order", sortOrder.value); - searchParams.set("limit", String(limit.value)); - searchParams.set("offset", String(offset.value)); - - const qs = searchParams.toString(); - const data = await apiGet( - `/api/tasks${qs ? `?${qs}` : ""}` - ); - tasks.value = data.tasks; - total.value = data.total; - } catch (e) { - useToastStore().show("Failed to load tasks", "error"); - throw e; - } finally { - loading.value = false; - } - } - async function fetchTask(id: number) { loading.value = true; try { @@ -102,10 +64,6 @@ export const useTasksStore = defineStore("tasks", () => { async function patchStatus(id: number, status: TaskStatus): Promise { try { const task = await apiPatch(`/api/tasks/${id}/status`, { status }); - const idx = tasks.value.findIndex((t) => t.id === id); - if (idx !== -1) { - tasks.value[idx] = task; - } if (currentTask.value?.id === id) { currentTask.value = task; } @@ -119,7 +77,6 @@ export const useTasksStore = defineStore("tasks", () => { async function deleteTask(id: number) { try { await apiDelete(`/api/tasks/${id}`); - tasks.value = tasks.value.filter((t) => t.id !== id); if (currentTask.value?.id === id) { currentTask.value = null; } @@ -144,83 +101,14 @@ export const useTasksStore = defineStore("tasks", () => { } } - function setStatusFilter(statuses: TaskStatus[]) { - statusFilter.value = statuses; - offset.value = 0; - refresh(); - } - - function setPriorityFilter(priorities: TaskPriority[]) { - priorityFilter.value = priorities; - offset.value = 0; - refresh(); - } - - function addTagFilter(tag: string) { - if (!activeTagFilters.value.includes(tag)) { - activeTagFilters.value.push(tag); - offset.value = 0; - refresh(); - } - } - - function removeTagFilter(tag: string) { - activeTagFilters.value = activeTagFilters.value.filter((t) => t !== tag); - offset.value = 0; - refresh(); - } - - function clearTagFilters() { - activeTagFilters.value = []; - offset.value = 0; - refresh(); - } - - function setSort(field: string, order: "asc" | "desc") { - sortField.value = field; - sortOrder.value = order; - offset.value = 0; - refresh(); - } - - function setOffset(newOffset: number) { - offset.value = newOffset; - refresh(); - } - - function setSearch(q: string) { - searchQuery.value = q; - offset.value = 0; - refresh(); - } - return { - tasks, currentTask, - total, loading, - activeTagFilters, - statusFilter, - priorityFilter, - limit, - offset, - sortField, - sortOrder, - searchQuery, - refresh, fetchTask, createTask, updateTask, patchStatus, deleteTask, startPlanning, - setStatusFilter, - setPriorityFilter, - addTagFilter, - removeTagFilter, - clearTagFilters, - setSort, - setOffset, - setSearch, }; }); diff --git a/frontend/src/types/chat.ts b/frontend/src/types/chat.ts deleted file mode 100644 index 162dd99..0000000 --- a/frontend/src/types/chat.ts +++ /dev/null @@ -1,67 +0,0 @@ -export interface GenerationTiming { - total_ms: number; - intent_ms: number | null; - ttft_ms: number | null; - generation_ms: number | null; - tools: Array<{ name: string; ms: number }>; -} - -export interface ToolCallRecord { - function: string; - arguments: Record; - result: { success: boolean; type?: string; data?: Record; error?: string; suggested_tags?: string[]; requires_confirmation?: boolean; similar_note?: { id: number; title: string } }; - status: "running" | "success" | "error" | "declined"; -} - -export interface ToolPendingRecord { - function: string; - arguments: Record; - label?: string; -} - -export interface Message { - id: number; - conversation_id: number; - role: "system" | "user" | "assistant"; - content: string; - status?: "complete" | "generating" | "error"; - context_note_id: number | null; - context_note_title?: string | null; - tool_calls?: ToolCallRecord[] | null; - metadata?: Record | null; - created_at: string; - timing?: GenerationTiming; - thinking?: string; -} - -export interface SendMessageResponse { - assistant_message_id: number; - status: string; -} - -export interface Conversation { - id: number; - title: string; - model: string; - message_count: number; - rag_project_id: number | null; - created_at: string; - updated_at: string; -} - -export interface ConversationDetail extends Conversation { - messages: Message[]; -} - -export interface ContextMeta { - context_note_id: number | null; - context_note_title: string | null; - auto_notes: { id: number; title: string; score?: number | null; auto_injected?: boolean }[]; - auto_injected_notes?: { id: number; title: string; score?: number | null }[]; -} - -export interface OllamaStatus { - ollama: "available" | "unavailable"; - model: "loaded" | "cold" | "not_found"; - default_model: string; -} diff --git a/frontend/src/types/settings.ts b/frontend/src/types/settings.ts index ec4862a..a1b388a 100644 --- a/frontend/src/types/settings.ts +++ b/frontend/src/types/settings.ts @@ -1,5 +1,6 @@ +// Settings are free-form string key/value pairs keyed by setting name; the +// backend has no fixed schema, so this is an open string map. (The former +// assistant_name/default_model hints were dead after the Phase-8 chat removal.) export interface AppSettings { - assistant_name?: string; - default_model?: string; [key: string]: string | undefined; } diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index eeeba59..b95baef 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -14,25 +14,10 @@ const toastStore = useToastStore(); const userTimezone = ref(""); const savingTimezone = ref(false); const timezoneSaved = ref(false); -const autoConsolidateTasks = ref(true); -const savingAutoConsolidate = ref(false); const trashRetentionDays = ref("90"); const savingRetention = ref(false); const retentionSaved = ref(false); -async function saveAutoConsolidate() { - savingAutoConsolidate.value = true; - try { - await apiPut('/api/settings', { - auto_consolidate_tasks: autoConsolidateTasks.value ? "true" : "false", - }); - } catch { - toastStore.show('Failed to save setting', 'error'); - } finally { - savingAutoConsolidate.value = false; - } -} - // think_enabled setting removed 2026-05-23. The chat+curator architecture // has tools=[] on the chat model; think on a no-tools conversational pass // is pure latency cost. See generation_task.py:run_generation comment for @@ -403,8 +388,6 @@ onMounted(async () => { const allSettings = await apiGet>("/api/settings"); userTimezone.value = allSettings.user_timezone ?? ""; trashRetentionDays.value = allSettings.trash_retention_days ?? "90"; - // Default true if unset; explicit "false" disables auto-consolidation. - autoConsolidateTasks.value = (allSettings.auto_consolidate_tasks ?? "true") !== "false"; if (allSettings.notify_task_reminders !== undefined) { notifyTaskReminders.value = allSettings.notify_task_reminders !== "false"; } @@ -992,29 +975,6 @@ function formatUserDate(iso: string): string {
- -
-

Tasks

-

- Task bodies are auto-summarized from accumulated work logs. The summary runs every few logs, plus on task close. -

-
- -

- When off, the task body is only refreshed when you click "Re-consolidate" - on a task. Existing summaries remain in place. -

-
-
-

Timezone

From cf4962d7e89bd48c671f22552174097e0795ab73 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:48:37 -0400 Subject: [PATCH 13/23] chore(profile): drop dead curator columns from UserProfile Drift-audit Group 7: learned_summary, observations_raw, and observations_updated_at were populated by the curator/LLM-profile machinery removed in the Phase-8 pivot. Nothing has written them since and the profile API returned permanently-empty fields. Remove them from the model + to_dict and drop the columns (migration 0062). Verified zero frontend/backend/test consumers. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0062_drop_userprofile_curator_columns.py | 41 +++++++++++++++++++ src/fabledassistant/models/user_profile.py | 18 +------- 2 files changed, 42 insertions(+), 17 deletions(-) create mode 100644 alembic/versions/0062_drop_userprofile_curator_columns.py diff --git a/alembic/versions/0062_drop_userprofile_curator_columns.py b/alembic/versions/0062_drop_userprofile_curator_columns.py new file mode 100644 index 0000000..00e5633 --- /dev/null +++ b/alembic/versions/0062_drop_userprofile_curator_columns.py @@ -0,0 +1,41 @@ +"""drop dead UserProfile curator columns + +Revision ID: 0062 +Revises: 0061 +Create Date: 2026-06-02 + +learned_summary, observations_raw, and observations_updated_at were written by +the curator/LLM-profile machinery removed in the Phase-8 MCP pivot. Nothing has +populated them since; the profile API returned permanently-empty fields. Drop +the columns. +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +revision = "0062" +down_revision = "0061" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.drop_column("user_profiles", "learned_summary") + op.drop_column("user_profiles", "observations_raw") + op.drop_column("user_profiles", "observations_updated_at") + + +def downgrade() -> None: + op.add_column( + "user_profiles", + sa.Column("observations_updated_at", sa.DateTime(timezone=True), nullable=True), + ) + op.add_column( + "user_profiles", + sa.Column("observations_raw", postgresql.JSONB(), nullable=True), + ) + op.add_column( + "user_profiles", + sa.Column("learned_summary", sa.Text(), nullable=True), + ) diff --git a/src/fabledassistant/models/user_profile.py b/src/fabledassistant/models/user_profile.py index 0e13e61..a5999d5 100644 --- a/src/fabledassistant/models/user_profile.py +++ b/src/fabledassistant/models/user_profile.py @@ -1,6 +1,4 @@ -from datetime import datetime - -from sqlalchemy import DateTime, ForeignKey, Integer, Text +from sqlalchemy import ForeignKey, Integer, Text from sqlalchemy.dialects.postgresql import ARRAY, JSONB from sqlalchemy.orm import Mapped, mapped_column @@ -27,13 +25,6 @@ class UserProfile(Base, TimestampMixin): interests: Mapped[list[str] | None] = mapped_column(ARRAY(Text), nullable=True) # {days: ["Mon","Tue",...], start: "09:00", end: "17:00"} work_schedule: Mapped[dict | None] = mapped_column(JSONB, nullable=True) - # LLM-consolidated summary of learned preferences - learned_summary: Mapped[str | None] = mapped_column(Text, nullable=True) - # [{date: "YYYY-MM-DD", bullets: "..."}, ...] - observations_raw: Mapped[list | None] = mapped_column(JSONB, nullable=True) - observations_updated_at: Mapped[datetime | None] = mapped_column( - DateTime(timezone=True), nullable=True - ) def to_dict(self) -> dict: return { @@ -45,11 +36,4 @@ class UserProfile(Base, TimestampMixin): "tone": self.tone or "casual", "interests": self.interests or [], "work_schedule": self.work_schedule or {}, - "learned_summary": self.learned_summary or "", - "observations_count": len(self.observations_raw or []), - "observations_updated_at": ( - self.observations_updated_at.isoformat() - if self.observations_updated_at - else None - ), } From 2c929a043557b8fe691f9b9a9d2a582ebfdafded Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 19:50:45 -0400 Subject: [PATCH 14/23] feat(reminders): per-occurrence reminders for recurring events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drift-audit Group 8 (final item). _fire_reminders previously gated on the base row (reminder_sent_at IS NULL AND start_dt > now), so a recurring event reminded at most once ever — once the first occurrence passed, no future occurrence qualified. Now recurring events are evaluated every sweep against their next occurrence (rrulestr.after(now)), and reminder_sent_at stores the start of the occurrence last reminded about. Each new occurrence has a distinct marker, so it re-arms and fires exactly once per occurrence. One-shot events keep the classic NULL gate. Also adds the deleted_at filter so trashed events stop reminding. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../services/event_scheduler.py | 67 +++++++++++++------ 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/src/fabledassistant/services/event_scheduler.py b/src/fabledassistant/services/event_scheduler.py index 25ef092..45cc50c 100644 --- a/src/fabledassistant/services/event_scheduler.py +++ b/src/fabledassistant/services/event_scheduler.py @@ -16,7 +16,8 @@ from datetime import datetime, timedelta, timezone from apscheduler.schedulers.background import BackgroundScheduler from apscheduler.triggers.interval import IntervalTrigger -from sqlalchemy import select +from dateutil.rrule import rrulestr +from sqlalchemy import and_, or_, select from fabledassistant.models import async_session from fabledassistant.models.event import Event @@ -32,7 +33,13 @@ _loop: asyncio.AbstractEventLoop | None = None # --------------------------------------------------------------------------- async def _fire_reminders() -> None: - """Find events with reminders due in the next 5 minutes and fire push notifications.""" + """Fire in-app reminders for events whose reminder time has arrived. + + One-shot events fire once (gated on reminder_sent_at IS NULL). Recurring + events fire once PER OCCURRENCE: reminder_sent_at stores the start of the + occurrence we last reminded about, so each new occurrence re-arms the + reminder instead of the whole series firing only once. + """ now = datetime.now(timezone.utc) window_end = now + timedelta(minutes=5) @@ -40,19 +47,38 @@ async def _fire_reminders() -> None: result = await session.execute( select(Event).where( Event.reminder_minutes.isnot(None), - Event.reminder_sent_at.is_(None), - Event.start_dt > now, # event hasn't started yet - # reminder fires when now >= start_dt - reminder_minutes - # i.e. start_dt <= now + reminder_minutes (approximated by window_end check) + Event.deleted_at.is_(None), + or_( + # Recurring events are evaluated every sweep against their + # next occurrence (the base start_dt is long past). + Event.recurrence.isnot(None), + # One-shot events: classic gate. + and_(Event.reminder_sent_at.is_(None), Event.start_dt > now), + ), ) ) candidates = list(result.scalars().all()) - to_notify: list[Event] = [] + # (event_id, occurrence_start) — occurrence_start is also the dedup marker + # written to reminder_sent_at, so a given occurrence reminds exactly once. + to_notify: list[tuple[int, datetime]] = [] for event in candidates: - reminder_dt = event.start_dt - timedelta(minutes=event.reminder_minutes) - if reminder_dt <= window_end: - to_notify.append(event) + if event.recurrence: + try: + rule = rrulestr(event.recurrence, dtstart=event.start_dt, ignoretz=False) + occ = rule.after(now, inc=True) + except Exception: + logger.warning("Failed to expand RRULE for event %d reminder", event.id, exc_info=True) + continue + if occ is None: + continue + reminder_dt = occ - timedelta(minutes=event.reminder_minutes) + if reminder_dt <= window_end and event.reminder_sent_at != occ: + to_notify.append((event.id, occ)) + else: + reminder_dt = event.start_dt - timedelta(minutes=event.reminder_minutes) + if reminder_dt <= window_end: + to_notify.append((event.id, event.start_dt)) if not to_notify: return @@ -61,22 +87,23 @@ async def _fire_reminders() -> None: from fabledassistant.services.notifications import create_in_app_notification async with async_session() as session: - for event in to_notify: - result = await session.execute( - select(Event).where(Event.id == event.id) - ) - ev = result.scalar_one_or_none() - if ev is None or ev.reminder_sent_at is not None: + for event_id, occurrence_start in to_notify: + ev = (await session.execute( + select(Event).where(Event.id == event_id) + )).scalar_one_or_none() + # Skip if this exact occurrence was already reminded (covers a + # concurrent sweep and the one-shot already-sent case). + if ev is None or ev.reminder_sent_at == occurrence_start: continue await create_in_app_notification(ev.user_id, "event_reminder", { "event_id": ev.id, "title": ev.title, - "start_dt": ev.start_dt.isoformat() if ev.start_dt else None, + "start_dt": occurrence_start.isoformat(), "url": "/calendar", }) - # Stamp only after the notification is created, so a delivery - # failure leaves the reminder eligible to retry next sweep. - ev.reminder_sent_at = datetime.now(timezone.utc) + # Stamp the occurrence marker only after the notification is + # created, so a delivery failure leaves it eligible to retry. + ev.reminder_sent_at = occurrence_start await session.commit() From 1babe598436da93451055003538be1c807eb398b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 22:26:29 -0400 Subject: [PATCH 15/23] feat(processes): add resolve_process name/id resolver Task 1 of the Stored Processes plan (#582). resolve_process(user_id, name_or_id) resolves a note_type=process note owner-scoped + non-trashed, precedence numeric id -> exact case-insensitive title -> substring; returns (note, other_candidates) so an ambiguous fuzzy match can be disambiguated. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/services/notes.py | 33 ++++++++++ tests/test_services_notes_process.py | 87 +++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 tests/test_services_notes_process.py diff --git a/src/fabledassistant/services/notes.py b/src/fabledassistant/services/notes.py index 042cfff..d971029 100644 --- a/src/fabledassistant/services/notes.py +++ b/src/fabledassistant/services/notes.py @@ -409,6 +409,39 @@ async def convert_task_to_note(user_id: int, note_id: int) -> Note: return note +async def resolve_process(user_id: int, name_or_id) -> tuple[Note | None, list[dict]]: + """Resolve a stored process by id or name. + + Owner-scoped, note_type='process', non-trashed. Precedence: numeric id → + exact case-insensitive title → substring. Returns (note, other_candidates); + on a substring tie with no exact hit, `note` is the most-recently-updated + match and `other_candidates` lists the rest as [{id, title}] so the caller + can disambiguate. Returns (None, []) when nothing matches. + """ + async with async_session() as session: + base = select(Note).where( + Note.user_id == user_id, + Note.note_type == "process", + Note.deleted_at.is_(None), + ) + s = str(name_or_id).strip() + if s.isdigit(): + row = (await session.execute(base.where(Note.id == int(s)))).scalars().first() + if row is not None: + return row, [] + exact = (await session.execute( + base.where(func.lower(Note.title) == s.lower()).order_by(Note.updated_at.desc()) + )).scalars().first() + if exact is not None: + return exact, [] + matches = (await session.execute( + base.where(Note.title.ilike(f"%{s}%")).order_by(Note.updated_at.desc()) + )).scalars().all() + if not matches: + return None, [] + return matches[0], [{"id": n.id, "title": n.title} for n in matches[1:]] + + async def get_notes_by_ids(user_id: int, note_ids: list[int]) -> dict[int, Note]: """Batch fetch notes by ID list. Returns {note_id: Note}.""" if not note_ids: diff --git a/tests/test_services_notes_process.py b/tests/test_services_notes_process.py new file mode 100644 index 0000000..037d903 --- /dev/null +++ b/tests/test_services_notes_process.py @@ -0,0 +1,87 @@ +"""resolve_process precedence (id → exact title → substring) in services/notes.py. + +Mocks async_session — no real DB, matching the other notes-service tests. +""" +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +def _make_mock_session(): + s = AsyncMock() + s.__aenter__ = AsyncMock(return_value=s) + s.__aexit__ = AsyncMock(return_value=False) + return s + + +def _result(first=None, all_=None): + """A SQLAlchemy-result mock exposing .scalars().first()/.all().""" + r = MagicMock() + r.scalars.return_value.first.return_value = first + r.scalars.return_value.all.return_value = all_ or [] + return r + + +def _note(id, title): + n = MagicMock() + n.id = id + n.title = title + return n + + +@pytest.mark.asyncio +async def test_resolve_process_by_numeric_id(): + note = _note(5, "Drift Audit") + session = _make_mock_session() + # numeric id → first execute (id lookup) hits + session.execute = AsyncMock(side_effect=[_result(first=note)]) + with patch("fabledassistant.services.notes.async_session") as cls: + cls.return_value = session + from fabledassistant.services.notes import resolve_process + found, candidates = await resolve_process(1, "5") + assert found is note + assert candidates == [] + assert session.execute.await_count == 1 + + +@pytest.mark.asyncio +async def test_resolve_process_exact_title_beats_substring(): + note = _note(7, "Drift Audit") + session = _make_mock_session() + # non-digit → exact-title query (first execute) hits; substring never runs + session.execute = AsyncMock(side_effect=[_result(first=note)]) + with patch("fabledassistant.services.notes.async_session") as cls: + cls.return_value = session + from fabledassistant.services.notes import resolve_process + found, candidates = await resolve_process(1, "Drift Audit") + assert found is note + assert candidates == [] + assert session.execute.await_count == 1 + + +@pytest.mark.asyncio +async def test_resolve_process_substring_returns_candidates(): + n1 = _note(7, "Drift Audit Remediation") + n2 = _note(9, "Drift Audit Notes") + session = _make_mock_session() + # exact miss, then substring returns two (most-recent first) + session.execute = AsyncMock(side_effect=[_result(first=None), _result(all_=[n1, n2])]) + with patch("fabledassistant.services.notes.async_session") as cls: + cls.return_value = session + from fabledassistant.services.notes import resolve_process + found, candidates = await resolve_process(1, "drift") + assert found is n1 + assert candidates == [{"id": 9, "title": "Drift Audit Notes"}] + assert session.execute.await_count == 2 + + +@pytest.mark.asyncio +async def test_resolve_process_no_match(): + session = _make_mock_session() + session.execute = AsyncMock(side_effect=[_result(first=None), _result(all_=[])]) + with patch("fabledassistant.services.notes.async_session") as cls: + cls.return_value = session + from fabledassistant.services.notes import resolve_process + found, candidates = await resolve_process(1, "nope") + assert found is None + assert candidates == [] From 7b5a75989a2de8dd19b7312bd7ad87c49719377a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 22:27:41 -0400 Subject: [PATCH 16/23] feat(processes): MCP create/list/get/update_process tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 2 of #582. New mcp/tools/processes.py mirrors entities.py — tools wrap notes_svc directly. get_process is the fire mechanism (returns the full prompt via resolve_process; surfaces other_matches on an ambiguous name). Registered in register_all. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/mcp/tools/__init__.py | 3 +- src/fabledassistant/mcp/tools/processes.py | 91 ++++++++++++++++++++++ tests/test_mcp_tool_processes.py | 91 ++++++++++++++++++++++ 3 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 src/fabledassistant/mcp/tools/processes.py create mode 100644 tests/test_mcp_tool_processes.py diff --git a/src/fabledassistant/mcp/tools/__init__.py b/src/fabledassistant/mcp/tools/__init__.py index 93a5393..58c776d 100644 --- a/src/fabledassistant/mcp/tools/__init__.py +++ b/src/fabledassistant/mcp/tools/__init__.py @@ -5,7 +5,7 @@ to a FastMCP instance. `register_all(mcp)` is the single entry point called from `mcp.server.build_mcp_server`. """ from fabledassistant.mcp.tools import ( - entities, events, milestones, notes, projects, recent, rulebooks, search, tags, tasks, trash, + entities, events, milestones, notes, processes, projects, recent, rulebooks, search, tags, tasks, trash, ) @@ -20,5 +20,6 @@ def register_all(mcp) -> None: tags.register(mcp) recent.register(mcp) entities.register(mcp) + processes.register(mcp) rulebooks.register(mcp) trash.register(mcp) diff --git a/src/fabledassistant/mcp/tools/processes.py b/src/fabledassistant/mcp/tools/processes.py new file mode 100644 index 0000000..7718dd3 --- /dev/null +++ b/src/fabledassistant/mcp/tools/processes.py @@ -0,0 +1,91 @@ +"""Stored-process MCP tools: reusable saved prompts (note_type='process'). + +A process is a Note whose body is a prompt the operator fires later +("run the X process"). Mirrors entities.py — the tools wrap notes_svc directly. +get_process is the fire mechanism: it returns the full prompt for Claude to run. +""" +from __future__ import annotations + +from fabledassistant.mcp._context import current_user_id +from fabledassistant.services import knowledge as knowledge_svc +from fabledassistant.services import notes as notes_svc + + +async def list_processes(q: str = "", tag: str = "", limit: int = 50) -> dict: + """List stored processes (reusable saved prompts). + + Args: + q: Free-text search across title + body (optional). + tag: Filter to a single tag (optional). + limit: Max results (1-100). + + Returns {"processes": [{id, title, tags, preview}], "total": int}. + """ + uid = current_user_id() + items, total = await knowledge_svc.query_knowledge( + user_id=uid, note_type="process", tags=[tag] if tag else [], + sort="modified", q=q or None, limit=max(1, min(limit, 100)), offset=0, + ) + procs = [{"id": it["id"], "title": it["title"], "tags": it.get("tags", []), + "preview": it.get("snippet", "")} for it in items] + return {"processes": procs, "total": total} + + +async def create_process(title: str, body: str, tags: list[str] | None = None) -> dict: + """Create a stored process (a reusable saved prompt). + + Args: + title: Process name, e.g. "Drift Audit" (required). + body: The full prompt to run later (markdown). Required. + tags: Plain-string tags, no # prefix. + """ + if not (title or "").strip() or not (body or "").strip(): + raise ValueError("create_process requires a non-empty title and body") + uid = current_user_id() + note = await notes_svc.create_note( + uid, title=title.strip(), body=body, note_type="process", tags=tags, + ) + return note.to_dict() + + +async def get_process(name_or_id: str) -> dict: + """Fetch a stored process by name or id and return its full prompt — the + fire mechanism. The operator says "run the process"; call this and + follow the returned body (including any 'clarify first' steps it contains). + + Resolution: numeric id → exact (case-insensitive) title → substring. On an + ambiguous substring match, the best (most-recent) match is returned with an + `other_matches` list so you can disambiguate with the operator. + """ + uid = current_user_id() + note, candidates = await notes_svc.resolve_process(uid, name_or_id) + if note is None: + raise ValueError(f"process {name_or_id!r} not found") + out = note.to_dict() + if candidates: + out["other_matches"] = candidates + return out + + +async def update_process(process_id: int, title: str = "", body: str = "", + tags: list[str] | None = None) -> dict: + """Update a stored process. Only provided fields change — empty title/body + leave that field unchanged; pass tags to replace the tag set.""" + uid = current_user_id() + note = await notes_svc.get_note(uid, process_id) + if note is None or note.note_type != "process": + raise ValueError(f"process {process_id} not found") + fields: dict = {} + if title.strip(): + fields["title"] = title.strip() + if body.strip(): + fields["body"] = body + if tags is not None: + fields["tags"] = tags + updated = await notes_svc.update_note(uid, process_id, **fields) + return updated.to_dict() + + +def register(mcp) -> None: + for fn in (list_processes, create_process, get_process, update_process): + mcp.tool(name=fn.__name__)(fn) diff --git a/tests/test_mcp_tool_processes.py b/tests/test_mcp_tool_processes.py new file mode 100644 index 0000000..3b6a784 --- /dev/null +++ b/tests/test_mcp_tool_processes.py @@ -0,0 +1,91 @@ +"""Tests for MCP process tools — patches the service layer.""" +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from fabledassistant.mcp._context import _user_id_ctx + + +@pytest.fixture(autouse=True) +def _bind_user(): + token = _user_id_ctx.set(7) + yield + _user_id_ctx.reset(token) + + +def _fake_note(id=1, title="Drift Audit", note_type="process"): + n = MagicMock() + n.id = id + n.title = title + n.note_type = note_type + n.to_dict.return_value = {"id": id, "title": title, "note_type": note_type} + return n + + +@pytest.mark.asyncio +async def test_create_process_requires_title_and_body(): + from fabledassistant.mcp.tools.processes import create_process + with pytest.raises(ValueError): + await create_process(title="", body="something") + with pytest.raises(ValueError): + await create_process(title="X", body=" ") + + +@pytest.mark.asyncio +async def test_create_process_sets_note_type(): + created = _fake_note() + with patch("fabledassistant.services.notes.create_note", + AsyncMock(return_value=created)) as mock_create: + from fabledassistant.mcp.tools.processes import create_process + out = await create_process(title="Drift Audit", body="the prompt", tags=["audit"]) + assert out["note_type"] == "process" + # the service was asked to create a process + assert mock_create.await_args.kwargs["note_type"] == "process" + assert mock_create.await_args.kwargs["title"] == "Drift Audit" + + +@pytest.mark.asyncio +async def test_get_process_returns_body_and_candidates(): + note = _fake_note(id=7) + with patch("fabledassistant.services.notes.resolve_process", + AsyncMock(return_value=(note, [{"id": 9, "title": "Drift Audit Notes"}]))): + from fabledassistant.mcp.tools.processes import get_process + out = await get_process("drift") + assert out["id"] == 7 + assert out["other_matches"] == [{"id": 9, "title": "Drift Audit Notes"}] + + +@pytest.mark.asyncio +async def test_get_process_not_found_raises(): + with patch("fabledassistant.services.notes.resolve_process", + AsyncMock(return_value=(None, []))): + from fabledassistant.mcp.tools.processes import get_process + with pytest.raises(ValueError): + await get_process("missing") + + +@pytest.mark.asyncio +async def test_update_process_rejects_non_process_note(): + plain = _fake_note(id=3, note_type="note") + with patch("fabledassistant.services.notes.get_note", + AsyncMock(return_value=plain)): + from fabledassistant.mcp.tools.processes import update_process + with pytest.raises(ValueError): + await update_process(process_id=3, title="x") + + +def test_register_attaches_four_tools(): + from fabledassistant.mcp.tools import processes + names: list[str] = [] + + class FakeMcp: + def tool(self, name): + names.append(name) + def deco(fn): + return fn + return deco + + processes.register(FakeMcp()) + assert set(names) == { + "list_processes", "create_process", "get_process", "update_process", + } From c2b2694ea392d8c6716f7f43e227a437762c2a24 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 22:29:08 -0400 Subject: [PATCH 17/23] docs(mcp): document Processes in server instructions Task 3 of #582. Tells Claude that note_type=process notes are reusable saved prompts and to fire them via list_processes/get_process on 'run the X process'. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/mcp/server.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/fabledassistant/mcp/server.py b/src/fabledassistant/mcp/server.py index 388bbe1..81fe11e 100644 --- a/src/fabledassistant/mcp/server.py +++ b/src/fabledassistant/mcp/server.py @@ -78,6 +78,13 @@ descendants) to the trash and returns a deleted_batch_id. Use list_trash() to see trashed batches, restore(deleted_batch_id) to undo a deletion, and purge_trash(deleted_batch_id, confirmed=True) for a permanent delete. Trash auto-purges after the operator's retention window. + +Scribe stores reusable Processes — saved prompts/workflows (note_type +"process"), e.g. a drift audit or a DRY pass. When the operator says "run the +X process" or otherwise references a saved process, call list_processes() / +get_process(name) and follow the returned prompt verbatim, including any +"clarify first" steps it contains. Author a new one with create_process(title, +body); edit with update_process. """ From fb1ae915e4ef06edae6ec3dcc8a3b7360ae622d8 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 22:29:08 -0400 Subject: [PATCH 18/23] feat(processes): expose process as a knowledge type Task 4 of #582. Add 'process' to the knowledge route _VALID_TYPES and to the get_knowledge_counts facet + total. query_knowledge/_apply_type_filter already handle arbitrary note_type, so listing by type=process works unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fabledassistant/routes/knowledge.py | 2 +- src/fabledassistant/services/knowledge.py | 6 +-- tests/test_services_knowledge_counts.py | 45 +++++++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 tests/test_services_knowledge_counts.py diff --git a/src/fabledassistant/routes/knowledge.py b/src/fabledassistant/routes/knowledge.py index b51ef21..7a2aa4d 100644 --- a/src/fabledassistant/routes/knowledge.py +++ b/src/fabledassistant/routes/knowledge.py @@ -10,7 +10,7 @@ logger = logging.getLogger(__name__) knowledge_bp = Blueprint("knowledge", __name__, url_prefix="/api/knowledge") -_VALID_TYPES = {"note", "person", "place", "list", "task", "plan"} +_VALID_TYPES = {"note", "person", "place", "list", "task", "plan", "process"} _VALID_SORTS = {"modified", "created", "alpha", "type"} diff --git a/src/fabledassistant/services/knowledge.py b/src/fabledassistant/services/knowledge.py index 42b1141..1da852b 100644 --- a/src/fabledassistant/services/knowledge.py +++ b/src/fabledassistant/services/knowledge.py @@ -237,7 +237,7 @@ async def get_knowledge_counts(user_id: int, tags: list[str] | None = None) -> d .where(Note.user_id == user_id) .where(Note.status.is_(None)) .where(Note.deleted_at.is_(None)) - .where(Note.note_type.in_(["note", "person", "place", "list"])) + .where(Note.note_type.in_(["note", "person", "place", "list", "process"])) .group_by(Note.note_type) ) if tags: @@ -273,9 +273,9 @@ async def get_knowledge_counts(user_id: int, tags: list[str] | None = None) -> d plan_stmt = plan_stmt.where(Note.tags.contains([tag])) counts["plan"] = (await session.execute(plan_stmt)).scalar_one() - for t in ("note", "person", "place", "list", "task", "plan"): + for t in ("note", "person", "place", "list", "task", "plan", "process"): counts.setdefault(t, 0) - counts["total"] = sum(counts[t] for t in ("note", "person", "place", "list", "task")) + counts["total"] = sum(counts[t] for t in ("note", "person", "place", "list", "task", "process")) return counts diff --git a/tests/test_services_knowledge_counts.py b/tests/test_services_knowledge_counts.py new file mode 100644 index 0000000..ae3a803 --- /dev/null +++ b/tests/test_services_knowledge_counts.py @@ -0,0 +1,45 @@ +"""get_knowledge_counts includes the 'process' type and counts it in total.""" +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +def _make_mock_session(): + s = AsyncMock() + s.__aenter__ = AsyncMock(return_value=s) + s.__aexit__ = AsyncMock(return_value=False) + return s + + +def _grouped(rows): + r = MagicMock() + r.all.return_value = rows + return r + + +def _scalar(n): + r = MagicMock() + r.scalar_one.return_value = n + return r + + +@pytest.mark.asyncio +async def test_counts_include_process_in_facet_and_total(): + session = _make_mock_session() + # 1) grouped non-task counts, 2) task count, 3) plan count + session.execute = AsyncMock(side_effect=[ + _grouped([("note", 3), ("process", 2)]), + _scalar(1), # tasks + _scalar(0), # plans + ]) + with patch("fabledassistant.services.knowledge.async_session") as cls: + cls.return_value = session + from fabledassistant.services.knowledge import get_knowledge_counts + counts = await get_knowledge_counts(user_id=1) + + assert counts["process"] == 2 + # facet keys all present (setdefault) + for key in ("note", "person", "place", "list", "task", "plan", "process"): + assert key in counts + # total = note(3) + person(0) + place(0) + list(0) + task(1) + process(2) + assert counts["total"] == 6 From 74b337b587b9128c043de797a2c53ceafb18c76a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 22:33:23 -0400 Subject: [PATCH 19/23] feat(processes): surface processes in the Knowledge view Task 5 of #582. Add 'process' to the KnowledgeItem/activeType/KnowledgeCounts types, a Processes entry in the type-filter row, a Workflow-icon quick-create button (createNew('process') -> /notes/new?type=process), and a Process card badge. Co-Authored-By: Claude Opus 4.8 (1M context) --- frontend/src/views/KnowledgeView.vue | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/frontend/src/views/KnowledgeView.vue b/frontend/src/views/KnowledgeView.vue index a5c9f31..201c125 100644 --- a/frontend/src/views/KnowledgeView.vue +++ b/frontend/src/views/KnowledgeView.vue @@ -10,6 +10,7 @@ import { User, MapPin, List, + Workflow, Search, Share2, ChevronLeft, @@ -23,7 +24,7 @@ const router = useRouter(); interface KnowledgeItem { id: number; - note_type: "note" | "person" | "place" | "list" | "task"; + note_type: "note" | "person" | "place" | "list" | "task" | "process"; title: string; snippet: string; tags: string[]; @@ -61,7 +62,7 @@ interface UpcomingEvent { // ─── Filter state ───────────────────────────────────────────────────────────── -const activeType = ref<"" | "note" | "person" | "place" | "list" | "task" | "plan">(""); +const activeType = ref<"" | "note" | "person" | "place" | "list" | "task" | "plan" | "process">(""); const activeTag = ref(""); const sortMode = ref<"modified" | "created" | "alpha" | "type">("modified"); const searchQuery = ref(""); @@ -69,8 +70,8 @@ let searchDebounce: ReturnType | null = null; // ─── Type counts ────────────────────────────────────────────────────────────── -interface KnowledgeCounts { note: number; person: number; place: number; list: number; task: number; plan: number; total: number } -const typeCounts = ref({ note: 0, person: 0, place: 0, list: 0, task: 0, plan: 0, total: 0 }); +interface KnowledgeCounts { note: number; person: number; place: number; list: number; task: number; plan: number; process: number; total: number } +const typeCounts = ref({ note: 0, person: 0, place: 0, list: 0, task: 0, plan: 0, process: 0, total: 0 }); async function fetchCounts() { try { @@ -403,6 +404,10 @@ onUnmounted(() => { List +
@@ -417,11 +422,11 @@ onUnmounted(() => { {{ typeCounts.total }}