Skip to content

feat(actor): add actors namespace with subscribe/send - #212

Merged
ImriKochWix merged 52 commits into
mainfrom
feat/realtime-handler
Jul 29, 2026
Merged

feat(actor): add actors namespace with subscribe/send#212
ImriKochWix merged 52 commits into
mainfrom
feat/realtime-handler

Conversation

@ImriKochWix

@ImriKochWix ImriKochWix commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

PR B of 3 for the Actor feature — the client SDK surface.

  • New src/modules/actors.tscreateActorsModule returns { module, closeAll }. module is a Proxy so base44.actors.GameRoom(instanceId) works for any deployed actor name; it returns a room handle:
    • .connect(opts?) — opens one PartySocket (idempotent). The client picks its own conn id (opts.id, else crypto.randomUUID()); it becomes the actor's conn.id, so a reconnect with the same id keeps the same server-side identity.
    • .subscribe(cb){ unsubscribe }; .send(data); .close().
    • Tokenless connect — the existing user access token (or nothing, anonymous) rides the WS query; the platform proxy authenticates it. No pre-connect token mint.
    • Half-open detection: pings every 1s, force-reconnects if nothing arrives in 3s; the query is re-read on each reconnect so a login/logout is picked up.
    • The Proxy get returns undefined for then/symbol keys so the module isn't mistaken for a thenable.
    • closeAll() (internal) closes every live room; client.cleanup() calls it so a forgotten room.close() doesn't leak its heartbeat timer.
  • New src/actor.ts — type-only Actor<Incoming, Outgoing> base class + Conn/Storage/client types (the bundler swaps the runtime at deploy).
  • New src/modules/actors.types.tsActorRegistry (hand-authored message types) and ActorNameRegistry (auto-generated by base44 types generate), plus ActorRoom/ActorSubscription/ActorConnectOptions.
  • src/client.ts — wires the actors module. The socket host is resolveActorsHost(serverUrl, window?.location?.origin): an absolute serverUrl is used as-is, and an empty/relative serverUrl (same-origin apps use a relative /api) falls back to the page origin. PartySocket owns the scheme (httpswss, ws for localhost) and trailing-slash strip, so there's no manual URL munging.
  • package.json — adds partysocket. Requires Node 22+ / a browser (global WebSocket + crypto.randomUUID).

Depends on

PR A deployed to preview (the app-origin /parties proxy + Dispatcher WS handling must be live).

🤖 Generated with Claude Code

Add a `realtime` module to the Base44 JS SDK that lets users subscribe
to and send messages to Cloudflare Durable Object-backed RealtimeHandlers
deployed by the Base44 platform. Uses PartySocket for WebSocket transport
with automatic token refresh on reconnect.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.40-pr.212.6649074

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/sdk@0.8.40-pr.212.6649074"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.40-pr.212.6649074"
  }
}

Preview published to npm registry — try new features instantly!

partysocket was added to package.json but lock file was never generated.
Also fixes .npmrc: was using env-var syntax (npm_config_registry=...)
instead of npmrc syntax (registry=...).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

- subscribe() now returns a sync unsubscribe function instead of Promise<RealtimeSubscription>
- send() is typed via RealtimeHandlerRegistry (user-declared message types)
- Add RealtimeHandlerNameRegistry for CLI codegen (no conflict with user augmentation)
- Drop RealtimeSubscription in favor of the simpler sync cleanup pattern

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

…et URL

partysocket@0.0.23's updateProperties only falls back for host/room/path,
not party. Passing {query:{token}} dropped party, changing the URL from
/parties/ChatRoom/room to /party/room.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

Don't connect until we have a token — avoids initial tokenless
connection being rejected and the updateProperties/reconnect timing race.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

Expose this.storage (DO KV) and onStart() lifecycle hook so handlers
can persist and load state. Both are backed by the compiled shim at
runtime; the stub implementations throw to surface misuse in local
dev/test outside a deployed context.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

partysocket only reconnects on a browser close/error event, so a silently
dead connection (TCP alive, no data — common behind proxies/LBs) hung until
the OS idle timeout (~60s), freezing the client. Add a ping/watchdog:
send {"type":"__ping"} every 5s and force ws.reconnect() if nothing arrives
for 12s, cutting detection from ~60s to seconds. __pong acks are swallowed.
Pairs with the handler shim answering __ping so idle handlers stay proven.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

12s was too long for interactive apps (a frozen game). Realtime handlers
push frequently, so ~2-3s of total silence reliably means a dead socket.
Recover in ≤3s instead of ≤12s.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

subscribe() now returns { id, unsubscribe } instead of a bare unsubscribe fn,
and accepts options.id to control the connection id (stable across
reconnects/tabs if supplied, else auto-generated per connection). id matches
the handler's conn.id, so clients can identify themselves without a server
your_id message.

BREAKING: subscribe() now returns RealtimeSubscription ({ id, unsubscribe() })
instead of a bare () => void; call sub.unsubscribe() instead of the old sub().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ImriKochWix
ImriKochWix force-pushed the feat/realtime-handler branch from d5416b2 to d9732eb Compare July 5, 2026 08:26
@claude

claude Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

Add protected tickIntervalMs + optional shouldTick() so handlers get types for
the platform-managed tick loop (implemented in the deployed shim). Opting in
means no more startLoop/stopLoop bookkeeping in the handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Analyze code quality & best practices
  • Check for bugs & issues
  • Review performance & security
  • Assess test coverage

Clean, well-documented iteration. The room-handle API reads nicely, the type registry (ActorRegistry / ActorNameRegistry) is a great DX touch, and src/actor.ts is a tidy type-only base. Below are the remaining items — the top one is the same functional risk flagged in prior passes.


🟠 Design / lifecycle

1. No cleanup wiring for actor rooms — the main functional risksrc/client.ts:231-236
userModules.cleanup() disconnects the entities socket + analytics but doesn't touch actor rooms. Each connect() starts a setInterval heartbeat (actors.ts:84) that fires every second forever if the app forgets room.close(). Since createActorsModule now owns per-client state, have it track live Rooms and expose a closeAll() that cleanup() invokes:

// actors.ts
export function createActorsModule(config: ActorsConfig) {
  const rooms = new Set<Room>();
  const module = new Proxy(/* ... */ {
    get(_, actorName: string) {
      if (typeof actorName !== "string") return undefined; // see #4
      return (instanceId: string) => {
        const room = new Room(actorName, instanceId, config);
        rooms.add(room); // and remove on close()
        return room as unknown as ActorRoom;
      };
    },
  });
  // attach closeAll() for client.cleanup() to call
}

2. send() can throw before the socket is OPENsrc/modules/actors.ts:113-118
connect() returns synchronously but the socket opens async, so room.connect().send(...) in the same tick calls ws.send before OPEN. The doc at actors.types.ts:73 says "buffered by the socket until open" — PartySocket only buffers once its internal socket exists, so an immediate send can still throw. Note the heartbeat's ws.send is wrapped in try/catch (:90-97) but the public send() isn't. Either guard readyState/buffer, or soften the doc.

3. Heartbeat is aggressivesrc/modules/actors.ts:21-22
PING_MS = 1_000 / DEAD_MS = 3_000 → one ping/sec per room. With many concurrent rooms that's a lot of wakeups and prevents Durable Objects from hibernating (cost). The rationale comment at :19-20 is good; consider making the interval configurable or relaxing it if second-level half-open detection isn't required.


🟡 Minor

4. Proxy get returns a factory for every keysrc/modules/actors.ts:132-135
The Proxy hands back a room factory for any property access, including then (which would make the actors object look thenable if ever awaited) and symbol keys. Add a guard: if (typeof actorName !== "string") return undefined;.

5. close() leaves connId stalesrc/modules/actors.ts:120-128
After close(), this.ws = null but this.connId is retained, so .id returns the old id between close and a subsequent connect(). A connect() after close with no id mints a fresh id but doesn't clear the old one first. Confirm the reconnect-after-close semantics are intended (arguably close() should null connId).

