fix: search review follow-ups (503 only when dir unreadable)

clean6378-max-it · clean6378-max-it · commit 781e1f74477e · 2026-07-07T17:13:50.000+08:00
Fresh Claude installs were getting 503 on search when ~/.claude/projects
did not exist yet. 503 now means the path is there but listdir failed.
CI smoke test uses create_app(testing=True). Search page nags on empty
query and highlights snippets by code point.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -38,7 +38,7 @@ jobs:
         run: |
           python - <<'PY'
           from app import create_app
-          app = create_app()
+          app = create_app(testing=True)
           client = app.test_client()
           assert client.get("/").status_code == 200
           PY
diff --git a/api/search.py b/api/search.py
@@ -77,14 +77,15 @@ def _rank_search_hits(results: list[SearchHitDict]) -> list[SearchHitDict]:
     )
 
 
-def _projects_dir_available(projects_dir: str) -> bool:
+def _projects_dir_inaccessible(projects_dir: str) -> bool:
+    """True when the projects path exists but cannot be listed (503 case)."""
     try:
         if not os.path.isdir(projects_dir):
             return False
         os.listdir(projects_dir)
-        return True
-    except OSError:
         return False
+    except OSError:
+        return True
 
 
 def _message_searchable_text(msg: MessageDict) -> str:
@@ -120,7 +121,7 @@ def _index_hit_excluded(
             file_path,
             exc_info=True,
         )
-        return False
+        return True
     return is_session_excluded(rules, session, project_name)
 
 
@@ -276,7 +277,7 @@ def search() -> FlaskReturn:
     )
 
     base = current_app.config.get("CLAUDE_PROJECTS_DIR") or get_claude_projects_dir()
