Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/CONSUMING.md
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,10 @@ do, and what does it need from me. Credential headers are reported with a
`supplied` flag rather than a value — a deployment holding one shared key
should not make every visitor paste it, and a visitor with an account of their
own can still override it, because a request header beats the environment.
The flag is resolved per request, the way a run resolves it, so a deployment
that sets a per-user credential header for a signed-in caller — a dependency in
front of the router ([5b](#5b-mounting-the-routes-into-your-own-application)) —
should set it on this route too, and those callers are not asked for it.

### 5d. The event stream

Expand Down
9 changes: 5 additions & 4 deletions js/agent-ui/src/agui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import type { Interrupt } from "@ag-ui/client";

import { apiUrl } from "./config";
import { apiFetch } from "./session";

import type { Declared } from "./credentials";

Expand Down Expand Up @@ -53,7 +54,7 @@ export async function readState(
turn?: number,
): Promise<StateValue> {
const at = turn === undefined ? "" : `?turn=${turn}`;
const response = await fetch(apiUrl(`/threads/${threadId}/state/${key}${at}`));
const response = await apiFetch(apiUrl(`/threads/${threadId}/state/${key}${at}`));
if (!response.ok) {
// The API's own wording, which distinguishes a turn that never existed
// (404) from one the checkpointer has pruned (410) — a difference worth
Expand All @@ -74,7 +75,7 @@ export async function readState(
* refuses a new message until it is.
*/
export async function readThread(threadId: string) {
const response = await fetch(apiUrl(`/threads/${threadId}`));
const response = await apiFetch(apiUrl(`/threads/${threadId}`));
// 404 is the ordinary answer for a thread id that has never run, which is
// what a hand-edited URL produces. The caller starts fresh instead.
if (response.status === 404) return null;
Expand All @@ -95,7 +96,7 @@ export async function readThread(threadId: string) {
* is really made of.
*/
export async function readTurns(threadId: string) {
const response = await fetch(apiUrl(`/threads/${threadId}/turns`));
const response = await apiFetch(apiUrl(`/threads/${threadId}/turns`));
if (!response.ok) throw new Error(`${response.status}`);
return (await response.json()) as {
threadId: string;
Expand Down Expand Up @@ -125,7 +126,7 @@ export async function readConnections(): Promise<{
toolsets: { name: string; credentials: { header: string; supplied: boolean }[] }[];
tools: { name: string; description: string }[];
}> {
const response = await fetch(apiUrl("/connections"));
const response = await apiFetch(apiUrl("/connections"));
if (!response.ok) throw new Error(`${response.status}`);
return await response.json();
}
Expand Down
3 changes: 2 additions & 1 deletion js/agent-ui/src/chat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
outstanding,
save as saveCredentials,
} from "./credentials";
import { apiFetch } from "./session";

/** What `GET /connections` says this deployment is. */
type Connected = Awaited<ReturnType<typeof readConnections>>;
Expand Down Expand Up @@ -741,7 +742,7 @@ export function Chat() {
new URLSearchParams(location.search).get("thread") || crypto.randomUUID(),
);
const agent = useMemo(
() => new HttpAgent({ url: apiUrl("/runs"), threadId }),
() => new HttpAgent({ url: apiUrl("/runs"), threadId, fetch: apiFetch }),
[threadId],
);
const log = useRef<HTMLDivElement>(null);
Expand Down
58 changes: 58 additions & 0 deletions js/agent-ui/src/session.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/** What the client does when the API stops recognising it.
*
* A deployment that puts a session in front of this page — an OIDC proxy that
* signs the visitor in and adds their token to each request — outlives the
* page's first load. An access token measured in minutes against a
* conversation that can run for an hour means the session lapses *between*
* fetches, and from then on every route answers `401`.
*
* The client cannot sign anyone in: it holds no credential and knows nothing
* about what sits in front of it. What it can do is reload, because a page
* navigation is the one request the proxy answers with its own redirect to the
* sign-in page. Without this, a lapsed session reads as a broken chat — a
* thread that silently starts empty, or a run that ends in `client error`.
*/

/** Where the time of the last reload for a `401` is kept, for the tab. */
const RELOADED_AT = "mcp-agent-ui:reloaded-for-401";

/** How long after one reload another `401` is taken as not a lapsed session.
*
* A page whose routes answer `401` straight after a fresh load was not fixed
* by reloading, and reloading again would loop. Past this, a `401` is a new
* lapse and gets its reload.
*/
const GRACE_MS = 30_000;

/** Reload, unless this tab has just reloaded for a `401` already.
*
* When it does not — a `401` that a reload did not cure, or storage the
* browser will not let the page use — the caller's error handling runs as it
* would have, which at least shows the status.
*/
function reloadOnce(): void {
try {
const last = Number(sessionStorage.getItem(RELOADED_AT) ?? 0);
if (Date.now() - last < GRACE_MS) return;
sessionStorage.setItem(RELOADED_AT, String(Date.now()));
} catch {
// Without somewhere to record it, a reload cannot tell whether it is the
// second, so it does not happen.
return;
}
location.reload();
}

/** `fetch`, for the API's routes: a `401` reloads the page.
*
* The response is still returned, so the caller's own handling runs while the
* reload is under way; nothing waits on it.
*/
export async function apiFetch(
input: RequestInfo | URL,
init?: RequestInit,
): Promise<Response> {
const response = await fetch(input, init);
if (response.status === 401) reloadOnce();
return response;
}
32 changes: 18 additions & 14 deletions src/mcp_agent_api/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@
once and cached rather than repeated on every turn that renders it.
``GET /connections``
The toolsets the agent connected to, the credential headers each declared
and whether this server already holds one, and every tool. Nothing about a
conversation: it is what a client needs before there is one — an opening
screen that names what is connected, and a prompt for the credentials that
a run would otherwise be refused for.
and whether this caller's runs would have one without sending it, and
every tool. Nothing about a conversation: it is what a client needs before
there is one — an opening screen that names what is connected, and a prompt
for the credentials that a run would otherwise be refused for.

**The agent arrives through a callable, not as an argument.**
:func:`~mcp_agent.main.build_agent` is async and connects to MCP servers, so it
Expand Down Expand Up @@ -337,9 +337,10 @@ class CredentialInfo(BaseModel):

header: str
supplied: bool = Field(
description="Whether this server already holds a value for the header, "
"from its own environment. A client prompting for credentials asks "
"only for the ones it does not."
description="Whether a run from this caller would have a value for the "
"header without the client sending one: from the server's environment, "
"or set on the request by the deployment for this caller. A client "
"prompting for credentials asks only for the ones it does not."
)


Expand Down Expand Up @@ -989,25 +990,28 @@ async def read_view(toolset: str, view: str) -> HTMLResponse:
return HTMLResponse(html)

@router.get("/connections", responses={200: {"model": ConnectionsResponse}})
async def read_connections() -> dict[str, Any]:
async def read_connections(request: Request) -> dict[str, Any]:
"""The connected toolsets, their credential headers, and every tool.

Read from the built agent rather than re-fetched from the index: this
is what the agent actually connected to, which is the thing a client
is asking about.

``supplied`` is computed the way a run resolves credentials — the
environment, with a request header beating it — so a deployment that
holds one shared key does not ask every visitor for it. The values
themselves never appear here, only whether there is one.
``supplied`` is resolved for *this request*, exactly as a run resolves
credentials: the environment, with a request header beating it. So a
deployment holding one shared key does not ask every visitor for it,
and one that supplies a per-user credential — a dependency in front of
the router setting the header for a signed-in caller — does not ask
the callers it can supply. The values never appear here, only whether
there is one.
"""
agent = built()
from_environment = resolve_credentials(agent.required, {})
resolved = credentials_for(request.headers, agent.required)
toolsets = [
{
"name": name,
"credentials": [
{"header": header, "supplied": header in from_environment}
{"header": header, "supplied": header in resolved}
for header in sorted(headers)
],
}
Expand Down
20 changes: 20 additions & 0 deletions tests/mcp_agent_api/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,26 @@ async def test_a_header_the_server_already_holds_is_not_asked_for(
]


async def test_a_header_set_for_this_caller_is_not_asked_for():
"""A deployment that supplies a per-user credential — a dependency in front
of the router setting the header for a signed-in caller — must not tell
that caller to paste it (#127). Resolved per request, as a run resolves it.
"""
built = _built(connections={"cds": {}}, required={"cds": ["x-cds-token"]})
async with _client(built) as client:
theirs = (
await client.get("/connections", headers={"x-cds-token": "for-them"})
).json()
anyone_else = (await client.get("/connections")).json()

assert theirs["toolsets"][0]["credentials"] == [
{"header": "x-cds-token", "supplied": True}
]
assert anyone_else["toolsets"][0]["credentials"] == [
{"header": "x-cds-token", "supplied": False}
]


async def test_the_value_of_a_credential_is_never_on_the_route(
monkeypatch: pytest.MonkeyPatch,
):
Expand Down
Loading