fix(docker): verify the container workdir and end the probe with exit 0

Two defects that only a real container exposes.

The probe chained `command -v X && echo X` with semicolons, and a script's exit
status is its last command's. A container without the last probed CLI made the
whole `sh -lc` exit 1, so a perfectly healthy container with tmux and claude was
reported as "could not exec into the container". A missing CLI is data here, not
failure, so the script now ends with `exit 0`.

containerWorkdir defaulted to hostWorkspacePath. That default holds for an owned
container only because the create-time bind mount puts the host directory at that
exact path; attaching mounts nothing, so the two are independent facts. A host
path absent inside the container makes `docker exec --workdir` fail with an OCI
chdir error that surfaces in the pane as a bare "execvp failed". The preflight now
proves the directory exists inside the container and refuses at link time.
This commit is contained in:
d fei
2026-09-03 01:57:56 -07:00
parent 7bd0259171
commit b8e134df93
5 changed files with 69 additions and 7 deletions
+37 -3
View File
@@ -1084,6 +1084,8 @@ export interface AdoptedContainerProbe {
tmuxPath?: string; tmuxPath?: string;
/** Modes whose CLI resolved inside the container (`command -v <mode>`). */ /** Modes whose CLI resolved inside the container (`command -v <mode>`). */
availableModes?: SessionMode[]; availableModes?: SessionMode[];
/** Whether the requested working directory exists INSIDE the container. */
workdirExists?: boolean;
error?: string; error?: string;
} }
@@ -1099,10 +1101,18 @@ export interface AdoptedContainerProbe {
*/ */
export async function probeAdoptableContainer( export async function probeAdoptableContainer(
docker: Pick<SessionDocker, 'engine' | 'context' | 'daemonHost' | 'containerName'>, docker: Pick<SessionDocker, 'engine' | 'context' | 'daemonHost' | 'containerName'>,
modes: SessionMode[] = [] modes: SessionMode[] = [],
containerWorkdir?: string
): Promise<AdoptedContainerProbe> { ): Promise<AdoptedContainerProbe> {
if (IS_TEST_MODE) { if (IS_TEST_MODE) {
return { ok: true, exists: true, running: true, tmuxPath: '/usr/bin/tmux', availableModes: modes }; return {
ok: true,
exists: true,
running: true,
tmuxPath: '/usr/bin/tmux',
availableModes: modes,
workdirExists: true,
};
} }
const argv = dockerEngineArgv(docker); const argv = dockerEngineArgv(docker);
let running = false; let running = false;
@@ -1136,7 +1146,19 @@ export async function probeAdoptableContainer(
// One exec resolves tmux plus every requested CLI, so adoption costs a single // One exec resolves tmux plus every requested CLI, so adoption costs a single
// round trip. Binaries are fixed mode names, never user input. // round trip. Binaries are fixed mode names, never user input.
const probes = ['tmux', ...modes.filter((m) => m !== 'shell')]; const probes = ['tmux', ...modes.filter((m) => m !== 'shell')];
const script = probes.map((bin) => `command -v ${bin} >/dev/null 2>&1 && echo ${bin}`).join('; '); // `; exit 0` is load-bearing: the script's status is its LAST command's, so a
// missing final CLI made the whole `sh -lc` exit 1 and the probe reported
// "could not exec into the container" for a container that was perfectly fine.
// Absence of a CLI is data here, not failure — only a real exec error is.
const steps = probes.map((bin) => `command -v ${bin} >/dev/null 2>&1 && echo ${bin}`);
// The workdir is checked INSIDE the container, and that is a fact independent
// of hostWorkspacePath: an owned container gets the host dir bind-mounted at the
// same absolute path at create time, but adoption mounts nothing, so the two
// paths only coincide if the user mounted it there themselves. `docker exec
// --workdir <missing>` fails with an OCI chdir error the pane surfaces as a bare
// "execvp failed", so it is resolved here into an actionable message.
if (containerWorkdir) steps.push(`[ -d ${shellescape(containerWorkdir)} ] && echo __workdir__`);
const script = `${steps.join('; ')}; exit 0`;
try { try {
const { stdout } = await execFileAsync( const { stdout } = await execFileAsync(
argv[0], argv[0],
@@ -1158,6 +1180,17 @@ export async function probeAdoptableContainer(
error: `container "${docker.containerName}" has no tmux (required for durable sessions; install it inside the container)`, error: `container "${docker.containerName}" has no tmux (required for durable sessions; install it inside the container)`,
}; };
} }
const workdirExists = containerWorkdir ? found.has('__workdir__') : undefined;
if (containerWorkdir && !workdirExists) {
return {
ok: false,
exists: true,
running: true,
image,
workdirExists: false,
error: `"${containerWorkdir}" does not exist inside container "${docker.containerName}". Adoption mounts nothing, so the container workdir must already exist there — set it to a path inside the container (it need not match the host workspace path).`,
};
}
return { return {
ok: true, ok: true,
exists: true, exists: true,
@@ -1165,6 +1198,7 @@ export async function probeAdoptableContainer(
image, image,
tmuxPath: 'tmux', tmuxPath: 'tmux',
availableModes: modes.filter((m) => m === 'shell' || found.has(m)), availableModes: modes.filter((m) => m === 'shell' || found.has(m)),
workdirExists,
}; };
} catch (err) { } catch (err) {
const msg = err instanceof Error ? err.message : String(err); const msg = err instanceof Error ? err.message : String(err);
+5
View File
@@ -2856,6 +2856,11 @@
<input type="text" id="dockerContainerName" placeholder="my-dev-box" pattern="[a-zA-Z0-9][a-zA-Z0-9_.-]+" autocomplete="off" autocapitalize="off" spellcheck="false"> <input type="text" id="dockerContainerName" placeholder="my-dev-box" pattern="[a-zA-Z0-9][a-zA-Z0-9_.-]+" autocomplete="off" autocapitalize="off" spellcheck="false">
<span class="form-hint">Must be running already. <button type="button" class="btn-inline-check" id="dockerAdoptCheckBtn">Check container</button></span> <span class="form-hint">Must be running already. <button type="button" class="btn-inline-check" id="dockerAdoptCheckBtn">Check container</button></span>
</div> </div>
<div class="form-row docker-adopt-only">
<label>Container Workdir</label>
<input type="text" id="dockerAdoptWorkdir" placeholder="/workspace" autocomplete="off" autocapitalize="off" autocorrect="off" spellcheck="false">
<span class="form-hint">A path that already exists <em>inside</em> the container. Adoption mounts nothing, so this need not match the host workspace path — leave blank to reuse it only if you mounted it there yourself.</span>
</div>
<div class="form-row"> <div class="form-row">
<label>Case Name</label> <label>Case Name</label>
<input type="text" id="dockerCaseName" placeholder="sandbox" pattern="[a-zA-Z0-9_-]+" autocomplete="off" autocapitalize="off" spellcheck="false"> <input type="text" id="dockerCaseName" placeholder="sandbox" pattern="[a-zA-Z0-9_-]+" autocomplete="off" autocapitalize="off" spellcheck="false">
+8 -2
View File
@@ -2975,6 +2975,7 @@ Object.assign(CodemanApp.prototype, {
async _dockerAdoptPreflight() { async _dockerAdoptPreflight() {
const statusEl = document.getElementById('dockerLinkStatus'); const statusEl = document.getElementById('dockerLinkStatus');
const container = document.getElementById('dockerContainerName')?.value.trim(); const container = document.getElementById('dockerContainerName')?.value.trim();
const containerWorkdir = document.getElementById('dockerAdoptWorkdir')?.value.trim();
const hostId = document.getElementById('dockerHostId').value.trim() || 'local'; const hostId = document.getElementById('dockerHostId').value.trim() || 'local';
if (!container) { if (!container) {
if (statusEl) statusEl.textContent = 'Enter a container name first.'; if (statusEl) statusEl.textContent = 'Enter a container name first.';
@@ -2986,7 +2987,7 @@ Object.assign(CodemanApp.prototype, {
const probe = await this._apiJson('/api/docker-cases/adopt-preflight', { const probe = await this._apiJson('/api/docker-cases/adopt-preflight', {
method: 'POST', method: 'POST',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ hostId, container }), body: JSON.stringify({ hostId, container, ...(containerWorkdir ? { containerWorkdir } : {}) }),
}); });
if (!statusEl) return; if (!statusEl) return;
if (!probe) { if (!probe) {
@@ -3009,6 +3010,7 @@ Object.assign(CodemanApp.prototype, {
const hostId = document.getElementById('dockerHostId').value.trim() || 'local'; const hostId = document.getElementById('dockerHostId').value.trim() || 'local';
const adopting = !!document.getElementById('dockerAdoptExisting')?.checked; const adopting = !!document.getElementById('dockerAdoptExisting')?.checked;
const container = document.getElementById('dockerContainerName')?.value.trim() || ''; const container = document.getElementById('dockerContainerName')?.value.trim() || '';
const adoptWorkdir = document.getElementById('dockerAdoptWorkdir')?.value.trim() || '';
const image = document.getElementById('dockerImage').value.trim() || 'codeman/agent:base'; const image = document.getElementById('dockerImage').value.trim() || 'codeman/agent:base';
const network = document.getElementById('dockerNetwork').value; const network = document.getElementById('dockerNetwork').value;
const memory = document.getElementById('dockerMemory').value.trim(); const memory = document.getElementById('dockerMemory').value.trim();
@@ -3076,7 +3078,11 @@ Object.assign(CodemanApp.prototype, {
const caseRes = await fetch(adopting ? '/api/cases/docker-adopt' : '/api/cases/docker-link', { const caseRes = await fetch(adopting ? '/api/cases/docker-adopt' : '/api/cases/docker-link', {
method: 'POST', method: 'POST',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(adopting ? { name, hostId, hostWorkspacePath, container } : { name, hostId, hostWorkspacePath }), body: JSON.stringify(
adopting
? { name, hostId, hostWorkspacePath, container, ...(adoptWorkdir ? { containerWorkdir: adoptWorkdir } : {}) }
: { name, hostId, hostWorkspacePath }
),
}); });
const caseData = await caseRes.json(); const caseData = await caseRes.json();
if (caseData.success) { if (caseData.success) {
+11 -2
View File
@@ -837,7 +837,15 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
availability.error || 'docker daemon is not available' availability.error || 'docker daemon is not available'
); );
} }
const probe = await probeAdoptableContainer(toSessionDocker(host, dockerCase), [...DOCKER_ADOPT_PROBE_MODES]); // The container workdir is validated INSIDE the container. It defaults to
// hostWorkspacePath only because that is what an owned container's bind
// mount guarantees; adoption mounts nothing, so the probe has to prove it.
const adoptDocker = toSessionDocker(host, dockerCase);
const probe = await probeAdoptableContainer(
adoptDocker,
[...DOCKER_ADOPT_PROBE_MODES],
adoptDocker.containerWorkdir
);
if (!probe.ok) { if (!probe.ok) {
return createErrorResponse(ApiErrorCode.OPERATION_FAILED, probe.error || 'container is not adoptable'); return createErrorResponse(ApiErrorCode.OPERATION_FAILED, probe.error || 'container is not adoptable');
} }
@@ -871,7 +879,8 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
daemonHost: host.daemonHost, daemonHost: host.daemonHost,
containerName: body.container, containerName: body.container,
}, },
[...DOCKER_ADOPT_PROBE_MODES] [...DOCKER_ADOPT_PROBE_MODES],
body.containerWorkdir
); );
return { success: true, data: probe }; return { success: true, data: probe };
}); });
+8
View File
@@ -857,6 +857,14 @@ export const DockerAdoptPreflightSchema = z.object({
.min(2) .min(2)
.max(128) .max(128)
.regex(/^[a-zA-Z0-9][a-zA-Z0-9_.-]+$/, 'Invalid container name'), .regex(/^[a-zA-Z0-9][a-zA-Z0-9_.-]+$/, 'Invalid container name'),
/** Optional: also verify this path exists INSIDE the container. */
containerWorkdir: z
.string()
.min(1)
.max(2000)
.regex(/^\//, 'Container workdir must be absolute')
.regex(NO_SHELL_META, 'Invalid characters in container workdir')
.optional(),
}); });
export const DockerExportSchema = z.object({ export const DockerExportSchema = z.object({