mirror of
https://github.com/instructkr/claw-code.git
synced 2026-06-06 01:42:47 -04:00
Compare commits
110 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
eaa2e320d9 | ||
|
|
be8112f5f5 | ||
|
|
503d515f38 | ||
|
|
114c2da9fc | ||
|
|
b90f18f75a | ||
|
|
5b15197117 | ||
|
|
61e8ad9a8e | ||
|
|
ef0d0c30a4 | ||
|
|
7305e5509a | ||
|
|
2f0b5b36eb | ||
|
|
c848eeb768 | ||
|
|
d4aad7103e | ||
|
|
b60cbebb3a | ||
|
|
7c4bcd92b6 | ||
|
|
f8822aabdb | ||
|
|
aca6584fd5 | ||
|
|
b8cbb1834f | ||
|
|
cb027adf65 | ||
|
|
5bdffbe161 | ||
|
|
04f18862a8 | ||
|
|
f0e10ff182 | ||
|
|
0cab03df75 | ||
|
|
eb86f4d2c3 | ||
|
|
6c3d7be370 | ||
|
|
e3ffaefba1 | ||
|
|
cd18cf5385 | ||
|
|
2f3120e70a | ||
|
|
f1a16398fe | ||
|
|
8d50276366 | ||
|
|
13992ade54 | ||
|
|
b04b1d6ac8 | ||
|
|
934bf2837a | ||
|
|
b94c49c323 | ||
|
|
5df485cba9 | ||
|
|
c8e973513c | ||
|
|
8f9315bdc9 | ||
|
|
12a091afe6 | ||
|
|
9f0cf3b888 | ||
|
|
6e94c1a97f | ||
|
|
38626cad20 | ||
|
|
e84f7c8034 | ||
|
|
b8f066347b | ||
|
|
5adc751053 | ||
|
|
adf5bd165e | ||
|
|
b220366176 | ||
|
|
b5d67ef249 | ||
|
|
9a40568e1c | ||
|
|
41b3566468 | ||
|
|
b86159cbb7 | ||
|
|
5e5b9966c0 | ||
|
|
33f771f3f5 | ||
|
|
3fbfcc40ca | ||
|
|
c7c5c11d1e | ||
|
|
c0447e2be1 | ||
|
|
1b22ed700a | ||
|
|
76f9a134ae | ||
|
|
9c11325e83 | ||
|
|
4708ab1611 | ||
|
|
311e719e5d | ||
|
|
b926a9d25f | ||
|
|
61d641d722 | ||
|
|
2f8679bd15 | ||
|
|
9ef21e23f3 | ||
|
|
6ac0386094 | ||
|
|
4c939c0ad6 | ||
|
|
4d41ab37e1 | ||
|
|
662a50bdc0 | ||
|
|
1da4aa454f | ||
|
|
3f50f33407 | ||
|
|
0ce4168c93 | ||
|
|
7f1dd0c116 | ||
|
|
42f56e7f77 | ||
|
|
f25fae6d22 | ||
|
|
be66d961bf | ||
|
|
b1a40a2364 | ||
|
|
726d55d1ea | ||
|
|
e4b8f9c07f | ||
|
|
b3a5a74237 | ||
|
|
ad76389b31 | ||
|
|
e68733d72e | ||
|
|
4d4d72cd49 | ||
|
|
9bc2f3631d | ||
|
|
f40927ba97 | ||
|
|
5bcbc2f874 | ||
|
|
a671969688 | ||
|
|
6757ebde74 | ||
|
|
2447273d09 | ||
|
|
5a76ecb760 | ||
|
|
db56498460 | ||
|
|
6fcd0c57ae | ||
|
|
de66bfc082 | ||
|
|
5d85739358 | ||
|
|
8fd11e82c4 | ||
|
|
9f9b14a76d | ||
|
|
78c2a49ec8 | ||
|
|
0e54ec4c04 | ||
|
|
58a30f6ab8 | ||
|
|
453d8945bb | ||
|
|
9e50cb6e20 | ||
|
|
346772a8b3 | ||
|
|
ef392e5938 | ||
|
|
41034bb3f3 | ||
|
|
76783377ec | ||
|
|
e459a727e9 | ||
|
|
04bc5f5788 | ||
|
|
571d3cdc0f | ||
|
|
414a1aca4f | ||
|
|
d8c57ed317 | ||
|
|
e8c8ef1142 | ||
|
|
1d516be779 |
@@ -226,6 +226,7 @@ Claw Code is built in the open alongside the broader UltraWorkers toolchain:
|
||||
- [oh-my-openagent](https://github.com/code-yeongyu/oh-my-openagent)
|
||||
- [oh-my-claudecode](https://github.com/Yeachan-Heo/oh-my-claudecode)
|
||||
- [oh-my-codex](https://github.com/Yeachan-Heo/oh-my-codex)
|
||||
- [gajae-code](https://github.com/Yeachan-Heo/gajae-code)
|
||||
- [UltraWorkers Discord](https://discord.gg/5TUQKqFWd)
|
||||
|
||||
## Ownership / affiliation disclaimer
|
||||
|
||||
681
ROADMAP.md
681
ROADMAP.md
File diff suppressed because one or more lines are too long
12
USAGE.md
12
USAGE.md
@@ -245,6 +245,7 @@ export ANTHROPIC_AUTH_TOKEN="anthropic-oauth-or-proxy-bearer-token"
|
||||
| `sk-ant-*` API key | `ANTHROPIC_API_KEY` | `x-api-key: sk-ant-...` | [console.anthropic.com](https://console.anthropic.com) |
|
||||
| OAuth access token (opaque) | `ANTHROPIC_AUTH_TOKEN` | `Authorization: Bearer ...` | an Anthropic-compatible proxy or OAuth flow that mints bearer tokens |
|
||||
| OpenRouter key (`sk-or-v1-*`) | `OPENAI_API_KEY` + `OPENAI_BASE_URL=https://openrouter.ai/api/v1` | `Authorization: Bearer ...` | [openrouter.ai/keys](https://openrouter.ai/keys) |
|
||||
| Ollama local instance | `OLLAMA_HOST` | no auth header (Ollama requires none) | local Ollama server at `http://127.0.0.1:11434` |
|
||||
|
||||
**Why this matters:** if you paste an `sk-ant-*` key into `ANTHROPIC_AUTH_TOKEN`, Anthropic's API will return `401 Invalid bearer token` because `sk-ant-*` keys are rejected over the Bearer header. The fix is a one-line env var swap — move the key to `ANTHROPIC_API_KEY`. Recent `claw` builds detect this exact shape (401 + `sk-ant-*` in the Bearer slot) and append a hint to the error message pointing at the fix.
|
||||
|
||||
@@ -305,18 +306,18 @@ cd rust
|
||||
### Ollama
|
||||
|
||||
```bash
|
||||
export OPENAI_BASE_URL="http://127.0.0.1:11434/v1"
|
||||
unset OPENAI_API_KEY
|
||||
export OLLAMA_HOST="http://127.0.0.1:11434"
|
||||
|
||||
cd rust
|
||||
./target/debug/claw --model "llama3.2" prompt "summarize this repository in one sentence"
|
||||
```
|
||||
|
||||
For Ollama tags with punctuation (for example `qwen2.5-coder:7b`), `OPENAI_BASE_URL` selects the local OpenAI-compatible route even when `OPENAI_API_KEY` is unset:
|
||||
`OLLAMA_HOST` is the preferred env var. Claw routes all models to the local Ollama endpoint automatically, and no API key is needed. The older `OPENAI_BASE_URL` + `OPENAI_API_KEY` workaround is also supported.
|
||||
|
||||
For Ollama tags with punctuation (for example `qwen2.5-coder:7b`), both approaches work:
|
||||
|
||||
```bash
|
||||
export OPENAI_BASE_URL="http://127.0.0.1:11434/v1"
|
||||
unset OPENAI_API_KEY
|
||||
export OLLAMA_HOST="http://127.0.0.1:11434"
|
||||
|
||||
cd rust
|
||||
./target/debug/claw --model "qwen2.5-coder:7b" prompt "reply with ready"
|
||||
@@ -615,6 +616,7 @@ The list is also the precedence chain: project-local settings override project s
|
||||
```
|
||||
|
||||
Object-style matchers are optional. When present, they match tool names case-insensitively and support `*` wildcards plus comma or pipe separated alternatives. Nested hook `type` may be omitted or set to `"command"`; each nested command runs in configuration order.
|
||||
Legacy bare-string hook entries still load for backward compatibility but emit deprecation warnings suggesting migration to object-style entries. Unknown hook event names (e.g. `Stop`, `Notification`) are recorded as invalid without rejecting valid hooks. `status --output-format json` mirrors partial hook validation under `hook_validation` with `valid_count`, `invalid_count`, and `invalid_hooks:[{event, index, hook_index, kind, error_field, reason, valid:false}]`. `doctor --output-format json` includes a `hook validation` check so automation can repair every rejected hook entry without losing usable hooks.
|
||||
|
||||
## Project instruction rules
|
||||
|
||||
|
||||
@@ -57,11 +57,12 @@ ollama serve
|
||||
In another shell:
|
||||
|
||||
```bash
|
||||
export OPENAI_BASE_URL="http://127.0.0.1:11434/v1"
|
||||
unset OPENAI_API_KEY
|
||||
export OLLAMA_HOST="http://127.0.0.1:11434"
|
||||
claw --model "qwen3:latest" prompt "Reply exactly HELLO_WORLD_123"
|
||||
```
|
||||
|
||||
`OLLAMA_HOST` is the preferred env var for Ollama. Claw routes all models to the local OpenAI-compatible endpoint automatically when this is set, and no API key is needed. The older `OPENAI_BASE_URL` + `OPENAI_API_KEY` workaround is also supported for existing setups.
|
||||
|
||||
If Ollama is running without auth, `unset OPENAI_API_KEY` is acceptable. Use a placeholder token rather than a real cloud API key if your local server requires an Authorization header.
|
||||
|
||||
## llama.cpp server
|
||||
|
||||
@@ -151,6 +151,7 @@ Top-level commands:
|
||||
`claw version --output-format json` is the provenance probe for automation: it reports full `git_sha`, derived `git_sha_short`, `is_dirty`, `branch`, `commit_date`, `commit_timestamp`, `rustc_version`, runtime `executable_path`, and `binary_provenance`; the text report is available as `human_readable` instead of a duplicate `message` field.
|
||||
`status --output-format json` reports loaded project memory files under `workspace.memory_files[]` with each file's `path`, `source` (`claude_md`, `claw_md`, `agents_md`, or scoped/rule sources), `origin`, `scope_path`, `outside_project`, `chars`, and `contributes`; `claw doctor --output-format json` includes a dedicated `memory` check. Root instruction-file priority is `CLAUDE.md`, then `CLAW.md`, then `AGENTS.md`, discovery is bounded to the current git root when present (otherwise cwd only), and all non-duplicate loaded files contribute to the rendered system prompt.
|
||||
`claw mcp --output-format json` reports partial MCP config success: valid servers remain in `servers[]` while malformed siblings appear in `invalid_servers[]`, with `total_configured`, `valid_count`, and `invalid_count` split out for automation. `status` mirrors this as `mcp_validation`, and doctor includes an `mcp validation` check.
|
||||
`status --output-format json` also reports partial hook config success under `hook_validation`: valid hook entries are retained while malformed or unknown-event siblings appear in `invalid_hooks[]`, with `valid_count`, `invalid_count`, and typed `kind` fields (`invalid_hooks_config` or `unknown_hook_event`) for automation. `doctor --output-format json` includes a `hook validation` check, and `config --output-format json` includes `hook_validation` metadata with degraded status when invalid entries exist.
|
||||
Shorthand prompt mode honors the POSIX `--` end-of-flags separator, so `claw -- "-prompt-with-dash"` and unknown dash-prefixed non-flag text stay on the prompt path instead of being treated as CLI options.
|
||||
`claw dump-manifests` is self-contained: it emits the Rust resolver inventory for the selected workspace (commands, tools, agents, skills, and bootstrap phases) without requiring an upstream Claude Code TypeScript checkout. Use `--manifests-dir PATH` only to scope resolver discovery to another directory.
|
||||
|
||||
|
||||
@@ -32,16 +32,25 @@ impl ProviderClient {
|
||||
OpenAiCompatConfig::xai(),
|
||||
)?)),
|
||||
ProviderKind::OpenAi => {
|
||||
// DashScope models (qwen-*) also return ProviderKind::OpenAi because they
|
||||
// speak the OpenAI wire format, but they need the DashScope config which
|
||||
// reads DASHSCOPE_API_KEY and points at dashscope.aliyuncs.com.
|
||||
let config = match providers::metadata_for_model(&resolved_model) {
|
||||
Some(meta) if meta.auth_env == "DASHSCOPE_API_KEY" => {
|
||||
OpenAiCompatConfig::dashscope()
|
||||
}
|
||||
_ => OpenAiCompatConfig::openai(),
|
||||
};
|
||||
Ok(Self::OpenAi(OpenAiCompatClient::from_env(config)?))
|
||||
// OLLAMA_HOST takes priority: local Ollama needs no API key
|
||||
// and ignores DashScope/OpenAI env-based dispatch.
|
||||
if std::env::var_os("OLLAMA_HOST").is_some() {
|
||||
Ok(Self::OpenAi(
|
||||
openai_compat::OpenAiCompatClient::from_ollama_env()
|
||||
.expect("from_ollama_env always returns Some"),
|
||||
))
|
||||
} else {
|
||||
// DashScope models (qwen-*) also return ProviderKind::OpenAi because they
|
||||
// speak the OpenAI wire format, but they need the DashScope config which
|
||||
// reads DASHSCOPE_API_KEY and points at dashscope.aliyuncs.com.
|
||||
let config = match providers::metadata_for_model(&resolved_model) {
|
||||
Some(meta) if meta.auth_env == "DASHSCOPE_API_KEY" => {
|
||||
OpenAiCompatConfig::dashscope()
|
||||
}
|
||||
_ => OpenAiCompatConfig::openai(),
|
||||
};
|
||||
Ok(Self::OpenAi(OpenAiCompatClient::from_env(config)?))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ const CONTEXT_WINDOW_ERROR_MARKERS: &[&str] = &[
|
||||
"completion tokens",
|
||||
"prompt tokens",
|
||||
"request is too large",
|
||||
"no parseable body",
|
||||
];
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -60,6 +61,9 @@ pub enum ApiError {
|
||||
retryable: bool,
|
||||
/// Suggested user action based on error type (e.g., "Reduce prompt size" for 413)
|
||||
suggested_action: Option<String>,
|
||||
/// Parsed Retry-After header value (seconds) for 429 responses.
|
||||
/// When present, overrides the exponential backoff delay.
|
||||
retry_after: Option<Duration>,
|
||||
},
|
||||
RetriesExhausted {
|
||||
attempts: u32,
|
||||
@@ -128,6 +132,17 @@ impl ApiError {
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
/// Return the `Retry-After` delay if this error came from a 429 response
|
||||
/// that included a `retry-after` header. Callers should prefer this value
|
||||
/// over the computed backoff delay when it exists.
|
||||
pub fn retry_after(&self) -> Option<Duration> {
|
||||
match self {
|
||||
Self::Api { retry_after, .. } => *retry_after,
|
||||
Self::RetriesExhausted { last_error, .. } => last_error.retry_after(),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn is_retryable(&self) -> bool {
|
||||
match self {
|
||||
Self::Http(error) => error.is_connect() || error.is_timeout() || error.is_request(),
|
||||
@@ -311,6 +326,36 @@ impl Display for ApiError {
|
||||
f,
|
||||
"failed to parse {provider} response for model {model}: {source}; first 200 chars of body: {body_snippet}"
|
||||
),
|
||||
// #28: enhance 401/403 errors with actionable auth guidance
|
||||
Self::Api {
|
||||
status,
|
||||
error_type,
|
||||
message,
|
||||
request_id,
|
||||
body,
|
||||
..
|
||||
} if matches!(status.as_u16(), 401 | 403) => {
|
||||
if let (Some(error_type), Some(message)) = (error_type, message) {
|
||||
write!(f, "api returned {status} ({error_type})")?;
|
||||
if let Some(request_id) = request_id {
|
||||
write!(f, " [trace {request_id}]")?;
|
||||
}
|
||||
write!(f, ": {message}")?;
|
||||
} else {
|
||||
write!(f, "api returned {status}")?;
|
||||
if let Some(request_id) = request_id {
|
||||
write!(f, " [trace {request_id}]")?;
|
||||
}
|
||||
write!(f, ": {body}")?;
|
||||
}
|
||||
write!(
|
||||
f,
|
||||
"\nhint: check that your API key is valid and matches the target provider. \
|
||||
For OpenAI-compatible providers set OPENAI_API_KEY or OPENAI_BASE_URL. \
|
||||
For Anthropic set ANTHROPIC_API_KEY. \
|
||||
Run `claw doctor` to verify your credential configuration."
|
||||
)
|
||||
}
|
||||
Self::Api {
|
||||
status,
|
||||
error_type,
|
||||
@@ -499,6 +544,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: true,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
|
||||
assert!(error.is_generic_fatal_wrapper());
|
||||
@@ -522,6 +568,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: true,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
}),
|
||||
};
|
||||
|
||||
@@ -543,6 +590,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
|
||||
assert!(error.is_context_window_failure());
|
||||
@@ -563,6 +611,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
|
||||
assert!(error.is_context_window_failure());
|
||||
|
||||
@@ -1,9 +1,69 @@
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::error::ApiError;
|
||||
|
||||
const HTTP_PROXY_KEYS: [&str; 2] = ["HTTP_PROXY", "http_proxy"];
|
||||
const HTTPS_PROXY_KEYS: [&str; 2] = ["HTTPS_PROXY", "https_proxy"];
|
||||
const NO_PROXY_KEYS: [&str; 2] = ["NO_PROXY", "no_proxy"];
|
||||
|
||||
/// Timeout configuration for outbound HTTP requests.
|
||||
///
|
||||
/// When set, the `reqwest::Client` will abort requests that take longer
|
||||
/// than the configured duration and return a timeout error (which is
|
||||
/// retryable by the existing exponential backoff logic).
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct TimeoutConfig {
|
||||
/// Maximum time to wait for a connection to be established.
|
||||
/// Defaults to 30 seconds.
|
||||
pub connect_timeout: Duration,
|
||||
/// Maximum time for the entire request (including reading the response
|
||||
/// body). For streaming responses this is the timeout for the initial
|
||||
/// handshake only; the stream itself is governed by SSE parsing.
|
||||
/// Defaults to 5 minutes (300 seconds).
|
||||
pub request_timeout: Duration,
|
||||
}
|
||||
|
||||
impl Default for TimeoutConfig {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
connect_timeout: Duration::from_secs(30),
|
||||
request_timeout: Duration::from_secs(300),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl TimeoutConfig {
|
||||
/// Read timeout settings from the process environment.
|
||||
/// - `CLAW_API_CONNECT_TIMEOUT` — connect timeout in seconds
|
||||
/// - `CLAW_API_REQUEST_TIMEOUT` — overall request timeout in seconds
|
||||
#[must_use]
|
||||
pub fn from_env() -> Self {
|
||||
let connect_timeout = std::env::var("CLAW_API_CONNECT_TIMEOUT")
|
||||
.ok()
|
||||
.and_then(|v| v.parse::<u64>().ok())
|
||||
.map(Duration::from_secs)
|
||||
.unwrap_or(Duration::from_secs(30));
|
||||
let request_timeout = std::env::var("CLAW_API_REQUEST_TIMEOUT")
|
||||
.ok()
|
||||
.and_then(|v| v.parse::<u64>().ok())
|
||||
.map(Duration::from_secs)
|
||||
.unwrap_or(Duration::from_secs(300));
|
||||
Self {
|
||||
connect_timeout,
|
||||
request_timeout,
|
||||
}
|
||||
}
|
||||
|
||||
/// Create from explicit second values (used by config file parsing).
|
||||
#[must_use]
|
||||
pub fn from_seconds(connect_secs: u64, request_secs: u64) -> Self {
|
||||
Self {
|
||||
connect_timeout: Duration::from_secs(connect_secs),
|
||||
request_timeout: Duration::from_secs(request_secs),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Snapshot of the proxy-related environment variables that influence the
|
||||
/// outbound HTTP client. Captured up front so callers can inspect, log, and
|
||||
/// test the resolved configuration without re-reading the process environment.
|
||||
@@ -61,7 +121,7 @@ impl ProxyConfig {
|
||||
/// `HTTPS_PROXY`, and `NO_PROXY` environment variables. When no proxy is
|
||||
/// configured the client behaves identically to `reqwest::Client::new()`.
|
||||
pub fn build_http_client() -> Result<reqwest::Client, ApiError> {
|
||||
build_http_client_with(&ProxyConfig::from_env())
|
||||
build_http_client_with_opts(&ProxyConfig::from_env(), &TimeoutConfig::from_env())
|
||||
}
|
||||
|
||||
/// Infallible counterpart to [`build_http_client`] for constructors that
|
||||
@@ -71,12 +131,13 @@ pub fn build_http_client() -> Result<reqwest::Client, ApiError> {
|
||||
/// first outbound request instead of at construction time.
|
||||
#[must_use]
|
||||
pub fn build_http_client_or_default() -> reqwest::Client {
|
||||
build_http_client().unwrap_or_else(|_| {
|
||||
reqwest::Client::builder()
|
||||
.user_agent("clawd-rust-tools/0.1")
|
||||
.build()
|
||||
.expect("default client with user_agent should always succeed")
|
||||
})
|
||||
build_http_client_with_opts(&ProxyConfig::from_env(), &TimeoutConfig::from_env())
|
||||
.unwrap_or_else(|_| {
|
||||
reqwest::Client::builder()
|
||||
.user_agent("clawd-rust-tools/0.1")
|
||||
.build()
|
||||
.expect("default client with user_agent should always succeed")
|
||||
})
|
||||
}
|
||||
|
||||
/// Build a `reqwest::Client` from an explicit [`ProxyConfig`]. Used by tests
|
||||
@@ -86,9 +147,20 @@ pub fn build_http_client_or_default() -> reqwest::Client {
|
||||
/// and `https_proxy` fields and is registered as both an HTTP and HTTPS
|
||||
/// proxy so a single value can route every outbound request.
|
||||
pub fn build_http_client_with(config: &ProxyConfig) -> Result<reqwest::Client, ApiError> {
|
||||
build_http_client_with_opts(config, &TimeoutConfig::from_env())
|
||||
}
|
||||
|
||||
/// Build a `reqwest::Client` from explicit [`ProxyConfig`] and [`TimeoutConfig`].
|
||||
/// Used by callers that want to control both proxy routing and request timing.
|
||||
pub fn build_http_client_with_opts(
|
||||
config: &ProxyConfig,
|
||||
timeout: &TimeoutConfig,
|
||||
) -> Result<reqwest::Client, ApiError> {
|
||||
let mut builder = reqwest::Client::builder()
|
||||
.no_proxy()
|
||||
.user_agent("clawd-rust-tools/0.1");
|
||||
.user_agent("clawd-rust-tools/0.1")
|
||||
.connect_timeout(timeout.connect_timeout)
|
||||
.timeout(timeout.request_timeout);
|
||||
|
||||
let no_proxy = config
|
||||
.no_proxy
|
||||
@@ -131,7 +203,7 @@ where
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
|
||||
use super::{build_http_client_with, ProxyConfig};
|
||||
use super::{build_http_client_with, build_http_client_with_opts, ProxyConfig, TimeoutConfig};
|
||||
|
||||
fn config_from_map(pairs: &[(&str, &str)]) -> ProxyConfig {
|
||||
let map: HashMap<String, String> = pairs
|
||||
@@ -143,30 +215,19 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn proxy_config_is_empty_when_no_env_vars_are_set() {
|
||||
// given
|
||||
let config = config_from_map(&[]);
|
||||
|
||||
// when
|
||||
let empty = config.is_empty();
|
||||
|
||||
// then
|
||||
assert!(empty);
|
||||
assert!(config.is_empty());
|
||||
assert_eq!(config, ProxyConfig::default());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn proxy_config_reads_uppercase_http_https_and_no_proxy() {
|
||||
// given
|
||||
let pairs = [
|
||||
("HTTP_PROXY", "http://proxy.internal:3128"),
|
||||
("HTTPS_PROXY", "http://secure.internal:3129"),
|
||||
("NO_PROXY", "localhost,127.0.0.1,.corp"),
|
||||
];
|
||||
|
||||
// when
|
||||
let config = config_from_map(&pairs);
|
||||
|
||||
// then
|
||||
assert_eq!(
|
||||
config.http_proxy.as_deref(),
|
||||
Some("http://proxy.internal:3128")
|
||||
@@ -184,17 +245,12 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn proxy_config_falls_back_to_lowercase_keys() {
|
||||
// given
|
||||
let pairs = [
|
||||
("http_proxy", "http://lower.internal:3128"),
|
||||
("https_proxy", "http://lower-secure.internal:3129"),
|
||||
("no_proxy", ".lower"),
|
||||
];
|
||||
|
||||
// when
|
||||
let config = config_from_map(&pairs);
|
||||
|
||||
// then
|
||||
assert_eq!(
|
||||
config.http_proxy.as_deref(),
|
||||
Some("http://lower.internal:3128")
|
||||
@@ -208,16 +264,11 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn proxy_config_prefers_uppercase_over_lowercase_when_both_set() {
|
||||
// given
|
||||
let pairs = [
|
||||
("HTTP_PROXY", "http://upper.internal:3128"),
|
||||
("http_proxy", "http://lower.internal:3128"),
|
||||
];
|
||||
|
||||
// when
|
||||
let config = config_from_map(&pairs);
|
||||
|
||||
// then
|
||||
assert_eq!(
|
||||
config.http_proxy.as_deref(),
|
||||
Some("http://upper.internal:3128")
|
||||
@@ -226,59 +277,39 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn proxy_config_treats_empty_strings_as_unset() {
|
||||
// given
|
||||
let pairs = [("HTTP_PROXY", ""), ("http_proxy", "")];
|
||||
|
||||
// when
|
||||
let config = config_from_map(&pairs);
|
||||
|
||||
// then
|
||||
assert!(config.http_proxy.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_http_client_succeeds_when_no_proxy_is_configured() {
|
||||
// given
|
||||
let config = ProxyConfig::default();
|
||||
|
||||
// when
|
||||
let result = build_http_client_with(&config);
|
||||
|
||||
// then
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_http_client_succeeds_with_valid_http_and_https_proxies() {
|
||||
// given
|
||||
let config = ProxyConfig {
|
||||
http_proxy: Some("http://proxy.internal:3128".to_string()),
|
||||
https_proxy: Some("http://secure.internal:3129".to_string()),
|
||||
no_proxy: Some("localhost,127.0.0.1".to_string()),
|
||||
proxy_url: None,
|
||||
};
|
||||
|
||||
// when
|
||||
let result = build_http_client_with(&config);
|
||||
|
||||
// then
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_http_client_returns_http_error_for_invalid_proxy_url() {
|
||||
// given
|
||||
let config = ProxyConfig {
|
||||
http_proxy: None,
|
||||
https_proxy: Some("not a url".to_string()),
|
||||
no_proxy: None,
|
||||
proxy_url: None,
|
||||
};
|
||||
|
||||
// when
|
||||
let result = build_http_client_with(&config);
|
||||
|
||||
// then
|
||||
let error = result.expect_err("invalid proxy URL must be reported as a build failure");
|
||||
assert!(
|
||||
matches!(error, crate::error::ApiError::Http(_)),
|
||||
@@ -288,10 +319,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn from_proxy_url_sets_unified_field_and_leaves_per_scheme_empty() {
|
||||
// given / when
|
||||
let config = ProxyConfig::from_proxy_url("http://unified.internal:3128");
|
||||
|
||||
// then
|
||||
assert_eq!(
|
||||
config.proxy_url.as_deref(),
|
||||
Some("http://unified.internal:3128")
|
||||
@@ -303,49 +331,56 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn build_http_client_succeeds_with_unified_proxy_url() {
|
||||
// given
|
||||
let config = ProxyConfig {
|
||||
proxy_url: Some("http://unified.internal:3128".to_string()),
|
||||
no_proxy: Some("localhost".to_string()),
|
||||
..ProxyConfig::default()
|
||||
};
|
||||
|
||||
// when
|
||||
let result = build_http_client_with(&config);
|
||||
|
||||
// then
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn proxy_url_takes_precedence_over_per_scheme_fields() {
|
||||
// given – both per-scheme and unified are set
|
||||
let config = ProxyConfig {
|
||||
http_proxy: Some("http://per-scheme.internal:1111".to_string()),
|
||||
https_proxy: Some("http://per-scheme.internal:2222".to_string()),
|
||||
no_proxy: None,
|
||||
proxy_url: Some("http://unified.internal:3128".to_string()),
|
||||
};
|
||||
|
||||
// when – building succeeds (the unified URL is valid)
|
||||
let result = build_http_client_with(&config);
|
||||
|
||||
// then
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_http_client_returns_error_for_invalid_unified_proxy_url() {
|
||||
// given
|
||||
let config = ProxyConfig::from_proxy_url("not a url");
|
||||
|
||||
// when
|
||||
let result = build_http_client_with(&config);
|
||||
|
||||
// then
|
||||
assert!(
|
||||
matches!(result, Err(crate::error::ApiError::Http(_))),
|
||||
"invalid unified proxy URL should fail: {result:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn timeout_config_defaults() {
|
||||
let config = TimeoutConfig::default();
|
||||
assert_eq!(config.connect_timeout, std::time::Duration::from_secs(30));
|
||||
assert_eq!(config.request_timeout, std::time::Duration::from_secs(300));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn timeout_config_from_seconds() {
|
||||
let config = TimeoutConfig::from_seconds(10, 60);
|
||||
assert_eq!(config.connect_timeout, std::time::Duration::from_secs(10));
|
||||
assert_eq!(config.request_timeout, std::time::Duration::from_secs(60));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_http_client_with_custom_timeouts() {
|
||||
let config = ProxyConfig::default();
|
||||
let timeout = TimeoutConfig::from_seconds(5, 120);
|
||||
let result = build_http_client_with_opts(&config, &timeout);
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,7 +12,8 @@ pub use client::{
|
||||
};
|
||||
pub use error::ApiError;
|
||||
pub use http_client::{
|
||||
build_http_client, build_http_client_or_default, build_http_client_with, ProxyConfig,
|
||||
build_http_client, build_http_client_or_default, build_http_client_with,
|
||||
build_http_client_with_opts, ProxyConfig, TimeoutConfig,
|
||||
};
|
||||
pub use prompt_cache::{
|
||||
CacheBreakEvent, PromptCache, PromptCacheConfig, PromptCachePaths, PromptCacheRecord,
|
||||
|
||||
@@ -211,6 +211,19 @@ impl AnthropicClient {
|
||||
self
|
||||
}
|
||||
|
||||
/// Replace the internal HTTP client with one that respects the given
|
||||
/// timeout configuration. This controls connect and request-level
|
||||
/// timeouts for all outbound API calls.
|
||||
#[must_use]
|
||||
pub fn with_timeout(mut self, timeout: &crate::http_client::TimeoutConfig) -> Self {
|
||||
self.http = crate::http_client::build_http_client_with_opts(
|
||||
&crate::http_client::ProxyConfig::from_env(),
|
||||
timeout,
|
||||
)
|
||||
.unwrap_or_else(|_| reqwest::Client::new());
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_session_tracer(mut self, session_tracer: SessionTracer) -> Self {
|
||||
self.session_tracer = Some(session_tracer);
|
||||
@@ -454,7 +467,13 @@ impl AnthropicClient {
|
||||
break;
|
||||
}
|
||||
|
||||
tokio::time::sleep(self.jittered_backoff_for_attempt(attempts)?).await;
|
||||
let delay = if let Some(retry_after) = last_error.as_ref().and_then(|e| e.retry_after())
|
||||
{
|
||||
retry_after
|
||||
} else {
|
||||
self.jittered_backoff_for_attempt(attempts)?
|
||||
};
|
||||
tokio::time::sleep(delay).await;
|
||||
}
|
||||
|
||||
Err(ApiError::RetriesExhausted {
|
||||
@@ -866,10 +885,12 @@ async fn expect_success(response: reqwest::Response) -> Result<reqwest::Response
|
||||
return Ok(response);
|
||||
}
|
||||
|
||||
let request_id = request_id_from_headers(response.headers());
|
||||
let headers = response.headers().clone();
|
||||
let request_id = request_id_from_headers(&headers);
|
||||
let body = response.text().await.unwrap_or_else(|_| String::new());
|
||||
let parsed_error = serde_json::from_str::<AnthropicErrorEnvelope>(&body).ok();
|
||||
let retryable = is_retryable_status(status);
|
||||
let retry_after = parse_retry_after(&headers, status);
|
||||
|
||||
Err(ApiError::Api {
|
||||
status,
|
||||
@@ -883,13 +904,44 @@ async fn expect_success(response: reqwest::Response) -> Result<reqwest::Response
|
||||
body,
|
||||
retryable,
|
||||
suggested_action: None,
|
||||
retry_after,
|
||||
})
|
||||
}
|
||||
|
||||
fn parse_retry_after(
|
||||
headers: &reqwest::header::HeaderMap,
|
||||
status: reqwest::StatusCode,
|
||||
) -> Option<std::time::Duration> {
|
||||
if status != reqwest::StatusCode::TOO_MANY_REQUESTS {
|
||||
return None;
|
||||
}
|
||||
headers
|
||||
.get("retry-after")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.and_then(|v| v.parse::<u64>().ok())
|
||||
.map(std::time::Duration::from_secs)
|
||||
}
|
||||
|
||||
const fn is_retryable_status(status: reqwest::StatusCode) -> bool {
|
||||
matches!(status.as_u16(), 408 | 409 | 429 | 500 | 502 | 503 | 504)
|
||||
}
|
||||
|
||||
/// Some providers return HTTP 400 with an unparseable body when a gateway
|
||||
/// or proxy flakes (e.g. "HTTP 400 from backend (no parseable body)").
|
||||
/// These are transient network blips, not actual bad requests, and should
|
||||
/// be retried. We detect them by checking the body for known gateway error
|
||||
/// phrases.
|
||||
fn is_retryable_400(status: reqwest::StatusCode, body: &str) -> bool {
|
||||
if status != reqwest::StatusCode::BAD_REQUEST {
|
||||
return false;
|
||||
}
|
||||
let lowered = body.to_ascii_lowercase();
|
||||
lowered.contains("no parseable body")
|
||||
|| lowered.contains("connection reset")
|
||||
|| lowered.contains("broken pipe")
|
||||
|| lowered.contains("empty reply from server")
|
||||
}
|
||||
|
||||
/// Anthropic API keys (`sk-ant-*`) are accepted over the `x-api-key` header
|
||||
/// and rejected with HTTP 401 "Invalid bearer token" when sent as a Bearer
|
||||
/// token via `ANTHROPIC_AUTH_TOKEN`. This happens often enough in the wild
|
||||
@@ -908,6 +960,8 @@ fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError {
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
..
|
||||
} = error
|
||||
else {
|
||||
return error;
|
||||
@@ -921,6 +975,7 @@ fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError {
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
};
|
||||
}
|
||||
let Some(bearer_token) = auth.bearer_token() else {
|
||||
@@ -932,6 +987,7 @@ fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError {
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
};
|
||||
};
|
||||
if !bearer_token.starts_with("sk-ant-") {
|
||||
@@ -943,6 +999,7 @@ fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError {
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
};
|
||||
}
|
||||
// Only append the hint when the AuthSource is pure BearerToken. If both
|
||||
@@ -958,6 +1015,7 @@ fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError {
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
};
|
||||
}
|
||||
let enriched_message = match message {
|
||||
@@ -972,6 +1030,7 @@ fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError {
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1596,6 +1655,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
|
||||
// when
|
||||
@@ -1637,6 +1697,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: true,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
|
||||
// when
|
||||
@@ -1666,6 +1727,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
|
||||
// when
|
||||
@@ -1694,6 +1756,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
|
||||
// when
|
||||
@@ -1719,6 +1782,7 @@ mod tests {
|
||||
body: String::new(),
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
|
||||
// when
|
||||
|
||||
@@ -351,6 +351,11 @@ fn looks_like_local_openai_model(model: &str) -> bool {
|
||||
|
||||
#[must_use]
|
||||
pub fn detect_provider_kind(model: &str) -> ProviderKind {
|
||||
// OLLAMA_HOST takes priority: if set, route all models through the local
|
||||
// OpenAI-compatible endpoint regardless of model name or other env vars.
|
||||
if std::env::var_os("OLLAMA_HOST").is_some() {
|
||||
return ProviderKind::OpenAi;
|
||||
}
|
||||
let resolved_model = resolve_model_alias(model);
|
||||
if let Some(metadata) = metadata_for_model(&resolved_model) {
|
||||
return metadata.provider;
|
||||
|
||||
@@ -49,6 +49,14 @@ const XAI_MAX_REQUEST_BODY_BYTES: usize = 52_428_800; // 50MB
|
||||
const OPENAI_MAX_REQUEST_BODY_BYTES: usize = 104_857_600; // 100MB
|
||||
const DASHSCOPE_MAX_REQUEST_BODY_BYTES: usize = 6_291_456; // 6MB (observed limit in dogfood)
|
||||
|
||||
pub const OLLAMA_CONFIG: OpenAiCompatConfig = OpenAiCompatConfig {
|
||||
provider_name: "Ollama",
|
||||
api_key_env: "OLLAMA_HOST",
|
||||
base_url_env: "OLLAMA_HOST",
|
||||
default_base_url: "http://127.0.0.1:11434/v1",
|
||||
max_request_body_bytes: 104_857_600,
|
||||
};
|
||||
|
||||
impl OpenAiCompatConfig {
|
||||
#[must_use]
|
||||
pub const fn xai() -> Self {
|
||||
@@ -149,6 +157,22 @@ impl OpenAiCompatClient {
|
||||
};
|
||||
Ok(Self::new(api_key, config).with_base_url(base_url))
|
||||
}
|
||||
/// Create an Ollama client from `OLLAMA_HOST` env var.
|
||||
/// Ollama requires no API key; a placeholder is used for the Authorization header.
|
||||
pub fn from_ollama_env() -> Option<Self> {
|
||||
let host =
|
||||
std::env::var("OLLAMA_HOST").unwrap_or_else(|_| "http://127.0.0.1:11434".to_string());
|
||||
let base_url = format!("{}/v1", host.trim_end_matches('/'));
|
||||
Some(Self {
|
||||
http: build_http_client_or_default(),
|
||||
api_key: "ollama".to_string(),
|
||||
config: OLLAMA_CONFIG,
|
||||
base_url,
|
||||
max_retries: DEFAULT_MAX_RETRIES,
|
||||
initial_backoff: DEFAULT_INITIAL_BACKOFF,
|
||||
max_backoff: DEFAULT_MAX_BACKOFF,
|
||||
})
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_base_url(mut self, base_url: impl Into<String>) -> Self {
|
||||
@@ -175,6 +199,18 @@ impl OpenAiCompatClient {
|
||||
self
|
||||
}
|
||||
|
||||
/// Replace the internal HTTP client with one that respects the given
|
||||
/// timeout configuration.
|
||||
#[must_use]
|
||||
pub fn with_timeout(mut self, timeout: &crate::http_client::TimeoutConfig) -> Self {
|
||||
self.http = crate::http_client::build_http_client_with_opts(
|
||||
&crate::http_client::ProxyConfig::from_env(),
|
||||
timeout,
|
||||
)
|
||||
.unwrap_or_else(|_| reqwest::Client::new());
|
||||
self
|
||||
}
|
||||
|
||||
pub async fn send_message(
|
||||
&self,
|
||||
request: &MessageRequest,
|
||||
@@ -217,6 +253,7 @@ impl OpenAiCompatClient {
|
||||
reqwest::StatusCode::from_u16(code.unwrap_or(400))
|
||||
.unwrap_or(reqwest::StatusCode::BAD_REQUEST),
|
||||
),
|
||||
retry_after: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -270,7 +307,12 @@ impl OpenAiCompatClient {
|
||||
break retryable_error;
|
||||
}
|
||||
|
||||
tokio::time::sleep(self.jittered_backoff_for_attempt(attempts)?).await;
|
||||
let delay = if let Some(retry_after) = retryable_error.retry_after() {
|
||||
retry_after
|
||||
} else {
|
||||
self.jittered_backoff_for_attempt(attempts)?
|
||||
};
|
||||
tokio::time::sleep(delay).await;
|
||||
};
|
||||
|
||||
Err(ApiError::RetriesExhausted {
|
||||
@@ -1561,6 +1603,7 @@ fn parse_sse_frame(
|
||||
body: trimmed.chars().take(500).collect(),
|
||||
retryable: false,
|
||||
suggested_action: suggested_action_for_status(status),
|
||||
retry_after: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -1576,6 +1619,7 @@ fn parse_sse_frame(
|
||||
body: trimmed.chars().take(200).collect(),
|
||||
retryable: false,
|
||||
suggested_action: Some("verify the API endpoint URL is correct".to_string()),
|
||||
retry_after: None,
|
||||
});
|
||||
}
|
||||
return Ok(None);
|
||||
@@ -1611,6 +1655,7 @@ fn parse_sse_frame(
|
||||
body: payload.clone(),
|
||||
retryable: false,
|
||||
suggested_action: suggested_action_for_status(status),
|
||||
retry_after: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -1627,6 +1672,7 @@ fn parse_sse_frame(
|
||||
body: payload.chars().take(200).collect(),
|
||||
retryable: false,
|
||||
suggested_action: Some("verify the API endpoint URL is correct".to_string()),
|
||||
retry_after: None,
|
||||
});
|
||||
}
|
||||
serde_json::from_str::<ChatCompletionChunk>(&payload)
|
||||
@@ -1678,10 +1724,12 @@ async fn expect_success(response: reqwest::Response) -> Result<reqwest::Response
|
||||
return Ok(response);
|
||||
}
|
||||
|
||||
let request_id = request_id_from_headers(response.headers());
|
||||
let headers = response.headers().clone();
|
||||
let request_id = request_id_from_headers(&headers);
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
let parsed_error = serde_json::from_str::<ErrorEnvelope>(&body).ok();
|
||||
let retryable = is_retryable_status(status);
|
||||
let retry_after = parse_retry_after(&headers, status);
|
||||
|
||||
let suggested_action = suggested_action_for_status(status);
|
||||
|
||||
@@ -1697,13 +1745,43 @@ async fn expect_success(response: reqwest::Response) -> Result<reqwest::Response
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
})
|
||||
}
|
||||
|
||||
fn parse_retry_after(
|
||||
headers: &reqwest::header::HeaderMap,
|
||||
status: reqwest::StatusCode,
|
||||
) -> Option<std::time::Duration> {
|
||||
if status != reqwest::StatusCode::TOO_MANY_REQUESTS {
|
||||
return None;
|
||||
}
|
||||
headers
|
||||
.get("retry-after")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.and_then(|v| v.parse::<u64>().ok())
|
||||
.map(std::time::Duration::from_secs)
|
||||
}
|
||||
|
||||
const fn is_retryable_status(status: reqwest::StatusCode) -> bool {
|
||||
matches!(status.as_u16(), 408 | 409 | 429 | 500 | 502 | 503 | 504)
|
||||
}
|
||||
|
||||
/// Some providers return HTTP 400 with an unparseable body when a gateway
|
||||
/// or proxy flakes (e.g. "HTTP 400 from backend (no parseable body)").
|
||||
/// These are transient network blips, not actual bad requests, and should
|
||||
/// be retried.
|
||||
fn is_retryable_400(status: reqwest::StatusCode, body: &str) -> bool {
|
||||
if status != reqwest::StatusCode::BAD_REQUEST {
|
||||
return false;
|
||||
}
|
||||
let lowered = body.to_ascii_lowercase();
|
||||
lowered.contains("no parseable body")
|
||||
|| lowered.contains("connection reset")
|
||||
|| lowered.contains("broken pipe")
|
||||
|| lowered.contains("empty reply from server")
|
||||
}
|
||||
|
||||
/// Generate a suggested user action based on the HTTP status code and error context.
|
||||
/// This provides actionable guidance when API requests fail.
|
||||
fn suggested_action_for_status(status: reqwest::StatusCode) -> Option<String> {
|
||||
|
||||
@@ -1572,7 +1572,10 @@ fn parse_clear_args(args: &[&str]) -> Result<bool, SlashCommandParseError> {
|
||||
fn parse_config_section(args: &[&str]) -> Result<Option<String>, SlashCommandParseError> {
|
||||
let section = optional_single_arg("config", args, "[env|hooks|model|plugins]")?;
|
||||
if let Some(section) = section {
|
||||
if matches!(section.as_str(), "env" | "hooks" | "model" | "plugins") {
|
||||
if matches!(
|
||||
section.as_str(),
|
||||
"env" | "hooks" | "model" | "plugins" | "help"
|
||||
) {
|
||||
return Ok(Some(section));
|
||||
}
|
||||
return Err(command_error(
|
||||
@@ -2147,7 +2150,7 @@ impl DefinitionSource {
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
struct AgentSummary {
|
||||
pub(crate) struct AgentSummary {
|
||||
name: String,
|
||||
description: Option<String>,
|
||||
model: Option<String>,
|
||||
@@ -2158,6 +2161,20 @@ struct AgentSummary {
|
||||
path: Option<PathBuf>,
|
||||
}
|
||||
|
||||
/// An agent definition file that could not be loaded.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct InvalidAgentConfig {
|
||||
pub(crate) path: PathBuf,
|
||||
pub(crate) reason: String,
|
||||
}
|
||||
|
||||
/// Loaded agent definitions plus any invalid entries that were skipped.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub(crate) struct AgentCollection {
|
||||
pub(crate) agents: Vec<AgentSummary>,
|
||||
pub(crate) invalid_agents: Vec<InvalidAgentConfig>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
struct SkillSummary {
|
||||
name: String,
|
||||
@@ -2167,6 +2184,23 @@ struct SkillSummary {
|
||||
origin: SkillOrigin,
|
||||
// #729: on-disk path parity with AgentSummary
|
||||
path: Option<PathBuf>,
|
||||
// #445: directory name for detecting name/dir mismatch
|
||||
dir_name: Option<String>,
|
||||
}
|
||||
|
||||
/// A skill where the frontmatter name differs from the directory name.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct SkillMetadataDrift {
|
||||
pub(crate) dir_name: String,
|
||||
pub(crate) frontmatter_name: String,
|
||||
pub(crate) path: PathBuf,
|
||||
}
|
||||
|
||||
/// Loaded skill definitions plus any metadata drift entries.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub(crate) struct SkillCollection {
|
||||
pub(crate) skills: Vec<SkillSummary>,
|
||||
pub(crate) metadata_drift: Vec<SkillMetadataDrift>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
@@ -2280,13 +2314,15 @@ pub fn handle_plugins_slash_command(
|
||||
});
|
||||
};
|
||||
let plugin = resolve_plugin_target(manager, target)?;
|
||||
let already_enabled = plugin.enabled;
|
||||
manager.enable(&plugin.metadata.id)?;
|
||||
Ok(PluginsCommandResult {
|
||||
message: format!(
|
||||
"Plugins\n Result enabled {}\n Name {}\n Version {}\n Status enabled",
|
||||
plugin.metadata.id, plugin.metadata.name, plugin.metadata.version
|
||||
"Plugins\n Result {}\n Name {}\n Version {}\n Status enabled",
|
||||
if already_enabled { "already enabled" } else { "enabled" },
|
||||
plugin.metadata.name, plugin.metadata.version
|
||||
),
|
||||
reload_runtime: true,
|
||||
reload_runtime: !already_enabled,
|
||||
})
|
||||
}
|
||||
Some("disable") => {
|
||||
@@ -2297,13 +2333,15 @@ pub fn handle_plugins_slash_command(
|
||||
});
|
||||
};
|
||||
let plugin = resolve_plugin_target(manager, target)?;
|
||||
let already_disabled = !plugin.enabled;
|
||||
manager.disable(&plugin.metadata.id)?;
|
||||
Ok(PluginsCommandResult {
|
||||
message: format!(
|
||||
"Plugins\n Result disabled {}\n Name {}\n Version {}\n Status disabled",
|
||||
plugin.metadata.id, plugin.metadata.name, plugin.metadata.version
|
||||
"Plugins\n Result {}\n Name {}\n Version {}\n Status disabled",
|
||||
if already_disabled { "already disabled" } else { "disabled" },
|
||||
plugin.metadata.name, plugin.metadata.version
|
||||
),
|
||||
reload_runtime: true,
|
||||
reload_runtime: !already_disabled,
|
||||
})
|
||||
}
|
||||
Some("remove") | Some("uninstall") => {
|
||||
@@ -2494,8 +2532,8 @@ pub fn handle_agents_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
||||
match normalize_optional_args(args) {
|
||||
None | Some("list") => {
|
||||
let roots = discover_definition_roots(cwd, "agents");
|
||||
let agents = load_agents_from_roots(&roots)?;
|
||||
Ok(render_agents_report_json(cwd, &agents))
|
||||
let collection = load_agents_from_roots_with_invalids(&roots)?;
|
||||
Ok(render_agents_report_json(cwd, &collection))
|
||||
}
|
||||
Some(args) if args.starts_with("list ") => {
|
||||
let filter = args["list ".len()..].trim().to_lowercase();
|
||||
@@ -2512,17 +2550,26 @@ pub fn handle_agents_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
||||
}));
|
||||
}
|
||||
let roots = discover_definition_roots(cwd, "agents");
|
||||
let agents = load_agents_from_roots(&roots)?;
|
||||
let filtered: Vec<_> = agents
|
||||
let collection = load_agents_from_roots_with_invalids(&roots)?;
|
||||
let filtered_agents: Vec<_> = collection
|
||||
.agents
|
||||
.into_iter()
|
||||
.filter(|a| a.name.to_lowercase().contains(&filter))
|
||||
.collect();
|
||||
Ok(render_agents_report_json(cwd, &filtered))
|
||||
let filtered_collection = AgentCollection {
|
||||
agents: filtered_agents,
|
||||
invalid_agents: collection.invalid_agents,
|
||||
};
|
||||
Ok(render_agents_report_json(cwd, &filtered_collection))
|
||||
}
|
||||
Some("show" | "info" | "describe") => {
|
||||
let roots = discover_definition_roots(cwd, "agents");
|
||||
let agents = load_agents_from_roots(&roots)?;
|
||||
Ok(render_agents_report_json_with_action(cwd, &agents, "show"))
|
||||
let collection = load_agents_from_roots_with_invalids(&roots)?;
|
||||
Ok(render_agents_report_json_with_action(
|
||||
cwd,
|
||||
&collection,
|
||||
"show",
|
||||
))
|
||||
}
|
||||
Some(args)
|
||||
if args.starts_with("show ")
|
||||
@@ -2553,8 +2600,9 @@ pub fn handle_agents_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
||||
}));
|
||||
}
|
||||
let roots = discover_definition_roots(cwd, "agents");
|
||||
let agents = load_agents_from_roots(&roots)?;
|
||||
let matched: Vec<_> = agents
|
||||
let collection = load_agents_from_roots_with_invalids(&roots)?;
|
||||
let matched: Vec<_> = collection
|
||||
.agents
|
||||
.into_iter()
|
||||
.filter(|a| a.name.to_lowercase() == name)
|
||||
.collect();
|
||||
@@ -2571,7 +2619,15 @@ pub fn handle_agents_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
||||
"hint": "Run `claw agents list` to see available agents.",
|
||||
}));
|
||||
}
|
||||
Ok(render_agents_report_json_with_action(cwd, &matched, "show"))
|
||||
let matched_collection = AgentCollection {
|
||||
agents: matched,
|
||||
invalid_agents: collection.invalid_agents,
|
||||
};
|
||||
Ok(render_agents_report_json_with_action(
|
||||
cwd,
|
||||
&matched_collection,
|
||||
"show",
|
||||
))
|
||||
}
|
||||
Some("create") => Ok(render_agents_missing_argument_json("create", "agent_name")),
|
||||
Some(args) if args.starts_with("create ") => {
|
||||
@@ -2704,15 +2760,26 @@ pub fn handle_skills_slash_command(args: Option<&str>, cwd: &Path) -> std::io::R
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"missing_argument: skills install requires an install source.\nUsage: claw skills install <path>",
|
||||
)),
|
||||
// #95: support --project flag for project-level install
|
||||
Some(args) if args.starts_with("install ") => {
|
||||
let target = args["install ".len()..].trim();
|
||||
let rest = args["install ".len()..].trim();
|
||||
let (target, project_flag) = if let Some(t) = rest.strip_prefix("--project") {
|
||||
(t.trim_start().trim_start_matches('=').trim(), true)
|
||||
} else {
|
||||
(rest, false)
|
||||
};
|
||||
if target.is_empty() {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"missing_argument: skills install requires an install source.\nUsage: claw skills install <path>",
|
||||
"missing_argument: skills install requires an install source.\nUsage: claw skills install [--project] <path>",
|
||||
));
|
||||
}
|
||||
let install = install_skill(target, cwd)?;
|
||||
let install = if project_flag {
|
||||
let project_root = cwd.join(".claw").join("skills");
|
||||
install_skill_into(target, cwd, &project_root)?
|
||||
} else {
|
||||
install_skill(target, cwd)?
|
||||
};
|
||||
Ok(render_skill_install_report(&install))
|
||||
}
|
||||
Some("uninstall" | "remove" | "delete") => Err(std::io::Error::new(
|
||||
@@ -2766,8 +2833,8 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
||||
match normalize_optional_args(args) {
|
||||
None | Some("list") => {
|
||||
let roots = discover_skill_roots(cwd);
|
||||
let skills = load_skills_from_roots(&roots)?;
|
||||
Ok(render_skills_report_json_with_action(&skills, "list"))
|
||||
let collection = load_skills_from_roots_with_drift(&roots)?;
|
||||
Ok(render_skills_report_json_with_action(&collection, "list"))
|
||||
}
|
||||
Some(args) if args.starts_with("list ") => {
|
||||
let filter = args["list ".len()..].trim().to_lowercase();
|
||||
@@ -2784,17 +2851,25 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
||||
}));
|
||||
}
|
||||
let roots = discover_skill_roots(cwd);
|
||||
let skills = load_skills_from_roots(&roots)?;
|
||||
let filtered: Vec<_> = skills
|
||||
let collection = load_skills_from_roots_with_drift(&roots)?;
|
||||
let filtered_skills: Vec<_> = collection
|
||||
.skills
|
||||
.into_iter()
|
||||
.filter(|s| s.name.to_lowercase().contains(&filter))
|
||||
.collect();
|
||||
Ok(render_skills_report_json_with_action(&filtered, "list"))
|
||||
let filtered_collection = SkillCollection {
|
||||
skills: filtered_skills,
|
||||
metadata_drift: collection.metadata_drift,
|
||||
};
|
||||
Ok(render_skills_report_json_with_action(
|
||||
&filtered_collection,
|
||||
"list",
|
||||
))
|
||||
}
|
||||
Some("show" | "info" | "describe") => {
|
||||
let roots = discover_skill_roots(cwd);
|
||||
let skills = load_skills_from_roots(&roots)?;
|
||||
Ok(render_skills_report_json_with_action(&skills, "show"))
|
||||
let collection = load_skills_from_roots_with_drift(&roots)?;
|
||||
Ok(render_skills_report_json_with_action(&collection, "show"))
|
||||
}
|
||||
Some(args)
|
||||
if args.starts_with("show ")
|
||||
@@ -2825,8 +2900,9 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
||||
}));
|
||||
}
|
||||
let roots = discover_skill_roots(cwd);
|
||||
let skills = load_skills_from_roots(&roots)?;
|
||||
let matched: Vec<_> = skills
|
||||
let collection = load_skills_from_roots_with_drift(&roots)?;
|
||||
let matched: Vec<_> = collection
|
||||
.skills
|
||||
.into_iter()
|
||||
.filter(|s| s.name.to_lowercase() == name)
|
||||
.collect();
|
||||
@@ -2843,23 +2919,42 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
||||
"hint": "Run `claw skills list` to see available skills.",
|
||||
}));
|
||||
}
|
||||
Ok(render_skills_report_json_with_action(&matched, "show"))
|
||||
let matched_collection = SkillCollection {
|
||||
skills: matched,
|
||||
metadata_drift: collection.metadata_drift,
|
||||
};
|
||||
Ok(render_skills_report_json_with_action(
|
||||
&matched_collection,
|
||||
"show",
|
||||
))
|
||||
}
|
||||
Some("install") => Ok(render_skills_missing_argument_json(
|
||||
"install",
|
||||
"install_source",
|
||||
"Usage: claw skills install <path>",
|
||||
)),
|
||||
// #95: support --project flag for project-level install
|
||||
Some(args) if args.starts_with("install ") => {
|
||||
let target = args["install ".len()..].trim();
|
||||
let rest = args["install ".len()..].trim();
|
||||
let (target, project_flag) = if let Some(t) = rest.strip_prefix("--project") {
|
||||
(t.trim_start().trim_start_matches('=').trim(), true)
|
||||
} else {
|
||||
(rest, false)
|
||||
};
|
||||
if target.is_empty() {
|
||||
return Ok(render_skills_missing_argument_json(
|
||||
"install",
|
||||
"install_source",
|
||||
"Usage: claw skills install <path>",
|
||||
"Usage: claw skills install [--project] <path>",
|
||||
));
|
||||
}
|
||||
match install_skill(target, cwd) {
|
||||
let result = if project_flag {
|
||||
let project_root = cwd.join(".claw").join("skills");
|
||||
install_skill_into(target, cwd, &project_root)
|
||||
} else {
|
||||
install_skill(target, cwd)
|
||||
};
|
||||
match result {
|
||||
Ok(install) => Ok(render_skill_install_report_json(&install)),
|
||||
Err(error) => Ok(render_skill_install_error_json(target, &error)),
|
||||
}
|
||||
@@ -3243,7 +3338,15 @@ fn render_mcp_report_json_for(
|
||||
"use `claw mcp show <server>` to inspect a server",
|
||||
))
|
||||
}
|
||||
Some(args) => Ok(render_mcp_usage_json(Some(args))),
|
||||
Some(args) => {
|
||||
// #681: unsupported mutation verbs (add, remove, delete, enable, disable)
|
||||
// and other unknown sub-actions return a typed error instead of help with exit 0.
|
||||
let verb = args.split_whitespace().next().unwrap_or(args);
|
||||
Ok(render_mcp_unsupported_action_json(
|
||||
args,
|
||||
&format!("`{verb}` is not a supported MCP sub-action; supported actions: list, show, help"),
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3902,30 +4005,69 @@ fn push_unique_skill_root(
|
||||
fn load_agents_from_roots(
|
||||
roots: &[(DefinitionSource, PathBuf)],
|
||||
) -> std::io::Result<Vec<AgentSummary>> {
|
||||
let collection = load_agents_from_roots_with_invalids(roots)?;
|
||||
Ok(collection.agents)
|
||||
}
|
||||
|
||||
/// Load agent definitions from all roots, collecting both valid agents and
|
||||
/// invalid entries (wrong extension, broken frontmatter, etc.).
|
||||
fn load_agents_from_roots_with_invalids(
|
||||
roots: &[(DefinitionSource, PathBuf)],
|
||||
) -> std::io::Result<AgentCollection> {
|
||||
let mut agents = Vec::new();
|
||||
let mut invalid_agents = Vec::new();
|
||||
let mut active_sources = BTreeMap::<String, DefinitionSource>::new();
|
||||
|
||||
for (source, root) in roots {
|
||||
let mut root_agents = Vec::new();
|
||||
for entry in fs::read_dir(root)? {
|
||||
let entry = entry?;
|
||||
if entry.path().extension().is_none_or(|ext| ext != "toml") {
|
||||
continue;
|
||||
let path = entry.path();
|
||||
let ext = path.extension().and_then(|e| e.to_str());
|
||||
match ext {
|
||||
Some("toml") => {
|
||||
let contents = fs::read_to_string(&path)?;
|
||||
let fallback_name = path.file_stem().map_or_else(
|
||||
|| entry.file_name().to_string_lossy().to_string(),
|
||||
|stem| stem.to_string_lossy().to_string(),
|
||||
);
|
||||
root_agents.push(AgentSummary {
|
||||
name: parse_toml_string(&contents, "name").unwrap_or(fallback_name),
|
||||
description: parse_toml_string(&contents, "description"),
|
||||
model: parse_toml_string(&contents, "model"),
|
||||
reasoning_effort: parse_toml_string(&contents, "model_reasoning_effort"),
|
||||
source: *source,
|
||||
shadowed_by: None,
|
||||
path: Some(path),
|
||||
});
|
||||
}
|
||||
Some("md") => {
|
||||
let contents = fs::read_to_string(&path)?;
|
||||
let (name, description, model, reasoning_effort) =
|
||||
parse_agent_frontmatter(&contents);
|
||||
if name.is_none() && description.is_none() {
|
||||
invalid_agents.push(InvalidAgentConfig {
|
||||
path,
|
||||
reason: "Markdown agent file has no YAML frontmatter with name or description fields".to_string(),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
let fallback_name = path.file_stem().map_or_else(
|
||||
|| entry.file_name().to_string_lossy().to_string(),
|
||||
|stem| stem.to_string_lossy().to_string(),
|
||||
);
|
||||
root_agents.push(AgentSummary {
|
||||
name: name.unwrap_or(fallback_name),
|
||||
description,
|
||||
model,
|
||||
reasoning_effort,
|
||||
source: *source,
|
||||
shadowed_by: None,
|
||||
path: Some(path),
|
||||
});
|
||||
}
|
||||
_ => continue,
|
||||
}
|
||||
let contents = fs::read_to_string(entry.path())?;
|
||||
let fallback_name = entry.path().file_stem().map_or_else(
|
||||
|| entry.file_name().to_string_lossy().to_string(),
|
||||
|stem| stem.to_string_lossy().to_string(),
|
||||
);
|
||||
root_agents.push(AgentSummary {
|
||||
name: parse_toml_string(&contents, "name").unwrap_or(fallback_name),
|
||||
description: parse_toml_string(&contents, "description"),
|
||||
model: parse_toml_string(&contents, "model"),
|
||||
reasoning_effort: parse_toml_string(&contents, "model_reasoning_effort"),
|
||||
source: *source,
|
||||
shadowed_by: None,
|
||||
path: Some(entry.path()),
|
||||
});
|
||||
}
|
||||
root_agents.sort_by(|left, right| left.name.cmp(&right.name));
|
||||
|
||||
@@ -3940,11 +4082,22 @@ fn load_agents_from_roots(
|
||||
}
|
||||
}
|
||||
|
||||
Ok(agents)
|
||||
Ok(AgentCollection {
|
||||
agents,
|
||||
invalid_agents,
|
||||
})
|
||||
}
|
||||
|
||||
fn load_skills_from_roots(roots: &[SkillRoot]) -> std::io::Result<Vec<SkillSummary>> {
|
||||
let collection = load_skills_from_roots_with_drift(roots)?;
|
||||
Ok(collection.skills)
|
||||
}
|
||||
|
||||
/// Load skill definitions from all roots, collecting metadata drift entries
|
||||
/// where the frontmatter name differs from the directory name.
|
||||
fn load_skills_from_roots_with_drift(roots: &[SkillRoot]) -> std::io::Result<SkillCollection> {
|
||||
let mut skills = Vec::new();
|
||||
let mut metadata_drift = Vec::new();
|
||||
let mut active_sources = BTreeMap::<String, DefinitionSource>::new();
|
||||
|
||||
for root in roots {
|
||||
@@ -3961,15 +4114,26 @@ fn load_skills_from_roots(roots: &[SkillRoot]) -> std::io::Result<Vec<SkillSumma
|
||||
continue;
|
||||
}
|
||||
let contents = fs::read_to_string(skill_path)?;
|
||||
let dir_name = entry.file_name().to_string_lossy().to_string();
|
||||
let (name, description) = parse_skill_frontmatter(&contents);
|
||||
// #445: detect name/dir mismatch
|
||||
if let Some(ref frontmatter_name) = name {
|
||||
if frontmatter_name != &dir_name {
|
||||
metadata_drift.push(SkillMetadataDrift {
|
||||
dir_name: dir_name.clone(),
|
||||
frontmatter_name: frontmatter_name.clone(),
|
||||
path: entry.path(),
|
||||
});
|
||||
}
|
||||
}
|
||||
root_skills.push(SkillSummary {
|
||||
name: name
|
||||
.unwrap_or_else(|| entry.file_name().to_string_lossy().to_string()),
|
||||
name: name.unwrap_or_else(|| dir_name.clone()),
|
||||
description,
|
||||
source: root.source,
|
||||
shadowed_by: None,
|
||||
origin: root.origin,
|
||||
path: Some(entry.path()),
|
||||
dir_name: Some(dir_name),
|
||||
});
|
||||
}
|
||||
SkillOrigin::LegacyCommandsDir => {
|
||||
@@ -4002,6 +4166,7 @@ fn load_skills_from_roots(roots: &[SkillRoot]) -> std::io::Result<Vec<SkillSumma
|
||||
shadowed_by: None,
|
||||
origin: root.origin,
|
||||
path: Some(markdown_path),
|
||||
dir_name: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -4019,7 +4184,10 @@ fn load_skills_from_roots(roots: &[SkillRoot]) -> std::io::Result<Vec<SkillSumma
|
||||
}
|
||||
}
|
||||
|
||||
Ok(skills)
|
||||
Ok(SkillCollection {
|
||||
skills,
|
||||
metadata_drift,
|
||||
})
|
||||
}
|
||||
|
||||
fn parse_toml_string(contents: &str, key: &str) -> Option<String> {
|
||||
@@ -4091,6 +4259,63 @@ fn unquote_frontmatter_value(value: &str) -> String {
|
||||
.to_string()
|
||||
}
|
||||
|
||||
/// Parse agent metadata from YAML frontmatter in `.md` agent files.
|
||||
/// Returns (name, description, model, reasoning_effort) extracted from
|
||||
/// the `---`-delimited YAML block at the top of the file.
|
||||
fn parse_agent_frontmatter(
|
||||
contents: &str,
|
||||
) -> (
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
Option<String>,
|
||||
) {
|
||||
let mut lines = contents.lines();
|
||||
if lines.next().map(str::trim) != Some("---") {
|
||||
return (None, None, None, None);
|
||||
}
|
||||
|
||||
let mut name = None;
|
||||
let mut description = None;
|
||||
let mut model = None;
|
||||
let mut reasoning_effort = None;
|
||||
for line in lines {
|
||||
let trimmed = line.trim();
|
||||
if trimmed == "---" {
|
||||
break;
|
||||
}
|
||||
if let Some(value) = trimmed.strip_prefix("name:") {
|
||||
let value = unquote_frontmatter_value(value.trim());
|
||||
if !value.is_empty() {
|
||||
name = Some(value);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if let Some(value) = trimmed.strip_prefix("description:") {
|
||||
let value = unquote_frontmatter_value(value.trim());
|
||||
if !value.is_empty() {
|
||||
description = Some(value);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if let Some(value) = trimmed.strip_prefix("model:") {
|
||||
let value = unquote_frontmatter_value(value.trim());
|
||||
if !value.is_empty() {
|
||||
model = Some(value);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if let Some(value) = trimmed.strip_prefix("model_reasoning_effort:") {
|
||||
let value = unquote_frontmatter_value(value.trim());
|
||||
if !value.is_empty() {
|
||||
reasoning_effort = Some(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
(name, description, model, reasoning_effort)
|
||||
}
|
||||
|
||||
fn render_agents_report(agents: &[AgentSummary]) -> String {
|
||||
if agents.is_empty() {
|
||||
return "No agents found.".to_string();
|
||||
@@ -4133,31 +4358,42 @@ fn render_agents_report(agents: &[AgentSummary]) -> String {
|
||||
lines.join("\n").trim_end().to_string()
|
||||
}
|
||||
|
||||
fn render_agents_report_json(cwd: &Path, agents: &[AgentSummary]) -> Value {
|
||||
render_agents_report_json_with_action(cwd, agents, "list")
|
||||
fn render_agents_report_json(cwd: &Path, collection: &AgentCollection) -> Value {
|
||||
render_agents_report_json_with_action(cwd, collection, "list")
|
||||
}
|
||||
|
||||
fn render_agents_report_json_with_action(
|
||||
cwd: &Path,
|
||||
agents: &[AgentSummary],
|
||||
collection: &AgentCollection,
|
||||
action: &str,
|
||||
) -> Value {
|
||||
let agents = &collection.agents;
|
||||
let invalid_agents = &collection.invalid_agents;
|
||||
let active = agents
|
||||
.iter()
|
||||
.filter(|agent| agent.shadowed_by.is_none())
|
||||
.count();
|
||||
let has_invalids = !invalid_agents.is_empty();
|
||||
let status = if has_invalids { "degraded" } else { "ok" };
|
||||
json!({
|
||||
"kind": "agents",
|
||||
"status": "ok",
|
||||
"status": status,
|
||||
"action": action,
|
||||
"working_directory": cwd.display().to_string(),
|
||||
"count": agents.len(),
|
||||
"valid_count": agents.len(),
|
||||
"invalid_count": invalid_agents.len(),
|
||||
"summary": {
|
||||
"total": agents.len(),
|
||||
"active": active,
|
||||
"shadowed": agents.len().saturating_sub(active),
|
||||
},
|
||||
"agents": agents.iter().map(agent_summary_json).collect::<Vec<_>>(),
|
||||
"invalid_agents": invalid_agents.iter().map(|invalid| json!({
|
||||
"path": invalid.path.display().to_string(),
|
||||
"reason": &invalid.reason,
|
||||
"valid": false,
|
||||
})).collect::<Vec<_>>(),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -4277,21 +4513,34 @@ fn render_skills_report(skills: &[SkillSummary]) -> String {
|
||||
lines.join("\n").trim_end().to_string()
|
||||
}
|
||||
|
||||
fn render_skills_report_json_with_action(skills: &[SkillSummary], action: &str) -> Value {
|
||||
fn render_skills_report_json_with_action(collection: &SkillCollection, action: &str) -> Value {
|
||||
let skills = &collection.skills;
|
||||
let metadata_drift = &collection.metadata_drift;
|
||||
let active = skills
|
||||
.iter()
|
||||
.filter(|skill| skill.shadowed_by.is_none())
|
||||
.count();
|
||||
let has_drift = !metadata_drift.is_empty();
|
||||
let status = if has_drift { "degraded" } else { "ok" };
|
||||
// #410: add `count` field for polymorphic consumption parity with agents list
|
||||
json!({
|
||||
"kind": "skills",
|
||||
"status": "ok",
|
||||
"status": status,
|
||||
"action": action,
|
||||
"count": skills.len(),
|
||||
"valid_count": skills.len(),
|
||||
"metadata_drift_count": metadata_drift.len(),
|
||||
"summary": {
|
||||
"total": skills.len(),
|
||||
"active": active,
|
||||
"shadowed": skills.len().saturating_sub(active),
|
||||
},
|
||||
"skills": skills.iter().map(skill_summary_json).collect::<Vec<_>>(),
|
||||
"metadata_drift": metadata_drift.iter().map(|drift| json!({
|
||||
"dir_name": &drift.dir_name,
|
||||
"frontmatter_name": &drift.frontmatter_name,
|
||||
"path": drift.path.display().to_string(),
|
||||
})).collect::<Vec<_>>(),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -4467,6 +4716,7 @@ fn render_mcp_summary_report_json(cwd: &Path, mcp: &McpConfigCollection) -> Valu
|
||||
json!({
|
||||
"kind": "mcp",
|
||||
"action": "list",
|
||||
"count": mcp.valid_count(),
|
||||
"working_directory": cwd.display().to_string(),
|
||||
"configured_servers": mcp.valid_count(),
|
||||
"total_configured": mcp.total_configured(),
|
||||
@@ -4652,7 +4902,7 @@ fn render_agents_usage_json(unexpected: Option<&str>) -> Value {
|
||||
"direct_cli": "claw agents [list|show <name>|create <name>|help]",
|
||||
"format": "toml",
|
||||
"create": "claw agents create <name>",
|
||||
"sources": [".claw/agents", "~/.claw/agents", "$CLAW_CONFIG_HOME/agents"],
|
||||
"sources": [".claw/agents", "~/.claw/agents", "~/.codex/agents", "$CLAW_CONFIG_HOME/agents"],
|
||||
},
|
||||
"unexpected": unexpected,
|
||||
})
|
||||
@@ -4661,12 +4911,12 @@ fn render_agents_usage_json(unexpected: Option<&str>) -> Value {
|
||||
fn render_skills_usage(unexpected: Option<&str>) -> String {
|
||||
let mut lines = vec![
|
||||
"Skills".to_string(),
|
||||
" Usage /skills [list|show <name>|install <path>|uninstall <name>|help|<skill> [args]]".to_string(),
|
||||
" Usage /skills [list|show <name>|install [--project] <path>|uninstall <name>|help|<skill> [args]]".to_string(),
|
||||
" Alias /skill".to_string(),
|
||||
" Direct CLI claw skills [list|show <name>|install <path>|uninstall <name>|help|<skill> [args]]".to_string(),
|
||||
" Direct CLI claw skills [list|show <name>|install [--project] <path>|uninstall <name>|help|<skill> [args]]".to_string(),
|
||||
" Lifecycle install <path>, uninstall <name>".to_string(),
|
||||
" Invoke /skills help overview -> $help overview".to_string(),
|
||||
" Install root $CLAW_CONFIG_HOME/skills or ~/.claw/skills".to_string(),
|
||||
" Install root $CLAW_CONFIG_HOME/skills or ~/.claw/skills (use --project for .claw/skills)".to_string(),
|
||||
" Sources .claw/skills, .omc/skills, .agents/skills, .codex/skills, .claude/skills, ~/.claw/skills, ~/.omc/skills, ~/.claude/skills/omc-learned, ~/.codex/skills, ~/.claude/skills, legacy /commands".to_string(),
|
||||
];
|
||||
if let Some(args) = unexpected {
|
||||
@@ -4782,7 +5032,7 @@ fn render_mcp_usage_json(unexpected: Option<&str>) -> Value {
|
||||
"usage": {
|
||||
"slash_command": "/mcp [list|show <server>|help]",
|
||||
"direct_cli": "claw mcp [list|show <server>|help]",
|
||||
"sources": [".claw/settings.json", ".claw/settings.local.json"],
|
||||
"sources": [".claw.json", ".claw/settings.json", ".claw/settings.local.json"],
|
||||
},
|
||||
"unexpected": unexpected,
|
||||
})
|
||||
@@ -4970,34 +5220,51 @@ fn mcp_oauth_json(oauth: Option<&McpOAuthConfig>) -> Value {
|
||||
}
|
||||
|
||||
fn mcp_server_details_json(config: &McpServerConfig) -> Value {
|
||||
// #90: redact sensitive fields — args/url/headers_helper can contain
|
||||
// credentials. Show structure without leaking secrets.
|
||||
match config {
|
||||
McpServerConfig::Stdio(config) => json!({
|
||||
"command": &config.command,
|
||||
"args": &config.args,
|
||||
"args_count": config.args.len(),
|
||||
"env_keys": config.env.keys().cloned().collect::<Vec<_>>(),
|
||||
"tool_call_timeout_ms": config.tool_call_timeout_ms,
|
||||
}),
|
||||
McpServerConfig::Sse(config) | McpServerConfig::Http(config) => json!({
|
||||
"url": &config.url,
|
||||
"header_keys": config.headers.keys().cloned().collect::<Vec<_>>(),
|
||||
"headers_helper": &config.headers_helper,
|
||||
"oauth": mcp_oauth_json(config.oauth.as_ref()),
|
||||
}),
|
||||
McpServerConfig::Ws(config) => json!({
|
||||
"url": &config.url,
|
||||
"header_keys": config.headers.keys().cloned().collect::<Vec<_>>(),
|
||||
"headers_helper": &config.headers_helper,
|
||||
}),
|
||||
McpServerConfig::Sse(config) | McpServerConfig::Http(config) => {
|
||||
let redacted_url = redact_url(&config.url);
|
||||
json!({
|
||||
"url": redacted_url,
|
||||
"header_keys": config.headers.keys().cloned().collect::<Vec<_>>(),
|
||||
"headers_helper_configured": config.headers_helper.is_some(),
|
||||
"oauth": mcp_oauth_json(config.oauth.as_ref()),
|
||||
})
|
||||
}
|
||||
McpServerConfig::Ws(config) => {
|
||||
let redacted_url = redact_url(&config.url);
|
||||
json!({
|
||||
"url": redacted_url,
|
||||
"header_keys": config.headers.keys().cloned().collect::<Vec<_>>(),
|
||||
"headers_helper_configured": config.headers_helper.is_some(),
|
||||
})
|
||||
}
|
||||
McpServerConfig::Sdk(config) => json!({
|
||||
"name": &config.name,
|
||||
}),
|
||||
McpServerConfig::ManagedProxy(config) => json!({
|
||||
"url": &config.url,
|
||||
"url": redact_url(&config.url),
|
||||
"id": &config.id,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
fn redact_url(url: &str) -> String {
|
||||
// #90: strip query params which may contain tokens, keep scheme+host+path
|
||||
if let Some(query_start) = url.find('?') {
|
||||
format!("{}?...", &url[..query_start])
|
||||
} else {
|
||||
url.to_string()
|
||||
}
|
||||
}
|
||||
|
||||
fn mcp_server_json(name: &str, server: &ScopedMcpServerConfig) -> Value {
|
||||
json!({
|
||||
"name": name,
|
||||
@@ -5127,7 +5394,7 @@ mod tests {
|
||||
render_agents_report_json, render_mcp_report_json_for, render_plugins_report,
|
||||
render_plugins_report_with_failures, render_skills_report, render_slash_command_help,
|
||||
render_slash_command_help_detail, resolve_skill_path, resume_supported_slash_commands,
|
||||
slash_command_specs, suggest_slash_commands, validate_slash_command_input,
|
||||
slash_command_specs, suggest_slash_commands, validate_slash_command_input, AgentCollection,
|
||||
DefinitionSource, SkillOrigin, SkillRoot, SkillSlashDispatch, SlashCommand,
|
||||
};
|
||||
use plugins::{
|
||||
@@ -6121,7 +6388,10 @@ mod tests {
|
||||
];
|
||||
let report = render_agents_report_json(
|
||||
&workspace,
|
||||
&load_agents_from_roots(&roots).expect("agent roots should load"),
|
||||
&AgentCollection {
|
||||
agents: load_agents_from_roots(&roots).expect("agent roots should load"),
|
||||
invalid_agents: Vec::new(),
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(report["kind"], "agents");
|
||||
@@ -6274,7 +6544,10 @@ mod tests {
|
||||
},
|
||||
];
|
||||
let report = super::render_skills_report_json_with_action(
|
||||
&load_skills_from_roots(&roots).expect("skills should load"),
|
||||
&super::SkillCollection {
|
||||
skills: load_skills_from_roots(&roots).expect("skills should load"),
|
||||
metadata_drift: Vec::new(),
|
||||
},
|
||||
"list",
|
||||
);
|
||||
assert_eq!(report["kind"], "skills");
|
||||
@@ -6352,12 +6625,13 @@ mod tests {
|
||||
let skills_help =
|
||||
super::handle_skills_slash_command(Some("--help"), &cwd).expect("skills help");
|
||||
assert!(skills_help.contains(
|
||||
"Usage /skills [list|show <name>|install <path>|uninstall <name>|help|<skill> [args]]"
|
||||
"Usage /skills [list|show <name>|install [--project] <path>|uninstall <name>|help|<skill> [args]]"
|
||||
));
|
||||
assert!(skills_help.contains("Alias /skill"));
|
||||
assert!(skills_help.contains("Lifecycle install <path>, uninstall <name>"));
|
||||
assert!(skills_help.contains("Invoke /skills help overview -> $help overview"));
|
||||
assert!(skills_help.contains("Install root $CLAW_CONFIG_HOME/skills or ~/.claw/skills"));
|
||||
// #95: install root now mentions --project flag
|
||||
assert!(skills_help.contains("Install root $CLAW_CONFIG_HOME/skills or ~/.claw/skills (use --project for .claw/skills)"));
|
||||
assert!(skills_help.contains(".omc/skills"));
|
||||
assert!(skills_help.contains(".agents/skills"));
|
||||
assert!(skills_help.contains("~/.claude/skills/omc-learned"));
|
||||
@@ -6370,7 +6644,7 @@ mod tests {
|
||||
let skills_install_help = super::handle_skills_slash_command(Some("install --help"), &cwd)
|
||||
.expect("nested skills help");
|
||||
assert!(skills_install_help.contains(
|
||||
"Usage /skills [list|show <name>|install <path>|uninstall <name>|help|<skill> [args]]"
|
||||
"Usage /skills [list|show <name>|install [--project] <path>|uninstall <name>|help|<skill> [args]]"
|
||||
));
|
||||
assert!(skills_install_help.contains("Alias /skill"));
|
||||
assert!(skills_install_help.contains("Unexpected install"));
|
||||
@@ -6378,7 +6652,7 @@ mod tests {
|
||||
let skills_unknown_help =
|
||||
super::handle_skills_slash_command(Some("show --help"), &cwd).expect("skills help");
|
||||
assert!(skills_unknown_help.contains(
|
||||
"Usage /skills [list|show <name>|install <path>|uninstall <name>|help|<skill> [args]]"
|
||||
"Usage /skills [list|show <name>|install [--project] <path>|uninstall <name>|help|<skill> [args]]"
|
||||
));
|
||||
assert!(skills_unknown_help.contains("Unexpected show"));
|
||||
|
||||
@@ -6647,7 +6921,7 @@ mod tests {
|
||||
let help =
|
||||
render_mcp_report_json_for(&loader, &workspace, Some("help")).expect("mcp help json");
|
||||
assert_eq!(help["action"], "help");
|
||||
assert_eq!(help["usage"]["sources"][0], ".claw/settings.json");
|
||||
assert_eq!(help["usage"]["sources"][0], ".claw.json");
|
||||
|
||||
let _ = fs::remove_dir_all(workspace);
|
||||
let _ = fs::remove_dir_all(config_home);
|
||||
@@ -6845,7 +7119,7 @@ mod tests {
|
||||
let disable = handle_plugins_slash_command(Some("disable"), Some("demo"), &mut manager)
|
||||
.expect("disable command should succeed");
|
||||
assert!(disable.reload_runtime);
|
||||
assert!(disable.message.contains("disabled demo@external"));
|
||||
assert!(disable.message.contains("Result disabled"));
|
||||
assert!(disable.message.contains("Name demo"));
|
||||
assert!(disable.message.contains("Status disabled"));
|
||||
|
||||
@@ -6857,7 +7131,7 @@ mod tests {
|
||||
let enable = handle_plugins_slash_command(Some("enable"), Some("demo"), &mut manager)
|
||||
.expect("enable command should succeed");
|
||||
assert!(enable.reload_runtime);
|
||||
assert!(enable.message.contains("enabled demo@external"));
|
||||
assert!(enable.message.contains("Result enabled"));
|
||||
assert!(enable.message.contains("Name demo"));
|
||||
assert!(enable.message.contains("Status enabled"));
|
||||
|
||||
|
||||
@@ -125,6 +125,27 @@ pub struct RuntimePluginConfig {
|
||||
max_output_tokens: Option<u32>,
|
||||
}
|
||||
|
||||
/// API timeout and retry configuration.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ApiTimeoutConfig {
|
||||
/// Connect timeout in seconds. Defaults to 30.
|
||||
pub connect_timeout_secs: u64,
|
||||
/// Request timeout in seconds. Defaults to 300 (5 minutes).
|
||||
pub request_timeout_secs: u64,
|
||||
/// Maximum retry attempts on transient failures. Defaults to 8.
|
||||
pub max_retries: u32,
|
||||
}
|
||||
|
||||
impl Default for ApiTimeoutConfig {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
connect_timeout_secs: 30,
|
||||
request_timeout_secs: 300,
|
||||
max_retries: 8,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Structured feature configuration consumed by runtime subsystems.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Default)]
|
||||
pub struct RuntimeFeatureConfig {
|
||||
@@ -139,6 +160,7 @@ pub struct RuntimeFeatureConfig {
|
||||
sandbox: SandboxConfig,
|
||||
provider_fallbacks: ProviderFallbackConfig,
|
||||
trusted_roots: Vec<String>,
|
||||
api_timeout: ApiTimeoutConfig,
|
||||
rules_import: RulesImportConfig,
|
||||
}
|
||||
|
||||
@@ -182,6 +204,7 @@ pub struct RuntimeHookConfig {
|
||||
pre_tool_use: Vec<RuntimeHookCommand>,
|
||||
post_tool_use: Vec<RuntimeHookCommand>,
|
||||
post_tool_use_failure: Vec<RuntimeHookCommand>,
|
||||
invalid_hooks: Vec<RuntimeInvalidHookConfig>,
|
||||
}
|
||||
|
||||
/// A hook command plus optional tool matcher from object-style hook config.
|
||||
@@ -191,6 +214,16 @@ pub struct RuntimeHookCommand {
|
||||
matcher: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct RuntimeInvalidHookConfig {
|
||||
pub event: String,
|
||||
pub index: Option<usize>,
|
||||
pub hook_index: Option<usize>,
|
||||
pub kind: String,
|
||||
pub error_field: String,
|
||||
pub reason: String,
|
||||
}
|
||||
|
||||
/// Raw permission rule lists grouped by allow, deny, and ask behavior.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Default)]
|
||||
pub struct RuntimePermissionRuleConfig {
|
||||
@@ -729,6 +762,7 @@ fn build_runtime_config(
|
||||
sandbox: parse_optional_sandbox_config(&merged_value)?,
|
||||
provider_fallbacks: parse_optional_provider_fallbacks(&merged_value)?,
|
||||
trusted_roots: parse_optional_trusted_roots(&merged_value)?,
|
||||
api_timeout: parse_optional_api_timeout_config(&merged_value)?,
|
||||
rules_import: parse_optional_rules_import(&merged_value)?,
|
||||
};
|
||||
|
||||
@@ -1198,6 +1232,7 @@ impl RuntimeHookConfig {
|
||||
pre_tool_use,
|
||||
post_tool_use,
|
||||
post_tool_use_failure,
|
||||
invalid_hooks: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1235,6 +1270,8 @@ impl RuntimeHookConfig {
|
||||
&mut self.post_tool_use_failure,
|
||||
other.post_tool_use_failure_entries(),
|
||||
);
|
||||
self.invalid_hooks
|
||||
.extend(other.invalid_hooks.iter().cloned());
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
@@ -1246,6 +1283,25 @@ impl RuntimeHookConfig {
|
||||
pub fn post_tool_use_failure_entries(&self) -> &[RuntimeHookCommand] {
|
||||
&self.post_tool_use_failure
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn invalid_hooks(&self) -> &[RuntimeInvalidHookConfig] {
|
||||
&self.invalid_hooks
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn invalid_count(&self) -> usize {
|
||||
self.invalid_hooks.len()
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn has_invalid_hooks(&self) -> bool {
|
||||
!self.invalid_hooks.is_empty()
|
||||
}
|
||||
|
||||
pub fn push_invalid_hook(&mut self, invalid: RuntimeInvalidHookConfig) {
|
||||
self.invalid_hooks.push(invalid);
|
||||
}
|
||||
}
|
||||
|
||||
fn hook_commands(commands: &[RuntimeHookCommand]) -> Vec<String> {
|
||||
@@ -1634,14 +1690,217 @@ fn parse_optional_hooks_config_object(
|
||||
return Ok(RuntimeHookConfig::default());
|
||||
};
|
||||
let hooks = expect_object(hooks_value, context)?;
|
||||
Ok(RuntimeHookConfig {
|
||||
pre_tool_use: optional_hook_command_array(hooks, "PreToolUse", context)?
|
||||
.unwrap_or_default(),
|
||||
post_tool_use: optional_hook_command_array(hooks, "PostToolUse", context)?
|
||||
.unwrap_or_default(),
|
||||
post_tool_use_failure: optional_hook_command_array(hooks, "PostToolUseFailure", context)?
|
||||
.unwrap_or_default(),
|
||||
})
|
||||
Ok(parse_hooks_object_partial(hooks, context))
|
||||
}
|
||||
|
||||
fn parse_hooks_object_partial(
|
||||
hooks: &BTreeMap<String, JsonValue>,
|
||||
context: &str,
|
||||
) -> RuntimeHookConfig {
|
||||
let mut config = RuntimeHookConfig::default();
|
||||
parse_hook_event_partial(
|
||||
&mut config,
|
||||
hooks,
|
||||
"PreToolUse",
|
||||
context,
|
||||
|config, command| {
|
||||
config.pre_tool_use.push(command);
|
||||
},
|
||||
);
|
||||
parse_hook_event_partial(
|
||||
&mut config,
|
||||
hooks,
|
||||
"PostToolUse",
|
||||
context,
|
||||
|config, command| {
|
||||
config.post_tool_use.push(command);
|
||||
},
|
||||
);
|
||||
parse_hook_event_partial(
|
||||
&mut config,
|
||||
hooks,
|
||||
"PostToolUseFailure",
|
||||
context,
|
||||
|config, command| {
|
||||
config.post_tool_use_failure.push(command);
|
||||
},
|
||||
);
|
||||
for event in hooks.keys().filter(|event| !is_supported_hook_event(event)) {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.clone(),
|
||||
index: None,
|
||||
hook_index: None,
|
||||
kind: "unknown_hook_event".to_string(),
|
||||
error_field: event.clone(),
|
||||
reason: format!("{context}: unknown hook event {event}"),
|
||||
});
|
||||
}
|
||||
config
|
||||
}
|
||||
|
||||
fn is_supported_hook_event(event: &str) -> bool {
|
||||
matches!(event, "PreToolUse" | "PostToolUse" | "PostToolUseFailure")
|
||||
}
|
||||
|
||||
fn parse_hook_event_partial(
|
||||
config: &mut RuntimeHookConfig,
|
||||
hooks: &BTreeMap<String, JsonValue>,
|
||||
event: &str,
|
||||
context: &str,
|
||||
mut push_command: impl FnMut(&mut RuntimeHookConfig, RuntimeHookCommand),
|
||||
) {
|
||||
let Some(value) = hooks.get(event) else {
|
||||
return;
|
||||
};
|
||||
let Some(array) = value.as_array() else {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index: None,
|
||||
hook_index: None,
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: event.to_string(),
|
||||
reason: format!("{context}: field {event} must be an array"),
|
||||
});
|
||||
return;
|
||||
};
|
||||
|
||||
for (index, item) in array.iter().enumerate() {
|
||||
if let Some(command) = item.as_str() {
|
||||
if command.trim().is_empty() {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index: Some(index),
|
||||
hook_index: None,
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: "command".to_string(),
|
||||
reason: format!("{context}: field {event}[{index}] must be a non-empty string"),
|
||||
});
|
||||
} else {
|
||||
push_command(config, RuntimeHookCommand::new(command.to_string()));
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
let Some(entry) = item.as_object() else {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index: Some(index),
|
||||
hook_index: None,
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: event.to_string(),
|
||||
reason: format!(
|
||||
"{context}: field {event}[{index}] must be a string or hook object"
|
||||
),
|
||||
});
|
||||
continue;
|
||||
};
|
||||
|
||||
let matcher = match optional_hook_matcher(entry, context, event, index) {
|
||||
Ok(matcher) => matcher,
|
||||
Err(error) => {
|
||||
config.push_invalid_hook(runtime_invalid_hook(
|
||||
event,
|
||||
Some(index),
|
||||
None,
|
||||
"matcher",
|
||||
error,
|
||||
));
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let Some(hook_array) = entry.get("hooks").and_then(JsonValue::as_array) else {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index: Some(index),
|
||||
hook_index: None,
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: "hooks".to_string(),
|
||||
reason: format!("{context}: field {event}[{index}].hooks must be an array"),
|
||||
});
|
||||
continue;
|
||||
};
|
||||
for (hook_index, hook) in hook_array.iter().enumerate() {
|
||||
let Some(hook_object) = hook.as_object() else {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index: Some(index),
|
||||
hook_index: Some(hook_index),
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: "hooks".to_string(),
|
||||
reason: format!(
|
||||
"{context}: field {event}[{index}].hooks[{hook_index}] must be an object"
|
||||
),
|
||||
});
|
||||
continue;
|
||||
};
|
||||
if let Some(hook_type) = hook_object.get("type") {
|
||||
let Some(hook_type) = hook_type.as_str() else {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index: Some(index),
|
||||
hook_index: Some(hook_index),
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: "type".to_string(),
|
||||
reason: format!(
|
||||
"{context}: field {event}[{index}].hooks[{hook_index}].type must be a string"
|
||||
),
|
||||
});
|
||||
continue;
|
||||
};
|
||||
if hook_type != "command" {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index: Some(index),
|
||||
hook_index: Some(hook_index),
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: "type".to_string(),
|
||||
reason: format!(
|
||||
"{context}: field {event}[{index}].hooks[{hook_index}].type must be \"command\""
|
||||
),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
}
|
||||
let Some(command) = hook_object
|
||||
.get("command")
|
||||
.and_then(JsonValue::as_str)
|
||||
.filter(|command| !command.trim().is_empty())
|
||||
else {
|
||||
config.push_invalid_hook(RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index: Some(index),
|
||||
hook_index: Some(hook_index),
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: "command".to_string(),
|
||||
reason: format!(
|
||||
"{context}: field {event}[{index}].hooks[{hook_index}].command must be a non-empty string"
|
||||
),
|
||||
});
|
||||
continue;
|
||||
};
|
||||
push_command(
|
||||
config,
|
||||
RuntimeHookCommand::with_matcher(command.to_string(), matcher.clone()),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn runtime_invalid_hook(
|
||||
event: &str,
|
||||
index: Option<usize>,
|
||||
hook_index: Option<usize>,
|
||||
error_field: &str,
|
||||
error: ConfigError,
|
||||
) -> RuntimeInvalidHookConfig {
|
||||
RuntimeInvalidHookConfig {
|
||||
event: event.to_string(),
|
||||
index,
|
||||
hook_index,
|
||||
kind: "invalid_hooks_config".to_string(),
|
||||
error_field: error_field.to_string(),
|
||||
reason: config_error_detail(&error),
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_optional_hooks_config(
|
||||
@@ -1784,6 +2043,26 @@ fn parse_optional_provider_fallbacks(
|
||||
Ok(ProviderFallbackConfig { primary, fallbacks })
|
||||
}
|
||||
|
||||
fn parse_optional_api_timeout_config(root: &JsonValue) -> Result<ApiTimeoutConfig, ConfigError> {
|
||||
let Some(timeout_value) = root.as_object().and_then(|obj| obj.get("apiTimeout")) else {
|
||||
return Ok(ApiTimeoutConfig::default());
|
||||
};
|
||||
let Some(obj) = timeout_value.as_object() else {
|
||||
return Ok(ApiTimeoutConfig::default());
|
||||
};
|
||||
let context = "merged settings.apiTimeout";
|
||||
let connect_timeout_secs = optional_u64(obj, "connectTimeout", context)?.unwrap_or(30);
|
||||
let request_timeout_secs = optional_u64(obj, "requestTimeout", context)?.unwrap_or(300);
|
||||
let max_retries = optional_u64(obj, "maxRetries", context)?
|
||||
.map(|v| v as u32)
|
||||
.unwrap_or(8);
|
||||
Ok(ApiTimeoutConfig {
|
||||
connect_timeout_secs,
|
||||
request_timeout_secs,
|
||||
max_retries,
|
||||
})
|
||||
}
|
||||
|
||||
fn parse_optional_trusted_roots(root: &JsonValue) -> Result<Vec<String>, ConfigError> {
|
||||
let Some(object) = root.as_object() else {
|
||||
return Ok(Vec::new());
|
||||
@@ -1861,6 +2140,45 @@ fn parse_optional_oauth_config(
|
||||
}))
|
||||
}
|
||||
|
||||
/// #92: expand `${VAR}` environment variable references and `~/` home directory
|
||||
/// prefix in a config string value. Returns the expanded string.
|
||||
fn expand_config_value(value: &str) -> String {
|
||||
// Expand ${VAR} and $VAR references from the environment
|
||||
let mut result = String::with_capacity(value.len());
|
||||
let mut chars = value.chars().peekable();
|
||||
while let Some(c) = chars.next() {
|
||||
if c == '$' {
|
||||
if chars.peek() == Some(&'{') {
|
||||
// ${VAR} form
|
||||
chars.next(); // consume '{'
|
||||
let mut var_name = String::new();
|
||||
for ch in chars.by_ref() {
|
||||
if ch == '}' {
|
||||
break;
|
||||
}
|
||||
var_name.push(ch);
|
||||
}
|
||||
if let Ok(val) = std::env::var(&var_name) {
|
||||
result.push_str(&val);
|
||||
}
|
||||
} else {
|
||||
// Bare $ — pass through
|
||||
result.push(c);
|
||||
}
|
||||
} else if c == '~' && result.is_empty() {
|
||||
// ~/... home directory expansion
|
||||
if let Ok(home) = std::env::var("HOME") {
|
||||
result.push_str(&home);
|
||||
} else {
|
||||
result.push(c);
|
||||
}
|
||||
} else {
|
||||
result.push(c);
|
||||
}
|
||||
}
|
||||
result
|
||||
}
|
||||
|
||||
fn parse_mcp_server_config(
|
||||
server_name: &str,
|
||||
value: &JsonValue,
|
||||
@@ -1870,9 +2188,14 @@ fn parse_mcp_server_config(
|
||||
let server_type =
|
||||
optional_string(object, "type", context)?.unwrap_or_else(|| infer_mcp_server_type(object));
|
||||
match server_type {
|
||||
// #92: expand ${VAR} and ~/ in command, args, and url fields
|
||||
"stdio" => Ok(McpServerConfig::Stdio(McpStdioServerConfig {
|
||||
command: expect_non_empty_string(object, "command", context)?.to_string(),
|
||||
args: optional_string_array(object, "args", context)?.unwrap_or_default(),
|
||||
command: expand_config_value(expect_non_empty_string(object, "command", context)?),
|
||||
args: optional_string_array(object, "args", context)?
|
||||
.unwrap_or_default()
|
||||
.iter()
|
||||
.map(|a| expand_config_value(a))
|
||||
.collect(),
|
||||
env: optional_string_map(object, "env", context)?.unwrap_or_default(),
|
||||
tool_call_timeout_ms: optional_u64(object, "toolCallTimeoutMs", context)?,
|
||||
})),
|
||||
@@ -1883,7 +2206,8 @@ fn parse_mcp_server_config(
|
||||
object, context,
|
||||
)?)),
|
||||
"ws" => Ok(McpServerConfig::Ws(McpWebSocketServerConfig {
|
||||
url: expect_string(object, "url", context)?.to_string(),
|
||||
// #92: expand ${VAR} and ~/ in URL
|
||||
url: expand_config_value(expect_string(object, "url", context)?),
|
||||
headers: optional_string_map(object, "headers", context)?.unwrap_or_default(),
|
||||
headers_helper: optional_string(object, "headersHelper", context)?.map(str::to_string),
|
||||
})),
|
||||
@@ -1891,7 +2215,8 @@ fn parse_mcp_server_config(
|
||||
name: expect_string(object, "name", context)?.to_string(),
|
||||
})),
|
||||
"claudeai-proxy" => Ok(McpServerConfig::ManagedProxy(McpManagedProxyServerConfig {
|
||||
url: expect_string(object, "url", context)?.to_string(),
|
||||
// #92: expand ${VAR} and ~/ in URL
|
||||
url: expand_config_value(expect_string(object, "url", context)?),
|
||||
id: expect_string(object, "id", context)?.to_string(),
|
||||
})),
|
||||
other => Err(ConfigError::Parse(format!(
|
||||
@@ -1913,7 +2238,8 @@ fn parse_mcp_remote_server_config(
|
||||
context: &str,
|
||||
) -> Result<McpRemoteServerConfig, ConfigError> {
|
||||
Ok(McpRemoteServerConfig {
|
||||
url: expect_string(object, "url", context)?.to_string(),
|
||||
// #92: expand ${VAR} and ~/ in URL
|
||||
url: expand_config_value(expect_string(object, "url", context)?),
|
||||
headers: optional_string_map(object, "headers", context)?.unwrap_or_default(),
|
||||
headers_helper: optional_string(object, "headersHelper", context)?.map(str::to_string),
|
||||
oauth: parse_optional_mcp_oauth_config(object, context)?,
|
||||
@@ -2108,77 +2434,6 @@ fn optional_string_array(
|
||||
}
|
||||
}
|
||||
|
||||
fn optional_hook_command_array(
|
||||
object: &BTreeMap<String, JsonValue>,
|
||||
key: &str,
|
||||
context: &str,
|
||||
) -> Result<Option<Vec<RuntimeHookCommand>>, ConfigError> {
|
||||
let Some(value) = object.get(key) else {
|
||||
return Ok(None);
|
||||
};
|
||||
let Some(array) = value.as_array() else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key} must be an array"
|
||||
)));
|
||||
};
|
||||
|
||||
let mut commands = Vec::new();
|
||||
for (index, item) in array.iter().enumerate() {
|
||||
if let Some(command) = item.as_str() {
|
||||
commands.push(RuntimeHookCommand::new(command.to_string()));
|
||||
continue;
|
||||
}
|
||||
|
||||
let Some(entry) = item.as_object() else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key}[{index}] must be a string or hook object"
|
||||
)));
|
||||
};
|
||||
let matcher = optional_hook_matcher(entry, context, key, index)?;
|
||||
let hooks = entry
|
||||
.get("hooks")
|
||||
.and_then(JsonValue::as_array)
|
||||
.ok_or_else(|| {
|
||||
ConfigError::Parse(format!(
|
||||
"{context}: field {key}[{index}].hooks must be an array"
|
||||
))
|
||||
})?;
|
||||
for (hook_index, hook) in hooks.iter().enumerate() {
|
||||
let Some(hook_object) = hook.as_object() else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key}[{index}].hooks[{hook_index}] must be an object"
|
||||
)));
|
||||
};
|
||||
if let Some(hook_type) = hook_object.get("type") {
|
||||
let Some(hook_type) = hook_type.as_str() else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key}[{index}].hooks[{hook_index}].type must be a string"
|
||||
)));
|
||||
};
|
||||
if hook_type != "command" {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key}[{index}].hooks[{hook_index}].type must be \"command\""
|
||||
)));
|
||||
}
|
||||
}
|
||||
let command = hook_object
|
||||
.get("command")
|
||||
.and_then(JsonValue::as_str)
|
||||
.filter(|command| !command.trim().is_empty())
|
||||
.ok_or_else(|| {
|
||||
ConfigError::Parse(format!(
|
||||
"{context}: field {key}[{index}].hooks[{hook_index}].command must be a non-empty string"
|
||||
))
|
||||
})?;
|
||||
commands.push(RuntimeHookCommand::with_matcher(
|
||||
command.to_string(),
|
||||
matcher.clone(),
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(Some(commands))
|
||||
}
|
||||
|
||||
fn optional_hook_matcher(
|
||||
entry: &BTreeMap<String, JsonValue>,
|
||||
context: &str,
|
||||
@@ -2247,6 +2502,10 @@ fn deep_merge_objects(
|
||||
(Some(JsonValue::Object(existing)), JsonValue::Object(incoming)) => {
|
||||
deep_merge_objects(existing, incoming);
|
||||
}
|
||||
// #106: concatenate arrays instead of replacing
|
||||
(Some(JsonValue::Array(existing)), JsonValue::Array(incoming)) => {
|
||||
existing.extend(incoming.iter().cloned());
|
||||
}
|
||||
_ => {
|
||||
target.insert(key.clone(), value.clone());
|
||||
}
|
||||
@@ -2428,7 +2687,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_object_style_hook_entries_without_command() {
|
||||
fn records_object_style_hook_entries_without_command_441() {
|
||||
let root = temp_dir();
|
||||
let cwd = root.join("project");
|
||||
let home = root.join("home").join(".claw");
|
||||
@@ -2440,12 +2699,20 @@ mod tests {
|
||||
)
|
||||
.expect("write settings");
|
||||
|
||||
let error = ConfigLoader::new(&cwd, &home)
|
||||
let loaded = ConfigLoader::new(&cwd, &home)
|
||||
.load()
|
||||
.expect_err("config should reject malformed hook entry");
|
||||
.expect("config should load valid siblings and record malformed hook entry");
|
||||
|
||||
assert!(error
|
||||
.to_string()
|
||||
assert!(loaded.hooks().pre_tool_use().is_empty());
|
||||
assert_eq!(loaded.hooks().invalid_count(), 1);
|
||||
assert_eq!(
|
||||
loaded.hooks().invalid_hooks()[0].kind,
|
||||
"invalid_hooks_config"
|
||||
);
|
||||
assert_eq!(loaded.hooks().invalid_hooks()[0].event, "PreToolUse");
|
||||
assert_eq!(loaded.hooks().invalid_hooks()[0].error_field, "command");
|
||||
assert!(loaded.hooks().invalid_hooks()[0]
|
||||
.reason
|
||||
.contains("command must be a non-empty string"));
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
@@ -3188,7 +3455,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_invalid_hook_entries_before_merge() {
|
||||
fn loads_valid_hook_entries_and_records_invalid_siblings_441() {
|
||||
// given
|
||||
let root = temp_dir();
|
||||
let cwd = root.join("project");
|
||||
@@ -3208,19 +3475,26 @@ mod tests {
|
||||
)
|
||||
.expect("write invalid project settings");
|
||||
|
||||
// when
|
||||
let error = ConfigLoader::new(&cwd, &home)
|
||||
let loaded = ConfigLoader::new(&cwd, &home)
|
||||
.load()
|
||||
.expect_err("config should fail");
|
||||
.expect("config should load valid hook entries and record invalid siblings");
|
||||
|
||||
// then — config validation now catches the mixed array before the hooks parser
|
||||
let rendered = error.to_string();
|
||||
assert!(
|
||||
rendered.contains("hooks.PreToolUse")
|
||||
&& rendered.contains("must be an array of strings"),
|
||||
"expected validation error for hooks.PreToolUse, got: {rendered}"
|
||||
// #106: arrays now concatenate across config layers, so both "base" and "project" are present
|
||||
assert_eq!(
|
||||
loaded.hooks().pre_tool_use(),
|
||||
&["base".to_string(), "project".to_string()]
|
||||
);
|
||||
assert!(!rendered.contains("merged settings.hooks"));
|
||||
assert_eq!(loaded.hooks().invalid_count(), 1);
|
||||
assert_eq!(loaded.hooks().invalid_hooks()[0].event, "PreToolUse");
|
||||
assert_eq!(
|
||||
loaded.hooks().invalid_hooks()[0].kind,
|
||||
"invalid_hooks_config"
|
||||
);
|
||||
// #106: invalid entry at index 2 after array concatenation
|
||||
assert_eq!(loaded.hooks().invalid_hooks()[0].index, Some(2));
|
||||
assert!(loaded.hooks().invalid_hooks()[0]
|
||||
.reason
|
||||
.contains("must be a string or hook object"));
|
||||
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
@@ -3363,7 +3637,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validates_wrong_type_for_known_field_with_field_path() {
|
||||
fn hook_event_wrong_type_is_recorded_without_config_failure_441() {
|
||||
// given
|
||||
let root = temp_dir();
|
||||
let cwd = root.join("project");
|
||||
@@ -3377,29 +3651,145 @@ mod tests {
|
||||
)
|
||||
.expect("write user settings");
|
||||
|
||||
// when
|
||||
let error = ConfigLoader::new(&cwd, &home)
|
||||
let loaded = ConfigLoader::new(&cwd, &home)
|
||||
.load()
|
||||
.expect_err("config should fail");
|
||||
.expect("config should record malformed hook event without failing");
|
||||
|
||||
// then
|
||||
let rendered = error.to_string();
|
||||
assert!(
|
||||
rendered.contains(&user_settings.display().to_string()),
|
||||
"error should include file path, got: {rendered}"
|
||||
assert!(loaded.hooks().pre_tool_use().is_empty());
|
||||
assert_eq!(loaded.hooks().invalid_count(), 1);
|
||||
assert_eq!(loaded.hooks().invalid_hooks()[0].event, "PreToolUse");
|
||||
assert_eq!(
|
||||
loaded.hooks().invalid_hooks()[0].kind,
|
||||
"invalid_hooks_config"
|
||||
);
|
||||
assert_eq!(loaded.hooks().invalid_hooks()[0].index, None);
|
||||
assert!(loaded.hooks().invalid_hooks()[0]
|
||||
.reason
|
||||
.contains("field PreToolUse must be an array"));
|
||||
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collects_all_invalid_hook_siblings_instead_of_halting_at_first_441() {
|
||||
// ROADMAP #441 finding (c): first-error-only halting means users must fix
|
||||
// one hook at a time. After #441 partial fix, all invalid entries in the
|
||||
// same config are collected.
|
||||
let root = temp_dir();
|
||||
let cwd = root.join("project");
|
||||
let home = root.join("home").join(".claw");
|
||||
fs::create_dir_all(&home).expect("home config dir");
|
||||
fs::create_dir_all(&cwd).expect("project dir");
|
||||
fs::write(
|
||||
home.join("settings.json"),
|
||||
r#"{"hooks":{"PreToolUse":[42],"PostToolUse":"not-an-array","InvalidEvent":["cmd"]}}"#,
|
||||
)
|
||||
.expect("write settings");
|
||||
|
||||
let loaded = ConfigLoader::new(&cwd, &home)
|
||||
.load()
|
||||
.expect("config should collect all invalid hooks without halting at first");
|
||||
|
||||
assert!(loaded.hooks().pre_tool_use().is_empty());
|
||||
assert!(loaded.hooks().post_tool_use().is_empty());
|
||||
// Three distinct invalid entries: 42, wrong type, unknown event
|
||||
assert_eq!(loaded.hooks().invalid_count(), 3);
|
||||
|
||||
let invalid = loaded.hooks().invalid_hooks();
|
||||
// PreToolUse[0]=42
|
||||
assert_eq!(invalid[0].event, "PreToolUse");
|
||||
assert_eq!(invalid[0].index, Some(0));
|
||||
assert_eq!(invalid[0].kind, "invalid_hooks_config");
|
||||
// PostToolUse wrong type
|
||||
assert_eq!(invalid[1].event, "PostToolUse");
|
||||
assert_eq!(invalid[1].index, None);
|
||||
assert_eq!(invalid[1].kind, "invalid_hooks_config");
|
||||
// Unknown event
|
||||
assert_eq!(invalid[2].event, "InvalidEvent");
|
||||
assert_eq!(invalid[2].index, None);
|
||||
assert_eq!(invalid[2].kind, "unknown_hook_event");
|
||||
assert!(invalid[2]
|
||||
.reason
|
||||
.contains("unknown hook event InvalidEvent"));
|
||||
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_hook_events_recorded_with_correct_kind_441() {
|
||||
// ROADMAP #441 finding (a): unknown event names like Stop/Notification
|
||||
// should not reject entire hooks config; they are recorded as invalid.
|
||||
let root = temp_dir();
|
||||
let cwd = root.join("project");
|
||||
let home = root.join("home").join(".claw");
|
||||
fs::create_dir_all(&home).expect("home config dir");
|
||||
fs::create_dir_all(&cwd).expect("project dir");
|
||||
fs::write(
|
||||
home.join("settings.json"),
|
||||
r#"{"hooks":{"PreToolUse":["valid-cmd"],"Stop":"not-an-array","Notification":[{}]}}"#,
|
||||
)
|
||||
.expect("write settings");
|
||||
|
||||
let loaded = ConfigLoader::new(&cwd, &home)
|
||||
.load()
|
||||
.expect("config should load valid hooks and record unknown event siblings");
|
||||
|
||||
// Valid PreToolUse hook should load
|
||||
assert_eq!(loaded.hooks().pre_tool_use(), &["valid-cmd".to_string()]);
|
||||
// Stop and Notification are unknown events; each gets one invalid entry
|
||||
// Notification:[{}] also has an empty-object entry issue but since we
|
||||
// don't parse unknown events, only the unknown-event invalid is recorded
|
||||
let invalid = loaded.hooks().invalid_hooks();
|
||||
assert!(
|
||||
rendered.contains("hooks"),
|
||||
"error should include field path component 'hooks', got: {rendered}"
|
||||
invalid.len() >= 2,
|
||||
"expected at least 2 invalid hooks, got {}",
|
||||
invalid.len()
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("PreToolUse"),
|
||||
"error should describe the type mismatch, got: {rendered}"
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("array"),
|
||||
"error should describe the expected type, got: {rendered}"
|
||||
|
||||
let stop = invalid
|
||||
.iter()
|
||||
.find(|h| h.event == "Stop")
|
||||
.expect("Stop invalid hook");
|
||||
assert_eq!(stop.kind, "unknown_hook_event");
|
||||
assert_eq!(stop.index, None);
|
||||
assert!(stop.reason.contains("unknown hook event Stop"));
|
||||
|
||||
let notif = invalid
|
||||
.iter()
|
||||
.find(|h| h.event == "Notification")
|
||||
.expect("Notification invalid hook");
|
||||
assert_eq!(notif.kind, "unknown_hook_event");
|
||||
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn documented_claude_code_hook_format_loads_without_error_441() {
|
||||
// ROADMAP #441: the Claude Code documented hook format
|
||||
// {"hooks":{"PreToolUse":[{"matcher":"Read","hooks":[{"type":"command","command":"..."}]}]}}
|
||||
// must load without config_load_error.
|
||||
let root = temp_dir();
|
||||
let cwd = root.join("project");
|
||||
let home = root.join("home").join(".claw");
|
||||
fs::create_dir_all(&home).expect("home config dir");
|
||||
fs::create_dir_all(&cwd).expect("project dir");
|
||||
fs::write(
|
||||
home.join("settings.json"),
|
||||
r#"{"hooks":{"PreToolUse":[{"matcher":"Read","hooks":[{"type":"command","command":"/bin/echo pretool"}]}]}}"#,
|
||||
)
|
||||
.expect("write settings");
|
||||
|
||||
let loaded = ConfigLoader::new(&cwd, &home)
|
||||
.load()
|
||||
.expect("Claude Code documented hook format must load without error");
|
||||
|
||||
assert_eq!(
|
||||
loaded.hooks().pre_tool_use(),
|
||||
&["/bin/echo pretool".to_string()]
|
||||
);
|
||||
assert_eq!(loaded.hooks().invalid_count(), 0);
|
||||
let entries = loaded.hooks().pre_tool_use_entries();
|
||||
assert_eq!(entries[0].matcher(), Some("Read"));
|
||||
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
|
||||
@@ -118,10 +118,7 @@ impl FieldType {
|
||||
Self::StringArray => value
|
||||
.as_array()
|
||||
.is_some_and(|arr| arr.iter().all(|v| v.as_str().is_some())),
|
||||
Self::HookArray => value.as_array().is_some_and(|arr| {
|
||||
arr.iter()
|
||||
.all(|entry| entry.as_str().is_some() || entry.as_object().is_some())
|
||||
}),
|
||||
Self::HookArray => true,
|
||||
Self::RulesImport => {
|
||||
value.as_str().is_some()
|
||||
|| value
|
||||
@@ -439,8 +436,56 @@ fn validate_object_keys(
|
||||
result
|
||||
}
|
||||
|
||||
/// Emit deprecation warnings for bare string hook entries in the hooks object.
|
||||
/// Legacy `["command-string"]` arrays still load but suggest migration to the
|
||||
/// structured `{matcher, hooks:[{type, command}]}` form.
|
||||
fn validate_hook_entry_format(
|
||||
hooks: &BTreeMap<String, JsonValue>,
|
||||
source: &str,
|
||||
path_display: &str,
|
||||
) -> ValidationResult {
|
||||
let mut result = ValidationResult {
|
||||
errors: Vec::new(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
for spec in HOOKS_FIELDS {
|
||||
let Some(value) = hooks.get(spec.name) else {
|
||||
continue;
|
||||
};
|
||||
let Some(array) = value.as_array() else {
|
||||
continue;
|
||||
};
|
||||
for item in array {
|
||||
if item.as_str().is_some() {
|
||||
result.warnings.push(ConfigDiagnostic {
|
||||
path: path_display.to_string(),
|
||||
field: format!("hooks.{}", spec.name),
|
||||
line: find_key_line(source, spec.name),
|
||||
kind: DiagnosticKind::Deprecated {
|
||||
replacement: "object-style hook entries with hooks:[{type:\"command\",command:\"...\"}]",
|
||||
},
|
||||
});
|
||||
// One deprecation warning per event is enough
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
result
|
||||
}
|
||||
|
||||
fn suggest_field(input: &str, candidates: &[&str]) -> Option<String> {
|
||||
let input_lower = input.to_ascii_lowercase();
|
||||
// #461: prefix-aware matching — if input is a prefix of a candidate,
|
||||
// treat it as distance 0 (perfect prefix match) to avoid edit-distance
|
||||
// misranking (e.g., "mcp" → "env" instead of "mcpServers").
|
||||
let prefix_match = candidates
|
||||
.iter()
|
||||
.filter(|c| c.to_ascii_lowercase().starts_with(&input_lower))
|
||||
.min_by_key(|c| c.len())
|
||||
.map(|name| name.to_string());
|
||||
if prefix_match.is_some() {
|
||||
return prefix_match;
|
||||
}
|
||||
candidates
|
||||
.iter()
|
||||
.filter_map(|candidate| {
|
||||
@@ -510,6 +555,7 @@ pub fn validate_config_file(
|
||||
source,
|
||||
&path_display,
|
||||
));
|
||||
result.merge(validate_hook_entry_format(hooks, source, &path_display));
|
||||
}
|
||||
if let Some(permissions) = object.get("permissions").and_then(JsonValue::as_object) {
|
||||
result.merge(validate_object_keys(
|
||||
@@ -714,7 +760,7 @@ mod tests {
|
||||
#[test]
|
||||
fn validates_nested_hooks_keys() {
|
||||
// given
|
||||
let source = r#"{"hooks": {"PreToolUse": ["cmd"], "BadHook": ["x"]}}"#;
|
||||
let source = r#"{"hooks": {"PreToolUse": [{"hooks":[{"type":"command","command":"cmd"}]}], "BadHook": ["x"]}}"#;
|
||||
let parsed = JsonValue::parse(source).expect("valid json");
|
||||
let object = parsed.as_object().expect("object");
|
||||
|
||||
@@ -723,7 +769,12 @@ mod tests {
|
||||
|
||||
// then
|
||||
assert!(result.errors.is_empty());
|
||||
assert_eq!(result.warnings.len(), 1);
|
||||
assert_eq!(
|
||||
result.warnings.len(),
|
||||
1,
|
||||
"expected only the unknown key warning, got {:?}",
|
||||
result.warnings
|
||||
);
|
||||
assert_eq!(result.warnings[0].field, "hooks.BadHook");
|
||||
}
|
||||
|
||||
@@ -739,15 +790,14 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_wrong_hook_entry_types() {
|
||||
fn allows_wrong_hook_entry_types_for_partial_runtime_validation_441() {
|
||||
let source = r#"{"hooks":{"PreToolUse":[42]}}"#;
|
||||
let parsed = JsonValue::parse(source).expect("valid json");
|
||||
let object = parsed.as_object().expect("object");
|
||||
|
||||
let result = validate_config_file(object, source, &test_path());
|
||||
|
||||
assert_eq!(result.errors.len(), 1);
|
||||
assert_eq!(result.errors[0].field, "hooks.PreToolUse");
|
||||
assert!(result.errors.is_empty(), "{:?}", result.errors);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -847,7 +897,7 @@ mod tests {
|
||||
// given
|
||||
let source = r#"{
|
||||
"model": "opus",
|
||||
"hooks": {"PreToolUse": ["guard"]},
|
||||
"hooks": {"PreToolUse": [{"hooks":[{"type":"command","command":"guard"}]}]},
|
||||
"permissions": {"defaultMode": "plan", "allow": ["Read"]},
|
||||
"mcpServers": {},
|
||||
"sandbox": {"enabled": false}
|
||||
|
||||
@@ -204,6 +204,13 @@ where
|
||||
self
|
||||
}
|
||||
|
||||
/// Update the auto-compaction threshold after construction. This allows the
|
||||
/// caller to tune the threshold based on runtime information (e.g., the
|
||||
/// server-returned context window size from a 400 error).
|
||||
pub fn set_auto_compaction_input_tokens_threshold(&mut self, threshold: u32) {
|
||||
self.auto_compaction_input_tokens_threshold = threshold;
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_hook_abort_signal(mut self, hook_abort_signal: HookAbortSignal) -> Self {
|
||||
self.hook_abort_signal = hook_abort_signal;
|
||||
|
||||
@@ -65,14 +65,14 @@ pub use compact::{
|
||||
get_compact_continuation_message, should_compact, CompactionConfig, CompactionResult,
|
||||
};
|
||||
pub use config::{
|
||||
suppress_config_warnings_for_json_mode, ConfigEntry, ConfigError, ConfigFileReport,
|
||||
ConfigFileStatus, ConfigInspection, ConfigLoader, ConfigSource, McpConfigCollection,
|
||||
McpInvalidServerConfig, McpManagedProxyServerConfig, McpOAuthConfig, McpRemoteServerConfig,
|
||||
McpSdkServerConfig, McpServerConfig, McpStdioServerConfig, McpTransport,
|
||||
suppress_config_warnings_for_json_mode, ApiTimeoutConfig, ConfigEntry, ConfigError,
|
||||
ConfigFileReport, ConfigFileStatus, ConfigInspection, ConfigLoader, ConfigSource,
|
||||
McpConfigCollection, McpInvalidServerConfig, McpManagedProxyServerConfig, McpOAuthConfig,
|
||||
McpRemoteServerConfig, McpSdkServerConfig, McpServerConfig, McpStdioServerConfig, McpTransport,
|
||||
McpWebSocketServerConfig, OAuthConfig, ProviderFallbackConfig, ResolvedPermissionMode,
|
||||
RulesImportConfig, RuntimeConfig, RuntimeFeatureConfig, RuntimeHookCommand, RuntimeHookConfig,
|
||||
RuntimePermissionRuleConfig, RuntimePluginConfig, ScopedMcpServerConfig,
|
||||
CLAW_SETTINGS_SCHEMA_NAME,
|
||||
RuntimeInvalidHookConfig, RuntimePermissionRuleConfig, RuntimePluginConfig,
|
||||
ScopedMcpServerConfig, CLAW_SETTINGS_SCHEMA_NAME,
|
||||
};
|
||||
pub use config_validate::{
|
||||
check_unsupported_format, format_diagnostics, validate_config_file, ConfigDiagnostic,
|
||||
|
||||
@@ -173,32 +173,112 @@ impl PermissionEnforcer {
|
||||
}
|
||||
}
|
||||
|
||||
/// Simple workspace boundary check via string prefix.
|
||||
/// Workspace boundary check.
|
||||
///
|
||||
/// Resolves `.` and `..` components lexically *before* comparing against the
|
||||
/// workspace root, so that traversal sequences like `/workspace/../../etc`
|
||||
/// cannot escape the sandbox via a naive string prefix match. Normalization is
|
||||
/// lexical (it does not touch the filesystem) because the target path may not
|
||||
/// exist yet on a write, and we must not depend on CWD.
|
||||
fn is_within_workspace(path: &str, workspace_root: &str) -> bool {
|
||||
let normalized = if path.starts_with('/') {
|
||||
let combined = if path.starts_with('/') {
|
||||
path.to_owned()
|
||||
} else {
|
||||
format!("{workspace_root}/{path}")
|
||||
};
|
||||
|
||||
let root = if workspace_root.ends_with('/') {
|
||||
workspace_root.to_owned()
|
||||
let normalized = lexically_normalize(&combined);
|
||||
let root = lexically_normalize(workspace_root);
|
||||
let root_with_slash = if root.ends_with('/') {
|
||||
root.clone()
|
||||
} else {
|
||||
format!("{workspace_root}/")
|
||||
format!("{root}/")
|
||||
};
|
||||
|
||||
normalized.starts_with(&root) || normalized == workspace_root.trim_end_matches('/')
|
||||
normalized == root || normalized.starts_with(&root_with_slash)
|
||||
}
|
||||
|
||||
/// Collapse `.` and `..` segments without consulting the filesystem.
|
||||
/// `..` that would climb above an absolute root is clamped at `/`, so the
|
||||
/// result can never be a prefix-match for a deeper workspace root.
|
||||
fn lexically_normalize(path: &str) -> String {
|
||||
let is_absolute = path.starts_with('/');
|
||||
let mut stack: Vec<&str> = Vec::new();
|
||||
for component in path.split('/') {
|
||||
match component {
|
||||
"" | "." => {}
|
||||
".." => {
|
||||
stack.pop();
|
||||
}
|
||||
other => stack.push(other),
|
||||
}
|
||||
}
|
||||
let joined = stack.join("/");
|
||||
if is_absolute {
|
||||
format!("/{joined}")
|
||||
} else {
|
||||
joined
|
||||
}
|
||||
}
|
||||
|
||||
/// Conservative heuristic: is this bash command read-only?
|
||||
///
|
||||
/// Hardening notes:
|
||||
/// - Any shell metacharacter that could chain, substitute, pipe, or redirect
|
||||
/// into a state-changing command rejects the whole line. This blocks
|
||||
/// `cat x; rm -rf y`, `cat x | sh`, `$(...)`, backticks, redirects, and
|
||||
/// subshells regardless of the leading token.
|
||||
/// - Language interpreters (`python`, `node`, `ruby`) and build drivers
|
||||
/// (`cargo`, `rustc`) are NOT read-only: they execute arbitrary code, so they
|
||||
/// are excluded from the allow-list.
|
||||
/// - `git` is allowed only for a known set of non-mutating subcommands.
|
||||
/// - `find` is rejected when it carries an action that can execute or delete.
|
||||
///
|
||||
/// Residual known gaps (documented, not yet closed): `sed`'s `w`/`e` script
|
||||
/// commands and `awk`'s `system()` can still mutate — these require quoting or
|
||||
/// metacharacters that the checks above usually catch, but a dedicated parser
|
||||
/// would be more robust. Tracked as follow-up.
|
||||
fn is_read_only_command(command: &str) -> bool {
|
||||
let first_token = command
|
||||
.split_whitespace()
|
||||
.next()
|
||||
.unwrap_or("")
|
||||
.rsplit('/')
|
||||
.next()
|
||||
.unwrap_or("");
|
||||
// Shell metacharacters that enable command chaining, substitution,
|
||||
// piping, redirection, or subshells. Presence of any of these means we
|
||||
// cannot reason about the command from its leading token alone.
|
||||
const SHELL_METACHARS: &[char] = &[';', '|', '&', '$', '`', '>', '<', '(', ')', '{', '}', '\n'];
|
||||
if command.contains(SHELL_METACHARS) {
|
||||
return false;
|
||||
}
|
||||
|
||||
let mut tokens = command.split_whitespace();
|
||||
let first_token = tokens.next().unwrap_or("").rsplit('/').next().unwrap_or("");
|
||||
|
||||
// `git` is only read-only for a curated set of subcommands.
|
||||
if first_token == "git" {
|
||||
let subcommand = tokens.next().unwrap_or("");
|
||||
return matches!(
|
||||
subcommand,
|
||||
"status"
|
||||
| "log"
|
||||
| "diff"
|
||||
| "show"
|
||||
| "branch"
|
||||
| "rev-parse"
|
||||
| "ls-files"
|
||||
| "blame"
|
||||
| "describe"
|
||||
| "tag"
|
||||
| "remote"
|
||||
);
|
||||
}
|
||||
|
||||
// `find` can execute or delete via actions; reject those forms.
|
||||
if first_token == "find"
|
||||
&& (command.contains("-exec")
|
||||
|| command.contains("-execdir")
|
||||
|| command.contains("-delete")
|
||||
|| command.contains("-ok")
|
||||
|| command.contains("-fprintf"))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
matches!(
|
||||
first_token,
|
||||
@@ -237,8 +317,6 @@ fn is_read_only_command(command: &str) -> bool {
|
||||
| "tr"
|
||||
| "cut"
|
||||
| "paste"
|
||||
| "tee"
|
||||
| "xargs"
|
||||
| "test"
|
||||
| "true"
|
||||
| "false"
|
||||
@@ -257,18 +335,8 @@ fn is_read_only_command(command: &str) -> bool {
|
||||
| "tree"
|
||||
| "jq"
|
||||
| "yq"
|
||||
| "python3"
|
||||
| "python"
|
||||
| "node"
|
||||
| "ruby"
|
||||
| "cargo"
|
||||
| "rustc"
|
||||
| "git"
|
||||
| "gh"
|
||||
) && !command.contains("-i ")
|
||||
&& !command.contains("--in-place")
|
||||
&& !command.contains(" > ")
|
||||
&& !command.contains(" >> ")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -375,6 +443,91 @@ mod tests {
|
||||
assert!(!is_read_only_command("sed -i 's/a/b/' file"));
|
||||
}
|
||||
|
||||
// --- Hardening regression tests (#2: read-only bypasses) ---
|
||||
|
||||
#[test]
|
||||
fn read_only_rejects_command_chaining() {
|
||||
// A leading read-only token must not launder a trailing destructive one.
|
||||
assert!(!is_read_only_command("cat foo; rm -rf bar"));
|
||||
assert!(!is_read_only_command("cat foo && rm -rf bar"));
|
||||
assert!(!is_read_only_command("ls || rm bar"));
|
||||
assert!(!is_read_only_command("cat foo | sh"));
|
||||
assert!(!is_read_only_command("echo `rm bar`"));
|
||||
assert!(!is_read_only_command("echo $(rm bar)"));
|
||||
assert!(!is_read_only_command("echo x>file")); // redirect without spaces
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_only_rejects_interpreters_and_build_drivers() {
|
||||
// These execute arbitrary code and are no longer read-only.
|
||||
assert!(!is_read_only_command(
|
||||
"python3 -c \"import os; os.system('rm -rf .')\""
|
||||
));
|
||||
assert!(!is_read_only_command("python script.py"));
|
||||
assert!(!is_read_only_command("node app.js"));
|
||||
assert!(!is_read_only_command("ruby x.rb"));
|
||||
assert!(!is_read_only_command("cargo run"));
|
||||
assert!(!is_read_only_command("rustc evil.rs"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_only_gates_git_subcommands() {
|
||||
// Read-only git subcommands remain allowed...
|
||||
assert!(is_read_only_command("git status"));
|
||||
assert!(is_read_only_command("git diff HEAD~1"));
|
||||
assert!(is_read_only_command("git show abc123"));
|
||||
// ...but mutating/exfiltrating ones are rejected.
|
||||
assert!(!is_read_only_command("git commit -m x"));
|
||||
assert!(!is_read_only_command("git push origin main"));
|
||||
assert!(!is_read_only_command("git reset --hard"));
|
||||
assert!(!is_read_only_command("git clean -fd"));
|
||||
assert!(!is_read_only_command("git config user.email a@b.c"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_only_rejects_find_actions() {
|
||||
assert!(is_read_only_command("find . -name Cargo.toml"));
|
||||
assert!(!is_read_only_command("find . -delete"));
|
||||
// -exec uses braces/semicolon which also trip the metachar guard,
|
||||
// but the explicit action check is the primary defense.
|
||||
assert!(!is_read_only_command("find . -execdir rm rf"));
|
||||
}
|
||||
|
||||
// --- Hardening regression tests (#1: workspace path traversal) ---
|
||||
|
||||
#[test]
|
||||
fn workspace_rejects_parent_traversal() {
|
||||
assert!(!is_within_workspace(
|
||||
"/workspace/../etc/passwd",
|
||||
"/workspace"
|
||||
));
|
||||
assert!(!is_within_workspace(
|
||||
"/workspace/../../etc/crontab",
|
||||
"/workspace"
|
||||
));
|
||||
assert!(!is_within_workspace("../etc/passwd", "/workspace"));
|
||||
assert!(!is_within_workspace(
|
||||
"/workspace/sub/../../outside",
|
||||
"/workspace"
|
||||
));
|
||||
// Legitimate paths still resolve inside.
|
||||
assert!(is_within_workspace(
|
||||
"/workspace/./src/main.rs",
|
||||
"/workspace"
|
||||
));
|
||||
assert!(is_within_workspace(
|
||||
"/workspace/src/../src/main.rs",
|
||||
"/workspace"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workspace_write_denies_traversal_escape() {
|
||||
let enforcer = make_enforcer(PermissionMode::WorkspaceWrite);
|
||||
let result = enforcer.check_file_write("/workspace/../../etc/crontab", "/workspace");
|
||||
assert!(matches!(result, EnforcementResult::Denied { .. }));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn active_mode_returns_policy_mode() {
|
||||
// given
|
||||
|
||||
@@ -149,7 +149,12 @@ impl PermissionPolicy {
|
||||
.iter()
|
||||
.map(|rule| PermissionRule::parse(rule))
|
||||
.collect();
|
||||
self.denied_tools = config.denied_tools().to_vec();
|
||||
// #94: normalize denied tool names to lowercase to match runtime convention
|
||||
self.denied_tools = config
|
||||
.denied_tools()
|
||||
.iter()
|
||||
.map(|t| t.to_lowercase())
|
||||
.collect();
|
||||
self
|
||||
}
|
||||
|
||||
@@ -375,7 +380,8 @@ impl PermissionRule {
|
||||
let matcher = parse_rule_matcher(content);
|
||||
return Self {
|
||||
raw: trimmed.to_string(),
|
||||
tool_name: tool_name.to_string(),
|
||||
// #94: normalize tool name to lowercase to match runtime convention
|
||||
tool_name: tool_name.to_lowercase(),
|
||||
matcher,
|
||||
};
|
||||
}
|
||||
@@ -384,7 +390,8 @@ impl PermissionRule {
|
||||
|
||||
Self {
|
||||
raw: trimmed.to_string(),
|
||||
tool_name: trimmed.to_string(),
|
||||
// #94: normalize tool name to lowercase to match runtime convention
|
||||
tool_name: trimmed.to_lowercase(),
|
||||
matcher: PermissionRuleMatcher::Any,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -231,8 +231,31 @@ impl Session {
|
||||
pub fn save_to_path(&self, path: impl AsRef<Path>) -> Result<(), SessionError> {
|
||||
let path = path.as_ref();
|
||||
let snapshot = self.render_jsonl_snapshot()?;
|
||||
rotate_session_file_if_needed(path)?;
|
||||
write_atomic(path, &snapshot)?;
|
||||
// #112: wrap ENOENT during rotate as concurrent modification
|
||||
match rotate_session_file_if_needed(path) {
|
||||
Ok(()) => {}
|
||||
Err(SessionError::Io(ref io_err)) if io_err.kind() == std::io::ErrorKind::NotFound => {
|
||||
return Err(SessionError::Io(std::io::Error::new(
|
||||
std::io::ErrorKind::NotFound,
|
||||
format!(
|
||||
"session file was removed during save (possible concurrent modification): {io_err}"
|
||||
),
|
||||
)));
|
||||
}
|
||||
Err(e) => return Err(e),
|
||||
}
|
||||
write_atomic(path, &snapshot).map_err(|e| {
|
||||
// #112: wrap ENOENT during write as concurrent modification
|
||||
match &e {
|
||||
SessionError::Io(io_err) if io_err.kind() == std::io::ErrorKind::NotFound => {
|
||||
SessionError::Io(std::io::Error::new(
|
||||
std::io::ErrorKind::NotFound,
|
||||
format!("session file was removed during write (possible concurrent modification): {io_err}"),
|
||||
))
|
||||
}
|
||||
_ => e,
|
||||
}
|
||||
})?;
|
||||
cleanup_rotated_logs(path)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -93,8 +93,19 @@ impl SessionStore {
|
||||
}
|
||||
|
||||
pub fn resolve_reference(&self, reference: &str) -> Result<SessionHandle, SessionControlError> {
|
||||
self.resolve_reference_excluding(reference, None)
|
||||
}
|
||||
|
||||
/// Resolve a session reference, optionally excluding a session by ID.
|
||||
/// When the reference is an alias, the excluded session is skipped
|
||||
/// so /resume latest returns the previous session, not the current one.
|
||||
pub fn resolve_reference_excluding(
|
||||
&self,
|
||||
reference: &str,
|
||||
exclude_id: Option<&str>,
|
||||
) -> Result<SessionHandle, SessionControlError> {
|
||||
if is_session_reference_alias(reference) {
|
||||
let latest = self.latest_session()?;
|
||||
let latest = self.latest_session_excluding(exclude_id)?;
|
||||
return Ok(SessionHandle {
|
||||
id: latest.id,
|
||||
path: latest.path,
|
||||
@@ -158,12 +169,45 @@ impl SessionStore {
|
||||
}
|
||||
|
||||
pub fn latest_session(&self) -> Result<ManagedSessionSummary, SessionControlError> {
|
||||
if let Some(latest) = self.list_sessions()?.into_iter().next() {
|
||||
self.latest_session_excluding(None)
|
||||
}
|
||||
|
||||
/// Find the most recent session, optionally excluding a session by ID
|
||||
/// and skipping sessions with 0 messages. Used by /resume latest to skip
|
||||
/// the current empty session and find the previous session with actual
|
||||
/// conversation history.
|
||||
pub fn latest_session_excluding(
|
||||
&self,
|
||||
exclude_id: Option<&str>,
|
||||
) -> Result<ManagedSessionSummary, SessionControlError> {
|
||||
let exclude = exclude_id.unwrap_or("");
|
||||
// First: look in the current workspace's session namespace
|
||||
if let Some(latest) = self
|
||||
.list_sessions()?
|
||||
.into_iter()
|
||||
.find(|s| s.id != exclude && s.message_count > 0)
|
||||
{
|
||||
return Ok(latest);
|
||||
}
|
||||
if let Some(latest) = self.scan_global_sessions()?.into_iter().next() {
|
||||
// Fallback: scan all workspace namespaces under ~/.claw/sessions/
|
||||
// and project-local .claw/sessions/ so /resume latest finds sessions
|
||||
// from other workspaces.
|
||||
if let Some(latest) = self
|
||||
.scan_global_sessions()?
|
||||
.into_iter()
|
||||
.find(|s| s.id != exclude && s.message_count > 0)
|
||||
{
|
||||
return Ok(latest);
|
||||
}
|
||||
// Distinguish between "no sessions at all" and "sessions exist but
|
||||
// all are empty" so the user gets a clear signal about what to do.
|
||||
let has_any_session = self.list_sessions()?.iter().any(|s| s.id != exclude)
|
||||
|| self.scan_global_sessions()?.iter().any(|s| s.id != exclude);
|
||||
if has_any_session {
|
||||
return Err(SessionControlError::Format(format_all_sessions_empty(
|
||||
&self.sessions_root,
|
||||
)));
|
||||
}
|
||||
Err(SessionControlError::Format(format_no_managed_sessions(
|
||||
&self.sessions_root,
|
||||
)))
|
||||
@@ -204,28 +248,41 @@ impl SessionStore {
|
||||
&self,
|
||||
reference: &str,
|
||||
) -> Result<LoadedManagedSession, SessionControlError> {
|
||||
match self.load_session(reference) {
|
||||
Ok(loaded) => Ok(loaded),
|
||||
Err(SessionControlError::WorkspaceMismatch { expected, actual })
|
||||
if is_session_reference_alias(reference) =>
|
||||
self.load_session_excluding(reference, None)
|
||||
}
|
||||
|
||||
/// Like `load_session_loose` but also excludes a session by ID.
|
||||
/// Used by /resume latest to skip the current empty session and find
|
||||
/// the previous session with actual conversation history.
|
||||
pub fn load_session_excluding(
|
||||
&self,
|
||||
reference: &str,
|
||||
exclude_id: Option<&str>,
|
||||
) -> Result<LoadedManagedSession, SessionControlError> {
|
||||
let handle = self.resolve_reference_excluding(reference, exclude_id)?;
|
||||
let session = Session::load_from_path(&handle.path)?;
|
||||
// For alias references, allow cross-workspace resume
|
||||
if is_session_reference_alias(reference) {
|
||||
if let Err(SessionControlError::WorkspaceMismatch {
|
||||
expected: _,
|
||||
actual,
|
||||
}) = self.validate_loaded_session(&handle.path, &session)
|
||||
{
|
||||
let handle = self.resolve_reference(reference)?;
|
||||
let session = Session::load_from_path(&handle.path)?;
|
||||
eprintln!(
|
||||
" Note: resuming session from a different workspace (origin: {})",
|
||||
actual.display()
|
||||
);
|
||||
let _ = expected; // suppress unused warning
|
||||
Ok(LoadedManagedSession {
|
||||
handle: SessionHandle {
|
||||
id: session.session_id.clone(),
|
||||
path: handle.path,
|
||||
},
|
||||
session,
|
||||
})
|
||||
}
|
||||
Err(other) => Err(other),
|
||||
} else {
|
||||
self.validate_loaded_session(&handle.path, &session)?;
|
||||
}
|
||||
Ok(LoadedManagedSession {
|
||||
handle: SessionHandle {
|
||||
id: session.session_id.clone(),
|
||||
path: handle.path,
|
||||
},
|
||||
session,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn fork_session(
|
||||
@@ -726,6 +783,16 @@ fn format_no_managed_sessions(sessions_root: &Path) -> String {
|
||||
)
|
||||
}
|
||||
|
||||
fn format_all_sessions_empty(sessions_root: &Path) -> String {
|
||||
let fingerprint_dir = sessions_root
|
||||
.file_name()
|
||||
.and_then(|f| f.to_str())
|
||||
.unwrap_or("<unknown>");
|
||||
format!(
|
||||
"all sessions are empty (0 messages) in .claw/sessions/{fingerprint_dir}/\nThis usually means a fresh `claw` session is running but no messages have been sent yet.\nWait for a response in your other session, then try `--resume {LATEST_SESSION_REFERENCE}` again."
|
||||
)
|
||||
}
|
||||
|
||||
fn format_legacy_session_missing_workspace_root(
|
||||
session_path: &Path,
|
||||
workspace_root: &Path,
|
||||
@@ -1220,6 +1287,114 @@ mod tests {
|
||||
fs::remove_dir_all(base).expect("temp dir should clean up");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn latest_session_returns_all_empty_error_when_sessions_exist_but_have_no_messages() {
|
||||
// given — create sessions with 0 messages (empty)
|
||||
let base = temp_dir();
|
||||
fs::create_dir_all(&base).expect("base dir should exist");
|
||||
let store = SessionStore::from_cwd(&base).expect("store should build");
|
||||
|
||||
let empty_handle = store.create_handle("empty-session");
|
||||
Session::new()
|
||||
.with_persistence_path(empty_handle.path.clone())
|
||||
.save_to_path(&empty_handle.path)
|
||||
.expect("empty session should save");
|
||||
|
||||
// when — latest_session should fail with the "all sessions empty" message
|
||||
let result = store.latest_session();
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"latest_session should fail when all sessions are empty"
|
||||
);
|
||||
let err_msg = result.unwrap_err().to_string();
|
||||
assert!(
|
||||
err_msg.contains("all sessions are empty"),
|
||||
"error should mention 'all sessions are empty', got: {err_msg}"
|
||||
);
|
||||
assert!(
|
||||
err_msg.contains("0 messages"),
|
||||
"error should mention '0 messages', got: {err_msg}"
|
||||
);
|
||||
|
||||
fs::remove_dir_all(base).expect("temp dir should clean up");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn latest_session_excluding_skips_excluded_id_and_returns_previous() {
|
||||
// given — two sessions WITH messages, newest excluded
|
||||
let base = temp_dir();
|
||||
fs::create_dir_all(&base).expect("base dir should exist");
|
||||
let store = SessionStore::from_cwd(&base).expect("store should build");
|
||||
let older = persist_session_via_store(&store, "older work");
|
||||
wait_for_next_millisecond();
|
||||
let newer = persist_session_via_store(&store, "newer work");
|
||||
|
||||
// when — exclude the newest session
|
||||
let latest = store
|
||||
.latest_session_excluding(Some(&newer.session_id))
|
||||
.expect("latest excluding newest should resolve");
|
||||
|
||||
// then — the older session wins because the newest is skipped
|
||||
assert_eq!(
|
||||
latest.id, older.session_id,
|
||||
"excluded id must be skipped, returning the previous session"
|
||||
);
|
||||
fs::remove_dir_all(base).expect("temp dir should clean up");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn latest_session_filters_out_zero_message_sessions() {
|
||||
// given — one empty (0-message) session and one non-empty session
|
||||
let base = temp_dir();
|
||||
fs::create_dir_all(&base).expect("base dir should exist");
|
||||
let store = SessionStore::from_cwd(&base).expect("store should build");
|
||||
|
||||
let empty_handle = store.create_handle("empty-session");
|
||||
Session::new()
|
||||
.with_persistence_path(empty_handle.path.clone())
|
||||
.save_to_path(&empty_handle.path)
|
||||
.expect("empty session should save");
|
||||
wait_for_next_millisecond();
|
||||
let non_empty = persist_session_via_store(&store, "real conversation");
|
||||
|
||||
// when
|
||||
let latest = store.latest_session().expect("latest should resolve");
|
||||
|
||||
// then — the non-empty session wins; the 0-message one is filtered out
|
||||
assert_eq!(
|
||||
latest.id, non_empty.session_id,
|
||||
"0-message session must be filtered out, non-empty session wins"
|
||||
);
|
||||
assert!(
|
||||
latest.message_count > 0,
|
||||
"resolved session must have messages"
|
||||
);
|
||||
fs::remove_dir_all(base).expect("temp dir should clean up");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_reference_excluding_latest_skips_excluded_id() {
|
||||
// given — two sessions WITH messages
|
||||
let base = temp_dir();
|
||||
fs::create_dir_all(&base).expect("base dir should exist");
|
||||
let store = SessionStore::from_cwd(&base).expect("store should build");
|
||||
let older = persist_session_via_store(&store, "older work");
|
||||
wait_for_next_millisecond();
|
||||
let newer = persist_session_via_store(&store, "newer work");
|
||||
|
||||
// when — resolve the "latest" alias while excluding the newest session
|
||||
let handle = store
|
||||
.resolve_reference_excluding("latest", Some(&newer.session_id))
|
||||
.expect("latest alias excluding newest should resolve");
|
||||
|
||||
// then — the excluded id is skipped, so the older session resolves
|
||||
assert_eq!(
|
||||
handle.id, older.session_id,
|
||||
"excluded id must be skipped when resolving the latest alias"
|
||||
);
|
||||
fs::remove_dir_all(base).expect("temp dir should clean up");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_exists_and_delete_are_scoped_to_workspace_store() {
|
||||
// given
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -112,6 +112,7 @@ fn assert_doctor_help_json_contract(parsed: &Value) {
|
||||
assert!(checks.iter().any(|check| check == "boot preflight"));
|
||||
assert!(checks.iter().any(|check| check == "memory"));
|
||||
assert!(checks.iter().any(|check| check == "mcp validation"));
|
||||
assert!(checks.iter().any(|check| check == "hook validation"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -840,14 +841,19 @@ fn acp_guidance_emits_json_when_requested() {
|
||||
let root = unique_temp_dir("acp-json");
|
||||
fs::create_dir_all(&root).expect("temp dir should exist");
|
||||
|
||||
let acp = assert_json_command(&root, &["--output-format", "json", "acp"]);
|
||||
// #443: acp serve exits 2 (not implemented) instead of 0
|
||||
let output = run_claw(&root, &["--output-format", "json", "acp"], &[]);
|
||||
assert_eq!(
|
||||
output.status.code(),
|
||||
Some(2),
|
||||
"acp should exit 2 (not implemented)"
|
||||
);
|
||||
let acp: Value =
|
||||
serde_json::from_slice(&output.stdout).expect("acp stdout should be valid json");
|
||||
assert_eq!(acp["kind"], "acp");
|
||||
assert_eq!(acp["schema_version"], "1.0");
|
||||
assert_eq!(acp["status"], "unsupported");
|
||||
assert_eq!(acp["phase"], "discoverability_only");
|
||||
assert_eq!(acp["status"], "not_implemented");
|
||||
assert_eq!(acp["supported"], false);
|
||||
assert_eq!(acp["exit_code"], 0);
|
||||
assert_eq!(acp["serve_alias_only"], true);
|
||||
assert_eq!(acp["protocol"]["json_rpc"], false);
|
||||
assert_eq!(acp["protocol"]["daemon"], false);
|
||||
assert!(acp["protocol"]["endpoint"].is_null());
|
||||
@@ -855,12 +861,23 @@ fn acp_guidance_emits_json_when_requested() {
|
||||
acp["contracts"]["unsupported_invocation_kind"],
|
||||
"unsupported_acp_invocation"
|
||||
);
|
||||
assert_eq!(acp["discoverability_tracking"], "ROADMAP #64a");
|
||||
assert_eq!(acp["tracking"], "ROADMAP #76 / #3033 / #3004");
|
||||
// #443: internal tracking IDs removed from public JSON
|
||||
assert!(
|
||||
acp.get("discoverability_tracking").is_none(),
|
||||
"discoverability_tracking should be removed (#443)"
|
||||
);
|
||||
assert!(
|
||||
acp.get("tracking").is_none(),
|
||||
"tracking should be removed (#443)"
|
||||
);
|
||||
assert!(
|
||||
acp.get("recommended_workflows").is_none(),
|
||||
"recommended_workflows should be removed (#443)"
|
||||
);
|
||||
assert!(acp["message"]
|
||||
.as_str()
|
||||
.expect("acp message")
|
||||
.contains("discoverability alias"));
|
||||
.contains("not implemented"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1459,7 +1476,7 @@ fn doctor_and_resume_status_emit_json_when_requested() {
|
||||
.is_some_and(|available| available.iter().any(|name| name == "web_fetch")));
|
||||
|
||||
let checks = doctor["checks"].as_array().expect("doctor checks");
|
||||
assert_eq!(checks.len(), 10);
|
||||
assert_eq!(checks.len(), 12);
|
||||
let check_names = checks
|
||||
.iter()
|
||||
.map(|check| {
|
||||
@@ -1479,8 +1496,10 @@ fn doctor_and_resume_status_emit_json_when_requested() {
|
||||
check_names,
|
||||
vec![
|
||||
"auth",
|
||||
"base urls",
|
||||
"config",
|
||||
"mcp validation",
|
||||
"hook validation",
|
||||
"install source",
|
||||
"workspace",
|
||||
"memory",
|
||||
@@ -2063,7 +2082,7 @@ fn local_json_surfaces_have_non_empty_action_contract_714() {
|
||||
&git_workspace,
|
||||
strings(&["--output-format", "json", "diff"]),
|
||||
),
|
||||
(&workspace, strings(&["--output-format", "json", "acp"])),
|
||||
// #443: ACP exits 2 (not implemented); tested separately in acp_guidance_emits_json_when_requested
|
||||
(&workspace, strings(&["--output-format", "json", "config"])),
|
||||
(
|
||||
&workspace,
|
||||
@@ -3346,7 +3365,7 @@ fn config_unsupported_section_json_hint_741() {
|
||||
fs::create_dir_all(&root).expect("temp dir");
|
||||
let bin = env!("CARGO_BIN_EXE_claw");
|
||||
|
||||
for section in &["list", "show", "bogus", "help"] {
|
||||
for section in &["list", "show", "bogus"] {
|
||||
let output = Command::new(bin)
|
||||
.current_dir(&root)
|
||||
.args(["--output-format", "json", "config", section])
|
||||
@@ -3384,6 +3403,36 @@ fn config_unsupported_section_json_hint_741() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_help_returns_structured_section_list_344() {
|
||||
// #344: /config help should return a structured section list, not an error
|
||||
use std::process::Command;
|
||||
let root = unique_temp_dir("config-help");
|
||||
fs::create_dir_all(&root).expect("temp dir");
|
||||
let bin = env!("CARGO_BIN_EXE_claw");
|
||||
let output = Command::new(bin)
|
||||
.current_dir(&root)
|
||||
.args(["--output-format", "json", "config", "help"])
|
||||
.output()
|
||||
.expect("claw config help should run");
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let parsed: serde_json::Value =
|
||||
serde_json::from_str(stdout.trim()).expect("config help should emit valid JSON");
|
||||
assert_eq!(parsed["kind"], "config", "config help kind must be config");
|
||||
assert_eq!(
|
||||
parsed["status"], "ok",
|
||||
"config help must return status:ok (#344)"
|
||||
);
|
||||
assert_eq!(
|
||||
parsed["section"], "help",
|
||||
"config help section must be help"
|
||||
);
|
||||
let sections = parsed["available_sections"]
|
||||
.as_array()
|
||||
.expect("config help must have available_sections array");
|
||||
assert!(!sections.is_empty(), "available_sections must not be empty");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_json_has_kind_702() {
|
||||
// #458/#702: `claw export --output-format json` must emit kind:export.
|
||||
@@ -3866,7 +3915,7 @@ fn agents_plugins_mcp_unknown_subcommand_have_hint_774() {
|
||||
};
|
||||
let parsed: serde_json::Value =
|
||||
serde_json::from_str(json_str.trim()).expect("mcp bogus should emit JSON");
|
||||
assert_eq!(parsed["error_kind"], "unknown_mcp_action");
|
||||
assert_eq!(parsed["error_kind"], "unsupported_action");
|
||||
let hint = parsed["hint"].as_str().unwrap_or("");
|
||||
assert!(!hint.is_empty(), "mcp bogus hint must be non-null (#774)");
|
||||
}
|
||||
@@ -4146,8 +4195,8 @@ fn acp_unsupported_invocation_has_hint_782() {
|
||||
.expect("hint must be non-null (#782)");
|
||||
assert!(!hint.is_empty(), "hint must not be empty");
|
||||
assert!(
|
||||
hint.contains("discoverability") || hint.contains("ROADMAP"),
|
||||
"hint should explain the discoverability-only status, got: {hint:?}"
|
||||
hint.contains("not implemented") || hint.contains("unsupported"),
|
||||
"hint should explain the not-implemented status, got: {hint:?}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -530,8 +530,9 @@ fn resumed_help_command_emits_structured_json() {
|
||||
let stdout = String::from_utf8(output.stdout).expect("utf8");
|
||||
let parsed: Value = serde_json::from_str(stdout.trim()).expect("should be json");
|
||||
assert_eq!(parsed["kind"], "help");
|
||||
assert!(parsed["text"].as_str().is_some());
|
||||
let text = parsed["text"].as_str().unwrap();
|
||||
// #338: resume help now uses 'message' field for parity with top-level help
|
||||
assert!(parsed["message"].as_str().is_some());
|
||||
let text = parsed["message"].as_str().unwrap();
|
||||
assert!(text.contains("/status"), "help text should list /status");
|
||||
}
|
||||
|
||||
|
||||
@@ -2701,6 +2701,20 @@ fn is_within_workspace(path: &str) -> bool {
|
||||
|
||||
let path = PathBuf::from(trimmed);
|
||||
|
||||
// Reject any parent-directory traversal. Callers never need `..` to refer
|
||||
// to files inside the workspace, and `..` defeats both checks below: the
|
||||
// relative branch only inspects the leading component, and the absolute
|
||||
// branch's `canonicalize()` silently falls back to the literal `..` path
|
||||
// when the target does not exist yet (e.g. a file about to be created).
|
||||
// Returning false here is the safe direction: it classifies the command as
|
||||
// requiring full-access permission rather than workspace-write.
|
||||
if path
|
||||
.components()
|
||||
.any(|component| matches!(component, std::path::Component::ParentDir))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
// If path is absolute, check if it starts with CWD
|
||||
if path.is_absolute() {
|
||||
if let Ok(cwd) = std::env::current_dir() {
|
||||
@@ -2718,6 +2732,26 @@ fn run_powershell(input: PowerShellInput) -> Result<String, String> {
|
||||
to_pretty_json(execute_powershell(input).map_err(|error| error.to_string())?)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod workspace_traversal_guard_tests {
|
||||
use super::is_within_workspace;
|
||||
|
||||
#[test]
|
||||
fn rejects_parent_traversal_components() {
|
||||
// Leading and embedded `..` must both be rejected (was previously a hole
|
||||
// because only the leading component was inspected).
|
||||
assert!(!is_within_workspace("../secrets"));
|
||||
assert!(!is_within_workspace("src/../../etc/passwd"));
|
||||
assert!(!is_within_workspace("a/b/../../../etc/crontab"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allows_plain_relative_paths() {
|
||||
assert!(is_within_workspace("src/main.rs"));
|
||||
assert!(is_within_workspace("Cargo.toml"));
|
||||
}
|
||||
}
|
||||
|
||||
fn to_pretty_json<T: serde::Serialize>(value: T) -> Result<String, String> {
|
||||
serde_json::to_string_pretty(&value).map_err(|error| error.to_string())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user