feat(api): add agent location and API integration [EXT-6966] #2362
Conversation
WalkthroughAdds an Agent runtime and types: introduces Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client/App
participant AgentFactory as createAgent
participant Channel
participant Signal as MemoizedSignal
participant AgentAPI as AgentAPI (handler store)
App->>AgentFactory: createAgent(channel, contextData)
AgentFactory->>Signal: init(contextData)
AgentFactory->>Channel: subscribe(contextChanged, toolbarAction)
AgentFactory-->>App: return AgentAPI { onContextChange, onToolbarAction }
App->>AgentAPI: onContextChange(handler)
AgentAPI->>AgentAPI: register handler
AgentAPI-->>App: return unsubscribe
Channel->>AgentFactory: contextChanged(newContext)
AgentFactory->>Signal: update(newContext)
AgentFactory->>AgentAPI: dispatch(updatedContext)
AgentAPI->>App: invoke registered handlers
Channel->>AgentFactory: toolbarAction(action)
AgentFactory->>AgentAPI: dispatch(action)
AgentAPI->>App: invoke toolbar handlers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/agent.ts (1)
13-19: Optional: Local context variable is updated but not read.The
contextvariable on line 13 is initialized and updated (line 18) but never read after being passed toMemoizedSignalon line 15. TheMemoizedSignalmaintains the actual state, so this local variable is redundant.Consider simplifying by removing the local variable:
- let context: AgentContext = contextData - - const contextChanged = new MemoizedSignal<[AgentContext]>(context) + const contextChanged = new MemoizedSignal<[AgentContext]>(contextData) channel.addHandler('contextChanged', (newContext: AgentContext) => { - context = newContext contextChanged.dispatch(newContext) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
lib/agent.ts(1 hunks)lib/api.ts(3 hunks)lib/locations.ts(1 hunks)lib/types/agent.types.ts(1 hunks)lib/types/api.types.ts(5 hunks)lib/types/index.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T16:31:58.950Z
Learnt from: tylerwashington888
Repo: contentful/ui-extensions-sdk PR: 2338
File: lib/asset.ts:49-51
Timestamp: 2025-09-24T16:31:58.950Z
Learning: Both asset API and entry API implementations use VoidFunction for onMetadataChanged handler parameter, maintaining consistency across the codebase despite type definitions specifying (metadata?: Metadata) => void.
Applied to files:
lib/types/agent.types.ts
🧬 Code graph analysis (4)
lib/agent.ts (4)
lib/channel.ts (1)
Channel(26-86)lib/types/api.types.ts (1)
ConnectMessage(408-436)lib/types/agent.types.ts (2)
AgentAPI(10-12)AgentContext(1-8)lib/signal.ts (1)
MemoizedSignal(24-57)
lib/api.ts (3)
lib/channel.ts (1)
Channel(26-86)lib/types/api.types.ts (1)
ConnectMessage(408-436)lib/agent.ts (1)
createAgent(5-25)
lib/types/api.types.ts (1)
lib/types/agent.types.ts (2)
AgentAPI(10-12)AgentContext(1-8)
lib/types/agent.types.ts (1)
lib/types/index.ts (2)
AgentContext(40-40)AgentAPI(40-40)
🔇 Additional comments (12)
lib/types/index.ts (1)
37-41: LGTM! Clean type exports for agent integration.The new exports for
AgentAppSDK,AgentAPI, andAgentContextare correctly structured and follow the existing export patterns in this file.lib/locations.ts (1)
13-13: LGTM! Location constant added correctly.The
LOCATION_AGENTconstant follows the established pattern and correctly integrates with theLocationstype definition inlib/types/api.types.ts.lib/agent.ts (1)
5-24: Solid implementation! Verify test coverage.The
createAgentfunction correctly:
- Validates required context data
- Uses
MemoizedSignalto ensure handlers are immediately invoked with current context- Subscribes to context change events via the channel
- Returns a properly typed
AgentAPIThe implementation follows existing SDK patterns and integrates cleanly with the channel-based event system.
The PR checklist shows tests are not marked as complete. Please ensure this new functionality is covered by tests, particularly:
- Context validation (line 9-11)
- Handler invocation on registration
- Context updates via channel events
- Handler cleanup (unsubscribe function)
lib/api.ts (3)
11-11: LGTM! Clean import integration.The
createAgentimport is correctly added alongside other API factory functions.
49-49: LGTM! Proper API producer mapping.The
LOCATION_AGENTis correctly mapped to include bothmakeSharedAPI(for common SDK functionality) andmakeAgentAPI(for agent-specific features), following the established pattern for other locations.
172-176: LGTM! API producer follows established patterns.The
makeAgentAPIfunction correctly:
- Follows the same signature as other API producers
- Destructures the
agentfield fromConnectMessage- Delegates to
createAgentfor initialization- Returns an object exposing the
agentAPIThe validation of the
agentfield is appropriately handled withincreateAgent.lib/types/agent.types.ts (1)
1-12: LGTM! Clean and consistent type definitions.The types are well-structured:
AgentContextclearly defines the agent context shapeAgentAPIfollows the SDK's established pattern of returningVoidFunctionfor cleanup, consistent with other APIs likeonMetadataChangedin asset/entry APIsBased on learnings
Minor: Verify the
metadatafield requirement.The
metadatafield onAgentContext(line 3) is required (not optional), though all its properties are optional. This means everyAgentContextmust have ametadataobject, even if all fields are undefined.If agent contexts might legitimately not have any metadata, consider making the field optional:
export interface AgentContext { view: string - metadata: { + metadata?: { entryId?: string contentTypeId?: string lastFocusedFieldId?: string } }If the current design is intentional (always require the object for simpler property access), this is fine as-is.
lib/types/api.types.ts (5)
25-25: LGTM! Required import for agent types.The import correctly brings in the new agent-related types from the dedicated types file.
346-352: LGTM! AgentAppSDK type is well-defined.The
AgentAppSDKtype correctly:
- Extends
BaseAppSDKwith support forInstallationParameters- Sets
InstanceParametersandInvocationParameterstonever(appropriate for agent location, consistent withHomeAppSDKandConfigAppSDK)- Adds the
agent: AgentAPIproperty for agent-specific functionality
366-366: LGTM! Union type properly extended.
AgentAppSDKis correctly added to theKnownAppSDKunion, maintaining type safety for all SDK locations.
405-405: LGTM! Location constant matches runtime definition.The
LOCATION_AGENTentry in theLocationsinterface correctly matches the runtime constant added inlib/locations.ts.
435-435: LGTM! Optional agent context in ConnectMessage.Making the
agentfield optional is correct—it will only be present when the SDK is initialized at theLOCATION_AGENTlocation. The validation increateAgentensures it's defined when needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/types/agent.types.ts (1)
12-15: Consider whether theactionfield is necessary.The
actionfield is currently hardcoded to the literal'click'. If all toolbar actions are clicks, this field adds no information and could be removed. Alternatively, if you anticipate other action types in the future (e.g.,'hover','focus'), consider making it a union type now.Current structure:
export interface ToolbarAction { name: ToolbarActionName action: 'click' }Option 1 - Remove redundant field:
export interface ToolbarAction { name: ToolbarActionName - action: 'click' }Option 2 - Make it extensible:
+export type ActionType = 'click' | 'hover' | 'focus' + export interface ToolbarAction { name: ToolbarActionName - action: 'click' + action: ActionType }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/agent.ts(1 hunks)lib/types/agent.types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
lib/agent.ts (4)
lib/channel.ts (1)
Channel(26-86)lib/types/api.types.ts (1)
ConnectMessage(408-436)lib/types/agent.types.ts (3)
AgentAPI(17-20)AgentContext(1-8)ToolbarAction(12-15)lib/signal.ts (2)
MemoizedSignal(24-57)Signal(3-22)
lib/types/agent.types.ts (1)
lib/types/index.ts (2)
AgentContext(40-40)AgentAPI(40-40)
🔇 Additional comments (8)
lib/types/agent.types.ts (2)
1-8: LGTM! Clean type definition for agent context.The AgentContext structure is well-designed with appropriate optional fields for flexibility.
17-20: LGTM! Well-designed event handler API.The API follows the standard pattern of returning an unsubscribe function, enabling proper cleanup.
lib/agent.ts (6)
1-4: LGTM! Imports are correct and minimal.All imported types and classes are used appropriately in the implementation.
10-12: Approve the null check; consider deeper validation.The null check is essential and correctly prevents invalid initialization. However, consider whether you should also validate that
contextDatacontains the required fields (view,metadata) to catch malformed data early.If runtime validation is desired:
if (!contextData) { throw new Error('Context data is required') } + if (!contextData.view || !contextData.metadata) { + throw new Error('Context data must contain view and metadata') + }
14-16: LGTM! Appropriate signal types for state vs. events.Using
MemoizedSignalfor context ensures new handlers immediately receive the current state, while usingSignalfor toolbar actions correctly treats them as ephemeral events.
18-24: LGTM! Channel handlers correctly wire events to signals.The implementation properly bridges channel events to the signal dispatchers. The handlers are not explicitly unregistered, which is appropriate if
createAgentis called only once per agent instance lifecycle.
26-29: LGTM! Clean API implementation with proper cleanup.The returned object correctly implements the
AgentAPIinterface, and both methods return unsubscribe functions for proper listener cleanup.
18-24: Note: Past review comment about chat input handling.A previous reviewer (tylerwashington888) mentioned that the agent should also handle consumption of editor/chat input (user typing messages). This functionality doesn't appear to be implemented in the current PR. Please confirm whether chat input handling is:
- Out of scope for this PR and tracked separately
- Already handled elsewhere in the codebase
- Still needs to be added
Based on learnings
- Introduced LOCATION_AGENT in locations for agent-specific functionality. - Added createAgent import and integrated makeAgentAPI for handling agent data in the API. - Updated types to include AgentAppSDK and related agent context types.
- Updated context handling in createAgent to directly dispatch new context without intermediate variable. - Added new connectMessageWithAgent mock for testing agent-specific functionality. - Expanded unit tests to verify agent API integration, including context handling and error scenarios for missing agent data.
- Introduced a new Signal for toolbar actions in createAgent. - Updated the AgentAPI to include onToolbarAction for handling toolbar events. - Defined ToolbarAction type to standardize action structure.
- Updated agent tests to verify the presence of onToolbarAction method. - Expanded context change handling tests to include toolbarAction event registration. - Added new tests for toolbar action handling, including multiple handlers and action type verification.
lewisjcs
force-pushed
the
feat/agent-location-EXT-6966
branch
from
f146777 to
953cfcd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/agent.ts(1 hunks)lib/types/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/agent.ts (4)
lib/channel.ts (1)
Channel(26-86)lib/types/api.types.ts (1)
ConnectMessage(408-436)lib/types/agent.types.ts (3)
AgentAPI(17-20)AgentContext(1-8)ToolbarAction(12-15)lib/signal.ts (2)
MemoizedSignal(24-57)Signal(3-22)
🔇 Additional comments (6)
lib/types/index.ts (1)
37-40: LGTM! Clean integration of agent types into public API.The new exports properly expose the agent-related types to SDK consumers, following the established pattern.
lib/agent.ts (5)
1-3: LGTM! Imports are clean and appropriate.All required dependencies are properly imported.
5-11: LGTM! Good defensive validation.The function correctly validates that
contextDatais provided before proceeding, preventing runtime errors downstream.
13-15: LGTM! Appropriate signal type choices.Using
MemoizedSignalfor context ensures new subscribers immediately receive the current state, while regularSignalfor toolbar actions correctly treats them as event-driven with no initial state.
17-23: Verify: Is chat input handling required?A previous review comment (line 20) mentions: "i think we need to also handle the consumption of editor input right? The user types 'analyze my entries into the chat window and presses submit' out widget render pipeline needs to be able to consume this chat and pass it to the custom app"
The current implementation only handles
contextChangedandtoolbarActionevents. Should there be a handler for user chat input, or is that functionality planned for a separate PR?
25-28: LGTM! Clean API surface following observer pattern.The returned API correctly provides subscription methods that return detach functions, allowing consumers to clean up their own handlers.
# [4.46.0](v4.45.0...v4.46.0) (2025-11-11) ### Bug Fixes * bump CircleCI Node version to 24.11.0 [] ([#2365](#2365)) ([e054b06](e054b06)) * **deps:** downgrade semantic-release and related dependencies ([#2366](#2366)) ([d27fde3](d27fde3)) ### Features * **api:** add agent location and API integration [EXT-6966] ([#2362](#2362)) ([2febd54](2febd54))
|
🎉 This PR is included in version 4.46.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Purpose of PR
PR Checklist
Summary by CodeRabbit
New Features
Tests