Walkthrough

Adds a public host configuration (default from RENDERER_HOST or 'localhost') and uses it when starting the Fastify worker so the renderer binds to the configured host and port.

Changes

Sequence Diagram(s)

sequenceDiagram
  participant Starter as Starter
  participant Config as ConfigBuilder
  participant Server as FastifyServer
  participant Logger as Logger

  Starter->>Config: getConfig()
  Config-->>Starter: config { port, host, ... }
  Starter->>Server: app.listen({ port, host })
  Server-->>Logger: onListening(host, port)
  Logger-->>Starter: log "listening on host:port"

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through configs, found a spot,
Added host so bind's not forgot.
From localhost or open shore,
Containers ping and servers roar—
🥕 a tiny hop, outputs a lot!

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 and usage tips.}

Greptile Summary

This PR adds a configurable host binding option to the Fastify-based node renderer worker, defaulting to 'localhost' (overridable via RENDERER_HOST env var). The change restores parity with the previous Express-based worker's 0.0.0.0 flexibility and unblocks external health checks in containerized environments (Docker, ECS with ALB).

Key changes:

• Config interface gains a host: string field with inline documentation
• defaultConfig reads RENDERER_HOST env var, falling back to 'localhost'
• worker.ts threads host into app.listen({ port, host }) and updates the startup log line to host:port

Issues found:

• RENDERER_HOST is not included in the envValuesUsed() function, so it will not appear in the startup configuration log output — a minor observability gap compared to every other env-backed setting
• The Config interface carries a comment reminding contributors to update ./docs/node-renderer/js-configuration.md when the interface changes; the docs file does not appear to be updated in this PR

Confidence Score: 4/5

• This PR is safe to merge; the change is minimal, well-scoped, and the default preserves existing behaviour.
• The logic change is small and correct — host is plumbed through cleanly from config to Fastify's listen call. The only gaps are a missing envValuesUsed() entry (affects startup-log observability, not runtime behaviour) and an un-updated docs file. Neither blocks correctness.
• packages/react-on-rails-pro-node-renderer/src/shared/configBuilder.ts — missing RENDERER_HOST in envValuesUsed() and docs not updated.

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant Operator as Operator / Container Env
    participant Config as configBuilder.ts
    participant Worker as worker.ts
    participant Fastify as Fastify Server

    Operator->>Config: Set RENDERER_HOST (e.g. 0.0.0.0) or leave default
    Config->>Config: defaultConfig.host = env.RENDERER_HOST || 'localhost'
    Worker->>Config: buildConfig(userConfig)
    Config-->>Worker: getConfig() → { port, host, ... }
    Worker->>Fastify: app.listen({ port, host })
    Fastify-->>Worker: Callback: listening on host:port
    Worker->>Worker: log.info("Node renderer … listening on host:port!")

    Note over Operator,Fastify: With host='0.0.0.0', external health checks<br/>(ALB, Docker) can reach the Fastify server

_{Last reviewed commit: 84b2b7b}

test comment please ignore

PR Review: The implementation is clean and correct — passing { port, host } to Fastify app.listen() is the right approach, and the default of localhost preserves existing behavior safely. Two issues: (1) Missing documentation update — configBuilder.ts line 23 has a comment reminding contributors to update js-configuration.md when config changes. The new host option backed by RENDERER_HOST was not documented there. (2) Missing CHANGELOG entry — this is a new user-facing configuration option that warrants an entry under Unreleased > Added in CHANGELOG.md.

size-limit report 📦

PR Review

The implementation is clean and correct — passing { port, host } to Fastify's app.listen() is the right approach, and the default of 'localhost' preserves existing behavior safely.

Two issues to address:

Missing documentation update

configBuilder.ts has an explicit instruction at line 23:

/* Update ./docs/node-renderer/js-configuration.md when something here changes */

The js-configuration.md file enumerates every config option with its env var, but the new host option (backed by RENDERER_HOST) was not added. A line like the following should appear after the port entry:

1. **host** (default: `process.env.RENDERER_HOST || 'localhost'`) - The host/IP address the renderer binds to. Set to `0.0.0.0` in containerized environments (Docker, ECS with ALB) where external health checks need to reach the server.

Missing CHANGELOG entry

This is a new user-facing configuration option. It warrants an entry under [Unreleased] > Added in CHANGELOG.md.

Review

The implementation is clean, correct, and addresses all the concerns raised by earlier reviewers. Specifically:

• ✅ host field added to Config interface with clear inline docs
• ✅ Default sourced from RENDERER_HOST env var with safe 'localhost' fallback
• ✅ envValuesUsed() now includes RENDERER_HOST (startup diagnostics will report the configured bind address)
• ✅ app.listen({ port, host }) is the correct Fastify API
• ✅ Documentation updated in js-configuration.md

One remaining gap: no CHANGELOG entry

This is a new user-facing configuration option for the Pro node renderer. Per the project's changelog guidelines, it should have an entry under [Unreleased] > #### Pro > Added in CHANGELOG.md, e.g.:

- **Configurable renderer host binding (Pro)**: Added `host` config option (default: `'localhost'`, env: `RENDERER_HOST`) to the Node Renderer Fastify worker. Set to `0.0.0.0` in containerized environments (Docker, ECS with ALB) to allow external health checks. [PR 2585](https://github.com/shakacode/react_on_rails/pull/2585) by [justin808](https://github.com/justin808).

Otherwise ready to merge once the CHANGELOG entry is added.

Overall this is a clean, well-scoped change. The default of 'localhost' preserves backward compatibility, the docs include a good security caution about 0.0.0.0 exposure, and the envValuesUsed() reporting is consistent with existing patterns.

Issue: Log message shows configured host, not actual bound address

In worker.ts, the app.listen callback form receives (err, address) as arguments where address is the fully-resolved string Fastify actually bound to. The current code ignores those callback params and logs the configured host:port values instead. This can be misleading when 'localhost' resolves to IPv6 (e.g. [::1]:3800). Using the address argument from the callback would make the log accurate regardless of how the OS resolves the hostname.

Minor: No validation of the host string

Invalid values (e.g. RENDERER_HOST=not-a-valid-host) will fail at runtime when Fastify tries to bind. Consistent with how port is handled, so acceptable — the failure is fast and visible at startup.

Minor: envValuesUsed() falsy check

!userConfig.host would also be truthy for an explicitly empty string host, causing the env variable to be reported as in use when it is not. Same pre-existing pattern as port, not a new issue introduced here.

Addressed in commit 4659dcf.

Added a new Pro changelog entry under Unreleased -> Pro -> Added in CHANGELOG.md for the configurable node-renderer host option (RENDERER_HOST / default localhost), including containerized 0.0.0.0 guidance.

This addresses the missing changelog feedback noted in:

• Add configurable host option to node renderer Fastify worker #2585 (comment)
• Add configurable host option to node renderer Fastify worker #2585 (comment)
• Add configurable host option to node renderer Fastify worker #2585 (comment)

Overall this is a clean, minimal, well-motivated change. The security documentation in the docs page is a nice touch.

What looks good

• Consistent with the port pattern — the host field follows exactly the same default/env-override/diagnostic pattern already used for port, making it easy to understand.
• Safe default — keeping 'localhost' as the default preserves existing behavior and avoids accidental exposure on upgrade.
• Security caution in docs — the warning about 0.0.0.0 exposing all interfaces is important and well-placed.
• Log improvement — showing host:port instead of just port is strictly more useful.

One issue worth addressing

Left an inline comment on the app.listen callback (line 465 of worker.ts). The callback ignores its err argument, which is a pre-existing gap, but it becomes more relevant now that users can supply arbitrary hosts. An invalid or un-bindable host will fail silently. The fix is straightforward — see the suggestion on the inline comment.

Review

Overall this is a clean, well-scoped change. The default of localhost is safe (preserves existing behavior), the security documentation is clear, and the improved error handling in the app.listen() callback is a nice bonus fix.

Strengths

• Safe default: 'localhost' keeps existing bind behavior unchanged for all current deployments.
• Security docs: The caution about 0.0.0.0 exposure and mitigations (private networking, firewall/ALB, password auth) are clearly documented.
• Error handling improvement: The new (err, address) callback correctly handles startup failures with process.exit(1) — the old callback silently dropped errors.
• Logging improvement: Using address from the callback instead of interpolating port means the logged address reflects what Fastify actually bound to (including the resolved host).

Minor issues

1. Error message missing context (inline on worker.ts:467): When startup fails, the log does not include which host/port was attempted, making it harder to triage a misconfigured RENDERER_HOST. Suggestion left inline.

2. No startup warning for non-localhost without password: If a user sets host: '0.0.0.0' but has not configured password, the renderer is network-accessible with no authentication. A log.warn(...) at startup would reinforce the security note in the docs and catch misconfiguration early.

3. Falsy fallback behavior (inline on configBuilder.ts:150, informational): RENDERER_HOST='' silently falls through to 'localhost'. Not a bug, just worth knowing — consistent with how RENDERER_PORT is handled.

None of these block merging — items 1 and 3 are minor and item 2 is a nice-to-have.

Overall this is a clean, well-scoped change. The default of localhost preserves current behaviour with zero risk to existing deployments; the security caution in the docs is clear and actionable.

Overall this is a clean, well-scoped change. The default of localhost preserves current behaviour with zero risk to existing deployments; the security caution in the docs is clear and actionable.

Improvement: Adding error handling to the app.listen callback is a genuine improvement -- previously a bind failure would silently pass through. Logging the resolved address from the callback (rather than configured strings) also correctly handles IPv6 address formatting.

Minor -- envValuesUsed falsy check (see inline): The host: empty-string case is treated as not-set, consistent with the existing port pattern, but checking whether the host key exists in userConfig (same guard as supportModules/stubTimers) would be strictly correct.

Minor -- no host validation: An invalid host string now fails loudly thanks to the new process.exit(1) path, but an upfront format check would give a clearer error message before Fastify even attempts to bind.

Both are minor; the current implementation is functionally correct for all realistic inputs.

Overall this is a clean, well-scoped change. The core implementation is solid: error handling in the app.listen callback is a genuine improvement over the previous fire-and-forget pattern, structured Pino log fields follow best practices, and using the resolved address from the callback for the success log correctly handles IPv6 address resolution. The security caution in the docs is appropriate.

envValuesUsed() inconsistency (configBuilder.ts line 203): RENDERER_HOST uses the in operator (key existence) while all other fields use falsy checks like !userConfig.port. This is more semantically correct for host, but inconsistent with the surrounding code, and the accompanying test only exercises an empty-string edge case. See inline comment.

Test coverage gaps: (1) The test passing host as an empty string uses an invalid bind address; a real value like 127.0.0.1 makes the intent clearer. (2) There is no test for the fully-default path (no RENDERER_HOST env var, no host key in userConfig -> defaults to localhost). (3) The jest.requireActual + jest.doMock pattern is correct and matches licenseValidator.test.ts, but a short comment on why requireActual is used would help future readers.

No host validation (nice-to-have): an invalid host from an env var typo will reach app.listen without early feedback. The new error handler in worker.ts catches this at startup and exits, which is acceptable. A guard in buildConfig similar to the maxVMPoolSize check would give a faster and more descriptive error, but this is optional.

None of these block merging - the feature is correct and the conservative default of localhost is safe.