diff --git a/.changeset/lively-birds-frolic.md b/.changeset/lively-birds-frolic.md new file mode 100644 index 000000000..f1e60d3a1 --- /dev/null +++ b/.changeset/lively-birds-frolic.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3276 +--- +**Codex agents now inherit the session model instead of getting a pinned per-tier model** — if you install for `codex` with a `runtime` set and any `model_profile` other than `inherit`, GSD no longer writes a `model` (or `model_reasoning_effort`) line into `~/.codex/agents/.toml`. This fixes typed agents failing to spawn with `400 invalid_request_error: "The 'sonnet' model is not supported when using Codex with a ChatGPT account"`, which degraded the whole plan/execute flow to a generic-agent fallback. **To keep pinning a model, set an explicit real-Codex id in `model_overrides`** (e.g. `{"model_overrides": {"gsd-planner": "gpt-5.6-sol"}}`) — that path is unchanged. The installer prints a one-time notice when it drops a pin. Codex-only; all other runtimes are untouched. (#3241) diff --git a/CONTEXT.md b/CONTEXT.md index 61c45d71a..a2609b062 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -192,8 +192,11 @@ Module owning agent-presence resolution and verification, extracted from the Cor ### Config Loader Module Module owning project configuration loading: reads `.planning/config.json`, merges built-in defaults (`CONFIG_DEFAULTS`/`CANONICAL_CONFIG_DEFAULTS`), normalizes legacy keys, applies the active-workstream overlay, validates against the config schema, and warns on unknown keys/profile overrides. Primary interface: `loadConfigResolved(cwd, options) → ConfigResolution { config, source, degraded }` (provenance-aware, ADR-1411 P2 / #1415) — `source` ∈ `'workstream' | 'root' | 'builtin-defaults' | 'global-defaults'`; `degraded:true` when a workstream was requested but its config.json was absent (fell back to root config). `loadConfig(cwd, options) → Record` is the back-compat thin wrapper over `loadConfigResolved` (byte-identical result). Resolution is **caller-anchored, not loader-anchored**: `loadConfigResolved` resolves `cwd` as-is (no walk-up), so `loadConfig` stays byte-identical for its callers; callers that need cwd-drift tolerance (e.g. `cmdAgentSkills`) anchor to the project root via `findProjectRoot` (Project-Root Resolution Module) *before* calling `loadConfigResolved`. Helper exports: `_deepMergeConfig`, `isGitIgnored`, `_warnUnknownProfileOverrides`. Depends only on leaf modules (`configuration`, `config-schema`, `planning-workspace`, `shell-command-projection`, `core-utils`, `model-catalog`) — no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2e (#885) as the prerequisite for the model-resolver extraction (the resolvers call `loadConfig`); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/config-loader.cjs` (generated from `src/config-loader.cts`). +### Model Catalog Module +Leaf module owning the **static** model tables and the closed vocabularies derived from them — the tier/runtime/provider enums (`VALID_TIERS`, `VALID_AGENT_TIERS`, `KNOWN_RUNTIMES`, `KNOWN_PROVIDERS`, `RUNTIMES_WITH_REASONING_EFFORT`, `RUNTIMES_WITH_FAST_MODE`, `ADAPTIVE_TIER_VALUES`), the alias and profile maps (`MODEL_ALIAS_MAP`, `RUNTIME_PROFILE_MAP`, `PROVIDER_PRESETS`), effort rendering (`renderEffortForRuntime`), and the agent→model projections (`getAgentToModelMapForProfile`, `formatAgentToModelMapAsTable`). A **genuine leaf**: it imports `node:path` and its own `model-catalog.json` and nothing else, which is what makes it the correct home for anything several unrelated surfaces must agree on. Model *ids* live in `model-catalog.json`, never inline — changing one means regenerating goldens (`UPDATE_GOLDEN`). Also owns the **Anthropic-flavored-model rule** (#3241, ADR-2313): `CLAUDE_AGENT_ALIASES` (the frozen four-alias set `opus`/`sonnet`/`haiku`/`fable`) and `isAnthropicFlavoredModel(model)`, which is true for a bare tier alias or for any `claude-*` id in any provider namespacing (`anthropic/claude-*`, `us.anthropic.claude-*`); no OpenAI/Codex model id contains "claude", so the case-insensitive substring test is safe and exhaustive. It was **moved down here from the Model Resolver Module** rather than shared from there, because the Codex posture check (`agent-install-check`, epic #2313 Phase 2) and the Codex `.toml` sync (`commands`, Phase 3) both need the rule and neither may take the `config-loader` dependency `model-resolver` would have brought; `model-resolver` re-exports it for back-compat and a parity test fails if the two ever fork. _Avoid_: "the model list" (ambiguous between the catalog JSON and the derived enums). See Model Resolver Module, ADR-2313, and ADR-0003. + ### Model Resolver Module -Module owning model and effort resolution policy: resolves the model, runtime tier, planning granularity, reasoning effort, and fast-mode for a given agent by reading project config and resolving against the model profiles and catalog (`resolveModelInternal`, `resolveModelPolicy`, `resolveTierEntry`, `resolveModelForTier`, `resolveGranularityInternal`, `resolveEffortInternal`, `resolveFastModeInternal`, `resolveEffortForTier`, `nextEffort`, `assertValidGranularityOverride`). Depends only on leaf modules (`config-loader` for `loadConfig`, `configuration` for defaults, `model-profiles` and `model-catalog` for the static tables) — no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2f (#888) — the final core.cts decomposition step; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/model-resolver.cjs` (generated from `src/model-resolver.cts`). +Module owning model and effort resolution policy: resolves the model, runtime tier, planning granularity, reasoning effort, and fast-mode for a given agent by reading project config and resolving against the model profiles and catalog (`resolveModelInternal`, `resolveModelPolicy`, `resolveTierEntry`, `resolveModelForTier`, `resolveGranularityInternal`, `resolveEffortInternal`, `resolveFastModeInternal`, `resolveEffortForTier`, `nextEffort`, `assertValidGranularityOverride`). Depends only on leaf modules (`config-loader` for `loadConfig`, `configuration` for defaults, `model-profiles` and `model-catalog` for the static tables) — no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2f (#888) — the final core.cts decomposition step; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. **`CLAUDE_AGENT_ALIASES` no longer lives here** — it moved down to the Model Catalog Module (#3241, ADR-2313 Phase 1) so the Agent Install Check and Codex-sync surfaces can consume the alias rule without taking a `config-loader` dependency this module would have dragged with it; it is still **re-exported** from here, so existing importers (`bin/install.js`, `tests/codex-config.test.cjs`) are unaffected and a parity test asserts both modules expose the same set. Source of truth: `gsd-core/bin/lib/model-resolver.cjs` (generated from `src/model-resolver.cts`). ### Package Identity Module [Planned] Single seam owning GSD's published-package coordinates so a repoint/rename is a one-line change instead of a tree-wide sweep. Source of truth is `package.json`; values are *derived*, not re-typed: `packageName` (`.name` → `@opengsd/gsd-core`), `binName` (`Object.keys(.bin)[0]` → `gsd-core`), `repoSlug` (parsed from `.repository.url` → `open-gsd/gsd-core`), plus derived `changelogRawUrl` and `manualInstallCommand({ scope, runtime })`. Generated `.cjs` per ADR-457 (generated-single-source); shipped under `gsd-core/bin/lib/`. Three consumer worlds: **Node** consumers `require()` it at runtime (worker, `check-latest-version.cjs`, `bin/install.js`); the **bash launcher** snippet receives the literal injected by `scripts/sync-runtime-launcher.cjs` at sync time; **prose/help** literals (`update.md`, installer help) carry a committed copy. A drift-guard lint (`scripts/lint-package-identity-drift.cjs`, sibling to `check:alias-drift`) fails CI on any raw package/repo literal outside `package.json`, the generated module, and the value-checked materialization sites — this is what keeps the seam real (`two adapters`, not one). Replaces the contradictory pair it consolidates: the runtime-broken `require('../package.json').name` in `hooks/gsd-check-update-worker.js` (#378, resolves to `undefined` post-install) and the hardcoded constant in `check-latest-version.cjs` (#2992). _Avoid_: "package name string", "the npm name" (when you mean the seam). See ADR-457 and Installer Module. diff --git a/bin/install.js b/bin/install.js index e0b07e86e..4542de43e 100755 --- a/bin/install.js +++ b/bin/install.js @@ -468,10 +468,10 @@ const _gsdLibDir = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib'); const { MODEL_PROFILES: GSD_MODEL_PROFILES } = require(path.join(_gsdLibDir, 'model-profiles.cjs')); const { RUNTIME_PROFILE_MAP: GSD_RUNTIME_PROFILE_MAP, + isAnthropicFlavoredModel: gsdIsAnthropicFlavoredModel, } = require(path.join(_gsdLibDir, 'model-catalog.cjs')); const { resolveTierEntry: gsdResolveTierEntry, - CLAUDE_AGENT_ALIASES, } = require(path.join(_gsdLibDir, 'model-resolver.cjs')); // #2071 — install-time effort resolution (readGsdEffectiveEffortConfig / @@ -4074,15 +4074,19 @@ purpose: ${toSingleLine(description)} /** * #2310 — True if `model` is an Anthropic-flavored value that must never appear as a * Codex agent `.toml` `model`. Two forms: (a) a bare Claude Agent-tool tier alias - * (opus/sonnet/haiku/fable — the canonical CLAUDE_AGENT_ALIASES, imported from - * src/model-resolver.cts so it can't diverge); (b) any Claude model id in any provider - * namespacing — `claude-*`, `anthropic/claude-*`, `us.anthropic.claude-*` (the forms the - * catalog assigns to opencode/hermes/kilo, reachable on a Codex .toml via the runtime- - * resolver path). No OpenAI/Codex model id contains "claude", so a case-insensitive - * substring test is a safe, exhaustive guard for (b). Codex/ChatGPT rejects all of these. + * (opus/sonnet/haiku/fable — the canonical CLAUDE_AGENT_ALIASES); (b) any Claude model + * id in any provider namespacing — `claude-*`, `anthropic/claude-*`, `us.anthropic.claude-*` + * (the forms the catalog assigns to opencode/hermes/kilo, reachable on a Codex .toml via + * the runtime-resolver path). No OpenAI/Codex model id contains "claude", so a + * case-insensitive substring test is a safe, exhaustive guard for (b). Codex/ChatGPT + * rejects all of these. + * + * #3241 — thin delegation to the shared predicate on src/model-catalog.cts (moved there + * so it can't diverge across Codex-posture surfaces); kept as a local name because it + * reads better at the call sites below. */ function _isAnthropicFlavoredModel(model) { - return typeof model === 'string' && (CLAUDE_AGENT_ALIASES.has(model) || model.toLowerCase().includes('claude')); + return gsdIsAnthropicFlavoredModel(model); } // #2310 — dedupe stderr warnings so repeated agent emits don't spam (mirrors the @@ -4101,6 +4105,42 @@ function _warnCodexModelOverrideDropped(agentName, value) { ); } +// #3241 — one-time per-install deprecation notice: the automatic runtime-resolver +// per-tier Codex model embed was removed (D1/D5, ADR-2313 passive-posture epic). When +// the resolver *would have* supplied a model and nothing else ends up pinned, this +// notice points the user at model_overrides as the explicit-pin replacement. Dedupes +// with a module-level boolean (mirrors _codexModelOverrideDroppedWarned's Set above) +// so a multi-agent install — every Codex agent hits this condition simultaneously — +// emits exactly one line, not one per agent. Reset once per install() call (see +// install()) so the "at most once" window is per-install, not per-process. That +// reset lives ONLY inside install() (~:10116) — generateCodexAgentToml is also +// exported standalone (~:13460), and a caller invoking it directly/repeatedly +// outside install() gets process-lifetime dedupe instead of per-install. No +// current test depends on the standalone caller's dedupe window. +let _codexResolverModelOmittedWarned = false; +function _warnCodexResolverModelOmitted() { + if (_codexResolverModelOmittedWarned) return; + _codexResolverModelOmittedWarned = true; + process.stderr.write( + 'gsd: notice — Codex agents no longer auto-pin a per-tier model from the runtime ' + + 'resolver; set model_overrides for an agent if you want a specific Codex model ' + + 'instead of the session model.\n', + ); +} + +// Test seam only — bin/install.js deliberately keeps per-install warning/notice +// dedupe in module scope (both the _codexModelOverrideDroppedWarned Set above and +// the _codexResolverModelOmittedWarned boolean; install() resets the latter at +// ~:10116). A unit test that drives generateCodexAgentToml() directly, without +// going through install(), has no other way to reset either store between +// assertions without busting the require.cache (which breaks module-instance +// sharing with the rest of the suite). This is the single sanctioned way for a +// unit test to clear both dedupe stores — exported so tests can call it instead. +function _resetCodexWarningDedupeForTests() { + _codexModelOverrideDroppedWarned.clear(); + _codexResolverModelOmittedWarned = false; +} + /** * Generate a per-agent .toml config file for Codex. * Sets required agent metadata, sandbox_mode, and developer_instructions @@ -4133,38 +4173,58 @@ function generateCodexAgentToml(agentName, agentContent, modelOverrides = null, // Embed model override when configured in ~/.gsd/defaults.json so that // model_overrides is respected on Codex (which uses static TOML, not inline // Task() model parameters). See #2256. - // Precedence: per-agent model_overrides > runtime-aware tier resolution (#2517). // #2310 — a Codex .toml `model` MUST be a real Codex/OpenAI model id. Codex is a // passive/session-only model host (ADR-1239): GSD cannot reliably route per-agent // tiers, and a bare GSD/Claude tier alias (opus/sonnet/haiku/fable) or a claude-* // id 400s on a ChatGPT-account Codex ("The 'sonnet' model is not supported when // using Codex with a ChatGPT account"). So: embed ONLY an explicit real-Codex // model pin from model_overrides; omit anything Anthropic-flavored so the agent - // inherits the always-available session model. (Removing the runtime-resolver - // per-tier embedding below is the ADR-2310 passive-posture epic.) + // inherits the always-available session model. + // #3241 (D1/D5) — the runtime-aware tier-resolver auto-embed that used to fall + // through here when model_overrides had nothing was removed: Codex is passive by + // default now, and only an explicit model_overrides pin survives. See the + // deprecation-notice block below for the population that used to get a pin from + // the resolver and no longer does. const rawModelOverride = modelOverrides?.[resolvedName] || modelOverrides?.[agentName]; let pinnedModel = null; if (rawModelOverride) { - if (typeof rawModelOverride === 'string' && rawModelOverride && !_isAnthropicFlavoredModel(rawModelOverride)) { - pinnedModel = rawModelOverride; // explicit real-Codex model pin → embed verbatim (#2256) + // Trim before the truthiness test (#3241 defect fix): a whitespace-only value + // (e.g. ' ') is a truthy JS string but not a model id — it must not be + // embedded verbatim (`model = " "`, a live pre-fix defect) or routed to + // _warnCodexModelOverrideDropped, whose "is not a valid Codex model + // (Anthropic alias/id)" text would misdescribe a blank config field. It is + // silently dropped, matching how '' already behaves (no pin, no warning). + const trimmedOverride = typeof rawModelOverride === 'string' ? rawModelOverride.trim() : rawModelOverride; + if (typeof trimmedOverride === 'string' && trimmedOverride && !_isAnthropicFlavoredModel(trimmedOverride)) { + pinnedModel = trimmedOverride; // explicit real-Codex model pin → embed verbatim (#2256) + } else if (typeof rawModelOverride === 'string' && trimmedOverride === '') { + // whitespace-only override — no pin, no warning (#3241). } else { _warnCodexModelOverrideDropped(resolvedName, rawModelOverride); // alias/claude-* → omit } } - if (!pinnedModel && runtimeResolver) { - // #2517 — runtime-aware tier resolution. Embeds Codex-native model + reasoning_effort - // from RUNTIME_PROFILE_MAP / model_profile_overrides for the configured tier. - // (Superseded on the default path by the ADR-2310 passive-posture epic.) - const entry = runtimeResolver.resolve(resolvedName) || runtimeResolver.resolve(agentName); - if (entry?.model) pinnedModel = entry.model; - } // #2310 — final safety gate: never emit an Anthropic-flavored model into a Codex - // .toml, even from the runtime-resolver path (e.g. a defaults.json runtime that - // does not match the codex install target). + // .toml, even one that reached here through some other path than the override + // check above. if (pinnedModel && _isAnthropicFlavoredModel(pinnedModel)) { _warnCodexModelOverrideDropped(resolvedName, pinnedModel); pinnedModel = null; } + // #3241 — one-time deprecation notice: if nothing ends up pinned but the + // runtime resolver would have supplied a per-tier model that would actually + // have been EMBEDDED (the population that loses a pin now that the + // auto-embed above is gone), point the user at model_overrides. The would-be + // model must also clear the #2310 Anthropic-flavored gate above — if it + // wouldn't have survived that gate, the user never had that pin pre-Phase-1 + // either, and the notice would be false. Never fires when the resolver is + // null, resolves to nothing, resolves to an Anthropic-flavored model, or an + // explicit real-Codex pin survived — in all of those cases nothing was lost. + if (!pinnedModel && runtimeResolver) { + const wouldHavePinned = runtimeResolver.resolve(resolvedName) || runtimeResolver.resolve(agentName); + if (wouldHavePinned?.model && !_isAnthropicFlavoredModel(wouldHavePinned.model)) { + _warnCodexResolverModelOmitted(); + } + } let hasPinnedModel = false; if (pinnedModel) { lines.push(`model = ${JSON.stringify(pinnedModel)}`); @@ -10069,6 +10129,12 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { const dirName = getDirName(runtime); const src = path.join(__dirname, '..'); + // #3241 — the Codex resolver-model-omitted notice dedupes "at most once", but + // scoped per install() call rather than per process — each install() run gets + // its own fresh window so a second install (e.g. a second runtime, or a test + // re-running install()) can warn again if the same condition recurs. + _codexResolverModelOmittedWarned = false; + if (_hostBehaviors(runtime).localInstallDeferred && !isGlobal) { console.log(` ${yellow}⚠${reset} Kimi local install is deferred for Phase 2.`); console.log(` No .kimi-code/skills or .agents/skills project artifacts were written.`); @@ -13458,6 +13524,7 @@ module.exports = { convertClaudeAgentToCursorAgent, convertClaudeAgentToCodexAgent, generateCodexAgentToml, + _resetCodexWarningDedupeForTests, cleanupCodexSkillMetadataSidecars, cleanupWindsurfLegacyDevinSkills, cleanupMovedSkillsOldLocation, diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index b4d652d63..e9b37fd3c 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -169,7 +169,7 @@ project one is reported, since that is the file you are most likely able to fix. | `mode` | enum | `interactive`, `yolo` | `interactive` | `yolo` auto-approves decisions; `interactive` confirms at each step | | `granularity` | enum | `coarse`, `standard`, `fine` | `standard` | Controls phase count: `coarse` (2-4), `standard` (4-6), `fine` (6-10) | | `model_profile` | enum | `quality`, `balanced`, `budget`, `adaptive`, `inherit` | `balanced` | Model tier for each agent (see [Model Profiles](#model-profiles)). `adaptive` was added per [#1713](https://github.com/open-gsd/gsd-core/issues/1713) / [#1806](https://github.com/open-gsd/gsd-core/issues/1806) and resolves the same way as the other tiers under runtime-aware profiles. | -| `runtime` | string | `claude`, `codex`, or any string | (none) | Active runtime for [runtime-aware profile resolution](#runtime-aware-profiles-2517). When set, profile tiers (opus/sonnet/haiku) resolve to runtime-native model IDs. The resolved ID is embedded into each agent's static frontmatter at install time on `codex` and `opencode` (whose `task` / `spawn_agent` interfaces do not accept an inline `model` parameter, so editing `model_overrides` requires re-running `gsd install ` to take effect — see [Per-Agent Overrides](#per-agent-overrides)); other runtimes consume the resolver at spawn time. When unset (default), behavior is unchanged from prior versions. Added in v1.39 | +| `runtime` | string | `claude`, `codex`, or any string | (none) | Active runtime for [runtime-aware profile resolution](#runtime-aware-profiles-2517). When set, profile tiers (opus/sonnet/haiku) resolve to runtime-native model IDs. The resolved ID is embedded into each agent's static frontmatter at install time on `opencode` (whose `spawn_agent` interface does not accept an inline `model` parameter, so editing `model_overrides` requires re-running `gsd install ` to take effect — see [Per-Agent Overrides](#per-agent-overrides)); other runtimes consume the resolver at spawn time. **`codex` is the exception: it embeds no per-tier model at all.** Codex is a passive / session-only model host ([ADR-2313](adr/2313-codex-passive-model-posture.md)) — a ChatGPT-account session exposes only its own model, so a pinned tier model returns `400 invalid_request_error` and the agent fails to spawn. Codex agents therefore inherit the session model, and only an explicit real-Codex id in `model_overrides` (e.g. `"gpt-5.6-sol"`) is written into the `.toml`. When unset (default), behavior is unchanged from prior versions. Added in v1.39; Codex behavior changed in v1.11 | | `model_profile_overrides..` | string \| object | per-runtime tier override | (none) | Override the runtime-aware tier mapping for a specific `(runtime, tier)`. Tier is one of `opus`, `sonnet`, `haiku`. Value is either a model ID string (e.g. `"gpt-5-pro"`) or `{ model, reasoning_effort }`. See [Runtime-Aware Profiles](#runtime-aware-profiles-2517). Added in v1.39 | | `model_policy.provider` | string | `openai`, `anthropic`, `anthropic-fable`, `google`, `qwen`, `generic` | (none) | Declares the model provider. Known providers (`openai`, `anthropic`, `anthropic-fable`, `google`, `qwen`) unlock catalog-backed presets. `generic` treats all model IDs as opaque strings — no prefix inference, no reasoning-effort defaults. `model_policy.runtime_tiers` resolves before legacy `model_profile_overrides`. See [Model Policy Presets](#model-policy-presets-model_policy--added-in-v142). Added in v1.42 ([#49](https://github.com/open-gsd/gsd-core/issues/49)) | | `model_policy.budget` | enum | `high`, `medium`, `low` | (none) | Selects a budget tier when using a known provider. GSD materializes the matching catalog preset into explicit tier mappings at resolve time. Ignored when `provider` is `generic` or `custom`. Added in v1.42 ([#49](https://github.com/open-gsd/gsd-core/issues/49)) | diff --git a/docs/adr/2313-codex-passive-model-posture.md b/docs/adr/2313-codex-passive-model-posture.md index bf8f273c9..0e56fdd27 100644 --- a/docs/adr/2313-codex-passive-model-posture.md +++ b/docs/adr/2313-codex-passive-model-posture.md @@ -279,3 +279,57 @@ to assemble it from Consequences and Scope boundary: ADR does not generalize the posture; extending it to another host would be its own decision. - **The posture is not real until Phase 1 merges.** `Accepted` locks the contract, not the tree — until #3241 lands, `generateCodexAgentToml` still embeds a per-tier model. + +## Amendment (2026-08-09): a deprecation notice IS offered (#3241) + +Recorded as a dated section rather than by editing the Migration section or the Known limits bullet +above, since ADRs here are append-only. Both now read as superseded on this one point; the rest of +each stands. + +**What changed.** This ADR's Migration section states *"No deprecation window is offered. The default +flips in a single release rather than warning first,"* and Known limits repeats it. Phase 1 (#3241) +ships a deprecation notice instead, by maintainer direction taken after this ADR merged. + +**Why the original position was wrong, precisely.** The argument for flipping silently was that the +behavior being removed *"is one most affected users could never successfully use"* — it 400s on a +ChatGPT-account Codex. That is true of the ChatGPT population and false of the API-key population, +which is exactly the group the Migration section already identifies as *losing something real*. The +ADR named a class of user harmed by the change and then declined to warn them, in the same document. +Hyrum's Law's own guidance — break a long-lived observable behavior when you must, but give a +migration path — was applied to the *recourse* (`model_overrides` stays) and not to the *notice*. + +**The notice.** One line to stderr per install, emitted only for the population that actually loses a +pin: the runtime resolver would have supplied a model, and nothing ends up pinned. It names +`model_overrides` as the recovery mechanism and the session model as what the agent gets instead. + +It deliberately names **no agent and no model**. The condition is per-install, not per-agent — every +agent hits it simultaneously — so per-agent detail would imply a per-agent decision that was not made, +and ~20 identical lines would train the reader to ignore them. It carries no interpolated +user-controlled value, which is why it needs none of the length-capping the adjacent +`_warnCodexModelOverrideDropped` applies. + +It does **not** fire when the resolver is null (`inherit`, or no configured `runtime`), when the +resolver resolves to nothing, or when an explicit real-Codex pin survives. In each of those cases +nothing was lost, and a notice would be noise that costs the signal its meaning. + +**Known limit this does not remove.** The notice fires at *install* time. A user who never +re-installs never sees it — Phase 2's health-check and Phase 3's sync are what reach them. The +Known-limits bullet above is therefore softened, not deleted: there is now a warning, but it is not +a full deprecation *release*, and no separate release ships before the flip. + +## Amendment (2026-08-09): whitespace-only `model_overrides` was a live defect (#3241) + +Surfaced while writing Phase 1's failing-first suite, and fixed there rather than filed. + +`model_overrides[] = " "` is **truthy**, survives the `typeof === 'string'` guard, is not +Anthropic-flavored, and was therefore embedded verbatim as `model = " "`. That is the same class +the #2310 guard exists to stop — a value that is not a real Codex model id reaching the `.toml` and +400-ing the agent — reached by a different route. + +Phase 1 trims before the truthiness test, so a whitespace-only override yields no pin. It is +deliberately **not** routed to `_warnCodexModelOverrideDropped`: that message says the value *"is not +a valid Codex model (Anthropic alias/id)"*, which misdescribes an empty config field. A blank value +is silently no-pin, matching how `""` already behaved. + +This ADR's D2 ("embed a `model` only for an explicit real-Codex pin") always implied this. The +implementation simply did not enforce it, and no test covered the case. diff --git a/docs/how-to/configure-model-profiles.md b/docs/how-to/configure-model-profiles.md index 99639d141..58efb169f 100644 --- a/docs/how-to/configure-model-profiles.md +++ b/docs/how-to/configure-model-profiles.md @@ -55,7 +55,7 @@ Valid values: `opus`, `sonnet`, `haiku`, `inherit`, or any fully-qualified model `model_overrides` can be set per-project in `.planning/config.json` or globally in `~/.gsd/defaults.json`. Per-project entries win on conflict; non-conflicting global entries are preserved. -**Important for Codex and OpenCode:** Those runtimes embed the resolved model into each agent's static config at install time. After editing `model_overrides`, re-run the installer for the change to take effect: +**Important for Codex and OpenCode:** Those runtimes embed the model into each agent's static config at install time rather than choosing it per spawn, so after editing `model_overrides` you must re-run the installer for the change to take effect: ```bash npx @opengsd/gsd-core@latest --codex --global # or --opencode, --kilo, etc. @@ -187,16 +187,46 @@ quota / rate-limit failures; other failures keep the tier ladder. Leaving If you installed GSD for Codex, OpenCode, Antigravity CLI, or Kilo, the installer already set `resolve_model_ids: "omit"` in your config. This tells GSD to skip Anthropic model ID resolution and let the runtime choose its own default model. No manual setup is needed for the basic case. -**If you want tiered models on Codex:** +### Codex does not do tier routing — pin explicitly instead + +**Codex agents inherit whatever model your Codex session is using.** GSD writes no `model` line into +`~/.codex/agents/.toml`, so setting `model_profile` has no effect on Codex. + +This is deliberate ([ADR-2313](../adr/2313-codex-passive-model-posture.md)). A ChatGPT-account Codex +session exposes only its own model, so a pinned tier model fails the request outright — +`400 invalid_request_error: "The 'sonnet' model is not supported when using Codex with a ChatGPT +account"` — and the agent never spawns. + +**To pin a model on Codex, name a real Codex model id per agent:** ```json { "runtime": "codex", - "model_profile": "balanced" + "model_overrides": { + "gsd-planner": "gpt-5.6-sol", + "gsd-executor": "gpt-5.6-terra" + } } ``` -GSD resolves each tier alias to the Codex-native model and reasoning effort defined in the runtime tier map. +Then re-run the installer, as with any `model_overrides` edit on Codex (see above). + +Two rules apply to what you can put there: + +- **It must be a real Codex model id.** A GSD tier alias (`opus`, `sonnet`, `haiku`, `fable`) or a + `claude-*` id is dropped with a warning rather than written, because Codex rejects them. +- **Your account must actually expose it.** GSD cannot check this — if you pin `gpt-5.6-sol` on an + account that does not have it, you get the same 400. When in doubt, omit the pin and let the + session model apply. + +`model_reasoning_effort` follows the model: with no pin, GSD writes no effort line either, so the +Codex UI drives both rather than one following GSD and the other following your session. + +> **Upgrading from v1.10 or earlier?** Codex installs used to embed a per-tier model +> (`opus→gpt-5.6-sol`, `sonnet→gpt-5.6-terra`, `haiku→gpt-5.6-luna`). If you were on an API-key +> account where those resolved successfully, add the `model_overrides` block above to keep them. +> The installer prints a one-time notice when it drops a pin. If you were on a ChatGPT account, this +> is the change that stops the 400s — nothing to do. **If you want per-agent model IDs on any non-Claude runtime:** diff --git a/src/model-catalog.cts b/src/model-catalog.cts index 3a2bb473d..bdfb48580 100644 --- a/src/model-catalog.cts +++ b/src/model-catalog.cts @@ -157,6 +157,29 @@ export const KNOWN_PROVIDERS: Set = new Set( .map(([name]) => name) ); +// ─── #3241 — Anthropic-flavored model detection ────────────────────────────── +// +// Moved here from src/model-resolver.cts (the "seam decision" in +// .gsd/phase/feat-3241-codex-omit-model-by-default/40-design.md): this leaf +// module is the one common dependency both model-resolver and the (layering- +// restricted) install-time Codex-posture checks can share without pulling +// model-resolver's config-loader dependency chain into a "pure read/verify" +// caller. model-resolver re-exports both names for back-compat. +// +// #2310 — True if `model` is an Anthropic-flavored value that must never appear as a +// Codex agent `.toml` `model`. Two forms: (a) a bare Claude Agent-tool tier alias +// (opus/sonnet/haiku/fable — CLAUDE_AGENT_ALIASES below); (b) any Claude model id in +// any provider namespacing — `claude-*`, `anthropic/claude-*`, `us.anthropic.claude-*` +// (the forms the catalog assigns to opencode/hermes/kilo, reachable on a Codex .toml +// via the runtime-resolver path). No OpenAI/Codex model id contains "claude", so a +// case-insensitive substring test is a safe, exhaustive guard for (b). Codex/ChatGPT +// rejects all of these. +export const CLAUDE_AGENT_ALIASES: Set = new Set(['opus', 'sonnet', 'haiku', 'fable']); + +export function isAnthropicFlavoredModel(model: unknown): boolean { + return typeof model === 'string' && (CLAUDE_AGENT_ALIASES.has(model) || model.toLowerCase().includes('claude')); +} + export function nextTier(currentTier: string): string | null { const order = ['light', 'standard', 'heavy']; const idx = order.indexOf(String(currentTier)); diff --git a/src/model-resolver.cts b/src/model-resolver.cts index c47e5f526..fb09d99bf 100644 --- a/src/model-resolver.cts +++ b/src/model-resolver.cts @@ -16,7 +16,8 @@ * - ./config-loader.cjs (loadConfig) * - ./configuration.cjs (CONFIG_DEFAULTS as CANONICAL_CONFIG_DEFAULTS) * - ./model-profiles.cjs (MODEL_PROFILES, AGENT_TO_PHASE_TYPE, AGENT_DEFAULT_TIERS, VALID_AGENT_TIERS, nextTier) - * - ./model-catalog.cjs (MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, PROVIDER_PRESETS, VALID_TIERS) + * - ./model-catalog.cjs (MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, PROVIDER_PRESETS, VALID_TIERS, + * CLAUDE_AGENT_ALIASES — re-exported below for back-compat, #3241) */ // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -30,7 +31,7 @@ import { CONFIG_DEFAULTS as CANONICAL_CONFIG_DEFAULTS } from './configuration.cj import modelProfiles = require('./model-profiles.cjs'); const { MODEL_PROFILES, AGENT_TO_PHASE_TYPE, AGENT_DEFAULT_TIERS, VALID_AGENT_TIERS, nextTier } = modelProfiles; -import { MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, PROVIDER_PRESETS, VALID_TIERS } from './model-catalog.cjs'; +import { MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, PROVIDER_PRESETS, VALID_TIERS, CLAUDE_AGENT_ALIASES } from './model-catalog.cjs'; import fs from 'node:fs'; import path from 'node:path'; @@ -179,7 +180,9 @@ const CLAUDE_POLICY_ID_TO_ALIAS: Record = { ), 'claude-fable-5': 'fable', }; -const CLAUDE_AGENT_ALIASES = new Set(['opus', 'sonnet', 'haiku', 'fable']); +// CLAUDE_AGENT_ALIASES moved to ./model-catalog.cts (#3241) — imported above +// and re-exported below for back-compat (bin/install.js:474, +// tests/codex-config.test.cjs:24 depend on the name being on this module). // Dedupe stderr warnings so repeated agent resolutions don't spam (#1133). const _modelPolicyUnmappableWarned = new Set(); diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index a48dfd3b1..f46e46db0 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -23,6 +23,13 @@ const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); const fc = require('fast-check'); const { CLAUDE_AGENT_ALIASES } = require('../gsd-core/bin/lib/model-resolver.cjs'); +// #3241 — the intended new home for CLAUDE_AGENT_ALIASES + isAnthropicFlavoredModel +// (see .gsd/phase/feat-3241-codex-omit-model-by-default/40-design.md "The seam +// decision"). Neither export exists on model-catalog.cjs yet; requiring the +// module does not throw (it just has no such keys today), but calling +// isAnthropicFlavoredModel does — see the new describe block below. +const modelCatalog = require('../gsd-core/bin/lib/model-catalog.cjs'); +const modelResolver = require('../gsd-core/bin/lib/model-resolver.cjs'); // #2153 follow-up: ensure hooks/dist/ exists before any install integration // test runs. The Codex install path copies hook files from hooks/dist/, which @@ -51,6 +58,7 @@ const { convertClaudeAgentToCodexAgent, convertClaudeCommandToCodexSkill, generateCodexAgentToml, + _resetCodexWarningDedupeForTests, cleanupCodexSkillMetadataSidecars, generateCodexConfigBlock, stripGsdFromCodexConfig, @@ -511,13 +519,17 @@ tools: Read, Grep, Glob ); }); - test('emits reasoning effort when runtime resolver pins Codex model (#838)', () => { + test('omits model and reasoning effort when only the runtime resolver would have pinned one (#838, #3241)', () => { + // #3241 flips this test: the runtime-resolver auto-embed block (D1) was + // removed, so a resolver alone (no explicit model_overrides) no longer + // pins a model at install time, and #838's model/effort coupling means + // neither line survives. const runtimeResolver = { resolve: () => ({ model: 'gpt-5.5' }) }; const result = generateCodexAgentToml('gsd-executor', sampleAgent, null, runtimeResolver); - assert.ok(result.includes('model = "gpt-5.5"'), 'runtime resolver must pin model'); + assert.ok(!result.includes('model = "gpt-5.5"'), 'runtime resolver alone must not pin model (#3241)'); assert.ok( - result.includes('model_reasoning_effort ='), - 'reasoning effort is safe to emit when runtime resolver pins model' + !result.includes('model_reasoning_effort ='), + 'reasoning effort must not survive an omitted resolver model (#838 coupling)' ); }); @@ -562,10 +574,13 @@ tools: Read, Grep, Glob assert.ok(!result.includes('model ='), 'unmappable Anthropic override falls through to Codex default (no model pinned)'); }); - test('a dropped claude-* override falls through to the runtime resolver (#2310)', () => { + test('a dropped claude-* override no longer falls through to the runtime resolver (#2310, #3241)', () => { + // #3241 (D1) removed the runtime-resolver fallback embed entirely, so a + // dropped alias/claude-* override now has nothing left to fall through to + // — it is simply omitted, same as the claude id never leaking. const runtimeResolver = { runtime: 'codex', resolve: () => ({ model: 'gpt-5.6-terra' }) }; const result = generateCodexAgentToml('gsd-executor', sampleAgent, { 'gsd-executor': 'claude-opus-4-8' }, runtimeResolver); - assert.ok(result.includes('model = "gpt-5.6-terra"'), 'falls through to the runtime-resolved Codex model'); + assert.ok(!result.includes('model = "gpt-5.6-terra"'), 'resolver fallback no longer fires (#3241 D1)'); assert.ok(!result.includes('claude-'), 'claude id must not leak even with a resolver present'); }); @@ -628,6 +643,212 @@ tools: Read, Grep, Glob }), { numRuns: 400 }); }); + // ─── #3241: omit the Codex per-agent model by default (resolver-only path) ──── + // Phase 1 removes the runtime-resolver auto-embed. These tests drive the + // shipping default shape — runtime set + model_profile:"balanced" (mocked + // here as a resolver object, matching the existing #2517/#838 tests above, + // e.g. L514-522) — with NO model_overrides, and assert the model line (and + // its coupled model_reasoning_effort, #838) are omitted. + + test('omits model and model_reasoning_effort when only the runtime resolver would have supplied one (#3241)', () => { + // RED (pre-fix): today this resolver-only path still embeds the tier + // model (see L514-522's "runtime resolver pins Codex model" test, which + // asserts the opposite of this on purpose and is left untouched per the + // Phase 1 rollout plan). Both assertions below fail against the current + // tree: `model = "gpt-5.6-sol"` and `model_reasoning_effort = "high"` + // are both present in `result` today. + const runtimeResolver = { runtime: 'codex', resolve: () => ({ model: 'gpt-5.6-sol' }) }; + const result = generateCodexAgentToml('gsd-executor', sampleAgent, null, runtimeResolver); + assert.ok(!/^model = /m.test(result), + 'a resolver-only tier model must not be embedded by default (#3241)'); + assert.ok(!result.includes('model_reasoning_effort ='), + 'reasoning effort must not survive an omitted resolver model (#838 coupling)'); + }); + + test('resolver is null (inherit profile or no runtime configured) emits no model and no warning (#3241)', (t) => { + // Regression guard — PASSES today already: readGsdRuntimeProfileResolver + // already returns null for both "no runtime" and model_profile:"inherit" + // (bin/install.js:1632, :1635), and generateCodexAgentToml already omits + // the model when runtimeResolver is null. Nothing in Phase 1 touches this + // branch; this test exists to prove it keeps holding after the fix lands. + const origWrite = process.stderr.write; + const stderrChunks = []; + process.stderr.write = (chunk) => { stderrChunks.push(String(chunk)); return true; }; + t.after(() => { process.stderr.write = origWrite; }); + const result = generateCodexAgentToml('gsd-executor', sampleAgent, null, null); + assert.ok(!/^model = /m.test(result), 'no model when the resolver is null'); + assert.strictEqual(stderrChunks.join(''), '', 'inherit/no-runtime users must never be warned — nothing was lost'); + }); + + test('resolver present but resolve() yields nothing emits no model and no warning (#3241)', (t) => { + // Regression guard — PASSES today already: entry?.model is undefined when + // resolve() returns null, so pinnedModel stays null and no warning branch + // is reachable in current code. Nothing was lost, so nothing should warn, + // before or after the fix (negative-space row in 40-design.md). + const origWrite = process.stderr.write; + const stderrChunks = []; + process.stderr.write = (chunk) => { stderrChunks.push(String(chunk)); return true; }; + t.after(() => { process.stderr.write = origWrite; }); + const runtimeResolver = { runtime: 'codex', resolve: () => null }; + const result = generateCodexAgentToml('gsd-executor', sampleAgent, null, runtimeResolver); + assert.ok(!/^model = /m.test(result), 'no model when resolve() yields nothing'); + assert.strictEqual(stderrChunks.join(''), '', 'a resolver that would not have pinned anything must never warn'); + }); + + test('empty-string and whitespace-only model_overrides are not pins and not warnings (#3241)', (t) => { + // '' — regression guard, PASSES today: '' is falsy, so the pin branch is + // never entered at all (no pin, no warning either old or new). + // + // ' ' (whitespace-only) — RED (pre-fix, live defect, fixed in this + // phase per maintainer direction): a whitespace-only override is a + // truthy JS string, is not Anthropic-flavored per `_isAnthropicFlavoredModel`, + // and is CURRENTLY pinned verbatim (`model = " "`) with no guard — + // the same class of bug the #2310 guard exists to stop (a non-model + // value reaching the .toml and 400-ing the Codex agent). Must be + // silently dropped, matching how '' already behaves — NOT routed through + // `_warnCodexModelOverrideDropped` (that warning's "is not a valid Codex + // model (Anthropic alias/id)" text would misdescribe a blank field), so + // no warning of any kind is expected for it either. + const origWrite = process.stderr.write; + const stderrChunks = []; + process.stderr.write = (chunk) => { stderrChunks.push(String(chunk)); return true; }; + t.after(() => { process.stderr.write = origWrite; }); + const emptyResult = generateCodexAgentToml('gsd-executor', sampleAgent, { 'gsd-executor': '' }); + const whitespaceResult = generateCodexAgentToml('gsd-executor', sampleAgent, { 'gsd-executor': ' ' }); + assert.ok(!/^model = /m.test(emptyResult), 'empty-string override must not be pinned'); + assert.ok(!/^model = /m.test(whitespaceResult), 'whitespace-only override must not be pinned (#3241 fix)'); + assert.strictEqual(stderrChunks.join(''), '', 'empty-string/whitespace overrides must never warn'); + }); + + test('non-string model_overrides values are ignored without throwing (#3241)', () => { + // Regression guard — PASSES today already: none of these ever reach the + // string-pin branch (`typeof rawModelOverride === 'string'` gates it), so + // no value here is ever emitted as `model =`, and none of them throw. + // Some truthy non-string values (42, {}, true, []) DO hit the existing + // `_warnCodexModelOverrideDropped` warn branch today — that is pre-2310 + // behavior this phase does not touch, so no assertion is made on warning + // presence/absence here, only "no pin" and "no crash" per the matrix. + const hostileValues = [42, {}, null, true, [], 0, NaN]; + for (const value of hostileValues) { + assert.doesNotThrow(() => { + const result = generateCodexAgentToml('gsd-executor', sampleAgent, { 'gsd-executor': value }); + assert.ok(!/^model = /m.test(result), `non-string override ${JSON.stringify(value)} must not be pinned`); + }, `non-string override ${JSON.stringify(value)} must not throw`); + } + }); + + // 50-test-matrix.md row 15 (oversized warning value truncated at 64 chars) + // is deliberately NOT covered here. Maintainer-confirmed pinned wording + // (see the describe block below) interpolates no user-controlled value — + // no agent name, no model string — so there is nothing in the message that + // could ever exhibit truncation. A test asserting truncation against a + // message with no interpolated value would be vacuous by construction. + + test('light-tier service_tier/model_verbosity survive independent of whether a model is pinned (#3241, #774 decoupling guard)', () => { + // Regression guard — PASSES today already: the light-tier emission block + // (bin/install.js ~L4198-4203) reads AGENT_DEFAULT_TIERS unconditionally + // and never inspects pinnedModel/hasPinnedModel. This test exists to + // catch a FUTURE implementation that wrongly couples these fields to + // hasPinnedModel while implementing #3241 — if that coupling is ever + // introduced, this is the test that turns red. It is not expected to be + // red before the #3241 fix lands, and per 50-test-matrix.md's own + // "Red-before-green" note this is the row most likely to be accidentally + // vacuous — flagged explicitly here rather than mis-classified. + const runtimeResolver = { runtime: 'codex', resolve: () => ({ model: 'gpt-5.6-sol' }) }; + const lightAgent = `--- +name: gsd-plan-checker +description: Checks plans quickly +tools: Read, Grep +--- + +You check plans.`; + const lightResult = generateCodexAgentToml('gsd-plan-checker', lightAgent, null, runtimeResolver); + assert.ok(lightResult.includes('service_tier = "flex"'), 'service_tier must not be coupled to whether a model is pinned'); + assert.ok(lightResult.includes('model_verbosity = "low"'), 'model_verbosity must not be coupled to whether a model is pinned'); + + // Other direction: a non-light agent with no model at all must still gain + // neither field (duplicates the existing #774 coverage at L647-652 + // intentionally — 50-test-matrix.md row 18 folds this into row 17 as the + // same independence guard, viewed from the opposite direction). + const standardResult = generateCodexAgentToml('gsd-executor', sampleAgent, null, null); + assert.ok(!standardResult.includes('service_tier'), 'standard-tier agent must not gain service_tier just because no model is pinned'); + assert.ok(!standardResult.includes('model_verbosity'), 'standard-tier agent must not gain model_verbosity just because no model is pinned'); + }); + + // ─── #3241 review: gate the deprecation notice on "would have been EMBEDDED", + // not "would have been returned" ──────────────────────────────────────────── + // The resolver-would-have-supplied-a-model check above (L652-665) doesn't + // inspect stderr, so it couldn't catch this: the notice must not fire when + // the would-be resolver model would ALSO have been rejected by the #2310 + // Anthropic-flavored gate (L4192) pre-Phase-1 — that user never had the pin + // in the first place, so telling them to set model_overrides is false. The + // notice's one-time dedupe is a module-level boolean shared across every + // test in this file (an earlier test in this describe block, e.g. + // L652-665, may have already latched it), so each test here resets it via + // the documented test seam (_resetCodexWarningDedupeForTests) instead of + // busting require.cache — a cache bust would create a second module + // instance and break every other test in this file that assumes a single + // shared instance. + + function captureStderr(t) { + const origWrite = process.stderr.write; + const chunks = []; + process.stderr.write = (chunk) => { chunks.push(String(chunk)); return true; }; + t.after(() => { process.stderr.write = origWrite; }); + return () => chunks.join('').split(/\r?\n/).filter((l) => l.length > 0); + } + + test('no deprecation notice when the resolver would only have produced an Anthropic-flavored model (#3241 review — defect fix)', (t) => { + _resetCodexWarningDedupeForTests(); + const getLines = captureStderr(t); + // Mixed-runtime config (runtime: opencode) resolving against a Codex + // install target — the #2310 gate (bin/install.js:4192) rejects this + // model BEFORE Phase 1 too, so this user never had the pin. Bare alias + // form covered too, since both routes hit the same gate. + for (const model of ['anthropic/claude-opus-4-8', 'sonnet']) { + const runtimeResolver = { runtime: 'opencode', resolve: () => ({ model }) }; + const result = generateCodexAgentToml('gsd-executor', sampleAgent, null, runtimeResolver); + assert.ok(!/^model = /m.test(result), `no model pinned for would-be resolver model "${model}"`); + } + const noticeLines = getLines().filter((l) => l.startsWith('gsd: notice — ')); + assert.strictEqual(noticeLines.length, 0, + 'no notice: the resolver model would never have survived the #2310 gate pre-Phase-1 either, so nothing was lost'); + }); + + test('deprecation notice still fires when the resolver would have produced a legal Codex model (#3241 review)', (t) => { + _resetCodexWarningDedupeForTests(); + const getLines = captureStderr(t); + const runtimeResolver = { runtime: 'codex', resolve: () => ({ model: 'gpt-5.6-sol' }) }; + const result = generateCodexAgentToml('gsd-executor', sampleAgent, null, runtimeResolver); + assert.ok(!/^model = /m.test(result), 'no model pinned by default (#3241 D1)'); + const noticeLines = getLines().filter((l) => l.startsWith('gsd: notice — ')); + assert.strictEqual(noticeLines.length, 1, + 'exactly one notice: a legal gpt-5.6-sol model would have been embedded pre-Phase-1, and now is not — the fix must not over-correct into silence'); + }); + + test('both the override-dropped warning and the resolver-omitted notice fire for an Anthropic override plus a legal resolver model (#3241 review — intentional, do NOT collapse to one message)', (t) => { + // NOT a defect. Two distinct true facts, two distinct prefixes: + // - model_overrides:"sonnet" is Anthropic-flavored → dropped pre-Phase-1 + // too (#2310 gate on the override path) → `gsd: warning — ` fires. + // - With the override dropped, execution falls through to the runtime + // resolver, which WOULD have supplied "gpt-5.6-sol" (a legal Codex + // model) and that pin WOULD have been embedded pre-Phase-1 → this user + // genuinely lost a pin → `gsd: notice — ` fires too. + // A future reader must not "fix" this down to one message. + _resetCodexWarningDedupeForTests(); + const getLines = captureStderr(t); + const runtimeResolver = { runtime: 'codex', resolve: () => ({ model: 'gpt-5.6-sol' }) }; + const result = generateCodexAgentToml( + 'gsd-executor', sampleAgent, { 'gsd-executor': 'sonnet' }, runtimeResolver, + ); + assert.ok(!/^model = /m.test(result), 'no model pinned (Anthropic override dropped, resolver model not auto-embedded)'); + const lines = getLines(); + const warningLines = lines.filter((l) => l.startsWith('gsd: warning — ')); + const noticeLines = lines.filter((l) => l.startsWith('gsd: notice — ')); + assert.strictEqual(warningLines.length, 1, 'exactly one warning: the Anthropic override was dropped'); + assert.strictEqual(noticeLines.length, 1, 'exactly one notice: the legal resolver model would have been embedded and now is not'); + }); + // ─── #774: service_tier / model_verbosity for light-tier agents ─────────────── test('emits service_tier="flex" and model_verbosity="low" for light-tier agents (#774)', () => { @@ -697,6 +918,75 @@ description: Maps the codebase }); }); +// ─── #3241: shared isAnthropicFlavoredModel / CLAUDE_AGENT_ALIASES surface ───── +// Phase 2/3 need one predicate. 40-design.md's "seam decision" moves +// CLAUDE_AGENT_ALIASES into model-catalog.cjs and defines isAnthropicFlavoredModel +// beside it, re-exporting from model-resolver.cjs for back-compat. Neither exists +// on model-catalog.cjs yet (verified: modelCatalog.isAnthropicFlavoredModel is +// `undefined` today), so every test below is RED against the current tree. + +describe('#3241 isAnthropicFlavoredModel + CLAUDE_AGENT_ALIASES (model-catalog owns it)', () => { + const parityAgent = `--- +name: gsd-executor +description: Executes plans +tools: Read, Write +--- + +You are an executor.`; + + test('predicate flags every Claude tier alias and case/namespace variant (#3241)', () => { + // RED (pre-fix): modelCatalog.isAnthropicFlavoredModel is undefined today, + // so `modelCatalog.isAnthropicFlavoredModel('opus')` throws + // "isAnthropicFlavoredModel is not a function" — this test fails on the + // very first call, before any assertion runs. + for (const alias of ['opus', 'sonnet', 'haiku', 'fable']) { + assert.strictEqual(modelCatalog.isAnthropicFlavoredModel(alias), true, `alias "${alias}" must be flagged`); + } + for (const id of ['claude-opus-4-5', 'anthropic/claude-x', 'us.anthropic.claude-x', 'CLAUDE-X']) { + assert.strictEqual(modelCatalog.isAnthropicFlavoredModel(id), true, `id "${id}" must be flagged (case/namespace variant)`); + } + }); + + test('predicate is false for real Codex ids and non-strings, without throwing (#3241)', () => { + // RED (pre-fix): same "not a function" throw as above — fails before any + // assertion is reached. + for (const value of ['gpt-5.6-sol', 'gpt-4', '', null, undefined, {}, 0]) { + assert.doesNotThrow(() => modelCatalog.isAnthropicFlavoredModel(value), `must not throw for ${JSON.stringify(value)}`); + assert.strictEqual(modelCatalog.isAnthropicFlavoredModel(value), false, `must be false for ${JSON.stringify(value)}`); + } + }); + + test('the alias set has exactly one owner across both modules (#3241)', () => { + // RED (pre-fix): modelCatalog.CLAUDE_AGENT_ALIASES is undefined today + // (model-catalog.cjs exports no such key), so deepStrictEqual against + // modelResolver's real Set fails. Divergence guard per + // 50-test-matrix.md rows 22-23: without this, Phase 2/3 can silently + // fork the rule. + assert.deepStrictEqual( + modelCatalog.CLAUDE_AGENT_ALIASES, + modelResolver.CLAUDE_AGENT_ALIASES, + 'model-catalog and model-resolver must share the exact same CLAUDE_AGENT_ALIASES contents' + ); + }); + + test('installer Codex .toml emission agrees with the shared predicate (#3241)', () => { + // RED (pre-fix): modelCatalog.isAnthropicFlavoredModel is undefined, so + // the first loop iteration throws "not a function" before any + // generateCodexAgentToml call happens. + const probeValues = [...CLAUDE_AGENT_ALIASES, 'claude-sonnet-5', 'gpt-5.6-sol']; + for (const value of probeValues) { + const expectedFlavored = modelCatalog.isAnthropicFlavoredModel(value); + const result = generateCodexAgentToml('gsd-executor', parityAgent, { 'gsd-executor': value }); + const modelLine = result.split(/\r?\n/).find((line) => /^model = /.test(line)); + if (expectedFlavored) { + assert.strictEqual(modelLine, undefined, `"${value}" is Anthropic-flavored per the shared predicate — installer must omit it`); + } else { + assert.strictEqual(modelLine, `model = ${JSON.stringify(value)}`, `"${value}" is NOT Anthropic-flavored per the shared predicate — installer must emit it verbatim`); + } + } + }); +}); + // ─── sandboxTier gate on generateCodexAgentToml ──────────────────────────────── describe('generateCodexAgentToml sandboxTier gate', () => { diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index a41ba1d11..96ba6a6e7 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -3773,10 +3773,16 @@ describe('#443 Config-driven: effort.agent_overrides drives install-time effort' fs.mkdirSync(path.join(projectDir, '.planning'), { recursive: true }); // Write a project config with effort.agent_overrides overriding gsd-planner to 'low'. - // runtime:"codex" pins a Codex-native model, so emitting model_reasoning_effort - // remains valid under the #838 model/effort coupling rule. + // #3241: runtime:"codex" alone (with no model_overrides) no longer auto-pins a + // per-tier model — the resolver-only embed was removed (D1). These tests add an + // explicit model_overrides pin below so a real Codex model id survives (#3241 + // row 4, "unchanged"), which keeps emitting model_reasoning_effort valid under + // the #838 model/effort coupling rule. const config = { runtime: 'codex', + model_overrides: { + 'gsd-planner': 'gpt-5.6-sol', + }, effort: { agent_overrides: { 'gsd-planner': 'low', @@ -3815,9 +3821,13 @@ describe('#443 Config-driven: effort.agent_overrides drives install-time effort' test('Codex .toml clamps effort max → xhigh when agent_overrides.gsd-planner=max', () => { const projectDir = path.dirname(codexHome); - // Overwrite config with max override + // Overwrite config with max override. #3241: include an explicit + // model_overrides pin (D1 removed the resolver-only auto-embed). const config = { runtime: 'codex', + model_overrides: { + 'gsd-planner': 'gpt-5.6-sol', + }, effort: { agent_overrides: { 'gsd-planner': 'max', @@ -3843,6 +3853,133 @@ describe('#443 Config-driven: effort.agent_overrides drives install-time effort' }); }); +// ─── describe 4b: #3241 — deprecation warning when a resolver-sourced model is dropped ─ +// +// Phase 1 (#3241) removes the automatic per-tier Codex model embed sourced from +// readGsdRuntimeProfileResolver. These tests assert the observable side of that +// removal at full-install granularity — a one-time deprecation warning on +// stderr, never on stdout, emitted exactly once across an install even though +// every Codex agent hits the same "resolver would have pinned a model" branch +// simultaneously (the design's Rejected #2: "warn per agent"). +// +// RED (pre-fix): no such warning exists anywhere in bin/install.js today, so +// captured stderr is empty for this config shape and every assertion below +// that looks for warning text fails. +// +// Pinned wording (maintainer-confirmed, so the implementer matches it): +// - single line, on stderr +// - begins "gsd: notice — " (deliberately distinct from the existing +// "gsd: warning — " dropped-override text — this is a notice about an +// intentional behavior change, not a malformed value) +// - contains the literal substring "model_overrides" (the recovery path) +// - contains the literal substring "session model" (what the agent gets +// instead) +// - names neither a specific agent nor a specific model — it is a +// whole-install condition, not a per-agent one +// Tests assert on these substrings plus "exactly one matching line", not on +// the full sentence, so the prose can improve without breaking the test. + +describe('#3241 Codex install: deprecation warning when a resolver-sourced model is dropped', () => { + let tmpDir; + let projectDir; + let codexHome; + let stderrChunks; + let stdoutChunks; + let origStderrWrite; + let origStdoutWrite; + + beforeEach(() => { + tmpDir = makeTmpDir('gsd-3241-codex-warn-'); + projectDir = path.join(tmpDir, 'project'); + codexHome = path.join(projectDir, '.codex'); + fs.mkdirSync(codexHome, { recursive: true }); + fs.mkdirSync(path.join(projectDir, '.planning'), { recursive: true }); + // runtime:"codex" + default model_profile:"balanced", no model_overrides — + // the exact shipping shape (per 50-test-matrix.md's "Altitude" note) that + // resolves a tier model per-agent via readGsdRuntimeProfileResolver (#2517). + fs.writeFileSync( + path.join(projectDir, '.planning', 'config.json'), + JSON.stringify({ runtime: 'codex' }, null, 2) + ); + stderrChunks = []; + stdoutChunks = []; + origStderrWrite = process.stderr.write; + origStdoutWrite = process.stdout.write; + process.stderr.write = (chunk) => { stderrChunks.push(String(chunk)); return true; }; + process.stdout.write = (chunk) => { stdoutChunks.push(String(chunk)); return true; }; + }); + + afterEach(() => { + process.stderr.write = origStderrWrite; + process.stdout.write = origStdoutWrite; + cleanup(tmpDir); + }); + + test('warns exactly once on stderr with the pinned notice wording, and never on stdout (#3241)', () => { + runGlobalInstall('codex', codexHome); + const stderr = stderrChunks.join(''); + const stdout = stdoutChunks.join(''); + const noticeLines = stderr.split(/\r?\n/).filter((line) => line.startsWith('gsd: notice — ')); + assert.strictEqual(noticeLines.length, 1, + `expected exactly one matching notice line, got ${noticeLines.length}\nstderr:\n${stderr}`); + assert.match(noticeLines[0], /model_overrides/, + 'the notice must name model_overrides as the recovery path'); + assert.match(noticeLines[0], /session model/, + 'the notice must name the session model as what the agent gets instead'); + assert.doesNotMatch(stdout, /gsd: notice — /, + 'stdout must never carry the notice text — stdout carries installer result data'); + }); + + test('the deprecation notice fires exactly once per install across every Codex agent, not once per agent, and no agent .toml pins a model by default (#3241)', () => { + // This test rides two properties deliberately: the notice's per-install + // dedupe (its original subject) AND the actual emitted-file content. The + // three describe-4 tests above (#443 Config-driven) all supply an + // explicit model_overrides pin to keep exercising unrelated effort- + // resolution behavior, so none of them prove the default (no + // model_overrides) install path actually stops pinning a model — only + // the unit-level generateCodexAgentToml tests in codex-config.test.cjs + // did. This is therefore the only install()-altitude coverage of Phase + // 1's headline behavior until Phase 4's health-check/sync land. + runGlobalInstall('codex', codexHome); + const stderr = stderrChunks.join(''); + const installedTomls = fs.readdirSync(path.join(codexHome, 'agents')) + .filter((name) => name.endsWith('.toml')); + // Sanity check: this config shape must actually install more than one + // Codex agent — otherwise "exactly once, not once per agent" is untestable. + assert.ok(installedTomls.length > 1, + `sanity check: install must produce more than one Codex agent .toml, got ${installedTomls.length}`); + const noticeLines = stderr.split(/\r?\n/).filter((line) => line.startsWith('gsd: notice — ')); + assert.strictEqual(noticeLines.length, 1, + `expected the notice exactly once across ${installedTomls.length} installed agents, saw ${noticeLines.length}\nstderr:\n${stderr}`); + + // Emission check. Trap (same one ADR-2313 flags for Phase 3's sync): + // `developer_instructions` is a `'''`-quoted TOML multiline block holding + // the agent's raw prompt text, and GSD agent prompts discuss "model" + // constantly — a bare `content.includes('model =')` would match prose + // inside that block and false-fail. Guard against it by only scanning the + // lines BEFORE the `developer_instructions = '''` marker (generateCodexAgentToml + // always emits model / model_reasoning_effort, if present, ahead of that + // marker — bin/install.js's `lines` array construction), and by anchoring + // each check on a line that STARTS a TOML key (`^model\s*=`), never a bare + // substring match. + for (const tomlName of installedTomls) { + const tomlPath = path.join(codexHome, 'agents', tomlName); + const content = fs.readFileSync(tomlPath, 'utf8'); + const lines = content.split(/\r?\n/); + const bodyStart = lines.findIndex((line) => line.startsWith("developer_instructions = '''")); + assert.notStrictEqual(bodyStart, -1, + `${tomlName}: expected a developer_instructions = ''' marker to scope the header scan against\nActual:\n${content.slice(0, 500)}`); + const header = lines.slice(0, bodyStart); + const modelLine = header.find((line) => /^model\s*=/.test(line)); + const effortLine = header.find((line) => /^model_reasoning_effort\s*=/.test(line)); + assert.strictEqual(modelLine, undefined, + `${tomlName} must not pin a model by default (#3241 — no model_overrides configured) — found line: ${JSON.stringify(modelLine)}`); + assert.strictEqual(effortLine, undefined, + `${tomlName} must not emit model_reasoning_effort with no model pinned (#838 coupling) — found line: ${JSON.stringify(effortLine)}`); + } + }); +}); + // ─── describe 5b: Invalid effort tokens fall through (Codex adversarial finding #2) ─ // // These tests FAIL before the fix: resolveInstallTimeEffort returns the raw @@ -3913,7 +4050,15 @@ describe('#443 resolveInstallTimeEffort: invalid tokens fall through to valid ef test('effort.default="ultra" (invalid) + runtime:"codex" -> Codex .toml model_reasoning_effort is VALID', () => { // BUG before fix: "ultra" written into .toml verbatim - writeProjectConfig({ runtime: 'codex', effort: { default: 'ultra' } }); + // #3241: include an explicit model_overrides pin — D1 removed the + // resolver-only auto-embed, and model_reasoning_effort is only emitted + // when a model is pinned (#838 coupling), so a pin is required to + // exercise this test's actual target (effort-token fallback validity). + writeProjectConfig({ + runtime: 'codex', + model_overrides: { 'gsd-planner': 'gpt-5.6-sol' }, + effort: { default: 'ultra' }, + }); runGlobalInstall('codex', codexHome); const tomlContent = fs.readFileSync( path.join(codexHome, 'agents', 'gsd-planner.toml'), 'utf8' diff --git a/tests/issue-2517-runtime-aware-profiles.test.cjs b/tests/issue-2517-runtime-aware-profiles.test.cjs index bf3f4c46a..17c0a5c0f 100644 --- a/tests/issue-2517-runtime-aware-profiles.test.cjs +++ b/tests/issue-2517-runtime-aware-profiles.test.cjs @@ -571,7 +571,10 @@ describe('issue #2517: install end-to-end — per-project config reaches Codex T assert.strictEqual(entry.model, 'gpt-5.6-sol'); }); - test('generated Codex TOML embeds model = and model_reasoning_effort = lines', () => { + test('generated Codex TOML omits model = and model_reasoning_effort = lines when only the resolver would have supplied them (#3241)', () => { + // #3241 flips this: the runtime-resolver auto-embed (D1) was removed, so a + // resolver alone with no explicit model_overrides no longer pins a model, + // and #838's coupling means the reasoning-effort line is omitted too. writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); const resolver = readGsdRuntimeProfileResolver(tmpDir); const toml = generateCodexAgentToml( @@ -580,16 +583,21 @@ describe('issue #2517: install end-to-end — per-project config reaches Codex T null, resolver ); - assert.match(toml, /^model = "gpt-5\.6-sol"$/m); - assert.match(toml, /^model_reasoning_effort = "xhigh"$/m); + assert.doesNotMatch(toml, /^model = "gpt-5\.6-sol"$/m); + assert.doesNotMatch(toml, /^model_reasoning_effort = "xhigh"$/m); }); - test('generated TOML always includes model_reasoning_effort even when model_profile_overrides sets reasoning_effort to empty (#443 unified)', () => { + test('generated TOML always includes model_reasoning_effort even when model_profile_overrides sets reasoning_effort to empty (#443 unified) (#3241: model now pinned via explicit model_overrides, not the resolver alone)', () => { // Under the unified effort design (#443), model_reasoning_effort in the Codex TOML // is driven by the unified effort resolver (resolveInstallTimeEffort / effortCfg), // NOT by model_profile_overrides.reasoning_effort. Setting reasoning_effort: '' in - // model_profile_overrides does NOT suppress the unified effort — the TOML always - // carries a valid model_reasoning_effort drawn from the agent's routing tier. + // model_profile_overrides does NOT suppress the unified effort when a model IS + // pinned — the TOML carries a valid model_reasoning_effort drawn from the agent's + // routing tier. + // #3241: the resolver alone no longer pins a model (D1), so this test now supplies + // an explicit model_overrides pin ('custom', a real-looking Codex id — row 4, + // "unchanged") to keep exercising the unrelated property under test: that + // model_profile_overrides.reasoning_effort is ignored by the unified resolver. // gsd-planner is a heavy-tier agent → unified default resolves to "xhigh". writeConfig(tmpDir, { runtime: 'codex', @@ -600,12 +608,13 @@ describe('issue #2517: install end-to-end — per-project config reaches Codex T const toml = generateCodexAgentToml( 'gsd-planner', '---\nname: gsd-planner\n---\nBody.\n', - null, + { 'gsd-planner': 'custom' }, resolver ); - // Model override (from model_profile_overrides) is still respected. + // Explicit model_overrides pin is respected (#3241 row 4 — unchanged). assert.match(toml, /^model = "custom"$/m); - // Unified effort always fires — model_reasoning_effort is present and valid. + // Unified effort always fires when a model is pinned — model_reasoning_effort is + // present and valid, ignoring model_profile_overrides.reasoning_effort. assert.match(toml, /^model_reasoning_effort = "(minimal|low|medium|high|xhigh)"$/m); // gsd-planner is heavy-tier, so with no effortCfg the manifest tier default applies → xhigh. assert.match(toml, /^model_reasoning_effort = "xhigh"$/m);