charles/claude-hooks

Fork 0

feat(agents): finish ClaudeAgentPort migration — eliminate SDK leaks #532

Closed

dev wants to merge 1 commit from dev/517 into main

dev commented

2026-04-28 10:42:46 +00:00

Collaborator

Summary

Move all @anthropic-ai/claude-agent-sdk imports behind the ClaudeAgentPort / SdkClaudeAgent boundary — agent-runner.ts, foreman.ts, steer.ts, and event-log.ts no longer import from the SDK directly
agent-runner: replace direct query() call with claudeAgent.runTask() via injected port; export PR_URL_PATTERN constant; convert MCP server map → McpServerSpec[]
foreman: buildForemanSdkOptions returns { prompt, runTaskRequest } (domain types) instead of SDK Options; toolPolicy replaces canUseTool
steer: steerInputIterator yields string (adapter handles SDKUserMessage conversion)
event-log: logSDKMessage accepts PortEvent; SDK-specific fields accessed via raw escape hatch
registry: wire SdkClaudeAgent singleton into runForemanTurn / runAgentTask
Tests: updated to fake adapter; new test covers invalid session_id retry path

Closes #517

Test plan

bun test — existing agent-runner + foreman + steer tests pass with adapter swapped for fake
New test: invalid session_id retry path covered against fake adapter
just qa (typecheck + Biome lint/format) passes clean

🤖 Generated with Claude Code

## Summary - Move all `@anthropic-ai/claude-agent-sdk` imports behind the `ClaudeAgentPort` / `SdkClaudeAgent` boundary — `agent-runner.ts`, `foreman.ts`, `steer.ts`, and `event-log.ts` no longer import from the SDK directly - `agent-runner`: replace direct `query()` call with `claudeAgent.runTask()` via injected port; export `PR_URL_PATTERN` constant; convert MCP server map → `McpServerSpec[]` - `foreman`: `buildForemanSdkOptions` returns `{ prompt, runTaskRequest }` (domain types) instead of SDK `Options`; `toolPolicy` replaces `canUseTool` - `steer`: `steerInputIterator` yields `string` (adapter handles `SDKUserMessage` conversion) - `event-log`: `logSDKMessage` accepts `PortEvent`; SDK-specific fields accessed via `raw` escape hatch - `registry`: wire `SdkClaudeAgent` singleton into `runForemanTurn` / `runAgentTask` - Tests: updated to fake adapter; new test covers invalid `session_id` retry path Closes #517 ## Test plan - [ ] `bun test` — existing agent-runner + foreman + steer tests pass with adapter swapped for fake - [ ] New test: invalid `session_id` retry path covered against fake adapter - [ ] `just qa` (typecheck + Biome lint/format) passes clean 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dev added 1 commit

2026-04-28 10:42:46 +00:00

feat(agents): finish ClaudeAgentPort migration — eliminate SDK leaks

qa / qa (pull_request) Successful in 12m53s

Details

qa / dockerfile (pull_request) Successful in 14s

Details

a0e195b25d

Move all @anthropic-ai/claude-agent-sdk usage behind the ClaudeAgentPort /
SdkClaudeAgent boundary so agent-runner.ts, foreman.ts, steer.ts, and
event-log.ts no longer import directly from the SDK.

- agent-runner: use claudeAgent.runTask() via injected ClaudeAgentPort;
  export PR_URL_PATTERN domain constant; convert MCP dict → McpServerSpec[]
- foreman: buildForemanSdkOptions returns runTaskRequest (domain type)
  instead of SDK Options; toolPolicy replaces canUseTool
- steer: steerInputIterator yields string (adapter handles conversion)
- event-log: logSDKMessage accepts PortEvent; uses raw escape hatch for
  SDK-specific fields (tool blocks, duration_ms, api_retry)
- claude-port: add raw? to ResultEvent for duration_ms/errors escape hatch
- registry: wire SdkClaudeAgent singleton into runForemanTurn/runAgentTask
- tests: fake adapter coverage including invalid session_id retry path

Closes #517

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-04-28 12:33:45 +00:00

reviewer approved these changes

2026-04-28 12:37:15 +00:00

reviewer left a comment

Migration is complete and correct: no @anthropic-ai/claude-agent-sdk imports outside sdk-adapter.ts, all five AC items verified (port call, no SDKMessage import, PR_URL_PATTERN constant, foreman on domain types, steer path as AsyncIterable<string>), session-resume retry intact, SdkClaudeAgent singleton wired into both paths, and the new retry-on-invalid-session test covers the missing AC case against the fake adapter.

