Skip to content

feat(runtime): store targeting metadata on ring entries for filtered replay#32790

Merged
dvargasfuertes merged 1 commit into
mainfrom
devin/1780322314-targeted-event-ring-buffering
Jun 1, 2026
Merged

feat(runtime): store targeting metadata on ring entries for filtered replay#32790
dvargasfuertes merged 1 commit into
mainfrom
devin/1780322314-targeted-event-ring-buffering

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Jun 1, 2026

Targeted events (those published with targetCapability, targetClientId, targetInterfaceId, or excludeClientId) were previously excluded from the per-conversation ring buffer via replayable: false. This prevented event leaks on replay but created permanent seq gaps that triggered false-positive reconcile fetches on the client. This PR stores targeting metadata on ring entries and re-applies the same delivery filter at replay time, so reconnecting clients only receive events they would have seen live — no leaks, no false gaps.


Prompt / plan

B7 followup item "Targeted-event ring buffering" from #32676.

Test plan

  • 26 unit tests (9 new) covering: targeted events buffered, monotonic seq across targeting, capability filtering, client targeting, interface targeting, excludeClientId suppression, mixed-targeting per-entry filtering, backwards-compatible unfiltered replay.
  • bunx tsc --noEmit clean, bun run lint clean.

…replay

Targeted events (those with targetCapability, targetClientId,
targetInterfaceId, or excludeClientId modifiers) are now buffered in
the per-conversation ring with their targeting metadata attached.

getReplayWindow accepts an optional ReplaySubscriber descriptor and
re-applies the same delivery filter that the live publish() path uses,
so reconnecting clients only receive events they would have seen live.

This eliminates false-positive seq gaps on reconnect -- previously,
targeted events were stamped with a seq but excluded from the ring,
creating permanent holes that triggered unnecessary reconcile fetches
on the client side.

Co-Authored-By: vargas@vellum.ai <vargas@vellum.ai>
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 921e75a1be

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +247 to +262
if (t.targetClientId != null) {
// Client targeting: bypass conversation filter, deliver only to the
// named client.
if (
subscriber.type !== "client" ||
subscriber.clientId !== t.targetClientId
) {
return false;
}
if (
t.targetCapability != null &&
!subscriber.capabilities?.includes(t.targetCapability)
) {
return false;
}
return true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve target-client replay across conversation scopes

When targetClientId is set, live fanout deliberately bypasses the subscriber's conversation filter because the target client may be connected to a different conversation than the event's conversationId (as documented in AssistantEventHub.publish). The replay path still stores the entry only in the source conversation's ring and GET /events only drains the ring for the subscriber's requested conversation, so a reconnecting targeted macOS/extension client will not receive the buffered host request unless it happens to subscribe to that source conversation; those requests can still be missed and time out during reconnects.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid observation. targetClientId live fanout bypasses the conversation filter (line 325-328 in assistant-event-hub.ts), but the ring is per-conversation and getReplayWindow is only called for the subscriber's requested conversation. So a reconnecting client subscribed to conversation B won't drain conversation A's ring.

This is a pre-existing limitation — before this PR, targetClientId events weren't buffered at all (replayable: false), so they were always lost on reconnect. This PR makes the same-conversation case work correctly; the cross-conversation case remains a gap that would require a fundamentally different approach (per-client ring or a cross-conversation replay index).

In practice, the main cross-conversation targetClientId use case is host-proxy requests (e.g., host_bash_request to a macOS client). Those have their own timeout + retry mechanism via pending-interactions, so a missed replay causes a retry rather than silent loss.

@dvargasfuertes dvargasfuertes merged commit 5a33fc3 into main Jun 1, 2026
13 checks passed
@dvargasfuertes dvargasfuertes deleted the devin/1780322314-targeted-event-ring-buffering branch June 1, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant