fix: harden session lifecycle and align config persistence logic

- resumeOrCreateSession: Remove destroy-on-error for the resume path. Config
  errors now propagate without destroying a pre-existing session. The destroy
  pattern remains in createSession (where the session is newly created and has
  no prior state to preserve).

- setSessionMode fallback: When session/set_mode returns -32601 and the
  fallback uses session/set_config_option, now keep modes.currentModeId
  in sync with the updated currentValue. Prevents stale cached state in
  getModes() when the fallback path is used.

- persistSessionStateFromMethod: Re-read the record from persistence instead
  of using a stale pre-await snapshot. Prevents race conditions where
  concurrent session/update events (processed by persistSessionStateFromEvent)
  are silently overwritten by optimistic updates.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
Nathan Flurry 2026-03-05 18:41:20 -08:00
parent bdbd1d46a9
commit 971689ed6e

View file

@ -737,23 +737,14 @@ export class SandboxAgent {
const existing = await this.persist.getSession(request.id); const existing = await this.persist.getSession(request.id);
if (existing) { if (existing) {
let session = await this.resumeSession(existing.id); let session = await this.resumeSession(existing.id);
try { if (request.mode) {
if (request.mode) { session = (await this.setSessionMode(session.id, request.mode)).session;
session = (await this.setSessionMode(session.id, request.mode)).session; }
} if (request.model) {
if (request.model) { session = (await this.setSessionModel(session.id, request.model)).session;
session = (await this.setSessionModel(session.id, request.model)).session; }
} if (request.thoughtLevel) {
if (request.thoughtLevel) { session = (await this.setSessionThoughtLevel(session.id, request.thoughtLevel)).session;
session = (await this.setSessionThoughtLevel(session.id, request.thoughtLevel)).session;
}
} catch (err) {
try {
await this.destroySession(session.id);
} catch {
// Best-effort cleanup
}
throw err;
} }
return session; return session;
} }
@ -959,7 +950,7 @@ export class SandboxAgent {
} }
const response = await live.sendSessionMethod(record.id, method, params, options); const response = await live.sendSessionMethod(record.id, method, params, options);
await this.persistSessionStateFromMethod(record, method, params, response); await this.persistSessionStateFromMethod(record.id, method, params, response);
const refreshed = await this.requireSessionRecord(record.id); const refreshed = await this.requireSessionRecord(record.id);
return { return {
session: this.upsertSessionHandle(refreshed), session: this.upsertSessionHandle(refreshed),
@ -968,34 +959,52 @@ export class SandboxAgent {
} }
private async persistSessionStateFromMethod( private async persistSessionStateFromMethod(
record: SessionRecord, sessionId: string,
method: string, method: string,
params: Record<string, unknown>, params: Record<string, unknown>,
response: unknown, response: unknown,
): Promise<void> { ): Promise<void> {
// Re-read the record from persistence so we merge against the latest
// state, not a stale snapshot captured before the RPC await.
const record = await this.persist.getSession(sessionId);
if (!record) {
return;
}
if (method === "session/set_config_option") { if (method === "session/set_config_option") {
const configOptions = extractConfigOptionsFromSetResponse(response); const configId = typeof params.configId === "string" ? params.configId : null;
if (configOptions) { const value = typeof params.value === "string" ? params.value : null;
await this.persist.updateSession({ const updates: Partial<SessionRecord> = {};
...record,
configOptions: cloneConfigOptions(configOptions), const serverConfigOptions = extractConfigOptionsFromSetResponse(response);
}); if (serverConfigOptions) {
} else if (record.configOptions) { updates.configOptions = cloneConfigOptions(serverConfigOptions);
} else if (record.configOptions && configId && value) {
// Server didn't return configOptions — optimistically update the // Server didn't return configOptions — optimistically update the
// cached currentValue so subsequent getConfigOptions() reflects the // cached currentValue so subsequent getConfigOptions() reflects the
// change without a round-trip. // change without a round-trip.
const configId = typeof params.configId === "string" ? params.configId : null; const updated = applyConfigOptionValue(record.configOptions, configId, value);
const value = typeof params.value === "string" ? params.value : null; if (updated) {
if (configId && value) { updates.configOptions = updated;
const updated = applyConfigOptionValue(record.configOptions, configId, value); }
if (updated) { }
await this.persist.updateSession({
...record, // When a mode-category config option is set via set_config_option
configOptions: updated, // (fallback path from setSessionMode), keep modes.currentModeId in sync.
}); if (configId && value) {
const source = updates.configOptions ?? record.configOptions;
const option = source ? findConfigOptionById(source, configId) : null;
if (option?.category === "mode") {
const nextModes = applyCurrentMode(record.modes, value);
if (nextModes) {
updates.modes = nextModes;
} }
} }
} }
if (Object.keys(updates).length > 0) {
await this.persist.updateSession({ ...record, ...updates });
}
return; return;
} }