Per-user local IPC transport, atomic recovery, daemon lifecycle (from #513) - #519
Closed
mgth wants to merge 4 commits into
Closed
Per-user local IPC transport, atomic recovery, daemon lifecycle (from #513)#519mgth wants to merge 4 commits into
mgth wants to merge 4 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…513) Adopts the transport rework from #513 (commits 3fdf340/6641ae5), reviewed fix by fix and rebased on the master elevation and exclusion models. Why the TCP transport had to go: port 25196 was open to every local process (any user could command the daemon or read FocusChanged paths), a squatted port made the daemon panic at boot, the client dispatched events out of order via Task.Run, swallowed send failures after 10 retries (a Stop could silently leave the hook capturing the mouse), and matched events by substring inside payloads. The replacement: a per-session named pipe with a user/SYSTEM DACL and session check on Windows, a 0600 Unix socket in XDG_RUNTIME_DIR on Linux; u32-length-prefixed UTF-8 frames capped at 1 MiB; bounded queues; a serialized command worker preserving Load-then-Run order; four-client cap. Same XML message contract - settings and Current.xml stay compatible; DaemonPort remains as a dead property so existing settings deserialize. Current.xml is now written atomically with a .bak the daemon falls back to only when the primary is unreadable or fails to parse - a valid stopped state no longer triggers recovery, and a Run without a successful Load is ignored. Stop gets a safety net: if IPC is down, known hook processes of this session (this UI's child on Unix) are killed rather than left routing input. On top of the #513 version, three of its bugs are fixed here: - the Windows accept loop died permanently on a transient create_pipe error; it now logs, backs off 500 ms and retries - the listener raised ConnectionFailed only once per connect cycle, so a daemon that crashed right after launch was never relaunched; it now re-raises every ~5 s (LaunchDaemon's already-running check makes that idempotent) - UnauthorizedAccessException or a malformed frame killed the event listener silently; both now reconnect like I/O failures And two of its regressions are not taken: the resume watchdog stays ungated (the #513 gating reintroduced the stale-layout-on-wake bug), and daemon launch keeps the #512 elevation model (no runas). The Unix bind also no longer chmods a parent directory it did not create (EPERM on shared parents like /tmp). LBM_HOOK_ENDPOINT replaces LBM_HOOK_PORT for side-by-side testing. The C++ LittleBigMouse.Hook daemon, which only speaks TCP, is retired from distributable builds (decision from the #513 review). Wire contract covered by tests on both sides, including full-duplex under load (events streaming while commands flow) and listener reconnection - the historical pipe failure modes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: MathieuARS <mathieu.maik.15@gmail.com>
Two review findings on the local IPC client: - A frame that is not valid UTF-8 threw DecoderFallbackException, which the listener's catch filter does not list: the listener died silently, forever. Surface it as InvalidDataException like an oversized frame so the reconnect path handles it. - The daemon honours LBM_HOOK_ENDPOINT but the UI client did not: setting the documented side-by-side testing override left the launched daemon bound to the test endpoint while the UI polled the production name — permanent Dead. The client now reads the variable (full pipe path on Windows, socket path elsewhere), and the README documents the full-path format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…out-Run - The zone serializer wrote attribute values raw: a monitor name containing & or a quote produced XML that the new recovery-file validation rejects (failing Start after the engine already started) and that the daemon parser could never read. Escape attribute values; roxmltree unescapes them on the daemon side. - Only Load batches rewrote Current.xml, so a user Stop left Load+Run in the file and a standalone (autostart) daemon re-hooked the mouse at the next boot despite the Stop. Strip the Run line on Stop: Load-without-Run is exactly the persisted stopped state the daemon replay already honours (valid_stopped_primary_does_not_fall_back_to_backup). Stop stays a safety operation — persistence failures are logged, never faulted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Landed on master as a0f3110..6d31774 (fast-forward of the rebased branch — byte-identical to a rebase merge). The PR was auto-closed when its stacked base branch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third extraction from #513 (stacked on #518): the transport rework, reviewed fix by fix, with three of its bugs fixed and two of its regressions left out.
Why replace the TCP transport
Port 25196 was open to every local process (commands + FocusChanged paths readable by anyone, port squatting = daemon panic at boot), the old client dispatched events out of order (
Task.Run), swallowed send failures after 10 retries — a Stop could silently leave the hook capturing the mouse — and matched events by substring inside payloads.What this brings
ipc/server.rs, tokio): per-session named pipe with user/SYSTEM DACL + session check on Windows; 0600 UDS in$XDG_RUNTIME_DIRon Linux. Bounded queues, 4-client cap, serialized command worker (Load→Run order), broadcast that never blocks the hook thread.ipc/framing.rs/LocalIpcClient): u32-LE length prefix + UTF-8, 1 MiB cap, both sides tested.DaemonMessage.TryParse): real XML parse, DTD refused, replacesargs.Contains("Stopped").&used to produce invalid XML).AtomicRecoveryFile(write-temp-fsync-replace) +.bakfallback in the daemon — only on unreadable/unparsable primary; a valid stopped state is authoritative, and Run without a successful Load is ignored.LaunchDaemon, ordered UI-thread event dispatch with exception isolation, tray Start persistsEnabled=true.Fixed on top of #513
create_pipeerror (log + 500 ms backoff + retry)ConnectionFailedre-raises every ~5 s instead of once — a daemon that crashes right after launch gets relaunched (idempotent thanks to the already-running check)UnauthorizedAccessException/ malformed frame no longer kill the event listener silently/tmp)Deliberately NOT taken from #513
MainServiceresume-watchdog gating (would reintroduce the stale-layout-on-wake bug the watchdog fixes)DaemonLaunchPolicy/runas (conflicts with the 5.4.1.0 does not work #512 elevation model — the daemon inherits the UI's level)Acted decisions
LittleBigMouse.Hook(TCP-only) is retired from distributable builds.LBM_HOOK_ENDPOINTreplacesLBM_HOOK_PORTfor side-by-side dev testing.Tested
DaemonProtocolTests) + 6 UI tests, full UI build on Linux🤖 Generated with Claude Code