diff --git a/.changeset/jolly-quails-howl.md b/.changeset/jolly-quails-howl.md new file mode 100644 index 000000000..22de0d64f --- /dev/null +++ b/.changeset/jolly-quails-howl.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3891 +--- +**Codex worktree executors now run on the model you pinned for them.** With `model_overrides.gsd-executor` set, `$gsd-execute-phase` spawned its worktree executor with no `--model` argument at all, so the child silently fell back to the global Codex session model — and because this path spawns a process rather than dispatching a named agent, the model baked into `gsd-executor.toml` could not apply either. An explicitly pinned model is now passed to the spawned process. An unpinned, blank, or `inherit` configuration still emits no model argument and keeps the session-model fallback, so Codex's session-only model posture is unchanged and no tier-derived model is ever sent. A pin that is Anthropic-flavored (`sonnet`, `opus`, `claude-*`), flag-shaped (e.g. `-c`), or otherwise outside the model-id character set is now dropped with a stderr warning instead of being sent to Codex — which would 400 — or aborting the whole run. (#3714) diff --git a/capabilities/codex/capability.json b/capabilities/codex/capability.json index eb60ed4bf..c0cb0a4a8 100644 --- a/capabilities/codex/capability.json +++ b/capabilities/codex/capability.json @@ -100,7 +100,8 @@ "exec" ], "cwdFlag": "--cd", - "promptFlag": null + "promptFlag": null, + "modelFlag": "--model" }, "hostBehaviors": { "reapplyCommand": "$gsd-update --reapply", diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index f9dadf0c9..5b5b3912c 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -1778,6 +1778,155 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load } } + // #3714 follow-up — the dispatch seam gated only on PRESENCE of an explicit + // pin, never on its VALUE, so an Anthropic-flavored global default + // (~/.gsd/defaults.json model_overrides["gsd-executor"] = "sonnet"/"opus"/ + // "claude-*") reached `codex exec --model sonnet`: the documented #2310/ + // #2311 400 on a passive-posture host (ADR-1239/ADR-2313). It also let a + // repo-committed .planning/config.json inject shell-hostile argv (a + // `-c approval_policy=never` suffix, `$(...)`/`;` command injection, + // embedded control characters) straight onto exec's argv. + // + // This mirrors — deliberately, not by re-derivation — the same VALUE + // policy bin/install.js's generateCodexAgentToml() already applies to the + // identical model_overrides["gsd-executor"] config key for the .toml + // surface (bin/install.js ~3983-4046): trim; a whitespace-only value drops + // silently (#3241, no warning); an Anthropic-flavored value + // (isAnthropicFlavoredModel, single-sourced on bin/lib/model-catalog.cjs + // per #3241 specifically so it cannot diverge across Codex-posture + // surfaces) drops WITH a warning; a real pin survives verbatim. Two + // additions beyond the .toml surface, both specific to this seam: the + // 'inherit' sentinel (case/whitespace-insensitive) is a no-op here already + // and must stay one, and a value that doesn't look like a model id at all + // (the injection case above — the .toml surface never had to consider this + // because TOML string-quoting isn't a shell argv boundary) is dropped with + // a warning rather than ever reaching child_process argv. + // Single source of truth for the model-id "allowed characters" notion + // (#3714 follow-up — "Generative Fix Divergence"): the accept regex + // (MODEL_ID_CHARSET_RE, used to ADMIT a pin) and the sanitizer keep-class + // (MODEL_ID_SANITIZE_STRIP_RE, used to RENDER a rejected pin into a + // warning) are both derived from this one character-class body so they + // cannot drift apart again the way they already did once (the '@' added + // for Vertex pins landed in the accept regex but not the sanitizer, + // rendering "text-bison@002" as "text-bison?002" in the warning). '@' is + // included for Vertex model-version pins ("text-bison@002", + // "chat-bison@001"), which are legitimate model ids reachable through a + // custom model_provider. + // This body is interpolated raw into BOTH a positive character class + // (MODEL_ID_CHARSET_RE, `[BODY]`) and a negated one + // (MODEL_ID_SANITIZE_STRIP_RE, `[^BODY]`) below — only plain characters + // and `x-y` ranges are safe here. A class metacharacter (`^`, `]`, `\`) + // would mean different things in the two derived regexes if ever added. + const MODEL_ID_CHARSET_BODY = 'A-Za-z0-9._:/@-'; + // The first character must be alphanumeric (#3714 hardening): a leading + // '.', '_', ':', '/', or '@' has no legitimate model-id use case and, for + // resolveOrchestratorExec's documented role as a GENERAL descriptor→argv + // seam other hosts may adopt, a leading '@' or '/' is exactly the shape an + // @-response-file or /-switch parser would key on. The leading-dash shape + // is enforced separately by LEADING_DASH_RE below — it is NOT relaxed + // here, since a flag-shaped value ("-c", "--config") must still fail the + // resolver's own unsafe_leading_dash_model guard path via that dedicated + // check, not this charset. + const MODEL_ID_CHARSET_RE = new RegExp(`^[A-Za-z0-9][${MODEL_ID_CHARSET_BODY}]*$`); + // Keep-class for sanitizing a REJECTED pin before it reaches the warning + // (a guaranteed-reachable raw-to-TTY sink — the dispatch step runs with no + // `2>` redirect). Built from the same MODEL_ID_CHARSET_BODY as the accept + // regex above, so every character the matcher accepts also survives the + // sanitizer unchanged, and a widened charset can never diverge from its + // rendering again. + // + // This `g`-flagged instance is for internal `.replace()` use ONLY — a + // `/g` regex is stateful (`.lastIndex` persists across calls) and + // `.test()` on it alternates true/false/true across repeated calls on the + // same string, a false-green trap for any test that reaches for `.test()` + // instead of `.replace()`. To make that trap impossible rather than just + // documenting it, this `g`-flagged object is never exported; the exported + // `MODEL_ID_SANITIZE_STRIP_RE` below is a separate, non-global instance + // built from the same body, safe for `.test()`/`.match()` in tests. + const MODEL_ID_SANITIZE_STRIP_RE_G = new RegExp(`[^${MODEL_ID_CHARSET_BODY}]`, 'g'); + // Non-global companion of MODEL_ID_SANITIZE_STRIP_RE_G, exported for + // tests. Do not use with `.replace()` on a value containing more than one + // disallowed character — it only replaces the first match. Production + // code must use the `g`-flagged instance above instead. + const MODEL_ID_SANITIZE_STRIP_RE = new RegExp(`[^${MODEL_ID_CHARSET_BODY}]`); + // A model id has no legitimate reason to be long; this also keeps a + // pathological pin away from the Windows argv ceiling (execFileSync aborts + // if argv > 32,767 chars — CLAUDE.md "Windows ARGV Overflow"). A pin over + // this length is DROPPED WITH A WARNING like every other rejection, never + // truncated into argv — a truncated model id is a different model id. + const MODEL_ID_MAX_LENGTH = 200; + const _dispatchModelPinDropWarned = new Set(); + function _warnDispatchModelPinDropped(agentName, rawValue, reason) { + const key = `${agentName}::${rawValue}::${reason}`; + if (_dispatchModelPinDropWarned.has(key)) return; + _dispatchModelPinDropWarned.add(key); + // Sanitize BEFORE truncating: every value that reaches this warning + // failed the model-id charset test by definition (or, for the + // over-length case, still only ever contains charset-legal bytes) — + // sanitizing first catches raw control/escape bytes (ESC, BEL, CSI + // sequences) using the identity-sanitizing pattern already used for + // --as at gsd-tools.cjs:1526. Sanitizing before truncating also ensures + // a truncated escape sequence can never survive (e.g. an SGR sequence + // cut before its reset, leaving sticky terminal state) — truncation + // only ever cuts already-safe characters. + const sanitized = String(rawValue).replace(MODEL_ID_SANITIZE_STRIP_RE_G, '?'); + const safe = sanitized.length > 64 ? `${sanitized.slice(0, 64)}…` : sanitized; + process.stderr.write( + `gsd: warning — dispatch model pin for agent "${agentName}" (value "${safe}") ${reason}; ` + + `dropping it so the spawned executor falls back to the session model.\n`, + ); + } + // A value starting with '-' (or '--') is a flag/option shape, not a model + // id — `-c`, `--config`, `-`, `--`, `-p` are unsafe to hand to + // resolveOrchestratorExec, whose own `unsafe_leading_dash_model` guard + // rejects them and fails the WHOLE resolution to `{ ok: false }` -> + // exec:null -> a FATAL wave abort (executor-isolation-dispatch.md:299-303), + // even on hosts (e.g. kimi-code) that declare no modelFlag at all and + // previously ignored the pin entirely. This check MUST run BEFORE + // MODEL_ID_CHARSET_RE below: the charset is anchored to `[A-Za-z0-9]` at + // the first character, so every dash-leading value already fails the + // charset test and would otherwise be swallowed by the generic "unsafe + // characters" message, losing the more specific and more actionable + // flag/option diagnosis. Reject here, at the VALUE-policy layer, so a + // leading-dash value degrades to "no model" like every other rejected + // shape, instead of reaching a resolver whose failure mode is fatal + // rather than a graceful drop. + const LEADING_DASH_RE = /^-/; + /** + * Resolve the VALUE policy for an explicit dispatch model pin. `rawValue` + * is whatever resolveAgentModelOverride(..., null) returned — a string + * pin, the 'inherit' sentinel, or null/''/undefined for "no explicit pin". + * Returns the trimmed model string to embed, or `undefined` to emit no + * --model flag at all. Never throws; never fails closed to an error — + * every rejection degrades to "no model" (drop-and-warn), matching the + * documented desired behavior of falling back to the session model rather + * than aborting the wave. + */ + function resolveDispatchModelPin(agentName, rawValue) { + if (typeof rawValue !== 'string') return undefined; // not a string -> no model + const trimmed = rawValue.trim(); + if (trimmed === '') return undefined; // whitespace-only -> no model, no warning (#3241) + if (trimmed.toLowerCase() === 'inherit') return undefined; // sentinel -> no model, no warning + const { isAnthropicFlavoredModel } = require('./lib/model-catalog.cjs'); + if (isAnthropicFlavoredModel(trimmed)) { + _warnDispatchModelPinDropped(agentName, rawValue, 'is an Anthropic-flavored model/alias, not a valid Codex model'); + return undefined; + } + if (LEADING_DASH_RE.test(trimmed)) { + _warnDispatchModelPinDropped(agentName, rawValue, 'looks like a flag/option, not a model id (leading "-")'); + return undefined; + } + if (!MODEL_ID_CHARSET_RE.test(trimmed)) { + _warnDispatchModelPinDropped(agentName, rawValue, 'does not look like a model id (unsafe characters)'); + return undefined; + } + if (trimmed.length > MODEL_ID_MAX_LENGTH) { + _warnDispatchModelPinDropped(agentName, rawValue, `exceeds the maximum model id length (${MODEL_ID_MAX_LENGTH} characters)`); + return undefined; + } + return trimmed; + } + const DISPATCH_ISOLATION_VOCABULARY = new Set(['harness-worktree', 'orchestrator-worktree', 'none']); /** @@ -1841,14 +1990,67 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load const promptIdx = args.indexOf('--prompt'); const promptArg = promptIdx !== -1 ? args[promptIdx + 1] : undefined; const hostIntegration = require('./lib/host-integration.cjs'); + // #3714: resolve an EXPLICIT, non-sentinel per-agent model pin for the + // spawned worktree executor. Passing `null` as the runtime resolver + // (3rd arg) is LOAD-BEARING, not an oversight — it is what keeps + // profile/tier-derived models out of argv. Codex's `modelMode: passive` + // posture (ADR-1239) and ADR-2313 forbid GSD driving model selection on + // this host; only an operator's EXPLICIT override may cross this seam. + // resolveAgentModelOverride(..., null) returns a value ONLY when the + // operator pinned one explicitly (measured: unpinned -> null, + // "inherit" -> "inherit" meaning "use the ambient session model, don't + // pass a flag", "" -> null, profile-only -> null). Do NOT swap in + // resolve-model / a full model-resolver here: that resolver falls back + // to a default (e.g. "sonnet") for the unpinned/profile-only cases, + // and emitting that on Codex's exec argv is exactly the documented + // #2310/#2311 regression (a model unknown to Codex forced into a + // passive-posture host). + // + // Presence of a pin is necessary but not sufficient: resolveDispatchModelPin + // applies the same VALUE policy the install-side .toml surface already + // applies to this config key (trim / drop-inherit / drop-Anthropic-flavored + // with a warning / drop-non-model-id-charset with a warning) so a global + // Anthropic-flavored default or an injected config value never reaches argv. + // + // This whole VALUE policy — including its warning — is gated on the + // resolved runtime's descriptor actually declaring a non-empty + // `modelFlag`. The policy runs at this host-NEUTRAL site, so a host + // with no modelFlag at all (kimi, kimi-code, opencode) was never + // going to emit a --model regardless of the pin's value; running the + // policy anyway produced a stderr warning claiming "dropping it so + // the spawned executor falls back to the session model" on every + // dispatch for such a host — misleading today, and actively wrong if + // a Claude-capable host ever declares a modelFlag. When the + // descriptor declares no modelFlag, skip the policy entirely: no + // model, no warning, argv byte-identical to before this pin policy + // existed. + const declaresModelFlag = typeof runtimeEntry?.runtime?.orchestratorExec?.modelFlag === 'string' && + runtimeEntry.runtime.orchestratorExec.modelFlag.length > 0; + let model; + if (declaresModelFlag) { + const { readGsdEffectiveModelOverrides, resolveAgentModelOverride } = + require('./lib/install-model-override-resolver.cjs'); + const pinned = resolveAgentModelOverride( + 'gsd-executor', readGsdEffectiveModelOverrides(cwd), null); + model = resolveDispatchModelPin('gsd-executor', pinned); + } const resolution = hostIntegration.resolveOrchestratorExec( runtimeEntry?.runtime?.orchestratorExec, cwdTarget, promptArg, + model, ); - // A host declaring orchestrator-worktree whose exec descriptor does - // not resolve cannot be spawned — degrade to sequential rather than - // hand the scheduler an unusable command. + // A host declaring orchestrator-worktree whose exec descriptor does not + // resolve halts THIS wave's dispatch: isolation is forced to 'none' here, + // and executor-isolation-dispatch.md:299-303 treats a null exec as FATAL + // (exit 1) after the worktree has already been created — it does not + // degrade to sequential execution. resolveDispatchModelPin rejects any + // leading-dash value (flag/option shape) before it ever reaches this + // resolver specifically so it cannot trip the resolver's own + // `unsafe_leading_dash_model` guard and turn a bad config value into + // this fatal path; every other unresolvable model value likewise + // degrades to "no --model" (session model fallback) rather than to + // resolution.ok === false. if (resolution.ok) { exec = { command: resolution.command, args: resolution.args, cwd: resolution.cwd }; } else { @@ -4434,5 +4636,21 @@ module.exports = { // #3275: exported for tests — the shared PATH+PATHEXT resolver behind // review-lane invoke's `deps.spawn` / `deps.hasBinary` seams. resolveSpawnBinary, + // #3714 follow-up: exported for tests — the dispatch model-pin VALUE + // policy (charset accept/render parity, max-length boundary, leading-char + // anchor) is otherwise unreachable from outside the dispatchOverlayCapabilityCommand closure. + resolveDispatchModelPin, + MODEL_ID_CHARSET_RE, + // The shared character-class body both MODEL_ID_CHARSET_RE and + // MODEL_ID_SANITIZE_STRIP_RE are derived from — exported so a test can + // assert its own expected charset literal EQUALS this value, making a + // silent widening of the production body fail the test instead of only + // the (unexported) regexes built from it. + MODEL_ID_CHARSET_BODY, + // Non-global companion of the internal g-flagged sanitize regex — see the + // comment at its definition for why the g-flagged instance is never + // exported. + MODEL_ID_SANITIZE_STRIP_RE, + MODEL_ID_MAX_LENGTH, }; diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index 265319a29..65425559b 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -1162,7 +1162,8 @@ const capabilities = { "exec" ], "cwdFlag": "--cd", - "promptFlag": null + "promptFlag": null, + "modelFlag": "--model" }, "hostBehaviors": { "reapplyCommand": "$gsd-update --reapply", @@ -5984,7 +5985,8 @@ const runtimes = { "exec" ], "cwdFlag": "--cd", - "promptFlag": null + "promptFlag": null, + "modelFlag": "--model" }, "hostBehaviors": { "reapplyCommand": "$gsd-update --reapply", diff --git a/gsd-core/bin/lib/capability-validator.cjs b/gsd-core/bin/lib/capability-validator.cjs index 88c68361e..53e4bccce 100644 --- a/gsd-core/bin/lib/capability-validator.cjs +++ b/gsd-core/bin/lib/capability-validator.cjs @@ -1588,6 +1588,16 @@ function validateRuntimeBody(cap) { 'runtime.orchestratorExec.promptFlag must be a string or null (got: ' + JSON.stringify(oe.promptFlag) + ')', ); } + + // modelFlag — optional; string or null (#3714). `null`/absent means the + // host offers no per-invocation model override on this exec path; a + // string names the flag that pins the executor's model (codex: --model). + // Deliberately asymmetric: only codex declares this today. + if (oe.modelFlag !== undefined && oe.modelFlag !== null && typeof oe.modelFlag !== 'string') { + errors.push( + 'runtime.orchestratorExec.modelFlag must be a string or null (got: ' + JSON.stringify(oe.modelFlag) + ')', + ); + } } } diff --git a/src/host-integration.cts b/src/host-integration.cts index 472fd47a4..cefa64e6b 100644 --- a/src/host-integration.cts +++ b/src/host-integration.cts @@ -798,6 +798,7 @@ interface OrchestratorExec { args?: string[]; cwdFlag?: string | null; promptFlag?: string | null; + modelFlag?: string | null; } type OrchestratorExecResolution = @@ -823,13 +824,29 @@ type OrchestratorExecResolution = * per-host branch ADR-1239 exists to remove. Omit `prompt` entirely and the * resolution is byte-identical to Phase 2's (the unconsumed-resolver shape). * - * Argv order is base args → cwd flag → prompt, so the prompt stays the final - * positional token for the hosts that read it that way. + * Argv order is base args → model flag → cwd flag → prompt, so the prompt + * stays the final positional token for the hosts that read it that way. The + * model flag is placed BEFORE the cwd flag (not after, and not appended at + * the very end) purely to keep it clear of that trailing positional — the + * model value itself is never the prompt-adjacent token a host might scan + * for last. + * + * `model` (Phase 4, #3714) is optional, descriptor-gated exactly like prompt: + * a `modelFlag` string on the descriptor names the flag that pins the + * spawned executor's model (codex: `--model`); `null`/absent means the host + * exposes no such override on this exec path, and `[modelFlag, model]` is + * appended ONLY when both the descriptor's `modelFlag` and the caller's + * `model` are non-empty strings. Omitting `model` entirely (or passing it to + * a host with no `modelFlag`) is byte-identical to the resolver's behavior + * before this parameter existed. This function decides no policy about WHICH + * model to pass or what 'inherit' means — that is entirely the caller's job; + * this seam only shapes descriptor + values into argv. */ function resolveOrchestratorExec( orchestratorExec: OrchestratorExec | undefined, cwd: string, prompt?: string, + model?: string, ): OrchestratorExecResolution { if (!orchestratorExec || typeof orchestratorExec !== 'object' || Array.isArray(orchestratorExec)) { return { ok: false, reason: 'missing_command' }; @@ -850,11 +867,22 @@ function resolveOrchestratorExec( if (oe.promptFlag !== undefined && oe.promptFlag !== null && typeof oe.promptFlag !== 'string') { return { ok: false, reason: 'invalid_prompt_flag' }; } + if (oe.modelFlag !== undefined && oe.modelFlag !== null && typeof oe.modelFlag !== 'string') { + return { ok: false, reason: 'invalid_model_flag' }; + } // An executor spawned with no instruction is a hang, not a degraded run — // fail closed rather than launching a prompt-less process. if (prompt !== undefined && (typeof prompt !== 'string' || prompt.length === 0)) { return { ok: false, reason: 'invalid_prompt' }; } + // Unlike `prompt` — where empty is a hang, not a degraded run, hence the + // fail-closed check above — an absent/null/empty model is simply "use the + // host default", the same benign degradation `cwdFlag: null` already + // expresses. Only a present-but-non-string value (number/bool/array/object) + // is a caller error; null/undefined/'' fall through to "omit the flag". + if (model !== undefined && model !== null && typeof model !== 'string') { + return { ok: false, reason: 'invalid_model' }; + } // Leading-dash guard, mirroring worktree-safety.cts's `unsafe_leading_dash` // check on git arguments. A positional prompt (or a cwd) beginning with '-' // is parsed by the spawned CLI as a FLAG, not a value — the same failure the @@ -869,11 +897,20 @@ function resolveOrchestratorExec( if (cwd.startsWith('-')) { return { ok: false, reason: 'unsafe_leading_dash_cwd' }; } + if (typeof model === 'string' && model.startsWith('-')) { + return { ok: false, reason: 'unsafe_leading_dash_model' }; + } const baseArgs = Array.isArray(oe.args) ? [...oe.args] : []; - const args = typeof oe.cwdFlag === 'string' && oe.cwdFlag.length > 0 - ? [...baseArgs, oe.cwdFlag, cwd] - : baseArgs; + let args = baseArgs; + + if (typeof oe.modelFlag === 'string' && oe.modelFlag.length > 0 && typeof model === 'string' && model.length > 0) { + args = [...args, oe.modelFlag, model]; + } + + args = typeof oe.cwdFlag === 'string' && oe.cwdFlag.length > 0 + ? [...args, oe.cwdFlag, cwd] + : args; if (typeof prompt === 'string') { if (typeof oe.promptFlag === 'string' && oe.promptFlag.length > 0) { diff --git a/src/model-catalog.cts b/src/model-catalog.cts index de56c8e2b..9fff17be5 100644 --- a/src/model-catalog.cts +++ b/src/model-catalog.cts @@ -215,7 +215,9 @@ export const KNOWN_PROVIDERS: Set = new Set( 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')); + if (typeof model !== 'string') return false; + const lower = model.toLowerCase(); + return CLAUDE_AGENT_ALIASES.has(lower) || lower.includes('claude'); } export function nextTier(currentTier: string): string | null { diff --git a/tests/dispatch-model-pin.test.cjs b/tests/dispatch-model-pin.test.cjs new file mode 100644 index 000000000..1b2038f3f --- /dev/null +++ b/tests/dispatch-model-pin.test.cjs @@ -0,0 +1,215 @@ +// Guards the dispatch model-pin VALUE policy in gsd-core/bin/gsd-tools.cjs +// (`resolveDispatchModelPin`, `MODEL_ID_CHARSET_RE`, `MODEL_ID_CHARSET_BODY`, +// `MODEL_ID_SANITIZE_STRIP_RE`, `MODEL_ID_MAX_LENGTH`): +// +// - Item 1: the accept-class (matcher) and the render-class (sanitizer) +// are single-sourced from one character-class body and can never drift +// apart again the way they already did once (accept regex gained '@' +// for Vertex pins; sanitizer keep-class did not, so a rejected +// Vertex-shaped pin rendered "text-bison?002" instead of +// "text-bison@002"). +// - Item 2: a pin longer than MODEL_ID_MAX_LENGTH (200) is dropped with a +// warning rather than reaching argv truncated. Boundary rows at +// limit-1/limit/limit+1 (199/200/201). +// - Item 3: the leading character must be alphanumeric, closing off +// '@'/'/' -shaped values (`@evil`, `/c`) from reaching argv, while five +// legitimate real-world model ids continue to pass unchanged. +// +// Every rejection path must degrade to "no model" (drop-and-warn) — never +// `undefined` being skipped in favor of an exception, and never a value +// that still contains an unsafe character escaping to argv. + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fc = require('fast-check'); + +const gsdTools = require('../gsd-core/bin/gsd-tools.cjs'); +const { + resolveDispatchModelPin, + MODEL_ID_CHARSET_RE, + MODEL_ID_CHARSET_BODY, + MODEL_ID_SANITIZE_STRIP_RE, + MODEL_ID_MAX_LENGTH, +} = gsdTools; + +/** Capture process.stderr.write() calls made during `fn()`. */ +function captureStderr(fn) { + const original = process.stderr.write; + const chunks = []; + process.stderr.write = (chunk) => { + chunks.push(String(chunk)); + return true; + }; + try { + fn(); + } finally { + process.stderr.write = original; + } + return chunks.join(''); +} + +describe('#3714 follow-up: dispatch model-pin VALUE policy', () => { + test('MODEL_ID_MAX_LENGTH is the documented 200', () => { + assert.strictEqual(MODEL_ID_MAX_LENGTH, 200); + }); + + test('five legitimate real-world model ids all pass through unchanged', () => { + const ids = [ + 'gpt-5.6-terra', + 'synthetic/hf:zai-org/GLM-5.2', + 'text-bison@002', + 'gpt-4o_mini', + 'azure/deployment', + ]; + for (const id of ids) { + const stderr = captureStderr(() => { + assert.strictEqual(resolveDispatchModelPin(`agent-${id}`, id), id); + }); + assert.strictEqual(stderr, '', `expected no warning for legitimate id "${id}"`); + } + }); + + test('item 3: leading "@" and leading "/" are dropped with a warning, never reach argv', () => { + for (const bad of ['@evil', '/c']) { + const stderr = captureStderr(() => { + assert.strictEqual(resolveDispatchModelPin(`agent-${bad}`, bad), undefined); + }); + assert.match(stderr, /gsd: warning/); + assert.match(stderr, /unsafe characters/); + } + }); + + test('leading-dash values report the flag/option message, not the generic unsafe-characters message (LEADING_DASH_RE must run before MODEL_ID_CHARSET_RE)', () => { + for (const bad of ['-c', '--config', '-', '--', '-p']) { + const stderr = captureStderr(() => { + assert.strictEqual(resolveDispatchModelPin(`agent-${bad}`, bad), undefined); + }); + assert.match(stderr, /gsd: warning/); + assert.match(stderr, /looks like a flag\/option, not a model id \(leading "-"\)/); + assert.doesNotMatch(stderr, /unsafe characters/); + } + }); + + test('non-dash out-of-charset values still report the generic unsafe-characters message', () => { + for (const bad of ['@evil', 'has a space']) { + const stderr = captureStderr(() => { + assert.strictEqual(resolveDispatchModelPin(`agent-x`, bad), undefined); + }); + assert.match(stderr, /gsd: warning/); + assert.match(stderr, /unsafe characters/); + assert.doesNotMatch(stderr, /flag\/option/); + } + }); + + test('item 2: boundary rows at limit-1/limit/limit+1 (199/200/201)', () => { + const at199 = 'a'.repeat(199); + const at200 = 'a'.repeat(200); + const at201 = 'a'.repeat(201); + + let stderr = captureStderr(() => { + assert.strictEqual(resolveDispatchModelPin('agent-199', at199), at199); + }); + assert.strictEqual(stderr, '', '199 chars must emit with no warning'); + + stderr = captureStderr(() => { + assert.strictEqual(resolveDispatchModelPin('agent-200', at200), at200); + }); + assert.strictEqual(stderr, '', '200 chars must emit with no warning'); + + stderr = captureStderr(() => { + assert.strictEqual(resolveDispatchModelPin('agent-201', at201), undefined); + }); + assert.match(stderr, /gsd: warning/); + assert.match(stderr, /exceeds the maximum model id length \(200 characters\)/); + }); + + test('item 2: an over-length pin is dropped, never truncated into a shorter value', () => { + // A truncated model id is a different model id — the resolver must + // never return a 200-char prefix of a 5000-char input. + const huge = 'a'.repeat(5000); + const result = captureStderr(() => resolveDispatchModelPin('agent-huge', huge)); + assert.match(result, /exceeds the maximum model id length/); + }); + + test('item 1 (parity): a rejected Vertex-shaped pin renders "@" correctly, not "?"', () => { + // Append a control byte so the value fails the charset test (and is + // therefore routed through the sanitizer) while still containing '@'. + const rawValue = 'text-bison@002' + String.fromCharCode(27); + const stderr = captureStderr(() => { + assert.strictEqual(resolveDispatchModelPin('agent-vertex', rawValue), undefined); + }); + assert.match(stderr, /"text-bison@002\?"/, 'the "@" must survive the sanitizer unchanged'); + assert.doesNotMatch(stderr, /text-bison\?002/, 'the "@" must never be sanitized to "?"'); + }); + + test('item 1 (parity, exhaustive): every character in the accept-class body survives the sanitizer unchanged', () => { + // Every printable character that MODEL_ID_CHARSET_RE accepts as a + // non-leading character must also survive MODEL_ID_SANITIZE_STRIP_RE + // unchanged — the two are derived from one shared body, so this can + // never regress silently again. + const acceptedNonLeadingChars = 'A-Za-z0-9._:/@-'; + // Expand the class body into a concrete character list (letters, digits, + // and the literal punctuation), independent of the regex-escaping used + // to define it, so the assertion doesn't just re-check the definition + // against itself. + const chars = []; + for (let c = 65; c <= 90; c++) chars.push(String.fromCharCode(c)); // A-Z + for (let c = 97; c <= 122; c++) chars.push(String.fromCharCode(c)); // a-z + for (let c = 48; c <= 57; c++) chars.push(String.fromCharCode(c)); // 0-9 + chars.push('.', '_', ':', '/', '@', '-'); + assert.ok(chars.length > 0); + + for (const ch of chars) { + const value = 'x' + ch; // 'x' keeps the leading-char anchor satisfied + assert.match(value, MODEL_ID_CHARSET_RE, `"${value}" should be accepted by MODEL_ID_CHARSET_RE`); + const sanitized = value.replace(MODEL_ID_SANITIZE_STRIP_RE, '?'); + assert.strictEqual(sanitized, value, `"${ch}" is accepted but was sanitized away`); + } + // Assert the literal class body above EQUALS the exported production + // value, so this test fails if gsd-tools.cjs's MODEL_ID_CHARSET_BODY is + // edited (e.g. widened) without updating this test's expectations. + assert.strictEqual( + acceptedNonLeadingChars, + MODEL_ID_CHARSET_BODY, + 'this test\'s expected charset has drifted from gsd-tools.cjs\'s MODEL_ID_CHARSET_BODY', + ); + }); + + test('item 1 (property): fast-check — any string accepted by MODEL_ID_CHARSET_RE is unchanged by the sanitizer', () => { + const bodyChar = fc.constantFrom( + ...'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789._:/@-'.split(''), + ); + const leadChar = fc.constantFrom( + ...'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'.split(''), + ); + fc.assert( + fc.property(leadChar, fc.array(bodyChar, { maxLength: 40 }), (lead, rest) => { + const value = lead + rest.join(''); + assert.match(value, MODEL_ID_CHARSET_RE); + const sanitized = value.replace(MODEL_ID_SANITIZE_STRIP_RE, '?'); + assert.strictEqual(sanitized, value); + }), + ); + }); + + test('every rejection path degrades to undefined (drop-and-warn), never throws', () => { + const inputs = [ + '@evil', + '/c', + '-c', + '--config', + 'a'.repeat(201), + 'sonnet', + String.fromCharCode(27) + 'malicious', + '', + ' ', + 'inherit', + 'INHERIT', + ]; + for (const input of inputs) { + assert.doesNotThrow(() => { + captureStderr(() => resolveDispatchModelPin('agent-x', input)); + }, `resolveDispatchModelPin must never throw for input ${JSON.stringify(input)}`); + } + }); +}); diff --git a/tests/host-integration.test.cjs b/tests/host-integration.test.cjs index c302eb09a..69a8655da 100644 --- a/tests/host-integration.test.cjs +++ b/tests/host-integration.test.cjs @@ -1853,6 +1853,810 @@ describe('#2584 orchestratorExec — validator', () => { }); }); +// --------------------------------------------------------------------------- +// #3714 — codex-worktree model pin. +// +// `resolveOrchestratorExec` currently takes NO `model` argument at all, so +// codex's argv never carries `--model` regardless of any configured +// model_overrides. THESE TESTS ARE FAILING-FIRST against today's code — do +// not "fix" resolveOrchestratorExec or gsd-tools.cjs to make them pass here; +// that is a separate change. See issue #3714. +// +// THE FIX THIS PINS (not yet implemented): +// 1. Descriptor gains an optional `modelFlag` (string|null). codex -> +// "--model"; every other shipped runtime leaves it absent/null. +// 2. resolveOrchestratorExec(orchestratorExec, cwd, prompt, model) gains an +// optional 4th positional `model`, appending [modelFlag, model] ONLY +// when modelFlag is a non-empty string AND model is a non-empty string. +// Argv order: baseArgs -> modelFlag,model -> cwdFlag,cwd -> prompt. The +// prompt remains the final positional token. Fails closed with +// 'unsafe_leading_dash_model' exactly like the existing +// unsafe_leading_dash_prompt / unsafe_leading_dash_cwd guards. +// 3. Policy (which model, if any, to pass) lives at the CALLER +// (bin/gsd-tools.cjs), via: +// resolveAgentModelOverride('gsd-executor', readGsdEffectiveModelOverrides(cwd), null) +// then dropping the 'inherit' sentinel. Passing null as the runtime +// resolver is what makes "no tier routing" structural (ADR-2313) — the +// seam itself decides no policy. +// --------------------------------------------------------------------------- + +describe('#3714 resolveOrchestratorExec — modelFlag/model seam (mechanical, RED pre-fix)', () => { + const CWD = '/repo/.claude/worktrees/agent-a1'; + const PROMPT = 'Execute plan 2 of phase 3.'; + const CODEX_MODEL_DESCRIPTOR = { command: 'codex', args: ['exec'], cwdFlag: '--cd', modelFlag: '--model' }; + const EXPECTED_ARGS_WITH_MODEL = ['exec', '--model', 'gpt-5.6-terra', '--cd', CWD, PROMPT]; + const EXPECTED_ARGS_NO_MODEL = ['exec', '--cd', CWD, PROMPT]; + + // MATRIX row 1 [FAIL]: explicit pin -> full argv identity, not includes(). + // Today's resolver ignores the 4th `model` argument entirely, so this + // produces ["exec","--cd",CWD,PROMPT] — no "--model" anywhere — and the + // deepEqual below is RED. + test('row 1: explicit model pin -> full argv is [baseArgs..., --model, , cwdFlag, cwd, prompt]', () => { + const result = resolveOrchestratorExec(CODEX_MODEL_DESCRIPTOR, CWD, PROMPT, 'gpt-5.6-terra'); + assert.equal(result.ok, true); + assert.deepEqual(result.args, EXPECTED_ARGS_WITH_MODEL, + 'today\'s resolver has no model parameter — it silently drops "gpt-5.6-terra" and emits no --model at all'); + }); + + // MATRIX row 6 [FAIL]: prompt is still the LAST element of args once a + // model is emitted. Pinned via the same full-array identity check as row 1 + // (a narrower args[len-1]===prompt check alone would pass today by + // coincidence, since nothing is ever inserted after the prompt either way). + test('row 6: when a model IS emitted, the prompt remains the LAST element of args', () => { + const result = resolveOrchestratorExec(CODEX_MODEL_DESCRIPTOR, CWD, PROMPT, 'gpt-5.6-terra'); + assert.equal(result.ok, true); + assert.deepEqual(result.args, EXPECTED_ARGS_WITH_MODEL); + assert.equal(result.args[result.args.length - 1], PROMPT); + }); + + // Boundary: modelFlag absent / null / "" -> no --model ever appears, + // regardless of a valid model value. All three CONTROL (pass today, by + // coincidence of today's total absence of model support — but this is also + // the documented post-fix contract, so these stay green after the fix). + test('modelFlag absent, model provided -> no --model on the wire', () => { + const result = resolveOrchestratorExec({ command: 'codex', args: ['exec'], cwdFlag: '--cd' }, CWD, PROMPT, 'gpt-5.6-terra'); + assert.equal(result.ok, true); + assert.deepEqual(result.args, EXPECTED_ARGS_NO_MODEL); + }); + + test('modelFlag null, model provided -> no --model on the wire', () => { + const result = resolveOrchestratorExec( + { command: 'codex', args: ['exec'], cwdFlag: '--cd', modelFlag: null }, CWD, PROMPT, 'gpt-5.6-terra', + ); + assert.equal(result.ok, true); + assert.deepEqual(result.args, EXPECTED_ARGS_NO_MODEL); + }); + + test('modelFlag "" (empty string), model provided -> no --model on the wire', () => { + const result = resolveOrchestratorExec( + { command: 'codex', args: ['exec'], cwdFlag: '--cd', modelFlag: '' }, CWD, PROMPT, 'gpt-5.6-terra', + ); + assert.equal(result.ok, true); + assert.deepEqual(result.args, EXPECTED_ARGS_NO_MODEL); + }); + + // Boundary: model absent / null / "" -> no --model ever appears, regardless + // of a valid modelFlag. All three CONTROL. + test('modelFlag present, model absent -> no --model on the wire', () => { + const result = resolveOrchestratorExec(CODEX_MODEL_DESCRIPTOR, CWD, PROMPT); + assert.equal(result.ok, true); + assert.deepEqual(result.args, EXPECTED_ARGS_NO_MODEL); + }); + + test('modelFlag present, model null -> no --model on the wire', () => { + const result = resolveOrchestratorExec(CODEX_MODEL_DESCRIPTOR, CWD, PROMPT, null); + assert.equal(result.ok, true); + assert.deepEqual(result.args, EXPECTED_ARGS_NO_MODEL); + }); + + test('modelFlag present, model "" (empty string) -> no --model on the wire', () => { + const result = resolveOrchestratorExec(CODEX_MODEL_DESCRIPTOR, CWD, PROMPT, ''); + assert.equal(result.ok, true); + assert.deepEqual(result.args, EXPECTED_ARGS_NO_MODEL); + }); + + // New fail-closed guard, mirroring unsafe_leading_dash_prompt/_cwd. RED + // today: the resolver has no model-validation branch at all, so a + // dash-leading model is simply ignored (ok:true) rather than rejected. + test('a dash-leading model is rejected: unsafe_leading_dash_model', () => { + for (const hostile of ['--dangerously-skip-permissions', '-p', '--help']) { + const result = resolveOrchestratorExec(CODEX_MODEL_DESCRIPTOR, CWD, PROMPT, hostile); + assert.equal(result.ok, false, `model=${hostile} must be rejected`); + assert.equal(result.reason, 'unsafe_leading_dash_model'); + } + }); + + test('a model merely CONTAINING a dash is fine — only a leading dash is a flag', () => { + const result = resolveOrchestratorExec(CODEX_MODEL_DESCRIPTOR, CWD, PROMPT, 'gpt-5.6-terra'); + assert.equal(result.ok, true); + assert.ok(result.args.includes('gpt-5.6-terra')); + }); + + // Every existing fail-closed guard must still fire, unaffected by a valid + // model argument riding alongside. CONTROL — these guards run before any + // model logic regardless of whether the model param exists yet. + test('existing fail-closed guards still fire with a model present', () => { + assert.equal(resolveOrchestratorExec(undefined, CWD, PROMPT, 'm').reason, 'missing_command'); + assert.equal(resolveOrchestratorExec({}, CWD, PROMPT, 'm').reason, 'missing_command'); + assert.equal(resolveOrchestratorExec({ command: 'codex' }, '', PROMPT, 'm').reason, 'invalid_cwd'); + assert.equal(resolveOrchestratorExec({ command: 'codex', args: 'exec' }, CWD, PROMPT, 'm').reason, 'invalid_args'); + assert.equal(resolveOrchestratorExec({ command: 'codex' }, CWD, '', 'm').reason, 'invalid_prompt'); + assert.equal( + resolveOrchestratorExec({ command: 'codex', cwdFlag: '--cd' }, '-oProxyCommand=x', PROMPT, 'm').reason, + 'unsafe_leading_dash_cwd', + ); + assert.equal( + resolveOrchestratorExec({ command: 'codex', cwdFlag: '--cd' }, CWD, '-p', 'm').reason, + 'unsafe_leading_dash_prompt', + ); + }); + + // ITEM 3: `invalid_model` is live for any non-string, non-null, non-undefined + // `model` argument — a caller error (number/bool/array/object), distinct + // from the benign "use the host default" degradation that null/undefined/'' + // already exercise. Previously zero test references (Stryker-visible gap). + test('ITEM 3: a non-string model (number, boolean, array, object) -> {ok:false, reason:"invalid_model"}', () => { + for (const bogus of [7, true, [], {}]) { + const result = resolveOrchestratorExec({ command: 'codex', modelFlag: '--model' }, CWD, PROMPT, bogus); + assert.deepEqual(result, { ok: false, reason: 'invalid_model' }, + `model=${JSON.stringify(bogus)} must take the invalid_model branch`); + } + }); + + // ITEM 3 CONTROL: null/undefined/'' must NOT take the invalid_model branch — + // they degrade to "omit the flag" and the resolution still succeeds. + test('ITEM 3 CONTROL: null/undefined/\'\' do NOT take invalid_model — they omit the flag and succeed', () => { + for (const benign of [null, undefined, '']) { + const result = resolveOrchestratorExec({ command: 'codex', modelFlag: '--model' }, CWD, PROMPT, benign); + assert.equal(result.ok, true, `model=${JSON.stringify(benign)} must resolve ok:true`); + assert.ok(!result.args.includes('--model'), `model=${JSON.stringify(benign)} must omit the --model flag`); + } + }); + + // MATRIX row 7 [CONTROL]: a host with no modelFlag is byte-identical to + // today whether or not a model is passed — kimi-code/opencode never get a + // 5th positional token nor a flag pair injected. + test('row 7 CONTROL: a host descriptor with NO modelFlag key resolves identically whether or not a model is passed', () => { + const descriptor = { command: 'kimi', args: ['--print'], cwdFlag: '--work-dir', promptFlag: '--prompt' }; + const withoutModelArg = resolveOrchestratorExec(descriptor, CWD, PROMPT); + const withModelArg = resolveOrchestratorExec(descriptor, CWD, PROMPT, 'some-model'); + assert.deepEqual(withModelArg, withoutModelArg, + 'a descriptor with no modelFlag must resolve identically whether or not a model is passed'); + assert.deepEqual(withoutModelArg.args, ['--print', '--work-dir', CWD, '--prompt', PROMPT]); + }); + + test('property: for any descriptor and any model input, when a prompt is supplied it is always args[args.length-1] (seed=3714)', () => { + const commandArb = fc.string({ minLength: 1 }).filter((s) => s.length > 0); + const argsArb = fc.array(fc.string()); + const cwdArb = fc.string({ minLength: 1 }).filter((s) => s.length > 0 && !s.startsWith('-')); + const flagArb = fc.oneof( + fc.constant(undefined), fc.constant(null), fc.constant(''), + fc.string({ minLength: 1 }).filter((s) => s.length > 0 && !s.startsWith('-')), + ); + const modelArb = fc.oneof( + fc.constant(undefined), fc.constant(null), fc.constant(''), + fc.string({ minLength: 1 }).filter((s) => s.length > 0 && !s.startsWith('-')), + ); + const promptArb = fc.string({ minLength: 1 }).filter((s) => s.length > 0 && !s.startsWith('-')); + + fc.assert( + fc.property(commandArb, argsArb, cwdArb, flagArb, modelArb, promptArb, + (command, args, cwd, modelFlag, model, prompt) => { + fc.pre(!args.includes(cwd) && !args.includes(prompt)); + const descriptor = modelFlag === undefined ? { command, args } : { command, args, modelFlag }; + const result = resolveOrchestratorExec(descriptor, cwd, prompt, model); + assert.equal(result.ok, true); + assert.equal(result.args[result.args.length - 1], prompt); + }), + { numRuns: 200, seed: 3714 }, + ); + }); +}); + +// --------------------------------------------------------------------------- +// #3714 — end-to-end policy: the caller (bin/gsd-tools.cjs) decides WHETHER +// to pass a model at all, via +// resolveAgentModelOverride('gsd-executor', readGsdEffectiveModelOverrides(cwd), null) +// then dropping the 'inherit' sentinel. Every row below is a MEASURED FACT +// (verified by direct execution against this repo's current code) about what +// that function returns for a given .planning/config.json shape — these +// describe the POLICY the fix must implement, not a currently-wired +// behavior: `query dispatch-isolation` never emits --model today for ANY +// config shape, so only the "explicit pin" rows are RED; the rest coincide +// with today's (absent) behavior and are CONTROLS. +// --------------------------------------------------------------------------- +describe('#3714 dispatch-isolation CLI — model policy end-to-end (RED pre-fix on explicit-pin rows)', () => { + const { runNode } = require('./helpers/process-seam.cjs'); + const { throwIfFailed } = require('./helpers/git-fixture.cjs'); + const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + const { createTempProject, cleanup, TEST_HOME_SANDBOX_MARKER } = require('./helpers.cjs'); + const os = require('node:os'); + const GSD_TOOLS = path.join(REPO_ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs'); + + function writeConfig(projectDir, config) { + fs.writeFileSync( + path.join(projectDir, '.planning', 'config.json'), + JSON.stringify(config), + ); + } + + // Hermeticity: `queryCodexJson` used to inherit the DEVELOPER's real + // HOME/USERPROFILE with no override, so any row that writes no + // `model_overrides` key at all was silently reading (and could red + // against) the operator's own ~/.gsd/defaults.json — exactly the surface + // the BLOCKER regression test below needs to control precisely. Every + // call now gets a fresh, per-call temp HOME (removed synchronously after + // the CLI returns, since the call is a blocking spawn). USERPROFILE is + // set alongside HOME because os.homedir() reads USERPROFILE on Windows; + // omitting it would make the isolation vacuous there. The sandbox marker + // satisfies the same passwd-less-host fallback installSpawnEnv documents. + // `beforeSpawn`, when provided, is called with the sandbox HOME dir path + // BEFORE the CLI spawns — the seam a caller needs to seed a GLOBAL + // ~/.gsd/defaults.json (see writeGlobalDefaults below) for a + // global-only-pin row. + function queryCodexJson(projectDir, extraEnv = {}, beforeSpawn = null) { + const sandboxHomeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3714-home-')); + try { + if (typeof beforeSpawn === 'function') beforeSpawn(sandboxHomeDir); + const r = runNode( + [GSD_TOOLS, 'query', 'dispatch-isolation', '--json', '--cwd-target', '/tmp/wt', '--prompt', 'do the thing'], + { + cwd: projectDir, + env: { + ...process.env, + GSD_RUNTIME: 'codex', + HOME: sandboxHomeDir, + USERPROFILE: sandboxHomeDir, + [TEST_HOME_SANDBOX_MARKER]: sandboxHomeDir, + ...extraEnv, + }, + timeoutMs: PROBE_TIMEOUT_MS, + }, + ); + throwIfFailed(r, 'gsd-tools query dispatch-isolation --json (codex, model policy)'); + return { json: JSON.parse(r.stdout), stderr: r.stderr }; + } finally { + cleanup(sandboxHomeDir); + } + } + + // Writes ~/.gsd/defaults.json (the GLOBAL model_overrides store) into the + // per-call sandbox HOME so a test can exercise "global-only pin, no + // per-project override" — the exact shape the BLOCKER describes. + function writeGlobalDefaults(homeDir, defaults) { + const gsdDir = path.join(homeDir, '.gsd'); + fs.mkdirSync(gsdDir, { recursive: true }); + fs.writeFileSync(path.join(gsdDir, 'defaults.json'), JSON.stringify(defaults)); + } + + function hasModelFlag(execArgs) { + return execArgs.includes('--model'); + } + + // MATRIX row 1: an explicit real-Codex gsd-executor override reaches argv + // as --model. + test('row 1: explicit model_overrides["gsd-executor"] -> exec.args contains ["--model","gpt-5.6-terra"] at the correct position', () => { + const dir = createTempProject('gsd-3714-row1-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': 'gpt-5.6-terra' } }); + const { json: result } = queryCodexJson(dir); + assert.equal(result.isolation, 'orchestrator-worktree'); + assert.deepEqual(result.exec.args, ['exec', '--model', 'gpt-5.6-terra', '--cd', '/tmp/wt', 'do the thing']); + } finally { + cleanup(dir); + } + }); + + // MATRIX row 2 [CONTROL]: no override configured -> no --model, and in + // particular the resolve-model tier value ("sonnet") must NEVER leak onto + // codex's argv (that is the #2310/#2311 400 ADR-2313 exists to prevent). + test('row 2 CONTROL: no override -> exec.args contains NO --model and no "sonnet" anywhere', () => { + const dir = createTempProject('gsd-3714-row2-'); + try { + writeConfig(dir, {}); + const { json: result } = queryCodexJson(dir); + assert.equal(result.isolation, 'orchestrator-worktree'); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + assert.ok(!result.exec.args.join(' ').includes('sonnet')); + } finally { + cleanup(dir); + } + }); + + // MATRIX row 3 [CONTROL]: the 'inherit' sentinel must never reach argv. + test('row 3 CONTROL: model_overrides["gsd-executor"] === "inherit" -> no --model (sentinel never on the wire)', () => { + const dir = createTempProject('gsd-3714-row3-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': 'inherit' } }); + const { json: result } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + assert.ok(!result.exec.args.join(' ').includes('inherit')); + } finally { + cleanup(dir); + } + }); + + // MATRIX row 4 [CONTROL]: an empty-string override resolves to null, same as no override. + test('row 4 CONTROL: model_overrides["gsd-executor"] === "" -> no --model', () => { + const dir = createTempProject('gsd-3714-row4-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': '' } }); + const { json: result } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + } finally { + cleanup(dir); + } + }); + + // MATRIX row 5 [CONTROL]: a model_profile alone (no per-agent override) must + // NOT route through the tier table onto codex's argv — ADR-2313 forbids + // tier routing to codex entirely; passing `null` as the runtimeResolver + // (per the fix design) is what makes this structural rather than incidental. + test('row 5 CONTROL: model_profile:"balanced" only (no per-agent override) -> no --model (tier routing forbidden, ADR-2313)', () => { + const dir = createTempProject('gsd-3714-row5-'); + try { + writeConfig(dir, { runtime: 'codex', model_profile: 'balanced' }); + const { json: result } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + assert.ok(!result.exec.args.join(' ').includes('sonnet')); + } finally { + cleanup(dir); + } + }); + + // BLOCKER regression test: a GLOBAL (~/.gsd/defaults.json) Anthropic-flavored + // pin must NOT reach codex's argv, and must produce a stderr warning. Before + // this fix, presence-only gating let this straight through to + // `codex exec --model sonnet` — the documented #2310/#2311 400. + test('BLOCKER: global-only model_overrides["gsd-executor"]="sonnet" -> NO --model, and a stderr warning', () => { + const dir = createTempProject('gsd-3714-blocker-global-anthropic-'); + try { + writeConfig(dir, {}); + const { json: result, stderr } = queryCodexJson(dir, {}, (homeDir) => { + writeGlobalDefaults(homeDir, { model_overrides: { 'gsd-executor': 'sonnet' } }); + }); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + assert.ok(!result.exec.args.join(' ').includes('sonnet')); + assert.match(stderr, /gsd-executor.*sonnet.*Anthropic/i); + } finally { + cleanup(dir); + } + }); + + test('global-only model_overrides["gsd-executor"]="gpt-5.6-terra" (real Codex pin) -> IS emitted', () => { + const dir = createTempProject('gsd-3714-global-real-'); + try { + writeConfig(dir, {}); + const { json: result, stderr } = queryCodexJson(dir, {}, (homeDir) => { + writeGlobalDefaults(homeDir, { model_overrides: { 'gsd-executor': 'gpt-5.6-terra' } }); + }); + assert.deepEqual(result.exec.args, ['exec', '--model', 'gpt-5.6-terra', '--cd', '/tmp/wt', 'do the thing']); + assert.equal(stderr, ''); + } finally { + cleanup(dir); + } + }); + + test('whitespace-only override " " -> no --model, no warning', () => { + const dir = createTempProject('gsd-3714-ws-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': ' ' } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.equal(stderr, ''); + } finally { + cleanup(dir); + } + }); + + test('"Inherit" and " inherit " (case/whitespace-insensitive sentinel) -> no --model', () => { + for (const value of ['Inherit', ' inherit ']) { + const dir = createTempProject('gsd-3714-inherit-ci-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': value } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing'], `value=${JSON.stringify(value)}`); + assert.equal(stderr, ''); + } finally { + cleanup(dir); + } + } + }); + + test('injection-shaped override values -> no --model, and a stderr warning', () => { + const injectionValues = [ + 'gpt-5 -c approval_policy=never', + 'gpt-5$(touch /tmp/x)', + 'gpt-5; touch /tmp/x', + 'gpt-5\nHOST_INJECTED', + 'gpt-5HOST_INJECTED', + ]; + for (const value of injectionValues) { + const dir = createTempProject('gsd-3714-injection-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': value } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing'], + `value=${JSON.stringify(value)} must never reach argv`); + assert.ok(!hasModelFlag(result.exec.args)); + assert.match(stderr, /gsd-executor/, `value=${JSON.stringify(value)} must warn`); + } finally { + cleanup(dir); + } + } + }); + + test('legitimate real-Codex ids survive: "gpt-5.6-terra" and "synthetic/hf:zai-org/GLM-5.2"', () => { + for (const value of ['gpt-5.6-terra', 'synthetic/hf:zai-org/GLM-5.2']) { + const dir = createTempProject('gsd-3714-legit-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': value } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--model', value, '--cd', '/tmp/wt', 'do the thing']); + assert.equal(stderr, ''); + } finally { + cleanup(dir); + } + } + }); + + // ITEM 1 follow-up: MODEL_ID_CHARSET_RE previously excluded '@', so a real + // Vertex model-version pin ("text-bison@002") was dropped as if it were an + // injection-shaped value. '@' is now permitted. The leading-dash row is + // kept adjacent so the anchor LEADING_DASH_RE still enforces is visibly + // still live even after widening the charset. + test('ITEM 1: "text-bison@002" (Vertex model-version pin) survives — "@" is a legitimate model-id character', () => { + const dir = createTempProject('gsd-3714-vertex-at-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': 'text-bison@002' } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--model', 'text-bison@002', '--cd', '/tmp/wt', 'do the thing']); + assert.equal(stderr, ''); + } finally { + cleanup(dir); + } + }); + + test('ITEM 1 anchor control: a leading dash still drops even though "@" is now permitted ("-@bad" -> no --model, a warning)', () => { + const dir = createTempProject('gsd-3714-vertex-at-leading-dash-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': '-@bad' } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + assert.match(stderr, /gsd-executor/); + } finally { + cleanup(dir); + } + }); + + // MATRIX row 8: the dispatch predicate (what gsd-tools.cjs's caller decides + // to pass) must agree, on every config shape below, with what the shared + // resolveAgentModelOverride('gsd-executor', overrides, null) function + // returns (dropping only the 'inherit' sentinel). This row is a resolver + // TAUTOLOGY — it agrees with the presence-only predicate and does not + // exercise the VALUE policy (Anthropic-flavored / charset) at all. It is + // kept as a presence-level regression guard; the real divergence guard is + // the CROSS-SURFACE PARITY test below. + test('row 8: dispatch predicate agrees with resolveAgentModelOverride(..., null) on every config shape', () => { + const installModelOverrideResolver = require('../gsd-core/bin/lib/install-model-override-resolver.cjs'); + const shapes = [ + { name: 'explicit pin', config: { model_overrides: { 'gsd-executor': 'gpt-5.6-terra' } } }, + { name: 'no override', config: {} }, + { name: 'override "inherit"', config: { model_overrides: { 'gsd-executor': 'inherit' } } }, + { name: 'override ""', config: { model_overrides: { 'gsd-executor': '' } } }, + { name: 'model_profile only', config: { runtime: 'codex', model_profile: 'balanced' } }, + ]; + for (const shape of shapes) { + const dir = createTempProject('gsd-3714-row8-'); + try { + writeConfig(dir, shape.config); + // The predicate the fix's caller must implement: explicit override, + // no runtime-tier resolver (null), with 'inherit' dropped. + const overrides = installModelOverrideResolver.readGsdEffectiveModelOverrides(dir); + const resolved = installModelOverrideResolver.resolveAgentModelOverride('gsd-executor', overrides, null); + const expectedModel = (resolved && resolved !== 'inherit') ? resolved : null; + const expectedHasModel = expectedModel !== null; + + const { json: result } = queryCodexJson(dir); + const actualHasModel = hasModelFlag(result.exec.args); + + assert.equal(actualHasModel, expectedHasModel, + `shape="${shape.name}": predicate says emit-model=${expectedHasModel} but actual argv ` + + `${expectedHasModel ? 'never carries' : 'unexpectedly carries'} --model (args=${JSON.stringify(result.exec.args)})`); + if (expectedHasModel) { + assert.deepEqual(result.exec.args, ['exec', '--model', expectedModel, '--cd', '/tmp/wt', 'do the thing']); + } + } finally { + cleanup(dir); + } + } + }); + + // DIVERGENCE GUARD — real cross-surface parity test. For each value below, + // assert that dispatch (this describe's CLI, driven for real end-to-end) + // and the install-side .toml policy (bin/install.js generateCodexAgentToml, + // NOT reachable in-process here — it lives in the generated installer + // module and is exercised only via `npm run build` / the install test + // suite) would reach the SAME emit-vs-drop decision for the identical + // model_overrides["gsd-executor"] value. + // + // What is asserted DIRECTLY (real code path, in-process): the dispatch + // side, via the real `gsd-tools query dispatch-isolation` CLI spawn. + // What is MIRRORED (not independently re-executed): the install-side half + // is derived from `isAnthropicFlavoredModel` (the single-sourced #3241 + // predicate, imported for real from bin/lib/model-catalog.cjs — so THAT + // predicate call is real, not re-implemented) plus the documented + // install-side rules from bin/install.js's generateCodexAgentToml + // (trim; drop empty/whitespace-only; drop Anthropic-flavored). Install-side + // does NOT apply a charset check — that is dispatch-only, added because + // dispatch crosses a shell-argv boundary that a static .toml string never + // does. A future reader: if bin/install.js's trim/drop rules for this key + // change without a matching update here, this comment is the thing that + // goes stale, not a shared executable — that is the acknowledged limit. + test('DIVERGENCE GUARD: dispatch model-pin decision matches install-side Codex .toml policy for the same value', () => { + const { isAnthropicFlavoredModel } = require('../gsd-core/bin/lib/model-catalog.cjs'); + function installSideWouldEmit(rawValue) { + if (typeof rawValue !== 'string') return false; + const trimmed = rawValue.trim(); + if (trimmed === '') return false; + if (isAnthropicFlavoredModel(trimmed)) return false; + return true; + } + const table = [ + 'gpt-5.6-terra', + 'synthetic/hf:zai-org/GLM-5.2', + 'sonnet', + 'opus', + 'claude-sonnet-4-5', + '', + ' ', + 'inherit', + 'Inherit', + '-p', + 'gpt-5 -c approval_policy=never', + ]; + for (const value of table) { + const dir = createTempProject('gsd-3714-parity-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': value } }); + const { json: result } = queryCodexJson(dir); + const dispatchEmitted = hasModelFlag(result.exec.args); + // Dispatch additionally drops 'inherit' (case/whitespace-insensitive) + // and non-model-id-charset values — neither is an install-side .toml + // concern (install never sees the literal string "inherit" as a + // meaningful sentinel, and a static TOML string is not a shell argv + // boundary), so those two are excluded from the parity assertion + // itself and asserted directly instead. + const trimmedLower = typeof value === 'string' ? value.trim().toLowerCase() : value; + if (trimmedLower === 'inherit') { + assert.equal(dispatchEmitted, false, `value=${JSON.stringify(value)}: inherit sentinel must never emit`); + continue; + } + if (value === '-p' || value === 'gpt-5 -c approval_policy=never') { + assert.equal(dispatchEmitted, false, `value=${JSON.stringify(value)}: unsafe-charset value must never emit`); + continue; + } + assert.equal(dispatchEmitted, installSideWouldEmit(value), + `value=${JSON.stringify(value)}: dispatch emit=${dispatchEmitted} but install-side policy says emit=${installSideWouldEmit(value)}`); + } finally { + cleanup(dir); + } + } + }); + + // --------------------------------------------------------------------------- + // Round-2 review regression rows — three real defects reproduced against + // this repo's actual CLI (see the fix commit for the full repro transcript). + // --------------------------------------------------------------------------- + + // DEFECT 1: a leading-dash pin ('-c', '--config', '-', '--') previously + // passed MODEL_ID_CHARSET_RE silently (it permits '-'), reached + // resolveOrchestratorExec, tripped its `unsafe_leading_dash_model` guard, + // and turned the WHOLE resolution to exec:null — a wave-fatal abort per + // executor-isolation-dispatch.md:299-303, with no warning at all. The fix + // rejects a leading '-' inside resolveDispatchModelPin itself so it + // degrades like every other rejected shape: no --model, a warning, exec + // still resolves. + test('DEFECT 1: leading-dash pins ("-c", "--config", "-", "--") -> no --model, a warning, exec NOT null (argv == no-model argv)', () => { + const leadingDashValues = ['-c', '--config', '-', '--']; + for (const value of leadingDashValues) { + const dir = createTempProject('gsd-3714-leading-dash-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': value } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.notEqual(result.exec, null, `value=${JSON.stringify(value)}: exec must not be null`); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing'], + `value=${JSON.stringify(value)}: argv must equal the no-model argv exactly`); + assert.ok(!hasModelFlag(result.exec.args)); + assert.match(stderr, /gsd-executor/, `value=${JSON.stringify(value)} must warn`); + } finally { + cleanup(dir); + } + } + }); + + // DEFECT 1 REGRESSION ROW: the same '-c' pin under a host with NO + // modelFlag at all (kimi-code) must resolve byte-identical argv to the + // no-pin case — before the fix, resolveOrchestratorExec's leading-dash + // guard fires BEFORE the modelFlag presence check, so this host (which + // previously ignored the model entirely) was newly broken by the pin + // policy. The pin policy (and its warning) is gated on the descriptor + // declaring a non-empty `modelFlag`; kimi-code declares none, so this must + // now produce NO stderr warning either (item 2 follow-up) — the value + // policy never runs at all for a host that was never going to emit + // --model. + test('DEFECT 1 REGRESSION: "-c" pin under kimi-code (no modelFlag host) -> argv byte-identical to no-pin, exec NOT null, stderr EMPTY', () => { + const noPinDir = createTempProject('gsd-3714-kimi-nopin-'); + const pinnedDir = createTempProject('gsd-3714-kimi-pinned-'); + try { + writeConfig(noPinDir, {}); + writeConfig(pinnedDir, { model_overrides: { 'gsd-executor': '-c' } }); + const { json: noPinResult } = queryCodexJson(noPinDir, { GSD_RUNTIME: 'kimi-code' }); + const { json: pinnedResult, stderr } = queryCodexJson(pinnedDir, { GSD_RUNTIME: 'kimi-code' }); + assert.notEqual(noPinResult.exec, null, 'kimi-code no-pin: exec must not be null'); + assert.notEqual(pinnedResult.exec, null, 'kimi-code "-c" pin: exec must not be null (this is the regression)'); + assert.deepEqual(pinnedResult.exec.args, noPinResult.exec.args, + 'kimi-code argv with a "-c" pin must be byte-identical to the no-pin argv'); + assert.equal(stderr, '', 'a host with no modelFlag must never run the pin policy, so no warning'); + } finally { + cleanup(noPinDir); + cleanup(pinnedDir); + } + }); + + // DEFECT 2: isAnthropicFlavoredModel lowercased its substring arm but not + // its CLAUDE_AGENT_ALIASES.has(...) arm, so a case variant of a bare alias + // ("Sonnet", "OPUS") or a mixed-case "claude-*" id ("Claude-Sonnet-4-5") + // slipped through as if it were a real Codex model id. Lowercase 'sonnet' + // is the control (already correctly dropped pre-fix). + // NOTE: "opus-4.1" is deliberately excluded from this table. It matches + // neither arm of isAnthropicFlavoredModel by DESIGN, independent of case: + // CLAUDE_AGENT_ALIASES holds only the bare tier names ('opus'/'sonnet'/ + // 'haiku'/'fable'), never version-qualified forms, and "opus-4.1" contains + // no "claude" substring (every real catalog Anthropic id is "claude-*"). + // This is a pre-existing predicate-design gap, not the case-sensitivity + // defect fixed here — flagged separately rather than asserted as fixed + // behavior in this test. + test('DEFECT 2: case variants of Anthropic-flavored pins ("Sonnet", "OPUS", "Claude-Sonnet-4-5") -> no --model + warning', () => { + const caseVariants = ['Sonnet', 'OPUS', 'Claude-Sonnet-4-5']; + for (const value of caseVariants) { + const dir = createTempProject('gsd-3714-case-flavor-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': value } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing'], + `value=${JSON.stringify(value)} must never reach argv`); + assert.ok(!hasModelFlag(result.exec.args)); + assert.match(stderr, /gsd-executor/, `value=${JSON.stringify(value)} must warn`); + } finally { + cleanup(dir); + } + } + }); + + test('DEFECT 2 CONTROL: lowercase "sonnet" still drops (unchanged behavior)', () => { + const dir = createTempProject('gsd-3714-case-flavor-control-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': 'sonnet' } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + assert.match(stderr, /gsd-executor/); + } finally { + cleanup(dir); + } + }); + + // DEFECT 3: _warnDispatchModelPinDropped wrote the rejected raw value to + // stderr, truncated but never escaped — a guaranteed-reachable raw-to-TTY + // sink for control/escape bytes, since every value reaching this warning + // failed the charset test by definition. Built with String.fromCharCode so + // the test source itself carries no literal control characters. + test('DEFECT 3: an ESC/BEL-bearing pin is dropped AND the stderr warning contains no raw control character', () => { + const ESC = String.fromCharCode(27); + const BEL = String.fromCharCode(7); + const hostileValue = `x${ESC}]0;PWNED${BEL}y`; + const dir = createTempProject('gsd-3714-defect3-hostile-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': hostileValue } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + assert.match(stderr, /gsd-executor/); + const stderrBody = stderr.endsWith('\n') ? stderr.slice(0, -1) : stderr; + // eslint-disable-next-line no-control-regex -- asserting the ABSENCE of raw control bytes is the point of this test + assert.doesNotMatch(stderrBody, /[\x00-\x1f\x7f]/, + 'stderr must contain no raw control character outside the trailing newline'); + } finally { + cleanup(dir); + } + }); + + // DEFECT 3 (truncation ordering): a long escape-bearing value must be + // sanitized BEFORE truncation, so a truncated escape sequence can never + // survive into the emitted warning (e.g. an SGR sequence cut before its + // reset, leaving sticky terminal state). + test('DEFECT 3: a long ESC-bearing pin (> 64 chars) is sanitized before truncation — no raw control survives', () => { + const ESC = String.fromCharCode(27); + const hostileValue = `${'x'.repeat(80)}${ESC}[31mHOSTILE`; + const dir = createTempProject('gsd-3714-defect3-long-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': hostileValue } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + const stderrBody = stderr.endsWith('\n') ? stderr.slice(0, -1) : stderr; + // eslint-disable-next-line no-control-regex -- asserting the ABSENCE of raw control bytes is the point of this test + assert.doesNotMatch(stderrBody, /[\x00-\x1f\x7f]/, + 'a truncated escape sequence must never survive into the emitted warning'); + } finally { + cleanup(dir); + } + }); + + // --------------------------------------------------------------------------- + // ITEM 2 follow-up — the pin policy (and its warning) is HOST-NEUTRAL code + // running at a site shared by every runtime, so it previously ran (and + // warned) even for hosts whose descriptor declares no `modelFlag` at all + // (opencode, kimi, kimi-code) — none of which were ever going to emit a + // --model regardless of the pin's value. The fix gates the whole policy on + // the resolved runtime's orchestratorExec declaring a non-empty modelFlag. + // --------------------------------------------------------------------------- + test('ITEM 2: a "sonnet" pin under kimi-code -> argv byte-identical to no-pin, stderr EMPTY (no modelFlag declared)', () => { + const noPinDir = createTempProject('gsd-3714-item2-kimicode-nopin-'); + const pinnedDir = createTempProject('gsd-3714-item2-kimicode-pinned-'); + try { + writeConfig(noPinDir, {}); + writeConfig(pinnedDir, { model_overrides: { 'gsd-executor': 'sonnet' } }); + const { json: noPinResult, stderr: noPinStderr } = queryCodexJson(noPinDir, { GSD_RUNTIME: 'kimi-code' }); + const { json: pinnedResult, stderr: pinnedStderr } = queryCodexJson(pinnedDir, { GSD_RUNTIME: 'kimi-code' }); + assert.deepEqual(pinnedResult.exec.args, noPinResult.exec.args, + 'kimi-code argv with a "sonnet" pin must be byte-identical to the no-pin argv'); + assert.equal(noPinStderr, ''); + assert.equal(pinnedStderr, '', 'kimi-code declares no modelFlag — the pin policy must not run at all, so no warning'); + } finally { + cleanup(noPinDir); + cleanup(pinnedDir); + } + }); + + test('ITEM 2: a "sonnet" pin under opencode -> argv byte-identical to no-pin, stderr EMPTY (no modelFlag declared)', () => { + const noPinDir = createTempProject('gsd-3714-item2-opencode-nopin-'); + const pinnedDir = createTempProject('gsd-3714-item2-opencode-pinned-'); + try { + writeConfig(noPinDir, {}); + writeConfig(pinnedDir, { model_overrides: { 'gsd-executor': 'sonnet' } }); + const { json: noPinResult, stderr: noPinStderr } = queryCodexJson(noPinDir, { GSD_RUNTIME: 'opencode' }); + const { json: pinnedResult, stderr: pinnedStderr } = queryCodexJson(pinnedDir, { GSD_RUNTIME: 'opencode' }); + assert.deepEqual(pinnedResult.exec.args, noPinResult.exec.args, + 'opencode argv with a "sonnet" pin must be byte-identical to the no-pin argv'); + assert.equal(noPinStderr, ''); + assert.equal(pinnedStderr, '', 'opencode declares no modelFlag — the pin policy must not run at all, so no warning'); + } finally { + cleanup(noPinDir); + cleanup(pinnedDir); + } + }); + + test('ITEM 2 CONTROL: a "sonnet" pin under codex (declares modelFlag) -> still dropped, WITH the warning', () => { + const dir = createTempProject('gsd-3714-item2-codex-control-'); + try { + writeConfig(dir, { model_overrides: { 'gsd-executor': 'sonnet' } }); + const { json: result, stderr } = queryCodexJson(dir); + assert.deepEqual(result.exec.args, ['exec', '--cd', '/tmp/wt', 'do the thing']); + assert.ok(!hasModelFlag(result.exec.args)); + assert.match(stderr, /gsd-executor.*sonnet.*Anthropic/i); + } finally { + cleanup(dir); + } + }); +}); + // --------------------------------------------------------------------------- // #2627 Phase 3 — the `dispatch-isolation` CLI route. //