Skip to content

fix(e2e): fix double-onboard assertion strings, CI timeout, and per-phase diagnostics #2658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cv merged 2 commits into from
Apr 29, 2026
Merged

fix(e2e): fix double-onboard assertion strings, CI timeout, and per-phase diagnostics #2658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00da573
fix(e2e): fix double-onboard assertion strings, CI timeout, and per-p…
jyaunches Apr 29, 2026
f70342d
fix(e2e): force registry reconciliation in Phase 7 cleanup
jyaunches Apr 29, 2026
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
2 changes: 1 addition & 1 deletion .github/workflows/nightly-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ jobs:
inputs.jobs == '' ||
contains(format(',{0},', inputs.jobs), ',double-onboard-e2e,'))
runs-on: ubuntu-latest
timeout-minutes: 60
timeout-minutes: 90
steps:
- name: Checkout
uses: actions/checkout@v6
Expand Down
59 changes: 56 additions & 3 deletions test/e2e/test-double-onboard.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,33 @@ section() {
}
info() { printf '\033[1;34m [info]\033[0m %s\n' "$1"; }

# TODO(#2562): replace shell timeout with structured timeout once unified abstraction lands

# Per-phase timeout in seconds (20 min per onboard phase, generous for CI)
PHASE_TIMEOUT="${NEMOCLAW_E2E_PHASE_TIMEOUT:-1200}"
Comment on lines +45 to +46

@coderabbitai coderabbitai Bot Apr 29, 2026

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.

⚠️ Potential issue | 🟠 Major

Per-phase 1200s timeout is effectively preempted by the 900s global script timeout.

e2e-timeout.sh is sourced at Line [21] and self-wraps the entire script using NEMOCLAW_E2E_DEFAULT_TIMEOUT (set to 900 at Line [18]). That means the script can be terminated around 15 minutes before a 1200s phase timeout path (and its diagnostics) is reached.

