From d2b3b2123901fcea48d3e7572596ed227c4c5628 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Thu, 9 Apr 2026 22:40:23 +1000 Subject: [PATCH] Extract AnthropicClient from AnthropicAgent First step of the SDK refactor series tracked in #232. Splitting the refactor into small, independently reviewable PRs so each step can land (or be reverted) on its own. Before this change, AnthropicAgent's constructor did four things: it resolved auth, wired up the token-refreshing HTTP client, wrapped that client in an AnthropicMessageStreamer, and held the Conversation. Only the last one is actually an agent concern. Auth, transport, and stream protocol are a transport concern. This extracts the first three into a new private AnthropicClient that extends IMessageStreamer directly. The previous AnthropicMessageStreamer wrapper class was a one-method passthrough, so it is removed rather than kept around as a second layer. AnthropicAgent now holds an IMessageStreamer (the abstract type), not the concrete client. Composition over inheritance. The constructor builds an AnthropicClient internally and stores it as the streamer. This preserves the FakeMessageStreamer contract used by AgentRun.spec.ts without touching any test imports. IMessageStreamer stays in MessageStreamer.ts for this PR. Moving it into interfaces.ts is a separate filename churn I did not want mixed into this commit. No public API or runtime behaviour changes. The agent's external shape is identical; all construction paths go through the same code. Verification: - pnpm --filter @shellicar/claude-sdk type-check: clean - pnpm test: 981/981 passing (core 5, sdk 78, sdk-tools 233, sdk-cli 426, cli 239) - pnpm build: 5/5 packages - pnpm biome check --diagnostic-level=error: 319 files, no fixes needed - changelog entry validated Step 1 of 6 for #232. --- .claude/sessions/2026-04-09.md | 122 ++++++++++++++++++ packages/claude-sdk/changes.jsonl | 1 + .../claude-sdk/src/private/AnthropicAgent.ts | 18 +-- .../claude-sdk/src/private/AnthropicClient.ts | 43 ++++++ .../claude-sdk/src/private/MessageStreamer.ts | 13 -- 5 files changed, 171 insertions(+), 26 deletions(-) create mode 100644 packages/claude-sdk/src/private/AnthropicClient.ts diff --git a/.claude/sessions/2026-04-09.md b/.claude/sessions/2026-04-09.md index e5d079d..d00be16 100644 --- a/.claude/sessions/2026-04-09.md +++ b/.claude/sessions/2026-04-09.md @@ -386,3 +386,125 @@ Picked `cloneForRequest()` over the other candidates (`snapshotForRequest()`, `c Held the line on PR 1's narrow scope. The following are all deferred to later PRs and still accurate as planned: `AnthropicClient` extraction, `AnthropicAgent` reshape with `configure(partial)` / `run({ systemReminder? })`, deletion of `ConversationStore` from the SDK, removal of `historyFile` from `AnthropicAgentOptions`, CLI-side persistence layer, collapse of `apps/claude-sdk-cli/src/runAgent.ts`, session management, audit path derivation. What's notable: the refactor turned out to be smaller than expected. The `AgentRun` call-site change was a single line; `RequestBuilder` simplifications were mostly deletion; all the behavioural weight sat in `Conversation.ts`. That is itself evidence the old design was wrong — the cache-control spread dance and the on-load trim and the compaction clear were all symptoms of the same confused ownership boundary. Separating stored state from API view eliminates all three at once. + +--- + +## Post-PR-231: plan resequenced into issue #232 + +Context: PR #231 shipped (squash-merge as `31de154` on `main`). After it landed, the +"PR 2 = one big reshape" framing captured in the "SDK refactor planning" section +above was superseded by issue #232 +(https://github.com/shellicar/claude-cli/issues/232). + +Issue #232 breaks the remaining work into six numbered steps, each explicitly +required to land as its own PR so reviews stay small and the refactor can pause at +any clean state: + +1. Extract `AnthropicClient` from `AnthropicAgent` (auth, token refresh, HTTP transport). +2. Reshape `AnthropicAgent` public API: `configure(partial)` + `run({ systemReminder? })`. +3. Remove `ConversationStore` from the SDK; drop `historyFile` from `AnthropicAgentOptions`. +4. CLI grows its own file-backed persistence helper. +5. Collapse `apps/claude-sdk-cli/src/runAgent.ts`. +6. First-class audit output (separate audit writer from the history file). + +**Issue #232 is the authoritative plan going forward.** The "PR 2 = one PR" note +above is a historical snapshot of the pre-work plan and should not be used to drive +new work. The log was not updated immediately after PR 231 landed, which is the +gap this section closes. + +### Branch naming convention (issue #232 series) + +Branches in this series use `feature/sdk-refactor-N-` where N is the step +number from issue #232. This keeps the series visibly grouped and makes ordering +obvious from the branch list alone. Step 1 will be +`feature/sdk-refactor-1-extract-client`. + +### Why record this now + +The session log is how future sessions (and this one, after compaction) reconstruct +current state. Leaving the "PR 2 = one PR" framing uncontested means the next +session reads it as current intent and either works against the wrong scope or has +to re-derive the resequencing from scratch. Cheaper to write this paragraph once +than to pay that cost repeatedly. + +--- + +## Issue #232 step 1: Extract `AnthropicClient` from `AnthropicAgent` + +Branch: `feature/sdk-refactor-1-extract-client` (cut from `main` after PR #231 merged). + +### What step 1 is + +The first of six PRs tracked by issue #232. Move auth, token refresh, and HTTP +transport out of `AnthropicAgent` into a dedicated `AnthropicClient` class. +`AnthropicAgent` becomes a thin composer that holds a client and a conversation. + +### Decisions locked in + +**Merge the paper-thin streamer into the client.** `AnthropicClient extends +IMessageStreamer`. The previous `AnthropicMessageStreamer` class (which just +delegated `.stream()` to `Anthropic.beta.messages.stream`) goes away. One class, +one identity: the client is the thing you stream from. + +**`AnthropicAgent` depends on `IMessageStreamer`, not `AnthropicClient`.** The +agent never sees the concrete class. Composition, not inheritance. This preserves +the test contract (`FakeMessageStreamer extends IMessageStreamer` in +`AgentRun.spec.ts`) without change. + +**`IMessageStreamer` stays in `MessageStreamer.ts` for this PR.** The file keeps +only the abstract class; the concrete class is removed. No filename churn, no +test import changes. Moving it into a shared `interfaces.ts` (Stephen's usual +pattern) is acknowledged but deferred as a separate cleanup. + +**Interface naming is deferred.** `IMessageStreamer` could use a better name but +that is a separate concern, not step 1's scope. + +### Steps (planned) + +1. Create `packages/claude-sdk/src/private/AnthropicClient.ts`. Extends + `IMessageStreamer`. Constructor takes `{ authToken, logger }`. Internally + builds `defaultHeaders` (user-agent with version), constructs + `TokenRefreshingAnthropic` with `customFetch(logger)`, holds the raw + `Anthropic` instance. `stream(body, options)` delegates to + `#raw.beta.messages.stream(body, options)`. +2. Remove `AnthropicMessageStreamer` from `MessageStreamer.ts`. Leave only the + abstract `IMessageStreamer` in that file. +3. Update `AnthropicAgent.ts`: drop imports of `TokenRefreshingAnthropic`, + `customFetch`, `AnthropicMessageStreamer`, `versionJson`. Add + `AnthropicClient` import. Replace inline construction with + `new AnthropicClient({ authToken, logger })`. Field becomes `#client` typed + as `IMessageStreamer` (composition over inheritance). `runAgent` passes + `this.#client` where `this.#streamer` used to go; `AgentRun`'s signature does + not change. +4. `pnpm --filter @shellicar/claude-sdk type-check` clean. +5. `pnpm test` all workspace suites green. +6. `pnpm build` all packages build. +7. `pnpm biome check --diagnostic-level=error` clean. +8. Add a `changed` entry to `packages/claude-sdk/changes.jsonl`. +9. Validate with `pnpm tsx scripts/src/validate-changes.ts + packages/claude-sdk/changes.jsonl`. +10. Single commit: session log edits + the three source changes + changelog + entry. Message explains the why. +11. Push and open PR. Labels: `enhancement`, `pkg: claude-sdk`. Milestone + `1.0`. Reviewer `bananabot9000`. Assignee `shellicar`. Auto-merge squash. + Body references #232, notes step 1 of 6. + +### What step 1 does NOT touch + +- Public API (`createAnthropicAgent`, `IAnthropicAgent`, `AnthropicAgentOptions`, + `RunAgentQuery`, `RunAgentResult`): unchanged. +- `AgentRun`, `Conversation`, `ConversationStore`, `RequestBuilder`, + `MessageStream`: untouched. +- CLI (`apps/claude-sdk-cli`): untouched. +- `historyFile` removal from `AnthropicAgentOptions`: step 3, not step 1. +- `configure(partial)` / `run({ systemReminder? })` API reshape: step 2, not + step 1. +- Interface naming and relocation: out of scope, flagged for later. + +### Why record the plan here + +If context compacts mid-PR, the next session needs enough to resume without +re-deriving decisions. This block is the resumable artifact: branch name, target, +shape decisions (with rationale), step list, and explicit non-goals. The earlier +lesson from not updating the log after PR 231 landed is the reason this is being +written before code rather than after. diff --git a/packages/claude-sdk/changes.jsonl b/packages/claude-sdk/changes.jsonl index f5272d9..370ebc0 100644 --- a/packages/claude-sdk/changes.jsonl +++ b/packages/claude-sdk/changes.jsonl @@ -1,2 +1,3 @@ {"description":"Package now publishes CJS alongside ESM with working sourcemaps","category":"fixed"} {"description":"Conversation retains full message history across compaction; adds internal `cloneForRequest()` that returns a deep-cloned post-compaction slice for API requests","category":"changed"} +{"description":"Extract `AnthropicClient` from `AnthropicAgent`: auth, token refresh, and HTTP transport now live in a dedicated private class. `AnthropicAgent` becomes a thin composer that holds a client and a conversation. The previous `AnthropicMessageStreamer` wrapper is removed; `AnthropicClient` extends `IMessageStreamer` directly.","category":"changed"} diff --git a/packages/claude-sdk/src/private/AnthropicAgent.ts b/packages/claude-sdk/src/private/AnthropicAgent.ts index 72ee9ad..5267c02 100644 --- a/packages/claude-sdk/src/private/AnthropicAgent.ts +++ b/packages/claude-sdk/src/private/AnthropicAgent.ts @@ -1,16 +1,14 @@ import type { BetaMessageParam } from '@anthropic-ai/sdk/resources/beta.js'; -import versionJson from '@shellicar/build-version/version'; import { IAnthropicAgent } from '../public/interfaces'; import type { AnthropicAgentOptions, ILogger, RunAgentQuery, RunAgentResult } from '../public/types'; import { AgentChannelFactory } from './AgentChannel'; import { AgentRun } from './AgentRun'; +import { AnthropicClient } from './AnthropicClient'; import { ConversationStore } from './ConversationStore'; -import { customFetch } from './http/customFetch'; -import { TokenRefreshingAnthropic } from './http/TokenRefreshingAnthropic'; -import { AnthropicMessageStreamer } from './MessageStreamer'; +import type { IMessageStreamer } from './MessageStreamer'; export class AnthropicAgent extends IAnthropicAgent { - readonly #streamer: AnthropicMessageStreamer; + readonly #client: IMessageStreamer; readonly #channelFactory: AgentChannelFactory; readonly #logger: ILogger | undefined; readonly #history: ConversationStore; @@ -18,22 +16,16 @@ export class AnthropicAgent extends IAnthropicAgent { public constructor(options: AnthropicAgentOptions) { super(); this.#logger = options.logger; - const defaultHeaders = { - 'user-agent': `@shellicar/claude-sdk/${versionJson.version}`, - }; - const client = new TokenRefreshingAnthropic({ + this.#client = new AnthropicClient({ authToken: options.authToken, - fetch: customFetch(options.logger), logger: options.logger, - defaultHeaders, }); - this.#streamer = new AnthropicMessageStreamer(client); this.#channelFactory = new AgentChannelFactory(); this.#history = new ConversationStore(options.historyFile); } public runAgent(options: RunAgentQuery): RunAgentResult { - const run = new AgentRun(this.#streamer, this.#channelFactory, this.#logger, options, this.#history); + const run = new AgentRun(this.#client, this.#channelFactory, this.#logger, options, this.#history); return { port: run.port, done: run.execute() }; } diff --git a/packages/claude-sdk/src/private/AnthropicClient.ts b/packages/claude-sdk/src/private/AnthropicClient.ts new file mode 100644 index 0000000..f552d3f --- /dev/null +++ b/packages/claude-sdk/src/private/AnthropicClient.ts @@ -0,0 +1,43 @@ +import type { Anthropic } from '@anthropic-ai/sdk'; +import type { BetaMessageStreamParams } from '@anthropic-ai/sdk/resources/beta/messages.js'; +import type { BetaRawMessageStreamEvent } from '@anthropic-ai/sdk/resources/beta.mjs'; +import versionJson from '@shellicar/build-version/version'; +import type { ILogger } from '../public/types'; +import { customFetch } from './http/customFetch'; +import { TokenRefreshingAnthropic } from './http/TokenRefreshingAnthropic'; +import { IMessageStreamer } from './MessageStreamer'; + +export type AnthropicClientOptions = { + authToken: () => Promise; + logger?: ILogger; +}; + +/** + * Anthropic API client: owns auth, token refresh, HTTP transport, and streaming. + * + * Extracted from `AnthropicAgent` so the agent can compose an API client with a + * conversation (first step of the refactor series in issue #232). Extends + * `IMessageStreamer` directly because streaming is this client's primary + * (currently only) job; the previous paper-thin `AnthropicMessageStreamer` + * wrapper has been removed as part of the same change. + */ +export class AnthropicClient extends IMessageStreamer { + readonly #raw: TokenRefreshingAnthropic; + + public constructor(options: AnthropicClientOptions) { + super(); + const defaultHeaders = { + 'user-agent': `@shellicar/claude-sdk/${versionJson.version}`, + }; + this.#raw = new TokenRefreshingAnthropic({ + authToken: options.authToken, + fetch: customFetch(options.logger), + logger: options.logger, + defaultHeaders, + }); + } + + public stream(body: BetaMessageStreamParams, options: Anthropic.RequestOptions): AsyncIterable { + return this.#raw.beta.messages.stream(body, options); + } +} diff --git a/packages/claude-sdk/src/private/MessageStreamer.ts b/packages/claude-sdk/src/private/MessageStreamer.ts index e96a566..260b017 100644 --- a/packages/claude-sdk/src/private/MessageStreamer.ts +++ b/packages/claude-sdk/src/private/MessageStreamer.ts @@ -5,16 +5,3 @@ import type { BetaRawMessageStreamEvent } from '@anthropic-ai/sdk/resources/beta export abstract class IMessageStreamer { public abstract stream(body: BetaMessageStreamParams, options: Anthropic.RequestOptions): AsyncIterable; } - -export class AnthropicMessageStreamer extends IMessageStreamer { - readonly #client: Anthropic; - - public constructor(client: Anthropic) { - super(); - this.#client = client; - } - - public stream(body: BetaMessageStreamParams, options: Anthropic.RequestOptions): AsyncIterable { - return this.#client.beta.messages.stream(body, options); - } -}