test(rivetkit): re-enable gateway URL and direct-registry coverage

[NathanFlurry]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix(test): stabilize lifecycle, sleep, queue, and run edge cases #4660 : 2 dependent PRs (#4664 , #4665 )
• test(rivetkit): re-enable gateway URL and direct-registry coverage #4659 👈 (View in Graphite)
• chore(engine): publish engine bases in ci #4649 : 2 other dependent PRs (#4650 , #4652 )
• chore: lockfile lefthook #4647
• fix(rivetkit): surface native sqlite kv errors to callers #4645
• fix(rivetkit): prevent sleep races during disconnect and db work #4644
• fix(rivetkit): isolate engine envoys and propagate startup failures #4643
• test(rivetkit): remove wasm sqlite-only fixtures and coverage #4642
• chore(sqlite-vfs): remove wasm sqlite packages and workspace wiring #4641
• chore(rivetkit): remove wasm sqlite runtime #4614
• fix(sqlite-native): restore native startup kv preload #4639
• fix(sqlite-native): delete metadata before chunk range #4638
• fix(sqlite-native): keep truncate cache coherent #4637
• fix(sqlite-vfs): use delete range for truncate cleanup #4636
• fix(sqlite-native): restore kv error hook #4635
• perf(sqlite-native): avoid cloning cached read chunks #4634
• perf(sqlite-native): gate kv operation labels #4633
• perf(sqlite-native): reuse read cache for partial writes #4632
• perf(sqlite-native): remove delete file existence probe #4631
• fix(sqlite-native): gate native read cache behind env var #4630
• fix(pegboard): isolate runner config dc lookup failures #4625 : 1 other dependent PR (#4626 )
• docs(pegboard): note runner config upsert split write risk #4624
• fix(pegboard): unpack global runner config keys correctly #4623
• fix(pegboard-gateway): enforce tunnel message state #4622
• fix(pegboard-runner): clear terminal tunnel routes #4621
• fix(pegboard): skip protocol version keys in runner pool backfill #4620
• fix(pegboard): create missing normal runner configs #4619
• fix(epoxy): reuse cached optimistic value #4618
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review (updated): test(rivetkit): re-enable gateway URL and direct-registry coverage

This is an updated review reflecting the latest changes.

Summary

This PR re-enables two test suites (raw-http-direct-registry and raw-websocket-direct-registry), rewrites driver-engine-ping.test.ts to use the serverless runner pattern, adds a waitForClientWarmup readiness barrier, adds retry logic to refreshRunnerMetadata, re-enables the dynamic registry test variant, and switches engine driver tests to describe.sequential.

---

Significant: Silent swallow of refreshRunnerMetadata errors

In driver-engine.test.ts (~line 196), all errors from refreshRunnerMetadata are caught and discarded. A permanent failure (wrong token, misconfigured endpoint) will be invisible and any downstream test failure will be misattributed. Consider logging the caught error at warn level so the CI trace retains a breadcrumb without blocking setup.

Significant: Non-null assertion on serverlessHandleStart

In src/test/mod.ts (~line 168) and driver-engine-ping.test.ts, the call actorDriver.serverlessHandleStart!(ctx) will throw an unhelpful TypeError if the driver does not implement this method. A guard returning HTTP 501 with a clear message would make failures immediately actionable.

Moderate: Duplicated helpers across test files

refreshRunnerMetadata and closeNodeServer appear in src/test/mod.ts, driver-engine.test.ts, and driver-engine-ping.test.ts. waitForOpen appears in driver-engine-ping.test.ts and raw-websocket-direct-registry.ts with differing listener cleanup quality (ping file properly cleans up; WebSocket file uses once:true without cleaning the other paths). buildGatewayRequestUrl appears in raw-http-direct-registry.ts and driver-engine-ping.test.ts with differing double-prefix guards. These should be consolidated into a shared tests/engine-test-utils.ts.

Moderate: serverless config block duplicated verbatim

The serverless runner config block is duplicated between src/test/mod.ts (~line 185) and driver-engine-ping.test.ts (~line 1665). Extracting it into a shared constant prevents the copies from diverging silently.

Minor: waitForClientWarmup uses a fixed key across parallel tests

The constant key driver-test-warmup works today because each test has its own namespace/pool, but a per-test ID would make isolation explicit and robust against future config changes.

Minor: Retry sleep uses raw setTimeout instead of antiox

Per CLAUDE.md, antiox is the preferred TypeScript concurrency primitive. The retry loops in refreshRunnerMetadata use raw setTimeout directly.

Minor: Missing conn_params test in the HTTP gateway suite

raw-http-direct-registry.ts does not include a conn_params test (the old code had one via HEADER_CONN_PARAMS). The WebSocket suite does test conn params. If the HTTP gateway path supports conn_params, this coverage gap should be tracked.

