Skip to content

perf(agents): warm the agent catalog when the server starts - #2401

Merged
devswha merged 2 commits into
mainfrom
perf/agent-catalog-warm-start
Oct 29, 2026
Merged

devswha merged 2 commits into
mainfrom
perf/agent-catalog-warm-start

Conversation

@devswha

@devswha devswha commented Oct 29, 2026

Copy link
Copy Markdown
Owner

What

The Ovis server now reads the agent catalog once in the background when it starts, so the first GET /api/agent-catalog is served from AgentDetector's cache instead of waiting for every agent's --version.

  • createServer's instance gets warmAgentCatalog(): one agentCatalogFor(...) on the server's own detector, not awaited, errors logged (agent catalog warm-up failed: …) and never thrown.
  • Only the two real entrypoints call it: server/index.ts's main start, and server/remote-entry.ts after the bridge has registered. Test and script servers never warm, so they never run real version commands.
  • stop() now closes the detector. Running version process groups are killed, and queued reads settle without launching anything.
  • The version spawn and its taskkill get windowsHide: true, like the other background spawns in server/.

No route, response or contract change. This touches server/ only.

Why

Measured on the maintainer's PC with Hermes installed:

The first-run setup guide (#2332) opens on that first answer ("Checking this PC…"), so one slow assistant held the whole first screen.

Cost:

  • Each real start now runs the installed agents' version commands, at most four at a time (PARALLEL_RUNS), plus a login-shell PATH lookup when it is needed.
  • Listening never waits for them.
  • A remote bridge pays the same once it is registered. Its catalog is proxied to other PCs, so that is where the first answer matters too.

Tests

server/agent-catalog-routes.test.ts:

  • The first GET after a warm start is served from the warm cache with one version run in total, including a GET that arrives while the warm probe is still held.
  • A failed warm-up is logged with its message, and the server keeps serving.
  • A version probe still held when stop() runs is ended.

The waits are deadline-bounded signals, with no fixed sleeps. The tests use temporary state and port: 0.

bun run check fast is green (8763 pass, 0 fail).

Reviews

omo-native-code-reviewer. Round 1 raised two MEDIUMs:

  • The warm-up was on by default for any non-zero port, so test children and scripts would have run real --version commands. It is now explicit.
  • The version commands had no windowsHide. Added.

Round 2 approved, with no HIGH or MEDIUM.

Codex (gpt-6.1-sol, xhigh). Round 1 raised two MEDIUMs:

  • The remote bridge warmed before registering, which a test reaches. It now warms after registering.
  • Probes outlived stop(). close() now ends them.

Round 2 found no HIGH or MEDIUM. One LOW is left: the login-shell PATH lookup is not cancelled by stop(). It ends on its own timeout, and the process exits on stop anyway.

The first GET /api/agent-catalog after a start waited for every agent's
--version; Hermes's alone takes ~7 s cold (its version timeout is 15 s since
#2382), so the first-run guide's "Checking this PC" sat for seconds while later
answers took 3 ms from AgentDetector's cache.

createServer's instance gets warmAgentCatalog(): one background catalog read on
the server's detector, errors logged, never thrown. Only the main start and the
remote bridge (after it registers) call it, so test and script servers never run
real version commands. stop() now closes the detector: running version process
groups are killed and queued reads settle without launching. Version spawns and
taskkill get windowsHide. No route, response or contract change.
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Summary

Summary by CodeRabbit

  • New Features
    • The agent catalog now checks agent versions in the background when the server starts, so opening the catalog after those checks finish no longer waits for slow version commands.
  • Bug Fixes
    • Version checks stop when the server shuts down, preventing lingering checks from delaying shutdown.

The server now warms the agent catalog during startup. Version checks can be cancelled during shutdown, and tests cover overlapping catalog reads, warm-up failures, and process termination.

Changes: Agent catalog warm-up

Layer / File(s) Summary
Version-read cancellation
server/agent-catalog-detect.ts
runVersion accepts an optional AbortSignal. AgentDetector.close() aborts active reads and releases queued slot waiters.
Server warm-up and lifecycle
server/index.ts, server/remote-entry.ts, server/agent-catalog-routes.test.ts, changelog.d/agent-catalog-warm-start.md
The server exposes and invokes catalog warm-up at startup, including after bridge registration. Shutdown closes the detector. Tests cover shared reads, explicit warm-up, logged failures, and stopping held processes. The changelog describes the startup checks.

Priority: Low

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

Change: Other

Possibly related PRs

  • devswha/ovis#2332: Adds the setup flow that reads the agent catalog through the endpoint affected by this warm-up.
  • devswha/ovis#2382: Changes per-agent timeout behavior in the version-detection path used by warm-up.
  • devswha/ovis#2326: Introduces the detector, catalog endpoint, and server wiring extended by this change.
Merge Risk: Minimal · previous review 663f5

The startup warm-up does not appear to block catalog requests or leave them stuck after a failed lookup. No merge-blocking issue remains from this review.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: warming the agent catalog when the server starts.
Description check Passed The description explains the problem, resulting behavior, scope, lifecycle handling, validation, and review findings. It uses headings different from the template and does not explicitly list skipped …
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.
Full details: Description check

Explanation

The description explains the problem, resulting behavior, scope, lifecycle handling, validation, and review findings. It uses headings different from the template and does not explicitly list skipped checks, but it includes the required change and validation information and is mostly complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

…ts own

Six fixture agents, more than run at once, every read held until the GET has
asked for each one: each executable's --version runs exactly once, queued reads
included. It fails when the detector stops sharing its in-flight reads.
@devswha

devswha commented Oct 29, 2026

Copy link
Copy Markdown
Owner Author

Measured the merge pass's case again on this PC: a real server (the main entrypoint, an isolated herdr session, fresh state dir), 15 agents including Hermes, 3 runs each, main (3a65b1b) and this PR alternating.

first GET /api/agent-catalog main PR
right at start (as soon as the port accepts) 1.66 / 1.51 / 1.29 s 1.12 / 1.51 / 1.26 s
2 s after start 1.49 / 1.76 / 1.18 s 0.10 / 0.003 / 0.002 s

strace -f -e trace=execve over a start plus the first GET right at start counts the same processes on both: 110 execs, with the same version probes (98 lines matching --version/version). An early GET does not start probes of its own. AgentDetector shares each in-flight version read by fingerprint (#running), queued reads included, and the warm-up and the route use the same detector.

New test (4aae3ec): six fixture agents (more than PARALLEL_RUNS), every read held until the GET has asked for each agent. Each executable's --version runs exactly once. It fails when the shared in-flight read is removed (checked by mutation), and passes here. It is a guard, not a fix: the sharing was already in place, so this test did not fail first on this branch.

So I could not reproduce "0 s is 34% slower" here. If your runs differ, the method may differ: when the clock starts, whether HOME has more agents, a cold hermes --version. Tell me and I'll rerun yours exactly.

@devswha

devswha commented Oct 29, 2026

Copy link
Copy Markdown
Owner Author

Merging 4aae3ec. The agent catalog's version reads now start when the server starts (the main entrypoint, and the remote bridge once registered; test and script servers never run a real --version), and a GET that arrives during that warm-up shares its in-flight reads rather than starting its own (test added). stop() closes the detector and kills any version process group still running; Windows runs them hidden (windowsHide). No HTTP contract change. Measured by the merge pass as a clean pair: real bun run server on loopback with an isolated state dir, the real PATH (15 agents detected, Hermes 0.21.5 included), 5 fresh servers per variant and delay, main and PR alternated run by run; first GET /api/agent-catalog, median: opened right after the server answered 3.385 s on main -> 2.831 s with this PR (16% faster); opened 2 s after start 5.430 s -> 2.807 s (48% faster). Host load averaged ~87 (1 min) during the runs, so absolute numbers are noisy; both medians improve. Reviewed by the merge pass.

@devswha
devswha merged commit c0f635d into main Oct 29, 2026
19 checks passed
@devswha
devswha deleted the perf/agent-catalog-warm-start branch October 29, 2026 23:08
@devswha

devswha commented Oct 29, 2026

Copy link
Copy Markdown
Owner Author

Measurement method (merge pass), for the record. Two detached worktrees, current main 7a6f8b0 and this PR 4aae3ec, each with bun install --frozen-lockfile --ignore-scripts. Each sample starts a fresh bun run server (the real entrypoint) with HOST=127.0.0.1, a free PORT, OVIS_TOKEN="", OVIS_TELEMETRY=0, a fresh OVIS_STATE_DIR / XDG_CONFIG_HOME / XDG_STATE_HOME, an isolated HERDR_SOCKET and HERDR_TEST_SESSION, and the user's real PATH and HOME (so the installed agents, Hermes warm, are found: 15 agents, Hermes 0.21.5, no version errors). The harness waits until the server answers /api/health, then times curl -s -o /dev/null -w '%{time_total}' http://127.0.0.1:PORT/api/agent-catalog either at once (0 s) or after a fixed 2 s, then stops the server and wipes its state. 5 samples per variant and delay, main and PR alternated run by run, one server at a time. Results, seconds, median in bold: 0 s main 8.400, 3.385, 3.572, 2.367, 2.808 (3.385) vs PR 3.003, 2.831, 2.767, 2.568, 2.871 (2.831); 2 s main 3.373, 5.430, 5.533, 5.576, 4.435 (5.430) vs PR 1.163, 3.074, 3.511, 2.807, 1.947 (2.807). Host load averaged ~87 (1 min) during the runs, so absolute times are noisy; an earlier non-alternating run had shown the 0 s case slower, which this alternating run and the lane's strace (same probes, an early GET starting none of its own) do not support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant