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 <noreply@anthropic.com>
This commit is contained in:
@@ -1123,9 +1123,6 @@ describe('OpenCodeInlinePrompt', () => {
|
|||||||
{ id: 'sess-B', title: 'second' },
|
{ id: 'sess-B', title: 'second' },
|
||||||
]
|
]
|
||||||
});
|
});
|
||||||
mockedSetActiveSession.mockResolvedValue({
|
|
||||||
ok: true, notebookPath: 'foo.ipynb', activeSessionId: 'sess-B',
|
|
||||||
});
|
|
||||||
mockedBindExistingSession.mockResolvedValue({
|
mockedBindExistingSession.mockResolvedValue({
|
||||||
ok: true, notebookPath: 'foo.ipynb', sessionId: 'sess-B',
|
ok: true, notebookPath: 'foo.ipynb', sessionId: 'sess-B',
|
||||||
});
|
});
|
||||||
@@ -1144,7 +1141,11 @@ describe('OpenCodeInlinePrompt', () => {
|
|||||||
select.value = 'sess-B';
|
select.value = 'sess-B';
|
||||||
select.dispatchEvent(new Event('change'));
|
select.dispatchEvent(new Event('change'));
|
||||||
await new Promise(r => setTimeout(r, 5));
|
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');
|
expect((prompt as any)._sessionId).toBe('sess-B');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -30,7 +30,6 @@ import {
|
|||||||
callOpenCodeReplyPermission,
|
callOpenCodeReplyPermission,
|
||||||
callOpenCodeReplyQuestion,
|
callOpenCodeReplyQuestion,
|
||||||
callOpenCodeSessionMessages,
|
callOpenCodeSessionMessages,
|
||||||
callOpenCodeSetActiveSession,
|
|
||||||
subscribeOpenCodeEvents,
|
subscribeOpenCodeEvents,
|
||||||
type OpenCodeEventSubscription
|
type OpenCodeEventSubscription
|
||||||
} from '../api/opencode_client';
|
} from '../api/opencode_client';
|
||||||
@@ -222,7 +221,12 @@ export class OpenCodeCellActions extends Widget {
|
|||||||
// Closing the SSE first ensures the old session's events stop
|
// Closing the SSE first ensures the old session's events stop
|
||||||
// coming in while we transition.
|
// coming in while we transition.
|
||||||
this._closeSse();
|
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,
|
this._context.notebookPath,
|
||||||
sessionId,
|
sessionId,
|
||||||
_serverSettings
|
_serverSettings
|
||||||
@@ -230,14 +234,6 @@ export class OpenCodeCellActions extends Widget {
|
|||||||
if (!r.ok) {
|
if (!r.ok) {
|
||||||
return false;
|
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);
|
this._prompt?.setSessionId(sessionId);
|
||||||
return true;
|
return true;
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user