From 69d5efe31e7807663e2d8ae16de6dc9ff64dfb8b Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Thu, 6 Aug 2026 15:47:31 +0000 Subject: [PATCH 01/15] fix(switchdash): offer to create a missing remote working directory (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a Switch agent on a remote host failed with a bare `FileSystemError: File or directory not found: ` when the chosen working directory's parent did not exist on the host. `SshFileSystem` is rooted at the agent's working directory, and its recursive mkdir walks up only as far as that root — so it could create the working directory itself but not its ancestors. The first credentials write therefore died on the containment guard, after `addAgent` had already minted an identity on the gateway, leaving an agent that existed in Switch and nowhere else. The guard is a sandbox boundary and stays as it is. Instead the directory is inspected before anything is minted: - `inspectRemoteDir` / `createRemoteDir` open an SSH filesystem at `/` and are the only callers allowed to reach past a working directory's containment. Inspection reports how many path segments are missing, which is what distinguishes "not created yet" from a typo. - The add-agent modal probes when the working directory is committed and shows an inline notice with a "Create directory" action, so a bad path is caught while the field is still on screen. Submit is gated on a usable directory. - `addAgent` performs the same check before `registerAgentIdentity` and returns a typed `directory-missing`, so the guarantee does not depend on the UI. Creation as a whole is still not atomic (CHOO-1415). Creating the directory is offered, never assumed: `mkdir -p` on whatever was typed would turn a typo into a real directory as silently as the old error was abrupt. Also removes `switchServers.provisionAgent` / `provisionRemoteAgent`, whose last caller went away with the flat-agent rework in CHOO-1440, along with the parameter types that only they used. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/main/core/agents/add-agent.test.ts | 65 +++++++ .../src/main/core/agents/add-agent.ts | 20 ++ .../src/main/core/agents/remote-dir.test.ts | 178 ++++++++++++++++++ .../src/main/core/agents/remote-dir.ts | 126 +++++++++++++ .../src/main/core/fs/impl/ssh-fs.test.ts | 42 ++++- .../src/main/core/remote-hosts/controller.ts | 15 ++ .../core/switch-servers/controller.test.ts | 7 - .../main/core/switch-servers/controller.ts | 79 -------- .../add-agent-modal/add-agent-modal.tsx | 61 +++++- .../add-agent-modal/remote-dir-notice.tsx | 122 ++++++++++++ .../shared/core/remote-hosts/remote-dir.ts | 43 +++++ .../core/switch-servers/switch-servers.ts | 55 +----- 12 files changed, 669 insertions(+), 144 deletions(-) create mode 100644 dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts create mode 100644 dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts create mode 100644 dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx create mode 100644 dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts index 8440b1b41..38853a3f0 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts @@ -43,6 +43,12 @@ const h = vi.hoisted(() => { apiKey: 'tok-123', })), createAgent: vi.fn(async (input: Record) => ({ ...input })), + inspectRemoteDir: vi.fn(async (_host: string, dir: string) => ({ + dir, + status: 'directory' as const, + existingAncestor: '', + missingSegments: [], + })), }; }); @@ -65,6 +71,9 @@ vi.mock('@main/core/locations/store', () => ({ getLocationByHostDir: vi.fn(async () => ({ id: 'loc-1' })), })); vi.mock('./agent-name-taken', () => ({ agentNameTaken: h.agentNameTaken })); +// Also keeps ssh-fs (and through it the Electron-bound db) out of this file's +// module graph. +vi.mock('./remote-dir', () => ({ inspectRemoteDir: h.inspectRemoteDir })); vi.mock('@main/core/locations/path-utils', () => ({ checkIsValidDirectory: () => true })); vi.mock('@main/core/locations/location-manager', () => ({ locationManager: { openLocation: vi.fn(async () => {}) }, @@ -105,6 +114,12 @@ describe('addAgent', () => { h.state.repoAgents = { writeDefinition: h.writeDefinition }; h.state.workspace = fakeFs(); h.registerAgentIdentity.mockResolvedValue({ kind: 'created', id: 'sw-1', apiKey: 'tok-123' }); + h.inspectRemoteDir.mockImplementation(async (_host: string, dir: string) => ({ + dir, + status: 'directory' as const, + existingAncestor: '', + missingSegments: [], + })); }); it('writes name-keyed credentials for a provider with no repo-agent definitions', async () => { @@ -176,4 +191,54 @@ describe('addAgent', () => { expect(h.registerAgentIdentity).not.toHaveBeenCalled(); expect(h.createAgent).not.toHaveBeenCalled(); }); + + describe('remote working directory', () => { + const remote = { sshHost: 'louis-1-cluster', dir: '/home/ubuntu/switch-agents/deploys' }; + + it('reports a missing remote directory without minting an identity', async () => { + // The whole point of checking first: this used to surface as a raw + // FileSystemError from the first credentials write, leaving the agent on + // the gateway but nowhere else (CHOO-1416). + h.inspectRemoteDir.mockResolvedValue({ + dir: remote.dir, + status: 'missing', + existingAncestor: '/home/ubuntu', + missingSegments: ['switch-agents', 'deploys'], + } as never); + + const result = await addAgent(params(remote)); + + expect(result).toEqual({ + kind: 'directory-missing', + sshHost: remote.sshHost, + inspection: expect.objectContaining({ status: 'missing' }), + }); + expect(h.registerAgentIdentity).not.toHaveBeenCalled(); + expect(h.createAgent).not.toHaveBeenCalled(); + }); + + it('refuses a remote path that is a file', async () => { + h.inspectRemoteDir.mockResolvedValue({ + dir: remote.dir, + status: 'file', + existingAncestor: '', + missingSegments: [], + } as never); + + expect((await addAgent(params(remote))).kind).toBe('directory-missing'); + expect(h.registerAgentIdentity).not.toHaveBeenCalled(); + }); + + it('proceeds when the remote directory exists', async () => { + const result = await addAgent(params(remote)); + + expect(h.inspectRemoteDir).toHaveBeenCalledWith(remote.sshHost, remote.dir); + expect(result.kind).toBe('created'); + }); + + it('does not probe over SSH for a local agent', async () => { + await addAgent(params()); + expect(h.inspectRemoteDir).not.toHaveBeenCalled(); + }); + }); }); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts index f1970368e..9ace6518e 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts @@ -9,6 +9,7 @@ import { log } from '@main/lib/logger'; import type { AgentProviderConfig } from '@shared/core/agents/agent-provider-config'; import type { Agent } from '@shared/core/agents/agents'; import type { AgentProviderId } from '@shared/core/providers/agent-provider-registry'; +import type { RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; import { basenameFromAnyPath } from '@shared/path-name'; import { agentEvents } from './agent-events'; import { agentNameTaken } from './agent-name-taken'; @@ -16,6 +17,7 @@ import { resolveWorkspaceFsFor } from './agent-workspace-fs'; import { createAgent } from './createAgent'; import { knownAgentTypeForProvider } from './known-agent-type'; import { registerAgentIdentity } from './register-agent-identity'; +import { inspectRemoteDir } from './remote-dir'; import { reconcileAgentAutoSessionFromGateway } from './setAgentAutoSession'; import { writeNeutralAgentSettingsFs } from './write-switch-settings'; @@ -51,6 +53,10 @@ export type AddAgentResult = | { kind: 'unauthenticated' } | { kind: 'name-conflict' } | { kind: 'invalid-name'; message: string } + /** The remote working directory does not exist (or is a file). Recoverable: + * the caller offers to create it and retries. Reported before anything is + * minted, so no Switch-side agent is left behind (CHOO-1416). */ + | { kind: 'directory-missing'; sshHost: string; inspection: RemoteDirInspection } | { kind: 'error'; message: string }; /** @@ -65,11 +71,25 @@ export type AddAgentResult = * written to disk and never returned. A recoverable gateway failure is mapped to * a typed result the modal can act on; a filesystem failure after registration * throws (leaving the gateway agent, as the pre-existing provision path did). + * + * Both run locations therefore check the working directory *before* minting the + * identity — locally with `checkIsValidDirectory`, remotely with + * `inspectRemoteDir`. A missing remote directory used to surface as a raw + * `FileSystemError` from the first credentials write, by which point the agent + * existed on the gateway but nowhere else (CHOO-1416). Ordering the check first + * removes that orphan for this failure; creation as a whole is still not atomic + * (CHOO-1415). */ export async function addAgent(params: AddAgentParams): Promise { if (params.sshHost === null && !checkIsValidDirectory(params.dir)) { return { kind: 'error', message: `Invalid directory: ${params.dir}` }; } + if (params.sshHost !== null) { + const inspection = await inspectRemoteDir(params.sshHost, params.dir); + if (inspection.status !== 'directory') { + return { kind: 'directory-missing', sshHost: params.sshHost, inspection }; + } + } const server = await getServer(params.serverId); if (!server) return { kind: 'error', message: `No Switch server with id ${params.serverId}` }; diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts new file mode 100644 index 000000000..ff377ec77 --- /dev/null +++ b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts @@ -0,0 +1,178 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { FileSystemError, FileSystemErrorCodes } from '@main/core/fs/types'; + +const stat = vi.hoisted(() => vi.fn()); +const mkdir = vi.hoisted(() => vi.fn()); +const close = vi.hoisted(() => vi.fn()); +const constructedWith = vi.hoisted(() => [] as string[]); + +vi.mock('@main/core/fs/impl/ssh-fs', () => ({ + SshFileSystem: class { + constructor(_proxy: unknown, base: string) { + constructedWith.push(base); + } + stat = stat; + mkdir = mkdir; + close = close; + }, +})); +vi.mock('@main/core/locations/location-transport', () => ({ + sshConnectionIdForHost: (host: string) => `conn:${host}`, +})); +vi.mock('@main/core/ssh/connect/connect-agent-ssh', () => ({ + ensureSshConnected: vi.fn(async () => ({})), +})); +vi.mock('@main/lib/logger', () => ({ log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() } })); + +const { createRemoteDir, inspectRemoteDir } = await import('./remote-dir'); + +/** Report `paths` as existing directories and everything else as absent. */ +function existingDirs(paths: string[]) { + stat.mockImplementation(async (path: string) => + paths.includes(path) ? { path, type: 'dir' } : null + ); +} + +const REPO_DIR = '/home/ubuntu/switch-agents/internal-deployments'; + +beforeEach(() => { + vi.clearAllMocks(); + constructedWith.length = 0; +}); + +describe('inspectRemoteDir', () => { + it('reports an existing directory as usable', async () => { + existingDirs([REPO_DIR]); + + expect(await inspectRemoteDir('host', REPO_DIR)).toEqual({ + dir: REPO_DIR, + status: 'directory', + existingAncestor: '', + missingSegments: [], + }); + }); + + // The ticket's repro: the directory *and* its parent are absent, which is + // what the per-directory FS could not recover from (CHOO-1416). + it('names every missing segment when several ancestors are absent', async () => { + existingDirs(['/home/ubuntu']); + + expect(await inspectRemoteDir('host', REPO_DIR)).toEqual({ + dir: REPO_DIR, + status: 'missing', + existingAncestor: '/home/ubuntu', + missingSegments: ['switch-agents', 'internal-deployments'], + }); + }); + + it('reports a single missing leaf under an existing parent', async () => { + existingDirs(['/home/ubuntu/switch-agents']); + + expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ + status: 'missing', + existingAncestor: '/home/ubuntu/switch-agents', + missingSegments: ['internal-deployments'], + }); + }); + + it('falls back to the root when no ancestor exists', async () => { + existingDirs([]); + + expect(await inspectRemoteDir('host', '/srv/agent')).toMatchObject({ + status: 'missing', + existingAncestor: '/', + missingSegments: ['srv', 'agent'], + }); + }); + + it('reports a path that is a file rather than offering to create it', async () => { + stat.mockImplementation(async (path: string) => + path === REPO_DIR ? { path, type: 'file' } : null + ); + + expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ + dir: REPO_DIR, + status: 'file', + }); + }); + + it('blames the ancestor when a parent is a file', async () => { + stat.mockImplementation(async (path: string) => { + if (path === '/home/ubuntu/switch-agents') return { path, type: 'file' }; + return null; + }); + + expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ + dir: '/home/ubuntu/switch-agents', + status: 'file', + }); + }); + + // Treating an unreadable path as missing would offer to create a directory + // that is already there, and the create would fail the same way the probe did. + it('propagates a probe failure that is not absence', async () => { + stat.mockRejectedValue( + new FileSystemError('Permission denied: /home', FileSystemErrorCodes.PERMISSION_DENIED) + ); + + await expect(inspectRemoteDir('host', REPO_DIR)).rejects.toThrow('Permission denied'); + }); + + it('rejects a relative path rather than resolving it against the login dir', async () => { + await expect(inspectRemoteDir('host', 'switch-agents/repo')).rejects.toThrow( + 'must be an absolute path' + ); + expect(stat).not.toHaveBeenCalled(); + }); + + it('normalises a trailing slash', async () => { + existingDirs([REPO_DIR]); + + expect(await inspectRemoteDir('host', `${REPO_DIR}/`)).toMatchObject({ dir: REPO_DIR }); + }); + + it('closes the SFTP channel even when the probe throws', async () => { + stat.mockRejectedValue(new Error('boom')); + + await expect(inspectRemoteDir('host', REPO_DIR)).rejects.toThrow('boom'); + expect(close).toHaveBeenCalled(); + }); +}); + +describe('createRemoteDir', () => { + it('creates the directory and its missing parents from the filesystem root', async () => { + existingDirs(['/home/ubuntu']); + + await createRemoteDir('host', REPO_DIR); + + expect(mkdir).toHaveBeenCalledWith(REPO_DIR, { recursive: true }); + // Rooting at `/` is what lets the ancestors be created at all — an FS + // rooted at the repo dir cannot create its own parents. + expect(constructedWith).toEqual(['/', '/']); + }); + + it('is a no-op when the directory already exists', async () => { + existingDirs([REPO_DIR]); + + await createRemoteDir('host', REPO_DIR); + + expect(mkdir).not.toHaveBeenCalled(); + }); + + it('refuses to create over an existing file', async () => { + stat.mockImplementation(async (path: string) => + path === REPO_DIR ? { path, type: 'file' } : null + ); + + await expect(createRemoteDir('host', REPO_DIR)).rejects.toThrow('a file already exists there'); + expect(mkdir).not.toHaveBeenCalled(); + }); + + it('closes the SFTP channel even when mkdir throws', async () => { + existingDirs(['/home/ubuntu']); + mkdir.mockRejectedValue(new Error('mkdir failed')); + + await expect(createRemoteDir('host', REPO_DIR)).rejects.toThrow('mkdir failed'); + expect(close).toHaveBeenCalled(); + }); +}); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts new file mode 100644 index 000000000..f23d446d9 --- /dev/null +++ b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts @@ -0,0 +1,126 @@ +import { posix as pathPosix } from 'node:path'; +import { SshFileSystem } from '@main/core/fs/impl/ssh-fs'; +import { sshConnectionIdForHost } from '@main/core/locations/location-transport'; +import { ensureSshConnected } from '@main/core/ssh/connect/connect-agent-ssh'; +import { log } from '@main/lib/logger'; +import type { RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; + +/** + * Both helpers open their {@link SshFileSystem} at the filesystem root rather + * than at the directory under test, because the directory under test is exactly + * what may not exist. Every other caller roots its FS at an agent's working + * directory, which also scopes that FS's path-traversal guard to it — and that + * guard is why a missing *parent* cannot be created through the ordinary write + * path: recursive mkdir walks up only as far as its own root. That containment + * is deliberate, so these two helpers reach past it explicitly and are the only + * place allowed to, instead of the guard being widened for everyone. + */ +async function rootFsFor(sshHost: string): Promise { + const proxy = await ensureSshConnected(sshConnectionIdForHost(sshHost), sshHost); + return new SshFileSystem(proxy, '/'); +} + +/** Absolute ancestors of `dir`, deepest first, stopping above the root. */ +function ancestorsOf(dir: string): string[] { + const ancestors: string[] = []; + let current = pathPosix.dirname(dir); + while (current !== '/' && current !== '.' && !ancestors.includes(current)) { + ancestors.push(current); + current = pathPosix.dirname(current); + } + return ancestors; +} + +/** + * Inspect a prospective remote working directory on `sshHost`: does it exist, + * is it actually a directory, and if it is missing, how much of its path is + * missing with it (CHOO-1416). + * + * `dir` must be absolute — a relative path would resolve against whatever + * directory the SSH session happens to start in, which is not a thing the user + * chose. + * + * A path that cannot be stat'd for any reason *other* than absence (permission + * denied, dead connection) propagates rather than being reported as `missing`. + * Reporting an unreadable path as missing would offer to create a directory + * that is already there, and the create would then fail for the same reason the + * probe did. + */ +export async function inspectRemoteDir(sshHost: string, dir: string): Promise { + if (!pathPosix.isAbsolute(dir)) { + throw new Error(`Remote working directory must be an absolute path: ${dir}`); + } + const normalized = pathPosix.normalize(dir).replace(/\/+$/, '') || '/'; + + const fs = await rootFsFor(sshHost); + try { + const entry = await fs.stat(normalized); + if (entry) { + return { + dir: normalized, + status: entry.type === 'dir' ? 'directory' : 'file', + existingAncestor: '', + missingSegments: [], + }; + } + + // Walk up to the deepest ancestor that does exist, so the caller can say + // how much of the path is absent rather than just naming the leaf. + let existingAncestor = '/'; + for (const ancestor of ancestorsOf(normalized)) { + const ancestorEntry = await fs.stat(ancestor); + if (ancestorEntry) { + // A file where a parent directory should be makes the whole path + // uncreatable; report it against the offending path, not the leaf. + if (ancestorEntry.type !== 'dir') { + return { + dir: ancestor, + status: 'file', + existingAncestor: '', + missingSegments: [], + }; + } + existingAncestor = ancestor; + break; + } + } + + const missingSegments = normalized + .slice(existingAncestor === '/' ? 1 : existingAncestor.length + 1) + .split('/') + .filter(Boolean); + + return { dir: normalized, status: 'missing', existingAncestor, missingSegments }; + } finally { + fs.close(); + } +} + +/** + * Create `dir` (and any missing parents) on `sshHost`. + * + * Only ever called after the user has been shown the path and has explicitly + * asked for it — creating a directory on someone's host is not something to do + * on their behalf because a write happened to fail. What was created is logged, + * since the user sees a directory appear but not how much of the path came with + * it. + */ +export async function createRemoteDir(sshHost: string, dir: string): Promise { + const inspection = await inspectRemoteDir(sshHost, dir); + if (inspection.status === 'directory') return; + if (inspection.status === 'file') { + throw new Error(`Cannot create ${inspection.dir} on ${sshHost}: a file already exists there`); + } + + const fs = await rootFsFor(sshHost); + try { + await fs.mkdir(inspection.dir, { recursive: true }); + } finally { + fs.close(); + } + log.info('remote-dir: created remote working directory', { + sshHost, + dir: inspection.dir, + created: inspection.missingSegments.length, + }); +} diff --git a/dash/apps/switchdash-desktop/src/main/core/fs/impl/ssh-fs.test.ts b/dash/apps/switchdash-desktop/src/main/core/fs/impl/ssh-fs.test.ts index 099633179..d0597fc28 100644 --- a/dash/apps/switchdash-desktop/src/main/core/fs/impl/ssh-fs.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/fs/impl/ssh-fs.test.ts @@ -5,6 +5,13 @@ import { SshFileSystem } from './ssh-fs'; type SftpMkdirError = Error & { code?: number }; +/** SSH_FX_NO_SUCH_FILE, as ssh2 reports a missing path. */ +const SFTP_NO_SUCH_FILE = 2; + +function noSuchFile(): SftpMkdirError { + return Object.assign(new Error('No such file'), { code: SFTP_NO_SUCH_FILE }); +} + function listResult(entries: FileEntry[]): FileListResult { return { entries, total: entries.length }; } @@ -19,7 +26,7 @@ function fileEntry(path: string, mtimeMs: number, size = 1): FileEntry { }; } -function makeMkdirFs(errors: Array) { +function makeMkdirFs(errors: Array, base = '/repo') { const mkdirCalls: string[] = []; const sftp = { on: vi.fn(), @@ -35,7 +42,7 @@ function makeMkdirFs(errors: Array) { }; return { - fs: new SshFileSystem(proxy as never, '/repo'), + fs: new SshFileSystem(proxy as never, base), mkdirCalls, }; } @@ -106,6 +113,37 @@ describe('SshFileSystem.mkdir', () => { await expect(fs.mkdir('parent/child', { recursive: true })).resolves.toBeUndefined(); expect(mkdirCalls).toEqual(['/repo/parent/child', '/repo/parent', '/repo/parent/child']); }); + + // Recursive mkdir stops at its own base: an FS rooted at an agent's working + // directory must not be able to create that directory's ancestors. This is + // the containment behaviour that made a missing parent surface as a bare + // "File or directory not found" during remote agent creation (CHOO-1416) — + // the fix probes and creates the directory through `remote-dir.ts` (an FS + // opened at `/`), rather than widening the guard here. + it('refuses to create parents above its base', async () => { + const repoDir = '/home/ubuntu/switch-agents/internal-deployments'; + const { fs, mkdirCalls } = makeMkdirFs([noSuchFile(), noSuchFile(), noSuchFile()], repoDir); + + await expect(fs.mkdir('.switch/agents', { recursive: true })).rejects.toThrow( + `File or directory not found: ${repoDir}` + ); + // Walked up as far as the base and stopped there: creating the base's own + // parent (`/home/ubuntu/switch-agents`) would escape the sandbox. + expect(mkdirCalls).toEqual([`${repoDir}/.switch/agents`, `${repoDir}/.switch`, repoDir]); + }); + + it('creates ancestors freely when rooted at the filesystem root', async () => { + const { fs, mkdirCalls } = makeMkdirFs([noSuchFile(), undefined, undefined], '/'); + + await expect( + fs.mkdir('/home/ubuntu/switch-agents/internal-deployments', { recursive: true }) + ).resolves.toBeUndefined(); + expect(mkdirCalls).toEqual([ + '/home/ubuntu/switch-agents/internal-deployments', + '/home/ubuntu/switch-agents', + '/home/ubuntu/switch-agents/internal-deployments', + ]); + }); }); describe('SshFileSystem.remove', () => { diff --git a/dash/apps/switchdash-desktop/src/main/core/remote-hosts/controller.ts b/dash/apps/switchdash-desktop/src/main/core/remote-hosts/controller.ts index b4742840c..36aa67f8b 100644 --- a/dash/apps/switchdash-desktop/src/main/core/remote-hosts/controller.ts +++ b/dash/apps/switchdash-desktop/src/main/core/remote-hosts/controller.ts @@ -8,6 +8,7 @@ import type { } from '@switchdash/core/deps/runtime'; import { isTransportFailure } from '@switchdash/core/exec'; import { detectSwitchAgentRemote } from '@main/core/agents/detect-remote'; +import { createRemoteDir, inspectRemoteDir } from '@main/core/agents/remote-dir'; import { getRemoteDependencyManager, remoteDependencyDescriptor, @@ -25,6 +26,7 @@ import { } from '@main/core/switch-setup/remote-switch-setup'; import { GH_AUTH_STATUS_ARGS, parseGhAuthStatus } from '@shared/core/npm-registry'; import { hostBlockedReason, type HostReachability } from '@shared/core/remote-hosts/reachability'; +import type { RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; import type { ConnectionState, SshHealthState } from '@shared/core/ssh/ssh'; import { createRPCController } from '@shared/lib/ipc/rpc'; import type { SwitchAgentConfig } from '@shared/switch-agents'; @@ -347,6 +349,19 @@ export const remoteHostsController = createRPCController({ }): Promise => detectSwitchAgentRemote(params.sshHost, params.remoteRepoDir), + /** + * Inspect a prospective remote working directory before it is committed to. + * `detectRemoteAgent` deliberately cannot answer this: it maps "not found" to + * "no agent configured here", so a missing directory and an empty one look + * identical to it (CHOO-1416). + */ + inspectRemoteDir: (params: { sshHost: string; dir: string }): Promise => + inspectRemoteDir(params.sshHost, params.dir), + + /** Create a remote working directory and any missing parents, on request. */ + createRemoteDir: (params: { sshHost: string; dir: string }): Promise => + createRemoteDir(params.sshHost, params.dir), + probeDeps: (sshHost: string): Promise => probeDeps(sshHost), /** Whether a host is set up to run agents, and which agent types are usable on it. */ diff --git a/dash/apps/switchdash-desktop/src/main/core/switch-servers/controller.test.ts b/dash/apps/switchdash-desktop/src/main/core/switch-servers/controller.test.ts index d29df6408..0f2649db4 100644 --- a/dash/apps/switchdash-desktop/src/main/core/switch-servers/controller.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/switch-servers/controller.test.ts @@ -11,14 +11,7 @@ const fetchAuthConfig = vi.hoisted(() => vi.fn()); vi.mock('@main/core/agents/agent-defaults', () => ({ suggestAgentDefaults: vi.fn() })); vi.mock('@main/core/agents/propagate-server-api-url', () => ({ propagateServerApiUrl: vi.fn() })); vi.mock('@main/core/agents/resolve-servers', () => ({ resolveAgentServers: vi.fn() })); -vi.mock('@main/core/agents/write-remote-switch-settings', () => ({ - writeRemoteSwitchSettings: vi.fn(), -})); -vi.mock('@main/core/agents/write-switch-settings', () => ({ writeSwitchSettings: vi.fn() })); vi.mock('@main/core/app/service', () => ({ appService: { openExternal: vi.fn() } })); -vi.mock('@main/core/fs/impl/ssh-fs', () => ({ SshFileSystem: vi.fn() })); -vi.mock('@main/core/locations/location-transport', () => ({ sshConnectionIdForHost: vi.fn() })); -vi.mock('@main/core/ssh/connect/connect-agent-ssh', () => ({ ensureSshConnected: vi.fn() })); vi.mock('@main/core/managed-switch-server/managed-server-status', () => ({ isManagedServerRunning, managedServerHostBlocked, diff --git a/dash/apps/switchdash-desktop/src/main/core/switch-servers/controller.ts b/dash/apps/switchdash-desktop/src/main/core/switch-servers/controller.ts index 655b2c2d2..eb84f5d3f 100644 --- a/dash/apps/switchdash-desktop/src/main/core/switch-servers/controller.ts +++ b/dash/apps/switchdash-desktop/src/main/core/switch-servers/controller.ts @@ -1,20 +1,13 @@ import type { Result } from '@switchdash/shared'; import { suggestAgentDefaults } from '@main/core/agents/agent-defaults'; -import { knownAgentTypeForProvider } from '@main/core/agents/known-agent-type'; import { propagateServerApiUrl } from '@main/core/agents/propagate-server-api-url'; -import { registerAgentIdentity } from '@main/core/agents/register-agent-identity'; import { resolveAgentServers } from '@main/core/agents/resolve-servers'; -import { writeRemoteSwitchSettings } from '@main/core/agents/write-remote-switch-settings'; -import { writeSwitchSettings } from '@main/core/agents/write-switch-settings'; import { appService } from '@main/core/app/service'; -import { SshFileSystem } from '@main/core/fs/impl/ssh-fs'; -import { sshConnectionIdForHost } from '@main/core/locations/location-transport'; import { isManagedServerRunning, managedServerHostBlocked, } from '@main/core/managed-switch-server/managed-server-status'; import { HostUnreachableError } from '@main/core/remote-hosts/host-reachability-service'; -import { ensureSshConnected } from '@main/core/ssh/connect/connect-agent-ssh'; import type { AgentProviderId } from '@shared/core/providers/agent-provider-registry'; import type { AddressingPolicy, @@ -24,9 +17,6 @@ import type { CreateRoomParams, CreateRoomResult, PasswordLoginParams, - ProvisionAgentParams, - ProvisionAgentResult, - ProvisionRemoteAgentParams, RemoteAgentRoom, RemoteAgentSummary, RemoteBridge, @@ -284,73 +274,4 @@ export const switchServersController = createRPCController({ dir: string; providerId: AgentProviderId; }): Promise => suggestAgentDefaults(params.dir, params.providerId), - - /** - * Register a new agent on the chosen server (owned by the signed-in user) and - * write its credentials into the directory's `.claude/settings.local.json`. - * This is the desktop equivalent of running the switch-connector `configure` - * skill. Recoverable gateway failures are mapped to a typed result; the minted - * token is written to disk and never returned. - */ - provisionAgent: async (params: ProvisionAgentParams): Promise => { - const server = await requireServer(params.serverId); - - const registered = await registerAgentIdentity(server, { - name: params.name, - description: params.description, - repoDir: params.dir, - autoSession: params.autoSession, - // Provisioning writes `.claude/settings.local.json` — this is the Claude - // Code path by construction, not a fallback. - agentType: knownAgentTypeForProvider('claude'), - }); - if (registered.kind !== 'created') return registered; - - // The connector's SWITCH_API_ENDPOINT must point at the Switch core (agent - // bridge), which is a distinct endpoint from the gateway. - await writeSwitchSettings({ - dir: params.dir, - apiEndpoint: server.apiUrl, - apiToken: registered.apiKey, - agentId: registered.id, - }); - - return { kind: 'created', agentId: registered.id }; - }, - - /** - * Register a new Claude Code agent and write its credentials into a REMOTE - * working directory over SSH — the remote-host equivalent of `provisionAgent`. - * The agent has no local directory: its `.claude/settings.local.json` is - * written on the host, where the runtime sidecar reads it (CHOO-1059). - */ - provisionRemoteAgent: async ( - params: ProvisionRemoteAgentParams - ): Promise => { - const server = await requireServer(params.serverId); - - const registered = await registerAgentIdentity(server, { - name: params.name, - description: params.description, - repoDir: params.remoteRepoDir, - autoSession: params.autoSession, - // Remote provisioning likewise writes `.claude/settings.local.json`. - agentType: knownAgentTypeForProvider('claude'), - }); - if (registered.kind !== 'created') return registered; - - const proxy = await ensureSshConnected(sshConnectionIdForHost(params.sshHost), params.sshHost); - const fs = new SshFileSystem(proxy, params.remoteRepoDir); - try { - await writeRemoteSwitchSettings(fs, { - apiEndpoint: server.apiUrl, - apiToken: registered.apiKey, - agentId: registered.id, - }); - } finally { - fs.close(); - } - - return { kind: 'created', agentId: registered.id }; - }, }); diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx index 47a653791..18c064a3f 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx @@ -1,5 +1,5 @@ import type { RepoAgentAttributes } from '@switchdash/core/agents/plugins'; -import { useQuery } from '@tanstack/react-query'; +import { useMutation, useQuery } from '@tanstack/react-query'; import { CheckCircle2, CircleAlert } from 'lucide-react'; import { observer } from 'mobx-react-lite'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; @@ -43,7 +43,6 @@ import { import { log } from '@renderer/utils/logger'; import type { AgentProviderConfig } from '@shared/core/agents/agent-provider-config'; import { getProvider } from '@shared/core/providers/agent-provider-registry'; -import type { ProvisionAgentResult } from '@shared/core/switch-servers/switch-servers'; import { basenameFromAnyPath } from '@shared/path-name'; import { AgentAdvancedConfig } from './agent-advanced-config'; import { AgentTypePicker } from './agent-type-picker'; @@ -56,6 +55,7 @@ import { OnboardExistingPanel, type OnboardableAgent, } from './onboard-existing-panel'; +import { RemoteDirNotice } from './remote-dir-notice'; // switchdash adds a Switch *agent* by pointing at a local directory that the // switch-connector `configure` skill has set up (its `.claude/settings.local.json` @@ -66,6 +66,14 @@ export type AddLocationModalProps = BaseModalProps; /** Sentinel `runHost` value meaning "run on this machine" (no remote host). */ const LOCAL_RUN_LOCATION = 'local'; +/** The recoverable outcomes of `addAgent` — everything the modal has to report + * rather than treat as success. Derived from the RPC so a new variant in the + * main process surfaces here as a type error rather than a silent no-op. */ +type AddAgentFailure = Exclude< + Awaited>, + { kind: 'created' } +>; + /** Canonical working-directory path: trimmed, with trailing slashes removed * (except a bare root), so `/repo` and `/repo/` behave identically through * detection, discovery, and location keying — the flow must not care (CHOO-1440). */ @@ -198,6 +206,28 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc enabled: shouldDetectRemote, }); + // Whether that directory exists at all. `detectRemoteAgent` cannot answer it: + // it maps "not found" to "no agent configured here", so a missing directory + // and an empty one are indistinguishable to it — which is how a typo used to + // reach the create button and fail there (CHOO-1416). + const remoteDirQuery = useQuery({ + queryKey: ['remoteDirInspect', runHost, trimmedRemoteDir], + queryFn: () => rpc.remoteHosts.inspectRemoteDir({ sshHost: runHost, dir: trimmedRemoteDir }), + enabled: shouldDetectRemote, + retry: false, + }); + const createRemoteDirMutation = useMutation({ + mutationFn: () => rpc.remoteHosts.createRemoteDir({ sshHost: runHost, dir: trimmedRemoteDir }), + onSuccess: () => remoteDirQuery.refetch().then(() => remoteAgentQuery.refetch()), + onError: (error) => + toast({ + title: 'Failed to create the directory', + description: String(error), + variant: 'destructive', + }), + }); + const remoteDirUsable = !shouldDetectRemote || remoteDirQuery.data?.status === 'directory'; + // Provider agents defined in the picked dir (`.claude/agents/*.md`) — both those // already set up for Switch and plain provider subagents a user created directly. // The modal suggests onboarding them alongside the create flow; a directory is a @@ -355,6 +385,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc !!switchAgent && verifyState === 'found' && runHostReachable && + remoteDirUsable && submitState === 'idle' : pickState.isValid && !isChecking && @@ -370,6 +401,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc !!pickState.providerId && remoteRunValid && runHostReachable && + remoteDirUsable && submitState === 'idle'; // Remote configure gate: no agent in the remote dir yet — a valid remote @@ -385,6 +417,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc !!pickState.providerId && trimmedRemoteDir.length > 0 && runHostReachable && + remoteDirUsable && submitState === 'idle'; const reportCreationError = (error: AgentOnboardingError) => { @@ -481,7 +514,19 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc } }; - const reportProvisionError = (result: ProvisionAgentResult) => { + const reportProvisionError = (result: AddAgentFailure) => { + // The UI gate normally catches this before submit; reaching it here means + // the directory went away between the probe and the submit. Re-probe so the + // notice reappears with whatever is true now. + if (result.kind === 'directory-missing') { + toast({ + title: 'Working directory not found', + description: `${result.inspection.dir} no longer exists on ${result.sshHost}.`, + variant: 'destructive', + }); + void remoteDirQuery.refetch(); + return; + } if (result.kind === 'unauthenticated' && pickState.serverId) { toast({ title: 'Sign in to register the agent', @@ -743,6 +788,16 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc )} + {shouldDetectRemote && runHostReachable && ( + createRemoteDirMutation.mutate()} + /> + )} {/* The host is the first gate: with it unreachable we cannot know which agent types it has, so offering a type picker (or a directory to scan) diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx new file mode 100644 index 000000000..c30f40dfc --- /dev/null +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx @@ -0,0 +1,122 @@ +import { FolderPlus, Loader2 } from 'lucide-react'; +import { Button } from '@renderer/lib/ui/button'; +import type { RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; + +/** + * Inline notice for a remote working directory that does not exist yet + * (CHOO-1416), alongside {@link HostReachabilityNotice} in the add-agent + * modal's run-location field. + * + * The directory is free text and was previously only touched at write time, so + * a typo surfaced as a raw `FileSystemError` after an identity had already been + * minted. Checking it when the user commits the path means they find out while + * the field is still in front of them. + * + * Creating it is offered, never assumed: `mkdir -p` on whatever was typed would + * turn a typo into a real directory just as silently as the old error was + * abrupt. Naming the path — and, when several segments are missing, saying so — + * is what lets the user tell "not created yet" apart from "wrong path". + * + * Renders nothing when the directory is fine, so it can be dropped into the + * form unconditionally. + */ +export function RemoteDirNotice({ + sshHost, + inspection, + checking, + error, + creating, + onCreate, +}: { + sshHost: string; + inspection: RemoteDirInspection | undefined; + checking: boolean; + error: Error | null; + creating: boolean; + onCreate: () => void; +}) { + if (checking) { + return ( +
+ + Checking the working directory on {sshHost}… +
+ ); + } + + // A probe that failed for any reason other than absence (permission denied, a + // dropped connection) is not evidence the directory is missing, so it is + // reported as itself rather than as an offer to create anything. + if (error) { + return ( + +

