fix(llm): trim whitespace and carriage returns from env-sourced endpo…

[Shashwat-911]

…ints

Description

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring (no functional changes)
• Documentation update
• CI / Build / Tooling

How Has This Been Tested?

• make test passes locally
• Manual testing (describe below)

Checklist

• My code follows the project's coding style (go fmt, go vet)
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or my feature works
• New and existing unit tests pass locally with my changes
• I have updated the documentation accordingly (if applicable)
• I have signed the CLA
• I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Related Issues

[CLAassistant]

All committers have signed the CLA.

[github-actions]

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

• ✅ Successfully posted inline: 0 comment(s)
• ❌ Failed to post inline: 3 comment(s)

---

📄 internal/llm/client.go (L993-L996)

⚠️ GitHub could not post this as an inline comment: Lines 993-996 could not be resolved (outside PR diff hunks)

This comment is now stale: it states "this path does not pass WithoutEnvironmentDefaults", but option.WithoutEnvironmentDefaults() was just added on line 988. The rationale for httpClientWithHeaderTimeout may still be valid, but the comment text should be updated to reflect the current code.

---

📄 internal/llm/resolver.go (L514-L516)

⚠️ GitHub could not post this as an inline comment: Lines 514-516 could not be resolved (outside PR diff hunks)

Inconsistent trimming of modelOverride: unlike tryOCREnv, tryCCEnv, and parseShellRC which all apply strings.TrimSpace(modelOverride) before use, this function uses the raw value. This means a model override with leading/trailing whitespace (e.g., from user input) will fail the ModelListContains check on line 516 even though the trimmed value would match, producing a misleading "model not available" error. The value should be trimmed before both the validation and assignment.

💡 Suggested Change

Before:

	if modelOverride != "" {
		if gateOverrideOnModelList && len(availableModels) > 0 {
			if !ModelListContains(availableModels, modelOverride) {

After:

	if modelOverride = strings.TrimSpace(modelOverride); modelOverride != "" {
		if gateOverrideOnModelList && len(availableModels) > 0 {
			if !ModelListContains(availableModels, modelOverride) {

---

📄 internal/llm/resolver.go (L603-L606)

⚠️ GitHub could not post this as an inline comment: Lines 603-606 could not be resolved (outside PR diff hunks)

Same inconsistency as tryProviderConfig: modelOverride is not trimmed here, unlike in tryOCREnv, tryCCEnv, and parseShellRC. A whitespace-only modelOverride (e.g., " ") would pass the model == "" check on line 623 (since it's non-empty), making the block appear complete when it shouldn't be. After finalizeResolvedEndpoint trims it, the resulting endpoint would have an empty model. Apply strings.TrimSpace to modelOverride before use.

💡 Suggested Change

Before:

	model := cfg.Llm.Model
	if modelOverride != "" {
		model = modelOverride
	}

After:

	model := cfg.Llm.Model
	if modelOverride = strings.TrimSpace(modelOverride); modelOverride != "" {
		model = modelOverride
	}

[NanaseInori]

The resolver-side whitespace normalization makes sense. There is substantial overlap with #1414, and #1415 has already been closed in favor of that implementation, so please coordinate on a single patch for the shared fix.

This PR also changes client-constructor normalization and introduces option.WithoutEnvironmentDefaults() for Anthropic. Those changes go beyond trimming environment reads. Before approving that broader part, I would like to see its intended environment/authentication precedence documented and covered by a request-level regression test. Rebasing the remaining changes after the shared fix lands would make that review much clearer.

This review was conducted by Qiyuanqiii's review bot, using the model GPT 6 Astra Max. If you need a human review, please manually @.