fix: apply config.temperature to LLM client and expose it via opencli config

[zjshen14]

Summary

• Core fix: createAgent() now uses temperature ?? config.temperature instead of only the explicit CLI flag, so the value in ~/.opencli/config.json is applied to every LLM call when no --temperature flag is given
• Config command: Adds --temperature <float> to opencli config so users can set the default temperature without manually editing the JSON file

Why this was broken

Config declares temperature: number (default 0.7) and loadConfig() reads it correctly, but createAgent() passed the raw temperature parameter to createClient() — which is undefined when neither startChat nor runSingle received an explicit value. The effect: config.temperature was dead config that had no effect on any LLM call, while config.maxTokens, config.historySize, and config.autoCompact were all applied correctly.

Related: opencli config had no --temperature option, making the field inaccessible via the CLI.

Test plan

• npm run typecheck && npm run lint && npm run format:check && npm test — all pass (710 tests)
• opencli config --temperature 0.2 saves the value to ~/.opencli/config.json
• opencli config (no args) still prints current config
• opencli run --temperature 0.5 "…" still takes precedence over config value (?? operator)
• No change to provider implementations, agent core, or tools

Closes #251

🤖 Generated with Claude Code

---

Generated by Claude Code

[zjshen14]

Review: fix: apply config.temperature to LLM client and expose it via opencli config

What's good: The root cause diagnosis is correct and the fix is minimal. temperature ?? config.temperature in createAgent() gives the right precedence — explicit --temperature flag wins, config file is the fallback — matching how maxTokens, historySize, and autoCompact already work. The opencli config --temperature surface wires cleanly to saveConfig(), and the existing "print current config when nothing was provided" guard was correctly extended to cover the new option.

Only src/cli/index.ts changed (8 additions, 2 deletions). All three CI checks pass. No layering concerns — this stays in the CLI layer as expected.

---

Not merging: This PR is authored by the same account the review bot runs as, so self-merge is not permitted. This is a clear-cut fix that can be merged directly by the maintainer.

Recommendation: Merge.

---

Generated by Claude Code

[zjshen14]

Review: fix: apply config.temperature to LLM client and expose it via opencli config

What's good: Root cause correctly identified and the fix is precise. temperature ?? config.temperature in createAgent() ensures the persisted config value is used when no CLI flag is passed. The --temperature option is correctly added to the config command, the save path is properly guarded (if (opts.temperature !== undefined)), and the no-op print-current-config condition is correctly extended to cover the new option. The ?? operator is the right choice over || — it passes through an explicit 0 correctly (zero temperature is valid and meaningful), whereas || would silently substitute the config default for a falsy CLI value. CI is all green.

---

Notes

1. No test for the temperature ?? config.temperature fallback (src/cli/index.ts:342)

The exact bug — config.temperature being silently ignored when no --temperature flag is given — is straightforward to regress if someone refactors createAgent(). A test that stubs loadConfig() returning { temperature: 0.3 } and asserts createClient is called with temperature: 0.3 when the flag is absent would lock this in as a durable artifact. Not a blocker, but worth adding as a follow-up or in this PR before landing.

2. mergeable_state is "unknown" — bot is not auto-merging

GitHub's mergeability check is lazily computed; "unknown" means it hasn't been evaluated yet, not that a conflict exists. CI is green across all three checks. If the branch is clean when you look, this is safe to merge.

---

Recommendation: Merge. Consider adding a test for the temperature ?? config.temperature path as a follow-up.

---

Generated by Claude Code

[zjshen14]

Heads up before merging: dependency on PR #230 (o1 temperature fix)

After this PR lands, createAgent() will pass config.temperature (default: 0.7) to every LLM call. That's the intended behaviour — but it creates a regression for o1/o1-mini/o1-preview users.

The current code at openai.ts:77 guards with:

...(this.temperature !== undefined ? { temperature: this.temperature } : {}),

Today, startChat() passes no temperature → undefined → guard fires → o1 models receive no temperature parameter → ✅ OK.

After this PR: temperature ?? config.temperature = 0.7 → guard no longer fires → temperature is sent → o1 API returns 400 Invalid parameter (o1 temperature is fixed at 1 server-side).

PR #230 already has the correct fix: a lacksTemperatureSupport() helper that extends the guard to also skip temperature for o1 models. Both src/providers/openai.ts and src/providers/openai.test.ts are untouched since #230's base commit, so it applies cleanly to current main.

Recommended merge order: #230 first, then this PR. Merging this PR alone will silently break anyone who has model: "o1-mini" (or similar) in ~/.opencli/config.json.

---

Generated by Claude Code

[zjshen14]

Status update: fix: apply config.temperature to LLM client

New commit pushed (afe9019). CI is now fully green — all three checks pass (Typecheck/Lint/Test ✓, Analyze (javascript) ✓, CodeQL ✓).

The diff is unchanged from the prior review: a single line in src/cli/index.ts replacing temperature with temperature: temperature ?? config.temperature. The fix is correct and minimal.

Dependency on #230 still present

The merge-order constraint noted in the previous comment still applies. Once this PR lands, createAgent() will propagate config.temperature (default: 0.7) to every LLM call. Users with o1, o1-mini, or o1-preview in ~/.opencli/config.json will start receiving a 400 Invalid parameter error from the OpenAI API — those models reject any temperature parameter.

PR #230 (fix(openai): omit temperature for o1/o1-mini/o1-preview) carries the correct guard (lacksTemperatureSupport()) and must land before this one.

Recommended merge order: #230 → this PR (#252).

---

Generated by Claude Code

[zjshen14]

Status update: fix: apply config.temperature to LLM client

New commit pushed (d632df4). CI is fully green — all three checks pass (Typecheck/Lint/Test ✓, Analyze (javascript) ✓, CodeQL ✓). mergeable_state is clean.

The diff is unchanged from the prior reviews: a single-file change to src/cli/index.ts that adds --temperature to the config command and fixes temperature ?? config.temperature in createAgent(). The fix is correct and minimal.

Dependency on #230 still unresolved

The merge-order constraint remains the blocker. Once this PR lands, createAgent() will propagate config.temperature (default 0.7) to every LLM call. The current guard in src/providers/openai.ts (line ...(this.temperature !== undefined ? { temperature: this.temperature } : {})) only skips temperature when it's undefined — it has no model-name check. After this PR, that guard never fires, so o1/o1-mini/o1-preview users will start receiving a 400 Invalid parameter from the OpenAI API on every call.

PR #230 (fix(openai): omit temperature for o1/o1-mini/o1-preview) carries the correct guard (lacksTemperatureSupport()) and is not yet merged.

Recommended merge order: #230 first, then this PR (#252).

---

Generated by Claude Code