Skip to content

Propagate client state changes to clients#181

Open
whme wants to merge 1 commit intoissue/179from
propagate-client-state
Open

Propagate client state changes to clients#181
whme wants to merge 1 commit intoissue/179from
propagate-client-state

Conversation

@whme
Copy link
Copy Markdown
Owner

@whme whme commented Apr 26, 2026

Introduce a per-client state channel that the daemon's pipe server
task subscribes to, replacing the previous PipeServerState mutex.
Each client's connection now multiplexes input records, state-change
notifications, and keep-alives via tokio::select!, sending a
TAG_STATE_CHANGE envelope on every transition.

The wire protocol gains a ClientState enum (currently a single
Active variant) carried as a one-byte payload after TAG_STATE_CHANGE.
The client tracks the last received state in its read/write loop so
future PRs can act on additional variants without further protocol
changes.

This PR is plumbing only — no control-mode bindings or visual
changes are wired up yet. Daemon::set_client_state is added as a
private helper for upcoming work.

GitHub: #179
Co-authored-by: Claude Opus 4.7 noreply@anthropic.com

Introduce a per-client state channel that the daemon's pipe server
task subscribes to, replacing the previous PipeServerState mutex.
Each client's connection now multiplexes input records, state-change
notifications, and keep-alives via tokio::select!, sending a
TAG_STATE_CHANGE envelope on every transition.

The wire protocol gains a ClientState enum (currently a single
Active variant) carried as a one-byte payload after TAG_STATE_CHANGE.
The client tracks the last received state in its read/write loop so
future PRs can act on additional variants without further protocol
changes.

This PR is plumbing only — no control-mode bindings or visual
changes are wired up yet. Daemon::set_client_state is added as a
private helper for upcoming work.

GitHub: #179
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end plumbing for propagating per-client state changes (daemon → client) over the existing named-pipe protocol, enabling the client main loop to track its authoritative state for future UI/control-mode work (Issue #179).

Changes:

  • Introduces ClientState and a new TAG_STATE_CHANGE framed message on the wire.
  • Updates daemon pipe server routine to multiplex input records, state updates, and keep-alives via tokio::select!.
  • Updates client read/write loop to apply incoming state-change messages and adds unit/integration tests for serialization/parsing and pipe behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/protocol.rs Adds ClientState, TAG_STATE_CHANGE, and framed-length constant; extends daemon→client message enum.
src/serde/serialization.rs Adds serialize_client_state and state-change envelope serialization.
src/serde/deserialization.rs Adds deserialize_client_state and parsing support for TAG_STATE_CHANGE.
src/daemon/mod.rs Replaces per-client mutex state with watch channel; multiplexes pipe outputs; adds write helper.
src/client/mod.rs Tracks and updates ClientState in the main loop via StateChange messages.
src/tests/serde/test_mod.rs Adds round-trip tests for ClientState and framed state-change message.
src/tests/daemon/test_mod.rs Updates client fixtures for watch sender and adds state-change forwarding test.
src/tests/client/test_mod.rs Adds test ensuring client loop applies state changes and still forwards input records.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/daemon/mod.rs
Comment on lines +1144 to +1163
async fn write_framed_message(server: &NamedPipeServer, frame: &[u8]) -> bool {
loop {
server.writable().await.unwrap_or_else(|err| {
error!("{}", err);
panic!("Timed out waiting for named pipe server to become writable",)
});
match server.try_write(frame) {
Ok(n) if n == frame.len() => {
debug!("Successfully written all data");
return true;
}
Ok(n) => {
// The data was only written partially, try again
warn!(
"Partially written data, expected {} but only wrote {}",
frame.len(),
n
);
continue;
}
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

write_framed_message retries partial writes by calling try_write(frame) again with the full frame. If try_write ever returns Ok(n) where n < frame.len(), the next iteration will resend the already-written prefix, corrupting the byte stream (and can also lead to an infinite loop if the pipe keeps accepting only a subset). Track an offset and retry with &frame[written..] until fully drained.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants