Add user-serving agent support with OAuth configuration to agent creation wizard#9888
Open
ranuka-laksika wants to merge 1 commit intowso2:masterfrom
Open
Add user-serving agent support with OAuth configuration to agent creation wizard#9888ranuka-laksika wants to merge 1 commit intowso2:masterfrom
ranuka-laksika wants to merge 1 commit intowso2:masterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request introduces support for Agent applications—a new application template type within the system. Changes include a multi-step agent creation wizard with conditional form fields based on agent type, API endpoints for updating agent OAuth/OIDC configuration, new data models and enums for agent type classification, application template configuration, and comprehensive internationalization support. Changes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
features/admin.extensions.v1/configs/application.tsx (1)
441-445: Consider using enum constant instead of string literal.Other template checks in this file use
ApplicationTemplateIdTypes.AGENT_APPLICATION(lines 234, 281, 387, 404), but this mapping uses the string literal"agent-application". For consistency and type safety, consider using the enum.♻️ Suggested change
- [ "agent-application" ]: [ + [ ApplicationTemplateIdTypes.AGENT_APPLICATION ]: [ ApplicationManagementConstants.AUTHORIZATION_CODE_GRANT, ApplicationManagementConstants.REFRESH_TOKEN_GRANT, ApplicationManagementConstants.CIBA_GRANT ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.extensions.v1/configs/application.tsx` around lines 441 - 445, Replace the string literal "agent-application" in the grant mapping with the enum constant ApplicationTemplateIdTypes.AGENT_APPLICATION to match other usages and improve type safety; locate the mapping that defines the grant array (the entry currently keyed by "agent-application") and swap the key to ApplicationTemplateIdTypes.AGENT_APPLICATION so it aligns with the other entries that reference ApplicationTemplateIdTypes (e.g., where APPLICATION_TEMPLATE constants are used on lines near uses of ApplicationTemplateIdTypes.AGENT_APPLICATION).features/admin.agents.v1/api/agents.ts (2)
240-242: Potential issue:deletemay not work if property is read-only or non-configurable.Using
deleteoncibaAuthenticationRequestshould work here sinceupdatedOidcConfigis a shallow copy. However, consider setting it toundefinedfor clearer intent, which also works with JSON serialization (the property will be omitted).♻️ Alternative approach
if (updatedOidcConfig.cibaAuthenticationRequest) { - delete updatedOidcConfig.cibaAuthenticationRequest; + updatedOidcConfig.cibaAuthenticationRequest = undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.agents.v1/api/agents.ts` around lines 240 - 242, Replace the delete operation on updatedOidcConfig.cibaAuthenticationRequest with explicitly setting the property to undefined to avoid issues with non-configurable/read-only properties and to ensure JSON serialization omits it; locate the code that checks if (updatedOidcConfig.cibaAuthenticationRequest) and change the removal step to updatedOidcConfig.cibaAuthenticationRequest = undefined so intent is clearer and safe across environments.
269-278: Simplify async/await usage for consistency.The function uses
async/awaitthroughout but switches to.then()/.catch()for the final PUT request. This can be simplified for consistency.♻️ Suggested refactor
- return httpClient(putRequestConfig) - .then((response: AxiosResponse) => { - return Promise.resolve(response); - }) - .catch((error: AxiosError) => { - return Promise.reject(error); - }); - } catch (error) { - return Promise.reject(error); - } + return await httpClient(putRequestConfig); + } catch (error: unknown) { + throw error; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.agents.v1/api/agents.ts` around lines 269 - 278, The final PUT call is using .then()/.catch() inside an async function; replace it with an awaited call to httpClient(putRequestConfig) and directly return the AxiosResponse to match the rest of the async/await style and let the surrounding try/catch handle errors; locate the httpClient call that uses putRequestConfig (and references AxiosResponse/AxiosError) and remove the Promise.resolve/Promise.reject wrappers so the function simply awaits the client and returns the result.features/admin.agents.v1/components/wizards/add-agent-wizard.tsx (2)
191-196: Race condition:creationResultmay be stale whenhandleCloseis called.
handleCloseresetscreationResulttonullon line 193 before callingonClose(creationResult)on line 195. Due to React's asynchronous state updates,creationResultin the closure still holds the previous value, so this works. However, the code order is confusing. Consider passing the result before resetting state.♻️ Clearer ordering
const handleClose = (): void => { + const result: AgentCreationResultInterface | null = creationResult; setIsShowingSuccessScreen(false); setCreationResult(null); setSubmittedValues(null); - onClose(creationResult); + onClose(result); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.agents.v1/components/wizards/add-agent-wizard.tsx` around lines 191 - 196, The handleClose function can pass a stale or confusing creationResult because state resets happen after calling onClose; capture the current creationResult into a local variable (e.g., const result = creationResult) and call onClose(result) before calling setIsShowingSuccessScreen(false), setCreationResult(null), and setSubmittedValues(null); alternatively, call onClose with creationResult first and then reset state—update the handleClose implementation (references: handleClose, creationResult, onClose, setIsShowingSuccessScreen, setCreationResult, setSubmittedValues) to ensure the result passed to onClose is the intended current value.
525-534: Consider using form ref instead of DOM query for submit.Using
document.getElementByIdand manual event dispatch is fragile (relies on string ID matching). Consider using a form ref or react-final-form's form API for more robust form submission.♻️ Suggested refactor using form ref
+import React, { FunctionComponent, ReactElement, useRef, useState } from "react"; + + const formRef: React.RefObject<HTMLFormElement> = useRef<HTMLFormElement>(null); + // In the form element: - <form id="addAgentForm" onSubmit={ handleSubmit }> + <form ref={ formRef } onSubmit={ handleSubmit }> // In the button onClick: - onClick={ () => { - document - .getElementById("addAgentForm") - .dispatchEvent( - new Event("submit", { - bubbles: true, - cancelable: true - }) - ); - } } + onClick={ () => formRef.current?.requestSubmit() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.agents.v1/components/wizards/add-agent-wizard.tsx` around lines 525 - 534, The current onClick handler uses document.getElementById("addAgentForm") and dispatchEvent to submit the form which is fragile; update the button's onClick to use a React ref (e.g., create a formRef and attach it to the <form id="addAgentForm">) or, if using react-final-form, call the form API (e.g., form.submit()) directly instead of querying the DOM; replace the manual Event dispatch in the onClick closure with formRef.current.requestSubmit() or form.submit() to make submission type-safe and refactor any references to the string ID ("addAgentForm") accordingly.features/admin.applications.v1/data/application-templates/templates/agent-application/agent-application.json (1)
48-52: Consider removing unusedhybridFlowResponseTypeconfiguration.The template sets
hybridFlowResponseType: "code_idtoken"but doesn't include theimplicitgrant type required for hybrid flow. Since the agent flow uses onlyauthorization_code,refresh_token, orciba(configured post-creation viaupdateAgentApplicationConfiguration), this setting appears unused and may cause confusion.🧹 Suggested cleanup
"refreshToken": { "expiryInSeconds": 86400, "renewRefreshToken": true - }, - "hybridFlowResponseType": "code_idtoken" + }Also applies to: 62-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.applications.v1/data/application-templates/templates/agent-application/agent-application.json` around lines 48 - 52, The template includes a hybridFlowResponseType ("code_idtoken") that is unused because grantTypes in agent-application.json only list "authorization_code", "refresh_token", and "urn:openid:params:grant-type:ciba"; remove the hybridFlowResponseType property from the agent-application.json template (and any duplicate instance at line 62) to avoid confusion, or if hybrid flow is intended, add "implicit" to the grantTypes and ensure updateAgentApplicationConfiguration aligns with that choice; locate the key hybridFlowResponseType and either delete it or update grantTypes accordingly (see updateAgentApplicationConfiguration and the grantTypes array for context).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@features/admin.agents.v1/components/wizards/add-agent-wizard.tsx`:
- Around line 140-155: The catch block conflates failures from
updateAgentApplicationConfiguration and getInboundProtocolConfig and always
dispatches the configUpdateFailed alert; split the operations or add a separate
try/catch around getInboundProtocolConfig so failures fetching the OAuth client
ID dispatch the clientIdFetchFailed alert (while
updateAgentApplicationConfiguration failures still dispatch configUpdateFailed).
Specifically, keep the existing try for updateAgentApplicationConfiguration and,
after that succeeds, call getInboundProtocolConfig inside its own try/catch that
sets result.oauthClientId on success and dispatches addAlert with the
clientIdFetchFailed message on failure, referencing result.oauthClientId,
getInboundProtocolConfig, updateAgentApplicationConfiguration, addAlert,
clientIdFetchFailed and configUpdateFailed.
---
Nitpick comments:
In `@features/admin.agents.v1/api/agents.ts`:
- Around line 240-242: Replace the delete operation on
updatedOidcConfig.cibaAuthenticationRequest with explicitly setting the property
to undefined to avoid issues with non-configurable/read-only properties and to
ensure JSON serialization omits it; locate the code that checks if
(updatedOidcConfig.cibaAuthenticationRequest) and change the removal step to
updatedOidcConfig.cibaAuthenticationRequest = undefined so intent is clearer and
safe across environments.
- Around line 269-278: The final PUT call is using .then()/.catch() inside an
async function; replace it with an awaited call to httpClient(putRequestConfig)
and directly return the AxiosResponse to match the rest of the async/await style
and let the surrounding try/catch handle errors; locate the httpClient call that
uses putRequestConfig (and references AxiosResponse/AxiosError) and remove the
Promise.resolve/Promise.reject wrappers so the function simply awaits the client
and returns the result.
In `@features/admin.agents.v1/components/wizards/add-agent-wizard.tsx`:
- Around line 191-196: The handleClose function can pass a stale or confusing
creationResult because state resets happen after calling onClose; capture the
current creationResult into a local variable (e.g., const result =
creationResult) and call onClose(result) before calling
setIsShowingSuccessScreen(false), setCreationResult(null), and
setSubmittedValues(null); alternatively, call onClose with creationResult first
and then reset state—update the handleClose implementation (references:
handleClose, creationResult, onClose, setIsShowingSuccessScreen,
setCreationResult, setSubmittedValues) to ensure the result passed to onClose is
the intended current value.
- Around line 525-534: The current onClick handler uses
document.getElementById("addAgentForm") and dispatchEvent to submit the form
which is fragile; update the button's onClick to use a React ref (e.g., create a
formRef and attach it to the <form id="addAgentForm">) or, if using
react-final-form, call the form API (e.g., form.submit()) directly instead of
querying the DOM; replace the manual Event dispatch in the onClick closure with
formRef.current.requestSubmit() or form.submit() to make submission type-safe
and refactor any references to the string ID ("addAgentForm") accordingly.
In
`@features/admin.applications.v1/data/application-templates/templates/agent-application/agent-application.json`:
- Around line 48-52: The template includes a hybridFlowResponseType
("code_idtoken") that is unused because grantTypes in agent-application.json
only list "authorization_code", "refresh_token", and
"urn:openid:params:grant-type:ciba"; remove the hybridFlowResponseType property
from the agent-application.json template (and any duplicate instance at line 62)
to avoid confusion, or if hybrid flow is intended, add "implicit" to the
grantTypes and ensure updateAgentApplicationConfiguration aligns with that
choice; locate the key hybridFlowResponseType and either delete it or update
grantTypes accordingly (see updateAgentApplicationConfiguration and the
grantTypes array for context).
In `@features/admin.extensions.v1/configs/application.tsx`:
- Around line 441-445: Replace the string literal "agent-application" in the
grant mapping with the enum constant
ApplicationTemplateIdTypes.AGENT_APPLICATION to match other usages and improve
type safety; locate the mapping that defines the grant array (the entry
currently keyed by "agent-application") and swap the key to
ApplicationTemplateIdTypes.AGENT_APPLICATION so it aligns with the other entries
that reference ApplicationTemplateIdTypes (e.g., where APPLICATION_TEMPLATE
constants are used on lines near uses of
ApplicationTemplateIdTypes.AGENT_APPLICATION).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ec0674d6-7bfd-42db-9e71-7e72fbbfed17
📒 Files selected for processing (14)
features/admin.agents.v1/api/agents.tsfeatures/admin.agents.v1/components/wizards/add-agent-wizard.scssfeatures/admin.agents.v1/components/wizards/add-agent-wizard.tsxfeatures/admin.agents.v1/models/agents.tsfeatures/admin.agents.v1/pages/agents.tsxfeatures/admin.applications.v1/components/forms/inbound-oidc-form.tsxfeatures/admin.applications.v1/data/application-templates/application-templates-config.tsfeatures/admin.applications.v1/data/application-templates/templates/agent-application/agent-application.jsonfeatures/admin.applications.v1/models/application.tsfeatures/admin.extensions.v1/application-templates/templates/agent-application/agent-application.jsonfeatures/admin.extensions.v1/configs/application.tsxfeatures/admin.extensions.v1/configs/models/application.tsmodules/i18n/src/models/namespaces/agents-ns.tsmodules/i18n/src/translations/en-US/portals/agents.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9888 +/- ##
=======================================
Coverage 56.01% 56.01%
=======================================
Files 42 42
Lines 1023 1023
Branches 231 231
=======================================
Hits 573 573
Misses 450 450
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Enhanced the agent creation wizard to support user-serving agents with
configurable OAuth/OIDC authentication flows. The wizard now handles both
Interactive (authorization code) and Background (CIBA) agent types with
appropriate OAuth configuration.
Changes
agents
OAuth settings post-creation
When creating a user-serving agent, the wizard now:
- Interactive: Sets authorization_code and refresh_token grants, configures
callback URL
- Background: Sets urn:openid:params:grant-type:ciba grant, configures CIBA
settings
entire operation)
Here is the Agent Creation Wizard :-
For Interactive Agents (User should be interact with agent always within that time period like chatbot, AI Assistant):-
For Background Agents(Agent run in Background hence this no need the user interaction always but at some point)
Final Result for User Serving Agents:-
Final Result Wizard for Non User Serving Agents:-