-
Notifications
You must be signed in to change notification settings - Fork 353
[FIX] Graceful UI handling when listing invalid Agent(s) #803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] Graceful UI handling when listing invalid Agent(s) #803
Conversation
de86576 to
bead3ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds graceful handling for invalid agents in the UI by introducing an accepted field to track agent reconciliation status. Previously, if any agent failed to reconcile (e.g., due to invalid modelConfig), no agents would be displayed at all.
- Adds
acceptedfield to track agent reconciliation status based on Kubernetes conditions - Modifies UI to display "Not Accepted" status for failed agents while keeping them visible
- Updates CLI output to include both deployment readiness and acceptance status
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ui/src/types/index.ts | Adds accepted boolean field to AgentResponse interface |
| ui/src/components/AgentCard.tsx | Updates UI to display "Not Accepted" status and disable interactions for non-accepted agents |
| go/pkg/client/api/types.go | Adds Accepted field to backend AgentResponse struct |
| go/internal/httpserver/handlers/agents.go | Implements logic to check agent reconciliation status and continues listing even with failures |
| go/internal/httpserver/handlers/agents_test.go | Adds comprehensive tests for agent status conditions |
| go/cli/internal/cli/get.go | Updates CLI output to show both deployment readiness and acceptance status |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
f6f198a to
5b65a66
Compare
… an empty angets list when an agent(s) fails to load Signed-off-by: Fabian Gonzalez <fabian.gonzalez@solo.io>
Signed-off-by: Fabian Gonzalez <fabian.gonzalez@solo.io>
…epted reconciliation) Signed-off-by: Fabian Gonzalez <fabian.gonzalez@solo.io>
Signed-off-by: Fabian Gonzalez <fabian.gonzalez@solo.io>
50e4d57 to
a492b3a
Compare
) Originally, when listing the agents, if a single agent failed (i.e. `modelConfig` is invalid), no Agents were shown. # Changes With these changes, we: 1. Add a new AgentResponse field `Accepted` (i.e. `Reconciled`, but front-end friendly name) - This is based on the `Accepted` state of the Agent. 2. **Don't** return an error response _to the frontend_ when an Agent (or multiple) had failures. - We now continue listing agents, making the UI usable for valid agents, and adding a `Not Accepted` (could also be `Not Reconciled`, or `Error` for simplest terms) status on broken agents -- similar to how we do `Not Ready` while they are being deployed. 3. No longer display unready/un-accepted Agents in the agent sidebar from chats. ## Alternative Approaches Another approach would be to avoid returning the not-reconciled agent altogether. This would avoid needing to set up the new response field and UI changes. I was originally leaning towards that one, but figured it may be useful/quicker for users to notice any broken agents if they appear in the UI with their failed status. Another idea I had, but would probably be overkill and could constitute as a new feature, is to have a "notification system", where we would _not_ display failed agents in the agents list, but instead have the reconciliation as a notification. # Testing ## Dashboard Build & run ```sh make create-kind-cluster helm-install build-cli-local KAGENT_HELM_REPO=./helm/ ./go/bin/kagent-local dashboard ``` Update an Agent to have an invalid `modelConfig` ```sh kubectl patch agents.kagent.dev/k8s-agent -n kagent --type='json' -p='[{"op": "replace", "path": "/spec/declarative/modelConfig", "value": "default-model-config-2"}]' ``` Reload page. View listing UI to see the failed state (Not Accepted). Fix the agent. ```sh kubectl patch agents.kagent.dev/k8s-agent -n kagent --type='json' -p='[{"op": "replace", "path": "/spec/declarative/modelConfig", "value": "default-model-config"}]' ``` Reload page. It’s all good again! ## CLI This would ideally also get tested through the CLI to test the behavior of `KAGENT_HELM_REPO=./helm/ ./go/bin/kagent-local get agent`, _but_ I’m hitting “userID is required” issues. My `$HOME/.kagent/config.yaml` file has it defined, so I'm not sure of the exact issue yet. --- Resolves: kagent-dev#797 --------- Signed-off-by: Fabian Gonzalez <fabian.gonzalez@solo.io>
Originally, when listing the agents, if a single agent failed (i.e.
modelConfigis invalid), no Agents were shown.Changes
With these changes, we:
Accepted(i.e.Reconciled, but front-end friendly name)Acceptedstate of the Agent.Not Accepted(could also beNot Reconciled, orErrorfor simplest terms) status on broken agents -- similar to how we doNot Readywhile they are being deployed.Alternative Approaches
Another approach would be to avoid returning the not-reconciled agent altogether. This would avoid needing to set up the new response field and UI changes. I was originally leaning towards that one, but figured it may be useful/quicker for users to notice any broken agents if they appear in the UI with their failed status.
Another idea I had, but would probably be overkill and could constitute as a new feature, is to have a "notification system", where we would not display failed agents in the agents list, but instead have the reconciliation as a notification.
Testing
Dashboard
Build & run
Update an Agent to have an invalid
modelConfigReload page.
View listing UI to see the failed state (Not Accepted).
Fix the agent.
Reload page.
It’s all good again!
CLI
This would ideally also get tested through the CLI to test the behavior of
KAGENT_HELM_REPO=./helm/ ./go/bin/kagent-local get agent, but I’m hitting “userID is required” issues. My$HOME/.kagent/config.yamlfile has it defined, so I'm not sure of the exact issue yet.Resolves: #797