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
4 changes: 4 additions & 0 deletions docs/reference/commands.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,10 @@ When the host Docker daemon is reachable but the per-sandbox container is stoppe
If the owning OpenShell gateway is healthy but no longer lists the registered Docker-driver sandbox, status attempts post-reboot recovery from the labeled container.
It waits for Docker readiness, restores the in-sandbox gateway and host forwards, and refreshes preflight before probing inference.
A successful recovery clears the stale stopped-container failure.
<AgentOnly variant="openclaw">
If OpenShell already reports the registered Docker-driver sandbox as present and `Ready`, status verifies the OpenClaw gateway and host forward.
It recovers either component when the verification reports it absent.
</AgentOnly>
If Docker readiness or the agent delivery chain cannot be proven, status exits non-zero and reports the failed recovery layer.
<AgentOnly variant="openclaw,hermes">
If the sandbox's recorded dashboard port is also held by a foreign listener, the header escalates to the `sandbox_dashboard_port_conflict` failure layer with the message `sandbox container is stopped and the dashboard port is held by a foreign listener.` so the operator can recover the port before restarting the sandbox.
Expand Down
26 changes: 26 additions & 0 deletions src/lib/actions/sandbox/process-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,32 @@ describe("recreated sandbox OpenShell readiness", () => {
expect(sleeps).toEqual([3]);
});

it("retries the exact same-sandbox Error phase when OpenShell also emits informational stdout", () => {
const captureOpenshellImpl = vi
.fn()
.mockReturnValueOnce({
status: 1,
output:
`Waiting for sandbox registration\n${OPENSHELL_TRANSIENT_ERROR_PHASE_STDERR}`.trim(),
stdout: "Waiting for sandbox registration\n",
stderr: OPENSHELL_TRANSIENT_ERROR_PHASE_STDERR,
})
.mockReturnValueOnce({ status: 0, output: "", stdout: "", stderr: "" });
const sleeps: number[] = [];

expect(
waitForRecreatedSandboxOpenShellReady("recreated-box", {
beforeProbe: () => true,
captureOpenshellImpl,
intervalSeconds: 3,
sleepImpl: (seconds) => sleeps.push(seconds),
timeoutSeconds: 30,
}),
).toBe(true);
expect(captureOpenshellImpl).toHaveBeenCalledTimes(2);
expect(sleeps).toEqual([3]);
});

