[Ariosolzq]

Pull Request

Linked issue

Closes #1103

What & why

MCP serve --auto-watch updated graph nodes without refreshing derived indexes, leaving FTS inconsistent with the graph.

Original root cause: MCP startup did not supply the existing run_post_processing callback, and start_watch_thread() did not forward it to watch().

Additional startup issue identified during review: watch() ran startup reconciliation and post-processing before starting the filesystem observer. Edits made after reconciliation but before observer startup were not captured, leaving affected files stale until another update.

Changes:

• FTS post-processing: Forward the existing callback through the MCP auto-watch path, preserving daemon argument compatibility.
• Observer-first startup: Start the filesystem observer before startup reconciliation and post-processing, so file events can be collected during startup.
• Serialized processing: Use a shared processor lock for both startup work and event batches. Startup holds the lock through reconciliation and post-processing; captured event batches acquire the same lock before updating the graph and FTS. This provides serialization without introducing a separate task queue.
• Lost-wakeup prevention: Replace watchdog's EventDebouncer with a small project-local _WatchDebouncer. Its initial condition wait checks pending events and stop state under the same lock, preventing an early notification from being missed.
• Startup failure cleanup: Stop the started observer and debouncer before propagating startup errors.
• Regression coverage: Add tests for startup-window edits, processing serialization, and first-event/stop wake-up behavior, alongside the original five MCP auto-watch tests.

The follow-up production changes are confined to code_review_graph/incremental.py. The original callback-wiring fix remains intact.

Recovery of previously stale indexes remains outside this PR and is tracked separately in #1104.

How it was tested

• Maintainer regression: test_edit_saved_during_startup_postprocessing_reaches_graph passes with its xfail removed. Its behavioral assertions are unchanged.
• Maintainer review tests: All 21 original tests from review/pr-1108 pass without modification, verified on Python 3.11 and 3.13.
• Original [Bug]: MCP serve --auto-watch skips FTS post-processing after file updates #1103 regression tests: All five pass, covering callback wiring, startup reconciliation, and create/modify/delete events.
• Full suite: 4,282 passed, 850 skipped, 2 xfailed, 2 xpassed.
• Opt-in concurrency tests: 40 passed.
• Focused change-impact verification: 214 passed.
• Native filesystem verification: macOS FSEvents and PollingObserver startup-window tests passed, including graph/FTS consistency checks.
• Event-burst verification: A bounded 40-file test repository produced 53 observed events during a delayed startup; they were processed across two batches with consistent graph and FTS results.
• Regression sensitivity: Controlled mutations of observer startup ordering, shared locking, and initial wake-up behavior caused the corresponding regression tests to fail.
• Static checks: Ruff, mypy, and Git whitespace checks passed.

The follow-up was not tested against the maintainer's full 17k-node workload or on Linux/Windows.

Commands executed:

uv run --locked --extra dev pytest tests/ --tb=short -q -r fEsxX

uv run --locked --extra dev ruff check code_review_graph/

uv run --locked --extra dev mypy code_review_graph/ --ignore-missing-imports --no-strict-optional

The original #1103 implementation was also smoke-tested with a real MCP stdio server and a native WSL2 filesystem event, with no missing or stale FTS IDs observed.

Checklist

• Tests added for new functionality
• All tests pass
• Linting passes
• Type checking passes
• Lines are at most 100 characters
• Docs updated where behavior changed (docstrings)

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[tirth8205]

Thank you for this PR. I tested it with a real serve --auto-watch session.

Before I can merge it, please fix these:

1. With --auto-watch, saves made during startup post-processing are lost. watch() now runs post-processing before it starts the observer (code_review_graph/incremental.py:3231-3239). Those files stay stale until they are saved again. Example: change a file while the server is off. Start the server, then save another file right away. On a 17k-node graph, 18 of 120 quick saves were lost (staging lost 4). Please start the observer first. Then run startup post-processing in the same queue as event batches. The failing test is test_edit_saved_during_startup_postprocessing_reaches_graph. It is in tests/test_mcp_auto_watch_startup_window.py, on the branch review/pr-1108 in this repo. Please remove its xfail mark when it passes.

After you push the fix, I will test it again.

[Ariosolzq]

Thanks for reproducing the startup issue and providing the regression test. I've pushed the follow-up fix in 09dca915.

