[wcy12378]

Fixes #421

Problem

The default local WebSocket server spawned a task per TCP connection with no cap, and accept_hdr_async had no timeout: a local client that opens a TCP connection and stalls before (or during) the HTTP Upgrade pinned a tokio task and a socket FD indefinitely. Opening many such connections exhausted daemon tasks and FDs.

Change (crates/bsk-cli/src/daemon/ws.rs)

• MAX_LOCAL_WS_CONNECTIONS = 64: the accept loop holds a tokio::sync::Semaphore; at the cap the fresh TCP stream is closed immediately instead of queueing another task. The permit moves into the connection task, so the slot is released exactly when the connection ends — client close, upgrade timeout, or first-frame timeout.
• WS_UPGRADE_TIMEOUT = 5s: accept_hdr_async runs under tokio::time::timeout, aligned with the existing HANDSHAKE_FIRST_FRAME_TIMEOUT for the post-upgrade phase. A stalled upgrade now logs and releases its slot like any dead connection.
• The remote (server-mode) bind path is untouched — the issue scopes this to the default local implementation.

Validation

Windows x64, cargo 1.96 (CARGO_INCREMENTAL=0):

• cargo test -p bsk --lib ws_connection_limit_tests → 2 passed
  • ws_upgrade_times_out_for_silent_clients: a real TCP client that connects and sends nothing is dropped by the daemon within WS_UPGRADE_TIMEOUT + slack
  • ws_connection_limit_rejects_then_releases_slots: 64 concurrent upgraded clients (valid extension origin) hold every slot; connection feat(install-skill): add Kimi Code harness support #65 is closed promptly; after all clients close, a fresh client is accepted again (slot released)
• cargo test -p bsk --lib → 433 passed, 2 ignored (full suite, no regressions)
• cargo clippy -p bsk --lib -- -D warnings: the 3 reported errors are pre-existing on this base (file_transfer.rs unused vars, harness.rs unneeded return) and untouched by this change; the new code is clean

[wcy12378]

CI failure root cause: cargo fmt --all -- --check rejected three hand-formatted spots in the new ws_connection_limit_tests module in crates/bsk-cli/src/daemon/ws.rs (the multi-line into_client_request() chain, the tokio::time::timeout block layout, and a one-line assert! that needed wrapping). The fmt step exited before clippy/tests ran.

Fix (commit 82a2e27):

• Ran cargo fmt --all to normalize the three spots.
• Removed the unused futures_util::SinkExt import and the unused url binding in ws_upgrade_times_out_for_silent_clients that clippy -D warnings flags.

No production code changes — everything is inside the #[cfg(test)] module. Verified locally: cargo fmt --check clean, cargo clippy no longer reports the two test-module errors, cargo test -p bsk --lib passes 433 tests including both new ws connection-limit tests.

[iuyo5678]

Thanks for addressing #421. The approach looks appropriate for the local server: acquire a connection permit before spawning the task, retain it for the full connection lifetime, and bound the HTTP Upgrade with a timeout. This should improve daemon resilience to stalled clients without changing normal browser or CLI workflows.

Reviewed commit 82a2e27. I did not find any blocking implementation issues. The permit lifetime covers both the upgrade and application-handshake phases, and the remote server path remains outside the scope of this change.

Two non-blocking suggestions:

1. Tighten the regression tests. The over-cap rejection check currently allows 5 seconds, which is also the upgrade timeout, and the reconnect check allows 10 seconds. A shorter deadline, clearly below the handshake timeout, would better distinguish immediate rejection/release from timeout-driven cleanup. It would also be useful to cover incomplete HTTP headers, closing one fully registered connection while the other 63 remain connected, and recovering the entire pool after upgrade or first-frame timeouts.
2. Rate-limit the capacity warning. Logging a warning for every rejected connection can produce substantial log volume under a sustained connection flood. The existing throttling in daemon/remote/server.rs would be a useful precedent.

Validation on macOS:

• All 17 WebSocket unit tests passed on the PR head.
• After applying the patch to main at b17abe9, all 18 selected integration tests passed, including three additional local boundary checks for incomplete upgrades, connection-slot reuse, and first-frame timeout cleanup.

The remaining CI failure is the video encoder pixel assertion (expected 254 to be less than 15). The failing encoder test does not exercise this local WebSocket server, so I have not found evidence linking it to this change. Please rerun that check and investigate if it persists before merging.

The implementation looks ready to merge once CI is resolved and the required reviews are satisfied. The two suggestions above can be handled as follow-ups.

[wcy12378]

The only failing check after 82a2e27, Browser video recording regression, was a flake unrelated to this PR: that commit only touches #[cfg(test)] code in daemon/ws.rs, which is not compiled into the shipped binary, and the same job passed on the previous head (6e51387), on main, and on other PRs the same day. The failure was a pixel assertion in encoder.browser.test.ts (the blue bar sampled as white, 254 instead of <15), a known browser-rendering flake on CI runners. Since rerunning the job requires admin rights, be3c9f3 is an empty commit to retrigger the workflow; the new run is fully green.