fix: raise a clear error for list-form system_prompt#1074
Conversation
There was a problem hiding this comment.
Pull request overview
This PR prevents a crash in the subprocess CLI transport when ClaudeAgentOptions.system_prompt is provided in the Messages-API “list of content blocks” form by rejecting that input with a clear ValueError, and adds a regression test for the improved error.
Changes:
- Add an explicit runtime check in
SubprocessCLITransport._build_commandto detect list-formsystem_promptand raise a clearValueErrorinstead of crashing withAttributeError. - Add a test asserting the clearer error message for list-form
system_prompt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/claude_agent_sdk/_internal/transport/subprocess_cli.py |
Adds list-type detection for system_prompt and raises a clearer error instead of calling .get on a list. |
tests/test_transport.py |
Adds regression coverage ensuring list-form system_prompt raises a helpful ValueError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elif isinstance(self._options.system_prompt, list): | ||
| raise ValueError( | ||
| "system_prompt does not accept a list of content blocks. Pass a " | ||
| 'string, a preset ({"type": "preset", ...}), or a file ' | ||
| '({"type": "file", ...}).' | ||
| ) | ||
| else: | ||
| sp = self._options.system_prompt | ||
| if sp.get("type") == "file": |
There was a problem hiding this comment.
Good point, rather than just fix the single one, I will implement the check for whats required(dict | str), and do a catch of all else and will abstract it out to a guard since the ClaudeAgentOptions is a dataclass with no validation.
Passing system_prompt as anything other than a string, preset, or file (e.g. the Messages-API list-of-content-blocks form, or any other unexpected type) crashed while building the CLI command with a cryptic "AttributeError: '<type>' object has no attribute 'get'", because the non-string branch assumed a dict and called sp.get(...). Gate the dict handling behind an explicit isinstance(sp, dict) check and raise a ValueError naming the accepted forms and the offending type for everything else. ClaudeAgentOptions is a dataclass with no runtime validation, so untyped callers reach this path. The subprocess CLI transport has no channel to forward a structured system array (--system-prompt-file is read as plain text, so cache_control blocks would be silently dropped), so a clear rejection is the correct behavior until the CLI gains that capability. Refs anthropics#899
49c0ba2 to
1542d75
Compare
|
Closing this in favor of the existing PRs that implement actual list-form system_prompt support rather than just raising a clearer error:
This PR only converted the AttributeError into a ValueError, so it is superseded by either of those. Stepping aside to avoid competing implementations. |
Summary
Refs #899. Passing
system_promptas a list of content blocks (the Messages-API list-of-blocks form, e.g. to placecache_controlon a trailing block) crashed while building the CLI command:The non-string branch in
_build_commandassumed a dict and calledsp.get("type").Changes
_internal/transport/subprocess_cli.py— detect the list form and raise aValueErrornaming the accepted forms (string, preset, file), instead of crashing withAttributeError.tests/test_transport.py— assert the clear error.Scope / why not full support
This intentionally does not add support for forwarding a structured
systemarray. The subprocess CLI transport has no channel for it:--system-prompt-fileis read as plain text, socache_controlblocks would be silently dropped — worse than a clear rejection. Actually forwarding a structured system array (the deeper request in #899) needs a new CLI flag and is left open. HenceRefs #899rather thanFixes.Test plan
ruff check src/ tests/— cleanruff format --check src/ tests/— cleanmypy src/— cleanpytest tests/— passing