mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix: recreate live sessions on logout when it clears a provider or gateway override (#1278)
## What ACP `logout` cleared `providerConfig` and the gateway credentials but left live queries running. Provider and gateway routing is baked into each query's env, so open sessions kept using the old route after logout. `logout` now recreates every live session through the same path as `providers/disable` (`enqueueProviderUpdate`), but only when a provider or gateway override was active. It waits for active turns, then resumes each session under its id with native routing. This also runs when `claude auth logout` fails, because the override is already cleared by then. A plain CLI-login logout does not touch live sessions: the running CLI picks up the logout by itself. ## Behavior change With an override active, `logout` now waits for running turns before it returns, like `providers/disable`. Under `--hide-claude-auth`, a session that cannot be recreated signed out is dropped, as with `providers/disable`.
This commit is contained in:
@@ -16,6 +16,14 @@ npm run format # prettier --write
|
||||
CI runs `format:check`, `lint`, `build` and `test:run`. Run `npm run check` before
|
||||
opening a PR.
|
||||
|
||||
## Code comments
|
||||
|
||||
Keep comments straightforward, concise, and concrete. Say what the code does and
|
||||
why, in plain terms: name the actual state, call, or failure involved instead of
|
||||
alluding to it. Prefer a short concrete example over an abstract description.
|
||||
Don't use vague phrases like "keeps it honest" or "the residual case" that make
|
||||
the reader reconstruct the meaning.
|
||||
|
||||
## Pull requests
|
||||
|
||||
Squash merges use the PR title as the commit subject, and release-please parses it
|
||||
|
||||
@@ -3286,6 +3286,7 @@ export class ClaudeAcpAgent {
|
||||
// provider routing set via `providers/set`. Neither touches the on-disk
|
||||
// credential store, so dropping these references is the whole logout for
|
||||
// those paths.
|
||||
const hadRoutingOverride = this.resolveProviderConfig() !== null;
|
||||
this.gatewayAuthRequest = undefined;
|
||||
this.providerConfig = undefined;
|
||||
// Any probe already running read the pre-logout world; the bump makes its
|
||||
@@ -3301,8 +3302,8 @@ export class ClaudeAcpAgent {
|
||||
// For the Claude/Console login methods the credentials live in the native
|
||||
// CLI's store (keychain or config dir), which only the binary can clear.
|
||||
// `claude auth logout` is non-interactive and idempotent.
|
||||
const cliPath = await claudeCliPath();
|
||||
try {
|
||||
const cliPath = await claudeCliPath();
|
||||
await execFileAsync(cliPath, ["auth", "logout"]);
|
||||
} catch (error) {
|
||||
const stderr =
|
||||
@@ -3313,6 +3314,16 @@ export class ClaudeAcpAgent {
|
||||
{ stderr: stderr || undefined },
|
||||
`claude auth logout failed: ${error instanceof Error ? error.message : String(error)}`,
|
||||
);
|
||||
} finally {
|
||||
// The cleared provider or gateway routing is baked into the env of every
|
||||
// live query, so recreate them the way `providers/disable` does: after
|
||||
// the active turns settle, resuming each session under its id. Done even
|
||||
// when the CLI logout failed, because the override is already gone. A
|
||||
// plain CLI login needs nothing here: the running CLI picks up the
|
||||
// logout by itself.
|
||||
if (hadRoutingOverride) {
|
||||
await this.enqueueProviderUpdate(undefined);
|
||||
}
|
||||
}
|
||||
|
||||
// Re-read the store rather than assuming "none": an API key from the env
|
||||
|
||||
@@ -1,34 +1,35 @@
|
||||
/**
|
||||
* `--hide-claude-auth`: this integration must never bill a claude.ai
|
||||
* subscription. The flag hides the claude.ai login method from `initialize`
|
||||
* and makes the agent behave like a logged-out CLI whenever a subscription
|
||||
* would pay for a turn. `acp-agent.ts` decides *when* the guard applies (flag
|
||||
* on, no provider override) and calls in here for the *what*.
|
||||
* subscription. Under the flag the agent hides the claude.ai login method from
|
||||
* `initialize`, and refuses with `authRequired` every session and every turn
|
||||
* that a subscription would pay for. `acp-agent.ts` decides when the guard
|
||||
* applies (flag on, no provider override); this module decides what it refuses.
|
||||
*
|
||||
* The account the CLI reports at `initialize` is the source of truth. The SDK
|
||||
* memoizes it, so it is a constant for the life of a query, and two layers
|
||||
* keep it honest:
|
||||
* The guard judges the `AccountInfo` that the SDK caches at `initialize`. That
|
||||
* cache never changes during the life of a query, but the CLI itself picks up
|
||||
* a login or logout made elsewhere. So the cached account can go stale: a
|
||||
* session started on an API key keeps passing the guard after the user swaps
|
||||
* the key for a claude.ai login, and the subscription pays.
|
||||
*
|
||||
* 1. Session creation refuses an account that holds no credential this
|
||||
* integration accepts. A logged-out session never exists, so the client's
|
||||
* sign-in always leads to a new session with a fresh `initialize`. The
|
||||
* per-turn guard repeats the check on the same cached account, which costs
|
||||
* microseconds and covers a session created under a provider override that
|
||||
* `providers/disable` later removed.
|
||||
* 2. A sign-out during the session ends the query. The next turn recreates it,
|
||||
* so the next `initialize` reports the real account and layer 1 judges it.
|
||||
* The recreation resumes the stored conversation. When the CLI never wrote
|
||||
* one, because the first turn was the one that signed out, the recreation
|
||||
* starts a fresh query under the same session id instead.
|
||||
* `authStatus` cannot fix this: it only reports to the client, and the guard
|
||||
* does not read it. The agent therefore recreates the query whenever the
|
||||
* account may have changed, and the new `initialize` gives the guard a current
|
||||
* account. Two signals trigger it:
|
||||
*
|
||||
* A file-based credential replaced by a subscription between two turns is seen
|
||||
* by the CLI probe the agent fires at the START of every user prompt. That read
|
||||
* is never awaited, so it usually lands mid-turn; all it does there is mark the
|
||||
* session. The guard consumes the mark at the turn boundary — the next prompt
|
||||
* recreates the query and layer 1 judges the account the new `initialize`
|
||||
* reports — so a probe never interrupts a running turn. A swap that happens
|
||||
* after the last probe stays unseen until the next prompt reads the store
|
||||
* again. Closing that window would cost a blocking read per turn.
|
||||
* - A turn fails with `auth_required`, which means a sign-out. The query is
|
||||
* closed at once and recreated on the next turn.
|
||||
* - The `claude auth status` check that runs at the start of every prompt (the
|
||||
* same check that feeds `authStatus`) reports a different kind of identity
|
||||
* than the session started with. The session is flagged and recreated at the
|
||||
* next turn boundary, so a running turn is never interrupted.
|
||||
*
|
||||
* The recreation resumes the conversation, or starts a new one under the same
|
||||
* session id when the CLI never saved it. Session creation also refuses an
|
||||
* account with no credential this integration accepts. So a signed-out session
|
||||
* never exists, and a sign-in always leads to a fresh `initialize`.
|
||||
*
|
||||
* The check is not awaited, so a credential swapped after it ran stays unseen
|
||||
* until the next prompt. Awaiting it would add a CLI run to every turn.
|
||||
*/
|
||||
|
||||
import { RequestError } from "@agentclientprotocol/sdk";
|
||||
|
||||
@@ -527,4 +527,39 @@ describe("providers", () => {
|
||||
baseUrl: "https://api.anthropic.com",
|
||||
});
|
||||
});
|
||||
|
||||
it("recreates live sessions without the cleared provider on logout", async () => {
|
||||
const [agent, mockQuery] = await createAgentMock();
|
||||
vi.stubEnv("CLAUDE_CODE_EXECUTABLE", "/bin/true");
|
||||
await agent.initialize({ protocolVersion: 1, clientCapabilities: {} });
|
||||
await agent.unstable_setProvider({
|
||||
providerId: "main",
|
||||
apiType: "anthropic",
|
||||
baseUrl: "https://provider.example",
|
||||
});
|
||||
const created = await agent.newSession({ cwd: process.cwd(), mcpServers: [] });
|
||||
const originalQuery = mockQuery.mock.results[0].value;
|
||||
|
||||
await agent.logout({});
|
||||
|
||||
expect(originalQuery.close).toHaveBeenCalledOnce();
|
||||
expect(mockQuery).toHaveBeenCalledTimes(2);
|
||||
expect(mockQuery.mock.calls[1][0].options.resume).toBe(created.sessionId);
|
||||
expect(mockQuery.mock.calls[1][0].options.env.ANTHROPIC_BASE_URL).toBe(
|
||||
process.env.ANTHROPIC_BASE_URL,
|
||||
);
|
||||
});
|
||||
|
||||
it("leaves live sessions alone on logout without a routing override", async () => {
|
||||
const [agent, mockQuery] = await createAgentMock();
|
||||
vi.stubEnv("CLAUDE_CODE_EXECUTABLE", "/bin/true");
|
||||
await agent.initialize({ protocolVersion: 1, clientCapabilities: {} });
|
||||
await agent.newSession({ cwd: process.cwd(), mcpServers: [] });
|
||||
const originalQuery = mockQuery.mock.results[0].value;
|
||||
|
||||
await agent.logout({});
|
||||
|
||||
expect(originalQuery.close).not.toHaveBeenCalled();
|
||||
expect(mockQuery).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user