From 66d3d024ca39f8d8adda56ab0911388e0e23fefd Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 10 Jun 2026 16:59:09 +0200 Subject: [PATCH] log the failing command on a dispatch error (T-80) The catch-all "dispatch threw" log omitted the request command, so a handler exception couldn't be correlated to what caused it (PTY/IPC audit item #26). Track the resolved cmd across the request handler and include it in the error log. The audit's other two items were already satisfied: errno/signal magic numbers are centralized in PosixErrno + ffi/libc.dart, and the IPC server already logs through the kernel Logger (no stderr.writeln left). Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 34 +++++++++++++++ .pql/changelog/tickets/2026-06.sql | 52 +++++++++++++++++++++++ lib/src/ipc/server.dart | 7 ++- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 210f0e8e..3149378a 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3199,3 +3199,37 @@ Notes (2026-06-10): 1. Applies to BOTH rails — the left sidebar icon rail and the right context-bar icon rail. Reordering + persistence must work the same on each. 2. After ordering, the left-most (first) item in the rail is the one that opens by default.', NULL, '2026-06-10 14:45:17', '2026-06-10 14:45:17', '2026-06-10 14:45:17', NULL, '86f1c477f9408ce87b624799a73b82c0', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB0TNQM734YZ060Q63H40EYG', 'status', 'backlog', 'ready', NULL, '2026-06-10 14:46:51', '2026-06-10 14:46:51', '2026-06-10 14:46:51', NULL, 'a3785538cc5ea4040c7f88ee615342b4', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB0TNQM734YZ060Q63H40EYG', 'status', 'ready', 'in_progress', NULL, '2026-06-10 14:53:03', '2026-06-10 14:53:03', '2026-06-10 14:53:03', NULL, '6fe6ac08d39ee01463fc6e629a0b00d7', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB0TNQM734YZ060Q63H40EYG', 'description', 'From the PTY/IPC error-handling audit (T-18, see docs/audits/pty-ipc-error-handling-2026-05-05.md). Two cleanup items rolled together: + +**Errno constants (audit item #23):** +- Magic numbers (`4` for EINTR, `9` for EBADF, `28` for SIGWINCH, `1` for SIGHUP, `32` for EPIPE) appear inline across `lib/src/pty/session.dart` and `lib/src/pty/native_pty.dart`. +- Centralize them in `lib/src/pty/errors.dart` or a sibling `posix.dart` as named constants. +- Existing `lib/src/ipc/errno_mapping.dart` already has a `PosixErrno` class — extend it or move to a shared location both layers import from. + +**Logger standardization (audit item #22, partial #26):** +- `lib/src/ipc/server.dart` uses `stderr.writeln(...)` directly; the rest of the daemon either uses no logger or a custom one. +- The Flutter-host process often consumes stderr, so log lines disappear silently. +- Pick one logger interface (kernel `log` already exists for the app side), wire `DaemonServer` and the daemon-side handlers to use it. +- Dispatch error messages should prefix with the request `cmd` so log correlation works (audit item #26). + +**Out of scope for this ticket:** changes to log-LEVEL policy, log retention, log files vs stderr — pure substitution job.', 'From the PTY/IPC error-handling audit (T-18, see docs/audits/pty-ipc-error-handling-2026-05-05.md). Two cleanup items rolled together: + +**Errno constants (audit item #23):** +- Magic numbers (`4` for EINTR, `9` for EBADF, `28` for SIGWINCH, `1` for SIGHUP, `32` for EPIPE) appear inline across `lib/src/pty/session.dart` and `lib/src/pty/native_pty.dart`. +- Centralize them in `lib/src/pty/errors.dart` or a sibling `posix.dart` as named constants. +- Existing `lib/src/ipc/errno_mapping.dart` already has a `PosixErrno` class — extend it or move to a shared location both layers import from. + +**Logger standardization (audit item #22, partial #26):** +- `lib/src/ipc/server.dart` uses `stderr.writeln(...)` directly; the rest of the daemon either uses no logger or a custom one. +- The Flutter-host process often consumes stderr, so log lines disappear silently. +- Pick one logger interface (kernel `log` already exists for the app side), wire `DaemonServer` and the daemon-side handlers to use it. +- Dispatch error messages should prefix with the request `cmd` so log correlation works (audit item #26). + +**Out of scope for this ticket:** changes to log-LEVEL policy, log retention, log files vs stderr — pure substitution job. + +Disposition (2026-06-10): mostly already done before pickup. +- #23 (errno constants): DONE prior. Magic numbers are centralized — errno values in lib/src/ipc/errno_mapping.dart (PosixErrno: eintr=4, ebadf=9, epipe=32, …), signals in lib/src/pty/ffi/libc.dart (sighup=1, sigwinch=28). native_pty.dart uses PosixErrno.* and libc.* throughout; no inline magic numbers remain. The ticket''s lib/src/pty/session.dart never existed at that path. +- #22 (logger): DONE prior. lib/src/ipc/server.dart imports the kernel Logger, holds a ''final Logger log'', and logs via log.error/warn/info(''ipc'', …). No stderr.writeln/print anywhere in lib/src/ipc, lib/src/pty, or lib/src/daemon. Folded in by the D-56 daemon dissolution + PTY FFI pivot. +- #26 (cmd correlation): the only live remnant — the catch-all ''dispatch threw'' log omitted the request cmd. Fixed: it now logs ''dispatch threw for ""''. Internal logging only; no changelog.', NULL, '2026-06-10 14:58:32', '2026-06-10 14:58:32', '2026-06-10 14:58:32', NULL, '067666253769388e306dd99c4f976df9', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB0TNQM734YZ060Q63H40EYG', 'status', 'in_progress', 'done', NULL, '2026-06-10 14:58:43', '2026-06-10 14:58:43', '2026-06-10 14:58:43', NULL, '192064fbd5628b282512128ddc3d2688', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 3b6e2fe4..d9e5f3cf 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -2647,3 +2647,55 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat - Dispatch error messages should prefix with the request `cmd` so log correlation works (audit item #26). **Out of scope for this ticket:** changes to log-LEVEL policy, log retention, log files vs stderr — pure substitution job.', 'ready', 'low', NULL, NULL, NULL, '2026-05-05 12:58:59', '2026-06-10 14:46:51', NULL, '1637ba89d9bf5d1e81a42247e6cf46bd', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB0TNQM734YZ060Q63H40EYG', 'task', NULL, 'Standardize errno constants + logger in PTY/IPC/daemon', 'From the PTY/IPC error-handling audit (T-18, see docs/audits/pty-ipc-error-handling-2026-05-05.md). Two cleanup items rolled together: + +**Errno constants (audit item #23):** +- Magic numbers (`4` for EINTR, `9` for EBADF, `28` for SIGWINCH, `1` for SIGHUP, `32` for EPIPE) appear inline across `lib/src/pty/session.dart` and `lib/src/pty/native_pty.dart`. +- Centralize them in `lib/src/pty/errors.dart` or a sibling `posix.dart` as named constants. +- Existing `lib/src/ipc/errno_mapping.dart` already has a `PosixErrno` class — extend it or move to a shared location both layers import from. + +**Logger standardization (audit item #22, partial #26):** +- `lib/src/ipc/server.dart` uses `stderr.writeln(...)` directly; the rest of the daemon either uses no logger or a custom one. +- The Flutter-host process often consumes stderr, so log lines disappear silently. +- Pick one logger interface (kernel `log` already exists for the app side), wire `DaemonServer` and the daemon-side handlers to use it. +- Dispatch error messages should prefix with the request `cmd` so log correlation works (audit item #26). + +**Out of scope for this ticket:** changes to log-LEVEL policy, log retention, log files vs stderr — pure substitution job.', 'in_progress', 'low', NULL, NULL, NULL, '2026-05-05 12:58:59', '2026-06-10 14:53:03', NULL, '38f75d5cabd06dd1aa45701442eb9720', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB0TNQM734YZ060Q63H40EYG', 'task', NULL, 'Standardize errno constants + logger in PTY/IPC/daemon', 'From the PTY/IPC error-handling audit (T-18, see docs/audits/pty-ipc-error-handling-2026-05-05.md). Two cleanup items rolled together: + +**Errno constants (audit item #23):** +- Magic numbers (`4` for EINTR, `9` for EBADF, `28` for SIGWINCH, `1` for SIGHUP, `32` for EPIPE) appear inline across `lib/src/pty/session.dart` and `lib/src/pty/native_pty.dart`. +- Centralize them in `lib/src/pty/errors.dart` or a sibling `posix.dart` as named constants. +- Existing `lib/src/ipc/errno_mapping.dart` already has a `PosixErrno` class — extend it or move to a shared location both layers import from. + +**Logger standardization (audit item #22, partial #26):** +- `lib/src/ipc/server.dart` uses `stderr.writeln(...)` directly; the rest of the daemon either uses no logger or a custom one. +- The Flutter-host process often consumes stderr, so log lines disappear silently. +- Pick one logger interface (kernel `log` already exists for the app side), wire `DaemonServer` and the daemon-side handlers to use it. +- Dispatch error messages should prefix with the request `cmd` so log correlation works (audit item #26). + +**Out of scope for this ticket:** changes to log-LEVEL policy, log retention, log files vs stderr — pure substitution job. + +Disposition (2026-06-10): mostly already done before pickup. +- #23 (errno constants): DONE prior. Magic numbers are centralized — errno values in lib/src/ipc/errno_mapping.dart (PosixErrno: eintr=4, ebadf=9, epipe=32, …), signals in lib/src/pty/ffi/libc.dart (sighup=1, sigwinch=28). native_pty.dart uses PosixErrno.* and libc.* throughout; no inline magic numbers remain. The ticket''s lib/src/pty/session.dart never existed at that path. +- #22 (logger): DONE prior. lib/src/ipc/server.dart imports the kernel Logger, holds a ''final Logger log'', and logs via log.error/warn/info(''ipc'', …). No stderr.writeln/print anywhere in lib/src/ipc, lib/src/pty, or lib/src/daemon. Folded in by the D-56 daemon dissolution + PTY FFI pivot. +- #26 (cmd correlation): the only live remnant — the catch-all ''dispatch threw'' log omitted the request cmd. Fixed: it now logs ''dispatch threw for ""''. Internal logging only; no changelog.', 'in_progress', 'low', NULL, NULL, NULL, '2026-05-05 12:58:59', '2026-06-10 14:58:32', NULL, '97a7a5a0bb8841f3a586c8b559a42819', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB0TNQM734YZ060Q63H40EYG', 'task', NULL, 'Standardize errno constants + logger in PTY/IPC/daemon', 'From the PTY/IPC error-handling audit (T-18, see docs/audits/pty-ipc-error-handling-2026-05-05.md). Two cleanup items rolled together: + +**Errno constants (audit item #23):** +- Magic numbers (`4` for EINTR, `9` for EBADF, `28` for SIGWINCH, `1` for SIGHUP, `32` for EPIPE) appear inline across `lib/src/pty/session.dart` and `lib/src/pty/native_pty.dart`. +- Centralize them in `lib/src/pty/errors.dart` or a sibling `posix.dart` as named constants. +- Existing `lib/src/ipc/errno_mapping.dart` already has a `PosixErrno` class — extend it or move to a shared location both layers import from. + +**Logger standardization (audit item #22, partial #26):** +- `lib/src/ipc/server.dart` uses `stderr.writeln(...)` directly; the rest of the daemon either uses no logger or a custom one. +- The Flutter-host process often consumes stderr, so log lines disappear silently. +- Pick one logger interface (kernel `log` already exists for the app side), wire `DaemonServer` and the daemon-side handlers to use it. +- Dispatch error messages should prefix with the request `cmd` so log correlation works (audit item #26). + +**Out of scope for this ticket:** changes to log-LEVEL policy, log retention, log files vs stderr — pure substitution job. + +Disposition (2026-06-10): mostly already done before pickup. +- #23 (errno constants): DONE prior. Magic numbers are centralized — errno values in lib/src/ipc/errno_mapping.dart (PosixErrno: eintr=4, ebadf=9, epipe=32, …), signals in lib/src/pty/ffi/libc.dart (sighup=1, sigwinch=28). native_pty.dart uses PosixErrno.* and libc.* throughout; no inline magic numbers remain. The ticket''s lib/src/pty/session.dart never existed at that path. +- #22 (logger): DONE prior. lib/src/ipc/server.dart imports the kernel Logger, holds a ''final Logger log'', and logs via log.error/warn/info(''ipc'', …). No stderr.writeln/print anywhere in lib/src/ipc, lib/src/pty, or lib/src/daemon. Folded in by the D-56 daemon dissolution + PTY FFI pivot. +- #26 (cmd correlation): the only live remnant — the catch-all ''dispatch threw'' log omitted the request cmd. Fixed: it now logs ''dispatch threw for ""''. Internal logging only; no changelog.', 'done', 'low', NULL, NULL, NULL, '2026-05-05 12:58:59', '2026-06-10 14:58:43', NULL, 'd7ba06c0e311a1945f8ccead0b132570', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/lib/src/ipc/server.dart b/lib/src/ipc/server.dart index 8a368218..df276f89 100644 --- a/lib/src/ipc/server.dart +++ b/lib/src/ipc/server.dart @@ -190,6 +190,9 @@ class IpcServer { final trimmed = line.trim(); if (trimmed.isEmpty) return; IpcResponse response; + // Tracked across the try so a dispatch failure can be logged against the + // command that caused it, for log correlation (audit #26 / T-80). + var reqCmd = '?'; try { final msg = IpcMessage.decode(trimmed); if (msg is! IpcRequest) { @@ -206,6 +209,7 @@ class IpcServer { // streaming check sees the unwrapped command (T-129). Plain // typed requests skip this path. var req = msg; + reqCmd = req.cmd; if (req.cmd == argvSentinelCmd) { final result = unwrapArgvRequest(req); if (result is ArgvError) { @@ -220,6 +224,7 @@ class IpcServer { return; } req = (result as ArgvParsed).request; + reqCmd = req.cmd; } if (_isTailSubscribe(req)) { // Long-lived subscription branch (T-129). Send the streaming @@ -240,7 +245,7 @@ class IpcServer { ), ); } catch (e, st) { - log.error('ipc', 'dispatch threw', error: e, stackTrace: st); + log.error('ipc', 'dispatch threw for "$reqCmd"', error: e, stackTrace: st); response = IpcResponse.err( id: '', error: IpcError(