Skip to content

fix(ops): Docker HEALTHCHECK probes /healthz not deep monitoring - #10307

Open
RaviTharuma wants to merge 9 commits into
diegosouzapw:release/v3.8.50from
RaviTharuma:fix/docker-healthcheck-use-healthz
Open

fix(ops): Docker HEALTHCHECK probes /healthz not deep monitoring#10307
RaviTharuma wants to merge 9 commits into
diegosouzapw:release/v3.8.50from
RaviTharuma:fix/docker-healthcheck-use-healthz

Conversation

@RaviTharuma

@RaviTharuma RaviTharuma commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Official image HEALTHCHECK runs node healthcheck.mjs, which probed /api/monitoring/health (SQLite ping + deep status) with a 5s Docker timeout.
  • That path is the one fix(ops): /healthz (and other HTTP) times out while event loop is busy (k8s probes flap) #10052 already called out as too heavy for interval probes. When catalog/compression pin the event loop, Docker marks the container Unhealthy and Compose/k8s restart the only replica mid-session.
  • Default probe is now /healthz (lifecycle ok/starting/stopping). /api/monitoring/health stays for humans/dashboard.
  • resolveHealthPath tests updated.

Related Issues

Validation

  • Production-code changes include a new or updated automated test in this PR
node --import tsx --test tests/unit/docker-healthcheck-base-path.test.ts tests/unit/docker-healthcheck-3151.test.ts
# tests 5, pass 5

Reviewer Notes

