Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions src/lib/onboard/docker-gpu-patch-finalize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ describe("finalizeDockerGpuPatchBackup", () => {
});

it.each(["Error", "Deleting"])(
"accepts %s only when the stopped replacement is the sole labeled container (#9962)",
"accepts %s for the exact stopped replacement despite stale legacy name metadata (#9962)",
(phase) => {
const replacementContainerId = "a".repeat(64);
const events: string[] = [];
Expand All @@ -175,7 +175,7 @@ describe("finalizeDockerGpuPatchBackup", () => {
events.push("remove backup");
return { status: 0 };
});
const dockerRun = vi.fn(() => {
const dockerRun = vi.fn((_args: readonly string[]) => {
events.push("confirm exact replacement");
return { status: 0, stdout: `${replacementContainerId}\n` };
});
Expand All @@ -184,15 +184,15 @@ describe("finalizeDockerGpuPatchBackup", () => {
return { status: 0 };
});
const runOpenshell = vi.fn(() => {
events.push("observe error");
return { status: 0, stdout: `alpha 2026-08-23 01:40:35 ${phase}\n` };
events.push("observe retiring phase");
return { status: 0, stdout: `restored-name 2026-08-23 01:40:35 ${phase}\n` };
});

const outcome = finalizeDockerGpuPatchBackup(
{
result: { ...deferredCreateResult(), newContainerId: replacementContainerId },
supervisorReady: true,
sandboxName: "alpha",
sandboxName: "restored-name",
lifecycleReleaseTimeoutSecs: 60,
},
{ dockerStop, dockerRm, dockerRun, dockerStart, runOpenshell, sleep: vi.fn() },
Expand All @@ -206,17 +206,26 @@ describe("finalizeDockerGpuPatchBackup", () => {
expect(events).toEqual([
"stop replacement",
"remove backup",
"observe error",
"observe retiring phase",
"confirm exact replacement",
"start replacement",
]);
expect(dockerRun).toHaveBeenCalledWith(
expect.arrayContaining([
[
"ps",
"-a",
"--no-trunc",
"--filter",
`id=${replacementContainerId}`,
"--filter",
"label=openshell.ai/managed-by=openshell",
"label=openshell.ai/sandbox-name=alpha",
]),
expect.objectContaining({ ignoreError: true }),
"--format",
"{{.ID}}",
],
expect.objectContaining({ ignoreError: true, suppressOutput: true }),
);
expect(dockerRun.mock.calls[0]?.[0]).not.toContain(
"label=openshell.ai/sandbox-name=restored-name",
);
},
);
Expand Down Expand Up @@ -277,7 +286,7 @@ describe("finalizeDockerGpuPatchBackup", () => {
{ status: 0, stdout: `${"a".repeat(64)}\n${"b".repeat(64)}\n` },
],
["a truncated replacement ID", { status: 0, stdout: `${"a".repeat(12)}\n` }],
])("does not accept Error with %s (#9962)", (_case, dockerResult) => {
])("does not accept Error when exact replacement corroboration returns %s (#9962)", (_case, dockerResult) => {
const replacementContainerId = "a".repeat(64);
const dockerStart = vi.fn(() => ({ status: 0 }));
const outcome = finalizeDockerGpuPatchBackup(
Expand Down
40 changes: 31 additions & 9 deletions src/lib/onboard/docker-gpu-patch-finalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ import {
import { fullDockerContainerId } from "./docker-gpu-patch-clone";
import type { DockerGpuPatchDeps, DockerGpuPatchResult } from "./docker-gpu-patch-types";
import { waitForOpenShellSandboxLifecycleRelease } from "./docker-gpu-supervisor-reconnect";
import { queryOpenShellDockerSandboxContainers } from "./openshell-docker-sandbox-containers";
import {
OPENSHELL_MANAGED_BY_LABEL,
OPENSHELL_MANAGED_BY_VALUE,
} from "./openshell-docker-sandbox-containers";

export {
restoreDockerGpuPatchBackupAfterRecreateFailure as rollbackDockerGpuPatchOnRecreateFailure,
Expand Down Expand Up @@ -62,20 +65,40 @@ export type DockerGpuPatchFinalizeOutcome = {
replacementPresence?: "absent" | "present" | "unknown";
};

function isSoleLabeledReplacement(
sandboxName: string,
function isExactOpenShellReplacement(
replacementContainerId: string,
dockerRun: NonNullable<DockerGpuPatchDeps["dockerRun"]>,
timeoutMs: number,
): boolean {
const expectedContainerId = fullDockerContainerId(replacementContainerId);
if (!expectedContainerId || timeoutMs <= 0) return false;
try {
const containers = queryOpenShellDockerSandboxContainers(sandboxName, { dockerRun }, timeoutMs);
const query = dockerRun(
[
"ps",
"-a",
"--no-trunc",
"--filter",
`id=${expectedContainerId}`,
"--filter",
`label=${OPENSHELL_MANAGED_BY_LABEL}=${OPENSHELL_MANAGED_BY_VALUE}`,
"--format",
"{{.ID}}",
],
{
ignoreError: true,
suppressOutput: true,
timeout: Math.max(1, Math.min(DOCKER_GPU_PATCH_TIMEOUT_MS, Math.floor(timeoutMs))),
},
);
if (!hasZeroDockerExitStatus(query)) return false;
const containerIds = String(query.stdout ?? "")
.split(/\r?\n/u)
.map((line) => line.trim())
.filter(Boolean);
return (
containers.ok &&
containers.ids.length === 1 &&
fullDockerContainerId(containers.ids[0]) === expectedContainerId
containerIds.length === 1 &&
fullDockerContainerId(containerIds[0]) === expectedContainerId
);
} catch {
return false;
Expand Down Expand Up @@ -129,8 +152,7 @@ export function finalizeDockerGpuPatchBackup(
runOpenshell: deps.runOpenshell,
sleep: deps.sleep,
soleLabeledReplacementCorroboratesRetiringPhase: (remainingMs) =>
isSoleLabeledReplacement(
sandboxName,
isExactOpenShellReplacement(
options.result.newContainerId,
resolved.dockerRun,
remainingMs,
Expand Down
11 changes: 6 additions & 5 deletions src/lib/onboard/docker-gpu-supervisor-reconnect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,12 @@ type DockerLifecycleReleaseDeps = Pick<
* - The caller enters this wait only after the replacement reached Ready and
* was deliberately stopped. A successful list normally omits the sandbox
* name. An Error or Deleting row is also sufficient only when a separate
* bounded Docker query confirms that exact stopped replacement is the sole
* remaining labeled container. This corroborates the release condition;
* the OpenShell row alone is not an identity-bound ownership receipt. The
* Deleting case breaks the otherwise circular wait where OpenShell retains
* the row until that exact replacement emits its restart event.
* bounded Docker query confirms that the transaction-owned full replacement
* ID still identifies exactly one OpenShell-managed container. This
* corroborates the release condition; the OpenShell row alone is not an
* identity-bound ownership receipt. The Deleting case breaks the otherwise
* circular wait where OpenShell retains the row until that exact replacement
* emits its restart event.
* - `waits for the sandbox name to disappear before restarting the
* replacement (#9531)` protects the event order. `rejects final handoff when
* OpenShell never releases the deleting lifecycle record (#9531)` protects
Expand Down
Loading