From 1928ede992bb71ba97c6e9650f66ff860d1c0e90 Mon Sep 17 00:00:00 2001 From: Brian Love Date: Tue, 12 May 2026 21:37:17 -0700 Subject: [PATCH] =?UTF-8?q?fix(chat,examples-chat):=20interrupt=20panel=20?= =?UTF-8?q?visual=20pass=20=E2=80=94=20one=20source=20of=20truth,=20clean?= =?UTF-8?q?=20reason=20text,=20theme-aware?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove inline primitive from the chat composition. The docked in the demo shell is the canonical action surface; the primitive is still exported via public-api for downstream consumers who want to render it themselves. - Replace raw JSON envelope dump with a human-readable reason. The panel now reads `value.reason` when it's a string, plain string values, or falls back to a pretty-printed JSON dump only as a last resort. - Use the existing chat tokens throughout the panel (--ngaf-chat-surface, --ngaf-chat-text, --ngaf-chat-text-muted, --ngaf-chat-separator, --ngaf-chat-warning-text) so the panel respects light/dark themes instead of being dark-only. - Refresh button hierarchy: Accept primary (filled), Edit/Respond secondary (bordered), Ignore tertiary (muted text). New --primary/--secondary/--tertiary BEM modifiers. - Demo-shell layout: clear the input + subagents strip via bottom: calc(80px + var(--demo-shell-interrupt-offset, 0px)); bump z-index to 999 (under search palette); drop the dark hardcoded bg. - Fix: hydrate pending interrupts from history on thread reload. The stream-manager bridge now projects tasks[i].interrupts from the latest checkpoint onto interrupt$ / interrupts$ when refreshHistory runs, so reloading a paused thread re-renders the interrupt panel instead of silently letting the user send a message into a BadRequestError. - Update smoke checklist with the new acceptance criteria. --- .../content/docs/chat/api/api-docs.json | 2 +- .../src/app/shell/demo-shell.component.css | 12 ++- examples/chat/smoke/CHECKLIST.md | 8 +- .../chat-interrupt-panel.component.spec.ts | 72 +++++++++++++-- .../chat-interrupt-panel.component.ts | 89 +++++++++++++------ .../lib/compositions/chat/chat.component.ts | 4 +- .../lib/internals/stream-manager.bridge.ts | 44 +++++++++ 7 files changed, 186 insertions(+), 45 deletions(-) diff --git a/apps/website/content/docs/chat/api/api-docs.json b/apps/website/content/docs/chat/api/api-docs.json index 713f48ef..0966e427 100644 --- a/apps/website/content/docs/chat/api/api-docs.json +++ b/apps/website/content/docs/chat/api/api-docs.json @@ -2750,7 +2750,7 @@ "optional": false }, { - "name": "interruptPayload", + "name": "interruptReason", "type": "Signal", "description": "", "optional": false diff --git a/examples/chat/angular/src/app/shell/demo-shell.component.css b/examples/chat/angular/src/app/shell/demo-shell.component.css index 7e4b9b97..895d7bda 100644 --- a/examples/chat/angular/src/app/shell/demo-shell.component.css +++ b/examples/chat/angular/src/app/shell/demo-shell.component.css @@ -51,15 +51,13 @@ .demo-shell__interrupt-panel { position: fixed; left: 50%; - bottom: 96px; + bottom: calc(80px + var(--demo-shell-interrupt-offset, 0px)); transform: translateX(-50%); - z-index: 998; + z-index: 999; width: min(640px, calc(100vw - 32px)); - background: #1a1d23; - border: 1px solid #4f8df5; - border-radius: 10px; - box-shadow: 0 6px 24px rgba(0, 0, 0, 0.45); - padding: 12px 14px; + max-width: min(640px, calc(100vw - 32px)); + box-shadow: 0 6px 24px rgba(0, 0, 0, 0.2); + border-radius: var(--ngaf-chat-radius-card, 10px); } .demo-shell__subagents { diff --git a/examples/chat/smoke/CHECKLIST.md b/examples/chat/smoke/CHECKLIST.md index 7a1190fa..bd1e1202 100644 --- a/examples/chat/smoke/CHECKLIST.md +++ b/examples/chat/smoke/CHECKLIST.md @@ -241,11 +241,17 @@ renders correctly both during streaming and after completion. - [ ] Click "Demo: ask for approval before a sensitive action" welcome suggestion - [ ] AI begins planning, then calls `request_approval` tool — graph pauses -- [ ] Interrupt panel appears above the input with the AI's reason text +- [ ] Interrupt panel renders ONCE — no duplicate inline banner inside the message stream +- [ ] Panel title reads "Agent paused" +- [ ] Panel body shows the human-readable `reason` text (NOT the raw JSON envelope like `{"type":"approval_request",...}`) +- [ ] Panel respects color scheme: in light mode the bg is light + text dark; in dark mode bg dark + text light +- [ ] Button hierarchy is visible: Accept primary (filled), Edit/Respond secondary (bordered), Ignore tertiary (muted text only) +- [ ] Panel docks above the chat input without overlapping it, even when the subagents strip is also present - [ ] Click Accept — graph resumes with `'approved'`; AI proceeds with the plan - [ ] (New conversation, click suggestion again) — Click Edit, type a custom response in the prompt — graph resumes with the typed text - [ ] (New conversation, click suggestion again) — Click Ignore — graph resumes with `'denied'`; AI acknowledges and stops - [ ] During pause: server state shows the interrupt — `curl localhost:2024/threads//state` reports `next` includes the interrupted node and a pending interrupt value +- [ ] On thread reload while paused at an interrupt: reload the page — interrupt panel re-appears with the same reason text (hydrated from checkpoint history) ## Citations diff --git a/libs/chat/src/lib/compositions/chat-interrupt-panel/chat-interrupt-panel.component.spec.ts b/libs/chat/src/lib/compositions/chat-interrupt-panel/chat-interrupt-panel.component.spec.ts index 5129316e..e8a5f41f 100644 --- a/libs/chat/src/lib/compositions/chat-interrupt-panel/chat-interrupt-panel.component.spec.ts +++ b/libs/chat/src/lib/compositions/chat-interrupt-panel/chat-interrupt-panel.component.spec.ts @@ -1,7 +1,10 @@ // SPDX-License-Identifier: MIT import { describe, it, expect } from 'vitest'; -import { signal, computed } from '@angular/core'; -import { getInterruptFromAgent, ChatInterruptPanelComponent } from './chat-interrupt-panel.component'; +import { + getInterruptFromAgent, + interruptReasonText, + ChatInterruptPanelComponent, +} from './chat-interrupt-panel.component'; import type { InterruptAction } from './chat-interrupt-panel.component'; import { mockAgent } from '../../testing/mock-agent'; import type { AgentInterrupt } from '../../agent/agent-interrupt'; @@ -41,18 +44,53 @@ describe('getInterruptFromAgent()', () => { }); }); +describe('interruptReasonText()', () => { + it('returns the reason string when value.reason is a string', () => { + const ix: AgentInterrupt = { + id: 'int-1', + value: { type: 'approval_request', reason: 'User asked for deletion of /etc/important' }, + resumable: true, + }; + expect(interruptReasonText(ix)).toBe('User asked for deletion of /etc/important'); + }); + + it('falls back to a JSON dump when value has no string reason field', () => { + const ix: AgentInterrupt = { + id: 'int-2', + value: { type: 'approval_request', meta: { count: 3 } }, + resumable: true, + }; + const out = interruptReasonText(ix); + expect(out).toContain('"type": "approval_request"'); + expect(out).toContain('"count": 3'); + }); + + it('returns string value directly when value is a plain string', () => { + const ix: AgentInterrupt = { id: 'int-3', value: 'Please confirm', resumable: true }; + expect(interruptReasonText(ix)).toBe('Please confirm'); + }); + + it('returns "" when interrupt is undefined', () => { + expect(interruptReasonText(undefined)).toBe(''); + }); + + it('falls back to JSON when reason is not a string (e.g. nested object)', () => { + const ix: AgentInterrupt = { + id: 'int-4', + value: { reason: { nested: 'oops' } }, + resumable: true, + }; + const out = interruptReasonText(ix); + expect(out).toContain('"nested": "oops"'); + }); +}); + describe('ChatInterruptPanelComponent', () => { it('is defined', () => { expect(ChatInterruptPanelComponent).toBeDefined(); expect(typeof ChatInterruptPanelComponent).toBe('function'); }); - it('has interruptPayload as a prototype member', () => { - // interruptPayload is a computed signal defined in the constructor body — - // it lives on instances, not the prototype. Verify via class existence. - expect(ChatInterruptPanelComponent).toBeDefined(); - }); - it('exports InterruptAction union type (compile-time check)', () => { const action: InterruptAction = 'accept'; expect(['accept', 'edit', 'respond', 'ignore']).toContain(action); @@ -62,4 +100,22 @@ describe('ChatInterruptPanelComponent', () => { const validActions: InterruptAction[] = ['accept', 'edit', 'respond', 'ignore']; expect(validActions).toHaveLength(4); }); + + it('template assigns primary/secondary/tertiary classes to buttons', () => { + // The component template is a string literal in the @Component decorator. + // Assert the class strings appear so a regression that drops one is caught. + const meta = Reflect.getOwnPropertyDescriptor(ChatInterruptPanelComponent, 'ɵcmp')?.value as + | { template?: string } + | undefined; + // Fall back: source check via the component's template string accessor. + // Some Angular versions expose template via ɵcmp; if absent, skip — the + // class hierarchy is also covered by the smoke checklist. + if (meta?.template) { + expect(meta.template).toContain('chat-interrupt-panel__btn--primary'); + expect(meta.template).toContain('chat-interrupt-panel__btn--secondary'); + expect(meta.template).toContain('chat-interrupt-panel__btn--tertiary'); + } else { + expect(true).toBe(true); + } + }); }); diff --git a/libs/chat/src/lib/compositions/chat-interrupt-panel/chat-interrupt-panel.component.ts b/libs/chat/src/lib/compositions/chat-interrupt-panel/chat-interrupt-panel.component.ts index 8ba3334e..5e604e52 100644 --- a/libs/chat/src/lib/compositions/chat-interrupt-panel/chat-interrupt-panel.component.ts +++ b/libs/chat/src/lib/compositions/chat-interrupt-panel/chat-interrupt-panel.component.ts @@ -21,6 +21,22 @@ export function getInterruptFromAgent(agent: Agent): AgentInterrupt | undefined return agent.interrupt?.(); } +/** + * Extracts a human-readable reason from an interrupt value. When the value + * is an object with a string `reason` field, returns that directly. Falls + * back to a JSON dump so consumers always see something rather than nothing. + * Exported for unit testing. + */ +export function interruptReasonText(interrupt: AgentInterrupt | undefined): string { + if (!interrupt) return ''; + const v = interrupt.value as { reason?: unknown } | undefined; + if (v && typeof v === 'object' && typeof (v as { reason?: unknown }).reason === 'string') { + return (v as { reason: string }).reason; + } + if (typeof v === 'string') return v; + return JSON.stringify(v ?? '', null, 2); +} + @Component({ selector: 'chat-interrupt-panel', standalone: true, @@ -29,12 +45,12 @@ export function getInterruptFromAgent(agent: Agent): AgentInterrupt | undefined CHAT_HOST_TOKENS, ` .chat-interrupt-panel { - background: var(--ngaf-chat-warning-bg); - color: var(--ngaf-chat-warning-text); + background: var(--ngaf-chat-surface); + color: var(--ngaf-chat-text); + border: 1px solid var(--ngaf-chat-separator); border-left: 3px solid var(--ngaf-chat-warning-text); border-radius: var(--ngaf-chat-radius-card); padding: 12px 16px; - margin: 0 var(--ngaf-chat-space-6) var(--ngaf-chat-space-2); font-size: var(--ngaf-chat-font-size-sm); } .chat-interrupt-panel__title { @@ -43,51 +59,80 @@ export function getInterruptFromAgent(agent: Agent): AgentInterrupt | undefined display: flex; align-items: center; gap: 6px; + color: var(--ngaf-chat-text); + } + .chat-interrupt-panel__title svg { + color: var(--ngaf-chat-warning-text); } .chat-interrupt-panel__body { - margin: 0 0 8px; - opacity: 0.95; + margin: 0 0 12px; + color: var(--ngaf-chat-text-muted); + white-space: pre-wrap; } .chat-interrupt-panel__actions { display: flex; gap: 8px; flex-wrap: wrap; + align-items: center; } .chat-interrupt-panel__btn { - padding: 4px 12px; + padding: 6px 14px; font-size: var(--ngaf-chat-font-size-sm); border-radius: var(--ngaf-chat-radius-button); border: 0; cursor: pointer; + font-weight: 500; + transition: transform 200ms ease, opacity 200ms ease; + } + .chat-interrupt-panel__btn:hover { transform: scale(1.03); } + .chat-interrupt-panel__btn--primary { background: var(--ngaf-chat-primary); color: var(--ngaf-chat-on-primary); - transition: transform 200ms ease; } - .chat-interrupt-panel__btn:hover { transform: scale(1.03); } .chat-interrupt-panel__btn--secondary { background: transparent; - color: var(--ngaf-chat-warning-text); - border: 1px solid var(--ngaf-chat-warning-text); + color: var(--ngaf-chat-text); + border: 1px solid var(--ngaf-chat-separator); + } + .chat-interrupt-panel__btn--tertiary { + background: transparent; + color: var(--ngaf-chat-text-muted); + padding: 6px 8px; + font-size: var(--ngaf-chat-font-size-sm); + } + .chat-interrupt-panel__btn--tertiary:hover { + color: var(--ngaf-chat-text); } `, ], template: ` @if (interrupt()) {