From d7c935977eb67217162cca98b14108cfa174f1f3 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Fri, 22 May 2026 15:39:59 +0200 Subject: [PATCH] fix daemon-not-connected race on startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The socket-loopback DaemonClient (T-127) raced the UI on first launch: panels queried before the socket finished connecting and cached a "daemon not connected" error, and the Claude pane's spawn gate tripped, leaving an empty terminal. Three fixes in the startup/connection path: - DaemonClient.request() now waits briefly (5s) for an in-flight connection instead of failing instantly, gated on _started so a never-started client still fails fast. start() sets _started synchronously so the gate is armed before the UI builds. - swapIpcServer reuses the live server when the opened project matches the workspace it already serves, instead of tearing it down — the project-open flow fired right as the Claude pane spawned, dropping the connection mid-spawn. - _connect bails if already connected, so start() arming the reconnect loop and swapIpcServer's reconnectAt can't open a second socket (which had been double-delivering events). This whole orchestration had no automated coverage — integration tests stub a FakeDaemonClient. Adds a real wait-then-connect client test. Co-Authored-By: Claude --- .pql/changelog/ticket_history/2026-05.sql | 119 ++++++++++++++++++++++ .pql/changelog/tickets/2026-05.sql | 117 +++++++++++++++++++++ CHANGELOG.md | 8 ++ lib/kernel/src/ipc/client.dart | 74 ++++++++++++-- lib/main.dart | 26 ++++- test/kernel/src/ipc/client_test.dart | 35 ++++++- 6 files changed, 363 insertions(+), 16 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index 2bae53af..116d8f4d 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -1954,3 +1954,122 @@ Next steps: ProjectManager sticky-startup logic is already covered by test/kernel/src/project_test.dart (14 cases).', NULL, '2026-05-22 06:55:02', '2026-05-22 06:55:02', '2026-05-22 06:55:02', NULL, 'b7c13548cef5d962c7b5b4500ea39b9e', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-122', 'priority', 'low', 'medium', NULL, '2026-05-22 06:55:02', '2026-05-22 06:55:02', '2026-05-22 06:55:02', NULL, 'e7d5dd1b89eded661de7f6d64edea04c', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-89', 'description', 'Today: total line coverage **34.9%** (2984/8549 lines). Per area: + +| area | hit / total | % | +|---|---|---| +| `lib/src/terminal/` | 12 / 3167 | **0.4%** | +| `lib/widgets/` | 537 / 1184 | 45.4% | +| `lib/src/pql/` | 45 / 94 | 47.9% | +| `lib/builtin/` | 269 / 550 | 48.9% | +| `lib/kernel/` | 889 / 1691 | 52.6% | +| `lib/src/daemon/` | 320 / 584 | 54.8% | +| `lib/src/pty/` | 130 / 216 | 60.2% | +| `lib/src/git/` | 356 / 518 | 68.7% | +| `lib/src/ipc/` | 110 / 156 | 70.5% | +| `lib/src/files/` | 98 / 133 | 73.7% | +| `lib/extension/` | 41 / 54 | 75.9% | +| `lib/src/editor/` | 107 / 129 | 82.9% | +| `lib/src/panes/` | 70 / 73 | 95.9% | + +The dominant hole is `lib/src/terminal/` — the terminal emulator code. Header still credits xterm.dart upstream, but per CLAUDE.md ("Own the rendering stack") and the explicit guidance attached to this epic: in-tree code is owned, no carve-outs, same coverage bar. + +**End state:** +- Total line coverage ≥ 95% (no per-area carve-outs). +- Pre-push gate hard-fails on coverage drops via a committed floor (`coverage/floor.txt`); floor ratchets up only. +- Per D-66 the gate lands at the *current* floor in the first child ticket; subsequent floor bumps come with the test-writing children. + +**Acceptance criteria for closing this epic:** +1. `coverage/floor.txt` reads `95` (or higher). +2. `flutter test --coverage --exclude-tags forkpty` produces ≥95% line coverage. +3. The pre-push gate computes coverage and rejects pushes below the floor. +4. No new `// ignore:` / `// ignore_for_file:` / analysis-options excludes were added to dodge the gate. Any suppression added during this campaign needs explicit approval and an inline reason at the suppression site. + +**Strategy / child shape:** +The gate plumbing lands first so future test additions are visible as floor bumps. Then per-area sweeps — biggest holes first because they move the global number fastest, but each area is its own child ticket with its own scope. + +1. **Gate plumbing** (first child) — install the ratchet, lock in current floor, expose `make coverage-gate` and wire it into `make push-check`. +2. **`lib/src/terminal/` sweep** (second child, by far the largest) — likely splits into sub-children once the area''s structure is mapped (parser, buffer, painter, gesture handling, mouse reporting, escape sequences, cell-grid measurement, etc.). +3. **Per-area sweeps** — claim a child ticket when starting an area; close when the area hits 95%. Order by impact: terminal → widgets → pql → builtin → kernel → daemon → pty → git → ipc → files → extension → editor. +4. **Final ratchet** — once total ≥ 95%, set floor to 95 and close the epic. + +**Out of scope:** +- Branch coverage (lcov data is weak for branch on Dart; line coverage is the contract). +- Integration / E2E coverage (only widget + unit feed lcov). +- `lib/test_app.dart` exclusion: verify in the gate child whether Dart''s coverage tooling already excludes it; if not, decide explicitly rather than carve it out silently. + +**Cross-references:** D-66 (this epic''s decision record), D-29 (pre-push gate scope), "Own the rendering stack" guardrail (CLAUDE.md). +', '## OUTCOME (closed 2026-05-18) + +Epic met. Final measured line coverage **95.27%** (9551 / 10025), floor `coverage_floor: 95` in `pubspec.yaml`, gate wired into `make push-check`. + +Final per-area (vs. opening 34.9% total): + +| area | hit / total | % | +|---|---|---| +| `lib/src/terminal/` | 3185 / 3188 | **99.9%** (was 0.4%) | +| `lib/extension/` | 54 / 54 | 100.0% | +| `lib/src/editor/` | 129 / 129 | 100.0% | +| `lib/src/files/` | 139 / 140 | 99.3% | +| `lib/src/pql/` | 91 / 92 | 98.9% | +| `lib/src/panes/` | 75 / 76 | 98.7% | +| `lib/src/git/` | 517 / 531 | 97.4% | +| `lib/src/daemon/` | 616 / 644 | 95.7% | +| `lib/kernel/` | 2122 / 2233 | 95.0% | +| `lib/src/ipc/` | 429 / 452 | 94.9% | +| `lib/widgets/` | 1178 / 1261 | 93.4% | +| `lib/builtin/` | 754 / 875 | 86.2% | +| `lib/src/pty/` | 177 / 262 | 67.6% (forkpty paths excluded from gate) | + +Acceptance: (1) floor reads 95 done (2) total >=95% done (3) gate hard-fails below floor done (4) no coverage-dodging suppressions done. The strategy''s tail per-area sweeps (widgets/builtin/pty) didn''t all individually reach 95%, but the contract is *total* line coverage, which is satisfied; remaining per-area headroom can be its own tickets if desired. + +--- + +Today: total line coverage **34.9%** (2984/8549 lines). Per area: + +| area | hit / total | % | +|---|---|---| +| `lib/src/terminal/` | 12 / 3167 | **0.4%** | +| `lib/widgets/` | 537 / 1184 | 45.4% | +| `lib/src/pql/` | 45 / 94 | 47.9% | +| `lib/builtin/` | 269 / 550 | 48.9% | +| `lib/kernel/` | 889 / 1691 | 52.6% | +| `lib/src/daemon/` | 320 / 584 | 54.8% | +| `lib/src/pty/` | 130 / 216 | 60.2% | +| `lib/src/git/` | 356 / 518 | 68.7% | +| `lib/src/ipc/` | 110 / 156 | 70.5% | +| `lib/src/files/` | 98 / 133 | 73.7% | +| `lib/extension/` | 41 / 54 | 75.9% | +| `lib/src/editor/` | 107 / 129 | 82.9% | +| `lib/src/panes/` | 70 / 73 | 95.9% | + +The dominant hole is `lib/src/terminal/` — the terminal emulator code. Header still credits xterm.dart upstream, but per CLAUDE.md ("Own the rendering stack") and the explicit guidance attached to this epic: in-tree code is owned, no carve-outs, same coverage bar. + +**End state:** +- Total line coverage ≥ 95% (no per-area carve-outs). +- Pre-push gate hard-fails on coverage drops via a committed floor (`coverage/floor.txt`); floor ratchets up only. +- Per D-66 the gate lands at the *current* floor in the first child ticket; subsequent floor bumps come with the test-writing children. + +**Acceptance criteria for closing this epic:** +1. `coverage/floor.txt` reads `95` (or higher). +2. `flutter test --coverage --exclude-tags forkpty` produces ≥95% line coverage. +3. The pre-push gate computes coverage and rejects pushes below the floor. +4. No new `// ignore:` / `// ignore_for_file:` / analysis-options excludes were added to dodge the gate. Any suppression added during this campaign needs explicit approval and an inline reason at the suppression site. + +**Strategy / child shape:** +The gate plumbing lands first so future test additions are visible as floor bumps. Then per-area sweeps — biggest holes first because they move the global number fastest, but each area is its own child ticket with its own scope. + +1. **Gate plumbing** (first child) — install the ratchet, lock in current floor, expose `make coverage-gate` and wire it into `make push-check`. +2. **`lib/src/terminal/` sweep** (second child, by far the largest) — likely splits into sub-children once the area''s structure is mapped (parser, buffer, painter, gesture handling, mouse reporting, escape sequences, cell-grid measurement, etc.). +3. **Per-area sweeps** — claim a child ticket when starting an area; close when the area hits 95%. Order by impact: terminal → widgets → pql → builtin → kernel → daemon → pty → git → ipc → files → extension → editor. +4. **Final ratchet** — once total ≥ 95%, set floor to 95 and close the epic. + +**Out of scope:** +- Branch coverage (lcov data is weak for branch on Dart; line coverage is the contract). +- Integration / E2E coverage (only widget + unit feed lcov). +- `lib/test_app.dart` exclusion: verify in the gate child whether Dart''s coverage tooling already excludes it; if not, decide explicitly rather than carve it out silently. + +**Cross-references:** D-66 (this epic''s decision record), D-29 (pre-push gate scope), "Own the rendering stack" guardrail (CLAUDE.md). +', NULL, '2026-05-22 13:37:43', '2026-05-22 13:37:43', '2026-05-22 13:37:43', NULL, 'b83aa45596e35a95e689b922c66c27ef', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-89', 'status', 'in_progress', 'done', NULL, '2026-05-22 13:37:46', '2026-05-22 13:37:46', '2026-05-22 13:37:46', NULL, '5698b8e8cbd5113a84791ae2dad26c6d', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-91', 'status', 'in_progress', 'done', NULL, '2026-05-22 13:37:46', '2026-05-22 13:37:46', '2026-05-22 13:37:46', NULL, 'dad314c05e04d8f17357ac2ea054a9bd', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index d27cef88..82a2935a 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -2279,3 +2279,120 @@ Next steps: - Once unblocked, this also unblocks lib/builtin/welcome/src/welcome_view.dart coverage (74 uncovered lines, the dominant lib/builtin/ gap toward the T-89 95% target). ProjectManager sticky-startup logic is already covered by test/kernel/src/project_test.dart (14 cases).', 'backlog', 'medium', NULL, NULL, NULL, '2026-05-18 09:06:59', '2026-05-22 06:55:02', NULL, 'bcb79420d9e359813ad5f75dfb89cc05', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_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 (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-91', 'task', 'T-89', 'test sweep: drive lib/src/terminal/ to 95%', 'Second child of T-89. The dominant coverage hole — `lib/src/terminal/` is 12 / 3167 lines (0.4%), about 56% of the global gap. + +**Today''s distribution within `lib/src/terminal/src/`:** + +| sub-area | hit / total | % | shape | +|---|---|---|---| +| `core/buffer/` | 0 / 581 | 0.0% | pure data structures | +| `core/escape/` | 1 / 473 | 0.2% | pure parser logic | +| `core/input/` | 4 / 292 | 1.4% | key + keytab tables | +| `core/(root)` | 0 / 188 | 0.0% | callback wiring | +| `core/mouse/` | 3 / 51 | 5.9% | encoder logic | +| `ui/` | 3 / 823 | 0.4% | painters + render objects | +| `(root)` | 0 / 471 | 0.0% | `Terminal` + `TerminalView` | +| `utils/` | 1 / 251 | 0.4% | circular buffer etc. | + +**Strategy — pure logic first, then UI:** + +1. **`core/buffer/`** (581 lines, biggest single chunk, all pure Dart) — line/segment/cell models, scrollback math, range queries. Unit tests exercise the data structure directly. +2. **`core/escape/`** (473 lines, pure state machine) — feed ANSI/VT byte streams in, assert the resulting buffer/cursor state. Reusable corpus from xterm.js / xterm.dart upstream is fair game. +3. **`core/input/`** (292 lines) — key + keytab tables; assert mapping from logical key to escape bytes per terminal mode. +4. **`core/mouse/`** (51 lines) — encoder for SGR-1006 / X11 mouse protocols. Small, fast. +5. **`utils/`** (251 lines) — circular buffer + helpers; trivial unit tests. +6. **`(root)`** (`terminal.dart`, `terminal_view.dart`, 471 lines) — orchestration; some unit-testable, some needs widget tests. +7. **`ui/`** (823 lines) — painters and render objects; widget + golden tests. Hardest, most pixel-sensitive. + +**Acceptance:** +- `lib/src/terminal/` line coverage ≥ 95%. +- All tests fast (<5 s for the area''s test files combined; no real PTY shell-out). +- Helper fixtures live in `test/terminal/_helpers/` if they get reused across files. +- Per D-66, no `// ignore:` directives or analysis_options excludes added to dodge coverage. +- `coverage_floor:` in `pubspec.yaml` is bumped in lockstep — every commit that adds covering tests bumps the floor by the measured delta (rounded down to the integer below the new percentage). + +**Strategy on commit cadence:** +Land tests in batches per sub-area (one commit per `core/buffer/`, `core/escape/`, etc.). Each batch: +- Adds tests under `test/terminal//`. +- Re-measures coverage, bumps `coverage_floor:` to the new integer (or as high as the gate currently allows). +- Documents in CHANGELOG under "Changed" (test-only changes can also fold into one CHANGELOG entry per area). + +**Out of scope:** +- Refactoring `lib/src/terminal/` for testability — if a piece is genuinely untestable, surface it (don''t silence it). Code under `lib/` is owned regardless of upstream attribution; refactor to make it testable rather than carving it out. +- Branch coverage (D-66 explicitly excludes). +- Mouse / keyboard *protocol* round-trips against real TUIs (that''s T-74 and integration work). + +**Cross-references:** D-66 (coverage gate), T-89 (epic), T-90 (gate plumbing). +', 'done', 'high', NULL, NULL, 'D-66', '2026-05-06 20:49:47', '2026-05-22 13:37:46', NULL, '076081a76e2301e97262f230c5cd5516', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_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 (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-89', 'epic', NULL, 'drive line coverage from 35% to 95%', '## OUTCOME (closed 2026-05-18) + +Epic met. Final measured line coverage **95.27%** (9551 / 10025), floor `coverage_floor: 95` in `pubspec.yaml`, gate wired into `make push-check`. + +Final per-area (vs. opening 34.9% total): + +| area | hit / total | % | +|---|---|---| +| `lib/src/terminal/` | 3185 / 3188 | **99.9%** (was 0.4%) | +| `lib/extension/` | 54 / 54 | 100.0% | +| `lib/src/editor/` | 129 / 129 | 100.0% | +| `lib/src/files/` | 139 / 140 | 99.3% | +| `lib/src/pql/` | 91 / 92 | 98.9% | +| `lib/src/panes/` | 75 / 76 | 98.7% | +| `lib/src/git/` | 517 / 531 | 97.4% | +| `lib/src/daemon/` | 616 / 644 | 95.7% | +| `lib/kernel/` | 2122 / 2233 | 95.0% | +| `lib/src/ipc/` | 429 / 452 | 94.9% | +| `lib/widgets/` | 1178 / 1261 | 93.4% | +| `lib/builtin/` | 754 / 875 | 86.2% | +| `lib/src/pty/` | 177 / 262 | 67.6% (forkpty paths excluded from gate) | + +Acceptance: (1) floor reads 95 done (2) total >=95% done (3) gate hard-fails below floor done (4) no coverage-dodging suppressions done. The strategy''s tail per-area sweeps (widgets/builtin/pty) didn''t all individually reach 95%, but the contract is *total* line coverage, which is satisfied; remaining per-area headroom can be its own tickets if desired. + +--- + +Today: total line coverage **34.9%** (2984/8549 lines). Per area: + +| area | hit / total | % | +|---|---|---| +| `lib/src/terminal/` | 12 / 3167 | **0.4%** | +| `lib/widgets/` | 537 / 1184 | 45.4% | +| `lib/src/pql/` | 45 / 94 | 47.9% | +| `lib/builtin/` | 269 / 550 | 48.9% | +| `lib/kernel/` | 889 / 1691 | 52.6% | +| `lib/src/daemon/` | 320 / 584 | 54.8% | +| `lib/src/pty/` | 130 / 216 | 60.2% | +| `lib/src/git/` | 356 / 518 | 68.7% | +| `lib/src/ipc/` | 110 / 156 | 70.5% | +| `lib/src/files/` | 98 / 133 | 73.7% | +| `lib/extension/` | 41 / 54 | 75.9% | +| `lib/src/editor/` | 107 / 129 | 82.9% | +| `lib/src/panes/` | 70 / 73 | 95.9% | + +The dominant hole is `lib/src/terminal/` — the terminal emulator code. Header still credits xterm.dart upstream, but per CLAUDE.md ("Own the rendering stack") and the explicit guidance attached to this epic: in-tree code is owned, no carve-outs, same coverage bar. + +**End state:** +- Total line coverage ≥ 95% (no per-area carve-outs). +- Pre-push gate hard-fails on coverage drops via a committed floor (`coverage/floor.txt`); floor ratchets up only. +- Per D-66 the gate lands at the *current* floor in the first child ticket; subsequent floor bumps come with the test-writing children. + +**Acceptance criteria for closing this epic:** +1. `coverage/floor.txt` reads `95` (or higher). +2. `flutter test --coverage --exclude-tags forkpty` produces ≥95% line coverage. +3. The pre-push gate computes coverage and rejects pushes below the floor. +4. No new `// ignore:` / `// ignore_for_file:` / analysis-options excludes were added to dodge the gate. Any suppression added during this campaign needs explicit approval and an inline reason at the suppression site. + +**Strategy / child shape:** +The gate plumbing lands first so future test additions are visible as floor bumps. Then per-area sweeps — biggest holes first because they move the global number fastest, but each area is its own child ticket with its own scope. + +1. **Gate plumbing** (first child) — install the ratchet, lock in current floor, expose `make coverage-gate` and wire it into `make push-check`. +2. **`lib/src/terminal/` sweep** (second child, by far the largest) — likely splits into sub-children once the area''s structure is mapped (parser, buffer, painter, gesture handling, mouse reporting, escape sequences, cell-grid measurement, etc.). +3. **Per-area sweeps** — claim a child ticket when starting an area; close when the area hits 95%. Order by impact: terminal → widgets → pql → builtin → kernel → daemon → pty → git → ipc → files → extension → editor. +4. **Final ratchet** — once total ≥ 95%, set floor to 95 and close the epic. + +**Out of scope:** +- Branch coverage (lcov data is weak for branch on Dart; line coverage is the contract). +- Integration / E2E coverage (only widget + unit feed lcov). +- `lib/test_app.dart` exclusion: verify in the gate child whether Dart''s coverage tooling already excludes it; if not, decide explicitly rather than carve it out silently. + +**Cross-references:** D-66 (this epic''s decision record), D-29 (pre-push gate scope), "Own the rendering stack" guardrail (CLAUDE.md). +', 'done', 'high', NULL, NULL, 'D-66', '2026-05-06 20:36:42', '2026-05-22 13:37:46', NULL, '294d10b6817279f2c76e147f3066db50', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_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/CHANGELOG.md b/CHANGELOG.md index 423510d7..4e2573f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,14 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- Daemon not connected on startup — the socket-loopback `DaemonClient` + raced the UI: panels queried before the socket finished connecting + and cached a "daemon not connected" error, and the Claude pane's + spawn gate tripped. Requests now wait briefly for the in-flight + connection instead of failing instantly, the server is no longer + needlessly restarted when the opened project matches the launch + workspace, and the client connects once instead of twice. + ### Security ## [2.1.0] — 2026-05-18 diff --git a/lib/kernel/src/ipc/client.dart b/lib/kernel/src/ipc/client.dart index bcd973bd..fe147893 100644 --- a/lib/kernel/src/ipc/client.dart +++ b/lib/kernel/src/ipc/client.dart @@ -26,26 +26,39 @@ class DaemonClient extends ChangeNotifier { Socket? _socket; bool _connected = false; bool _disposed = false; + bool _started = false; Timer? _reconnectTimer; Duration _backoff = const Duration(milliseconds: 200); int _nextId = 0; final Map> _pending = {}; + /// Requests that arrived before the socket was connected park here + /// until the connection comes up (or the wait times out). + final List> _connectWaiters = []; + + /// How long a request will wait for an in-progress connection before + /// giving up with a not-connected error. Covers the startup window + /// where the UI queries before the socket has finished connecting. + static const Duration _connectWait = Duration(seconds: 5); + bool get isConnected => _connected; Future start() async { _disposed = false; + _started = true; await _connect(); } Future stop() async { _disposed = true; + _started = false; _reconnectTimer?.cancel(); _reconnectTimer = null; final s = _socket; _socket = null; await s?.close(); _failPending('client stopped'); + _wakeConnectWaiters(); _setConnected(false); } @@ -67,6 +80,7 @@ class DaemonClient extends ChangeNotifier { _failPending('socket path changed'); _setConnected(false); _disposed = false; + _started = true; _backoff = const Duration(milliseconds: 200); await _connect(); } @@ -74,16 +88,25 @@ class DaemonClient extends ChangeNotifier { Future request( String cmd, { Map args = const {}, - }) { + }) async { if (!_connected || _socket == null) { - return Future.value(IpcResponse.err( - id: '', - error: IpcError( - code: IpcExitCode.toolError, - kind: IpcErrorKind.toolError, - message: 'daemon not connected', - ), - )); + // A connection attempt is in flight (startup or reconnect) — wait + // for it rather than failing instantly, so queries issued during + // the startup window don't get a spurious not-connected error. + // If the client was never started (or is disposed), fail fast. + if (_started && !_disposed) { + await _awaitConnected(_connectWait); + } + if (!_connected || _socket == null) { + return IpcResponse.err( + id: '', + error: IpcError( + code: IpcExitCode.toolError, + kind: IpcErrorKind.toolError, + message: 'daemon not connected', + ), + ); + } } final id = '${_nextId++}'; final completer = Completer(); @@ -93,8 +116,33 @@ class DaemonClient extends ChangeNotifier { return completer.future; } + /// Complete when the socket connects, or after [timeout] (whichever + /// first). Returns immediately if already connected. + Future _awaitConnected(Duration timeout) async { + if (_connected) return; + final c = Completer(); + _connectWaiters.add(c); + try { + await c.future.timeout(timeout); + } on TimeoutException { + _connectWaiters.remove(c); + } + } + + void _wakeConnectWaiters() { + if (_connectWaiters.isEmpty) return; + final waiters = List>.from(_connectWaiters); + _connectWaiters.clear(); + for (final c in waiters) { + if (!c.isCompleted) c.complete(); + } + } + Future _connect() async { - if (_disposed) return; + // Already connected? Don't open a second socket. Guards against + // racing connect attempts (e.g. start() arming the reconnect loop + // while swapIpcServer's reconnectAt connects on first boot). + if (_disposed || _connected) return; try { final addr = InternetAddress(_socketPath, type: InternetAddressType.unix); final socket = await Socket.connect(addr, 0); @@ -173,6 +221,10 @@ class DaemonClient extends ChangeNotifier { void _setConnected(bool v) { if (_connected == v) return; _connected = v; + // Release any requests parked waiting for the connection — on a + // successful connect they proceed to send; this runs before the + // dispose guard so a connect always wakes them. + if (v) _wakeConnectWaiters(); // Skip side-effects (event emit + notifyListeners) after dispose — // the socket stream's onDone can fire post-dispose and would // otherwise hit ChangeNotifier's "used after disposed" assert. @@ -184,10 +236,12 @@ class DaemonClient extends ChangeNotifier { @override void dispose() { _disposed = true; + _started = false; _reconnectTimer?.cancel(); unawaited(_socket?.close()); _socket = null; _failPending('client disposed'); + _wakeConnectWaiters(); super.dispose(); } } diff --git a/lib/main.dart b/lib/main.dart index 7e3ac9e4..4460df7f 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -99,6 +99,20 @@ Future main() async { Future swapIpcServer(DaemonDispatcher dispatcher, Directory workRoot) async { if (kIsWeb) return; + // Already serving this exact workspace? Reuse the live server. + // The startup factory binds the launch CWD, then the project-open + // flow fires for (usually) that same path — tearing the server + // down and rebinding would drop every live connection (the UI's + // DaemonClient, the Claude pane's spawn gate fires right on + // ProjectOpened) for no gain, leaving panes stranded. A genuine + // project switch (different path) falls through and rebinds. + final live = ipcServer; + if (live != null && live.isRunning && live.workspaceRoot == workRoot.path) { + ipcLog.info('ipc', 'already serving ${workRoot.path}; reusing the live server'); + // Idempotent — a no-op when the client is already connected here. + await ipcClient?.reconnectAt(live.socketPath); + return; + } try { await ipcServer?.stop(); } catch (e, st) { @@ -188,10 +202,14 @@ Future main() async { events: events, ); ipcClient = client; - unawaited(() async { - await swapIpcServer(dispatcher, workRoot); - await client.start(); - }()); + // start() synchronously marks the client "connecting" (so + // requests issued during the startup window park for the + // socket instead of failing) and arms the reconnect loop. + // swapIpcServer then binds the server and reconnectAt makes + // the connect immediate. _connect's already-connected guard + // keeps these two paths from opening a second socket. + unawaited(client.start()); + unawaited(swapIpcServer(dispatcher, workRoot)); return client; }, onProjectOpen: kIsWeb diff --git a/test/kernel/src/ipc/client_test.dart b/test/kernel/src/ipc/client_test.dart index 5283d5d6..474c74d3 100644 --- a/test/kernel/src/ipc/client_test.dart +++ b/test/kernel/src/ipc/client_test.dart @@ -131,8 +131,8 @@ void main() { }); group('DaemonClient — error + lifecycle paths', () { - test('request while disconnected returns a not-connected error', () async { - // Don't connect — point at a non-existent socket path. + test('request while disconnected (never started) returns a not-connected error fast', () async { + // Never started → no connect attempt in flight → fail fast (no wait). final bus = DaemonBus(); addTearDown(bus.dispose); final client = _build('/tmp/does-not-exist.sock', bus); @@ -142,6 +142,37 @@ void main() { expect(resp.error?.message, contains('not connected')); }); + test('a request issued before connect waits, then sends once connected', () async { + // Reproduces the startup race: the UI queries before the socket + // finishes connecting. Started (so a connect is in flight) but no + // server yet — the request must park, not fail, and send once the + // server comes up. + final path = await _tmpSocket(); + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build(path, bus); + addTearDown(client.dispose); + + await client.start(); // no server yet → connect fails, reconnect armed + expect(client.isConnected, isFalse); + + final respFuture = client.request('ping'); // parks (does not fail) + + // Bring the server up; the reconnect loop connects, waking the request. + final daemon = _TestDaemon(path); + await daemon.start(); + addTearDown(daemon.close); + + final line = await daemon.lines.first; + expect(line, contains('ping')); + final req = IpcMessage.decode(line) as IpcRequest; + daemon.send(IpcResponse.ok(id: req.id, data: const {'pong': true}).encode()); + + final resp = await respFuture; + expect(resp.ok, isTrue); + expect(resp.data['pong'], isTrue); + }); + test('malformed line is logged and skipped, real lines still work', () async { final path = await _tmpSocket(); final daemon = _TestDaemon(path);