feat(cli): make startup readiness budget configurable (#13369) - #13433
Merged
diegosouzapw merged 1 commit intoSep 15, 2026
Merged
diegosouzapw merged 1 commit into
diegosouzapw merged 1 commit into
Conversation
…3369) The 60s readiness probe budget was hardcoded at both the definition (pid.mjs:waitForServer) and call site (serve.mjs), with no env var or flag to raise it. On slow cold starts (Windows with antivirus/fs watchers, heavy containers) the warning fires on every single start even though the server comes up fine — users see it as an error. Changes: - Add resolveReadyTimeoutMs() that reads OMNIROUTE_READY_TIMEOUT_MS env var (falls back to 60000). Follows the same pattern as OMNIROUTE_HTTP_TIMEOUT_MS. - Add --ready-timeout <ms> flag to omniroute serve. - Register the env var in the CLI env listing (env.mjs). - Update reportReadinessTimeout() to show the actual timeout value and suggest doubling it when the warning fires. - Add 11 unit tests for resolveReadyTimeoutMs covering defaults, env var override, explicit override, precedence, and edge cases. - Document in ENVIRONMENT.md (CLI helpers) and TROUBLESHOOTING.md (new Slow Startup section). Fixes diegosouzapw#13369
diegosouzapw
merged commit Sep 15, 2026
fc111dc
into
diegosouzapw:release/v3.8.51
8 of 16 checks passed
This was referenced Sep 15, 2026
diegosouzapw
added a commit
to dmlanday/OmniRoute
that referenced
this pull request
Sep 15, 2026
…e readiness budget Merge origin/release/v3.8.51 (which already carries diegosouzapw#13433's resolveReadyTimeoutMs()/--ready-timeout) and reconcile it with this PR's per-probe timeout escalation in waitForServer()/pollHealthOnce() and the onOutcome-aware reportReadinessTimeout() diagnostic — both capabilities are now preserved: a configurable total readiness budget and an escalating per-probe ceiling within it. Also thread the already-resolved readyTimeoutMs from runServe() into runWithSupervisor() instead of reading opts.readyTimeout out of scope inside that function (opts is not a parameter of runWithSupervisor and was never actually reachable there), so --ready-timeout/OMNIROUTE_READY_TIMEOUT_MS correctly govern the waitForServer() call instead of a hardcoded 60000. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
diegosouzapw
added a commit
to dmlanday/OmniRoute
that referenced
this pull request
Sep 15, 2026
Merge origin/release/v3.8.51 to resolve the import-list conflict with diegosouzapw#13433's resolveReadyTimeoutMs() addition (no logic overlap with this PR's port preflight). The "finds a real listening socket (end-to-end)" test called the real findListeningPids() without mocking its lsof/netstat dependency, so it always failed on any POSIX runner without lsof installed instead of skipping — exactly the gap findListeningPids() itself already handles gracefully in production (falls back to reporting "port free" rather than a false "busy"). Detect lsof availability the same way the production code discovers it (spawn it and check for ENOENT) and skip the test with a clear reason when it's absent, so a CI runner without lsof reports the test as skipped instead of a false regression. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This was referenced Sep 18, 2026
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…3369) (diegosouzapw#13433) The CLI readiness budget is configurable through `OMNIROUTE_READY_TIMEOUT_MS` or `omniroute serve --ready-timeout <ms>` (default unchanged at 60s). The timeout warning prints the budget it actually used and suggests a larger value, for slow cold starts such as Windows (diegosouzapw#13369). Documented in `ENVIRONMENT.md` and `TROUBLESHOOTING.md`, with 11 resolver cases. Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green. Thanks @KooshaPari!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The 60s readiness probe budget was hardcoded with no env var or flag to raise it. On slow cold starts (Windows with antivirus/fs watchers, heavy containers) the warning fires on every single start even though the server comes up fine — users see it as an error.
Reported in #10754 where a Windows user's boot takes ~6 minutes, so the warning always fires.
Changes
--ready-timeout <ms>flag toomniroute serve. Wire it toresolveReadyTimeoutMs(). UpdatereportReadinessTimeout()to show the actual timeout value and suggest doubling it.OMNIROUTE_READY_TIMEOUT_MSin the CLI env listing.Tests
11 unit tests in
tests/unit/cli/ready-timeout.test.tscovering:Acceptance Criteria (from #13369)
Closes #13369