feat: add streaming support for Claude Code CLI provider#6833
feat: add streaming support for Claude Code CLI provider#6833DOsinga merged 1 commit intoblock:mainfrom
Conversation
rabi
changed the title
rabi
force-pushed
the
6694
branch
3 times, most recently
from
ad5b039 to
e4a0cf1
Compare
|
@jamadeo Hey! it's probably the last one in the series of streaming support. PTAL when possible. |
|
Hmm. I've to now rewrite the streaming implementation fresh as we moved to persistent child process that stays alive across turns vs spawning a fresh claude CLI process per request in #7029 that merged quickly. |
rabi
force-pushed
the
6694
branch
2 times, most recently
from
084c392 to
810c54d
Compare
| model, | ||
| name: CLAUDE_CODE_PROVIDER_NAME.to_string(), | ||
| cli_process: tokio::sync::OnceCell::new(), | ||
| cli_process: Arc::new(tokio::sync::Mutex::new(None)), |
There was a problem hiding this comment.
since we aren't respawning can you revert the process control logic back? Options are a smell unless proven otherwise, and this change as it sits shouldn't need it. let's pare down the logic to the streaming part?
codefromthecrypt
left a comment
codefromthecrypt
left a comment
There was a problem hiding this comment.
ps I should have started with saying thanks for working on this.
note that currently we have no CI job setup to live test claude. what you can do instead is run the providers.rs test manually for the claude, just to make sure. I'll chase up live tests this week.
I don't see claude_code provider testing in @crates/goose/tests/providers.rs atm, unless you're asking me to add it. ProviderTester::run_test_suite() won't work as-is for claude_code as it does not support tools or /models api, so it's just about running test_basic_response(), which I have already tested locally. |
|
right the test refactor was caged to another PR. I extracted this which should hopefully land before your next change #7108 |
I'll rebase my PR once you're done with all your changes as I don't want to keep doing it again and again, let me know. |
|
If you prefer I can just do this. The changes I have been doing was to level set and begin testing claude and codex as both were very broken and will be replaced by acp soon. Lemme know |
|
@rabi cool you're on it! so I'm not doing anything else related to the provider, so your merge is safe this time ;) I have something personal come up so I won't be here to support the merge, so hopefully someone else can help. |
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
the one for gemini might have some overlapping code, probably not worth reusing?
Thanks @DOsinga, the new functions added in these two commits are similar in pattern but differ in return types and error handling due to different JSON schemas. Having said that, I agree that there are some existing repeated functions across all CLI providers in existing code (ex. generate_simple_session_description(), is_session_description_request()) that are copy pasted across providers. I'll create a follow-up PR to extract the truly shared ones into a common module. |
- Add cancellation-safe streaming support for claude code - Move common CLI provider utilities (extract_usage_tokens, error_from_event, is_session_description_request, generate_simple_session_description) into cli_common.rs Change-Id: I26cdb5e760c963e7203826d1ebc97c361591f8cd Signed-off-by: rabi <ramishra@redhat.com>
Now that #7118 has meged, I've refactored what could be moved to a common module as suggested, but it has bloated this PR. |
…ntext * 'main' of github.com:block/goose: feat: add onFallbackRequest handler to McpAppRenderer (#7208) feat: add streaming support for Claude Code CLI provider (#6833) fix: The detected filetype is PLAIN_TEXT, but the provided filetype was HTML (#6885) Add prompts (#7212) Add testing instructions for speech to text (#7185) Diagnostic files copying (#7209) fix: allow concurrent tool execution within the same MCP extension (#7202) fix: handle missing arguments in MCP tool calls to prevent GUI crash (#7143) Filter Apps page to only show standalone Goose Apps (#6811) opt: use static for Regex (#7205) nit: show dir in title, and less... jank (#7138) feat(gemini-cli): use stream-json output and re-use session (#7118) chore(deps): bump qs from 6.14.1 to 6.14.2 in /documentation (#7191) Switch jsonwebtoken to use aws-lc-rs (already used by rustls) (#7189) chore(deps): bump qs from 6.14.1 to 6.14.2 in /evals/open-model-gym/mcp-harness (#7184) Add SLSA build provenance attestations to release workflows (#7097) fix save and run recipe not working (#7186) Upgraded npm packages for latest security updates (#7183) docs: reasoning effort levels for Codex provider (#6798)
* origin/main: (42 commits) fix: use dynamic port for Tetrate auth callback server (#7228) docs: removing LLM Usage admonitions (#7227) feat(otel): respect standard OTel env vars for exporter selection (#7144) fix: fork session (#7219) Bump version numbers for 1.24.0 release (#7214) Move platform extensions into their own folder (#7210) fix: ignore deprecated skills extension (#7139) Add a goosed over HTTP integration test, and test the developer tool PATH (#7178) feat: add onFallbackRequest handler to McpAppRenderer (#7208) feat: add streaming support for Claude Code CLI provider (#6833) fix: The detected filetype is PLAIN_TEXT, but the provided filetype was HTML (#6885) Add prompts (#7212) Add testing instructions for speech to text (#7185) Diagnostic files copying (#7209) fix: allow concurrent tool execution within the same MCP extension (#7202) fix: handle missing arguments in MCP tool calls to prevent GUI crash (#7143) Filter Apps page to only show standalone Goose Apps (#6811) opt: use static for Regex (#7205) nit: show dir in title, and less... jank (#7138) feat(gemini-cli): use stream-json output and re-use session (#7118) ...
3 participants
Summary
Implement stream() method using Claude CLI's --output-format stream-json and --include-partial-messages flags for incremental text output. Parse NDJSON events (content_block_delta, message_start, message_delta, result) to yield text chunks and usage information.
Type of Change
AI Assistance
Testing
Unit tests for parsing steaming events and tested locally.
Closes: #6694