Conversation
18 tasks
Collaborator
Author
This was referenced Mar 27, 2026
|
|
This was referenced Mar 27, 2026
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds W3C baggage parsing to extract Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
18 tasks
Contributor
Confidence Score: 5/5Safe to merge; all remaining findings are P2 style/cleanup suggestions that do not block correctness. No P0 or P1 defects found. The dead RealtimeToolOutput type and missing turn-hook tests are minor quality concerns that do not affect runtime behavior. Prior race-condition concerns were correctly addressed. session.go (dead RealtimeToolOutput type) and session_test.go (missing turn-hook coverage) warrant a follow-up, but neither blocks merge. Important Files Changed
Reviews (15): Last reviewed commit: "feat: add realtime session state and tra..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
transports/bifrost-http/websocket/session.go (1)
148-156: Considerstrings.Builderfor high-frequency text accumulation.Repeated string concatenation with
+=creates new string allocations on each append. If assistant turns produce many delta events, this could cause allocation pressure.♻️ Optional optimization using strings.Builder
// In Session struct: - realtimeOutputText string + realtimeOutputText strings.Builder // AppendRealtimeOutputText: func (s *Session) AppendRealtimeOutputText(text string) { if text == "" { return } s.mu.Lock() defer s.mu.Unlock() - s.realtimeOutputText += text + s.realtimeOutputText.WriteString(text) } // ConsumeRealtimeOutputText: func (s *Session) ConsumeRealtimeOutputText() string { s.mu.Lock() defer s.mu.Unlock() - text := s.realtimeOutputText - s.realtimeOutputText = "" + text := s.realtimeOutputText.String() + s.realtimeOutputText.Reset() return text }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session.go` around lines 148 - 156, AppendRealtimeOutputText currently concatenates into s.realtimeOutputText with += causing repeated allocations; switch to using a strings.Builder stored on Session (e.g., replace or add field realtimeOutputBuilder *strings.Builder or strings.Builder) and update AppendRealtimeOutputText to write to that builder under s.mu.Lock()/defer s.mu.Unlock() using builder.WriteString(text). Also add or update any accessor (e.g., RealtimeOutputText, String, or finalize method) to return builder.String() safely under the same mutex so callers get the accumulated text.transports/bifrost-http/websocket/session_test.go (1)
9-92: Consider adding test coverage for turn hook lifecycle.The new
TryBeginRealtimeTurnHooks,AbortRealtimeTurnHooks,SetRealtimeTurnHooks,ConsumeRealtimeTurnHooks, andClearRealtimeTurnHooksmethods have specific concurrency semantics (single-slot reservation viarealtimeTurnBusy) that would benefit from dedicated tests, especially:
TryBeginRealtimeTurnHooksreturns false when already busy or hooks are setAbortRealtimeTurnHooksreleases the busy slot without installing hooksSetRealtimeTurnHookscalls cleanup on existing hooks before replacingSnapshot()returns a copy without mutating internal state🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session_test.go` around lines 9 - 92, Add unit tests to cover the realtime turn hook lifecycle: create tests that assert TryBeginRealtimeTurnHooks returns false when realtimeTurnBusy is already true or when hooks are already set, that AbortRealtimeTurnHooks clears realtimeTurnBusy without installing hooks, that SetRealtimeTurnHooks calls Cleanup on any existing hooks before replacing them, and that Snapshot() returns a deep copy (mutating the returned snapshot does not change session internals). Target the Session methods TryBeginRealtimeTurnHooks, AbortRealtimeTurnHooks, SetRealtimeTurnHooks, ConsumeRealtimeTurnHooks, ClearRealtimeTurnHooks, and Snapshot in new test cases mirroring the style of existing tests (use newTestConn(), NewSession/NewSessionManager) and assert expected boolean returns, cleanup side-effects, busy-slot semantics, and immutability of snapshots.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@transports/bifrost-http/lib/ctx_test.go`:
- Around line 253-266: TestConvertToBifrostContext_BaggageSessionIDSetsGrouping
calls ConvertToBifrostContext with only three arguments but the function
signature (ConvertToBifrostContext) requires a fourth; update the test to pass
the missing fourth argument (use schemas.WhiteList{} as in the other tests) when
calling ConvertToBifrostContext in
TestConvertToBifrostContext_BaggageSessionIDSetsGrouping so the call matches the
function signature and compiles.
- Around line 268-281: The test
TestConvertToBifrostContext_EmptyBaggageSessionIDIgnored calls
ConvertToBifrostContext with three arguments but the function signature requires
a fourth mcpHeaderCombinedAllowlist parameter; update the call to include the
missing fourth argument (e.g., pass nil or the appropriate allowlist variable)
so ConvertToBifrostContext(ctx, false, nil, mcpHeaderCombinedAllowlist) matches
the function signature and the test compiles.
In `@transports/bifrost-http/lib/ctx.go`:
- Around line 201-207: The code currently maps the baggage "session-id" to both
BifrostContextKeyParentRequestID and BifrostContextKeyRealtimeSessionID (via
ParseSessionIDFromBaggage and bifrostCtx.SetValue), causing semantic aliasing;
change this so "session-id" only sets BifrostContextKeyRealtimeSessionID and do
NOT populate BifrostContextKeyParentRequestID from it—instead read
ParentRequestID from an explicit parent-request-id baggage member or the
existing x-bf-parent-request-id header and set BifrostContextKeyParentRequestID
only from that dedicated source.
---
Nitpick comments:
In `@transports/bifrost-http/websocket/session_test.go`:
- Around line 9-92: Add unit tests to cover the realtime turn hook lifecycle:
create tests that assert TryBeginRealtimeTurnHooks returns false when
realtimeTurnBusy is already true or when hooks are already set, that
AbortRealtimeTurnHooks clears realtimeTurnBusy without installing hooks, that
SetRealtimeTurnHooks calls Cleanup on any existing hooks before replacing them,
and that Snapshot() returns a deep copy (mutating the returned snapshot does not
change session internals). Target the Session methods TryBeginRealtimeTurnHooks,
AbortRealtimeTurnHooks, SetRealtimeTurnHooks, ConsumeRealtimeTurnHooks,
ClearRealtimeTurnHooks, and Snapshot in new test cases mirroring the style of
existing tests (use newTestConn(), NewSession/NewSessionManager) and assert
expected boolean returns, cleanup side-effects, busy-slot semantics, and
immutability of snapshots.
In `@transports/bifrost-http/websocket/session.go`:
- Around line 148-156: AppendRealtimeOutputText currently concatenates into
s.realtimeOutputText with += causing repeated allocations; switch to using a
strings.Builder stored on Session (e.g., replace or add field
realtimeOutputBuilder *strings.Builder or strings.Builder) and update
AppendRealtimeOutputText to write to that builder under s.mu.Lock()/defer
s.mu.Unlock() using builder.WriteString(text). Also add or update any accessor
(e.g., RealtimeOutputText, String, or finalize method) to return
builder.String() safely under the same mutex so callers get the accumulated
text.
🪄 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
Run ID: 06bd666e-08c7-45a6-a562-747ca8bc000c
📒 Files selected for processing (4)
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/bifrost-http/websocket/session.gotransports/bifrost-http/websocket/session_test.go
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
81edf1b to
f9bb3b3
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
e97b881 to
f8b747f
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
transports/bifrost-http/websocket/session.go (1)
148-165: Considerstrings.Builderfor output accumulation if this becomes a hot path.String concatenation via
+=creates a new string on each append. If provider turns involve many small deltas, consider usingstrings.Builderfor O(n) instead of O(n²) complexity. However, if deltas are infrequent or turns are short, current implementation is acceptable.♻️ Optional optimization with strings.Builder
type Session struct { // ... - realtimeOutputText string + realtimeOutputText strings.Builder // ... } func (s *Session) AppendRealtimeOutputText(text string) { if text == "" { return } s.mu.Lock() defer s.mu.Unlock() - s.realtimeOutputText += text + s.realtimeOutputText.WriteString(text) } func (s *Session) ConsumeRealtimeOutputText() string { s.mu.Lock() defer s.mu.Unlock() - text := s.realtimeOutputText - s.realtimeOutputText = "" + text := s.realtimeOutputText.String() + s.realtimeOutputText.Reset() return text }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session.go` around lines 148 - 165, The current AppendRealtimeOutputText uses string += on s.realtimeOutputText which can be quadratic for many small appends; change Session to use a strings.Builder (e.g., add a field like realtimeOutputBuilder) protected by the same s.mu, update AppendRealtimeOutputText to WriteString to the builder, and update ConsumeRealtimeOutputText to call builder.String(), reset the builder (e.g., replace with a new strings.Builder) and return the string; ensure Session.ConsumeRealtimeOutputText and AppendRealtimeOutputText still lock s.mu and handle empty/no-op correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@transports/bifrost-http/websocket/session.go`:
- Around line 148-165: The current AppendRealtimeOutputText uses string += on
s.realtimeOutputText which can be quadratic for many small appends; change
Session to use a strings.Builder (e.g., add a field like realtimeOutputBuilder)
protected by the same s.mu, update AppendRealtimeOutputText to WriteString to
the builder, and update ConsumeRealtimeOutputText to call builder.String(),
reset the builder (e.g., replace with a new strings.Builder) and return the
string; ensure Session.ConsumeRealtimeOutputText and AppendRealtimeOutputText
still lock s.mu and handle empty/no-op correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fc52767-ed8d-405b-89c1-b98177aa7907
📒 Files selected for processing (4)
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/bifrost-http/websocket/session.gotransports/bifrost-http/websocket/session_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- transports/bifrost-http/lib/ctx_test.go
- transports/bifrost-http/websocket/session_test.go
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
f9bb3b3 to
40f5251
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
f8b747f to
7e6f0c6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
transports/bifrost-http/websocket/session.go (1)
248-257: Consider potential for lock contention if Cleanup() blocks.
SetRealtimeTurnHookscallsCleanup()while holdingmu. If the cleanup function performs I/O or takes significant time, it could cause lock contention. If cleanup callbacks are guaranteed to be fast and non-blocking, this is fine. Otherwise, consider releasing the lock before calling cleanup (with appropriate state guards).This is a design trade-off. The current approach ensures atomic state transitions but could block other session operations during cleanup. If profiling shows this as a bottleneck, the cleanup could be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session.go` around lines 248 - 257, SetRealtimeTurnHooks currently calls s.realtimeTurnHooks.Cleanup() while holding s.mu which can cause lock contention if Cleanup blocks; change it to capture the old hooks under lock (e.g., old := s.realtimeTurnHooks), perform the minimal state updates while holding s.mu (set s.realtimeTurnBusy, assign s.realtimeTurnHooks = state), then release the lock and call old.Cleanup() only if old != nil && old.Cleanup != nil; alternatively, if cleanup is safe to run concurrently, invoke old.Cleanup() in a background goroutine after releasing the lock—ensure you keep the same atomic state transition semantics for SetRealtimeTurnHooks and guard against nil old hooks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@transports/bifrost-http/websocket/session.go`:
- Around line 284-292: The ConsumeRealtimeTurnHooks method currently clears
s.realtimeTurnHooks and returns the state without calling Cleanup(), creating an
inconsistency with ClearRealtimeTurnHooks/SetRealtimeTurnHooks; modify
ConsumeRealtimeTurnHooks so that after capturing state := s.realtimeTurnHooks
(and while holding s.mu) you call state.Cleanup() before setting
s.realtimeTurnHooks = nil and s.realtimeTurnBusy = false, then return the
(cleaned) state; this ensures Cleanup() is invoked under the lock just like in
ClearRealtimeTurnHooks.
---
Nitpick comments:
In `@transports/bifrost-http/websocket/session.go`:
- Around line 248-257: SetRealtimeTurnHooks currently calls
s.realtimeTurnHooks.Cleanup() while holding s.mu which can cause lock contention
if Cleanup blocks; change it to capture the old hooks under lock (e.g., old :=
s.realtimeTurnHooks), perform the minimal state updates while holding s.mu (set
s.realtimeTurnBusy, assign s.realtimeTurnHooks = state), then release the lock
and call old.Cleanup() only if old != nil && old.Cleanup != nil; alternatively,
if cleanup is safe to run concurrently, invoke old.Cleanup() in a background
goroutine after releasing the lock—ensure you keep the same atomic state
transition semantics for SetRealtimeTurnHooks and guard against nil old hooks.
🪄 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
Run ID: b6935a40-df76-4319-8cb7-f8685aa4fdd5
📒 Files selected for processing (4)
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/bifrost-http/websocket/session.gotransports/bifrost-http/websocket/session_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- transports/bifrost-http/websocket/session_test.go
- transports/bifrost-http/lib/ctx_test.go
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
7e6f0c6 to
8f0f53b
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
40f5251 to
44d1515
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
transports/bifrost-http/websocket/session.go (1)
148-156: Consider usingstrings.Builderfor output text accumulation.String concatenation with
+=allocates a new string on each append. For high-frequency streaming scenarios (many small text deltas), astrings.Builderwould reduce allocations and improve performance.♻️ Optional optimization using strings.Builder
In the
Sessionstruct, replace:- realtimeOutputText string + realtimeOutputText strings.BuilderThen update the methods:
func (s *Session) AppendRealtimeOutputText(text string) { if text == "" { return } s.mu.Lock() defer s.mu.Unlock() - s.realtimeOutputText += text + s.realtimeOutputText.WriteString(text) } func (s *Session) ConsumeRealtimeOutputText() string { s.mu.Lock() defer s.mu.Unlock() - text := s.realtimeOutputText - s.realtimeOutputText = "" + text := s.realtimeOutputText.String() + s.realtimeOutputText.Reset() return text }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session.go` around lines 148 - 156, The AppendRealtimeOutputText method currently appends to s.realtimeOutputText with += causing repeated allocations; switch to using a strings.Builder stored on the Session (e.g., add a realtimeOutputBuilder field of type strings.Builder alongside mu and realtimeOutputText or replace realtimeOutputText) and have AppendRealtimeOutputText write to that builder under the existing s.mu lock (s.realtimeOutputBuilder.WriteString(text)); update any methods that read realtimeOutputText (e.g., getters or Reset/Flush functions) to call s.realtimeOutputBuilder.String() (or s.realtimeOutputBuilder.Reset() when clearing) while holding the mutex to preserve thread-safety.transports/bifrost-http/websocket/session_test.go (1)
94-134: Consider adding test coverage for turn hook lifecycle methods and Peek accessors.*
TestSessionRealtimeStatecovers the basic append/consume flows but omits:
PeekRealtimeInputText,PeekRealtimeInputRaw,PeekRealtimeToolOutputs— ensuring peek doesn't clear state- Turn hook lifecycle:
TryBeginRealtimeTurnHooks,AbortRealtimeTurnHooks,SetRealtimeTurnHooks,ConsumeRealtimeTurnHooks,ClearRealtimeTurnHooksSessionManager.Snapshot()Session.Close()cleanup behavior (turn hooks cleanup)These methods have specific semantics (e.g.,
TryBeginRealtimeTurnHooksreservation, cleanup ownership transfer inConsumeRealtimeTurnHooks) that would benefit from explicit test coverage.Would you like me to generate test cases for the turn hook lifecycle methods?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session_test.go` around lines 94 - 134, Add explicit unit tests to cover the missing peek accessors and turn-hook lifecycle: create a test that exercises PeekRealtimeInputText, PeekRealtimeInputRaw, and PeekRealtimeToolOutputs to assert they return current values without clearing state (consume should still return the same values afterwards); add tests around TryBeginRealtimeTurnHooks, SetRealtimeTurnHooks, AbortRealtimeTurnHooks, ConsumeRealtimeTurnHooks, and ClearRealtimeTurnHooks to validate reservation behavior, abort releases reservation, Set assigns hooks, Consume transfers ownership and clears internal state, and Clear removes hooks; add assertions that SessionManager.Snapshot() includes current realtime state and turn-hook metadata; and add a test that Session.Close() performs cleanup (e.g., clears any reserved/active turn hooks) so no hooks remain after close. Ensure each test references the methods TryBeginRealtimeTurnHooks, SetRealtimeTurnHooks, AbortRealtimeTurnHooks, ConsumeRealtimeTurnHooks, ClearRealtimeTurnHooks, PeekRealtimeInputText, PeekRealtimeInputRaw, PeekRealtimeToolOutputs, SessionManager.Snapshot, and Session.Close to locate code under test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@transports/bifrost-http/lib/ctx.go`:
- Around line 49-55: The parsed session-id value (variable name value) must be
guarded to max length before returning to avoid DB errors when stored as
ParentRequestID; in the loop where you detect key == "session-id" (in ctx.go),
check the length and truncate to 255 characters (use rune-safe truncation) if it
exceeds that limit, then return the truncated value; do not change calling
sites, only enforce the max-length guard just prior to returning the session-id.
---
Nitpick comments:
In `@transports/bifrost-http/websocket/session_test.go`:
- Around line 94-134: Add explicit unit tests to cover the missing peek
accessors and turn-hook lifecycle: create a test that exercises
PeekRealtimeInputText, PeekRealtimeInputRaw, and PeekRealtimeToolOutputs to
assert they return current values without clearing state (consume should still
return the same values afterwards); add tests around TryBeginRealtimeTurnHooks,
SetRealtimeTurnHooks, AbortRealtimeTurnHooks, ConsumeRealtimeTurnHooks, and
ClearRealtimeTurnHooks to validate reservation behavior, abort releases
reservation, Set assigns hooks, Consume transfers ownership and clears internal
state, and Clear removes hooks; add assertions that SessionManager.Snapshot()
includes current realtime state and turn-hook metadata; and add a test that
Session.Close() performs cleanup (e.g., clears any reserved/active turn hooks)
so no hooks remain after close. Ensure each test references the methods
TryBeginRealtimeTurnHooks, SetRealtimeTurnHooks, AbortRealtimeTurnHooks,
ConsumeRealtimeTurnHooks, ClearRealtimeTurnHooks, PeekRealtimeInputText,
PeekRealtimeInputRaw, PeekRealtimeToolOutputs, SessionManager.Snapshot, and
Session.Close to locate code under test.
In `@transports/bifrost-http/websocket/session.go`:
- Around line 148-156: The AppendRealtimeOutputText method currently appends to
s.realtimeOutputText with += causing repeated allocations; switch to using a
strings.Builder stored on the Session (e.g., add a realtimeOutputBuilder field
of type strings.Builder alongside mu and realtimeOutputText or replace
realtimeOutputText) and have AppendRealtimeOutputText write to that builder
under the existing s.mu lock (s.realtimeOutputBuilder.WriteString(text)); update
any methods that read realtimeOutputText (e.g., getters or Reset/Flush
functions) to call s.realtimeOutputBuilder.String() (or
s.realtimeOutputBuilder.Reset() when clearing) while holding the mutex to
preserve thread-safety.
🪄 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
Run ID: 695d2807-c121-47eb-92ee-8c07d0c08d33
📒 Files selected for processing (4)
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/bifrost-http/websocket/session.gotransports/bifrost-http/websocket/session_test.go
✅ Files skipped from review due to trivial changes (1)
- transports/bifrost-http/lib/ctx_test.go
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
8f0f53b to
061807b
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
44d1515 to
3062e94
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
transports/bifrost-http/websocket/session.go (1)
148-156: Consider usingstrings.Builderfor output text accumulation.String concatenation with
+=is O(n) per append, which becomes O(n²) when accumulating many small streaming deltas. Astrings.Builderprovides O(1) amortized appends.♻️ Proposed refactor
Change the field type and methods:
// Session struct field change - realtimeOutputText string + realtimeOutputText strings.Builder func (s *Session) AppendRealtimeOutputText(text string) { if text == "" { return } s.mu.Lock() defer s.mu.Unlock() - s.realtimeOutputText += text + s.realtimeOutputText.WriteString(text) } func (s *Session) ConsumeRealtimeOutputText() string { s.mu.Lock() defer s.mu.Unlock() - text := s.realtimeOutputText - s.realtimeOutputText = "" + text := s.realtimeOutputText.String() + s.realtimeOutputText.Reset() return text }Add
"strings"to imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session.go` around lines 148 - 156, The AppendRealtimeOutputText method currently concatenates into s.realtimeOutputText with += which is O(n^2) for many small appends; change the Session.realtimeOutputText field from string to strings.Builder, add "strings" to imports, and update AppendRealtimeOutputText to call s.realtimeOutputText.WriteString(text) while holding s.mu (keep s.mu.Lock()/defer Unlock()). Also update any accessors (e.g., getters or serializers that read realtimeOutputText) to call s.realtimeOutputText.String() to obtain the accumulated string.transports/bifrost-http/websocket/session_test.go (1)
94-134: Add test coverage for turn hooks lifecycle methods.The test covers realtime state (ID, provider session, output/input text, tool outputs) but omits the turn hooks lifecycle methods (
TryBeginRealtimeTurnHooks,SetRealtimeTurnHooks,AbortRealtimeTurnHooks,ConsumeRealtimeTurnHooks,ClearRealtimeTurnHooks,PeekRealtimeTurnHooks). These methods manage critical single-slot reservation semantics that should be validated.🧪 Suggested test addition
func TestSessionRealtimeTurnHooks(t *testing.T) { session := NewSession(newTestConn()) // TryBegin should succeed on fresh session if !session.TryBeginRealtimeTurnHooks() { t.Fatal("TryBeginRealtimeTurnHooks() should succeed on fresh session") } // TryBegin should fail when busy if session.TryBeginRealtimeTurnHooks() { t.Fatal("TryBeginRealtimeTurnHooks() should fail when already busy") } // Abort should release the slot session.AbortRealtimeTurnHooks() if !session.TryBeginRealtimeTurnHooks() { t.Fatal("TryBeginRealtimeTurnHooks() should succeed after Abort") } session.AbortRealtimeTurnHooks() // SetRealtimeTurnHooks should install hooks cleanupCalled := false state := &RealtimeTurnPluginState{ RequestID: "req-123", Cleanup: func() { cleanupCalled = true }, } session.SetRealtimeTurnHooks(state) // TryBegin should fail when hooks installed if session.TryBeginRealtimeTurnHooks() { t.Fatal("TryBeginRealtimeTurnHooks() should fail when hooks installed") } // Peek should return installed hooks if got := session.PeekRealtimeTurnHooks(); got != state { t.Fatal("PeekRealtimeTurnHooks() should return installed hooks") } // Consume should return and clear hooks consumed := session.ConsumeRealtimeTurnHooks() if consumed != state { t.Fatal("ConsumeRealtimeTurnHooks() should return installed hooks") } if session.PeekRealtimeTurnHooks() != nil { t.Fatal("PeekRealtimeTurnHooks() should return nil after Consume") } if cleanupCalled { t.Fatal("Consume should not call Cleanup (ownership transfer)") } // ClearRealtimeTurnHooks should call Cleanup cleanupCalled = false session.SetRealtimeTurnHooks(&RealtimeTurnPluginState{ Cleanup: func() { cleanupCalled = true }, }) session.ClearRealtimeTurnHooks() if !cleanupCalled { t.Fatal("ClearRealtimeTurnHooks() should call Cleanup") } // TryBegin should fail on closed session session.Close() if session.TryBeginRealtimeTurnHooks() { t.Fatal("TryBeginRealtimeTurnHooks() should fail on closed session") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session_test.go` around lines 94 - 134, The test suite is missing coverage for the realtime turn-hooks lifecycle (TryBeginRealtimeTurnHooks, SetRealtimeTurnHooks, AbortRealtimeTurnHooks, ConsumeRealtimeTurnHooks, ClearRealtimeTurnHooks, PeekRealtimeTurnHooks); add a new test (e.g., TestSessionRealtimeTurnHooks) that verifies: TryBeginRealtimeTurnHooks succeeds on a fresh session and fails when busy, AbortRealtimeTurnHooks releases the slot, SetRealtimeTurnHooks installs a RealtimeTurnPluginState with Cleanup, TryBeginRealtimeTurnHooks fails while hooks are installed, PeekRealtimeTurnHooks returns the installed state, ConsumeRealtimeTurnHooks returns and clears the state without calling Cleanup, ClearRealtimeTurnHooks calls Cleanup, and TryBeginRealtimeTurnHooks fails after session.Close(); use the existing NewSession(newTestConn()) helper and the RealtimeTurnPluginState struct in your assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@transports/bifrost-http/lib/ctx.go`:
- Around line 55-57: The log call in ctx.go currently emits the raw,
user-controlled baggage "session-id" (logger.Warn with value variable), which
may leak sensitive identifiers; update the logger.Warn invocation in the same
block (the code referencing logger.Warn, session-id, and the value variable) to
stop logging the raw value and only log metadata such as the length (e.g.,
include length=%d) and any non-sensitive context instead of value=%s.
---
Nitpick comments:
In `@transports/bifrost-http/websocket/session_test.go`:
- Around line 94-134: The test suite is missing coverage for the realtime
turn-hooks lifecycle (TryBeginRealtimeTurnHooks, SetRealtimeTurnHooks,
AbortRealtimeTurnHooks, ConsumeRealtimeTurnHooks, ClearRealtimeTurnHooks,
PeekRealtimeTurnHooks); add a new test (e.g., TestSessionRealtimeTurnHooks) that
verifies: TryBeginRealtimeTurnHooks succeeds on a fresh session and fails when
busy, AbortRealtimeTurnHooks releases the slot, SetRealtimeTurnHooks installs a
RealtimeTurnPluginState with Cleanup, TryBeginRealtimeTurnHooks fails while
hooks are installed, PeekRealtimeTurnHooks returns the installed state,
ConsumeRealtimeTurnHooks returns and clears the state without calling Cleanup,
ClearRealtimeTurnHooks calls Cleanup, and TryBeginRealtimeTurnHooks fails after
session.Close(); use the existing NewSession(newTestConn()) helper and the
RealtimeTurnPluginState struct in your assertions.
In `@transports/bifrost-http/websocket/session.go`:
- Around line 148-156: The AppendRealtimeOutputText method currently
concatenates into s.realtimeOutputText with += which is O(n^2) for many small
appends; change the Session.realtimeOutputText field from string to
strings.Builder, add "strings" to imports, and update AppendRealtimeOutputText
to call s.realtimeOutputText.WriteString(text) while holding s.mu (keep
s.mu.Lock()/defer Unlock()). Also update any accessors (e.g., getters or
serializers that read realtimeOutputText) to call s.realtimeOutputText.String()
to obtain the accumulated string.
🪄 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
Run ID: c141c0e7-d202-4c59-86f6-6b3b87b97320
📒 Files selected for processing (4)
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/bifrost-http/websocket/session.gotransports/bifrost-http/websocket/session_test.go
✅ Files skipped from review due to trivial changes (1)
- transports/bifrost-http/lib/ctx_test.go
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
061807b to
bf57993
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
3062e94 to
abc9e04
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
transports/bifrost-http/websocket/session_test.go (1)
94-134: Consider adding tests for the turn hooks lifecycle methods inTestSessionRealtimeStateor a separate test function.The test validates ID generation, provider session ID, output/input text accumulation, and tool output queuing. However, there's no coverage for the turn hooks lifecycle methods (
TryBeginRealtimeTurnHooks,SetRealtimeTurnHooks,ConsumeRealtimeTurnHooks,ClearRealtimeTurnHooks,AbortRealtimeTurnHooks,PeekRealtimeTurnHooks), which manage the turn-scoped plugin pipeline state with lock coordination and conditional logic branches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session_test.go` around lines 94 - 134, Add tests that exercise the turn-hooks lifecycle methods (TryBeginRealtimeTurnHooks, SetRealtimeTurnHooks, PeekRealtimeTurnHooks, ConsumeRealtimeTurnHooks, ClearRealtimeTurnHooks, AbortRealtimeTurnHooks) either by extending TestSessionRealtimeState or in a new TestSessionTurnHooks function: create a Session (NewSession(newTestConn())), call TryBeginRealtimeTurnHooks and assert it returns true then SetRealtimeTurnHooks with a sample hook list, use PeekRealtimeTurnHooks to verify hooks are visible without consuming, then call ConsumeRealtimeTurnHooks and assert the returned hooks match and subsequent ConsumeRealtimeTurnHooks/PeekRealtimeTurnHooks return empty; also test ClearRealtimeTurnHooks clears hooks (start a turn, set hooks, call ClearRealtimeTurnHooks, assert no hooks), and test AbortRealtimeTurnHooks behavior when a turn is active (ensure it releases the lock so TryBeginRealtimeTurnHooks can start another turn) plus a negative branch where TryBeginRealtimeTurnHooks returns false if a turn is already active. Include assertions around locking/visibility transitions to cover conditional branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@transports/bifrost-http/websocket/session_test.go`:
- Around line 94-134: Add tests that exercise the turn-hooks lifecycle methods
(TryBeginRealtimeTurnHooks, SetRealtimeTurnHooks, PeekRealtimeTurnHooks,
ConsumeRealtimeTurnHooks, ClearRealtimeTurnHooks, AbortRealtimeTurnHooks) either
by extending TestSessionRealtimeState or in a new TestSessionTurnHooks function:
create a Session (NewSession(newTestConn())), call TryBeginRealtimeTurnHooks and
assert it returns true then SetRealtimeTurnHooks with a sample hook list, use
PeekRealtimeTurnHooks to verify hooks are visible without consuming, then call
ConsumeRealtimeTurnHooks and assert the returned hooks match and subsequent
ConsumeRealtimeTurnHooks/PeekRealtimeTurnHooks return empty; also test
ClearRealtimeTurnHooks clears hooks (start a turn, set hooks, call
ClearRealtimeTurnHooks, assert no hooks), and test AbortRealtimeTurnHooks
behavior when a turn is active (ensure it releases the lock so
TryBeginRealtimeTurnHooks can start another turn) plus a negative branch where
TryBeginRealtimeTurnHooks returns false if a turn is already active. Include
assertions around locking/visibility transitions to cover conditional branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6893b90a-8480-4b92-b856-a0ef54d76d13
📒 Files selected for processing (4)
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/bifrost-http/websocket/session.gotransports/bifrost-http/websocket/session_test.go
✅ Files skipped from review due to trivial changes (2)
- transports/bifrost-http/lib/ctx_test.go
- transports/bifrost-http/lib/ctx.go
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
abc9e04 to
5cc0b46
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
bf57993 to
87f5ecf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
transports/bifrost-http/websocket/session_test.go (1)
94-134: Please add hook-lifecycle coverage here too.These assertions cover the simple buffers, but the new single-slot turn state (
TryBeginRealtimeTurnHooks,AbortRealtimeTurnHooks,ConsumeRealtimeTurnHooks,ClearRealtimeTurnHooks, andClose()cleanup) is still untested. A small regression matrix around those transitions would protect the trickiest part of this change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session_test.go` around lines 94 - 134, Add tests exercising the single-slot turn-hook lifecycle on the Session created in TestSessionRealtimeState: call TryBeginRealtimeTurnHooks() on the session to assert it returns true when empty and returns false if called again without clearing; use AddRealtimeTurnHook (or the hook-adding API on Session) then call ConsumeRealtimeTurnHooks() to assert the returned hooks match what was added; call AbortRealtimeTurnHooks() and ClearRealtimeTurnHooks() to verify they free the slot (TryBeginRealtimeTurnHooks() returns true afterward and ConsumeRealtimeTurnHooks() yields empty); finally call Close() on the session and assert that subsequent TryBeginRealtimeTurnHooks() fails or that hooks are cleared to validate cleanup. Ensure you reference the methods TryBeginRealtimeTurnHooks, AbortRealtimeTurnHooks, ConsumeRealtimeTurnHooks, ClearRealtimeTurnHooks, and Close in the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@transports/bifrost-http/websocket/session_test.go`:
- Around line 136-138: newTestConn currently returns a zero-value *ws.Conn which
leaves its internal net conn nil so session.Close() returns ErrNilNetConn and
never exercises real connection cleanup; replace newTestConn with a real or
mockable websocket connection whose underlying net connection is non-nil (e.g.,
create a paired net.Conn via net.Pipe and build a ws.Conn around one end, or
inject a mock ws.Conn implementing the same Close behavior) so that
session.Close(), SessionManager.Remove(), and CloseAll() actually invoke
clientConn.Close() and run real close-path logic.
In `@transports/bifrost-http/websocket/session.go`:
- Around line 320-323: Close() currently only nils s.realtimeTurnHooks if the
Cleanup hook is non-nil, leaving a state with Cleanup: nil reachable via
PeekRealtimeTurnHooks()/ConsumeRealtimeTurnHooks(); change Close() so it always
sets s.realtimeTurnHooks = nil (clear the pointer) and, if
s.realtimeTurnHooks.Cleanup != nil, call Cleanup() before nulling—i.e., call
Cleanup when present but ensure s.realtimeTurnHooks is cleared in all cases to
match ClearRealtimeTurnHooks() semantics and avoid returning stale hooks from
PeekRealtimeTurnHooks()/ConsumeRealtimeTurnHooks().
- Around line 325-327: The stack currently closes the same websocket in two
places (handleUpgrade defers conn.Close() and Session.Close closes
s.clientConn), causing double-close races; pick one owner — centralize ownership
in Session by removing the deferred conn.Close() in handleUpgrade so the socket
is only closed by Session.Close (called via SessionManager.Remove). Update
handleUpgrade to assign conn to the Session without deferring Close, ensure
Session.Close uses s.clientConn.Close(), sets s.clientConn = nil and is
idempotent (safe to call multiple times).
---
Nitpick comments:
In `@transports/bifrost-http/websocket/session_test.go`:
- Around line 94-134: Add tests exercising the single-slot turn-hook lifecycle
on the Session created in TestSessionRealtimeState: call
TryBeginRealtimeTurnHooks() on the session to assert it returns true when empty
and returns false if called again without clearing; use AddRealtimeTurnHook (or
the hook-adding API on Session) then call ConsumeRealtimeTurnHooks() to assert
the returned hooks match what was added; call AbortRealtimeTurnHooks() and
ClearRealtimeTurnHooks() to verify they free the slot
(TryBeginRealtimeTurnHooks() returns true afterward and
ConsumeRealtimeTurnHooks() yields empty); finally call Close() on the session
and assert that subsequent TryBeginRealtimeTurnHooks() fails or that hooks are
cleared to validate cleanup. Ensure you reference the methods
TryBeginRealtimeTurnHooks, AbortRealtimeTurnHooks, ConsumeRealtimeTurnHooks,
ClearRealtimeTurnHooks, and Close in the assertions.
🪄 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
Run ID: 26e6d066-7f09-4824-8dbf-3a3e08b3b637
📒 Files selected for processing (4)
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/bifrost-http/websocket/session.gotransports/bifrost-http/websocket/session_test.go
✅ Files skipped from review due to trivial changes (1)
- transports/bifrost-http/lib/ctx_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- transports/bifrost-http/lib/ctx.go
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
87f5ecf to
291d148
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
transports/bifrost-http/websocket/session_test.go (3)
114-120: Lock in clear-after-read semantics for the input buffers.
ConsumeRealtimeOutputText()andConsumeRealtimeToolOutputs()already assert the buffer is cleared, but the input/raw branches never do. If either consume method stopped clearing session state, this test would still pass.Suggested test tweak
session.SetRealtimeInputText("hello") if got := session.ConsumeRealtimeInputText(); got != "hello" { t.Fatalf("ConsumeRealtimeInputText() = %q, want %q", got, "hello") } + if got := session.ConsumeRealtimeInputText(); got != "" { + t.Fatalf("ConsumeRealtimeInputText() after clear = %q, want empty string", got) + } session.SetRealtimeInputRaw(`{"type":"conversation.item.create"}`) if got := session.ConsumeRealtimeInputRaw(); got != `{"type":"conversation.item.create"}` { t.Fatalf("ConsumeRealtimeInputRaw() = %q, want raw input", got) } + if got := session.ConsumeRealtimeInputRaw(); got != "" { + t.Fatalf("ConsumeRealtimeInputRaw() after clear = %q, want empty string", got) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session_test.go` around lines 114 - 120, The test needs to assert clear-after-read semantics for input buffers: after calling session.ConsumeRealtimeInputText() and session.ConsumeRealtimeInputRaw() also verify the buffers were cleared (e.g., by calling the consume methods again or checking the raw/text getters return empty) so the test fails if those consume methods stop clearing state; update the test around SetRealtimeInputText/ConsumeRealtimeInputText and SetRealtimeInputRaw/ConsumeRealtimeInputRaw to include an additional assertion that the buffer is empty immediately after consumption.
95-98: Assert thatSession.ID()is stable, not just populated.This only proves the ID is non-empty. The new contract in this stack is a stable per-session UUID, so a regression that regenerated the ID on each
ID()call would still pass here.Suggested test tweak
session := NewSession(newTestConn()) - if session.ID() == "" { + id := session.ID() + if id == "" { t.Fatal("expected session ID to be populated") } + if got := session.ID(); got != id { + t.Fatalf("ID() on second call = %q, want stable %q", got, id) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session_test.go` around lines 95 - 98, The test currently only checks that NewSession(...).ID() is non-empty; change it to assert stability by capturing the first value from session.ID() (call it initialID) and then call session.ID() multiple times (at least once more) and assert each subsequent value equals initialID and is non-empty so the ID is a stable per-session UUID; reference the NewSession constructor and the Session.ID() method when updating the test.
39-58: Exercise the new turn-hook cleanup path in the close tests.Both close-path tests only check
session.closed. This PR also adds realtime turn-hook cleanup inSession.Close(), so a regression that stopped aborting or clearing hook state would still pass. Seed turn-hook state beforeRemove()/CloseAll()and assert it is torn down.Also applies to: 70-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/websocket/session_test.go` around lines 39 - 58, The tests TestSessionManagerRemove and the other close-path test should also exercise the realtime turn-hook cleanup: before calling manager.Remove(conn) (and before calling CloseAll in the other test), set up the session's turn-hook state (e.g., populate whatever field or registry the Session uses for turn hooks), then call SessionManager.Remove / Session.CloseAll and assert that the turn-hook state has been cleared or aborted in addition to checking session.closed; locate the session via manager.Create and Session struct used in those tests and add assertions that the hook-related fields (the turn-hook registry/state) are nil/empty or marked aborted after the removal/close to ensure the new cleanup path is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@transports/bifrost-http/websocket/session_test.go`:
- Around line 114-120: The test needs to assert clear-after-read semantics for
input buffers: after calling session.ConsumeRealtimeInputText() and
session.ConsumeRealtimeInputRaw() also verify the buffers were cleared (e.g., by
calling the consume methods again or checking the raw/text getters return empty)
so the test fails if those consume methods stop clearing state; update the test
around SetRealtimeInputText/ConsumeRealtimeInputText and
SetRealtimeInputRaw/ConsumeRealtimeInputRaw to include an additional assertion
that the buffer is empty immediately after consumption.
- Around line 95-98: The test currently only checks that NewSession(...).ID() is
non-empty; change it to assert stability by capturing the first value from
session.ID() (call it initialID) and then call session.ID() multiple times (at
least once more) and assert each subsequent value equals initialID and is
non-empty so the ID is a stable per-session UUID; reference the NewSession
constructor and the Session.ID() method when updating the test.
- Around line 39-58: The tests TestSessionManagerRemove and the other close-path
test should also exercise the realtime turn-hook cleanup: before calling
manager.Remove(conn) (and before calling CloseAll in the other test), set up the
session's turn-hook state (e.g., populate whatever field or registry the Session
uses for turn hooks), then call SessionManager.Remove / Session.CloseAll and
assert that the turn-hook state has been cleared or aborted in addition to
checking session.closed; locate the session via manager.Create and Session
struct used in those tests and add assertions that the hook-related fields (the
turn-hook registry/state) are nil/empty or marked aborted after the
removal/close to ensure the new cleanup path is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b36a9fe2-b4bc-4060-9704-26c4fa6f36cb
📒 Files selected for processing (4)
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/bifrost-http/websocket/session.gotransports/bifrost-http/websocket/session_test.go
✅ Files skipped from review due to trivial changes (2)
- transports/bifrost-http/lib/ctx_test.go
- transports/bifrost-http/websocket/session.go
🚧 Files skipped from review as they are similar to previous changes (1)
- transports/bifrost-http/lib/ctx.go
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
291d148 to
5b3bf16
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
2 times, most recently
from
79fd370 to
5c9625f
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
2 times, most recently
from
0d37096 to
bc98af8
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
2 times, most recently
from
9f6e663 to
aec9dc0
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
bc98af8 to
11cfa9e
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
11cfa9e to
9af380f
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
aec9dc0 to
702f62b
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
9af380f to
12565cc
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
702f62b to
06d48bd
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
branch
from
06d48bd to
b3aac17
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
from
12565cc to
73e51ea
Compare
Contributor
Merge activity
|
akshaydeo
changed the base branch from
feature/03-27-feat_add_realtime_turn_logging_to_logging_plugin
to
graphite-base/2340
akshaydeo
dismissed
coderabbitai[bot]’s stale review
The base branch was changed.
akshaydeo
deleted the
feature/03-27-feat_add_realtime_session_state_and_transport_context_helpers
branch
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
3 participants
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
Adds W3C baggage header parsing for session ID propagation and extends the
WebSocket session struct with realtime turn state management. Sessions now track
output/input text accumulation, tool outputs, provider session IDs, and
turn-scoped plugin hooks.
Changes
ParseSessionIDFromBaggage(header)for extractingsession-idfrom W3Cbaggage headers
ConvertToBifrostContextto setParentRequestIDandRealtimeSessionIDfrom baggageid(stable UUID),providerSessionID,realtimeOutputText,realtimeInputText,realtimeInputRaw,realtimeToolOutputs,realtimeTurnHooks,realtimeTurnBusyRealtimeToolOutputandRealtimeTurnPluginStatetypesAppendRealtimeOutputText,ConsumeRealtimeOutputText), input tracking, tool output queuing, and turnhook lifecycle management (
TryBeginRealtimeTurnHooks,AbortRealtimeTurnHooks,ConsumeRealtimeTurnHooks)SessionManager.Snapshot()for inspecting active sessionsSession.Close()to clean up turn hooksType of change
Affected areas
How to test
Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
Checklist
docs/contributing/README.mdand followed the guidelines