feat(workflow): compose finite WebSocket sessions - #27
Conversation
|
Warning Review limit reached
Next review available in: 18 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughArazzo workflows now support local finite WebSocket session sources beside OpenAPI sources. Plans embed sealed WebSocket child plans, validate bounded bindings and outputs, aggregate grants and risk, enforce policy fingerprints and workflow timeouts, and execute mixed HTTP/WebSocket steps. ChangesWebSocket workflow integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ArazzoWorkflow
participant WorkflowPlanner
participant WebSocketExecutor
participant WebSocketPeer
ArazzoWorkflow->>WorkflowPlanner: plan mixed HTTP and websocket steps
WorkflowPlanner->>WebSocketExecutor: invoke sealed websocket child plan
WebSocketExecutor->>WebSocketPeer: execute finite websocket session
WebSocketPeer-->>WebSocketExecutor: return messages and close data
WebSocketExecutor-->>ArazzoWorkflow: return bounded outputs and observations
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
crates/kahea-workflow/src/lib.rs (2)
941-961: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the close selectors return the same shape when close data is absent.
$websocket.close.codeand$websocket.close.reasonreturnValue::Nullwhenobservation.closeisNone.$websocket.close.initiatorreturnsWorkflowError::Invalidin the same state. A workflow that selects the initiator then fails the whole step, while a workflow that selects the code receivesnull. ReturnValue::Nullfor the initiator as well, or fail all three selectors consistently.♻️ Proposed change
"$websocket.close.initiator" => { return observation .close .as_ref() .map(serde_json::to_value) .transpose()? .map(|close| close["initiator"].clone()) - .ok_or_else(|| WorkflowError::Invalid("WebSocket close data is absent".into())); + .map_or(Ok(Value::Null), Ok); }🤖 Prompt for 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. In `@crates/kahea-workflow/src/lib.rs` around lines 941 - 961, Update the "$websocket.close.initiator" selector to return Value::Null when observation.close is absent, matching the existing behavior of "$websocket.close.code" and "$websocket.close.reason"; preserve the current initiator extraction when close data exists.
1994-2008: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the test peer accept and read calls.
spawn_websocket_peercallslistener.accept()andsocket.read()without a timeout. If planning rejects the session or the client never connects, the worker thread blocks andjoin()at the end of the test hangs instead of failing. The executor tests already solve this withaccept_test_connection, which polls a non-blocking listener against a deadline. Apply the same pattern here, and set read and write timeouts on the accepted stream.🤖 Prompt for 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. In `@crates/kahea-workflow/src/lib.rs` around lines 1994 - 2008, The spawn_websocket_peer test helper currently performs unbounded network operations. Reuse the existing accept_test_connection polling/deadline pattern for the listener.accept step, and configure read and write timeouts on the accepted stream before creating the WebSocket; ensure subsequent socket.read and socket.send operations fail within the test deadline so worker join cannot hang.
🤖 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 `@crates/kahea-exec/src/websocket.rs`:
- Around line 430-433: Update the invocation deadline calculation near
total_deadline so overflow from started.checked_add(options.timeout) falls back
to the plan-derived deadline rather than Instant::now(). Ensure total_deadline
does not become immediately expired when options.timeout is extremely large,
while preserving the existing minimum behavior for non-overflowing invocation
deadlines.
In `@crates/kahea-workflow/src/lib.rs`:
- Around line 784-796: Update the WebSocketConnectResult::Denied branch to
record the denial and break immediately, removing the unconditional_retry,
retry-limit, delay, and continue logic from this path. Preserve returning the
original denial result so WebSocket denials remain terminal, matching the HTTP
denial behavior.
- Around line 695-760: In invoke_websocket_workflow_step, after rebuilding
websocket_plan from the bound source, validate that its risk and required_grants
match the sealed step.websocket_plan values, alongside the existing operation
identity check. Return WorkflowError::StepPlan with a clear rejection reason on
any mismatch, before store_websocket_plan or invocation.
---
Nitpick comments:
In `@crates/kahea-workflow/src/lib.rs`:
- Around line 941-961: Update the "$websocket.close.initiator" selector to
return Value::Null when observation.close is absent, matching the existing
behavior of "$websocket.close.code" and "$websocket.close.reason"; preserve the
current initiator extraction when close data exists.
- Around line 1994-2008: The spawn_websocket_peer test helper currently performs
unbounded network operations. Reuse the existing accept_test_connection
polling/deadline pattern for the listener.accept step, and configure read and
write timeouts on the accepted stream before creating the WebSocket; ensure
subsequent socket.read and socket.send operations fail within the test deadline
so worker join cannot hang.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c2533c7-8142-428f-93cb-f42b551f9435
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
README.mdcrates/kahea-core/src/lib.rscrates/kahea-exec/src/websocket.rscrates/kahea-workflow/Cargo.tomlcrates/kahea-workflow/src/lib.rscrates/kahea/tests/cli.rsdocs/architecture/0001-websocket-sessions.mdfixtures/workflows/events.websocket.jsonfixtures/workflows/mixed.arazzo.yamlplugins/kahea/skills/kahea/SKILL.md
1 participant
Summary
x-kahea-source-kind: websocket-sessionextension and add deterministic fixturesSafety and compatibility
Validation
scripts/gates.shCloses #15
Summary by CodeRabbit
New Features
Bug Fixes
Documentation