it("rides out a transient Error phase past the old 30s budget by default (#7227)", () => {
// No timeoutSeconds option and no env override: the default recovery budget
// must be large enough (120s, aligned with connect's readiness wait) to keep
Expand Down
18 changes: 9 additions & 9 deletions src/lib/actions/sandbox/process-recovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -599,30 +599,30 @@ function normalizeOpenshellStructuredError(value: string): string {
return stripAnsi(value).replace(/[×│]/gu, " ").replace(/\s+/gu, " ").trim();
}

function hasRetryableOpenshellResultShape(result: ReturnType<typeof captureOpenshell>): boolean {
return (
result.status === 1 &&
!result.error &&
String(result.stdout ?? "").trim() === "" &&
String(result.stderr ?? "").trim() !== ""
);
function hasRetryableOpenshellFailureShape(result: ReturnType<typeof captureOpenshell>): boolean {
return result.status === 1 && !result.error && String(result.stderr ?? "").trim() !== "";
}

function isRetryableOpenshellReRegistrationState(
result: ReturnType<typeof captureOpenshell>,
sandboxName: string,
): boolean {
if (!hasRetryableOpenshellResultShape(result)) return false;
if (!hasRetryableOpenshellFailureShape(result)) return false;
const error = normalizeOpenshellStructuredError(String(result.stderr));
if (error === OPENSHELL_SANDBOX_NOT_READY) return true;
// OpenShell can publish Ready before replacement registration settles.
// Retry only if the readiness probe reports phase Error for this sandbox.
// The CLI can emit informational stdout before this exact stderr refusal;
// stdout does not change the result of the read-only `true` probe.
if (
error ===
`Error: sandbox '${sandboxName}' is not ready (phase: Error); wait for it to reach Ready state.`
) {
return true;
}
// All less-specific transient signatures remain constrained to an otherwise
// empty stdout stream so unrelated command output cannot be reclassified.
if (String(result.stdout ?? "").trim() !== "") return false;
if (error === OPENSHELL_SANDBOX_NOT_READY) return true;

// OpenShell 0.0.85 can keep the recreated sandbox's cached phase at Ready
// while its replacement supervisor session is still registering. The exec
Expand Down
23 changes: 21 additions & 2 deletions src/lib/actions/sandbox/status-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ describe("showSandboxStatus flow", () => {
lookup: {
state: "sandbox_recovery_failed",
output:
" Docker restored sandbox 'alpha', but its agent delivery chain is not ready " +
" Sandbox 'alpha' is present, but its agent delivery chain could not be proven " +
"(forward-recovery: OpenShell forward state unavailable).",
recoveredSandbox: true,
},
Expand All @@ -567,12 +567,31 @@ describe("showSandboxStatus flow", () => {

const output = harness.logSpy.mock.calls.flat().join("\n");
expect(output).toContain("restored from Docker");
expect(output).toContain("agent delivery chain could not be recovered safely");
expect(output).toContain("agent delivery chain could not be proven");
expect(output).toContain("forward-recovery: OpenShell forward state unavailable");
expect(output).toContain("Retry `nemoclaw alpha recover`");
expect(output).not.toContain("Could not verify against live gateway");
});

it("does not claim Docker restoration when a visible sandbox fails delivery recovery", async () => {
const harness = createStatusFlowHarness({
inferenceHealth: null,
lookup: {
state: "sandbox_recovery_failed",
output:
" Sandbox 'alpha' is present, but its agent delivery chain could not be proven " +
"(gateway-recovery: the managed agent gateway could not be restarted).",
},
});

await expect(harness.showSandboxStatus("alpha")).rejects.toThrow("process.exit(1)");

const output = harness.logSpy.mock.calls.flat().join("\n");
expect(output).toContain("Sandbox 'alpha' is present");
expect(output).toContain("agent delivery chain could not be proven");
expect(output).not.toContain("restored from Docker");
});

it("renders missing gateway metadata after restart without claiming recovery", async () => {
const harness = createStatusFlowHarness({
inferenceHealth: null,
Expand Down
5 changes: 4 additions & 1 deletion src/lib/actions/sandbox/status-lookup-rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,11 @@ function printSandboxRecoveryFailedLookupStatus({
lookup,
}: SandboxGatewayLookupStatusContext): void {
console.log("");
const recoveredFromDocker = "recoveredSandbox" in lookup && lookup.recoveredSandbox === true;
console.log(
` Sandbox '${sandboxName}' was restored from Docker, but its agent delivery chain could not be recovered safely.`,
recoveredFromDocker
? ` Sandbox '${sandboxName}' was restored from Docker, but its agent delivery chain could not be proven.`
: ` Sandbox '${sandboxName}' is present, but its agent delivery chain could not be proven.`,
);
if (lookup.output) console.log(lookup.output);
console.log(
Expand Down
93 changes: 93 additions & 0 deletions src/lib/actions/sandbox/status-snapshot-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,99 @@ function snapshotDeps(recoveryResult: unknown) {
}

describe("collectSandboxStatusSnapshot Docker recovery", () => {
it("recovers the delivery chain when OpenShell already reports the restarted container (#7824)", async () => {
const deps = {
...snapshotDeps({
checked: true,
wasRunning: false,
recovered: true,
forwardRecovered: true,
}),
reconcile: () =>
Promise.resolve({
state: "present" as const,
output: "Phase: Ready",
}),
};

const snapshot = await collectSandboxStatusSnapshot("alpha", { deps });

expect(deps.recoverSandboxProcesses).toHaveBeenCalledWith("alpha", { quiet: true });
expect(snapshot.lookup.state).toBe("present");
});

it("fails closed when the visible restarted container cannot recover OpenClaw (#7824)", async () => {
const deps = {
...snapshotDeps({
checked: true,
wasRunning: false,
recovered: false,
forwardRecovered: false,
}),
reconcile: () =>
Promise.resolve({
state: "present" as const,
output: "Phase: Ready",
}),
};

const snapshot = await collectSandboxStatusSnapshot("alpha", { deps });

expect(snapshot.lookup.state).toBe("sandbox_recovery_failed");
expect(snapshot.lookup.output).toContain(
"Sandbox 'alpha' is present, but its agent delivery chain could not be proven",
);
expect(deps.probeSandboxInferenceGatewayHealthImpl).not.toHaveBeenCalled();
});

it.each([
"Provisioning",
"Failed",
])("keeps the existing %s phase diagnosis ahead of markerless recovery (#7824)", async (phase) => {
const deps = {
...snapshotDeps({
checked: true,
wasRunning: false,
recovered: false,
forwardRecovered: false,
}),
reconcile: () =>
Promise.resolve({
state: "present" as const,
output: `Phase: ${phase}`,
}),
};

const snapshot = await collectSandboxStatusSnapshot("alpha", { deps });

expect(deps.recoverSandboxProcesses).not.toHaveBeenCalled();
expect(snapshot.lookup.state).toBe("present");
});
Comment on lines +126 to +148

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Strengthen the precedence regression assertions.

Both tests currently prove only that recovery was skipped and the lookup remained present; they do not prove that the existing diagnosis or preflight behavior was preserved.

  • src/lib/actions/sandbox/status-snapshot-recovery.test.ts#L126-L148: assert that the output retains Phase: ${phase}.
  • src/lib/actions/sandbox/status-snapshot-recovery.test.ts#L150-L172: assert the observable preflight effect, such as inference-gateway probe suppression.
📍 Affects 1 file
  • src/lib/actions/sandbox/status-snapshot-recovery.test.ts#L126-L148 (this comment)
  • src/lib/actions/sandbox/status-snapshot-recovery.test.ts#L150-L172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/actions/sandbox/status-snapshot-recovery.test.ts` around lines 126 -
148, The parameterized precedence tests in
src/lib/actions/sandbox/status-snapshot-recovery.test.ts:126-148 must assert
that snapshot output retains `Phase: ${phase}` in addition to the existing
recovery and lookup assertions. The related test at
src/lib/actions/sandbox/status-snapshot-recovery.test.ts:150-172 must assert the
observable preflight behavior, including suppression of the inference-gateway
probe, while preserving its existing expectations.

Source: Path instructions


it("keeps a host preflight failure ahead of markerless recovery (#7824)", async () => {
const deps = {
...snapshotDeps({
checked: true,
wasRunning: false,
recovered: false,
forwardRecovered: false,
}),
reconcile: () =>
Promise.resolve({
state: "present" as const,
output: "Phase: Ready",
}),
};

const snapshot = await collectSandboxStatusSnapshot("alpha", {
deps,
preflight: stoppedPreflight,
});

expect(deps.recoverSandboxProcesses).not.toHaveBeenCalled();
expect(snapshot.lookup.state).toBe("present");
});

it.each([
[
"inspection",
Expand Down
20 changes: 14 additions & 6 deletions src/lib/actions/sandbox/status-snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,13 +414,21 @@ export async function collectSandboxStatusSnapshot(
};
}
const dockerRecovered = lookup.recoveredSandbox === true;
if (lookup.state === "present" && lookup.recoveredSandbox) {
const managedOpenClawDeliveryMustBeProven =
lookup.state === "present" &&
sb?.openshellDriver === "docker" &&
(sb.agent ?? "openclaw") === "openclaw" &&
parseSandboxPhase(lookup.output || "") === "Ready" &&
!opts.preflight?.failure;
if (
lookup.state === "present" &&
(lookup.recoveredSandbox || managedOpenClawDeliveryMustBeProven)
) {
let failure: SandboxProcessRecoveryFailure | null;
try {
// Docker recovery makes the sandbox visible to OpenShell again, but a
// host reboot also tears down the managed agent process and port-forward.
// Reuse the guarded connect recovery only for this explicit mutation
// path, before status probes the delivery chain.
// The managed gateway service can restart a Docker sandbox before status
// runs. OpenShell then reports Ready without a recoveredSandbox marker,
// while the OpenClaw gateway and host forward can still be absent.
const recovery = (opts.deps?.recoverSandboxProcesses ?? loadRecoverSandboxProcesses())(
sandboxName,
{
Expand All @@ -439,7 +447,7 @@ export async function collectSandboxStatusSnapshot(
...lookup,
state: "sandbox_recovery_failed",
output:
` Docker restored sandbox '${sandboxName}', but its agent delivery chain is not ready ` +
` Sandbox '${sandboxName}' is present, but its agent delivery chain could not be proven ` +
`(${failure.layer}: ${failure.detail}).`,
};
}
Expand Down
4 changes: 3 additions & 1 deletion test/cli/sandbox-status-json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ function createInferenceRouteStatusSetup(options: {
writeSandboxRegistry(home, sandboxName, {
model: "nvidia/nemotron",
provider: "nvidia-prod",
openshellDriver: "docker",
// These cases test only inference.local classification. Use the VM driver
// so Docker post-reboot delivery recovery does not affect their assertions.
openshellDriver: "vm",
});
fs.writeFileSync(
path.join(localBin, "docker"),
Expand Down
Loading