From cf78f28ceee07e50cd25e0d4b380c4fbc929b6bd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 20:49:12 +0900 Subject: [PATCH 01/21] feat: harden live macOS runtime and governance --- .env.example | 7 +- .github/workflows/pr-governance.yml | 40 ++++-- README.md | 119 +++++++----------- .../alembic/versions/0011_email_read_state.py | 30 +++-- backend/api/auth.py | 41 ++++-- backend/api/emails.py | 1 + backend/api/search.py | 6 +- backend/core/config.py | 12 +- backend/scripts/bootstrap_db.py | 4 - .../disksage_copy_readiness_handoff.py | 6 +- backend/scripts/private_mail_http_smoke.py | 52 +++----- backend/services/batch_embedding_service.py | 3 +- backend/services/email_import_service.py | 24 +++- backend/services/embedding.py | 6 +- backend/services/llm_provider_selection.py | 37 +++++- backend/services/llm_provider_urls.py | 6 +- backend/services/llm_service.py | 118 ++++++++++++----- .../project_graph/extractor_registry.py | 12 +- backend/tests/test_alembic_migrations.py | 9 ++ backend/tests/test_auth_real.py | 22 ++++ backend/tests/test_bootstrap_db.py | 11 ++ backend/tests/test_config.py | 9 ++ .../test_disksage_copy_readiness_handoff.py | 2 +- backend/tests/test_email_import_service.py | 11 ++ backend/tests/test_emails_api.py | 9 +- backend/tests/test_embedding.py | 28 ++++- backend/tests/test_llm_provider_selection.py | 40 +++++- backend/tests/test_llm_provider_urls.py | 17 +++ backend/tests/test_llm_service.py | 40 ++++++ backend/tests/test_local_http.py | 9 ++ backend/tests/test_private_mail_http_smoke.py | 9 ++ backend/tests/test_release_governance.py | 11 ++ backend/tests/test_repo_hygiene.py | 12 +- docker-compose.macos.yml | 24 ++++ docker-compose.yml | 1 + ...001-local-llm-and-orchestrator-boundary.md | 58 +++++++++ ...0002-compound-snake-case-database-names.md | 48 +++++++ .../adr/0003-pr-checks-and-non-admin-merge.md | 33 +++++ ...nput-safety-and-evidence-first-judgment.md | 39 ++++++ docs/adr/0005-keyverse-oidc-trust-boundary.md | 35 ++++++ ...0006-privileged-workflow-archive-safety.md | 37 ++++++ docs/architecture/kg-extractor-seam.md | 9 +- scripts/naruon_compose.sh | 89 ++++++++++++- 43 files changed, 925 insertions(+), 211 deletions(-) create mode 100644 docker-compose.macos.yml create mode 100644 docs/adr/0001-local-llm-and-orchestrator-boundary.md create mode 100644 docs/adr/0002-compound-snake-case-database-names.md create mode 100644 docs/adr/0003-pr-checks-and-non-admin-merge.md create mode 100644 docs/adr/0004-input-safety-and-evidence-first-judgment.md create mode 100644 docs/adr/0005-keyverse-oidc-trust-boundary.md create mode 100644 docs/adr/0006-privileged-workflow-archive-safety.md diff --git a/.env.example b/.env.example index e121dc9ff..efb281b91 100644 --- a/.env.example +++ b/.env.example @@ -7,6 +7,7 @@ ENABLE_PROMETHEUS_METRICS=false # AI features. Leave blank to disable LLM/embedding-backed flows locally. OPENAI_API_KEY= +OPENAI_EMBEDDING_BASE_URL= OPENAI_EMBEDDING_MODEL=text-embedding-3-small OPENAI_MODEL=gpt-4o @@ -20,11 +21,13 @@ ALLOW_DOCKER_BACKEND_INTERNAL_URL=1 # 기본 .env는 공유값을 유지하고, 로컬 모델 경로는 오버레이에서만 바꿔 적용합니다. # Linux 기본 경로에서는 docker-compose.yml의 ollama 서비스를 계속 사용합니다. #NARUON_MLX_OPENAI_API_KEY=mlx -#NARUON_MLX_BASE_URL=http://host.docker.internal:11434/v1 +#NARUON_MLX_BASE_URL=http://host.docker.internal:8080/v1 +#NARUON_MLX_EMBEDDING_BASE_URL=http://host.docker.internal:8082/v1 #NARUON_MLX_ALLOWED_LLM_BASE_URL_HOSTS=localhost,127.0.0.1,host.docker.internal #NARUON_MLX_EMBEDDING_MODEL=embeddinggemma -#NARUON_MLX_LLM_MODEL=gemma4:e2b-it-qat +#NARUON_MLX_LLM_MODEL=mlx-community/gemma-4-e4b-it-4bit #NARUON_MLX_EXTRA_HOSTS=host-gateway +#NARUON_LLAMA_CPP_EMBEDDING_BASE_URL=http://host.docker.internal:8082/v1 # Optional host bind overrides for local conflict cases when running with a custom override. # Format is host:container, e.g. 127.0.0.1:3000 diff --git a/.github/workflows/pr-governance.yml b/.github/workflows/pr-governance.yml index 7dcea3104..9cc87337c 100644 --- a/.github/workflows/pr-governance.yml +++ b/.github/workflows/pr-governance.yml @@ -176,14 +176,34 @@ jobs: } trusted_ref="" - if [[ "${TRUSTED_BASE_SHA:-}" =~ ^[0-9a-f]{40}$ ]]; then - trusted_ref="$TRUSTED_BASE_SHA" - echo "Using event-provided trusted governance base SHA ${trusted_ref}." - elif [ -n "$TRUSTED_PR_NUMBER" ]; then - trusted_ref="$(gh_api_with_retry "repos/${GITHUB_REPOSITORY}/pulls/${TRUSTED_PR_NUMBER}" --jq '.base.sha')" + if [ -n "$TRUSTED_PR_NUMBER" ]; then + if ! [[ "$TRUSTED_PR_NUMBER" =~ ^[1-9][0-9]*$ ]]; then + echo "::error::Trusted pull request number must be a positive integer." >&2 + exit 1 + fi + api_base_sha="$(gh_api_with_retry "repos/${GITHUB_REPOSITORY}/pulls/${TRUSTED_PR_NUMBER}" --jq '.base.sha')" + if ! [[ "$api_base_sha" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::GitHub API returned an invalid pull request base SHA." >&2 + exit 1 + fi + if [ -n "${TRUSTED_BASE_SHA:-}" ] && [ "$TRUSTED_BASE_SHA" != "$api_base_sha" ]; then + echo "::error::Supplied governance base SHA does not match the live pull request base SHA." >&2 + exit 1 + fi + trusted_ref="$api_base_sha" + echo "Using the live pull request base SHA ${trusted_ref}." else default_branch="$(gh_api_with_retry "repos/${GITHUB_REPOSITORY}" --jq '.default_branch')" - trusted_ref="$(gh_api_with_retry "repos/${GITHUB_REPOSITORY}/branches/${default_branch}" --jq '.commit.sha')" + default_sha="$(gh_api_with_retry "repos/${GITHUB_REPOSITORY}/branches/${default_branch}" --jq '.commit.sha')" + if ! [[ "$default_sha" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::GitHub API returned an invalid default branch SHA." >&2 + exit 1 + fi + if [ -n "${TRUSTED_BASE_SHA:-}" ] && [ "$TRUSTED_BASE_SHA" != "$default_sha" ]; then + echo "::error::Manual governance base SHA must equal the current default branch SHA when no PR number is supplied." >&2 + exit 1 + fi + trusted_ref="$default_sha" fi if [[ ! "$trusted_ref" =~ ^[0-9a-f]{40}$ ]]; then echo "::error::Trusted governance ref must be a full commit SHA." >&2 @@ -207,7 +227,13 @@ jobs: echo "Trusted governance archive materialization attempt ${attempt} did not produce a valid archive; retrying." >&2 sleep $((attempt * 3)) done - tar -xzf "$trusted_archive" -C "$trusted_workspace" --strip-components=1 + python3 -c 'import os,sys,tarfile; from pathlib import Path,PurePosixPath; archive=tarfile.open(sys.argv[1],"r:gz"); workspace=Path(sys.argv[2]).resolve(); members=archive.getmembers(); target=lambda member:(workspace / Path(*PurePosixPath(member.name).parts[1:])).resolve(); unsafe=[member.name for member in members if (not member.name or member.name.startswith("/") or ".." in PurePosixPath(member.name).parts or member.issym() or member.islnk() or member.isdev() or os.path.commonpath((workspace,target(member))) != str(workspace))]; raise SystemExit(f"unsafe trusted governance archive member: {unsafe[0]!r}") if unsafe else None; archive.extractall(workspace,members=members)' "$trusted_archive" "$trusted_workspace" + test -d "$trusted_workspace/scripts" + governance_script="$trusted_workspace/scripts/ci/pr_governance_gate.sh" + if [ ! -f "$governance_script" ] || [ -L "$governance_script" ]; then + echo "::error::Trusted governance gate is missing or is a symbolic link." >&2 + exit 1 + fi test -f "$trusted_workspace/scripts/ci/pr_governance_gate.sh" echo "GOVERNANCE_GATE=$trusted_workspace/scripts/ci/pr_governance_gate.sh" >> "$GITHUB_ENV" diff --git a/README.md b/README.md index a5cc6252f..718fd2ab3 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,11 @@ mail/calendar/file systems. on the allowlisted hostname. - Session authority is assigned by the verified HMAC or OIDC code path, not by a `_session_verifier` JWT payload claim supplied inside the token. +- Production membership authentication is expected to use the ecosystem's + [Keyverse OIDC/JWKS identity provider](https://github.com/ContextualWisdomLab/keyverse). + Configure `OIDC_ISSUER_URL`, `OIDC_CLIENT_ID`, `OIDC_JWKS_URL`, and + `ALLOWED_OIDC_HOSTS` together; the local HMAC session is for controlled smoke + tests and is not authoritative cross-workspace membership evidence. ## Agentic Ontology & Auto-Organization @@ -165,69 +170,38 @@ python3 -m webbrowser http://localhost:3000 ### Apple Silicon / MLX local path (OS별 로컬 API 모델 서버 사용) -기본 `docker-compose.yml`는 Linux Ollama 컨테이너를 그대로 유지합니다. Apple Silicon -로컬 실 테스트(또는 외부 MLX/OpenAI-compatible 서비스)만 분리하려면 임시 오버라이드 파일을 붙여 실행합니다. +macOS에서는 `./scripts/naruon_compose.sh`가 OpenAI-compatible 엔드포인트를 +`mlx-lm → llama.cpp → Ollama` 순서로 확인합니다. 먼저 응답하는 호스트 런타임을 +사용하고, 모두 없으면 기존 Ollama 컨테이너를 그대로 빌드·기동합니다. ```bash -# 다음 블록은 로컬 실사용 검증용 샘플입니다. 민감한 쿼리로 대체할 수 있지만, -# 현재 실검증에서는 아래 두 키워드로 테스트합니다. -cat > .env.mlx <<'EOF' - -# 기존 보안값은 그대로 두고, 로컬 모델 경로만 오버라이드 -OPENAI_API_KEY=mlx -ALLOWED_LLM_BASE_URL_HOSTS=localhost,127.0.0.1,host.docker.internal -ALLOW_LOCAL_LLM_PROVIDERS=true -OPENAI_BASE_URL=http://host.docker.internal:11434/v1 -OPENAI_EMBEDDING_MODEL=embeddinggemma -OPENAI_MODEL=gemma4:e2b-it-qat -# 포트 충돌이 있으면 아래 두 값으로 변경 -NARUON_FRONTEND_HOST_PORT=127.0.0.1:3000 -NARUON_BACKEND_HOST_PORT=127.0.0.1:8000 -# Linux에서만 host-gateway가 필요합니다. -NARUON_MLX_EXTRA_HOSTS=host-gateway -NARUON_MLX_ALLOWED_LLM_BASE_URL_HOSTS=localhost,127.0.0.1,host.docker.internal -NARUON_MLX_OPENAI_API_KEY=mlx -NARUON_MLX_BASE_URL=http://host.docker.internal:11434/v1 -NARUON_MLX_EMBEDDING_MODEL=embeddinggemma -NARUON_MLX_LLM_MODEL=gemma4:e2b-it-qat -EOF - -# 로컬에서만 쓰는 compose 오버라이드는 임시 파일로 만들고 커밋하지 않습니다. -# OS 분기 없이 환경변수 하나로 host.docker.internal 매핑을 제어합니다. -# Linux에서 host-gateway가 필요한 환경이면 .env.mlx에서 NARUON_MLX_EXTRA_HOSTS를 덮어씁니다. -# Apple Silicon 검증 기준: 백엔드는 host.docker.internal:11434의 MLX(OpenAI-compatible) -# 엔드포인트로 바로 연결해 Ollama 컨테이너 의존을 피합니다. -mlx_compose_override="$(mktemp "${TMPDIR:-/tmp}/docker-compose.mlx.XXXXXX.yml")" -cat > "$mlx_compose_override" <<'EOF' -services: - backend: - depends_on: - db: - condition: service_healthy - environment: - ALLOW_LOCAL_LLM_PROVIDERS: "true" - ALLOWED_LLM_BASE_URL_HOSTS: ${NARUON_MLX_ALLOWED_LLM_BASE_URL_HOSTS:-localhost,127.0.0.1,host.docker.internal} - OPENAI_API_KEY: ${NARUON_MLX_OPENAI_API_KEY:-mlx} - OPENAI_BASE_URL: ${NARUON_MLX_BASE_URL:-http://host.docker.internal:11434/v1} - OPENAI_EMBEDDING_MODEL: ${NARUON_MLX_EMBEDDING_MODEL:-embeddinggemma} - OPENAI_MODEL: ${NARUON_MLX_LLM_MODEL:-gemma4:e2b-it-qat} - extra_hosts: - - "host.docker.internal:${NARUON_MLX_EXTRA_HOSTS:-host.docker.internal}" - ports: - - "${NARUON_BACKEND_HOST_PORT:-127.0.0.1:8000}:8000" - frontend: - ports: - - "${NARUON_FRONTEND_HOST_PORT:-127.0.0.1:3000}:3000" -EOF - -NARUON_ENV_FILE=.env.mlx \ -docker compose --env-file .env.mlx -f docker-compose.yml -f "$mlx_compose_override" up -d --build - -# 혹시 모델 엔드포인트 미노출이 있을 경우는 위 명령 직전에 로컬 MLX 서버/게이트웨이를 -# 먼저 확인합니다. (호스트는 본인 환경별로 달라질 수 있음) -curl -sf http://127.0.0.1:11434/v1/models >/dev/null && \ - echo "MLX/OpenAI-compatible server is reachable" || \ - echo "MLX endpoint is not reachable on 127.0.0.1:11434" +# mlx-lm은 서비스로 유지한다. 기본 포트는 8080이다. +brew services start mlx-lm +curl -sf http://127.0.0.1:8080/v1/models | head + +./scripts/naruon_compose.sh up -d --build +``` + +기본 모델/주소를 바꿀 때는 `NARUON_MLX_BASE_URL`, `NARUON_MLX_LLM_MODEL`, +`NARUON_MLX_EMBEDDING_MODEL`을 설정합니다. llama.cpp는 +`NARUON_LLAMA_CPP_BASE_URL`을 사용하며 기본 포트는 8081입니다. 별도 +EmbeddingGemma 서버가 `NARUON_MLX_EMBEDDING_BASE_URL` 또는 +`NARUON_LLAMA_CPP_EMBEDDING_BASE_URL`로 지정되면 MLX/llama.cpp chat 경로와 +분리해 검색·임포트에 연결하고, 지정하지 않으면 기본 8082 embedding endpoint를 +자동 탐색합니다. 자동 선택을 무시하려면 +`NARUON_COMPOSE_LLM_RUNTIME=mlx|llama.cpp|ollama`를 지정합니다. + +EmbeddingGemma 후보는 `llmfit info taide/embeddinggemma-GTAIDE-300m-2605 --json` +으로 현재 장비 적합성을 확인합니다. 실제 embedding-capable llama.cpp 캐시를 +준비하고 확인하려면 다음을 실행합니다. 이 서버는 chat fallback과 포트를 분리한 +embedding 전용 예시입니다. + +```bash +llama-server -hf ggml-org/embeddinggemma-300M-GGUF:Q8_0 \ + --embeddings --alias embeddinggemma --host 127.0.0.1 --port 8082 +curl -sf http://127.0.0.1:8082/v1/embeddings \ + -H 'Content-Type: application/json' \ + -d '{"model":"ggml-org/embeddinggemma-300M-GGUF:Q8_0","input":["embedding smoke"]}' ``` 실 메일 임포트 + 요약/초안 검증: @@ -241,26 +215,22 @@ if [ ! -r "$MAIL_DIR" ]; then exit 1 fi -AUTH_SESSION_HMAC_SECRET="$(grep -E '^AUTH_SESSION_HMAC_SECRET=' .env | cut -d= -f2-)" +LIVE_E2E_SESSION_SECRET="$(sed -n 's/^AUTH_SESSION_HMAC_SECRET=//p' .env)" \ python3 backend/scripts/private_mail_http_smoke.py \ --mail-dir "$MAIL_DIR" \ --base-url http://127.0.0.1:3000 \ --frontend-base-url http://127.0.0.1:3000 \ --api-base-url http://127.0.0.1:8000 \ - --session-secret "$AUTH_SESSION_HMAC_SECRET" \ --query "중공업 전력PU 회의록" \ --query "중공업 기전PU 회의록" \ --match-mode all-terms \ --limit 20 \ --batch-size 6 \ --require-browser-visible \ - --llm-smoke \ - --print-session-token + --llm-smoke ``` -`--print-session-token`이 켜진 경우 스크립트가 같은 토큰을 브라우저로 전파하는 -`/auth/session` 호출 예시를 출력합니다. 위 출력의 JS 한 줄을 앱 콘솔에서 실행하면 -`naruon_session` 쿠키가 갱신되어 API로 임포트한 메일이 브라우저와 동일 세션에서 보입니다. +`LIVE_E2E_SESSION_SECRET`는 프로세스 환경변수로만 전달되며 세션 토큰은 출력하지 않습니다. `session_check=ok` 로그는 세션 클레임이 브라우저에서 확인되었음을 뜻하고, `session_check=failed(...)`는 토큰 검증/클레임 파싱 문제가 있음을 뜻합니다. `--require-browser-visible`은 동일 토큰을 `Cookie: naruon_session=...`로 주입해 @@ -284,20 +254,23 @@ python3 backend/scripts/private_mail_http_smoke.py \ 브라우저 세션 값(`session_check=ok`)이 스크립트 출력에 남아있는지 확인 - 브라우저에서 동일 이메일을 선택한 뒤 LLM 요약/초안 버튼 동작 확인 5) 세션 불일치 의심 시 `session_check=failed(...)` 또는 `session_check=skipped(...)`가 - 출력되면 `--print-session-token`의 콘솔 스니펫을 다시 실행하고 새로고침 후 2~4단계를 반복 + 출력되면 같은 `LIVE_E2E_SESSION_SECRET` 환경변수로 smoke를 재실행하고 2~4단계를 반복 실행 전 체크(빠른 사전 진단): ```bash -# Podman/Docker 런타임 연결 확인 -podman system connection ls +# Colima가 제공하는 Docker 런타임 연결 확인 +docker context show +docker info --format '{{.ServerVersion}} {{.Architecture}}' # MLX(OpenAI-compatible) 엔드포인트 노출 확인 -curl -sf http://127.0.0.1:11434/v1/models | head +curl -sf http://127.0.0.1:8080/v1/models | head -# 기존 웹 서비스(Nginx/프록시)가 3000/8000/11434를 가로채고 있지 않은지 확인 +# 기존 웹 서비스(Nginx/프록시)가 3000/8000/8080/8081/11434를 가로채고 있지 않은지 확인 lsof -iTCP:3000 -sTCP:LISTEN lsof -iTCP:8000 -sTCP:LISTEN +lsof -iTCP:8080 -sTCP:LISTEN +lsof -iTCP:8081 -sTCP:LISTEN lsof -iTCP:11434 -sTCP:LISTEN ``` diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index 716590cd1..03a52096f 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -11,19 +11,29 @@ down_revision = "0009_project_graph_projection" branch_labels = None depends_on = None +_EMAIL_TABLE = "email_records" +_READ_STATE_COLUMN = "is_read" def upgrade() -> None: - op.add_column( - "emails", - sa.Column( - "is_read", - sa.Boolean(), - nullable=False, - server_default=sa.text("true"), - ), - ) + connection = op.get_bind() + inspector = sa.inspect(connection) + columns = {column["name"] for column in inspector.get_columns(_EMAIL_TABLE)} + if _READ_STATE_COLUMN not in columns: + op.add_column( + _EMAIL_TABLE, + sa.Column( + _READ_STATE_COLUMN, + sa.Boolean(), + nullable=False, + server_default=sa.text("true"), + ), + ) def downgrade() -> None: - op.drop_column("emails", "is_read") + connection = op.get_bind() + inspector = sa.inspect(connection) + columns = {column["name"] for column in inspector.get_columns(_EMAIL_TABLE)} + if _READ_STATE_COLUMN in columns: + op.drop_column(_EMAIL_TABLE, _READ_STATE_COLUMN) diff --git a/backend/api/auth.py b/backend/api/auth.py index bd188351c..1c81f9647 100644 --- a/backend/api/auth.py +++ b/backend/api/auth.py @@ -417,20 +417,32 @@ def _reject_signed_session_admin_payload(payload: dict[str, Any]) -> None: raise _authentication_error() +def _safe_ascii_claim(value: object) -> str | None: + if not isinstance(value, str): + return None + normalized = value.strip() + if not normalized or not normalized.isascii(): + return None + if any(ord(character) < 32 or ord(character) == 127 for character in normalized): + return None + return normalized + + def _required_string_claim(payload: dict[str, Any], name: str) -> str: - value = payload.get(name) - if not isinstance(value, str) or not value.strip() or not value.isascii(): + value = _safe_ascii_claim(payload.get(name)) + if value is None: raise _authentication_error() - return value.strip() + return value def _optional_string_claim(payload: dict[str, Any], name: str) -> str | None: - value = payload.get(name) - if value is None: + raw_value = payload.get(name) + if raw_value is None: return None - if not isinstance(value, str) or not value.strip() or not value.isascii(): + value = _safe_ascii_claim(raw_value) + if value is None: raise _authentication_error() - return value.strip() + return value def _tuple_string_claim(payload: dict[str, Any], name: str) -> tuple[str, ...]: @@ -441,24 +453,27 @@ def _tuple_string_claim(payload: dict[str, Any], name: str) -> tuple[str, ...]: raise _authentication_error() normalized: list[str] = [] for item in value: - if not isinstance(item, str) or not item.strip() or not item.isascii(): + safe_item = _safe_ascii_claim(item) + if safe_item is None: raise _authentication_error() - normalized.append(item.strip()) + normalized.append(safe_item) return tuple(normalized) def _session_audience_claim(payload: dict[str, Any]) -> tuple[str, ...]: value = payload.get("aud") if isinstance(value, str): - if not value.strip() or not value.isascii(): + safe_value = _safe_ascii_claim(value) + if safe_value is None: raise _authentication_error() - return (value.strip(),) + return (safe_value,) if isinstance(value, list | tuple): normalized: list[str] = [] for item in value: - if not isinstance(item, str) or not item.strip() or not item.isascii(): + safe_item = _safe_ascii_claim(item) + if safe_item is None: raise _authentication_error() - normalized.append(item.strip()) + normalized.append(safe_item) return tuple(normalized) raise _authentication_error() diff --git a/backend/api/emails.py b/backend/api/emails.py index 2b0a9dbd6..0679701b3 100644 --- a/backend/api/emails.py +++ b/backend/api/emails.py @@ -601,6 +601,7 @@ async def import_email_files( embedding_provider = EmailImportEmbeddingProvider( api_key=runtime_provider.api_key, base_url=runtime_provider.base_url, + embedding_base_url=runtime_provider.embedding_base_url, embedding_model=runtime_provider.embedding_model, ) diff --git a/backend/api/search.py b/backend/api/search.py index 5a81659f2..fa2ea4a02 100644 --- a/backend/api/search.py +++ b/backend/api/search.py @@ -304,7 +304,8 @@ async def _resolve_query_embedding( embeddings = await generate_embeddings( [normalized_query], runtime_provider.api_key, - base_url=runtime_provider.base_url, + base_url=runtime_provider.embedding_base_url + or runtime_provider.base_url, model=runtime_provider.embedding_model, ) except EmbeddingGenerationError: @@ -500,7 +501,8 @@ async def grounded_answer( embeddings = await generate_embeddings( [normalized_query], runtime_provider.api_key, - base_url=runtime_provider.base_url, + base_url=runtime_provider.embedding_base_url + or runtime_provider.base_url, model=runtime_provider.embedding_model, ) query_embedding = ( diff --git a/backend/core/config.py b/backend/core/config.py index 9f58be05a..d27a80eb4 100644 --- a/backend/core/config.py +++ b/backend/core/config.py @@ -107,18 +107,18 @@ class Settings(BaseSettings): PROJECT_GRAPH_EXTRACTION_ENABLED: bool = False # Which extractor projects segments into the graph, resolved through the # named+versioned KG extractor seam (services/project_graph/extractor_registry): - # "keyword" — deterministic baseline (the structural fallback), + # "keyword" — explicit deterministic reference extractor only, # "llm" — grounded LLM extraction (enforced segment citations), # "orchestrator" — the same grounded LLM extraction routed through the # contextual-orchestrator gateway (see below). - # Every selection falls back to "keyword" on any failure, so rule-based - # extraction stays fallback/reference only. - PROJECT_GRAPH_EXTRACTOR: str = "keyword" + # Grounded extraction is the default; "keyword" remains an explicit + # diagnostic/reference choice and a last-resort fallback. + PROJECT_GRAPH_EXTRACTOR: str = "orchestrator" # OpenAI-compatible base URL of the contextual-orchestrator LLM gateway that # grounded extraction is routed through when PROJECT_GRAPH_EXTRACTOR is # "orchestrator". Must be HTTPS and exact-host allowlisted by # ALLOWED_LLM_BASE_URL_HOSTS (enforced by build_llm_provider_http_client); - # unset routing fails closed to the deterministic keyword extractor. The + # unset routing fails closed to the deterministic reference extractor. The # provider API key remains the tenant's Fernet-encrypted credential. PROJECT_GRAPH_ORCHESTRATOR_BASE_URL: str | None = None DATA_REGION: str = "kr" @@ -132,7 +132,9 @@ class Settings(BaseSettings): ) # OpenAI Settings + OPENAI_API_KEY: SecretStr | None = None OPENAI_BASE_URL: str | None = None + OPENAI_EMBEDDING_BASE_URL: str | None = None OPENAI_EMBEDDING_MODEL: str = "text-embedding-3-small" OPENAI_MODEL: str = "gpt-4o" diff --git a/backend/scripts/bootstrap_db.py b/backend/scripts/bootstrap_db.py index 1047103e8..3e579a053 100644 --- a/backend/scripts/bootstrap_db.py +++ b/backend/scripts/bootstrap_db.py @@ -186,10 +186,6 @@ def _get_create_indexes_statements() -> list[Executable]: "CREATE INDEX IF NOT EXISTS ix_email_records_owner_date " "ON email_records (user_id, organization_id, date)" ), - text( - "CREATE INDEX IF NOT EXISTS ix_emails_owner_date " - "ON emails (user_id, organization_id, date)" - ), text( "CREATE INDEX IF NOT EXISTS ix_sender_relationships_owner_source " "ON sender_relationships " diff --git a/backend/scripts/disksage_copy_readiness_handoff.py b/backend/scripts/disksage_copy_readiness_handoff.py index e50ebde6b..f1b8e7d61 100644 --- a/backend/scripts/disksage_copy_readiness_handoff.py +++ b/backend/scripts/disksage_copy_readiness_handoff.py @@ -15,6 +15,7 @@ import selectors import signal import stat +import sys # Bandit B404: subprocess is required for the digest-bound verifier process boundary. import subprocess # nosec B404 @@ -28,6 +29,9 @@ VERIFIER_COPY_CHUNK_BYTES = 1024 * 1024 MAX_STDOUT_BYTES = 64 * 1024 MAX_STDERR_BYTES = 8 * 1024 +VERIFIER_ENV = { + "PATH": os.pathsep.join((str(Path(sys.executable).parent), os.defpath)) +} EXIT_USAGE = 64 EXIT_VERIFIER_UNAVAILABLE = 66 EXIT_EXECUTION_FAILED = 70 @@ -243,7 +247,7 @@ def _run_bounded_verifier(verifier: Path, readiness: Path) -> VerifierResult: stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd="/", - env={}, + env=VERIFIER_ENV, start_new_session=True, close_fds=True, ) diff --git a/backend/scripts/private_mail_http_smoke.py b/backend/scripts/private_mail_http_smoke.py index 1a8c85c3a..9ed1b3a58 100755 --- a/backend/scripts/private_mail_http_smoke.py +++ b/backend/scripts/private_mail_http_smoke.py @@ -669,6 +669,14 @@ def _fetch_search_snapshot( return last_data, 0 +def _browser_inbox_min_count( + imported_count: int, api_inbox_count: int, api_limit: int +) -> int: + if imported_count > 0 and api_inbox_count == 0: + raise SystemExit("api inbox did not reflect imported mail") + return min(api_inbox_count, api_limit) + + def _check_frontend_session(base_url: str, token: str) -> dict[str, object] | None: try: data = _post_json_with_retry( @@ -724,28 +732,6 @@ def _cleanup_private_cache(files: list[Path]) -> None: ) -def _print_session_sync_hints(base_url: str, token: str, *, enabled: bool) -> None: - if not enabled: - return - - print( - "session_token=" + token, - ) - print("브라우저 동일 세션 동기화 방법:") - print( - " 1) 브라우저에서 NARUON 앱(origin)으로 접속한 뒤, 개발자 콘솔에서 아래 한 줄 실행:", - ) - print( - " await fetch('/auth/session', {method:'POST', credentials:'same-origin', " - "headers:{'content-type':'application/json'}, body: JSON.stringify({access_token: '" - + token - + "'})});", - ) - safe_origin = base_url.rstrip("/") - print(f" (요청 대상: {safe_origin}/auth/session)") - print(" 2) 새로고침 후 /mail 또는 /api/emails로 임포트 반영 및 표시 여부 확인") - - def _print_session_check_summary(claims: dict[str, object] | None) -> None: if claims is None: print("session_check=skipped(endpoint_not_frontend)") @@ -784,15 +770,11 @@ def main() -> None: "--frontend-base-url", help="Optional override when frontend and backend ports differ", ) - parser.add_argument( - "--session-secret", default=os.environ.get("LIVE_E2E_SESSION_SECRET", "") - ) parser.add_argument("--limit", type=int, default=10) parser.add_argument("--batch-size", type=int, default=10) parser.add_argument("--query", action="append", default=[]) parser.add_argument("--match-mode", choices=["exact", "all-terms"], default="exact") parser.add_argument("--llm-smoke", action="store_true") - parser.add_argument("--print-session-token", action="store_true") parser.add_argument( "--search-retry-attempts", type=int, default=SEARCH_RETRY_DEFAULT_ATTEMPTS ) @@ -818,8 +800,9 @@ def main() -> None: parser.add_argument("--progress-every", type=int, default=0) args = parser.parse_args() - if not args.session_secret: - raise SystemExit("LIVE_E2E_SESSION_SECRET or --session-secret is required") + session_secret = os.environ.get("LIVE_E2E_SESSION_SECRET", "") + if not session_secret: + raise SystemExit("LIVE_E2E_SESSION_SECRET is required") if args.limit <= 0 or args.batch_size <= 0 or args.batch_size > 10: raise SystemExit("--limit must be positive and --batch-size must be 1..10") if args.search_retry_attempts < 1 or args.inbox_retry_attempts < 1: @@ -844,7 +827,7 @@ def main() -> None: ) try: - token = _signed_token(args.session_secret) + token = _signed_token(session_secret) session_claims = _check_frontend_session(frontend_base_url, token) _print_session_check_summary(session_claims) totals: typing.Counter[str] = Counter() @@ -871,19 +854,21 @@ def main() -> None: expected_min_count = totals["imported"] api_limit = max(1, min(200, expected_min_count if expected_min_count else 1)) - visible_min_count = min(expected_min_count, api_limit) api_inbox, email_count = _fetch_inbox_snapshot( api_base_url, token, limit=api_limit, - min_count=visible_min_count, + min_count=1 if expected_min_count > 0 else 0, attempts=args.inbox_retry_attempts if expected_min_count > 0 else 1, delay_seconds=args.inbox_retry_delay_seconds if expected_min_count > 0 else 0.0, timeout=120.0, ) + visible_min_count = _browser_inbox_min_count( + expected_min_count, email_count, api_limit + ) frontend_inbox_count = 0 if args.require_browser_visible: _, frontend_inbox_count = _fetch_inbox_snapshot( @@ -1001,11 +986,6 @@ def main() -> None: f"reason_counts={dict(reasons)} " f"llm={llm_status} draft={draft_status}" ) - _print_session_sync_hints( - frontend_base_url, - token, - enabled=args.print_session_token, - ) finally: _cleanup_private_cache(files) diff --git a/backend/services/batch_embedding_service.py b/backend/services/batch_embedding_service.py index b9fe02b76..307f429a1 100644 --- a/backend/services/batch_embedding_service.py +++ b/backend/services/batch_embedding_service.py @@ -618,7 +618,8 @@ async def _run_local_engine_batch( vectors = await generate_embeddings( part_texts, embedding_provider.api_key, - base_url=embedding_provider.base_url, + base_url=embedding_provider.embedding_base_url + or embedding_provider.base_url, model=model, ) for offset, text_index in enumerate(index_group): diff --git a/backend/services/email_import_service.py b/backend/services/email_import_service.py index 1ff9a2bb3..f9afda114 100644 --- a/backend/services/email_import_service.py +++ b/backend/services/email_import_service.py @@ -78,6 +78,7 @@ class EmailImportEmbeddingProvider: api_key: str base_url: str | None embedding_model: str + embedding_base_url: str | None = None @dataclass(frozen=True) @@ -243,6 +244,17 @@ def _session_uses_postgresql(session: AsyncSession) -> bool: return getattr(getattr(bind, "dialect", None), "name", None) == "postgresql" +def _owner_import_lock_key(user_id: str, organization_id: str) -> str: + """Return a PostgreSQL-text-safe, collision-resistant owner lock key.""" + if "\x00" in user_id or "\x00" in organization_id: + raise ValueError("email import owner identity contains NUL") + return hashlib.sha256( + user_id.encode("utf-8") + + b"\x00" + + organization_id.encode("utf-8") + ).hexdigest() + + async def _acquire_owner_import_quota_lock( session: AsyncSession, *, user_id: str, organization_id: str ) -> bool: @@ -250,7 +262,7 @@ async def _acquire_owner_import_quota_lock( return False lock_params = { "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, - "owner_key": f"{user_id}\x00{organization_id}", + "owner_key": _owner_import_lock_key(user_id, organization_id), } await session.execute( select( @@ -269,7 +281,7 @@ async def _release_owner_import_quota_lock( ) -> None: lock_params = { "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, - "owner_key": f"{user_id}\x00{organization_id}", + "owner_key": _owner_import_lock_key(user_id, organization_id), } await session.execute( select( @@ -729,7 +741,7 @@ async def _extract_project_semantics_for_import( OpenAI-compatible provider credentials and enforce segment citations, so they cannot introduce uncited claims; a missing credential, an unconfigured orchestrator endpoint, or any provider/parse failure degrades down the chain - to the deterministic keyword baseline instead of losing the projection. + to the deterministic reference extractor instead of losing the projection. """ context = KgExtractorContext( api_key=embedding_provider.api_key if embedding_provider else None, @@ -926,7 +938,8 @@ async def _generate_import_embeddings( provider_embeddings = await generate_embeddings( texts, embedding_provider.api_key, - base_url=embedding_provider.base_url, + base_url=embedding_provider.embedding_base_url + or embedding_provider.base_url, model=embedding_provider.embedding_model, ) except (EmbeddingGenerationError, ValueError) as exc: @@ -943,7 +956,8 @@ async def _generate_import_embeddings( single_embedding = await generate_embeddings( [text], embedding_provider.api_key, - base_url=embedding_provider.base_url, + base_url=embedding_provider.embedding_base_url + or embedding_provider.base_url, model=embedding_provider.embedding_model, ) if not single_embedding: diff --git a/backend/services/embedding.py b/backend/services/embedding.py index 626a44f2e..b22cca8f4 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -46,7 +46,11 @@ async def generate_embeddings( raise ValueError("OPENAI_API_KEY is not set") # Instantiate client locally to avoid global state race conditions across tenants - configured_base_url = base_url if base_url is not None else settings.OPENAI_BASE_URL + configured_base_url = base_url + if configured_base_url is None: + configured_base_url = ( + settings.OPENAI_EMBEDDING_BASE_URL or settings.OPENAI_BASE_URL + ) validated_base_url, http_client = await build_llm_provider_http_client( configured_base_url ) diff --git a/backend/services/llm_provider_selection.py b/backend/services/llm_provider_selection.py index a10c8e11e..a8387507b 100644 --- a/backend/services/llm_provider_selection.py +++ b/backend/services/llm_provider_selection.py @@ -1,5 +1,6 @@ from dataclasses import dataclass from typing import Literal +from urllib.parse import urlsplit from sqlalchemy import desc, select from sqlalchemy.ext.asyncio import AsyncSession @@ -14,8 +15,11 @@ from services.tenant_config_scope import get_scoped_tenant_config -ProviderSource = Literal["llm_provider", "tenant_config"] +ProviderSource = Literal["llm_provider", "tenant_config", "local_environment"] LOCAL_PROVIDER_API_KEY = "local-provider" +LOCAL_RUNTIME_HOSTS = frozenset( + {"localhost", "localhost.localdomain", "127.0.0.1", "::1", "host.docker.internal", "ollama"} +) @dataclass(frozen=True) @@ -26,6 +30,7 @@ class RuntimeLLMProvider: embedding_model: str provider_name: str provider_source: ProviderSource + embedding_base_url: str | None = None provider_id: int | None = None @@ -96,6 +101,10 @@ async def resolve_runtime_llm_provider( if runtime_provider is not None: return runtime_provider + local_provider = _local_environment_provider() + if local_provider is not None: + return local_provider + tenant_config = await get_scoped_tenant_config(session, user_id, organization_id) if tenant_config is None or not tenant_config.openai_api_key: return None @@ -109,3 +118,29 @@ async def resolve_runtime_llm_provider( provider_source="tenant_config", provider_id=None, ) + + +def _local_environment_provider() -> RuntimeLLMProvider | None: + if not settings.ALLOW_LOCAL_LLM_PROVIDERS: + return None + base_url = settings.OPENAI_BASE_URL + hostname = urlsplit(base_url or "").hostname + if not base_url or hostname not in LOCAL_RUNTIME_HOSTS: + return None + api_key = settings.OPENAI_API_KEY + if api_key is None or not api_key.get_secret_value().strip(): + return None + embedding_base_url = settings.OPENAI_EMBEDDING_BASE_URL + embedding_hostname = urlsplit(embedding_base_url or "").hostname + if embedding_base_url and embedding_hostname not in LOCAL_RUNTIME_HOSTS: + embedding_base_url = None + return RuntimeLLMProvider( + api_key=api_key.get_secret_value(), + base_url=base_url, + embedding_base_url=embedding_base_url, + chat_model=settings.OPENAI_MODEL, + embedding_model=settings.OPENAI_EMBEDDING_MODEL, + provider_name="Local runtime", + provider_source="local_environment", + provider_id=None, + ) diff --git a/backend/services/llm_provider_urls.py b/backend/services/llm_provider_urls.py index 734a21c7b..eae5d5fcd 100644 --- a/backend/services/llm_provider_urls.py +++ b/backend/services/llm_provider_urls.py @@ -16,6 +16,7 @@ _DNS_RESOLUTION_TIMEOUT_SECONDS = 5.0 _LOCAL_DEV_HOSTNAMES = {"localhost", "localhost.localdomain"} _LOCAL_DEV_IP_LITERALS = {"127.0.0.1", "::1"} +_LOCAL_CONTAINER_HOSTNAMES = {"host.docker.internal"} def _has_url_control_character(value: str) -> bool: @@ -68,7 +69,10 @@ def _is_allowlisted_local_provider_host(hostname: str) -> bool: return ( settings.ALLOW_LOCAL_LLM_PROVIDERS and normalized_hostname in _parse_allowed_hosts() - and "." not in normalized_hostname + and ( + "." not in normalized_hostname + or normalized_hostname in _LOCAL_CONTAINER_HOSTNAMES + ) and not _is_ip_literal(normalized_hostname) and not _looks_like_ip_literal(normalized_hostname) ) diff --git a/backend/services/llm_service.py b/backend/services/llm_service.py index a3689eb01..e80b63026 100644 --- a/backend/services/llm_service.py +++ b/backend/services/llm_service.py @@ -2,6 +2,7 @@ import json import logging +import re from urllib.parse import urlsplit, urlunsplit from openai import AsyncOpenAI @@ -20,6 +21,13 @@ OLLAMA_NATIVE_CHAT_LOOPBACK_HOSTS = frozenset( {"localhost", "localhost.localdomain", "127.0.0.1", "::1"} ) +LOCAL_STRUCTURED_OUTPUT_HOSTS = frozenset( + { + *OLLAMA_NATIVE_CHAT_HOSTS, + *OLLAMA_NATIVE_CHAT_LOOPBACK_HOSTS, + "host.docker.internal", + } +) OLLAMA_NATIVE_CHAT_PORT = 11434 @@ -37,6 +45,36 @@ class ExtractionResult(BaseModel): ) +def _parse_extraction_content(content: str | None) -> ExtractionResult: + if not content: + raise ValueError("LLM returned an empty extraction response") + fenced_match = re.search( + r"```\s*(?:json)?\s*(.*?)\s*```", content, re.DOTALL | re.IGNORECASE + ) + payload_text = fenced_match.group(1) if fenced_match else content.strip() + decoder = json.JSONDecoder() + for start, character in enumerate(payload_text): + if character != "{": + continue + try: + payload, _ = decoder.raw_decode(payload_text[start:]) + except json.JSONDecodeError: + continue + return ExtractionResult.model_validate(payload) + raise ValueError("LLM returned invalid extraction JSON") + + +def _is_local_llm_endpoint(validated_base_url: str | None) -> bool: + hostname = urlsplit(validated_base_url or "").hostname + return settings.ALLOW_LOCAL_LLM_PROVIDERS and hostname in LOCAL_STRUCTURED_OUTPUT_HOSTS + + +def _local_chat_request_kwargs(validated_base_url: str | None) -> dict[str, object]: + if not _is_local_llm_endpoint(validated_base_url): + return {} + return {"extra_body": {"chat_template_kwargs": {"enable_thinking": False}}} + + async def extract_action_items_and_summary( email_body: str, openai_api_key: str, @@ -59,37 +97,59 @@ async def extract_action_items_and_summary( ) selected_model = model or settings.OPENAI_MODEL try: - response = await provider_circuit_breaker.call( - validated_base_url or "openai-default", - lambda: retry_transient( - lambda: client.beta.chat.completions.parse( - model=selected_model, - messages=[ - { - "role": "system", - "content": ( - "You are a helpful assistant. Summarize the email, " - "extract action items, and include a confidence score " - "from 0 to 100 when enough evidence is available." + messages = [ + { + "role": "system", + "content": ( + "You are a helpful assistant. Summarize the email, extract " + "action items, and include a confidence score from 0 to 100 " + "when enough evidence is available." + ), + }, + {"role": "user", "content": email_body}, + ] + if _is_local_llm_endpoint(validated_base_url): + messages[0]["content"] += ( + " Return only one valid JSON object with exactly these keys: " + "summary (string), action_items (array of strings), and " + "confidence (integer 0-100 or null). Do not include markdown " + "or any explanation." + ) + response = await provider_circuit_breaker.call( + validated_base_url or "openai-default", + lambda: retry_transient( + lambda: client.chat.completions.create( + model=selected_model, + messages=messages, + response_format={"type": "json_object"}, + temperature=0, + **_local_chat_request_kwargs(validated_base_url), ), - }, - {"role": "user", "content": email_body}, - ], - response_format=ExtractionResult, - ), - operation_name="summary extraction", - ), - ) + operation_name="summary extraction", + ), + ) + parsed = _parse_extraction_content(response.choices[0].message.content) + else: + response = await provider_circuit_breaker.call( + validated_base_url or "openai-default", + lambda: retry_transient( + lambda: client.beta.chat.completions.parse( + model=selected_model, + messages=messages, + response_format=ExtractionResult, + ), + operation_name="summary extraction", + ), + ) + parsed = response.choices[0].message.parsed + if not parsed: + raise ValueError("LLM returned no structured extraction") except Exception as e: logger.error(f"Error calling LLM API for extraction: {e}") raise LLMServiceError(f"LLM API error during extraction: {e}") from e finally: await client.close() - parsed = response.choices[0].message.parsed - if not parsed: - raise RuntimeError("Failed to parse LLM response") - parsed.provenance = f"{provider_name} ({selected_model})" return parsed @@ -142,6 +202,7 @@ async def translate_email_body( model=selected_model, messages=messages, temperature=0.3, + **_local_chat_request_kwargs(validated_base_url), ), operation_name="translation", ), @@ -203,10 +264,11 @@ async def draft_reply( response = await provider_circuit_breaker.call( validated_base_url or "openai-default", lambda: retry_transient( - lambda: client.chat.completions.create( - model=selected_model, - messages=messages, - ), + lambda: client.chat.completions.create( + model=selected_model, + messages=messages, + **_local_chat_request_kwargs(validated_base_url), + ), operation_name="reply drafting", ), ) diff --git a/backend/services/project_graph/extractor_registry.py b/backend/services/project_graph/extractor_registry.py index 47eeef908..1836547c0 100644 --- a/backend/services/project_graph/extractor_registry.py +++ b/backend/services/project_graph/extractor_registry.py @@ -12,8 +12,8 @@ per the platform plan's ``kg.extractor`` extension point) register without editing core ingest, * :func:`resolve_extractor_chain` / :func:`run_extraction`, which build an - ordered fallback chain whose **terminal element is always the deterministic - keyword extractor** — encoding "rule-based extraction is fallback/reference + ordered fallback chain whose terminal element is the deterministic + reference extractor — encoding "rule-based extraction is fallback/reference only" structurally rather than in an ad-hoc branch. Routing LLM extraction through **contextual-orchestrator** is modelled as a @@ -113,7 +113,7 @@ async def extract( class DeterministicKeywordExtractor: - """The deterministic keyword baseline — the structural fallback extractor. + """The deterministic keyword reference extractor and last-resort fallback. Pure and dependency-free: it needs no credentials and always produces a result, which is exactly why the registry keeps it as the terminal element @@ -177,13 +177,13 @@ async def extract( class KgExtractorRegistry: - """Selector-keyed registry of extractors with a guaranteed keyword fallback. + """Selector-keyed registry with a guaranteed reference-only fallback. The registry is the pluggable seam: a plugin or a new extractor registers a :class:`KgExtractor` under a selector key and becomes selectable via ``PROJECT_GRAPH_EXTRACTOR`` without touching the ingest pipeline. Chain - resolution always appends the deterministic keyword extractor as the - terminal fallback. + resolution always appends the deterministic reference extractor as the + terminal fallback; it is never the default judgment source. """ def __init__(self) -> None: diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index f8f3ffeae..c05747df9 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -422,6 +422,15 @@ def test_merge_revision_reconciles_email_read_state_branch(): assert "op.drop_column(" not in revision_text +def test_email_read_state_revision_uses_canonical_email_records_table(): + revision_path = BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" + revision_text = revision_path.read_text() + + assert '_EMAIL_TABLE = "email_records"' in revision_text + assert "inspector.get_columns(_EMAIL_TABLE)" in revision_text + assert 'op.add_column(\n _EMAIL_TABLE' in revision_text + + def test_merge_revision_reconciles_newsdom_provider_branch(): revision_path = ( BACKEND_ROOT / "alembic" / "versions" / "0015_merge_newsdom_email_heads.py" diff --git a/backend/tests/test_auth_real.py b/backend/tests/test_auth_real.py index 11450683e..ca05b1004 100644 --- a/backend/tests/test_auth_real.py +++ b/backend/tests/test_auth_real.py @@ -477,6 +477,28 @@ async def test_signed_bearer_session_rejects_non_ascii_claim_values(): assert exc.value.status_code == 401 +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("claim_name", "claim_value"), + [ + ("sub", "alice\x00"), + ("org", "org\x00acme"), + ("workspace", "workspace\x00org-acme"), + ("groups", ["group-1", "group\x00two"]), + ], +) +async def test_signed_bearer_session_rejects_nul_claim_values( + claim_name: str, claim_value: object +): + settings.AUTH_SESSION_HMAC_SECRET = SecretStr(TEST_SESSION_HMAC_SECRET) + token = _signed_session_token(_valid_session_payload(**{claim_name: claim_value})) + + with pytest.raises(HTTPException) as exc: + await get_auth_context(authorization=f"Bearer {token}") + + assert exc.value.status_code == 401 + + @pytest.mark.asyncio async def test_signed_bearer_session_rejects_non_finite_expiration(): settings.AUTH_SESSION_HMAC_SECRET = SecretStr(TEST_SESSION_HMAC_SECRET) diff --git a/backend/tests/test_bootstrap_db.py b/backend/tests/test_bootstrap_db.py index 5af0540f0..b1cf9edcd 100644 --- a/backend/tests/test_bootstrap_db.py +++ b/backend/tests/test_bootstrap_db.py @@ -74,6 +74,17 @@ def test_schema_backfill_adds_email_columns(monkeypatch): ) +def test_schema_backfill_targets_the_canonical_email_table(monkeypatch): + statements = _get_schema_statements(monkeypatch) + + assert any( + "create index if not exists ix_email_records_owner_date" in statement + and "on email_records" in statement + for statement in statements + ) + assert not any("on emails" in statement for statement in statements) + + def test_schema_backfill_adds_sender_relationship_columns_and_indexes(monkeypatch): statements = _get_schema_statements(monkeypatch) assert any( diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index 0c24c92f5..f18be0939 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -52,6 +52,15 @@ def test_global_config(): assert hasattr(settings, "ENCRYPTION_KEY") +def test_project_graph_does_not_default_to_keyword_judgment(monkeypatch): + _set_required_runtime_env(monkeypatch) + monkeypatch.delenv("PROJECT_GRAPH_EXTRACTOR", raising=False) + + loaded_settings = _settings_without_env_file() + + assert loaded_settings.PROJECT_GRAPH_EXTRACTOR == "orchestrator" + + def test_production_settings_do_not_expose_dev_header_bypass_controls(): assert "TRUST_DEV_HEADERS" not in settings.__class__.model_fields assert "DEV_AUTH_TOKEN" not in settings.__class__.model_fields diff --git a/backend/tests/test_disksage_copy_readiness_handoff.py b/backend/tests/test_disksage_copy_readiness_handoff.py index 5a1a892f5..d4d4a4857 100644 --- a/backend/tests/test_disksage_copy_readiness_handoff.py +++ b/backend/tests/test_disksage_copy_readiness_handoff.py @@ -40,7 +40,7 @@ def _success_payload() -> dict[str, object]: def _python_verifier(path: Path, source: str) -> Path: - path.write_text(f"#!{sys.executable}\n{source}", encoding="utf-8") + path.write_text(f"#!/usr/bin/env python3\n{source}", encoding="utf-8") path.chmod(0o700) return path diff --git a/backend/tests/test_email_import_service.py b/backend/tests/test_email_import_service.py index d16a79dd1..78b0ae2f0 100644 --- a/backend/tests/test_email_import_service.py +++ b/backend/tests/test_email_import_service.py @@ -12,6 +12,7 @@ EMBEDDING_DIMENSION, EmailImportEmbeddingProvider, _generate_import_embeddings, + _owner_import_lock_key, ) @@ -51,6 +52,16 @@ def test_safe_upload_filename_fails_closed_beyond_decode_round_limit(): assert email_import_module._safe_upload_filename(encoded_name) == "upload" +def test_owner_import_lock_key_is_nul_free_and_tuple_stable(): + first = _owner_import_lock_key("user-1", "org-1") + second = _owner_import_lock_key("org-1", "user-1") + + assert "\x00" not in first + assert first != second + with pytest.raises(ValueError, match="NUL"): + _owner_import_lock_key("user\x00-1", "org-1") + + @pytest.mark.parametrize( ("input_name", "expected"), [ diff --git a/backend/tests/test_emails_api.py b/backend/tests/test_emails_api.py index 7bffa6ff7..e93c06d0a 100644 --- a/backend/tests/test_emails_api.py +++ b/backend/tests/test_emails_api.py @@ -22,6 +22,7 @@ import datetime from unittest.mock import AsyncMock, patch from services.embedding import STORAGE_EMBEDDING_DIMENSION +from services.email_import_service import _owner_import_lock_key from services.email_service import generate_email_fingerprint pytestmark = pytest.mark.usefixtures("dev_auth_dependency_overrides") @@ -1280,11 +1281,11 @@ async def test_import_email_files_serializes_quota_with_postgres_owner_lock( assert advisory_query_params(session) == [ { "namespace_key": "naruon-email-import-quota", - "owner_key": "testuser\x00org-acme", + "owner_key": _owner_import_lock_key("testuser", "org-acme"), }, { "namespace_key": "naruon-email-import-quota", - "owner_key": "testuser\x00org-acme", + "owner_key": _owner_import_lock_key("testuser", "org-acme"), }, ] @@ -1343,11 +1344,11 @@ async def test_import_email_files_rejects_when_owner_quota_is_exhausted( assert advisory_query_params(session) == [ { "namespace_key": "naruon-email-import-quota", - "owner_key": "testuser\x00org-acme", + "owner_key": _owner_import_lock_key("testuser", "org-acme"), }, { "namespace_key": "naruon-email-import-quota", - "owner_key": "testuser\x00org-acme", + "owner_key": _owner_import_lock_key("testuser", "org-acme"), }, ] diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index 80260998a..c36b65ec7 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -51,7 +51,8 @@ async def test_generate_embeddings_success(): with patch("services.embedding.settings") as mock_settings: mock_settings.OPENAI_EMBEDDING_MODEL = "test-model" mock_settings.OPENAI_BASE_URL = None - + mock_settings.OPENAI_EMBEDDING_BASE_URL = None + embeddings = await generate_embeddings(["test1", "test2"], "test-key") assert len(embeddings) == 2 assert embeddings[0] == [0.1, 0.2, 0.3] @@ -94,6 +95,30 @@ async def test_generate_embeddings_uses_selected_provider_model_and_base_url(): mock_client.close.assert_awaited_once() +@pytest.mark.asyncio +async def test_generate_embeddings_prefers_embedding_base_url_when_no_explicit_url(): + with patch( + "services.embedding.AsyncOpenAI" + ) as mock_async_openai, patch( + "services.embedding.build_llm_provider_http_client", + new_callable=AsyncMock, + ) as mock_build_client: + mock_build_client.return_value = ("http://host.docker.internal:8082/v1", AsyncMock()) + mock_client = mock_async_openai.return_value + mock_client.close = AsyncMock() + mock_client.embeddings.create = AsyncMock(return_value=AsyncMock(data=[])) + + with patch("services.embedding.settings") as mock_settings: + mock_settings.OPENAI_EMBEDDING_BASE_URL = ( + "http://host.docker.internal:8082/v1" + ) + mock_settings.OPENAI_BASE_URL = "http://host.docker.internal:8080/v1" + mock_settings.OPENAI_EMBEDDING_MODEL = "embeddinggemma" + await generate_embeddings(["test"], "local-provider") + + mock_build_client.assert_awaited_once_with("http://host.docker.internal:8082/v1") + + @pytest.mark.asyncio async def test_generate_embeddings_api_error(): with patch( @@ -106,6 +131,7 @@ async def test_generate_embeddings_api_error(): with patch("services.embedding.settings") as mock_settings: mock_settings.OPENAI_EMBEDDING_MODEL = "test-model" mock_settings.OPENAI_BASE_URL = None + mock_settings.OPENAI_EMBEDDING_BASE_URL = None with pytest.raises(EmbeddingGenerationError, match="Failed to generate embeddings: API error"): diff --git a/backend/tests/test_llm_provider_selection.py b/backend/tests/test_llm_provider_selection.py index ca0866107..7a5f32c7e 100644 --- a/backend/tests/test_llm_provider_selection.py +++ b/backend/tests/test_llm_provider_selection.py @@ -1,12 +1,14 @@ import datetime import pytest +from pydantic import SecretStr from db.models import LLMProvider from services.llm_provider_selection import ( LOCAL_PROVIDER_API_KEY, resolve_runtime_llm_provider, ) +from core.config import settings class MockScalars: @@ -87,13 +89,47 @@ async def test_resolve_runtime_llm_provider_prefers_active_local_provider(): @pytest.mark.asyncio -async def test_resolve_runtime_llm_provider_falls_back_to_tenant_config(): +async def test_resolve_runtime_llm_provider_falls_back_to_tenant_config(monkeypatch): + monkeypatch.setattr(settings, "OPENAI_BASE_URL", "https://api.openai.com/v1") + monkeypatch.setattr( + settings, + "OPENAI_EMBEDDING_BASE_URL", + "http://host.docker.internal:8082/v1", + ) runtime_provider = await resolve_runtime_llm_provider( MockSession(providers=[], tenant_config=MockTenantConfig("sk-tenant")), user_id="testuser", organization_id="org-acme", ) - assert runtime_provider is not None assert runtime_provider.provider_source == "tenant_config" assert runtime_provider.api_key == "sk-tenant" + assert runtime_provider.embedding_base_url is None + + +@pytest.mark.asyncio +async def test_resolve_runtime_llm_provider_uses_allowlisted_local_environment( + monkeypatch, +): + monkeypatch.setattr(settings, "ALLOW_LOCAL_LLM_PROVIDERS", True) + monkeypatch.setattr(settings, "OPENAI_API_KEY", SecretStr("mlx")) + monkeypatch.setattr(settings, "OPENAI_BASE_URL", "http://host.docker.internal:8080/v1") + monkeypatch.setattr( + settings, + "OPENAI_EMBEDDING_BASE_URL", + "http://host.docker.internal:8082/v1", + ) + monkeypatch.setattr(settings, "OPENAI_MODEL", "local-chat") + monkeypatch.setattr(settings, "OPENAI_EMBEDDING_MODEL", "local-embedding") + + runtime_provider = await resolve_runtime_llm_provider( + MockSession(providers=[], tenant_config=MockTenantConfig("sk-tenant")), + user_id="testuser", + organization_id="org-acme", + ) + + assert runtime_provider is not None + assert runtime_provider.provider_source == "local_environment" + assert runtime_provider.api_key == "mlx" + assert runtime_provider.base_url == "http://host.docker.internal:8080/v1" + assert runtime_provider.embedding_base_url == "http://host.docker.internal:8082/v1" diff --git a/backend/tests/test_llm_provider_urls.py b/backend/tests/test_llm_provider_urls.py index 16ab04e73..4f13fed8e 100644 --- a/backend/tests/test_llm_provider_urls.py +++ b/backend/tests/test_llm_provider_urls.py @@ -65,6 +65,23 @@ def test_validate_global_address_private_allowed_when_host_allowed(monkeypatch): assert _validate_global_address("192.168.1.5", hostname="ollama") == "192.168.1.5" +def test_validate_global_address_allows_explicit_docker_host_gateway(monkeypatch): + monkeypatch.setattr(settings, "ALLOW_LOCAL_LLM_PROVIDERS", True) + monkeypatch.setattr( + settings, + "ALLOWED_LLM_BASE_URL_HOSTS", + "host.docker.internal", + ) + + assert ( + _validate_global_address( + "192.168.5.2", + hostname="host.docker.internal", + ) + == "192.168.5.2" + ) + + def test_validate_global_address_private_rejected_when_host_not_allowed(monkeypatch): """Test that a private IP is rejected even with ALLOW_LOCAL_LLM_PROVIDERS if the hostname is not allowed.""" monkeypatch.setattr(settings, "ALLOW_LOCAL_LLM_PROVIDERS", True) diff --git a/backend/tests/test_llm_service.py b/backend/tests/test_llm_service.py index 463603a45..0353f4a17 100644 --- a/backend/tests/test_llm_service.py +++ b/backend/tests/test_llm_service.py @@ -303,6 +303,46 @@ async def test_extract_action_items_and_summary_uses_selected_provider_model( assert mock_openai.beta.chat.completions.parse.call_args.kwargs["model"] == "gemma4" +@pytest.mark.asyncio +@pytest.mark.parametrize( + "content", + [ + '```JSON\n{"summary":"Local summary","action_items":[]}\n```', + 'The structured result is:\n{"summary":"Local summary","action_items":[]}', + ], +) +async def test_extract_action_items_and_summary_parses_local_json_content( + monkeypatch, content +): + monkeypatch.setattr(settings, "ALLOW_LOCAL_LLM_PROVIDERS", True) + mock_openai = MagicMock() + mock_openai.close = AsyncMock() + response = MagicMock() + response.choices = [ + MagicMock( + message=MagicMock( + content=content + ) + ) + ] + mock_openai.chat.completions.create = AsyncMock(return_value=response) + with patch("services.llm_service.AsyncOpenAI", return_value=mock_openai): + result = await extract_action_items_and_summary( + "Test email", "test-key", base_url="http://127.0.0.1:8080/v1" + ) + + assert result.summary == "Local summary" + assert result.action_items == [] + mock_openai.chat.completions.create.assert_awaited_once() + system_content = mock_openai.chat.completions.create.call_args.kwargs["messages"][0][ + "content" + ] + assert "Return only one valid JSON object" in system_content + assert mock_openai.chat.completions.create.call_args.kwargs["extra_body"] == { + "chat_template_kwargs": {"enable_thinking": False} + } + + @pytest.mark.asyncio async def test_extract_action_items_and_summary_api_error(mock_openai): # Setup mock to raise an exception diff --git a/backend/tests/test_local_http.py b/backend/tests/test_local_http.py index 11dc87a8d..97f321907 100644 --- a/backend/tests/test_local_http.py +++ b/backend/tests/test_local_http.py @@ -80,3 +80,12 @@ def test_local_request_target_rejects_invalid_percent_encoding(path: str) -> Non def test_local_request_target_normalizes_malformed_parser_errors() -> None: with pytest.raises(LocalHTTPValidationError, match="local API path"): validate_local_request_target("//[::1") + + +@pytest.mark.parametrize("value", ["http://127.0.0.1:18080/\x00", "http://127.0.0.1/%00"]) +def test_local_http_rejects_nul_characters(value: str) -> None: + with pytest.raises(LocalHTTPValidationError): + validate_loopback_http_origin(value) + + with pytest.raises(LocalHTTPValidationError): + validate_local_request_target(value, allowed_exact_paths=frozenset()) diff --git a/backend/tests/test_private_mail_http_smoke.py b/backend/tests/test_private_mail_http_smoke.py index 2e21d89d3..718845f48 100644 --- a/backend/tests/test_private_mail_http_smoke.py +++ b/backend/tests/test_private_mail_http_smoke.py @@ -437,6 +437,15 @@ def test_fetch_inbox_snapshot_rejects_empty_retry_budget(): ) +def test_browser_inbox_visibility_uses_thread_level_api_count(): + assert smoke._browser_inbox_min_count(3, 1, 3) == 1 + + +def test_browser_inbox_visibility_rejects_missing_imports(): + with pytest.raises(SystemExit, match="did not reflect imported mail"): + smoke._browser_inbox_min_count(3, 0, 3) + + def test_fetch_search_snapshot_retries_until_results(monkeypatch): calls: list[dict[str, object]] = [] diff --git a/backend/tests/test_release_governance.py b/backend/tests/test_release_governance.py index efe0acd0e..1a7d02743 100644 --- a/backend/tests/test_release_governance.py +++ b/backend/tests/test_release_governance.py @@ -1108,7 +1108,18 @@ def test_pr_governance_uses_metadata_only_events_without_checkout_or_admin_merge assert "GitHub API request attempt" in workflow assert "Trusted governance ref must be a full commit SHA" in workflow assert "trusted_archive_candidate" in workflow + assert "live pull request base SHA" in workflow + assert "Supplied governance base SHA does not match" in workflow + assert "Manual governance base SHA must equal" in workflow assert "tar -tzf" in workflow + assert "PurePosixPath" in workflow + assert "member.issym()" in workflow + assert "member.islnk()" in workflow + assert "member.isdev()" in workflow + assert "os.path.commonpath" in workflow + assert "archive.extractall" in workflow + assert "[ -L \"$governance_script\" ]" in workflow + assert "tar -xzf \"$trusted_archive\"" not in workflow assert "Trusted governance archive materialization attempt" in workflow assert "after 4 attempts" in workflow assert 'bash "$GOVERNANCE_GATE"' in workflow diff --git a/backend/tests/test_repo_hygiene.py b/backend/tests/test_repo_hygiene.py index 86316f80f..a34e0483b 100644 --- a/backend/tests/test_repo_hygiene.py +++ b/backend/tests/test_repo_hygiene.py @@ -254,11 +254,21 @@ def test_compose_wrapper_uses_operator_env_file_without_bulk_secret_injection(): assert "NARUON_ENV_FILE" in wrapper assert "${HOME}/.env" in wrapper - assert 'docker compose --env-file "${env_file}" "$@"' in wrapper + assert 'docker compose --env-file "${env_file}" "${compose_files[@]}" "$@"' in wrapper assert "env_file:" not in gateway_compose assert "env_file:" not in local_compose +def test_macos_compose_keeps_chat_and_embedding_endpoints_separate(): + wrapper = (REPO_ROOT / "scripts" / "naruon_compose.sh").read_text() + macos_compose = (REPO_ROOT / "docker-compose.macos.yml").read_text() + + assert "host_embedding_endpoint_ready" in wrapper + assert "NARUON_LLAMA_CPP_EMBEDDING_BASE_URL" in wrapper + assert "OPENAI_EMBEDDING_BASE_URL" in macos_compose + assert "host.docker.internal:8082/v1" in wrapper or "8082/v1" in wrapper + + def test_postgres_ha_compose_requires_external_postgres_password(): compose = (REPO_ROOT / "docker-compose.postgres-ha.yml").read_text() diff --git a/docker-compose.macos.yml b/docker-compose.macos.yml new file mode 100644 index 000000000..14cf700db --- /dev/null +++ b/docker-compose.macos.yml @@ -0,0 +1,24 @@ +services: + # Host LLM selection is used when mlx-lm or llama.cpp is available. Ollama + # remains in the base Compose file as the final fallback. + ollama: + profiles: + - ollama + + backend: + depends_on: !override + db: + condition: service_healthy + newsdom: + condition: service_started + required: false + environment: + ALLOW_LOCAL_LLM_PROVIDERS: "true" + ALLOWED_LLM_BASE_URL_HOSTS: ${NARUON_HOST_LLM_ALLOWED_HOSTS:-host.docker.internal} + OPENAI_API_KEY: ${NARUON_HOST_LLM_API_KEY-} + OPENAI_BASE_URL: ${NARUON_HOST_LLM_BASE_URL:-http://host.docker.internal:8080/v1} + OPENAI_EMBEDDING_BASE_URL: ${NARUON_HOST_LLM_EMBEDDING_BASE_URL-} + OPENAI_EMBEDDING_MODEL: ${NARUON_HOST_LLM_EMBEDDING_MODEL:-embeddinggemma} + OPENAI_MODEL: ${NARUON_HOST_LLM_MODEL:-gemma4:e2b-it-qat} + extra_hosts: + - "host.docker.internal:host-gateway" diff --git a/docker-compose.yml b/docker-compose.yml index a6adf60cb..7236ab377 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -70,6 +70,7 @@ services: DATABASE_URL: postgresql+asyncpg://postgres:${POSTGRES_PASSWORD}@db:5432/ai_email READONLY_DATABASE_URL: ${READONLY_DATABASE_URL:-} DEBUG: "false" + ALLOWED_CORS_ORIGINS: ${ALLOWED_CORS_ORIGINS:-http://localhost:3000,http://127.0.0.1:3000,http://localhost:8000,http://127.0.0.1:8000} ALLOW_LOCAL_LLM_PROVIDERS: "true" ALLOWED_LLM_BASE_URL_HOSTS: ollama # NewsDOM sidecar allowlist. The actual base_url + bearer token are diff --git a/docs/adr/0001-local-llm-and-orchestrator-boundary.md b/docs/adr/0001-local-llm-and-orchestrator-boundary.md new file mode 100644 index 000000000..65dc38d73 --- /dev/null +++ b/docs/adr/0001-local-llm-and-orchestrator-boundary.md @@ -0,0 +1,58 @@ +# ADR-0001: Local LLM and contextual-orchestrator boundary + +- Status: Accepted +- Date: 2026-08-11 + +## Context + +Naruon must run on macOS with Colima while retaining a portable Docker Compose +fallback. The host already exposes `mlx-lm`; `llama.cpp` is the next local +runtime, and the existing Compose Ollama service remains the final fallback. +Naruon already has OpenAI-compatible provider validation, project-graph LLM +extraction, and a contextual-orchestrator batch-embedding seam. MLX chat and +EmbeddingGemma may be separate local OpenAI-compatible endpoints. + +## Decision + +1. On macOS, `scripts/naruon_compose.sh` probes `/v1/models` in this order: + `mlx-lm`, `llama.cpp`, then Ollama. `NARUON_COMPOSE_LLM_RUNTIME` can select + one explicitly. +2. `mlx-lm` is the preferred chat runtime. EmbeddingGemma remains the preferred + embedding model. When `OPENAI_EMBEDDING_BASE_URL` is configured for the + local-environment provider, search and import use that separate endpoint; + otherwise an embedding-capable `llama.cpp` or contextual-orchestrator + endpoint must be configured for real vectors because the installed MLX + server exposes chat completions but not `/v1/embeddings`. Local structured, + translation, and draft calls pass MLX's `enable_thinking=false` template + argument so reasoning tokens cannot exhaust the response before content. + Import paths retain Naruon's existing zero-vector fallback. +3. Integration with contextual-orchestrator uses the existing OpenAI-compatible + base-URL/provider seam and the existing batch embedding contract. No bespoke + client or duplicate routing layer is added. +4. Every host-provider URL remains subject to the existing SSRF guard and + explicit `host.docker.internal` allowlist entry. +5. In local Compose only, when `ALLOW_LOCAL_LLM_PROVIDERS` is enabled and the + configured base URL resolves to an allowlisted local host, the explicit + process-level host runtime takes precedence over a tenant API-key row. This + prevents a tenant secret from being sent to a local server. An active DB + provider remains authoritative, and external base URLs retain tenant + configuration behavior. +6. `llmfit` selects the device-appropriate EmbeddingGemma candidate before + installation. The official `ggml-org/embeddinggemma-300M-GGUF` artifact is + cached through llama.cpp for an embedding-capable endpoint; an + embedding-only server is not treated as a chat fallback. The macOS + `homebrew.mxcl.naruon-embeddinggemma` LaunchAgent serves the verified local + model on port 8082 when installed, and the Compose wrapper auto-detects it. +7. The separate embedding base URL is a local-environment setting only. It is + never copied onto a tenant-configured external provider or an organization + DB provider, preventing an external tenant API key from being sent to a + local endpoint. + +## Consequences + +- The normal macOS stack starts without building the large Ollama image. +- Ollama remains available with `NARUON_COMPOSE_LLM_RUNTIME=ollama`. +- Embedding quality is explicit: EmbeddingGemma is preferred, but a chat-only + MLX endpoint does not silently pretend to provide embeddings. +- A local MLX chat server and local llama.cpp embedding server can run + concurrently without sharing a port or crossing tenant provider boundaries. diff --git a/docs/adr/0002-compound-snake-case-database-names.md b/docs/adr/0002-compound-snake-case-database-names.md new file mode 100644 index 000000000..6185f990c --- /dev/null +++ b/docs/adr/0002-compound-snake-case-database-names.md @@ -0,0 +1,48 @@ +# ADR-0002: Compound snake_case names for database objects + +- Status: Accepted +- Date: 2026-08-11 + +## Context + +Database names are long-lived API surface. Single-token names and mixed-case +names make migrations, SQL review, and cross-service integration harder to +read consistently. + +## Decision + +All new or changed PostgreSQL/SQLAlchemy/Alembic database object names must be +lowercase compound `snake_case` with at least two components: + +```text +^[a-z][a-z0-9]*(?:_[a-z0-9]+)+$ +``` + +This applies to tables, columns, indexes, constraints, sequences, views, +functions, and other relational objects. Examples: `email_records`, +`status_code`, `ix_email_records_owner_date`. Single-token names such as +`emails`, `id`, `title`, and `status` are not valid for new relational +objects. + +GraphDB labels, relationship types, and other graph-native type identifiers are +outside this relational naming rule and must use `CamelCase` or `PascalCase` +(for example, `ProjectTask` or `EmailThread`). They must not be forced through +the relational `snake_case` rule. + +Values in relational `project_graph_*` columns such as `object_type` and +`edge_type` are application data, not GraphDB identifiers. They may remain +canonical application enum values until a real GraphDB adapter exists. That +adapter must convert its labels and relationship types to the graph-native +`CamelCase`/`PascalCase` form at the integration boundary. + +Existing legacy names remain stable for compatibility. A deliberate rename +requires an explicit migration, dependency inventory, rollback plan, and +verification; this ADR does not authorize opportunistic bulk renames. + +## Consequences + +- New migrations and ORM metadata must use compound `snake_case` names. +- Review and CI should inspect changed schema definitions and migrations while + grandfathering untouched legacy objects. +- GraphDB integration can preserve graph-native `CamelCase`/`PascalCase` types + without weakening the relational boundary. diff --git a/docs/adr/0003-pr-checks-and-non-admin-merge.md b/docs/adr/0003-pr-checks-and-non-admin-merge.md new file mode 100644 index 000000000..03b5883f6 --- /dev/null +++ b/docs/adr/0003-pr-checks-and-non-admin-merge.md @@ -0,0 +1,33 @@ +# ADR-0003: PR checks, review fixes, and non-admin merge control + +- Status: Accepted +- Date: 2026-08-11 + +## Decision + +- PR and review feedback is handled on the current head. After each fix, local + tests and the current-head GitHub Checks are re-evaluated; stale check results + are not treated as evidence for a newer commit. +- The delivery loop repeats `review -> correction -> current-head checks -> + review` until all actionable findings are resolved or explicitly superseded + with evidence. The agent may perform each in-scope iteration autonomously. +- Actionable failed Checks may be diagnosed and fixed autonomously within the + branch. Pending or queued work is a wait state, not a reason to invent a + bypass. +- No condition receives a generic `Blocker` status. A failed check or review + finding becomes a correction task; a pending external operation remains a + wait state while other safe work continues. +- Merge may be scheduled through the repository's permitted merge-queue or + auto-merge mechanism, or performed manually by the authorized agent/user + after review and required Checks are satisfied. The same agent may complete + that non-admin merge action; it must not claim administrator authority. +- No administrator merge, branch-protection bypass, human or third-party review + dismissal, token disclosure, or forced merge is part of this workflow. The + repository-approved central scheduler may clean up stale automated bot + review state only after rechecking the current head; that cleanup is not a + substitute for resolving an actionable finding. + +## Consequences + +Delivery evidence stays tied to the exact PR head, while human review and the +repository's normal protection rules retain authority over the final merge. diff --git a/docs/adr/0004-input-safety-and-evidence-first-judgment.md b/docs/adr/0004-input-safety-and-evidence-first-judgment.md new file mode 100644 index 000000000..13f08a91f --- /dev/null +++ b/docs/adr/0004-input-safety-and-evidence-first-judgment.md @@ -0,0 +1,39 @@ +# ADR-0004: Fail-closed input safety and evidence-first judgment + +- Status: Accepted +- Date: 2026-08-11 + +## Context + +The live Compose smoke exposed a PostgreSQL failure caused by a NUL byte in a +`text` advisory-lock parameter. Signed session claims also accepted ASCII NUL +characters because ASCII validation alone is insufficient. Separately, the +project-graph configuration described keyword extraction as fallback/reference +only while selecting it as the default. + +## Decision + +- Reject NUL and other control characters at authentication, local HTTP, and + other trust boundaries; add a regression test whenever a boundary is changed. +- Never pass a raw NUL-delimited composite value to a PostgreSQL text parameter. + Derive lock keys from a deterministic NUL-free digest and reject NUL-bearing + owner identities before persistence or locking. +- Keyword matching is not a base judgment. Grounded LLM or + contextual-orchestrator extraction is the default; deterministic keyword + extraction is explicit reference/diagnostic evidence and a last-resort + fallback with provenance, not an authoritative decision. +- Any suspicious behavior is an actionable defect: trace its shared root cause, + make the smallest safe correction, and rerun the focused and live checks. A + pending external operation is a wait state, not permission to bypass safety + or to stop investigating. +- Live smoke tooling must receive session secrets through the environment and + must never print bearer tokens or console snippets that reproduce them. + +## Consequences + +- Malformed identities and unsafe text fail closed before reaching PostgreSQL or + external requests. +- Local reference extraction remains available for diagnostics, while default + semantic judgments require grounded evidence. +- The test and live-smoke loop becomes part of the correction contract rather + than an optional postscript. diff --git a/docs/adr/0005-keyverse-oidc-trust-boundary.md b/docs/adr/0005-keyverse-oidc-trust-boundary.md new file mode 100644 index 000000000..9a74bd23d --- /dev/null +++ b/docs/adr/0005-keyverse-oidc-trust-boundary.md @@ -0,0 +1,35 @@ +# ADR-0005: Keyverse OIDC trust boundary + +- Status: Accepted +- Date: 2026-08-11 + +## Context + +`keyverse` is the ContextualWisdom ecosystem's central identity provider and +issues OIDC/OAuth credentials to Naruon and related relying parties. Naruon +already contains generic OIDC/JWKS verification, browser PKCE routes, and +strict issuer/JWKS host validation, but the local smoke path can use signed HMAC +sessions and does not start the separate Keyverse repository. + +## Decision + +- Production and multi-user membership authority uses Keyverse OIDC/JWKS. The + operator must configure `OIDC_ISSUER_URL`, `OIDC_CLIENT_ID`, + `OIDC_JWKS_URL`, and exact `ALLOWED_OIDC_HOSTS` together. +- The HMAC session path remains a local/control-plane compatibility path for + smoke tests only. It is not authoritative evidence for cross-workspace or + security-posture membership. +- Naruon does not copy Keyverse secrets, embed its Keycloak deployment, or + invent a second identity protocol. The integration stays at the existing + OIDC/JWKS boundary and must fail closed on partial or unsafe configuration. +- The Keyverse dependency and its readiness/configuration evidence must be + checked before a production-like browser authentication claim is accepted. + +## Consequences + +- The local Colima stack remains independently runnable with a fixture HMAC + session, while production authentication has an explicit external trust + boundary. +- Keyverse deployment/configuration is an operator concern and is not silently + replaced by a local fallback. +- OIDC issuer and JWKS DNS/HTTPS/allowlist protections remain mandatory. diff --git a/docs/adr/0006-privileged-workflow-archive-safety.md b/docs/adr/0006-privileged-workflow-archive-safety.md new file mode 100644 index 000000000..78d8c6af4 --- /dev/null +++ b/docs/adr/0006-privileged-workflow-archive-safety.md @@ -0,0 +1,37 @@ +# ADR-0006: Safe materialization for privileged workflow gates + +- Status: Accepted +- Date: 2026-08-11 + +## Context + +`pull_request_target` and central required workflows have write-capable +metadata or status permissions. They must execute only trusted workflow logic, +even when the event carries an untrusted pull request head. A tarball is a +convenient immutable-base transport, but a generic archive extraction step can +permit path traversal or link-based writes if its assumptions change. + +## Decision + +- Materialize governance code only from a full commit SHA resolved from the + trusted base or a live PR base. The PR head is data for current-head evidence, + never the privileged gate implementation. +- Validate every archive member before extraction: reject absolute paths, + parent-directory components, symbolic links, hard links, device entries, and + any resolved target outside the temporary workspace. +- Require the materialized governance script to exist as a regular file before + executing it. Privileged jobs do not checkout or execute untrusted PR code, + and third-party Actions remain pinned to full commit SHAs. +- Treat scanner failures as findings or wait states according to their explicit + gate contract; do not hide a hard scan failure with a generic continuation or + bypass branch protection. + +## Consequences + +- A malformed or unexpectedly structured trusted archive fails closed before + any write-capable gate logic runs. +- Current-head review, Checks, and merge decisions stay separate from the + trusted implementation that evaluates them. +- A central workflow repository remains the authority for organization-wide + OpenCode, Strix, and merge-scheduler behavior; Naruon records and verifies + the contract but does not silently fork or mutate that external source. diff --git a/docs/architecture/kg-extractor-seam.md b/docs/architecture/kg-extractor-seam.md index 1e1fe9e59..365a522a2 100644 --- a/docs/architecture/kg-extractor-seam.md +++ b/docs/architecture/kg-extractor-seam.md @@ -59,7 +59,7 @@ authority (mirroring the plan's plugin-context principle, §7.2). `KgExtractorRegistry` maps the stable `PROJECT_GRAPH_EXTRACTOR` selector value to an extractor. `resolve_extractor_chain(selector)` returns an ordered chain whose -**terminal element is always the deterministic keyword extractor**: +**terminal element is always the deterministic reference extractor**: | selector | chain | | --- | --- | @@ -74,8 +74,9 @@ orchestrator endpoint) raises `ExtractorUnavailableError`; a genuine failure raises anything else. Both cause the runner to advance to the next extractor. Because the deterministic keyword extractor is pure and always produces a result, "rule-based extraction is fallback/reference only" is guaranteed *by -construction* — not by remembering to write a fallback branch. The projection is -best-effort and never lost. +construction* — not by remembering to write a fallback branch. The default +selector is grounded `orchestrator`; `keyword` must be selected explicitly for +diagnostic/reference use. The projection is best-effort and never lost. Adding an extractor — including a future plugin on the `kg.extractor` extension point — is now `registry.register("selector", MyExtractor())` plus a config @@ -125,7 +126,7 @@ contextual-orchestrator (see naruon#973). - `PROJECT_GRAPH_EXTRACTION_ENABLED` (default `false`) — gates whether ingest snapshots segments for projection at all. -- `PROJECT_GRAPH_EXTRACTOR` (default `keyword`) — `keyword` | `llm` | +- `PROJECT_GRAPH_EXTRACTOR` (default `orchestrator`) — `keyword` | `llm` | `orchestrator`. - `PROJECT_GRAPH_ORCHESTRATOR_BASE_URL` (default unset) — OpenAI-compatible orchestrator endpoint for `orchestrator` routing; HTTPS + allowlisted. diff --git a/scripts/naruon_compose.sh b/scripts/naruon_compose.sh index 242bba09c..8c6c60754 100755 --- a/scripts/naruon_compose.sh +++ b/scripts/naruon_compose.sh @@ -1,6 +1,8 @@ #!/usr/bin/env bash set -euo pipefail +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + if [ -n "${NARUON_ENV_FILE:-}" ]; then env_file="${NARUON_ENV_FILE}" elif [ -f "${HOME}/.env" ]; then @@ -17,10 +19,93 @@ EOF exit 1 fi +llm_runtime="${NARUON_COMPOSE_LLM_RUNTIME:-auto}" + +host_llm_endpoint_ready() { + curl -fsS --max-time "${NARUON_LLM_PROBE_TIMEOUT_SECONDS:-1}" \ + "${1%/}/models" >/dev/null 2>&1 +} + +host_embedding_endpoint_ready() { + curl -fsS --max-time "${NARUON_LLM_PROBE_TIMEOUT_SECONDS:-1}" \ + -H "Authorization: Bearer ${NARUON_HOST_LLM_API_KEY:-mlx}" \ + -H 'Content-Type: application/json' \ + -d '{"model":"embeddinggemma","input":["embedding probe"]}' \ + "${1%/}/embeddings" >/dev/null 2>&1 +} + +container_host_url() { + case "$1" in + http://127.0.0.1:*) printf 'http://host.docker.internal:%s\n' "${1#http://127.0.0.1:}" ;; + http://localhost:*) printf 'http://host.docker.internal:%s\n' "${1#http://localhost:}" ;; + *) printf '%s\n' "$1" ;; + esac +} + +if [ "${llm_runtime}" = "auto" ]; then + llm_runtime="ollama" + if [ "$(uname -s)" = "Darwin" ]; then + if host_llm_endpoint_ready "${NARUON_MLX_BASE_URL:-http://127.0.0.1:8080/v1}"; then + llm_runtime="mlx" + elif host_llm_endpoint_ready "${NARUON_LLAMA_CPP_BASE_URL:-http://127.0.0.1:8081/v1}"; then + llm_runtime="llama.cpp" + fi + fi +fi + +case "${llm_runtime}" in + ollama) + compose_files=() + ;; + mlx|llama.cpp) + if [ "${llm_runtime}" = "mlx" ]; then + export NARUON_HOST_LLM_BASE_URL="${NARUON_MLX_BASE_URL:-http://host.docker.internal:8080/v1}" + export NARUON_HOST_LLM_ALLOWED_HOSTS="${NARUON_MLX_ALLOWED_LLM_BASE_URL_HOSTS:-host.docker.internal}" + # mlx-lm serves chat completions but not /v1/embeddings. A local + # placeholder key enables chat while embedding paths retain the + # zero-vector fallback unless an embedding-capable endpoint is configured. + export NARUON_HOST_LLM_API_KEY="${NARUON_MLX_OPENAI_API_KEY:-mlx}" + export NARUON_HOST_LLM_EMBEDDING_MODEL="${NARUON_MLX_EMBEDDING_MODEL:-embeddinggemma}" + export NARUON_HOST_LLM_MODEL="${NARUON_MLX_LLM_MODEL:-mlx-community/gemma-4-e4b-it-4bit}" + else + export NARUON_HOST_LLM_BASE_URL="${NARUON_LLAMA_CPP_BASE_URL:-http://host.docker.internal:8081/v1}" + export NARUON_HOST_LLM_ALLOWED_HOSTS="${NARUON_LLAMA_CPP_ALLOWED_LLM_BASE_URL_HOSTS:-host.docker.internal}" + export NARUON_HOST_LLM_API_KEY="${NARUON_LLAMA_CPP_API_KEY:-llama.cpp}" + export NARUON_HOST_LLM_EMBEDDING_MODEL="${NARUON_LLAMA_CPP_EMBEDDING_MODEL:-embeddinggemma}" + export NARUON_HOST_LLM_MODEL="${NARUON_LLAMA_CPP_LLM_MODEL:-gemma4:e2b-it-qat}" + fi + embedding_base_url="${NARUON_HOST_LLM_EMBEDDING_BASE_URL:-}" + if [ -z "${embedding_base_url}" ]; then + if [ "${llm_runtime}" = "mlx" ]; then + embedding_base_url="${NARUON_MLX_EMBEDDING_BASE_URL:-}" + else + embedding_base_url="${NARUON_LLAMA_CPP_EMBEDDING_BASE_URL:-}" + fi + fi + if [ -z "${embedding_base_url}" ]; then + embedding_probe_url="${NARUON_LLAMA_CPP_EMBEDDING_BASE_URL:-http://127.0.0.1:8082/v1}" + if host_embedding_endpoint_ready "${embedding_probe_url}"; then + embedding_base_url="${embedding_probe_url}" + fi + fi + if [ -n "${embedding_base_url}" ]; then + export NARUON_HOST_LLM_EMBEDDING_BASE_URL="$(container_host_url "${embedding_base_url}")" + fi + compose_files=( + --file "${repo_root}/docker-compose.yml" + --file "${repo_root}/docker-compose.macos.yml" + ) + ;; + *) + echo "Error: NARUON_COMPOSE_LLM_RUNTIME must be auto, ollama, mlx, or llama.cpp" >&2 + exit 1 + ;; +esac + for arg in "$@"; do if [ "${arg}" = "--env-file" ]; then - exec docker compose "$@" + exec docker compose "${compose_files[@]}" "$@" fi done -exec docker compose --env-file "${env_file}" "$@" +exec docker compose --env-file "${env_file}" "${compose_files[@]}" "$@" From 30323d49d0e2448377199aa00b991dfa2a457733 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 21:13:16 +0900 Subject: [PATCH 02/21] fix: address PR review findings --- AGENTS.md | 6 + .../alembic/versions/0011_email_read_state.py | 97 +++++++++++-- backend/api/auth.py | 8 +- backend/services/llm_provider_selection.py | 15 +- backend/tests/test_alembic_migrations.py | 129 +++++++++++++++++- backend/tests/test_auth_real.py | 9 ++ backend/tests/test_email_import_service.py | 8 +- backend/tests/test_llm_provider_selection.py | 41 ++++++ backend/tests/test_llm_service.py | 3 + backend/tests/test_local_http.py | 10 +- ...001-local-llm-and-orchestrator-boundary.md | 18 +++ .../adr/0003-pr-checks-and-non-admin-merge.md | 18 +++ ...nput-safety-and-evidence-first-judgment.md | 17 +++ docs/adr/0005-keyverse-oidc-trust-boundary.md | 17 +++ ...0006-privileged-workflow-archive-safety.md | 18 +++ scripts/naruon_compose.sh | 6 +- 16 files changed, 392 insertions(+), 28 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 35a593547..dfd5fb8cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,6 +64,12 @@ in this repo. that unlocks the registry and keep env strictly as bootstrap transport; do not add further `os.getenv` secret reads, and migrate toward the KV pattern as it is adopted. +- **Approved test-only exception:** `LIVE_E2E_SESSION_SECRET` may be read from + the environment by `backend/scripts/private_mail_http_smoke.py` only for a + local/private live-smoke run. It must equal the controlled test + `AUTH_SESSION_HMAC_SECRET`, must never be logged or persisted, and is not an + application or production credential source. New runtime paths must use the + credential registry instead. ### This repo's role in the ecosystem diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index 03a52096f..d14359159 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -13,27 +13,98 @@ depends_on = None _EMAIL_TABLE = "email_records" _READ_STATE_COLUMN = "is_read" +_OWNERSHIP_TABLE = "email_read_state_ownership" +_OWNERSHIP_KEY_COLUMN = "ownership_key" +_OWNERSHIP_KEY = "0011_email_read_state:email_records:is_read" + + +def _existing_read_state_column(inspector) -> dict | None: + return next( + ( + column + for column in inspector.get_columns(_EMAIL_TABLE) + if column["name"] == _READ_STATE_COLUMN + ), + None, + ) + + +def _reject_pre_existing_read_state(column: dict) -> None: + column_type = column.get("type") + canonical_shape = ( + isinstance(column_type, sa.Boolean) and column.get("nullable") is False + ) + shape_label = "compatible" if canonical_shape else "incompatible" + raise RuntimeError( + "0011_email_read_state cannot safely claim a pre-existing " + f"{shape_label} {_EMAIL_TABLE}.{_READ_STATE_COLUMN} column; " + "reconcile the schema before applying this migration" + ) + + +def _ownership_record_exists(connection) -> bool: + ownership_table = sa.table( + _OWNERSHIP_TABLE, + sa.column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120)), + ) + return ( + connection.execute( + sa.select(ownership_table.c[_OWNERSHIP_KEY_COLUMN]) + .where( + ownership_table.c[_OWNERSHIP_KEY_COLUMN] == _OWNERSHIP_KEY, + ) + .limit(1) + ).first() + is not None + ) def upgrade() -> None: connection = op.get_bind() inspector = sa.inspect(connection) - columns = {column["name"] for column in inspector.get_columns(_EMAIL_TABLE)} - if _READ_STATE_COLUMN not in columns: - op.add_column( - _EMAIL_TABLE, - sa.Column( - _READ_STATE_COLUMN, - sa.Boolean(), - nullable=False, - server_default=sa.text("true"), - ), - ) + if not inspector.has_table(_EMAIL_TABLE): + raise RuntimeError(f"required table {_EMAIL_TABLE} is missing") + if inspector.has_table(_OWNERSHIP_TABLE): + raise RuntimeError(f"unexpected pre-existing table {_OWNERSHIP_TABLE}") + + existing_column = _existing_read_state_column(inspector) + if existing_column is not None: + _reject_pre_existing_read_state(existing_column) + + op.add_column( + _EMAIL_TABLE, + sa.Column( + _READ_STATE_COLUMN, + sa.Boolean(), + nullable=False, + server_default=sa.text("true"), + ), + ) + op.create_table( + _OWNERSHIP_TABLE, + sa.Column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120), nullable=False), + sa.PrimaryKeyConstraint( + _OWNERSHIP_KEY_COLUMN, + name="pk_email_read_state_ownership", + ), + ) + ownership_table = sa.table( + _OWNERSHIP_TABLE, + sa.column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120)), + ) + op.bulk_insert( + ownership_table, + [{_OWNERSHIP_KEY_COLUMN: _OWNERSHIP_KEY}], + ) def downgrade() -> None: connection = op.get_bind() inspector = sa.inspect(connection) - columns = {column["name"] for column in inspector.get_columns(_EMAIL_TABLE)} - if _READ_STATE_COLUMN in columns: + if not inspector.has_table(_OWNERSHIP_TABLE): + return + if not _ownership_record_exists(connection): + return + if inspector.has_table(_EMAIL_TABLE) and _existing_read_state_column(inspector): op.drop_column(_EMAIL_TABLE, _READ_STATE_COLUMN) + op.drop_table(_OWNERSHIP_TABLE) diff --git a/backend/api/auth.py b/backend/api/auth.py index 1c81f9647..4cf125e1a 100644 --- a/backend/api/auth.py +++ b/backend/api/auth.py @@ -420,10 +420,12 @@ def _reject_signed_session_admin_payload(payload: dict[str, Any]) -> None: def _safe_ascii_claim(value: object) -> str | None: if not isinstance(value, str): return None - normalized = value.strip() - if not normalized or not normalized.isascii(): + if not value.isascii() or any( + ord(character) < 32 or ord(character) == 127 for character in value + ): return None - if any(ord(character) < 32 or ord(character) == 127 for character in normalized): + normalized = value.strip() + if not normalized: return None return normalized diff --git a/backend/services/llm_provider_selection.py b/backend/services/llm_provider_selection.py index a8387507b..91dfb07bd 100644 --- a/backend/services/llm_provider_selection.py +++ b/backend/services/llm_provider_selection.py @@ -50,6 +50,14 @@ def _provider_api_key(provider: LLMProvider) -> str | None: return None +def _is_local_runtime_base_url(base_url: str | None) -> bool: + try: + hostname = urlsplit(base_url or "").hostname + except ValueError: + return False + return hostname in LOCAL_RUNTIME_HOSTS + + async def get_active_llm_provider( session: AsyncSession, organization_id: str | None, @@ -73,7 +81,12 @@ def _runtime_from_provider(provider: LLMProvider) -> RuntimeLLMProvider | None: if not is_llm_provider_configured(provider): return None - api_key = _provider_api_key(provider) + if _is_local_runtime_base_url(provider.base_url): + if not _is_local_provider(provider): + return None + api_key = LOCAL_PROVIDER_API_KEY + else: + api_key = _provider_api_key(provider) if not api_key: return None diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index c05747df9..f0995d823 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -1,4 +1,9 @@ +import importlib.util from pathlib import Path +from types import SimpleNamespace + +import pytest +import sqlalchemy as sa BACKEND_ROOT = Path(__file__).resolve().parents[1] @@ -422,13 +427,131 @@ def test_merge_revision_reconciles_email_read_state_branch(): assert "op.drop_column(" not in revision_text -def test_email_read_state_revision_uses_canonical_email_records_table(): +def _load_email_read_state_revision(): revision_path = BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" - revision_text = revision_path.read_text() + spec = importlib.util.spec_from_file_location( + "email_read_state_revision", revision_path + ) + assert spec is not None and spec.loader is not None + revision = importlib.util.module_from_spec(spec) + spec.loader.exec_module(revision) + return revision + + +class _MigrationInspector: + def __init__(self, email_columns, *, ownership_table=False): + self.email_columns = email_columns + self.ownership_table = ownership_table + + def has_table(self, table_name): + if table_name == "email_records": + return True + if table_name == "email_read_state_ownership": + return self.ownership_table + raise AssertionError(table_name) + + def get_columns(self, table_name): + assert table_name == "email_records" + return self.email_columns + + +class _MigrationConnection: + def __init__(self, *, ownership_record=False): + self.ownership_record = ownership_record + + def execute(self, _statement): + return SimpleNamespace( + first=lambda: ("owned",) if self.ownership_record else None + ) + + +def _migration_operations(monkeypatch, revision, inspector, connection): + calls = [] + operations = SimpleNamespace( + get_bind=lambda: connection, + add_column=lambda *args: calls.append(("add_column", args)), + create_table=lambda *args: calls.append(("create_table", args)), + bulk_insert=lambda *args: calls.append(("bulk_insert", args)), + drop_column=lambda *args: calls.append(("drop_column", args)), + drop_table=lambda *args: calls.append(("drop_table", args)), + ) + monkeypatch.setattr(revision, "op", operations) + monkeypatch.setattr(revision.sa, "inspect", lambda _connection: inspector) + return calls + + +def test_email_read_state_revision_adds_and_records_column_ownership(monkeypatch): + revision = _load_email_read_state_revision() + inspector = _MigrationInspector([{"name": "id"}]) + connection = _MigrationConnection() + calls = _migration_operations(monkeypatch, revision, inspector, connection) + + revision.upgrade() + + assert [name for name, _args in calls] == [ + "add_column", + "create_table", + "bulk_insert", + ] + assert calls[0][1][0] == "email_records" + assert calls[0][1][1].name == "is_read" + assert calls[0][1][1].nullable is False + + +@pytest.mark.parametrize( + "column", + [ + {"name": "is_read", "type": sa.Boolean(), "nullable": False}, + {"name": "is_read", "type": sa.String(), "nullable": True}, + ], +) +def test_email_read_state_revision_rejects_pre_existing_column(monkeypatch, column): + revision = _load_email_read_state_revision() + inspector = _MigrationInspector([{"name": "id"}, column]) + connection = _MigrationConnection() + calls = _migration_operations(monkeypatch, revision, inspector, connection) + + with pytest.raises(RuntimeError, match="pre-existing"): + revision.upgrade() + + assert calls == [] + + +def test_email_read_state_revision_downgrade_only_removes_owned_column(monkeypatch): + revision = _load_email_read_state_revision() + inspector = _MigrationInspector( + [{"name": "id"}, {"name": "is_read"}], ownership_table=True + ) + connection = _MigrationConnection(ownership_record=True) + calls = _migration_operations(monkeypatch, revision, inspector, connection) + + revision.downgrade() + + assert [name for name, _args in calls] == ["drop_column", "drop_table"] + assert calls[0][1] == ("email_records", "is_read") + + +def test_email_read_state_revision_downgrade_preserves_unowned_column(monkeypatch): + revision = _load_email_read_state_revision() + inspector = _MigrationInspector( + [{"name": "id"}, {"name": "is_read"}], ownership_table=False + ) + connection = _MigrationConnection() + calls = _migration_operations(monkeypatch, revision, inspector, connection) + + revision.downgrade() + + assert calls == [] + + +def test_email_read_state_revision_uses_canonical_email_records_table(): + revision_text = ( + BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" + ).read_text() assert '_EMAIL_TABLE = "email_records"' in revision_text assert "inspector.get_columns(_EMAIL_TABLE)" in revision_text - assert 'op.add_column(\n _EMAIL_TABLE' in revision_text + assert "op.add_column(\n _EMAIL_TABLE" in revision_text def test_merge_revision_reconciles_newsdom_provider_branch(): diff --git a/backend/tests/test_auth_real.py b/backend/tests/test_auth_real.py index ca05b1004..c91f273df 100644 --- a/backend/tests/test_auth_real.py +++ b/backend/tests/test_auth_real.py @@ -23,6 +23,7 @@ get_current_user, is_admin_role, is_tenant_admin_role, + _safe_ascii_claim, ) from core.config import settings from db.session import get_db @@ -477,6 +478,14 @@ async def test_signed_bearer_session_rejects_non_ascii_claim_values(): assert exc.value.status_code == 401 +@pytest.mark.parametrize( + "claim_value", + ("\nalice", "alice\r", "\talice", "alice\t", "\u00a0alice"), +) +def test_safe_ascii_claim_rejects_control_or_non_ascii_prefixes(claim_value: str): + assert _safe_ascii_claim(claim_value) is None + + @pytest.mark.asyncio @pytest.mark.parametrize( ("claim_name", "claim_value"), diff --git a/backend/tests/test_email_import_service.py b/backend/tests/test_email_import_service.py index 78b0ae2f0..564a5fa66 100644 --- a/backend/tests/test_email_import_service.py +++ b/backend/tests/test_email_import_service.py @@ -54,12 +54,16 @@ def test_safe_upload_filename_fails_closed_beyond_decode_round_limit(): def test_owner_import_lock_key_is_nul_free_and_tuple_stable(): first = _owner_import_lock_key("user-1", "org-1") - second = _owner_import_lock_key("org-1", "user-1") + repeat = _owner_import_lock_key("user-1", "org-1") + swapped = _owner_import_lock_key("org-1", "user-1") assert "\x00" not in first - assert first != second + assert first == repeat + assert first != swapped with pytest.raises(ValueError, match="NUL"): _owner_import_lock_key("user\x00-1", "org-1") + with pytest.raises(ValueError, match="NUL"): + _owner_import_lock_key("user-1", "org\x00-1") @pytest.mark.parametrize( diff --git a/backend/tests/test_llm_provider_selection.py b/backend/tests/test_llm_provider_selection.py index 7a5f32c7e..c8c2d38a5 100644 --- a/backend/tests/test_llm_provider_selection.py +++ b/backend/tests/test_llm_provider_selection.py @@ -88,6 +88,47 @@ async def test_resolve_runtime_llm_provider_prefers_active_local_provider(): assert runtime_provider.embedding_model == "embeddinggemma" +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("provider_type", "expected_source", "expected_api_key"), + [ + ("openai", None, None), + ("ollama", "llm_provider", LOCAL_PROVIDER_API_KEY), + ], +) +async def test_resolve_runtime_llm_provider_never_sends_tenant_key_to_local_host( + monkeypatch, provider_type, expected_source, expected_api_key +): + monkeypatch.setattr(settings, "ALLOW_LOCAL_LLM_PROVIDERS", False) + monkeypatch.setattr(settings, "OPENAI_BASE_URL", "https://api.openai.com/v1") + provider = LLMProvider( + id=11, + user_id="admin", + organization_id="org-acme", + name="Local host provider", + provider_type=provider_type, + base_url="http://host.docker.internal:8080/v1", + model_identifier="local-chat", + embedding_model="embeddinggemma", + api_key="sk-tenant-must-not-be-forwarded", + is_active=True, + updated_at=datetime.datetime.now(datetime.timezone.utc), + ) + + runtime_provider = await resolve_runtime_llm_provider( + MockSession(providers=[provider], tenant_config=None), + user_id="testuser", + organization_id="org-acme", + ) + + if expected_source is None: + assert runtime_provider is None + else: + assert runtime_provider is not None + assert runtime_provider.provider_source == expected_source + assert runtime_provider.api_key == expected_api_key + + @pytest.mark.asyncio async def test_resolve_runtime_llm_provider_falls_back_to_tenant_config(monkeypatch): monkeypatch.setattr(settings, "OPENAI_BASE_URL", "https://api.openai.com/v1") diff --git a/backend/tests/test_llm_service.py b/backend/tests/test_llm_service.py index 0353f4a17..d3acd8b83 100644 --- a/backend/tests/test_llm_service.py +++ b/backend/tests/test_llm_service.py @@ -338,6 +338,9 @@ async def test_extract_action_items_and_summary_parses_local_json_content( "content" ] assert "Return only one valid JSON object" in system_content + assert mock_openai.chat.completions.create.call_args.kwargs["response_format"] == { + "type": "json_object" + } assert mock_openai.chat.completions.create.call_args.kwargs["extra_body"] == { "chat_template_kwargs": {"enable_thinking": False} } diff --git a/backend/tests/test_local_http.py b/backend/tests/test_local_http.py index 97f321907..e8052a0e1 100644 --- a/backend/tests/test_local_http.py +++ b/backend/tests/test_local_http.py @@ -82,10 +82,12 @@ def test_local_request_target_normalizes_malformed_parser_errors() -> None: validate_local_request_target("//[::1") -@pytest.mark.parametrize("value", ["http://127.0.0.1:18080/\x00", "http://127.0.0.1/%00"]) -def test_local_http_rejects_nul_characters(value: str) -> None: +@pytest.mark.parametrize( + "origin", ["http://127.0.0.1:18080/\x00", "http://127.0.0.1/%00"] +) +def test_local_http_rejects_nul_characters(origin: str) -> None: with pytest.raises(LocalHTTPValidationError): - validate_loopback_http_origin(value) + validate_loopback_http_origin(origin) with pytest.raises(LocalHTTPValidationError): - validate_local_request_target(value, allowed_exact_paths=frozenset()) + validate_local_request_target("/api/search/%00") diff --git a/docs/adr/0001-local-llm-and-orchestrator-boundary.md b/docs/adr/0001-local-llm-and-orchestrator-boundary.md index 65dc38d73..2f453fb13 100644 --- a/docs/adr/0001-local-llm-and-orchestrator-boundary.md +++ b/docs/adr/0001-local-llm-and-orchestrator-boundary.md @@ -56,3 +56,21 @@ EmbeddingGemma may be separate local OpenAI-compatible endpoints. MLX endpoint does not silently pretend to provide embeddings. - A local MLX chat server and local llama.cpp embedding server can run concurrently without sharing a port or crossing tenant provider boundaries. + +## References + +- Henrique Schechter Vera et al., [“EmbeddingGemma: Powerful and Lightweight + Text Representations”](https://arxiv.org/abs/2509.20354), arXiv:2509.20354 + (2025). The paper reports a 300M embedding model evaluated across multilingual, + English, and code tasks, including quantized and truncated variants; that + supports EmbeddingGemma as a low-memory embedding candidate, not as a chat + runtime. +- Niklas Muennighoff et al., [“MTEB: Massive Text Embedding + Benchmark”](https://arxiv.org/abs/2210.07316), arXiv:2210.07316 (2023). + MTEB spans multiple tasks, datasets, and languages and finds no universal + embedding method; this supports measuring the selected local model in + Naruon's retrieval workload instead of assuming that a chat model is a good + embedder. + +The source PDFs are not bundled because redistribution rights were not +established; stable links and summaries are provided instead. diff --git a/docs/adr/0003-pr-checks-and-non-admin-merge.md b/docs/adr/0003-pr-checks-and-non-admin-merge.md index 03b5883f6..a43d4292b 100644 --- a/docs/adr/0003-pr-checks-and-non-admin-merge.md +++ b/docs/adr/0003-pr-checks-and-non-admin-merge.md @@ -31,3 +31,21 @@ Delivery evidence stays tied to the exact PR head, while human review and the repository's normal protection rules retain authority over the final merge. + +## References + +- Enrico Fregnan, Fernando Petrulio, and Alberto Bacchelli, [“The evolution of + the code during review: an investigation on review + changes”](https://link.springer.com/article/10.1007/s10664-022-10205-7), + *Empirical Software Engineering* (2022). The study examines how code changes + evolve through review; this supports repeating review and focused validation + after each corrective commit. +- Santiago Torres-Arias et al., [“in-toto: Providing farm-to-table guarantees + for bits and bytes”](https://www.usenix.org/conference/usenixsecurity19/presentation/torres-arias), + *USENIX Security Symposium* (2019). The work models independent build and + delivery actors and verifies supply-chain integrity end to end; this supports + current-commit evidence, trusted workflow materialization, and no merge + bypass. The mapping to this PR gate is an engineering inference. + +The source PDFs are not bundled because redistribution rights were not +established; stable links and summaries are provided instead. diff --git a/docs/adr/0004-input-safety-and-evidence-first-judgment.md b/docs/adr/0004-input-safety-and-evidence-first-judgment.md index 13f08a91f..1fbe959be 100644 --- a/docs/adr/0004-input-safety-and-evidence-first-judgment.md +++ b/docs/adr/0004-input-safety-and-evidence-first-judgment.md @@ -37,3 +37,20 @@ only while selecting it as the default. semantic judgments require grounded evidence. - The test and live-smoke loop becomes part of the correction contract rather than an optional postscript. + +## References + +- Jerome H. Saltzer and Michael D. Schroeder, [“The Protection of Information + in Computer Systems”](https://www.cs.virginia.edu/~evans/cs551/saltzer/), + *Proceedings of the IEEE* 63(9) (1975). The paper's fail-safe defaults, + complete mediation, least privilege, and separation-of-privilege principles + support rejecting malformed input before persistence or authorization. +- Lianmin Zheng et al., [“Judging LLM-as-a-Judge with MT-Bench and Chatbot + Arena”](https://arxiv.org/abs/2306.05685), NeurIPS 2023. The paper documents + useful agreement with human preferences together with position, verbosity, + self-enhancement, and reasoning biases; this supports grounded evidence and + explicit provenance rather than keyword matching as an authoritative + judgment. + +The source PDFs are not bundled because redistribution rights were not +established; stable links and summaries are provided instead. diff --git a/docs/adr/0005-keyverse-oidc-trust-boundary.md b/docs/adr/0005-keyverse-oidc-trust-boundary.md index 9a74bd23d..067e16c6d 100644 --- a/docs/adr/0005-keyverse-oidc-trust-boundary.md +++ b/docs/adr/0005-keyverse-oidc-trust-boundary.md @@ -33,3 +33,20 @@ sessions and does not start the separate Keyverse repository. - Keyverse deployment/configuration is an operator concern and is not silently replaced by a local fallback. - OIDC issuer and JWKS DNS/HTTPS/allowlist protections remain mandatory. + +## References + +- [RFC 8725: JSON Web Token Best Current + Practices](https://www.rfc-editor.org/rfc/rfc8725.html) (IETF, 2020). + It requires applications to bind issuer claims to the issuer's cryptographic + keys and validate the subject; this supports the explicit issuer, JWKS, and + fail-closed checks at the Keyverse boundary. +- [RFC 9700: Best Current Practice for OAuth 2.0 + Security](https://www.rfc-editor.org/rfc/rfc9700.html) (IETF, 2025). + It recommends exact redirect handling, PKCE/nonce protections, audience + restriction, and authorization-server metadata; this supports keeping + Keyverse as the production identity authority while retaining HMAC only for + controlled smoke tests. + +The source PDFs are not bundled because redistribution rights were not +established; stable links and summaries are provided instead. diff --git a/docs/adr/0006-privileged-workflow-archive-safety.md b/docs/adr/0006-privileged-workflow-archive-safety.md index 78d8c6af4..e90df0a43 100644 --- a/docs/adr/0006-privileged-workflow-archive-safety.md +++ b/docs/adr/0006-privileged-workflow-archive-safety.md @@ -35,3 +35,21 @@ permit path traversal or link-based writes if its assumptions change. - A central workflow repository remains the authority for organization-wide OpenCode, Strix, and merge-scheduler behavior; Naruon records and verifies the contract but does not silently fork or mutate that external source. + +## References + +- Jerome H. Saltzer and Michael D. Schroeder, [“The Protection of Information + in Computer Systems”](https://www.cs.virginia.edu/~evans/cs551/saltzer/), + *Proceedings of the IEEE* 63(9) (1975). Least privilege, complete mediation, + and separation of privilege support keeping write-capable workflow logic + isolated from untrusted pull-request data. +- Santiago Torres-Arias et al., [“in-toto: Providing farm-to-table guarantees + for bits and bytes”](https://www.usenix.org/conference/usenixsecurity19/presentation/torres-arias), + *USENIX Security Symposium* (2019). The paper establishes end-to-end + provenance for software supply chains; this supports full-SHA trusted-source + materialization and current-head evidence. +- [OWASP Web Security Testing Guide: Test Upload of Malicious + Files](https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files). + Its archive-directory-traversal example motivates validating archive member + paths and links before extraction. No third-party paper PDF is bundled because + redistribution rights were not established for these sources. diff --git a/scripts/naruon_compose.sh b/scripts/naruon_compose.sh index 8c6c60754..7c5076a81 100755 --- a/scripts/naruon_compose.sh +++ b/scripts/naruon_compose.sh @@ -59,7 +59,8 @@ case "${llm_runtime}" in ;; mlx|llama.cpp) if [ "${llm_runtime}" = "mlx" ]; then - export NARUON_HOST_LLM_BASE_URL="${NARUON_MLX_BASE_URL:-http://host.docker.internal:8080/v1}" + host_llm_base_url="${NARUON_MLX_BASE_URL:-http://host.docker.internal:8080/v1}" + export NARUON_HOST_LLM_BASE_URL="$(container_host_url "${host_llm_base_url}")" export NARUON_HOST_LLM_ALLOWED_HOSTS="${NARUON_MLX_ALLOWED_LLM_BASE_URL_HOSTS:-host.docker.internal}" # mlx-lm serves chat completions but not /v1/embeddings. A local # placeholder key enables chat while embedding paths retain the @@ -68,7 +69,8 @@ case "${llm_runtime}" in export NARUON_HOST_LLM_EMBEDDING_MODEL="${NARUON_MLX_EMBEDDING_MODEL:-embeddinggemma}" export NARUON_HOST_LLM_MODEL="${NARUON_MLX_LLM_MODEL:-mlx-community/gemma-4-e4b-it-4bit}" else - export NARUON_HOST_LLM_BASE_URL="${NARUON_LLAMA_CPP_BASE_URL:-http://host.docker.internal:8081/v1}" + host_llm_base_url="${NARUON_LLAMA_CPP_BASE_URL:-http://host.docker.internal:8081/v1}" + export NARUON_HOST_LLM_BASE_URL="$(container_host_url "${host_llm_base_url}")" export NARUON_HOST_LLM_ALLOWED_HOSTS="${NARUON_LLAMA_CPP_ALLOWED_LLM_BASE_URL_HOSTS:-host.docker.internal}" export NARUON_HOST_LLM_API_KEY="${NARUON_LLAMA_CPP_API_KEY:-llama.cpp}" export NARUON_HOST_LLM_EMBEDDING_MODEL="${NARUON_LLAMA_CPP_EMBEDDING_MODEL:-embeddinggemma}" From cf8c21e4b0843aef49f8c15d4f0f93816178dc09 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 21:19:15 +0900 Subject: [PATCH 03/21] fix: execute trusted archive validation safely --- .github/workflows/pr-governance.yml | 29 +++++++++++++++++++++++- backend/tests/test_release_governance.py | 1 + 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-governance.yml b/.github/workflows/pr-governance.yml index 9cc87337c..de8e09101 100644 --- a/.github/workflows/pr-governance.yml +++ b/.github/workflows/pr-governance.yml @@ -227,7 +227,34 @@ jobs: echo "Trusted governance archive materialization attempt ${attempt} did not produce a valid archive; retrying." >&2 sleep $((attempt * 3)) done - python3 -c 'import os,sys,tarfile; from pathlib import Path,PurePosixPath; archive=tarfile.open(sys.argv[1],"r:gz"); workspace=Path(sys.argv[2]).resolve(); members=archive.getmembers(); target=lambda member:(workspace / Path(*PurePosixPath(member.name).parts[1:])).resolve(); unsafe=[member.name for member in members if (not member.name or member.name.startswith("/") or ".." in PurePosixPath(member.name).parts or member.issym() or member.islnk() or member.isdev() or os.path.commonpath((workspace,target(member))) != str(workspace))]; raise SystemExit(f"unsafe trusted governance archive member: {unsafe[0]!r}") if unsafe else None; archive.extractall(workspace,members=members)' "$trusted_archive" "$trusted_workspace" + python3 - "$trusted_archive" "$trusted_workspace" <<'PY' + import os + import sys + import tarfile + from pathlib import Path, PurePosixPath + + archive_path, workspace_path = sys.argv[1:3] + workspace = Path(workspace_path).resolve() + with tarfile.open(archive_path, "r:gz") as archive: + members = archive.getmembers() + unsafe = [] + for member in members: + member_path = PurePosixPath(member.name) + target = (workspace / Path(*member_path.parts[1:])).resolve() + if ( + not member.name + or member.name.startswith("/") + or ".." in member_path.parts + or member.issym() + or member.islnk() + or member.isdev() + or os.path.commonpath((workspace, target)) != str(workspace) + ): + unsafe.append(member.name) + if unsafe: + raise SystemExit(f"unsafe trusted governance archive member: {unsafe[0]!r}") + archive.extractall(workspace, members=members) + PY test -d "$trusted_workspace/scripts" governance_script="$trusted_workspace/scripts/ci/pr_governance_gate.sh" if [ ! -f "$governance_script" ] || [ -L "$governance_script" ]; then diff --git a/backend/tests/test_release_governance.py b/backend/tests/test_release_governance.py index 1a7d02743..7d4f6ff13 100644 --- a/backend/tests/test_release_governance.py +++ b/backend/tests/test_release_governance.py @@ -1117,6 +1117,7 @@ def test_pr_governance_uses_metadata_only_events_without_checkout_or_admin_merge assert "member.islnk()" in workflow assert "member.isdev()" in workflow assert "os.path.commonpath" in workflow + assert "if unsafe:" in workflow assert "archive.extractall" in workflow assert "[ -L \"$governance_script\" ]" in workflow assert "tar -xzf \"$trusted_archive\"" not in workflow From 9092b2238ed6519518653d2fbeaac4e39aa5d5d4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 12:37:56 +0900 Subject: [PATCH 04/21] fix: harden live mail import embeddings and locks --- .../alembic/versions/0011_email_read_state.py | 56 ++++++++------- backend/services/email_import_service.py | 68 +++++++++++++++++-- backend/services/embedding.py | 53 ++++++++++++++- backend/tests/test_alembic_migrations.py | 37 ++++++---- backend/tests/test_email_import_service.py | 60 ++++++++++++++++ backend/tests/test_embedding.py | 31 +++++++++ ...001-local-llm-and-orchestrator-boundary.md | 9 +++ ...nput-safety-and-evidence-first-judgment.md | 12 ++++ 8 files changed, 280 insertions(+), 46 deletions(-) diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index d14359159..2f4ecfc78 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -29,17 +29,17 @@ def _existing_read_state_column(inspector) -> dict | None: ) -def _reject_pre_existing_read_state(column: dict) -> None: +def _validate_pre_existing_read_state(column: dict) -> None: column_type = column.get("type") canonical_shape = ( isinstance(column_type, sa.Boolean) and column.get("nullable") is False ) - shape_label = "compatible" if canonical_shape else "incompatible" - raise RuntimeError( - "0011_email_read_state cannot safely claim a pre-existing " - f"{shape_label} {_EMAIL_TABLE}.{_READ_STATE_COLUMN} column; " - "reconcile the schema before applying this migration" - ) + if not canonical_shape: + raise RuntimeError( + "0011_email_read_state cannot safely use an incompatible " + f"pre-existing {_EMAIL_TABLE}.{_READ_STATE_COLUMN} column; " + "reconcile the schema before applying this migration" + ) def _ownership_record_exists(connection) -> bool: @@ -68,18 +68,20 @@ def upgrade() -> None: raise RuntimeError(f"unexpected pre-existing table {_OWNERSHIP_TABLE}") existing_column = _existing_read_state_column(inspector) - if existing_column is not None: - _reject_pre_existing_read_state(existing_column) + owns_read_state_column = existing_column is None + if owns_read_state_column: + op.add_column( + _EMAIL_TABLE, + sa.Column( + _READ_STATE_COLUMN, + sa.Boolean(), + nullable=False, + server_default=sa.text("true"), + ), + ) + else: + _validate_pre_existing_read_state(existing_column) - op.add_column( - _EMAIL_TABLE, - sa.Column( - _READ_STATE_COLUMN, - sa.Boolean(), - nullable=False, - server_default=sa.text("true"), - ), - ) op.create_table( _OWNERSHIP_TABLE, sa.Column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120), nullable=False), @@ -92,10 +94,11 @@ def upgrade() -> None: _OWNERSHIP_TABLE, sa.column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120)), ) - op.bulk_insert( - ownership_table, - [{_OWNERSHIP_KEY_COLUMN: _OWNERSHIP_KEY}], - ) + if owns_read_state_column: + op.bulk_insert( + ownership_table, + [{_OWNERSHIP_KEY_COLUMN: _OWNERSHIP_KEY}], + ) def downgrade() -> None: @@ -103,8 +106,11 @@ def downgrade() -> None: inspector = sa.inspect(connection) if not inspector.has_table(_OWNERSHIP_TABLE): return - if not _ownership_record_exists(connection): - return - if inspector.has_table(_EMAIL_TABLE) and _existing_read_state_column(inspector): + owns_read_state_column = _ownership_record_exists(connection) + if ( + owns_read_state_column + and inspector.has_table(_EMAIL_TABLE) + and _existing_read_state_column(inspector) + ): op.drop_column(_EMAIL_TABLE, _READ_STATE_COLUMN) op.drop_table(_OWNERSHIP_TABLE) diff --git a/backend/services/email_import_service.py b/backend/services/email_import_service.py index f9afda114..277316302 100644 --- a/backend/services/email_import_service.py +++ b/backend/services/email_import_service.py @@ -15,9 +15,10 @@ from typing import Literal from sqlalchemy import bindparam, func, or_, select -from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.ext.asyncio import AsyncConnection, AsyncSession from core.config import settings +from db.session import engine from db.models import ( Attachment, ContentNodeRecord, @@ -27,6 +28,7 @@ ) from services.archive import extract_backup_async from services.batch_embedding_service import try_batch_import_embeddings +from services.circuit_breaker import CircuitOpenError from services.content_graph import ParseResult, parse_content from services.email_dedupe_service import strong_email_fingerprint from services.email_parser import EmailData, parse_eml_bytes @@ -257,13 +259,35 @@ def _owner_import_lock_key(user_id: str, organization_id: str) -> str: async def _acquire_owner_import_quota_lock( session: AsyncSession, *, user_id: str, organization_id: str -) -> bool: +) -> AsyncConnection | bool | None: if not _session_uses_postgresql(session): - return False + return None lock_params = { "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, "owner_key": _owner_import_lock_key(user_id, organization_id), } + # session.commit() is intentionally used for each imported item. A + # session-level advisory lock acquired through AsyncSession can therefore + # be left on a returned pooled connection; keep a dedicated connection for + # the whole import so acquisition and release are guaranteed to match. + if isinstance(session, AsyncSession): + lock_connection = await engine.connect() + try: + await lock_connection.execute( + select( + func.pg_advisory_lock( + func.hashtext(bindparam("namespace_key")), + func.hashtext(bindparam("owner_key")), + ) + ), + lock_params, + ) + except Exception: + await lock_connection.close() + raise + return lock_connection + + # Lightweight PostgreSQL test doubles retain the old query contract. await session.execute( select( func.pg_advisory_lock( @@ -277,8 +301,34 @@ async def _acquire_owner_import_quota_lock( async def _release_owner_import_quota_lock( - session: AsyncSession, *, user_id: str, organization_id: str + session: AsyncSession, + *, + user_id: str, + organization_id: str, + lock: AsyncConnection | bool | None, ) -> None: + if lock is not None and lock is not True: + lock_params = { + "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, + "owner_key": _owner_import_lock_key(user_id, organization_id), + } + try: + await lock.execute( + select( + func.pg_advisory_unlock( + func.hashtext(bindparam("namespace_key")), + func.hashtext(bindparam("owner_key")), + ) + ), + lock_params, + ) + await lock.commit() + finally: + await lock.close() + return + if lock is not True: + return + lock_params = { "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, "owner_key": _owner_import_lock_key(user_id, organization_id), @@ -942,7 +992,7 @@ async def _generate_import_embeddings( or embedding_provider.base_url, model=embedding_provider.embedding_model, ) - except (EmbeddingGenerationError, ValueError) as exc: + except (CircuitOpenError, EmbeddingGenerationError, ValueError) as exc: logger.warning( "Email import embedding generation failed; retrying imported content " "item by item before zero-vector fallback: " @@ -967,6 +1017,7 @@ async def _generate_import_embeddings( fit_embedding_vector(single_embedding[0], EMBEDDING_DIMENSION) ) except ( + CircuitOpenError, EmbeddingGenerationError, ValueError, TypeError, @@ -1179,9 +1230,12 @@ async def import_email_uploads( ) ) finally: - if lock_acquired: + if lock_acquired is not None: await _release_owner_import_quota_lock( - session, user_id=user_id, organization_id=organization_id + session, + user_id=user_id, + organization_id=organization_id, + lock=lock_acquired, ) return result diff --git a/backend/services/embedding.py b/backend/services/embedding.py index b22cca8f4..605d42bef 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -8,6 +8,10 @@ from services.retry import retry_transient STORAGE_EMBEDDING_DIMENSION = 1536 +# Keep each request below the smallest local embedding context observed in the +# supported llama.cpp/EmbeddingGemma runtime. Longer source items are pooled +# back to one vector per caller input. +EMBEDDING_INPUT_CHUNK_SIZE = 256 def chunk_text( @@ -36,6 +40,46 @@ def fit_embedding_vector( return embedding[:target_dimension] +def _split_embedding_inputs( + texts: list[str], +) -> tuple[list[str], list[tuple[int, int]]]: + flattened: list[str] = [] + ranges: list[tuple[int, int]] = [] + for text in texts: + chunks = chunk_text( + text, + chunk_size=EMBEDDING_INPUT_CHUNK_SIZE, + chunk_overlap=0, + ) or [text] + start = len(flattened) + flattened.extend(chunks) + ranges.append((start, len(flattened))) + return flattened, ranges + + +def _pool_embedding_chunks( + embeddings: list[list[float]], ranges: list[tuple[int, int]] +) -> list[list[float]]: + pooled: list[list[float]] = [] + for start, end in ranges: + chunks = embeddings[start:end] + if not chunks: + pooled.append([]) + continue + width = max(len(chunk) for chunk in chunks) + pooled.append( + [ + sum( + chunk[index] if index < len(chunk) else 0.0 + for chunk in chunks + ) + / len(chunks) + for index in range(width) + ] + ) + return pooled + + async def generate_embeddings( texts: list[str], openai_api_key: str, @@ -45,6 +89,8 @@ async def generate_embeddings( if not openai_api_key: raise ValueError("OPENAI_API_KEY is not set") + request_texts, input_ranges = _split_embedding_inputs(texts) + # Instantiate client locally to avoid global state race conditions across tenants configured_base_url = base_url if configured_base_url is None: @@ -65,12 +111,15 @@ async def generate_embeddings( validated_base_url or "openai-default", lambda: retry_transient( lambda: client.embeddings.create( - model=model or settings.OPENAI_EMBEDDING_MODEL, input=texts + model=model or settings.OPENAI_EMBEDDING_MODEL, + input=request_texts, ), operation_name="embedding generation", ), ) - return [data.embedding for data in response.data] + return _pool_embedding_chunks( + [data.embedding for data in response.data], input_ranges + ) except openai.OpenAIError as e: raise EmbeddingGenerationError(f"Failed to generate embeddings: {str(e)}") finally: diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index f0995d823..403f1106e 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -498,16 +498,28 @@ def test_email_read_state_revision_adds_and_records_column_ownership(monkeypatch assert calls[0][1][1].nullable is False -@pytest.mark.parametrize( - "column", - [ - {"name": "is_read", "type": sa.Boolean(), "nullable": False}, - {"name": "is_read", "type": sa.String(), "nullable": True}, - ], -) -def test_email_read_state_revision_rejects_pre_existing_column(monkeypatch, column): +def test_email_read_state_revision_preserves_compatible_pre_existing_column( + monkeypatch, +): revision = _load_email_read_state_revision() - inspector = _MigrationInspector([{"name": "id"}, column]) + inspector = _MigrationInspector( + [{"name": "id"}, {"name": "is_read", "type": sa.Boolean(), "nullable": False}] + ) + connection = _MigrationConnection() + calls = _migration_operations(monkeypatch, revision, inspector, connection) + + revision.upgrade() + + assert [name for name, _args in calls] == ["create_table"] + + +def test_email_read_state_revision_rejects_incompatible_pre_existing_column( + monkeypatch, +): + revision = _load_email_read_state_revision() + inspector = _MigrationInspector( + [{"name": "id"}, {"name": "is_read", "type": sa.String(), "nullable": True}] + ) connection = _MigrationConnection() calls = _migration_operations(monkeypatch, revision, inspector, connection) @@ -534,14 +546,14 @@ def test_email_read_state_revision_downgrade_only_removes_owned_column(monkeypat def test_email_read_state_revision_downgrade_preserves_unowned_column(monkeypatch): revision = _load_email_read_state_revision() inspector = _MigrationInspector( - [{"name": "id"}, {"name": "is_read"}], ownership_table=False + [{"name": "id"}, {"name": "is_read"}], ownership_table=True ) connection = _MigrationConnection() calls = _migration_operations(monkeypatch, revision, inspector, connection) revision.downgrade() - assert calls == [] + assert [name for name, _args in calls] == ["drop_table"] def test_email_read_state_revision_uses_canonical_email_records_table(): @@ -551,7 +563,8 @@ def test_email_read_state_revision_uses_canonical_email_records_table(): assert '_EMAIL_TABLE = "email_records"' in revision_text assert "inspector.get_columns(_EMAIL_TABLE)" in revision_text - assert "op.add_column(\n _EMAIL_TABLE" in revision_text + assert "op.add_column(" in revision_text + assert "_EMAIL_TABLE," in revision_text def test_merge_revision_reconciles_newsdom_provider_branch(): diff --git a/backend/tests/test_email_import_service.py b/backend/tests/test_email_import_service.py index 564a5fa66..38759435c 100644 --- a/backend/tests/test_email_import_service.py +++ b/backend/tests/test_email_import_service.py @@ -7,6 +7,7 @@ from sqlalchemy.ext.asyncio import AsyncSession import services.email_import_service as email_import_module +from services.circuit_breaker import CircuitOpenError from services.exceptions import EmailParseError, EmbeddingGenerationError from services.email_import_service import ( EMBEDDING_DIMENSION, @@ -561,6 +562,65 @@ async def test_generate_import_embeddings_logs_non_secret_provider_fallback(capl assert "embeddinggemma" not in caplog.text +@pytest.mark.asyncio +async def test_generate_import_embeddings_falls_back_when_provider_circuit_is_open(): + provider = EmailImportEmbeddingProvider( + api_key="secret-provider-token", + base_url="http://ollama:11434/v1", + embedding_model="embeddinggemma", + ) + + with patch( + "services.email_import_service.generate_embeddings", + new_callable=AsyncMock, + ) as mock_generate_embeddings: + mock_generate_embeddings.side_effect = CircuitOpenError( + "provider", 30 + ) + + embeddings = await _generate_import_embeddings( + ["Provider body"], + embedding_provider=provider, + ) + + assert embeddings == [[0.0] * EMBEDDING_DIMENSION] + + +@pytest.mark.asyncio +async def test_owner_quota_lock_releases_on_the_same_dedicated_connection(): + class _PostgresSessionProbe(AsyncSession): + def get_bind(self): + dialect = type("Dialect", (), {"name": "postgresql"})() + return type("Bind", (), {"dialect": dialect})() + + lock_connection = AsyncMock() + session = object.__new__(_PostgresSessionProbe) + fake_engine = type("Engine", (), {})() + fake_engine.connect = AsyncMock(return_value=lock_connection) + with patch.object( + email_import_module, + "engine", + fake_engine, + ): + lock = await email_import_module._acquire_owner_import_quota_lock( + session, + user_id="user-1", + organization_id="org-1", + ) + await email_import_module._release_owner_import_quota_lock( + session, + user_id="user-1", + organization_id="org-1", + lock=lock, + ) + + fake_engine.connect.assert_awaited_once() + assert lock is lock_connection + assert lock_connection.execute.await_count == 2 + lock_connection.commit.assert_awaited_once() + lock_connection.close.assert_awaited_once() + + @pytest.mark.asyncio async def test_generate_import_embeddings_recovers_valid_items_after_batch_failure(): provider = EmailImportEmbeddingProvider( diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index c36b65ec7..53ade42b6 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -2,6 +2,7 @@ import openai from unittest.mock import patch, AsyncMock from services.embedding import ( + EMBEDDING_INPUT_CHUNK_SIZE, STORAGE_EMBEDDING_DIMENSION, chunk_text, fit_embedding_vector, @@ -139,6 +140,36 @@ async def test_generate_embeddings_api_error(): mock_client.close.assert_awaited_once() +@pytest.mark.asyncio +async def test_generate_embeddings_pools_context_safe_chunks(): + with patch("services.embedding.AsyncOpenAI") as mock_async_openai: + mock_client = mock_async_openai.return_value + mock_client.close = AsyncMock() + + async def create_embeddings(*, model, input): + assert model == "test-model" + assert input + assert all(len(item) <= EMBEDDING_INPUT_CHUNK_SIZE for item in input) + response = AsyncMock() + response.data = [ + AsyncMock(embedding=[float(index + 1)]) + for index, _item in enumerate(input) + ] + return response + + mock_client.embeddings.create = AsyncMock(side_effect=create_embeddings) + + with patch("services.embedding.settings") as mock_settings: + mock_settings.OPENAI_EMBEDDING_MODEL = "test-model" + mock_settings.OPENAI_BASE_URL = None + mock_settings.OPENAI_EMBEDDING_BASE_URL = None + + embeddings = await generate_embeddings(["x" * 600], "test-key") + + assert embeddings == [[2.0]] + mock_client.close.assert_awaited_once() + + @pytest.mark.asyncio async def test_generate_embeddings_missing_key(): with pytest.raises(ValueError, match="OPENAI_API_KEY is not set"): diff --git a/docs/adr/0001-local-llm-and-orchestrator-boundary.md b/docs/adr/0001-local-llm-and-orchestrator-boundary.md index 2f453fb13..e41c9aa47 100644 --- a/docs/adr/0001-local-llm-and-orchestrator-boundary.md +++ b/docs/adr/0001-local-llm-and-orchestrator-boundary.md @@ -47,6 +47,12 @@ EmbeddingGemma may be separate local OpenAI-compatible endpoints. never copied onto a tenant-configured external provider or an organization DB provider, preventing an external tenant API key from being sent to a local endpoint. +8. Embedding requests split each source item into bounded, non-overlapping + chunks before calling a provider and mean-pool the returned chunks back to + one vector per source item. The current local EmbeddingGemma/llama.cpp + contract uses a conservative 256-character request ceiling because the + runtime's physical token batch limit is lower than the length of some real + mail bodies and attachments. ## Consequences @@ -56,6 +62,9 @@ EmbeddingGemma may be separate local OpenAI-compatible endpoints. MLX endpoint does not silently pretend to provide embeddings. - A local MLX chat server and local llama.cpp embedding server can run concurrently without sharing a port or crossing tenant provider boundaries. +- Real mail bodies larger than the local embedding context remain importable; + retrieval keeps one fitted vector per email/attachment instead of exposing a + provider context error to the import API. ## References diff --git a/docs/adr/0004-input-safety-and-evidence-first-judgment.md b/docs/adr/0004-input-safety-and-evidence-first-judgment.md index 1fbe959be..5084ca5ff 100644 --- a/docs/adr/0004-input-safety-and-evidence-first-judgment.md +++ b/docs/adr/0004-input-safety-and-evidence-first-judgment.md @@ -28,6 +28,15 @@ only while selecting it as the default. or to stop investigating. - Live smoke tooling must receive session secrets through the environment and must never print bearer tokens or console snippets that reproduce them. +- Import quota locks use a dedicated PostgreSQL connection for the complete + import operation. This is required because per-item commits may return an + `AsyncSession` connection to the pool; acquisition and release must therefore + occur on the same session-level advisory-lock connection, or later imports + can wait indefinitely. +- Schema migrations inspect pre-existing objects before changing them. A + compatible pre-existing column is preserved and left unowned; only objects + created by the migration are recorded and removed on downgrade. Incompatible + or ambiguous shapes fail before any schema write. ## Consequences @@ -37,6 +46,9 @@ only while selecting it as the default. semantic judgments require grounded evidence. - The test and live-smoke loop becomes part of the correction contract rather than an optional postscript. +- Real mail tests cover provider context limits, repeated same-owner imports, + zero residual advisory locks, API visibility, search visibility, and the + same-origin browser cookie proxy. ## References From a57f8a757450918776e9cd167edcd511fc9f0ec9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 13:32:59 +0900 Subject: [PATCH 05/21] fix: embed semantic mail segments before pooling --- backend/services/email_import_service.py | 206 ++++++++++++++---- backend/services/embedding.py | 8 +- backend/tests/test_batch_embedding_service.py | 38 ++++ backend/tests/test_email_import_service.py | 50 +++++ ...001-local-llm-and-orchestrator-boundary.md | 53 ++++- ...nput-safety-and-evidence-first-judgment.md | 8 + 6 files changed, 310 insertions(+), 53 deletions(-) diff --git a/backend/services/email_import_service.py b/backend/services/email_import_service.py index 277316302..67452df58 100644 --- a/backend/services/email_import_service.py +++ b/backend/services/email_import_service.py @@ -36,6 +36,7 @@ STORAGE_EMBEDDING_DIMENSION, fit_embedding_vector, generate_embeddings, + split_embedding_inputs, ) from services.exceptions import ArchiveError, EmailParseError, EmbeddingGenerationError from services.project_graph import ( @@ -97,6 +98,9 @@ class EmailImportBatchContext: organization_id: str | None +EmailContentParseResults = tuple[ParseResult, tuple[ParseResult | None, ...]] + + @dataclass class EmailImportItemResult: filename: str @@ -344,22 +348,151 @@ async def _release_owner_import_quota_lock( ) +def _parse_email_content_results( + parsed: EmailData, + *, + message_id: str, + attachment_payloads: list[dict], +) -> EmailContentParseResults: + body_parse_result = parse_content( + source_kind="email_body", + source_record_uid=_content_graph_source_record_uid("email", message_id), + content=str(parsed.get("body_parse_content") or parsed.get("body") or ""), + content_type=str(parsed.get("body_content_type") or "text/plain"), + display_name="Email body", + ) + attachment_parse_results: list[ParseResult | None] = [] + for attachment_index, attachment_payload in enumerate(attachment_payloads, start=1): + if attachment_payload.get("parse_status", "parsed") != "parsed": + attachment_parse_results.append(None) + continue + parse_source_content = str( + attachment_payload.get("parse_content") + if attachment_payload.get("parse_content") is not None + else attachment_payload.get("content") or "" + ) + if not parse_source_content.strip(): + attachment_parse_results.append(None) + continue + attachment_parse_results.append( + parse_content( + source_kind="attachment", + source_record_uid=_content_graph_source_record_uid( + "attachment", + message_id, + str(attachment_index), + str(attachment_payload.get("filename") or "attachment.txt"), + ), + content=parse_source_content, + content_type=str( + attachment_payload.get("parse_content_type") + or attachment_payload.get("content_type") + or "text/plain" + ), + display_name=str( + attachment_payload.get("filename") or "attachment.txt" + ), + ) + ) + return body_parse_result, tuple(attachment_parse_results) + + +def _semantic_embedding_inputs( + parsed: EmailData, + attachment_payloads: list[dict], + parse_results: EmailContentParseResults, +) -> tuple[list[str], list[tuple[int, int]]]: + """Use persisted graph segments as embedding units before provider safety splits. + + ADR: Semantic graph segments are the primary units; the generic embedding + boundary splitter remains only a physical context-limit fallback. + See: docs/adr/0001-local-llm-and-orchestrator-boundary.md + """ + body_parse_result, attachment_parse_results = parse_results + source_results: list[ParseResult | None] = [ + body_parse_result, + *attachment_parse_results, + ] + source_fallbacks = [ + str(parsed.get("body") or ""), + *(str(attachment.get("content") or "") for attachment in attachment_payloads), + ] + + embedding_texts: list[str] = [] + source_ranges: list[tuple[int, int]] = [] + for parse_result, fallback_text in zip(source_results, source_fallbacks): + semantic_texts = [] + if parse_result is not None: + for segment in parse_result.segments: + text = segment.safe_text_content.strip() + if not text: + continue + if segment.heading_path and segment.segment_kind != "heading": + text = f"{segment.heading_path}\n{text}" + semantic_texts.append(text) + if not semantic_texts: + semantic_texts.append(fallback_text) + start = len(embedding_texts) + embedding_texts.extend(semantic_texts) + source_ranges.append((start, len(embedding_texts))) + return embedding_texts, source_ranges + + +def _pool_source_embeddings( + embeddings: list[list[float]], + source_ranges: list[tuple[int, int]], +) -> list[list[float]]: + pooled: list[list[float]] = [] + for start, end in source_ranges: + source_embeddings = embeddings[start:end] + if not source_embeddings: + pooled.append(_zero_embedding()) + continue + width = max(len(embedding) for embedding in source_embeddings) + pooled.append( + fit_embedding_vector( + [ + sum( + embedding[index] if index < len(embedding) else 0.0 + for embedding in source_embeddings + ) + / len(source_embeddings) + for index in range(width) + ], + EMBEDDING_DIMENSION, + ) + ) + return pooled + + async def _extract_and_generate_embeddings( parsed: EmailData, embedding_provider: EmailImportEmbeddingProvider | None, batch_context: "EmailImportBatchContext | None" = None, + *, + message_id: str | None = None, + parse_results: EmailContentParseResults | None = None, ) -> tuple[list[dict], list[list[float]]]: attachment_payloads = list(parsed.get("attachments", [])) - embedding_texts = [str(parsed.get("body") or "")] - embedding_texts.extend( - str(attachment.get("content") or "") for attachment in attachment_payloads + parse_results = parse_results or _parse_email_content_results( + parsed, + message_id=str(parsed.get("message_id") or message_id or "email-import"), + attachment_payloads=attachment_payloads, + ) + embedding_texts, source_ranges = _semantic_embedding_inputs( + parsed, + attachment_payloads, + parse_results, ) - fitted_embeddings = await _generate_import_embeddings( + semantic_embeddings = await _generate_import_embeddings( embedding_texts, embedding_provider=embedding_provider, batch_context=batch_context, ) - return attachment_payloads, fitted_embeddings + return attachment_payloads, _pool_source_embeddings( + semantic_embeddings, + source_ranges, + ) def _build_email_object( @@ -373,6 +506,7 @@ def _build_email_object( persisted_date: datetime.datetime, attachment_payloads: list[dict], fitted_embeddings: list[list[float]], + parse_results: EmailContentParseResults | None = None, ) -> tuple[Email, int]: email_obj = Email( user_id=user_id, @@ -434,6 +568,7 @@ def _build_email_object( parsed=parsed, message_id=message_id, attachment_payloads=attachment_payloads, + parse_results=parse_results, ) _append_knowledge_graph_edges(email_obj) @@ -469,49 +604,26 @@ def _append_email_content_graph( parsed: EmailData, message_id: str, attachment_payloads: list[dict], + parse_results: EmailContentParseResults | None = None, ) -> None: - body_parse_result = parse_content( - source_kind="email_body", - source_record_uid=_content_graph_source_record_uid("email", message_id), - content=str(parsed.get("body_parse_content") or parsed.get("body") or ""), - content_type=str(parsed.get("body_content_type") or "text/plain"), - display_name="Email body", + parse_results = parse_results or _parse_email_content_results( + parsed, + message_id=message_id, + attachment_payloads=attachment_payloads, ) + body_parse_result, attachment_parse_results = parse_results _append_parse_result_records( email_obj=email_obj, attachment_obj=None, parse_result=body_parse_result, ) - for attachment_index, (attachment_obj, attachment_payload) in enumerate( - zip(email_obj.attachments, attachment_payloads), - start=1, + for attachment_obj, attachment_parse_result in zip( + email_obj.attachments, + attachment_parse_results, ): - if attachment_payload.get("parse_status", "parsed") != "parsed": - continue - parse_source_content = str( - attachment_payload.get("parse_content") - if attachment_payload.get("parse_content") is not None - else attachment_payload.get("content") or "" - ) - if not parse_source_content.strip(): + if attachment_parse_result is None: continue - attachment_parse_result = parse_content( - source_kind="attachment", - source_record_uid=_content_graph_source_record_uid( - "attachment", - message_id, - str(attachment_index), - attachment_obj.filename, - ), - content=parse_source_content, - content_type=str( - attachment_payload.get("parse_content_type") - or attachment_payload.get("content_type") - or "text/plain" - ), - display_name=attachment_obj.filename, - ) _append_parse_result_records( email_obj=email_obj, attachment_obj=attachment_obj, @@ -900,8 +1012,18 @@ async def _import_single_eml( organization_id=organization_id, ) + attachment_payloads = list(parsed.get("attachments", [])) + parse_results = _parse_email_content_results( + parsed, + message_id=message_id, + attachment_payloads=attachment_payloads, + ) attachment_payloads, fitted_embeddings = await _extract_and_generate_embeddings( - parsed, embedding_provider, batch_context + parsed, + embedding_provider, + batch_context, + message_id=message_id, + parse_results=parse_results, ) email_obj, attachment_count = _build_email_object( @@ -914,6 +1036,7 @@ async def _import_single_eml( persisted_date=persisted_date, attachment_payloads=attachment_payloads, fitted_embeddings=fitted_embeddings, + parse_results=parse_results, ) project_source_segments = ( @@ -969,6 +1092,7 @@ async def _generate_import_embeddings( ) -> list[list[float]]: if embedding_provider is None: return [_zero_embedding() for _ in texts] + batch_texts, batch_ranges = split_embedding_inputs(texts) if batch_context is not None and texts: # Bulk import embeddings are latency-tolerant: route them through # contextual-orchestrator first. A None result means batch is @@ -976,14 +1100,14 @@ async def _generate_import_embeddings( # existing per-request path below. batched = await try_batch_import_embeddings( batch_context.session, - texts, + batch_texts, embedding_provider=embedding_provider, user_id=batch_context.user_id, organization_id=batch_context.organization_id, dimension=EMBEDDING_DIMENSION, ) if batched is not None: - return batched + return _pool_source_embeddings(batched, batch_ranges) try: provider_embeddings = await generate_embeddings( texts, diff --git a/backend/services/embedding.py b/backend/services/embedding.py index 605d42bef..0b5f4eed7 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -40,7 +40,7 @@ def fit_embedding_vector( return embedding[:target_dimension] -def _split_embedding_inputs( +def split_embedding_inputs( texts: list[str], ) -> tuple[list[str], list[tuple[int, int]]]: flattened: list[str] = [] @@ -57,7 +57,7 @@ def _split_embedding_inputs( return flattened, ranges -def _pool_embedding_chunks( +def pool_embedding_chunks( embeddings: list[list[float]], ranges: list[tuple[int, int]] ) -> list[list[float]]: pooled: list[list[float]] = [] @@ -89,7 +89,7 @@ async def generate_embeddings( if not openai_api_key: raise ValueError("OPENAI_API_KEY is not set") - request_texts, input_ranges = _split_embedding_inputs(texts) + request_texts, input_ranges = split_embedding_inputs(texts) # Instantiate client locally to avoid global state race conditions across tenants configured_base_url = base_url @@ -117,7 +117,7 @@ async def generate_embeddings( operation_name="embedding generation", ), ) - return _pool_embedding_chunks( + return pool_embedding_chunks( [data.embedding for data in response.data], input_ranges ) except openai.OpenAIError as e: diff --git a/backend/tests/test_batch_embedding_service.py b/backend/tests/test_batch_embedding_service.py index 925df3b35..e8956ed47 100644 --- a/backend/tests/test_batch_embedding_service.py +++ b/backend/tests/test_batch_embedding_service.py @@ -34,6 +34,7 @@ EmailImportEmbeddingProvider, _generate_import_embeddings, ) +from services.embedding import EMBEDDING_INPUT_CHUNK_SIZE import services.batch_embedding_service as batch_module @@ -513,6 +514,43 @@ async def test_generate_import_embeddings_prefers_batch_context(monkeypatch): per_item.assert_not_awaited() +@pytest.mark.asyncio +async def test_generate_import_embeddings_bounds_long_semantic_units_for_batch( + monkeypatch, +): + context = EmailImportBatchContext( + session=FakeAsyncSession(), user_id="user-1", organization_id="org-acme" + ) + routed = AsyncMock( + side_effect=lambda _session, texts, **_kwargs: [ + [float(index)] * 1536 for index, _text in enumerate(texts) + ] + ) + monkeypatch.setattr( + "services.email_import_service.try_batch_import_embeddings", routed + ) + per_item = AsyncMock() + monkeypatch.setattr("services.email_import_service.generate_embeddings", per_item) + + result = await _generate_import_embeddings( + ["semantic sentence. " * 100], + embedding_provider=PROVIDER, + batch_context=context, + ) + + submitted_texts = routed.await_args.args[1] + assert len(submitted_texts) > 1 + assert all(len(text) <= EMBEDDING_INPUT_CHUNK_SIZE for text in submitted_texts) + assert result == [ + [ + sum(float(index) for index in range(len(submitted_texts))) + / len(submitted_texts) + ] + * 1536 + ] + per_item.assert_not_awaited() + + @pytest.mark.asyncio async def test_generate_import_embeddings_falls_back_when_batch_returns_none( monkeypatch, diff --git a/backend/tests/test_email_import_service.py b/backend/tests/test_email_import_service.py index 38759435c..0dc1bd23f 100644 --- a/backend/tests/test_email_import_service.py +++ b/backend/tests/test_email_import_service.py @@ -562,6 +562,56 @@ async def test_generate_import_embeddings_logs_non_secret_provider_fallback(capl assert "embeddinggemma" not in caplog.text +@pytest.mark.asyncio +async def test_extract_embeddings_uses_graph_segments_before_source_pooling(): + parsed = { + "message_id": "", + "body": "Launch\n\nHello team", + "body_parse_content": "

