Skip to content

web: full hostContext delivery + live host-context-changed updates#1654

Closed
cliffhall wants to merge 3 commits into
v2/1566-per-app-cspfrom
v2/1567-hostcontext-delivery
Closed

web: full hostContext delivery + live host-context-changed updates#1654
cliffhall wants to merge 3 commits into
v2/1566-per-app-cspfrom
v2/1567-hostcontext-delivery

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #1567

Delivers the full hostContext to MCP App views and keeps it live (re-implementation of the relevant slice of PR #1510).

Changes

  • createAppBridgeFactory seeds the complete hostContext via snapshotHostContext(iframe, HOST_AVAILABLE_DISPLAY_MODES)theme, styles (Mantine design tokens), displayMode, availableDisplayModes, containerDimensions — instead of theme-only. The factory's local currentTheme is removed in favor of hostContext.ts.
  • AppRenderer gains displayMode and containerRef props and pushes live changes through the SDK's sendHostContextChange (partial host-context-changed notifications) — no hand-maintained snapshot:
    • theme + styles via a MutationObserver on <html data-mantine-color-scheme>,
    • containerDimensions via a ResizeObserver on the host container (iframe fallback), gated on initialized, skipping 0×0 and value-equal repeats, plus a one-shot push on initialize,
    • displayMode whenever the prop changes (Maximize/Restore), gated on init.

The view-originated request-display-mode handler and size-changed forwarding are intentionally out of scope — they belong to #1568.

Tests

  • New AppRenderer host-context tests (theme flip incl. resolved styles, displayMode push + init gate, container-dimensions on init and on resize, observer teardown/fallback). Factory + story mocks updated for the new shape and sendHostContextChange.
  • Per-file coverage ≥90 on all four dimensions; full npm run ci passes locally.

Stacking

Stacked on #1566 (v2/1566-per-app-csp). Review/merge #1565#1566 → this in order.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

- createAppBridgeFactory now seeds the full hostContext via
  snapshotHostContext(iframe, HOST_AVAILABLE_DISPLAY_MODES): theme, styles
  (Mantine design tokens), displayMode ("inline"), availableDisplayModes, and
  containerDimensions — replacing the theme-only seed. The local currentTheme
  helper is dropped in favor of hostContext.ts.
- AppRenderer gains `displayMode` and `containerRef` props and pushes live
  host-context changes via the SDK's sendHostContextChange (partial
  host-context-changed notifications), never a hand-maintained snapshot:
  - theme + styles via a MutationObserver on <html data-mantine-color-scheme>,
  - containerDimensions via a ResizeObserver on the host container (or the
    iframe fallback), gated on `initialized`, skipping 0x0 and value-equal
    repeats, plus a one-shot push on initialize once layout settles,
  - displayMode whenever the prop changes (Maximize/Restore), gated on init.

Adds AppRenderer host-context tests (theme flip incl. resolved styles,
displayMode push + init gate, container-dimensions on init and resize, observer
teardown) and updates the factory + story mocks for the new hostContext shape
and sendHostContextChange method.

Closes #1567

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Jul 11, 2026
@cliffhall cliffhall linked an issue Jul 11, 2026 that may be closed by this pull request
@cliffhall

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 2m 41s —— View job


Code Review — PR #1654 (hostContext delivery + live updates)

  • Read the PR diff against origin/v2/1566-per-app-csp
  • Review createAppBridgeFactory.ts + hostContext.ts seeding
  • Review AppRenderer.tsx observer effects
  • Review tests + stories
  • Post review feedback

Overall this is a clean, well-documented change. The "each observer owns one field, read bridgeRef.current at callback time" design is nice, the JSDoc on the new props is genuinely useful, and the test suite is thorough (init gate, teardown, fallback-to-iframe, 0×0 / value-equal skip). A few things worth a second look before merge — none are blockers.


1. Theme observer is not gated on initialized, but displayMode/containerDimensions are

