From 5452ad5c5ae314bec730a21688f3b74a443cdef4 Mon Sep 17 00:00:00 2001 From: d fei Date: Sat, 29 Aug 2026 23:31:28 -0700 Subject: [PATCH] feat(docker): add a folder picker to both path fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both paths in the adoption form had to be typed. Each gets a Browse button using the same path-input-group markup Link Existing uses, so the two look and behave alike. What they can browse differs, and that is the point. The host workspace path reuses the existing host picker. The container workdir cannot: an adopted container has nothing mounted at a matching host path, so a host listing would be a different filesystem — and getting this field wrong is the source of the opaque OCI chdir error at launch, which makes it the field that most needs to be clickable. Adds a read-only POST /api/docker-cases/browse: one `ls` through docker exec, no writes, no lifecycle, path shell-escaped like every other value. `ls -Ap` marks directories with a trailing slash and keeps names with spaces intact. PathPicker takes an optional fetchListing source rather than being forked: the container variant only swaps where the rows come from, and reuses the rendering, navigation, Up and Choose/Select unchanged. --- src/docker-hosts.ts | 54 ++++++++++++++++++++++++++ src/web/public/index.html | 10 ++++- src/web/public/keyboard-accessory.js | 10 +++-- src/web/public/session-ui.js | 56 +++++++++++++++++++++++++++ src/web/routes/case-routes.ts | 26 ++++++++++++- src/web/schemas.ts | 16 ++++++++ test/docker-adopted-container.test.ts | 34 ++++++++++++++++ 7 files changed, 200 insertions(+), 6 deletions(-) diff --git a/src/docker-hosts.ts b/src/docker-hosts.ts index a6ce66ad..e2801b55 100644 --- a/src/docker-hosts.ts +++ b/src/docker-hosts.ts @@ -1239,6 +1239,60 @@ export async function probeAdoptableContainer( } } +/** One directory listing from INSIDE a container, shaped like the host picker's. */ +export interface DockerBrowseResult { + path: string; + parent: string | null; + entries: Array<{ name: string; path: string; type: 'directory' | 'file' }>; + error?: string; +} + +/** + * List a directory INSIDE a container, for the adoption form's container-workdir + * picker. The host filesystem picker cannot serve this: the path lives in the + * container, and for an adopted container nothing is mounted at a matching host + * location, so the user would otherwise be typing a path blind. + * + * Read-only: one `ls` through `docker exec`, no writes, no lifecycle. The path + * is shell-escaped like every other value this module interpolates, and output + * is parsed as NUL-free lines with a leading type marker so a filename with + * spaces survives. + */ +export async function browseInContainer( + docker: Pick, + path: string +): Promise { + const target = path && path.startsWith('/') ? path : '/'; + const parent = target === '/' ? null : target.replace(/\/+$/, '').split('/').slice(0, -1).join('/') || '/'; + if (IS_TEST_MODE) return { path: target, parent, entries: [] }; + const argv = dockerEngineArgv(docker); + // `-p` marks directories with a trailing slash; `-A` shows dotfiles but not + // the . and .. entries the picker navigates with its own Up control. + const script = `cd ${shellescape(target)} 2>/dev/null && ls -Ap 2>/dev/null || echo __ERR__`; + try { + const { stdout } = await execFileAsync( + argv[0], + [...argv.slice(1), 'exec', docker.containerName, 'sh', '-lc', script], + { timeout: DOCKER_PROBE_TIMEOUT_MS, maxBuffer: 4 * 1024 * 1024 } + ); + if (stdout.includes('__ERR__')) return { path: target, parent, entries: [], error: 'Not a readable directory' }; + const base = target.endsWith('/') ? target : `${target}/`; + const entries = stdout + .split('\n') + .map((line) => line.trim()) + .filter(Boolean) + .map((name) => { + const isDir = name.endsWith('/'); + const clean = isDir ? name.slice(0, -1) : name; + return { name: clean, path: `${base}${clean}`, type: (isDir ? 'directory' : 'file') as 'directory' | 'file' }; + }) + .sort((a, b) => Number(b.type === 'directory') - Number(a.type === 'directory') || a.name.localeCompare(b.name)); + return { path: target, parent, entries }; + } catch (err) { + return { path: target, parent, entries: [], error: err instanceof Error ? err.message : String(err) }; + } +} + /** * Resolve the host's IP on the default docker bridge (the address a container * reaches as `host.docker.internal`), so the server can bind a hooks-only listener diff --git a/src/web/public/index.html b/src/web/public/index.html index f83a17d9..6df870b3 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -2844,7 +2844,10 @@
- +
+ + +
A path that already exists inside the container. Adoption mounts nothing, so this need not match the host workspace path.
@@ -2854,7 +2857,10 @@
- +
+ + +
Absolute HOST directory, bind-mounted into the container. Codeman scaffolds CLAUDE.md + hooks into it.
diff --git a/src/web/public/keyboard-accessory.js b/src/web/public/keyboard-accessory.js index 35b67c3e..7ec9a890 100644 --- a/src/web/public/keyboard-accessory.js +++ b/src/web/public/keyboard-accessory.js @@ -185,9 +185,13 @@ const PathPicker = { if (this._options.sessionId) params.set('sessionId', this._options.sessionId); if (this._showHidden) params.set('showHidden', 'true'); try { - const response = await fetch(`/api/filesystem/browse?${params.toString()}`); - const result = await response.json(); - if (!response.ok || !result.success) throw new Error(result.error || 'Failed to browse this folder'); + // A caller may supply its own source (the container-workdir picker browses + // INSIDE a container, which the host filesystem endpoint cannot answer). + // It returns the same shape, so everything below is unchanged. + const result = this._options.fetchListing + ? await this._options.fetchListing(path) + : await (await fetch(`/api/filesystem/browse?${params.toString()}`)).json(); + if (!result?.success) throw new Error(result?.error || 'Failed to browse this folder'); if (!this.overlay || loadSequence !== this._loadSequence) return; this.render(result.data); } catch (error) { diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index dc4503bd..15109b16 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -2895,6 +2895,62 @@ Object.assign(CodemanApp.prototype, { }); }, + /** HOST workspace directory — the same picker Link Existing uses. */ + openDockerWorkspacePathPicker() { + const pathInput = document.getElementById('dockerWorkspacePath'); + PathPicker.open({ + title: 'Select Host Workspace Folder', + initialPath: pathInput.value.trim(), + directoriesOnly: true, + onSelect: (path) => { + pathInput.value = path; + const nameInput = document.getElementById('dockerCaseName'); + if (nameInput && !nameInput.value.trim()) { + const folder = path.split('/').filter(Boolean).pop() || ''; + if (/^[a-zA-Z0-9_-]+$/.test(folder)) nameInput.value = folder; + } + }, + }); + }, + + /** + * Container workdir. Browses INSIDE the container, because for an adopted + * container nothing is mounted at a matching host path — the host picker would + * be listing a different filesystem, and typing this field blind is exactly + * what makes the launch fail with an OCI chdir error. + */ + openDockerWorkdirPicker() { + const pathInput = document.getElementById('dockerAdoptWorkdir'); + const container = document.getElementById('dockerContainerName')?.value.trim(); + const hostId = document.getElementById('dockerHostId')?.value.trim() || 'local'; + if (!container) { + this.showToast('Enter the container name first', 'error'); + return; + } + PathPicker.open({ + title: `Select Folder Inside ${container}`, + initialPath: pathInput.value.trim() || '/', + directoriesOnly: true, + fetchListing: async (path) => { + const data = await this._apiJson('/api/docker-cases/browse', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ hostId, container, path: path || '/' }), + }); + if (!data) return { success: false, error: `Could not read ${container}. Is it running?` }; + if (data.error) return { success: false, error: data.error }; + // Shape it like the host endpoint: one root, so Up/Location behave. + return { + success: true, + data: { ...data, root: '/', roots: [{ label: container, path: '/' }], truncated: false }, + }; + }, + onSelect: (path) => { + pathInput.value = path; + }, + }); + }, + async linkRemoteCase() { const name = document.getElementById('remoteCaseName').value.trim(); const remotePath = document.getElementById('remoteCasePath').value.trim(); diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 11ac46e7..5aa4a2f0 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -26,6 +26,7 @@ import { DockerCaseLinkSchema, DockerCaseAdoptSchema, DockerAdoptPreflightSchema, + DockerBrowseSchema, DockerHostSchema, DockerExportSchema, DockerImportSchema, @@ -70,6 +71,7 @@ import { dockerDisplayPath, probeAdoptableContainer, listDockerContainers, + browseInContainer, DOCKER_ADOPT_PROBE_MODES, readDockerCases, readDockerHosts, @@ -78,7 +80,7 @@ import { writeDockerCases, writeDockerHosts, } from '../../docker-hosts.js'; -import type { AdoptedContainerProbe, DockerContainerInfo } from '../../docker-hosts.js'; +import type { AdoptedContainerProbe, DockerBrowseResult, DockerContainerInfo } from '../../docker-hosts.js'; import { buildDockerRemoveCommand } from '../../tmux-manager.js'; import { checkRemoteTmuxAvailable, @@ -895,6 +897,28 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } ); + /** + * Browse a directory INSIDE a container, for the adoption form's + * container-workdir picker. The host picker cannot answer this: for an adopted + * container nothing is mounted at a matching host path, so the field would + * otherwise be typed blind. Read-only — one `ls` through `docker exec`. + */ + app.post('/api/docker-cases/browse', async (req): Promise> => { + const body = parseBody(DockerBrowseSchema, req.body); + const host = (await readDockerHosts(CODEMAN_CONFIG_DIR)).find((item) => item.id === body.hostId); + if (!host) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Docker host not found'); + const result = await browseInContainer( + { + engine: host.engine ?? 'docker', + context: host.context, + daemonHost: host.daemonHost, + containerName: body.container, + }, + body.path || '/' + ); + return { success: true, data: result }; + }); + app.post('/api/docker-cases/adopt-preflight', async (req): Promise> => { const body = parseBody(DockerAdoptPreflightSchema, req.body); const host = (await readDockerHosts(CODEMAN_CONFIG_DIR)).find((item) => item.id === body.hostId); diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 65ef1395..ffffd388 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -844,6 +844,22 @@ export const DockerAdoptPreflightSchema = z.object({ .optional(), }); +/** Read-only directory listing inside a container (adoption workdir picker). */ +export const DockerBrowseSchema = z.object({ + hostId: z.string().regex(/^[a-zA-Z0-9_-]+$/, 'Invalid docker host id'), + container: z + .string() + .min(2) + .max(128) + .regex(/^[a-zA-Z0-9][a-zA-Z0-9_.-]+$/, 'Invalid container name'), + path: z + .string() + .max(2000) + .regex(/^\//, 'Path must be absolute') + .regex(NO_SHELL_META, 'Invalid characters in path') + .optional(), +}); + export const DockerExportSchema = z.object({ mode: z.enum(['full', 'workspace']).optional(), }); diff --git a/test/docker-adopted-container.test.ts b/test/docker-adopted-container.test.ts index 2d9c6c1b..36f86780 100644 --- a/test/docker-adopted-container.test.ts +++ b/test/docker-adopted-container.test.ts @@ -272,6 +272,40 @@ describe('adopted container: run modes come from the CONTAINER, not the host', ( }); }); +describe('adopted container: both path fields get a folder picker', () => { + const html = readFileSync(new URL('../src/web/public/index.html', import.meta.url), 'utf8'); + const ui = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf8'); + const picker = readFileSync(new URL('../src/web/public/keyboard-accessory.js', import.meta.url), 'utf8'); + + it('wires a Browse button to each of the two paths', () => { + expect(html).toContain('app.openDockerWorkspacePathPicker()'); + expect(html).toContain('app.openDockerWorkdirPicker()'); + // Same markup Link Existing uses, so the two look and behave alike. + expect(html.match(/path-input-browse/g)?.length).toBeGreaterThanOrEqual(3); + }); + + it('browses the CONTAINER for the container workdir, not the host', () => { + // For an adopted container nothing is mounted at a matching host path, so a + // host listing would be a different filesystem — and typing this field blind + // is what makes the launch fail with an OCI chdir error. + const fn = ui.slice(ui.indexOf('openDockerWorkdirPicker()'), ui.indexOf('async linkRemoteCase()')); + expect(fn).toContain('/api/docker-cases/browse'); + expect(fn).not.toContain('/api/filesystem/browse'); + expect(fn).toContain('fetchListing'); + }); + + it('keeps the host picker for the host workspace path', () => { + const fn = ui.slice(ui.indexOf('openDockerWorkspacePathPicker()'), ui.indexOf('openDockerWorkdirPicker()')); + expect(fn).toContain('PathPicker.open'); + expect(fn).not.toContain('fetchListing'); + }); + + it('reuses one PathPicker via an optional source rather than forking it', () => { + expect(picker).toContain('this._options.fetchListing'); + expect(picker).toContain('/api/filesystem/browse'); + }); +}); + describe('adopted container: drift is not evaluated', () => { it('reports no drift rather than demanding a recreate we may not perform', async () => { // An adopted container carries no codeman.confighash label, so a real