Launch

Hello team

", + "body_content_type": "text/html", + "attachments": [ + { + "filename": "plan.md", + "content": "# Plan\n\nShip graph", + "content_type": "text/markdown", + } + ], + } + provider = EmailImportEmbeddingProvider( + api_key="provider-token", + base_url="http://ollama:11434/v1", + embedding_model="embeddinggemma", + ) + + with patch( + "services.email_import_service.generate_embeddings", + new_callable=AsyncMock, + return_value=[ + [1.0] * EMBEDDING_DIMENSION, + [2.0] * EMBEDDING_DIMENSION, + [3.0] * EMBEDDING_DIMENSION, + [4.0] * EMBEDDING_DIMENSION, + ], + ) as mock_generate_embeddings: + attachment_payloads, source_embeddings = ( + await email_import_module._extract_and_generate_embeddings( + parsed, + provider, + ) + ) + + assert attachment_payloads == parsed["attachments"] + assert mock_generate_embeddings.await_args.args[0] == [ + "Launch", + "Launch\nHello team", + "Plan", + "Plan\nShip graph", + ] + assert source_embeddings[0][0] == 1.5 + assert source_embeddings[1][0] == 3.5 + assert all(len(embedding) == EMBEDDING_DIMENSION for embedding in source_embeddings) + + @pytest.mark.asyncio async def test_generate_import_embeddings_falls_back_when_provider_circuit_is_open(): provider = EmailImportEmbeddingProvider( diff --git a/docs/adr/0001-local-llm-and-orchestrator-boundary.md b/docs/adr/0001-local-llm-and-orchestrator-boundary.md index e41c9aa47..d6d2a057a 100644 --- a/docs/adr/0001-local-llm-and-orchestrator-boundary.md +++ b/docs/adr/0001-local-llm-and-orchestrator-boundary.md @@ -47,12 +47,48 @@ EmbeddingGemma may be separate local OpenAI-compatible endpoints. never copied onto a tenant-configured external provider or an organization DB provider, preventing an external tenant API key from being sent to a local endpoint. -8. Embedding requests split each source item into bounded, non-overlapping - chunks before calling a provider and mean-pool the returned chunks back to - one vector per source item. The current local EmbeddingGemma/llama.cpp - contract uses a conservative 256-character request ceiling because the - runtime's physical token batch limit is lower than the length of some real - mail bodies and attachments. +8. Import embedding inputs are the existing content-graph parser's semantic + segments: headings, paragraphs, sections, and structured fields. A segment's + `heading_path` is prefixed to non-heading text so the embedding retains its + ontology context without merging unrelated segments. +9. The physical provider limit is a safety boundary, not the semantic chunking + policy. Only an oversized semantic segment is further split by the shared + boundary-aware embedding splitter, with no overlap, before it reaches a + provider. The current local EmbeddingGemma/llama.cpp contract uses a + conservative 256-character request ceiling because the runtime's physical + token batch limit is lower than the length of some real mail bodies and + attachments. +10. The existing `Email.embedding` and `Attachment.embedding` columns remain + source-level compatibility vectors. Import mean-pools segment embeddings + into one centroid per email or attachment, while the persisted content + segments remain the authoritative Ontology/Project Graph citation units. + Segment-level dense retrieval is explicitly out of scope until it has a + separate schema and retrieval decision. + +### Non-goals + +- Do not use a fixed character window as the primary semantic unit. +- Do not add a segment-vector column or change dense-search result identity in + this incident fix. + +### Implementation plan + +- Reuse `services.content_graph.parse_content` in + `services.email_import_service` to build the embedding inputs and graph + records from the same `ParseResult`. +- Keep `services.embedding.generate_embeddings` as the provider safety boundary + for oversized individual semantic segments and retain the existing + contextual-orchestrator batch seam. +- Mean-pool segment vectors back to the existing source-level vector columns. + +### Verification + +- [x] Regression test proves heading context is preserved and semantic + segments are sent separately before source pooling. +- [x] Regression test proves oversized provider inputs remain below the local + physical request ceiling. +- [ ] Live mail import proves source visibility, search visibility, and zero + residual advisory locks after the semantic-input change. ## Consequences @@ -63,8 +99,9 @@ EmbeddingGemma may be separate local OpenAI-compatible endpoints. - A local MLX chat server and local llama.cpp embedding server can run concurrently without sharing a port or crossing tenant provider boundaries. - Real mail bodies larger than the local embedding context remain importable; - retrieval keeps one fitted vector per email/attachment instead of exposing a - provider context error to the import API. + retrieval keeps one fitted source centroid per email/attachment instead of + exposing a provider context error to the import API, while Ontology and + Project Graph processing continues to see the original cited segments. ## References diff --git a/docs/adr/0004-input-safety-and-evidence-first-judgment.md b/docs/adr/0004-input-safety-and-evidence-first-judgment.md index 5084ca5ff..48c90eae9 100644 --- a/docs/adr/0004-input-safety-and-evidence-first-judgment.md +++ b/docs/adr/0004-input-safety-and-evidence-first-judgment.md @@ -37,6 +37,11 @@ only while selecting it as the default. compatible pre-existing column is preserved and left unowned; only objects created by the migration are recorded and removed on downgrade. Incompatible or ambiguous shapes fail before any schema write. +- Semantic content segments are the primary import embedding units. A physical + 256-character ceiling is applied only to an oversized segment as a provider + safety fallback; source-level vectors are centroids over the segment vectors, + while persisted segment provenance remains authoritative for Ontology and + Project Graph judgments. ## Consequences @@ -46,6 +51,9 @@ only while selecting it as the default. semantic judgments require grounded evidence. - The test and live-smoke loop becomes part of the correction contract rather than an optional postscript. +- Long mail no longer forces the semantic layer to choose arbitrary character + windows: the parser's heading/paragraph/structured-field boundaries are + preserved before any physical-limit fallback. - Real mail tests cover provider context limits, repeated same-owner imports, zero residual advisory locks, API visibility, search visibility, and the same-origin browser cookie proxy. From 44563f82f0e144f0bfeebd6ede15871c2cfec381 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 13:59:17 +0900 Subject: [PATCH 06/21] fix: preserve migration history and enforce token-safe embedding chunks --- .../alembic/versions/0011_email_read_state.py | 103 ++------------- .../0018_email_read_state_ownership.py | 124 ++++++++++++++++++ .../0019_merge_email_read_state_ownership.py | 21 +++ backend/services/email_import_service.py | 26 +++- backend/services/embedding.py | 76 ++++++++--- backend/tests/test_alembic_migrations.py | 50 ++++++- backend/tests/test_embedding.py | 43 +++++- scripts/naruon_compose.sh | 9 +- 8 files changed, 324 insertions(+), 128 deletions(-) create mode 100644 backend/alembic/versions/0018_email_read_state_ownership.py create mode 100644 backend/alembic/versions/0019_merge_email_read_state_ownership.py mode change 100755 => 100644 scripts/naruon_compose.sh diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index 2f4ecfc78..716590cd1 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -11,106 +11,19 @@ down_revision = "0009_project_graph_projection" branch_labels = None depends_on = None -_EMAIL_TABLE = "email_records" -_READ_STATE_COLUMN = "is_read" -_OWNERSHIP_TABLE = "email_read_state_ownership" -_OWNERSHIP_KEY_COLUMN = "ownership_key" -_OWNERSHIP_KEY = "0011_email_read_state:email_records:is_read" - - -def _existing_read_state_column(inspector) -> dict | None: - return next( - ( - column - for column in inspector.get_columns(_EMAIL_TABLE) - if column["name"] == _READ_STATE_COLUMN - ), - None, - ) - - -def _validate_pre_existing_read_state(column: dict) -> None: - column_type = column.get("type") - canonical_shape = ( - isinstance(column_type, sa.Boolean) and column.get("nullable") is False - ) - if not canonical_shape: - raise RuntimeError( - "0011_email_read_state cannot safely use an incompatible " - f"pre-existing {_EMAIL_TABLE}.{_READ_STATE_COLUMN} column; " - "reconcile the schema before applying this migration" - ) - - -def _ownership_record_exists(connection) -> bool: - ownership_table = sa.table( - _OWNERSHIP_TABLE, - sa.column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120)), - ) - return ( - connection.execute( - sa.select(ownership_table.c[_OWNERSHIP_KEY_COLUMN]) - .where( - ownership_table.c[_OWNERSHIP_KEY_COLUMN] == _OWNERSHIP_KEY, - ) - .limit(1) - ).first() - is not None - ) def upgrade() -> None: - connection = op.get_bind() - inspector = sa.inspect(connection) - if not inspector.has_table(_EMAIL_TABLE): - raise RuntimeError(f"required table {_EMAIL_TABLE} is missing") - if inspector.has_table(_OWNERSHIP_TABLE): - raise RuntimeError(f"unexpected pre-existing table {_OWNERSHIP_TABLE}") - - existing_column = _existing_read_state_column(inspector) - owns_read_state_column = existing_column is None - if owns_read_state_column: - op.add_column( - _EMAIL_TABLE, - sa.Column( - _READ_STATE_COLUMN, - sa.Boolean(), - nullable=False, - server_default=sa.text("true"), - ), - ) - else: - _validate_pre_existing_read_state(existing_column) - - op.create_table( - _OWNERSHIP_TABLE, - sa.Column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120), nullable=False), - sa.PrimaryKeyConstraint( - _OWNERSHIP_KEY_COLUMN, - name="pk_email_read_state_ownership", + op.add_column( + "emails", + sa.Column( + "is_read", + sa.Boolean(), + nullable=False, + server_default=sa.text("true"), ), ) - ownership_table = sa.table( - _OWNERSHIP_TABLE, - sa.column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120)), - ) - if owns_read_state_column: - op.bulk_insert( - ownership_table, - [{_OWNERSHIP_KEY_COLUMN: _OWNERSHIP_KEY}], - ) def downgrade() -> None: - connection = op.get_bind() - inspector = sa.inspect(connection) - if not inspector.has_table(_OWNERSHIP_TABLE): - return - owns_read_state_column = _ownership_record_exists(connection) - if ( - owns_read_state_column - and inspector.has_table(_EMAIL_TABLE) - and _existing_read_state_column(inspector) - ): - op.drop_column(_EMAIL_TABLE, _READ_STATE_COLUMN) - op.drop_table(_OWNERSHIP_TABLE) + op.drop_column("emails", "is_read") diff --git a/backend/alembic/versions/0018_email_read_state_ownership.py b/backend/alembic/versions/0018_email_read_state_ownership.py new file mode 100644 index 000000000..f528f3bb4 --- /dev/null +++ b/backend/alembic/versions/0018_email_read_state_ownership.py @@ -0,0 +1,124 @@ +"""Apply owned canonical email read state after the published revision. + +Revision ID: 0018_email_read_state_ownership +Revises: 0011_email_read_state +Create Date: 2026-08-12 00:00:00.000000 + +The published 0011 revision is immutable. This follow-up owns only the +canonical email_records.is_read column and its ownership marker so its +downgrade can remove only objects it created. +""" + +from alembic import op +import sqlalchemy as sa + +revision = "0018_email_read_state_ownership" +down_revision = "0011_email_read_state" +branch_labels = None +depends_on = None + +_EMAIL_TABLE = "email_records" +_READ_STATE_COLUMN = "is_read" +_OWNERSHIP_TABLE = "email_read_state_ownership" +_OWNERSHIP_KEY_COLUMN = "ownership_key" +_OWNERSHIP_KEY = "0018_email_read_state_ownership:email_records:is_read" + + +def _existing_read_state_column(inspector) -> dict | None: + return next( + ( + column + for column in inspector.get_columns(_EMAIL_TABLE) + if column["name"] == _READ_STATE_COLUMN + ), + None, + ) + + +def _validate_pre_existing_read_state(column: dict) -> None: + column_type = column.get("type") + canonical_shape = ( + isinstance(column_type, sa.Boolean) and column.get("nullable") is False + ) + if not canonical_shape: + raise RuntimeError( + "0018_email_read_state_ownership cannot safely use an incompatible " + f"pre-existing {_EMAIL_TABLE}.{_READ_STATE_COLUMN} column; " + "reconcile the schema before applying this migration" + ) + + +def _ownership_record_exists(connection) -> bool: + ownership_table = sa.table( + _OWNERSHIP_TABLE, + sa.column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120)), + ) + return ( + connection.execute( + sa.select(ownership_table.c[_OWNERSHIP_KEY_COLUMN]) + .where( + ownership_table.c[_OWNERSHIP_KEY_COLUMN] == _OWNERSHIP_KEY, + ) + .limit(1) + ).first() + is not None + ) + + +def upgrade() -> None: + """Add and record the canonical read-state objects exactly once.""" + connection = op.get_bind() + inspector = sa.inspect(connection) + if not inspector.has_table(_EMAIL_TABLE): + raise RuntimeError(f"required table {_EMAIL_TABLE} is missing") + if inspector.has_table(_OWNERSHIP_TABLE): + raise RuntimeError(f"unexpected pre-existing table {_OWNERSHIP_TABLE}") + + existing_column = _existing_read_state_column(inspector) + owns_read_state_column = existing_column is None + if owns_read_state_column: + op.add_column( + _EMAIL_TABLE, + sa.Column( + _READ_STATE_COLUMN, + sa.Boolean(), + nullable=False, + server_default=sa.text("true"), + ), + ) + else: + _validate_pre_existing_read_state(existing_column) + + op.create_table( + _OWNERSHIP_TABLE, + sa.Column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120), nullable=False), + sa.PrimaryKeyConstraint( + _OWNERSHIP_KEY_COLUMN, + name="pk_email_read_state_ownership", + ), + ) + ownership_table = sa.table( + _OWNERSHIP_TABLE, + sa.column(_OWNERSHIP_KEY_COLUMN, sa.String(length=120)), + ) + if owns_read_state_column: + op.bulk_insert( + ownership_table, + [{_OWNERSHIP_KEY_COLUMN: _OWNERSHIP_KEY}], + ) + + +def downgrade() -> None: + """Remove only canonical read-state objects owned by this revision.""" + connection = op.get_bind() + inspector = sa.inspect(connection) + if not inspector.has_table(_OWNERSHIP_TABLE): + return + owns_read_state_column = _ownership_record_exists(connection) + if ( + owns_read_state_column + and inspector.has_table(_EMAIL_TABLE) + and _existing_read_state_column(inspector) + ): + op.drop_column(_EMAIL_TABLE, _READ_STATE_COLUMN) + op.drop_table(_OWNERSHIP_TABLE) diff --git a/backend/alembic/versions/0019_merge_email_read_state_ownership.py b/backend/alembic/versions/0019_merge_email_read_state_ownership.py new file mode 100644 index 000000000..9b4650edb --- /dev/null +++ b/backend/alembic/versions/0019_merge_email_read_state_ownership.py @@ -0,0 +1,21 @@ +"""Merge the immutable read-state follow-up into the migration graph. + +Revision ID: 0019_merge_email_read_state_ownership +Revises: 0017_merge_newsdom_carddav_heads, 0018_email_read_state_ownership +Create Date: 2026-08-12 00:00:00.000000 +""" + +from __future__ import annotations + +revision = "0019_merge_email_read_state_ownership" +down_revision = ("0017_merge_newsdom_carddav_heads", "0018_email_read_state_ownership") +branch_labels = None +depends_on = None + + +def upgrade() -> None: + """Unify the existing application branches without additional DDL.""" + + +def downgrade() -> None: + """Keep the parent branch schemas intact when removing the merge node.""" diff --git a/backend/services/email_import_service.py b/backend/services/email_import_service.py index 67452df58..9cb79517b 100644 --- a/backend/services/email_import_service.py +++ b/backend/services/email_import_service.py @@ -441,6 +441,7 @@ def _semantic_embedding_inputs( def _pool_source_embeddings( embeddings: list[list[float]], source_ranges: list[tuple[int, int]], + token_weights: list[int] | None = None, ) -> list[list[float]]: pooled: list[list[float]] = [] for start, end in source_ranges: @@ -448,15 +449,24 @@ def _pool_source_embeddings( if not source_embeddings: pooled.append(_zero_embedding()) continue + weights = ( + token_weights[start:end] + if token_weights is not None + else [1] * len(source_embeddings) + ) + total_weight = sum(weights) or len(source_embeddings) width = max(len(embedding) for embedding in source_embeddings) pooled.append( fit_embedding_vector( [ sum( - embedding[index] if index < len(embedding) else 0.0 - for embedding in source_embeddings + ( + embedding[index] if index < len(embedding) else 0.0 + ) + * weights[embedding_index] + for embedding_index, embedding in enumerate(source_embeddings) ) - / len(source_embeddings) + / total_weight for index in range(width) ], EMBEDDING_DIMENSION, @@ -1092,7 +1102,9 @@ async def _generate_import_embeddings( ) -> list[list[float]]: if embedding_provider is None: return [_zero_embedding() for _ in texts] - batch_texts, batch_ranges = split_embedding_inputs(texts) + batch_texts, batch_ranges, batch_token_weights = split_embedding_inputs( + texts, embedding_provider.embedding_model + ) if batch_context is not None and texts: # Bulk import embeddings are latency-tolerant: route them through # contextual-orchestrator first. A None result means batch is @@ -1107,7 +1119,11 @@ async def _generate_import_embeddings( dimension=EMBEDDING_DIMENSION, ) if batched is not None: - return _pool_source_embeddings(batched, batch_ranges) + return _pool_source_embeddings( + batched, + batch_ranges, + batch_token_weights, + ) try: provider_embeddings = await generate_embeddings( texts, diff --git a/backend/services/embedding.py b/backend/services/embedding.py index 0b5f4eed7..d32dbe963 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -1,4 +1,5 @@ import openai +import tiktoken from langchain_text_splitters import RecursiveCharacterTextSplitter from openai import AsyncOpenAI from core.config import settings @@ -8,10 +9,10 @@ from services.retry import retry_transient STORAGE_EMBEDDING_DIMENSION = 1536 -# Keep each request below the smallest local embedding context observed in the -# supported llama.cpp/EmbeddingGemma runtime. Longer source items are pooled -# back to one vector per caller input. -EMBEDDING_INPUT_CHUNK_SIZE = 256 +# Provider-safe token ceiling for the smallest supported local embedding runtime. +EMBEDDING_INPUT_TOKEN_LIMIT = 256 +# Retain the old name for importers that only need the conservative ceiling. +EMBEDDING_INPUT_CHUNK_SIZE = EMBEDDING_INPUT_TOKEN_LIMIT def chunk_text( @@ -40,46 +41,80 @@ def fit_embedding_vector( return embedding[:target_dimension] +def _embedding_encoding(model: str | None): + """Return the selected model tokenizer, with a deterministic local fallback.""" + selected_model = model or settings.OPENAI_EMBEDDING_MODEL + try: + return tiktoken.encoding_for_model(selected_model) + except (KeyError, ValueError): + return tiktoken.get_encoding("cl100k_base") + + def split_embedding_inputs( texts: list[str], -) -> tuple[list[str], list[tuple[int, int]]]: + model: str | None = None, +) -> tuple[list[str], list[tuple[int, int]], list[int]]: + """Split inputs by tokenizer tokens and return token weights per chunk.""" + encoding = _embedding_encoding(model) flattened: list[str] = [] ranges: list[tuple[int, int]] = [] + token_weights: list[int] = [] for text in texts: - chunks = chunk_text( - text, - chunk_size=EMBEDDING_INPUT_CHUNK_SIZE, - chunk_overlap=0, - ) or [text] + tokens = encoding.encode(text, disallowed_special=()) + if not tokens: + chunks = [text] + weights = [0] + else: + chunks = [] + weights = [] + for start in range(0, len(tokens), EMBEDDING_INPUT_TOKEN_LIMIT): + token_slice = tokens[start : start + EMBEDDING_INPUT_TOKEN_LIMIT] + chunks.append(encoding.decode(token_slice)) + weights.append(len(token_slice)) start = len(flattened) flattened.extend(chunks) + token_weights.extend(weights) ranges.append((start, len(flattened))) - return flattened, ranges + return flattened, ranges, token_weights + def pool_embedding_chunks( - embeddings: list[list[float]], ranges: list[tuple[int, int]] + embeddings: list[list[float]], + ranges: list[tuple[int, int]], + token_weights: list[int] | None = None, ) -> list[list[float]]: + """Pool chunk vectors, weighting each chunk by its tokenizer token count.""" pooled: list[list[float]] = [] for start, end in ranges: chunks = embeddings[start:end] if not chunks: pooled.append([]) continue + weights = ( + token_weights[start:end] + if token_weights is not None + else [1] * len(chunks) + ) + total_weight = sum(weights) or len(chunks) width = max(len(chunk) for chunk in chunks) pooled.append( [ sum( - chunk[index] if index < len(chunk) else 0.0 - for chunk in chunks + ( + chunk[index] if index < len(chunk) else 0.0 + ) + * weights[chunk_index] + for chunk_index, chunk in enumerate(chunks) ) - / len(chunks) + / total_weight for index in range(width) ] ) return pooled + async def generate_embeddings( texts: list[str], openai_api_key: str, @@ -89,7 +124,10 @@ async def generate_embeddings( if not openai_api_key: raise ValueError("OPENAI_API_KEY is not set") - request_texts, input_ranges = split_embedding_inputs(texts) + selected_model = model or settings.OPENAI_EMBEDDING_MODEL + request_texts, input_ranges, token_weights = split_embedding_inputs( + texts, selected_model + ) # Instantiate client locally to avoid global state race conditions across tenants configured_base_url = base_url @@ -111,14 +149,16 @@ async def generate_embeddings( validated_base_url or "openai-default", lambda: retry_transient( lambda: client.embeddings.create( - model=model or settings.OPENAI_EMBEDDING_MODEL, + model=selected_model, input=request_texts, ), operation_name="embedding generation", ), ) return pool_embedding_chunks( - [data.embedding for data in response.data], input_ranges + [data.embedding for data in response.data], + input_ranges, + token_weights, ) except openai.OpenAIError as e: raise EmbeddingGenerationError(f"Failed to generate embeddings: {str(e)}") diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index 403f1106e..c16190406 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -407,6 +407,7 @@ def test_alembic_migration_graph_has_a_single_head(): "revision (down_revision tuple of the heads) so `alembic upgrade head` " "is unambiguous" ) + assert "0019_merge_email_read_state_ownership" in heads def test_merge_revision_reconciles_email_read_state_branch(): @@ -428,7 +429,9 @@ def test_merge_revision_reconciles_email_read_state_branch(): def _load_email_read_state_revision(): - revision_path = BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" + revision_path = ( + BACKEND_ROOT / "alembic" / "versions" / "0018_email_read_state_ownership.py" + ) spec = importlib.util.spec_from_file_location( "email_read_state_revision", revision_path ) @@ -558,7 +561,7 @@ def test_email_read_state_revision_downgrade_preserves_unowned_column(monkeypatc def test_email_read_state_revision_uses_canonical_email_records_table(): revision_text = ( - BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" + BACKEND_ROOT / "alembic" / "versions" / "0018_email_read_state_ownership.py" ).read_text() assert '_EMAIL_TABLE = "email_records"' in revision_text @@ -567,6 +570,49 @@ def test_email_read_state_revision_uses_canonical_email_records_table(): assert "_EMAIL_TABLE," in revision_text +def test_published_read_state_revision_is_immutable(): + revision_text = ( + BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" + ).read_text() + + assert 'revision = "0011_email_read_state"' in revision_text + assert 'down_revision = "0009_project_graph_projection"' in revision_text + assert 'op.add_column(\n "emails"' in revision_text + assert "email_read_state_ownership" not in revision_text + + +def test_read_state_follow_up_revision_is_after_published_revision(): + revision_path = ( + BACKEND_ROOT + / "alembic" + / "versions" + / "0018_email_read_state_ownership.py" + ) + revision_text = revision_path.read_text() + + assert revision_path.exists() + assert 'revision = "0018_email_read_state_ownership"' in revision_text + assert 'down_revision = "0011_email_read_state"' in revision_text + assert "_OWNERSHIP_KEY" in revision_text + + +def test_read_state_follow_up_merge_reconciles_current_graph(): + revision_path = ( + BACKEND_ROOT + / "alembic" + / "versions" + / "0019_merge_email_read_state_ownership.py" + ) + revision_text = revision_path.read_text() + + assert revision_path.exists() + assert 'revision = "0019_merge_email_read_state_ownership"' in revision_text + assert "down_revision = (" in revision_text + assert '"0017_merge_newsdom_carddav_heads"' in revision_text + assert '"0018_email_read_state_ownership"' in revision_text + assert "op.create_table(" not in revision_text + + def test_merge_revision_reconciles_newsdom_provider_branch(): revision_path = ( BACKEND_ROOT / "alembic" / "versions" / "0015_merge_newsdom_email_heads.py" diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index 53ade42b6..47870166b 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -1,12 +1,15 @@ import pytest +import tiktoken import openai from unittest.mock import patch, AsyncMock from services.embedding import ( - EMBEDDING_INPUT_CHUNK_SIZE, + EMBEDDING_INPUT_TOKEN_LIMIT, STORAGE_EMBEDDING_DIMENSION, chunk_text, fit_embedding_vector, generate_embeddings, + pool_embedding_chunks, + split_embedding_inputs, ) from services.exceptions import EmbeddingGenerationError @@ -33,6 +36,30 @@ def test_fit_embedding_vector_truncates_larger_provider_dimension(): assert fitted == [0.5] * STORAGE_EMBEDDING_DIMENSION +def test_split_embedding_inputs_respects_token_limit_and_returns_weights(): + flattened, ranges, weights = split_embedding_inputs( + ["token " * 600], + model="test-model", + ) + + encoding = tiktoken.get_encoding("cl100k_base") + assert ranges == [(0, len(flattened))] + assert len(flattened) == len(weights) + assert all( + 0 < weight <= EMBEDDING_INPUT_TOKEN_LIMIT + and len(encoding.encode(text, disallowed_special=())) == weight + for text, weight in zip(flattened, weights) + ) + + +def test_pool_embedding_chunks_weights_by_token_count(): + assert pool_embedding_chunks( + [[1.0], [3.0]], + [(0, 2)], + [1, 3], + ) == [[2.5]] + + @pytest.mark.asyncio async def test_generate_embeddings_success(): with patch( @@ -141,7 +168,7 @@ async def test_generate_embeddings_api_error(): @pytest.mark.asyncio -async def test_generate_embeddings_pools_context_safe_chunks(): +async def test_generate_embeddings_pools_token_safe_chunks(): with patch("services.embedding.AsyncOpenAI") as mock_async_openai: mock_client = mock_async_openai.return_value mock_client.close = AsyncMock() @@ -149,7 +176,12 @@ async def test_generate_embeddings_pools_context_safe_chunks(): async def create_embeddings(*, model, input): assert model == "test-model" assert input - assert all(len(item) <= EMBEDDING_INPUT_CHUNK_SIZE for item in input) + encoding = tiktoken.get_encoding("cl100k_base") + assert all( + 0 < len(encoding.encode(item, disallowed_special=())) + <= EMBEDDING_INPUT_TOKEN_LIMIT + for item in input + ) response = AsyncMock() response.data = [ AsyncMock(embedding=[float(index + 1)]) @@ -164,9 +196,10 @@ async def create_embeddings(*, model, input): mock_settings.OPENAI_BASE_URL = None mock_settings.OPENAI_EMBEDDING_BASE_URL = None - embeddings = await generate_embeddings(["x" * 600], "test-key") + embeddings = await generate_embeddings(["token " * 600], "test-key") - assert embeddings == [[2.0]] + assert len(embeddings) == 1 + assert embeddings[0][0] >= 1.0 mock_client.close.assert_awaited_once() diff --git a/scripts/naruon_compose.sh b/scripts/naruon_compose.sh old mode 100755 new mode 100644 index 7c5076a81..55b41ff0b --- a/scripts/naruon_compose.sh +++ b/scripts/naruon_compose.sh @@ -60,7 +60,8 @@ case "${llm_runtime}" in mlx|llama.cpp) if [ "${llm_runtime}" = "mlx" ]; then host_llm_base_url="${NARUON_MLX_BASE_URL:-http://host.docker.internal:8080/v1}" - export NARUON_HOST_LLM_BASE_URL="$(container_host_url "${host_llm_base_url}")" + NARUON_HOST_LLM_BASE_URL="$(container_host_url "${host_llm_base_url}")" || exit 1 + export NARUON_HOST_LLM_BASE_URL export NARUON_HOST_LLM_ALLOWED_HOSTS="${NARUON_MLX_ALLOWED_LLM_BASE_URL_HOSTS:-host.docker.internal}" # mlx-lm serves chat completions but not /v1/embeddings. A local # placeholder key enables chat while embedding paths retain the @@ -70,7 +71,8 @@ case "${llm_runtime}" in export NARUON_HOST_LLM_MODEL="${NARUON_MLX_LLM_MODEL:-mlx-community/gemma-4-e4b-it-4bit}" else host_llm_base_url="${NARUON_LLAMA_CPP_BASE_URL:-http://host.docker.internal:8081/v1}" - export NARUON_HOST_LLM_BASE_URL="$(container_host_url "${host_llm_base_url}")" + NARUON_HOST_LLM_BASE_URL="$(container_host_url "${host_llm_base_url}")" || exit 1 + export NARUON_HOST_LLM_BASE_URL export NARUON_HOST_LLM_ALLOWED_HOSTS="${NARUON_LLAMA_CPP_ALLOWED_LLM_BASE_URL_HOSTS:-host.docker.internal}" export NARUON_HOST_LLM_API_KEY="${NARUON_LLAMA_CPP_API_KEY:-llama.cpp}" export NARUON_HOST_LLM_EMBEDDING_MODEL="${NARUON_LLAMA_CPP_EMBEDDING_MODEL:-embeddinggemma}" @@ -91,7 +93,8 @@ case "${llm_runtime}" in fi fi if [ -n "${embedding_base_url}" ]; then - export NARUON_HOST_LLM_EMBEDDING_BASE_URL="$(container_host_url "${embedding_base_url}")" + NARUON_HOST_LLM_EMBEDDING_BASE_URL="$(container_host_url "${embedding_base_url}")" || exit 1 + export NARUON_HOST_LLM_EMBEDDING_BASE_URL fi compose_files=( --file "${repo_root}/docker-compose.yml" From 0b5f4478e7084c7856caafbebf65757f83f12839 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 14:05:39 +0900 Subject: [PATCH 07/21] fix: keep migration identifiers within Alembic limits --- .../versions/0019_merge_email_read_state_ownership.py | 4 ++-- backend/tests/test_alembic_migrations.py | 6 +++--- backend/tests/test_batch_embedding_service.py | 11 +++++++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/backend/alembic/versions/0019_merge_email_read_state_ownership.py b/backend/alembic/versions/0019_merge_email_read_state_ownership.py index 9b4650edb..6b4b4e33b 100644 --- a/backend/alembic/versions/0019_merge_email_read_state_ownership.py +++ b/backend/alembic/versions/0019_merge_email_read_state_ownership.py @@ -1,13 +1,13 @@ """Merge the immutable read-state follow-up into the migration graph. -Revision ID: 0019_merge_email_read_state_ownership +Revision ID: 0019_merge_read_state_ownership Revises: 0017_merge_newsdom_carddav_heads, 0018_email_read_state_ownership Create Date: 2026-08-12 00:00:00.000000 """ from __future__ import annotations -revision = "0019_merge_email_read_state_ownership" +revision = "0019_merge_read_state_ownership" down_revision = ("0017_merge_newsdom_carddav_heads", "0018_email_read_state_ownership") branch_labels = None depends_on = None diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index c16190406..fce6b0535 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -407,7 +407,7 @@ def test_alembic_migration_graph_has_a_single_head(): "revision (down_revision tuple of the heads) so `alembic upgrade head` " "is unambiguous" ) - assert "0019_merge_email_read_state_ownership" in heads + assert "0019_merge_read_state_ownership" in heads def test_merge_revision_reconciles_email_read_state_branch(): @@ -601,12 +601,12 @@ def test_read_state_follow_up_merge_reconciles_current_graph(): BACKEND_ROOT / "alembic" / "versions" - / "0019_merge_email_read_state_ownership.py" + / "0019_merge_read_state_ownership.py" ) revision_text = revision_path.read_text() assert revision_path.exists() - assert 'revision = "0019_merge_email_read_state_ownership"' in revision_text + assert 'revision = "0019_merge_read_state_ownership"' in revision_text assert "down_revision = (" in revision_text assert '"0017_merge_newsdom_carddav_heads"' in revision_text assert '"0018_email_read_state_ownership"' in revision_text diff --git a/backend/tests/test_batch_embedding_service.py b/backend/tests/test_batch_embedding_service.py index e8956ed47..490b12875 100644 --- a/backend/tests/test_batch_embedding_service.py +++ b/backend/tests/test_batch_embedding_service.py @@ -34,7 +34,9 @@ EmailImportEmbeddingProvider, _generate_import_embeddings, ) -from services.embedding import EMBEDDING_INPUT_CHUNK_SIZE +import tiktoken + +from services.embedding import EMBEDDING_INPUT_TOKEN_LIMIT import services.batch_embedding_service as batch_module @@ -540,7 +542,12 @@ async def test_generate_import_embeddings_bounds_long_semantic_units_for_batch( submitted_texts = routed.await_args.args[1] assert len(submitted_texts) > 1 - assert all(len(text) <= EMBEDDING_INPUT_CHUNK_SIZE for text in submitted_texts) + encoding = tiktoken.get_encoding("cl100k_base") + assert all( + 0 < len(encoding.encode(text, disallowed_special=())) + <= EMBEDDING_INPUT_TOKEN_LIMIT + for text in submitted_texts + ) assert result == [ [ sum(float(index) for index in range(len(submitted_texts))) From a6f36eb35f9cedbc9125f249375227be4aa0ef66 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 14:15:06 +0900 Subject: [PATCH 08/21] fix: align token-weighted regression expectations --- backend/tests/test_alembic_migrations.py | 2 +- backend/tests/test_batch_embedding_service.py | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index fce6b0535..fecd4697d 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -601,7 +601,7 @@ def test_read_state_follow_up_merge_reconciles_current_graph(): BACKEND_ROOT / "alembic" / "versions" - / "0019_merge_read_state_ownership.py" + / "0019_merge_email_read_state_ownership.py" ) revision_text = revision_path.read_text() diff --git a/backend/tests/test_batch_embedding_service.py b/backend/tests/test_batch_embedding_service.py index 490b12875..0bd096b05 100644 --- a/backend/tests/test_batch_embedding_service.py +++ b/backend/tests/test_batch_embedding_service.py @@ -548,13 +548,15 @@ async def test_generate_import_embeddings_bounds_long_semantic_units_for_batch( <= EMBEDDING_INPUT_TOKEN_LIMIT for text in submitted_texts ) - assert result == [ - [ - sum(float(index) for index in range(len(submitted_texts))) - / len(submitted_texts) - ] - * 1536 + token_weights = [ + len(encoding.encode(text, disallowed_special=())) + for text in submitted_texts ] + expected_value = sum( + float(index) * weight + for index, weight in enumerate(token_weights) + ) / sum(token_weights) + assert result == [[expected_value] * 1536] per_item.assert_not_awaited() From 844a3cbae7644b2945afcbb7691cee31b2c891b3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 14:32:16 +0900 Subject: [PATCH 09/21] test: cover canonical read state and UTF-8 token boundaries --- backend/tests/test_alembic_migrations.py | 3 ++- backend/tests/test_embedding.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index fecd4697d..fa588c8d4 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -577,7 +577,8 @@ def test_published_read_state_revision_is_immutable(): assert 'revision = "0011_email_read_state"' in revision_text assert 'down_revision = "0009_project_graph_projection"' in revision_text - assert 'op.add_column(\n "emails"' in revision_text + assert 'op.add_column(\n "email_records"' in revision_text + assert '"emails"' not in revision_text assert "email_read_state_ownership" not in revision_text diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index 47870166b..28bc37656 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -14,6 +14,25 @@ from services.exceptions import EmbeddingGenerationError +def test_split_embedding_inputs_preserves_unicode_at_token_boundaries(): + """Keep source text intact when a token boundary bisects UTF-8 bytes.""" + source = "é" * 237 + "😀" * 10 + + flattened, ranges, weights = split_embedding_inputs( + [source], + model="test-model", + ) + + encoding = tiktoken.get_encoding("cl100k_base") + assert ranges == [(0, len(flattened))] + assert "".join(flattened) == source + assert "\ufffd" not in "".join(flattened) + assert all( + len(encoding.encode(text, disallowed_special=())) <= EMBEDDING_INPUT_TOKEN_LIMIT + for text in flattened + ) + assert sum(weights) == len(encoding.encode(source, disallowed_special=())) + def test_chunk_text(): text = "This is a long test string. " * 100 chunks = chunk_text(text, chunk_size=50) From d225700ddf17767a337949d37c3fb0e1914e7eb0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 14:40:48 +0900 Subject: [PATCH 10/21] fix: preserve canonical migration and Unicode embedding text --- .../alembic/versions/0011_email_read_state.py | 6 ++-- backend/services/embedding.py | 22 +++++++++++---- backend/tests/test_alembic_migrations.py | 28 +++++++++++++------ 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index 716590cd1..b16d3bc30 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -1,4 +1,4 @@ -"""Add is_read to emails (IMAP \\Seen read state). +"""Add is_read to email_records (IMAP \\Seen read state). Existing rows default to read so historical/file imports do not surface as unread. """ @@ -15,7 +15,7 @@ def upgrade() -> None: op.add_column( - "emails", + "email_records", sa.Column( "is_read", sa.Boolean(), @@ -26,4 +26,4 @@ def upgrade() -> None: def downgrade() -> None: - op.drop_column("emails", "is_read") + op.drop_column("email_records", "is_read") diff --git a/backend/services/embedding.py b/backend/services/embedding.py index d32dbe963..a2d3688b4 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -54,7 +54,7 @@ def split_embedding_inputs( texts: list[str], model: str | None = None, ) -> tuple[list[str], list[tuple[int, int]], list[int]]: - """Split inputs by tokenizer tokens and return token weights per chunk.""" + """Split inputs by tokenizer tokens without corrupting UTF-8 boundaries.""" encoding = _embedding_encoding(model) flattened: list[str] = [] ranges: list[tuple[int, int]] = [] @@ -67,10 +67,22 @@ def split_embedding_inputs( else: chunks = [] weights = [] - for start in range(0, len(tokens), EMBEDDING_INPUT_TOKEN_LIMIT): - token_slice = tokens[start : start + EMBEDDING_INPUT_TOKEN_LIMIT] - chunks.append(encoding.decode(token_slice)) - weights.append(len(token_slice)) + _, offsets = encoding.decode_with_offsets(tokens) + start = 0 + while start < len(tokens): + end = min(start + EMBEDDING_INPUT_TOKEN_LIMIT, len(tokens)) + if end < len(tokens): + while end > start and offsets[end] == offsets[end - 1]: + end -= 1 + if end == start: + end = min(start + EMBEDDING_INPUT_TOKEN_LIMIT, len(tokens)) + while end < len(tokens) and offsets[end] == offsets[start]: + end += 1 + start_character = offsets[start] + end_character = len(text) if end == len(tokens) else offsets[end] + chunks.append(text[start_character:end_character]) + weights.append(end - start) + start = end start = len(flattened) flattened.extend(chunks) token_weights.extend(weights) diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index fa588c8d4..df4a7ce9b 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -570,16 +570,28 @@ def test_email_read_state_revision_uses_canonical_email_records_table(): assert "_EMAIL_TABLE," in revision_text -def test_published_read_state_revision_is_immutable(): - revision_text = ( +def test_read_state_revision_targets_canonical_email_records_table(monkeypatch): + revision_path = ( BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" - ).read_text() + ) + spec = importlib.util.spec_from_file_location("published_read_state", revision_path) + assert spec is not None and spec.loader is not None + revision = importlib.util.module_from_spec(spec) + spec.loader.exec_module(revision) + calls = [] + operations = SimpleNamespace( + add_column=lambda *args: calls.append(("add_column", args)), + drop_column=lambda *args: calls.append(("drop_column", args)), + ) + monkeypatch.setattr(revision, "op", operations) - assert 'revision = "0011_email_read_state"' in revision_text - assert 'down_revision = "0009_project_graph_projection"' in revision_text - assert 'op.add_column(\n "email_records"' in revision_text - assert '"emails"' not in revision_text - assert "email_read_state_ownership" not in revision_text + revision.upgrade() + revision.downgrade() + + assert calls[0][0] == "add_column" + assert calls[0][1][0] == "email_records" + assert calls[0][1][1].name == "is_read" + assert calls[-1] == ("drop_column", ("email_records", "is_read")) def test_read_state_follow_up_revision_is_after_published_revision(): From 8cc7e16964449ff06bde62b45046231856eab305 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 14:46:53 +0900 Subject: [PATCH 11/21] test: require model-native tokenizer for local embeddings --- backend/tests/test_embedding.py | 45 +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index 28bc37656..7589eed92 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -2,6 +2,19 @@ import tiktoken import openai from unittest.mock import patch, AsyncMock +import types + + +class _ProviderResponse: + def __init__(self, payload): + self.status_code = 200 + self._payload = payload + + def json(self): + return self._payload + + def raise_for_status(self): + return None from services.embedding import ( EMBEDDING_INPUT_TOKEN_LIMIT, STORAGE_EMBEDDING_DIMENSION, @@ -142,6 +155,38 @@ async def test_generate_embeddings_uses_selected_provider_model_and_base_url(): mock_client.close.assert_awaited_once() +@pytest.mark.asyncio +async def test_generate_embeddings_uses_provider_native_tokenizer_for_local_model(): + with patch("services.embedding.AsyncOpenAI") as mock_async_openai, patch( + "services.embedding.build_llm_provider_http_client", + new_callable=AsyncMock, + ) as mock_build_client: + mock_http_client = AsyncMock() + mock_http_client.post = AsyncMock( + side_effect=[ + _ProviderResponse({"tokens": [1, 2, 3]}), + _ProviderResponse({"content": "test"}), + ] + ) + mock_build_client.return_value = ("http://host.docker.internal:8082/v1", mock_http_client) + mock_client = mock_async_openai.return_value + mock_client.close = AsyncMock() + mock_response = AsyncMock() + mock_response.data = [AsyncMock(embedding=[0.1, 0.2, 0.3])] + mock_client.embeddings.create = AsyncMock(return_value=mock_response) + + embeddings = await generate_embeddings( + ["test"], + "local-provider", + base_url="http://host.docker.internal:8082/v1", + model="embeddinggemma", + ) + + assert embeddings == [[0.1, 0.2, 0.3]] + assert mock_http_client.post.await_count == 2 + assert mock_http_client.post.await_args_list[0].args[0].endswith("/tokenize") + assert mock_http_client.post.await_args_list[1].args[0].endswith("/detokenize") + @pytest.mark.asyncio async def test_generate_embeddings_prefers_embedding_base_url_when_no_explicit_url(): with patch( From 7306930dd24e7027954bf44b96e6fbc80c91489e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 14:49:28 +0900 Subject: [PATCH 12/21] fix: use provider-native tokenization for local embeddings --- backend/services/embedding.py | 129 +++++++++++++++++- backend/tests/test_embedding.py | 18 ++- ...001-local-llm-and-orchestrator-boundary.md | 14 +- 3 files changed, 148 insertions(+), 13 deletions(-) diff --git a/backend/services/embedding.py b/backend/services/embedding.py index a2d3688b4..53d670b12 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -2,6 +2,7 @@ import tiktoken from langchain_text_splitters import RecursiveCharacterTextSplitter from openai import AsyncOpenAI +from urllib.parse import urlsplit, urlunsplit from core.config import settings from services.llm_provider_urls import build_llm_provider_http_client from services.exceptions import EmbeddingGenerationError @@ -50,6 +51,107 @@ def _embedding_encoding(model: str | None): return tiktoken.get_encoding("cl100k_base") +def _requires_provider_tokenizer(model: str, base_url: str | None) -> bool: + """Require a native tokenizer for an unknown model on a configured endpoint.""" + if not base_url: + return False + try: + tiktoken.encoding_for_model(model) + except (KeyError, ValueError): + return True + return False + + +def _provider_endpoint(base_url: str, resource: str) -> str: + """Build a root-level llama.cpp tokenizer endpoint from an OpenAI base URL.""" + parsed = urlsplit(base_url) + base_path = parsed.path.rstrip("/") + if base_path.endswith("/v1"): + base_path = base_path[:-3] + return urlunsplit( + (parsed.scheme, parsed.netloc, f"{base_path}/{resource}", "", "") + ) + + +async def _provider_json(http_client, url: str, payload: dict) -> dict: + """POST one native tokenizer request and validate its JSON object response.""" + response = await http_client.post(url, json=payload) + if response.status_code == 404: + raise ValueError( + "embedding provider must expose native /tokenize and /detokenize endpoints" + ) + response.raise_for_status() + body = response.json() + if not isinstance(body, dict): + raise ValueError("embedding provider returned a non-object tokenizer response") + return body + + +async def _split_embedding_inputs_with_provider_tokenizer( + texts: list[str], + model: str, + base_url: str, + http_client, + api_key: str, +) -> tuple[list[str], list[tuple[int, int]], list[int]]: + """Split unknown local-model inputs using the provider tokenizer itself.""" + tokenize_url = _provider_endpoint(base_url, "tokenize") + detokenize_url = _provider_endpoint(base_url, "detokenize") + headers = {"Authorization": f"Bearer {api_key}"} if api_key else {} + flattened: list[str] = [] + ranges: list[tuple[int, int]] = [] + token_weights: list[int] = [] + for text in texts: + if not text: + start = len(flattened) + flattened.append(text) + token_weights.append(0) + ranges.append((start, len(flattened))) + continue + token_response = await _provider_json( + http_client, + tokenize_url, + { + "content": text, + "add_special": False, + "parse_special": False, + }, + ) + tokens = token_response.get("tokens") + if not isinstance(tokens, list) or not all( + isinstance(token, int) and not isinstance(token, bool) for token in tokens + ): + raise ValueError("embedding provider returned invalid tokenizer tokens") + if not tokens: + start = len(flattened) + flattened.append(text) + token_weights.append(0) + ranges.append((start, len(flattened))) + continue + start = len(flattened) + for token_start in range(0, len(tokens), EMBEDDING_INPUT_TOKEN_LIMIT): + token_slice = tokens[ + token_start : token_start + EMBEDDING_INPUT_TOKEN_LIMIT + ] + detokenized = await _provider_json( + http_client, + detokenize_url, + {"tokens": token_slice}, + ) + chunk = detokenized.get("content") + if not isinstance(chunk, str): + raise ValueError( + "embedding provider returned invalid detokenized content" + ) + flattened.append(chunk) + token_weights.append(len(token_slice)) + if "".join(flattened[start:]) != text: + raise ValueError( + "embedding provider tokenizer did not preserve source text exactly" + ) + ranges.append((start, len(flattened))) + return flattened, ranges, token_weights + def split_embedding_inputs( texts: list[str], model: str | None = None, @@ -137,19 +239,36 @@ async def generate_embeddings( raise ValueError("OPENAI_API_KEY is not set") selected_model = model or settings.OPENAI_EMBEDDING_MODEL - request_texts, input_ranges, token_weights = split_embedding_inputs( - texts, selected_model - ) - - # Instantiate client locally to avoid global state race conditions across tenants configured_base_url = base_url if configured_base_url is None: configured_base_url = ( settings.OPENAI_EMBEDDING_BASE_URL or settings.OPENAI_BASE_URL ) + + # Instantiate the pinned client before tokenization so unknown local models + # use the exact tokenizer loaded by the provider rather than cl100k_base. validated_base_url, http_client = await build_llm_provider_http_client( configured_base_url ) + try: + if _requires_provider_tokenizer(selected_model, validated_base_url): + request_texts, input_ranges, token_weights = ( + await _split_embedding_inputs_with_provider_tokenizer( + texts, + selected_model, + validated_base_url, + http_client, + openai_api_key, + ) + ) + else: + request_texts, input_ranges, token_weights = split_embedding_inputs( + texts, selected_model + ) + except Exception: + await http_client.aclose() + raise + client = AsyncOpenAI( api_key=openai_api_key, base_url=validated_base_url, diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index 7589eed92..2a149be32 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -2,7 +2,6 @@ import tiktoken import openai from unittest.mock import patch, AsyncMock -import types class _ProviderResponse: @@ -130,6 +129,12 @@ async def test_generate_embeddings_uses_selected_provider_model_and_base_url(): new_callable=AsyncMock, ) as mock_build_client: mock_http_client = AsyncMock() + mock_http_client.post = AsyncMock( + side_effect=[ + _ProviderResponse({"tokens": [1]}), + _ProviderResponse({"content": "test"}), + ] + ) mock_build_client.return_value = ("http://ollama:11434/v1", mock_http_client) mock_client = mock_async_openai.return_value mock_client.close = AsyncMock() @@ -195,7 +200,16 @@ async def test_generate_embeddings_prefers_embedding_base_url_when_no_explicit_u "services.embedding.build_llm_provider_http_client", new_callable=AsyncMock, ) as mock_build_client: - mock_build_client.return_value = ("http://host.docker.internal:8082/v1", AsyncMock()) + mock_http_client = AsyncMock() + mock_http_client.post = AsyncMock( + side_effect=[ + _ProviderResponse({"tokens": [1]}), + _ProviderResponse({"content": "test"}), + ] + ) + mock_build_client.return_value = ( + "http://host.docker.internal:8082/v1", mock_http_client + ) mock_client = mock_async_openai.return_value mock_client.close = AsyncMock() mock_client.embeddings.create = AsyncMock(return_value=AsyncMock(data=[])) diff --git a/docs/adr/0001-local-llm-and-orchestrator-boundary.md b/docs/adr/0001-local-llm-and-orchestrator-boundary.md index d6d2a057a..fb6f3d475 100644 --- a/docs/adr/0001-local-llm-and-orchestrator-boundary.md +++ b/docs/adr/0001-local-llm-and-orchestrator-boundary.md @@ -55,9 +55,10 @@ EmbeddingGemma may be separate local OpenAI-compatible endpoints. policy. Only an oversized semantic segment is further split by the shared boundary-aware embedding splitter, with no overlap, before it reaches a provider. The current local EmbeddingGemma/llama.cpp contract uses a - conservative 256-character request ceiling because the runtime's physical - token batch limit is lower than the length of some real mail bodies and - attachments. + conservative 256-token request ceiling. Unknown local model IDs are split + by the provider-native `/tokenize` and `/detokenize` endpoints before the + OpenAI-compatible `/v1/embeddings` request, so a tiktoken fallback cannot + silently exceed the runtime tokenizer limit. 10. The existing `Email.embedding` and `Attachment.embedding` columns remain source-level compatibility vectors. Import mean-pools segment embeddings into one centroid per email or attachment, while the persisted content @@ -77,8 +78,9 @@ EmbeddingGemma may be separate local OpenAI-compatible endpoints. `services.email_import_service` to build the embedding inputs and graph records from the same `ParseResult`. - Keep `services.embedding.generate_embeddings` as the provider safety boundary - for oversized individual semantic segments and retain the existing - contextual-orchestrator batch seam. + for oversized individual semantic segments, using the native tokenizer API + for unknown local models, and retain the existing contextual-orchestrator + batch seam. - Mean-pool segment vectors back to the existing source-level vector columns. ### Verification @@ -111,7 +113,7 @@ EmbeddingGemma may be separate local OpenAI-compatible endpoints. English, and code tasks, including quantized and truncated variants; that supports EmbeddingGemma as a low-memory embedding candidate, not as a chat runtime. -- Niklas Muennighoff et al., [“MTEB: Massive Text Embedding +- ggml-org. (2026). [*llama.cpp server REST API*](https://github.com/ggml-org/llama.cpp/blob/master/tools/server/README.md) [Computer software]. GitHub. The documented `/tokenize` and `/detokenize` endpoints expose the model-loaded tokenizer and make the 256-token boundary provider-authoritative instead of assuming that a different offline tokenizer is equivalent.\n- Niklas Muennighoff et al., [“MTEB: Massive Text Embedding Benchmark”](https://arxiv.org/abs/2210.07316), arXiv:2210.07316 (2023). MTEB spans multiple tasks, datasets, and languages and finds no universal embedding method; this supports measuring the selected local model in From 1d4483f169460b2cb22073cd15b8bab9d025216c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 15:04:16 +0900 Subject: [PATCH 13/21] test: keep embedding helper after imports --- backend/tests/test_embedding.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index 2a149be32..2dd268868 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -4,16 +4,6 @@ from unittest.mock import patch, AsyncMock -class _ProviderResponse: - def __init__(self, payload): - self.status_code = 200 - self._payload = payload - - def json(self): - return self._payload - - def raise_for_status(self): - return None from services.embedding import ( EMBEDDING_INPUT_TOKEN_LIMIT, STORAGE_EMBEDDING_DIMENSION, @@ -25,6 +15,17 @@ def raise_for_status(self): ) from services.exceptions import EmbeddingGenerationError +class _ProviderResponse: + def __init__(self, payload): + self.status_code = 200 + self._payload = payload + + def json(self): + return self._payload + + def raise_for_status(self): + return None + def test_split_embedding_inputs_preserves_unicode_at_token_boundaries(): """Keep source text intact when a token boundary bisects UTF-8 bytes.""" From d0920c2a83be2bd2904364593f29d99ba2be6509 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 15:08:59 +0900 Subject: [PATCH 14/21] fix: send provider auth to native tokenizer endpoints --- backend/services/embedding.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/backend/services/embedding.py b/backend/services/embedding.py index 53d670b12..f48b336c6 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -73,9 +73,14 @@ def _provider_endpoint(base_url: str, resource: str) -> str: ) -async def _provider_json(http_client, url: str, payload: dict) -> dict: +async def _provider_json( + http_client, + url: str, + payload: dict, + headers: dict[str, str] | None = None, +) -> dict: """POST one native tokenizer request and validate its JSON object response.""" - response = await http_client.post(url, json=payload) + response = await http_client.post(url, json=payload, headers=headers) if response.status_code == 404: raise ValueError( "embedding provider must expose native /tokenize and /detokenize endpoints" @@ -116,6 +121,7 @@ async def _split_embedding_inputs_with_provider_tokenizer( "add_special": False, "parse_special": False, }, + headers=headers, ) tokens = token_response.get("tokens") if not isinstance(tokens, list) or not all( @@ -137,6 +143,7 @@ async def _split_embedding_inputs_with_provider_tokenizer( http_client, detokenize_url, {"tokens": token_slice}, + headers=headers, ) chunk = detokenized.get("content") if not isinstance(chunk, str): From 8c0a6542c461961ca7a6cc85a0788188cf0a3575 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 15:14:16 +0900 Subject: [PATCH 15/21] fix: normalize weighted embedding pooling --- backend/services/embedding.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/services/embedding.py b/backend/services/embedding.py index f48b336c6..99c0d6cf2 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -225,10 +225,9 @@ def pool_embedding_chunks( ( chunk[index] if index < len(chunk) else 0.0 ) - * weights[chunk_index] + * (weights[chunk_index] / total_weight) for chunk_index, chunk in enumerate(chunks) ) - / total_weight for index in range(width) ] ) From e78179a1eda7b9c56306c727c6dbca2a05a407ea Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 15:23:56 +0900 Subject: [PATCH 16/21] test: cover published migration and remote embedding fallback --- backend/tests/test_alembic_migrations.py | 6 ++-- backend/tests/test_embedding.py | 38 ++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index df4a7ce9b..3d758cdd0 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -496,7 +496,7 @@ def test_email_read_state_revision_adds_and_records_column_ownership(monkeypatch "create_table", "bulk_insert", ] - assert calls[0][1][0] == "email_records" + assert calls[0][1][0] == "emails" assert calls[0][1][1].name == "is_read" assert calls[0][1][1].nullable is False @@ -570,7 +570,7 @@ def test_email_read_state_revision_uses_canonical_email_records_table(): assert "_EMAIL_TABLE," in revision_text -def test_read_state_revision_targets_canonical_email_records_table(monkeypatch): +def test_published_read_state_revision_targets_published_emails_table(monkeypatch): revision_path = ( BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" ) @@ -591,7 +591,7 @@ def test_read_state_revision_targets_canonical_email_records_table(monkeypatch): assert calls[0][0] == "add_column" assert calls[0][1][0] == "email_records" assert calls[0][1][1].name == "is_read" - assert calls[-1] == ("drop_column", ("email_records", "is_read")) + assert calls[-1] == ("drop_column", ("emails", "is_read")) def test_read_state_follow_up_revision_is_after_published_revision(): diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index 2dd268868..3cd4c412e 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -16,8 +16,8 @@ from services.exceptions import EmbeddingGenerationError class _ProviderResponse: - def __init__(self, payload): - self.status_code = 200 + def __init__(self, payload, *, status_code=200): + self.status_code = status_code self._payload = payload def json(self): @@ -193,6 +193,40 @@ async def test_generate_embeddings_uses_provider_native_tokenizer_for_local_mode assert mock_http_client.post.await_args_list[0].args[0].endswith("/tokenize") assert mock_http_client.post.await_args_list[1].args[0].endswith("/detokenize") +@pytest.mark.asyncio +async def test_generate_embeddings_falls_back_for_remote_unknown_model_without_native_tokenizer(): + with patch("services.embedding.AsyncOpenAI") as mock_async_openai, patch( + "services.embedding.build_llm_provider_http_client", + new_callable=AsyncMock, + ) as mock_build_client: + mock_http_client = AsyncMock() + mock_http_client.post = AsyncMock( + return_value=_ProviderResponse({}, status_code=404) + ) + mock_build_client.return_value = ( + "https://remote.example/v1", mock_http_client + ) + mock_client = mock_async_openai.return_value + mock_client.close = AsyncMock() + mock_client.embeddings.create = AsyncMock( + return_value=AsyncMock(data=[AsyncMock(embedding=[0.1, 0.2, 0.3])]) + ) + + embeddings = await generate_embeddings( + ["test"], + "remote-key", + base_url="https://remote.example/v1", + model="remote-custom-model", + ) + + assert embeddings == [[0.1, 0.2, 0.3]] + mock_http_client.post.assert_awaited_once() + mock_client.embeddings.create.assert_awaited_once_with( + model="remote-custom-model", input=["test"] + ) + mock_client.close.assert_awaited_once() + + @pytest.mark.asyncio async def test_generate_embeddings_prefers_embedding_base_url_when_no_explicit_url(): with patch( From 071a9d6df607873169b739d46ef56a75d2d6f2e3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 15:24:52 +0900 Subject: [PATCH 17/21] fix: preserve migration contract and fallback tokenizer --- .../alembic/versions/0011_email_read_state.py | 6 ++-- backend/services/embedding.py | 29 ++++++++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index b16d3bc30..716590cd1 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -1,4 +1,4 @@ -"""Add is_read to email_records (IMAP \\Seen read state). +"""Add is_read to emails (IMAP \\Seen read state). Existing rows default to read so historical/file imports do not surface as unread. """ @@ -15,7 +15,7 @@ def upgrade() -> None: op.add_column( - "email_records", + "emails", sa.Column( "is_read", sa.Boolean(), @@ -26,4 +26,4 @@ def upgrade() -> None: def downgrade() -> None: - op.drop_column("email_records", "is_read") + op.drop_column("emails", "is_read") diff --git a/backend/services/embedding.py b/backend/services/embedding.py index 99c0d6cf2..11533229d 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -73,6 +73,10 @@ def _provider_endpoint(base_url: str, resource: str) -> str: ) +class _ProviderTokenizerUnavailable(ValueError): + """Signal that a provider does not expose the optional native tokenizer.""" + + async def _provider_json( http_client, url: str, @@ -82,8 +86,8 @@ async def _provider_json( """POST one native tokenizer request and validate its JSON object response.""" response = await http_client.post(url, json=payload, headers=headers) if response.status_code == 404: - raise ValueError( - "embedding provider must expose native /tokenize and /detokenize endpoints" + raise _ProviderTokenizerUnavailable( + "embedding provider does not expose native tokenizer endpoints" ) response.raise_for_status() body = response.json() @@ -258,15 +262,20 @@ async def generate_embeddings( ) try: if _requires_provider_tokenizer(selected_model, validated_base_url): - request_texts, input_ranges, token_weights = ( - await _split_embedding_inputs_with_provider_tokenizer( - texts, - selected_model, - validated_base_url, - http_client, - openai_api_key, + try: + request_texts, input_ranges, token_weights = ( + await _split_embedding_inputs_with_provider_tokenizer( + texts, + selected_model, + validated_base_url, + http_client, + openai_api_key, + ) + ) + except _ProviderTokenizerUnavailable: + request_texts, input_ranges, token_weights = split_embedding_inputs( + texts, selected_model ) - ) else: request_texts, input_ranges, token_weights = split_embedding_inputs( texts, selected_model From ab0ab18ec1d9afc2e184cfa9f0191edc79424661 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 15:31:32 +0900 Subject: [PATCH 18/21] test: keep published and canonical migration contracts distinct --- backend/tests/test_alembic_migrations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index 3d758cdd0..da732761e 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -496,7 +496,7 @@ def test_email_read_state_revision_adds_and_records_column_ownership(monkeypatch "create_table", "bulk_insert", ] - assert calls[0][1][0] == "emails" + assert calls[0][1][0] == "email_records" assert calls[0][1][1].name == "is_read" assert calls[0][1][1].nullable is False @@ -589,7 +589,7 @@ def test_published_read_state_revision_targets_published_emails_table(monkeypatc revision.downgrade() assert calls[0][0] == "add_column" - assert calls[0][1][0] == "email_records" + assert calls[0][1][0] == "emails" assert calls[0][1][1].name == "is_read" assert calls[-1] == ("drop_column", ("emails", "is_read")) From 5dcd313bafb7c13e13a7d681c1f4f6d203be13e1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 20 Aug 2026 19:43:04 +0900 Subject: [PATCH 19/21] fix(governance): extract trusted gate at workspace root --- .github/workflows/pr-governance.yml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-governance.yml b/.github/workflows/pr-governance.yml index de8e09101..899228b3d 100644 --- a/.github/workflows/pr-governance.yml +++ b/.github/workflows/pr-governance.yml @@ -228,6 +228,7 @@ jobs: sleep $((attempt * 3)) done python3 - "$trusted_archive" "$trusted_workspace" <<'PY' + import copy import os import sys import tarfile @@ -237,9 +238,14 @@ jobs: workspace = Path(workspace_path).resolve() with tarfile.open(archive_path, "r:gz") as archive: members = archive.getmembers() + safe_members = [] unsafe = [] for member in members: member_path = PurePosixPath(member.name) + if len(member_path.parts) == 1: + if not member.isdir(): + unsafe.append(member.name) + continue target = (workspace / Path(*member_path.parts[1:])).resolve() if ( not member.name @@ -251,9 +257,15 @@ jobs: or os.path.commonpath((workspace, target)) != str(workspace) ): unsafe.append(member.name) + continue + safe_member = copy.copy(member) + safe_member.name = PurePosixPath( + *member_path.parts[1:] + ).as_posix() + safe_members.append(safe_member) if unsafe: raise SystemExit(f"unsafe trusted governance archive member: {unsafe[0]!r}") - archive.extractall(workspace, members=members) + archive.extractall(workspace, members=safe_members) PY test -d "$trusted_workspace/scripts" governance_script="$trusted_workspace/scripts/ci/pr_governance_gate.sh" From 8772ca3249cc392952e1ecf1e173044c438f9f08 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 21 Aug 2026 00:31:52 +0900 Subject: [PATCH 20/21] fix(embedding): validate provider results and cleanup --- backend/services/embedding.py | 26 ++++++++++++-------- backend/tests/test_alembic_migrations.py | 31 ++++++++++++++++++++++++ backend/tests/test_embedding.py | 20 ++++++++++++++- 3 files changed, 66 insertions(+), 11 deletions(-) diff --git a/backend/services/embedding.py b/backend/services/embedding.py index cad1ee9ff..f38ce51cb 100644 --- a/backend/services/embedding.py +++ b/backend/services/embedding.py @@ -287,11 +287,15 @@ async def generate_embeddings( await http_client.aclose() raise - client = AsyncOpenAI( - api_key=openai_api_key, - base_url=validated_base_url, - http_client=http_client, - ) + try: + client = AsyncOpenAI( + api_key=openai_api_key, + base_url=validated_base_url, + http_client=http_client, + ) + except Exception: + await http_client.aclose() + raise try: response = await provider_circuit_breaker.call( @@ -309,11 +313,13 @@ async def generate_embeddings( operation_name="embedding generation", ), ) - return pool_embedding_chunks( - [data.embedding for data in response.data], - input_ranges, - token_weights, - ) + provider_embeddings = [data.embedding for data in response.data] + if len(provider_embeddings) != len(request_texts): + raise ValueError( + "embedding provider returned an unexpected vector count: " + f"expected {len(request_texts)}, received {len(provider_embeddings)}" + ) + return pool_embedding_chunks(provider_embeddings, input_ranges, token_weights) except openai.OpenAIError as e: raise EmbeddingGenerationError(f"Failed to generate embeddings: {str(e)}") finally: diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index da732761e..513863129 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -626,6 +626,37 @@ def test_read_state_follow_up_merge_reconciles_current_graph(): assert "op.create_table(" not in revision_text +def test_read_state_follow_up_merge_executes_without_schema_operations(monkeypatch): + revision_path = ( + BACKEND_ROOT + / "alembic" + / "versions" + / "0019_merge_email_read_state_ownership.py" + ) + spec = importlib.util.spec_from_file_location( + "merge_email_read_state_ownership", revision_path + ) + assert spec is not None and spec.loader is not None + revision = importlib.util.module_from_spec(spec) + spec.loader.exec_module(revision) + + calls = [] + + class _OperationRecorder: + def __getattr__(self, operation_name): + def record(*args, **kwargs): + calls.append((operation_name, args, kwargs)) + + return record + + monkeypatch.setattr(revision, "op", _OperationRecorder(), raising=False) + + revision.upgrade() + revision.downgrade() + + assert calls == [] + + def test_merge_revision_reconciles_newsdom_provider_branch(): revision_path = ( BACKEND_ROOT / "alembic" / "versions" / "0015_merge_newsdom_email_heads.py" diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index 34803151c..e0f44d431 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -255,11 +255,29 @@ async def test_generate_embeddings_prefers_embedding_base_url_when_no_explicit_u ) mock_settings.OPENAI_BASE_URL = "http://host.docker.internal:8080/v1" mock_settings.OPENAI_EMBEDDING_MODEL = "embeddinggemma" - await generate_embeddings(["test"], "local-provider") + with pytest.raises(ValueError, match="unexpected vector count"): + await generate_embeddings(["test"], "local-provider") mock_build_client.assert_awaited_once_with("http://host.docker.internal:8082/v1") +@pytest.mark.asyncio +async def test_generate_embeddings_closes_http_client_when_openai_constructor_fails(): + mock_http_client = AsyncMock() + with patch( + "services.embedding.build_llm_provider_http_client", + new_callable=AsyncMock, + return_value=(None, mock_http_client), + ), patch( + "services.embedding.AsyncOpenAI", + side_effect=RuntimeError("client construction failed"), + ): + with pytest.raises(RuntimeError, match="client construction failed"): + await generate_embeddings(["test"], "provider-key") + + mock_http_client.aclose.assert_awaited_once() + + @pytest.mark.asyncio async def test_generate_embeddings_requests_storage_dimensions_for_openai_v3(): with patch( From 248652dbb0f807ad59187f168dfa9aff9fb9772f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 21 Aug 2026 00:35:12 +0900 Subject: [PATCH 21/21] test(embedding): cover tokenizer failure contracts --- backend/tests/test_embedding.py | 75 +++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/backend/tests/test_embedding.py b/backend/tests/test_embedding.py index e0f44d431..834bcebc2 100644 --- a/backend/tests/test_embedding.py +++ b/backend/tests/test_embedding.py @@ -7,6 +7,9 @@ from services.embedding import ( EMBEDDING_INPUT_TOKEN_LIMIT, STORAGE_EMBEDDING_DIMENSION, + _ProviderTokenizerUnavailable, + _provider_json, + _split_embedding_inputs_with_provider_tokenizer, chunk_text, fit_embedding_vector, generate_embeddings, @@ -221,6 +224,15 @@ async def test_generate_embeddings_falls_back_for_remote_unknown_model_without_n assert embeddings == [[0.1, 0.2, 0.3]] mock_http_client.post.assert_awaited_once() + assert mock_http_client.post.await_args.args[0] == "https://remote.example/tokenize" + assert mock_http_client.post.await_args.kwargs == { + "json": { + "content": "test", + "add_special": False, + "parse_special": False, + }, + "headers": {"Authorization": "Bearer remote-key"}, + } mock_client.embeddings.create.assert_awaited_once_with( model="remote-custom-model", input=["test"] ) @@ -278,6 +290,69 @@ async def test_generate_embeddings_closes_http_client_when_openai_constructor_fa mock_http_client.aclose.assert_awaited_once() +@pytest.mark.asyncio +async def test_provider_json_rejects_missing_native_tokenizer_endpoint(): + mock_http_client = AsyncMock() + mock_http_client.post.return_value = _ProviderResponse({}, status_code=404) + + with pytest.raises(_ProviderTokenizerUnavailable, match="does not expose"): + await _provider_json(mock_http_client, "https://local.example/tokenize", {}) + + +@pytest.mark.asyncio +async def test_provider_tokenizer_rejects_invalid_token_payload(): + mock_http_client = AsyncMock() + mock_http_client.post.return_value = _ProviderResponse({"tokens": [True]}) + + with pytest.raises(ValueError, match="invalid tokenizer tokens"): + await _split_embedding_inputs_with_provider_tokenizer( + ["test"], + "embeddinggemma", + "https://local.example/v1", + mock_http_client, + "provider-key", + ) + + +@pytest.mark.asyncio +async def test_provider_tokenizer_rejects_source_mismatch(): + mock_http_client = AsyncMock() + mock_http_client.post.side_effect = [ + _ProviderResponse({"tokens": [1]}), + _ProviderResponse({"content": "different"}), + ] + + with pytest.raises(ValueError, match="did not preserve source text exactly"): + await _split_embedding_inputs_with_provider_tokenizer( + ["test"], + "embeddinggemma", + "https://local.example/v1", + mock_http_client, + "provider-key", + ) + + +@pytest.mark.asyncio +async def test_generate_embeddings_closes_http_client_when_tokenizer_fails(): + mock_http_client = AsyncMock() + mock_http_client.post.side_effect = RuntimeError("tokenizer failed") + + with patch( + "services.embedding.build_llm_provider_http_client", + new_callable=AsyncMock, + return_value=("https://local.example/v1", mock_http_client), + ): + with pytest.raises(RuntimeError, match="tokenizer failed"): + await generate_embeddings( + ["test"], + "provider-key", + base_url="https://local.example/v1", + model="provider-local-model", + ) + + mock_http_client.aclose.assert_awaited_once() + + @pytest.mark.asyncio async def test_generate_embeddings_requests_storage_dimensions_for_openai_v3(): with patch(