fix(transport): support Anthropic list-form system prompts (types + CLI)#900
fix(transport): support Anthropic list-form system prompts (types + CLI)#900zion-off wants to merge 2 commits into
system prompts (types + CLI)#900Conversation
…t in ClaudeAgentOptions
seeincodes
left a comment
There was a problem hiding this comment.
Drive-by review (not a maintainer; flagging two issues that look load-bearing).
The shape of the change is good — public type alias, materialize/cleanup helpers, cleanup wired into close() and the existing connect() error branches. Tests cover the round-trip and the close-time cleanup.
Two issues worth addressing before merge:
-
Tempfile leaks on cancellation during
connect()._build_command()at subprocess_cli.py:423 creates the tempfile, then theconnect()try block awaitsanyio.open_process()at line 474. If the parent task is cancelled during that await (or anywhere else inside the try), the cancellation is BaseException-derived (anyio'sCancelledError/trio.Cancelled), so it bypasses bothexcept FileNotFoundErrorandexcept Exceptionat lines 502 / 513 — and the tempfile leaks. Same shape forKeyboardInterrupt. Likely fix: structure astry/finallythat runs cleanup wheneverself._readynever gets set toTrue, rather than relying onexceptbranches. -
Cleanup uses
suppress(FileNotFoundError)only. On Windows, if the CLI subprocess still has the file open whenpath.unlink()runs, you getPermissionError, notFileNotFoundError. In practice the CLI reads--system-prompt-fileat startup and closes it, so this is unlikely to bite, but worth either widening the suppression to(FileNotFoundError, PermissionError)or noting the assumption.
Smaller things:
- The memoization in
_materialize_system_prompt_blocks(skips creating a new file if_generated_system_prompt_file is not None) is fragile if the same transport instance ever gets_build_command()-then-cleanup-then-_build_command()with different blocks. Cleanup nulls the field so this path is currently unreachable, but a fresh-tempfile-per-call model would be simpler to reason about. - Test gap: no coverage for the cleanup-on-error path. Mocking
anyio.open_processto raise before_ready=Trueand asserting the temp file is gone would catch (1) above as a regression test.
Nothing blocking on the happy path.
Summary: Fix a crash when callers pass Anthropic-style list-form
systemprompts. The subprocess CLI transport previously assumed non-stringsystem_promptvalues were mappings and called.get()on them, raising AttributeError for lists. This change makes the SDK accept list-form system prompts in the public types and forwards them to the bundled Claude CLI by writing a temporary JSON file and passing it via the existing--system-prompt-fileflag. The temporary file is cleaned up on error or transport close.Changes:
SystemPromptBlocksand allowlist[dict[str, Any]]forClaudeAgentOptions.system_promptto reflect Anthropic Messages API shape. See types.py.SubprocessCLITransportnow detects list-form system prompts, writes them to a temp JSON file, passes--system-prompt-file <path>to the CLI, and removes the temp file on close/error. See subprocess_cli.py.Motivation: Upstream Anthropic docs allow
systemto be either a string or a list of content blocks (to support per-block metadata likecache_control). Previously the SDK crashed early on list inputs; this patch preserves the list form and forwards it unchanged to the CLI, enabling correct caching semantics and avoiding forcing callers to flatten blocks into a string.Fixes #899.