diff --git a/AGENTS.md b/AGENTS.md index 6ef75f24..aad60689 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 4f7ac8af..69dac0e2 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -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 diff --git a/src/hide-claude-auth.ts b/src/hide-claude-auth.ts index 2fd51583..cd7e9323 100644 --- a/src/hide-claude-auth.ts +++ b/src/hide-claude-auth.ts @@ -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"; diff --git a/src/tests/providers.test.ts b/src/tests/providers.test.ts index af7b8b22..9a4559e9 100644 --- a/src/tests/providers.test.ts +++ b/src/tests/providers.test.ts @@ -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(); + }); });