fix(rivetkit): protect local serverless handler ownership

[NathanFlurry]

No description provided.

[NathanFlurry]

Stack for rivet-dev/rivet

• #5512 fix(actors): surface unregistered actor startup errors
• #5514 fix(rivetkit): protect local serverless handler ownership 👈
• #5513 fix(rivetkit): reject conflicting local envoy connections
• #5510 feat(rivetkit): isolate managed engine storage by port
• #5511 feat(engine-process): support caller-defined engine instance …
• main

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

_{change qvtxsxkn}

[claude]

Review

Solid feature: detecting when a second local project accidentally claims the same serverless pool is a real pain point, and the retry-loop special-casing to fail fast on the new conflict error (instead of retrying it away) is a nice touch.

Correctness

isDev() is a broader gate than "local engine" (src/serverless/configure.ts:143). assertLocalServerlessHandlerOwnership is skipped only when NODE_ENV === "production", but configurePool isn't local-only — it works against any resolved endpoint (self-hosted, Rivet Cloud, staging), not just a locally-spawned engine (see the configurePool requires either endpoint or startEngine check in registry/config/index.ts). Plenty of legitimate non-local environments run with NODE_ENV unset or != "production" (CI, staging, preview deploys on platforms like Vercel/Railway). In those cases this check now runs, does a getRunnerConfig + per-candidate /metadata probe, and — if two independent, legitimately-live deployments happen to share a namespace + pool name with different URLs (e.g. two preview branches on a shared staging namespace) — throws local_serverless_handler_conflict and fails registry startup (this promise is await-ed in registry/index.ts handler()/listen()). The error message itself ("Stop the other project", RIVET_RUN_ENGINE_PORT) reads as local-dev-specific, so the gate should probably be something more precise like getRivetRunEngine() (explicit "we spawned a local engine" signal) or a check that the resolved endpoint is loopback, rather than isDev().

Unencoded URL falls through to a confusing retry loop instead of failing fast. normalizeHandlerUrl/new URL(existingUrl) (src/serverless/configure.ts:181-186) will throw if requestedUrl or a URL previously stored in the engine's runner config isn't a valid absolute URL. That throw isn't a RivetError with local_serverless_handler_conflict, so it falls into the generic catch in configureServerlessPool's retry loop (the error instanceof RivetError && ... check doesn't match) and gets silently retried/logged as a warning until the overall timeout instead of surfacing the real config problem immediately.

Design / minor

• Check-then-write race: two processes starting at nearly the same instant can both pass assertLocalServerlessHandlerOwnership (neither's URL is registered yet) and then both call updateRunnerConfig, still overwriting each other. Probably fine as a best-effort dev guard, but worth a short comment noting it's best-effort rather than a hard lock, since the PR's framing ("protect... ownership") could otherwise read as a guarantee.
• getRunnerConfig's encodeURIComponent(name) fix is good and matches the pattern used elsewhere in api-endpoints.ts. While in the area: updateRunnerConfig's /runner-configs/${runnerName} a few lines down has the same missing-encoding gap (pre-existing, not introduced here, but same root cause and now inconsistent with the sibling getRunnerConfig call in the same file).

Test coverage

The new test file only covers normalizeHandlerUrl. The actual new behavior — dev-only gating, same-URL skip, live-vs-dead handler probing, and the conflict throw/rethrow special-casing in configureServerlessPool's catch block — has no coverage. Per CLAUDE.md's testing guidance (no vi.mock, prefer real infrastructure), this seems testable without mocking: spin up a tiny real HTTP server standing in for the engine's /runner-configs GET and another for the "other project"'s /metadata endpoint, then assert assertLocalServerlessHandlerOwnership throws/doesn't throw across the branches (same URL, dead handler, live handler, non-dev).

Nit

The line if (normalizeHandlerUrl(existingUrl) === normalizeHandlerUrl(requestedUrl)) { in assertLocalServerlessHandlerOwnership looks like it may exceed the project's usual line width — worth a pass through the formatter (node scripts/format/agent-format.mjs) if not already run.