Claude encountered an error —— View job

I'll analyze this and get back to you.

🚀 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 Package: @base44-preview/sdk@0.8.40-pr.212.6649074
• 🔗 View this commit on GitHub

Preview published to npm registry — try new features instantly!

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

Claude encountered an error —— View job

I'll analyze this and get back to you.

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 risk — src/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 OPEN — src/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 aggressive — src/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 key — src/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 stale — src/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.
• functionsVersion → fv: 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 job • feat/realtime-handler

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 risk — src/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 OPEN — src/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 aggressive — src/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 key — src/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 stale — src/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.
• functionsVersion → fv: 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 job • feat/realtime-handler

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 risk — src/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 OPEN — src/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 aggressive — src/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 key — src/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 stale — src/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.
• functionsVersion → fv: 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 job • feat/realtime-handler

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 finished @ImriKochWix's task —— View job

PR Review: Actors namespace (room-handle API)

• Read changed files
• Verify author's fixes (SDK Improvements #1 cleanup, reload page after logout if no redirect #4 Proxy thenable, Add Claude Code GitHub Workflow #5 connId)
• Analyze remaining items & test coverage
• Post review

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 — resolved ✅ createActorsModule 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 — resolved ✅ close() 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 onClose — actors.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.
• functionsVersion → fv: 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 job • feat/realtime-handler

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. behavior — actors.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 cost — actors.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().
• functionsVersion → fv: 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 job • feat/realtime-handler

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 behavior — actors.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 validated — actors.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 cost — actors.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 job • feat/realtime-handler

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-send — actors.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 cost — actors.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

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-send — actors.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 cost — actors.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 job • feat/realtime-handler

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 relative — actors.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 source — client.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-send — actors.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 cost — actors.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 job • feat/realtime-handler

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 relative — actors.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 source — client.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-send — actors.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 cost — actors.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 job • feat/realtime-handler

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 relative — actors.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 source — client.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-send — actors.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 cost — actors.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 job • feat/realtime-handler

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 taken — src/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 relative — actors.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 reconciliation — client.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-send — actors.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

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 taken — src/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 ActorRef → Connection 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-send — actors.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 relative — actors.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 cost — actors.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 ActorRef → Connection 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 job • feat/realtime-handler