feat: store and expose hostContext in App class #139
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commit: |
Fixes #129 - The App class now stores the initial hostContext received during initialization and exposes it via getHostContext(). Changes: - Add _hostContext private property to App class - Store result.hostContext in connect() method - Add getHostContext() getter method (parallel to getHostCapabilities()) - Update onhostcontextchanged setter to merge partial updates into stored context - Add default notification handler to update context even when user hasn't set onhostcontextchanged This enables apps to: - Access toolInfo immediately after connection (only in initial context) - Render with correct theme/locale without waiting for first notification - Synchronously access viewport, timezone, and other initial state 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
ochafik
force-pushed
the
ochafik/fix-issue-129-host-context
branch
from
36aa8a8 to
1a95181
Compare
jonathanhefner
previously approved these changes
Dec 11, 2025
src/app-bridge.test.ts
Outdated
Comment on lines
319
to
341
| it("getHostContext accumulates multiple partial updates", async () => { | ||
| // Need fresh transports for new bridge | ||
| const [newAppTransport, newBridgeTransport] = | ||
| InMemoryTransport.createLinkedPair(); | ||
|
|
||
| const initialContext = { | ||
| theme: "light" as const, | ||
| locale: "en-US", | ||
| viewport: { width: 800, height: 600 }, | ||
| }; | ||
| const newBridge = new AppBridge( | ||
| createMockClient() as Client, | ||
| testHostInfo, | ||
| testHostCapabilities, | ||
| { hostContext: initialContext }, | ||
| ); | ||
| const newApp = new App(testAppInfo, {}, { autoResize: false }); | ||
|
|
||
| await newBridge.connect(newBridgeTransport); | ||
| await newApp.connect(newAppTransport); | ||
|
|
||
| // Update only theme | ||
| newBridge.setHostContext({ ...initialContext, theme: "dark" }); |
Member
There was a problem hiding this comment.
I might be misunderstanding, but is this actually testing getHostContext accumulates multiple partial updates? Or should we omit ...initialContext here?
Collaborator
Author
There was a problem hiding this comment.
oops good catch, thanks, fixed
src/app-bridge.test.ts
Outdated
Comment on lines
340
to
350
| // Update only theme | ||
| newBridge.setHostContext({ ...initialContext, theme: "dark" }); | ||
| await flush(); | ||
|
|
||
| // Update viewport | ||
| newBridge.setHostContext({ | ||
| theme: "dark", | ||
| locale: "en-US", | ||
| viewport: { width: 1024, height: 768 }, | ||
| }); | ||
| await flush(); |
Member
There was a problem hiding this comment.
Why two newBridge.setHostContext() calls?
Collaborator
Author
There was a problem hiding this comment.
Fixed to address intent:
// Send partial update: only theme changes
newBridge.sendHostContextChange({ theme: "dark" });
await flush();
// Send another partial update: only viewport changes
newBridge.sendHostContextChange({ viewport: { width: 1024, height: 768 } });
await flush();
// getHostContext should have accumulated all updates:
// - locale from initial (unchanged)
// - theme from first partial update
// - viewport from second partial update
const context = newApp.getHostContext();
expect(context?.theme).toBe("dark");
expect(context?.locale).toBe("en-US");
expect(context?.viewport).toEqual({ width: 1024, height: 768 });Address PR review feedback: The test was using setHostContext with full context spreads which didn't clearly demonstrate partial update accumulation. Now uses sendHostContextChange directly to send truly partial updates (only theme, then only viewport), making it clear the App correctly merges multiple partial notifications. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
jonathanhefner
approved these changes
Dec 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #129 - The
Appclass now stores the initialhostContextreceived during initialization and exposes it viagetHostContext()._hostContextprivate property toAppclassresult.hostContextinconnect()methodgetHostContext()getter method (parallel togetHostCapabilities())onhostcontextchangedsetter to merge partial updates into stored contextonhostcontextchangedThis enables apps to:
toolInfoimmediately after connection (only available in initial context)Test plan
App receives initial hostContext after connectgetHostContext returns undefined before connectgetHostContext merges updates from onhostcontextchangedgetHostContext updates even without user setting onhostcontextchangedgetHostContext accumulates multiple partial updates🤖 Generated with Claude Code