From d47bb2c549cba86ca6c4936bf764b1080bd2fe1d Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 8 Jul 2026 17:12:51 +0000 Subject: [PATCH] feat(mcp): improve remote ingest guidance Make upload-ticket auth the default remote-agent path by returning complete upload URLs and updating service output/docs. Add validate_upload and preview_upload_metadata for lightweight file checks and exact sha256 duplicate warnings before ingest. Expose version, capability schema, file-index freshness, queue state, and known limitations through get_mcp_capabilities. Validation: python3 -m unittest discover -v; python3 -m compileall obsidian_wiki tests scripts; git diff --check. --- AGENTS.md | 2 +- README.md | 16 ++-- obsidian_wiki/mcp_ingest.py | 156 +++++++++++++++++++++++++++++- obsidian_wiki/mcp_server.py | 163 +++++++++++++++++++++++++++++--- scripts/manage_mcp_service.py | 63 +++++++++--- tests/test_mcp_ingest.py | 75 +++++++++++++++ tests/test_mcp_server_remote.py | 88 ++++++++++++++++- tests/test_mcp_service.py | 22 ++++- 8 files changed, 549 insertions(+), 36 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 250f1ac..5b07e25 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -130,7 +130,7 @@ See `wiki-query` and `wiki-export` skills for how the filter is applied. - **Keep product lookup fields explicit.** Carry forward `bjd_part_numbers`, `competitor_part_numbers`, `series`, `pitch`, `pin_count`, `product_lines`, `customers`, `competitors`, `standards`, `business_categories`, `related_pages`, and `catalog_paths` whenever they are known. - **Merge, do not overwrite.** Automatic identifier extraction may add terms, but it must not discard curated registry metadata. Registry values are the durable source for future index rebuilds and remote MCP lookup. - **Prefer precise identifiers in search ranking.** Exact or prefix matches in structured part-number and series fields should rank above incidental filename substrings such as timestamps or archive hashes. -- **Keep remote ingest gated.** MCP ingest tools are write-capable and must remain disabled unless the service config explicitly enables the `ingest` tool group via `OBSIDIAN_MCP_ENABLED_TOOLS=file-assets,ingest`, `mcp_service.sh tools enable ingest`, or `mcp_service.sh start --enable-ingest`. Uploaded files and direct text notes must stage outside the vault under the MCP runtime root; `wiki-ingest` is responsible for accepting, archiving, and indexing sources inside the vault. Use `ingest_text_note` only for short remote-agent thoughts/notes; it must wrap the text as a temporary markdown upload and then use the fixed `/wiki-ingest` job path. Use the HTTP multipart endpoint `/mcp-ingest/uploads` for file transfer; if the remote agent cannot read the main MCP bearer token, it must call `create_ingest_upload_ticket` and send the returned single-use `X-Obsidian-Wiki-Upload-Token` header. Upload tickets are consumed only by authorized `POST` uploads, the upload endpoint must enforce configured host/origin restrictions, and oversized request bodies must fail with HTTP `413`. Do not expose base64 or chunked upload tools in the MCP tool surface. They may launch local OpenCode only through the fixed `/wiki-ingest` prompt; never expose arbitrary prompt or shell execution through MCP. Remote ingest responses must not expose local runtime paths; internal job records may keep paths for the fixed local prompt. Remote ingest must keep the OpenCode readiness preflight in front of job launch, and `check_ingest_opencode_readiness(run_smoke_test=true)` should stay available for operators to verify that OpenCode can load the project `wiki-ingest` skill before accepting uploads. +- **Keep remote ingest gated.** MCP ingest tools are write-capable and must remain disabled unless the service config explicitly enables the `ingest` tool group via `OBSIDIAN_MCP_ENABLED_TOOLS=file-assets,ingest`, `mcp_service.sh tools enable ingest`, or `mcp_service.sh start --enable-ingest`. Uploaded files and direct text notes must stage outside the vault under the MCP runtime root; `wiki-ingest` is responsible for accepting, archiving, and indexing sources inside the vault. Use `ingest_text_note` only for short remote-agent thoughts/notes; it must wrap the text as a temporary markdown upload and then use the fixed `/wiki-ingest` job path. Remote agents should use `create_ingest_upload_ticket` and the returned `upload_url` plus `X-Obsidian-Wiki-Upload-Token` for HTTP multipart uploads; bearer-token uploads remain only for local/admin compatibility. Prefer `start_job=false`, then `preview_upload_metadata`, then `start_ingest_job` after the preview looks correct. `validate_upload` and `preview_upload_metadata` may report exact sha256 duplicates, but must not claim fuzzy dedupe. Upload tickets are consumed only by authorized `POST` uploads, the upload endpoint must enforce configured host/origin restrictions, and oversized request bodies must fail with HTTP `413`. Do not expose base64 or chunked upload tools in the MCP tool surface. They may launch local OpenCode only through the fixed `/wiki-ingest` prompt; never expose arbitrary prompt or shell execution through MCP. Remote ingest responses must not expose local runtime paths; internal job records may keep paths for the fixed local prompt. Remote ingest must keep the OpenCode readiness preflight in front of job launch, and `check_ingest_opencode_readiness(run_smoke_test=true)` should stay available for operators to verify that OpenCode can load the project `wiki-ingest` skill before accepting uploads. - **Keep remote query gated.** MCP query tools must remain disabled unless the service config explicitly enables the `query` tool group via `OBSIDIAN_MCP_ENABLED_TOOLS=file-assets,query`, `mcp_service.sh tools enable query`, or `mcp_service.sh start --enable-query`. They may launch local OpenCode only through the fixed `/wiki-query` prompt, with no arbitrary prompt or shell execution. `ask_wiki` is the preferred remote-agent query entrypoint; the async `start_wiki_query`/status/result tools remain available for long jobs. `public_only` defaults to false, meaning remote answers may use the full local vault visibility; enable the tool group only for trusted clients and set `public_only=true` per call for filtered user-facing answers. Remote query responses may expose log tails but must not expose local log file paths, and structured fields such as `answer_markdown`, `citations`, `warnings`, and `confidence` must remain backward-compatible with the legacy `answer` field. - **Keep OpenCode model choice server-side.** MCP clients must not pass arbitrary model names through ingest/query tools. Configure local defaults in `.runtime/obsidian-wiki-mcp.env`: ingest uses `OBSIDIAN_MCP_INGEST_OPENCODE_MODEL=openai/gpt-5.5` with `OBSIDIAN_MCP_INGEST_OPENCODE_VARIANT=high`; query uses `OBSIDIAN_MCP_QUERY_OPENCODE_MODEL=openai/gpt-5.5` with `OBSIDIAN_MCP_QUERY_OPENCODE_VARIANT=minimal`. diff --git a/README.md b/README.md index 589cb07..c057879 100644 --- a/README.md +++ b/README.md @@ -464,31 +464,33 @@ This adds write-capable MCP tools to the same `wiki-file-assets` server and bear - `get_ingest_upload_instructions` — returns the preferred upload methods, HTTP endpoint path, fields, limits, and curl example for remote agents. - `ingest_text_note` — accepts a short thought/note as text, wraps it as a temporary markdown upload, and optionally starts the normal fixed `/wiki-ingest` job. -- `create_ingest_upload_ticket` — returns a short-lived, single-use upload token for the HTTP endpoint so remote agents can upload with curl without reading the main MCP bearer token. +- `create_ingest_upload_ticket` — returns a short-lived, single-use upload token plus the upload URL for remote-agent HTTP uploads. +- `validate_upload` — checks planned file names, sizes, sha256 values, count limits, and exact duplicate sha matches before upload. +- `preview_upload_metadata` — previews completed uploads, inferred metadata, and exact duplicate warnings before starting ingest. - `check_ingest_opencode_readiness` — verifies the local OpenCode binary, required run flags, project ingest skills, and optionally runs a no-write smoke test that confirms `wiki-ingest` can be loaded. - `start_ingest_job` — queues a local `opencode run --auto --format json` job with a fixed `/wiki-ingest` prompt for the completed uploads. - `get_ingest_job_status`, `get_ingest_job_result`, `cancel_ingest_job` — inspect or cancel the local job. -The managed service config sets the OpenCode model server-side. By default ingest uses `OBSIDIAN_MCP_INGEST_OPENCODE_MODEL=openai/gpt-5.5` and `OBSIDIAN_MCP_INGEST_OPENCODE_VARIANT=high` for stronger file processing. Change those in `.runtime/obsidian-wiki-mcp.env` on the server if needed; remote MCP clients do not choose the model per call. +The managed service config sets the OpenCode model server-side. By default ingest uses `OBSIDIAN_MCP_INGEST_OPENCODE_MODEL=openai/gpt-5.5` and `OBSIDIAN_MCP_INGEST_OPENCODE_VARIANT=high` for stronger file processing. It also writes `OBSIDIAN_MCP_PUBLIC_BASE_URL` from the detected remote address so ticket responses can include a complete upload URL; set it manually when the service is behind a tunnel, reverse proxy, or custom DNS name. Change these values in `.runtime/obsidian-wiki-mcp.env` on the server if needed; remote MCP clients do not choose the model per call. For short text thoughts, notes, findings, or pasted context, call `ingest_text_note(text, title?, project?, doc_type_hint?, note?, start_job?)`. It uses `doc_type_hint="thought"` by default and returns the created upload plus the queued job when `start_job=true`. -For binary files and larger source documents, use the HTTP upload endpoint. If the remote agent cannot read the main MCP bearer token, first call `create_ingest_upload_ticket`, then upload with the returned token: +For binary files and larger source documents, remote agents should use upload tickets by default: call `create_ingest_upload_ticket(start_job=false)`, upload with the returned `upload_url` and `X-Obsidian-Wiki-Upload-Token`, call `preview_upload_metadata(upload_ids)`, then call `start_ingest_job(upload_ids)` when the preview looks correct: ```bash curl -H "X-Obsidian-Wiki-Upload-Token: " \ -F "files=@drawing.pdf" \ -F "project=bjd" \ -F "doc_type_hint=drawing" \ - -F "start_job=true" \ - http://127.0.0.1:8765/mcp-ingest/uploads + -F "start_job=false" \ + ``` -The endpoint accepts repeated `files` fields plus optional `project`, `doc_type_hint`, `note`, and `start_job` fields. It accepts either the same bearer token as `/mcp` or the single-use upload ticket header. Upload tickets are consumed only by authorized `POST` uploads, not by accidental `GET`/wrong-method requests. The upload route enforces configured `--allowed-host` and `--allowed-origin` checks, and rejects oversized request bodies with HTTP `413` before parsing multipart content. Base64 file upload is intentionally not exposed as an MCP tool because large JSON arguments are fragile in many agent tool-call implementations. +The endpoint accepts repeated `files` fields plus optional `project`, `doc_type_hint`, `note`, and `start_job` fields. It still accepts the same bearer token as `/mcp` for local/admin compatibility, but remote agents should prefer single-use upload tickets. Upload tickets are consumed only by authorized `POST` uploads, not by accidental `GET`/wrong-method requests. The upload route enforces configured `--allowed-host` and `--allowed-origin` checks, and rejects oversized request bodies with HTTP `413` before parsing multipart content. Base64 file upload is intentionally not exposed as an MCP tool because large JSON arguments are fragile in many agent tool-call implementations. Duplicate detection is exact sha256 matching only; it does not replace human review for near-duplicate or renamed source files. Run `check_ingest_opencode_readiness` before the first remote upload. Use `run_smoke_test: true` when you want OpenCode itself to prove it can load the project `wiki-ingest` skill; otherwise the static preflight is also run automatically before every `start_ingest_job`. -Uploads and text notes land under `.runtime/mcp-ingest/uploads/`, outside the Obsidian vault, and are treated only as temporary source staging. Job state and logs live under `.runtime/mcp-ingest/jobs/`. Remote upload and job responses expose upload/job ids, hashes, states, and log tails, but omit local runtime file paths; the internal job record still keeps paths so the fixed local OpenCode prompt can ingest only the uploaded files. The MCP server refuses to enable ingest if `OBSIDIAN_MCP_INGEST_RUNTIME_ROOT` points inside the vault; let `wiki-ingest` decide how to archive sources into `_sources/archive/`, update registries, and rebuild the file index. The ingest prompt is generated by the MCP service, requires OpenCode to load `wiki-ingest`, and restricts OpenCode to the uploaded file list; clients cannot pass arbitrary shell commands through this interface. Because enabling ingest gives the remote token write access to the vault through local OpenCode, keep it on a trusted network and rotate the token if it has been shared too broadly. +Uploads and text notes land under `.runtime/mcp-ingest/uploads/`, outside the Obsidian vault, and are treated only as temporary source staging. Job state and logs live under `.runtime/mcp-ingest/jobs/`. Remote upload and job responses expose upload/job ids, hashes, states, and log tails, but omit local runtime file paths; the internal job record still keeps paths so the fixed local OpenCode prompt can ingest only the uploaded files. `get_mcp_capabilities` includes `server_version`, `capabilities_schema_version`, `tools_version`, file-index freshness, ingest queue state, and known limitations so remote agents can detect stale client instructions. The MCP server refuses to enable ingest if `OBSIDIAN_MCP_INGEST_RUNTIME_ROOT` points inside the vault; let `wiki-ingest` decide how to archive sources into `_sources/archive/`, update registries, and rebuild the file index. The ingest prompt is generated by the MCP service, requires OpenCode to load `wiki-ingest`, and restricts OpenCode to the uploaded file list; clients cannot pass arbitrary shell commands through this interface. Because enabling ingest gives the remote token write access to the vault through local OpenCode, keep it on a trusted network and rotate the token if it has been shared too broadly. To let a remote MCP client ask synthesized questions against the compiled wiki, enable the `query` tool group: diff --git a/obsidian_wiki/mcp_ingest.py b/obsidian_wiki/mcp_ingest.py index 27f991c..e6f41ec 100644 --- a/obsidian_wiki/mcp_ingest.py +++ b/obsidian_wiki/mcp_ingest.py @@ -10,6 +10,7 @@ import os import re import secrets import shutil +import sqlite3 import subprocess import time import uuid @@ -18,7 +19,7 @@ from pathlib import Path from threading import RLock from typing import Any -from obsidian_wiki.file_assets import clean_config_value +from obsidian_wiki.file_assets import clean_config_value, db_path_for_vault, registry_path_for_vault DEFAULT_INGEST_MAX_FILE_BYTES = 100 * 1024 * 1024 DEFAULT_INGEST_MAX_CHUNK_BYTES = 4 * 1024 * 1024 @@ -195,6 +196,63 @@ class IngestManager: self._write_json(self._upload_ticket_path(token_hash), meta) return {**self._public_upload_ticket(meta), "upload_token": token} + def validate_upload_plan( + self, + files: list[dict[str, Any]], + *, + project: str | None = None, + doc_type_hint: str | None = None, + ) -> dict[str, Any]: + if not isinstance(files, list): + raise IngestError("files must be a list") + global_errors: list[str] = [] + if not files: + global_errors.append("files must not be empty") + if len(files) > self.max_files_per_job: + global_errors.append(f"too many files for one ingest job: {len(files)} > {self.max_files_per_job}") + results = [self._validate_upload_file_plan(item) for item in files] + ok = not global_errors and all(not item["errors"] for item in results) + return { + "ok": ok, + "project": self._safe_optional_text(project), + "doc_type_hint": self._safe_optional_text(doc_type_hint), + "file_count": len(files), + "max_file_bytes": self.max_file_bytes, + "max_files_per_job": self.max_files_per_job, + "errors": global_errors, + "files": results, + "next_action": "upload_with_ticket" if ok else "fix_errors_before_upload", + } + + def preview_upload_metadata(self, upload_ids: list[str]) -> dict[str, Any]: + if not upload_ids: + raise IngestError("upload_ids must not be empty") + if len(upload_ids) > self.max_files_per_job: + raise IngestError(f"too many files for one ingest job: {len(upload_ids)} > {self.max_files_per_job}") + uploads: list[dict[str, Any]] = [] + with self._lock: + for upload_id in upload_ids: + meta = self._load_upload(upload_id) + if meta["state"] != "complete": + raise IngestError(f"upload is not complete: {upload_id}") + public = self._public_upload(meta) + duplicates = self._duplicates_for_sha(meta["sha256"]) + uploads.append( + { + **public, + "file_extension": Path(meta["file_name"]).suffix.lower(), + "duplicate_count": len(duplicates), + "duplicates": duplicates, + "warnings": ["exact sha256 duplicate already exists"] if duplicates else [], + } + ) + return { + "ok": True, + "upload_count": len(uploads), + "uploads": uploads, + "next_action": "start_ingest_job", + } + def consume_upload_ticket(self, upload_token: str) -> dict[str, Any]: with self._lock: token_hash = self._upload_ticket_hash(upload_token) @@ -597,6 +655,102 @@ class IngestManager: "start_job": bool(meta.get("start_job", True)), } + def _validate_upload_file_plan(self, item: dict[str, Any]) -> dict[str, Any]: + errors: list[str] = [] + warnings: list[str] = [] + raw_name = str(item.get("file_name") or "") + raw_sha = str(item.get("sha256") or "") + file_name = raw_name + digest = "" + size_bytes: int | None + try: + file_name = self._validate_file_name(raw_name) + except IngestError: + errors.append(f"unsafe file name: {raw_name}") + try: + size_bytes = int(item.get("size_bytes")) + except (TypeError, ValueError): + size_bytes = None + errors.append("size_bytes must be an integer") + if size_bytes is not None: + if size_bytes < 0: + errors.append("size_bytes must be non-negative") + if self.max_file_bytes > 0 and size_bytes > self.max_file_bytes: + errors.append(f"file exceeds max upload size: {size_bytes} > {self.max_file_bytes}") + try: + digest = self._normalize_sha256(raw_sha) + except IngestError: + errors.append("sha256 must be a 64-character hex digest, optionally prefixed with sha256:") + duplicates = self._duplicates_for_sha(digest) if digest else [] + if duplicates: + warnings.append("exact sha256 duplicate already exists") + status = "error" if errors else "warning" if warnings else "ok" + return { + "file_name": file_name or raw_name, + "size_bytes": size_bytes, + "sha256": ("sha256:" + digest) if digest else raw_sha, + "file_extension": Path(file_name or raw_name).suffix.lower(), + "status": status, + "errors": errors, + "warnings": warnings, + "duplicates": duplicates, + "duplicate_count": len(duplicates), + } + + def _duplicates_for_sha(self, digest: str) -> list[dict[str, Any]]: + normalized = self._normalize_sha256(digest) + duplicates: list[dict[str, Any]] = [] + seen: set[tuple[str, str]] = set() + registry = registry_path_for_vault(self.vault) + if registry.is_file(): + for raw in registry.read_text(encoding="utf-8").splitlines(): + if not raw.strip(): + continue + try: + record = json.loads(raw) + except json.JSONDecodeError: + continue + if str(record.get("source_id") or "").removeprefix("sha256:").lower() != normalized: + continue + entry = { + "source": "source_registry", + "source_id": "sha256:" + normalized, + "archive_path": record.get("archive_path"), + "original_name": record.get("original_name"), + } + key = (entry["source"], str(entry.get("archive_path") or entry.get("original_name") or "")) + if key not in seen: + duplicates.append(entry) + seen.add(key) + db_path = db_path_for_vault(self.vault) + if db_path.is_file(): + try: + conn = sqlite3.connect(str(db_path)) + conn.row_factory = sqlite3.Row + rows = conn.execute( + "SELECT file_id, archive_path, original_name FROM file_assets WHERE sha256 = ? LIMIT 20", + (normalized,), + ).fetchall() + except sqlite3.Error: + rows = [] + finally: + try: + conn.close() + except UnboundLocalError: + pass + for row in rows: + entry = { + "source": "file_index", + "file_id": row["file_id"], + "archive_path": row["archive_path"], + "original_name": row["original_name"], + } + key = (entry["source"], str(entry.get("file_id") or "")) + if key not in seen: + duplicates.append(entry) + seen.add(key) + return duplicates + def _public_job(self, job: dict[str, Any]) -> dict[str, Any]: return { "job_id": job["job_id"], diff --git a/obsidian_wiki/mcp_server.py b/obsidian_wiki/mcp_server.py index d1c63b5..acdfc10 100644 --- a/obsidian_wiki/mcp_server.py +++ b/obsidian_wiki/mcp_server.py @@ -6,13 +6,16 @@ import argparse import json import hmac import os +import sqlite3 import sys from email import policy from email.parser import BytesParser from fnmatch import fnmatch from pathlib import Path from typing import Any +from urllib.parse import urlsplit, urlunsplit +from obsidian_wiki import __version__ from obsidian_wiki.config import config_value, resolve_vault_path from obsidian_wiki.file_assets import clean_config_value, db_path_for_vault, registry_path_for_vault from obsidian_wiki.mcp_ingest import ( @@ -54,6 +57,8 @@ DEFAULT_HTTP_PATH = "/mcp" DEFAULT_INGEST_UPLOAD_PATH = "/mcp-ingest/uploads" DEFAULT_TOKEN_ENV = "OBSIDIAN_MCP_TOKEN" MCP_SERVER_NAME = "wiki-file-assets" +CAPABILITIES_SCHEMA_VERSION = 1 +TOOLS_VERSION = 1 TOOL_FILE_ASSETS = "file-assets" TOOL_INGEST = "ingest" TOOL_QUERY = "query" @@ -187,6 +192,7 @@ def create_server( host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT, streamable_http_path: str = DEFAULT_HTTP_PATH, + public_base_url: str | None = None, allowed_hosts: list[str] | None = None, allowed_origins: list[str] | None = None, enable_file_assets: bool = True, @@ -221,6 +227,8 @@ def create_server( vault = Path(clean_config_value(str(vault_path))).expanduser() db = _resolve_db_path(vault, db_path) project = Path(project_root).expanduser().resolve() if project_root is not None else Path.cwd().resolve() + public_base = _normalize_public_base_url(public_base_url or runtime_config_value("OBSIDIAN_MCP_PUBLIC_BASE_URL", "")) + public_upload_url = _upload_url_from_public_base(public_base, DEFAULT_INGEST_UPLOAD_PATH) ingest_runtime = ( Path(ingest_runtime_root).expanduser().resolve() if ingest_runtime_root is not None @@ -288,13 +296,17 @@ def create_server( ) return { "server_name": MCP_SERVER_NAME, + "server_version": __version__, + "capabilities_schema_version": CAPABILITIES_SCHEMA_VERSION, + "tools_version": TOOLS_VERSION, "enabled_tools": enabled, "recommended_first_steps": [ "Use ask_wiki for ordinary wiki, business facts, or hybrid questions when query is enabled.", "Use search_file_assets before reading source file resources.", "Use ingest_text_note for short text notes when ingest is enabled.", - "Use get_ingest_upload_instructions before HTTP file upload.", + "Use create_ingest_upload_ticket before remote HTTP file upload.", ], + "file_index": _file_index_capabilities(vault, db), "file_assets": { "enabled": enable_file_assets, "tools": ( @@ -339,17 +351,35 @@ def create_server( "supports_business_facts": enable_query, "supports_hybrid_questions": enable_query, "public_only_disables_business_facts": True, + "supports_structured_result": enable_query, "max_query_chars": query_max_chars, + "known_limitations": ( + [ + "answer_markdown is produced by the local wiki-query agent; citations and source flags are best-effort structured metadata.", + "Long-running queries may return a job_id and require polling get_wiki_query_result.", + ] + if enable_query + else [] + ), }, "ingest": { "enabled": enable_ingest, "preferred_text_tool": "ingest_text_note" if enable_ingest else None, "http_upload_path": DEFAULT_INGEST_UPLOAD_PATH if enable_ingest else None, + "http_upload_url": public_upload_url if enable_ingest else None, + "http_upload_url_warning": ( + None + if public_upload_url or not enable_ingest + else "Set OBSIDIAN_MCP_PUBLIC_BASE_URL so remote agents can receive a complete upload URL." + ), + "preferred_upload_auth": "upload_ticket" if enable_ingest else None, "tools": ( [ "get_ingest_upload_instructions", "ingest_text_note", "create_ingest_upload_ticket", + "validate_upload", + "preview_upload_metadata", "start_ingest_job", "get_ingest_job_status", "get_ingest_job_result", @@ -361,6 +391,17 @@ def create_server( ), "max_file_bytes": ingest_max_file_bytes, "max_files_per_job": ingest_max_files_per_job, + "last_ingest_at": _last_ingest_at(ingest_manager) if enable_ingest and ingest_manager is not None else None, + "queue": _ingest_queue_summary(ingest_manager) if enable_ingest and ingest_manager is not None else None, + "known_limitations": ( + [ + "Remote file uploads should use single-use upload tickets; bearer upload is retained for local/admin compatibility.", + "Upload preview detects exact sha256 duplicates only, not fuzzy near-duplicates.", + "Full dry-run ingest is not exposed; OpenCode runs only through the fixed wiki-ingest prompt.", + ] + if enable_ingest + else [] + ), }, "safety": { "arbitrary_shell": False, @@ -387,32 +428,38 @@ def create_server( @server.tool() def get_ingest_upload_instructions() -> dict[str, Any]: """Return preferred upload methods and limits for remote MCP ingest clients.""" + upload_url_warning = ( + None + if public_upload_url + else "Set OBSIDIAN_MCP_PUBLIC_BASE_URL so remote agents can receive a complete upload URL." + ) return { - "preferred_method": "use ingest_text_note for short text; POST multipart/form-data files to the HTTP upload endpoint for files", + "preferred_method": "use ingest_text_note for short text; for files, call create_ingest_upload_ticket and upload with X-Obsidian-Wiki-Upload-Token", + "preferred_upload_auth": "upload_ticket", "http_upload": { "method": "POST", "path": DEFAULT_INGEST_UPLOAD_PATH, + "url": public_upload_url, + "url_warning": upload_url_warning, "file_field": "files", - "auth": ( - "use the same Authorization: Bearer header as the MCP endpoint, " - "or call create_ingest_upload_ticket and send X-Obsidian-Wiki-Upload-Token" - ), + "auth": "call create_ingest_upload_ticket and send X-Obsidian-Wiki-Upload-Token", + "bearer_auth": "admin_or_local_only", "optional_fields": ["project", "doc_type_hint", "note", "start_job"], "start_job_default": True, "curl_example": ( - "curl -H 'Authorization: Bearer ' " + "curl -H 'X-Obsidian-Wiki-Upload-Token: ' " "-F 'files=@drawing.pdf' -F 'project=bjd' " "-F 'doc_type_hint=drawing' -F 'start_job=true' " - "http://:/mcp-ingest/uploads" + f"{public_upload_url or ''}" ), }, "upload_ticket_tool": { "name": "create_ingest_upload_ticket", "token_header": "X-Obsidian-Wiki-Upload-Token", - "best_for": "remote agents that can run curl but cannot read the main MCP bearer token", + "best_for": "default file upload path for remote agents", "curl_example": ( "curl -H 'X-Obsidian-Wiki-Upload-Token: ' " - "-F 'files=@drawing.pdf' http://:/mcp-ingest/uploads" + f"-F 'files=@drawing.pdf' {public_upload_url or ''}" ), }, "direct_text_tool": { @@ -429,6 +476,12 @@ def create_server( "staging": { "note": "uploads are temporary staging files outside the vault; wiki-ingest decides final archive/index placement", }, + "recommended_file_workflow": [ + "create_ingest_upload_ticket(start_job=false)", + "POST multipart files with X-Obsidian-Wiki-Upload-Token", + "preview_upload_metadata(upload_ids)", + "start_ingest_job(upload_ids) when preview looks correct", + ], } @server.tool() @@ -472,15 +525,35 @@ def create_server( return { **ticket, "upload_path": DEFAULT_INGEST_UPLOAD_PATH, + "upload_url": public_upload_url, + "upload_url_warning": ( + None + if public_upload_url + else "Set OBSIDIAN_MCP_PUBLIC_BASE_URL so remote agents can receive a complete upload URL." + ), "token_header": "X-Obsidian-Wiki-Upload-Token", "expires_seconds_max": MAX_UPLOAD_TICKET_SECONDS, "curl_example": ( "curl -H 'X-Obsidian-Wiki-Upload-Token: " + ticket["upload_token"] - + "' -F 'files=@/path/to/file.pdf' http://:/mcp-ingest/uploads" + + f"' -F 'files=@/path/to/file.pdf' {public_upload_url or ''}" ), } + @server.tool() + def validate_upload( + files: list[dict[str, Any]], + project: str | None = None, + doc_type_hint: str | None = None, + ) -> dict[str, Any]: + """Validate a planned remote upload without creating files or starting ingest.""" + return ingest_manager.validate_upload_plan(files, project=project, doc_type_hint=doc_type_hint) + + @server.tool() + def preview_upload_metadata(upload_ids: list[str]) -> dict[str, Any]: + """Preview completed upload metadata and exact duplicate warnings before ingest.""" + return ingest_manager.preview_upload_metadata(upload_ids) + @server.tool() def start_ingest_job(upload_ids: list[str], note: str | None = None) -> dict[str, Any]: """Start or queue a local OpenCode wiki-ingest job for completed uploads.""" @@ -1200,6 +1273,74 @@ def _resolve_db_path(vault: Path, raw_path: str | Path | None) -> Path: return db_path +def _normalize_public_base_url(raw_url: str | None) -> str: + value = clean_config_value(str(raw_url or "")).strip().rstrip("/") + if not value: + return "" + parsed = urlsplit(value) + if not parsed.scheme or not parsed.netloc: + return "" + return urlunsplit((parsed.scheme, parsed.netloc, parsed.path.rstrip("/"), "", "")) + + +def _upload_url_from_public_base(public_base_url: str, upload_path: str) -> str | None: + if not public_base_url: + return None + parsed = urlsplit(public_base_url) + return urlunsplit((parsed.scheme, parsed.netloc, upload_path, "", "")) + + +def _file_index_capabilities(vault: Path, db: Path) -> dict[str, Any]: + registry = registry_path_for_vault(vault) + record_count: int | None = None + if db.is_file(): + try: + conn = sqlite3.connect(str(db)) + record_count = int(conn.execute("SELECT COUNT(*) FROM file_assets").fetchone()[0]) + except sqlite3.Error: + record_count = None + finally: + try: + conn.close() + except UnboundLocalError: + pass + return { + "exists": db.is_file(), + "registry_exists": registry.is_file(), + "record_count": record_count, + "last_rebuild_at": _path_mtime_iso(db) if db.is_file() else None, + } + + +def _path_mtime_iso(path: Path) -> str | None: + try: + from datetime import datetime, timezone + + return datetime.fromtimestamp(path.stat().st_mtime, tz=timezone.utc).replace(microsecond=0).isoformat().replace("+00:00", "Z") + except OSError: + return None + + +def _ingest_queue_summary(manager: IngestManager) -> dict[str, int]: + jobs = manager._iter_jobs() + return { + "queued": sum(1 for job in jobs if job.get("state") == "queued"), + "running": sum(1 for job in jobs if job.get("state") == "running"), + "terminal": sum(1 for job in jobs if job.get("state") in {"succeeded", "failed", "cancelled"}), + } + + +def _last_ingest_at(manager: IngestManager) -> str | None: + timestamps = [] + for job in manager._iter_jobs(): + for key in ("finished_at", "started_at", "created_at"): + value = str(job.get(key) or "") + if value: + timestamps.append(value) + break + return max(timestamps) if timestamps else None + + def _as_mcp_results(rows: list[dict[str, Any]]) -> list[dict[str, Any]]: return [_as_mcp_result(row) for row in rows] diff --git a/scripts/manage_mcp_service.py b/scripts/manage_mcp_service.py index ed24827..acdce3f 100755 --- a/scripts/manage_mcp_service.py +++ b/scripts/manage_mcp_service.py @@ -16,6 +16,7 @@ import time from dataclasses import dataclass from pathlib import Path from typing import Sequence +from urllib.parse import urlsplit, urlunsplit REPO_ROOT = Path(__file__).resolve().parent.parent if str(REPO_ROOT) not in sys.path: @@ -31,6 +32,7 @@ DEFAULT_HOST = "0.0.0.0" DEFAULT_PORT = 8765 DEFAULT_PATH = "/mcp" DEFAULT_INGEST_UPLOAD_PATH = "/mcp-ingest/uploads" +PUBLIC_BASE_URL_ENV = "OBSIDIAN_MCP_PUBLIC_BASE_URL" DEFAULT_TOKEN_BYTES = 32 DEFAULT_UV_CACHE_DIR = "/tmp/uv-cache" DEFAULT_OPENCODE_PATH = "opencode" @@ -263,6 +265,7 @@ def write_token_env_file( host: str = DEFAULT_HOST, port: int = DEFAULT_PORT, path: str = DEFAULT_PATH, + public_base_url: str | None = None, enabled_tools: Sequence[str] | None = None, enable_ingest: bool | None = None, enable_query: bool | None = None, @@ -281,6 +284,8 @@ def write_token_env_file( env_file.write(f"OBSIDIAN_MCP_HOST={host}\n") env_file.write(f"OBSIDIAN_MCP_PORT={port}\n") env_file.write(f"OBSIDIAN_MCP_PATH={path}\n") + if public_base_url: + env_file.write(f"{PUBLIC_BASE_URL_ENV}={public_base_url.rstrip('/')}\n") env_file.write(f"OBSIDIAN_MCP_ENABLED_TOOLS={format_enabled_tools(tools)}\n") env_file.write(f"OBSIDIAN_MCP_ENABLE_INGEST={1 if TOOL_INGEST in tools else 0}\n") env_file.write(f"OBSIDIAN_MCP_INGEST_OPENCODE_MODEL={models.ingest_model}\n") @@ -325,7 +330,7 @@ def model_settings_from_config(values: dict[str, str]) -> OpenCodeModelSettings: ) -def saved_service_config(project_root: Path) -> tuple[str, str, int, str, tuple[str, ...], str, OpenCodeModelSettings]: +def saved_service_config(project_root: Path) -> tuple[str, str, int, str, str, tuple[str, ...], str, OpenCodeModelSettings]: paths = resolve_service_paths(project_root) values = read_service_env_file(paths.env_path) token = values.get("OBSIDIAN_MCP_TOKEN", "").strip() @@ -334,12 +339,13 @@ def saved_service_config(project_root: Path) -> tuple[str, str, int, str, tuple[ host = values.get("OBSIDIAN_MCP_HOST") or DEFAULT_HOST port = int(values.get("OBSIDIAN_MCP_PORT") or DEFAULT_PORT) path = values.get("OBSIDIAN_MCP_PATH") or DEFAULT_PATH + public_base_url = values.get(PUBLIC_BASE_URL_ENV, "").strip() enabled_tools = enabled_tools_from_config(values) opencode_path = ( values.get("OBSIDIAN_MCP_OPENCODE_PATH", "").strip() or values.get("OBSIDIAN_MCP_INGEST_OPENCODE_PATH", "").strip() ) - return token, host, port, path, enabled_tools, opencode_path, model_settings_from_config(values) + return token, host, port, path, public_base_url, enabled_tools, opencode_path, model_settings_from_config(values) def write_service_unit( @@ -389,17 +395,32 @@ def detect_remote_hosts() -> list[str]: return cleaned +def public_base_url_from_hosts(hosts: Sequence[str], *, port: int = DEFAULT_PORT, path: str = DEFAULT_PATH) -> str: + host = hosts[0] if hosts else "" + return f"http://{host}:{port}{path}" if host else "" + + +def upload_url_from_public_base(public_base_url: str, *, fallback_host: str, port: int = DEFAULT_PORT) -> str: + parsed = urlsplit(public_base_url) + if parsed.scheme and parsed.netloc: + return urlunsplit((parsed.scheme, parsed.netloc, DEFAULT_INGEST_UPLOAD_PATH, "", "")) + return f"http://{fallback_host}:{port}{DEFAULT_INGEST_UPLOAD_PATH}" + + def render_remote_config( *, hosts: Sequence[str], token: str, port: int = DEFAULT_PORT, path: str = DEFAULT_PATH, + public_base_url: str | None = None, config_path: Path | None = None, enabled_tools: Sequence[str] | None = None, ) -> str: host = hosts[0] if hosts else "" url = f"http://{host}:{port}{path}" + public_url = (public_base_url or url).rstrip("/") + upload_url = upload_url_from_public_base(public_url, fallback_host=host, port=port) alternatives = "\n".join(f"- http://{candidate}:{port}{path}" for candidate in hosts[1:]) alternatives_section = f"\nOther detected addresses:\n{alternatives}\n" if alternatives else "" config_section = f"\nConfig file:\n{config_path}\n" if config_path is not None else "" @@ -408,7 +429,6 @@ def render_remote_config( enabled_section = f"\nEnabled tool groups:\n{format_enabled_tools(enabled_tools) or 'none'}\n" ingest_upload_section = "" if enabled_tools is not None and TOOL_INGEST in normalize_enabled_tools(enabled_tools): - upload_url = f"http://{host}:{port}{DEFAULT_INGEST_UPLOAD_PATH}" upload_alternatives = "\n".join( f"- http://{candidate}:{port}{DEFAULT_INGEST_UPLOAD_PATH}" for candidate in hosts[1:] ) @@ -420,7 +440,8 @@ def render_remote_config( "Ingest file upload endpoint:\n" f"{upload_url}\n" f"{upload_alternatives_section}" - "Use the same Authorization header. POST multipart/form-data with one or more `files` fields.\n" + "Remote agents should upload with create_ingest_upload_ticket and X-Obsidian-Wiki-Upload-Token.\n" + "Bearer upload is for local/admin use. POST multipart/form-data with one or more `files` fields.\n" ) query_section = "" if enabled_tools is not None and TOOL_QUERY in normalize_enabled_tools(enabled_tools): @@ -462,12 +483,13 @@ def render_remote_config( def show_config(project_root: Path) -> str: paths = resolve_service_paths(project_root) - token, _host, port, path, enabled_tools, _opencode_path, _models = saved_service_config(project_root) + token, _host, port, path, public_base_url, enabled_tools, _opencode_path, _models = saved_service_config(project_root) return render_remote_config( hosts=detect_remote_hosts(), token=token, port=port, path=path, + public_base_url=public_base_url, config_path=paths.env_path, enabled_tools=enabled_tools, ) @@ -502,10 +524,13 @@ def new_token( enabled_tools = DEFAULT_ENABLED_TOOLS opencode_path = "" model_settings = OpenCodeModelSettings() + public_base_url = "" try: - _old_token, _old_host, _old_port, _old_path, enabled_tools, opencode_path, model_settings = saved_service_config(project_root) + _old_token, _old_host, _old_port, _old_path, public_base_url, enabled_tools, opencode_path, model_settings = saved_service_config(project_root) except RuntimeError: pass + hosts = detect_remote_hosts() + public_base_url = public_base_url or public_base_url_from_hosts(hosts, port=port, path=path) token = generate_token(token_bytes) write_token_env_file( paths.env_path, @@ -513,16 +538,18 @@ def new_token( host=host, port=port, path=path, + public_base_url=public_base_url, enabled_tools=enabled_tools, opencode_path=opencode_path, model_settings=model_settings, ) restart_service_if_installed(paths) return render_remote_config( - hosts=detect_remote_hosts(), + hosts=hosts, token=token, port=port, path=path, + public_base_url=public_base_url, config_path=paths.env_path, enabled_tools=enabled_tools, ) @@ -541,7 +568,7 @@ def start_service( ) -> str: paths = resolve_service_paths(project_root) try: - token, host, port, path, enabled_tools, opencode_path, model_settings = saved_service_config(project_root) + token, host, port, path, public_base_url, enabled_tools, opencode_path, model_settings = saved_service_config(project_root) except RuntimeError: if paths.env_path.exists(): raise @@ -549,16 +576,20 @@ def start_service( enabled_tools = DEFAULT_ENABLED_TOOLS opencode_path = "" model_settings = OpenCodeModelSettings() + public_base_url = "" enabled_tools = update_ingest_tool(enabled_tools, enable_ingest) enabled_tools = update_query_tool(enabled_tools, enable_query) if ({TOOL_INGEST, TOOL_QUERY} & set(enabled_tools)) and not opencode_path: opencode_path = resolve_opencode_path() + hosts = detect_remote_hosts() + public_base_url = public_base_url or public_base_url_from_hosts(hosts, port=port, path=path) write_token_env_file( paths.env_path, token, host=host, port=port, path=path, + public_base_url=public_base_url, enabled_tools=enabled_tools, opencode_path=opencode_path, model_settings=model_settings, @@ -574,10 +605,11 @@ def start_service( run_systemctl(["enable", SERVICE_NAME]) run_systemctl(["restart", SERVICE_NAME]) return render_remote_config( - hosts=detect_remote_hosts(), + hosts=hosts, token=token, port=port, path=path, + public_base_url=public_base_url, config_path=paths.env_path, enabled_tools=enabled_tools, ) @@ -606,15 +638,16 @@ def render_tool_status( return "\n".join(lines) + "\n" -def ensure_service_config(project_root: Path) -> tuple[ServicePaths, str, str, int, str, tuple[str, ...], str, OpenCodeModelSettings]: +def ensure_service_config(project_root: Path) -> tuple[ServicePaths, str, str, int, str, str, tuple[str, ...], str, OpenCodeModelSettings]: paths = resolve_service_paths(project_root) try: - token, host, port, path, enabled_tools, opencode_path, model_settings = saved_service_config(project_root) + token, host, port, path, public_base_url, enabled_tools, opencode_path, model_settings = saved_service_config(project_root) except RuntimeError: token = generate_token() host = DEFAULT_HOST port = DEFAULT_PORT path = DEFAULT_PATH + public_base_url = public_base_url_from_hosts(detect_remote_hosts(), port=port, path=path) enabled_tools = DEFAULT_ENABLED_TOOLS opencode_path = "" model_settings = OpenCodeModelSettings() @@ -624,20 +657,21 @@ def ensure_service_config(project_root: Path) -> tuple[ServicePaths, str, str, i host=host, port=port, path=path, + public_base_url=public_base_url, enabled_tools=enabled_tools, model_settings=model_settings, ) - return paths, token, host, port, path, enabled_tools, opencode_path, model_settings + return paths, token, host, port, path, public_base_url, enabled_tools, opencode_path, model_settings def list_tools(project_root: Path) -> str: - paths, _token, _host, _port, _path, enabled_tools, _opencode_path, _models = ensure_service_config(project_root) + paths, _token, _host, _port, _path, _public_base_url, enabled_tools, _opencode_path, _models = ensure_service_config(project_root) return render_tool_status(config_path=paths.env_path, enabled_tools=enabled_tools) def set_tool_enabled(project_root: Path, tool: str, enabled: bool) -> str: tool_name = normalize_tool_name(tool) - paths, token, host, port, path, enabled_tools, opencode_path, model_settings = ensure_service_config(project_root) + paths, token, host, port, path, public_base_url, enabled_tools, opencode_path, model_settings = ensure_service_config(project_root) current = set(enabled_tools) if enabled: current.add(tool_name) @@ -652,6 +686,7 @@ def set_tool_enabled(project_root: Path, tool: str, enabled: bool) -> str: host=host, port=port, path=path, + public_base_url=public_base_url, enabled_tools=updated_tools, opencode_path=opencode_path, model_settings=model_settings, diff --git a/tests/test_mcp_ingest.py b/tests/test_mcp_ingest.py index 9acc952..f678e03 100644 --- a/tests/test_mcp_ingest.py +++ b/tests/test_mcp_ingest.py @@ -236,6 +236,81 @@ class McpIngestTests(unittest.TestCase): with self.assertRaises(IngestError): self.manager.create_completed_upload(file_name="too-large.pdf", payload=b"x" * 101) + def test_validate_upload_plan_reports_limits_and_exact_duplicates(self): + duplicate_digest = self._sha256(b"already archived") + registry = self.vault / "_meta" / "source-registry.jsonl" + registry.parent.mkdir(parents=True) + registry.write_text( + json.dumps( + { + "record_type": "source_archive", + "source_id": "sha256:" + duplicate_digest, + "archive_path": "_sources/archive/existing.pdf", + "original_name": "existing.pdf", + } + ) + + "\n", + encoding="utf-8", + ) + + result = self.manager.validate_upload_plan( + [ + {"file_name": "existing.pdf", "size_bytes": 12, "sha256": duplicate_digest}, + {"file_name": "too-large.pdf", "size_bytes": 101, "sha256": "b" * 64}, + {"file_name": "../secret.pdf", "size_bytes": 10, "sha256": "c" * 64}, + {"file_name": "bad-sha.pdf", "size_bytes": 10, "sha256": "not-a-sha"}, + ], + project="bjd", + doc_type_hint="drawing", + ) + + self.assertFalse(result["ok"]) + self.assertEqual(result["project"], "bjd") + self.assertEqual(result["doc_type_hint"], "drawing") + self.assertEqual(result["max_file_bytes"], 100) + self.assertEqual(result["max_files_per_job"], 10) + self.assertEqual(result["files"][0]["status"], "warning") + self.assertTrue(result["files"][0]["duplicates"]) + self.assertEqual(result["files"][1]["status"], "error") + self.assertIn("exceeds max upload size", result["files"][1]["errors"][0]) + self.assertIn("unsafe file name", result["files"][2]["errors"][0]) + self.assertIn("sha256", result["files"][3]["errors"][0]) + + def test_preview_upload_metadata_reports_duplicate_and_requires_completed_uploads(self): + payload = b"duplicate payload" + digest = self._sha256(payload) + registry = self.vault / "_meta" / "source-registry.jsonl" + registry.parent.mkdir(parents=True) + registry.write_text( + json.dumps( + { + "record_type": "source_archive", + "source_id": "sha256:" + digest, + "archive_path": "_sources/archive/duplicate.pdf", + "original_name": "duplicate.pdf", + } + ) + + "\n", + encoding="utf-8", + ) + complete = self.manager.create_completed_upload( + "duplicate.pdf", + payload, + project="bjd", + doc_type_hint="drawing", + ) + incomplete = self.manager.create_upload("pending.pdf", 7, self._sha256(b"pending")) + + preview = self.manager.preview_upload_metadata([complete["upload_id"]]) + + self.assertTrue(preview["ok"]) + self.assertEqual(preview["next_action"], "start_ingest_job") + self.assertEqual(preview["uploads"][0]["file_extension"], ".pdf") + self.assertTrue(preview["uploads"][0]["duplicates"]) + self.assertEqual(preview["uploads"][0]["duplicate_count"], 1) + with self.assertRaises(IngestError): + self.manager.preview_upload_metadata([incomplete["upload_id"]]) + def test_start_ingest_job_constructs_restricted_opencode_command(self): payload = b"source data" upload = self._complete_upload("source.pdf", payload) diff --git a/tests/test_mcp_server_remote.py b/tests/test_mcp_server_remote.py index b759844..539ae52 100644 --- a/tests/test_mcp_server_remote.py +++ b/tests/test_mcp_server_remote.py @@ -597,6 +597,7 @@ class RemoteMcpServerTests(unittest.TestCase): project_root=root / "project", ingest_runtime_root=root / "ingest-runtime", query_runtime_root=root / "query-runtime", + public_base_url="https://wiki.example.test/mcp", ingest_manager=IngestManager( vault_path=root / "vault", project_root=root / "project", @@ -618,13 +619,28 @@ class RemoteMcpServerTests(unittest.TestCase): capabilities = FakeMCP.tools["get_mcp_capabilities"]() self.assertEqual(capabilities["server_name"], "wiki-file-assets") + self.assertIn("server_version", capabilities) + self.assertEqual(capabilities["capabilities_schema_version"], 1) + self.assertEqual(capabilities["tools_version"], 1) self.assertEqual(capabilities["enabled_tools"], ["file-assets", "ingest", "query"]) + self.assertIn("file_index", capabilities) + self.assertIn("record_count", capabilities["file_index"]) + self.assertIn("last_rebuild_at", capabilities["file_index"]) self.assertTrue(capabilities["query"]["enabled"]) self.assertEqual(capabilities["query"]["preferred_tool"], "ask_wiki") self.assertIn("ask_wiki", capabilities["query"]["tools"]) + self.assertTrue(capabilities["query"]["supports_structured_result"]) + self.assertTrue(capabilities["query"]["known_limitations"]) self.assertTrue(capabilities["ingest"]["enabled"]) + self.assertEqual(capabilities["ingest"]["preferred_upload_auth"], "upload_ticket") self.assertEqual(capabilities["ingest"]["http_upload_path"], "/mcp-ingest/uploads") + self.assertEqual(capabilities["ingest"]["http_upload_url"], "https://wiki.example.test/mcp-ingest/uploads") self.assertEqual(capabilities["ingest"]["preferred_text_tool"], "ingest_text_note") + self.assertIn("validate_upload", capabilities["ingest"]["tools"]) + self.assertIn("preview_upload_metadata", capabilities["ingest"]["tools"]) + self.assertIn("queue", capabilities["ingest"]) + self.assertIn("last_ingest_at", capabilities["ingest"]) + self.assertTrue(capabilities["ingest"]["known_limitations"]) self.assertIn("search_file_assets", capabilities["file_assets"]["tools"]) self.assertNotIn("vault_path", json.dumps(capabilities)) self.assertNotIn("runtime", json.dumps(capabilities).lower()) @@ -753,6 +769,7 @@ class RemoteMcpServerTests(unittest.TestCase): enable_ingest=True, project_root=root / "project", ingest_runtime_root=root / "runtime", + public_base_url="https://wiki.example.test/mcp", ) ticket = FakeMCP.tools["create_ingest_upload_ticket"]( expires_seconds=60, @@ -766,11 +783,80 @@ class RemoteMcpServerTests(unittest.TestCase): self.assertIn("upload_token", ticket) self.assertEqual(ticket["token_header"], "X-Obsidian-Wiki-Upload-Token") self.assertEqual(ticket["upload_path"], "/mcp-ingest/uploads") + self.assertEqual(ticket["upload_url"], "https://wiki.example.test/mcp-ingest/uploads") + self.assertIn("X-Obsidian-Wiki-Upload-Token", ticket["curl_example"]) + self.assertNotIn("Authorization: Bearer", ticket["curl_example"]) self.assertEqual(ticket["max_files"], 1) self.assertFalse(ticket["start_job"]) - self.assertIn("ingest_text_note", instructions["preferred_method"]) + self.assertEqual(instructions["preferred_upload_auth"], "upload_ticket") + self.assertEqual(instructions["http_upload"]["url"], "https://wiki.example.test/mcp-ingest/uploads") + self.assertIn("create_ingest_upload_ticket", instructions["preferred_method"]) + self.assertEqual(instructions["http_upload"]["bearer_auth"], "admin_or_local_only") + self.assertNotIn("Authorization: Bearer", instructions["http_upload"]["curl_example"]) self.assertEqual(instructions["direct_text_tool"]["name"], "ingest_text_note") + def test_ingest_tools_validate_upload_and_preview_metadata(self): + class FakeMCP: + tools = {} + + def __init__(self, _name, **_kwargs): + pass + + def tool(self): + def decorator(func): + self.tools[func.__name__] = func + return func + + return decorator + + def resource(self, *_args, **_kwargs): + def decorator(func): + return func + + return decorator + + def prompt(self, *_args, **_kwargs): + def decorator(func): + return func + + return decorator + + with ( + tempfile.TemporaryDirectory() as tmp, + patch.dict("sys.modules", {"mcp.server.fastmcp": type("Mod", (), {"FastMCP": FakeMCP})()}), + ): + root = Path(tmp) + manager = IngestManager( + vault_path=root / "vault", + project_root=root / "project", + runtime_root=root / "runtime", + max_file_bytes=20, + ) + mcp_server.create_server( + vault_path=root / "vault", + enable_ingest=True, + project_root=root / "project", + ingest_runtime_root=root / "runtime", + ingest_manager=manager, + ) + validate = FakeMCP.tools["validate_upload"]( + files=[ + {"file_name": "ok.pdf", "size_bytes": 2, "sha256": "a" * 64}, + {"file_name": "large.pdf", "size_bytes": 21, "sha256": "b" * 64}, + ], + project="bjd", + doc_type_hint="drawing", + ) + upload = manager.create_completed_upload("ok.pdf", b"ok", project="bjd", doc_type_hint="drawing") + preview = FakeMCP.tools["preview_upload_metadata"]([upload["upload_id"]]) + + self.assertFalse(validate["ok"]) + self.assertEqual(validate["files"][0]["status"], "ok") + self.assertEqual(validate["files"][1]["status"], "error") + self.assertTrue(preview["ok"]) + self.assertEqual(preview["uploads"][0]["file_name"], "ok.pdf") + self.assertEqual(preview["next_action"], "start_ingest_job") + def test_ingest_text_note_tool_creates_completed_markdown_upload(self): class FakeMCP: tools = {} diff --git a/tests/test_mcp_service.py b/tests/test_mcp_service.py index 0ae312f..60f3f68 100644 --- a/tests/test_mcp_service.py +++ b/tests/test_mcp_service.py @@ -62,6 +62,7 @@ class McpServiceConfigTests(unittest.TestCase): host="0.0.0.0", port=8765, path="/mcp", + public_base_url="http://192.168.1.10:8765/mcp", enable_ingest=True, ) @@ -70,6 +71,7 @@ class McpServiceConfigTests(unittest.TestCase): self.assertIn("OBSIDIAN_MCP_HOST=0.0.0.0\n", content) self.assertIn("OBSIDIAN_MCP_PORT=8765\n", content) self.assertIn("OBSIDIAN_MCP_PATH=/mcp\n", content) + self.assertIn("OBSIDIAN_MCP_PUBLIC_BASE_URL=http://192.168.1.10:8765/mcp\n", content) self.assertIn("OBSIDIAN_MCP_ENABLED_TOOLS=file-assets,ingest\n", content) self.assertIn("OBSIDIAN_MCP_ENABLE_INGEST=1\n", content) self.assertIn("OBSIDIAN_MCP_INGEST_OPENCODE_MODEL=openai/gpt-5.5\n", content) @@ -125,6 +127,21 @@ class McpServiceConfigTests(unittest.TestCase): self.assertIn("http://192.168.1.10:8765/mcp", output) self.assertIn("http://192.168.1.10:8765/mcp-ingest/uploads", output) self.assertIn("http://10.0.0.2:8765/mcp-ingest/uploads", output) + self.assertIn("Remote agents should upload with create_ingest_upload_ticket", output) + self.assertIn("Bearer upload is for local/admin use", output) + + def test_render_remote_config_uses_public_base_url_for_upload_endpoint(self): + output = manage_mcp_service.render_remote_config( + hosts=["192.168.1.10"], + token="test-token", + port=8765, + path="/mcp", + public_base_url="https://wiki.example.test/custom-mcp", + enabled_tools=("file-assets", "ingest"), + ) + + self.assertIn("https://wiki.example.test/mcp-ingest/uploads", output) + self.assertNotIn("http://192.168.1.10:8765/mcp-ingest/uploads", output) def test_new_token_writes_token_without_touching_systemd_when_unit_missing(self): with tempfile.TemporaryDirectory() as tmp: @@ -236,6 +253,7 @@ class McpServiceConfigTests(unittest.TestCase): self.assertIn("OBSIDIAN_MCP_TOKEN=created-token\n", env_content) self.assertIn("OBSIDIAN_MCP_PORT=9876\n", env_content) self.assertIn("OBSIDIAN_MCP_PATH=/wiki-mcp\n", env_content) + self.assertIn("OBSIDIAN_MCP_PUBLIC_BASE_URL=http://10.0.0.4:9876/wiki-mcp\n", env_content) self.assertIn("OBSIDIAN_MCP_ENABLED_TOOLS=file-assets,ingest\n", env_content) self.assertIn("OBSIDIAN_MCP_ENABLE_INGEST=1\n", env_content) self.assertIn("OBSIDIAN_MCP_INGEST_OPENCODE_PATH=/home/nac/.opencode/bin/opencode\n", env_content) @@ -279,7 +297,8 @@ class McpServiceConfigTests(unittest.TestCase): + "OBSIDIAN_MCP_INGEST_OPENCODE_MODEL=openai/custom-strong\n" + "OBSIDIAN_MCP_INGEST_OPENCODE_VARIANT=max\n" + "OBSIDIAN_MCP_QUERY_OPENCODE_MODEL=openai/custom-fast\n" - + "OBSIDIAN_MCP_QUERY_OPENCODE_VARIANT=low\n", + + "OBSIDIAN_MCP_QUERY_OPENCODE_VARIANT=low\n" + + "OBSIDIAN_MCP_PUBLIC_BASE_URL=https://wiki.example.test/mcp\n", encoding="utf-8", ) @@ -301,6 +320,7 @@ class McpServiceConfigTests(unittest.TestCase): self.assertIn("OBSIDIAN_MCP_INGEST_OPENCODE_VARIANT=max\n", env_content) self.assertIn("OBSIDIAN_MCP_QUERY_OPENCODE_MODEL=openai/custom-fast\n", env_content) self.assertIn("OBSIDIAN_MCP_QUERY_OPENCODE_VARIANT=low\n", env_content) + self.assertIn("OBSIDIAN_MCP_PUBLIC_BASE_URL=https://wiki.example.test/mcp\n", env_content) def test_start_service_can_enable_query_without_ingest(self): with tempfile.TemporaryDirectory() as tmp: