feat(agent-applications): auth & identity section with admission editing#3021
Draft
benjackwhite wants to merge 22 commits into
Draft
feat(agent-applications): auth & identity section with admission editing#3021benjackwhite wants to merge 22 commits into
benjackwhite wants to merge 22 commits into
Conversation
Lets an owner attach a shared MCP connection to an agent and edit its per-tool permissions, reusing PostHog Code's native mcp_store connect flow. Pairs with the runtime in posthog#66007 (spec.mcps[].connection). - mcp-server-manager: new shared module — useMcpConnect (installations query + connect/reauthorize over the host OAuth callback) + the moved AddCustomServerForm. mcp-servers' useMcpServers now consumes the shared mcpKeys/createOAuthCallback. - agent-applications: editable McpsOverview (add an MCP from a connection, or connect a new server inline) + McpBody (connection picker, per-tool requires_approval toggles), persisted via useApplyAgentSpec. - foundation (built on main): api-client updateAgentRevisionSpec + useApplyAgentSpec (+ test) — draft-branch-on-save then PATCH. Compatible with the in-flight agent-model-policy-ui branch so they merge cleanly. - drive-by: cast spec.entrypoint in ModelBody to fix a pre-existing @posthog/ui typecheck error (AgentSpec index signature) unrelated to this change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- useApplyAgentSpec: invalidate the revision detail via a new agentApplicationsKeys.revisionPrefix() instead of a hardcoded key array, so the centralised key registry stays the single source of truth. - McpBody: destructure the ctx fields the apply() callback reads (revisionId, revisionState, onSelectRevision) into its dep array; ctx is a fresh object literal each render, so depending on it defeated the useCallback memoization. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dock
The Edit-with-AI meta-agent's connect_mcp client tool parks the turn and renders
a PREFILLED AddCustomServerForm in the dock; the user completes the OAuth/api-key
connect themselves (auth never reaches the agent). On success the new mcp_store
connection is attached to the target agent's draft spec (mcps[].connection) and
the session is woken with { connected, connection_id, mcp_id }. Mirrors the
set_secret punch-out.
- agentBuilderStore: pendingMcpConnect state + setter
- useAgentBuilderClientTools: connect_mcp handler (parks + defers)
- AgentBuilderDock: connect form rendering + connect -> attach-to-spec -> resolve
- AddCustomServerForm: initialValues prop for prefill
- useMcpConnect: connectCustomAsync + refetchInstallations
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… legacy integrations - buildTree renders the authorable folder sections (triggers, secrets, skills, tools, mcps, identities) even when empty, so the add/connect affordance is reachable on a fresh agent (you couldn't add an MCP to an agent that had none). - Remove the deprecated integrations UI from the config pane (tree section, overview/body, routing, description, the legacy mcps[].auth.integration row, now-unused LinkIcon). Removing integrations from the agent-platform spec + runtime is a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The agent-builder dock now sends the kind:'client' tool ids it can execute (AGENT_BUILDER_CLIENT_TOOLS — set_secret, connect_mcp, focus_*, toast, get_context) in the /run body so the runner knows it can punch out the interactive connect_mcp form here instead of relaying a URL. Threaded AGENT_BUILDER_CLIENT_TOOLS → useAgentChat (supportedClientTools option) → AgentChatSession → agentChatService → runAgentSession (api-client) → POST /run body. Replaces relying on the never-sent x-posthog-client header. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The inline punch-out crammed the full AddCustomServerForm into the dock's narrow above-composer strip — ugly. Move connect_mcp to a proper Dialog (AgentBuilderMcpConnectDialog); the set_secret one-line punch-out stays inline. Adds a hideHeader prop to AddCustomServerForm so the dialog owns the title/close chrome (no in-form Back button / duplicate heading). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Producer counterpart to the agent-platform server change that gates client-tool exposure on a per-run capability list. The agent-builder dock now declares the `kind:'client'` tool ids it can fulfil and sends them in the /run body as `supported_client_tools`, so the runner exposes only those to the model (spec ∩ supported). - `runAgentSession` (api-client): optional `supportedClientTools`, added to the /run body when non-empty. - `AgentChatSession` (core): carries `supportedClientTools`; `agentChatService` forwards it to `runAgentSession`. - `useAgentChat` (ui): new `supportedClientTools` option threaded into the session config. - `AGENT_BUILDER_CLIENT_TOOLS` declares the dock's fulfillable ids (set_secret, focus_*, toast, get_context) — verified to exactly match the agent-builder spec's `kind:'client'` tools — and is passed from the dock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The agent-config MCP sections expanded the custom-server form inline under a Connect-new/Cancel toggle. Pop it out in a dialog instead (matching the agent builder's connect_mcp punch-out). Extract AddCustomServerDialog as the shared modal chrome and refactor AgentBuilderMcpConnectDialog to delegate to it, so both the manual UI and the punch-out share one wrapper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…' into ben/agent-mcp-connections # Conflicts: # packages/api-client/src/posthog-client.ts # packages/core/src/agent-chat/identifiers.ts # packages/ui/src/features/agent-applications/agent-builder/useAgentBuilderClientTools.ts # packages/ui/src/features/agent-applications/hooks/useAgentChat.ts
The MCP config could add (Connect new / Add from a connection) and edit the connection, but had no way to drop an mcps[] entry — only clear its connection. Add a Remove server action to the MCP detail that filters the entry out of the spec and returns to the list. The shared mcp_store installation is untouched; only the agent's reference goes away. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ions # Conflicts: # packages/ui/src/features/agent-applications/agent-builder/useAgentBuilderClientTools.ts # packages/ui/src/features/agent-applications/hooks/useAgentChat.ts
…ntSpec The UI removal of the integrations section (IntegrationsOverview/IntegrationBody, cfg:integrations node, SECTION_INFO entry) left `AgentSpec.integrations?: string[]` behind — a soft regression where a shipped spec's integrations would be silently un-editable. Nothing in the app reads the field (the agent-platform schema already dropped it), so remove it for a total removal. Per review: this is a deliberate "nobody uses integrations" call, not collateral from the MCP refactor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ions # Conflicts: # packages/ui/src/features/agent-applications/components/AgentConfigurationPane.tsx # packages/ui/src/features/agent-applications/hooks/agentApplicationsKeys.ts # packages/ui/src/features/agent-applications/hooks/useApplyAgentSpec.test.ts # packages/ui/src/features/agent-applications/hooks/useApplyAgentSpec.ts
…+ per-tool overrides) The connection MCP detail now shows the server's live tool catalog (useMcpInstallationTools) with a connection-wide default permission and per-tool overrides, reusing the standalone manager's ToolRow / ToolPolicyToggle. Levels map allow/approve/deny <-> approved/needs_approval/do_not_use at the boundary; edits persist to spec.mcps[].default_tool_approval + tools[].level via the existing draft-branch-then-PATCH apply(). Add-from-connection now stamps a safe default_tool_approval: 'approve' so the runtime and UI agree from first save. Also clarifies the agent-level (shared connection — owner connects once, every asker reuses) vs principal-level (per-asker identity provider) distinction in the connection description. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…issions Extract the searchable/expandable tool list (counts, per-tool policy toggle, expandable descriptions) into a presentational ToolPermissionList beside the mcp-servers parts, and wire McpBody to it. The parent owns config + persistence (spec default_tool_approval + per-tool level overrides); the component is callback-driven so it can also back the global MCP-servers manager unchanged. ServerDetailView is left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… on MCP detail Replace the implicit "set a connection vs wire auth.provider" model with an explicit credential-model dropdown reading/writing the new required spec.mcps[].kind: - Agent-level → connection selector + per-tool permissions. - Principal-level → identity-provider selector (+ jump, or a prompt to declare one). Switching modes clears the opposite credential and stamps `kind`. - Tidy the layout: the mode dropdown and "Remove server" share one compact header row at the top with a single contextual helper line, instead of a verbose blurb up top and a remove button buried at the bottom. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the principal-level identity UI nicer: the provider dropdown shows a fingerprint icon per option, and the selected identity renders as a JumpRow preview card (icon + provider + "per-asker linked identity" status, amber "undeclared" badge when auth.provider isn't in identity_providers[]) that jumps to the identity detail — replacing the plain text link. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
main dropped the `sonner` dependency (toast moved to @posthog/ui/primitives/ toast, #2965), but these two files still imported from it — passing local typecheck (sonner still in node_modules) yet breaking CI's clean build/ typecheck ("Rollup failed to resolve import 'sonner'"). Point both at the ui primitive like the rest of the app. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Renames the read-only Identities section to 'auth & identity' and moves it above triggers — admission gates whether any trigger can start a session. Adds full editing for the transport-identity admission model: - authoritative_provider picker (None / declared providers; providers that can't prove a subject are disabled), with a validation warning when the selected provider is undeclared or non-subject-proving (mirrors the zod superRefine). - Add a PostHog identity (one click) or a bring-your-own oauth2 provider (inline form; userinfo_url is what makes it eligible as authoritative). - Per-provider make/clear authoritative + remove, and an 'auth'/'authoritative' badge in the tree and overview. Persists via useApplyAgentSpec (draft-branch-then-PATCH, whole-spec replace).
|
React Doctor found 3 issues in 1 file · 3 warnings. 3 warnings
Reviewed by React Doctor for commit |
Contributor
|
Reviews (1): Last reviewed commit: "feat(agent-applications): auth & identit..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+2179
to
+2195
| const submit = () => { | ||
| if (!valid) return; | ||
| const entry: Record<string, unknown> = { | ||
| kind: "oauth2", | ||
| id: trimmedId, | ||
| binding: "principal", | ||
| authorize_url: authorizeUrl.trim(), | ||
| token_url: tokenUrl.trim(), | ||
| client_id: clientId.trim(), | ||
| scopes: scopes | ||
| .split(/[\s,]+/) | ||
| .map((s) => s.trim()) | ||
| .filter(Boolean), | ||
| }; | ||
| const ui = userinfoUrl.trim(); | ||
| if (ui) entry.userinfo_url = ui; | ||
| onAdd(entry); |
Contributor
There was a problem hiding this comment.
client_secret absent from the OAuth2 entry
The submit function builds the provider entry with client_id but no client_secret. Standard OAuth2 authorization-code flow requires the secret server-side to exchange the authorization code for a token at token_url. If the backend spec (posthog PR #66050) expects a client_secret field, any session gated on this provider will fail silently at the token-exchange step — the error won't surface until a real user tries to authenticate.
If PKCE is used exclusively (no secret needed), a comment here would prevent the same question from landing in every review. If the secret must be supplied, a client_secret field (or an explicit reference to which env-key holds it) is missing from the form. Is the OAuth2 token exchange PKCE-only, or does the backend spec require a client_secret on the provider entry? If the latter, this form needs a field for it (or a way to reference a secret key).
Comment on lines
+1978
to
+1994
| const saveSpec = (next: AgentSpec, onDone?: () => void) => { | ||
| if (!ctx.revisionState) return; | ||
| applySpec.mutate( | ||
| { | ||
| revision: { id: ctx.revisionId, state: ctx.revisionState }, | ||
| spec: next, | ||
| }, | ||
| { | ||
| onSuccess: (rev) => { | ||
| if (rev.id !== ctx.revisionId) ctx.onSelectRevision?.(rev.id); | ||
| onDone?.(); | ||
| }, | ||
| onError: (e) => | ||
| toast.error(e.message || "Failed to save identity providers"), | ||
| }, | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
saveSpec defined identically in two components
This function is defined in IdentitiesOverview (here) and again in IdentityBody (~line 2290) with the only difference being the error-message noun ("providers" vs "provider"). Both have identical mutation logic, onSuccess redirect, and error handling. Extracting this into a shared hook (e.g. useApplySpec(ctx, defaultErrorMessage)) would make future changes to the retry/redirect logic atomic.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // `spec.authoritative_provider` = the one identity_providers[] id that gates | ||
| // admission. It can only be authoritative if it proves a subject (kind posthog, | ||
| // or oauth2 + userinfo_url) — mirrors AgentSpecSchema.superRefine. | ||
| const NONE_PROVIDER = "__none__"; |
Contributor
There was a problem hiding this comment.
NONE_PROVIDER sentinel can collide with a real provider id
If a user names their OAuth2 provider __none__, the select dropdown will contain two <Select.Item> elements with the same value ("__none__"). Radix UI's Select doesn't deduplicate — both items render, and onValueChange fires "__none__" regardless of which is chosen, so the setAuthoritative handler will always clear authoritative_provider rather than set it to the provider. A Symbol or a value that is structurally impossible as a spec id (e.g. the empty string, which the backend likely rejects) would eliminate the collision risk.
The runtime no longer enforces the shared connection owner's per-tool approval marks — the agent's own allow/approve/deny config is authoritative. Surface that in the MCP "Tool permissions" section so authors know the owner's marks are informational here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Stacked on #2929 (
ben/agent-mcp-connections). The agent-builder UI counterpart to posthog PR #66050 (authoritative-provider identity admission).Adds an Auth & identity section to the agent configuration pane: edit a revision's identity providers and the
authoritative_providerthat gates admission, alongside the MCP connection work from the base branch.🤖 Generated with Claude Code