fix decision first-click + editor reveal via retained reader nav

Two reveal-on-open bugs:

Decisions opened only on the second click (T-196): the detail view
subscribed in didChangeDependencies, which runs after the tab is
revealed, so the broadcast 'selection' that triggered the reveal was
already gone. Hoist the back/forward history out of per-view State into
a retained per-reader ReaderNav (kernel ChangeNotifier in a
ReaderNavRegistry, D-81). The nav records selections, emits 'load' (the
single channel readers display from), and survives mount/unmount — the
reader grabs nav.current on mount, so the first selection lands. Both
the markdown and decisions readers move to this model; the per-view
ReaderHistoryMixin and the markdown post-frame forward hack are gone.

The editor pane never opened (T-197): EditorExtension contributed a
workspace tab but nothing activated it on editor.open. Add an activate()
that reveals the tab on editor.opened / editor.active-changed; the
view's hydrate() pulls the active buffer on mount.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-01 12:45:48 +02:00
co-authored by Claude Opus 4.8
parent 1db65f8481
commit 0eb7b0df2f
17 changed files with 499 additions and 460 deletions
@@ -89,7 +89,13 @@ void main() {
f = await KernelFixture.create();
await _bootExtension(f);
});
tearDown(() => f.dispose());
tearDown(() async {
// Deactivate before dispose so any post-frame forward scheduled by
// these (non-pumping) tests is neutralised — otherwise it fires in
// a later testWidgets against a torn-down bus.
await f.services.extensions.deactivate('builtin.decisions');
await f.dispose();
});
test('activate contributes decisions.detail as a static tab', () {
final tabs = f.services.panels.tabsFor(Slots.contextPanel);
@@ -237,8 +243,9 @@ void main() {
// Starts empty.
expect(find.text('Select a decision to view details.'), findsOneWidget);
// Publish a selection.
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-7'});
// The view loads on 'load' (forwarded by the extension post-frame
// after it reveals the tab; T-196).
f.services.messages.publish('builtin.decisions', 'load', {'id': 'D-7'});
// Give the broadcast stream a microtask to deliver.
await pumpAsync(tester);
@@ -251,7 +258,7 @@ void main() {
await pumpView(tester, initialId: 'D-1');
expect(find.text('Decision D-1'), findsWidgets);
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
f.services.messages.publish('builtin.decisions', 'load', {'id': 'D-2'});
await pumpAsync(tester);
expect(find.text('Decision D-2'), findsWidgets);
@@ -262,7 +269,7 @@ void main() {
await pumpView(tester, initialId: 'D-5');
expect(find.text('Decision D-5'), findsWidgets);
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-5'});
f.services.messages.publish('builtin.decisions', 'load', {'id': 'D-5'});
await pumpAsync(tester);
// Still shows D-5, no crash.
@@ -273,7 +280,7 @@ void main() {
await pumpView(tester);
for (var i = 1; i <= 5; i++) {
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-$i'});
f.services.messages.publish('builtin.decisions', 'load', {'id': 'D-$i'});
}
await pumpAsync(tester);
@@ -541,6 +548,13 @@ void main() {
});
tearDown(() => f.dispose());
// Drive the retained nav (the history source); its 'load' emit makes
// the mounted view display the entry (T-196).
Future<void> open(WidgetTester tester, String id) async {
f.services.readerNav.navFor('builtin.decisions', dataKey: 'id').open(id);
await pumpAsync(tester);
}
Future<void> pumpView(WidgetTester tester, {String? initialId}) async {
tester.view.physicalSize = const Size(600, 800);
tester.view.devicePixelRatio = 1.0;
@@ -548,8 +562,9 @@ void main() {
tester.view.resetPhysicalSize();
tester.view.resetDevicePixelRatio();
});
await tester.pumpWidget(harness(f, DecisionDetailView(initialId: initialId)));
await tester.pumpWidget(harness(f, const DecisionDetailView()));
await pumpAsync(tester);
if (initialId != null) await open(tester, initialId);
}
testWidgets('back disabled on initial load', (tester) async {
@@ -565,8 +580,7 @@ void main() {
testWidgets('back enabled after two selections', (tester) async {
await pumpView(tester, initialId: 'D-1');
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
await pumpAsync(tester);
await open(tester, 'D-2');
expect(
find.byWidgetPredicate(
@@ -578,8 +592,7 @@ void main() {
testWidgets('back navigates to previous decision', (tester) async {
await pumpView(tester, initialId: 'D-1');
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
await pumpAsync(tester);
await open(tester, 'D-2');
// Title appears in pane header subtitle + body card.
expect(find.text('Decision D-2'), findsWidgets);
@@ -594,14 +607,13 @@ void main() {
testWidgets('back/forward does NOT re-publish selection bus event', (tester) async {
await pumpView(tester, initialId: 'D-1');
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
await pumpAsync(tester);
await open(tester, 'D-2');
final selections = <Message>[];
final sub = f.services.messages.subscribe(publisher: 'builtin.decisions', channel: 'selection').listen(selections.add);
addTearDown(sub.cancel);
// Go back — should NOT publish a selection message.
// Go back — re-emits on 'load', NOT 'selection'.
final backBtn = find.byWidgetPredicate(
(w) => w is Semantics && w.properties.label == 'Back' && (w.properties.enabled ?? true),
);
@@ -613,8 +625,7 @@ void main() {
testWidgets('forward disabled at end of history', (tester) async {
await pumpView(tester, initialId: 'D-1');
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
await pumpAsync(tester);
await open(tester, 'D-2');
expect(
find.byWidgetPredicate(
@@ -626,8 +637,7 @@ void main() {
testWidgets('forward navigates after back', (tester) async {
await pumpView(tester, initialId: 'D-1');
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
await pumpAsync(tester);
await open(tester, 'D-2');
// Go back to D-1.
final backBtn = find.byWidgetPredicate(
@@ -648,8 +658,7 @@ void main() {
testWidgets('new selection truncates forward history', (tester) async {
await pumpView(tester, initialId: 'D-1');
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
await pumpAsync(tester);
await open(tester, 'D-2');
// Go back to D-1.
final backBtn = find.byWidgetPredicate(
@@ -658,9 +667,8 @@ void main() {
await tester.tap(backBtn.first);
await pumpAsync(tester);
// Load D-3 — truncates D-2 forward history.
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-3'});
await pumpAsync(tester);
// Open D-3 — truncates D-2 forward history.
await open(tester, 'D-3');
expect(
find.byWidgetPredicate(
@@ -687,6 +695,11 @@ void main() {
});
tearDown(() => f.dispose());
Future<void> open(WidgetTester tester, String id) async {
f.services.readerNav.navFor('builtin.decisions', dataKey: 'id').open(id);
await pumpAsync(tester);
}
Future<void> pumpView(WidgetTester tester, {String? initialId}) async {
tester.view.physicalSize = const Size(600, 800);
tester.view.devicePixelRatio = 1.0;
@@ -694,8 +707,9 @@ void main() {
tester.view.resetPhysicalSize();
tester.view.resetDevicePixelRatio();
});
await tester.pumpWidget(harness(f, DecisionDetailView(initialId: initialId)));
await tester.pumpWidget(harness(f, const DecisionDetailView()));
await pumpAsync(tester);
if (initialId != null) await open(tester, initialId);
}
testWidgets('pin jump affordance not visible before pin set', (tester) async {
@@ -734,8 +748,7 @@ void main() {
await pumpAsync(tester);
// Navigate to D-2.
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
await pumpAsync(tester);
await open(tester, 'D-2');
// Title appears in pane header subtitle + body card.
expect(find.text('Decision D-2'), findsWidgets);
@@ -762,8 +775,7 @@ void main() {
await pumpAsync(tester);
// Navigate to D-2.
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-2'});
await pumpAsync(tester);
await open(tester, 'D-2');
// Replace pin with D-2.
await tester.tap(find
@@ -774,8 +786,7 @@ void main() {
await pumpAsync(tester);
// Navigate to D-3.
f.services.messages.publish('builtin.decisions', 'selection', {'id': 'D-3'});
await pumpAsync(tester);
await open(tester, 'D-3');
// Jump to pin — should go to D-2 (replaced), not D-1.
await tester.tap(find
@@ -0,0 +1,75 @@
/// T-197: EditorExtension reveals its workspace tab when a buffer opens.
///
/// `editor.open` opens the buffer daemon-side and emits `editor.opened`,
/// but nothing else brings the editor tab to front over the Claude
/// pane. The extension's activate() listens for the editor lifecycle
/// events and activates the workspace tab.
library;
import 'package:clide/builtin/editor/src/extension.dart';
import 'package:clide/extension/extension.dart' show TabContribution;
import 'package:clide/kernel/kernel.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
import '../../helpers/kernel_fixture.dart';
void main() {
late KernelFixture f;
setUp(() async {
f = await KernelFixture.create();
f.services.panels.registerSlot(const SlotDefinition(id: Slots.workspace, position: SlotPosition.center));
// A pre-existing workspace tab so 'editor.active' is NOT the default
// active tab — the reveal must switch to it explicitly.
f.services.panels.contribute(TabContribution(
id: 'claude.primary',
slot: Slots.workspace,
title: 'Claude',
build: (_) => const SizedBox(),
));
f.services.extensions.register(EditorExtension());
await f.services.extensions.activate('builtin.editor');
});
tearDown(() => f.dispose());
void emitEditor(String kind, {String? id}) {
f.services.events.emit(DaemonEvent(subsystem: 'editor', kind: kind, data: {'id': id}, ts: DateTime.now().toUtc()));
}
test('contributes editor.active but leaves Claude active by default', () {
expect(f.services.panels.tabsFor(Slots.workspace).any((t) => t.id == 'editor.active'), isTrue);
expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary');
});
test('editor.opened reveals (activates) the editor tab', () async {
emitEditor('editor.opened', id: 'b_1');
await Future<void>.delayed(Duration.zero);
expect(f.services.panels.activeTabIn(Slots.workspace), 'editor.active');
});
test('editor.active-changed also reveals the editor tab', () async {
emitEditor('editor.active-changed', id: 'b_2');
await Future<void>.delayed(Duration.zero);
expect(f.services.panels.activeTabIn(Slots.workspace), 'editor.active');
});
test('a non-editor event leaves the active tab unchanged', () async {
f.services.events.emit(DaemonEvent(subsystem: 'git', kind: 'changed', data: const {}, ts: DateTime.now().toUtc()));
await Future<void>.delayed(Duration.zero);
expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary');
});
test('an unrelated editor event kind does not reveal', () async {
emitEditor('editor.saved', id: 'b_1');
await Future<void>.delayed(Duration.zero);
expect(f.services.panels.activeTabIn(Slots.workspace), 'claude.primary');
});
test('after deactivate, editor events no longer reveal', () async {
await f.services.extensions.deactivate('builtin.editor');
emitEditor('editor.opened', id: 'b_9');
await Future<void>.delayed(Duration.zero);
expect(f.services.panels.activeTabIn(Slots.workspace), isNot('editor.active'));
});
}
@@ -50,9 +50,10 @@ Future<void> pumpView(WidgetTester tester, KernelFixture f) async {
await pumpAsync(tester);
}
/// Trigger a file load via the 'load' channel (mirrors the extension bridge).
/// Open a file through the retained nav (the history source); its 'load'
/// emit makes the mounted viewer display it (T-196).
Future<void> loadFile(WidgetTester tester, KernelFixture f, String path) async {
f.services.messages.publish('builtin.markdown', 'load', {'path': path});
f.services.readerNav.navFor('builtin.markdown', dataKey: 'path').open(path);
await pumpAsync(tester);
}
-222
View File
@@ -1,222 +0,0 @@
/// Unit tests for [ReaderHistory] and [ReaderHistoryMixin] (T-189, T-190).
library;
import 'package:clide/builtin/shared/reader_chrome.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
// ---------------------------------------------------------------------------
// ReaderHistory unit tests (no Flutter needed — plain test())
// ---------------------------------------------------------------------------
void main() {
group('ReaderHistory', () {
late ReaderHistory<String> h;
setUp(() => h = ReaderHistory<String>());
test('starts empty — canGoBack/Forward false, current null', () {
expect(h.canGoBack, isFalse);
expect(h.canGoForward, isFalse);
expect(h.current, isNull);
});
test('push one — current is that entry, no back/forward', () {
h.push('A');
expect(h.current, 'A');
expect(h.canGoBack, isFalse);
expect(h.canGoForward, isFalse);
});
test('push two — canGoBack true, canGoForward false', () {
h.push('A');
h.push('B');
expect(h.current, 'B');
expect(h.canGoBack, isTrue);
expect(h.canGoForward, isFalse);
});
test('back() after two pushes returns first entry', () {
h.push('A');
h.push('B');
final result = h.back();
expect(result, 'A');
expect(h.current, 'A');
expect(h.canGoBack, isFalse);
expect(h.canGoForward, isTrue);
});
test('forward() after back() returns second entry', () {
h.push('A');
h.push('B');
h.back();
final result = h.forward();
expect(result, 'B');
expect(h.current, 'B');
expect(h.canGoForward, isFalse);
});
test('back() at start returns null', () {
h.push('A');
expect(h.back(), isNull);
});
test('forward() at end returns null', () {
h.push('A');
h.push('B');
expect(h.forward(), isNull);
});
test('new push truncates forward history', () {
h.push('A');
h.push('B');
h.push('C');
h.back(); // now at B
h.back(); // now at A
expect(h.canGoForward, isTrue);
h.push('D'); // truncates [B, C], appends D
expect(h.current, 'D');
expect(h.canGoBack, isTrue);
expect(h.canGoForward, isFalse);
final prev = h.back();
expect(prev, 'A');
});
test('pushing duplicate of current is a no-op', () {
h.push('A');
h.push('A');
expect(h.canGoBack, isFalse); // still only one entry
expect(h.current, 'A');
});
test('three entries back/forward round-trip', () {
h.push('A');
h.push('B');
h.push('C');
expect(h.back(), 'B');
expect(h.back(), 'A');
expect(h.forward(), 'B');
expect(h.forward(), 'C');
expect(h.canGoForward, isFalse);
});
});
// -------------------------------------------------------------------------
// ReaderHistoryMixin widget integration test — uses a minimal StatefulWidget.
// -------------------------------------------------------------------------
group('ReaderHistoryMixin', () {
testWidgets('pin current / jump-to-pin round-trip', (tester) async {
String? jumpedTo;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: _MixinHarness(onJump: (v) => jumpedTo = v),
),
);
final state = tester.state<_MixinHarnessState>(find.byType(_MixinHarness));
// No pin yet.
expect(state.hasPinned, isFalse);
expect(state.pinnedEntry, isNull);
// Push 'A', then pin it.
state.historyPush('A');
await tester.pump();
state.pinCurrent();
await tester.pump();
expect(state.hasPinned, isTrue);
expect(state.pinnedEntry, 'A');
// Push 'B', jump to pin → should get 'A'.
state.historyPush('B');
await tester.pump();
final pinEntry = state.jumpToPin();
jumpedTo = pinEntry;
expect(jumpedTo, 'A');
});
testWidgets('pin replaces previous pin', (tester) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: _MixinHarness(onJump: (_) {}),
),
);
final state = tester.state<_MixinHarnessState>(find.byType(_MixinHarness));
state.historyPush('A');
state.pinCurrent();
await tester.pump();
expect(state.pinnedEntry, 'A');
state.historyPush('B');
state.pinCurrent();
await tester.pump();
expect(state.pinnedEntry, 'B'); // replaced
});
testWidgets('historyBack / historyForward returns correct entries', (tester) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: _MixinHarness(onJump: (_) {}),
),
);
final state = tester.state<_MixinHarnessState>(find.byType(_MixinHarness));
state.historyPush('X');
state.historyPush('Y');
await tester.pump();
expect(state.canGoBack, isTrue);
expect(state.canGoForward, isFalse);
final back = state.historyBack();
await tester.pump();
expect(back, 'X');
expect(state.canGoBack, isFalse);
expect(state.canGoForward, isTrue);
final fwd = state.historyForward();
await tester.pump();
expect(fwd, 'Y');
});
testWidgets('pinCurrent with empty history is a no-op', (tester) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: _MixinHarness(onJump: (_) {}),
),
);
final state = tester.state<_MixinHarnessState>(find.byType(_MixinHarness));
state.pinCurrent(); // no current entry — must not throw
await tester.pump();
expect(state.hasPinned, isFalse);
});
});
}
// ---------------------------------------------------------------------------
// Minimal harness widget that mixes in ReaderHistoryMixin.
// ---------------------------------------------------------------------------
class _MixinHarness extends StatefulWidget {
const _MixinHarness({required this.onJump});
final void Function(String?) onJump;
@override
State<_MixinHarness> createState() => _MixinHarnessState();
}
class _MixinHarnessState extends State<_MixinHarness> with ReaderHistoryMixin<String, _MixinHarness> {
@override
Widget build(BuildContext context) => const SizedBox.shrink();
}
+137
View File
@@ -0,0 +1,137 @@
/// Unit tests for the retained right-pane nav history (T-196).
///
/// [ReaderNav] records selections (even before the reader mounts), holds
/// browser-style back/forward history, exposes the latest as [current]
/// for grab-on-mount, and re-emits every navigation on the `load`
/// channel so the reader has a single load path.
library;
import 'package:clide/kernel/src/events/message_bus.dart';
import 'package:clide/kernel/src/reader_nav.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
late MessageBus bus;
late ReaderNav nav;
late List<String> loads;
setUp(() {
bus = MessageBus();
nav = ReaderNav(messages: bus, publisherId: 'builtin.decisions', dataKey: 'id');
loads = [];
bus.subscribe(publisher: 'builtin.decisions', channel: 'load').listen((m) {
final id = m.data['id'];
if (id is String) loads.add(id);
});
});
tearDown(() {
nav.dispose();
bus.dispose();
});
// Let the broadcast bus deliver.
Future<void> tick() => Future<void>.delayed(Duration.zero);
test('starts empty', () {
expect(nav.current, isNull);
expect(nav.canGoBack, isFalse);
expect(nav.canGoForward, isFalse);
expect(nav.hasPinned, isFalse);
});
test('open records the entry, sets current, and emits a load', () async {
nav.open('D-1');
expect(nav.current, 'D-1');
expect(nav.canGoBack, isFalse);
await tick();
expect(loads, ['D-1']);
});
test('a selection on the bus is recorded (retained) and emits a load', () async {
bus.publish('builtin.decisions', 'selection', {'id': 'D-9'});
await tick();
expect(nav.current, 'D-9');
expect(loads, ['D-9']);
});
test('two opens enable back; back re-emits the prior entry', () async {
nav.open('D-1');
nav.open('D-2');
expect(nav.canGoBack, isTrue);
expect(nav.canGoForward, isFalse);
nav.back();
expect(nav.current, 'D-1');
expect(nav.canGoForward, isTrue);
await tick();
expect(loads, ['D-1', 'D-2', 'D-1']);
});
test('forward after back re-emits the later entry', () async {
nav.open('D-1');
nav.open('D-2');
nav.back();
nav.forward();
expect(nav.current, 'D-2');
});
test('back at start / forward at end are no-ops', () async {
nav.open('D-1');
nav.back(); // canGoBack false → no-op
nav.forward(); // canGoForward false → no-op
await tick();
expect(loads, ['D-1']); // only the open emitted
expect(nav.current, 'D-1');
});
test('a new open truncates forward history', () {
nav.open('D-1');
nav.open('D-2');
nav.open('D-3');
nav.back(); // D-2
nav.back(); // D-1
nav.open('D-9'); // truncates D-2/D-3
expect(nav.current, 'D-9');
expect(nav.canGoForward, isFalse);
expect(nav.canGoBack, isTrue);
});
test('opening the current entry again re-emits but does not push', () async {
nav.open('D-1');
nav.open('D-1');
expect(nav.canGoBack, isFalse); // no duplicate pushed
await tick();
expect(loads, ['D-1', 'D-1']); // but both re-emit a load
});
test('pin + jumpToPin returns to the pinned entry and re-emits', () async {
nav.open('D-1');
nav.pin();
expect(nav.hasPinned, isTrue);
nav.open('D-2');
nav.open('D-3');
nav.jumpToPin();
expect(nav.current, 'D-1');
await tick();
expect(loads.last, 'D-1');
});
test('pin replaces the previous pin', () {
nav.open('D-1');
nav.pin();
nav.open('D-2');
nav.pin(); // replaces
nav.open('D-3');
nav.jumpToPin();
expect(nav.current, 'D-2');
});
test('registry retains one nav per reader id', () {
final reg = ReaderNavRegistry(bus);
addTearDown(reg.dispose);
final a = reg.navFor('builtin.markdown', dataKey: 'path');
final b = reg.navFor('builtin.markdown', dataKey: 'path');
expect(identical(a, b), isTrue);
final c = reg.navFor('builtin.decisions', dataKey: 'id');
expect(identical(a, c), isFalse);
});
}