+ Couldn’t check the working directory on {sshHost} +

+

{error.message}

+
+ ); + } + + if (!inspection || inspection.status === 'directory') return null; + + if (inspection.status === 'file') { + return ( + +

+ {inspection.dir} is a file +

+

+ An agent's working directory has to be a directory. Choose another path. +

+
+ ); + } + + const { missingSegments, existingAncestor } = inspection; + return ( + +
+
+

+ {inspection.dir} does not exist on{' '} + {sshHost} +

+ {missingSegments.length > 1 && ( +

+ {missingSegments.length} directories would be created — everything below{' '} + {existingAncestor}. Check the path is right before + creating it. +

+ )} +

+ Create it, or correct the path and set the location again. +

+
+ +
+
+ ); +} + +function NoticeShell({ children }: { children: React.ReactNode }) { + return ( +
+ {children} +
+ ); +} diff --git a/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts b/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts new file mode 100644 index 000000000..8476d9e54 --- /dev/null +++ b/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts @@ -0,0 +1,43 @@ +/** + * Existence model for a prospective remote working directory (CHOO-1416). + * + * A remote agent's working directory is typed as free text, so it is the one + * input in the add-agent flow that can be wrong in a way nothing else catches: + * the SSH host is probed for reachability, the server is picked from a list, + * but the directory is only ever touched at write time — by which point an + * identity has already been minted on the gateway. + * + * Inspecting it up front turns that into a decision the user makes knowingly. + * `missingSegments` is what makes a typo legible: a single missing leaf under + * an existing parent is an ordinary "not created yet", whereas several missing + * ancestors usually means the path is wrong, and the two should not look the + * same in the UI. + */ + +/** What an inspection found at a remote path. */ +export type RemoteDirStatus = + /** The path exists and is a directory — usable as a working directory. */ + | 'directory' + /** The path exists but is a regular file, so it can never be created. */ + | 'file' + /** The path does not exist. `missingSegments` says what creating it implies. */ + | 'missing'; + +/** The result of inspecting a prospective remote working directory. */ +export type RemoteDirInspection = { + /** The absolute path inspected, as resolved on the host. */ + dir: string; + status: RemoteDirStatus; + /** + * Deepest ancestor of `dir` that already exists, e.g. `/home/ubuntu` for a + * missing `/home/ubuntu/switch-agents/internal-deployments`. Empty when the + * path is not `missing`. + */ + existingAncestor: string; + /** + * Path components that creating `dir` would have to make, outermost first + * (`['switch-agents', 'internal-deployments']` for the example above). Empty + * when the path is not `missing`. Length > 1 is the typo signal. + */ + missingSegments: string[]; +}; diff --git a/dash/apps/switchdash-desktop/src/shared/core/switch-servers/switch-servers.ts b/dash/apps/switchdash-desktop/src/shared/core/switch-servers/switch-servers.ts index 721f88a47..e4df4cdfc 100644 --- a/dash/apps/switchdash-desktop/src/shared/core/switch-servers/switch-servers.ts +++ b/dash/apps/switchdash-desktop/src/shared/core/switch-servers/switch-servers.ts @@ -238,59 +238,8 @@ export type AgentDefaults = { }; /** - * Whether this Claude Code install can drive the development-channels flag. - * `anthropic` (Anthropic login / API key) → `channels_enabled = true` → - * session_addressable. `third-party` (Vertex AI / Bedrock / other) → - * `channels_enabled = false` → session_passive. There is no safe default; the - * user must choose, since the wrong value corrupts the agent's room behaviour. - */ -export type AgentProviderKind = 'anthropic' | 'third-party'; - -/** - * Parameters to register a brand-new Claude Code agent on a server and write - * its credentials into the directory's `.claude/settings.local.json` — the - * desktop equivalent of running the switch-connector `configure` skill. - */ -export type ProvisionAgentParams = { - serverId: string; - /** The agent's working directory; the settings file is written here and used - * as `repo_dir` so an offline-session command can `cd` into it. */ - dir: string; - name: string; - description: string; - providerKind: AgentProviderKind; - /** Bridge handle to @-mention in offline-session notices; omit to skip. */ - notifyUser?: string; - /** Register with the `auto_session` connection model: switchdash watches the - * agent's rooms and auto-spawns a session on notification. Defaults to off. */ - autoSession?: boolean; -}; - -/** - * Parameters to register a brand-new Claude Code agent on a server and write its - * credentials into a REMOTE working directory's `.claude/settings.local.json` - * over SSH — the remote-host equivalent of {@link ProvisionAgentParams}. There - * is no local directory: the agent's config lives entirely on the host. - */ -export type ProvisionRemoteAgentParams = { - serverId: string; - /** SSH alias of the onboarded host the agent runs on. */ - sshHost: string; - /** The agent's working directory on the host; the settings file is written - * here and used as `repo_dir`. */ - remoteRepoDir: string; - name: string; - description: string; - providerKind: AgentProviderKind; - /** Bridge handle to @-mention in offline-session notices; omit to skip. */ - notifyUser?: string; - /** Register with the `auto_session` connection model. Defaults to off. */ - autoSession?: boolean; -}; - -/** - * Outcome of provisioning a new agent. `created` carries the new Switch agent - * id; the other variants map a recoverable gateway failure to a specific + * Outcome of registering a new agent identity. `created` carries the new Switch + * agent id; the other variants map a recoverable gateway failure to a specific * message the modal can act on (re-login, rename) rather than a raw throw. The * minted API token is written to disk by the main process and never returned. */ From 575929874e991c3e0c12e52033ee1546926cffd7 Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Thu, 6 Aug 2026 16:06:29 +0000 Subject: [PATCH 02/15] fix(switchdash): only offer to create a remote directory that can be created (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live testing on a host surfaced the other half of this: a missing directory whose nearest existing ancestor is not writable was reported as plainly "missing", so the notice offered a Create directory button whose `mkdir` could only ever fail: FileSystemError: Permission denied: /home/louis_amauduz That is the typo case the notice exists to catch — a misspelt username resolves under `/home`, which nobody can write to — and the offer to create turned a legible "this path is wrong" into a failed action. Inspection now resolves whether the deepest existing ancestor is writable and reports it as `creatable`. The question is put to the host (`test -w`) rather than reconstructed from `stat`'s mode bits, which would mean replaying the kernel's permission check against a uid and group list belonging to another machine. The probe reports its verdict on stdout and always exits 0, so a non-zero exit still means the command itself failed and propagates. When a path is not creatable the notice explains why and points at the typo instead of offering a button; `createRemoteDir` refuses for the same reason rather than reaching `mkdir`. A create that fails anyway (a race, a permission change) re-probes so a stale offer is not left on screen, and reports what the main process actually said rather than the raw Electron IPC wrapper. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/main/core/agents/remote-dir.test.ts | 52 +++++++++++++++++++ .../src/main/core/agents/remote-dir.ts | 41 ++++++++++++++- .../add-agent-modal/add-agent-modal.tsx | 20 +++++-- .../add-agent-modal/remote-dir-notice.tsx | 20 +++++++ .../shared/core/remote-hosts/remote-dir.ts | 12 +++++ 5 files changed, 141 insertions(+), 4 deletions(-) diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts index ff377ec77..95975b348 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts @@ -4,6 +4,7 @@ import { FileSystemError, FileSystemErrorCodes } from '@main/core/fs/types'; const stat = vi.hoisted(() => vi.fn()); const mkdir = vi.hoisted(() => vi.fn()); const close = vi.hoisted(() => vi.fn()); +const exec = vi.hoisted(() => vi.fn(async () => ({ stdout: 'writable\n', stderr: '' }))); const constructedWith = vi.hoisted(() => [] as string[]); vi.mock('@main/core/fs/impl/ssh-fs', () => ({ @@ -22,6 +23,11 @@ vi.mock('@main/core/locations/location-transport', () => ({ vi.mock('@main/core/ssh/connect/connect-agent-ssh', () => ({ ensureSshConnected: vi.fn(async () => ({})), })); +vi.mock('@main/core/execution-context/ssh-execution-context', () => ({ + SshExecutionContext: class { + exec = exec; + }, +})); vi.mock('@main/lib/logger', () => ({ log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() } })); const { createRemoteDir, inspectRemoteDir } = await import('./remote-dir'); @@ -38,6 +44,7 @@ const REPO_DIR = '/home/ubuntu/switch-agents/internal-deployments'; beforeEach(() => { vi.clearAllMocks(); constructedWith.length = 0; + exec.mockResolvedValue({ stdout: 'writable\n', stderr: '' }); }); describe('inspectRemoteDir', () => { @@ -49,7 +56,10 @@ describe('inspectRemoteDir', () => { status: 'directory', existingAncestor: '', missingSegments: [], + creatable: false, }); + // No point asking whether an existing directory could be created. + expect(exec).not.toHaveBeenCalled(); }); // The ticket's repro: the directory *and* its parent are absent, which is @@ -62,7 +72,16 @@ describe('inspectRemoteDir', () => { status: 'missing', existingAncestor: '/home/ubuntu', missingSegments: ['switch-agents', 'internal-deployments'], + creatable: true, }); + // Writability is asked of the deepest *existing* ancestor — the directory + // the eventual mkdir actually has to write into. + expect(exec).toHaveBeenCalledWith('sh', [ + '-c', + 'if [ -w "$1" ]; then echo writable; else echo readonly; fi', + 'sh', + '/home/ubuntu', + ]); }); it('reports a single missing leaf under an existing parent', async () => { @@ -85,6 +104,29 @@ describe('inspectRemoteDir', () => { }); }); + // A misspelt username lands on `/home/`, whose parent `/home` nobody + // can write to. Reporting this as plainly "missing" offered a Create button + // that could only ever fail with EACCES. + it('reports a missing path under an unwritable ancestor as not creatable', async () => { + existingDirs(['/home']); + exec.mockResolvedValue({ stdout: 'readonly\n', stderr: '' }); + + expect(await inspectRemoteDir('host', '/home/louis_amauduz/repo')).toEqual({ + dir: '/home/louis_amauduz/repo', + status: 'missing', + existingAncestor: '/home', + missingSegments: ['louis_amauduz', 'repo'], + creatable: false, + }); + }); + + it('ignores login-shell banner noise before the verdict', async () => { + existingDirs(['/home/ubuntu']); + exec.mockResolvedValue({ stdout: 'Welcome to Ubuntu\nwritable\n', stderr: '' }); + + expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ creatable: true }); + }); + it('reports a path that is a file rather than offering to create it', async () => { stat.mockImplementation(async (path: string) => path === REPO_DIR ? { path, type: 'file' } : null @@ -159,6 +201,16 @@ describe('createRemoteDir', () => { expect(mkdir).not.toHaveBeenCalled(); }); + it('refuses to create under an unwritable ancestor instead of failing at mkdir', async () => { + existingDirs(['/home']); + exec.mockResolvedValue({ stdout: 'readonly\n', stderr: '' }); + + await expect(createRemoteDir('host', '/home/louis_amauduz/repo')).rejects.toThrow( + 'no write access to /home' + ); + expect(mkdir).not.toHaveBeenCalled(); + }); + it('refuses to create over an existing file', async () => { stat.mockImplementation(async (path: string) => path === REPO_DIR ? { path, type: 'file' } : null diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts index f23d446d9..51eeee7d0 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts @@ -1,4 +1,5 @@ import { posix as pathPosix } from 'node:path'; +import { SshExecutionContext } from '@main/core/execution-context/ssh-execution-context'; import { SshFileSystem } from '@main/core/fs/impl/ssh-fs'; import { sshConnectionIdForHost } from '@main/core/locations/location-transport'; import { ensureSshConnected } from '@main/core/ssh/connect/connect-agent-ssh'; @@ -20,6 +21,31 @@ async function rootFsFor(sshHost: string): Promise { return new SshFileSystem(proxy, '/'); } +/** + * Whether the SSH user can create entries inside `dir`. + * + * Asked of the host rather than derived from the mode bits `stat` returns: + * answering it locally would mean resolving the SSH user's uid and its full + * group list and replaying the kernel's permission check, which is a lot of + * ways to be subtly wrong about someone else's machine. `test -w` is the same + * question the kernel will answer when `mkdir` runs. + * + * The command reports its verdict on stdout and always exits 0, so a non-zero + * exit still means something genuinely went wrong and propagates. The path goes + * in as an argument, never interpolated into the script. + */ +async function isWritable(sshHost: string, dir: string): Promise { + const proxy = await ensureSshConnected(sshConnectionIdForHost(sshHost), sshHost); + const ctx = new SshExecutionContext(proxy); + const { stdout } = await ctx.exec('sh', [ + '-c', + 'if [ -w "$1" ]; then echo writable; else echo readonly; fi', + 'sh', + dir, + ]); + return stdout.trim().endsWith('writable'); +} + /** Absolute ancestors of `dir`, deepest first, stopping above the root. */ function ancestorsOf(dir: string): string[] { const ancestors: string[] = []; @@ -61,6 +87,7 @@ export async function inspectRemoteDir(sshHost: string, dir: string): Promise; /** Sentinel `runHost` value meaning "run on this machine" (no remote host). */ const LOCAL_RUN_LOCATION = 'local'; +/** Electron prefixes a rejected IPC call with its own plumbing + * (`Error invoking remote method 'x.y': `), and stacks the error names on top of + * that. Strip both so a toast shows what the main process actually said. */ +function rpcErrorMessage(error: unknown): string { + const raw = error instanceof Error ? error.message : String(error); + return raw + .replace(/^Error invoking remote method '[^']*':\s*/, '') + .replace(/^(?:\w*Error:\s*)+/, ''); +} + /** The recoverable outcomes of `addAgent` — everything the modal has to report * rather than treat as success. Derived from the RPC so a new variant in the * main process surfaces here as a type error rather than a silent no-op. */ @@ -219,12 +229,16 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc const createRemoteDirMutation = useMutation({ mutationFn: () => rpc.remoteHosts.createRemoteDir({ sshHost: runHost, dir: trimmedRemoteDir }), onSuccess: () => remoteDirQuery.refetch().then(() => remoteAgentQuery.refetch()), - onError: (error) => + onError: (error) => { toast({ title: 'Failed to create the directory', - description: String(error), + description: rpcErrorMessage(error), variant: 'destructive', - }), + }); + // Whatever stopped the create (a race, a permission change) is a fact + // about the host, so re-probe rather than leaving a stale offer up. + void remoteDirQuery.refetch(); + }, }); const remoteDirUsable = !shouldDetectRemote || remoteDirQuery.data?.status === 'directory'; diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx index c30f40dfc..9359c03e4 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx @@ -74,6 +74,26 @@ export function RemoteDirNotice({ } const { missingSegments, existingAncestor } = inspection; + + // Nothing can be created under an ancestor the SSH user cannot write to, so + // the offer is withheld rather than made and then failed. In practice this is + // usually a misspelt path under `/home` — the user's own directory would be + // writable, so landing on an unwritable one says the name is wrong. + if (!inspection.creatable) { + return ( + +

+ {inspection.dir} does not exist on {sshHost}, + and cannot be created +

+

+ You do not have write access to {existingAncestor}. + Check the path for a typo, or create the directory on the host yourself. +

+
+ ); + } + return (
diff --git a/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts b/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts index 8476d9e54..470cf218a 100644 --- a/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts +++ b/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts @@ -40,4 +40,16 @@ export type RemoteDirInspection = { * when the path is not `missing`. Length > 1 is the typo signal. */ missingSegments: string[]; + /** + * Whether the SSH user can actually write into {@link existingAncestor}, and + * so whether creating `dir` is possible at all. False when the path is + * `missing` but lands somewhere unwritable — typically `/home/` for a + * misspelt user, which no amount of `mkdir` will fix. Meaningless (and false) + * when the path is not `missing`. + * + * Offering to create a directory is only honest if the create can succeed, so + * this is resolved during inspection rather than discovered by the user + * pressing a button that fails. + */ + creatable: boolean; }; From 6e0697a347296ffa61c664f96f8d0477ca64eea3 Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Thu, 6 Aug 2026 16:15:24 +0000 Subject: [PATCH 03/15] fix(switchdash): report a missing remote working directory instead of creating it (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops the create-the-directory affordance. Making a directory on someone's host is their decision, and a path typed by hand is exactly where switchdash would otherwise turn a typo into a real, wrong directory. Reporting which path is missing is enough. Removes `createRemoteDir` and its IPC method, the Create directory button, and with them the writability probe that only existed to decide whether the button could work. `inspectRemoteDir` keeps the existence check and still reports the deepest existing ancestor: that is what separates "not made yet" from a misspelt path, and it costs nothing beyond the walk already being done. The notice and the typed `directory-missing` result now say the same thing — this path does not exist on this host, create it there and set the location again. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/main/core/agents/add-agent.test.ts | 4 - .../src/main/core/agents/add-agent.ts | 7 +- .../src/main/core/agents/remote-dir.test.ts | 133 +++--------------- .../src/main/core/agents/remote-dir.ts | 122 ++-------------- .../src/main/core/fs/impl/ssh-fs.test.ts | 6 +- .../src/main/core/remote-hosts/controller.ts | 6 +- .../add-agent-modal/add-agent-modal.tsx | 30 +--- .../add-agent-modal/remote-dir-notice.tsx | 96 +++---------- .../shared/core/remote-hosts/remote-dir.ts | 41 ++---- 9 files changed, 78 insertions(+), 367 deletions(-) diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts index 38853a3f0..bb2bd03ed 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts @@ -47,7 +47,6 @@ const h = vi.hoisted(() => { dir, status: 'directory' as const, existingAncestor: '', - missingSegments: [], })), }; }); @@ -118,7 +117,6 @@ describe('addAgent', () => { dir, status: 'directory' as const, existingAncestor: '', - missingSegments: [], })); }); @@ -203,7 +201,6 @@ describe('addAgent', () => { dir: remote.dir, status: 'missing', existingAncestor: '/home/ubuntu', - missingSegments: ['switch-agents', 'deploys'], } as never); const result = await addAgent(params(remote)); @@ -222,7 +219,6 @@ describe('addAgent', () => { dir: remote.dir, status: 'file', existingAncestor: '', - missingSegments: [], } as never); expect((await addAgent(params(remote))).kind).toBe('directory-missing'); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts index 9ace6518e..77515f7c7 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts @@ -53,9 +53,10 @@ export type AddAgentResult = | { kind: 'unauthenticated' } | { kind: 'name-conflict' } | { kind: 'invalid-name'; message: string } - /** The remote working directory does not exist (or is a file). Recoverable: - * the caller offers to create it and retries. Reported before anything is - * minted, so no Switch-side agent is left behind (CHOO-1416). */ + /** The remote working directory does not exist (or is a file). The user + * creates it on the host and retries; switchdash does not create it for them. + * Reported before anything is minted, so no Switch-side agent is left behind + * (CHOO-1416). */ | { kind: 'directory-missing'; sshHost: string; inspection: RemoteDirInspection } | { kind: 'error'; message: string }; diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts index 95975b348..4f8cb499c 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts @@ -2,9 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { FileSystemError, FileSystemErrorCodes } from '@main/core/fs/types'; const stat = vi.hoisted(() => vi.fn()); -const mkdir = vi.hoisted(() => vi.fn()); const close = vi.hoisted(() => vi.fn()); -const exec = vi.hoisted(() => vi.fn(async () => ({ stdout: 'writable\n', stderr: '' }))); const constructedWith = vi.hoisted(() => [] as string[]); vi.mock('@main/core/fs/impl/ssh-fs', () => ({ @@ -13,7 +11,6 @@ vi.mock('@main/core/fs/impl/ssh-fs', () => ({ constructedWith.push(base); } stat = stat; - mkdir = mkdir; close = close; }, })); @@ -23,14 +20,8 @@ vi.mock('@main/core/locations/location-transport', () => ({ vi.mock('@main/core/ssh/connect/connect-agent-ssh', () => ({ ensureSshConnected: vi.fn(async () => ({})), })); -vi.mock('@main/core/execution-context/ssh-execution-context', () => ({ - SshExecutionContext: class { - exec = exec; - }, -})); -vi.mock('@main/lib/logger', () => ({ log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() } })); -const { createRemoteDir, inspectRemoteDir } = await import('./remote-dir'); +const { inspectRemoteDir } = await import('./remote-dir'); /** Report `paths` as existing directories and everything else as absent. */ function existingDirs(paths: string[]) { @@ -44,7 +35,6 @@ const REPO_DIR = '/home/ubuntu/switch-agents/internal-deployments'; beforeEach(() => { vi.clearAllMocks(); constructedWith.length = 0; - exec.mockResolvedValue({ stdout: 'writable\n', stderr: '' }); }); describe('inspectRemoteDir', () => { @@ -55,33 +45,22 @@ describe('inspectRemoteDir', () => { dir: REPO_DIR, status: 'directory', existingAncestor: '', - missingSegments: [], - creatable: false, }); - // No point asking whether an existing directory could be created. - expect(exec).not.toHaveBeenCalled(); }); // The ticket's repro: the directory *and* its parent are absent, which is - // what the per-directory FS could not recover from (CHOO-1416). - it('names every missing segment when several ancestors are absent', async () => { + // what the per-directory FS could not even see past (CHOO-1416). + it('finds the deepest existing ancestor when several are absent', async () => { existingDirs(['/home/ubuntu']); expect(await inspectRemoteDir('host', REPO_DIR)).toEqual({ dir: REPO_DIR, status: 'missing', existingAncestor: '/home/ubuntu', - missingSegments: ['switch-agents', 'internal-deployments'], - creatable: true, }); - // Writability is asked of the deepest *existing* ancestor — the directory - // the eventual mkdir actually has to write into. - expect(exec).toHaveBeenCalledWith('sh', [ - '-c', - 'if [ -w "$1" ]; then echo writable; else echo readonly; fi', - 'sh', - '/home/ubuntu', - ]); + // Opened at the host root: an FS rooted at the missing directory could not + // stat its way out to find what does exist. + expect(constructedWith).toEqual(['/']); }); it('reports a single missing leaf under an existing parent', async () => { @@ -90,44 +69,30 @@ describe('inspectRemoteDir', () => { expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ status: 'missing', existingAncestor: '/home/ubuntu/switch-agents', - missingSegments: ['internal-deployments'], }); }); - it('falls back to the root when no ancestor exists', async () => { - existingDirs([]); - - expect(await inspectRemoteDir('host', '/srv/agent')).toMatchObject({ - status: 'missing', - existingAncestor: '/', - missingSegments: ['srv', 'agent'], - }); - }); - - // A misspelt username lands on `/home/`, whose parent `/home` nobody - // can write to. Reporting this as plainly "missing" offered a Create button - // that could only ever fail with EACCES. - it('reports a missing path under an unwritable ancestor as not creatable', async () => { + // A misspelt username leaves `/home` as the deepest match, which is the + // signal that the path is wrong rather than merely unmade. + it('falls back to a shallow ancestor for a misspelt path', async () => { existingDirs(['/home']); - exec.mockResolvedValue({ stdout: 'readonly\n', stderr: '' }); - expect(await inspectRemoteDir('host', '/home/louis_amauduz/repo')).toEqual({ - dir: '/home/louis_amauduz/repo', + expect(await inspectRemoteDir('host', '/home/louis_amauduz/repo')).toMatchObject({ status: 'missing', existingAncestor: '/home', - missingSegments: ['louis_amauduz', 'repo'], - creatable: false, }); }); - it('ignores login-shell banner noise before the verdict', async () => { - existingDirs(['/home/ubuntu']); - exec.mockResolvedValue({ stdout: 'Welcome to Ubuntu\nwritable\n', stderr: '' }); + it('falls back to the root when no ancestor exists', async () => { + existingDirs([]); - expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ creatable: true }); + expect(await inspectRemoteDir('host', '/srv/agent')).toMatchObject({ + status: 'missing', + existingAncestor: '/', + }); }); - it('reports a path that is a file rather than offering to create it', async () => { + it('reports a path that is a file', async () => { stat.mockImplementation(async (path: string) => path === REPO_DIR ? { path, type: 'file' } : null ); @@ -138,20 +103,8 @@ describe('inspectRemoteDir', () => { }); }); - it('blames the ancestor when a parent is a file', async () => { - stat.mockImplementation(async (path: string) => { - if (path === '/home/ubuntu/switch-agents') return { path, type: 'file' }; - return null; - }); - - expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ - dir: '/home/ubuntu/switch-agents', - status: 'file', - }); - }); - - // Treating an unreadable path as missing would offer to create a directory - // that is already there, and the create would fail the same way the probe did. + // An unreadable path is not a missing one; saying so would send the user off + // to create a directory that is already there. it('propagates a probe failure that is not absence', async () => { stat.mockRejectedValue( new FileSystemError('Permission denied: /home', FileSystemErrorCodes.PERMISSION_DENIED) @@ -180,51 +133,3 @@ describe('inspectRemoteDir', () => { expect(close).toHaveBeenCalled(); }); }); - -describe('createRemoteDir', () => { - it('creates the directory and its missing parents from the filesystem root', async () => { - existingDirs(['/home/ubuntu']); - - await createRemoteDir('host', REPO_DIR); - - expect(mkdir).toHaveBeenCalledWith(REPO_DIR, { recursive: true }); - // Rooting at `/` is what lets the ancestors be created at all — an FS - // rooted at the repo dir cannot create its own parents. - expect(constructedWith).toEqual(['/', '/']); - }); - - it('is a no-op when the directory already exists', async () => { - existingDirs([REPO_DIR]); - - await createRemoteDir('host', REPO_DIR); - - expect(mkdir).not.toHaveBeenCalled(); - }); - - it('refuses to create under an unwritable ancestor instead of failing at mkdir', async () => { - existingDirs(['/home']); - exec.mockResolvedValue({ stdout: 'readonly\n', stderr: '' }); - - await expect(createRemoteDir('host', '/home/louis_amauduz/repo')).rejects.toThrow( - 'no write access to /home' - ); - expect(mkdir).not.toHaveBeenCalled(); - }); - - it('refuses to create over an existing file', async () => { - stat.mockImplementation(async (path: string) => - path === REPO_DIR ? { path, type: 'file' } : null - ); - - await expect(createRemoteDir('host', REPO_DIR)).rejects.toThrow('a file already exists there'); - expect(mkdir).not.toHaveBeenCalled(); - }); - - it('closes the SFTP channel even when mkdir throws', async () => { - existingDirs(['/home/ubuntu']); - mkdir.mockRejectedValue(new Error('mkdir failed')); - - await expect(createRemoteDir('host', REPO_DIR)).rejects.toThrow('mkdir failed'); - expect(close).toHaveBeenCalled(); - }); -}); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts index 51eeee7d0..c4d805b54 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts @@ -1,51 +1,9 @@ import { posix as pathPosix } from 'node:path'; -import { SshExecutionContext } from '@main/core/execution-context/ssh-execution-context'; import { SshFileSystem } from '@main/core/fs/impl/ssh-fs'; import { sshConnectionIdForHost } from '@main/core/locations/location-transport'; import { ensureSshConnected } from '@main/core/ssh/connect/connect-agent-ssh'; -import { log } from '@main/lib/logger'; import type { RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; -/** - * Both helpers open their {@link SshFileSystem} at the filesystem root rather - * than at the directory under test, because the directory under test is exactly - * what may not exist. Every other caller roots its FS at an agent's working - * directory, which also scopes that FS's path-traversal guard to it — and that - * guard is why a missing *parent* cannot be created through the ordinary write - * path: recursive mkdir walks up only as far as its own root. That containment - * is deliberate, so these two helpers reach past it explicitly and are the only - * place allowed to, instead of the guard being widened for everyone. - */ -async function rootFsFor(sshHost: string): Promise { - const proxy = await ensureSshConnected(sshConnectionIdForHost(sshHost), sshHost); - return new SshFileSystem(proxy, '/'); -} - -/** - * Whether the SSH user can create entries inside `dir`. - * - * Asked of the host rather than derived from the mode bits `stat` returns: - * answering it locally would mean resolving the SSH user's uid and its full - * group list and replaying the kernel's permission check, which is a lot of - * ways to be subtly wrong about someone else's machine. `test -w` is the same - * question the kernel will answer when `mkdir` runs. - * - * The command reports its verdict on stdout and always exits 0, so a non-zero - * exit still means something genuinely went wrong and propagates. The path goes - * in as an argument, never interpolated into the script. - */ -async function isWritable(sshHost: string, dir: string): Promise { - const proxy = await ensureSshConnected(sshConnectionIdForHost(sshHost), sshHost); - const ctx = new SshExecutionContext(proxy); - const { stdout } = await ctx.exec('sh', [ - '-c', - 'if [ -w "$1" ]; then echo writable; else echo readonly; fi', - 'sh', - dir, - ]); - return stdout.trim().endsWith('writable'); -} - /** Absolute ancestors of `dir`, deepest first, stopping above the root. */ function ancestorsOf(dir: string): string[] { const ancestors: string[] = []; @@ -59,8 +17,14 @@ function ancestorsOf(dir: string): string[] { /** * Inspect a prospective remote working directory on `sshHost`: does it exist, - * is it actually a directory, and if it is missing, how much of its path is - * missing with it (CHOO-1416). + * is it actually a directory, and if not, what is the deepest part of the path + * that does exist (CHOO-1416). + * + * The filesystem is opened at the host's root rather than at the directory + * under test, because the directory under test is exactly what may not exist. + * Every other caller roots its FS at an agent's working directory, which also + * scopes that FS's path-traversal guard to it — so an FS rooted at a missing + * directory cannot even stat its way out to find what is there instead. * * `dir` must be absolute — a relative path would resolve against whatever * directory the SSH session happens to start in, which is not a thing the user @@ -68,9 +32,8 @@ function ancestorsOf(dir: string): string[] { * * A path that cannot be stat'd for any reason *other* than absence (permission * denied, dead connection) propagates rather than being reported as `missing`. - * Reporting an unreadable path as missing would offer to create a directory - * that is already there, and the create would then fail for the same reason the - * probe did. + * An unreadable path is not a missing one, and saying so would send the user + * off to fix the wrong problem. */ export async function inspectRemoteDir(sshHost: string, dir: string): Promise { if (!pathPosix.isAbsolute(dir)) { @@ -78,7 +41,8 @@ export async function inspectRemoteDir(sshHost: string, dir: string): Promise { - const inspection = await inspectRemoteDir(sshHost, dir); - if (inspection.status === 'directory') return; - if (inspection.status === 'file') { - throw new Error(`Cannot create ${inspection.dir} on ${sshHost}: a file already exists there`); - } - if (!inspection.creatable) { - throw new Error( - `Cannot create ${inspection.dir} on ${sshHost}: no write access to ${inspection.existingAncestor}` - ); - } - - const fs = await rootFsFor(sshHost); - try { - await fs.mkdir(inspection.dir, { recursive: true }); + return { dir: normalized, status: 'missing', existingAncestor }; } finally { fs.close(); } - log.info('remote-dir: created remote working directory', { - sshHost, - dir: inspection.dir, - created: inspection.missingSegments.length, - }); } diff --git a/dash/apps/switchdash-desktop/src/main/core/fs/impl/ssh-fs.test.ts b/dash/apps/switchdash-desktop/src/main/core/fs/impl/ssh-fs.test.ts index d0597fc28..9a489ecb1 100644 --- a/dash/apps/switchdash-desktop/src/main/core/fs/impl/ssh-fs.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/fs/impl/ssh-fs.test.ts @@ -117,9 +117,9 @@ describe('SshFileSystem.mkdir', () => { // Recursive mkdir stops at its own base: an FS rooted at an agent's working // directory must not be able to create that directory's ancestors. This is // the containment behaviour that made a missing parent surface as a bare - // "File or directory not found" during remote agent creation (CHOO-1416) — - // the fix probes and creates the directory through `remote-dir.ts` (an FS - // opened at `/`), rather than widening the guard here. + // "File or directory not found" during remote agent creation (CHOO-1416). + // The fix reports the missing directory before anything is written, via a + // probe rooted at `/` in `remote-dir.ts`, rather than widening this guard. it('refuses to create parents above its base', async () => { const repoDir = '/home/ubuntu/switch-agents/internal-deployments'; const { fs, mkdirCalls } = makeMkdirFs([noSuchFile(), noSuchFile(), noSuchFile()], repoDir); diff --git a/dash/apps/switchdash-desktop/src/main/core/remote-hosts/controller.ts b/dash/apps/switchdash-desktop/src/main/core/remote-hosts/controller.ts index 36aa67f8b..f30c38d6b 100644 --- a/dash/apps/switchdash-desktop/src/main/core/remote-hosts/controller.ts +++ b/dash/apps/switchdash-desktop/src/main/core/remote-hosts/controller.ts @@ -8,7 +8,7 @@ import type { } from '@switchdash/core/deps/runtime'; import { isTransportFailure } from '@switchdash/core/exec'; import { detectSwitchAgentRemote } from '@main/core/agents/detect-remote'; -import { createRemoteDir, inspectRemoteDir } from '@main/core/agents/remote-dir'; +import { inspectRemoteDir } from '@main/core/agents/remote-dir'; import { getRemoteDependencyManager, remoteDependencyDescriptor, @@ -358,10 +358,6 @@ export const remoteHostsController = createRPCController({ inspectRemoteDir: (params: { sshHost: string; dir: string }): Promise => inspectRemoteDir(params.sshHost, params.dir), - /** Create a remote working directory and any missing parents, on request. */ - createRemoteDir: (params: { sshHost: string; dir: string }): Promise => - createRemoteDir(params.sshHost, params.dir), - probeDeps: (sshHost: string): Promise => probeDeps(sshHost), /** Whether a host is set up to run agents, and which agent types are usable on it. */ diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx index 14102b376..7ac9b3da2 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx @@ -1,5 +1,5 @@ import type { RepoAgentAttributes } from '@switchdash/core/agents/plugins'; -import { useMutation, useQuery } from '@tanstack/react-query'; +import { useQuery } from '@tanstack/react-query'; import { CheckCircle2, CircleAlert } from 'lucide-react'; import { observer } from 'mobx-react-lite'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; @@ -66,16 +66,6 @@ export type AddLocationModalProps = BaseModalProps; /** Sentinel `runHost` value meaning "run on this machine" (no remote host). */ const LOCAL_RUN_LOCATION = 'local'; -/** Electron prefixes a rejected IPC call with its own plumbing - * (`Error invoking remote method 'x.y': `), and stacks the error names on top of - * that. Strip both so a toast shows what the main process actually said. */ -function rpcErrorMessage(error: unknown): string { - const raw = error instanceof Error ? error.message : String(error); - return raw - .replace(/^Error invoking remote method '[^']*':\s*/, '') - .replace(/^(?:\w*Error:\s*)+/, ''); -} - /** The recoverable outcomes of `addAgent` — everything the modal has to report * rather than treat as success. Derived from the RPC so a new variant in the * main process surfaces here as a type error rather than a silent no-op. */ @@ -226,20 +216,6 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc enabled: shouldDetectRemote, retry: false, }); - const createRemoteDirMutation = useMutation({ - mutationFn: () => rpc.remoteHosts.createRemoteDir({ sshHost: runHost, dir: trimmedRemoteDir }), - onSuccess: () => remoteDirQuery.refetch().then(() => remoteAgentQuery.refetch()), - onError: (error) => { - toast({ - title: 'Failed to create the directory', - description: rpcErrorMessage(error), - variant: 'destructive', - }); - // Whatever stopped the create (a race, a permission change) is a fact - // about the host, so re-probe rather than leaving a stale offer up. - void remoteDirQuery.refetch(); - }, - }); const remoteDirUsable = !shouldDetectRemote || remoteDirQuery.data?.status === 'directory'; // Provider agents defined in the picked dir (`.claude/agents/*.md`) — both those @@ -535,7 +511,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc if (result.kind === 'directory-missing') { toast({ title: 'Working directory not found', - description: `${result.inspection.dir} no longer exists on ${result.sshHost}.`, + description: `${result.inspection.dir} does not exist on ${result.sshHost}. Create it on the host and try again.`, variant: 'destructive', }); void remoteDirQuery.refetch(); @@ -808,8 +784,6 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc inspection={remoteDirQuery.data} checking={remoteDirQuery.isFetching} error={remoteDirQuery.error} - creating={createRemoteDirMutation.isPending} - onCreate={() => createRemoteDirMutation.mutate()} /> )} diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx index 9359c03e4..d794e62ab 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/remote-dir-notice.tsx @@ -1,21 +1,19 @@ -import { FolderPlus, Loader2 } from 'lucide-react'; -import { Button } from '@renderer/lib/ui/button'; +import { Loader2 } from 'lucide-react'; import type { RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; /** - * Inline notice for a remote working directory that does not exist yet - * (CHOO-1416), alongside {@link HostReachabilityNotice} in the add-agent - * modal's run-location field. + * Inline notice for a remote working directory that is not usable (CHOO-1416), + * alongside `HostReachabilityNotice` in the add-agent modal's run-location + * field. * * The directory is free text and was previously only touched at write time, so - * a typo surfaced as a raw `FileSystemError` after an identity had already been - * minted. Checking it when the user commits the path means they find out while - * the field is still in front of them. + * a wrong path surfaced as a raw `FileSystemError` after an identity had + * already been minted. Checking it when the user commits the path means they + * find out while the field is still in front of them. * - * Creating it is offered, never assumed: `mkdir -p` on whatever was typed would - * turn a typo into a real directory just as silently as the old error was - * abrupt. Naming the path — and, when several segments are missing, saying so — - * is what lets the user tell "not created yet" apart from "wrong path". + * switchdash does not create the directory — it says which path is missing and + * leaves that to the user. Naming the deepest part of the path that *does* + * exist is what separates "not made yet" from a typo. * * Renders nothing when the directory is fine, so it can be dropped into the * form unconditionally. @@ -25,15 +23,11 @@ export function RemoteDirNotice({ inspection, checking, error, - creating, - onCreate, }: { sshHost: string; inspection: RemoteDirInspection | undefined; checking: boolean; error: Error | null; - creating: boolean; - onCreate: () => void; }) { if (checking) { return ( @@ -45,8 +39,8 @@ export function RemoteDirNotice({ } // A probe that failed for any reason other than absence (permission denied, a - // dropped connection) is not evidence the directory is missing, so it is - // reported as itself rather than as an offer to create anything. + // dropped connection) is not evidence the directory is missing, and is + // reported as itself so the user does not go and fix the wrong thing. if (error) { return ( @@ -64,7 +58,7 @@ export function RemoteDirNotice({ return (

- {inspection.dir} is a file + {inspection.dir} is a file

An agent's working directory has to be a directory. Choose another path. @@ -73,62 +67,18 @@ export function RemoteDirNotice({ ); } - const { missingSegments, existingAncestor } = inspection; - - // Nothing can be created under an ancestor the SSH user cannot write to, so - // the offer is withheld rather than made and then failed. In practice this is - // usually a misspelt path under `/home` — the user's own directory would be - // writable, so landing on an unwritable one says the name is wrong. - if (!inspection.creatable) { - return ( - -

- {inspection.dir} does not exist on {sshHost}, - and cannot be created -

-

- You do not have write access to {existingAncestor}. - Check the path for a typo, or create the directory on the host yourself. -

-
- ); - } - return ( -
-
-

- {inspection.dir} does not exist on{' '} - {sshHost} -

- {missingSegments.length > 1 && ( -

- {missingSegments.length} directories would be created — everything below{' '} - {existingAncestor}. Check the path is right before - creating it. -

- )} -

- Create it, or correct the path and set the location again. -

-
- -
+

+ {inspection.dir} does not exist on {sshHost} +

+

+ The deepest part of that path that exists is{' '} + {inspection.existingAncestor}. +

+

+ Create the directory on the host and set the location again, or correct the path. +

); } diff --git a/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts b/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts index 470cf218a..abfdf29dc 100644 --- a/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts +++ b/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts @@ -4,23 +4,22 @@ * A remote agent's working directory is typed as free text, so it is the one * input in the add-agent flow that can be wrong in a way nothing else catches: * the SSH host is probed for reachability, the server is picked from a list, - * but the directory is only ever touched at write time — by which point an - * identity has already been minted on the gateway. + * but the directory was only ever touched at write time — by which point an + * identity had already been minted on the gateway. * - * Inspecting it up front turns that into a decision the user makes knowingly. - * `missingSegments` is what makes a typo legible: a single missing leaf under - * an existing parent is an ordinary "not created yet", whereas several missing - * ancestors usually means the path is wrong, and the two should not look the - * same in the UI. + * switchdash does not create the directory. Making one on someone's host is + * their decision, and a path typed by hand is exactly where a typo would be + * silently turned into a real, wrong directory. Inspecting it up front is only + * so the flow can say which path is missing, while the field is still on screen. */ /** What an inspection found at a remote path. */ export type RemoteDirStatus = /** The path exists and is a directory — usable as a working directory. */ | 'directory' - /** The path exists but is a regular file, so it can never be created. */ + /** The path exists but is a regular file. */ | 'file' - /** The path does not exist. `missingSegments` says what creating it implies. */ + /** The path does not exist. */ | 'missing'; /** The result of inspecting a prospective remote working directory. */ @@ -29,27 +28,13 @@ export type RemoteDirInspection = { dir: string; status: RemoteDirStatus; /** - * Deepest ancestor of `dir` that already exists, e.g. `/home/ubuntu` for a + * Deepest ancestor of `dir` that does exist, e.g. `/home/ubuntu` for a * missing `/home/ubuntu/switch-agents/internal-deployments`. Empty when the * path is not `missing`. - */ - existingAncestor: string; - /** - * Path components that creating `dir` would have to make, outermost first - * (`['switch-agents', 'internal-deployments']` for the example above). Empty - * when the path is not `missing`. Length > 1 is the typo signal. - */ - missingSegments: string[]; - /** - * Whether the SSH user can actually write into {@link existingAncestor}, and - * so whether creating `dir` is possible at all. False when the path is - * `missing` but lands somewhere unwritable — typically `/home/` for a - * misspelt user, which no amount of `mkdir` will fix. Meaningless (and false) - * when the path is not `missing`. * - * Offering to create a directory is only honest if the create can succeed, so - * this is resolved during inspection rather than discovered by the user - * pressing a button that fails. + * Worth surfacing because it separates the two ways this goes wrong: an + * ancestor one level up means the directory simply has not been made yet, + * whereas a much shallower one means the path is probably misspelt. */ - creatable: boolean; + existingAncestor: string; }; From 1962bf437571f7fdc27392e5db69279d1137611c Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Thu, 6 Aug 2026 16:22:36 +0000 Subject: [PATCH 04/15] fix(switchdash): only refuse a remote directory whose parent is missing too (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gating on "the directory exists" was too strict and took away behaviour that already worked: recursive mkdir stops at its own root but may create that root, so a missing directory under an existing parent has always been created by the first credentials write. Only a missing parent is unrecoverable. Inspection now answers exactly that, in two stats — `directory`, `creatable` (missing, parent exists), `file`, or `missing` (parent gone too). `creatable` passes through silently, so `/home//module/repo` under an existing `/home//module` behaves as it did before this ticket. `isUsableRemoteDir` keeps the UI gate and the `addAgent` guard agreeing on which of those are acceptable. The notice is one line: the path does not exist on the host, create it there first. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/main/core/agents/add-agent.test.ts | 23 ++++--- .../src/main/core/agents/add-agent.ts | 15 +++-- .../src/main/core/agents/remote-dir.test.ts | 61 +++++++------------ .../src/main/core/agents/remote-dir.ts | 47 ++++---------- .../add-agent-modal/add-agent-modal.tsx | 5 +- .../add-agent-modal/remote-dir-notice.tsx | 58 ++++++------------ .../shared/core/remote-hosts/remote-dir.ts | 35 ++++++----- 7 files changed, 96 insertions(+), 148 deletions(-) diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts index bb2bd03ed..06c394a88 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.test.ts @@ -46,7 +46,6 @@ const h = vi.hoisted(() => { inspectRemoteDir: vi.fn(async (_host: string, dir: string) => ({ dir, status: 'directory' as const, - existingAncestor: '', })), }; }); @@ -116,7 +115,6 @@ describe('addAgent', () => { h.inspectRemoteDir.mockImplementation(async (_host: string, dir: string) => ({ dir, status: 'directory' as const, - existingAncestor: '', })); }); @@ -197,11 +195,7 @@ describe('addAgent', () => { // The whole point of checking first: this used to surface as a raw // FileSystemError from the first credentials write, leaving the agent on // the gateway but nowhere else (CHOO-1416). - h.inspectRemoteDir.mockResolvedValue({ - dir: remote.dir, - status: 'missing', - existingAncestor: '/home/ubuntu', - } as never); + h.inspectRemoteDir.mockResolvedValue({ dir: remote.dir, status: 'missing' } as never); const result = await addAgent(params(remote)); @@ -215,11 +209,7 @@ describe('addAgent', () => { }); it('refuses a remote path that is a file', async () => { - h.inspectRemoteDir.mockResolvedValue({ - dir: remote.dir, - status: 'file', - existingAncestor: '', - } as never); + h.inspectRemoteDir.mockResolvedValue({ dir: remote.dir, status: 'file' } as never); expect((await addAgent(params(remote))).kind).toBe('directory-missing'); expect(h.registerAgentIdentity).not.toHaveBeenCalled(); @@ -232,6 +222,15 @@ describe('addAgent', () => { expect(result.kind).toBe('created'); }); + // The credentials write creates a leaf under an existing parent by itself, + // so gating on "exists" would have broken a path that worked before. + it('proceeds when the remote directory does not exist but its parent does', async () => { + h.inspectRemoteDir.mockResolvedValue({ dir: remote.dir, status: 'creatable' } as never); + + expect((await addAgent(params(remote))).kind).toBe('created'); + expect(h.registerAgentIdentity).toHaveBeenCalled(); + }); + it('does not probe over SSH for a local agent', async () => { await addAgent(params()); expect(h.inspectRemoteDir).not.toHaveBeenCalled(); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts index 77515f7c7..d0b248e18 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/add-agent.ts @@ -9,7 +9,7 @@ import { log } from '@main/lib/logger'; import type { AgentProviderConfig } from '@shared/core/agents/agent-provider-config'; import type { Agent } from '@shared/core/agents/agents'; import type { AgentProviderId } from '@shared/core/providers/agent-provider-registry'; -import type { RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; +import { isUsableRemoteDir, type RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; import { basenameFromAnyPath } from '@shared/path-name'; import { agentEvents } from './agent-events'; import { agentNameTaken } from './agent-name-taken'; @@ -53,9 +53,9 @@ export type AddAgentResult = | { kind: 'unauthenticated' } | { kind: 'name-conflict' } | { kind: 'invalid-name'; message: string } - /** The remote working directory does not exist (or is a file). The user - * creates it on the host and retries; switchdash does not create it for them. - * Reported before anything is minted, so no Switch-side agent is left behind + /** The remote working directory is unusable: it is a file, or neither it nor + * its parent exists so the credentials write cannot create it. Reported + * before anything is minted, so no Switch-side agent is left behind * (CHOO-1416). */ | { kind: 'directory-missing'; sshHost: string; inspection: RemoteDirInspection } | { kind: 'error'; message: string }; @@ -75,7 +75,10 @@ export type AddAgentResult = * * Both run locations therefore check the working directory *before* minting the * identity — locally with `checkIsValidDirectory`, remotely with - * `inspectRemoteDir`. A missing remote directory used to surface as a raw + * `inspectRemoteDir`. The remote check refuses only what the write genuinely + * cannot handle: a file, or a directory whose parent is missing too. A missing + * directory under an existing parent still goes through and is created by the + * write, as it always was. The refused case used to surface as a raw * `FileSystemError` from the first credentials write, by which point the agent * existed on the gateway but nowhere else (CHOO-1416). Ordering the check first * removes that orphan for this failure; creation as a whole is still not atomic @@ -87,7 +90,7 @@ export async function addAgent(params: AddAgentParams): Promise } if (params.sshHost !== null) { const inspection = await inspectRemoteDir(params.sshHost, params.dir); - if (inspection.status !== 'directory') { + if (!isUsableRemoteDir(inspection)) { return { kind: 'directory-missing', sshHost: params.sshHost, inspection }; } } diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts index 4f8cb499c..20d8e5a66 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.test.ts @@ -38,69 +38,54 @@ beforeEach(() => { }); describe('inspectRemoteDir', () => { - it('reports an existing directory as usable', async () => { + it('reports an existing directory', async () => { existingDirs([REPO_DIR]); expect(await inspectRemoteDir('host', REPO_DIR)).toEqual({ dir: REPO_DIR, status: 'directory', - existingAncestor: '', }); }); - // The ticket's repro: the directory *and* its parent are absent, which is - // what the per-directory FS could not even see past (CHOO-1416). - it('finds the deepest existing ancestor when several are absent', async () => { - existingDirs(['/home/ubuntu']); + // Long-standing behaviour, and not something this ticket should take away: + // recursive mkdir may create the working directory itself, just not its + // ancestors, so a missing leaf under an existing parent needs no intervention. + it('reports a missing directory whose parent exists as creatable', async () => { + existingDirs(['/home/ubuntu/switch-agents']); expect(await inspectRemoteDir('host', REPO_DIR)).toEqual({ dir: REPO_DIR, - status: 'missing', - existingAncestor: '/home/ubuntu', + status: 'creatable', }); - // Opened at the host root: an FS rooted at the missing directory could not - // stat its way out to find what does exist. - expect(constructedWith).toEqual(['/']); }); - it('reports a single missing leaf under an existing parent', async () => { - existingDirs(['/home/ubuntu/switch-agents']); - - expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ - status: 'missing', - existingAncestor: '/home/ubuntu/switch-agents', - }); - }); - - // A misspelt username leaves `/home` as the deepest match, which is the - // signal that the path is wrong rather than merely unmade. - it('falls back to a shallow ancestor for a misspelt path', async () => { - existingDirs(['/home']); + // The ticket's repro: the parent is missing too, so the write cannot recover. + it('reports a missing directory whose parent is also missing', async () => { + existingDirs(['/home/ubuntu']); - expect(await inspectRemoteDir('host', '/home/louis_amauduz/repo')).toMatchObject({ + expect(await inspectRemoteDir('host', REPO_DIR)).toEqual({ + dir: REPO_DIR, status: 'missing', - existingAncestor: '/home', }); + // Opened at the host root: an FS rooted at the missing directory could not + // stat its way out to look at the parent. + expect(constructedWith).toEqual(['/']); }); - it('falls back to the root when no ancestor exists', async () => { - existingDirs([]); + it('reports a path that is a file', async () => { + stat.mockImplementation(async (path: string) => + path === REPO_DIR ? { path, type: 'file' } : null + ); - expect(await inspectRemoteDir('host', '/srv/agent')).toMatchObject({ - status: 'missing', - existingAncestor: '/', - }); + expect(await inspectRemoteDir('host', REPO_DIR)).toEqual({ dir: REPO_DIR, status: 'file' }); }); - it('reports a path that is a file', async () => { + it('refuses a directory whose parent is a file', async () => { stat.mockImplementation(async (path: string) => - path === REPO_DIR ? { path, type: 'file' } : null + path === '/home/ubuntu/switch-agents' ? { path, type: 'file' } : null ); - expect(await inspectRemoteDir('host', REPO_DIR)).toMatchObject({ - dir: REPO_DIR, - status: 'file', - }); + expect(await inspectRemoteDir('host', REPO_DIR)).toEqual({ dir: REPO_DIR, status: 'missing' }); }); // An unreadable path is not a missing one; saying so would send the user off diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts index c4d805b54..7c73fc54d 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/remote-dir.ts @@ -4,27 +4,18 @@ import { sshConnectionIdForHost } from '@main/core/locations/location-transport' import { ensureSshConnected } from '@main/core/ssh/connect/connect-agent-ssh'; import type { RemoteDirInspection } from '@shared/core/remote-hosts/remote-dir'; -/** Absolute ancestors of `dir`, deepest first, stopping above the root. */ -function ancestorsOf(dir: string): string[] { - const ancestors: string[] = []; - let current = pathPosix.dirname(dir); - while (current !== '/' && current !== '.' && !ancestors.includes(current)) { - ancestors.push(current); - current = pathPosix.dirname(current); - } - return ancestors; -} - /** - * Inspect a prospective remote working directory on `sshHost`: does it exist, - * is it actually a directory, and if not, what is the deepest part of the path - * that does exist (CHOO-1416). + * Inspect a prospective remote working directory on `sshHost` (CHOO-1416). * - * The filesystem is opened at the host's root rather than at the directory - * under test, because the directory under test is exactly what may not exist. - * Every other caller roots its FS at an agent's working directory, which also - * scopes that FS's path-traversal guard to it — so an FS rooted at a missing - * directory cannot even stat its way out to find what is there instead. + * Only two stats, because only two things decide the outcome: whether the + * directory is there, and — if not — whether its parent is. A missing + * directory under an existing parent is created by the first credentials + * write, as it always has been; a missing parent is not, because the + * working directory's own FS is rooted at the directory and its recursive + * mkdir stops there. + * + * The FS here is opened at the host root instead, since one rooted at a + * missing directory cannot stat its way out to look at the parent. * * `dir` must be absolute — a relative path would resolve against whatever * directory the SSH session happens to start in, which is not a thing the user @@ -46,23 +37,11 @@ export async function inspectRemoteDir(sshHost: string, dir: string): Promise -

- Couldn’t check the working directory on {sshHost} -

-

{error.message}

+ Couldn’t check the working directory on {sshHost} — {error.message}
); } - if (!inspection || inspection.status === 'directory') return null; - - if (inspection.status === 'file') { - return ( - -

- {inspection.dir} is a file -

-

- An agent's working directory has to be a directory. Choose another path. -

-
- ); - } + if (!inspection || isUsableRemoteDir(inspection)) return null; return ( -

- {inspection.dir} does not exist on {sshHost} -

-

- The deepest part of that path that exists is{' '} - {inspection.existingAncestor}. -

-

- Create the directory on the host and set the location again, or correct the path. -

+ {inspection.status === 'file' ? ( + <> + {inspection.dir} is a file, not a directory. + + ) : ( + <> + {inspection.dir} does not exist on {sshHost}. + Create it there first. + + )}
); } function NoticeShell({ children }: { children: React.ReactNode }) { return ( -
+
{children}
); diff --git a/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts b/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts index abfdf29dc..cfcb117e8 100644 --- a/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts +++ b/dash/apps/switchdash-desktop/src/shared/core/remote-hosts/remote-dir.ts @@ -6,20 +6,24 @@ * the SSH host is probed for reachability, the server is picked from a list, * but the directory was only ever touched at write time — by which point an * identity had already been minted on the gateway. - * - * switchdash does not create the directory. Making one on someone's host is - * their decision, and a path typed by hand is exactly where a typo would be - * silently turned into a real, wrong directory. Inspecting it up front is only - * so the flow can say which path is missing, while the field is still on screen. */ /** What an inspection found at a remote path. */ export type RemoteDirStatus = - /** The path exists and is a directory — usable as a working directory. */ + /** Exists and is a directory. */ | 'directory' - /** The path exists but is a regular file. */ + /** + * Does not exist, but its parent does, so the first credentials write creates + * it — which is what already happened before this check existed. Usable. + */ + | 'creatable' + /** Exists, but is a regular file. */ | 'file' - /** The path does not exist. */ + /** + * Neither the directory nor its parent exists. This is the failing case: a + * working directory's FS is rooted at the directory itself, and its recursive + * mkdir will not create anything above that root. + */ | 'missing'; /** The result of inspecting a prospective remote working directory. */ @@ -27,14 +31,9 @@ export type RemoteDirInspection = { /** The absolute path inspected, as resolved on the host. */ dir: string; status: RemoteDirStatus; - /** - * Deepest ancestor of `dir` that does exist, e.g. `/home/ubuntu` for a - * missing `/home/ubuntu/switch-agents/internal-deployments`. Empty when the - * path is not `missing`. - * - * Worth surfacing because it separates the two ways this goes wrong: an - * ancestor one level up means the directory simply has not been made yet, - * whereas a much shallower one means the path is probably misspelt. - */ - existingAncestor: string; }; + +/** Whether an agent can be created in this directory. */ +export function isUsableRemoteDir(inspection: RemoteDirInspection): boolean { + return inspection.status === 'directory' || inspection.status === 'creatable'; +} From c8df3e084e48f2d21b7ca7fdd7efaf23cd413304 Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Thu, 6 Aug 2026 16:35:30 +0000 Subject: [PATCH 05/15] fix(switchdash): don't ask for agent details a location cannot hold (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With a missing working directory the modal showed the warning and then went on rendering the whole agent form beneath it — name, description, auto-session, advanced config — inviting the user to describe an agent that could not be created. The modal already gates on a cascade of prerequisites: a reachable host lets you pick a type, a ready one lets you configure. The agent-level fields now also wait for the two things they describe an agent *of*: a chosen type, and a working directory that will exist to hold it. The directory field and the type picker are deliberately untouched — they are how the warning gets resolved. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/add-agent-modal/add-agent-modal.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx index 6873405f8..8bcbd9d22 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx @@ -401,7 +401,13 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc // picker stays live and you can pick one the host already has. const hostLevelBlocked = isRemoteRun && hostReadiness.blocked && hostReadiness.scope === 'host'; const canChooseAgentType = runHostReachable && !hostLevelBlocked; - const canConfigureAgent = canChooseAgentType && runHostReady; + // Everything that describes the agent itself — its name, config, the + // definitions found alongside it — waits for the two things it is an agent + // *of*: a chosen type, and a working directory that will exist to hold it. + // Without both, those fields ask the user to describe something that cannot + // be created, directly under a notice saying so (CHOO-1416). + const canConfigureAgent = + canChooseAgentType && runHostReady && !!pickState.providerId && remoteDirUsable; const canSubmitDetected = isRemoteRun ? !!pickState.providerId && From 3172bd7a15dc839f0ec5bcc2b86294ab540d9d09 Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Thu, 6 Aug 2026 16:43:21 +0000 Subject: [PATCH 06/15] fix(switchdash): a pending directory check is not a failed one (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults from gating the agent fields on the directory check, both from treating "no verdict yet" as a negative verdict. The gate read `data !== undefined && usable`, so it was false while the probe was in flight and false forever if the probe errored. That hid the entire agent form during the check, and could hide it permanently — including where a Switch agent had just been detected in the directory, which proves the directory exists. An agent being found somewhere is never a reason to refuse adding another one there. It now blocks only on an actual verdict. A probe in flight or a failed probe leaves the form alone; a failed probe is still reported by the notice, and `addAgent` re-checks server-side before minting anything, so nothing rests on the UI having been right. Switching agent type also withdrew the working-directory field, because readiness is probed per type and the field was gated on that probe. The directory is not a function of the agent type. It is now held back only before one has been set, which is the case that gate was added for. Co-Authored-By: Claude Opus 5 (1M context) --- .../add-agent-modal/add-agent-modal.tsx | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx index 8bcbd9d22..a6a8c7fe9 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx @@ -230,9 +230,15 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc enabled: shouldDetectRemote, retry: false, }); - const remoteDirUsable = - !shouldDetectRemote || - (remoteDirQuery.data !== undefined && isUsableRemoteDir(remoteDirQuery.data)); + // Only a verdict blocks. A probe still in flight, or one that failed, is not + // evidence against the directory — treating it as such hid the whole form + // while the check ran, and hid it for good if the check errored, including in + // the case where a Switch agent had just been detected in that very directory + // and so it demonstrably existed. + const remoteDirBlocked = + shouldDetectRemote && + remoteDirQuery.data !== undefined && + !isUsableRemoteDir(remoteDirQuery.data); // Provider agents defined in the picked dir (`.claude/agents/*.md`) — both those // already set up for Switch and plain provider subagents a user created directly. @@ -407,7 +413,14 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc // Without both, those fields ask the user to describe something that cannot // be created, directly under a notice saying so (CHOO-1416). const canConfigureAgent = - canChooseAgentType && runHostReady && !!pickState.providerId && remoteDirUsable; + canChooseAgentType && runHostReady && !!pickState.providerId && !remoteDirBlocked; + // The working directory is not a function of the agent type. Switching type + // re-probes host readiness for the new type, and withdrawing the field + // mid-probe made it look as though the location itself were being rechecked. + // Hold it back only before anything has been committed, which is the case + // `hostReadiness.checking` was gating in the first place. + const canChooseLocation = + canChooseAgentType && (!hostReadiness.checking || trimmedRemoteDir.length > 0); const canSubmitDetected = isRemoteRun ? !!pickState.providerId && @@ -416,7 +429,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc !!switchAgent && verifyState === 'found' && runHostReachable && - remoteDirUsable && + !remoteDirBlocked && runHostReady && submitState === 'idle' : pickState.isValid && @@ -433,7 +446,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc !!pickState.providerId && remoteRunValid && runHostReachable && - remoteDirUsable && + !remoteDirBlocked && runHostReady && submitState === 'idle'; @@ -450,7 +463,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc !!pickState.providerId && trimmedRemoteDir.length > 0 && runHostReachable && - remoteDirUsable && + !remoteDirBlocked && runHostReady && submitState === 'idle'; @@ -814,8 +827,9 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc )} {/* Not while we are still finding out what the host has: asking for a working directory under a "checking…" spinner invites the user to - fill in a form we may be about to refuse. */} - {isRemoteRun && canChooseAgentType && !hostReadiness.checking && ( + fill in a form we may be about to refuse. Once a directory is set, + it stays put — a later probe is about the agent type, not the path. */} + {isRemoteRun && canChooseLocation && (
Date: Thu, 6 Aug 2026 18:09:08 +0000 Subject: [PATCH 07/15] fix(switchdash): an agent already in a directory does not block adding another (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pointing the add-agent modal at a directory that already held a Switch agent offered exactly one thing: adopt that agent. Once adopted there was nothing left to do, and no way to add a second agent there. The create flow was gated on `!switchAgent` — the absence of a Switch identity in the legacy shared settings file. CHOO-1440 made a directory a flat container of agents, and the comment above that very gate says so, but the gate kept the pre-1440 assumption that a directory holds one agent. A detected agent is now treated the way a discovered definition already was: it leads with adopting it, and "Create a new agent instead" is offered alongside. The two gates are renamed for what they now decide (`canCreateLocalAgent` / `canCreateRemoteAgent`) rather than for the absence they used to test. Co-Authored-By: Claude Opus 5 (1M context) --- .../add-agent-modal/add-agent-modal.tsx | 77 +++++++++++-------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx index a6a8c7fe9..90d880d80 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx @@ -329,7 +329,6 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc setSelectedNames(new Set(onboardableKey ? onboardableKey.split('|') : [])); setCreateMode(false); }, [onboardableKey]); - const showCreate = !hasOnboardable || createMode; const toggleSelected = useCallback((name: string, checked: boolean) => { setSelectedNames((prev) => { const next = new Set(prev); @@ -355,14 +354,23 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc const switchAgent = isRemoteRun ? (remoteAgentQuery.data ?? null) : (inspection?.switchAgent ?? null); - // A local directory with no legacy Switch agent config offers the create flow. - // This is available even when the directory already contains other agents — a - // directory is a flat container, so you can always add another (CHOO-1440); - // onboarding pre-existing definitions is offered alongside it (see below). - const isMissingSwitchAgent = !isRemoteRun && !isChecking && shouldCheckPathStatus && !switchAgent; - // A remote dir with no Switch agent offers the remote configure flow: register - // the agent on the server and write its creds into the remote dir over SSH. - const isMissingRemoteAgent = isRemoteRun && !isChecking && shouldDetectRemote && !switchAgent; + // Anything already in the directory that can be taken on as-is, rather than + // created: a discovered definition, or an agent configured in the legacy + // shared settings file. Either way the modal leads with adopting it and keeps + // `createMode` as the way past it. + const hasAdoptable = hasOnboardable || !!switchAgent; + const showCreate = !hasAdoptable || createMode; + // Creating an agent is offered for any directory we have finished inspecting. + // A directory is a flat container, so an agent already living there is no + // reason to refuse another — an existing one used to suppress the create flow + // outright, which left a configured directory with nothing on offer but the + // one agent already in it. + const canCreateAgentHere = + !isChecking && (isRemoteRun ? shouldDetectRemote : shouldCheckPathStatus); + const canCreateLocalAgent = !isRemoteRun && canCreateAgentHere && showCreate; + // The remote create flow registers the agent on the server and writes its + // creds into the remote dir over SSH. + const canCreateRemoteAgent = isRemoteRun && canCreateAgentHere && showCreate; // An agent always binds to the active Switch server, so there is no server // picker. Silently verify the detected agent exists on that server to gate @@ -438,7 +446,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc verifyState === 'found' && submitState === 'idle'; const canSubmitConfigure = - isMissingSwitchAgent && + canCreateLocalAgent && !isChecking && configureForm.isValid && !policyHasDeadRule(configureForm.addressingPolicy) && @@ -455,7 +463,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc // here (the agent does not exist yet); it is verified after registration by // the create path (createRemoteLocation). const canSubmitConfigureRemote = - isMissingRemoteAgent && + canCreateRemoteAgent && !isChecking && remoteConfigureForm.isValid && !policyHasDeadRule(remoteConfigureForm.addressingPolicy) && @@ -750,7 +758,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc } footer={ - {switchAgent ? ( + {switchAgent && !createMode ? ( void handleSubmit()} @@ -766,7 +774,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc > {submitState === 'creating' ? 'Adding...' : `Add ${selectedNames.size} selected`} - ) : isMissingRemoteAgent ? ( + ) : canCreateRemoteAgent ? ( void handleConfigureRemote()} @@ -774,7 +782,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc > {submitState === 'creating' ? 'Registering...' : 'Configure & Add Agent'} - ) : isMissingSwitchAgent ? ( + ) : canCreateLocalAgent ? ( void handleConfigure()} @@ -875,29 +883,32 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc /> )} {canConfigureAgent && !isRemoteRun && ( - + )} {isChecking && (

Scanning directory for agents…

)} {canConfigureAgent && hasOnboardable && !createMode && ( - <> - - - + + )} + {/* Offered for a detected agent too, not just a discovered definition: + either way something is already here, and adding another alongside it + is allowed. */} + {canConfigureAgent && hasAdoptable && !createMode && ( + )} - {hasOnboardable && createMode && ( + {hasAdoptable && createMode && ( -
-
- )} - - )} {canConfigureAgent && canCreateLocalAgent && ( <> ({ @@ -49,18 +49,6 @@ vi.mock('@renderer/lib/stores/view-state-cache', () => ({ }, })); -function location(overrides: Partial = {}): Location { - return { - id: 'location-id', - name: 'Location', - sshHost: null, - dir: '/location', - createdAt: '2026-05-28T00:00:00.000Z', - updatedAt: '2026-05-28T00:00:00.000Z', - ...overrides, - }; -} - function agent(overrides: Partial = {}): Agent { return { id: 'agent-id', @@ -79,109 +67,6 @@ function agent(overrides: Partial = {}): Agent { }; } -describe('LocationManagerStore agent onboarding', () => { - beforeEach(() => { - vi.clearAllMocks(); - mocks.inspectLocationPath.mockResolvedValue({ isDirectory: true }); - mocks.onboardAgent.mockResolvedValue({ success: true, data: agent() }); - mocks.getLocations.mockResolvedValue([location()]); - mocks.openLocation.mockReturnValue(new Promise(() => {})); - }); - - it('returns an existing location without starting onboarding', async () => { - mocks.inspectLocationPath.mockResolvedValueOnce({ - isDirectory: true, - existingLocation: location({ id: 'existing-location' }), - }); - const store = new LocationManagerStore(); - - const result = await store.startAgentOnboarding( - { - mode: 'pick', - name: 'Location', - path: '/location', - serverId: 'server-1', - providerId: 'claude', - }, - { id: 'optimistic-location' } - ); - - expect(result).toEqual({ kind: 'existing', locationId: 'existing-location' }); - expect(mocks.onboardAgent).not.toHaveBeenCalled(); - expect(store.locations.has('optimistic-location')).toBe(false); - expect(store.pendingCreationIds.has('optimistic-location')).toBe(false); - }); - - it('creates unregistered location state before returning creating', async () => { - let resolveOnboard: (a: Agent) => void = () => {}; - mocks.onboardAgent.mockReturnValueOnce( - new Promise((resolve) => { - resolveOnboard = (a) => resolve({ success: true, data: a }); - }) - ); - const store = new LocationManagerStore(); - - const result = await store.startAgentOnboarding( - { - mode: 'pick', - name: 'Location', - path: '/location', - serverId: 'server-1', - providerId: 'claude', - }, - { id: 'optimistic-location' } - ); - - expect(result.kind).toBe('creating'); - const pending = store.locations.get('optimistic-location'); - expect(pending && isUnregisteredLocation(pending)).toBe(true); - expect(pending?.phase).toBe('registering'); - expect(store.pendingCreationIds.has('optimistic-location')).toBe(true); - expect(mocks.inspectLocationPath).toHaveBeenCalledTimes(1); - - resolveOnboard(agent()); - if (result.kind === 'creating') await result.completion; - - expect(store.pendingCreationIds.has('optimistic-location')).toBe(false); - }); - - it('marks onboarding as failed when the RPC returns a typed error', async () => { - mocks.onboardAgent.mockResolvedValueOnce({ - success: false, - error: { - type: 'switch-server-unauthenticated', - dir: '/location', - serverId: 'server-1', - serverName: 'Pilot', - }, - }); - const store = new LocationManagerStore(); - - const result = await store.startAgentOnboarding( - { - mode: 'pick', - name: 'Location', - path: '/location', - serverId: 'server-1', - providerId: 'claude', - }, - { id: 'optimistic-location' } - ); - - expect(result.kind).toBe('creating'); - if (result.kind === 'creating') { - await expect(result.completion).resolves.toMatchObject({ success: false }); - } - - const loc = store.locations.get('optimistic-location'); - expect(loc && isUnregisteredLocation(loc)).toBe(true); - if (loc && isUnregisteredLocation(loc)) { - expect(loc.phase).toBe('error'); - expect(loc.error).toBe('Sign in to Pilot before adding this agent.'); - } - }); -}); - describe('LocationManagerStore removeAgent', () => { // removeAgent only gets/deletes the location by key, so a placeholder stands in // for the (heavy) real LocationStore. diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/stores/location-manager.ts b/dash/apps/switchdash-desktop/src/renderer/features/locations/stores/location-manager.ts index 8a83b1907..1aa7904d5 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/stores/location-manager.ts +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/stores/location-manager.ts @@ -1,4 +1,3 @@ -import { err, ok } from '@switchdash/shared'; import { makeObservable, observable, runInAction } from 'mobx'; import { events, rpc } from '@renderer/lib/ipc'; import { appState } from '@renderer/lib/stores/app-state'; @@ -6,17 +5,10 @@ import { viewStateCache } from '@renderer/lib/stores/view-state-cache'; import { type Location } from '@shared/core/locations/locations'; import { hostReachabilityEventChannel } from '@shared/core/remote-hosts/reachability'; import type { LocationViewSnapshot } from '@shared/view-state'; -import type { - AgentOnboardingCompletion, - AgentOnboardingError, - ModeData, - StartAgentOnboardingOptions, - StartAgentOnboardingResult, -} from './agent-onboarding-types'; +import type { AgentOnboardingError } from './agent-onboarding-types'; import { agentsStore } from './agents-store'; import { createUnmountedLocation, - createUnregisteredLocation, isUnmountedLocation, isUnregisteredLocation, type LocationStore, @@ -87,14 +79,6 @@ export class LocationManagerStore { await Promise.allSettled(toMount.map((id) => this.mountLocation(id))); } - async createAgent(data: ModeData, id?: string): Promise { - const result = await this.startAgentOnboarding(data, { id }); - if (result.kind === 'existing') return result.locationId; - - const completion = await result.completion; - return completion.success ? result.locationId : undefined; - } - /** * Add a brand-new agent to a location (local or remote): mint its identity, * write its definition + credentials, and create the row — all server-side via @@ -118,82 +102,6 @@ export class LocationManagerStore { return result; } - async startAgentOnboarding( - data: ModeData, - options: StartAgentOnboardingOptions = {} - ): Promise { - const placeholderId = options.id ?? crypto.randomUUID(); - const dir = data.remote ? data.remote.dir : data.path; - // Local onboarding can dedup against an existing location for the same dir. - if (!data.remote && dir !== undefined) { - const inspection = await rpc.locations.inspectLocationPath({ path: dir }); - if (inspection.existingLocation) { - return { kind: 'existing', locationId: inspection.existingLocation.id }; - } - } - - runInAction(() => { - this.pendingCreationIds.add(placeholderId); - this.locations.set( - placeholderId, - createUnregisteredLocation(placeholderId, data.name, 'registering', 'pick') - ); - }); - - const completion = this._doOnboardAgent(data, placeholderId).finally(() => { - runInAction(() => this.pendingCreationIds.delete(placeholderId)); - }); - - return { kind: 'creating', locationId: placeholderId, completion }; - } - - private async _doOnboardAgent( - data: ModeData, - placeholderId: string - ): Promise { - const dir = data.remote ? data.remote.dir : data.path; - if (dir === undefined) { - const error: AgentOnboardingError = { - type: 'invalid-directory', - dir: '', - message: 'A directory is required', - }; - this._markCreationError(placeholderId, error); - return err(error); - } - - let result: AgentOnboardingCompletion; - try { - const onboarded = await rpc.agents.onboardAgent({ - name: data.name, - serverId: data.serverId, - providerId: data.providerId, - dir, - sshHost: data.remote?.sshHost, - autoApprove: data.autoApprove, - }); - if (!onboarded.success) { - result = err(onboarded.error); - } else { - const location = (await rpc.locations.getLocations()).find( - (l) => l.id === onboarded.data.locationId - ); - if (!location) - throw new Error(`Onboarded agent's location ${onboarded.data.locationId} not found`); - // Drop the optimistic placeholder; key the store by the real location id. - runInAction(() => this.locations.delete(placeholderId)); - this._setAndOpenLocation(location.id, location); - result = ok(); - } - } catch (error) { - this._markUnexpectedCreationError(placeholderId, error); - throw error; - } - - if (!result.success) this._markCreationError(placeholderId, result.error); - return result; - } - mountLocation(locationId: string): Promise { const inFlight = this._locationMountPromises.get(locationId); if (inFlight) return inFlight; diff --git a/dash/apps/switchdash-desktop/src/shared/core/agents/onboarding.ts b/dash/apps/switchdash-desktop/src/shared/core/agents/onboarding.ts index 948073cfc..8b502e313 100644 --- a/dash/apps/switchdash-desktop/src/shared/core/agents/onboarding.ts +++ b/dash/apps/switchdash-desktop/src/shared/core/agents/onboarding.ts @@ -1,31 +1,11 @@ import type { Result } from '@switchdash/shared'; import type { Agent } from '@shared/core/agents/agents'; -import type { AgentProviderId } from '@shared/core/providers/agent-provider-registry'; /** - * Onboarding creates an agent at a location, creating the location row first - * if no location exists yet for (sshHost, dir). The directory must already be - * configured as a Switch agent (its `.claude/settings.local.json` resolves an - * identity) — minting a new identity happens before this call, via - * `switchServers.provisionAgent` / `provisionRemoteAgent`. + * How adopting an existing agent at a location can fail. Shared by the attach + * path (`attachConfiguredAgents`) and the definition path + * (`onboardLocationAgents`). */ -export type OnboardAgentParams = { - id?: string; - name: string; - /** The registered Switch server the user chose for this agent. The agent must - * exist on it — verified server-side at onboard time. */ - serverId: string; - /** The agent type (CLI provider) the user picked when onboarding. */ - providerId: AgentProviderId; - /** Where the agent runs: `~/.ssh/config` Host alias (omit for this machine) - * plus the absolute working directory on that host. */ - sshHost?: string; - dir: string; - /** Seed for the per-agent bypass-permissions flag. Omit to take the - * default (false for local, true for remote). */ - autoApprove?: boolean; -}; - export type OnboardAgentError = | { type: 'invalid-directory'; dir: string; message: string } /** The chosen Switch server does not have this agent — the user picked the diff --git a/dash/apps/switchdash-desktop/src/shared/core/locations/locations.ts b/dash/apps/switchdash-desktop/src/shared/core/locations/locations.ts index 64487b265..a36a6ec6c 100644 --- a/dash/apps/switchdash-desktop/src/shared/core/locations/locations.ts +++ b/dash/apps/switchdash-desktop/src/shared/core/locations/locations.ts @@ -1,5 +1,3 @@ -import type { SwitchAgentConfig } from '@shared/switch-agents'; - /** * A Location: where agents' sessions run — a working directory on a host. * Local locations live on this machine (`sshHost` null); remote ones on an @@ -34,12 +32,6 @@ export type InspectLocationPathParams = { export type LocationPathInspection = LocationPathStatus & { existingLocation?: Location; - /** - * The Switch agent configured in this directory, if any (read from the dir's - * `.claude/settings.local.json`). switchdash only allows onboarding - * directories that resolve a Switch agent. - */ - switchAgent?: SwitchAgentConfig | null; }; export type OpenLocationError = From 97794f160732ad55d4f89d1b96b8eac36aad3f92 Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Thu, 6 Aug 2026 18:50:22 +0000 Subject: [PATCH 13/15] refactor(switchdash): stop reading credentials from the shared settings file (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finishes removing `.claude/settings.local.json` as a Switch identity. The detection half went with the previous commit; this is everything that still read it as a last-resort credentials source: - the session preflight's candidate list, the notification poller, the auto-session watcher, and the sidecar, which each tried the per-agent file and then fell through to the shared one; - `readSwitchAgentCredentials`, the reader they shared; - the delete-time teardown (`removeSwitchCredentials`, `removeSwitchSettings`) that stripped our keys back out of it; - the migration's third recovery source, and with it the last use of `SWITCH_SETTINGS_RELATIVE_PATH`, now deleted. The sidecar's agent slug becomes required rather than optional: it was optional only so the fallback could cover a sidecar launched by an older build, and with no fallback an absent slug is a bug worth failing on. The migration's identity cross-check survives, exercised through the id-keyed neutral file instead — several agents can share a directory, so a credentials file found there is still not necessarily this agent's. Comments describing the old fallbacks are updated rather than left to rot; `.claude/settings.local.json` still appears where it is genuinely Claude Code's own file — the hooks writer, and why a missing identity degrades differently for Claude than for every other provider. Co-Authored-By: Claude Opus 5 (1M context) --- .../agent-runtime/impl/local-agent-runtime.ts | 7 +- .../src/main/core/agents/deleteAgent.ts | 2 - .../core/agents/migrate-agent-storage.test.ts | 51 ++------- .../main/core/agents/migrate-agent-storage.ts | 18 +-- .../propagate-server-api-url.db.test.ts | 42 +------ .../agents/remove-switch-settings.test.ts | 78 ------------- .../core/agents/remove-switch-settings.ts | 38 ------- .../src/main/core/agents/renameAgent.ts | 4 +- .../core/agents/switch-settings-paths.test.ts | 3 - .../main/core/agents/switch-settings-paths.ts | 7 -- .../core/agents/write-switch-settings.test.ts | 103 ----------------- .../main/core/agents/write-switch-settings.ts | 105 ++---------------- .../locations/location-runtime-factory.ts | 2 +- .../core/sessions/remote-session-preflight.ts | 10 +- .../src/main/core/sessions/session-builder.ts | 11 +- .../core/switch-rooms/auto-session-watcher.ts | 13 +-- .../core/switch-rooms/switch-credentials.ts | 29 +---- .../switch-notification-poller.ts | 11 +- .../switchdash-desktop/src/main/db/schema.ts | 4 +- .../add-agent-modal/add-agent-modal.tsx | 8 +- .../src/shared/core/agents/agents.ts | 2 +- .../switchdash-desktop/src/sidecar/index.ts | 29 ++--- .../src/sidecar/sidecar-runtime.ts | 2 +- 23 files changed, 67 insertions(+), 512 deletions(-) delete mode 100644 dash/apps/switchdash-desktop/src/main/core/agents/remove-switch-settings.test.ts delete mode 100644 dash/apps/switchdash-desktop/src/main/core/agents/remove-switch-settings.ts diff --git a/dash/apps/switchdash-desktop/src/main/core/agent-runtime/impl/local-agent-runtime.ts b/dash/apps/switchdash-desktop/src/main/core/agent-runtime/impl/local-agent-runtime.ts index 64d8be896..c09e26ab0 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agent-runtime/impl/local-agent-runtime.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agent-runtime/impl/local-agent-runtime.ts @@ -167,13 +167,10 @@ export class LocalAgentRuntime implements AgentRuntimeProvider { cachedStatePath, }); - // A session talks to Switch as its own agent, not whatever identity happens - // to sit in `.claude/settings.local.json`. Real env vars outrank every + // A session talks to Switch as its own agent. Real env vars outrank every // settings file and reach the spawned MCP server, so inject the agent's // identity last (highest precedence): a subagent from its definition creds, - // and a plain agent from its provider-neutral `.switch/agents/.json` - // (empty when absent — the session then falls back to settings.local.json, - // which Claude reads natively). + // and a plain agent from its provider-neutral `.switch/agents/.json`. // Resolved before the command is built because a provider that registers // the Switch server at launch keys it on this identity (see below). const workspaceFs = createPluginFs(this.sessionPath); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/deleteAgent.ts b/dash/apps/switchdash-desktop/src/main/core/agents/deleteAgent.ts index cc334743d..adb151129 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/deleteAgent.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/deleteAgent.ts @@ -22,7 +22,6 @@ import { connectRemoteAgent } from './connect-remote-agent'; import { getAgentById } from './getAgentById'; import { stopRemoteWatcher } from './remote-watcher'; import { removeAgentLaunchProfile } from './remove-launch-profile'; -import { removeSwitchCredentials } from './remove-switch-settings'; import { agentSettingsRelativePath } from './switch-settings-paths'; export type DeleteAgentOptions = { @@ -96,7 +95,6 @@ async function removeProvisionedFiles(agent: Agent, location: Location): Promise error: String(error), }); }); - await removeSwitchCredentials(agent.providerId, ctx.fs); // A provider that registers the Switch server itself (Codex) leaves a // per-agent launch profile under the user's home — a different scope than // ctx.fs, reached through its own home filesystem (local or remote). diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.test.ts index 51a0cf465..e75ad2073 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.test.ts @@ -167,28 +167,11 @@ describe('migrateAgentStorage', () => { expect(h.writeDefinition).not.toHaveBeenCalled(); }); - it('falls back to .claude/settings.local.json when its identity matches the Claude row', async () => { - const ws = fakeFs({ - '.claude/settings.local.json': credsJson('sw-1'), - '.claude/agents/cc-hoot-main.md': '# def', - }); - h.state.workspace = ws; - - await migrateAgentStorage(); - - const written = JSON.parse((await ws.read('.switch/agents/cc-hoot-main.json')) as string); - expect(written.env.SWITCH_AGENT_ID).toBe('sw-1'); - expect(written.env.SWITCH_API_TOKEN).toBe('tok-123'); - expect(written.env.SWITCH_API_ENDPOINT).toBe('http://switch.example'); - expect(log.warn).not.toHaveBeenCalled(); - }); - - it('never reads .claude/settings.local.json for a provider without repo-agents', async () => { - // The shared settings file is written only by provisionAgent / - // provisionRemoteAgent, always as the Claude "main" agent, so for any other - // provider it is a different agent's identity and token. Adopting it would - // make the session launch AS that agent — a silent success where launching - // unidentified is a visible failure. + it('never reads .claude/settings.local.json', async () => { + // The shared settings file is no longer an identity source for anyone: it + // held one "main" agent per directory, which is not a thing since CHOO-1440. + // Adopting it would launch this agent AS whoever that was — a silent success + // where launching unidentified is a visible failure. h.state.agents = [ { ...baseAgent, providerId: 'codex', name: 'codex-hoot', switchAgentId: 'sw-codex' }, ]; @@ -203,27 +186,11 @@ describe('migrateAgentStorage', () => { expect(await ws.exists('.switch/agents/codex-hoot.json')).toBe(false); }); - it('does not adopt .claude/settings.local.json for a provider without repo-agents when the row has no identity to compare', async () => { - // A row with no `switchAgentId` has nothing to compare against, so the - // behavior gate — not the identity cross-check — is what keeps the Claude - // main agent's credentials out of this agent's file. - h.state.agents = [ - { ...baseAgent, providerId: 'codex', name: 'codex-hoot', switchAgentId: null }, - ]; - h.state.repoAgents = null; - const ws = fakeFs({ '.claude/settings.local.json': credsJson('sw-CLAUDE-MAIN') }); - h.state.workspace = ws; - - await migrateAgentStorage(); - - expect(await ws.exists('.switch/agents/codex-hoot.json')).toBe(false); - }); - - it('skips visibly when .claude/settings.local.json names a different agent than the Claude row', async () => { - // Multiple Claude agents can share a location; only one of them owns the - // shared settings file. Adopting a mismatched identity is never right. + it('skips visibly when a recovered file names a different agent than the row', async () => { + // Several agents can share a location, so a credentials file found there is + // not necessarily this agent's. Adopting a mismatched identity is never right. const ws = fakeFs({ - '.claude/settings.local.json': credsJson('sw-other'), + '.switch/agents/agent-id-1.json': credsJson('sw-other'), '.claude/agents/cc-hoot-main.md': '# def', }); h.state.workspace = ws; diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.ts b/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.ts index 18c07062d..ce1f0be1e 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.ts @@ -13,7 +13,7 @@ import { } from './agent-storage-migration-marker'; import { resolveWorkspaceFsFor } from './agent-workspace-fs'; import { getAgents } from './getAgents'; -import { agentSettingsRelativePath, SWITCH_SETTINGS_RELATIVE_PATH } from './switch-settings-paths'; +import { agentSettingsRelativePath } from './switch-settings-paths'; import { writeNeutralAgentSettingsFs } from './write-switch-settings'; /** @@ -21,8 +21,7 @@ import { writeNeutralAgentSettingsFs } from './write-switch-settings'; * layout (CHOO-1440): every agent is a repository-defined agent with a per-agent * credentials file at `.switch/agents/.json` and an on-disk definition, * both keyed by the agent's single `name`. Pre-CHOO-1440 installs kept - * credentials in the legacy `.claude/switch-subagents/.settings.json` (or - * the shared `.claude/settings.local.json`). + * credentials in the legacy `.claude/switch-subagents/.settings.json`. * * Runs once at boot, best-effort: each agent is migrated in isolation so one bad * directory or unreachable host never aborts the rest, and every step is @@ -101,12 +100,8 @@ async function migrateOne(agent: Agent, completedGeneration: number): Promise.settings.json`); - // c. the shared `.claude/settings.local.json` (legacy "main" agent). - // (b) and (c) are Claude-only sources, hence behavior-gated: the shared - // settings file is written solely by `provisionAgent`/`provisionRemoteAgent`, - // always as the Claude "main" agent, so for any other provider it holds a - // *different* agent's identity and token. + // then `.claude/switch-subagents/.settings.json`), a + // Claude-only source and hence behavior-gated. // The token is minted once and lives only on disk, so this is the only way // to recover it — nothing can reconstruct it from the gateway. const idKeyedRelPath = agentSettingsRelativePath(agent.id); @@ -115,10 +110,7 @@ async function migrateOne(agent: Agent, completedGeneration: number): Promise ({ db: undefined as AppDb | undefined, @@ -35,17 +35,6 @@ vi.mock('@main/core/locations/location-transport', () => ({ sshConnectionIdForHost: (host: string) => host, })); -async function writeSettings(dir: string, contents: Record): Promise { - const file = path.join(dir, SWITCH_SETTINGS_RELATIVE_PATH); - await nodeFs.mkdir(path.dirname(file), { recursive: true }); - await nodeFs.writeFile(file, JSON.stringify(contents, null, 2), 'utf8'); -} - -async function readEnv(dir: string): Promise> { - const raw = await nodeFs.readFile(path.join(dir, SWITCH_SETTINGS_RELATIVE_PATH), 'utf8'); - return (JSON.parse(raw) as { env: Record }).env; -} - /** Write the per-agent credentials file every agent has had since CHOO-1440. */ async function writeNeutral( dir: string, @@ -141,35 +130,6 @@ describe('propagateServerApiUrl', () => { expect(row?.apiEndpoint).toBe('https://new-api.example.com'); }); - // The shared settings file is no longer a credentials source, so a stray one - // left in a directory is none of propagation's business. - it('leaves a legacy settings file alone', async () => { - const dir = path.join(tmpRoot, 'both'); - const env = { - SWITCH_API_ENDPOINT: 'https://old-api.example.com', - SWITCH_API_TOKEN: 'secret-token', - SWITCH_AGENT_ID: 'switch-agent-8', - }; - await writeNeutral(dir, 'both-agent', { env }); - await writeSettings(dir, { env }); - await fixture.db.insert(locations).values({ id: 'loc-b', name: 'Local', sshHost: '', dir }); - await fixture.db.insert(agents).values({ - id: 'agent-b', - locationId: 'loc-b', - name: 'both-agent', - providerId: 'claude', - apiEndpoint: 'https://old-api.example.com', - serverId: 'pilot', - }); - - await propagateServerApiUrl('pilot', 'https://new-api.example.com'); - - expect((await readNeutralEnv(dir, 'both-agent')).SWITCH_API_ENDPOINT).toBe( - 'https://new-api.example.com' - ); - expect((await readEnv(dir)).SWITCH_API_ENDPOINT).toBe('https://old-api.example.com'); - }); - it('reports an unprovisioned agent as not-provisioned without writing a file', async () => { const dir = path.join(tmpRoot, 'unprovisioned'); await nodeFs.mkdir(dir, { recursive: true }); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remove-switch-settings.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remove-switch-settings.test.ts deleted file mode 100644 index 1ab4a3b3d..000000000 --- a/dash/apps/switchdash-desktop/src/main/core/agents/remove-switch-settings.test.ts +++ /dev/null @@ -1,78 +0,0 @@ -import type { PluginFs } from '@switchdash/core/agents/plugins'; -import { describe, expect, it } from 'vitest'; -import { removeSwitchCredentials } from './remove-switch-settings'; -import { mergeSwitchSettings } from './write-switch-settings'; - -const SETTINGS_PATH = '.claude/settings.local.json'; - -/** An in-memory PluginFs backed by a Map, enough for the default teardown path. */ -function fakeFs(initial: Record): PluginFs & { files: Map } { - const files = new Map(Object.entries(initial)); - return { - files, - read: (p: string) => Promise.resolve(files.get(p) ?? null), - write: (p: string, content: string) => { - files.set(p, content); - return Promise.resolve(); - }, - delete: (p: string) => { - files.delete(p); - return Promise.resolve(); - }, - exists: (p: string) => Promise.resolve(files.has(p)), - list: () => Promise.resolve([]), - }; -} - -// The claude provider declares no switchSetup behavior, so it exercises the -// default `.claude/settings.local.json` reverse-merge path. -describe('removeSwitchCredentials (default .claude teardown)', () => { - it('deletes a settings file that held only provisioned Switch credentials', async () => { - const fs = fakeFs({ - [SETTINGS_PATH]: mergeSwitchSettings(null, { - apiEndpoint: 'https://switch.example.com', - apiToken: 'secret-token', - agentId: 'agent-123', - }), - }); - - await removeSwitchCredentials('claude', fs); - - expect(fs.files.has(SETTINGS_PATH)).toBe(false); - }); - - it('preserves the user’s own keys, stripping only the Switch block', async () => { - const fs = fakeFs({ - [SETTINGS_PATH]: JSON.stringify({ - permissions: { allow: ['Bash', 'mcp__plugin_switch-connector_switch'] }, - env: { - EDITOR: 'vim', - SWITCH_API_ENDPOINT: 'e', - SWITCH_API_TOKEN: 't', - SWITCH_AGENT_ID: 'a', - }, - }), - }); - - await removeSwitchCredentials('claude', fs); - - const parsed = JSON.parse(fs.files.get(SETTINGS_PATH)!) as Record; - expect(parsed.env).toEqual({ EDITOR: 'vim' }); - expect(parsed.permissions).toEqual({ allow: ['Bash'] }); - }); - - it('leaves a file that is not a provisioned Switch agent untouched', async () => { - const original = JSON.stringify({ env: { EDITOR: 'vim' } }); - const fs = fakeFs({ [SETTINGS_PATH]: original }); - - await removeSwitchCredentials('claude', fs); - - expect(fs.files.get(SETTINGS_PATH)).toBe(original); - }); - - it('is a no-op when there is no settings file', async () => { - const fs = fakeFs({}); - await expect(removeSwitchCredentials('claude', fs)).resolves.toBeUndefined(); - expect(fs.files.size).toBe(0); - }); -}); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/remove-switch-settings.ts b/dash/apps/switchdash-desktop/src/main/core/agents/remove-switch-settings.ts deleted file mode 100644 index a88931bcb..000000000 --- a/dash/apps/switchdash-desktop/src/main/core/agents/remove-switch-settings.ts +++ /dev/null @@ -1,38 +0,0 @@ -import type { PluginFs } from '@switchdash/core/agents/plugins'; -import { getPlugin } from '@main/core/providers/plugin-registry'; -import { SWITCH_SETTINGS_RELATIVE_PATH } from './switch-settings-paths'; -import { removeSwitchSettings } from './write-switch-settings'; - -/** - * Reverse the default `.claude/settings.local.json` provisioning: strip the - * `SWITCH_*` env block and connector allow-rules, deleting the file if it was - * ours alone and leaving it untouched if it was never a provisioned Switch agent. - * Operates through `PluginFs`, so it works byte-identically for a local working - * directory and a remote SSH host. - */ -async function removeDefaultSwitchCredentials(fs: PluginFs): Promise { - const existing = await fs.read(SWITCH_SETTINGS_RELATIVE_PATH); - const result = removeSwitchSettings(existing); - if (result.kind === 'skip') return; - if (result.kind === 'delete') { - await fs.delete(SWITCH_SETTINGS_RELATIVE_PATH); - return; - } - await fs.write(SWITCH_SETTINGS_RELATIVE_PATH, result.content); -} - -/** - * Tear down the Switch credentials an agent of `providerId` wrote at provision - * time. Providers that store their credentials somewhere other than the default - * `.claude` layout own that teardown via `behavior.switchSetup.removeCredentials`; - * every other provider falls through to the default reverse-merge. `fs` is rooted - * at the agent's working directory (local or remote), so one call covers both. - */ -export async function removeSwitchCredentials(providerId: string, fs: PluginFs): Promise { - const behavior = getPlugin(providerId).behavior.switchSetup; - if (behavior) { - await behavior.removeCredentials(fs); - return; - } - await removeDefaultSwitchCredentials(fs); -} diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/renameAgent.ts b/dash/apps/switchdash-desktop/src/main/core/agents/renameAgent.ts index 2623c354e..0fd18b181 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/renameAgent.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/renameAgent.ts @@ -58,8 +58,8 @@ async function moveSidecarToNewName(previous: Agent, renamed: Agent): Promise`). A rename that only updates the row leaves both behind * under the old key, and the credentials are unrecoverable: the token is minted - * once and lives nowhere else, so the agent would silently fall back to the - * shared `.claude/settings.local.json` identity — possibly another agent's. + * once and lives nowhere else, so the agent would be left with no identity at + * all under its new name. * * The new files are written before the old ones are removed, so an interruption * leaves a recoverable duplicate rather than nothing. diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/switch-settings-paths.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/switch-settings-paths.test.ts index a5e3b09f0..27b2e99dd 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/switch-settings-paths.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/switch-settings-paths.test.ts @@ -3,7 +3,6 @@ import { agentSettingsRelativePath, SWITCH_AGENTS_DIR_RELATIVE, SWITCH_AGENTS_GITIGNORE_RELATIVE, - SWITCH_SETTINGS_RELATIVE_PATH, SWITCH_SUBAGENTS_DIR_RELATIVE, } from './switch-settings-paths'; @@ -12,7 +11,6 @@ import { // would, when switchdash itself runs on Windows. describe('relative Switch settings paths', () => { const relatives = { - SWITCH_SETTINGS_RELATIVE_PATH, SWITCH_SUBAGENTS_DIR_RELATIVE, SWITCH_AGENTS_DIR_RELATIVE, SWITCH_AGENTS_GITIGNORE_RELATIVE, @@ -26,7 +24,6 @@ describe('relative Switch settings paths', () => { } it('resolves to the documented POSIX layout', () => { - expect(SWITCH_SETTINGS_RELATIVE_PATH).toBe('.claude/settings.local.json'); expect(SWITCH_SUBAGENTS_DIR_RELATIVE).toBe('.claude/switch-subagents'); expect(SWITCH_AGENTS_DIR_RELATIVE).toBe('.switch/agents'); expect(SWITCH_AGENTS_GITIGNORE_RELATIVE).toBe('.switch/agents/.gitignore'); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/switch-settings-paths.ts b/dash/apps/switchdash-desktop/src/main/core/agents/switch-settings-paths.ts index 8f8ef197b..9b18a22de 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/switch-settings-paths.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/switch-settings-paths.ts @@ -13,13 +13,6 @@ import path from 'node:path'; * forward-slash tail correctly on every platform. */ -/** - * Relative path, from an agent's working directory, to the Claude Code settings - * file that the switch-connector `configure` skill writes the `SWITCH_*` env - * block into for a per-location agent. - */ -export const SWITCH_SETTINGS_RELATIVE_PATH = '.claude/settings.local.json'; - /** * Directory, relative to an agent's working directory, where the switch-connector * `configure` skill writes per-subagent Switch credential files diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/write-switch-settings.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/write-switch-settings.test.ts index cdce67a9d..b645fe3b6 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/write-switch-settings.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/write-switch-settings.test.ts @@ -6,12 +6,9 @@ import { createPluginFs } from '@main/core/providers/plugin-fs'; import { agentSettingsRelativePath, SWITCH_AGENTS_GITIGNORE_RELATIVE, - SWITCH_SETTINGS_RELATIVE_PATH, } from './switch-settings-paths'; import { mergeSwitchApiEndpoint, - mergeSwitchSettings, - removeSwitchSettings, writeAgentNeutralSettings, writeNeutralAgentSettingsFs, } from './write-switch-settings'; @@ -179,103 +176,3 @@ describe('mergeSwitchApiEndpoint', () => { ).toBeNull(); }); }); - -describe('removeSwitchSettings', () => { - it('round-trips a freshly provisioned file to deletion', () => { - // A file written by mergeSwitchSettings with no pre-existing content is ours - // alone, so tearing it down should leave nothing behind. - const provisioned = mergeSwitchSettings(null, { - apiEndpoint: 'https://switch.example.com', - apiToken: 'secret-token', - agentId: 'agent-123', - }); - - expect(removeSwitchSettings(provisioned)).toEqual({ kind: 'delete' }); - }); - - it('strips only the SWITCH_* keys and connector rules, preserving everything else', () => { - const existing = JSON.stringify({ - permissions: { allow: ['Bash', 'mcp__plugin_switch-connector_switch'], deny: ['Read'] }, - hooks: { PostToolUse: [{ command: 'x' }] }, - env: { - EXISTING_KEY: 'keep-me', - SWITCH_API_ENDPOINT: 'https://switch.example.com', - SWITCH_API_TOKEN: 'secret-token', - SWITCH_AGENT_ID: 'agent-123', - }, - }); - - const result = removeSwitchSettings(existing); - expect(result.kind).toBe('write'); - const parsed = JSON.parse((result as { content: string }).content) as Record; - - // Our env keys are gone; the user's stays. - expect(parsed.env).toEqual({ EXISTING_KEY: 'keep-me' }); - // The connector allow rule is removed; the user's allow/deny rules stay. - expect(parsed.permissions).toEqual({ allow: ['Bash'], deny: ['Read'] }); - // Unrelated keys are untouched. - expect(parsed.hooks).toEqual({ PostToolUse: [{ command: 'x' }] }); - }); - - it('drops now-empty env and permissions blocks but keeps other keys', () => { - const existing = JSON.stringify({ - hooks: { PostToolUse: [{ command: 'x' }] }, - permissions: { - allow: [ - 'mcp__plugin_switch-connector_switch', - 'mcp__plugin_switch-connector_switch-channel', - ], - }, - env: { SWITCH_API_ENDPOINT: 'e', SWITCH_API_TOKEN: 't', SWITCH_AGENT_ID: 'a' }, - }); - - const result = removeSwitchSettings(existing); - expect(result.kind).toBe('write'); - const parsed = JSON.parse((result as { content: string }).content) as Record; - - // Both blocks held only our contributions — including the retired - // switch-channel rule an older switchdash wrote — so both are dropped. - expect(parsed).toEqual({ hooks: { PostToolUse: [{ command: 'x' }] } }); - expect('env' in parsed).toBe(false); - expect('permissions' in parsed).toBe(false); - }); - - it('skips files that are not a provisioned Switch agent', () => { - // Absent file. - expect(removeSwitchSettings(null)).toEqual({ kind: 'skip' }); - // Unparseable. - expect(removeSwitchSettings('{not json')).toEqual({ kind: 'skip' }); - // Empty object. - expect(removeSwitchSettings('{}')).toEqual({ kind: 'skip' }); - // A real config with no Switch creds -> leave it untouched. - expect(removeSwitchSettings(JSON.stringify({ env: { OTHER: 'x' } }))).toEqual({ kind: 'skip' }); - }); - - it('tears down even a partially-provisioned file (only some SWITCH_* keys)', () => { - // Defensive: if a write was interrupted and only the token survived, teardown - // must still remove it rather than leaving a dangling secret. - const existing = JSON.stringify({ env: { SWITCH_API_TOKEN: 'secret-token' } }); - expect(removeSwitchSettings(existing)).toEqual({ kind: 'delete' }); - }); - - it('leaves the directory undetectable as a Switch agent after teardown', async () => { - await nodeFs.mkdir(path.join(dir, '.claude'), { recursive: true }); - await nodeFs.writeFile( - path.join(dir, SWITCH_SETTINGS_RELATIVE_PATH), - mergeSwitchSettings(null, { - apiEndpoint: 'https://switch.example.com', - apiToken: 'secret-token', - agentId: 'agent-123', - }), - 'utf8' - ); - const raw = await nodeFs.readFile(path.join(dir, SWITCH_SETTINGS_RELATIVE_PATH), 'utf8'); - - const result = removeSwitchSettings(raw); - // The file was ours alone -> delete it, which makes the dir undetectable. - expect(result).toEqual({ kind: 'delete' }); - await nodeFs.rm(path.join(dir, SWITCH_SETTINGS_RELATIVE_PATH), { force: true }); - - await expect(nodeFs.access(path.join(dir, SWITCH_SETTINGS_RELATIVE_PATH))).rejects.toThrow(); - }); -}); diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/write-switch-settings.ts b/dash/apps/switchdash-desktop/src/main/core/agents/write-switch-settings.ts index be5e54ab0..7efe8f43b 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/write-switch-settings.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/write-switch-settings.ts @@ -1,7 +1,4 @@ -import { - RECOGNISED_SWITCH_CONNECTOR_TOOL_RULES, - SWITCH_CONNECTOR_TOOL_RULES, -} from '@switchdash/core/agents/plugins'; +import { SWITCH_CONNECTOR_TOOL_RULES } from '@switchdash/core/agents/plugins'; import type { PluginFs } from '@switchdash/core/agents/plugins'; import { createPluginFs } from '@main/core/providers/plugin-fs'; import { @@ -16,8 +13,8 @@ export interface SwitchSettingsCredentials { } /** - * Merge the `SWITCH_*` env block (and the connector tool-allow rules) into the - * contents of a `.claude/settings.local.json`, returning the new file text. + * Merge the `SWITCH_*` env block (and the connector tool-allow rules) into a + * Claude-settings-shaped credentials file, returning the new file text. * * The file is merged, not clobbered: any unrelated top-level keys and any other * `env` entries the user already has are preserved, and only the three @@ -25,8 +22,8 @@ export interface SwitchSettingsCredentials { * `permissions.allow` so they are auto-approved ("don't ask"). * * Pure: takes the existing file text (or null when absent/unreadable) and - * returns the text to write. Shared by the local writer and the remote (SFTP) - * writer so on-disk and over-SSH setup produce byte-identical files. + * returns the text to write, so a local write and one over SFTP produce + * byte-identical files. */ export function mergeSwitchSettings( existingRaw: string | null, @@ -128,97 +125,11 @@ export function mergeSwitchApiEndpoint( return `${JSON.stringify(merged, null, 2)}\n`; } -/** - * Reverse of {@link mergeSwitchSettings}: strip the `SWITCH_*` env block and the - * connector tool-allow rules that provisioning added, returning what to do with - * the file. Every other key — user env entries, other `permissions.allow` rules, - * `hooks`, and any unrelated top-level keys — is preserved byte-for-byte. - * - * The result is a small command rather than a bare string so the caller can tell - * "nothing of ours here, leave it" (`skip`) from "our keys were the only thing in - * the file, remove it" (`delete`) from "rewrite with our keys gone" (`write`): - * - `skip` — file absent/unparseable, or it carries no `SWITCH_*` credentials - * (not a provisioned agent) — do not touch it. - * - `delete` — after removing our keys the object is empty `{}` — the file was - * ours alone, so remove it rather than leaving an empty husk. - * - `write` — the cleaned file text, with our keys gone and everything else kept. - * - * Pure: takes the existing file text (or null when absent/unreadable) and returns - * the command. Shared by the local and remote (SFTP) teardown paths so both - * produce byte-identical results. - */ -export type RemoveSwitchSettingsResult = - | { kind: 'skip' } - | { kind: 'write'; content: string } - | { kind: 'delete' }; - -export function removeSwitchSettings(existingRaw: string | null): RemoveSwitchSettingsResult { - if (existingRaw === null) return { kind: 'skip' }; - - let parsed: unknown; - try { - parsed = JSON.parse(existingRaw); - } catch { - return { kind: 'skip' }; - } - if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return { kind: 'skip' }; - - const existing = parsed as Record; - const env = - existing.env && typeof existing.env === 'object' && !Array.isArray(existing.env) - ? { ...(existing.env as Record) } - : null; - - // No `SWITCH_*` credentials -> this is not a provisioned Switch agent; leave the - // file exactly as it is rather than rewriting someone else's config. - const hasSwitchCreds = - env !== null && - ('SWITCH_API_ENDPOINT' in env || 'SWITCH_API_TOKEN' in env || 'SWITCH_AGENT_ID' in env); - if (!hasSwitchCreds) return { kind: 'skip' }; - - const result: Record = { ...existing }; - - delete env.SWITCH_API_ENDPOINT; - delete env.SWITCH_API_TOKEN; - delete env.SWITCH_AGENT_ID; - if (Object.keys(env).length > 0) { - result.env = env; - } else { - delete result.env; - } - - const perms = - existing.permissions && - typeof existing.permissions === 'object' && - !Array.isArray(existing.permissions) - ? { ...(existing.permissions as Record) } - : null; - if (perms && Array.isArray(perms.allow)) { - const allow = (perms.allow as unknown[]) - .map(String) - .filter((rule) => !RECOGNISED_SWITCH_CONNECTOR_TOOL_RULES.includes(rule)); - if (allow.length > 0) { - perms.allow = allow; - } else { - delete perms.allow; - } - if (Object.keys(perms).length > 0) { - result.permissions = perms; - } else { - delete result.permissions; - } - } - - if (Object.keys(result).length === 0) return { kind: 'delete' }; - return { kind: 'write', content: `${JSON.stringify(result, null, 2)}\n` }; -} - /** * Write an agent's Switch credentials to its provider-neutral per-agent file - * `.switch/agents/.json` (CHOO-1440), alongside the connector-owned - * `.claude/settings.local.json`. switchdash injects this file's env at launch, so - * it is the authoritative per-agent identity — letting multiple agents share a - * location without colliding on the single `settings.local.json` identity. + * `.switch/agents/.json` (CHOO-1440). switchdash injects this file's env + * at launch, so it is the authoritative per-agent identity — letting any number + * of agents share a working directory without colliding. * * `apiToken` is the agent's secret API key — written here and never returned to * the renderer or logged. A `.gitignore` keeps the directory out of version diff --git a/dash/apps/switchdash-desktop/src/main/core/locations/location-runtime-factory.ts b/dash/apps/switchdash-desktop/src/main/core/locations/location-runtime-factory.ts index a54bfcf90..7c892e177 100644 --- a/dash/apps/switchdash-desktop/src/main/core/locations/location-runtime-factory.ts +++ b/dash/apps/switchdash-desktop/src/main/core/locations/location-runtime-factory.ts @@ -220,7 +220,7 @@ type AgentRuntimeOpts = { sessionEnvVars: Record; /** Candidate creds files (relative to the working dir) the remote preflight * checks, in priority order — the agent's neutral `.switch/agents/.json` - * first, then the legacy `.claude/settings.local.json` (CHOO-1440). */ + * first, then the earlier id-keyed variant of it (CHOO-1440). */ credsRelPaths: string[]; }; diff --git a/dash/apps/switchdash-desktop/src/main/core/sessions/remote-session-preflight.ts b/dash/apps/switchdash-desktop/src/main/core/sessions/remote-session-preflight.ts index 4ef4d20e6..4641e7e6b 100644 --- a/dash/apps/switchdash-desktop/src/main/core/sessions/remote-session-preflight.ts +++ b/dash/apps/switchdash-desktop/src/main/core/sessions/remote-session-preflight.ts @@ -20,8 +20,8 @@ import { parseSwitchAgentCredentials } from '@main/core/switch-rooms/switch-cred * only stabilised in Node 18; * 2. the agent's Switch creds exist on the remote host — checked at the agent's * provider-neutral per-agent path (`.switch/agents/.json`) first, then - * the legacy `.claude/settings.local.json` for un-migrated installs - * (CHOO-1440) — read in parallel with (1); + * the earlier id-keyed variant of it for un-migrated installs (CHOO-1440) — + * read in parallel with (1); * 3. the host can actually reach the Switch API endpoint — the sidecar polls * from the VM, so no egress means a dead agent. * @@ -66,7 +66,7 @@ export interface RemotePreflightDeps { workDir: string; /** Candidate creds files (relative to the working dir) to check, in priority * order — the agent's neutral `.switch/agents/.json` first, then the - * legacy `.claude/settings.local.json`. The first that parses is used. */ + * earlier id-keyed variant of it. The first that parses is used. */ credsRelPaths: string[]; } @@ -153,8 +153,8 @@ async function readRemoteEndpoint(deps: RemotePreflightDeps): Promise { const primary = deps.credsRelPaths[0] ?? '.switch/agents'; // Track the FIRST file that was present but unparseable/incomplete, so the // error names the actual offending file rather than always the primary path — - // a stale fallback (an old id-keyed `.switch/agents/.json` or the legacy - // `.claude/settings.local.json`) is a common cause and must be pinpointed. + // a stale fallback (an old id-keyed `.switch/agents/.json`) is a common + // cause and must be pinpointed. let incompletePath: string | null = null; for (const relPath of deps.credsRelPaths) { let content: string; diff --git a/dash/apps/switchdash-desktop/src/main/core/sessions/session-builder.ts b/dash/apps/switchdash-desktop/src/main/core/sessions/session-builder.ts index f22016c71..4e78b78fa 100644 --- a/dash/apps/switchdash-desktop/src/main/core/sessions/session-builder.ts +++ b/dash/apps/switchdash-desktop/src/main/core/sessions/session-builder.ts @@ -1,8 +1,5 @@ import { agentCredsSlug } from '@main/core/agents/agent-creds-slug'; -import { - agentSettingsRelativePath, - SWITCH_SETTINGS_RELATIVE_PATH, -} from '@main/core/agents/switch-settings-paths'; +import { agentSettingsRelativePath } from '@main/core/agents/switch-settings-paths'; import type { LocationProvider } from '@main/core/locations/location-provider'; import type { LocationRuntime } from '@main/core/locations/location-runtime'; import { locationRuntimeRegistry } from '@main/core/locations/location-runtime-registry'; @@ -110,15 +107,13 @@ export async function buildSessionFromRuntime( ); // The remote preflight verifies the session's own creds file, keyed by the - // agent's NAME (`.switch/agents/.json`). The agent-id path and the legacy - // shared `.claude/settings.local.json` are last-resort fallbacks for agents not - // yet migrated (CHOO-1440). + // agent's NAME (`.switch/agents/.json`); the agent-id path is a + // last-resort fallback for agents not yet migrated (CHOO-1440). const credsRelPaths = [ ...new Set([ agentSettingsRelativePath(agentCredsSlug(session)), agentSettingsRelativePath(session.agentId), ]), - SWITCH_SETTINGS_RELATIVE_PATH, ]; return buildAgentRuntime(transport, { diff --git a/dash/apps/switchdash-desktop/src/main/core/switch-rooms/auto-session-watcher.ts b/dash/apps/switchdash-desktop/src/main/core/switch-rooms/auto-session-watcher.ts index eae49f2fe..27cca76a2 100644 --- a/dash/apps/switchdash-desktop/src/main/core/switch-rooms/auto-session-watcher.ts +++ b/dash/apps/switchdash-desktop/src/main/core/switch-rooms/auto-session-watcher.ts @@ -17,7 +17,6 @@ import { setAutoSessionSubagent, } from './auto-session-store'; import { - readSwitchAgentCredentials, readSwitchAgentCredentialsFromSettings, type SwitchAgentCredentials, } from './switch-credentials'; @@ -199,13 +198,13 @@ class AutoSessionWatcher { }); return; } - // Read the agent's own identity from its provider-neutral per-agent file so - // agents sharing a location watch as themselves; fall back to the location's - // `.claude/settings.local.json` for un-migrated installs (CHOO-1440). + // Read the agent's own identity from its provider-neutral per-agent file, so + // agents sharing a location watch as themselves. const slug = agent?.name ?? localAgentId; - const creds = - (await readSwitchAgentCredentialsFromSettings(agentSettingsPath(rootPath, slug), log)) ?? - (await readSwitchAgentCredentials(rootPath, log)); + const creds = await readSwitchAgentCredentialsFromSettings( + agentSettingsPath(rootPath, slug), + log + ); if (!creds) { log.warn('AutoSessionWatcher: missing Switch credentials; cannot watch', { localAgentId, diff --git a/dash/apps/switchdash-desktop/src/main/core/switch-rooms/switch-credentials.ts b/dash/apps/switchdash-desktop/src/main/core/switch-rooms/switch-credentials.ts index 91af7d29e..413285a7f 100644 --- a/dash/apps/switchdash-desktop/src/main/core/switch-rooms/switch-credentials.ts +++ b/dash/apps/switchdash-desktop/src/main/core/switch-rooms/switch-credentials.ts @@ -1,10 +1,6 @@ import { promises as fs } from 'node:fs'; -import path from 'node:path'; import type { PluginFs } from '@switchdash/core/agents/plugins'; -import { - agentSettingsRelativePath, - SWITCH_SETTINGS_RELATIVE_PATH, -} from '@main/core/agents/switch-settings-paths'; +import { agentSettingsRelativePath } from '@main/core/agents/switch-settings-paths'; export interface SwitchAgentCredentials { agentId: string; @@ -34,26 +30,9 @@ function asNonEmptyString(value: unknown): string | null { } /** - * Read the full Switch agent credentials (including the API token) from an - * agent directory's `.claude/settings.local.json` env block. - * - * Unlike {@link detectSwitchAgent}, this reads `SWITCH_API_TOKEN` — it is used - * only by the notification poller, which must authenticate to the agent bridge - * on the agent's behalf. Returns null when the file is missing/unparseable or - * any of the three values is absent. - */ -export async function readSwitchAgentCredentials( - dir: string, - log: CredentialsLogger -): Promise { - return readSwitchAgentCredentialsFromSettings(path.join(dir, SWITCH_SETTINGS_RELATIVE_PATH), log); -} - -/** - * Read Switch agent credentials from a specific settings file. Used to poll as a - * subagent (its `.claude/switch-subagents/.settings.json`) rather than the - * parent's `.claude/settings.local.json`, so the session receives the events - * addressed to the subagent — not the parent. + * Read Switch agent credentials from a specific credentials file, so a session + * polls as the agent that owns that file — one of possibly several sharing a + * working directory — and receives the events addressed to it. */ export async function readSwitchAgentCredentialsFromSettings( settingsPath: string, diff --git a/dash/apps/switchdash-desktop/src/main/core/switch-rooms/switch-notification-poller.ts b/dash/apps/switchdash-desktop/src/main/core/switch-rooms/switch-notification-poller.ts index d987a0c71..9ee152d4f 100644 --- a/dash/apps/switchdash-desktop/src/main/core/switch-rooms/switch-notification-poller.ts +++ b/dash/apps/switchdash-desktop/src/main/core/switch-rooms/switch-notification-poller.ts @@ -15,10 +15,7 @@ import { PluginPromptInjector } from './plugin-prompt-injector'; import { RoomConnection } from './room-connection'; import { sessionConnectionId } from './session-connection-id'; import { resolveSessionControl } from './session-control'; -import { - readSwitchAgentCredentials, - readSwitchAgentCredentialsFromSettings, -} from './switch-credentials'; +import { readSwitchAgentCredentialsFromSettings } from './switch-credentials'; import { switchRoomService, type SessionRoomContext } from './switch-room-service'; /** @@ -198,12 +195,10 @@ class SwitchNotificationPoller { // with the agent row. const slug = loaded.name; - // Fall back to the legacy subagent path, then the location's - // `.claude/settings.local.json`, for un-migrated installs (CHOO-1440). + // Fall back to the legacy subagent path for un-migrated installs (CHOO-1440). const creds = (await readSwitchAgentCredentialsFromSettings(agentSettingsPath(rootPath, slug), log)) ?? - (await readSwitchAgentCredentialsFromSettings(subagentSettingsPath(rootPath, slug), log)) ?? - (await readSwitchAgentCredentials(rootPath, log)); + (await readSwitchAgentCredentialsFromSettings(subagentSettingsPath(rootPath, slug), log)); if (!creds) { log.warn( 'SwitchNotificationPoller: missing Switch credentials (SWITCH_API_TOKEN/ENDPOINT/AGENT_ID) — cannot poll room', diff --git a/dash/apps/switchdash-desktop/src/main/db/schema.ts b/dash/apps/switchdash-desktop/src/main/db/schema.ts index 2c65f330b..4a36ae285 100644 --- a/dash/apps/switchdash-desktop/src/main/db/schema.ts +++ b/dash/apps/switchdash-desktop/src/main/db/schema.ts @@ -130,8 +130,8 @@ export const switchServers = sqliteTable( * A Switch agent: an agent identity bound to a single provider, living at a * location. Many agents may share a location (e.g. a Claude Code and a Codex * agent in the same repo). `switchAgentId` / `apiEndpoint` are populated when - * the location dir is configured as a Switch agent (detected from - * `.claude/settings.local.json`); they are null for a plain local agent. + * the agent has a Switch identity on disk (`.switch/agents/.json`); + * they are null for a plain local agent. * * `serverId` binds the agent to the one registered Switch server it belongs to. * It is resolved by matching the detected `apiEndpoint` against the registered diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx index c71556eab..3f43bcf3b 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx @@ -56,10 +56,10 @@ import { } from './onboard-existing-panel'; import { RemoteDirNotice } from './remote-dir-notice'; -// switchdash adds a Switch *agent* by pointing at a local directory that the -// switch-connector `configure` skill has set up (its `.claude/settings.local.json` -// carries the SWITCH_* env block). The richer switchdash flows — SSH, clone, create -// new GitHub repo — are out of scope for v0, so this modal is local + pick only. +// switchdash adds a Switch *agent* by pointing at a working directory, local or +// on an SSH host: it mints the identity and writes it to +// `.switch/agents/.json`, or adopts agents already configured there. The +// richer flows — clone, create new GitHub repo — remain out of scope. export type AddLocationModalProps = BaseModalProps; /** Sentinel `runHost` value meaning "run on this machine" (no remote host). */ diff --git a/dash/apps/switchdash-desktop/src/shared/core/agents/agents.ts b/dash/apps/switchdash-desktop/src/shared/core/agents/agents.ts index d0af134d2..94671af98 100644 --- a/dash/apps/switchdash-desktop/src/shared/core/agents/agents.ts +++ b/dash/apps/switchdash-desktop/src/shared/core/agents/agents.ts @@ -5,7 +5,7 @@ import type { AgentProviderId } from '@shared/core/providers/agent-provider-regi * A Switch agent: an agent identity bound to a single provider, living at a * location (a working directory on this machine or an SSH host). Many agents * may share a location. `switchAgentId` / `apiEndpoint` carry the Switch - * identity detected from the location dir's `.claude/settings.local.json`. + * identity from the agent's own `.switch/agents/.json`. * `serverId` is the registered Switch server the agent belongs to (resolved * from `apiEndpoint`); null means unlinked — the server it points at is not * registered in this app. diff --git a/dash/apps/switchdash-desktop/src/sidecar/index.ts b/dash/apps/switchdash-desktop/src/sidecar/index.ts index 78e4560a8..20e02bfe0 100644 --- a/dash/apps/switchdash-desktop/src/sidecar/index.ts +++ b/dash/apps/switchdash-desktop/src/sidecar/index.ts @@ -6,10 +6,7 @@ import { fileURLToPath } from 'node:url'; import { promisify } from 'node:util'; import { HookEventLog, HookServer } from '@main/core/agent-hooks/hook-server'; import { agentSettingsPath } from '@main/core/agents/switch-settings-paths'; -import { - readSwitchAgentCredentials, - readSwitchAgentCredentialsFromSettings, -} from '@main/core/switch-rooms/switch-credentials'; +import { readSwitchAgentCredentialsFromSettings } from '@main/core/switch-rooms/switch-credentials'; import { createTmuxRun } from '@main/core/switch-rooms/tmux-injection-sink'; import { type AgentLaunchSpec } from './agent-launch-spec'; import { atomicWriteFile } from './atomic-file'; @@ -47,10 +44,9 @@ import { exactTmuxTarget, parseAgentTmuxSessionName } from './vm-tmux'; * * Pure Node (no Electron, no database). The agent's Switch credentials come from * its provider-neutral per-agent file `.switch/agents/.json` (the slug is - * passed in `SWITCHDASH_SIDECAR_AGENT_SLUG`), falling back to the legacy shared - * `.claude/settings.local.json` for un-migrated installs (CHOO-1440); the - * provider-specific launch recipe for auto-started sessions comes from the launch - * spec switchdash writes to the VM. + * passed in `SWITCHDASH_SIDECAR_AGENT_SLUG`); the provider-specific launch recipe + * for auto-started sessions comes from the launch spec switchdash writes to the + * VM. * On startup it prints one JSON line to stdout — `{event:"ready",port,token}` — * so the launcher can point switchdash's remote sessions at this hook server. */ @@ -100,18 +96,13 @@ async function main(): Promise { const repoDir = requireEnv('SWITCHDASH_SIDECAR_REPO_DIR'); const deeplinkScheme = process.env.SWITCHDASH_SIDECAR_DEEPLINK_SCHEME?.trim() || 'switchdash'; - // Prefer the agent's provider-neutral per-agent creds file; fall back to the - // legacy shared settings.local.json for un-migrated installs (CHOO-1440). - const credsSlug = process.env.SWITCHDASH_SIDECAR_AGENT_SLUG?.trim(); - const creds = - (credsSlug - ? await readSwitchAgentCredentialsFromSettings(agentSettingsPath(repoDir, credsSlug), log) - : null) ?? (await readSwitchAgentCredentials(repoDir, log)); + // The agent's provider-neutral per-agent creds file, named by the slug the + // launcher passes (CHOO-1440). + const credsSlug = requireEnv('SWITCHDASH_SIDECAR_AGENT_SLUG'); + const credsPath = agentSettingsPath(repoDir, credsSlug); + const creds = await readSwitchAgentCredentialsFromSettings(credsPath, log); if (!creds) { - const where = credsSlug - ? agentSettingsPath(repoDir, credsSlug) - : `${repoDir}/.claude/settings.local.json`; - throw new Error(`sidecar: no Switch credentials at ${where} — run remote setup first`); + throw new Error(`sidecar: no Switch credentials at ${credsPath} — run remote setup first`); } // Per-agent state paths, so multiple agents in one repo dir each drive their diff --git a/dash/apps/switchdash-desktop/src/sidecar/sidecar-runtime.ts b/dash/apps/switchdash-desktop/src/sidecar/sidecar-runtime.ts index 3c441e604..3075f2fd3 100644 --- a/dash/apps/switchdash-desktop/src/sidecar/sidecar-runtime.ts +++ b/dash/apps/switchdash-desktop/src/sidecar/sidecar-runtime.ts @@ -90,7 +90,7 @@ interface SessionConnection { * per session. * * Runs entirely on the VM with no database or Electron — the agent's Switch - * credentials come from its `.claude/settings.local.json`. + * credentials come from its `.switch/agents/.json`. */ export class SidecarRuntime { /** sessionId → its live room connection. */ From f2044ae798f962873559c86f83242253a1963ef9 Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Thu, 6 Aug 2026 19:07:00 +0000 Subject: [PATCH 14/15] fix(switchdash): switching agent type no longer re-checks the location (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things in the add-agent modal are keyed on the agent type, and both retracted everything below the working directory while they re-ran — so changing the type looked like the location being re-examined, though the location had not changed. Host readiness is probed per host *and* type, and `checking` was treated as a reason to hide the form. It is not a verdict, only the absence of one: showing now waits for a bad verdict, while submitting still waits for a verdict either way. Same split the directory check already uses. The definition scan is keyed on the agent type too, so switching type starts a new one, and its pending state blocked the create flow and put a "Scanning directory for agents…" line under the directory field. It now blocks only the first scan of a given directory, which is the case it was added for — a create form that flashes up before discovery lands and then flips to the onboard list. A re-scan refreshes in place. Co-Authored-By: Claude Opus 5 (1M context) --- .../add-agent-modal/add-agent-modal.tsx | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx index 3f43bcf3b..751677e54 100644 --- a/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx +++ b/dash/apps/switchdash-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx @@ -73,6 +73,18 @@ type AddAgentFailure = Exclude< { kind: 'created' } >; +/** + * Whether `key` has been seen in a settled (non-pending) state at least once. + * + * Lets a gate distinguish "we know nothing yet" from "we are refreshing what we + * already know", so a re-query does not retract what is already on screen. + */ +function useSettledOnce(key: string, pending: boolean): boolean { + const settled = useRef(new Set()); + if (!pending) settled.current.add(key); + return settled.current.has(key); +} + /** Canonical working-directory path: trimmed, with trailing slashes removed * (except a bare root), so `/repo` and `/repo/` behave identically through * detection, discovery, and location keying — the flow must not care (CHOO-1440). */ @@ -323,14 +335,24 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc }); }, []); - // Discovery (`.claude/agents` scan) is a separate query from agent-detection; - // fold its pending state into `isChecking` so the modal decides "onboard - // existing vs create new" only once BOTH have settled — otherwise the create - // form flashes up first and then flips to the onboard list when discovery lands - // (CHOO-1440). - const isDiscovering = + // Discovery (`.claude/agents` scan) is a separate query; fold its pending + // state into `isChecking` so the modal decides "onboard existing vs create + // new" only once both have settled — otherwise the create form flashes up + // first and then flips to the onboard list when discovery lands (CHOO-1440). + // + // Only while a directory is being scanned for the FIRST time, though. The + // definition scan is keyed on the agent type, so switching type starts a new + // one, and blocking on that retracted everything below the working directory + // and put a "Scanning directory…" line under it — which reads as the location + // being re-checked, when the location has not changed at all. + const discoveryPending = (!!pickState.providerId && discoverDir.trim().length > 0 && discoverQuery.isPending) || (discoverDir.trim().length > 0 && configuredQuery.isPending); + const scannedOnce = useSettledOnce( + `${discoverSshHost ?? 'local'}:${discoverDir}`, + discoveryPending + ); + const isDiscovering = discoveryPending && !scannedOnce; const isChecking = (isRemoteRun ? false : shouldCheckPathStatus && pathStatusQuery.isPending) || isDiscovering; // Never create an agent on a host we know we cannot reach — it would be born @@ -362,7 +384,12 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc isRemoteRun ? runHost : null, pickState.providerId ?? null ); + // Submitting waits for a verdict; showing the form only waits for a bad one. + // Readiness is probed per agent type, so switching type starts a new probe — + // and treating "checking" as a reason to hide made the whole form vanish and + // come back on every type change. const runHostReady = !isRemoteRun || (!hostReadiness.blocked && !hostReadiness.checking); + const runHostNotBlocked = !isRemoteRun || !hostReadiness.blocked; // Where the block stops the flow. A host missing its own prerequisites cannot // run anything, so nothing below the location picker is worth filling in — @@ -377,7 +404,7 @@ export const AddAgentModal = observer(function AddAgentModal({ onClose }: AddLoc // Without both, those fields ask the user to describe something that cannot // be created, directly under a notice saying so (CHOO-1416). const canConfigureAgent = - canChooseAgentType && runHostReady && !!pickState.providerId && !remoteDirBlocked; + canChooseAgentType && runHostNotBlocked && !!pickState.providerId && !remoteDirBlocked; // The working directory is not a function of the agent type. Switching type // re-probes host readiness for the new type, and withdrawing the field // mid-probe made it look as though the location itself were being rechecked. From 45ddef3f723951492de069f8325e83f022573ccd Mon Sep 17 00:00:00 2001 From: Louis Amaudruz Date: Fri, 7 Aug 2026 17:46:42 +0000 Subject: [PATCH 15/15] Merge origin/main into remote-agent-missing-parent-dir (CHOO-1416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 52 commits, no conflicts. Main's sidebar work (CHOO-2007) restores drag-to-*reorder* within the sidebar, which is a different mechanism from the folder-drop onboarding this branch removed, so the two do not overlap. Also drops a stale reference to `SWITCH_SETTINGS_RELATIVE_PATH` from a migration test's doc comment — the constant went with the legacy credential source earlier on this branch. --- .../src/main/core/agents/migrate-agent-storage.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.test.ts b/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.test.ts index e75ad2073..f20c4eb69 100644 --- a/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.test.ts +++ b/dash/apps/switchdash-desktop/src/main/core/agents/migrate-agent-storage.test.ts @@ -3,8 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; /** * An in-memory {@link PluginFs} keyed by the exact relative paths the migration - * uses, so path helpers (`agentSettingsRelativePath`, `SWITCH_SETTINGS_RELATIVE_PATH`) - * resolve against real content. + * uses, so `agentSettingsRelativePath` resolves against real content. */ function fakeFs(seed: Record): PluginFs { const files = new Map(Object.entries(seed));