# Fix IPC framing, leaks and lifecycle bugs found by multi-model review \(\#6\) ยท lucenx/signaltty [View on GitCafe](https://git.cafe/lucenx/signaltty/commit/a8e1afd99eb46f8cd2f9c2ccffbc3f35a057748a) Repository: [lucenx/signaltty](https://git.cafe/lucenx/signaltty) Visibility: public Requested revision: a8e1afd99eb46f8cd2f9c2ccffbc3f35a057748a Requested commit: a8e1afd99eb46f8cd2f9c2ccffbc3f35a057748a Commit: a8e1afd99eb46f8cd2f9c2ccffbc3f35a057748a Tree: da214da8dd6d865bab40e96ec387d073e1c70b3f Author: Simone Committer: GitHub ## Message ``` Fix IPC framing, leaks and lifecycle bugs found by multi-model review (#6) * ipc: keep partly received requests across event wakeups The connection loop raced `read_line` against the broadcast receiver in `select!`. `read_line` is not cancel-safe: tokio moves the String into the future, so when any event won the race mid-line the received head was dropped and the tail answered with BAD_PARAMS "invalid JSON". Any request larger than one socket read, or written in pieces, could hit it on a busy server. Use `Lines::next_line`, which is cancel-safe. Co-Authored-By: Claude Opus 5.5 (1M context) * cli: keep partly received events in attach `attach` raced `read_line` against stdin in `select!`; a keystroke arriving while a large `pty.data` line was half read dropped its head, and the tail failed JSON parsing, aborting the attach. Use the cancel-safe `Lines::next_line`. No test seam: the loop reads the real tty stdin; the server regression test covers the same mechanism. Co-Authored-By: Claude Opus 5.5 (1M context) * ipc: reject unknown signal names before closing panes `pane.close`/`tab.close`/`workspace.close` with an unknown `signal` ignored the parse error from `PtyManager::signal`, removed the pane and its PTY handle anyway, and left the child running with no pane to reach it until the server exited. `pane.signal` reported the same typo as IO_ERROR. Validate the name while decoding params so every method answers BAD_PARAMS and nothing is torn down. Found by a Muse Spark 1.3 review pass; confirmed with an integration test that counts the child in /proc after close. Co-Authored-By: Claude Opus 5.5 (1M context) * term: resume scanning after an aborted OSC `find_osc_end` reported a new `ESC ]` the same way as an incomplete sequence, so `push` carried the aborted OSC forward and rescanned it on every later chunk. One interrupted title write starved every following OSC (notifications included) until 8 KiB of output overflowed the carry buffer and dropped it all. Distinguish the abort and restart from the new OSC. Found by a Muse Spark 1.3 review pass. Co-Authored-By: Claude Opus 5.5 (1M context) * term: keep UTF-8 split across PTY reads in scrollback `Surface::feed` decoded each read with `from_utf8_lossy`, so a multibyte character cut by the 8 KiB read boundary was stored as two U+FFFD in `pane.read` tail output while the screen stayed correct. Carry an incomplete trailing sequence (at most 3 bytes) to the next feed. Found by a Muse Spark 1.3 review pass. Co-Authored-By: Claude Opus 5.5 (1M context) * ipc: check the pane before resizing on attach `pane.attach` with `cols`/`rows` emitted (and audited) `pane.resized` before checking the pane existed, so a missing id produced a phantom event alongside NO_SUCH_PANE. A real resize was never marked for persistence, and its payload was `{pane_id}` while `pane.resize` sends `{pane}`. Resolve the pane first and emit the same payload as `pane.resize`. Found by a Muse Spark 1.3 review pass. Co-Authored-By: Claude Opus 5.5 (1M context) * ipc: pick workspace handles under the insert lock `workspace.create` chose a handle under a read lock, ran `git_info`, then inserted under a separate write lock. Concurrent creates with the same name all saw the handle free: 8 parallel `api` creates produced 2-3 distinct handles, leaving the rest unreachable by handle. Choose the handle and insert under one write lock, as the worktree path already does. Found by a Muse Spark 1.3 review pass. Co-Authored-By: Claude Opus 5.5 (1M context) * ipc: carry decision_id when a failed answer clears its gate When the child exits between consuming a decision and writing the answer bytes, `decision.answer` emitted `decision.cleared` without the `decision_id` that docs/08-ipc.md promises, so subscribers could not tell which gate dropped. Found by a Muse Spark 1.3 review pass. No deterministic seam: the child must exit between two adjacent calls. Co-Authored-By: Claude Opus 5.5 (1M context) * agents: name the tool in Codex approval notifications Codex `PermissionRequest` payloads carry `tool_name` (the shared native permission contract reads it), but the notification looked up `tool`, so every "Codex needs approval" toast had an empty body. Found by a Gemini 3.8 Flash review pass. Co-Authored-By: Claude Opus 5.5 (1M context) * agents: clear OpenCode attention when a new turn starts `session.status busy` moved the pane to working but left attention untouched, so a pane that finished a turn (done + unread) or failed (error) kept that badge while the next turn ran. Clear it, as the Claude and Codex adapters do on UserPromptSubmit. Found by a Gemini 3.8 Flash review pass. Co-Authored-By: Claude Opus 5.5 (1M context) * cli: end attach when its pane exits `pane attach` waited for `pane.exited`, but the server only streams `pty.data` to an attached connection, so the event never arrived and attach sat on a dead pane until Ctrl+]. Two more problems stayed hidden behind that: any other pane's exit would have ended the session, and once `run` returned, the pending tokio stdin read held runtime shutdown until the next keypress. Subscribe to `pane.exited` before attaching, match the pane id, return at once for an already-exited pane, and exit the process explicitly after a successful command. Found by a Gemini 3.8 Flash review pass. No in-repo seam (needs a real tty); verified with a `script`-driven loop: before, still attached after 10s; after, exits 3.0s after a `sleep 3` child, unaffected by another pane exiting at 1s. Co-Authored-By: Claude Opus 5.5 (1M context) * gui: free closed panes' terminals The VTE `commit` and `has-focus` handlers captured a strong `Rc` while the widget owns that terminal, a reference cycle: every closed pane kept its PaneWidget, VTE terminal and scrollback alive for the life of the GUI. Capture weak references, as the pane's other handlers already do. Found by a Gemini 3.8 Flash review pass. Co-Authored-By: Claude Opus 5.5 (1M context) * gui: keep a divider drag made while the previous send is in flight `send_succeeded` cleared the pending drag unconditionally. Dragging again before the first `tab.set_ratio` answered dropped the second drag: the server kept the first ratio while the divider showed the second. Clear the queue only when it still holds the ratio that landed. Surfaced while checking a Gemini 3.8 Flash finding about divider resends; the resends themselves are idempotent and left as is. Co-Authored-By: Claude Opus 5.5 (1M context) * docs: Document the optional signal parameter for tab.close --------- Co-authored-by: Lucenx9 <185146821+Lucenx9@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 (1M context) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> ``` ## Parents - [60f9af6f80e09840e1738fc2edfeebd7ccfa0128](https://git.cafe/lucenx/signaltty/commit/60f9af6f80e09840e1738fc2edfeebd7ccfa0128?format=markdown) [Source at this commit](https://git.cafe/lucenx/signaltty/tree/a8e1afd99eb46f8cd2f9c2ccffbc3f35a057748a?format=markdown)