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
110 changes: 106 additions & 4 deletions test/brev-launchable-e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ function fixture(
repoSha?: string;
runtimeOverrides?: boolean;
schemaVersion?: number;
sshReadyAfter?: number;
sourceRepository?: string;
sourcePath?: string;
timeoutBlockCommand?: "brev refresh" | "ssh -T";
} = {},
) {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-launchable-e2e-"));
Expand All @@ -46,10 +48,27 @@ function fixture(
const workDir = path.join(root, "evidence");
const state = path.join(root, "workspace.json");
const calls = path.join(root, "calls.log");
const sshAttempts = path.join(root, "ssh-attempts");
const timeoutBlock = path.join(root, "timeout-block");
fs.mkdirSync(bin);
fs.mkdirSync(workDir);
fs.writeFileSync(timeoutBlock, "block\n");

executable(path.join(bin, "timeout"), '#!/usr/bin/env bash\nshift\nexec "$@"\n');
executable(
path.join(bin, "timeout"),
`#!/usr/bin/env bash
set -euo pipefail
duration="$1"
shift
printf 'timeout %s %s\n' "$duration" "$*" >> "$FAKE_CALLS"
if [ -f "$FAKE_TIMEOUT_BLOCK" ] && [ "\${1:-} \${2:-}" = "$FAKE_TIMEOUT_BLOCK_COMMAND" ]; then
rm -f "$FAKE_TIMEOUT_BLOCK"
/bin/sleep "\${duration%s}"
exit 124
fi
exec "$@"
`,
);
executable(
path.join(bin, "sleep"),
'#!/usr/bin/env bash\nprintf "sleep %s\\n" "$*" >> "$FAKE_CALLS"\n',
Expand Down Expand Up @@ -131,6 +150,20 @@ esac
path.join(bin, "ssh"),
`#!/usr/bin/env bash
set -euo pipefail
if [ "\${*: -1}" = true ]; then
required=(-T "-o BatchMode=yes" "-o ConnectTimeout=10" "-o ConnectionAttempts=1" "-o NumberOfPasswordPrompts=0" "-o RequestTTY=no" "-o LogLevel=ERROR")
for argument in "\${required[@]}"; do
[[ " $* " == *" $argument "* ]]
done
[ "\${*: -2:1}" = "$INSTANCE_NAME-host" ]
Comment on lines +154 to +158

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

Make the SSH argument check exact.

The fixture checks required tokens in the joined argument string. The assertion uses expect.arrayContaining. These checks allow extra positional arguments, duplicate or conflicting -o values, and extra security-affecting options. An incorrect readiness command can therefore pass the test.

Parse the raw SSH argument vector. Compare the complete normalized option set. Reject duplicate option keys and require the exact <target> true tail.

Based on the PR objective, this test must validate the exact noninteractive SSH target and options.

Also applies to: 240-256

🤖 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 `@test/brev-launchable-e2e.test.ts` around lines 154 - 158, Update the SSH
argument validation in the fixture test to parse the raw argument vector rather
than matching tokens in the joined string or using expect.arrayContaining.
Normalize the SSH options, reject duplicate or conflicting option keys, compare
the complete option set exactly against required, and require the final
arguments to be precisely <target> true.

attempts=0
[ ! -f "$FAKE_SSH_ATTEMPTS" ] || attempts="$(cat "$FAKE_SSH_ATTEMPTS")"
attempts=$((attempts + 1))
printf '%s\n' "$attempts" > "$FAKE_SSH_ATTEMPTS"
printf 'ssh host readiness attempt %s: %s\n' "$attempts" "$*" >> "$FAKE_CALLS"
[ "$attempts" -ge "$FAKE_SSH_READY_AFTER" ]
exit
fi
script="$(cat)"
grep -q 'NEMOCLAW_E2E_SETUP_MODE=preinstalled-launchable' <<<"$script"
grep -q 'NEMOCLAW_SOURCE_PATH=/opt/nemoclaw-image/NemoClaw' <<<"$script"
Expand Down Expand Up @@ -166,9 +199,15 @@ printf 'NEMOCLAW_FULL_E2E_PASSED\\n'
FAKE_REPO_SHA: options.repoSha ?? candidateSha,
FAKE_RUNTIME_OVERRIDES: options.runtimeOverrides ? "true" : "false",
FAKE_SCHEMA_VERSION: String(options.schemaVersion ?? 1),
FAKE_SSH_ATTEMPTS: sshAttempts,
FAKE_SSH_READY_AFTER: String(options.sshReadyAfter ?? 1),
FAKE_SOURCE_REPOSITORY: options.sourceRepository ?? "NVIDIA/NemoClaw",
FAKE_SOURCE_PATH: options.sourcePath ?? "/opt/nemoclaw-image/NemoClaw",
FAKE_STATE: state,
FAKE_TIMEOUT_BLOCK: options.timeoutBlockCommand
? timeoutBlock
: path.join(root, "timeout-disabled"),
FAKE_TIMEOUT_BLOCK_COMMAND: options.timeoutBlockCommand ?? "",
GH_TOKEN: "github-test-token",
GITHUB_RUN_ATTEMPT: "1",
GITHUB_RUN_ID: "789",
Expand All @@ -178,7 +217,7 @@ printf 'NEMOCLAW_FULL_E2E_PASSED\\n'
RUNNER_TEMP: root,
WORK_DIR: workDir,
};
return { calls, env, state, workDir };
return { calls, env, sshAttempts, state, workDir };
}

function run(env: NodeJS.ProcessEnv) {
Expand All @@ -187,7 +226,7 @@ function run(env: NodeJS.ProcessEnv) {

describe("focused staging Brev Launchable lane", () => {
it("binds the producer run, verifies the clean booted SHA, runs E2E, and deletes (#6943)", () => {
const { calls, env, state, workDir } = fixture();
const { calls, env, sshAttempts, state, workDir } = fixture({ sshReadyAfter: 3 });
const result = run(env);
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
const commands = fs.readFileSync(calls, "utf8");
Expand All @@ -197,6 +236,25 @@ describe("focused staging Brev Launchable lane", () => {
commands.indexOf("create nclaw-e2e-test-1 --launchable env-staging123"),
);
expect(commands).toContain("create nclaw-e2e-test-1 --launchable env-staging123");
expect(commands.match(/ssh host readiness attempt/gu)).toHaveLength(3);
const readinessCall = commands
.split("\n")
.find((line) => line.startsWith("ssh host readiness attempt 1: "));
expect(readinessCall).toBeDefined();
const readinessArgs = readinessCall?.split(": ").at(1)?.split(" ") ?? [];
expect(readinessArgs).toEqual(
expect.arrayContaining([
"-T",
"BatchMode=yes",
"ConnectTimeout=10",
"ConnectionAttempts=1",
"NumberOfPasswordPrompts=0",
"RequestTTY=no",
"LogLevel=ERROR",
]),
);
expect(readinessArgs.slice(-2)).toEqual(["nclaw-e2e-test-1-host", "true"]);
expect(fs.readFileSync(sshAttempts, "utf8").trim()).toBe("3");
expect(commands).toContain("ssh preinstalled full-e2e.test.ts");
expect(commands).not.toContain("nvapi-test-value");
expect(commands).not.toMatch(/rsync|install\.sh|npm (?:ci|install)|git clone/u);
Expand Down Expand Up @@ -281,7 +339,7 @@ describe("focused staging Brev Launchable lane", () => {
expect(fs.readFileSync(boot.calls, "utf8")).not.toContain("full-e2e.test.ts");
expect(fs.existsSync(boot.state)).toBe(false);
}
});
}, 90_000);

it("reports E2E failure only after verified workspace cleanup", () => {
const { env, state, workDir } = fixture({ e2eFails: true });
Expand All @@ -294,6 +352,50 @@ describe("focused staging Brev Launchable lane", () => {
});
});

it("fails after host SSH readiness times out and deletes the workspace", () => {
const { calls, env, state, workDir } = fixture({ sshReadyAfter: Number.MAX_SAFE_INTEGER });
const result = run({ ...env, BREV_HOST_SSH_TIMEOUT_SECONDS: "1" });
expect(result.status).not.toBe(0);
expect(result.stderr).toContain("host SSH readiness timed out");
expect(fs.readFileSync(calls, "utf8")).not.toMatch(/brev exec|full-e2e\.test\.ts/u);
expect(fs.existsSync(state)).toBe(false);
expect(JSON.parse(fs.readFileSync(path.join(workDir, "cleanup.json"), "utf8"))).toMatchObject({
status: "ABSENT",
});
});

it("caps a blocking refresh by the host SSH deadline and deletes the workspace", () => {
const { calls, env, state, workDir } = fixture({ timeoutBlockCommand: "brev refresh" });
const startedAt = performance.now();
const result = run({ ...env, BREV_HOST_SSH_TIMEOUT_SECONDS: "1" });
const elapsedMs = performance.now() - startedAt;
expect(result.status).not.toBe(0);
expect(result.stderr).toContain("host SSH readiness timed out");
expect(elapsedMs).toBeLessThan(10_000);
expect(fs.readFileSync(calls, "utf8")).toContain("timeout 1s brev refresh");
expect(fs.readFileSync(calls, "utf8")).not.toMatch(/brev exec|full-e2e\.test\.ts/u);
expect(fs.existsSync(state)).toBe(false);
expect(JSON.parse(fs.readFileSync(path.join(workDir, "cleanup.json"), "utf8"))).toMatchObject({
status: "ABSENT",
});
}, 90_000);

it("caps a blocking SSH probe by the host SSH deadline and deletes the workspace", () => {
const { calls, env, state, workDir } = fixture({ timeoutBlockCommand: "ssh -T" });
const startedAt = performance.now();
const result = run({ ...env, BREV_HOST_SSH_TIMEOUT_SECONDS: "2" });
const elapsedMs = performance.now() - startedAt;
expect(result.status).not.toBe(0);
expect(result.stderr).toContain("host SSH readiness timed out");
expect(elapsedMs).toBeLessThan(10_000);
expect(fs.readFileSync(calls, "utf8")).toContain("timeout 2s ssh -T");
expect(fs.readFileSync(calls, "utf8")).not.toMatch(/brev exec|full-e2e\.test\.ts/u);
expect(fs.existsSync(state)).toBe(false);
expect(JSON.parse(fs.readFileSync(path.join(workDir, "cleanup.json"), "utf8"))).toMatchObject({
status: "ABSENT",
});
}, 90_000);

it("preserves the booted image when the provision receipt is missing", () => {
const { calls, env, state, workDir } = fixture({ missingProvisionReceipt: true });
const result = run(env);
Expand Down
29 changes: 29 additions & 0 deletions tools/e2e/brev-launchable-e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,34 @@ workspace() {
else error("workspace name is ambiguous") end'
}

wait_for_host_ssh() {
local timeout_seconds="${BREV_HOST_SSH_TIMEOUT_SECONDS:-600}"
local deadline=$((SECONDS + timeout_seconds))
local remaining refresh_timeout sleep_seconds ssh_timeout
log "Waiting for host SSH access"
while [ "$SECONDS" -lt "$deadline" ]; do
remaining=$((deadline - SECONDS))
[ "$remaining" -gt 0 ] || break
refresh_timeout=$((remaining < 60 ? remaining : 60))
timeout "${refresh_timeout}s" brev refresh >/dev/null 2>&1 || true
remaining=$((deadline - SECONDS))
[ "$remaining" -gt 0 ] || break
ssh_timeout=$((remaining < 15 ? remaining : 15))
if timeout "${ssh_timeout}s" ssh -T -o BatchMode=yes -o ConnectTimeout=10 \
-o ConnectionAttempts=1 -o NumberOfPasswordPrompts=0 \
-o RequestTTY=no -o LogLevel=ERROR "${INSTANCE_NAME}-host" true \
>/dev/null 2>&1; then
log "SSH access to ${INSTANCE_NAME}-host succeeded"
return 0
fi
remaining=$((deadline - SECONDS))
[ "$remaining" -gt 0 ] || break
sleep_seconds="${POLL_SECONDS:-15}"
sleep "$((sleep_seconds < remaining ? sleep_seconds : remaining))"
done
die "host SSH readiness timed out"
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

cleanup() {
local record deadline absent=0 workspace_id=""
record="$(workspace || true)"
Expand Down Expand Up @@ -176,6 +204,7 @@ jq -e '.status == "RUNNING" and (.shell_status // .shellStatus) == "READY" and
<<<"${ready:-null}" >/dev/null || die "workspace readiness timed out"
workspace_id="$(jq -r '.id // ""' <<<"$ready")"
log "Workspace $INSTANCE_NAME ($workspace_id) is ready"
wait_for_host_ssh

# Record the booted image before reading the baked runtime receipt so a stale
# Launchable image remains visible when the receipt is absent.
Expand Down
Loading