fix(personal): bound multi-file upload memory

/api/personal/upload and DELETE /api/personal/file mutated the same
vector and tracking state add/remove/reload serialize on, outside
_index_job_lock and inline on the event loop.

Both now stage async work on the loop, then run the complete transition
(vector writes, disk change, personal_docs_manager update) in one
offloaded critical section under the shared lock, acquired before the
offload so queued requests park on the loop rather than pinning a
threadpool worker.

Adds add-vs-upload and add-vs-file ordering regressions.

.github/workflows/ci.yml

@@ -2,7 +2,7 @@ name: CI
 
 on:
   push:
-    branches: [main]
+    branches: [main, dev]
   pull_request:
 
 # Least privilege: none of the jobs write to the repo.
@@ -103,10 +103,7 @@ jobs:
   python-tests:
     name: Python tests (pytest)
     runs-on: ubuntu-latest
-    # Informational for now: the suite has known flaky / environment-dependent
+    # Make Python test validation authoritative for the configured scope.
-    # failures (test isolation + embedding-model assertions). Tracked under the
-    # ROADMAP "fresh install smoke tests" item; make this required once green.
-    continue-on-error: true
     steps:
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0  # v7.0.0
         with:

core/database.py

@@ -5,7 +5,7 @@ from datetime import datetime, timezone
 from pathlib import Path
 from typing import Optional
 from urllib.parse import unquote, urlparse
-from sqlalchemy import event, create_engine, Column, String, Text, Boolean, DateTime, Integer, ForeignKey, JSON, Index, func, text
+from sqlalchemy import DDL, event, create_engine, Column, String, Text, Boolean, DateTime, Integer, ForeignKey, JSON, Index, func, inspect, text
 from sqlalchemy.engine import Engine, make_url
 from sqlalchemy.types import TypeDecorator
 from sqlalchemy.ext.declarative import declarative_base, declared_attr
@@ -430,6 +430,93 @@ class EmailAccount(TimestampMixin, Base):
     )
 
 
+class EmailAccountOwnerLock(Base):
+    """Durable per-owner mutex for email-account default mutations.
+
+    Row-locking databases serialize mutations by locking this row before they
+    inspect or stage EmailAccount changes.  SQLite uses ``BEGIN IMMEDIATE``
+    instead, because it ignores ``SELECT ... FOR UPDATE``; keeping the table in
+    the shared metadata still makes the non-SQLite path available without a
+    separate migration.  The empty key represents the normalized legacy /
+    unconfigured scope shared by ``owner IS NULL`` and ``owner = ''`` rows.
+    """
+    __tablename__ = "email_account_owner_locks"
+
+    owner_key = Column(String, primary_key=True)
+
+
+_EMAIL_ACCOUNT_DEFAULT_INDEX = "ux_email_accounts_one_default_per_owner"
+_EMAIL_ACCOUNT_DEFAULT_INDEX_DDL = {
+    "sqlite": (
+        f"CREATE UNIQUE INDEX IF NOT EXISTS {_EMAIL_ACCOUNT_DEFAULT_INDEX} "
+        "ON email_accounts (COALESCE(owner, '')) WHERE is_default = 1"
+    ),
+    "postgresql": (
+        f"CREATE UNIQUE INDEX IF NOT EXISTS {_EMAIL_ACCOUNT_DEFAULT_INDEX} "
+        "ON email_accounts ((COALESCE(owner, ''))) WHERE is_default IS TRUE"
+    ),
+}
+
+
+# SQLAlchemy cannot express one portable partial, functional index across the
+# two supported database families.  Register dialect-specific DDL so fresh
+# databases get the invariant as part of create_all(); the startup migration
+# below installs the same index on existing databases after normalizing legacy
+# duplicate rows.
+for _dialect_name, _index_ddl in _EMAIL_ACCOUNT_DEFAULT_INDEX_DDL.items():
+    event.listen(
+        EmailAccount.__table__,
+        "after_create",
+        DDL(_index_ddl).execute_if(dialect=_dialect_name),
+    )
+
+
+def lock_email_account_owner_mutations(db, *owners: str) -> None:
+    """Lock normalized email-account owner scopes in canonical order.
+
+    ``NULL`` and the empty string are one legacy/single-user owner partition,
+    matching the unique default-account index.  SQLite has only a database
+    writer reservation, while row-locking databases use durable mutex rows.
+    Sorting all requested owner keys keeps multi-owner operations such as user
+    rename from deadlocking with another mutation that requests the same keys
+    in the opposite order.
+    """
+    from sqlalchemy.exc import IntegrityError
+
+    owner_keys = sorted({owner or "" for owner in owners} or {""})
+    if db.get_bind().dialect.name == "sqlite":
+        db.execute(text("BEGIN IMMEDIATE"))
+        return
+
+    for owner_key in owner_keys:
+        lock_row = db.get(
+            EmailAccountOwnerLock,
+            owner_key,
+            with_for_update=True,
+        )
+        if lock_row is not None:
+            continue
+
+        inserted = False
+        try:
+            with db.begin_nested():
+                db.add(EmailAccountOwnerLock(owner_key=owner_key))
+                db.flush()
+                inserted = True
+        except IntegrityError:
+            # A competing transaction created the mutex row first.  Once its
+            # insert commits, lock that durable row before touching accounts.
+            pass
+
+        if not inserted:
+            (
+                db.query(EmailAccountOwnerLock)
+                .filter(EmailAccountOwnerLock.owner_key == owner_key)
+                .with_for_update()
+                .one()
+            )
+
+
 class ModelEndpoint(TimestampMixin, Base):
     """Admin-configured model endpoints. Models are auto-discovered via /v1/models."""
     __tablename__ = "model_endpoints"
@@ -1812,72 +1899,142 @@ class Integration(TimestampMixin, Base):
 
 
 
-def _migrate_seed_email_account():
+def _migrate_email_account_default_invariant():
-    """If email_accounts is empty and settings.json has legacy flat imap_host/smtp_host
+    """Normalize legacy duplicates and install durable at-most-one enforcement.
-    keys, create a single default account from them so nothing breaks for users who
+
-    upgraded. Safe to run repeatedly — it short-circuits once any row exists."""
+    Older databases only had a non-unique ``(owner, is_default)`` lookup index.
+    Keep the oldest default deterministically in each normalized owner scope,
+    then add the same partial functional unique index used for fresh schemas.
+    """
+    dialect_name = engine.dialect.name
+    index_ddl = _EMAIL_ACCOUNT_DEFAULT_INDEX_DDL.get(dialect_name)
+    if index_ddl is None:
+        logger.warning(
+            "Email-account default uniqueness is not available for database "
+            "dialect %s; mutations remain serialized but are not protected by "
+            "a database constraint",
+            dialect_name,
+        )
+        return
+
     try:
-        with engine.connect() as conn:
+        with engine.begin() as conn:
-            tables = [r[0] for r in conn.execute(text(
+            if not inspect(conn).has_table(EmailAccount.__tablename__):
-                "SELECT name FROM sqlite_master WHERE type='table' AND name='email_accounts'"
-            ))]
-            if "email_accounts" not in tables:
-                return
-            existing = conn.execute(text("SELECT COUNT(*) FROM email_accounts")).scalar() or 0
-            if existing > 0:
                 return
+            default_rows = conn.execute(text("""
+                SELECT id, owner
+                FROM email_accounts
+                WHERE is_default IS TRUE
+                ORDER BY
+                    COALESCE(owner, ''),
+                    CASE WHEN created_at IS NULL THEN 1 ELSE 0 END,
+                    created_at,
+                    id
+            """)).mappings()
+            seen_owner_keys = set()
+            duplicate_ids = []
+            for row in default_rows:
+                owner_key = row["owner"] or ""
+                if owner_key in seen_owner_keys:
+                    duplicate_ids.append(row["id"])
+                else:
+                    seen_owner_keys.add(owner_key)
 
-        import json as _json
+            for account_id in duplicate_ids:
-        import uuid as _uuid
+                conn.execute(
-        from pathlib import Path
+                    text("UPDATE email_accounts SET is_default = :value WHERE id = :id"),
-        settings_file = Path(SETTINGS_FILE)
+                    {"value": False, "id": account_id},
-        if not settings_file.exists():
+                )
-            return
+            conn.execute(text(index_ddl))
-        try:
-            s = _json.loads(settings_file.read_text(encoding="utf-8"))
-        except Exception:
-            return
 
-        imap_host = (s.get("imap_host") or "").strip()
+        if duplicate_ids:
-        smtp_host = (s.get("smtp_host") or "").strip()
+            logger.warning(
-        if not imap_host and not smtp_host:
+                "Normalized %d duplicate default email account(s) before "
-            return  # nothing to migrate
+                "installing %s",
+                len(duplicate_ids),
+                _EMAIL_ACCOUNT_DEFAULT_INDEX,
+            )
+    except Exception:
+        # Starting without the constraint would silently retain the race this
+        # migration is intended to close.  Fail startup so an operator sees and
+        # can repair an incompatible schema instead of accepting unsafe writes.
+        logger.exception("Failed to enforce the email-account default invariant")
+        raise
+
+
+def _migrate_seed_email_account():
+    """Atomically seed one legacy default account when no account exists.
+
+    Reading settings is intentionally done before taking the owner mutex.  The
+    decisive emptiness check and insert share one locked transaction, so two
+    application workers starting together cannot both seed a default row.
+    """
+    import json as _json
+    import uuid as _uuid
+
+    settings_file = Path(SETTINGS_FILE)
+    if not settings_file.exists():
+        return
+    try:
+        s = _json.loads(settings_file.read_text(encoding="utf-8"))
+    except Exception:
+        return
+
+    imap_host = (s.get("imap_host") or "").strip()
+    smtp_host = (s.get("smtp_host") or "").strip()
+    if not imap_host and not smtp_host:
+        return
+
+    db = None
+    try:
+        if not inspect(engine).has_table(EmailAccount.__tablename__):
+            return
+        db = SessionLocal()
+        lock_email_account_owner_mutations(db, "")
+        existing = db.execute(text("SELECT COUNT(*) FROM email_accounts")).scalar() or 0
+        if existing > 0:
+            return
 
         now = utcnow_naive()
-        with engine.begin() as conn:
+        db.execute(text("""
-            conn.execute(text("""
+            INSERT INTO email_accounts
-                INSERT INTO email_accounts
+              (id, owner, name, is_default, enabled,
-                  (id, owner, name, is_default, enabled,
+               imap_host, imap_port, imap_user, imap_password, imap_starttls,
-                   imap_host, imap_port, imap_user, imap_password, imap_starttls,
+               smtp_host, smtp_port, smtp_user, smtp_password,
-                   smtp_host, smtp_port, smtp_user, smtp_password,
+               from_address, created_at, updated_at)
-                   from_address, created_at, updated_at)
+            VALUES
-                VALUES
+              (:id, :owner, :name, :is_default, :enabled,
-                  (:id, :owner, :name, :is_default, :enabled,
+               :imap_host, :imap_port, :imap_user, :imap_password, :imap_starttls,
-                   :imap_host, :imap_port, :imap_user, :imap_password, :imap_starttls,
+               :smtp_host, :smtp_port, :smtp_user, :smtp_password,
-                   :smtp_host, :smtp_port, :smtp_user, :smtp_password,
+               :from_address, :created_at, :updated_at)
-                   :from_address, :created_at, :updated_at)
+        """), {
-            """), {
+            "id": _uuid.uuid4().hex,
-                "id": _uuid.uuid4().hex,
+            "owner": None,
-                "owner": None,
+            "name": "Default",
-                "name": "Default",
+            "is_default": True,
-                "is_default": True,
+            "enabled": True,
-                "enabled": True,
+            "imap_host": imap_host,
-                "imap_host": imap_host,
+            "imap_port": int(s.get("imap_port") or 993),
-                "imap_port": int(s.get("imap_port") or 993),
+            "imap_user": s.get("imap_user") or "",
-                "imap_user": s.get("imap_user") or "",
+            "imap_password": s.get("imap_password") or "",
-                "imap_password": s.get("imap_password") or "",
+            "imap_starttls": bool(s.get("imap_starttls", True)),
-                "imap_starttls": bool(s.get("imap_starttls", True)),
+            "smtp_host": smtp_host,
-                "smtp_host": smtp_host,
+            "smtp_port": int(s.get("smtp_port") or 465),
-                "smtp_port": int(s.get("smtp_port") or 465),
+            "smtp_user": s.get("smtp_user") or "",
-                "smtp_user": s.get("smtp_user") or "",
+            "smtp_password": s.get("smtp_password") or "",
-                "smtp_password": s.get("smtp_password") or "",
+            "from_address": s.get("email_from") or "",
-                "from_address": s.get("email_from") or "",
+            "created_at": now,
-                "created_at": now,
+            "updated_at": now,
-                "updated_at": now,
+        })
-            })
+        db.commit()
-            logging.getLogger(__name__).info("Seeded email_accounts 'Default' from settings.json")
+        logger.info("Seeded email_accounts 'Default' from settings.json")
     except Exception as e:
-        logging.getLogger(__name__).warning(f"seed email account migration: {e}")
+        if db is not None:
+            db.rollback()
+        logger.warning("seed email account migration: %s", e)
+    finally:
+        if db is not None:
+            db.close()
 
 
 # WARNING: Foreign-key enforcement is enabled globally for all SQLite connections.
@@ -1960,6 +2117,7 @@ def init_db():
     _migrate_add_crew_member_id()
     _migrate_add_assistant_columns()
     _migrate_add_email_smtp_security()
+    _migrate_email_account_default_invariant()
     _migrate_seed_email_account()
     _migrate_add_calendar_metadata()
     _migrate_add_calendar_is_utc()

docs/setup.md

@@ -309,6 +309,32 @@ container. Cookbook **Serve** is a separate workflow for serving downloaded
 models through Odysseus/llama.cpp, so Windows users with an existing Ollama
 install usually only need to add the endpoint in Settings.
 
+**Tool calls not firing on a manually-added Ollama `/v1` endpoint.** By
+design, a local Ollama `/v1` endpoint defaults to the conservative
+text-based (fenced-block) tool-calling path rather than native structured
+tool calls, since some locally-served models mishandle native schemas (see
+#1567). This is correct for most local setups, but if you know your specific
+model reliably supports native tool calling (check `ollama show <model>` for
+`tools` under Capabilities), you can opt that endpoint in explicitly. There
+is currently no UI control for this on manually-added endpoints (see #5192);
+the flag can still be set directly against the existing API, from a browser
+console on an authenticated admin session:
+
+```js
+fetch('/api/model-endpoints/<endpoint-id>', {
+  method: 'PATCH',
+  credentials: 'same-origin',
+  headers: {'Content-Type': 'application/json'},
+  body: JSON.stringify({supports_tools: true})
+}).then(r => r.json()).then(console.log)
+```
+
+Find `<endpoint-id>` by inspecting the `/api/model-endpoints` response (or
+your browser's network tab while Settings loads the endpoint list). Send
+`supports_tools: false` to disable native structured tool calls and force the
+conservative fenced/text path, or `supports_tools: null` to return the endpoint
+to the Auto heuristic.
+
 **Useful checks.**
 
 ```bash

routes/auth_routes.py

@@ -345,9 +345,61 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
         # docs, email accounts, tasks, etc.
         try:
             from sqlalchemy import func
-            from core.database import Base, SessionLocal
+            from core.database import (
+                Base,
+                EmailAccount,
+                SessionLocal,
+                lock_email_account_owner_mutations,
+            )
             db = SessionLocal()
             try:
+                # Email-account defaults are protected by per-owner mutex rows.
+                # A rename crosses two owner partitions, so lock both in the
+                # shared helper's canonical order before inspecting either.
+                lock_email_account_owner_mutations(
+                    db, old_username, new_username
+                )
+
+                source_default_ids = [
+                    row[0]
+                    for row in (
+                        db.query(EmailAccount.id)
+                        .filter(
+                            func.lower(EmailAccount.owner) == old_username,
+                            EmailAccount.is_default == True,  # noqa: E712
+                        )
+                        .order_by(EmailAccount.created_at.asc(), EmailAccount.id.asc())
+                        .all()
+                    )
+                ]
+                destination_default_ids = [
+                    row[0]
+                    for row in (
+                        db.query(EmailAccount.id)
+                        .filter(
+                            func.lower(EmailAccount.owner) == new_username,
+                            EmailAccount.is_default == True,  # noqa: E712
+                        )
+                        .order_by(EmailAccount.created_at.asc(), EmailAccount.id.asc())
+                        .all()
+                    )
+                ]
+                if destination_default_ids:
+                    clear_default_ids = (
+                        destination_default_ids[1:] + source_default_ids
+                    )
+                else:
+                    clear_default_ids = source_default_ids[1:]
+                if clear_default_ids:
+                    (
+                        db.query(EmailAccount)
+                        .filter(EmailAccount.id.in_(clear_default_ids))
+                        .update(
+                            {EmailAccount.is_default: False},
+                            synchronize_session=False,
+                        )
+                    )
+
                 for mapper in Base.registry.mappers:
                     model = mapper.class_
                     if not hasattr(model, "owner"):

routes/calendar_routes.py

@@ -10,6 +10,7 @@ from typing import Optional, List
 from fastapi import APIRouter, HTTPException, Request, UploadFile, File
 from pydantic import BaseModel
 from sqlalchemy import or_, and_
+from sqlalchemy.exc import IntegrityError
 from dateutil.rrule import rrulestr
 
 from core.database import SessionLocal, CalendarCal, CalendarDeletedEvent, CalendarEvent
@@ -221,22 +222,125 @@ class EventUpdate(BaseModel):
 
 # ── Helpers ──
 
+_DEFAULT_CALENDAR_NAMESPACE = uuid.UUID("4840613a-9847-4a3b-bd75-19e6bc5fc3ce")
+
+
+def _default_calendar_id(owner: str, collision_index: int = 0) -> str:
+    """Return one stable primary-key candidate for an owner's lazy default.
+
+    Slot zero preserves the original owner-derived identifier.  Later slots
+    let a username be reused after its prior calendar was migrated to another
+    owner during a rename, without making concurrent first use choose random
+    and therefore divergent identifiers.
+    """
+    if collision_index == 0:
+        candidate_name = owner
+    else:
+        candidate_name = json.dumps(
+            [owner, collision_index],
+            ensure_ascii=False,
+            separators=(",", ":"),
+        )
+    return str(uuid.uuid5(_DEFAULT_CALENDAR_NAMESPACE, candidate_name))
+
+
+def _begin_sqlite_default_write(db) -> None:
+    """Serialize an absent-default check with other SQLite writers.
+
+    SQLite's default deferred transactions allow two workers to both read an
+    empty calendar set before either writes.  ``BEGIN IMMEDIATE`` acquires the
+    writer reservation before the second, authoritative lookup.  We issue it
+    only when the driver has not already opened a write transaction; a caller
+    with a pending write already owns the required reservation.
+    """
+    connection = db.connection()
+    dbapi_connection = connection.connection
+    driver_connection = getattr(
+        dbapi_connection,
+        "driver_connection",
+        dbapi_connection,
+    )
+    if not getattr(driver_connection, "in_transaction", False):
+        connection.exec_driver_sql("BEGIN IMMEDIATE")
+
+
 def _ensure_default_calendar(db, owner: str = None) -> CalendarCal:
-    """Create default calendar if none exist for this owner."""
+    """Return the owner's calendar, staging a default in the caller's transaction.
+
+    A stable owner-derived primary key makes concurrent first-use inserts
+    converge on one row on every SQL backend.  SQLite additionally serializes
+    the absent-row check because its deferred transactions otherwise permit
+    both workers to read the gap before either writes.  Other backends recover
+    a lost insert race inside a savepoint so the caller's event transaction
+    remains usable and atomic.
+    """
     owner = owner or FALLBACK_OWNER
     cal = db.query(CalendarCal).filter(CalendarCal.owner == owner).first()
-    if not cal:
+    if cal:
+        return cal
+
+    dialect = db.get_bind().dialect.name
+    if dialect == "sqlite":
+        _begin_sqlite_default_write(db)
+        # Another worker may have committed while BEGIN IMMEDIATE waited.
+        cal = db.query(CalendarCal).filter(CalendarCal.owner == owner).first()
+        if cal:
+            return cal
+
+    collision_index = 0
+    while True:
+        default_id = _default_calendar_id(owner, collision_index)
+
+        if dialect == "sqlite":
+            # BEGIN IMMEDIATE above makes this occupancy check authoritative:
+            # another SQLite writer cannot rename, delete, or claim this slot
+            # until the caller commits or rolls back.
+            occupant = db.query(CalendarCal).filter(
+                CalendarCal.id == default_id,
+            ).first()
+            if occupant is not None:
+                if occupant.owner == owner:
+                    return occupant
+                collision_index += 1
+                continue
+
         cal = CalendarCal(
-            id=str(uuid.uuid4()),
+            id=default_id,
             owner=owner,
             name="Personal",
             color="#5b8abf",
             source="local",
         )
-        db.add(cal)
+
-        db.commit()
+        if dialect == "sqlite":
-        db.refresh(cal)
+            db.add(cal)
-    return cal
+            db.flush()
+            return cal
+
+        try:
+            # A uniqueness failure rolls back only this savepoint, not an event
+            # or reminder already staged by the caller's outer transaction.
+            with db.begin_nested():
+                db.add(cal)
+                db.flush()
+            return cal
+        except IntegrityError:
+            # Use a locking/current read so repeatable-read backends can observe
+            # the row that won after our transaction's original empty snapshot.
+            occupant = db.query(CalendarCal).filter(
+                CalendarCal.id == default_id,
+            ).with_for_update().first()
+            if occupant is None:
+                # Do not misclassify an unrelated integrity failure as an ID
+                # collision and loop forever. A concurrently deleted winner is
+                # safe for the caller to retry as a fresh transaction.
+                raise
+            if occupant.owner == owner:
+                return occupant
+            # A renamed calendar owns this deterministic slot. Advance to the
+            # next stable slot; concurrent callers for this owner will still
+            # converge there.
+            collision_index += 1
 
 
 # Per-request user time context. chat_routes sets this from browser timezone
@@ -1015,6 +1119,9 @@ def setup_calendar_routes(upload_handler=None) -> APIRouter:
         db = SessionLocal()
         try:
             _ensure_default_calendar(db, owner)
+            # Listing calendars intentionally lazily creates a durable default.
+            # Other callers commit it with the event they are creating.
+            db.commit()
             cals = db.query(CalendarCal).filter(CalendarCal.owner == owner).all()
             return {"calendars": [
                 {"name": c.name, "href": c.id, "color": c.color, "source": c.source}
@@ -1023,6 +1130,7 @@ def setup_calendar_routes(upload_handler=None) -> APIRouter:
         except HTTPException:
             raise
         except Exception as e:
+            db.rollback()
             logger.error("Failed to list calendars: %s", e)
             raise HTTPException(500, "Failed to list calendars")
         finally:

routes/chat_routes.py

@@ -21,6 +21,7 @@ from src import agent_runs
 from src.model_context import estimate_tokens
 from src.chat_helpers import coerce_message_and_session
 from src.endpoint_resolver import normalize_base as _normalize_base, build_chat_url
+from src.foreground_model_routing import build_foreground_model_candidates
 from src.session_search import search_session_messages
 from src.prompt_security import untrusted_context_message
 from core.exceptions import SessionNotFoundError
@@ -1399,14 +1400,14 @@ def setup_chat_routes(
             thinking_response = ""
             last_metrics = None
 
-            # Configured fallback chain for the default chat model. Tried in
+            # Foreground Chat and Agent requests use one owner-aware policy
-            # order if the session's primary model fails before producing
+            # boundary. Legacy `default_model_fallbacks` data is not eligible.
-            # output. Resolved once per request.
+            _foreground_candidates = build_foreground_model_candidates(
-            try:
+                sess.endpoint_url,
-                from src.endpoint_resolver import resolve_chat_fallback_candidates
+                sess.model,
-                _fallback_candidates = resolve_chat_fallback_candidates(owner=_user)
+                sess.headers,
-            except Exception:
+                owner=_user,
-                _fallback_candidates = []
+            )
 
             # Send model name early so the frontend can show it during streaming
             _model_suffix = "Research" if effective_do_research else None
@@ -1522,9 +1523,8 @@ def setup_chat_routes(
                 _actual_model = None
                 # ── Chat mode: call stream_llm directly, NO tools, NO document access ──
                 try:
-                    _chat_candidates = [(sess.endpoint_url, sess.model, sess.headers)] + _fallback_candidates
                     async for chunk in stream_llm_with_fallback(
-                        _chat_candidates,
+                        _foreground_candidates,
                         messages,
                         temperature=ctx.preset.temperature,
                         # Respect the preset; 0/unset = let the server decide (no
@@ -1710,7 +1710,7 @@ def setup_chat_routes(
                         disabled_tools=disabled_tools if disabled_tools else None,
                         tool_policy=tool_policy,
                         owner=_user,
-                        fallbacks=_fallback_candidates,
+                        fallbacks=_foreground_candidates[1:],
                         plan_mode=plan_mode,
                         approved_plan=approved_plan or None,
                         workspace=workspace or None,

routes/cookbook_routes.py

@@ -73,6 +73,30 @@ _HF_TOKEN_STATUS_SNIPPET = (
 )
 
 
+def _windows_local_pid_record_line(pid_path: Path, ready_path: Path) -> str:
+    """Build the Git Bash prelude that records a Win32-stoppable PID.
+
+    Python publishes the detached outer process's Win32 PID first, then touches
+    ``ready_path``. The inner Git Bash runner waits for that publication before
+    replacing the fallback with its own Win32 PID from /proc/<msys-pid>/winpid.
+
+    Missing, malformed, or late mappings leave the valid outer PID untouched.
+    """
+    pp = shlex.quote(pid_path.as_posix())
+    rp = shlex.quote(ready_path.as_posix())
+    return (
+        "i=0; "
+        f"while [ ! -e {rp} ] && [ \"$i\" -lt 500 ]; do "
+        "i=$((i+1)); sleep 0.01; done; "
+        f"if [ -e {rp} ]; then "
+        "winpid=\"$(cat /proc/$$/winpid 2>/dev/null || true)\"; "
+        "case \"$winpid\" in ''|*[!0-9]*) ;; "
+        f"*) printf '%s\\n' \"$winpid\" > {pp} ;; esac; "
+        "fi; "
+        f"rm -f {rp}"
+    )
+
+
 def _append_mlx_image_server_script(runner_lines: list[str]) -> None:
     """Write the MLX image API helper next to the tmux runner on remote hosts."""
     script_path = Path(__file__).resolve().parents[1] / "scripts" / "mlx_image_server.py"
@@ -978,15 +1002,18 @@ def setup_cookbook_routes() -> APIRouter:
         directly (simple commands only). Returns the launched job record."""
         log_path = TMUX_LOG_DIR / f"{session_id}.log"
         pid_path = TMUX_LOG_DIR / f"{session_id}.pid"
+        pid_ready_path: Path | None = None
         bash = find_bash()
         if bash:
             # Run the existing bash wrapper verbatim through Git Bash, redirecting
             # all output to the log the poller reads. Paths handed to bash use
             # POSIX form + shell-quoting so drive paths / spaces survive.
             inner = TMUX_LOG_DIR / f"{session_id}_run.sh"
-            pp = shlex.quote(pid_path.as_posix())
+            pid_ready_path = TMUX_LOG_DIR / f"{session_id}.pid.ready"
+            pid_ready_path.unlink(missing_ok=True)
             inner.write_text(
-                f"printf '%s\\n' \"$$\" > {pp}\n" + "\n".join(bash_lines) + "\n",
+                _windows_local_pid_record_line(pid_path, pid_ready_path) + "\n"
+                + "\n".join(bash_lines) + "\n",
                 encoding="utf-8",
             )
             lp = shlex.quote(log_path.as_posix())
@@ -1020,7 +1047,18 @@ def setup_cookbook_routes() -> APIRouter:
             env=env,
             **detached_popen_kwargs(),
         )
+        # Publish a valid Win32 ancestor first. The Git Bash runner may then
+        # replace it with its own Win32 pid, but never before this fallback exists.
         pid_path.write_text(str(proc.pid), encoding="utf-8")
+        if pid_ready_path is not None:
+            try:
+                pid_ready_path.touch()
+            except OSError as e:
+                logger.warning(
+                    "Could not publish Windows local PID handoff for %s: %s",
+                    session_id,
+                    e,
+                )
         return {"pid": proc.pid, "log_path": str(log_path)}
 
     @router.post("/api/model/download")

routes/email_routes.py

@@ -45,6 +45,7 @@ from src.upload_limits import read_upload_limited, EMAIL_COMPOSE_UPLOAD_MAX_BYTE
 
 from routes.email_helpers import (
     _strip_think, _extract_reply, _apply_email_style_mechanics, require_owner, require_user, _assert_owns_account,
+    _account_visible_to_owner,
     _q, _attach_compose_uploads, _cleanup_compose_uploads,
     _load_settings, _save_settings, _get_email_config,
     _send_smtp_message, _smtp_security_mode,
@@ -195,6 +196,64 @@ def _coerce_port(value, default):
         return None, f"Invalid port {value!r}; must be a whole number"
 
 
+def _lock_email_account_owner_mutation(db, *owners: str) -> None:
+    """Delegate account/default serialization to the shared DB primitive."""
+    from core.database import lock_email_account_owner_mutations
+
+    lock_email_account_owner_mutations(db, *owners)
+
+
+def _email_account_owner_scope(query, owner: str):
+    """Restrict a query to one normalized EmailAccount owner partition."""
+    from core.database import EmailAccount
+    from sqlalchemy import or_
+
+    if owner:
+        return query.filter(EmailAccount.owner == owner)
+    return query.filter(or_(EmailAccount.owner == None, EmailAccount.owner == ""))  # noqa: E711
+
+
+def _discover_email_account_mutation_scope(account_id: str, owner: str) -> str:
+    """Read the initial lock key and fail closed before a mutation session."""
+    from core.database import EmailAccount, SessionLocal
+
+    db = SessionLocal()
+    try:
+        row = db.get(EmailAccount, account_id)
+        if row is None or (owner and not _account_visible_to_owner(row, owner)):
+            raise HTTPException(404, "Account not found")
+        return row.owner or ""
+    except HTTPException:
+        raise
+    except Exception as exc:
+        logger.error("Account-owner mutation check failed: %s", exc)
+        raise HTTPException(503, "Account check failed")
+    finally:
+        db.close()
+
+
+def _lock_and_reload_email_account(db, account_id: str, owner: str, scope: str):
+    """Lock, reload, and revalidate an account, retrying if its owner moved."""
+    from core.database import EmailAccount
+
+    owner_scopes = {scope or ""}
+    while True:
+        _lock_email_account_owner_mutation(db, *owner_scopes)
+        row = db.get(EmailAccount, account_id, populate_existing=True)
+        if row is None or (owner and not _account_visible_to_owner(row, owner)):
+            raise HTTPException(404, "Account not found")
+
+        current_scope = row.owner or ""
+        if current_scope in owner_scopes or db.get_bind().dialect.name == "sqlite":
+            return row
+
+        # The account changed owner after discovery but before lock acquisition.
+        # Release the partial lock set and reacquire all observed scopes in the
+        # shared helper's canonical order, then validate from the database again.
+        db.rollback()
+        owner_scopes.add(current_scope)
+
+
 def _email_tag_owner_aliases(account_id: str | None, owner: str = "") -> list[str]:
     aliases = [owner or ""]
     try:
@@ -5268,9 +5327,9 @@ def setup_email_routes():
             # Build a candidate chain so a stale session-stored API key
             # (the most common cause of "authentication failed" here)
             # doesn't kill AI Reply outright — fall through to the
-            # user's Utility / Default endpoints AND their configured
+            # user's Utility / Default endpoints and the active Utility
-            # fallback chains. Dedupe by url+model so we don't retry
+            # fallback chain. The retired default-fallback hook stays empty.
-            # the same broken endpoint.
+            # Dedupe by url+model so we don't retry the same broken endpoint.
             from src.llm_core import llm_call_async_with_fallback
             from src.endpoint_resolver import (
                 resolve_utility_fallback_candidates,
@@ -5299,7 +5358,7 @@ def setup_email_routes():
                 _add(_d_url, _d_model, _d_headers)
             except Exception:
                 pass
-            # Configured fallback chains last.
+            # Active Utility fallbacks, then the retired default hook.
             for cand in resolve_utility_fallback_candidates(owner=owner) or []:
                 _add(*cand)
             for cand in resolve_chat_fallback_candidates(owner=owner) or []:
@@ -5487,9 +5546,9 @@ def setup_email_routes():
         import uuid as _uuid
         db = SessionLocal()
         try:
+            _lock_email_account_owner_mutation(db, owner)
             q = db.query(EmailAccount).filter(EmailAccount.is_default == True)  # noqa: E712
-            if owner:
+            q = _email_account_owner_scope(q, owner)
-                q = q.filter(EmailAccount.owner == owner)
             row = q.first()
             if row is None:
                 row = EmailAccount(id=_uuid.uuid4().hex, owner=owner, name="Default", is_default=True, enabled=True)
@@ -5515,8 +5574,7 @@ def setup_email_routes():
             if data.get("smtp_password"):
                 row.smtp_password = _enc(data["smtp_password"])
             clear_q = db.query(EmailAccount).filter(EmailAccount.id != row.id)
-            if owner:
+            clear_q = _email_account_owner_scope(clear_q, owner)
-                clear_q = clear_q.filter(EmailAccount.owner == owner)
             clear_q.update({EmailAccount.is_default: False})
             db.commit()
         finally:
@@ -5611,6 +5669,7 @@ def setup_email_routes():
             return {"ok": False, "error": port_err}
         db = SessionLocal()
         try:
+            _lock_email_account_owner_mutation(db, owner)
             row = EmailAccount(
                 id=_uuid.uuid4().hex,
                 name=name,
@@ -5637,9 +5696,7 @@ def setup_email_routes():
             # the one-default invariant — but scope it to THIS user's accounts,
             # otherwise creating a default would clear every other user's
             # default flag too.
-            scope_q = db.query(EmailAccount)
+            scope_q = _email_account_owner_scope(db.query(EmailAccount), owner)
-            if owner:
-                scope_q = scope_q.filter(EmailAccount.owner == owner)
             existing_count = scope_q.count()
             if row.is_default or existing_count == 0:
                 scope_q.update({EmailAccount.is_default: False})
@@ -5690,28 +5747,39 @@ def setup_email_routes():
 
     @router.delete("/accounts/{account_id}")
     async def delete_email_account(account_id: str, owner: str = Depends(require_user)):
-        _assert_owns_account(account_id, owner)
+        initial_scope = _discover_email_account_mutation_scope(account_id, owner)
         from core.database import SessionLocal, EmailAccount
         db = SessionLocal()
         try:
-            row = db.get(EmailAccount, account_id)
+            row = _lock_and_reload_email_account(
-            if not row:
+                db, account_id, owner, initial_scope
-                return {"ok": False, "error": "Account not found"}
+            )
+            row_scope = row.owner or ""
             was_default = bool(row.is_default)
             db.delete(row)
-            db.commit()
+            # Flush the removal before staging a replacement default.  The
+            # partial unique index is checked statement-by-statement, and the
+            # ORM is otherwise free to UPDATE the promoted row before DELETE.
+            db.flush()
             # If the deleted row was default, promote the next-oldest enabled
             # row owned by THIS user. Without the owner filter we'd promote
             # another user's account and the deleter would silently inherit
             # it as their default.
             if was_default:
-                promote_q = db.query(EmailAccount).filter(EmailAccount.enabled == True)  # noqa: E712
+                promote_q = db.query(EmailAccount).filter(
-                if owner:
+                    EmailAccount.id != account_id,
-                    promote_q = promote_q.filter(EmailAccount.owner == owner)
+                    EmailAccount.enabled == True,  # noqa: E712
-                promote = promote_q.order_by(EmailAccount.created_at.asc()).first()
+                )
+                promote_q = _email_account_owner_scope(promote_q, row_scope)
+                promote = promote_q.order_by(
+                    EmailAccount.created_at.asc(), EmailAccount.id.asc()
+                ).first()
                 if promote:
                     promote.is_default = True
-                    db.commit()
+            # Deletion and any replacement promotion are one durable state
+            # transition, so another worker can never observe or race the old
+            # split-commit gap.
+            db.commit()
             return {"ok": True}
         finally:
             db.close()
@@ -5924,18 +5992,18 @@ def setup_email_routes():
 
     @router.post("/accounts/{account_id}/set-default")
     async def set_default_account(account_id: str, owner: str = Depends(require_user)):
-        _assert_owns_account(account_id, owner)
+        initial_scope = _discover_email_account_mutation_scope(account_id, owner)
         from core.database import SessionLocal, EmailAccount
         db = SessionLocal()
         try:
-            row = db.get(EmailAccount, account_id)
+            row = _lock_and_reload_email_account(
-            if not row:
+                db, account_id, owner, initial_scope
-                return {"ok": False, "error": "Account not found"}
+            )
-            # SECURITY: scope the "clear other defaults" sweep to this user's
+            # Scope the sweep to the target row's normalized owner partition;
-            # accounts so we don't unset another user's default flag.
+            # this also handles visible legacy NULL/empty-owner accounts.
-            clear_q = db.query(EmailAccount)
+            clear_q = _email_account_owner_scope(
-            if owner:
+                db.query(EmailAccount), row.owner or ""
-                clear_q = clear_q.filter(EmailAccount.owner == owner)
+            )
             clear_q.update({EmailAccount.is_default: False})
             row.is_default = True
             db.commit()
@@ -5954,7 +6022,7 @@ def setup_email_routes():
             raise HTTPException(400, "GOOGLE_OAUTH_CLIENT_ID not set — add it to .env")
         redirect_uri = (
             os.environ.get("GOOGLE_OAUTH_REDIRECT_URI")
-            or f"http://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback"
+            or f"{request.url.scheme}://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback"
         )
         state = make_oauth_state(account_id, owner)
         params = urllib.parse.urlencode({
@@ -5991,7 +6059,7 @@ def setup_email_routes():
         client_secret = os.environ.get("GOOGLE_OAUTH_CLIENT_SECRET", "")
         redirect_uri = (
             os.environ.get("GOOGLE_OAUTH_REDIRECT_URI")
-            or f"http://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback"
+            or f"{request.url.scheme}://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback"
         )
         import httpx as _httpx
         try:

routes/gallery/gallery_routes.py

@@ -127,6 +127,25 @@ def _load_grounding_backend():
     return cached
 
 
+def _model_input_to_device(value, device: str, torch):
+    if not hasattr(value, "to"):
+        return value
+    if (
+        device == "mps"
+        and hasattr(torch, "float64")
+        and getattr(value, "dtype", None) == torch.float64
+    ):
+        return value.to(device=device, dtype=torch.float32)
+    return value.to(device)
+
+
+def _model_inputs_to_device(inputs, device: str, torch) -> Dict[str, Any]:
+    return {
+        key: _model_input_to_device(value, device, torch)
+        for key, value in inputs.items()
+    }
+
+
 def _ground_text_to_box(image, text: str, *, threshold: float = 0.05):
     query = (text or "").strip()
     if not query:
@@ -142,10 +161,7 @@ def _ground_text_to_box(image, text: str, *, threshold: float = 0.05):
         labels.append(f"a photo of {query}")
     try:
         inputs = processor(text=[labels], images=image, return_tensors="pt")
-        model_inputs = {
+        model_inputs = _model_inputs_to_device(inputs, device, torch)
-            k: (v.to(device) if hasattr(v, "to") else v)
-            for k, v in inputs.items()
-        }
         with torch.no_grad():
             outputs = model(**model_inputs)
         target_sizes = torch.tensor([[image.height, image.width]])
@@ -1869,10 +1885,7 @@ def setup_gallery_routes() -> APIRouter:
 
         try:
             inputs = processor(image, **kwargs)
-            model_inputs = {
+            model_inputs = _model_inputs_to_device(inputs, device, torch)
-                k: (v.to(device) if hasattr(v, "to") else v)
-                for k, v in inputs.items()
-            }
             with torch.no_grad():
                 outputs = model(**model_inputs)
             masks = processor.image_processor.post_process_masks(

routes/model_routes.py

@@ -46,10 +46,11 @@ _ENDPOINT_SETTING_FIELDS = {
 }
 
 _ENDPOINT_FALLBACK_FIELDS = {
-    "default_model_fallbacks": "Default Model Fallbacks",
     "utility_model_fallbacks": "Utility Model Fallbacks",
     "vision_model_fallbacks":  "Vision Model Fallbacks",
 }
+# `default_model_fallbacks` is intentionally absent. The legacy data remains
+# stored as-is even when an endpoint is removed, but no longer affects routing.
 
 
 def _speech_settings_using_endpoint(settings: dict, ep_id: str) -> list:
@@ -2437,7 +2438,6 @@ def setup_model_routes(model_discovery):
             _user_prefs = _load_for_user(_user) or {}
             ep_id = (_user_prefs.get("default_endpoint_id") or "").strip()
             model = (_user_prefs.get("default_model") or "").strip()
-            _fallbacks = _user_prefs.get("default_model_fallbacks") or []
             # If user has no personal default, fall back to global default
             # But only based on the "share_defaults_with_users" flag
             # (only if share_defaults_with_users is enabled)
@@ -2446,12 +2446,9 @@ def setup_model_routes(model_discovery):
                     ep_id = settings.get("default_endpoint_id", "")
                 if not model:
                     model = settings.get("default_model", "")
-                if not _fallbacks:
-                    _fallbacks = settings.get("default_model_fallbacks") or []
         else:
             ep_id = settings.get("default_endpoint_id", "")
             model = settings.get("default_model", "")
-            _fallbacks = settings.get("default_model_fallbacks") or []
         db = SessionLocal()
         try:
             ep = None
@@ -2466,33 +2463,6 @@ def setup_model_routes(model_discovery):
                 if _user and not _is_admin:
                     ep_q = owner_filter(ep_q, ModelEndpoint, _user)
                 ep = ep_q.first()
-            # Configured fallback chain — when the chosen default endpoint is
-            # gone/disabled, honor the user's configured `default_model_fallbacks`
-            # in order BEFORE arbitrarily grabbing the first enabled endpoint.
-            # (Previously this jumped straight to "first enabled", which is why
-            # deleting/changing the main endpoint silently reassigned the default
-            # chat to some unrelated endpoint instead of the fallback.)
-            if not ep:
-                for entry in _fallbacks:
-                    if not isinstance(entry, dict):
-                        continue
-                    fid = (entry.get("endpoint_id") or "").strip()
-                    if not fid:
-                        continue
-                    cand_q = db.query(ModelEndpoint).filter(
-                        ModelEndpoint.id == fid, ModelEndpoint.is_enabled == True
-                    )
-                    if _user and not _is_admin:
-                        cand_q = owner_filter(cand_q, ModelEndpoint, _user)
-                    cand = cand_q.first()
-                    if cand:
-                        ep = cand
-                        # Use the fallback entry's model. Reset even when empty
-                        # so we don't carry the prior endpoint's stale model onto
-                        # this fallback — the cached-models lookup below then
-                        # fills it from the fallback endpoint.
-                        model = (entry.get("model") or "").strip()
-                        break
             # Last resort: first enabled endpoint owned by THIS user. Do not
             # include null-owner/shared endpoints here: a brand-new user with
             # no explicit default should not auto-open a pending chat using an

routes/personal_routes.py

@@ -1,11 +1,13 @@
 # routes/personal_routes.py
 """Routes for personal documents management."""
+import asyncio
 import os
 import logging
 import shutil
 import uuid
 from typing import Any, Dict, List, Tuple
 from fastapi import APIRouter, HTTPException, Query, Request, UploadFile, File, Depends
+from fastapi.concurrency import run_in_threadpool
 from src.request_models import DirectoryRequest
 from core.constants import BASE_DIR, PERSONAL_DIR, PERSONAL_UPLOADS_DIR
 from src.rag_singleton import get_rag_manager
@@ -18,7 +20,6 @@ UPLOADS_DIR = PERSONAL_UPLOADS_DIR
 
 logger = logging.getLogger(__name__)
 
-
 def _personal_upload_dir_for_owner(owner: str | None, *, create: bool = True) -> str:
     """Return the per-owner upload directory used for direct RAG uploads."""
     owner_segment = secure_filename((owner or "local").strip())[:80] or "local"
@@ -141,6 +142,22 @@ def setup_personal_routes(personal_docs_manager, rag_manager, rag_available):
     """
     router = APIRouter(prefix="/api/personal")
 
+    # Serializes directory index jobs across requests. Indexing runs in the
+    # threadpool (#5558), so concurrent requests would otherwise run in parallel
+    # and race PersonalDocsManager's unsynchronized list mutations and file
+    # writes; before the threadpool move they serialized on the blocked event
+    # loop, so one-at-a-time is behavior parity.
+    #
+    # An asyncio.Lock acquired in the async handler BEFORE offloading: a waiting
+    # request parks on the event loop instead of pinning a threadpool worker (an
+    # earlier threading.Lock taken INSIDE the worker meant queued jobs held pool
+    # tokens while blocked, starving every other run_in_threadpool caller).
+    # add/remove/reload all take this lock, so their mutations never interleave.
+    # Per-router (not module-global) so each app binds it to its own event loop.
+    # Scope is the single process: multi-worker deployments would need a shared
+    # lock (out of scope for #5558).
+    _index_job_lock = asyncio.Lock()
+
     def _rag():
         """Get the current RAG manager, retrying init if needed."""
         return get_rag_manager()
@@ -172,8 +189,12 @@ def setup_personal_routes(personal_docs_manager, rag_manager, rag_available):
         return {"files": files, "directories": directories}
     
     @router.post("/reload")
-    def api_personal_reload(owner: str = Depends(require_user), _admin: None = Depends(require_admin)):
+    async def api_personal_reload(owner: str = Depends(require_user), _admin: None = Depends(require_admin)):
-        personal_docs_manager.refresh_index()
+        # refresh_index() re-extracts text across every tracked directory —
+        # blocking work. Take the shared job lock (so it cannot race an add /
+        # remove) and run it off the event loop.
+        async with _index_job_lock:
+            await run_in_threadpool(personal_docs_manager.refresh_index)
         return {"ok": True, "count": len(personal_docs_manager.index)}
     
     @router.post("/add_directory")
@@ -207,12 +228,26 @@ def setup_personal_routes(personal_docs_manager, rag_manager, rag_available):
             # Use the RAGManager to index the directory
             rag = _rag()
             if rag:
-                result = rag.index_personal_documents(directory, owner=owner)
+                def _index_directory():
-                
+                    result = rag.index_personal_documents(directory, owner=owner)
+                    if result["success"]:
+                        # Also update the personal_docs_manager to track this
+                        # directory. Kept inside the offloaded call: it triggers
+                        # refresh_index(), which re-extracts text across tracked
+                        # directories.
+                        personal_docs_manager.add_directory(directory, index=False)
+                    return result
+
+                # Indexing walks, embeds, and stores the whole tree — minutes
+                # on a real directory. The handler is async, so calling it
+                # inline runs it on the event loop and every other request
+                # queues behind it until it finishes (#5558). Serialize on the
+                # async job lock BEFORE offloading so a queued request parks on
+                # the loop instead of pinning a threadpool worker.
+                async with _index_job_lock:
+                    result = await run_in_threadpool(_index_directory)
+
                 if result["success"]:
-                    # Also update the personal_docs_manager to track this directory
-                    personal_docs_manager.add_directory(directory, index=False)
-                    
                     return {
                         "success": True,
                         "message": f"Successfully indexed {result['indexed_count']} chunks from {directory}",
@@ -251,17 +286,25 @@ def setup_personal_routes(personal_docs_manager, rag_manager, rag_available):
 
             logger.info(f"Removing directory from RAG: {directory}")
 
-            # Always remove from personal_docs_manager tracking
-            if hasattr(personal_docs_manager, 'remove_directory'):
-                personal_docs_manager.remove_directory(directory)
-
-            # Remove from RAG vector store (best-effort)
             rag = _rag()
-            if rag:
+
-                try:
+            def _remove_directory():
-                    rag.remove_directory(directory)
+                # Always remove from personal_docs_manager tracking. This
-                except Exception as e:
+                # mutates the same unsynchronized list/index an add job touches
-                    logger.warning(f"RAG removal failed for directory {directory}: {e}")
+                # and re-extracts text (refresh_index), so it is blocking work.
+                if hasattr(personal_docs_manager, 'remove_directory'):
+                    personal_docs_manager.remove_directory(directory)
+                # Remove from RAG vector store (best-effort).
+                if rag:
+                    try:
+                        rag.remove_directory(directory)
+                    except Exception as e:
+                        logger.warning(f"RAG removal failed for directory {directory}: {e}")
+
+            # Same job lock as add/reload so remove cannot interleave with an
+            # in-flight add; offloaded off the event loop.
+            async with _index_job_lock:
+                await run_in_threadpool(_remove_directory)
 
             return {
                 "success": True,
@@ -289,54 +332,73 @@ def setup_personal_routes(personal_docs_manager, rag_manager, rag_available):
         total_failed = 0
         uploaded_files = []
 
-        for upload in files:
+        # Chunking, embedding and the tracking update are blocking work over the
-            try:
+        # same vector/tracking state add_directory mutates (#5634). Take the
-                file_path, stored_name, safe_name = _unique_personal_upload_path(upload_dir, upload.filename)
+        # shared job lock BEFORE offloading so a queued request parks on the loop
-                content_bytes = await upload.read(PERSONAL_UPLOAD_MAX_BYTES + 1)
+        # instead of pinning a threadpool worker, matching add_directory.
-                if len(content_bytes) > PERSONAL_UPLOAD_MAX_BYTES:
+        # Read and process one capped payload at a time so a multi-file request
-                    logger.warning(f"Rejected oversized personal upload: {upload.filename!r}")
+        # cannot retain len(files) * PERSONAL_UPLOAD_MAX_BYTES in memory.
-                    total_failed += 1
+        async with _index_job_lock:
-                    continue
+            for upload in files:
-                with open(file_path, "wb") as f:
+                try:
-                    f.write(content_bytes)
+                    file_path, stored_name, safe_name = _unique_personal_upload_path(
-
+                        upload_dir, upload.filename
-                ext = os.path.splitext(safe_name)[1].lower()
+                    )
-                if ext == ".pdf":
+                    content_bytes = await upload.read(PERSONAL_UPLOAD_MAX_BYTES + 1)
-                    from src.personal_docs import extract_pdf_text
+                    if len(content_bytes) > PERSONAL_UPLOAD_MAX_BYTES:
-                    text = extract_pdf_text(file_path)
+                        logger.warning(f"Rejected oversized personal upload: {upload.filename!r}")
-                else:
-                    text = content_bytes.decode("utf-8", errors="replace")
-
-                if not text or not text.strip():
-                    total_failed += 1
-                    continue
-
-                # Chunk and index
-                chunks = rag._split_into_chunks(text, chunk_size=500)
-                for i, chunk in enumerate(chunks):
-                    metadata = {
-                        "source": file_path,
-                        "filename": safe_name,
-                        "stored_filename": stored_name,
-                        "directory": upload_dir,
-                        "type": ext,
-                        "chunk_id": i,
-                    }
-                    if user:
-                        metadata["owner"] = user
-                    if rag.add_document(chunk, metadata):
-                        total_indexed += 1
-                    else:
                         total_failed += 1
+                        continue
 
-                uploaded_files.append(safe_name)
+                    def _index_upload():
-            except Exception as e:
+                        with open(file_path, "wb") as f:
-                logger.error(f"Failed to upload/index {upload.filename}: {e}")
+                            f.write(content_bytes)
-                total_failed += 1
 
-        # Track uploads directory
+                        ext = os.path.splitext(safe_name)[1].lower()
-        if uploaded_files and hasattr(personal_docs_manager, "add_directory"):
+                        if ext == ".pdf":
-            personal_docs_manager.add_directory(upload_dir, index=False)
+                            from src.personal_docs import extract_pdf_text
+                            text = extract_pdf_text(file_path)
+                        else:
+                            text = content_bytes.decode("utf-8", errors="replace")
+
+                        if not text or not text.strip():
+                            return 0, 1, None
+
+                        indexed = 0
+                        failed = 0
+                        chunks = rag._split_into_chunks(text, chunk_size=500)
+                        for i, chunk in enumerate(chunks):
+                            metadata = {
+                                "source": file_path,
+                                "filename": safe_name,
+                                "stored_filename": stored_name,
+                                "directory": upload_dir,
+                                "type": ext,
+                                "chunk_id": i,
+                            }
+                            if user:
+                                metadata["owner"] = user
+                            if rag.add_document(chunk, metadata):
+                                indexed += 1
+                            else:
+                                failed += 1
+                        return indexed, failed, safe_name
+
+                    indexed, failed, uploaded_name = await run_in_threadpool(_index_upload)
+                    total_indexed += indexed
+                    total_failed += failed
+                    if uploaded_name:
+                        uploaded_files.append(uploaded_name)
+                except Exception as e:
+                    logger.error(f"Failed to upload/index {upload.filename}: {e}")
+                    total_failed += 1
+
+            # Same transition, same lock: the tracking update must not land
+            # while another job is mid-write over the same state.
+            if uploaded_files and hasattr(personal_docs_manager, "add_directory"):
+                await run_in_threadpool(
+                    personal_docs_manager.add_directory, upload_dir, index=False
+                )
 
         return {
             "success": True,
@@ -349,38 +411,47 @@ def setup_personal_routes(personal_docs_manager, rag_manager, rag_available):
     async def delete_file_from_rag(filepath: str = Query(...), owner: str = Depends(require_user), _admin: None = Depends(require_admin)):
         """Delete a specific file from RAG index and optionally from disk."""
         try:
-            # Remove chunks from RAG vector store (best-effort)
+            def _delete_file():
-            removed = 0
+                # Remove chunks from RAG vector store (best-effort)
-            rag = _rag()
+                removed = 0
-            if rag:
+                rag = _rag()
-                try:
+                if rag:
-                    removed = rag.delete_by_source(filepath)
+                    try:
-                except Exception as e:
+                        removed = rag.delete_by_source(filepath)
-                    logger.warning(f"RAG removal failed for {filepath}: {e}")
+                    except Exception as e:
+                        logger.warning(f"RAG removal failed for {filepath}: {e}")
 
-            # Delete file from disk if it's in the caller's own uploads dir.
+                # Delete file from disk if it's in the caller's own uploads dir.
-            # Scope to the per-owner subdir, not the shared uploads root, so one
+                # Scope to the per-owner subdir, not the shared uploads root, so one
-            # admin can't delete another user's personal files by path.
+                # admin can't delete another user's personal files by path.
-            deleted_from_disk = False
+                deleted_from_disk = False
-            try:
-                abs_target = os.path.realpath(filepath)
-                base_abs = os.path.realpath(_personal_upload_dir_for_owner(owner, create=False))
-                in_uploads = (
-                    abs_target == base_abs
-                    or os.path.commonpath([abs_target, base_abs]) == base_abs
-                )
-            except ValueError:
-                # commonpath raises on mixed drives / non-comparable paths
-                in_uploads = False
-            if in_uploads and abs_target != base_abs:
                 try:
-                    os.remove(abs_target)
+                    abs_target = os.path.realpath(filepath)
-                    deleted_from_disk = True
+                    base_abs = os.path.realpath(_personal_upload_dir_for_owner(owner, create=False))
-                except FileNotFoundError:
+                    in_uploads = (
-                    pass  # already gone — race with another request or cleanup
+                        abs_target == base_abs
+                        or os.path.commonpath([abs_target, base_abs]) == base_abs
+                    )
+                except ValueError:
+                    # commonpath raises on mixed drives / non-comparable paths
+                    in_uploads = False
+                if in_uploads and abs_target != base_abs:
+                    try:
+                        os.remove(abs_target)
+                        deleted_from_disk = True
+                    except FileNotFoundError:
+                        pass  # already gone — race with another request or cleanup
 
-            # Exclude the file from the listing (persists across restarts)
+                # Exclude the file from the listing (persists across restarts)
-            personal_docs_manager.exclude_file(filepath)
+                personal_docs_manager.exclude_file(filepath)
+                return removed, deleted_from_disk
+
+            # Vector removal, the disk unlink and the exclusion write are one
+            # transition over the same state add_directory mutates (#5634), and
+            # all three block. Take the shared job lock BEFORE offloading, as
+            # add_directory does.
+            async with _index_job_lock:
+                removed, deleted_from_disk = await run_in_threadpool(_delete_file)
 
             return {
                 "success": True,

scripts/demo_email/demo_account.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-"""Create/remove the switchable, non-default 'Demo' EmailAccount in Odysseus.
+"""Create/remove the switchable 'Demo' EmailAccount in Odysseus.
 
 Mirrors the existing local-Dovecot account (localhost:31143, STARTTLS) but points
 at the throwaway demo@odysseus.local mailbox. Password is stored Fernet-encrypted
@@ -20,7 +20,14 @@ from pathlib import Path
 ROOT = Path(__file__).resolve().parent.parent.parent
 sys.path.insert(0, str(ROOT))
 
-from core.database import SessionLocal, EmailAccount, Base, engine  # noqa: E402
+from core.database import (  # noqa: E402
+    Base,
+    EmailAccount,
+    SessionLocal,
+    engine,
+    lock_email_account_owner_mutations,
+)
+from sqlalchemy import or_  # noqa: E402
 from src.secret_storage import encrypt  # noqa: E402
 
 NAME = "Demo"
@@ -31,18 +38,98 @@ IMAP_PASSWORD = "demodemo"
 OWNER = ""
 
 
-def setup() -> int:
+def _owner_scope(query, owner: str):
-    Base.metadata.create_all(bind=engine)
+    if owner:
+        return query.filter(EmailAccount.owner == owner)
+    return query.filter(or_(EmailAccount.owner == None, EmailAccount.owner == ""))  # noqa: E711
+
+
+def _discover_demo_scopes() -> set[str]:
     db = SessionLocal()
     try:
-        acct = db.query(EmailAccount).filter(
+        return {
-            EmailAccount.name == NAME, EmailAccount.imap_user == IMAP_USER
+            row.owner or ""
-        ).first()
+            for row in db.query(EmailAccount).filter(
+                EmailAccount.name == NAME,
+                EmailAccount.imap_user == IMAP_USER,
+            ).all()
+        }
+    finally:
+        db.close()
+
+
+def _lock_and_load_demo_rows(db, scopes: set[str]):
+    """Reload Demo rows under every observed owner lock."""
+    scopes = set(scopes) or {OWNER}
+    while True:
+        lock_email_account_owner_mutations(db, *scopes)
+        rows = (
+            db.query(EmailAccount)
+            .filter(
+                EmailAccount.name == NAME,
+                EmailAccount.imap_user == IMAP_USER,
+            )
+            .order_by(EmailAccount.created_at.asc(), EmailAccount.id.asc())
+            .all()
+        )
+        current_scopes = {row.owner or "" for row in rows}
+        if current_scopes.issubset(scopes) or db.get_bind().dialect.name == "sqlite":
+            return rows
+        db.rollback()
+        scopes.update(current_scopes)
+
+
+def _promote_oldest_enabled(db, owner: str, excluded_ids: list[str]) -> None:
+    remaining = _owner_scope(
+        db.query(EmailAccount).filter(
+            EmailAccount.enabled == True,  # noqa: E712
+            ~EmailAccount.id.in_(excluded_ids),
+        ),
+        owner,
+    )
+    if remaining.filter(EmailAccount.is_default == True).first() is not None:  # noqa: E712
+        return
+    promote = remaining.order_by(
+        EmailAccount.created_at.asc(), EmailAccount.id.asc()
+    ).first()
+    if promote is not None:
+        promote.is_default = True
+
+
+def setup() -> int:
+    Base.metadata.create_all(bind=engine)
+    scopes = _discover_demo_scopes() | {OWNER}
+    db = SessionLocal()
+    try:
+        rows = _lock_and_load_demo_rows(db, scopes)
+        acct = rows[0] if rows else None
         if acct is None:
             acct = EmailAccount(id=uuid.uuid4().hex, name=NAME)
             db.add(acct)
+        old_scope = acct.owner or ""
+        was_default = bool(acct.is_default)
+        if old_scope != OWNER:
+            # Move a non-default row first so the unique index cannot see two
+            # defaults transiently while SQLAlchemy flushes the owner move and
+            # old-scope promotion in separate UPDATE statements.
+            acct.is_default = False
+            acct.owner = OWNER
+            db.flush()
+            if was_default:
+                _promote_oldest_enabled(db, old_scope, [acct.id])
+
+        target_default = _owner_scope(
+            db.query(EmailAccount).filter(
+                EmailAccount.id != acct.id,
+                EmailAccount.is_default == True,  # noqa: E712
+            ),
+            OWNER,
+        ).first()
         acct.owner = OWNER
-        acct.is_default = False          # never default — user switches to it
+        # Keep Demo non-default when a real default exists.  If it is the only
+        # enabled account, it must be default to preserve normal create
+        # semantics and avoid leaving the owner partition without one.
+        acct.is_default = target_default is None
         acct.enabled = True
         acct.imap_host = "localhost"
         acct.imap_port = 31143
@@ -57,20 +144,27 @@ def setup() -> int:
         acct.smtp_password = encrypt(IMAP_PASSWORD)
         acct.from_address = IMAP_USER
         db.commit()
-        print(f"'{NAME}' account ready (id={acct.id}, non-default, switchable).")
+        state = "default" if acct.is_default else "non-default"
+        print(f"'{NAME}' account ready (id={acct.id}, {state}, switchable).")
         return 0
     finally:
         db.close()
 
 
 def teardown() -> int:
+    scopes = _discover_demo_scopes()
     db = SessionLocal()
     try:
-        rows = db.query(EmailAccount).filter(
+        rows = _lock_and_load_demo_rows(db, scopes)
-            EmailAccount.name == NAME, EmailAccount.imap_user == IMAP_USER
+        deleted_ids = [row.id for row in rows]
-        ).all()
+        default_scopes = {row.owner or "" for row in rows if row.is_default}
         for r in rows:
             db.delete(r)
+        # Ensure the old default DELETE reaches the database before a
+        # replacement UPDATE; the unique index is enforced per statement.
+        db.flush()
+        for owner in default_scopes:
+            _promote_oldest_enabled(db, owner, deleted_ids)
         db.commit()
         print(f"removed {len(rows)} '{NAME}' account row(s).")
         return 0

services/memory/skill_format.py

@@ -100,6 +100,18 @@ def _parse_scalar(raw: str) -> Any:
     if raw.lower() in ("null", "none", "~"):
         return None
     if (raw[0] == raw[-1]) and raw[0] in ("'", '"'):
+        if raw[0] == '"':
+            # _emit_scalar writes double-quoted scalars with json.dumps, so
+            # decode the escapes instead of only stripping the quotes. Without
+            # this, `\"` / `\\` / `\uXXXX` stayed verbatim in the value and the
+            # next save escaped their backslashes again, doubling them on every
+            # load/save cycle (issue #5210).
+            try:
+                return json.loads(raw)
+            except ValueError:
+                # Hand-written file using escapes JSON rejects (e.g. a bare
+                # Windows path). Keep the previous literal reading.
+                pass
         return raw[1:-1]
     # Try number
     try:
@@ -171,6 +183,26 @@ def parse_frontmatter(text: str) -> tuple[Dict[str, Any], str]:
     return fm, body
 
 
+# Characters that force a quoted scalar. The punctuation would otherwise change
+# how the value reads back; the second row is every character str.splitlines()
+# treats as a line break, and parse_frontmatter() reads one scalar per line, so
+# emitting one of those bare would split the value across lines.
+_FM_MUST_QUOTE = (
+    ":", "#", "[", "]", "{", "}", ",", "&", "*", "!", "|", ">", "'", '"', "%", "@",
+    "\n", "\r", "\v", "\f", "\x1c", "\x1d", "\x1e", "\x85", "\u2028", "\u2029",
+)
+
+# json.dumps escapes every C0 control character, but with ensure_ascii=False it
+# passes NEL / LINE SEPARATOR / PARAGRAPH SEPARATOR through literally, and
+# str.splitlines() still breaks on all three. Re-escape exactly those, which
+# json.loads decodes again on the way in, so the pair stays symmetric.
+_FM_POST_DUMPS_ESCAPES = (
+    ("\x85", "\\u0085"),
+    ("\u2028", "\\u2028"),
+    ("\u2029", "\\u2029"),
+)
+
+
 def _emit_scalar(v: Any) -> str:
     if v is None:
         return "null"
@@ -181,8 +213,15 @@ def _emit_scalar(v: Any) -> str:
     if isinstance(v, list):
         return "[" + ", ".join(_emit_scalar(x) for x in v) + "]"
     s = str(v)
-    if any(c in s for c in (":", "#", "\n", "[", "]", "{", "}", ",", "&", "*", "!", "|", ">", "'", '"', "%", "@")):
+    if any(c in s for c in _FM_MUST_QUOTE):
-        return json.dumps(s)
+        # ensure_ascii=False keeps non-ASCII text as itself. SKILL.md is UTF-8 at
+        # both ends (skills.py reads it, atomic_write_text writes it), so the
+        # \uXXXX form bought nothing and leaked into the parsed value (#5210).
+        out = json.dumps(s, ensure_ascii=False)
+        for ch, esc in _FM_POST_DUMPS_ESCAPES:
+            if ch in out:
+                out = out.replace(ch, esc)
+        return out
     return s
 
 

src/builtin_actions.py

@@ -20,6 +20,395 @@ from src.interactive_gate import wait_for_interactive_quiet
 logger = logging.getLogger(__name__)
 
 
+def _read_email_urgency_state(state_path):
+    """Read one atomic urgency checkpoint, tolerating the legacy shape."""
+    from pathlib import Path
+
+    state_path = Path(state_path)
+    try:
+        state = (
+            json.loads(state_path.read_text(encoding="utf-8"))
+            if state_path.exists()
+            else {}
+        )
+    except Exception:
+        return {}
+    return state if isinstance(state, dict) else {}
+
+
+def _email_urgency_account_generations(state):
+    """Return normalized per-account checkpoint/complete generations.
+
+    Checkpoint generations fence every accepted state mutation. Complete
+    generations advance only for a non-stale complete scan. Missing metadata
+    is the legacy generation zero.
+    """
+    raw = state.get("account_generations", {}) if isinstance(state, dict) else {}
+    if not isinstance(raw, dict):
+        return {}
+
+    generations = {}
+    for account_id, value in raw.items():
+        if isinstance(value, dict):
+            checkpoint = value.get("checkpoint", 0)
+            complete = value.get("complete", 0)
+        else:
+            # Tolerate an intermediate scalar representation as one completed
+            # checkpoint generation instead of discarding its fence.
+            checkpoint = value
+            complete = value
+        try:
+            checkpoint = max(0, int(checkpoint))
+        except (TypeError, ValueError):
+            checkpoint = 0
+        try:
+            complete = max(0, int(complete))
+        except (TypeError, ValueError):
+            complete = 0
+        generations[str(account_id)] = {
+            "checkpoint": checkpoint,
+            "complete": complete,
+        }
+    return generations
+
+
+def _email_urgency_string_set(value):
+    if not isinstance(value, (list, tuple, set, frozenset)):
+        return set()
+    return {str(item) for item in value if isinstance(item, (str, int))}
+
+
+def _acquire_email_urgency_state_lock(
+    state_path,
+    lock_db_path,
+    cancel_event,
+    timeout_seconds=120,
+):
+    """Acquire the cross-process urgency lock without blocking the app loop."""
+    import sqlite3
+    import time
+    from pathlib import Path
+
+    state_path = Path(state_path)
+    state_path.parent.mkdir(parents=True, exist_ok=True)
+    deadline = time.monotonic() + timeout_seconds
+
+    while not cancel_event.is_set():
+        remaining = deadline - time.monotonic()
+        if remaining <= 0:
+            raise sqlite3.OperationalError("timed out waiting for urgency state lock")
+        conn = sqlite3.connect(
+            str(lock_db_path),
+            timeout=min(0.25, max(0.01, remaining)),
+            check_same_thread=False,
+        )
+        try:
+            conn.execute("BEGIN IMMEDIATE")
+        except sqlite3.OperationalError as exc:
+            conn.close()
+            if "locked" not in str(exc).lower():
+                raise
+            cancel_event.wait(min(0.05, max(0.0, remaining)))
+            continue
+        except BaseException:
+            conn.close()
+            raise
+
+        if cancel_event.is_set():
+            conn.rollback()
+            conn.close()
+            return None, None
+        return conn, _read_email_urgency_state(state_path)
+
+    return None, None
+
+
+def _close_email_urgency_state_lock(conn):
+    if conn is None:
+        return
+    try:
+        try:
+            conn.rollback()
+        except Exception:
+            pass
+    finally:
+        conn.close()
+
+
+def _commit_email_urgency_state(conn, state_path, next_state):
+    """Atomically publish JSON before releasing the SQLite write lock."""
+    import uuid
+    from pathlib import Path
+
+    state_path = Path(state_path)
+    temp_path = state_path.with_name(
+        f".{state_path.name}.{uuid.uuid4().hex}.tmp"
+    )
+    try:
+        temp_path.write_text(json.dumps(next_state), encoding="utf-8")
+        temp_path.replace(state_path)
+        conn.commit()
+    except BaseException:
+        conn.rollback()
+        raise
+    finally:
+        temp_path.unlink(missing_ok=True)
+        conn.close()
+
+
+async def _run_email_urgency_state_transaction(
+    state_path,
+    lock_db_path,
+    operation,
+):
+    """Serialize one urgency decision while keeping async work on this loop.
+
+    Only lock acquisition waits in a worker thread. ``operation`` is awaited
+    on the caller's long-lived event loop, where shared async clients, locks,
+    and the browser-notification queue belong. Cancellation rolls back the
+    SQLite transaction and never publishes a checkpoint.
+    """
+    import asyncio
+    import threading
+
+    loop = asyncio.get_running_loop()
+    cancel_event = threading.Event()
+    acquire_future = loop.run_in_executor(
+        None,
+        _acquire_email_urgency_state_lock,
+        state_path,
+        lock_db_path,
+        cancel_event,
+    )
+    try:
+        conn, prior = await asyncio.shield(acquire_future)
+    except asyncio.CancelledError as cancelled:
+        cancel_event.set()
+        # The acquisition worker owns any connection until it returns. Wait
+        # for its short busy-poll to observe cancellation, then close a lock it
+        # may have won concurrently with the cancellation request.
+        while True:
+            try:
+                conn, _prior = await asyncio.shield(acquire_future)
+                break
+            except asyncio.CancelledError:
+                continue
+            except Exception:
+                conn = None
+                break
+        _close_email_urgency_state_lock(conn)
+        raise cancelled
+
+    if conn is None:
+        raise asyncio.CancelledError
+
+    try:
+        result, next_state = await operation(prior)
+        # Keep this small atomic publish synchronous. There is no await between
+        # the successful operation and commit, so cancellation cannot be
+        # observed and then followed by a checkpoint.
+        try:
+            _commit_email_urgency_state(conn, state_path, next_state)
+        finally:
+            conn = None
+        return result
+    except BaseException:
+        _close_email_urgency_state_lock(conn)
+        raise
+
+
+def _email_urgency_account_key(message_key):
+    return str(message_key).split(":", 1)[0]
+
+
+def _email_urgency_payload_account_ids(state):
+    """Return account IDs that still own user-visible urgency payload."""
+    if not isinstance(state, dict):
+        return set()
+
+    per_uid = state.get("per_uid", {})
+    per_uid_keys = per_uid if isinstance(per_uid, dict) else {}
+    return {
+        _email_urgency_account_key(key) for key in per_uid_keys
+    } | {
+        _email_urgency_account_key(key)
+        for key in _email_urgency_string_set(state.get("notified_uids", []))
+    }
+
+
+def _email_urgency_known_account_ids(state):
+    """Return payload owners plus generation-only active/retired markers."""
+    return _email_urgency_payload_account_ids(state) | set(
+        _email_urgency_account_generations(state)
+    )
+
+
+def _email_urgency_stale_accounts(
+    prior,
+    base_account_generations,
+    account_ids,
+):
+    prior_generations = _email_urgency_account_generations(prior)
+    base_generations = _email_urgency_account_generations(
+        {"account_generations": base_account_generations}
+    )
+    return {
+        str(account_id)
+        for account_id in account_ids
+        if prior_generations.get(str(account_id), {}).get("checkpoint", 0)
+        != base_generations.get(str(account_id), {}).get("checkpoint", 0)
+    }
+
+
+def _merge_email_urgency_state(
+    prior,
+    *,
+    owner,
+    per_uid_scores,
+    notified_uids,
+    all_unread_keys,
+    fully_scanned_account_ids,
+    base_account_generations,
+    timestamp,
+    retired_account_ids=(),
+    base_payload_account_ids=(),
+    known_account_ids=(),
+):
+    """Merge a scan without letting an older snapshot erase newer facts."""
+    prior_per_uid = prior.get("per_uid", {})
+    if not isinstance(prior_per_uid, dict):
+        prior_per_uid = {}
+    complete = {str(account_id) for account_id in fully_scanned_account_ids}
+    prior_generations = _email_urgency_account_generations(prior)
+    retire_requested = {str(account_id) for account_id in retired_account_ids}
+    observed_accounts = {
+        _email_urgency_account_key(key) for key in per_uid_scores
+    } | complete | retire_requested
+    stale_accounts = _email_urgency_stale_accounts(
+        prior,
+        base_account_generations,
+        observed_accounts,
+    )
+    prior_payload_accounts = _email_urgency_payload_account_ids(prior)
+    base_payload_accounts = {
+        str(account_id) for account_id in base_payload_account_ids
+    }
+    # A selected account can be absent from the base snapshot. If another
+    # worker creates its first payload before this transaction wins the lock,
+    # membership itself is a fence even when both snapshots normalize to the
+    # legacy generation zero.
+    retired_accounts = {
+        account_id
+        for account_id in retire_requested - stale_accounts
+        if not (
+            account_id in prior_payload_accounts
+            and account_id not in base_payload_accounts
+        )
+    }
+    fresh_complete = complete - stale_accounts - retired_accounts
+    changed_accounts = set(fresh_complete)
+
+    merged_per_uid = {
+        key: value
+        for key, value in prior_per_uid.items()
+        if _email_urgency_account_key(key) not in retired_accounts
+    }
+    for key in list(merged_per_uid):
+        account_id = _email_urgency_account_key(key)
+        if account_id in fresh_complete:
+            merged_per_uid.pop(key, None)
+            changed_accounts.add(account_id)
+    # Partial scans may add or refresh facts, but absence from a partial scan
+    # is not evidence that another checkpoint or UI row is stale. When another
+    # worker committed after this scan captured its base generation, discard
+    # this account's whole stale snapshot. A key absent from the newer state
+    # may have been removed/read, so even a stale-only key is not safely
+    # additive without another fresh scan.
+    for key, value in per_uid_scores.items():
+        account_id = _email_urgency_account_key(key)
+        if account_id in stale_accounts or account_id in retired_accounts:
+            continue
+        if merged_per_uid.get(key) != value:
+            changed_accounts.add(account_id)
+        merged_per_uid[key] = value
+
+    prior_notified = _email_urgency_string_set(prior.get("notified_uids", []))
+    merged_notified = {
+        key
+        for key in prior_notified
+        if _email_urgency_account_key(key) not in retired_accounts
+    }
+    for key in _email_urgency_string_set(notified_uids) - prior_notified:
+        account_id = _email_urgency_account_key(key)
+        if account_id in stale_accounts or account_id in retired_accounts:
+            continue
+        merged_notified.add(key)
+        changed_accounts.add(account_id)
+    for key in list(merged_notified):
+        if (
+            _email_urgency_account_key(key) in fresh_complete
+            and key not in all_unread_keys
+        ):
+            merged_notified.discard(key)
+            changed_accounts.add(_email_urgency_account_key(key))
+
+    next_generations = {
+        account_id: dict(value)
+        for account_id, value in prior_generations.items()
+    }
+    for account_id in changed_accounts:
+        generation = next_generations.setdefault(
+            account_id,
+            {"checkpoint": 0, "complete": 0},
+        )
+        generation["checkpoint"] += 1
+        if account_id in fresh_complete:
+            generation["complete"] += 1
+    for account_id in {str(value) for value in known_account_ids}:
+        next_generations.setdefault(
+            account_id,
+            {"checkpoint": 0, "complete": 0},
+        )
+    for account_id in retired_accounts:
+        # Every authoritative absence advances its generation, even when the
+        # prior state is already a payload-empty tombstone. A re-enabled scan
+        # may have captured that previous tombstone immediately before the
+        # account was disabled/deleted again; monotonic advancement is what
+        # makes that in-flight scan stale.
+        generation = next_generations.setdefault(
+            account_id,
+            {"checkpoint": 0, "complete": 0},
+        )
+        generation["checkpoint"] += 1
+
+    total_unread = 0
+    total_urgent = 0
+    max_score = 0
+    for value in merged_per_uid.values():
+        if not isinstance(value, dict):
+            continue
+        try:
+            score = max(0, min(3, int(value.get("score", 0))))
+        except (TypeError, ValueError):
+            score = 0
+        max_score = max(max_score, score)
+        if value.get("unread"):
+            total_unread += 1
+            if score >= 2:
+                total_urgent += 1
+
+    return {
+        "ts": timestamp,
+        "owner": owner or "",
+        "total_unread": total_unread,
+        "total_urgent": total_urgent,
+        "max_score": max_score,
+        "per_uid": merged_per_uid,
+        "notified_uids": sorted(merged_notified),
+        "account_generations": next_generations,
+    }
+
+
 class TaskNoop(BaseException):
     """Raised by an action when it determined there's nothing to do.
 
@@ -1878,6 +2267,7 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
         # filename for single-user installs (matches prior behaviour).
         _owner_slug = "".join(c if (c.isalnum() or c in "-_.@") else "_" for c in (owner or "default"))
         STATE_PATH = _P(DATA_DIR) / f"email_urgency_state_{_owner_slug}.json"
+        STATE_LOCK_DB = STATE_PATH.with_suffix(".lock.sqlite3")
         CACHE_DIR = _P(EMAIL_URGENCY_CACHE_DIR)
         CACHE_DIR.mkdir(parents=True, exist_ok=True)
         STATE_PATH.parent.mkdir(parents=True, exist_ok=True)
@@ -1892,35 +2282,144 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
             "shopping", "social", "work", "personal", "legal", "support", "promo",
         }
 
-        # ── 1. Resolve LLM candidates (utility primary + utility fallbacks; fall
+        # Resolve with the task owner as before, but defer the availability
-        # through to default chat as a last resort).
+        # gate until after authoritative account cleanup. State retirement must
+        # still run when no model is configured.
         from src.task_endpoint import resolve_task_candidates
         candidates = resolve_task_candidates(owner=owner)
-        if not candidates:
-            return "No LLM endpoint available", False
-
         target_account_id = _email_task_account_id(kwargs)
 
-        # ── 2. Enumerate enabled accounts. Match this task's owner AND fall
+        # ── 1. Enumerate enabled accounts. Match this task's owner AND fall
         # back to the legacy "unowned account whose imap_user / from_address
         # == this owner" pattern — same rule `_get_email_config` uses, so a
         # pre-multi-user account row still gets picked up for the seeded task.
-        db = _SL()
+        def _enumerate_enabled_accounts():
-        try:
+            db = _SL()
-            from sqlalchemy import and_ as _and, or_ as _or
+            try:
-            q = db.query(_EA).filter(_EA.enabled == True)  # noqa: E712
+                from sqlalchemy import and_ as _and, or_ as _or
-            if owner:
+                q = db.query(_EA).filter(_EA.enabled == True)  # noqa: E712
-                unowned = _or(_EA.owner == None, _EA.owner == "")  # noqa: E711
+                if owner:
-                same_mailbox = _or(_EA.imap_user == owner, _EA.from_address == owner)
+                    unowned = _or(_EA.owner == None, _EA.owner == "")  # noqa: E711
-                q = q.filter(_or(_EA.owner == owner, _and(unowned, same_mailbox)))
+                    same_mailbox = _or(
-            if target_account_id:
+                        _EA.imap_user == owner,
-                q = q.filter(_EA.id == target_account_id)
+                        _EA.from_address == owner,
-            accounts = q.all()
+                    )
-        finally:
+                    q = q.filter(
-            db.close()
+                        _or(_EA.owner == owner, _and(unowned, same_mailbox))
+                    )
+                if target_account_id:
+                    q = q.filter(_EA.id == target_account_id)
+                return q.all()
+            finally:
+                db.close()
+
+        initial_accounts = _enumerate_enabled_accounts()
+        initial_account_ids = {
+            str(account.id) for account in initial_accounts
+        }
+
+        # Register every account before IMAP work, including its first-ever
+        # scan. A concurrent zero-account cleanup can then advance this marker
+        # and fence delivery even before the scan has produced payload.
+        registered_state = None
+        if initial_account_ids:
+            async def _register_accounts(prior):
+                next_state = _merge_email_urgency_state(
+                    prior,
+                    owner=owner,
+                    per_uid_scores={},
+                    notified_uids=prior.get("notified_uids", []),
+                    all_unread_keys=set(),
+                    fully_scanned_account_ids=set(),
+                    base_account_generations=(
+                        _email_urgency_account_generations(prior)
+                    ),
+                    timestamp=_time.time(),
+                    known_account_ids=initial_account_ids,
+                )
+                # Return the exact state committed by registration. This is
+                # the scan's generation token: adopting a later checkpoint
+                # after account cleanup would let the stale scan appear fresh.
+                return next_state, next_state
+
+            registered_state = await _run_email_urgency_state_transaction(
+                STATE_PATH,
+                STATE_LOCK_DB,
+                _register_accounts,
+            )
+
+        # Revalidate after registration. If deletion/disable and its cleanup
+        # completed before the marker was published, this second enumeration
+        # observes the absence and this action retires its own marker instead
+        # of starting IMAP. Accounts newly appearing between the two reads are
+        # left for the next pass rather than scanned without prior registration.
+        verified_accounts = _enumerate_enabled_accounts()
+        enabled_account_ids = {
+            str(account.id) for account in verified_accounts
+        }
+        accounts = [
+            account
+            for account in verified_accounts
+            if str(account.id) in initial_account_ids
+        ]
+
+        # Capture the checkpoint basis before cleanup or IMAP. A full
+        # owner-wide enumeration authoritatively retires all known state IDs
+        # absent from the current enabled/visible set. A scoped task may retire
+        # only its selected missing/disabled account. Existing accounts remain
+        # present even if their later network scan fails, so transient IMAP
+        # failure never erases their last known state.
+        base_state = (
+            registered_state
+            if registered_state is not None
+            else _read_email_urgency_state(STATE_PATH)
+        )
+        base_account_generations = _email_urgency_account_generations(
+            base_state
+        )
+        base_payload_account_ids = _email_urgency_payload_account_ids(base_state)
+        known_state_account_ids = _email_urgency_known_account_ids(base_state)
+        if target_account_id:
+            retired_account_ids = (
+                {str(target_account_id)}
+                if str(target_account_id) not in enabled_account_ids
+                else set()
+            )
+        else:
+            retired_account_ids = (
+                known_state_account_ids - enabled_account_ids
+            )
+
+        if retired_account_ids:
+            async def _retire_accounts(prior):
+                next_state = _merge_email_urgency_state(
+                    prior,
+                    owner=owner,
+                    per_uid_scores={},
+                    notified_uids=prior.get("notified_uids", []),
+                    all_unread_keys=set(),
+                    fully_scanned_account_ids=set(),
+                    base_account_generations=base_account_generations,
+                    timestamp=_time.time(),
+                    retired_account_ids=retired_account_ids,
+                    base_payload_account_ids=base_payload_account_ids,
+                )
+                return None, next_state
+
+            await _run_email_urgency_state_transaction(
+                STATE_PATH,
+                STATE_LOCK_DB,
+                _retire_accounts,
+            )
         if not accounts:
             raise TaskNoop("no email accounts configured")
 
+        # ── 2. Account retirement above is state maintenance and does not
+        # depend on model availability. Scanning still requires the utility
+        # primary/fallback candidates resolved for this task owner.
+        if not candidates:
+            return "No LLM endpoint available", False
+
         urgency_prompt = settings.get("urgent_email_prompt", "")
         per_uid_scores = {}   # key = "<acc_id>:<uid>" → {"score": 0-3, "reason": "..."}
         all_unread_keys = set()
@@ -1929,6 +2428,7 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
         failed_classifications = []
         tag_write_details = []
         scanned = 0
+        fully_scanned_account_ids = set()
 
         def _heuristic_email_verdict(item: dict) -> dict:
             blob = (
@@ -2024,16 +2524,27 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
             def _scan_one(account=acc, cache_uids=cache.get("uids", {})):
                 """Sync IMAP work runs in a thread."""
                 results = []
+                scan_complete = True
                 conn = _imap_connect(account.id)
                 try:
-                    conn.select("INBOX", readonly=True)
+                    select_status, _select_data = conn.select("INBOX", readonly=True)
+                    if select_status != "OK":
+                        return results, False
                     # Tag recent inbox mail, not only unread mail. Urgency
                     # reminders below still only notify for unread messages.
                     since_str = AGE_CUTOFF.strftime("%d-%b-%Y")
                     status, data = conn.uid("SEARCH", None, f'(SINCE {since_str})')
-                    if status != "OK" or not data or not data[0]:
+                    if status != "OK":
-                        return results
+                        return results, False
-                    uids = data[0].split()[-30:]
+                    if not data or not data[0]:
+                        return results, True
+                    matching_uids = data[0].split()
+                    if len(matching_uids) > 30:
+                        # The scale guard deliberately processes only the most
+                        # recent 30. That is a partial account snapshot, so it
+                        # cannot justify pruning older checkpoint facts.
+                        scan_complete = False
+                    uids = matching_uids[-30:]
                     for uid_b in uids:
                         uid = uid_b.decode() if isinstance(uid_b, bytes) else str(uid_b)
                         key = f"{account.id}:{uid}"
@@ -2041,12 +2552,41 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
                         cached_ok = isinstance(cached, dict) and cached.get("triage_version") == TRIAGE_VERSION
                         results.append({"key": key, "uid": uid, "cached": cached if cached_ok else None})
                         if cached_ok:
-                            # Already classified — skip the fetch.
+                            # Cached verdicts still need a lightweight FLAGS
+                            # refresh. Without it a cached unread message looks
+                            # read and its successful notification checkpoint
+                            # is pruned on the next pass.
+                            try:
+                                st, flag_data = conn.uid("FETCH", uid_b, "(UID FLAGS)")
+                                if st != "OK" or not flag_data:
+                                    scan_complete = False
+                                    results.pop()
+                                    continue
+                                flag_parts = []
+                                for part in flag_data:
+                                    if isinstance(part, (bytes, bytearray)):
+                                        flag_parts.append(bytes(part))
+                                    elif (
+                                        isinstance(part, tuple)
+                                        and part
+                                        and isinstance(part[0], (bytes, bytearray))
+                                    ):
+                                        flag_parts.append(bytes(part[0]))
+                                flags_blob = b" ".join(flag_parts)
+                                results[-1]["unread"] = b"\\Seen" not in flags_blob
+                            except Exception as _fe:
+                                scan_complete = False
+                                results.pop()
+                                logger.debug(
+                                    f"urgency: flag fetch for uid {uid} failed: {_fe}"
+                                )
                             continue
                         # Pull headers + first ~800 chars of plaintext body.
                         try:
                             st, msg_data = conn.uid("FETCH", uid_b, "(UID FLAGS RFC822.HEADER BODY.PEEK[TEXT]<0.800>)")
                             if st != "OK" or not msg_data:
+                                scan_complete = False
+                                results.pop()
                                 continue
                             flags_blob = b" ".join(
                                 part[0] for part in msg_data
@@ -2060,6 +2600,8 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
                                 if isinstance(part, tuple) and part[1]:
                                     raw += part[1] + b"\n\n"
                             if not raw:
+                                scan_complete = False
+                                results.pop()
                                 continue
                             msg = _email_mod.message_from_bytes(raw)
                             # Skip Odysseus-generated reminders so the scanner
@@ -2115,17 +2657,21 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
                                 "unread": is_unread,
                             })
                         except Exception as _fe:
+                            scan_complete = False
+                            results.pop()
                             logger.debug(f"urgency: header fetch for uid {uid} failed: {_fe}")
                 finally:
                     try: conn.logout()
                     except Exception: pass
-                return results
+                return results, scan_complete
 
             try:
-                items = await _aio.to_thread(_scan_one)
+                items, scan_complete = await _aio.to_thread(_scan_one)
             except Exception as e:
                 logger.warning(f"urgency: IMAP scan failed for account {acc.id}: {e}")
                 continue
+            if scan_complete:
+                fully_scanned_account_ids.add(str(acc.id))
 
             for item in items:
                 scanned += 1
@@ -2262,13 +2808,13 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
                     logger.debug(f"urgency: LLM classify failed for {key}: {e}")
                     continue
 
-            # ── Prune cache entries for UIDs that are no longer in the recent
+            if scan_complete:
-            # scan window. Read messages remain cached because tags are useful
+                # Only a complete account scan proves a cached UID left the
-            # on read mail too; unread state is refreshed per scan above.
+                # recent window. Partial/failing scans preserve prior facts.
-            seen_uids = {it["uid"] for it in items}
+                seen_uids = {it["uid"] for it in items}
-            cache_uids = cache.get("uids", {})
+                cache_uids = cache.get("uids", {})
-            for stale in [u for u in cache_uids if u not in seen_uids]:
+                for stale in [u for u in cache_uids if u not in seen_uids]:
-                cache_uids.pop(stale, None)
+                    cache_uids.pop(stale, None)
 
             try:
                 cache_file.write_text(_json.dumps(cache), encoding="utf-8")
@@ -2372,40 +2918,34 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
 
         # ── 4. Aggregate state. urgent = score ≥ 2.
         urgent_keys = [k for k, v in per_uid_scores.items() if v.get("score", 0) >= 2 and v.get("unread")]
-        max_score = max((v.get("score", 0) for v in per_uid_scores.values()), default=0)
-        total_urgent = len(urgent_keys)
 
-        # Load prior state to know which urgent UIDs we've already notified.
+        # ── 5. Fire a reminder only when a previously-unnotified UID scores
-        try:
+        # urgent. The read, decision, delivery, and checkpoint are serialized
-            prior = _json.loads(STATE_PATH.read_text(encoding="utf-8")) if STATE_PATH.exists() else {}
+        # below so two scheduler workers cannot both act on the same stale
-        except Exception:
+        # state or overwrite each other's successful checkpoint.
-            prior = {}
-        notified_uids = set(prior.get("notified_uids", []))
-
-        # ── 5. Fire reminder ONLY when a previously-unnotified UID scores urgent.
-        new_urgent = [k for k in urgent_keys if k not in notified_uids]
         newly_notified = set()
         notify_failed = set()
-        if new_urgent:
+
-            title = "Urgent email" if total_urgent == 1 else f"{total_urgent} urgent emails"
+        def _urgency_reminder_payload(reminder_keys):
-            # Build a real listing — subject · sender · reason for each urgent
+            total = len(reminder_keys)
-            # one — so the reminder email tells you which messages to act on,
+            title = "Urgent email" if total == 1 else f"{total} urgent emails"
-            # not just "4 needing reply". Optional deep-link when the user has
-            # `app_public_url` configured in Settings (so the email row links
-            # straight into the Odysseus Email tab).
-            # Sort: highest-scored UIDs first; cap at 10 to keep the email tidy.
             sorted_urgent = sorted(
-                ((k, per_uid_scores[k]) for k in urgent_keys),
+                ((key, per_uid_scores[key]) for key in reminder_keys),
-                key=lambda kv: kv[1].get("score", 0), reverse=True,
+                key=lambda item: item[1].get("score", 0),
+                reverse=True,
             )[:10]
             _pub = (settings.get("app_public_url") or "").strip().rstrip("/")
             from urllib.parse import quote as _quote
-            lines = [f"{total_urgent} email" + ("" if total_urgent == 1 else "s") + " need an urgent reply:", ""]
+            lines = [
-            for i, (k, v) in enumerate(sorted_urgent, 1):
+                f"{total} email" + ("" if total == 1 else "s")
-                subj = (v.get("subject") or "(no subject)")[:160]
+                + " need an urgent reply:",
-                frm = v.get("from") or ""
+                "",
-                why = v.get("reason") or ""
+            ]
-                uid_for_link = str(k).split(":", 1)[-1]
+            for i, (key, value) in enumerate(sorted_urgent, 1):
+                subj = (value.get("subject") or "(no subject)")[:160]
+                frm = value.get("from") or ""
+                why = value.get("reason") or ""
+                uid_for_link = str(key).split(":", 1)[-1]
                 hash_link = f"#email={_quote('INBOX', safe='')}:{uid_for_link}"
                 open_link = f"{_pub}/{hash_link}" if _pub else hash_link
                 line = f"{i}. {subj}"
@@ -2415,57 +2955,94 @@ async def action_check_email_urgency(owner: str, **kwargs) -> Tuple[str, bool]:
                     line += f"  ·  {why}"
                 lines.append(line)
                 lines.append(f"   Open email: {open_link}")
-            if total_urgent > len(sorted_urgent):
+            if total > len(sorted_urgent):
                 lines.append("")
-                lines.append(f"…and {total_urgent - len(sorted_urgent)} more.")
+                lines.append(f"…and {total - len(sorted_urgent)} more.")
-            body = "\n".join(lines)
+            return title, "\n".join(lines)
-            try:
+
-                # Call dispatch_reminder DIRECTLY (no HTTP/auth roundtrip — the
+        async def _dispatch_urgency_reminder(reminder_keys):
-                # endpoint version 401's the background scheduler because it
+            # Call dispatch_reminder directly: a scheduler has no browser
-                # has no session cookie).
+            # session cookie with which to call the HTTP endpoint.
-                from routes.note_routes import dispatch_reminder
+            from routes.note_routes import dispatch_reminder
-                dispatch_result = await dispatch_reminder(
+            title, body = _urgency_reminder_payload(reminder_keys)
-                    title=title, note_body=body, note_id="urgent-email",
+            return await dispatch_reminder(
-                    owner=owner or "",
+                title=title,
-                )
+                note_body=body,
-                channel = (settings.get("reminder_channel") or "browser").strip().lower()
+                note_id="urgent-email",
-                delivered = bool(dispatch_result.get("browser_sent"))
+                owner=owner or "",
-                if channel == "email":
+            )
-                    delivered = bool(dispatch_result.get("email_sent"))
+
-                elif channel == "ntfy":
+        async def _dispatch_and_checkpoint(prior):
-                    delivered = bool(dispatch_result.get("ntfy_sent"))
+            notified_uids = _email_urgency_string_set(
-                elif channel == "webhook":
+                prior.get("notified_uids", [])
-                    delivered = bool(dispatch_result.get("webhook_sent"))
+            )
-                if delivered:
+            observed_accounts = {
-                    newly_notified.update(new_urgent)
+                _email_urgency_account_key(key) for key in per_uid_scores
-                else:
+            } | fully_scanned_account_ids
+            stale_accounts = _email_urgency_stale_accounts(
+                prior,
+                base_account_generations,
+                observed_accounts,
+            )
+            # Generation fencing must happen before delivery, not only during
+            # merge. A stale-only unread UID may have been removed, read, or
+            # downgraded by the newer completed scan.
+            deliverable_urgent = [
+                key
+                for key in urgent_keys
+                if _email_urgency_account_key(key) not in stale_accounts
+            ]
+            new_urgent = [
+                key
+                for key in deliverable_urgent
+                if key not in notified_uids
+            ]
+            if new_urgent:
+                try:
+                    dispatch_result = await _dispatch_urgency_reminder(
+                        deliverable_urgent
+                    )
+                    channel = (settings.get("reminder_channel") or "browser").strip().lower()
+                    delivered = bool(dispatch_result.get("browser_sent"))
+                    if channel == "email":
+                        delivered = bool(dispatch_result.get("email_sent"))
+                    elif channel == "ntfy":
+                        delivered = bool(dispatch_result.get("ntfy_sent"))
+                    elif channel == "webhook":
+                        delivered = bool(dispatch_result.get("webhook_sent"))
+                    if delivered:
+                        newly_notified.update(new_urgent)
+                        notified_uids.update(new_urgent)
+                    else:
+                        notify_failed.update(new_urgent)
+                        logger.warning(
+                            "urgency: reminder dispatch returned no successful "
+                            f"delivery path: {dispatch_result}"
+                        )
+                except Exception as e:
+                    logger.warning(f"urgency: reminder dispatch failed: {e}")
                     notify_failed.update(new_urgent)
-                    logger.warning(f"urgency: reminder dispatch returned no successful delivery path: {dispatch_result}")
-            except Exception as e:
-                logger.warning(f"urgency: reminder dispatch failed: {e}")
-                notify_failed.update(new_urgent)
-            # Mark only successfully delivered UIDs as notified so a transient
-            # SMTP/ntfy/browser failure retries instead of lying forever.
-            notified_uids.update(newly_notified)
 
-        # Prune notified_uids that aren't unread anymore (so a future re-urgent
+            next_state = _merge_email_urgency_state(
-        # message with the same UID — rare but possible after archive→unarchive
+                prior,
-        # — can re-notify). Keep only UIDs still in `all_unread_keys`.
+                owner=owner,
-        notified_uids = {u for u in notified_uids if u in all_unread_keys}
+                per_uid_scores=per_uid_scores,
+                notified_uids=notified_uids,
+                all_unread_keys=all_unread_keys,
+                fully_scanned_account_ids=fully_scanned_account_ids,
+                base_account_generations=base_account_generations,
+                timestamp=_time.time(),
+            )
+            return notified_uids, next_state
 
-        state = {
-            "ts": _time.time(),
-            "owner": owner or "",
-            "total_unread": len(all_unread_keys),
-            "total_urgent": total_urgent,
-            "max_score": max_score,
-            "per_uid": per_uid_scores,
-            "notified_uids": sorted(notified_uids),
-        }
         try:
-            STATE_PATH.write_text(_json.dumps(state), encoding="utf-8")
+            await _run_email_urgency_state_transaction(
+                STATE_PATH,
+                STATE_LOCK_DB,
+                _dispatch_and_checkpoint,
+            )
         except Exception as e:
-            logger.warning(f"urgency: state write failed: {e}")
+            logger.warning(f"urgency: state transaction failed: {e}")
 
         # ── 6. Activity-log summary — counts line on top, then per-tier
         # bulleted breakdown so the user can see WHICH emails ranked where

src/endpoint_resolver.py

@@ -443,28 +443,14 @@ def resolve_endpoint_by_id(
 
 
 def resolve_chat_fallback_candidates(owner: Optional[str] = None) -> list:
-    """Build the configured default-chat fallback chain as a list of
+    """Compatibility shim for the retired default-chat fallback chain."""
-    (chat_url, model, headers) tuples, skipping any that can't resolve.
 
-    The primary model is NOT included — callers prepend their session's
+    del owner
-    current (url, model, headers) so per-session model overrides are honored.
+    return []
-    """
-    return _resolve_fallback_candidates("default_model_fallbacks", owner=owner)
 
 
 def resolve_utility_fallback_candidates(owner: Optional[str] = None) -> list:
     """Configured fallback chain for the Utility model (`utility_model_fallbacks`)."""
-    try:
-        from src.settings import get_user_setting, load_settings
-        settings = load_settings()
-        utility_ep = (get_user_setting("utility_endpoint_id", owner or "", settings.get("utility_endpoint_id", "")) or "").strip()
-        if not utility_ep:
-            utility_chain = get_user_setting("utility_model_fallbacks", owner or "", settings.get("utility_model_fallbacks") or []) or []
-            if utility_chain:
-                return _resolve_fallback_candidates("utility_model_fallbacks", owner=owner)
-            return _resolve_fallback_candidates("default_model_fallbacks", owner=owner)
-    except Exception:
-        pass
     return _resolve_fallback_candidates("utility_model_fallbacks", owner=owner)
 
 

src/foreground_model_routing.py

@@ -0,0 +1,31 @@
+"""Foreground Chat and Agent model-routing policy.
+
+The selected session model is strict by default. Historical
+``default_model_fallbacks`` values remain stored for compatibility, but this
+policy intentionally does not read or migrate them.
+"""
+
+from typing import Any, Dict, Optional
+
+
+def resolve_foreground_fallback_candidates(owner: Optional[str] = None) -> list:
+    """Return fallback candidates for a foreground Chat or Agent request.
+
+    Foreground routing is strict, so no alternate endpoint/model is eligible.
+    ``owner`` is accepted to keep this policy boundary owner-aware.
+    """
+
+    del owner
+    return []
+
+
+def build_foreground_model_candidates(
+    endpoint_url: str,
+    model: str,
+    headers: Optional[Dict[str, Any]] = None,
+    owner: Optional[str] = None,
+) -> list:
+    """Build the ordered candidate list for a foreground request."""
+
+    primary = (endpoint_url, model, headers or {})
+    return [primary] + resolve_foreground_fallback_candidates(owner=owner)

src/llm_core.py

@@ -1,6 +1,7 @@
 # src/llm_core.py
 import httpx
 import asyncio
+import copy
 import time
 import json
 import logging
@@ -644,7 +645,7 @@ def _build_ollama_payload(
     if options:
         payload["options"] = options
     if tools:
-        payload["tools"] = tools
+        payload["tools"] = _alias_harmony_tools(tools, model)
     return payload
 
 
@@ -1055,6 +1056,57 @@ def _model_disallows_reasoning_effort_with_chat_tools(model: str) -> bool:
     return bool(re.match(r"^(?:openai/)?gpt-5(?:[.\-]\d+)?(?:[-_:].*)?$", m))
 
 
+# gpt-oss (harmony) ships BUILT-IN tools named `python` and `browser`, invoked
+# with the raw body as the argument (`to=python` + bare source), while custom
+# functions use `to=functions.NAME` + JSON. A tool we expose under a built-in's
+# name therefore gets called with the built-in convention: the model emits raw
+# code, the server tries to parse it as JSON, and the whole request dies
+# ("error parsing tool call: raw='import sys, ...'"). In streaming mode Ollama
+# does not even report it — it truncates the stream, so the turn looks like an
+# empty response. `bash` collides the same way in practice.
+#
+# Measured on gpt-oss:20b via Ollama /v1 with a fixed agentic prompt:
+#   tools named python+bash ............ 2/6 succeeded (4 parse failures)
+#   python renamed ..................... 5/6
+#   python and bash renamed ............ 6/6
+#
+# So rename the colliding tools on the way out and map the names back on the
+# way in. Confined to the transport layer: callers keep using the real names.
+_HARMONY_TOOL_ALIASES = {
+    "python": "run_python_code",
+    "bash": "run_shell_command",
+    "browser": "web_browser_tool",
+}
+_HARMONY_TOOL_ALIASES_REVERSE = {v: k for k, v in _HARMONY_TOOL_ALIASES.items()}
+
+
+def _is_harmony_model(model: str) -> bool:
+    """True for gpt-oss / harmony-format models, which have built-in tool names."""
+    return "gpt-oss" in (model or "").lower()
+
+
+def _alias_harmony_tools(tools: Optional[List[Dict]], model: str) -> Optional[List[Dict]]:
+    """Rename tools that collide with harmony built-ins. Returns a copy."""
+    if not tools or not _is_harmony_model(model):
+        return tools
+    out = []
+    for t in tools:
+        fn = t.get("function") or {}
+        alias = _HARMONY_TOOL_ALIASES.get(fn.get("name"))
+        if alias:
+            t = copy.deepcopy(t)
+            t["function"]["name"] = alias
+        out.append(t)
+    return out
+
+
+def _unalias_harmony_tool_name(name: str, model: str) -> str:
+    """Map an aliased tool name in a model response back to the real name."""
+    if not _is_harmony_model(model):
+        return name
+    return _HARMONY_TOOL_ALIASES_REVERSE.get(name, name)
+
+
 def _scrub_openai_chat_tool_reasoning(payload: Dict, target_url: str, model: str) -> None:
     if not payload.get("tools"):
         return
@@ -1267,8 +1319,8 @@ _MISTRAL_REASONING_EFFORT = os.getenv("ODYSSEUS_MISTRAL_REASONING_EFFORT", "high
 
 # Models that support structured thinking — may output </think> without opening tag
 _THINKING_MODEL_PATTERNS = (
-    "qwen3", "qwq", "deepseek-r1", "deepseek-reasoner", "minimax",
+    "qwen3", "qwq", "deepseek-r1", "deepseek-reasoner", "deepseek-v4",
-    "m2-reap", "gemma", "stepfun", "step-3", "step3",
+    "minimax", "m2-reap", "gemma", "stepfun", "step-3", "step3",
     "magistral", "mistral-small", "mistral-medium",
 )
 
@@ -1897,11 +1949,10 @@ def _dedupe_candidates(candidates):
     """Filter malformed entries and drop a later repeat of an already-seen
     ``(url, model)`` route, preserving order (first occurrence wins).
 
-    The chain is the primary target followed by the configured fallbacks, so a
+    The chain is the primary target followed by any caller-authorized
-    fallback that repeats the session's current model — a common misconfiguration,
+    fallbacks.  A fallback that repeats the session's current model would
-    since callers prepend the live ``(url, model)`` to ``default_model_fallbacks``
+    otherwise make the chain re-attempt the very route that just failed: a
-    — would otherwise make the chain re-attempt the very route that just failed:
+    wasted round-trip plus a spurious ``fallback`` notice for a switch that did
-    a wasted round-trip plus a spurious ``fallback`` notice for a switch that did
     not happen. Headers are not part of the key; the first tuple (with its
     headers) is the one kept.
     """
@@ -2109,7 +2160,17 @@ async def llm_call_async(
                     response = _parse_ollama_response(data)
                 else:
                     msg = data["choices"][0]["message"]
-                    response = msg.get("content") or msg.get("reasoning_content") or ""
+                    content = msg.get("content")
+                    if isinstance(content, list):
+                        # Mistral structured content — extract thinking + text
+                        # (same contract as llm_call / stream_llm; see #5435).
+                        text_part, thinking_part = _normalize_mistral_content(content)
+                        if thinking_part:
+                            response = thinking_part + "\n\n" + (text_part or "")
+                        else:
+                            response = text_part or msg.get("reasoning_content") or ""
+                    else:
+                        response = content or msg.get("reasoning_content") or ""
                 _set_cached_response(cache_key, response)
                 return response
             except Exception:
@@ -2226,7 +2287,7 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
             tok_key = "max_completion_tokens" if _uses_max_completion_tokens(model) else "max_tokens"
             payload[tok_key] = max_tokens
         if tools:
-            payload["tools"] = tools
+            payload["tools"] = _alias_harmony_tools(tools, model)
         elif tool_choice_none:
             payload["tool_choice"] = "none"
         # Mistral thinking-capable models — send reasoning_effort so Mistral
@@ -2360,7 +2421,7 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
                         if fn.get("name"):
                             _ollama_tool_calls.append({
                                 "id": tc.get("id") or f"call_{len(_ollama_tool_calls)}",
-                                "name": fn.get("name") or "",
+                                "name": _unalias_harmony_tool_name(fn.get("name") or "", model),
                                 "arguments": json.dumps(fn.get("arguments") or {}),
                             })
                     if j.get("done"):
@@ -2740,7 +2801,10 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
                                             if tc.get("extra_content"):
                                                 _tc_acc[idx]["extra_content"] = tc["extra_content"]
                                             if func.get("name"):
-                                                _tc_acc[idx]["name"] = func["name"]
+                                                # Map harmony aliases back to real
+                                                # tool names before anything
+                                                # downstream sees them.
+                                                _tc_acc[idx]["name"] = _unalias_harmony_tool_name(func["name"], model)
                                             if "arguments" in func:
                                                 # Guard against a null arguments delta: `func` can be
                                                 # {"arguments": None} (JSON null), and a raw `+= None`

src/settings.py

@@ -138,14 +138,13 @@ DEFAULT_SETTINGS = {
     # Email replies use email_writing_style instead because greetings,
     # signatures, and mailbox identity rules are medium-specific.
     "document_writing_style": "",
-    # Ordered fallback chain for the default chat model. Each entry is
+    # Legacy ordered fallback chain for the default chat model. Values remain
-    # {"endpoint_id": "...", "model": "..."}. If the primary model fails
+    # stored for compatibility and rollback reference, but model routing no
-    # before producing output (endpoint offline / errors), the chat
+    # longer reads this key.
-    # dispatch retries the next entry in order.
     "default_model_fallbacks": [],
-    # When True, non-admin users inherit global default model/endpoint/fallbacks
+    # When True, non-admin users inherit the global default model/endpoint when
-    # when they have no personal defaults. When False, users only use their
+    # they have no personal defaults. When False, users only use their personal
-    # personal defaults (no global fallback). Default is False.
+    # defaults. Default is False.
     "share_defaults_with_users": False,
     "utility_endpoint_id": "",
     "utility_model": "",

src/task_endpoint.py

@@ -32,7 +32,7 @@ def resolve_task_candidates(
     2. Utility endpoint/model
     3. Default endpoint/model
     4. Utility fallback chain
-    5. Default fallback chain
+    5. Retired default-fallback compatibility hook (currently empty)
     """
     candidates = []
 

src/teacher_escalation.py

@@ -233,7 +233,8 @@ async def _call_teacher(teacher_model_spec: str, prompt: str,
                         owner: Optional[str] = None) -> Optional[str]:
     """Call the configured teacher endpoint with the escalation prompt."""
     from src.llm_core import llm_call_async
-    from src.ai_interaction import _resolve_model, _TEACHER_SYSTEM_PROMPT
+    from src.ai_interaction import _resolve_model
+    from src.agent_tools.model_interaction_tools import _TEACHER_SYSTEM_PROMPT
     try:
         url, model, headers = await asyncio.to_thread(_resolve_model, teacher_model_spec, owner=owner)
     except Exception as e:

src/tool_parsing.py

@@ -193,7 +193,7 @@ _QWEN_ROLE_MARKER_RE = re.compile(r"</?\|(?:assistant|assistan|user|system|tool)
 # one; see #5547. `|end`, `end|`, `|end|` and `/|end|` still strip as before.
 _QWEN_BARE_MARKER_RE = re.compile(
     r"(?:^|[\t\r\n ])(?:/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|"
-    r"(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)",
+    r"(?:^|[\r\n])[ \t]*assistan(?:t)?[ \t]*(?=[\r\n]|$)",
     re.IGNORECASE,
 )
 
@@ -929,6 +929,46 @@ def _parse_xml_direct_tool(name, body) -> Optional[ToolBlock]:
     return function_call_to_tool_block(mapped, json.dumps(params))
 
 
+def _looks_like_json_body(body: str) -> bool:
+    """True when a <tool_call> wrapper body is JSON, not XML markup."""
+    return body.lstrip()[:1] in ("{", "[")
+
+
+def _parse_json_tool_call_body(body: str) -> Optional[ToolBlock]:
+    """Parse a Qwen/Hermes text-mode wrapper body: bare JSON inside <tool_call>.
+
+      <tool_call>
+      {"name": "bash", "arguments": {"command": "mkdir -p agent-test"}}
+      </tool_call>
+
+    Strict by design (issue #5187 / tracker #5333): the body must decode to an
+    object with a string "name", and "arguments" — when present — must itself
+    be an object. Anything else returns None rather than being coerced, so a
+    malformed call is dropped instead of dispatching with mangled arguments.
+    raw_decode tolerates trailing chatter after the JSON object; the trailing
+    text is never scanned for tool markup. Conversion goes through
+    function_call_to_tool_block so aliases and per-tool argument formatting
+    stay identical to the XML invoke path.
+    """
+    stripped = body.strip()
+    if not stripped.startswith("{"):
+        return None
+    try:
+        parsed, _end = json.JSONDecoder().raw_decode(stripped)
+    except json.JSONDecodeError:
+        return None
+    if not isinstance(parsed, dict):
+        return None
+    name = parsed.get("name")
+    if not isinstance(name, str) or not name.strip():
+        return None
+    if "arguments" in parsed and not isinstance(parsed["arguments"], dict):
+        return None
+    args = parsed.get("arguments", {})
+    from src.tool_schemas import function_call_to_tool_block
+    return function_call_to_tool_block(name.strip().lower(), json.dumps(args))
+
+
 def _iter_stepfun_tool_calls(text: str):
     """Yield StepFun native tool-call token bodies without regex backtracking."""
     pos = 0
@@ -1330,10 +1370,21 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]:
         if blocks:
             return blocks
         # Try wrapped: <tool_call><invoke ...>...</invoke></tool_call>
+        # A wrapper body that is JSON (Qwen/Hermes text mode, issue #5187) is
+        # parsed as JSON or dropped — never scanned by the XML iterators, so
+        # XML-like text inside JSON argument values stays data instead of
+        # selecting a different tool.
+        json_body_seen = False
         for _ms, inner_start, inner_end, _me in _iter_delimited(
             text, _XML_TOOL_CALL_OPEN_RE, _XML_TOOL_CALL_CLOSE_RE
         ):
             body = text[inner_start:inner_end]
+            if _looks_like_json_body(body):
+                json_body_seen = True
+                block = _parse_json_tool_call_body(body)
+                if block:
+                    blocks.append(block)
+                continue
             for inv_name, inv_body in _iter_xml_invoke(body):
                 block = _parse_xml_invoke(inv_name, inv_body)
                 if block:
@@ -1348,6 +1399,13 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]:
         if not blocks:
             for m in _XML_OPEN_TOOL_CALL_RE.finditer(text):
                 body = m.group(1)
+                if _looks_like_json_body(body):
+                    # Same fail-closed rule as above for an unclosed wrapper.
+                    json_body_seen = True
+                    block = _parse_json_tool_call_body(body)
+                    if block:
+                        blocks.append(block)
+                    break
                 for inv_name, inv_body in _iter_xml_invoke(body):
                     block = _parse_xml_invoke(inv_name, inv_body)
                     if block:
@@ -1358,8 +1416,11 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]:
                     block = _parse_xml_direct_tool(d_name, d_body)
                     if block:
                         blocks.append(block)
-        # Try bare <invoke> without wrapper
+        # Try bare <invoke> without wrapper. Skipped when a JSON wrapper body
-        if not blocks:
+        # was seen but produced no block: this rescan covers the full text,
+        # wrapper bodies included, and <invoke> markup inside a (possibly
+        # malformed) JSON payload must stay data rather than dispatch.
+        if not blocks and not json_body_seen:
             for inv_name, inv_body in _iter_xml_invoke(text):
                 block = _parse_xml_invoke(inv_name, inv_body)
                 if block:

src/tools/calendar.py

@@ -196,6 +196,9 @@ async def do_manage_calendar(content: str, owner: Optional[str] = None) -> Dict:
     try:
         if action == "list_calendars":
             _ensure_default_calendar(db, owner)
+            # This read path intentionally persists the lazily-created default;
+            # event creation commits it in the event's transaction instead.
+            db.commit()
             cals = _calendar_query().all()
             result = [{"name": c.name, "href": c.id} for c in cals]
             if result:

src/upload_handler.py

@@ -35,6 +35,16 @@ import logging
 
 logger = logging.getLogger(__name__)
 
+UploadIndexFileSignature = tuple[
+    str,
+    Optional[int],
+    Optional[int],
+    Optional[int],
+    Optional[int],
+    Optional[int],
+]
+UploadIndexSignature = tuple[UploadIndexFileSignature, ...]
+
 
 class UploadCleanupSafetyError(RuntimeError):
     """Raised when cleanup cannot prove that destructive work is safe."""
@@ -242,7 +252,7 @@ class UploadHandler:
 
         # In-memory index cache to avoid O(N) disk I/O on every request
         self._index_cache: Optional[Dict[str, Any]] = None
-        self._index_mtime: float = 0.0
+        self._index_signature: Optional[UploadIndexSignature] = None
     
     def inside_base_dir(self, path: str) -> bool:
         """Check if path is inside base directory"""
@@ -727,62 +737,119 @@ class UploadHandler:
         # Update cache if this is the main index
         if path.endswith("uploads.json"):
             self._index_cache = data
+            self._index_signature = self._upload_index_signature(
+                (path, path + ".bak")
+            )
+
+    @staticmethod
+    def _upload_index_signature(
+        paths: tuple[str, ...],
+    ) -> Optional[UploadIndexSignature]:
+        """Return file identities strong enough to validate the index cache.
+
+        Modification time alone is insufficient: a torn write can change a
+        file without receiving a strictly newer timestamp on some filesystems.
+        Size, inode, and nanosecond change times make those mutations visible
+        while preserving the cache fast path for unchanged files.
+        """
+        signature: list[UploadIndexFileSignature] = []
+        for candidate in paths:
             try:
-                self._index_mtime = os.path.getmtime(path)
+                stat_result = os.stat(candidate)
+            except FileNotFoundError:
+                signature.append((candidate, None, None, None, None, None))
+                continue
             except OSError:
-                self._index_mtime = time.time()
+                return None
+            signature.append(
+                (
+                    candidate,
+                    stat_result.st_dev,
+                    stat_result.st_ino,
+                    stat_result.st_size,
+                    stat_result.st_mtime_ns,
+                    stat_result.st_ctime_ns,
+                )
+            )
+        return tuple(signature)
 
     def _load_upload_index(self, *, fail_on_error: bool = False) -> Dict[str, Any]:
-        """Load the upload index from disk/cache. Uses mtime-based validation
+        """Load the upload index from disk/cache. Uses file-identity validation
-        to avoid redundant parsing on hot paths. When ``fail_on_error`` is
+        to avoid redundant parsing on hot paths without missing same-timestamp
-        true, a missing, malformed, or unreadable live index raises so
+        mutations. When ``fail_on_error`` is true, a missing, malformed, or
-        destructive callers cannot mistake corruption for an empty store.
+        unreadable live index raises so destructive callers cannot mistake
+        corruption for an empty store.
         """
         uploads_db_path = os.path.join(self.upload_dir, "uploads.json")
         candidates = (uploads_db_path, uploads_db_path + ".bak")
-        if fail_on_error:
+        for _attempt in range(3):
-            # A backup is intentionally the previous snapshot. It is useful for
+            signature = self._upload_index_signature(candidates)
-            # non-destructive reads, but cannot authorize deletion when the live
+            if fail_on_error:
-            # index is missing or corrupt.
+                # A backup is intentionally the previous snapshot. It is useful for
-            if not os.path.exists(uploads_db_path):
+                # non-destructive reads, but cannot authorize deletion when the live
-                raise ValueError("live uploads database is missing")
+                # index is missing or corrupt.
-            existing_candidates = [uploads_db_path]
+                if not os.path.exists(uploads_db_path):
-        else:
+                    raise ValueError("live uploads database is missing")
-            existing_candidates = [path for path in candidates if os.path.exists(path)]
+                existing_candidates = [uploads_db_path]
-        if not existing_candidates:
+            else:
-            self._index_cache = {}
+                existing_candidates = [
-            self._index_mtime = 0.0
+                    path for path in candidates if os.path.exists(path)
-            return {}
+                ]
+            if not existing_candidates:
+                self._index_cache = {}
+                self._index_signature = signature
+                return {}
 
-        # Check cache validity
+            # Check cache validity
-        try:
-            mtime = max(os.path.getmtime(path) for path in existing_candidates)
             if (
                 not fail_on_error
+                and signature is not None
                 and self._index_cache is not None
-                and mtime <= self._index_mtime
+                and signature == self._index_signature
             ):
                 return self._index_cache
-        except OSError:
-            mtime = 0.0
 
-        # Try the live file first, fall back to the .bak sibling if the
+            # Try the live file first, fall back to the .bak sibling if the
-        # live file is truncated/corrupted.
+            # live file is truncated/corrupted. A candidate parsed from an old
-        for candidate in existing_candidates:
+            # inode is accepted only when the whole index signature stays
-            try:
+            # stable through the read; otherwise retry so the cache cannot pair
-                with open(candidate, "r", encoding="utf-8") as f:
+            # stale data with a fresh replacement signature.
-                    data = json.load(f)
+            index_changed_during_read = False
-                if isinstance(data, dict):
+            for candidate in existing_candidates:
-                    self._index_cache = data
+                try:
-                    self._index_mtime = mtime
+                    with open(candidate, "r", encoding="utf-8") as f:
-                    return data
+                        data = json.load(f)
-            except Exception as e:
+                    verified_signature = self._upload_index_signature(candidates)
-                logger.warning(f"Failed to read uploads database ({candidate}): {e}")
+                    if (
+                        signature is not None
+                        and verified_signature is not None
+                        and verified_signature != signature
+                    ):
+                        index_changed_during_read = True
+                        break
+                    if isinstance(data, dict):
+                        self._index_cache = data
+                        self._index_signature = verified_signature
+                        return data
+                except Exception as e:
+                    logger.warning(f"Failed to read uploads database ({candidate}): {e}")
+                    verified_signature = self._upload_index_signature(candidates)
+                    if (
+                        signature is not None
+                        and verified_signature is not None
+                        and verified_signature != signature
+                    ):
+                        index_changed_during_read = True
+                        break
+                    continue
+            if index_changed_during_read:
                 continue
+            break
 
         if fail_on_error:
             raise ValueError("live uploads database is unreadable")
         self._index_cache = {}
+        self._index_signature = self._upload_index_signature(candidates)
         return {}
 
     def get_upload_info(self, upload_id: str) -> Optional[Dict[str, Any]]:

static/app.js

@@ -28,6 +28,7 @@ import memoryModule from './js/memory.js?v=20260722memoryloading1';
 import voiceRecorderModule from './js/voiceRecorder.js';
 import censorModule from './js/censor.js';
 import galleryModule from './js/gallery.js';
+import { UI_VIS_DEFAULT_OFF, resolveVisibility } from './js/ui_visibility.js';
 import tasksModule from './js/tasks.js?v=20260723tasksbulkfeedback1';
 import calendarModule from './js/calendar.js';
 import notesModule from './js/notes.js';
@@ -2725,46 +2726,6 @@ function initializeEventListeners() {
   // ── UI Visibility (Customize UI modal) ──
   const UI_VIS_KEY = 'odysseus-ui-visibility';
 
-  // Selector map: key → CSS selector(s) for targets
-  const UI_VIS_MAP = {
-    'sidebar-brand':       '.sidebar-brand-title',
-    'sidebar-new-chat':    '#sidebar-new-chat-btn',
-    'sidebar-search':      '#sidebar-search-btn',
-	    'sessions-section':    '#sessions-section',
-	    'email-section':       '#email-section',
-	    'tools-section':       '#tools-section',
-    // Per-tool visibility — fine-grained control over which entries show
-    // inside the Tools section in the sidebar.
-    'tool-calendar':       '#tool-calendar-btn',
-    'tool-compare':        '#tool-compare-btn',
-    'tool-cookbook':       '#tool-cookbook-btn',
-    'tool-research':       '#tool-research-btn',
-    'tool-gallery':        '#tool-gallery-btn',
-    'tool-library':        '#tool-library-btn',
-    'tool-memory':         '#tool-memory-btn',
-    'tool-notes':          '#tool-notes-btn',
-    'tool-tasks':          '#tool-tasks-btn',
-    'tool-theme':          '#tool-theme-btn',
-    'user-bar':            '#user-bar-profile',
-    'sidebar-settings-btn':'#user-bar-settings',
-    'chat-meta':           '.chat-meta-overlay',
-    'welcome-text':        '.welcome-name, .welcome-sub, #welcome-tip',
-    'incognito-btn':       '.incognito-btn',
-    'web-toggle-btn':      '#web-toggle-btn',
-    'doc-toggle-btn':      '#overflow-doc-btn',
-    'rag-toggle-btn':      '#overflow-rag-btn',
-    'bash-toggle-btn':     '#bash-toggle-btn',
-    'overflow-plus-btn':   '.overflow-wrapper',
-    'mode-toggle':         '.mode-toggle',
-    'preset-mini-btn':     '#overflow-preset-btn',
-    'attach-btn':          '#overflow-attach-btn',
-    'research-btn':        '#overflow-research-btn',
-    'rail-new-chat':       '#rail-new-session',
-  };
-
-  // Keys hidden by default on first run (no localStorage yet)
-	  const UI_VIS_DEFAULT_OFF = new Set(['rag-toggle-btn', 'text-emojis', 'chat-fullwidth']);
-
   // Keys that need admin to toggle off (reserved for future use)
   const UI_VIS_ADMIN_ONLY = new Set([]);
 
@@ -2777,14 +2738,14 @@ function initializeEventListeners() {
   }
 
   function applyUIVis(state) {
-    Object.entries(UI_VIS_MAP).forEach(([key, selector]) => {
+    // resolveVisibility computes selector→visible (pure; ui_visibility.js),
-      // section-drag-reorder uses a body class instead of inline styles
+    // including the tools-section parent rule that hides every tool rail
-      if (key === 'section-drag-reorder') return;
+    // launcher when Tools is off. Apply the result to the DOM here.
-      const visible = key in state ? state[key] !== false : !UI_VIS_DEFAULT_OFF.has(key);
+    for (const [selector, visible] of Object.entries(resolveVisibility(state))) {
       document.querySelectorAll(selector).forEach(el => {
         el.style.display = visible ? '' : 'none';
       });
-    });
+    }
     // Drag reorder: use body class so dynamically created handles are covered
     const dragEnabled = state['section-drag-reorder'] === true;
     document.body.classList.toggle('rearrange-mode', dragEnabled);

static/index.html

@@ -248,6 +248,15 @@
       }, { once: true });
     })();
   </script>
+  <!-- Preload the two faces first paint actually uses: Fira Code 400 and 600,
+       the app font and the weight the sidebar and header text render at. They
+       are declared in style.css, so without a hint they are only discovered
+       after the stylesheet parses and then queue behind the module graph.
+       crossorigin is required even though these are same-origin: fonts are
+       always fetched in CORS mode, and a preload whose mode does not match the
+       real request is discarded and the font fetched a second time. -->
+  <link rel="preload" as="font" type="font/woff2" crossorigin href="/static/fonts/FiraCode-Regular.woff2">
+  <link rel="preload" as="font" type="font/woff2" crossorigin href="/static/fonts/FiraCode-SemiBold.woff2">
   <link rel="stylesheet" href="/static/style.css?v=20260808startupshell1">
   <link rel="modulepreload" href="/static/app.js?v=20260808startupshell1">
   <link rel="modulepreload" href="/static/js/chat.js?v=20260801fix1">
@@ -1495,7 +1504,7 @@
                 <span class="adm-model-logo" id="set-defaultModelSelect-logo" style="display:inline-flex;align-items:center;justify-content:center;width:18px;height:18px;flex-shrink:0;opacity:0.9;color:var(--fg);"></span>
                 <select id="set-defaultModelSelect" class="settings-select"></select>
               </div>
-              <div class="settings-row" style="align-items:flex-start;">
+              <div class="settings-row" style="align-items:flex-start;" hidden>
                 <label class="settings-label" style="margin-top:6px;">Fallbacks</label>
                 <div style="flex:1;display:flex;flex-direction:column;gap:6px;">
                   <div id="set-defaultFallbacks" class="settings-fallbacks"></div>

static/js/chatRenderer.js

@@ -481,7 +481,7 @@ const QWEN_ROLE_MARKER_RE = /<\/?\|(?:assistant|assistan|user|system|tool)\|>?|<
 // Keep in sync with _QWEN_BARE_MARKER_RE in src/tool_parsing.py. At least one
 // pipe is required around `end`: with both optional (`\|?end\|?`) this also ate
 // a bare `end` on its own line, breaking Ruby/Lua/shell snippets (#5547).
-const QWEN_BARE_MARKER_RE = /(?:^|[\t\r\n ])(?:\/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)/gi;
+const QWEN_BARE_MARKER_RE = /(?:^|[\t\r\n ])(?:\/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|(?:^|[\r\n])[ \t]*assistan(?:t)?[ \t]*(?=[\r\n]|$)/gi;
 // Self-narration about tool results (model echoing stdout/exit_code)
 const TOOL_NARRATION_RE = /(?:The (?:result|output) shows?:?\s*)?-?\s*(?:stdout|stderr|exit_code):\s*.+/gi;
 

static/js/settings.js

@@ -448,7 +448,7 @@ async function initDefaultChat() {
   var fbContainer = el('set-defaultFallbacks');
   var addFbBtn = el('set-defaultAddFallback');
   var _endpoints = [];
-  var _fallbacks = []; // [{endpoint_id, model}] — tried in order if primary fails
+  var _fallbacks = []; // Hidden legacy DOM hook; stored values are not loaded or saved.
 
   function enabledEndpoints() {
     return _endpoints.filter(function(e) { return e.is_enabled; });
@@ -534,11 +534,6 @@ async function initDefaultChat() {
     var settings = await res.json();
     if (settings.default_endpoint_id) epSel.value = settings.default_endpoint_id;
     refreshModels(settings.default_model || '');
-    _fallbacks = Array.isArray(settings.default_model_fallbacks)
-      ? settings.default_model_fallbacks.map(function(f) {
-          return { endpoint_id: (f && f.endpoint_id) || '', model: (f && f.model) || '' };
-        })
-      : [];
     renderFallbacks();
   } catch (e) { console.warn('Failed to load default chat settings', e); }
 
@@ -547,13 +542,11 @@ async function initDefaultChat() {
 
   async function saveDefault() {
     try {
-      var clean = _fallbacks.filter(function(f) { return f.endpoint_id && f.model; });
       await fetch('/api/auth/settings', { method: 'POST', credentials: 'same-origin',
         headers: { 'Content-Type': 'application/json' },
         body: JSON.stringify({
           default_endpoint_id: epSel.value,
-          default_model: modelSel.value,
+          default_model: modelSel.value
-          default_model_fallbacks: clean
         })
       });
       msg.textContent = 'Saved'; msg.style.color = 'var(--fg)';

static/js/spinner.js

@@ -4,6 +4,13 @@
  * ASCII Spinner Module for AI thinking/processing status
  */
 
+// How long a canvas spinner may keep animating before its element has ever
+// been inserted into the document. start() runs synchronously, before the
+// caller appends the element, so frame 1 is always disconnected. Callers do
+// append in the same task, so anything past this window means the element is
+// never coming and the frames are drawing for nobody.
+const UNATTACHED_GRACE_MS = 2000;
+
 class Spinner {
   constructor(message = "AI is processing", style = "right", animation = "spinner") {
     // Different animation frames
@@ -21,6 +28,9 @@ class Spinner {
     this.intervalId = null;
     this.rafId = null;
     this.element = null;
+    this._wpWasConnected = false;
+    this._wpUnattachedSince = null;
+    this._visHandler = null;
   }
 
   /**
@@ -74,6 +84,7 @@ class Spinner {
   }
 
   _drawSineWave() {
+    if (!this.isRunning) return;
     const ctx = this._ctx;
     const W = this._canvas.width;
     const H = this._canvas.height;
@@ -120,9 +131,7 @@ class Spinner {
     ctx.fillStyle = 'rgba(156, 222, 242, 0.9)';
     ctx.fill();
 
-    if (this.isRunning) {
+    if (this.isRunning) this._requestFrame();
-      this.rafId = requestAnimationFrame(() => this._drawSineWave());
-    }
   }
 
   _createWhirlpoolElement() {
@@ -158,6 +167,7 @@ class Spinner {
   }
 
   _drawWhirlpool() {
+    if (!this.isRunning) return;
     const ctx = this._wpCtx;
     const W = this._wpCanvas.width;
     const H = this._wpCanvas.height;
@@ -229,18 +239,77 @@ class Spinner {
     ctx.fill();
     ctx.globalAlpha = 1;
 
-    if (!this.isRunning) return;
+    // Leak-safe self-terminate. "Nobody can see this spinner" has two shapes
-    // Leak-safe self-terminate: stop once our element WAS in the DOM and then
+    // and we have to catch both:
-    // got removed (e.g. a loading row replaced by results). But keep spinning
+    //   1. the element WAS in the DOM and then got removed (a loading row
-    // before it's first appended — start() runs synchronously, before the
+    //      replaced by results);
-    // caller inserts the element, so it isn't connected on frame 1.
+    //   2. the element was NEVER inserted, and the grace window for inserting
+    //      it has expired. The caller started a spinner and then took an early
+    //      return (aborted request, panel that resolved from cache), so no
+    //      frame we draw will ever be observed.
+    // Case 2 is why this needs a deadline at all: while the element has never
+    // been connected, `!this._wpWasConnected` stays true forever, so without
+    // the grace check the loop re-arms until the tab closes.
     const connected = !!(this.element && this.element.isConnected);
-    if (connected) this._wpWasConnected = true;
+    if (connected) {
-    if (connected || !this._wpWasConnected) {
+      this._wpWasConnected = true;
-      this.rafId = requestAnimationFrame(() => this._drawWhirlpool());
+      this._wpUnattachedSince = null;
-    } else {
+    } else if (!this._wpWasConnected) {
-      this.isRunning = false;
+      if (this._wpUnattachedSince === null) this._wpUnattachedSince = performance.now();
+      if (performance.now() - this._wpUnattachedSince > UNATTACHED_GRACE_MS) {
+        this.stop();
+        return;
+      }
     }
+
+    if (connected || !this._wpWasConnected) {
+      this._requestFrame();
+    } else {
+      this.stop();
+    }
+  }
+
+  /**
+   * Arm the next animation frame. Clearing rafId as the callback enters keeps
+   * it a truthful "a frame is pending" flag, which is what stop() and the
+   * visibility handler cancel against.
+   */
+  _requestFrame() {
+    this.rafId = requestAnimationFrame(() => {
+      this.rafId = null;
+      if (this.animation === 'sinewave') this._drawSineWave();
+      else this._drawWhirlpool();
+    });
+  }
+
+  /**
+   * Stop drawing while the tab is hidden. Browsers throttle background rAF but
+   * do not reliably stop the canvas work, and a spinner nobody is looking at
+   * should cost nothing. The listener is owned by start()/stop() so it is never
+   * left behind on a dead spinner.
+   */
+  _armVisibilityPause() {
+    if (this._visHandler) return;
+    this._visHandler = () => {
+      if (document.hidden) {
+        if (this.rafId) {
+          cancelAnimationFrame(this.rafId);
+          this.rafId = null;
+        }
+      } else if (this.isRunning && !this.rafId) {
+        // Reset the wave clock so the hidden interval doesn't arrive as one
+        // huge dt and skip the animation forward.
+        this._wavePrev = performance.now();
+        this._requestFrame();
+      }
+    };
+    document.addEventListener('visibilitychange', this._visHandler);
+  }
+
+  _disarmVisibilityPause() {
+    if (!this._visHandler) return;
+    document.removeEventListener('visibilitychange', this._visHandler);
+    this._visHandler = null;
   }
 
   /**
@@ -272,12 +341,15 @@ class Spinner {
 
     if (this.animation === 'sinewave') {
       this._wavePrev = performance.now();
+      this._armVisibilityPause();
       this._drawSineWave();
       return;
     }
 
     if (this.animation === 'whirlpool') {
       this._wpStartedAt = performance.now();
+      this._wpUnattachedSince = null;
+      this._armVisibilityPause();
       this._drawWhirlpool();
       return;
     }
@@ -302,6 +374,7 @@ class Spinner {
       cancelAnimationFrame(this.rafId);
       this.rafId = null;
     }
+    this._disarmVisibilityPause();
   }
 
   /**

static/js/ui_visibility.js

@@ -0,0 +1,73 @@
+// static/js/ui_visibility.js
+//
+// Per-item visibility for the sidebar and collapsed icon rail. Drives the
+// Settings → Appearance ("Customize UI") checkboxes, persisted in localStorage
+// under `odysseus-ui-visibility` (loaded/saved by app.js).
+//
+// Each key maps to the CSS selector(s) it controls. Tool/section selectors pair
+// the full-sidebar element with its #rail-* launcher so a tab hidden in the
+// full view also hides when the sidebar is minimized to the icon rail (id
+// mapping mirrors _railToolMap in app.js; #tool-library-btn ↔ #rail-archive).
+
+// Selector map: UI customization key → CSS selector(s) for its target(s).
+export const UI_VIS_MAP = {
+  'sidebar-brand':       '.sidebar-brand-title',
+  'sidebar-new-chat':    '#sidebar-new-chat-btn',
+  'sidebar-search':      '#sidebar-search-btn',
+  'sessions-section':   '#sessions-section',
+  'email-section':       '#email-section, #rail-email',
+  'tools-section':       '#tools-section',
+  // Per-tool entries pair the sidebar button with its rail launcher.
+  'tool-calendar':       '#tool-calendar-btn, #rail-calendar',
+  'tool-compare':        '#tool-compare-btn, #rail-compare',
+  'tool-cookbook':       '#tool-cookbook-btn, #rail-cookbook',
+  'tool-research':       '#tool-research-btn, #rail-research',
+  'tool-gallery':        '#tool-gallery-btn, #rail-gallery',
+  'tool-library':        '#tool-library-btn, #rail-archive',
+  'tool-memory':         '#tool-memory-btn, #rail-memory',
+  'tool-notes':          '#tool-notes-btn, #rail-notes',
+  'tool-tasks':          '#tool-tasks-btn, #rail-tasks',
+  'tool-theme':          '#tool-theme-btn, #rail-theme',
+  'user-bar':            '#user-bar-profile',
+  'sidebar-settings-btn':'#user-bar-settings',
+  'chat-meta':           '.chat-meta-overlay',
+  'welcome-text':        '.welcome-name, .welcome-sub, #welcome-tip',
+  'incognito-btn':       '.incognito-btn',
+  'web-toggle-btn':      '#web-toggle-btn',
+  'doc-toggle-btn':      '#overflow-doc-btn',
+  'rag-toggle-btn':      '#overflow-rag-btn',
+  'bash-toggle-btn':     '#bash-toggle-btn',
+  'overflow-plus-btn':   '.overflow-wrapper',
+  'mode-toggle':         '.mode-toggle',
+  'preset-mini-btn':     '#overflow-preset-btn',
+  'attach-btn':          '#overflow-attach-btn',
+  'research-btn':        '#overflow-research-btn',
+  'rail-new-chat':       '#rail-new-session',
+};
+
+// Keys hidden by default on first run (no localStorage yet).
+export const UI_VIS_DEFAULT_OFF = new Set(['rag-toggle-btn', 'text-emojis', 'chat-fullwidth']);
+
+/**
+ * Resolve every UI_VIS_MAP selector to visible (true) or hidden (false) for the
+ * given saved state. Pure: no DOM, no localStorage — app.js applies the result.
+ *
+ * A key is visible when its stored value is not `false`, defaulting to on
+ * unless it is in UI_VIS_DEFAULT_OFF. Per-tool entries also require the Tools
+ * section to be on: hiding Tools hides every tool, mirroring the full sidebar
+ * where the #tools-section container already hides them (the rail has no
+ * container, so this rule keeps it in sync).
+ *
+ * @param {Record<string, boolean>} state
+ * @returns {Record<string, boolean>} selector → visible
+ */
+export const resolveVisibility = (state = {}) => {
+  const toolsOn = state['tools-section'] !== false;
+  const out = {};
+  for (const [key, selector] of Object.entries(UI_VIS_MAP)) {
+    let visible = key in state ? state[key] !== false : !UI_VIS_DEFAULT_OFF.has(key);
+    if (!toolsOn && key.startsWith('tool-')) visible = false;
+    out[selector] = visible;
+  }
+  return out;
+};

tests/test_add_directory_event_loop.py

@@ -0,0 +1,394 @@
+"""Regression guard for #5558 — POST /api/personal/add_directory must not run
+the indexing job on the event loop.
+
+The handler is ``async def`` but called ``rag.index_personal_documents``
+(os.walk + file reads + per-chunk embedding + Chroma inserts) inline, so
+FastAPI ran the whole job on the event loop and every other request queued
+behind it: indexing a real directory froze the UI and API for 25+ minutes.
+``personal_docs_manager.add_directory`` sits in the same blocking section — it
+triggers ``refresh_index()``, which re-extracts text across tracked dirs.
+
+These tests build the real router with fake managers and compare the thread
+the indexing work runs on against the event loop's thread.
+"""
+import asyncio
+import os
+import threading
+
+os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:")
+
+import httpx
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+
+
+def _serialization_probe():
+    """Shared counter proving two critical sections never overlap."""
+    state = {"active": 0, "max_active": 0}
+    lock = threading.Lock()
+
+    def enter():
+        with lock:
+            state["active"] += 1
+            state["max_active"] = max(state["max_active"], state["active"])
+
+    def leave():
+        with lock:
+            state["active"] -= 1
+
+    return state, enter, leave
+
+
+# Concurrency tests are `async def` (pyproject asyncio_mode="auto") and drive the
+# ASGI app through httpx.ASGITransport + AsyncClient + asyncio.gather, NOT starlette
+# TestClient + ThreadPoolExecutor: the job lock is an asyncio.Lock acquired in the
+# async handler, and TestClient's portal-thread dispatch deadlocks against it (same
+# reason test_notes_fail_closed_auth.py uses ASGITransport). asyncio.gather runs both
+# requests on the test's own loop.
+def _async_client(app):
+    return httpx.AsyncClient(transport=httpx.ASGITransport(app=app), base_url="http://t")
+
+import routes.personal_routes as personal_routes
+from core.middleware import require_admin
+from src.auth_helpers import require_user
+
+
+class _FakeRag:
+    def __init__(self, record):
+        self._record = record
+
+    def index_personal_documents(self, directory, owner=None):
+        self._record["index_thread"] = threading.get_ident()
+        return {"success": True, "indexed_count": 3, "failed_count": 0}
+
+    def _split_into_chunks(self, text, chunk_size=500):
+        return [text]
+
+    def add_document(self, chunk, metadata):
+        self._record["add_document_thread"] = threading.get_ident()
+        return True
+
+    def delete_by_source(self, filepath):
+        self._record["delete_thread"] = threading.get_ident()
+        return 1
+
+
+class _FakeDocsManager:
+    def __init__(self, record):
+        self._record = record
+        self.index = []
+
+    def add_directory(self, directory, *, index=True, owner=None):
+        self._record["bookkeeping_thread"] = threading.get_ident()
+        self._record["bookkeeping_index_flag"] = index
+
+    def exclude_file(self, filepath):
+        self._record["exclude_thread"] = threading.get_ident()
+
+
+def _build_app(tmp_path, monkeypatch, record):
+    monkeypatch.setattr(personal_routes, "PERSONAL_DIR", str(tmp_path))
+    monkeypatch.setattr(personal_routes, "get_rag_manager", lambda: _FakeRag(record))
+
+    app = FastAPI()
+    app.include_router(
+        personal_routes.setup_personal_routes(_FakeDocsManager(record), None, True)
+    )
+    app.dependency_overrides[require_user] = lambda: "tester"
+    app.dependency_overrides[require_admin] = lambda: None
+
+    @app.get("/loop-thread")
+    async def loop_thread_probe():
+        return {"thread": threading.get_ident()}
+
+    return app
+
+
+def test_indexing_runs_off_the_event_loop(tmp_path, monkeypatch):
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    target = tmp_path / "docs"
+    target.mkdir()
+
+    # Context-manager client: one portal/event loop serves both requests, so
+    # the probe and the POST are guaranteed to see the same loop thread.
+    with TestClient(app) as client:
+        loop_thread = client.get("/loop-thread").json()["thread"]
+        resp = client.post(
+            "/api/personal/add_directory", json={"directory": str(target)}
+        )
+
+    assert resp.status_code == 200
+    assert record["index_thread"] != loop_thread, (
+        "index_personal_documents ran on the event loop thread — every other "
+        "request queues behind the indexing job (#5558)"
+    )
+    assert record["bookkeeping_thread"] != loop_thread, (
+        "personal_docs_manager.add_directory (refresh_index) ran on the event "
+        "loop thread"
+    )
+
+
+def test_response_and_bookkeeping_unchanged(tmp_path, monkeypatch):
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    target = tmp_path / "docs"
+    target.mkdir()
+
+    client = TestClient(app)
+    resp = client.post("/api/personal/add_directory", json={"directory": str(target)})
+
+    assert resp.status_code == 200
+    body = resp.json()
+    assert body["success"] is True
+    assert body["indexed_count"] == 3
+    assert body["failed_count"] == 0
+    assert body["directory"] == os.path.realpath(str(target))
+    assert record["bookkeeping_index_flag"] is False
+
+
+async def test_concurrent_add_directory_requests_serialize_indexing(tmp_path, monkeypatch):
+    """Off-loop execution must not mean parallel index jobs: concurrent
+    requests would race PersonalDocsManager's unsynchronized list mutations
+    and file writes (save_directories/_save_excluded are plain open('w'))."""
+    import time
+
+    state, enter, leave = _serialization_probe()
+
+    def _slow_index(self, directory, owner=None):
+        enter(); time.sleep(0.2); leave()
+        return {"success": True, "indexed_count": 1, "failed_count": 0}
+
+    monkeypatch.setattr(_FakeRag, "index_personal_documents", _slow_index)
+
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    for name in ("docs_a", "docs_b"):
+        (tmp_path / name).mkdir()
+
+    async with _async_client(app) as ac:
+        results = await asyncio.gather(
+            ac.post("/api/personal/add_directory", json={"directory": str(tmp_path / "docs_a")}),
+            ac.post("/api/personal/add_directory", json={"directory": str(tmp_path / "docs_b")}),
+        )
+
+    assert all(r.status_code == 200 for r in results)
+    assert state["max_active"] == 1, (
+        f"{state['max_active']} index jobs ran in parallel — concurrent "
+        "add_directory requests must serialize"
+    )
+
+
+def test_failed_indexing_still_returns_500(tmp_path, monkeypatch):
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    target = tmp_path / "docs"
+    target.mkdir()
+
+    def _fail(directory, owner=None):
+        return {"success": False, "message": "boom"}
+
+    monkeypatch.setattr(_FakeRag, "index_personal_documents", staticmethod(_fail))
+
+    client = TestClient(app)
+    resp = client.post("/api/personal/add_directory", json={"directory": str(target)})
+    assert resp.status_code == 500
+    assert "boom" in resp.json()["detail"]
+
+
+async def test_add_and_remove_serialize(tmp_path, monkeypatch):
+    """#5634: remove must hold the SAME job lock as add. Otherwise a remove
+    running while an add job is in flight races PersonalDocsManager's
+    unsynchronized list/index mutations — the inconsistent state the PR's
+    'add/remove are serialized' guarantee claims to prevent."""
+    import time
+
+    state, enter, leave = _serialization_probe()
+
+    def _slow_index(self, directory, owner=None):
+        enter(); time.sleep(0.25); leave()
+        return {"success": True, "indexed_count": 1, "failed_count": 0}
+
+    def _slow_remove(self, directory):
+        enter(); time.sleep(0.25); leave()
+
+    monkeypatch.setattr(_FakeRag, "index_personal_documents", _slow_index)
+    monkeypatch.setattr(_FakeDocsManager, "remove_directory", _slow_remove, raising=False)
+
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    (tmp_path / "docs_a").mkdir()
+    (tmp_path / "docs_b").mkdir()
+
+    async with _async_client(app) as ac:
+        results = await asyncio.gather(
+            ac.post("/api/personal/add_directory", json={"directory": str(tmp_path / "docs_a")}),
+            ac.delete("/api/personal/remove_directory", params={"directory": str(tmp_path / "docs_b")}),
+        )
+
+    assert all(r.status_code == 200 for r in results)
+    assert state["max_active"] == 1, (
+        f"{state['max_active']} add/remove critical sections overlapped — "
+        "remove must hold the same index job lock as add"
+    )
+
+
+async def test_add_and_upload_serialize(tmp_path, monkeypatch):
+    """#5634 follow-up: POST /upload writes chunks into the vector store and then
+    calls personal_docs_manager.add_directory — the same vector/tracking state
+    add_directory mutates. It must hold the SAME job lock, or an upload landing
+    mid-add interleaves two writers over unsynchronized state."""
+    import time
+
+    state, enter, leave = _serialization_probe()
+
+    def _slow_index(self, directory, owner=None):
+        enter(); time.sleep(0.25); leave()
+        return {"success": True, "indexed_count": 1, "failed_count": 0}
+
+    def _slow_add_document(self, chunk, metadata):
+        self._record["add_document_thread"] = threading.get_ident()
+        enter(); time.sleep(0.25); leave()
+        return True
+
+    monkeypatch.setattr(_FakeRag, "index_personal_documents", _slow_index)
+    monkeypatch.setattr(_FakeRag, "add_document", _slow_add_document)
+
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    monkeypatch.setattr(personal_routes, "UPLOADS_DIR", str(tmp_path / "uploads"))
+    monkeypatch.setattr(personal_routes, "require_privilege", lambda request, key: "tester")
+    (tmp_path / "docs_a").mkdir()
+
+    async with _async_client(app) as ac:
+        results = await asyncio.gather(
+            ac.post("/api/personal/add_directory", json={"directory": str(tmp_path / "docs_a")}),
+            ac.post("/api/personal/upload", files={"files": ("a.txt", b"hello world", "text/plain")}),
+        )
+
+    assert all(r.status_code == 200 for r in results)
+    # The test coroutine runs on the event loop, so this IS the loop thread.
+    assert record["add_document_thread"] != threading.get_ident(), (
+        "rag.add_document ran on the event loop thread — chunk writes block "
+        "every other request for the duration of the upload"
+    )
+    assert state["max_active"] == 1, (
+        f"{state['max_active']} add/upload critical sections overlapped — "
+        "upload must hold the same index job lock as add"
+    )
+
+
+async def test_upload_processes_each_payload_before_reading_the_next(tmp_path, monkeypatch):
+    """A multi-file upload must retain at most one capped payload at a time."""
+    from starlette.datastructures import UploadFile as StarletteUploadFile
+
+    reads = []
+    original_read = StarletteUploadFile.read
+
+    async def _recording_read(upload, size=-1):
+        reads.append(upload.filename)
+        return await original_read(upload, size)
+
+    def _record_first_index(self, chunk, metadata):
+        self._record.setdefault("reads_at_first_index", len(reads))
+        return True
+
+    monkeypatch.setattr(StarletteUploadFile, "read", _recording_read)
+    monkeypatch.setattr(_FakeRag, "add_document", _record_first_index)
+
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    monkeypatch.setattr(personal_routes, "UPLOADS_DIR", str(tmp_path / "uploads"))
+    monkeypatch.setattr(personal_routes, "require_privilege", lambda request, key: "tester")
+
+    files = [
+        ("files", ("a.txt", b"alpha", "text/plain")),
+        ("files", ("b.txt", b"bravo", "text/plain")),
+        ("files", ("c.txt", b"charlie", "text/plain")),
+    ]
+    async with _async_client(app) as ac:
+        response = await ac.post("/api/personal/upload", files=files)
+
+    assert response.status_code == 200
+    assert response.json()["uploaded"] == ["a.txt", "b.txt", "c.txt"]
+    assert reads == ["a.txt", "b.txt", "c.txt"]
+    assert record["reads_at_first_index"] == 1, (
+        "all upload bodies were retained before worker processing began"
+    )
+
+
+async def test_add_and_delete_file_serialize(tmp_path, monkeypatch):
+    """#5634 follow-up: DELETE /file removes chunks from the vector store and
+    calls personal_docs_manager.exclude_file. Both mutate state add_directory
+    also touches, so the delete must hold the SAME job lock as add."""
+    import time
+
+    state, enter, leave = _serialization_probe()
+
+    def _slow_index(self, directory, owner=None):
+        enter(); time.sleep(0.25); leave()
+        return {"success": True, "indexed_count": 1, "failed_count": 0}
+
+    def _slow_delete(self, filepath):
+        self._record["delete_thread"] = threading.get_ident()
+        enter(); time.sleep(0.25); leave()
+        return 1
+
+    monkeypatch.setattr(_FakeRag, "index_personal_documents", _slow_index)
+    monkeypatch.setattr(_FakeRag, "delete_by_source", _slow_delete)
+
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    monkeypatch.setattr(personal_routes, "UPLOADS_DIR", str(tmp_path / "uploads"))
+    (tmp_path / "docs_a").mkdir()
+    doomed = tmp_path / "doomed.txt"
+    doomed.write_text("bye")
+
+    async with _async_client(app) as ac:
+        results = await asyncio.gather(
+            ac.post("/api/personal/add_directory", json={"directory": str(tmp_path / "docs_a")}),
+            ac.delete("/api/personal/file", params={"filepath": str(doomed)}),
+        )
+
+    assert all(r.status_code == 200 for r in results)
+    assert record["delete_thread"] != threading.get_ident(), (
+        "rag.delete_by_source ran on the event loop thread"
+    )
+    assert state["max_active"] == 1, (
+        f"{state['max_active']} add/delete critical sections overlapped — "
+        "delete must hold the same index job lock as add"
+    )
+
+
+async def test_reload_serializes_with_add(tmp_path, monkeypatch):
+    """#5634: POST /reload rebuilds the index via refresh_index(); it must hold
+    the same job lock so it cannot race an in-flight add job."""
+    import time
+
+    state, enter, leave = _serialization_probe()
+
+    def _slow_index(self, directory, owner=None):
+        enter(); time.sleep(0.25); leave()
+        return {"success": True, "indexed_count": 1, "failed_count": 0}
+
+    def _slow_refresh(self):
+        enter(); time.sleep(0.25); leave()
+
+    monkeypatch.setattr(_FakeRag, "index_personal_documents", _slow_index)
+    monkeypatch.setattr(_FakeDocsManager, "refresh_index", _slow_refresh, raising=False)
+
+    record = {}
+    app = _build_app(tmp_path, monkeypatch, record)
+    (tmp_path / "docs_a").mkdir()
+
+    async with _async_client(app) as ac:
+        results = await asyncio.gather(
+            ac.post("/api/personal/add_directory", json={"directory": str(tmp_path / "docs_a")}),
+            ac.post("/api/personal/reload"),
+        )
+
+    assert all(r.status_code == 200 for r in results)
+    assert state["max_active"] == 1, (
+        f"{state['max_active']} add/reload critical sections overlapped — "
+        "reload must hold the same index job lock as add"
+    )

tests/test_calendar_default_transaction.py

@@ -0,0 +1,538 @@
+"""Default calendar creation belongs to the caller's transaction.
+
+Before this regression, ``_ensure_default_calendar`` committed independently.
+If event persistence then failed, the event rolled back but a new ``Personal``
+calendar remained (``calendar_count=1``, ``event_count=0``).
+"""
+
+import json
+import threading
+from contextlib import contextmanager
+from datetime import datetime, timedelta
+from types import SimpleNamespace
+
+import pytest
+from fastapi import HTTPException
+from sqlalchemy import create_engine, event
+from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm import Session, sessionmaker
+from sqlalchemy.pool import NullPool
+
+from tests.helpers.import_state import clear_fake_database_modules
+
+clear_fake_database_modules()
+
+import core.database as cdb  # noqa: E402
+import routes.calendar_routes as calendar_routes  # noqa: E402
+from core.database import CalendarCal, CalendarEvent  # noqa: E402
+from routes.calendar_routes import EventCreate  # noqa: E402
+from routes.calendar_routes import (  # noqa: E402
+    _default_calendar_id,
+    _ensure_default_calendar,
+)
+
+
+class _RejectEventCommit(Session):
+    """Reproduce an event commit failure after default-calendar creation."""
+
+    def commit(self):
+        if any(isinstance(row, CalendarEvent) for row in self.new):
+            raise RuntimeError("commit guard rejected event commit")
+        return super().commit()
+
+
+@pytest.fixture
+def session_factory(tmp_path, monkeypatch):
+    engine = create_engine(
+        f"sqlite:///{tmp_path / 'calendar.db'}",
+        connect_args={"check_same_thread": False},
+        poolclass=NullPool,
+    )
+    cdb.Base.metadata.create_all(engine)
+    factory = sessionmaker(
+        bind=engine,
+        autoflush=False,
+        autocommit=False,
+        class_=_RejectEventCommit,
+    )
+    monkeypatch.setattr(cdb, "SessionLocal", factory)
+    monkeypatch.setattr(calendar_routes, "SessionLocal", factory)
+    try:
+        yield factory
+    finally:
+        engine.dispose()
+
+
+def _request():
+    return SimpleNamespace(state=SimpleNamespace(current_user="alice"))
+
+
+def _endpoint(method, suffix):
+    router = calendar_routes.setup_calendar_routes()
+    for route in router.routes:
+        if route.path.endswith(suffix) and method in route.methods:
+            return route.endpoint
+    raise RuntimeError(f"{method} *{suffix} not found")
+
+
+def _counts(factory):
+    db = factory()
+    try:
+        return db.query(CalendarCal).count(), db.query(CalendarEvent).count()
+    finally:
+        db.close()
+
+
+async def test_route_event_failure_rolls_back_new_default_calendar(session_factory):
+    create_event = _endpoint("POST", "/events")
+
+    with pytest.raises(HTTPException) as caught:
+        await create_event(
+            _request(),
+            EventCreate(summary="Planning", dtstart="2126-07-20T09:00:00Z"),
+        )
+
+    assert caught.value.status_code == 500
+    assert _counts(session_factory) == (0, 0)
+
+
+async def test_route_event_validation_failure_rolls_back_new_default_calendar(
+    session_factory,
+):
+    create_event = _endpoint("POST", "/events")
+
+    with pytest.raises(HTTPException) as caught:
+        await create_event(
+            _request(),
+            EventCreate(summary="Planning", dtstart="not-a-datetime"),
+        )
+
+    assert caught.value.status_code == 500
+    assert _counts(session_factory) == (0, 0)
+
+
+async def test_tool_event_failure_rolls_back_new_default_calendar(session_factory):
+    from src.tools.calendar import do_manage_calendar
+
+    result = await do_manage_calendar(
+        json.dumps({
+            "action": "create_event",
+            "summary": "Planning",
+            "dtstart": "2126-07-20T09:00:00Z",
+        }),
+        owner="alice",
+    )
+
+    assert result["exit_code"] == 1
+    assert "commit guard rejected event commit" in result["error"]
+    assert _counts(session_factory) == (0, 0)
+
+
+async def test_tool_event_validation_failure_rolls_back_new_default_calendar(
+    session_factory,
+):
+    from src.tools.calendar import do_manage_calendar
+
+    result = await do_manage_calendar(
+        json.dumps({
+            "action": "create_event",
+            "summary": "Planning",
+            "dtstart": "not-a-datetime",
+        }),
+        owner="alice",
+    )
+
+    assert result["exit_code"] == 1
+    assert "Could not parse dtstart" in result["error"]
+    assert _counts(session_factory) == (0, 0)
+
+
+async def test_route_list_calendars_persists_lazy_default(session_factory):
+    list_calendars = _endpoint("GET", "/calendars")
+
+    result = await list_calendars(_request())
+
+    assert [calendar["name"] for calendar in result["calendars"]] == ["Personal"]
+    assert _counts(session_factory) == (1, 0)
+
+
+async def test_tool_list_calendars_persists_lazy_default(session_factory):
+    from src.tools.calendar import do_manage_calendar
+
+    result = await do_manage_calendar(
+        json.dumps({"action": "list_calendars"}),
+        owner="alice",
+    )
+
+    assert result["exit_code"] == 0
+    assert [calendar["name"] for calendar in result["calendars"]] == ["Personal"]
+    assert _counts(session_factory) == (1, 0)
+
+
+def test_repeated_rename_and_reuse_uses_stable_fallback_ids(tmp_path):
+    engine = create_engine(
+        f"sqlite:///{tmp_path / 'renamed-calendar.db'}",
+        connect_args={"check_same_thread": False},
+        poolclass=NullPool,
+    )
+    cdb.Base.metadata.create_all(engine)
+    factory = sessionmaker(bind=engine, autoflush=False, autocommit=False)
+    db = factory()
+    try:
+        first = _ensure_default_calendar(db, "alice")
+        assert first.id == _default_calendar_id("alice")
+        db.commit()
+
+        # The supported user-rename migration changes owner columns while
+        # deliberately preserving durable row identifiers.
+        first.owner = "bob"
+        db.commit()
+
+        second = _ensure_default_calendar(db, "alice")
+        assert second.id == _default_calendar_id("alice", 1)
+        db.commit()
+
+        # Repeating the same lifecycle must advance deterministically instead
+        # of failing or choosing a random identifier.
+        second.owner = "carol"
+        db.commit()
+
+        third = _ensure_default_calendar(db, "alice")
+        assert third.id == _default_calendar_id("alice", 2)
+        db.commit()
+
+        rows = db.query(CalendarCal).order_by(CalendarCal.owner).all()
+        assert [(row.owner, row.id) for row in rows] == [
+            ("alice", _default_calendar_id("alice", 2)),
+            ("bob", _default_calendar_id("alice")),
+            ("carol", _default_calendar_id("alice", 1)),
+        ]
+    finally:
+        db.close()
+        engine.dispose()
+
+
+def _assert_concurrent_first_use(tmp_path, occupied_owner=None):
+    engine = create_engine(
+        f"sqlite:///{tmp_path / 'concurrent-calendar.db'}",
+        connect_args={"check_same_thread": False, "timeout": 10},
+        poolclass=NullPool,
+    )
+    cdb.Base.metadata.create_all(engine)
+    factory = sessionmaker(bind=engine, autoflush=False, autocommit=False)
+    expected_collision_index = 0
+    if occupied_owner is not None:
+        seed = factory()
+        try:
+            seed.add(CalendarCal(
+                id=_default_calendar_id("alice"),
+                owner=occupied_owner,
+                name="Personal",
+                source="local",
+            ))
+            seed.commit()
+            expected_collision_index = 1
+        finally:
+            seed.close()
+    first_staged = threading.Event()
+    second_selected = threading.Event()
+    errors = []
+
+    @event.listens_for(engine, "after_cursor_execute")
+    def observe_second_gap(conn, cursor, statement, parameters, context, executemany):
+        if (
+            threading.current_thread().name == "calendar-worker-second"
+            and statement.lstrip().upper().startswith("SELECT")
+            and "FROM calendars" in statement
+        ):
+            second_selected.set()
+
+    def create_default(worker, hold=False):
+        db = factory()
+        try:
+            if not hold:
+                assert first_staged.wait(5)
+            cal = _ensure_default_calendar(db, "alice")
+            start = datetime(2126, 7, 20, 9 if hold else 10)
+            db.add(CalendarEvent(
+                uid=worker,
+                calendar_id=cal.id,
+                summary=f"Event {worker}",
+                dtstart=start,
+                dtend=start + timedelta(hours=1),
+            ))
+            if hold:
+                first_staged.set()
+                # The second session has observed the uncommitted gap before
+                # this transaction releases its writer reservation.
+                assert second_selected.wait(5)
+            db.commit()
+            assert cal.id == _default_calendar_id("alice", expected_collision_index)
+        except BaseException as exc:  # pragma: no cover - asserted below
+            errors.append((worker, exc))
+            db.rollback()
+        finally:
+            db.close()
+
+    first = threading.Thread(
+        target=create_default,
+        args=("first", True),
+        name="calendar-worker-first",
+    )
+    second = threading.Thread(
+        target=create_default,
+        args=("second",),
+        name="calendar-worker-second",
+    )
+    first.start()
+    second.start()
+    first.join(10)
+    second.join(10)
+
+    try:
+        assert not first.is_alive() and not second.is_alive()
+        assert errors == []
+        db = factory()
+        try:
+            rows = db.query(CalendarCal).filter(CalendarCal.owner == "alice").all()
+            assert [(row.id, row.name) for row in rows] == [
+                (_default_calendar_id("alice", expected_collision_index), "Personal")
+            ]
+            assert db.query(CalendarEvent).count() == 2
+            if occupied_owner is not None:
+                occupied = db.query(CalendarCal).filter(
+                    CalendarCal.id == _default_calendar_id("alice"),
+                ).one()
+                assert occupied.owner == occupied_owner
+        finally:
+            db.close()
+    finally:
+        engine.dispose()
+
+
+def test_concurrent_first_use_creates_one_sqlite_default(tmp_path):
+    _assert_concurrent_first_use(tmp_path)
+
+
+def test_concurrent_first_use_after_rename_creates_one_fallback_default(tmp_path):
+    _assert_concurrent_first_use(tmp_path, occupied_owner="bob")
+
+
+def test_sqlite_default_stays_in_callers_transaction(session_factory):
+    db = session_factory()
+    try:
+        cal = _ensure_default_calendar(db, "rollback-owner")
+        assert cal.id == _default_calendar_id("rollback-owner")
+        db.rollback()
+    finally:
+        db.close()
+
+    verify = session_factory()
+    try:
+        assert (
+            verify.query(CalendarCal)
+            .filter(CalendarCal.owner == "rollback-owner")
+            .count()
+            == 0
+        )
+    finally:
+        verify.close()
+
+
+def test_sqlite_fallback_default_stays_in_callers_transaction(session_factory):
+    seed = session_factory()
+    try:
+        seed.add(CalendarCal(
+            id=_default_calendar_id("alice"),
+            owner="bob",
+            name="Personal",
+            source="local",
+        ))
+        seed.commit()
+    finally:
+        seed.close()
+
+    db = session_factory()
+    try:
+        cal = _ensure_default_calendar(db, "alice")
+        assert cal.id == _default_calendar_id("alice", 1)
+        db.rollback()
+    finally:
+        db.close()
+
+    verify = session_factory()
+    try:
+        assert verify.query(CalendarCal).filter(CalendarCal.owner == "alice").count() == 0
+        assert verify.query(CalendarCal).filter(CalendarCal.owner == "bob").count() == 1
+    finally:
+        verify.close()
+
+
+class _FakeDialect:
+    name = "postgresql"
+
+
+class _FakeBind:
+    dialect = _FakeDialect()
+
+
+class _FakeQuery:
+    def __init__(self, session):
+        self.session = session
+
+    def filter(self, *conditions):
+        return self
+
+    def with_for_update(self):
+        self.session.locking_read = True
+        return self
+
+    def first(self):
+        self.session.query_count += 1
+        if self.session.query_count == 1:
+            return None
+        return self.session.winner
+
+
+class _GenericRaceSession:
+    """Minimal non-SQLite session that loses the deterministic-ID race."""
+
+    def __init__(self):
+        self.query_count = 0
+        self.nested_entries = 0
+        self.locking_read = False
+        self.candidate = None
+        self.winner = CalendarCal(
+            id=_default_calendar_id("alice"),
+            owner="alice",
+            name="Personal",
+            source="local",
+        )
+
+    def get_bind(self):
+        return _FakeBind()
+
+    def query(self, model):
+        assert model is CalendarCal
+        return _FakeQuery(self)
+
+    @contextmanager
+    def begin_nested(self):
+        self.nested_entries += 1
+        yield
+
+    def add(self, row):
+        self.candidate = row
+
+    def flush(self):
+        raise IntegrityError("insert", {}, RuntimeError("duplicate primary key"))
+
+
+def test_generic_backend_lost_race_recovers_inside_savepoint():
+    db = _GenericRaceSession()
+
+    winner = _ensure_default_calendar(db, "alice")
+
+    assert winner is db.winner
+    assert db.nested_entries == 1
+    assert db.locking_read is True
+    assert db.candidate.id == db.winner.id
+
+
+def test_generic_backend_unattributed_integrity_error_is_not_retried():
+    db = _GenericRaceSession()
+    db.winner = None
+
+    with pytest.raises(IntegrityError):
+        _ensure_default_calendar(db, "alice")
+
+    assert db.nested_entries == 1
+
+
+class _GenericRenamedSlotSession(_GenericRaceSession):
+    """A different owner occupies slot zero; slot one remains available."""
+
+    def __init__(self):
+        super().__init__()
+        self.candidates = []
+        self.winner = CalendarCal(
+            id=_default_calendar_id("alice"),
+            owner="bob",
+            name="Personal",
+            source="local",
+        )
+
+    def add(self, row):
+        self.candidate = row
+        self.candidates.append(row)
+
+    def flush(self):
+        if len(self.candidates) == 1:
+            raise IntegrityError("insert", {}, RuntimeError("duplicate primary key"))
+
+
+def test_generic_backend_renamed_slot_advances_inside_savepoint():
+    db = _GenericRenamedSlotSession()
+
+    fallback = _ensure_default_calendar(db, "alice")
+
+    assert fallback is db.candidates[-1]
+    assert fallback.id == _default_calendar_id("alice", 1)
+    assert fallback.owner == "alice"
+    assert db.nested_entries == 2
+    assert db.locking_read is True
+    assert db.winner.owner == "bob"
+
+
+def test_generic_backend_fallback_keeps_outer_transaction_usable(tmp_path):
+    engine = create_engine(
+        f"sqlite:///{tmp_path / 'generic-savepoint-calendar.db'}",
+        poolclass=NullPool,
+    )
+    cdb.Base.metadata.create_all(engine)
+    # SQLite supplies a lightweight local SQL executor here; changing only the
+    # dispatch name exercises the real Session/savepoint branch used by
+    # PostgreSQL-style backends without pretending to validate their dialect.
+    engine.dialect.name = "postgresql"
+    factory = sessionmaker(bind=engine, autoflush=False, autocommit=False)
+
+    seed = factory()
+    try:
+        seed.add(CalendarCal(
+            id=_default_calendar_id("alice"),
+            owner="bob",
+            name="Personal",
+            source="local",
+        ))
+        seed.commit()
+    finally:
+        seed.close()
+
+    db = factory()
+    try:
+        cal = _ensure_default_calendar(db, "alice")
+        start = datetime(2126, 7, 20, 9)
+        db.add(CalendarEvent(
+            uid="after-fallback",
+            calendar_id=cal.id,
+            summary="Atomic",
+            dtstart=start,
+            dtend=start + timedelta(hours=1),
+        ))
+        db.commit()
+    finally:
+        db.close()
+
+    verify = factory()
+    try:
+        assert [
+            (row.owner, row.id)
+            for row in verify.query(CalendarCal).order_by(CalendarCal.owner).all()
+        ] == [
+            ("alice", _default_calendar_id("alice", 1)),
+            ("bob", _default_calendar_id("alice")),
+        ]
+        assert verify.query(CalendarEvent).count() == 1
+    finally:
+        verify.close()
+        engine.dispose()

tests/test_ci_authoritative_validation.py

@@ -0,0 +1,35 @@
+"""Regression coverage for authoritative Python CI validation."""
+
+import re
+from pathlib import Path
+
+
+_WORKFLOW = (
+    Path(__file__).resolve().parent.parent / ".github" / "workflows" / "ci.yml"
+)
+
+
+def _indented_block(text: str, heading: str, indent: int) -> str:
+    pattern = re.compile(
+        rf"(?ms)^{' ' * indent}{re.escape(heading)}:\n"
+        rf"(?P<body>(?:(?:{' ' * (indent + 2)}.*|\s*)\n)*)"
+    )
+    match = pattern.search(text)
+    assert match is not None, f"missing {heading!r} block"
+    return match.group(0)
+
+
+def test_ci_runs_on_integrated_dev_pushes():
+    workflow = _WORKFLOW.read_text()
+    push = _indented_block(workflow, "push", 2)
+
+    assert re.search(r"(?m)^    branches:\s*\[main,\s*dev\]\s*$", push)
+    assert "paths-ignore:" not in push
+
+
+def test_python_tests_are_authoritative():
+    workflow = _WORKFLOW.read_text()
+    python_tests = _indented_block(workflow, "python-tests", 2)
+
+    assert "python -m pytest -q" in python_tests
+    assert "continue-on-error:" not in python_tests

tests/test_cookbook_helpers.py

@@ -723,7 +723,12 @@ def test_local_windows_download_pid_tracks_inner_bash_and_stop_kills_tree():
     routes_src = (Path(__file__).resolve().parents[1] / "routes" / "cookbook_routes.py").read_text(encoding="utf-8")
     running_src = (Path(__file__).resolve().parents[1] / "static" / "js" / "cookbookRunning.js").read_text(encoding="utf-8")
 
-    assert 'printf \'%s\\\\n\' \\"$$\\" > {pp}' in routes_src
+    # The Windows-local runner publishes Python's valid Win32 fallback before
+    # allowing Git Bash to replace it with /proc/$$/winpid.
+    assert "_windows_local_pid_record_line(pid_path, pid_ready_path)" in routes_src
+    assert "/proc/$$/winpid" in routes_src
+    assert "pid_ready_path.touch()" in routes_src
+    assert '\\"$$\\" > {pp}' not in routes_src
     assert "function Stop-Tree([int]$Id)" in running_src
     assert "('ParentProcessId = ' + $Id)" in running_src
     assert "Stop-Tree ([int]$p)" in running_src

tests/test_cookbook_local_serve_pid_winpid.py

@@ -0,0 +1,180 @@
+"""Behavioral regression coverage for Windows-local Cookbook PID recording."""
+
+import os
+import subprocess
+import time
+from pathlib import Path
+
+from routes.cookbook_routes import _windows_local_pid_record_line
+
+
+ROOT = Path(__file__).resolve().parents[1]
+COOKBOOK_ROUTES = ROOT / "routes" / "cookbook_routes.py"
+
+
+def _fake_cat(tmp_path: Path, body: str) -> Path:
+    fake_bin = tmp_path / "bin"
+    fake_bin.mkdir()
+    cat = fake_bin / "cat"
+    cat.write_text("#!/bin/sh\n" + body + "\n", encoding="utf-8")
+    cat.chmod(0o755)
+    return fake_bin
+
+
+def _env_for(fake_bin: Path, **extra: str) -> dict[str, str]:
+    env = dict(os.environ)
+    env["PATH"] = str(fake_bin) + os.pathsep + env.get("PATH", "")
+    env.update(extra)
+    return env
+
+
+def _run_pid_line(
+    pid_path: Path,
+    ready_path: Path,
+    fake_bin: Path,
+    **extra_env: str,
+) -> subprocess.CompletedProcess:
+    return subprocess.run(
+        ["bash", "-c", _windows_local_pid_record_line(pid_path, ready_path)],
+        capture_output=True,
+        text=True,
+        env=_env_for(fake_bin, **extra_env),
+        timeout=10,
+    )
+
+
+def test_windows_local_pid_line_records_numeric_winpid_after_fallback(tmp_path):
+    pid_path = tmp_path / "serve.pid"
+    ready_path = tmp_path / "serve.pid.ready"
+
+    pid_path.write_text("11111", encoding="utf-8")
+    ready_path.touch()
+
+    cat_arg = tmp_path / "cat-arg.txt"
+    fake_bin = _fake_cat(
+        tmp_path,
+        'printf "%s\\n" "$1" > "$FAKE_CAT_ARG"\n'
+        'printf "%s\\n" "$FAKE_WINPID"',
+    )
+
+    result = _run_pid_line(
+        pid_path,
+        ready_path,
+        fake_bin,
+        FAKE_CAT_ARG=str(cat_arg),
+        FAKE_WINPID="42324",
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert pid_path.read_text(encoding="utf-8").strip() == "42324"
+    assert not ready_path.exists()
+
+    proc_path = cat_arg.read_text(encoding="utf-8").strip()
+    parts = proc_path.strip("/").split("/")
+    assert len(parts) == 3
+    assert parts[0] == "proc"
+    assert parts[1].isdigit()
+    assert parts[2] == "winpid"
+
+
+def test_windows_local_pid_line_waits_for_python_fallback_before_replacing(tmp_path):
+    pid_path = tmp_path / "serve.pid"
+    ready_path = tmp_path / "serve.pid.ready"
+
+    fake_bin = _fake_cat(
+        tmp_path,
+        'printf "%s\\n" "$FAKE_WINPID"',
+    )
+
+    proc = subprocess.Popen(
+        [
+            "bash",
+            "-c",
+            _windows_local_pid_record_line(pid_path, ready_path),
+        ],
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        text=True,
+        env=_env_for(fake_bin, FAKE_WINPID="42324"),
+    )
+
+    # The inner shell has started, but Python has not published its fallback yet.
+    time.sleep(0.05)
+    assert proc.poll() is None
+    assert not pid_path.exists()
+
+    # Simulate the post-Popen Python publication order.
+    pid_path.write_text("31100", encoding="utf-8")
+    ready_path.touch()
+
+    stdout, stderr = proc.communicate(timeout=10)
+
+    assert proc.returncode == 0, stderr or stdout
+    assert pid_path.read_text(encoding="utf-8").strip() == "42324"
+    assert not ready_path.exists()
+
+
+def test_windows_local_pid_line_preserves_outer_pid_when_mapping_missing(tmp_path):
+    pid_path = tmp_path / "serve.pid"
+    ready_path = tmp_path / "serve.pid.ready"
+
+    pid_path.write_text("31100", encoding="utf-8")
+    ready_path.touch()
+
+    fake_bin = _fake_cat(tmp_path, "exit 1")
+
+    result = _run_pid_line(
+        pid_path,
+        ready_path,
+        fake_bin,
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert pid_path.read_text(encoding="utf-8").strip() == "31100"
+    assert not ready_path.exists()
+
+
+def test_windows_local_pid_line_rejects_malformed_mapping(tmp_path):
+    pid_path = tmp_path / "serve.pid"
+    ready_path = tmp_path / "serve.pid.ready"
+
+    pid_path.write_text("31100", encoding="utf-8")
+    ready_path.touch()
+
+    fake_bin = _fake_cat(
+        tmp_path,
+        'printf "not-a-win32-pid\\n"',
+    )
+
+    result = _run_pid_line(
+        pid_path,
+        ready_path,
+        fake_bin,
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert pid_path.read_text(encoding="utf-8").strip() == "31100"
+    assert not ready_path.exists()
+
+
+def test_local_windows_launcher_publishes_fallback_before_releasing_inner_runner():
+    source = COOKBOOK_ROUTES.read_text(encoding="utf-8")
+    start = source.index("    def _launch_local_detached(")
+    end = source.index(
+        '    @router.post("/api/model/download")',
+        start,
+    )
+    launcher = source[start:end]
+
+    assert "_windows_local_pid_record_line(pid_path, pid_ready_path)" in launcher
+    assert "pid_ready_path.unlink(missing_ok=True)" in launcher
+
+    fallback = launcher.index(
+        'pid_path.write_text(str(proc.pid), encoding="utf-8")'
+    )
+    release = launcher.index("pid_ready_path.touch()")
+
+    assert fallback < release
+
+    # Never write Git Bash's bare MSYS $$ to the session pid file.
+    assert '\\"$$\\" > {pp}' not in launcher

tests/test_email_account_default_serialization.py

@@ -0,0 +1,522 @@
+"""Regressions for process-safe email-account default mutations.
+
+The file-backed SQLite fixture uses a fresh connection for every Session.
+That exercises the same database lock boundary used by separate web workers,
+rather than relying on an in-process Python lock.
+"""
+
+import asyncio
+import json
+import sys
+import threading
+import types
+from pathlib import Path
+from types import SimpleNamespace
+from unittest import mock
+
+import pytest
+from fastapi import HTTPException
+from sqlalchemy import create_engine, create_mock_engine, text
+from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm import sessionmaker
+from sqlalchemy.pool import NullPool
+
+
+@pytest.fixture
+def account_db(tmp_path, monkeypatch):
+    from core import database as core_db
+
+    engine = create_engine(
+        f"sqlite:///{tmp_path / 'accounts.db'}",
+        connect_args={"check_same_thread": False, "timeout": 5},
+        poolclass=NullPool,
+    )
+    core_db.Base.metadata.create_all(engine)
+    factory = sessionmaker(
+        bind=engine,
+        autocommit=False,
+        autoflush=False,
+    )
+    monkeypatch.setattr(core_db, "SessionLocal", factory)
+    yield factory
+    engine.dispose()
+
+
+def _endpoint(method, path):
+    from routes import email_routes
+
+    with mock.patch.object(email_routes, "_start_poller"):
+        router = email_routes.setup_email_routes()
+    for route in router.routes:
+        if route.path == path and method in getattr(route, "methods", set()):
+            return route.endpoint
+    raise AssertionError(f"email route not found: {method} {path}")
+
+
+def _named_endpoint(router, name):
+    for route in router.routes:
+        if getattr(getattr(route, "endpoint", None), "__name__", "") == name:
+            return route.endpoint
+    raise AssertionError(f"route not found: {name}")
+
+
+def _seed_account(factory, account_id, owner, *, is_default=False, enabled=True):
+    from core.database import EmailAccount
+
+    db = factory()
+    try:
+        db.add(
+            EmailAccount(
+                id=account_id,
+                owner=owner,
+                name=account_id,
+                is_default=is_default,
+                enabled=enabled,
+            )
+        )
+        db.commit()
+    finally:
+        db.close()
+
+
+def _rows(factory):
+    from core.database import EmailAccount
+
+    db = factory()
+    try:
+        return [
+            (row.id, row.owner, bool(row.is_default))
+            for row in db.query(EmailAccount).order_by(EmailAccount.id).all()
+        ]
+    finally:
+        db.close()
+
+
+def _install_lock_pause(monkeypatch, paused_thread_name):
+    """Pause one worker after acquisition and observe another waiting."""
+    from routes import email_routes
+
+    real_lock = email_routes._lock_email_account_owner_mutation
+    first_acquired = threading.Event()
+    release_first = threading.Event()
+    contender_attempted = threading.Event()
+    contender_acquired = threading.Event()
+
+    def controlled_lock(db, owner):
+        is_first = threading.current_thread().name == paused_thread_name
+        if not is_first:
+            contender_attempted.set()
+        real_lock(db, owner)
+        if is_first:
+            first_acquired.set()
+            assert release_first.wait(5), "timed out releasing first mutation"
+        else:
+            contender_acquired.set()
+
+    monkeypatch.setattr(
+        email_routes,
+        "_lock_email_account_owner_mutation",
+        controlled_lock,
+    )
+    return first_acquired, release_first, contender_attempted, contender_acquired
+
+
+def test_concurrent_first_account_creates_choose_one_default(account_db, monkeypatch):
+    create_account = _endpoint("POST", "/api/email/accounts")
+    first_acquired, release_first, attempted, acquired = _install_lock_pause(
+        monkeypatch, "first-account"
+    )
+    results = {}
+
+    def create(name):
+        results[name] = asyncio.run(
+            create_account({"name": name, "is_default": False}, owner="alice")
+        )
+
+    first = threading.Thread(target=create, args=("First",), name="first-account")
+    second = threading.Thread(target=create, args=("Second",), name="second-account")
+    first.start()
+    assert first_acquired.wait(5)
+    second.start()
+    assert attempted.wait(5)
+    assert not acquired.wait(0.1), "second session bypassed the database mutation lock"
+
+    release_first.set()
+    first.join(5)
+    second.join(5)
+
+    assert not first.is_alive()
+    assert not second.is_alive()
+    assert results["First"]["ok"] is True
+    assert results["Second"]["ok"] is True
+    defaults = [row for row in _rows(account_db) if row[2]]
+    assert [(row[1], row[2]) for row in defaults] == [("alice", True)]
+    assert len(defaults) == 1
+
+
+def test_delete_promotion_and_set_default_are_one_serial_transition(
+    account_db, monkeypatch
+):
+    from sqlalchemy.orm import Session as OrmSession
+
+    _seed_account(account_db, "alice-a", "alice", is_default=True)
+    _seed_account(account_db, "alice-b", "alice")
+    _seed_account(account_db, "alice-c", "alice")
+    _seed_account(account_db, "bob-a", "bob", is_default=True)
+
+    delete_account = _endpoint("DELETE", "/api/email/accounts/{account_id}")
+    set_default = _endpoint("POST", "/api/email/accounts/{account_id}/set-default")
+    first_acquired, release_first, attempted, acquired = _install_lock_pause(
+        monkeypatch, "delete-default"
+    )
+    delete_commit_finished = threading.Event()
+    release_delete_after_commit = threading.Event()
+    real_commit = OrmSession.commit
+    results = {}
+
+    def pause_after_delete_commit(session):
+        real_commit(session)
+        if (
+            threading.current_thread().name == "delete-default"
+            and not delete_commit_finished.is_set()
+        ):
+            delete_commit_finished.set()
+            assert release_delete_after_commit.wait(5), (
+                "timed out releasing delete after its first commit"
+            )
+
+    monkeypatch.setattr(OrmSession, "commit", pause_after_delete_commit)
+
+    def delete_old_default():
+        results["delete"] = asyncio.run(
+            delete_account("alice-a", owner="alice")
+        )
+
+    def select_new_default():
+        results["set"] = asyncio.run(
+            set_default("alice-c", owner="alice")
+        )
+
+    delete_thread = threading.Thread(target=delete_old_default, name="delete-default")
+    set_thread = threading.Thread(target=select_new_default, name="set-default")
+    delete_thread.start()
+    assert first_acquired.wait(5)
+    set_thread.start()
+    assert attempted.wait(5)
+    assert not acquired.wait(0.1), "set-default bypassed the delete transaction"
+
+    release_first.set()
+    assert delete_commit_finished.wait(5)
+    # The deletion transaction has committed.  Let the contender complete
+    # before the deleting handler can continue: if promotion were still a
+    # second commit, it would now run after set-default and recreate two
+    # defaults deterministically.
+    assert acquired.wait(5)
+    set_thread.join(5)
+    release_delete_after_commit.set()
+    delete_thread.join(5)
+
+    assert not delete_thread.is_alive()
+    assert not set_thread.is_alive()
+    assert results == {"delete": {"ok": True}, "set": {"ok": True}}
+    assert _rows(account_db) == [
+        ("alice-b", "alice", False),
+        ("alice-c", "alice", True),
+        ("bob-a", "bob", True),
+    ]
+
+
+def test_upgrade_normalizes_legacy_defaults_and_installs_unique_index(
+    tmp_path, monkeypatch
+):
+    """A pre-index schema upgrades without requiring newer account columns."""
+    from core import database as core_db
+
+    engine = create_engine(
+        f"sqlite:///{tmp_path / 'legacy-accounts.db'}",
+        connect_args={"check_same_thread": False},
+        poolclass=NullPool,
+    )
+    try:
+        with engine.begin() as conn:
+            conn.execute(text("""
+                CREATE TABLE email_accounts (
+                    id VARCHAR PRIMARY KEY,
+                    owner VARCHAR,
+                    name VARCHAR NOT NULL,
+                    is_default BOOLEAN NOT NULL,
+                    enabled BOOLEAN NOT NULL,
+                    created_at DATETIME,
+                    updated_at DATETIME
+                )
+            """))
+            conn.execute(text("""
+                INSERT INTO email_accounts
+                    (id, owner, name, is_default, enabled, created_at, updated_at)
+                VALUES
+                    ('legacy-old', NULL, 'Old', 1, 1, '2024-01-01', '2024-01-01'),
+                    ('legacy-new', '', 'New', 1, 1, '2025-01-01', '2025-01-01')
+            """))
+
+        monkeypatch.setattr(core_db, "engine", engine)
+        core_db._migrate_email_account_default_invariant()
+        core_db._migrate_email_account_default_invariant()  # idempotent replay
+
+        with engine.connect() as conn:
+            defaults = conn.execute(text("""
+                SELECT id FROM email_accounts
+                WHERE is_default IS TRUE
+                ORDER BY id
+            """)).scalars().all()
+            index_names = {
+                row[1] for row in conn.execute(text("PRAGMA index_list(email_accounts)"))
+            }
+        assert defaults == ["legacy-old"]
+        assert core_db._EMAIL_ACCOUNT_DEFAULT_INDEX in index_names
+
+        with pytest.raises(IntegrityError):
+            with engine.begin() as conn:
+                conn.execute(text("""
+                    INSERT INTO email_accounts
+                        (id, owner, name, is_default, enabled, created_at, updated_at)
+                    VALUES
+                        ('legacy-third', NULL, 'Third', 1, 1, '2026-01-01', '2026-01-01')
+                """))
+    finally:
+        engine.dispose()
+
+
+def test_concurrent_legacy_seed_is_one_locked_transaction(
+    tmp_path, monkeypatch, caplog
+):
+    from core import database as core_db
+
+    engine = create_engine(
+        f"sqlite:///{tmp_path / 'seed-accounts.db'}",
+        connect_args={"check_same_thread": False, "timeout": 5},
+        poolclass=NullPool,
+    )
+    core_db.Base.metadata.create_all(engine)
+    factory = sessionmaker(bind=engine, autocommit=False, autoflush=False)
+    settings_file = tmp_path / "settings.json"
+    settings_file.write_text(
+        json.dumps({"imap_host": "imap.example.test", "imap_user": "alice"}),
+        encoding="utf-8",
+    )
+    monkeypatch.setattr(core_db, "engine", engine)
+    monkeypatch.setattr(core_db, "SessionLocal", factory)
+    monkeypatch.setattr(core_db, "SETTINGS_FILE", str(settings_file))
+
+    read_barrier = threading.Barrier(2)
+    real_read_text = Path.read_text
+
+    def synchronized_read(path, *args, **kwargs):
+        value = real_read_text(path, *args, **kwargs)
+        if path == settings_file:
+            read_barrier.wait(5)
+        return value
+
+    monkeypatch.setattr(Path, "read_text", synchronized_read)
+    threads = [
+        threading.Thread(target=core_db._migrate_seed_email_account)
+        for _ in range(2)
+    ]
+    try:
+        for thread in threads:
+            thread.start()
+        for thread in threads:
+            thread.join(5)
+        assert all(not thread.is_alive() for thread in threads)
+
+        with engine.connect() as conn:
+            rows = conn.execute(text("""
+                SELECT owner, is_default FROM email_accounts
+                ORDER BY id
+            """)).all()
+        assert rows == [(None, 1)]
+        assert "seed email account migration:" not in caplog.text
+    finally:
+        engine.dispose()
+
+
+def test_multi_owner_row_locks_are_acquired_in_canonical_order():
+    from core.database import lock_email_account_owner_mutations
+
+    class FakeSession:
+        def __init__(self):
+            self.locked = []
+
+        def get_bind(self):
+            return SimpleNamespace(dialect=SimpleNamespace(name="postgresql"))
+
+        def get(self, _model, owner_key, **kwargs):
+            assert kwargs == {"with_for_update": True}
+            self.locked.append(owner_key)
+            return object()
+
+    db = FakeSession()
+    lock_email_account_owner_mutations(db, "zeta", "", "alpha", "zeta")
+    assert db.locked == ["", "alpha", "zeta"]
+
+
+def test_postgresql_fresh_schema_emits_default_unique_index():
+    from core import database as core_db
+
+    statements = []
+    engine_holder = {}
+
+    def capture(statement, *_args, **_kwargs):
+        statements.append(
+            str(statement.compile(dialect=engine_holder["engine"].dialect))
+        )
+
+    mock_engine = create_mock_engine("postgresql://", capture)
+    engine_holder["engine"] = mock_engine
+    core_db.EmailAccount.__table__.create(mock_engine)
+
+    assert any(
+        core_db._EMAIL_ACCOUNT_DEFAULT_INDEX in statement
+        and "COALESCE(owner, '')" in statement
+        and "WHERE is_default IS TRUE" in statement
+        for statement in statements
+    )
+
+
+def test_rename_serializes_old_and_new_owner_and_stale_set_default_fails_closed(
+    account_db, monkeypatch, tmp_path
+):
+    from core import database as core_db
+    from routes import auth_routes
+
+    _seed_account(account_db, "alice-a", "alice", is_default=True)
+    _seed_account(account_db, "alice-b", "alice")
+    _seed_account(account_db, "bob-a", "bob", is_default=True)
+
+    prefs_module = types.ModuleType("routes.prefs_routes")
+    prefs_module._load = lambda: {}
+    prefs_module._save = lambda _data: None
+    monkeypatch.setitem(sys.modules, "routes.prefs_routes", prefs_module)
+    monkeypatch.setattr(
+        auth_routes, "DEEP_RESEARCH_DIR", str(tmp_path / "deep_research")
+    )
+    monkeypatch.setattr(auth_routes, "MEMORY_FILE", str(tmp_path / "memory.json"))
+    monkeypatch.setattr(auth_routes, "SKILLS_DIR", str(tmp_path / "skills"))
+
+    auth_manager = mock.MagicMock()
+    auth_manager.get_username_for_token.return_value = "admin"
+    auth_manager.is_admin.return_value = True
+    auth_manager.users = {"admin": {}, "alice": {}}
+    auth_manager.rename_user.return_value = True
+    rename_user = _named_endpoint(
+        auth_routes.setup_auth_routes(auth_manager), "rename_user"
+    )
+    set_default = _endpoint("POST", "/api/email/accounts/{account_id}/set-default")
+
+    rename_acquired = threading.Event()
+    release_rename = threading.Event()
+    set_attempted = threading.Event()
+    set_acquired = threading.Event()
+    real_lock = core_db.lock_email_account_owner_mutations
+
+    def controlled_lock(db, *owners):
+        thread_name = threading.current_thread().name
+        if thread_name == "rename-owner":
+            real_lock(db, *owners)
+            rename_acquired.set()
+            assert release_rename.wait(5)
+            return
+        if thread_name == "stale-set-default":
+            set_attempted.set()
+            real_lock(db, *owners)
+            set_acquired.set()
+            return
+        real_lock(db, *owners)
+
+    monkeypatch.setattr(core_db, "lock_email_account_owner_mutations", controlled_lock)
+    request = SimpleNamespace(
+        cookies={"odysseus_session": "admin-token"},
+        app=SimpleNamespace(
+            state=SimpleNamespace(
+                invalidate_token_cache=lambda: None,
+                session_manager=None,
+                research_handler=None,
+                upload_handler=None,
+                personal_docs_manager=None,
+            )
+        ),
+    )
+    results = {}
+
+    def rename_owner():
+        results["rename"] = asyncio.run(
+            rename_user("alice", SimpleNamespace(username="bob"), request)
+        )
+
+    def select_stale_default():
+        try:
+            results["set"] = asyncio.run(
+                set_default("alice-b", owner="alice")
+            )
+        except Exception as exc:  # asserted below with its HTTP status
+            results["set_error"] = exc
+
+    rename_thread = threading.Thread(target=rename_owner, name="rename-owner")
+    set_thread = threading.Thread(
+        target=select_stale_default, name="stale-set-default"
+    )
+    rename_thread.start()
+    assert rename_acquired.wait(5)
+    set_thread.start()
+    assert set_attempted.wait(5)
+    assert not set_acquired.wait(0.1), "set-default bypassed the rename lock"
+
+    release_rename.set()
+    rename_thread.join(5)
+    set_thread.join(5)
+
+    assert not rename_thread.is_alive()
+    assert not set_thread.is_alive()
+    assert results["rename"]["ok"] is True
+    assert isinstance(results["set_error"], HTTPException)
+    assert results["set_error"].status_code == 404
+    assert _rows(account_db) == [
+        ("alice-a", "bob", False),
+        ("alice-b", "bob", False),
+        ("bob-a", "bob", True),
+    ]
+
+
+def test_demo_teardown_promotes_replacement_in_same_transaction(
+    account_db, monkeypatch
+):
+    from core.database import EmailAccount
+    from scripts.demo_email import demo_account
+
+    db = account_db()
+    try:
+        db.add_all([
+            EmailAccount(
+                id="real",
+                owner="",
+                name="Real",
+                is_default=False,
+                enabled=True,
+            ),
+            EmailAccount(
+                id="demo",
+                owner="",
+                name=demo_account.NAME,
+                imap_user=demo_account.IMAP_USER,
+                is_default=True,
+                enabled=True,
+            ),
+        ])
+        db.commit()
+    finally:
+        db.close()
+
+    monkeypatch.setattr(demo_account, "SessionLocal", account_db)
+    monkeypatch.setattr(demo_account, "engine", account_db.kw["bind"])
+
+    assert demo_account.teardown() == 0
+    assert _rows(account_db) == [("real", "", True)]

tests/test_email_oauth.py

@@ -29,6 +29,7 @@ import base64
 import json
 import time
 import unittest.mock as mock
+from types import SimpleNamespace
 
 import pytest
 
@@ -272,8 +273,14 @@ def _callback_endpoint():
 
 
 class _FakeRequest:
-    """Minimal stand-in for starlette Request — the callback only reads headers."""
+    """Minimal stand-in for starlette Request — the callback reads the Host header
-    headers = {"host": "localhost:7000"}
+    and the request scheme. Behind a TLS terminator uvicorn's proxy-headers
+    middleware rewrites the scheme from `X-Forwarded-Proto`, so the route sees
+    `https` there and `http` on a plain origin."""
+
+    def __init__(self, scheme="http", host="localhost:7000"):
+        self.headers = {"host": host}
+        self.url = SimpleNamespace(scheme=scheme)
 
 
 def _location(resp):
@@ -415,6 +422,119 @@ async def test_callback_valid_owner_writes_encrypted_tokens_to_intended_account(
     assert other.oauth_access_token is None, "tokens must only touch the intended account"
 
 
+# ── Redirect URI scheme ───────────────────────────────────────────
+#
+# Google rejects the token exchange unless the callback's `redirect_uri` is
+# byte-identical to the one the authorize step sent, so both routes have to
+# agree — including on the scheme. Deriving it from the request keeps HTTPS
+# deployments working without pinning GOOGLE_OAUTH_REDIRECT_URI by hand;
+# hardcoding `http://` produced an unusable redirect behind any TLS front.
+
+def _authorize_endpoint():
+    """Return the live google_oauth_authorize endpoint from the email router."""
+    from routes.email_routes import setup_email_routes
+    router = setup_email_routes()
+    for route in router.routes:
+        if route.path == "/api/email/oauth/google/authorize" and "GET" in getattr(route, "methods", set()):
+            return route.endpoint
+    raise AssertionError("google_oauth_authorize route not found")
+
+
+def _posted_redirect_uri(mock_post):
+    """Pull `redirect_uri` out of the mocked Google token-exchange POST."""
+    return mock_post.call_args.kwargs["data"]["redirect_uri"]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("scheme", ("http", "https"))
+async def test_callback_redirect_uri_follows_the_request_scheme(scheme, monkeypatch):
+    """The token exchange must echo the scheme the request actually arrived on —
+    `https` behind a TLS terminator, `http` on a plain origin."""
+    from routes.email_helpers import make_oauth_state
+
+    monkeypatch.delenv("GOOGLE_OAUTH_REDIRECT_URI", raising=False)
+
+    db, Factory = _make_db()
+    _make_account(db, account_id="acct-s", owner="alice", imap_user="alice@example.com")
+    db.close()
+
+    token_resp = mock.MagicMock()
+    token_resp.raise_for_status = mock.MagicMock()
+    token_resp.json.return_value = {"access_token": "ya29.t", "refresh_token": "1//r", "expires_in": 3600}
+    userinfo_resp = mock.MagicMock()
+    userinfo_resp.is_success = True
+    userinfo_resp.json.return_value = {"email": "alice@example.com", "name": "Alice"}
+
+    state = make_oauth_state("acct-s", "alice")
+
+    with mock.patch("httpx.post", return_value=token_resp) as mock_post, \
+         mock.patch("httpx.get", return_value=userinfo_resp), \
+         mock.patch("core.database.SessionLocal", Factory):
+        callback = _callback_endpoint()
+        await callback(
+            code="4/code", state=state, error=None,
+            request=_FakeRequest(scheme=scheme, host="odysseus.example.ts.net:7443"),
+        )
+
+    assert _posted_redirect_uri(mock_post) == (
+        f"{scheme}://odysseus.example.ts.net:7443/api/email/oauth/google/callback"
+    )
+
+
+@pytest.mark.asyncio
+async def test_callback_redirect_uri_env_override_still_wins(monkeypatch):
+    """An explicit GOOGLE_OAUTH_REDIRECT_URI is used verbatim — deriving the
+    scheme must not override a value the operator pinned by hand."""
+    from routes.email_helpers import make_oauth_state
+
+    pinned = "https://mail.example.com/api/email/oauth/google/callback"
+    monkeypatch.setenv("GOOGLE_OAUTH_REDIRECT_URI", pinned)
+
+    db, Factory = _make_db()
+    _make_account(db, account_id="acct-p", owner="alice", imap_user="alice@example.com")
+    db.close()
+
+    token_resp = mock.MagicMock()
+    token_resp.raise_for_status = mock.MagicMock()
+    token_resp.json.return_value = {"access_token": "ya29.t", "refresh_token": "1//r", "expires_in": 3600}
+    userinfo_resp = mock.MagicMock()
+    userinfo_resp.is_success = True
+    userinfo_resp.json.return_value = {"email": "alice@example.com", "name": "Alice"}
+
+    state = make_oauth_state("acct-p", "alice")
+
+    with mock.patch("httpx.post", return_value=token_resp) as mock_post, \
+         mock.patch("httpx.get", return_value=userinfo_resp), \
+         mock.patch("core.database.SessionLocal", Factory):
+        callback = _callback_endpoint()
+        await callback(code="4/code", state=state, error=None, request=_FakeRequest(scheme="http"))
+
+    assert _posted_redirect_uri(mock_post) == pinned
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("scheme", ("http", "https"))
+async def test_authorize_redirect_uri_follows_the_request_scheme(scheme, monkeypatch):
+    """The authorize step builds the same redirect_uri the callback will send.
+    `owner=""` is the unconfigured / single-user case, so no DB is touched."""
+    import urllib.parse
+
+    monkeypatch.delenv("GOOGLE_OAUTH_REDIRECT_URI", raising=False)
+    monkeypatch.setenv("GOOGLE_OAUTH_CLIENT_ID", "client-id.apps.googleusercontent.com")
+
+    authorize = _authorize_endpoint()
+    resp = await authorize(
+        account_id="acct-a",
+        request=_FakeRequest(scheme=scheme, host="odysseus.example.ts.net:7443"),
+        owner="",
+    )
+
+    query = urllib.parse.parse_qs(urllib.parse.urlparse(resp.headers["location"]).query)
+    assert query["redirect_uri"] == [
+        f"{scheme}://odysseus.example.ts.net:7443/api/email/oauth/google/callback"
+    ]
+
+
 @pytest.mark.asyncio
 async def test_callback_rejects_token_for_a_different_mailbox_identity():
     """Reconnecting with another Google identity must not replace the token

tests/test_foreground_model_routing.py

@@ -0,0 +1,277 @@
+"""Regression coverage for strict foreground model selection."""
+
+import asyncio
+import json
+from types import SimpleNamespace
+
+import pytest
+import src.agent_loop as agent_loop
+import src.endpoint_resolver as endpoint_resolver
+import src.foreground_model_routing as foreground_model_routing
+import routes.chat_routes as chat_routes
+from src.foreground_model_routing import (
+    build_foreground_model_candidates,
+    resolve_foreground_fallback_candidates,
+)
+
+
+def _collect(gen):
+    async def _run():
+        return [chunk async for chunk in gen]
+
+    return asyncio.run(_run())
+
+
+class _EmptyQuery:
+    def filter(self, *args, **kwargs):
+        return self
+
+    def order_by(self, *args, **kwargs):
+        return self
+
+    def first(self):
+        return None
+
+
+class _EmptyDb:
+    def query(self, *args, **kwargs):
+        return _EmptyQuery()
+
+    def close(self):
+        return None
+
+
+class _RouteRequest:
+    def __init__(self, mode):
+        self.headers = {}
+        self.app = SimpleNamespace(state=SimpleNamespace(auth_manager=None))
+        self._form = {
+            "message": "hello",
+            "session": "session-1",
+            "mode": mode,
+            "compare_mode": "true",
+        }
+
+    async def form(self):
+        return self._form
+
+
+def _chat_stream_endpoint(monkeypatch, mode, captured):
+    session = SimpleNamespace(
+        endpoint_url="https://selected.example/v1",
+        model="selected-model",
+        headers={"Authorization": "Bearer selected"},
+        name="test",
+        history=[],
+        add_message=lambda message: None,
+    )
+    session_manager = SimpleNamespace(
+        get_session=lambda session_id: session,
+        save_sessions=lambda: None,
+    )
+    context = SimpleNamespace(
+        user="alice",
+        messages=[{"role": "user", "content": "hello"}],
+        preprocessed=SimpleNamespace(attachment_meta=[]),
+        auto_opened_docs=[],
+        rag_sources=[],
+        web_sources=[],
+        used_memories=[],
+        uploaded_files=[],
+        uprefs={},
+        was_compacted=False,
+        context_trimmed=False,
+        context_length=4096,
+        context_messages_before_trim=1,
+        context_messages_after_trim=1,
+        context_tokens_before_trim=10,
+        context_tokens_after_trim=10,
+        preset=SimpleNamespace(temperature=0.2, max_tokens=128, character_name=None),
+    )
+
+    async def fake_build_context(*args, **kwargs):
+        return context
+
+    async def fake_chat_stream(candidates, messages, **kwargs):
+        captured["chat"] = candidates
+        yield f'data: {json.dumps({"delta": "done"})}\n\n'
+        yield "data: [DONE]\n\n"
+
+    async def fake_agent_stream(endpoint_url, model, messages, **kwargs):
+        captured["agent"] = {
+            "primary": (endpoint_url, model, kwargs.get("headers")),
+            "fallbacks": kwargs.get("fallbacks"),
+        }
+        yield f'data: {json.dumps({"delta": "done"})}\n\n'
+        yield "data: [DONE]\n\n"
+
+    monkeypatch.setattr(chat_routes, "coerce_message_and_session", lambda *args, **kwargs: ("hello", "session-1"))
+    monkeypatch.setattr(chat_routes, "_verify_session_owner", lambda *args, **kwargs: None)
+    monkeypatch.setattr(chat_routes, "effective_user", lambda request: "alice")
+    monkeypatch.setattr(chat_routes, "_clear_orphaned_session_endpoint", lambda *args, **kwargs: False)
+    monkeypatch.setattr(chat_routes, "_recover_empty_session_model", lambda *args, **kwargs: False)
+    monkeypatch.setattr(chat_routes, "_enforce_chat_privileges", lambda *args, **kwargs: None)
+    monkeypatch.setattr(chat_routes, "resolve_session_auth", lambda *args, **kwargs: None)
+    monkeypatch.setattr(chat_routes, "get_session_mode", lambda session_id: "chat")
+    monkeypatch.setattr(chat_routes, "set_session_mode", lambda *args, **kwargs: None)
+    monkeypatch.setattr(chat_routes, "build_chat_context", fake_build_context)
+    monkeypatch.setattr(chat_routes, "SessionLocal", _EmptyDb)
+    monkeypatch.setattr(chat_routes, "_is_image_generation_session", lambda *args, **kwargs: False)
+    monkeypatch.setattr(chat_routes, "stream_llm_with_fallback", fake_chat_stream)
+    monkeypatch.setattr(chat_routes, "stream_agent_loop", fake_agent_stream)
+    monkeypatch.setattr(chat_routes, "save_assistant_response", lambda *args, **kwargs: None)
+    monkeypatch.setattr(chat_routes, "run_post_response_tasks", lambda *args, **kwargs: None)
+    monkeypatch.setattr(chat_routes, "estimate_tokens", lambda messages: 10)
+    monkeypatch.setattr(
+        endpoint_resolver,
+        "resolve_chat_fallback_candidates",
+        lambda owner=None: [("https://legacy.example/v1", "legacy-model", {})],
+    )
+
+    import src.settings as settings
+
+    monkeypatch.setattr(
+        settings,
+        "get_setting",
+        lambda key, default=None: default,
+    )
+    monkeypatch.setattr(
+        settings,
+        "get_user_setting",
+        lambda key, owner="", default=None: (
+            [{"endpoint_id": "legacy", "model": "legacy-model"}]
+            if key == "default_model_fallbacks"
+            else default
+        ),
+    )
+
+    router = chat_routes.setup_chat_routes(
+        session_manager,
+        SimpleNamespace(),
+        SimpleNamespace(),
+        SimpleNamespace(),
+        SimpleNamespace(),
+        SimpleNamespace(),
+    )
+    return next(route.endpoint for route in router.routes if route.path == "/api/chat_stream")
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("mode", ["chat", "agent"])
+async def test_chat_stream_route_keeps_selected_model_strict_with_legacy_data(monkeypatch, mode):
+    captured = {}
+    endpoint = _chat_stream_endpoint(monkeypatch, mode, captured)
+
+    response = await endpoint(_RouteRequest(mode))
+    async for _ in response.body_iterator:
+        pass
+
+    selected = (
+        "https://selected.example/v1",
+        "selected-model",
+        {"Authorization": "Bearer selected"},
+    )
+    if mode == "chat":
+        assert captured == {"chat": [selected]}
+    else:
+        assert captured == {"agent": {"primary": selected, "fallbacks": []}}
+
+
+def test_candidate_builder_appends_only_policy_authorized_fallbacks(monkeypatch):
+    """Chat and Agent share the same candidate-building policy boundary."""
+
+    authorized = [("https://opt-in.example/v1", "opt-in-model", {})]
+    monkeypatch.setattr(
+        foreground_model_routing,
+        "resolve_foreground_fallback_candidates",
+        lambda owner=None: authorized,
+    )
+
+    assert build_foreground_model_candidates(
+        "https://selected.example/v1",
+        "selected-model",
+        {"Authorization": "Bearer selected"},
+        owner="alice",
+    ) == [
+        ("https://selected.example/v1", "selected-model", {"Authorization": "Bearer selected"}),
+        *authorized,
+    ]
+
+
+def test_strict_policy_builds_only_the_selected_chat_candidate():
+    candidates = build_foreground_model_candidates(
+        "https://selected.example/v1",
+        "selected-model",
+        {"Authorization": "Bearer selected"},
+        owner="alice",
+    )
+
+    assert candidates == [
+        ("https://selected.example/v1", "selected-model", {"Authorization": "Bearer selected"})
+    ]
+
+
+def test_legacy_chat_resolver_is_disconnected():
+    assert endpoint_resolver.resolve_chat_fallback_candidates(owner="alice") == []
+
+
+def test_utility_resolver_does_not_inherit_legacy_chat_fallbacks(monkeypatch):
+    seen_keys = []
+
+    def fake_resolve(setting_key, owner=None):
+        seen_keys.append((setting_key, owner))
+        return [("https://utility.example/v1", "utility-model", {})]
+
+    monkeypatch.setattr(endpoint_resolver, "_resolve_fallback_candidates", fake_resolve)
+
+    assert endpoint_resolver.resolve_utility_fallback_candidates(owner="alice") == [
+        ("https://utility.example/v1", "utility-model", {})
+    ]
+    assert seen_keys == [("utility_model_fallbacks", "alice")]
+
+
+def test_multi_round_agent_uses_only_selected_model(monkeypatch):
+    """Every Agent round receives only the selected foreground candidate."""
+
+    seen_candidates = []
+    round_number = 0
+
+    monkeypatch.setattr(agent_loop, "get_setting", lambda key, default=None: default)
+    monkeypatch.setattr(agent_loop, "get_mcp_manager", lambda: None)
+    monkeypatch.setattr(agent_loop, "estimate_tokens", lambda *args, **kwargs: 10)
+
+    async def fake_stream(candidates, messages, **kwargs):
+        nonlocal round_number
+        round_number += 1
+        seen_candidates.append([(url, model) for url, model, _headers in candidates])
+        if round_number == 1:
+            call = {"name": "bash", "arguments": json.dumps({"command": "printf ok"})}
+            yield f'data: {json.dumps({"type": "tool_calls", "calls": [call]})}\n\n'
+        else:
+            yield f'data: {json.dumps({"delta": "done"})}\n\n'
+        yield "data: [DONE]\n\n"
+
+    async def fake_execute(block, *args, **kwargs):
+        return "bash", {"output": "ok", "exit_code": 0}
+
+    monkeypatch.setattr(agent_loop, "stream_llm_with_fallback", fake_stream)
+    monkeypatch.setattr(agent_loop, "execute_tool_block", fake_execute)
+
+    fallbacks = resolve_foreground_fallback_candidates(owner="alice")
+    chunks = _collect(
+        agent_loop.stream_agent_loop(
+            "https://selected.example/v1",
+            "selected-model",
+            [{"role": "user", "content": "Run one tool and report back."}],
+            max_rounds=3,
+            relevant_tools={"bash"},
+            fallbacks=fallbacks,
+            _is_teacher_run=True,
+        )
+    )
+
+    assert seen_candidates == [
+        [("https://selected.example/v1", "selected-model")],
+        [("https://selected.example/v1", "selected-model")],
+    ]
+    assert any('"delta": "done"' in chunk for chunk in chunks)

tests/test_gallery_model_input_device.py

@@ -0,0 +1,34 @@
+import routes.gallery_routes as gallery_routes
+
+
+class _TorchSentinel:
+    float32 = object()
+    float64 = object()
+
+
+class _FakeTensor:
+    def __init__(self, dtype):
+        self.dtype = dtype
+        self.to_args = None
+
+    def to(self, *args, **kwargs):
+        self.to_args = (args, kwargs)
+        return self
+
+
+def test_model_inputs_to_device_casts_mps_float64_to_float32():
+    float_tensor = _FakeTensor(_TorchSentinel.float64)
+    int_tensor = _FakeTensor("int64")
+    plain_value = object()
+
+    result = gallery_routes._model_inputs_to_device(
+        {"points": float_tensor, "labels": int_tensor, "plain": plain_value},
+        "mps",
+        _TorchSentinel,
+    )
+
+    assert result["points"] is float_tensor
+    assert float_tensor.to_args == ((), {"device": "mps", "dtype": _TorchSentinel.float32})
+    assert int_tensor.to_args == (("mps",), {})
+    assert result["plain"] is plain_value
+

tests/test_harmony_tool_aliasing.py

@@ -0,0 +1,78 @@
+"""Tools whose names collide with harmony built-ins must be aliased for gpt-oss.
+
+gpt-oss (harmony format) ships BUILT-IN tools named `python` and `browser`,
+invoked with the raw body as the argument (`to=python` + bare source), while
+custom functions use `to=functions.NAME` + JSON. Exposing our own tool under a
+built-in's name makes the model answer with the built-in convention: it emits
+raw code, the server tries to parse it as JSON, and the request dies with
+"error parsing tool call: raw='import sys, ...'". Streaming is worse — Ollama
+truncates the stream instead of reporting it, so the turn looks like an empty
+response and the agent loop reads it as a stall.
+
+Measured on gpt-oss:20b via Ollama /v1 with a fixed agentic prompt:
+python+bash as-is 2/6, python renamed 5/6, both renamed 6/6.
+
+The aliasing is transport-only and gpt-oss-only: every other model's tool
+schemas must pass through untouched, and real tool names must come back out.
+"""
+from src.llm_core import (
+    _alias_harmony_tools,
+    _unalias_harmony_tool_name,
+    _is_harmony_model,
+)
+
+
+def _tools(*names):
+    return [
+        {"type": "function", "function": {"name": n, "parameters": {}}}
+        for n in names
+    ]
+
+
+def _names(tools):
+    return [t["function"]["name"] for t in tools]
+
+
+def test_gpt_oss_colliding_names_are_aliased():
+    out = _alias_harmony_tools(_tools("python", "bash", "web_search"), "gpt-oss:20b")
+    assert _names(out) == ["run_python_code", "run_shell_command", "web_search"]
+
+
+def test_non_harmony_models_are_untouched():
+    tools = _tools("python", "bash", "web_search")
+    for model in ("qwen3-coder:30b", "gemma4:12b", "claude-opus-5", "gpt-4o", "llama-3.3"):
+        out = _alias_harmony_tools(tools, model)
+        assert _names(out) == ["python", "bash", "web_search"], model
+        assert out is tools, f"{model} should get the same list object, not a copy"
+
+
+def test_aliasing_does_not_mutate_the_caller_list():
+    tools = _tools("python")
+    _alias_harmony_tools(tools, "gpt-oss:20b")
+    assert _names(tools) == ["python"], "caller's schema list must not be mutated"
+
+
+def test_response_names_map_back_for_gpt_oss():
+    assert _unalias_harmony_tool_name("run_python_code", "gpt-oss:20b") == "python"
+    assert _unalias_harmony_tool_name("run_shell_command", "gpt-oss:20b") == "bash"
+    # Unrelated names pass through untouched.
+    assert _unalias_harmony_tool_name("web_search", "gpt-oss:20b") == "web_search"
+
+
+def test_response_names_untouched_for_other_models():
+    # A non-harmony model that genuinely has a tool called run_python_code
+    # must not have it rewritten to `python`.
+    assert _unalias_harmony_tool_name("run_python_code", "qwen3-coder:30b") == "run_python_code"
+
+
+def test_harmony_detection():
+    assert _is_harmony_model("gpt-oss:20b") is True
+    assert _is_harmony_model("GPT-OSS:120B") is True
+    assert _is_harmony_model("qwen3-coder:30b") is False
+    assert _is_harmony_model("") is False
+    assert _is_harmony_model(None) is False
+
+
+def test_empty_and_none_tools_are_safe():
+    assert _alias_harmony_tools(None, "gpt-oss:20b") is None
+    assert _alias_harmony_tools([], "gpt-oss:20b") == []

tests/test_legacy_default_fallback_ui.py

@@ -0,0 +1,29 @@
+"""The retired default fallback editor must not imply active routing."""
+
+from pathlib import Path
+
+from bs4 import BeautifulSoup
+
+
+_REPO = Path(__file__).resolve().parents[1]
+
+
+def test_legacy_default_fallback_editor_is_hidden():
+    soup = BeautifulSoup(
+        (_REPO / "static" / "index.html").read_text(encoding="utf-8"),
+        "html.parser",
+    )
+    editor = soup.find(id="set-defaultFallbacks")
+
+    assert editor is not None
+    assert editor.find_parent(class_="settings-row").has_attr("hidden")
+
+
+def test_default_model_save_does_not_rewrite_legacy_fallbacks():
+    source = (_REPO / "static" / "js" / "settings.js").read_text(encoding="utf-8")
+    start = source.index("async function initDefaultChat()")
+    end = source.index("/* ── Utility Model ── */", start)
+    default_chat_source = source[start:end]
+
+    assert "settings.default_model_fallbacks" not in default_chat_source
+    assert "default_model_fallbacks:" not in default_chat_source

tests/test_llm_core_async_mistral_content.py

@@ -0,0 +1,71 @@
+"""Integration regression test for #5435.
+
+llm_call_async must normalise Mistral structured content to a plain string,
+matching llm_call (sync) and stream_llm. Before the fix, the async
+non-streaming parser returned the raw list when Mistral reasoning was enabled,
+violating its -> str contract, leaking a non-string into callers such as
+auto-title generation and memory extraction, and poisoning _response_cache
+with a non-string value.
+"""
+import asyncio
+
+import src.llm_core as llm_core
+
+
+class _FakeResponse:
+    is_success = True
+    status_code = 200
+    text = ""
+
+    def __init__(self, payload):
+        self._payload = payload
+
+    def json(self):
+        return self._payload
+
+
+def _payload(content):
+    return {"choices": [{"message": {"role": "assistant", "content": content}}]}
+
+
+def _call(monkeypatch, content):
+    async def fake_post(client, url, headers, **kwargs):
+        return _FakeResponse(_payload(content))
+
+    monkeypatch.setattr(llm_core, "httpx_post_kimi_aware_async", fake_post)
+    llm_core._response_cache.clear()
+    return asyncio.run(llm_core.llm_call_async(
+        "http://mistral.test/v1/chat/completions",
+        "mistral-medium",
+        [{"role": "user", "content": "q"}],
+    ))
+
+
+def test_llm_call_async_normalizes_mistral_structured_content(monkeypatch):
+    out = _call(monkeypatch, [
+        {"type": "thinking",
+         "thinking": [{"type": "text", "text": "Let me work through this..."}],
+         "closed": True},
+        {"type": "text", "text": "The answer is 42."},
+    ])
+    assert isinstance(out, str), f"expected str, got {type(out).__name__}"
+    assert "The answer is 42." in out
+    assert "Let me work through this..." in out
+    # The cache must hold the normalised string, not the raw list,
+    # otherwise repeat calls serve the poisoned value.
+    assert all(isinstance(v, str) for v in llm_core._response_cache.values())
+
+
+def test_llm_call_async_thinking_only_still_returns_str(monkeypatch):
+    out = _call(monkeypatch, [
+        {"type": "thinking",
+         "thinking": [{"type": "text", "text": "still thinking"}],
+         "closed": True},
+    ])
+    assert isinstance(out, str)
+    assert "still thinking" in out
+
+
+def test_llm_call_async_plain_string_passthrough(monkeypatch):
+    out = _call(monkeypatch, "plain answer")
+    assert out == "plain answer"

tests/test_llm_core_thinking_models.py

@@ -0,0 +1,27 @@
+"""Regression coverage for structured-thinking model detection."""
+
+import os
+
+os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:")
+
+import pytest
+
+from src.llm_core import _supports_thinking
+
+
+@pytest.mark.parametrize(
+    "model",
+    [
+        "deepseek-v4",
+        "deepseek-v4-flash",
+        "DeepSeek-V4-Flash",
+        "deepseek/deepseek-v4-flash",
+    ],
+)
+def test_deepseek_v4_models_support_thinking(model):
+    assert _supports_thinking(model) is True
+
+
+@pytest.mark.parametrize("model", ["deepseek-v3", "deepseek-chat"])
+def test_other_deepseek_models_are_not_promoted_to_thinking(model):
+    assert _supports_thinking(model) is False

tests/test_model_defaults.py

@@ -144,10 +144,10 @@ def test_get_default_chat_user_no_prefs_share_disabled_resolves_nothing(monkeypa
     assert test_data["model"] == "", "Should get empty model"
 
 
-def test_get_default_chat_user_no_prefs_share_enabled_resolves_global_defaults_fallbacks(monkeypatch):
+def test_get_default_chat_user_no_prefs_share_enabled_resolves_global_defaults(monkeypatch):
     """
     Non-admin user without personal preferences should resolve to global
-    defaults for ep_id, model, and fallbacks when share_defaults_with_users is enabled.
+    defaults for ep_id and model when share_defaults_with_users is enabled.
     """
 
     test_data = _run_get_default_chat_test(monkeypatch, share_defaults_enabled=True)
@@ -158,16 +158,45 @@ def test_get_default_chat_user_no_prefs_share_enabled_resolves_global_defaults_f
     assert test_data["endpoint_id"] == "global-ep-123", \
         "Should get global endpoint_id"
 
-def test_get_default_chat_user_no_prefs_share_enabled_resolves_global_defaults(monkeypatch):
+def test_get_default_chat_does_not_read_legacy_fallbacks(monkeypatch):
     """
-    Non-admin user without personal preferences should resolve to global
+    The preserved legacy list must not influence default model resolution.
-    defaults for ep_id, model, and fallbacks when share_defaults_with_users is enabled.
     """
 
-    test_data = _run_get_default_chat_test(monkeypatch, share_defaults_enabled=True, second_endpoint_only=True)
+    class LegacyReadGuard(dict):
+        def get(self, key, default=None):
+            if key == "default_model_fallbacks":
+                raise AssertionError("legacy fallback list was read")
+            return super().get(key, default)
 
-    assert test_data["model"] == "qwen-3.6", \
+    guarded_settings = LegacyReadGuard({
-        "model should be resolved from global default_model"
+        "default_endpoint_id": "global-ep-123",
+        "default_model": "qwen-3.6",
+        "default_model_fallbacks": [
+            {"endpoint_id": "fallback-ep", "model": "fallback-model"}
+        ],
+        "share_defaults_with_users": True,
+    })
+    monkeypatch.setattr(model_routes, "_load_settings", lambda: guarded_settings)
+    monkeypatch.setattr(prefs_routes, "_load_for_user", lambda user: LegacyReadGuard({}))
 
-    assert test_data["endpoint_id"] == "fallback-ep", \
+    fake_auth_manager = MagicMock()
-        "Should get global endpoint_id"
+    fake_auth_manager.is_admin = lambda user: False
+    endpoint = _FakeEndpoint(
+        id="global-ep-123",
+        base_url="http://global-endpoint:8000/v1",
+        is_enabled=True,
+    )
+    fake_db = _make_db_session([endpoint], user="regular_user")
+    monkeypatch.setattr(model_routes, "SessionLocal", lambda: fake_db)
+    monkeypatch.setattr(model_routes, "_normalize_base", lambda url: url)
+    monkeypatch.setattr(model_routes, "build_chat_url", lambda base: f"{base}/chat")
+
+    router = model_routes.setup_model_routes(model_discovery=None)
+    get_default_chat = _get_default_chat_route(router)
+    fake_request = _make_request(user="regular_user", auth_manager=fake_auth_manager)
+
+    test_data = get_default_chat(fake_request)
+
+    assert test_data["endpoint_id"] == "global-ep-123"
+    assert test_data["model"] == "qwen-3.6"

tests/test_model_routes.py

@@ -90,7 +90,7 @@ def test_clear_speech_endpoint_settings_resets_tts_and_stt():
     }
 
 
-def test_endpoint_cleanup_removes_primary_and_fallback_references():
+def test_endpoint_cleanup_preserves_legacy_default_fallback_data():
     settings = {
         "default_endpoint_id": "dead",
         "default_model": "primary",
@@ -106,14 +106,12 @@ def test_endpoint_cleanup_removes_primary_and_fallback_references():
 
     assert _endpoint_settings_using_endpoint(settings, "dead", include_speech=True) == [
         "Default Model",
-        "Default Model Fallbacks",
         "Utility Model Fallbacks",
         "Vision Model Fallbacks",
         "Speech to Text",
     ]
     assert _clear_endpoint_settings_for_endpoint(settings, "dead", include_speech=True) == [
         "Default Model",
-        "Default Model Fallbacks",
         "Utility Model Fallbacks",
         "Vision Model Fallbacks",
         "Speech to Text",
@@ -121,6 +119,7 @@ def test_endpoint_cleanup_removes_primary_and_fallback_references():
     assert settings["default_endpoint_id"] == ""
     assert settings["default_model"] == ""
     assert settings["default_model_fallbacks"] == [
+        {"endpoint_id": "dead", "model": "fallback-a"},
         {"endpoint_id": "keep", "model": "fallback-b"},
     ]
     assert settings["utility_model_fallbacks"] == []
@@ -129,7 +128,7 @@ def test_endpoint_cleanup_removes_primary_and_fallback_references():
     assert settings["stt_model"] == "base"
 
 
-def test_endpoint_cleanup_updates_scoped_and_legacy_user_prefs():
+def test_endpoint_cleanup_updates_active_scoped_prefs_but_preserves_legacy_data():
     scoped = {
         "_users": {
             "alice": {
@@ -154,8 +153,10 @@ def test_endpoint_cleanup_updates_scoped_and_legacy_user_prefs():
     legacy = {
         "default_model_fallbacks": [{"endpoint_id": "dead", "model": "chat"}],
     }
-    assert _clear_user_pref_endpoint_refs(legacy, "dead") == 1
+    assert _clear_user_pref_endpoint_refs(legacy, "dead") == 0
-    assert legacy["default_model_fallbacks"] == []
+    assert legacy["default_model_fallbacks"] == [
+        {"endpoint_id": "dead", "model": "chat"}
+    ]
 
 
 # ── _default_endpoint_needs_assignment (add-endpoint auto-default) ──

tests/test_rename_user_owner_sync.py

@@ -114,6 +114,12 @@ def _force_sql_owner_migration_failure(monkeypatch):
         def filter(self, *_args, **_kwargs):
             return self
 
+        def order_by(self, *_args, **_kwargs):
+            return self
+
+        def all(self):
+            return []
+
         def update(self, *_args, **_kwargs):
             raise RuntimeError("forced owner migration failure")
 
@@ -125,6 +131,12 @@ def _force_sql_owner_migration_failure(monkeypatch):
         def query(self, _model):
             return FailingQuery()
 
+        def get_bind(self):
+            return SimpleNamespace(dialect=SimpleNamespace(name="postgresql"))
+
+        def get(self, _model, _key, **_kwargs):
+            return object()
+
         def rollback(self):
             self.rolled_back = True
 

tests/test_skill_frontmatter_escape_roundtrip.py

@@ -0,0 +1,214 @@
+"""Regression for issue #5210 — SKILL.md frontmatter scalars must round-trip.
+
+``_emit_scalar`` quotes a scalar with ``json.dumps`` whenever it contains
+punctuation that would change how the line reads back. ``_parse_scalar`` used
+to undo that with a bare ``raw[1:-1]``: it stripped the quotes but never
+decoded the escapes. So ``"Pr\\u00fcfung"`` was read back as the literal text
+``Pr\\u00fcfung``, and the next save escaped *that* backslash again.
+
+The damage compounds — each save doubles the backslash run — so a German or
+Japanese skill description degrades into backslash noise after a handful of
+edits, and the same happens to a plain-ASCII description that merely contains
+a quote character. The escapes are also shown verbatim in the skills list and
+the ``/skills`` catalog.
+
+The fix is symmetric: emit with ``ensure_ascii=False`` (SKILL.md is UTF-8 at
+both ends) and parse double-quoted scalars with ``json.loads``.
+"""
+
+import json
+
+import pytest
+
+from services.memory.skill_format import (
+    Skill,
+    _emit_scalar,
+    _parse_scalar,
+    parse_frontmatter,
+)
+from services.memory.skills import SkillsManager
+
+# Umlauts plus a comma — the comma is what forces the quoted form, which is the
+# only path that was corrupted. Reported verbatim in issue #5210.
+GERMAN = "Einstiegs- und Pr\u00fcfungslinie f\u00fcr AGB, Datenschutz"
+JAPANESE = "\u30b9\u30ad\u30eb: \u30c6\u30b9\u30c8\u7528\u306e\u8aac\u660e"
+QUOTED_ASCII = 'Use the "grep" tool, then summarise'
+
+
+def _cycle(skill: Skill, times: int = 1) -> Skill:
+    """Save to markdown and read it straight back, `times` times over."""
+    for _ in range(times):
+        skill = Skill.from_markdown(skill.to_markdown())
+    return skill
+
+
+# ---------------------------------------------------------------------------
+# The reported corruption
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    "description",
+    [GERMAN, JAPANESE, QUOTED_ASCII],
+    ids=["german", "japanese", "quoted-ascii"],
+)
+def test_description_survives_repeated_saves(description):
+    """Five load/save cycles must leave the text byte-identical.
+
+    One cycle is enough to corrupt it; five is where the doubling became
+    obvious in the field.
+    """
+    result = _cycle(Skill(name="demo", description=description), times=5)
+    assert result.description == description
+
+
+def test_corruption_does_not_compound_across_saves():
+    """Pin the *growth*, not just the mismatch.
+
+    The original defect was not a one-off mangling — the escaped form was
+    re-escaped on every save, so the value grew without bound. A regression
+    that reintroduced single-level mangling would still be caught by the test
+    above; this one catches the runaway specifically.
+    """
+    skill = Skill(name="demo", description=GERMAN)
+    lengths = []
+    for _ in range(5):
+        skill = _cycle(skill)
+        lengths.append(len(skill.description))
+    assert len(set(lengths)) == 1, f"description length drifted across saves: {lengths}"
+
+
+# ---------------------------------------------------------------------------
+# What actually lands on disk
+# ---------------------------------------------------------------------------
+
+
+def test_non_ascii_is_written_as_utf8_not_ascii_escapes():
+    """SKILL.md is opened as UTF-8 at both ends, so \\uXXXX buys nothing and
+    only makes the file unreadable to a human editing it."""
+    markdown = Skill(name="demo", description=GERMAN).to_markdown()
+    line = next(l for l in markdown.splitlines() if l.startswith("description:"))
+    assert "Pr\u00fcfungslinie" in line
+    assert "\\u00fc" not in line
+
+
+def test_quoted_scalar_is_valid_json():
+    """The emitted form is what the parser now feeds to json.loads, so the two
+    halves cannot drift apart without this failing."""
+    emitted = _emit_scalar(QUOTED_ASCII)
+    assert json.loads(emitted) == QUOTED_ASCII
+
+
+# ---------------------------------------------------------------------------
+# Existing files
+# ---------------------------------------------------------------------------
+
+
+def test_legacy_ascii_escaped_file_is_read_correctly():
+    """Files already written by the old emitter hold real JSON escapes, so the
+    new parser recovers the intended text instead of the escape source."""
+    markdown = '---\nname: demo\ndescription: "Pr\\u00fcfung, x"\n---\n\n'
+    assert Skill.from_markdown(markdown).description == "Pr\u00fcfung, x"
+
+
+def test_already_corrupted_file_heals_one_level_per_load():
+    """A file that took one round of damage carries a doubled backslash. That
+    is still valid JSON, so reading it yields the single-backslash form and the
+    value stops degrading."""
+    markdown = '---\nname: demo\ndescription: "Pr\\\\u00fcfung, x"\n---\n\n'
+    once = Skill.from_markdown(markdown)
+    assert once.description == "Pr\\u00fcfung, x"
+    # And it is now stable rather than growing on every subsequent save.
+    assert _cycle(once, times=3).description == "Pr\\u00fcfung, x"
+
+
+def test_non_json_escape_falls_back_to_literal_reading():
+    """A hand-written frontmatter value can hold escapes JSON rejects (a bare
+    Windows path is the common one). Those must keep their previous literal
+    reading rather than raising."""
+    assert _parse_scalar('"C:\\Users\\demo"') == "C:\\Users\\demo"
+
+
+# ---------------------------------------------------------------------------
+# Unchanged behaviour
+# ---------------------------------------------------------------------------
+
+
+def test_plain_scalars_are_still_emitted_bare():
+    """Only values needing quotes get them — the common case must not suddenly
+    start quoting, which would churn every SKILL.md on disk."""
+    assert _emit_scalar("open-pr-from-branch") == "open-pr-from-branch"
+    assert _emit_scalar("1.0.0") == "1.0.0"
+    assert _emit_scalar(True) == "true"
+    assert _emit_scalar(None) == "null"
+    assert _emit_scalar(0.8) == "0.8"
+
+
+def test_single_quoted_scalar_keeps_literal_reading():
+    """Only double-quoted scalars are JSON. Single-quoted ones are read the way
+    they always were."""
+    assert _parse_scalar("'plain, text'") == "plain, text"
+
+
+def test_lists_round_trip_with_non_ascii_entries():
+    skill = Skill(name="demo", tags=["b\u00fcro", "recht, steuern"])
+    assert _cycle(skill, times=3).tags == ["b\u00fcro", "recht, steuern"]
+
+
+# ---------------------------------------------------------------------------
+# Line-break characters
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    "sep",
+    ["\u2028", "\u2029", "\x85", "\r", "\v", "\f", "\x1c"],
+    ids=["ls", "ps", "nel", "cr", "vt", "ff", "fs"],
+)
+def test_line_break_characters_do_not_split_the_frontmatter(sep):
+    """parse_frontmatter() reads one scalar per line via str.splitlines(),
+    which breaks on far more than \\n. Any of these landing unescaped in the
+    file would silently truncate the value and shift the remainder into a
+    bogus key.
+
+    json.dumps covers the C0 ones, but with ensure_ascii=False it passes NEL,
+    LINE SEPARATOR and PARAGRAPH SEPARATOR through as themselves — so those
+    three are re-escaped explicitly.
+    """
+    description = f"before{sep}after, x"
+    markdown = Skill(name="demo", description=description).to_markdown()
+
+    frontmatter_text = markdown.split("---")[1]
+    assert len(frontmatter_text.strip().splitlines()) == len(
+        [l for l in frontmatter_text.strip().split("\n") if l.strip()]
+    ), "a scalar leaked a line break into the frontmatter"
+
+    fm, _body = parse_frontmatter(markdown)
+    assert fm["description"] == description
+
+
+# ---------------------------------------------------------------------------
+# End to end, through real files
+# ---------------------------------------------------------------------------
+
+
+def test_description_survives_real_save_load_cycles_on_disk(tmp_path):
+    """The unit tests above go straight through to_markdown/from_markdown.
+    This drives the same path the app does — SkillsManager writing UTF-8 files
+    with atomic_write_text and reading them back — because the encoding used at
+    either end is part of the fix.
+    """
+    manager = SkillsManager(str(tmp_path))
+    manager.add_skill(name="agb-pruefung", description=GERMAN, category="general")
+
+    for _ in range(4):
+        assert manager.update_skill("agb-pruefung", {"status": "published"})
+
+    stored = [s for s in manager.load_all() if s["name"] == "agb-pruefung"]
+    assert len(stored) == 1
+    assert stored[0]["description"] == GERMAN
+
+    on_disk = (tmp_path / "skills" / "general" / "agb-pruefung" / "SKILL.md").read_text(
+        encoding="utf-8"
+    )
+    assert GERMAN in on_disk

tests/test_spinner_stops_when_never_attached_js.py

@@ -0,0 +1,319 @@
+"""Pin the self-termination contract of the canvas spinners in static/js/spinner.js.
+
+Background: the whirlpool spinner drives itself with requestAnimationFrame and
+decides whether to keep going by looking at `element.isConnected`. It used to
+re-arm forever whenever the element had *never* been connected, on the theory
+that start() runs before the caller appends the element. Callers that start a
+spinner and then take an early return - an aborted request, a panel that
+resolved from cache before the loading row was inserted - therefore left a rAF
+loop redrawing an 84-segment spiral into a detached canvas until the tab closed.
+Measured on an idle app: ~110 whirlpool frames per second with zero canvases in
+the document.
+
+These tests lock in all four exits (never attached, attached-then-removed,
+stop(), tab hidden) and, just as importantly, the one case that must NOT stop:
+a spinner that is actually on screen.
+
+Driven through `node --input-type=module` so the real module runs, same idiom as
+test_esc_menu_stack_js.py. The module source is inlined rather than imported by
+path because the repo has no `"type": "module"` in package.json; spinner.js has
+no imports of its own, so inlining is exact. A fake clock and a manual frame
+pump replace performance.now()/requestAnimationFrame, so nothing here depends on
+wall-clock time or real frame timing.
+"""
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+_REPO = Path(__file__).resolve().parent.parent
+_MODULE = _REPO / "static" / "js" / "spinner.js"
+_HAS_NODE = shutil.which("node") is not None
+_SRC = _MODULE.read_text(encoding="utf-8") if _MODULE.exists() else ""
+
+# Browser stand-ins, installed before the module body runs. `clock` is advanced
+# only by pump(), so every timing decision in the module is deterministic.
+_STUBS = r"""
+let clock = 0;
+Object.defineProperty(globalThis, 'performance', {
+  value: { now: () => clock }, configurable: true, writable: true,
+});
+
+const pending = new Map();
+let nextFrameId = 1;
+let framesRun = 0;
+globalThis.requestAnimationFrame = (cb) => {
+  const id = nextFrameId++;
+  pending.set(id, cb);
+  return id;
+};
+globalThis.cancelAnimationFrame = (id) => { pending.delete(id); };
+
+/** Advance the clock `steps` frames of `msPerFrame` and run whatever is queued. */
+function pump(steps, msPerFrame = 16) {
+  for (let i = 0; i < steps; i++) {
+    clock += msPerFrame;
+    const due = [...pending.values()];
+    pending.clear();
+    for (const cb of due) { framesRun++; cb(); }
+  }
+}
+function framesPending() { return pending.size; }
+function framesSince(mark) { return framesRun - mark; }
+function frameMark() { return framesRun; }
+
+function makeCtx() {
+  const noop = () => {};
+  return {
+    clearRect: noop, beginPath: noop, arc: noop, moveTo: noop, lineTo: noop,
+    stroke: noop, fill: noop, save: noop, restore: noop,
+    strokeStyle: '', fillStyle: '', lineWidth: 0, globalAlpha: 1,
+    lineCap: '', lineJoin: '',
+  };
+}
+
+function makeElement(tag) {
+  const el = {
+    tagName: tag, className: '', textContent: '', innerHTML: '',
+    width: 0, height: 0, isConnected: false, parentNode: null,
+    style: { cssText: '' },
+    children: [],
+    classList: { add: () => {}, remove: () => {}, contains: () => false },
+    getContext: () => makeCtx(),
+    appendChild(child) {
+      child.parentNode = this;
+      this.children.push(child);
+      return child;
+    },
+    removeChild(child) {
+      this.children = this.children.filter((c) => c !== child);
+      child.parentNode = null;
+      return child;
+    },
+  };
+  return el;
+}
+
+const docListeners = [];
+globalThis.document = {
+  hidden: false,
+  documentElement: makeElement('html'),
+  createElement: makeElement,
+  createTextNode: (t) => ({ textContent: t }),
+  addEventListener: (type, fn) => { docListeners.push([type, fn]); },
+  removeEventListener: (type, fn) => {
+    const i = docListeners.findIndex(([t, f]) => t === type && f === fn);
+    if (i >= 0) docListeners.splice(i, 1);
+  },
+};
+globalThis.getComputedStyle = () => ({ getPropertyValue: () => '' });
+
+function visibilityListeners() {
+  return docListeners.filter(([t]) => t === 'visibilitychange').length;
+}
+function fireVisibility(hidden) {
+  document.hidden = hidden;
+  for (const [t, fn] of [...docListeners]) if (t === 'visibilitychange') fn();
+}
+
+/** A started whirlpool spinner whose element is not in the document. */
+function startedWhirlpool() {
+  const sp = new Spinner('', 'clean', 'whirlpool');
+  sp.createElement();
+  sp.start();
+  return sp;
+}
+"""
+
+
+def _run(body: str) -> dict:
+    """Run `body` with the real spinner module and the browser stubs in scope."""
+    js = _STUBS + "\n" + _SRC + "\n" + body
+    proc = subprocess.run(
+        ["node", "--input-type=module"],
+        input=js, capture_output=True, text=True, encoding="utf-8",
+        cwd=str(_REPO), timeout=30,
+    )
+    assert proc.returncode == 0, proc.stderr
+    return json.loads(proc.stdout.strip())
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_never_attached_whirlpool_stops_itself():
+    # The leak: element created, spinner started, element never inserted. Past
+    # the grace window it must give up rather than re-arm forever.
+    body = """
+    const sp = startedWhirlpool();
+    pump(30);                       // 480 ms - inside the grace window
+    const early = { running: sp.isRunning, pending: framesPending() };
+    pump(120);                      // ~2.4 s total - past the grace window
+    const mark = frameMark();
+    pump(60);                       // nothing should be left to run
+    console.log(JSON.stringify({
+      early,
+      running: sp.isRunning,
+      rafId: sp.rafId,
+      pending: framesPending(),
+      framesAfterStop: framesSince(mark),
+    }));
+    """
+    out = _run(body)
+    assert out["early"] == {"running": True, "pending": 1}, "gave up during the grace window"
+    assert out["running"] is False
+    assert out["rafId"] is None
+    assert out["pending"] == 0
+    assert out["framesAfterStop"] == 0, "loop kept drawing after it gave up"
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_attached_whirlpool_keeps_running_past_the_grace_window():
+    # The converse guard: the fix must not kill spinners that are on screen.
+    body = """
+    const sp = new Spinner('', 'clean', 'whirlpool');
+    sp.createElement();
+    sp.element.isConnected = true;
+    sp.start();
+    pump(400);                      // ~6.4 s, far past the grace window
+    const mark = frameMark();
+    pump(10);
+    console.log(JSON.stringify({
+      running: sp.isRunning,
+      pending: framesPending(),
+      framesDrawn: framesSince(mark),
+    }));
+    """
+    out = _run(body)
+    assert out["running"] is True
+    assert out["pending"] == 1
+    assert out["framesDrawn"] == 10, "a visible spinner stopped animating"
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_attached_then_removed_whirlpool_stops():
+    # The pre-existing exit - a loading row replaced by results - still works.
+    body = """
+    const sp = new Spinner('', 'clean', 'whirlpool');
+    sp.createElement();
+    sp.element.isConnected = true;
+    sp.start();
+    pump(200);
+    const whileAttached = sp.isRunning;
+    sp.element.isConnected = false; // results arrived, row swapped out
+    pump(3);
+    const mark = frameMark();
+    pump(20);
+    console.log(JSON.stringify({
+      whileAttached,
+      running: sp.isRunning,
+      pending: framesPending(),
+      framesAfterRemoval: framesSince(mark),
+    }));
+    """
+    out = _run(body)
+    assert out["whileAttached"] is True
+    assert out["running"] is False
+    assert out["pending"] == 0
+    assert out["framesAfterRemoval"] == 0
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_loading_row_helper_stops_when_the_row_is_never_inserted():
+    # createLoadingRow() starts the spinner for the caller and hands back a
+    # detached row, so a caller that early-returns is the real leak shape.
+    body = """
+    const row = createLoadingRow('Loading...', 16);
+    pump(200);
+    const mark = frameMark();
+    pump(40);
+    console.log(JSON.stringify({
+      pending: framesPending(),
+      framesAfterStop: framesSince(mark),
+      rowHasChildren: row.children.length > 0,
+    }));
+    """
+    out = _run(body)
+    assert out["rowHasChildren"] is True, "harness built the wrong row"
+    assert out["pending"] == 0
+    assert out["framesAfterStop"] == 0
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_stop_cancels_the_pending_frame_and_releases_the_listener():
+    # stop() must be authoritative: no queued frame survives it, and it leaves
+    # no visibilitychange listener behind on a dead spinner.
+    body = """
+    const before = visibilityListeners();
+    const sp = new Spinner('', 'clean', 'whirlpool');
+    sp.createElement();
+    sp.element.isConnected = true;
+    sp.start();
+    const armed = visibilityListeners();
+    sp.stop();
+    const mark = frameMark();
+    pump(20);
+    console.log(JSON.stringify({
+      before, armed, after: visibilityListeners(),
+      running: sp.isRunning,
+      rafId: sp.rafId,
+      pending: framesPending(),
+      framesAfterStop: framesSince(mark),
+    }));
+    """
+    out = _run(body)
+    assert (out["before"], out["armed"], out["after"]) == (0, 1, 0)
+    assert out["running"] is False
+    assert out["rafId"] is None
+    assert out["pending"] == 0
+    assert out["framesAfterStop"] == 0
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_hidden_tab_pauses_frames_and_showing_resumes_them():
+    body = """
+    const sp = new Spinner('', 'clean', 'whirlpool');
+    sp.createElement();
+    sp.element.isConnected = true;
+    sp.start();
+    pump(5);
+    fireVisibility(true);
+    const hiddenMark = frameMark();
+    pump(30);
+    const whileHidden = { drawn: framesSince(hiddenMark), pending: framesPending() };
+    fireVisibility(false);
+    const shownMark = frameMark();
+    pump(10);
+    console.log(JSON.stringify({
+      whileHidden,
+      running: sp.isRunning,
+      drawnAfterShow: framesSince(shownMark),
+    }));
+    """
+    out = _run(body)
+    assert out["whileHidden"] == {"drawn": 0, "pending": 0}, "kept drawing in a hidden tab"
+    assert out["running"] is True
+    assert out["drawnAfterShow"] == 10, "did not resume when the tab came back"
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_restarted_spinner_gets_a_fresh_grace_window():
+    # The grace deadline is per-run. A spinner reused after stop() must not
+    # inherit the previous run's timestamp and die on its first frame.
+    body = """
+    const sp = startedWhirlpool();
+    pump(200);                      // times out, never attached
+    const stopped = sp.isRunning;
+    sp.element.isConnected = true;  // now inserted for real
+    sp.start();
+    pump(30);
+    console.log(JSON.stringify({
+      stopped,
+      running: sp.isRunning,
+      pending: framesPending(),
+    }));
+    """
+    out = _run(body)
+    assert out["stopped"] is False
+    assert out["running"] is True
+    assert out["pending"] == 1

tests/test_teacher_audit_owner_scope.py

@@ -21,10 +21,17 @@ def test_call_teacher_scopes_model_resolution_to_owner(monkeypatch):
         return ("http://endpoint.local/v1", "teacher-model", {})
 
     async def fake_llm_call_async(url, model, messages, **kwargs):
+        seen["messages"] = messages
         return "teacher reply"
 
+    from src.agent_tools import model_interaction_tools
+
     monkeypatch.setattr("src.ai_interaction._resolve_model", fake_resolve_model)
-    monkeypatch.setattr("src.ai_interaction._TEACHER_SYSTEM_PROMPT", "sys", raising=False)
+    monkeypatch.setattr(
+        model_interaction_tools,
+        "_TEACHER_SYSTEM_PROMPT",
+        "sys",
+    )
     monkeypatch.setattr("src.llm_core.llm_call_async", fake_llm_call_async)
 
     result = asyncio.run(
@@ -34,6 +41,7 @@ def test_call_teacher_scopes_model_resolution_to_owner(monkeypatch):
     assert result == "teacher reply"
     assert seen["owner"] == "alice"
     assert seen["spec"] == "teacher-model"
+    assert seen["messages"][0] == {"role": "system", "content": "sys"}
 
 
 def test_audit_teacher_resolution_scoped_to_owner(monkeypatch):

tests/test_tool_parsing_bare_end_marker.py

@@ -34,6 +34,7 @@ KEPT = [
     ("append end", "append end"),
     ("END", "END"),
     ("\nEnd\n", "End"),
+    ("x assistant y", "x assistant y"),          # mid-sentence must survive (#5971)
 ]
 
 # Real markers — at least one pipe, plus the role word — with the exact output
@@ -44,7 +45,9 @@ STRIPPED = [
     ("a /|end| b", "a  b"),
     ("a |end b", "a  b"),
     ("a end| b", "a  b"),
-    ("x assistant y", "x  y"),
+    ("Before\nassistant\nAfter", "Before \nAfter"),   # bare-marker on its own line still stripped
+    ("Before\n  assistant\t \nAfter", "Before \nAfter"),     # whitespace-padded marker still stripped
+    ("Before\n\tassistan  \nAfter", "Before \nAfter"),       # truncated marker variant still stripped
 ]
 
 

tests/test_tool_parsing_hermes_json.py

@@ -0,0 +1,94 @@
+"""Qwen/Hermes text-mode tool calls: bare JSON inside <tool_call> wrappers.
+
+Issue #5187: <tool_call>{"name": "bash", "arguments": {...}}</tool_call>
+parsed to zero blocks because wrapper bodies were only fed to the XML
+iterators. The JSON body form now parses through the same canonical
+function_call_to_tool_block converter as the XML paths, and JSON-looking
+bodies fail closed instead of falling through to XML scanning (tracker #5333):
+XML-like text inside JSON argument values must stay data, and a non-object
+"arguments" value is rejected rather than coerced.
+"""
+import src.agent_tools  # noqa: F401  (break agent_tools<->tool_parsing import cycle)
+from src.tool_parsing import parse_tool_blocks, strip_tool_blocks
+
+# Verbatim payload from issue #5187.
+ISSUE_PAYLOAD = '<tool_call>\n{"name": "bash", "arguments": {"command": "mkdir -p agent-test"}}\n</tool_call>'
+
+
+def test_issue_5187_payload_parses():
+    blocks = parse_tool_blocks(ISSUE_PAYLOAD)
+    assert len(blocks) == 1
+    assert blocks[0].tool_type == "bash"
+    assert blocks[0].content == "mkdir -p agent-test"
+
+
+def test_multiple_sequential_wrappers():
+    text = (
+        '<tool_call>\n{"name": "bash", "arguments": {"command": "ls"}}\n</tool_call>\n'
+        'Now the second step:\n'
+        '<tool_call>\n{"name": "bash", "arguments": {"command": "pwd"}}\n</tool_call>'
+    )
+    blocks = parse_tool_blocks(text)
+    assert [(b.tool_type, b.content) for b in blocks] == [("bash", "ls"), ("bash", "pwd")]
+
+
+def test_unclosed_wrapper_still_parses():
+    text = '<tool_call>\n{"name": "bash", "arguments": {"command": "ls -la"}}'
+    blocks = parse_tool_blocks(text)
+    assert len(blocks) == 1
+    assert blocks[0].tool_type == "bash"
+    assert blocks[0].content == "ls -la"
+
+
+def test_xml_inside_json_arguments_stays_data():
+    # P1: a valid JSON body whose argument values contain XML-like tool markup
+    # must parse as the JSON-named tool; the embedded markup is content.
+    text = (
+        '<tool_call>{"name": "write_file", "arguments": '
+        '{"path": "notes.txt", "content": "<bash>echo unsafe</bash>"}}</tool_call>'
+    )
+    blocks = parse_tool_blocks(text)
+    assert len(blocks) == 1
+    assert blocks[0].tool_type == "write_file"
+    assert "<bash>echo unsafe</bash>" in blocks[0].content
+    assert all(b.tool_type != "bash" for b in blocks)
+
+
+def test_malformed_json_body_never_falls_through_to_xml():
+    # P1 fail-closed: a JSON-looking body that doesn't decode must not be
+    # rescanned as XML, even when it contains well-formed tool markup.
+    text = (
+        '<tool_call>{"name": "write_file", "arguments": {broken json '
+        '<invoke name="bash"><parameter name="command">echo unsafe</parameter></invoke>'
+        '</tool_call>'
+    )
+    assert parse_tool_blocks(text) == []
+
+
+def test_non_dict_arguments_rejected():
+    # P2: "arguments" must be an object; scalars/arrays are rejected, not coerced.
+    for args in ('["ls"]', '"ls"', '1', 'null'):
+        text = '<tool_call>{"name": "bash", "arguments": %s}</tool_call>' % args
+        assert parse_tool_blocks(text) == [], f"arguments={args} should be rejected"
+
+
+def test_strip_tool_blocks_removes_json_wrapper_spans():
+    text = "Before.\n" + ISSUE_PAYLOAD + "\nAfter."
+    cleaned = strip_tool_blocks(text)
+    assert "tool_call" not in cleaned
+    assert "mkdir -p agent-test" not in cleaned
+    assert "Before." in cleaned
+    assert "After." in cleaned
+
+
+def test_xml_body_wrapper_regression():
+    # The pre-existing XML wrapper form must keep parsing exactly as before.
+    text = (
+        '<tool_call><invoke name="bash">'
+        '<parameter name="command">echo hi</parameter>'
+        '</invoke></tool_call>'
+    )
+    blocks = parse_tool_blocks(text)
+    assert len(blocks) == 1
+    assert blocks[0].tool_type == "bash"
+    assert blocks[0].content == "echo hi"

tests/test_ui_visibility_js.py

@@ -0,0 +1,139 @@
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+
+ROOT = Path(__file__).resolve().parents[1]
+pytestmark = pytest.mark.skipif(not shutil.which("node"), reason="node binary not on PATH")
+
+
+def _node_eval(source):
+    result = subprocess.run(
+        ["node", "--input-type=module", "-e", source],
+        cwd=ROOT,
+        check=True,
+        capture_output=True,
+        text=True,
+    )
+    return json.loads(result.stdout)
+
+
+def _resolve(state):
+    """Run resolveVisibility(state) in node; return {selector: visible}."""
+    return _node_eval(
+        f"""
+        const {{ resolveVisibility }} = await import('./static/js/ui_visibility.js');
+        console.log(JSON.stringify(resolveVisibility({json.dumps(state)})));
+        """
+    )
+
+
+def _map():
+    return _node_eval(
+        """
+        const { UI_VIS_MAP } = await import('./static/js/ui_visibility.js');
+        console.log(JSON.stringify(UI_VIS_MAP));
+        """
+    )
+
+
+# Selectors (kept in one place so the tests read as plain assertions).
+EMAIL = "#email-section, #rail-email"
+TOOLS = "#tools-section"
+CAL = "#tool-calendar-btn, #rail-calendar"
+COMPARE = "#tool-compare-btn, #rail-compare"
+LIB = "#tool-library-btn, #rail-archive"
+RESEARCH = "#tool-research-btn, #rail-research"
+NEWCHAT = "#rail-new-session"
+RAG = "#overflow-rag-btn"
+
+# Full-sidebar tabs that have an icon-rail counterpart must pair it into their
+# UI_VIS_MAP selector; otherwise minimizing the sidebar re-shows a tab the user
+# turned off in the full view (#tool-library-btn's rail counterpart is #rail-archive).
+EXPECTED_RAIL_PAIRS = {
+    "email-section": "#rail-email",
+    "tool-calendar": "#rail-calendar",
+    "tool-compare": "#rail-compare",
+    "tool-cookbook": "#rail-cookbook",
+    "tool-research": "#rail-research",
+    "tool-gallery": "#rail-gallery",
+    "tool-library": "#rail-archive",
+    "tool-memory": "#rail-memory",
+    "tool-notes": "#rail-notes",
+    "tool-tasks": "#rail-tasks",
+    "tool-theme": "#rail-theme",
+}
+
+
+def test_every_customizable_tab_pairs_its_rail_button():
+    ui_vis_map = _map()
+    missing = {
+        key: rail
+        for key, rail in EXPECTED_RAIL_PAIRS.items()
+        if rail not in ui_vis_map.get(key, "")
+    }
+    assert not missing, (
+        "these tabs are missing their icon-rail counterpart in UI_VIS_MAP "
+        f"(minimizing the sidebar would re-show them): {missing}"
+    )
+
+
+def test_defaults_everything_visible_except_default_off():
+    m = _resolve({})
+    assert m[EMAIL] is True
+    assert m[TOOLS] is True
+    assert m[CAL] is True
+    assert m[NEWCHAT] is True
+    assert m[RAG] is False  # rag-toggle-btn is default-off
+
+
+def test_email_off_hides_email_and_its_rail_only():
+    m = _resolve({"email-section": False})
+    assert m[EMAIL] is False
+    assert m[CAL] is True
+    assert m[TOOLS] is True
+
+
+def test_tool_off_hides_its_rail_launcher():
+    m = _resolve({"tool-calendar": False})
+    assert m[CAL] is False
+    assert m[COMPARE] is True
+
+
+def test_library_off_hides_archive_rail():
+    # tool-library's rail counterpart is #rail-archive (mirrors _railToolMap).
+    m = _resolve({"tool-library": False})
+    assert m[LIB] is False
+
+
+def test_tools_off_hides_every_tool_rail_but_not_email():
+    m = _resolve({"tools-section": False})
+    assert m[TOOLS] is False
+    for sel in (CAL, COMPARE, LIB, RESEARCH):
+        assert m[sel] is False, sel
+    assert m[EMAIL] is True  # email is independent of the Tools section
+
+
+def test_tools_off_overrides_per_tool_on():
+    # A tool individually "on" must still hide when its parent Tools is off.
+    m = _resolve({"tools-section": False, "tool-calendar": True})
+    assert m[CAL] is False
+
+
+def test_tools_on_with_tool_off_hides_only_that_tool():
+    m = _resolve({"tools-section": True, "tool-research": False})
+    assert m[RESEARCH] is False
+    assert m[CAL] is True
+
+
+def test_rail_new_chat_off_hides_new_session():
+    m = _resolve({"rail-new-chat": False})
+    assert m[NEWCHAT] is False
+
+
+def test_explicit_false_takes_precedence_over_default_on():
+    m = _resolve({"rag-toggle-btn": True})
+    assert m[RAG] is True

tests/test_upload_handler_atomicity.py

@@ -15,6 +15,7 @@ These tests exercise:
 * Smoke tests: normal upload, duplicate detection, info lookup after
   a backup-recovery scenario.
 """
+import builtins
 import concurrent.futures
 import io
 import json
@@ -59,6 +60,16 @@ def _db_path(handler: UploadHandler) -> str:
     return os.path.join(handler.upload_dir, "uploads.json")
 
 
+def _truncate_without_newer_mtime(path: str) -> None:
+    """Model a filesystem where a torn write shares the cached timestamp."""
+    before = os.stat(path)
+    with open(path, "rb") as f:
+        full = f.read()
+    with open(path, "wb") as f:
+        f.write(full[: max(1, len(full) // 2)])
+    os.utime(path, ns=(before.st_atime_ns, before.st_mtime_ns))
+
+
 def _seed_entry(owner: str, file_hash: str, file_id: str) -> dict:
     return {
         "id": file_id,
@@ -246,10 +257,7 @@ def test_partial_write_recovery_via_bak(tmp_path):
         "Production _atomic_write_json must create a .bak sibling on subsequent writes."
     )
 
-    full = open(db_path, "rb").read()
+    _truncate_without_newer_mtime(db_path)
-    truncated_len = max(1, len(full) // 2)
-    with open(db_path, "wb") as f:
-        f.write(full[:truncated_len])
 
     recovered = handler._load_upload_index()
     missing = [k for k in original if k not in recovered]
@@ -259,6 +267,69 @@ def test_partial_write_recovery_via_bak(tmp_path):
     )
 
 
+def test_partial_write_recovery_via_bak_after_restart(tmp_path):
+    """A fresh handler must recover the previous snapshot from ``.bak``."""
+    handler = _make_handler(tmp_path)
+    db_path = _db_path(handler)
+    original = {
+        f"owner:hash_{i}": _seed_entry("owner", f"hash_{i}", f"id_{i}")
+        for i in range(3)
+    }
+    handler._atomic_write_json(db_path, original)
+    handler._atomic_write_json(db_path, {"latest": True})
+    _truncate_without_newer_mtime(db_path)
+
+    restarted_handler = UploadHandler(
+        base_dir=handler.base_dir,
+        upload_dir=handler.upload_dir,
+    )
+
+    assert restarted_handler._load_upload_index() == original
+
+
+def test_unchanged_upload_index_uses_cache(tmp_path, monkeypatch):
+    """The stronger file signature must preserve the unchanged-index fast path."""
+    handler = _make_handler(tmp_path)
+    original = {"owner:hash": _seed_entry("owner", "hash", "id")}
+    handler._atomic_write_json(_db_path(handler), original)
+
+    def fail_if_parsed(_file):
+        raise AssertionError("unchanged upload index should be served from cache")
+
+    monkeypatch.setattr(json, "load", fail_if_parsed)
+
+    assert handler._load_upload_index() == original
+
+
+def test_upload_index_retries_when_replaced_during_read(tmp_path, monkeypatch):
+    """Do not cache old JSON under the signature of a newer atomic replace."""
+    handler = _make_handler(tmp_path)
+    db_path = _db_path(handler)
+    old_index = {"owner:old": _seed_entry("owner", "old", "old_id")}
+    new_index = {"owner:new": _seed_entry("owner", "new", "new_id")}
+    handler._atomic_write_json(db_path, old_index)
+    handler._index_cache = None
+    handler._index_signature = None
+
+    real_open = builtins.open
+    replaced = False
+
+    def racing_open(file, mode="r", *args, **kwargs):
+        nonlocal replaced
+        handle = real_open(file, mode, *args, **kwargs)
+        if os.fspath(file) == db_path and "r" in mode and not replaced:
+            replaced = True
+            replacement = db_path + ".replacement"
+            with real_open(replacement, "w", encoding="utf-8") as out:
+                json.dump(new_index, out)
+            os.replace(replacement, db_path)
+        return handle
+
+    monkeypatch.setattr(builtins, "open", racing_open)
+
+    assert handler._load_upload_index() == new_index
+
+
 # ---------------------------------------------------------------------------
 # Atomicity primitive audit on the production module.
 # ---------------------------------------------------------------------------
@@ -390,10 +461,8 @@ def test_smoke_info_lookup_after_bak_recovery(tmp_path):
     handler._atomic_write_json(db_path, {"sentinel": True})
     assert os.path.exists(db_path + ".bak")
 
-    # Truncate the live file.
+    # Truncate the live file without assuming the filesystem advances mtime.
-    full = open(db_path, "rb").read()
+    _truncate_without_newer_mtime(db_path)
-    with open(db_path, "wb") as f:
-        f.write(full[: max(1, len(full) // 2)])
 
     info = handler.get_upload_info(first["id"])
     assert info is not None, "Info lookup must succeed after .bak recovery."