diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index d9c991ac..da146201 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -2322,3 +2322,6 @@ 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-191', 'status', 'backlog', 'in_progress', NULL, '2026-05-31 16:13:39', '2026-05-31 16:13:39', '2026-05-31 16:13:39', NULL, '76e72b3d04c0456e8a39d8ca7dba7644', 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-190', 'status', 'backlog', 'in_progress', NULL, '2026-05-31 16:13:39', '2026-05-31 16:13:39', '2026-05-31 16:13:39', NULL, 'cc7072933e46f402bf65c02595843185', 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-186', 'status', 'backlog', 'done', NULL, '2026-05-31 16:40:19', '2026-05-31 16:40:19', '2026-05-31 16:40:19', NULL, '8f5241e0d76f6cef282f5a04d274345a', 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-193', 'description', 'test/builtin/claude/transcript_publisher_test.dart ''TranscriptPublisher republishes reader items onto the bus (lead channel + item key)'' fails intermittently during ''make test-coverage'' / push-check (the full parallel flutter test run) but PASSES reliably in isolation (flutter test = 3/3 green). Observed 2026-05-31 — one run failed it, the immediately-preceding agent run passed all 2221. Not caused by T-192 (the coverage path uses default concurrency, unchanged) — it''s a pre-existing timing/ordering flake, likely a MessageBus delivery race: the test asserts a republished item arrived on the lead channel, and under concurrent load the broadcast-stream delivery may not have been awaited deterministically. Fix: make the test await delivery deterministically (pump/await the bus microtask, or expectLater on the stream) rather than relying on timing; check TranscriptPublisher for any real ordering bug too. Impact: intermittently red push-check → tempts --no-verify (see T-192). Acceptance: the test passes deterministically across repeated full-suite runs; root cause (test-only vs a real publisher race) identified.', 'test/builtin/claude/transcript_publisher_test.dart ''TranscriptPublisher republishes reader items onto the bus (lead channel + item key)'' fails intermittently during ''make test-coverage'' / push-check (the full parallel flutter test run) but PASSES reliably in isolation (flutter test = 3/3 green). Observed 2026-05-31 — one run failed it, the immediately-preceding agent run passed all 2221. Not caused by T-192 (the coverage path uses default concurrency, unchanged) — it''s a pre-existing timing/ordering flake, likely a MessageBus delivery race: the test asserts a republished item arrived on the lead channel, and under concurrent load the broadcast-stream delivery may not have been awaited deterministically. Fix: make the test await delivery deterministically (pump/await the bus microtask, or expectLater on the stream) rather than relying on timing; check TranscriptPublisher for any real ordering bug too. Impact: intermittently red push-check → tempts --no-verify (see T-192). Acceptance: the test passes deterministically across repeated full-suite runs; root cause (test-only vs a real publisher race) identified. + +MECHANISM (the systemic answer, per discussion): tests are parallel by default (--concurrency global = invisible parallel=true); express ''parallel=false'' for a vulnerable test via a TAG + a serial pass. The ''pty'' tag is the half-built precedent: it splits PTY tests into a separate ''dart test'' pass but did NOT serialize it (and leaned on retry:2). DONE 2026-05-31: that pty pass now runs --concurrency=1 and the retry:2 band-aids are removed — PTY/registry flake verified stable 4/4 runs. REMAINING: generalize to a ''serial'' tag for FLUTTER-side vulnerable tests (transcript_publisher''s ''republishes reader items onto the bus''): declare ''serial'' in dart_test.yaml; the parallel flutter run adds --exclude-tags serial; add a serial flutter pass (flutter test --tags serial --concurrency=1). WRINKLE for the gate: two flutter --coverage passes can''t just concatenate lcov (duplicate SF records double-count) — must real-merge (package:coverage format_coverage / lcov -a), OR run the serial pass without coverage if those tests'' source lines are covered elsewhere (check transcript_publisher.dart coverage from other tests first). ALTERNATIVE for the single transcript_publisher case: fix its await to be deterministic (await the bus delivery / expectLater on the stream) rather than quarantine — cheaper if it''s a test-timing bug vs a real bus race. Acceptance: no flaky test runs in the parallel pool; vulnerable tests are tagged + run serially; gate is deterministic across repeated full runs.', NULL, '2026-05-31 17:01:58', '2026-05-31 17:01:58', '2026-05-31 17:01:58', NULL, 'efff36d6e6969000e1085c073788425c', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index 85c9fb06..375d16f7 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -2960,3 +2960,6 @@ INSERT INTO tickets (id, type, parent_id, title, description, status, priority, 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-189', 'story', 'T-186', 'Back/forward navigation on the markdown & decision readers', 'Add Back/Forward buttons to both sidebar readers so you can return to what you were reading. Neither reader keeps history today: _MarkdownViewerState (markdown_viewer.dart) holds only _path/_content; _DecisionDetailViewState (decision_detail_view.dart) holds only _decision. Add a history stack + index to each, pushing on each new open; Back/Forward walk the stack and load the entry WITHOUT re-publishing a ''selection'' (in-widget navigation). For the decision reader, back/forward must not trigger the extension tab re-contribution — coordinate with the decision-open bug fix (T-188), which should stop re-contributing per selection. Custom chrome only (no Material). Acceptance: Back/Forward controls on both readers walk reading history and disable at the ends; widget tests. Sequence after the reader-open bug fixes (T-187/T-188) since it shares those widgets.', 'in_progress', 'medium', NULL, NULL, NULL, '2026-05-31 11:38:36', '2026-05-31 16:13:39', NULL, '8e8e53cd7d4bb35535281c433e8d122d', 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-190', 'story', 'T-186', 'Pin a doc in the markdown & decision readers (single-slot quick-return)', 'Add a one-click PIN to both sidebar readers. Semantics per the reporter: a SINGLE pin slot per reader — clicking pin remembers the CURRENT doc; setting a new pin REPLACES the previous one (no list to manage); the pinned doc is always one click away to return to, so it is easy to use inline as you work. It is a quick-return bookmark, NOT a freeze-incoming toggle. Add a pinned-doc ref to _MarkdownViewerState and _DecisionDetailViewState; a pin button (set/replace) plus a jump-to-pin affordance in the reader chrome; persist for at least the session. Custom chrome only (no Material). Acceptance: pinning a doc lets you one-click return to it later; pinning another replaces it; works on both readers; widget tests. Sequence after the reader-open bug fixes (T-187/T-188); pairs naturally with back/forward (T-189).', 'in_progress', 'medium', NULL, NULL, NULL, '2026-05-31 11:38:42', '2026-05-31 16:13:39', NULL, 'd0ec78c056f0d7f8e35e521f13d229ed', 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-186', 'epic', NULL, 'Sidebar readers: fix message-bus open + add back/forward + pin', 'The right-side sidebar READERS — the markdown reader (builtin.markdown, lib/builtin/markdown/src/markdown_viewer.dart) and the decision reader (builtin.decisions, lib/builtin/decisions/src/decision_detail_view.dart) — are separate from the editor and open via the kernel MessageBus, the same pattern as the still-working ticket panel (lib/builtin/tickets): a click publishes (publisher, channel ''selection''); an extension activates the context-panel tab; the reader widget subscribes and renders. Two open paths are currently broken and we also want reading-navigation (back/forward) plus a single-slot pin on BOTH readers. Grouped as one set because all four tickets touch the same two reader widgets. Root-cause investigation done 2026-05-31 (see child tickets).', 'done', 'high', NULL, NULL, NULL, '2026-05-31 11:37:34', '2026-05-31 16:40:19', NULL, 'f32bbd391b6fd1d8448af994fae28a13', 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-193', 'bug', NULL, 'Flaky: transcript_publisher_test ''republishes reader items onto the bus'' fails under the full parallel run', 'test/builtin/claude/transcript_publisher_test.dart ''TranscriptPublisher republishes reader items onto the bus (lead channel + item key)'' fails intermittently during ''make test-coverage'' / push-check (the full parallel flutter test run) but PASSES reliably in isolation (flutter test = 3/3 green). Observed 2026-05-31 — one run failed it, the immediately-preceding agent run passed all 2221. Not caused by T-192 (the coverage path uses default concurrency, unchanged) — it''s a pre-existing timing/ordering flake, likely a MessageBus delivery race: the test asserts a republished item arrived on the lead channel, and under concurrent load the broadcast-stream delivery may not have been awaited deterministically. Fix: make the test await delivery deterministically (pump/await the bus microtask, or expectLater on the stream) rather than relying on timing; check TranscriptPublisher for any real ordering bug too. Impact: intermittently red push-check → tempts --no-verify (see T-192). Acceptance: the test passes deterministically across repeated full-suite runs; root cause (test-only vs a real publisher race) identified. + +MECHANISM (the systemic answer, per discussion): tests are parallel by default (--concurrency global = invisible parallel=true); express ''parallel=false'' for a vulnerable test via a TAG + a serial pass. The ''pty'' tag is the half-built precedent: it splits PTY tests into a separate ''dart test'' pass but did NOT serialize it (and leaned on retry:2). DONE 2026-05-31: that pty pass now runs --concurrency=1 and the retry:2 band-aids are removed — PTY/registry flake verified stable 4/4 runs. REMAINING: generalize to a ''serial'' tag for FLUTTER-side vulnerable tests (transcript_publisher''s ''republishes reader items onto the bus''): declare ''serial'' in dart_test.yaml; the parallel flutter run adds --exclude-tags serial; add a serial flutter pass (flutter test --tags serial --concurrency=1). WRINKLE for the gate: two flutter --coverage passes can''t just concatenate lcov (duplicate SF records double-count) — must real-merge (package:coverage format_coverage / lcov -a), OR run the serial pass without coverage if those tests'' source lines are covered elsewhere (check transcript_publisher.dart coverage from other tests first). ALTERNATIVE for the single transcript_publisher case: fix its await to be deterministic (await the bus delivery / expectLater on the stream) rather than quarantine — cheaper if it''s a test-timing bug vs a real bus race. Acceptance: no flaky test runs in the parallel pool; vulnerable tests are tagged + run serially; gate is deterministic across repeated full runs.', 'backlog', 'medium', NULL, NULL, NULL, '2026-05-31 16:12:42', '2026-05-31 17:01:58', NULL, 'a53bd052ae3024be526c4a6d54da23ef', 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/ci/merge_lcov.py b/ci/merge_lcov.py new file mode 100644 index 00000000..9f3bd1e2 --- /dev/null +++ b/ci/merge_lcov.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 +"""Merge lcov files into one, on stdout. + +clide runs coverage in two passes — the parallel pool and a serial +(`--tags serial --concurrency=1`) pass for tests that can't share the +parallel runner (T-193). Each `flutter test --coverage` pass overwrites +coverage/lcov.info, and the coverage gate naively sums LF:/LH: across +records, so a source file appearing in BOTH passes would double-count. + +This unions DA (line→hits) per source file, taking the MAX hit count (a +line executed in EITHER pass counts as hit), then recomputes LF/LH. Line +coverage only — which is all flutter emits and all the gate reads. No +`lcov` dependency. + +Usage: merge_lcov.py a.info b.info [...] > merged.info +""" +import sys + +files = {} # source path -> {line: hits} +order = [] # first-seen order, for stable output + +for path in sys.argv[1:]: + cur = None + with open(path) as fh: + for raw in fh: + line = raw.rstrip("\n") + if line.startswith("SF:"): + cur = line[3:] + if cur not in files: + files[cur] = {} + order.append(cur) + elif line.startswith("DA:") and cur is not None: + num, _, hits = line[3:].partition(",") + num, hits = int(num), int(hits) + files[cur][num] = max(files[cur].get(num, 0), hits) + elif line == "end_of_record": + cur = None + +out = [] +for sf in order: + da = files[sf] + out.append("SF:" + sf) + for num in sorted(da): + out.append(f"DA:{num},{da[num]}") + out.append(f"LF:{len(da)}") + out.append(f"LH:{sum(1 for h in da.values() if h > 0)}") + out.append("end_of_record") + +sys.stdout.write("\n".join(out) + "\n") diff --git a/ci/test.sh b/ci/test.sh index 3be44b2d..c70fff35 100755 --- a/ci/test.sh +++ b/ci/test.sh @@ -30,10 +30,22 @@ echo "==> dart test (pty — unreliable under the flutter test runner; serial)" # for resource-bound tests, vs. the old per-test `retry:` band-aid. (T-193) dart test --concurrency=1 --tags pty test/pty/session_test.dart test/panes/registry_test.dart +# The parallel pool excludes both pty (runs under dart test, above) and +# serial-tagged tests (concurrency-vulnerable — run in their own --concurrency=1 +# pass below). See dart_test.yaml + T-193. if [[ "$coverage" == 1 ]]; then - echo "==> flutter test --coverage (gate; unit + widget + golden + a11y)" - flutter test --coverage --exclude-tags pty --timeout 60s + echo "==> flutter test --coverage (parallel pool; excludes pty + serial)" + flutter test --coverage --exclude-tags "pty || serial" --timeout 60s + cp coverage/lcov.info coverage/lcov.parallel.info + echo "==> flutter test --coverage (serial-tagged; --concurrency=1)" + flutter test --coverage --tags serial --concurrency=1 --timeout 60s + echo "==> merge coverage (parallel + serial passes → coverage/lcov.info)" + python3 ci/merge_lcov.py coverage/lcov.parallel.info coverage/lcov.info > coverage/lcov.merged.info + mv coverage/lcov.merged.info coverage/lcov.info + rm -f coverage/lcov.parallel.info else - echo "==> flutter test (dev; no coverage, parallel)" - flutter test --exclude-tags pty --concurrency=12 --timeout 60s + echo "==> flutter test (dev; parallel pool, excludes pty + serial)" + flutter test --exclude-tags "pty || serial" --concurrency=12 --timeout 60s + echo "==> flutter test (dev; serial-tagged, --concurrency=1)" + flutter test --tags serial --concurrency=1 --timeout 60s fi diff --git a/dart_test.yaml b/dart_test.yaml index df577599..0c2fb963 100644 --- a/dart_test.yaml +++ b/dart_test.yaml @@ -10,3 +10,10 @@ tags: # under `dart test`. (Tag was historically named `forkpty`, before # T-96 replaced forkpty with posix_spawn.) See `test/pty/session_test.dart`. pty: + # Concurrency-vulnerable tests that flake in the parallel flutter pool + # (e.g. MessageBus delivery races). The parallel run excludes them + # (`--exclude-tags "pty || serial"`); ci/test.sh runs a separate + # `--tags serial --concurrency=1` pass and merges its coverage. Prefer a + # deterministic await first — only tag `serial` when isolation is the real + # fix. (T-193) + serial: diff --git a/test/builtin/claude/transcript_publisher_test.dart b/test/builtin/claude/transcript_publisher_test.dart index ba57722c..200759d7 100644 --- a/test/builtin/claude/transcript_publisher_test.dart +++ b/test/builtin/claude/transcript_publisher_test.dart @@ -44,7 +44,9 @@ void main() { setUp(() async => base = await Directory.systemTemp.createTemp('transcript_publisher_test_')); tearDown(() async => base.delete(recursive: true)); - test('republishes reader items onto the bus (lead channel + item key)', () async { + // Serialized: this MessageBus republish assertion is timing-sensitive and + // flaked in the parallel flutter pool; runs in the --concurrency=1 pass (T-193). + test('republishes reader items onto the bus (lead channel + item key)', tags: ['serial'], () async { final dir = Directory('${base.path}/${workspace.replaceAll('/', '-')}'); await dir.create(recursive: true); File('${dir.path}/session-abc.jsonl').writeAsStringSync(