`gate_mcp_resources` caps the list at `MCP_RESOURCE_LIMIT` (200 resources),
then describes that truncation with `truncation_notice`, which is written
for byte budgets and was handed `MCP_TEXT_LIMIT`. A server returning 250
resources produced:
[truncated: ListMcpResources result exceeded 32768 bytes; showing 200 of 250 bytes]
Neither figure describes what happened: 32768 is a text budget this path
never applies, and the counts are resources rather than bytes. The notice
goes into a sentinel resource's description, so it is what the model reads
to learn why the list is short -- and it invites the conclusion that the
list was cut for size and would fit under a smaller byte budget.
State the cap that was actually applied, in its own unit, matching the
wording the sibling item-count truncation in `gate_mcp` already uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`8942287` renamed `ProxyMode::System` to `ProxyMode::Default`, changing the
persisted wire value from `"system"` to `"default"`. No migration rewrites
the existing `service_settings` row and `ProxyMode` has no alias, so any
install that ever saved proxy settings on an earlier build now holds a row
this build cannot deserialize:
unknown variant `system`, expected `default` or `custom`
`proxy_settings_secret` turned that into a hard error, and it is the first
statement of every outbound client factory, so on upgrade the failure hits
model calls, plugin installs, search, and the Cursor upstream proxy alike.
It is also unrecoverable from the UI. `proxy_settings` reads the same row,
so the settings page cannot render the proxy card, and `set_proxy_settings`
reads the existing row before it writes, so the user cannot overwrite the
row that broke them. Only editing SQLite by hand clears it.
Read the row through a fallback that logs and returns the default instead.
The affected rows are exactly the ones whose mode meant "no outbound proxy",
which is what the default already is, so nothing is silently changed for
them; a genuinely corrupt row costs the user a re-entered address instead of
a dead install.
Deliberately not `#[serde(alias = "system")]`: `8942287` added a test
asserting that value no longer parses, and this keeps that true while making
the persisted row survivable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- output.rs: the read_lints test helper (#398) builds ToolCall without
the argument_error field added in d83e14a, so the lib-test target
does not compile
- connect_wire.rs: cursor::router takes NetworkClients since 2dad593
- runtime.rs: clippy::nonminimal_bool from 5cdf642
- update/mod.rs: clippy::needless_return in the Windows arm, ddaa61c
A provider that reuses a tool call id across two rounds of one run
wedges the run permanently. `ToolDispatcher::start_batch` skips any call
whose id is in `ToolBatchState::completed`, so the second call is never
dispatched and never produces a `ToolCompletion`, while
`tool_round::execute` blocks waiting for `calls.len()` results with no
timeout on that path. The client sees the tool call appear and then
nothing: no completion, no further output, no end-stream frame.
The `completed` set is built once per run and never cleared, so it is
run-scoped. A tool call id is only unique within a round, which the
schema already states as `UNIQUE (round_id, call_id)`; the sibling
runtime completed-map is likewise already cleared per round at
output.rs:615.
Clear `completed` when `ExecuteToolRound` begins a new round, tracked
independently of `active_round` so it does not depend on the order in
which ToolRoundStarted and ExecuteToolRound are observed. Replaying a
round still skips the calls that round already committed.
The practical trigger is openai_chat.rs:196, which synthesizes
`call-{index}` from a per-stream index when a provider omits tool call
ids, so `call-0` recurs on every model call. That file is left alone
here.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
`release.yml` only runs on `v*` tags, so nothing verifies a commit
before it lands on `main`. `main` is currently red on three of the five
gates `make check` defines, which is the drift this is meant to catch.
Adds a single job running the three Rust gates verbatim as the Makefile
spells them:
cargo fmt --all -- --check
cargo clippy --workspace --all-targets -- -D warnings
cargo test --workspace --all-targets
Conventions follow release.yml so the two workflows stay consistent:
ubuntu-22.04, the same apt packages (the workspace includes
apps/desktop/src-tauri, so even `cargo check` needs webkit2gtk),
dtolnay/rust-toolchain@stable and Swatinem/rust-cache@v2 with the same
`. -> target` workspace key.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`make check` currently fails on main before any change is made: one
`cargo fmt --all -- --check` diff and four `cargo clippy --workspace
--all-targets -- -D warnings` errors. All five are pre-existing and
none of them change behaviour.
- `server/tests/knowledge_rules.rs:125` — rustfmt wants the long
`assert!` split across lines. Applied `cargo fmt --all` verbatim.
- `server/src/plugin/data.rs:206,215` — `path` is only read under
`#[cfg(unix)]`, so every other target sees an unused binding. Added
a `#[cfg(not(unix))] { let _ = path; }` arm, matching the
`let _ = error;` idiom already used at line 189 of the same file.
Windows behaviour is unchanged: these helpers stay no-ops there.
- `server/src/provider/openai_responses.rs:158` — `collapsible_match`.
Applied clippy's own suggestion (move `thinking_open` into a match
guard). The match ends in `_ => {}`, so a failed guard falls through
to a no-op exactly as the inner `if` did.
- `server/src/store/models.rs:327` — `items_after_test_module`. Moved
`optional_u64` and `to_i64` above `mod tests`; the bodies are
untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`Usage::add_assign` folds every field through
fn sum(left: Option<u64>, right: Option<u64>) -> Option<u64> {
left?.checked_add(right?)
}
so `sum(Some(900), None)` is `None`, not `Some(900)`. Adding a record that does
not report a field therefore does not leave the running total alone - it wipes
it.
Both accumulators are per-turn and run over every provider call in the turn:
`run/engine.rs::accumulate_usage` (the compaction call plus each model cycle)
and `cursor/conversation/output.rs::turn_usage`, which is what `turn_ended`
reports to Cursor.
Providers report these fields inconsistently *across calls of one turn*, which
is all it takes. `openai_usage` reads `cache_read_tokens` from
`prompt_tokens_details`, an object most OpenAI-compatible gateways omit until
the prompt cache warms: call 1 (cold) yields `None`, call 2 yields
`Some(1024)`, and the turn reports `None`. The same happens to
`reasoning_tokens` when only some cycles reason, and to `total_tokens` on
gateways that omit it from the streaming usage chunk. Any tool-using turn makes
more than one call, so this is the normal case rather than an edge case.
Treat an unreported count as zero and keep the field unknown only when neither
side reported it. `checked_add` also becomes `saturating_add`: with the new
rule, silently turning an overflow into `None` would be the same erasure by
another route, and token counts never approach `u64::MAX` anyway.
`context_input_tokens` is untouched.
Before (with `left?.checked_add(right?)` restored):
cargo test -p cursor-server --lib model::observability
-> 2 failed: assertion `left == right` failed: left: None, right: Some(900)
After:
cargo test -p cursor-server --lib model::observability -> 3 passed
`truncate_text` looks for a fixed point where the kept prefix length equals the
byte count printed in its own truncation notice. Two things go wrong when the
limit is small, and both are reachable because two call sites pass a *remaining*
budget rather than a constant:
* `gate_grep_content` -> `truncate_text("Grep", .., budget.content_bytes)`
* `gate_mcp` -> `truncate_text("MCP text", .., remaining_text)`
1. The loop can spin forever. `notice.len()` grows with the decimal digit count
of `shown`, so `kept.len()` alternates between two values across a power-of-
ten boundary and never equals `shown`. With `tool_name = "Grep"` this happens
at `limit` 78 and 170; with `"MCP text"` at 82 and 174. The conversation task
then spins at 100% CPU and the turn never completes. `truncate_middle` in
`model/tool_result_replay.rs` already guards against exactly this.
2. When the notice itself does not fit, `available` saturates to 0 and the
function returns the ~68 byte notice alone, i.e. *more* than `limit`.
`gate_grep_content` then evaluates `budget.content_bytes -= <68 bytes>` on a
budget of at most 68, which panics with "attempt to subtract with overflow"
in debug/test builds and wraps in release, silently disabling the 32 KiB
content cap for the rest of the result.
Both are ordinary Grep results away: 16 matches of ~2 KiB leave a two-digit
remainder of the 32 KiB budget, and the next match then hits the small-limit
path.
Fix: return a plain prefix when the notice cannot fit, and stop as soon as the
kept length repeats a previous value. The reported byte count is then off by one
at most in that rare oscillating case, and the result is guaranteed to be at
most `limit` bytes. Every constant-limit call site is unaffected: their notices
always fit and their limits do not oscillate. `budget.content_bytes` now uses
`saturating_sub`, matching every other subtraction in this file.
Verified against the unfixed function:
cargo test -p cursor-server --lib gate::tests::truncate_text_never_exceeds_its_limit
-> FAILED: limit 1 produced 67 bytes
cargo test -p cursor-server --lib gate::tests::grep_content_gate_survives
-> FAILED: panicked at gate.rs:261: attempt to subtract with overflow
cargo test -p cursor-server --lib gate::tests::truncate_text_terminates
-> never returns (killed after 45s)
cargo test -p cursor-server --lib gate::tests::grep_content_gate_terminates
-> never returns (killed after 60s)
After: cargo test -p cursor-server --lib tools::tool_call_result::gate -> 4 passed
`map_finish` matched `"stop" | "content_filter"` before the `has_tools`
fallback, so the fallback only ever applied to finish reasons the adapter did
not recognise. When an OpenAI-compatible server streams tool calls and then
reports `finish_reason: "stop"` the adapter yielded `Done(Stop)` even though
`ToolCallStart` / `ToolCallEnd` had already been emitted.
`run/model_cycle.rs` rejects that combination:
let has_tool_calls = !calls.is_empty();
if matches!(finish_reason, FinishReason::ToolUse) != has_tool_calls {
return Err(failure(RunFailure::Protocol(
"finish reason and tool calls disagree".into()), ...));
}
so the whole turn fails with a protocol error and the tool never runs. Servers
that report `"stop"` alongside `tool_calls` are common in BYOK setups
(llama.cpp, Ollama's OpenAI shim, several proxies), which makes those models
unusable for anything agentic.
Move the `has_tools` arm ahead of `"stop" | "content_filter"` so an observed
tool call outranks the label the provider attached to the stop. This matches
the two sibling adapters (`anthropic.rs` checks `_ if saw_tool` before every
reason except `Length`, `openai_responses.rs` derives the reason from
`saw_tool` alone) and this adapter's own `[DONE]` fallback, which already
infers `ToolUse` from `!tools.is_empty()`. `"length"` still wins so a
truncated response is still reported as truncated, and behaviour with no tool
calls is byte-for-byte unchanged.
Before (with the arm restored):
cargo test -p cursor-server --lib provider::openai_chat
-> observed_tool_calls_outrank_a_stop_finish_reason FAILED
assertion `left == right` failed: left: Stop, right: ToolUse
After:
cargo test -p cursor-server --lib provider::openai_chat -> 3 passed
`local_markdown_rules_land_in_the_request_context_message` never finishes:
`cargo test --workspace` fails on `main` with
panicked at server\tests\local_rules_context.rs:64:14:
run finishes within timeout: Elapsed(())
The Run publishes the conversation checkpoint by asking the client to write
Blobs, and it does not continue until every `KvServerMessage` is answered
with a `SetBlobResult`. The test drained the output stream without replying,
so the Run stalled after the first frame, the provider was never invoked, and
none of the assertions the test exists for were ever reached.
Answer the Blob writes the way every other transport test already does
(`error_lifecycle.rs`, `conversation_delivery.rs`, `interrupt.rs`). With the
acknowledgement in place the Run reaches `EndStream` in ~0.3s and the original
assertions run and pass, so `merge_local_rules` is now genuinely covered:
exactly one `request-context:` message is projected and it carries
`<user_rule>Always answer in haiku.</user_rule>`.
No production code changes.
Before: `cargo test -p cursor-server --test local_rules_context`
-> FAILED (0 passed; 1 failed) after a 5s timeout
After: `cargo test -p cursor-server --test local_rules_context`
-> ok (1 passed; 0 failed) in 0.28s
A tool call that carries no arguments streams no argument text, so
`arguments_text` is empty and `from_str("")` fails with `EOF while parsing
a value`, aborting the whole run. The model cycle already guards this, but
two other consumers did not:
- `ConversationOutput` re-parses the streamed text on `ToolCallEnd`; and
- `create_tool_round` stored the empty text verbatim in the
`arguments_json` column, so re-loading the round (`commit_tool_result`
and the round loader) then failed on `from_str("")`.
Treat empty argument text as an empty object in the output projection, and
persist `{}` for it so the `arguments_json` column always holds valid JSON.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Runtime user messages and context injections both queue into
`pending_injections`, but with different keys: injections use the raw
injection id (committed under `inject-context:{id}`) while user messages
use the full `user-message:{id}` event id. The commit-correlation handler
only stripped the `inject-context:` prefix, so a user message's entry was
never removed.
Consequences:
- the client never received `ContextInjectionDelivered` /
`UserMessageAppended` for the message; and
- `pending_injections` stayed non-empty, so every later `ExecuteToolRound`
was detached without dispatching its tools and `tool_round::execute`
blocked forever -- a hung turn whenever the model made a tool call after
the interruption.
Derive the lookup key by stripping the injection prefix when present and
otherwise using the event id verbatim, so both kinds are cleared and their
delivered/appended events fire.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The tool dispatcher already treats `bash`/`Bash` as an alias of `Shell`
(routing, `is_shell_tool`, and `block_until_ms` normalization), but the
codec only matched `shell`:
- `tool_placeholder` returned `unsupported tool: bash`, which aborts the
turn while streaming the tool call, before it ever runs;
- `request` returned `tool bash is not executed through ExecServerMessage`
(after already reserving an exec slot); and
- `stream_closed` built its shell-specific error result only for `Shell`.
Anthropic models frequently emit `Bash` even when the tool is advertised
as `Shell`, so the alias must hold across the codec. Match `bash` wherever
the codec special-cases `shell`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
StrReplace rejects an empty `old_string`, but the EditNotebook cell-edit
path did not. Because `str::match_indices("")` matches at every byte
boundary, editing a non-empty cell with an empty `old_string` failed with
a misleading "old_string is not unique in the notebook cell; found N
occurrences" error, and editing an empty cell silently prepended
`new_string`.
Add the same guard StrReplace already uses so both edit tools reject an
empty `old_string` consistently.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