AppRenderer.tsx:280-302 — the theme MutationObserver pushes sendHostContextChange({ theme, styles }) unconditionally, whereas the container (:317-318) and displayMode (:336-337) effects both guard on initializedRef.current. This asymmetry means a theme toggle in the window between bridge construction and the view's initialized handshake will emit a host-context-changed before initialization completes.

The comments elsewhere state the init gate exists so "the notification only fires once the handshake is complete." If that ordering matters for the other two signals, it should matter for theme too — otherwise an early theme flip races the handshake. Worth confirming this is intentional; if not, gating theme on initializedRef.current (the factory already seeds the correct theme into the handshake, so nothing is lost) would make the three signals consistent.

Fix this →

2. Handshake seeds displayMode: "inline" unconditionally — the displayMode prop is never reconciled into the seed

hostContext.ts:151 hardcodes displayMode: "inline" in the snapshot, and the init-time push (AppRenderer.tsx:243-249) only sends containerDimensions, not displayMode. The displayMode effect only fires on a change to the prop (:339 dep array). So if AppRenderer ever mounts with displayMode="fullscreen" (e.g. persisted/restored state), the view is told "inline" at handshake and stays stale until the prop later changes.

This is harmless today — AppsScreen.tsx:293 doesn't pass displayMode, and the panel always opens inline — but it's a latent trap for the #1568 maximize wiring. Options: pass the actual displayMode through to snapshotHostContext, or also push displayMode in the initialized handler when it differs from "inline". At minimum worth a code comment noting the seed assumes inline-at-open.

3. Minor: init push and resize-effect dedup don't share state

The initialized handler pushes containerDimensions (:247-249), but the resize effect's last (:316) starts undefined and isn't seeded with that value. If a ResizeObserver fire after init reports the same size that was already pushed at init, the resize effect will re-push an identical containerDimensions. Idempotent and harmless, just a redundant notification — flagging only because the code deliberately dedups elsewhere.

4. Nit: containerRef in the build-effect dependency array (:262)

The build effect only reads containerRef?.current inside the initialized handler, and the sameInputs check (:179-183) deliberately ignores containerRef, so a containerRef identity change hits the StrictMode reuse path rather than rebuilding — functionally fine. But it means the dep isn't really needed for correctness (a ref object's .current is read lazily), and it leans on the reuse dance to avoid a spurious rebuild. Not worth changing unless you're already touching that block; a one-line comment on why it's a dep would help future readers.


