mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-16 01:35:36 -04:00
Compare commits
13 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| b0ade9964d | |||
| b010b99bd4 | |||
| b58af4267b | |||
| 8ff76f083c | |||
| 2196869c86 | |||
| dd2e23c9af | |||
| facc50cb0f | |||
| 074a1e6eff | |||
| 2fab378c6a | |||
| 5bafc30622 | |||
| d6d2e17214 | |||
| f4e8990635 | |||
| fc3a5e555e |
@@ -331,8 +331,8 @@ if AUTH_ENABLED:
|
||||
request.state.current_user = "internal-tool"
|
||||
request.state.api_token = False
|
||||
return await call_next(request)
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as _e:
|
||||
logger.warning("Internal tool auth header check failed", exc_info=_e)
|
||||
# Allow DIRECT localhost requests (internal service calls from
|
||||
# heartbeats etc.). Tunnel/proxy-forwarded requests are excluded by
|
||||
# _is_trusted_loopback so LOCALHOST_BYPASS can't be abused over a
|
||||
@@ -385,11 +385,10 @@ if AUTH_ENABLED:
|
||||
_db.close()
|
||||
try:
|
||||
await _asyncio.to_thread(_do)
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as _e:
|
||||
logger.debug("Failed to update token last_used_at", exc_info=_e)
|
||||
_asyncio.create_task(_touch_last_used(matched_id))
|
||||
# Keep bearer-token callers out of normal cookie/user
|
||||
# routes. API-aware routes can read api_token_owner.
|
||||
request.state.current_user = "api"
|
||||
request.state.api_token = True
|
||||
request.state.api_token_id = matched_id
|
||||
@@ -464,8 +463,8 @@ async def serve_generated_image(filename: str, request: Request):
|
||||
_db.close()
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as _e:
|
||||
logger.warning("Image ownership verification failed for %r", filename, exc_info=_e)
|
||||
ext = filename.rsplit('.', 1)[-1].lower()
|
||||
mime = {
|
||||
"png": "image/png", "jpg": "image/jpeg", "jpeg": "image/jpeg",
|
||||
|
||||
+17
-3
@@ -5,8 +5,9 @@ offers and pair to it, without duplicating any LLM logic.
|
||||
|
||||
Auth is enforced globally by AuthMiddleware (app.py), so reaching a handler here
|
||||
means the caller is authenticated by either a cookie session or a Bearer `ody_`
|
||||
API token. The read endpoints (ping/info/models) accept either; the pairing
|
||||
endpoints are admin-cookie only.
|
||||
API token. Ping/info accept either credential type, models requires a chat-
|
||||
scoped API token for bearer callers, and the pairing endpoints are admin-cookie
|
||||
only.
|
||||
|
||||
Pairing CSRF posture: minting happens ONLY on POST. The session cookie is
|
||||
SameSite=Lax (routes/auth_routes.py), which a browser does not send on a
|
||||
@@ -18,7 +19,7 @@ on a GET would be unsafe (Lax cookies ride top-level GET navigations), so GET
|
||||
|
||||
import html
|
||||
|
||||
from fastapi import APIRouter, Request
|
||||
from fastapi import APIRouter, HTTPException, Request
|
||||
from fastapi.responses import HTMLResponse
|
||||
|
||||
from core.middleware import require_admin
|
||||
@@ -52,6 +53,18 @@ def owner_can_see(row_owner, owner) -> bool:
|
||||
return row_owner is None or row_owner == owner
|
||||
|
||||
|
||||
def require_models_scope(request: Request) -> None:
|
||||
"""Require the companion chat scope for bearer-token model inventory."""
|
||||
if not getattr(request.state, "api_token", False):
|
||||
return
|
||||
scopes = getattr(request.state, "api_token_scopes", None) or []
|
||||
if isinstance(scopes, str):
|
||||
scopes = [scope.strip() for scope in scopes.split(",")]
|
||||
scope_set = {str(scope).strip() for scope in scopes if str(scope).strip()}
|
||||
if _pairing.COMPANION_SCOPE not in scope_set:
|
||||
raise HTTPException(403, "API token requires chat scope")
|
||||
|
||||
|
||||
def mint_pairing_token(owner: str, invalidate=None) -> tuple[str, str]:
|
||||
"""Mint a pairing token AND invalidate the auth middleware's in-memory token
|
||||
cache, so the new token is accepted on the very next request without a server
|
||||
@@ -103,6 +116,7 @@ def setup_companion_routes() -> APIRouter:
|
||||
rows -- the same rule as owner_filter. Read-only; never returns api_key
|
||||
material.
|
||||
"""
|
||||
require_models_scope(request)
|
||||
import json as _json
|
||||
|
||||
from core.database import SessionLocal, ModelEndpoint
|
||||
|
||||
+22
-2
@@ -2,12 +2,15 @@ import os
|
||||
import logging
|
||||
import sqlite3
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
from sqlalchemy import event, create_engine, Column, String, Text, Boolean, DateTime, Integer, ForeignKey, JSON, Index, func, text
|
||||
from sqlalchemy.engine import Engine
|
||||
from sqlalchemy.types import TypeDecorator
|
||||
from sqlalchemy.ext.declarative import declarative_base, declared_attr
|
||||
from sqlalchemy.orm import relationship, sessionmaker, backref
|
||||
|
||||
from src.runtime_paths import get_app_root
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Create base class for declarative models
|
||||
@@ -29,9 +32,26 @@ class TimestampMixin:
|
||||
def updated_at(cls):
|
||||
return Column(DateTime, default=utcnow_naive, onupdate=utcnow_naive, nullable=False)
|
||||
|
||||
# Get database URL from environment, default to SQLite in DATA_DIR
|
||||
# Ensure the writable data directory exists before SQLite connects.
|
||||
from src.constants import DATA_DIR, AUTH_FILE, MEMORY_FILE, USER_PREFS_FILE, SETTINGS_FILE
|
||||
DATABASE_URL = os.getenv("DATABASE_URL", f"sqlite:///{DATA_DIR}/app.db")
|
||||
Path(DATA_DIR).mkdir(parents=True, exist_ok=True)
|
||||
|
||||
|
||||
def _default_database_url() -> str:
|
||||
return f"sqlite:///{Path(DATA_DIR) / 'app.db'}"
|
||||
|
||||
|
||||
def _normalize_sqlite_url(url: str) -> str:
|
||||
if not url.startswith("sqlite:///"):
|
||||
return url
|
||||
db_path = url.replace("sqlite:///", "", 1)
|
||||
if db_path == ":memory:" or os.path.isabs(db_path):
|
||||
return url
|
||||
return f"sqlite:///{(Path(get_app_root()) / db_path).resolve().as_posix()}"
|
||||
|
||||
|
||||
# Get database URL from environment, default to SQLite in DATA_DIR
|
||||
DATABASE_URL = _normalize_sqlite_url(os.getenv("DATABASE_URL", _default_database_url()))
|
||||
|
||||
# Create engine
|
||||
engine = create_engine(
|
||||
|
||||
@@ -160,6 +160,8 @@ def setup_api_token_routes() -> APIRouter:
|
||||
payload = await request.json()
|
||||
except Exception:
|
||||
payload = {}
|
||||
if not isinstance(payload, dict):
|
||||
payload = {}
|
||||
with get_db_session() as db:
|
||||
token = db.query(ApiToken).filter(ApiToken.id == token_id).first()
|
||||
if not token:
|
||||
|
||||
@@ -14,7 +14,7 @@ from core.database import Session as DBSession, ModelEndpoint
|
||||
from src.llm_core import normalize_model_id
|
||||
from src.endpoint_resolver import normalize_base
|
||||
from src.context_compactor import maybe_compact, trim_for_context
|
||||
from src.auth_helpers import get_current_user
|
||||
from src.auth_helpers import effective_user
|
||||
from src.prompt_security import untrusted_context_message
|
||||
from routes.prefs_routes import _load_for_user as load_prefs_for_user
|
||||
|
||||
@@ -78,7 +78,7 @@ def _enforce_chat_privileges(request, sess) -> None:
|
||||
which means unrestricted allowed_models / zero cap -> no-op for them.
|
||||
"""
|
||||
try:
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
except Exception:
|
||||
user = None
|
||||
if not user:
|
||||
@@ -346,11 +346,11 @@ def add_user_message(sess, chat_handler, preprocessed: PreprocessedMessage, inco
|
||||
def fire_message_event(request, webhook_manager, session_id: str, sess, message: str, compare_mode: bool = False):
|
||||
"""Fire webhook and event_bus events for a new user message."""
|
||||
if webhook_manager and not compare_mode:
|
||||
asyncio.create_task(webhook_manager.fire("chat.message", {
|
||||
webhook_manager.fire_and_forget("chat.message", {
|
||||
"session_id": session_id, "model": sess.model, "message": message[:2000],
|
||||
}))
|
||||
})
|
||||
from src.event_bus import fire_event
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
fire_event("message_sent", user)
|
||||
|
||||
|
||||
@@ -577,7 +577,7 @@ async def build_chat_context(
|
||||
fire_message_event(request, webhook_manager, session_id, sess, message, compare_mode)
|
||||
|
||||
# Resolve user prefs
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
uprefs = load_prefs_for_user(user)
|
||||
|
||||
# Memory enabled?
|
||||
@@ -1120,10 +1120,10 @@ def run_post_response_tasks(
|
||||
|
||||
# Webhook
|
||||
if webhook_manager and not compare_mode:
|
||||
asyncio.create_task(webhook_manager.fire("chat.completed", {
|
||||
webhook_manager.fire_and_forget("chat.completed", {
|
||||
"session_id": session_id, "model": sess.model,
|
||||
"user_message": message, "response": full_response[:2000],
|
||||
}))
|
||||
})
|
||||
|
||||
# Auto-name
|
||||
if needs_auto_name(sess.name):
|
||||
|
||||
+13
-10
@@ -23,7 +23,7 @@ from src.endpoint_resolver import normalize_base as _normalize_base, build_chat_
|
||||
from src.session_search import search_session_messages
|
||||
from src.prompt_security import untrusted_context_message
|
||||
from core.exceptions import SessionNotFoundError
|
||||
from src.auth_helpers import get_current_user
|
||||
from src.auth_helpers import effective_user, get_current_user
|
||||
from routes.session_routes import _verify_session_owner
|
||||
from routes.document_helpers import _owner_session_filter
|
||||
from core.database import SessionLocal, get_session_mode, set_session_mode
|
||||
@@ -126,7 +126,8 @@ def _clear_orphaned_session_endpoint(sess, owner: str | None = None) -> bool:
|
||||
sess.model = ""
|
||||
sess.headers = {}
|
||||
return True
|
||||
except Exception:
|
||||
except Exception as e:
|
||||
logger.warning("Failed to clear orphaned session endpoint", exc_info=e)
|
||||
db.rollback()
|
||||
return False
|
||||
finally:
|
||||
@@ -144,7 +145,8 @@ def _endpoint_cache_contains_model(endpoint, model: str) -> bool:
|
||||
return True
|
||||
try:
|
||||
models = json.loads(raw) if isinstance(raw, str) else raw
|
||||
except Exception:
|
||||
except Exception as e:
|
||||
logger.warning("Failed to parse cached models list, treating as containing model", exc_info=e)
|
||||
return True
|
||||
if not isinstance(models, list) or not models:
|
||||
return True
|
||||
@@ -236,7 +238,8 @@ def _recover_empty_session_model(sess, session_id: str, owner: str | None = None
|
||||
is_chatgpt_subscription = False
|
||||
try:
|
||||
cached = json.loads(ep.cached_models) if isinstance(ep.cached_models, str) else (ep.cached_models or [])
|
||||
except Exception:
|
||||
except Exception as e:
|
||||
logger.warning("Failed to parse cached_models for endpoint %r", getattr(ep, "id", "?"), exc_info=e)
|
||||
cached = []
|
||||
if not cached:
|
||||
visible = []
|
||||
@@ -360,7 +363,7 @@ def setup_chat_routes(
|
||||
sess = session_manager.get_session(session)
|
||||
except KeyError:
|
||||
raise HTTPException(404, f"Session '{session}' not found")
|
||||
owner = get_current_user(request)
|
||||
owner = effective_user(request)
|
||||
if _clear_orphaned_session_endpoint(sess, owner=owner):
|
||||
raise HTTPException(400, "Selected model endpoint was removed. Pick another model in Settings.")
|
||||
|
||||
@@ -600,7 +603,7 @@ def setup_chat_routes(
|
||||
# but BEFORE loading. Prevents cross-user session hijack.
|
||||
_verify_session_owner(request, session)
|
||||
sess = session_manager.get_session(session)
|
||||
owner = get_current_user(request)
|
||||
owner = effective_user(request)
|
||||
if _clear_orphaned_session_endpoint(sess, owner=owner):
|
||||
raise HTTPException(400, "Selected model endpoint was removed. Pick another model in Settings.")
|
||||
# Issue #587: picker shows a model from the endpoint cache but
|
||||
@@ -631,7 +634,7 @@ def setup_chat_routes(
|
||||
_enforce_chat_privileges(request, sess)
|
||||
|
||||
# Ensure session has auth headers
|
||||
resolve_session_auth(sess, session, owner=get_current_user(request))
|
||||
resolve_session_auth(sess, session, owner=effective_user(request))
|
||||
|
||||
# Check for research_pending BEFORE mode persist overwrites it
|
||||
do_research = str(use_research).lower() == "true"
|
||||
@@ -646,8 +649,8 @@ def setup_chat_routes(
|
||||
elif attachments:
|
||||
try:
|
||||
att_ids = [str(x) for x in json.loads(attachments)]
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.warning("Failed to parse attachments JSON, ignoring attachments", exc_info=e)
|
||||
|
||||
no_memory = str(form_data.get("no_memory", "")).lower() == "true"
|
||||
pre_context_tool_policy = build_effective_tool_policy(
|
||||
@@ -1482,7 +1485,7 @@ def setup_chat_routes(
|
||||
if not q or not q.strip():
|
||||
return []
|
||||
|
||||
_user = get_current_user(request)
|
||||
_user = effective_user(request)
|
||||
return [
|
||||
result.to_dict()
|
||||
for result in search_session_messages(
|
||||
|
||||
@@ -505,6 +505,8 @@ def _cached_model_scan_script(model_dirs: list[str] | None = None, add_hf_cache:
|
||||
" if u.startswith('KB'): return int(n * 1024)",
|
||||
" return int(n)",
|
||||
"def scan_ollama():",
|
||||
" if any(m.get('is_ollama') for m in models): return",
|
||||
" if os.name == 'nt' and not os.environ.get('ODYSSEUS_ALLOW_OLLAMA_CLI_SCAN'): return",
|
||||
" if not shutil.which('ollama'): return",
|
||||
" try:",
|
||||
" p = subprocess.run(['ollama', 'list'], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, text=True, timeout=6)",
|
||||
@@ -535,8 +537,8 @@ def _cached_model_scan_script(model_dirs: list[str] | None = None, add_hf_cache:
|
||||
" models.append({'repo_id':name,'size_bytes':size_bytes,'nb_files':1,'has_incomplete':False,'path':'ollama','backend':'ollama','is_ollama':True})",
|
||||
" return",
|
||||
"for _hf_cache in hf_cache_paths(): scan_hf(_hf_cache)",
|
||||
"scan_ollama()",
|
||||
"scan_ollama_api()",
|
||||
"scan_ollama()",
|
||||
]
|
||||
for model_dir in model_dirs or []:
|
||||
lines.append(f"scan_dir(os.path.expanduser({model_dir!r}))")
|
||||
|
||||
@@ -503,7 +503,8 @@ def setup_document_routes(session_manager, upload_handler=None) -> APIRouter:
|
||||
user = get_current_user(request)
|
||||
try:
|
||||
data = await request.json()
|
||||
except Exception:
|
||||
except Exception as e:
|
||||
logger.warning("Failed to parse export request body, defaulting to empty", exc_info=e)
|
||||
data = {}
|
||||
ids = data.get("ids") or []
|
||||
if not ids:
|
||||
@@ -645,8 +646,8 @@ def setup_document_routes(session_manager, upload_handler=None) -> APIRouter:
|
||||
try:
|
||||
from src.agent_tools.document_tools import clear_active_document
|
||||
clear_active_document(doc_id)
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.warning("Failed to clear active document %r on detach", doc_id, exc_info=e)
|
||||
db.commit()
|
||||
db.refresh(doc)
|
||||
return _doc_to_dict(doc)
|
||||
|
||||
@@ -79,15 +79,16 @@ def _email_tag_owner_aliases(account_id: str | None, owner: str = "") -> list[st
|
||||
cfg.get("smtp_user") or "",
|
||||
cfg.get("from_address") or "",
|
||||
])
|
||||
except Exception:
|
||||
except Exception as _e:
|
||||
logger.warning("Failed to resolve email account alias", exc_info=_e)
|
||||
resolved_account_id = None
|
||||
row = db.get(_EA, resolved_account_id) if resolved_account_id else None
|
||||
if row:
|
||||
aliases.extend([row.owner or "", row.imap_user or "", row.from_address or ""])
|
||||
finally:
|
||||
db.close()
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as _e:
|
||||
logger.warning("Failed to load email aliases", exc_info=_e)
|
||||
out = []
|
||||
for a in aliases:
|
||||
a = (a or "").strip()
|
||||
|
||||
@@ -9,6 +9,7 @@ from pathlib import Path
|
||||
from fastapi import APIRouter, HTTPException, Form, Depends
|
||||
from core.constants import EMBEDDING_ENDPOINT_FILE, FASTEMBED_CACHE_DIR
|
||||
from core.middleware import require_admin
|
||||
from src.runtime_paths import get_app_root
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@ from core.session_manager import SessionManager
|
||||
from core.models import ChatMessage
|
||||
from src.request_models import SessionResponse
|
||||
from core.database import Session as DbSession, SessionLocal, Document, GalleryImage, utcnow_naive
|
||||
from src.auth_helpers import get_current_user, effective_user, _auth_disabled, owner_filter
|
||||
from src.auth_helpers import effective_user, _auth_disabled, owner_filter
|
||||
from src.session_actions import is_session_recently_active
|
||||
|
||||
|
||||
@@ -328,7 +328,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
||||
endpoint_id: str = Form(""),
|
||||
):
|
||||
skip_val = str(skip_validation).lower() == "true"
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
endpoint_api_key = ""
|
||||
endpoint_base_url = ""
|
||||
_reject_raw_endpoint_url_for_non_admin(request, user, endpoint_id, endpoint_url)
|
||||
@@ -477,7 +477,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
||||
db.close()
|
||||
# Switch model/endpoint mid-session
|
||||
if model is not None and endpoint_url is not None:
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
_reject_raw_endpoint_url_for_non_admin(request, user, endpoint_id, endpoint_url)
|
||||
endpoint_api_key = ""
|
||||
endpoint_base_url = ""
|
||||
|
||||
@@ -7,7 +7,7 @@ from fastapi import APIRouter, Request, File, UploadFile, HTTPException
|
||||
from typing import List
|
||||
import logging
|
||||
from core.middleware import require_admin
|
||||
from src.auth_helpers import get_current_user
|
||||
from src.auth_helpers import effective_user
|
||||
from src.upload_handler import count_recent_uploads
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -78,7 +78,7 @@ def setup_upload_routes(upload_handler):
|
||||
|
||||
for u in files:
|
||||
try:
|
||||
meta = upload_handler.save_upload(u, client_ip, owner=get_current_user(request))
|
||||
meta = upload_handler.save_upload(u, client_ip, owner=effective_user(request))
|
||||
out.append({
|
||||
"id": meta["id"],
|
||||
"name": meta["name"],
|
||||
@@ -138,7 +138,7 @@ def setup_upload_routes(upload_handler):
|
||||
original_name = info.get("name", file_id)
|
||||
auth_mgr = getattr(request.app.state, "auth_manager", None)
|
||||
auth_configured = bool(auth_mgr and auth_mgr.is_configured)
|
||||
current_user = get_current_user(request)
|
||||
current_user = effective_user(request)
|
||||
file_owner = info.get("owner") if info else None
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
@@ -204,7 +204,7 @@ def setup_upload_routes(upload_handler):
|
||||
info = _load_upload_info(file_id)
|
||||
auth_mgr = getattr(request.app.state, "auth_manager", None)
|
||||
auth_configured = bool(auth_mgr and auth_mgr.is_configured)
|
||||
current_user = get_current_user(request)
|
||||
current_user = effective_user(request)
|
||||
file_owner = info.get("owner") if info else None
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
@@ -247,7 +247,7 @@ def setup_upload_routes(upload_handler):
|
||||
raise HTTPException(404, "File not found")
|
||||
auth_mgr = getattr(request.app.state, "auth_manager", None)
|
||||
auth_configured = bool(auth_mgr and auth_mgr.is_configured)
|
||||
current_user = get_current_user(request)
|
||||
current_user = effective_user(request)
|
||||
file_owner = info.get("owner")
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
"""Webhook, API Token, and sync chat routes."""
|
||||
|
||||
import asyncio
|
||||
import uuid
|
||||
import logging
|
||||
from typing import Optional
|
||||
@@ -385,10 +384,10 @@ def setup_webhook_routes(
|
||||
sess.add_message(ChatMessage("assistant", reply))
|
||||
session_manager.save_sessions()
|
||||
|
||||
asyncio.create_task(webhook_manager.fire("chat.completed", {
|
||||
webhook_manager.fire_and_forget("chat.completed", {
|
||||
"session_id": session_id, "model": sess.model,
|
||||
"user_message": message[:2000], "response": reply[:2000],
|
||||
}))
|
||||
})
|
||||
|
||||
return {"response": reply, "session_id": session_id, "model": sess.model}
|
||||
|
||||
|
||||
@@ -19,6 +19,10 @@ GPU_BANDWIDTH = {
|
||||
"6950 xt": 576, "6900 xt": 512, "6800 xt": 512, "6800": 512, "6700 xt": 384, "6600 xt": 256, "6600": 224,
|
||||
"mi300x": 5300, "mi300": 5300, "mi250x": 3277, "mi250": 3277, "mi210": 1638, "mi100": 1229,
|
||||
"9070 xt": 624, "9070": 488, "9060 xt": 322, "9060": 322,
|
||||
# NVIDIA GB10 Grace-Blackwell superchip (DGX Spark). Unified LPDDR5X memory,
|
||||
# not Apple Silicon, so it lives in the generic GPU table — the Apple-only
|
||||
# lookup never matches it (its name carries no "apple").
|
||||
"gb10": 273,
|
||||
}
|
||||
|
||||
# Pre-sort keys by length descending for correct substring matching
|
||||
|
||||
+162
-13
@@ -15,6 +15,8 @@ from urllib.parse import urljoin, urlparse
|
||||
import httpx
|
||||
from bs4 import BeautifulSoup
|
||||
|
||||
from src.constants import WEB_FETCH_SOFT_MAX_BYTES, WEB_FETCH_HARD_MAX_BYTES
|
||||
|
||||
from .analytics import RateLimitError, error_logger
|
||||
from .cache import (
|
||||
CONTENT_CACHE_DIR,
|
||||
@@ -89,18 +91,128 @@ def _public_http_url(url: str) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _get_public_url(url: str, headers: dict, timeout: int, max_redirects: int = 5) -> httpx.Response:
|
||||
class BodyTooLargeError(Exception):
|
||||
"""The server declared a body larger than the hard fetch ceiling."""
|
||||
|
||||
def __init__(self, url: str, declared_bytes: int):
|
||||
self.url = url
|
||||
self.declared_bytes = declared_bytes
|
||||
super().__init__(
|
||||
f"response body is {declared_bytes:,} bytes, over the "
|
||||
f"{WEB_FETCH_HARD_MAX_BYTES:,}-byte hard cap"
|
||||
)
|
||||
|
||||
|
||||
class _CappedFetch:
|
||||
"""Result of a size-capped streaming GET.
|
||||
|
||||
Carries just what fetch_webpage_content needs from an httpx.Response,
|
||||
plus the cap bookkeeping: the (possibly truncated) body, whether the
|
||||
cap cut it short, and the size the server declared via Content-Length
|
||||
(wire bytes; None when absent).
|
||||
"""
|
||||
|
||||
__slots__ = ("status_code", "headers", "content", "truncated",
|
||||
"declared_bytes", "encoding", "url")
|
||||
|
||||
def __init__(self, status_code, headers, content, truncated,
|
||||
declared_bytes, encoding, url):
|
||||
self.status_code = status_code
|
||||
self.headers = headers
|
||||
self.content = content
|
||||
self.truncated = truncated
|
||||
self.declared_bytes = declared_bytes
|
||||
self.encoding = encoding
|
||||
self.url = url
|
||||
|
||||
@property
|
||||
def text(self) -> str:
|
||||
return self.content.decode(self.encoding or "utf-8", errors="replace")
|
||||
|
||||
def raise_for_status(self):
|
||||
if self.status_code >= 400:
|
||||
request = httpx.Request("GET", self.url)
|
||||
raise httpx.HTTPStatusError(
|
||||
f"HTTP {self.status_code} for {self.url}",
|
||||
request=request,
|
||||
response=httpx.Response(self.status_code, request=request),
|
||||
)
|
||||
|
||||
|
||||
def _get_public_url(url: str, headers: dict, timeout: int, max_redirects: int = 5,
|
||||
max_bytes: int = None) -> "_CappedFetch":
|
||||
"""Capped streaming GET with SSRF-guarded manual redirects.
|
||||
|
||||
The body is streamed and buffering stops at ``max_bytes`` (default: the
|
||||
soft cap), so an oversized resource cannot be pulled into memory or the
|
||||
content cache in full. When Content-Length already declares a body over
|
||||
the hard ceiling, the fetch is refused before any body bytes are read.
|
||||
"""
|
||||
cap = min(max_bytes or WEB_FETCH_SOFT_MAX_BYTES, WEB_FETCH_HARD_MAX_BYTES)
|
||||
current = url
|
||||
for _ in range(max_redirects + 1):
|
||||
if not _public_http_url(current):
|
||||
raise httpx.RequestError("Blocked private/internal URL", request=httpx.Request("GET", current))
|
||||
response = httpx.get(current, headers=headers, timeout=timeout, follow_redirects=False)
|
||||
if response.status_code not in (301, 302, 303, 307, 308):
|
||||
return response
|
||||
location = response.headers.get("location")
|
||||
if not location:
|
||||
return response
|
||||
current = urljoin(str(response.url), location)
|
||||
# Force identity transfer-encoding. With gzip/deflate the wire bytes
|
||||
# (and Content-Length) can be a small fraction of the decoded body, so
|
||||
# a tiny compressed response could pass the hard-cap preflight and then
|
||||
# expand past the ceiling in a single decoded chunk before the streamed
|
||||
# cap below can slice it. Identity makes Content-Length the true body
|
||||
# size and keeps each streamed chunk bounded by the network read.
|
||||
req_headers = dict(headers or {})
|
||||
req_headers["Accept-Encoding"] = "identity"
|
||||
with httpx.stream("GET", current, headers=req_headers, timeout=timeout,
|
||||
follow_redirects=False) as response:
|
||||
if response.status_code in (301, 302, 303, 307, 308):
|
||||
location = response.headers.get("location")
|
||||
if not location:
|
||||
return _CappedFetch(response.status_code, response.headers, b"",
|
||||
False, None, response.encoding, str(response.url))
|
||||
current = urljoin(str(response.url), location)
|
||||
continue
|
||||
|
||||
# A server can ignore the identity request and still return a
|
||||
# compressed body; httpx.iter_bytes would then decode it, and a tiny
|
||||
# gzip can balloon into one decoded chunk far past the cap before we
|
||||
# slice. Refuse a compressed Content-Encoding so the streamed cap
|
||||
# stays a real memory bound (Content-Length is the compressed wire
|
||||
# length here, so the preflight and size metadata are unreliable too).
|
||||
enc = (response.headers.get("content-encoding") or "").strip().lower()
|
||||
if enc and enc != "identity":
|
||||
raise httpx.RequestError(
|
||||
f"Refusing compressed response (Content-Encoding: {enc}) after "
|
||||
"requesting identity: cannot bound decoded body size",
|
||||
request=httpx.Request("GET", current),
|
||||
)
|
||||
|
||||
declared = None
|
||||
raw_len = response.headers.get("content-length")
|
||||
if raw_len and raw_len.isdigit():
|
||||
declared = int(raw_len)
|
||||
# Refuse before buffering anything when the server already tells
|
||||
# us the body exceeds the absolute ceiling (Content-Length is wire
|
||||
# bytes; the decompressed body can only be larger).
|
||||
if declared is not None and declared > WEB_FETCH_HARD_MAX_BYTES:
|
||||
raise BodyTooLargeError(current, declared)
|
||||
|
||||
chunks = []
|
||||
read = 0
|
||||
truncated = False
|
||||
# We requested identity above, so iter_bytes yields the raw body in
|
||||
# network-read-sized chunks (no decompression expansion); the cap
|
||||
# therefore bounds what we actually buffer.
|
||||
for chunk in response.iter_bytes():
|
||||
read += len(chunk)
|
||||
if read > cap:
|
||||
keep = cap - (read - len(chunk))
|
||||
if keep > 0:
|
||||
chunks.append(chunk[:keep])
|
||||
truncated = True
|
||||
break
|
||||
chunks.append(chunk)
|
||||
return _CappedFetch(response.status_code, response.headers,
|
||||
b"".join(chunks), truncated, declared,
|
||||
response.encoding, str(response.url))
|
||||
raise httpx.RequestError("Too many redirects", request=httpx.Request("GET", current))
|
||||
|
||||
# PDF extraction (optional dependency)
|
||||
@@ -222,9 +334,19 @@ def _empty_result(url: str, error: str = "") -> dict:
|
||||
# ----------------------------------------------------------------------
|
||||
# Main content fetcher
|
||||
# ----------------------------------------------------------------------
|
||||
def fetch_webpage_content(url: str, timeout: int = 5, retry_attempt: int = 0) -> dict:
|
||||
"""Fetch and extract meaningful content from a webpage with caching."""
|
||||
cache_key = generate_cache_key(url)
|
||||
def fetch_webpage_content(url: str, timeout: int = 5, retry_attempt: int = 0,
|
||||
max_bytes: int = None) -> dict:
|
||||
"""Fetch and extract meaningful content from a webpage with caching.
|
||||
|
||||
``max_bytes`` raises the download budget per call (clamped to the hard
|
||||
cap); the default is the soft cap. When the body is cut short the result
|
||||
carries ``truncated``/``fetched_bytes``/``total_bytes`` so callers can
|
||||
tell the model the content is partial (#3812).
|
||||
"""
|
||||
effective_cap = min(max_bytes or WEB_FETCH_SOFT_MAX_BYTES, WEB_FETCH_HARD_MAX_BYTES)
|
||||
# The cap is part of the cache identity: a truncated soft-cap fetch must
|
||||
# not be served to a later full-budget request for the same URL.
|
||||
cache_key = generate_cache_key(f"{url}#cap={effective_cap}")
|
||||
cache_file = CONTENT_CACHE_DIR / f"{cache_key}.cache"
|
||||
|
||||
# Check cache
|
||||
@@ -250,15 +372,21 @@ def fetch_webpage_content(url: str, timeout: int = 5, retry_attempt: int = 0) ->
|
||||
"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36",
|
||||
"Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8",
|
||||
"Accept-Language": "en-US,en;q=0.5",
|
||||
"Accept-Encoding": "gzip, deflate",
|
||||
# identity so the streamed size cap in _get_public_url stays honest
|
||||
# (a compressed body can decode to far more than Content-Length).
|
||||
"Accept-Encoding": "identity",
|
||||
"Connection": "keep-alive",
|
||||
}
|
||||
response = _get_public_url(url, headers=headers, timeout=timeout)
|
||||
response = _get_public_url(url, headers=headers, timeout=timeout,
|
||||
max_bytes=effective_cap)
|
||||
|
||||
if response.status_code == 429:
|
||||
raise RateLimitError(f"Rate limit hit for {url} (attempt {retry_attempt})")
|
||||
|
||||
response.raise_for_status()
|
||||
except BodyTooLargeError as e:
|
||||
error_logger.warning(f"Refused oversized body for {url}: {e}")
|
||||
return _empty_result(url, f"TooLarge: {e}")
|
||||
except httpx.HTTPStatusError as e:
|
||||
error_logger.warning(f"HTTP {e.response.status_code} fetching {url}: {e}")
|
||||
return _empty_result(url, f"HTTP {e.response.status_code}: {e}")
|
||||
@@ -269,9 +397,27 @@ def fetch_webpage_content(url: str, timeout: int = 5, retry_attempt: int = 0) ->
|
||||
error_logger.error(str(e))
|
||||
return _empty_result(url, str(e))
|
||||
|
||||
# Size bookkeeping shared by every content branch below. getattr keeps
|
||||
# plain httpx.Response stand-ins (tests) working without the cap fields.
|
||||
_size_fields = {
|
||||
"truncated": getattr(response, "truncated", False),
|
||||
"fetched_bytes": len(response.content),
|
||||
"total_bytes": getattr(response, "declared_bytes", None),
|
||||
}
|
||||
|
||||
# PDF handling
|
||||
content_type = response.headers.get("Content-Type", "").lower()
|
||||
if "application/pdf" in content_type or url.lower().endswith(".pdf"):
|
||||
if _size_fields["truncated"]:
|
||||
# A PDF cut mid-stream is not parseable; unlike text there is no
|
||||
# useful partial result, so report the budget problem instead.
|
||||
_declared = _size_fields["total_bytes"]
|
||||
return _empty_result(
|
||||
url,
|
||||
f"TooLarge: PDF exceeds the {effective_cap:,}-byte fetch budget"
|
||||
+ (f" (size {_declared:,} bytes)" if _declared else "")
|
||||
+ "; retry with a larger budget if it fits under the hard cap",
|
||||
)
|
||||
if pdf_extract_text is None:
|
||||
logger.error("pdfminer.six is not installed; cannot extract PDF text.")
|
||||
pdf_text = ""
|
||||
@@ -295,6 +441,7 @@ def fetch_webpage_content(url: str, timeout: int = 5, retry_attempt: int = 0) ->
|
||||
"js_message": "",
|
||||
"success": bool(pdf_text),
|
||||
"error": "" if pdf_text else "Failed to extract PDF text",
|
||||
**_size_fields,
|
||||
}
|
||||
_cache_result(cache_file, cache_key, result, url)
|
||||
return result
|
||||
@@ -329,6 +476,7 @@ def fetch_webpage_content(url: str, timeout: int = 5, retry_attempt: int = 0) ->
|
||||
"js_message": "",
|
||||
"success": bool(text_body),
|
||||
"error": "" if text_body else "Empty response body",
|
||||
**_size_fields,
|
||||
}
|
||||
_cache_result(cache_file, cache_key, result, url)
|
||||
return result
|
||||
@@ -391,6 +539,7 @@ def fetch_webpage_content(url: str, timeout: int = 5, retry_attempt: int = 0) ->
|
||||
"js_message": js_message,
|
||||
"success": True,
|
||||
"error": "",
|
||||
**_size_fields,
|
||||
}
|
||||
_cache_result(cache_file, cache_key, result, url)
|
||||
return result
|
||||
|
||||
@@ -9,14 +9,12 @@ from urllib.parse import urljoin, urlparse, parse_qs
|
||||
import httpx
|
||||
from bs4 import BeautifulSoup
|
||||
|
||||
from src.constants import SEARXNG_INSTANCE
|
||||
from src.constants import SEARXNG_INSTANCE, REQUEST_TIMEOUT
|
||||
from .analytics import RateLimitError, error_logger
|
||||
from .query import build_enhanced_query
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
REQUEST_TIMEOUT = 20
|
||||
|
||||
# Provider registry — maps setting value to (label, needs_key, needs_url)
|
||||
PROVIDER_INFO = {
|
||||
"searxng": ("SearXNG", False, True),
|
||||
|
||||
+5
-5
@@ -524,7 +524,7 @@ def get_builtin_overrides() -> dict:
|
||||
ov = get_setting("builtin_tool_overrides", {})
|
||||
return ov if isinstance(ov, dict) else {}
|
||||
except Exception as e:
|
||||
logger.warning('Failed to load builtin tool overrides: %s', e)
|
||||
logger.warning("Failed to load builtin tool overrides, using defaults", exc_info=e)
|
||||
return {}
|
||||
|
||||
|
||||
@@ -929,8 +929,8 @@ def _build_system_prompt(
|
||||
try:
|
||||
from src.user_time import current_datetime_context_message
|
||||
_datetime_message = current_datetime_context_message()
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.warning("Failed to build datetime context message", exc_info=e)
|
||||
|
||||
# Document context is kept as a SEPARATE message (not merged into the tool
|
||||
# prompt) so the context trimmer doesn't destroy it when truncating the
|
||||
@@ -973,8 +973,8 @@ def _build_system_prompt(
|
||||
try:
|
||||
from src.pdf_form_doc import find_source_upload_id
|
||||
_is_form_backed = bool(find_source_upload_id(active_document.current_content or ""))
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.warning("Failed to detect if document is form-backed, assuming plain", exc_info=e)
|
||||
|
||||
if _is_form_backed:
|
||||
doc_ctx = (
|
||||
|
||||
@@ -57,13 +57,23 @@ class WebSearchTool:
|
||||
class WebFetchTool:
|
||||
async def execute(self, content: str, ctx: dict) -> dict:
|
||||
from src.search.content import fetch_webpage_content
|
||||
from src.constants import WEB_FETCH_HARD_MAX_BYTES
|
||||
raw = content.strip()
|
||||
url = ""
|
||||
max_bytes = None
|
||||
if raw.startswith("{"):
|
||||
try:
|
||||
parsed = json.loads(raw)
|
||||
if isinstance(parsed, dict):
|
||||
url = str(parsed.get("url") or "").strip()
|
||||
# Download-budget override (#3812): "full": true raises the
|
||||
# budget to the hard cap; an explicit max_bytes is clamped
|
||||
# to the hard cap downstream. Default stays the soft cap.
|
||||
if parsed.get("full") is True:
|
||||
max_bytes = WEB_FETCH_HARD_MAX_BYTES
|
||||
mb = parsed.get("max_bytes")
|
||||
if isinstance(mb, int) and mb > 0:
|
||||
max_bytes = mb
|
||||
except json.JSONDecodeError:
|
||||
url = ""
|
||||
if not url:
|
||||
@@ -78,7 +88,7 @@ class WebFetchTool:
|
||||
loop = asyncio.get_running_loop()
|
||||
try:
|
||||
result = await asyncio.wait_for(
|
||||
loop.run_in_executor(None, lambda: fetch_webpage_content(url, timeout=10)),
|
||||
loop.run_in_executor(None, lambda: fetch_webpage_content(url, timeout=10, max_bytes=max_bytes)),
|
||||
timeout=30,
|
||||
)
|
||||
except asyncio.TimeoutError:
|
||||
@@ -94,8 +104,28 @@ class WebFetchTool:
|
||||
return {"error": f"web_fetch: {url}: {err}", "exit_code": 1}
|
||||
return {"error": f"web_fetch: {url}: no readable text content (not HTML, or the page needs JS/login)", "exit_code": 1}
|
||||
|
||||
# Tell the model when the download budget cut the body short and how
|
||||
# to get the rest, instead of silently presenting a partial page as
|
||||
# the whole thing.
|
||||
size_note = ""
|
||||
if result.get("truncated"):
|
||||
fetched = result.get("fetched_bytes") or 0
|
||||
total = result.get("total_bytes")
|
||||
total_txt = f" of {total:,} bytes" if total else ""
|
||||
size_note = (
|
||||
f"[partial content: download stopped at {fetched:,} bytes{total_txt}. "
|
||||
f'Re-call with {{"url": "{url}", "full": true}} to fetch up to '
|
||||
f"{WEB_FETCH_HARD_MAX_BYTES:,} bytes.]\n\n"
|
||||
)
|
||||
|
||||
# The notice must lead the output so the MAX_OUTPUT_CHARS trim below can
|
||||
# never drop it. The title is untrusted, uncapped page content, so a
|
||||
# giant title ahead of the notice could push it out of range; keep the
|
||||
# notice first and cap the title as a second guard.
|
||||
if len(title) > 300:
|
||||
title = title[:300] + "..."
|
||||
header = (f"# {title}\n" if title else "") + f"Source: {url}\n\n"
|
||||
output = header + text
|
||||
output = size_note + header + text
|
||||
if len(output) > MAX_OUTPUT_CHARS:
|
||||
output = output[:MAX_OUTPUT_CHARS] + "\n\n[...truncated]"
|
||||
return {"output": output, "exit_code": 0}
|
||||
|
||||
+3
-2
@@ -14,6 +14,7 @@ import subprocess
|
||||
import sys
|
||||
|
||||
from core.platform_compat import IS_WINDOWS, which_tool
|
||||
from src.runtime_paths import get_app_root
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -81,7 +82,7 @@ _BUILTIN_NPX_SERVERS = {
|
||||
"name": "Built-in: Browser",
|
||||
"command": "npx",
|
||||
"args": ["-y", "@playwright/mcp@latest", "--headless", "--caps", "vision"],
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
# Global flag to disable MCP if there are compatibility issues
|
||||
@@ -94,7 +95,7 @@ async def register_builtin_servers(mcp_manager):
|
||||
logger.info("Built-in MCP servers disabled via ODYSSEUS_DISABLE_MCP")
|
||||
return
|
||||
|
||||
base_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
base_dir = get_app_root()
|
||||
python = sys.executable
|
||||
|
||||
async def _connect_python_server(server_id: str, script_path: str, name: str):
|
||||
|
||||
+3
-2
@@ -5,6 +5,7 @@ from pydantic_settings import BaseSettings, SettingsConfigDict
|
||||
from pydantic import Field, field_validator
|
||||
|
||||
from src.constants import DATA_DIR as _DATA_DIR_CONST
|
||||
from src.runtime_paths import get_app_root
|
||||
|
||||
# Cross-platform OS flag, exposed here so callers can `from src.config import
|
||||
# IS_WINDOWS`. Defined locally (a trivial `os.name == "nt"`) rather than imported
|
||||
@@ -19,7 +20,7 @@ IS_WINDOWS = os.name == "nt"
|
||||
class DataConfig(BaseSettings):
|
||||
"""Configuration for data storage and file handling."""
|
||||
# Base directory
|
||||
base_dir: Path = Field(default=Path(__file__).parent.parent, description="Base directory for the application")
|
||||
base_dir: Path = Field(default=Path(get_app_root()), description="Base directory for the application")
|
||||
|
||||
# Data paths
|
||||
data_dir: Path = Field(default=Path(_DATA_DIR_CONST), description="Main data directory")
|
||||
@@ -138,7 +139,7 @@ class AppConfig(BaseSettings):
|
||||
if isinstance(v, dict) and "base_dir" in v:
|
||||
base_dir = v["base_dir"]
|
||||
else:
|
||||
base_dir = Path(__file__).parent.parent
|
||||
base_dir = Path(get_app_root())
|
||||
|
||||
# Convert string paths to Path objects relative to base_dir
|
||||
data_dir = Path(_DATA_DIR_CONST)
|
||||
|
||||
+12
-2
@@ -2,12 +2,14 @@
|
||||
"""Application-wide constants and configuration values."""
|
||||
import os
|
||||
|
||||
from src.runtime_paths import get_app_root, get_default_data_dir
|
||||
|
||||
APP_VERSION = "1.0.0"
|
||||
|
||||
# Base paths
|
||||
BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + "/"
|
||||
BASE_DIR = os.path.join(get_app_root(), "")
|
||||
STATIC_DIR = os.path.join(BASE_DIR, "static")
|
||||
DATA_DIR = os.getenv("ODYSSEUS_DATA_DIR", os.path.join(BASE_DIR, "data"))
|
||||
DATA_DIR = os.getenv("ODYSSEUS_DATA_DIR", get_default_data_dir())
|
||||
|
||||
# Data file paths
|
||||
# Single source of truth: every persisted file/dir lives under DATA_DIR, which
|
||||
@@ -63,6 +65,14 @@ MAX_OUTPUT_CHARS = 10_000 # cap for bash/python/web_search/web_fetch outpu
|
||||
MAX_READ_CHARS = 20_000 # cap for read_file / document preview
|
||||
MAX_DIFF_LINES = 400 # cap for edit_file unified-diff display
|
||||
|
||||
# web_fetch response-size policy (#3812). MAX_OUTPUT_CHARS above only trims
|
||||
# what the agent SEES; these caps bound what the server downloads, parses,
|
||||
# and writes to the content cache. The soft cap is the default download
|
||||
# budget; the agent can raise it per call (full/max_bytes) but never past
|
||||
# the hard cap, so a model can't decide to pull a multi-GB file.
|
||||
WEB_FETCH_SOFT_MAX_BYTES = 2_000_000 # default download budget (2 MB)
|
||||
WEB_FETCH_HARD_MAX_BYTES = 20_000_000 # absolute ceiling, even with override (20 MB)
|
||||
|
||||
# API Configuration
|
||||
MAX_CONTEXT_MESSAGES = 90
|
||||
REQUEST_TIMEOUT = 20
|
||||
|
||||
@@ -31,6 +31,8 @@ import numpy as np
|
||||
import httpx
|
||||
from typing import List, Optional
|
||||
|
||||
from src.runtime_paths import get_app_root
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_DEFAULT_MODEL = "all-minilm:l6-v2"
|
||||
|
||||
+4
-3
@@ -283,7 +283,8 @@ def _is_ollama_native_url(url: str) -> bool:
|
||||
"""Return True for native Ollama API URLs, including Ollama Cloud."""
|
||||
try:
|
||||
parsed = urlparse(url or "")
|
||||
except Exception:
|
||||
except Exception as e:
|
||||
logger.warning("Failed to parse URL for Ollama detection", exc_info=e)
|
||||
return False
|
||||
host = parsed.hostname or ""
|
||||
path = (parsed.path or "").rstrip("/")
|
||||
@@ -1345,8 +1346,8 @@ def list_model_ids(
|
||||
r = httpx.get(root + "/api/tags", timeout=timeout)
|
||||
r.raise_for_status()
|
||||
return [m.get("name") or m.get("model") for m in (r.json().get("models") or []) if m.get("name") or m.get("model")]
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.warning("Failed to fetch model list from configured endpoint", exc_info=e)
|
||||
return []
|
||||
|
||||
def normalize_model_id(
|
||||
|
||||
+3
-1
@@ -11,6 +11,8 @@ import os
|
||||
import re
|
||||
from typing import Any, Dict, List, Optional, Set, Tuple
|
||||
|
||||
from src.runtime_paths import get_app_root
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
def _format_mcp_connection_error(name: str, command: str = "", args: Optional[List[str]] = None, error: Exception = None) -> str:
|
||||
@@ -508,7 +510,7 @@ class McpManager:
|
||||
return False
|
||||
|
||||
script_rel, name = _BUILTIN_SERVERS[server_id]
|
||||
base_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
base_dir = get_app_root()
|
||||
script_path = os.path.join(base_dir, script_rel)
|
||||
|
||||
# Clean up old connection
|
||||
|
||||
@@ -7,6 +7,7 @@ import time
|
||||
from pathlib import Path
|
||||
|
||||
from src.constants import RAG_DIR
|
||||
from src.runtime_paths import get_app_root
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
"""Helpers for resolving runtime paths in source and frozen builds."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
||||
|
||||
def get_app_root() -> str:
|
||||
"""Return the app root directory.
|
||||
|
||||
In normal source runs, this is the repository root. In a frozen Windows
|
||||
build, it is the bundle content root (PyInstaller's internal directory)
|
||||
so bundled runtime folders like `static/`, `scripts/`, and `data/` stay
|
||||
together with the executable payload.
|
||||
"""
|
||||
if getattr(sys, "frozen", False):
|
||||
return getattr(sys, "_MEIPASS", os.path.dirname(os.path.abspath(sys.executable)))
|
||||
return os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
|
||||
|
||||
def get_default_data_dir() -> str:
|
||||
"""Return the default path to the data directory.
|
||||
|
||||
In normal runs, this is a 'data' subdirectory under the app root.
|
||||
In frozen builds, it is a persistent user directory (~/.odysseus/data)
|
||||
to prevent SQLite databases and other persistent files from being
|
||||
written to the ephemeral, temporary extraction bundle directory.
|
||||
"""
|
||||
if getattr(sys, "frozen", False):
|
||||
return os.path.join(os.path.expanduser("~"), ".odysseus", "data")
|
||||
return os.path.join(get_app_root(), "data")
|
||||
@@ -3797,7 +3797,7 @@ async def do_resolve_contact(content: str, owner: Optional[str] = None) -> Dict:
|
||||
if not name:
|
||||
return {"error": "name is required", "exit_code": 1}
|
||||
|
||||
contacts = {} # email -> {name, source}
|
||||
contacts = {} # email_or_phone -> {name, source, phone?}
|
||||
|
||||
# 1. CardDAV (Radicale) — structured contacts. Call in-process: a
|
||||
# server-side httpx GET to /api/contacts/search carries no session
|
||||
@@ -3812,10 +3812,18 @@ async def do_resolve_contact(content: str, owner: Optional[str] = None) -> Dict:
|
||||
match = q in hay_name or any(q in (e or "").lower() for e in c.get("emails", []))
|
||||
if not match:
|
||||
continue
|
||||
has_email = False
|
||||
for email in (c.get("emails") or []):
|
||||
email = (email or "").strip().lower()
|
||||
if email and "@" in email:
|
||||
contacts[email] = {"name": c.get("name") or email, "source": "contacts"}
|
||||
has_email = True
|
||||
# Fall back to phone numbers when the contact has no email address
|
||||
if not has_email:
|
||||
for phone in (c.get("phones") or []):
|
||||
phone = (phone or "").strip()
|
||||
if phone:
|
||||
contacts[phone] = {"name": c.get("name") or phone, "source": "contacts", "phone": phone}
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
@@ -3835,8 +3843,11 @@ async def do_resolve_contact(content: str, owner: Optional[str] = None) -> Dict:
|
||||
return {"output": f"No contacts found matching '{name}'.", "exit_code": 0}
|
||||
|
||||
lines = [f"Contacts matching '{name}':"]
|
||||
for email, info in contacts.items():
|
||||
lines.append(f"- {info['name']} <{email}> ({info['source']})")
|
||||
for key, info in contacts.items():
|
||||
if info.get("phone"):
|
||||
lines.append(f"- {info['name']} — phone: {info['phone']} ({info['source']})")
|
||||
else:
|
||||
lines.append(f"- {info['name']} <{key}> ({info['source']})")
|
||||
return {"output": "\n".join(lines), "exit_code": 0}
|
||||
|
||||
|
||||
|
||||
+4
-3
@@ -68,11 +68,12 @@ FUNCTION_TOOL_SCHEMAS = [
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_fetch",
|
||||
"description": "Fetch and read the text content of a specific URL the user names (e.g. 'check example.com', 'what's on this page <url>'). Use when you already have a concrete URL/domain. NOT for open-ended searches (use web_search) or 'research X' jobs (use trigger_research).",
|
||||
"description": "Fetch and read the text content of a specific URL the user names (e.g. 'check example.com', 'what's on this page <url>'). Use when you already have a concrete URL/domain. NOT for open-ended searches (use web_search) or 'research X' jobs (use trigger_research). Downloads are size-budgeted; a '[partial content: ...]' notice in the result means the body was cut short and you can re-call with full=true for the rest.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"url": {"type": "string", "description": "The URL or domain to fetch (http/https; a bare domain like example.com is fine)"}
|
||||
"url": {"type": "string", "description": "The URL or domain to fetch (http/https; a bare domain like example.com is fine)"},
|
||||
"full": {"type": "boolean", "description": "Raise the download budget to the hard cap for large pages/files. Use only after a result reported partial content."}
|
||||
},
|
||||
"required": ["url"]
|
||||
}
|
||||
@@ -1008,7 +1009,7 @@ FUNCTION_TOOL_SCHEMAS = [
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "resolve_contact",
|
||||
"description": "Look up a contact's email address by name. Searches CardDAV address book and sent email history. Use when the user says 'message [name]' or 'email [name]' without an email address.",
|
||||
"description": "Look up a contact by name. Searches CardDAV address book and sent email history. Returns email addresses (when available) or phone numbers. Use when the user says 'message [name]', 'email [name]', or asks for someone's contact details.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
@@ -0,0 +1,326 @@
|
||||
# Oversized Test File Split Plan
|
||||
|
||||
## Purpose
|
||||
|
||||
This document plans future oversized test-file splits using current repo data.
|
||||
It does not move files, rewrite assertions, extract helpers, or change CI.
|
||||
|
||||
## Roadmap context
|
||||
|
||||
- Issue: #3983
|
||||
- Parent tracker: #2523
|
||||
- Follows #3973 / #3982, the report-only order-sensitivity diagnostics slice.
|
||||
|
||||
## Methodology
|
||||
|
||||
Metrics were generated from the current test tree using:
|
||||
|
||||
- physical line counts for every recursive `test_*.py` file under `tests/`;
|
||||
- AST counts for `test_*` functions and `Test*` classes;
|
||||
- one `pytest --collect-only -q tests` run to count collected items per file;
|
||||
- current taxonomy classification from `tests._taxonomy.classify_test_path`; and
|
||||
- static setup-signal scans for route/API, DB/session, import-state, security, filesystem, subprocess/script, async/threading, and UI/static indicators.
|
||||
|
||||
Static signals are not proof of risk. They are review prompts.
|
||||
Future split PRs must still inspect each file manually before editing.
|
||||
|
||||
## Current summary
|
||||
|
||||
- test files scanned: 583
|
||||
- collected pytest items counted: 3586
|
||||
- large-file threshold: 300 lines
|
||||
- large-collected threshold: 20 collected items
|
||||
|
||||
Area distribution:
|
||||
|
||||
| Value | Files |
|
||||
|---|---:|
|
||||
| cli | 28 |
|
||||
| helpers | 1 |
|
||||
| js | 39 |
|
||||
| routes | 23 |
|
||||
| security | 77 |
|
||||
| services | 144 |
|
||||
| uncategorized | 234 |
|
||||
| unit | 37 |
|
||||
|
||||
Sub-area distribution:
|
||||
|
||||
| Value | Files |
|
||||
|---|---:|
|
||||
| api | 6 |
|
||||
| atomic | 3 |
|
||||
| auth | 9 |
|
||||
| calendar | 10 |
|
||||
| cli | 28 |
|
||||
| confinement | 7 |
|
||||
| cookbook | 13 |
|
||||
| document | 11 |
|
||||
| email | 12 |
|
||||
| embedding | 3 |
|
||||
| gallery | 5 |
|
||||
| history | 3 |
|
||||
| js | 39 |
|
||||
| llm | 16 |
|
||||
| mcp | 8 |
|
||||
| memory | 15 |
|
||||
| nondict | 7 |
|
||||
| nonstring | 22 |
|
||||
| owner | 14 |
|
||||
| owner_scope | 23 |
|
||||
| parse | 4 |
|
||||
| provider | 6 |
|
||||
| research | 16 |
|
||||
| route | 6 |
|
||||
| routes | 9 |
|
||||
| scheduler | 3 |
|
||||
| scope | 5 |
|
||||
| security | 9 |
|
||||
| session | 16 |
|
||||
| ssrf | 3 |
|
||||
| webhook | 3 |
|
||||
| xss | 5 |
|
||||
|
||||
Values below 2 files: 244 values covering 244 files.
|
||||
|
||||
## Top files by collected pytest items
|
||||
|
||||
| File | Lines | Collected tests | Test defs | Test classes | Area | Sub-area | Signals |
|
||||
|---|---:|---:|---:|---:|---|---|---|
|
||||
| `tests/test_model_routes.py` | 1778 | 139 | 116 | 10 | routes | routes | route/api, db/session, import-state, async/threading |
|
||||
| `tests/test_security_regressions.py` | 1224 | 92 | 68 | 0 | security | security | route/api, db/session, import-state, security, filesystem, async/threading, ui/static |
|
||||
| `tests/test_provider_classification.py` | 188 | 67 | 21 | 4 | services | provider | - |
|
||||
| `tests/test_cookbook_helpers.py` | 912 | 65 | 65 | 0 | services | cookbook | route/api, filesystem, subprocess/script, async/threading, ui/static |
|
||||
| `tests/test_shell_routes.py` | 481 | 63 | 48 | 8 | routes | routes | route/api, import-state, filesystem |
|
||||
| `tests/test_pr_blocker_audit.py` | 964 | 58 | 58 | 0 | uncategorized | pr_blocker_audit | import-state, security, filesystem |
|
||||
| `tests/test_provider_endpoints.py` | 241 | 58 | 18 | 1 | services | provider | subprocess/script |
|
||||
| `tests/test_agent_loop.py` | 469 | 52 | 52 | 5 | uncategorized | agent_loop | db/session, import-state |
|
||||
| `tests/test_service_health.py` | 472 | 47 | 42 | 0 | uncategorized | service_health | async/threading |
|
||||
| `tests/test_run_focus.py` | 399 | 47 | 44 | 0 | uncategorized | run_focus | security, filesystem, subprocess/script, ui/static |
|
||||
| `tests/test_llm_core_temperature.py` | 196 | 41 | 17 | 0 | services | llm | - |
|
||||
| `tests/test_endpoint_probing.py` | 411 | 34 | 30 | 6 | uncategorized | endpoint_probing | route/api, db/session, import-state |
|
||||
| `tests/test_llm_core_anthropic_temp_omit.py` | 94 | 32 | 6 | 0 | services | llm | db/session |
|
||||
| `tests/test_chat_helpers.py` | 264 | 31 | 18 | 0 | uncategorized | chat_helpers | route/api |
|
||||
| `tests/test_provider_detection.py` | 148 | 31 | 31 | 5 | services | provider | - |
|
||||
| `tests/test_model_context.py` | 251 | 30 | 30 | 4 | uncategorized | model_context | db/session, import-state |
|
||||
| `tests/test_endpoint_resolver.py` | 148 | 30 | 30 | 6 | uncategorized | endpoint_resolver | - |
|
||||
| `tests/test_embedding_lanes.py` | 1104 | 29 | 29 | 0 | services | embedding | filesystem |
|
||||
| `tests/test_upload_limits_centralized.py` | 110 | 29 | 5 | 0 | uncategorized | upload_limits_centralized | import-state, filesystem |
|
||||
| `tests/test_email_oauth.py` | 580 | 28 | 25 | 0 | services | email | route/api, db/session, security, async/threading |
|
||||
| `tests/test_review_regressions.py` | 930 | 26 | 26 | 0 | uncategorized | review_regressions | route/api, db/session, import-state, filesystem, async/threading |
|
||||
| `tests/test_rename_user_owner_sync.py` | 686 | 26 | 26 | 0 | security | owner | route/api, db/session, import-state, filesystem, async/threading |
|
||||
| `tests/test_helpers_import_state.py` | 426 | 26 | 26 | 0 | helpers | helpers | route/api, db/session, import-state |
|
||||
| `tests/test_taxonomy.py` | 145 | 26 | 16 | 0 | uncategorized | taxonomy | security, ui/static |
|
||||
| `tests/test_tool_path_confinement.py` | 282 | 24 | 24 | 0 | security | confinement | import-state, filesystem, async/threading |
|
||||
| `tests/test_copilot.py` | 170 | 23 | 16 | 0 | uncategorized | copilot | - |
|
||||
| `tests/test_research_utils.py` | 97 | 23 | 23 | 2 | services | research | - |
|
||||
| `tests/test_api_chat_security.py` | 401 | 22 | 8 | 0 | security | security | route/api, db/session, import-state, filesystem, async/threading |
|
||||
| `tests/test_tool_support_heuristic.py` | 166 | 22 | 22 | 3 | uncategorized | tool_support_heuristic | - |
|
||||
| `tests/test_platform_compat.py` | 318 | 21 | 21 | 0 | uncategorized | platform_compat | import-state, filesystem, subprocess/script |
|
||||
|
||||
## Top files by physical line count
|
||||
|
||||
| File | Lines | Collected tests | Test defs | Test classes | Area | Sub-area | Signals |
|
||||
|---|---:|---:|---:|---:|---|---|---|
|
||||
| `tests/test_model_routes.py` | 1778 | 139 | 116 | 10 | routes | routes | route/api, db/session, import-state, async/threading |
|
||||
| `tests/test_security_regressions.py` | 1224 | 92 | 68 | 0 | security | security | route/api, db/session, import-state, security, filesystem, async/threading, ui/static |
|
||||
| `tests/test_embedding_lanes.py` | 1104 | 29 | 29 | 0 | services | embedding | filesystem |
|
||||
| `tests/test_pr_blocker_audit.py` | 964 | 58 | 58 | 0 | uncategorized | pr_blocker_audit | import-state, security, filesystem |
|
||||
| `tests/test_review_regressions.py` | 930 | 26 | 26 | 0 | uncategorized | review_regressions | route/api, db/session, import-state, filesystem, async/threading |
|
||||
| `tests/test_cookbook_helpers.py` | 912 | 65 | 65 | 0 | services | cookbook | route/api, filesystem, subprocess/script, async/threading, ui/static |
|
||||
| `tests/test_rename_user_owner_sync.py` | 686 | 26 | 26 | 0 | security | owner | route/api, db/session, import-state, filesystem, async/threading |
|
||||
| `tests/test_email_oauth.py` | 580 | 28 | 25 | 0 | services | email | route/api, db/session, security, async/threading |
|
||||
| `tests/test_api_token_routes.py` | 578 | 17 | 17 | 0 | routes | api_routes | route/api, db/session, import-state, async/threading |
|
||||
| `tests/test_shell_routes.py` | 481 | 63 | 48 | 8 | routes | routes | route/api, import-state, filesystem |
|
||||
| `tests/test_email_owner_scope.py` | 474 | 9 | 9 | 0 | security | owner_scope | route/api, db/session, filesystem, async/threading |
|
||||
| `tests/test_service_health.py` | 472 | 47 | 42 | 0 | uncategorized | service_health | async/threading |
|
||||
| `tests/test_agent_loop.py` | 469 | 52 | 52 | 5 | uncategorized | agent_loop | db/session, import-state |
|
||||
| `tests/test_kv_cache_invalidation_2927.py` | 463 | 8 | 8 | 0 | uncategorized | kv_cache_invalidation_2927 | route/api, db/session, import-state, async/threading |
|
||||
| `tests/test_helpers_import_state.py` | 426 | 26 | 26 | 0 | helpers | helpers | route/api, db/session, import-state |
|
||||
| `tests/test_endpoint_owner_scope_followup.py` | 414 | 11 | 11 | 0 | security | owner_scope | route/api, db/session, filesystem |
|
||||
| `tests/test_endpoint_probing.py` | 411 | 34 | 30 | 6 | uncategorized | endpoint_probing | route/api, db/session, import-state |
|
||||
| `tests/test_imap_leak_fixes.py` | 404 | 15 | 15 | 0 | uncategorized | imap_leak_fixes | route/api, db/session, security, filesystem |
|
||||
| `tests/test_companion_readonly.py` | 402 | 17 | 17 | 0 | uncategorized | companion_readonly | db/session, import-state |
|
||||
| `tests/test_api_chat_security.py` | 401 | 22 | 8 | 0 | security | security | route/api, db/session, import-state, filesystem, async/threading |
|
||||
| `tests/test_upload_handler_atomicity.py` | 401 | 9 | 9 | 0 | uncategorized | upload_handler_atomicity | filesystem, async/threading |
|
||||
| `tests/test_run_focus.py` | 399 | 47 | 44 | 0 | uncategorized | run_focus | security, filesystem, subprocess/script, ui/static |
|
||||
| `tests/test_auth_regressions.py` | 375 | 15 | 15 | 0 | security | auth | route/api, db/session, import-state, async/threading |
|
||||
| `tests/test_calendar_owner_scope.py` | 345 | 7 | 7 | 0 | security | owner_scope | route/api, db/session, import-state, filesystem, async/threading, ui/static |
|
||||
| `tests/test_null_owner_gates.py` | 342 | 20 | 20 | 0 | security | owner | route/api, db/session, import-state |
|
||||
| `tests/test_agent_migration_manifest.py` | 340 | 15 | 15 | 0 | uncategorized | agent_migration_manifest | import-state, filesystem |
|
||||
| `tests/test_calendar_recurrence.py` | 338 | 19 | 19 | 0 | services | calendar | - |
|
||||
| `tests/test_tool_policy.py` | 330 | 13 | 13 | 0 | uncategorized | tool_policy | import-state, async/threading |
|
||||
| `tests/test_workspace_confine.py` | 328 | 18 | 18 | 0 | uncategorized | workspace_confine | route/api, filesystem, subprocess/script, async/threading |
|
||||
| `tests/test_diffusion_server_security.py` | 325 | 14 | 14 | 0 | security | security | route/api, import-state, security, filesystem, async/threading, ui/static |
|
||||
|
||||
## Split planning candidates
|
||||
|
||||
This section is generated from metrics, not from manual judgement.
|
||||
Files are included when they meet at least one threshold:
|
||||
|
||||
- at least 300 physical lines; or
|
||||
- at least 20 collected pytest items.
|
||||
|
||||
These are planning candidates only. A later split PR still needs a focused manual review of each file before moving tests.
|
||||
|
||||
| File | Why included | Setup/risk signals | Suggested handling |
|
||||
|---|---|---|---|
|
||||
| `tests/test_model_routes.py` | 1778 lines, 139 collected tests | route/api, db/session, import-state, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_security_regressions.py` | 1224 lines, 92 collected tests | route/api, db/session, import-state, security, filesystem, async/threading, ui/static | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_provider_classification.py` | 67 collected tests | No obvious setup signals from static scan. | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_cookbook_helpers.py` | 912 lines, 65 collected tests | route/api, filesystem, subprocess/script, async/threading, ui/static | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_shell_routes.py` | 481 lines, 63 collected tests | route/api, import-state, filesystem | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_pr_blocker_audit.py` | 964 lines, 58 collected tests | import-state, security, filesystem | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_provider_endpoints.py` | 58 collected tests | subprocess/script | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_agent_loop.py` | 469 lines, 52 collected tests | db/session, import-state | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_service_health.py` | 472 lines, 47 collected tests | async/threading | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_run_focus.py` | 399 lines, 47 collected tests | security, filesystem, subprocess/script, ui/static | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_llm_core_temperature.py` | 41 collected tests | No obvious setup signals from static scan. | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_endpoint_probing.py` | 411 lines, 34 collected tests | route/api, db/session, import-state | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_llm_core_anthropic_temp_omit.py` | 32 collected tests | db/session | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_chat_helpers.py` | 31 collected tests | route/api | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_provider_detection.py` | 31 collected tests | No obvious setup signals from static scan. | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_model_context.py` | 30 collected tests | db/session, import-state | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_endpoint_resolver.py` | 30 collected tests | No obvious setup signals from static scan. | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_embedding_lanes.py` | 1104 lines, 29 collected tests | filesystem | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_upload_limits_centralized.py` | 29 collected tests | import-state, filesystem | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_email_oauth.py` | 580 lines, 28 collected tests | route/api, db/session, security, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_review_regressions.py` | 930 lines, 26 collected tests | route/api, db/session, import-state, filesystem, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_rename_user_owner_sync.py` | 686 lines, 26 collected tests | route/api, db/session, import-state, filesystem, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_helpers_import_state.py` | 426 lines, 26 collected tests | route/api, db/session, import-state | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_taxonomy.py` | 26 collected tests | security, ui/static | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_tool_path_confinement.py` | 24 collected tests | import-state, filesystem, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_copilot.py` | 23 collected tests | No obvious setup signals from static scan. | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_research_utils.py` | 23 collected tests | No obvious setup signals from static scan. | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_api_chat_security.py` | 401 lines, 22 collected tests | route/api, db/session, import-state, filesystem, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_tool_support_heuristic.py` | 22 collected tests | No obvious setup signals from static scan. | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_platform_compat.py` | 318 lines, 21 collected tests | import-state, filesystem, subprocess/script | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_context_compactor.py` | 21 collected tests | db/session, import-state, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_prompt_security.py` | 21 collected tests | No obvious setup signals from static scan. | Good first manual-review candidate if test themes are cohesive. |
|
||||
| `tests/test_null_owner_gates.py` | 342 lines, 20 collected tests | route/api, db/session, import-state | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_youtube_handler_consolidation.py` | 20 collected tests | route/api, import-state | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_calendar_recurrence.py` | 338 lines | No obvious setup signals from static scan. | Plan split boundaries before editing. |
|
||||
| `tests/test_workspace_confine.py` | 328 lines | route/api, filesystem, subprocess/script, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_api_token_routes.py` | 578 lines | route/api, db/session, import-state, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_companion_readonly.py` | 402 lines | db/session, import-state | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_set_admin.py` | 317 lines | route/api, import-state, filesystem, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_imap_leak_fixes.py` | 404 lines | route/api, db/session, security, filesystem | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_auth_regressions.py` | 375 lines | route/api, db/session, import-state, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_agent_migration_manifest.py` | 340 lines | import-state, filesystem | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_diffusion_server_security.py` | 325 lines | route/api, import-state, security, filesystem, async/threading, ui/static | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_tool_policy.py` | 330 lines | import-state, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_endpoint_owner_scope_followup.py` | 414 lines | route/api, db/session, filesystem | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_upload_routes_owner_scope.py` | 315 lines | route/api, filesystem, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_email_owner_scope.py` | 474 lines | route/api, db/session, filesystem, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_upload_handler_atomicity.py` | 401 lines | filesystem, async/threading | Plan split boundaries before editing. |
|
||||
| `tests/test_kv_cache_invalidation_2927.py` | 463 lines | route/api, db/session, import-state, async/threading | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_calendar_owner_scope.py` | 345 lines | route/api, db/session, import-state, filesystem, async/threading, ui/static | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
| `tests/test_skills_manager_owner_isolation.py` | 306 lines | import-state, filesystem | Defer mechanical split until setup/risk boundaries are mapped. |
|
||||
|
||||
## Taxonomy coverage gaps among split candidates
|
||||
|
||||
`uncategorized` is a current taxonomy area, not a builder failure.
|
||||
This plan does not reclassify tests because taxonomy changes should be reviewed separately from oversized-file split planning.
|
||||
|
||||
Before using any of these files as a split target, first decide whether the taxonomy should be refined in a separate focused issue/PR.
|
||||
|
||||
| File | Lines | Collected tests | Sub-area | Signals | Suggested follow-up |
|
||||
|---|---:|---:|---|---|---|
|
||||
| `tests/test_pr_blocker_audit.py` | 964 | 58 | pr_blocker_audit | import-state, security, filesystem | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_agent_loop.py` | 469 | 52 | agent_loop | db/session, import-state | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_service_health.py` | 472 | 47 | service_health | async/threading | Review taxonomy mapping before using as a split target. |
|
||||
| `tests/test_run_focus.py` | 399 | 47 | run_focus | security, filesystem, subprocess/script, ui/static | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_endpoint_probing.py` | 411 | 34 | endpoint_probing | route/api, db/session, import-state | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_chat_helpers.py` | 264 | 31 | chat_helpers | route/api | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_model_context.py` | 251 | 30 | model_context | db/session, import-state | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_endpoint_resolver.py` | 148 | 30 | endpoint_resolver | - | Review taxonomy mapping before using as a split target. |
|
||||
| `tests/test_upload_limits_centralized.py` | 110 | 29 | upload_limits_centralized | import-state, filesystem | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_review_regressions.py` | 930 | 26 | review_regressions | route/api, db/session, import-state, filesystem, async/threading | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_taxonomy.py` | 145 | 26 | taxonomy | security, ui/static | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_copilot.py` | 170 | 23 | copilot | - | Review taxonomy mapping before using as a split target. |
|
||||
| `tests/test_tool_support_heuristic.py` | 166 | 22 | tool_support_heuristic | - | Review taxonomy mapping before using as a split target. |
|
||||
| `tests/test_platform_compat.py` | 318 | 21 | platform_compat | import-state, filesystem, subprocess/script | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_context_compactor.py` | 233 | 21 | context_compactor | db/session, import-state, async/threading | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_youtube_handler_consolidation.py` | 104 | 20 | youtube_handler_consolidation | route/api, import-state | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_workspace_confine.py` | 328 | 18 | workspace_confine | route/api, filesystem, subprocess/script, async/threading | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_companion_readonly.py` | 402 | 17 | companion_readonly | db/session, import-state | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_set_admin.py` | 317 | 17 | set_admin | route/api, import-state, filesystem, async/threading | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_imap_leak_fixes.py` | 404 | 15 | imap_leak_fixes | route/api, db/session, security, filesystem | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_agent_migration_manifest.py` | 340 | 15 | agent_migration_manifest | import-state, filesystem | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_tool_policy.py` | 330 | 13 | tool_policy | import-state, async/threading | Review taxonomy and setup/risk boundaries before any split. |
|
||||
| `tests/test_upload_handler_atomicity.py` | 401 | 9 | upload_handler_atomicity | filesystem, async/threading | Review taxonomy mapping before using as a split target. |
|
||||
| `tests/test_kv_cache_invalidation_2927.py` | 463 | 8 | kv_cache_invalidation_2927 | route/api, db/session, import-state, async/threading | Review taxonomy and setup/risk boundaries before any split. |
|
||||
|
||||
## Suggested first manual-review candidates
|
||||
|
||||
These are not automatic split approvals. They are categorized candidates with enough size/collection value and no route/API, DB/session, import-state, or security signal from the static scan.
|
||||
|
||||
Files still in the `uncategorized` taxonomy area are listed separately below so taxonomy review does not get mixed into the first split decision.
|
||||
|
||||
| File | Lines | Collected tests | Area | Sub-area | Signals | Why this is a candidate |
|
||||
|---|---:|---:|---|---|---|---|
|
||||
| `tests/test_provider_classification.py` | 188 | 67 | services | provider | - | 67 collected tests |
|
||||
| `tests/test_provider_endpoints.py` | 241 | 58 | services | provider | subprocess/script | 58 collected tests |
|
||||
| `tests/test_llm_core_temperature.py` | 196 | 41 | services | llm | - | 41 collected tests |
|
||||
| `tests/test_provider_detection.py` | 148 | 31 | services | provider | - | 31 collected tests |
|
||||
| `tests/test_embedding_lanes.py` | 1104 | 29 | services | embedding | filesystem | 1104 lines, 29 collected tests |
|
||||
| `tests/test_research_utils.py` | 97 | 23 | services | research | - | 23 collected tests |
|
||||
| `tests/test_prompt_security.py` | 203 | 21 | security | security | - | 21 collected tests |
|
||||
| `tests/test_calendar_recurrence.py` | 338 | 19 | services | calendar | - | 338 lines |
|
||||
|
||||
## High-risk candidates to defer first
|
||||
|
||||
These files may still be split later, but not as the first implementation slice without a separate manual boundary review.
|
||||
|
||||
| File | Lines | Collected tests | High-risk signals |
|
||||
|---|---:|---:|---|
|
||||
| `tests/test_model_routes.py` | 1778 | 139 | db/session, import-state, route/api |
|
||||
| `tests/test_security_regressions.py` | 1224 | 92 | db/session, import-state, route/api, security |
|
||||
| `tests/test_cookbook_helpers.py` | 912 | 65 | route/api |
|
||||
| `tests/test_shell_routes.py` | 481 | 63 | import-state, route/api |
|
||||
| `tests/test_pr_blocker_audit.py` | 964 | 58 | import-state, security |
|
||||
| `tests/test_agent_loop.py` | 469 | 52 | db/session, import-state |
|
||||
| `tests/test_run_focus.py` | 399 | 47 | security |
|
||||
| `tests/test_endpoint_probing.py` | 411 | 34 | db/session, import-state, route/api |
|
||||
| `tests/test_llm_core_anthropic_temp_omit.py` | 94 | 32 | db/session |
|
||||
| `tests/test_chat_helpers.py` | 264 | 31 | route/api |
|
||||
| `tests/test_model_context.py` | 251 | 30 | db/session, import-state |
|
||||
| `tests/test_upload_limits_centralized.py` | 110 | 29 | import-state |
|
||||
| `tests/test_email_oauth.py` | 580 | 28 | db/session, route/api, security |
|
||||
| `tests/test_review_regressions.py` | 930 | 26 | db/session, import-state, route/api |
|
||||
| `tests/test_rename_user_owner_sync.py` | 686 | 26 | db/session, import-state, route/api |
|
||||
|
||||
## Rules for future split PRs
|
||||
|
||||
- One file or one coherent file-family per PR.
|
||||
- No assertion rewrites mixed with file moves.
|
||||
- No helper extraction mixed with file moves.
|
||||
- No production code changes.
|
||||
- No CI workflow changes.
|
||||
- Preserve existing markers and taxonomy unless the split issue explicitly says otherwise.
|
||||
- Validate the original file's collected tests before and after the split.
|
||||
- Validate any neighboring taxonomy/focused-runner behavior if paths change.
|
||||
- Treat files with route/API, DB/session, import-state, or security signals as higher-risk until manually reviewed.
|
||||
|
||||
## Suggested next step
|
||||
|
||||
Use this plan to choose the first actual oversized-file split issue.
|
||||
The first split should prefer a file with high review value and low setup risk.
|
||||
Do not start a split PR from this planning issue alone if the file's boundaries are still ambiguous.
|
||||
|
||||
## Reproduction command
|
||||
|
||||
This document was generated with:
|
||||
|
||||
```bash
|
||||
.venv/bin/python tests/tools/build_oversized_test_split_plan.py
|
||||
```
|
||||
|
||||
## Freshness check
|
||||
|
||||
After editing the builder or rebasing the branch, regenerate the plan and confirm no unexpected plan drift:
|
||||
|
||||
```bash
|
||||
.venv/bin/python tests/tools/build_oversized_test_split_plan.py
|
||||
git diff --exit-code -- tests/OVERSIZED_TEST_SPLIT_PLAN.md
|
||||
```
|
||||
@@ -502,3 +502,77 @@ def test_delete_token_owner_check_skipped_when_auth_disabled(monkeypatch, token_
|
||||
resp = delete_token(request=req, token_id="tok123")
|
||||
assert resp == {"status": "deleted"}
|
||||
fake_session.delete.assert_called_once_with(fake_token)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 7. PATCH /api/tokens/{id} — non-object JSON bodies must not 500
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_update_token_with_array_body_does_not_500(monkeypatch, token_routes_mod):
|
||||
"""PATCH body of [] must be normalised to {} and not raise."""
|
||||
monkeypatch.setenv("AUTH_ENABLED", "true")
|
||||
mod = token_routes_mod
|
||||
|
||||
token = SimpleNamespace(
|
||||
id="tok123", name="original", owner="alice",
|
||||
token_prefix="ody_orig", scopes="email:read", 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, [])
|
||||
update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}")
|
||||
resp = asyncio.run(update_token(request=req, token_id="tok123"))
|
||||
|
||||
# Name and scopes must be unchanged — payload was normalised to {}
|
||||
assert token.name == "original"
|
||||
assert token.scopes == "email:read"
|
||||
assert resp["name"] == "original"
|
||||
|
||||
|
||||
def test_update_token_with_null_body_does_not_500(monkeypatch, token_routes_mod):
|
||||
"""PATCH body of null must be normalised to {} and not raise."""
|
||||
monkeypatch.setenv("AUTH_ENABLED", "true")
|
||||
mod = token_routes_mod
|
||||
|
||||
token = SimpleNamespace(
|
||||
id="tok123", name="original", owner="alice",
|
||||
token_prefix="ody_orig", scopes="chat", 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, None)
|
||||
update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}")
|
||||
resp = asyncio.run(update_token(request=req, token_id="tok123"))
|
||||
|
||||
assert token.name == "original"
|
||||
assert token.scopes == "chat"
|
||||
|
||||
|
||||
def test_update_token_normal_object_still_works(monkeypatch, token_routes_mod):
|
||||
"""Normal dict payload continues to update fields as before."""
|
||||
monkeypatch.setenv("AUTH_ENABLED", "true")
|
||||
mod = token_routes_mod
|
||||
|
||||
token = SimpleNamespace(
|
||||
id="tok123", name="original", owner="alice",
|
||||
token_prefix="ody_orig", scopes="email:read", 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": "updated"})
|
||||
update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}")
|
||||
resp = asyncio.run(update_token(request=req, token_id="tok123"))
|
||||
|
||||
assert token.name == "updated"
|
||||
assert resp["name"] == "updated"
|
||||
invalidator.assert_called_once()
|
||||
|
||||
@@ -30,7 +30,7 @@ class _Session:
|
||||
|
||||
|
||||
def test_allowed_models_legacy_empty_list_remains_unrestricted(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
_enforce_chat_privileges(
|
||||
_Request({"allowed_models": [], "max_messages_per_day": 0}),
|
||||
@@ -39,7 +39,7 @@ def test_allowed_models_legacy_empty_list_remains_unrestricted(monkeypatch):
|
||||
|
||||
|
||||
def test_allowed_models_explicit_empty_restricted_list_blocks_all_models(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
_enforce_chat_privileges(
|
||||
@@ -56,7 +56,7 @@ def test_allowed_models_explicit_empty_restricted_list_blocks_all_models(monkeyp
|
||||
|
||||
|
||||
def test_allowed_models_nonempty_list_still_restricts_without_new_flag(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
_enforce_chat_privileges(
|
||||
_Request({"allowed_models": ["provider/model-a"], "max_messages_per_day": 0}),
|
||||
@@ -70,7 +70,7 @@ def test_allowed_models_nonempty_list_still_restricts_without_new_flag(monkeypat
|
||||
|
||||
|
||||
def test_no_restriction_allows_any_model(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
privs = {"allowed_models": [], "block_all_models": False, "max_messages_per_day": 0}
|
||||
_enforce_chat_privileges(_Request(privs), _Session("provider/model-a"))
|
||||
@@ -78,7 +78,7 @@ def test_no_restriction_allows_any_model(monkeypatch):
|
||||
|
||||
|
||||
def test_specific_allowlist_blocks_models_outside_it(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
privs = {
|
||||
"allowed_models": ["gpt-4"],
|
||||
@@ -92,7 +92,7 @@ def test_specific_allowlist_blocks_models_outside_it(monkeypatch):
|
||||
|
||||
|
||||
def test_block_all_models_blocks_regardless_of_allowed_models_contents(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
# Even if allowed_models contains entries, block_all_models wins.
|
||||
privs = {
|
||||
@@ -111,7 +111,7 @@ def test_block_all_models_blocks_regardless_of_allowed_models_contents(monkeypat
|
||||
def test_admin_user_is_never_blocked(monkeypatch):
|
||||
from core.auth import ADMIN_PRIVILEGES
|
||||
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "admin")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "admin")
|
||||
|
||||
class _AdminAuthManager:
|
||||
def get_privileges(self, username):
|
||||
|
||||
@@ -13,6 +13,9 @@ import json
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
|
||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
|
||||
# core.database instantiates SQLAlchemy declarative classes at import time, which
|
||||
@@ -225,12 +228,34 @@ def test_models_route_scopes_api_token_to_token_owner(monkeypatch):
|
||||
endpoints = _call_models_route(
|
||||
monkeypatch,
|
||||
rows,
|
||||
_request(api_token=True, api_token_owner="alice", current_user="api"),
|
||||
_request(
|
||||
api_token=True,
|
||||
api_token_owner="alice",
|
||||
api_token_scopes=["chat"],
|
||||
current_user="api",
|
||||
),
|
||||
)
|
||||
|
||||
assert _endpoint_names(endpoints) == ["alice-endpoint", "shared-endpoint"]
|
||||
|
||||
|
||||
def test_models_route_rejects_api_token_without_chat_scope(monkeypatch):
|
||||
monkeypatch.setattr(companion_routes, "get_current_user", lambda request: "api")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
_models_route()(
|
||||
_request(
|
||||
api_token=True,
|
||||
api_token_owner="alice",
|
||||
api_token_scopes=["todos:read"],
|
||||
current_user="api",
|
||||
)
|
||||
)
|
||||
|
||||
assert exc.value.status_code == 403
|
||||
assert "chat scope" in exc.value.detail
|
||||
|
||||
|
||||
def test_models_route_unresolved_owner_returns_only_shared_rows(monkeypatch):
|
||||
rows = [
|
||||
_ep(1, "alice-endpoint", "alice"),
|
||||
@@ -242,7 +267,12 @@ def test_models_route_unresolved_owner_returns_only_shared_rows(monkeypatch):
|
||||
endpoints = _call_models_route(
|
||||
monkeypatch,
|
||||
rows,
|
||||
_request(api_token=True, api_token_owner=None, current_user="api"),
|
||||
_request(
|
||||
api_token=True,
|
||||
api_token_owner=None,
|
||||
api_token_scopes=["chat"],
|
||||
current_user="api",
|
||||
),
|
||||
)
|
||||
|
||||
assert _endpoint_names(endpoints) == ["shared-endpoint"]
|
||||
|
||||
@@ -786,6 +786,50 @@ def test_cached_model_scan_reports_plain_dir_gguf(tmp_path):
|
||||
assert ggufs[3]["quant"] == "BF16"
|
||||
|
||||
|
||||
def test_cached_model_scan_uses_ollama_api_before_cli_and_windows_opt_in():
|
||||
script = _cached_model_scan_script()
|
||||
|
||||
assert "scan_ollama_api()\nscan_ollama()" in script
|
||||
assert "if any(m.get('is_ollama') for m in models): return" in script
|
||||
assert "os.name == 'nt'" in script
|
||||
assert "ODYSSEUS_ALLOW_OLLAMA_CLI_SCAN" in script
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="Windows Ollama CLI startup guard")
|
||||
def test_cached_model_scan_does_not_launch_ollama_cli_on_windows(tmp_path):
|
||||
"""Official Ollama for Windows can auto-start the tray/server on `ollama list`.
|
||||
The read-only cache scanner must not invoke that CLI unless explicitly opted in.
|
||||
"""
|
||||
marker = tmp_path / "ollama-called.txt"
|
||||
fake_ollama = tmp_path / "ollama.cmd"
|
||||
fake_ollama.write_text(
|
||||
"@echo off\r\n"
|
||||
f'echo called>"{marker}"\r\n'
|
||||
"echo NAME ID SIZE MODIFIED\r\n"
|
||||
"echo local-model:latest abc 1 GB now\r\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
empty_home = tmp_path / "home"
|
||||
empty_home.mkdir()
|
||||
scan_py = tmp_path / "scan_cache.py"
|
||||
scan_py.write_text(_cached_model_scan_script(), encoding="utf-8")
|
||||
env = dict(os.environ)
|
||||
env["PATH"] = str(tmp_path) + os.pathsep + env.get("PATH", "")
|
||||
env["HOME"] = str(empty_home)
|
||||
env.pop("ODYSSEUS_ALLOW_OLLAMA_CLI_SCAN", None)
|
||||
proc = subprocess.run(
|
||||
[sys.executable, str(scan_py)],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
)
|
||||
|
||||
assert marker.exists() is False
|
||||
assert all(m.get("backend") != "ollama" for m in json.loads(proc.stdout))
|
||||
|
||||
|
||||
def test_cached_model_scan_uses_huggingface_cache_env(tmp_path):
|
||||
"""Docker recreates can leave the persisted HF cache outside HOME.
|
||||
The Serve scanner should honor the cache env path instead of only ~/.cache.
|
||||
|
||||
@@ -79,7 +79,7 @@ def _build_context_harness(monkeypatch, chat_helpers, history):
|
||||
monkeypatch.setattr(chat_helpers, "extract_preset", fake_extract_preset)
|
||||
monkeypatch.setattr(chat_helpers, "add_user_message", fake_add_user_message)
|
||||
monkeypatch.setattr(chat_helpers, "load_prefs_for_user", lambda user: {})
|
||||
monkeypatch.setattr(chat_helpers, "get_current_user", lambda request: "tester")
|
||||
monkeypatch.setattr(chat_helpers, "effective_user", lambda request: "tester")
|
||||
monkeypatch.setattr(chat_helpers, "normalize_model_id", lambda endpoint_url, model, **kwargs: None)
|
||||
monkeypatch.setattr(chat_helpers, "maybe_compact", fake_maybe_compact)
|
||||
monkeypatch.setattr(chat_helpers, "trim_for_context", lambda messages, context_length: messages)
|
||||
|
||||
@@ -385,7 +385,7 @@ async def test_build_chat_context_incognito_does_not_duplicate_current_user_mess
|
||||
monkeypatch.setattr(chat_helpers, "extract_preset", fake_extract_preset)
|
||||
monkeypatch.setattr(chat_helpers, "add_user_message", fake_add_user_message)
|
||||
monkeypatch.setattr(chat_helpers, "load_prefs_for_user", lambda user: {})
|
||||
monkeypatch.setattr(chat_helpers, "get_current_user", lambda request: "tester")
|
||||
monkeypatch.setattr(chat_helpers, "effective_user", lambda request: "tester")
|
||||
monkeypatch.setattr(chat_helpers, "normalize_model_id", lambda endpoint_url, model, **kwargs: None)
|
||||
monkeypatch.setattr(chat_helpers, "maybe_compact", fake_maybe_compact)
|
||||
monkeypatch.setattr(chat_helpers, "trim_for_context", lambda messages, context_length: messages)
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import os
|
||||
import sys
|
||||
from unittest import mock
|
||||
import pytest
|
||||
from src.runtime_paths import get_app_root, get_default_data_dir
|
||||
|
||||
|
||||
def test_get_app_root_normal_run():
|
||||
"""Verify that get_app_root returns the repository root parent of src/ when not frozen."""
|
||||
with mock.patch.object(sys, "frozen", False, create=True):
|
||||
app_root = get_app_root()
|
||||
# Verify it is a valid directory path and matches expected parent structure
|
||||
assert os.path.isdir(app_root)
|
||||
assert os.path.exists(os.path.join(app_root, "src"))
|
||||
|
||||
|
||||
def test_get_app_root_frozen_with_meipass():
|
||||
"""Verify that get_app_root returns the sys._MEIPASS directory when frozen by PyInstaller."""
|
||||
mock_meipass = os.path.abspath("mock_meipass_dir")
|
||||
with mock.patch.object(sys, "frozen", True, create=True), \
|
||||
mock.patch.object(sys, "_MEIPASS", mock_meipass, create=True):
|
||||
app_root = get_app_root()
|
||||
assert app_root == mock_meipass
|
||||
|
||||
|
||||
def test_get_app_root_frozen_without_meipass():
|
||||
"""Verify that get_app_root falls back to the sys.executable parent directory when frozen but _MEIPASS is absent."""
|
||||
mock_exe_path = os.path.join(os.path.abspath("mock_exe_dir"), "Odysseus.exe")
|
||||
with mock.patch.object(sys, "frozen", True, create=True), \
|
||||
mock.patch.object(sys, "executable", mock_exe_path, create=True):
|
||||
# Remove sys._MEIPASS if it exists in the test process environment
|
||||
if hasattr(sys, "_MEIPASS"):
|
||||
delattr(sys, "_MEIPASS")
|
||||
app_root = get_app_root()
|
||||
assert app_root == os.path.abspath("mock_exe_dir")
|
||||
|
||||
|
||||
def test_get_default_data_dir_normal():
|
||||
"""Verify that get_default_data_dir resolves to get_app_root() / 'data' when not frozen."""
|
||||
with mock.patch.object(sys, "frozen", False, create=True):
|
||||
res = get_default_data_dir()
|
||||
assert res == os.path.join(get_app_root(), "data")
|
||||
|
||||
|
||||
def test_get_default_data_dir_frozen():
|
||||
"""Verify that get_default_data_dir resolves to a persistent user path under ~ when frozen."""
|
||||
with mock.patch.object(sys, "frozen", True, create=True):
|
||||
res = get_default_data_dir()
|
||||
expected = os.path.join(os.path.expanduser("~"), ".odysseus", "data")
|
||||
assert res == expected
|
||||
@@ -58,7 +58,7 @@ def test_content_fetcher_extracts_og_image_and_body_fallback(module, tmp_path, m
|
||||
|
||||
monkeypatch.setattr(module, "CONTENT_CACHE_DIR", tmp_path)
|
||||
module.content_cache_index.clear()
|
||||
monkeypatch.setattr(module, "_get_public_url", lambda url, headers, timeout: _FakeResponse(html))
|
||||
monkeypatch.setattr(module, "_get_public_url", lambda url, headers, timeout, **kwargs: _FakeResponse(html))
|
||||
|
||||
result = module.fetch_webpage_content("https://example.com/parity-test")
|
||||
|
||||
@@ -82,7 +82,7 @@ def test_fetch_webpage_content_returns_empty_result_on_http_status_error(status_
|
||||
monkeypatch.setattr(
|
||||
service_content,
|
||||
"_get_public_url",
|
||||
lambda url, headers, timeout: _FakeErrorResponse(status_code),
|
||||
lambda url, headers, timeout, **kwargs: _FakeErrorResponse(status_code),
|
||||
)
|
||||
|
||||
result = service_content.fetch_webpage_content(f"https://example.com/status-{status_code}")
|
||||
@@ -119,7 +119,7 @@ def test_fetch_webpage_content_429_takes_distinct_rate_limit_path(tmp_path, monk
|
||||
monkeypatch.setattr(
|
||||
service_content,
|
||||
"_get_public_url",
|
||||
lambda url, headers, timeout: _FakeRateLimitResponse(),
|
||||
lambda url, headers, timeout, **kwargs: _FakeRateLimitResponse(),
|
||||
)
|
||||
|
||||
result = service_content.fetch_webpage_content("https://example.com/rate-limited")
|
||||
|
||||
@@ -904,7 +904,13 @@ def test_web_fetch_guard_blocks_redirect_into_private(monkeypatch):
|
||||
url = "http://public.example/start"
|
||||
headers = {"location": "http://169.254.169.254/latest/meta-data/"}
|
||||
|
||||
monkeypatch.setattr(httpx, "get", lambda url, **kwargs: _Resp())
|
||||
from contextlib import contextmanager
|
||||
|
||||
@contextmanager
|
||||
def _fake_stream(method, url, **kwargs):
|
||||
yield _Resp()
|
||||
|
||||
monkeypatch.setattr(httpx, "stream", _fake_stream)
|
||||
|
||||
with _pytest.raises(httpx.RequestError) as exc:
|
||||
content._get_public_url("http://public.example/start", headers={}, timeout=5)
|
||||
|
||||
@@ -52,6 +52,6 @@ def test_chat_endpoint_recovery_paths_are_owner_scoped():
|
||||
assert "def _clear_orphaned_session_endpoint(sess, owner:" in chat_routes
|
||||
assert "def _recover_empty_session_model(sess, session_id: str, owner:" in chat_routes
|
||||
assert "q = owner_filter(q, ModelEndpoint, owner)" in chat_routes
|
||||
assert "resolve_session_auth(sess, session, owner=get_current_user(request))" in chat_routes
|
||||
assert "resolve_session_auth(sess, session, owner=effective_user(request))" in chat_routes
|
||||
assert "def resolve_session_auth(sess, session_id: str, owner:" in chat_helpers
|
||||
assert "update_q = update_q.filter(DBSession.owner == owner)" in chat_helpers
|
||||
|
||||
@@ -35,7 +35,7 @@ def _patch_fetch(monkeypatch, text, content_type):
|
||||
monkeypatch.setattr(
|
||||
content_mod,
|
||||
"_get_public_url",
|
||||
lambda url, headers=None, timeout=5: _FakeResponse(text, content_type),
|
||||
lambda url, headers=None, timeout=5, **kwargs: _FakeResponse(text, content_type),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,206 @@
|
||||
"""web_fetch download budgets (#3812).
|
||||
|
||||
MAX_OUTPUT_CHARS only trims what the agent sees; these caps bound what the
|
||||
server downloads, parses, and caches. Soft cap by default with a truncation
|
||||
notice, per-call override clamped to the hard cap, and a pre-buffer refusal
|
||||
when Content-Length already exceeds the hard ceiling.
|
||||
"""
|
||||
import json
|
||||
from contextlib import contextmanager
|
||||
|
||||
import pytest
|
||||
|
||||
from src.constants import WEB_FETCH_SOFT_MAX_BYTES, WEB_FETCH_HARD_MAX_BYTES
|
||||
from services.search import content as content_mod
|
||||
|
||||
|
||||
class _FakeStream:
|
||||
"""Stands in for the httpx.stream(...) context manager."""
|
||||
|
||||
def __init__(self, body: bytes, content_type="text/plain", content_length=None,
|
||||
status_code=200, chunk=8192):
|
||||
self._body = body
|
||||
self._chunk = chunk
|
||||
self.status_code = status_code
|
||||
self.encoding = "utf-8"
|
||||
self.url = "https://example.com/x"
|
||||
self.headers = {"Content-Type": content_type}
|
||||
if content_length is not None:
|
||||
self.headers["content-length"] = str(content_length)
|
||||
self.body_reads = 0
|
||||
|
||||
def iter_bytes(self):
|
||||
for i in range(0, len(self._body), self._chunk):
|
||||
self.body_reads += 1
|
||||
yield self._body[i:i + self._chunk]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def no_cache(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr(content_mod, "CONTENT_CACHE_DIR", tmp_path)
|
||||
monkeypatch.setattr(content_mod, "_cache_result", lambda *a, **k: None)
|
||||
monkeypatch.setattr(content_mod, "_public_http_url", lambda u: True)
|
||||
|
||||
|
||||
def _patch_stream(monkeypatch, fake):
|
||||
@contextmanager
|
||||
def fake_stream(method, url, **kwargs):
|
||||
yield fake
|
||||
monkeypatch.setattr(content_mod.httpx, "stream", fake_stream)
|
||||
return fake
|
||||
|
||||
|
||||
def test_body_under_cap_is_untouched(monkeypatch, no_cache):
|
||||
_patch_stream(monkeypatch, _FakeStream(b"hello world"))
|
||||
r = content_mod.fetch_webpage_content("https://example.com/a.txt")
|
||||
assert r["success"] is True
|
||||
assert r["content"] == "hello world"
|
||||
assert r["truncated"] is False
|
||||
assert r["fetched_bytes"] == len(b"hello world")
|
||||
|
||||
|
||||
def test_body_over_soft_cap_truncates_with_flags(monkeypatch, no_cache):
|
||||
body = b"x" * (WEB_FETCH_SOFT_MAX_BYTES + 50_000)
|
||||
_patch_stream(monkeypatch, _FakeStream(body, content_length=len(body)))
|
||||
r = content_mod.fetch_webpage_content("https://example.com/big.txt")
|
||||
assert r["truncated"] is True
|
||||
assert r["fetched_bytes"] == WEB_FETCH_SOFT_MAX_BYTES
|
||||
assert r["total_bytes"] == len(body)
|
||||
assert len(r["content"]) == WEB_FETCH_SOFT_MAX_BYTES
|
||||
|
||||
|
||||
def test_max_bytes_override_raises_budget(monkeypatch, no_cache):
|
||||
body = b"y" * (WEB_FETCH_SOFT_MAX_BYTES + 50_000)
|
||||
_patch_stream(monkeypatch, _FakeStream(body))
|
||||
r = content_mod.fetch_webpage_content(
|
||||
"https://example.com/big.txt", max_bytes=len(body) + 1
|
||||
)
|
||||
assert r["truncated"] is False
|
||||
assert r["fetched_bytes"] == len(body)
|
||||
|
||||
|
||||
def test_override_is_clamped_to_hard_cap(monkeypatch, no_cache):
|
||||
# Ask for more than the ceiling; the effective budget must be the ceiling.
|
||||
fake = _patch_stream(monkeypatch, _FakeStream(b"z" * 10, chunk=4))
|
||||
r = content_mod.fetch_webpage_content(
|
||||
"https://example.com/a.txt", max_bytes=WEB_FETCH_HARD_MAX_BYTES * 10
|
||||
)
|
||||
assert r["success"] is True
|
||||
# The clamp itself: effective cap recorded in the cache key path is the
|
||||
# hard cap, and a declared body over the ceiling is refused regardless.
|
||||
big = _FakeStream(b"", content_length=WEB_FETCH_HARD_MAX_BYTES + 1)
|
||||
_patch_stream(monkeypatch, big)
|
||||
r = content_mod.fetch_webpage_content(
|
||||
"https://example.com/huge.bin", max_bytes=WEB_FETCH_HARD_MAX_BYTES * 10
|
||||
)
|
||||
assert r["success"] is False
|
||||
assert "TooLarge" in r["error"]
|
||||
assert big.body_reads == 0 # refused before buffering
|
||||
|
||||
|
||||
def test_declared_over_hard_cap_refused_before_buffering(monkeypatch, no_cache):
|
||||
fake = _FakeStream(b"irrelevant", content_length=WEB_FETCH_HARD_MAX_BYTES + 1)
|
||||
_patch_stream(monkeypatch, fake)
|
||||
r = content_mod.fetch_webpage_content("https://example.com/huge.iso")
|
||||
assert r["success"] is False
|
||||
assert "TooLarge" in r["error"]
|
||||
assert fake.body_reads == 0
|
||||
|
||||
|
||||
def test_truncated_pdf_is_an_error_not_garbage(monkeypatch, no_cache):
|
||||
body = b"%PDF-1.4 " + b"p" * (WEB_FETCH_SOFT_MAX_BYTES + 10)
|
||||
_patch_stream(monkeypatch, _FakeStream(body, content_type="application/pdf"))
|
||||
r = content_mod.fetch_webpage_content("https://example.com/big.pdf")
|
||||
assert r["success"] is False
|
||||
assert "TooLarge" in r["error"]
|
||||
|
||||
|
||||
def test_fetch_requests_identity_encoding(monkeypatch, no_cache):
|
||||
# Compressed responses can decode to far more than Content-Length, so the
|
||||
# streamed cap and the hard-cap preflight are only honest when we refuse
|
||||
# transfer compression. Pin that the fetch advertises identity, not gzip.
|
||||
seen = {}
|
||||
|
||||
@contextmanager
|
||||
def fake_stream(method, url, **kwargs):
|
||||
seen["headers"] = kwargs.get("headers") or {}
|
||||
yield _FakeStream(b"hello")
|
||||
monkeypatch.setattr(content_mod.httpx, "stream", fake_stream)
|
||||
|
||||
content_mod.fetch_webpage_content("https://example.com/a.txt")
|
||||
assert seen["headers"].get("Accept-Encoding") == "identity"
|
||||
|
||||
|
||||
def test_rejects_compressed_response_that_ignored_identity(monkeypatch, no_cache):
|
||||
# We request Accept-Encoding: identity, but a server can ignore it and send
|
||||
# gzip anyway. httpx would decode it, so a tiny compressed body could balloon
|
||||
# past the cap in one decoded chunk. Refuse before reading the body.
|
||||
fake = _FakeStream(b"x" * 5000, content_length=40)
|
||||
fake.headers["content-encoding"] = "gzip"
|
||||
_patch_stream(monkeypatch, fake)
|
||||
r = content_mod.fetch_webpage_content("https://example.com/a.txt")
|
||||
assert r["success"] is False
|
||||
assert "Content-Encoding" in r["error"] or "compressed" in r["error"]
|
||||
assert fake.body_reads == 0 # refused before decoding any body
|
||||
|
||||
|
||||
def test_oversized_title_does_not_hide_partial_notice(monkeypatch):
|
||||
# The partial-content notice is the PR's core contract; an untrusted,
|
||||
# oversized page title must not push it past MAX_OUTPUT_CHARS.
|
||||
import asyncio
|
||||
from src.agent_tools.web_tools import WebFetchTool
|
||||
from src.constants import MAX_OUTPUT_CHARS
|
||||
|
||||
def fake_fetch(url, timeout=10, max_bytes=None):
|
||||
return {
|
||||
"content": "partial body",
|
||||
"title": "T" * (MAX_OUTPUT_CHARS + 5_000),
|
||||
"error": "",
|
||||
"truncated": True,
|
||||
"fetched_bytes": WEB_FETCH_SOFT_MAX_BYTES,
|
||||
"total_bytes": 9_000_000,
|
||||
}
|
||||
|
||||
import src.search.content as alias_mod
|
||||
monkeypatch.setattr(alias_mod, "fetch_webpage_content", fake_fetch)
|
||||
|
||||
out = asyncio.run(WebFetchTool().execute(
|
||||
json.dumps({"url": "https://example.com/big.txt"}), ctx={}
|
||||
))
|
||||
assert out["exit_code"] == 0
|
||||
assert out["output"].startswith("[partial content:")
|
||||
assert '"full": true' in out["output"]
|
||||
|
||||
|
||||
def test_tool_layer_emits_partial_notice_and_parses_full(monkeypatch):
|
||||
import asyncio
|
||||
from src.agent_tools.web_tools import WebFetchTool
|
||||
|
||||
calls = {}
|
||||
|
||||
def fake_fetch(url, timeout=10, max_bytes=None):
|
||||
calls["max_bytes"] = max_bytes
|
||||
return {
|
||||
"content": "partial body",
|
||||
"title": "Big File",
|
||||
"error": "",
|
||||
"truncated": True,
|
||||
"fetched_bytes": WEB_FETCH_SOFT_MAX_BYTES,
|
||||
"total_bytes": 5_000_000,
|
||||
}
|
||||
|
||||
import src.search.content as alias_mod
|
||||
monkeypatch.setattr(alias_mod, "fetch_webpage_content", fake_fetch)
|
||||
|
||||
out = asyncio.run(WebFetchTool().execute(
|
||||
json.dumps({"url": "https://example.com/big.txt"}), ctx={}
|
||||
))
|
||||
assert out["exit_code"] == 0
|
||||
assert "[partial content:" in out["output"]
|
||||
assert '"full": true' in out["output"]
|
||||
assert calls["max_bytes"] is None
|
||||
|
||||
asyncio.run(WebFetchTool().execute(
|
||||
json.dumps({"url": "https://example.com/big.txt", "full": True}), ctx={}
|
||||
))
|
||||
assert calls["max_bytes"] == WEB_FETCH_HARD_MAX_BYTES
|
||||
@@ -0,0 +1,54 @@
|
||||
"""Guard: every public webhook emitter goes through the manager.
|
||||
|
||||
Public emitters in `routes/` must schedule their fire through
|
||||
`webhook_manager.fire_and_forget(...)` (or `_spawn_tracked`). A bare
|
||||
`asyncio.create_task(webhook_manager.fire(...))` escapes
|
||||
`WebhookManager._bg_tasks`, so asyncio only holds a weak reference to the
|
||||
delivery task and the GC can collect it before it sends — silently dropping
|
||||
the webhook. Catching this with a scan stops a regression from sneaking
|
||||
back in via a copy-paste.
|
||||
"""
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
ROUTES_DIR = Path(__file__).resolve().parent.parent / "routes"
|
||||
|
||||
|
||||
def _untracked_fire_calls(tree: ast.AST) -> list[tuple[int, str]]:
|
||||
"""Return (lineno, snippet) for any asyncio.create_task(webhook_manager.fire(...))."""
|
||||
hits: list[tuple[int, str]] = []
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.Call):
|
||||
continue
|
||||
func = node.func
|
||||
if not (isinstance(func, ast.Attribute) and func.attr == "create_task"):
|
||||
continue
|
||||
if not (isinstance(func.value, ast.Name) and func.value.id == "asyncio"):
|
||||
continue
|
||||
if not node.args:
|
||||
continue
|
||||
inner = node.args[0]
|
||||
if not isinstance(inner, ast.Call):
|
||||
continue
|
||||
inner_func = inner.func
|
||||
if (
|
||||
isinstance(inner_func, ast.Attribute)
|
||||
and inner_func.attr == "fire"
|
||||
and isinstance(inner_func.value, ast.Name)
|
||||
and inner_func.value.id == "webhook_manager"
|
||||
):
|
||||
hits.append((node.lineno, ast.unparse(node)))
|
||||
return hits
|
||||
|
||||
|
||||
def test_no_untracked_webhook_fire_in_routes():
|
||||
offenders: list[str] = []
|
||||
for path in ROUTES_DIR.rglob("*.py"):
|
||||
tree = ast.parse(path.read_text(), filename=str(path))
|
||||
for lineno, snippet in _untracked_fire_calls(tree):
|
||||
offenders.append(f"{path.relative_to(ROUTES_DIR.parent)}:{lineno}: {snippet}")
|
||||
assert not offenders, (
|
||||
"Public webhook emitters must use webhook_manager.fire_and_forget(...) "
|
||||
"so the delivery task is tracked in WebhookManager._bg_tasks. Found "
|
||||
"untracked emitter(s):\n " + "\n ".join(offenders)
|
||||
)
|
||||
@@ -0,0 +1,574 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Build the oversized test-file split plan for issue #3983.
|
||||
|
||||
The output is a planning document only. It does not move tests, rewrite
|
||||
assertions, extract helpers, or change CI.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from collections import Counter
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
TESTS_DIR = ROOT / "tests"
|
||||
OUTPUT = TESTS_DIR / "OVERSIZED_TEST_SPLIT_PLAN.md"
|
||||
RAW_OUTPUT = Path("/tmp/oversized-test-file-metrics.json")
|
||||
|
||||
LARGE_LINE_THRESHOLD = 300
|
||||
LARGE_NODE_THRESHOLD = 20
|
||||
TOP_LIMIT = 30
|
||||
|
||||
HIGH_RISK_SIGNALS = {"route/api", "db/session", "import-state", "security"}
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class FileMetric:
|
||||
path: str
|
||||
lines: int
|
||||
nonblank: int
|
||||
test_defs: int
|
||||
test_classes: int
|
||||
collected: int
|
||||
area: str
|
||||
sub_area: str
|
||||
signals: tuple[str, ...]
|
||||
|
||||
|
||||
def read_text(path: Path) -> str:
|
||||
return path.read_text(encoding="utf-8", errors="replace")
|
||||
|
||||
|
||||
def count_ast_tests(text: str) -> tuple[int, int]:
|
||||
tree = ast.parse(text)
|
||||
test_defs = 0
|
||||
test_classes = 0
|
||||
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
|
||||
if node.name.startswith("test_"):
|
||||
test_defs += 1
|
||||
elif isinstance(node, ast.ClassDef):
|
||||
if node.name.startswith("Test"):
|
||||
test_classes += 1
|
||||
|
||||
return test_defs, test_classes
|
||||
|
||||
|
||||
def load_taxonomy_classifier():
|
||||
sys.path.insert(0, str(ROOT))
|
||||
from tests._taxonomy import classify_test_path
|
||||
|
||||
return classify_test_path
|
||||
|
||||
|
||||
def classify(path: Path, classify_test_path) -> tuple[str, str]:
|
||||
rel_path = Path(path.relative_to(ROOT).as_posix())
|
||||
|
||||
try:
|
||||
result = classify_test_path(rel_path)
|
||||
except Exception:
|
||||
return "unknown", "unknown"
|
||||
|
||||
return getattr(result, "area", "unknown"), getattr(result, "sub_area", "unknown")
|
||||
|
||||
|
||||
def collect_node_counts() -> Counter[str]:
|
||||
cmd = [
|
||||
sys.executable,
|
||||
"-m",
|
||||
"pytest",
|
||||
"--collect-only",
|
||||
"-q",
|
||||
"tests",
|
||||
]
|
||||
env = dict(os.environ)
|
||||
env["PY_COLORS"] = "0"
|
||||
|
||||
result = subprocess.run(
|
||||
cmd,
|
||||
cwd=ROOT,
|
||||
env=env,
|
||||
text=True,
|
||||
capture_output=True,
|
||||
)
|
||||
|
||||
if result.returncode != 0:
|
||||
print(result.stdout)
|
||||
print(result.stderr, file=sys.stderr)
|
||||
raise SystemExit(result.returncode)
|
||||
|
||||
counts: Counter[str] = Counter()
|
||||
for line in result.stdout.splitlines():
|
||||
line = line.strip()
|
||||
if "::" not in line:
|
||||
continue
|
||||
if not line.startswith("tests/"):
|
||||
continue
|
||||
file_path = line.split("::", 1)[0]
|
||||
counts[file_path] += 1
|
||||
|
||||
return counts
|
||||
|
||||
|
||||
def detect_signals(text: str, path: str) -> tuple[str, ...]:
|
||||
signal_patterns = {
|
||||
"route/api": [
|
||||
r"\bTestClient\b",
|
||||
r"\bapp\.",
|
||||
r"\broutes\.",
|
||||
r"\bfrom routes\b",
|
||||
r"\bimport routes\b",
|
||||
],
|
||||
"db/session": [
|
||||
r"\bSessionLocal\b",
|
||||
r"\bsqlite\b",
|
||||
r"\bDATABASE_URL\b",
|
||||
r"\bcore\.database\b",
|
||||
r"\bdb\.query\b",
|
||||
r"\bcommit\(",
|
||||
],
|
||||
"import-state": [
|
||||
r"\bsys\.modules\b",
|
||||
r"\bimportlib\b",
|
||||
r"\bclear_module\b",
|
||||
r"\bpreserve_import_state\b",
|
||||
r"\bmonkeypatch\.setitem\b",
|
||||
],
|
||||
"security": [
|
||||
r"\bsecurity\b",
|
||||
r"\bssrf\b",
|
||||
r"\bpath traversal\b",
|
||||
r"\bcsrf\b",
|
||||
r"\bpermission\b",
|
||||
],
|
||||
"filesystem": [
|
||||
r"\btmp_path\b",
|
||||
r"\bTemporaryDirectory\b",
|
||||
r"\bPath\(",
|
||||
r"\bmkdir\b",
|
||||
r"\bwrite_text\b",
|
||||
r"\bread_text\b",
|
||||
],
|
||||
"subprocess/script": [
|
||||
r"\bsubprocess\b",
|
||||
r"\brunpy\b",
|
||||
r"\bload_script\b",
|
||||
r"\bsys\.argv\b",
|
||||
],
|
||||
"async/threading": [
|
||||
r"\basyncio\b",
|
||||
r"\bthreading\b",
|
||||
r"\bconcurrent\.futures\b",
|
||||
r"\bThreadPoolExecutor\b",
|
||||
],
|
||||
"ui/static": [
|
||||
r"\bstatic/",
|
||||
r"\bjsdom\b",
|
||||
r"\bnode\b",
|
||||
r"\.js\b",
|
||||
],
|
||||
}
|
||||
|
||||
signals = []
|
||||
for name, patterns in signal_patterns.items():
|
||||
if any(re.search(pattern, text, flags=re.IGNORECASE) for pattern in patterns):
|
||||
signals.append(name)
|
||||
|
||||
if path.startswith("tests/cli/"):
|
||||
signals.append("cli-directory")
|
||||
|
||||
return tuple(signals)
|
||||
|
||||
|
||||
def metric_for(path: Path, node_counts: Counter[str], classify_test_path) -> FileMetric:
|
||||
rel = path.relative_to(ROOT).as_posix()
|
||||
text = read_text(path)
|
||||
lines = len(text.splitlines())
|
||||
nonblank = sum(1 for line in text.splitlines() if line.strip())
|
||||
test_defs, test_classes = count_ast_tests(text)
|
||||
area, sub_area = classify(path, classify_test_path)
|
||||
|
||||
return FileMetric(
|
||||
path=rel,
|
||||
lines=lines,
|
||||
nonblank=nonblank,
|
||||
test_defs=test_defs,
|
||||
test_classes=test_classes,
|
||||
collected=node_counts.get(rel, 0),
|
||||
area=area,
|
||||
sub_area=sub_area,
|
||||
signals=detect_signals(text, rel),
|
||||
)
|
||||
|
||||
|
||||
def test_files() -> list[Path]:
|
||||
return sorted(TESTS_DIR.rglob("test_*.py"))
|
||||
|
||||
|
||||
def as_metric_row(metric: FileMetric) -> str:
|
||||
signals = ", ".join(metric.signals) if metric.signals else "-"
|
||||
return (
|
||||
f"| `{metric.path}` | {metric.lines} | {metric.collected} | "
|
||||
f"{metric.test_defs} | {metric.test_classes} | "
|
||||
f"{metric.area} | {metric.sub_area} | {signals} |"
|
||||
)
|
||||
|
||||
|
||||
def metric_table(title: str, metrics: list[FileMetric]) -> list[str]:
|
||||
lines = [
|
||||
f"## {title}",
|
||||
"",
|
||||
"| File | Lines | Collected tests | Test defs | Test classes | Area | Sub-area | Signals |",
|
||||
"|---|---:|---:|---:|---:|---|---|---|",
|
||||
]
|
||||
lines.extend(as_metric_row(metric) for metric in metrics)
|
||||
lines.append("")
|
||||
return lines
|
||||
|
||||
|
||||
def candidate_metrics(metrics: list[FileMetric]) -> list[FileMetric]:
|
||||
return [
|
||||
metric
|
||||
for metric in metrics
|
||||
if metric.lines >= LARGE_LINE_THRESHOLD
|
||||
or metric.collected >= LARGE_NODE_THRESHOLD
|
||||
]
|
||||
|
||||
|
||||
def include_reasons(metric: FileMetric) -> str:
|
||||
reasons = []
|
||||
if metric.lines >= LARGE_LINE_THRESHOLD:
|
||||
reasons.append(f"{metric.lines} lines")
|
||||
if metric.collected >= LARGE_NODE_THRESHOLD:
|
||||
reasons.append(f"{metric.collected} collected tests")
|
||||
return ", ".join(reasons)
|
||||
|
||||
|
||||
def risk_notes(metric: FileMetric) -> str:
|
||||
if not metric.signals:
|
||||
return "No obvious setup signals from static scan."
|
||||
return ", ".join(metric.signals)
|
||||
|
||||
|
||||
def suggested_handling(metric: FileMetric) -> str:
|
||||
if HIGH_RISK_SIGNALS.intersection(metric.signals):
|
||||
return "Defer mechanical split until setup/risk boundaries are mapped."
|
||||
if metric.collected >= LARGE_NODE_THRESHOLD:
|
||||
return "Good first manual-review candidate if test themes are cohesive."
|
||||
return "Plan split boundaries before editing."
|
||||
|
||||
|
||||
def candidate_section(metrics: list[FileMetric]) -> list[str]:
|
||||
lines = [
|
||||
"## Split planning candidates",
|
||||
"",
|
||||
"This section is generated from metrics, not from manual judgement.",
|
||||
"Files are included when they meet at least one threshold:",
|
||||
"",
|
||||
f"- at least {LARGE_LINE_THRESHOLD} physical lines; or",
|
||||
f"- at least {LARGE_NODE_THRESHOLD} collected pytest items.",
|
||||
"",
|
||||
"These are planning candidates only. A later split PR still needs a focused manual review of each file before moving tests.",
|
||||
"",
|
||||
"| File | Why included | Setup/risk signals | Suggested handling |",
|
||||
"|---|---|---|---|",
|
||||
]
|
||||
|
||||
for metric in metrics:
|
||||
lines.append(
|
||||
f"| `{metric.path}` | {include_reasons(metric)} | "
|
||||
f"{risk_notes(metric)} | {suggested_handling(metric)} |"
|
||||
)
|
||||
|
||||
lines.append("")
|
||||
return lines
|
||||
|
||||
|
||||
def first_manual_review_section(metrics: list[FileMetric]) -> list[str]:
|
||||
low_risk = [
|
||||
metric
|
||||
for metric in metrics
|
||||
if metric.area != "uncategorized"
|
||||
and not HIGH_RISK_SIGNALS.intersection(metric.signals)
|
||||
]
|
||||
low_risk = sorted(low_risk, key=lambda m: (m.collected, m.lines), reverse=True)
|
||||
|
||||
lines = [
|
||||
"## Suggested first manual-review candidates",
|
||||
"",
|
||||
"These are not automatic split approvals. They are categorized candidates with enough size/collection value and no route/API, DB/session, import-state, or security signal from the static scan.",
|
||||
"",
|
||||
"Files still in the `uncategorized` taxonomy area are listed separately below so taxonomy review does not get mixed into the first split decision.",
|
||||
"",
|
||||
"| File | Lines | Collected tests | Area | Sub-area | Signals | Why this is a candidate |",
|
||||
"|---|---:|---:|---|---|---|---|",
|
||||
]
|
||||
|
||||
if not low_risk:
|
||||
lines.append("| _None_ | - | - | - | - | - | - |")
|
||||
|
||||
for metric in low_risk[:10]:
|
||||
signals = ", ".join(metric.signals) if metric.signals else "-"
|
||||
lines.append(
|
||||
f"| `{metric.path}` | {metric.lines} | {metric.collected} | "
|
||||
f"{metric.area} | {metric.sub_area} | {signals} | {include_reasons(metric)} |"
|
||||
)
|
||||
|
||||
lines.append("")
|
||||
return lines
|
||||
|
||||
|
||||
def taxonomy_gap_section(metrics: list[FileMetric]) -> list[str]:
|
||||
uncategorized = [
|
||||
metric
|
||||
for metric in metrics
|
||||
if metric.area == "uncategorized"
|
||||
]
|
||||
uncategorized = sorted(
|
||||
uncategorized,
|
||||
key=lambda m: (m.collected, m.lines),
|
||||
reverse=True,
|
||||
)
|
||||
|
||||
lines = [
|
||||
"## Taxonomy coverage gaps among split candidates",
|
||||
"",
|
||||
"`uncategorized` is a current taxonomy area, not a builder failure.",
|
||||
"This plan does not reclassify tests because taxonomy changes should be reviewed separately from oversized-file split planning.",
|
||||
"",
|
||||
"Before using any of these files as a split target, first decide whether the taxonomy should be refined in a separate focused issue/PR.",
|
||||
"",
|
||||
"| File | Lines | Collected tests | Sub-area | Signals | Suggested follow-up |",
|
||||
"|---|---:|---:|---|---|---|",
|
||||
]
|
||||
|
||||
if not uncategorized:
|
||||
lines.append("| _None_ | - | - | - | - | - |")
|
||||
|
||||
for metric in uncategorized:
|
||||
signals = ", ".join(metric.signals) if metric.signals else "-"
|
||||
follow_up = "Review taxonomy mapping before using as a split target."
|
||||
if HIGH_RISK_SIGNALS.intersection(metric.signals):
|
||||
follow_up = "Review taxonomy and setup/risk boundaries before any split."
|
||||
lines.append(
|
||||
f"| `{metric.path}` | {metric.lines} | {metric.collected} | "
|
||||
f"{metric.sub_area} | {signals} | {follow_up} |"
|
||||
)
|
||||
|
||||
lines.append("")
|
||||
return lines
|
||||
|
||||
|
||||
def deferred_section(metrics: list[FileMetric]) -> list[str]:
|
||||
deferred = [
|
||||
metric
|
||||
for metric in metrics
|
||||
if HIGH_RISK_SIGNALS.intersection(metric.signals)
|
||||
]
|
||||
deferred = sorted(deferred, key=lambda m: (m.collected, m.lines), reverse=True)
|
||||
|
||||
lines = [
|
||||
"## High-risk candidates to defer first",
|
||||
"",
|
||||
"These files may still be split later, but not as the first implementation slice without a separate manual boundary review.",
|
||||
"",
|
||||
"| File | Lines | Collected tests | High-risk signals |",
|
||||
"|---|---:|---:|---|",
|
||||
]
|
||||
|
||||
for metric in deferred[:15]:
|
||||
signals = ", ".join(sorted(HIGH_RISK_SIGNALS.intersection(metric.signals)))
|
||||
lines.append(
|
||||
f"| `{metric.path}` | {metric.lines} | {metric.collected} | {signals} |"
|
||||
)
|
||||
|
||||
lines.append("")
|
||||
return lines
|
||||
|
||||
|
||||
def write_distribution(
|
||||
lines: list[str],
|
||||
title: str,
|
||||
values: Counter[str],
|
||||
*,
|
||||
min_count: int = 1,
|
||||
) -> None:
|
||||
displayed = [
|
||||
(value, count)
|
||||
for value, count in sorted(values.items())
|
||||
if count >= min_count
|
||||
]
|
||||
omitted_values = sum(1 for count in values.values() if count < min_count)
|
||||
omitted_files = sum(count for count in values.values() if count < min_count)
|
||||
|
||||
lines.extend([
|
||||
f"{title}:",
|
||||
"",
|
||||
"| Value | Files |",
|
||||
"|---|---:|",
|
||||
])
|
||||
for value, count in displayed:
|
||||
lines.append(f"| {value} | {count} |")
|
||||
|
||||
if omitted_values:
|
||||
lines.extend([
|
||||
"",
|
||||
f"Values below {min_count} files: {omitted_values} values covering {omitted_files} files.",
|
||||
])
|
||||
|
||||
lines.append("")
|
||||
|
||||
|
||||
def write_report(metrics: list[FileMetric], node_count_total: int) -> None:
|
||||
by_lines = sorted(metrics, key=lambda m: (m.lines, m.collected), reverse=True)
|
||||
by_collected = sorted(metrics, key=lambda m: (m.collected, m.lines), reverse=True)
|
||||
candidates = sorted(
|
||||
candidate_metrics(metrics),
|
||||
key=lambda m: (m.collected, m.lines),
|
||||
reverse=True,
|
||||
)
|
||||
|
||||
areas = Counter(metric.area for metric in metrics)
|
||||
sub_areas = Counter(metric.sub_area for metric in metrics)
|
||||
|
||||
lines = [
|
||||
"# Oversized Test File Split Plan",
|
||||
"",
|
||||
"## Purpose",
|
||||
"",
|
||||
"This document plans future oversized test-file splits using current repo data.",
|
||||
"It does not move files, rewrite assertions, extract helpers, or change CI.",
|
||||
"",
|
||||
"## Roadmap context",
|
||||
"",
|
||||
"- Issue: #3983",
|
||||
"- Parent tracker: #2523",
|
||||
"- Follows #3973 / #3982, the report-only order-sensitivity diagnostics slice.",
|
||||
"",
|
||||
"## Methodology",
|
||||
"",
|
||||
"Metrics were generated from the current test tree using:",
|
||||
"",
|
||||
"- physical line counts for every recursive `test_*.py` file under `tests/`;",
|
||||
"- AST counts for `test_*` functions and `Test*` classes;",
|
||||
"- one `pytest --collect-only -q tests` run to count collected items per file;",
|
||||
"- current taxonomy classification from `tests._taxonomy.classify_test_path`; and",
|
||||
"- static setup-signal scans for route/API, DB/session, import-state, security, filesystem, subprocess/script, async/threading, and UI/static indicators.",
|
||||
"",
|
||||
"Static signals are not proof of risk. They are review prompts.",
|
||||
"Future split PRs must still inspect each file manually before editing.",
|
||||
"",
|
||||
"## Current summary",
|
||||
"",
|
||||
f"- test files scanned: {len(metrics)}",
|
||||
f"- collected pytest items counted: {node_count_total}",
|
||||
f"- large-file threshold: {LARGE_LINE_THRESHOLD} lines",
|
||||
f"- large-collected threshold: {LARGE_NODE_THRESHOLD} collected items",
|
||||
"",
|
||||
]
|
||||
|
||||
write_distribution(lines, "Area distribution", areas)
|
||||
write_distribution(lines, "Sub-area distribution", sub_areas, min_count=2)
|
||||
|
||||
lines.extend(metric_table("Top files by collected pytest items", by_collected[:TOP_LIMIT]))
|
||||
lines.extend(metric_table("Top files by physical line count", by_lines[:TOP_LIMIT]))
|
||||
lines.extend(candidate_section(candidates))
|
||||
lines.extend(taxonomy_gap_section(candidates))
|
||||
lines.extend(first_manual_review_section(candidates))
|
||||
lines.extend(deferred_section(candidates))
|
||||
|
||||
lines.extend([
|
||||
"## Rules for future split PRs",
|
||||
"",
|
||||
"- One file or one coherent file-family per PR.",
|
||||
"- No assertion rewrites mixed with file moves.",
|
||||
"- No helper extraction mixed with file moves.",
|
||||
"- No production code changes.",
|
||||
"- No CI workflow changes.",
|
||||
"- Preserve existing markers and taxonomy unless the split issue explicitly says otherwise.",
|
||||
"- Validate the original file's collected tests before and after the split.",
|
||||
"- Validate any neighboring taxonomy/focused-runner behavior if paths change.",
|
||||
"- Treat files with route/API, DB/session, import-state, or security signals as higher-risk until manually reviewed.",
|
||||
"",
|
||||
"## Suggested next step",
|
||||
"",
|
||||
"Use this plan to choose the first actual oversized-file split issue.",
|
||||
"The first split should prefer a file with high review value and low setup risk.",
|
||||
"Do not start a split PR from this planning issue alone if the file's boundaries are still ambiguous.",
|
||||
"",
|
||||
"## Reproduction command",
|
||||
"",
|
||||
"This document was generated with:",
|
||||
"",
|
||||
"```bash",
|
||||
".venv/bin/python tests/tools/build_oversized_test_split_plan.py",
|
||||
"```",
|
||||
"",
|
||||
"## Freshness check",
|
||||
"",
|
||||
"After editing the builder or rebasing the branch, regenerate the plan and confirm no unexpected plan drift:",
|
||||
"",
|
||||
"```bash",
|
||||
".venv/bin/python tests/tools/build_oversized_test_split_plan.py",
|
||||
"git diff --exit-code -- tests/OVERSIZED_TEST_SPLIT_PLAN.md",
|
||||
"```",
|
||||
"",
|
||||
])
|
||||
|
||||
OUTPUT.write_text("\n".join(lines), encoding="utf-8")
|
||||
|
||||
|
||||
def write_raw(metrics: list[FileMetric]) -> None:
|
||||
raw = [
|
||||
{
|
||||
"area": metric.area,
|
||||
"collected": metric.collected,
|
||||
"lines": metric.lines,
|
||||
"nonblank": metric.nonblank,
|
||||
"path": metric.path,
|
||||
"signals": list(metric.signals),
|
||||
"sub_area": metric.sub_area,
|
||||
"test_classes": metric.test_classes,
|
||||
"test_defs": metric.test_defs,
|
||||
}
|
||||
for metric in metrics
|
||||
]
|
||||
RAW_OUTPUT.write_text(json.dumps(raw, indent=2, sort_keys=True), encoding="utf-8")
|
||||
|
||||
|
||||
def assert_taxonomy_worked(metrics: list[FileMetric]) -> None:
|
||||
if not metrics:
|
||||
raise SystemExit("ERROR: no test files were scanned")
|
||||
|
||||
unknown = sum(1 for metric in metrics if metric.area == "unknown")
|
||||
if unknown == len(metrics):
|
||||
raise SystemExit("ERROR: taxonomy classification returned unknown for every file")
|
||||
|
||||
|
||||
def main() -> int:
|
||||
if not TESTS_DIR.exists():
|
||||
print("ERROR: tests/ directory not found", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
classify_test_path = load_taxonomy_classifier()
|
||||
node_counts = collect_node_counts()
|
||||
metrics = [metric_for(path, node_counts, classify_test_path) for path in test_files()]
|
||||
|
||||
assert_taxonomy_worked(metrics)
|
||||
write_report(metrics, sum(node_counts.values()))
|
||||
write_raw(metrics)
|
||||
|
||||
print(f"Wrote {OUTPUT.relative_to(ROOT)}")
|
||||
print(f"Wrote {RAW_OUTPUT}")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main())
|
||||
Reference in New Issue
Block a user