diff --git a/.pql/pql-plan.json b/.pql/pql-plan.json index 0a437ea8..3c62785e 100644 --- a/.pql/pql-plan.json +++ b/.pql/pql-plan.json @@ -1,5 +1,5 @@ { - "exported_at": "2026-05-05T12:51:04Z", + "exported_at": "2026-05-05T12:59:14Z", "decisions": [ { "id": "D-1", @@ -2007,11 +2007,11 @@ "id": "T-18", "type": "task", "title": "audit error handling in PTY and IPC critical paths", - "status": "ready", + "status": "done", "priority": "medium", "decision_ref": "D-5", "created_at": "2026-04-22 13:26:29", - "updated_at": "2026-05-03 20:37:30" + "updated_at": "2026-05-05 12:59:05" }, { "id": "T-19", @@ -2291,11 +2291,11 @@ "parent_id": "T-3", "title": "Ship clide tmux.conf for Claude pane sessions", "description": "Bundle a clide-specific tmux.conf (no status bar, large scrollback, zero escape delay, 256color passthrough). Claude pane spawn passes -f to use it. Includes bottom-anchoring via initial escape sequence or shell profile trick after attach.", - "status": "in_progress", + "status": "done", "priority": "high", "decision_ref": "D-41", "created_at": "2026-04-22 22:01:13", - "updated_at": "2026-05-03 20:33:10" + "updated_at": "2026-05-05 12:51:24" }, { "id": "T-44", @@ -2538,6 +2538,132 @@ "priority": "medium", "created_at": "2026-04-24 06:34:16", "updated_at": "2026-05-03 19:26:04" + }, + { + "id": "T-68", + "type": "task", + "title": "inline terminal emulator (drop xterm.dart dependency)", + "status": "done", + "priority": "medium", + "created_at": "2026-05-05 12:52:48", + "updated_at": "2026-05-05 12:53:15" + }, + { + "id": "T-69", + "type": "story", + "title": "Claude pane: fullscreen mode + mouse-wheel scroll", + "status": "done", + "priority": "medium", + "created_at": "2026-05-05 12:52:52", + "updated_at": "2026-05-05 12:53:15" + }, + { + "id": "T-70", + "type": "bug", + "title": "Bold text breaks terminal cell grid (workaround: suppress bold)", + "status": "done", + "priority": "medium", + "created_at": "2026-05-05 12:52:56", + "updated_at": "2026-05-05 12:53:15" + }, + { + "id": "T-71", + "type": "task", + "title": "PTY read buffer: 4KB → 64KB", + "status": "done", + "priority": "medium", + "created_at": "2026-05-05 12:53:01", + "updated_at": "2026-05-05 12:53:15" + }, + { + "id": "T-72", + "type": "task", + "title": "tmux: isolated -L clide socket so user's tmux config doesn't bleed into clide", + "status": "done", + "priority": "medium", + "created_at": "2026-05-05 12:53:05", + "updated_at": "2026-05-05 12:53:15" + }, + { + "id": "T-73", + "type": "task", + "title": "ship matching bold weight to restore semantic bold rendering", + "status": "backlog", + "priority": "medium", + "created_at": "2026-05-05 12:53:21", + "updated_at": "2026-05-05 12:53:21" + }, + { + "id": "T-74", + "type": "task", + "title": "forward real mouse events to TUI apps (not just PgUp/PgDown)", + "status": "backlog", + "priority": "medium", + "created_at": "2026-05-05 12:53:22", + "updated_at": "2026-05-05 12:53:22" + }, + { + "id": "T-75", + "type": "task", + "title": "PTY: surface errno from forkpty/execve/write/ioctl failures", + "status": "backlog", + "priority": "high", + "created_at": "2026-05-05 12:58:59", + "updated_at": "2026-05-05 12:58:59" + }, + { + "id": "T-76", + "type": "task", + "title": "PTY: fix resource leaks and reader-isolate races", + "status": "backlog", + "priority": "high", + "created_at": "2026-05-05 12:58:59", + "updated_at": "2026-05-05 12:58:59" + }, + { + "id": "T-77", + "type": "task", + "title": "IPC server: per-request timeout, broadcast logging, stale-socket race", + "status": "backlog", + "priority": "high", + "created_at": "2026-05-05 12:58:59", + "updated_at": "2026-05-05 12:58:59" + }, + { + "id": "T-78", + "type": "bug", + "title": "files.read path traversal — validate paths stay under workspace root", + "status": "backlog", + "priority": "high", + "created_at": "2026-05-05 12:58:59", + "updated_at": "2026-05-05 12:58:59" + }, + { + "id": "T-79", + "type": "task", + "title": "pane.spawn / editor.open: map errno to actionable error kinds", + "status": "backlog", + "priority": "medium", + "created_at": "2026-05-05 12:58:59", + "updated_at": "2026-05-05 12:58:59" + }, + { + "id": "T-80", + "type": "task", + "title": "Standardize errno constants + logger in PTY/IPC/daemon", + "status": "backlog", + "priority": "low", + "created_at": "2026-05-05 12:58:59", + "updated_at": "2026-05-05 12:58:59" + }, + { + "id": "T-81", + "type": "task", + "title": "Misc PTY/IPC/git polish (audit medium-priority items)", + "status": "backlog", + "priority": "low", + "created_at": "2026-05-05 12:58:59", + "updated_at": "2026-05-05 12:58:59" } ], "ticket_deps": null, @@ -3709,6 +3835,97 @@ "old_value": "in_progress", "new_value": "done", "changed_at": "2026-05-03 20:40:22" + }, + { + "ticket_id": "T-43", + "field": "status", + "old_value": "in_progress", + "new_value": "done", + "changed_at": "2026-05-05 12:51:24" + }, + { + "ticket_id": "T-68", + "field": "status", + "old_value": "backlog", + "new_value": "done", + "changed_at": "2026-05-05 12:53:11" + }, + { + "ticket_id": "T-69", + "field": "status", + "old_value": "backlog", + "new_value": "done", + "changed_at": "2026-05-05 12:53:11" + }, + { + "ticket_id": "T-70", + "field": "status", + "old_value": "backlog", + "new_value": "done", + "changed_at": "2026-05-05 12:53:11" + }, + { + "ticket_id": "T-71", + "field": "status", + "old_value": "backlog", + "new_value": "done", + "changed_at": "2026-05-05 12:53:11" + }, + { + "ticket_id": "T-72", + "field": "status", + "old_value": "backlog", + "new_value": "done", + "changed_at": "2026-05-05 12:53:11" + }, + { + "ticket_id": "T-68", + "field": "status", + "old_value": "done", + "new_value": "done", + "changed_at": "2026-05-05 12:53:15" + }, + { + "ticket_id": "T-69", + "field": "status", + "old_value": "done", + "new_value": "done", + "changed_at": "2026-05-05 12:53:15" + }, + { + "ticket_id": "T-70", + "field": "status", + "old_value": "done", + "new_value": "done", + "changed_at": "2026-05-05 12:53:15" + }, + { + "ticket_id": "T-71", + "field": "status", + "old_value": "done", + "new_value": "done", + "changed_at": "2026-05-05 12:53:15" + }, + { + "ticket_id": "T-72", + "field": "status", + "old_value": "done", + "new_value": "done", + "changed_at": "2026-05-05 12:53:15" + }, + { + "ticket_id": "T-18", + "field": "status", + "old_value": "ready", + "new_value": "in_progress", + "changed_at": "2026-05-05 12:54:39" + }, + { + "ticket_id": "T-18", + "field": "status", + "old_value": "in_progress", + "new_value": "done", + "changed_at": "2026-05-05 12:59:05" } ] } diff --git a/docs/audits/pty-ipc-error-handling-2026-05-05.md b/docs/audits/pty-ipc-error-handling-2026-05-05.md new file mode 100644 index 00000000..69e346c5 --- /dev/null +++ b/docs/audits/pty-ipc-error-handling-2026-05-05.md @@ -0,0 +1,155 @@ +# PTY + IPC error-handling audit + +Date: 2026-05-05 +Ticket: T-18 +Decision ref: D-5 + +Punch list of error-handling issues in `lib/src/pty/`, `lib/src/ipc/`, +and `lib/src/daemon/`. Severity-ranked. Each item references the +follow-up ticket where the fix lands. + +## Critical — silent failures, leaks, races + +1. **`lib/src/pty/native_pty.dart:155-158`** — `forkpty()` failure + throws `StateError('forkpty() failed')` with no errno. Caller + can't distinguish ENOMEM/EAGAIN/ENOENT-of-/dev/ptmx. Capture + errno before `_freeAll` (which may trample it) and surface via + `PtyException`. → T-75 + +2. **`lib/src/pty/native_pty.dart:160-165`** — Child process: `chdir` + and `execve` returns are ignored. If `execve` returns (i.e. + fails), we fall through to `_exit(1)` with no diagnostic. Write + a one-line error envelope to fd 1 before exiting so the parent's + reader sees "exec failed: ENOENT" instead of immediate EOF. → T-75 + +3. **`lib/src/pty/native_pty.dart:244-251`** — `write()` ignores + `_nativeWrite` return. Short writes silently drop bytes; -1/EPIPE + reported as successful "wrote -1". Loop until full length is + written or surface errno on negative returns. → T-75 + +4. **`lib/src/pty/native_pty.dart:259-262`** — `resize()` ignores + `_ioctl` and `_nativeKill` return values. EBADF on a half-closed + fd silently no-ops. Set `_dead = true` on EBADF. → T-75 + +5. **`lib/src/pty/native_pty.dart:198-210`** — Race: `_spawnReader` + is `async` but `NativePty.start` returns immediately. `close()` + racing with isolate spawn can leave the isolate orphaned. Make + `start` await reader spawn or track the spawn-future. → T-76 + +6. **`lib/src/pty/native_pty.dart:280-290`** — `close()` sets + `_dead = true` *before* `_nativeClose(_fd)`, but the reader + isolate continues polling on `_fd`. If a new fd reuses that + number, the reader's `poll` may briefly target the wrong file. + Send shutdown signal via SendPort or self-pipe before closing. → T-76 + +7. **`lib/src/pty/session.dart:135-153`** — Resource leak: if + `_recvFdAsync`, `setWinsize`, `proc.stdout.first.timeout`, or + `_extractPid` throws, the spawned ptyc Process and (in some + cases) the received `masterFd` leak. Only line 151 closes + `masterFd`. Wrap post-spawn block in try/catch that kills `proc`, + closes `masterFd`, and rethrows. → T-76 + +8. **`lib/src/pty/session.dart:240`** — `_recvFdAsync`: if + `Isolate.spawn` itself throws, `port` is leaked. Wrap in + try/catch. → T-76 + +9. **`lib/src/pty/session.dart:165-176`** — `write()` returns raw + `libc.write` result without checking < 0 / errno or looping for + short writes. Same as #3. → T-75 + +10. **`lib/src/pty/session.dart:271-275`** — `Isolate.spawn(...).then(...)` + is fire-and-forget. If spawn fails, the error is silently + swallowed and `_readerIsolate` remains null forever. Add + `.catchError` or await it. → T-76 + +11. **`lib/src/ipc/server.dart:30-39`** — `broadcast()` `try/catch (_)` + swallows write errors with no logging. At least log the kind. → T-77 + +12. **`lib/src/ipc/server.dart:107`** — `client.writeln(resp.encode())` + is not awaited and not guarded. If client disconnected mid-dispatch, + this throws asynchronously with no `onError` handler. Wrap in + try/catch and remove the client from `_clients`. → T-77 + +13. **`lib/src/ipc/server.dart:83-108`** — `_handleLine` runs + `await dispatch(msg)` with no per-request timeout. A misbehaving + handler blocks the connection's read pipeline indefinitely. → T-77 + +14. **`lib/src/ipc/server.dart:46-50`** — Stale-socket retry deletes + the socket file unconditionally on `SocketException`. If two + daemon instances race to start, the second rips the first's live + socket out from under it. Try `connect()` first; refuse if a + live daemon answers. → T-77 + +## High — degraded UX / debugging + +15. **`lib/src/daemon/pane_commands.dart:87-96`** — `_spawn` + catch-all flattens every failure into `tool_error: pane.spawn + failed: `. "binary not found", "permission denied", + "out of pty fds" all look the same. Map `PtyException.errno` + (ENOENT/EACCES/EMFILE) to distinct hints/codes. → T-79 + +16. **`lib/src/daemon/editor_commands.dart:67-76`** — Same pattern; + `editor.open` catch-all loses FileSystemException distinctions + (ENOENT vs EACCES vs EISDIR). → T-79 + +17. **`lib/src/daemon/files_commands.dart:78`** — `file.readAsStringSync()` + is unguarded; UTF-8 errors, permission errors, races with deletion + turn into a 500-style dispatch error instead of a clean + `IpcResponse.err`. Wrap in try/catch. → T-81 + +18. **`lib/src/daemon/files_commands.dart:74`** — Path is concatenated + with `/` and never validated. `path: "../../../etc/passwd"` + traverses out of `files.root`. Resolve and verify the resulting + path stays under `root.absolute.path`. → T-78 (security) + +19. **`lib/src/pty/session.dart:201-234`** — `close()` distinguishes + EOF/EBADF/EIO only in comments. The 500ms timeout is silent + (`onTimeout: () {}`). Log the timeout so we know when SIGKILL + was actually needed. → T-81 + +20. **`lib/src/pty/session.dart:390-394`** — Reader isolate treats + any negative read return that isn't EINTR as EOF — including + transient EAGAIN or recoverable EIO. Inspect errno and log + non-EBADF/EIO/0 cases. → T-81 + +21. **`lib/src/pty/ffi/scm_rights.dart:115-116`** — Returned cmsg-data + fd is read without sanity-checking against `msgControllen`. A + malformed peer that sends only a partial cmsg could let us read + garbage as an fd. Verify `dataOffset + 4 <= msgControllen` + before deref. → T-81 + +22. **`lib/src/ipc/server.dart:41-56`** — `start()` logs to + `stderr.writeln` but the rest of the daemon uses no logger. In + the Flutter-host process stderr is often consumed by the engine. + Standardize on a logger. → T-80 + +## Medium — cleanliness + +23. **`lib/src/pty/session.dart:390`, `native_pty.dart:262, 285`** — + Magic errno/signal numbers (`4=EINTR`, `9=SIGKILL`, `28=SIGWINCH`, + `_kSighup=1`). Pull into named constants. → T-80 + +24. **`lib/src/pty/ffi/libc.dart:232-245`** — `errno` getter does a + `lookupFunction` on every access (catching ArgumentError every + call on macOS). Cache the resolved function pointer. → T-80 + +25. **`lib/src/daemon/git_commands.dart:283`** — `_gitError` always + reports `tool_error`. A `git push` rejection or merge conflict is + user-actionable, not a tool failure; could map to + `IpcExitCode.conflict` when stderr matches known patterns. → T-81 + +26. **`lib/src/ipc/server.dart:97`** — Dispatch error shows + `dispatch failed: $e` (full exception toString). Trim and add + the request `cmd` for log correlation. → T-80 + +27. **`lib/src/daemon/pane_commands.dart:136`** — `registry.write(id, bytes)` + return value `n` is shown to caller, but if `n == -1` (write failed) + we still respond `ok`. Distinguish. → T-81 + +28. **`lib/src/ipc/envelope.dart:88-94`** — `IpcResponse.fromJson` + throws `TypeError` if `ok=false` but `error` is missing. No + graceful degradation for a malformed peer response. → T-81 + +29. **`lib/src/pty/native_pty.dart:111-119`** — PATH resolution + silently uses the first existing match without checking `X_OK`. + A non-executable file shadows a valid binary further along PATH. → T-81