feat(ui): rewrite the web interface on Vite, and give conversations a first-class API - #2569
feat(ui): rewrite the web interface on Vite, and give conversations a first-class API#2569Charlesthebird wants to merge 1 commit into
Conversation
9d67c52 to
6426683
Compare
github-actions
Bot
added
enhancement
06e71ff to
11aaee0
Compare
kagent-ui-crud.mp4🤖 written by Claude |
kagent-ui-agent-chat.mp4🤖 written by Claude |
3228eee to
8264ec6
Compare
… first-class API The Next.js app is replaced by a Vite + React 19 single-page app. Routing is React Router's, reads go through SWR, components come from antd 6 and styling from Emotion. Settings reach the app at runtime from `window.environmentVariables` rather than being frozen into a build, so one image serves every deployment. The pages follow the v1alpha3 model rather than the one the old app was written for. An agent is not a resource: it is what exists once a Harness admits an AgentTemplate, read out of `AgentTemplate.status.harnesses[]`. The agents landing page says so, in an overview that maps the four concepts — AgentTemplate, Harness, Agent, AgentInstance — onto the Agent Substrate words for the same things, and its three tabs are the way to each. An agent's own page lists its conversations, and a conversation is the chat. Chat runs over A2A on gRPC-Web, which needed a server-side half: - `AgentInstance` gains a name, end to end — migration, sqlc, store, proto, service, gRPC handler and policy — so a conversation can be called something. Additive: an empty name renders by id. - `ListAgentInstances` takes a query rather than six positional arguments, and can narrow to one agent by template and harness. The pair is resolved through `prepared_revision`, so no new column is needed and rows written before the filter existed still match. - The A2A gateway serves an instance in any state for reads, so a suspended conversation can still be opened and its transcript read. - A share resolves to the share *and* the instance's owner, because that is what a share grants: the reader stays themselves, and the token widens what they may read. - The gRPC server can hand its gRPC-Web handler to the HTTP server, so a browser reaches the services over the origin it was served from. The split happens outside the router's middleware chain: that chain is for REST-shaped handlers, and a gRPC-Web frame neither survives it nor needs it. Test coverage is 376 unit tests and 88 browser tests, all against the real pages. What could not be covered honestly is written down in `ui/playwright/DEFERRED.md` with the surface each spec is waiting on, rather than committed as a skipped test — a skipped spec reads as coverage and that list does not. `ui/dev-scripts/` builds a Kind cluster with kagent on it in one command, for anyone who wants to try the app against a real backend. Without a cluster, `ENABLE_MOCK_UI=true` serves every page from in-browser fixtures, and each such page says on itself that the data is not real. Signed-off-by: Nicholas Bucher <behappy54321@gmail.com>
8264ec6 to
ec60b82
Compare
| }, nil | ||
| } | ||
|
|
||
| func (s *grpcServer) RenameAgentInstance(ctx context.Context, request *apiv1alpha1.RenameAgentInstanceRequest) (*apiv1alpha1.RenameAgentInstanceResponse, error) { |
There was a problem hiding this comment.
Do we still need this? I think we're doing auto-suspend/resume
| // InterruptActiveAgentInstanceTask fails the expected task and records an | ||
| // interruption. It returns false if that task is no longer active. | ||
| InterruptActiveAgentInstanceTask(context.Context, string, string) (bool, error) | ||
| // AbandonActiveAgentInstanceTask cancels the expected task, releasing the |
There was a problem hiding this comment.
This is A LOT of new queries, do we need all of these just to get the UI off the ground. These are the sort of thing that are harder to roll back
| return &agentTemplateServer{service: service, maxMessageBytes: maxMessageBytes} | ||
| } | ||
|
|
||
| func (s *agentTemplateServer) ListAgentTemplates(ctx context.Context, request *apiv1alpha1.ListAgentTemplatesRequest) (*apiv1alpha1.ListAgentTemplatesResponse, error) { |
There was a problem hiding this comment.
All of the CRUD functionality is identical, can we use generics?
| if config.AgentService != nil { | ||
| apiv1alpha1.RegisterAgentServiceServer(grpcServer, newAgentServer(config.AgentService, config.MaxMessageBytes)) | ||
| } | ||
| if config.AgentTemplateService != nil { |
There was a problem hiding this comment.
if x == nil is a claudism, this can't be nil here
| if s.config.GrpcWebRouter == nil { | ||
| return next | ||
| } |
There was a problem hiding this comment.
claudism, no need for this
| // more than one binary serving HTTP beside that server, and a second copy of | ||
| // the rule would drift. | ||
| // | ||
| // Optional: left nil, this server behaves exactly as it did before one existed. |
There was a problem hiding this comment.
Is it actually nil though, don't we pass it every time?
There was a problem hiding this comment.
This is also identical to harness.Service. Can we use generics?
There was a problem hiding this comment.
A concrete way to keep this small is the prototype pattern used by skv2: parameterize the service with T client.Object and L client.ObjectList, pass &v1alpha3.Harness{} / &v1alpha3.HarnessList{} (or the AgentTemplate equivalents), and allocate with prototype.DeepCopyObject().(T). See https://github.com/solo-io/skv2/blob/main/pkg/client/client.go. That should share the CRUD mechanics without reflection or constructor plumbing; only the resource-specific spec/status transition needs a small callback.
| // | ||
| // Asked rather than assumed: a session share reaching the A2A gateway must not be | ||
| // treated as authority over an instance that happens to share its id space. | ||
| func (s *ShareContext) IsForAgentInstance(instanceID string) bool { |
There was a problem hiding this comment.
Sessions are dead, just replace this
| rpc ListHarnesses(ListHarnessesRequest) returns (ListHarnessesResponse); | ||
| rpc GetHarness(GetHarnessRequest) returns (GetHarnessResponse); | ||
| rpc CreateHarness(CreateHarnessRequest) returns (CreateHarnessResponse); | ||
| rpc UpdateHarness(UpdateHarnessRequest) returns (UpdateHarnessResponse); |
There was a problem hiding this comment.
Can we omit GetHarness and UpdateHarness from this PR? The rewritten UI calls list, create, and delete but has no consumer for either RPC. They pull later CLI and apply API surface into the UI cutover and add proto, generated, handler, service, policy, and test code speculatively. Add them with the consumer that needs them.
|
|
||
| // substrateCache memoises the substrate reads for substrateCacheTTL. | ||
| // | ||
| // The singleflight group is the other half of the point: without it, the three |
There was a problem hiding this comment.
This cache does not collapse the three reads described here: summary, actors, and workers deliberately use different keys, and singleflight only coalesces equal keys. Its 400ms TTL also expires before normal polling, while the UI client already deduplicates identical reads. Can we delete this cache and compute the requested result directly until measurements show duplicate same-key calls are a real production cost? That removes the any cache, locking, eviction, and about 280 lines including tests.
| @@ -0,0 +1,720 @@ | |||
| package system | |||
There was a problem hiding this comment.
Can we split the Substrate inventory scalability work into a separate PR? The new summary/paged RPCs, streaming selector, cache, proto surface, and tests are roughly two thousand handwritten/test lines plus generated output. This is a substantial independent backend feature and makes the UI transport/conversation changes much harder to review atomically.
| // Optional display name. Omit it to create an unnamed conversation. Unvalidated | ||
| // because empty is the ordinary case: a conversation is usually named later, or | ||
| // never. | ||
| string name = 5; |
There was a problem hiding this comment.
This request-intrinsic validation belongs in the proto, per the shared Protovalidate interceptor. max_len handles the 200-character bound and CEL can reject surrounding whitespace and control characters while allowing empty names. That lets us delete validateName; its UTF-8 check is redundant because protobuf strings are already valid UTF-8. The new template and harness filter validation should move here as well rather than being duplicated in the service.
| // working state so a reply can be delivered, returning the task as it was | ||
| // parked and whether this call claimed it. A second caller is refused, which | ||
| // is what stops a duplicate reply being delivered twice. | ||
| ClaimParkedAgentInstanceTask(context.Context, string, string) (*a2a.Task, bool, error) |
There was a problem hiding this comment.
ClaimParkedAgentInstanceTask and RestoreParkedAgentInstanceTask are added and extensively tested, but no production caller uses either one. prepareReply still does GetAgentInstanceTask followed by StoreAgentInstanceTaskEvent, so two concurrent replies can both observe the parked state and both dispatch. Please either make prepareReply use the claim and restore replay guard, or remove the unused interface, SQL, implementation, and tests; the current version pays for both approaches while retaining the race.
2 participants
Summary
Rebuilds the web interface on Vite + React 19 — React Router, SWR, antd 6, Emotion. It is a static bundle behind nginx with no server process; settings come from
window.environmentVariables, rewritten by the container on every start, so one image serves every deployment.The application API is reached over gRPC-Web:
grpcserver.WebHandlerwraps the existing*grpc.Server, and the HTTP server routes gRPC-Web requests to it ahead of its middleware chain.Note
Reading the diff. 616 of the 696 files are
ui/, which is the rewrite's tree — read it as a new app, not as a diff. The other 80 are three things:helm/— the UI pod stops running a Next.js server and becomes nginx serving a static bundle, songinx.conf,supervisord.conf,ui-deployment.yamland the UI values change together with their tests.go/andproto/— the gRPC-Web seam (grpcserver/grpcweb.go,httpserver/server.go,app.go) and the fiveAgentInstancechanges below, plus generated proto and sqlc output.CLAUDE.mdand.nvmrc— the repo guide's UI section, and the pinned Node version.Testing this PR
One command builds a Kind cluster and installs this checkout on it — controller, UI and agent runtime all built from the working tree, over the chart's published images. It ends holding a port-forward, so the last thing it prints is a working URL.
./ui/dev-scripts/setup-cluster.sh # ~25 min, mostly image buildshttp://localhost:8080
Tip
The script also leaves one agent on the cluster — an
assistanttemplate on akagentharness — so Agents has something in it and you can send a message straight away, without creating anything first.Warning
make create-kind-cluster && make helm-installdoes not work, and fails silently five different ways — including that the chart installs published images, so none of your changes are on the cluster while everything looks healthy.ui/dev-scripts/README.mdcovers each one, and the dev-server loop for iterating.UI Extensions
The app declares vendor extension points anyone can use to add to it or restyle it, all in one configuration object:
navItemsordernavOverridesroutesrouteHandlesslotsformFieldstableColumnsapiprovidersthemeshellbrandingproviderIconsagentLinksInstalling one is two edits: build a
VendorExtensionConfig, then pointsrc/vendorExtensions/activeConfig.tsat it. Overriding theme tokens restyles the app's own components, not just the extension's.Note
📖
ui/docs/vendor-extensions.md— every extension point and what it receives. Worth reading before reviewing thevendorExtensions/tree.Substrate
The pages follow the CRDs. An Agent is derived, not a resource — a
Harness×AgentTemplatepair read fromAgentTemplate.status.harnesses[], so there is no "New agent" button. The landing page explains the four concepts over three tabs; an agent's page lists its conversations, and a conversation is anAgentInstance.Five additive server-side changes, none affecting an existing caller:
AgentInstancegains anameListAgentInstancestakes a queryprepared_revision.app.godefaults the A2A gatewayTest Coverage
376 unit tests. The browser suite runs in Chromium and Firefox, plus a Chromium pass with the example extension installed.
Follow Ups
UI Playwright E2Eworkflow that did that is removed here, along with the harness it drove (playwright/scripts/setup.sh,playwright/mocks/server.mjs); restoring it is follow-up work.yarn test:pw:livecovers a few journeys against a real cluster in the meantime.ui/playwright/DEFERRED.mdlists the rest, and the surface each spec waits on. Nothing is committed as a skipped test.Note
go test ./...fails one pre-existing test on macOS —TestFetchSourceReusesExistingMaterialization,/varvs/private/var, in a package this change does not touch.Upgrade Notes
Caution
Breaking. The chart no longer sets
NEXT_PUBLIC_BACKEND_URL,BACKEND_INTERNAL_URLorBACKEND_GRPC_URL, and dropsui.backendInternalUrl,ui.backendGrpcUrlandui.volumes.nextjsCache.The oauth2-proxy
skip-auth-regexnow names/assets/andenv-config.jsinstead of the Next.js paths, which no longer exist.Also included, unrelated to the rewrite:
helm/tools/grafana-mcpnow passes-allowed-hosts. The server rejects any Host header it was not told about, so one reached over the cluster network answered the MCP handshake withForbidden— theRemoteMCPServersatAccepted=Falseand every agent using it failed to build its tool set. Found while testing the tools pages; happy to split it out if preferred.🤖 written by Claude