Repository navigation
feat(setup): separate env namespace for custom OpenAI-compat provider - #3248
TheArchitectit wants to merge 770 commits into
Conversation
…ath_is_directory kind + hint; wire fallback_hint_for_error_kind into both resume error emission sites
…rror envelope; use exit(1) instead of Err propagation
…t 1; parity with skills (ultraworkers#788) and mcp (ultraworkers#68)
… typed unknown_option kind + non-null hint
…nt via \n-delimited usage string
…pty success; now returns unknown_option error
… not-found hint:null
…lugin_source_not_found instead of unknown+null
…now return non-null hints via fallback table
…ng not-found instead of unexpected_extra_args
…limited usage string Parity with ultraworkers#791 (config extra-arg fix). The plugins arg parser emitted 'unexpected extra arguments after claw plugins show ...' with no newline delimiter, so split_error_hint returned None. Added usage hint after newline. 60 CLI contract tests pass.
…ed usage string claw '' and claw ' ' returned empty_prompt + hint:null because the error message had no newline delimiter. Added usage hint. 61 CLI contract tests pass.
…tion classifier arms Two classifier arms had no corresponding assert_eq! in test_classify_error_kind_returns_correct_discriminants: invalid_history_count (both prefix and contains paths) and unknown_option (ultraworkers#790). Now 49/39 = full coverage of all classify_error_kind return values.
…(plugins extra-arg, empty-prompt, classifier coverage)
…rror_kind, hint, and message fields
… now include hint field
… silently returned empty success
… returned wrong error instead of unexpected_extra_args
…returned empty success instead of error
…ltraworkers#3161) Keep malformed diff invocations with trailing JSON format flags on the parser error path and lock the contract with focused output-format regressions. Constraint: Do not touch tracked .omx state files. Rejected: Repeating direct binary smoke loops | local auth/provider configuration intercepts those invocations and obscures parser behavior. Confidence: high Scope-risk: narrow Tested: git diff --check; cargo fmt --check; cargo test -p rusty-claude-cli diff_extra_args_have_typed_error_kind_and_hint_766 --test output_format_contract; cargo test -p rusty-claude-cli diff_trailing_json_after_malformed_args_is_bounded_json_3129 --test output_format_contract; cargo test -p rusty-claude-cli diff_non_git_dir_has_error_kind_and_hint_801 --test output_format_contract
… empty success instead of error
…ailures Extend auto-compaction error detection to handle additional error patterns from llama.cpp backends: 'Context size has been exceeded', 'exceed_context_size_error', 'exceeds the available context size'. Also recover from reqwest 'error decoding response body' errors — some llama.cpp instances return a non-SSE plaintext HTTP 500 on context overflow, causing the SSE deserializer to fail. Add dynamic threshold adaptation: parse server-reported context window size from error messages (e.g., '(81920 tokens)') and set the auto- compaction trigger at 70% of that value. This replaces the need for a hardcoded threshold, adapting automatically to any backend's limits. This patch was developed with assistance from OpenCode and local Qwen 3.6 API server.
|
Good separation of concerns — having a dedicated env namespace avoids collisions with standard OpenAI vars. The migration path for existing users looks clean. Thanks @TheArchitectit! |
|
Ready for review ✅
Once this lands, #3250 will be rebased to drop commits 1-3 and should be ready to merge as well. @ultraworkers/maintainers — requesting review when you get a chance. |
|
The custom-openai namespace separation is clean and well-tested. The custom/ prefix routing is a nice touch that keeps model IDs unambiguous. The migration path for existing users (re-run /setup) is documented. All checks passing, good to merge. |
|
Rebased cleanly and the custom-openai namespace approach is solid. All checks green — merging. Thanks for the thorough work @TheArchitectit! |
|
Thank you! Once this merges I'll start work on the follow-up PR (#3250) — I'll rebase it to drop the first three commits as you noted. More PRs to come as well! |
|
Merged, thanks! Looking forward to #3250 — the custom-openai namespace work is a solid foundation. Will keep an eye on the follow-up PRs for review. |
|
Separating env namespace for custom OpenAI-compatible providers is a clean design choice. This avoids collision with the built-in OpenAI provider config and makes the setup more predictable. |
No behavior change. Move the inline probe out of unshare_user_namespace_works into a reusable unshare_probe helper and a cached working_unshare_mapping() that picks the first working candidate from UNSHARE_MAPPING_CANDIDATES, so the launcher and the capability probe share one code path.
…icted Plain `unshare --user --map-root-user` fails on kernels and containers that block unprivileged writes to /proc/self/uid_map (e.g. GitHub Actions, restricted AppArmor profiles). On those systems util-linux delegates to the setuid newuidmap/newgidmap helpers when --map-auto is also present. Add the combined form as a fallback candidate and build the launcher args from the probed mapping, so systems without newuidmap/newgidmap or a /etc/subuid range keep using the plain form.
… fallback The fallback candidate relies on the setuid newuidmap/newgidmap helpers (uidmap package) plus a subuid/subgid range for the current user. Note in the candidate docs that the startup probe rejects the candidate when those are missing, so the plain --map-root-user form is used instead.
The startup probe validated only the mapping flags against the trivial program `true`, but the real launcher always adds --mount --ipc --pid --uts --fork. On environments where the user namespace is created but mount propagation inside it is restricted (e.g. AppArmor-restricted CI runners), the fallback mapping passed the probe and the sandbox activated, yet every sandboxed command died with "cannot change root filesystem propagation: Permission denied", silently returning empty tool output and breaking the mock parity suite. The candidates now define the complete static launcher shape (mapping flags + namespace flags), so probe success implies launch success; the launcher reuses the candidate instead of re-appending the namespace flags, keeping probe and launch as one source of truth. The order-guarding test asserts the namespace flags are present in every candidate. Co-authored-by: linkst <2024023709@m.scnu.edu.cn>
…ap-auto-fallback fix(sandbox): fall back to --map-auto when root-user mapping is restricted
Root knowledge base plus complexity-scored subdirectory files for the rust/ workspace, its five highest-mass crates (runtime, rusty-claude-cli, api, tools, commands, plugins), and the src/ Python porting workspace. Generated via init-deep: 13 parallel explore agents, LSP/ast-grep code map, centrality-scored placement. Snapshot in .omo/init-deep.json (local).
|
Friendly ping — this is a small, self-contained change (3 commits, 6 files): a dedicated It has been CLEAN/mergeable for a while, and PR #3250 (team enhancements) is queued on top of it — its final review note says "ready once #3248 lands and the base commits are rebased away." Happy to rebase onto current main if that helps move it along — just say the word. |
The /setup wizard saves apiKey and baseUrl to ~/.claw/settings.json, but the API client constructors (OpenAiCompatClient::from_env, AnthropicClient::from_env) only read environment variables. This caused saved provider settings to be silently ignored — you'd run /setup, set a custom URL and API key, and the runtime would still try to use the default endpoint. Now AnthropicRuntimeClient::new() calls inject_config_as_env_fallbacks() before constructing the API client. This function loads the config file's provider settings and sets the corresponding env vars (OPENAI_API_KEY, OPENAI_BASE_URL, etc.) only when they aren't already set — preserving the 3-tier resolution order: env var > .env file > stored config. This is a process-level env injection (set_var), so it only affects the current claw process and its children, not the parent shell.
…oints - Move config-to-env injection out of AnthropicRuntimeClient::new so parallel unit tests are not affected by global env mutations. - Call inject_config_as_env_fallbacks() once at binary startup in run(), preserving the env-var > .env > stored-config precedence. - Normalize bare model names (e.g. openclaw) to openai/openclaw when a custom OpenAI-compatible base URL is configured, so validation passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Custom (OpenAI-compat) /setup option was saving kind: openai and injecting OPENAI_API_KEY / OPENAI_BASE_URL. That collides with users who have real OpenAI/NeuralWatt credentials in their environment. Introduce a dedicated custom-openai provider kind that uses its own environment variables: - CLAWCUSTOMOPENAI_API_KEY - CLAWCUSTOMOPENAI_BASE_URL A new custom/ routing prefix selects the OpenAI-compatible client with those env vars and is stripped on the wire, so the proxy receives the bare model id. /setup now saves kind: custom-openai and prompts for the new env vars. Bare model names saved by /setup are normalized to custom/<model>. Manual verification against http://100.96.49.42:4001/v1 succeeds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code)
0c339e7 to
c4cbd2b
Compare
|
Namespace separation looks good — dedicated CLAWCUSTOMOPENAI_* vars cleanly avoid colliding with real OpenAI credentials. One suggestion: the migration path currently relies on users re-running /setup. Could we print a deprecation warning at startup when an old saved config (kind=openai with custom base_url) is detected, instead of silently ignoring it? That way users whose proxy URL was being ignored get a clear hint they need to migrate. |
|
Thanks — the namespace separation review is appreciated. On the deprecation warning: we dug in, and the framing is slightly off. The real silent failure is shadowing: the A blanket "deprecated config" warning would false-positive on the deliberate proxy setups. We'll land a narrower warning instead: fire only when a stored config value is actually ignored because env already differs, once per process at startup, suppressed in JSON/quiet mode. Will link the follow-up here when it's up. |
|
明白——CLAUCCUSTOMOPENAI_* 命名空间与旧 kind=openai 兼容方案并存是合理的，不会被当作"降级"。我会更新评审结论：baseUrl 仍支持注入 fallback，#3250 合并后无需额外弃用警告。该 PR 自包含且 CLEAN，可以合。 |
|
Thanks for the quick follow-up and for clarifying the namespace decision — good to know the |
|
收到，确认可以合并。这个命名空间方案我会在合并后的运行时回归里把 CLAUCCUSTOMOPENAI_* 与旧 kind=openai 共存的场景一并覆盖，确保 fallback 注入行为不回归。#3250 等你 rebase 掉前三个 commit 后提上来，我继续跟进评审。 |
|
Merged — thanks for clarifying the shadowing behavior. Good to know the real silent-failure case is the std::env::var guards skipping the stored proxy baseUrl whenever real OPENAI_* vars exist, not the namespacing itself. I've added that coexistence scenario (CLAWCUSTOMOPENAI_* alongside legacy kind=openai) to the post-merge regression coverage so the fallback-injection behavior stays intact. With this in, #3250's rebase is now unblocked — I'll continue that review. |
|
合并完成，感谢确认。落地的回归覆盖我会具体断言三点：① fallback 注入顺序保持 env var > .env > stored config 不变；② 影子场景——存在真实 OPENAI_* 环境变量时，stored model 仍能经 CLAWCUSTOMOPENAI_* 正确路由到 custom/ 前缀；③ 与旧 kind=openai 共存时互不污染。跑通后我把结果贴到 #3250 的评审里。 |
|
Quick status check on this one, since it directly gates #3250. As of now this PR is still open: So the merge-order block is still physically in place, and the test restore for #3250 can't be validated yet: Meanwhile #3250 is already in the shape that was asked for: the 8 team commits only, rebased onto current I pre-ran the #3250 rebase locally against a simulated
Resolution is mechanical: take #3250's Proposed order: merge this PR → rebase #3250 onto the new If the merge landed somewhere other than |
c4cbd2b to
eafa950
Compare
12 participants
Problem
The
/setupwizard's "Custom (OpenAI-compat)" option savedkind: "openai"and injectedOPENAI_API_KEY/OPENAI_BASE_URL. That collides with users who already have real OpenAI, NeuralWatt, or other platform credentials in their environment, so the saved custom proxy URL was ignored and requests were misrouted.Changes
New provider kind:
custom-openaiCLAWCUSTOMOPENAI_API_KEYCLAWCUSTOMOPENAI_BASE_URLcustom/selects the OpenAI-compatible client with those env vars and is stripped on the wire, so the proxy receives the bare model id./setupwizard updatedkind: "custom-openai".CLAWCUSTOMOPENAI_API_KEY/CLAWCUSTOMOPENAI_BASE_URL./setupare normalized tocustom/<model>.Saved settings are applied at startup
inject_config_as_env_fallbacks()is called once inrun()before any runtime threads are spawned..envfile > stored config.API routing
metadata_for_modelanddetect_provider_kindrecognizecustom/.OpenAiCompatConfig::custom_openai()reads the new env vars.wire_model_for_base_urlstripscustom/prefix.Tests
custom/prefix routes toCLAWCUSTOMOPENAI_*env vars.custom/is stripped on the wire.inject_config_as_env_fallbackssets both standard and custom env vars.custom/.Docs
USAGE.mdprovider matrix and prefix-routing section./setupwizard section explaining the custom provider.Verification
cargo test -p apipasses.cargo test -p rusty-claude-cli --bin claw config_modelpasses.cargo test -p rusty-claude-cli --bin claw inject_configpasses.http://100.96.49.42:4001/v1with modelopenclaw_3750succeeded.Notes
upstream/mainwhere the TUI changes are not present.kind: "openai"with a custom base URL can re-run/setupto migrate to the new namespace.🤖 Generated with Claude Code
Co-Authored-By: Claude Fable 5 noreply@anthropic.com