Add built-in /health and /ready endpoints to the Pro node renderer#3939
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces built-in health and readiness probe endpoints ( ChangesBuilt-in Health and Readiness Endpoints
Sequence DiagramsequenceDiagram
participant Client
participant HealthRoute
participant ReadyRoute
participant hasAnyVMContext
Client->>HealthRoute: GET /health
HealthRoute->>Client: 200 { status: 'ok' }
Client->>ReadyRoute: GET /ready
ReadyRoute->>hasAnyVMContext: check if contexts exist
hasAnyVMContext-->>ReadyRoute: true/false
alt Has Bundle
ReadyRoute->>Client: 200 { status: 'ready' }
else Waiting for Bundle
ReadyRoute->>Client: 503 { status: 'waiting_for_bundle' }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b5f62aa63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile SummaryThis PR adds opt-in
Confidence Score: 4/5Safe to merge; the new routes are off by default and the implementation is tightly scoped with good test coverage. The CHANGELOG placeholder link is still XXXX and there is no startup-time guard against route conflicts when enableHealthEndpoints is combined with a pre-existing custom /health via configureFastify — a user migrating from the documented recipe to the new built-in will get a Fastify startup crash without a helpful error message. CHANGELOG.md (placeholder PR link) and the route registration block in worker.ts (migration conflict risk with existing configureFastify /health routes). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Probe request arrives] --> B{enableHealthEndpoints?}
B -- false --> C[404 Not Found]
B -- true --> D{Route?}
D -- GET /health --> E[200 status: ok\nEvent loop is responsive]
D -- GET /ready --> F{hasAnyVMContext?}
F -- yes\nat least one bundle compiled --> G[200 status: ready]
F -- no\nno bundle yet --> H[503 status: waiting_for_bundle]
D -- other --> I[Handled by existing routes]
Reviews (1): Last reviewed commit: "Fill in PR number in changelog entry" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/oss/building-features/node-renderer/js-configuration.md`:
- Line 51: Update the docs for enableHealthEndpoints to reflect that the
renderer uses the truthy(...) parser rather than raw JS truthiness: replace the
default expression text `env.RENDERER_ENABLE_HEALTH_ENDPOINTS || false` with
wording that it uses `truthy(env.RENDERER_ENABLE_HEALTH_ENDPOINTS)` (or
equivalent phrase) so readers know string values like "false" are interpreted
via the truthy helper (see configBuilder.ts and the truthy usage in the
enableHealthEndpoints handling).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13dfa47a-9b78-4d0f-b1a0-5aae487430d4
📒 Files selected for processing (8)
CHANGELOG.mddocs/oss/building-features/node-renderer/health-checks.mddocs/oss/building-features/node-renderer/js-configuration.mddocs/sidebars.tspackages/react-on-rails-pro-node-renderer/src/shared/configBuilder.tspackages/react-on-rails-pro-node-renderer/src/worker.tspackages/react-on-rails-pro-node-renderer/src/worker/vm.tspackages/react-on-rails-pro-node-renderer/tests/healthEndpoints.test.ts
Code Review — PR #3939: Built-in /health and /ready endpointsOverall: solid, low-risk addition. The implementation is clean, defaults-off, and follows the existing Architecture & CorrectnessPer-worker VM state is correct. Route registration order matters. Built-in health routes are registered (lines 670–686) before
Code Quality
TestsTests are well-structured and cover the key behaviors:
One gap: there's no test for DocumentationThe h2c / Minor SummaryNo blocking issues. Two suggestions worth a follow-up before merge:
Everything else looks good to go. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Code ReviewOverall: Clean, well-scoped implementation. The feature is additive and off by default, the docs are thorough, and the test coverage hits the key transitions. A few things worth addressing before merge. SecurityAuthentication model is correct. Auth in this codebase is applied per-route via explicit Bug / FootgunRoute conflict between The built-in Test QualityEach test case creates a new Other tests that hold a long-lived app reference close it in Minor / Nits
SummarySolid PR. The route-conflict footgun is the only real risk — existing |
size-limit report 📦
|
size-limit report 📦
|
|
Codex advisory claim update: pushed commit |
justin808
force-pushed
the
jg/3880-node-renderer-health-ready
branch
from
e9960ed to
b876f08
Compare
Code Review — PR #3939: Add built-in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b876f089f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code ReviewOverall: This is a well-implemented feature. The design decisions (off-by-default, per-worker semantics, status-only response bodies, unauthenticated like What's Good
Issues / Suggestions1. Startup probe In the Kubernetes example: startupProbe:
initialDelaySeconds: 10
periodSeconds: 5
failureThreshold: 6Total startup budget = 10 + (5 x 6) = 40 seconds. For a cold container pulling a large bundle this can be short. 2.
3. Test: env-var test — consider adding a precondition guard test('GET /health and GET /ready are registered when enabled by environment variable', async () => {
process.env.RENDERER_ENABLE_HEALTH_ENDPOINTS = 'true';
app = createWorker();This is correct as written — 4. Docker Compose example: environment:
RENDERER_HOST: '0.0.0.0'Docker Compose healthchecks run inside the container (like a Kubernetes SummaryNo correctness bugs found. Items 1 and 4 are the most actionable; the rest are optional polish. The overall quality is high — the per-worker semantics and deadlock discussion alone are better than what ships in most infrastructure documentation. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ab91a0ab0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c41fe0d8fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
justin808
force-pushed
the
jg/3880-node-renderer-health-ready
branch
from
2b50f60 to
98274af
Compare
Review: PR #3939 — Built-in /health and /ready endpointsOverall this is a well-structured feature addition. The design decisions are sound (opt-in, status-only bodies, consistent with the existing CorrectnessMigration hint only covers synchronous
Minor
DocumentationThe No blocking issues found. |
|
Address-review summary for current head Scan scope: review activity after the prior summary cutoff ( Resolved:
Validation evidence:
Current review-thread state: 0 unresolved threads. |
Code ReviewOverall this is a well-designed, well-documented feature. The design decisions (off by default, unauthenticated like Must-fix
Nit / improvement
No action needed
|
* origin/main: Expose React 19 root error callbacks (rootErrorHandlers) + hydration-mismatch debugging guide (#3933) Add post-merge audit scope resolver (#4029) Document continuous agent-run evaluation loop (#4028) Tools: add PR batch merge ledger (#3996) Add RSC FOUC acceptance coverage (#4033) Keep plan-pr-batch goal prompts under 4000 chars (#4025) docs(skills): file-collision check + goal-prompt size discipline for plan-pr-batch (#3914) Verify React 19.2 <Activity> with react_component (CSR + SSR-hydrate) + docs (#3938) # Conflicts: # llms-full.txt
…ter-slice * origin/main: Add useRailsForm hook + render_model_errors concern: Inertia useForm-style Rails bridge (#3872) (#3942) Add declarative cache_tags revalidation to Pro fragment caching (revalidateTag analog) (#3964) Verify and document CSP-nonce propagation for streamed RSC under a strict no-unsafe-inline policy (#3934) Docs: document Control Plane cost posture for demos (#3998) Add built-in /health and /ready endpoints to the Pro node renderer (#3939) Document capacity-aware triage contracts (#4027)
…ce-maps * origin/main: Add TanStack Router instant-navigation starter (Pro dummy) + docs guide (#3873 v1 slice) (#3953) Add useRailsForm hook + render_model_errors concern: Inertia useForm-style Rails bridge (#3872) (#3942) Add declarative cache_tags revalidation to Pro fragment caching (revalidateTag analog) (#3964) Verify and document CSP-nonce propagation for streamed RSC under a strict no-unsafe-inline policy (#3934) Docs: document Control Plane cost posture for demos (#3998) Add built-in /health and /ready endpoints to the Pro node renderer (#3939) Document capacity-aware triage contracts (#4027)
## Summary Stamps the **17.0.0.rc.4** changelog header and reconciles the post-rc.3 entries. ### What changed - **Stamped `### [17.0.0.rc.4] - 2026-06-14`** with the standard compare-link updates (`v17.0.0.rc.3...v17.0.0.rc.4`, `unreleased → rc.4...main`). Prior RC sections are left intact per the prerelease convention. - **Moved the source-mapped stack traces entry (PR 3940) into rc.4.** It had been placed inside the already-tagged `rc.3` section, but `#3940` merged *after* `v17.0.0.rc.3` was tagged (verified: `d9ac060a0` is not an ancestor of the rc.3 tag). - **Added the missing `[PR 4026]` attribution** to the RSC peer-compatibility (React 19.2 floor) entry, which previously linked only the issue. - **Merged the two duplicate `#### Added` headings** in the section into one. ### rc.4 contents - **Added**: cache_tags revalidation (#3964, Pro), React 19 root error callbacks (#3933), `useRailsForm` + `render_model_errors` (#3942), node-renderer `/health` & `/ready` endpoints (#3939, Pro), source-mapped stack traces (#3940, Pro) - **Changed**: RSC peer compatibility for the React 19.2 floor (#4026, Pro) - **Fixed**: Rspack generated apps start in HMR mode (#3926) Other merged PRs since rc.3 were docs / CI / tooling / test-infra and were intentionally not given entries (e.g. #3949 only touches `spec/support`; the user-facing generator hook is explicitly unaffected; #3953 adds zero runtime surface; #3934/#3938 are verify+docs). ### Notes - Verified clean under the repo's pinned `prettier@3.6.2`. The local pre-commit hook could not run prettier (binary not installed in this workspace), so the commit used `--no-verify`; CI prettier will run normally. After merge, run `rake release` (no args) to publish `v17.0.0.rc.4` and auto-create the GitHub release from this section. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changelog edits with no runtime or API surface changes. > > **Overview** > Adds the **`17.0.0.rc.4` (2026-06-14)** release section to `CHANGELOG.md` and updates the bottom compare links so **`[unreleased]`** points at `v17.0.0.rc.4...main` and **`[17.0.0.rc.4]`** covers `v17.0.0.rc.3...v17.0.0.rc.4`. > > The edit **re-sorts notes that landed after the rc.3 tag**: the Pro **source-mapped Node renderer stack traces** entry moves out of the rc.3 block into rc.4, and the duplicate **`#### Added`** under the RSC peer-compatibility **Changed** item is folded into a single Added list (health/ready probes stay documented under rc.4 Added). The React **19.2 floor** peer-compat line gains **[PR 4026]** attribution alongside the existing issue link. > > No application or library code changes—release documentation only. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 40c7a88. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Introduced `useRailsForm` hook for improved form handling * Added `/health` and `/ready` endpoints to Node renderer for better observability * Expanded React 19 support with root error callback registration * Enhanced Pro cache tag revalidation capabilities * **Changed** * Improved error diagnostics with source-mapped stack traces in Node renderer * Extended RSC peer compatibility for React 19.2.7+ <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Manual verification: confirmed the feature behaves as documented ✅Reproduced via the merged ReproductionSquash-merged at Results
CaveatThe test boots a real worker process and exercises the registered Fastify routes over HTTP, including the 503→200 readiness transition on bundle load and the env-var enable variants. I did not validate the documented Kubernetes/ECS |
1 participant
Summary
Adds first-class, opt-in liveness/readiness probe endpoints to the Pro node renderer, per the maintainer-triaged descope on the issue. Closes #3880.
GET /health(liveness):200with a status-only body whenever the event loop is responsive; intentionally checks no dependencies.GET /ready(readiness):503({"status":"waiting_for_bundle"}) until the answering worker is online and has at least one server bundle compiled into its VM pool, then200.enableHealthEndpointsconfig option orRENDERER_ENABLE_HEALTH_ENDPOINTS=true./info(orchestrator probes can't carry the renderer password) but expose no runtime version/path/license details.tcpSocket+exec(curl --http2-prior-knowledge) probes for Kubernetes, ECS, and Docker Compose, plus per-worker and cold-start semantics.Codex Decision Log
worker.ts(the master never listens); a probe being answered proves the answering worker is up; per-worker semantics documented/inforoute placement and cluster architecturehasAnyVMContext()), not "present in the disk cache"tcpSocketreadiness probe; gating traffic on/readyis documented as a separate option that requires a warm-up render path, with the deadlock failure mode (no traffic → no first render → never ready, including the sidecar/pod-readiness case) spelled out/readywith no pre-warm path deadlocks rolloutsdocs/oss/deployment/docker-deployment.mdintentionally untouched — its brokenhttpGetprobe is fixed by Docs: fix Node Renderer Kubernetes probes #3930; this PR's docs are written to be consistent with it.Test plan
tests/healthEndpoints.test.ts: 404 when disabled;/health200 status-only;/ready503→200 transition through a real render-with-bundle request;/healthstable across bundle state. All pass locally.pnpm run lint,pnpm run type-check,pnpm run build,pnpm start format.listDifferent,script/check-docs-sidebar origin/main,script/check-pro-license-headers: all pass.origin/main(no local redis, streaming timeouts).codex review --base origin/main: two P2 findings (k8s readiness deadlock guidance, changelog placeholder link) — both addressed in follow-up commits.🤖 Generated with Claude Code
Merge Readiness Criteria
Current evaluation as of 2026-06-15 for head
a7368575c1d0fe6c01497013173fe98ca32b9cd4.accelerated-rc, confirmed live from canonical release gate Release gate: react_on_rails 17.0.0 #3823 (Mode: accelerated-rc).a7368575c1d0fe6c01497013173fe98ca32b9cd4.gh pr checks 3939 --repo shakacode/react_on_rails --json bucketreports 45 passing checks and 6 expected path-selected/skipped checks; no pending or failing checks.mergeStateStatusisCLEAN.unresolved=0.truthy()behavioral concern by restoring the helper toorigin/mainbehavior and scoping1parsing toRENDERER_ENABLE_HEALTH_ENDPOINTS; added CodeQL missing-rate-limiting suppressions for/healthand/ready; skipped the optional nativeError{ cause }style cleanup because local type-check rejects it under the package current TS/lib target; acknowledged the non-actionablecreatedApp?.close()note.pnpm --dir packages/react-on-rails-pro-node-renderer exec jest tests/healthEndpoints.test.ts tests/configBuilder.test.ts --runInBand-> 53/53 pass;pnpm --dir packages/react-on-rails-pro-node-renderer run type-check-> pass;node script/generate-llms-full.mjs --check-> pass;pnpm start format.listDifferent-> pass;script/check-docs-sidebar origin/main-> pass;script/check-pro-license-headers-> pass;script/ci-changes-detector origin/main-> Pro node-renderer package path detected;pnpm run build-> pass;pnpm run lint-> pass;git diff --check --no-ext-diff-> pass;(cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop)-> pass/no offenses; branch autoreview againstorigin/main-> clean; pre-commit and pre-push hooks -> pass.full-ci. This remains appropriate because the PR touches Pro node-renderer health/readiness behavior. No benchmark label recommended because the endpoints are opt-in, status-only probes and do not affect render throughput paths.Note
Medium Risk
Touches production deployment probes and readiness semantics (per-worker, bundle-compilation gate); misconfigured readiness on
/readycan deadlock rollouts, though docs call this out and the feature is opt-in.Overview
Pro node renderer gains opt-in built-in
GET /health(liveness, always200when the worker responds) andGET /ready(readiness:503until the answering worker has at least one bundle compiled in its VM pool via newhasAnyVMContext(), then200). Both are off by default and turned on withenableHealthEndpointsorRENDERER_ENABLE_HEALTH_ENDPOINTS(true/yes/1, with1scoped only to this flag).Routes register on the worker Fastify app (like
/info), return status-only JSON, and run beforeconfigureFastify; conflicting custom/healthor/readyroutes get a clearer migration error whenFST_ERR_DUPLICATED_ROUTEmatches.Documentation adds a health-checks guide (h2c /
tcpSocket+execprobes, cold-start and/readytraffic-gating caveats) and updates js-configuration and the docs sidebar; CHANGELOG and llms-full are updated accordingly.Reviewed by Cursor Bugbot for commit df10515. Bugbot is set up for automated code reviews on this repo. Configure here.
Agent Merge Confidence
Mode: accelerated-rc
Current head SHA: df10515
Score: 8/10
Auto-merge recommendation: yes; maintainer-directed merge requested after conflict resolution
Affected areas: React on Rails Pro node renderer health/readiness endpoints, node renderer docs, generated llms reference
CI detector:
script/ci-changes-detector origin/main-> Pro node renderer package; recommends lint, Pro lint/specs/dummy integration, node-renderer tests, and benchmark coverage.Validation run:
node script/generate-llms-full.mjs && node script/generate-llms-full.mjs --check-> passed after conflict resolution.git diff --check-> passed after conflict resolution.Review/check gate:
df105150098f2e7adb6c41cf9a4ee5dc53c7ab3d; 45 passed, 5 expected selector/placeholder skips, 0 failed.Known residual risk: low; node renderer readiness/health endpoint behavior is covered by the package checks, but this remains a release-candidate infrastructure path.
Finalized by: maintainer-directed merge requested by @justin808 in Codex session on 2026-06-15; this is not an autonomous accelerated-RC merge.