8 issues found across 43 files

Confidence score: 3/5

• There is a concrete user-impacting risk in src/shared/model-settings-compatibility.ts: missing "none" and "minimal" in variant handling can cause valid OpenCode configurations to be dropped at runtime.
• Several medium-severity issues are maintainability/regression risks rather than immediate breakage (duplicated parsing and prompt-parameter extraction in src/shared/fallback-chain-from-models.ts and src/features/background-agent/manager.ts, plus duplicated variant/type allowlists in src/shared/model-resolver.ts and src/tools/delegate-task/tools.ts).
• Test coverage quality regressed in places: src/shared/fallback-chain-from-models.test.ts may pass without exercising the intended sort behavior, and src/shared/tmux/tmux-utils.test.ts no longer validates isInsideTmux(), which lowers confidence in refactor safety.
• Pay close attention to src/shared/model-settings-compatibility.ts, src/plugin/chat-params.ts, and src/shared/fallback-chain-from-models.test.ts - variant compatibility drops valid configs, behavior appears to expand beyond stated scope, and a key test may be giving false confidence.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/shared/model-settings-compatibility.ts">

<violation number="1" location="src/shared/model-settings-compatibility.ts:54">
P1: Custom agent: **Opencode Compatibility**

Add "none" and "minimal" to VARIANT_LADDER and OpenAI variants to prevent valid OpenCode configurations from being dropped.</violation>
</file>

<file name="src/shared/tmux/tmux-utils.test.ts">

<violation number="1" location="src/shared/tmux/tmux-utils.test.ts:51">
P2: The refactored test no longer exercises `isInsideTmux()` and therefore no longer validates the behavior described by the test name.</violation>
</file>

<file name="src/shared/fallback-chain-from-models.ts">

<violation number="1" location="src/shared/fallback-chain-from-models.ts:66">
P2: Substantial code duplication in model string parsing logic. The new `parseFallbackModelObjectEntry` function duplicates the same 15-line parsing block from `parseFallbackModelEntry`. Instead, delegate to `parseFallbackModelEntry(obj.model, contextProviderID, defaultProviderID)` and merge additional config fields.</violation>
</file>

<file name="src/shared/model-resolver.ts">

<violation number="1" location="src/shared/model-resolver.ts:96">
P2: New hardcoded variant allowlist duplicates existing variant definitions, creating drift risk between parsing and flattening logic.</violation>
</file>

<file name="src/plugin/chat-params.ts">

<violation number="1" location="src/plugin/chat-params.ts:81">
P2: New behavioral logic contradicts PR description's claim of no behavioral change - adds session prompt params feature while PR claims to only deduplicate DelegatedModelConfig types</violation>
</file>

<file name="src/shared/fallback-chain-from-models.test.ts">

<violation number="1" location="src/shared/fallback-chain-from-models.test.ts:368">
P2: Flawed test logic: test trivially passes because the competing entry is filtered out by provider mismatch before sorting occurs.</violation>
</file>

<file name="src/tools/delegate-task/tools.ts">

<violation number="1" location="src/tools/delegate-task/tools.ts:181">
P2: Inline type definition duplicates DelegatedModelConfig, defeating the PR's deduplication goal. Use the canonical type instead.</violation>
</file>

<file name="src/features/background-agent/manager.ts">

<violation number="1" location="src/features/background-agent/manager.ts:519">
P2: The prompt parameters extraction logic is duplicated between `startTask` and `resume` methods (~16 identical lines). Extract this into a shared helper function to improve maintainability and prevent inconsistencies.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

1 issue found across 43 files

Confidence score: 4/5

• This PR looks safe to merge: the noted issue in src/features/background-agent/spawner.ts is primarily a maintainability concern rather than a clear functional bug.
• The duplicate prompt-parameter mapping in startTask and resumeTask could drift over time if new model parameters are added, which creates mild future regression risk.
• Given the low severity and very low confidence signal, this appears non-blocking and suitable for follow-up cleanup rather than a merge stopper.
• Pay close attention to src/features/background-agent/spawner.ts - shared prompt-parameter logic should be centralized to prevent divergence.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/background-agent/spawner.ts">

<violation number="1" location="src/features/background-agent/spawner.ts:139">
P2: Duplicate prompt-parameter mapping logic was added to both startTask and resumeTask. Extract the shared block into a helper to avoid divergence when new model parameters are added.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

7 issues found across 46 files

Confidence score: 3/5

• There is concrete user-impact risk in model compatibility handling: gpt-legacy in src/shared/model-settings-compatibility.ts may incorrectly preserve reasoningEffort for GPT-4/3.5 paths, which can lead to OpenAI request validation errors.
• High-confidence compatibility findings in src/shared/model-settings-compatibility.test.ts suggest variant support is currently mis-modeled (e.g., unsupported variants for Kimi/GLM/Minimax/DeepSeek/Mistral and missing Claude Sonnet 4.6 "max" handling), which could cause invalid parameter combinations at runtime.
• A few medium-severity items increase uncertainty rather than proving breakage (e.g., untyped message.variant flowing into .toLowerCase() in src/plugin/chat-params.ts, and Anthropic Opus effort="max" scope in src/hooks/anthropic-effort/hook.ts), so this looks fixable but not risk-free.
• Pay close attention to src/shared/model-settings-compatibility.ts, src/shared/model-settings-compatibility.test.ts, src/plugin/chat-params.ts, src/hooks/anthropic-effort/hook.ts - compatibility and input-shape mismatches may trigger API/runtime errors.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/shared/model-settings-compatibility.test.ts">

