fix: make spawn model and effort handling honest #351
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,13 @@ | ||
| import type { CliType } from "./agent-types.js"; | ||
|
|
||
| export const MODEL_OVERRIDE_ENV = "REPOGOLEM_ALLOW_MODEL"; | ||
| export const CODEX_EFFORT_VALUES = [ | ||
| "medium", | ||
| "high", | ||
| "xhigh", | ||
| "ultra", | ||
| ] as const; | ||
| export type CodexEffort = (typeof CODEX_EFFORT_VALUES)[number]; | ||
|
|
||
| export interface CliModelPolicyContract { | ||
| defaultModel: string; | ||
|
|
@@ -129,6 +136,47 @@ function ownModelAlias(cli: CliType, normalized: string): string | null { | |
| return typeof alias === "string" && alias ? alias : null; | ||
| } | ||
|
|
||
| function modelAliasUsesUngatedLauncherPath( | ||
| cli: CliType, | ||
| alias: string, | ||
| ): boolean { | ||
| if (cli === "claude") return alias === "sonnet"; | ||
| return cli === "gemini" || cli === "kiro"; | ||
| } | ||
|
|
||
| function acceptedModelNames( | ||
| cli: CliType, | ||
| allowModelOverride: boolean, | ||
| ): string[] { | ||
| const contract = MODEL_POLICY_CONTRACT.cli[cli]; | ||
| const aliases = Object.keys(contract.modelAliases).filter( | ||
| (alias) => | ||
| allowModelOverride || modelAliasUsesUngatedLauncherPath(cli, alias), | ||
| ); | ||
| return [...new Set([contract.defaultModel, ...aliases])]; | ||
| } | ||
|
|
||
| export function resolveSpawnEffort( | ||
| cli: CliType, | ||
| effort?: string, | ||
| ): CodexEffort | null { | ||
| const requested = effort?.trim(); | ||
| if (!requested) return null; | ||
|
|
||
| if (!(CODEX_EFFORT_VALUES as readonly string[]).includes(requested)) { | ||
| throw new Error( | ||
| `Invalid Codex effort "${requested}". Accepted values: ${CODEX_EFFORT_VALUES.join(", ")}. No agent was spawned.`, | ||
| ); | ||
| } | ||
| if (cli !== "codex") { | ||
| throw new Error( | ||
| `Codex effort "${requested}" cannot be used with cli "${cli}". Set cli to "codex" or omit effort. No agent was spawned.`, | ||
| ); | ||
| } | ||
|
|
||
| return requested as CodexEffort; | ||
| } | ||
|
|
||
| export function resolveModelAlias(cli: CliType, model: string): string { | ||
| const trimmed = model.trim(); | ||
| const normalized = normalizeModelKey(trimmed); | ||
|
|
@@ -154,6 +202,13 @@ export function resolveLaunchModelFlag( | |
| } | ||
|
|
||
| const alias = ownModelAlias(cli, normalizeModelKey(requested)); | ||
| if ( | ||
| cli === "claude" && | ||
| alias !== "sonnet" && | ||
| !opts?.allowModelOverride | ||
| ) { | ||
| return null; | ||
| } | ||
| return alias ?? null; | ||
| } | ||
|
|
||
|
|
@@ -170,6 +225,22 @@ export function resolveSpawnModelPolicy( | |
| const requestedOrDefault = requestedWasOmitted ? defaultModel : requestedModel; | ||
| const resolvedRequested = resolveModelAlias(cli, requestedOrDefault); | ||
|
|
||
| // Cursor deliberately accepts arbitrary model strings behind its escape | ||
| // hatch. Every launcher-backed CLI, however, has a finite alias table. Do | ||
| // not let the older Codex coercion branch turn an unknown alias into an | ||
| // apparently successful default-model spawn. | ||
| if ( | ||
| !requestedWasOmitted && | ||
| cli !== "cursor" && | ||
| !modelMatchesDefault(cli, requestedModel) && | ||
| ownModelAlias(cli, normalizeModelKey(requestedModel)) === null | ||
| ) { | ||
| const acceptedModels = acceptedModelNames(cli, overrideAllowed); | ||
| throw new Error( | ||
| `Unsupported model "${requestedModel}" for cli "${cli}": without a valid alias, the launcher would actually run "${defaultModel}". Accepted models: ${acceptedModels.join(", ")}. No agent was spawned.`, | ||
| ); | ||
| } | ||
|
|
||
| if ( | ||
| !requestedWasOmitted && | ||
| !contract.allowModelOverrideByDefault && | ||
|
|
@@ -195,14 +266,27 @@ export function resolveSpawnModelPolicy( | |
| }; | ||
| } | ||
|
|
||
| const launcherModel = requestedWasOmitted | ||
| ? null | ||
| : resolveLaunchModelFlag(cli, resolvedRequested, { | ||
| allowModelOverride: overrideAllowed, | ||
| }); | ||
| if ( | ||
| !requestedWasOmitted && | ||
| !modelMatchesDefault(cli, resolvedRequested) && | ||
| launcherModel === null | ||
| ) { | ||
| const acceptedModels = acceptedModelNames(cli, overrideAllowed); | ||
| throw new Error( | ||
| `Unsupported model "${requestedModel}" for cli "${cli}": without a valid alias, the launcher would actually run "${defaultModel}". Accepted models: ${acceptedModels.join(", ")}. No agent was spawned.`, | ||
| ); | ||
|
Comment on lines
+280
to
+282
There was a problem hiding this comment.
When Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Fixed in 5c38c8f. |
||
| } | ||
|
|
||
| return { | ||
| cli, | ||
| requested_model: requestedModel, | ||
| effective_model: resolvedRequested, | ||
| launcher_model: launcherModel, | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| coerced: false, | ||
| warnings: [], | ||
| override_env: MODEL_OVERRIDE_ENV, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opusspawn aliasWhen
REPOGOLEM_ALLOW_MODELis unset, callers that explicitly request Claudemodel: "opus"now reach this branch and fail, even thoughopuswas accepted before this commit and the deployed Pages demo still presents successfulspawn_agent(..., model="opus")calls inlanding/index.html:1074andlanding/index.html:1538. This is especially disruptive for the legacy aggregate tools whose nested model remains required; normalizeopusto the default Claude launch with no model flag rather than rejecting an established tool input.AGENTS.md reference: AGENTS.md:L44-L47
Useful? React with 👍 / 👎.