Merge branch 'upstream/dev' into fix/pr-6742-email-export

tests/css_snapshot/baseline.json

@@ -1,10 +1,10 @@
 {
- "digest": "9868d50a542b7ad1",
+ "digest": "f7e60e4a567c46dc",
  "elements": {
   "app-shell": {
    "app-loader": "f04ad5bf6312d53f",
    "attach-strip": "a6474951e2481214",
-   "body": "58df17f0afa190b9",
+   "body": "4956f449e6a30e93",
    "chat-container": "b00a7c6be4e107b1",
    "chat-context-pill": "dd905b87245a9a71",
    "chat-history": "e65bdc51f4d1675f",
@@ -23,8 +23,8 @@
    "input-icon-btn": "3d1c773acd84a825",
    "memory-close-btn": "c3f4ba99098a6e4b",
    "memory-list": "da9a37a3de3c1a31",
-   "memory-modal": "5efc81537eab4526",
-   "memory-modal-content": "3cadc8581361436e",
+   "memory-modal": "e2f97b123dc9bc55",
+   "memory-modal-content": "5c5fe055c711e642",
    "memory-modal-header": "a8c822fe8c6d20e7",
    "memory-search-input": "8b0fbb33401750d4",
    "memory-toolbar-btn": "9f985efa7c896f90",
@@ -42,7 +42,7 @@
    "rail-new-chat": "f8f1c2f6be426ab9",
    "reasoning-effort-btn": "2a07cdd8ec2b4d26",
    "rename-session-modal": "9345624780009d14",
-   "root": "820190c144fda419",
+   "root": "a9f7ace2c1cefb24",
    "save-custom-preset": "443da70ef5fe9b88",
    "scroll-bottom-btn": "24602d72e2949991",
    "search-input": "53b94f3e5fdb384d",
@@ -54,7 +54,7 @@
    "settings-btn-sm": "c31bcb9bdfbae10d",
    "settings-fallbacks": "e339280590935fe3",
    "settings-logs-tab": "e470140373597143",
-   "settings-modal": "5efc81537eab4526",
+   "settings-modal": "e2f97b123dc9bc55",
    "settings-nav-search": "f4484b2980f2ccc5",
    "settings-row": "de568724268cec6e",
    "settings-select": "a96e8430b0ec6668",
@@ -69,7 +69,7 @@
    "theme-grid": "4d4b2aeb8952cc6f",
    "theme-io-btn": "859f222c79a8c721",
    "theme-modal": "0a65ca916656cdbb",
-   "theme-popup": "3aa65e6f313bbc65",
+   "theme-popup": "fb86cc5f81d0aaa6",
    "theme-tabs": "0cc4987f39876dce",
    "toast": "c60c6a60d4982005",
    "tool-indicator": "0dee45eaca8b703a",
@@ -83,7 +83,7 @@
    "#agent-indicator": "09a1b8d94b1487ee",
    "#agent-indicator.active": "67e2ad3fa48acd3b",
    "#compare-model-overlay .modal-body": "0b1ee357313551b9",
-   "#compare-model-overlay .modal-content": "56883147d181bd9c",
+   "#compare-model-overlay .modal-content": "9b2145292f439a88",
    "#cookbook-modal .cookbook-body": "20bd8a7268cee788",
    "#cookbook-modal .hwfit-cached-list.cookbook-serve-models-just-loaded > .doclib-card": "48f6787a3df3f19e",
    "#cookbook-modal .modal-content": "0c4e1e686df0990e",
@@ -101,7 +101,7 @@
    "#email-lib-modal .email-lib-sync-status": "6d1a0374a16cf7f4",
    "#email-lib-modal .email-reader-atts": "c5d68ecda3c14bb8",
    "#email-lib-modal .email-reader-header": "85afca11c8cbd38e",
-   "#email-lib-modal .modal-content": "d8b8fff38c613c95",
+   "#email-lib-modal .modal-content": "262753f287410717",
    "#email-lib-settings-btn": "9ae15bf04b6a22ce",
    "#email-unread-dot.sidebar-notif-dot": "c80f1e03592983b6",
    "#export-compact-btn": "7d0325a38e51134b",
@@ -686,44 +686,44 @@
  },
  "variants": {
   "app-shell": {
-   "desktop-dark-comfortable": "65b23a35b5a34126",
-   "desktop-dark-compact": "1bac3a0aa4467555",
-   "desktop-dark-spacious": "944583b1854fecbb",
-   "desktop-light-comfortable": "d279432f8c120b58",
-   "desktop-light-compact": "aa768c34ba3c7abc",
-   "desktop-light-spacious": "b74f53e95bfa1d82",
-   "laptop-dark-comfortable": "d98d16f3d60ed275",
-   "laptop-dark-compact": "c3eaf3c4278a0120",
-   "laptop-dark-spacious": "a085fe0f7d9eb66c",
-   "laptop-light-comfortable": "0d7c64f324bebe87",
-   "laptop-light-compact": "da50bf03b0f1be58",
-   "laptop-light-spacious": "a6789fd05a66bf5a",
-   "phone-dark-comfortable": "21b94f88da549eb5",
-   "phone-dark-compact": "b9aa7def2b85414e",
-   "phone-dark-spacious": "21b94f88da549eb5",
-   "phone-light-comfortable": "4a0cf45db775ea7d",
-   "phone-light-compact": "f7fbcb94d99c8730",
-   "phone-light-spacious": "4a0cf45db775ea7d",
-   "tablet-dark-comfortable": "e18256ce58398f11",
-   "tablet-dark-compact": "3cf3e52a10313d44",
-   "tablet-dark-spacious": "e18256ce58398f11",
-   "tablet-light-comfortable": "c2a45155c391f26c",
-   "tablet-light-compact": "9fa7dc03dd02b723",
-   "tablet-light-spacious": "c2a45155c391f26c"
+   "desktop-dark-comfortable": "0c9123117135e362",
+   "desktop-dark-compact": "c694f5d5118ac0e2",
+   "desktop-dark-spacious": "90a67613e8cfdf5d",
+   "desktop-light-comfortable": "17727a9945ccc77b",
+   "desktop-light-compact": "2a887c30d231e357",
+   "desktop-light-spacious": "94b4666734b87625",
+   "laptop-dark-comfortable": "26d334b139b653f2",
+   "laptop-dark-compact": "fea30bad6824357d",
+   "laptop-dark-spacious": "4fdb28deaaaec870",
+   "laptop-light-comfortable": "c7fc986a7e70aac8",
+   "laptop-light-compact": "147645431229ddff",
+   "laptop-light-spacious": "d513c285d30c8289",
+   "phone-dark-comfortable": "551761344bec8d09",
+   "phone-dark-compact": "293d66518c3ec994",
+   "phone-dark-spacious": "551761344bec8d09",
+   "phone-light-comfortable": "cb41acd3f9af3578",
+   "phone-light-compact": "dbef6e9e373e9c10",
+   "phone-light-spacious": "cb41acd3f9af3578",
+   "tablet-dark-comfortable": "0f682621b4ead393",
+   "tablet-dark-compact": "e6f3fece5e716a2c",
+   "tablet-dark-spacious": "0f682621b4ead393",
+   "tablet-light-comfortable": "6e398df9ca69c58b",
+   "tablet-light-compact": "dca75981c8675bfa",
+   "tablet-light-spacious": "6e398df9ca69c58b"
   },
   "bench": {
-   "desktop-dark-comfortable": "f9fa9d54db5ad45a",
-   "desktop-dark-compact": "7603a804cb2e23a2",
-   "desktop-dark-spacious": "b0c69306349fc078",
-   "desktop-light-comfortable": "d3e98c4c1e90686f",
-   "desktop-light-compact": "a04ae70088bc8b35",
-   "desktop-light-spacious": "4aa01a125f541429",
-   "laptop-dark-comfortable": "025e60ebe84e0f98",
-   "laptop-dark-compact": "53d6555bb7a6b5bb",
-   "laptop-dark-spacious": "c124d9ce1633d027",
-   "laptop-light-comfortable": "e8328c45f5127bb0",
-   "laptop-light-compact": "dc2da3534534c888",
-   "laptop-light-spacious": "abe5483724c987e0",
+   "desktop-dark-comfortable": "a9709cb47eec7544",
+   "desktop-dark-compact": "bfab96cb72fd6faf",
+   "desktop-dark-spacious": "25469178b443428a",
+   "desktop-light-comfortable": "2166794d643fae4c",
+   "desktop-light-compact": "81d8ab64407b1d9a",
+   "desktop-light-spacious": "d6d19f33476665af",
+   "laptop-dark-comfortable": "57169e64e1fb5324",
+   "laptop-dark-compact": "6ac903cd8a711bcf",
+   "laptop-dark-spacious": "a47535332e9c5de7",
+   "laptop-light-comfortable": "1e2b381de3556fac",
+   "laptop-light-compact": "6dbfb465121949c1",
+   "laptop-light-spacious": "1b8bfcd4bc4ed214",
    "phone-dark-comfortable": "67ca3c78e6b9495a",
    "phone-dark-compact": "ad6f3a9f5c8d81cd",
    "phone-dark-spacious": "1e6f66e56519ba5d",

tests/test_manage_tasks_admin_action_policy.py

@@ -0,0 +1,289 @@
+"""manage_tasks must apply the admin-only task-action policy.
+
+``POST /api/tasks`` (routes/task/task_routes.py) and the scheduler
+(src/task_scheduler.py) both refuse ``action`` tasks whose action is in
+``ADMIN_ONLY_TASK_ACTIONS`` (run_local, run_script, ssh_command,
+cookbook_serve) for owners without admin task privileges — those actions run
+``subprocess.run(shell=True)`` or SSH with no sandbox. The model-facing
+``do_manage_tasks`` applied neither check, so an agent could store such a task
+on create/edit and resume/run it; the scheduler would only pause it at
+execution time. Regression for issue #6021: the tool now enforces the same
+policy on create, edit, resume and run, and keeps allowing admins and the
+non-privileged actions.
+"""
+
+import json
+import tempfile
+
+import pytest
+from sqlalchemy import create_engine
+from sqlalchemy.orm import 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
+from core.database import ScheduledTask
+import src.task_action_policy as policy_mod
+import src.tools.system as system_mod
+from src.tools.system import do_manage_tasks
+
+_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
+_ENGINE = create_engine(
+    f"sqlite:///{_TMPDB.name}",
+    connect_args={"check_same_thread": False},
+    poolclass=NullPool,
+)
+cdb.Base.metadata.create_all(_ENGINE)
+_TS = sessionmaker(bind=_ENGINE, autoflush=False, autocommit=False)
+
+
+@pytest.fixture(autouse=True)
+def _bind_session_factory(monkeypatch):
+    # do_manage_tasks does `from core.database import SessionLocal` at call
+    # time. Bind it per test (not at import) so this file stays
+    # order-independent from sibling files that point it at their own DB.
+    monkeypatch.setattr(cdb, "SessionLocal", _TS)
+
+
+@pytest.fixture
+def privileges(monkeypatch):
+    """Drive owner_has_admin_task_privileges from a plain dict."""
+    admins = {"root"}
+    monkeypatch.setattr(
+        policy_mod, "owner_has_admin_task_privileges",
+        lambda owner: owner in admins,
+    )
+    return admins
+
+
+def _seed(task_id, owner, task_type="llm", action=None, status="active"):
+    db = _TS()
+    try:
+        db.add(ScheduledTask(
+            id=task_id, owner=owner, name=task_id, prompt="original",
+            task_type=task_type, action=action, trigger_type="webhook",
+            status=status, output_target="session",
+        ))
+        db.commit()
+    finally:
+        db.close()
+
+
+def _get(task_id):
+    db = _TS()
+    try:
+        return db.query(ScheduledTask).filter(ScheduledTask.id == task_id).first()
+    finally:
+        db.close()
+
+
+def _count(owner):
+    db = _TS()
+    try:
+        return db.query(ScheduledTask).filter(ScheduledTask.owner == owner).count()
+    finally:
+        db.close()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("action_name", sorted(policy_mod.ADMIN_ONLY_TASK_ACTIONS))
+async def test_create_admin_only_action_denied_for_non_admin(privileges, action_name):
+    before = _count("alice")
+    out = await do_manage_tasks(json.dumps({
+        "action": "create", "task_type": "action", "action_name": action_name,
+        "name": "pwn", "schedule": "daily", "scheduled_time": "09:00",
+    }), owner="alice")
+    assert out["exit_code"] == 1, out
+    assert "requires admin privileges" in out["error"]
+    assert _count("alice") == before
+
+
+@pytest.mark.asyncio
+async def test_create_admin_only_action_allowed_for_admin(privileges):
+    out = await do_manage_tasks(json.dumps({
+        "action": "create", "task_type": "action", "action_name": "run_local",
+        "name": "ok", "schedule": "daily", "scheduled_time": "09:00",
+    }), owner="root")
+    assert out["exit_code"] == 0, out
+    assert _get(out["task_id"]).action == "run_local"
+
+
+@pytest.mark.asyncio
+async def test_create_regular_action_allowed_for_non_admin(privileges):
+    out = await do_manage_tasks(json.dumps({
+        "action": "create", "task_type": "action", "action_name": "tidy_sessions",
+        "name": "ok", "schedule": "daily", "scheduled_time": "09:00",
+    }), owner="alice")
+    assert out["exit_code"] == 0, out
+
+
+@pytest.mark.asyncio
+async def test_create_llm_task_allowed_for_non_admin(privileges):
+    out = await do_manage_tasks(json.dumps({
+        "action": "create", "task_type": "llm", "prompt": "summarize",
+        "schedule": "daily", "scheduled_time": "09:00",
+    }), owner="alice")
+    assert out["exit_code"] == 0, out
+
+
+@pytest.mark.asyncio
+async def test_edit_cannot_turn_llm_task_into_shell_action(privileges):
+    _seed("alice-llm", "alice")
+    out = await do_manage_tasks(json.dumps({
+        "action": "edit", "task_id": "alice-llm",
+        "task_type": "action", "action_name": "ssh_command",
+    }), owner="alice")
+    assert out["exit_code"] == 1, out
+    assert "requires admin privileges" in out["error"]
+    task = _get("alice-llm")
+    assert task.task_type == "llm" and task.action is None
+
+
+@pytest.mark.asyncio
+async def test_edit_cannot_swap_action_name_to_admin_only(privileges):
+    _seed("alice-action", "alice", task_type="action", action="tidy_sessions")
+    out = await do_manage_tasks(json.dumps({
+        "action": "edit", "task_id": "alice-action", "action_name": "run_script",
+    }), owner="alice")
+    assert out["exit_code"] == 1, out
+    assert _get("alice-action").action == "tidy_sessions"
+
+
+@pytest.mark.asyncio
+async def test_edit_of_unrelated_field_on_admin_only_task_denied_for_non_admin(privileges):
+    # A task that slipped in before the policy (or via demotion of its owner)
+    # cannot be kept alive by a non-admin: the route rejects the same edit.
+    _seed("alice-legacy", "alice", task_type="action", action="run_local")
+    out = await do_manage_tasks(json.dumps({
+        "action": "edit", "task_id": "alice-legacy", "name": "renamed",
+    }), owner="alice")
+    assert out["exit_code"] == 1, out
+    assert _get("alice-legacy").name == "alice-legacy"
+
+
+@pytest.mark.asyncio
+async def test_resume_admin_only_task_denied_for_non_admin(privileges):
+    _seed("alice-paused", "alice", task_type="action", action="run_local", status="paused")
+    out = await do_manage_tasks(json.dumps({
+        "action": "resume", "task_id": "alice-paused",
+    }), owner="alice")
+    assert out["exit_code"] == 1, out
+    assert _get("alice-paused").status == "paused"
+
+
+@pytest.mark.asyncio
+async def test_pause_admin_only_task_still_allowed(privileges):
+    # Narrowing is always fine: a non-admin may pause a task they own.
+    _seed("alice-active", "alice", task_type="action", action="run_local")
+    out = await do_manage_tasks(json.dumps({
+        "action": "pause", "task_id": "alice-active",
+    }), owner="alice")
+    assert out["exit_code"] == 0, out
+    assert _get("alice-active").status == "paused"
+
+
+@pytest.mark.asyncio
+async def test_run_admin_only_task_denied_before_scheduler(privileges, monkeypatch):
+    _seed("alice-run", "alice", task_type="action", action="ssh_command")
+    calls = []
+
+    class _Sched:
+        async def run_task_now(self, task_id):
+            calls.append(task_id)
+            return True
+
+    import src.event_bus as event_bus
+    monkeypatch.setattr(event_bus, "get_task_scheduler", lambda: _Sched())
+    out = await do_manage_tasks(json.dumps({
+        "action": "run", "task_id": "alice-run",
+    }), owner="alice")
+    assert out["exit_code"] == 1, out
+    assert calls == []
+
+
+@pytest.mark.asyncio
+async def test_run_admin_only_task_allowed_for_admin(privileges, monkeypatch):
+    _seed("root-run", "root", task_type="action", action="ssh_command")
+    calls = []
+
+    class _Sched:
+        async def run_task_now(self, task_id):
+            calls.append(task_id)
+            return True
+
+    import src.event_bus as event_bus
+    monkeypatch.setattr(event_bus, "get_task_scheduler", lambda: _Sched())
+    out = await do_manage_tasks(json.dumps({
+        "action": "run", "task_id": "root-run",
+    }), owner="root")
+    assert out["exit_code"] == 0, out
+    assert calls == ["root-run"]
+
+
+def test_policy_helper_uses_shared_constants(privileges):
+    # The tool must consult the shared policy module, not a private copy of
+    # the action names, so a new admin-only action is covered everywhere.
+    assert system_mod._admin_task_action_denied("alice", "action", "run_local") is not None
+    assert system_mod._admin_task_action_denied("root", "action", "run_local") is None
+    assert system_mod._admin_task_action_denied("alice", "action", "tidy_sessions") is None
+    assert system_mod._admin_task_action_denied("alice", "llm", "run_local") is None
+
+
+@pytest.mark.asyncio
+async def test_run_admin_only_task_by_name_denied_for_non_admin(privileges, monkeypatch):
+    _seed("alice-run-name", "alice", task_type="action", action="ssh_command")
+    calls = []
+
+    class _Sched:
+        async def run_task_now(self, task_id):
+            calls.append(task_id)
+            return True
+
+    import src.event_bus as event_bus
+    monkeypatch.setattr(event_bus, "get_task_scheduler", lambda: _Sched())
+    out = await do_manage_tasks(json.dumps({
+        "action": "run", "name": "alice-run-name",
+    }), owner="alice")
+    assert out["exit_code"] == 1, out
+    assert "requires admin privileges" in out["error"]
+    assert calls == []
+
+
+@pytest.mark.asyncio
+async def test_create_admin_only_action_seals_authority_for_admin(privileges):
+    out = await do_manage_tasks(json.dumps({
+        "action": "create", "task_type": "action", "action_name": "run_local",
+        "name": "sealed-task", "schedule": "daily", "scheduled_time": "09:00",
+    }), owner="root")
+    assert out["exit_code"] == 0, out
+    task = _get(out["task_id"])
+    assert task.action == "run_local"
+    assert task.request_authority_json is not None
+
+
+def test_real_owner_has_admin_task_privileges_branches(monkeypatch):
+    """Verify owner_has_admin_task_privileges without mocking."""
+    # Auth disabled branch
+    monkeypatch.setenv("AUTH_ENABLED", "false")
+    assert policy_mod.owner_has_admin_task_privileges("alice") is True
+    assert policy_mod.owner_has_admin_task_privileges(None) is True
+
+    # Auth enabled branch with configured AuthManager
+    monkeypatch.delenv("AUTH_ENABLED", raising=False)
+
+    class FakeAuth:
+        is_configured = True
+
+        def is_admin(self, u):
+            return u == "admin"
+
+    import core.auth as c_auth
+    monkeypatch.setattr(c_auth, "AuthManager", lambda: FakeAuth())
+
+    assert policy_mod.owner_has_admin_task_privileges("admin") is True
+    assert policy_mod.owner_has_admin_task_privileges("alice") is False
+    assert policy_mod.owner_has_admin_task_privileges("internal-tool") is True
+    assert policy_mod.owner_has_admin_task_privileges(None) is False

tests/test_mcp_oauth.py

@@ -264,3 +264,231 @@ def test_launchers_export_the_port_they_serve_on():
         text = (_repo_root() / name).read_text(encoding="utf-8")
         assert uvicorn_flag in text, f"{name}: launcher no longer passes {uvicorn_flag}"
         assert export in text, f"{name}: serves on a port the app cannot read back"
+
+
+# ── Callback & Exchange Expired State Handling ───────────────────
+
+def test_oauth_callback_expired_state_returns_dedicated_expired_page(monkeypatch):
+    """An unknown or expired generic MCP OAuth state in the callback must return
+    the dedicated 'Authorization Expired' page instead of a bare 403 'Admin only'
+    or a 404 'Server not found'."""
+    from unittest.mock import MagicMock
+    from fastapi import FastAPI, HTTPException
+    from fastapi.testclient import TestClient
+    from routes.mcp import mcp_routes
+
+    # Simulate unauthenticated request that would fail admin check if invoked
+    def _deny_admin(request):
+        raise HTTPException(403, "Admin only")
+
+    monkeypatch.setattr(mcp_routes, "require_admin", _deny_admin)
+
+    # Empty DB / no matching server
+    class FakeQuery:
+        def filter(self, *a):
+            return self
+
+        def first(self):
+            return None
+
+    class FakeSession:
+        def query(self, *a):
+            return FakeQuery()
+
+        def close(self):
+            pass
+
+    monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: FakeSession())
+
+    app = FastAPI()
+    manager = MagicMock()
+    router = mcp_routes.setup_mcp_routes(manager)
+    app.include_router(router)
+    client = TestClient(app)
+
+    # Both code and state are provided, but state is not in _pending (expired/unknown)
+    resp = client.get("/api/mcp/oauth/callback?code=some-code&state=expired-state-123")
+    assert resp.status_code == 400
+    assert "Authorization Expired" in resp.text
+    assert (
+        "This authorization round has expired. Press Reconnect in Odysseus and try again."
+        in resp.text
+    )
+    # Ensure it did not return the bare 403 "Admin only"
+    assert "Admin only" not in resp.text
+
+
+def test_oauth_callback_active_pending_state_succeeds_without_admin(monkeypatch):
+    """An active pending generic MCP OAuth state resolves without requiring admin auth."""
+    from unittest.mock import MagicMock
+    from fastapi import FastAPI, HTTPException
+    from fastapi.testclient import TestClient
+    from routes.mcp import mcp_routes
+
+    # Admin check fails if invoked
+    def _deny_admin(request):
+        raise HTTPException(403, "Admin only")
+
+    monkeypatch.setattr(mcp_routes, "require_admin", _deny_admin)
+
+    app = FastAPI()
+    manager = MagicMock()
+    router = mcp_routes.setup_mcp_routes(manager)
+    app.include_router(router)
+    client = TestClient(app)
+
+    async def go():
+        fut = mcp_oauth.register_pending("pending-round-1")
+        resp = client.get("/api/mcp/oauth/callback?code=auth-code-xyz&state=pending-round-1")
+        assert resp.status_code == 200
+        assert "Authorization Successful" in resp.text
+        code, state = await asyncio.wait_for(fut, timeout=1)
+        assert code == "auth-code-xyz"
+        assert state == "pending-round-1"
+
+    asyncio.run(go())
+
+
+def test_oauth_callback_legacy_google_server_requires_admin(monkeypatch):
+    """Legacy Google OAuth callbacks (where state is server_id of a server with
+    oauth_config) still enforce require_admin."""
+    import json
+    from unittest.mock import MagicMock
+    from fastapi import FastAPI, HTTPException
+    from fastapi.testclient import TestClient
+    from routes.mcp import mcp_routes
+    from core.database import McpServer
+
+    # Server exists in DB with Google oauth_config
+    google_srv = McpServer(
+        id="google-srv-1",
+        name="Google Workspace",
+        oauth_config=json.dumps({"provider": "google", "keys_file": "k.json", "token_file": "t.json"}),
+    )
+
+    class FakeQuery:
+        def filter(self, *a):
+            return self
+
+        def first(self):
+            return google_srv
+
+    class FakeSession:
+        def query(self, *a):
+            return FakeQuery()
+
+        def close(self):
+            pass
+
+    monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: FakeSession())
+
+    # When require_admin denies, 403 is raised for the legacy Google path
+    def _deny_admin(request):
+        raise HTTPException(403, "Admin only")
+
+    monkeypatch.setattr(mcp_routes, "require_admin", _deny_admin)
+
+    app = FastAPI()
+    manager = MagicMock()
+    router = mcp_routes.setup_mcp_routes(manager)
+    app.include_router(router)
+    client = TestClient(app)
+
+    resp = client.get("/api/mcp/oauth/callback?code=code-123&state=google-srv-1")
+    assert resp.status_code == 403
+    assert "Admin only" in resp.text
+
+
+def test_oauth_exchange_expired_generic_state_returns_expired_page(monkeypatch):
+    """Remote paste-back flow for a generic MCP server returns the dedicated expired
+    page when the state is no longer pending, instead of 'No OAuth config.'."""
+    from unittest.mock import MagicMock
+    from fastapi import FastAPI
+    from fastapi.testclient import TestClient
+    from routes.mcp import mcp_routes
+    from core.database import McpServer
+
+    monkeypatch.setattr(mcp_routes, "require_admin", lambda r: None)
+
+    # Server exists in DB without oauth_config (generic MCP server)
+    generic_srv = McpServer(
+        id="http-srv-1",
+        name="Remote HTTP MCP",
+        oauth_config=None,
+    )
+
+    class FakeQuery:
+        def filter(self, *a):
+            return self
+
+        def first(self):
+            return generic_srv
+
+    class FakeSession:
+        def query(self, *a):
+            return FakeQuery()
+
+        def close(self):
+            pass
+
+    monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: FakeSession())
+
+    app = FastAPI()
+    manager = MagicMock()
+    router = mcp_routes.setup_mcp_routes(manager)
+    app.include_router(router)
+    client = TestClient(app)
+
+    resp = client.post(
+        "/api/mcp/oauth/exchange/http-srv-1",
+        data={"callback_url": "http://localhost:7000/api/mcp/oauth/callback?code=code-1&state=expired-state"},
+    )
+    assert resp.status_code == 400
+    assert "Authorization Expired" in resp.text
+    assert (
+        "This authorization round has expired. Press Reconnect in Odysseus and try again."
+        in resp.text
+    )
+    assert "No OAuth config." not in resp.text
+
+
+def test_oauth_callback_missing_state_returns_malformed_error_not_expired():
+    """Missing state parameter in callback must return a bad-request error and must
+    not be treated as an expired authorization round."""
+    from unittest.mock import MagicMock
+    from fastapi import FastAPI
+    from fastapi.testclient import TestClient
+    from routes.mcp import mcp_routes
+
+    app = FastAPI()
+    manager = MagicMock()
+    router = mcp_routes.setup_mcp_routes(manager)
+    app.include_router(router)
+    client = TestClient(app)
+
+    resp = client.get("/api/mcp/oauth/callback?code=some-code")
+    assert resp.status_code == 400
+    assert "No OAuth state parameter found in the callback URL." in resp.text
+    assert "Authorization Expired" not in resp.text
+    assert "This authorization round has expired" not in resp.text
+
+
+def test_oauth_callback_missing_code_returns_malformed_error_not_expired():
+    """Missing code parameter in callback must return a bad-request error and must
+    not be treated as an expired authorization round."""
+    from unittest.mock import MagicMock
+    from fastapi import FastAPI
+    from fastapi.testclient import TestClient
+    from routes.mcp import mcp_routes
+
+    app = FastAPI()
+    manager = MagicMock()
+    router = mcp_routes.setup_mcp_routes(manager)
+    app.include_router(router)
+    client = TestClient(app)
+
+    resp = client.get("/api/mcp/oauth/callback?state=some-state")
+    assert resp.status_code == 400
+    assert "No authorization code found in the URL." in resp.text
+    assert "Authorization Expired" not in resp.text
+    assert "This authorization round has expired" not in resp.text

tests/test_scheduler_research_settings_guard.py

@@ -0,0 +1,102 @@
+"""Guarded resolution of scheduled-research settings (#5049).
+
+The scheduler reads ``research_max_tokens`` /
+``research_extraction_timeout_seconds`` / ``research_extraction_concurrency``
+from settings.json with bare ``int()`` casts. A hand-edited value like
+``{"research_max_tokens": "invalid"}`` used to raise inside
+``TaskScheduler._execute_research_task`` and kill every scheduled research
+run until settings.json was fixed by hand.
+
+``_resolve_research_settings`` must fall back to the documented defaults
+(8192 / 90 / 3) for non-numeric input while preserving valid-value behavior
+(numeric strings still parse). ``src/research_handler.py`` already guards the
+same reads defensively; this is the scheduler's equivalent.
+"""
+
+from __future__ import annotations
+
+import json
+
+from src.task_scheduler import _resolve_research_settings
+
+
+def _fake_get_setting(values):
+    """A get_setting stand-in backed by a plain dict."""
+
+    def get(key, default=None):
+        return values.get(key, default)
+
+    return get
+
+
+def test_non_numeric_values_fall_back_to_documented_defaults():
+    get = _fake_get_setting(
+        {
+            "research_max_tokens": "invalid",
+            "research_extraction_timeout_seconds": "bogus",
+            "research_extraction_concurrency": "not-a-number",
+        }
+    )
+
+    assert _resolve_research_settings(get) == (8192, 90, 3)
+
+
+def test_none_values_fall_back_to_documented_defaults():
+    get = _fake_get_setting(
+        {
+            "research_max_tokens": None,
+            "research_extraction_timeout_seconds": None,
+            "research_extraction_concurrency": None,
+        }
+    )
+
+    assert _resolve_research_settings(get) == (8192, 90, 3)
+
+
+def test_numeric_strings_still_parse():
+    get = _fake_get_setting(
+        {
+            "research_max_tokens": "4096",
+            "research_extraction_timeout_seconds": "45",
+            "research_extraction_concurrency": "2",
+        }
+    )
+
+    assert _resolve_research_settings(get) == (4096, 45, 2)
+
+
+def test_missing_keys_keep_documented_defaults():
+    assert _resolve_research_settings(_fake_get_setting({})) == (8192, 90, 3)
+
+
+def test_valid_ints_pass_through():
+    get = _fake_get_setting(
+        {
+            "research_max_tokens": 2048,
+            "research_extraction_timeout_seconds": 120,
+            "research_extraction_concurrency": 6,
+        }
+    )
+
+    assert _resolve_research_settings(get) == (2048, 120, 6)
+
+
+def test_non_finite_json_numbers_fall_back_individually():
+    """JSON exponent overflow produces infinity, which int() cannot convert."""
+    keys = (
+        "research_max_tokens",
+        "research_extraction_timeout_seconds",
+        "research_extraction_concurrency",
+    )
+    valid = (4096, 45, 2)
+    defaults = (8192, 90, 3)
+
+    for non_finite in (json.loads("1e309"), json.loads("-1e309")):
+        for index, key in enumerate(keys):
+            values = dict(zip(keys, valid))
+            values[key] = non_finite
+            expected = list(valid)
+            expected[index] = defaults[index]
+            assert _resolve_research_settings(
+                _fake_get_setting(values)
+            ) == tuple(expected)