diff --git a/src/docker-hosts.ts b/src/docker-hosts.ts index d83ef866..6a0d705b 100644 --- a/src/docker-hosts.ts +++ b/src/docker-hosts.ts @@ -293,6 +293,64 @@ export function toSessionDocker(host: DockerHost, dockerCase: DockerCase): Sessi return session; } +/** + * Which existing case, if any, blocks adopting `container` at `containerWorkdir`. + * + * One container may back SEVERAL adopted cases, each pointing at a different + * directory inside it — that is the whole reason to adopt the same container + * twice, and it is safe because the in-container tmux session is named per + * SESSION (`dockerTmuxSessionName`, `codeman-dkr-`) and not per case, so a + * session teardown kills exactly one session and its siblings on the shared + * in-container tmux server are untouched. Nothing else reaches an adopted + * container's lifecycle either: stop/remove throw at the builder, recreate + * refuses `owned === false`, and the orphan reaper filters on the + * `codeman.managed=1` label that only Codeman-created containers carry. + * + * So the conflicts that remain are NOT about the tmux server: + * - `owned-case` the container backs a case Codeman CREATED, whose lifecycle + * it owns; a recreate or delete there would destroy the + * adopted case's container out from under it. + * - `other-owner` already adopted by a different user. Adoption hands out a + * shell inside someone else's container, so it stays scoped. + * - `duplicate` same container AND same directory: the second case would + * behave identically to the first, so name the first instead + * of silently creating a twin. A DIFFERENT directory is the + * supported case and returns null. + */ +export type AdoptContainerConflict = + | { kind: 'owned-case'; caseName: string } + | { kind: 'other-owner'; caseName: string } + | { kind: 'duplicate'; caseName: string } + | null; + +export function classifyAdoptContainerConflict(params: { + container: string; + /** Directory inside the container this adoption targets (already defaulted). */ + containerWorkdir: string; + existing: ReadonlyArray< + Pick + >; + /** Owner visibility test (canAccessOwned bound to the caller). */ + canAccess: (owner?: string) => boolean; +}): AdoptContainerConflict { + const { container, containerWorkdir, existing, canAccess } = params; + const sharing = existing.filter((item) => (item.container ?? dockerContainerName(item.name)) === container); + if (sharing.length === 0) return null; + + // `owned` is optional and an ABSENT flag means owned (legacy cases predate the + // field), so this must test `!== false` rather than truthiness. + const owned = sharing.find((item) => item.owned !== false); + if (owned) return { kind: 'owned-case', caseName: owned.name }; + + const foreign = sharing.find((item) => !canAccess(item.owner)); + if (foreign) return { kind: 'other-owner', caseName: foreign.name }; + + const twin = sharing.find((item) => (item.containerWorkdir ?? item.hostWorkspacePath) === containerWorkdir); + if (twin) return { kind: 'duplicate', caseName: twin.name }; + + return null; +} + /** * An ADOPTED container is one the user built and runs themselves. Codeman may * only exec into it; it must never create, start, stop, restart or remove it. diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index d202c8bc..31551df0 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -79,6 +79,7 @@ import { dockerContainerName, dockerDisplayPath, probeAdoptableContainer, + classifyAdoptContainerConflict, listDockerContainers, browseInContainer, dockerAdoptProbeModes, @@ -906,12 +907,35 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config ) { return createErrorResponse(ApiErrorCode.ALREADY_EXISTS, 'Case already exists'); } - // Two cases must never share one adopted container: session close kills the - // in-container tmux by session id, but a shared adoption would let one case's - // teardown and another's launch race over the same tmux server. + // One container may back SEVERAL adopted cases, each pointing at a different + // directory inside it. What still blocks it, and why, lives in + // classifyAdoptContainerConflict — note that none of it is about the shared + // in-container tmux server, which is safe precisely because sessions there + // are named per SESSION id (`codeman-dkr-`), never per case. const container = dockerCase.container; - if (dockerCases.some((item) => (item.container ?? dockerContainerName(item.name)) === container)) { - return createErrorResponse(ApiErrorCode.ALREADY_EXISTS, `Container "${container}" is already linked to a case`); + const conflict = classifyAdoptContainerConflict({ + container, + containerWorkdir: dockerCase.containerWorkdir ?? dockerCase.hostWorkspacePath, + existing: dockerCases, + canAccess: (owner) => canAccessOwned(getAuthUser(req), owner), + }); + if (conflict?.kind === 'owned-case') { + return createErrorResponse( + ApiErrorCode.ALREADY_EXISTS, + `Container "${container}" belongs to case "${conflict.caseName}", which Codeman created and whose lifecycle it manages. Adopt a container you started yourself, or open that case directly.` + ); + } + if (conflict?.kind === 'other-owner') { + return createErrorResponse( + ApiErrorCode.FORBIDDEN, + `Container "${container}" is already adopted by another user.` + ); + } + if (conflict?.kind === 'duplicate') { + return createErrorResponse( + ApiErrorCode.ALREADY_EXISTS, + `Case "${conflict.caseName}" already adopts "${container}" at that same directory. Point this one at another directory inside the container.` + ); } if (!isWorkingDirAllowed(getAuthUser(req), dockerCase.hostWorkspacePath)) { diff --git a/test/docker-adopted-container.test.ts b/test/docker-adopted-container.test.ts index 967c5237..1f3c4afb 100644 --- a/test/docker-adopted-container.test.ts +++ b/test/docker-adopted-container.test.ts @@ -19,6 +19,8 @@ import { checkDockerConfigDrift, dockerConfigHash, dockerAdoptProbeModes, + classifyAdoptContainerConflict, + dockerContainerName, } from '../src/docker-hosts.js'; import { enabledCliIds, getCli } from '../src/config/cli-registry/index.js'; import { @@ -451,3 +453,113 @@ describe('adopted container: a missing container means different things per owne expect(routes).toContain('...(dockerCase.owned === false ? { owned: false } : {}),'); }); }); + +describe('adopted container: one container may back several cases', () => { + const base = { + type: 'docker' as const, + hostId: 'h1', + hostWorkspacePath: '/srv/work', + }; + const mk = (over: Record) => ({ ...base, ...over }) as never; + const mine = () => true; + + it('allows a second adoption of the same container at a DIFFERENT directory', () => { + // The whole point of the feature: one container, two folders, two cases. + const conflict = classifyAdoptContainerConflict({ + container: 'devbox', + containerWorkdir: '/app/api', + existing: [mk({ name: 'web', container: 'devbox', containerWorkdir: '/app/web', owned: false })], + canAccess: mine, + }); + expect(conflict).toBeNull(); + }); + + it('refuses an exact twin (same container AND same directory) and names the first case', () => { + const conflict = classifyAdoptContainerConflict({ + container: 'devbox', + containerWorkdir: '/app/web', + existing: [mk({ name: 'web', container: 'devbox', containerWorkdir: '/app/web', owned: false })], + canAccess: mine, + }); + expect(conflict).toEqual({ kind: 'duplicate', caseName: 'web' }); + }); + + it('falls back to hostWorkspacePath when containerWorkdir is absent on either side', () => { + // containerWorkdir defaults to hostWorkspacePath, so an absent field on the + // stored case must compare equal to an incoming adoption that omits it too — + // otherwise the twin check silently stops firing for the default case. + const conflict = classifyAdoptContainerConflict({ + container: 'devbox', + containerWorkdir: '/srv/work', + existing: [mk({ name: 'web', container: 'devbox', owned: false })], + canAccess: mine, + }); + expect(conflict).toEqual({ kind: 'duplicate', caseName: 'web' }); + }); + + it('still refuses a container backing a case Codeman CREATED', () => { + // Codeman owns that container's lifecycle: a recreate or case-delete there + // would destroy the adopted case's container out from under it. + const conflict = classifyAdoptContainerConflict({ + container: 'codeman-case-web', + containerWorkdir: '/app/api', + existing: [mk({ name: 'web', container: 'codeman-case-web', owned: true })], + canAccess: mine, + }); + expect(conflict).toEqual({ kind: 'owned-case', caseName: 'web' }); + }); + + it('treats an ABSENT owned flag as owned, so legacy cases keep the old refusal', () => { + const conflict = classifyAdoptContainerConflict({ + container: 'legacy', + containerWorkdir: '/app/api', + existing: [mk({ name: 'old', container: 'legacy' })], + canAccess: mine, + }); + expect(conflict).toEqual({ kind: 'owned-case', caseName: 'old' }); + }); + + it('derives the container name from the case name when the field is absent', () => { + const conflict = classifyAdoptContainerConflict({ + container: dockerContainerName('web'), + containerWorkdir: '/app/api', + existing: [mk({ name: 'web', owned: true })], + canAccess: mine, + }); + expect(conflict).toEqual({ kind: 'owned-case', caseName: 'web' }); + }); + + it('refuses a container another user already adopted', () => { + const conflict = classifyAdoptContainerConflict({ + container: 'devbox', + containerWorkdir: '/app/api', + existing: [mk({ name: 'theirs', container: 'devbox', owned: false, owner: 'bob' })], + canAccess: (owner) => owner === 'alice', + }); + expect(conflict).toEqual({ kind: 'other-owner', caseName: 'theirs' }); + }); + + it('an owned case outranks a foreign adoption, so the message names the real blocker', () => { + const conflict = classifyAdoptContainerConflict({ + container: 'devbox', + containerWorkdir: '/app/api', + existing: [ + mk({ name: 'theirs', container: 'devbox', owned: false, owner: 'bob' }), + mk({ name: 'built', container: 'devbox', owned: true }), + ], + canAccess: (owner) => owner === 'alice', + }); + expect(conflict).toEqual({ kind: 'owned-case', caseName: 'built' }); + }); + + it('leaves an unrelated container alone', () => { + expect( + classifyAdoptContainerConflict({ + container: 'fresh', + containerWorkdir: '/app', + existing: [mk({ name: 'web', container: 'devbox', owned: false })], + canAccess: mine, + }) + ).toBeNull(); + }); +});