From 9607124c2bfc983c3c09aa322640aa15741bec81 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 10 Jun 2026 15:00:58 +0200 Subject: [PATCH] fix ClideStatusIndicator duplicate-key crash on rapid status flips (T-326) AnimatedSwitcher cross-fades exiting + entering glyphs for 200ms; with a fixed per-status ValueKey, flipping a status back (running -> success -> running within the fade, e.g. two bound Claude panes) left two children keyed 'running' in the Stack -> "Duplicate keys found" + a cascade of follow-on framework errors. Make the indicator stateful and fold a per-change sequence counter into the key, so each appearance is unique and a same-status rebuild still doesn't re-animate. Regression test flips status mid-cross-fade with real animations. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 3 ++ .pql/changelog/ticket_idmap/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 18 +++++++++++ CHANGELOG.md | 5 +++ lib/widgets/src/clide_status_indicator.dart | 31 +++++++++++++++--- .../src/clide_status_indicator_test.dart | 32 +++++++++++++++++++ 6 files changed, 85 insertions(+), 5 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 3ab2e047..e9d503c5 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3175,3 +3175,6 @@ 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 ('06FB1Q1Y3CYJHD7W5F68VDB6T4', 'status', 'ready', 'done', NULL, '2026-06-10 12:31:23', '2026-06-10 12:31:23', '2026-06-10 12:31:23', NULL, 'dbac53eef0b8d58dbb5f00a2f77ee52f', 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 ('06FB1Q1Y3CYJHD7W5F68VDB6T4', 'status', 'done', 'done', NULL, '2026-06-10 12:32:07', '2026-06-10 12:32:07', '2026-06-10 12:32:07', NULL, 'ea979d683072b321ce6eae3dbbbf6c31', 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 ('06FB2W4G9K8ZF782W7H2TM5XA8', 'status', 'ready', 'done', NULL, '2026-06-10 12:51:50', '2026-06-10 12:51:50', '2026-06-10 12:51:50', NULL, 'a8a700b9b92afcb998b5c2e69660e1b2', 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 ('06FB1S7613SYF0M9XQT5JNWM40', 'status', 'backlog', 'ready', NULL, '2026-06-10 12:56:38', '2026-06-10 12:56:38', '2026-06-10 12:56:38', NULL, '0c7b9f022143b733461ca7b1b715c435', 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 ('06FB37JZSFZKWPK9PDFYJY2YC0', 'status', 'backlog', 'ready', NULL, '2026-06-10 13:00:19', '2026-06-10 13:00:19', '2026-06-10 13:00:19', NULL, 'a394b4a8b72a7bc99c7fc1a1ac6a2f19', 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 ('06FB2TY91VHK7TPKPMZ11EG3TM', 'status', 'ready', 'done', NULL, '2026-06-10 13:00:29', '2026-06-10 13:00:29', '2026-06-10 13:00:29', NULL, '25246286c46b36a114817316658049c7', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/ticket_idmap/2026-06.sql b/.pql/changelog/ticket_idmap/2026-06.sql index 71e48514..29db145d 100644 --- a/.pql/changelog/ticket_idmap/2026-06.sql +++ b/.pql/changelog/ticket_idmap/2026-06.sql @@ -152,3 +152,4 @@ INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_ INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB2T11GCV1EV07DYD5BZENTM', 'T-325', '2026-06-10 12:00:52', '2026-06-10 12:00:52', NULL, '47b8f31d9337c5bbc6476d62c7f6ed4d', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB2TY91VHK7TPKPMZ11EG3TM', 'T-326', '2026-06-10 12:04:51', '2026-06-10 12:04:51', NULL, '348dee79d9d3fe0a8b260c1f9c47a289', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB2W4G9K8ZF782W7H2TM5XA8', 'T-327', '2026-06-10 12:10:04', '2026-06-10 12:10:04', NULL, '967aab6bc692754a5d3a192cb0008871', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); +INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB37JZSFZKWPK9PDFYJY2YC0', 'T-328', '2026-06-10 13:00:06', '2026-06-10 13:00:06', NULL, '553b6683bbcddfbbab39ee545a99c98c', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index da8d20ac..2ba888eb 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -2423,3 +2423,21 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat - Clicking it publishes a single message-bus event carrying the full ticket; the focused Claude pane receives an injected prompt containing all ticket fields and an instruction to begin. - The sidebar code does not import or call the session orchestrator directly — interaction is bus-only. - Clicking with no focused Claude pane degrades gracefully (no crash).', 'done', 'medium', NULL, NULL, NULL, '2026-06-10 12:10:04', '2026-06-10 12:51:50', NULL, 'a9982bdc3ed2e36cc57787e4582c4e46', 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 ('06FB1S7613SYF0M9XQT5JNWM40', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'Lightbox: click anywhere outside the image should dismiss', 'When the image lightbox is open, clicking outside the image should close it, matching Esc and the close (x) button. + +Today only the thin margin around the lightbox dismisses. The DialogRouter host (lib/kernel/src/dialog.dart:95-103) renders a backdrop that dismisses on tap, but wraps the dialog content in a tap-swallowing GestureDetector (onTap: () {}). ClideLightbox fills a 94%-of-screen SizedBox whose Positioned.fill InteractiveViewer (lib/widgets/src/clide_lightbox.dart:93-113) covers the entire box. So taps on the dark area beside a smaller-than-94% image hit the swallower, not the backdrop, and nothing happens. + +Fix: make taps that land outside the actual image bounds dismiss — e.g. add an onTap to the lightbox''s outer GestureDetector that calls onDismiss when the tap is not on the image (InteractiveViewer child), rather than relying on the host backdrop only reaching the 6% margin. Preserve double-tap-to-reset, scroll-to-zoom, and pan; a single tap on empty canvas closes, a tap/drag on the image does not. + +Acceptance: single click on the dimmed area surrounding the image closes the lightbox; clicking/dragging/zooming the image does not; Esc and the x still close.', 'ready', 'low', NULL, NULL, NULL, '2026-06-10 09:37:31', '2026-06-10 12:56:38', NULL, '9c888af2ae0a8621ae428ac090508e0c', 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 ('06FB37JZSFZKWPK9PDFYJY2YC0', 'task', '06FB0TNQM5TWC00GW0P3X02HZW', 'Deny & simplify note: don''t narrate the reformulation, just retry simpler', 'The T-311 ''Deny & simplify'' note prompts Claude to retry in a simpler form, but Claude tends to narrate the change (''right, splitting that command'', etc.) instead of silently retrying. Add a clause to the preformatted note (lib/builtin/claude/src/prompt_card.dart _kDenySimplifyNote) telling Claude NOT to narrate or re-explain the changes it makes — just proceed as it was, only simpler. Target: a quiet simpler retry, no commentary. Surfaced live while the button was tested against git commands.', 'backlog', 'low', NULL, NULL, NULL, '2026-06-10 13:00:06', '2026-06-10 13:00:06', NULL, 'b6ed8a4e8b82ec89ca361032a3a0ba8c', 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 ('06FB37JZSFZKWPK9PDFYJY2YC0', 'task', '06FB0TNQM5TWC00GW0P3X02HZW', 'Deny & simplify note: don''t narrate the reformulation, just retry simpler', 'The T-311 ''Deny & simplify'' note prompts Claude to retry in a simpler form, but Claude tends to narrate the change (''right, splitting that command'', etc.) instead of silently retrying. Add a clause to the preformatted note (lib/builtin/claude/src/prompt_card.dart _kDenySimplifyNote) telling Claude NOT to narrate or re-explain the changes it makes — just proceed as it was, only simpler. Target: a quiet simpler retry, no commentary. Surfaced live while the button was tested against git commands.', 'ready', 'low', NULL, NULL, NULL, '2026-06-10 13:00:06', '2026-06-10 13:00:19', NULL, '19e1551d324a36c8a2d737265847c448', 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 ('06FB2TY91VHK7TPKPMZ11EG3TM', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'ClideStatusIndicator: duplicate ''running'' key crashes AnimatedSwitcher on rapid status flips', 'AnimatedSwitcher in ClideStatusIndicator throws "Duplicate keys found" (Stack has multiple children with key [<''running''>]) during normal app run, cascading into a flood of follow-on errors ("Tried to build dirty widget in the wrong build scope", "debugNeedsLayout is not true", "ScrollController attached to multiple scroll views", etc). + +Location: lib/widgets/src/clide_status_indicator.dart:37 (AnimatedSwitcher at build()). + +Root cause: each status maps to a child with a fixed ValueKey (''running'' / ''success'' / ''error''). AnimatedSwitcher cross-fades the outgoing and incoming child inside a Stack for its 200ms duration. When the status flips back to a value whose previous child is still animating out (e.g. running -> success -> running within 200ms, or repeated running rebuilds), the still-exiting child and the new child both carry ValueKey(''running'') and collide in the Stack -> duplicate-key assertion. The downstream exceptions are the framework unwinding from the failed build. + +Repro: observed live during `make run` with two Claude panes bound (primary + secondary-1); status indicators flipping quickly trigger it. + +Fix direction: the ValueKey must be unique per indicator instance, not just per status, so two instances (or an in-flight transition) never share a key. Options: key by status combined with a stable per-widget id, or drop the const keys and let AnimatedSwitcher key on child type. Add a widget test that rapidly toggles status within the switch duration and pumps mid-transition to guard the regression.', 'done', 'high', NULL, NULL, NULL, '2026-06-10 12:04:51', '2026-06-10 13:00:29', NULL, '8e5bd637d25313c220c5565e54ee657f', 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 0bfc61bb..da6fbec8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- **Run-status indicators no longer crash on rapid flips.** Switching status + back and forth within the 200ms cross-fade (e.g. running → success → running + across two bound Claude panes) tripped an AnimatedSwitcher duplicate-key + assertion and a cascade of follow-on errors. Each glyph now carries a key + unique per change, so an exiting and entering glyph never collide. (T-326) - **The activity-card run-status spinner is now legible.** At 12px the spinning logo mark read as a static speck; the run-status indicator on collapsible cards is bumped to a `clideIconHero` (26) so the running state is clear at a diff --git a/lib/widgets/src/clide_status_indicator.dart b/lib/widgets/src/clide_status_indicator.dart index e13a47ac..55c62861 100644 --- a/lib/widgets/src/clide_status_indicator.dart +++ b/lib/widgets/src/clide_status_indicator.dart @@ -17,19 +17,40 @@ import 'package:flutter/widgets.dart'; enum ClideRunStatus { running, success, error } -class ClideStatusIndicator extends StatelessWidget { +class ClideStatusIndicator extends StatefulWidget { const ClideStatusIndicator({super.key, required this.status, this.size = 14}); final ClideRunStatus status; final double size; + @override + State createState() => _ClideStatusIndicatorState(); +} + +class _ClideStatusIndicatorState extends State { + /// Monotonic id bumped on every status change, folded into the child key. A + /// per-status-only key collides inside [AnimatedSwitcher]'s Stack when a + /// status re-appears (running → success → running within the cross-fade) + /// while its previous glyph is still animating out — the exiting and entering + /// children share `ValueKey('running')` and trip the duplicate-key assertion + /// (T-326). The sequence makes each appearance's key unique; a same-status + /// rebuild keeps the key, so it still doesn't re-animate. + int _seq = 0; + + @override + void didUpdateWidget(ClideStatusIndicator old) { + super.didUpdateWidget(old); + if (old.status != widget.status) _seq++; + } + @override Widget build(BuildContext context) { final tokens = ClideTheme.of(context).surface; - final (Widget glyph, String label) = switch (status) { - ClideRunStatus.running => (ClideSpinner(size: size, color: tokens.globalTextMuted, key: const ValueKey('running')), 'running'), - ClideRunStatus.success => (ClideIcon(const CheckIcon(), size: size, color: tokens.statusSuccess, key: const ValueKey('success')), 'succeeded'), - ClideRunStatus.error => (ClideIcon(const CloseIcon(), size: size, color: tokens.statusError, key: const ValueKey('error')), 'failed'), + final key = ValueKey('${widget.status.name}-$_seq'); + final (Widget glyph, String label) = switch (widget.status) { + ClideRunStatus.running => (ClideSpinner(size: widget.size, color: tokens.globalTextMuted, key: key), 'running'), + ClideRunStatus.success => (ClideIcon(const CheckIcon(), size: widget.size, color: tokens.statusSuccess, key: key), 'succeeded'), + ClideRunStatus.error => (ClideIcon(const CloseIcon(), size: widget.size, color: tokens.statusError, key: key), 'failed'), }; return Semantics( label: label, diff --git a/test/widgets/src/clide_status_indicator_test.dart b/test/widgets/src/clide_status_indicator_test.dart index c3e6f365..f5ae5c88 100644 --- a/test/widgets/src/clide_status_indicator_test.dart +++ b/test/widgets/src/clide_status_indicator_test.dart @@ -57,5 +57,37 @@ void main() { await tester.pumpAndSettle(); expect(iconWith(CloseIcon), findsOneWidget); }); + + testWidgets('rapid status flips within the cross-fade do not collide (T-326)', (tester) async { + // Real animations (no disableAnimations) so the 200ms cross-fade overlaps + // exiting and entering glyphs — the condition that tripped a duplicate + // 'running' key. Pre-fix this threw "Duplicate keys found". + var status = ClideRunStatus.running; + late StateSetter set; + await tester.pumpWidget(harness( + f, + Center( + child: StatefulBuilder(builder: (ctx, s) { + set = s; + return ClideStatusIndicator(status: status); + }), + ), + )); + + void flip(ClideRunStatus next) => set(() => status = next); + + flip(ClideRunStatus.success); + await tester.pump(const Duration(milliseconds: 40)); + flip(ClideRunStatus.running); // a 2nd 'running' while the 1st is still exiting + await tester.pump(const Duration(milliseconds: 40)); + flip(ClideRunStatus.error); + await tester.pump(const Duration(milliseconds: 40)); + flip(ClideRunStatus.success); // land on a static glyph + await tester.pump(const Duration(milliseconds: 40)); + + expect(tester.takeException(), isNull); + // Unmount so any in-flight spinner controller disposes (no pending timers). + await tester.pumpWidget(const SizedBox()); + }); }); }