feat(rivetkit): add ConnectionMap readonly Map wrapper for actor connections

[abcxff]

duplicate of https://app.graphite.com/github/pr/rivet-dev/rivet/5021

[abcxff]

• fix(workflow-engine): only commit step state after success #5010 : 2 dependent PRs (#5048 , #5052 )
• fix(rivetkit): decode bridge errors with actor: null #5056
• fix(rivetkit): bind methods through createWriteThroughProxy #4987
• feat(rivetkit): add ConnectionMap readonly Map wrapper for actor connections #5047 👈 (View in Graphite)
• fix(rivetkit-core): chunk apply_batch puts/deletes at 128 #5046
• fix(pegboard): validate drain_grace_period < request_lifespan #4998
• chore(rivetkit): log inspector errors + standardize pino errorKey error #4986
• fix(rivetkit): remove async drizzle db proxy #4984 : 2 other dependent PRs (#4985 , #4991 )
• main

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

[claude]

Code Review: ConnectionMap readonly Map wrapper for actor connections

Overview

This PR replaces the eager Map snapshot in ActorContextHandleAdapter.get conns() with a lazy NativeConnectionMap wrapper that implements ReadonlyMap. Each operation on the new wrapper calls actorConns individually rather than once per conns access.

---

Issues

1. Type declaration mismatch (bug)

config.ts:323 still declares conns as the mutable Map<string, Conn<...>>:

// src/actor/config.ts:323
readonly conns: Map<string, Conn<...>>;

The implementation in native.ts now returns ReadonlyMap, which is a subtype of Map — but the public interface type still exposes mutable methods (.set, .delete, .clear). The interface should be updated to ReadonlyMap in the same change to keep the declared API accurate and to prevent callers from accidentally calling mutating methods (which would fail at runtime since NativeConnectionMap does not implement them).

2. Redundant actorConns calls per operation

Every individual map method issues its own callNativeSync(() => this.#runtime.actorConns(this.#ctx)). Code like:

if (ctx.conns.has(id)) {
  const conn = ctx.conns.get(id);
}

...makes two native round-trips where the old snapshot approach made one. The two existing callsites (native.ts:3482 and 3696) use Array.from(actorCtx.conns.values()) which is a single actorConns call, so it's fine for those. But it's a non-obvious semantic change for the general case and worth a note on the class.

3. O(n) linear scan in get() and has()

Both methods fetch all connections and scan by ID:

const conn = conns.find((c) => this.#runtime.connId(c) === key);

This was also true of the old eager Map build (which iterated all connections to construct it), so it's not a regression. But conns.get(id) is now O(n) on every call. Worth noting, or considering a getConn(id) native shortcut if this becomes a hot path.

4. #connMap singleton is never invalidated

The #connMap is allocated on first access and never cleared. This is safe because NativeConnectionMap holds no state itself (it always queries the runtime), but the object lives for the actor's lifetime even for actors that access conns only once. Minor, but worth being aware of.

5. satisfies MapIterator<T> usage is unconventional

return conns.map((c) => this.#runtime.connId(c))[Symbol.iterator]() satisfies MapIterator<string>;

Array.prototype[Symbol.iterator]() returns IterableIterator<T>, which TypeScript accepts as structurally compatible with MapIterator<T>. The satisfies here is a valid compile-time assertion but unconventional — typically the structural check is implicit. Not a correctness issue, just a style note.

---

Positives

• Correct semantic narrowing: ReadonlyMap is the right type for a connections view; actors should not be adding or deleting entries directly.
• Avoids eager allocation: Actors that never call conns (or only call has/get for a single connection) no longer pay to build a full Map snapshot.
• Live view: The new implementation always reflects the current connection set, rather than a snapshot taken at the moment conns was accessed. This is more correct for long-lived references to the context.

---

Suggestions

1. Update config.ts:323 to readonly conns: ReadonlyMap<string, Conn<...>> in this PR.
2. Add a brief comment on NativeConnectionMap noting that each method call issues a fresh actorConns query (i.e., this is a live view, not a snapshot).
3. Consider a test that exercises the live-view property: connection added between two iterator calls shows up in the second.

---

Test coverage

No tests are included for NativeConnectionMap. The existing driver tests will cover the integration path, but a unit test verifying size, get, has, and iteration against a known connection set would improve confidence in the new implementation.

[abcxff]

Merge activity

• May 13, 6:35 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 13, 6:46 PM UTC: Graphite rebased this pull request as part of a merge.
• May 13, 6:47 PM UTC: @abcxff merged this pull request with Graphite.