mirror of
https://github.com/instructkr/claude-code.git
synced 2026-05-27 16:06:44 +00:00
fix: remove stale retry_after field, Team variant, config_load_error_kind, denied_tools initializer errors
- Remove retry_after: None from ApiError::Api structs in openai_compat.rs (field was removed) - Remove SlashCommand::Team parse arm (variant was removed from enum) - Add config_load_error_kind: None to doctor path StatusContext initializer - Add Thinking arm to all ContentBlock match blocks in trident.rs - Remove cargo fmt drift across commands, config, compact, tools, trident
This commit is contained in:
@@ -1503,7 +1503,6 @@ fn parse_sse_frame(
|
||||
body: trimmed.chars().take(500).collect(),
|
||||
retryable: false,
|
||||
suggested_action: suggested_action_for_status(status),
|
||||
retry_after: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -1519,7 +1518,6 @@ 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);
|
||||
@@ -1571,7 +1569,6 @@ 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)
|
||||
|
||||
@@ -1472,7 +1472,6 @@ pub fn validate_slash_command_input(
|
||||
}
|
||||
"plan" => SlashCommand::Plan { mode: remainder },
|
||||
"review" => SlashCommand::Review { scope: remainder },
|
||||
"team" => SlashCommand::Team { action: remainder },
|
||||
"tasks" => SlashCommand::Tasks { args: remainder },
|
||||
"theme" => SlashCommand::Theme { name: remainder },
|
||||
"voice" => SlashCommand::Voice { mode: remainder },
|
||||
|
||||
@@ -90,6 +90,10 @@ pub struct RuntimePermissionRuleConfig {
|
||||
allow: Vec<String>,
|
||||
deny: Vec<String>,
|
||||
ask: Vec<String>,
|
||||
/// #159: simple tool-name denials parsed from the `deniedTools` config field.
|
||||
/// Unlike the `deny` rules (pattern-based), `denied_tools` is a flat list of
|
||||
/// tool names that are unconditionally denied regardless of permission mode.
|
||||
denied_tools: Vec<String>,
|
||||
}
|
||||
|
||||
/// Collection of configured MCP servers after scope-aware merging.
|
||||
@@ -738,8 +742,18 @@ impl RuntimeHookConfig {
|
||||
|
||||
impl RuntimePermissionRuleConfig {
|
||||
#[must_use]
|
||||
pub fn new(allow: Vec<String>, deny: Vec<String>, ask: Vec<String>) -> Self {
|
||||
Self { allow, deny, ask }
|
||||
pub fn new(
|
||||
allow: Vec<String>,
|
||||
deny: Vec<String>,
|
||||
ask: Vec<String>,
|
||||
denied_tools: Vec<String>,
|
||||
) -> Self {
|
||||
Self {
|
||||
allow,
|
||||
deny,
|
||||
ask,
|
||||
denied_tools,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
@@ -756,6 +770,11 @@ impl RuntimePermissionRuleConfig {
|
||||
pub fn ask(&self) -> &[String] {
|
||||
&self.ask
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn denied_tools(&self) -> &[String] {
|
||||
&self.denied_tools
|
||||
}
|
||||
}
|
||||
|
||||
impl McpConfigCollection {
|
||||
@@ -926,6 +945,12 @@ fn parse_optional_permission_rules(
|
||||
.unwrap_or_default(),
|
||||
ask: optional_string_array(permissions, "ask", "merged settings.permissions")?
|
||||
.unwrap_or_default(),
|
||||
denied_tools: optional_string_array(
|
||||
permissions,
|
||||
"deniedTools",
|
||||
"merged settings.permissions",
|
||||
)?
|
||||
.unwrap_or_default(),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -227,6 +227,10 @@ const PERMISSIONS_FIELDS: &[FieldSpec] = &[
|
||||
name: "allow",
|
||||
expected: FieldType::StringArray,
|
||||
},
|
||||
FieldSpec {
|
||||
name: "deniedTools",
|
||||
expected: FieldType::StringArray,
|
||||
},
|
||||
FieldSpec {
|
||||
name: "deny",
|
||||
expected: FieldType::StringArray,
|
||||
|
||||
@@ -102,6 +102,10 @@ pub struct PermissionPolicy {
|
||||
allow_rules: Vec<PermissionRule>,
|
||||
deny_rules: Vec<PermissionRule>,
|
||||
ask_rules: Vec<PermissionRule>,
|
||||
/// #159: simple tool-name denials. Tools in this list are unconditionally
|
||||
/// denied regardless of permission mode, checked before the rule-based
|
||||
/// deny/allow/ask evaluation.
|
||||
denied_tools: Vec<String>,
|
||||
}
|
||||
|
||||
impl PermissionPolicy {
|
||||
@@ -113,6 +117,7 @@ impl PermissionPolicy {
|
||||
allow_rules: Vec::new(),
|
||||
deny_rules: Vec::new(),
|
||||
ask_rules: Vec::new(),
|
||||
denied_tools: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -144,6 +149,7 @@ impl PermissionPolicy {
|
||||
.iter()
|
||||
.map(|rule| PermissionRule::parse(rule))
|
||||
.collect();
|
||||
self.denied_tools = config.denied_tools().to_vec();
|
||||
self
|
||||
}
|
||||
|
||||
@@ -179,6 +185,15 @@ impl PermissionPolicy {
|
||||
context: &PermissionContext,
|
||||
prompter: Option<&mut dyn PermissionPrompter>,
|
||||
) -> PermissionOutcome {
|
||||
// #159: check denied_tools before rule-based evaluation. Tools listed
|
||||
// in the denied_tools config are unconditionally denied regardless of
|
||||
// permission mode.
|
||||
if self.denied_tools.iter().any(|t| t == tool_name) {
|
||||
return PermissionOutcome::Deny {
|
||||
reason: format!("tool '{tool_name}' has been denied by denied_tools configuration"),
|
||||
};
|
||||
}
|
||||
|
||||
if let Some(rule) = Self::find_matching_rule(&self.deny_rules, tool_name, input) {
|
||||
return PermissionOutcome::Deny {
|
||||
reason: format!(
|
||||
@@ -571,6 +586,7 @@ mod tests {
|
||||
vec!["bash(git:*)".to_string()],
|
||||
vec!["bash(rm -rf:*)".to_string()],
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
);
|
||||
let policy = PermissionPolicy::new(PermissionMode::ReadOnly)
|
||||
.with_tool_requirement("bash", PermissionMode::DangerFullAccess)
|
||||
@@ -586,12 +602,39 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn denied_tools_denies_listed_tools_unconditionally() {
|
||||
let rules = RuntimePermissionRuleConfig::new(
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
vec!["bash".to_string(), "write_file".to_string()],
|
||||
);
|
||||
let policy = PermissionPolicy::new(PermissionMode::Allow).with_permission_rules(&rules);
|
||||
|
||||
let result = policy.authorize("bash", "echo hello", None);
|
||||
assert!(matches!(
|
||||
result,
|
||||
PermissionOutcome::Deny { reason } if reason.contains("denied_tools")
|
||||
));
|
||||
|
||||
let result = policy.authorize("write_file", "{}", None);
|
||||
assert!(matches!(
|
||||
result,
|
||||
PermissionOutcome::Deny { reason } if reason.contains("denied_tools")
|
||||
));
|
||||
|
||||
let result = policy.authorize("read_file", "{}", None);
|
||||
assert_eq!(result, PermissionOutcome::Allow);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ask_rules_force_prompt_even_when_mode_allows() {
|
||||
let rules = RuntimePermissionRuleConfig::new(
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
vec!["bash(git:*)".to_string()],
|
||||
Vec::new(),
|
||||
);
|
||||
let policy = PermissionPolicy::new(PermissionMode::DangerFullAccess)
|
||||
.with_tool_requirement("bash", PermissionMode::DangerFullAccess)
|
||||
@@ -617,6 +660,7 @@ mod tests {
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
vec!["bash(git:*)".to_string()],
|
||||
Vec::new(),
|
||||
);
|
||||
let policy = PermissionPolicy::new(PermissionMode::ReadOnly)
|
||||
.with_tool_requirement("bash", PermissionMode::DangerFullAccess)
|
||||
|
||||
@@ -252,6 +252,7 @@ fn extract_file_operation(block: &ContentBlock) -> Option<(String, FileOp)> {
|
||||
Some((path, op_type))
|
||||
}
|
||||
ContentBlock::Text { .. } => None,
|
||||
ContentBlock::Thinking { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -357,6 +358,7 @@ fn is_chatty_message(msg: &ConversationMessage) -> bool {
|
||||
ContentBlock::Text { text } => text.len(),
|
||||
ContentBlock::ToolUse { input, .. } => input.len(),
|
||||
ContentBlock::ToolResult { output, .. } => output.len(),
|
||||
ContentBlock::Thinking { thinking, .. } => thinking.len(),
|
||||
})
|
||||
.sum();
|
||||
|
||||
@@ -546,6 +548,9 @@ fn fingerprint_message(index: usize, msg: &ConversationMessage) -> Option<Messag
|
||||
ContentBlock::Text { text } => {
|
||||
text_length += text.len();
|
||||
}
|
||||
ContentBlock::Thinking { thinking, .. } => {
|
||||
text_length += thinking.len();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -618,6 +623,7 @@ fn generate_cluster_summary(messages: &[&ConversationMessage]) -> String {
|
||||
}
|
||||
}
|
||||
ContentBlock::Text { .. } => {}
|
||||
ContentBlock::Thinking { .. } => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -653,6 +659,7 @@ fn estimate_message_tokens(message: &ConversationMessage) -> usize {
|
||||
ContentBlock::ToolResult {
|
||||
tool_name, output, ..
|
||||
} => (tool_name.len() + output.len()) / 4 + 1,
|
||||
ContentBlock::Thinking { thinking, .. } => thinking.len() / 4 + 1,
|
||||
})
|
||||
.sum()
|
||||
}
|
||||
|
||||
@@ -286,6 +286,10 @@ fn classify_error_kind(message: &str) -> &'static str {
|
||||
"confirmation_required"
|
||||
} else if message.contains("api failed") || message.contains("api returned") {
|
||||
"api_http_error"
|
||||
} else if message.contains("mcpServers") {
|
||||
"malformed_mcp_config"
|
||||
} else if message.starts_with("empty prompt") {
|
||||
"empty_prompt"
|
||||
} else {
|
||||
"unknown"
|
||||
}
|
||||
@@ -2084,6 +2088,7 @@ fn render_doctor_report() -> Result<DoctorReport, Box<dyn std::error::Error>> {
|
||||
// Doctor path has its own config check; StatusContext here is only
|
||||
// fed into health renderers that don't read config_load_error.
|
||||
config_load_error: config.as_ref().err().map(ToString::to_string),
|
||||
config_load_error_kind: None,
|
||||
};
|
||||
Ok(DoctorReport {
|
||||
checks: vec![
|
||||
@@ -3032,6 +3037,11 @@ struct StatusContext {
|
||||
/// `status: "degraded"` so claws can distinguish "status ran but config
|
||||
/// is broken" from "status ran cleanly".
|
||||
config_load_error: Option<String>,
|
||||
/// #143: machine-readable kind for the config load error, derived from
|
||||
/// `classify_error_kind`. Included in JSON output alongside the human
|
||||
/// readable string so downstream claws can switch on the kind token
|
||||
/// instead of regex-scraping the prose.
|
||||
config_load_error_kind: Option<&'static str>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -6549,6 +6559,8 @@ fn status_json_value(
|
||||
// are still populated). `config_load_error` carries the parse-error string
|
||||
// when present; it's a string rather than a typed object in Phase 1 and
|
||||
// will join the typed-error taxonomy in Phase 2 (ROADMAP §4.44).
|
||||
// `config_load_error_kind` is the machine-readable kind token derived from
|
||||
// `classify_error_kind` so downstream claws can switch on it directly.
|
||||
let degraded = context.config_load_error.is_some();
|
||||
let model_source = provenance.map(|p| p.source.as_str());
|
||||
let model_raw = provenance.and_then(|p| p.raw.clone());
|
||||
@@ -6557,6 +6569,7 @@ fn status_json_value(
|
||||
"kind": "status",
|
||||
"status": if degraded { "degraded" } else { "ok" },
|
||||
"config_load_error": context.config_load_error,
|
||||
"config_load_error_kind": context.config_load_error_kind,
|
||||
"model": model,
|
||||
"model_source": model_source,
|
||||
"model_raw": model_raw,
|
||||
@@ -6642,23 +6655,30 @@ fn status_context(
|
||||
// health surface (workspace, git, model, permission, sandbox can still be
|
||||
// reported independently).
|
||||
let runtime_config = loader.load();
|
||||
let (loaded_config_files, sandbox_status, config_load_error) = match runtime_config.as_ref() {
|
||||
Ok(runtime_config) => (
|
||||
runtime_config.loaded_entries().len(),
|
||||
resolve_sandbox_status(runtime_config.sandbox(), &cwd),
|
||||
None,
|
||||
),
|
||||
Err(err) => (
|
||||
0,
|
||||
// Fall back to defaults for sandbox resolution so claws still see
|
||||
// a populated sandbox section instead of a missing field. Defaults
|
||||
// produce the same output as a runtime config with no sandbox
|
||||
// overrides, which is the right degraded-mode shape: we cannot
|
||||
// report what the user *intended*, only what is actually in effect.
|
||||
resolve_sandbox_status(&runtime::SandboxConfig::default(), &cwd),
|
||||
Some(err.to_string()),
|
||||
),
|
||||
};
|
||||
let (loaded_config_files, sandbox_status, config_load_error, config_load_error_kind) =
|
||||
match runtime_config.as_ref() {
|
||||
Ok(cfg) => (
|
||||
cfg.loaded_entries().len(),
|
||||
resolve_sandbox_status(cfg.sandbox(), &cwd),
|
||||
None,
|
||||
None,
|
||||
),
|
||||
Err(err) => {
|
||||
let err_string = err.to_string();
|
||||
let err_kind = classify_error_kind(&err_string);
|
||||
(
|
||||
0,
|
||||
// Fall back to defaults for sandbox resolution so claws still see
|
||||
// a populated sandbox section instead of a missing field. Defaults
|
||||
// produce the same output as a runtime config with no sandbox
|
||||
// overrides, which is the right degraded-mode shape: we cannot
|
||||
// report what the user *intended*, only what is actually in effect.
|
||||
resolve_sandbox_status(&runtime::SandboxConfig::default(), &cwd),
|
||||
Some(err_string),
|
||||
Some(err_kind),
|
||||
)
|
||||
}
|
||||
};
|
||||
let project_context = ProjectContext::discover_with_git(&cwd, DEFAULT_DATE)?;
|
||||
let (project_root, git_branch) =
|
||||
parse_git_status_metadata(project_context.git_status.as_deref());
|
||||
@@ -6687,6 +6707,7 @@ fn status_context(
|
||||
boot_preflight,
|
||||
sandbox_status,
|
||||
config_load_error,
|
||||
config_load_error_kind,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -12171,6 +12192,18 @@ mod tests {
|
||||
classify_error_kind("api failed after 3 attempts: ..."),
|
||||
"api_http_error"
|
||||
);
|
||||
assert_eq!(
|
||||
classify_error_kind("/tmp/settings.json: mcpServers.foo: expected JSON object"),
|
||||
"malformed_mcp_config"
|
||||
);
|
||||
assert_eq!(
|
||||
classify_error_kind("settings.json: mcpServers: field must be an object"),
|
||||
"malformed_mcp_config"
|
||||
);
|
||||
assert_eq!(
|
||||
classify_error_kind("empty prompt: provide a subcommand or a non-empty prompt string"),
|
||||
"empty_prompt"
|
||||
);
|
||||
assert_eq!(
|
||||
classify_error_kind("something completely unknown"),
|
||||
"unknown"
|
||||
|
||||
Reference in New Issue
Block a user