Root cause and fix

The observer previously started after startup reconciliation and post-processing. Edits made during that interval were not captured, leaving affected files stale until another update.

The fix:

• Observer-first startup: Start the observer before reconciliation so edits during startup can be captured.
• Shared serialization: Startup reconciliation/post-processing and event batches use the same processor lock. Events are buffered during startup, and batch updates execute only after startup processing releases the lock. This provides the required serialization without introducing a separate task queue.
• Lost-wakeup prevention: Replaced watchdog's EventDebouncer with a small _WatchDebouncer that checks pending events and stop state before waiting, preventing the first notification from being missed.
• Failure cleanup: Stop the observer and debouncer if startup fails.

Before vs. After

sequenceDiagram
    participant U as Editor
    participant W as Startup
    participant O as Observer
    participant G as GraphStore

    Note over W,O: BEFORE
    W->>G: Reconcile files
    W->>G: Post-processing
    U->>U: Save file during startup
    Note over O: Not started — event missed
    W->>O: Start observer
    Note over G: File remains stale until another update

Loading

sequenceDiagram
    participant U as Editor
    participant W as Startup
    participant L as Processor Lock
    participant O as Observer
    participant D as Debouncer
    participant G as GraphStore

    Note over W,G: AFTER
    W->>D: Start debouncer
    W->>L: Acquire
    W->>O: Start observer
    W->>G: Reconcile files
    W->>G: Post-processing
    U->>O: Save file during startup
    O->>D: Capture and buffer event
    D->>L: Batch waits for lock
    W->>L: Release
    L-->>D: Lock acquired
    D->>G: Update graph + refresh FTS
    D->>L: Release

Loading

Verification

• Your test_edit_saved_during_startup_postprocessing_reaches_graph now passes with xfail removed; its assertions are unchanged.
• All 21 original review-branch tests pass unmodified on Python 3.11 and 3.13.
• Full suite: 4,282 passed, 850 skipped, 2 xfailed, 2 xpassed.
• Opt-in concurrency: 40 passed.
• Native macOS FSEvents, PollingObserver and FTS consistency checks passed.
• Ruff and mypy passed.

The changes are ready for your retest. Thanks again for the detailed reproduction.

[Ariosolzq]

One additional finding from the shutdown/concurrency review: I opened a separate issue, #1121 — MCP auto-watch can close GraphStore while its watcher is still active.

The MCP shutdown path currently closes watch_store without stopping and joining the background watcher first. A deterministic, instrumented reproduction confirms the lifecycle race on both upstream staging and this PR's head. The issue includes a standalone regression test, RED logs, and a proposed stop → join → close contract.

This is a pre-existing defect, not introduced by #1108. However, the additional watcher post-processing in this PR may increase the exposure window; that impact has not been measured.

I've kept the shutdown fix out of #1108 to preserve its scope. No SQLite crash or data corruption from MCP shutdown has been demonstrated.

Would you prefer to merge #1108 independently and address #1121 in a follow-up PR, or resolve the shutdown lifecycle issue before merging #1108?

[Copilot]

🟡 Changes recommended

A critical startup race allows queued event batches to write to a rejected foreign-root graph during cleanup; repository-root validation must precede observer creation, with regression coverage for queued events.

1 open issue

• Reject foreign-root graphs before starting the observer · New

_{Review effort: Balanced}

[Ariosolzq]

Thanks for catching this. Confirmed and fixed in [650510a](https://github.com/tirth8205/code-review-graph/commit/650510a7fd63df816e95d50c5bde3856838367bf).

The root cause was that the observer could queue an event before startup reconciliation rejected a foreign-root graph. Once the startup lock was released, the queued batch could bypass root validation via reconcile_stale=False and write to the wrong graph.

The fix moves repository-root validation before observer initialization, while preserving observer-first reconciliation for valid repositories.

Verification:

• Independently reproduced the original race with actual writes to a disposable foreign-root database (RED → GREEN).
• Added regression coverage for queued events and foreign-root rejection.
• Confirmed the maintainer's startup-window test still passes.
• Full local suite: 4,283 passed; Ruff and mypy passed.

Both GitHub Actions workflows are currently awaiting maintainer approval, so CI has not run yet.

Thanks again for the review!