6. crypto.randomUUID() vs uuidv4()src/modules/actors.ts:47
Safe given the Node≥22 baseline, but uuid is already a dependency (src/utils/axios-client.ts:3). uuidv4() would be more consistent. Purely stylistic.


🔒 Security

Nothing concerning. The token rides the WS query and is re-read on each (re)connect via the query() closure (actors.ts:56-64) so login/logout is picked up; anonymous connects correctly omit it (covered by test at actors.test.ts:110). Precedence token || getAccessToken() (client.ts:229) matches the functions module — good.


🧪 Test coverage

Solid baseline in tests/unit/actors.test.ts — Proxy dispatch, idempotent connect, throw-before-connect, multi-listener + per-listener unsubscribe, __pong swallowing, send serialization, close teardown, anonymous connect, per-id independence. Gaps worth adding:

  • Heartbeat (fake timers): assert __ping is sent every PING_MS, and a stale lastMsg triggers reconnect() after DEAD_MS. FakeSocket already stubs reconnect(), so this is easy.
  • functionsVersionfv: assert it flows into the query when set and is omitted when unset (actors.ts:62).
  • resolvedActorsWsUrl derivation (client.ts:88-98): https→wss, http→ws, trailing-slash strip, and appBaseUrl/window/serverUrl precedence — currently untested.
  • closeAll() once SDK Improvements #1 lands: cleanup clears heartbeats/sockets.

Blocker I'd flag: cleanup wiring (#1) — a forgotten close() leaks a 1s timer permanently. Everything else is polish/optional. Nice work overall. 👍

View jobfeat/realtime-handler

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Analyze code quality & best practices
  • Check for bugs & issues
  • Review performance & security
  • Assess test coverage

Clean, well-documented feature. The room-handle API reads nicely, the type registry (ActorRegistry / ActorNameRegistry) is a great DX touch, and src/actor.ts is a tidy type-only base. Below are the remaining items — the top one is the same functional risk flagged across prior passes and is still unaddressed.


🟠 Design / lifecycle

1. No cleanup wiring for actor rooms — the main functional risksrc/client.ts:231-236 + actors.ts:130-136
userModules.cleanup() disconnects the entities socket + analytics but never touches actor rooms. Each connect() starts a setInterval heartbeat (actors.ts:84) that fires every second forever if the app forgets room.close(). The Proxy in createActorsModule mints a fresh Room per actors.Name(id) call and keeps no reference to it, so there's no way to reclaim them. Have createActorsModule track live Rooms and expose a closeAll() that cleanup() invokes:

export function createActorsModule(config: ActorsConfig) {
  const rooms = new Set<Room>();
  const module = new Proxy({} as Record<string, (instanceId: string) => ActorRoom>, {
    get(_, actorName: string) {
      if (typeof actorName !== "string") return undefined; // see #4
      return (instanceId: string) => {
        const room = new Room(actorName, instanceId, config, () => rooms.delete(room));
        rooms.add(room);
        return room as unknown as ActorRoom;
      };
    },
  });
  return Object.assign(module, { closeAll: () => { for (const r of rooms) r.close(); rooms.clear(); } });
}

