test(e2e): raise full-e2e onboard budget to 205s for cold-build variance (#6002)#6663
Conversation
…nce (#6002) The [1/8]-to-first-response gate in full-e2e failed 3x consecutively on main (180.8s, 181.5s, 180.6s vs the 180s cap). Artifact decomposition (onboard-progress-budget.json) shows the overshoot is entirely in the cold onboard/BuildKit image-build phase, not inference: onboardSecs ranged 163s (passing run f4cd7ea) to 173s (failing run fcc121d) on post-#6265 main, a ~10s run-to-run swing driven by Docker Hub pull speed and runner I/O. The first hosted agent turn itself is only ~5-8s. The 180s cap left ~7s of headroom, so slow-build runs tipped over. Raise to 205s to absorb build variance while still catching gross onboard regressions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe cold onboarding end-to-end test increases its acceptance budget from 180 to 205 seconds, documents BuildKit-related timing variance, and adds live-to-fast mock parity coverage. ChangesCold onboarding timing
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 77%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@test/e2e/live/full-e2e.test.ts`:
- Around line 36-44: Update test/e2e/README.md and related CI configuration or
documentation references for the full-e2e cold-onboard acceptance limit from 180
seconds to 205 seconds, matching the ONBOARD_BUDGET_SECS constant in
full-e2e.test.ts. Search for all references to the 180-second full-e2e limit and
synchronize them with the new documented contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4ba58b5d-d361-461c-99a6-ecdef05ef022
📒 Files selected for processing (1)
test/e2e/live/full-e2e.test.ts
| // Cold-path acceptance budget for install + BuildKit image build + gateway | ||
| // health + first hosted agent turn (#6002, #6265). The dominant term is the | ||
| // cold BuildKit image build, which swings run-to-run with Docker Hub pull | ||
| // speed and hosted-runner I/O. Observed post-#6265 onboard phase alone ranged | ||
| // 163s (pass) to 173s (fail) on identical `main` commits — a ~10s swing — so | ||
| // the original 180s cap left only ~7s of headroom and tipped over on slow | ||
| // builds. Raised to 205s to absorb build-phase variance while still catching | ||
| // gross onboard regressions (first hosted turn itself is only ~5-8s). | ||
| const ONBOARD_BUDGET_SECS = 205; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Synchronize the documented cold-onboard contract.
test/e2e/README.md still describes the hard full-e2e acceptance limit as 180 seconds, while this test now enforces and reports 205 seconds. Update the documentation and any related CI references in this change so operators do not receive conflicting limits.
🤖 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/e2e/live/full-e2e.test.ts` around lines 36 - 44, Update
test/e2e/README.md and related CI configuration or documentation references for
the full-e2e cold-onboard acceptance limit from 180 seconds to 205 seconds,
matching the ONBOARD_BUDGET_SECS constant in full-e2e.test.ts. Search for all
references to the 180-second full-e2e limit and synchronize them with the new
documented contract.
Source: Path instructions
PR Review Advisor (Nemotron Ultra) — No blocking findingsMerge posture: No blocking advisor findings This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Changing test/e2e/live/full-e2e.test.ts trips the e2e-mock-parity gate (scripts/checks/e2e-mock-parity.ts), which requires every touched live test to declare its fast coverage. Map full-e2e to its fast twin test/e2e/support/onboard-performance.test.ts, which unit-tests the same measurement helpers (maximumOutputSilenceMs, readOnboardTraceWindow, extractOpenClawAgentPayloadText) the live test relies on; the cold-build wall-clock budget itself remains inherently live. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
…nce (NVIDIA#6002) (NVIDIA#6663) ## Summary Raise the `full-e2e` cold-onboard acceptance budget (`ONBOARD_BUDGET_SECS`) from **180s → 205s**. This is the umbrella PR for today's live-E2E failures on `main`; scope and evidence per job below. ## Failure triage (main, 2026-07-10) A full `E2E` dispatch on `main` (run [29124128082](https://github.com/NVIDIA/NemoClaw/actions/runs/29124128082)) came back **78 passed / 5 skipped / 3 failed**. Each failure was root-caused, not retried blindly: | Job | Verdict | Root cause | |-----|---------|-----------| | `agent-turn-latency` | ✅ flake, self-cleared | Passed on first retry — hosted-inference timing variance. | | `full-e2e` | 🔧 **fixed here** | Consistent ~1s overshoot of a too-tight 180s onboard budget (see below). | | `rebuild-hermes` | ⏳ **verification pending** | Both observed failures were infra (`operation was canceled`, `runner lost communication with the server`) — **not** a test assertion or image-build error. Retry on post-bump `main` in flight ([29129133666](https://github.com/NVIDIA/NemoClaw/actions/runs/29129133666)). See "Hermes v0.18" below. | ## `full-e2e` — full analysis (no gaps) **Symptom.** The `[1/8]-to-first-response` gate (`full-e2e.test.ts:215`) failed **3 consecutive times**: | Run | to-first-response | vs 180s budget | |-----|-------------------|----------------| | [29124128082](https://github.com/NVIDIA/NemoClaw/actions/runs/29124128082) | 180,829 ms | +0.8s | | [29125880976](https://github.com/NVIDIA/NemoClaw/actions/runs/29125880976) | 181,550 ms | +1.5s | | [29127670707](https://github.com/NVIDIA/NemoClaw/actions/runs/29127670707) | 180,602 ms | +0.6s | **Not flake, not inference, not a code regression** — proven by the `onboard-progress-budget.json` artifact decomposition: | | `onboardSecs` | `totalSecs` | headroom | notes | |---|---|---|---|---| | Passing run [29128496025](https://github.com/NVIDIA/NemoClaw/actions/runs/29128496025) (`f4cd7ea9`) | **163** | 168 | +12s | BuildKit prebuild ✓, 0 classic steps | | Failing run [29127670707](https://github.com/NVIDIA/NemoClaw/actions/runs/29127670707) (`fcc121d5`) | **173** | 181 | −1s | BuildKit prebuild ✓, 0 classic steps | - The entire delta is in the **cold onboard/BuildKit image-build phase** (163s → 173s, a ~10s run-to-run swing) on **identical, post-NVIDIA#6265 `main`** — both heads are after the Hermes v0.18 bump, so the bump is not the cause. - The first hosted agent turn is only **~5–8s** (`totalSecs − onboardSecs`); inference is a rounding error. - The 180s cap (introduced 4 days ago in NVIDIA#6265) left only ~7s of headroom against a phase that varies ~10s with Docker Hub pull speed and hosted-runner I/O — so slow-build runs tip over. **Fix.** Raise to **205s**: covers the observed 173s worst case plus build-variance headroom, while still catching gross onboard regressions (a real regression blows well past 205s; `MAX_SILENCE_SECS` and BuildKit-fallback assertions are unchanged). ## Hermes v0.18 context Today's `main` includes `feat(hermes): upgrade to v0.18 and enable Slack Block Kit` (NVIDIA#6507, 17:45Z), plus `NVIDIA#6624` (release-matched sandbox bases) and `NVIDIA#6623` (surface cluster image build failures). Because `rebuild-hermes` rebuilds the Hermes image, a v0.18-induced regression *could* in principle surface as runner resource-exhaustion. **This PR does not yet claim `rebuild-hermes` is a flake** — the in-flight retry on post-bump `main` is the deciding evidence: - retry **passes** → confirmed infra flake, no code change needed, this PR ships as-is; - retry **fails** (build error / OOM / repeat comms-loss) → v0.18 is implicated and a fix is added to this branch before merge. ## Test evidence - `commitlint`, `gitleaks`, test-size/shape budgets: passed (pre-commit). - No product-code change; single test-constant edit. Behavioral proof is the artifact decomposition above. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Adjusted end-to-end onboarding timing thresholds by increasing the “acceptance budget” to allow for typical variability in image build times. * Updated test parity mappings to ensure the added live coverage is correctly linked with the corresponding faster test set. * Added inline guidance for why timing can fluctuate between runs, and why the previous cap needed more headroom. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Raise the
full-e2ecold-onboard acceptance budget (ONBOARD_BUDGET_SECS) from 180s → 205s. This is the umbrella PR for today's live-E2E failures onmain; scope and evidence per job below.Failure triage (main, 2026-07-10)
A full
E2Edispatch onmain(run 29124128082) came back 78 passed / 5 skipped / 3 failed. Each failure was root-caused, not retried blindly:agent-turn-latencyfull-e2erebuild-hermesoperation was canceled,runner lost communication with the server) — not a test assertion or image-build error. Retry on post-bumpmainin flight (29129133666). See "Hermes v0.18" below.full-e2e— full analysis (no gaps)Symptom. The
[1/8]-to-first-responsegate (full-e2e.test.ts:215) failed 3 consecutive times:Not flake, not inference, not a code regression — proven by the
onboard-progress-budget.jsonartifact decomposition:onboardSecstotalSecsf4cd7ea9)fcc121d5)main— both heads are after the Hermes v0.18 bump, so the bump is not the cause.totalSecs − onboardSecs); inference is a rounding error.Fix. Raise to 205s: covers the observed 173s worst case plus build-variance headroom, while still catching gross onboard regressions (a real regression blows well past 205s;
MAX_SILENCE_SECSand BuildKit-fallback assertions are unchanged).Hermes v0.18 context
Today's
mainincludesfeat(hermes): upgrade to v0.18 and enable Slack Block Kit(#6507, 17:45Z), plus#6624(release-matched sandbox bases) and#6623(surface cluster image build failures). Becauserebuild-hermesrebuilds the Hermes image, a v0.18-induced regression could in principle surface as runner resource-exhaustion. This PR does not yet claimrebuild-hermesis a flake — the in-flight retry on post-bumpmainis the deciding evidence:Test evidence
commitlint,gitleaks, test-size/shape budgets: passed (pre-commit).Signed-off-by: Prekshi Vyas prekshiv@nvidia.com
🤖 Generated with Claude Code
Summary by CodeRabbit