feat: add event-handler WebSocket support (issue #555) - #593
Conversation
Implement real-time WebSocket servers with natural-language syntax:
listen for websockets on port 8080 as chat_server
on websocket connect to chat_server as conn:
send websocket message "Welcome!" to conn
end on
on websocket message from chat_server as msg:
broadcast websocket message body of msg to chat_server
end on
WebSockets reuse the HTTP server's single-threaded design: warp runs each
socket in background tokio tasks that push connect/message/disconnect events
over a channel, and the interpreter dispatches to the registered
`on websocket ...` handler blocks while the program is inside a `wait`.
Changes:
- Parser/AST: listen-for-websockets, on-websocket handler blocks, send and
broadcast statements; no new keywords (kept `message` as an identifier).
- Interpreter: WflWebSocketServer, per-connection warp task, event pump hooked
into `wait for <duration>`, handler dispatch, send/broadcast; `close server`
now also closes a WebSocket server and its connections.
- Fix `property of object` access (`body of msg`, `method of request`): it
parsed as a call and errored at runtime; added an object-field fallback in
FunctionCall evaluation so natural property reads resolve.
- Analyzer/typechecker/transpiler wired for the new statements.
- Tests: tests/websocket_test.rs (real tokio-tungstenite client, echo +
broadcast) and TestPrograms/websocket_echo_server.wfl (CI-safe).
- Docs: WebSocket section in Docs/04-advanced-features/web-servers.md; dev diary.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Mp4HoighK1ALD1BxdMcBk
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds WebSocket support to WFL: new syntax, AST nodes, semantic checks, runtime event dispatch, docs, demo programs, and end-to-end tests. ChangesWebSocket feature implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant WflProgram
participant Interpreter
participant WsConnectionRegistry
participant ClientSocket
WflProgram->>Interpreter: wait for duration
Interpreter->>Interpreter: pump_websocket_events
ClientSocket->>Interpreter: connect/message/disconnect event
Interpreter->>Interpreter: dispatch_ws_event
Interpreter->>WsConnectionRegistry: enqueue outbound frame
WsConnectionRegistry->>ClientSocket: send text/close frame
Possibly related PRs
🚥 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baf25221ae
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let ws_properties = ["id", "ip", "body", "sender"]; | ||
| for prop in ws_properties { | ||
| self.action_parameters.insert(prop.to_string()); |
There was a problem hiding this comment.
If a program already has a variable named body, id, ip, or sender in an outer scope (an earlier HTTP wait for request also creates body implicitly), the documented body of msg/id of conn syntax still fails semantic analysis: analyze_expression resolves the callee to that non-function symbol before consulting action_parameters, reports '<name>' is not a function, and the CLI aborts before the new runtime property fallback can run. The WebSocket field allowance needs to take precedence over non-function symbols while analyzing handler bodies.
Useful? React with 👍 / 👎.
|
|
||
| let event_obj = build_ws_event_object(&event); | ||
| let child_env = Environment::new_child_env(&handler_env); | ||
| if let Err(msg) = child_env.borrow_mut().define(&binding, event_obj) { |
There was a problem hiding this comment.
When the handler binding name already exists in the captured environment, for example store conn as ... before on websocket connect ... as conn, Environment::define rejects the binding because it checks parent scopes. The analyzer models this as a scoped handler variable, and other parameter-like runtime bindings use shadowing/direct definition, so the first matching event aborts the surrounding wait with a runtime error instead of running the handler.
Useful? React with 👍 / 👎.
![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 (5)
tests/websocket_test.rs (2)
97-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test coverage for
on websocket disconnectandclose serverwith WebSocket servers.The PR objectives mention connect, message, and disconnect events, but only connect and message are tested. Adding a test for the disconnect handler and verifying
close serverproperly closes WebSocket servers and their connections would strengthen coverage for the new feature.As per coding guidelines, TDD is mandatory for new features in Rust code.
🤖 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 `@tests/websocket_test.rs` around lines 97 - 152, Add missing WebSocket coverage in tests/websocket_test.rs by extending the existing websocket test suite with a case that exercises the `on websocket disconnect` handler and another that verifies `close server` shuts down the WebSocket server and terminates active connections. Use the existing helpers like `start_ws_server`, `connect`, and `next_text`, and place the new assertions alongside `websocket_connect_and_echo` and `websocket_broadcast_reaches_all_clients` so the new behavior is covered end-to-end.Source: Coding guidelines
25-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid leaking
TempDir— return it instead.
std::mem::forget(dir)permanently leaks the temp directory and its files. Sincekill_on_drop(true)already ties the child's lifetime to the test scope, returning theTempDiralongside theChildensures it is cleaned up when the test ends or panics, with no behavioral change.♻️ Proposed refactor
async fn start_ws_server(program: &str) -> (Child, u16) { +async fn start_ws_server(program: &str) -> (Child, u16, tempfile::TempDir) { let dir = tempfile::tempdir().expect("tempdir"); let prog_path = dir.path().join("ws_server.wfl"); std::fs::write(&prog_path, program).expect("write program"); - // Keep the temp dir alive for the process lifetime by leaking it; the OS - // reclaims it when the test process exits. - std::mem::forget(dir); // ... - (child, port) + (child, port, dir) }Then update both call sites:
- let (mut child, port) = start_ws_server(ECHO_PROGRAM).await; + let (mut child, port, _dir) = start_ws_server(ECHO_PROGRAM).await;- let (mut child, port) = start_ws_server(BROADCAST_PROGRAM).await; + let (mut child, port, _dir) = start_ws_server(BROADCAST_PROGRAM).await;🤖 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 `@tests/websocket_test.rs` around lines 25 - 27, The test helper is leaking the TempDir via std::mem::forget(dir), which should be removed. Update the helper that creates the temp workspace so it returns the TempDir together with the Child, and keep both owned by the caller instead of dropping the directory manually. Then update the two websocket test call sites to hold on to the returned TempDir alongside the child process so cleanup happens automatically through normal scope drop.src/interpreter/mod.rs (2)
7315-7336: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: per-event receiver re-locking. Each dispatched event rebuilds
recv_futsand re-acquires every server'sevent_receiverlock beforeselect_all. This is O(servers) lock churn per event. Fine for a handful of servers, but if you expect high message throughput you could drain multiple ready events per poll (e.g. loop ontry_recvafter the select wakes) before rebuilding.🤖 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 `@src/interpreter/mod.rs` around lines 7315 - 7336, The event polling in the receiver loop rebuilds recv_futs and re-locks every server event_receiver on each dispatched event, causing O(servers) lock churn. Update the select_all flow in the receiver-handling block to drain additional ready events after one wakeup, preferably by looping on try_recv or similar before rebuilding the futures, while keeping dispatch_ws_event and the existing timeout behavior intact.
143-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
WsRegisteredHandlercaptures the environment with a strongRc, unlikeFunctionValue/EventHandler(which useWeak).Because the handler set lives in
web_socket_serversfor the whole program, the capturedenv(and everything it transitively holds) is retained for the program's lifetime rather than being released when the registering scope exits. This is harmless for top-level handlers but deviates from the codebase'sWeak-capture convention. ConsiderWeak+ upgrade-at-dispatch if you want handler lifetimes to track their defining scope.🤖 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 `@src/interpreter/mod.rs` around lines 143 - 147, WsRegisteredHandler is holding the captured Environment with a strong Rc, which keeps the defining scope alive for the whole program. Update WsRegisteredHandler to store a Weak reference like FunctionValue and EventHandler, then upgrade it at dispatch time in the websocket handler path; if the environment is gone, skip or fail the callback gracefully.src/typechecker/mod.rs (1)
2052-2072: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winServer operand type isn't statically validated for WebSocket handler/broadcast.
WebSocketHandlerStatement.serverandBroadcastWebSocketMessageStatement.serverare the same kind of server-key operand thatcheck_server_expression_typealready validates forWaitForRequestStatement,StopAcceptingConnectionsStatement, andCloseServerStatement(interpreter'sresolve_ws_server_keyrequiresText). These two arms only callinfer_expression_type, so a wrongly-typed server expression is caught only at runtime instead of statically, unlike its siblings.♻️ Proposed fix
Statement::WebSocketHandlerStatement { server, body, .. } => { // The server operand and handler body are checked; the handler's // bound variable resolves as an object at runtime (gradual typing // keeps member access like `content of msg` permissive). - self.infer_expression_type(server); + self.check_server_expression_type(server, *line, *column); for stmt in body { self.check_statement_types(stmt); } } Statement::SendWebSocketMessageStatement { message, target, .. } => { self.infer_expression_type(message); self.infer_expression_type(target); } Statement::BroadcastWebSocketMessageStatement { message, server, .. } => { self.infer_expression_type(message); - self.infer_expression_type(server); + self.check_server_expression_type(server, *line, *column); }Note:
WebSocketHandlerStatement's destructuring uses..forline/column; these would need to be bound explicitly to pass tocheck_server_expression_type.🤖 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 `@src/typechecker/mod.rs` around lines 2052 - 2072, The WebSocket server operand is only inferred, not validated, in `WebSocketHandlerStatement` and `BroadcastWebSocketMessageStatement`, unlike the other server-key statements. Update the `check_statement_types` match arms to call `check_server_expression_type` for `server` in both cases, using the existing `check_server_expression_type` helper and the `WebSocketHandlerStatement`/`BroadcastWebSocketMessageStatement` symbols to locate the change. For `WebSocketHandlerStatement`, bind the needed position fields explicitly instead of `..` so you can pass them into the checker consistently.
🤖 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 `@src/analyzer/mod.rs`:
- Around line 1704-1754: The WebSocket handler setup in
analyzer::analyze_statement is removing shared action_parameters entries without
tracking what was already present, which can delete enclosing action parameters
that are still valid. Update the Statement::WebSocketHandlerStatement branch to
remember whether binding and each ws_properties entry were already in
action_parameters before inserting them, then only remove the names that this
handler actually added after analyzing body. Keep the fix localized to the
handler-scoping logic so outer parameters remain available after the handler
finishes.
In `@src/interpreter/mod.rs`:
- Around line 7368-7372: The websocket handler binding setup in
dispatch_ws_event is using define, which can fail when conn/msg already exist in
an outer scope instead of shadowing them. Update the binding insertion on the
child Environment to use define_direct for the event binding so websocket
handlers behave consistently with the other handler bindings and do not bubble a
collision out as a RuntimeError.
In `@tests/websocket_test.rs`:
- Around line 138-139: The websocket broadcast test uses a fixed
tokio::time::sleep to wait for client b to register, which can race under CI
load. Update BROADCAST_PROGRAM to include a connect handler that emits a
readiness message, and change the test flow in websocket_test.rs to wait for
both clients to receive that message before a sends the broadcast. This removes
the timing dependency and makes the synchronization deterministic using the
existing connect/broadcast test setup.
---
Nitpick comments:
In `@src/interpreter/mod.rs`:
- Around line 7315-7336: The event polling in the receiver loop rebuilds
recv_futs and re-locks every server event_receiver on each dispatched event,
causing O(servers) lock churn. Update the select_all flow in the
receiver-handling block to drain additional ready events after one wakeup,
preferably by looping on try_recv or similar before rebuilding the futures,
while keeping dispatch_ws_event and the existing timeout behavior intact.
- Around line 143-147: WsRegisteredHandler is holding the captured Environment
with a strong Rc, which keeps the defining scope alive for the whole program.
Update WsRegisteredHandler to store a Weak reference like FunctionValue and
EventHandler, then upgrade it at dispatch time in the websocket handler path; if
the environment is gone, skip or fail the callback gracefully.
In `@src/typechecker/mod.rs`:
- Around line 2052-2072: The WebSocket server operand is only inferred, not
validated, in `WebSocketHandlerStatement` and
`BroadcastWebSocketMessageStatement`, unlike the other server-key statements.
Update the `check_statement_types` match arms to call
`check_server_expression_type` for `server` in both cases, using the existing
`check_server_expression_type` helper and the
`WebSocketHandlerStatement`/`BroadcastWebSocketMessageStatement` symbols to
locate the change. For `WebSocketHandlerStatement`, bind the needed position
fields explicitly instead of `..` so you can pass them into the checker
consistently.
In `@tests/websocket_test.rs`:
- Around line 97-152: Add missing WebSocket coverage in tests/websocket_test.rs
by extending the existing websocket test suite with a case that exercises the
`on websocket disconnect` handler and another that verifies `close server` shuts
down the WebSocket server and terminates active connections. Use the existing
helpers like `start_ws_server`, `connect`, and `next_text`, and place the new
assertions alongside `websocket_connect_and_echo` and
`websocket_broadcast_reaches_all_clients` so the new behavior is covered
end-to-end.
- Around line 25-27: The test helper is leaking the TempDir via
std::mem::forget(dir), which should be removed. Update the helper that creates
the temp workspace so it returns the TempDir together with the Child, and keep
both owned by the caller instead of dropping the directory manually. Then update
the two websocket test call sites to hold on to the returned TempDir alongside
the child process so cleanup happens automatically through normal scope drop.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fcece197-77e7-4c80-9350-a02b6933ceec
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
Cargo.tomlDev diary/2026-07-08-websockets-issue-555.mdDocs/04-advanced-features/web-servers.mdTestPrograms/web_server_websocket_test.wflTestPrograms/websocket_echo_server.wflsrc/analyzer/mod.rssrc/analyzer/static_analyzer.rssrc/interpreter/mod.rssrc/parser/ast.rssrc/parser/mod.rssrc/parser/stmt/web.rssrc/transpiler/javascript.rssrc/typechecker/mod.rstests/websocket_test.rs
…tests Correctness fixes from PR #593 review (Codex + CodeRabbit): - Interpreter: use `define_direct` for the handler event binding so a same-named outer variable (`store conn as ...` before `on websocket connect ... as conn`) is shadowed instead of aborting the handler with a redefinition error. - Analyzer: a handler property name (`id`/`ip`/`body`/`sender`) now resolves as an `of`-form property read even when an outer scope defines a non-function symbol of the same name (e.g. an earlier HTTP `wait for request` binds `body`), instead of failing analysis with "is not a function". - Analyzer: only remove the `action_parameters` entries this handler actually added, so a same-named enclosing action parameter survives the handler. - Typechecker: validate the server operand of `on websocket ...` and `broadcast websocket message ... to <server>` with `check_server_expression_type`, matching the other server-key statements. Tests: - Replace the fixed 200ms sleep in the broadcast test with a deterministic readiness handshake (per-client connect greeting) to avoid CI flakiness. - Add disconnect-handler and close-server end-to-end coverage. - Echo test now uses outer `conn`/`body` variables that collide with the handler binding and a property name, exercising the two shadowing fixes above. - Return the temp dir from the helper instead of leaking it with mem::forget. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Mp4HoighK1ALD1BxdMcBk
Add a mandatory rule to CLAUDE.md that any change altering user-facing behavior (syntax, keywords, statements, stdlib, CLI flags, config) must ship its documentation — guide page, keyword references, a validated example, and a Dev Diary note — in the same change. A feature is not complete until its docs are written. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Mp4HoighK1ALD1BxdMcBk
Implement real-time WebSocket servers with natural-language syntax:
listen for websockets on port 8080 as chat_server
on websocket connect to chat_server as conn:
send websocket message "Welcome!" to conn
end on
on websocket message from chat_server as msg:
broadcast websocket message body of msg to chat_server
end on
WebSockets reuse the HTTP server's single-threaded design: warp runs each
socket in background tokio tasks that push connect/message/disconnect events
over a channel, and the interpreter dispatches to the registered
on websocket ...handler blocks while the program is inside await.Changes:
broadcast statements; no new keywords (kept
messageas an identifier).into
wait for <duration>, handler dispatch, send/broadcast;close servernow also closes a WebSocket server and its connections.
property of objectaccess (body of msg,method of request): itparsed as a call and errored at runtime; added an object-field fallback in
FunctionCall evaluation so natural property reads resolve.
broadcast) and TestPrograms/websocket_echo_server.wfl (CI-safe).
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_019Mp4HoighK1ALD1BxdMcBk
Summary by CodeRabbit
New Features
Bug Fixes
waitperiods.Documentation
Tests