[SLOP(gpt-5)] fix(run): split engine host and port env from endpoint

[NathanFlurry]

No description provided.

[NathanFlurry]

Stack for rivet-dev/rivet

• [SLOP(gpt-5)] fix(run): split engine host and port env from endpoint #5218 ykzlmptz · 2026-06-11 08:44:46 👈
• main

Get stack: forklift get 5218
Push local edits: forklift submit
Merge when ready: forklift merge 5218

[railway-app]

🚅 Deployed to the rivet-pr-5218 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
website	😴 Sleeping (View Logs)	Web	Jun 11, 2026 at 8:55 am
kitchen-sink	❌ Build Failed (View Logs)	Web	Jun 11, 2026 at 8:52 am
frontend-inspector	❌ Build Failed (View Logs)	Web	Jun 11, 2026 at 8:47 am
mcp-hub	✅ Success (View Logs)	Web	Jun 11, 2026 at 8:47 am
frontend-cloud	❌ Build Failed (View Logs)	Web	Jun 11, 2026 at 8:47 am
ladle	❌ Build Failed (View Logs)	Web	Jun 11, 2026 at 8:46 am

[claude]

Code Review: PR #5218 — Split engine host and port env from endpoint

Overall this is a clean, well-motivated change. The core idea is correct: RIVET_ENDPOINT (what RivetKit connects to) and the bind address of the spawned local engine are separate concepts, and conflating them prevented scenarios like binding on 0.0.0.0 or a non-default port. The implementation is largely straightforward, but there are several issues ranging from a definite compile error down to style and design nits.

---

Critical — Compile Error

engine_process.rs inline test test_config is missing the new struct fields

rivetkit-rust/packages/rivetkit-core/src/engine_process.rs, lines ~498–506:

EngineResolverConfig does not derive or implement Default, so Rust struct literal syntax requires all fields. The new bind_host and bind_port fields added to the struct are absent from the test_config helper. Fix: add bind_host: None, bind_port: None to the struct literal in test_config.

---

Major — Policy Violation: vi.mock in Tests

tests/registry-constructor.test.ts, line 6:

CLAUDE.md explicitly states: "Never use vi.mock, jest.mock, or module-level mocking."

The new buildServeConfig test mocks @rivetkit/engine-cli to inject a binary path. Options to fix without module mocking:

1. Test RegistryConfigSchema.parse(...) at the schema transform level — the schema-level assertions on engineHost/enginePort are the valuable part and don't require calling buildServeConfig.
2. Inject the engine CLI path as a parameter instead of resolving it via a mocked module import.

---

Bug — Dead Code in isLocalEngineEndpoint (IPv6 bracket checks)

src/common/engine.ts:

The checks for "[::]" and "[::1]" are dead code. The WHATWG URL API normalizes IPv6 addresses by stripping brackets in url.hostname — for new URL("http://[::]:6420"), url.hostname is "::", not "[::]". The bracket forms will never match. Remove them.

---

Bug — parseInt on RIVET_RUN_ENGINE_PORT does not guard against NaN

src/utils/env-vars.ts:

If RIVET_RUN_ENGINE_PORT is set to a non-numeric string, parseInt returns NaN, which propagates as a number. Zod's .min(1).max(65_535) comparisons with NaN both return false, so the value passes schema validation silently with a nonsensical port. The Rust side handles this correctly with .and_then(|v| v.parse().ok()). Fix:

const parsed = parseInt(value, 10);
return Number.isNaN(parsed) ? undefined : parsed;

---

Design — Dev-mode endpoint fallback ignores engineHost/enginePort

src/registry/config/index.ts:

When startEngine is false and no explicit endpoint is provided, the dev-mode fallback uses hardcoded ENGINE_HOST/ENGINE_PORT (127.0.0.1:6420) instead of the configured engineHost/enginePort. A user who sets RIVET_RUN_ENGINE_PORT=7654 without enabling RIVET_RUN_ENGINE=1 will find that enginePort only affects the spawn bind address, not the auto-detected dev-mode connection endpoint.

If the intent is that engineHost/enginePort strictly control the spawn bind address (not the connection endpoint), the names are confusing — bindHost/bindPort would communicate that more clearly, and a comment explaining the distinction would help.

---

Minor — Regex is more permissive than intended

src/common/engine.ts:

/^127(?:\.\d{1,3}){0,3}$/.test(hostname) — {0,3} accepts "127" alone or "127.1" as loopback, and \d{1,3} matches octets up to 999. Consider {3} instead of {0,3} for a proper four-octet check.

---

Minor — Naming inconsistency: bind_host/bind_port vs engine_host/engine_port

The Rust EngineResolverConfig uses bind_host/bind_port while ServeConfig and JsServeConfig use engine_host/engine_port. Within the same crate this adds cognitive overhead. bind_host/bind_port is more precise; at minimum the mapping site in registry/mod.rs deserves a comment noting that engine_host becomes the spawn bind host.

---

Summary

#	Severity	File	Issue
1	Compile error	rivetkit-rust/.../engine_process.rs	test_config missing bind_host/bind_port fields
2	Policy	rivetkit-typescript/.../registry-constructor.test.ts:6	vi.mock is prohibited by CLAUDE.md
3	Bug	rivetkit-typescript/.../engine.ts	Dead "[::]" / "[::1]" checks (URL strips brackets)
4	Bug	rivetkit-typescript/.../env-vars.ts	parseInt returns NaN on bad input; not guarded
5	Design	rivetkit-typescript/.../config/index.ts	Dev-mode fallback ignores engineHost/enginePort
6	Nit	rivetkit-typescript/.../engine.ts	Regex accepts partial/out-of-range 127.x addresses
7	Minor	Rust layer	Naming mismatch: bind_host vs engine_host for same concept