(MOT-4401) feat(harness,console): push trigger-binding change events, drop the console trigger poll - #775
Conversation
… drop the console trigger poll Every open console chat tab polled harness::triggers::list on a 5s interval (each call a full-scope harness::state::list on the state worker). Replace the poll with a harness::triggers-changed doorbell — the same pattern #452 used to kill the status poll: - harness: 5th event trigger type; {session_id, timestamp} emitted from the four BindingStore CAS primitives (reserve, attach_trigger_id, claim_fire, delete_if_unchanged), so every mutation path incl. the out-of-turn ones (expiry sweep, GC, other-tab unregister, deferred release) rings from one layer. - console: onTriggersChanged backend hook (shape of onQueuedMessage); ChatView swaps the interval for initial fetch + doorbell subscription + visibilitychange catch-up. Always-on for the selected conversation, unlike the streaming-gated queued subscription. Version skew is graceful both ways: an old harness parks the unknown trigger-type registration (console degrades to mount/visibility/post- unregister refetches); an old console keeps polling a new harness.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 57 skipped (no docs/).
Four for four. Nicely done. |
|
Warning Review limit reached
Next review available in: 8 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe harness now emits session-scoped trigger-change events. The web backend subscribes to these events. ChangesTrigger refresh flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BindingStore
participant TurnEvents
participant realBackend
participant ChatView
BindingStore->>TurnEvents: emit triggers-changed for session
TurnEvents->>realBackend: deliver session event
realBackend->>ChatView: invoke onTriggersChanged callback
ChatView->>ChatView: refetch trigger list
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@console/web/src/components/chat/ChatView.tsx`:
- Around line 342-356: Update refreshTriggers in the trigger-list subscription
flow to prevent overlapping listTriggers responses from applying stale data.
Track a monotonic request revision (or serialize refreshes) and only update
seenTriggersRef and sessionTriggers when the completed request is still the
newest; preserve the existing doorbell and visibility-change refresh behavior.
In `@console/web/src/lib/backend/types.ts`:
- Around line 317-323: Expose an explicit capability or registration-result
contract for onTriggersChanged in console/web/src/lib/backend/types.ts:317-323,
propagate unsupported or failed subscription setup from the real backend
implementation at console/web/src/lib/backend/real.ts:525-541, and update
ChatView at console/web/src/components/chat/ChatView.tsx:337-357 to retain
bounded refresh polling until subscription support is confirmed. Add a
mixed-version integration test covering a current console connected to a harness
without harness::triggers-changed registration.
🪄 Autofix
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: 23635333-50b5-4baf-8442-be457c31203a
📒 Files selected for processing (11)
console/web/src/components/chat/ChatView.tsxconsole/web/src/lib/backend/real.tsconsole/web/src/lib/backend/triggers.tsconsole/web/src/lib/backend/turn-events-live.test.tsconsole/web/src/lib/backend/turn-events-live.tsconsole/web/src/lib/backend/types.tsconsole/web/src/types/iii-agent-event.tsharness/src/bindings/store.rsharness/src/deps.rsharness/src/events.rsharness/src/subscriptions/fired.rs
…on gaps Review fixes on #775: - serialRefresh: doorbells arrive at-least-once and burst on rapid fires; refetches now serialize behind one in-flight list read with a single trailing rerun, so an older snapshot can never resolve after and overwrite a newer one. reset() discards in-flight responses on conversation switch/unmount (also fixes the pre-existing late-write into a switched conversation). - ChatView subscribes to the doorbell BEFORE the initial snapshot, so a mutation in the setup gap rings instead of being missed. - realOnTriggersChanged reseeds on every 'connected' transition: the SDK replays trigger registrations on reconnect but outage-window doorbells are gone (same repair TracesV2 uses). The reseed is ordered after the SDK's synchronous registration replay, so it also covers the first connect.
Why
Live-engine traces showed every open console chat tab polling
harness::triggers::liston a 5s interval (setIntervalin ChatView, added in #452, pointed at the harness in #687) — 3,704 stored spans on the dev stack, almost all returning{"subscriptions":[]}. Each poll costs two invocations: the handler does a full-scopeharness::state::liston the state worker and filters client-side.Fires/expiry were already push-visible (durable
trigger_firedtranscript entries), but binding-set mutations had no event — and several happen out-of-turn (expiry sweep, GC, unregister from another tab,harness::function::trigger, deferred approval release), so the poll couldn't be replaced by inference from existing events.What
Same pattern #452 used to kill the status poll with
harness::message-queued:harness
harness::triggers-changedevent trigger type (5th), registered alongside the existing four.{session_id, timestamp}— consumers refetchharness::triggers::list.BindingStoreCAS primitives (reserve,attach_trigger_id,claim_fire,delete_if_unchanged), so every mutation path — registration, agent/console unregister, fires, once/max_fires retirement, expiry sweep, GC, teardown, rollbacks — rings from one layer.deleteloops throughdelete_if_unchanged, so it's covered.console/web
onTriggersChangedbackend hook (same shape asonQueuedMessage),startTriggersChangedSubscriptionreusing the genericbind()in turn-events-live.visibilitychangecatch-up (missed doorbells while hidden/throttled). Always-on for the selected conversation, unlike the streaming-gated queued subscription, because bindings mutate outside turns.seenTriggersRef,mergeFiredTriggers) unchanged; post-unregister refetches kept.Version skew
Verification
cargo fmt --check,clippy --all-features -D warnings,cargo test --all-features(harness) ✓pnpm test(1149 incl. 3 new doorbell subscription tests),pnpm typecheck, biome scoped to touched files ✓ (ChatView has two pre-existing biome errors on main, untouched)harness::triggers-changedsubscription for a test session, then registered a binding through the harness interceptor and unregistered it viaharness::triggers::unregister. Traces show exactly three doorbell deliveries (reserve, attach, delete) with the session filter applied, and no periodictriggers::listtraffic from the subscriber. Test session, subscription, and engine trigger cleaned up afterwards.Summary by CodeRabbit
New Features
Bug Fixes
Tests