fix(email): close IMAP socket when connect/login fails (#3174) (#3363)

* fix(email): close IMAP socket when connect/login fails (#3174)

_imap_connect opened a live socket via _open_imap_connection and then
called conn.login() with no try/finally, and _open_imap_connection called
conn.starttls() unguarded. When auth fails (e.g. an Office 365 app password
on an MFA-enabled tenant, #3174) or STARTTLS is rejected, the already-open
socket was orphaned. Every IMAP caller funnels through _imap_connect,
including the 30-minute _auto_summarize_poller, so a persistently
misconfigured account leaked one descriptor per pass toward FD exhaustion.

The previously merged leak fixes (#1325/#1330/#1423/#1530) only guard the
post-connect body and monkeypatch _imap_connect to succeed, so this
connect-time path was uncovered. Wrap login() and starttls() so a failure
calls conn.shutdown() (low-level close; logout() can't run pre-auth) before
re-raising. Adds two regression tests that fail without the guard.

* fix(email): guard MCP IMAP+SMTP connect-time leaks too (#3174)

Folds in the sibling connect-time leaks vdmkenny flagged on #3363, so the
whole connect-then-step leak class is closed in one place:

- mcp_servers/email_server.py::_imap_connect — guard starttls() and login();
  close pre-auth with conn.shutdown() before re-raising.
- mcp_servers/email_server.py::_smtp_connect — guard starttls() and login();
  SMTP has no shutdown(), so close with conn.close() (socket close, no QUIT).

Routes SMTP (_send_smtp_message) is already safe via 'with smtplib.SMTP(...)'.
Adds four regression tests (one per guard), verified to fail without the fix.

.github/workflows/ci.yml

@@ -20,6 +20,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5  # v4
+        with:
+          persist-credentials: false
       - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065  # v5
         with:
           python-version: "3.11"
@@ -31,6 +33,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5  # v4
+        with:
+          persist-credentials: false
       - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020  # v4
         with:
           node-version: "20"
@@ -53,6 +57,7 @@ jobs:
       - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5  # v4
         with:
           fetch-depth: 0
+          persist-credentials: false
 
       # Detect whether this PR only touches documentation files.
       # If so, skip the expensive pytest run while still reporting a passing check.

.github/workflows/docker-publish.yml

@@ -22,7 +22,6 @@ concurrency:
 
 permissions:
   contents: read
-  packages: write
 
 env:
   REGISTRY: ghcr.io
@@ -32,6 +31,9 @@ jobs:
   build:
     name: build (${{ matrix.arch }})
     runs-on: ${{ matrix.runner }}
+    permissions:
+      contents: read
+      packages: write
     strategy:
       fail-fast: false
       matrix:
@@ -44,6 +46,8 @@ jobs:
             runner: ubuntu-24.04-arm
     steps:
       - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10  # v6.0.3
+        with:
+          persist-credentials: false
       - name: Set up Buildx
         uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5  # v4.1.0
       - name: Log in to GHCR
@@ -78,8 +82,13 @@ jobs:
     name: merge manifest + tag
     runs-on: ubuntu-latest
     needs: build
+    permissions:
+      contents: read
+      packages: write
     steps:
       - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10  # v6.0.3
+        with:
+          persist-credentials: false
       - name: Read APP_VERSION + short sha
         id: ver
         run: |
@@ -116,6 +125,8 @@ jobs:
         run: |
           tags=$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")
           digests=$(printf "${REGISTRY}/${IMAGE_NAME}@sha256:%s " *)
+          # word-splitting is intended: $tags and $digests each expand to multiple args
+          # shellcheck disable=SC2086
           docker buildx imagetools create $tags $digests
         env:
           REGISTRY: ${{ env.REGISTRY }}

.github/workflows/issue-description-check.yml

@@ -14,10 +14,11 @@ jobs:
     # Skip bots (Dependabot, release-drafter, etc.)
     if: ${{ github.event.issue.user.type != 'Bot' }}
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10  # v6.0.3
         with:
           sparse-checkout: .github/scripts
+          persist-credentials: false
 
-      - uses: actions/github-script@v7
+      - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3  # v9.0.0
         with:
           script: return require('./.github/scripts/check-issue-description.js')({github, context, core})

.github/workflows/pr-description-check.yml

@@ -1,26 +1,25 @@
 name: ci / PR checks
 
 on:
-  pull_request_target:
+  # pull_request_target runs in the base-repo context (has secrets) so the check
+  # works on fork PRs. Safe here: the checkout pins to the base branch (no fork
+  # code runs) and the scripts only read context.payload and call the GitHub API.
+  pull_request_target:  # zizmor: ignore[dangerous-triggers]
     types: [opened, edited, synchronize, reopened, ready_for_review]
 
-# pull_request_target runs in the base-repo context (has secrets).
-# The checkout below pins to the base branch so no fork code is executed.
-# The script only reads context.payload and calls the GitHub API.
-# Least privilege: contents:read for the base-ref checkout, pull-requests:write
-# to add/remove labels and post comments on PRs, and issues:write for the same
-# on real issues. NOTE: modifying a *pull request's* labels/comments needs the
-# `pull-requests` scope even though the REST path is under `/issues/{n}/...`;
-# `issues:write` alone returns 403 on PRs.
-permissions:
-  contents: read
-  pull-requests: write
-  issues: write
+# Default-deny at the workflow level; each job opts into only the scopes it needs.
+# Note: modifying a PR's labels/comments needs pull-requests:write even though the
+# REST path is under /issues/{n}/...; issues:write alone returns 403 on PRs.
+permissions: {}
 
 jobs:
   check-description:
     name: Check PR description
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: write
+      issues: write
     # Skip bots: they open PRs programmatically and have their own process.
     if: github.event.pull_request.user.type != 'Bot'
     steps:
@@ -28,6 +27,7 @@ jobs:
         with:
           ref: ${{ github.base_ref }}
           sparse-checkout: .github/scripts
+          persist-credentials: false
 
       - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3  # v9.0.0
         with:
@@ -36,6 +36,7 @@ jobs:
   check-title:
     name: Check PR title (Conventional Commits)
     runs-on: ubuntu-latest
+    permissions: {}
     # Skip bots: they open PRs programmatically and have their own process.
     if: github.event.pull_request.user.type != 'Bot'
     steps:
@@ -59,6 +60,9 @@ jobs:
   check-mergeable:
     name: Flag unmergeable PRs
     runs-on: ubuntu-latest
+    permissions:
+      pull-requests: write
+      issues: write
     # Skip bots: they open PRs programmatically and have their own process.
     if: github.event.pull_request.user.type != 'Bot'
     steps:

README.md

@@ -333,6 +333,12 @@ To expose Odysseus on a local network or Tailscale with HTTPS:
 | `PyMuPDF` | PDF page rendering in the side viewer panel and form-filling. (Note: AGPL-3.0) |
 | `markitdown` | Office/EPUB document text extraction (converts .docx/.xlsx/.pptx/.xls/.epub to Markdown). |
 
+### Outlook / Office 365 email
+Odysseus email accounts currently use IMAP/SMTP username-password auth. Outlook
+and Microsoft 365 generally require OAuth instead, so normal Microsoft mailbox
+passwords will fail. See [docs/email-outlook.md](docs/email-outlook.md) for the
+current limitation and the planned integration direction.
+
 ## Security Notes
 Odysseus is a self-hosted workspace with powerful local tools: shell access, file uploads, model downloads, web research, email/calendar integrations, and API tokens. Treat it like an admin console.
 

docs/email-outlook.md

@@ -0,0 +1,17 @@
+# Outlook / Office 365 email accounts
+
+Odysseus email accounts currently use IMAP and SMTP with username/password
+authentication. That works for providers that still allow app passwords or
+mailbox passwords for IMAP/SMTP.
+
+Microsoft disables basic authentication for Outlook and Microsoft 365 in most
+modern accounts and tenants. If you try to add an Outlook account with a normal
+password, Microsoft may return errors such as:
+
+- `IMAP: AUTHENTICATE failed`
+- `SMTP: 535 5.7.139 Authentication unsuccessful, basic authentication is disabled`
+
+This is expected. Odysseus does not support Microsoft OAuth or Graph Mail yet,
+so Outlook / Office 365 accounts cannot currently be added through the password
+form. Use another email provider with app-password support, or track the future
+Microsoft Graph OAuth integration.

mcp_servers/email_server.py

@@ -245,10 +245,27 @@ def _imap_connect(account: str | None = None):
             timeout=EMAIL_SOCKET_TIMEOUT,
         )
         if cfg["imap_starttls"]:
-            conn.starttls()
+            try:
+                conn.starttls()
+            except Exception:
+                # Don't leak the open plain socket on a rejected STARTTLS. (#3174)
+                try:
+                    conn.shutdown()
+                except Exception:
+                    pass
+                raise
     if getattr(conn, "sock", None):
         conn.sock.settimeout(EMAIL_SOCKET_TIMEOUT)
-    conn.login(cfg["imap_user"], cfg["imap_password"])
+    try:
+        conn.login(cfg["imap_user"], cfg["imap_password"])
+    except Exception:
+        # A failed login otherwise orphans the connected socket; close it
+        # before propagating (shutdown() is the pre-auth low-level close). (#3174)
+        try:
+            conn.shutdown()
+        except Exception:
+            pass
+        raise
     return conn
 
 
@@ -778,7 +795,16 @@ def _smtp_connect(account=None, cfg=None):
             port,
             timeout=EMAIL_SOCKET_TIMEOUT,
         )
-        conn.starttls()
+        try:
+            conn.starttls()
+        except Exception:
+            # Don't leak the open plain socket on a rejected STARTTLS. SMTP has
+            # no shutdown(); close() is the low-level socket close (no QUIT). (#3174)
+            try:
+                conn.close()
+            except Exception:
+                pass
+            raise
     elif security == "ssl":
         conn = smtplib.SMTP_SSL(
             cfg["smtp_host"],
@@ -792,7 +818,16 @@ def _smtp_connect(account=None, cfg=None):
             timeout=EMAIL_SOCKET_TIMEOUT,
         )
     if cfg["smtp_user"] and cfg["smtp_password"]:
-        conn.login(cfg["smtp_user"], cfg["smtp_password"])
+        try:
+            conn.login(cfg["smtp_user"], cfg["smtp_password"])
+        except Exception:
+            # A failed login otherwise orphans the connected socket; close it
+            # before propagating (SMTP has no shutdown(); close() = socket close). (#3174)
+            try:
+                conn.close()
+            except Exception:
+                pass
+            raise
     return conn
 
 

routes/api_token_routes.py

@@ -155,22 +155,30 @@ def setup_api_token_routes() -> APIRouter:
             payload = await request.json()
         except Exception:
             payload = {}
-        scope_list = _normalize_scopes(payload.get("scopes"))
-        scopes_value = ",".join(scope_list)
         with get_db_session() as db:
             token = db.query(ApiToken).filter(ApiToken.id == token_id).first()
             if not token:
                 raise HTTPException(404, "Token not found")
             if isinstance(payload.get("name"), str) and payload["name"].strip():
                 token.name = payload["name"].strip()[:MAX_NAME_LEN]
-            token.scopes = scopes_value
+            # Only touch scopes when the caller actually sent them. A partial
+            # update such as a rename ({"name": ...} with no "scopes" key) must
+            # not silently reset the token to the default scope — that dropped
+            # every previously granted scope.
+            if "scopes" in payload:
+                token.scopes = ",".join(_normalize_scopes(payload.get("scopes")))
             db.add(token)
+            current_scopes = [
+                s.strip()
+                for s in (getattr(token, "scopes", "") or DEFAULT_SCOPES).split(",")
+                if s.strip()
+            ]
             response = {
                 "id": token_id,
                 "name": getattr(token, "name", ""),
                 "owner": getattr(token, "owner", None),
                 "token_prefix": getattr(token, "token_prefix", ""),
-                "scopes": scope_list,
+                "scopes": current_scopes,
             }
         _invalidate_cache(request)
         return response

routes/email_helpers.py

@@ -71,6 +71,38 @@ def _send_smtp_message(cfg: dict, from_addr: str, recipients: list[str], message
         smtp.sendmail(from_addr, recipients, message)
 
 
+def _friendly_email_auth_error(protocol: str, host: str, error: object) -> str:
+    """Return a clearer setup error for known provider auth policies."""
+    raw = str(error or "")
+    lower = raw.lower()
+    host_lower = (host or "").lower()
+    microsoft_host = any(
+        marker in host_lower
+        for marker in (
+            "outlook.office365.com",
+            "smtp.office365.com",
+            "office365.com",
+            "outlook.com",
+            "hotmail.com",
+            "live.com",
+        )
+    )
+    microsoft_basic_auth_failure = (
+        "5.7.139" in lower
+        or "basic authentication is disabled" in lower
+        or ("authenticate failed" in lower and microsoft_host)
+        or ("authentication unsuccessful" in lower and microsoft_host)
+    )
+    if microsoft_basic_auth_failure:
+        return (
+            "Microsoft no longer accepts normal mailbox passwords for "
+            "Outlook/Office 365 IMAP/SMTP in most accounts. Odysseus "
+            "does not support Microsoft OAuth/Graph mail yet, so Outlook "
+            "accounts cannot be added with this password form."
+        )
+    return raw[:200]
+
+
 def _strip_think(text: str) -> str:
     """Email-flavored think strip — thin wrapper over the central helper.
 
@@ -706,7 +738,16 @@ def _open_imap_connection(host: str, port: int, *, starttls: bool, timeout: int
     port = int(port or 993)
     if starttls:
         conn = imaplib.IMAP4(host, port, timeout=timeout)
-        conn.starttls()
+        try:
+            conn.starttls()
+        except Exception:
+            # Don't leak the open plain socket if the STARTTLS upgrade is
+            # rejected; close it before propagating. (#3174)
+            try:
+                conn.shutdown()
+            except Exception:
+                pass
+            raise
     elif port == 993:
         conn = imaplib.IMAP4_SSL(host, port, timeout=timeout)
     else:
@@ -739,7 +780,18 @@ def _imap_connect(account_id: str | None = None, owner: str = ""):
         starttls=bool(cfg.get("imap_starttls")),
         timeout=_IMAP_TIMEOUT_SECONDS,
     )
-    conn.login(cfg["imap_user"], cfg["imap_password"])
+    try:
+        conn.login(cfg["imap_user"], cfg["imap_password"])
+    except Exception:
+        # A failed AUTHENTICATE (e.g. an Office 365 app password on an
+        # MFA-enabled tenant, #3174) otherwise orphans the already-connected
+        # socket; close it before propagating so a misconfigured account
+        # can't leak one descriptor per retry / background poller pass.
+        try:
+            conn.shutdown()
+        except Exception:
+            pass
+        raise
     return conn
 
 

routes/email_routes.py

@@ -48,6 +48,7 @@ from routes.email_helpers import (
     _extract_attachment_to_disk, _extract_html, _extract_text,
     _fetch_sender_thread_context, _pre_retrieve_context,
     _EMAIL_REPLY_SYS_PROMPT_BASE, _POOL_HOOKS,
+    _friendly_email_auth_error,
     SendEmailRequest, ExtractStyleRequest,
     ATTACHMENTS_DIR, COMPOSE_UPLOADS_DIR, SCHEDULED_DB,
     attachment_extract_dir, _email_cache_owner_clause,
@@ -3163,7 +3164,7 @@ def setup_email_routes():
                     try: conn.logout()
                     except Exception: pass
             except Exception as e:
-                imap_result = {"ok": False, "error": str(e)[:200]}
+                imap_result = {"ok": False, "error": _friendly_email_auth_error("IMAP", imap_host, e)}
 
         smtp_host = (body.get("smtp_host") or "").strip()
         if smtp_host:
@@ -3185,7 +3186,7 @@ def setup_email_routes():
                     try: smtp.quit()
                     except Exception: pass
             except Exception as e:
-                smtp_result = {"ok": False, "error": str(e)[:200]}
+                smtp_result = {"ok": False, "error": _friendly_email_auth_error("SMTP", smtp_host, e)}
 
         return {
             "ok": imap_result["ok"] and (smtp_result is None or smtp_result["ok"]),

routes/history_routes.py

@@ -490,7 +490,13 @@ def setup_history_routes(session_manager) -> APIRouter:
             # Copy messages up to keep_count
             msgs_to_copy = source.history[:keep_count]
             for msg in msgs_to_copy:
-                new_session.add_message(ChatMessage(msg.role, msg.content, msg.metadata))
+                # Copy the metadata dict. Sharing it would let the fork's
+                # persistence (add_message -> _persist_message stamps
+                # _db_id/timestamp onto the dict) mutate the SOURCE session's
+                # in-memory messages, corrupting their _db_id and breaking
+                # edit/delete-by-id on the original conversation.
+                meta = dict(msg.metadata) if isinstance(msg.metadata, dict) else None
+                new_session.add_message(ChatMessage(msg.role, msg.content, meta))
             try:
                 from src.event_bus import fire_event
                 fire_event("session_created", getattr(source, 'owner', None))

routes/model_routes.py

@@ -1232,8 +1232,9 @@ def setup_model_routes(model_discovery):
                 raise HTTPException(401, "Not authenticated")
         except HTTPException:
             raise
-        except Exception:
-            pass
+        except Exception as e:
+            logger.error('Auth gate error in GET /api/models, failing closed: %s', e)
+            raise HTTPException(status_code=500, detail='Internal error')
         # Admins see every endpoint (they manage the global pool); regular
         # users get the owner-scoped view.
         _is_admin = False

routes/preset_routes.py

@@ -9,6 +9,7 @@ from pydantic import BaseModel, Field
 
 from src.request_models import PresetUpdateRequest
 from core.middleware import require_admin
+from src.auth_helpers import effective_user
 
 logger = logging.getLogger(__name__)
 
@@ -100,7 +101,8 @@ def setup_preset_routes(preset_manager) -> APIRouter:
 
         try:
             model_spec = data.get("model") or ""
-            url, model, headers = _resolve_model(model_spec)
+            user = effective_user(request)
+            url, model, headers = _resolve_model(model_spec, owner=user)
             result = await llm_call_async(url, model, messages, temperature=0.8, max_tokens=500, headers=headers)
             return {"success": True, "prompt": result.strip()}
         except Exception as e:

routes/skills_routes.py

@@ -1020,7 +1020,7 @@ def _resolve_audit_models(owner=None):
             spec = (get_setting("teacher_model", "") or "").strip()
             if spec:
                 from src.ai_interaction import _resolve_model
-                t_url, t_model, t_headers = _resolve_model(spec)
+                t_url, t_model, t_headers = _resolve_model(spec, owner=owner)
                 if t_url and t_model:
                     teacher = (t_url, t_model, t_headers)
     except Exception as e:

services/memory/memory_extractor.py

@@ -236,6 +236,43 @@ def _is_text_duplicate(new_text: str, existing: list, threshold: float = 0.6) ->
     return False
 
 
+def _parse_extraction_json(raw: str) -> list:
+    """Parse the extraction LLM's reply into a list of facts, tolerating
+    reasoning-model noise.
+
+    The model emits <think>…</think> (and sometimes a prose preamble or a
+    ```json fence) AROUND the JSON array; without stripping it, json.loads
+    bombs and the run silently yields "0 candidates". Pure str -> list (no
+    LLM/network); returns [] on any parse failure instead of raising.
+    """
+    text = (raw or "").strip()
+    try:
+        from src.text_helpers import strip_think as _strip_think
+        text = _strip_think(text, prose=True, prompt_echo=True).strip()
+    except Exception:
+        pass
+    if text.startswith("```"):
+        text = text.split("\n", 1)[-1].rsplit("```", 1)[0].strip()
+    # JSON may still be embedded in surrounding commentary (leading prose or
+    # trailing remarks like "[...] Done!") — slice from the first '[' to the
+    # last ']' whenever both exist. Slice unconditionally: a reply that starts
+    # with '[' can still carry trailing commentary that breaks json.loads.
+    _start = text.find("[")
+    _end = text.rfind("]")
+    if 0 <= _start < _end:
+        text = text[_start : _end + 1]
+
+    try:
+        facts = json.loads(text)
+    except json.JSONDecodeError:
+        logger.debug("Memory extraction returned non-JSON: %r", (raw or "")[:120])
+        return []
+    except Exception:
+        logger.debug("Memory extraction returned non-JSON: %r", (raw or "")[:120])
+        return []
+    return facts if isinstance(facts, list) else []
+
+
 async def extract_and_store(
     session,
     memory_manager,
@@ -284,9 +321,34 @@ async def extract_and_store(
 
         fallback_facts = _fallback_memory_candidates(stripped_recent)
 
+        # Flatten the window into a SINGLE user message instead of appending the
+        # raw alternating role messages. Passed as raw chat messages, the model
+        # treats the window as a conversation to CONTINUE rather than a transcript
+        # to ANALYZE, so it reliably extracts nothing — typically returning `[]`
+        # (and, depending on the input, sometimes an empty or <think>-only
+        # completion when the window ends on an assistant turn). This was the real
+        # cause of auto-memory logging "0 candidates" on every run. Reframing it as
+        # one "analyze this transcript, return the JSON array" user message makes
+        # the model actually extract. Controlled repro on this model: 0/6 trials
+        # with the old structure vs 6/6 with this one. The skill extractor flattens
+        # for the same reason.
+        def _flatten_msg(m):
+            c = m.get("content", "")
+            if isinstance(c, list):
+                c = " ".join(
+                    b.get("text", "") for b in c
+                    if isinstance(b, dict) and b.get("type") == "text"
+                )
+            return f"{m.get('role', '?')}: {c}"
+
+        transcript = "\n\n".join(_flatten_msg(m) for m in stripped_recent)
         extraction_messages = [
             {"role": "system", "content": EXTRACT_SYSTEM_PROMPT},
-        ] + stripped_recent
+            {"role": "user", "content": (
+                "Conversation to analyze:\n\n" + transcript
+                + "\n\nReturn the JSON array of durable facts now (or [] if none)."
+            )},
+        ]
 
         facts = []
         try:
@@ -295,19 +357,20 @@ async def extract_and_store(
                 model,
                 extraction_messages,
                 temperature=0.1,
-                max_tokens=500,
+                # A reasoning model spends most of its budget on <think> tokens
+                # BEFORE emitting the JSON, so the old 500 truncated the response
+                # before any JSON appeared → every run logged "0 candidates". The
+                # audit path hit the same wall and raised to 16384; extraction's
+                # output (a short facts list) is small, so an ample ceiling is
+                # enough once thinking has room.
+                max_tokens=4096,
                 headers=headers,
             )
 
-            # Parse JSON from response (handle markdown fences if model wraps them)
-            text = raw.strip()
-            if text.startswith("```"):
-                text = text.split("\n", 1)[-1].rsplit("```", 1)[0].strip()
-
-            try:
-                facts = json.loads(text)
-            except json.JSONDecodeError:
-                logger.debug("Memory extraction returned non-JSON")
+            # Parse JSON, tolerating reasoning-model noise (<think> blocks, a
+            # ```json fence, and leading/trailing commentary). See
+            # _parse_extraction_json — returns [] rather than raising.
+            facts = _parse_extraction_json(raw)
         except Exception as e:
             logger.warning(f"LLM memory extraction failed; using fallback candidates if available: {e}")
 

src/builtin_actions.py

@@ -168,7 +168,6 @@ async def action_consolidate_memory(owner: str, **kwargs) -> Tuple[str, bool]:
                     drop_items = decision.get("drop") if isinstance(decision, dict) else None
                     if isinstance(keep_items, list) and isinstance(drop_items, list):
                         by_id = {m.get("id"): m for m in group_memories if m.get("id")}
-                        keep_ids = set()
                         cleaned_by_id = {}
                         for item in keep_items:
                             if not isinstance(item, dict):
@@ -179,7 +178,6 @@ async def action_consolidate_memory(owner: str, **kwargs) -> Tuple[str, bool]:
                             text = (item.get("text") or "").strip()
                             if not text:
                                 continue
-                            keep_ids.add(mid)
                             cleaned = {
                                 "category": (item.get("category") or by_id[mid].get("category") or "fact").strip(),
                             }
@@ -188,11 +186,20 @@ async def action_consolidate_memory(owner: str, **kwargs) -> Tuple[str, bool]:
                                 cleaned["text"] = text
                             cleaned_by_id[mid] = cleaned
 
-                        # If the model only saw a truncated memory, do not let
-                        # that partial view delete or rewrite the full memory.
-                        keep_ids.update(mid for mid in truncated_ids if mid in by_id)
+                        # Delete only memories the model EXPLICITLY dropped, never
+                        # ones it merely omitted from `keep`. Treating the
+                        # complement of `keep` as deletions meant a model that
+                        # forgot to re-list an id (common) silently destroyed that
+                        # memory. Honor the explicit `drop` set instead.
+                        drop_ids = {
+                            d.get("id")
+                            for d in drop_items
+                            if isinstance(d, dict) and d.get("id") in by_id
+                        }
+                        # Never delete a memory the model only saw truncated.
+                        drop_ids -= truncated_ids
 
-                        if keep_ids:
+                        if drop_ids or cleaned_by_id:
                             changed_text = 0
                             group_ref_ids = {id(m) for m in group_memories}
                             kept_all = []
@@ -201,7 +208,7 @@ async def action_consolidate_memory(owner: str, **kwargs) -> Tuple[str, bool]:
                                     kept_all.append(mem)
                                     continue
                                 mid = mem.get("id")
-                                if mid not in keep_ids:
+                                if mid in drop_ids:
                                     continue
                                 cleaned = cleaned_by_id.get(mid) or {}
                                 if mid in truncated_ids:
@@ -213,7 +220,7 @@ async def action_consolidate_memory(owner: str, **kwargs) -> Tuple[str, bool]:
                                     mem["category"] = cleaned["category"]
                                 kept_all.append(mem)
 
-                            removed = len(group_memories) - len(keep_ids)
+                            removed = sum(1 for m in group_memories if m.get("id") in drop_ids)
                             total_scanned += len(group_memories)
                             if removed or changed_text:
                                 all_memories = kept_all

src/caldav_sync.py

@@ -242,6 +242,20 @@ def _build_dav_client(url: str, username: str, password: str):
     return client
 
 
+def _should_prune_window(seen_uids: set, parse_failed: bool) -> bool:
+    """Whether the post-sync prune of vanished CalDAV events is safe to run.
+
+    The prune deletes local ``origin=="caldav"`` rows in the window whose UID the
+    server did not just return. Any parse failure (total or partial) makes
+    ``seen_uids`` an incomplete view of the server, so pruning against it can
+    delete events that still exist upstream but could not be read: a total
+    failure wipes the whole window, a partial failure deletes just the
+    unreadable ones. Only prune on a clean read. An empty ``seen_uids`` after a
+    clean read is a genuinely empty window, which is safe to prune.
+    """
+    return not parse_failed
+
+
 def _sync_blocking(owner: str, url: str, username: str, password: str, account_id: str = "") -> dict:
     """The actual sync — synchronous, intended to run in a threadpool.
     Returns counts: {calendars, events, deleted, errors}."""
@@ -328,6 +342,7 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i
                 # duplicate UIDs within the same batch are updated, not re-inserted
                 # (which would violate the UNIQUE constraint on commit).
                 pending: dict = {}
+                parse_failed = False
                 try:
                     objs = remote_cal.date_search(start=start, end=end, expand=False)
                 except Exception as e:
@@ -339,6 +354,7 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i
                         ical = iCal.from_ical(obj.data)
                     except Exception as e:
                         result["errors"].append(f"{display_name}: parse failed ({e})")
+                        parse_failed = True
                         continue
 
                     for comp in ical.walk():
@@ -415,17 +431,23 @@ def _sync_blocking(owner: str, url: str, username: str, password: str, account_i
                 # are prunable; locally-created events (agent / email triage / a
                 # UI event whose write-back failed) carry origin NULL and must
                 # never be deleted just because the server didn't return them.
-                stale = db.query(CalendarEvent).filter(
-                    CalendarEvent.calendar_id == local_cal.id,
-                    CalendarEvent.origin == "caldav",
-                    CalendarEvent.dtstart >= start,
-                    CalendarEvent.dtstart <= end,
-                    ~CalendarEvent.uid.in_(seen_uids) if seen_uids else CalendarEvent.uid.isnot(None),
-                ).all()
-                for ev in stale:
-                    db.delete(ev)
-                result["deleted"] += len(stale)
-                db.commit()
+                # Skip the prune on any parse failure: seen_uids is then an
+                # incomplete view of the server, so pruning against it would
+                # delete events that still exist upstream but could not be read
+                # (the empty-seen_uids case wipes the whole window; a partial
+                # failure deletes just the unreadable rows).
+                if _should_prune_window(seen_uids, parse_failed):
+                    stale = db.query(CalendarEvent).filter(
+                        CalendarEvent.calendar_id == local_cal.id,
+                        CalendarEvent.origin == "caldav",
+                        CalendarEvent.dtstart >= start,
+                        CalendarEvent.dtstart <= end,
+                        ~CalendarEvent.uid.in_(seen_uids) if seen_uids else CalendarEvent.uid.isnot(None),
+                    ).all()
+                    for ev in stale:
+                        db.delete(ev)
+                    result["deleted"] += len(stale)
+                    db.commit()
             except Exception as e:
                 logger.exception("CalDAV sync failed for one calendar")
                 result["errors"].append(str(e)[:200])

src/preset_manager.py

@@ -115,9 +115,12 @@ Use precise language. Show causal relationships explicitly. Quantify uncertainty
     def save(self, presets: Dict[str, Any]) -> bool:
         """Save presets to file"""
         try:
-            os.makedirs(os.path.dirname(self.presets_file), exist_ok=True)
-            with open(self.presets_file, 'w', encoding="utf-8") as f:
-                json.dump(presets, f, indent=2)
+            # Atomic write (tmp file + os.replace) so a crash or serialization
+            # error mid-write can't truncate presets.json and lose every saved
+            # preset. Lazy import keeps this module free of the heavy core
+            # package import graph at load time.
+            from core.atomic_io import atomic_write_json
+            atomic_write_json(self.presets_file, presets, indent=2)
             self.presets = presets
             return True
         except Exception as e:

src/teacher_escalation.py

@@ -229,12 +229,13 @@ portable across users / hosts.
 """
 
 
-async def _call_teacher(teacher_model_spec: str, prompt: str) -> Optional[str]:
+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
     try:
-        url, model, headers = _resolve_model(teacher_model_spec)
+        url, model, headers = _resolve_model(teacher_model_spec, owner=owner)
     except Exception as e:
         logger.warning(f"teacher endpoint not resolvable ({teacher_model_spec!r}): {e}")
         return None
@@ -388,7 +389,7 @@ async def escalate_and_learn(
         untrusted_trace_guard=_UNTRUSTED_TRACE_GUARD,
         trace=_format_trace(tool_results, agent_reply),
     )
-    response = await _call_teacher(teacher_spec, prompt)
+    response = await _call_teacher(teacher_spec, prompt, owner=owner)
     if not response:
         return None
 
@@ -523,7 +524,7 @@ async def run_teacher_inline(
     # Resolve teacher endpoint
     try:
         from src.ai_interaction import _resolve_model
-        teacher_url, teacher_model, teacher_headers = _resolve_model(teacher_spec)
+        teacher_url, teacher_model, teacher_headers = _resolve_model(teacher_spec, owner=owner)
     except Exception as e:
         logger.warning(f"teacher endpoint not resolvable ({teacher_spec!r}): {e}")
         yield (
@@ -617,7 +618,7 @@ async def run_teacher_inline(
         untrusted_trace_guard=_UNTRUSTED_TRACE_GUARD,
         trace=_format_trace(captured_tool_events, teacher_text),
     )
-    skill_response = await _call_teacher(teacher_spec, prompt)
+    skill_response = await _call_teacher(teacher_spec, prompt, owner=owner)
     if skill_response and "NO_SKILL" in skill_response and not _extract_skill_json(skill_response):
         logger.info("teacher declined to write a skill (NO_SKILL)")
         yield (

static/js/admin.js

@@ -432,8 +432,7 @@ async function loadEndpoints() {
               ${ep.is_enabled ? '' : '<span class="admin-badge admin-badge-off">disabled</span>'}
               ${hasModels ? '<span style="font-size:10px;opacity:0.4;">Click to manage models</span>' : ''}
             </div>
-            <div class="admin-ep-actions">
-              ${_isLocalEndpoint(ep.base_url) ? '<select class="admin-tools-select" data-adm-tools-select="' + ep.id + '" title="Native tool calling mode. Auto = use heuristic (fenced blocks for Ollama /v1). On = always use native function calling. Off = always use fenced blocks."><option value="auto"' + (ep.supports_tools !== true && ep.supports_tools !== false ? ' selected' : '') + '>Tools: Auto</option><option value="true"' + (ep.supports_tools === true ? ' selected' : '') + '>Tools: On</option><option value="false"' + (ep.supports_tools === false ? ' selected' : '') + '>Tools: Off</option></select>' : ''}
+            <div style="display:flex;gap:4px;align-items:center;">
               <button class="admin-btn-sm" data-adm-toggle-ep="${ep.id}">${ep.is_enabled ? 'Disable' : 'Enable'}</button>
               <button class="admin-btn-delete" data-adm-del-ep="${ep.id}" data-adm-ep-online="${ep.online ? '1' : '0'}">Delete</button>
               ${hasModels ? '<svg class="admin-user-chevron" width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round" style="opacity:0.3;transition:transform 0.2s,opacity 0.2s;"><polyline points="6 9 12 15 18 9"/></svg>' : ''}
@@ -478,22 +477,6 @@ async function loadEndpoints() {
     queryAll('[data-adm-toggle-ep]').forEach(btn => {
       btn.addEventListener('click', async (e) => { e.stopPropagation(); await fetch(`/api/model-endpoints/${btn.dataset.admToggleEp}`, { method: 'PATCH' }); loadEndpoints(); });
     });
-    queryAll('[data-adm-tools-select]').forEach(sel => {
-      sel.addEventListener('change', async (e) => {
-        e.stopPropagation();
-        const epId = sel.dataset.admToolsSelect;
-        const val = sel.value;
-        const body = {};
-        if (val === 'auto') body.supports_tools = null;
-        else body.supports_tools = val === 'true';
-        await fetch(`/api/model-endpoints/${epId}`, {
-          method: 'PATCH',
-          headers: { 'Content-Type': 'application/json' },
-          body: JSON.stringify(body),
-        });
-        loadEndpoints();
-      });
-    });
     queryAll('[data-adm-copy-url]').forEach(btn => {
       btn.addEventListener('click', (e) => {
         e.stopPropagation();

static/js/modelPicker.js

@@ -35,11 +35,6 @@ function _pushRecent(mid) {
   next.unshift(mid);
   _saveList(RECENT_KEY, next.slice(0, RECENT_MAX));
 }
-function _removeRecent(mid) {
-  if (!mid) return;
-  const next = _loadRecent().filter(x => x !== mid);
-  _saveList(RECENT_KEY, next);
-}
 function _loadFavorites() { return _loadList(FAVORITES_KEY); }
 function _toggleFavorite(mid) {
   const favs = _loadFavorites();
@@ -309,7 +304,7 @@ function _initModelPickerDropdown() {
       empty.textContent = text;
       listEl.appendChild(empty);
     }
-    function _addRow(m, onRemove) {
+    function _addRow(m) {
       const row = document.createElement('div');
       row.className = 'model-switch-item';
       if (m.stale) {
@@ -381,20 +376,6 @@ function _initModelPickerDropdown() {
       });
       row.appendChild(favDot);
 
-      // Remove-from-recent button (shown only for Recent section items).
-      if (onRemove) {
-        const rmBtn = document.createElement('button');
-        rmBtn.type = 'button';
-        rmBtn.className = 'mp-remove-dot';
-        rmBtn.textContent = '×';
-        rmBtn.title = 'Remove from recent';
-        rmBtn.addEventListener('click', (e) => {
-          e.stopPropagation();
-          onRemove();
-        });
-        row.appendChild(rmBtn);
-      }
-
       row.addEventListener('click', () => _pick(m));
       listEl.appendChild(row);
     }
@@ -411,7 +392,8 @@ function _initModelPickerDropdown() {
       return;
     }
 
-    // ── Browse mode: sections in order: Favorites → Recent (big catalogs only) → All / Providers ──
+    // ── Browse mode: Favorites (manual) + Recent (auto), with dedupe. ──
+    // Rules:
     //   1. Never list the same model twice in the dropdown. Favorites
     //      win over Recent (if you favorited it, that's where it
     //      belongs — Recent shouldn't show it again as duplicate).
@@ -436,13 +418,7 @@ function _initModelPickerDropdown() {
         .slice(0, RECENT_MAX);
       if (recentModels.length) {
         _addSection('Recent');
-        recentModels.forEach(m => {
-          shown.add(m.mid);
-          _addRow(m, () => {
-            _removeRecent(m.mid);
-            _populate('');
-          });
-        });
+        recentModels.forEach(m => { shown.add(m.mid); _addRow(m); });
       }
     }
 

static/js/settings.js

@@ -2736,13 +2736,14 @@ async function initEmailAccountsSettings() {
       <h3 style="font-size:12px;margin:0 0 8px">${isEdit ? 'Edit Account' : 'New Account'}</h3>
       <div class="settings-col">
         <div class="settings-row"><label class="settings-label">Provider${_hint('Pick a known provider to auto-fill the IMAP and SMTP host/port. Choose Custom to type your own.')}</label><select id="eaf-provider" class="settings-select"><option value="">Custom…</option>${_providerOptions}</select></div>
+        <div id="eaf-provider-note" style="display:none;font-size:11px;line-height:1.5;padding:8px 10px;margin:2px 0 4px;border:1px solid color-mix(in srgb, var(--fg) 15%, transparent);border-left:3px solid var(--accent, var(--red));border-radius:4px;background:color-mix(in srgb, var(--fg) 4%, transparent);"></div>
         <div class="settings-row"><label class="settings-label">Name${_hint('Optional label for this account (e.g. “Work” or “Personal”). Leave blank to use the email address.')}</label><input id="eaf-name" class="settings-input" placeholder="(optional — leave blank to use email)" value="${esc(a.name || '')}"></div>
         <div class="settings-row"><label class="settings-label">Email${_hint('Your email address. Used as the From: header on outgoing mail and as the display label when Name is blank.')}</label><input id="eaf-from" class="settings-input" placeholder="you@example.com" value="${esc(a.from_address || '')}"></div>
         <div style="font-size:11px;font-weight:600;opacity:0.6;margin:6px 0 2px">IMAP (Receiving)</div>
         <div class="settings-row"><label class="settings-label">Host${_hint('Your IMAP server, e.g. imap.gmail.com, imap.migadu.com, a LAN host, or a Tailscale IP for Dovecot.')}</label><input id="eaf-imap-host" class="settings-input" value="${esc(a.imap_host || '')}"></div>
         <div class="settings-row"><label class="settings-label">Port${_hint('993 for IMAPS (most providers), 143 for plain or STARTTLS. Local servers often use a custom port like 31143.')}</label><input id="eaf-imap-port" class="settings-input" type="number" value="${esc(a.imap_port || 993)}" style="max-width:100px"></div>
         <div class="settings-row"><label class="settings-label">Username${_hint('Usually your full email address.')}</label><input id="eaf-imap-user" class="settings-input" value="${esc(a.imap_user || '')}"></div>
-        <div class="settings-row"><label class="settings-label">Password${_hint('Your IMAP login password. Use an app-specific password if your provider requires 2FA (Gmail, iCloud, etc.).')}</label><input id="eaf-imap-pass" class="settings-input" type="password" placeholder="${isEdit && a.has_imap_password ? '(unchanged)' : ''}"></div>
+        <div class="settings-row"><label class="settings-label">Password${_hint('Your IMAP login password. Use an app-specific password if your provider requires 2FA. Outlook / Office 365 generally requires OAuth and will not work with a normal password here.')}</label><input id="eaf-imap-pass" class="settings-input" type="password" placeholder="${isEdit && a.has_imap_password ? '(unchanged)' : ''}"></div>
         <div class="settings-row"><label class="settings-label">STARTTLS${_hint('Turn ON for port 143/587 to upgrade plain to TLS. Turn OFF for port 993 (IMAPS — already encrypted) or a local server with no TLS configured.')}</label><label class="admin-switch"><input type="checkbox" id="eaf-imap-starttls" ${a.imap_starttls !== false ? 'checked' : ''}><span class="admin-slider"></span></label></div>
         <div style="font-size:11px;font-weight:600;opacity:0.6;margin:8px 0 2px">SMTP (Sending) <span style="font-weight:normal;opacity:0.7">— optional, leave blank for read-only</span></div>
         <div class="settings-row"><label class="settings-label">Host${_hint('Your outgoing-mail server, e.g. smtp.gmail.com, smtp.migadu.com. Leave blank to make this account read-only.')}</label><input id="eaf-smtp-host" class="settings-input" value="${esc(a.smtp_host || '')}"></div>
@@ -2750,7 +2751,7 @@ async function initEmailAccountsSettings() {
         <div class="settings-row"><label class="settings-label">Security${_hint('SSL for port 465, STARTTLS for port 587, or None for local SMTP bridges such as Proton Mail Bridge.')}</label><select id="eaf-smtp-security" class="settings-select"><option value="ssl">SSL</option><option value="starttls">STARTTLS</option><option value="none">None</option></select></div>
         <div class="settings-row"><label class="settings-label">Same as IMAP${_hint('Use the IMAP username and password for SMTP too (this is right for almost every provider). Turn off to enter separate SMTP credentials.')}</label><label class="admin-switch"><input type="checkbox" id="eaf-smtp-same" ${(!isEdit || (a.smtp_user && a.imap_user && a.smtp_user === a.imap_user)) ? 'checked' : ''}><span class="admin-slider"></span></label></div>
         <div class="settings-row eaf-smtp-creds"><label class="settings-label">Username${_hint('Usually the same as your IMAP username (your email address).')}</label><input id="eaf-smtp-user" class="settings-input" value="${esc(a.smtp_user || '')}"></div>
-        <div class="settings-row eaf-smtp-creds"><label class="settings-label">Password${_hint('Your SMTP password — often the same as your IMAP password.')}</label><input id="eaf-smtp-pass" class="settings-input" type="password" placeholder="${isEdit && a.has_smtp_password ? '(unchanged)' : ''}"></div>
+        <div class="settings-row eaf-smtp-creds"><label class="settings-label">Password${_hint('Your SMTP password — often the same as your IMAP password. Outlook / Office 365 generally requires OAuth and will not work with a normal password here.')}</label><input id="eaf-smtp-pass" class="settings-input" type="password" placeholder="${isEdit && a.has_smtp_password ? '(unchanged)' : ''}"></div>
         <div class="settings-row" style="margin-top:10px;align-items:center;">
           <button class="admin-btn-add" id="eaf-save" style="background:var(--red);border-color:var(--red);color:#fff;display:inline-flex;align-items:center;gap:5px;font-weight:600;">
             <svg width="11" height="11" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="3" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><polyline points="20 6 9 17 4 12"/></svg>
@@ -2765,8 +2766,29 @@ async function initEmailAccountsSettings() {
       </div>
     `;
 
+    const eafProviderNotes = {
+      outlook: {
+        title: 'Outlook / Office 365 needs OAuth',
+        body: 'Microsoft disables normal password login for IMAP/SMTP in most Outlook and Microsoft 365 accounts. Odysseus does not support Microsoft OAuth/Graph mail yet, so this preset is only a placeholder for future support.',
+      },
+    };
+    const eafNoteEl = el('eaf-provider-note');
+    const _renderEafProviderNote = (key) => {
+      const n = eafProviderNotes[key];
+      if (!eafNoteEl || !n) {
+        if (eafNoteEl) {
+          eafNoteEl.style.display = 'none';
+          eafNoteEl.innerHTML = '';
+        }
+        return;
+      }
+      eafNoteEl.style.display = '';
+      eafNoteEl.innerHTML = `<div style="font-weight:600;margin-bottom:3px;">${esc(n.title)}</div><div style="opacity:0.8;">${esc(n.body)}</div>`;
+    };
+
     // Provider preset → autofill host/port/STARTTLS for both halves.
     el('eaf-provider').addEventListener('change', (e) => {
+      _renderEafProviderNote(e.target.value);
       const p = PROVIDERS[e.target.value];
       if (!p) return;
       el('eaf-imap-host').value = p.imap.host;
@@ -4071,7 +4093,7 @@ async function initUnifiedIntegrations() {
           <div class="settings-row"><label class="settings-label">Host${_hint('Your IMAP server, e.g. imap.gmail.com, imap.migadu.com, a LAN host, or a Tailscale IP for Dovecot.')}</label><input id="uf-imap-host" class="settings-input" placeholder="imap.example.com"></div>
           <div class="settings-row"><label class="settings-label">Port${_hint('993 for IMAPS (most providers), 143 for plain or STARTTLS. Local servers often use a custom port like 31143.')}</label><input id="uf-imap-port" class="settings-input" type="number" placeholder="993" style="max-width:100px"></div>
           <div class="settings-row"><label class="settings-label">Username${_hint('Yes — your full email address goes here too (e.g. you@gmail.com). Same as the Email field above for almost every provider.')}</label><input id="uf-imap-user" class="settings-input" placeholder="you@example.com"></div>
-          <div class="settings-row"><label class="settings-label">Password${_hint('For Gmail, iCloud, and Yahoo: paste your App Password (NOT your normal account password — those are blocked for IMAP). For Migadu, Fastmail, Outlook, etc.: your regular mailbox password works.')}</label><input id="uf-imap-pass" class="settings-input" type="password" placeholder="${placeholderPass}"></div>
+          <div class="settings-row"><label class="settings-label">Password${_hint('For Gmail, iCloud, and Yahoo: paste your App Password (NOT your normal account password). For Migadu and Fastmail, your mailbox password usually works. Outlook / Office 365 generally requires OAuth and will not work with this password form.')}</label><input id="uf-imap-pass" class="settings-input" type="password" placeholder="${placeholderPass}"></div>
           <div class="settings-row"><label class="settings-label">STARTTLS${_hint('Turn ON for port 143/587 to upgrade plain to TLS. Turn OFF for port 993 (IMAPS — already encrypted) or a local server with no TLS configured.')}</label><label class="admin-switch" style="margin-left:0"><input type="checkbox" id="uf-imap-starttls" checked><span class="admin-slider"></span></label></div>
           <div style="font-size:11px;font-weight:600;opacity:0.6;margin:8px 0 2px;display:flex;align-items:center;gap:5px;"><svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" style="color:var(--accent, var(--red));flex-shrink:0;" aria-hidden="true"><line x1="22" y1="2" x2="11" y2="13"/><polygon points="22 2 15 22 11 13 2 9 22 2"/></svg>SMTP (Sending) <span style="font-weight:normal;opacity:0.7">— optional, leave blank for read-only</span></div>
           <div class="settings-row"><label class="settings-label">Host${_hint('Your outgoing-mail server, e.g. smtp.gmail.com. Leave blank to make this account read-only.')}</label><input id="uf-smtp-host" class="settings-input" placeholder="smtp.example.com"></div>
@@ -4079,7 +4101,7 @@ async function initUnifiedIntegrations() {
           <div class="settings-row"><label class="settings-label">Security${_hint('SSL for port 465, STARTTLS for port 587, or None for local SMTP bridges such as Proton Mail Bridge.')}</label><select id="uf-smtp-security" class="settings-select"><option value="ssl">SSL</option><option value="starttls">STARTTLS</option><option value="none">None</option></select></div>
           <div class="settings-row"><label class="settings-label">Same as IMAP${_hint('Use the IMAP username and password for SMTP too (right for almost every provider). Turn off to enter separate SMTP credentials.')}</label><label class="admin-switch" style="margin-left:0"><input type="checkbox" id="uf-smtp-same" checked><span class="admin-slider"></span></label></div>
           <div class="settings-row uf-smtp-creds"><label class="settings-label">Username${_hint('Usually the same as your IMAP username (your email address).')}</label><input id="uf-smtp-user" class="settings-input"></div>
-          <div class="settings-row uf-smtp-creds"><label class="settings-label">Password${_hint('Your SMTP password — often the same as your IMAP password.')}</label><input id="uf-smtp-pass" class="settings-input" type="password" placeholder="${placeholderPass}"></div>
+          <div class="settings-row uf-smtp-creds"><label class="settings-label">Password${_hint('Your SMTP password — often the same as your IMAP password. Outlook / Office 365 generally requires OAuth and will not work with this password form.')}</label><input id="uf-smtp-pass" class="settings-input" type="password" placeholder="${placeholderPass}"></div>
           <div class="settings-row" style="margin-top:4px"><label class="settings-label">Default${_hint('Use this account whenever no specific account is chosen.')}</label><label class="admin-switch" style="margin-left:0"><input type="checkbox" id="uf-email-default"><span class="admin-slider"></span></label><span style="font-size:10px;opacity:0.5;margin-left:6px">Used when nothing else is selected</span></div>
           <div class="settings-row" style="margin-top:10px;align-items:center;">
             <button class="admin-btn-add" id="uf-email-save" style="background:var(--red);border-color:var(--red);color:#fff;display:inline-flex;align-items:center;gap:5px;font-weight:600;">
@@ -4124,6 +4146,12 @@ async function initUnifiedIntegrations() {
         body: 'Generate an App Password from Yahoo Account Security (requires 2-Step Verification enabled) and paste it as the Password.',
         url: 'https://login.yahoo.com/account/security/app-passwords',
       },
+      outlook: {
+        title: 'Outlook / Office 365 needs OAuth',
+        body: 'Microsoft disables normal password login for IMAP/SMTP in most Outlook and Microsoft 365 accounts. Odysseus does not support Microsoft OAuth/Graph mail yet, so this preset is only a placeholder for future support.',
+        url: 'https://learn.microsoft.com/exchange/clients-and-mobile-in-exchange-online/disable-basic-authentication-in-exchange-online',
+        linkLabel: 'Read Microsoft note',
+      },
     };
     const noteEl = el('uf-email-provider-note');
     const _copyProviderUrl = async (text) => {
@@ -4181,7 +4209,7 @@ async function initUnifiedIntegrations() {
         <div style="display:flex;align-items:center;gap:6px;flex-wrap:wrap;">
           <a href="${esc(n.url)}" target="_blank" rel="noopener noreferrer" class="admin-btn-sm" style="background:var(--red);border-color:var(--red);color:#fff;text-decoration:none;display:inline-flex;align-items:center;gap:5px;font-weight:600;">
             <svg width="10" height="10" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M18 13v6a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2V8a2 2 0 0 1 2-2h6"/><polyline points="15 3 21 3 21 9"/><line x1="10" y1="14" x2="21" y2="3"/></svg>
-            Generate App Password
+            ${esc(n.linkLabel || 'Generate App Password')}
           </a>
           <button type="button" class="admin-btn-sm uf-prov-copy" data-url="${esc(n.url)}" style="opacity:0.7;display:inline-flex;align-items:center;gap:5px;">
             <svg width="10" height="10" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><rect x="9" y="9" width="13" height="13" rx="2"/><path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1"/></svg>

static/style.css

@@ -2940,34 +2940,6 @@ body.bg-pattern-sparkles {
       45% { text-shadow: 0 0 10px color-mix(in srgb, var(--accent, var(--red)) 60%, transparent); }
       100% { text-shadow: 0 0 0 color-mix(in srgb, var(--accent, var(--red)) 0%, transparent); }
     }
-    /* Inline remove-from-recent button — only shown on Recent rows. */
-    .model-picker-list .mp-remove-dot {
-      flex: 0 0 auto;
-      display: inline-flex;
-      align-items: center;
-      justify-content: center;
-      width: 24px;
-      height: 24px;
-      margin: -4px -4px -4px 2px;
-      padding: 0;
-      border: none;
-      background: transparent;
-      cursor: pointer;
-      color: color-mix(in srgb, var(--fg) 28%, transparent);
-      font-family: inherit;
-      font-size: 15px;
-      line-height: 1;
-      transition: color 0.15s ease, opacity 0.15s ease, transform 0.12s ease;
-      -webkit-tap-highlight-color: transparent;
-    }
-    .model-picker-list .mp-remove-dot:hover {
-      color: var(--red, #ff5555);
-      transform: scale(1.1);
-    }
-    .model-picker-list .mp-remove-dot:focus-visible {
-      outline: none;
-      color: var(--red, #ff5555);
-    }
     /* First-run hint when a large catalog has no Recent/Favorites yet. */
     .model-picker-list .mp-empty-hint {
       flex-direction: column;
@@ -14154,22 +14126,6 @@ body:has(.doc-version-panel:not(.hidden)) .hamburger-btn {
   background: var(--border);
   border-color: var(--red);
 }
-.admin-tools-select {
-  padding: 3px 6px;
-  border: 1px solid var(--border);
-  border-radius: 6px;
-  background: var(--panel);
-  color: var(--fg);
-  cursor: pointer;
-  font-size: 11px;
-  font-family: inherit;
-  height: 26px;
-  min-width: 90px;
-}
-.admin-tools-select:hover {
-  background: var(--border);
-  border-color: var(--red);
-}
 .admin-spinner {
   display: inline-block;
   width: 12px;
@@ -14285,7 +14241,6 @@ body:has(.doc-version-panel:not(.hidden)) .hamburger-btn {
 .admin-ep-actions {
   display: flex;
   gap: 4px;
-  align-items: center;
   flex-shrink: 0;
 }
 

tests/test_api_token_routes.py

@@ -5,6 +5,7 @@ Uses direct endpoint extraction from setup_api_token_routes().routes and
 fake objects only — no real DB, no network, no external services.
 """
 
+import asyncio
 import contextlib
 import datetime
 import secrets as _secrets_mod
@@ -292,3 +293,84 @@ def test_delete_missing_token_returns_404_without_invalidating_cache(monkeypatch
         delete_token(request=req, token_id="missing99")
     assert exc.value.status_code == 404
     invalidator.assert_not_called()
+
+
+# ---------------------------------------------------------------------------
+# 6. PATCH /api/tokens/{id} — a partial update must not wipe scopes
+# ---------------------------------------------------------------------------
+
+
+def _patch_request(invalidator, body):
+    """An admin request whose async .json() yields `body`."""
+    req = _req("alice", is_admin=True, invalidator=invalidator)
+
+    async def _json():
+        return body
+
+    req.json = _json
+    return req
+
+
+def test_update_token_rename_preserves_scopes(monkeypatch, token_routes_mod):
+    """Renaming a token (no 'scopes' key in the body) must keep its scopes.
+
+    Previously update_token recomputed scopes from payload.get("scopes"),
+    which is None on a rename, so _normalize_scopes(None) reset every token to
+    the default ["chat"] scope — a silent privilege/data loss.
+    """
+    monkeypatch.setenv("AUTH_ENABLED", "true")
+    mod = token_routes_mod
+
+    token = SimpleNamespace(
+        id="tok123", name="original", owner="alice",
+        token_prefix="ody_orig", scopes="email:read,email:draft", is_active=True,
+    )
+    fake_session = MagicMock()
+    fake_session.query.return_value.filter.return_value.first.return_value = token
+    monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
+
+    invalidator = MagicMock()
+    req = _patch_request(invalidator, {"name": "renamed"})
+    update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}")
+    resp = asyncio.run(update_token(request=req, token_id="tok123"))
+
+    assert token.scopes == "email:read,email:draft"  # untouched
+    assert resp["scopes"] == ["email:read", "email:draft"]
+    assert token.name == "renamed"
+    invalidator.assert_called_once()
+
+
+def test_update_token_applies_explicit_scopes(monkeypatch, token_routes_mod):
+    """When the body includes 'scopes', they are normalized and written."""
+    monkeypatch.setenv("AUTH_ENABLED", "true")
+    mod = token_routes_mod
+
+    token = SimpleNamespace(
+        id="tok123", name="original", owner="alice",
+        token_prefix="ody_orig", scopes="email:read,email:draft", is_active=True,
+    )
+    fake_session = MagicMock()
+    fake_session.query.return_value.filter.return_value.first.return_value = token
+    monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
+
+    req = _patch_request(MagicMock(), {"scopes": ["chat"]})
+    update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}")
+    resp = asyncio.run(update_token(request=req, token_id="tok123"))
+
+    assert token.scopes == "chat"
+    assert resp["scopes"] == ["chat"]
+
+
+def test_update_missing_token_returns_404(monkeypatch, token_routes_mod):
+    monkeypatch.setenv("AUTH_ENABLED", "true")
+    mod = token_routes_mod
+
+    fake_session = MagicMock()
+    fake_session.query.return_value.filter.return_value.first.return_value = None
+    monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
+
+    req = _patch_request(MagicMock(), {"name": "x"})
+    update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}")
+    with pytest.raises(HTTPException) as exc:
+        asyncio.run(update_token(request=req, token_id="missing99"))
+    assert exc.value.status_code == 404

tests/test_caldav_prune_parse_failure.py

@@ -0,0 +1,37 @@
+"""CalDAV sync must not prune the window when it can't fully read the server.
+
+The prune deletes local caldav rows whose UID the server didn't return. `seen_uids`
+is built only from objects that parsed, so any parse failure (total or partial)
+makes it an incomplete view of the server:
+
+- total failure: `seen_uids` is empty and the prune falls back to `uid.isnot(None)`
+  (match-all), wiping every event in the window;
+- partial failure: the events that failed to parse are absent from `seen_uids`, so
+  `~uid.in_(seen_uids)` deletes those still-upstream events.
+
+`_should_prune_window` therefore only allows the prune on a clean read.
+"""
+from src.caldav_sync import _should_prune_window
+
+
+def test_prune_runs_on_clean_read():
+    # Clean read with events -> the normal ~uid.in_(seen) prune is safe.
+    assert _should_prune_window({"uid-a", "uid-b"}, parse_failed=False) is True
+
+
+def test_prune_runs_when_calendar_genuinely_empty():
+    # Clean read, no objects -> genuinely empty window -> safe to prune.
+    assert _should_prune_window(set(), parse_failed=False) is True
+
+
+def test_prune_skipped_when_all_objects_failed_to_parse():
+    # Every object failed -> empty seen_uids is "couldn't read", not "empty
+    # calendar" -> must NOT prune (would delete the whole window).
+    assert _should_prune_window(set(), parse_failed=True) is False
+
+
+def test_prune_skipped_on_partial_parse_failure():
+    # Some objects parsed and at least one failed: seen_uids is incomplete, so
+    # pruning would delete the unparsed-but-still-upstream events. Skipping the
+    # prune keeps the local copy of the unparsed event instead of deleting it.
+    assert _should_prune_window({"parsed-uid"}, parse_failed=True) is False

tests/test_consolidate_memory_explicit_drops.py

@@ -0,0 +1,57 @@
+"""Memory consolidation must delete only memories the model explicitly drops.
+
+The AI tidy path computed deletions as the complement of the model's `keep`
+list, so any memory the model simply omitted (a common LLM lapse) was silently
+deleted. The fix honors the explicit `drop` set, so an omitted memory survives.
+"""
+import asyncio
+import json
+
+import src.builtin_actions as ba
+
+
+class _FakeMM:
+    saved = None
+
+    def __init__(self, *args, **kwargs):
+        pass
+
+    def load_all(self):
+        return [
+            {"id": "a", "owner": "alice", "text": "Likes dark roast coffee", "category": "preference"},
+            {"id": "b", "owner": "alice", "text": "Likes dark roast coffee too", "category": "preference"},
+            {"id": "c", "owner": "alice", "text": "Lives in Cairo", "category": "fact"},
+        ]
+
+    def save(self, entries):
+        _FakeMM.saved = list(entries)
+
+
+def test_omitted_memory_survives_only_explicit_drop(monkeypatch):
+    import src.memory
+    import src.endpoint_resolver
+    import src.llm_core
+
+    _FakeMM.saved = None
+    monkeypatch.setattr(src.memory, "MemoryManager", _FakeMM)
+    monkeypatch.setattr(
+        src.endpoint_resolver, "resolve_endpoint",
+        lambda kind, owner=None: ("http://x/v1", "model", {}),
+    )
+
+    async def fake_llm(**kwargs):
+        # Model keeps 'a', drops 'b', and OMITS 'c' entirely.
+        return json.dumps({
+            "keep": [{"id": "a", "text": "Likes dark roast coffee", "category": "preference"}],
+            "drop": [{"id": "b", "reason": "duplicate of a"}],
+        })
+
+    monkeypatch.setattr(src.llm_core, "llm_call_async", fake_llm)
+
+    msg, ok = asyncio.run(ba.action_consolidate_memory("alice"))
+
+    assert ok, msg
+    ids = {m["id"] for m in _FakeMM.saved}
+    assert "c" in ids, "omitted memory must NOT be deleted"
+    assert "a" in ids
+    assert "b" not in ids, "explicitly dropped memory should be removed"

tests/test_fork_session_metadata.py

@@ -0,0 +1,84 @@
+"""Forking a session must not mutate the source session's messages.
+
+ChatMessage.metadata is a dict. add_message() -> _persist_message() stamps
+_db_id (and timestamp) onto that dict in place. The fork handler used to pass
+the source message's metadata dict by reference into the new session, so
+persisting the fork rewrote the SOURCE messages' _db_id — breaking
+edit/delete-by-id on the original conversation. The fork must copy the dict.
+"""
+import asyncio
+from types import SimpleNamespace
+
+from core.models import ChatMessage
+import routes.history_routes as mod
+
+
+class _FakeSession:
+    def __init__(self, name="", owner=None):
+        self.name = name
+        self.owner = owner
+        self.endpoint_url = ""
+        self.model = ""
+        self.history = []
+
+    def add_message(self, message):
+        # Mirror _persist_message: stamp the in-memory message's metadata.
+        if message.metadata is None:
+            message.metadata = {}
+        message.metadata["_db_id"] = f"new-{len(self.history)}"
+        self.history.append(message)
+
+
+class _FakeSessionManager:
+    def __init__(self, source):
+        self.sessions = {"src-id": source}
+        self.created = None
+
+    def create_session(self, session_id=None, name=None, endpoint_url=None,
+                       model=None, rag=False, owner=None):
+        self.created = _FakeSession(name=name, owner=owner)
+        return self.created
+
+    def save_sessions(self):
+        pass
+
+
+def _fork_handler(router):
+    for route in router.routes:
+        if "/fork" in getattr(route, "path", "") and "POST" in getattr(route, "methods", set()):
+            return route.endpoint
+    raise AssertionError("fork route not found")
+
+
+def test_fork_does_not_corrupt_source_message_metadata(monkeypatch):
+    monkeypatch.setattr(mod, "_verify_session_owner", lambda *a, **k: None)
+
+    source = _FakeSession(name="Original", owner="alice")
+    source.history = [
+        ChatMessage("user", "hi", {"_db_id": "src-0"}),
+        ChatMessage("assistant", "yo", {"_db_id": "src-1"}),
+    ]
+    sm = _FakeSessionManager(source)
+
+    req = SimpleNamespace()
+
+    async def _json():
+        return {"keep_count": 2}
+
+    req.json = _json
+
+    router = mod.setup_history_routes(sm)
+    fork = _fork_handler(router)
+    result = asyncio.run(fork(request=req, session_id="src-id"))
+
+    assert result["status"] == "ok"
+    assert result["kept"] == 2
+
+    # The forked session got its own metadata dicts...
+    new_session = sm.created
+    assert new_session.history[0].metadata is not source.history[0].metadata
+    assert new_session.history[1].metadata is not source.history[1].metadata
+
+    # ...and the source session's _db_id values are untouched.
+    assert source.history[0].metadata["_db_id"] == "src-0"
+    assert source.history[1].metadata["_db_id"] == "src-1"

tests/test_imap_leak_fixes.py

@@ -9,6 +9,7 @@ Functions covered:
     _download_attachment
 """
 
+import imaplib
 import os
 import sys
 import tempfile
@@ -228,3 +229,176 @@ def test_mcp_download_attachment_logs_out_on_select_failure(monkeypatch):
         f"conn.logout() must be called after select raises in _download_attachment. "
         f"Got logout_calls={captured.get('logout_calls')}"
     )
+
+
+# ── connect-time leak: _imap_connect / _open_imap_connection (#3174) ──────────
+# The cases above all monkeypatch _imap_connect to *succeed*; these cover the
+# gap where the connect itself fails (bad/expired app password, rejected
+# STARTTLS) and the already-open socket would otherwise be orphaned.
+
+
+def test_imap_connect_shuts_down_socket_on_login_failure(monkeypatch):
+    """A failed login() must close the already-connected socket, not leak it."""
+    import routes.email_helpers as helpers
+
+    captured = {}
+    conn = MagicMock()
+    conn.shutdown = MagicMock(side_effect=lambda: captured.__setitem__(
+        "shutdown_calls", captured.get("shutdown_calls", 0) + 1
+    ))
+    conn.login = MagicMock(side_effect=imaplib.IMAP4.error(b"AUTHENTICATE failed."))
+
+    monkeypatch.setattr(helpers, "_get_email_config", lambda *a, **kw: {
+        "imap_host": "imap.example.com",
+        "imap_port": 993,
+        "imap_starttls": False,
+        "imap_user": "user@example.com",
+        "imap_password": "wrong",
+    })
+    monkeypatch.setattr(helpers, "_open_imap_connection", lambda *a, **kw: conn)
+
+    raised = False
+    try:
+        helpers._imap_connect()
+    except Exception:
+        raised = True
+
+    assert raised, "login failure must propagate to the caller"
+    assert captured.get("shutdown_calls", 0) == 1, (
+        f"conn.shutdown() must be called exactly once when login fails. "
+        f"Got shutdown_calls={captured.get('shutdown_calls')}"
+    )
+
+
+def test_open_imap_connection_shuts_down_on_starttls_failure(monkeypatch):
+    """A rejected STARTTLS upgrade must close the open plain socket."""
+    import routes.email_helpers as helpers
+
+    captured = {}
+    conn = MagicMock()
+    conn.shutdown = MagicMock(side_effect=lambda: captured.__setitem__(
+        "shutdown_calls", captured.get("shutdown_calls", 0) + 1
+    ))
+    conn.starttls = MagicMock(side_effect=RuntimeError("STARTTLS rejected"))
+
+    monkeypatch.setattr(helpers.imaplib, "IMAP4", lambda *a, **kw: conn)
+
+    raised = False
+    try:
+        helpers._open_imap_connection("imap.example.com", 143, starttls=True)
+    except Exception:
+        raised = True
+
+    assert raised, "starttls failure must propagate to the caller"
+    assert captured.get("shutdown_calls", 0) == 1, (
+        f"conn.shutdown() must be called exactly once when STARTTLS fails. "
+        f"Got shutdown_calls={captured.get('shutdown_calls')}"
+    )
+
+
+# ── connect-time leak: mcp_servers/email_server.py (folded in per review #3363) ──
+# Same connect-then-step pattern as the routes path. IMAP closes pre-auth with
+# shutdown(); SMTP has no shutdown(), so close() (socket close, no QUIT).
+
+
+def _cfg_imap(ssl=True, starttls=False):
+    return {
+        "imap_ssl": ssl, "imap_starttls": starttls,
+        "imap_host": "imap.example.com", "imap_port": 993,
+        "imap_user": "user@example.com", "imap_password": "wrong",
+    }
+
+
+def test_mcp_imap_connect_shuts_down_on_login_failure(monkeypatch):
+    import mcp_servers.email_server as srv
+
+    captured = {}
+    conn = MagicMock()
+    conn.shutdown = MagicMock(side_effect=lambda: captured.__setitem__(
+        "shutdown_calls", captured.get("shutdown_calls", 0) + 1))
+    conn.login = MagicMock(side_effect=imaplib.IMAP4.error(b"AUTHENTICATE failed."))
+    monkeypatch.setattr(srv, "_load_config", lambda *a, **kw: _cfg_imap(ssl=True))
+    monkeypatch.setattr(srv.imaplib, "IMAP4_SSL", lambda *a, **kw: conn)
+
+    raised = False
+    try:
+        srv._imap_connect()
+    except Exception:
+        raised = True
+    assert raised, "login failure must propagate"
+    assert captured.get("shutdown_calls", 0) == 1, (
+        f"shutdown() must be called once on MCP IMAP login failure. Got {captured.get('shutdown_calls')}")
+
+
+def test_mcp_imap_connect_shuts_down_on_starttls_failure(monkeypatch):
+    import mcp_servers.email_server as srv
+
+    captured = {}
+    conn = MagicMock()
+    conn.shutdown = MagicMock(side_effect=lambda: captured.__setitem__(
+        "shutdown_calls", captured.get("shutdown_calls", 0) + 1))
+    conn.starttls = MagicMock(side_effect=RuntimeError("STARTTLS rejected"))
+    monkeypatch.setattr(srv, "_load_config", lambda *a, **kw: _cfg_imap(ssl=False, starttls=True))
+    monkeypatch.setattr(srv.imaplib, "IMAP4", lambda *a, **kw: conn)
+
+    raised = False
+    try:
+        srv._imap_connect()
+    except Exception:
+        raised = True
+    assert raised, "starttls failure must propagate"
+    assert captured.get("shutdown_calls", 0) == 1, (
+        f"shutdown() must be called once on MCP IMAP STARTTLS failure. Got {captured.get('shutdown_calls')}")
+
+
+def _cfg_smtp(security):
+    return {
+        "smtp_host": "smtp.example.com",
+        "smtp_port": 587 if security == "starttls" else 465,
+        "smtp_security": security, "smtp_user": "user@example.com",
+        "smtp_password": "wrong", "account_name": "test",
+    }
+
+
+def test_mcp_smtp_connect_closes_on_login_failure(monkeypatch):
+    import mcp_servers.email_server as srv
+
+    captured = {}
+    conn = MagicMock()
+    conn.close = MagicMock(side_effect=lambda: captured.__setitem__(
+        "close_calls", captured.get("close_calls", 0) + 1))
+    conn.login = MagicMock(side_effect=Exception("SMTP auth failed"))
+    monkeypatch.setattr(srv, "_load_config", lambda *a, **kw: _cfg_smtp("ssl"))
+    monkeypatch.setattr(srv, "_smtp_ready", lambda cfg: True)
+    monkeypatch.setattr(srv.smtplib, "SMTP_SSL", lambda *a, **kw: conn)
+
+    raised = False
+    try:
+        srv._smtp_connect()
+    except Exception:
+        raised = True
+    assert raised, "login failure must propagate"
+    assert captured.get("close_calls", 0) == 1, (
+        f"close() must be called once on MCP SMTP login failure. Got {captured.get('close_calls')}")
+
+
+def test_mcp_smtp_connect_closes_on_starttls_failure(monkeypatch):
+    import mcp_servers.email_server as srv
+
+    captured = {}
+    conn = MagicMock()
+    conn.close = MagicMock(side_effect=lambda: captured.__setitem__(
+        "close_calls", captured.get("close_calls", 0) + 1))
+    conn.starttls = MagicMock(side_effect=Exception("STARTTLS rejected"))
+    monkeypatch.setattr(srv, "_load_config", lambda *a, **kw: _cfg_smtp("starttls"))
+    monkeypatch.setattr(srv, "_smtp_ready", lambda cfg: True)
+    monkeypatch.setattr(srv.smtplib, "SMTP", lambda *a, **kw: conn)
+
+    raised = False
+    try:
+        srv._smtp_connect()
+    except Exception:
+        raised = True
+    assert raised, "starttls failure must propagate"
+    assert captured.get("close_calls", 0) == 1, (
+        f"close() must be called once on MCP SMTP STARTTLS failure. Got {captured.get('close_calls')}")

tests/test_memory_extraction_parse.py

@@ -0,0 +1,36 @@
+"""_parse_extraction_json must survive reasoning-model noise.
+
+The extraction model wraps its JSON array in <think> blocks, ```json fences,
+or leading/trailing prose. The helper strips that noise and slices the array
+unconditionally — a reply that starts with '[' can still carry trailing
+commentary like "[...] Done!" that would otherwise break json.loads.
+"""
+
+from services.memory.memory_extractor import _parse_extraction_json
+
+
+def test_think_prefixed_array_parses_to_one_fact():
+    raw = '<think>reasoning...</think>\n[{"text": "x", "category": "fact"}]'
+    assert _parse_extraction_json(raw) == [{"text": "x", "category": "fact"}]
+
+
+def test_fenced_json_block_parses():
+    raw = '```json\n[{"text": "x", "category": "fact"}]\n```'
+    assert _parse_extraction_json(raw) == [{"text": "x", "category": "fact"}]
+
+
+def test_leading_prose_before_array_parses():
+    raw = 'Here are the durable facts:\n[{"text": "x", "category": "fact"}]'
+    assert _parse_extraction_json(raw) == [{"text": "x", "category": "fact"}]
+
+
+def test_trailing_commentary_after_array_parses():
+    # Exercises the unconditional slice: text starts with '[' but has trailing
+    # commentary that the old `text[0] != "["` guard skipped, breaking json.loads.
+    raw = '[{"text": "x", "category": "fact"}] Done!'
+    assert _parse_extraction_json(raw) == [{"text": "x", "category": "fact"}]
+
+
+def test_malformed_no_array_returns_empty():
+    assert _parse_extraction_json("no array here, sorry") == []
+    assert _parse_extraction_json("") == []

tests/test_model_routes.py

@@ -10,6 +10,7 @@ from types import SimpleNamespace
 
 import httpx
 import pytest
+from fastapi import HTTPException
 
 from tests.helpers.import_state import clear_fake_endpoint_resolver_modules, preserve_import_state
 
@@ -1273,6 +1274,24 @@ def test_background_refresh_failure_keeps_existing_cached_models(monkeypatch):
     assert json.loads(ep.cached_models) == ["cached-model"]
 
 
+def test_api_models_auth_gate_fails_closed_on_unexpected_error(monkeypatch):
+    """A non-HTTPException raised while checking auth must yield 500, not a
+    silent pass-through that leaks the model list to an unauthenticated caller."""
+    router = model_routes.setup_model_routes(model_discovery=None)
+
+    monkeypatch.setattr(model_routes, "_auth_disabled", lambda: (_ for _ in ()).throw(RuntimeError("boom")))
+
+    request = SimpleNamespace(
+        state=SimpleNamespace(current_user=None),
+        app=SimpleNamespace(state=SimpleNamespace(auth_manager=SimpleNamespace(is_configured=True))),
+    )
+
+    with pytest.raises(HTTPException) as exc:
+        _route_endpoint(router, "/api/models")(request)
+
+    assert exc.value.status_code == 500
+
+
 def test_llm_core_list_model_ids_uses_cached_configured_proxy(monkeypatch):
     ep = _route_ep(
         "proxy",

tests/test_preset_atomic_save.py

@@ -0,0 +1,43 @@
+"""Regression: PresetManager.save() must persist presets atomically.
+
+save() used a plain open("w") + json.dump, which truncates presets.json before
+writing the new content. A crash / power loss / serialization error mid-write
+leaves the file truncated or empty — the user loses every saved preset. The
+save now goes through core.atomic_io.atomic_write_json (tmp file + os.replace),
+which the rest of the codebase already uses for JSON state files.
+"""
+import inspect
+import json
+
+from src.preset_manager import PresetManager
+
+
+class _Unserializable:
+    """json.dump cannot serialize this — stands in for a mid-write failure."""
+
+
+def test_save_uses_atomic_write_json():
+    src = inspect.getsource(PresetManager.save)
+    assert "atomic_write_json" in src, "save() must persist via atomic_write_json"
+    assert "open(" not in src, "save() must not write presets.json with a plain open('w')"
+
+
+def test_failed_save_does_not_truncate_existing_file(tmp_path):
+    mgr = PresetManager(str(tmp_path))
+    assert mgr.save({"custom": {"name": "keep"}}) is True
+    before = (tmp_path / "presets.json").read_text(encoding="utf-8")
+
+    # A payload that cannot be serialized must not clobber the good file.
+    assert mgr.save({"custom": {"obj": _Unserializable()}}) is False
+
+    after = (tmp_path / "presets.json").read_text(encoding="utf-8")
+    assert after == before
+    assert json.loads(after) == {"custom": {"name": "keep"}}
+
+
+def test_save_round_trip(tmp_path):
+    mgr = PresetManager(str(tmp_path))
+    assert mgr.save({"custom": {"name": "X", "temperature": 0.5}}) is True
+
+    reloaded = PresetManager(str(tmp_path))
+    assert reloaded.presets["custom"]["name"] == "X"

tests/test_preset_expand_owner_scope.py

@@ -0,0 +1,86 @@
+"""Route-level owner-scope test for POST /api/presets/expand.
+
+`expand_character_prompt` resolves a model endpoint to run its LLM call. It must
+scope that lookup to the calling user, otherwise it can resolve another owner's
+ModelEndpoint (and its decrypted api_key) in a multi-user deployment. See #2283.
+"""
+
+import asyncio
+from types import SimpleNamespace
+from unittest.mock import MagicMock
+
+from routes.preset_routes import setup_preset_routes
+
+
+class _FakeRequest:
+    """Minimal stand-in: an async ``json()`` body plus a ``state`` namespace."""
+
+    def __init__(self, body, **state):
+        self._body = body
+        self.state = SimpleNamespace(**state)
+
+    async def json(self):
+        return self._body
+
+
+def _expand_endpoint():
+    router = setup_preset_routes(MagicMock())
+    for route in router.routes:
+        if getattr(route, "path", "") == "/api/presets/expand" and "POST" in getattr(route, "methods", set()):
+            return route.endpoint
+    raise AssertionError("POST /api/presets/expand route not registered")
+
+
+def _patch_model_pipeline(monkeypatch):
+    """Capture the owner passed to _resolve_model and stub the LLM call."""
+    seen = {}
+
+    def fake_resolve_model(spec, owner=None):
+        seen["spec"] = spec
+        seen["owner"] = owner
+        return ("http://endpoint.local/v1", "test-model", {})
+
+    async def fake_llm_call_async(url, model, messages, **kwargs):
+        return "  expanded prompt  "
+
+    monkeypatch.setattr("src.ai_interaction._resolve_model", fake_resolve_model)
+    monkeypatch.setattr("src.llm_core.llm_call_async", fake_llm_call_async)
+    return seen
+
+
+def test_expand_scopes_model_resolution_to_cookie_user(monkeypatch):
+    seen = _patch_model_pipeline(monkeypatch)
+    endpoint = _expand_endpoint()
+
+    req = _FakeRequest({"name": "Pirate", "prompt": "talks like a pirate", "model": "test-model"},
+                       current_user="alice")
+    result = asyncio.run(endpoint(req))
+
+    assert seen["owner"] == "alice"
+    assert seen["spec"] == "test-model"
+    assert result == {"success": True, "prompt": "expanded prompt"}
+
+
+def test_expand_attributes_bearer_token_to_its_owner(monkeypatch):
+    # effective_user (not get_current_user) resolves a bearer ody_ caller to the
+    # token's real owner instead of the sandbox "api" pseudo-user.
+    seen = _patch_model_pipeline(monkeypatch)
+    endpoint = _expand_endpoint()
+
+    req = _FakeRequest({"name": "Pirate", "model": ""},
+                       current_user="api", api_token=True, api_token_owner="bob")
+    asyncio.run(endpoint(req))
+
+    assert seen["owner"] == "bob"
+
+
+def test_expand_short_circuits_without_input(monkeypatch):
+    seen = _patch_model_pipeline(monkeypatch)
+    endpoint = _expand_endpoint()
+
+    req = _FakeRequest({}, current_user="alice")
+    result = asyncio.run(endpoint(req))
+
+    # Nothing to expand: no model resolution attempted.
+    assert result["success"] is False
+    assert "owner" not in seen

tests/test_security_regressions.py

@@ -233,6 +233,43 @@ def test_q_empty_input():
     assert _q(None) == '""'
 
 
+# ── provider auth error normalization ──────────────────────────
+
+def _import_friendly_email_auth_error():
+    sys.modules.pop("routes.email_helpers", None)
+    from routes.email_helpers import _friendly_email_auth_error  # noqa: WPS433
+    return _friendly_email_auth_error
+
+
+def test_outlook_smtp_basic_auth_error_is_actionable():
+    normalize = _import_friendly_email_auth_error()
+    msg = normalize(
+        "SMTP",
+        "smtp.office365.com",
+        "(535, b'5.7.139 Authentication unsuccessful, basic authentication is disabled.')",
+    )
+
+    assert "Microsoft no longer accepts normal mailbox passwords" in msg
+    assert "OAuth/Graph" in msg
+    assert "535" not in msg
+
+
+def test_outlook_imap_authenticate_failed_is_actionable():
+    normalize = _import_friendly_email_auth_error()
+    msg = normalize("IMAP", "outlook.office365.com", "b'AUTHENTICATE failed.'")
+
+    assert "Microsoft no longer accepts normal mailbox passwords" in msg
+    assert "Outlook/Office 365" in msg
+
+
+def test_generic_auth_error_still_passes_through_truncated():
+    normalize = _import_friendly_email_auth_error()
+    msg = normalize("IMAP", "imap.example.com", "bad credentials " + ("x" * 300))
+
+    assert msg.startswith("bad credentials")
+    assert len(msg) == 200
+
+
 # ── compose-upload path traversal block ─────────────────────────
 
 @pytest.mark.parametrize(

tests/test_teacher_audit_owner_scope.py

@@ -0,0 +1,64 @@
+"""Owner-scope tests for the remaining _resolve_model call sites.
+
+Both the teacher-escalation path and the skill-audit teacher resolution map a
+model spec to an endpoint (and its decrypted api_key). Like /presets/expand,
+that lookup must be scoped to the calling user, otherwise it can resolve another
+owner's ModelEndpoint in a multi-user deployment. See #2283.
+"""
+
+import asyncio
+
+import src.teacher_escalation as teacher_escalation
+import routes.skills_routes as skills_routes
+
+
+def test_call_teacher_scopes_model_resolution_to_owner(monkeypatch):
+    seen = {}
+
+    def fake_resolve_model(spec, owner=None):
+        seen["spec"] = spec
+        seen["owner"] = owner
+        return ("http://endpoint.local/v1", "teacher-model", {})
+
+    async def fake_llm_call_async(url, model, messages, **kwargs):
+        return "teacher reply"
+
+    monkeypatch.setattr("src.ai_interaction._resolve_model", fake_resolve_model)
+    monkeypatch.setattr("src.ai_interaction._TEACHER_SYSTEM_PROMPT", "sys", raising=False)
+    monkeypatch.setattr("src.llm_core.llm_call_async", fake_llm_call_async)
+
+    result = asyncio.run(
+        teacher_escalation._call_teacher("teacher-model", "prompt", owner="alice")
+    )
+
+    assert result == "teacher reply"
+    assert seen["owner"] == "alice"
+    assert seen["spec"] == "teacher-model"
+
+
+def test_audit_teacher_resolution_scoped_to_owner(monkeypatch):
+    seen = {}
+
+    def fake_resolve_endpoint(role, owner=None):
+        return ("http://worker.local/v1", "worker-model", {})
+
+    def fake_get_setting(key, default=None):
+        return {"teacher_enabled": True, "teacher_model": "teacher-model"}.get(key, default)
+
+    def fake_resolve_model(spec, owner=None):
+        seen["spec"] = spec
+        seen["owner"] = owner
+        return ("http://endpoint.local/v1", "teacher-model", {})
+
+    monkeypatch.setattr("src.endpoint_resolver.resolve_endpoint", fake_resolve_endpoint)
+    monkeypatch.setattr("src.settings.get_setting", fake_get_setting)
+    monkeypatch.setattr("src.ai_interaction._resolve_model", fake_resolve_model)
+    # list_model_ids is best-effort; force it to no-op so the worker model passes through.
+    monkeypatch.setattr("src.llm_core.list_model_ids", lambda url, headers=None: [])
+
+    url, model, headers, teacher = skills_routes._resolve_audit_models(owner="alice")
+
+    assert (url, model) == ("http://worker.local/v1", "worker-model")
+    assert teacher == ("http://endpoint.local/v1", "teacher-model", {})
+    assert seen["owner"] == "alice"
+    assert seen["spec"] == "teacher-model"