From 70c293b16324f758a409a88983d26257540b224d Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 19 May 2026 12:03:44 +0200 Subject: [PATCH] T-127: replace InProcessClient with socket loopback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth slice of T-99. The UI's DaemonClient now talks to its own IpcServer through the same per-workspace Unix socket the C `clide` client uses — one transport, one wire contract, no second path through the dispatch tree. Changes: * lib/kernel/src/ipc/in_process.dart deleted. Nothing imports it. * DaemonClient.socketPath becomes mutable + new `reconnectAt(path)` method swaps an active client onto a different socket without restart. Project switch in main.dart uses it — the dispatcher + IpcServer are rebuilt for the new workspace, and the client reconnects to the new path. * main.dart's daemonClientFactory now builds a real DaemonClient pointed at workspaceSocketPath(workRoot); swapIpcServer kicks off server.start() then client.start() in sequence. * lib/test_app.dart's pane.spawn smoke test uses dispatcher.dispatch directly instead of InProcessClient — same coverage, no dead-end import. * DaemonClient client_test gets a reconnectAt round-trip test. T-128 (delete IsolateClient + Backend + backend_entry.dart) unblocked. Co-Authored-By: Claude --- .pql/changelog/ticket_history/2026-05.sql | 2 ++ .pql/changelog/tickets/2026-05.sql | 10 ++++++ CHANGELOG.md | 10 ++++++ lib/kernel/src/ipc/client.dart | 34 ++++++++++++++++--- lib/kernel/src/ipc/in_process.dart | 29 ---------------- lib/main.dart | 40 +++++++++++++++++------ lib/test_app.dart | 13 +++++--- test/kernel/src/ipc/client_test.dart | 40 +++++++++++++++++++++++ 8 files changed, 130 insertions(+), 48 deletions(-) delete mode 100644 lib/kernel/src/ipc/in_process.dart diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index e5759068..2ab41094 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -1858,3 +1858,5 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, 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-125', 'status', 'in_progress', 'done', NULL, '2026-05-18 15:51:09', '2026-05-18 15:51:09', '2026-05-18 15:51:09', NULL, '938624de487aa2f02e034424cae40c77', 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-126', 'status', 'backlog', 'in_progress', NULL, '2026-05-18 15:54:20', '2026-05-18 15:54:20', '2026-05-18 15:54:20', NULL, '3266e7ea943867f49ff20a90b7f12ae6', 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-126', 'status', 'in_progress', 'done', NULL, '2026-05-18 16:04:29', '2026-05-18 16:04:29', '2026-05-18 16:04:29', NULL, 'b6c783f599585e55ecc36f69ccbcefb1', 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-127', 'status', 'backlog', 'in_progress', NULL, '2026-05-19 09:57:18', '2026-05-19 09:57:18', '2026-05-19 09:57:18', NULL, '64a14cb5d8b9274fd9d72d1e7474040c', 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-127', 'status', 'in_progress', 'done', NULL, '2026-05-19 10:03:26', '2026-05-19 10:03:26', '2026-05-19 10:03:26', NULL, 'b8bf877d2057f7baa5acf6af7a13d529', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index 8db426bf..babb841d 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -2134,3 +2134,13 @@ Acceptance: 5. Documented in assets/licenses.yaml + a one-paragraph note in CONTRIBUTING.md. Source: T-99 sketch. Depends on T-124 (server) + T-125 (argv translator).', 'done', 'high', NULL, NULL, NULL, '2026-05-18 11:58:57', '2026-05-18 16:04:29', NULL, 'e4be31e3d80dbaadd64e3a4a2fa029e0', 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-127', 'task', 'T-99', 'replace InProcessClient call sites with the socket loopback', 'Fourth slice of T-99(a). Convert the in-process callers — KernelServices.boot''s daemonClientFactory and friends — to talk to the local socket instead of the direct in-process dispatch path. Delete InProcessClient afterward. + +Acceptance: +1. lib/kernel/src/ipc/in_process.dart removed; nothing imports it. +2. main.dart''s daemonClientFactory builds a socket-connected DaemonClient pointed at the local server. +3. Test fixtures use a synthetic in-memory socket pair (or short-lived AF_UNIX socket in /tmp) for isolation. +4. Wall-clock perf delta is within reason (no widget rebuild storm; round-trip ~ms) — measure before/after under a representative test. +5. Falls back to in-process direct dispatch if the perf delta is unacceptable; surface the finding in a Q-record before doing so. + +Source: T-99 sketch. Depends on T-124 (server must exist). Blocks T-128 (legacy IPC cleanup).', 'done', 'high', NULL, NULL, NULL, '2026-05-18 11:59:02', '2026-05-19 10:03:26', NULL, 'af4a706880181411b2aecf51d90ea748', 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 c6d50e4d..da073752 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. the umbrella commands (`status`, `tail`, `version`, `ping`) per D-6 into the wire envelope. Pure Dart; lets the C client (T-126) stay a dumb pipe (T-99 / T-125). +- `DaemonClient.reconnectAt(newPath)` — swap an active client onto a + different socket without restart (project switch in T-127). - C `clide` shell client at `native/clide-cli/clide.c`. Walks CWD up to the git root, hashes to the per-workspace socket (D-70), ships argv. `make clide-cli` builds it; on PATH, `clide status` works @@ -43,10 +45,18 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Changed +- In-process IPC dispatch swapped for socket loopback (T-127). The + Flutter UI's `DaemonClient` now talks to its own `IpcServer` over + the same per-workspace Unix socket the C `clide` client uses — one + transport, one contract. + ### Deprecated ### Removed +- `lib/kernel/src/ipc/in_process.dart` (`InProcessClient`) — replaced + by the socket-loopback `DaemonClient` (T-127). + ### Fixed ### Security diff --git a/lib/kernel/src/ipc/client.dart b/lib/kernel/src/ipc/client.dart index 67954d49..bcd973bd 100644 --- a/lib/kernel/src/ipc/client.dart +++ b/lib/kernel/src/ipc/client.dart @@ -11,13 +11,15 @@ import 'package:flutter/foundation.dart'; class DaemonClient extends ChangeNotifier { DaemonClient({ - required this.socketPath, + required String socketPath, required Logger log, required DaemonBus events, - }) : _log = log, + }) : _socketPath = socketPath, + _log = log, _events = events; - final String socketPath; + String _socketPath; + String get socketPath => _socketPath; final Logger _log; final DaemonBus _events; @@ -47,6 +49,28 @@ class DaemonClient extends ChangeNotifier { _setConnected(false); } + /// Point the client at a different socket path and reconnect. + /// Used on project switch — the workspace-derived socket path + /// (D-70) changes when the user opens a different project, so the + /// client follows. Cancels the reconnect timer, closes the live + /// socket (failing in-flight requests with `disconnect`), updates + /// the path, and re-arms the connect loop. Idempotent if the new + /// path equals the current one. + Future reconnectAt(String newPath) async { + if (newPath == _socketPath && _connected) return; + _socketPath = newPath; + _reconnectTimer?.cancel(); + _reconnectTimer = null; + final s = _socket; + _socket = null; + await s?.close(); + _failPending('socket path changed'); + _setConnected(false); + _disposed = false; + _backoff = const Duration(milliseconds: 200); + await _connect(); + } + Future request( String cmd, { Map args = const {}, @@ -72,12 +96,12 @@ class DaemonClient extends ChangeNotifier { Future _connect() async { if (_disposed) return; try { - final addr = InternetAddress(socketPath, type: InternetAddressType.unix); + final addr = InternetAddress(_socketPath, type: InternetAddressType.unix); final socket = await Socket.connect(addr, 0); _socket = socket; _backoff = const Duration(milliseconds: 200); _setConnected(true); - _log.info('ipc', 'connected to $socketPath'); + _log.info('ipc', 'connected to $_socketPath'); socket.cast>().transform(utf8.decoder).transform(const LineSplitter()).listen( _handleLine, onDone: _handleDisconnect, diff --git a/lib/kernel/src/ipc/in_process.dart b/lib/kernel/src/ipc/in_process.dart deleted file mode 100644 index b263d1a9..00000000 --- a/lib/kernel/src/ipc/in_process.dart +++ /dev/null @@ -1,29 +0,0 @@ -import 'package:clide/clide.dart'; -import 'package:clide/kernel/src/ipc/client.dart'; - -class InProcessClient extends DaemonClient { - InProcessClient({ - required super.log, - required super.events, - required this.dispatcher, - }) : super(socketPath: ''); - - DaemonDispatcher dispatcher; - int _nextReqId = 0; - - @override - bool get isConnected => true; - - @override - Future start() async {} - - @override - Future stop() async {} - - @override - Future request(String cmd, {Map args = const {}}) { - final id = '${_nextReqId++}'; - final req = IpcRequest(id: id, cmd: cmd, args: args); - return dispatcher.dispatch(req); - } -} diff --git a/lib/main.dart b/lib/main.dart index dd085a9b..956cf064 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -29,7 +29,6 @@ import 'package:clide/builtin/welcome/welcome.dart'; import 'dart:io' show Directory, Platform; import 'package:clide/kernel/kernel.dart'; -import 'package:clide/kernel/src/ipc/in_process.dart'; import 'package:clide/src/daemon/dispatcher.dart'; import 'package:clide/src/daemon/editor_commands.dart'; import 'package:clide/src/daemon/files_commands.dart'; @@ -40,6 +39,7 @@ import 'package:clide/src/editor/registry.dart' show EditorRegistry; import 'package:clide/src/git/client.dart'; import 'package:clide/src/cli/argv_dispatch.dart'; import 'package:clide/src/ipc/envelope.dart'; +import 'package:clide/src/ipc/paths.dart' show workspaceSocketPath; import 'package:clide/src/ipc/server.dart'; import 'package:clide/src/panes/event_sink.dart'; import 'package:clide/src/panes/registry.dart'; @@ -78,11 +78,13 @@ Future main() async { toolchain.applyResolved(resolveToolchainPaths()); } - InProcessClient? ipcClient; + DaemonClient? ipcClient; DaemonBus? daemonBus; // IPC socket server (T-99 / T-124, per D-70/71/72). One server per // workspace; restarted when the active project switches because the - // socket path is workspace-derived. + // socket path is workspace-derived. The local DaemonClient connects + // back to it over the socket so all IPC — including from UI widgets + // in the same process — goes through the wire contract (T-127). IpcServer? ipcServer; final ipcLog = Logger(); @@ -100,6 +102,14 @@ Future main() async { await server.start(); } catch (e, st) { ipcLog.error('ipc', 'server start failed', error: e, stackTrace: st); + return; + } + // Point the in-process DaemonClient at the new socket. On first + // boot (no client yet) the daemonClientFactory below kicks it + // off; on project switch we just reconnect to the new path. + final client = ipcClient; + if (client != null) { + await client.reconnectAt(server.socketPath); } } @@ -133,18 +143,28 @@ Future main() async { daemonBus = events; final workRoot = FilesService.atCwd(events: _BusEventSink(events)).root; final dispatcher = buildDispatcher(events, toolchain, workRoot); - ipcClient = InProcessClient(log: log, events: events, dispatcher: dispatcher); - // Fire-and-forget: bring up the IPC socket server alongside. - // Failure is logged, not fatal — the UI still works. - unawaited(swapIpcServer(dispatcher, workRoot)); - return ipcClient!; + // Build the client at the workspace's socket path. The + // server is started below (swapIpcServer) which the + // client will then auto-connect to via its reconnect + // loop. autoStartDaemonClient:false means we own the + // lifecycle here. + final client = DaemonClient( + socketPath: workspaceSocketPath(workRoot.path), + log: log, + events: events, + ); + ipcClient = client; + unawaited(() async { + await swapIpcServer(dispatcher, workRoot); + await client.start(); + }()); + return client; }, onProjectOpen: kIsWeb ? null : (path) async { - if (ipcClient == null || daemonBus == null) return; + if (daemonBus == null) return; final dispatcher = buildDispatcher(daemonBus!, toolchain, Directory(path)); - ipcClient!.dispatcher = dispatcher; await swapIpcServer(dispatcher, Directory(path)); }, ); diff --git a/lib/test_app.dart b/lib/test_app.dart index 1438081a..c766c982 100644 --- a/lib/test_app.dart +++ b/lib/test_app.dart @@ -29,7 +29,6 @@ import 'dart:ffi' as ffi; import 'package:ffi/ffi.dart' as pkg_ffi; import 'kernel/kernel.dart'; import 'src/pty/ffi/libc.dart' as libc; -import 'kernel/src/ipc/in_process.dart'; import 'src/daemon/pane_commands.dart'; import 'src/ipc/envelope.dart'; import 'src/panes/event_sink.dart'; @@ -325,19 +324,25 @@ class _ClideTestAppState extends State { Future _runTerminalTests(Toolchain tc, String workDir) async { _say('--- terminal ---'); - // Test PTY via InProcessClient — same path as the real app. + // Test PTY via the dispatcher directly — skip the socket + // round-trip for the smoke test since it adds setup without + // testing anything new for pane.spawn. The real app's path is + // covered by the IPC server + client tests under test/ipc/. await _testAsync('pane.spawn via IPC', () async { final dispatcher = DaemonDispatcher(); final bus = DaemonBus(); final eventSink = _TestEventSink(bus); final paneRegistry = PaneRegistry(events: eventSink); registerPaneCommands(dispatcher, paneRegistry); - final ipc = InProcessClient(log: Logger(), events: bus, dispatcher: dispatcher); + + Future dispatch(String cmd, Map args) { + return dispatcher.dispatch(IpcRequest(id: 'tm-${DateTime.now().microsecondsSinceEpoch}', cmd: cmd, args: args)); + } // Spawn a pane running /bin/echo. // Use interactive shell — fast-exiting commands lose output on macOS // because the slave closes before we can read the master. - final spawnResp = await ipc.request('pane.spawn', args: { + final spawnResp = await dispatch('pane.spawn', { 'argv': [tc.shell], 'kind': 'terminal', }); diff --git a/test/kernel/src/ipc/client_test.dart b/test/kernel/src/ipc/client_test.dart index 4330412b..5283d5d6 100644 --- a/test/kernel/src/ipc/client_test.dart +++ b/test/kernel/src/ipc/client_test.dart @@ -276,5 +276,45 @@ void main() { await Future.delayed(const Duration(milliseconds: 100)); expect(flips, contains(true)); }); + + test('reconnectAt swaps socket paths and re-binds (T-127)', () async { + final pathA = await _tmpSocket(); + final daemonA = _TestDaemon(pathA); + await daemonA.start(); + addTearDown(daemonA.close); + + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build(pathA, bus); + addTearDown(client.dispose); + await client.start(); + await daemonA.waitForClient(); + await Future.delayed(const Duration(milliseconds: 50)); + expect(client.isConnected, isTrue); + expect(client.socketPath, pathA); + + // Spin up a SECOND daemon on a different socket and move the + // client over to it. + final pathB = await _tmpSocket(); + final daemonB = _TestDaemon(pathB); + await daemonB.start(); + addTearDown(daemonB.close); + + await client.reconnectAt(pathB); + await daemonB.waitForClient(); + await Future.delayed(const Duration(milliseconds: 50)); + expect(client.socketPath, pathB); + expect(client.isConnected, isTrue); + + // A request goes to the NEW daemon — verify by reading the + // line off daemonB.lines. + final lineFuture = daemonB.lines.first; + final responseFuture = client.request('ping'); + final reqLine = await lineFuture; + final reqJson = jsonDecode(reqLine) as Map; + daemonB.send(IpcResponse.ok(id: reqJson['id'] as String, data: const {'pong': true}).encode()); + final resp = await responseFuture; + expect(resp.ok, isTrue); + }); }); }