---

Positive Observations

• waitForClientWarmup with a ping() action on a dedicated warmup actor is a much cleaner readiness barrier than the previous setTimeout(250).
• describe.sequential for the engine driver suite is correct for tests that share a single engine process.
• buildGatewayWebSocketUrl in raw-websocket-direct-registry.ts properly handles query-string passthrough.
• AbortSignal.timeout(2_000) per retry attempt in refreshRunnerMetadata prevents a single stalled request from consuming the full retry budget.
• Re-enabling the dynamic registry variant closes genuine coverage that was dark.
• Namespace creation with 409-tolerance in ensureNamespaceExists is correct for idempotent test setup.

Review generated by Claude Sonnet 4.6

[claude]

PR Review: test(rivetkit): re-enable gateway URL and direct-registry coverage

This PR re-enables two previously-disabled driver test suites (runRawHttpDirectRegistryTests, runRawWebSocketDirectRegistryTests), re-enables the dynamic registry variant in driver-engine.test.ts, refactors the setupTest helper in src/test/mod.ts from the old Runtime-based approach to the new EngineActorDriver/serverless architecture, and adds a warmup probe actor to make test readiness more reliable. The overall direction is correct.

---

Issues

1. Code duplication across three files (medium)

buildGatewayRequestUrl and buildGatewayWebSocketUrl are independently defined in three places:

• tests/driver-engine-ping.test.ts
• src/driver-test-suite/tests/raw-http-direct-registry.ts
• src/driver-test-suite/tests/raw-websocket-direct-registry.ts

Similarly, waitForOpen, closeNodeServer, and refreshRunnerMetadata are duplicated across test files and now also in src/test/mod.ts. These should be extracted into a shared test utility module to prevent implementations drifting out of sync.

2. Inconsistent buildGatewayRequestUrl implementations (low-medium)

The version in raw-http-direct-registry.ts has a defensive guard against double-prefixing (startsWith("request/")) that the driver-engine-ping.test.ts version lacks. These should share one implementation.

3. Errors silently swallowed in driver-engine.test.ts (low)

The refreshRunnerMetadata catch block is empty aside from a comment. Per CLAUDE.md fail-by-default convention, either log the error or remove the call if the warmup probe is the true readiness barrier. Completely swallowing errors makes failures harder to diagnose.

4. Non-null assertion on optional method without guard (low)

In src/test/mod.ts:

return await actorDriver.serverlessHandleStart!(ctx);

The ! is used without an invariant guard, while driver-engine.test.ts correctly validates the method exists before calling it. Add a guard or use a consistent pattern.

5. waitForOpen rejects with event object instead of Error (low)

In raw-websocket-direct-registry.ts:

ws.addEventListener("error", reject, { once: true });
ws.addEventListener("close", reject, { once: true });

This passes the raw event (not an Error) as the rejection reason, producing poor failure messages. The driver-engine-ping.test.ts version wraps these into proper Error objects with descriptive messages. The test suite version should match.

6. Fixed warmup key shared across sessions (low)

The warmup probe uses the hardcoded key ["driver-test-warmup"]. If tests ever share a namespace in CI this could cause cross-test interference. A per-test UUID key would be more isolated and consistent with how other actors are keyed throughout these tests.

7. max_concurrent_actors not in RegistryConfigRequest (pre-existing, worth fixing here)

driver-engine.test.ts passes max_concurrent_actors: 10000 in the serverless config, which is not a field in the RegistryConfigRequest.serverless interface. The new test files correctly use headers: {} and runners_margin: 0. The old call site should be updated to match.

8. describe label mismatch (cosmetic)

runRawHttpDirectRegistryTests uses describe("raw http - gateway query urls", ...) and the websocket equivalent similarly labels itself "gateway query urls" -- not matching the function names containing "direct-registry". Consider aligning names to avoid confusion in test output.

---

Positives

• The waitForClientWarmup retry loop (6 attempts x 1 s) is a solid replacement for the previous fixed setTimeout(250).
• Switching refreshRunnerMetadata to a retry loop with AbortSignal.timeout(2_000) per attempt is a meaningful improvement.
• describe.sequential in driver-engine.test.ts is appropriate for tests sharing engine state.
• closeNodeServer correctly calls closeIdleConnections + closeAllConnections to avoid hangs from keep-alive connections.
• Per-test UUID namespacing and onTestFinished cleanup ordering (client dispose -> driver shutdown -> server close) are correct.
• ensureNamespaceExists correctly treats HTTP 409 as success.
• buildGatewayWebSocketUrl in raw-websocket-direct-registry.ts properly preserves query strings when appending subpaths.