Session pool reset detection via message count drop

[sterling-prog]

Summary

• Adds lastMessageCount to PooledProcess (init: 0) and messageCount to PendingRequest
• Detects gateway session resets (via /new, idle timeout, compaction) by tracking incoming messageCount per request
• When messageCount < proc.lastMessageCount on a locked process: recycles stale CLI process and routes to a fresh warm/cold slot
• Idle process: recycles immediately (clearSessionLock + killAndRespawn)
• Busy process: marks orphaned=true, clears lock from map; releaseProcess kills after current request completes
• Threads messageCount through all call sites: execute(), routeToProcess(), assignToProcess(), enqueueOnProcess(), enqueueOnSentinel(), drainNextRequest(), cold spawn .then()
• Routes layer passes body.messages.length to router.execute()

Test plan

• 5 requests with increasing message counts → all route to same locked process, no recycle
• Request with message count lower than previous → session_reset_detected logged, fresh process claimed
• messageCount: 0 edge case → no recycle (guard: messageCount > 0)
• First request on fresh process → no recycle (guard: proc.lastMessageCount > 0)
• Busy process during reset → process enters orphaned, completes current request, then recycles
• npm run build passes clean ✅

🤖 Generated with Claude Code

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In SessionPoolRouter.execute(), the busy-process session reset path directly calls lockedSessions.delete(sessionKey) and mutates proc.lockedTo rather than going through clearSessionLock(), which breaks the invariant that all unlocks use the canonical helper and leaves fields like agentChannel/requestCount uncleared; consider refactoring this branch to reuse clearSessionLock() (or a variant that marks the process orphaned) to keep lock state consistent.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In SessionPoolRouter.execute(), the busy-process session reset path directly calls `lockedSessions.delete(sessionKey)` and mutates `proc.lockedTo` rather than going through `clearSessionLock()`, which breaks the invariant that all unlocks use the canonical helper and leaves fields like `agentChannel`/`requestCount` uncleared; consider refactoring this branch to reuse `clearSessionLock()` (or a variant that marks the process orphaned) to keep lock state consistent.

## Individual Comments

### Comment 1
<location path="src/subprocess/router.ts" line_range="385-387" />
<code_context>
+    this.warmPool.get(model)!.push(proc);
+  }
+
+  private async spawnCold(model: PooledModel): Promise<PooledProcess> {
+    const proc = this.spawnProcess(model);
+    await new Promise<void>((resolve) => {
+      if (proc.ready) {
+        resolve();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The spawnCold readiness wait never actually triggers because `proc.ready` is set synchronously in spawnProcess.

Because `spawnProcess` sets `pooled.ready = true` just before returning, the `spawnCold` promise always resolves immediately and never uses the `stdout.once("data", onReady)`/timeout path. As a result, `spawnCold` effectively behaves like `spawnWarm`.

If you do want to wait for CLI output before marking readiness, consider moving the `pooled.ready = true` assignment to the point where the first init message is parsed and use that here. If not, it would be clearer to remove the unused readiness logic from `spawnCold`.

Suggested implementation:

```typescript
  private async spawnCold(model: PooledModel): Promise<PooledProcess> {
    return this.spawnProcess(model);
  }

```

If the existing implementation of `spawnCold` in your local file differs slightly in formatting or structure (e.g. missing `return proc;` or different brace layout), adjust the SEARCH block accordingly so that the entire function body (including the `await new Promise(...)` readiness logic) is replaced with the simplified implementation that just returns `this.spawnProcess(model)`.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (bug_risk): The spawnCold readiness wait never actually triggers because proc.ready is set synchronously in spawnProcess.

Because spawnProcess sets pooled.ready = true just before returning, the spawnCold promise always resolves immediately and never uses the stdout.once("data", onReady)/timeout path. As a result, spawnCold effectively behaves like spawnWarm.

If you do want to wait for CLI output before marking readiness, consider moving the pooled.ready = true assignment to the point where the first init message is parsed and use that here. If not, it would be clearer to remove the unused readiness logic from spawnCold.

Suggested implementation:

  private async spawnCold(model: PooledModel): Promise<PooledProcess> {
    return this.spawnProcess(model);
  }

If the existing implementation of spawnCold in your local file differs slightly in formatting or structure (e.g. missing return proc; or different brace layout), adjust the SEARCH block accordingly so that the entire function body (including the await new Promise(...) readiness logic) is replaced with the simplified implementation that just returns this.spawnProcess(model).

[gemini-code-assist]

Code Review

This pull request implements a session-aware process pool for the Claude CLI proxy, transitioning from a subprocess-per-request model to persistent, session-locked processes to improve performance and prevent context leakage. The changes include the SessionPoolRouter for pool management, request queuing, and orphan reclamation, along with integration into the Express routes and standalone server. Review feedback emphasizes the need for architectural consistency by using the canonical clearSessionLock method in all session termination paths, immediately rejecting queued requests upon session reset, and ensuring that client disconnects do not kill processes, as required by the specification.

[gemini-code-assist]

This block manually deletes the session lock and clears lockedTo instead of using the canonical clearSessionLock() method. This violates the "Major" rule defined in the specification (Finding N8). Additionally, it should call this.rejectProcessQueue(proc) here to immediately reject any stale requests that might be queued for this session, ensuring they are failed as soon as the reset is detected rather than waiting for the active request to complete.

Suggested change

	this.lockedSessions.delete(sessionKey);
	proc.lockedTo = null;
	proc.orphaned = true;
	this.rejectProcessQueue(proc);
	this.clearSessionLock(sessionKey, proc);
	proc.orphaned = true;

References

1. Every code path that unlocks a session MUST call the canonical clearSessionLock() function to prevent stale artifacts (Finding N8).

[gemini-code-assist]

The implementation kills and respawns the process when a client disconnects. This contradicts the specification in specs/session-pooling.spec.md (Module 2 AC and "Client Disconnect Handling"), which states that the process should be allowed to finish generating its response and then return to an idle state. While killing the process prevents context divergence, it increases latency for subsequent requests due to the need for a fresh process claim or cold start.

[gemini-code-assist]

In the busy branch of orphan reclamation, the session lock is not cleared. While releaseProcess will eventually clear it, the specification (Finding N8) requires all unlock paths to use the canonical clearSessionLock() method. Clearing it here immediately (as is done for the idle case at line 951) would be more consistent and ensure the old session key is unbound immediately.

          this.clearSessionLock(key, proc);
          proc.orphaned = true;

References

1. Every code path that unlocks a session MUST call the canonical clearSessionLock() function to prevent stale artifacts (Finding N8).