Skip to content

Commit 74c7fc0

Browse files
ZhiXiao-Linclaude
andauthored
feat(api): wire LlmClient injection seam; stop stderr writes; prune dead Planner trait (#68)
Framework-vs-host boundary tightening from the architecture audit. Added — SessionOptions::with_llm_client(Arc<dyn LlmClient>): - The LlmClient trait + Arc<dyn> engine already existed but were injectable only in #[cfg(test)] code (build_session); every public session path resolved via a hardcoded provider-string factory. This adds the public seam and has resolve_session_llm_client prefer a host-supplied client, bringing the Action-layer backend to parity with workspace/memory/store/security (all object-injectable). The provider/model factory stays the default when unset. Enables custom/unsupported providers, record/replay clients, and proxy/audit wrappers without forking core. Fixed — framework no longer writes to the host's stderr: - Replaced a stray eprintln!("[DEBUG] HTTP error...") in the OpenAI client (fired on every transport error) with tracing::error!, matching the rest of the crate. Removed — dead Planner trait (planning::Planner): - Re-exported but had zero dyn-dispatch and no consumer; all call sites use LlmPlanner's inherent methods. Pruned per Rule 9 (the only real variability, the LLM, is now swappable via with_llm_client). LlmPlanner unchanged. Tests: 2 new resolver tests (override bypasses config; control case errors). cargo test -p a3s-code-core --lib: 1757 passed, 0 failed. clippy clean. Co-authored-by: Claude <claude@anthropic.com>
1 parent 890679c commit 74c7fc0

7 files changed

Lines changed: 107 additions & 45 deletions

File tree

CHANGELOG.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
computed fresh each turn in `turn_context.rs` (no shell-out). Most importantly
1515
it pins the current date, which the model otherwise cannot infer past its
1616
training cutoff.
17+
- **`SessionOptions::with_llm_client`** — hosts can now inject a custom
18+
`Arc<dyn LlmClient>` (custom/unsupported provider, deterministic record/replay
19+
client, or HTTP proxy/audit wrapper). The `LlmClient` trait and `Arc<dyn>`
20+
engine already existed but were only injectable in test code; this wires the
21+
seam through the public API, bringing the Action-layer backend to parity with
22+
workspace/memory/store/security (all object-injectable). The `provider/model`
23+
factory remains the default when unset.
1724

1825
### Security
1926

@@ -39,6 +46,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3946
discourages re-printing already-read code and creating unsolicited report
4047
`.md` files.
4148

49+
### Fixed
50+
51+
- **Framework no longer writes to the host's stderr** — replaced a stray
52+
`eprintln!("[DEBUG] HTTP error...")` in the OpenAI client (fired on every
53+
transport error, also leaking into the host terminal) with `tracing::error!`,
54+
consistent with the rest of the crate.
55+
56+
### Removed
57+
58+
- **Dead `Planner` trait** (`planning::Planner`) — it was re-exported but had no
59+
`dyn Planner` dispatch and no consumer; every call site uses `LlmPlanner`'s
60+
inherent methods directly. Removed per the pruning rule (the real variability,
61+
the LLM, is swappable via `with_llm_client`). `LlmPlanner` is unchanged.
62+
4263
## [3.4.0] - 2026-05-30
4364

4465
Programmable, deterministic multi-agent orchestration — a grammar for

core/src/agent_api.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,14 @@ pub struct SessionOptions {
136136
pub queue_config: Option<SessionQueueConfig>,
137137
/// Optional security provider for taint tracking and output sanitization
138138
pub security_provider: Option<Arc<dyn crate::security::SecurityProvider>>,
139+
/// Optional host-supplied LLM client.
140+
///
141+
/// When set, it is used directly, overriding the `provider/model`
142+
/// factory resolution — the one Action-layer backend that was previously
143+
/// only injectable in test code. Lets a host plug in a provider the
144+
/// built-in factory does not cover, a deterministic record/replay client,
145+
/// or an HTTP-layer proxy/audit wrapper. Mirrors `workspace_services`.
146+
pub llm_client: Option<Arc<dyn crate::llm::LlmClient>>,
139147
/// Optional context providers for RAG
140148
pub context_providers: Vec<Arc<dyn crate::context::ContextProvider>>,
141149
/// Optional confirmation manager for HITL

core/src/agent_api/session_config.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@ pub(super) fn resolve_session_llm_client(
1010
opts: &SessionOptions,
1111
session_id: Option<&str>,
1212
) -> Result<Arc<dyn LlmClient>> {
13+
// A host-supplied client overrides the provider-string factory entirely:
14+
// the host owns the full Action-layer dependency (custom provider, replay
15+
// client, proxy/audit wrapper). Config-based model resolution is bypassed.
16+
if let Some(ref client) = opts.llm_client {
17+
return Ok(Arc::clone(client));
18+
}
19+
1320
let model_ref = if let Some(ref model) = opts.model {
1421
model.as_str()
1522
} else {
@@ -94,6 +101,62 @@ pub(super) fn resolve_session_memory(opts: &SessionOptions) -> ResolvedSessionMe
94101
}
95102
}
96103

104+
#[cfg(test)]
105+
mod tests {
106+
use super::*;
107+
use crate::llm::{LlmResponse, Message, StreamEvent, ToolDefinition};
108+
// The LlmClient trait returns anyhow::Result; shadow super's crate::error::Result.
109+
use anyhow::Result;
110+
use async_trait::async_trait;
111+
use tokio::sync::mpsc;
112+
use tokio_util::sync::CancellationToken;
113+
114+
struct DummyClient;
115+
116+
#[async_trait]
117+
impl LlmClient for DummyClient {
118+
async fn complete(
119+
&self,
120+
_: &[Message],
121+
_: Option<&str>,
122+
_: &[ToolDefinition],
123+
) -> Result<LlmResponse> {
124+
anyhow::bail!("resolver short-circuits before the client is called")
125+
}
126+
async fn complete_streaming(
127+
&self,
128+
_: &[Message],
129+
_: Option<&str>,
130+
_: &[ToolDefinition],
131+
_: CancellationToken,
132+
) -> Result<mpsc::Receiver<StreamEvent>> {
133+
anyhow::bail!("not used")
134+
}
135+
}
136+
137+
// A default CodeConfig has no default_model, so the provider-string factory
138+
// path errors — proving the override is what makes resolution succeed.
139+
#[test]
140+
fn host_supplied_llm_client_overrides_factory() {
141+
let config = CodeConfig::default();
142+
let opts = SessionOptions::new().with_llm_client(Arc::new(DummyClient));
143+
assert!(
144+
resolve_session_llm_client(&config, &opts, None).is_ok(),
145+
"with_llm_client must bypass provider/model config resolution"
146+
);
147+
}
148+
149+
#[test]
150+
fn without_llm_client_missing_default_model_errors() {
151+
let config = CodeConfig::default();
152+
let opts = SessionOptions::new();
153+
assert!(
154+
resolve_session_llm_client(&config, &opts, None).is_err(),
155+
"no host client + no default_model should error (control case)"
156+
);
157+
}
158+
}
159+
97160
pub(super) fn resolve_session_store(
98161
code_config: &CodeConfig,
99162
opts: &SessionOptions,

core/src/agent_api/session_options.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ impl std::fmt::Debug for SessionOptions {
2121
.field("skill_dirs", &self.skill_dirs)
2222
.field("queue_config", &self.queue_config)
2323
.field("security_provider", &self.security_provider.is_some())
24+
.field("llm_client", &self.llm_client.is_some())
2425
.field("context_providers", &self.context_providers.len())
2526
.field("confirmation_manager", &self.confirmation_manager.is_some())
2627
.field("permission_checker", &self.permission_checker.is_some())
@@ -110,6 +111,18 @@ impl SessionOptions {
110111
self
111112
}
112113

114+
/// Provide a custom LLM client for this session.
115+
///
116+
/// When set, this client is used directly, overriding the `provider/model`
117+
/// factory resolution. Use it to plug in a provider the built-in factory
118+
/// does not cover, a deterministic record/replay client for tests, or an
119+
/// HTTP-layer proxy/audit wrapper. Mirrors [`Self::with_workspace_backend`];
120+
/// the `provider/model` config path remains the default when unset.
121+
pub fn with_llm_client(mut self, client: Arc<dyn crate::llm::LlmClient>) -> Self {
122+
self.llm_client = Some(client);
123+
self
124+
}
125+
113126
/// Add a file system context provider for simple RAG
114127
pub fn with_fs_context(mut self, root_path: impl Into<PathBuf>) -> Self {
115128
let config = crate::context::FileSystemContextConfig::new(root_path);

core/src/llm/openai.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ impl LlmClient for OpenAiClient {
356356
}
357357
}
358358
Err(e) => {
359-
eprintln!("[DEBUG] HTTP error: {:?}", e);
359+
tracing::error!("HTTP error: {e:?}");
360360
AttemptOutcome::Fatal(e)
361361
}
362362
}

core/src/planning/llm_planner.rs

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
use crate::llm::{LlmClient, Message};
88
use crate::planning::{AgentGoal, Complexity, ExecutionPlan, Task};
99
use anyhow::{Context, Result};
10-
use async_trait::async_trait;
1110
use serde::{Deserialize, Serialize};
1211
use std::sync::Arc;
1312

@@ -33,27 +32,6 @@ pub struct PreAnalysis {
3332
pub optimized_input: String,
3433
}
3534

36-
/// Trait for planning providers
37-
///
38-
/// Abstracts plan generation, goal extraction, and achievement evaluation.
39-
/// Allows different implementations (LLM-based, heuristic, test mocks).
40-
#[async_trait]
41-
pub trait Planner: Send + Sync {
42-
/// Generate an execution plan from a prompt
43-
async fn create_plan(&self, llm: &Arc<dyn LlmClient>, prompt: &str) -> Result<ExecutionPlan>;
44-
45-
/// Extract a goal with success criteria from a prompt
46-
async fn extract_goal(&self, llm: &Arc<dyn LlmClient>, prompt: &str) -> Result<AgentGoal>;
47-
48-
/// Evaluate whether a goal has been achieved given current state
49-
async fn check_achievement(
50-
&self,
51-
llm: &Arc<dyn LlmClient>,
52-
goal: &AgentGoal,
53-
current_state: &str,
54-
) -> Result<AchievementResult>;
55-
}
56-
5735
/// LLM-powered planner that generates plans, extracts goals, and evaluates achievement
5836
pub struct LlmPlanner;
5937

@@ -380,27 +358,6 @@ impl LlmPlanner {
380358
}
381359
}
382360

383-
// Implement Planner trait for LlmPlanner
384-
#[async_trait]
385-
impl Planner for LlmPlanner {
386-
async fn create_plan(&self, llm: &Arc<dyn LlmClient>, prompt: &str) -> Result<ExecutionPlan> {
387-
LlmPlanner::create_plan(llm, prompt).await
388-
}
389-
390-
async fn extract_goal(&self, llm: &Arc<dyn LlmClient>, prompt: &str) -> Result<AgentGoal> {
391-
LlmPlanner::extract_goal(llm, prompt).await
392-
}
393-
394-
async fn check_achievement(
395-
&self,
396-
llm: &Arc<dyn LlmClient>,
397-
goal: &AgentGoal,
398-
current_state: &str,
399-
) -> Result<AchievementResult> {
400-
LlmPlanner::check_achievement(llm, goal, current_state).await
401-
}
402-
}
403-
404361
// ============================================================================
405362
// Tests
406363
// ============================================================================

core/src/planning/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
88
pub mod llm_planner;
99

10-
pub use llm_planner::{AchievementResult, LlmPlanner, Planner, PreAnalysis};
10+
pub use llm_planner::{AchievementResult, LlmPlanner, PreAnalysis};
1111

1212
use serde::{Deserialize, Serialize};
1313
use std::fmt;

0 commit comments

Comments
 (0)