perf(onboard): use deadlines for gateway health waits

[HOYALIM]

Refs #3768.

Summary

• switch gateway health waits from a fixed attempt cap to a single deadline budget derived from the existing poll-count and poll-interval settings
• apply the same deadline semantics to the package-managed Docker-driver gateway service readiness path
• include the configured deadline in gateway health failure messages while preserving fast success, retry success, and zero-count fail-fast behavior

Validation

• npx @biomejs/biome check src/lib/onboard.ts src/lib/onboard/gateway-health-wait.ts src/lib/onboard/gateway-health-wait.test.ts src/lib/onboard/docker-driver-gateway-service.ts src/lib/onboard/docker-driver-gateway-service.test.ts
• npx vitest run --project cli src/lib/onboard/gateway-health-wait.test.ts src/lib/onboard/docker-driver-gateway-service.test.ts
• npx vitest run --project cli --project integration test/onboard.test.ts test/gateway-final-failure-cleanup.test.ts src/lib/onboard/gateway-start-failure-integration.test.ts
• npm run build:cli
• npm run test-conditionals:scan -- --top 25
• npm run source-shape:check
• npm run check:diff

Summary by CodeRabbit

• Bug Fixes
  • Improved gateway startup reliability by using a time-based health polling deadline (with special handling for zero-interval immediate probes).
  • Updated gateway startup failure messages to include the exact configured health wait limit.
• New Features
  • Added an optional injectable time source for gateway health waiting to improve determinism.
• Tests
  • Improved health-polling test stability by using a virtual clock and revised expectations for deadline-driven behavior.

Signed-off-by: Ho Lim subhoya@gmail.com

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

📝 Walkthrough

Walkthrough

Gateway health polling now uses deadline-based timing with computed wait budgets. The package-managed gateway path, related tests, and onboarding startup error handling were updated to use the new behavior and preserve more specific failure messages.

Changes

Deadline-based health wait

Layer / File(s)	Summary
Wait budget helpers and waitForGatewayHealth deadline logic src/lib/onboard/gateway-health-wait.ts	Adds exported wait-budget helpers, adds an optional now function, and switches waitForGatewayHealth from attempt-count-based polling to deadline-based waiting.
Package-managed gateway startup uses deadline budget src/lib/onboard/docker-driver-gateway-service.ts	Adds an injectable time source, computes a wait budget, uses a deadline for polling, and formats timeout errors with the configured budget.
Virtual clock test support src/lib/onboard/__test-helpers__/virtual-clock.ts, src/lib/onboard/gateway-health-wait.test.ts, src/lib/onboard/docker-driver-gateway-service.test.ts	Adds a virtual clock helper and updates the gateway wait and package-managed gateway tests to drive polling deterministically and assert deadline-based behavior and messages.
Onboard startup error message src/lib/onboard.ts	Reformats the gateway health import and rethrows the underlying error instead of replacing it with a generic startup failure.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: area: onboarding

Suggested reviewers: cv, jyaunches

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: gateway health waits now use deadline-based timing.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/lib/onboard/gateway-health-wait.test.ts (2)

104-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding a local issue reference to the new test title.

This test is the core regression coverage for the deadline-vs-cap behavior tied to #3768; as per coding guidelines for **/*.test.ts, consider a trailing (#3768) suffix.

🤖 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/onboard/gateway-health-wait.test.ts` at line 104, Update the new test
title in gateway-health-wait.test.ts to include the local issue reference
suffix, so the regression coverage for the deadline-vs-cap behavior is clearly
tied to `#3768`; adjust the it(...) description for the polling test to end with
"(`#3768`)".

Source: Coding guidelines

---

29-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

makeVirtualClock duplicated across two test files.

The same helper (minus the sleeper field) is redefined in src/lib/onboard/docker-driver-gateway-service.test.ts. Consider extracting to a shared test utility.

