Facet broadcasts after the frame completes cost two root RPCs per message (getAgentByName on every broadcast) #2252

Activity

1. agent-think

   added

   bugSomething isn't workingSomething isn't working

   on Sep 11, 2026
2. akim136 commented on Sep 14, 2026

   

   akim136

   on Sep 14, 2026

   More actions

   I went to implement suggestion (1) — cache the root stub per facet instance in rootAlarmOwner() — and stopped, because I think the __unsafe_ensureInitialized() it removes is load-bearing rather than redundant.

   The per-broadcast count matches the code as you describe it: rootAlarmOwner() resolves through getAgentByName() on every call, so N broadcasts outside a live frame cost 2N root requests. A per-instance cache would bring the healthy path to N+1 per facet instance (N+F across F participating facets). Your tail numbers can't tell us F, so I won't guess at the projected total.

   The problem is what the stub guarantees. getAgentByName() documents itself as returning a stub after its lifecycle startup has completed, and that startup is the only thing supplying it on this path — _cf_broadcastToSubAgent delegates straight into broadcastToPath() without touching the lifecycle, unlike the other RPC entry points that do await the initializer.

   A probe against unmodified main (a7b2913), using the Workers test runtime:

   const root = await getAgentByName(env.TestSubAgentParent, name);
   await evictDurableObject(root);
   // spy on the reconstructed instance's onStart
   await root._cf_broadcastToSubAgent([{ className: "TestSubAgentParent", name }], "after-eviction");
   expect(start).toHaveBeenCalledTimes(0);   // cached stub: broadcast succeeds, no startup
   await getAgentByName(env.TestSubAgentParent, name);
   expect(start).toHaveBeenCalledTimes(1);   // this is what runs it

   So a cached stub keeps routing correctly into a root that was reconstructed after eviction and never ran onStart(). Error-driven invalidation wouldn't catch it, because nothing fails — the broadcast succeeds against an unstarted root.

   Two other things a cache would need to settle. Failed stubs have to be discarded without a late failure from an old reference clobbering a newer good entry, and rootAlarmOwner() has more consumers than broadcast — lifecycle routing, stale-prefix cleanup, nested facet deletion, queued send/state/close, connection hydration, plus keepAlive(), fiber registration/cleanup and facet destruction via _rootAlarmOwner(). Keep-alive disposers and fiber cleanup hold the returned surface across async work, so clearing one field wouldn't repair every retained reference.

   Which makes this a design question before it's a patch: should the root RPC endpoints guarantee lifecycle startup themselves? If they did, caching the stub becomes straightforward and this collapses to 2N → N+1. If they shouldn't, then the initializer isn't redundant and the win has to come from somewhere else — coalescing broadcasts behind the same tail, maybe, which is your suggestion (2) and a bigger change.

   Happy to implement whichever direction you prefer, with regression coverage for hibernation, failed-stub recovery, concurrent first callers, and sibling-instance isolation. @anuarhdz — I'd take you up on testing a patch once there's a candidate worth running.
3. eddielin0926 commented on Sep 17, 2026

   

   eddielin0926

   on Sep 17, 2026

   More actions

   We ran into this from a slightly different angle, and I think it is worth mentioning because this path is difficult for us to avoid.

   We run a long, deterministic job from an HTTP handler. It takes about 30 minutes, uses a single model loop with tool calls, and has no conversation. The user only sees the current phase, elapsed time, and a download link; model output is never shown. Roughly eight status updates per run would be enough.

   runAgentTool() seemed like the right fit. The docs explicitly list "deterministic workflows, scheduled work, HTTP handlers, or fan-out code," which matches our use case.

   The problem is that the child must extend Think or AIChatAgent. _asAgentToolChildAdapter checks for the four adapter methods and throws if they are missing. We already extend Think for the tool loop, so runAgentTool() would lock that in rather than let us drop it.

   Think, however, broadcasts every generated chunk to clients through _broadcastChat → this.broadcast(). This is still always enabled in think@0.18.0, and the facet broadcast path it lands on is unchanged in agents@0.22.0 and 0.23.0. When running as a facet, those chunks also pass through the root.

   In our case there is no consumer for that stream and no supported way to disable it. The root also resolves the connection list, so both RPCs happen before discovering that there are no matching connections. Our users commonly start a run and close the tab, which means most chunks in a 30-minute run become round trips that ultimately target zero connections.

   The cleaner architecture for us would be to stop using a facet entirely: use our own Durable Object with a plain Agent. In that case broadcast() can use the local connection.send path, so there is no root RPC to count. The downside is that we would lose runAgentTool(), which we would prefer to keep.

   If this is sufficiently different from the RPC-count issue here, I'm happy to split it into a separate issue.
