diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6a0cafd1fd..907487acb8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -616,4 +616,3 @@ jobs: curl -fsSL -X POST "$WEBHOOK_URL" \ -H "Content-Type: application/json" \ -d "{\"action\":\"published\",\"release\":{\"tag_name\":\"${RELEASE_TAG}\",\"html_url\":\"https://github.com/${{ github.repository }}/releases/tag/${RELEASE_TAG}\"}}" - diff --git a/docs/contracts/delivery-evidence-v1.json b/docs/contracts/delivery-evidence-v1.json index e6d78b74b1..991032a60d 100644 --- a/docs/contracts/delivery-evidence-v1.json +++ b/docs/contracts/delivery-evidence-v1.json @@ -1 +1 @@ -{"checklist":[{"evidence":{"evidence_class":"rust-integration-test","harness_command":"cargo test --manifest-path rust/Cargo.toml --test main suite::setup_ci_smoke::setup_bootstrap_doctor_status_json_smoke -- --exact --test-threads=1","path":"rust/tests/suite/setup_ci_smoke.rs","selector":"fn setup_bootstrap_doctor_status_json_smoke()","sha256":"9645a4ffcb9b4ad1b9b33d63aab8709f54265607482f1a712c0810803ca12998"},"external_acceptance_required":true,"local_status":"component-evidence","stage":"setup"},{"evidence":{"evidence_class":"rust-integration-test","harness_command":"cargo test --manifest-path rust/Cargo.toml --test main suite::onboard_doctor_clean::onboard_yes_leaves_doctor_fully_green -- --exact","path":"rust/tests/suite/onboard_doctor_clean.rs","selector":"fn onboard_yes_leaves_doctor_fully_green()","sha256":"3268b39b4e6bb7c95f712d40a3ba857a644e51bce3367810a3bf5ebb76faf256"},"external_acceptance_required":true,"local_status":"component-evidence","stage":"doctor"},{"evidence":{"evidence_class":"rust-unit-test","harness_command":"cargo test --manifest-path rust/Cargo.toml --lib doctor::migrate::tests::contract_outcome_reports_frozen_set -- --exact","path":"rust/src/doctor/migrate.rs","selector":"fn contract_outcome_reports_frozen_set()","sha256":"68c7a247dcd45de6bf4324c703df58bfeca4699dcf4ee51044d748ba2486210b"},"external_acceptance_required":true,"local_status":"component-evidence","stage":"upgrade"},{"evidence":{"evidence_class":"python-unittest","harness_command":"python3 tests/delivery/test_rehearse_delivery.py DeliveryRehearsalTests.test_rehearses_verified_candidate_and_rollback_without_deployment","path":"tests/delivery/test_rehearse_delivery.py","selector":"def test_rehearses_verified_candidate_and_rollback_without_deployment(self):","sha256":"9c8841eea94ae560ca549e5d514c003eb4e1349c748e6b85dc0725e6bee9363a"},"external_acceptance_required":true,"local_status":"offline-rehearsal","stage":"rollback"},{"evidence":{"evidence_class":"rust-integration-test","harness_command":"cargo test --manifest-path rust/Cargo.toml --test main suite::cli_characterization::uninstall_dry_run_exits_zero -- --exact","path":"rust/tests/suite/cli_characterization.rs","selector":"fn uninstall_dry_run_exits_zero()","sha256":"a9fb18a382eb328e79f067415aeabafcbbf2a867dadc96bb6cf0fbb6bff0ddbe"},"external_acceptance_required":true,"local_status":"component-evidence","stage":"uninstall"}],"delivery_manifest":{"fixture":{"path":"tests/delivery/valid/delivery-manifest.json","selector":"\"schema_version\":\"leanctx.delivery/v1\"","sha256":"071b28af209804c7f1e0d5a25bae5fe94ebfe45c51983a416d656c5a84917fd6"},"schema":{"path":"docs/contracts/delivery-manifest-v1.schema.json","selector":"https://leanctx.dev/contracts/delivery-manifest-v1.schema.json","sha256":"80ac91c631969a9ce305d8701d3eb0ca92b69b6defcd3a2ef6f9b4a80c7ae291"},"trust_root":{"path":"tests/delivery/valid/release-trust-root.json","selector":"\"algorithm\":\"Ed25519\"","sha256":"660fbf46674b8a16166026551e50b032a1ccf58a4d260c4bb4e4218235066083"},"verifier":{"path":"scripts/verify-delivery-manifest.py","selector":"def verify(manifest_path, root, trust_root=None, rotation_plan=None):","sha256":"636e85f32ce7cd7bfb7a6257b6abf78e4a5d3719c063537ccaa04be9163f21cc"}},"owner":{"handle":"@yvgude","source":{"path":".github/CODEOWNERS","selector":"* @yvgude","sha256":"9c52bffaef2f21976aac61d2ce42311a0ae6e393a95006a36f54e1d1edc72656"}},"release":{"publish_channels":[{"id":"engine-release","source_path":".github/workflows/release.yml","source_sha256":"51e665df5e98d6ba35ff1df44ce6362f3096a07e6ce305fc1e963f41947c35f0","trigger_pattern":"v[0-9]*"},{"id":"sdk-release","source_path":".github/workflows/publish-sdk.yml","source_sha256":"11d2820857f5d2284a713c249136ffdfd7051ab7d8151d5e50f4df80e8b40f28","trigger_pattern":"sdk-v[0-9]*"},{"id":"client-release","source_path":".github/workflows/publish-clients.yml","source_sha256":"90c28cb48887b551c79b4a265c14d084e90b02d4b8eec20fcd00e60dd858a7c1","trigger_pattern":"client-v[0-9]*"}],"targets":[{"artifact":"x86_64-unknown-linux-gnu","certification":"not-asserted","runner":"ubuntu-22.04","target":"x86_64-unknown-linux-gnu"},{"artifact":"x86_64-unknown-linux-gnu-cuda","certification":"not-asserted","runner":"ubuntu-22.04","target":"x86_64-unknown-linux-gnu"},{"artifact":"aarch64-unknown-linux-gnu","certification":"not-asserted","runner":"ubuntu-22.04","target":"aarch64-unknown-linux-gnu"},{"artifact":"x86_64-unknown-linux-musl","certification":"not-asserted","runner":"ubuntu-22.04","target":"x86_64-unknown-linux-musl"},{"artifact":"aarch64-unknown-linux-musl","certification":"not-asserted","runner":"ubuntu-22.04","target":"aarch64-unknown-linux-musl"},{"artifact":"x86_64-apple-darwin","certification":"not-asserted","runner":"macos-latest","target":"x86_64-apple-darwin"},{"artifact":"aarch64-apple-darwin","certification":"not-asserted","runner":"macos-latest","target":"aarch64-apple-darwin"},{"artifact":"x86_64-pc-windows-msvc","certification":"not-asserted","runner":"windows-latest","target":"x86_64-pc-windows-msvc"},{"artifact":"x86_64-pc-windows-gnu","certification":"not-asserted","runner":"windows-latest","target":"x86_64-pc-windows-gnu"}],"version_gates":[{"id":"release-tag","source":{"path":"scripts/check-release-tag.py","selector":"def verify_tag(tag: str, root: Path) -> str:","sha256":"35147606b1775ac0b6ce191e0743a2253649e54a666c77112c481ee96de8a2ea"},"workflow_selector":"python3 scripts/check-release-tag.py \"${GITHUB_REF_NAME}\""},{"id":"sdk-contract-version","source":{"path":"scripts/check-sdk-versions.py","selector":"def main() -> int:","sha256":"c30437f0e401b52b8464f602f8ed4aa737ff85caa5b8c0823534e3c0ccf406c2"},"workflow_selector":"python3 scripts/check-sdk-versions.py"},{"id":"package-version","source":{"path":"scripts/check-package-versions.py","selector":"def main() -> int:","sha256":"3dd0eefff0fa148eadecd2023f49bff04d00469d1d2c6bb8cb92fe7ad5681b9d"},"workflow_selector":"python3 scripts/check-package-versions.py"}],"workflow":{"path":".github/workflows/release.yml","selector":"name: Release\n","sha256":"51e665df5e98d6ba35ff1df44ce6362f3096a07e6ce305fc1e963f41947c35f0"},"workflow_tag_glob":"v[0-9]*"},"requirement_ids":["BC-04","EN-05","RG-07","RG-11"],"schema_version":"leanctx.delivery-evidence/v1","scope":{"customer_delivery_acceptance":false,"external_operational_acceptance":false,"local_contract_consistency":true,"os_certification":false,"zero_downtime_verified":false},"status":"partial"} +{"checklist":[{"evidence":{"evidence_class":"rust-integration-test","harness_command":"cargo test --manifest-path rust/Cargo.toml --test main suite::setup_ci_smoke::setup_bootstrap_doctor_status_json_smoke -- --exact --test-threads=1","path":"rust/tests/suite/setup_ci_smoke.rs","selector":"fn setup_bootstrap_doctor_status_json_smoke()","sha256":"9645a4ffcb9b4ad1b9b33d63aab8709f54265607482f1a712c0810803ca12998"},"external_acceptance_required":true,"local_status":"component-evidence","stage":"setup"},{"evidence":{"evidence_class":"rust-integration-test","harness_command":"cargo test --manifest-path rust/Cargo.toml --test main suite::onboard_doctor_clean::onboard_yes_leaves_doctor_fully_green -- --exact","path":"rust/tests/suite/onboard_doctor_clean.rs","selector":"fn onboard_yes_leaves_doctor_fully_green()","sha256":"3268b39b4e6bb7c95f712d40a3ba857a644e51bce3367810a3bf5ebb76faf256"},"external_acceptance_required":true,"local_status":"component-evidence","stage":"doctor"},{"evidence":{"evidence_class":"rust-unit-test","harness_command":"cargo test --manifest-path rust/Cargo.toml --lib doctor::migrate::tests::contract_outcome_reports_frozen_set -- --exact","path":"rust/src/doctor/migrate.rs","selector":"fn contract_outcome_reports_frozen_set()","sha256":"68c7a247dcd45de6bf4324c703df58bfeca4699dcf4ee51044d748ba2486210b"},"external_acceptance_required":true,"local_status":"component-evidence","stage":"upgrade"},{"evidence":{"evidence_class":"python-unittest","harness_command":"python3 tests/delivery/test_rehearse_delivery.py DeliveryRehearsalTests.test_rehearses_verified_candidate_and_rollback_without_deployment","path":"tests/delivery/test_rehearse_delivery.py","selector":"def test_rehearses_verified_candidate_and_rollback_without_deployment(self):","sha256":"9c8841eea94ae560ca549e5d514c003eb4e1349c748e6b85dc0725e6bee9363a"},"external_acceptance_required":true,"local_status":"offline-rehearsal","stage":"rollback"},{"evidence":{"evidence_class":"rust-integration-test","harness_command":"cargo test --manifest-path rust/Cargo.toml --test main suite::cli_characterization::uninstall_dry_run_exits_zero -- --exact","path":"rust/tests/suite/cli_characterization.rs","selector":"fn uninstall_dry_run_exits_zero()","sha256":"a9fb18a382eb328e79f067415aeabafcbbf2a867dadc96bb6cf0fbb6bff0ddbe"},"external_acceptance_required":true,"local_status":"component-evidence","stage":"uninstall"}],"delivery_manifest":{"fixture":{"path":"tests/delivery/valid/delivery-manifest.json","selector":"\"schema_version\":\"leanctx.delivery/v1\"","sha256":"071b28af209804c7f1e0d5a25bae5fe94ebfe45c51983a416d656c5a84917fd6"},"schema":{"path":"docs/contracts/delivery-manifest-v1.schema.json","selector":"https://leanctx.dev/contracts/delivery-manifest-v1.schema.json","sha256":"80ac91c631969a9ce305d8701d3eb0ca92b69b6defcd3a2ef6f9b4a80c7ae291"},"trust_root":{"path":"tests/delivery/valid/release-trust-root.json","selector":"\"algorithm\":\"Ed25519\"","sha256":"660fbf46674b8a16166026551e50b032a1ccf58a4d260c4bb4e4218235066083"},"verifier":{"path":"scripts/verify-delivery-manifest.py","selector":"def verify(manifest_path, root, trust_root=None, rotation_plan=None):","sha256":"636e85f32ce7cd7bfb7a6257b6abf78e4a5d3719c063537ccaa04be9163f21cc"}},"owner":{"handle":"@yvgude","source":{"path":".github/CODEOWNERS","selector":"* @yvgude","sha256":"9c52bffaef2f21976aac61d2ce42311a0ae6e393a95006a36f54e1d1edc72656"}},"release":{"publish_channels":[{"id":"engine-release","source_path":".github/workflows/release.yml","source_sha256":"f760e7050eaad358bd739b57c52c416015c6970820c120f1e46cd8adecfd4a1f","trigger_pattern":"v[0-9]*"},{"id":"sdk-release","source_path":".github/workflows/publish-sdk.yml","source_sha256":"11d2820857f5d2284a713c249136ffdfd7051ab7d8151d5e50f4df80e8b40f28","trigger_pattern":"sdk-v[0-9]*"},{"id":"client-release","source_path":".github/workflows/publish-clients.yml","source_sha256":"90c28cb48887b551c79b4a265c14d084e90b02d4b8eec20fcd00e60dd858a7c1","trigger_pattern":"client-v[0-9]*"}],"targets":[{"artifact":"x86_64-unknown-linux-gnu","certification":"not-asserted","runner":"ubuntu-22.04","target":"x86_64-unknown-linux-gnu"},{"artifact":"x86_64-unknown-linux-gnu-cuda","certification":"not-asserted","runner":"ubuntu-22.04","target":"x86_64-unknown-linux-gnu"},{"artifact":"aarch64-unknown-linux-gnu","certification":"not-asserted","runner":"ubuntu-22.04","target":"aarch64-unknown-linux-gnu"},{"artifact":"x86_64-unknown-linux-musl","certification":"not-asserted","runner":"ubuntu-22.04","target":"x86_64-unknown-linux-musl"},{"artifact":"aarch64-unknown-linux-musl","certification":"not-asserted","runner":"ubuntu-22.04","target":"aarch64-unknown-linux-musl"},{"artifact":"x86_64-apple-darwin","certification":"not-asserted","runner":"macos-latest","target":"x86_64-apple-darwin"},{"artifact":"aarch64-apple-darwin","certification":"not-asserted","runner":"macos-latest","target":"aarch64-apple-darwin"},{"artifact":"x86_64-pc-windows-msvc","certification":"not-asserted","runner":"windows-latest","target":"x86_64-pc-windows-msvc"},{"artifact":"x86_64-pc-windows-gnu","certification":"not-asserted","runner":"windows-latest","target":"x86_64-pc-windows-gnu"}],"version_gates":[{"id":"release-tag","source":{"path":"scripts/check-release-tag.py","selector":"def verify_tag(tag: str, root: Path) -> str:","sha256":"35147606b1775ac0b6ce191e0743a2253649e54a666c77112c481ee96de8a2ea"},"workflow_selector":"python3 scripts/check-release-tag.py \"${GITHUB_REF_NAME}\""},{"id":"sdk-contract-version","source":{"path":"scripts/check-sdk-versions.py","selector":"def main() -> int:","sha256":"c30437f0e401b52b8464f602f8ed4aa737ff85caa5b8c0823534e3c0ccf406c2"},"workflow_selector":"python3 scripts/check-sdk-versions.py"},{"id":"package-version","source":{"path":"scripts/check-package-versions.py","selector":"def main() -> int:","sha256":"3dd0eefff0fa148eadecd2023f49bff04d00469d1d2c6bb8cb92fe7ad5681b9d"},"workflow_selector":"python3 scripts/check-package-versions.py"}],"workflow":{"path":".github/workflows/release.yml","selector":"name: Release\n","sha256":"f760e7050eaad358bd739b57c52c416015c6970820c120f1e46cd8adecfd4a1f"},"workflow_tag_glob":"v[0-9]*"},"requirement_ids":["BC-04","EN-05","RG-07","RG-11"],"schema_version":"leanctx.delivery-evidence/v1","scope":{"customer_delivery_acceptance":false,"external_operational_acceptance":false,"local_contract_consistency":true,"os_certification":false,"zero_downtime_verified":false},"status":"partial"} diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 55c8718237..ddb9865635 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -496,9 +496,9 @@ dependencies = [ [[package]] name = "chacha20" -version = "0.10.1" +version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81" +checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06" dependencies = [ "cfg-if", "cpufeatures 0.3.0", @@ -3043,7 +3043,7 @@ version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c7f5fa3a058cd35567ef9bfa5e75732bee0f9e4c55fa90477bef2dfcdbc4be80" dependencies = [ - "chacha20 0.10.1", + "chacha20 0.10.2", "getrandom 0.4.3", "rand_core 0.10.1", ] diff --git a/rust/src/core/mcp_manifest.rs b/rust/src/core/mcp_manifest.rs index 0189a45086..27b1a3fbb2 100644 --- a/rust/src/core/mcp_manifest.rs +++ b/rust/src/core/mcp_manifest.rs @@ -5,7 +5,10 @@ use serde_json::{Value, json}; use crate::core::contracts::MCP_MANIFEST_SCHEMA_VERSION; -const READ_MODES: [&str; 10] = [ +/// The mode list advertised in `ctx_read`'s schema. Shared with the runtime so +/// an unknown-mode error can name exactly what the schema promises — the two +/// drifting apart is what let `diff` be advertised but rejected (#1584). +pub(crate) const READ_MODES: [&str; 10] = [ "auto", "full", "map", diff --git a/rust/src/core/savings_footer.rs b/rust/src/core/savings_footer.rs index 906ad363af..0e5ec96aba 100644 --- a/rust/src/core/savings_footer.rs +++ b/rust/src/core/savings_footer.rs @@ -50,7 +50,10 @@ impl Drop for ModeGuard { } } -fn current_mode() -> Option { +/// The mode the innermost live `ModeGuard` was created with, if any. Lets a +/// deep renderer name the mode the caller actually asked for without threading +/// it through every signature (used by the no-compression banner). +pub(crate) fn current_mode() -> Option { CURRENT_MODE.with(|m| m.borrow().clone()) } diff --git a/rust/src/core/task_relevance.rs b/rust/src/core/task_relevance.rs index 3fe554a413..1a692b013e 100644 --- a/rust/src/core/task_relevance.rs +++ b/rust/src/core/task_relevance.rs @@ -543,9 +543,44 @@ pub fn information_bottleneck_filter_typed( task_type: Option, force_keep: &[String], ) -> String { + let selected = ib_select(content, task_keywords, budget_ratio, task_type, force_keep); + if selected.is_empty() { + return String::new(); + } + let body: Vec<&str> = selected.iter().map(|(_, line)| *line).collect(); + let body = body.join("\n"); + if task_keywords.is_empty() { + body + } else { + format!("[task: {}]\n{body}", task_keywords.join(", ")) + } +} + +/// Ranked line selection behind every task-mode view: scores each line, applies +/// the MMR redundancy penalty, and hands back the winners **in source order** +/// as `(line_index, line)` pairs. +/// +/// Two properties matter to callers and were both missing before (#1589): +/// +/// * **Blank lines are never candidates.** A blank line scored 0.05 — above the +/// tail of a real ranking — and then bypassed the MMR similarity penalty +/// entirely, because an empty token set has no overlap to punish. Once the +/// ranked candidates fell below that floor, blanks won every remaining slot; +/// a field report saw ~35 consecutive blank lines where the body should have +/// been. +/// * **The result is ordered by line number, not by score.** A task view that +/// reorders a file's fragments is not a compressed file, it is a different +/// file — it reads as though the code executes in that order. +pub fn ib_select<'a>( + content: &'a str, + task_keywords: &[String], + budget_ratio: f64, + task_type: Option, + force_keep: &[String], +) -> Vec<(usize, &'a str)> { let lines: Vec<&str> = content.lines().collect(); if lines.is_empty() { - return String::new(); + return Vec::new(); } let n = lines.len(); @@ -577,10 +612,16 @@ pub fn information_bottleneck_filter_typed( let mut scored_lines: Vec<(usize, &str, f64)> = lines .iter() .enumerate() - .map(|(i, line)| { + .filter_map(|(i, line)| { let trimmed = line.trim(); if trimmed.is_empty() { - return (i, *line, 0.05); + // #1589: a blank line carries no information to select *for*. + // Only an explicit protect token can pull one into the view. + return super::protect::line_is_protected(line, force_keep).then_some(( + i, + *line, + f64::INFINITY, + )); } let line_lower = trimmed.to_lowercase(); @@ -653,7 +694,7 @@ pub fn information_bottleneck_filter_typed( score }; - (i, *line, score) + Some((i, *line, score)) }) .collect(); @@ -668,27 +709,10 @@ pub fn information_bottleneck_filter_typed( scored_lines.sort_by(|a, b| b.2.partial_cmp(&a.2).unwrap_or(std::cmp::Ordering::Equal)); - let selected = mmr_select(&scored_lines, budget, 0.3); - - let mut output_lines: Vec<&str> = Vec::with_capacity(budget + 1); - - if !kw_lower.is_empty() { - output_lines.push(""); - } - - for (_, line, _) in &selected { - output_lines.push(line); - } - - if !kw_lower.is_empty() { - let summary = format!("[task: {}]", task_keywords.join(", ")); - let mut result = summary; - result.push('\n'); - result.push_str(&output_lines[1..].to_vec().join("\n")); - return result; - } - - output_lines.join("\n") + let mut selected = mmr_select(&scored_lines, budget, 0.3); + // Rank decided *what* survives; the file decides in which order it is read. + selected.sort_by_key(|(i, _, _)| *i); + selected.into_iter().map(|(i, line, _)| (i, line)).collect() } /// Maximum Marginal Relevance selection — greedy selection that penalizes @@ -923,18 +947,60 @@ mod tests { } #[test] - fn info_bottleneck_score_sorted() { + fn info_bottleneck_preserves_source_order() { + // #1589: selection is ranked, output is not. A task view that reorders + // fragments reads as if the code ran in that order. let content = "fn important() {\n let x = 1;\n let y = 2;\n let z = 3;\n}\n}\n"; let result = information_bottleneck_filter(content, &[], 0.6, &[]); let lines: Vec<&str> = result.lines().collect(); let def_pos = lines.iter().position(|l| l.contains("fn important")); let brace_pos = lines.iter().position(|l| l.trim() == "}"); if let (Some(d), Some(b)) = (def_pos, brace_pos) { + assert!(d < b, "the definition precedes its closing brace in source"); + } + + let numbered = "alpha_one\nbeta_two\ngamma_three\ndelta_four\nepsilon_five\nzeta_six\n"; + let selected = ib_select(numbered, &["gamma".to_string()], 0.6, None, &[]); + let indices: Vec = selected.iter().map(|(i, _)| *i).collect(); + let mut sorted = indices.clone(); + sorted.sort_unstable(); + assert_eq!( + indices, sorted, + "selection must be returned in source order" + ); + } + + #[test] + fn info_bottleneck_never_selects_blank_lines() { + // #1589: blanks scored 0.05 and skipped the MMR similarity penalty, so + // they out-competed real content once the ranking tail fell below that + // floor — a field report saw ~35 consecutive blank lines. + let mut content = String::new(); + for i in 0..40 { + content.push_str(&format!("fn handler_{i}(input: &str) -> usize {{\n")); + content.push_str("\n\n\n"); + content.push_str(" input.len()\n}\n\n\n"); + } + let result = information_bottleneck_filter(&content, &["handler".to_string()], 0.3, &[]); + for line in result.lines().skip(1) { assert!( - d < b, - "definitions should appear before closing braces in score-sorted output" + !line.trim().is_empty(), + "no blank line may consume a slot in the task budget" ); } + assert!(result.contains("handler_"), "real content must survive"); + } + + #[test] + fn protected_blank_line_still_survives() { + // The blank-line ban is a ranking rule, not a censor: an explicit + // protect token still pulls its line through (#709). + let content = "fn a() {}\n \nfn b() {}\n"; + let kept = ib_select(content, &["a".to_string()], 0.1, None, &[" ".to_string()]); + assert!( + kept.iter().any(|(_, l)| l.trim().is_empty()), + "an explicitly protected line is kept even when blank" + ); } #[test] diff --git a/rust/src/server/context_gate.rs b/rust/src/server/context_gate.rs index 2d41b6a7a9..9fe2c08961 100644 --- a/rust/src/server/context_gate.rs +++ b/rust/src/server/context_gate.rs @@ -57,9 +57,23 @@ pub fn pre_dispatch_read( project_root: Option<&str>, pressure: Option<&PressureAction>, ) -> PreDispatchResult { - pre_dispatch_read_for_agent(path, requested_mode, task, project_root, pressure, None) + pre_dispatch_read_for_agent( + path, + requested_mode, + task, + project_root, + pressure, + None, + false, + ) } +/// `fresh` is the caller's explicit escape hatch (#1588). Bounce-prevention and +/// intent-target exist to stop an agent from *drifting* into a compressed view +/// it will immediately re-read in full; neither should be able to pin a file to +/// `full` forever. A caller who passes `fresh=true` has said, in the request +/// itself, that they want this view recomputed — honour it, or those modes are +/// simply unreachable for the rest of the session. pub fn pre_dispatch_read_for_agent( path: &str, requested_mode: &str, @@ -67,6 +81,7 @@ pub fn pre_dispatch_read_for_agent( project_root: Option<&str>, pressure: Option<&PressureAction>, agent_id: Option<&str>, + fresh: bool, ) -> PreDispatchResult { let no_change = PreDispatchResult { overridden_mode: None, @@ -105,7 +120,8 @@ pub fn pre_dispatch_read_for_agent( if is_precise_pinned_mode(requested_mode) { return result; } - let rest = pre_dispatch_inner(path, requested_mode, task, project_root, pressure); + let rest = + pre_dispatch_inner(path, requested_mode, task, project_root, pressure, fresh); return PreDispatchResult { budget_warning: result.budget_warning, ..rest @@ -115,7 +131,34 @@ pub fn pre_dispatch_read_for_agent( } } - pre_dispatch_inner(path, requested_mode, task, project_root, pressure) + pre_dispatch_inner(path, requested_mode, task, project_root, pressure, fresh) +} + +/// Does `norm` (a normalized path) actually name the intent target `target`? +/// +/// The old rule was `norm.ends_with(t) || norm.contains(t)`, which matched on +/// any substring: a task mentioning `src` or `read` pinned every path +/// containing those letters to `full` (#1588), including matches that land in +/// the middle of a longer component (`print` inside `printer.rs`). A target now +/// has to line up with a whole path component or the file stem, and be long +/// enough to be a name rather than noise. +fn path_matches_target(norm: &str, target: &str) -> bool { + const MIN_TARGET_LEN: usize = 4; + let target = target.trim().trim_matches('/'); + if target.len() < MIN_TARGET_LEN { + return false; + } + // A target that is itself a path suffix (`src/server/context_gate.rs`). + if norm == target || norm.ends_with(&format!("/{target}")) { + return true; + } + // Otherwise: a whole path component, with or without its extension. + norm.split('/').any(|component| { + component == target + || component + .rsplit_once('.') + .is_some_and(|(stem, _)| stem == target) + }) } fn pre_dispatch_inner( @@ -124,6 +167,7 @@ fn pre_dispatch_inner( task: Option<&str>, project_root: Option<&str>, pressure: Option<&PressureAction>, + fresh: bool, ) -> PreDispatchResult { let no_change = PreDispatchResult { overridden_mode: None, @@ -169,7 +213,11 @@ fn pre_dispatch_inner( } } - if let Ok(bt) = crate::core::bounce_tracker::global().lock() + // `fresh` is the documented way out (#1588): the caller re-requested this + // view deliberately, so the "you keep bouncing back to full" heuristic has + // nothing left to prevent. + if !fresh + && let Ok(bt) = crate::core::bounce_tracker::global().lock() && bt.should_force_full(path) { return PreDispatchResult { @@ -182,13 +230,12 @@ fn pre_dispatch_inner( }; } - if let Some(task_str) = task { + if let Some(task_str) = task + && !fresh + { let intent = crate::core::intent_engine::StructuredIntent::from_query(task_str); let norm = crate::core::pathutil::normalize_tool_path(path); - let is_target = intent - .targets - .iter() - .any(|t| norm.ends_with(t) || norm.contains(t)); + let is_target = intent.targets.iter().any(|t| path_matches_target(&norm, t)); if is_target { return PreDispatchResult { overridden_mode: Some("full".to_string()), @@ -758,6 +805,89 @@ mod tests { ); } + #[test] + fn fresh_escapes_bounce_prevention() { + // #1588: after an edit the gate pins the file to `full`. Without an + // escape hatch, signatures/map/reference stay unreachable for the rest + // of the session — `fresh=true` is that hatch. + { + let mut bt = crate::core::bounce_tracker::global() + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + bt.set_seq(301); + bt.record_edit("fresh-escape-1586.rs"); + bt.set_seq(302); + } + + let pinned = pre_dispatch_read("fresh-escape-1586.rs", "signatures", None, None, None); + assert_eq!( + pinned.overridden_mode.as_deref(), + Some("full"), + "precondition: the tracker must actually be armed for this path" + ); + + let escaped = pre_dispatch_read_for_agent( + "fresh-escape-1586.rs", + "signatures", + None, + None, + None, + None, + true, + ); + assert!( + escaped.overridden_mode.is_none(), + "fresh=true must keep the requested mode reachable" + ); + } + + #[test] + fn intent_target_matches_whole_components_only() { + // #1588: substring matching pinned unrelated files to `full`. + assert!(path_matches_target( + "/repo/src/server/context_gate.rs", + "context_gate" + )); + assert!(path_matches_target( + "/repo/src/server/context_gate.rs", + "context_gate.rs" + )); + assert!(path_matches_target( + "/repo/src/server/context_gate.rs", + "src/server/context_gate.rs" + )); + + assert!( + !path_matches_target("/repo/src/printer/link_gate.rs", "print"), + "a target must not match the middle of a longer component" + ); + assert!( + !path_matches_target("/repo/src/server/context_gate.rs", "src"), + "targets below the minimum length are noise, not names" + ); + assert!(!path_matches_target( + "/repo/src/server/context_gate.rs", + "gate" + )); + } + + #[test] + fn fresh_escapes_intent_target() { + let result = pre_dispatch_read_for_agent( + "/repo/src/server/context_gate.rs", + "reference", + Some("fix context_gate overrides"), + None, + None, + None, + true, + ); + assert!( + result.overridden_mode.is_none(), + "fresh=true must also escape the intent-target override" + ); + } + #[test] fn pre_dispatch_no_override_without_signals() { let result = pre_dispatch_read("src/unknown.rs", "auto", None, None, None); diff --git a/rust/src/tools/ctx_edit/implementation.rs b/rust/src/tools/ctx_edit/implementation.rs index eaf100a560..5c759d3179 100644 --- a/rust/src/tools/ctx_edit/implementation.rs +++ b/rust/src/tools/ctx_edit/implementation.rs @@ -19,7 +19,10 @@ pub struct EditParams { pub replace_all: bool, pub create: bool, /// Optional preimage guards. If provided, ctx_edit fails if the current file preimage differs. - pub expected_md5: Option, + /// + /// The content guard is BLAKE3, named for what it is (#1592); the wire + /// parameter `expected_md5` stays accepted as an alias for existing callers. + pub expected_blake3: Option, pub expected_size: Option, pub expected_mtime_ms: Option, /// Optional backup before writing. @@ -59,12 +62,12 @@ fn verify_expected_preimage(pre: &FilePreimage, params: &EditParams) -> Result<( params.path, expected, pre.fp.mtime_ms )); } - if let Some(expected) = params.expected_md5.as_deref() - && expected != pre.fp.md5 + if let Some(expected) = params.expected_blake3.as_deref() + && expected != pre.fp.blake3 { return Err(format!( - "ERROR: preimage mismatch for {}: expected_md5={}, actual_md5={}", - params.path, expected, pre.fp.md5 + "ERROR: preimage mismatch for {}: expected_blake3={}, actual_blake3={}", + params.path, expected, pre.fp.blake3 )); } Ok(()) @@ -549,14 +552,14 @@ fn do_replace( let post_fp = FileFingerprint { size: new_content.len() as u64, mtime_ms: post_mtime_ms, - md5: crate::core::hasher::hash_hex(new_content.as_bytes()), + blake3: crate::core::hasher::hash_hex(new_content.as_bytes()), }; let mut out = format!( "✓ {short}: {replaced_str}, {delta_str} lines ({old_tokens}→{new_tokens} tok)\n\ -preimage: bytes={}, mtime_ms={}, md5={}\n\ -postimage: bytes={}, mtime_ms={}, md5={}", - pre.fp.size, pre.fp.mtime_ms, pre.fp.md5, post_fp.size, post_fp.mtime_ms, post_fp.md5 +preimage: bytes={}, mtime_ms={}, blake3={}\n\ +postimage: bytes={}, mtime_ms={}, blake3={}", + pre.fp.size, pre.fp.mtime_ms, pre.fp.blake3, post_fp.size, post_fp.mtime_ms, post_fp.blake3 ); if let Some(bp) = backup_path { out.push_str(&format!("\nbackup: {bp}")); diff --git a/rust/src/tools/ctx_edit/tests.rs b/rust/src/tools/ctx_edit/tests.rs index d197193637..df1ada3fd5 100644 --- a/rust/src/tools/ctx_edit/tests.rs +++ b/rust/src/tools/ctx_edit/tests.rs @@ -17,7 +17,7 @@ mod tests { new_string: new.to_string(), replace_all, create, - expected_md5: None, + expected_blake3: None, expected_size: None, expected_mtime_ms: None, backup: false, @@ -417,11 +417,11 @@ mod tests { } #[test] - fn expected_md5_mismatch_fails_without_writing() { + fn expected_blake3_mismatch_fails_without_writing() { let f = make_temp("aaa\n"); let mut cache = SessionCache::new(); let mut p = mk_params(f.path(), "aaa", "bbb", false, false); - p.expected_md5 = Some("deadbeef".to_string()); + p.expected_blake3 = Some("deadbeef".to_string()); let result = handle(&mut cache, &p); assert!( result.contains("preimage mismatch"), @@ -431,6 +431,42 @@ mod tests { assert_eq!(content, "aaa\n"); } + /// #1592: the receipt used to label its 64-hex digest `md5=`, sending a + /// user off to verify a hash against three algorithms none of which could + /// ever match. The digest is BLAKE3; the receipt now says so, and the + /// mismatch error names the same algorithm. + #[test] + fn receipt_labels_the_digest_by_its_actual_algorithm() { + let f = make_temp("aaa\n"); + let mut cache = SessionCache::new(); + let out = handle(&mut cache, &mk_params(f.path(), "aaa", "bbb", false, false)); + + assert!(out.contains("blake3="), "receipt must name BLAKE3: {out}"); + assert!( + !out.contains("md5="), + "no `md5=` label may survive on a BLAKE3 digest: {out}" + ); + + let digest = out + .lines() + .find(|l| l.starts_with("postimage:")) + .and_then(|l| l.split("blake3=").nth(1)) + .expect("receipt carries a postimage blake3 field"); + assert_eq!(digest.len(), 64, "BLAKE3 is 64 hex chars, got {digest:?}"); + assert!(digest.chars().all(|c| c.is_ascii_hexdigit())); + assert_eq!( + digest, + crate::core::hasher::hash_hex(b"bbb\n"), + "the postimage digest must be BLAKE3 of the bytes on disk" + ); + + let mut p = mk_params(f.path(), "bbb", "ccc", false, false); + p.expected_blake3 = Some("deadbeef".to_string()); + let err = handle(&mut cache, &p); + assert!(err.contains("expected_blake3="), "{err}"); + assert!(err.contains("actual_blake3="), "{err}"); + } + #[test] fn backup_is_created_when_enabled() { let f = make_temp("aaa\n"); diff --git a/rust/src/tools/ctx_handoff.rs b/rust/src/tools/ctx_handoff.rs index 4c8b5e19b4..f97eabdca6 100644 --- a/rust/src/tools/ctx_handoff.rs +++ b/rust/src/tools/ctx_handoff.rs @@ -8,7 +8,9 @@ pub fn format_created(path: &Path, ledger: &HandoffLedgerV1) -> String { |w| format!("{}@{}", w.spec.name, w.current), ); format!( - "ctx_handoff create\n path: {}\n md5: {}\n manifest_md5: {}\n workflow: {}\n evidence_keys: {}\n curated_refs: {}\n knowledge_facts: {}", + // #1592: both digests are BLAKE3. The on-disk ledger keeps its historical + // `*_md5` field names; what a reader sees is labelled for what it is. + "ctx_handoff create\n path: {}\n blake3: {}\n manifest_blake3: {}\n workflow: {}\n evidence_keys: {}\n curated_refs: {}\n knowledge_facts: {}", path.display(), ledger.content_md5, ledger.manifest_md5, diff --git a/rust/src/tools/ctx_patch/mod.rs b/rust/src/tools/ctx_patch/mod.rs index 4f150609c7..4e60984d0c 100644 --- a/rust/src/tools/ctx_patch/mod.rs +++ b/rust/src/tools/ctx_patch/mod.rs @@ -43,7 +43,10 @@ pub struct PatchParams { pub ops: Vec, /// Optional whole-file preimage guard (BLAKE3 hex, as printed by ctx_edit's /// `postimage:` line). When set, the edit fails if the file's hash differs. - pub expected_md5: Option, + /// + /// Named for the algorithm it actually uses (#1592); the wire parameter + /// `expected_md5` stays accepted as an alias for existing callers. + pub expected_blake3: Option, pub backup: bool, pub backup_path: Option, pub evidence: bool, @@ -117,13 +120,13 @@ pub fn run_io(params: &PatchParams, _last_mode: &str) -> (String, CacheEffect) { } }; - if let Some(expected) = params.expected_md5.as_deref() - && expected != pre.fp.md5 + if let Some(expected) = params.expected_blake3.as_deref() + && expected != pre.fp.blake3 { return ( format!( - "ERROR: preimage mismatch for {file_path}: expected_md5={expected}, actual_md5={}", - pre.fp.md5 + "ERROR: preimage mismatch for {file_path}: expected_blake3={expected}, actual_blake3={}", + pre.fp.blake3 ), CacheEffect::None, ); @@ -244,7 +247,7 @@ pub fn run_io(params: &PatchParams, _last_mode: &str) -> (String, CacheEffect) { &new_content, pre.fp.size, pre.fp.mtime_ms, - &pre.fp.md5, + &pre.fp.blake3, lines_before, new_lines.len(), n_edits, diff --git a/rust/src/tools/ctx_patch/tests.rs b/rust/src/tools/ctx_patch/tests.rs index 71873adc71..7a60ba2223 100644 --- a/rust/src/tools/ctx_patch/tests.rs +++ b/rust/src/tools/ctx_patch/tests.rs @@ -25,7 +25,7 @@ fn params(path: &std::path::Path, ops: Vec) -> PatchParams { PatchParams { path: path.to_string_lossy().to_string(), ops, - expected_md5: None, + expected_blake3: None, backup: false, backup_path: None, evidence: false, @@ -300,7 +300,7 @@ fn overlapping_batch_is_rejected() { } #[test] -fn expected_md5_guard_blocks_mismatch() { +fn expected_blake3_guard_blocks_mismatch() { let f = make_temp("aaa\n"); let mut p = params( f.path(), @@ -310,7 +310,7 @@ fn expected_md5_guard_blocks_mismatch() { new_text: "bbb".to_string(), }], ); - p.expected_md5 = Some("deadbeef".to_string()); + p.expected_blake3 = Some("deadbeef".to_string()); let (text, effect) = run_io(&p, ""); assert!(text.contains("preimage mismatch"), "{text}"); assert!(matches!(effect, CacheEffect::None)); diff --git a/rust/src/tools/ctx_read/core_logic.rs b/rust/src/tools/ctx_read/core_logic.rs index 8eec53cd82..b2b8c7a340 100644 --- a/rust/src/tools/ctx_read/core_logic.rs +++ b/rust/src/tools/ctx_read/core_logic.rs @@ -225,7 +225,13 @@ pub(super) fn handle_with_options_inner( // same capped, byte-stable body. let out = if mode_allows_raw_cap(&resolved_mode) { let framed_tokens = count_tokens(&out); - cap_to_raw(out, framed_tokens, &content, original_tokens) + cap_to_raw( + out, + framed_tokens, + &content, + original_tokens, + &resolved_mode, + ) } else { out }; @@ -323,11 +329,14 @@ pub(super) fn handle_with_options_inner( output.push_str(&format!("\n{hint}")); } let framed_tokens = count_tokens(&output); + // Verbatim `full` — the cap only strips framing, never a summary, so no + // no-compression banner is warranted here. let output = cap_to_raw( output, framed_tokens, &content, store_result.original_tokens, + "full", ); let output = crate::core::redaction::redact_text_if_enabled(&output); let sent = count_tokens(&output); @@ -377,6 +386,7 @@ pub(super) fn handle_with_options_inner( framed_tokens, &content, store_result.original_tokens, + &resolved_mode, ) } else { output @@ -421,15 +431,40 @@ pub(super) fn handle_with_options_inner( /// is roughly token-neutral and applied to whichever string wins), so the /// comparison is apples-to-apples with `original_tokens`. Empty files /// (`raw_tokens == 0`) keep their framing so the reader still gets a signal. +/// #361 anti-inflation cap: when framing a whole-file view costs more tokens +/// than the file itself, return the bare file instead. +/// +/// `requested_mode` names the view the caller asked for. If that view was a +/// *compressed* one, the bare file is prefixed with a one-line banner: the +/// caller ordered a summary and is getting the whole file, and without the +/// banner that failure is invisible — indistinguishable from a summary that +/// happened to need every line, at full-file cost. Verbatim views (`full`, +/// `anchored`, …) are returned byte-identical, so callers that demand exact +/// bytes (compress-protected paths, #1150) are never given a prefix. pub(crate) fn cap_to_raw( framed: String, framed_tokens: usize, raw_content: &str, raw_tokens: usize, + requested_mode: &str, ) -> String { if raw_tokens > 0 && framed_tokens > raw_tokens { let prevented = (framed_tokens - raw_tokens) as u64; crate::core::cache_telemetry::record_raw_cap(prevented); + // #1587: a caller who asked for a *summary* and silently receives the + // whole file pays full-file tokens believing they compressed. Say so — + // but only above the banner threshold, so the cap keeps its #361 + // guarantee (a read never costs more than the raw file) on the small + // files where framing alone was the inflation. + let compressed_request = requested_mode + .parse::() + .is_ok_and(|m| m.counts_as_compressed()); + if compressed_request + && let Some(banner) = + crate::tools::ctx_read::render::no_compression_banner(requested_mode, raw_tokens) + { + return format!("{banner}\n{raw_content}"); + } raw_content.to_string() } else { framed @@ -572,7 +607,13 @@ pub(super) fn handle_full_with_auto_delta( ) } -fn handle_diff(cache: &mut SessionCache, path: &str, file_ref: &str) -> (String, usize) { +/// Render the delta between the cached baseline of `path` and its current disk +/// content, refreshing the baseline. Used by both read paths: the CLI/in-process +/// path (`handle_with_options_inner`) and the MCP tool handler — `diff` is a +/// delta view with no whole-file renderer, so it must never reach +/// `process_mode_tuned` (which would report it as an unknown mode and fall back +/// to dumping the entire file, #1584). +pub(crate) fn handle_diff(cache: &mut SessionCache, path: &str, file_ref: &str) -> (String, usize) { let _mode_guard = crate::core::savings_footer::ModeGuard::new("diff"); let short = protocol::shorten_path(path); let old_content = cache diff --git a/rust/src/tools/ctx_read/render.rs b/rust/src/tools/ctx_read/render.rs index 5b503c2a9f..b7dd14ccb4 100644 --- a/rust/src/tools/ctx_read/render.rs +++ b/rust/src/tools/ctx_read/render.rs @@ -12,6 +12,27 @@ fn monotonic_check(original: usize, compressed: usize) -> bool { compressed < original } +/// Below this many raw tokens a silent full-content fallback is not worth a +/// banner: the file is small enough that the framing itself was the expensive +/// part (which is exactly what the #361 cap exists to strip), and a banner +/// would push the read back above the raw file it just protected. Above it, the +/// caller is being handed a whole file they did not order and must be told. +pub(crate) const NO_COMPRESSION_BANNER_MIN_TOKENS: usize = 400; + +/// One-line notice that a compression path gave up and returned the untouched +/// file. Without it the caller pays full-file tokens believing a summary was +/// delivered — the failure is invisible in the output and surfaces only on the +/// bill. `None` for files below [`NO_COMPRESSION_BANNER_MIN_TOKENS`], where the +/// fallback is the cap working as designed rather than a degradation. +pub(crate) fn no_compression_banner(requested_mode: &str, raw_tokens: usize) -> Option { + (raw_tokens >= NO_COMPRESSION_BANNER_MIN_TOKENS).then(|| { + format!( + "[lean-ctx] no compression applied (mode={requested_mode}): \ + output was not smaller than the file — returning full content ({raw_tokens} tok)" + ) + }) +} + fn raw_fallback( path: &str, content: &str, @@ -21,7 +42,15 @@ fn raw_fallback( tracing::debug!( "monotonic guard: {path} compressed {compressed} >= original {original_tokens}, using raw" ); - (content.to_string(), original_tokens) + let mode = crate::core::savings_footer::current_mode().unwrap_or_else(|| "compressed".into()); + match no_compression_banner(&mode, original_tokens) { + Some(banner) => { + let out = format!("{banner}\n{content}"); + let sent = count_tokens(&out); + (out, sent) + } + None => (content.to_string(), original_tokens), + } } /// Per-read tuning threaded into the per-mode renderers. `Default` reproduces @@ -394,10 +423,25 @@ pub(crate) fn process_mode_tuned( sent, ) } + // `diff` is a delta view rendered against the session cache by + // `core_logic::handle_diff`, not a whole-file render — there is no + // content-only way to produce it. Reaching the generic renderer means a + // caller bypassed the cache-aware path; answer in one bounded line + // instead of falling through to `unknown` and dumping the whole file + // under a warning the caller pays full tokens for (#1584). + "diff" => { + let msg = format!( + "{short}: mode=diff renders against the session cache and cannot be produced here. \ + Read the file once with mode=full, then request mode=diff on the re-read." + ); + let sent = count_tokens(&msg); + (msg, sent) + } unknown => { let header = build_header(file_ref, short, ext, content, line_count, true); let out = format!( - "[WARNING: unknown mode '{unknown}', falling back to full]\n{header}\n{content}" + "[WARNING: unknown mode '{unknown}', falling back to full — valid modes: {}]\n{header}\n{content}", + crate::core::mcp_manifest::READ_MODES.join(", ") ); let sent = count_tokens(&out); (out, sent) @@ -589,6 +633,84 @@ mod render_tests { " [density target=0.40]" ); } + + /// #1584: `diff` reaching the whole-file dispatcher used to be reported as + /// an *unknown* mode and answered with the entire file under a warning — + /// the caller paid full tokens for a delta they never got. It now answers + /// in one bounded line that names the way to get a real delta. + #[test] + fn diff_mode_never_falls_through_to_the_unknown_mode_dump() { + let content = "fn a() {}\nfn b() {}\nfn c() {}\n"; + let (out, _) = super::process_mode_tuned( + content, + "diff", + "", + "sample.rs", + "rs", + 32, + crate::tools::CrpMode::Off, + "sample.rs", + None, + super::ReadTuning { + aggressiveness: None, + protect: &[], + }, + ); + assert!( + !out.contains("unknown mode"), + "diff is a documented mode, not an unknown one: {out}" + ); + assert!( + !out.contains("fn b()"), + "the fallback must not dump the file the caller did not ask for: {out}" + ); + assert!( + out.contains("mode=full"), + "it must name the way forward: {out}" + ); + } + + /// #1584: when a mode really is unknown, the warning names what is valid + /// instead of leaving the caller to guess — the schema list and the runtime + /// now come from one constant, which is what let `diff` drift apart. + #[test] + fn unknown_mode_warning_names_the_valid_modes() { + let (out, _) = super::process_mode_tuned( + "fn a() {}\n", + "definitely-not-a-mode", + "", + "sample.rs", + "rs", + 8, + crate::tools::CrpMode::Off, + "sample.rs", + None, + super::ReadTuning { + aggressiveness: None, + protect: &[], + }, + ); + assert!( + out.contains("unknown mode 'definitely-not-a-mode'"), + "{out}" + ); + for mode in crate::core::mcp_manifest::READ_MODES { + assert!(out.contains(mode), "warning must list `{mode}`: {out}"); + } + } + + /// #1587: a compression request that degrades to the whole file says so. + /// Below the threshold it stays silent, so the #361 cap still guarantees a + /// read never costs more than the raw file. + #[test] + fn no_compression_banner_only_above_threshold() { + assert!(super::no_compression_banner("signatures", 10).is_none()); + let banner = + super::no_compression_banner("signatures", super::NO_COMPRESSION_BANNER_MIN_TOKENS) + .expect("a whole file handed back instead of a summary must be announced"); + assert!(banner.contains("no compression applied"), "{banner}"); + assert!(banner.contains("mode=signatures"), "{banner}"); + } } /// Shared, `Copy` bundle of the per-call rendering context threaded to the @@ -1166,6 +1288,30 @@ fn render_entropy(content: &str, ctx: RenderCtx<'_>, tuning: &ReadTuning<'_>) -> ) } +/// Renders a task-mode selection as numbered lines with explicit gap markers. +/// +/// Task mode returns *fragments*. Delivered bare they read like a small, +/// complete file: nothing says where a fragment sits or that anything was +/// dropped between two adjacent lines, so the view gives a false picture of the +/// source (#1589). `N|` matches the numbering `lines:N-M` and `anchored` use, +/// so the numbers double as coordinates for the follow-up read or patch. +fn render_task_selection(keywords: &[String], selected: &[(usize, &str)]) -> String { + let mut out = format!("[task: {}]", keywords.join(", ")); + let mut prev_line: Option = None; + for (idx, line) in selected { + let lineno = idx + 1; + if let Some(prev) = prev_line + && lineno > prev + 1 + { + let skipped = lineno - prev - 1; + out.push_str(&format!("\n… {skipped}L")); + } + out.push_str(&format!("\n{lineno}|{line}")); + prev_line = Some(lineno); + } + out +} + fn render_task_mode(content: &str, ctx: RenderCtx<'_>, tuning: &ReadTuning<'_>) -> (String, usize) { let RenderCtx { file_ref, @@ -1211,13 +1357,26 @@ fn render_task_mode(content: &str, ctx: RenderCtx<'_>, tuning: &ReadTuning<'_>) AggressivenessProfile::from_level(a).ib_budget_ratio }); let is_markdown = matches!(ext, "md" | "markdown" | "mdx" | "rst"); - let filtered = crate::core::task_relevance::information_bottleneck_filter_with_headers( - content, - &keywords, - ib_budget, - tuning.protect, - is_markdown, - ); + let filtered = if is_markdown { + crate::core::task_relevance::information_bottleneck_filter_with_headers( + content, + &keywords, + ib_budget, + tuning.protect, + true, + ) + } else { + // #1589: code fragments need coordinates. Prose does not — markdown + // keeps its section-header reconstruction instead. + let selected = crate::core::task_relevance::ib_select( + content, + &keywords, + ib_budget, + None, + tuning.protect, + ); + render_task_selection(&keywords, &selected) + }; let filtered_lines = filtered.lines().count(); let header = if crate::core::protocol::meta_visible() && !file_ref.is_empty() { format!("{file_ref}={short} {line_count}L [task-filtered: {line_count}→{filtered_lines}]") diff --git a/rust/src/tools/ctx_read/tests_inflation.rs b/rust/src/tools/ctx_read/tests_inflation.rs index 4a8504f73d..ef50023073 100644 --- a/rust/src/tools/ctx_read/tests_inflation.rs +++ b/rust/src/tools/ctx_read/tests_inflation.rs @@ -20,9 +20,9 @@ fn cap_to_raw_falls_back_when_framing_inflates() { "fixture must inflate to exercise the guard" ); assert_eq!( - cap_to_raw(framed, framed_tokens, raw, raw_tokens), + cap_to_raw(framed, framed_tokens, raw, raw_tokens, "signatures"), raw, - "framing larger than raw must fall back to bare content" + "framing larger than raw must fall back to bare content — and on a file\n this small the notice is suppressed so the #361 cap still holds" ); } @@ -32,7 +32,7 @@ fn cap_to_raw_keeps_framing_when_not_larger() { let framed = "sig summary".to_string(); let framed_tokens = count_tokens(&framed); assert_eq!( - cap_to_raw(framed.clone(), framed_tokens, raw, 100), + cap_to_raw(framed.clone(), framed_tokens, raw, 100, "signatures"), framed, "output at or below raw must be returned untouched" ); @@ -45,12 +45,72 @@ fn cap_to_raw_keeps_framing_for_empty_file() { let framed = "F1=empty.rs 0L".to_string(); let framed_tokens = count_tokens(&framed); assert_eq!( - cap_to_raw(framed.clone(), framed_tokens, "", 0), + cap_to_raw(framed.clone(), framed_tokens, "", 0, "signatures"), framed, "empty files keep their framing signal" ); } +// --------------------------------------------------------------------------- +// #1587: the cap must not degrade *silently*. When a compressed view collapses +// to full content on a file large enough for that to cost real tokens, the +// caller is told; a verbatim `full` request is never labelled (nothing was +// withheld), and small files stay bare so #361 above is not undone. +// --------------------------------------------------------------------------- + +/// Raw body comfortably above `NO_COMPRESSION_BANNER_MIN_TOKENS`. +fn big_raw() -> String { + "pub fn item(value: u32) -> u32 { value.saturating_add(1) }\n".repeat(120) +} + +#[test] +fn cap_to_raw_labels_silent_fallback_for_compressed_request() { + let raw = big_raw(); + let raw_tokens = count_tokens(&raw); + assert!( + raw_tokens >= crate::tools::ctx_read::render::NO_COMPRESSION_BANNER_MIN_TOKENS, + "fixture must exceed the banner threshold" + ); + let framed = format!("F1=big.rs 120L\n{raw}"); + let framed_tokens = count_tokens(&framed); + let out = cap_to_raw(framed, framed_tokens, &raw, raw_tokens, "signatures"); + assert!( + out.starts_with("[lean-ctx] no compression applied (mode=signatures)"), + "a summary request answered with the whole file must say so: {}", + out.lines().next().unwrap_or_default() + ); + assert!( + out.ends_with(&raw), + "the full content still follows the notice" + ); +} + +#[test] +fn cap_to_raw_stays_silent_for_verbatim_full_request() { + let raw = big_raw(); + let raw_tokens = count_tokens(&raw); + let framed = format!("F1=big.rs 120L\n{raw}"); + let framed_tokens = count_tokens(&framed); + assert_eq!( + cap_to_raw(framed, framed_tokens, &raw, raw_tokens, "full"), + raw, + "`full` asked for the whole file — stripping framing is not a degradation" + ); +} + +#[test] +fn cap_to_raw_notice_never_breaks_the_361_cap() { + // Small file: the notice would push the read above the raw cost it just + // protected, so it is suppressed. + let raw = "pub fn a() {}\n"; + let raw_tokens = count_tokens(raw); + let framed = format!("F1=x.rs 1L\n deps foo,bar\n{raw}"); + let framed_tokens = count_tokens(&framed); + let out = cap_to_raw(framed, framed_tokens, raw, raw_tokens, "map"); + assert_eq!(out, raw); + assert!(count_tokens(&out) <= raw_tokens); +} + #[test] fn auto_read_never_inflates_small_file() { let dir = tempfile::tempdir().unwrap(); diff --git a/rust/src/tools/ctx_refactor/mod.rs b/rust/src/tools/ctx_refactor/mod.rs index 904a825265..b3e4d9cd03 100644 --- a/rust/src/tools/ctx_refactor/mod.rs +++ b/rust/src/tools/ctx_refactor/mod.rs @@ -335,7 +335,10 @@ pub(crate) fn resolve_name_path_scoped( match leaves.len() { 0 => Err(format!( - "NO_SYMBOL: '{name_path}' did not resolve to any indexed symbol" + "NO_SYMBOL: '{name_path}' did not resolve to any indexed symbol. \ + A file created since the index was built is not in it yet — run \ + ctx_search(action=reindex), pass 'path' so the file can be parsed \ + directly, or target the span with 'path'+'line'." )), 1 => Ok(Resolved { rel_path: leaves[0].file.clone(), @@ -358,6 +361,55 @@ pub(crate) fn resolve_name_path_scoped( } } +/// Resolve a `name_path` by parsing the file the caller named, instead of +/// consulting the symbol index (#1593). +/// +/// The index is a repo scan; a file created during the session is not in it, so +/// `replace_symbol` on a just-written file failed with `NO_SYMBOL` even though +/// `ctx_read(mode=signatures)` had listed the symbol seconds earlier. When the +/// caller names the file, the file itself is authority enough — same tree-sitter +/// extractor the signature view uses, so the two can no longer disagree. +pub(crate) fn resolve_name_path_in_file( + name_path: &str, + project_root: &str, + path: &str, +) -> Result { + let abs = crate::core::path_resolve::resolve_tool_path(Some(project_root), None, path) + .map_err(|e| format!("NO_SYMBOL: path blocked by jail: {e}"))?; + let content = std::fs::read_to_string(&abs) + .map_err(|e| format!("NO_SYMBOL: cannot read '{path}': {e}"))?; + let ext = std::path::Path::new(&abs) + .extension() + .and_then(|e| e.to_str()) + .unwrap_or(""); + let leaf = name_path + .split(['/', ':']) + .rfind(|s| !s.is_empty()) + .ok_or_else(|| "NO_SYMBOL: empty name_path".to_string())?; + + let mut hits = crate::core::signatures::extract_signatures(&content, ext) + .into_iter() + .filter(|s| s.name == leaf) + .filter_map(|s| Some((s.start_line?, s.end_line?))); + let (start_line, end_line) = hits.next().ok_or_else(|| { + format!( + "NO_SYMBOL: '{name_path}' not found in '{path}' \ + (parsed the file directly; the symbol index did not have it either)" + ) + })?; + if hits.next().is_some() { + return Err(format!( + "AMBIGUOUS_SYMBOL: '{leaf}' appears more than once in '{path}'; \ + target the one you mean with 'line'" + )); + } + Ok(Resolved { + rel_path: path.to_string(), + start_line, + end_line, + }) +} + /// Read the current on-disk text covered by a usage's range, jail-checking its /// path first. Out-of-jail / unreadable / bad range → `Err` (spec §5.4 Multi-File /// jail: every plugin-reported path is re-checked against `project_root`). @@ -464,7 +516,15 @@ fn handle_symbol_edit(action: &str, args: &Value, project_root: &str) -> String let (rel_path, start_line, end_line) = if let Some(np) = args.get("name_path").and_then(Value::as_str) { - match resolve_name_path(np, project_root) { + // #1593: the index is a repo scan and misses files created this session. + // When the caller named the file, fall back to parsing it directly. + let resolved = match (resolve_name_path(np, project_root), explicit_path) { + (Err(e), Some(p)) if e.starts_with("NO_SYMBOL") => { + resolve_name_path_in_file(np, project_root, p) + } + (other, _) => other, + }; + match resolved { Ok(r) => { // #803 WORKTREE SAFETY: when the caller also provided `path`, // verify the index-resolved file matches the caller's target. diff --git a/rust/src/tools/ctx_refactor/tests.rs b/rust/src/tools/ctx_refactor/tests.rs index 3dcbc661e5..ae6fc5c798 100644 --- a/rust/src/tools/ctx_refactor/tests.rs +++ b/rust/src/tools/ctx_refactor/tests.rs @@ -254,10 +254,38 @@ fn resolve_name_path_unknown_is_no_symbol() { let err = super::resolve_name_path("ZzzNoSuchSymbol123", &root).unwrap_err(); assert!(err.starts_with("NO_SYMBOL"), "got: {err}"); + assert!( + err.contains("reindex"), + "NO_SYMBOL must name a way forward: {err}" + ); crate::test_env::remove_var("LEAN_CTX_DATA_DIR"); } +/// #1593: a file written after the index was built still resolves, because the +/// caller named it and the file is parsed directly. +#[test] +fn resolve_name_path_in_file_finds_unindexed_symbol() { + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(proj.join("src")).unwrap(); + std::fs::write( + proj.join("src/fresh.rs"), + "pub fn brand_new_fn(a: u8) -> u8 {\n a + 1\n}\n", + ) + .unwrap(); + let root = proj.to_string_lossy().to_string(); + + let r = super::resolve_name_path_in_file("brand_new_fn", &root, "src/fresh.rs") + .expect("file parse resolves what the index has never seen"); + assert_eq!(r.rel_path, "src/fresh.rs"); + assert_eq!(r.start_line, 1); + assert!(r.end_line >= 3, "span must cover the body: {r:?}"); + + let err = super::resolve_name_path_in_file("no_such_thing", &root, "src/fresh.rs").unwrap_err(); + assert!(err.starts_with("NO_SYMBOL"), "got: {err}"); +} + #[test] #[ignore = "requires pre-built symbol index; flaky under parallel cargo test"] fn resolve_name_path_trait_impl_method() { diff --git a/rust/src/tools/ctx_search/implementation.rs b/rust/src/tools/ctx_search/implementation.rs index a75a250ebe..135d268666 100644 --- a/rust/src/tools/ctx_search/implementation.rs +++ b/rust/src/tools/ctx_search/implementation.rs @@ -447,7 +447,14 @@ pub fn handle_filtered( .collect() }; - let mut result = format!("{} matches in {} files", matches.len(), files_searched); + // #1591: "N matches in M files" read as "M files matched" — but `M` was the + // number of files *searched*. Report what matched; keep the scanned count as + // a clearly-labelled secondary number so scope is still visible. + let mut result = format!( + "{} matches in {} files (scanned {files_searched})", + matches.len(), + matched_files.len() + ); if matched_files.len() > 1 { if matched_files.len() <= 10 { result.push_str(" ["); diff --git a/rust/src/tools/ctx_search/tests.rs b/rust/src/tools/ctx_search/tests.rs index 4ec1bd445d..f6c840736d 100644 --- a/rust/src/tools/ctx_search/tests.rs +++ b/rust/src/tools/ctx_search/tests.rs @@ -633,3 +633,40 @@ fn search_refuses_home_directory_root() { "home root must be refused: {out}" ); } + +/// #1591: "13 matches in 40 files" reported the number of files *scanned* as +/// the number that matched. The two counts are now separate and labelled. +#[test] +fn match_count_reports_matched_files_not_scanned_files() { + let dir = tempfile::tempdir().unwrap(); + for i in 0..8 { + std::fs::write(dir.path().join(format!("quiet{i}.rs")), "fn nothing() {}\n").unwrap(); + } + std::fs::write( + dir.path().join("hit_a.rs"), + "let needle = 1;\nlet needle = 2;\n", + ) + .unwrap(); + std::fs::write(dir.path().join("hit_b.rs"), "let needle = 3;\n").unwrap(); + + let out = handle( + "needle", + &dir.path().to_string_lossy(), + Some("*.rs"), + 50, + CrpMode::Off, + true, + true, + false, + ) + .text; + + assert!( + out.starts_with("3 matches in 2 files"), + "the headline count must be files that matched, not files walked: {out}" + ); + assert!( + out.contains("scanned 10"), + "the scanned count stays visible, explicitly labelled: {out}" + ); +} diff --git a/rust/src/tools/edit_io.rs b/rust/src/tools/edit_io.rs index b0d01fc00f..564291b11f 100644 --- a/rust/src/tools/edit_io.rs +++ b/rust/src/tools/edit_io.rs @@ -24,11 +24,15 @@ use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; /// Size + mtime + content hash of a file, used as a TOCTOU fingerprint. +/// +/// The digest is BLAKE3 (64 hex chars), not MD5 — the field and every receipt +/// that prints it say so, after a user reported chasing a `md5=` label that +/// matched neither md5 nor sha256 (#1592). #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct FileFingerprint { pub(crate) size: u64, pub(crate) mtime_ms: u64, - pub(crate) md5: String, + pub(crate) blake3: String, } /// A file read for editing: its fingerprint, permissions, raw bytes, decoded @@ -125,7 +129,7 @@ pub(crate) fn fingerprint_from_bytes(bytes: &[u8], meta: &std::fs::Metadata) -> FileFingerprint { size: bytes.len() as u64, mtime_ms: meta.modified().map_or(0, system_time_to_millis), - md5: crate::core::hasher::hash_hex(bytes), + blake3: crate::core::hasher::hash_hex(bytes), } } @@ -184,14 +188,14 @@ pub(crate) fn ensure_preimage_still_matches( let now = fingerprint_from_bytes(&bytes, &meta); if &now != expected { return Err(format!( - "ERROR: file changed since read (TOCTOU guard). Re-read and retry: {}\nexpected: size={}, mtime_ms={}, md5={}\nactual: size={}, mtime_ms={}, md5={}", + "ERROR: file changed since read (TOCTOU guard). Re-read and retry: {}\nexpected: size={}, mtime_ms={}, blake3={}\nactual: size={}, mtime_ms={}, blake3={}", path.display(), expected.size, expected.mtime_ms, - expected.md5, + expected.blake3, now.size, now.mtime_ms, - now.md5 + now.blake3 )); } Ok(()) @@ -274,6 +278,6 @@ mod tests { let a = read_preimage(&path, cap, false).unwrap().fp; let b = read_preimage(&path, cap, false).unwrap().fp; assert_eq!(a, b, "same bytes → same fingerprint"); - assert_eq!(a.md5, crate::core::hasher::hash_hex(b"hello\n")); + assert_eq!(a.blake3, crate::core::hasher::hash_hex(b"hello\n")); } } diff --git a/rust/src/tools/registered/ctx_edit.rs b/rust/src/tools/registered/ctx_edit.rs index f7f55eb1b2..292aebf8a5 100644 --- a/rust/src/tools/registered/ctx_edit.rs +++ b/rust/src/tools/registered/ctx_edit.rs @@ -51,7 +51,10 @@ impl McpTool for CtxEditTool { .ok_or_else(|| ErrorData::invalid_params("new_string is required", None))?; let replace_all = get_bool(args, "replace_all").unwrap_or(false); let create = get_bool(args, "create").unwrap_or(false); - let expected_md5 = get_str(args, "expected_md5"); + // #1592: the guard is a BLAKE3 digest; `expected_md5` was a misnomer and + // stays accepted so existing callers keep working. + let expected_blake3 = + get_str(args, "expected_blake3").or_else(|| get_str(args, "expected_md5")); let expected_size = get_int(args, "expected_size").and_then(|v| u64::try_from(v).ok()); let expected_mtime_ms = get_int(args, "expected_mtime_ms").and_then(|v| u64::try_from(v).ok()); @@ -70,7 +73,7 @@ impl McpTool for CtxEditTool { new_string, replace_all, create, - expected_md5, + expected_blake3, expected_size, expected_mtime_ms, backup, diff --git a/rust/src/tools/registered/ctx_patch.rs b/rust/src/tools/registered/ctx_patch.rs index aa70a999ce..24decaee7b 100644 --- a/rust/src/tools/registered/ctx_patch.rs +++ b/rust/src/tools/registered/ctx_patch.rs @@ -16,7 +16,8 @@ impl McpTool for CtxPatchTool { // Schema diet (#576 pattern): the advertised surface carries only the // functional teaching (anchor source, op routing, batch atomicity). - // Handler-only params stay supported but unadvertised: expected_md5, + // Handler-only params stay supported but unadvertised: expected_blake3 (alias + // expected_md5), // backup, backup_path, validate_syntax, evidence, diff_max_lines, // allow_lossy_utf8 — same hidden-params contract as ctx_edit. fn tool_def(&self) -> Tool { @@ -127,7 +128,10 @@ fn handle_anchored(args: &Map, ctx: &ToolContext) -> Result, ctx: &ToolContext) -> Result= deadline { @@ -182,7 +161,22 @@ impl CtxReadTool { std::thread::sleep(std::time::Duration::from_millis(25)); } }; - let task_ref = current_task.as_deref(); + // #1590: `auto_infer_task` fabricates descriptions like + // "Working on /repo/src/printer (explore)" from touched-file patterns. + // That is fine as a telemetry hint, but its "keywords" (Working, + // explore, a directory path) carry no signal about a file's contents — + // filtering a file down to a quarter of its lines on that basis, or + // pinning it to `full` as an intent target, invents a task the caller + // never stated. Only an explicitly set task steers a read. + let task_is_inferred = current_task + .as_ref() + .is_some_and(|(_, intent)| !task_intent_steers_read(intent.as_deref())); + let current_task = current_task.map(|(description, _)| description); + let task_ref = if task_is_inferred { + None + } else { + current_task.as_deref() + }; // #513: `raw=true` is the intuitive "give me the exact bytes" escape an // agent reaches for. Alias it to mode="raw" (verbatim, unframed) and // force a fresh disk read below so a re-read never collapses to an @@ -277,6 +271,7 @@ impl CtxReadTool { Some(&ctx.project_root), pressure_action, resolved_agent_id.as_deref(), + fresh, ); let mut engine_policy_admission = engine::admission_or_reject( engine_interface_v1, @@ -294,8 +289,15 @@ impl CtxReadTool { { if explicit_mode { let reason = gate_result.reason.unwrap_or("context-gate"); + // #1588: an override the caller cannot escape is a dead end. + // Name the escape hatch for the two heuristics that honour it. + let escape = if matches!(reason, "bounce-prevention" | "intent-target") { + ", pass fresh=true to keep the requested mode" + } else { + "" + }; mode_override_note = Some(format!( - "[mode overridden: {mode} -> {overridden}, reason={reason}]" + "[mode overridden: {mode} -> {overridden}, reason={reason}{escape}]" )); } mode = overridden; @@ -589,7 +591,12 @@ impl CtxReadTool { // // New: prepare (brief lock) → compute (no lock) → store (brief lock). - let task_ref = task_owned.as_deref(); + // #1590: an inferred task never steers a lossy view. + let task_ref = if task_is_inferred { + None + } else { + task_owned.as_deref() + }; let tuning = crate::tools::ctx_read::ReadTuning::resolve(aggressiveness, &protect_owned); @@ -688,13 +695,21 @@ impl CtxReadTool { } if !effective_fresh { - let stale = cache.get(&path_owned).is_some_and(|e| { - crate::core::cache::is_cache_entry_stale_verified( - &path_owned, - e.stored_mtime, - &e.hash, - ) - }); + // #1584: `diff` is the one mode for which a stale + // entry is the *point* — it holds the baseline the + // delta is computed against. Dropping it here left + // `handle_diff` with nothing to compare and made + // every post-edit diff answer "no cached version". + // Every other mode still refuses to serve content + // that no longer matches the file on disk. + let stale = mode_eff != "diff" + && cache.get(&path_owned).is_some_and(|e| { + crate::core::cache::is_cache_entry_stale_verified( + &path_owned, + e.stored_mtime, + &e.hash, + ) + }); if stale { cache.invalidate(&path_owned); reuse_outcome = ReuseOutcome::Stale; @@ -705,7 +720,41 @@ impl CtxReadTool { .get(&path_owned) .map(|e| (e.original_tokens, e.content())); - if let Some((orig_tok, content_opt)) = snap { + // #1584: `diff` is a delta view, not a whole-file + // render. It needs the cached baseline and has no arm in + // `process_mode_tuned`, so letting it fall through to the + // generic compute path made the MCP handler answer + // "[WARNING: unknown mode 'diff']" and return the entire + // file — while the CLI path handled it correctly. Both + // paths now share `handle_diff`. + if mode_eff == "diff" { + let (out, _sent) = if fresh { + let warning = "[warning] fresh+diff is redundant — fresh invalidates the cache, so no baseline is left to diff against. Use mode=full with fresh=true instead."; + ( + warning.to_string(), + crate::core::tokens::count_tokens(warning), + ) + } else { + crate::tools::ctx_read::handle_diff( + &mut cache, + &path_owned, + &file_ref, + ) + }; + let out = crate::core::redaction::redact_text_if_enabled(&out); + let orig = cache.get(&path_owned).map_or(0, |e| e.original_tokens); + let fref = cache.file_ref_map().get(path_owned.as_str()).cloned(); + let s = cache.get_stats(); + PrepareOutcome::Hit( + out, + "diff".to_string(), + orig, + false, + fref, + (s.total_reads(), s.cache_hits()), + reuse_outcome, + ) + } else if let Some((orig_tok, content_opt)) = snap { let resolved = if mode_eff == "auto" { tuning.auto_density_mode().unwrap_or_else(|| { crate::tools::ctx_read::resolve_auto_mode( @@ -914,6 +963,7 @@ impl CtxReadTool { ft, &compute_content, original_tokens, + "full", ); (out, "full".to_string()) } @@ -937,6 +987,7 @@ impl CtxReadTool { ft, &compute_content, original_tokens, + &resolved_mode, ) } else { out @@ -1410,6 +1461,16 @@ mod engine; #[path = "ctx_read_image.rs"] mod image; use image::read_image_file; +// #660 LOC gate: leaf helpers live in their own module. +#[path = "ctx_read_helpers.rs"] +mod helpers; +pub(crate) use helpers::task_intent_steers_read; +// `apply_verdict`'s only caller in this module is the inline test file, which +// reaches it as `super::apply_verdict` — hence the cfg(test) import. +#[cfg(test)] +use helpers::apply_verdict; +use helpers::{auto_degrade_read_mode, extract_file_summary, record_attribution_result}; + #[path = "ctx_read_window.rs"] mod window; #[allow(unused_imports)] @@ -1419,70 +1480,6 @@ use window::{ resolve_line_window, resolve_raw_alias, scoped_read_ranges, }; -fn apply_verdict( - mode: &str, - verdict: crate::core::degradation_policy::DegradationVerdictV1, -) -> (String, bool) { - use crate::core::degradation_policy::DegradationVerdictV1; - match verdict { - DegradationVerdictV1::Ok => (mode.to_string(), false), - DegradationVerdictV1::Warn => match mode { - "full" => ("map".to_string(), true), - other => (other.to_string(), false), - }, - DegradationVerdictV1::Throttle => match mode { - "full" | "map" => ("signatures".to_string(), true), - other => (other.to_string(), false), - }, - DegradationVerdictV1::Block => { - if mode == "signatures" { - ("signatures".to_string(), false) - } else { - ("signatures".to_string(), true) - } - } - } -} - -fn auto_degrade_read_mode(mode: &str) -> (String, Option) { - if crate::core::config::Config::load().no_degrade_effective() { - return (mode.to_string(), None); - } - let profile = crate::core::profiles::active_profile(); - if !profile.degradation.enforce_effective() { - return (mode.to_string(), None); - } - let policy = crate::core::degradation_policy::evaluate_v1_for_tool("ctx_read", None); - let (new_mode, degraded) = apply_verdict(mode, policy.decision.verdict); - let warning = if degraded { - Some(format!( - "⚠ Context pressure: mode={mode} was downgraded to mode={new_mode} \ - (verdict: {:?}). Use start_line=1 to bypass, or run ctx_compress to free budget.", - policy.decision.verdict - )) - } else { - None - }; - (new_mode, warning) -} - -fn extract_file_summary(output: &str, path: &str) -> String { - let hint = crate::core::auto_findings::extract_content_hint(output); - if !hint.is_empty() { - return hint; - } - let ext = std::path::Path::new(path) - .extension() - .and_then(|e| e.to_str()) - .unwrap_or(""); - let line_count = output.lines().count(); - if line_count > 5 { - format!("{ext} file, {line_count} lines") - } else { - String::new() - } -} - // #660 LOC gate: inline tests split out to keep this file under the line cap. #[cfg(test)] #[path = "ctx_read_inline_tests.rs"] diff --git a/rust/src/tools/registered/ctx_read_helpers.rs b/rust/src/tools/registered/ctx_read_helpers.rs new file mode 100644 index 0000000000..dd0dc955f5 --- /dev/null +++ b/rust/src/tools/registered/ctx_read_helpers.rs @@ -0,0 +1,108 @@ +//! Mode-degradation and attribution helpers for `ctx_read`. +//! +//! Split out of `ctx_read.rs` to keep that file under the #660 LOC gate. These +//! are leaf helpers with no access to the read pipeline's local state. + +use std::sync::atomic::Ordering; + +use crate::server::tool_trait::{ToolContext, ToolOutput}; + +pub(super) fn record_attribution_result(ctx: &ToolContext, source: String, output: &ToolOutput) { + let session_id = crate::core::task_spine::TaskSpine::task_id() + .or_else(|| { + ctx.session + .as_ref() + .and_then(|session| session.try_read().ok().map(|state| state.id.clone())) + }) + .unwrap_or_else(|| "mcp-session".to_string()); + let turn_provided = ctx + .call_count + .as_ref() + .map_or(0, |count| count.load(Ordering::Relaxed) as u64); + let token_cost = crate::core::tokens::count_tokens(&output.text); + let chunk = crate::core::causal_attribution::ContextChunkRecord::new( + &output.text, + source, + token_cost, + turn_provided, + ); + if let Err(error) = crate::core::causal_attribution::record_chunk(&session_id, chunk) { + tracing::debug!(%error, "causal attribution ctx_read recording failed"); + } +} + +pub(super) fn apply_verdict( + mode: &str, + verdict: crate::core::degradation_policy::DegradationVerdictV1, +) -> (String, bool) { + use crate::core::degradation_policy::DegradationVerdictV1; + match verdict { + DegradationVerdictV1::Ok => (mode.to_string(), false), + DegradationVerdictV1::Warn => match mode { + "full" => ("map".to_string(), true), + other => (other.to_string(), false), + }, + DegradationVerdictV1::Throttle => match mode { + "full" | "map" => ("signatures".to_string(), true), + other => (other.to_string(), false), + }, + DegradationVerdictV1::Block => { + if mode == "signatures" { + ("signatures".to_string(), false) + } else { + ("signatures".to_string(), true) + } + } + } +} + +pub(super) fn auto_degrade_read_mode(mode: &str) -> (String, Option) { + if crate::core::config::Config::load().no_degrade_effective() { + return (mode.to_string(), None); + } + let profile = crate::core::profiles::active_profile(); + if !profile.degradation.enforce_effective() { + return (mode.to_string(), None); + } + let policy = crate::core::degradation_policy::evaluate_v1_for_tool("ctx_read", None); + let (new_mode, degraded) = apply_verdict(mode, policy.decision.verdict); + let warning = if degraded { + Some(format!( + "⚠ Context pressure: mode={mode} was downgraded to mode={new_mode} \ + (verdict: {:?}). Use start_line=1 to bypass, or run ctx_compress to free budget.", + policy.decision.verdict + )) + } else { + None + }; + (new_mode, warning) +} + +pub(super) fn extract_file_summary(output: &str, path: &str) -> String { + let hint = crate::core::auto_findings::extract_content_hint(output); + if !hint.is_empty() { + return hint; + } + let ext = std::path::Path::new(path) + .extension() + .and_then(|e| e.to_str()) + .unwrap_or(""); + let line_count = output.lines().count(); + if line_count > 5 { + format!("{ext} file, {line_count} lines") + } else { + String::new() + } +} + +/// May a session task with this `intent` steer how a file is read? +/// +/// Only a task the caller actually stated may. `auto_infer_task` marks the +/// descriptions it fabricates from touched-file patterns — "Working on +/// /repo/src/printer (explore)" — with `intent = "inferred"`. Those words are a +/// telemetry label, not a statement about any file's contents, and letting them +/// drive the information-bottleneck filter or the intent-target override +/// silently answered a question the caller never asked (#1590). +pub(crate) fn task_intent_steers_read(intent: Option<&str>) -> bool { + intent != Some("inferred") +} diff --git a/rust/src/tools/registered/ctx_read_inline_tests.rs b/rust/src/tools/registered/ctx_read_inline_tests.rs index 2e1a015420..b10fac424e 100644 --- a/rust/src/tools/registered/ctx_read_inline_tests.rs +++ b/rust/src/tools/registered/ctx_read_inline_tests.rs @@ -1310,3 +1310,41 @@ async fn mcp_ctx_read_serves_relay_content_from_another_agent() { "relay must NOT mark full_content_delivered in session cache" ); } + +/// #1590: only a task the caller stated may steer a read. `auto_infer_task` +/// fabricates "Working on /repo/src/printer (explore)" from touched-file +/// patterns and tags it `intent="inferred"`; its keywords ("Working", +/// "explore", a directory) say nothing about any file's contents, so letting +/// them drive the IB filter or the intent-target override answered a question +/// nobody asked. +#[test] +fn inferred_session_task_does_not_steer_a_read() { + assert!( + !task_intent_steers_read(Some("inferred")), + "a fabricated task must not filter or pin a file" + ); + + // Every intent that reflects a real signal still steers. + for stated in ["explicit", "plan", "git", "user"] { + assert!( + task_intent_steers_read(Some(stated)), + "`{stated}` is grounded in something the caller or repo actually said" + ); + } + assert!( + task_intent_steers_read(None), + "an untagged task predates the intent field; keep the old behaviour" + ); +} + +/// Guards the coupling the test above depends on: the marker asserted here is +/// the one `auto_infer_task` actually writes. If that string ever changes, +/// this fails loudly instead of silently re-enabling the bug (#1590). +#[test] +fn auto_inferred_tasks_are_tagged_with_the_marker_we_filter_on() { + let mut state = crate::core::session::SessionState::default(); + state.set_task("Working on /repo/src/printer (explore)", Some("inferred")); + let intent = state.task.as_ref().and_then(|t| t.intent.clone()); + assert_eq!(intent.as_deref(), Some("inferred")); + assert!(!task_intent_steers_read(intent.as_deref())); +} diff --git a/rust/src/tools/registered/ctx_read_window.rs b/rust/src/tools/registered/ctx_read_window.rs index e91e3c0e4b..3207784fc9 100644 --- a/rust/src/tools/registered/ctx_read_window.rs +++ b/rust/src/tools/registered/ctx_read_window.rs @@ -41,8 +41,13 @@ pub(super) fn anchored_lines_mode(start: i64, limit: Option) -> String { } } pub(super) fn resolve_instruction_file_mode(path: &str, mode: &str) -> (String, Option) { + // #1584: `diff` is preserved alongside the other lossless views. A delta + // against the cached baseline never *withholds* content the agent has not + // already been given, so the "instruction files need complete content" + // rule does not apply — overriding it to `full` re-sent the whole file and + // silently discarded the delta the caller asked for. if !crate::tools::ctx_read::is_instruction_file(path) - || matches!(mode, "full" | "raw" | "anchored") + || matches!(mode, "full" | "raw" | "anchored" | "diff") || mode.starts_with("anchored:") || mode.starts_with("lines:") { @@ -205,4 +210,24 @@ mod tests { Some("lines:5-10".into()), ); } + + /// #1584: `mode=diff` was advertised in the schema and then rejected at + /// runtime. One of the two paths that silently rewrote it was the + /// instruction-file rule, which forced every "lossy-looking" mode to + /// `full`. A delta withholds nothing the caller has not already been + /// given, so it belongs with the lossless views. + #[test] + fn instruction_file_rule_preserves_diff_mode() { + let (mode, note) = resolve_instruction_file_mode("AGENTS.md", "diff"); + assert_eq!(mode, "diff", "diff must survive the instruction-file rule"); + assert!( + note.is_none(), + "no override note when nothing was overridden" + ); + + // The rule itself is intact for genuinely lossy views. + let (mode, note) = resolve_instruction_file_mode("AGENTS.md", "signatures"); + assert_eq!(mode, "full"); + assert!(note.is_some()); + } } diff --git a/rust/tests/quality_loop_golden.rs b/rust/tests/quality_loop_golden.rs index a15ce348de..5039db1848 100644 --- a/rust/tests/quality_loop_golden.rs +++ b/rust/tests/quality_loop_golden.rs @@ -16,7 +16,7 @@ fn params_for(path: &str, old_string: &str) -> EditParams { new_string: "fn replaced() {}".to_string(), replace_all: false, create: false, - expected_md5: None, + expected_blake3: None, expected_size: None, expected_mtime_ms: None, backup: false, diff --git a/rust/tests/suite/edit_reliability.rs b/rust/tests/suite/edit_reliability.rs index 2d9c56dfb4..165b292f67 100644 --- a/rust/tests/suite/edit_reliability.rs +++ b/rust/tests/suite/edit_reliability.rs @@ -138,7 +138,7 @@ fn edit_params(path: &Path, old: &str, new: &str) -> EditParams { new_string: new.to_string(), replace_all: false, create: false, - expected_md5: None, + expected_blake3: None, expected_size: None, expected_mtime_ms: None, backup: false, @@ -153,7 +153,7 @@ fn patch_params(path: &Path, ops: Vec) -> PatchParams { PatchParams { path: path.to_string_lossy().into_owned(), ops, - expected_md5: None, + expected_blake3: None, backup: false, backup_path: None, evidence: false,