OAuth route lifecycle: restart-free credential pickup (proposed series) - #77
Draft
timcharper wants to merge 4 commits into
Draft
OAuth route lifecycle: restart-free credential pickup (proposed series)#77timcharper wants to merge 4 commits into
timcharper wants to merge 4 commits into
Conversation
Documents current mount-at-startup-only behavior, the resulting restart-to-reauthorize gap, and the target design for closing it (always mount, fail fast locally with no credential, single-flight connect, disk as sole source of truth) as a stack of small incremental changes rather than one large rewrite. Also adds AGENTS.md (none existed) indexing the docs and pointing at build/format/test commands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No behavior change. Pulls the goroutine body of the startup connect loop (Start/Initialize, tool wiring, route mounting) out into a standalone function taking an already-built Client/Server pair, so it can be re-run later for a single server without repeating the whole loop or restarting the daemon. Groundwork for docs/OAUTH_LIFECYCLE.md step 3/4. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A server that fails to connect at startup (most commonly: no valid OAuth token yet) now still gets its HTTP route mounted. Requests to it get a clear 503 with the failure reason - for OAuth servers, the exact `-authorize` command to run - instead of a bare 404 that reads as "this route doesn't exist." The failure is captured once at connect time and served as a fixed response; there's no per-request network attempt or retry yet (that's the next step). Update USAGE.md's OAuth section to match: restart is still required to pick up a fresh `-authorize`, but the interim failure mode is now a clear error rather than a 404. Implements design points 1-2 / delivery step 3 in docs/OAUTH_LIFECYCLE.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds serverRoute (route.go): the handler mounted for a server that failed to connect at startup. Each request against it first does a purely local, no-network check (checkServerAuth, the same one -auth-status already performs) - if there's plausibly no usable credential yet, it fails immediately with no connection attempt, so retrying on every request to a still-unauthorized server costs nothing. If a credential now looks present, it attempts a real connect, with concurrent requests coalesced through a per-server singleflight.Group into one attempt instead of a stampede. On success it becomes a plain pass-through to the now-working handler, and this machinery is never consulted again for that server. This is the headline fix: running `-authorize` now takes effect on the very next request, no daemon restart, no re-mount (mounting still happens exactly once, at startup - only the internal ready/not-ready state changes afterward). Failed retry attempts close their half-built client so a persistently broken server doesn't leak a connection per request; shutdown now also closes any serverRoute that did connect, alongside the existing clients map. Extracted wrapHandler (middleware wrapping) out of connectAndMount so both the startup path and the retry path share it. Updates USAGE.md's OAuth section: restart is no longer needed after -authorize. Implements design point 3a / delivery step 4, and notes point 2's liveness-vs-credential distinction (no backoff yet, deliberately) in docs/OAUTH_LIFECYCLE.md. Co-Authored-By: Claude Sonnet 5 <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.
Draft PR covering the full proposed series described in
docs/OAUTH_LIFECYCLE.md: closing the gap where a downstream server that isn't OAuth-authorized yet at startup never gets its route mounted, requiring a full daemon restart after running-authorize.This is a stack of 4 commits, each independently buildable/testable, intended to land as separate PRs one at a time rather than as one big change:
docs: add OAuth/route lifecycle design notes, add AGENTS.md— the design doc this series implements, plus a repoAGENTS.md.refactor: extract per-server connect/mount into connectAndMount— no behavior change, groundwork for calling connect logic more than once.feat: always mount a server's route, fail fast instead of 404— every configured server's route is mounted at startup; a server with no valid credential yet responds503(with the exact-authorizecommand to run) instead of a bare 404.feat: lazy connect-on-request for not-yet-ready routes, no restart— a not-yet-ready route retries the connection lazily on the next request (coalesced per-server viasingleflight, so concurrent requests share one attempt), so running-authorizeis enough — no restart, no re-mount.I opened this as a draft to establish direction and get early feedback on the overall approach before splitting it up. #78 is the first commit of this series (the design doc) submitted on its own as a real, mergeable PR.
Design rationale, rejected alternatives (fsnotify, in-memory token cache, crash-on-change, config hot-reload), and a documented known limitation (concurrent refresh-token use isn't serialized by
mcp-go, accepted as-is) are all written up in the doc itself.🤖 Generated with Claude Code