From 36f4561ca182f379ed287020b30659dc3a5e68ca Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 10 Jun 2026 18:57:17 +0200 Subject: [PATCH] expand PATH on Linux so desktop-launched clide finds pql (T-347) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A desktop launcher gives the app a minimal PATH (e.g. /usr/bin:/bin) with no ~/.local/bin, where pql installs — so _findOnPath('pql') returned null, clide spawned the literal 'pql', and Process.start failed with ENOENT; the pql pane errored. The PATH re-expansion that re-adds ~/.local/bin + /usr/local/bin ran on macOS only; Linux GUI launches hit the same wall. Extend it to Linux (homebrew dirs stay macOS-only). Extract the logic into a pure expandToolPath() so the platform gating is unit-tested. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 23 +++++++++++++++++ CHANGELOG.md | 4 +++ lib/kernel/src/toolchain_paths.dart | 29 ++++++++++++++++------ test/kernel/src/toolchain_paths_test.dart | 30 +++++++++++++++++++++++ 5 files changed, 80 insertions(+), 7 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 136207b8..89a0d16a 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3507,3 +3507,4 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang 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 ('06FB3DWCJSGZH9WYDNFWZBAYYR', 'status', 'backlog', 'ready', NULL, '2026-06-10 16:22:28', '2026-06-10 16:22:28', '2026-06-10 16:22:28', NULL, '4b58326ae217b60be81a07603afee36d', 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 ('06FB2T11GCV1EV07DYD5BZENTM', 'status', 'backlog', 'ready', NULL, '2026-06-10 16:22:37', '2026-06-10 16:22:37', '2026-06-10 16:22:37', NULL, 'c62c5ac18a88ba763b5194f89a2a7482', 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 ('06FB4RD1DDSYM4J7WYEGTXARB4', 'status', 'backlog', 'done', NULL, '2026-06-10 16:34:51', '2026-06-10 16:34:51', '2026-06-10 16:34:51', NULL, '56cde33409eb796ad4fb420ed3033c70', 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 ('06FB4XCM5KBXDDSCWJ37GPYG3R', 'status', 'backlog', 'done', NULL, '2026-06-10 16:57:07', '2026-06-10 16:57:07', '2026-06-10 16:57:07', NULL, '1ce291d54831d3ede687384037c379ff', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 9b623e54..30fbc9ff 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -3174,3 +3174,26 @@ Fix: give each run its own coverage output via flutter test --coverage-path , with a per-run mktemp dir for the two passes + the merge; only the final merged result lands in coverage/lcov.info via an atomic rename within coverage/. No shared intermediate paths, so concurrent runs can''t corrupt each other.', 'done', 'medium', NULL, NULL, NULL, '2026-06-10 16:33:23', '2026-06-10 16:34:51', NULL, '75938b68f10edf857e25100141d08544', 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 ('06FB4VG3N3YJSV8G7M1HFSYW2W', 'task', NULL, 'Profile and speed up the push-check gate', 'The push gate (make push-check) is dominated by ''flutter test --coverage'' (coverage instrumentation ~doubles flutter-test wall-time vs the ~20s no-coverage ''make test''). Recent slowness was traced to concurrent-session contention (fixed by T-345), not suite bloat — but there is real speedup headroom. + +Findings (2026-06-10): +- 245 test files; the 39 Flutter-free core files (test/ipc, daemon, git, panes, files, editor, pql) run TWICE in the gate: under flutter test --coverage (for the coverage floor) AND under dart test (test-core, the Flutter-free contract guard). Each serves a purpose but it is the clearest redundancy. +- 33 test files use pumpAndSettle (ci/test.sh''s own comment warns against it; prefer the pumpAsync helper) — a common wall-time/hang sink. +- 47 ''Duration(seconds:)'' occurrences in tests; some are legit timeouts, some are real-time waits that add wall-clock. + +Scope: +1. Profile: one timed run (JSON reporter / --reporter) to rank the ~20 slowest tests; the pumpAndSettle + Duration(seconds:) files are prime suspects. +2. Fix the genuinely slow ones (bounded pumps, fake-async, no real sleeps) per feedback_tests_run_light + feedback_runasync_for_io. +3. Investigate collapsing the core double-run — collect coverage from the dart test core pass and drop those 39 files from the flutter --coverage pass — only if the profile shows that slice is large enough to justify the refactor. + +Do NOT cut tests for speed: 245 files for a public tool with a 95% floor is comprehensive, not over-tested. Make slow tests fast instead.', 'backlog', 'medium', NULL, NULL, NULL, '2026-06-10 16:46:54', '2026-06-10 16:46:54', NULL, '481a097650e0b1badea0b24cdf48afda', 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 ('06FB4XCM5KBXDDSCWJ37GPYG3R', 'bug', NULL, 'pql pane fails in desktop-launched clide: PATH not expanded on Linux', 'The pql sidebar pane (and any PATH-resolved tool) fails when clide is launched from a desktop launcher rather than a terminal. lib/kernel/src/toolchain_paths.dart _expandedPath() augments PATH with ~/.local/bin + /usr/local/bin etc. ONLY on macOS (line: ''if (!Platform.isMacOS) return base;''). On Linux it uses the raw inherited PATH. + +A GUI/desktop-launched app inherits a minimal PATH (e.g. /usr/bin:/bin) with no ~/.local/bin, so _findOnPath(''pql'') returns null, clide falls back to spawning the literal ''pql'', and Process.start fails with ENOENT — the pql pane errors. pql is installed to ~/.local/bin. Reproduced: ''env -i PATH=/usr/bin:/bin command -v pql'' → not found. Works under ''make run'' only because the terminal PATH includes ~/.local/bin. + +Fix: extend _expandedPath() to also augment on Linux — prepend ~/.local/bin and /usr/local/bin (keep the homebrew dirs macOS-only). Affects pql, tmux, and PATH-resolved git alike. User running the installed build needs a rebuild+reinstall after the fix; immediate workaround is launching clide from a terminal.', 'backlog', 'high', NULL, NULL, NULL, '2026-06-10 16:55:10', '2026-06-10 16:55:10', NULL, '4a647a5c33b044df6ace163651fc8e93', 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 ('06FB4XCM5KBXDDSCWJ37GPYG3R', 'bug', NULL, 'pql pane fails in desktop-launched clide: PATH not expanded on Linux', 'The pql sidebar pane (and any PATH-resolved tool) fails when clide is launched from a desktop launcher rather than a terminal. lib/kernel/src/toolchain_paths.dart _expandedPath() augments PATH with ~/.local/bin + /usr/local/bin etc. ONLY on macOS (line: ''if (!Platform.isMacOS) return base;''). On Linux it uses the raw inherited PATH. + +A GUI/desktop-launched app inherits a minimal PATH (e.g. /usr/bin:/bin) with no ~/.local/bin, so _findOnPath(''pql'') returns null, clide falls back to spawning the literal ''pql'', and Process.start fails with ENOENT — the pql pane errors. pql is installed to ~/.local/bin. Reproduced: ''env -i PATH=/usr/bin:/bin command -v pql'' → not found. Works under ''make run'' only because the terminal PATH includes ~/.local/bin. + +Fix: extend _expandedPath() to also augment on Linux — prepend ~/.local/bin and /usr/local/bin (keep the homebrew dirs macOS-only). Affects pql, tmux, and PATH-resolved git alike. User running the installed build needs a rebuild+reinstall after the fix; immediate workaround is launching clide from a terminal.', 'done', 'high', NULL, NULL, NULL, '2026-06-10 16:55:10', '2026-06-10 16:57:07', NULL, '98d93fef85789907f6bf6efc52a3fcdb', 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 ebc672b1..07d49142 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- **Tools like `pql` resolve when clide is launched from the desktop on Linux.** + A desktop launch inherits a minimal PATH without `~/.local/bin`, so the pql + pane (and other PATH-resolved tools) failed — the PATH expansion that fixes + this previously ran on macOS only. It now also runs on Linux. (T-347) - **Consistent card font sizes in the Claude conversation.** Tool/result cards and the Activity/run collapser cards now share the same header-label (14) and collapsed-summary (13) sizes, so neighbouring cards in the stream no longer diff --git a/lib/kernel/src/toolchain_paths.dart b/lib/kernel/src/toolchain_paths.dart index 9a37c410..9d5ca735 100644 --- a/lib/kernel/src/toolchain_paths.dart +++ b/lib/kernel/src/toolchain_paths.dart @@ -145,14 +145,29 @@ String? _firstExisting(List candidates) { } /// Build expanded PATH inline — must be self-contained for isolate use. -String _expandedPath() { - final base = Platform.environment['PATH'] ?? ''; - if (!Platform.isMacOS) return base; - final home = Platform.environment['HOME'] ?? ''; +String _expandedPath() => expandToolPath( + Platform.environment['PATH'] ?? '', + isMac: Platform.isMacOS, + isLinux: Platform.isLinux, + home: Platform.environment['HOME'], + ); + +/// Pure PATH-expansion logic, extracted so it's testable without touching the +/// process environment. +/// +/// A desktop-launched app (macOS or Linux) inherits a minimal PATH that lacks +/// the user bin dirs where tools like `pql` install (`~/.local/bin`), so tool +/// resolution fails even though a terminal launch would find them. Re-add the +/// common user/local bin dirs — that any are missing means they're prepended, +/// so they take precedence over a stale system copy (T-347). Homebrew dirs are +/// macOS-only. On other platforms the base PATH passes through unchanged. +String expandToolPath(String base, {required bool isMac, required bool isLinux, String? home}) { + if (!isMac && !isLinux) return base; + final h = home ?? ''; final extras = [ - if (home.isNotEmpty) '$home/.local/bin', - '/opt/homebrew/bin', - '/opt/homebrew/sbin', + if (h.isNotEmpty) '$h/.local/bin', + if (isMac) '/opt/homebrew/bin', + if (isMac) '/opt/homebrew/sbin', '/usr/local/bin', ]; final existing = base.split(':').toSet(); diff --git a/test/kernel/src/toolchain_paths_test.dart b/test/kernel/src/toolchain_paths_test.dart index 924cfbc2..88913244 100644 --- a/test/kernel/src/toolchain_paths_test.dart +++ b/test/kernel/src/toolchain_paths_test.dart @@ -47,4 +47,34 @@ void main() { expect(v.allOk, isFalse); }); }); + + group('expandToolPath (T-347)', () { + const minimal = '/usr/bin:/bin'; // a desktop-launch PATH, no ~/.local/bin + + test('Linux prepends ~/.local/bin + /usr/local/bin (desktop-launch fix)', () { + final out = expandToolPath(minimal, isMac: false, isLinux: true, home: '/home/u'); + expect(out, '/home/u/.local/bin:/usr/local/bin:/usr/bin:/bin'); + // No homebrew dirs on Linux. + expect(out.contains('/opt/homebrew'), isFalse); + }); + + test('macOS also adds the homebrew dirs', () { + final out = expandToolPath(minimal, isMac: true, isLinux: false, home: '/Users/u'); + expect(out.split(':'), containsAll(['/Users/u/.local/bin', '/opt/homebrew/bin', '/opt/homebrew/sbin', '/usr/local/bin'])); + }); + + test('does not duplicate dirs already on PATH', () { + final base = '/home/u/.local/bin:/usr/local/bin:/usr/bin'; + expect(expandToolPath(base, isMac: false, isLinux: true, home: '/home/u'), base); + }); + + test('skips ~/.local/bin when HOME is empty', () { + final out = expandToolPath(minimal, isMac: false, isLinux: true, home: ''); + expect(out, '/usr/local/bin:/usr/bin:/bin'); + }); + + test('other platforms pass PATH through unchanged', () { + expect(expandToolPath(minimal, isMac: false, isLinux: false, home: '/home/u'), minimal); + }); + }); }