ggsql-jupyter's heartbeat socket can panic the whole kernel (zeromq RepSocket, not yet implemented)

[samclark2015]

ggsql-jupyter's heartbeat socket can panic the whole kernel process (zeromq crate RepSocket, not yet implemented)

What happens

While verifying the Python protocol test fixes for posit-dev/positron#15512 (running ggsql-jupyter/tests/test_compliance.py and test_integration.py locally, several times in a row), the kernel process itself crashed with a Rust panic, not a test failure:

thread 'main' panicked at /Users/sclark/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/zeromq-0.4.1/src/rep.rs:168:40:
not yet implemented

This is inside the vendored zeromq crate's RepSocket::recv() — the socket type ggsql-jupyter uses for the heartbeat channel (ggsql-jupyter/src/kernel.rs:30,85). The panic is not caught anywhere: kernel.rs's main select! loop only guards the Ok case —

msg = self.heartbeat.recv() => {
    // Echo heartbeat
    if let Ok(msg) = msg {
        self.heartbeat.send(msg).await?;
    }
}

(ggsql-jupyter/src/kernel.rs:203-209)

— but a panic inside recv() isn't a Result::Err the if let can intercept; it unwinds the whole process. Every open comm, the whole session, and (for PositronConsole sessions) the plot pane's connection all die with it.

Reproducing

Locally, on this branch (sclark/posit-dev-positron-15512), running the compliance suite repeatedly:

cd ggsql-jupyter/tests
source .venv/bin/activate  # after: python -m venv .venv && pip install -r requirements.txt
python -u -m pytest test_compliance.py -v -s

Hit this on 3 of 4 consecutive runs, always within ~100ms of kernel startup — right after jupyter_client's wait_for_ready() sends its characteristic burst of repeated kernel_info_requests (visible in the kernel's own log as 4-6 Received shell message: kernel_info_request / Sending kernel_info_reply pairs within under 2ms of each other; this is normal jupyter_client behavior — its wait_for_ready loop resends kernel_info() whenever the IOPub check inside it comes up empty, docs/source in jupyter_client/client.py's _async_wait_for_ready).

Working hypothesis

BlockingKernelClient.start_channels() starts a live heartbeat channel (HBChannel) alongside the shell channel by default. If the kernel's single-threaded async event loop is busy answering that shell burst, a heartbeat ping's reply can be delayed past the client's own heartbeat timeout, which reconnects/resends. Two heartbeat requests arriving before the first is answered would violate the strict request-reply lockstep a zeromq RepSocket assumes — and this crate's RepSocket::recv() (zeromq-0.4.1/src/rep.rs:158-169) hits an unimplemented branch (_ => todo!()) for anything other than the single expected message shape, rather than erroring gracefully:

match message {
    Message::Message(mut m) => { /* ... */ }
    _ => todo!(),
},
Some((_peer_id, _)) => todo!(),

This is consistent with ggsql-jupyter/tests/test_integration.py never hitting it across 28 kernel launches in my testing (each kernel's lifetime is short: one test, then torn down) versus test_compliance.py hitting it repeatedly (one kernel process, shared across many test methods via jkt.KernelTests.setUpClass, so much more heartbeat-channel exposure time per process). Not fully root-caused — I did not instrument the actual wire traffic to confirm the double-ping, so treat the above as a strong lead, not a confirmed diagnosis.

Why this matters now

#15512 fixes the stale/hanging Python protocol tests and wires them into CI. That CI job will be flaky until this is fixed — test_compliance.py in particular starts one kernel process per test class and keeps its heartbeat channel alive throughout, which is exactly the exposure window this seems to need.

Suggested next step

Either:

• Harden ggsql-jupyter's heartbeat handling against this — e.g. don't hold a bare RepSocket open to naive misuse; catch/recover from a bad frame rather than crash, or model heartbeat differently (a raw echo doesn't need zeromq's REQ/REP framing semantics at all).
• Or determine whether a newer zeromq crate release has fixed this todo!() and bump the dependency.

Happy to help scope whichever direction once someone's had a chance to weigh in — flagging now mainly so the flakiness in the new CI job (#15512) has a paper trail and isn't mistaken for a fresh regression.