-    if not _projects_dir_available(base):
+    if _projects_dir_inaccessible(base):
         return error_response(
             ErrorCode.SEARCH_PROJECTS_UNAVAILABLE,
             "Claude projects directory is not accessible",
diff --git a/app.py b/app.py
@@ -92,8 +92,11 @@ def validate_startup_cli(args: argparse.Namespace) -> None:
 def create_app(
     base_dir: str | None = None,
     exclusion_rules_path: str | None = None,
+    *,
+    testing: bool = False,
 ) -> Flask:
     app = Flask(__name__)
+    app.config["TESTING"] = testing
     app.config["CLAUDE_PROJECTS_DIR"] = base_dir
 
     resolved = resolve_exclusion_rules_path(exclusion_rules_path)
@@ -106,7 +109,7 @@ def create_app(
     app.register_blueprint(export_bp)
     app.register_blueprint(schema_report_bp)
 
-    if not app.config.get("TESTING"):
+    if not testing:
         try:
             from utils.search_index import start_search_index_background
 
diff --git a/docs/api-reference.md b/docs/api-reference.md
@@ -59,7 +59,7 @@ Extra fields may appear for specific codes (for example `since` on invalid bulk-
 | `SEARCH_EMPTY_QUERY` | 400 | `GET /api/search` | Query param `q` is empty or whitespace |
 | `SEARCH_QUERY_TOO_LONG` | 400 | `GET /api/search` | Query param `q` exceeds 500 characters |
 | `SEARCH_INVALID_SINCE_DAYS` | 400 | `GET /api/search` | Query param `since_days` is not a positive integer |
-| `SEARCH_PROJECTS_UNAVAILABLE` | 503 | `GET /api/search` | Claude projects directory is missing or not readable |
+| `SEARCH_PROJECTS_UNAVAILABLE` | 503 | `GET /api/search` | Claude projects directory exists but is not readable |
 | `SEARCH_INDEX_UNAVAILABLE` | 503 | `GET /api/search` | FTS index is locked during rebuild |
 | `INVALID_PATH` | 400 | Session, stats, export session | Path traversal or rejected URL segment |
 | `SESSION_NOT_FOUND` | 404 | Session, stats, export session | File missing on disk or session excluded by rules |
@@ -319,7 +319,7 @@ By default, messages older than 30 days are excluded. Sessions without a parseab
 | 400 | `SEARCH_QUERY_TOO_LONG` | `q` exceeds 500 characters |
 | 400 | `SEARCH_INVALID_LIMIT` | `limit` not a positive integer (e.g. `abc`, `0`, `1.5`) |
 | 400 | `SEARCH_INVALID_SINCE_DAYS` | `since_days` not a positive integer |
-| 503 | `SEARCH_PROJECTS_UNAVAILABLE` | Projects directory missing or not readable |
+| 503 | `SEARCH_PROJECTS_UNAVAILABLE` | Projects directory exists but is not readable |
 | 503 | `SEARCH_INDEX_UNAVAILABLE` | FTS index locked during rebuild |
 | 500 | `INTERNAL_ERROR` | Unexpected server failure |
 
diff --git a/static/js/search.js b/static/js/search.js
@@ -13,17 +13,24 @@ let lastSearchRequestId = 0;
 export function highlightSnippet(snippet, query) {
     if (!snippet) return '';
     if (!query) return esc(snippet);
-    const hay = snippet.toLowerCase();
-    const needle = query.toLowerCase();
-    const idx = hay.indexOf(needle);
-    if (idx < 0) return esc(snippet);
-    return (
-        esc(snippet.slice(0, idx))
-        + '<mark>'
-        + esc(snippet.slice(idx, idx + query.length))
-        + '</mark>'
-        + esc(snippet.slice(idx + query.length))
-    );
+    const chars = [...snippet];
+    const needle = [...query].map((ch) => ch.toLowerCase());
+    const hay = chars.map((ch) => ch.toLowerCase());
+    for (let i = 0; i <= hay.length - needle.length; i += 1) {
+        let matched = true;
+        for (let j = 0; j < needle.length; j += 1) {
+            if (hay[i + j] !== needle[j]) {
+                matched = false;
+                break;
+            }
+        }
+        if (!matched) continue;
+        const before = chars.slice(0, i).join('');
+        const match = chars.slice(i, i + needle.length).join('');
+        const after = chars.slice(i + needle.length).join('');
+        return esc(before) + '<mark>' + esc(match) + '</mark>' + esc(after);
+    }
+    return esc(snippet);
 }
 
 export function showSearchPage() {
@@ -77,11 +84,14 @@ export async function doSearch() {
     const localRequestId = ++lastSearchRequestId;
     const input = document.getElementById('search-input');
     if (!input) { showSearchPage(); return; }
+    const container = document.getElementById('search-results');
     const query = input.value.trim();
-    if (!query) return;
+    if (!query) {
+        container.innerHTML = '<p class="search-error">Enter a search term.</p>';
+        return;
+    }
 
     const allHistory = document.getElementById('search-all-history')?.checked;
-    const container = document.getElementById('search-results');
     container.innerHTML = '<div class="search-loading">Searching...</div>';
 
     const params = new URLSearchParams({
diff --git a/static/js/search.test.js b/static/js/search.test.js
@@ -56,6 +56,16 @@ describe('search page', () => {
         expect(document.getElementById('content').innerHTML).toContain('30 days');
     });
 
+    it('doSearch prompts when query is empty', async () => {
+        showSearchPage();
+        document.getElementById('search-input').value = '   ';
+
+        await doSearch();
+
+        expect(document.getElementById('search-results').innerHTML).toContain('Enter a search term');
+        expect(fetch).not.toHaveBeenCalled();
+    });
+
     it('doSearch renders results with highlighted snippet text', async () => {
         showSearchPage();
         fetch.mockResolvedValue({
diff --git a/tests/benchmarks/conftest.py b/tests/benchmarks/conftest.py
@@ -119,6 +119,5 @@ def export_corpus(tmp_path: Path, request: pytest.FixtureRequest) -> Path:
 def bench_client_search_corpus(tmp_path: Path):
     """Flask test client backed by a 50-session synthetic project tree."""
     seed_search_corpus(tmp_path)
-    app = create_app(base_dir=str(tmp_path))
-    app.config["TESTING"] = True
+    app = create_app(base_dir=str(tmp_path), testing=True)
     return app.test_client()
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -41,8 +41,7 @@ def _make_test_client(tmp_path, session_files: Mapping[str, str] | None = None):
         project_dir.mkdir(parents=True)
         for dest_name, fixture_name in session_files.items():
             shutil.copy(FIXTURES / fixture_name, project_dir / dest_name)
-    app = create_app(base_dir=str(tmp_path))
-    app.config["TESTING"] = True
+    app = create_app(base_dir=str(tmp_path), testing=True)
     return app.test_client()
 
 
diff --git a/tests/test_api_routes.py b/tests/test_api_routes.py
@@ -39,7 +39,7 @@ def test_session_stats_excluded_session_returns_404(tmp_path, export_state_file)
     rules_path = tmp_path / "exclusion-rules.txt"
     rules_path.write_text("integration fixture\n", encoding="utf-8")
 
-    app = create_app(base_dir=str(tmp_path), exclusion_rules_path=str(rules_path))
+    app = create_app(base_dir=str(tmp_path), exclusion_rules_path=str(rules_path), testing=True)
     app.config["TESTING"] = True
     excluded_client = app.test_client()
 
diff --git a/tests/test_search.py b/tests/test_search.py
@@ -11,6 +11,7 @@
 from pathlib import Path
 from unittest.mock import patch
 
+from api.search import _index_hit_excluded
 from app import create_app
 from tests.conftest import FIXTURES, assert_error_response
 from utils.search_index import build_search_index, reset_background_for_tests
@@ -105,12 +106,18 @@ def test_invalid_since_days_zero(client_single):
 
 
 def test_projects_unavailable(client_single, monkeypatch):
-    monkeypatch.setattr("api.search._projects_dir_available", lambda _path: False)
+    monkeypatch.setattr("api.search._projects_dir_inaccessible", lambda _path: True)
     resp = client_single.get("/api/search?q=Hello")
     assert resp.status_code == 503
     assert_error_response(resp, expected_code="SEARCH_PROJECTS_UNAVAILABLE")
 
 
+def test_missing_projects_dir_is_not_unavailable(client_single, monkeypatch):
+    monkeypatch.setattr("api.search._projects_dir_inaccessible", lambda _path: False)
+    resp = client_single.get("/api/search?q=Hello")
+    assert resp.status_code == 200
+
+
 def test_index_unavailable_when_locked(tmp_path, monkeypatch):
     recent_ts = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%SZ")
     client = _seed_indexed_client(tmp_path, monkeypatch, timestamp=recent_ts)
@@ -154,8 +161,7 @@ def _seed_indexed_client(tmp_path, monkeypatch, *, timestamp: str):
     with patches[0]:
         assert build_search_index(str(tmp_path / "projects"), [], force=True) is True
 
-    app = create_app(base_dir=str(tmp_path / "projects"))
-    app.config["TESTING"] = True
+    app = create_app(base_dir=str(tmp_path / "projects"), testing=True)
     return app.test_client()
 
 
@@ -185,6 +191,36 @@ def test_search_uses_index_when_usable(tmp_path, monkeypatch):
     assert len(resp.get_json()) >= 1
 
 
+def test_index_hit_excluded_fails_closed_when_session_unreadable():
+    rules = [[("word", "secret")]]
+    with (
+        patch("api.search.get_summary", return_value=None),
+        patch("api.search.get_cached_session", side_effect=OSError("unreadable")),
+    ):
+        assert _index_hit_excluded(
+            rules,
+            "rules-fp",
+            project_name="demo",
+            file_path="/tmp/session.jsonl",
+            mtime=1.0,
+        )
+
+
+def test_search_falls_back_on_tokenless_query(tmp_path, monkeypatch):
+    recent_ts = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%SZ")
+    client = _seed_indexed_client(tmp_path, monkeypatch, timestamp=recent_ts)
+    seen: list[bool] = []
+
+    def _fake_live_scan(*_args, **_kwargs):
+        seen.append(True)
+        return []
+
+    with patch("api.search._search_live_scan", side_effect=_fake_live_scan):
+        resp = client.get("/api/search?q=!!!")
+    assert resp.status_code == 200
+    assert seen == [True]
+
+
 def test_search_falls_back_when_index_query_fails(tmp_path, monkeypatch):
     recent_ts = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%SZ")
     client = _seed_indexed_client(tmp_path, monkeypatch, timestamp=recent_ts)
diff --git a/tests/test_search_index.py b/tests/test_search_index.py
@@ -350,6 +350,13 @@ def close(self) -> None:
             assert result["query_ok"] is False
             assert result["hits"] == []
 
+    def test_tokenless_query_is_not_index_ok(self, indexed_tree):
+        patches = _index_patches(indexed_tree["cache_root"])
+        with patches[0]:
+            result = query_index_hits("!!!", since_ms=None, max_results=5)
+            assert result["query_ok"] is False
+            assert result["hits"] == []
+
 
 class TestBypassAndBackground:
     def test_no_search_index_env_disables_index(self, indexed_tree, monkeypatch):
diff --git a/utils/search_index.py b/utils/search_index.py
@@ -14,6 +14,7 @@
 import os
 import re
 import sqlite3
+import sys
 import threading
 import time
 import uuid
@@ -55,6 +56,7 @@
 _index_lock = threading.Lock()
 _index_build_lock = threading.Lock()
 _background_started = False
+_background_lock_fd: int | None = None
 _usability_cache: dict[tuple[str, str], tuple[bool, float]] = {}
 _usability_cache_lock = threading.Lock()
 _USABILITY_CACHE_TTL_SECONDS = 30.0
@@ -557,6 +559,31 @@ def ensure_search_index(projects_dir: str, rules: list[Any]) -> None:
             build_search_index(projects_dir, rules)
 
 
+def _try_acquire_cross_process_background_lock() -> bool:
+    """Return True when this process should own the background index worker."""
+    global _background_lock_fd
+    if _background_lock_fd is not None:
+        return True
+    root = cache_dir()
+    root.mkdir(parents=True, exist_ok=True)
+    lock_path = root / "search_index.background.lock"
+    fd = os.open(str(lock_path), os.O_CREAT | os.O_RDWR)
+    try:
+        if sys.platform == "win32":
+            import msvcrt
+
+            msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)
+        else:
+            import fcntl
+
+            fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
+    except (OSError, BlockingIOError):
+        os.close(fd)
+        return False
+    _background_lock_fd = fd
+    return True
+
+
 def start_search_index_background(
     projects_dir: str,
     rules: list[Any],
@@ -570,6 +597,8 @@ def start_search_index_background(
     with _index_lock:
         if _background_started:
             return
+        if not _try_acquire_cross_process_background_lock():
+            return
         _background_started = True
 
     def _worker() -> None:
@@ -617,13 +646,6 @@ def query_index_hits(
     sql_offset: int = 0,
 ) -> IndexQueryResult:
     """Return message hits from the FTS index (pre-exclusion, pre-snippet)."""
-    empty: IndexQueryResult = {
-        "hits": [],
-        "query_ok": True,
-        "sql_rows_fetched": 0,
-        "sql_exhausted": True,
-        "index_locked": False,
-    }
     if not query_lower or not index_search_enabled():
         return {
             "hits": [],
@@ -635,7 +657,13 @@ def query_index_hits(
 
     fts_q = _fts_match_query(query_lower)
     if not fts_q:
-        return empty
+        return {
+            "hits": [],
+            "query_ok": False,
+            "sql_rows_fetched": 0,
+            "sql_exhausted": True,
+            "index_locked": False,
+        }
 
     sql_limit = max(max_results, _FTS_BATCH_SIZE)
     with _index_db_conn(readonly=True) as conn:
@@ -713,7 +741,13 @@ def query_index_hits(
 
 def reset_background_for_tests() -> None:
     """Allow tests to restart the background worker."""
-    global _background_started
+    global _background_started, _background_lock_fd
     with _index_lock:
         _background_started = False
+        if _background_lock_fd is not None:
+            try:
+                os.close(_background_lock_fd)
+            except OSError:
+                pass
+            _background_lock_fd = None
     _clear_usability_cache()
