fix: preserve exact Codex model across resumes (QUA-1910)

[Desperado]

Summary

• add an exact allowlist and deterministic QualityMax default for Codex models
• pass the selected model to both initial and resumed codex exec commands
• preserve and validate the model in durable continuity checkpoints
• reject unsupported or option-like model values before process start

Verification

• go test ./...
• go vet ./...
• go build ./...

Linear: QUA-1910

[sigilix]

Sigilix Overview

Effort: 4/5 (large)

Quality gates

• ✅ PR title follows convention
• ✅ PR description is complete
• ℹ️ PR is linked to an issue — No Closes #N / Closes SIG-N keyword found in PR body or commit messages.

Summary — latest push

This change fixes QUA-1910 by threading an exact Codex model through initial and resumed codex exec commands, persisting it in durable checkpoints, and rejecting unsupported or option-like model values before process start. It introduces a strict allowlist of supported models and a deterministic default, ensuring resumed threads cannot silently downgrade or switch models. Two security findings (P2/P3) were flagged: the allowlist is case-sensitive, accepting only lowercase, and the --model flag is injected into CLI args without sanitization, risking argument injection if the allowlist is ever bypassed.

Important files

File	Score	Notes	Next step
codexrunner/model.go	5/5	Introduces the exact model allowlist, default constant, and ValidateModel function that gates all model usage.	Add case-insensitive matching or explicitly normalize model strings to lowercase to prevent bypass via different casings.
codexrunner/runner.go	5/5	Threads the validated model into CLI args for both initial and resumed turns and persists it in the checkpoint hook.	Sanitize or escape the model string before interpolating it into the args slice to harden against argument injection if allowlist validation is ever relaxed.
codexrunner/continuity.go	4/5	Preserves the model in the Continuity checkpoint state and validates it on Restore.	Ensure all callers constructing Checkpoints externally validate the Model field, or rely on Restore as the single validation gate.
codexrunner/runner_test.go	4/5	Adds tests verifying exact model passthrough on initial/resumed turns and rejection of unsupported models before process start.	Add a test case for a mixed-case valid model (e.g., 'GPT-5.6-TERRA') to confirm rejection behavior.
codexrunner/continuity_test.go	3/5	Updates existing checkpoint restoration tests to include the Model field and asserts rejection of non-exact models like 'auto'.	Cover the edge case where a checkpoint is restored with an empty Model string to ensure it behaves identically to pre-model behavior.

Confidence: 3/5

The allowlist is case-sensitive and model values are interpolated directly into CLI args without escaping, creating argument injection risk if validation is ever bypassed.

• model.go lines 24-30: The allowlist is case-sensitive; 'GPT-5.6-TERRA' is rejected but 'gpt-5.6-terra' is accepted, which could lead to unexpected auth or routing mismatches downstream.
• runner.go lines 244-249: The validated model string is directly interpolated into the args slice without shell or argument escaping, risking local argument injection if a bypass is introduced.
• runner.go lines 244-249: When both Model and ThreadID are provided, the resume command is correctly constructed, but verify the CLI actually supports the --model flag position before --json.
• continuity.go lines 73-77: Restore validates the model, but an empty Model string is allowed and will silently skip model pinning on resumed turns, diverging from the stated goal of exact model preservation.

Suggested labels: bug security

---

_{Posted · fc92fa2 · 2 findings — View review
Proof: 2 model-only
runner-verified = CI receipt · reproduced = sandbox observed diff · grounded = deterministic detector/worker-token · model-only = model judgment only
Dismiss @sigilix dismiss <reason> (not-a-bug | bad-anchor | already-covered | too-minor | wrong-context) · Re-run /sigilix review · Review #1
Sigilix · 0 of 50 reviews used in past 5h}

[qualitymaxapp]

QualityMax Review — canonical overview updated; inline findings are attached to this review.

[qualitymaxapp]

QualityMax Review

Verdict: COMMENT · Confidence: evidence-backed scan

Files eligible: 6 · Files reviewed: 6 · Files with findings: 0 · Findings: 0 · Inline cards: 0

Priority findings

priority	location	finding
—	—	No blocking findings

Review gates

gate	status
AI diff review	completed · eligible 6, reviewed 6 · LLM · served mistral-large-latest · requested gemini-3.1-flash-lite
SAST	completed · eligible 6, reviewed 6 · hybrid
Overall review evidence	clean
Inline evidence	not needed

Important files

file	risk	note	next step
—	—	No findings	—

Change diagram — Flow

flowchart TD
    A[Turn] -->|Model| B[ValidateModel]
    B -->|Valid| C[Build CLI Args]
    B -->|Invalid| D[ErrInvalidModel]
    C --> E[Run Process]
    E --> F[CheckpointSink]
    F --> G[Checkpoint{ThreadID, Model}]

Loading

Review lifecycle

Use 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. @qmax Q&A is tracked separately.

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 fc92fa2a804601cebac5ae3bf15f6eee07a724d3 · run 2026-08-26T12:02:33+00:00 · model served mistral-large-latest · model requested gemini-3.1-flash-lite · model review substantive — 248 model output tokens · model source repository ai_review_preferences.preferred_model · re-review 1 · proof counts {}}

[Desperado]

Bot-review disposition (evidence-backed): no code change is required for the two requested findings.

• The default gpt-5.6-terra is deliberately fixed by the shared QUA-1910 backend/mobile/worker contract, and is itself a member of the exact allowlist. Dynamically asking a locally installed CLI for a default would let repositories drift and would make the mobile-saved choice non-deterministic. Upstream model changes are handled by a coordinated contract revision.
• Exact model matching is intentionally case-sensitive. Values outside the allowlist, including option-looking strings, fail before process start.
• Arguments are passed directly to exec.Command as an argv slice; no shell parses or expands the model value.
• The validation-order finding states the opposite of the implementation: runner.go validates ThreadID first (lines 232–234), then Model (235–238), so an invalid thread is not masked by an invalid model. Either way both fail before process start.
• Empty Model remains supported only for backwards-compatible standalone qmax-code callers. The QUA-1910 cloud worker and shared accepted-assignment contract require and validate an exact model before invoking this layer.

Native CI, Go tests/vet/build, QualityMax review, Sigilix runner, and Socket security checks are green on this head aside from these review-state findings.

Dismissed as non-defects after verifying the fixed shared contract, exact allowlist, direct argv execution, and actual validation order; see evidence comment on PR.

[Desperado]

QUA-1910 exact-head release evidence for fc92fa2a804601cebac5ae3bf15f6eee07a724d3:

• GitHub build-and-test passed.
• GitHub Run Go Tests passed.
• Socket and Sigilix checks passed.
• Local go test ./..., go vet ./..., and go build ./... passed.
• Independent adversarial review found no blocking continuity defect.
• The external QualityMax Pipeline failure is non-evidentiary for this Go repository because it reported 0/0 tests; native Go CI above executed and passed the actual suite.

This exact commit is intentionally being preserved by merge commit because qmax-cloud-agent pins its Go pseudo-version.