diff --git a/packages/api/src/openshell-gateway-info.ts b/packages/api/src/openshell-gateway-info.ts index 2f195ffb9..312c8bf62 100644 --- a/packages/api/src/openshell-gateway-info.ts +++ b/packages/api/src/openshell-gateway-info.ts @@ -52,7 +52,17 @@ export type GatewayInfo = z.output; export const SandboxInfoSchema = z.object({ id: z.string(), name: z.string(), - phase: z.enum(['Provisioning', 'Ready', 'Error', 'Deleting', 'Unknown', 'Unspecified']), + phase: z.enum([ + 'Provisioning', + 'Ready', + 'Error', + 'Deleting', + 'Unknown', + 'Unspecified', + 'Starting', + 'Stopping', + 'Stopped', + ]), created_at: z .string() .transform(ts => { diff --git a/packages/main/src/plugin/acp/acp-session-manager.spec.ts b/packages/main/src/plugin/acp/acp-session-manager.spec.ts index f0f144a47..8a1d525b9 100644 --- a/packages/main/src/plugin/acp/acp-session-manager.spec.ts +++ b/packages/main/src/plugin/acp/acp-session-manager.spec.ts @@ -44,12 +44,13 @@ const apiSender: ApiSenderType = { const openshellCli: OpenshellCli = { getCliPath: vi.fn().mockReturnValue('/usr/bin/openshell'), - listSandboxes: vi.fn(), uploadToSandbox: vi.fn(), } as unknown as OpenshellCli; +const mockSandboxList = vi.fn(); const sdkSandbox = { execInteractive: vi.fn(), + list: mockSandboxList, }; const openshellSdkClientManager = { getClient: vi.fn().mockResolvedValue({ sandbox: sdkSandbox }), @@ -104,8 +105,8 @@ describe('AcpSessionManager', () => { beforeEach(() => { vi.resetAllMocks(); vi.mocked(directories.getAcpSessionsDirectory).mockReturnValue(FAKE_SESSIONS_DIR); - vi.mocked(openshellCli.listSandboxes).mockResolvedValue([]); vi.mocked(openshellSdkClientManager.getClient).mockResolvedValue({ sandbox: sdkSandbox } as never); + mockSandboxList.mockResolvedValue([]); manager = new AcpSessionManager(apiSender, openshellCli, agentRegistry, directories, openshellSdkClientManager); }); @@ -853,8 +854,8 @@ describe('AcpSessionManager', () => { events: [], }), ); - vi.mocked(openshellCli.listSandboxes).mockResolvedValue([ - { id: 'other-id', name: 'other-sandbox', phase: 'Ready' }, + mockSandboxList.mockResolvedValue([ + { id: 'other-id', name: 'other-sandbox', phase: 'ready', labels: {}, resourceVersion: '1' }, ]); await manager.init(); @@ -883,7 +884,9 @@ describe('AcpSessionManager', () => { events: [], }), ); - vi.mocked(openshellCli.listSandboxes).mockResolvedValue([{ id: 'sb-id', name: 'my-sandbox', phase: 'Ready' }]); + mockSandboxList.mockResolvedValue([ + { id: 'sb-id', name: 'my-sandbox', phase: 'ready', labels: {}, resourceVersion: '1' }, + ]); await manager.init(); @@ -911,7 +914,9 @@ describe('AcpSessionManager', () => { events: [], }), ); - vi.mocked(openshellCli.listSandboxes).mockResolvedValue([{ id: 'sb-id', name: 'my-sandbox', phase: 'Deleting' }]); + mockSandboxList.mockResolvedValue([ + { id: 'sb-id', name: 'my-sandbox', phase: 'deleting', labels: {}, resourceVersion: '1' }, + ]); await manager.init(); @@ -939,7 +944,7 @@ describe('AcpSessionManager', () => { events: [], }), ); - vi.mocked(openshellCli.listSandboxes).mockRejectedValue(new Error('CLI not found')); + mockSandboxList.mockRejectedValue(new Error('CLI not found')); await manager.init(); @@ -969,7 +974,7 @@ describe('AcpSessionManager', () => { events: [], }), ); - vi.mocked(openshellCli.listSandboxes).mockResolvedValue([]); + mockSandboxList.mockResolvedValue([]); await manager.init(); @@ -1240,7 +1245,9 @@ describe('AcpSessionManager', () => { const agent = createAgentInfo(); vi.mocked(agentRegistry.getAgent).mockResolvedValue(agent); - vi.mocked(openshellCli.listSandboxes).mockResolvedValue([createSandbox()]); + mockSandboxList.mockResolvedValue([ + { id: 'sandbox-1', name: 'test-sandbox', phase: 'ready', labels: {}, resourceVersion: '1' }, + ]); type ExecStreamEvent = { stream: 'stdout' | 'stderr'; data: Buffer } | { type: 'exit'; exitCode: number }; const events: ExecStreamEvent[] = []; diff --git a/packages/main/src/plugin/acp/acp-session-manager.ts b/packages/main/src/plugin/acp/acp-session-manager.ts index 39fcdb86d..cfd83aeb9 100644 --- a/packages/main/src/plugin/acp/acp-session-manager.ts +++ b/packages/main/src/plugin/acp/acp-session-manager.ts @@ -30,6 +30,7 @@ import { AgentRegistry } from '/@/plugin/agent-registry.js'; import { Directories } from '/@/plugin/directories.js'; import { OpenshellCli } from '/@/plugin/openshell-cli/openshell-cli.js'; import { OpenshellSdkClientManager } from '/@/plugin/openshell-cli/openshell-sdk-client-manager.js'; +import { mapSdkSandboxRef } from '/@/plugin/openshell-cli/openshell-sdk-sandbox-mapper.js'; import type { AcpAttachment, AcpElicitationResponseData, @@ -50,6 +51,8 @@ import { createAcpDebug } from './acp-debug.js'; const MAX_STDERR_LINES = 100; const PTY_COLS = 65_535; + +// eslint-disable-next-line sonarjs/publicly-writable-directories const ATTACHMENT_UPLOAD_DIR = '/sandbox/.kaiden-attachments'; const debugPty = createAcpDebug('pty'); @@ -92,7 +95,7 @@ export class AcpSessionManager { @inject(OpenshellCli) private readonly openshellCli: OpenshellCli, @inject(AgentRegistry) private readonly agentRegistry: AgentRegistry, @inject(Directories) private readonly directories: Directories, - @inject(OpenshellSdkClientManager) private readonly openshellSdkClientManager: OpenshellSdkClientManager, + @inject(OpenshellSdkClientManager) private readonly sdkClientManager: OpenshellSdkClientManager, ) {} async init(): Promise { @@ -179,7 +182,7 @@ export class AcpSessionManager { } async createSession(options: AcpSessionCreateOptions): Promise { - const sandboxes = await this.openshellCli.listSandboxes(); + const sandboxes = await this.#listSandboxes(); const sandbox = sandboxes.find(s => s.name === options.sandboxName); if (!sandbox) { throw new Error(`Sandbox "${options.sandboxName}" not found`); @@ -196,7 +199,7 @@ export class AcpSessionManager { debugPty(`${sandbox.name} execInteractive: ${command.join(' ')}`); const abortController = new AbortController(); - const sdkClient = await this.openshellSdkClientManager.getClient(gatewayName); + const sdkClient = await this.sdkClientManager.getClient(gatewayName); const execSession = await sdkClient.sandbox.execInteractive(sandbox.name, command, { tty: false, cols: PTY_COLS, @@ -691,7 +694,7 @@ export class AcpSessionManager { debugPty(`${session.info.sandboxName} reconnecting via execInteractive: ${session.agentCommand.join(' ')}`); const abortController = new AbortController(); - const sdkClient = await this.openshellSdkClientManager.getClient(session.gatewayName); + const sdkClient = await this.sdkClientManager.getClient(session.gatewayName); const execSession = await sdkClient.sandbox.execInteractive(session.info.sandboxName, session.agentCommand, { tty: false, cols: PTY_COLS, @@ -1299,7 +1302,7 @@ export class AcpSessionManager { private async validateSandboxes(): Promise { if (this.sessions.size === 0) return; try { - const sandboxes = await this.openshellCli.listSandboxes(); + const sandboxes = await this.#listSandboxes(); const readySandboxes = new Map(sandboxes.filter(s => s.phase === 'Ready').map(s => [s.name, s.id])); for (const session of this.sessions.values()) { if (readySandboxes.has(session.info.sandboxName)) { @@ -1340,4 +1343,10 @@ export class AcpSessionManager { // file may not exist } } + + async #listSandboxes(): Promise { + const client = await this.sdkClientManager.getClient(); + const refs = await client.sandbox.list(); + return refs.map(mapSdkSandboxRef); + } } diff --git a/packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts b/packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts index 2f5dbd5c5..38bcff15e 100644 --- a/packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts +++ b/packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts @@ -52,8 +52,8 @@ import type { Exec } from '/@/plugin/util/exec.js'; import type { AgentWorkspaceCreateOptions } from '/@api/agent-workspace-info.js'; import type { ApiSenderType } from '/@api/api-sender/api-sender-type.js'; import type { IConfigurationPropertyRecordedSchema, IConfigurationRegistry } from '/@api/configuration/models.js'; -import type { GatewayInfo, GatewaySandboxes } from '/@api/openshell-gateway-info.js'; -import { AGENT_LABEL, decodeWorkspaceLabels } from '/@api/openshell-gateway-info.js'; +import type { GatewayInfo } from '/@api/openshell-gateway-info.js'; +import { AGENT_LABEL, decodeWorkspaceLabels, WORKSPACE_LABEL } from '/@api/openshell-gateway-info.js'; import type { TaskState, TaskStatus } from '/@api/taskInfo.js'; import { AgentWorkspaceManager, encodeWorkspaceLabels } from './agent-workspace-manager.js'; @@ -67,23 +67,50 @@ vi.mock(import('/@/plugin/openshell-cli/openshell-policy-manager.js')); const openshellPolicyManager = new OpenshellPolicyManager({} as never); -const TEST_SUMMARIES: GatewaySandboxes[] = [ +const TEST_SDK_REFS: { + id: string; + name: string; + phase: string; + labels: Record; + resourceVersion: string; +}[] = [ { - gateway: { - name: 'kaiden', - endpoint: 'http://localhost:10080', - }, - sandboxes: [ - { id: 'ws-1', name: 'test-workspace-1', phase: 'Ready', sourcePath: '/tmp/ws1' }, - { - id: 'ws-2', - name: 'test-workspace-2', - phase: 'Ready', - }, - ], + id: 'ws-1', + name: 'test-workspace-1', + phase: 'ready', + labels: { [WORKSPACE_LABEL]: Buffer.from('/tmp/ws1').toString('base64url') }, + resourceVersion: '1', + }, + { + id: 'ws-2', + name: 'test-workspace-2', + phase: 'ready', + labels: {}, + resourceVersion: '2', }, ]; +const TEST_GATEWAY: GatewayInfo = { + name: 'kaiden', + endpoint: 'http://localhost:10080', +}; + +function mockSdkListSandboxes( + refs: { + id: string; + name: string; + phase: string; + labels: Record; + resourceVersion: string; + }[] = TEST_SDK_REFS, + gateways: GatewayInfo[] = [TEST_GATEWAY], +): void { + vi.mocked(openshellGatewayStateManager.listGateways).mockReturnValue(gateways); + vi.mocked(openshellSdkClientManager.getClient).mockResolvedValue({ + sandbox: { ...sdkSandbox, list: vi.fn().mockResolvedValue(refs) }, + } as never); +} + let manager: AgentWorkspaceManager; const apiSender: ApiSenderType = { @@ -95,6 +122,7 @@ const openshellCli = new OpenshellCli({} as Exec, {} as CliToolRegistry); const sdkSandbox = { create: vi.fn(), delete: vi.fn(), + list: vi.fn(), waitDeleted: vi.fn(), waitReady: vi.fn(), execInteractive: vi.fn(), @@ -146,7 +174,6 @@ const providerRegistry = { let gatewayStartCallback: (() => void) | undefined; let gatewayInitFailedCallback: ((message: string) => void) | undefined; let gatewayStateUpdateCallback: (() => void) | undefined; -let sandboxListChangeCallback: (() => void) | undefined; const openshellGateway = { createLocalGateway: vi.fn(), @@ -246,15 +273,6 @@ beforeEach(() => { gatewayStartCallback = undefined; gatewayInitFailedCallback = undefined; gatewayStateUpdateCallback = undefined; - sandboxListChangeCallback = undefined; - Object.defineProperty(openshellCli, 'onDidSandboxListChange', { - value: vi.fn((cb: () => void) => { - sandboxListChangeCallback = cb; - return { dispose: vi.fn() }; - }), - writable: true, - configurable: true, - }); manager = new AgentWorkspaceManager( apiSender, ipcHandle, @@ -368,15 +386,6 @@ describe('init', () => { gatewayStateUpdateCallback!(); expect(apiSender.send).toHaveBeenCalledWith('agent-gateway-update'); }); - - test('subscribes to sandbox list change event', () => { - expect(openshellCli.onDidSandboxListChange).toHaveBeenCalled(); - }); - - test('sends agent-workspace-update when sandbox list changes from polling', () => { - sandboxListChangeCallback!(); - expect(apiSender.send).toHaveBeenCalledWith('agent-workspace-update'); - }); }); test('rejects with a descriptive error when model is missing at runtime', async () => { @@ -1445,20 +1454,33 @@ describe('ensureModelSecret', () => { }); describe('list', () => { - test('delegates to kdnCli.list and returns items', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + test('delegates to SDK client and returns items', async () => { + mockSdkListSandboxes(); const result = await manager.listOpenshellSandboxes(); - expect(openshellCli.listSandboxesPerGateway).toHaveBeenCalled(); + expect(openshellGatewayStateManager.listGateways).toHaveBeenCalled(); + expect(openshellSdkClientManager.getClient).toHaveBeenCalledWith('kaiden'); expect(result).toHaveLength(1); expect(result.flatMap(gw => gw.sandboxes).map(s => s.id)).toEqual(['ws-1', 'ws-2']); }); - test('rejects when kdnCli.list fails', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockRejectedValue(new Error('command not found')); + test('returns empty sandboxes when SDK client fails for a gateway', async () => { + vi.mocked(openshellGatewayStateManager.listGateways).mockReturnValue([TEST_GATEWAY]); + vi.mocked(openshellSdkClientManager.getClient).mockRejectedValue(new Error('connection refused')); + + const result = await manager.listOpenshellSandboxes(); + + expect(result).toHaveLength(1); + expect(result[0]!.sandboxes).toEqual([]); + }); + + test('returns empty when no gateways exist', async () => { + vi.mocked(openshellGatewayStateManager.listGateways).mockReturnValue([]); - await expect(manager.listOpenshellSandboxes()).rejects.toThrow('command not found'); + const result = await manager.listOpenshellSandboxes(); + + expect(result).toEqual([]); }); }); @@ -1512,7 +1534,7 @@ describe('remove', () => { test('refreshes sandboxes while SDK deletion is still pending and keeps the task running', async () => { vi.useFakeTimers(); let finishDelete!: () => void; - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(sdkSandbox.delete).mockReturnValue( new Promise(resolve => { finishDelete = resolve; @@ -1534,25 +1556,25 @@ describe('remove', () => { expect(mockTask.state).toBe('completed'); expect(mockTask.status).toBe('success'); - expect(sdkSandbox.waitDeleted).not.toHaveBeenCalled(); + expect(sdkSandbox.waitDeleted).toHaveBeenCalledWith('test-workspace-1', 120); } finally { vi.useRealTimers(); } }); test('deletes through the SDK and returns the workspace id', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(sdkSandbox.delete).mockResolvedValue(undefined); const result = await manager.remove('ws-1', 'kaiden'); expect(sdkSandbox.delete).toHaveBeenCalledWith('test-workspace-1'); - expect(sdkSandbox.waitDeleted).not.toHaveBeenCalled(); + expect(sdkSandbox.waitDeleted).toHaveBeenCalledWith('test-workspace-1', 120); expect(result).toEqual({ id: 'ws-1' }); }); test('creates a task with workspace name and sets success status on completion', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(sdkSandbox.delete).mockResolvedValue(undefined); await manager.remove('ws-1', 'kaiden'); @@ -1563,7 +1585,7 @@ describe('remove', () => { }); test('uses workspace id as fallback when workspace not found in list', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue([]); + mockSdkListSandboxes([]); vi.mocked(sdkSandbox.delete).mockResolvedValue(undefined); await manager.remove('unknown-id', 'kaiden'); @@ -1572,7 +1594,7 @@ describe('remove', () => { }); test('sets task failure status when SDK deletion fails', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(sdkSandbox.delete).mockRejectedValue(new Error('workspace not found: unknown-id')); await expect(manager.remove('unknown-id', 'kaiden')).rejects.toThrow('workspace not found: unknown-id'); @@ -1583,7 +1605,7 @@ describe('remove', () => { }); test('preserves error detail in task error message', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(sdkSandbox.delete).mockRejectedValue(new Error('failed to remove workspace: permission denied')); await expect(manager.remove('ws-1', 'kaiden')).rejects.toThrow('failed to remove workspace: permission denied'); @@ -1592,7 +1614,7 @@ describe('remove', () => { }); test('emits agent-workspace-update event', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(sdkSandbox.delete).mockResolvedValue(undefined); await manager.remove('ws-1', 'kaiden'); @@ -1606,7 +1628,7 @@ describe('remove', () => { }); test('cleans up global config directory after sandbox deletion', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(sdkSandbox.delete).mockResolvedValue(undefined); await manager.remove('ws-1', 'kaiden'); @@ -1637,7 +1659,7 @@ describe('deleteOpenshellSandbox', () => { expect(openshellSdkClientManager.getClient).toHaveBeenCalledWith('remote-gateway'); expect(sdkSandbox.delete).toHaveBeenCalledWith('shared-name'); - expect(sdkSandbox.waitDeleted).not.toHaveBeenCalled(); + expect(sdkSandbox.waitDeleted).toHaveBeenCalledWith('shared-name', 120); }); test('cleans up global config directory after sandbox deletion', async () => { @@ -1654,18 +1676,18 @@ describe('deleteOpenshellSandbox', () => { describe('getConfiguration', () => { test('reads JSON configuration file from workspace directory', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(readFile).mockResolvedValue('{"mounts":{"dependencies":[]}}'); const result = await manager.getConfiguration('ws-1'); - expect(openshellCli.listSandboxesPerGateway).toHaveBeenCalled(); + expect(openshellGatewayStateManager.listGateways).toHaveBeenCalled(); expect(readFile).toHaveBeenCalledWith(join('/tmp/ws1/.kaiden', 'workspace.json'), 'utf-8'); expect(result).toEqual({ mounts: { dependencies: [] } }); }); test('throws when workspace id is not found in list', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); await expect(manager.getConfiguration('unknown-id')).rejects.toThrow( 'workspace "unknown-id" not found. Use "workspace list" to see available workspaces.', @@ -1673,7 +1695,7 @@ describe('getConfiguration', () => { }); test('returns empty configuration when file does not exist', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); const enoent = Object.assign(new Error('ENOENT: no such file'), { code: 'ENOENT' }); vi.mocked(readFile).mockRejectedValue(enoent); @@ -1683,7 +1705,7 @@ describe('getConfiguration', () => { }); test('rejects when reading the configuration file fails with a non-ENOENT error', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); const eacces = Object.assign(new Error('EACCES: permission denied'), { code: 'EACCES' }); vi.mocked(readFile).mockRejectedValue(eacces); @@ -1691,7 +1713,7 @@ describe('getConfiguration', () => { }); test('reads from global directory for no-folder workspaces', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(readFile).mockResolvedValue('{"network":{"mode":"allow"}}'); const result = await manager.getConfiguration('ws-2'); @@ -1704,7 +1726,7 @@ describe('getConfiguration', () => { }); test('returns empty config when global directory file does not exist for no-folder workspace', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(readFile).mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); const result = await manager.getConfiguration('ws-2'); @@ -1715,7 +1737,7 @@ describe('getConfiguration', () => { describe('updateConfiguration', () => { test('delegates to kdnCli.updateWorkspaceConfig with the workspace configuration path', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); const spy = vi.spyOn(configWriter, 'updateWorkspaceConfig'); await manager.updateConfiguration('ws-1', { skills: ['/path/to/skill'] }); @@ -1724,7 +1746,7 @@ describe('updateConfiguration', () => { }); test('emits agent-workspace-update event', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); await manager.updateConfiguration('ws-1', { network: { mode: 'allow' } }); @@ -1732,7 +1754,7 @@ describe('updateConfiguration', () => { }); test('throws when workspace id is not found', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); await expect(manager.updateConfiguration('unknown-id', {})).rejects.toThrow( 'workspace "unknown-id" not found. Use "workspace list" to see available workspaces.', @@ -1740,14 +1762,14 @@ describe('updateConfiguration', () => { }); test('propagates errors from kdnCli.updateWorkspaceConfig', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); vi.mocked(configWriter.updateWorkspaceConfig).mockRejectedValue(new Error('permission denied')); await expect(manager.updateConfiguration('ws-1', {})).rejects.toThrow('permission denied'); }); test('writes to global directory for no-folder workspaces', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); const spy = vi.spyOn(configWriter, 'updateWorkspaceConfig'); await manager.updateConfiguration('ws-2', { skills: ['/new/skill'] }); @@ -1919,7 +1941,7 @@ describe('dispose', () => { } test('closes active terminal sessions', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); const mockSession = createMockExecSessionForIpc(); sdkSandbox.execInteractive.mockResolvedValue(mockSession); @@ -1941,7 +1963,7 @@ describe('dispose', () => { }); test('terminal IPC handler rejects when workspace id is not found', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); const terminalHandler = vi .mocked(ipcHandle) @@ -1958,7 +1980,7 @@ describe('dispose', () => { }); test('does not send terminal data when webContents is destroyed', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); type StreamEvent = { stream: 'stdout' | 'stderr'; data: Buffer }; const events: StreamEvent[] = []; @@ -2039,7 +2061,7 @@ describe('terminal IPC session lifecycle', () => { } test('closes an active workspace terminal before opening a fresh one', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); const first = createTerminalMockExecSession(); sdkSandbox.execInteractive.mockResolvedValue(first.session); @@ -2063,7 +2085,7 @@ describe('terminal IPC session lifecycle', () => { }); test('routes send and resize through the workspace terminal session and removes it on close', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(TEST_SUMMARIES); + mockSdkListSandboxes(); const mock = createTerminalMockExecSession(); sdkSandbox.execInteractive.mockResolvedValue(mock.session); @@ -2097,23 +2119,21 @@ describe('terminal IPC session lifecycle', () => { }); describe('terminal agent command execution', () => { - const SANDBOXES_WITH_AGENT: GatewaySandboxes[] = [ + const SDK_REFS_WITH_AGENT: { + id: string; + name: string; + phase: string; + labels: Record; + resourceVersion: string; + }[] = [ { - gateway: { name: 'kaiden', endpoint: 'http://localhost:10080' }, - sandboxes: [ - { - id: 'ws-agent', - name: 'agent-workspace', - phase: 'Ready', - labels: { [AGENT_LABEL]: 'test-agent' }, - }, - { - id: 'ws-no-label', - name: 'no-label-workspace', - phase: 'Ready', - }, - ], + id: 'ws-agent', + name: 'agent-workspace', + phase: 'ready', + labels: { [AGENT_LABEL]: 'test-agent' }, + resourceVersion: '1', }, + { id: 'ws-no-label', name: 'no-label-workspace', phase: 'ready', labels: {}, resourceVersion: '2' }, ]; function createTerminalMockExecSession(): MockExecSession { @@ -2129,7 +2149,7 @@ describe('terminal agent command execution', () => { } test('executes agent command on first terminal data', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(SANDBOXES_WITH_AGENT); + mockSdkListSandboxes(SDK_REFS_WITH_AGENT); vi.mocked(agentRegistry.getAgent).mockResolvedValue({ id: 'test-agent', name: 'Test Agent', @@ -2147,7 +2167,7 @@ describe('terminal agent command execution', () => { }); test('does not execute agent command on subsequent connections', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(SANDBOXES_WITH_AGENT); + mockSdkListSandboxes(SDK_REFS_WITH_AGENT); vi.mocked(agentRegistry.getAgent).mockResolvedValue({ id: 'test-agent', name: 'Test Agent', @@ -2173,7 +2193,7 @@ describe('terminal agent command execution', () => { }); test('retries agent command when replaced before first terminal data', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(SANDBOXES_WITH_AGENT); + mockSdkListSandboxes(SDK_REFS_WITH_AGENT); vi.mocked(agentRegistry.getAgent).mockResolvedValue({ id: 'test-agent', name: 'Test Agent', @@ -2200,7 +2220,7 @@ describe('terminal agent command execution', () => { }); test('does not execute command when workspace has no agent label', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(SANDBOXES_WITH_AGENT); + mockSdkListSandboxes(SDK_REFS_WITH_AGENT); const mock = createTerminalMockExecSession(); sdkSandbox.execInteractive.mockResolvedValue(mock.session); @@ -2213,7 +2233,7 @@ describe('terminal agent command execution', () => { }); test('does not execute command when agent has no command', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(SANDBOXES_WITH_AGENT); + mockSdkListSandboxes(SDK_REFS_WITH_AGENT); vi.mocked(agentRegistry.getAgent).mockResolvedValue({ id: 'test-agent', name: 'Test Agent', @@ -2232,7 +2252,7 @@ describe('terminal agent command execution', () => { }); test('executes agent command only once despite multiple data events', async () => { - vi.mocked(openshellCli.listSandboxesPerGateway).mockResolvedValue(SANDBOXES_WITH_AGENT); + mockSdkListSandboxes(SDK_REFS_WITH_AGENT); vi.mocked(agentRegistry.getAgent).mockResolvedValue({ id: 'test-agent', name: 'Test Agent', diff --git a/packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts b/packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts index d3e513db2..fb2e2d684 100644 --- a/packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts +++ b/packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts @@ -36,6 +36,7 @@ import { OpenshellGatewayStateManager } from '/@/plugin/openshell-cli/openshell- import { buildPolicyObject, rewriteLocalhostUrl } from '/@/plugin/openshell-cli/openshell-network-policy.js'; import { OpenshellPolicyManager } from '/@/plugin/openshell-cli/openshell-policy-manager.js'; import { OpenshellSdkClientManager } from '/@/plugin/openshell-cli/openshell-sdk-client-manager.js'; +import { mapSdkSandboxRef } from '/@/plugin/openshell-cli/openshell-sdk-sandbox-mapper.js'; import { ProviderRegistry } from '/@/plugin/provider-registry.js'; import { SecretManager } from '/@/plugin/secret-manager/secret-manager.js'; import { TaskManager } from '/@/plugin/tasks/task-manager.js'; @@ -531,6 +532,12 @@ export class AgentWorkspaceManager implements Disposable { // delete doesnt log like create does so matched the convention here console.log(`[workspace-timing] deleteSandbox: deleting "${name}" on gateway "${gateway}"`); await sdkClient.sandbox.delete(name); + try { + await sdkClient.sandbox.waitDeleted(name, SANDBOX_DELETE_TIMEOUT_SECONDS); + } catch (waitErr: unknown) { + const detail = waitErr instanceof Error ? waitErr.message : String(waitErr); + console.warn(`[workspace-timing] deleteSandbox: waitDeleted failed for "${name}": ${detail}`); + } this.apiSender.send('agent-workspace-update'); if (terminalId) this.closeWorkspaceTerminal(terminalId); await rm(this.getGlobalConfigDir(gateway, name), { recursive: true, force: true }); @@ -612,12 +619,28 @@ export class AgentWorkspaceManager implements Disposable { } async listOpenshellSandboxes(): Promise { - const results = await this.openshellCli.listSandboxesPerGateway(); - for (const entry of results) { - for (const sandbox of entry.sandboxes) { - if (sandbox.labels) { - sandbox.sourcePath = decodeWorkspaceLabels(sandbox.labels); + const gateways = this.openshellGatewayStateManager.listGateways(); + if (gateways.length === 0) { + return []; + } + + const results: GatewaySandboxes[] = []; + for (const gateway of gateways) { + try { + const client = await this.openshellSdkClientManager.getClient(gateway.name); + const refs = await client.sandbox.list(); + const sandboxes: SandboxInfo[] = refs.map(mapSdkSandboxRef); + for (const sandbox of sandboxes) { + if (sandbox.labels) { + sandbox.sourcePath = decodeWorkspaceLabels(sandbox.labels); + } } + results.push({ gateway, sandboxes }); + } catch (err: unknown) { + console.warn( + `[openshell] failed to list sandboxes for gateway ${gateway.name}: ${err instanceof Error ? err.message : String(err)}`, + ); + results.push({ gateway, sandboxes: [] }); } } return results; @@ -959,12 +982,6 @@ export class AgentWorkspaceManager implements Disposable { this.apiSender.send('agent-gateway-update'); }), ); - - this.disposables.push( - this.openshellCli.onDidSandboxListChange(() => { - this.apiSender.send('agent-workspace-update'); - }), - ); } @preDestroy() diff --git a/packages/main/src/plugin/openshell-cli/openshell-cli.spec.ts b/packages/main/src/plugin/openshell-cli/openshell-cli.spec.ts index 1823973e4..90049dd38 100644 --- a/packages/main/src/plugin/openshell-cli/openshell-cli.spec.ts +++ b/packages/main/src/plugin/openshell-cli/openshell-cli.spec.ts @@ -20,7 +20,7 @@ import { existsSync } from 'node:fs'; import { join } from 'node:path'; import type { RunError, RunResult } from '@openkaiden/api'; -import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; import type { CliToolRegistry } from '/@/plugin/cli-tool-registry.js'; import type { Proxy } from '/@/plugin/proxy.js'; @@ -115,25 +115,6 @@ describe('getVersion', () => { }); }); -describe('listSandboxes', () => { - test('executes openshell sandbox list with json output', async () => { - const payload = [{ id: 'sb-1', name: 'sb-1', phase: 'Ready' }]; - vi.mocked(exec.exec).mockResolvedValue(mockExecResult(JSON.stringify(payload))); - - const result = await openshellCli.listSandboxes(); - - expect(exec.exec).toHaveBeenCalledWith(OPENSHELL_CLI_PATH, ['sandbox', 'list', '-o', 'json'], undefined); - expect(result).toEqual(payload); - }); - - test('rejects when CLI fails', async () => { - vi.spyOn(console, 'error').mockImplementation(() => undefined); - vi.mocked(exec.exec).mockRejectedValue(new Error('command not found')); - - await expect(openshellCli.listSandboxes()).rejects.toThrow('command not found'); - }); -}); - describe('startSandbox', () => { test('executes openshell sandbox start with name', async () => { vi.spyOn(console, 'log').mockImplementation(() => undefined); @@ -402,277 +383,6 @@ describe('getGatewayInfo', () => { }); }); -describe('listSandboxesForGateway', () => { - test('lists sandboxes for a specific gateway using -g flag', async () => { - const gateways = [ - { name: 'gw-1', endpoint: 'https://gw1.example.com', active: true }, - { name: 'gw-2', endpoint: 'https://gw2.example.com', active: false }, - ]; - const sandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Ready' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(sandboxes))); - - const result = await openshellCli.listSandboxesForGateway('gw-2'); - - expect(result.gateway.name).toBe('gw-2'); - expect(result.sandboxes).toEqual(sandboxes); - expect(exec.exec).toHaveBeenCalledWith( - OPENSHELL_CLI_PATH, - ['sandbox', 'list', '-g', 'gw-2', '-o', 'json'], - undefined, - ); - expect(exec.exec).not.toHaveBeenCalledWith(OPENSHELL_CLI_PATH, expect.arrayContaining(['gateway', 'select'])); - }); - - test('throws when gateway is not found', async () => { - const gateways = [{ name: 'gw-1', endpoint: 'https://gw1.example.com', active: true }]; - vi.mocked(exec.exec).mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))); - - await expect(openshellCli.listSandboxesForGateway('unknown')).rejects.toThrow('Gateway not found: unknown'); - }); -}); - -describe('listSandboxesPerGateway', () => { - test('returns sandboxes for each gateway using -g flag', async () => { - const gateways = [ - { name: 'gw-1', endpoint: 'https://gw1.example.com', active: true }, - { name: 'gw-2', endpoint: 'https://gw2.example.com', active: false }, - ]; - const sandboxes1 = [{ id: 'sb-1', name: 'sb-1', phase: 'Ready' }]; - const sandboxes2 = [{ id: 'sb-2', name: 'sb-2', phase: 'Unknown' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(sandboxes1))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(sandboxes2))); - - const results = await openshellCli.listSandboxesPerGateway(); - - expect(results).toHaveLength(2); - const [first, second] = results; - expect(first?.gateway.name).toBe('gw-1'); - expect(first?.sandboxes).toEqual(sandboxes1); - expect(second?.gateway.name).toBe('gw-2'); - expect(second?.sandboxes).toEqual(sandboxes2); - expect(exec.exec).toHaveBeenCalledWith( - OPENSHELL_CLI_PATH, - ['sandbox', 'list', '-g', 'gw-1', '-o', 'json'], - undefined, - ); - expect(exec.exec).toHaveBeenCalledWith( - OPENSHELL_CLI_PATH, - ['sandbox', 'list', '-g', 'gw-2', '-o', 'json'], - undefined, - ); - expect(exec.exec).not.toHaveBeenCalledWith(OPENSHELL_CLI_PATH, expect.arrayContaining(['gateway', 'select'])); - }); - - test('returns empty array when no gateways exist', async () => { - vi.mocked(exec.exec).mockResolvedValueOnce(mockExecResult(JSON.stringify([]))); - - const results = await openshellCli.listSandboxesPerGateway(); - - expect(results).toEqual([]); - }); - - test('returns empty sandboxes for a gateway that fails to list', async () => { - vi.spyOn(console, 'warn').mockImplementation(() => undefined); - vi.spyOn(console, 'error').mockImplementation(() => undefined); - const gateways = [{ name: 'gw-1', endpoint: 'https://gw1.example.com', active: true }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockRejectedValueOnce(new Error('connection refused')); - - const results = await openshellCli.listSandboxesPerGateway(); - - expect(results).toHaveLength(1); - expect(results.at(0)?.sandboxes).toEqual([]); - }); -}); - -describe('listSandboxesPerGateway transitional auto-refresh', () => { - const gateways = [{ name: 'gw-1', endpoint: 'https://gw1.example.com', active: true }]; - - beforeEach(() => { - vi.useFakeTimers(); - }); - - afterEach(() => { - openshellCli.dispose(); - vi.useRealTimers(); - }); - - test('schedules re-list 5s after detecting a Deleting sandbox and fires emitter', async () => { - const deletingSandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Deleting' }]; - const readySandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Ready' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(readySandboxes))); - - const listener = vi.fn(); - openshellCli.onDidSandboxListChange(listener); - - await openshellCli.listSandboxesPerGateway(); - - expect(listener).not.toHaveBeenCalled(); - - await vi.advanceTimersByTimeAsync(5000); - - expect(listener).toHaveBeenCalledOnce(); - expect(listener).toHaveBeenCalledWith([{ gateway: gateways[0], sandboxes: readySandboxes }]); - }); - - test('schedules re-list 5s after detecting a Provisioning sandbox and fires emitter', async () => { - const provisioningSandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Provisioning' }]; - const readySandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Ready' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(provisioningSandboxes))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(readySandboxes))); - - const listener = vi.fn(); - openshellCli.onDidSandboxListChange(listener); - - await openshellCli.listSandboxesPerGateway(); - - expect(listener).not.toHaveBeenCalled(); - - await vi.advanceTimersByTimeAsync(5000); - - expect(listener).toHaveBeenCalledOnce(); - expect(listener).toHaveBeenCalledWith([{ gateway: gateways[0], sandboxes: readySandboxes }]); - }); - - test('does not schedule poll when no sandbox is in a transitional state', async () => { - const readySandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Ready' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(readySandboxes))); - - const listener = vi.fn(); - openshellCli.onDidSandboxListChange(listener); - - await openshellCli.listSandboxesPerGateway(); - await vi.advanceTimersByTimeAsync(10000); - - expect(listener).not.toHaveBeenCalled(); - }); - - test('does not schedule a second timer if one is already pending', async () => { - const deletingSandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Deleting' }]; - const readySandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Ready' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(readySandboxes))); - - const listener = vi.fn(); - openshellCli.onDidSandboxListChange(listener); - - await openshellCli.listSandboxesPerGateway(); - await openshellCli.listSandboxesPerGateway(); - - await vi.advanceTimersByTimeAsync(5000); - - expect(listener).toHaveBeenCalledOnce(); - }); - - test('does not fire emitter when sandbox phases are unchanged', async () => { - const deletingSandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Deleting' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))); - - const listener = vi.fn(); - openshellCli.onDidSandboxListChange(listener); - - await openshellCli.listSandboxesPerGateway(); - await vi.advanceTimersByTimeAsync(5000); - - expect(listener).not.toHaveBeenCalled(); - }); - - test('continues polling while transitional sandboxes remain and fires only on change', async () => { - const deletingSandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Deleting' }]; - const readySandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Ready' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(readySandboxes))); - - const listener = vi.fn(); - openshellCli.onDidSandboxListChange(listener); - - await openshellCli.listSandboxesPerGateway(); - - await vi.advanceTimersByTimeAsync(5000); - expect(listener).not.toHaveBeenCalled(); - - await vi.advanceTimersByTimeAsync(5000); - expect(listener).toHaveBeenCalledOnce(); - - await vi.advanceTimersByTimeAsync(5000); - expect(listener).toHaveBeenCalledOnce(); - }); - - test('handles errors in the polling re-list gracefully', async () => { - const deletingSandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Deleting' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))) - .mockRejectedValueOnce(new Error('connection lost')); - - const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); - vi.spyOn(console, 'error').mockImplementation(() => undefined); - const listener = vi.fn(); - openshellCli.onDidSandboxListChange(listener); - - await openshellCli.listSandboxesPerGateway(); - await vi.advanceTimersByTimeAsync(5000); - - expect(listener).not.toHaveBeenCalled(); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('transitional-poll refresh failed')); - }); - - test('dispose clears pending poll timer', async () => { - const deletingSandboxes = [{ id: 'sb-1', name: 'sb-1', phase: 'Deleting' }]; - - vi.mocked(exec.exec) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(gateways))) - .mockResolvedValueOnce(mockExecResult(JSON.stringify(deletingSandboxes))); - - const listener = vi.fn(); - openshellCli.onDidSandboxListChange(listener); - - await openshellCli.listSandboxesPerGateway(); - openshellCli.dispose(); - await vi.advanceTimersByTimeAsync(10000); - - expect(listener).not.toHaveBeenCalled(); - }); -}); - describe('checkEndpointStatus', () => { test('returns true when endpoint is healthy', async () => { vi.mocked(exec.exec).mockResolvedValue(mockExecResult('')); diff --git a/packages/main/src/plugin/openshell-cli/openshell-cli.ts b/packages/main/src/plugin/openshell-cli/openshell-cli.ts index 48bc6fbc2..180c24dfd 100644 --- a/packages/main/src/plugin/openshell-cli/openshell-cli.ts +++ b/packages/main/src/plugin/openshell-cli/openshell-cli.ts @@ -20,13 +20,11 @@ import { existsSync } from 'node:fs'; import { join } from 'node:path'; import type { RunError, RunOptions } from '@openkaiden/api'; -import { inject, injectable, preDestroy } from 'inversify'; +import { inject, injectable } from 'inversify'; import z from 'zod'; import { CliToolRegistry } from '/@/plugin/cli-tool-registry.js'; -import { Emitter } from '/@/plugin/events/emitter.js'; import { Exec } from '/@/plugin/util/exec.js'; -import type { Event } from '/@api/event.js'; import { type CreateProviderOptions, type GatewayAddOptions, @@ -34,13 +32,10 @@ import { GatewayInfoSchema, type GatewayRuntimeInfo, GatewayRuntimeInfoSchema, - type GatewaySandboxes, type OpenshellProfile, OpenshellProfileSchema, type OpenshellProviderInfo, OpenshellProviderInfoSchema, - type SandboxInfo, - SandboxInfoSchema, type SetInferenceOptions, } from '/@api/openshell-gateway-info.js'; @@ -61,7 +56,6 @@ const OpenshellSettingsSchema = z.looseObject({ * Low-level wrapper around the `openshell` CLI binary. * * Sandbox commands: - * - `openshell sandbox list` * - `openshell sandbox start` * - `openshell sandbox stop` * - `openshell sandbox connect` @@ -82,16 +76,8 @@ const OpenshellSettingsSchema = z.looseObject({ * - `openshell provider delete ` * - `openshell provider create` */ -const TRANSITIONAL_PHASES = new Set(['Deleting', 'Provisioning']); -const TRANSITIONAL_POLL_INTERVAL_MS = 5_000; -const MAX_TRANSITIONAL_POLL_RETRIES = 3; - @injectable() export class OpenshellCli { - private readonly _onDidSandboxListChange = new Emitter(); - readonly onDidSandboxListChange: Event = this._onDidSandboxListChange.event; - private _transitionalPollTimer: ReturnType | undefined; - constructor( @inject(Exec) private readonly exec: Exec, @@ -167,15 +153,6 @@ export class OpenshellCli { // ── sandbox commands ────────────────────────────────────────────── - async listSandboxes(gatewayName?: string): Promise { - const args = ['sandbox', 'list']; - if (gatewayName) { - args.push('-g', gatewayName); - } - const data = await this.execCLI(args); - return z.array(SandboxInfoSchema).parse(data); - } - async startSandbox(name: string): Promise { await this.runCli(['sandbox', 'start', name]); } @@ -204,76 +181,6 @@ export class OpenshellCli { await this.runCli(args); } - async listSandboxesForGateway(gatewayName: string): Promise { - const gateways = await this.listGateways(); - const targetGateway = gateways.find(g => g.name === gatewayName); - if (!targetGateway) { - throw new Error(`Gateway not found: ${gatewayName}`); - } - - const sandboxes = await this.listSandboxes(gatewayName); - return { gateway: targetGateway, sandboxes }; - } - - async listSandboxesPerGateway(): Promise { - const gateways = await this.listGateways(); - if (gateways.length === 0) { - return []; - } - - const results: GatewaySandboxes[] = []; - for (const gateway of gateways) { - try { - const sandboxes = await this.listSandboxes(gateway.name); - results.push({ gateway, sandboxes }); - } catch (err: unknown) { - console.warn( - `[openshell] failed to list sandboxes for gateway ${gateway.name}: ${err instanceof Error ? err.message : String(err)}`, - ); - results.push({ gateway, sandboxes: [] }); - } - } - - this.scheduleTransitionalPollIfNeeded(results); - return results; - } - - private snapshotPhases(sandboxes: SandboxInfo[]): string { - return sandboxes - .map(s => `${s.id}:${s.phase}`) - .sort((a, b) => a.localeCompare(b)) - .join(','); - } - - private scheduleTransitionalPollIfNeeded(results: GatewaySandboxes[], retries = 0): void { - const allSandboxes = results.flatMap(entry => entry.sandboxes); - const transitionalCount = allSandboxes.filter(s => TRANSITIONAL_PHASES.has(s.phase)).length; - if ( - transitionalCount === 0 || - retries > MAX_TRANSITIONAL_POLL_RETRIES || - this._transitionalPollTimer !== undefined - ) { - return; - } - const previousSnapshot = this.snapshotPhases(allSandboxes); - this._transitionalPollTimer = setTimeout(() => { - this._transitionalPollTimer = undefined; - this.listSandboxesPerGateway() - .then(updated => { - const updatedSandboxes = updated.flatMap(entry => entry.sandboxes); - if (this.snapshotPhases(updatedSandboxes) !== previousSnapshot) { - this._onDidSandboxListChange.fire(updated); - } - }) - .catch((err: unknown) => { - console.warn( - `[openshell] transitional-poll refresh failed: ${err instanceof Error ? err.message : String(err)}`, - ); - this.scheduleTransitionalPollIfNeeded(results, retries + 1); - }); - }, TRANSITIONAL_POLL_INTERVAL_MS); - } - // ── gateway registration commands ───────────────────────────────── async addGateway(options: GatewayAddOptions): Promise { @@ -459,13 +366,4 @@ export class OpenshellCli { throw new Error(detail); } } - - @preDestroy() - dispose(): void { - if (this._transitionalPollTimer !== undefined) { - clearTimeout(this._transitionalPollTimer); - this._transitionalPollTimer = undefined; - } - this._onDidSandboxListChange.dispose(); - } } diff --git a/packages/main/src/plugin/openshell-cli/openshell-sdk-client-manager.spec.ts b/packages/main/src/plugin/openshell-cli/openshell-sdk-client-manager.spec.ts index 16763ce65..2eed6a675 100644 --- a/packages/main/src/plugin/openshell-cli/openshell-sdk-client-manager.spec.ts +++ b/packages/main/src/plugin/openshell-cli/openshell-sdk-client-manager.spec.ts @@ -101,6 +101,28 @@ describe('OpenshellSdkClientManager', () => { expect(mockConnect).toHaveBeenCalledTimes(1); }); + test('concurrent calls for same gateway share a single connection attempt', async () => { + const gw = gateway(); + const sdkClient = createSdkClient(async () => [gw]); + + const [first, second] = await Promise.all([sdkClient.getClient(), sdkClient.getClient()]); + + expect(first).toBe(second); + expect(mockConnect).toHaveBeenCalledTimes(1); + }); + + test('evicts cached promise when connect rejects', async () => { + const gw = gateway(); + const sdkClient = createSdkClient(async () => [gw]); + mockConnect.mockRejectedValueOnce(new Error('connection refused')); + + await expect(sdkClient.getClient()).rejects.toThrow('connection refused'); + + mockConnect.mockResolvedValueOnce({ sandbox: {}, raw: {}, transport: {} }); + await expect(sdkClient.getClient()).resolves.toBeDefined(); + expect(mockConnect).toHaveBeenCalledTimes(2); + }); + test('creates separate clients for different gateways', async () => { const localGw = gateway({ name: 'local', endpoint: 'http://127.0.0.1:17670', active: true }); const remoteGw = gateway({ name: 'remote', endpoint: 'http://10.0.0.1:17670', active: false }); diff --git a/packages/main/src/plugin/openshell-cli/openshell-sdk-client-manager.ts b/packages/main/src/plugin/openshell-cli/openshell-sdk-client-manager.ts index 69533b85f..d09fa375b 100644 --- a/packages/main/src/plugin/openshell-cli/openshell-sdk-client-manager.ts +++ b/packages/main/src/plugin/openshell-cli/openshell-sdk-client-manager.ts @@ -32,7 +32,7 @@ import type { GatewayInfo } from '/@api/openshell-gateway-info.js'; */ @injectable() export class OpenshellSdkClientManager { - readonly #cache = new Map(); + readonly #cache = new Map>(); constructor( @inject(OpenshellCli) @@ -49,11 +49,14 @@ export class OpenshellSdkClientManager { return cached; } - const { OpenShellClient: ClientClass } = await import('@nvidia/openshell-sdk'); - const options = await this.gatewayConfig.buildConnectOptions(gateway); - const client = await ClientClass.connect(options); - this.#cache.set(gateway.name, client); - return client; + const connecting = this.#connect(gateway); + this.#cache.set(gateway.name, connecting); + try { + return await connecting; + } catch (err: unknown) { + this.#cache.delete(gateway.name); + throw err; + } } invalidate(gatewayName?: string): void { @@ -69,6 +72,12 @@ export class OpenshellSdkClientManager { this.#cache.clear(); } + async #connect(gateway: GatewayInfo): Promise { + const { OpenShellClient: ClientClass } = await import('@nvidia/openshell-sdk'); + const options = await this.gatewayConfig.buildConnectOptions(gateway); + return ClientClass.connect(options); + } + async #resolveGateway(gatewayName?: string): Promise { const gateways = await this.openshellCli.listGateways(); diff --git a/packages/main/src/plugin/openshell-cli/openshell-sdk-sandbox-mapper.ts b/packages/main/src/plugin/openshell-cli/openshell-sdk-sandbox-mapper.ts new file mode 100644 index 000000000..3d911b17b --- /dev/null +++ b/packages/main/src/plugin/openshell-cli/openshell-sdk-sandbox-mapper.ts @@ -0,0 +1,47 @@ +/********************************************************************** + * Copyright (C) 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ + +import type { SandboxPhaseName, SandboxRef } from '@nvidia/openshell-sdk'; + +import type { SandboxInfo } from '/@api/openshell-gateway-info.js'; + +const SDK_PHASE_MAP: Record = { + unspecified: 'Unspecified', + provisioning: 'Provisioning', + ready: 'Ready', + error: 'Error', + deleting: 'Deleting', + unknown: 'Unknown', + starting: 'Starting', + stopping: 'Stopping', + stopped: 'Stopped', +}; + +/** + * Maps an OpenShell SDK {@link SandboxRef} to Kaiden's {@link SandboxInfo}. + * Converts the SDK's lowercase phase names to PascalCase. + */ +export function mapSdkSandboxRef(ref: SandboxRef): SandboxInfo { + return { + id: ref.id, + name: ref.name, + phase: SDK_PHASE_MAP[ref.phase] ?? 'Unknown', + labels: ref.labels, + resource_version: Number(ref.resourceVersion), + }; +}