fix(config): warn when llm.* settings are silently ignored due to active provider

[aalhadxx]

Problem

When a provider is already active, writing an llm.* setting via the CLI succeeds without error but is discarded at runtime. The resolver checks cfg.Provider != "" and invokes tryProviderConfig, leaving tryLegacyLlmConfig as the fallback path. Users have no idea their setting was ignored.

Reproduction

1. ocr config provider → select any provider (e.g. anthropic)
2. ocr config set llm.url https://custom.example.com → succeeds silently
3. At runtime, llm.url is ignored because the provider takes priority.

Fix

• runConfigSet: issue a stderr warning when llm.* values are written while a provider is present.
• runConfigUnset: enable unsetting the provider and model fields so users can switch back to legacy llm config.

The resolver's priority ordering is intentional and remains untouched.

Test

Added regression tests:

• TestRunConfigSetWarnsWhenLlmShadowedByProvider
• TestRunConfigSetNoWarningWhenProviderNotActive
• TestRunConfigUnsetProvider
• TestRunConfigUnsetModel
• TestRunConfigUnsetInvalidKey

Fixes #583

[CLAassistant]

All committers have signed the CLA.

[github-actions]

🔍 OpenCodeReview found 1 issue(s) in this PR.

• ✅ Successfully posted inline: 1 comment(s)

[lizhengfeng101]

Code Review

Overview

This PR addresses a real UX pain point: llm.* settings are silently ignored when a provider is active. The fix is conservative — it adds warnings without touching the resolver's priority logic — and comes with solid test coverage.

However, the PR bundles an unrelated VS Code fix and has a subtle bug in the unset model path.

---

Issues

1. Unrelated change: VS Code shell: true fix

The CliService.ts changes (Windows shell option) have nothing to do with config warnings. Mixing unrelated fixes complicates reverts and muddies git blame.

Suggestion: Split into a separate PR with its own issue reference.

2. runConfigUnset("model") does not clear top-level cfg.Model when a provider is active

case "model":
    if cfg.Provider != "" {
        // Only clears the provider entry's Model field
        // Top-level cfg.Model is NOT cleared
    } else {
        cfg.Model = ""
    }

If a user previously set both a top-level Model and a provider entry model, the top-level value will persist as a stale ghost. The test passes because the fixture happens to have Model: "claude-opus-4-6" which gets serialized with omitempty (or similar) — but the in-memory struct still holds the old value.

Suggestion: Also set cfg.Model = "" in the provider-active branch to avoid residual state.

3. os.Stderr redirection in tests is fragile

oldStderr := os.Stderr
r, w, _ := os.Pipe()
os.Stderr = w

This mutates global state. If anyone adds t.Parallel() in the future, tests will race. Consider injecting an io.Writer parameter into runConfigSet/runConfigUnset, or at minimum document that these tests must not run in parallel.

4. os.Pipe() error is discarded

r, w, _ := os.Pipe()

Minor, but should be:

r, w, err := os.Pipe()
if err != nil {
    t.Fatalf("os.Pipe: %v", err)
}

5. unset provider silently clears model — document this

The behavior of clearing model when unsetting provider is reasonable, but it should be mentioned in CLI help text or docs so users aren't surprised.

---

Nits

• VS Code test: expect.not.objectContaining({ shell: true }) works but expect.objectContaining({ shell: false }) would be more explicit about intent.

---

Summary

Aspect	Verdict
Correctness	Bug in unset model path (residual top-level Model)
Scope	VS Code fix should be a separate PR
Tests	Good coverage, but fragile stderr pattern
Style	Clean, follows project conventions

Recommendation: Split the VS Code fix, address the cfg.Model residual bug, then this is good to merge.

[lizhengfeng101]

Hi @aalhadxx, thank you for your contribution to this issue! We appreciate the effort you put into this PR.

However, since we haven't heard back from you regarding the review feedback, and the CI checks are currently failing, we've decided to move forward and merge #588 which addresses the same problem.

That said, we'd love to see you continue contributing! Feel free to pick up any other open issues or propose new improvements — contributions of all kinds are welcome. Thanks again!