web: surface ui/message and app logging in panels below the running app

[cliffhall]

Closes #1570

Surfaces two view→host message streams below the running widget (re-implementation of the relevant slice of PR #1510).

Changes

AppRenderer

• onMessage prop backs an onmessage bridge handler that surfaces the view's ui/message content and returns the spec-required empty result (no conversation leak); with no handler the submission is declined (isError).
• onLog prop forwards MCP notifications/message log entries via the bridge loggingmessage event, honoring the advertised logging capability.

AppsScreen

• Adds messages/appLogs state (cleared with the reported height via resetAppChannels on select/open/close/back) and handleMessage/handleLog.
• Renders a Messages from app panel (data-testid="apps-messages", reusing ContentViewer) and a default-expanded collapsible App logs panel (data-testid="apps-logs", reusing LogLevelBadge) with logger names and a Clear button — both pinned below the running widget. All new subcomponents are .withProps() constants; no inline styles / raw colors.

Tests

• AppRenderer: log forward + no-handler no-throw; ui/message route / empty-result / decline.
• AppsScreen: message log render + empty result + clear-on-close; log panel expand/collapse/clear.
• Per-file coverage ≥90 on all four dimensions; full npm run ci passes locally.

Stacking

Stacked on #1569 (v2/1569-download-file). Merge order: #1565 → #1566 → #1567 → #1568 → #1569 → this.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 3m 42s —— View job

---

PR Review — surface ui/message and app logging

I read the four changed files and cross-checked the bridge contract against the installed @modelcontextprotocol/ext-apps@1.7.1 type definitions (app-bridge.d.ts / spec.types.d.ts).

Verdict: LGTM. This is a clean, spec-faithful slice with solid test coverage. A few minor, non-blocking observations below.

• Read the diff (AppRenderer + AppsScreen + tests)
• Analyze correctness / spec-compliance / conventions
• Post review feedback

✅ What's correct

• ui/message return contract matches the spec exactly. McpUiMessageResult is { isError?: boolean; [key: string]: unknown }, and the bridge's own JSDoc example returns {} on success / { isError: true } on rejection. AppRenderer.tsx:320-325 does precisely that, and the "no conversation leak" comment aligns with the spec's own warning against returning conversation content. 👍
• Event/handler names are right. loggingmessage is a real key in AppBridgeEventMap (app-bridge.d.ts:106), and using addEventListener("loggingmessage", …) over the deprecated onloggingmessage setter is the recommended path. bridge.onmessage is the correct request-handler setter (parallel to the existing onrequestdisplaymode).
• logging host capability is actually advertised (createAppBridgeFactory.ts:52 logging: {}), so honoring the notifications here is consistent — otherwise the view would never emit them.
• The ref-latch pattern is applied correctly. onMessageRef / onLogRef are refreshed in the render-phase useEffect and read lazily inside the bridge callbacks, so changing handler identity never forces a bridge rebuild — matching the existing onSizeChange / onRequestDisplayMode treatment.
• Lifecycle is tight. resetAppChannels() is wired into select/open/close/back and re-expands the log panel, so each run starts clean. Types line up (AppMessage is structurally identical to McpUiMessageRequest["params"]).
• Conventions honored — all new subcomponents are .withProps() constants, no inline styles, no raw colors, data-testids present, reuses ContentViewer / LogLevelBadge. Tests cover forward/no-handler/route/decline plus render/clear/expand-collapse.

🟡 Minor suggestions (non-blocking)

1. formatLogData is typed : string but can return undefined. JSON.stringify(undefined) returns the value undefined (not the string "undefined"), so if a log arrives with no data, entry.text is actually undefined despite the annotation. LoggingMessageNotification.params.data is spec-required so this is unlikely in practice, but a ?? String(data) (or ?? "") after the JSON.stringify would make the return type honest. AppsScreen.tsx:formatLogData

2. appLogs / messages grow unbounded during a run. They're only cleared on open/close/switch, so a chatty app emitting logs in a loop accumulates entries (and DOM rows) for the life of the run. Consider a soft cap / ring buffer (e.g. keep the last N and note truncation) if you expect noisy widgets. Low priority given the Clear button and reset-on-close.

3. Panel affordance asymmetry. The App logs panel is collapsible and has a Clear button, but Messages from app has neither. That's a defensible choice (messages are usually sparser), just calling out the inconsistency in case a Clear on the message panel is also wanted.

4. A11y nit: the App-logs toggle sets aria-expanded but has no aria-controls pointing at the Collapse region. Adding an id on the collapse content + aria-controls would make the relationship explicit to assistive tech.

None of these block merge — they're polish. Nice work keeping the host-side "no leak" semantics front-and-center.

[cliffhall]

Thanks — LGTM appreciated. Addressed the polish items in 0190220:

1. formatLogData type honesty — applied. JSON.stringify(data) ?? "" so a data-less log yields "" rather than the value undefined leaking past the : string annotation. Added a test emitting a loggingmessage with no data.

2. Unbounded appLogs/messages growth — applied. Added appendCapped with MAX_APP_CHANNEL_ENTRIES = 500; both channels keep only the most recent N (oldest dropped), so a chatty widget can't grow the panels/DOM without bound between Clear/close. Added a test: 501 logs → count clamps at 500, oldest dropped, newest retained.

3. Panel affordance asymmetry (Messages has no Clear/collapse) — declined, with reason. Kept intentionally: messages are typically far sparser than logs, and the panel is already cleared on close/switch (and now capped). Adding a second Clear/collapse pair felt like noise for the common case. Easy to revisit if noisy-message widgets show up.

4. A11y aria-controls — applied. The App-logs toggle now sets aria-controls="apps-logs-region" and the Collapse carries that id, so assistive tech gets the explicit toggle↔region relationship. Added an assertion.

npm run ci is green.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 13s —— View job

---

Re-review — follow-up commit 01902200

I re-read the four changed files against origin/v2/1569-download-file and focused on the follow-up commit that addressed my earlier polish notes.

Verdict: still LGTM. The three "applied" items are implemented correctly and each is covered by a new test. The one declined item (Clear/collapse on the Messages panel) has a reasonable rationale.

• Read the diff vs origin/v2/1569-download-file
• Review 01902200 follow-up (formatLogData, appendCapped, a11y)
• Post review feedback

✅ Follow-up items verified

1. formatLogData type honesty — JSON.stringify(data) ?? "" (AppsScreen.tsx) makes the : string annotation honest for a data-less log, and the String(data) catch is correctly annotated with a justified v8 ignore (postMessage payloads are already structured-clone-safe, so the throw path is unreachable). null/number/boolean data all stringify fine; only genuine undefined hits the ?? "". Covered by "renders a log notification that carries no data without crashing" (AppsScreen.test.tsx:520).
2. Unbounded growth — appendCapped<T> + MAX_APP_CHANNEL_ENTRIES = 500 keeps the most-recent N and drops the oldest, applied to both handleMessage and handleLog. Slice math is correct (grown.slice(grown.length - MAX)). Covered by the 501→500 clamp test (AppsScreen.test.tsx:545).
3. A11y aria-controls — the toggle now sets aria-controls="apps-logs-region" and the Collapse carries that id; the region always exists whenever the toggle does (both live inside the same appLogs.length > 0 block), so the reference never dangles. Asserted at AppsScreen.test.tsx:515.

🟡 One tiny, non-blocking nit

• Message list key vs. the new cap. appLogs rows use the stable entry.id, but the Messages panel still uses key={index} (and an [{index}] label). Now that appendCapped can drop from the front, once a run exceeds 500 messages the indices shift and the keys/labels renumber — React would reconcile by position rather than identity. It's purely cosmetic and realistically unreachable (500 ui/message submissions in one run is implausible), but giving messages the same stable-id treatment as logs would make the two panels consistent. Not worth blocking on. Fix this →

Everything else from the first pass still holds — spec-faithful ui/message empty-result contract, correct loggingmessage/onmessage bridge wiring, tight resetAppChannels lifecycle, and Mantine conventions (all .withProps() constants, no inline styles / raw colors). Nice, clean follow-up.

(CI run 29171920162 is still in progress at review time.)
• v2/1570-message-logs

[cliffhall]

Applied the round-2 nit in a follow-up: AppMessage now carries a stable id (assigned in handleMessage via nextMessageIdRef), and the Messages panel keys on message.id instead of the array index — so the appendCapped front-drop can't renumber React keys, consistent with AppLogEntry. The [{index}] label stays (it's a positional display, not a key). npm run ci green. Thanks for the reviews!