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); - } -}