From 893f049eb79584351734c2027fdc83a1625a95ec Mon Sep 17 00:00:00 2001 From: "tao.chen" <93983997+taochen-ct@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:39:27 +0800 Subject: [PATCH] fix: call bind (not set_active) when switching to a fresh session Bug: 400 from PUT /opencode-bridge/sessions/active when the user picked a session from the global list that wasn't yet bound to this notebook. The cell-action onSwitchSession callback was: 1. PUT /sessions/active (server checks bound -> 400) 2. PUT /sessions/notebook (bind, but never reached) The order was wrong: set_active requires the session to be bound FIRST. Fix: the PUT /sessions/notebook route does both bind AND set active in a single call (server handler: bind_existing() then response includes the new activeSessionId). So the callback now only needs to call bind-existing. The /sessions/active endpoint is still useful for future "switch between already-bound sessions" flows but the current "pick from global list" path doesn't need it. Tests: updated the switching test to assert bind-existing is called and set-active is NOT. Also added not.toHaveBeenCalled() to lock in the new behavior. 76 jest, 73 pytest, build green. Co-Authored-By: Claude --- src/__tests__/opencode_cell_actions.spec.ts | 9 +++++---- src/components/opencode_cell_actions.ts | 16 ++++++---------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/__tests__/opencode_cell_actions.spec.ts b/src/__tests__/opencode_cell_actions.spec.ts index 54ab25f..61f3216 100644 --- a/src/__tests__/opencode_cell_actions.spec.ts +++ b/src/__tests__/opencode_cell_actions.spec.ts @@ -1123,9 +1123,6 @@ describe('OpenCodeInlinePrompt', () => { { id: 'sess-B', title: 'second' }, ] }); - mockedSetActiveSession.mockResolvedValue({ - ok: true, notebookPath: 'foo.ipynb', activeSessionId: 'sess-B', - }); mockedBindExistingSession.mockResolvedValue({ ok: true, notebookPath: 'foo.ipynb', sessionId: 'sess-B', }); @@ -1144,7 +1141,11 @@ describe('OpenCodeInlinePrompt', () => { select.value = 'sess-B'; select.dispatchEvent(new Event('change')); await new Promise(r => setTimeout(r, 5)); - expect(mockedSetActiveSession).toHaveBeenCalled(); + // bind-existing is the single call (it does both bind AND set + // active on the server). Calling set_active first would 400 + // because the session is not yet bound. + expect(mockedBindExistingSession).toHaveBeenCalled(); + expect(mockedSetActiveSession).not.toHaveBeenCalled(); expect((prompt as any)._sessionId).toBe('sess-B'); }); diff --git a/src/components/opencode_cell_actions.ts b/src/components/opencode_cell_actions.ts index e19d061..ed8f272 100644 --- a/src/components/opencode_cell_actions.ts +++ b/src/components/opencode_cell_actions.ts @@ -30,7 +30,6 @@ import { callOpenCodeReplyPermission, callOpenCodeReplyQuestion, callOpenCodeSessionMessages, - callOpenCodeSetActiveSession, subscribeOpenCodeEvents, type OpenCodeEventSubscription } from '../api/opencode_client'; @@ -222,7 +221,12 @@ export class OpenCodeCellActions extends Widget { // Closing the SSE first ensures the old session's events stop // coming in while we transition. this._closeSse(); - const r = await callOpenCodeSetActiveSession( + // PUT /sessions/notebook binds AND sets active in one call. + // (The separate PUT /sessions/active is only for switching + // between sessions that are ALREADY bound; for a session the + // user just picked from the global list, we need to bind + // first — calling set_active before bind returns 400.) + const r = await callOpenCodeBindExistingSession( this._context.notebookPath, sessionId, _serverSettings @@ -230,14 +234,6 @@ export class OpenCodeCellActions extends Widget { if (!r.ok) { return false; } - // The session was not necessarily bound to this notebook yet. - // Bind it (idempotent; the server's PUT /sessions/notebook - // also sets it as active, so we don't need a separate call). - await callOpenCodeBindExistingSession( - this._context.notebookPath, - sessionId, - _serverSettings - ); this._prompt?.setSessionId(sessionId); return true; },