Skip to content

Commit 2af4d74

Browse files
authored
fix(cli): omit null/empty request fields so older instances accept them (#2799)
The CLI unconditionally attaches optional fields to the find/search/add_resource request bodies — `args` as an empty `{}` and `tags`/`context_type` as `null` — even when the user never set them. OpenViking kernels use `model_config = ConfigDict(extra="forbid")` on these routes, so any instance that predates a field rejects the whole request with `body.<field>: Extra inputs are not permitted`. This breaks whenever the CLI is newer than the target instance (and, symmetrically, when a field is later renamed/removed): e.g. `ov add-resource` fails on `body.args` against a pre-#2549 instance, and `ov find` fails on `body.tags` against a strict pre-#2706 instance. - Add `compact_request_body()` and apply it in find/search/add_resource: drop null-valued keys and an empty `args` object before sending. Scoped to these read/create routes only, where a missing optional field and an explicit null are equivalent — not applied globally, since null may mean "clear" on a future update/PATCH route. This mirrors the existing `create_parent` backward-compat convention. - When a field is explicitly set but unsupported, translate the raw pydantic `Extra inputs are not permitted` error into a version-mismatch hint and suggest `ov health`, instead of surfacing the opaque API error. Tests: unit tests for `compact_request_body` and `extra_forbidden_field`.
1 parent 9beb18f commit 2af4d74

3 files changed

Lines changed: 125 additions & 2 deletions

File tree

crates/ov_cli/src/client.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,32 @@ pub use crate::base_client::{BaseClient, FileUploader, TimeoutConfig};
77

88
use crate::error::{Error, Result};
99

10+
/// Drop null-valued keys (and an empty `args` object) from a request body before
11+
/// sending it. Older, stricter servers use `extra="forbid"` and reject any field
12+
/// they do not yet define, so unconditionally attaching optional fields (even as
13+
/// `null`/`{}`) breaks against instances that predate that field. Omitting them is
14+
/// safe for read/create routes where a missing optional field and an explicit
15+
/// `null` are equivalent — do NOT use this for update/PATCH bodies where `null`
16+
/// may mean "clear this field".
17+
fn compact_request_body(body: &mut Value) {
18+
let Some(obj) = body.as_object_mut() else {
19+
return;
20+
};
21+
obj.retain(|key, value| {
22+
if value.is_null() {
23+
return false;
24+
}
25+
// `args` is always attached by the CLI but absent from pre-#2549 models;
26+
// only forward it when the caller actually provided arguments.
27+
if key == "args" {
28+
if let Some(map) = value.as_object() {
29+
return !map.is_empty();
30+
}
31+
}
32+
true
33+
});
34+
}
35+
1036
// ============ HttpClient ============
1137

1238
/// High-level HTTP client for OpenViking API
@@ -441,6 +467,7 @@ impl HttpClient {
441467
"tags": tags,
442468
});
443469
self.attach_legacy_agent_scope(&mut body);
470+
compact_request_body(&mut body);
444471
self.post("/api/v1/search/find", &body).await
445472
}
446473

@@ -472,6 +499,7 @@ impl HttpClient {
472499
"tags": tags,
473500
});
474501
self.attach_legacy_agent_scope(&mut body);
502+
compact_request_body(&mut body);
475503
self.post("/api/v1/search/search", &body).await
476504
}
477505

@@ -558,6 +586,7 @@ impl HttpClient {
558586
.expect("add_resource request body must be an object")
559587
.insert("create_parent".to_string(), serde_json::Value::Bool(true));
560588
}
589+
compact_request_body(&mut body);
561590
body
562591
};
563592