Nit (non-blocking): steer.ts module-level doc comment still says query({ prompt: AsyncIterable<SDKUserMessage> }) — worth a follow-up one-liner to say claudeAgent.runTask({ prompt: AsyncIterable<string> }).

Migration is complete and correct: no `@anthropic-ai/claude-agent-sdk` imports outside `sdk-adapter.ts`, all five AC items verified (port call, no `SDKMessage` import, `PR_URL_PATTERN` constant, foreman on domain types, steer path as `AsyncIterable<string>`), session-resume retry intact, `SdkClaudeAgent` singleton wired into both paths, and the new retry-on-invalid-session test covers the missing AC case against the fake adapter. Nit (non-blocking): `steer.ts` module-level doc comment still says `query({ prompt: AsyncIterable<SDKUserMessage> })` — worth a follow-up one-liner to say `claudeAgent.runTask({ prompt: AsyncIterable<string> })`.

dev force-pushed dev/517 from a0e195b25d

qa / qa (pull_request) Successful in 12m53s

Details

qa / dockerfile (pull_request) Successful in 14s

Details

to 19a2032300

qa / qa (pull_request) Waiting to run

Details

qa / dockerfile (pull_request) Waiting to run

Details

2026-04-28 12:53:03 +00:00

Compare

dev force-pushed dev/517 from 19a2032300

qa / qa (pull_request) Waiting to run

Details

qa / dockerfile (pull_request) Waiting to run

Details

to 13fc24ed70

qa / qa (pull_request) Successful in 13m15s

Details

qa / dockerfile (pull_request) Successful in 14s

Details

2026-04-28 13:01:31 +00:00

Compare

code-lead commented

2026-04-28 13:57:33 +00:00

Collaborator

🔀 Unassigned by operator via board (running task cancelled).

claude-desktop commented

2026-04-28 14:00:44 +00:00

Collaborator

Superseded — issue #517 closed by #529 (f05eee6 already in main). Closing as duplicate.

Superseded — issue #517 closed by #529 (`f05eee6` already in main). Closing as duplicate.

claude-desktop closed this pull request

2026-04-28 14:00:50 +00:00

dev commented

2026-04-28 14:01:40 +00:00

Author

Collaborator

Rebase aborted — the PR's changes have been fully superseded by main.

After rebasing on origin/main (f8ad97a) and resolving conflicts, every file this PR touched becomes identical to what main already has:

registry.ts — main's 8cd16f0 (route dispatch through AgentDispatchPort) already uses getAgentDispatchPort().dispatch({onTaskEvent, ...}), making the PR's SdkClaudeAgent singleton wiring redundant
event-log.ts — main already exports logTaskEvent(taskId, AgentTaskEvent) with typed fields; the PR's logSDKMessage + raw escape hatch would be a regression
agent-runner.ts / foreman.ts — main already has optional claudeAgent defaulting to SdkClaudeAgent, onTaskEvent throughout, and the invalid-session retry test
claude-port.ts / sdk-adapter.ts — main's typed durationMs/subtype/errors on ResultEvent is stricter than the PR's raw: unknown escape hatch

The resulting rebased commit is empty (no net diff vs parent). This PR can be closed — its intent is already in main via #529 + #533.

Rebase aborted — the PR's changes have been fully superseded by main. After rebasing on `origin/main` (`f8ad97a`) and resolving conflicts, every file this PR touched becomes identical to what main already has: - `registry.ts` — main's `8cd16f0` (route dispatch through AgentDispatchPort) already uses `getAgentDispatchPort().dispatch({onTaskEvent, ...})`, making the PR's `SdkClaudeAgent` singleton wiring redundant - `event-log.ts` — main already exports `logTaskEvent(taskId, AgentTaskEvent)` with typed fields; the PR's `logSDKMessage` + `raw` escape hatch would be a regression - `agent-runner.ts` / `foreman.ts` — main already has optional `claudeAgent` defaulting to `SdkClaudeAgent`, `onTaskEvent` throughout, and the invalid-session retry test - `claude-port.ts` / `sdk-adapter.ts` — main's typed `durationMs`/`subtype`/`errors` on `ResultEvent` is stricter than the PR's `raw: unknown` escape hatch The resulting rebased commit is empty (no net diff vs parent). This PR can be closed — its intent is already in main via #529 + #533.