feat(sessions): relay live frames to every authorized reader behind AGENTA_SESSIONS_SHARED_READER - #6522
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 SummarySummary by CodeRabbit
WalkthroughThis change adds an opt-in live session frame relay. Runners publish temporary frames, Redis workers relay them through SSE, and shared readers render the frames until durable records become available. ChangesLive session relay
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to The opt-in live relay can expose runner authorization through its fallback transport and may show inconsistent shared-reader previews when frames are retried or reuse conflicting identities. These issues should be resolved before merge or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant LiveFramePublisher
participant SessionStreamsRouter
participant RedisStream
participant LiveRelayWorker
participant live_event_stream
participant EventSource
LiveFramePublisher->>SessionStreamsRouter: POST live frame
SessionStreamsRouter->>RedisStream: publish_live_frame
LiveRelayWorker->>RedisStream: consume live frame
LiveRelayWorker->>SessionStreamsRouter: publish session-channel frame
SessionStreamsRouter->>live_event_stream: authorize SSE connection
live_event_stream->>EventSource: emit SSE frame
EventSource->>EventSource: reduce and render preview
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 7.58% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 66 functions across 37 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1📝 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 |
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
AGENT-GENERATED review, low weight. One agent read the branch as a mergeable unit. Treat every line as a pointer to check, not a gate.
Reading guide (5 lines):
- Verdict: ship after fixes. The design is followed on nearly every judged point; two items to fix or accept before the flag turns on in production.
- P1 (streaming.py / records_worker.py): durable records now share one global
MAXLEN=100000stream with the frame flood, and XDEL drain is removed unconditionally. The contract set retention per session. - P2 (client livePreview.ts): every frame of a turn is retained and re-reduced, roughly O(n^2), with no trim; fine at the measured 3161 frames, not at the 100000 cap.
- Verified: one ingress and one stream, envelope roles, producer-set monotonic frame_index, non-blocking ingress and relay, records-worker skips frames, flag-off 404, cross-replica pub/sub, client dedup/order/tool-by-entity, sender stream untouched, tests per behavior.
- Cross-replica and slow-reader paths both check out; the durability risk is the one thing that needs a decision.
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
AGENT-GENERATED follow-up, low weight. Round 2 re-review at 7ae0792252. Both round-1 findings are addressed. Verdict: ship.
P1 durability (42ce70d). Durable records no longer share a global count cap. Frames are trimmed by age via MINID at min(age_cutoff, records_ack_frontier), so a durable entry newer than the records worker's acknowledged/pending frontier is never trimmed. The records worker XDELs consumed durable records after XACK and leaves frames for the relay. The count MAXLEN is now a caught-up-only safety valve (pending==0 and lag==0), raised to 100M. The flood-with-stalled-worker test, the post-ACK durable-only deletion test, and the MINID age-trim test all assert these invariants.
P2 client memory (7ae0792). Preview state is per entity plus lastFrameIndex, with no frame array or frame-id set, so memory is bounded by entity count, not frame count. The 5,000-delta test keeps one entity and the same final text.
Capability wording now matches decisions.md (deployment-wide switch, no version-one project allowlist).
Tests I ran for this re-review: the relay and ingest files (test_live_relay.py, test_live_frame_ingest.py) 12 passed against integration Postgres on 5440 (Redis stream semantics on fakeredis, since the stack Redis is not host-published); the chat package suite 641 passed including the 5 livePreview reducer tests.
Two non-blocking notes remain: trim_live_stream runs inline in the ingest path every 64 frames and trims the whole global stream, which repeats redundantly under concurrency (turn is not blocked); and the client now drops out-of-order frames rather than reordering, which is safe under the in-order sequential pump and reconnect-clears-preview.
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
AGENT-GENERATED follow-up, low weight. Round 3 at 2146213306: ship. Desktop now subscribes through the shared useSessionLivePreview hook via the centralized sharedReaderAdvertised && runningElsewhere predicate (one subscription, sender's invoke stream untouched); the AGENTA_RUNNER_LIVE_FRAMES passthrough is in all 7 Compose files, Railway template + configure.sh, and the Helm runner deployment/schema/values with production defaults off, and the 4 env examples document the settings. Verified: 7/7 docker compose config pass, Helm renders the var true/false when set and omits it by default, chat 645 and oss 442 (1 skip) pass, and the subscription predicate has a 4-case test.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: fc76bded-8b12-4eb0-8bee-bbf242c80e71
📒 Files selected for processing (50)
api/entrypoints/worker_streams.pyapi/oss/src/apis/fastapi/sessions/live_events.pyapi/oss/src/apis/fastapi/sessions/models.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/apis/fastapi/sessions/utils.pyapi/oss/src/core/sessions/records/dtos.pyapi/oss/src/core/sessions/records/streaming.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/dbs/redis/sessions/contract.pyapi/oss/src/tasks/asyncio/sessions/live_relay_worker.pyapi/oss/src/tasks/asyncio/sessions/records_worker.pyapi/oss/src/utils/env.pyapi/oss/tests/pytest/unit/sessions/test_live_frame_ingest.pyapi/oss/tests/pytest/unit/sessions/test_live_relay.pyhosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/ee/docker-compose.gh.local.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.gh.local.ymlhosting/docker-compose/oss/docker-compose.gh.ssl.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/oss/env.oss.gh.examplehosting/kubernetes/helm/templates/runner-deployment.yamlhosting/kubernetes/helm/values.schema.jsonhosting/kubernetes/helm/values.yamlhosting/railway/oss/scripts/configure.shhosting/railway/oss/template/template.jsonservices/runner/src/sessions/live-frames.tsservices/runner/src/sessions/persist.tsservices/runner/tests/unit/live-frames.test.tsweb/mobile/src/features/chat/ChatScreen.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.tsweb/oss/src/components/AgentChatSlice/state/liveness.tsweb/packages/agenta-chat/src/hooks/index.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/hooks/useSessionLivePreview.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/livePreview.tsweb/packages/agenta-chat/src/transport/index.tsweb/packages/agenta-chat/src/transport/sessionLiveEvents.tsweb/packages/agenta-chat/tests/unit/model/livePreview.test.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/src/session/state/livePreview.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.
Railway Preview Environment
Updated at 2026-09-05T10:39:18.050Z |
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
🤖 The AI agent says:
This PR relays live frames to other clients while preserving the durable record consumer.
Focused simplification review using the new simplify skill: one non-blocking suggestion below. This is a static design/code review, not a rerun of the live release gate or a replacement for existing correctness reviews.
|
🤖 The AI agent says: Correctness follow-up at |
📘 Docs preview
This comment updates in place on every push. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/oss/src/apis/fastapi/sessions/router.py (1)
847-862: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftEnforce frame idempotency before publication.
Lines 847-862 publish every accepted frame. An identical retry after a lost response creates another Redis entry and SSE emission. A conflicting payload that reuses
frame_or_event_idorframe_indexis also accepted. Atomically compare and store the frame identity and payload fingerprint. Treat identical retries as no-ops and reject conflicting reuse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: fa687350-e735-4372-8c94-0ac3a777917f
📒 Files selected for processing (22)
api/entrypoints/worker_streams.pyapi/oss/src/apis/fastapi/sessions/models.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/records/dtos.pyapi/oss/src/core/sessions/records/streaming.pyapi/oss/src/tasks/asyncio/sessions/live_relay_worker.pyapi/oss/src/tasks/asyncio/sessions/records_worker.pyapi/oss/src/utils/env.pyapi/oss/tests/pytest/unit/sessions/test_live_frame_ingest.pyapi/oss/tests/pytest/unit/sessions/test_live_relay.pydocs/design/session-control-and-live-events/contracts/events.mddocs/design/session-control-and-live-events/decisions.mddocs/design/session-control-and-live-events/live-frame-envelope.mdhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/oss/env.oss.gh.exampleweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/transport/sessionLiveEvents.tsweb/packages/agenta-chat/tests/unit/transport/sessionLiveEvents.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- hosting/docker-compose/ee/env.ee.gh.example
- api/oss/src/apis/fastapi/sessions/models.py
- api/entrypoints/worker_streams.py
- hosting/docker-compose/oss/env.oss.gh.example
- web/oss/src/components/AgentChatSlice/hooks/useSessionHydration.ts
- web/oss/src/components/AgentChatSlice/AgentConversation.tsx
- web/packages/agenta-chat/src/hooks/useAgentConversation.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Agent-generated Codex review, low weight.
SHIP AFTER FIXES
The dedicated live stream correctly isolates live trimming and acknowledgement from durable records, and the ownership, slow-reader, capability, and non-blocking failure paths are otherwise well covered. I found three release-relevant gaps: frame discontinuities are rendered instead of rejected, the flag-off durable queue no longer has its base retention cap, and a relay-only startup failure can prevent every durable consumer from starting. The new event-contract document also describes the next sender/replay increment as if it shipped here. Details and requested regressions are inline.
I reviewed ded2cc3...29337bd and all 15 resolved threads. I did not rerun the already-passing focused suites; two preview end-to-end CI jobs were still pending at review time.
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Agent-generated Codex review, low weight.
SHIP
No findings in 29337bdfbd..38df392a17.
The reducer now rejects an initial index above zero and any internal gap, clears and suppresses the preview, and triggers durable revalidation; a reader receiving 0, 1, ... still renders immediately. Durable records restore the base MAXLEN=100000 with approximate trimming in both flag states. Relay construction, XGROUP, and prune failures share one logged relay-only boundary after durable initialization, so durable consumers still run. events.md now separates shipped frame-only SSE behavior from later durable sender/replay work and matches the implementation.
Verification: the focused worker-streams startup test passed (1 test, no xdist, nice 19), and the correction diff passes git diff --check.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: f206cfe0-6c56-4af6-93bd-e2877f437981
📒 Files selected for processing (9)
api/entrypoints/worker_streams.pyapi/oss/src/core/sessions/records/streaming.pyapi/oss/tests/pytest/unit/sessions/test_live_frame_ingest.pyapi/oss/tests/pytest/unit/sessions/test_worker_streams_startup.pydocs/design/session-control-and-live-events/contracts/events.mdweb/packages/agenta-chat/src/hooks/useSessionLivePreview.tsweb/packages/agenta-chat/src/model/livePreview.tsweb/packages/agenta-chat/tests/unit/model/livePreview.test.tsweb/packages/agenta-entities/src/session/state/livePreview.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
38df392 to
3cfea3a
Compare
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Agent-generated Codex review, low weight.
SHIP
No findings in the seven adapted commit pairs. The adaptations preserve milestone-1 Stop models, records-worker ACK/retry behavior, the runner Stop path, and watchdog behavior while retaining PR 6522 behavior: a separate bounded live stream, the 64 KiB limit, relay startup isolation, gap rejection, and no-store.
The rebased 54-file lane contains no files outside the original 55-file PR set. records_worker.py is the sole old-only file because the rebased final version is byte-identical to milestone 1. I relied on the assembly post-rebase validation and did not rerun a test.
Publish temporary session frames to a dedicated bounded Redis Stream. Restore the durable records queue and both consumers to independent ACK and delete behavior, with count and age retention tests. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Reject live frames above 64 KiB during request validation and immediately before Redis publication. Apply the same limit to declared HTTP request bodies and cover each boundary with regression tests. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Mark the identity-scoped session event stream as no-store so intermediaries and browsers do not retain live session content. Pin the response header in a route regression test. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Document one HTTP ingress with separate durable-record and disposable-frame Redis Streams. Record the deployment-wide frame bounds and clarify that clients receive the global shared-reader capability. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Validate session live events with the shared diagnostic helper and cover invalid-frame logging. Shorten the three reviewed live-relay comments without changing behavior. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Require each live execution to start at frame zero and advance contiguously. Suppress the preview after a gap and refresh durable session records. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Restore the base branch's 100,000-entry approximate trim for durable record publication. Pin the flag-off Redis stream arguments in a regression test. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Initialize durable consumers before the optional live relay. Log relay setup failures and continue running the durable stream consumers. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Document the live-frame relay and gap handling as the shipped contract. Mark sender handoff, durable event envelopes, sequences, watermarks, and Postgres replay as later target behavior. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
3cfea3a to
ecb2214
Compare
1 participant
Context
Only the browser that starts a turn sees its live text and tool progress; every other client waits for change notices and reloads completed records. This is increment 4, part one, of the session-control design on PR #6495: shared live output for secondary readers. The sender keeps its invoke stream; snapshot and replay come in part two.
Changes
live-frame-envelope.md:kind: frame,execution_id, a monotonicframe_index,entity_id, the unchanged eventtype,payload,created_at. Fire-and-forget with a bounded buffer and a drop counter; behind the runner settingAGENTA_RUNNER_LIVE_FRAMES(default off).GET /sessions/{id}/events, beside the existing watch route. Authorization is the transcript's project check, re-checked on an interval; a slow reader is closed with a frame that says why and never blocks the consumer. BehindAGENTA_SESSIONS_SHARED_READER(default off; the route returns 404 when off).@agenta/chatand@agenta/entitiesrenders live text and tool progress for a session this browser did not start, keyed by(execution_id, frame_index)withentity_idfor updates; on disconnect it discards previews and refetches the transcript. Desktop wired in AgentChatSlice; mobile if the hook fits (see the status file).Tests
42ce70dcd5,7ae0792252): frames are now trimmed by age and never past the records worker's acknowledged frontier, so a worker stall cannot evict an unconsumed durable record, and the worker drains consumed durable records again; the client preview is bounded per entity (5,000-delta test). Re-review verdict: ship. The first live proof on its own stack showed the server side working end to end (synthetic frames reached a real browser reader within a second; two consumer groups at lag 0; flag off gave 404 and on gave 200) and found two gaps in this PR's scope: the desktop chat never wired the shared reader (only mobile did), and the runner compose service did not passAGENTA_RUNNER_LIVE_FRAMES. Round 3 (2146213306) wires the desktop through the shared hook and passes the flag through every compose file, the Railway template, and the Helm chart with production defaults off, and documents the relay settings in the env examples. Live proof on2146213306(stack agenta-ee-dev-session-inc4, screenshotsr3-*under~/agenta-qa-evidence/2026-09-04-inc4-relay/):relay-closewith reasonslow_readerBoth consumer groups stayed at lag 0 through the burst.
Agent-generated, low weight. Not merged.
https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV