From 3cc03eb9dad9cd6e4361f8d63756a29bb9a7f96b Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Sun, 31 May 2026 03:11:10 +0100 Subject: [PATCH] fix(hub): skip redundant set-session-config RPC on resume (#740) resumeSession already passes permissionMode to spawnSession. The follow-up applySessionConfig raced session-alive (handler not registered yet) and returned resume_failed after hub restart. Co-authored-by: Cursor --- .../sync/permissionModePersistence.test.ts | 20 +--- hub/src/sync/sessionModel.test.ts | 97 +++++++++++++++++++ hub/src/sync/syncEngine.ts | 10 +- 3 files changed, 104 insertions(+), 23 deletions(-) diff --git a/hub/src/sync/permissionModePersistence.test.ts b/hub/src/sync/permissionModePersistence.test.ts index 761665c2..13c78941 100644 --- a/hub/src/sync/permissionModePersistence.test.ts +++ b/hub/src/sync/permissionModePersistence.test.ts @@ -123,7 +123,7 @@ describe('permission mode persistence', () => { restartedEngine.handleMachineAlive({ machineId: machine.id, time: Date.now() }) let capturedSpawnPermissionMode: string | undefined - const calls: Array<{ type: 'spawn' } | { type: 'config'; sessionId: string; permissionMode?: string }> = [] + let configRpcCalls = 0 ;(restartedEngine as any).rpcGateway.spawnSession = async ( _machineId: string, _directory: string, @@ -138,7 +138,6 @@ describe('permission mode persistence', () => { permissionMode?: string ) => { capturedSpawnPermissionMode = permissionMode - calls.push({ type: 'spawn' }) restartedEngine.handleSessionAlive({ sid: session.id, time: Date.now(), @@ -146,17 +145,9 @@ describe('permission mode persistence', () => { }) return { type: 'success', sessionId: session.id } } - ;(restartedEngine as any).rpcGateway.requestSessionConfig = async ( - sessionId: string, - config: { permissionMode?: string } - ) => { - calls.push({ type: 'config', sessionId, permissionMode: config.permissionMode }) - restartedEngine.handleSessionAlive({ - sid: sessionId, - time: Date.now(), - permissionMode: config.permissionMode as never - }) - return { applied: { permissionMode: config.permissionMode } } + ;(restartedEngine as any).rpcGateway.requestSessionConfig = async () => { + configRpcCalls += 1 + throw new Error('RPC handler not registered') } ;(restartedEngine as any).waitForSessionActive = async () => true @@ -164,7 +155,6 @@ describe('permission mode persistence', () => { expect(result).toEqual({ type: 'success', sessionId: session.id }) expect(capturedSpawnPermissionMode).toBe('yolo') - expect(calls).toContainEqual({ type: 'spawn' }) - expect(calls).toContainEqual({ type: 'config', sessionId: session.id, permissionMode: 'yolo' }) + expect(configRpcCalls).toBe(0) }) }) diff --git a/hub/src/sync/sessionModel.test.ts b/hub/src/sync/sessionModel.test.ts index 7da3e020..eda43510 100644 --- a/hub/src/sync/sessionModel.test.ts +++ b/hub/src/sync/sessionModel.test.ts @@ -892,6 +892,103 @@ describe('session model', () => { } }) + it('resume succeeds when session-alive races ahead of set-session-config and merges spawned session', async () => { + const store = new Store(':memory:') + const engine = new SyncEngine( + store, + {} as never, + new RpcRegistry(), + { broadcast() {} } as never + ) + + try { + const oldSession = engine.getOrCreateSession( + 'session-resume-config-race', + { + path: '/tmp/project', + host: 'localhost', + machineId: 'machine-1', + flavor: 'codex', + codexSessionId: 'codex-thread-race' + }, + null, + 'default', + 'gpt-5.4' + ) + engine.getOrCreateMachine( + 'machine-1', + { host: 'localhost', platform: 'linux', happyCliVersion: '0.1.0' }, + null, + 'default' + ) + engine.handleMachineAlive({ machineId: 'machine-1', time: Date.now() }) + engine.handleSessionAlive({ + sid: oldSession.id, + permissionMode: 'yolo', + time: Date.now() + }) + engine.handleSessionEnd({ sid: oldSession.id, time: Date.now() }) + + const spawnedSession = engine.getOrCreateSession( + 'session-resume-config-race-spawned', + { + path: '/tmp/project', + host: 'localhost', + machineId: 'machine-1', + flavor: 'codex', + codexSessionId: 'codex-thread-race' + }, + null, + 'default', + 'gpt-5.4' + ) + const spawnedSessionId = spawnedSession.id + let configRpcCalls = 0 + let mergeCalls = 0 + const sessionCache = (engine as any).sessionCache + const mergeSessions = sessionCache.mergeSessions.bind(sessionCache) + sessionCache.mergeSessions = async (oldSessionId: string, newSessionId: string, namespace: string) => { + mergeCalls += 1 + return mergeSessions(oldSessionId, newSessionId, namespace) + } + ;(engine as any).rpcGateway.spawnSession = async ( + _machineId: string, + _directory: string, + _agent: string, + _model?: string, + _modelReasoningEffort?: string, + _yolo?: boolean, + _sessionType?: string, + _worktreeName?: string, + _resumeSessionId?: string, + _effort?: string, + permissionMode?: string + ) => { + engine.handleSessionAlive({ + sid: spawnedSessionId, + time: Date.now(), + permissionMode: permissionMode as never + }) + return { type: 'success', sessionId: spawnedSessionId } + } + ;(engine as any).rpcGateway.requestSessionConfig = async () => { + configRpcCalls += 1 + throw new Error('RPC handler not registered') + } + ;(engine as any).waitForSessionActive = async () => true + + const result = await engine.resumeSession(oldSession.id, 'default') + + expect(result).toEqual({ type: 'success', sessionId: spawnedSessionId }) + expect(configRpcCalls).toBe(0) + expect(mergeCalls).toBe(1) + expect(engine.getSession(spawnedSessionId)?.permissionMode).toBe('yolo') + expect(store.sessions.getSession(oldSession.id)).toBeNull() + } finally { + engine.stop() + } + }) + it('resolves a local resume target for a Codex session', () => { const store = new Store(':memory:') const engine = new SyncEngine( diff --git a/hub/src/sync/syncEngine.ts b/hub/src/sync/syncEngine.ts index 6ae12076..73c284a3 100644 --- a/hub/src/sync/syncEngine.ts +++ b/hub/src/sync/syncEngine.ts @@ -685,14 +685,8 @@ export class SyncEngine { return { type: 'error', message: 'Session failed to become active', code: 'resume_failed' } } - if (preferredPermissionMode !== undefined) { - try { - await this.applySessionConfig(spawnResult.sessionId, { permissionMode: preferredPermissionMode }) - } catch (error) { - const message = error instanceof Error ? error.message : 'Failed to restore permission mode' - return { type: 'error', message, code: 'resume_failed' } - } - } + // permissionMode is passed to spawnSession above; do not call set-session-config here. + // session-alive can arrive before the CLI registers that RPC handler, which caused resume_failed. if (spawnResult.sessionId !== access.sessionId) { // The old session may have already been merged by the automatic dedup path