diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index 75dd3557..2bae53af 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -1926,3 +1926,31 @@ Acceptance: Source: consultants.md "Security — Findings — [Major]" item 1. Decision: D-74.', NULL, '2026-05-20 15:54:35', '2026-05-20 15:54:35', '2026-05-20 15:54:35', NULL, '7f972739c82134897869432a3978d6ec', 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-120', 'status', 'backlog', 'in_progress', NULL, '2026-05-20 15:54:38', '2026-05-20 15:54:38', '2026-05-20 15:54:38', NULL, 'fa11190ac1bd9cd8f67e3041a8ca7236', 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-120', 'status', 'in_progress', 'done', NULL, '2026-05-20 16:02:35', '2026-05-20 16:02:35', '2026-05-20 16:02:35', NULL, 'efffdd07c7281977d37ce0ad4285a36a', 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-97', 'status', 'backlog', 'done', NULL, '2026-05-20 16:02:46', '2026-05-20 16:02:46', '2026-05-20 16:02:46', NULL, '9c36d26b5ac3702441a87c7f88dbf830', 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', 'description', 'T-115''s render-only widget test at test/builtin/welcome/widget_test.dart ''sticky-startup toggle renders on a seeded recent row'' verifies the toggle is constructed but does not exercise the tap → setStickyStartup path. Adding a tap-driven assertion hung the 10-minute Flutter test timeout consistently (find.byTooltip, find.bySemanticsLabel, and find.byKey + tap variants all reproduced). + +Hypothesis: ClideTooltip wraps the toggle in a MouseRegion whose onEnter awaits Future.delayed(showDelay). Some path during tap simulation (or surrounding pump scheduling) keeps that timer pending, and the test runner waits the full 10-minute idle timeout before declaring it stuck. + +Next steps: +- Reproduce in isolation against a stripped-down harness pumping just the _StickyToggle widget. +- Determine whether the hang is the MouseRegion timer or another future (e.g., the welcome-tab activation, toolchain check). +- Either fix the underlying cause or use fakeAsync.run() to drain the timers explicitly. + +Until then, ProjectManager unit tests in test/kernel/src/project_test.dart cover the sticky-startup logic (14 cases including round-trip, no-op, idempotent flip, ambiguity, and openStickyOrNothing).', 'T-115 follow-up: tap-driven widget test for sticky-startup toggle. + +Adding a tap-driven (or even render-only) test that pumps WelcomeView with a non-empty recents list hangs the Flutter test runner until timeout. + +Investigation 2026-05-22 (narrowed the cause): +- The hang is SYNCHRONOUS: `flutter test --timeout 45s` never fires; only an external `timeout` kills it. So the Dart isolate event loop is blocked, not awaiting. +- Reduced to a minimal repro: pumping two NESTED ClideTappable widgets under the shared test harness (test/helpers/widget_harness.dart) hangs. A SINGLE ClideTappable (as in _ActionRow) renders fine — that is why all existing WelcomeView tests pass (empty recents = no nested tappable). The recents path nests one: _RecentRow is a ClideTappable whose subtree contains _StickyToggle (a Semantics-wrapped ClideTappable). +- The shared harness uses `Overlay(canSizeOverlay: true)` over a zero-size `MediaQuery`, which triggers an intrinsic-sizing pass. Strongly suspect the nested ClideTappable (Focus + MouseRegion + GestureDetector + DecoratedBox stack) diverges under intrinsic dimension computation. +- Workarounds that did NOT help: setting tester.view.physicalSize; wrapping in SizedBox; wrapping in Center+SizedBox; a custom bounded MediaQuery harness (that custom harness hung even on a single tappable — likely its own confound, do not reuse it). +- The prior ClideTooltip/MouseRegion-timer hypothesis is WRONG: the recents widgets pass no tooltip to ClideTappable, so no ClideTooltip is built, and _StickyToggle uses Semantics(tooltip:) (metadata only, no widget/timer). + +Next steps: +- Decide whether this is a real ClideTappable bug (nested tappables would also hang the live app welcome screen with recents) or strictly a canSizeOverlay-intrinsic-sizing test artifact. Check the running app: open welcome with >=1 recent project and confirm it does NOT hang. If the app is fine, the fix is harness-side (give WelcomeView tight constraints so no intrinsic pass), and the shared harness or a welcome-specific harness needs adjusting. If the app DOES hang, ClideTappable has a real nested-layout bug — fix it (likely in lib/widgets/src/clide_tappable.dart intrinsic/layout handling) and this becomes higher priority. +- 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).', 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; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index 0d849837..d27cef88 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -2240,3 +2240,42 @@ Acceptance: 5. Tests cover: schema accept/reject per constraint kind, dispatcher-level rejection, and that an unschema-d command still dispatches. Source: consultants.md "Security — Findings — [Major]" item 1. Decision: D-74.', 'in_progress', 'medium', NULL, NULL, NULL, '2026-05-18 07:53:08', '2026-05-20 15:54:38', NULL, '3c33fccd4b7442444ca598d430fab7f8', 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-120', 'task', 'T-97', 'typed IPC command schema framework (split from T-104)', 'T-104 covered the spot-fixes (argv-injection rejection, size/count caps on specific commands). What remains is the framework piece: a typed schema per IPC command — branch/remote/path/etc. with regex/charset constraints — applied at DaemonDispatcher dispatch time rather than scattered through individual handlers. + +--- +Design resolved 2026-05-20 (see D-74). Co-registration model, NOT a central map: + +- The argv parser (lib/src/cli/argv_to_request.dart) only knows syntactic shape (identifier/flag charset, -- terminator); it has NO per-command argument knowledge. So this is build-fresh, not a lift — each handler currently hand-reads its own keys. +- Extend the DaemonDispatcher registration API to carry an optional typed schema per command. The dispatcher accumulates a cmd->schema registry as commands register (built-in register*Commands modules supply theirs; extension CommandContributions carry their own — central static map rejected because it cannot see extension-contributed commands, D-46). +- DaemonDispatcher.dispatch validates req.args against schema[cmd] BEFORE invoking the handler (hook at dispatcher.dart line 39, the `return h(req)` call). Violation -> userError (sysexit 64). Commands with no schema dispatch unvalidated (opt-in per command) until migrated. +- Constraint vocabulary is hand-rolled (prefer-zero-deps): per-arg required/optional, kind, charset/regex, numeric range. Collapses the _ResizeArgs-style hand-lifts (panel_commands.dart, T-119) into schema. +- KEEP existing T-104 checks as defense-in-depth: validateGitRef (lib/src/git/operations.dart:46) and the count/path caps (lib/src/daemon/git_commands.dart:18,23). The git client is callable directly from the Flutter UI, not only via the dispatcher — do not remove them. +- OUT OF SCOPE: MCP tools/list generation from the registry — deferred to the T-130 track. + +Acceptance: +1. Dispatcher validates req.args against a registered per-command schema before the handler runs; violations return userError. +2. At least the security-relevant commands (git ref/remote/path args, panel.resize) carry schemas; argv-injection + caps still enforced (no regression vs T-104). +3. validateGitRef + git caps retained as defense-in-depth. +4. No third-party validation dep; no lint suppressions. +5. Tests cover: schema accept/reject per constraint kind, dispatcher-level rejection, and that an unschema-d command still dispatches. + +Source: consultants.md "Security — Findings — [Major]" item 1. Decision: D-74.', 'done', 'medium', NULL, NULL, NULL, '2026-05-18 07:53:08', '2026-05-20 16:02:35', NULL, 'f52928536cfeb0f08091ec5d55bda3db', 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-97', 'epic', NULL, 'Address 2026-05-14 consultant review', 'Six-reviewer external assessment found four critical gaps (workspace RCE, IPC server unimplemented, keyboard not operable, onboarding docs describe a dissolved architecture) plus ten major items and a handful of quick wins. Source: consultants.md (committed alongside this epic). Children are filed individually so they can land independently. + +Acceptance: all critical findings resolved or formally rejected with a D-record; all major findings either shipped or moved to a follow-up epic with rationale; quick-win batches closed.', 'done', 'high', NULL, NULL, NULL, '2026-05-17 18:47:08', '2026-05-20 16:02:46', NULL, 'bfc0abf31809a8e7afbb4bff62118d06', 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-122', 'task', 'T-115', 'T-115 follow-up: tap-driven widget test for sticky-startup toggle', 'T-115 follow-up: tap-driven widget test for sticky-startup toggle. + +Adding a tap-driven (or even render-only) test that pumps WelcomeView with a non-empty recents list hangs the Flutter test runner until timeout. + +Investigation 2026-05-22 (narrowed the cause): +- The hang is SYNCHRONOUS: `flutter test --timeout 45s` never fires; only an external `timeout` kills it. So the Dart isolate event loop is blocked, not awaiting. +- Reduced to a minimal repro: pumping two NESTED ClideTappable widgets under the shared test harness (test/helpers/widget_harness.dart) hangs. A SINGLE ClideTappable (as in _ActionRow) renders fine — that is why all existing WelcomeView tests pass (empty recents = no nested tappable). The recents path nests one: _RecentRow is a ClideTappable whose subtree contains _StickyToggle (a Semantics-wrapped ClideTappable). +- The shared harness uses `Overlay(canSizeOverlay: true)` over a zero-size `MediaQuery`, which triggers an intrinsic-sizing pass. Strongly suspect the nested ClideTappable (Focus + MouseRegion + GestureDetector + DecoratedBox stack) diverges under intrinsic dimension computation. +- Workarounds that did NOT help: setting tester.view.physicalSize; wrapping in SizedBox; wrapping in Center+SizedBox; a custom bounded MediaQuery harness (that custom harness hung even on a single tappable — likely its own confound, do not reuse it). +- The prior ClideTooltip/MouseRegion-timer hypothesis is WRONG: the recents widgets pass no tooltip to ClideTappable, so no ClideTooltip is built, and _StickyToggle uses Semantics(tooltip:) (metadata only, no widget/timer). + +Next steps: +- Decide whether this is a real ClideTappable bug (nested tappables would also hang the live app welcome screen with recents) or strictly a canSizeOverlay-intrinsic-sizing test artifact. Check the running app: open welcome with >=1 recent project and confirm it does NOT hang. If the app is fine, the fix is harness-side (give WelcomeView tight constraints so no intrinsic pass), and the shared harness or a welcome-specific harness needs adjusting. If the app DOES hang, ClideTappable has a real nested-layout bug — fix it (likely in lib/widgets/src/clide_tappable.dart intrinsic/layout handling) and this becomes higher priority. +- 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); diff --git a/test/extension/src/extension_context_test.dart b/test/extension/src/extension_context_test.dart new file mode 100644 index 00000000..796ea426 --- /dev/null +++ b/test/extension/src/extension_context_test.dart @@ -0,0 +1,102 @@ +/// Covers the `ClideExtensionContext` sugar extensions (publish / +/// subscribe / t / tr) and the default no-op `ClideExtension` +/// lifecycle hooks, driving the *real* context the ExtensionManager +/// hands to `activate()`. +library; + +import 'package:clide/extension/extension.dart'; +import 'package:clide/kernel/kernel.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../../helpers/kernel_fixture.dart'; + +/// Extension that exercises the context sugar inside activate(). +class _SugarExt extends ClideExtension { + _SugarExt(this.onActivate); + final Future Function(ClideExtensionContext ctx) onActivate; + @override + String get id => 'sugar.ext'; + @override + String get title => 'Sugar'; + @override + String get version => '0.0.0-test'; + @override + List get contributions => const []; + @override + Future activate(ClideExtensionContext ctx) => onActivate(ctx); +} + +/// Extension that overrides nothing — its activate/deactivate are the +/// base-class defaults (the lines under test). +class _BareExt extends ClideExtension { + @override + String get id => 'bare.ext'; + @override + String get title => 'Bare'; + @override + String get version => '0.0.0-test'; + @override + List get contributions => const []; +} + +void main() { + group('ClideExtensionContext sugar', () { + late KernelFixture f; + + setUp(() async { + f = await KernelFixture.create(); + }); + + tearDown(() async { + await f.dispose(); + }); + + test('publish + subscribe round-trip through the message bus', () async { + Message? received; + f.services.extensions.register(_SugarExt((ctx) async { + final first = ctx.subscribe(channel: 'greet').first; + ctx.publish('greet', {'hello': 'world'}); + received = await first; + })); + await f.services.extensions.activate('sugar.ext'); + expect(received, isNotNull); + expect(received!.publisher, 'sugar.ext'); + expect(received!.channel, 'greet'); + expect(received!.data['hello'], 'world'); + }); + + test('t + tr resolve against the extension namespace', () async { + String? t; + String? tr; + f.services.extensions.register(_SugarExt((ctx) async { + t = ctx.t('missing.key'); + tr = ctx.tr('missing.key', replacers: const []); + })); + await f.services.extensions.activate('sugar.ext'); + // No catalog loaded for this namespace → i18n falls back, but the + // sugar getters still execute and return a non-null string. + expect(t, isNotNull); + expect(tr, isNotNull); + }); + }); + + group('ClideExtension default lifecycle', () { + test('base activate + deactivate are safe no-ops', () async { + final ext = _BareExt(); + // Default activate (no context needed by the base impl) + + // default deactivate must both complete without throwing. + await expectLater(ext.deactivate(), completes); + }); + + test('manager deactivate invokes the base deactivate', () async { + final f = await KernelFixture.create(); + addTearDown(f.dispose); + f.services.extensions.register(_BareExt()); + await f.services.extensions.activate('bare.ext'); + await f.services.extensions.deactivate('bare.ext'); + // Reaching here means the base ClideExtension.deactivate() ran + // through the manager without error. + expect(f.services.extensions.failedExtensions['bare.ext'], isNull); + }); + }); +} diff --git a/test/extension/src/host_test.dart b/test/extension/src/host_test.dart index 91868e7d..dee90943 100644 --- a/test/extension/src/host_test.dart +++ b/test/extension/src/host_test.dart @@ -52,5 +52,19 @@ void main() { final out = await const ExtensionScanner().discover(root: root); expect(out.map((m) => m.id).toList(), ['ext.ok']); }); + + test('defaultRoot points at ~/.clide/extensions', () { + final scanner = ExtensionScanner(); + final home = Platform.environment['HOME'] ?? '/tmp'; + expect(scanner.defaultRoot().path, '$home/.clide/extensions'); + }); + + test('discover() with no root falls back to defaultRoot', () async { + // Exercises the `root ?? defaultRoot()` fallback. The install + // root rarely exists in CI, so this returns a (possibly empty) + // list rather than throwing. + final out = await const ExtensionScanner().discover(); + expect(out, isA>()); + }); }); }