From 6af980f31c6144ab311ef784fad5bdb39e142a5e Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sat, 27 Jun 2026 08:11:55 +0200 Subject: [PATCH] fix(cli): honor CLIDE_SOCK as an explicit target, fail loudly if dead (T-247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `clide` shell client only ever computed the socket path from the workspace's FNV-1a hash — it never read CLIDE_SOCK, so the var the app exports to spawned agents was a no-op and a bogus CLIDE_SOCK was silently ignored (it still hit the discovered instance). Now: when CLIDE_SOCK is set the client connects to it directly, beating discovery, and a dead socket aborts with EX_UNAVAILABLE rather than falling back to a different instance (the split-brain footgun). Unset → the deterministic per-workspace path (D-70) as before. The server keeps deterministic binding; this is a client-side targeting override only. Also fixes a pre-existing -Wstringop-truncation warning in find_workspace_root (strncpy+manual-null → snprintf). Tests: the e2e suite now clears the inherited CLIDE_SOCK so discovery tests stay hermetic (the suite may run inside a clide instance), plus two new tests — a valid CLIDE_SOCK pins from a non-git dir, a dead one fails loudly and returns no data. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 10 ++++++ CHANGELOG.md | 5 +++ native/clide-cli/clide.c | 37 +++++++++++++------ test/cli/clide_cli_e2e_test.dart | 43 +++++++++++++++++++++-- 5 files changed, 82 insertions(+), 14 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 8106366b..64c372a3 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -6784,3 +6784,4 @@ is enough. Removed the Team-tab account card (and its usage prop + i18n keys); the budget now lives only on the Activity tab, next to the /usage refresh control that fetches it (T-415). The three-tab card facelift stays. Q-34 answered: budget surfaces once, on Activity.', NULL, '2026-06-27 05:55:56', '2026-06-27 05:55:56.468', '2026-06-27 05:55:56.468', NULL, 'ba7329e54bf44c2cb5a9e7fb63e685f3', 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 ('06FB0TNQM6QCYT7JBF352YVJ48', 'status', 'backlog', 'in_progress', NULL, '2026-06-27 06:06:03', '2026-06-27 06:06:03.770', '2026-06-27 06:06:03.770', NULL, 'c1cff9a2f682f3bb8f719f6771cf36c3', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index dae98b73..cd442502 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -8436,3 +8436,13 @@ is enough. Removed the Team-tab account card (and its usage prop + i18n keys); the budget now lives only on the Activity tab, next to the /usage refresh control that fetches it (T-415). The three-tab card facelift stays. Q-34 answered: budget surfaces once, on Activity.', 'done', 'low', NULL, NULL, NULL, '2026-05-23 20:48:38', '2026-06-27 05:55:56.468', NULL, 'c69aaa48a145eccdaed208eb01ab8a57', 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 ('06FB0TNQM6QCYT7JBF352YVJ48', 'bug', NULL, 'clide CLI ignores CLIDE_SOCK and auto-discovers; stale sockets orphaned in runtime dir', 'The `clide` CLI does not honor the `CLIDE_SOCK` env var — it auto-discovers a running instance instead. Proof: with `CLIDE_SOCK=/run/user/1000/clide/DOES_NOT_EXIST.sock`, `clide version` still returns `{"version":"2.1.0"}` from the live app. Separately, the runtime socket dir accumulates orphaned socket files: `/run/user/1000/clide/` held two `.sock` entries while only one GUI process (the `build/linux/x64/debug/bundle/clide` bundle) was running, so a previous run''s socket was never cleaned up. + +Why it matters: today (single instance) it''s harmless, but it''s a latent split-brain + observability hole. If two clide instances are ever live on the same machine, the CLI attaches to whichever discovery resolves first, with (a) no way to target a specific instance and (b) no way to find out which one you''re talking to. Combined with the stale-socket litter, `clide` could silently drive the wrong window. This is a D-6 surface gap — the CLI must be able to address the same instance the user is in. + +Fix (scope to confirm): +- Honor `CLIDE_SOCK` when set (explicit target beats discovery); error clearly if that socket is dead rather than silently falling back. +- Clean up orphaned/stale socket files on app startup (and on clean shutdown) — detect a dead listener and unlink before binding a new hash. +- Add a way to enumerate/identify live instances (e.g. a `clide instances` verb, or include the instance id/socket path + pid in `clide version`) so a human or agent can pick the right one. + +Acceptance: a bogus `CLIDE_SOCK` fails loudly instead of returning data from a different instance; a valid `CLIDE_SOCK` pins the CLI to that instance; startup leaves exactly one live socket for one running app (no orphan accumulation); there is a CLI affordance to list/identify running instances.', 'in_progress', 'medium', NULL, NULL, 'D-6', '2026-06-05 21:28:48', '2026-06-27 06:06:03.770', NULL, '15ee4bd8e47ee2b0c1d2919fc2e3e013', 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/CHANGELOG.md b/CHANGELOG.md index cb91ffcb..c722e5c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- **`clide` CLI honors `CLIDE_SOCK`.** The shell client now connects to the + socket named by `CLIDE_SOCK` when set — an explicit target (e.g. a spawned + agent pinning its parent instance) that beats workspace auto-discovery — and + fails loudly if that socket is dead instead of silently driving a different + instance. (T-247) - **Default window opens larger (1600×900) on Linux.** 720p was short enough that the welcome screen's version/theme footer overlapped the tips card; the taller default clears it, matching the macOS default. (T-477) diff --git a/native/clide-cli/clide.c b/native/clide-cli/clide.c index 2162d2cc..ca22ec63 100644 --- a/native/clide-cli/clide.c +++ b/native/clide-cli/clide.c @@ -183,8 +183,7 @@ static int find_workspace_root(const char *start, char *out, size_t out_size) { snprintf(probe, sizeof(probe), "%s/.git", cwd); struct stat st; if (lstat(probe, &st) == 0) { - strncpy(out, cwd, out_size - 1); - out[out_size - 1] = '\0'; + snprintf(out, out_size, "%s", cwd); return 0; } /* Climb one. /foo/bar -> /foo, / -> stop. */ @@ -367,16 +366,32 @@ int main(int argc, char **argv) { return EX_USAGE; } - char ws_root[4096]; - if (find_workspace_root(NULL, ws_root, sizeof(ws_root)) != 0) { - fprintf(stderr, "clide: not inside a git repository — no workspace to talk to\n"); - return EX_USAGE; - } - + /* CLIDE_SOCK is an explicit target that beats workspace discovery (T-247): + * a spawned agent inherits the parent app's socket path here, and a human + * can pin a specific instance. When it's set we connect to it and FAIL + * LOUDLY if it's dead — never silently fall back to discovering a different + * instance (that's the split-brain footgun this fixes). Unset → the + * deterministic per-workspace path (D-70). */ char sock_path[4096]; - if (socket_path_for(ws_root, sock_path, sizeof(sock_path)) >= (int)sizeof(sock_path)) { - fprintf(stderr, "clide: socket path overflow\n"); - return EX_SOFTWARE; + const char *env_sock = getenv("CLIDE_SOCK"); + if (env_sock && *env_sock) { + if (strlen(env_sock) >= sizeof(sock_path)) { + fprintf(stderr, "clide: CLIDE_SOCK path too long\n"); + return EX_USAGE; + } + strncpy(sock_path, env_sock, sizeof(sock_path) - 1); + sock_path[sizeof(sock_path) - 1] = '\0'; + } else { + char ws_root[4096]; + if (find_workspace_root(NULL, ws_root, sizeof(ws_root)) != 0) { + fprintf(stderr, "clide: not inside a git repository — no workspace to talk to " + "(set CLIDE_SOCK to target a specific instance)\n"); + return EX_USAGE; + } + if (socket_path_for(ws_root, sock_path, sizeof(sock_path)) >= (int)sizeof(sock_path)) { + fprintf(stderr, "clide: socket path overflow\n"); + return EX_SOFTWARE; + } } sock_t fd = connect_unix(sock_path); diff --git a/test/cli/clide_cli_e2e_test.dart b/test/cli/clide_cli_e2e_test.dart index 74146bd2..1fa35e5c 100644 --- a/test/cli/clide_cli_e2e_test.dart +++ b/test/cli/clide_cli_e2e_test.dart @@ -71,8 +71,12 @@ void main() { } }); + // Clear any inherited CLIDE_SOCK so these tests exercise workspace discovery + // hermetically — the suite may run *inside* a clide instance, which exports + // CLIDE_SOCK to child processes (T-247). Empty string reads as unset to the + // client, which then falls back to per-workspace discovery. Future runCli(List argv) { - return Process.run(binaryPath, argv, workingDirectory: workspaceRoot.path); + return Process.run(binaryPath, argv, workingDirectory: workspaceRoot.path, environment: const {'CLIDE_SOCK': ''}); } group('clide-cli (T-126)', () { @@ -93,7 +97,7 @@ void main() { } final outside = Directory.systemTemp.createTempSync('clide-no-git-'); addTearDown(() => outside.deleteSync(recursive: true)); - final r = await Process.run(binaryPath, ['status'], workingDirectory: outside.path); + final r = await Process.run(binaryPath, ['status'], workingDirectory: outside.path, environment: const {'CLIDE_SOCK': ''}); expect(r.exitCode, 64); expect(r.stderr.toString(), contains('git repository')); }); @@ -141,7 +145,12 @@ void main() { markTestSkipped('cc not available'); return; } - final proc = await Process.start(binaryPath, ['tail', '--events', '--filter', 'pane'], workingDirectory: workspaceRoot.path); + final proc = await Process.start( + binaryPath, + ['tail', '--events', '--filter', 'pane'], + workingDirectory: workspaceRoot.path, + environment: const {'CLIDE_SOCK': ''}, + ); addTearDown(() => proc.kill()); final lines = []; final sub = proc.stdout.transform(utf8.decoder).transform(const LineSplitter()).listen(lines.add); @@ -166,5 +175,33 @@ void main() { expect(concatenated, contains('"kind":"spawned"')); expect(concatenated, contains('"kind":"closed"')); }); + + test('CLIDE_SOCK pins to that instance, beating workspace discovery (T-247)', () async { + if (!hasCC) { + markTestSkipped('cc not available'); + return; + } + // From a NON-git dir (where discovery would fail), an explicit CLIDE_SOCK + // still connects — it's the explicit target. + final outside = Directory.systemTemp.createTempSync('clide-sock-'); + addTearDown(() => outside.deleteSync(recursive: true)); + final r = await Process.run(binaryPath, ['ping'], workingDirectory: outside.path, environment: {'CLIDE_SOCK': server.socketPath}); + expect(r.exitCode, 0, reason: 'stderr: ${r.stderr}'); + expect((jsonDecode(r.stdout.toString().trim()) as Map)['pong'], isTrue); + }); + + test('a dead CLIDE_SOCK fails loudly, never falling back to discovery (T-247)', () async { + if (!hasCC) { + markTestSkipped('cc not available'); + return; + } + // Run from the REAL workspace — discovery WOULD succeed — to prove the + // bogus explicit target aborts instead of silently hitting another instance. + final bogus = '${workspaceRoot.path}/DOES_NOT_EXIST.sock'; + final r = await Process.run(binaryPath, ['ping'], workingDirectory: workspaceRoot.path, environment: {'CLIDE_SOCK': bogus}); + expect(r.exitCode, isNot(0)); + expect(r.stderr.toString(), contains('cannot connect')); + expect(r.stdout.toString().trim(), isEmpty, reason: 'must not return data from a different instance'); + }); }); }