Repository navigation
Conversation
… credentials briefly Request bodies are buffered with a 32 MiB cap (413 body_too_large beyond it) so a request can be sent again; responses stream as they arrive. Waiting for the gateway's response headers is limited to 600 s (504 gateway_timeout); the body has no total limit, a long completion is normal, and a non-streaming completion sends nothing until it is done, so the header limit bounds it too. There is no idle-read timeout on the body; the client owns the connection and dropping it cancels upstream. Controller-issued credentials are cached in the listener for at most 60 s, and never past their own expiry margin (30 s): revocation is enforced at issuance and the gateway validates tokens statelessly, so the cache TTL (counted from the gateway's first answer with the credential, which follows the header wait) bounds how long a revoked device keeps using the proxy; a token the controller has issued stays valid at the gateway until it expires whether or not the daemon caches it. Entries are keyed by client id, gateway URL and audience, tagged with the enrolled device id and checked against identity.json on every hit (device id only, read with identity::load_device_id; the secret store is not opened), so a logout or re-enrollment never reuses an old token. Deadlines are checked on both the monotonic and the wall clock. The lock is never held across a fetch. A fetch runs on its own task, so a client disconnect or the request's wait does not cancel it half-way (an OAuth refresh finishes and is saved). The controller connection and call inside the fetch are bounded to 15 s (remote::llm_gateway_credential; this bound also applies to the local API and CLI credential paths, which share the function), so a stalled controller cannot accumulate open calls and the detached fetch ends by itself on that side; the OAuth refresh ahead of it (oidc.rs, existing code) has no bound of its own and is serialised by a mutex, so a stalled identity provider can still leave detached fetches waiting. The request waits at most 20 s for the fetch (agentdesktop_credential on timeout). An OIDC acquisition is not bounded since it may wait for a browser login. A fetched credential is put into the cache only after the gateway has answered with something other than 401. The worst case for one request is the header limit (600 s), a fetch (20 s), and a second header limit. OIDC credentials never go through the cache; the secret store already is one. When the gateway answers 401, the rejected credential came from the cache and no client token was forwarded in x-llm-token, the entry is dropped (only if it still holds that credential) and the request is sent once more with a freshly fetched credential; this covers a token that expired between fetch and use. A freshly fetched credential that is rejected, an OIDC credential, and a request that carried a client token are not retried: the second attempt could not differ, or the 401 may be about the client token. In the client-token case the cached gateway credential is kept (dropping it per 401 would turn a bad client token into a controller fetch per request); the README says so. Tests: cache device tagging / TTL cap / conditional invalidation / expiry margin, no retry for uncached credentials with the buffered body delivered once, body over the cap, and an end-to-end run with a pre-seeded cache and a fake identity.json (serve_with_cache): an accepted cached credential is used and kept, a rejected one is dropped and the retry fails with agentdesktop_credential because no controller exists, each body reaching the gateway exactly once per attempt; a client-token 401 on the prefix-less route is not retried and keeps the cached credential. The insert-only-after-non-401 gate is not covered by a test (it needs a controller stub). The cap is 64 KiB under cfg(test). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner
Author
|
Superseded: landed upstream in agentdesktop-dev#134 (local LLM proxy). |
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.
Stacked on this fork so the diff is this PR only. James gets one integration PR from
copilot/integration(the tip of the stack) when the series is ready.Third listener change, stacked on #4 (includes #2, #3, #4 until they merge).
body_too_large), so a request can be sent again; responses stream as they arrive. Response headers must arrive within 600 s (504gateway_timeout; a non-streaming completion sends nothing until it is done, so this bounds it too); no total limit on the body, no idle-read timeout (the client owns the connection, dropping it cancels upstream).identity.json(identity::load_device_id, no secret-store access) on every hit, so a logout or re-enrollment never reuses an old token. A fetched credential enters the cache only after the gateway has answered with something other than 401. OIDC credentials are not cached.remote::llm_gateway_credentialthe controller connection and call are bounded to 15 s (this also bounds the local API and CLI credential paths, which share the function); the proxy waits at most 20 s (agentdesktop_credential). The OAuth refresh ahead of the controller call (existing code inoidc.rs) has no bound of its own; noted, not changed here.githubOAuth.source: requestshapes are not retried; in the client-token case the cached gateway credential is kept, because dropping it per 401 would turn a bad client token into a controller fetch per request. Worst case for one request: 600 s + 20 s + 600 s.Review process. Frozen acceptance spec, three review lenses on two models, refutation, delta re-reviews until two passes without majors. Folded in over four versions: global lock across an untimed fetch; cache outliving logout; a timeout that cancelled an in-flight refresh; a detached fetch with no bound of its own; a rejected credential that could land in the cache; the retry firing on a client-token 401; device-id lookup that opened the secret store; README wording on the revocation window.
Checks: agent 115 + core 18 tests, clippy
-D warnings, fmt. Tests: cache tagging / TTL cap / conditional invalidation / margin; uncached 401 not retried with the body delivered once; body over the cap; end-to-end with a pre-seeded cache and a fakeidentity.json(serve_with_cacheseam): accepted cached credential used and kept, rejected one dropped and the retry fails withagentdesktop_credential(no controller in the suite); client-token 401 not retried and the cache kept (mutation-checked). Not covered by a test: the insert-only-after-non-401 gate (needs a controller stub).Lab, Linux VM, user mode, independent run against the stacked build: after a daemon restart, 5 requests on
/copilot-cliwithin 60 s → controller issuances +1, then 3 on/vscode-copilot→ +1 (separate client id);POST /v1/logout→ the next proxied request fails in 2 ms withagentdesktop_credential("device is not enrolled"); re-enrollment with re-paired client files → Copilot CLI 1.0.88 answers; 33554433-byte body → 413body_too_largein 26 ms; SSE completioncontent-type: text/event-stream, first byte at 0.56 s of 1.58 s total, 123data:lines ending in[DONE]. Binary under test was the previous revision of this branch (a2920fe), which differs from the head only by comment, README and one error-message wording.🤖 Generated with Claude Code