Repository navigation
fix(cli,sdk,api): parallel tool-result pairing, terminal-scope path limit, credential hint - #1
Closed
c01dsummer wants to merge 10 commits into
Closed
c01dsummer wants to merge 10 commits into
c01dsummer wants to merge 10 commits into
Conversation
Finder metadata kept showing up as an untracked file in this macOS checkout. (cherry picked from commit 868a8ac)
Drives the real `claw` binary against the mock Anthropic service for the
`multi_tool_turn_roundtrip` scenario and asserts on the payload the provider
would receive: every `tool_use` of an assistant message must be answered by
`tool_result` blocks in the message immediately after it, and a turn with two
tool calls must still produce exactly one tool-result message.
This lives in `tests/` rather than beside `convert_messages` so it can be
reverted independently of the fix. Run against the pre-fix implementation it
fails with the payload that caused the live 400:
request 1: ["user[text]", "assistant[tool_use, tool_use]",
"user[tool_result]", "user[tool_result]"]
left: ["toolu_multi_read"]
right: ["toolu_multi_read", "toolu_multi_grep"]
The mock service does not validate pairing itself, which is why the contract is
asserted here. `cargo test -p rusty-claude-cli --test
parallel_tool_result_pairing` passes with the fix and fails without it.
(cherry picked from commit 4eba1e2)
`convert_messages` emitted one wire message per `MessageRole::Tool` record. The Anthropic Messages API requires every `tool_use` of an assistant message to be answered by `tool_result` blocks in the single message that follows it, so the second and later results of a parallel tool-call turn landed one message too late and the provider rejected the whole request: api returned 400 Bad Request (invalid_request_error): messages.3: `tool_use` ids were found without `tool_result` blocks immediately after: call_01_4QJqcZ0075kla1ILwra75582 Single-call turns were unaffected, which is why the defect only surfaced when the model requested several tools at once. Evidence: `.claw/sessions/ e49619b247aa3fce/session-1792170907799-0.jsonl`, whose step-1 assistant message carries two `tool_use` blocks followed by two separate tool-result messages. Fix: `convert_messages` is split into a per-message `convert_message` plus a folding accumulator `absorb_tool_result_message`, which merges a run of adjacent tool-result messages into the one `user` message that follows the assistant turn. The merge only extends a message whose content is tool results and nothing else, so user text and assistant turns can never be joined. `converts_tool_roundtrip_messages` keeps its previous single-call contract. Verification: `cargo test -p rusty-claude-cli` (224 unit tests plus the integration targets); `scripts/fmt.sh --check`. (cherry picked from commit 1c0ba66)
… fix Records the reported symptom, the root cause, the fix, and the verification that fails against the pre-fix code. Id 835 is the next free id from `scripts/roadmap-next-id.sh`. (cherry picked from commit 9af3c77)
`TerminalScopeStore::record_path` expanded the correlation key into full hex and chunked it into 180-byte path components. A maximal 512-byte key expands to 1024 hex characters, so the generated path ran past the 1024-byte total-path limit enforced by APFS and `set`/`get` failed with ENAMETOOLONG: read terminal scope '…/6b6b…(1024 hex chars)….json': File name too long (os error 63) The misleading part is where the limit lives: every component stayed within the 255-byte filename limit (five 180-byte directories plus a 129-byte leaf), so the errno named a generated leaf name the caller never chose, and shrinking the chunk cannot help because the hex expansion stays 1024 characters however it is split. The same code passes on Linux, where `PATH_MAX` is 4096, so the `cargo test --workspace` job on ubuntu-latest could not catch it. Fix: records are named from a fixed-width SHA-256 digest (`<32 hex>.json`) and the multi-level chunking is gone. The full key still lives in `TerminalScopeRecord::correlation_key` and is still checked on read, so `encoded_keys_are_isolated_and_cannot_traverse_paths` keeps holding. `sha2` joins the crate's dependencies, reusing the version `runtime` already pulls, so `Cargo.lock` gains one line. The regression test now asserts on the path shape (longest component ≤ 255 bytes, total path < 1024 bytes) instead of only round-tripping, because a round trip inside a short temporary root succeeds even when the generated path is unusable in practice. Verification: `cargo test -p sdk` (119 unit tests, 12 + 109 integration); `scripts/fmt.sh --check`; the new assertion fails against the pre-fix code. (cherry picked from commit 07f020f)
Records the symptom, why the errno pointed at a harmless-looking leaf name, why CI could not see it, and the verification that fails against the pre-fix code. (cherry picked from commit 1a66aad)
The `#[cfg(test)]` budgets (200ms/300ms against production 10s/30s) are deliberately short so the scripted stdio fixtures finish quickly, but they are also why the default parallel `cargo test --workspace` reports failures in `mcp_stdio::tests` and `mcp_tool_bridge::tests`: the fixtures are real `python3` children (cold start measured at 18-21ms here), and a dozen of them starting at once spends the budget on process startup alone. The comment records the shape of the failure, the `--test-threads=1` workaround, and why the budgets stay short, so the next reader does not misread a scheduling artifact as a product defect. (cherry picked from commit f058e84)
…around KNOWN FLAKE (not fixed): records the cause, the measured numbers, why only the two in-crate unit-test modules go red, and the deliberate decision to keep the budgets short rather than widen them for parallel tolerance. This branch also carries pinpoints 835 and 836, which document fixes landing on their own branches: the roadmap is append-only, so the three entries are filed together to keep ROADMAP.md touched by a single PR. (cherry picked from commit 7cb2482)
…al error `claw prompt "say hello"` with `ANTHROPIC_BASE_URL=https://api.deepseek.com/ anthropic` and no credential exported reported: missing Anthropic credentials; export ANTHROPIC_AUTH_TOKEN or ANTHROPIC_API_KEY before calling the Anthropic API Truthful but misleading. Third-party Anthropic-compatible endpoints accept the `ANTHROPIC_API_KEY` / `ANTHROPIC_AUTH_TOKEN` names purely as a wire-compatibility convention — the value has to be a credential the endpoint itself issued, and an `sk-ant-` key is rejected — so the message reads as "obtain an Anthropic key" while never mentioning the endpoint the user had already configured. `anthropic_missing_credentials_hint()` sniffed only `FOREIGN_PROVIDER_ENV_VARS` and ignored `ANTHROPIC_BASE_URL`, so users who had already pointed claw at a gateway got the generic wall. Fix: a `custom_anthropic_base_url_hint()` branch fires whenever `ANTHROPIC_BASE_URL` differs from `anthropic::DEFAULT_BASE_URL` (empty and whitespace-only values are ignored), names the configured URL, and explains the variable-name convention. It runs before the foreign-provider loop: a configured endpoint is a fact about what the user asked for, while a presence check on another provider's key only guesses at intent. Base-url resolution reuses `anthropic::read_base_url()` so the hint cannot disagree with the client's own endpoint. Seven pre-existing hint/error tests depended on the ambient `ANTHROPIC_BASE_URL` and failed under a shell that sets it; they now pin the variable explicitly. Verification: `cargo test -p api` (172 unit tests plus four integration targets); `scripts/fmt.sh --check`; the exact failing command now reports the endpoint it was configured with. (cherry picked from commit ae07657)
Pre-registered as 838 because branches for 835/836/837 are already open against the same append-only section. (cherry picked from commit ff347ce)
Owner
Author
|
Superseded by the upstream PR ultraworkers#3360 — the review target is the upstream repo, not this fork. |
3 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four defects found while investigating a live
clawREPL failure, plus the ROADMAPentries that record them. They are combined here because each one touches the
append-only tail of
ROADMAP.md, so separate PRs would conflict on every merge.fix(cli)— parallel tool-call results were one message too late.convert_messagesemitted one wire message per tool result, so a turn with two tool calls produced
assistant[tool_use, tool_use],user[tool_result],user[tool_result]. The AnthropicMessages API requires every
tool_useto be answered in the single message that followsit, so the request was rejected. Runs of tool results now merge into that one message.
(ROADMAP Claude vibe coding vibe marketing交流群 + ultraworkers/claw-code#835)
fix(sdk)— terminal-scope record paths exceeded the filesystem path limit. A maximal512-byte key expanded to 1024 hex characters and the generated path ran past the 1024-byte
APFS total-path limit, so
set/getfailed withFile name too long. Records are nownamed from a fixed-width SHA-256 digest; the full key still lives in the record and is
still checked on read. (ROADMAP https://github.com/e10nMa2k/cc-mini ultraworkers/claw-code#836)
fix(api)— the missing-credential error ignored a configured non-Anthropic endpoint.With
ANTHROPIC_BASE_URLpointing at a third-party gateway the message named two env vars,which reads as "go get an Anthropic key" and never mentioned the endpoint the user had
configured. A new hint branch names the endpoint and explains the variable-name convention.
(ROADMAP 怎么全都是中文issue xs ultraworkers/claw-code#838)
docs(runtime)— recorded the MCP stdio fixture flake (ROADMAP 为什么还在持续commit？ ultraworkers/claw-code#837, KNOWN FLAKE, notfixed): the
#[cfg(test)]200ms/300ms handshake budgets are spent onpython3cold startwhen a dozen fixture children start at once. The budgets stay short on purpose, so the
cause is documented where the numbers are read instead.
chore—.DS_Storeadded to.gitignore.Anti-slop triage
Classification: actionable-fix
Evidence: the CLI defect reproduces from this repository's own dogfood session
(
.claw/sessions/e49619b247aa3fce/session-1792170907799-0.jsonl), whose step-1 assistantmessage carries two
tool_useblocks followed by two separate tool-result messages:The SDK defect reproduces deterministically with a 512-byte key:
The API defect reproduces with
claw prompt "say hello"andANTHROPIC_BASE_URL=https://api.deepseek.com/anthropicand no credential exported.Non-destructive review result: merge candidate
Verification
Both new regression assertions fail against the pre-fix code. The CLI one was verified
by extracting
mainwithgit archiveinto a separate tree, rebuilding there, and running thenew integration test, which reported the exact payload behind the live 400:
Every file on this branch is byte-identical to the branch it was cherry-picked from
(
git diffper file againstfix/parallel-tool-result-pairing-835,fix/terminal-scope-path-length-836,docs/roadmap-835-837,fix/anthropic-credential-hint-base-url).cargo test --workspace -- --test-threads=1→ 1803 passed, 0 failed, 1 ignored.The ignored case is
api/tests/client_integration.rs::live_stream_smoke_test, markedrequires ANTHROPIC_API_KEY and network access.cargo test -p api -p sdk -p rusty-claude-clion this branch → all targets pass(api 172, sdk 119 + 12 + 109, cli 224 + 109 + 13 + 11 + 8 + 1 + 1).
cargo test --workspace(default parallel) still reports 10-11runtimeMCP fixturefailures: that is the pre-existing flake recorded in ROADMAP 为什么还在持续commit？ ultraworkers/claw-code#837, not a regression — see
the serial baseline above.
scripts/fmt.sh --check;scripts/roadmap-check-ids.sh;python .github/scripts/check_doc_source_of_truth.py;python .github/scripts/check_release_readiness.py;python -m unittest discover -s tests.cargo clippy --workspace --all-targets -- -D warningsreports no findings in the changedcrates; the pre-existing findings in
runtime/trident.rs,runtime/file_ops.rs,runtime/line_range_selector.rs,runtime/compact.rsandclaw-rag-serviceare untouched.No live secrets, tokens, or private logs are included. A credential appeared in the local
conversation log during dogfooding and is not part of this diff (verified by grepping the
whole tree for it).
Resolution gate
ROADMAP Claude vibe coding vibe marketing交流群 + ultraworkers/claw-code#835/https://github.com/e10nMa2k/cc-mini ultraworkers/claw-code#836/为什么还在持续commit？ ultraworkers/claw-code#837/怎么全都是中文issue xs ultraworkers/claw-code#838 are the records)
rely on vibes.