fix: send session_end notification before wipe/close (#6)#14
Conversation
- Add 'end' wire message type sent to peer before shutdown - Handle incoming 'end' messages in receive_loop to cleanly close session - Both close_session and panic_wipe now notify peer before destroying state - Peer receives session_closed event with reason='peer_ended' Co-Authored-By: Oz <oz-agent@warp.dev>
rafabd1
force-pushed
the
fix/issue-6-wipe-session-notification
branch
from
a798b96 to
4f6f82b
Compare
|
@codex review |
![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: 4f6f82b5ed
ℹ️ 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".
| if let Some(mut s) = sess.take() { | ||
| // Notify peer that session is ending before destroying everything | ||
| let end_msg = serde_json::to_vec(&SessionEnd { t: "end".into() }).unwrap_or_default(); | ||
| let _ = write_framed(&mut s.stream_writer, &end_msg).await; |
There was a problem hiding this comment.
do_panic_wipe now awaits write_framed before clearing session/identity state, so a stalled or backpressured socket can delay the wipe indefinitely (the write path uses write_all/flush and has no timeout). In the panic-wipe scenario this is a correctness and safety regression: local sensitive state is kept alive longer specifically when the peer/network is unhealthy. Make the end notification non-blocking (e.g., bounded timeout or spawned best-effort task) and perform local wipe immediately.
Useful? React with 👍 / 👎.
1 participant
Fix: Send session_end notification before wipe/close
Closes #6
Problem
When a user triggers
panic_wipe()orclose_session(), the local session is shut down but the peer receives no protocol-level notification. This creates an asymmetric state where one side thinks the session is active while the other has already destroyed it.Solution
"end"wire message type to the protocolclose_sessionanddo_panic_wipenow send a{"t":"end"}frame to the peer before shutting down the stream writerreceive_loop/handle_incoming_messagenow recognizes"end"messages and:session_closedwithreason: "peer_ended"to the frontendSecurity considerations
"end"message is sent as a best-effort signal — if the I2P tunnel is already broken, the send may fail silently, which is acceptable since the stream will eventually error out on the peer side"end"frameThis PR was generated with Oz.