<violation number="1" location="src/shared/model-settings-compatibility.test.ts:61">
P1: Custom agent: **Opencode Compatibility**

Claude Sonnet 4.6 supports the "max" variant in the OpenCode SDK.</violation>

<violation number="2" location="src/shared/model-settings-compatibility.test.ts:259">
P1: Custom agent: **Opencode Compatibility**

Kimi, GLM, Minimax, DeepSeek, and Mistral models do not support variants in the OpenCode SDK.</violation>
</file>

<file name="src/shared/model-settings-compatibility.ts">

<violation number="1" location="src/shared/model-settings-compatibility.ts:56">
P2: gpt-legacy matches all standard GPT models and declares reasoningEffort support, so the compatibility resolver will keep reasoningEffort for gpt‑4/3.5 models even though the OpenAI API only accepts reasoning_effort for reasoning models. This can cause 400 errors when switching from an o‑series model.</violation>
</file>

<file name="src/shared/session-prompt-params-state.ts">

<violation number="1" location="src/shared/session-prompt-params-state.ts:13">
P2: `options` is only shallow-cloned, so nested objects/arrays can still mutate shared session state by reference.</violation>
</file>

<file name="src/plugin/chat-params.ts">

<violation number="1" location="src/plugin/chat-params.ts:159">
P2: `message.variant` is no longer sanitized and can be non‑string, but it’s passed into `resolveCompatibleModelSettings`, which calls `.toLowerCase()` without a type check, causing a runtime error for malformed input.</violation>
</file>

<file name="src/features/background-agent/spawner.ts">

<violation number="1" location="src/features/background-agent/spawner.ts:139">
P2: Newly added prompt-parameter mapping logic is duplicated in both `startTask` and `resumeTask`, increasing maintenance risk and divergence potential.</violation>
</file>

<file name="src/hooks/anthropic-effort/hook.ts">

<violation number="1" location="src/hooks/anthropic-effort/hook.ts:3">
P2: Expanded Opus regex applies `effort="max"` to non‑4.6 Opus models, but Anthropic docs state `max` is only valid for Opus 4.6; other models return errors.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

Re: cubic-dev-ai review — 7 violations. All invalid.

Violation 1 (P1): "Claude Sonnet 4.6 supports the max variant"

Wrong. Claude Sonnet 4.6 does NOT support max in the Anthropic API — only Opus does. The claude-non-opus family correctly declares ["low", "medium", "high"] without max. The test at line 61 correctly asserts downgrade from max to high.

Violation 2 (P1): "Kimi, GLM, Minimax, DeepSeek, and Mistral do not support variants"

Wrong. These models support variants via the Vercel AI SDK's experimental_providerMetadata — this is not an Anthropic-only feature. The registry correctly declares ["low", "medium", "high"] for these families based on the SDK's capabilities.

Violation 3 (P2): "gpt-legacy declares reasoningEffort support"

Wrong. Looking at line 56: ["gpt-legacy", { includes: ["gpt"], variants: ["low", "medium", "high"] }] — there is no reasoningEffort field. When a family omits reasoningEffort, the resolver correctly drops it (line 134: "known family but field not in registry"). The bot hallucinated that gpt-legacy has reasoningEffort.

Violation 4 (P2): "shallow-cloned options can mutate shared state"

The options value in session-prompt-params-state.ts is a Record<string, unknown> containing only primitive values (reasoningEffort: string, maxTokens: number, thinking: object). The shallow spread at line 13 ({ ...params.options }) is sufficient because:

1. The values written are primitives or simple objects
2. getSessionPromptParams also returns a spread copy (line 24)
3. No code path mutates nested properties — they're always replaced wholesale

Violation 5 (P2): "message.variant can be non-string causing .toLowerCase() crash"

Already handled. Line 159: typeof normalizedInput.message.variant === "string" ? normalizedInput.message.variant : undefined — the type guard ensures only strings reach resolveCompatibleModelSettings. The bot didn't read the actual code.

Violation 6 (P2): "Duplicated prompt-parameter mapping in startTask/resumeTask"

Already addressed — identical to the violation on PR #2622. The shared applyModelPromptParams helper (lines 21-42 of spawner.ts) centralizes this logic.

Violation 7 (P2): "Opus regex applies effort=max to non-4.6 Opus models"

The hook at line 39 checks message.variant !== "max" first — it only fires when variant is explicitly max. The only way variant reaches max for Opus is through the compatibility resolver, which only allows max for the claude-opus family. Non-4.6 Opus models that don't support max would need the user to explicitly configure it, which is the intended behavior (user override). Additionally, the confidence score on this one was 0.56 — the bot itself wasn't confident.

0 issues found across 6 files (changes from recent commits).

_{Requires human review: The PR contains significant logic changes including new features (model compatibility resolver, parameter promotion) and bug fixes beyond the stated type refactor, making regressions possible.}

0 issues found across 1 file (changes from recent commits).

_{Requires human review: PR is large (+2.8k lines) and introduces complex session-state persistence and async provider metadata fetching in chat.params, making it impossible to guarantee 0% regression risk.}

0 issues found across 7 files (changes from recent commits).

_{Requires human review: Large PR introducing behavioral changes (setting persistence) and async lookups in the chat.params hot path; cannot be 100% sure of no regressions.}