…then call userModules.actors.closeAll() inside cleanup(). (serviceRoleModules has no actors module, so it's fine there.)

2. send() can throw before the socket is OPENsrc/modules/actors.ts:112-117
connect() returns synchronously but the socket opens async, so room.connect().send(...) in the same tick calls ws.send before OPEN. The doc at actors.types.ts:73 says "buffered by the socket until open" — PartySocket only buffers once its internal socket exists, so an immediate send can still throw. Note the heartbeat's ws.send is wrapped in try/catch (:90-96) but the public send() (:116) isn't. Either guard readyState/buffer, or soften the doc.

3. Heartbeat is aggressivesrc/modules/actors.ts:21-22
PING_MS = 1_000 / DEAD_MS = 3_000 → one ping/sec per room. With many concurrent rooms that's a lot of wakeups and prevents Durable Objects from hibernating (cost). The rationale comment at :19-20 is good; consider making the interval configurable or relaxing it if second-level half-open detection isn't required.


🟡 Minor

4. Proxy get returns a factory for every keysrc/modules/actors.ts:132-134
The Proxy hands back a room factory for any property access, including then (which makes the actors object look thenable if it's ever awaited) and symbol keys. Add a guard: if (typeof actorName !== "string") return undefined;.

5. close() leaves connId stalesrc/modules/actors.ts:119-127
After close(), this.ws = null but this.connId is retained, so .id returns the old id between close and a subsequent connect(). A connect() after close with no id mints a fresh id but doesn't clear the old one first. Confirm the reconnect-after-close semantics are intended (arguably close() should null connId).

6. crypto.randomUUID() vs uuidv4()src/modules/actors.ts:47
Safe given the Node≥22 baseline, but uuid is already a dependency (src/utils/axios-client.ts). uuidv4() would be more consistent. Purely stylistic.


🔒 Security

Nothing concerning. The token rides the WS query and is re-read on each (re)connect via the query() closure (actors.ts:56-64) so login/logout is picked up; anonymous connects correctly omit it (covered by test at actors.test.ts:110). Precedence token || getAccessToken() (client.ts:229) matches the functions module — good.


🧪 Test coverage

Solid baseline in tests/unit/actors.test.ts — Proxy dispatch, idempotent connect, throw-before-connect, multi-listener + per-listener unsubscribe, __pong swallowing, send serialization, close teardown, anonymous connect, per-id independence. Gaps worth adding:

  • Heartbeat (fake timers): assert __ping is sent every PING_MS (:93), and a stale lastMsg triggers reconnect() after DEAD_MS (:85-88). FakeSocket already stubs reconnect(), so this is easy.
  • functionsVersionfv: assert it flows into the query when set and is omitted when unset (actors.ts:62). Current config sets it undefined.
  • resolvedActorsWsUrl derivation (client.ts:88-98): https→wss, http→ws, trailing-slash strip, and appBaseUrl/window/serverUrl precedence — currently untested.
  • closeAll() once SDK Improvements #1 lands: cleanup clears heartbeats/sockets.

Blocker I'd flag: cleanup wiring (#1) — a forgotten close() leaks a 1s timer permanently, and the module keeps no handle to reclaim it. Everything else is polish/optional. Nice work overall. 👍

View jobfeat/realtime-handler

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Analyze code quality & best practices
  • Check for bugs & issues
  • Review performance & security
  • Assess test coverage

Clean, well-documented feature. The room-handle API reads nicely, the type registry (ActorRegistry / ActorNameRegistry) is a great DX touch, and src/actor.ts is a tidy type-only base. The code is unchanged since the last pass, so the same items remain — #1 is still the one functional risk worth resolving before merge.


🟠 Design / lifecycle

1. No cleanup wiring for actor rooms — the main functional risksrc/client.ts:229-234 + actors.ts:130-136
userModules.cleanup() (client.ts:229) disconnects analytics + the entities socket but never touches actor rooms. Each connect() starts a setInterval heartbeat (actors.ts:84) that fires every second forever if the app forgets room.close(). The Proxy in createActorsModule mints a fresh Room per actors.Name(id) call and keeps no reference, so there's no way to reclaim leaked rooms. Have createActorsModule track live rooms and expose a closeAll() that cleanup() invokes:

export function createActorsModule(config: ActorsConfig) {
  const rooms = new Set<Room>();
  const module = new Proxy({} as Record<string, (instanceId: string) => ActorRoom>, {
    get(_, actorName: string) {
      if (typeof actorName !== "string") return undefined; // see #4
      return (instanceId: string) => {
        const room = new Room(actorName, instanceId, config, () => rooms.delete(room));
        rooms.add(room);
        return room as unknown as ActorRoom;
      };
    },
  });
  return Object.assign(module, {
    closeAll: () => { for (const r of rooms) r.close(); rooms.clear(); },
  });
}

…then call userModules.actors.closeAll() inside cleanup(). (serviceRoleModules has no actors module, so it's fine there.)

2. send() can throw before the socket is OPENsrc/modules/actors.ts:112-117
connect() returns synchronously but the socket opens async, so room.connect().send(...) in the same tick calls ws.send before OPEN. The doc at actors.types.ts:73 says "buffered by the socket until open" — PartySocket only buffers once its internal socket exists, so an immediate send can still throw. Note the heartbeat's ws.send is wrapped in try/catch (:90-96) but the public send() (:116) isn't. Either guard readyState/buffer, or soften the doc.

3. Heartbeat is aggressivesrc/modules/actors.ts:21-22
PING_MS = 1_000 / DEAD_MS = 3_000 → one ping/sec per room. With many concurrent rooms that's a lot of wakeups and prevents Durable Objects from hibernating (cost). The rationale comment at :19-20 is good; consider making the interval configurable or relaxing it if second-level half-open detection isn't required.


🟡 Minor

4. Proxy get returns a factory for every keysrc/modules/actors.ts:132-134
The Proxy hands back a room factory for any property access, including then (which makes the actors object look thenable if it's ever awaited) and symbol keys. Add a guard: if (typeof actorName !== "string") return undefined;.

5. close() leaves connId stalesrc/modules/actors.ts:119-127
After close(), this.ws = null but this.connId is retained, so .id returns the old id between close and a subsequent connect(). A connect() after close with no id mints a fresh id but doesn't clear the old one first. Confirm the reconnect-after-close semantics are intended (arguably close() should null connId).

6. crypto.randomUUID() vs uuidv4()src/modules/actors.ts:47
Safe given the Node≥22 baseline, but uuid is already a dependency (src/utils/axios-client.ts). uuidv4() would be more consistent. Purely stylistic.


🔒 Security

Nothing concerning. The token rides the WS query and is re-read on each (re)connect via the query() closure (actors.ts:56-64) so login/logout is picked up; anonymous connects correctly omit it (test at actors.test.ts:110). Precedence token || getAccessToken() (client.ts:227) matches the functions module — good.


🧪 Test coverage

Solid baseline in tests/unit/actors.test.ts — Proxy dispatch, idempotent connect, throw-before-connect, multi-listener + per-listener unsubscribe, __pong swallowing, send serialization, close teardown, anonymous connect, per-id independence. Gaps worth adding:

  • Heartbeat (fake timers): assert __ping is sent every PING_MS (:93), and a stale lastMsg triggers reconnect() after DEAD_MS (:85-88). FakeSocket already stubs reconnect(), so this is easy.
  • functionsVersionfv: assert it flows into the query when set and is omitted when unset (actors.ts:62). Current config sets it undefined.
  • resolvedActorsWsUrl derivation (client.ts:88-98): https→wss, http→ws, trailing-slash strip, and appBaseUrl/window/serverUrl precedence — currently untested.
  • closeAll() once SDK Improvements #1 lands: cleanup clears heartbeats/sockets.

Blocker I'd flag: cleanup wiring (#1) — a forgotten close() leaks a 1s timer permanently, and the module keeps no handle to reclaim it. Everything else is polish/optional. Nice work overall. 👍

View jobfeat/realtime-handler

ImriKochWix and others added 3 commits July 28, 2026 15:22
createActorsModule now tracks live rooms per client and exposes closeAll(),
which client.cleanup() invokes — a forgotten room.close() no longer leaks its
1s heartbeat timer (which also keeps the Node event loop alive). The Proxy get
trap consults the target first so closeAll resolves instead of being read as an
actor name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The get trap returned a room factory for `then`, so awaiting base44.actors (or
passing it through a Promise chain) called the factory as a thenable and hung.
Resolve `then` (and inherited/symbol keys) normally so the module isn't mistaken
for a Promise.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
close() left connId set, so room.id returned a stale id between close and the
next connect(). Null it in close() — reading id after close now throws the same
connect()-first guard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ImriKochWix

Copy link
Copy Markdown
Contributor Author

Thanks — walked all six:

  • SDK Improvements #1 (cleanup leak) — fixed (d06d15a). createActorsModule now tracks live rooms per client and exposes closeAll(), which client.cleanup() invokes; the Proxy get consults the target first so closeAll resolves. A forgotten room.close() no longer leaks its 1s timer.
  • functions - invoke style #2 (send before OPEN) — not an issue. send() calls PartySocket's send, which buffers when not OPEN (partysocket/dist/ws.js:277-288) and flushes on open (:434). connect().send() enqueues rather than throwing; the doc ("buffered until open") is accurate.
  • expose loginViaUsernamePassword on auth module #3 (aggressive heartbeat) — addressed server-side. apper #16970 registers setWebSocketAutoResponse('{"type":"__ping"}' -> '{"type":"__pong"}'), so the edge answers the 1s ping without waking the DO — idle rooms hibernate despite the heartbeat. Kept 1s/3s (correct for interactive actors). No SDK change; the ping frame matches the auto-response pair byte-for-byte.
  • reload page after logout if no redirect #4 (Proxy thenable) — fixed (543d9e6). then, symbol, and inherited keys now resolve normally, so the module isn't mistaken for a Promise when awaited.
  • Add Claude Code GitHub Workflow #5 (stale connId) — fixed (973772e). close() nulls connId; .id is valid only while connected.
  • Add API Key support to created client #6 (crypto.randomUUID vs uuidv4) — keeping native. Given the Node 22+ baseline, native crypto.randomUUID() is dependency-free; we'd rather not spread the uuid dep further.

Test coverage: closeAll now covered. Remaining gaps (heartbeat timing, functionsVersion -> fv, resolvedActorsWsUrl derivation) are noted — will add.

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

Thanks for walking all six — I re-read the current tree and can confirm the fixes landed cleanly. This is in good shape to merge.


🟢 Verified fixes since last pass

  • SDK Improvements #1 cleanup leak — resolvedcreateActorsModule now keeps a rooms Set (actors.ts:136), each Room self-removes on close() via the onClose callback (:153, :129), and closeAll() (:138-140) is invoked from client.cleanup() (client.ts:231). A forgotten room.close() no longer leaks its 1s heartbeat. Iterating [...rooms] (:139) is the right call since close() mutates the Set mid-loop.
  • reload page after logout if no redirect #4 Proxy thenable — resolved ✅ The get trap now returns Reflect.get for then, symbols, and own keys (:149-151), so the module can't be mistaken for a Promise and closeAll resolves correctly. Covered by the new test at actors.test.ts:130.
  • Add Claude Code GitHub Workflow #5 stale connId — resolvedclose() nulls connId (:128); .id throws between close and reconnect. Test at actors.test.ts:110.
  • functions - invoke style #2 send-before-open — accepted your PartySocket-buffers explanation; the doc at actors.types.ts:73 matches. 👍
  • expose loginViaUsernamePassword on auth module #3 heartbeat — accepted the server-side setWebSocketAutoResponse mitigation for DO hibernation. Makes sense for interactive actors.
  • Add API Key support to created client #6 crypto.randomUUID — fine to keep native on the Node 22+ baseline.

🟡 Minor (non-blocking, optional)

1. closeAll() clears the Set only via each room's onCloseactors.ts:138-140
closeAll relies on every Room.close() firing onClose to delete itself. That holds today, but if a future close() path ever skipped onClose, rooms could retain dead entries. A defensive rooms.clear() after the loop would make closeAll self-sufficient regardless of close() internals. Purely belt-and-suspenders.

2. reconnect() after close()actors.ts:85-98
The heartbeat interval is cleared in close(), so no stray reconnect() fires afterward — good. Just noting the ws.reconnect() path (:88) has no guard for a socket that's since been closed; since the interval is cleared first this is unreachable, so no action needed.


🧪 Test coverage

Strong baseline in tests/unit/actors.test.ts — Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, anonymous connect, per-id independence, not-thenable, and both closeAll paths. As you noted, the still-open gaps are:

  • Heartbeat (fake timers): assert __ping every PING_MS (:94) and that stale lastMsg triggers reconnect() after DEAD_MS (:86-89). FakeSocket already stubs reconnect(), so this is a couple of lines.
  • functionsVersionfv: the config sets it undefined, so the omit-branch (actors.ts:63) is covered but the present-branch isn't. One { ...config, functionsVersion: "draft" } case would close it.
  • resolvedActorsWsUrl derivation (client.ts:88-98): https→wss, http→ws, trailing-slash strip, appBaseUrl/window/serverUrl precedence — still untested.

None of these block merge.


Verdict: the one functional blocker from prior passes (cleanup wiring) is now fixed and tested. Remaining items are optional polish. Nice work getting this over the line. 👍

View jobfeat/realtime-handler

createActorsModule returns { module, closeAll } instead of bolting closeAll onto
the Proxy. The Proxy now only maps names to room factories (2-condition guard),
and closeAll is a plain internal function client.cleanup() calls — dropping the
api object, Object.assign, the intersection cast, and the key-in-target branch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Verify author's recent fixes
  • Analyze code quality, bugs, performance, security
  • Assess test coverage
  • Post review

Re-reviewed the current tree. All prior blockers are resolved and the code reads cleanly. This is in good shape to merge — remaining notes are optional polish.


🟢 Verified fixes from prior passes

  • Cleanup wiring (SDK Improvements #1) ✅ — createActorsModule tracks live rooms in a Set (actors.ts:136); each Room self-removes via the onClose callback (:145, :129); closeAll() (:154-156) is invoked from client.cleanup() (client.ts:233). Iterating [...rooms] snapshots the set so close()'s mid-loop mutation is safe. A forgotten room.close() no longer leaks its 1s heartbeat.
  • Proxy thenable (reload page after logout if no redirect #4) ✅ — the get trap returns undefined for symbols and then (actors.ts:143); covered by the not-thenable test (actors.test.ts:133).
  • Stale connId (Add Claude Code GitHub Workflow #5) ✅ — close() nulls connId (actors.ts:128) so .id throws between close and reconnect; covered at actors.test.ts:113.

🟡 Minor (non-blocking, optional)

1. send()/connect() doc vs. behavioractors.types.ts:73
The doc says a send is "buffered by the socket until open." That's true only after PartySocket's internal socket exists; a send in the same synchronous tick as connect() can still throw. You've validated the buffering path — just flagging that the "always buffered" phrasing is slightly optimistic for the immediate-send edge case. Consider softening to "buffered by the socket once connecting."

2. Heartbeat costactors.ts:21-22
PING_MS = 1_000 / DEAD_MS = 3_000 → one ping/sec per room. Accepted per your server-side setWebSocketAutoResponse mitigation (edge answers __ping__pong without waking the DO, so idle rooms still hibernate). Reasonable for interactive actors; only reconsider if many idle rooms per client become common. No change needed.

3. crypto.randomUUID() vs uuidv4()actors.ts:48
Fine on the Node ≥22 baseline. Purely a consistency nit with the rest of the SDK (utils/axios-client.ts uses uuidv4).


🔒 Security

Nothing concerning. The token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:57-64), so login/logout is picked up; anonymous connects omit it (test at actors.test.ts:120). Precedence token || getAccessToken() (client.ts:190) matches the functions module.


🧪 Test coverage

Strong baseline in tests/unit/actors.test.ts — Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, id-clearing, anonymous connect, per-id independence, not-thenable, and both closeAll paths. Gaps still worth adding (none block merge):

  • Heartbeat (fake timers): assert __ping is sent every PING_MS (:94) and a stale lastMsg triggers reconnect() after DEAD_MS (:86-88). FakeSocket already stubs reconnect().
  • functionsVersionfv: the config sets it undefined, so only the omit-branch (actors.ts:63) is exercised; one { ...config, functionsVersion: "draft" } case closes it.
  • resolvedActorsWsUrl derivation (client.ts:88-98): https→wss, http→ws, trailing-slash strip, appBaseUrl/window/serverUrl precedence — still untested.

Verdict: No blockers. The one functional risk from earlier passes (cleanup leak) is fixed and tested. Nice work getting this over the line. 👍

View jobfeat/realtime-handler

- Extract resolveActorsWsUrl into a pure exported helper (client.ts calls it)
  and unit-test its precedence + protocol/trailing-slash normalization.
- Add heartbeat tests (fake timers): __ping every PING_MS, reconnect after
  DEAD_MS of silence; and functionsVersion -> fv present/omitted.
- Soften the send() doc: buffered "once connecting" (matches PartySocket).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Verify recently-landed fixes
  • Analyze code quality, bugs, performance, security
  • Assess test coverage
  • Post review

Re-reviewed the current tree. This has come a long way — all prior blockers are resolved, and the new commits since the last pass (resolveActorsWsUrl now exported + tested, heartbeat timing test, fv present/absent test) close most of the outstanding gaps I flagged. This is in good shape to merge. Only optional polish remains.


🟢 Verified since last pass

  • Cleanup wiring (SDK Improvements #1) ✅ — createActorsModule now returns { module, closeAll } (actors.ts:172-178); each Room self-removes via onClose (:165, :129); closeAll() iterates a [...rooms] snapshot so mid-loop close() mutation is safe (:175); client.cleanup() invokes it (client.ts:225). Both closeAll paths are tested (actors.test.ts:161, :170).
  • Proxy thenable (reload page after logout if no redirect #4) ✅ — get returns undefined for symbols and then (actors.ts:163); covered by the not-thenable test (actors.test.ts:156).
  • Stale connId (Add Claude Code GitHub Workflow #5) ✅ — close() nulls connId (actors.ts:128); .id throws between close and reconnect (actors.test.ts:113).
  • URL derivation now tested ✅ — resolveActorsWsUrl is extracted, exported, and covered across https→wss, http→ws, trailing-slash strip, and appBaseUrl/browserOrigin/serverUrl precedence (actors.test.ts:180-207). Nice — this was an open gap in every prior pass.
  • Heartbeat + fv tests ✅ — fake-timer heartbeat test asserts __ping every PING_MS and reconnect() after DEAD_MS (:140); fv present/absent both covered (:133).

🟡 Minor (non-blocking, optional)

1. send() doc vs. immediate-send behavioractors.types.ts:73
Doc says a send is "buffered by the socket once connecting." That's accurate once PartySocket's internal socket exists, but a room.connect().send(...) in the same synchronous tick can still hit ws.send before the socket is constructed. You verified PartySocket's buffering covers the realistic path; just noting the same-tick edge remains a theoretical throw. Current phrasing is reasonable — no change needed.

2. resolveActorsWsUrl protocol replace is anchored, appBaseUrl isn't validatedactors.ts:145-150
.replace(/^https:\/\//, ...) only rewrites a leading protocol, so a bare host like app.example (no scheme) passes through unchanged and PartySocket would receive a scheme-less host. Not a regression and probably fine given callers always pass an origin, but a scheme-less appBaseUrl would silently produce a non-wss host. Optional to guard.

3. Heartbeat costactors.ts:21-22
PING_MS = 1_000 / DEAD_MS = 3_000 → one ping/sec per room. Accepted per the server-side setWebSocketAutoResponse mitigation (edge answers __ping__pong without waking the DO). Reasonable for interactive actors; only revisit if many idle rooms per client become common.

4. crypto.randomUUID() vs uuidv4()actors.ts:48
Fine on the Node ≥22 baseline. Purely a consistency nit with the rest of the SDK. Keeping native is a defensible call.


🔒 Security

Nothing concerning. The token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:57-65), so login/logout is picked up; anonymous connects omit it (test at actors.test.ts:120). Precedence token || getAccessToken() (client.ts:182) matches the functions module. Note the same subtle stale-token-closure applies here as elsewhere — after setToken(), the initial token const won't update, but getAccessToken() reads storage so the effective token stays correct. Consistent with the existing modules.


🧪 Test coverage

Strong — tests/unit/actors.test.ts now covers Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, id-clearing, anonymous connect, per-id independence, fv present/absent, heartbeat ping+reconnect, not-thenable, both closeAll paths, and full resolveActorsWsUrl derivation. Essentially all previously-noted gaps are closed. Only nice-to-have left: a close()connect() re-open cycle mints a fresh socket (currently connect() is guarded by if (this.ws), and close() nulls ws, so a re-open path exists but isn't directly asserted).


Verdict: No blockers. Every functional risk from prior passes is fixed and tested; the remaining items are polish/optional. Nice work getting this over the line. 👍

View jobfeat/realtime-handler

…on code

serverUrl is the app's own origin, and PartySocket already strips the scheme,
strips a trailing slash, and picks wss (ws for localhost) from the host. So the
resolveActorsWsUrl helper, the actorsWsUrl client option, and the
browserOrigin/appBaseUrl derivation were all redundant — pass serverUrl straight
through as PartySocket's host.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Verify recent commits (serverUrl refactor)
  • Analyze code quality, bugs, performance, security
  • Assess test coverage
  • Post review

Re-reviewed the current tree. The recent commits simplified things nicely: serverUrl is now passed straight through as PartySocket's host (PartySocket swaps the scheme itself), dropping the old resolveActorsWsUrl derivation and the actorsWsUrl client option entirely. All prior functional blockers remain resolved. No blockers here — only notes below.


🟢 State of prior items

  • Cleanup wiring ✅ — createActorsModule returns { module, closeAll } (actors.ts:154-159); each Room self-removes via onClose (:147, :131); closeAll iterates a [...rooms] snapshot so mid-loop close() mutation is safe (:157); client.cleanup() invokes it (client.ts:214). Both paths tested (actors.test.ts:163, :172).
  • Proxy thenable ✅ — get returns undefined for symbols and then (actors.ts:145); tested (actors.test.ts:158).
  • Stale connId ✅ — close() nulls connId (actors.ts:130); tested (actors.test.ts:115).
  • Heartbeat timing + fv ✅ — both now covered (actors.test.ts:135, :142).

🟠 Notes on the serverUrl refactor

1. PR body is now stale. The description still mentions actorsWsUrl (defaulting to app origin → wss://) and dispatcherWsUrl, but the code now just forwards serverUrl as PartySocket's host. Worth updating the description so reviewers/future readers aren't misled. Also note resolveActorsWsUrl and its dedicated derivation tests are gone — the old actors.test.ts URL-derivation assertions no longer exist since the logic moved into PartySocket.

2. serverUrl semantics changed. Previously the socket was same-origin via an app-proxied /parties; now it connects to config.serverUrl (the Base44 backend, e.g. https://base44.app). Just confirming this is intentional and that the backend serves /parties directly for the SDK's use case (the doc comment at actors.ts:16-18 still says "the app proxies /parties", which reads slightly at odds with passing serverUrl rather than the app origin). Minor doc reconciliation.


🟡 Minor (non-blocking, optional — carried over)

3. send() doc vs. immediate-sendactors.types.ts:73 says a send is "buffered by the socket once connecting." Accurate once PartySocket's internal socket exists; a room.connect().send(...) in the same synchronous tick can still hit ws.send before construction. You validated the realistic path — phrasing is reasonable.

4. Heartbeat costactors.ts:23-24 (PING_MS=1s/DEAD_MS=3s, one ping/sec per room). Accepted per the server-side setWebSocketAutoResponse mitigation. Fine for interactive actors.

5. crypto.randomUUID() vs uuidv4()actors.ts:50. Fine on the Node ≥22 baseline; purely a consistency nit with utils/axios-client.ts.


🔒 Security

Nothing concerning. Token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:59-67), so login/logout is picked up; anonymous connects omit it (actors.test.ts:122). Precedence token || getAccessToken() (client.ts:171) matches the functions module.


🧪 Test coverage

Strong — tests/unit/actors.test.ts covers Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, id-clearing, anonymous connect, per-id independence, fv present/absent, heartbeat ping+reconnect, not-thenable, and both closeAll paths. One coverage change to flag: the previous resolveActorsWsUrl derivation tests were removed along with the function. Since host is now passed verbatim, that's reasonable — the one remaining assertion is opts.host === serverUrl (:56), which is sufficient given PartySocket owns the scheme swap. A close()connect() re-open cycle is still the only untested path.


Verdict: No blockers. The serverUrl simplification is clean and well-tested. Main ask is refreshing the PR description (and the actors.ts:16-18 comment) to match the new serverUrl-passthrough behavior. Nice work. 👍
feat/realtime-handler

Actors deployed by the bundler now expose an anonymous Base44 client as
this.client. Mirror it on the type-only Actor base class (throw-stub like
storage/instanceId) so actor authors get it typed as Base44Client; the
runtime value is provided by the bundler shim.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Verify recent commits (serverUrl passthrough, this.client getter)
  • Analyze code quality, bugs, performance, security
  • Assess test coverage
  • Post review

Re-reviewed the current tree. The two new commits since the last pass — e75ce02 (pass serverUrl straight through as PartySocket's host, dropping the URL-derivation code) and 9717c67 (type-only this.client getter on Actor) — are clean. All prior functional blockers remain resolved. No blockers; notes below are polish/doc reconciliation.


🟢 State of prior items

  • Cleanup wiring ✅ — createActorsModule returns { module, closeAll } (actors.ts:154-159); each Room self-removes via onClose (:147, :131); closeAll iterates a [...rooms] snapshot so mid-loop close() mutation is safe (:157); client.cleanup() invokes it (client.ts:214). Both paths tested (actors.test.ts:163, :172).
  • Proxy thenable ✅ — get returns undefined for symbols and then (actors.ts:145); tested (actors.test.ts:158).
  • Stale connId ✅ — close() nulls connId (actors.ts:130); tested (actors.test.ts:115).
  • Heartbeat timing + fv ✅ — both covered (actors.test.ts:135, :142).

🟠 Doc / naming reconciliation (from the serverUrl refactor)

1. PR body is now stale. The description still mentions the actorsWsUrl client option (defaulting to app origin → wss://) and a dispatcherWsUrl, but the code now just forwards config.serverUrl as PartySocket's host — no such option or derivation exists anymore. Worth refreshing the description so future readers aren't misled.

2. serverUrl doc comment reads at odds with the passthrough. actors.ts:16-18 still says "The app's own origin … the socket is same-origin (the app proxies /parties)." But the value now passed is config.serverUrl (defaults to https://base44.app, i.e. the Base44 backend — client.ts:67,169), not the app origin. If the backend serves /parties directly this is fine, but the comment should be reconciled with the new source of the value. Minor.


🟡 Minor (non-blocking, optional — carried over)

3. send() doc vs. immediate-sendactors.types.ts:73 says a send is "buffered by the socket once connecting." Accurate once PartySocket's internal socket exists; a room.connect().send(...) in the same synchronous tick can still hit ws.send before construction. You validated the realistic buffering path — phrasing is reasonable.

4. Heartbeat costactors.ts:23-24 (PING_MS=1s/DEAD_MS=3s, one ping/sec per room). Accepted per the server-side setWebSocketAutoResponse mitigation (edge answers __ping__pong without waking the DO, so idle rooms hibernate). Fine for interactive actors.

5. crypto.randomUUID() vs uuidv4()actors.ts:50. Fine on the Node ≥22 baseline; purely a consistency nit with utils/axios-client.ts.

6. Duplicate blank lines in actor.ts:13-14, :39-40, :113-114. Trivial cosmetic; drop if your formatter flags it.


🔒 Security

Nothing concerning. Token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:59-67), so login/logout is picked up; anonymous connects omit it (actors.test.ts:122). Precedence token || getAccessToken() (client.ts:171) matches the functions module. createClientFromRequest continues to forward only the closed X-Data-Env set (client.ts:460) — good.


🧪 Test coverage

Strong — tests/unit/actors.test.ts covers Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, id-clearing, anonymous connect, per-id independence, fv present/absent, heartbeat ping+reconnect, not-thenable, and both closeAll paths. Coverage note tied to the refactor: the old resolveActorsWsUrl derivation tests were removed with the function (reasonable — host is now passed verbatim; the one assertion opts.host === serverUrl at :56 suffices since PartySocket owns the scheme swap). Only remaining untested path is a close()connect() re-open cycle minting a fresh socket.


Verdict: No blockers. The serverUrl simplification is clean and well-tested. Main ask is refreshing the PR description and the actors.ts:16-18 comment to match the new serverUrl-passthrough behavior. Nice work. 👍

View jobfeat/realtime-handler

…he socket

serverUrl is often "" (same-origin apps use a relative /api), and PartySocket
can't resolve a relative host — it builds a hostless wss:///parties/... So add
resolveActorsHost: use serverUrl when absolute, else fall back to the browser
origin. PartySocket still owns the scheme swap and trailing-slash strip.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Verify recent commits (serverUrl→origin resolution)
  • Analyze code quality, bugs, performance, security
  • Assess test coverage
  • Post review

Re-reviewed the current tree. The recent commits are clean: resolveActorsHost now resolves an empty/relative serverUrl to the page origin and hands it to PartySocket as host (PartySocket owns the scheme swap), and Actor gained a type-only client getter. All prior functional blockers remain resolved. No blockers — notes below are polish/doc reconciliation.


🟢 State of prior items

  • Cleanup wiring ✅ — createActorsModule returns { module, closeAll } (actors.ts:164-169); each Room self-removes via onClose (actors.ts:157, :131); closeAll iterates a [...rooms] snapshot so mid-loop close() mutation is safe (:167); client.cleanup() invokes it (client.ts:219). Both paths tested (actors.test.ts:163, :172).
  • Proxy thenable ✅ — get returns undefined for symbols and then (actors.ts:155); tested (actors.test.ts:158).
  • Stale connId ✅ — close() nulls connId (actors.ts:130); tested (actors.test.ts:115).
  • Heartbeat timing + fv ✅ — both covered (actors.test.ts:135, :142).
  • URL derivation now tested ✅ — resolveActorsHost is extracted, exported, and covered across absolute / empty / relative / no-origin cases (actors.test.ts:182-200).

🟠 Small correctness note

1. resolveActorsHost only treats a leading / as relativeactors.ts:141-143
serverUrl && !serverUrl.startsWith("/") means a scheme-less host (e.g. "api.example.com" or a protocol-relative "//host") is treated as absolute and passed straight to PartySocket. Given callers pass either https://base44.app or ""//api, this is fine in practice — just flagging that a bare-host serverUrl would slip through unchanged. Optional to guard.


🟠 Doc / naming reconciliation (carried over)

2. PR body is stale. The description still mentions the actorsWsUrl option (defaulting to app origin → wss://) and a dispatcherWsUrl derivation — neither exists anymore; the code forwards a resolved host to PartySocket. Worth refreshing so future readers aren't misled.

3. Comment vs. value sourceclient.ts:169-170 / actors.ts:16-18. Comments read "same-origin app / the app proxies /parties," but the value now defaults to config.serverUrl (https://base44.app) unless it's empty/relative, in which case it falls back to the page origin. If the backend serves /parties directly, fine — just reconcile the wording with the actual resolveActorsHost precedence (serverUrl-wins-when-absolute).


🟡 Minor (non-blocking, optional — carried over)

4. send() doc vs. immediate-sendactors.types.ts:73 says a send is "buffered by the socket once connecting." Accurate once PartySocket's internal socket exists; a room.connect().send(...) in the same synchronous tick can still hit ws.send before construction. You validated the realistic buffering path — phrasing is reasonable.

5. Heartbeat costactors.ts:23-24 (PING_MS=1s / DEAD_MS=3s, one ping/sec per room). Accepted per the server-side setWebSocketAutoResponse mitigation (edge answers __ping__pong without waking the DO, so idle rooms hibernate). Fine for interactive actors.

6. crypto.randomUUID() vs uuidv4()actors.ts:50. Fine on the Node ≥22 baseline; purely a consistency nit with utils/axios-client.ts. Keeping native is defensible.

7. Duplicate blank lines in actor.ts:13-14, :39-40, :113-114. Trivial cosmetic; drop if your formatter flags them.


🔒 Security

Nothing concerning. Token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:59-67), so login/logout is picked up; anonymous connects omit it (actors.test.ts:122). Precedence token || getAccessToken() (client.ts:176) matches the functions module. Parsed inbound messages are JSON with a try/catch guard (actors.ts:77-81) and __pong frames are swallowed — no unsafe eval or trust of message shape.


🧪 Test coverage

Strong — tests/unit/actors.test.ts covers Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, id-clearing, anonymous connect, per-id independence, fv present/absent, heartbeat ping+reconnect, not-thenable, both closeAll paths, and full resolveActorsHost derivation. Only remaining untested path is a close()connect() re-open cycle minting a fresh socket (the if (this.ws) guard + close() nulling ws makes this reachable but it isn't directly asserted).


Verdict: No blockers. The serverUrl-resolution refactor is clean and well-tested. Main ask remains refreshing the PR description and the actors.ts:16-18 / client.ts:169-170 comments to match the resolveActorsHost behavior. Nice work. 👍

View jobfeat/realtime-handler

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Verify recent commits
  • Analyze code quality, bugs, performance, security
  • Assess test coverage
  • Post review

Re-reviewed the current tree. The code is clean and all prior functional blockers remain resolved. The recent commits (resolveActorsHost resolving empty/relative serverUrl to the page origin, the type-only this.client getter, and the blank-line cleanup in actor.ts) all landed cleanly. No blockers — notes below are polish/doc reconciliation.


🟢 State of prior items (all verified resolved)

  • Cleanup wiring ✅ — createActorsModule returns { module, closeAll } (actors.ts:164-169); each Room self-removes via onClose (actors.ts:157, :131); closeAll iterates a [...rooms] snapshot so mid-loop close() mutation is safe (:167); client.cleanup() invokes it (client.ts:219). Both paths tested (actors.test.ts:163, :172).
  • Proxy thenable ✅ — get returns undefined for symbols and then (actors.ts:155); tested (actors.test.ts:158).
  • Stale connId ✅ — close() nulls connId (actors.ts:130); tested (actors.test.ts:115).
  • Heartbeat timing + fv ✅ — both covered (actors.test.ts:135, :142).
  • resolveActorsHost derivation ✅ — extracted, exported, and covered across absolute / empty / relative / no-origin (actors.test.ts:182-199).

🟠 Small correctness note (carried over)

1. resolveActorsHost only treats a leading / as relativeactors.ts:141-143
serverUrl && !serverUrl.startsWith("/") means a scheme-less host (e.g. "api.example.com" or protocol-relative "//host") is treated as absolute and passed verbatim to PartySocket. Given callers pass either https://base44.app or ""//api, this is fine in practice — just flagging that a bare-host serverUrl would slip through. Optional to guard.


🟠 Doc / naming reconciliation (carried over — still worth doing)

2. PR body is stale. The description still mentions the actorsWsUrl option (defaulting to app origin → wss://) and a dispatcherWsUrl derivation — neither exists anymore; the code forwards a resolved host to PartySocket via resolveActorsHost. Worth refreshing so future readers aren't misled.

3. Comment vs. value sourceclient.ts:169-170 / actors.ts:16-18. The comments still read "same-origin app / the app proxies /parties," but the value now defaults to config.serverUrl (https://base44.app) when absolute, only falling back to the page origin when empty/relative. If the backend serves /parties directly this is fine — just reconcile the wording with the actual resolveActorsHost precedence.


🟡 Minor (non-blocking, optional — carried over)

4. send() doc vs. immediate-sendactors.types.ts:73 says a send is "buffered by the socket once connecting." Accurate once PartySocket's internal socket exists; a room.connect().send(...) in the same synchronous tick can still hit ws.send before construction. You validated the realistic buffering path — phrasing is reasonable.

5. Heartbeat costactors.ts:23-24 (PING_MS=1s / DEAD_MS=3s, one ping/sec per room). Accepted per the server-side setWebSocketAutoResponse mitigation (edge answers __ping__pong without waking the DO, so idle rooms hibernate). Fine for interactive actors.

6. crypto.randomUUID() vs uuidv4()actors.ts:50. Fine on the Node ≥22 baseline; purely a consistency nit with utils/axios-client.ts. Keeping native is defensible.


🔒 Security

Nothing concerning. Token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:59-67), so login/logout is picked up; anonymous connects omit it (actors.test.ts:122). Precedence token || getAccessToken() (client.ts:176) matches the functions module. Inbound messages are JSON-parsed inside a try/catch (actors.ts:77-81) with __pong frames swallowed — no unsafe eval or trust of message shape.


🧪 Test coverage

Strong — tests/unit/actors.test.ts covers Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, id-clearing, anonymous connect, per-id independence, fv present/absent, heartbeat ping+reconnect, not-thenable, both closeAll paths, and full resolveActorsHost derivation. The only untested path is a close()connect() re-open cycle minting a fresh socket (if (this.ws) guard + close() nulling ws makes it reachable but it isn't directly asserted). Not a blocker.


Verdict: No blockers. Every functional risk from prior passes is fixed and tested; the remaining items are polish/optional. The main ask remains refreshing the PR description and the actors.ts:16-18 / client.ts:169-170 comments to match the resolveActorsHost behavior. Nice work getting this over the line. 👍

View jobfeat/realtime-handler

Closes the last coverage note from review — the close()->connect() re-open path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Verify prior fixes are in place
  • Analyze code quality, bugs, performance, security
  • Assess test coverage
  • Post review

Re-reviewed the current tree. This is clean and ready to merge — every functional blocker from the prior passes is resolved and tested. Notes below are polish/doc reconciliation only.


🟢 State of prior items (all verified resolved)

  • Cleanup wiring ✅ — createActorsModule returns { module, closeAll } (actors.ts:164-169); each Room self-removes via onClose (:157, :131); closeAll iterates a [...rooms] snapshot so mid-loop close() mutation is safe (:167); client.cleanup() invokes it (client.ts:219). Both paths tested (actors.test.ts:173, :182).
  • Proxy thenable ✅ — get returns undefined for symbols and then (actors.ts:155); tested (actors.test.ts:168).
  • Stale connId ✅ — close() nulls connId (actors.ts:130); tested (actors.test.ts:125).
  • Heartbeat timing + fv ✅ — both covered (actors.test.ts:152, :145).
  • resolveActorsHost derivation ✅ — extracted, exported, covered across absolute / empty / relative / no-origin (actors.test.ts:192-209).
  • close()connect() re-open ✅ — now directly asserted (actors.test.ts:115), closing the last coverage gap from prior passes.

🟠 Small correctness note (carried over)

1. resolveActorsHost only treats a leading / as relativeactors.ts:141-142
serverUrl && !serverUrl.startsWith("/") means a scheme-less host (e.g. "api.example.com" or protocol-relative "//host") is treated as absolute and passed verbatim to PartySocket. Given callers pass either https://base44.app or ""//api, this is fine in practice — just flagging the bare-host edge. Optional to guard.


🟠 Doc / naming reconciliation (carried over — still worth doing)

2. PR body is stale. The description still mentions resolveActorsHost(serverUrl, window?.location?.origin) — that part matches, but earlier phrasing and prior descriptions referenced actorsWsUrl/dispatcherWsUrl which no longer exist. Worth a final skim of the description so it matches the resolveActorsHost passthrough exactly.

3. Comment vs. value sourceclient.ts:169-170 / actors.ts:16-18. The comments read "same-origin app / the app proxies /parties," but the resolved value is config.serverUrl when absolute (defaults to the Base44 backend), only falling back to the page origin when empty/relative. If the backend serves /parties directly this is fine — just reconcile the wording with the actual resolveActorsHost precedence.


🟡 Minor (non-blocking, optional — carried over)

4. send() doc vs. immediate-sendactors.types.ts:73 says a send is "buffered by the socket once connecting." Accurate once PartySocket's internal socket exists; a room.connect().send(...) in the same synchronous tick can still hit ws.send before construction. You validated the realistic buffering path — phrasing is reasonable.

5. Heartbeat costactors.ts:23-24 (PING_MS=1s / DEAD_MS=3s, one ping/sec per room). Accepted per the server-side setWebSocketAutoResponse mitigation (edge answers __ping__pong without waking the DO, so idle rooms hibernate). Fine for interactive actors.

6. crypto.randomUUID() vs uuidv4()actors.ts:50. Fine on the Node ≥22 baseline; purely a consistency nit with utils/axios-client.ts. Keeping native is defensible.


🔒 Security

Nothing concerning. Token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:59-67), so login/logout is picked up; anonymous connects omit it (actors.test.ts:132). Precedence token || getAccessToken() (client.ts:176) matches the functions module. Inbound messages are JSON-parsed inside a try/catch (actors.ts:77-81) with __pong frames swallowed — no unsafe eval or trust of message shape.


🧪 Test coverage

Strong and complete — tests/unit/actors.test.ts covers Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, id-clearing, close()connect() re-open, anonymous connect, per-id independence, fv present/absent, heartbeat ping+reconnect, not-thenable, both closeAll paths, and full resolveActorsHost derivation. No meaningful gaps remain.


Verdict: No blockers. The one functional risk from earlier passes (cleanup leak) is fixed and tested, and the previously-open coverage gaps are now closed. Remaining items are optional polish — mainly reconciling the PR description and the actors.ts:16-18 / client.ts:169-170 comments with the resolveActorsHost behavior. Nice work getting this over the line. 👍

View jobfeat/realtime-handler

Comment thread src/modules/actors.ts Outdated
An actor runs server-side with no per-connection identity, so a Test DB preview
selected in the editor doesn't apply to this.client — it's an anonymous,
prod-scoped client (logged-out-visitor semantics). Document the limitation for
actor authors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (room-handle API)

  • Read changed files
  • Analyze code quality, bugs, performance, security
  • Address the inline naming question at actors.ts:26
  • Assess test coverage
  • Post review

Re-reviewed the current tree. The code is clean and every functional blocker from prior passes is resolved and tested. Below I address @netanelgilad's inline naming question, then the remaining polish items.


💬 Re: @netanelgilad's question — "shouldn't we call this Actor?" (actors.ts:26)

I'd argue Room is the more accurate name here — and renaming it to Actor would actually be confusing given the rest of the codebase:

  1. Actor is already takensrc/actor.ts:54 exports abstract class Actor, the type-only base class users extend server-side (class MyActor extends Actor). Having a second, unrelated client-side class Actor in actors.ts would collide conceptually (and could collide on import if either ever leaks out of its module).
  2. This class isn't an actor — it's a handle to one room of an actor. An "actor" (e.g. GameRoom) is the deployed Durable Object type; each instanceId is a distinct live instance. This class wraps a single (actorName, instanceId) socket connection — i.e. one room. The public type it implements is even called ActorRoom (actors.types.ts:63), and the API reads actors.GameRoom("room-1") → a room. So Room matches both the domain model and the public type.
  3. If the goal is just to disambiguate from PartySocket/room terminology, ActorRoom (to mirror the exported interface) would be clearer than Actor. But plain Room is fine since it's module-private.

Suggestion: keep Room (or rename to ActorRoom to mirror the public interface), but not Actor — that name belongs to the server-side base class in actor.ts.


🟢 State of prior items (all verified resolved)

  • Cleanup wiring ✅ — createActorsModule returns { module, closeAll } (actors.ts:164-169); each Room self-removes via onClose (:157, :131); closeAll iterates a [...rooms] snapshot so mid-loop close() mutation is safe (:167); client.cleanup() invokes it (client.ts:219). Both paths tested (actors.test.ts:173, :182).
  • Proxy thenable ✅ — get returns undefined for symbols and then (actors.ts:155); tested (actors.test.ts:168).
  • Stale connId ✅ — close() nulls connId (actors.ts:130); close()connect() re-open tested (actors.test.ts:115, :125).
  • Heartbeat timing + fv ✅ — both covered (actors.test.ts:152, :145).
  • resolveActorsHost derivation ✅ — extracted, exported, covered across absolute / empty / relative / no-origin (actors.test.ts:192-209).

🟡 Minor (non-blocking, optional — carried over)

1. resolveActorsHost only treats a leading / as relativeactors.ts:141-142. A scheme-less host ("api.example.com", protocol-relative "//host") is treated as absolute and passed verbatim to PartySocket. Fine given callers pass https://… or ""//api; just flagging the bare-host edge.

2. Doc/naming reconciliationclient.ts:169-170 / actors.ts:16-18 still describe "same-origin app / the app proxies /parties," but the resolved value is config.serverUrl when absolute, only falling back to the page origin when empty/relative. Worth reconciling the wording with the actual resolveActorsHost precedence.

3. send() doc vs. immediate-sendactors.types.ts:73 says a send is "buffered by the socket once connecting." Accurate once PartySocket's internal socket exists; a room.connect().send(...) in the same synchronous tick can still hit ws.send before construction. Realistic path is fine — phrasing is reasonable.

4. crypto.randomUUID() vs uuidv4()actors.ts:50. Fine on the Node ≥22 baseline; purely a consistency nit with utils/axios-client.ts. Keeping native is defensible.


🔒 Security

Nothing concerning. Token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:59-67), so login/logout is picked up; anonymous connects omit it (actors.test.ts:132). Precedence token || getAccessToken() (client.ts:176) matches the functions module. Inbound messages are JSON-parsed inside a try/catch (actors.ts:77-81) with __pong frames swallowed — no unsafe eval or trust of message shape.


🧪 Test coverage

Strong and complete — tests/unit/actors.test.ts covers Proxy dispatch, idempotent connect, throw-before-connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, id-clearing, close()connect() re-open, anonymous connect, per-id independence, fv present/absent, heartbeat ping+reconnect, not-thenable, both closeAll paths, and full resolveActorsHost derivation. No meaningful gaps remain.


Verdict: No blockers. On the inline question — I'd keep Room (or ActorRoom) rather than Actor, since Actor is the server-side base class in actor.ts and this class models a single room, not the actor type. Everything else is optional polish. Nice work. 👍
feat/realtime-handler

actors.X(id) now returns an ActorRef whose only method is connect(); connect()
returns a Connection with { id, subscribe, send, close }. subscribe/send are only
reachable from a live Connection, so the "connect() before subscribe/send" guards
are gone (invalid state is unrepresentable). Renames the Room handle away entirely
(ActorRoom -> Connection + ActorRef). Chained usage
(actors.X(id).connect().subscribe(...)) is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ImriKochWix's task —— View job


PR Review: Actors namespace (connect → Connection API)

  • Read changed files
  • Verify prior fixes & address the Actor naming question
  • Assess code quality, bugs, performance, security, tests
  • Post review

Re-reviewed the current tree. The API has been refactored since the last pass into a cleaner two-step model: actors.MyActor(id) returns an ActorRef, and ref.connect() returns a Connection (subscribe/send/close). This directly answers @netanelgilad's inline question — see below. All prior functional blockers remain resolved. No blockers.


💬 Re: @netanelgilad"shouldn't we call this Actor?" (actors.ts)

The current naming already resolves this well, and I'd keep it as-is rather than rename to Actor:

  • Actor is takensrc/actor.ts:54 exports abstract class Actor, the type-only base users extend server-side (class MyActor extends Actor). A second client-side Actor would collide conceptually.
  • The class isn't an actor — it's a live connection to one instance. The class is now named Connection (actors.ts:32) and implements the public Connection interface (actors.types.ts:63), which is accurate: it wraps a single socket for one (actorName, instanceId).
  • The intermediate handle is ActorRef (actors.ts:121) — "a reference to one actor instance." Also accurate.

So the trio ActorRefConnection reads cleanly and avoids the Actor collision. I'd leave it.


🟢 State of prior items (all verified resolved)

  • Cleanup wiring ✅ — createActorsModule returns { module, closeAll } (actors.ts:168-173); each Connection self-removes via onClose (:131-133, :116); closeAll iterates a [...connections] snapshot so mid-loop close() is safe (:171); client.cleanup() invokes it (client.ts:219). Both paths tested (actors.test.ts:160, :169).
  • Proxy thenable ✅ — get returns undefined for symbols and then (actors.ts:162); tested (actors.test.ts:155).
  • Stale connId ✅ — close()onClose nulls conn, so a fresh connect() mints a new socket/id (actors.ts:133); tested (actors.test.ts:109).
  • Heartbeat timing + fv ✅ — both covered (actors.test.ts:139, :132).
  • resolveActorsHost derivation ✅ — extracted, exported, covered across absolute/empty/relative/no-origin (actors.test.ts:179-197).

🟡 Minor (non-blocking, optional)

1. send() doc vs. immediate-sendactors.types.ts:70 says a send is "Buffered by the socket until it's open." Since a Connection is only obtainable after connect() constructs the PartySocket (:48), the internal socket always exists by the time you hold a Connection, so PartySocket's buffering does cover the realistic path here — the refactor actually made this doc more accurate than before. 👍 No change needed.

2. resolveActorsHost only treats a leading / as relativeactors.ts:148-150. A scheme-less host ("api.example.com", protocol-relative "//host") is treated as absolute and passed verbatim to PartySocket. Fine given callers pass https://… or ""//api; just flagging the bare-host edge.

3. Heartbeat costactors.ts:24-25 (PING_MS=1s / DEAD_MS=3s, one ping/sec per connection). Accepted per the server-side setWebSocketAutoResponse mitigation (edge answers __ping__pong without waking the DO). Fine for interactive actors.

4. crypto.randomUUID() vs uuidv4()actors.ts:46. Fine on the Node ≥22 baseline; purely a consistency nit with utils/axios-client.ts. Keeping native is defensible.

5. Duplicate blank lines in actor.ts:13, :29, :37 regions. Trivial cosmetic; drop if your formatter flags them.


🔒 Security

Nothing concerning. Token rides the WS query and is re-read on every (re)connect via the query() closure (actors.ts:54-62), so login/logout is picked up; anonymous connects omit it (actors.test.ts:119). Precedence token || getAccessToken() (client.ts:176) matches the functions module. Inbound messages are JSON-parsed inside a try/catch (actors.ts:72-76) with __pong frames swallowed — no unsafe eval or trust of message shape.


🧪 Test coverage

Strong and complete — tests/unit/actors.test.ts covers Proxy dispatch, idempotent connect, multi-listener/unsubscribe, __pong swallowing, send serialization, close teardown, close()connect() re-open with fresh id, anonymous connect, per-id independence, fv present/absent, heartbeat ping+reconnect, not-thenable, both closeAll paths, and full resolveActorsHost derivation. No meaningful gaps remain.

One small note: the docs elsewhere still describe the older subscribe-throws-before-connect model, but the code no longer has a pre-connect state (you only get a Connection post-connect()), so there's nothing to test there — the refactor removed that failure mode entirely. Clean.


Verdict: No blockers. The ActorRefConnection refactor is a nice simplification — it eliminates the pre-connect guard state and makes the naming question moot (Connection ≠ the server-side Actor base). Remaining items are optional polish. Nice work getting this over the line. 👍

View jobfeat/realtime-handler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-draft PR has auto-drafted documentation suggestions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants