Repository navigation
Add sign-in challenges and the authenticate tool to aitools mcp serve - #7553
Draft
simonfaltum wants to merge 55 commits into
Draft
simonfaltum wants to merge 55 commits into
simonfaltum wants to merge 55 commits into
Conversation
Contributor
|
Commit: Regression test reportSelected 15 changed tests (limit=20, 0 not selected) 15 tests: regression=15 unreleased=0 coverage=0 failing=0
TestAccept/cmd/aitools/mcp/add-agent-not-installed: branch ✅ | main (8e70c8b) ❌ | latest (v1.26.0) ❌main (8e70c8b): latest (v1.26.0): TestAccept/cmd/aitools/mcp/add-default-config: branch ✅ | main (8e70c8b) ❌ | latest (v1.26.0) ❌main (8e70c8b): latest (v1.26.0): TestAccept/cmd/aitools/mcp/add-text: branch ✅ | main (8e70c8b) ❌ | latest (v1.26.0) ❌main (8e70c8b): latest (v1.26.0): TestAccept/cmd/aitools/mcp/add-two-agents: branch ✅ | main (8e70c8b) ❌ | latest (v1.26.0) ❌main (8e70c8b): latest (v1.26.0): TestAccept/cmd/aitools/mcp/add-two-profiles: branch ✅ | main (8e70c8b) ❌ | latest (v1.26.0) ❌main (8e70c8b): latest (v1.26.0): 10 more failure details omitted, see the full report in the run. |
Collaborator
✅ Integration test reportCommit: 6b41591
Top 8 slowest tests (at least 2 minutes):
|
simonfaltum
force-pushed
the
aitools-mcp/07-login
branch
from
November 12, 2026 14:41
62d20d0 to
d7c9aec
Compare
simonfaltum
force-pushed
the
aitools-mcp/08-challenge
branch
from
November 12, 2026 14:42
38cdd13 to
1d8e6b3
Compare
simonfaltum
force-pushed
the
aitools-mcp/07-login
branch
from
November 12, 2026 17:13
d7c9aec to
d2dd977
Compare
simonfaltum
force-pushed
the
aitools-mcp/08-challenge
branch
from
November 12, 2026 17:14
1d8e6b3 to
7b98424
Compare
simonfaltum
force-pushed
the
aitools-mcp/07-login
branch
from
November 13, 2026 02:37
d2dd977 to
2a52223
Compare
simonfaltum
force-pushed
the
aitools-mcp/08-challenge
branch
from
November 13, 2026 05:35
7b98424 to
cb80384
Compare
simonfaltum
force-pushed
the
aitools-mcp/07-login
branch
from
November 13, 2026 07:16
0aa7987 to
a08bc42
Compare
simonfaltum
force-pushed
the
aitools-mcp/08-challenge
branch
from
November 13, 2026 07:25
1c6aee6 to
7a9dca9
Compare
For a service that needs provider sign-in, the gateway rejects tools/list until the user signs in, so an agent sees no tools. The proxy now answers a challenged tools/list with one authenticate tool. Calling it runs the sign-in and asks the agent to re-list. A challenged tools/call names the login command and asks the agent to re-list, without opening a browser. The sign-in page opens at most once per session, and again only after a timed-out attempt. Other services remain a pure passthrough. Co-authored-by: Isaac <no-reply@databricks.com>
The serve notice for a server with no install record no longer ends with a period, so the golden changes only in those two lines. Co-authored-by: Isaac <no-reply@databricks.com>
The proxy now accepts an upstream answer only when its id is the id of the request. Two challenge tests answered a request with a fixture that carried another request's id, so the answer was reported as a failed upstream response. Give each request in those tests its own fixture. Co-authored-by: Isaac <no-reply@databricks.com>
The sign-in now names the profile on stderr before the page, as `login` does, because the serve command passes the profile to the shared sign-in. Co-authored-by: Isaac <no-reply@databricks.com>
The authenticate tool runs the same sign-in that `login` runs. The shared helper now also reports whether the caller was already signed in and whether the sign-in page was shown, so the proxy no longer reads the credential before the sign-in reads it again. The sign-in timeout carries its wait and the failure of its last poll, and the workspace ID is resolved once per session. The shared sign-in helper also prints its progress on stderr. Refactor only, apart from the stderr progress the helper adds to the tool's output. Co-authored-by: Isaac <no-reply@databricks.com>
Git Bash on Windows converts a leading slash in the path argument of $CLI, so the calls made by the new serve acceptance scripts need the variable like the other scripts do. Co-authored-by: Isaac <no-reply@databricks.com>
…ints The sign-in hints of the authenticate tool wrote the login command by hand. The command now builds it with the same helper as add and login, so a server pinned to a config file names that file, and a profile name with a space is quoted. Co-authored-by: Isaac <no-reply@databricks.com>
A cancelled request has no response in the MCP spec, and the client has stopped waiting for one. The sign-in of a cancelled authenticate call returned a context error that was written as a failed sign-in. Co-authored-by: Isaac <no-reply@databricks.com>
The authenticate tool opened the sign-in page once per session and again only after a timeout, so a call that completed, failed, or was refused before the page showed left later calls waiting on a page that was never opened. The page is now held back only while a cancelled attempt that showed it may still be open in the user's browser. Co-authored-by: Isaac <no-reply@databricks.com>
Before a challenge the proxy has no tool to offer, so a tool named authenticate that the upstream lists is the upstream's and is forwarded. The proxy answers the call once the session has been challenged. Co-authored-by: Isaac <no-reply@databricks.com>
Binding and Unity Catalog compare service names without case, but a sign-in URL with a different case from the configured service name was not recognized, and the known-service table was looked up by a case-sensitive leaf. The URL text check also missed a sign-in URL that follows an opening bracket or quote. Discovery uses the same lookup, so a service listed in another case gets the same description. Co-authored-by: Isaac <no-reply@databricks.com>
…d-in answer An authenticate call answered "Already signed in" sent tools/list_changed, so a service that still challenged the caller, for example after its grant was revoked, started a loop of re-lists, challenges and the same answer. The answer no longer asks the agent to list again, since nothing changed. A challenge that follows it makes the next authenticate delete the caller's credential first, so the sign-in waits for a new one. Co-authored-by: Isaac <no-reply@databricks.com>
…startup read The sign-in client authenticated outside the guard that stops the proxy on a token refresh failure, so such a failure during authenticate reported the raw error without the login hint and left the proxy running. A failed sign-in or startup call is now followed by one authentication through the guard. The startup read of a server with no install record is bounded as the list --check checks are, so it does not wait for the SDK's retry budget, and its auth failure exits 2 with the login hint, as the other credential failures do. Co-authored-by: Isaac <no-reply@databricks.com>
An authenticate call waits up to five minutes, longer than the request timeout of common MCP clients, and sent nothing until it answered. A call that carries a progress token now gets a notifications/progress message on each poll of the sign-in. The sign-in helper takes its options as the mcp package's, so the poll callback reaches it. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
When the credential is already active, for example from a sign-in in a terminal, the agent's cached tool list still shows only authenticate, so the answer is followed by tools/list_changed. A challenge after that answer still makes the next authenticate start a new sign-in, so the two cannot loop. Co-authored-by: Isaac <no-reply@databricks.com>
The URL elicitation and HTTP 401 challenge modes of the fake were used only by the fake's own tests, and the proxy's tests cover -32042 and 401 with their own handlers. The fake's credential check now matches the service name without case, as its service lookup does. Co-authored-by: Isaac <no-reply@databricks.com>
A bare 401, or one whose body is a Databricks authentication error, means the Databricks token was rejected, which a provider sign-in does not fix. Such an answer now gets the login hint, as the refresh guard words it, and no authenticate tool. A 401 with a sign-in body (JSON-RPC -32006, or a sign-in URL elicitation) is still a challenge. Before, a rejected token after an "Already signed in" answer restarted the provider sign-in and deleted a valid credential. Co-authored-by: Isaac <no-reply@databricks.com>
…redential A server with no install record used to exit when its startup read of the service failed, even for a service that never needs sign-in. The read now fails open: the serve starts with the sign-in handling on, which acts only on a challenge that names the service, and one line on stderr says the read failed. A failure of the credential still exits 2 with the login hint. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
A token failure that is not a rejection now fails only its request. The tests that expect the proxy to stop use a rejected token, and the two proxy tests that keep serving use a transient failure with the bridge's guard. Co-authored-by: Isaac <no-reply@databricks.com>
… proxy A token failure that is not a rejection fails only the sign-in's request, with the transient message, and the guard does not stop the proxy. Co-authored-by: Isaac <no-reply@databricks.com>
The sign-in hooks saw every upstream message of an exchange, including event-stream notifications, and counted any 2xx answer as success. A progress notification or a JSON-RPC error on HTTP 200 could therefore end the "Already signed in" state or the authenticate tool. Only the response to the request is read now, and only a response with a result and no error counts as success. Co-authored-by: Isaac <no-reply@databricks.com>
A challenge after "Already signed in" asks for a restart of the sign-in. A success from the service before the next authenticate left that restart pending, so a much later, unrelated challenge could delete a working credential. A success now ends the restart too. Co-authored-by: Isaac <no-reply@databricks.com>
A challenged tools/call tells the agent to call authenticate, but the agent's next tools/list could pass unchallenged, which removed the tool from the list and left the agent without it. While the last challenge came from a call, a passing list now carries the authenticate tool, until a sign-in ends or a call passes. Co-authored-by: Isaac <no-reply@databricks.com>
A poll of a sign-in can fire after the client cancelled the call. Its progress notification was still written for the cancelled request. The progress is now checked against the call's context before it is sent. Co-authored-by: Isaac <no-reply@databricks.com>
The progress of a sign-in named the page only when the attempt did not open it, so a client that hides stderr, or a browser that did not open, never saw where to finish until the wait timed out. The page is now named on every poll. Co-authored-by: Isaac <no-reply@databricks.com>
The proxy rebuilt the timeout text that the sign-in already returns, from fields that only it used. The timeout now carries the last poll's failure alone, and the proxy answers with the error's text. Co-authored-by: Isaac <no-reply@databricks.com>
A package-level regexp compiles at init. The pattern now follows the package's other patterns and compiles on first use. Co-authored-by: Isaac <no-reply@databricks.com>
signIn overwrote the poll and timeout of the options it was given, so the fields a caller set for them were silently ignored. Callers now pass the profile, the page opener and the poll callback, and signIn builds the sign-in options itself. Co-authored-by: Isaac <no-reply@databricks.com>
The sign-in of the authenticate tool returned the errors of the credential delete, the workspace ID read and the wait after a restart as they came. Each names the profile and the login command now, as the login command does. A restart that removes the caller's sign-in says so on the sign-in's stderr before the page. The wait for a restart's delete is bounded as a whole. Its credential reads could hold it for the SDK's retry budget, which is minutes. Co-authored-by: Isaac <no-reply@databricks.com>
A signal during the startup read printed a read failure and served the proxy on a cancelled context. The start now ends as a shutdown does. The sign-in client was built and its profile resolved again on every serve, even for a server whose record says it needs no sign-in, so it is built only when the start reads the service or the record says it needs sign-in. Co-authored-by: Isaac <no-reply@databricks.com>
The passthrough scenario created its service twice, and the authenticate scenario sent its setup output to /dev/null. The duplicate create is gone, and the setup commands write to LOG files. The retry waits still discard their output, since it repeats the captured stdout on every poll. Co-authored-by: Isaac <no-reply@databricks.com>
resultResponse lived beside the sign-in code and built its own envelope, while errorResponse had a second one. Both are in messages.go over one response type, with the version as a constant. Co-authored-by: Isaac <no-reply@databricks.com>
The two tests checked the same stop with the same server and guard, one per call. They are one table test now. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
…p read A rejected token cancels serve's context, so the startup read returned the cancellation and serve exited 0 with no line. The refresh failure is now checked first, and the auth failure is returned ahead of the cancellation. Co-authored-by: Isaac <no-reply@databricks.com>
Only the responses to tools/list and tools/call are decoded for a challenge or a success, and an HTTP 401 is still read. A response whose result or error has a shape the checks do not read is neither, so it passes through instead of failing the request. Co-authored-by: Isaac <no-reply@databricks.com>
A tools/list that passes after a challenged tool call no longer ends the state of that call's "Already signed in" answer, so a later challenge to a call starts a new sign-in. An isError result without a sign-in URL is neither a challenge nor a success and changes no state. Co-authored-by: Isaac <no-reply@databricks.com>
…ed in The proxy no longer deletes a credential. When the service challenges again after an "Already signed in" answer, the next authenticate answers with an error that names the logout command, and clears the state, so the authenticate after that runs the sign-in as usual. The restart wait, its notice and the restart tests are removed. Co-authored-by: Isaac <no-reply@databricks.com>
…thenticate tool A paginated list after a challenged tools/call gets the authenticate tool on its first page only. A list that already has a tool named authenticate keeps it alone, and calls to that tool go to the service. Co-authored-by: Isaac <no-reply@databricks.com>
…cancel The browser command of serve gets no stdin, since serve's stdin is the protocol stream, and runs with a context that the authenticate request's cancellation does not reach, so a cancelled call does not kill the browser. Open and OpenOutputToStderr keep their behavior. Co-authored-by: Isaac <no-reply@databricks.com>
…tdin Co-authored-by: Isaac <no-reply@databricks.com>
mcp.SignIn opens the page only when Open is set, as it already does for OnPoll, so serve no longer passes a no-op opener. The testserver's sign-in answer is a plain function again, the proxy's list uses the tools key constant, and the bridge's loop keeps its variable name. The sign-in test that sets no progress callback now sets one. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
simonfaltum
force-pushed
the
aitools-mcp/07-login
branch
from
November 13, 2026 15:05
a08bc42 to
4a8c08c
Compare
simonfaltum
force-pushed
the
aitools-mcp/08-challenge
branch
from
November 13, 2026 15:05
1abf7d0 to
6b41591
Compare
This branch has not been deployed
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
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.
This pull request is part of a stack that adds the hidden
databricks aitools mcpcommand group, on top of #7551. A service that needs provider sign-in rejectstools/listuntil the user signs in, so the agent has no tool to start it. The proxy now offers oneauthenticatetool that runs the sign-in.Changes
-32006, a-32042sign-in URL on the profile's host, or an error result naming that URL) makestools/listreturn onlyauthenticate. A 401 is a challenge only with such a body; any other 401 gets thedatabricks auth loginhint.tools/callnamesauthenticateand the login command. Iftools/liststill passes,authenticateis added to its first page until a call succeeds or the sign-in completes.authenticaterunslogin's sign-in, sends progress naming the page, and sendstools/list_changedwhen signed in. A timeout names the URL, the last failed check and the login command.authenticateanswers with themcp logoutcommand to run first.libs/browser.OpenOutputToStderrWithoutStdin.mcp listlabels upper-case names too.Why
Discovery fails before sign-in, so the agent can only start a sign-in through a tool that exists before it. A rejected Databricks token is not fixed by a provider sign-in, so it gets the login hint instead.
Decisions to review
mcp logout.Tests
challenge_test.gocovers the challenge bodies, 401s, which answers end the sign-in state, call-only challenges, the logout answer, page reopening, queued and cancelled calls, and progress.mcp_serve_test.gocovers the guard on sign-in requests, the startup read (rejected token, signal) and the opener's context.libs/browsercovers the no-stdin opener.serve-authenticate-toolruns the stdio flow withBROWSER=none.-raceon the touched packages, gotestsum withBROWSERunset andBROWSER=none,./task test-exp-aitools, the cmd/aitools acceptance tests,GOOS=windows go vet,./task lint-q,./task fmt-q,./task checks.Not tested: a live
-32042challenge, and a real browser.This pull request and its description were written by Isaac.