Add GPT 6 Astra and Claude Fable 5.1; refresh Go dependencies - #186
Conversation
Sigilix OverviewEffort: 4/5 (large) Quality gates
Summary — latest pushAdds GPT 6 Astra and Claude Fable 5.1 model support, persisting the Codex model selection separately from Claude preferences across resumed turns and Important files
Sequence diagramsequenceDiagram
participant User
participant TUI as Picker (tui_backend)
participant Repl as Orchestrator (repl.go)
participant Main as Main (main.go)
participant Agent as Agent / CodexRunner
User->>TUI: Select model & backend
TUI->>Repl: result (backend, modelID)
Repl->>Repl: persistOrchModelSelection()
alt backend == codex
Repl->>Repl: cfg.CodexModel = modelID
else backend != codex
Repl->>Repl: cfg.ModelOverride = modelID
end
Repl->>Agent: NewCodexAgent(modelID) / applyAPIModelSelection(modelID)
Agent->>Agent: Route to selected model
Agent-->>User: Response
Confidence: 2/5The model resolution refactor introduces backend-specific branching logic with unverified edge cases around auto-routing and persistence, and the separate Codex model persistence path could silently erase user preferences if the branching conditions are not exhaustive.
Suggested labels:
|
QualityMax ReviewVerdict: COMMENT · Confidence: evidence-backed scan Files eligible: 29 · Files reviewed: 29 · Files with findings: 0 · Findings: 0 · Inline cards: 0 Priority findings
Review gates
Important files
Change diagram — Flowgraph TD
User[User Selects Model] --> TUI[TUI Picker]
TUI --> Persist[persistOrchModelSelection]
Persist --> Config[Update Config]
Config --> Agent[Agent Execution]
Agent --> API[API Call]
API --> Filter[Filter Thinking Blocks]
Filter --> Response[Return Cleaned Response]
Review lifecycleUse the inline cards to inspect evidence and suggested remediation. Re-run the QualityMax review after pushing a fix; unchanged cards are identified by their stable finding marker. Dismiss with a reason through the existing QualityMax/GitHub review feedback flow. 0 prior card(s) are stale/resolved on this head. Proof legend: VERIFIED independently judged patch · REPRODUCED verified finding · GROUNDED deterministic evidence · MODEL-ONLY model judgment. QualityMax project results are available in the configured project. Receipt · commit |
sigilix
Bot
added
documentation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
![qualitymaxapp[bot]](https://avatars.githubusercontent.com/in/2748338?s=60&v=4){kind=small}
![sigilix[bot]](https://avatars.githubusercontent.com/in/3558711?s=60&v=4){kind=small}
|
| Gate | Result |
|---|---|
| 🔍 AI diff review | ✅ Clean · gemini-3.1-flash-lite · completed · 23 eligible / 23 reviewed · gemini-3.1-flash-lite |
| 🔍 SAST | completed · 29 eligible / 29 reviewed · qwen3.7-plus |
| 🔍 Canonical PR review delivery | completed · 0 eligible / 0 reviewed · exact-head review #5119954367 and overview #5547501742 confirmed |
| 🧪 Repo Tests | ✅ 751/751 passed (go) |
Powered by QualityMax — AI-Powered Test Automation
Dismissed after checking all four findings against the code and passing targeted tests. In-place compaction is safe; display labels do not enter persisted model IDs; resolver mutation is intentional and documented; picker and persistence cases are already covered. Per-finding evidence is recorded in the inline replies.
| if !api.IsValidClaudeModelName(model) { | ||
| return "", fmt.Errorf("unrecognized Claude model %q; valid: %s", model, api.ValidClaudeModelsHelp()) | ||
| } | ||
| if cfg.Backend == "cc" && requested != "" { | ||
| cfg.ModelOverride = model |
There was a problem hiding this comment.
resolveSessionModel writes cfg.ModelOverride = model for the cc backend even when model is the resolved default, resurrecting a previously-cleared override
When cfg.Backend == "cc" and requested != "", the function sets cfg.ModelOverride = model where model is the result of api.ResolveClaudeModel(model). If requested is a non-empty alias like "fable" that resolves to api.ModelFable51, this is correct. But if requested is a non-empty value that resolves to the same model as the saved default, the write is still performed, overwriting any prior ModelOverride that may have been intentionally cleared (e.g., by a previous --model auto invocation). The if model == "auto" guard only clears the override when the resolved model is exactly "auto"; it does not handle the case where a non-auto requested model should clear a stale override. The invariant that ModelOverride reflects the user's most recent explicit CLI choice is broken when a non-auto request follows a cleared override.
More Info
- Threat model: A user who previously ran
qmax-code --model auto(clearingModelOverride) and then runsqmax-code --model fablewill haveModelOverrideset toclaude-fable-5-1. If they later runqmax-code --model autoagain, the override is cleared. But if they runqmax-code --model sonnet(a non-auto alias), the override is set toclaude-sonnet-5, which is correct. The bug is that a non-auto request always writes the override, even when the resolved model equals the savedDefaultModel, so a user who clears the override and then requests the default model by its full ID will have the override re-set, causing the next session to use the override instead of the saved default. - Specific code citations:
main_models.golines 36-44:if cfg.Backend == "cc" && requested != "" { cfg.ModelOverride = model; if model == "auto" { cfg.ModelOverride = "" } } - Existing protections: The
if model == "auto"guard only clears the override for the literal"auto"value; it does not comparemodelagainstcfg.DefaultModelor any other sentinel. - Proposed mitigation: Only write
cfg.ModelOverridewhenmodel != cfg.DefaultModel, or clear it whenmodel == cfg.DefaultModelto preserve the invariant that the override is only set for a non-default explicit choice. - Alternative mitigations considered: A separate
--modelflag that always writes the override and a--reset-modelflag to clear it would be more explicit but is a larger API change. - Severity calibration: The bug requires a specific sequence of user actions (clear override, then request the default model by full ID) and only affects the next session's model selection, so it is a likely production bug under plausible conditions but not a data-loss or crash.
Prompt To Fix With AI
This is a comment left during a code review.
Path: main_models.go
Line: 36-40
Comment:
**`resolveSessionModel` writes `cfg.ModelOverride = model` for the `cc` backend even when `model` is the resolved default, resurrecting a previously-cleared override**
When `cfg.Backend == "cc"` and `requested != ""`, the function sets `cfg.ModelOverride = model` where `model` is the result of `api.ResolveClaudeModel(model)`. If `requested` is a non-empty alias like `"fable"` that resolves to `api.ModelFable51`, this is correct. But if `requested` is a non-empty value that resolves to the same model as the saved default, the write is still performed, overwriting any prior `ModelOverride` that may have been intentionally cleared (e.g., by a previous `--model auto` invocation). The `if model == "auto"` guard only clears the override when the resolved model is exactly `"auto"`; it does not handle the case where a non-auto requested model should clear a stale override. The invariant that `ModelOverride` reflects the user's most recent explicit CLI choice is broken when a non-auto request follows a cleared override.
Threat model:
A user who previously ran `qmax-code --model auto` (clearing `ModelOverride`) and then runs `qmax-code --model fable` will have `ModelOverride` set to `claude-fable-5-1`. If they later run `qmax-code --model auto` again, the override is cleared. But if they run `qmax-code --model sonnet` (a non-auto alias), the override is set to `claude-sonnet-5`, which is correct. The bug is that a non-auto request always writes the override, even when the resolved model equals the saved `DefaultModel`, so a user who clears the override and then requests the default model by its full ID will have the override re-set, causing the next session to use the override instead of the saved default.
Specific code citations:
`main_models.go` lines 36-44: `if cfg.Backend == "cc" && requested != "" { cfg.ModelOverride = model; if model == "auto" { cfg.ModelOverride = "" } }`
Existing protections:
The `if model == "auto"` guard only clears the override for the literal `"auto"` value; it does not compare `model` against `cfg.DefaultModel` or any other sentinel.
Proposed mitigation:
Only write `cfg.ModelOverride` when `model != cfg.DefaultModel`, or clear it when `model == cfg.DefaultModel` to preserve the invariant that the override is only set for a non-default explicit choice.
Alternative mitigations considered:
A separate `--model` flag that always writes the override and a `--reset-model` flag to clear it would be more explicit but is a larger API change.
Severity calibration:
The bug requires a specific sequence of user actions (clear override, then request the default model by full ID) and only affects the next session's model selection, so it is a likely production bug under plausible conditions but not a data-loss or crash.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Adds GPT 6 Astra to the native Codex harness and Claude Fable 5.1 to Claude Code and the Anthropic API loop. Both models can be selected through
/orchor--model;fableresolves toclaude-fable-5-1./clear, while preserving the native Codex default option.Validation passed:
go test ./...,go vet ./..., release-style build, golangci-lint v1.64.8,go mod verify, andgit diff --check.govulncheckreported no vulnerabilities. Regression tests cover model selection, persistence, Codex initial/resume/clear arguments, and Fable tool continuations. Diff review found no blocking issues; the changed-file secret scan found no candidates.Native CLI authentication and approval/sandbox policies remain in the providers' control; no credential extraction or API client SDK was introduced. Live model inference was not exercised.