Does not change omniroute health CLI (#6677 still hits monitoring/health). Only the container HEALTHCHECK.

diegosouzapw and others added 6 commits August 8, 2026 00:08
@diegosouzapw
fix(deps): bump nanoid, dompurify for 2 new Dependabot alerts (diegos…
b090b60 
…ouzapw#189, diegosouzapw#190)

Bumps: nanoid ^3.3.17 (was transitive, now overridden), dompurify ^3.4.13
(with monaco-editor scoped override). Closes Dependabot diegosouzapw#189, diegosouzapw#190.

Remaining diegosouzapw#182-diegosouzapw#188 (js-yaml + mermaid) already closed by diegosouzapw#9651 merge —
awaiting Dependabot re-scan.

npm audit → 0 vulnerabilities.
@diegosouzapw
fix(deps): bump nanoid, dompurify for Dependabot diegosouzapw#189, di…
026e1ca 
…egosouzapw#190

Closes Dependabot diegosouzapw#189 (dompurify 3.4.13) and diegosouzapw#190 (nanoid 3.3.17). npm audit → 0.
@diegosouzapw
fix(repo): harden .gitignore to also ignore a _tasks symlink (/_tasks)
918fba5 
_tasks is a SEPARATE nested git repo (gitignored). The pattern _tasks/ (trailing
slash) ignores only a directory, not a SYMLINK named _tasks. A self-referential
_tasks symlink can slip in via git add -A and, once pulled, checkout materializes
it over the real _tasks repo (destroying plans/specs/hands-off). Anchored /_tasks
ignores the symlink too, preventing re-capture.
@ritheshcn25
Hide health-check excluded models from /v1/models catalog (diegosouza…
5f0a394 
…pw#10026)

Mirror the request-time exclusion rule (provider_specific_data.excludedModels)
in the unified catalog builder: a model is hidden when its provider has
connections but none of them is eligible for it. Applied across the
PROVIDER_MODELS, synced, custom, alias-backed, and managed-fallback loops
so ghost models no longer appear as available.

Co-authored-by: ritheshcn25 <ritheshcn25@users.noreply.github.com>
@RaviTharuma @cursoragent
fix(models): memoize getModelsDevPricing (event loop / healthz) (dieg…
ca23eed 
…osouzapw#10055)

* fix(models): memoize getModelsDevPricing for /v1/models catalog

resolveCatalogPricing called getModelsDevPricing once per model while
building GET /v1/models. Each call re-scanned models_dev_pricing and
JSON.parsed every row (~10k SQL scans + multi-GB parse work), pegging
the event loop so even /healthz timed out (diegosouzapw#9685, diegosouzapw#10052).

Memoize the parsed map until saveModelsDevPricing / clearModelsDevPricing
and add a unit test for invalidation.

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>

* fix(db): invalidate modelsDevPricing cache on DB reset (diegosouzapw#10055)

Copilot review fixes:
1. Register invalidateModelsDevPricingCache() with DB state reset system
   so resetDbInstance() clears the process-local memo, preventing stale
   pricing data from surviving across DB reset/restore operations.
2. Add test assertion verifying DB reset bypasses the memo (Copilot diegosouzapw#10055).

The process-local memo at modelsDevSync.ts:204 caches getModelsDevPricing()
results until saveModelsDevPricing()/clearModelsDevPricing() to avoid
re-scanning all pricing rows on every /v1/models request. Without this hook,
backup restore and test DB resets would serve stale cached data from the
previous connection.

Tests: npm run test:unit:serial -- tests/unit/modelsDevSync-extended.test.ts

---------

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
@RaviTharuma
fix(ops): Docker HEALTHCHECK probes /healthz not deep monitoring
f3d08ab 
/api/monitoring/health does a SQLite ping and more. When the event loop
is busy the official image HEALTHCHECK (5s timeout) marks the container
Unhealthy and orchestrators restart the only replica mid-session.
@RaviTharuma

RaviTharuma commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Implements / pairs with #10311. Related #10052 #10297.

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.50 August 14, 2026 19:26
@diegosouzapw diegosouzapw mentioned this pull request Aug 14, 2026
Open
@diegosouzapw

diegosouzapw commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Thanks for tracking down #10052 and following through with the actual Docker HEALTHCHECK fix — the healthcheck.mjs change is clean, well-commented, and the test updates line up exactly with the new /healthz default. Good scoping too: keeping omniroute health CLI on the deep endpoint (#6677) is the right call and you called it out explicitly.

One thing I need before this can merge: the branch was cut from main rather than release/v3.8.50, so it's currently carrying 5 commits that aren't yours (two dependabot bumps, a .gitignore hardening commit, and two already-merged PRs — #10026 and your own #10055) on top of your actual change. That's why GitHub is showing this PR as un-mergeable (conflicting) and why the diff looks much bigger than the real fix. Could you rebase/cherry-pick just your fix(ops): Docker HEALTHCHECK probes /healthz not deep monitoring commit onto a fresh branch off origin/release/v3.8.50 and force-push? That should clear the conflict and very likely both the Build and Quality Ratchet CI failures too, since neither looks related to your actual diff.

Small optional follow-up while you're in there: docs/guides/DOCKER_GUIDE.md (around line 272) still says the Docker healthcheck probes /api/monitoring/health — worth a one-line update to match the new default.

Once it's rebased onto the release branch cleanly I'm happy to see this merged.

@RaviTharuma @diegosouzapw
Merge branch 'release/v3.8.50' into fix/docker-healthcheck-use-healthz
5710a8b 
Refresh the PR head against origin/release/v3.8.50 (2280 commits ahead of the
PR's old-main base). Resolves conflicts preferring the base's evolved
models_dev_pricing memo (pricingMemo/pricingMemoVersion), base lockfile and
base .gitignore; keeps this PR's healthcheck change (DEFAULT_HEALTH_PATH=/healthz).

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
@diegosouzapw

diegosouzapw commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Babysit summary — CI green via refresh-merge

The two reds here (Build, Quality Ratchet) were stale verdicts from base drift — the PR branch was cut from an old main base and sat ~9 days / 2280 commits behind release/v3.8.50 (the failing runs were computed against the pre-refresh merge-ref; every substantive gate on that stale run already passed).

Fix (refresh-merge): merged origin/release/v3.8.50 into fix/docker-healthcheck-use-healthz → commit 5710a8b11a. Conflicts resolved by preferring the base's evolved implementation for changes that already landed on the release line in more advanced form:

  • src/lib/modelsDevSync.ts + its extended test → took base's versioned pricingMemo/pricingMemoVersion memo (supersedes the bundled older #10055 memo variant)
  • .gitignore, package-lock.json → took base (the bundled dependabot/.gitignore bytes are already on base)
  • Kept the PR's genuine contribution: scripts/dev/healthcheck.mjs probes /healthz instead of /api/monitoring/health, the catalog.ts exclusion guard, and the updated healthcheck unit tests. Net diff vs base tip = 5 files, +28/−7.

Validation: typecheck:core clean; healthcheck unit tests 5/5 pass; npm run pre-commit hooks passed on the merge commit.

Result — gh pr checks 10307 (fresh run 31881886433): all non-advisory checks pass — Build (advisory) ✅, Fast Quality Gates ✅, No new ESLint warnings ✅, Unit Tests fast-path 1–4/4 ✅, Vitest ✅, Merge integrity ✅, Docs Gates ✅, dast-smoke ✅, semgrep ✅. No unresolved review threads. Ready for human review & merge.

adevwithpurpose and others added 2 commits August 15, 2026 18:23
@adevwithpurpose
Merge remote-tracking branch 'origin/release/v3.8.50' into HEAD
8ce864b
@RaviTharuma @diegosouzapw @adevwithpurpose
fix(ops): keep healthcheck PR scoped to the /healthz probe
71f7955 
Drop the stray catalog ghost-model exclusion that leaked into this branch
from main (already covered upstream). Restore catalog.ts to the release
version so the PR contains only the Docker HEALTHCHECK /healthz fix, its
tests, and the changelog entry.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diegosouzapw diegosouzapw Awaiting requested review from diegosouzapw diegosouzapw is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@RaviTharuma @diegosouzapw @ritheshcn25 @adevwithpurpose