Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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: 3 additions & 1 deletion scripts/install-openshell.sh
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,9 @@ download_with_curl() {
curl_progress=(-sS)
fi
for name in "${ASSETS[@]}" "${CHECKSUM_FILES[@]}"; do
curl -fL "${curl_progress[@]}" "https://github.com/NVIDIA/OpenShell/releases/download/${RELEASE_TAG}/$name" \
curl -fL "${curl_progress[@]}" \
--retry 3 --retry-delay 1 --retry-all-errors \
"https://github.com/NVIDIA/OpenShell/releases/download/${RELEASE_TAG}/$name" \

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.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n '(^FROM |curl|retry-all-errors)' \
  --glob 'Dockerfile*' \
  --glob '*.yml' \
  --glob '*.yaml' \
  --glob '*.sh' .

curl --version | head -n 1

Repository: NVIDIA/NemoClaw

Length of output: 29840


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- installer metadata and header ---'
git ls-files -s scripts/install-openshell.sh
sed -n '1,30p' scripts/install-openshell.sh

printf '%s\n' '--- relevant installer context ---'
sed -n '600,660p' scripts/install-openshell.sh
sed -n '850,930p' scripts/install-openshell.sh

printf '%s\n' '--- curl requirements and installer environment checks ---'
rg -n -i 'curl|supported|minimum|version|macos|darwin|ubuntu|debian|fedora|brew|apt|dnf|apk' \
  scripts/install-openshell.sh README.md docs .github 2>/dev/null | head -n 250

printf '%s\n' '--- curl option help ---'
curl --help all 2>/dev/null | rg -n -- '--retry|retry-all-errors|retry-delay|fail' || true

Repository: NVIDIA/NemoClaw

Length of output: 36640


🌐 Web query:

Official curl documentation for --retry, --retry-all-errors, --fail, and the minimum curl version for --retry-all-errors

💡 Result:

The official documentation for curl provides several options for managing transfer failures and retries [1][2]. The following details summarize these options and their behavior: --retry Normally, curl performs a single attempt and returns an error if it fails. This option specifies the number of times curl should retry a transfer if a transient error occurs [1]. Transient errors include timeouts, FTP 4xx response codes, and HTTP 408, 429, 500, 502, 503, or 504 response codes [1]. By default, curl does not retry (0 attempts) [1]. --retry-all-errors This option, introduced in curl 7.71.0, is a more aggressive form of retrying that tells curl to treat all transfer failures as a reason to retry, rather than just transient errors [1][3][4]. It must be used in conjunction with --retry [3][5]. It is considered a sledgehammer approach and users are warned that it may cause unintended consequences, such as receiving duplicate data, especially when used with redirected output [3][5]. --fail (HTTP) This option tells curl to fail silently with an error code (22) and no response body when an HTTP server returns an error code of 400 or greater [2][6]. It is useful for detecting server errors that might otherwise be ignored if the transfer itself completes [3][5]. It can be combined with --retry and --retry-all-errors to ensure that HTTP errors (4xx and 5xx) trigger a retry [3][5]. Minimum Version for --retry-all-errors The --retry-all-errors option was added in curl version 7.71.0 [3][4][7].

Citations:


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path("scripts/install-openshell.sh")
text = p.read_text()
lines = text.splitlines()
for i in range(880, 898):
    print(f"{i+1}: {lines[i]}")
print("has_shebang:", text.startswith("#!"))
print("has_spdx:", "SPDX-License-Identifier:" in "\n".join(lines[:40]))
PY

Repository: NVIDIA/NemoClaw

Length of output: 866


Align the retry policy with the stated contract.

--retry 3 permits three retries after the initial transfer, so each asset can receive four attempts. With -f, --retry-all-errors also retries HTTP failures such as missing release assets.

If three total attempts are required, use --retry 2 and remove --retry-all-errors. Retain --retry-all-errors only when supported installer hosts provide curl 7.71.0 or newer.

🤖 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 `@scripts/install-openshell.sh` around lines 892 - 894, Update the curl
invocation in the asset download flow to allow exactly three total attempts by
changing the retry count to two and removing --retry-all-errors. Keep the
existing failure handling and progress options unchanged.

-o "$tmpdir/$name"
done
}
Expand Down
39 changes: 32 additions & 7 deletions test/e2e/fixtures/phases/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const STATUS_TIMEOUT_MS = 5 * 60_000;
const REBUILD_TIMEOUT_MS = 20 * 60_000;
const SANDBOX_READY_ATTEMPTS = 30;
const SANDBOX_READY_DELAY_MS = 5_000;
const STOPPED_SANDBOX_STATUS = "Failure layer: sandbox_container_stopped";
const USER_SERVICE_UNAVAILABLE_EXIT = 75;
const NEMOCLAW_OPENSHELL_GATEWAY_USER_SERVICE_MARKER_LINE =
"# NEMOCLAW_MANAGED_OPENSHELL_GATEWAY=1";
Expand Down Expand Up @@ -442,13 +443,10 @@ export class LifecyclePhaseFixture {
// We invoke status through the host CLI client so artifacts are
// captured and the command goes through the same
// shellProbe/redaction layer the rest of the fixture code uses.
// Status must exit zero to prove the restored delivery path is ready;
// state-validation still verifies registry and container preservation.
const statusResult = await this.host.expectStatus(instance.sandboxName, {
artifactName: `lifecycle-post-reboot-nemoclaw-status-${instance.sandboxName}`,
env: buildAvailabilityProbeEnv(),
timeoutMs: STATUS_TIMEOUT_MS,
});
// OpenShell can report the gateway as connected before it starts the
// preserved sandbox container. Retry only that observed transition.
// Other status failures remain terminal.
const statusResult = await this.waitForPostRebootSandboxStatus(instance.sandboxName);
steps.push({
id: `nemoclaw-status:${instance.sandboxName}`,
results: [statusResult],
Expand All @@ -457,6 +455,33 @@ export class LifecyclePhaseFixture {
return { profile: "post-reboot-recovery", steps };
}

private async waitForPostRebootSandboxStatus(sandboxName: string): Promise<ShellProbeResult> {
let last: ShellProbeResult | undefined;
const deadlineMs = Date.now() + STATUS_TIMEOUT_MS;
for (let attempt = 1; attempt <= SANDBOX_READY_ATTEMPTS; attempt += 1) {
const remainingMs = deadlineMs - Date.now();
if (remainingMs <= 0) break;
last = await this.host.nemoclaw([sandboxName, "status"], {
artifactName:
`lifecycle-post-reboot-nemoclaw-status-${sandboxName}-` +
`attempt-${String(attempt).padStart(2, "0")}`,
env: buildAvailabilityProbeEnv(),
timeoutMs: Math.min(60_000, remainingMs),
});
if (last.exitCode === 0) return last;
const detail = `${last.stdout}\n${last.stderr}`;
if (!detail.includes(STOPPED_SANDBOX_STATUS)) {
assertExitZero(last, `nemoclaw ${sandboxName} status`);
}
if (attempt < SANDBOX_READY_ATTEMPTS && Date.now() < deadlineMs) {
await sleep(Math.min(SANDBOX_READY_DELAY_MS, deadlineMs - Date.now()));
}
}
if (!last) throw new Error(`nemoclaw ${sandboxName} status did not run before its deadline`);
assertExitZero(last, `nemoclaw ${sandboxName} status`);
return last;
}

private async ensureOpenShellGatewayUserService(): Promise<UserServiceStageResult> {
const result = await this.host.command(
"bash",
Expand Down
60 changes: 31 additions & 29 deletions test/e2e/live/gateway-guard-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
* - Deliberately out of scope for this merge gate: physical DGX Spark /
* GB10 / aarch64 hardware, provider breadth beyond `cloud-openclaw`, and
* destructive host reboot / OOM / manual `kubectl delete pod` triggers.
* The Docker-driver branch below does restart the registered sandbox
* container, then proves the legacy keepalive migration restores the
* The Docker-driver branch below restarts the registered sandbox
* container, then proves that its persisted startup command restores the
* managed supervisor topology without relying on ordinary sandbox exec.
* Kubernetes triggers still need a dedicated platform-runtime job.
*
Expand Down Expand Up @@ -147,7 +147,7 @@ test("gateway recovery restores /tmp guard chain after pod-recreate wipe (#2701)
"wipe guard chain and gateway tree",
"recover gateway through connect probe",
"validate recovered guard and stable PID",
"restart legacy Docker sandbox",
"restart Docker sandbox with persisted startup command",
"recover managed supervisor and inference",
],
},
Expand All @@ -167,14 +167,14 @@ test("gateway recovery restores /tmp guard chain after pod-recreate wipe (#2701)
await artifacts.target.declare({
id: "gateway-guard-recovery",
boundary: "sandbox-lifecycle",
issues: ["#2701", "#2478", "#6635"],
issues: ["#2701", "#2478"],
acceptanceCoverage: {
covered: [
"production connect --probe-only recovery route",
"authenticated PID 1 OpenClaw recovery supervisor",
"pod-recreate-equivalent empty /tmp guard chain plus missing gateway process",
"Docker container restart with a legacy keepalive startup",
"container-identity-pinned supervisor recreation with managed health proof",
"Docker container restart with a persisted managed startup command",
"container identity preservation with managed supervisor health proof",
"no rebuild required for the recovered runtime state",
],
intentionallyOutOfScope: [
Expand Down Expand Up @@ -282,32 +282,32 @@ test("gateway recovery restores /tmp guard chain after pod-recreate wipe (#2701)

expect(stablePid).toBeGreaterThan(0);

progress.phase("restart legacy Docker sandbox");
// ── Assert #6635 legacy Docker restart recovery ────────────────
// Fresh non-GPU OpenClaw containers on this OpenShell floor still carry the
// legacy keepalive. Restarting the container therefore kills the initial
// OpenShell workload session and deterministically leaves no managed
// supervisor. Recovery must upgrade that container through the host-side
// transaction and commit only after managed control accepts the new tree.
const originalContainerId = await findSandboxContainer(host, "legacy-restart-container-before");
expect(
await inspectStartupCommand(host, originalContainerId, "legacy-restart-command-before"),
).toBe("sleep infinity");
progress.phase("restart Docker sandbox with persisted startup command");
// A Docker restart must reuse the container and its credential-free managed
// startup command. The command must restore the supervisor without a
// container recreation transaction.
const originalContainerId = await findSandboxContainer(host, "restart-container-before");
const originalStartupCommand = await inspectStartupCommand(
host,
originalContainerId,
"restart-command-before",
);
expect(originalStartupCommand).toMatch(/(?:^| )nemoclaw-start$/);
await host.cleanupForward(18789, {
artifactName: "legacy-restart-stop-dashboard-forward",
artifactName: "restart-stop-dashboard-forward",
env: buildAvailabilityProbeEnv(),
});
const restart = await host.command("docker", ["restart", originalContainerId], {
artifactName: "legacy-restart-docker-restart",
artifactName: "restart-docker-restart",
env: buildAvailabilityProbeEnv(),
timeoutMs: 120_000,
});
expect(restart.exitCode, resultText(restart)).toBe(0);

progress.phase("recover managed supervisor and inference");
const credentialCanary = "nemoclaw-e2e-recovery-secret-6635";
const credentialCanary = "nemoclaw-e2e-recovery-secret-restart";
const trustedRecovery = await host.nemoclaw([instance.sandboxName, "recover"], {
artifactName: "legacy-restart-trusted-recover",
artifactName: "restart-trusted-recover",
env: {
...buildAvailabilityProbeEnv(),
NEMOCLAW_EXTRA_PLACEHOLDER_KEYS: "CUSTOM_PROVIDER_CREDENTIAL",
Expand All @@ -318,14 +318,16 @@ test("gateway recovery restores /tmp guard chain after pod-recreate wipe (#2701)
});
expect(trustedRecovery.timedOut, resultText(trustedRecovery)).toBe(false);
expect(trustedRecovery.exitCode, resultText(trustedRecovery)).toBe(0);
expect(resultText(trustedRecovery)).toContain("Probe complete: recovered OpenClaw gateway");
expect(resultText(trustedRecovery)).toMatch(
/Probe complete: (?:recovered OpenClaw gateway|OpenClaw gateway is running)/,
);

const recoveredContainerId = await findSandboxContainer(host, "legacy-restart-container-after");
expect(recoveredContainerId).not.toBe(originalContainerId);
const recoveredContainerId = await findSandboxContainer(host, "restart-container-after");
expect(recoveredContainerId).toBe(originalContainerId);
const recoveredStartupCommand = await inspectStartupCommand(
host,
recoveredContainerId,
"legacy-restart-command-after",
"restart-command-after",
);
expect(recoveredStartupCommand).toMatch(/(?:^| )nemoclaw-start$/);
expect(recoveredStartupCommand).not.toContain("CUSTOM_PROVIDER_CREDENTIAL");
Expand All @@ -335,7 +337,7 @@ test("gateway recovery restores /tmp guard chain after pod-recreate wipe (#2701)
instance.sandboxName,
["python3", "-c", SUPERVISOR_TOPOLOGY_SCRIPT],
{
artifactName: "legacy-restart-managed-supervisor-topology",
artifactName: "restart-managed-supervisor-topology",
env: buildAvailabilityProbeEnv(),
},
);
Expand All @@ -346,7 +348,7 @@ test("gateway recovery restores /tmp guard chain after pod-recreate wipe (#2701)
"curl",
["-sS", "-o", "/dev/null", "-w", "%{http_code}", "http://127.0.0.1:18789/health"],
{
artifactName: "legacy-restart-forwarded-health",
artifactName: "restart-forwarded-health",
env: buildAvailabilityProbeEnv(),
timeoutMs: 30_000,
},
Expand All @@ -362,12 +364,12 @@ test("gateway recovery restores /tmp guard chain after pod-recreate wipe (#2701)
"main",
"--json",
"--session-id",
`e2e-6635-${Date.now()}-${process.pid}`,
`e2e-restart-${Date.now()}-${process.pid}`,
"-m",
"What is 6 multiplied by 7? Reply with only the integer, no extra words.",
],
{
artifactName: "legacy-restart-agent-inference",
artifactName: "restart-agent-inference",
env: buildAvailabilityProbeEnv(),
timeoutMs: 120_000,
},
Expand Down
41 changes: 40 additions & 1 deletion test/e2e/support/e2e-phase-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import fs from "node:fs";
import os from "node:os";
import path from "node:path";

import { describe, expect, expectTypeOf, it } from "vitest";
import { afterEach, describe, expect, expectTypeOf, it, vi } from "vitest";

import {
type CommandRunner,
Expand Down Expand Up @@ -126,6 +126,10 @@ function restoreEnv(name: string, value: string | undefined): void {
Object.assign(process.env, value === undefined ? {} : { [name]: value });
}

afterEach(() => {
vi.useRealTimers();
});

describe("LifecyclePhaseFixture.preparePostReboot", () => {
it("installs OpenShell and stages the gateway user service when openshell-gateway is unavailable", async () => {
const runner = new FakeRunner();
Expand Down Expand Up @@ -217,6 +221,41 @@ describe("LifecyclePhaseFixture.simulate post-reboot-recovery (stop-original)",
);
});

it("retries the stopped-container status until the sandbox starts", async () => {
vi.useFakeTimers();
const runner = new FakeRunner();
const cleanup = new FakeCleanup();
const prepared = await preparedPostRebootFixture(runner, cleanup);
runner.enqueue(shellResult(0, "container-1\n")); // discover
runner.enqueue(shellResult(0)); // docker stop
runner.enqueue(shellResult(0)); // forward stop
runner.enqueue(shellResult(0)); // gateway stop
runner.enqueue(shellResult(0)); // pid stop
runner.enqueue(shellResult(0)); // container stop
runner.enqueue(shellResult(0)); // user service restart
runner.enqueue(shellResult(0, "Connected to nemoclaw\n")); // openshell status
runner.enqueue(
shellResult(
1,
"Failure layer: sandbox_container_stopped — sandbox container exists but is not running.",
),
);
runner.enqueue(shellResult(0)); // status after OpenShell starts the container

const simulation = prepared.simulate("post-reboot-recovery", instance());
await vi.advanceTimersByTimeAsync(5_000);
const result = await simulation;

expect(result.steps.at(-1)?.results[0]?.exitCode).toBe(0);
expect(
runner.calls.filter(
(call) =>
call.command === "nemoclaw" &&
call.args.join(" ") === "e2e-ubuntu-repo-cloud-openclaw status",
),
).toHaveLength(2);
});

it("fails when no Docker container carries the OpenShell sandbox-name label", async () => {
const runner = new FakeRunner();
const cleanup = new FakeCleanup();
Expand Down
2 changes: 2 additions & 0 deletions test/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,7 @@ describe("regression guards", () => {
const out = (result.stdout || "") + (result.stderr || "");
expect(result.status, out).toBe(0);
expect(out).toContain("CURL_DIRECT");
expect(out).toContain("--retry 3 --retry-delay 1 --retry-all-errors");
expect(out).not.toContain("gh CLI download failed");
} finally {
fs.rmSync(tmpBin, { recursive: true, force: true });
Expand Down Expand Up @@ -884,6 +885,7 @@ describe("regression guards", () => {
const out = (result.stdout || "") + (result.stderr || "");
expect(out).toContain("falling back to curl");
expect(out).toContain("CURL_FALLBACK");
expect(out).toContain("--retry 3 --retry-delay 1 --retry-all-errors");
expect(fs.readFileSync(checksumLog, "utf-8")).toContain("SHA256SUM -c -");
} finally {
fs.rmSync(tmpBin, { recursive: true, force: true });
Expand Down
Loading