Compare commits

..

6 Commits

Author SHA1 Message Date
Yeachan-Heo
61fd7cfec5 Lock in Linux hook stdin BrokenPipe coverage
Latest main already contains the functional BrokenPipe tolerance in
plugins::hooks::CommandWithStdin::output_with_stdin, but the only
coverage for the original CI failure was the higher-level plugin hook
test. Add a deterministic regression that exercises the exact low-level
EPIPE path by spawning a hook child that closes stdin immediately while
the parent writes an oversized payload.

This keeps the real root cause explicit: Linux surfaced BrokenPipe from
the parent's stdin write after the hook child closed fd 0 early. Missing
execute bits were not the primary bug.

Constraint: Keep the change surgical on top of latest main
Rejected: Re-open the production code path | latest main already contains the runtime fix
Rejected: Inflate HookRunner payloads in the regression | HOOK_* env injection hit ARG_MAX before the pipe path
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep BrokenPipe coverage near CommandWithStdin so future refactors do not regress the Linux EPIPE path
Tested: cargo test -p plugins hooks::tests::collects_and_runs_hooks_from_enabled_plugins -- --exact (10x)
Tested: cargo test -p plugins hooks::tests::output_with_stdin_tolerates_broken_pipe_when_child_closes_stdin_early -- --exact (10x)
Tested: cargo test --workspace
Not-tested: GitHub Actions rerun on the PR branch
2026-04-08 11:04:27 +00:00
Jobdori
5c69713158 docs(roadmap): file OpenAI-compat model-id passthrough gap as #32 2026-04-08 19:48:34 +09:00
Jobdori
939d0dbaa3 docs(roadmap): file OpenAI-compat model-id passthrough gap as #32 2026-04-08 19:48:28 +09:00
Jobdori
bfd5772716 docs(roadmap): file OpenAI-compat model-id passthrough gap as #32 2026-04-08 19:48:21 +09:00
Jobdori
e0c3ff1673 docs(roadmap): file executor-contract leaks as ROADMAP #31 2026-04-08 18:34:58 +09:00
Jobdori
252536be74 fix(tools): serialize web_search env-var tests with env_lock to prevent race
web_search_extracts_and_filters_results set CLAWD_WEB_SEARCH_BASE_URL
without holding env_lock(), while the sibling test
web_search_handles_generic_links_and_invalid_base_url always held it.
Under parallel test execution the two tests interleave set_var/remove_var
calls, pointing the search client at the wrong mock server port and
causing assertion failures.

Fix: add env_lock() guard at the top of web_search_extracts_and_filters_results,
matching the serialization pattern already used by every other env-mutating
test in this module.

Root cause of CI flake on run 24127551802.
Identified and fixed during dogfood session.
2026-04-08 18:34:06 +09:00
3 changed files with 55 additions and 0 deletions

View File

@@ -480,3 +480,11 @@ Model name prefix now wins unconditionally over env-var presence. Regression tes
**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 <file>` 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.

View File

@@ -561,4 +561,43 @@ mod tests {
);
}
}
#[test]
fn output_with_stdin_tolerates_broken_pipe_when_child_closes_stdin_early() {
// given: a hook that immediately closes stdin without consuming the
// JSON payload. Use an oversized payload so the parent keeps writing
// long enough for Linux to surface EPIPE on the old implementation.
let root = temp_dir("stdin-close");
let script = root.join("close-stdin.sh");
fs::create_dir_all(&root).expect("temp hook dir");
fs::write(
&script,
"#!/bin/sh\nexec 0<&-\nprintf 'stdin closed early\\n'\nsleep 0.05\n",
)
.expect("write stdin-closing hook");
make_executable(&script);
let mut child = super::shell_command(script.to_str().expect("utf8 path"));
child.stdin(std::process::Stdio::piped());
child.stdout(std::process::Stdio::piped());
child.stderr(std::process::Stdio::piped());
let large_input = vec![b'x'; 2 * 1024 * 1024];
// when
let output = child
.output_with_stdin(&large_input)
.expect("broken pipe should be tolerated");
// then
assert!(
output.status.success(),
"child should still exit cleanly: {output:?}"
);
assert_eq!(
String::from_utf8_lossy(&output.stdout).trim(),
"stdin closed early"
);
let _ = fs::remove_dir_all(root);
}
}

View File

@@ -6249,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(