fix: drain before invalidating watchers, keep cache-clear on failed cleanup

- indexer.ts: the generation bump ran before drainWatcherOps(), so a watcher
  that had already completed addDocs() would see itself stale and return
  without recording those chunks in pathChunkCounts. The rebuild then computed
  staleIds from counts that omitted them and they survived in the index.
  `indexing` already blocks new watcher work, so the drain now happens first —
  letting in-flight operations record what they wrote — and the generation is
  bumped afterwards to cover anything that slips through.

- indexer.ts: discardPartialIndex() reset discardedPreviousIndex when the
  delete failed, which made the caller skip clearIndexCache() and leave a
  meta.json describing the previous documents — already deleted by this
  rebuild. restoreFromMeta() would then report a ready index for documents
  that no longer exist on the next launch, which is worse than the leftovers
  it was protecting. The flag now stays true so the stale cache is dropped,
  while pathChunkCounts is kept for a retry in the same session. This reverses
  the choice made in bc906da.

- server.py: runner.run() can end on a transport or LLM error, reaching none
  of the disconnect / watchdog / lifespan paths, and the finally discarded the
  only session handle without cancelling grade tasks — leaving a grader
  subprocess and its Ollama request running until timeout, then queueing into
  a dead worker. The finally now awaits session.shutdown() first, suppressed
  so teardown cannot mask the original error.

Verified: a watcher writing two chunks as a rebuild starts is stranded under
bump-before-drain and cleaned up under drain-then-bump; a failed partial
cleanup now still clears the cache while keeping the counts; and a runner error
leaves the grader alive without the shutdown call and terminated with it, with
shutdown() confirmed safe to call twice.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: samanyugoyal2010

apps/moss-interview-coach/backend/server.py

@@ -961,6 +961,13 @@ async def _handshake_watchdog(current: ActiveSession) -> None:
                 await watchdog
     finally:
         if session is not None:
+            # runner.run() can also end on a transport or LLM error, which
+            # reaches none of the disconnect / watchdog / lifespan paths. Tear
+            # the session down here before dropping the only handle to it, or a
+            # spawned grader keeps its subprocess and Ollama request alive until
+            # timeout and then queues a result into a dead worker.
+            with suppress(Exception):
+                await session.shutdown()
             active_sessions.discard(session)
 
 

apps/moss-vscode/src/indexer/indexer.ts

@@ -112,14 +112,20 @@ export class CodebaseIndexer {
     if (this.indexing) {
       return;
     }
+    // `indexing` is already set, so no *new* watcher work can start.
     this.indexing = true;
     this.discardedPreviousIndex = false;
-    // Invalidate watcher work already in flight, then wait for it to settle so
-    // no late write lands between our delete and our rewrite.
-    this.generation += 1;
 
     try {
+      // Drain before bumping the generation, not after. A watcher that has
+      // already called addDocs() must be allowed to record those chunks in
+      // pathChunkCounts — marking it stale first would make it return silently,
+      // leaving its writes out of the staleIds we compute below and stranding
+      // them in the index. Once everything in flight has settled, the bump
+      // covers anything that slips through afterwards.
       await this.drainWatcherOps();
+      this.generation += 1;
+
       const files = await scanWorkspaceFiles(token);
       this.setStatus({ state: "indexing", processed: 0, total: files.length });
 
@@ -482,7 +488,13 @@ export class CodebaseIndexer {
       try {
         await this.deleteInBatches(partialIds);
       } catch {
-        this.discardedPreviousIndex = false;
+        // Deliberately leave discardedPreviousIndex true so the caller still
+        // drops the persisted cache. That cache describes the *previous*
+        // documents, which this rebuild already deleted — keeping it would let
+        // restoreFromMeta() come back on the next launch reporting a ready
+        // index for documents that no longer exist, which is worse than the
+        // leftovers. pathChunkCounts is kept so a retry in this session can
+        // still delete what we failed to remove here.
         return;
       }
     }