mirror of
https://wget.la/https://github.com/leookun/cursor-byok
synced 2026-10-04 02:52:55 +08:00
fix(tools): report the ReadLints paths that were never checked
ReadLints accepts an array of paths, but codec::request encodes only
paths[0] into the DiagnosticsArgs exec, and the exec protocol cannot
carry more than one path. Nothing downstream mentions the drop: for
{"paths": ["a.ts", "b.ts", "c.ts"]} the model is handed
"No diagnostics found in a.ts" with is_error false, so it concludes
b.ts and c.ts are clean when neither was ever opened. The existing
truncation notice cannot catch this, since it compares total_diagnostics
against a per-file diagnostics count.
Name the unread paths in the model-facing result, using the bracketed
notice convention already in this file and the ToolCall that output()
is already given (as task() and render::read() already use it).
Single-path calls are unchanged.
This does not close the capability gap. A real fix fans out one
DiagnosticsArgs exec per path and aggregates the results into the
repeated FileDiagnostics that ReadLintsToolSuccess already defines,
which needs multi-exec reservation and a completion barrier because
take_exec drops the pending entry on the first result. That is a
separate change; this one only stops the silent misinformation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
ee2592c469
commit
1269110615
@@ -12,7 +12,7 @@ pub(super) fn output(
|
||||
Message::WriteResult(value) => write(value),
|
||||
Message::DeleteResult(value) => delete(value),
|
||||
Message::GrepResult(value) => grep(value),
|
||||
Message::DiagnosticsResult(value) => diagnostics(value),
|
||||
Message::DiagnosticsResult(value) => diagnostics(value, call),
|
||||
Message::McpResult(value) => mcp(value),
|
||||
Message::ReadMcpResourceExecResult(value) => read_mcp(value),
|
||||
Message::SubagentResult(value) => task(value, call),
|
||||
@@ -212,14 +212,14 @@ fn grep_truncation(
|
||||
}
|
||||
}
|
||||
|
||||
fn diagnostics(value: &pb::DiagnosticsResult) -> Result<(String, bool)> {
|
||||
fn diagnostics(value: &pb::DiagnosticsResult, call: &ToolCall) -> Result<(String, bool)> {
|
||||
use pb::diagnostics_result::Result as R;
|
||||
match value
|
||||
.result
|
||||
.as_ref()
|
||||
.ok_or_else(|| missing("diagnostics"))?
|
||||
{
|
||||
R::Success(value) => Ok((diagnostics_success(value), false)),
|
||||
R::Success(value) => Ok((diagnostics_success(value, call), false)),
|
||||
R::Error(value) => Ok((value.error.clone(), true)),
|
||||
R::Rejected(value) => Ok((value.reason.clone(), true)),
|
||||
R::FileNotFound(value) => Ok((format!("file not found: {}", value.path), true)),
|
||||
@@ -227,9 +227,40 @@ fn diagnostics(value: &pb::DiagnosticsResult) -> Result<(String, bool)> {
|
||||
}
|
||||
}
|
||||
|
||||
fn diagnostics_success(value: &pb::DiagnosticsSuccess) -> String {
|
||||
/// `codec::request` encodes only `paths[0]` into the `DiagnosticsArgs` exec, and
|
||||
/// `DiagnosticsSuccess` carries a single `path`, so a multi-path ReadLints call
|
||||
/// only ever inspects the first entry. Name the rest instead of letting a clean
|
||||
/// result for one file read as a clean bill of health for all of them.
|
||||
fn unchecked_lint_paths(call: &ToolCall) -> Vec<&str> {
|
||||
call.arguments
|
||||
.get("paths")
|
||||
.and_then(serde_json::Value::as_array)
|
||||
.map(|paths| {
|
||||
paths
|
||||
.iter()
|
||||
.skip(1)
|
||||
.filter_map(serde_json::Value::as_str)
|
||||
.filter(|path| !path.is_empty())
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn diagnostics_success(value: &pb::DiagnosticsSuccess, call: &ToolCall) -> String {
|
||||
let unchecked = unchecked_lint_paths(call);
|
||||
let notice = (!unchecked.is_empty()).then(|| {
|
||||
format!(
|
||||
"[Only {} was checked; ReadLints reads one path per call. Not checked: {}]",
|
||||
value.path,
|
||||
unchecked.join(", ")
|
||||
)
|
||||
});
|
||||
if value.diagnostics.is_empty() {
|
||||
return format!("No diagnostics found in {}", value.path);
|
||||
let clean = format!("No diagnostics found in {}", value.path);
|
||||
return match notice {
|
||||
Some(notice) => format!("{clean}\n{notice}"),
|
||||
None => clean,
|
||||
};
|
||||
}
|
||||
let mut lines = value
|
||||
.diagnostics
|
||||
@@ -261,6 +292,7 @@ fn diagnostics_success(value: &pb::DiagnosticsSuccess) -> String {
|
||||
value.diagnostics.len()
|
||||
));
|
||||
}
|
||||
lines.extend(notice);
|
||||
lines.join("\n")
|
||||
}
|
||||
|
||||
@@ -413,3 +445,82 @@ fn creates_subagent(call: &ToolCall) -> bool {
|
||||
fn missing(name: &str) -> Error {
|
||||
Error::Protocol(format!("{name} returned no result"))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use serde_json::json;
|
||||
|
||||
fn read_lints(paths: serde_json::Value) -> ToolCall {
|
||||
ToolCall {
|
||||
index: 0,
|
||||
call_id: "lints".into(),
|
||||
model_call_id: "model:0".into(),
|
||||
name: "ReadLints".into(),
|
||||
arguments_text: String::new(),
|
||||
arguments: json!({ "paths": paths }),
|
||||
}
|
||||
}
|
||||
|
||||
fn clean_result(path: &str) -> pb::exec_client_message::Message {
|
||||
pb::exec_client_message::Message::DiagnosticsResult(pb::DiagnosticsResult {
|
||||
result: Some(pb::diagnostics_result::Result::Success(
|
||||
pb::DiagnosticsSuccess {
|
||||
path: path.into(),
|
||||
diagnostics: Vec::new(),
|
||||
total_diagnostics: 0,
|
||||
},
|
||||
)),
|
||||
})
|
||||
}
|
||||
|
||||
/// The codec encodes only `paths[0]` into the DiagnosticsArgs exec, so a
|
||||
/// clean result for that one path must not read as "these files are all
|
||||
/// clean" for the paths that were never looked at.
|
||||
#[test]
|
||||
fn read_lints_names_the_paths_it_did_not_check() {
|
||||
let call = read_lints(json!(["a.ts", "b.ts", "c.ts"]));
|
||||
let (content, is_error) = output(&clean_result("a.ts"), &call).unwrap();
|
||||
assert!(!is_error);
|
||||
assert!(content.contains("a.ts"));
|
||||
assert!(
|
||||
content.contains("b.ts") && content.contains("c.ts"),
|
||||
"unchecked paths must be reported, got: {content}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The overwhelmingly common single-path call must be byte-for-byte
|
||||
/// unchanged.
|
||||
#[test]
|
||||
fn read_lints_single_path_result_is_unchanged() {
|
||||
let call = read_lints(json!(["a.ts"]));
|
||||
let (content, _) = output(&clean_result("a.ts"), &call).unwrap();
|
||||
assert_eq!(content, "No diagnostics found in a.ts");
|
||||
}
|
||||
|
||||
/// The notice belongs on a result that did report diagnostics too: those
|
||||
/// diagnostics are still only a.ts's.
|
||||
#[test]
|
||||
fn read_lints_reports_unchecked_paths_alongside_diagnostics() {
|
||||
let call = read_lints(json!(["a.ts", "b.ts"]));
|
||||
let message = pb::exec_client_message::Message::DiagnosticsResult(pb::DiagnosticsResult {
|
||||
result: Some(pb::diagnostics_result::Result::Success(
|
||||
pb::DiagnosticsSuccess {
|
||||
path: "a.ts".into(),
|
||||
diagnostics: vec![pb::Diagnostic {
|
||||
message: "unused import".into(),
|
||||
severity: pb::DiagnosticSeverity::Warning as i32,
|
||||
..Default::default()
|
||||
}],
|
||||
total_diagnostics: 1,
|
||||
},
|
||||
)),
|
||||
});
|
||||
let (content, _) = output(&message, &call).unwrap();
|
||||
assert!(content.contains("unused import"));
|
||||
assert!(
|
||||
content.contains("Not checked: b.ts"),
|
||||
"unchecked paths must be reported, got: {content}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user