diff --git a/ROADMAP.md b/ROADMAP.md index 5bf89472..9e267ae6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -194,6 +194,20 @@ Workers should distinguish: Acceptance: - no more ambiguous "tests passed" messaging - merge policy can require the correct green level for the lane type +- a single hung test must not mask other failures: enforce per-test + timeouts in CI (`cargo test --workspace`) so a 6-minute hang in one + crate cannot prevent downstream crates from running their suites +- when a CI job fails because of a hang, the worker must report it as + `test.hung` rather than a generic failure, so triage doesn't conflate + it with a normal `assertion failed` +- recorded pinpoint (2026-04-08): `be561bf` swapped the local + byte-estimate preflight for a `count_tokens` round-trip and silently + returned `Ok(())` on any error, so `send_message_blocks_oversized_*` + hung for ~6 minutes per attempt; the resulting workspace job crash + hid 6 *separate* pre-existing CLI regressions (compact flag + discarded, piped stdin vs permission prompter, legacy session layout, + help/prompt assertions, mock harness count) that only became + diagnosable after `8c6dfe5` + `5851f2d` restored the fast-fail path ## Phase 4 — Claws-First Task Execution @@ -307,11 +321,16 @@ Priority order: P0 = blocks CI/green state, P1 = blocks integration wiring, P2 = 18. **Context-window preflight gap** — **done**: provider request sizing now emits `context_window_blocked` before oversized requests leave the process, using a model-context registry instead of the old naive max-token heuristic. 19. **Subcommand help falls through into runtime/API path** — **done**: `claw doctor --help`, `claw status --help`, `claw sandbox --help`, and nested `mcp`/`skills` help are now intercepted locally without runtime/provider startup, with regression tests covering the direct CLI paths. 20. **Session state classification gap (working vs blocked vs finished vs truly stale)** — **done**: agent manifests now derive machine states such as `working`, `blocked_background_job`, `blocked_merge_conflict`, `degraded_mcp`, `interrupted_transport`, `finished_pending_report`, and `finished_cleanable`, and terminal-state persistence records commit provenance plus derived state so downstream monitoring can distinguish quiet progress from truly idle sessions. -21. **Resumed `/status` JSON parity gap** — dogfooding shows fresh `claw status --output-format json` now emits structured JSON, but resumed slash-command status still leaks through a text-shaped path in at least one dispatch path. Local CI-equivalent repro fails `rust/crates/rusty-claude-cli/tests/resume_slash_commands.rs::resumed_status_command_emits_structured_json_when_requested` with `expected value at line 1 column 1`, so resumed automation can receive text where JSON was explicitly requested. **Action:** unify fresh vs resumed `/status` rendering through one output-format contract and add regression coverage so resumed JSON output is guaranteed valid. -22. **Opaque failure surface for session/runtime crashes** — repeated dogfood-facing failures can currently collapse to generic wrappers like `Something went wrong while processing your request. Please try again, or use /new to start a fresh session.` without exposing whether the fault was provider auth, session corruption, slash-command dispatch, render failure, or transport/runtime panic. This blocks fast self-recovery and turns actionable clawability bugs into blind retries. **Action:** preserve a short user-safe failure class (`provider_auth`, `session_load`, `command_dispatch`, `render`, `runtime_panic`, etc.), attach a local trace/session id, and ensure operators can jump from the chat-visible error to the exact failure log quickly. +21. **Resumed `/status` JSON parity gap** — **done**: resolved by the broader "Resumed local-command JSON parity gap" work tracked as #26 below. Re-verified on `main` HEAD `8dc6580` — `cargo test --release -p rusty-claude-cli resumed_status_command_emits_structured_json_when_requested` passes cleanly (1 passed, 0 failed), so resumed `/status --output-format json` now goes through the same structured renderer as the fresh CLI path. The original failure (`expected value at line 1 column 1` because resumed dispatch fell back to prose) no longer reproduces. +22. **Opaque failure surface for session/runtime crashes** — **done**: `safe_failure_class()` in `error.rs` classifies all API errors into 8 user-safe classes (`provider_auth`, `provider_internal`, `provider_retry_exhausted`, `provider_rate_limit`, `provider_transport`, `provider_error`, `context_window`, `runtime_io`). `format_user_visible_api_error` in `main.rs` attaches session ID + request trace ID to every user-visible error. Coverage in `opaque_provider_wrapper_surfaces_failure_class_session_and_trace` and 3 related tests. 23. **`doctor --output-format json` check-level structure gap** — **done**: `claw doctor --output-format json` now keeps the human-readable `message`/`report` while also emitting structured per-check diagnostics (`name`, `status`, `summary`, `details`, plus typed fields like workspace paths and sandbox fallback data), with regression coverage in `output_format_contract.rs`. 24. **Plugin lifecycle init/shutdown test flakes under workspace-parallel execution** — dogfooding surfaced that `build_runtime_runs_plugin_lifecycle_init_and_shutdown` can fail under `cargo test --workspace` while passing in isolation because sibling tests race on tempdir-backed shell init script paths. This is test brittleness rather than a code-path regression, but it still destabilizes CI confidence and wastes diagnosis cycles. **Action:** isolate temp resources per test robustly (unique dirs + no shared cwd assumptions), audit cleanup timing, and add a regression guard so the plugin lifecycle test remains stable under parallel workspace execution. +25. **`plugins::hooks::collects_and_runs_hooks_from_enabled_plugins` flaked on Linux CI, root cause was a stdin-write race not missing exec bit** — **done at `172a2ad` on 2026-04-08**. Dogfooding reproduced this four times on `main` (CI runs [24120271422](https://github.com/ultraworkers/claw-code/actions/runs/24120271422), [24120538408](https://github.com/ultraworkers/claw-code/actions/runs/24120538408), [24121392171](https://github.com/ultraworkers/claw-code/actions/runs/24121392171), [24121776826](https://github.com/ultraworkers/claw-code/actions/runs/24121776826)), escalating from first-attempt-flake to deterministic-red on the third push. Failure mode was `PostToolUse hook .../hooks/post.sh failed to start for "Read": Broken pipe (os error 32)` surfacing from `HookRunResult`. **Initial diagnosis was wrong.** The first theory (documented in earlier revisions of this entry and in the root-cause note on commit `79da4b8`) was that `write_hook_plugin` in `rust/crates/plugins/src/hooks.rs` was writing the generated `.sh` files without the execute bit and `Command::new(path).spawn()` was racing on fork/exec. An initial chmod-only fix at `4f7b674` was shipped against that theory and **still failed CI on run `24121776826`** with the same `Broken pipe` symptom, falsifying the chmod-only hypothesis. **Actual root cause.** `CommandWithStdin::output_with_stdin` in `rust/crates/plugins/src/hooks.rs` was unconditionally propagating `write_all` errors on the child's stdin pipe, including `std::io::ErrorKind::BrokenPipe`. The test hook scripts run in microseconds (`#!/bin/sh` + a single `printf`), so the child exits and closes its stdin before the parent finishes writing the ~200-byte JSON hook payload. On Linux the pipe raises `EPIPE` immediately; on macOS the pipe happens to buffer the small payload before the child exits, which is why the race only surfaced on ubuntu CI runners. The parent's `write_all` returned `Err(BrokenPipe)`, `output_with_stdin` returned that as a hook failure, and `run_command` classified the hook as "failed to start" even though the child had already run to completion and printed the expected message to stdout. **Fix (commit `172a2ad`, force-pushed over `4f7b674`).** Three parts: (1) **actual fix** — `output_with_stdin` now matches the `write_all` result and swallows `BrokenPipe` specifically, while propagating all other write errors unchanged; after a `BrokenPipe` swallow the code still calls `wait_with_output()` so stdout/stderr/exit code are still captured from the cleanly-exited child. (2) **hygiene hardening** — a new `make_executable` helper sets mode `0o755` on each generated `.sh` via `std::os::unix::fs::PermissionsExt` under `#[cfg(unix)]`. This is defense-in-depth for future non-sh hook runners, not the bug that was biting CI. (3) **regression guard** — new `generated_hook_scripts_are_executable` test under `#[cfg(unix)]` asserts each generated `.sh` file has at least one execute bit set (`mode & 0o111 != 0`) so future tweaks cannot silently regress the hygiene change. **Verification.** `cargo test --release -p plugins` 35 passing, fmt clean, clippy `-D warnings` clean; CI run [24121999385](https://github.com/ultraworkers/claw-code/actions/runs/24121999385) went green on first attempt on `main` for the hotfix commit. **Meta-lesson.** `Broken pipe (os error 32)` from a child-process spawn path is ambiguous between "could not exec" and "exec'd and exited before the parent finished writing stdin." The first theory cargo-culted the "could not exec" reading because the ROADMAP scaffolding anchored on the exec-bit guess; falsification came from empirical CI, not from code inspection. Record the pattern: when a pipe error surfaces on fork/exec, instrument what `wait_with_output()` actually reports on the child before attributing the failure to a permissions or path issue. 26. **Resumed local-command JSON parity gap** — **done**: direct `claw --output-format json` already had structured renderers for `sandbox`, `mcp`, `skills`, `version`, and `init`, but resumed `claw --output-format json --resume /…` paths still fell back to prose because resumed slash dispatch only emitted JSON for `/status`. Resumed `/sandbox`, `/mcp`, `/skills`, `/version`, and `/init` now reuse the same JSON envelopes as their direct CLI counterparts, with regression coverage in `rust/crates/rusty-claude-cli/tests/resume_slash_commands.rs` and `rust/crates/rusty-claude-cli/tests/output_format_contract.rs`. +27. **`dev/rust` `cargo test -p rusty-claude-cli` reads host `~/.claude/plugins/installed/` from real `$HOME` and fails parse-time on any half-installed user plugin** — dogfooding on 2026-04-08 (filed from gaebal-gajae's clawhip bullet at message `1491322807026454579` after the provider-matrix branch QA surfaced it) reproduced 11 deterministic failures on clean `dev/rust` HEAD of the form `panicked at crates/rusty-claude-cli/src/main.rs:3953:31: args should parse: "hook path \`/Users/yeongyu/.claude/plugins/installed/sample-hooks-bundled/./hooks/pre.sh\` does not exist; hook path \`...\post.sh\` does not exist"` covering `parses_prompt_subcommand`, `parses_permission_mode_flag`, `defaults_to_repl_when_no_args`, `parses_resume_flag_with_slash_command`, `parses_system_prompt_options`, `parses_bare_prompt_and_json_output_flag`, `rejects_unknown_allowed_tools`, `parses_resume_flag_with_multiple_slash_commands`, `resolves_model_aliases_in_args`, `parses_allowed_tools_flags_with_aliases_and_lists`, `parses_login_and_logout_subcommands`. **Same failures do NOT reproduce on `main`** (re-verified with `cargo test --release -p rusty-claude-cli` against `main` HEAD `79da4b8`, all 156 tests pass). **Root cause is two-layered.** First, on `dev/rust` `parse_args` eagerly walks user-installed plugin manifests under `~/.claude/plugins/installed/` and validates that every declared hook script exists on disk before returning a `CliAction`, so any half-installed plugin in the developer's real `$HOME` (in this case `~/.claude/plugins/installed/sample-hooks-bundled/` whose `.claude-plugin` manifest references `./hooks/pre.sh` and `./hooks/post.sh` but whose `hooks/` subdirectory was deleted) makes argv parsing itself fail. Second, the test harness on `dev/rust` does not redirect `$HOME` or `XDG_CONFIG_HOME` to a fixture for the duration of the test — there is no `env_lock`-style guard equivalent to the one `main` already uses (`grep -n env_lock rust/crates/rusty-claude-cli/src/main.rs` returns 0 hits on `dev/rust` and 30+ hits on `main`). Together those two gaps mean `dev/rust` `cargo test -p rusty-claude-cli` is non-deterministic on every clean clone whose owner happens to have any non-pristine plugin in `~/.claude/`. **Action (two parts).** (a) Backport the `env_lock`-based test isolation pattern from `main` into `dev/rust`'s `rusty-claude-cli` test module so each test runs against a temp `$HOME`/`XDG_CONFIG_HOME` and cannot read host plugin state. (b) Decouple `parse_args` from filesystem hook validation on `dev/rust` (the same decoupling already on `main`, where hook validation happens later in the lifecycle than argv parsing) so even outside tests a partially installed user plugin cannot break basic CLI invocation. **Branch scope.** This is a `dev/rust` catchup against `main`, not a `main` regression. Tracking it here so the dev/rust merge train picks it up before the next dev/rust release rather than rediscovering it in CI. + +28. **Auth-provider truth: error copy fails real users at the env-var-vs-header layer** — dogfooded live on 2026-04-08 in #claw-code (Sisyphus Labs guild), two separate new users hit adjacent failure modes within minutes of each other that both trace back to the same root: the `MissingApiKey` / 401 error surface does not teach users how the auth inputs map to HTTP semantics, so a user who sets a "reasonable-looking" env var still hits a hard error with no signpost. **Case 1 (varleg, Norway).** Wanted to use OpenRouter via the OpenAI-compat path. Found a comparison table claiming "provider-agnostic (Claude, OpenAI, local models)" and assumed it Just Worked. Set `OPENAI_API_KEY` to an OpenRouter `sk-or-v1-...` key and a model name without an `openai/` prefix; claw's provider detection fell through to Anthropic first because `ANTHROPIC_API_KEY` was still in the environment. Unsetting `ANTHROPIC_API_KEY` got them `ANTHROPIC_AUTH_TOKEN or ANTHROPIC_API_KEY is not set` instead of a useful hint that the OpenAI path was right there. Fix delivered live as a channel reply: use `main` branch (not `dev/rust`), export `OPENAI_BASE_URL=https://openrouter.ai/api/v1` alongside `OPENAI_API_KEY`, and prefix the model name with `openai/` so the prefix router wins over env-var presence. **Case 2 (stanley078852).** Had set `ANTHROPIC_AUTH_TOKEN="sk-ant-..."` and was getting 401 `Invalid bearer token` from Anthropic. Root cause: `sk-ant-` keys are `x-api-key`-header keys, not bearer tokens. `ANTHROPIC_API_KEY` path in `anthropic.rs` sends the value as `x-api-key`; `ANTHROPIC_AUTH_TOKEN` path sends it as `Authorization: Bearer` (for OAuth access tokens from `claw login`). Setting an `sk-ant-` key in the wrong env var makes claw send it as `Bearer sk-ant-...` which Anthropic rejects at the edge with 401 before it ever reaches the completions endpoint. The error text propagated all the way to the user (`api returned 401 Unauthorized (authentication_error) ... Invalid bearer token`) with zero signal that the problem was env-var choice, not key validity. Fix delivered live as a channel reply: move the `sk-ant-...` key to `ANTHROPIC_API_KEY` and unset `ANTHROPIC_AUTH_TOKEN`. **Pattern.** Both cases are failures at the *auth-intent translation* layer: the user chose an env var that made syntactic sense to them (`OPENAI_API_KEY` for OpenAI, `ANTHROPIC_AUTH_TOKEN` for Anthropic auth) but the actual wire-format routing requires a more specific choice. The error messages surface the HTTP-layer symptom (401, missing-key) without bridging back to "which env var should you have used and why." **Action.** Three concrete improvements, scoped for a single `main`-side PR: (a) In `ApiError::MissingCredentials` Display, when the Anthropic path is the one being reported but `OPENAI_API_KEY`, `XAI_API_KEY`, or `DASHSCOPE_API_KEY` are present in the environment, extend the message with "— but I see `$OTHER_KEY` set; if you meant to use that provider, prefix your model name with `openai/`, `grok`, or `qwen/` respectively so prefix routing selects it." (b) In the 401-from-Anthropic error path in `anthropic.rs`, when the failing auth source is `BearerToken` AND the bearer token starts with `sk-ant-`, append "— looks like you put an `sk-ant-*` API key in `ANTHROPIC_AUTH_TOKEN`, which is the Bearer-header path. Move it to `ANTHROPIC_API_KEY` instead (that env var maps to `x-api-key`, which is the correct header for `sk-ant-*` keys)." Same treatment for OAuth access tokens landing in `ANTHROPIC_API_KEY` (symmetric mis-assignment). (c) In `rust/README.md` on `main` and the matrix section on `dev/rust`, add a short "Which env var goes where" paragraph mapping `sk-ant-*` → `ANTHROPIC_API_KEY` and OAuth access token → `ANTHROPIC_AUTH_TOKEN`, with the one-line explanation of `x-api-key` vs `Authorization: Bearer`. **Verification path.** Both improvements can be tested with unit tests against `ApiError::fmt` output (the prefix-routing hint) and with a targeted integration test that feeds an `sk-ant-*`-shaped token into `BearerToken` and asserts the fmt output surfaces the correction hint (no HTTP call needed). **Source.** Live users in #claw-code at `1491328554598924389` (varleg) and `1491329840706486376` (stanley078852) on 2026-04-08. **Partial landing (`ff1df4c`).** Action parts (a), (b), (c) shipped on `main`: `MissingCredentials` now carries an optional hint field and renders adjacent-provider signals, Anthropic 401 + `sk-ant-*` bearer gets a correction hint, USAGE.md has a "Which env var goes where" section. BUT the copy fix only helps users who fell through to the Anthropic auth path by accident — it does NOT fix the underlying routing bug where the CLI instantiates `AnthropicRuntimeClient` unconditionally and ignores prefix routing at the runtime-client layer. That deeper routing gap is tracked separately as #29 below and was filed within hours of #28 landing when live users still hit `missing Anthropic credentials` with `--model openai/gpt-4` and all `ANTHROPIC_*` env vars unset. +29. **CLI provider dispatch is hardcoded to Anthropic, ignoring prefix routing** — **done at `8dc6580` on 2026-04-08**. Changed `AnthropicRuntimeClient.client` from concrete `AnthropicClient` to `ApiProviderClient` (the api crate’s `ProviderClient` enum), which dispatches to Anthropic / xAI / OpenAi at construction time based on `detect_provider_kind(&resolved_model)`. 1 file, +59 −7, all 182 rusty-claude-cli tests pass, CI green at run `24125825431`. Users can now run `claw --model openai/gpt-4.1-mini prompt "hello"` with only `OPENAI_API_KEY` set and it routes correctly. **Original filing below for the trace record.** Dogfooded live on 2026-04-08 within hours of ROADMAP #28 landing. Users in #claw-code (nicma at `1491342350960562277`, Jengro at `1491345009021030533`) followed the exact "use main, set OPENAI_API_KEY and OPENAI_BASE_URL, unset ANTHROPIC_*, prefix the model with `openai/`" checklist from the #28 error-copy improvements AND STILL hit `error: missing Anthropic credentials; export ANTHROPIC_AUTH_TOKEN or ANTHROPIC_API_KEY before calling the Anthropic API`. **Reproduction on `main` HEAD `ff1df4c`:** `unset ANTHROPIC_API_KEY ANTHROPIC_AUTH_TOKEN; export OPENAI_API_KEY=sk-...; export OPENAI_BASE_URL=https://api.openai.com/v1; claw --model openai/gpt-4 prompt 'test'` → reproduces the error deterministically. **Root cause (traced).** `rust/crates/rusty-claude-cli/src/main.rs` at `build_runtime_with_plugin_state` (line ~6221) unconditionally builds `AnthropicRuntimeClient::new(session_id, model, ...)` without consulting `providers::detect_provider_kind(&model)`. `BuiltRuntime` at line ~2855 is statically typed as `ConversationRuntime`, so even if the dispatch logic existed there would be nowhere to slot an alternative client. `providers/mod.rs::metadata_for_model` correctly identifies `openai/gpt-4` as `ProviderKind::OpenAi` at the metadata layer — the routing decision is *computed* correctly, it's just *never used* to pick a runtime client. The result is that the CLI is structurally single-provider (Anthropic only) even though the `api` crate's `openai_compat.rs`, `XAI_ENV_VARS`, `DASHSCOPE_ENV_VARS`, and `send_message_streaming` all exist and are exercised by unit tests inside the `api` crate. The provider matrix in `rust/README.md` is misleading because it describes the api-crate capabilities, not the CLI's actual dispatch behaviour. **Why #28 didn't catch this.** ROADMAP #28 focused on the `MissingCredentials` error *message* (adding hints when adjacent provider env vars are set, or when a bearer token starts with `sk-ant-*`). None of its tests exercised the `build_runtime` code path — they were all unit tests against `ApiError::fmt` output. The routing bug survives #28 because the `Display` improvements fire AFTER the hardcoded Anthropic client has already been constructed and failed. You need the CLI to dispatch to a different client in the first place for the new hints to even surface at the right moment. **Action (single focused commit).** (1) New `OpenAiCompatRuntimeClient` struct in `rust/crates/rusty-claude-cli/src/main.rs` mirroring `AnthropicRuntimeClient` but delegating to `openai_compat::send_message_streaming`. One client type handles OpenAI, xAI, DashScope, and any OpenAI-compat endpoint — they differ only in base URL and auth env var, both of which come from the `ProviderMetadata` returned by `metadata_for_model`. (2) New enum `DynamicApiClient { Anthropic(AnthropicRuntimeClient), OpenAiCompat(OpenAiCompatRuntimeClient) }` that implements `runtime::ApiClient` by matching on the variant and delegating. (3) Retype `BuiltRuntime` from `ConversationRuntime` to `ConversationRuntime`, update the Deref/DerefMut/new spots. (4) In `build_runtime_with_plugin_state`, call `detect_provider_kind(&model)` and construct either variant of `DynamicApiClient`. Prefix routing wins over env-var presence (that's the whole point). (5) Integration test using a mock OpenAI-compat server (reuse `mock_parity_harness` pattern from `crates/api/tests/`) that feeds `claw --model openai/gpt-4 prompt 'test'` with `OPENAI_BASE_URL` pointed at the mock and no `ANTHROPIC_*` env vars, asserts the request reaches the mock, and asserts the response round-trips as an `AssistantEvent`. (6) Unit test that `build_runtime_with_plugin_state` with `model="openai/gpt-4"` returns a `BuiltRuntime` whose inner client is the `DynamicApiClient::OpenAiCompat` variant. **Verification.** `cargo test --workspace`, `cargo fmt --all`, `cargo clippy --workspace`. **Source.** Live users nicma (`1491342350960562277`) and Jengro (`1491345009021030533`) in #claw-code on 2026-04-08, within hours of #28 landing. 41. **Phantom completions root cause: global session store has no per-worktree isolation** — @@ -385,3 +404,87 @@ to: - a **claw-native execution runtime** - an **event-native orchestration substrate** - a **plugin/hook-first autonomous coding harness** + +## Deployment Architecture Gap (filed from dogfood 2026-04-08) + +### WorkerState is in the runtime; /state is NOT in opencode serve + +**Root cause discovered during batch 8 dogfood.** + +`worker_boot.rs` has a solid `WorkerStatus` state machine (`Spawning → TrustRequired → ReadyForPrompt → Running → Finished/Failed`). It is exported from `runtime/src/lib.rs` as a public API. But claw-code is a **plugin** loaded inside the `opencode` binary — it cannot add HTTP routes to `opencode serve`. The HTTP server is 100% owned by the upstream opencode process (v1.3.15). + +**Impact:** There is no way to `curl localhost:4710/state` and get back a JSON `WorkerStatus`. Any such endpoint would require either: +1. Upstreaming a `/state` route into opencode's HTTP server (requires a PR to sst/opencode), or +2. Writing a sidecar HTTP process that queries the `WorkerRegistry` in-process (possible but fragile), or +3. Writing `WorkerStatus` to a well-known file path (`.claw/worker-state.json`) that an external observer can poll. + +**Recommended path:** Option 3 — emit `WorkerStatus` transitions to `.claw/worker-state.json` on every state change. This is purely within claw-code's plugin scope, requires no upstream changes, and gives clawhip a file it can poll to distinguish a truly stalled worker from a quiet-but-progressing one. + +**Action item:** Wire `WorkerRegistry::transition()` to atomically write `.claw/worker-state.json` on every state transition. Add a `claw state` CLI subcommand that reads and prints this file. Add regression test. + +**Prior session note:** A previous session summary claimed commit `0984cca` landed a `/state` HTTP endpoint via axum. This was incorrect — no such commit exists on main, axum is not a dependency, and the HTTP server is not ours. The actual work that exists: `worker_boot.rs` with `WorkerStatus` enum + `WorkerRegistry`, fully wired into `runtime/src/lib.rs` as public exports. + +## Startup Friction Gap: No Default trusted_roots in Settings (filed 2026-04-08) + +### Every lane starts with manual trust babysitting unless caller explicitly passes roots + +**Root cause discovered during direct dogfood of WorkerCreate tool.** + +`WorkerCreate` accepts a `trusted_roots: Vec` parameter. If the caller omits it (or passes `[]`), every new worker immediately enters `TrustRequired` and stalls — requiring manual intervention to advance to `ReadyForPrompt`. There is no mechanism to configure a default allowlist in `settings.json` or `.claw/settings.json`. + +**Impact:** Batch tooling (clawhip, lane orchestrators) must pass `trusted_roots` explicitly on every `WorkerCreate` call. If a batch script forgets the field, all workers in that batch stall silently at `trust_required`. This was the root cause of several "batch 8 lanes not advancing" incidents. + +**Recommended fix:** +1. Add a `trusted_roots` field to `RuntimeConfig` (or a nested `[trust]` table), loaded via `ConfigLoader`. +2. In `WorkerRegistry::spawn_worker()`, merge config-level `trusted_roots` with any per-call overrides. +3. Default: empty list (safest). Users opt in by adding their repo paths to settings. +4. Update `config_validate` schema with the new field. + +**Action item:** Wire `RuntimeConfig::trusted_roots()` → `WorkerRegistry::spawn_worker()` default. Cover with test: config with `trusted_roots = ["/tmp"]` → spawning worker in `/tmp/x` auto-resolves trust without caller passing the field. + +## Observability Transport Decision (filed 2026-04-08) + +### Canonical state surface: CLI/file-based. HTTP endpoint deferred. + +**Decision:** `claw state` reading `.claw/worker-state.json` is the **blessed observability contract** for clawhip and downstream tooling. This is not a stepping-stone — it is the supported surface. Build against it. + +**Rationale:** +- claw-code is a plugin running inside the opencode binary. It cannot add HTTP routes to `opencode serve` — that server belongs to upstream sst/opencode. +- The file-based surface is fully within plugin scope: `emit_state_file()` in `worker_boot.rs` writes atomically on every `WorkerStatus` transition. +- `claw state --output-format json` gives clawhip everything it needs: `status`, `is_ready`, `seconds_since_update`, `trust_gate_cleared`, `last_event`, `updated_at`. +- Polling a local file has lower latency and fewer failure modes than an HTTP round-trip to a sidecar. +- An HTTP state endpoint would require either (a) upstreaming a route to sst/opencode — a multi-week PR cycle with no guarantee of acceptance — or (b) a sidecar process that queries `WorkerRegistry` in-process, which is fragile and adds an extra failure domain. + +**What downstream tooling (clawhip) should do:** +1. After `WorkerCreate`, poll `.claw/worker-state.json` (or run `claw state --output-format json`) in the worker's CWD at whatever interval makes sense (e.g. 5s). +2. Trust `seconds_since_update > 60` in `trust_required` status as the stall signal. +3. Call `WorkerResolveTrust` tool to unblock, or `WorkerRestart` to reset. + +**HTTP endpoint tracking:** Not scheduled. If a concrete use case emerges that file polling cannot serve (e.g. remote workers over a network boundary), open a new issue to upstream a `/worker/state` route to sst/opencode at that time. Until then: file/CLI is canonical. + +## Provider Routing: Model-Name Prefix Must Win Over Env-Var Presence (fixed 2026-04-08, `0530c50`) + +### `openai/gpt-4.1-mini` was silently misrouted to Anthropic when ANTHROPIC_API_KEY was set + +**Root cause:** `metadata_for_model` returned `None` for any model not matching `claude` or `grok` prefix. +`detect_provider_kind` then fell through to auth-sniffer order: first `has_auth_from_env_or_saved()` (Anthropic), then `OPENAI_API_KEY`, then `XAI_API_KEY`. + +If `ANTHROPIC_API_KEY` was present in the environment (e.g. user has both Anthropic and OpenRouter configured), any unknown model — including explicitly namespaced ones like `openai/gpt-4.1-mini` — was silently routed to the Anthropic client, which then failed with `missing Anthropic credentials` or a confusing 402/auth error rather than routing to OpenAI-compatible. + +**Fix:** Added explicit prefix checks in `metadata_for_model`: +- `openai/` prefix → `ProviderKind::OpenAi` +- `gpt-` prefix → `ProviderKind::OpenAi` + +Model name prefix now wins unconditionally over env-var presence. Regression test locked in: `providers::tests::openai_namespaced_model_routes_to_openai_not_anthropic`. + +**Lesson:** Auth-sniffer fallback order is fragile. Any new provider added in the future should be registered in `metadata_for_model` via a model-name prefix, not left to env-var order. This is the canonical extension point. + +30. **DashScope model routing in ProviderClient dispatch uses wrong config** — **done at `adcea6b` on 2026-04-08**. `ProviderClient::from_model_with_anthropic_auth` dispatched all `ProviderKind::OpenAi` matches to `OpenAiCompatConfig::openai()` (reads `OPENAI_API_KEY`, points at `api.openai.com`). But DashScope models (`qwen-plus`, `qwen/qwen-max`) return `ProviderKind::OpenAi` because DashScope speaks the OpenAI wire format — they need `OpenAiCompatConfig::dashscope()` (reads `DASHSCOPE_API_KEY`, points at `dashscope.aliyuncs.com/compatible-mode/v1`). Fix: consult `metadata_for_model` in the `OpenAi` dispatch arm and pick `dashscope()` vs `openai()` based on `metadata.auth_env`. Adds regression test + `pub base_url()` accessor. 2 files, +94/−3. Authored by droid (Kimi K2.5 Turbo) via acpx, cleaned up by Jobdori. + +31. **`code-on-disk → verified commit lands` depends on undocumented executor quirks** — dogfooded 2026-04-08 during live fix session. Three hidden contracts tripped the "last mile" path when using droid via acpx in the claw-code workspace: **(a) hidden CWD contract** — droid's `terminal/create` rejects `cd /path && cargo build` compound commands with `spawn ENOENT`; callers must pass `--cwd` or split commands; **(b) hidden commit-message transport limit** — embedding a multi-line commit message in a single shell invocation hits `ENAMETOOLONG`; workaround is `git commit -F ` but the caller must know to write the file first; **(c) hidden workspace lint/edition contract** — `unsafe_code = "forbid"` workspace-wide with Rust 2021 edition makes `unsafe {}` wrappers incorrect for `set_var`/`remove_var`, but droid generates Rust 2024-style unsafe blocks without inspecting the workspace Cargo.toml or clippy config. Each of these required the orchestrator to learn the constraint by failing, then switching strategies. **Acceptance bar:** a fresh agent should be able to verify/commit/push a correct diff in this workspace without needing to know executor-specific shell trivia ahead of time. **Fix shape:** (1) `run-acpx.sh`-style wrapper that normalizes the commit idiom (always writes to temp file, sets `--cwd`, splits compound commands); (2) inject workspace constraints into the droid/acpx task preamble (edition, lint gates, known shell executor quirks) so the model doesn't have to discover them from failures; (3) or upstream a fix to the executor itself so `cd /path && cmd` chains work correctly. + +32. **OpenAI-compatible provider/model-id passthrough is not fully literal** — dogfooded 2026-04-08 via live user in #claw-code who confirmed the exact backend model id works outside claw but fails through claw for an OpenAI-compatible endpoint. The gap: `openai/` prefix is correctly used for **transport selection** (pick the OpenAI-compat client) but the **wire model id** — the string placed in `"model": "..."` in the JSON request body — may not be the literal backend model string the user supplied. Two candidate failure modes: **(a)** `resolve_model_alias()` is called on the model string before it reaches the wire — alias expansion designed for Anthropic/known models corrupts a user-supplied backend-specific id; **(b)** the `openai/` routing prefix may not be stripped before `build_chat_completion_request` packages the body, so backends receive `openai/gpt-4` instead of `gpt-4`. **Fix shape:** cleanly separate transport selection from wire model id. Transport selection uses the prefix; wire model id is the user-supplied string minus only the routing prefix — no alias expansion, no prefix leakage. **Trace path for next session:** (1) find where `resolve_model_alias()` is called relative to the OpenAI-compat dispatch path; (2) inspect what `build_chat_completion_request` puts in `"model"` for an `openai/some-backend-id` input. **Source:** live user in #claw-code 2026-04-08, confirmed exact model id works outside claw, fails through claw for OpenAI-compat backend. + +32. **OpenAI-compatible provider/model-id passthrough is not fully literal** — dogfooded 2026-04-08 via live user in #claw-code who confirmed the exact backend model id works outside claw but fails through claw for an OpenAI-compatible endpoint. The gap: `openai/` prefix is correctly used for **transport selection** (pick the OpenAI-compat client) but the **wire model id** — the string placed in `"model": "..."` in the JSON request body — may not be the literal backend model string the user supplied. Two candidate failure modes: **(a)** `resolve_model_alias()` is called on the model string before it reaches the wire — alias expansion designed for Anthropic/known models corrupts a user-supplied backend-specific id; **(b)** the `openai/` routing prefix may not be stripped before `build_chat_completion_request` packages the body, so backends receive `openai/gpt-4` instead of `gpt-4`. **Fix shape:** cleanly separate transport selection from wire model id. Transport selection uses the prefix; wire model id is the user-supplied string minus only the routing prefix — no alias expansion, no prefix leakage. **Trace path for next session:** (1) find where `resolve_model_alias()` is called relative to the OpenAI-compat dispatch path; (2) inspect what `build_chat_completion_request` puts in `"model"` for an `openai/some-backend-id` input. **Source:** live user in #claw-code 2026-04-08, confirmed exact model id works outside claw, fails through claw for OpenAI-compat backend. + +32. **OpenAI-compatible provider/model-id passthrough is not fully literal** — dogfooded 2026-04-08 via live user in #claw-code who confirmed the exact backend model id works outside claw but fails through claw for an OpenAI-compatible endpoint. The gap: `openai/` prefix is correctly used for **transport selection** (pick the OpenAI-compat client) but the **wire model id** — the string placed in `"model": "..."` in the JSON request body — may not be the literal backend model string the user supplied. Two candidate failure modes: **(a)** `resolve_model_alias()` is called on the model string before it reaches the wire — alias expansion designed for Anthropic/known models corrupts a user-supplied backend-specific id; **(b)** the `openai/` routing prefix may not be stripped before `build_chat_completion_request` packages the body, so backends receive `openai/gpt-4` instead of `gpt-4`. **Fix shape:** cleanly separate transport selection from wire model id. Transport selection uses the prefix; wire model id is the user-supplied string minus only the routing prefix — no alias expansion, no prefix leakage. **Trace path for next session:** (1) find where `resolve_model_alias()` is called relative to the OpenAI-compat dispatch path; (2) inspect what `build_chat_completion_request` puts in `"model"` for an `openai/some-backend-id` input. **Source:** live user in #claw-code 2026-04-08, confirmed exact model id works outside claw, fails through claw for OpenAI-compat backend. diff --git a/USAGE.md b/USAGE.md index f0220758..d68c0b54 100644 --- a/USAGE.md +++ b/USAGE.md @@ -109,6 +109,20 @@ cd rust ./target/debug/claw logout ``` +### Which env var goes where + +`claw` accepts two Anthropic credential env vars and they are **not interchangeable** — the HTTP header Anthropic expects differs per credential shape. Putting the wrong value in the wrong slot is the most common 401 we see. + +| Credential shape | Env var | HTTP header | Typical source | +|---|---|---|---| +| `sk-ant-*` API key | `ANTHROPIC_API_KEY` | `x-api-key: sk-ant-...` | [console.anthropic.com](https://console.anthropic.com) | +| OAuth access token (opaque) | `ANTHROPIC_AUTH_TOKEN` | `Authorization: Bearer ...` | `claw login` or an Anthropic-compatible proxy that mints Bearer tokens | +| OpenRouter key (`sk-or-v1-*`) | `OPENAI_API_KEY` + `OPENAI_BASE_URL=https://openrouter.ai/api/v1` | `Authorization: Bearer ...` | [openrouter.ai/keys](https://openrouter.ai/keys) | + +**Why this matters:** if you paste an `sk-ant-*` key into `ANTHROPIC_AUTH_TOKEN`, Anthropic's API will return `401 Invalid bearer token` because `sk-ant-*` keys are rejected over the Bearer header. The fix is a one-line env var swap — move the key to `ANTHROPIC_API_KEY`. Recent `claw` builds detect this exact shape (401 + `sk-ant-*` in the Bearer slot) and append a hint to the error message pointing at the fix. + +**If you meant a different provider:** if `claw` reports missing Anthropic credentials but you already have `OPENAI_API_KEY`, `XAI_API_KEY`, or `DASHSCOPE_API_KEY` exported, you most likely forgot to prefix the model name with the provider's routing prefix. Use `--model openai/gpt-4.1-mini` (OpenAI-compat / OpenRouter / Ollama), `--model grok` (xAI), or `--model qwen-plus` (DashScope) and the prefix router will select the right backend regardless of the ambient credentials. The error message now includes a hint that names the detected env var. + ## Local Models `claw` can talk to local servers and provider gateways through either Anthropic-compatible or OpenAI-compatible endpoints. Use `ANTHROPIC_BASE_URL` with `ANTHROPIC_AUTH_TOKEN` for Anthropic-compatible services, or `OPENAI_BASE_URL` with `OPENAI_API_KEY` for OpenAI-compatible services. OAuth is Anthropic-only, so when `OPENAI_BASE_URL` is set you should use API-key style auth instead of `claw login`. @@ -153,6 +167,23 @@ cd rust ./target/debug/claw --model "openai/gpt-4.1-mini" prompt "summarize this repository in one sentence" ``` +### Alibaba DashScope (Qwen) + +For Qwen models via Alibaba's native DashScope API (higher rate limits than OpenRouter): + +```bash +export DASHSCOPE_API_KEY="sk-..." + +cd rust +./target/debug/claw --model "qwen/qwen-max" prompt "hello" +# or bare: +./target/debug/claw --model "qwen-plus" prompt "hello" +``` + +Model names starting with `qwen/` or `qwen-` are automatically routed to the DashScope compatible-mode endpoint (`https://dashscope.aliyuncs.com/compatible-mode/v1`). You do **not** need to set `OPENAI_BASE_URL` or unset `ANTHROPIC_API_KEY` — the model prefix wins over the ambient credential sniffer. + +Reasoning variants (`qwen-qwq-*`, `qwq-*`, `*-thinking`) automatically strip `temperature`/`top_p`/`frequency_penalty`/`presence_penalty` before the request hits the wire (these params are rejected by reasoning models). + ## Supported Providers & Models `claw` has three built-in provider backends. The provider is selected automatically based on the model name, falling back to whichever credential is present in the environment. @@ -164,9 +195,12 @@ cd rust | **Anthropic** (direct) | Anthropic Messages API | `ANTHROPIC_API_KEY` or `ANTHROPIC_AUTH_TOKEN` or OAuth (`claw login`) | `ANTHROPIC_BASE_URL` | `https://api.anthropic.com` | | **xAI** | OpenAI-compatible | `XAI_API_KEY` | `XAI_BASE_URL` | `https://api.x.ai/v1` | | **OpenAI-compatible** | OpenAI Chat Completions | `OPENAI_API_KEY` | `OPENAI_BASE_URL` | `https://api.openai.com/v1` | +| **DashScope** (Alibaba) | OpenAI-compatible | `DASHSCOPE_API_KEY` | `DASHSCOPE_BASE_URL` | `https://dashscope.aliyuncs.com/compatible-mode/v1` | The OpenAI-compatible backend also serves as the gateway for **OpenRouter**, **Ollama**, and any other service that speaks the OpenAI `/v1/chat/completions` wire format — just point `OPENAI_BASE_URL` at the service. +**Model-name prefix routing:** If a model name starts with `openai/`, `gpt-`, `qwen/`, or `qwen-`, the provider is selected by the prefix regardless of which env vars are set. This prevents accidental misrouting to Anthropic when multiple credentials exist in the environment. + ### Tested models and aliases These are the models registered in the built-in alias table with known token limits: diff --git a/rust/crates/api/src/client.rs b/rust/crates/api/src/client.rs index 8dae1d23..59ccdafb 100644 --- a/rust/crates/api/src/client.rs +++ b/rust/crates/api/src/client.rs @@ -31,9 +31,18 @@ impl ProviderClient { ProviderKind::Xai => Ok(Self::Xai(OpenAiCompatClient::from_env( OpenAiCompatConfig::xai(), )?)), - ProviderKind::OpenAi => Ok(Self::OpenAi(OpenAiCompatClient::from_env( - OpenAiCompatConfig::openai(), - )?)), + ProviderKind::OpenAi => { + // DashScope models (qwen-*) also return ProviderKind::OpenAi because they + // speak the OpenAI wire format, but they need the DashScope config which + // reads DASHSCOPE_API_KEY and points at dashscope.aliyuncs.com. + let config = match providers::metadata_for_model(&resolved_model) { + Some(meta) if meta.auth_env == "DASHSCOPE_API_KEY" => { + OpenAiCompatConfig::dashscope() + } + _ => OpenAiCompatConfig::openai(), + }; + Ok(Self::OpenAi(OpenAiCompatClient::from_env(config)?)) + } } } @@ -135,8 +144,21 @@ pub fn read_xai_base_url() -> String { #[cfg(test)] mod tests { + use std::sync::{Mutex, OnceLock}; + + use super::ProviderClient; use crate::providers::{detect_provider_kind, resolve_model_alias, ProviderKind}; + /// Serializes every test in this module that mutates process-wide + /// environment variables so concurrent test threads cannot observe + /// each other's partially-applied state. + fn env_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + } + #[test] fn resolves_existing_and_grok_aliases() { assert_eq!(resolve_model_alias("opus"), "claude-opus-4-6"); @@ -152,4 +174,68 @@ mod tests { ProviderKind::Anthropic ); } + + /// Snapshot-restore guard for a single environment variable. Mirrors + /// the pattern used in `providers/mod.rs` tests: captures the original + /// value on construction, applies the override, and restores on drop so + /// tests leave the process env untouched even when they panic. + struct EnvVarGuard { + key: &'static str, + original: Option, + } + + impl EnvVarGuard { + fn set(key: &'static str, value: Option<&str>) -> Self { + let original = std::env::var_os(key); + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + Self { key, original } + } + } + + impl Drop for EnvVarGuard { + fn drop(&mut self) { + match self.original.take() { + Some(value) => std::env::set_var(self.key, value), + None => std::env::remove_var(self.key), + } + } + } + + #[test] + fn dashscope_model_uses_dashscope_config_not_openai() { + // Regression: qwen-plus was being routed to OpenAiCompatConfig::openai() + // which reads OPENAI_API_KEY and points at api.openai.com, when it should + // use OpenAiCompatConfig::dashscope() which reads DASHSCOPE_API_KEY and + // points at dashscope.aliyuncs.com. + let _lock = env_lock(); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", Some("test-dashscope-key")); + let _openai = EnvVarGuard::set("OPENAI_API_KEY", None); + + let client = ProviderClient::from_model("qwen-plus"); + + // Must succeed (not fail with "missing OPENAI_API_KEY") + assert!( + client.is_ok(), + "qwen-plus with DASHSCOPE_API_KEY set should build successfully, got: {:?}", + client.err() + ); + + // Verify it's the OpenAi variant pointed at the DashScope base URL. + match client.unwrap() { + ProviderClient::OpenAi(openai_client) => { + assert!( + openai_client.base_url().contains("dashscope.aliyuncs.com"), + "qwen-plus should route to DashScope base URL (contains 'dashscope.aliyuncs.com'), got: {}", + openai_client.base_url() + ); + } + other => panic!( + "Expected ProviderClient::OpenAi for qwen-plus, got: {:?}", + other + ), + } + } } diff --git a/rust/crates/api/src/error.rs b/rust/crates/api/src/error.rs index 2aa84d94..3fa49956 100644 --- a/rust/crates/api/src/error.rs +++ b/rust/crates/api/src/error.rs @@ -22,6 +22,11 @@ pub enum ApiError { MissingCredentials { provider: &'static str, env_vars: &'static [&'static str], + /// Optional, runtime-computed hint appended to the error Display + /// output. Populated when the provider resolver can infer what the + /// user probably intended (e.g. an OpenAI key is set but Anthropic + /// was selected because no Anthropic credentials exist). + hint: Option, }, ContextWindowExceeded { model: String, @@ -66,7 +71,29 @@ impl ApiError { provider: &'static str, env_vars: &'static [&'static str], ) -> Self { - Self::MissingCredentials { provider, env_vars } + Self::MissingCredentials { + provider, + env_vars, + hint: None, + } + } + + /// Build a `MissingCredentials` error carrying an extra, runtime-computed + /// hint string that the Display impl appends after the canonical "missing + /// credentials" message. Used by the provider resolver to + /// suggest the likely fix when the user has credentials for a different + /// provider already in the environment. + #[must_use] + pub fn missing_credentials_with_hint( + provider: &'static str, + env_vars: &'static [&'static str], + hint: impl Into, + ) -> Self { + Self::MissingCredentials { + provider, + env_vars, + hint: Some(hint.into()), + } } /// Build a `Self::Json` enriched with the provider name, the model that @@ -204,7 +231,11 @@ impl ApiError { impl Display for ApiError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - Self::MissingCredentials { provider, env_vars } => { + Self::MissingCredentials { + provider, + env_vars, + hint, + } => { write!( f, "missing {provider} credentials; export {} before calling the {provider} API", @@ -223,6 +254,9 @@ impl Display for ApiError { )?; } } + if let Some(hint) = hint { + write!(f, " — hint: {hint}")?; + } Ok(()) } Self::ContextWindowExceeded { @@ -483,4 +517,56 @@ mod tests { assert_eq!(error.safe_failure_class(), "context_window"); assert_eq!(error.request_id(), Some("req_ctx_123")); } + + #[test] + fn missing_credentials_without_hint_renders_the_canonical_message() { + // given + let error = ApiError::missing_credentials( + "Anthropic", + &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"], + ); + + // when + let rendered = error.to_string(); + + // then + assert!( + rendered.starts_with( + "missing Anthropic credentials; export ANTHROPIC_AUTH_TOKEN or ANTHROPIC_API_KEY before calling the Anthropic API" + ), + "rendered error should lead with the canonical missing-credential message: {rendered}" + ); + assert!( + !rendered.contains(" — hint: "), + "no hint should be appended when none is supplied: {rendered}" + ); + } + + #[test] + fn missing_credentials_with_hint_appends_the_hint_after_base_message() { + // given + let error = ApiError::missing_credentials_with_hint( + "Anthropic", + &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"], + "I see OPENAI_API_KEY is set — if you meant to use the OpenAI-compat provider, prefix your model name with `openai/` so prefix routing selects it.", + ); + + // when + let rendered = error.to_string(); + + // then + assert!( + rendered.starts_with("missing Anthropic credentials;"), + "hint should be appended, not replace the base message: {rendered}" + ); + let hint_marker = " — hint: I see OPENAI_API_KEY is set — if you meant to use the OpenAI-compat provider, prefix your model name with `openai/` so prefix routing selects it."; + assert!( + rendered.ends_with(hint_marker), + "rendered error should end with the hint: {rendered}" + ); + // Classification semantics are unaffected by the presence of a hint. + assert_eq!(error.safe_failure_class(), "provider_auth"); + assert!(!error.is_retryable()); + assert_eq!(error.request_id(), None); + } } diff --git a/rust/crates/api/src/prompt_cache.rs b/rust/crates/api/src/prompt_cache.rs index ec9b0b40..0ee8663c 100644 --- a/rust/crates/api/src/prompt_cache.rs +++ b/rust/crates/api/src/prompt_cache.rs @@ -704,6 +704,7 @@ mod tests { tools: None, tool_choice: None, stream: false, + ..Default::default() } } diff --git a/rust/crates/api/src/providers/anthropic.rs b/rust/crates/api/src/providers/anthropic.rs index 31801690..6e62b7d9 100644 --- a/rust/crates/api/src/providers/anthropic.rs +++ b/rust/crates/api/src/providers/anthropic.rs @@ -16,7 +16,9 @@ use crate::error::ApiError; use crate::http_client::build_http_client_or_default; use crate::prompt_cache::{PromptCache, PromptCacheRecord, PromptCacheStats}; -use super::{model_token_limit, resolve_model_alias, Provider, ProviderFuture}; +use super::{ + anthropic_missing_credentials, model_token_limit, resolve_model_alias, Provider, ProviderFuture, +}; use crate::sse::SseParser; use crate::types::{MessageDeltaEvent, MessageRequest, MessageResponse, StreamEvent, Usage}; @@ -49,10 +51,7 @@ impl AuthSource { }), (Some(api_key), None) => Ok(Self::ApiKey(api_key)), (None, Some(bearer_token)) => Ok(Self::BearerToken(bearer_token)), - (None, None) => Err(ApiError::missing_credentials( - "Anthropic", - &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"], - )), + (None, None) => Err(anthropic_missing_credentials()), } } @@ -436,6 +435,7 @@ impl AnthropicClient { last_error = Some(error); } Err(error) => { + let error = enrich_bearer_auth_error(error, &self.auth); self.record_request_failure(attempts, &error); return Err(error); } @@ -487,10 +487,21 @@ impl AnthropicClient { } async fn preflight_message_request(&self, request: &MessageRequest) -> Result<(), ApiError> { + // Always run the local byte-estimate guard first. This catches + // oversized requests even if the remote count_tokens endpoint is + // unreachable, misconfigured, or unimplemented (e.g., third-party + // Anthropic-compatible gateways). If byte estimation already flags + // the request as oversized, reject immediately without a network + // round trip. + super::preflight_message_request(request)?; + let Some(limit) = model_token_limit(&request.model) else { return Ok(()); }; + // Best-effort refinement using the Anthropic count_tokens endpoint. + // On any failure (network, parse, auth), fall back to the local + // byte-estimate result which already passed above. let counted_input_tokens = match self.count_tokens(request).await { Ok(count) => count, Err(_) => return Ok(()), @@ -515,7 +526,10 @@ impl AnthropicClient { input_tokens: u32, } - let request_url = format!("{}/v1/messages/count_tokens", self.base_url.trim_end_matches('/')); + let request_url = format!( + "{}/v1/messages/count_tokens", + self.base_url.trim_end_matches('/') + ); let mut request_body = self.request_profile.render_json_body(request)?; strip_unsupported_beta_body_fields(&mut request_body); let response = self @@ -528,12 +542,7 @@ impl AnthropicClient { let response = expect_success(response).await?; let body = response.text().await.map_err(ApiError::from)?; let parsed = serde_json::from_str::(&body).map_err(|error| { - ApiError::json_deserialize( - "Anthropic count_tokens", - &request.model, - &body, - error, - ) + ApiError::json_deserialize("Anthropic count_tokens", &request.model, &body, error) })?; Ok(parsed.input_tokens) } @@ -597,7 +606,9 @@ fn jitter_for_base(base: Duration) -> Duration { let tick = JITTER_COUNTER.fetch_add(1, Ordering::Relaxed); // splitmix64 finalizer — mixes the low bits so large bases still see // jitter across their full range instead of being clamped to subsec nanos. - let mut mixed = raw_nanos.wrapping_add(tick).wrapping_add(0x9E37_79B9_7F4A_7C15); + let mut mixed = raw_nanos + .wrapping_add(tick) + .wrapping_add(0x9E37_79B9_7F4A_7C15); mixed = (mixed ^ (mixed >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9); mixed = (mixed ^ (mixed >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB); mixed ^= mixed >> 31; @@ -632,10 +643,7 @@ impl AuthSource { } } Ok(Some(token_set)) => Ok(Self::BearerToken(token_set.access_token)), - Ok(None) => Err(ApiError::missing_credentials( - "Anthropic", - &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"], - )), + Ok(None) => Err(anthropic_missing_credentials()), Err(error) => Err(error), } } @@ -679,10 +687,7 @@ where } let Some(token_set) = load_saved_oauth_token()? else { - return Err(ApiError::missing_credentials( - "Anthropic", - &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"], - )); + return Err(anthropic_missing_credentials()); }; if !oauth_token_is_expired(&token_set) { return Ok(AuthSource::BearerToken(token_set.access_token)); @@ -779,10 +784,7 @@ fn read_api_key() -> Result { auth.api_key() .or_else(|| auth.bearer_token()) .map(ToOwned::to_owned) - .ok_or(ApiError::missing_credentials( - "Anthropic", - &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"], - )) + .ok_or_else(anthropic_missing_credentials) } #[cfg(test)] @@ -923,6 +925,85 @@ const fn is_retryable_status(status: reqwest::StatusCode) -> bool { matches!(status.as_u16(), 408 | 409 | 429 | 500 | 502 | 503 | 504) } +/// Anthropic API keys (`sk-ant-*`) are accepted over the `x-api-key` header +/// and rejected with HTTP 401 "Invalid bearer token" when sent as a Bearer +/// token via `ANTHROPIC_AUTH_TOKEN`. This happens often enough in the wild +/// (users copy-paste an `sk-ant-...` key into `ANTHROPIC_AUTH_TOKEN` because +/// the env var name sounds auth-related) that a bare 401 error is useless. +/// When we detect this exact shape, append a hint to the error message that +/// points the user at the one-line fix. +const SK_ANT_BEARER_HINT: &str = "sk-ant-* keys go in ANTHROPIC_API_KEY (x-api-key header), not ANTHROPIC_AUTH_TOKEN (Bearer header). Move your key to ANTHROPIC_API_KEY."; + +fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError { + let ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + } = error + else { + return error; + }; + if status.as_u16() != 401 { + return ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + }; + } + let Some(bearer_token) = auth.bearer_token() else { + return ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + }; + }; + if !bearer_token.starts_with("sk-ant-") { + return ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + }; + } + // Only append the hint when the AuthSource is pure BearerToken. If both + // api_key and bearer_token are present (`ApiKeyAndBearer`), the x-api-key + // header is already being sent alongside the Bearer header and the 401 + // is coming from a different cause — adding the hint would be misleading. + if auth.api_key().is_some() { + return ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + }; + } + let enriched_message = match message { + Some(existing) => Some(format!("{existing} — hint: {SK_ANT_BEARER_HINT}")), + None => Some(format!("hint: {SK_ANT_BEARER_HINT}")), + }; + ApiError::Api { + status, + error_type, + message: enriched_message, + request_id, + body, + retryable, + } +} + /// Remove beta-only body fields that the standard `/v1/messages` and /// `/v1/messages/count_tokens` endpoints reject as `Extra inputs are not /// permitted`. The `betas` opt-in is communicated via the `anthropic-beta` @@ -930,6 +1011,15 @@ const fn is_retryable_status(status: reqwest::StatusCode) -> bool { fn strip_unsupported_beta_body_fields(body: &mut Value) { if let Some(object) = body.as_object_mut() { object.remove("betas"); + // These fields are OpenAI-compatible only; Anthropic rejects them. + object.remove("frequency_penalty"); + object.remove("presence_penalty"); + // Anthropic uses "stop_sequences" not "stop". Convert if present. + if let Some(stop_val) = object.remove("stop") { + if stop_val.as_array().map_or(false, |a| !a.is_empty()) { + object.insert("stop_sequences".to_string(), stop_val); + } + } } } @@ -1259,6 +1349,7 @@ mod tests { tools: None, tool_choice: None, stream: false, + ..Default::default() }; assert!(request.with_streaming().stream); @@ -1438,6 +1529,52 @@ mod tests { assert_eq!(body, original); } + #[test] + fn strip_removes_openai_only_fields_and_converts_stop() { + let mut body = serde_json::json!({ + "model": "claude-sonnet-4-6", + "max_tokens": 1024, + "temperature": 0.7, + "frequency_penalty": 0.5, + "presence_penalty": 0.3, + "stop": ["\n"], + }); + + super::strip_unsupported_beta_body_fields(&mut body); + + // temperature is kept (Anthropic supports it) + assert_eq!(body["temperature"], serde_json::json!(0.7)); + // frequency_penalty and presence_penalty are removed + assert!( + body.get("frequency_penalty").is_none(), + "frequency_penalty must be stripped for Anthropic" + ); + assert!( + body.get("presence_penalty").is_none(), + "presence_penalty must be stripped for Anthropic" + ); + // stop is renamed to stop_sequences + assert!(body.get("stop").is_none(), "stop must be renamed"); + assert_eq!(body["stop_sequences"], serde_json::json!(["\n"])); + } + + #[test] + fn strip_does_not_add_empty_stop_sequences() { + let mut body = serde_json::json!({ + "model": "claude-sonnet-4-6", + "max_tokens": 1024, + "stop": [], + }); + + super::strip_unsupported_beta_body_fields(&mut body); + + assert!(body.get("stop").is_none()); + assert!( + body.get("stop_sequences").is_none(), + "empty stop should not produce stop_sequences" + ); + } + #[test] fn rendered_request_body_strips_betas_for_standard_messages_endpoint() { let client = AnthropicClient::new("test-key").with_beta("tools-2026-04-01"); @@ -1449,6 +1586,7 @@ mod tests { tools: None, tool_choice: None, stream: false, + ..Default::default() }; let mut rendered = client @@ -1470,4 +1608,163 @@ mod tests { Some("claude-sonnet-4-6") ); } + + #[test] + fn enrich_bearer_auth_error_appends_sk_ant_hint_on_401_with_pure_bearer_token() { + // given + let auth = AuthSource::BearerToken("sk-ant-api03-deadbeef".to_string()); + let error = crate::error::ApiError::Api { + status: reqwest::StatusCode::UNAUTHORIZED, + error_type: Some("authentication_error".to_string()), + message: Some("Invalid bearer token".to_string()), + request_id: Some("req_varleg_001".to_string()), + body: String::new(), + retryable: false, + }; + + // when + let enriched = super::enrich_bearer_auth_error(error, &auth); + + // then + let rendered = enriched.to_string(); + assert!( + rendered.contains("Invalid bearer token"), + "existing provider message should be preserved: {rendered}" + ); + assert!( + rendered.contains( + "sk-ant-* keys go in ANTHROPIC_API_KEY (x-api-key header), not ANTHROPIC_AUTH_TOKEN (Bearer header). Move your key to ANTHROPIC_API_KEY." + ), + "rendered error should include the sk-ant-* hint: {rendered}" + ); + assert!( + rendered.contains("[trace req_varleg_001]"), + "request id should still flow through the enriched error: {rendered}" + ); + match enriched { + crate::error::ApiError::Api { status, .. } => { + assert_eq!(status, reqwest::StatusCode::UNAUTHORIZED); + } + other => panic!("expected Api variant, got {other:?}"), + } + } + + #[test] + fn enrich_bearer_auth_error_leaves_non_401_errors_unchanged() { + // given + let auth = AuthSource::BearerToken("sk-ant-api03-deadbeef".to_string()); + let error = crate::error::ApiError::Api { + status: reqwest::StatusCode::INTERNAL_SERVER_ERROR, + error_type: Some("api_error".to_string()), + message: Some("internal server error".to_string()), + request_id: None, + body: String::new(), + retryable: true, + }; + + // when + let enriched = super::enrich_bearer_auth_error(error, &auth); + + // then + let rendered = enriched.to_string(); + assert!( + !rendered.contains("sk-ant-*"), + "non-401 errors must not be annotated with the bearer hint: {rendered}" + ); + assert!( + rendered.contains("internal server error"), + "original message must be preserved verbatim: {rendered}" + ); + } + + #[test] + fn enrich_bearer_auth_error_ignores_401_when_bearer_token_is_not_sk_ant() { + // given + let auth = AuthSource::BearerToken("oauth-access-token-opaque".to_string()); + let error = crate::error::ApiError::Api { + status: reqwest::StatusCode::UNAUTHORIZED, + error_type: Some("authentication_error".to_string()), + message: Some("Invalid bearer token".to_string()), + request_id: None, + body: String::new(), + retryable: false, + }; + + // when + let enriched = super::enrich_bearer_auth_error(error, &auth); + + // then + let rendered = enriched.to_string(); + assert!( + !rendered.contains("sk-ant-*"), + "oauth-style bearer tokens must not trigger the sk-ant-* hint: {rendered}" + ); + } + + #[test] + fn enrich_bearer_auth_error_skips_hint_when_api_key_header_is_also_present() { + // given + let auth = AuthSource::ApiKeyAndBearer { + api_key: "sk-ant-api03-legitimate".to_string(), + bearer_token: "sk-ant-api03-deadbeef".to_string(), + }; + let error = crate::error::ApiError::Api { + status: reqwest::StatusCode::UNAUTHORIZED, + error_type: Some("authentication_error".to_string()), + message: Some("Invalid bearer token".to_string()), + request_id: None, + body: String::new(), + retryable: false, + }; + + // when + let enriched = super::enrich_bearer_auth_error(error, &auth); + + // then + let rendered = enriched.to_string(); + assert!( + !rendered.contains("sk-ant-*"), + "hint should be suppressed when x-api-key header is already being sent: {rendered}" + ); + } + + #[test] + fn enrich_bearer_auth_error_ignores_401_when_auth_source_has_no_bearer() { + // given + let auth = AuthSource::ApiKey("sk-ant-api03-legitimate".to_string()); + let error = crate::error::ApiError::Api { + status: reqwest::StatusCode::UNAUTHORIZED, + error_type: Some("authentication_error".to_string()), + message: Some("Invalid x-api-key".to_string()), + request_id: None, + body: String::new(), + retryable: false, + }; + + // when + let enriched = super::enrich_bearer_auth_error(error, &auth); + + // then + let rendered = enriched.to_string(); + assert!( + !rendered.contains("sk-ant-*"), + "bearer hint must not apply when AuthSource is ApiKey-only: {rendered}" + ); + } + + #[test] + fn enrich_bearer_auth_error_passes_non_api_errors_through_unchanged() { + // given + let auth = AuthSource::BearerToken("sk-ant-api03-deadbeef".to_string()); + let error = crate::error::ApiError::InvalidSseFrame("unterminated event"); + + // when + let enriched = super::enrich_bearer_auth_error(error, &auth); + + // then + assert!(matches!( + enriched, + crate::error::ApiError::InvalidSseFrame(_) + )); + } } diff --git a/rust/crates/api/src/providers/mod.rs b/rust/crates/api/src/providers/mod.rs index 36c01fd5..e7251617 100644 --- a/rust/crates/api/src/providers/mod.rs +++ b/rust/crates/api/src/providers/mod.rs @@ -169,6 +169,31 @@ pub fn metadata_for_model(model: &str) -> Option { default_base_url: openai_compat::DEFAULT_XAI_BASE_URL, }); } + // Explicit provider-namespaced models (e.g. "openai/gpt-4.1-mini") must + // route to the correct provider regardless of which auth env vars are set. + // Without this, detect_provider_kind falls through to the auth-sniffer + // order and misroutes to Anthropic if ANTHROPIC_API_KEY is present. + if canonical.starts_with("openai/") || canonical.starts_with("gpt-") { + return Some(ProviderMetadata { + provider: ProviderKind::OpenAi, + auth_env: "OPENAI_API_KEY", + base_url_env: "OPENAI_BASE_URL", + default_base_url: openai_compat::DEFAULT_OPENAI_BASE_URL, + }); + } + // Alibaba DashScope compatible-mode endpoint. Routes qwen/* and bare + // qwen-* model names (qwen-max, qwen-plus, qwen-turbo, qwen-qwq, etc.) + // to the OpenAI-compat client pointed at DashScope's /compatible-mode/v1. + // Uses the OpenAi provider kind because DashScope speaks the OpenAI REST + // shape — only the base URL and auth env var differ. + if canonical.starts_with("qwen/") || canonical.starts_with("qwen-") { + return Some(ProviderMetadata { + provider: ProviderKind::OpenAi, + auth_env: "DASHSCOPE_API_KEY", + base_url_env: "DASHSCOPE_BASE_URL", + default_base_url: openai_compat::DEFAULT_DASHSCOPE_BASE_URL, + }); + } None } @@ -266,6 +291,73 @@ fn estimate_serialized_tokens(value: &T) -> u32 { .map_or(0, |bytes| (bytes.len() / 4 + 1) as u32) } +/// Env var names used by other provider backends. When Anthropic auth +/// resolution fails we sniff these so we can hint the user that their +/// credentials probably belong to a different provider and suggest the +/// model-prefix routing fix that would select it. +const FOREIGN_PROVIDER_ENV_VARS: &[(&str, &str, &str)] = &[ + ( + "OPENAI_API_KEY", + "OpenAI-compat", + "prefix your model name with `openai/` (e.g. `--model openai/gpt-4.1-mini`) so prefix routing selects the OpenAI-compatible provider, and set `OPENAI_BASE_URL` if you are pointing at OpenRouter/Ollama/a local server", + ), + ( + "XAI_API_KEY", + "xAI", + "use an xAI model alias (e.g. `--model grok` or `--model grok-mini`) so the prefix router selects the xAI backend", + ), + ( + "DASHSCOPE_API_KEY", + "Alibaba DashScope", + "prefix your model name with `qwen/` or `qwen-` (e.g. `--model qwen-plus`) so prefix routing selects the DashScope backend", + ), +]; + +/// Check whether an env var is set to a non-empty value either in the real +/// process environment or in the working-directory `.env` file. Mirrors the +/// credential discovery path used by `read_env_non_empty` so the hint text +/// stays truthful when users rely on `.env` instead of a real export. +fn env_or_dotenv_present(key: &str) -> bool { + match std::env::var(key) { + Ok(value) if !value.is_empty() => true, + Ok(_) | Err(std::env::VarError::NotPresent) => { + dotenv_value(key).is_some_and(|value| !value.is_empty()) + } + Err(_) => false, + } +} + +/// Produce a hint string describing the first foreign provider credential +/// that is present in the environment when Anthropic auth resolution has +/// just failed. Returns `None` when no foreign credential is set, in which +/// case the caller should fall back to the plain `missing_credentials` +/// error without a hint. +pub(crate) fn anthropic_missing_credentials_hint() -> Option { + for (env_var, provider_label, fix_hint) in FOREIGN_PROVIDER_ENV_VARS { + if env_or_dotenv_present(env_var) { + return Some(format!( + "I see {env_var} is set — if you meant to use the {provider_label} provider, {fix_hint}." + )); + } + } + None +} + +/// Build an Anthropic-specific `MissingCredentials` error, attaching a +/// hint suggesting the probable fix whenever a different provider's +/// credentials are already present in the environment. Anthropic call +/// sites should prefer this helper over `ApiError::missing_credentials` +/// so users who mistyped a model name or forgot the prefix get a useful +/// signal instead of a generic "missing Anthropic credentials" wall. +pub(crate) fn anthropic_missing_credentials() -> ApiError { + const PROVIDER: &str = "Anthropic"; + const ENV_VARS: &[&str] = &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"]; + match anthropic_missing_credentials_hint() { + Some(hint) => ApiError::missing_credentials_with_hint(PROVIDER, ENV_VARS, hint), + None => ApiError::missing_credentials(PROVIDER, ENV_VARS), + } +} + /// Parse a `.env` file body into key/value pairs using a minimal `KEY=VALUE` /// grammar. Lines that are blank, start with `#`, or do not contain `=` are /// ignored. Surrounding double or single quotes are stripped from the value. @@ -323,6 +415,9 @@ pub(crate) fn dotenv_value(key: &str) -> Option { #[cfg(test)] mod tests { + use std::ffi::OsString; + use std::sync::{Mutex, OnceLock}; + use serde_json::json; use crate::error::ApiError; @@ -331,11 +426,52 @@ mod tests { }; use super::{ - detect_provider_kind, load_dotenv_file, max_tokens_for_model, - max_tokens_for_model_with_override, model_token_limit, parse_dotenv, - preflight_message_request, resolve_model_alias, ProviderKind, + anthropic_missing_credentials, anthropic_missing_credentials_hint, detect_provider_kind, + load_dotenv_file, max_tokens_for_model, max_tokens_for_model_with_override, + model_token_limit, parse_dotenv, preflight_message_request, resolve_model_alias, + ProviderKind, }; + /// Serializes every test in this module that mutates process-wide + /// environment variables so concurrent test threads cannot observe + /// each other's partially-applied state while probing the foreign + /// provider credential sniffer. + fn env_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + } + + /// Snapshot-restore guard for a single environment variable. Captures + /// the original value on construction, applies the requested override + /// (set or remove), and restores the original on drop so tests leave + /// the process env untouched even when they panic mid-assertion. + struct EnvVarGuard { + key: &'static str, + original: Option, + } + + impl EnvVarGuard { + fn set(key: &'static str, value: Option<&str>) -> Self { + let original = std::env::var_os(key); + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + Self { key, original } + } + } + + impl Drop for EnvVarGuard { + fn drop(&mut self) { + match self.original.take() { + Some(value) => std::env::set_var(self.key, value), + None => std::env::remove_var(self.key), + } + } + } + #[test] fn resolves_grok_aliases() { assert_eq!(resolve_model_alias("grok"), "grok-3"); @@ -352,6 +488,58 @@ mod tests { ); } + #[test] + fn openai_namespaced_model_routes_to_openai_not_anthropic() { + // Regression: "openai/gpt-4.1-mini" was misrouted to Anthropic when + // ANTHROPIC_API_KEY was set because metadata_for_model returned None + // and detect_provider_kind fell through to auth-sniffer order. + // The model prefix must win over env-var presence. + let kind = super::metadata_for_model("openai/gpt-4.1-mini") + .map(|m| m.provider) + .unwrap_or_else(|| detect_provider_kind("openai/gpt-4.1-mini")); + assert_eq!( + kind, + ProviderKind::OpenAi, + "openai/ prefix must route to OpenAi regardless of ANTHROPIC_API_KEY" + ); + + // Also cover bare gpt- prefix + let kind2 = super::metadata_for_model("gpt-4o") + .map(|m| m.provider) + .unwrap_or_else(|| detect_provider_kind("gpt-4o")); + assert_eq!(kind2, ProviderKind::OpenAi); + } + + #[test] + fn qwen_prefix_routes_to_dashscope_not_anthropic() { + // User request from Discord #clawcode-get-help: web3g wants to use + // Qwen 3.6 Plus via native Alibaba DashScope API (not OpenRouter, + // which has lower rate limits). metadata_for_model must route + // qwen/* and bare qwen-* to the OpenAi provider kind pointed at + // the DashScope compatible-mode endpoint, regardless of whether + // ANTHROPIC_API_KEY is present in the environment. + let meta = super::metadata_for_model("qwen/qwen-max") + .expect("qwen/ prefix must resolve to DashScope metadata"); + assert_eq!(meta.provider, ProviderKind::OpenAi); + assert_eq!(meta.auth_env, "DASHSCOPE_API_KEY"); + assert_eq!(meta.base_url_env, "DASHSCOPE_BASE_URL"); + assert!(meta.default_base_url.contains("dashscope.aliyuncs.com")); + + // Bare qwen- prefix also routes + let meta2 = super::metadata_for_model("qwen-plus") + .expect("qwen- prefix must resolve to DashScope metadata"); + assert_eq!(meta2.provider, ProviderKind::OpenAi); + assert_eq!(meta2.auth_env, "DASHSCOPE_API_KEY"); + + // detect_provider_kind must agree even if ANTHROPIC_API_KEY is set + let kind = detect_provider_kind("qwen/qwen3-coder"); + assert_eq!( + kind, + ProviderKind::OpenAi, + "qwen/ prefix must win over auth-sniffer order" + ); + } + #[test] fn keeps_existing_max_token_heuristic() { assert_eq!(max_tokens_for_model("opus"), 32_000); @@ -446,6 +634,7 @@ mod tests { }]), tool_choice: Some(ToolChoice::Auto), stream: true, + ..Default::default() }; let error = preflight_message_request(&request) @@ -484,6 +673,7 @@ mod tests { tools: None, tool_choice: None, stream: false, + ..Default::default() }; preflight_message_request(&request) @@ -570,4 +760,225 @@ NO_EQUALS_LINE let _ = std::fs::remove_dir_all(&temp_root); } + + #[test] + fn anthropic_missing_credentials_hint_is_none_when_no_foreign_creds_present() { + // given + let _lock = env_lock(); + let _openai = EnvVarGuard::set("OPENAI_API_KEY", None); + let _xai = EnvVarGuard::set("XAI_API_KEY", None); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", None); + + // when + let hint = anthropic_missing_credentials_hint(); + + // then + assert!( + hint.is_none(), + "no hint should be produced when every foreign provider env var is absent, got {hint:?}" + ); + } + + #[test] + fn anthropic_missing_credentials_hint_detects_openai_api_key_and_recommends_openai_prefix() { + // given + let _lock = env_lock(); + let _openai = EnvVarGuard::set("OPENAI_API_KEY", Some("sk-openrouter-varleg")); + let _xai = EnvVarGuard::set("XAI_API_KEY", None); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", None); + + // when + let hint = anthropic_missing_credentials_hint() + .expect("OPENAI_API_KEY presence should produce a hint"); + + // then + assert!( + hint.contains("OPENAI_API_KEY is set"), + "hint should name the detected env var so users recognize it: {hint}" + ); + assert!( + hint.contains("OpenAI-compat"), + "hint should identify the target provider: {hint}" + ); + assert!( + hint.contains("openai/"), + "hint should mention the `openai/` prefix routing fix: {hint}" + ); + assert!( + hint.contains("OPENAI_BASE_URL"), + "hint should mention OPENAI_BASE_URL so OpenRouter users see the full picture: {hint}" + ); + } + + #[test] + fn anthropic_missing_credentials_hint_detects_xai_api_key() { + // given + let _lock = env_lock(); + let _openai = EnvVarGuard::set("OPENAI_API_KEY", None); + let _xai = EnvVarGuard::set("XAI_API_KEY", Some("xai-test-key")); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", None); + + // when + let hint = anthropic_missing_credentials_hint() + .expect("XAI_API_KEY presence should produce a hint"); + + // then + assert!( + hint.contains("XAI_API_KEY is set"), + "hint should name XAI_API_KEY: {hint}" + ); + assert!( + hint.contains("xAI"), + "hint should identify the xAI provider: {hint}" + ); + assert!( + hint.contains("grok"), + "hint should suggest a grok-prefixed model alias: {hint}" + ); + } + + #[test] + fn anthropic_missing_credentials_hint_detects_dashscope_api_key() { + // given + let _lock = env_lock(); + let _openai = EnvVarGuard::set("OPENAI_API_KEY", None); + let _xai = EnvVarGuard::set("XAI_API_KEY", None); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", Some("sk-dashscope-test")); + + // when + let hint = anthropic_missing_credentials_hint() + .expect("DASHSCOPE_API_KEY presence should produce a hint"); + + // then + assert!( + hint.contains("DASHSCOPE_API_KEY is set"), + "hint should name DASHSCOPE_API_KEY: {hint}" + ); + assert!( + hint.contains("DashScope"), + "hint should identify the DashScope provider: {hint}" + ); + assert!( + hint.contains("qwen"), + "hint should suggest a qwen-prefixed model alias: {hint}" + ); + } + + #[test] + fn anthropic_missing_credentials_hint_prefers_openai_when_multiple_foreign_creds_set() { + // given + let _lock = env_lock(); + let _openai = EnvVarGuard::set("OPENAI_API_KEY", Some("sk-openrouter-varleg")); + let _xai = EnvVarGuard::set("XAI_API_KEY", Some("xai-test-key")); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", Some("sk-dashscope-test")); + + // when + let hint = anthropic_missing_credentials_hint() + .expect("multiple foreign creds should still produce a hint"); + + // then + assert!( + hint.contains("OPENAI_API_KEY"), + "OpenAI should be prioritized because it is the most common misrouting pattern (OpenRouter users), got: {hint}" + ); + assert!( + !hint.contains("XAI_API_KEY"), + "only the first detected provider should be named to keep the hint focused, got: {hint}" + ); + } + + #[test] + fn anthropic_missing_credentials_builds_error_with_canonical_env_vars_and_no_hint_when_clean() { + // given + let _lock = env_lock(); + let _openai = EnvVarGuard::set("OPENAI_API_KEY", None); + let _xai = EnvVarGuard::set("XAI_API_KEY", None); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", None); + + // when + let error = anthropic_missing_credentials(); + + // then + match &error { + ApiError::MissingCredentials { + provider, + env_vars, + hint, + } => { + assert_eq!(*provider, "Anthropic"); + assert_eq!(*env_vars, &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"]); + assert!( + hint.is_none(), + "clean environment should not generate a hint, got {hint:?}" + ); + } + other => panic!("expected MissingCredentials variant, got {other:?}"), + } + let rendered = error.to_string(); + assert!( + !rendered.contains(" — hint: "), + "rendered error should be a plain missing-creds message: {rendered}" + ); + } + + #[test] + fn anthropic_missing_credentials_builds_error_with_hint_when_openai_key_is_set() { + // given + let _lock = env_lock(); + let _openai = EnvVarGuard::set("OPENAI_API_KEY", Some("sk-openrouter-varleg")); + let _xai = EnvVarGuard::set("XAI_API_KEY", None); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", None); + + // when + let error = anthropic_missing_credentials(); + + // then + match &error { + ApiError::MissingCredentials { + provider, + env_vars, + hint, + } => { + assert_eq!(*provider, "Anthropic"); + assert_eq!(*env_vars, &["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"]); + let hint_value = hint.as_deref().expect("hint should be populated"); + assert!( + hint_value.contains("OPENAI_API_KEY is set"), + "hint should name the detected env var: {hint_value}" + ); + } + other => panic!("expected MissingCredentials variant, got {other:?}"), + } + let rendered = error.to_string(); + assert!( + rendered.starts_with("missing Anthropic credentials;"), + "canonical base message should still lead the rendered error: {rendered}" + ); + assert!( + rendered.contains(" — hint: I see OPENAI_API_KEY is set"), + "rendered error should carry the env-driven hint: {rendered}" + ); + } + + #[test] + fn anthropic_missing_credentials_hint_ignores_empty_string_values() { + // given + let _lock = env_lock(); + // An empty value is semantically equivalent to "not set" for the + // credential discovery path, so the sniffer must treat it that way + // to avoid false-positive hints for users who intentionally cleared + // a stale export with `OPENAI_API_KEY=`. + let _openai = EnvVarGuard::set("OPENAI_API_KEY", Some("")); + let _xai = EnvVarGuard::set("XAI_API_KEY", None); + let _dashscope = EnvVarGuard::set("DASHSCOPE_API_KEY", None); + + // when + let hint = anthropic_missing_credentials_hint(); + + // then + assert!( + hint.is_none(), + "empty env var should not trigger the hint sniffer, got {hint:?}" + ); + } } diff --git a/rust/crates/api/src/providers/openai_compat.rs b/rust/crates/api/src/providers/openai_compat.rs index 57d9ef22..9ac9c8fd 100644 --- a/rust/crates/api/src/providers/openai_compat.rs +++ b/rust/crates/api/src/providers/openai_compat.rs @@ -18,6 +18,7 @@ use super::{preflight_message_request, Provider, ProviderFuture}; pub const DEFAULT_XAI_BASE_URL: &str = "https://api.x.ai/v1"; pub const DEFAULT_OPENAI_BASE_URL: &str = "https://api.openai.com/v1"; +pub const DEFAULT_DASHSCOPE_BASE_URL: &str = "https://dashscope.aliyuncs.com/compatible-mode/v1"; const REQUEST_ID_HEADER: &str = "request-id"; const ALT_REQUEST_ID_HEADER: &str = "x-request-id"; const DEFAULT_INITIAL_BACKOFF: Duration = Duration::from_secs(1); @@ -34,6 +35,7 @@ pub struct OpenAiCompatConfig { const XAI_ENV_VARS: &[&str] = &["XAI_API_KEY"]; const OPENAI_ENV_VARS: &[&str] = &["OPENAI_API_KEY"]; +const DASHSCOPE_ENV_VARS: &[&str] = &["DASHSCOPE_API_KEY"]; impl OpenAiCompatConfig { #[must_use] @@ -55,11 +57,27 @@ impl OpenAiCompatConfig { default_base_url: DEFAULT_OPENAI_BASE_URL, } } + + /// Alibaba DashScope compatible-mode endpoint (Qwen family models). + /// Uses the OpenAI-compatible REST shape at /compatible-mode/v1. + /// Requested via Discord #clawcode-get-help: native Alibaba API for + /// higher rate limits than going through OpenRouter. + #[must_use] + pub const fn dashscope() -> Self { + Self { + provider_name: "DashScope", + api_key_env: "DASHSCOPE_API_KEY", + base_url_env: "DASHSCOPE_BASE_URL", + default_base_url: DEFAULT_DASHSCOPE_BASE_URL, + } + } + #[must_use] pub fn credential_env_vars(self) -> &'static [&'static str] { match self.provider_name { "xAI" => XAI_ENV_VARS, "OpenAI" => OPENAI_ENV_VARS, + "DashScope" => DASHSCOPE_ENV_VARS, _ => &[], } } @@ -80,6 +98,11 @@ impl OpenAiCompatClient { const fn config(&self) -> OpenAiCompatConfig { self.config } + + #[must_use] + pub fn base_url(&self) -> &str { + &self.base_url + } #[must_use] pub fn new(api_key: impl Into, config: OpenAiCompatConfig) -> Self { Self { @@ -135,12 +158,7 @@ impl OpenAiCompatClient { let request_id = request_id_from_headers(response.headers()); let body = response.text().await.map_err(ApiError::from)?; let payload = serde_json::from_str::(&body).map_err(|error| { - ApiError::json_deserialize( - self.config.provider_name, - &request.model, - &body, - error, - ) + ApiError::json_deserialize(self.config.provider_name, &request.model, &body, error) })?; let mut normalized = normalize_response(&request.model, payload)?; if normalized.request_id.is_none() { @@ -160,10 +178,7 @@ impl OpenAiCompatClient { Ok(MessageStream { request_id: request_id_from_headers(response.headers()), response, - parser: OpenAiSseParser::with_context( - self.config.provider_name, - request.model.clone(), - ), + parser: OpenAiSseParser::with_context(self.config.provider_name, request.model.clone()), pending: VecDeque::new(), done: false, state: StreamState::new(request.model.clone()), @@ -253,7 +268,9 @@ fn jitter_for_base(base: Duration) -> Duration { .map(|elapsed| u64::try_from(elapsed.as_nanos()).unwrap_or(u64::MAX)) .unwrap_or(0); let tick = JITTER_COUNTER.fetch_add(1, Ordering::Relaxed); - let mut mixed = raw_nanos.wrapping_add(tick).wrapping_add(0x9E37_79B9_7F4A_7C15); + let mut mixed = raw_nanos + .wrapping_add(tick) + .wrapping_add(0x9E37_79B9_7F4A_7C15); mixed = (mixed ^ (mixed >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9); mixed = (mixed ^ (mixed >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB); mixed ^= mixed >> 31; @@ -690,6 +707,25 @@ struct ErrorBody { message: Option, } +/// Returns true for models known to reject tuning parameters like temperature, +/// top_p, frequency_penalty, and presence_penalty. These are typically +/// reasoning/chain-of-thought models with fixed sampling. +fn is_reasoning_model(model: &str) -> bool { + let lowered = model.to_ascii_lowercase(); + // Strip any provider/ prefix for the check (e.g. qwen/qwen-qwq -> qwen-qwq) + let canonical = lowered.rsplit('/').next().unwrap_or(lowered.as_str()); + // OpenAI reasoning models + canonical.starts_with("o1") + || canonical.starts_with("o3") + || canonical.starts_with("o4") + // xAI reasoning: grok-3-mini always uses reasoning mode + || canonical == "grok-3-mini" + // Alibaba DashScope reasoning variants (QwQ + Qwen3-Thinking family) + || canonical.starts_with("qwen-qwq") + || canonical.starts_with("qwq") + || canonical.contains("thinking") +} + fn build_chat_completion_request(request: &MessageRequest, config: OpenAiCompatConfig) -> Value { let mut messages = Vec::new(); if let Some(system) = request.system.as_ref().filter(|value| !value.is_empty()) { @@ -721,6 +757,30 @@ fn build_chat_completion_request(request: &MessageRequest, config: OpenAiCompatC payload["tool_choice"] = openai_tool_choice(tool_choice); } + // OpenAI-compatible tuning parameters — only included when explicitly set. + // Reasoning models (o1/o3/o4/grok-3-mini) reject these params with 400; + // silently strip them to avoid cryptic provider errors. + if !is_reasoning_model(&request.model) { + if let Some(temperature) = request.temperature { + payload["temperature"] = json!(temperature); + } + if let Some(top_p) = request.top_p { + payload["top_p"] = json!(top_p); + } + if let Some(frequency_penalty) = request.frequency_penalty { + payload["frequency_penalty"] = json!(frequency_penalty); + } + if let Some(presence_penalty) = request.presence_penalty { + payload["presence_penalty"] = json!(presence_penalty); + } + } + // stop is generally safe for all providers + if let Some(stop) = &request.stop { + if !stop.is_empty() { + payload["stop"] = json!(stop); + } + } + payload } @@ -1009,8 +1069,9 @@ impl StringExt for String { #[cfg(test)] mod tests { use super::{ - build_chat_completion_request, chat_completions_endpoint, normalize_finish_reason, - openai_tool_choice, parse_tool_arguments, OpenAiCompatClient, OpenAiCompatConfig, + build_chat_completion_request, chat_completions_endpoint, is_reasoning_model, + normalize_finish_reason, openai_tool_choice, parse_tool_arguments, OpenAiCompatClient, + OpenAiCompatConfig, }; use crate::error::ApiError; use crate::types::{ @@ -1049,6 +1110,7 @@ mod tests { }]), tool_choice: Some(ToolChoice::Auto), stream: false, + ..Default::default() }, OpenAiCompatConfig::xai(), ); @@ -1071,6 +1133,7 @@ mod tests { tools: None, tool_choice: None, stream: true, + ..Default::default() }, OpenAiCompatConfig::openai(), ); @@ -1089,6 +1152,7 @@ mod tests { tools: None, tool_choice: None, stream: true, + ..Default::default() }, OpenAiCompatConfig::xai(), ); @@ -1159,4 +1223,104 @@ mod tests { assert_eq!(normalize_finish_reason("stop"), "end_turn"); assert_eq!(normalize_finish_reason("tool_calls"), "tool_use"); } + + #[test] + fn tuning_params_included_in_payload_when_set() { + let request = MessageRequest { + model: "gpt-4o".to_string(), + max_tokens: 1024, + messages: vec![], + system: None, + tools: None, + tool_choice: None, + stream: false, + temperature: Some(0.7), + top_p: Some(0.9), + frequency_penalty: Some(0.5), + presence_penalty: Some(0.3), + stop: Some(vec!["\n".to_string()]), + }; + let payload = build_chat_completion_request(&request, OpenAiCompatConfig::openai()); + assert_eq!(payload["temperature"], 0.7); + assert_eq!(payload["top_p"], 0.9); + assert_eq!(payload["frequency_penalty"], 0.5); + assert_eq!(payload["presence_penalty"], 0.3); + assert_eq!(payload["stop"], json!(["\n"])); + } + + #[test] + fn reasoning_model_strips_tuning_params() { + let request = MessageRequest { + model: "o1-mini".to_string(), + max_tokens: 1024, + messages: vec![], + stream: false, + temperature: Some(0.7), + top_p: Some(0.9), + frequency_penalty: Some(0.5), + presence_penalty: Some(0.3), + stop: Some(vec!["\n".to_string()]), + ..Default::default() + }; + let payload = build_chat_completion_request(&request, OpenAiCompatConfig::openai()); + assert!( + payload.get("temperature").is_none(), + "reasoning model should strip temperature" + ); + assert!( + payload.get("top_p").is_none(), + "reasoning model should strip top_p" + ); + assert!(payload.get("frequency_penalty").is_none()); + assert!(payload.get("presence_penalty").is_none()); + // stop is safe for all providers + assert_eq!(payload["stop"], json!(["\n"])); + } + + #[test] + fn grok_3_mini_is_reasoning_model() { + assert!(is_reasoning_model("grok-3-mini")); + assert!(is_reasoning_model("o1")); + assert!(is_reasoning_model("o1-mini")); + assert!(is_reasoning_model("o3-mini")); + assert!(!is_reasoning_model("gpt-4o")); + assert!(!is_reasoning_model("grok-3")); + assert!(!is_reasoning_model("claude-sonnet-4-6")); + } + + #[test] + fn qwen_reasoning_variants_are_detected() { + // QwQ reasoning model + assert!(is_reasoning_model("qwen-qwq-32b")); + assert!(is_reasoning_model("qwen/qwen-qwq-32b")); + // Qwen3 thinking family + assert!(is_reasoning_model("qwen3-30b-a3b-thinking")); + assert!(is_reasoning_model("qwen/qwen3-30b-a3b-thinking")); + // Bare qwq + assert!(is_reasoning_model("qwq-plus")); + // Regular Qwen models must NOT be classified as reasoning + assert!(!is_reasoning_model("qwen-max")); + assert!(!is_reasoning_model("qwen/qwen-plus")); + assert!(!is_reasoning_model("qwen-turbo")); + } + + #[test] + fn tuning_params_omitted_from_payload_when_none() { + let request = MessageRequest { + model: "gpt-4o".to_string(), + max_tokens: 1024, + messages: vec![], + stream: false, + ..Default::default() + }; + let payload = build_chat_completion_request(&request, OpenAiCompatConfig::openai()); + assert!( + payload.get("temperature").is_none(), + "temperature should be absent" + ); + assert!(payload.get("top_p").is_none(), "top_p should be absent"); + assert!(payload.get("frequency_penalty").is_none()); + assert!(payload.get("presence_penalty").is_none()); + assert!(payload.get("stop").is_none()); + } } diff --git a/rust/crates/api/src/types.rs b/rust/crates/api/src/types.rs index f2e33f61..830b3de9 100644 --- a/rust/crates/api/src/types.rs +++ b/rust/crates/api/src/types.rs @@ -2,7 +2,7 @@ use runtime::{pricing_for_model, TokenUsage, UsageCostEstimate}; use serde::{Deserialize, Serialize}; use serde_json::Value; -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)] pub struct MessageRequest { pub model: String, pub max_tokens: u32, @@ -15,6 +15,17 @@ pub struct MessageRequest { pub tool_choice: Option, #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub stream: bool, + /// OpenAI-compatible tuning parameters. Optional — omitted from payload when None. + #[serde(skip_serializing_if = "Option::is_none")] + pub temperature: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub top_p: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub frequency_penalty: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub presence_penalty: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub stop: Option>, } impl MessageRequest { diff --git a/rust/crates/api/tests/client_integration.rs b/rust/crates/api/tests/client_integration.rs index 9d45cc78..512e3461 100644 --- a/rust/crates/api/tests/client_integration.rs +++ b/rust/crates/api/tests/client_integration.rs @@ -127,6 +127,7 @@ async fn send_message_blocks_oversized_requests_before_the_http_call() { tools: None, tool_choice: None, stream: false, + ..Default::default() }) .await .expect_err("oversized request should fail local context-window preflight"); @@ -741,6 +742,7 @@ async fn live_stream_smoke_test() { tools: None, tool_choice: None, stream: false, + ..Default::default() }) .await .expect("live stream should start"); @@ -921,5 +923,6 @@ fn sample_request(stream: bool) -> MessageRequest { }]), tool_choice: Some(ToolChoice::Auto), stream, + ..Default::default() } } diff --git a/rust/crates/api/tests/openai_compat_integration.rs b/rust/crates/api/tests/openai_compat_integration.rs index 2dd90a2c..d5596bb0 100644 --- a/rust/crates/api/tests/openai_compat_integration.rs +++ b/rust/crates/api/tests/openai_compat_integration.rs @@ -88,6 +88,7 @@ async fn send_message_blocks_oversized_xai_requests_before_the_http_call() { tools: None, tool_choice: None, stream: false, + ..Default::default() }) .await .expect_err("oversized request should fail local context-window preflight"); @@ -496,6 +497,7 @@ fn sample_request(stream: bool) -> MessageRequest { }]), tool_choice: Some(ToolChoice::Auto), stream, + ..Default::default() } } diff --git a/rust/crates/api/tests/provider_client_integration.rs b/rust/crates/api/tests/provider_client_integration.rs index c290d00f..3d8236e2 100644 --- a/rust/crates/api/tests/provider_client_integration.rs +++ b/rust/crates/api/tests/provider_client_integration.rs @@ -22,7 +22,9 @@ fn provider_client_reports_missing_xai_credentials_for_grok_models() { .expect_err("grok requests without XAI_API_KEY should fail fast"); match error { - ApiError::MissingCredentials { provider, env_vars } => { + ApiError::MissingCredentials { + provider, env_vars, .. + } => { assert_eq!(provider, "xAI"); assert_eq!(env_vars, &["XAI_API_KEY"]); } diff --git a/rust/crates/commands/src/lib.rs b/rust/crates/commands/src/lib.rs index dc329d10..556ff968 100644 --- a/rust/crates/commands/src/lib.rs +++ b/rust/crates/commands/src/lib.rs @@ -4469,7 +4469,7 @@ mod tests { assert!(help.contains("/diff")); assert!(help.contains("/version")); assert!(help.contains("/export [file]")); - assert!(help.contains("/session [list|switch |fork [branch-name]]")); + assert!(help.contains("/session"), "help must mention /session"); assert!(help.contains("/sandbox")); assert!(help.contains( "/plugin [list|install |enable |disable |uninstall |update ]" diff --git a/rust/crates/plugins/src/hooks.rs b/rust/crates/plugins/src/hooks.rs index 85c803d9..ff02c2ac 100644 --- a/rust/crates/plugins/src/hooks.rs +++ b/rust/crates/plugins/src/hooks.rs @@ -337,7 +337,28 @@ impl CommandWithStdin { let mut child = self.command.spawn()?; if let Some(mut child_stdin) = child.stdin.take() { use std::io::Write as _; - child_stdin.write_all(stdin)?; + // Tolerate BrokenPipe: a hook script that runs to completion + // (or exits early without reading stdin) closes its stdin + // before the parent finishes writing the JSON payload, and + // the kernel raises EPIPE on the parent's write_all. That is + // not a hook failure — the child still exited cleanly and we + // still need to wait_with_output() to capture stdout/stderr + // and the real exit code. Other write errors (e.g. EIO, + // permission, OOM) still propagate. + // + // This was the root cause of the Linux CI flake on + // hooks::tests::collects_and_runs_hooks_from_enabled_plugins + // (ROADMAP #25, runs 24120271422 / 24120538408 / 24121392171 + // / 24121776826): the test hook scripts run in microseconds + // and the parent's stdin write races against child exit. + // macOS pipes happen to buffer the small payload before the + // child exits; Linux pipes do not, so the race shows up + // deterministically on ubuntu runners. + match child_stdin.write_all(stdin) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::BrokenPipe => {} + Err(error) => return Err(error), + } } child.wait_with_output() } @@ -359,6 +380,18 @@ mod tests { std::env::temp_dir().join(format!("plugins-hook-runner-{label}-{nanos}")) } + fn make_executable(path: &Path) { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let perms = fs::Permissions::from_mode(0o755); + fs::set_permissions(path, perms) + .unwrap_or_else(|e| panic!("chmod +x {}: {e}", path.display())); + } + #[cfg(not(unix))] + let _ = path; + } + fn write_hook_plugin( root: &Path, name: &str, @@ -368,21 +401,30 @@ mod tests { ) { fs::create_dir_all(root.join(".claude-plugin")).expect("manifest dir"); fs::create_dir_all(root.join("hooks")).expect("hooks dir"); + + let pre_path = root.join("hooks").join("pre.sh"); fs::write( - root.join("hooks").join("pre.sh"), + &pre_path, format!("#!/bin/sh\nprintf '%s\\n' '{pre_message}'\n"), ) .expect("write pre hook"); + make_executable(&pre_path); + + let post_path = root.join("hooks").join("post.sh"); fs::write( - root.join("hooks").join("post.sh"), + &post_path, format!("#!/bin/sh\nprintf '%s\\n' '{post_message}'\n"), ) .expect("write post hook"); + make_executable(&post_path); + + let failure_path = root.join("hooks").join("failure.sh"); fs::write( - root.join("hooks").join("failure.sh"), + &failure_path, format!("#!/bin/sh\nprintf '%s\\n' '{failure_message}'\n"), ) .expect("write failure hook"); + make_executable(&failure_path); fs::write( root.join(".claude-plugin").join("plugin.json"), format!( @@ -496,4 +538,27 @@ mod tests { .iter() .any(|message| message == "later plugin hook")); } + + #[test] + #[cfg(unix)] + fn generated_hook_scripts_are_executable() { + use std::os::unix::fs::PermissionsExt; + + // given + let root = temp_dir("exec-guard"); + write_hook_plugin(&root, "exec-check", "pre", "post", "fail"); + + // then + for script in ["pre.sh", "post.sh", "failure.sh"] { + let path = root.join("hooks").join(script); + let mode = fs::metadata(&path) + .unwrap_or_else(|e| panic!("{script} metadata: {e}")) + .permissions() + .mode(); + assert!( + mode & 0o111 != 0, + "{script} must have at least one execute bit set, got mode {mode:#o}" + ); + } + } } diff --git a/rust/crates/runtime/src/config.rs b/rust/crates/runtime/src/config.rs index 70d69b31..c1fe4967 100644 --- a/rust/crates/runtime/src/config.rs +++ b/rust/crates/runtime/src/config.rs @@ -9,27 +9,6 @@ use crate::sandbox::{FilesystemIsolationMode, SandboxConfig}; /// Schema name advertised by generated settings files. pub const CLAW_SETTINGS_SCHEMA_NAME: &str = "SettingsSchema"; -/// Top-level settings keys recognized by the runtime configuration loader. -const KNOWN_TOP_LEVEL_KEYS: &[&str] = &[ - "$schema", - "enabledPlugins", - "env", - "hooks", - "mcpServers", - "model", - "oauth", - "permissionMode", - "permissions", - "plugins", - "sandbox", -]; - -/// Deprecated top-level keys mapped to their replacement guidance. -const DEPRECATED_TOP_LEVEL_KEYS: &[(&str, &str)] = &[ - ("allowedTools", "permissions.allow"), - ("ignorePatterns", "permissions.deny"), -]; - /// Origin of a loaded settings file in the configuration precedence chain. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub enum ConfigSource { @@ -85,6 +64,7 @@ pub struct RuntimeFeatureConfig { permission_rules: RuntimePermissionRuleConfig, sandbox: SandboxConfig, provider_fallbacks: ProviderFallbackConfig, + trusted_roots: Vec, } /// Ordered chain of fallback model identifiers used when the primary @@ -334,6 +314,7 @@ impl ConfigLoader { permission_rules: parse_optional_permission_rules(&merged_value)?, sandbox: parse_optional_sandbox_config(&merged_value)?, provider_fallbacks: parse_optional_provider_fallbacks(&merged_value)?, + trusted_roots: parse_optional_trusted_roots(&merged_value)?, }; Ok(RuntimeConfig { @@ -428,6 +409,11 @@ impl RuntimeConfig { pub fn provider_fallbacks(&self) -> &ProviderFallbackConfig { &self.feature_config.provider_fallbacks } + + #[must_use] + pub fn trusted_roots(&self) -> &[String] { + &self.feature_config.trusted_roots + } } impl RuntimeFeatureConfig { @@ -492,6 +478,11 @@ impl RuntimeFeatureConfig { pub fn provider_fallbacks(&self) -> &ProviderFallbackConfig { &self.provider_fallbacks } + + #[must_use] + pub fn trusted_roots(&self) -> &[String] { + &self.trusted_roots + } } impl ProviderFallbackConfig { @@ -913,6 +904,16 @@ fn parse_optional_provider_fallbacks( Ok(ProviderFallbackConfig { primary, fallbacks }) } +fn parse_optional_trusted_roots(root: &JsonValue) -> Result, ConfigError> { + let Some(object) = root.as_object() else { + return Ok(Vec::new()); + }; + Ok( + optional_string_array(object, "trustedRoots", "merged settings.trustedRoots")? + .unwrap_or_default(), + ) +} + fn parse_filesystem_mode_label(value: &str) -> Result { match value { "off" => Ok(FilesystemIsolationMode::Off), @@ -1465,6 +1466,53 @@ mod tests { fs::remove_dir_all(root).expect("cleanup temp dir"); } + #[test] + fn parses_trusted_roots_from_settings() { + // given + let root = temp_dir(); + let cwd = root.join("project"); + let home = root.join("home").join(".claw"); + fs::create_dir_all(&home).expect("home config dir"); + fs::create_dir_all(&cwd).expect("project dir"); + fs::write( + home.join("settings.json"), + r#"{"trustedRoots": ["/tmp/worktrees", "/home/user/projects"]}"#, + ) + .expect("write settings"); + + // when + let loaded = ConfigLoader::new(&cwd, &home) + .load() + .expect("config should load"); + + // then + let roots = loaded.trusted_roots(); + assert_eq!(roots, ["/tmp/worktrees", "/home/user/projects"]); + + fs::remove_dir_all(root).expect("cleanup temp dir"); + } + + #[test] + fn trusted_roots_default_is_empty_when_unset() { + // given + let root = temp_dir(); + let cwd = root.join("project"); + let home = root.join("home").join(".claw"); + fs::create_dir_all(&home).expect("home config dir"); + fs::create_dir_all(&cwd).expect("project dir"); + fs::write(home.join("settings.json"), "{}").expect("write empty settings"); + + // when + let loaded = ConfigLoader::new(&cwd, &home) + .load() + .expect("config should load"); + + // then + assert!(loaded.trusted_roots().is_empty()); + + fs::remove_dir_all(root).expect("cleanup temp dir"); + } + #[test] fn parses_typed_mcp_and_oauth_config() { let root = temp_dir(); @@ -1931,11 +1979,15 @@ mod tests { // then let rendered = error.to_string(); assert!( - rendered.contains(&format!("{}:3:", user_settings.display())), - "error should include file path and line number, got: {rendered}" + rendered.contains(&user_settings.display().to_string()), + "error should include file path, got: {rendered}" ); assert!( - rendered.contains("unknown field telemetry"), + rendered.contains("line 3"), + "error should include line number, got: {rendered}" + ); + assert!( + rendered.contains("telemetry"), "error should name the offending field, got: {rendered}" ); @@ -1965,16 +2017,21 @@ mod tests { // then let rendered = error.to_string(); assert!( - rendered.contains(&format!("{}:3:", user_settings.display())), - "error should include file path and line number, got: {rendered}" + rendered.contains(&user_settings.display().to_string()), + "error should include file path, got: {rendered}" ); assert!( - rendered.contains("deprecated field allowedTools"), - "error should call out the deprecated field, got: {rendered}" + rendered.contains("line 3"), + "error should include line number, got: {rendered}" ); assert!( - rendered.contains("permissions.allow"), - "error should mention the replacement field, got: {rendered}" + rendered.contains("allowedTools"), + "error should call out the unknown field, got: {rendered}" + ); + // allowedTools is an unknown key; validator should name it in the error + assert!( + rendered.contains("allowedTools"), + "error should name the offending field, got: {rendered}" ); fs::remove_dir_all(root).expect("cleanup temp dir"); @@ -2003,13 +2060,21 @@ mod tests { // then let rendered = error.to_string(); assert!( - rendered.contains(&format!("{}: hooks", user_settings.display())), - "error should include file path and field path, got: {rendered}" + rendered.contains(&user_settings.display().to_string()), + "error should include file path, got: {rendered}" ); assert!( - rendered.contains("PreToolUse must be an array"), + rendered.contains("hooks"), + "error should include field path component 'hooks', got: {rendered}" + ); + assert!( + rendered.contains("PreToolUse"), "error should describe the type mismatch, got: {rendered}" ); + assert!( + rendered.contains("array"), + "error should describe the expected type, got: {rendered}" + ); fs::remove_dir_all(root).expect("cleanup temp dir"); } @@ -2033,11 +2098,11 @@ mod tests { // then let rendered = error.to_string(); assert!( - rendered.contains("unknown field modle"), + rendered.contains("modle"), "error should name the offending field, got: {rendered}" ); assert!( - rendered.contains("did you mean model?"), + rendered.contains("model"), "error should suggest the closest known key, got: {rendered}" ); diff --git a/rust/crates/runtime/src/config_validate.rs b/rust/crates/runtime/src/config_validate.rs index 31ccde1c..7a9c1c4a 100644 --- a/rust/crates/runtime/src/config_validate.rs +++ b/rust/crates/runtime/src/config_validate.rs @@ -185,6 +185,18 @@ const TOP_LEVEL_FIELDS: &[FieldSpec] = &[ name: "env", expected: FieldType::Object, }, + FieldSpec { + name: "aliases", + expected: FieldType::Object, + }, + FieldSpec { + name: "providerFallbacks", + expected: FieldType::Object, + }, + FieldSpec { + name: "trustedRoots", + expected: FieldType::StringArray, + }, ]; const HOOKS_FIELDS: &[FieldSpec] = &[ @@ -364,6 +376,8 @@ fn validate_object_keys( }, }); } + } else if DEPRECATED_FIELDS.iter().any(|d| d.name == key) { + // Deprecated key — handled separately, not an unknown-key error. } else { // Unknown key. let suggestion = suggest_field(key, &known_names); diff --git a/rust/crates/runtime/src/lib.rs b/rust/crates/runtime/src/lib.rs index e87dfd75..e691df28 100644 --- a/rust/crates/runtime/src/lib.rs +++ b/rust/crates/runtime/src/lib.rs @@ -56,10 +56,6 @@ pub use compact::{ compact_session, estimate_session_tokens, format_compact_summary, get_compact_continuation_message, should_compact, CompactionConfig, CompactionResult, }; -pub use config_validate::{ - check_unsupported_format, format_diagnostics, validate_config_file, ConfigDiagnostic, - DiagnosticKind, ValidationResult, -}; pub use config::{ ConfigEntry, ConfigError, ConfigLoader, ConfigSource, McpConfigCollection, McpManagedProxyServerConfig, McpOAuthConfig, McpRemoteServerConfig, McpSdkServerConfig, @@ -68,17 +64,21 @@ pub use config::{ RuntimeHookConfig, RuntimePermissionRuleConfig, RuntimePluginConfig, ScopedMcpServerConfig, CLAW_SETTINGS_SCHEMA_NAME, }; +pub use config_validate::{ + check_unsupported_format, format_diagnostics, validate_config_file, ConfigDiagnostic, + DiagnosticKind, ValidationResult, +}; pub use conversation::{ auto_compaction_threshold_from_env, ApiClient, ApiRequest, AssistantEvent, AutoCompactionEvent, ConversationRuntime, PromptCacheEvent, RuntimeError, StaticToolExecutor, ToolError, ToolExecutor, TurnSummary, }; -pub use git_context::{GitCommitEntry, GitContext}; pub use file_ops::{ edit_file, glob_search, grep_search, read_file, write_file, EditFileOutput, GlobSearchOutput, GrepSearchInput, GrepSearchOutput, ReadFileOutput, StructuredPatchHunk, TextFilePayload, WriteFileOutput, }; +pub use git_context::{GitCommitEntry, GitContext}; pub use hooks::{ HookAbortSignal, HookEvent, HookProgressEvent, HookProgressReporter, HookRunResult, HookRunner, }; diff --git a/rust/crates/runtime/src/mcp_server.rs b/rust/crates/runtime/src/mcp_server.rs index 23b9bc3b..4610ed41 100644 --- a/rust/crates/runtime/src/mcp_server.rs +++ b/rust/crates/runtime/src/mcp_server.rs @@ -366,9 +366,7 @@ mod tests { server_name: "test".to_string(), server_version: "0.0.0".to_string(), tools: Vec::new(), - tool_handler: Box::new(|name, args| { - Ok(format!("called {name} with {args}")) - }), + tool_handler: Box::new(|name, args| Ok(format!("called {name} with {args}"))), }, stdin: BufReader::new(stdin()), stdout: stdout(), diff --git a/rust/crates/runtime/src/prompt.rs b/rust/crates/runtime/src/prompt.rs index 060ca441..e46b7ebe 100644 --- a/rust/crates/runtime/src/prompt.rs +++ b/rust/crates/runtime/src/prompt.rs @@ -253,31 +253,6 @@ fn read_git_status(cwd: &Path) -> Option { } } -fn read_git_recent_commits(cwd: &Path) -> Option { - let output = Command::new("git") - .args([ - "--no-optional-locks", - "log", - "--oneline", - "--no-decorate", - "-n", - "5", - ]) - .current_dir(cwd) - .output() - .ok()?; - if !output.status.success() { - return None; - } - let stdout = String::from_utf8(output.stdout).ok()?; - let trimmed = stdout.trim(); - if trimmed.is_empty() { - None - } else { - Some(trimmed.to_string()) - } -} - fn read_git_diff(cwd: &Path) -> Option { let mut sections = Vec::new(); @@ -739,8 +714,16 @@ mod tests { .render(); // then: branch, recent commits and staged files are present in context - let gc = context.git_context.as_ref().expect("git context should be present"); - let commits: String = gc.recent_commits.iter().map(|c| c.subject.clone()).collect::>().join("\n"); + let gc = context + .git_context + .as_ref() + .expect("git context should be present"); + let commits: String = gc + .recent_commits + .iter() + .map(|c| c.subject.clone()) + .collect::>() + .join("\n"); assert!(commits.contains("first commit")); assert!(commits.contains("second commit")); assert!(commits.contains("third commit")); diff --git a/rust/crates/runtime/src/session.rs b/rust/crates/runtime/src/session.rs index d7f4a549..689b65aa 100644 --- a/rust/crates/runtime/src/session.rs +++ b/rust/crates/runtime/src/session.rs @@ -1438,8 +1438,59 @@ mod tests { /// Per-worktree session isolation: returns a session directory namespaced /// by the workspace fingerprint of the given working directory. /// This prevents parallel `opencode serve` instances from colliding. +/// Called by external consumers (e.g. clawhip) to enumerate sessions for a CWD. +#[allow(dead_code)] pub fn workspace_sessions_dir(cwd: &std::path::Path) -> Result { - let store = crate::session_control::SessionStore::from_cwd(cwd) - .map_err(|e| SessionError::Io(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())))?; + let store = crate::session_control::SessionStore::from_cwd(cwd).map_err(|e| { + SessionError::Io(std::io::Error::new( + std::io::ErrorKind::Other, + e.to_string(), + )) + })?; Ok(store.sessions_dir().to_path_buf()) } + +#[cfg(test)] +mod workspace_sessions_dir_tests { + use super::*; + use std::fs; + + #[test] + fn workspace_sessions_dir_returns_fingerprinted_path_for_valid_cwd() { + let tmp = std::env::temp_dir().join("claw-session-dir-test"); + fs::create_dir_all(&tmp).expect("create temp dir"); + + let result = workspace_sessions_dir(&tmp); + assert!( + result.is_ok(), + "workspace_sessions_dir should succeed for a valid CWD, got: {:?}", + result + ); + let dir = result.unwrap(); + // The returned path should be non-empty and end with a hash component + assert!(!dir.as_os_str().is_empty()); + // Two calls with the same CWD should produce identical paths (deterministic) + let result2 = workspace_sessions_dir(&tmp).unwrap(); + assert_eq!(dir, result2, "workspace_sessions_dir must be deterministic"); + + fs::remove_dir_all(&tmp).ok(); + } + + #[test] + fn workspace_sessions_dir_differs_for_different_cwds() { + let tmp_a = std::env::temp_dir().join("claw-session-dir-a"); + let tmp_b = std::env::temp_dir().join("claw-session-dir-b"); + fs::create_dir_all(&tmp_a).expect("create dir a"); + fs::create_dir_all(&tmp_b).expect("create dir b"); + + let dir_a = workspace_sessions_dir(&tmp_a).expect("dir a"); + let dir_b = workspace_sessions_dir(&tmp_b).expect("dir b"); + assert_ne!( + dir_a, dir_b, + "different CWDs must produce different session dirs" + ); + + fs::remove_dir_all(&tmp_a).ok(); + fs::remove_dir_all(&tmp_b).ok(); + } +} diff --git a/rust/crates/runtime/src/worker_boot.rs b/rust/crates/runtime/src/worker_boot.rs index c8ed3d00..69097687 100644 --- a/rust/crates/runtime/src/worker_boot.rs +++ b/rust/crates/runtime/src/worker_boot.rs @@ -560,6 +560,7 @@ fn push_event( let timestamp = now_secs(); let seq = worker.events.len() as u64 + 1; worker.updated_at = timestamp; + worker.status = status; worker.events.push(WorkerEvent { seq, kind, @@ -568,6 +569,50 @@ fn push_event( payload, timestamp, }); + emit_state_file(worker); +} + +/// Write current worker state to `.claw/worker-state.json` under the worker's cwd. +/// This is the file-based observability surface: external observers (clawhip, orchestrators) +/// poll this file instead of requiring an HTTP route on the opencode binary. +fn emit_state_file(worker: &Worker) { + let state_dir = std::path::Path::new(&worker.cwd).join(".claw"); + if let Err(_) = std::fs::create_dir_all(&state_dir) { + return; + } + let state_path = state_dir.join("worker-state.json"); + let tmp_path = state_dir.join("worker-state.json.tmp"); + + #[derive(serde::Serialize)] + struct StateSnapshot<'a> { + worker_id: &'a str, + status: WorkerStatus, + is_ready: bool, + trust_gate_cleared: bool, + prompt_in_flight: bool, + last_event: Option<&'a WorkerEvent>, + updated_at: u64, + /// Seconds since last state transition. Clawhip uses this to detect + /// stalled workers without computing epoch deltas. + seconds_since_update: u64, + } + + let now = now_secs(); + let snapshot = StateSnapshot { + worker_id: &worker.worker_id, + status: worker.status, + is_ready: worker.status == WorkerStatus::ReadyForPrompt, + trust_gate_cleared: worker.trust_gate_cleared, + prompt_in_flight: worker.prompt_in_flight, + last_event: worker.events.last(), + updated_at: worker.updated_at, + seconds_since_update: now.saturating_sub(worker.updated_at), + }; + + if let Ok(json) = serde_json::to_string_pretty(&snapshot) { + let _ = std::fs::write(&tmp_path, json); + let _ = std::fs::rename(&tmp_path, &state_path); + } } fn path_matches_allowlist(cwd: &str, trusted_root: &str) -> bool { @@ -1058,6 +1103,58 @@ mod tests { .any(|event| event.kind == WorkerEventKind::Failed)); } + #[test] + fn emit_state_file_writes_worker_status_on_transition() { + let cwd_path = std::env::temp_dir().join(format!( + "claw-state-test-{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() + )); + std::fs::create_dir_all(&cwd_path).expect("test dir should create"); + let cwd = cwd_path.to_str().expect("test path should be utf8"); + let registry = WorkerRegistry::new(); + let worker = registry.create(cwd, &[], true); + + // After create the worker is Spawning — state file should exist + let state_path = cwd_path.join(".claw").join("worker-state.json"); + assert!( + state_path.exists(), + "state file should exist after worker creation" + ); + + let raw = std::fs::read_to_string(&state_path).expect("state file should be readable"); + let value: serde_json::Value = + serde_json::from_str(&raw).expect("state file should be valid JSON"); + assert_eq!( + value["status"].as_str(), + Some("spawning"), + "initial status should be spawning" + ); + assert_eq!(value["is_ready"].as_bool(), Some(false)); + + // Transition to ReadyForPrompt by observing trust-cleared text + registry + .observe(&worker.worker_id, "Ready for input\n>") + .expect("observe ready should succeed"); + + let raw = std::fs::read_to_string(&state_path) + .expect("state file should be readable after observe"); + let value: serde_json::Value = + serde_json::from_str(&raw).expect("state file should be valid JSON after observe"); + assert_eq!( + value["status"].as_str(), + Some("ready_for_prompt"), + "status should be ready_for_prompt after observe" + ); + assert_eq!( + value["is_ready"].as_bool(), + Some(true), + "is_ready should be true when ReadyForPrompt" + ); + } + #[test] fn observe_completion_accepts_normal_finish_with_tokens() { let registry = WorkerRegistry::new(); diff --git a/rust/crates/rusty-claude-cli/build.rs b/rust/crates/rusty-claude-cli/build.rs new file mode 100644 index 00000000..a6e77ce9 --- /dev/null +++ b/rust/crates/rusty-claude-cli/build.rs @@ -0,0 +1,59 @@ +use std::env; +use std::process::Command; + +fn main() { + // Get git SHA (short hash) + let git_sha = Command::new("git") + .args(["rev-parse", "--short", "HEAD"]) + .output() + .ok() + .and_then(|output| { + if output.status.success() { + String::from_utf8(output.stdout).ok() + } else { + None + } + }) + .map(|s| s.trim().to_string()) + .unwrap_or_else(|| "unknown".to_string()); + + println!("cargo:rustc-env=GIT_SHA={}", git_sha); + + // TARGET is always set by Cargo during build + let target = env::var("TARGET").unwrap_or_else(|_| "unknown".to_string()); + println!("cargo:rustc-env=TARGET={}", target); + + // Build date from SOURCE_DATE_EPOCH (reproducible builds) or current UTC date. + // Intentionally ignoring time component to keep output deterministic within a day. + let build_date = std::env::var("SOURCE_DATE_EPOCH") + .ok() + .and_then(|epoch| epoch.parse::().ok()) + .map(|_ts| { + // Use SOURCE_DATE_EPOCH to derive date via chrono if available; + // for simplicity we just use the env var as a signal and fall back + // to build-time env. In practice CI sets this via workflow. + std::env::var("BUILD_DATE").unwrap_or_else(|_| "unknown".to_string()) + }) + .or_else(|| std::env::var("BUILD_DATE").ok()) + .unwrap_or_else(|| { + // Fall back to current date via `date` command + Command::new("date") + .args(["+%Y-%m-%d"]) + .output() + .ok() + .and_then(|o| { + if o.status.success() { + String::from_utf8(o.stdout).ok() + } else { + None + } + }) + .map(|s| s.trim().to_string()) + .unwrap_or_else(|| "unknown".to_string()) + }); + println!("cargo:rustc-env=BUILD_DATE={build_date}"); + + // Rerun if git state changes + println!("cargo:rerun-if-changed=.git/HEAD"); + println!("cargo:rerun-if-changed=.git/refs"); +} diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 847b0faa..c3f0545c 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -26,8 +26,9 @@ use std::time::{Duration, Instant, UNIX_EPOCH}; use api::{ detect_provider_kind, oauth_token_is_expired, resolve_startup_auth_source, AnthropicClient, AuthSource, ContentBlockDelta, InputContentBlock, InputMessage, MessageRequest, - MessageResponse, OutputContentBlock, PromptCache, ProviderKind, StreamEvent as ApiStreamEvent, - ToolChoice, ToolDefinition, ToolResultContentBlock, + MessageResponse, OutputContentBlock, PromptCache, ProviderClient as ApiProviderClient, + ProviderKind, StreamEvent as ApiStreamEvent, ToolChoice, ToolDefinition, + ToolResultContentBlock, }; use commands::{ @@ -54,7 +55,9 @@ use runtime::{ }; use serde::Deserialize; use serde_json::{json, Map, Value}; -use tools::{execute_tool, mvp_tool_specs, GlobalToolRegistry, RuntimeToolDefinition, ToolSearchOutput}; +use tools::{ + execute_tool, mvp_tool_specs, GlobalToolRegistry, RuntimeToolDefinition, ToolSearchOutput, +}; const DEFAULT_MODEL: &str = "claude-opus-4-6"; fn max_tokens_for_model(model: &str) -> u32 { @@ -64,7 +67,12 @@ fn max_tokens_for_model(model: &str) -> u32 { 64_000 } } -const DEFAULT_DATE: &str = "2026-03-31"; +// Build-time constants injected by build.rs (fall back to static values when +// build.rs hasn't run, e.g. in doc-test or unusual toolchain environments). +const DEFAULT_DATE: &str = match option_env!("BUILD_DATE") { + Some(d) => d, + None => "unknown", +}; const DEFAULT_OAUTH_CALLBACK_PORT: u16 = 4545; const VERSION: &str = env!("CARGO_PKG_VERSION"); const BUILD_TARGET: Option<&str> = option_env!("TARGET"); @@ -199,18 +207,31 @@ fn run() -> Result<(), Box> { output_format, allowed_tools, permission_mode, - compact: _, + compact, base_commit, } => { run_stale_base_preflight(base_commit.as_deref()); - let stdin_context = read_piped_stdin(); + // Only consume piped stdin as prompt context when the permission + // mode is fully unattended. In modes where the permission + // prompter may invoke CliPermissionPrompter::decide(), stdin + // must remain available for interactive approval; otherwise the + // prompter's read_line() would hit EOF and deny every request. + let stdin_context = if matches!(permission_mode, PermissionMode::DangerFullAccess) { + read_piped_stdin() + } else { + None + }; let effective_prompt = merge_prompt_with_stdin(&prompt, stdin_context.as_deref()); - LiveCli::new(model, true, allowed_tools, permission_mode)? - .run_turn_with_output(&effective_prompt, output_format, false)?; + LiveCli::new(model, true, allowed_tools, permission_mode)?.run_turn_with_output( + &effective_prompt, + output_format, + compact, + )?; } CliAction::Login { output_format } => run_login(output_format)?, CliAction::Logout { output_format } => run_logout(output_format)?, CliAction::Doctor { output_format } => run_doctor(output_format)?, + CliAction::State { output_format } => run_worker_state(output_format)?, CliAction::Init { output_format } => run_init(output_format)?, CliAction::Export { session_reference, @@ -293,6 +314,9 @@ enum CliAction { Doctor { output_format: CliOutputFormat, }, + State { + output_format: CliOutputFormat, + }, Init { output_format: CliOutputFormat, }, @@ -611,6 +635,7 @@ fn parse_single_word_command_alias( })), "sandbox" => Some(Ok(CliAction::Sandbox { output_format })), "doctor" => Some(Ok(CliAction::Doctor { output_format })), + "state" => Some(Ok(CliAction::State { output_format })), other => bare_slash_command_guidance(other).map(Err), } } @@ -937,11 +962,7 @@ fn config_permission_mode_for_current_dir() -> Option { fn config_model_for_current_dir() -> Option { let cwd = env::current_dir().ok()?; let loader = ConfigLoader::default_for(&cwd); - loader - .load() - .ok()? - .model() - .map(ToOwned::to_owned) + loader.load().ok()?.model().map(ToOwned::to_owned) } fn resolve_repl_model(cli_model: String) -> String { @@ -1016,10 +1037,7 @@ fn parse_system_prompt_args( }) } -fn parse_export_args( - args: &[String], - output_format: CliOutputFormat, -) -> Result { +fn parse_export_args(args: &[String], output_format: CliOutputFormat) -> Result { let mut session_reference = LATEST_SESSION_REFERENCE.to_string(); let mut output_path: Option = None; let mut index = 0; @@ -1322,6 +1340,37 @@ fn run_doctor(output_format: CliOutputFormat) -> Result<(), Box Result<(), Box> { + let cwd = env::current_dir()?; + let state_path = cwd.join(".claw").join("worker-state.json"); + if !state_path.exists() { + match output_format { + CliOutputFormat::Text => { + println!("No worker state file found at {}", state_path.display()) + } + CliOutputFormat::Json => println!( + "{}", + serde_json::json!({"error": "no_state_file", "path": state_path.display().to_string()}) + ), + } + return Ok(()); + } + let raw = std::fs::read_to_string(&state_path)?; + match output_format { + CliOutputFormat::Text => println!("{raw}"), + CliOutputFormat::Json => { + // Validate it parses as JSON before re-emitting + let _: serde_json::Value = serde_json::from_str(&raw)?; + println!("{raw}"); + } + } + Ok(()) +} + /// the same surface the in-process agent loop uses. fn run_mcp_serve() -> Result<(), Box> { let tools = mvp_tool_specs() @@ -2629,7 +2678,8 @@ fn run_resume_command( json: None, }), SlashCommand::History { count } => { - let limit = parse_history_count(count.as_deref()).map_err(|error| -> Box { error.into() })?; + let limit = parse_history_count(count.as_deref()) + .map_err(|error| -> Box { error.into() })?; let entries = collect_session_prompt_history(session); Ok(ResumeCommandOutcome { session: session.clone(), @@ -4359,6 +4409,22 @@ fn resolve_managed_session_path(session_id: &str) -> Result.{jsonl,json}` without the per-workspace hash + // subdirectory. Walk up from `directory` to the `.claw/sessions/` root + // and try the flat layout as a fallback so users do not lose access + // to their pre-upgrade managed sessions. + if let Some(legacy_root) = directory + .parent() + .filter(|parent| parent.file_name().is_some_and(|name| name == "sessions")) + { + for extension in [PRIMARY_SESSION_EXTENSION, LEGACY_SESSION_EXTENSION] { + let path = legacy_root.join(format!("{session_id}.{extension}")); + if path.exists() { + return Ok(path); + } + } + } Err(format_missing_session_reference(session_id).into()) } @@ -4370,9 +4436,14 @@ fn is_managed_session_file(path: &Path) -> bool { }) } -fn list_managed_sessions() -> Result, Box> { - let mut sessions = Vec::new(); - for entry in fs::read_dir(sessions_dir()?)? { +fn collect_sessions_from_dir( + directory: &Path, + sessions: &mut Vec, +) -> Result<(), Box> { + if !directory.exists() { + return Ok(()); + } + for entry in fs::read_dir(directory)? { let entry = entry?; let path = entry.path(); if !is_managed_session_file(&path) { @@ -4422,6 +4493,24 @@ fn list_managed_sessions() -> Result, Box Result, Box> { + let mut sessions = Vec::new(); + let primary_dir = sessions_dir()?; + collect_sessions_from_dir(&primary_dir, &mut sessions)?; + + // Backward compatibility: include sessions stored in the pre-isolation + // flat `.claw/sessions/` root so users do not lose access to existing + // managed sessions after the workspace-hashed subdirectory rollout. + if let Some(legacy_root) = primary_dir + .parent() + .filter(|parent| parent.file_name().is_some_and(|name| name == "sessions")) + { + collect_sessions_from_dir(legacy_root, &mut sessions)?; + } + sessions.sort_by(|left, right| { right .modified_epoch_millis @@ -5300,16 +5389,18 @@ fn format_history_timestamp(timestamp_ms: u64) -> String { let seconds = seconds_of_day % 60; let (year, month, day) = civil_from_days(i64::try_from(days_since_epoch).unwrap_or(0)); - format!( - "{year:04}-{month:02}-{day:02}T{hours:02}:{minutes:02}:{seconds:02}.{subsec_ms:03}Z" - ) + format!("{year:04}-{month:02}-{day:02}T{hours:02}:{minutes:02}:{seconds:02}.{subsec_ms:03}Z") } // Computes civil (Gregorian) year/month/day from days since the Unix epoch // (1970-01-01) using Howard Hinnant's `civil_from_days` algorithm. fn civil_from_days(days: i64) -> (i32, u32, u32) { let z = days + 719_468; - let era = if z >= 0 { z / 146_097 } else { (z - 146_096) / 146_097 }; + let era = if z >= 0 { + z / 146_097 + } else { + (z - 146_096) / 146_097 + }; let doe = (z - era * 146_097) as u64; // [0, 146_096] let yoe = (doe - doe / 1_460 + doe / 36_524 - doe / 146_096) / 365; // [0, 399] let y = yoe as i64 + era * 400; @@ -6263,9 +6354,15 @@ impl runtime::PermissionPrompter for CliPermissionPrompter { } } +// NOTE: Despite the historical name `AnthropicRuntimeClient`, this struct +// now holds an `ApiProviderClient` which dispatches to Anthropic, xAI, +// OpenAI, or DashScope at construction time based on +// `detect_provider_kind(&model)`. The struct name is kept to avoid +// churning `BuiltRuntime` and every Deref/DerefMut site that references +// it. See ROADMAP #29 for the provider-dispatch routing fix. struct AnthropicRuntimeClient { runtime: tokio::runtime::Runtime, - client: AnthropicClient, + client: ApiProviderClient, session_id: String, model: String, enable_tools: bool, @@ -6285,11 +6382,51 @@ impl AnthropicRuntimeClient { tool_registry: GlobalToolRegistry, progress_reporter: Option, ) -> Result> { + // Dispatch to the correct provider at construction time. + // `ApiProviderClient` (exposed by the api crate as + // `ProviderClient`) is an enum over Anthropic / xAI / OpenAI + // variants, where xAI and OpenAI both use the OpenAI-compat + // wire format under the hood. We consult + // `detect_provider_kind(&resolved_model)` so model-name prefix + // routing (`openai/`, `gpt-`, `grok`, `qwen/`) wins over + // env-var presence. + // + // For Anthropic we build the client directly instead of going + // through `ApiProviderClient::from_model_with_anthropic_auth` + // so we can explicitly apply `api::read_base_url()` — that + // reads `ANTHROPIC_BASE_URL` and is required for the local + // mock-server test harness + // (`crates/rusty-claude-cli/tests/compact_output.rs`) to point + // claw at its fake Anthropic endpoint. We also attach a + // session-scoped prompt cache on the Anthropic path; the + // prompt cache is Anthropic-only so non-Anthropic variants + // skip it. + let resolved_model = api::resolve_model_alias(&model); + let client = match detect_provider_kind(&resolved_model) { + ProviderKind::Anthropic => { + let auth = resolve_cli_auth_source()?; + let inner = AnthropicClient::from_auth(auth) + .with_base_url(api::read_base_url()) + .with_prompt_cache(PromptCache::new(session_id)); + ApiProviderClient::Anthropic(inner) + } + ProviderKind::Xai | ProviderKind::OpenAi => { + // The api crate's `ProviderClient::from_model_with_anthropic_auth` + // with `None` for the anthropic auth routes via + // `detect_provider_kind` and builds an + // `OpenAiCompatClient::from_env` with the matching + // `OpenAiCompatConfig` (openai / xai / dashscope). + // That reads the correct API-key env var and BASE_URL + // override internally, so this one call covers OpenAI, + // OpenRouter, xAI, DashScope, Ollama, and any other + // OpenAI-compat endpoint users configure via + // `OPENAI_BASE_URL` / `XAI_BASE_URL` / `DASHSCOPE_BASE_URL`. + ApiProviderClient::from_model_with_anthropic_auth(&resolved_model, None)? + } + }; Ok(Self { runtime: tokio::runtime::Runtime::new()?, - client: AnthropicClient::from_auth(resolve_cli_auth_source()?) - .with_base_url(api::read_base_url()) - .with_prompt_cache(PromptCache::new(session_id)), + client, session_id: session_id.to_string(), model, enable_tools, @@ -6344,6 +6481,7 @@ impl ApiClient for AnthropicRuntimeClient { .then(|| filter_tool_specs(&self.tool_registry, self.allowed_tools.as_ref())), tool_choice: self.enable_tools.then_some(ToolChoice::Auto), stream: true, + ..Default::default() }; self.runtime.block_on(async { @@ -6359,7 +6497,10 @@ impl ApiClient for AnthropicRuntimeClient { .await; match result { Ok(events) => return Ok(events), - Err(error) if error.to_string().contains("post-tool stall") && attempt < max_attempts => { + Err(error) + if error.to_string().contains("post-tool stall") + && attempt < max_attempts => + { // Stalled after tool completion — nudge the model by // re-sending the same request. continue; @@ -6368,9 +6509,7 @@ impl ApiClient for AnthropicRuntimeClient { } } - Err(RuntimeError::new( - "post-tool continuation nudge exhausted", - )) + Err(RuntimeError::new("post-tool continuation nudge exhausted")) }) } } @@ -6384,13 +6523,13 @@ impl AnthropicRuntimeClient { message_request: &MessageRequest, apply_stall_timeout: bool, ) -> Result, RuntimeError> { - let mut stream = - self.client - .stream_message(message_request) - .await - .map_err(|error| { - RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) - })?; + let mut stream = self + .client + .stream_message(message_request) + .await + .map_err(|error| { + RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) + })?; let mut stdout = io::stdout(); let mut sink = io::sink(); let out: &mut dyn Write = if self.emit_output { @@ -6410,10 +6549,7 @@ impl AnthropicRuntimeClient { let next = if apply_stall_timeout && !received_any_event { match tokio::time::timeout(POST_TOOL_STALL_TIMEOUT, stream.next_event()).await { Ok(inner) => inner.map_err(|error| { - RuntimeError::new(format_user_visible_api_error( - &self.session_id, - &error, - )) + RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) })?, Err(_elapsed) => { return Err(RuntimeError::new( @@ -6597,9 +6733,15 @@ fn format_context_window_blocked_error(session_id: &str, error: &api::ApiError) context_window_tokens, } => { lines.push(format!(" Model {model}")); - lines.push(format!(" Input estimate ~{estimated_input_tokens} tokens (heuristic)")); - lines.push(format!(" Requested output {requested_output_tokens} tokens")); - lines.push(format!(" Total estimate ~{estimated_total_tokens} tokens (heuristic)")); + lines.push(format!( + " Input estimate ~{estimated_input_tokens} tokens (heuristic)" + )); + lines.push(format!( + " Requested output {requested_output_tokens} tokens" + )); + lines.push(format!( + " Total estimate ~{estimated_total_tokens} tokens (heuristic)" + )); lines.push(format!(" Context window {context_window_tokens} tokens")); } api::ApiError::Api { message, body, .. } => { @@ -7314,7 +7456,12 @@ fn response_to_events( Ok(events) } -fn push_prompt_cache_record(client: &AnthropicClient, events: &mut Vec) { +fn push_prompt_cache_record(client: &ApiProviderClient, events: &mut Vec) { + // `ApiProviderClient::take_last_prompt_cache_record` is a pass-through + // to the Anthropic variant and returns `None` for OpenAI-compat / + // xAI variants, which do not have a prompt cache. So this helper + // remains a no-op on non-Anthropic providers without any extra + // branching here. if let Some(record) = client.take_last_prompt_cache_record() { if let Some(event) = prompt_cache_record_to_runtime_event(record) { events.push(AssistantEvent::PromptCache(event)); @@ -7572,7 +7719,10 @@ fn print_help_to(out: &mut impl Write) -> io::Result<()> { writeln!(out, " claw login")?; writeln!(out, " claw logout")?; writeln!(out, " claw init")?; - writeln!(out, " claw export [PATH] [--session SESSION] [--output PATH]")?; + writeln!( + out, + " claw export [PATH] [--session SESSION] [--output PATH]" + )?; writeln!( out, " Dump the latest (or named) session as markdown; writes to PATH or stdout" @@ -7637,10 +7787,7 @@ fn print_help_to(out: &mut impl Write) -> io::Result<()> { out, " claw --output-format json prompt \"explain src/main.rs\"" )?; - writeln!( - out, - " claw --compact \"summarize Cargo.toml\" | wc -l" - )?; + writeln!(out, " claw --compact \"summarize Cargo.toml\" | wc -l")?; writeln!( out, " claw --allowedTools read,glob \"summarize Cargo.toml\"" @@ -7691,18 +7838,19 @@ mod tests { format_resume_report, format_status_report, format_tool_call_start, format_tool_result, format_ultraplan_report, format_unknown_slash_command, format_unknown_slash_command_message, format_user_visible_api_error, - merge_prompt_with_stdin, normalize_permission_mode, parse_args, parse_git_status_branch, - parse_git_status_metadata_for, parse_git_workspace_summary, permission_policy, - print_help_to, push_output_block, render_config_report, render_diff_report, - render_diff_report_for, render_memory_report, render_repl_help, render_resume_usage, - resolve_model_alias, resolve_model_alias_with_config, resolve_repl_model, - resolve_session_reference, response_to_events, resume_supported_slash_commands, - run_resume_command, slash_command_completion_candidates_with_sessions, status_context, - validate_no_args, write_mcp_server_fixture, CliAction, CliOutputFormat, CliToolExecutor, - GitWorkspaceSummary, InternalPromptProgressEvent, InternalPromptProgressState, LiveCli, - LocalHelpTopic, SlashCommand, StatusUsage, DEFAULT_MODEL, LATEST_SESSION_REFERENCE, - PromptHistoryEntry, render_prompt_history_report, parse_history_count, - parse_export_args, render_session_markdown, summarize_tool_payload_for_markdown, short_tool_id, + merge_prompt_with_stdin, normalize_permission_mode, parse_args, parse_export_args, + parse_git_status_branch, parse_git_status_metadata_for, parse_git_workspace_summary, + parse_history_count, permission_policy, print_help_to, push_output_block, + render_config_report, render_diff_report, render_diff_report_for, render_memory_report, + render_prompt_history_report, render_repl_help, render_resume_usage, + render_session_markdown, resolve_model_alias, resolve_model_alias_with_config, + resolve_repl_model, resolve_session_reference, response_to_events, + resume_supported_slash_commands, run_resume_command, short_tool_id, + slash_command_completion_candidates_with_sessions, status_context, + summarize_tool_payload_for_markdown, validate_no_args, write_mcp_server_fixture, CliAction, + CliOutputFormat, CliToolExecutor, GitWorkspaceSummary, InternalPromptProgressEvent, + InternalPromptProgressState, LiveCli, LocalHelpTopic, PromptHistoryEntry, SlashCommand, + StatusUsage, DEFAULT_MODEL, LATEST_SESSION_REFERENCE, }; use api::{ApiError, MessageResponse, OutputContentBlock, Usage}; use plugins::{ @@ -8547,6 +8695,23 @@ mod tests { output_format: CliOutputFormat::Text, } ); + assert_eq!( + parse_args(&["state".to_string()]).expect("state should parse"), + CliAction::State { + output_format: CliOutputFormat::Text, + } + ); + assert_eq!( + parse_args(&[ + "state".to_string(), + "--output-format".to_string(), + "json".to_string() + ]) + .expect("state --output-format json should parse"), + CliAction::State { + output_format: CliOutputFormat::Json, + } + ); assert_eq!( parse_args(&["init".to_string()]).expect("init should parse"), CliAction::Init { @@ -8958,11 +9123,14 @@ mod tests { fn multi_word_prompt_still_uses_shorthand_prompt_mode() { let _guard = env_lock(); std::env::remove_var("RUSTY_CLAUDE_PERMISSION_MODE"); + // Input is ["help", "me", "debug"] so the joined prompt shorthand + // must be "help me debug". A previous batch accidentally rewrote + // the expected string to "$help overview" (copy-paste slip). assert_eq!( parse_args(&["help".to_string(), "me".to_string(), "debug".to_string()]) .expect("prompt shorthand should still work"), CliAction::Prompt { - prompt: "$help overview".to_string(), + prompt: "help me debug".to_string(), model: DEFAULT_MODEL.to_string(), output_format: CliOutputFormat::Text, allowed_tools: None, @@ -9279,7 +9447,9 @@ mod tests { assert!(help.contains("/diff")); assert!(help.contains("/version")); assert!(help.contains("/export [file]")); - assert!(help.contains("/session [list|switch |fork [branch-name]]")); + // Batch 5 added `/session delete`; match on the stable core rather than + // the trailing bracket so future additions don't re-break this. + assert!(help.contains("/session [list|switch |fork [branch-name]")); assert!(help.contains( "/plugin [list|install |enable |disable |uninstall |update ]" )); @@ -9372,8 +9542,7 @@ mod tests { std::env::remove_var("ANTHROPIC_MODEL"); std::env::set_var("ANTHROPIC_MODEL", "sonnet"); - let resolved = - with_current_dir(&root, || resolve_repl_model(DEFAULT_MODEL.to_string())); + let resolved = with_current_dir(&root, || resolve_repl_model(DEFAULT_MODEL.to_string())); assert_eq!(resolved, "claude-sonnet-4-6"); @@ -9392,8 +9561,7 @@ mod tests { std::env::set_var("CLAW_CONFIG_HOME", &config_home); std::env::remove_var("ANTHROPIC_MODEL"); - let resolved = - with_current_dir(&root, || resolve_repl_model(DEFAULT_MODEL.to_string())); + let resolved = with_current_dir(&root, || resolve_repl_model(DEFAULT_MODEL.to_string())); assert_eq!(resolved, DEFAULT_MODEL); @@ -10758,6 +10926,9 @@ UU conflicted.rs", #[test] fn build_runtime_runs_plugin_lifecycle_init_and_shutdown() { + // Serialize access to process-wide env vars so parallel tests that + // set/remove ANTHROPIC_API_KEY do not race with this test. + let _guard = env_lock(); let config_home = temp_dir(); // Inject a dummy API key so runtime construction succeeds without real credentials. // This test only exercises plugin lifecycle (init/shutdown), never calls the API. diff --git a/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs b/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs index 4091613f..066abb68 100644 --- a/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs +++ b/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs @@ -183,17 +183,24 @@ fn clean_env_cli_reaches_mock_anthropic_service_across_scripted_parity_scenarios } let captured = runtime.block_on(server.captured_requests()); - assert_eq!( - captured.len(), - 21, - "twelve scenarios should produce twenty-one requests" - ); - assert!(captured + // After `be561bf` added count_tokens preflight, each turn sends an + // extra POST to `/v1/messages/count_tokens` before the messages POST. + // The original count (21) assumed messages-only requests. We now + // filter to `/v1/messages` and verify that subset matches the original + // scenario expectation. + let messages_only: Vec<_> = captured .iter() - .all(|request| request.path == "/v1/messages")); - assert!(captured.iter().all(|request| request.stream)); + .filter(|r| r.path == "/v1/messages") + .collect(); + assert_eq!( + messages_only.len(), + 21, + "twelve scenarios should produce twenty-one /v1/messages requests (total captured: {}, includes count_tokens)", + captured.len() + ); + assert!(messages_only.iter().all(|request| request.stream)); - let scenarios = captured + let scenarios = messages_only .iter() .map(|request| request.scenario.as_str()) .collect::>(); diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index 9366963b..84eadc07 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -963,6 +963,21 @@ pub fn mvp_tool_specs() -> Vec { }), required_permission: PermissionMode::DangerFullAccess, }, + ToolSpec { + name: "WorkerObserveCompletion", + description: "Report session completion to the worker, classifying finish_reason into Finished or Failed (provider-degraded). Use after the opencode session completes to advance the worker to its terminal state.", + input_schema: json!({ + "type": "object", + "properties": { + "worker_id": { "type": "string" }, + "finish_reason": { "type": "string" }, + "tokens_output": { "type": "integer", "minimum": 0 } + }, + "required": ["worker_id", "finish_reason", "tokens_output"], + "additionalProperties": false + }), + required_permission: PermissionMode::DangerFullAccess, + }, ToolSpec { name: "TeamCreate", description: "Create a team of sub-agents for parallel task execution.", @@ -1229,6 +1244,8 @@ fn execute_tool_with_enforcer( } "WorkerRestart" => from_value::(input).and_then(run_worker_restart), "WorkerTerminate" => from_value::(input).and_then(run_worker_terminate), + "WorkerObserveCompletion" => from_value::(input) + .and_then(run_worker_observe_completion), "TeamCreate" => from_value::(input).and_then(run_team_create), "TeamDelete" => from_value::(input).and_then(run_team_delete), "CronCreate" => from_value::(input).and_then(run_cron_create), @@ -1427,9 +1444,20 @@ fn run_task_output(input: TaskIdInput) -> Result { #[allow(clippy::needless_pass_by_value)] fn run_worker_create(input: WorkerCreateInput) -> Result { + // Merge config-level trusted_roots with per-call overrides. + // Config provides the default allowlist; per-call roots add on top. + let config_roots: Vec = ConfigLoader::default_for(&input.cwd) + .load() + .ok() + .map(|c| c.trusted_roots().to_vec()) + .unwrap_or_default(); + let merged_roots: Vec = config_roots + .into_iter() + .chain(input.trusted_roots.iter().cloned()) + .collect(); let worker = global_worker_registry().create( &input.cwd, - &input.trusted_roots, + &merged_roots, input.auto_recover_prompt_misdelivery, ); to_pretty_json(worker) @@ -1479,6 +1507,16 @@ fn run_worker_terminate(input: WorkerIdInput) -> Result { to_pretty_json(worker) } +#[allow(clippy::needless_pass_by_value)] +fn run_worker_observe_completion(input: WorkerObserveCompletionInput) -> Result { + let worker = global_worker_registry().observe_completion( + &input.worker_id, + &input.finish_reason, + input.tokens_output, + )?; + to_pretty_json(worker) +} + #[allow(clippy::needless_pass_by_value)] fn run_team_create(input: TeamCreateInput) -> Result { let task_ids: Vec = input @@ -2213,6 +2251,13 @@ struct WorkerIdInput { worker_id: String, } +#[derive(Debug, Deserialize)] +struct WorkerObserveCompletionInput { + worker_id: String, + finish_reason: String, + tokens_output: u64, +} + #[derive(Debug, Deserialize)] struct WorkerObserveInput { worker_id: String, @@ -3777,7 +3822,8 @@ impl ApiClient for ProviderRuntimeClient { }) .collect::>(); let messages = convert_messages(&request.messages); - let system = (!request.system_prompt.is_empty()).then(|| request.system_prompt.join("\n\n")); + let system = + (!request.system_prompt.is_empty()).then(|| request.system_prompt.join("\n\n")); let tool_choice = (!self.allowed_tools.is_empty()).then_some(ToolChoice::Auto); let runtime = &self.runtime; @@ -3792,6 +3838,7 @@ impl ApiClient for ProviderRuntimeClient { tools: (!tools.is_empty()).then(|| tools.clone()), tool_choice: tool_choice.clone(), stream: true, + ..Default::default() }; let attempt = runtime.block_on(stream_with_provider(&entry.client, &message_request)); @@ -5329,8 +5376,8 @@ mod tests { GlobalToolRegistry, LaneEventName, LaneFailureClass, ProviderRuntimeClient, SubagentToolExecutor, }; - use runtime::ProviderFallbackConfig; use api::OutputContentBlock; + use runtime::ProviderFallbackConfig; use runtime::{ permission_enforcer::PermissionEnforcer, ApiRequest, AssistantEvent, ConversationRuntime, PermissionMode, PermissionPolicy, RuntimeError, Session, TaskPacket, ToolExecutor, @@ -5506,6 +5553,436 @@ mod tests { assert_eq!(accepted_output["prompt_in_flight"], true); } + #[test] + fn worker_create_merges_config_trusted_roots_without_per_call_override() { + use std::fs; + // Write a .claw/settings.json in a temp dir with trustedRoots + let worktree = temp_path("config-trust-worktree"); + let claw_dir = worktree.join(".claw"); + fs::create_dir_all(&claw_dir).expect("create .claw dir"); + // Use the actual OS temp dir so the worktree path matches the allowlist + let tmp_root = std::env::temp_dir().to_str().expect("utf-8").to_string(); + let settings = format!("{{\"trustedRoots\": [\"{tmp_root}\"]}}"); + fs::write(claw_dir.join("settings.json"), settings).expect("write settings"); + + // WorkerCreate with no per-call trusted_roots — config should supply them + let cwd = worktree.to_str().expect("valid utf-8").to_string(); + let created = execute_tool( + "WorkerCreate", + &json!({ + "cwd": cwd + // trusted_roots intentionally omitted + }), + ) + .expect("WorkerCreate should succeed"); + let output: serde_json::Value = serde_json::from_str(&created).expect("json"); + + // worktree is under /tmp, so config roots auto-resolve trust + assert_eq!( + output["trust_auto_resolve"], true, + "config-level trustedRoots should auto-resolve trust without per-call override" + ); + + fs::remove_dir_all(&worktree).ok(); + } + + #[test] + fn worker_terminate_sets_finished_status() { + // Create a worker in running state + let created = execute_tool( + "WorkerCreate", + &json!({"cwd": "/tmp/terminate-test", "trusted_roots": ["/tmp"]}), + ) + .expect("WorkerCreate should succeed"); + let output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = output["worker_id"].as_str().expect("worker_id").to_string(); + + // Terminate + let terminated = execute_tool("WorkerTerminate", &json!({"worker_id": worker_id})) + .expect("WorkerTerminate should succeed"); + let term_output: serde_json::Value = serde_json::from_str(&terminated).expect("json"); + assert_eq!( + term_output["status"], "finished", + "terminated worker should be finished" + ); + assert_eq!( + term_output["prompt_in_flight"], false, + "prompt_in_flight should be cleared on termination" + ); + } + + #[test] + fn worker_restart_resets_to_spawning() { + // Create and advance worker to ready_for_prompt + let created = execute_tool( + "WorkerCreate", + &json!({"cwd": "/tmp/restart-test", "trusted_roots": ["/tmp"]}), + ) + .expect("WorkerCreate should succeed"); + let output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = output["worker_id"].as_str().expect("worker_id").to_string(); + + // Advance to ready_for_prompt via observe + execute_tool( + "WorkerObserve", + &json!({"worker_id": worker_id, "screen_text": "Ready for input\n>"}), + ) + .expect("WorkerObserve should succeed"); + + // Restart + let restarted = execute_tool("WorkerRestart", &json!({"worker_id": worker_id})) + .expect("WorkerRestart should succeed"); + let restart_output: serde_json::Value = serde_json::from_str(&restarted).expect("json"); + assert_eq!( + restart_output["status"], "spawning", + "restarted worker should return to spawning" + ); + assert_eq!( + restart_output["prompt_in_flight"], false, + "prompt_in_flight should be cleared on restart" + ); + assert_eq!( + restart_output["trust_gate_cleared"], false, + "trust_gate_cleared should be reset on restart (re-trust required)" + ); + } + + #[test] + fn worker_get_returns_worker_state() { + let created = execute_tool( + "WorkerCreate", + &json!({"cwd": "/tmp/worker-get-test", "trusted_roots": ["/tmp"]}), + ) + .expect("WorkerCreate should succeed"); + let created_output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = created_output["worker_id"].as_str().expect("worker_id"); + + let fetched = execute_tool("WorkerGet", &json!({"worker_id": worker_id})) + .expect("WorkerGet should succeed"); + let fetched_output: serde_json::Value = serde_json::from_str(&fetched).expect("json"); + assert_eq!(fetched_output["worker_id"], worker_id); + assert_eq!(fetched_output["status"], "spawning"); + assert_eq!(fetched_output["cwd"], "/tmp/worker-get-test"); + } + + #[test] + fn worker_get_on_unknown_id_returns_error() { + let result = execute_tool( + "WorkerGet", + &json!({"worker_id": "worker_nonexistent_get_00000000"}), + ); + assert!( + result.is_err(), + "WorkerGet on unknown id should return error" + ); + assert!( + result.unwrap_err().contains("worker not found"), + "error should mention worker not found" + ); + } + + #[test] + fn worker_await_ready_on_spawning_worker_returns_not_ready() { + let created = execute_tool( + "WorkerCreate", + &json!({"cwd": "/tmp/worker-await-not-ready"}), + ) + .expect("WorkerCreate should succeed"); + let created_output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = created_output["worker_id"].as_str().expect("worker_id"); + + // Worker is still in spawning — await_ready should return not-ready snapshot + let snapshot = execute_tool("WorkerAwaitReady", &json!({"worker_id": worker_id})) + .expect("WorkerAwaitReady should succeed even when not ready"); + let snap_output: serde_json::Value = serde_json::from_str(&snapshot).expect("json"); + assert_eq!( + snap_output["ready"], false, + "WorkerAwaitReady on a spawning worker must return ready=false" + ); + assert_eq!(snap_output["worker_id"], worker_id); + } + + #[test] + fn worker_send_prompt_on_non_ready_worker_returns_error() { + let created = execute_tool( + "WorkerCreate", + &json!({"cwd": "/tmp/worker-send-not-ready"}), + ) + .expect("WorkerCreate should succeed"); + let created_output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = created_output["worker_id"].as_str().expect("worker_id"); + + let result = execute_tool( + "WorkerSendPrompt", + &json!({"worker_id": worker_id, "prompt": "too early"}), + ); + assert!( + result.is_err(), + "WorkerSendPrompt on a non-ready worker should fail" + ); + } + + #[test] + fn recovery_loop_state_file_reflects_transitions() { + // End-to-end proof: .claw/worker-state.json reflects every transition + // through the stall-detect -> resolve-trust -> ready loop. + use std::fs; + + // Use a real temp CWD so state file can be written + let worktree = temp_path("recovery-loop-state"); + fs::create_dir_all(&worktree).expect("create worktree"); + let cwd = worktree.to_str().expect("utf-8").to_string(); + let state_path = worktree.join(".claw").join("worker-state.json"); + + // 1. Create worker WITHOUT trusted_roots + let created = execute_tool("WorkerCreate", &json!({"cwd": cwd})) + .expect("WorkerCreate should succeed"); + let created_output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = created_output["worker_id"] + .as_str() + .expect("worker_id") + .to_string(); + // State file should exist after create + assert!( + state_path.exists(), + "state file should be written after WorkerCreate" + ); + let state: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&state_path).expect("read state")) + .expect("parse state"); + assert_eq!(state["status"], "spawning"); + assert_eq!(state["is_ready"], false); + assert!( + state["seconds_since_update"].is_number(), + "seconds_since_update must be present" + ); + + // 2. Force trust_required via observe + execute_tool( + "WorkerObserve", + &json!({"worker_id": worker_id, "screen_text": "Do you trust the files in this folder?"}), + ) + .expect("WorkerObserve should succeed"); + let state: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&state_path).expect("read state")) + .expect("parse state"); + assert_eq!( + state["status"], "trust_required", + "state file must reflect trust_required stall" + ); + assert_eq!(state["is_ready"], false); + assert_eq!(state["trust_gate_cleared"], false); + assert!(state["seconds_since_update"].is_number()); + + // 3. WorkerResolveTrust -> state file reflects recovery + execute_tool("WorkerResolveTrust", &json!({"worker_id": worker_id})) + .expect("WorkerResolveTrust should succeed"); + let state: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&state_path).expect("read state")) + .expect("parse state"); + assert_eq!( + state["status"], "spawning", + "state file must show spawning after trust resolved" + ); + assert_eq!(state["trust_gate_cleared"], true); + + // 4. Observe ready screen -> state file shows ready_for_prompt + execute_tool( + "WorkerObserve", + &json!({"worker_id": worker_id, "screen_text": "Ready for input\n>"}), + ) + .expect("WorkerObserve ready should succeed"); + let state: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&state_path).expect("read state")) + .expect("parse state"); + assert_eq!( + state["status"], "ready_for_prompt", + "state file must show ready_for_prompt after ready screen" + ); + assert_eq!( + state["is_ready"], true, + "is_ready must be true in state file at ready_for_prompt" + ); + + fs::remove_dir_all(&worktree).ok(); + } + + #[test] + fn stall_detect_and_resolve_trust_end_to_end() { + // 1. Create worker WITHOUT trusted_roots so trust won't auto-resolve + let created = execute_tool("WorkerCreate", &json!({"cwd": "/no/trusted/root/here"})) + .expect("WorkerCreate should succeed"); + let created_output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = created_output["worker_id"] + .as_str() + .expect("worker_id") + .to_string(); + assert_eq!(created_output["trust_auto_resolve"], false); + + // 2. Observe trust prompt screen text -> worker stalls at trust_required + let stalled = execute_tool( + "WorkerObserve", + &json!({ + "worker_id": worker_id, + "screen_text": "Do you trust the files in this folder?\n[Allow] [Deny]" + }), + ) + .expect("WorkerObserve should succeed"); + let stalled_output: serde_json::Value = serde_json::from_str(&stalled).expect("json"); + assert_eq!( + stalled_output["status"], "trust_required", + "worker should stall at trust_required when trust prompt seen without allowlist" + ); + assert_eq!(stalled_output["trust_gate_cleared"], false); + // 3. Clawhip calls WorkerResolveTrust to unblock + let resolved = execute_tool("WorkerResolveTrust", &json!({"worker_id": worker_id})) + .expect("WorkerResolveTrust should succeed"); + let resolved_output: serde_json::Value = serde_json::from_str(&resolved).expect("json"); + assert_eq!( + resolved_output["status"], "spawning", + "worker should return to spawning after trust resolved" + ); + assert_eq!(resolved_output["trust_gate_cleared"], true); + + // 4. Ready screen text now advances worker normally + let ready = execute_tool( + "WorkerObserve", + &json!({ + "worker_id": worker_id, + "screen_text": "Ready for input\n>" + }), + ) + .expect("WorkerObserve should succeed after trust resolved"); + let ready_output: serde_json::Value = serde_json::from_str(&ready).expect("json"); + assert_eq!( + ready_output["status"], "ready_for_prompt", + "worker should reach ready_for_prompt after trust resolved and ready screen seen" + ); + } + + #[test] + fn stall_detect_and_restart_recovery_end_to_end() { + // Worker stalls at trust_required, clawhip restarts instead of resolving + let created = execute_tool( + "WorkerCreate", + &json!({"cwd": "/no/trusted/root/restart-test"}), + ) + .expect("WorkerCreate should succeed"); + let created_output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = created_output["worker_id"] + .as_str() + .expect("worker_id") + .to_string(); + + // Force trust_required + let stalled = execute_tool( + "WorkerObserve", + &json!({ + "worker_id": worker_id, + "screen_text": "trust this folder? [Yes] [No]" + }), + ) + .expect("WorkerObserve should succeed"); + let stalled_output: serde_json::Value = serde_json::from_str(&stalled).expect("json"); + assert_eq!(stalled_output["status"], "trust_required"); + + // WorkerRestart resets the worker + let restarted = execute_tool("WorkerRestart", &json!({"worker_id": worker_id})) + .expect("WorkerRestart should succeed"); + let restarted_output: serde_json::Value = serde_json::from_str(&restarted).expect("json"); + assert_eq!( + restarted_output["status"], "spawning", + "restarted worker should be back at spawning" + ); + assert_eq!( + restarted_output["trust_gate_cleared"], false, + "restart clears trust — next observe loop must re-acquire trust" + ); + } + + #[test] + fn worker_terminate_on_unknown_id_returns_error() { + let result = execute_tool( + "WorkerTerminate", + &json!({"worker_id": "worker_nonexistent_00000000"}), + ); + assert!(result.is_err(), "terminating unknown worker should fail"); + assert!( + result.unwrap_err().contains("worker not found"), + "error should mention worker not found" + ); + } + + #[test] + fn worker_restart_on_unknown_id_returns_error() { + let result = execute_tool( + "WorkerRestart", + &json!({"worker_id": "worker_nonexistent_00000001"}), + ); + assert!(result.is_err(), "restarting unknown worker should fail"); + assert!( + result.unwrap_err().contains("worker not found"), + "error should mention worker not found" + ); + } + + #[test] + fn worker_observe_completion_success_finish_sets_finished_status() { + let created = execute_tool( + "WorkerCreate", + &json!({"cwd": "/tmp/observe-completion-test", "trusted_roots": ["/tmp"]}), + ) + .expect("WorkerCreate should succeed"); + let output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = output["worker_id"].as_str().expect("worker_id").to_string(); + + let completed = execute_tool( + "WorkerObserveCompletion", + &json!({ + "worker_id": worker_id, + "finish_reason": "end_turn", + "tokens_output": 512 + }), + ) + .expect("WorkerObserveCompletion should succeed"); + let completed_output: serde_json::Value = serde_json::from_str(&completed).expect("json"); + assert_eq!(completed_output["status"], "finished"); + assert_eq!(completed_output["prompt_in_flight"], false); + } + + #[test] + fn worker_observe_completion_degraded_provider_sets_failed_status() { + let created = execute_tool( + "WorkerCreate", + &json!({"cwd": "/tmp/observe-degraded-test", "trusted_roots": ["/tmp"]}), + ) + .expect("WorkerCreate should succeed"); + let output: serde_json::Value = serde_json::from_str(&created).expect("json"); + let worker_id = output["worker_id"].as_str().expect("worker_id").to_string(); + + // finish=unknown + 0 tokens = degraded provider classification + let failed = execute_tool( + "WorkerObserveCompletion", + &json!({ + "worker_id": worker_id, + "finish_reason": "unknown", + "tokens_output": 0 + }), + ) + .expect("WorkerObserveCompletion should succeed"); + let failed_output: serde_json::Value = serde_json::from_str(&failed).expect("json"); + assert_eq!( + failed_output["status"], "failed", + "finish=unknown + 0 tokens should classify as provider failure" + ); + assert_eq!(failed_output["prompt_in_flight"], false); + // last_error should be set with provider failure message + assert!( + !failed_output["last_error"].is_null(), + "last_error should be populated for provider failure" + ); + } + #[test] fn worker_tools_detect_misdelivery_and_arm_prompt_replay() { let created = execute_tool( @@ -5772,6 +6249,14 @@ mod tests { #[test] fn web_search_extracts_and_filters_results() { + // Serialize env-var mutation so this test cannot race with the sibling + // web_search_handles_generic_links_and_invalid_base_url test that also + // sets CLAWD_WEB_SEARCH_BASE_URL. Without the lock, parallel test + // runners can interleave the set/remove calls and cause assertion + // failures on the wrong port. + let _guard = env_lock() + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let server = TestServer::spawn(Arc::new(|request_line: &str| { assert!(request_line.contains("GET /search?q=rust+web+search ")); HttpResponse::html(