From f1ed3a58e1536c70ede344488d5464c63a77926a Mon Sep 17 00:00:00 2001 From: d fei Date: Tue, 1 Sep 2026 20:45:13 -0700 Subject: [PATCH] feat(docker): add "copy an existing case" to the adopt panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backend already lets one adopted container back several cases pointing at different in-container directories, but using it meant retyping the container name, host and workspace one by one — exactly the friction that leaves a capability unused. Picking an existing case from a dropdown now carries those three over, leaving only the two fields that must differ: the case name and the in-container directory. Clearing those two is the point of the feature, not a convenience: keeping the old name is refused by the server as "case already exists", and keeping the old directory is refused as "a twin case on the same container and directory". Both errors are clear, but a form pre-filled with values that are guaranteed to be rejected is a trap. Focus lands on the in-container directory — the thing the user came here to change. ⚠️ Only adopted containers are listed (docker.owned === false). A Codeman-built container's lifecycle belongs to its one case — a second case would be torn out by that case's recreate or delete — so the server refuses it anyway, and listing it here would only manufacture a baffling error. `owned` may be absent and absent means owned, so the test is `!== false`, not truthiness. CaseInfo.docker gains containerWorkdir and owned for this: the former is the "which directory does this case use" half of the picker, without which the user cannot tell what to change it to; the latter backs the filter above. ⚠️ Both places that build a docker CaseInfo (the list endpoint and the single-case query) must set them — filling in only one makes the picker work or not depending on which read path was taken, and a test pins "exactly two". --- src/types/api.ts | 9 +++ src/web/public/index.html | 7 +++ src/web/public/session-ui.js | 78 ++++++++++++++++++++++- src/web/routes/case-routes.ts | 4 ++ test/docker-adopt-duplicate.test.ts | 96 +++++++++++++++++++++++++++++ 5 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 test/docker-adopt-duplicate.test.ts diff --git a/src/types/api.ts b/src/types/api.ts index a57b2795..779724af 100644 --- a/src/types/api.ts +++ b/src/types/api.ts @@ -166,6 +166,15 @@ export interface CaseInfo { container: string; image?: string; path: string; + /** Directory INSIDE the container (defaults to `path` when unset). */ + containerWorkdir?: string; + /** + * False = an ADOPTED container the user built and runs. Only those may back + * several cases at once (classifyAdoptContainerConflict), so this is what lets + * the UI offer "duplicate for another directory" on exactly the right cases — + * an owned container's lifecycle belongs to its one case. + */ + owned?: boolean; network?: string; /** * CLIs available INSIDE the container. A container case runs its agents in diff --git a/src/web/public/index.html b/src/web/public/index.html index f13f538c..c8cdbdfe 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -2863,6 +2863,13 @@ On: Codeman only runs docker exec into a container you already built and run — it never creates, starts, stops or removes it. The CLIs must already be installed and logged in inside it. +
diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 973cb9c0..bd84fbb2 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -3139,7 +3139,10 @@ Object.assign(CodemanApp.prototype, { const adopting = document.getElementById('dockerAdoptExisting')?.checked; if (adopting) modal.setAttribute('data-docker-adopt', '1'); else modal.removeAttribute('data-docker-adopt'); - if (adopting) void this._loadDockerContainerOptions(); + if (adopting) { + void this._loadDockerContainerOptions(); + void this._loadDockerCloneOptions(); + } }, /** @@ -3151,6 +3154,79 @@ Object.assign(CodemanApp.prototype, { * Best-effort by design — the endpoint returns [] for an unreachable daemon, * and an empty list simply leaves the field as plain text input. */ + /** + * Fill the "Duplicate an Existing Case" picker with the ADOPTED docker cases. + * + * One adopted container can back several cases, each pointing at a different + * directory inside it (classifyAdoptContainerConflict) — but re-typing the + * container, host and workspace by hand for every directory is exactly the + * friction that makes the capability go unused. Picking a case here fills those + * three and leaves only the two fields that MUST differ: the case name and the + * container workdir. + * + * ⚠️ Adopted cases only (`docker.owned === false`). An owned container's + * lifecycle belongs to its one case — a second case on it would be destroyed + * out from under itself by that case's recreate or delete — and the server + * refuses it, so offering it here would only produce a confusing error. + */ + async _loadDockerCloneOptions() { + const select = document.getElementById('dockerAdoptCloneFrom'); + const row = document.getElementById('dockerAdoptCloneRow'); + if (!select || !row) return; + let cases = []; + try { + const res = await fetch('/api/cases'); + const data = await res.json(); + cases = (Array.isArray(data) ? data : data?.data || []).filter( + (c) => c?.docker && c.docker.owned === false + ); + } catch { + cases = []; + } + select.textContent = ''; + const blank = document.createElement('option'); + blank.value = ''; + blank.textContent = 'Start from scratch'; + select.appendChild(blank); + for (const c of cases) { + const option = document.createElement('option'); + option.value = c.name; + // Server-supplied strings: textContent, never markup. + option.textContent = `${c.name} — ${c.docker.container}:${c.docker.containerWorkdir || c.docker.path}`; + option.dataset.container = c.docker.container; + option.dataset.hostId = c.docker.hostId; + option.dataset.path = c.docker.path; + select.appendChild(option); + } + // Nothing to duplicate yet: an empty picker is noise on the first adoption. + row.hidden = cases.length === 0; + }, + + /** + * Apply the picked case: carry over what STAYS the same, clear what must not. + * + * The two cleared fields are the point of the feature — a duplicate that kept + * the original's name would be rejected as an existing case, and one that kept + * its container workdir would be rejected as an exact twin (both by the server, + * with a clear message, but a form that pre-fills a value it knows will be + * refused is just a trap). + */ + applyDockerCloneSource() { + const select = document.getElementById('dockerAdoptCloneFrom'); + const option = select?.selectedOptions?.[0]; + if (!option || !option.value) return; + const set = (id, value) => { + const el = document.getElementById(id); + if (el) el.value = value || ''; + }; + set('dockerContainerName', option.dataset.container); + set('dockerHostId', option.dataset.hostId); + set('dockerWorkspacePath', option.dataset.path); + set('dockerCaseName', ''); + set('dockerAdoptWorkdir', ''); + document.getElementById('dockerAdoptWorkdir')?.focus(); + }, + async _loadDockerContainerOptions() { const list = document.getElementById('dockerContainerList'); if (!list) return; diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 9c68762d..8b63f540 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -301,6 +301,8 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config container, image: host.image, path: dockerCase.hostWorkspacePath, + containerWorkdir: dockerCase.containerWorkdir ?? dockerCase.hostWorkspacePath, + owned: dockerCase.owned !== false, network: host.network ?? 'bridge', ...(dockerCase.availableModes ? { availableModes: dockerCase.availableModes } : {}), }, @@ -1491,6 +1493,8 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config container, image: host.image, path: dockerCase.hostWorkspacePath, + containerWorkdir: dockerCase.containerWorkdir ?? dockerCase.hostWorkspacePath, + owned: dockerCase.owned !== false, network: host.network ?? 'bridge', }, }; diff --git a/test/docker-adopt-duplicate.test.ts b/test/docker-adopt-duplicate.test.ts new file mode 100644 index 00000000..3196907d --- /dev/null +++ b/test/docker-adopt-duplicate.test.ts @@ -0,0 +1,96 @@ +/** + * @fileoverview "Duplicate an existing case" in the container-adoption form. + * + * The server already allows one ADOPTED container to back several cases, each + * pointing at a different directory inside it (classifyAdoptContainerConflict). + * Re-typing the container, host and workspace by hand for every directory is the + * friction that would leave that capability unused, so the form carries them over + * and clears only the two fields that MUST differ. + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { describe, it, expect } from 'vitest'; + +const html = readFileSync(resolve(import.meta.dirname, '../src/web/public/index.html'), 'utf8'); +const ui = readFileSync(resolve(import.meta.dirname, '../src/web/public/session-ui.js'), 'utf8'); +const routes = readFileSync(resolve(import.meta.dirname, '../src/web/routes/case-routes.ts'), 'utf8'); +const apiTypes = readFileSync(resolve(import.meta.dirname, '../src/types/api.ts'), 'utf8'); + +describe('the API exposes what the picker needs', () => { + it('reports each docker case s directory inside the container', () => { + // Without it the picker cannot show WHICH directory a case already uses, which + // is the one thing the user needs to see before choosing a different one. + expect(apiTypes).toMatch(/containerWorkdir\?: string;/); + expect(routes).toContain('containerWorkdir: dockerCase.containerWorkdir ?? dockerCase.hostWorkspacePath'); + }); + + it('reports whether the container is owned, on EVERY case-shaped response', () => { + // Two sites build a docker CaseInfo (the list and the single-case lookup); + // filling only one leaves the picker blind depending on which the UI read. + expect(routes.match(/owned: dockerCase\.owned !== false,/g) ?? []).toHaveLength(2); + }); + + it('treats an ABSENT owned flag as owned, so legacy cases are not offered', () => { + // `owned` is optional and predates this field; truthiness would read a legacy + // case as adopted and offer a duplicate the server then refuses. + expect(routes).toContain('dockerCase.owned !== false'); + }); +}); + +describe('the picker only offers what the server would accept', () => { + const fn = ui.slice(ui.indexOf('async _loadDockerCloneOptions()'), ui.indexOf('applyDockerCloneSource()')); + + it('filters to ADOPTED cases only', () => { + expect(fn).toMatch(/c\.docker\.owned === false/); + }); + + it('hides the row entirely when there is nothing to duplicate', () => { + expect(fn).toMatch(/row\.hidden = cases\.length === 0/); + }); + + it('builds options with textContent, never markup', () => { + // Case names and container names are user- and engine-supplied strings. + expect(fn).toContain('option.textContent ='); + expect(fn).not.toContain('innerHTML'); + }); +}); + +describe('applying a source fills what stays and clears what must differ', () => { + const fn = ui.slice(ui.indexOf('applyDockerCloneSource()'), ui.indexOf('applyDockerCloneSource()') + 1400); + + it('carries over container, host and workspace', () => { + for (const id of ['dockerContainerName', 'dockerHostId', 'dockerWorkspacePath']) { + expect(fn).toContain(`set('${id}', option.dataset.`); + } + }); + + it('clears the case name and the container workdir', () => { + // Keeping either would pre-fill a value the server is certain to refuse — + // the name as an existing case, the workdir as an exact twin. + expect(fn).toContain("set('dockerCaseName', '')"); + expect(fn).toContain("set('dockerAdoptWorkdir', '')"); + }); + + it('focuses the workdir, the field the user came here to change', () => { + expect(fn).toMatch(/getElementById\('dockerAdoptWorkdir'\)\?\.focus\(\)/); + }); + + it('does nothing for the blank "start from scratch" option', () => { + expect(fn).toMatch(/if \(!option \|\| !option\.value\) return;/); + }); +}); + +describe('the row is wired into the adoption panel', () => { + it('lives in the adopt-only block and starts hidden', () => { + expect(html).toMatch(/id="dockerAdoptCloneRow"[^>]*hidden/); + expect(html).toMatch(/class="form-row docker-adopt-only" id="dockerAdoptCloneRow"/); + }); + + it('loads its options whenever adopt mode turns on', () => { + // Slice from the DEFINITION, not the first call site. + const start = ui.indexOf('_syncDockerAdoptMode() {'); + expect(start).toBeGreaterThan(-1); + const sync = ui.slice(start, start + 900); + expect(sync).toContain('_loadDockerCloneOptions()'); + }); +});