Suggested fix (align global timeout with phase budget) 
-export NEMOCLAW_E2E_DEFAULT_TIMEOUT=900
+export NEMOCLAW_E2E_DEFAULT_TIMEOUT="${NEMOCLAW_E2E_DEFAULT_TIMEOUT:-4500}"
@@
-# Per-phase timeout in seconds (20 min per onboard phase, generous for CI)
-PHASE_TIMEOUT="${NEMOCLAW_E2E_PHASE_TIMEOUT:-1200}"
+# Per-phase timeout in seconds (20 min per onboard phase, generous for CI)
+PHASE_TIMEOUT="${NEMOCLAW_E2E_PHASE_TIMEOUT:-1200}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Per-phase timeout in seconds (20 min per onboard phase, generous for CI)
PHASE_TIMEOUT="${NEMOCLAW_E2E_PHASE_TIMEOUT:-1200}"
export NEMOCLAW_E2E_DEFAULT_TIMEOUT="${NEMOCLAW_E2E_DEFAULT_TIMEOUT:-4500}"
# ...
# Per-phase timeout in seconds (20 min per onboard phase, generous for CI)
PHASE_TIMEOUT="${NEMOCLAW_E2E_PHASE_TIMEOUT:-1200}"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-double-onboard.sh` around lines 45 - 46, The per-phase
PHASE_TIMEOUT (variable PHASE_TIMEOUT set from NEMOCLAW_E2E_PHASE_TIMEOUT)
exceeds the global script wrapper timeout NEMOCLAW_E2E_DEFAULT_TIMEOUT applied
by sourcing e2e-timeout.sh, so align them: update the script to derive
NEMOCLAW_E2E_DEFAULT_TIMEOUT from the desired per-phase budget (or set
PHASE_TIMEOUT to not exceed NEMOCLAW_E2E_DEFAULT_TIMEOUT), i.e., ensure
NEMOCLAW_E2E_DEFAULT_TIMEOUT >= PHASE_TIMEOUT (or set
PHASE_TIMEOUT="${NEMOCLAW_E2E_PHASE_TIMEOUT:-${NEMOCLAW_E2E_DEFAULT_TIMEOUT}}")
so the global wrapper won't preempt per-phase timeouts; modify the variables
where they are defined/used (PHASE_TIMEOUT, NEMOCLAW_E2E_PHASE_TIMEOUT,
NEMOCLAW_E2E_DEFAULT_TIMEOUT and the e2e-timeout.sh sourcing) to enforce this
invariant.


# Elapsed-time helpers
phase_start_time() { date +%s; }
phase_elapsed() {
local start="$1"
local now
now="$(date +%s)"
echo $((now - start))
}

# Diagnostic dump — called on phase timeout or failure to aid debugging
dump_diagnostics() {
local phase_label="${1:-unknown}"
info "=== Diagnostics for ${phase_label} ==="
info "openshell status:"
openshell status 2>&1 | sed 's/^/ /' || true
info "openshell sandbox list:"
openshell sandbox list 2>&1 | sed 's/^/ /' || true
info "docker ps:"
docker ps 2>&1 | sed 's/^/ /' || true
info "=== End diagnostics ==="
}

registry_has() {
local sandbox_name="$1"
[ -f "$REGISTRY" ] && grep -q "$sandbox_name" "$REGISTRY"
Expand Down Expand Up @@ -141,6 +168,7 @@ PY
return 1
}

# TODO(#2562): replace shell timeout with structured timeout once unified abstraction lands
run_onboard() {
local sandbox_name="$1"
local recreate="${2:-0}"
Expand All @@ -161,7 +189,7 @@ run_onboard() {
env_args+=("NEMOCLAW_RECREATE_SANDBOX=1")
fi

env "${env_args[@]}" "${NEMOCLAW_CMD[@]}" onboard --non-interactive >"$log_file" 2>&1
run_with_timeout "$PHASE_TIMEOUT" env "${env_args[@]}" "${NEMOCLAW_CMD[@]}" onboard --non-interactive >"$log_file" 2>&1
RUN_ONBOARD_EXIT=$?
RUN_ONBOARD_OUTPUT="$(cat "$log_file")"
rm -f "$log_file"
Expand Down Expand Up @@ -234,14 +262,20 @@ fi
section "Phase 2: First onboard ($SANDBOX_A)"
info "Running successful non-interactive onboard against local compatible endpoint..."

PHASE2_START="$(phase_start_time)"
run_onboard "$SANDBOX_A"
output1="$RUN_ONBOARD_OUTPUT"
exit1="$RUN_ONBOARD_EXIT"
info "Phase 2 elapsed: $(phase_elapsed "$PHASE2_START")s"

if [ "$exit1" -eq 0 ]; then
pass "First onboard completed successfully"
elif [ "$exit1" -eq 124 ]; then
fail "First onboard timed out after ${PHASE_TIMEOUT}s (exit 124)"
dump_diagnostics "Phase 2"
else
fail "First onboard exited $exit1 (expected 0)"
dump_diagnostics "Phase 2"
fi

if grep -q "Sandbox '${SANDBOX_A}' created" <<<"$output1"; then
Expand Down Expand Up @@ -274,17 +308,23 @@ fi
section "Phase 3: Second onboard ($SANDBOX_A — same name, recreate)"
info "Running nemoclaw onboard with NEMOCLAW_RECREATE_SANDBOX=1..."

PHASE3_START="$(phase_start_time)"
run_onboard "$SANDBOX_A" "1"
output2="$RUN_ONBOARD_OUTPUT"
exit2="$RUN_ONBOARD_EXIT"
info "Phase 3 elapsed: $(phase_elapsed "$PHASE3_START")s"

if [ "$exit2" -eq 0 ]; then
pass "Second onboard completed successfully"
elif [ "$exit2" -eq 124 ]; then
fail "Second onboard timed out after ${PHASE_TIMEOUT}s (exit 124)"
dump_diagnostics "Phase 3"
else
fail "Second onboard exited $exit2 (expected 0)"
dump_diagnostics "Phase 3"
fi

if grep -q "Reusing existing NemoClaw gateway" <<<"$output2"; then
if grep -q "Reusing healthy NemoClaw gateway" <<<"$output2"; then
pass "Healthy gateway reused on second onboard"
else
fail "Healthy gateway was not reused on second onboard"
Expand Down Expand Up @@ -314,17 +354,23 @@ fi
section "Phase 4: Third onboard ($SANDBOX_B — different name)"
info "Running nemoclaw onboard with new sandbox name..."

PHASE4_START="$(phase_start_time)"
run_onboard "$SANDBOX_B"
output3="$RUN_ONBOARD_OUTPUT"
exit3="$RUN_ONBOARD_EXIT"
info "Phase 4 elapsed: $(phase_elapsed "$PHASE4_START")s"

if [ "$exit3" -eq 0 ]; then
pass "Third onboard completed successfully"
elif [ "$exit3" -eq 124 ]; then
fail "Third onboard timed out after ${PHASE_TIMEOUT}s (exit 124)"
dump_diagnostics "Phase 4"
else
fail "Third onboard exited $exit3 (expected 0)"
dump_diagnostics "Phase 4"
fi

if grep -q "Reusing existing NemoClaw gateway" <<<"$output3"; then
if grep -q "Reusing healthy NemoClaw gateway" <<<"$output3"; then
pass "Healthy gateway reused on third onboard"
else
fail "Healthy gateway was not reused on third onboard"
Expand Down Expand Up @@ -441,6 +487,13 @@ openshell sandbox delete "$SANDBOX_B" 2>/dev/null || true
openshell forward stop 18789 2>/dev/null || true
openshell gateway destroy -g nemoclaw 2>/dev/null || true

# Force registry reconciliation: when the gateway is in a degraded state
# (stopped in Phase 6), `nemoclaw destroy` may delete the sandbox from
# OpenShell but fail to clean its own registry entry. Running `status` for
# each sandbox triggers the stale-entry reconciliation path.
run_nemoclaw "$SANDBOX_A" status 2>/dev/null || true
run_nemoclaw "$SANDBOX_B" status 2>/dev/null || true

if openshell sandbox get "$SANDBOX_A" >/dev/null 2>&1; then
fail "Sandbox '$SANDBOX_A' still exists after cleanup"
else
Expand Down
Loading