Add SystemPromptFile support for --system-prompt-file CLI flag#591
Conversation
b0a525e to
01c6730
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #591 +/- ##
=======================================
Coverage ? 82.95%
=======================================
Files ? 14
Lines ? 2089
Branches ? 0
=======================================
Hits ? 1733
Misses ? 356
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
qing-ant
left a comment
There was a problem hiding this comment.
Tested locally — works end-to-end, the CLI picks up the file prompt correctly. Two things blocking lint:
ruff format — line 183 in subprocess_cli.py needs wrapping (run ruff format src/ and it'll fix itself).
mypy (hidden behind the ruff failure — will surface once formatting is fixed):
subprocess_cli.py:180: error: TypedDict "SystemPromptFile" has no key "append"
subprocess_cli.py:183: error: TypedDict "SystemPromptPreset" has no key "path"
.get("type") == "..." doesn't narrow the SystemPromptPreset | SystemPromptFile union for mypy, so it still sees both types when you index ["append"] / ["path"]. Checking "path" in self._options.system_prompt instead of the type tag should do it, or a cast().
Otherwise LGTM — no injection risk, logic is correct.
|
Thanks! Updating |
|
Re-running the checkers, may need 1 more update? Thanks! |
|
@claude review |
There was a problem hiding this comment.
LGTM — straightforward feature addition following existing patterns, with good test coverage.
Extended reasoning...
Overview
This PR adds SystemPromptFile support to the SDK, allowing users to pass --system-prompt-file <path> to the CLI instead of inlining large system prompts (which can hit ARGMAX limits). Changes touch 4 files: a new SystemPromptFile TypedDict in types.py, command-building logic in subprocess_cli.py, and corresponding tests in both test files.
Security risks
No security concerns. The change passes a user-supplied file path through to the CLI as a --system-prompt-file argument — no path traversal, injection, or auth implications. The CLI itself handles file reading.
Level of scrutiny
Low scrutiny warranted. This is a small, mechanical addition that follows the exact same pattern as the existing SystemPromptPreset support. The branching logic change (from checking type == "preset" and "append" to checking "path" in ... then "append" in ...) is safe because SystemPromptFile and SystemPromptPreset have non-overlapping keys. The existing test_build_command_with_system_prompt_preset and test_build_command_with_system_prompt_preset_and_append tests confirm backward compatibility.
Other factors
Codecov confirms all modified lines are covered. No CODEOWNERS file exists. The PR is well-scoped with clear motivation (ARGMAX errors on large prompts). The refactored if/elif in _build_command is actually slightly cleaner than the original since it uses duck-typing on keys rather than redundantly checking both type and key presence.
Allow users to specify a file path for the system prompt via a new SystemPromptFile TypedDict, which passes --system-prompt-file to the CLI.
Use `in` checks instead of `.get("type")` comparisons so mypy can
properly narrow the TypedDict union type.
mypy cannot narrow TypedDict unions via `in` or `.get("type")` checks.
Use `cast()` to explicitly narrow after runtime type-tag checks.
c5cf5ac to
87cf727
Compare
I'm trying to pass a really big system prompt to Claude and it fails on an ARGMAX error. This change exposes --system-prompt-file to the SDK.
Summary
SystemPromptFileTypedDict withtype: "file"andpathfields totypes.pySystemPromptFileto thesystem_promptunion type onClaudeAgentOptionssubprocess_cli.pyby passing--system-prompt-file <path>to the CLItest_types.pyandtest_transport.pyTest plan
test_claude_code_options_with_system_prompt_file— verifiesClaudeAgentOptionsaccepts and stores aSystemPromptFiledicttest_build_command_with_system_prompt_file— verifies the command builder emits--system-prompt-filewith the path, and does not emit--system-promptor--append-system-prompttest_types.pyandtest_transport.pycontinue to pass