@@ -1432,6 +1461,37 @@ mod tests {
14321461
use tokio::net::TcpListener;
14331462
use tokio::sync::oneshot;
14341463

1464+
#[test]
1465+
fn compact_request_body_drops_null_and_empty_args() {
1466+
let mut body = json!({
1467+
"query": "hi",
1468+
"score_threshold": null,
1469+
"tags": null,
1470+
"args": {},
1471+
"wait": false,
1472+
"create_parent": true,
1473+
"filter": {"k": "v"},
1474+
});
1475+
super::compact_request_body(&mut body);
1476+
let obj = body.as_object().unwrap();
1477+
// Non-null values are kept, including `false` and non-empty objects.
1478+
assert!(obj.contains_key("query"));
1479+
assert!(obj.contains_key("wait"));
1480+
assert!(obj.contains_key("create_parent"));
1481+
assert!(obj.contains_key("filter"));
1482+
// Null fields and an empty `args` are dropped so pre-field servers accept it.
1483+
assert!(!obj.contains_key("score_threshold"));
1484+
assert!(!obj.contains_key("tags"));
1485+
assert!(!obj.contains_key("args"));
1486+
}
1487+
1488+
#[test]
1489+
fn compact_request_body_keeps_non_empty_args() {
1490+
let mut body = json!({"path": "x", "args": {"feishu_access_token": "u-x"}});
1491+
super::compact_request_body(&mut body);
1492+
assert!(body.as_object().unwrap().contains_key("args"));
1493+
}
1494+
14351495
#[test]
14361496
fn timeout_config_calculation() {
14371497
let config = TimeoutConfig::new(60, 2.0);

crates/ov_cli/src/error_classifier.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,51 @@ pub(crate) fn looks_like_auth_error(message: &str) -> bool {
99
.any(|token| token == "auth")
1010
}
1111

12+
/// Detect the kernel's pydantic `extra="forbid"` rejection — e.g.
13+
/// "body.tags: Extra inputs are not permitted" — and return the offending field
14+
/// name. This usually means the target OpenViking instance is on a different
15+
/// version than the CLI (the field is missing, renamed, or removed), so callers
16+
/// can surface a version-mismatch hint instead of the raw API error.
17+
pub(crate) fn extra_forbidden_field(message: &str) -> Option<String> {
18+
if !message.contains("Extra inputs are not permitted") {
19+
return None;
20+
}
21+
// The kernel reports the location as `body.<field>: Extra inputs ...`.
22+
let after = message.split("body.").nth(1)?;
23+
let field = after
24+
.split(|ch: char| ch == ':' || ch.is_whitespace())
25+
.next()?
26+
.trim();
27+
if field.is_empty() {
28+
None
29+
} else {
30+
Some(field.to_string())
31+
}
32+
}
33+
1234
#[cfg(test)]
1335
mod tests {
14-
use super::looks_like_auth_error;
36+
use super::{extra_forbidden_field, looks_like_auth_error};
37+
38+
#[test]
39+
fn extracts_extra_forbidden_field() {
40+
assert_eq!(
41+
extra_forbidden_field(
42+
"[INVALID_ARGUMENT] Invalid request parameters: body.tags: Extra inputs are not permitted."
43+
),
44+
Some("tags".to_string())
45+
);
46+
assert_eq!(
47+
extra_forbidden_field("body.args: Extra inputs are not permitted"),
48+
Some("args".to_string())
49+
);
50+
}
51+
52+
#[test]
53+
fn ignores_non_extra_forbidden_errors() {
54+
assert_eq!(extra_forbidden_field("API key is invalid"), None);
55+
assert_eq!(extra_forbidden_field("body.query: Field required"), None);
56+
}
1557

1658
#[test]
1759
fn detects_auth_errors() {

crates/ov_cli/src/error_ui.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use unicode_width::UnicodeWidthStr;
55

66
use crate::{
77
error::Error,
8-
error_classifier::looks_like_auth_error,
8+
error_classifier::{extra_forbidden_field, looks_like_auth_error},
99
i18n::{Language, copy},
1010
terminal_ui::{fit_to_display_width, truncate_to_display_width},
1111
theme,
@@ -241,6 +241,27 @@ pub(crate) fn report_for_runtime_error(command: impl Into<String>, error: &Error
241241
ErrorAction::new("ov config", copy(language, "Edit this config", "编辑这个配置")),
242242
ErrorAction::new("ov config switch", copy(language, "Use another config", "使用其他配置")),
243243
]),
244+
Error::Api { message, .. } if extra_forbidden_field(message).is_some() => {
245+
let field = extra_forbidden_field(message).unwrap_or_default();
246+
ErrorReport::new(
247+
copy(language, "OpenViking API Error", "OpenViking API 错误"),
248+
match language {
249+
Language::En => format!(
250+
"OpenViking rejected an unsupported field \"{field}\". This instance's version likely does not match your CLI (the field may be missing, renamed, or removed)."
251+
),
252+
Language::ZhCn => format!(
253+
"OpenViking 拒绝了不支持的字段 \"{field}\":该实例版本可能与当前 CLI 不匹配(字段可能缺失、改名或已被移除)。"
254+
),
255+
},
256+
)
257+
.with_command(command)
258+
.with_detail(message)
259+
.with_actions(vec![
260+
ErrorAction::new("ov health", copy(language, "Check the instance version", "查看实例版本")),
261+
ErrorAction::new("ov config validate", copy(language, "Check the active config", "检查当前配置")),
262+
ErrorAction::new("ov status", copy(language, "Check OpenViking status", "查看 OpenViking 状态")),
263+
])
264+
}
244265
Error::Api { message, .. } => ErrorReport::new(
245266
copy(language, "OpenViking API Error", "OpenViking API 错误"),
246267
api_error_message(language, message),

0 commit comments

Comments
 (0)