feat: enable sharing of chat sessions#1935
Conversation
Closes kagent-dev#1933 Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds end-to-end “shared session” support via share tokens, including UI affordances to create/manage share links, frontend propagation of X-Share-Token, and backend persistence + enforcement (read-only vs read-write) for shared access.
Changes:
- UI: create/manage share links, show shared-session indicators, and propagate share tokens through chat/task/A2A calls.
- Backend (Go): add session share tables, middleware validation/enforcement, share CRUD endpoints, and session listing enhancements with share metadata.
- Agent runtimes (Python/Go ADK) + Helm/CRDs: optional “share tools” injection and
KAGENT_UI_URLplumbing for generating clickable share URLs.
Reviewed changes
Copilot reviewed 54 out of 59 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/types/index.ts | Extends Session type with optional share_token and share_read_only. |
| ui/src/lib/auth.ts | Forwards x-share-token header by default. |
| ui/src/lib/a2aClient.ts | Allows attaching X-Share-Token to streaming A2A requests; tightens types. |
| ui/src/lib/tests/auth.test.ts | Updates default-forward-header expectations for share token forwarding. |
| ui/src/components/sidebars/SessionGroup.tsx | Passes share metadata down to chat items. |
| ui/src/components/sidebars/ChatItem.tsx | Appends ?share=... to shared-session links and shows shared icon. |
| ui/src/components/chat/ShareButton.tsx | New client component to create/copy/revoke share links. |
| ui/src/components/chat/ChatMessage.tsx | Simplifies ask-user submit callback wiring. |
| ui/src/components/chat/ChatInterface.tsx | Propagates share token through reads/A2A and enforces read-only UI mode; adds ShareButton. |
| ui/src/components/chat/AskUserDisplay.tsx | Disables ask-user interactions when handlers are absent (read-only mode). |
| ui/src/components/ToolDisplay.tsx | Disables approve/reject buttons when no handlers (read-only mode). |
| ui/src/app/agents/[namespace]/[name]/chat/[chatId]/page.tsx | Reads share query param and passes it to ChatInterface (with Suspense). |
| ui/src/app/actions/sessions.ts | Adds optional shareToken header support for session/task fetches + getSessionWithEvents. |
| ui/src/app/actions/sessionShares.ts | New server actions for session share CRUD. |
| python/packages/kagent-adk/tests/unittests/test_share_tools.py | Adds unit tests for Python share tools. |
| python/packages/kagent-adk/src/kagent/adk/types.py | Adds share_tools flag to AgentConfig and injects share tools when enabled. |
| python/packages/kagent-adk/src/kagent/adk/tools/share_tools.py | Implements Python ADK share tools and URL generation. |
| python/packages/kagent-adk/src/kagent/adk/tools/init.py | Exports the new Python share tools. |
| helm/kagent/values.yaml | Adds controller.externalUrl for setting KAGENT_UI_URL. |
| helm/kagent/templates/controller-deployment.yaml | Injects KAGENT_UI_URL into controller when configured. |
| helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml | Adds shareTools field to rendered CRD schema. |
| helm/kagent-crds/templates/kagent.dev_agents.yaml | Adds shareTools field to rendered CRD schema. |
| go/core/pkg/migrations/core/000005_session_shares.up.sql | Adds session_share and session_share_access tables. |
| go/core/pkg/migrations/core/000005_session_shares.down.sql | Adds down migration (currently only drops one table). |
| go/core/pkg/env/kagent.go | Registers KAGENT_UI_URL env var. |
| go/core/pkg/auth/share_test.go | Unit tests for ShareContext helpers. |
| go/core/pkg/auth/share.go | Adds ShareContext storage/retrieval in request context. |
| go/core/internal/httpserver/server_share_middleware_test.go | Adds tests for share-token middleware behavior. |
| go/core/internal/httpserver/server.go | Registers share routes and share-token middleware. |
| go/core/internal/httpserver/middleware.go | Implements share-token validation, read-only enforcement, and access recording. |
| go/core/internal/httpserver/handlers/sessions_test.go | Extends session handler tests for read_only field + updated list type. |
| go/core/internal/httpserver/handlers/sessions_share_context_test.go | Adds tests for effective user ID resolution under share context. |
| go/core/internal/httpserver/handlers/sessions.go | Uses ShareContext for DB lookups + returns read_only in responses. |
| go/core/internal/httpserver/handlers/session_shares_test.go | Adds tests for session share CRUD handlers. |
| go/core/internal/httpserver/handlers/session_shares.go | Implements session share CRUD endpoints. |
| go/core/internal/httpserver/handlers/handlers.go | Wires SessionSharesHandler into handler set. |
| go/core/internal/httpserver/auth/authn.go | Forces agent upstream calls to use share owner’s X-User-Id when share context present. |
| go/core/internal/database/queries/sessions.sql | Enhances session listing to include shared sessions and share metadata. |
| go/core/internal/database/queries/session_shares.sql | Adds SQL queries for share CRUD and access upsert. |
| go/core/internal/database/client_postgres.go | Implements DB methods for shares + new session list return type. |
| go/core/internal/controller/translator/agent/manifest_builder.go | Propagates KAGENT_UI_URL into agent pods when set. |
| go/core/internal/controller/translator/agent/compiler.go | Passes shareTools flag into ADK AgentConfig. |
| go/core/internal/a2a/manager_test.go | Tests initiated_by injection when share context is present. |
| go/core/internal/a2a/manager.go | Injects initiated_by metadata for shared-session A2A messages. |
| go/api/v1alpha2/agent_types.go | Adds shareTools field to Agent CRD Go types. |
| go/api/database/models.go | Adds SessionWithShareToken and SessionShare models. |
| go/api/database/client.go | Extends DB client interface with share methods and updated session list signature. |
| go/api/config/crd/bases/kagent.dev_sandboxagents.yaml | Adds shareTools to generated CRD base. |
| go/api/config/crd/bases/kagent.dev_agents.yaml | Adds shareTools to generated CRD base. |
| go/api/adk/types.go | Adds share_tools to ADK AgentConfig JSON handling. |
| go/adk/pkg/tools/share_tools_test.go | Adds tests for Go ADK share tools. |
| go/adk/pkg/tools/share_tools.go | Implements Go ADK share tools and URL generation. |
| go/adk/pkg/runner/adapter.go | Conditionally injects share tools into Go ADK runner. |
| go/adk/cmd/main.go | Passes HTTP client + kagent URL into runner config for share tools. |
Files not reviewed (5)
- go/api/v1alpha2/zz_generated.deepcopy.go: Language not supported
- go/core/internal/database/gen/models.go: Language not supported
- go/core/internal/database/gen/querier.go: Language not supported
- go/core/internal/database/gen/session_shares.sql.go: Language not supported
- go/core/internal/database/gen/sessions.sql.go: Language not supported
Comments suppressed due to low confidence (8)
go/core/pkg/migrations/core/000005_session_shares.down.sql:1
- The down migration drops
session_sharebut notsession_share_access. Sincesession_share_accesshas a foreign key referencingsession_share, droppingsession_sharefirst will fail in Postgres (or leave orphaned objects depending on settings). Dropsession_share_accessfirst, thensession_share(and consider dropping the index too for completeness).
ui/src/components/chat/ShareButton.tsx:1 - This references
windowduring render. In Next.js App Router, client components can still be pre-rendered on the server, wherewindowis undefined, causing a runtime crash. Computeorigininside an effect (store in state), or guard withtypeof window !== \"undefined\"before readingwindow.location.origin.
ui/src/components/chat/ChatInterface.tsx:1 shareReadOnlyis only set inside theshareTokenbranch. If the user navigates from a shared read-only session to a non-shared session without a full remount,shareReadOnlycan remaintrueand incorrectly disable interactions. ResetshareReadOnlytofalsewhenshareTokenis absent (e.g., at the start of initialization or in theelsebranch).
ui/src/components/chat/ChatInterface.tsx:1shareReadOnlyis only set inside theshareTokenbranch. If the user navigates from a shared read-only session to a non-shared session without a full remount,shareReadOnlycan remaintrueand incorrectly disable interactions. ResetshareReadOnlytofalsewhenshareTokenis absent (e.g., at the start of initialization or in theelsebranch).
ui/src/components/chat/ChatInterface.tsx:1shareReadOnlyis only set inside theshareTokenbranch. If the user navigates from a shared read-only session to a non-shared session without a full remount,shareReadOnlycan remaintrueand incorrectly disable interactions. ResetshareReadOnlytofalsewhenshareTokenis absent (e.g., at the start of initialization or in theelsebranch).
ui/src/components/sidebars/ChatItem.tsx:1- The share token is inserted into the URL query string without encoding and will be stored in browser history and may leak via referrer headers/logs. At minimum, wrap the token with
encodeURIComponent(or build viaURLSearchParams) to prevent malformed URLs; consider using a URL fragment (e.g.#share=...) if feasible so the token is not emitted as an HTTP referrer, while still being readable by the client.
python/packages/kagent-adk/src/kagent/adk/tools/share_tools.py:1 - Docstring says that without
KAGENT_UI_URLthe function returns only the raw token, but the implementation returns a relative path containing?share=<token>. Update the docstring to match actual behavior (relative path vs raw token).
ui/src/components/chat/ShareButton.tsx:1 - This input has no accessible name (no associated
<Label>and noaria-label), which makes the share URL harder to discover for screen reader users. Add anaria-label(e.g. "Share link") or associate it with a visible label.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tion session_share_access has a FK referencing session_share, so dropping session_share first fails in Postgres. Reverse the order. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
…TERAL ListSessionsForAgent was running two separate correlated subqueries per session row to fetch share_token and share_read_only, doubling the work for the planner. A single LATERAL join fetches both fields in one pass and also eliminates the redundant EXISTS check in the WHERE clause. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
The delete handler was calling DeleteSessionShare without first checking that the session belongs to the caller. A non-owner would receive 200 OK even though nothing was deleted. Added the same GetSession ownership check that create and list already perform, returning 404 on mismatch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
shareReadOnly was only set inside the shareToken branch. Navigating from a read-only shared session to an owned session without a full remount left it stuck at true, incorrectly disabling interactions. Reset it to false at the start of initializeChat so the non-shared path is always clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
…input window.location.origin was read at render time, which crashes during Next.js server-side pre-rendering where window is undefined. Moved to a useEffect so origin is only read client-side. Also added aria-label to the read-only share URL input so screen readers can identify it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
The docstring claimed the function returns the raw token when KAGENT_UI_URL is unset. In reality it returns a relative path with the share query param. Updated the docstring to accurately describe both branches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
|
Noticed that the build failed after latest merge; can't replicate the failure locally though so maybe a retry will sort it out. |
And set on controller ConfigMap rather than directly on the Deployment Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Exposes the DeclarativeAgentSpec.shareTools field in the UI so agents can be configured to gain the built-in share link tools without needing to edit the CRD directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
supreme-gg-gg
left a comment
There was a problem hiding this comment.
I've tried this and it works very well, this implementation looks great overall according to the design in #1933. Just some small comments
| @@ -9,6 +9,9 @@ data: | |||
| A2A_BASE_URL: {{ include "kagent.a2aBaseUrl" . | quote }} | |||
| DEFAULT_MODEL_CONFIG_NAME: {{ include "kagent.defaultModelConfigName" . | quote }} | |||
| KAGENT_CONTROLLER_NAME: {{ include "kagent.fullname" . }}-controller | |||
| {{- if .Values.ui.externalUrl }} | |||
| KAGENT_UI_URL: {{ .Values.controller.externalUrl | quote }} | |||
There was a problem hiding this comment.
this should be .Values.ui.externalUrl?
| return f"{_KAGENT_UI_URL}{path}" if _KAGENT_UI_URL else path | ||
|
|
||
|
|
||
| async def _kagent_request( |
There was a problem hiding this comment.
For the python adk tool, I think it can be better if we reuse the existing http client which already has token service. This would require moving the tools initialization to _a2a.py instead, something like this:
def create_runner() -> Runner:
root_agent = self.root_agent_factory()
if not local and http_client is not None and self.agent_config and self.agent_config.share_tools:
from kagent.adk.tools.share_tools import CreateShareLinkTool, ListShareLinksTool, DeleteShareLinkTool
root_agent.tools.extend([
CreateShareLinkTool(http_client),
ListShareLinksTool(http_client),
DeleteShareLinkTool(http_client),
])
...so in the tools it can directly use the http client to make requests. This would reduce code duplication and share existing components like how it's done in go adk.
There was a problem hiding this comment.
Cool - will make it happen
There was a problem hiding this comment.
Personally I think it would be helpful to show a table of all share urls created for this session (so the user can copy / delete generated tokens), similar to how the agent can use the list token tool to view them
There was a problem hiding this comment.
OK. Was torn on this myself... But I guess if the backend supports multiple share links then the UI should reflect that too.
| // When the request carries a share token, the agent pod must use the | ||
| // session owner's identity — not the caller's — when calling back to | ||
| // the controller, so that session lookups resolve to the owner's row. | ||
| if sc, ok := auth.ShareContextFrom(ctx); ok { |
There was a problem hiding this comment.
It seems like the A2A contextId is not bound to the shared token, unlike the REST endpoint where getEffectiveUserIDForSession checks that sc.SessionId == sessionID so that it only returns the owner's ID when the token's scoped session is the requested session. For the A2A case it always adds the owner without checking contextID == sc.SessionID. I think this can be added in A2A send methods in manager.go
There was a problem hiding this comment.
I noted in #1933 you mentioned needing <meta name="referrer" content="no-referrer"> in the security considerations section but it hasn't been added here (or in the layout)
|
|
||
| share, err := s.config.DbClient.GetSessionShareByToken(r.Context(), token) | ||
| if err != nil { | ||
| http.Error(w, "Invalid or expired share token", http.StatusForbidden) |
There was a problem hiding this comment.
nit: distinguish token not found from DB operation failure, it's a invalid for expired token only if the error is pgx.ErrNoRows otherwise we should return 500
There was a problem hiding this comment.
Fair point. Will fix.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
…middleware Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
… leakage Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Move share tool injection from AgentConfig.to_agent() into _a2a.py's create_runner(), where the pre-built httpx.AsyncClient (with token auth event hooks) is available. The tools now accept the client instead of creating a new one per request and reading the auth token manually. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
…y/delete Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
- Resolved modify/delete conflict on go/core/internal/a2a/manager.go: main deleted it (trpc A2A library replaced); our validateShareContext and injectInitiatedBy logic moved to passthrough_handler.go - Resolved conflict in handlers.go and sessions.go (import changes) - Resolved conflict in ui/src/app/agents/new/page.tsx and ChatInterface.tsx Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
…hare tools Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
main added 000005_a2a_protocol_version; avoid duplicate migration number. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
- Add min-w-0 to URL span so truncate works inside flex row - Fall back to execCommand copy when Clipboard API unavailable (HTTP/non-secure contexts) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Thanks for the review. Think I have addressed all your comments. Also merged main which required a bit of reworking due to the a2a libray migration. |
The middleware now distinguishes pgx.ErrNoRows (403) from other DB errors (500). Update the invalid/revoked token test cases to return pgx.ErrNoRows so they expect 403 as intended. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
supreme-gg-gg
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments quickly! lgtm overall
Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Design choices are documented in #1933. Will address/respond to comments either here or there if needed and keep the implementation in sync as we go.
For testing locally, I first installed locally and started a few session as the default user
admin@kagent.dev, I then reinstalled with trusted proxy enabled whereby I could test as my real user.Closes #1933