diff --git a/app/lib/models/handler_state.dart b/app/lib/models/handler_state.dart index 9835fc62..2b4e0c98 100644 --- a/app/lib/models/handler_state.dart +++ b/app/lib/models/handler_state.dart @@ -401,6 +401,30 @@ class HandlerEscalationChoice { } } +/// Urgent first, then oldest-first within each band. +/// +/// The band exists for the rows that unblock a session for one tap: the engine +/// mints `high` itself, with no judge call at all, for a blocking prompt — a +/// permission request or a question the agent is stopped on right now — and a +/// flat oldest-first order filed those under every stale question already on +/// the list. +/// +/// It is NOT only that. `escalate` passes the judge's own `notify.urgency` +/// through (bridge/src/handler/engine.ts), and the decide prompt offers the +/// model both words, so a judge-authored `high` sorts into the same band and +/// wears the same marker. Nothing on the wire tells the two apart today; +/// anything that needs to must reach for `kind`, not `urgency`. +/// +/// Age still decides WITHIN a band and never across it: a `normal` that has +/// waited an hour is not the thing holding the agent up. An urgency a newer +/// bridge invents ranks as `normal` — the safe band, since it claims nothing. +int compareEscalations(HandlerEscalation a, HandlerEscalation b) { + final byUrgency = _urgencyRank(a.urgency) - _urgencyRank(b.urgency); + return byUrgency != 0 ? byUrgency : a.at.compareTo(b.at); +} + +int _urgencyRank(String urgency) => urgency == 'high' ? 0 : 1; + class HandlerEscalation { final String escalationId; final String terminalId; @@ -666,8 +690,17 @@ class HandlerState { int get pendingEscalations => sessions.values.fold(0, (n, s) => n + s.pendingEscalations); - String? get latestEscalationId => - escalations.isEmpty ? null : escalations.last.escalationId; + // Folded on `at` rather than read off the tail: [escalations] is banded by + // [compareEscalations], so `.last` is the newest NORMAL one and an urgent row + // — the only kind anything asking for "the latest" would want to land on — + // can never be it. + String? get latestEscalationId { + HandlerEscalation? newest; + for (final e in escalations) { + if (newest == null || e.at >= newest.at) newest = e; + } + return newest?.escalationId; + } /// What [terminalId] has in flight, oldest first — empty for a terminal with /// nothing outstanding, so no caller needs a null branch to ask. diff --git a/app/lib/providers/handler_discovery.dart b/app/lib/providers/handler_discovery.dart index 8bf36b07..0168ebb9 100644 --- a/app/lib/providers/handler_discovery.dart +++ b/app/lib/providers/handler_discovery.dart @@ -99,10 +99,14 @@ class HandlerAwayAttentionSinceNotifier extends Notifier { /// thunk (an absent per-session tool means the project default). ONE provider /// so the header shield, the away hint, and the explainer can never answer the /// coverage question differently for the same session. +/// [judgeCapable] is null under exactly the same condition [observable] is — +/// both are read off one descriptor, so an agent the catalog has never +/// described answers neither question rather than half of one. typedef FocusedSessionCoverage = ({ String? agent, String? agentLabel, bool? observable, + bool? judgeCapable, }); final focusedSessionCoverageProvider = @@ -120,6 +124,13 @@ final focusedSessionCoverageProvider = agent, chat: entry?.mode == 'chat', ), + // The bridge's own second question, asked the same way: an armed + // session resolves its judge as `storedJudge ?? the session's own tool` + // (observabilityFor, bridge/src/handler/engine.ts). Nothing writes a + // stored judge today, so the fallback IS the answer and the catalog + // already holds it — this predicts, it does not approximate. Whatever + // lands a judge picker owns keeping that true. + judgeCapable: catalog[agent]?.judgeCapable, ); }); diff --git a/app/lib/services/handler_service.dart b/app/lib/services/handler_service.dart index 78526437..7c3cf451 100644 --- a/app/lib/services/handler_service.dart +++ b/app/lib/services/handler_service.dart @@ -256,7 +256,7 @@ class HandlerService { // it too. This is what lets the "needs you" rows survive an app restart // or reconnect instead of leaving a badge that points at nothing. final escalations = [for (final s in sessions.values) ...s.escalations] - ..sort((a, b) => a.at.compareTo(b.at)); + ..sort(compareEscalations); // An id the bridge no longer replays has been retired there, so nothing is // left to suppress and the set cannot grow with the session's history. _answeredEscalations.retainWhere( @@ -329,7 +329,14 @@ class HandlerService { choices: msg.choices, ); _emit( - _state.copyWith(escalations: [..._state.escalations, escalation]), + _state.copyWith( + // Sorted on the way in, not appended: a status frame re-sorts + // within milliseconds, but the push is what raises the toast, and + // between the two the row the user came to answer would be sitting + // at the bottom of the list. + escalations: [..._state.escalations, escalation] + ..sort(compareEscalations), + ), ); // Read back out of the state rather than forwarded: the floors in // [_withChoiceFloors] may have withdrawn the card on the way in, and a diff --git a/app/lib/widgets/agent_panel.dart b/app/lib/widgets/agent_panel.dart index 1125c1e6..13e9d39a 100644 --- a/app/lib/widgets/agent_panel.dart +++ b/app/lib/widgets/agent_panel.dart @@ -445,18 +445,17 @@ class HandlerHeaderControl extends ConsumerWidget { notifyOnly: state.defaultNotifyOnly, agentObservable: coverage.observable, agentLabel: coverage.agentLabel, + judgeCapable: coverage.judgeCapable, ), ); } - // Arming is one tap, so this tooltip is the only place the pre-arm - // coverage answer can reach the user — an agent that reports nothing - // arms just as silently as one that is merely quiet. - final shieldTooltip = session != null - ? 'Disarm Handler' - : coverage.observable == false - ? unwatchableNotice(coverage.agentLabel) - : 'Arm Handler'; + final shieldTooltip = handlerShieldTooltip( + armed: session != null, + observable: coverage.observable, + judgeCapable: coverage.judgeCapable, + agentLabel: coverage.agentLabel, + ); return Row( mainAxisSize: MainAxisSize.min, diff --git a/app/lib/widgets/handler/handler_arm_explainer.dart b/app/lib/widgets/handler/handler_arm_explainer.dart index bfa41459..f0e0ef3f 100644 --- a/app/lib/widgets/handler/handler_arm_explainer.dart +++ b/app/lib/widgets/handler/handler_arm_explainer.dart @@ -28,10 +28,21 @@ import 'handler_item_status.dart'; /// notice saying arming would stay silent — and this is the one screen whose /// whole job is to set the expectation before the user walks away. A session /// that will say nothing has nothing to say about what it starts from. +/// +/// [judgeCapable] is the second, independent half of the same coverage answer +/// — the session IS watched, but its judge cannot run headless, so every pause +/// reaches the user. The bridge already reports it post-arm as +/// `escalate_only`, on a chip found only after walking away and coming back. +/// +/// On the `true` arm only, and only when the catalog said so outright. The +/// `false` arm already carries the stronger fact and stacking a second caveat +/// under it just dilutes the one that matters; the `null` arm has claimed +/// nothing about coverage and must not start here. String handlerArmExplainerBody({ required bool? agentObservable, String? agentLabel, bool hasOpeningPrompt = false, + bool? judgeCapable, }) { const base = "Handler watches this session while you're away. When the agent pauses " @@ -42,7 +53,7 @@ String handlerArmExplainerBody({ 'session, and queues that as your backlog.' : base; return switch (agentObservable) { - true => head, + true => judgeCapable == false ? '$head\n\n$escalateOnlyNotice' : head, false => '$base\n\n${unwatchableNotice(agentLabel)}', null => "$head\n\nThis agent hasn't reported what Handler can see here, so it " @@ -57,6 +68,7 @@ Future showHandlerArmExplainer( required bool? agentObservable, String? agentLabel, bool hasOpeningPrompt = false, + bool? judgeCapable, }) => AbConfirmDialog.show( context: context, title: 'Arm Handler', @@ -64,6 +76,7 @@ Future showHandlerArmExplainer( agentObservable: agentObservable, agentLabel: agentLabel, hasOpeningPrompt: hasOpeningPrompt, + judgeCapable: judgeCapable, ), confirmLabel: 'Arm Handler', cancelLabel: 'Not now', @@ -98,6 +111,7 @@ Future armWithFirstRunExplainer({ required bool notifyOnly, required bool? agentObservable, String? agentLabel, + bool? judgeCapable, }) async { final goal = container.read(sessionOpeningPromptsProvider)[terminalId]; if (!container.read(firstRunProvider).handlerArmedOnce) { @@ -106,6 +120,7 @@ Future armWithFirstRunExplainer({ agentObservable: agentObservable, agentLabel: agentLabel, hasOpeningPrompt: goal != null, + judgeCapable: judgeCapable, ); if (!ok) return; } diff --git a/app/lib/widgets/handler/handler_away_hint.dart b/app/lib/widgets/handler/handler_away_hint.dart index 607b6165..c288379a 100644 --- a/app/lib/widgets/handler/handler_away_hint.dart +++ b/app/lib/widgets/handler/handler_away_hint.dart @@ -60,6 +60,7 @@ class HandlerAwayHint extends ConsumerWidget { notifyOnly: handlerState.defaultNotifyOnly, agentObservable: coverage.observable, agentLabel: coverage.agentLabel, + judgeCapable: coverage.judgeCapable, ), ); }, diff --git a/app/lib/widgets/handler/handler_item_status.dart b/app/lib/widgets/handler/handler_item_status.dart index c852fc47..afb4c4a8 100644 --- a/app/lib/widgets/handler/handler_item_status.dart +++ b/app/lib/widgets/handler/handler_item_status.dart @@ -130,6 +130,30 @@ String unwatchableNotice(String? agentLabel) => const escalateOnlyNotice = "This judge can't run headless, so every pause comes to you."; +/// What the shield says before it is pressed. +/// +/// Top-level so the precedence is unit-testable without pumping the panel, the +/// same reason [handlerArmExplainerBody] is. Arming is one tap, so this tooltip +/// is the only pre-arm surface that answers EVERY time: the explainer carries +/// the same facts but sits behind FirstRunState.handlerArmedOnce, a once-ever +/// latch, while coverage is per-agent — so a user whose first arm was a capable +/// agent would meet an escalate-only one with no warning at all. +/// +/// [observable] false outranks [judgeCapable] false: a session that reports +/// nothing cannot be watched, which makes what its judge could have done moot. +/// Either being null claims nothing, exactly as the catalog requires. +String handlerShieldTooltip({ + required bool armed, + required bool? observable, + required bool? judgeCapable, + String? agentLabel, +}) { + if (armed) return 'Disarm Handler'; + if (observable == false) return unwatchableNotice(agentLabel); + if (judgeCapable == false) return escalateOnlyNotice; + return 'Arm Handler'; +} + /// Statuses an item never leaves, so they are the ones that don't count as /// remaining work. const _terminalItemStatuses = {'done', 'skipped', 'failed'}; diff --git a/app/lib/widgets/handler/handler_screen.dart b/app/lib/widgets/handler/handler_screen.dart index 7887d84b..d70d3ee8 100644 --- a/app/lib/widgets/handler/handler_screen.dart +++ b/app/lib/widgets/handler/handler_screen.dart @@ -7,6 +7,7 @@ import '../../design/ab_colors.dart'; import '../../design/ab_icons.dart'; import '../../design/ab_tokens.dart'; import '../../design/widgets/ab_chip.dart'; +import '../../design/widgets/ab_confirm_dialog.dart'; import '../../design/widgets/ab_empty_state.dart'; import '../../design/widgets/ab_icon.dart'; import '../../design/widgets/ab_list_row.dart'; @@ -18,6 +19,7 @@ import '../../design/widgets/ab_tooltip.dart'; import '../../models/handler_state.dart'; import '../../providers/providers.dart'; import '../../providers/sessions.dart'; +import '../../util/detached.dart'; import '../../util/relative_time.dart'; import 'handler_backlog_drawer.dart'; import 'handler_blocked_action_card.dart'; @@ -108,12 +110,55 @@ class HandlerScreen extends ConsumerWidget { focusedServiceOrNull(container, (s) => s.handlerService)?.reply(e, text); } - Widget meta(String terminalId, int at) => _RowMeta( + // Confirmed for one action out of four. Undoing a hard reset, a recursive + // delete or a clean touches this machine only; undoing a force push writes + // to a shared remote, and the row it is offered on is a scrolling list row + // whose whole body is the tap target (§5.2 buys prevention back as one tap). + // The dialog is the only thing standing between a thumb landing where the + // scroll stopped and a ref overwritten for everyone on it. + // + // Re-resolved after the dialog for the same reason `answer` re-resolves + // after its sheet: the focused project's session can be rebuilt while the + // dialog is open, and the build-time instance is disposed by then. + Future undo(HandlerSnapshot s) async { + if (s.action == 'force_push') { + final ok = await AbConfirmDialog.show( + context: context, + title: 'Undo this force push?', + // No promise of recovery: the bridge pins the current remote tip + // before overwriting it, but only when the ref still exists there — + // a ref already gone from the remote is restored with a bare + // `--force` and nothing pinned (snapshot.ts). + body: + 'This force-pushes the remote back to where it was before the ' + "agent's push. Whatever is on it now is overwritten.\n\n" + '${s.summary}', + confirmLabel: 'Undo force push', + destructive: true, + ); + if (!ok) return; + } + focusedServiceOrNull(container, (x) => x.handlerService)?.undo(s); + } + + // `urgent` rides the meta column rather than each row's own body: an + // escalation renders as one of three unrelated widgets (blocked card, + // decision card, plain row) and this is the only piece all three share, so + // it is the only place the marker cannot be added to two of them and + // forgotten on the third. + Widget meta(String terminalId, int at, {bool urgent = false}) => _RowMeta( sessionName: showSessionLabels ? nameOf(terminalId) : null, at: at, p: p, + urgent: urgent, ); + // The urgency test itself, once, for that same reason: spelled out at each + // of the three call sites it is three chances to omit, and a fourth row + // shape starts life without it. + Widget escalationMeta(HandlerEscalation e) => + meta(e.terminalId, e.at, urgent: e.urgency == 'high'); + return CustomScrollView( slivers: [ // Actionable first. The old order opened with the session headers, so @@ -130,7 +175,7 @@ class HandlerScreen extends ConsumerWidget { if (e.kind == 'guard_blocked') HandlerBlockedActionCard( escalation: e, - trailing: meta(e.terminalId, e.at), + trailing: escalationMeta(e), // Re-resolved through the container for the same reason // `answer` re-resolves after its sheet: the build-time // instance can be disposed by the time a tap lands. @@ -145,7 +190,7 @@ class HandlerScreen extends ConsumerWidget { else if (e.choices != null) HandlerDecisionCard( escalation: e, - trailing: meta(e.terminalId, e.at), + trailing: escalationMeta(e), // The id, not the choice: the service resolves it against // the escalation's own offered set, so the text on the wire // is always the one the bridge authored. @@ -196,7 +241,7 @@ class HandlerScreen extends ConsumerWidget { ), ], ), - trailing: meta(e.terminalId, e.at), + trailing: escalationMeta(e), onTap: () => answer(e), ), ], @@ -253,10 +298,8 @@ class HandlerScreen extends ConsumerWidget { snapshot: s, meta: meta(s.terminalId, s.at), pending: state.pendingUndo.contains(s.snapshotId), - onUndo: () => focusedServiceOrNull( - container, - (x) => x.handlerService, - )?.undo(s), + onUndo: () => + detached('HandlerScreen', 'undo snapshot', () => undo(s)), p: p, ); }, @@ -314,11 +357,16 @@ class _RowMeta extends StatelessWidget { required this.sessionName, required this.at, required this.p, + this.urgent = false, }); final String? sessionName; final int at; final AbColors p; + /// Only escalations pass this. Snapshots and activity rows are history, and + /// nothing about them is waiting on the user. + final bool urgent; + @override Widget build(BuildContext context) { final style = AbTokens.monoStyle( @@ -329,6 +377,10 @@ class _RowMeta extends StatelessWidget { mainAxisSize: MainAxisSize.min, crossAxisAlignment: CrossAxisAlignment.end, children: [ + // Above the session name, so the eye reaches it on the way down to the + // timestamp rather than after it. System-assigned data, so the mono + // uppercase chip, matching ESCALATE ONLY on the session card. + if (urgent) AbChip.system(label: 'URGENT', color: p.warning), if (sessionName != null) Text(sessionName!, style: style), Text(_fmtTime(at), style: style), ], diff --git a/app/test/models/handler_state_test.dart b/app/test/models/handler_state_test.dart index 1f86c59f..5db473f0 100644 --- a/app/test/models/handler_state_test.dart +++ b/app/test/models/handler_state_test.dart @@ -14,7 +14,59 @@ HandlerSessionState _session(String terminalId, {required int pending}) { ); } +HandlerEscalation _esc(String id, {required String urgency, required int at}) => + HandlerEscalation( + escalationId: id, + terminalId: 't1', + question: 'q', + reasoning: 'r', + draftReply: 'd', + urgency: urgency, + at: at, + ); + void main() { + group('compareEscalations', () { + test('urgent first, and oldest first inside each band', () { + final ordered = + [ + _esc('normal-old', urgency: 'normal', at: 1), + _esc('urgent-new', urgency: 'high', at: 9), + _esc('normal-new', urgency: 'normal', at: 7), + _esc('urgent-old', urgency: 'high', at: 5), + ]..sort(compareEscalations); + expect(ordered.map((e) => e.escalationId), [ + 'urgent-old', + 'urgent-new', + 'normal-old', + 'normal-new', + ]); + }); + + test('age never crosses the band', () { + // The oldest row on the list still sorts under a `high` that arrived a + // moment ago: one has been waiting, the other is holding the agent up. + final ordered = + [ + _esc('ancient', urgency: 'normal', at: 1), + _esc('fresh', urgency: 'high', at: 9999), + ]..sort(compareEscalations); + expect(ordered.first.escalationId, 'fresh'); + }); + + test('an urgency a newer bridge invents ranks as normal', () { + // The unknown band is the safe one. Reading an unrecognised word as + // urgent would let a bridge outrank the one value the app knows means + // the agent is stopped. + final ordered = + [ + _esc('invented', urgency: 'critical', at: 1), + _esc('known', urgency: 'high', at: 9), + ]..sort(compareEscalations); + expect(ordered.first.escalationId, 'known'); + }); + }); + const backlogWire = [ {'id': 'i1', 'text': 'run the tests', 'status': 'done', 'createdAt': 1}, { diff --git a/app/test/services/handler_service_test.dart b/app/test/services/handler_service_test.dart index 77b8013f..4b06902f 100644 --- a/app/test/services/handler_service_test.dart +++ b/app/test/services/handler_service_test.dart @@ -69,13 +69,15 @@ Map _escalationJson( String escalationId, { String? kind, List>? choices, + String urgency = 'normal', + int at = 1, }) => { 'escalationId': escalationId, 'question': 'q', 'reasoning': 'r', 'draftReply': 'd', - 'urgency': 'normal', - 'at': 1, + 'urgency': urgency, + 'at': at, 'kind': ?kind, 'choices': ?choices, }; @@ -90,6 +92,87 @@ const _choicesJson = [ ]; void main() { + test('a live urgent escalation outranks the ones already listed', () async { + // The push is what raises the toast, and the status frame that re-sorts + // arrives milliseconds later — but the user taps in between, and an + // appended row sat at the bottom of the very list the toast sent them to. + final t = FakeAgentTransport(); + final session = await _newSession(t); + final svc = HandlerService.fromSession(session); + final sub = session.heavyStream.listen((_) {}); + + t.emit('handler:status', { + 'projectId': 'p', + 'sessions': [ + _sessionJson( + terminalId: 't1', + pendingEscalations: 2, + state: 'needs_you', + escalations: [ + _escalationJson('waiting-1', at: 1), + _escalationJson('waiting-2', at: 2), + ], + ), + ], + }); + await Future.delayed(Duration.zero); + + t.emit('handler:escalation', { + 'projectId': 'p', + 'escalationId': 'blocking', + 'terminalId': 't1', + 'question': 'q', + 'reasoning': 'r', + 'draftReply': 'd', + 'urgency': 'high', + }); + await Future.delayed(Duration.zero); + + expect(svc.currentState.escalations.map((e) => e.escalationId), [ + 'blocking', + 'waiting-1', + 'waiting-2', + ]); + + await sub.cancel(); + await svc.dispose(); + await session.close(); + }); + + test('a replayed set comes back banded, not merely in age order', () async { + // Reconnect replays every unanswered escalation at once. Age order alone + // put the blocking one last on a list the user opened to unblock it. + final t = FakeAgentTransport(); + final session = await _newSession(t); + final svc = HandlerService.fromSession(session); + + t.emit('handler:status', { + 'projectId': 'p', + 'sessions': [ + _sessionJson( + terminalId: 't1', + pendingEscalations: 3, + state: 'needs_you', + escalations: [ + _escalationJson('waiting', at: 1), + _escalationJson('blocking', at: 3, urgency: 'high'), + _escalationJson('waiting-later', at: 2), + ], + ), + ], + }); + await Future.delayed(Duration.zero); + + expect(svc.currentState.escalations.map((e) => e.escalationId), [ + 'blocking', + 'waiting', + 'waiting-later', + ]); + + await svc.dispose(); + await session.close(); + }); + TestWidgetsFlutterBinding.ensureInitialized(); setUp(() { diff --git a/app/test/widgets/handler/handler_screen_test.dart b/app/test/widgets/handler/handler_screen_test.dart index 7851ba60..f2c7e74b 100644 --- a/app/test/widgets/handler/handler_screen_test.dart +++ b/app/test/widgets/handler/handler_screen_test.dart @@ -50,14 +50,17 @@ Map snapshotJson({ String snapshotId = 's1', String state = 'available', String? detail, + String action = 'force_push', + String trigger = 'git push --force origin feat/x', + String summary = 'pre-push SHA abc1234 recorded', }) => { 'projectId': 'p', 'snapshotId': snapshotId, 'terminalId': 't1', 'at': 1, - 'action': 'force_push', - 'trigger': 'git push --force origin feat/x', - 'summary': 'pre-push SHA abc1234 recorded', + 'action': action, + 'trigger': trigger, + 'summary': summary, 'state': state, 'detail': ?detail, }; @@ -71,14 +74,17 @@ List> choicesJson() => [ ]; /// The one-shot `handler:escalation` push. -Map escalationJson({List>? choices}) => { +Map escalationJson({ + List>? choices, + String urgency = 'high', +}) => { 'projectId': 'p', 'escalationId': 'e1', 'terminalId': 't1', 'question': 'bun or vitest?', 'reasoning': 'Affects CI wiring.', 'draftReply': 'use bun', - 'urgency': 'high', + 'urgency': urgency, 'choices': ?choices, }; @@ -575,7 +581,45 @@ void main() { debugDefaultTargetPlatformOverride = null; }); - testWidgets('a snapshot advert becomes a one-tap undo on the wire', ( + testWidgets('an urgent escalation is marked on a plain row', (tester) async { + final t = await pumpLiveHandlerScreen(tester); + t.emit('handler:status', armedStatusJson()); + await pumpDelivery(tester); + t.emit('handler:escalation', escalationJson()); + await pumpDelivery(tester); + + expect(find.text('bun or vitest?'), findsOneWidget); + expect(find.text('URGENT'), findsOneWidget); + }); + + testWidgets('and on a decision card, from the same meta column', ( + tester, + ) async { + // The payoff of hanging the marker off the shared trailing widget: three + // unrelated row shapes render an escalation, and none of them can be the + // one that forgot. + final t = await pumpLiveHandlerScreen(tester); + t.emit('handler:status', armedStatusJson()); + await pumpDelivery(tester); + t.emit('handler:escalation', escalationJson(choices: choicesJson())); + await pumpDelivery(tester); + + expect(find.byType(HandlerDecisionCard), findsOneWidget); + expect(find.text('URGENT'), findsOneWidget); + }); + + testWidgets('a normal escalation is not marked', (tester) async { + final t = await pumpLiveHandlerScreen(tester); + t.emit('handler:status', armedStatusJson()); + await pumpDelivery(tester); + t.emit('handler:escalation', escalationJson(urgency: 'normal')); + await pumpDelivery(tester); + + expect(find.text('bun or vitest?'), findsOneWidget); + expect(find.text('URGENT'), findsNothing); + }); + + testWidgets('a force push undo asks before it writes to the remote', ( tester, ) async { final t = await pumpLiveHandlerScreen(tester); @@ -586,7 +630,16 @@ void main() { expect(find.text('git push --force origin feat/x'), findsOneWidget); await tester.tap(find.text('Undo')); - await tester.pump(); + await tester.pumpAndSettle(); + + // Nothing on the wire yet — the tap opened a question, not a push. + expect(t.sent.where((m) => m['type'] == 'handler:undo'), isEmpty); + // The dialog names the entry, so the ref being overwritten is on screen + // rather than left to the row behind it. + expect(find.text('pre-push SHA abc1234 recorded'), findsWidgets); + + await tester.tap(find.text('Undo force push')); + await tester.pumpAndSettle(); final sent = t.sent.where((m) => m['type'] == 'handler:undo').toList(); expect(sent, hasLength(1)); @@ -594,6 +647,47 @@ void main() { expect(sent.single['snapshotId'], 's1'); }); + testWidgets('cancelling the force push confirm sends nothing', ( + tester, + ) async { + final t = await pumpLiveHandlerScreen(tester); + t.emit('handler:snapshot', snapshotJson()); + await pumpDelivery(tester); + + await tester.tap(find.text('Undo')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Cancel')); + await tester.pumpAndSettle(); + + expect(t.sent.where((m) => m['type'] == 'handler:undo'), isEmpty); + // And the offer survives the refusal: a cancelled undo is not a spent one. + expect(find.text('Undo'), findsOneWidget); + }); + + testWidgets('an undo that stays on this machine is still one tap', ( + tester, + ) async { + // The confirm is bought by the blast radius, not by the word "undo". A + // hard reset restores this checkout and nobody else's, so §5.2's one-tap + // prevention stands where it was always right. + final t = await pumpLiveHandlerScreen(tester); + t.emit( + 'handler:snapshot', + snapshotJson( + action: 'reset_hard', + trigger: 'git reset --hard HEAD~1', + summary: 'stashed 3 files', + ), + ); + await pumpDelivery(tester); + + expect(find.text('Hard reset'), findsOneWidget); + await tester.tap(find.text('Undo')); + await tester.pump(); + + expect(t.sent.where((m) => m['type'] == 'handler:undo'), hasLength(1)); + }); + testWidgets('a spent undo offers no tap, and a re-advert replaces its row', ( tester, ) async { @@ -625,7 +719,10 @@ void main() { expect(find.text('remote rejected the push'), findsOneWidget); await tester.tap(find.text('Retry undo')); - await tester.pump(); + await tester.pumpAndSettle(); + // A retry is the same push as the first attempt, so it asks the same way. + await tester.tap(find.text('Undo force push')); + await tester.pumpAndSettle(); expect(t.sent.where((m) => m['type'] == 'handler:undo'), hasLength(1)); }); diff --git a/app/test/widgets/handler_arm_onboarding_test.dart b/app/test/widgets/handler_arm_onboarding_test.dart index ffc1845b..f0dfd583 100644 --- a/app/test/widgets/handler_arm_onboarding_test.dart +++ b/app/test/widgets/handler_arm_onboarding_test.dart @@ -84,6 +84,55 @@ void main() { ); }); + test('a watched session with no headless judge says so before arming', () { + // The bridge already knows this the moment the session arms and shows it + // as ESCALATE ONLY on the card — a chip you find by walking away and + // coming back to a session that woke you for everything. + final body = handlerArmExplainerBody( + agentObservable: true, + judgeCapable: false, + ); + expect(body, startsWith(base)); + expect(body, endsWith(escalateOnlyNotice)); + }); + + test('a headless judge adds nothing', () { + expect( + handlerArmExplainerBody(agentObservable: true, judgeCapable: true), + base, + ); + }); + + test('the judge caveat reads after the seeded goal', () { + final body = handlerArmExplainerBody( + agentObservable: true, + judgeCapable: false, + hasOpeningPrompt: true, + ); + expect(body, contains('queues that as your backlog')); + expect(body, endsWith(escalateOnlyNotice)); + }); + + test('an unwatchable agent does not stack a second caveat', () { + // It reports nothing Handler can act on, so what its judge could have + // done is moot — and a hedge under the stronger fact only dilutes it. + final body = handlerArmExplainerBody( + agentObservable: false, + agentLabel: 'Claude Code', + judgeCapable: false, + ); + expect(body, isNot(contains(escalateOnlyNotice))); + expect(body, endsWith(unwatchableNotice('Claude Code'))); + }); + + test('unknown coverage claims nothing about the judge either', () { + final body = handlerArmExplainerBody( + agentObservable: null, + judgeCapable: null, + ); + expect(body, isNot(contains(escalateOnlyNotice))); + }); + test('the coverage warning still reads last', () { final body = handlerArmExplainerBody( agentObservable: false, @@ -94,6 +143,64 @@ void main() { }); }); + group('handlerShieldTooltip', () { + // The explainer's copy matrix has its own group above. This is the surface + // that answers every time, and the two must agree about precedence. + test('an armed session offers only the way out', () { + expect( + handlerShieldTooltip(armed: true, observable: false, judgeCapable: false), + 'Disarm Handler', + ); + }); + + test('an escalate-only agent is named before the arm, not after', () { + expect( + handlerShieldTooltip( + armed: false, + observable: true, + judgeCapable: false, + ), + escalateOnlyNotice, + ); + }); + + test('unwatchable outranks escalate-only', () { + // Both true of the same agent says one thing: it reports nothing. What + // its judge could have done never comes up. + expect( + handlerShieldTooltip( + armed: false, + observable: false, + judgeCapable: false, + agentLabel: 'Claude Code', + ), + unwatchableNotice('Claude Code'), + ); + }); + + test('a fully covered agent gets the plain label', () { + expect( + handlerShieldTooltip( + armed: false, + observable: true, + judgeCapable: true, + ), + 'Arm Handler', + ); + }); + + test('an undescribed agent claims neither fault', () { + expect( + handlerShieldTooltip( + armed: false, + observable: null, + judgeCapable: null, + ), + 'Arm Handler', + ); + }); + }); + group('shieldShowsLabel', () { test('labels only before the first arm and never while armed', () { expect(shieldShowsLabel(armedOnce: false, sessionArmed: false), isTrue); diff --git a/bridge/src/handler/backlog.ts b/bridge/src/handler/backlog.ts index 8c972ee9..0b4b4e8b 100644 --- a/bridge/src/handler/backlog.ts +++ b/bridge/src/handler/backlog.ts @@ -318,9 +318,11 @@ export function allTerminal(backlog: InstructionItem[]): boolean { // Every field rendered into a prompt is extraction output, ids included, so any of // them can carry a newline that would forge an extra list line — and a forged line // hands the evaluator an id the user-authored vocabulary §2.1 rests on never -// contained. The ONE copy of that rule, here because this module imports nothing: -// the extraction prompt renders the same fields for the same reason, and -// reply-shape re-exports it for the engine's push bodies. +// contained. The ONE copy of that rule, and it lives here because this module +// sits BELOW every consumer of it: its only imports are zod and ./evidence, +// which imports nothing at all, so extract/reply-shape/engine can all reach it +// with no cycle. The extraction prompt renders the same fields for the same +// reason, and reply-shape re-exports it for the engine's push bodies. export function oneLine(s: string): string { return s.replace(/\s+/g, " ").trim(); } diff --git a/bridge/src/handler/reply-shape.ts b/bridge/src/handler/reply-shape.ts index 7b5e450f..4018dc4e 100644 --- a/bridge/src/handler/reply-shape.ts +++ b/bridge/src/handler/reply-shape.ts @@ -13,7 +13,8 @@ const CONTROL_CHARS = /[\x00-\x1f\x7f]/; // Re-exported because the engine flattens the same way for its push bodies, where // a stray newline renders as a broken multi-line notification. It is DEFINED in -// backlog.ts, which is import-free: the prompt renderers there and this module +// backlog.ts because that module sits below every consumer of the rule and can +// be reached from any of them: the prompt renderers there and this module // enforce one flattening rule, and a second copy is a second place to keep it. export { oneLine };