Repository navigation
fix(cli,sdk,api): parallel tool-result pairing, terminal-scope path limit, credential hint - #3360
c01dsummer wants to merge 15 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)
|
Thanks for the fix! This addresses the parallel tool-result pairing failure reported in #3361 (API 400 when concurrent tool results arrive one message too late). Merging adjacent tool-result blocks to preserve pairing, with regression tests, is the right approach. Glad the terminal-scope path limit and the Anthropic credential hint are included too - both were real footguns. |
…sults Review of PR ultraworkers#3360 found two defects in the first cut of the tool-result merge: 1. `absorb_tool_result_message` checked only the accumulated message, so any message that arrived after a tool result was folded into it. With `user -> assistant(tool_use) -> tool_result -> assistant("done")` the reply became an extra `text` block inside the tool-result message and its assistant role was lost. `AnthropicRuntimeClient::stream` converts the whole history on every request, so the corrupted message survived into later turns, and the path was reachable on single-tool-call turns too — the previous tests stopped at the tool-result message and never exercised the round after it. 2. The same `convert_messages` exists in `tools` for the subagent path and was never updated, so the parallel-tool-call 400 was still live there. Fix: a merge now requires both the accumulated message and the incoming one to be pure tool results. `tools/src/lib.rs` folds through the same rule, using the same `absorb_tool_result_message` / `is_tool_result_only` shape. The ROADMAP ultraworkers#835 entry is corrected accordingly: "single-call turns were unaffected" was too strong (the 400 is parallel-only, but the role-swallowing corruption was not), and a "Correction during review" note records both review points. New tests, each failing without the tightened condition: - `keeps_the_assistant_reply_that_follows_tool_results` - `keeps_a_user_follow_up_that_follows_tool_results_separate` - `subagent_merge_requires_both_sides_to_be_tool_results` `subagent_parallel_tool_results_share_one_message` covers the previously unfixed subagent pairing path (it is green either way; it guards the original defect, not the regression). Verification: `cargo test --workspace -- --test-threads=1` -> 1810 passed, 0 failed, 1 ignored (up from 1806 by exactly these four tests); `scripts/fmt.sh --check`; roadmap and doc gates pass.
|
Both review points were correct, and the first one was worse than "scope too broad" — it was a live regression.
Reproduced exactly that with a test before touching the fix. Two consequences worth calling out: it did not require parallel calls (single-call turns hit it too), and because New tests, both failing without the tightened condition:
The missed call site. Actually harmful claim removed. ROADMAP #835 said "single-call turns were unaffected, which is why the defect only showed up when the model requested parallel tools". The 400 is parallel-only, but the role-swallowing corruption was not, so that sentence now reads differently and the entry carries a "Correction during review" note. Baseline, re-measured on the branch just pushed: 1806 → 1810 is exactly the four new tests. Rendering refactor in #836 (32-hex filename still carrying the full key in the record for the read-time check), #838 (wording only; auth resolution untouched; custom base URL ahead of the foreign-key guess) and #837 (budgets deliberately left alone) are unchanged, as reviewed. |
`tools` (lib test) reported two findings, both from the tests added in the
review fix for the subagent path:
- unused import `ConversationMessage` — the tests spelled the type as
`runtime::ConversationMessage::...`, leaving the import idle. The tests now
use the imported name.
- `tool_use_message` was never used — its one call site had been inlined into
`ContentBlock::ToolUse` when the test was written. Removed.
No behavior change; `cargo test -p tools` is 115 passed, and the two warnings
are gone (`cargo clippy -p tools --all-targets`).
The remaining review observation — CLI and `tools` each carry the same folding
logic — is left alone on purpose. The two conversions are not identical:
`tools` drops empty `Text` blocks (`InputContentBlock::Text { text } if
text.is_empty()`) because the subagent path treats them as absent, while the CLI
keeps them. Hoisting one shared helper therefore means first deciding which
behavior wins and changing one caller's wire output, which is a separate change
rather than a mechanical move, and it does not belong in a bug-fix PR.
|
Both warnings were real, and both were mine — introduced by The import was idle because the tests spelled the type as On the duplicated folding logic: I agree it is new duplication from
Hoisting one shared helper therefore requires deciding which behavior wins and changing one caller's wire output, which is a behavior change, not a mechanical move. It does not belong inside a bug-fix PR where every hunk should be individually justifiable. If you would rather have the dedup here, say which behavior is correct for both paths and I will fold it in (and adjust the one test that pins the current CLI behavior: Baseline after the cleanup, re-measured on Unchanged from the previous measurement, as expected for a refactor that only removes dead code — the warnings were the only deltas. All five per-defect branches were realigned to this head ( 836/838/837 are untouched in this commit, as reviewed. |
Review of PR ultraworkers#3360 corrected the rationale recorded in 79dbf85. That message implied a shared helper is impossible because the two `convert_messages` call sites differ on empty `Text` blocks. The difference is real but it lives in `convert_message`, not in the fold: `absorb_tool_result_message` and `is_tool_result_only` are byte-for-byte identical in `rusty-claude-cli` and `tools`, and hoisting them would not touch empty-text handling at all. The empty-text filter in `tools` also predates this work (introduced by 75c08bc). The duplication is still kept — a reviewer decision, since unifying the two call sites is a separate behavior change — but the reason is now stated accurately at both definitions instead of being inferred from a wrong premise. No behavior change.
|
You are right, and the correction matters: I moved a true statement about Checked your three points against the tree:
So the accurate split is exactly the one you stated: the empty-text difference justifies not unifying the whole Warnings are gone (
|
|
Nice catch on the parallel pairing — the Anthropic contract does require every tool_use to be answered in the single message that follows, so emitting one wire message per result would break any turn with more than one parallel call. Tightening the merge rule so both sides must be pure tool results (and mirroring it in the subagent convert_messages) also closes the role-loss bug on single-call turns. The SHA-256 naming for terminal-scope records is a clean fix too — keeping the full key inside the record means lookups stay verifiable without paying the path-length cost. One question: does the wire-contract test cover the subagent path as well, or is that conversion still only exercised indirectly? |
1 similar comment
|
Nice catch on the parallel pairing — the Anthropic contract does require every tool_use to be answered in the single message that follows, so emitting one wire message per result would break any turn with more than one parallel call. Tightening the merge rule so both sides must be pure tool results (and mirroring it in the subagent convert_messages) also closes the role-loss bug on single-call turns. The SHA-256 naming for terminal-scope records is a clean fix too — keeping the full key inside the record means lookups stay verifiable without paying the path-length cost. One question: does the wire-contract test cover the subagent path as well, or is that conversion still only exercised indirectly? |
Review of PR ultraworkers#3360 caught the same conflation twice, and it is worth fixing in the source rather than only in a comment thread. The note added in 5d58eca implied the fold cannot be hoisted because the two `convert_messages` call sites disagree on empty `Text` blocks. That disagreement is real, but it belongs to `convert_message`; `absorb_tool_result_message` and `is_tool_result_only` are identical in both crates and hoisting only them would not touch empty text. The earlier claim that empty text cannot occur was also too strong. `flush_text_block` filters the model stream, but `ConversationMessage::user_text` does not, and a persisted `"text":""` restored from a session log goes through `required_string` with no emptiness check. Both reach `convert_messages`, so the CLI would send an empty `Text` block where `tools` drops it — narrow, but reachable by session recovery rather than impossible. The comment now states which piece the divergence constrains and how it is reached. No behavior change; duplication kept, per review.
|
Two corrections accepted, both mine. "Empty text cannot occur" was too strong. The Duplication is kept, per your decision; the comment still says the two copies must be changed together. State of the branch: |
|
Fair question — the honest answer is no: the subagent conversion has unit coverage of What exists:
The only production caller is Closing it is mechanical but not free:
The reason I did not include it initially is that |
|
Shorter version of the above, since the distinction is what matters: The wire-contract test only drives the The subagent converter is covered directly, by two unit tests in
There is no captured HTTP body for that path. Those unit tests assert the shape of the message that is about to be sent, not what was actually transmitted, so a regression in serialisation would still slip through. That gap is closeable: add |
…ults Review of PR ultraworkers#3360 asked whether the wire-contract test covers the subagent path. It did not: `parallel_tool_result_pairing.rs` drives the `claw` binary, so it exercises the CLI converter only. The subagent converter in `tools` is reached exclusively through `ProviderRuntimeClient::stream`, which no test called, and the two unit tests for it assert on the converted `InputMessage` values rather than on the bytes that leave the process. This adds the missing end-to-end check: start `mock-anthropic-service`, point `ANTHROPIC_BASE_URL` (and a dummy credential) at it, drive `ProviderRuntimeClient::stream` with a turn whose two tool calls already have results, and assert on `captured_requests()[0].raw_body` — exactly one message carries `tool_use` blocks, exactly one carries `tool_result` blocks, and both results are answered in call order. Against the unfixed merge the body contains two separate `role: "user"` result messages and the test fails, which is the regression it is there to catch. Notes on the test harness, since neither is obvious from the code: - the mock service needs a multi-thread runtime: with `new_current_thread` the single worker is blocked by the synchronous `stream()` call from the test thread and the request never completes; - `stream()` issues a `/v1/messages/count_tokens` preflight first, so the test selects `path == "/v1/messages"` instead of assuming a single captured request; - the client reads both credential variables and the base URL from the process environment, so the test saves and restores all three. `mock-anthropic-service` is added to `tools`' dev-dependencies (it had none). No production code changes. Verification: `cargo test -p tools --lib` -> 116 passed (was 115); `cargo test --workspace -- --test-threads=1` -> 1811 passed, 0 failed, 1 ignored (was 1810); `scripts/fmt.sh --check`.
|
Closed the gap in
Verified against the unfixed merge: with the merge condition disabled the captured body has two separate Three details worth knowing, none obvious from reading the code:
Workspace baseline: 1811 passed / 0 failed / 1 ignored with |
At the time you asked: no. The wire-contract test drives the Fixed in
Checked against the unfixed merge: with the condition disabled the captured body contains two separate Three things that were not visible from reading the code and cost me the first two attempts:
|
|
Nice catch on the SDK path limit — a 512-byte key hex-expanding past the APFS 1024-byte path cap is the kind of edge case that is easy to miss until it fails in production. Naming records from a fixed-width SHA-256 digest while keeping the full key inside the record and still verifying it on read keeps the scheme safe without losing the original key. The tightened merge condition (both sides must be pure tool results) also looks correct — folding an assistant message that follows a tool result would have silently corrupted history, and reproducing it with a failing test first is exactly the right way to pin that down. One question: do existing keys written under the old hex scheme need migration, or is terminal-scope data short-lived enough that it is fine to let them expire naturally? |
|
db147cd closes the subagent wire-coverage gap I flagged earlier — asserting on the captured raw_body instead of converted values is the right level, and the three gotchas (multi-thread mock runtime, count_tokens preflight, env save/restore) are exactly the details that would have cost someone else two attempts. On the #836 migration question from my previous comment: whether existing hex-named records need a one-off rename depends on how the read path locates a file. If reads derive the path through the same naming function, old hex entries become unreachable once the digest scheme ships; if reads keep a legacy-path fallback, natural expiry is fine. Worth pinning that down in the ROADMAP #836 entry so the behavior is explicit either way. |
|
db147cd closes the subagent wire-coverage gap I flagged earlier — asserting on the captured raw_body instead of converted values is the right level, and the three gotchas (multi-thread mock runtime, count_tokens preflight, env save/restore) are exactly the details that would have cost someone else two attempts. On the #836 migration question from my previous comment: whether existing hex-named records need a one-off rename depends on how the read path locates a file. If reads derive the path through the same naming function, old hex entries become unreachable once the digest scheme ships; if reads keep a legacy-path fallback, natural expiry is fine. Worth pinning that down in the ROADMAP #836 entry so the behavior is explicit either way. |
2 similar comments
|
db147cd closes the subagent wire-coverage gap I flagged earlier — asserting on the captured raw_body instead of converted values is the right level, and the three gotchas (multi-thread mock runtime, count_tokens preflight, env save/restore) are exactly the details that would have cost someone else two attempts. On the #836 migration question from my previous comment: whether existing hex-named records need a one-off rename depends on how the read path locates a file. If reads derive the path through the same naming function, old hex entries become unreachable once the digest scheme ships; if reads keep a legacy-path fallback, natural expiry is fine. Worth pinning that down in the ROADMAP #836 entry so the behavior is explicit either way. |
|
db147cd closes the subagent wire-coverage gap I flagged earlier — asserting on the captured raw_body instead of converted values is the right level, and the three gotchas (multi-thread mock runtime, count_tokens preflight, env save/restore) are exactly the details that would have cost someone else two attempts. On the #836 migration question from my previous comment: whether existing hex-named records need a one-off rename depends on how the read path locates a file. If reads derive the path through the same naming function, old hex entries become unreachable once the digest scheme ships; if reads keep a legacy-path fallback, natural expiry is fine. Worth pinning that down in the ROADMAP #836 entry so the behavior is explicit either way. |
…ults Review of PR ultraworkers#3360 asked whether the wire-contract test covers the subagent path. It did not: `parallel_tool_result_pairing.rs` drives the `claw` binary, so it exercises the CLI converter only. The subagent converter in `tools` is reached exclusively through `ProviderRuntimeClient::stream`, which no test called, and the two unit tests for it assert on the converted `InputMessage` values rather than on the bytes that leave the process. This adds the missing end-to-end check: start `mock-anthropic-service`, point `ANTHROPIC_BASE_URL` (and a dummy credential) at it, drive `ProviderRuntimeClient::stream` with a turn whose two tool calls already have results, and assert on `captured_requests()[0].raw_body` — exactly one message carries `tool_use` blocks, exactly one carries `tool_result` blocks, and both results are answered in call order. Against the unfixed merge the body contains two separate `role: "user"` result messages and the test fails, which is the regression it is there to catch. Notes on the test harness, since neither is obvious from the code: - the mock service needs a multi-thread runtime: with `new_current_thread` the single worker is blocked by the synchronous `stream()` call from the test thread and the request never completes; - `stream()` issues a `/v1/messages/count_tokens` preflight first, so the test selects `path == "/v1/messages"` instead of assuming a single captured request; - the client reads both credential variables and the base URL from the process environment, so the test saves and restores all three. `mock-anthropic-service` is added to `tools`' dev-dependencies (it had none). No production code changes. Verification: `cargo test -p tools --lib` -> 116 passed (was 115); `cargo test --workspace -- --test-threads=1` -> 1811 passed, 0 failed, 1 ignored (was 1810); `scripts/fmt.sh --check`.
db147cd to
0df74ab
Compare
…ial hint Merge PR #3360 from c01dsummer/claw-code (fix/roadmap-835-838) Four fixes addressing issues #3361 and #3360: 1. Parallel tool-result pairing (835): The Anthropic Messages API requires all tool results of a turn to be in a single message. When a turn issued two tool calls at once, the results were sent in separate messages, causing a 400 error. Fixed by merging adjacent tool-result-only messages into one. 2. Terminal-scope path limit (836): Correlation keys up to 512 bytes expanded to 1024+ hex characters, exceeding APFS's 1024-byte total path limit. Fixed by using SHA256 digest (32 hex chars) for path components while storing the full key in the record for exact matching. 3. Credential hint (838): When ANTHROPIC_BASE_URL points to a non-Anthropic endpoint (DeepSeek, corporate gateways), the missing-credentials error now explains that the endpoint expects its own credentials, not Anthropic's. 4. ROADMAP entries: Documents all three fixes with verification details. All tests pass (370+ for rusty-claude-cli, 240+ for sdk, 122+ for tools, 209+ for api). Formatting verified with cargo fmt. CI checks approved and passed. Fixes #3361, #3360
3 participants
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交流群 + #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 #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 #838)
docs(runtime)— recorded the MCP stdio fixture flake (ROADMAP 为什么还在持续commit？ #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.
fix(cli,tools)— review follow-up. The first cut of (1) checked only the accumulatedmessage when merging, which folded the message that followed a tool result into it and lost its
role — reachable on single-call turns too, and re-read on every request. A merge now requires
both sides to be pure tool results, and
tools/src/lib.rs, which carries its ownconvert_messagesfor the subagent path, applies the same rule; the parallel-call 400 was stilllive there.
test(tools)— wire-level coverage for the subagent path. The existing wire-contracttest only drives the
clawbinary, so it covers the CLI converter. The subagent converter intoolsis reached only throughProviderRuntimeClient::stream, which no test called, and itstwo unit tests assert on converted values rather than the transmitted bytes. A new test starts
the mock service, drives
streamwith a two-call turn, and asserts on the captured body.chore—.DS_Storeadded to.gitignore.Same-repo branches for per-defect review
This PR is one linear branch so that
ROADMAP.mdis touched by a single change set. The samecommits also exist as one branch per defect in the fork, which are easier to review in isolation.
All five were realigned after the review fixes below, so every file matches this branch exactly
(verified per file with
git diff fix/roadmap-835-838 <branch> -- <paths>):fix/parallel-tool-result-pairing-835convert_messagespairing, the tightened merge rule, and the wire-contract test9b6c89bfix/terminal-scope-path-length-836edf7274fix/anthropic-credential-hint-base-urlANTHROPIC_BASE_URLef86586docs/roadmap-835-8370de4f54chore/ignore-ds-store.gitignore868a8acThe ROADMAP conflicts between them were resolved here by keeping every numbered entry in numeric
order; no entry text was altered in the process, and the #835 entry additionally carries the
correction recorded in this PR's review reply.
The branches are realigned to the current head
db147cd, so the SHAs above are the current heads.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→ 1811 passed, 0 failed, 1 ignored(measured on this branch, not transcribed).
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？ #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
resolves triage: parallel tool-call turns send
tool_resultblocks one message too late, so the next request is rejected with 400 #3361 (the parallel tool-result 400), and ROADMAPClaude vibe coding vibe marketing交流群 + #835/https://github.com/e10nMa2k/cc-mini #836/为什么还在持续commit？ #837/怎么全都是中文issue xs #838 record all four changes. "Refs" rather than "Closes" so the
maintainer decides when the issue is done.
rely on vibes.