🤖 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/onboard/gateway-health-wait.test.ts` around lines 29 - 41, The
makeVirtualClock helper is duplicated across gateway-health-wait.test.ts and
docker-driver-gateway-service.test.ts, so extract the shared clock setup into a
common test utility and reuse it from both tests. Keep the existing behavior
used by makeVirtualClock, advance, now, and sleeper, but centralize the
implementation so future changes only need to be made once.

🤖 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 `@src/lib/onboard.ts`:
- Line 1899: The final startup failure path is masking the timeout/deadline
message generated in the retry body. Update the outer catch in the gateway
startup flow around the healthWait retry logic so terminal failures preserve and
rethrow the original deadline-aware error instead of replacing it with the
generic “Gateway failed to start” message. Keep the existing timeout throw that
includes healthWait.count and healthWait.interval, and ensure that same error
text reaches the caller on the last failure.

---

Nitpick comments:
In `@src/lib/onboard/gateway-health-wait.test.ts`:
- Line 104: Update the new test title in gateway-health-wait.test.ts to include
the local issue reference suffix, so the regression coverage for the
deadline-vs-cap behavior is clearly tied to `#3768`; adjust the it(...)
description for the polling test to end with "(`#3768`)".
- Around line 29-41: The makeVirtualClock helper is duplicated across
gateway-health-wait.test.ts and docker-driver-gateway-service.test.ts, so
extract the shared clock setup into a common test utility and reuse it from both
tests. Keep the existing behavior used by makeVirtualClock, advance, now, and
sleeper, but centralize the implementation so future changes only need to be
made once.

🪄 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: edff3e71-1387-49b1-9175-b3a869670569

📥 Commits

Reviewing files that changed from the base of the PR and between 5ddf9a1 and 33fcc1c.

📒 Files selected for processing (5)

• src/lib/onboard.ts
• src/lib/onboard/docker-driver-gateway-service.test.ts
• src/lib/onboard/docker-driver-gateway-service.ts
• src/lib/onboard/gateway-health-wait.test.ts
• src/lib/onboard/gateway-health-wait.ts

[apurvvkumaria]

This is a useful deadline conversion for two gateway paths, but it does not yet satisfy the issue it closes:

1. dco-check is failing because the PR body has no contributor declaration. Please add Signed-off-by: Ho Lim <subhoya@gmail.com> to the PR description yourself; maintainers cannot do that on a contributor's behalf.
2. Fixes #3768 is too broad for the current diff. #3768 covers the remaining sandbox, dashboard, gateway-recovery, agent-gateway, and create-stream readiness loops, plus fast polling with capped backoff. This PR changes the gateway health wait and package-managed Docker-driver path and retains a fixed interval. Either complete the remaining acceptance criteria or change the linkage to a partial/mitigation statement so #3768 remains open.
3. The exact-head static gate is red because test/langchain-deepagents-code-image.test.ts is 1501 lines against the 1500-line budget. Rebase onto a green base or split/ratchet the test without raising the ceiling, then rerun the required gate.

The deadline-aware error and virtual-clock coverage look appropriate for the narrower partial scope.

[apurvvkumaria]

The earlier DCO, issue-linkage, and stale line-budget concerns are resolved on this head. One blocking deadline regression remains:

getGatewayHealthWaitBudgetMs() converts a positive poll count with healthPollIntervalSeconds = 0 into a 1 ms deadline. The shared waiter checks that deadline before each probe, so scheduling overhead can produce zero probes; otherwise the gateway receives roughly one probe. The previous maxAttempts behavior supported the configured number of immediate probes. Zero is an accepted environment value, is used by existing tests, and the package-managed happy-path test changes its interval from 0 to 1, masking the compatibility break.

Please either validate and reject a zero interval explicitly, or preserve a bounded zero-interval path that still performs the configured probes. Add deterministic coverage for pre-probe expiry and count > 1 with interval zero in both the shared budget helper and package-managed caller. Also add a rejected/slow probe case that proves the original error and deadline behavior are preserved.

