mirror of
https://github.com/katanemo/plano.git
synced 2026-06-08 14:55:14 +02:00
chore(claude-cli): drop dead code, drift-proof env defaults, clippy nits
- main.rs: rebuild claude_cli_config_from_env on top of
SessionManagerConfig::default() and only override fields that have a
parsed env var, so the defaults live in exactly one place.
- hermesllm/apis/claude_cli.rs: delete the dead
`_touch_messages_message_type` stub and its unused MessagesMessage
import; apply pedantic-clippy fixes that touch the new code
(clone_from over `= x.clone()`, Map::default() over Default::default(),
map_or_else over .map(...).unwrap_or_else(...), str::to_string method
reference, collapsed identical match arms).
- hermesllm/providers/id.rs: collapse the two match arms that mapped
"claude-cli" and "claude_cli" to ProviderId::ClaudeCli.
- hermesllm/tests/claude_cli_fixtures.rs: collect text deltas straight
into a String instead of `.collect::<Vec<_>>().join("")`.
- brightstaff/tests/claude_cli_bridge.rs: add a Drop impl on
BridgeFixture so a panicking test still releases the listener task.
This commit is contained in:
parent
3c58185389
commit
2aa9981f46
5 changed files with 60 additions and 50 deletions
|
|
@ -4,9 +4,7 @@ static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
|
|||
|
||||
use brightstaff::app_state::AppState;
|
||||
use brightstaff::handlers::agents::orchestrator::agent_chat;
|
||||
use brightstaff::handlers::claude_cli::{
|
||||
self, ClaudeCliConfig, SessionManager, SessionManagerConfig,
|
||||
};
|
||||
use brightstaff::handlers::claude_cli::{self, SessionManager, SessionManagerConfig};
|
||||
use brightstaff::handlers::debug;
|
||||
use brightstaff::handlers::empty;
|
||||
use brightstaff::handlers::function_calling::function_calling_chat_handler;
|
||||
|
|
@ -586,6 +584,11 @@ async fn run_server(state: Arc<AppState>) -> Result<(), Box<dyn std::error::Erro
|
|||
/// Build the [`SessionManagerConfig`] from environment variables. Returns
|
||||
/// `None` when `CLAUDE_CLI_LISTEN_ADDR` is unset, signaling that the bridge
|
||||
/// should not start at all (zero-cost when no claude-cli provider exists).
|
||||
///
|
||||
/// Starts from `SessionManagerConfig::default()` and only overrides fields
|
||||
/// for which the corresponding env var is both set and parses successfully.
|
||||
/// This keeps the defaults in one place (the `Default` impls) so they can't
|
||||
/// drift between this function and the library types.
|
||||
fn claude_cli_config_from_env() -> Option<(std::net::SocketAddr, SessionManagerConfig)> {
|
||||
let addr_str = env::var("CLAUDE_CLI_LISTEN_ADDR").ok()?;
|
||||
let addr: std::net::SocketAddr = match addr_str.parse() {
|
||||
|
|
@ -599,35 +602,33 @@ fn claude_cli_config_from_env() -> Option<(std::net::SocketAddr, SessionManagerC
|
|||
return None;
|
||||
}
|
||||
};
|
||||
let binary = env::var("CLAUDE_CLI_BIN").unwrap_or_else(|_| "claude".to_string());
|
||||
let permission_mode =
|
||||
env::var("CLAUDE_CLI_PERMISSION_MODE").unwrap_or_else(|_| "bypassPermissions".to_string());
|
||||
let session_ttl = env::var("CLAUDE_CLI_SESSION_TTL_SECS")
|
||||
|
||||
let mut cfg = SessionManagerConfig::default();
|
||||
if let Ok(s) = env::var("CLAUDE_CLI_BIN") {
|
||||
cfg.process.binary = s;
|
||||
}
|
||||
if let Ok(s) = env::var("CLAUDE_CLI_PERMISSION_MODE") {
|
||||
cfg.process.permission_mode = s;
|
||||
}
|
||||
if let Some(secs) = env::var("CLAUDE_CLI_SESSION_TTL_SECS")
|
||||
.ok()
|
||||
.and_then(|s| s.parse::<u64>().ok())
|
||||
.map(Duration::from_secs)
|
||||
.unwrap_or_else(|| Duration::from_secs(600));
|
||||
let watchdog = env::var("CLAUDE_CLI_WATCHDOG_SECS")
|
||||
{
|
||||
cfg.process.session_ttl = Duration::from_secs(secs);
|
||||
}
|
||||
if let Some(secs) = env::var("CLAUDE_CLI_WATCHDOG_SECS")
|
||||
.ok()
|
||||
.and_then(|s| s.parse::<u64>().ok())
|
||||
.map(Duration::from_secs)
|
||||
.unwrap_or_else(|| Duration::from_secs(120));
|
||||
let max_sessions = env::var("CLAUDE_CLI_MAX_SESSIONS")
|
||||
{
|
||||
cfg.process.watchdog = Duration::from_secs(secs);
|
||||
}
|
||||
if let Some(n) = env::var("CLAUDE_CLI_MAX_SESSIONS")
|
||||
.ok()
|
||||
.and_then(|s| s.parse::<usize>().ok())
|
||||
.unwrap_or(claude_cli::session::DEFAULT_MAX_SESSIONS);
|
||||
Some((
|
||||
addr,
|
||||
SessionManagerConfig {
|
||||
max_sessions,
|
||||
process: ClaudeCliConfig {
|
||||
binary,
|
||||
permission_mode,
|
||||
session_ttl,
|
||||
watchdog,
|
||||
},
|
||||
},
|
||||
))
|
||||
{
|
||||
cfg.max_sessions = n;
|
||||
}
|
||||
Some((addr, cfg))
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -84,6 +84,20 @@ impl BridgeFixture {
|
|||
}
|
||||
}
|
||||
|
||||
/// Best-effort cleanup if a test panics before `stop().await`. We can't
|
||||
/// `.await` from `Drop`, so we just abort the listener task; that's enough to
|
||||
/// keep the runtime from leaking the spawned future.
|
||||
impl Drop for BridgeFixture {
|
||||
fn drop(&mut self) {
|
||||
if let Some(tx) = self.shutdown.take() {
|
||||
let _ = tx.send(());
|
||||
}
|
||||
if let Some(h) = self.handle.take() {
|
||||
h.abort();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn anthropic_request(stream: bool) -> Value {
|
||||
json!({
|
||||
"model": "claude-cli/sonnet",
|
||||
|
|
|
|||
|
|
@ -9,15 +9,15 @@
|
|||
//! does the actual spawning and streaming.
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde_json::{json, Value};
|
||||
use serde_json::{json, Map, Value};
|
||||
use serde_with::skip_serializing_none;
|
||||
use thiserror::Error;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::apis::anthropic::{
|
||||
MessagesContentBlock, MessagesContentDelta, MessagesMessage, MessagesMessageContent,
|
||||
MessagesMessageDelta, MessagesRequest, MessagesResponse, MessagesRole, MessagesStopReason,
|
||||
MessagesStreamEvent, MessagesStreamMessage, MessagesSystemPrompt, MessagesUsage,
|
||||
MessagesContentBlock, MessagesContentDelta, MessagesMessageContent, MessagesMessageDelta,
|
||||
MessagesRequest, MessagesResponse, MessagesRole, MessagesStopReason, MessagesStreamEvent,
|
||||
MessagesStreamMessage, MessagesSystemPrompt, MessagesUsage,
|
||||
};
|
||||
|
||||
/// Errors produced by translation between Anthropic Messages and Claude Code
|
||||
|
|
@ -208,7 +208,7 @@ pub fn messages_request_to_stdin_payload(
|
|||
role: "user",
|
||||
content,
|
||||
},
|
||||
session_id: session_id.map(|s| s.to_string()),
|
||||
session_id: session_id.map(str::to_string),
|
||||
});
|
||||
}
|
||||
Ok(out)
|
||||
|
|
@ -292,10 +292,10 @@ where
|
|||
ClaudeCliEvent::StreamEvent { event } => match event {
|
||||
MessagesStreamEvent::MessageStart { message } => {
|
||||
if id.is_empty() {
|
||||
id = message.id.clone();
|
||||
id.clone_from(&message.id);
|
||||
}
|
||||
if !message.model.is_empty() {
|
||||
model_out = message.model.clone();
|
||||
model_out.clone_from(&message.model);
|
||||
}
|
||||
usage = message.usage.clone();
|
||||
}
|
||||
|
|
@ -337,7 +337,6 @@ where
|
|||
// clients but dropped from the non-streaming aggregate.
|
||||
_ => {}
|
||||
},
|
||||
MessagesStreamEvent::ContentBlockStop { .. } => {}
|
||||
MessagesStreamEvent::MessageDelta {
|
||||
delta,
|
||||
usage: msg_usage,
|
||||
|
|
@ -351,7 +350,9 @@ where
|
|||
// The MessageDelta usage carries final output_tokens.
|
||||
usage.output_tokens = msg_usage.output_tokens;
|
||||
}
|
||||
MessagesStreamEvent::MessageStop | MessagesStreamEvent::Ping => {}
|
||||
MessagesStreamEvent::ContentBlockStop { .. }
|
||||
| MessagesStreamEvent::MessageStop
|
||||
| MessagesStreamEvent::Ping => {}
|
||||
},
|
||||
ClaudeCliEvent::Assistant { message } => {
|
||||
last_assistant_message = Some(message);
|
||||
|
|
@ -411,7 +412,7 @@ where
|
|||
BlockKind::ToolUse => {
|
||||
if let Some((tool_id, name, raw_input)) = tool_accum.remove(&idx) {
|
||||
let input_value = if raw_input.is_empty() {
|
||||
Value::Object(Default::default())
|
||||
Value::Object(Map::default())
|
||||
} else {
|
||||
serde_json::from_str(&raw_input)
|
||||
.unwrap_or_else(|_| Value::String(raw_input))
|
||||
|
|
@ -505,9 +506,10 @@ pub fn cli_error_to_anthropic_error_body(message: &str) -> Value {
|
|||
/// the CLI did not emit one (it usually does, but very small turns can skip
|
||||
/// straight to `assistant`/`result`).
|
||||
pub fn synthetic_message_start(model: &str, session_id: Option<&str>) -> MessagesStreamEvent {
|
||||
let id = session_id
|
||||
.map(|s| format!("msg_cli_{}", s))
|
||||
.unwrap_or_else(|| format!("msg_cli_{}", Uuid::new_v4().simple()));
|
||||
let id = session_id.map_or_else(
|
||||
|| format!("msg_cli_{}", Uuid::new_v4().simple()),
|
||||
|s| format!("msg_cli_{s}"),
|
||||
);
|
||||
MessagesStreamEvent::MessageStart {
|
||||
message: MessagesStreamMessage {
|
||||
id,
|
||||
|
|
@ -537,11 +539,6 @@ pub fn parse_ndjson_line(line: &str) -> Option<Result<ClaudeCliEvent, serde_json
|
|||
Some(serde_json::from_str(trimmed))
|
||||
}
|
||||
|
||||
// Unused helper to keep MessagesMessage in scope in case future tool_result
|
||||
// translation needs to reach into the message shape directly.
|
||||
#[allow(dead_code)]
|
||||
fn _touch_messages_message_type(_m: MessagesMessage) {}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
|
|
|||
|
|
@ -86,8 +86,7 @@ impl TryFrom<&str> for ProviderId {
|
|||
"do_ai" => Ok(ProviderId::DigitalOcean), // alias
|
||||
"vercel" => Ok(ProviderId::Vercel),
|
||||
"openrouter" => Ok(ProviderId::OpenRouter),
|
||||
"claude-cli" => Ok(ProviderId::ClaudeCli),
|
||||
"claude_cli" => Ok(ProviderId::ClaudeCli), // alias
|
||||
"claude-cli" | "claude_cli" => Ok(ProviderId::ClaudeCli),
|
||||
_ => Err(format!("Unknown provider: {}", value)),
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -56,17 +56,16 @@ fn text_response_aggregates_into_messages_response() {
|
|||
));
|
||||
let final_event = stream.last().unwrap();
|
||||
assert!(matches!(final_event, MessagesStreamEvent::MessageStop));
|
||||
let text_deltas = stream
|
||||
let text_deltas: String = stream
|
||||
.iter()
|
||||
.filter_map(|ev| match ev {
|
||||
MessagesStreamEvent::ContentBlockDelta {
|
||||
delta: MessagesContentDelta::TextDelta { text },
|
||||
..
|
||||
} => Some(text.clone()),
|
||||
} => Some(text.as_str()),
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("");
|
||||
.collect();
|
||||
assert_eq!(text_deltas, "Hello, world!");
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue