From 25d69e049ce5886750219115ae41a03797855757 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Mon, 27 Jul 2026 01:27:05 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20main=20does=20not=20typecheck=20?= =?UTF-8?q?=E2=80=94=20stale=20recordDestruction=20call=20from=20#256?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit origin/main at e48527a fails `bun run typecheck`: src/daemon/session-manager.ts(1583,27): error TS2339: Property 'recordDestruction' does not exist on type 'RateLimiter'. The main CI run for e48527a is red. How it got in, because the mechanism matters more than the one-line fix: #256 (collaboration role-children) was branched before #258 landed. #258 removed RateLimiter.recordDestruction — the concurrency count is now derived from the live session set rather than stored, precisely because a stored counter reset on restart and only drifted upward. #256's new collaboration-rollback path called the method that #258 deleted. Neither PR conflicted: they touched different lines of session-manager.ts, so git merged them cleanly. And CI only ever validated #256 against the main it was branched from, never against the merge result. A textually-clean merge of two independently-green branches is not a compiling merge, and nothing in the current setup checks that. The fix: drop the call. Removing the session from #sessions IS the rollback now, since the count is derived. The hourly creation timestamp stays recorded — the create was attempted, and refunding it would let a failing spawn drive a retry loop for free. Verified against pristine origin/main in a detached worktree, so this is the merge result and not a local artifact: typecheck + biome + build clean, 2007 tests / 0 fail. Worth considering separately: requiring branches to be current with main before merge would have caught this at the source. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Yash Datta --- src/daemon/session-manager.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index 2b80484..1b178e9 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -1579,8 +1579,13 @@ mcpHub: this.#mcpHub, } catch { // Best-effort — the error we report is the spawn failure. } + // Removing it from #sessions is the whole rollback: the concurrency + // count is derived from the live session set (see rate-limit.ts on why + // it is no longer a stored counter), so there is nothing to decrement. + // The hourly creation timestamp stays recorded on purpose — the create + // was attempted, and a failed spawn shouldn't refund an attempt into a + // retry loop. this.#sessions.delete(session.id); - this.#rateLimiter.recordDestruction(auth.sub); return { type: "response.error", requestId: msg.id,