Relevant paths: src/lib/onboard/gateway-health-wait.ts around lines 22-30 and 78, plus src/lib/onboard/docker-driver-gateway-service.ts around lines 327-353.

I found no credential, authorization, SSRF, sandbox-escape, or dependency vulnerability in the six-file exact-head diff. This request is correctness and negative-test coverage for an accepted configuration boundary.

[apurvvkumaria]

Maintainer follow-up 7251c2c05 addresses the exact-head zero-interval finding while preserving Ho Lim as co-author.

• Positive intervals remain deadline-driven.
• An accepted zero interval now uses the prior bounded immediate-attempt behavior, so scheduling overhead cannot suppress every probe.
• Both gateway-health callers share the same wait option builder.
• Added deterministic coverage for zero-interval retries in both callers, pre-probe deadline expiry, rejected readiness errors, and overflowing budget multiplication.

Validation on the pushed head: 24 focused tests passed; CLI build and type-check passed; formatting, repository checks, test-size checks, and secret scanning passed. The change request stays in place until fresh CI and automated review settle.

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/onboard/gateway-health-wait.ts (1)

22-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared normalization helper to reduce duplication.

The Number.isFinite(x) ? Math.max(0, x) : 0 pattern is repeated near-verbatim in getGatewayHealthWaitBudgetMs (lines 26-29), formatGatewayHealthWaitLimit (lines 52-58), and createGatewayHealthWaitOptions (lines 71-76). A single clampNonNegativeFinite helper would remove the drift risk across these three call sites.

♻️ Proposed refactor

+function clampNonNegativeFinite(value: number): number {
+  return Number.isFinite(value) ? Math.max(0, value) : 0;
+}
+
 export function getGatewayHealthWaitBudgetMs(
   healthPollCount: number,
   healthPollIntervalSeconds: number,
 ): number {
-  const normalizedCount = Number.isFinite(healthPollCount) ? Math.max(0, healthPollCount) : 0;
-  const normalizedIntervalSeconds = Number.isFinite(healthPollIntervalSeconds)
-    ? Math.max(0, healthPollIntervalSeconds)
-    : 0;
+  const normalizedCount = clampNonNegativeFinite(healthPollCount);
+  const normalizedIntervalSeconds = clampNonNegativeFinite(healthPollIntervalSeconds);

Also applies to: 48-63, 65-97

🤖 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/onboard/gateway-health-wait.ts` around lines 22 - 35, The same
non-negative finite normalization logic is duplicated across
getGatewayHealthWaitBudgetMs, formatGatewayHealthWaitLimit, and
createGatewayHealthWaitOptions, so extract it into a shared
clampNonNegativeFinite helper and use that helper at each call site to keep the
behavior consistent and avoid drift.

🤖 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.

Nitpick comments:
In `@src/lib/onboard/gateway-health-wait.ts`:
- Around line 22-35: The same non-negative finite normalization logic is
duplicated across getGatewayHealthWaitBudgetMs, formatGatewayHealthWaitLimit,
and createGatewayHealthWaitOptions, so extract it into a shared
clampNonNegativeFinite helper and use that helper at each call site to keep the
behavior consistent and avoid drift.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: efeef74e-ef59-44fe-8481-90aa51c0d98e

📥 Commits

Reviewing files that changed from the base of the PR and between 258433c and 7251c2c.

📒 Files selected for processing (4)

• src/lib/onboard/docker-driver-gateway-service.test.ts
• src/lib/onboard/docker-driver-gateway-service.ts
• src/lib/onboard/gateway-health-wait.test.ts
• src/lib/onboard/gateway-health-wait.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/onboard/docker-driver-gateway-service.ts

[apurvvkumaria]

Superseded by #6542, which carries the reviewed deadline change on current main, includes the zero-interval compatibility fix, and credits @HOYALIM as a commit co-author. Closing this duplicate so there is one active, mergeable head.