4. akim136 commented on Sep 17, 2026

   

   akim136

   on Sep 17, 2026

   More actions

   I don't think runAgentTool() actually requires a Think or AIChatAgent subclass. _asAgentToolChildAdapter just checks for four methods (startAgentToolRun, cancelAgentToolRun, inspectAgentToolRun, getAgentToolChunks), and ChatCapableAgentClass turns out to be a plain alias for SubAgentClass<Agent>. Both the error message and the type name read stricter than the check is.

   That wouldn't get you off the facet path though. runAgentTool goes through subAgent() into dynamicAgents.get(), so the child is a facet whatever it extends, and you're right that your own DO is the only real way out. But a plain Agent child does drop Think's _broadcastChat, which is the thing firing per chunk. You'd be making your 8 status broadcasts instead of thousands. Fair warning that Think's four methods aren't small if you go reimplement them. Chunk storage, abort handling, run-row finalization across restarts.

   Additionally, the root filters connections after the call lands (broadcastToPath, dynamic-agents.ts:905-911). Tab closed means both round trips go to nobody. So maybe let a facet skip the broadcast when it has no matching connections, since it already tracks them in #virtualConnections. Catch is the hydration path warns a facet can't see root-owned hibernated sockets, so a naive zero check could drop messages to those. The operative question is still what can a facet assume locally without asking the root?

   The Think-broadcasts-everything part I'd split off, different package. Open to suggestions though.
5. poggersdave commented on Sep 18, 2026

   

   poggersdave

   on Sep 18, 2026

   More actions

   Adding a second data point from agents@0.23.0, and a complementary fix that stops the calls rather than making them cheaper.

   Setup. A Slack-driven parent: root Agent per user, a Think facet per thread, and a Think Agent Tool child facet for a long workbook-review task. Nothing ever holds a WebSocket on the facet, so the root has zero connections to fan out to, yet every streamed chunk still takes the root path in routeBroadcast() once the inbound frame has completed.

   Measured on one child run (Honeycomb, sample rate 1, spans in the child's window):

   Build	Spans	_cf_broadcastToSubAgent calls
   Stock 0.23.0	85,302	thousands (one per chunk)
   + terminal-only Agent Tool delivery (child chunks not re-forwarded)	18,560	17
   + skip broadcastToParent when the facet has no virtual connections	4,654 – 5,173	0

   The remaining root traffic after the skip is _cf_subAgentConnectionMetas, 15–21 per run, i.e. once per facet lifetime.

   Approach. In DynamicAgentsInternal:

   • settleVirtualConnections() memoises hydrateConnectionsFromRoot() per facet lifetime as a promise (reset to unsettled on failure so the next broadcast retries; held under ctx.waitUntil).
   • broadcastToParent() returns early when the mirror is settled and #virtualConnections.size === 0; otherwise it routes as today.
   • restoreFacetContext() uses the same settle, so there is one writer of the mirror.
   • A forwarded connect still repopulates the mirror through storeVirtualConnection, so fan-out resumes from the next chunk once a client actually attaches.

   Known gap: chunks emitted between a client's connect at the root and the facet's storeVirtualConnection are not fanned out by this path; Think's persisted stream still serves reattachment, so the client catches up on replay.

   This is orthogonal to suggestion (1) (caching the root stub): that halves the cost of a broadcast, this avoids the broadcast when nobody is listening. Happy to open a PR with the patch and the workers-pool tests (settle once, resume with a client, retry after a failed root read) if that shape is acceptable.
6. threepointone

   closed this as completedin #2399on Sep 28, 2026
7. threepointone

   added a commit that references this issue on Sep 28, 2026

   fix(agents): one root call per sub-agent broadcast, none without reci…

   ...

   cbb859b