Tests / scope: coverage of the new paths looks solid, and the intentional out-of-scope items (request-display-mode, size-changed#1568) are clearly called out. The containerDimensions describe block's ResizeObserver stub + getBoundingClientRect patching is a clean way to drive the callback. No concerns on the stories/mocks updates.
· branch v2/1567-hostcontext-delivery

…tions (PR #1654)

Address the claude[bot] review:
- Finding #1 (applied): gate the theme+styles MutationObserver on the view's
  `initialized` signal, matching the container/displayMode pushes — a theme
  flip in the construction→handshake window no longer races ui/initialize. The
  factory seeds the construction-time theme, and the first post-init flip
  carries the current value. Added tests: pre-init flip is dropped; the live
  flip test now completes the handshake first.
- Finding #2 (applied, comment): note in snapshotHostContext that the
  `displayMode: "inline"` seed assumes inline-at-open (AppsScreen always opens
  inline; #1568's displayMode push carries transitions).
- Finding #4 (applied, comment): explain why `containerRef` is a build-effect
  dep yet doesn't force a rebuild (sameInputs ignores it; read lazily).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
@cliffhall

Copy link
Copy Markdown
Member Author

Thanks — addressed in 0426f1b:

1. Theme observer not gated on initialized — applied. Good consistency catch. Gated the theme+styles MutationObserver on initializedRef.current, matching the container and displayMode pushes, so a theme flip in the construction→handshake window no longer races ui/initialize. Nothing is lost: the factory seeds the construction-time theme/styles into the handshake, and the first post-init flip carries the current value. Added two tests — a pre-init flip is dropped (locks in the gate), and the live-flip test now completes the handshake before flipping.

2. displayMode: "inline" seed never reconciled — applied (comment). Added a comment in snapshotHostContext documenting that the seed assumes inline-at-open. That holds today (AppsScreen always mounts the renderer inline; maximize is a later user action), and #1568's displayMode effect pushes any subsequent inline↔fullscreen transition. Noted the "thread the actual mode in here if a caller ever mounts already-maximized" escape hatch, per your suggestion.

3. Init push and resize dedup don't share state — declined, with reason. The redundant notification is idempotent and harmless (as you note). Sharing last between the initialized handler (inside the build effect) and the separate resize effect would require hoisting a dedicated ref and coupling two otherwise-independent effects, to suppress one benign duplicate containerDimensions right at open. Not worth the added coupling; leaving as-is.

4. containerRef in the build-effect deps — applied (comment). Added a comment explaining it's listed for exhaustive-deps completeness but doesn't force a rebuild (the sameInputs check ignores it, and the initialized handler reads containerRef?.current lazily, so the live ref is always used).

npm run ci is green (the one integration failure mid-run was the known-flaky remote-auth-branches test; passes in isolation — coverage gate re-ran clean at 271 files / 3872 tests, smoke + Storybook green).

@cliffhall

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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


Re-review — PR #1654 (round 2, commit 0426f1ba)

  • Read the diff vs origin/v2/1566-per-app-csp
  • Verify the fixes from the last round
  • Review AppRenderer.tsx observers + hostContext.ts seeding
  • Review tests + stories
  • Post review

Verified all four follow-ups from the previous review. This is good to merge — no new blockers.


✅ 1. Theme observer init gate — applied correctly

AppRenderer.tsx:300-307 — the MutationObserver callback now early-returns on !initializedRef.current, matching the container (:329) and displayMode (:348) pushes. The lazy read of initializedRef.current (not a captured value) is right, and the doc comment at :282-289 explains why nothing is lost — the factory seeds the construction-time theme/styles into the handshake, so the first post-init flip carries the current value. Both new tests are on point:

  • AppRenderer.test.tsx:350-372 — a pre-init flip is dropped (locks in the gate).
  • :319-321 — the live-flip test now completes the handshake before flipping, so it exercises the post-init path rather than accidentally passing on the ungated code.

✅ 2. displayMode: "inline" seed — documented

hostContext.ts:151-155 — clear comment on the inline-at-open assumption, why it holds today (AppsScreen always mounts inline), how #1568's displayMode effect carries later transitions, and the "thread the actual mode in here" escape hatch. Good enough for a latent-trap-with-no-current-caller; agreed a comment is the right weight.

✅ 3. Init/resize dedup coupling — declined, and I agree

The duplicate is idempotent and hoisting a shared last ref to couple two independent effects isn't worth suppressing one benign notification at open. Sound call.

✅ 4. containerRef dep comment — applied

AppRenderer.tsx:259-263 — explains it's listed for exhaustive-deps but doesn't force a rebuild (sameInputs ignores it; the initialized handler reads containerRef?.current lazily). Reads well for the next person.


Other observations (no action needed):

  • The three observers reading bridgeRef.current at callback time (rather than capturing a bridge) remains the right design — it survives the async factory resolve and the StrictMode reuse dance. hostContext.ts now being the single source of truth for both the seed (snapshotHostContext) and the live pushes (currentTheme/currentStyles/measureContainerDimensions) is a clean consolidation vs. the old factory-local currentTheme.
  • Coverage of the new gate paths is complete (pre-init drop, teardown, iframe fallback, 0×0 / value-equal skip). The ResizeObserver stub + getBoundingClientRect patching in the containerDimensions block is a tidy way to drive the callback deterministically.
  • Scope discipline is good — request-display-mode and size-changed forwarding are correctly deferred to web: honor ui/notifications/size-changed and ui/request-display-mode #1568.

LGTM. 👍
· branch v2/1567-hostcontext-delivery

@cliffhall

Copy link
Copy Markdown
Member Author

Superseded by the wave rollup PR #1660 (retargeted to v2/main), which contains this change as part of the full cumulative diff. Closing this per-issue PR in favor of the rollup so the wave merges as one PR. Branch retained until #1660 merges; the linked issue will be closed and its board card moved to Done when the rollup lands.

@cliffhall cliffhall closed this Jul 12, 2026
cliffhall added a commit that referenced this pull request Jul 12, 2026
…1660)

* web: sandbox hardening — opaque-origin inner frame, srcdoc-only, frame-ancestors

Harden the MCP Apps sandbox isolation model:

- The inner (untrusted-widget) iframe is sandboxed WITHOUT `allow-same-origin`
  (default "allow-scripts allow-forms") and any server-supplied sandbox value
  has `allow-same-origin` stripped case-insensitively, so the widget runs under
  an opaque origin and cannot reach the proxy's DOM to bypass its own CSP via
  the parent realm.
- Widget delivery is `srcdoc`-only (no document.write), which respects the
  opaque origin.
- The relay accepts the inner frame's "null" origin with
  `event.source === inner.contentWindow` as the unforgeable identity check.
- sandbox-controller.ts serves the proxy with
  `Content-Security-Policy: frame-ancestors http://127.0.0.1:* http://localhost:*`;
  fetch directives are deliberately omitted so a srcdoc iframe's inherited
  policy container does not override the per-app CSP.

Updated the isolation-model comments and the sandbox-controller integration
test asserts the frame-ancestors header (and the absence of fetch directives).

Closes #1565

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* web: enforce per-app _meta.ui.csp in the app bridge + surface resource-read failures

- createAppBridgeFactory now builds each app's `_meta.ui.csp` host-side via the
  sandbox-csp library (approveCspSources → buildSandboxCspPolicy) and wraps the
  untrusted widget HTML with wrapSandboxedHtml so the CSP <meta> is always the
  first <head> child before the HTML reaches the sandboxed iframe. The approved
  (post-filter) csp + permissions are echoed back via hostCapabilities.sandbox
  so the view sees what was granted. The csp is no longer sent inline through
  sendSandboxResourceReady — it is enforced via the wrapped meta.
- A per-app copy of HOST_CAPABILITIES prevents the sandbox echo from mutating
  the shared constant.
- New optional `onResourceError` dep on AppBridgeFactoryDeps routes resource
  read/post failures to the caller (also always console.error'd) instead of
  leaving a silent blank frame.

Closes #1566

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* web: full hostContext delivery + live host-context-changed updates

- createAppBridgeFactory now seeds the full hostContext via
  snapshotHostContext(iframe, HOST_AVAILABLE_DISPLAY_MODES): theme, styles
  (Mantine design tokens), displayMode ("inline"), availableDisplayModes, and
  containerDimensions — replacing the theme-only seed. The local currentTheme
  helper is dropped in favor of hostContext.ts.
- AppRenderer gains `displayMode` and `containerRef` props and pushes live
  host-context changes via the SDK's sendHostContextChange (partial
  host-context-changed notifications), never a hand-maintained snapshot:
  - theme + styles via a MutationObserver on <html data-mantine-color-scheme>,
  - containerDimensions via a ResizeObserver on the host container (or the
    iframe fallback), gated on `initialized`, skipping 0x0 and value-equal
    repeats, plus a one-shot push on initialize once layout settles,
  - displayMode whenever the prop changes (Maximize/Restore), gated on init.

Adds AppRenderer host-context tests (theme flip incl. resolved styles,
displayMode push + init gate, container-dimensions on init and resize, observer
teardown) and updates the factory + story mocks for the new hostContext shape
and sendHostContextChange method.

Closes #1567

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* web: honor ui/notifications/size-changed and ui/request-display-mode

AppRenderer:
- New `onSizeChange` prop forwards the view's ui/notifications/size-changed
  reports (via the bridge `sizechange` event).
- New `onRequestDisplayMode` prop backs an `onrequestdisplaymode` bridge handler
  that returns the mode the host actually applied; with no handler the request
  is declined by returning the current host-side mode (falling back to inline).

AppsScreen:
- Adds a host-controlled RendererContainer (measured for containerDimensions)
  with an inner box sized by the view-reported height (`appHeight`), so
  host→view container size and view→host size-changed don't couple.
- `handleSizeChange` records the reported height (ignored while maximized);
  `displayMode` is derived from the maximize toggle and pushed to the view;
  `handleRequestDisplayMode` filters against HOST_AVAILABLE_DISPLAY_MODES
  (declining e.g. "pip") and maps fullscreen↔inline onto the maximize state.

Adds AppRenderer tests (size forwarding, request-display-mode apply/decline/
fallback) and AppsScreen tests (frame sizing on report, no-height ignore,
fullscreen/restore/decline via request-display-mode). Story mocks gain
sendHostContextChange/addEventListener.

Closes #1568

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* web: honor ui/download-file with confirmation and http(s) allowlist

createAppBridgeFactory now advertises the `downloadFile` host capability and
attaches an `ondownloadfile` handler:

- Requires a host-mediated confirmation (window.confirm) listing the requested
  file(s); a declined prompt or an empty payload returns isError without acting.
- Inline EmbeddedResources (text or base64 blob) are written to disk via the
  shared downloadBlob object-URL anchor; base64 is decoded via base64ToBytes.
- ResourceLinks are opened in a new tab, restricted to http(s) by the shared
  isHttpUrl allowlist (javascript:/data:/file:/malformed are rejected).
- Labels shown in the confirmation are sanitized (control + format chars
  stripped, length clamped) so a server-supplied filename/URI can't inject
  newlines or reflow the prompt.
- Partial-batch success is surfaced (isError only when nothing downloaded), with
  skipped items warned.

Adds factory tests for the full download surface (text/blob/link, decline,
empty, throw, non-http rejection, label sanitization/clamping, partial batch).

Closes #1569

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

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

AppRenderer:
- New `onMessage` prop backs an `onmessage` bridge handler that surfaces the
  view's ui/message content and returns the spec-required empty result;
  with no handler the submission is declined (isError).
- New `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 on
  select/open/close/back via resetAppChannels) 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 as pinned panels below the running widget.

Adds AppRenderer tests (log forward + no-handler no-throw, ui/message
route/empty-result/decline) and AppsScreen tests (message log render + empty
result + clear-on-close, log panel expand/collapse/clear).

Closes #1570

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* web: stage partial input — replay tool-input-partial before final tool input

AppRenderer:
- New `partialInputs` prop (ordered fragments), snapshotted into pendingPartials
  at bridge-build time (read via a ref so prop churn never rebuilds the iframe)
  and cleared on dispose. flushPending replays them via
  bridge.sendToolInputPartial BEFORE the complete tool-input, per spec.

AppsScreen:
- New `partialStages` state + `handleStagePartialInput` (snapshots current form
  values). resetAppChannels gains a `keepPartials` option so Open App preserves
  the staged fragments (they're consumed by the renderer) while select/close/
  back clear them.
- Adds a "Stage partial input" control (fielded apps only) with a staged count
  and "Clear staged", wired to AppRenderer's `partialInputs`.

Adds AppRenderer tests (ordered replay before tool-input; none when omitted) and
AppsScreen tests (stage/count/clear gated on fields; staged partials survive
Open and are replayed).

Closes #1571

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* web: app lifecycle status + machine-readable data attributes on the Apps screen

AppRenderer:
- New AppRendererStatus ("loading" | "ready" | "error") + onAppStatusChange
  callback. Fires "loading" at the start of every (re)build, "ready" on the
  view's initialized signal, and "error" on a factory throw/rejection.

AppsScreen:
- appStatus/appError state; handleAppError captures the reason locally and
  forwards to the parent onError. resetAppChannels clears both.
- Surfaces the contract as data-testid="apps-form" carrying data-app-status and
  data-app-error, renders an error panel (data-testid="apps-error") in place of
  the blank frame, and tags the stage button data-testid="apps-stage".
- AppDetailPanel's Open App button gains data-testid="open-app".

App.tsx:
- Wires the bridge factory's onResourceError to a red toast so a malformed/404
  UI resource is no longer console-only (the renderer separately drives
  data-app-status, so a driver times out on never-reaching-ready and reads it).

Documents the data-testid/data-* automation contract in clients/web/README.md.
Adds AppRenderer status-transition tests and AppsScreen status/error-panel
tests.

Closes #1572

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: warn when server sandbox drops allow-scripts + revert incidental lockfile churn

Addresses the claude[bot] review on PR #1650:
- sandbox_proxy.html: log a console.warn when a server-supplied `sandbox`
  value lacks `allow-scripts` after the `allow-same-origin` strip, so a
  fail-closed (app-never-runs) sandbox is diagnosable instead of silent.
- Restore clients/{cli,launcher,tui,web}/package-lock.json to their
  v2/main state — the earlier churn (@emnapi/@napi-rs optional deps, a
  dropped version field) was incidental `npm install` regeneration
  unrelated to the sandbox change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: document CSP-vs-permissions filtering asymmetry (PR #1652)

Address the claude[bot] review's finding #2: add a comment explaining why
`csp` is injection-filtered (approveCspSources) while `permissions` is passed
through verbatim — permissions is a structured boolean object consumed only by
the sandbox proxy's buildAllowAttribute() (fixed key→token mapping), so it
never reaches the iframe sandbox/allow attribute as raw text. No logic change.

Also merges v2/1565-sandbox-hardening (review fixes) up the stack.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* test: lock in per-app HOST_CAPABILITIES isolation (PR #1652 round 2)

Add a test asserting the shared HOST_CAPABILITIES.sandbox stays undefined after
a sandboxready run sets hostCapabilities.sandbox — guards against a silent
regression if the per-app spread copy were ever dropped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: gate theme observer on initialized + document seed/dep assumptions (PR #1654)

Address the claude[bot] review:
- Finding #1 (applied): gate the theme+styles MutationObserver on the view's
  `initialized` signal, matching the container/displayMode pushes — a theme
  flip in the construction→handshake window no longer races ui/initialize. The
  factory seeds the construction-time theme, and the first post-init flip
  carries the current value. Added tests: pre-init flip is dropped; the live
  flip test now completes the handshake first.
- Finding #2 (applied, comment): note in snapshotHostContext that the
  `displayMode: "inline"` seed assumes inline-at-open (AppsScreen always opens
  inline; #1568's displayMode push carries transitions).
- Finding #4 (applied, comment): explain why `containerRef` is a build-effect
  dep yet doesn't force a rebuild (sameInputs ignores it; read lazily).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: guard 0-height size reports, distinct RendererFrame, shared payload type (PR #1656)

Address the claude[bot] review:
- #1 (applied): handleSizeChange ignores a non-positive height — a view's
  ResizeObserver can transiently fire 0 pre-layout/teardown, which would
  collapse the frame; mirrors AppRenderer's 0×0 skip. Added a test.
- #2 (applied): reinstate a distinct `RendererFrame` constant for the inner
  content-sized box so its role reads clearly vs. the outer RendererContainer.
- #3 (declined + comment): keep carrying `appHeight` across maximize→restore —
  clearing it would flash the frame to full-card height until the next
  size-changed; the carry-over restores at the prior size. Documented.
- #4 (applied): type handleSizeChange's param as AppBridgeEventMap["sizechange"]
  so the payload shape has a single source of truth with AppRenderer's prop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: distinguish links, cap batch size, comment truncation (PR #1657)

Address the claude[bot] review:
- #1 (applied): the confirmation now reads "download or open N item(s)" and
  resource_links are prefixed "↗" in the summary, so a link that opens in a tab
  is distinguishable from an embedded file that saves to disk.
- #2 (applied): cap a single ui/download-file batch at MAX_DOWNLOAD_ITEMS (20);
  an oversized batch is rejected (isError) before the prompt, with a warn.
- #3 (declined): can't check window.open's return value to detect a blocked
  popup — we pass `noopener`, and per spec window.open returns null even on a
  successful noopener open, so null can't distinguish blocked from succeeded.
- #4b (applied, comment): note the start-of-label truncation is intentional so a
  link's scheme+host stays visible for the trust decision.

Adds tests: oversized batch rejected without confirming/acting; resource-link ↗
prefix in the summary.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: skip embedded resource with neither text nor blob (PR #1657 round 2)

Address the round-2 observation: an untrusted EmbeddedResource carrying neither
`blob` nor a string `text` previously produced `new Blob([undefined])` — a file
containing the literal text "undefined", counted as success. Now such an item
is skipped (returns false, like a rejected link), so isError reflects it. Added
a test asserting no object URL is created and the batch reports isError.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: type-honest formatLogData, cap channels, log-panel a11y (PR #1658)

Address the claude[bot] review:
- #1 (applied): formatLogData coalesces JSON.stringify(undefined) → "" so the
  `: string` return type is honest for a data-less log.
- #2 (applied): cap retained messages/logs at MAX_APP_CHANNEL_ENTRIES (500) via
  appendCapped — a chatty widget no longer grows the panels/DOM without bound
  between Clear/close; oldest entries drop.
- #3 (declined): keep the Messages panel without a Clear/collapse — messages are
  sparser and already cleared on close/switch; keeping it simple.
- #4 (applied): the App-logs toggle now has aria-controls pointing at the
  Collapse region (id="apps-logs-region") for assistive tech.

Adds tests: no-data log renders (coalesce branch), 501 logs cap at 500 (oldest
dropped), aria-controls assertion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: stable id key for Messages panel (PR #1658 round 2)

Address the round-2 nit: give AppMessage a stable `id` (assigned via
nextMessageIdRef in handleMessage) and key the Messages panel on it instead of
the array index — so the appendCapped front-drop can't renumber keys once a run
exceeds the cap, matching AppLogEntry's treatment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: note partialStages intentionally survives handleOpen (PR #1659)

Address the claude[bot] review's observation #2: add a comment at the
handleOpen call site clarifying that partialStages is intentionally NOT cleared
there — AppRenderer snapshots the fragments into its own pendingPartialsRef at
build time, and the staging UI only renders while not running, so the surviving
state is invisible until the next reset drains it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

* review: align error-panel docs with behavior + gate data-app-error on running (PR #1660)

Address the claude[bot] review:
- #1 (applied, docs): the error panel renders BELOW the (blank) frame, not in
  place of it — the renderer stays mounted so a factory error still shows the
  reason alongside the iframe (gating the frame off on error would drop the
  auto-launched iframe an integration test relies on). Reworded the README row
  and the test comment to say "below the frame"; added a code comment.
- #2 (applied): gate data-app-error on `running` (like data-app-status) so the
  two attributes can't desync if a future path sets an error without running.
- #3 (no change): resource-read failures staying at data-app-status="loading"
  (toast only) is intentional and documented.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cliffhall cliffhall deleted the v2/1567-hostcontext-delivery branch July 12, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

web: full hostContext delivery + live host-context-changed updates

1 participant