Skip to content

feat(web): extract hostContext utilities for the Apps host#1591

Open
cliffhall wants to merge 1 commit into
v2/mainfrom
v2/1559-hostcontext-utils
Open

feat(web): extract hostContext utilities for the Apps host#1591
cliffhall wants to merge 1 commit into
v2/mainfrom
v2/1559-hostcontext-utils

Conversation

@cliffhall

@cliffhall cliffhall commented Jul 1, 2026

Copy link
Copy Markdown
Member

Closes #1559

Summary

Wave 1 of the PR #1510 decomposition (tracking issue #1579). Extracts the MCP Apps host-context logic into a new pure utilities module at clients/web/src/components/elements/AppRenderer/hostContext.ts, so the follow-up hostContext-delivery / size-changed features can build on a testable, renderer-agnostic base.

Scope is deliberately narrow: new module + test only. The AppRenderer.tsx / AppsScreen.tsx / createAppBridgeFactory.ts rewrites that consume these utilities belong to Wave 2 and are untouched here (the existing inline currentTheme() in createAppBridgeFactory.ts is left in place for Wave 2 to fold in).

What's added

hostContext.ts:

  • currentTheme() — reads the resolved Mantine color scheme from <html data-mantine-color-scheme> (DOM, not React state, to keep the bridge-factory identity stable across theme flips), with a matchMedia fallback.
  • currentStyles() — maps the inspector's Mantine / --inspector-* design tokens to a spec McpUiHostStyles snapshot via STYLE_VARIABLE_SOURCES; omits unresolved tokens and returns undefined rather than advertising an empty styles object.
  • measureContainerDimensions() — whole-pixel { width, height } measurement of the host container; undefined for a 0×0 / unattached element.
  • snapshotHostContext() — assembles the initial McpUiHostContext seed (theme, inline display mode, available modes, plus styles/dimensions when they resolve), omitting optional fields so the SDK diff stays accurate.

Testing

  • New hostContext.test.ts — 20 tests covering theme resolution (attribute, matchMedia fallback, SSR guards), token mapping / trimming / empty-omission, dimension rounding and zero-size handling, and snapshot assembly. Global stubs (vi.stubGlobal) exercise the non-DOM guard branches so no v8 ignore is needed.
  • 100% coverage on all four dimensions for the new module; validate:web and the full web coverage gate pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

@cliffhall @claude
feat(web): extract hostContext utilities for the Apps host (closes #1559
5831b5c 
)

Add clients/web/src/components/elements/AppRenderer/hostContext.ts — a pure
utilities module for the MCP Apps host:

- currentTheme(): read the resolved Mantine color scheme from the DOM
- currentStyles(): map the inspector's Mantine/-inspector design tokens to a
  McpUiHostStyles snapshot via STYLE_VARIABLE_SOURCES
- measureContainerDimensions(): whole-pixel container measurement
- snapshotHostContext(): assemble the initial McpUiHostContext seed

Wave 1 of the PR #1510 decomposition (#1579). Pure extraction only — the
AppRenderer/AppsScreen/createAppBridgeFactory rewrites belong to Wave 2.
Full unit coverage (100% lines/statements/functions/branches).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Jul 1, 2026
@cliffhall cliffhall linked an issue Jul 1, 2026 that may be closed by this pull request
web: extract hostContext utilities for the Apps host (AppRenderer/hostContext.ts) #1559
Open
@cliffhall

cliffhall commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Review — hostContext utilities (Wave 1)

  • Read the new module + tests
  • Cross-check token mappings against --inspector-* / Mantine tokens
  • Check for correctness / edge-case issues
  • Verify test coverage & alignment with repo guidelines

Overall this is a clean, well-scoped extraction. The module is genuinely pure/renderer-agnostic, the SSR/non-DOM guards are deliberate and tested, and the "omit optional fields rather than set them undefined" discipline (so the SDK diff stays accurate) is applied consistently in both currentStyles() and snapshotHostContext(). I found no correctness bugs. A few observations below, all minor.

✅ Verified

  • Every mapped --inspector-* token exists in App.css :root. I checked all ten referenced (surface-card, surface-subtle, text-secondary, text-inverse, log-info, log-error, status-connected, log-warning, border-default, border-subtle) — no dangling tokens that would silently resolve to empty and get dropped.
  • Edge handling is solid: Math.round then <= 0 guard means a sub-pixel 0.4 collapses to undefined rather than seeding a {width:0}; single-zero-dimension case is covered; whitespace-only CSS values are trimmed and omitted.
  • Test coverage matches the 100%/four-dimension claim, and the vi.stubGlobal approach to exercise the typeof window/document branches (rather than a v8 ignore) is exactly what the repo guidelines ask for.

💡 Observations (non-blocking)

  1. Intentional currentTheme() duplication. The module's currentTheme() is byte-for-byte identical to the one still living in createAppBridgeFactory.ts:62. The PR body explicitly defers folding that in to Wave 2, so this is by design — flagging only so it's on the radar that the two copies can drift until then. A near-zero-risk option would be to have createAppBridgeFactory.ts import currentTheme from ./hostContext now (delete its local copy), collapsing the duplication a wave early without touching any of the Wave 2 rewrites. Purely optional. Fix this →

  2. {} as McpUiStyles cast in currentStyles() (line 94). Minor type-safety concession — the object is built up field by field afterward, so this is a pragmatic choice rather than a Record<..., string> that would allow any key. Fine as is; just noting it's the one spot where the strong typing is relaxed.

  3. measureContainerDimensions return type. Returning the concrete { width, height } pair instead of the spec's ContainerDimensions union is well-documented (callers need to compare both fields for the future size-changed diff). No action — the doc comment already explains the intent.

Verdict

Looks good to merge as a Wave 1 base. No changes required; the duplication note (#1) is the only thing I'd consider addressing early, and it's optional.

This was referenced Jul 2, 2026
Open
Open
@cliffhall

cliffhall commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

📋 Smoke test & AGENTS.md audit for this PR: #1559 (comment) (posted on the linked issue #1559).

@cliffhall cliffhall mentioned this pull request Jul 2, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

v2 Issues and PRs for v2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

web: extract hostContext utilities for the Apps host (AppRenderer/hostContext.ts)

1 participant

@cliffhall