diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index aa96c59e..fa521f37 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -1713,3 +1713,9 @@ 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-102', 'status', 'backlog', 'in_progress', NULL, '2026-05-17 18:58:56', '2026-05-17 18:58:56', '2026-05-17 18:58:56', NULL, '7fa075d7232df5302ffdfb5d1c3d79f2', 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-101', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:01:39', '2026-05-17 19:01:39', '2026-05-17 19:01:39', NULL, '56e8da656c5fc53af241900e6313805e', 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-102', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:01:39', '2026-05-17 19:01:39', '2026-05-17 19:01:39', NULL, '7f09d2a1501d3d072e071822c191f253', 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-103', 'status', 'backlog', 'in_progress', NULL, '2026-05-17 19:03:18', '2026-05-17 19:03:18', '2026-05-17 19:03:18', NULL, '2635d907f94402ec506d3b56dfc25b21', 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-106', 'status', 'backlog', 'in_progress', NULL, '2026-05-17 19:03:18', '2026-05-17 19:03:18', '2026-05-17 19:03:18', NULL, 'cc39b3fd718e99423823e640950ccf1b', 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-113', 'status', 'backlog', 'in_progress', NULL, '2026-05-17 19:03:18', '2026-05-17 19:03:18', '2026-05-17 19:03:18', NULL, 'cc8605833ba5cb97e068eb5a6d57d8f4', 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-106', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:18:43', '2026-05-17 19:18:43', '2026-05-17 19:18:43', NULL, '8dccc83b93c5005db744a100a5b3baea', 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-113', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:18:43', '2026-05-17 19:18:43', '2026-05-17 19:18:43', NULL, '8f7812831e564005eb796708543c54b3', 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-103', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:18:43', '2026-05-17 19:18:43', '2026-05-17 19:18:43', NULL, 'c7d1b76cb6854efb7e9990cc7204d899', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index 54898d89..5f6d65f9 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -1623,3 +1623,48 @@ INSERT INTO tickets (id, type, parent_id, title, description, status, priority, 4. Spot-check other doc references to dissolved components. Source: consultants.md "Docs — Findings — [Critical]".', 'done', 'high', NULL, NULL, NULL, '2026-05-17 18:47:29', '2026-05-17 19:01:39', NULL, 'a7d378267119361f2b2188977881a666', 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-115', 'story', NULL, 'startup project picker; "always open this project" override', 'Startup flow change: on launch, always show the project picker screen unless the user has explicitly checked an "always open this project" box for a specific project (sticky default). + +**Acceptance:** +1. Default startup: project picker is the first screen, regardless of last-opened state. +2. Picker has an "always open this project on launch" checkbox per project entry. +3. If exactly one project has that flag set, startup skips the picker and opens that project directly. +4. The flag is per-project, persisted in app settings (probably under `app.startupProject`). +5. The checkbox can be unticked from the welcome view to restore picker-first behavior. + +**Notes:** today the app auto-opens `app.lastProject` on boot (kernel/src/project.dart `openLast`). The new flow inverts the default — picker is the steady state, sticky-open is opt-in per project. + +**Out of scope:** multi-project workspaces, project groups, recent-project ordering changes. + +**Source:** user request 2026-05-17.', 'backlog', 'medium', NULL, NULL, NULL, '2026-05-17 19:10:21', '2026-05-17 19:10:21', NULL, 'ce156fb92fbad8ba9bf860a4eb71d1ac', 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-116', 'bug', 'T-97', 'integration_test/theme_picker_test.dart hangs pumpAndSettle on theme.pick', 'When running `flutter test integration_test/theme_picker_test.dart` against the real boot path, calling `services.commands.execute(''theme.pick'')` followed by `await tester.pumpAndSettle()` does not settle — the test hangs for 60+ seconds and never reaches the `Select theme` / `Cancel` assertions. Likely an animation loop or a never-completing Future in the modal mount path. + +Blocks T-103 from adding integration tests to `make push-check` (the other two integration tests pass after viewport + assertion fixes). + +**Acceptance:** +1. `flutter test integration_test/theme_picker_test.dart` completes in <10s. +2. Both assertions (`Select theme` visible after `theme.pick`; gone after `Cancel`) pass. +3. T-103''s `push-check-full` can include integration tests without the theme-picker hanging. + +Source: surfaced 2026-05-17 during T-103 work.', 'backlog', 'medium', NULL, NULL, NULL, '2026-05-17 19:14:31', '2026-05-17 19:14:31', NULL, 'df0c6bdd610410d4bd27bfeb2be9acbc', 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-113', 'task', 'T-97', 'quick-wins: governance + docs reconciliation', 'Batch of small consultant findings that share a domain (governance/docs): + +1. **Amend D-66** to match current floor location/value: the floor lives in `pubspec.yaml` `coverage_floor:` (single source of truth as of this session), not `coverage/floor.txt`. Add a dated amendment line. +2. **Reconcile `licenses.yaml` vs `pubspec.yaml`** — `test` version drift (`1.25.8` vs `1.30.0`), phantom `lints 5.0.0` not in pubspec. Two-step-commit rule from D-42 is being violated. +3. **Triage stale Q-records** — Q-1/Q-2/Q-3 ("defer until Tier 1 is in real use") are due now that Tier 1 has shipped. Q-25 (body text face) is de facto resolved by D-43/D-44. +4. **Drop `--no-fatal-infos` from `ci/test.sh`** now that the import queue is clear (post test-sweep this session). +5. Add a `.claude/skills/README.md` inventory. + +Source: consultants.md (Docs + Tests + Security Findings — Minor).', 'done', 'low', NULL, NULL, NULL, '2026-05-17 18:48:24', '2026-05-17 19:18:43', NULL, '6988f892ed906cb397bea5068fdcd30e', 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-106', 'bug', 'T-97', 'fix SchedulerService._startTicker isolate-spawn race', '`scheduler.dart:71`: `Isolate.spawn(_isolateEntry, _port!.sendPort).then((iso) => _isolate = iso)`. If `_stopTicker()` runs before the spawn future completes, `_isolate` is still null, nothing is killed, and the just-spawned isolate (with its `Timer.periodic`) leaks forever. + +Same race shape we just fixed in `NativePty` via `_readerReady` (T-96). + +**Fix:** track the spawn future as `_isolateReady`; `_stopTicker` awaits it before killing. + +Source: consultants.md "Code quality — Findings — [Major]".', 'done', 'medium', NULL, NULL, NULL, '2026-05-17 18:47:51', '2026-05-17 19:18:43', NULL, 'b529416bab7c66fde471f990671f3b25', 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-103', 'task', 'T-97', 'add test-integration + smoke-bundle to make push-check', '`make push-check` runs `decisions-validate test-core test test-a11y coverage-gate changelog-gate`. `test-integration` and `smoke-bundle` are omitted — a boot-order regression sails through the pre-push gate. + +**Acceptance:** `make push-check` runs the integration test layer + smoke-bundle. Wall-clock budget acceptable for pre-push (target <2 min total). If they''re too slow, gate them behind a separate `make push-check-full` and document. + +Source: consultants.md "Tests — Findings — [Major] make push-check does not run integration tests".', 'done', 'high', NULL, NULL, NULL, '2026-05-17 18:47:39', '2026-05-17 19:18:43', NULL, 'cbae1915f1c89a8a2b05da71148d77dc', 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 7981585a..bc5c17b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -98,6 +98,8 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. - `README.md` rewritten to match current architecture; `docs/initial-plan.md` bannered as historical; new `docs/architecture.md` describes today's shape (T-101). +- `SchedulerService._stopTicker` now awaits the in-flight isolate spawn + before killing — closes the same race shape we fixed in PTY (T-106). - Terminal panes now render bold attributes with a real bold weight — bundled JetBrainsMono Bold + BoldItalic are registered with the `JetBrainsMono` family at `weight: 700`. The painter's bold diff --git a/lib/kernel/src/facade.dart b/lib/kernel/src/facade.dart index 9a2154a7..4609181d 100644 --- a/lib/kernel/src/facade.dart +++ b/lib/kernel/src/facade.dart @@ -238,7 +238,7 @@ class KernelServices { focus.dispose(); project.dispose(); extensions.dispose(); - scheduler.dispose(); + await scheduler.dispose(); await log.dispose(); messages.dispose(); await events.dispose(); diff --git a/lib/kernel/src/scheduler.dart b/lib/kernel/src/scheduler.dart index 66a3354d..7e6bbeb8 100644 --- a/lib/kernel/src/scheduler.dart +++ b/lib/kernel/src/scheduler.dart @@ -38,6 +38,13 @@ class SchedulerService { StreamSubscription? _sub; StreamSubscription? _projectSub; + /// Tracks the spawn future so [_stopTicker] can await it before + /// killing — otherwise a stop racing a still-spawning isolate + /// leaves `_isolate` null at kill time and the just-spawned isolate + /// (with its `Timer.periodic`) leaks forever. Same race shape we + /// fixed in `NativePty` via `_readerReady` (T-96). + Future? _isolateReady; + /// Listen for project lifecycle events. The periodic ticker only runs /// while a project is open — no wasted cycles on the welcome screen. void start() { @@ -47,8 +54,8 @@ class SchedulerService { /// Start the periodic ticker and fire an immediate first cycle so /// all panels refresh without waiting for the first interval. - void _startTicker() { - _stopTicker(); + Future _startTicker() async { + await _stopTicker(); // Stagger the initial ticks to avoid a rebuild storm on project open. var delay = 0; @@ -68,14 +75,26 @@ class SchedulerService { _events.emit(SchedulerTick(tier: tier)); } }); - Isolate.spawn(_isolateEntry, _port!.sendPort).then((iso) => _isolate = iso); + _isolateReady = Isolate.spawn(_isolateEntry, _port!.sendPort); + _isolateReady!.then((iso) => _isolate = iso).catchError((_) => null); } - void _stopTicker() { + Future _stopTicker() async { + // Await any in-flight spawn so we never miss killing an isolate + // that's mid-creation — see _isolateReady. + if (_isolateReady != null) { + try { + final pending = await _isolateReady; + _isolate ??= pending; + } catch (_) { + // Spawn failed; nothing to kill. + } + } _sub?.cancel(); _port?.close(); _isolate?.kill(priority: Isolate.immediate); _isolate = null; + _isolateReady = null; _port = null; _sub = null; } @@ -97,8 +116,8 @@ class SchedulerService { } } - void dispose() { + Future dispose() async { _projectSub?.cancel(); - _stopTicker(); + await _stopTicker(); } } diff --git a/test/kernel/src/services_bigger_test.dart b/test/kernel/src/services_bigger_test.dart index 009fbb61..7db8f81b 100644 --- a/test/kernel/src/services_bigger_test.dart +++ b/test/kernel/src/services_bigger_test.dart @@ -220,5 +220,22 @@ void main() { bus.emit(const ProjectClosed()); // Smoke — no throw. }); + + test('dispose immediately after ProjectOpened awaits the in-flight spawn (T-106)', () async { + // Race window: ProjectOpened triggers _startTicker which calls + // `Isolate.spawn(...)`; if dispose lands before the spawn future + // resolves, the old code left _isolate=null and the just-spawned + // ticker leaked forever. Now dispose awaits _isolateReady before + // killing. + final bus = DaemonBus(); + final s = SchedulerService(bus); + s.start(); + bus.emit(const ProjectOpened(path: '/tmp/x')); + // Don't wait for the spawn to settle — dispose must do that itself. + await s.dispose(); + // No throw, no leaked isolate (test runner would flag a lingering + // isolate by failing to exit cleanly). Reaching this line is the + // assertion. + }); }); }