fix(gateway): wake WebChat writes on the lws service thread

[adriannoes]

Summary

Consolidates the cursor critical-bug fix originally in cursor/critical-bug-identification-2a2f (PR #127) as an operator-authored commit.

• fix(gateway): wake WebChat writes on the lws service thread — lws_callback_on_writable updates the poll set without the per-thread lock; replies and provider-status broadcasts called it from the agent thread while lws_service ran on the gateway thread, racing pollfd updates against a closing wsi. Frames are now queued, the service wait is cancelled, and writable is marked only from EVENT_WAIT_CANCELLED.

Rebased on top of #119 (merged): the shared chunk of ws_send_to/ws_broadcast_text was fused manually so the closed-socket drops from #119 and the service-thread wake both hold; test_ws runs all regression sets from both fixes.

Testing

• make test_ws — all tests passed (drops + wake tests together)
• CI=true make test — all 45 test binaries passed
• make static — passed

Supersedes #127.

[cursor]

Summary

This moves WebChat wakeups off the agent thread: ws_send_to and ws_broadcast_text enqueue, call lws_cancel_service, and lws_callback_on_writable runs only from LWS_CALLBACK_EVENT_WAIT_CANCELLED on the service thread. The approach is correct for libwebsockets (lws_cancel_service is the thread-safe wake, and the cancel event reaches the single ws protocol once per cancel). The #119 drop-on-close behavior is preserved. I ran make test_ws LDLIBS=-lm locally and all tests pass (the stock target needs -lcurl, which this image lacks). No blockers, but the shutdown guard does not fully close the race it claims to.

Must Fix

None.

Should Fix

1. src/gateway/ws.c ws_wake_service (lines 93-102) reads g_lws_ctx under the mutex, unlocks, then calls lws_cancel_service(ctx). The comment in http.c says the ws_set_context(NULL) before destroy stops a late send from cancelling a freed context, but a sender that copied the pointer just before http_stop clears it still calls lws_cancel_service on a destroyed context. lws_cancel_service only writes to the wake fd and never takes g_mutex, so calling it while holding g_mutex is safe and makes the NULL-out an actual barrier (ws_on_service_cancel takes the mutex on the service thread, not inside lws_cancel_service, so there is no inversion). In practice senders are the main thread and http_stop runs from cleanup, so the window is narrow, but the code claims to close it.
2. src/gateway/http.c lines 143-146: ws_set_context(ctx->lws_ctx) runs before pthread_create. If pthread_create fails, the context is destroyed without ws_set_context(NULL), leaving a dangling g_lws_ctx. Clear it on that failure path, or move ws_set_context after a successful pthread_create.
3. Test gap: all new tests use a stub lws_cancel_service/lws_callback_on_writable in SHELLCLAW_WS_TEST, so the claim that the real EVENT_WAIT_CANCELLED path flushes frames is not exercised. test_gateway_http has no WebSocket send round trip. A GATEWAY=1 test that connects a WS client, calls ws_send_to/ws_broadcast_text from the main thread and reads the frame would lock the actual fix. Also missing: a test that a conn with no pending frames gets no writable call when another conn does (the second ws_on_service_cancel in test_send_wakes_service_thread only covers the drained case), and a test that a dropped send (no live wsi) does not cancel.

Nice to Have

• src/gateway/ws.c line 408 (ws_enqueue_outgoing_locked(m);) and tests/test_ws.c line 414 (RUN(test_unregister_drops_pending_outgoing());) lost their indentation in the rebase. Restore the tab so formatting and git blame stay clean.
• ws_on_service_cancel is O(conns x queue) under the mutex (16 x 64 max), fine today. If MAX_QUEUE grows, a per-conn pending flag set at enqueue would avoid the nested scan.
• ws.h ws_send_to docs could state that delivery is asynchronous via the service thread, in addition to the existing note about return 0 on a dropped send.
• The PR targets development; AGENTS.md says main is the active line. Fine if intentional, just confirm.

Positive Highlights

• The fix addresses the real cause (poll-set update without the per-thread lock) instead of adding locking around lws_callback_on_writable, and the ws.h comment explains why the call is thread-restricted.
• Broadcast now wakes once instead of once per connection, and ws_on_service_cancel copies wsi pointers out under the lock and calls lws after unlocking, so no lws call happens with g_mutex held. Unregister runs on the same service thread, so those pointers cannot go stale.
• CHANGELOG.md is updated and the tests assert both counters (cancel on the sender side, writable only after the hook), which pins the thread contract.

  

_{Sent by Cursor Automation: Code review}

[cursor]

Should Fix: ctx is copied under the mutex but used after unlock. http_stop can clear it and lws_context_destroy between the copy and this call, so the NULL-out in http.c is not a barrier. Call lws_cancel_service(ctx) while still holding g_mutex (it only writes the wake fd, no re-entry into g_mutex).

[cursor]

Nit: lost its leading tab in the rebase.

[cursor]

Should Fix: the pthread_create failure path in http_start (around line 146) destroys the lws context without ws_set_context(NULL), leaving g_lws_ctx dangling. Clear it there too.