mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
+4
-7
@@ -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;
|
||||
};
|
||||
|
||||
@@ -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 } : {}),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user