From 9591b973cf24599121a732ae3f229a4778d75292 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Mon, 14 Sep 2026 23:48:10 +0200 Subject: [PATCH] fix(docker): carry the owned flag on the wire the way master already does The cherry-picked "copy an existing case" commit declared a second `CaseInfo.docker.owned` and emitted `owned: true|false` on every docker case, while master had meanwhile shipped the same field from the adopted-container work with a narrower wire shape: `owned` is present only when false, absent means owned. Two declarations failed typecheck, and two emit styles on one response would have made the picker's answer depend on which read path filled it. Keep master's shape at both response sites (the case list and the single-case lookup, which lacked the field entirely), fold the picker's reason for the field into the existing doc comment, and repoint the test that pinned "set on exactly two sites" at the surviving form, adding a negative pin so the duplicate style cannot come back. Co-Authored-By: Claude Fable 5.1 --- src/types/api.ts | 11 ++++------- src/web/routes/case-routes.ts | 3 +-- test/docker-adopt-duplicate.test.ts | 11 +++++++---- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/types/api.ts b/src/types/api.ts index 11000e5e..1a6193c7 100644 --- a/src/types/api.ts +++ b/src/types/api.ts @@ -186,13 +186,6 @@ export interface CaseInfo { 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 @@ -210,6 +203,10 @@ export interface CaseInfo { * first session: the container is created on demand by the launch chain, so treating * "not found" as a fault there hid every agent mode behind an error telling the user * to start a container Codeman was about to create itself. + * + * It also gates the Add Case panel's "copy an existing case" picker: only an ADOPTED + * container may back several cases at once (`classifyAdoptContainerConflict`), since an + * owned container's lifecycle belongs to its one case. */ owned?: boolean; }; diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 492b52a3..643283e9 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -329,7 +329,6 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config image: host.image, path: dockerCase.hostWorkspacePath, containerWorkdir: dockerCase.containerWorkdir ?? dockerCase.hostWorkspacePath, - owned: dockerCase.owned !== false, network: host.network ?? 'bridge', ...(dockerCase.availableModes ? { availableModes: dockerCase.availableModes } : {}), ...(dockerCase.owned === false ? { owned: false } : {}), @@ -1610,8 +1609,8 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config image: host.image, path: dockerCase.hostWorkspacePath, containerWorkdir: dockerCase.containerWorkdir ?? dockerCase.hostWorkspacePath, - owned: dockerCase.owned !== false, network: host.network ?? 'bridge', + ...(dockerCase.owned === false ? { owned: false } : {}), }, }; } diff --git a/test/docker-adopt-duplicate.test.ts b/test/docker-adopt-duplicate.test.ts index 10bb90f2..e9cfd77c 100644 --- a/test/docker-adopt-duplicate.test.ts +++ b/test/docker-adopt-duplicate.test.ts @@ -27,13 +27,16 @@ describe('the API exposes what the picker needs', () => { 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); + expect(routes.match(/\.\.\.\(dockerCase\.owned === false \? \{ 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'); + // `owned` is optional and predates this field; the wire carries it ONLY when + // false (absent = owned, the shape master already used), so the picker must + // test `=== false` rather than truthiness, or a legacy case would read as + // adopted and be offered a duplicate the server then refuses. + expect(routes).toContain('...(dockerCase.owned === false ? { owned: false } : {}),'); + expect(routes).not.toContain('owned: dockerCase.owned !== false'); }); });