fix: vec index usable across threads — phone-capture approve crashed

[wimaan3]

Fixes #41.

What broke

grandplan gui --serve with phone captures: the first approve crashed with
sqlite3.ProgrammingError: SQLite objects created in a thread can only be used in that same thread,
and every subsequent capture in the session then failed during analysis. The capture pipeline was
effectively down (notes were never lost — the JSONL event log is the store; the vec db is a
rebuildable index that resyncs on reopen).

Root cause

gui.py builds the repository — and VecIndexedRepository's sqlite connection (#35) — on the
main thread; the coordinator's grandplan-capture worker thread performs every
assess/commit. Python's sqlite3 refuses cross-thread connection use by default. It only surfaced at
approve because a fresh index has no recorded dim, so most_similar silently fell back to brute
force until the first real DB write.

Fix

• sqlite3.connect(..., check_same_thread=False) + one threading.RLock serializing every DB
  access and the _dim/_degraded state it guards (re-entrant: _sync → _index nests).
• Module docstring documents the thread-shared contract.

Test plan

• New regression test test_usable_from_threads_other_than_the_creating_one — creates the
  index on one thread, drives add_note/most_similar from another; failed with the exact
  production error before the fix (RED), passes after (GREEN).
• Full local gate: ruff format/check, mypy, bandit clean; 1227 tests pass, 96% coverage.
• Code review (agent): APPROVE, no blocking findings; two LOW follow-ups noted (a true
  concurrent-race test; _sync holds the lock across inner-repo calls — safe today, both fine as
  future work).

[wimaan3]

Automated review (Claude code-review sub-agent)

Verdict: request changes (one HIGH scope gap; the core cross-thread fix itself is sound)

Ran python -m pytest tests/adapters/test_vec_index.py -q on 4b6109b (detached checkout of fix/vec-index-thread-affinity): 8 passed, including the new test_usable_from_threads_other_than_the_creating_one.

[HIGH] The fix only covers the vec-index's own sqlite state — the wrapped inner NoteRepository stays unsynchronized, and this PR is what lets two threads actually race on it now.
self._lock correctly guards self._db and _dim/_degraded (every self._db.execute/.commit() is inside with self._lock:), but calls into self._inner (add_note, delete_note, get_note, ...) are never covered by any lock. JsonlNoteRepository/InMemoryNoteRepository (core/note_store.py, core/repository.py) have zero synchronization: plain dict mutations plus unsynchronized open(path, "a") appends, with check-then-act races in add_note/delete_note/set_status. gui.py wires one shared repo into both the grandplan-capture worker thread and every grandplan-chat thread (chat_window.py's _apply/_respond/_draft/_improve → apply_plan_draft/apply_improvement_draft). Before this fix, the sqlite crash killed cross-thread capture outright, masking this; after it, that same concurrency is unlocked while the inner store stays racy (e.g. a chat edit and a capture approve on the same note_id can interleave). The module docstring's new "Thread-shared" claim overstates what's actually guaranteed. Suggest either extending synchronization around inner-repo mutations or explicitly narrowing the docstring + filing a tracked follow-up, rather than leaving it implying full thread-safety.

[LOW] most_similar releases the lock before iterating self._inner.get_note(...), unlike _sync/_index which hold it across inner calls — harmless (tombstone-null already handled) but inconsistent with the stated pattern; a comment would help.

Other checks — clean: (a) every self._db access is lock-protected; (b) RLock re-entrancy for _sync → _index is correct, no lock-ordering hazard since the inner repo holds no locks of its own; (c) the regression test is deterministic (HashingEmbedder is non-random) and would genuinely fail pre-fix (sqlite3.connect defaults to check_same_thread=True), though it only exercises sequential handoff, not true concurrent contention. ruff check on both touched files passes.

[wimaan3]

Maintainer response to the automated review above

The HIGH finding is real but pre-existing, not introduced here: without the optional [index] extra, maybe_indexed returns the bare inner repository, which the capture worker and chat threads already share unlocked today — and the indexed configuration's brute-force fallback path called the inner store cross-thread as well. This PR neither adds a writer nor widens the sharing; it removes a deterministic crash in the indexed path. Blocking the crash fix on the latent inner-store contract wouldn't make that gap smaller — it would just keep phone capture broken.

Scoped disposition:

• The inner-repository concurrency contract (plus the docstring-wording note and the LOW lock-scope inconsistency) is now tracked as Thread-safety of the inner NoteRepository under GUI concurrency (capture worker × chat threads) #44 with concrete acceptance criteria, including a true concurrent-contention test.
• This PR merges as the crash fix it is: regression test + all sqlite access serialized in the wrapper.

Both LOW notes from the first review are also folded into #44.