Repository navigation
perf(agents): warm the agent catalog when the server starts - #2401
Conversation
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.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
| 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:
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: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
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.
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.
|
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.
New test (4aae3ec): six fixture agents (more than 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 |
|
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 |
|
Measurement method (merge pass), for the record. Two detached worktrees, current main 7a6f8b0 and this PR 4aae3ec, each with |
What
The Ovis server now reads the agent catalog once in the background when it starts, so the first
GET /api/agent-catalogis served fromAgentDetector's cache instead of waiting for every agent's--version.createServer's instance getswarmAgentCatalog(): oneagentCatalogFor(...)on the server's own detector, not awaited, errors logged (agent catalog warm-up failed: …) and never thrown.server/index.ts's main start, andserver/remote-entry.tsafter 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.taskkillgetwindowsHide: true, like the other background spawns inserver/.No route, response or contract change. This touches
server/only.Why
Measured on the maintainer's PC with Hermes installed:
hermes --versionalone takes 7.4 s, and itsversion_timeout_msis 15 s since fix(agents): catalog findings from real installs on the maintainer's PC #2382.The first-run setup guide (#2332) opens on that first answer ("Checking this PC…"), so one slow assistant held the whole first screen.
Cost:
PARALLEL_RUNS), plus a login-shell PATH lookup when it is needed.Tests
server/agent-catalog-routes.test.ts: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 fastis green (8763 pass, 0 fail).Reviews
omo-native-code-reviewer. Round 1 raised two MEDIUMs:
--versioncommands. It is now explicit.windowsHide. Added.Round 2 approved, with no HIGH or MEDIUM.
Codex (gpt-6.1-sol, xhigh). Round 1 raised two MEDIUMs:
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.