feat: add interactive provider wizard with /setup, claw setup, and Ctrl+P
Adds an interactive setup wizard that lets users configure their provider, API key, base URL, and model without setting environment variables. Configuration is persisted to ~/.claw/settings.json (with 0600 permissions). New features: - `claw setup` CLI subcommand runs the wizard from the terminal - `/setup` slash command runs the wizard inside the REPL (hot-swaps model) - Ctrl+P hotkey in the REPL triggers /setup for in-session provider swap - Stored provider config used as fallback when env vars are absent - Three-tier auth resolution: env var > .env file > stored config - RuntimeProviderConfig struct and validation in settings schema Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
TheArchitectit
parent
d9bd37c3a9
commit
da19b7c997
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 9e50cb6e200b2579e2c51f402bf8a39a82cc1c5c
|
||||
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 9115b44f2c21745f5619c948b757d28df25c1cb1
|
||||
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 67da2dcf75ab6383394ebafb56a21b3c9678c580
|
||||
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 77c7d08ab3ecf01016b0e15319c2aec8583d431e
|
||||
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 346772a8b309fb5cd0c030049211571288eaeca8
|
||||
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 4ba51771e5047b02b6fb851764fe9c7b9541b049
|
||||
|
|
@ -0,0 +1 @@
|
|||
Subproject commit a0e9833ff3b577ecbece599b649c80700ec136f8
|
||||
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 2a1d14269407f271d7b7b73aef61b8683c860ded
|
||||
|
|
@ -0,0 +1 @@
|
|||
Subproject commit d54f0280b877d02a846d166fb55ba8f38ec2ad3e
|
||||
|
|
@ -0,0 +1,177 @@
|
|||
# PR #3211 — Execution Plan: 3-Tier Credential Resolution (REVISED)
|
||||
|
||||
**Branch**: `worktree-provider-config-fallback`
|
||||
**Worktree**: `.claude/worktrees/provider-config-fallback`
|
||||
**CI State**: Build ✅ Test ✅ Clippy ✅ Docs ✅ Windows ✅ **Fmt ❌**
|
||||
|
||||
## Reviewer Feedback
|
||||
|
||||
From `1716775457damn`:
|
||||
> "the `from_env_or_saved` naming across multiple provider files could be consolidated
|
||||
> into a shared trait or helper to reduce duplication. Also, cargo fmt is failing"
|
||||
|
||||
## Investigation Findings (Updated)
|
||||
|
||||
### Key discovery: `from_env` vs `from_env_or_saved` are DIFFERENT things
|
||||
|
||||
There are **TWO** `AuthSource` methods in `anthropic.rs`:
|
||||
- `from_env()` (line 44) — **tiers 1+2 only** (process env + .env), NO stored config
|
||||
- `from_env_or_saved()` (line 629) — **tiers 1+2+3** (process env + .env + stored config)
|
||||
|
||||
And TWO in `openai_compat.rs`:
|
||||
- `from_env(config)` (line 132) — **tiers 1+2 only**
|
||||
- `from_env_or_saved(config)` (line 151) — **tiers 1+2+3**
|
||||
|
||||
The `_or_saved` suffix is **semantically meaningful** — it distinguishes the 3-tier path from the 2-tier path. Renaming them identically would lose this distinction.
|
||||
|
||||
### What IS actually duplicated and safe to consolidate
|
||||
|
||||
| Duplication | anthropic.rs | openai_compat.rs | mod.rs (shared) |
|
||||
|---|---|---|---|
|
||||
| `read_env_non_empty(key)` | ✅ line 773 | ✅ line 1608 | ❌ not shared |
|
||||
| `read_base_url()` | ✅ line 798 | ✅ line 1625 | ❌ not shared |
|
||||
|
||||
Both `read_base_url()` functions follow the same 3-tier pattern:
|
||||
```
|
||||
Tier 1: std::env::var(key) → return if non-empty
|
||||
Tier 2: dotenv_value(key) → return if non-empty
|
||||
Tier 3: read_base_url_from_config(provider_kind) → return if Some
|
||||
Default: fallback string
|
||||
```
|
||||
|
||||
But there's a complication: `anthropic::read_base_url()` hardcodes the env var and provider kind, while `openai_compat::read_base_url(config)` takes them from the config struct. The shared helper needs to parameterize both.
|
||||
|
||||
Similarly, `read_env_non_empty` is identical in both files — just reads env + .env fallback.
|
||||
|
||||
### Why NOT a shared trait (confirmed)
|
||||
|
||||
Anthropic's `AuthSource` has 4 variants (None, ApiKey, BearerToken, ApiKeyAndBearer) while OpenAI-compat has only ApiKey. Different return types → trait adds complexity.
|
||||
|
||||
## Execution Plan (Revised)
|
||||
|
||||
### Step 1: Extract `read_env_non_empty()` into `mod.rs`
|
||||
|
||||
Both files have identical `read_env_non_empty(key)` implementations. Move to `mod.rs` as `pub(crate) fn read_env_non_empty(key: &str) -> Result<Option<String>, ApiError>` and update both files to call `super::read_env_non_empty(key)`.
|
||||
|
||||
**Files changed**:
|
||||
- `rust/crates/api/src/providers/mod.rs` — add shared `read_env_non_empty()`
|
||||
- `rust/crates/api/src/providers/anthropic.rs` — remove local `read_env_non_empty()`, use `super::read_env_non_empty()`
|
||||
- `rust/crates/api/src/providers/openai_compat.rs` — remove local `read_env_non_empty()`, use `super::read_env_non_empty()`
|
||||
|
||||
### Step 2: Extract `resolve_base_url()` into `mod.rs`
|
||||
|
||||
Both `read_base_url()` functions follow the identical 3-tier pattern, differing only in the env var name, provider kind, and default value.
|
||||
|
||||
```rust
|
||||
pub fn resolve_base_url(env_var: &str, provider_kind: &str, default: &str) -> String {
|
||||
// Tier 1: environment variable
|
||||
if let Ok(value) = std::env::var(env_var) {
|
||||
if !value.is_empty() {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
// Tier 2: .env file
|
||||
if let Some(value) = dotenv_value(env_var) {
|
||||
if !value.is_empty() {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
// Tier 3: stored config in ~/.claw/settings.json
|
||||
if let Some(base_url) = read_base_url_from_config(provider_kind) {
|
||||
return base_url;
|
||||
}
|
||||
default.to_string()
|
||||
}
|
||||
```
|
||||
|
||||
Then:
|
||||
- `anthropic::read_base_url()` → delegates to `super::resolve_base_url("ANTHROPIC_BASE_URL", "anthropic", DEFAULT_BASE_URL)`
|
||||
- `openai_compat::read_base_url(config)` → delegates to `super::resolve_base_url(config.base_url_env, provider_kind, config.default_base_url)`
|
||||
|
||||
**Files changed**:
|
||||
- `rust/crates/api/src/providers/mod.rs` — add `resolve_base_url()`
|
||||
- `rust/crates/api/src/providers/anthropic.rs` — simplify `read_base_url()` to one-liner delegation
|
||||
- `rust/crates/api/src/providers/openai_compat.rs` — simplify `read_base_url()` to one-liner delegation
|
||||
|
||||
### Step 3: Do NOT rename `from_env_or_saved` → `from_env`
|
||||
|
||||
The `_or_saved` suffix is semantically meaningful and distinguishes the 3-tier path from the 2-tier `from_env()` method. Removing it would conflate two different code paths. The reviewer asked to "reduce duplication" — Steps 1+2 do that by extracting the duplicated logic into shared helpers while preserving the intentional naming distinction.
|
||||
|
||||
### Step 4: Run `cargo fmt --all`
|
||||
|
||||
Fixes the only failing CI check.
|
||||
|
||||
### Step 5: Run `cargo clippy --workspace --all-targets -- -D warnings` + `cargo test --workspace`
|
||||
|
||||
Verify nothing breaks.
|
||||
|
||||
### Step 6: Commit + push
|
||||
|
||||
```
|
||||
refactor: extract shared credential resolution helpers into mod.rs
|
||||
|
||||
Addresses reviewer feedback on #3211 requesting consolidation of
|
||||
duplicated credential-resolution logic across provider files.
|
||||
|
||||
Changes:
|
||||
|
||||
1. Extract read_env_non_empty() into mod.rs
|
||||
- Previously duplicated identically in anthropic.rs (line 773)
|
||||
and openai_compat.rs (line 1608).
|
||||
- Both copies read a process env var, fall back to .env via
|
||||
dotenv_value(), and return Result<Option<String>, ApiError>.
|
||||
- Now defined once as pub(crate) in mod.rs; both provider files
|
||||
call super::read_env_non_empty() instead.
|
||||
|
||||
2. Extract resolve_base_url() into mod.rs
|
||||
- Both anthropic::read_base_url() and openai_compat::read_base_url()
|
||||
implemented the same 3-tier base URL resolution:
|
||||
Tier 1: process env var (e.g. ANTHROPIC_BASE_URL)
|
||||
Tier 2: .env file via dotenv_value()
|
||||
Tier 3: stored config via read_base_url_from_config()
|
||||
Default: provider-specific fallback string
|
||||
- New shared helper: resolve_base_url(env_var, provider_kind, default)
|
||||
- anthropic::read_base_url() now delegates to
|
||||
super::resolve_base_url("ANTHROPIC_BASE_URL", "anthropic", DEFAULT_BASE_URL)
|
||||
- openai_compat::read_base_url(config) now delegates to
|
||||
super::resolve_base_url(config.base_url_env, provider_kind, config.default_base_url)
|
||||
|
||||
3. Preserve from_env_or_saved() naming
|
||||
- The "_or_saved" suffix distinguishes the 3-tier credential path
|
||||
(env + .env + stored config) from the 2-tier from_env() path
|
||||
(env + .env only). Both methods exist on the same AuthSource type
|
||||
and serve different callers. Renaming would lose this semantic
|
||||
distinction.
|
||||
|
||||
CI: cargo fmt --all run to fix the formatting check failure.
|
||||
```
|
||||
|
||||
## Commit Details
|
||||
|
||||
**Commit**: `c0855e12` on `worktree-provider-config-fallback`
|
||||
|
||||
**Files changed**:
|
||||
- `rust/crates/api/src/providers/mod.rs` — added shared `read_env_non_empty()` (+8 lines) and `resolve_base_url()` (+30 lines)
|
||||
- `rust/crates/api/src/providers/anthropic.rs` — replaced local `read_env_non_empty()` and `read_base_url()` with `super::` delegations (-17 lines)
|
||||
- `rust/crates/api/src/providers/openai_compat.rs` — same pattern (-17 lines)
|
||||
- `rust/crates/runtime/src/config.rs` — cargo fmt fix on assert (+5/-1)
|
||||
|
||||
**Net**: +46 insertions, -45 deletions
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Low risk**: Moving identical code into shared functions, no behavior change
|
||||
- **Call sites**: All `from_env_or_saved` / `from_env` call sites are internal to the providers crate
|
||||
- **Public API**: `api::read_base_url()` and `api::read_xai_base_url()` still work — they call the provider-local wrappers which delegate to the shared helper
|
||||
- **No behavior change**: Same code paths, just better organized
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [x] `read_env_non_empty` only defined once (in `mod.rs`)
|
||||
- [x] `resolve_base_url` exists in `mod.rs` and is used by both provider files
|
||||
- [x] `from_env_or_saved` naming preserved (semantic distinction from `from_env`)
|
||||
- [x] `cargo fmt --all` passes
|
||||
- [x] `cargo test -p api` passes (147 unit + 37 integration, all green)
|
||||
- [x] `cargo check -p api` passes
|
||||
- [x] Changes committed and pushed to `worktree-provider-config-fallback` branch (commit `c0855e12`)
|
||||
- [ ] Pre-existing clippy issues in runtime/trident.rs — not from our changes, noted
|
||||
|
|
@ -0,0 +1,97 @@
|
|||
# PR #3214 — Execution Plan: API Timeout/Retry
|
||||
|
||||
**Branch**: `worktree-api-timeout-retry-v2`
|
||||
**Worktree**: `.claude/worktrees/api-timeout-retry-v2`
|
||||
**CI State**: **Build ❌** Test ❌ Fmt ❌ Clippy ✅ Docs ✅ Windows ✅
|
||||
|
||||
## Reviewer Feedback
|
||||
|
||||
From `1716775457damn`:
|
||||
> "Good revival of the stalled #2816. Retry-After header and 400 transient retry are particularly valuable for handling API rate limits. Are defaults preserved so existing behavior is unchanged?"
|
||||
|
||||
## Investigation Findings
|
||||
|
||||
### Root cause of all 3 CI failures: ONE missing field
|
||||
|
||||
**Build ❌**: `error[E0063]: missing field retry_after in initializer of ApiError` at `main.rs:11685`
|
||||
|
||||
A test constructs `ApiError::Api { ... }` but omits the new `retry_after` field. Same error blocks **Test ❌** (can't compile → can't run tests). **Fmt ❌** is separate (formatting issues).
|
||||
|
||||
Local build succeeds because local Rust may be a slightly different version, but CI uses stable with stricter defaults.
|
||||
|
||||
### Answering the reviewer's question
|
||||
|
||||
**"Are defaults preserved so existing behavior is unchanged?"** → **Yes, fully preserved.**
|
||||
|
||||
1. `ApiTimeoutConfig::default()` → `connect_timeout_secs: 30`, `request_timeout_secs: 300`, `max_retries: 8`
|
||||
2. `CLAW_API_CONNECT_TIMEOUT` / `CLAW_API_REQUEST_TIMEOUT` env vars fall back to 30s/300s when unset
|
||||
3. Both `AnthropicClient` and `OpenAiCompatClient` constructors use `DEFAULT_MAX_RETRIES` (8), `DEFAULT_INITIAL_BACKOFF` (1s), `DEFAULT_MAX_BACKOFF` (128s)
|
||||
4. `with_retry_policy()` is **opt-in** — must be explicitly called to override defaults
|
||||
5. The `retry_after` field on `ApiError::Api` defaults to `None` (no Retry-After header → no override of backoff)
|
||||
|
||||
No existing behavior changes unless the user explicitly configures new settings.
|
||||
|
||||
### Additional issue: duplicate `#[must_use]` attribute
|
||||
|
||||
`error.rs:134` and `error.rs:138` both have `#[must_use]` on the same `retry_after()` method. The compiler warns about this.
|
||||
|
||||
## Execution Plan
|
||||
|
||||
### Step 1: Fix missing `retry_after` field in main.rs test
|
||||
|
||||
Add `retry_after: None` to the `ApiError::Api` construction at line 11685.
|
||||
|
||||
**File**: `rust/crates/rusty-claude-cli/src/main.rs`
|
||||
|
||||
### Step 2: Fix duplicate `#[must_use]` attribute in error.rs
|
||||
|
||||
Remove the duplicate `#[must_use]` on line 138 of `error.rs`. Keep the one on line 134 (above the doc comment, which is the conventional placement).
|
||||
|
||||
**File**: `rust/crates/api/src/error.rs`
|
||||
|
||||
### Step 3: Run `cargo fmt --all`
|
||||
|
||||
Fix formatting.
|
||||
|
||||
### Step 4: Verify — cargo check, cargo test -p api, cargo test -p rusty-claude-cli
|
||||
|
||||
Ensure building and testing passes.
|
||||
|
||||
## Commit Details
|
||||
|
||||
**Commit**: `76783377` on `worktree-api-timeout-retry-v2`
|
||||
|
||||
```
|
||||
fix: address CI failures and reviewer feedback on #3214
|
||||
|
||||
- Add missing retry_after: None field to ApiError::Api construction
|
||||
in main.rs test. This field was introduced by the Retry-After
|
||||
header support but was not added to the test's error initializer,
|
||||
causing a compile error under CI's strict mode.
|
||||
|
||||
- Remove duplicate #[must_use] attribute on retry_after() method
|
||||
in error.rs (lines 134+138 both had it; kept the outer one
|
||||
above the doc comment per convention).
|
||||
|
||||
- Cargo fmt --all run.
|
||||
|
||||
- Reviewer question "Are defaults preserved?" — answered yes:
|
||||
ApiTimeoutConfig defaults to 30s connect / 300s request / 8 retries.
|
||||
with_retry_policy() is opt-in. No behavior change without explicit
|
||||
configuration.
|
||||
```
|
||||
|
||||
**Files changed**:
|
||||
- `rust/crates/api/src/error.rs` — removed duplicate `#[must_use]` on line 138
|
||||
- `rust/crates/rusty-claude-cli/src/main.rs` — added `retry_after: None` to test + fmt fix
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [x] `retry_after: None` added to `ApiError::Api` test construction at main.rs:11685
|
||||
- [x] Duplicate `#[must_use]` removed from `error.rs`
|
||||
- [x] `cargo fmt --all` run
|
||||
- [x] `cargo build -p api` passes
|
||||
- [x] `cargo build -p rusty-claude-cli` passes
|
||||
- [x] `cargo test -p api` passes (147 unit + 37 integration)
|
||||
- [x] Changes committed and pushed (commit `76783377`)
|
||||
- [x] Reviewer question answered: defaults preserved (30s/300s/8, opt-in)
|
||||
|
|
@ -0,0 +1,118 @@
|
|||
# PR #3216 — Execution Plan: Session Resume
|
||||
|
||||
**Branch**: `worktree-session-resume-fixes`
|
||||
**Worktree**: `.claude/worktrees/wf_616683a9-d46-2`
|
||||
**CI State**: Build ❌ Test ❌ Fmt ✅ Clippy ✅ Docs ✅ Windows ✅
|
||||
|
||||
## Reviewer Feedback
|
||||
|
||||
From `1716775457damn`:
|
||||
> "The unified session resolution with load_session_excluding() is a clean approach. I notice CI has 2 failing checks (build + cargo test) — worth investigating before merge. One edge case question: what happens when all sessions have 0 messages (fresh install / all deleted)? A clear error message would be better than silently returning nothing."
|
||||
|
||||
## Investigation Findings
|
||||
|
||||
### Root cause of CI failures: test creates empty sessions
|
||||
|
||||
The test `latest_session_alias_resolves_most_recent_managed_session` creates sessions with `Session::new().save_to_path()`, which produces sessions with **0 messages**. The new `latest_session_excluding()` method filters out sessions with `message_count > 0` — so the test sessions are excluded and `resolve_session_reference("latest")` returns `Err`.
|
||||
|
||||
Error from CI:
|
||||
```
|
||||
latest session should resolve: Format("no managed sessions found in .claw/sessions/a0f293440386ce2b/\nStart `claw` to create a session, then rerun with `--resume latest`.\nNote: /resume latest searches all workspaces.")
|
||||
```
|
||||
|
||||
The build ❌ is also from this — CI's build step includes test compilation, and the test runtime failure causes the step to fail.
|
||||
|
||||
### Reviewer's "empty sessions" edge case
|
||||
|
||||
Currently, `latest_session_excluding()` returns the same "no managed sessions found" error whether there are zero sessions at all OR all sessions have 0 messages. These are different situations:
|
||||
1. **No sessions exist** → fresh install, user has never started a session
|
||||
2. **Sessions exist but all empty** → user just started claw in another terminal, hasn't typed anything yet
|
||||
|
||||
The reviewer wants these cases distinguished with clear error messages.
|
||||
|
||||
## Execution Plan
|
||||
|
||||
### Step 1: Fix the failing test
|
||||
|
||||
The test `latest_session_alias_resolves_most_recent_managed_session` needs to create sessions with at least one message so they pass the `message_count > 0` filter.
|
||||
|
||||
**File**: `rust/crates/rusty-claude-cli/src/main.rs`
|
||||
|
||||
Instead of:
|
||||
```rust
|
||||
Session::new()
|
||||
.with_persistence_path(older.path.clone())
|
||||
.save_to_path(&older.path)
|
||||
.expect("older session should save");
|
||||
```
|
||||
|
||||
We need to add a message to the session before saving:
|
||||
```rust
|
||||
let mut older_session = Session::new()
|
||||
.with_persistence_path(older.path.clone());
|
||||
older_session.add_user_message("test message for older session");
|
||||
older_session.save_to_path(&older.path).expect("older session should save");
|
||||
```
|
||||
|
||||
**Need to check**: What's the actual API for adding messages to a `Session`? The `Session` struct might use `ConversationMessage` or similar.
|
||||
|
||||
### Step 2: Add "all sessions empty" error distinction
|
||||
|
||||
When `latest_session_excluding()` finds sessions but they all have 0 messages, return a distinct error message:
|
||||
|
||||
- **No sessions at all**: "No managed sessions found in `{path}`. Start `claw` to create a session, then rerun with `--resume latest`."
|
||||
- **Sessions exist but all empty**: "All sessions are empty (0 messages). This usually means a fresh `claw` session has been started but no messages have been sent yet. Wait for a response in your other session, then try `--resume latest` again."
|
||||
|
||||
Implementation: In `latest_session_excluding()`, before returning the error, check if `list_sessions()` returns any sessions at all (regardless of message count). If so, use the "all empty" message.
|
||||
|
||||
**File**: `rust/crates/runtime/src/session_control.rs`
|
||||
|
||||
### Step 3: Update existing tests in session_control.rs
|
||||
|
||||
The runtime crate may also have tests that create empty sessions and test `latest_session()`. Need to verify and fix.
|
||||
|
||||
**File**: `rust/crates/runtime/src/session_control.rs`
|
||||
|
||||
### Step 4: Verify — cargo test -p runtime, cargo test -p rusty-claude-cli
|
||||
|
||||
## Commit Details
|
||||
|
||||
**Commit**: `41034bb3` on `worktree-session-resume-fixes`
|
||||
|
||||
```
|
||||
fix: address CI test failure and add empty-session error message
|
||||
|
||||
- Fix latest_session_alias_resolves_most_recent_managed_session test:
|
||||
the test created sessions with 0 messages, which are now filtered out
|
||||
by the message_count > 0 check in latest_session_excluding(). Updated
|
||||
the test to call push_user_text() before saving so sessions have
|
||||
at least one message and are findable by /resume latest.
|
||||
|
||||
- Add distinct error message when all sessions are empty (0 messages).
|
||||
Previously, the same "no managed sessions found" message was returned
|
||||
whether there were zero sessions or all sessions had 0 messages. Now:
|
||||
- No sessions at all → "no managed sessions found in {path}. Start
|
||||
claw to create a session..."
|
||||
- Sessions exist but all empty → "all sessions are empty (0 messages)
|
||||
in {path}. This usually means a fresh claw session is running but
|
||||
no messages have been sent yet. Wait for a response in your other
|
||||
session, then try --resume latest again."
|
||||
|
||||
- Add test for the all-sessions-empty error path:
|
||||
latest_session_returns_all_empty_error_when_sessions_exist_but_have_no_messages
|
||||
|
||||
Addresses reviewer feedback on #3216.
|
||||
```
|
||||
|
||||
**Files changed**:
|
||||
- `rust/crates/runtime/src/session_control.rs` — added `format_all_sessions_empty()`, added "has any session" check in `latest_session_excluding()`, added new test
|
||||
- `rust/crates/rusty-claude-cli/src/main.rs` — fixed test to use `push_user_text()` before saving
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [x] Test `latest_session_alias_resolves_most_recent_managed_session` fixed (sessions now have messages)
|
||||
- [x] `format_all_sessions_empty()` added — distinct error when sessions exist but all have 0 messages
|
||||
- [x] `format_no_managed_sessions()` preserved — error when no sessions exist at all
|
||||
- [x] New test `latest_session_returns_all_empty_error_when_sessions_exist_but_have_no_messages` passes
|
||||
- [x] All 17 session_control tests pass
|
||||
- [x] Changes committed and pushed (commit `41034bb3`)
|
||||
|
|
@ -0,0 +1,46 @@
|
|||
# PR #3217 — Execution Plan: Auto-Compact Retry
|
||||
|
||||
**Branch**: `worktree-auto-compact-retry`
|
||||
**Worktree**: `.claude/worktrees/wf_616683a9-d46-1`
|
||||
**CI State**: Build ✅ Test ✅ Clippy ✅ Docs ✅ Windows ✅ **Fmt ❌**
|
||||
|
||||
## Reviewer Feedback
|
||||
|
||||
From `1716775457damn`:
|
||||
> "Nice improvement — extending auto-compact retry from REPL-only to all execution paths. Using RuntimeError::is_context_window_failure() via the canonical api crate markers is the right approach. The progressive compaction strategy (4→2→0) with MAX_COMPACT_RETRIES=3 is well-bounded. One note: I see cargo fmt is failing in CI — consider running cargo fmt --all before merge to keep checks green."
|
||||
|
||||
## Investigation Findings
|
||||
|
||||
The ONLY issue is `cargo fmt`. Uncommitted fmt changes already exist in the worktree from a prior run (5 insertions, 10 deletions in `main.rs`). These are legitimate formatting fixes:
|
||||
- Collapsed multi-line `format_compact_report()` call to single line
|
||||
- Reformatted `match self.prepare_turn_runtime(true)` block
|
||||
|
||||
No other CI issues. The reviewer had no code concerns, just the fmt fix.
|
||||
|
||||
## Execution Plan
|
||||
|
||||
### Step 1: Commit the fmt changes (already applied)
|
||||
|
||||
The uncommitted changes are from `cargo fmt --all` — just formatting, no behavior change.
|
||||
|
||||
### Step 2: Verify — cargo test -p rusty-claude-cli
|
||||
|
||||
### Step 3: Push
|
||||
|
||||
## Commit Details
|
||||
|
||||
**Commit**: `592bae5` on `worktree-auto-compact-retry`
|
||||
|
||||
```
|
||||
style: run cargo fmt --all
|
||||
|
||||
Fixes cargo fmt CI check failure noted in reviewer feedback on #3217.
|
||||
```
|
||||
|
||||
**Files changed**:
|
||||
- `rust/crates/rusty-claude-cli/src/main.rs` — fmt: collapsed multi-line `format_compact_report()` call to single line, reformatted `match self.prepare_turn_runtime(true)` block (5 ins / 10 dels)
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [x] `cargo fmt --all` changes committed and pushed (commit `592bae5`)
|
||||
- [x] CI Fmt ❌ → ✅ expected
|
||||
|
|
@ -0,0 +1,78 @@
|
|||
# PR #3218 — Execution Plan: Wizard Entry Points
|
||||
|
||||
**Branch**: `worktree-wizard-entry-points`
|
||||
**Worktree**: `.claude/worktrees/wizard-entry-points`
|
||||
**CI State**: **Build ❌** Test ❌ Fmt ✅ Clippy ✅ Docs ✅ Windows ✅
|
||||
|
||||
## Reviewer Feedback
|
||||
|
||||
No reviewer comments yet (pre-emptive fix).
|
||||
|
||||
## Investigation Findings
|
||||
|
||||
### Root cause of CI failures: slash command count assertion off by 1
|
||||
|
||||
The test `renders_help_from_shared_specs` in `commands/src/lib.rs:5243` asserts:
|
||||
```rust
|
||||
assert_eq!(slash_command_specs().len(), 139);
|
||||
```
|
||||
|
||||
This PR added the `/setup` command (registered at line 723), bringing the total to **140**. The test wasn't updated to match.
|
||||
|
||||
Error from CI:
|
||||
```
|
||||
assertion `left == right` failed
|
||||
left: 140
|
||||
right: 139
|
||||
```
|
||||
|
||||
Both Build ❌ and Test ❌ are from this single test failure.
|
||||
|
||||
### No other issues found
|
||||
|
||||
The PR description mentions `/setup` command, `claw setup` subcommand, and `RuntimeProviderConfig`. The CI clippy, fmt, docs, and windows checks all pass — only this one test count is wrong.
|
||||
|
||||
## Execution Plan
|
||||
|
||||
### Step 1: Update the slash command count in the test
|
||||
|
||||
Change `139` → `140` at `commands/src/lib.rs:5243`.
|
||||
|
||||
### Step 2: Add `/setup` content assertion to the test
|
||||
|
||||
The test checks that many slash commands appear in the help output but doesn't verify `/setup`. Add:
|
||||
```rust
|
||||
assert!(help.contains("/setup"));
|
||||
```
|
||||
|
||||
### Step 3: Verify — cargo test -p commands
|
||||
|
||||
### Step 4: Commit + push
|
||||
|
||||
## Commit Details
|
||||
|
||||
**Commit**: `1122b16` on `worktree-wizard-entry-points`
|
||||
|
||||
```
|
||||
fix: update slash command count and add /setup assertion in test
|
||||
|
||||
- Update slash_command_specs().len() assertion from 139 to 140.
|
||||
The /setup command added by this PR increased the spec count by 1
|
||||
but the test's expected count was not updated, causing CI failure.
|
||||
|
||||
- Add assert!(help.contains("/setup")) to the
|
||||
renders_help_from_shared_specs test so the new command is
|
||||
verified in the help output.
|
||||
|
||||
Fixes CI Build ❌ and Test ❌ on #3218.
|
||||
```
|
||||
|
||||
**Files changed**:
|
||||
- `rust/crates/commands/src/lib.rs` — line 5242: added `assert!(help.contains("/setup"));`, line 5243: updated count `139` → `140`
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [x] `slash_command_specs().len()` assertion updated to 140
|
||||
- [x] `/setup` content assertion added
|
||||
- [x] `cargo test -p commands` passes (42 tests)
|
||||
- [x] Changes committed and pushed (commit `1122b16`)
|
||||
|
|
@ -0,0 +1,161 @@
|
|||
# PR #3219 — Execution Plan: LSP Integration v3
|
||||
|
||||
**Branch**: `worktree-lsp-v3`
|
||||
**Worktree**: `.claude/worktrees/wf_616683a9-d46-5`
|
||||
**CI State**: **Build ❌** Test ❌ Fmt ✅ Clippy ✅ Docs ✅ Windows ✅
|
||||
|
||||
## Reviewer Feedback
|
||||
|
||||
From `1716775457damn`:
|
||||
> "This is an impressive piece of work. The modular split across 4 crates with 58 unit tests is exactly the right architecture. The lazy-start pattern and diagnostic enrichment in tool output make this genuinely useful.
|
||||
>
|
||||
> Two things to check before merge:
|
||||
> - build and cargo test are currently failing in CI — worth investigating if these are test assertions that need updating for the new LSP types
|
||||
> - The auto-discovery of 14 LSP servers via PATH probing is great, but consider adding a `claw lsp status` command so users can confirm which servers were discovered and their health"
|
||||
|
||||
## Investigation Findings
|
||||
|
||||
### CI failure: same slash command count issue as #3218
|
||||
|
||||
The test `renders_help_from_shared_specs` asserts `slash_command_specs().len() == 139`, but this PR adds `/lsp` (1 new command), making it 140. Same pattern as #3218.
|
||||
|
||||
Locally confirmed:
|
||||
```
|
||||
assertion `left == right` failed
|
||||
left: 140
|
||||
right: 139
|
||||
```
|
||||
|
||||
Both Build ❌ and Test ❌ are from this single test failure.
|
||||
|
||||
### LSP module architecture
|
||||
|
||||
The LSP code lives as **modules within the `runtime` crate** (not separate crates), despite the PR description:
|
||||
- `rust/crates/runtime/src/lsp_discovery.rs` — auto-discovery of 14 LSP servers
|
||||
- `rust/crates/runtime/src/lsp_client/` — LSP client lifecycle, dispatch, types
|
||||
- `rust/crates/runtime/src/lsp_transport/` — transport layer
|
||||
- `rust/crates/runtime/src/lsp_process/` — process management
|
||||
|
||||
### Current `/lsp` command state
|
||||
|
||||
The `/lsp` slash command is **registered but not implemented** — executing it just prints "not yet implemented in this build." at `main.rs:6124`.
|
||||
|
||||
The `SlashCommand::Lsp` variant has `action: Option<String>` and `target: Option<String>`, so it supports:
|
||||
- `/lsp` (no args)
|
||||
- `/lsp status`
|
||||
- `/lsp start <target>`
|
||||
- `/lsp stop <target>`
|
||||
- `/lsp list`
|
||||
|
||||
### Available LSP discovery API
|
||||
|
||||
Currently exported from `runtime`:
|
||||
- `discover_available_servers()` → `Vec<LspServerDescriptor>` — servers found on PATH
|
||||
- `known_lsp_servers()` → `Vec<LspServerDescriptor>` — all 14 known servers
|
||||
- `find_server_for_file(path)` → `Option<LspServerDescriptor>` — server for a file extension
|
||||
- `command_exists_on_path(cmd)` → `bool` — check if a binary is available
|
||||
|
||||
Not exported but available:
|
||||
- `check_lsp_availability()` → `Vec<LspInstallAction>` — detailed status (Installed/Missing/RustupProxyMissing)
|
||||
- `format_install_prompt(actions)` → `String` — human-readable install instructions
|
||||
|
||||
### `LspInstallAction` variants
|
||||
|
||||
```rust
|
||||
pub enum LspInstallAction {
|
||||
Installed,
|
||||
Missing { language: String, instructions: Vec<InstallInstruction> },
|
||||
RustupProxyMissing { language: String, component: String },
|
||||
}
|
||||
```
|
||||
|
||||
## Execution Plan
|
||||
|
||||
### Step 1: Fix CI — update slash command count
|
||||
|
||||
Change `139` → `140` at `commands/src/lib.rs:5246`. Add `/lsp` assertion to the test.
|
||||
|
||||
### Step 2: Export `check_lsp_availability` and related types from runtime
|
||||
|
||||
Add to `runtime/src/lib.rs` pub use:
|
||||
- `check_lsp_availability`
|
||||
- `LspInstallAction`
|
||||
- `InstallInstruction`
|
||||
- `format_install_prompt`
|
||||
|
||||
### Step 3: Implement `/lsp` slash command handler in main.rs
|
||||
|
||||
Replace the "not yet implemented" stub with a handler that dispatches on `action`:
|
||||
|
||||
| Action | Behavior |
|
||||
|--------|----------|
|
||||
| `None` or `"status"` | Show all discovered servers, their availability, and install hints for missing ones |
|
||||
| `"list"` | List known LSP servers (all 14) with availability status |
|
||||
| `"start"` / `"stop"` | Print "not yet available" — lazy-start is handled by tool infrastructure, not manual commands |
|
||||
|
||||
The `status` output format:
|
||||
```
|
||||
LSP Server Status (14 known, X available):
|
||||
|
||||
✅ rust-analyzer — .rs files [installed]
|
||||
✅ typescript-language-server — .ts/.tsx/.js/.jsx [installed]
|
||||
❌ gopls — .go files [missing: go install golang.org/x/tools/gopls@latest]
|
||||
❌ pylsp — .py files [missing: pip install python-lsp-server]
|
||||
⚠️ rust-analyzer — .rs files [rustup proxy: run `rustup component add rust-analyzer`]
|
||||
|
||||
Run /lsp list for all known servers.
|
||||
```
|
||||
|
||||
### Step 4: Update the test assertions
|
||||
|
||||
Add `assert!(help.contains("/lsp"))` to `renders_help_from_shared_specs`.
|
||||
|
||||
### Step 5: Verify — cargo test -p commands, cargo test -p runtime, cargo test -p rusty-claude-cli
|
||||
|
||||
### Step 6: Commit + push
|
||||
|
||||
## Commit Details
|
||||
|
||||
**Commit**: `369452c` on `worktree-lsp-v3`
|
||||
|
||||
```
|
||||
fix: address CI failure and implement /lsp status command
|
||||
|
||||
- Fix renders_help_from_shared_specs test: update
|
||||
slash_command_specs().len() from 139 to 140. The /lsp command
|
||||
added by this PR increased the spec count by 1 but the test
|
||||
was not updated. Also add assert!(help.contains("/lsp")).
|
||||
|
||||
- Export check_lsp_availability, LspInstallAction,
|
||||
InstallInstruction, and format_install_prompt from the runtime
|
||||
crate's public API so the CLI can access LSP discovery status.
|
||||
|
||||
- Implement /lsp slash command handler:
|
||||
- /lsp or /lsp status — show all known LSP servers with
|
||||
availability (installed ✅, missing ❌ with install hint,
|
||||
rustup proxy ⚠️)
|
||||
- /lsp list — list all known servers with language and extensions
|
||||
- /lsp start/stop — note that lazy-start is automatic, not manual
|
||||
|
||||
Addresses reviewer feedback on #3219 requesting a way for
|
||||
users to confirm which LSP servers were discovered and their
|
||||
health.
|
||||
```
|
||||
|
||||
**Files changed**:
|
||||
- `rust/crates/commands/src/lib.rs` — update count 139→140, add `/lsp` assertion
|
||||
- `rust/crates/runtime/src/lib.rs` — export `check_lsp_availability`, `LspInstallAction`, `InstallInstruction`, `format_install_prompt`
|
||||
- `rust/crates/rusty-claude-cli/src/main.rs` — implement `/lsp` slash command handler replacing "not yet implemented" stub, add runtime imports
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [x] `slash_command_specs().len()` assertion updated to 140
|
||||
- [x] `/lsp` content assertion added
|
||||
- [x] `check_lsp_availability` and related types exported from runtime
|
||||
- [x] `/lsp status` shows discovered servers with availability + install hints
|
||||
- [x] `/lsp list` shows all known servers
|
||||
- [x] `cargo test -p commands` passes (42 tests)
|
||||
- [x] `cargo test -p runtime` (LSP tests) passes (58 tests)
|
||||
- [x] `cargo build -p rusty-claude-cli` passes
|
||||
- [x] Changes committed and pushed (commit `369452c`)
|
||||
- [x] 8 pre-existing CLI test failures (local config parse issue) — not from our changes
|
||||
|
|
@ -0,0 +1,377 @@
|
|||
# Reviewer Feedback Sprint
|
||||
|
||||
## Execution Log
|
||||
|
||||
| Time | PR | Action | Status |
|
||||
|------|-----|--------|--------|
|
||||
| 2026-06-02 | #3211 | Plan written at `docs/pr-3211-plan.md` | ✅ Plan approved |
|
||||
| 2026-06-02 | #3211 | Executed: extracted `read_env_non_empty` + `resolve_base_url` into `mod.rs` | ✅ Committed |
|
||||
| 2026-06-02 | #3211 | Pushed commit `c0855e12` to `worktree-provider-config-fallback` | ✅ Pushed |
|
||||
| 2026-06-02 | #3211 | CI: cargo fmt ✅, cargo test (147 unit + 37 integration) ✅, cargo check ✅ | ✅ Verified |
|
||||
| 2026-06-02 | #3211 | Pre-existing clippy issues in runtime/trident.rs (not from our changes) — noted | ⚠️ Known |
|
||||
| 2026-06-02 | #3214 | Plan written at `docs/pr-3214-plan.md` | ✅ Plan approved |
|
||||
| 2026-06-02 | #3214 | All 3 CI failures (build ❌ test ❌ fmt ❌) traced to one root cause: missing `retry_after: None` in main.rs test | ✅ Root caused |
|
||||
| 2026-06-02 | #3214 | Fixed: added `retry_after: None`, removed duplicate `#[must_use]`, ran `cargo fmt --all` | ✅ Committed |
|
||||
| 2026-06-02 | #3214 | Pushed commit `76783377` to `worktree-api-timeout-retry-v2` | ✅ Pushed |
|
||||
| 2026-06-02 | #3214 | Reviewer Q "Are defaults preserved?" — YES: ApiTimeoutConfig defaults 30s/300s/8, with_retry_policy() is opt-in | ✅ Answered |
|
||||
| 2026-06-02 | #3216 | Plan written at `docs/pr-3216-plan.md` | ✅ Plan approved |
|
||||
| 2026-06-02 | #3216 | Root cause: test creates sessions with 0 messages → filtered out by `message_count > 0` | ✅ Root caused |
|
||||
| 2026-06-02 | #3216 | Fix: test now adds `push_user_text()` before saving; added `format_all_sessions_empty()` error message | ✅ Committed |
|
||||
| 2026-06-02 | #3216 | Added test `latest_session_returns_all_empty_error_when_sessions_exist_but_have_no_messages` | ✅ New test |
|
||||
| 2026-06-02 | #3216 | Pushed commit `41034bb3` to `worktree-session-resume-fixes` | ✅ Pushed |
|
||||
| 2026-06-02 | #3217 | Plan written at `docs/pr-3217-plan.md` — trivial: just cargo fmt | ✅ Plan approved |
|
||||
| 2026-06-02 | #3217 | Committed pre-existing fmt changes, pushed `592bae5` | ✅ Done |
|
||||
| 2026-06-02 | #3218 | Plan written at `docs/pr-3218-plan.md` — slash command count off by 1 | ✅ Plan approved |
|
||||
| 2026-06-02 | #3218 | Fixed: count 139→140, added `/setup` assertion, pushed `1122b16` | ✅ Done |
|
||||
| 2026-06-02 | #3219 | Plan written at `docs/pr-3219-plan.md` — CI count fix + implement `/lsp status` | ✅ Plan approved |
|
||||
| 2026-06-02 | #3219 | Fixed CI: count 139→140, added `/lsp` assertion | ✅ CI fix |
|
||||
| 2026-06-02 | #3219 | Exported `check_lsp_availability`, `LspInstallAction`, `InstallInstruction`, `format_install_prompt` from runtime | ✅ Exports |
|
||||
| 2026-06-02 | #3219 | Implemented `/lsp status` (✅❌⚠️), `/lsp list`, `/lsp start/stop` handler replacing stub | ✅ Feature |
|
||||
| 2026-06-02 | #3219 | Pushed commit `369452c` to `worktree-lsp-v3` | ✅ Pushed |
|
||||
|
||||
### 2026-06-04 — Round 2: upstream-merge cycle (new reviewer comments + conflicts)
|
||||
|
||||
Reviewer (`1716775457damn`) left new comments on 5 PRs. Root cause across all: every PR was cut from stale `upstream/main @ 4d3dc5b`; upstream advanced to `4619375` (config/runtime refactor → conflicts + flaky CI). Fix = merge current upstream + address each specific ask. Investigated with 5 read-only agents, fixed with 5 agents in isolated worktrees, each diff reviewed (no leftover markers, feature preserved, build + targeted tests green, remote SHA verified).
|
||||
|
||||
| Time | PR | Action | Status |
|
||||
|------|-----|--------|--------|
|
||||
| 2026-06-04 | #3214 | Reviewer: resolve conflicts + clarify 5-vs-3 commits. Merged upstream; threaded `api_timeout` through new `build_runtime_config` helper; conflicts in config.rs/lib.rs | ✅ Resolved |
|
||||
| 2026-06-04 | #3214 | Verified: 158+13+11+4+7 api tests pass, no leftover markers, MERGEABLE/CLEAN. Pushed merge `9e50cb6` | ✅ Pushed |
|
||||
| 2026-06-04 | #3216 | Reviewer: fix 3 CI checks + add exclude_id tests. Clean merge; `cargo fmt` (real failure); added 3 tests (exclude_id skip, 0-msg filter, resolve_reference_excluding) | ✅ Resolved |
|
||||
| 2026-06-04 | #3216 | Verified: 21 session_control tests pass, fmt clean, MERGEABLE/CLEAN. Pushed `346772a`. Note: build/test reds were env-only `subagentModel` config-parse, not PR-caused | ✅ Pushed |
|
||||
| 2026-06-04 | #3217 | Reviewer: fix 2 CI checks + integration test. Clean merge refreshed flaky arg-parse tests; extracted `PRESERVE_SCHEDULE` const + added bounds test | ✅ Resolved |
|
||||
| 2026-06-04 | #3217 | Verified: 206 CLI tests pass incl. previously-flaky pair, MERGEABLE/CLEAN. Pushed `77c7d08` | ✅ Pushed |
|
||||
| 2026-06-04 | #3218 | Reviewer: resolve conflicts + document subagentModel. 4-file merge (kept upstream `Doctor{output_format,permission_mode}` sig + this PR's `setup` arm); doc comment on `TOP_LEVEL_FIELDS` | ✅ Resolved |
|
||||
| 2026-06-04 | #3218 | Verified: 24 config_validate tests pass, builds clean, MERGEABLE/CLEAN. Pushed `d54f028` | ✅ Pushed |
|
||||
| 2026-06-04 | #3219 | Reviewer: resolve conflicts + nightly rustup probe. 3-file merge; `rustup_component_works()` now probes stable→nightly, returns working toolchain; both call sites updated | ✅ Resolved |
|
||||
| 2026-06-04 | #3219 | Verified: 58 LSP tests pass, builds clean, MERGEABLE (CI re-running). Pushed `a0e9833` | ✅ Pushed |
|
||||
| 2026-06-04 | #3211 | Resolved merge conflicts with upstream/main (config.rs + lib.rs); preserved RuntimeProviderConfig + `read_env_non_empty`/`resolve_base_url` helpers; rebuilt corrupted test section | ✅ Resolved |
|
||||
| 2026-06-04 | #3211 | Verified: api + runtime crates compile, shared helpers intact in mod.rs, `67da2dc` on `worktree-provider-config-fallback` | ✅ Pushed |
|
||||
| 2026-06-04 | #3211 | Posted maintainer summary on PR #3211: shared helpers explained, `from_env` vs `from_env_or_saved` naming rationale, merge details | ✅ Commented |
|
||||
|
||||
**Date:** 2026-06-03
|
||||
**Upstream:** `ultraworkers/claw-code`
|
||||
|
||||
---
|
||||
|
||||
## Closed PRs — Done, No Action
|
||||
|
||||
| PR | What happened |
|
||||
|----|---------------|
|
||||
| #3212 | Owner merged equivalent (`1bd18be`) |
|
||||
| #3215 | Owner closed — wants internal doc pointers, not external links |
|
||||
| #3220 | Owner merged equivalent (`9c8375d`) with validation/tests added |
|
||||
|
||||
---
|
||||
|
||||
## Open PRs — Reality Check
|
||||
|
||||
| # | PR | Title | build | test | fmt | mergeable | Reviewer | Difficulty |
|
||||
|---|-----|-------|-------|------|-----|-----------|----------|------------|
|
||||
| ① | #3217 | Auto-compact retry | ✅ | ✅ | ❌ | YES | 1716775457damn | Trivial |
|
||||
| ② | #3211 | 3-tier credential resolution | ✅ | ✅ | ❌ | ? | 1716775457damn | Medium |
|
||||
| ③ | #3214 | API timeout/retry | ❌ | ❌ | ❌ | ? | 1716775457damn | Medium |
|
||||
| ④ | #3216 | Session resume | ❌ | ❌ | ✅ | YES | 1716775457damn | Medium |
|
||||
| ⑤ | #3218 | Wizard entry points | ❌ | ❌ | ✅ | ? | *(none yet)* | Medium |
|
||||
| ⑥ | #3219 | LSP integration v3 | ❌ | ❌ | ✅ | ? | 1716775457damn | Hard |
|
||||
|
||||
Order: ① → ② → ③ → ④ → ⑤ → ⑥ (green CI first, then red, easy → hard)
|
||||
|
||||
---
|
||||
|
||||
## ① #3217 — Auto-compact retry
|
||||
|
||||
**Branch:** `worktree-auto-compact-retry`
|
||||
**Worktree:** `.claude/worktrees/wf_616683a9-d46-1`
|
||||
|
||||
### Reviewer said:
|
||||
|
||||
> "Nice improvement — extending auto-compact retry from REPL-only to all execution paths fixes a real UX pain point. Using RuntimeError::is_context_window_failure() via the canonical api crate markers is the right approach. The progressive compaction strategy (4→2→0) with MAX_COMPACT_RETRIES=3 is well-bounded."
|
||||
>
|
||||
> **Action:** "I see cargo fmt is failing in CI — consider running cargo fmt --all before merge to keep checks green."
|
||||
|
||||
### CI: 5/6 green. Only `cargo fmt` fails.
|
||||
|
||||
### Plan:
|
||||
|
||||
| Step | What | Command |
|
||||
|------|------|---------|
|
||||
| 1 | Format | `cd rust/ && cargo fmt --all` |
|
||||
| 2 | Verify | `cargo fmt --all -- --check` && `cargo check --workspace` |
|
||||
| 3 | Commit | `"style: cargo fmt"` |
|
||||
| 4 | Push | `git push origin worktree-auto-compact-retry` |
|
||||
|
||||
Fmt touches 1 file: `main.rs` (5 ins / 10 del, pure formatting). No logic changes.
|
||||
|
||||
### Status: ⬜
|
||||
|
||||
---
|
||||
|
||||
## ② #3211 — 3-tier credential resolution
|
||||
|
||||
**Branch:** `worktree-provider-config-fallback`
|
||||
**Worktree:** `.claude/worktrees/provider-config-fallback`
|
||||
|
||||
### Reviewer said:
|
||||
|
||||
> "This makes the setup wizard actually functional. The 3-tier priority is well thought out. Covering all four providers with both API key and base URL resolution is thorough."
|
||||
>
|
||||
> **Ask 1:** "The from_env_or_saved naming across multiple provider files could be consolidated into a shared trait or helper to reduce duplication."
|
||||
>
|
||||
> **Ask 2:** "Also, cargo fmt is failing — a quick cargo fmt --all before merge will fix that."
|
||||
|
||||
### CI: 5/6 green. Only `cargo fmt` fails.
|
||||
|
||||
### Plan:
|
||||
|
||||
| Step | What | Detail |
|
||||
|------|------|--------|
|
||||
| 1 | Investigate | Read all 4 provider files. Catalog `from_env_or_saved` signatures, env vars, return types. Find where `ResolvedCredentials` is defined. |
|
||||
| 2 | Add helper | `CredentialSources` struct + `resolve_provider_credentials()` fn in `config.rs` |
|
||||
| 3 | Refactor × 4 | Replace each provider's `from_env_or_saved` body with helper call |
|
||||
| 4 | Export | `pub use` in `lib.rs` if needed |
|
||||
| 5 | Fmt + verify | `cargo fmt --all` → `cargo check --workspace` → `cargo test -p runtime -p api` |
|
||||
| 6 | Commit + push | `"refactor: consolidate from_env_or_saved into shared resolve_provider_credentials() helper"` |
|
||||
|
||||
**Helper design:**
|
||||
|
||||
```rust
|
||||
pub struct CredentialSources {
|
||||
pub api_key_env: &'static str,
|
||||
pub api_key_alt_env: Option<&'static str>,
|
||||
pub api_key_config: fn(&RuntimeProviderConfig) -> &Option<String>,
|
||||
pub base_url_env: &'static str,
|
||||
pub base_url_config: fn(&RuntimeProviderConfig) -> &Option<String>,
|
||||
pub default_base_url: &'static str,
|
||||
}
|
||||
|
||||
pub fn resolve_provider_credentials(
|
||||
config: &RuntimeProviderConfig,
|
||||
sources: &CredentialSources,
|
||||
) -> Option<ResolvedCredentials> { … }
|
||||
```
|
||||
|
||||
**Open Qs to resolve in Step 1:**
|
||||
- Is `ResolvedCredentials` shared or per-provider?
|
||||
- Does `.env` tier use `dotenv::var()`?
|
||||
- Any tests call `from_env_or_saved` directly?
|
||||
|
||||
### Status: ✅ Resolved (2026-06-04)
|
||||
|
||||
Merged upstream/main into `worktree-provider-config-fallback`. Conflicts in `runtime/src/config.rs` and `runtime/src/lib.rs` both resolved while preserving the `read_env_non_empty` / `resolve_base_url` shared helpers and the `RuntimeProviderConfig` struct/impl.
|
||||
|
||||
Key manual fix: The merge-generated config.rs had corrupted test sections (interleaved function declarations from both versions). Reconstructed clean provider_config_* and rules_import_* test bodies from original commits.
|
||||
|
||||
Build verification: `cargo check -p api` ✅, `cargo check -p runtime` ✅. Pre-existing clippy issues in trident.rs and claw-rag-service unchanged.
|
||||
|
||||
Commit: `67da2dc`
|
||||
|
||||
---
|
||||
|
||||
## ③ #3214 — API timeout/retry
|
||||
|
||||
**Branch:** `worktree-api-timeout-retry-v2`
|
||||
**Worktree:** `.claude/worktrees/api-timeout-retry-v2`
|
||||
|
||||
### Reviewer said:
|
||||
|
||||
> "Good revival of the stalled #2816. Retry-After header and 400 transient retry are particularly valuable for handling API rate limits."
|
||||
>
|
||||
> **Question:** "Are defaults preserved so existing behavior is unchanged?"
|
||||
|
||||
### CI: 3/6 green. Build ❌, test ❌, fmt ❌. Fix CI first, then answer.
|
||||
|
||||
### Plan:
|
||||
|
||||
**Phase A — Fix CI:**
|
||||
|
||||
| Step | What | Detail |
|
||||
|------|------|--------|
|
||||
| 1 | Investigate build | `cargo check --workspace 2>&1` — capture errors |
|
||||
| 2 | Fix build | Based on Step 1 findings |
|
||||
| 3 | Investigate test | `cargo test -p runtime -p api 2>&1` — capture failures |
|
||||
| 4 | Fix tests | Based on Step 3 findings |
|
||||
| 5 | Fmt | `cargo fmt --all` |
|
||||
| 6 | Verify green | All 3 checks pass locally |
|
||||
| 7 | Commit + push | |
|
||||
|
||||
**Phase B — Answer reviewer:**
|
||||
|
||||
| Step | What | Detail |
|
||||
|------|------|--------|
|
||||
| 8 | Verify defaults | `TimeoutConfig` timeout=300s, `RetryConfig` max_retries=2, 400-transient is additive-only |
|
||||
| 9 | Post comment | Answer with specifics (draft below, adjust after Step 8) |
|
||||
|
||||
**Comment draft:**
|
||||
```
|
||||
Yes, all defaults are preserved:
|
||||
- Timeout: 300s (unchanged)
|
||||
- Max retries: 2 (unchanged)
|
||||
- Retry-After: only activates when server sends the header
|
||||
- 400 transient: purely additive — retries only on specific transient 400 codes
|
||||
|
||||
Backward-compatible with no behavioral changes unless user explicitly configures new fields.
|
||||
```
|
||||
⚠ Don't post until Step 8 confirms. If any default changed, fix code first.
|
||||
|
||||
**Open Qs:**
|
||||
- Actual build errors? (Step 1)
|
||||
- Actual test failures? (Step 3)
|
||||
- Defaults actually preserved? (Step 8)
|
||||
|
||||
### Status: ⬜
|
||||
|
||||
---
|
||||
|
||||
## ④ #3216 — Session resume
|
||||
|
||||
**Branch:** `worktree-session-resume-fixes`
|
||||
**Worktree:** `.claude/worktrees/wf_616683a9-d46-2`
|
||||
|
||||
### Reviewer said:
|
||||
|
||||
> "The unified session resolution with load_session_excluding() is a clean approach."
|
||||
>
|
||||
> **Concern 1:** "I notice CI has 2 failing checks (build + cargo test) — worth investigating before merge."
|
||||
>
|
||||
> **Concern 2:** "What happens when all sessions have 0 messages (fresh install / all deleted)? A clear error message would be better than silently returning nothing."
|
||||
|
||||
### CI: 4/6 green. Build ❌, test ❌. fmt ✅.
|
||||
|
||||
### Plan:
|
||||
|
||||
| Step | What | Detail |
|
||||
|------|------|--------|
|
||||
| 1 | Investigate build | `cargo check --workspace 2>&1` — capture errors |
|
||||
| 2 | Fix build | Based on Step 1. Likely missing imports in `session_control.rs` |
|
||||
| 3 | Investigate test | `cargo test -p runtime 2>&1` — capture failures |
|
||||
| 4 | Fix tests | Based on Step 3 |
|
||||
| 5 | Add empty-session error | If all candidates have 0 messages → `Err("No sessions with messages found. Start a new session first.")` |
|
||||
| 6 | Surface in CLI | `/resume` handler prints error gracefully (no panic) |
|
||||
| 7 | Fmt + verify | `cargo fmt --all` → `cargo check --workspace` → `cargo test -p runtime` |
|
||||
| 8 | Commit + push | |
|
||||
|
||||
**Empty-session error:**
|
||||
```rust
|
||||
// session_control.rs — after scanning:
|
||||
if candidates.is_empty() || candidates.iter().all(|s| s.message_count == 0) {
|
||||
return Err(SessionError::NoSessionsFound);
|
||||
}
|
||||
// main.rs — /resume handler:
|
||||
Err(SessionError::NoSessionsFound) => {
|
||||
eprintln!("No sessions with messages found. Start a new session first.");
|
||||
}
|
||||
```
|
||||
|
||||
**Open Qs:**
|
||||
- Actual build errors? (Step 1)
|
||||
- Actual test failures? (Step 3)
|
||||
- Does `SessionError` enum already exist?
|
||||
|
||||
### Status: ⬜
|
||||
|
||||
---
|
||||
|
||||
## ⑤ #3218 — Wizard entry points
|
||||
|
||||
**Branch:** `worktree-wizard-entry-points`
|
||||
**Worktree:** `.claude/worktrees/wizard-entry-points`
|
||||
|
||||
### Reviewer: None yet. Fix proactively before one arrives.
|
||||
|
||||
### CI: 4/6 green. Build ❌, test ❌. fmt ✅.
|
||||
|
||||
### Likely cause: `setup_wizard.rs` imports `RuntimeProviderConfig` which doesn't exist on upstream/main (only in PR #3211).
|
||||
|
||||
### Strategy: Add `RuntimeProviderConfig` to this PR too. Safe duplication — when #3211 merges first, rebase deduplicates.
|
||||
|
||||
### Plan:
|
||||
|
||||
| Step | What | Detail |
|
||||
|------|------|--------|
|
||||
| 1 | Investigate build | `cargo check --workspace 2>&1` — confirm root cause |
|
||||
| 2 | Add `RuntimeProviderConfig` | Minimal fields in `config.rs` (same as #3211) |
|
||||
| 3 | Export | `pub use` in `lib.rs` |
|
||||
| 4 | Fix other compile errors | If any |
|
||||
| 5 | Investigate test | `cargo test -p runtime -p api 2>&1` |
|
||||
| 6 | Fix tests | Based on findings |
|
||||
| 7 | Fmt + verify | `cargo fmt --all` → `cargo check --workspace` → `cargo test -p runtime -p api` |
|
||||
| 8 | Commit + push | |
|
||||
|
||||
**Open Qs:**
|
||||
- Is `RuntimeProviderConfig` the only missing type?
|
||||
- Any other dependencies on #3211?
|
||||
|
||||
### Status: ⬜
|
||||
|
||||
---
|
||||
|
||||
## ⑥ #3219 — LSP integration v3
|
||||
|
||||
**Branch:** `worktree-lsp-v3`
|
||||
**Worktree:** `.claude/worktrees/wf_616683a9-d46-5`
|
||||
|
||||
### Reviewer said:
|
||||
|
||||
> "This is an impressive piece of work. The modular split across 4 crates with 58 unit tests is exactly the right architecture. The lazy-start pattern and diagnostic enrichment are genuinely useful."
|
||||
>
|
||||
> **Blocker 1:** "build and cargo test are currently failing — worth investigating"
|
||||
>
|
||||
> **Blocker 2:** "Consider adding a claw lsp status command so users can confirm which servers were discovered and their health"
|
||||
|
||||
### CI: 4/6 green. Build ❌, test ❌. fmt ✅.
|
||||
|
||||
### Plan:
|
||||
|
||||
**Phase A — Fix CI:**
|
||||
|
||||
| Step | What | Detail |
|
||||
|------|------|--------|
|
||||
| 1 | Investigate build | `cargo check --workspace 2>&1` |
|
||||
| 2 | Fix build | Module visibility / missing imports in LSP crates |
|
||||
| 3 | Investigate test | `cargo test -p runtime 2>&1` |
|
||||
| 4 | Fix tests | Assertions needing update for new types |
|
||||
| 5 | Verify | `cargo check --workspace` + `cargo test -p runtime` |
|
||||
| 6 | Commit | `"fix: resolve CI build and test failures for LSP integration"` |
|
||||
|
||||
**Phase B — Add `claw lsp status`:**
|
||||
|
||||
| Step | What | Detail |
|
||||
|------|------|--------|
|
||||
| 7 | Add `LspServerStatus` + `LspStatus` | In `lsp_discovery.rs`: `{ name, status: {Running\|Stopped\|NotFound}, path }` |
|
||||
| 8 | Add `discover_all()` | Probe PATH → `Vec<LspServerStatus>` |
|
||||
| 9 | CLI subcommand | `claw lsp status` → call `discover_all()` → print table |
|
||||
| 10 | Slash command | `/lsp status` → check in-process manager → print |
|
||||
| 11 | Fmt + verify | `cargo fmt --all` → `cargo check --workspace` → `cargo test -p runtime` |
|
||||
| 12 | Commit + push | |
|
||||
|
||||
**Output format:**
|
||||
```
|
||||
LSP Server Status:
|
||||
rust-analyzer running /home/user/.cargo/bin/rust-analyzer
|
||||
gopls stopped /usr/local/bin/gopls
|
||||
pyright not found (not in PATH)
|
||||
|
||||
2 servers discovered, 1 running
|
||||
```
|
||||
|
||||
**Open Qs:**
|
||||
- Actual build errors? (Step 1)
|
||||
- Actual test failures? (Step 3)
|
||||
- Does LSP crate already have `discover_all()`-equivalent?
|
||||
- CLI: probe fresh or connect to running instance?
|
||||
|
||||
### Status: ⬜
|
||||
|
||||
---
|
||||
|
||||
## Progress
|
||||
|
||||
| # | PR | Status | CI | Reviewer | Pushed |
|
||||
|---|-----|--------|-----|----------|--------|
|
||||
| ① | #3217 | ⬜ | — | — | — |
|
||||
| ② | #3211 | ⬜ | — | — | — |
|
||||
| ③ | #3214 | ⬜ | — | — | — |
|
||||
| ④ | #3216 | ⬜ | — | — | — |
|
||||
| ⑤ | #3218 | ⬜ | — | — | — |
|
||||
| ⑥ | #3219 | ⬜ | — | — | — |
|
||||
|
|
@ -0,0 +1,567 @@
|
|||
# Upstream PR Sprint Plan
|
||||
|
||||
**Created:** 2026-06-02
|
||||
**Updated:** 2026-06-02
|
||||
**Branch:** `feat-tui` (working branch)
|
||||
**Upstream:** `ultraworkers/claw-code`
|
||||
**Author:** TheArchitectit
|
||||
|
||||
---
|
||||
|
||||
## Sprint Progress
|
||||
|
||||
### Completed (9/9) — ALL ITEMS DONE ✅
|
||||
|
||||
| # | Item | PR | Branch |
|
||||
|---|------|-----|--------|
|
||||
| 1 | Provider config 3-tier fallback | [ultraworkers/claw-code#3211](https://github.com/ultraworkers/claw-code/pull/3211) | `worktree-provider-config-fallback` |
|
||||
| 8 | GitShow --format | [ultraworkers/claw-code#3212](https://github.com/ultraworkers/claw-code/pull/3212) | `worktree-gitshow-format` |
|
||||
| 4 | API timeout/retry v2 | [ultraworkers/claw-code#3214](https://github.com/ultraworkers/claw-code/pull/3214) | `worktree-api-timeout-retry-v2` |
|
||||
| 3 | Auto-compact & retry on context window errors | [ultraworkers/claw-code#3217](https://github.com/ultraworkers/claw-code/pull/3217) | `worktree-auto-compact-retry` |
|
||||
| 6 | Session resume fixes | [ultraworkers/claw-code#3216](https://github.com/ultraworkers/claw-code/pull/3216) | `worktree-session-resume-fixes` |
|
||||
| 9 | README referral links | [ultraworkers/claw-code#3215](https://github.com/ultraworkers/claw-code/pull/3215) | `worktree-readme-referral` |
|
||||
| 2 | Wizard entry points | [ultraworkers/claw-code#3218](https://github.com/ultraworkers/claw-code/pull/3218) | `worktree-wizard-entry-points` |
|
||||
| 5 | LSP clean rebuild | [ultraworkers/claw-code#3219](https://github.com/ultraworkers/claw-code/pull/3219) | `worktree-lsp-v3` |
|
||||
| 7 | Project rules rebuild | [ultraworkers/claw-code#3220](https://github.com/ultraworkers/claw-code/pull/3220) | `worktree-project-rules-v3` |
|
||||
|
||||
### Stale PRs Closed
|
||||
|
||||
| PR | Closed In Favor Of |
|
||||
|----|-------------------|
|
||||
| #2816 | #3214 (fresh branch, cherry-picked commits) |
|
||||
| #2810 | #3211 + #3218 (split into credential resolution + entry points) |
|
||||
|
||||
---
|
||||
|
||||
## Context
|
||||
|
||||
We have 16 PRs submitted to upstream `ultraworkers/claw-code`. Four merged successfully, ten are closed (superseded, conflicts, or review issues), and two are open but stale with merge conflicts. This sprint addresses every actionable item.
|
||||
|
||||
### Upstream Reviewer Roster
|
||||
|
||||
| Handle | Role | Pattern |
|
||||
|--------|------|---------|
|
||||
| `code-yeongyu` | CI/rebase gatekeeper | Requests rebase on any merge conflict; marks `CHANGES_REQUESTED` |
|
||||
| `1716775457damn` | Feature reviewer | Gives detailed positive feedback + suggestions; looks at UX and architecture |
|
||||
| `Yeachan-Heo` | Repo owner (gaebal-gajae/clawdbot) | Closes PRs that are "not reviewable" — reverting upstream commits triggers this |
|
||||
|
||||
### Critical Lesson Learned
|
||||
|
||||
Our LSP PRs (#3098, #3099) were closed by the repo owner because rebased branches **reverted upstream commits**. The net diff showed +731/−2309 across only 3 files, making it appear we were deleting upstream fixes rather than adding a feature. Root cause: the branch was forked before 5 critical upstream commits landed, and our rebases kept the old versions of `config.rs` and `main.rs`.
|
||||
|
||||
**Rule going forward:** Every new PR branch must be created from **current `upstream/main`**, not rebased from an old fork. Verify with `git log` that no upstream commits are missing before opening a PR.
|
||||
|
||||
### Commits That Must Never Be Reverted
|
||||
|
||||
| Commit | PR# | Description |
|
||||
|--------|-----|-------------|
|
||||
| `b64df991` | #698 | Process-lifetime dedup of config deprecation warnings (`EMITTED_CONFIG_WARNINGS` / `emit_config_warning_once`) + tempfile dev-dep fix |
|
||||
| `91a0681a` | #697 | Typed errors for providers |
|
||||
| `c345ce6d` | — | MCP/agents/skills envelope exit-1 |
|
||||
| `85e736c7` | — | Add status to sandbox JSON envelope |
|
||||
| `5b79413e` | #458 | Add status to version/init/system-prompt envelopes |
|
||||
|
||||
After branching from `upstream/main`, verify: `git log --oneline | grep -E "b64df99|91a0681|c345ce6|85e736c|5b79413"` — all five must be present.
|
||||
|
||||
---
|
||||
|
||||
## Completed PRs (No Action Needed)
|
||||
|
||||
| PR | Title | Status | Reviewer Comments |
|
||||
|----|-------|--------|-------------------|
|
||||
| #3017 | Interactive provider wizard with fast model selection | **MERGED** | Positive. Requested screenshots/description of wizard flow — address in future PRs. |
|
||||
| #3015 | Streaming robustness — OpenAI parsing, error detection, reasoning content | **MERGED** | Very positive. Called "solid groundwork for downstream feature PRs." |
|
||||
| #2832 | Git-aware context tools | **MERGED** | Positive. Suggested `GitShow --format` option → tracked as Sprint Item 8. |
|
||||
| #2811 | /resume latest searches all workspaces | **MERGED** | No reviewer feedback. |
|
||||
| #2808 | Auto-compact and retry on context window errors | **MERGED** | No reviewer feedback. |
|
||||
|
||||
---
|
||||
|
||||
## Superseded PRs (Stay Closed)
|
||||
|
||||
| PR | Title | Reason Closed | Why No Action |
|
||||
|----|-------|---------------|---------------|
|
||||
| #2983 | Handle reasoning/thinking content from models | Reviewer objected to `PR_DESCRIPTION.md` + referral links | **Superseded** — PR #3015 already merged the reasoning content parsing at the streaming layer. Our commit `c6da823` on `feat-tui` covers the runtime-layer additions. |
|
||||
| #2831 | Parallel tool execution + agent teams | Self-closed — too large for review | Split approach is correct. Individual features can be re-submitted. |
|
||||
| #2830 | Project rules with .claw/rules/ (v1) | Conflicts, superseded by #3018 | Replaced by v2 attempt. |
|
||||
| #2814 | Claw setup writes model at wrong JSON level | Closed | Fix was included in subsequent PRs. |
|
||||
|
||||
---
|
||||
|
||||
## Sprint Backlog
|
||||
|
||||
### Execution Order
|
||||
|
||||
```
|
||||
Item 1 → Item 8 → Item 4 → Item 3 → Item 2 → Item 6 → Item 5 → Item 7 → Item 9
|
||||
```
|
||||
|
||||
Rationale:
|
||||
- **Item 1 first** — Broken UX on upstream main right now (wizard saved but never read)
|
||||
- **Item 8 second** — Zero-dependency quick win, can ship while working on Item 1
|
||||
- **Item 4 before 5** — Both touch `openai_compat.rs`; LSP rebuild is easier after timeout/retry lands
|
||||
- **Items 5 and 7 last** — Highest risk/effort, benefit from all other changes being settled
|
||||
|
||||
---
|
||||
|
||||
### Item 1: Provider Config 3-Tier Fallback
|
||||
|
||||
**Priority:** P0 — BROKEN UX ON UPSTREAM MAIN
|
||||
**Effort:** 2–3 hours
|
||||
**Risk:** LOW
|
||||
**Related PR:** #2810 (open, stale)
|
||||
**Branch:** `feat/provider-config-fallback` (new, from `upstream/main`)
|
||||
|
||||
**Problem:** PR #3017 merged `setup_wizard.rs` but `anthropic.rs` and `openai_compat.rs` read API keys only from env vars. Settings saved by the wizard are never consumed. Users who run the wizard see no effect.
|
||||
|
||||
**Scope (4 files):**
|
||||
- `rust/crates/api/src/providers/mod.rs` — Add `read_env_or_config()`, `provider_config_value()`, `stored_provider_kind()`
|
||||
- `rust/crates/api/src/providers/anthropic.rs` — Replace inline `env::var()` calls with `read_env_or_config()`
|
||||
- `rust/crates/api/src/providers/openai_compat.rs` — Same treatment as anthropic.rs
|
||||
- `rust/crates/runtime/src/config.rs` — `save_user_provider_settings()` / `clear_user_provider_settings()` (may already be in from #3017; verify)
|
||||
|
||||
**3-Tier Resolution Logic:**
|
||||
1. Environment variable (highest priority)
|
||||
2. `.env` file
|
||||
3. Stored config in settings.json (lowest priority)
|
||||
|
||||
**Steps:**
|
||||
```bash
|
||||
git fetch upstream
|
||||
git checkout -b feat/provider-config-fallback upstream/main
|
||||
# Verify critical commits present:
|
||||
git log --oneline | grep -E "b64df99|91a0681|c345ce6|85e736c|5b79413"
|
||||
|
||||
# Transplant the config fallback functions from feat-tui
|
||||
# Verify per-file diff is clean — no reverts of upstream changes
|
||||
cargo test --workspace
|
||||
cargo clippy --workspace --all-targets -- -D warnings
|
||||
scripts/fmt.sh --check
|
||||
```
|
||||
|
||||
**Verification:**
|
||||
- [ ] `read_env_or_config()` resolves env → .env → stored config
|
||||
- [ ] `anthropic.rs` uses `read_env_or_config()` for API key
|
||||
- [ ] `openai_compat.rs` uses `read_env_or_config()` for API key
|
||||
- [ ] Settings saved by wizard are actually consumed by providers
|
||||
- [ ] All existing tests pass
|
||||
- [ ] No upstream commits are reverted in the diff
|
||||
|
||||
---
|
||||
|
||||
### Item 2: Wizard Entry Points
|
||||
|
||||
**Priority:** P0 — Dead code without this
|
||||
**Effort:** 3–4 hours
|
||||
**Risk:** LOW-MEDIUM
|
||||
**Depends on:** Item 1
|
||||
**Related PR:** #2810 (open, stale)
|
||||
**Branch:** `feat/provider-wizard-entry` (new, from `upstream/main`)
|
||||
|
||||
**Problem:** Without entry points (Ctrl+P, `/setup`, `claw setup`), `setup_wizard.rs` is unreachable code. Users cannot access the wizard that Item 1 makes functional.
|
||||
|
||||
**Scope (3 files):**
|
||||
- `rust/crates/rusty-claude-cli/src/input.rs` — Ctrl+P hotkey handling
|
||||
- `rust/crates/rusty-claude-cli/src/main.rs` — `/setup` command dispatch + `claw setup` subcommand
|
||||
- `rust/crates/commands/src/lib.rs` — `/setup` slash command registration
|
||||
|
||||
**Steps:**
|
||||
```bash
|
||||
git checkout -b feat/provider-wizard-entry upstream/main
|
||||
# Cherry-pick: "fix: Ctrl+P provider swap with visual feedback + clippy cleanup"
|
||||
# Manually add /setup dispatch and claw setup subcommand
|
||||
# Verify TUI input loop is not broken by Ctrl+P handler
|
||||
cargo test --workspace
|
||||
cargo clippy --workspace --all-targets -- -D warnings
|
||||
```
|
||||
|
||||
**Caveat:** `input.rs` Ctrl+P handling may need adjustment if the TUI input loop has diverged on upstream. Visual feedback for provider swap must not break the existing input flow.
|
||||
|
||||
**Verification:**
|
||||
- [ ] `/setup` command launches wizard from REPL
|
||||
- [ ] `claw setup` launches wizard from CLI
|
||||
- [ ] Ctrl+P triggers provider swap with visual feedback
|
||||
- [ ] Existing input handling unaffected
|
||||
- [ ] All tests pass
|
||||
|
||||
---
|
||||
|
||||
### Item 3: Auto-Compact & Retry on Context Window Errors
|
||||
|
||||
**Priority:** P1
|
||||
**Effort:** 3–5 hours
|
||||
**Risk:** MEDIUM
|
||||
**Independent:** Yes (but enhanced by Item 4's error detection improvements)
|
||||
**Branch:** `feat/auto-compact-retry` (new, from `upstream/main`)
|
||||
|
||||
**Problem:** Hitting a context window error is currently a hard stop. The session must be manually compacted and retried.
|
||||
|
||||
**Scope:**
|
||||
- `rust/crates/rusty-claude-cli/src/main.rs` — Catch context window overflow, invoke compact, retry turn
|
||||
- `rust/crates/api/src/error.rs` — `CONTEXT_WINDOW_ERROR_MARKERS` (enhanced by Item 4)
|
||||
|
||||
**Steps:**
|
||||
```bash
|
||||
git checkout -b feat/auto-compact-retry upstream/main
|
||||
# Cherry-pick the auto-compact-retry commit
|
||||
# Add max-retry guard to prevent infinite loops
|
||||
cargo test --workspace
|
||||
cargo clippy --workspace --all-targets -- -D warnings
|
||||
```
|
||||
|
||||
**Risk:** Must correctly identify context-window errors and avoid infinite retry loops (compact fails to shrink enough → retries forever). Add a `MAX_COMPACT_RETRIES = 3` guard.
|
||||
|
||||
**Verification:**
|
||||
- [ ] Context window error detected and auto-compaction triggered
|
||||
- [ ] Turn retried after successful compaction
|
||||
- [ ] Max retry guard prevents infinite loops
|
||||
- [ ] Non-context-window errors are not caught by this path
|
||||
- [ ] All tests pass
|
||||
|
||||
---
|
||||
|
||||
### Item 4: API Timeout/Retry — Full Rebase & Resubmit
|
||||
|
||||
**Priority:** P1
|
||||
**Effort:** 4–6 hours
|
||||
**Risk:** MEDIUM
|
||||
**Related PR:** #2816 (open, stale, needs rebase)
|
||||
**Branch:** `feat/api-timeout-retry-v2` (new, from `upstream/main`)
|
||||
|
||||
**Problem:** PR #2816 is open but hasn't been rebased since May 25. It has merge conflicts and `CHANGES_REQUESTED` status. The 6 commits are self-contained and valuable, but must be cherry-picked onto fresh `upstream/main`.
|
||||
|
||||
**Scope (9 files, 6 commits — cherry-pick individually):**
|
||||
|
||||
| # | Commit | Description | Note |
|
||||
|---|--------|-------------|------|
|
||||
| 1 | `ade853984` | Core: timeout config, Retry-After, configurable retry | Will conflict with `openai_compat.rs` |
|
||||
| 2 | `8a8834307` | Retry 400 with transient gateway error | |
|
||||
| 3 | `ed91a61ee` | "No parseable body" error marker | Complements Item 3 |
|
||||
| 4 | `453ab6421` | Optional `id` field in OpenAI response | **May already be in via PR #3015** — check, skip if so |
|
||||
| 5 | `baa8d1ba6` | HTML response detection in streaming | **May already be in via PR #3015** — check, skip if so |
|
||||
| 6 | `33d2f7892` | Raw JSON error detection in streaming | **May already be in via PR #3015** — check, skip if so |
|
||||
|
||||
**Steps:**
|
||||
```bash
|
||||
git fetch upstream
|
||||
git checkout -b feat/api-timeout-retry-v2 upstream/main
|
||||
|
||||
# Verify critical commits:
|
||||
git log --oneline | grep -E "b64df99|91a0681|c345ce6|85e736c|5b79413"
|
||||
|
||||
# Cherry-pick each commit, resolving conflicts per file
|
||||
git cherry-pick ade853984 # timeout/retry core
|
||||
git cherry-pick 8a8834307 # 400 transient retry
|
||||
git cherry-pick ed91a61ee # no-parseable-body marker
|
||||
|
||||
# Check if commits 4-6 are already in upstream:
|
||||
git log upstream/main --oneline | grep -i "optional.*id\|make id"
|
||||
git log upstream/main --oneline | grep -i "html response\|json error"
|
||||
|
||||
# If not in upstream, cherry-pick:
|
||||
git cherry-pick 453ab6421 # optional id
|
||||
git cherry-pick baa8d1ba6 # HTML detection
|
||||
git cherry-pick 33d2f7892 # JSON error detection
|
||||
|
||||
# Full verification:
|
||||
cd rust && cargo clippy --workspace --all-targets -- -D warnings && cargo test --workspace
|
||||
scripts/fmt.sh --check
|
||||
```
|
||||
|
||||
**After verification:** Push and update PR #2816, or close it and open a new PR with a clean diff.
|
||||
|
||||
**Verification:**
|
||||
- [ ] All 6 commits cherry-picked (or skipped if already upstream)
|
||||
- [ ] No upstream commits reverted in the diff
|
||||
- [ ] `--api-timeout` / `CLAW_API_TIMEOUT` works
|
||||
- [ ] Retry-After header parsing works
|
||||
- [ ] Exponential backoff with jitter
|
||||
- [ ] All tests pass
|
||||
|
||||
---
|
||||
|
||||
### Item 5: LSP Integration — Clean Rebuild from `upstream/main`
|
||||
|
||||
**Priority:** P2
|
||||
**Effort:** 12–16 hours
|
||||
**Risk:** HIGH
|
||||
**Do after:** Item 4 (avoids re-resolving same `openai_compat.rs` conflicts)
|
||||
**Related PRs:** #2813, #3016, #3098, #3099 (all closed)
|
||||
**Branch:** `feat/lsp-v3` (new, from `upstream/main`)
|
||||
|
||||
**Problem:** All 4 LSP PRs were closed because rebased branches reverted upstream commits. The repo owner explicitly stated: "Please rebuild the branch from current `origin/main` and verify locally before opening a new PR."
|
||||
|
||||
**Approach:** Fresh branch from `upstream/main`. Transplant LSP module files from `feat-tui`. Manually integrate modifications to existing files. Include reviewer-requested additions.
|
||||
|
||||
**New Files to Transplant (12 modules from `feat-tui`):**
|
||||
```
|
||||
rust/crates/runtime/src/lsp_client/mod.rs
|
||||
rust/crates/runtime/src/lsp_client/dispatch.rs
|
||||
rust/crates/runtime/src/lsp_client/types.rs
|
||||
rust/crates/runtime/src/lsp_client/tests.rs
|
||||
rust/crates/runtime/src/lsp_client/tests_lifecycle.rs
|
||||
rust/crates/runtime/src/lsp_discovery.rs
|
||||
rust/crates/runtime/src/lsp_process/mod.rs
|
||||
rust/crates/runtime/src/lsp_process/parse.rs
|
||||
rust/crates/runtime/src/lsp_process/tests.rs
|
||||
rust/crates/runtime/src/lsp_transport/mod.rs
|
||||
rust/crates/runtime/src/lsp_transport/tests.rs
|
||||
```
|
||||
|
||||
**Existing Files Modified (~10):**
|
||||
- `rust/crates/commands/src/lib.rs` — `/lsp` slash command registration
|
||||
- `rust/crates/tools/src/lib.rs` — LSP diagnostic injection into tool responses
|
||||
- `rust/crates/runtime/src/config.rs` — `lspAutoStart` config
|
||||
- `rust/crates/runtime/src/config_validate.rs` — LSP config validation
|
||||
- `rust/crates/runtime/src/lib.rs` — Module exports
|
||||
- `rust/crates/runtime/src/compact.rs` — LSP awareness during compaction
|
||||
- `rust/crates/runtime/src/policy_engine.rs` — LSP health policy rules
|
||||
- `rust/crates/runtime/src/sandbox.rs` — LSP process sandboxing
|
||||
- `rust/crates/rusty-claude-cli/src/main.rs` — LSP server lifecycle on startup
|
||||
- `rust/crates/api/src/providers/openai_compat.rs` — LSP-related config
|
||||
|
||||
**Reviewer-Requested Additions (from PR #2813 review by 1716775457damn):**
|
||||
- [ ] `/lsp status` command — users can check which servers are running
|
||||
- [ ] Configurable diagnostic wait timeout (don't block startup indefinitely)
|
||||
- [ ] Surface LSP server version info in status output
|
||||
|
||||
**Steps:**
|
||||
```bash
|
||||
git fetch upstream
|
||||
git checkout -b feat/lsp-v3 upstream/main
|
||||
|
||||
# Verify critical commits are present:
|
||||
git log --oneline | grep -E "b64df99|91a0681|c345ce6|85e736c|5b79413"
|
||||
# ALL FIVE must be present. If any missing, STOP — wrong base.
|
||||
|
||||
# Copy LSP module files from feat-tui branch:
|
||||
for f in lsp_client/mod.rs lsp_client/dispatch.rs lsp_client/types.rs \
|
||||
lsp_client/tests.rs lsp_client/tests_lifecycle.rs lsp_discovery.rs \
|
||||
lsp_process/mod.rs lsp_process/parse.rs lsp_process/tests.rs \
|
||||
lsp_transport/mod.rs lsp_transport/tests.rs; do
|
||||
git show feat-tui:rust/crates/runtime/src/$f > rust/crates/runtime/src/$f
|
||||
done
|
||||
|
||||
# Manually apply modifications to existing files
|
||||
# (commands, tools, config, lib, compact, policy_engine, sandbox, main, openai_compat)
|
||||
# DO NOT copy entire files from feat-tui — only add the LSP-specific sections
|
||||
# Compare: git diff upstream/main..feat-tui -- <file> | grep "^+" | grep -i lsp
|
||||
|
||||
# Add reviewer-requested features:
|
||||
# - /lsp status command
|
||||
# - Diagnostic wait timeout config
|
||||
# - LSP server version reporting
|
||||
|
||||
# Full verification:
|
||||
cd rust && cargo clippy --workspace --all-targets -- -D warnings && cargo test --workspace
|
||||
scripts/fmt.sh --check
|
||||
|
||||
# CRITICAL — Before pushing, verify no upstream commits are reverted:
|
||||
git diff upstream/main --stat
|
||||
# The diff should show ONLY additions and LSP-specific changes.
|
||||
# If any file shows large net-negative changes, STOP and investigate.
|
||||
```
|
||||
|
||||
**Verification:**
|
||||
- [ ] LSP servers auto-start on REPL launch (14 languages)
|
||||
- [ ] Tool responses append LSP diagnostics
|
||||
- [ ] Distro-aware install prompts work
|
||||
- [ ] `/lsp start|stop|restart|status` commands work
|
||||
- [ ] Diagnostic wait timeout is configurable
|
||||
- [ ] LSP server version info surfaced
|
||||
- [ ] No upstream commits are reverted in the diff
|
||||
- [ ] `git diff upstream/main --stat` shows feature-only changes
|
||||
- [ ] All tests pass
|
||||
|
||||
---
|
||||
|
||||
### Item 6: Session Resume Fixes
|
||||
|
||||
**Priority:** P1
|
||||
**Effort:** 1–2 hours
|
||||
**Risk:** LOW
|
||||
**Independent:** Yes
|
||||
**Related PR:** #2810 (session resume commits)
|
||||
**Branch:** `feat/session-resume-fixes` (new, from `upstream/main`)
|
||||
|
||||
**Scope (2 files):**
|
||||
- `rust/crates/runtime/src/session_control.rs` — `load_session_reference_excluding`
|
||||
- `rust/crates/rusty-claude-cli/src/main.rs` — Session resume path updates
|
||||
|
||||
**Steps:**
|
||||
```bash
|
||||
git checkout -b feat/session-resume-fixes upstream/main
|
||||
# Cherry-pick the 3 session resume commits from #2810
|
||||
cargo test --workspace
|
||||
cargo clippy --workspace --all-targets -- -D warnings
|
||||
```
|
||||
|
||||
**Verification:**
|
||||
- [ ] `/resume latest` correctly searches all workspaces
|
||||
- [ ] Session exclusion works
|
||||
- [ ] All tests pass
|
||||
|
||||
---
|
||||
|
||||
### Item 7: Project Rules — Clean Rebuild
|
||||
|
||||
**Priority:** P2
|
||||
**Effort:** 4–6 hours
|
||||
**Risk:** MEDIUM
|
||||
**Do after:** Items 1 and 5 (avoids `config.rs` / `prompt.rs` conflict re-resolution)
|
||||
**Related PR:** #3018 (closed, conflicts)
|
||||
**Branch:** `feat/project-rules-v3` (new, from `upstream/main`)
|
||||
|
||||
**Problem:** PR #3018 was closed due to merge conflicts. The `feat/project-rules` branch predates upstream changes. Must extract only the rules-specific diff and omit `ROADMAP.md`/`progress.txt` changes.
|
||||
|
||||
**Scope (rules-specific from commit `ddc788e`):**
|
||||
- `rust/crates/runtime/src/config.rs` — `.claw/rules/` config loading
|
||||
- `rust/crates/runtime/src/prompt.rs` — Rules injection into system prompt
|
||||
- ~10 other files with partial rules support
|
||||
|
||||
**Steps:**
|
||||
```bash
|
||||
git checkout -b feat/project-rules-v3 upstream/main
|
||||
# Extract only rules-specific changes from commit ddc788e
|
||||
# Use: git show ddc788e -- <files> to get clean patches
|
||||
# OMIT: ROADMAP.md, progress.txt (reviewer objections)
|
||||
# Verify: cargo test --workspace
|
||||
```
|
||||
|
||||
**Verification:**
|
||||
- [ ] `.claw/rules/` directory loaded and parsed
|
||||
- [ ] Multi-framework auto-import works (CLAUDE.md, .cursorrules, .windsurfrules, etc.)
|
||||
- [ ] Rules injected into system prompt
|
||||
- [ ] No `ROADMAP.md` or `progress.txt` in the diff
|
||||
- [ ] All tests pass
|
||||
|
||||
---
|
||||
|
||||
### Item 8: GitShow `--format` Enhancement
|
||||
|
||||
**Priority:** P1
|
||||
**Effort:** 1–2 hours
|
||||
**Risk:** VERY LOW
|
||||
**Independent:** Yes — quick win, do anytime
|
||||
**Related PR:** #2832 (merged — reviewer suggestion)
|
||||
**Branch:** `feat/gitshow-format` (new, from `upstream/main`)
|
||||
|
||||
**Problem:** The reviewer of our merged PR #2832 suggested adding a `GitShow --format` option so the model can request patch vs metadata selectively.
|
||||
|
||||
**Scope (1 file):**
|
||||
- `rust/crates/tools/src/lib.rs` — `GitShowInput` struct + `run_git_show()`
|
||||
|
||||
**Design:**
|
||||
```
|
||||
format: Option<String> // "patch" (default) | "stat" | "metadata"
|
||||
|
||||
"patch" → git show <ref> (full diff, default - backward compatible)
|
||||
"stat" → git show --stat <ref> (summary stats)
|
||||
"metadata" → git show --format=medium --no-patch <ref> (commit info only)
|
||||
```
|
||||
|
||||
**Steps:**
|
||||
```bash
|
||||
git checkout -b feat/gitshow-format upstream/main
|
||||
# In GitShowInput: add format: Option<String> (serde rename = "format")
|
||||
# In run_git_show: map format values to git flags
|
||||
# Update ToolSpec input_schema and tool description
|
||||
# Keep stat: Option<bool> with deprecation note for backward compat
|
||||
cargo test --workspace
|
||||
cargo clippy --workspace --all-targets -- -D warnings
|
||||
```
|
||||
|
||||
**Verification:**
|
||||
- [ ] `format: "patch"` produces full diff
|
||||
- [ ] `format: "stat"` produces summary stats
|
||||
- [ ] `format: "metadata"` produces commit info only
|
||||
- [ ] Default (no format) is "patch" — backward compatible
|
||||
- [ ] Tool description updated for model discoverability
|
||||
- [ ] All tests pass
|
||||
|
||||
---
|
||||
|
||||
### Item 9: README Referral Links
|
||||
|
||||
**Priority:** P3
|
||||
**Effort:** 30 minutes
|
||||
**Risk:** VERY LOW
|
||||
**Independent:** Yes
|
||||
**Related PR:** #2810 (4 referral link commits)
|
||||
**Branch:** `feat/readme-referral` (new, from `upstream/main`)
|
||||
|
||||
**Scope:** `README.md` only — purely cosmetic addition of provider referral links.
|
||||
|
||||
**Caveat:** PR #2983's reviewer objected to referral links as "unrelated promotional content." Submit as a standalone PR with a clear, narrow scope — if reviewers push back, drop without impact.
|
||||
|
||||
**Verification:**
|
||||
- [ ] Referral links added to README
|
||||
- [ ] No other files changed
|
||||
- [ ] markdown renders cleanly
|
||||
|
||||
---
|
||||
|
||||
## Dependency Graph
|
||||
|
||||
```
|
||||
Item 1 (provider config fallback) ─── P0
|
||||
├─> Item 2 (wizard entry points) ─── P0
|
||||
└─> Item 4 shares openai_compat.rs ── do before LSP
|
||||
└─> Item 5 (LSP rebuild) ────── shares main.rs, config.rs, openai_compat.rs
|
||||
└─> Item 7 (project rules) ── shares config.rs, prompt.rs
|
||||
|
||||
Item 3 (auto-compact-retry) ─── independent, enhanced by Item 4
|
||||
Item 6 (session resume) ──────── independent
|
||||
Item 8 (GitShow --format) ───── independent quick win
|
||||
Item 9 (README referral) ────── independent, lowest priority
|
||||
```
|
||||
|
||||
## Effort Summary
|
||||
|
||||
| # | Item | Effort | Risk | Priority | Branch Name |
|
||||
|---|------|--------|------|----------|-------------|
|
||||
| 1 | Provider config fallback | 2–3h | LOW | **P0** | `feat/provider-config-fallback` |
|
||||
| 2 | Wizard entry points | 3–4h | LOW-MED | **P0** | `feat/provider-wizard-entry` |
|
||||
| 3 | Auto-compact-retry | 3–5h | MED | P1 | `feat/auto-compact-retry` |
|
||||
| 4 | API timeout/retry v2 | 4–6h | MED | P1 | `feat/api-timeout-retry-v2` |
|
||||
| 5 | LSP clean rebuild | 12–16h | HIGH | P2 | `feat/lsp-v3` |
|
||||
| 6 | Session resume fixes | 1–2h | LOW | P1 | `feat/session-resume-fixes` |
|
||||
| 7 | Project rules rebuild | 4–6h | MED | P2 | `feat/project-rules-v3` |
|
||||
| 8 | GitShow --format | 1–2h | V.LOW | P1 | `feat/gitshow-format` |
|
||||
| 9 | README referral links | 30m | V.LOW | P3 | `feat/readme-referral` |
|
||||
|
||||
**Total estimated effort:** 31–45 hours
|
||||
|
||||
---
|
||||
|
||||
## PR Submission Checklist
|
||||
|
||||
For every PR opened against `ultraworkers/claw-code`:
|
||||
|
||||
- [ ] Branch created from **current `upstream/main`** (not rebased from old fork)
|
||||
- [ ] All 5 critical upstream commits are present in the branch: `b64df991`, `91a0681a`, `c345ce6d`, `85e736c7`, `5b79413e`
|
||||
- [ ] `git diff upstream/main --stat` shows **feature-only changes** — no large net-negative diffs
|
||||
- [ ] No `PR_DESCRIPTION.md` or other non-code files that belong in the PR body
|
||||
- [ ] No referral links unless the PR is specifically about that (Item 9)
|
||||
- [ ] No `ROADMAP.md` or `progress.txt` changes
|
||||
- [ ] `cargo test --workspace` passes
|
||||
- [ ] `cargo clippy --workspace --all-targets -- -D warnings` passes
|
||||
- [ ] `scripts/fmt.sh --check` passes
|
||||
- [ ] PR body includes clear description, screenshots if applicable
|
||||
- [ ] PR is scoped to a single feature/fix
|
||||
|
||||
---
|
||||
|
||||
## Reviewer Feedback Tracker
|
||||
|
||||
| Reviewer | Feedback | Action | Sprint Item |
|
||||
|----------|----------|--------|-------------|
|
||||
| code-yeongyu | Rebase needed on PRs #2810, #2816 | Fresh branches from upstream/main | Items 1, 4 |
|
||||
| code-yeongyu | LSP PRs revert upstream commits | Rebuild from scratch, verify diff | Item 5 |
|
||||
| 1716775457damn | Add `/lsp status` command | Include in LSP rebuild | Item 5 |
|
||||
| 1716775457damn | Make diagnostics configurable (wait timeout) | Include in LSP rebuild | Item 5 |
|
||||
| 1716775457damn | Surface LSP server version info | Include in LSP rebuild | Item 5 |
|
||||
| 1716775457damn | Add `GitShow --format` for patch vs metadata | Standalone PR | Item 8 |
|
||||
| 1716775457damn | Add screenshots for wizard flow | Address in future PR description | — |
|
||||
| 1716775457damn | Positive on provider wizard 3-tier auth | Confirm working with Item 1 | Item 1 |
|
||||
| Yeachan-Heo | LSP PRs "not reviewable" — rebuild from main | Clean rebuild | Item 5 |
|
||||
|
|
@ -756,11 +756,7 @@ fn now_unix_timestamp() -> u64 {
|
|||
}
|
||||
|
||||
fn read_env_non_empty(key: &str) -> Result<Option<String>, ApiError> {
|
||||
match std::env::var(key) {
|
||||
Ok(value) if !value.is_empty() => Ok(Some(value)),
|
||||
Ok(_) | Err(std::env::VarError::NotPresent) => Ok(super::dotenv_value(key)),
|
||||
Err(error) => Err(ApiError::from(error)),
|
||||
}
|
||||
super::read_env_or_config(key)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
@ -781,7 +777,10 @@ fn read_auth_token() -> Option<String> {
|
|||
|
||||
#[must_use]
|
||||
pub fn read_base_url() -> String {
|
||||
std::env::var("ANTHROPIC_BASE_URL").unwrap_or_else(|_| DEFAULT_BASE_URL.to_string())
|
||||
super::read_env_or_config("ANTHROPIC_BASE_URL")
|
||||
.ok()
|
||||
.flatten()
|
||||
.unwrap_or_else(|| DEFAULT_BASE_URL.to_string())
|
||||
}
|
||||
|
||||
fn request_id_from_headers(headers: &reqwest::header::HeaderMap) -> Option<String> {
|
||||
|
|
|
|||
|
|
@ -391,9 +391,65 @@ pub fn detect_provider_kind(model: &str) -> ProviderKind {
|
|||
if std::env::var_os("OPENAI_BASE_URL").is_some() {
|
||||
return ProviderKind::OpenAi;
|
||||
}
|
||||
// Fallback: check stored provider config from setup wizard.
|
||||
if let Some(kind) = stored_provider_kind() {
|
||||
return kind;
|
||||
}
|
||||
ProviderKind::Anthropic
|
||||
}
|
||||
|
||||
/// Look up a stored provider config value by env var name.
|
||||
/// Returns the stored API key or base URL when the env var matches the
|
||||
/// configured provider kind, enabling the setup wizard to persist credentials
|
||||
/// that work without shell env vars.
|
||||
pub fn provider_config_value(key: &str) -> Option<String> {
|
||||
let cwd = std::env::current_dir().ok()?;
|
||||
let config = runtime::ConfigLoader::default_for(&cwd).load().ok()?;
|
||||
let provider = config.provider();
|
||||
let kind = provider.kind()?;
|
||||
match (key, kind) {
|
||||
("ANTHROPIC_API_KEY" | "ANTHROPIC_AUTH_TOKEN", "anthropic") => provider.api_key().map(ToOwned::to_owned),
|
||||
("ANTHROPIC_BASE_URL", "anthropic") => provider.base_url().map(ToOwned::to_owned),
|
||||
("XAI_API_KEY", "xai") => provider.api_key().map(ToOwned::to_owned),
|
||||
("XAI_BASE_URL", "xai") => provider.base_url().map(ToOwned::to_owned),
|
||||
("OPENAI_API_KEY", "openai") => provider.api_key().map(ToOwned::to_owned),
|
||||
("OPENAI_BASE_URL", "openai") => provider.base_url().map(ToOwned::to_owned),
|
||||
("DASHSCOPE_API_KEY", "dashscope") => provider.api_key().map(ToOwned::to_owned),
|
||||
("DASHSCOPE_BASE_URL", "dashscope") => provider.base_url().map(ToOwned::to_owned),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Read an env var with a 3-tier fallback: process env -> .env file -> stored config.
|
||||
/// Environment variables always take priority over stored settings.
|
||||
pub fn read_env_or_config(key: &str) -> Result<Option<String>, ApiError> {
|
||||
match std::env::var(key) {
|
||||
Ok(value) if !value.is_empty() => return Ok(Some(value)),
|
||||
Ok(_) | Err(std::env::VarError::NotPresent) => {}
|
||||
Err(error) => return Err(ApiError::from(error)),
|
||||
}
|
||||
if let Some(value) = dotenv_value(key) {
|
||||
return Ok(Some(value));
|
||||
}
|
||||
if let Some(value) = provider_config_value(key) {
|
||||
return Ok(Some(value));
|
||||
}
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
/// Return the stored ProviderKind from config, if set.
|
||||
fn stored_provider_kind() -> Option<ProviderKind> {
|
||||
let cwd = std::env::current_dir().ok()?;
|
||||
let config = runtime::ConfigLoader::default_for(&cwd).load().ok()?;
|
||||
let kind = config.provider().kind()?;
|
||||
match kind {
|
||||
"anthropic" => Some(ProviderKind::Anthropic),
|
||||
"xai" => Some(ProviderKind::Xai),
|
||||
"openai" => Some(ProviderKind::OpenAi),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub const fn model_family_identity_for_kind(kind: ProviderKind) -> runtime::ModelFamilyIdentity {
|
||||
match kind {
|
||||
|
|
|
|||
|
|
@ -1697,11 +1697,7 @@ fn parse_sse_frame(
|
|||
}
|
||||
|
||||
fn read_env_non_empty(key: &str) -> Result<Option<String>, ApiError> {
|
||||
match std::env::var(key) {
|
||||
Ok(value) if !value.is_empty() => Ok(Some(value)),
|
||||
Ok(_) | Err(std::env::VarError::NotPresent) => Ok(super::dotenv_value(key)),
|
||||
Err(error) => Err(ApiError::from(error)),
|
||||
}
|
||||
super::read_env_or_config(key)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
|
|
@ -1714,7 +1710,10 @@ pub fn has_api_key(key: &str) -> bool {
|
|||
|
||||
#[must_use]
|
||||
pub fn read_base_url(config: OpenAiCompatConfig) -> String {
|
||||
std::env::var(config.base_url_env).unwrap_or_else(|_| config.default_base_url.to_string())
|
||||
super::read_env_or_config(config.base_url_env)
|
||||
.ok()
|
||||
.flatten()
|
||||
.unwrap_or_else(|| config.default_base_url.to_string())
|
||||
}
|
||||
|
||||
fn chat_completions_endpoint(base_url: &str) -> String {
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ pub enum ReadOutcome {
|
|||
struct SlashCommandHelper {
|
||||
completions: Vec<String>,
|
||||
current_line: RefCell<String>,
|
||||
ctrl_p_pending: RefCell<bool>,
|
||||
}
|
||||
|
||||
impl SlashCommandHelper {
|
||||
|
|
@ -30,6 +31,7 @@ impl SlashCommandHelper {
|
|||
Self {
|
||||
completions: normalize_completions(completions),
|
||||
current_line: RefCell::new(String::new()),
|
||||
ctrl_p_pending: RefCell::new(false),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -90,6 +92,13 @@ impl Highlighter for SlashCommandHelper {
|
|||
}
|
||||
|
||||
fn highlight_char(&self, line: &str, _pos: usize, _kind: CmdKind) -> bool {
|
||||
// Detect Ctrl+P: when previous line was empty and the new line is
|
||||
// just "P", that's Ctrl+P on an empty buffer (AcceptLine inserts
|
||||
// the character then submits). Set flag so read_line can intercept.
|
||||
let prev = self.current_line();
|
||||
if prev.is_empty() && line == "P" {
|
||||
*self.ctrl_p_pending.borrow_mut() = true;
|
||||
}
|
||||
self.set_current_line(line);
|
||||
false
|
||||
}
|
||||
|
|
@ -115,6 +124,11 @@ impl LineEditor {
|
|||
editor.set_helper(Some(SlashCommandHelper::new(completions)));
|
||||
editor.bind_sequence(KeyEvent(KeyCode::Char('J'), Modifiers::CTRL), Cmd::Newline);
|
||||
editor.bind_sequence(KeyEvent(KeyCode::Enter, Modifiers::SHIFT), Cmd::Newline);
|
||||
// Ctrl+P: accept line to trigger provider wizard in the REPL loop
|
||||
editor.bind_sequence(
|
||||
KeyEvent(KeyCode::Char('P'), Modifiers::CTRL),
|
||||
Cmd::AcceptLine,
|
||||
);
|
||||
|
||||
Self {
|
||||
prompt: prompt.into(),
|
||||
|
|
@ -147,7 +161,20 @@ impl LineEditor {
|
|||
}
|
||||
|
||||
match self.editor.readline(&self.prompt) {
|
||||
Ok(line) => Ok(ReadOutcome::Submit(line)),
|
||||
Ok(line) => {
|
||||
// Check if Ctrl+P was detected by the highlighter.
|
||||
// The highlighter sets a flag when it sees the previous
|
||||
// empty line change to uppercase "P", which is what
|
||||
// Ctrl+P + AcceptLine produces on an empty buffer.
|
||||
let is_ctrl_p = self
|
||||
.editor
|
||||
.helper()
|
||||
.is_some_and(|h| h.ctrl_p_pending.replace(false));
|
||||
if is_ctrl_p {
|
||||
return Ok(ReadOutcome::Submit("/setup".to_string()));
|
||||
}
|
||||
Ok(ReadOutcome::Submit(line))
|
||||
}
|
||||
Err(ReadlineError::Interrupted) => {
|
||||
let has_input = !self.current_line().is_empty();
|
||||
self.finish_interrupted_read()?;
|
||||
|
|
|
|||
Loading…
Reference in New Issue