[codex] Make realtime delivery node-only#214
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds node and observer token contracts, updates realtime/auth and delivery routing to use node transport, migrates spawn/release into action invocations, and propagates node role and delivery metadata through SDKs, routes, tests, docs, and trajectory records. ChangesNode transport and observer-token split
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.0).agentworkforce/trajectories/active/traj_nzewjwo33lb9/trajectory.jsonFile contains syntax errors that prevent linting: Line 46: End of file expected; Line 46: Invalid exponent part; Line 46: String values must be double quoted.; Line 46: Missing closing quote; Line 47: End of file expected .agentworkforce/trajectories/active/traj_l5zzs0j95qli/trajectory.jsonFile contains syntax errors that prevent linting: Line 122: String values must be double quoted.; Line 122: Missing closing quote; Line 123: End of file expected; Line 123: End of file expected; Line 123: End of file expected; Line 123: End of file expected; Line 124: End of file expected; Line 124: End of file expected; Line 124: End of file expected; Line 124: End of file expected; Line 125: End of file expected; Line 125: End of file expected; Line 125: End of file expected; Line 129: End of file expected 🔧 OpenGrep (1.23.0).agentworkforce/trajectories/completed/2026-06/traj_fd17bm29mln9/trajectory.json┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.17][ERROR]: unable to find a config; path .agentworkforce/trajectories/completed/2026-06/traj_fff8ie8vivf0.trace.json┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.20][ERROR]: unable to find a config; path .agentworkforce/trajectories/completed/2026-06/traj_fff8ie8vivf0/trajectory.json┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.20][ERROR]: unable to find a config; path
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the node model by splitting the node kind (transport) from the node role (ownership/capability) and consolidating WebSocket delivery onto a single node envelope. Feedback highlights a critical edge case where direct nodes could be created with unlimited capacity if max_agents is set to 0, bypassing the single-agent restriction in both createNodeToken and the node creation route. Additionally, it is recommended to remove the static default: broker from the OpenAPI schema for the role field to prevent client SDKs from overriding the server's dynamic defaulting logic.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+198
to
+200
| if (role === 'direct' && maxAgents > 1) { | ||
| throw codedError('Direct nodes can bind at most one agent', 'direct_node_capacity_exceeded', 400); | ||
| } |
There was a problem hiding this comment.
A direct node is designed to bind at most one agent. However, if max_agents is explicitly set to 0 (which represents unlimited capacity in the binding logic), the current check maxAgents > 1 evaluates to false (since 0 > 1 is false). This allows a direct node to be created with unlimited capacity, bypassing the single-agent restriction.
We should enforce that maxAgents is exactly 1 for direct nodes.
Suggested change
| if (role === 'direct' && maxAgents > 1) { | |
| throw codedError('Direct nodes can bind at most one agent', 'direct_node_capacity_exceeded', 400); | |
| } | |
| if (role === 'direct' && maxAgents !== 1) { | |
| throw codedError('Direct nodes can bind at most one agent', 'direct_node_capacity_exceeded', 400); | |
| } |
Member
Author
There was a problem hiding this comment.
Already enforced — createNodeToken in engine/node.ts throws direct_node_capacity_exceeded (400) when role === 'direct' && maxAgents !== 1, so max_agents: 0 is rejected for direct nodes. No change needed.
Comment on lines
+107
to
+109
| if (role === 'direct' && (parsed.data.max_agents ?? existing?.maxAgents ?? 1) > 1) { | ||
| return jsonError(c, 'direct_node_capacity_exceeded', 'direct nodes can bind at most one agent', 400); | ||
| } |
There was a problem hiding this comment.
Similar to the check in createNodeToken, if max_agents is explicitly set to 0 (unlimited), the check maxAgents > 1 will evaluate to false and allow the request to proceed. We should ensure that direct nodes are strictly limited to a capacity of 1 and cannot be configured with 0 (unlimited) capacity.
Suggested change
| if (role === 'direct' && (parsed.data.max_agents ?? existing?.maxAgents ?? 1) > 1) { | |
| return jsonError(c, 'direct_node_capacity_exceeded', 'direct nodes can bind at most one agent', 400); | |
| } | |
| if (role === 'direct' && (parsed.data.max_agents ?? existing?.maxAgents ?? 1) !== 1) { | |
| return jsonError(c, 'direct_node_capacity_exceeded', 'direct nodes can bind at most one agent', 400); | |
| } |
Member
Author
There was a problem hiding this comment.
Already enforced — routes/node.ts returns direct_node_capacity_exceeded (400) for role === 'direct' with capacity ≠ 1, covering the max_agents: 0 case. No change needed.
Comment on lines
+3051
to
+3055
| role: | ||
| type: string | ||
| enum: [direct, broker] | ||
| default: broker | ||
| description: Node ownership/capability role. `direct` nodes bind one agent; `broker` nodes may host multiple agents and use node control frames. |
There was a problem hiding this comment.
Specifying default: broker for the role field in the OpenAPI schema can cause generated client SDKs or API explorers to automatically populate and send "broker" when the field is omitted. This overrides the server's dynamic defaulting logic, which is designed to default http_push and poll nodes to "direct".
It is safer to remove the default: broker property from the schema and document the dynamic defaults in the description instead.
role:
type: string
enum: [direct, broker]
description: Node ownership/capability role. Defaults to `broker` for `ws` nodes, and `direct` for `http_push` or `poll` nodes. `direct` nodes bind one agent; `broker` nodes may host multiple agents and use node control frames.
Member
Author
There was a problem hiding this comment.
Done — the node-create role schema has no default: and its description documents the dynamic defaults (ws → broker, http_push/poll → direct). Confirmed in 5f24882.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 195358d72a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const deliveryAdapter = normalizeDeliveryAdapter(target?.deliveryAdapter ?? delivery.deliveryAdapter, nodeKind); | ||
|
|
||
| if (locationType === 'via_node' && locationNodeId && nodeKind === 'fleet_ws') { | ||
| if (locationType === 'via_node' && locationNodeId && deliveryAdapter === 'ws.node.v1') { |
There was a problem hiding this comment.
When the recipient is an implicit direct node (node_direct_*), this branch now routes the delivery as a ws.node.v1 deliver frame; the Node adapter special-cases that by pushing the raw type: "deliver" frame to the normal /v1/ws agent socket. The SDK subscription path still wires subscribe() only to message.created, thread.reply, dm.received, and group_dm.received handlers, so directly connected agents using the documented realtime API/README quickstart no longer receive message callbacks even though the delivery is marked delivered. Either translate direct-node frames back to client events or update the SDKs to consume deliver frames before marking them delivered.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Routing verified correct: direct-node deliveries are sent as well-formed ws.node.v1 deliver frames and only marked delivered when sendToNode succeeds. The SDKs were updated to consume deliver frames (Python/TypeScript/Rust/Swift now connect via /v1/node/ws as a direct node and translate deliver payloads back to client events), so directly-connected agents receive callbacks. See 5f24882.
| ?? defaultAdapter(kind, deliveryConfig); | ||
| const maxAgents = data.max_agents ?? existing?.maxAgents ?? (kind === 'http_push' ? 1 : 0); | ||
| const maxAgents = data.max_agents ?? existing?.maxAgents ?? (role === 'direct' ? 1 : 0); | ||
| if (role === 'direct' && maxAgents > 1) { |
There was a problem hiding this comment.
Because max_agents is allowed to be 0 and reserveNodeAgentSlot treats maxAgents = 0 as unlimited capacity, a request such as POST /v1/nodes with role: "direct" and max_agents: 0 passes this > 1 check and can then bind multiple agents to a node advertised as direct/node-of-one. Reject 0 for direct nodes or coerce it to 1 so the role invariant is enforced.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Already enforced — both createNodeToken (engine) and the /v1/nodes route reject role: 'direct' with capacity ≠ 1 (incl. max_agents: 0) via direct_node_capacity_exceeded. No change needed.
willwashburn
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/engine/src/engine/node.ts (1)
343-368: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep direct-node heartbeat state clamped.
Direct heartbeats skip capability updates and clamp
maxAgents, but still persistmessage.active_agentsandmessage.handlers_live. A direct node can therefore publishactiveAgents > 1orhandlersLive: true, breaking the role invariants used by routing/capacity.Proposed fix
+ const [heartbeatNodeRow] = await db + .select({ role: nodes.role }) + .from(nodes) + .where(and(eq(nodes.workspaceId, workspaceId), eq(nodes.id, nodeId))); + const isDirect = heartbeatNodeRow?.role === 'direct'; + const rosterUpdate: Partial<typeof nodes.$inferInsert> = {}; if (message.name !== undefined) rosterUpdate.name = message.name; - if (message.capabilities !== undefined) { - const [node] = await db - .select({ role: nodes.role }) - .from(nodes) - .where(and(eq(nodes.workspaceId, workspaceId), eq(nodes.id, nodeId))); - if (node?.role !== 'direct') { - rosterUpdate.capabilities = normalizeCapabilities(message.capabilities); - } + if (message.capabilities !== undefined && !isDirect) { + rosterUpdate.capabilities = normalizeCapabilities(message.capabilities); } if (message.max_agents !== undefined) { - const [node] = await db - .select({ role: nodes.role }) - .from(nodes) - .where(and(eq(nodes.workspaceId, workspaceId), eq(nodes.id, nodeId))); - rosterUpdate.maxAgents = node?.role === 'direct' ? 1 : message.max_agents; + rosterUpdate.maxAgents = isDirect ? 1 : message.max_agents; } if (message.version !== undefined) rosterUpdate.version = message.version; const [updated] = await db .update(nodes) .set({ ...rosterUpdate, status: 'online', load: message.load, - activeAgents: message.active_agents, - handlersLive: message.handlers_live, + activeAgents: isDirect ? 1 : message.active_agents, + handlersLive: isDirect ? false : message.handlers_live, lastHeartbeatAt: new Date(), })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/engine/node.ts` around lines 343 - 368, The direct-node heartbeat handling in node.ts only clamps capabilities and maxAgents, but still writes through message.active_agents and message.handlers_live, which can violate direct-node invariants. Update the heartbeat update flow so the node role check also constrains these fields when the node role is direct, keeping activeAgents at 1 and handlersLive false before the db.update(nodes) call. Use the existing node role lookup logic in the heartbeat path to centralize the direct-node clamping alongside the current rosterUpdate handling.packages/engine/src/engine/nodeContext.ts (1)
127-149: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftKeep presence context scoped to the subject agent.
args.subjectAgentIdis no longer used, so every presence update is sent to all active via-node agents in the workspace. That can leak presence context to unrelated agents and creates unnecessary fanout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/engine/nodeContext.ts` around lines 127 - 149, The presence lookup in nodeContext is too broad because args.subjectAgentId is no longer used, so the query now returns every active via-node agent in the workspace. Update the query built in the presence context path to scope results to the subject agent by adding the subject-agent filter on agentNodeBindings/agents (using args.subjectAgentId) before broadcasting, and keep the existing workspace/status/node-kind constraints intact.packages/engine/src/routes/deliveryRouting.ts (1)
376-401: 🩺 Stability & Availability | 🟠 MajorFix routing for
http_pushnodes relying on legacy schema defaultsThe
normalizeDeliveryAdapterlogic prioritizes any existingdeliveryAdaptervalue overnodeKind. Since thenodestable defaultsdelivery_adapterto'ws.node.v1', anyhttp_pushnode created without an explicit adapter (relying on the DB default) is incorrectly routed to the WebSocket branch.Add a specific mismatch guard to coerce
'ws.node.v1'to'http.basic.v1'whennodeKind === 'http_push':function normalizeDeliveryAdapter(adapter: string | null | undefined, nodeKind: string | null | undefined): string | null { if (!adapter) { if (nodeKind === 'http_push') return 'http.basic.v1'; if (nodeKind === 'ws' || nodeKind === 'fleet_ws' || nodeKind === 'direct_ws') return 'ws.node.v1'; return null; } if (adapter === 'fleet.ws.v1' || adapter === 'direct.ws.v1') return 'ws.node.v1'; // Fixes misrouting for http_push nodes that inherited the 'ws.node.v1' schema default if (nodeKind === 'http_push' && adapter === 'ws.node.v1') return 'http.basic.v1'; if (nodeKind === 'ws' && adapter === 'http.basic.v1') return 'ws.node.v1'; return adapter; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/routes/deliveryRouting.ts` around lines 376 - 401, The routing logic in deliveryRouting’s normalizeDeliveryAdapter is letting the nodes table default adapter override http_push behavior, which sends legacy http_push nodes down the WebSocket path. Update normalizeDeliveryAdapter to detect the nodeKind/http_push and ws.node.v1 mismatch and coerce it to http.basic.v1, while preserving the existing ws/http fallback behavior for the other node kinds. Make sure the fix is applied before the via_node branches that call sendToNode and dispatchHttpPush so http_push targets route correctly.
🧹 Nitpick comments (5)
packages/sdk-swift/Sources/Relaycast/AgentClient.swift (1)
163-181: 🩺 Stability & Availability | 🔵 TrivialClarify token persistence, but note the lack of per-reconnect fetching
Relaycast direct node tokens (
nt_live_*) are long-lived and do not expire automatically, so the immediate risk of expiration on reconnect is low. However, this implementation fetches the token once, whereas the TypeScript client uses a token fetch function to retrieve a fresh token on every reconnection attempt.Using a static token means that if the credentials are manually rotated or invalidated externally (e.g., by an admin), the client will continue using the stale token and fail to reconnect indefinitely. Consider refactoring to use a closure or helper that fetches a fresh token whenever
configureNodeTransportis invoked.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sdk-swift/Sources/Relaycast/AgentClient.swift` around lines 163 - 181, The direct node token handling in AgentClient should not cache a one-time token fetch, because configureNodeTransport may need a fresh token on each reconnect attempt. Refactor the Task block around self.me() and self.client.post("/v1/agent/node-token") so the token comes from a closure or helper that is invoked whenever the transport is configured, matching the reconnect behavior used by the TypeScript client. Keep the existing DirectNodeRegistration setup, but ensure socket.connect() can rely on a freshly retrieved token instead of a stale nt_live_* value.packages/sdk-rust/tests/parity.rs (1)
2283-2323: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the decoded
node_rolecontract.The binding fixtures now include
node_role, but the test only checksagent_name/priority. Add assertions so role regressions don’t pass silently.Proposed test assertions
let bindings = relay .list_node_agents("http-node") .await .expect("list_node_agents failed"); assert_eq!(bindings[0].agent_name, "billing-agent"); + assert_eq!(bindings[0].node_role, "direct"); @@ let binding = relay .bind_agent_to_node( "http-node", @@ .await .expect("bind_agent_to_node failed"); assert_eq!(binding.priority, 5); + assert_eq!(binding.node_role, "direct");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sdk-rust/tests/parity.rs` around lines 2283 - 2323, The parity test for list_node_agents/bind_agent_to_node is missing coverage for the decoded node_role field, so role regressions could slip through. In the existing assertions around relay.list_node_agents and relay.bind_agent_to_node, add checks on the returned binding objects’ node_role using the values from the fixture/response (for example, alongside the existing agent_name and priority assertions) so the contract is verified by the test.packages/sdk-python/src/relay_sdk/models.py (1)
385-385: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
NodeRoleon role-bearing fields.
NodeRoleis introduced here, but these fields still accept any string. Reuse the literal type for validation; move the alias aboveDeliveryif applying it toroute_node_role.Proposed typing cleanup
- route_node_role: str | None = None + route_node_role: NodeRole | None = None @@ - role: str | None = None + role: NodeRole | None = None @@ - node_role: str + node_role: NodeRoleAlso applies to: 455-455, 495-495
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sdk-python/src/relay_sdk/models.py` at line 385, The role-bearing fields still use plain string annotations instead of the new NodeRole literal, so tighten their types for validation. Update route_node_role and the other affected fields in the Delivery-related models to use NodeRole rather than str | None, and move the NodeRole alias above Delivery if needed so the type is available where it is referenced.packages/engine/src/engine/action.ts (1)
308-312: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a zod schema for release input validation.
This adds ad-hoc shape checks for a new action contract; a small schema keeps validation consistent with the rest of the TS surface. As per coding guidelines, “Prefer zod schemas for validation instead of ad-hoc manual checks.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/engine/action.ts` around lines 308 - 312, The release input validation in action.ts is using manual shape checks on recordInput(args.data.input) instead of the project’s preferred zod-based validation. Replace the ad-hoc typeof input.name check in the action handling path with a dedicated zod schema for the release input contract, and use that schema to parse/validate before proceeding. Keep the validation logic near the existing release action handling symbols (recordInput, codedError, and the release input branch) so the contract stays consistent with the rest of the TS surface.Source: Coding guidelines
packages/engine/src/routes/deliveryRouting.ts (1)
28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
nodeRoleis resolved intoDeliveryTargetbut never consumed by routing.
nodeRoleis selected and populated acrossresolveLiveLocations/resolveRecordedTargets, butrouteOneDeliveryOutcomekeys only ondeliveryAdapter/nodeKind. If it's intended purely for future use or serialized elsewhere, ignore; otherwise it's dead data on this path.Also applies to: 180-180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/routes/deliveryRouting.ts` at line 28, The DeliveryTarget field nodeRole is populated by resolveLiveLocations and resolveRecordedTargets but is never used by routeOneDeliveryOutcome, which currently routes only by deliveryAdapter and nodeKind. Either remove nodeRole from this delivery routing path if it is not needed, or wire it into the routing logic where DeliveryTarget is consumed so the resolved value affects dispatch decisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openapi.yaml`:
- Around line 989-995: The success envelope schemas in the affected response
definitions are too loose: `ok` is not constrained to true and `ok`/`data` are
not required. Update the response schema blocks for the referenced success
responses so they explicitly require `ok` and `data`, and constrain `ok` to the
literal true value while keeping `data` referenced to the existing payload
schema. Use the existing success response definitions in the OpenAPI document as
the targets to keep the `{ ok: true, data: ... }` contract consistent.
In `@packages/engine/src/adapters/node/realtime.ts`:
- Around line 313-316: The best-effort flush in attachAgentSocket() is targeting
deliverPendingToNode(), but that path uses sendToNode() and cannot reach the
newly attached agent socket because directNodeIdForAgent(agentId) is not
registered in nodeSockets. Update the flush logic in attachAgentSocket() so
pending deliveries are sent through the socket registry that can actually
deliver them, using the existing attachAgentSocket/getAgent/nodeSockets flow and
the directNodeIdForAgent(agentId) mapping as the reference point.
In `@packages/engine/src/auth/index.ts`:
- Around line 59-70: The requireAuth logic in auth/index.ts is too permissive
because it allows nt_live_ node tokens to pass when require is 'any', which lets
node credentials reach workspace-bound routes. Update the token dispatch in
requireAuth so that node tokens are rejected for user-facing/workspace routes
and only workspace/agent-appropriate keys are accepted there, while keeping
node-token authentication available only through requireNodeToken. Preserve the
existing unauthorized responses and use the current require branches and token
prefix checks in requireAuth to tighten the access rules.
In `@packages/engine/src/engine/action.ts`:
- Around line 321-340: After the release flow in action.ts (the release path
around createInvocation and dispatchNodeInvocation), clear the agent’s node
location only when the active node binding was actually deactivated. Update the
agent row so locationType and locationNodeId no longer point to the released
node after a successful binding change, and make sure the logic in the release
completion path also guards against re-running the decrement when no active
binding exists. Apply the same fix to the other release-related block referenced
by the comment so both paths keep agent location state and active binding state
in sync.
- Around line 770-784: In action.ts, the release path in the invocation handling
flow can fall through to generic placement when targetAgentForInvocation()
returns null because release invocations have no actionId. Add an explicit guard
in the invocation dispatch logic around targetAgentForInvocation() and
dispatchNodeInvocation() so release actions are handled only by the intended
agent-owned node path and do not continue into the fallback placement logic.
In `@packages/engine/src/engine/node.ts`:
- Line 1235: The post-`node.register` flush in `deliverPendingToNode` is causing
a second error reply if it throws after `sendControl(... ok: true ...)` has
already succeeded. Update the `node.ts` register flow so the
`deliverPendingToNode(args.db, args.registry, args.workspaceId, args.nodeId)`
call is best-effort/log-only and cannot bubble to the outer catch; if needed,
handle its failure locally in the register handler and keep the existing
successful reply path from `sendControl` unchanged.
- Around line 191-204: Recompute both the delivery adapter and maxAgents
whenever the node shape changes in node.ts, instead of always reusing
existing.deliveryAdapter and existing.maxAgents when delivery/max_agents are
omitted. Update the logic around normalizeLegacyNodeShape, defaultAdapter, and
the direct-node capacity check so that changing kind or role (for example
http_push to ws, or broker to direct) derives a fresh adapter and resets
capacity appropriately, while only preserving existing values when the shape has
not changed.
In `@packages/engine/src/routes/node.ts`:
- Around line 87-90: The node enrollment defaults in the route handler are too
permissive: in the node creation/update flow, omitted non-ws roles can fall
through to single-agent direct behavior and capacity 1, which can change
broker-style nodes unexpectedly. Update the defaulting logic around the parsed
node fields in the node route so that when `role` is omitted for non-`ws` kinds
like `http_push` or `poll`, it resolves to `broker` unless the caller explicitly
sets `direct`, and make the capacity handling in the same flow preserve broker
semantics when `max_agents` is absent. Check the `kind`, `role`, and capacity
logic together where the request is normalized before persistence.
In `@packages/mcp/src/__tests__/server-multi-workspace.test.ts`:
- Around line 45-54: The release mock in server-multi-workspace.test.ts uses the
wrong input shape, causing the MCP test to drift from the actual agent.remove
payload. Update the release fixture in the mocked invocation data to match the
release invocation’s real input shape by using the same field expected by the
release path in the server/multi-workspace flow, and keep the rest of the mock
aligned with the release actionName and invocationId.
In `@packages/sdk-python/tests/test_ws.py`:
- Around line 232-234: The token assertion in
test_connect_url_includes_origin_query_params is inconsistent with the WsClient
setup, causing the test to fail. Update the test so the expected token matches
the value passed when constructing WsClient in the test setup, or change the
constructor token to match the asserted URL parameter. Use the existing
test_connect_url_includes_origin_query_params and WsClient references to keep
the token expectation and client initialization aligned.
In `@packages/sdk-rust/src/ws.rs`:
- Around line 404-407: The auto-ack path in ws.rs is ignoring send failures from
write.send inside the node_delivery_ack flow, so a failed ack can be silently
dropped while the delivery still proceeds. Update the
node_registration.is_some() branch to handle the Message::Text ack write result
explicitly in ws.rs, and if the send fails, stop treating the delivery as
successfully acknowledged by surfacing or logging the error through the
surrounding websocket handling in this area.
In `@packages/sdk-typescript/src/ws.ts`:
- Around line 414-476: `handleNodeMessage` is bypassing the replay-deduplication
path for `nodeMode` deliver frames, so duplicate `deliver` events can be
re-emitted after reconnect. Update `handleNodeMessage` to apply the same
`seenEventIds`/`rememberEventId` validation used by the normal `onmessage` flow
before calling `emitServerLikeEvent`, especially for the `deliver` branch, so
redelivered frames are ignored consistently.
In `@README.md`:
- Around line 393-401: The node websocket path is inconsistent with the rest of
the docs. Update the transport description in README so the broker-controlled
agent route used by the relevant node delivery section matches the established
`/v1/node/ws` path referenced elsewhere, keeping the wording aligned with the
realtime transport and SDK flow docs.
---
Outside diff comments:
In `@packages/engine/src/engine/node.ts`:
- Around line 343-368: The direct-node heartbeat handling in node.ts only clamps
capabilities and maxAgents, but still writes through message.active_agents and
message.handlers_live, which can violate direct-node invariants. Update the
heartbeat update flow so the node role check also constrains these fields when
the node role is direct, keeping activeAgents at 1 and handlersLive false before
the db.update(nodes) call. Use the existing node role lookup logic in the
heartbeat path to centralize the direct-node clamping alongside the current
rosterUpdate handling.
In `@packages/engine/src/engine/nodeContext.ts`:
- Around line 127-149: The presence lookup in nodeContext is too broad because
args.subjectAgentId is no longer used, so the query now returns every active
via-node agent in the workspace. Update the query built in the presence context
path to scope results to the subject agent by adding the subject-agent filter on
agentNodeBindings/agents (using args.subjectAgentId) before broadcasting, and
keep the existing workspace/status/node-kind constraints intact.
In `@packages/engine/src/routes/deliveryRouting.ts`:
- Around line 376-401: The routing logic in deliveryRouting’s
normalizeDeliveryAdapter is letting the nodes table default adapter override
http_push behavior, which sends legacy http_push nodes down the WebSocket path.
Update normalizeDeliveryAdapter to detect the nodeKind/http_push and ws.node.v1
mismatch and coerce it to http.basic.v1, while preserving the existing ws/http
fallback behavior for the other node kinds. Make sure the fix is applied before
the via_node branches that call sendToNode and dispatchHttpPush so http_push
targets route correctly.
---
Nitpick comments:
In `@packages/engine/src/engine/action.ts`:
- Around line 308-312: The release input validation in action.ts is using manual
shape checks on recordInput(args.data.input) instead of the project’s preferred
zod-based validation. Replace the ad-hoc typeof input.name check in the action
handling path with a dedicated zod schema for the release input contract, and
use that schema to parse/validate before proceeding. Keep the validation logic
near the existing release action handling symbols (recordInput, codedError, and
the release input branch) so the contract stays consistent with the rest of the
TS surface.
In `@packages/engine/src/routes/deliveryRouting.ts`:
- Line 28: The DeliveryTarget field nodeRole is populated by
resolveLiveLocations and resolveRecordedTargets but is never used by
routeOneDeliveryOutcome, which currently routes only by deliveryAdapter and
nodeKind. Either remove nodeRole from this delivery routing path if it is not
needed, or wire it into the routing logic where DeliveryTarget is consumed so
the resolved value affects dispatch decisions.
In `@packages/sdk-python/src/relay_sdk/models.py`:
- Line 385: The role-bearing fields still use plain string annotations instead
of the new NodeRole literal, so tighten their types for validation. Update
route_node_role and the other affected fields in the Delivery-related models to
use NodeRole rather than str | None, and move the NodeRole alias above Delivery
if needed so the type is available where it is referenced.
In `@packages/sdk-rust/tests/parity.rs`:
- Around line 2283-2323: The parity test for list_node_agents/bind_agent_to_node
is missing coverage for the decoded node_role field, so role regressions could
slip through. In the existing assertions around relay.list_node_agents and
relay.bind_agent_to_node, add checks on the returned binding objects’ node_role
using the values from the fixture/response (for example, alongside the existing
agent_name and priority assertions) so the contract is verified by the test.
In `@packages/sdk-swift/Sources/Relaycast/AgentClient.swift`:
- Around line 163-181: The direct node token handling in AgentClient should not
cache a one-time token fetch, because configureNodeTransport may need a fresh
token on each reconnect attempt. Refactor the Task block around self.me() and
self.client.post("/v1/agent/node-token") so the token comes from a closure or
helper that is invoked whenever the transport is configured, matching the
reconnect behavior used by the TypeScript client. Keep the existing
DirectNodeRegistration setup, but ensure socket.connect() can rely on a freshly
retrieved token instead of a stale nt_live_* value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 07e08128-d547-4045-bdc9-448e39ce06e9
📒 Files selected for processing (67)
README.mdopenapi.yamlpackages/engine/src/__tests__/conformance/delivery.test.tspackages/engine/src/__tests__/conformance/harness.tspackages/engine/src/__tests__/conformance/node.test.tspackages/engine/src/__tests__/conformance/nodeDeliveryContracts.test.tspackages/engine/src/__tests__/conformance/sdk-contract.test.tspackages/engine/src/__tests__/nodeUpgradeAuth.test.tspackages/engine/src/adapters/node/realtime.tspackages/engine/src/auth/index.tspackages/engine/src/db/migrations/0025_node_kind_role_adapter.sqlpackages/engine/src/db/schema.tspackages/engine/src/engine.tspackages/engine/src/engine/__tests__/invocationCompletion.test.tspackages/engine/src/engine/action.tspackages/engine/src/engine/delivery.tspackages/engine/src/engine/deliveryWire.tspackages/engine/src/engine/deliveryWrites.tspackages/engine/src/engine/invocationCompletion.tspackages/engine/src/engine/node.tspackages/engine/src/engine/nodeContext.tspackages/engine/src/engine/nodeDeliver.tspackages/engine/src/engine/wsAuth.tspackages/engine/src/engine/wsTransform.tspackages/engine/src/entrypoints/node.tspackages/engine/src/env.tspackages/engine/src/middleware/auth.tspackages/engine/src/ports/auth.tspackages/engine/src/ports/realtime.tspackages/engine/src/routes/action.tspackages/engine/src/routes/agent.tspackages/engine/src/routes/deliveryRouting.tspackages/engine/src/routes/node.tspackages/engine/src/routes/reaction.tspackages/engine/src/routes/receipt.tspackages/mcp/src/__tests__/server-multi-workspace.test.tspackages/mcp/src/tools/programmability.tspackages/sdk-python/src/relay_sdk/models.pypackages/sdk-python/src/relay_sdk/ws.pypackages/sdk-python/tests/test_relay.pypackages/sdk-python/tests/test_ws.pypackages/sdk-rust/README.mdpackages/sdk-rust/src/agent.rspackages/sdk-rust/src/lib.rspackages/sdk-rust/src/registration.rspackages/sdk-rust/src/types.rspackages/sdk-rust/src/ws.rspackages/sdk-rust/tests/parity.rspackages/sdk-swift/Sources/Relaycast/AgentClient.swiftpackages/sdk-swift/Sources/Relaycast/Models.swiftpackages/sdk-swift/Sources/Relaycast/WsClient.swiftpackages/sdk-swift/Tests/RelaycastTests/RelaycastTests.swiftpackages/sdk-typescript/README.mdpackages/sdk-typescript/src/__tests__/agent-ws.test.tspackages/sdk-typescript/src/__tests__/communicate.test.tspackages/sdk-typescript/src/__tests__/invite-consumption.test.tspackages/sdk-typescript/src/agent.tspackages/sdk-typescript/src/types.tspackages/sdk-typescript/src/ws.tspackages/types/fixtures/fleet-wire/action.invoke.jsonpackages/types/fixtures/fleet-wire/deliver.jsonpackages/types/src/__tests__/types.test.tspackages/types/src/agent.tspackages/types/src/delivery.tspackages/types/src/events.tspackages/types/src/fleet-wire.tspackages/types/src/telemetry.ts
💤 Files with no reviewable changes (3)
- packages/engine/src/entrypoints/node.ts
- packages/engine/src/engine/wsTransform.ts
- packages/types/src/telemetry.ts
* Add scoped observer tokens * Add observer token SDK parity coverage * Address observer token review feedback * Tighten observer token scope filtering * Block observer access to DM channels * Align observer file and stream filters * Close observer stream DM thread gaps * Filter observer node and channel results * Make observer DM gating authoritative * Remove relay workspace state and SDK workflow Delete generated relay workspace metadata (memory/workspace/.relay/outbox/capabilities.json and memory/workspace/.relay/state.json) and remove the large SDK setup workflow script (workflows/sdk-setup-client-80-100.ts). These are cleanup deletions of workspace state/artifact files and an obsolete workflow; no functional source code changes were made. * Align observer auth contracts * Clean up migration leftovers and React races * Remove dead dashboard auth and feed code * Validate channel message attachments * Wire API usage quotas
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/e2e-actions.ts (1)
175-195: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy liftUpdate the E2E to use node transport frames.
This test still relies on agent tokens receiving workspace WebSocket events, but
/v1/wsnow rejectsat_live_*. Mint/v1/agent/node-token, connect to/v1/node/ws, register the direct node, then assert the handler receivesaction.invokeand the caller receives the completion through adeliverpayload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/e2e-actions.ts` around lines 175 - 195, The e2e flow in `scripts/e2e-actions.ts` is still using workspace WebSocket/token behavior, but it now needs to exercise the node transport path instead. Update the setup around the WebSocket clients and `req('POST', '/v1/actions/...')` flow to mint `/v1/agent/node-token`, connect through `/v1/node/ws`, register the direct node, and then assert the handler gets `action.invoke` and the caller receives the completion via a `deliver` payload instead of the old workspace event path.packages/engine/src/routes/agent.ts (1)
372-376: 🔒 Security & Privacy | 🔴 CriticalObserver tokens can invoke spawn/release endpoints
The
requireAuthmiddleware is instantiated with'any'and infersrequire === 'observer'for tokens starting withot_live_(lines 79-80 inmiddleware/auth.ts). Whilepackages/engine/src/auth/index.tscorrectly rejects observer tokens for operations requiring'workspace','agent', or'node'roles (lines 41, 56, 73), it explicitly allows observer tokens whenrequire === 'observer'(lines 83-91).Since
requireAuthdelegates to this logic with no explicit role override, the middleware successfully authenticates observer tokens for/agents/spawnand/agents/release. This creates a security vulnerability where read-only observers can trigger state-changing lifecycle events.Replace
requireAuthwith a role-restricted middleware likerequireWorkspaceKeyor ensurerequireAuthrejects observer tokens for mutation endpoints.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/routes/agent.ts` around lines 372 - 376, The /agents/spawn (and matching release) route currently uses requireAuth, which allows observer tokens via the auth flow in middleware/auth.ts and auth/index.ts. Update the agent route to use a stricter middleware such as requireWorkspaceKey, or otherwise make requireAuth reject observer tokens for these mutation endpoints so only workspace-scoped credentials can invoke agent spawn/release actions. Locate the fix in agentRoutes and the requireAuth-based authorization path.packages/engine/src/routes/message.ts (1)
221-227: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftObserver filtering after pagination can hide matching messages.
This applies
observerAllowsMessage()only aftermessageEngine.getMessages()has already enforcedlimit/before/after. For observer tokens withagent_idsorcreated_afterfilters, a page can come back empty or under-filled even though matching older rows still exist, because the filtered-out rows already consumed the page window. The observer constraints need to be pushed into the query (or the route needs to keep fetching until it fills the page or exhausts history).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/routes/message.ts` around lines 221 - 227, The message route is filtering with observerAllowsMessage() only after messageEngine.getMessages() has already applied pagination, which can drop matching rows for observer tokens with agent_ids or created_after constraints. Update the message retrieval flow in the route and/or messageEngine.getMessages() so observer criteria are part of the query itself, or keep fetching until the page is filled before returning jsonOk; use the existing observerAllowsMessage, getMessages, and pagination parameters (limit/before/after) as the entry points.
🟡 Minor comments (11)
packages/react/src/__tests__/provider.test.tsx-52-53 (1)
52-53: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse block bodies for the
forEachcallbacks.Biome flags these expression callbacks because they return
handler(event). Wrap the calls so the callback returns nothing.Proposed fix
- this.handlers.get(type)?.forEach((handler) => handler(event)); - this.handlers.get('*')?.forEach((handler) => handler(event)); + this.handlers.get(type)?.forEach((handler) => { + handler(event); + }); + this.handlers.get('*')?.forEach((handler) => { + handler(event); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/__tests__/provider.test.tsx` around lines 52 - 53, The `forEach` callbacks in `provider.test.tsx` are using expression bodies that implicitly return `handler(event)`, which Biome flags; update the callbacks in the event dispatch logic on `this.handlers.get(type)` and `this.handlers.get('*')` to use block bodies so they only invoke `handler(event)` and return nothing.Source: Linters/SAST tools
packages/sdk-python/src/relay_sdk/ws.py-101-107 (1)
101-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHonor explicit
node_registrationbefore implicit agent-token transport.Right now an
at_token withnode_registration=still takes the/v1/agent/node-tokenpath unless the caller also setspath, so the new explicit node config is silently ignored on a reachable code path.Suggested fix
async def _transport(self) -> tuple[str, str, dict[str, Any] | None, bool]: - if self._path is None and self._token.startswith("at_"): - token, path, registration = await self._resolve_agent_node_transport() - return token, path, registration, True if self._node_registration is not None: return self._token, self._path or "/v1/node/ws", self._node_registration, self._auto_ack_deliveries + if self._path is None and self._token.startswith("at_"): + token, path, registration = await self._resolve_agent_node_transport() + return token, path, registration, True return self._token, self._path or "/v1/ws", None, self._auto_ack_deliveries🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sdk-python/src/relay_sdk/ws.py` around lines 101 - 107, The _transport method is checking the agent-token path before honoring an explicitly provided _node_registration, so node_registration can be ignored for at_ tokens. Update _transport in ws.py to prioritize self._node_registration first, then fall back to the implicit _resolve_agent_node_transport() flow only when no explicit node registration is set. Keep the existing defaults for /v1/node/ws and /v1/ws when _path is unset.packages/sdk-typescript/src/relay.ts-133-136 (1)
133-136: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClarify that
connect()still requires an observer token.
RelayCaststill pipesapiKeystraight intoWsClient, so/v1/wsonly works when this is anot_*token. The new wording reads like anrk_*client can also use workspace observation, which will send consumers down the wrong path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sdk-typescript/src/relay.ts` around lines 133 - 136, The doc comment on the relay API is misleading because `RelayCast` still passes `apiKey` directly into `WsClient`, so `connect()` only works with an observer token. Update the wording near `RelayCast`/`connect()` to state that workspace realtime observation requires an `ot_*` token, and that `rk_*` admin keys are for administration only, so consumers are not led to use the wrong credential type.packages/sdk-rust/tests/parity.rs-2149-2259 (1)
2149-2259: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winUse token-less fixtures on metadata reads.
observer_payloadis reused for list/get/update and still carries"token": "ot_live_secret". That means this parity test will pass even if metadata endpoints start leaking raw observer-token material, and it never exercises the normalNone/missing-token case that the Swift suite already covers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sdk-rust/tests/parity.rs` around lines 2149 - 2259, The observer-token parity test is reusing a payload that includes the live token for metadata-only reads, so it does not verify the expected token-less shape. Split the fixture in the parity test around create/rotate versus list/get/update, and use a metadata fixture for the read/update paths that omits the token field or sets it to None; keep the existing token-bearing payload only for create and rotate assertions in create_observer_token, list_observer_tokens, get_observer_token, update_observer_token, and rotate_observer_token.openapi.yaml-1194-1200 (1)
1194-1200: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winConstrain observer-token request scopes in OpenAPI.
The engine rejects scopes outside
OBSERVER_SCOPES, but these request schemas documentscopes.itemsas any string, so generated clients can send payloads the server rejects. Add the same enum here, and consider reusing a typedObserverTokenFiltersschema instead of baretype: object.As per coding guidelines, “Keep
README.mdandopenapi.yamlaligned with actual behavior.”Also applies to: 1300-1306
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi.yaml` around lines 1194 - 1200, The observer-token request schemas currently allow any string for scopes and an untyped filters object, which does not match server validation. Update the relevant request schemas in openapi.yaml, including the referenced observer-token sections, so scopes.items uses the same OBSERVER_SCOPES enum as the engine, and replace the bare filters object with the existing ObserverTokenFilters schema or an equivalent typed schema. Keep the OpenAPI definition aligned with the actual request validation used by the token-related endpoints.Source: Coding guidelines
packages/engine/src/routes/channel.ts-52-57 (1)
52-57: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winRemove the mixed-case channel fallback shape.
channelTypeis accepted here but never normalized before callingobserverAllowsChannel, so callers with camelCase data can bypass thechannel_typecheck. Keep this helper snake_case-only and normalize at the source. As per coding guidelines, “Do not introduce mixed-case field fallbacks” and “HTTP JSON wire fields are snake_case.”Proposed fix
function observerChannelAllowed( observer: ObserverToken | undefined, - channel: { id?: string | null; name?: string | null; channel_type?: number | null; channelType?: number | null }, + channel: { id?: string | null; name?: string | null; channel_type?: number | null }, ) { return observerAllowsChannel(observer, channel); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/routes/channel.ts` around lines 52 - 57, The helper observerChannelAllowed is still accepting a mixed-case channelType fallback even though it is passed through unchanged to observerAllowsChannel, so remove the camelCase field from this shape and keep the helper snake_case-only. Normalize any camelCase input at the source before it reaches observerChannelAllowed, and ensure observerAllowsChannel and related channel checks rely only on channel_type.Source: Coding guidelines
packages/engine/src/routes/channel.ts-463-464 (1)
463-464: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the invite result’s resolved agent id.
The new
agent_idcan beundefinedif the post-invite member lookup misses or name normalization differs. Emit the resolved id frominviteAgent’s result instead.Proposed fix
- const invitedMember = members.find((member) => member.agent_name === agentName); - const eventData = { channel_name: name, agent_id: invitedMember?.agent_id, agent_name: agentName }; + const eventData = { channel_name: name, agent_id: result.agent_id, agent_name: agentName };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/routes/channel.ts` around lines 463 - 464, The invite event is currently re-reading the member list to populate agent_id, which can miss the newly invited member and leave it undefined. Update the channel invite flow in the inviteAgent path to use the resolved agent id returned by inviteAgent itself when building eventData, instead of looking it up again via members.find. Keep the existing channel_name and agent_name fields, but ensure the emitted event always carries the id from the invite result.packages/engine/src/__tests__/conformance/usageQuota.test.ts-28-35 (1)
28-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the
stackRefreturn type.
stackisTestStack | undefined, so() => stackdoes not satisfy the constructor’s() => TestStackcontract under strict TypeScript checking. Use a definite assignment for this suite-local stack or make the provider handle the undefined case explicitly.Proposed fix
describe('usage quotas', () => { - let stack: TestStack | undefined; + let stack: TestStack; - afterEach(() => stack?.close()); + afterEach(() => stack.close());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/__tests__/conformance/usageQuota.test.ts` around lines 28 - 35, The `ApiCallQuotaEntitlements` provider in `usageQuota.test.ts` is returning `stack` from a `TestStack | undefined`, which does not satisfy the required `() => TestStack` type. Update the suite-local stack handling around `stack` and `makeNodeStack` so the provider in the `it('records authenticated API calls...')` test always returns a definite `TestStack` value, either by using a definite assignment pattern for `stack` or by explicitly handling the undefined case before passing the callback.packages/engine/src/routes/console.ts-55-55 (1)
55-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAllow DM-scoped observers through the console message log route.
This endpoint can return DM logs after
filterObserverSearchResults, butrequireWorkspaceRead('messages:read')rejects observers that only havedms:read. Use both read scopes and let the existing event/resource filter trim the result set.Proposed fix
-consoleRoutes.get('/console/messages', requireWorkspaceRead('messages:read'), rateLimit, async (c) => { +consoleRoutes.get('/console/messages', requireWorkspaceRead(['messages:read', 'dms:read']), rateLimit, async (c) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/routes/console.ts` at line 55, The console message log route currently blocks DM-scoped observers because it only uses requireWorkspaceRead('messages:read'). Update the consoleRoutes.get('/console/messages', ...) handler to accept both read scopes, so users with either messages:read or dms:read can reach this endpoint. Keep the existing filterObserverSearchResults/event-resource filtering in place to trim the returned results as before.packages/engine/src/__tests__/conformance/observerToken.test.ts-835-848 (1)
835-848: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReplace the fixed 50ms sleeps with polling assertions.
These stream checks are timing-sensitive right now. A busy CI runner can miss the 50ms window and fail the test even when delivery is correct. Please switch these waits to a retry/polling assertion helper instead of a hard sleep.
Also applies to: 907-983
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/__tests__/conformance/observerToken.test.ts` around lines 835 - 848, The conformance test in observerToken.test.ts uses fixed 50ms sleeps before checking stream delivery, which makes the assertions flaky on slower CI runs. Replace the hard waits in the affected stream-delivery checks with a polling/retry assertion helper so the test waits until the expected events appear or a timeout is reached. Update the relevant assertions around the message.created and message.read checks, and apply the same pattern to the other timing-sensitive blocks noted in the test.packages/engine/src/engine/message.ts-36-41 (1)
36-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve punctuation-adjacent mentions.
The new regex only matches
@nameat the start or after whitespace, so(@alice)andhey,@alice`` won’t trigger mention delivery or appear inmentions. Use a non-email boundary instead.Proposed fix
- const mentionPattern = /(?:^|\s)@(\w+)/g; + const mentionPattern = /(^|[^\w.@])@(\w+)/g; const mentionedHandles = new Set<string>(); for (let match = mentionPattern.exec(data.text); match !== null; match = mentionPattern.exec(data.text)) { - mentionedHandles.add(match[1]); + mentionedHandles.add(match[2]); }Also applies to: 112-138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/engine/message.ts` around lines 36 - 41, The mention parsing in the message handling logic is too restrictive because it only matches handles at the start of a string or after whitespace, so punctuation-adjacent mentions like (`@alice`) and hey,`@alice` are missed. Update the mention extraction in message.ts (the loop using mentionPattern and mentionedHandles) to use a non-email boundary approach that still excludes email addresses while allowing punctuation before `@mentions`, and make sure the downstream mention delivery/mentions list logic uses the same corrected matching behavior.
🧹 Nitpick comments (1)
packages/engine/src/routes/observerToken.ts (1)
40-54: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake the top-level observer token request schemas strict.
These two Zod objects still strip unknown keys by default, so payloads with typos or mixed-case fields like
expiresAtget accepted and silently ignored instead of failing validation. That weakens the wire contract for a new public API.Suggested change
const createObserverTokenSchema = z.object({ name: z.string().min(1), description: z.string().nullable().optional(), scopes: z.array(observerScopeSchema).min(1), filters: observerFiltersSchema.optional(), expires_at: isoTimestamp.nullable().optional(), -}); +}).strict(); const updateObserverTokenSchema = z.object({ name: z.string().min(1).optional(), description: z.string().nullable().optional(), scopes: z.array(observerScopeSchema).min(1).optional(), filters: observerFiltersSchema.optional(), expires_at: isoTimestamp.nullable().optional(), -}); +}).strict();As per coding guidelines, HTTP JSON wire fields are snake_case and “Do not add mixed-case compatibility fallbacks.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/routes/observerToken.ts` around lines 40 - 54, Make the top-level observer token request schemas strict by applying strict validation to both createObserverTokenSchema and updateObserverTokenSchema so unknown keys are rejected instead of stripped. Update the Zod objects in observerToken.ts to fail on unexpected payload fields, while keeping the existing snake_case request fields like expires_at and avoiding any mixed-case compatibility fallbacks such as expiresAt.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openapi.yaml`:
- Around line 4177-4188: The `/v1/ws` realtime auth documentation in
openapi.yaml is inconsistent with the actual Hono route behavior because the
route only reads the query token, not an Authorization header. Either update the
websocket handling path to accept bearer tokens consistently or remove the
Authorization-header recommendation from the `description` and `token` parameter
docs so the OpenAPI spec matches what `/v1/ws` actually supports.
In `@packages/engine/src/adapters/node/realtime.ts`:
- Around line 55-58: The workspace socket authorization flow is caching the full
observer token in WorkspaceConn, so revoked or narrowed tokens remain valid on
existing sockets. Update attachWorkspaceSocket() and publishToWorkspaceStream()
to revalidate against the current token state by token id on each publish, or
ensure token mutations (PATCH/rotate/DELETE) actively disconnect any matching
WorkspaceConn sockets. Use the existing WorkspaceConn, attachWorkspaceSocket,
and publishToWorkspaceStream symbols to keep the fix aligned with the current
connection lifecycle.
In `@packages/engine/src/engine/console.ts`:
- Around line 172-203: The console log query in listMessageLogsForWindow is
loading every matching row into memory, which then makes the summary helpers
scan an unbounded result set. Update the console aggregation path (including the
helpers that feed /console/agents and /console/costs) to use SQL-side
grouping/aggregation or add a strict paging/cap before calling publicMessageLog,
so the query returns a bounded set instead of materializing all logs.
In `@packages/engine/src/engine/dm.ts`:
- Around line 183-184: Validate send attachments before creating the DM
conversation to avoid orphaned DM metadata. In the DM flow around
resolveConversation and resolveSendAttachments in dm.ts, move the attachment
resolution ahead of conversation creation so invalid attachments fail early.
Keep the existing resolveSendAttachments and resolveConversation logic intact,
but ensure the conversation/channel/participant rows are only created after
attachments have successfully validated.
In `@packages/engine/src/engine/observerToken.ts`:
- Around line 502-515: Skip channel filtering for workspace-wide events that do
not carry channel context. In observerToken.ts within the event routing logic
around observerAllowsChannel and observerAllowsConversation, only apply channel
checks for event types that actually have a channel scope, and treat
agent.status.* and similar non-channel events as eligible for agent_id matching
even when channel_ids/channel_names are set. Keep rejecting true channel-scoped
events when channel_id/channel name metadata is missing, but bypass
observerAllowsChannel for events without channel context so observerAllowsAgent
can still match.
In `@packages/engine/src/routes/channel.ts`:
- Around line 136-151: The channel read handlers in channel.ts still return
unfiltered membership details for observer tokens, which can expose agents
outside the observer’s allowed scope. Update the channel response assembly in
the relevant route handlers (the one using getObserverTokenFromContext,
getChannelObserverResource, and channelEngine.getChannel, plus the
member-listing path around the other referenced block) to apply
observerAllowsAgent to every member/agent payload before returning it. Make sure
only agents permitted by the observer’s agent_ids filters are included in the
returned details and member lists, and preserve the existing channelNotFound
behavior for disallowed observers.
In `@packages/engine/src/routes/console.ts`:
- Around line 51-53: The console message event classification in
consoleMessageLogEventTypes is too broad because every conversation_id log is
returned as both DM event types. Update the filtering logic to use the log’s
real DM vs group-DM classification, or ensure the console log row carries that
classification before filtering, so observers only match the correct event type
instead of all conversation logs.
In `@packages/engine/src/routes/fanout.ts`:
- Line 11: The node fanout skip set in fanoutToChannel/NODE_DELIVERY_EVENT_TYPES
still allows message.read to be delivered as node-context, causing duplicate
receipt frames. Update the fanout filtering so message.read is excluded
alongside the existing event types, and make sure the node delivery path in
sendNodeDeliveriesForChannel remains the only route for receipt events.
In `@packages/engine/src/routes/node.ts`:
- Around line 178-184: The node bindings route in
nodeRoutes.get('/nodes/:name/agents') should match the single-node visibility
behavior when observer filtering hides everything. After calling listNodeAgents
and filterNodeAgentsForObserver, if the filtered result is empty while the
underlying node is not visible to the caller, return jsonNotFound with the same
node_not_found response instead of 200 []; use the existing nodeRoutes,
listNodeAgents, filterNodeAgentsForObserver, and getObserverTokenFromContext
flow to locate the change.
In `@packages/engine/src/routes/reaction.ts`:
- Around line 14-15: The reaction handlers still use the legacy realtime fanout
path alongside node deliveries, causing duplicate events. In the add/remove
reaction flow in reaction.ts, remove the fanout block that calls fanoutToAgents
and fanoutToChannel, and keep only sendNodeDeliveriesForChannel for delivery.
Also drop the now-unused fanout imports and make sure the reaction delivery path
stays node-only through the relevant handler functions.
In `@packages/engine/src/routes/search.ts`:
- Around line 24-28: The REST search route currently relies on
requireWorkspaceRead with the default allowNode behavior, which lets node tokens
access /v1/search. Update the searchRoutes.get("/search", ...) setup to pass {
allowNode: false } explicitly so the auth policy matches the documented REST
surface; keep the change localized to the requireWorkspaceRead call in
searchRoutes.
In `@packages/engine/src/routes/workspace.ts`:
- Around line 12-18: The DM conversation preview filtering in workspace routing
is missing agent-level enforcement, so observers with agent_ids can still see
previews for disallowed senders. Update the conversation preview path in
workspace.ts, especially the logic around observerAllowsConversation and the
preview timestamp check, to also validate the preview’s last_message agent using
the existing observerAllowsMessage/observerAllowsCreatedAt helpers and/or
filterObserverSearchResults. Make sure the preview is only returned when both
the conversation scope and the last_message agent filters are satisfied.
In `@packages/observer-dashboard/src/components/ChatFeed.tsx`:
- Around line 221-228: Clear the stale DM list immediately when the selected
conversation changes in ChatFeed’s useEffect so the previous conversation’s
messages do not remain visible while relay.dmMessages is loading. Update the
effect around requestSeq.current, setLoading, and setMessages so the messages
state is reset before starting the new fetch, while still preserving the
existing sequence guard and toMessageWithMeta mapping.
- Around line 242-246: The stale-response path in loadOlderPage is incorrectly
signaling exhaustion by returning 0, which makes usePaginatedFeed treat the
conversation as having no more history after a switch. Update the loadOlderPage
callback in ChatFeed so that requestSeq mismatches return a non-exhaustion
sentinel (or otherwise preserve the previous pagination state) instead of 0,
while keeping the valid older-page return value unchanged for the current
conversation.
In `@packages/sdk-python/src/relay_sdk/ws.py`:
- Around line 195-230: The final fallback in _handle_node_frame() is swallowing
generic control frames by returning True for reply, error, pong, and resync_ack,
which prevents the outer message loop from emitting them. Update this helper so
it only returns True for frames it fully handles (deliver, context.update,
action.invoke) and lets unhandled control frames propagate to the existing event
emission path; use _handle_node_frame() and the surrounding frame dispatch logic
to keep on("error"), on("pong"), on("reply"), and wildcard handlers working.
---
Outside diff comments:
In `@packages/engine/src/routes/agent.ts`:
- Around line 372-376: The /agents/spawn (and matching release) route currently
uses requireAuth, which allows observer tokens via the auth flow in
middleware/auth.ts and auth/index.ts. Update the agent route to use a stricter
middleware such as requireWorkspaceKey, or otherwise make requireAuth reject
observer tokens for these mutation endpoints so only workspace-scoped
credentials can invoke agent spawn/release actions. Locate the fix in
agentRoutes and the requireAuth-based authorization path.
In `@packages/engine/src/routes/message.ts`:
- Around line 221-227: The message route is filtering with
observerAllowsMessage() only after messageEngine.getMessages() has already
applied pagination, which can drop matching rows for observer tokens with
agent_ids or created_after constraints. Update the message retrieval flow in the
route and/or messageEngine.getMessages() so observer criteria are part of the
query itself, or keep fetching until the page is filled before returning jsonOk;
use the existing observerAllowsMessage, getMessages, and pagination parameters
(limit/before/after) as the entry points.
In `@scripts/e2e-actions.ts`:
- Around line 175-195: The e2e flow in `scripts/e2e-actions.ts` is still using
workspace WebSocket/token behavior, but it now needs to exercise the node
transport path instead. Update the setup around the WebSocket clients and
`req('POST', '/v1/actions/...')` flow to mint `/v1/agent/node-token`, connect
through `/v1/node/ws`, register the direct node, and then assert the handler
gets `action.invoke` and the caller receives the completion via a `deliver`
payload instead of the old workspace event path.
---
Minor comments:
In `@openapi.yaml`:
- Around line 1194-1200: The observer-token request schemas currently allow any
string for scopes and an untyped filters object, which does not match server
validation. Update the relevant request schemas in openapi.yaml, including the
referenced observer-token sections, so scopes.items uses the same
OBSERVER_SCOPES enum as the engine, and replace the bare filters object with the
existing ObserverTokenFilters schema or an equivalent typed schema. Keep the
OpenAPI definition aligned with the actual request validation used by the
token-related endpoints.
In `@packages/engine/src/__tests__/conformance/observerToken.test.ts`:
- Around line 835-848: The conformance test in observerToken.test.ts uses fixed
50ms sleeps before checking stream delivery, which makes the assertions flaky on
slower CI runs. Replace the hard waits in the affected stream-delivery checks
with a polling/retry assertion helper so the test waits until the expected
events appear or a timeout is reached. Update the relevant assertions around the
message.created and message.read checks, and apply the same pattern to the other
timing-sensitive blocks noted in the test.
In `@packages/engine/src/__tests__/conformance/usageQuota.test.ts`:
- Around line 28-35: The `ApiCallQuotaEntitlements` provider in
`usageQuota.test.ts` is returning `stack` from a `TestStack | undefined`, which
does not satisfy the required `() => TestStack` type. Update the suite-local
stack handling around `stack` and `makeNodeStack` so the provider in the
`it('records authenticated API calls...')` test always returns a definite
`TestStack` value, either by using a definite assignment pattern for `stack` or
by explicitly handling the undefined case before passing the callback.
In `@packages/engine/src/engine/message.ts`:
- Around line 36-41: The mention parsing in the message handling logic is too
restrictive because it only matches handles at the start of a string or after
whitespace, so punctuation-adjacent mentions like (`@alice`) and hey,`@alice` are
missed. Update the mention extraction in message.ts (the loop using
mentionPattern and mentionedHandles) to use a non-email boundary approach that
still excludes email addresses while allowing punctuation before `@mentions`, and
make sure the downstream mention delivery/mentions list logic uses the same
corrected matching behavior.
In `@packages/engine/src/routes/channel.ts`:
- Around line 52-57: The helper observerChannelAllowed is still accepting a
mixed-case channelType fallback even though it is passed through unchanged to
observerAllowsChannel, so remove the camelCase field from this shape and keep
the helper snake_case-only. Normalize any camelCase input at the source before
it reaches observerChannelAllowed, and ensure observerAllowsChannel and related
channel checks rely only on channel_type.
- Around line 463-464: The invite event is currently re-reading the member list
to populate agent_id, which can miss the newly invited member and leave it
undefined. Update the channel invite flow in the inviteAgent path to use the
resolved agent id returned by inviteAgent itself when building eventData,
instead of looking it up again via members.find. Keep the existing channel_name
and agent_name fields, but ensure the emitted event always carries the id from
the invite result.
In `@packages/engine/src/routes/console.ts`:
- Line 55: The console message log route currently blocks DM-scoped observers
because it only uses requireWorkspaceRead('messages:read'). Update the
consoleRoutes.get('/console/messages', ...) handler to accept both read scopes,
so users with either messages:read or dms:read can reach this endpoint. Keep the
existing filterObserverSearchResults/event-resource filtering in place to trim
the returned results as before.
In `@packages/react/src/__tests__/provider.test.tsx`:
- Around line 52-53: The `forEach` callbacks in `provider.test.tsx` are using
expression bodies that implicitly return `handler(event)`, which Biome flags;
update the callbacks in the event dispatch logic on `this.handlers.get(type)`
and `this.handlers.get('*')` to use block bodies so they only invoke
`handler(event)` and return nothing.
In `@packages/sdk-python/src/relay_sdk/ws.py`:
- Around line 101-107: The _transport method is checking the agent-token path
before honoring an explicitly provided _node_registration, so node_registration
can be ignored for at_ tokens. Update _transport in ws.py to prioritize
self._node_registration first, then fall back to the implicit
_resolve_agent_node_transport() flow only when no explicit node registration is
set. Keep the existing defaults for /v1/node/ws and /v1/ws when _path is unset.
In `@packages/sdk-rust/tests/parity.rs`:
- Around line 2149-2259: The observer-token parity test is reusing a payload
that includes the live token for metadata-only reads, so it does not verify the
expected token-less shape. Split the fixture in the parity test around
create/rotate versus list/get/update, and use a metadata fixture for the
read/update paths that omits the token field or sets it to None; keep the
existing token-bearing payload only for create and rotate assertions in
create_observer_token, list_observer_tokens, get_observer_token,
update_observer_token, and rotate_observer_token.
In `@packages/sdk-typescript/src/relay.ts`:
- Around line 133-136: The doc comment on the relay API is misleading because
`RelayCast` still passes `apiKey` directly into `WsClient`, so `connect()` only
works with an observer token. Update the wording near `RelayCast`/`connect()` to
state that workspace realtime observation requires an `ot_*` token, and that
`rk_*` admin keys are for administration only, so consumers are not led to use
the wrong credential type.
---
Nitpick comments:
In `@packages/engine/src/routes/observerToken.ts`:
- Around line 40-54: Make the top-level observer token request schemas strict by
applying strict validation to both createObserverTokenSchema and
updateObserverTokenSchema so unknown keys are rejected instead of stripped.
Update the Zod objects in observerToken.ts to fail on unexpected payload fields,
while keeping the existing snake_case request fields like expires_at and
avoiding any mixed-case compatibility fallbacks such as expiresAt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1daac4ff-5989-4629-aa7f-d88a3e5567d2
📒 Files selected for processing (90)
README.mdmemory/workspace/.relay/outbox/capabilities.jsonmemory/workspace/.relay/state.jsonopenapi.yamlpackages/engine/src/__tests__/conformance/harness.tspackages/engine/src/__tests__/conformance/messageAttachments.test.tspackages/engine/src/__tests__/conformance/node.test.tspackages/engine/src/__tests__/conformance/nodeDeliveryContracts.test.tspackages/engine/src/__tests__/conformance/observerToken.test.tspackages/engine/src/__tests__/conformance/usageQuota.test.tspackages/engine/src/__tests__/nodeUpgradeAuth.test.tspackages/engine/src/adapters/node/realtime.tspackages/engine/src/auth/index.tspackages/engine/src/db/migrations/0026_observer_tokens.sqlpackages/engine/src/db/schema.tspackages/engine/src/engine.tspackages/engine/src/engine/activity.tspackages/engine/src/engine/agent.tspackages/engine/src/engine/attachments.tspackages/engine/src/engine/console.tspackages/engine/src/engine/deliveryWrites.tspackages/engine/src/engine/dm.tspackages/engine/src/engine/groupDm.tspackages/engine/src/engine/message.tspackages/engine/src/engine/observerToken.tspackages/engine/src/engine/search.tspackages/engine/src/engine/thread.tspackages/engine/src/engine/wsAuth.tspackages/engine/src/engine/wsTransform.tspackages/engine/src/entrypoints/node.tspackages/engine/src/env.tspackages/engine/src/middleware/auth.tspackages/engine/src/middleware/planLimits.tspackages/engine/src/middleware/rateLimit.tspackages/engine/src/middleware/usageTracker.tspackages/engine/src/ports/auth.tspackages/engine/src/ports/entitlements.tspackages/engine/src/ports/realtime.tspackages/engine/src/providers/static-entitlements.tspackages/engine/src/routes/agent.tspackages/engine/src/routes/channel.tspackages/engine/src/routes/console.tspackages/engine/src/routes/deliveryRouting.tspackages/engine/src/routes/fanout.tspackages/engine/src/routes/file.tspackages/engine/src/routes/message.tspackages/engine/src/routes/node.tspackages/engine/src/routes/observerToken.tspackages/engine/src/routes/presence.tspackages/engine/src/routes/reaction.tspackages/engine/src/routes/receipt.tspackages/engine/src/routes/search.tspackages/engine/src/routes/thread.tspackages/engine/src/routes/workspace.tspackages/mcp/src/__tests__/ws-bridge.test.tspackages/mcp/src/resources/ws-bridge.tspackages/observer-dashboard/src/app/api/auth/check/route.tspackages/observer-dashboard/src/components/ChatFeed.tsxpackages/observer-dashboard/src/hooks/use-activity-feed.tspackages/observer-dashboard/src/lib/auth.tspackages/observer-dashboard/src/types/dashboard.tspackages/react/src/__tests__/fetchRaceGuards.test.tsxpackages/react/src/__tests__/provider.test.tsxpackages/react/src/hooks/useChannel.tspackages/react/src/hooks/useMessages.tspackages/react/src/hooks/useThread.tspackages/react/src/provider.tsxpackages/sdk-python/src/relay_sdk/models.pypackages/sdk-python/src/relay_sdk/relay.pypackages/sdk-python/src/relay_sdk/ws.pypackages/sdk-python/tests/test_models.pypackages/sdk-python/tests/test_relay.pypackages/sdk-python/tests/test_ws.pypackages/sdk-rust/src/lib.rspackages/sdk-rust/src/relay.rspackages/sdk-rust/src/types.rspackages/sdk-rust/tests/parity.rspackages/sdk-swift/Sources/Relaycast/Models.swiftpackages/sdk-swift/Sources/Relaycast/RelayCast.swiftpackages/sdk-swift/Tests/RelaycastTests/RelaycastTests.swiftpackages/sdk-typescript/src/__tests__/relay.test.tspackages/sdk-typescript/src/relay.tspackages/sdk-typescript/src/types.tspackages/types/src/command.tspackages/types/src/events.tspackages/types/src/index.tspackages/types/src/subscription.tspackages/types/src/workspace.tsscripts/e2e-actions.tsworkflows/sdk-setup-client-80-100.ts
💤 Files with no reviewable changes (12)
- memory/workspace/.relay/state.json
- memory/workspace/.relay/outbox/capabilities.json
- packages/types/src/command.ts
- packages/observer-dashboard/src/lib/auth.ts
- packages/observer-dashboard/src/hooks/use-activity-feed.ts
- packages/observer-dashboard/src/app/api/auth/check/route.ts
- packages/observer-dashboard/src/types/dashboard.ts
- packages/engine/src/engine/agent.ts
- packages/types/src/subscription.ts
- workflows/sdk-setup-client-80-100.ts
- packages/types/src/index.ts
- packages/types/src/events.ts
✅ Files skipped from review due to trivial changes (2)
- packages/engine/src/db/migrations/0026_observer_tokens.sql
- packages/react/src/tests/fetchRaceGuards.test.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/engine/src/env.ts
- packages/engine/src/tests/conformance/node.test.ts
- README.md
- packages/engine/src/auth/index.ts
- packages/engine/src/routes/deliveryRouting.ts
- packages/engine/src/entrypoints/node.ts
Comment on lines
+172
to
+203
| export async function listMessageLogsForWindow( | ||
| db: Db, | ||
| workspaceId: string, | ||
| windowDays = 7, | ||
| ): Promise<ConsoleMessageLog[]> { | ||
| const since = getWindowStart(windowDays); | ||
| const rows = await db | ||
| .select({ | ||
| id: messageLogs.id, | ||
| messageId: messageLogs.messageId, | ||
| channelId: messageLogs.channelId, | ||
| channelName: channels.name, | ||
| channelType: channels.channelType, | ||
| agentId: messageLogs.agentId, | ||
| agentName: agents.name, | ||
| conversationId: messageLogs.conversationId, | ||
| deliveryKind: messageLogs.deliveryKind, | ||
| body: messageLogs.body, | ||
| contentType: messageLogs.contentType, | ||
| metadata: messageLogs.metadata, | ||
| attachmentCount: messageLogs.attachmentCount, | ||
| mentionCount: messageLogs.mentionCount, | ||
| latencyMs: messageLogs.latencyMs, | ||
| createdAt: messageLogs.createdAt, | ||
| }) | ||
| .from(messageLogs) | ||
| .leftJoin(agents, eq(messageLogs.agentId, agents.id)) | ||
| .leftJoin(channels, eq(messageLogs.channelId, channels.id)) | ||
| .where(and(eq(messageLogs.workspaceId, workspaceId), gte(messageLogs.createdAt, since))) | ||
| .orderBy(desc(messageLogs.id)); | ||
|
|
||
| return rows.map(publicMessageLog); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Avoid unbounded in-memory console aggregation.
listMessageLogsForWindow materializes every log in the window, then the summary helpers iterate all rows. On busy workspaces, /console/agents or /console/costs can become slow or memory-heavy. Prefer SQL aggregation/grouping or enforce a bounded paging/cap before summarizing.
Also applies to: 206-340
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/engine/src/engine/console.ts` around lines 172 - 203, The console
log query in listMessageLogsForWindow is loading every matching row into memory,
which then makes the summary helpers scan an unbounded result set. Update the
console aggregation path (including the helpers that feed /console/agents and
/console/costs) to use SQL-side grouping/aggregation or add a strict paging/cap
before calling publicMessageLog, so the query returns a bounded set instead of
materializing all logs.
Member
Author
There was a problem hiding this comment.
Skipped (out of scope for this PR). The non-observer /console/* paths already use SQL aggregation; only the observer path materializes the window, and a bounded LIMIT can't be added safely because the summary helpers compute counts/costs over the full set (truncation would silently produce wrong aggregates). The correct fix — pushing observer filters into SQL aggregation — is a heavy refactor better done separately.
There was a problem hiding this comment.
@willwashburn Makes sense — thanks for the clarification.
Agreed: a LIMIT here would silently skew the observer-scoped stats, and it sounds like the existing non-observer /console/* paths already stay on SQL-side aggregation. Given that the real fix is pushing observer filters down into SQL, I’m okay leaving this out of this PR.
If you want, I can open a follow-up issue for the observer-side aggregation refactor.
✏️ Learnings added
Learnt from: willwashburn
Repo: AgentWorkforce/relaycast PR: 214
File: packages/engine/src/engine/console.ts:172-203
Timestamp: 2026-06-25T16:34:23.028Z
Learning: In `packages/engine/src/engine/console.ts` and the observer-scoped `/console/*` routes, the non-observer console paths already use SQL aggregation, while the observer path currently materializes the full window in memory. Adding a simple `LIMIT` is not safe there because it would silently produce incorrect counts/cost aggregates; the correct future fix is to push observer filtering into SQL aggregation.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Introduce a replay utility and add multiple AgentWorkforce trajectory artifacts. Adds .agentworkforce/replay-demo.py to rebuild the Relay Rush demo workspace (teardown, re-register agents, replay messages/reactions, checkpoint state). Also adds numerous trajectory JSONs, traces and summary files under .agentworkforce/trajectories (active and completed for 2026-06) to record work history and traces. Minor .gitignore update included.
Node delivery / capacity: - node.ts: recompute delivery adapter + maxAgents when node shape changes; make post-node.register pending flush best-effort to avoid double error reply - routes/node.ts: return node_not_found (not 200 []) for observer-hidden nodes - realtime.ts: drop dead no-op agent-socket flush; revalidate observer tokens at publish time (5s TTL) so revoked/narrowed tokens stop receiving - fanout.ts: add message.read and message.reacted to node-delivery skip-set to kill duplicate context frames Release flow: - action.ts: guard release double-decrement, clear agent node location on release, stop release retries from falling through to generic placement - mcp test: fix release fixture input shape (name vs agent_name) Auth / observer scoping: - auth/index.ts: reject node tokens on user/workspace routes (node tokens authenticate only via node transport) - search.ts: allowNode:false - observerToken.ts: skip channel filter for workspace-wide events so agent_ids can match; still fail closed for channel-scoped events missing metadata - channel.ts: filter member lists by observerAllowsAgent - routes/console.ts: classify conversation logs via channel_type, not both DM types - workspace.ts + dmAll.ts + types/workspace.ts: plumb agent_id into DM preview and filter previews via observerAllowsMessage Other correctness: - dm.ts: validate attachments before creating conversation rows - ChatFeed.tsx: clear stale messages on conversation switch; fix stale pagination - sdk-rust ws.rs: surface auto-ack send failures instead of dropping them - sdk-typescript ws.ts: dedupe node deliver frames via seenEventIds - sdk-python ws.py: stop _handle_node_frame swallowing reply/error/pong/resync_ack Docs: - openapi.yaml: /v1/ws is query-token only (no Authorization header) - README.md: align /node/ws -> /v1/node/ws Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/engine/src/auth/__tests__/tokenKind.test.ts (1)
51-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the allowed transport-token paths too.
This verifies node/observer tokens are forbidden elsewhere, but not that
nodetokens satisfyrequire: 'node'and observer tokens satisfyrequire: 'observer'—the core new auth paths.Suggested test addition
+ it('allows transport-only tokens on their own auth paths', () => { + expect(validateTokenRequirement('node', 'node')).toEqual({ ok: true }); + expect(validateTokenRequirement('observer', 'observer')).toEqual({ ok: true }); + }); + it('keeps transport-only tokens forbidden outside their own auth paths', () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/auth/__tests__/tokenKind.test.ts` around lines 51 - 62, The current tokenKind test only covers forbidden transport-token cases and misses the new allowed auth paths; extend the existing validateTokenRequirement coverage in tokenKind.test.ts to assert that a node token passes when the required kind is node and an observer token passes when the required kind is observer. Use the same validateTokenRequirement helper and keep the new assertions alongside the existing transport-only token checks so the intended behavior of node/observer auth paths is explicitly verified.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agentworkforce/trajectories/active/traj_l5zzs0j95qli/trajectory.json:
- Around line 122-129: Remove the stray trailing JSON fragment at the end of the
trajectory JSON so the file remains valid and parseable. Edit the
trajectory.json content to keep only the single complete JSON object that ends
cleanly before the duplicated garbage, and ensure no extra tokens remain after
the closing brace; the issue is in the final section of the file rather than in
any schema field like _trace or projectId.
In @.agentworkforce/trajectories/active/traj_nzewjwo33lb9/trajectory.json:
- Around line 46-48: Remove the stray trailing fragment after the final closing
brace so the trajectory JSON ends cleanly at the last valid object boundary.
Update the trajectory.json content to keep only the valid JSON structure and
ensure there are no extra characters after the final closing brace.
In
@.agentworkforce/trajectories/completed/2026-06/traj_a30u25uguo04/trajectory.json:
- Around line 13-23: The trajectory metadata is inconsistent with the described
work because commits/filesChanged are empty and _trace.startRef matches
_trace.endRef, making the completed entry look like a no-op. Update the
trajectory JSON so the provenance fields in the summary object are populated
with the actual changed files and commit refs, and ensure _trace reflects the
real before/after refs; if no code changed, revise the retrospective text to
match the recorded metadata. Use the existing top-level fields like commits,
filesChanged, and _trace to locate and correct the provenance data.
In `@packages/engine/src/adapters/node/realtime.ts`:
- Around line 218-236: The observer-token revalidation logic in realtime.ts
keeps retrying the database on every event after a loadActiveObserverToken()
exception because observerCheckedAt is never updated in the catch path. Update
the observer connection handling in the revalidation block so that, when
loadActiveObserverToken fails, conn.observerCheckedAt is still advanced to now
(or a backoff timestamp) while preserving the cached observerToken snapshot,
preventing repeated immediate retries for each workspace event.
---
Nitpick comments:
In `@packages/engine/src/auth/__tests__/tokenKind.test.ts`:
- Around line 51-62: The current tokenKind test only covers forbidden
transport-token cases and misses the new allowed auth paths; extend the existing
validateTokenRequirement coverage in tokenKind.test.ts to assert that a node
token passes when the required kind is node and an observer token passes when
the required kind is observer. Use the same validateTokenRequirement helper and
keep the new assertions alongside the existing transport-only token checks so
the intended behavior of node/observer auth paths is explicitly verified.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3351ed40-e334-4fa7-a91b-556a05b746d2
📒 Files selected for processing (126)
.agentworkforce/trajectories/active/traj_l5zzs0j95qli/trajectory.json.agentworkforce/trajectories/active/traj_nzewjwo33lb9/trajectory.json.agentworkforce/trajectories/compacted/compact_65oq3n7um67w_2026-06-25.json.agentworkforce/trajectories/compacted/compact_65oq3n7um67w_2026-06-25.md.agentworkforce/trajectories/completed/2026-06/traj_08tx11x73e0s.trace.json.agentworkforce/trajectories/completed/2026-06/traj_08tx11x73e0s/summary.md.agentworkforce/trajectories/completed/2026-06/traj_08tx11x73e0s/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_1cvz2te266aw/summary.md.agentworkforce/trajectories/completed/2026-06/traj_1cvz2te266aw/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_3a7q65ksviqc.trace.json.agentworkforce/trajectories/completed/2026-06/traj_3a7q65ksviqc/summary.md.agentworkforce/trajectories/completed/2026-06/traj_3a7q65ksviqc/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_3nn0bl3wtgfz/summary.md.agentworkforce/trajectories/completed/2026-06/traj_3nn0bl3wtgfz/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_5d1p83x8twu5.trace.json.agentworkforce/trajectories/completed/2026-06/traj_5d1p83x8twu5/summary.md.agentworkforce/trajectories/completed/2026-06/traj_5d1p83x8twu5/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_6u3au0fquyoj.trace.json.agentworkforce/trajectories/completed/2026-06/traj_6u3au0fquyoj/summary.md.agentworkforce/trajectories/completed/2026-06/traj_6u3au0fquyoj/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_890auw9ixqw1/summary.md.agentworkforce/trajectories/completed/2026-06/traj_890auw9ixqw1/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_93f25qjubagx/summary.md.agentworkforce/trajectories/completed/2026-06/traj_93f25qjubagx/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_983h5d585n4b.trace.json.agentworkforce/trajectories/completed/2026-06/traj_983h5d585n4b/summary.md.agentworkforce/trajectories/completed/2026-06/traj_983h5d585n4b/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_a30u25uguo04/summary.md.agentworkforce/trajectories/completed/2026-06/traj_a30u25uguo04/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_add28xzpsw8p/summary.md.agentworkforce/trajectories/completed/2026-06/traj_add28xzpsw8p/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_da4wt5z16dll.trace.json.agentworkforce/trajectories/completed/2026-06/traj_da4wt5z16dll/summary.md.agentworkforce/trajectories/completed/2026-06/traj_da4wt5z16dll/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_f98xy8yo6yio/summary.md.agentworkforce/trajectories/completed/2026-06/traj_f98xy8yo6yio/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_fd17bm29mln9.trace.json.agentworkforce/trajectories/completed/2026-06/traj_fd17bm29mln9/summary.md.agentworkforce/trajectories/completed/2026-06/traj_fd17bm29mln9/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_fff8ie8vivf0.trace.json.agentworkforce/trajectories/completed/2026-06/traj_fff8ie8vivf0/summary.md.agentworkforce/trajectories/completed/2026-06/traj_fff8ie8vivf0/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_fomjzvhz8uv0.trace.json.agentworkforce/trajectories/completed/2026-06/traj_fomjzvhz8uv0/summary.md.agentworkforce/trajectories/completed/2026-06/traj_fomjzvhz8uv0/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_gcua3hnucbil.trace.json.agentworkforce/trajectories/completed/2026-06/traj_gcua3hnucbil/summary.md.agentworkforce/trajectories/completed/2026-06/traj_gcua3hnucbil/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_gjqemr0lou4l.trace.json.agentworkforce/trajectories/completed/2026-06/traj_gjqemr0lou4l/summary.md.agentworkforce/trajectories/completed/2026-06/traj_gjqemr0lou4l/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_hs9nlqqktma6.trace.json.agentworkforce/trajectories/completed/2026-06/traj_hs9nlqqktma6/summary.md.agentworkforce/trajectories/completed/2026-06/traj_hs9nlqqktma6/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_jvzosckbsdmo/summary.md.agentworkforce/trajectories/completed/2026-06/traj_jvzosckbsdmo/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_kwsld40j6o0q.trace.json.agentworkforce/trajectories/completed/2026-06/traj_kwsld40j6o0q/summary.md.agentworkforce/trajectories/completed/2026-06/traj_kwsld40j6o0q/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_lgzs0cvgaldw.trace.json.agentworkforce/trajectories/completed/2026-06/traj_lgzs0cvgaldw/summary.md.agentworkforce/trajectories/completed/2026-06/traj_lgzs0cvgaldw/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_ol0yjdw87422/summary.md.agentworkforce/trajectories/completed/2026-06/traj_ol0yjdw87422/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_s26lxygworep/summary.md.agentworkforce/trajectories/completed/2026-06/traj_s26lxygworep/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_saln4k9fiil6.trace.json.agentworkforce/trajectories/completed/2026-06/traj_saln4k9fiil6/summary.md.agentworkforce/trajectories/completed/2026-06/traj_saln4k9fiil6/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_sfvyafvn0v4j.trace.json.agentworkforce/trajectories/completed/2026-06/traj_sfvyafvn0v4j/summary.md.agentworkforce/trajectories/completed/2026-06/traj_sfvyafvn0v4j/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_skw3lpvhwyd4.trace.json.agentworkforce/trajectories/completed/2026-06/traj_skw3lpvhwyd4/summary.md.agentworkforce/trajectories/completed/2026-06/traj_skw3lpvhwyd4/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_t8p6b889cxx5.trace.json.agentworkforce/trajectories/completed/2026-06/traj_t8p6b889cxx5/summary.md.agentworkforce/trajectories/completed/2026-06/traj_t8p6b889cxx5/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_to6gxc6ojsjz/summary.md.agentworkforce/trajectories/completed/2026-06/traj_to6gxc6ojsjz/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_uiyfvt9ydnxz.trace.json.agentworkforce/trajectories/completed/2026-06/traj_uiyfvt9ydnxz/summary.md.agentworkforce/trajectories/completed/2026-06/traj_uiyfvt9ydnxz/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_umo7p21qpv43/summary.md.agentworkforce/trajectories/completed/2026-06/traj_umo7p21qpv43/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_wm6vbs57cmut/summary.md.agentworkforce/trajectories/completed/2026-06/traj_wm6vbs57cmut/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_wzfxaqffmmzm/summary.md.agentworkforce/trajectories/completed/2026-06/traj_wzfxaqffmmzm/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_xpuanikzwt4t/summary.md.agentworkforce/trajectories/completed/2026-06/traj_xpuanikzwt4t/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_yteoa3d7jyit.trace.json.agentworkforce/trajectories/completed/2026-06/traj_yteoa3d7jyit/summary.md.agentworkforce/trajectories/completed/2026-06/traj_yteoa3d7jyit/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_z1kqrja8kfyc/summary.md.agentworkforce/trajectories/completed/2026-06/traj_z1kqrja8kfyc/trajectory.json.agentworkforce/trajectories/completed/2026-06/traj_zhlya22s096b.trace.json.agentworkforce/trajectories/completed/2026-06/traj_zhlya22s096b/summary.md.agentworkforce/trajectories/completed/2026-06/traj_zhlya22s096b/trajectory.json.gitignoreREADME.mdopenapi.yamlpackages/engine/src/adapters/node/realtime.tspackages/engine/src/auth/__tests__/tokenKind.test.tspackages/engine/src/auth/index.tspackages/engine/src/auth/tokenKind.tspackages/engine/src/engine/action.tspackages/engine/src/engine/dm.tspackages/engine/src/engine/dmAll.tspackages/engine/src/engine/node.tspackages/engine/src/engine/observerToken.tspackages/engine/src/engine/wsAuth.tspackages/engine/src/middleware/auth.tspackages/engine/src/middleware/logger.tspackages/engine/src/routes/channel.tspackages/engine/src/routes/console.tspackages/engine/src/routes/fanout.tspackages/engine/src/routes/node.tspackages/engine/src/routes/search.tspackages/engine/src/routes/workspace.tspackages/mcp/src/__tests__/server-multi-workspace.test.tspackages/observer-dashboard/src/components/ChatFeed.tsxpackages/sdk-python/src/relay_sdk/ws.pypackages/sdk-rust/src/ws.rspackages/sdk-typescript/src/ws.tspackages/types/src/workspace.ts
✅ Files skipped from review due to trivial changes (81)
- .agentworkforce/trajectories/completed/2026-06/traj_umo7p21qpv43/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_wm6vbs57cmut/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_s26lxygworep/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_890auw9ixqw1/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_fff8ie8vivf0/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_93f25qjubagx/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_kwsld40j6o0q/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_fff8ie8vivf0/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_z1kqrja8kfyc/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_da4wt5z16dll/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_jvzosckbsdmo/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_3a7q65ksviqc/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_xpuanikzwt4t/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_uiyfvt9ydnxz/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_sfvyafvn0v4j/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_983h5d585n4b/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_umo7p21qpv43/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_1cvz2te266aw/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_f98xy8yo6yio/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_3a7q65ksviqc/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_z1kqrja8kfyc/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_gjqemr0lou4l/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_s26lxygworep/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_skw3lpvhwyd4/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_sfvyafvn0v4j.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_hs9nlqqktma6/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_t8p6b889cxx5/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_yteoa3d7jyit/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_lgzs0cvgaldw/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_gcua3hnucbil/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_983h5d585n4b/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_lgzs0cvgaldw.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_add28xzpsw8p/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_fd17bm29mln9.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_08tx11x73e0s.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_5d1p83x8twu5/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_yteoa3d7jyit.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_fd17bm29mln9/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_hs9nlqqktma6.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_skw3lpvhwyd4.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_kwsld40j6o0q/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_to6gxc6ojsjz/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_hs9nlqqktma6/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_5d1p83x8twu5/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_skw3lpvhwyd4/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_fff8ie8vivf0.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_ol0yjdw87422/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_t8p6b889cxx5.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_wzfxaqffmmzm/trajectory.json
- .agentworkforce/trajectories/compacted/compact_65oq3n7um67w_2026-06-25.md
- .agentworkforce/trajectories/completed/2026-06/traj_zhlya22s096b/trajectory.json
- .agentworkforce/trajectories/compacted/compact_65oq3n7um67w_2026-06-25.json
- .agentworkforce/trajectories/completed/2026-06/traj_lgzs0cvgaldw/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_gjqemr0lou4l/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_08tx11x73e0s/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_da4wt5z16dll/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_saln4k9fiil6/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_jvzosckbsdmo/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_saln4k9fiil6/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_fomjzvhz8uv0.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_6u3au0fquyoj/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_fomjzvhz8uv0/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_saln4k9fiil6.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_wzfxaqffmmzm/summary.md
- .gitignore
- .agentworkforce/trajectories/completed/2026-06/traj_da4wt5z16dll.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_08tx11x73e0s/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_983h5d585n4b.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_uiyfvt9ydnxz.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_3nn0bl3wtgfz/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_kwsld40j6o0q.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_fomjzvhz8uv0/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_gjqemr0lou4l.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_uiyfvt9ydnxz/trajectory.json
- .agentworkforce/trajectories/completed/2026-06/traj_6u3au0fquyoj.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_zhlya22s096b.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_ol0yjdw87422/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_wm6vbs57cmut/summary.md
- .agentworkforce/trajectories/completed/2026-06/traj_gcua3hnucbil.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_3a7q65ksviqc.trace.json
- .agentworkforce/trajectories/completed/2026-06/traj_5d1p83x8twu5.trace.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/engine/src/auth/index.ts
- packages/engine/src/engine/action.ts
- README.md
- openapi.yaml
Comment on lines
+122
to
+129
| }ged": [], | ||
| "projectId": "AgentWorkforce/relaycast", | ||
| "tags": [], | ||
| "_trace": { | ||
| "startRef": "0c13f5942d1da9424276bbaaa3fe7fd4d548355e", | ||
| "endRef": "0c13f5942d1da9424276bbaaa3fe7fd4d548355e" | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the stray trailing JSON fragment.
The file closes correctly at Line 121, then appends duplicate garbage starting at Line 122. That makes this trajectory file unparseable.
🧰 Tools
🪛 Biome (2.5.0)
[error] 122-122: String values must be double quoted.
(parse)
[error] 122-122: Missing closing quote
(parse)
[error] 123-123: End of file expected
(parse)
[error] 123-123: End of file expected
(parse)
[error] 123-123: End of file expected
(parse)
[error] 123-123: End of file expected
(parse)
[error] 124-124: End of file expected
(parse)
[error] 124-124: End of file expected
(parse)
[error] 124-124: End of file expected
(parse)
[error] 124-124: End of file expected
(parse)
[error] 125-125: End of file expected
(parse)
[error] 125-125: End of file expected
(parse)
[error] 125-128: End of file expected
(parse)
[error] 129-129: End of file expected
(parse)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agentworkforce/trajectories/active/traj_l5zzs0j95qli/trajectory.json around
lines 122 - 129, Remove the stray trailing JSON fragment at the end of the
trajectory JSON so the file remains valid and parseable. Edit the
trajectory.json content to keep only the single complete JSON object that ends
cleanly before the duplicated garbage, and ensure no extra tokens remain after
the closing brace; the issue is in the final section of the file rather than in
any schema field like _trace or projectId.
Source: Linters/SAST tools
Comment on lines
+46
to
+48
| }9170e948e314402049353b54a8d6744" | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the trailing fragment after the closing brace.
The file already ends at Line 45; the extra text on Lines 46-48 makes the JSON invalid and will break parsing.
🧰 Tools
🪛 Biome (2.5.0)
[error] 46-46: End of file expected
(parse)
[error] 46-46: Invalid exponent part
(parse)
[error] 46-46: String values must be double quoted.
(parse)
[error] 46-46: Missing closing quote
(parse)
[error] 47-48: End of file expected
(parse)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agentworkforce/trajectories/active/traj_nzewjwo33lb9/trajectory.json around
lines 46 - 48, Remove the stray trailing fragment after the final closing brace
so the trajectory JSON ends cleanly at the last valid object boundary. Update
the trajectory.json content to keep only the valid JSON structure and ensure
there are no extra characters after the final closing brace.
Source: Linters/SAST tools
Comment on lines
+13
to
+23
| "summary": "Hardened node delivery contracts after adversarial review: HTTP push retries now redrive from delivery rows, dispatch revalidates URLs and rejects redirects, recipient routing no longer blocks on slow webhooks, manual node capacity uses an atomic active-agent reservation, delivery config redaction is defensive, and manual HTTP ack docs were clarified.", | ||
| "approach": "Standard approach", | ||
| "confidence": 0.9 | ||
| }, | ||
| "commits": [], | ||
| "filesChanged": [], | ||
| "projectId": "AgentWorkforce/relaycast", | ||
| "tags": [], | ||
| "_trace": { | ||
| "startRef": "7b1f3d740f4c6e0239c59e0452f0e7264e4ce02c", | ||
| "endRef": "7b1f3d740f4c6e0239c59e0452f0e7264e4ce02c" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Populate the trajectory provenance fields.
This completed trajectory describes concrete code changes, but commits/filesChanged are empty and _trace.startRef equals _trace.endRef, making the artifact look like a no-op. Please populate the changed files/commit refs or adjust the retrospective to match the recorded metadata.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.agentworkforce/trajectories/completed/2026-06/traj_a30u25uguo04/trajectory.json
around lines 13 - 23, The trajectory metadata is inconsistent with the described
work because commits/filesChanged are empty and _trace.startRef matches
_trace.endRef, making the completed entry look like a no-op. Update the
trajectory JSON so the provenance fields in the summary object are populated
with the actual changed files and commit refs, and ensure _trace reflects the
real before/after refs; if no code changed, revise the retrospective text to
match the recorded metadata. Use the existing top-level fields like commits,
filesChanged, and _trace to locate and correct the provenance data.
Comment on lines
+218
to
+236
| if (conn.observerToken && (conn.observerCheckedAt === undefined || now - conn.observerCheckedAt >= OBSERVER_TOKEN_REVALIDATE_MS)) { | ||
| try { | ||
| const fresh = await this.loadActiveObserverToken(args.workspaceId, conn.observerToken.id); | ||
| if (!fresh) { | ||
| // Token revoked / expired / deleted: drop the socket entirely. | ||
| set.delete(conn); | ||
| try { | ||
| conn.socket.close(1008, 'observer token revoked'); | ||
| } catch { | ||
| // already closed | ||
| } | ||
| continue; | ||
| } | ||
| conn.observerToken = fresh; | ||
| conn.observerCheckedAt = now; | ||
| } catch { | ||
| // Transient DB error: keep using the cached snapshot and retry on the | ||
| // next event rather than dropping a legitimate observer. | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Back off observer-token revalidation after DB errors.
When loadActiveObserverToken() throws, observerCheckedAt remains stale, so every subsequent workspace event retries the DB lookup for each observer connection. During a DB blip, this can turn fanout into a serial retry loop. Stamp a retry/backoff time even when keeping the cached snapshot.
Suggested fix
} catch {
- // Transient DB error: keep using the cached snapshot and retry on the
- // next event rather than dropping a legitimate observer.
+ conn.observerCheckedAt = now;
+ // Transient DB error: keep using the cached snapshot and retry after
+ // the normal revalidation window rather than hammering the DB.
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (conn.observerToken && (conn.observerCheckedAt === undefined || now - conn.observerCheckedAt >= OBSERVER_TOKEN_REVALIDATE_MS)) { | |
| try { | |
| const fresh = await this.loadActiveObserverToken(args.workspaceId, conn.observerToken.id); | |
| if (!fresh) { | |
| // Token revoked / expired / deleted: drop the socket entirely. | |
| set.delete(conn); | |
| try { | |
| conn.socket.close(1008, 'observer token revoked'); | |
| } catch { | |
| // already closed | |
| } | |
| continue; | |
| } | |
| conn.observerToken = fresh; | |
| conn.observerCheckedAt = now; | |
| } catch { | |
| // Transient DB error: keep using the cached snapshot and retry on the | |
| // next event rather than dropping a legitimate observer. | |
| } | |
| if (conn.observerToken && (conn.observerCheckedAt === undefined || now - conn.observerCheckedAt >= OBSERVER_TOKEN_REVALIDATE_MS)) { | |
| try { | |
| const fresh = await this.loadActiveObserverToken(args.workspaceId, conn.observerToken.id); | |
| if (!fresh) { | |
| // Token revoked / expired / deleted: drop the socket entirely. | |
| set.delete(conn); | |
| try { | |
| conn.socket.close(1008, 'observer token revoked'); | |
| } catch { | |
| // already closed | |
| } | |
| continue; | |
| } | |
| conn.observerToken = fresh; | |
| conn.observerCheckedAt = now; | |
| } catch { | |
| conn.observerCheckedAt = now; | |
| // Transient DB error: keep using the cached snapshot and retry after | |
| // the normal revalidation window rather than hammering the DB. | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/engine/src/adapters/node/realtime.ts` around lines 218 - 236, The
observer-token revalidation logic in realtime.ts keeps retrying the database on
every event after a loadActiveObserverToken() exception because
observerCheckedAt is never updated in the catch path. Update the observer
connection handling in the revalidation block so that, when
loadActiveObserverToken fails, conn.observerCheckedAt is still advanced to now
(or a backoff timestamp) while preserving the cached observerToken snapshot,
preventing repeated immediate retries for each workspace event.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/v1/wsobserver-only while agent SDKs mint direct node tokens and connect to/v1/node/wsws.node.v1, withkindas transport androleas ownership/capacity semanticsaction.invokeframes; deliver action completion/denial, reactions, and receipts as nodedeliverframesWhy
Direct agent sockets and broker node sockets were separate delivery paths even though they should share the same node delivery contract. That split caused double-delivery risks, missing node context, and action lifecycle gaps once nodes became first-class. This PR makes every agent have exactly one node route, makes the workspace stream an observer capability, and keeps SDK ergonomics by normalizing node frames into the existing app callbacks.
Validation
npx turbo build && npx turbo test && npx turbo lintcargo test --manifest-path packages/sdk-rust/Cargo.tomlswift test --package-path packages/sdk-swiftnpm --workspace @relaycast/engine test -- invocationCompletion.test.ts sdk-contract.test.ts node.test.tsnpm --workspace @relaycast/types test -- fleet-wire-fixtures.test.ts sdk-openapi-sync.test.tspython3 -m pytest packages/sdk-python/tests/test_ws.pycould not run locally because this environment does not havepytestinstalled (No module named pytest)