diff --git a/.changeset/calm-bears-sing.md b/.changeset/calm-bears-sing.md new file mode 100644 index 000000000..bb25e3574 --- /dev/null +++ b/.changeset/calm-bears-sing.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3539 +--- +**`effort.routing_tier_defaults` now merges over the built-in tier defaults instead of replacing them** — previously, creating an `effort` block without `routing_tier_defaults` silently disabled the built-in tier ladder (light:low / standard:high / heavy:xhigh), collapsing every non-overridden agent to `high`; one `agent_overrides` entry could reshape 20+ agents you never named. A partial block now fills gaps from the built-ins, and an invalid value falls back to that tier's built-in. (#3531) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 6c6689124..24e241312 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -1384,7 +1384,10 @@ The model-catalog's `reasoning_effort` per-tier hint is a legacy field kept for **Precedence (highest → lowest):** 1. Invocation override (e.g. `--effort` flag on `resolve-execution`) 2. `effort.agent_overrides[]` -3. `effort.routing_tier_defaults[]` +3. `effort.routing_tier_defaults[]`, **merged per-tier over the + built-in tier defaults** (`light: low`, `standard: high`, `heavy: xhigh`) — a partial + block fills its gaps from the built-ins instead of discarding them, and an invalid + value falls back to that tier's built-in ([#3531](https://github.com/open-gsd/gsd-core/issues/3531)) 4. `effort.default` 5. `"high"` (Anthropic Opus 4.8 universal default) diff --git a/src/install-effort-resolver.cts b/src/install-effort-resolver.cts index 3b9690a8f..2439ddde9 100644 --- a/src/install-effort-resolver.cts +++ b/src/install-effort-resolver.cts @@ -23,6 +23,9 @@ import os from 'node:os'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- model-resolver.cjs is an export= CommonJS module import modelResolver = require('./model-resolver.cjs'); const { EFFORT_SET: GSD_EFFORT_SET } = modelResolver as { EFFORT_SET: Set }; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- model-catalog.cjs is an export= CommonJS module +import modelCatalog = require('./model-catalog.cjs'); +const { mergeEffortTierDefaults } = modelCatalog as { mergeEffortTierDefaults: typeof import('./model-catalog.cjs').mergeEffortTierDefaults }; interface EffortConfig { agent_overrides?: Record; @@ -161,6 +164,11 @@ function readGsdEffectiveEffortConfig(targetDir: string | null = null): EffortCo // Per-project wins on conflict within each sub-field. Merge field-by-field so // a project config that only sets agent_overrides still inherits global // routing_tier_defaults and default. + // #3531 (10c): routing_tier_defaults is deep-merged per-tier like + // agent_overrides — a project block naming only `heavy` must not discard the + // home block's `light`/`standard` entries (the top-level spread would + // otherwise replace the whole block, the same defect class as the + // manifest-replacement this change fixes). return { ...(homeEffort || {}), ...(projectEffort || {}), @@ -169,6 +177,11 @@ function readGsdEffectiveEffortConfig(targetDir: string | null = null): EffortCo ...((homeEffort && homeEffort.agent_overrides) || {}), ...((projectEffort && projectEffort.agent_overrides) || {}), }, + // Deep-merge routing_tier_defaults (project wins per-tier) + routing_tier_defaults: { + ...((homeEffort && homeEffort.routing_tier_defaults) || {}), + ...((projectEffort && projectEffort.routing_tier_defaults) || {}), + }, }; } @@ -179,8 +192,10 @@ function readGsdEffectiveEffortConfig(targetDir: string | null = null): EffortCo * * Precedence (mirrors resolveEffortInternal): * 1. effortCfg.agent_overrides[agentName] - * 2. effortCfg.routing_tier_defaults[agentTier] (if effortCfg present) - * — OR manifest tier defaults when effortCfg is null + * 2. routing_tier_defaults merged over the manifest tier defaults + * (#3531/10c: a config block — present or partial — no longer disables + * the built-in tier ladder; invalid config values are dropped by the + * merge so the manifest value for the tier surfaces) * 3. effortCfg.default * 4. 'high' (hardcoded fallback) * @@ -207,17 +222,14 @@ function resolveInstallTimeEffort(effortCfg: EffortConfig | null, agentName: str const { AGENT_DEFAULT_TIERS, EFFORT_MANIFEST_TIER_DEFAULTS, EFFORT_MANIFEST_DEFAULT } = _getGsdEffortCatalog(); const agentTier = AGENT_DEFAULT_TIERS[agentName]; if (agentTier) { - if (effortCfg && effortCfg.routing_tier_defaults && - typeof effortCfg.routing_tier_defaults === 'object' && - !Array.isArray(effortCfg.routing_tier_defaults)) { - const v = effortCfg.routing_tier_defaults[agentTier]; - if (typeof v === 'string' && GSD_EFFORT_SET.has(v)) return v; - } else if (!effortCfg) { - // No effort config — use manifest tier defaults - const v = EFFORT_MANIFEST_TIER_DEFAULTS[agentTier]; - if (typeof v === 'string' && GSD_EFFORT_SET.has(v)) return v; - } - // effortCfg exists but has no routing_tier_defaults — fall through + const isValidEffort = (v: unknown): v is string => typeof v === 'string' && GSD_EFFORT_SET.has(v); + const merged = mergeEffortTierDefaults( + EFFORT_MANIFEST_TIER_DEFAULTS, + effortCfg ? effortCfg.routing_tier_defaults : undefined, + isValidEffort, + ); + const v = merged[agentTier]; + if (isValidEffort(v)) return v; } // Step 3: effort.default diff --git a/src/model-catalog.cts b/src/model-catalog.cts index f68b14198..eb8eb2151 100644 --- a/src/model-catalog.cts +++ b/src/model-catalog.cts @@ -343,5 +343,36 @@ export function renderEffortForRuntime(runtime: string, universalEffort: string) }; } +/** + * #3531 (10c) — Merge a config `effort.routing_tier_defaults` block over the + * manifest tier defaults instead of replacing them. A partial config must not + * discard built-ins: per tier, a valid override value wins and an invalid one + * is ignored so the manifest value for that tier surfaces (ADR-443 D1's + * "invalid values fall through" holds within the merged layer). + * + * Pure: returns a new object and never mutates either input — the manifest + * constants (`CANONICAL_CONFIG_DEFAULTS`, the catalog cache) stay frozen. The + * validator is injected because `EFFORT_SET` lives in model-resolver, which + * imports this leaf (a reverse import would be a cycle); both effort + * resolvers pass their own `(v) => typeof v === 'string' && EFFORT_SET.has(v)`. + */ +export function mergeEffortTierDefaults( + manifest: Record | null | undefined, + override: unknown, + isValid: (v: unknown) => boolean, +): Record { + const merged: Record = { ...(manifest || {}) }; + if (override && typeof override === 'object' && !Array.isArray(override)) { + for (const [tier, value] of Object.entries(override as Record)) { + // House pollution guard (mirrors _deepMergeConfig in config-loader): the + // string-only validator already makes these inert, but an explicit skip + // keeps this merge safe even if a caller's validator is ever relaxed. + if (tier === '__proto__' || tier === 'constructor' || tier === 'prototype') continue; + if (isValid(value)) merged[tier] = value as string; + } + } + return merged; +} + // ─── Fast mode propagation ─────────────────────────────────────────────────── export const RUNTIMES_WITH_FAST_MODE: Set = new Set(['api']); diff --git a/src/model-resolver.cts b/src/model-resolver.cts index 63cbbe11a..fd844fc91 100644 --- a/src/model-resolver.cts +++ b/src/model-resolver.cts @@ -31,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, CLAUDE_AGENT_ALIASES } from './model-catalog.cjs'; +import { MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, PROVIDER_PRESETS, VALID_TIERS, CLAUDE_AGENT_ALIASES, mergeEffortTierDefaults } from './model-catalog.cjs'; import fs from 'node:fs'; import path from 'node:path'; @@ -801,23 +801,26 @@ function resolveEffortInternal(cwd: string, agentType: string, opts?: EffortOpts } // Step 3: routing_tier_defaults by agent's default tier. + // #3531 (10c): the config block merges OVER the manifest tier defaults + // rather than replacing them — an effort block without + // routing_tier_defaults (or missing this agent's tier) falls back to the + // manifest built-in for that tier instead of skipping to effort.default. + // Invalid config values are dropped by the merge, so the manifest value for + // the tier surfaces (the same "invalid falls through" rule every layer has). const agentTier = (AGENT_DEFAULT_TIERS)[agentType]; if (agentTier) { - if (effortCfg && effortCfg['routing_tier_defaults'] && - typeof effortCfg['routing_tier_defaults'] === 'object' && - !Array.isArray(effortCfg['routing_tier_defaults'])) { - const v = (effortCfg['routing_tier_defaults'] as Record)[agentTier]; - if (typeof v === 'string' && EFFORT_SET.has(v)) return v; - } else if (!effortCfg) { - const canonicalEffort = (CANONICAL_CONFIG_DEFAULTS)['effort']; - const manifestDefaults = canonicalEffort && typeof canonicalEffort === 'object' - ? (canonicalEffort as Record)['routing_tier_defaults'] - : undefined; - if (manifestDefaults && typeof manifestDefaults === 'object') { - const v = (manifestDefaults as Record)[agentTier]; - if (typeof v === 'string' && EFFORT_SET.has(v)) return v; - } - } + const canonicalEffort = (CANONICAL_CONFIG_DEFAULTS)['effort']; + const manifestDefaults = canonicalEffort && typeof canonicalEffort === 'object' + ? (canonicalEffort as Record)['routing_tier_defaults'] as Record | undefined + : undefined; + const isValidEffort = (v: unknown): v is string => typeof v === 'string' && EFFORT_SET.has(v); + const merged = mergeEffortTierDefaults( + manifestDefaults, + effortCfg ? effortCfg['routing_tier_defaults'] : undefined, + isValidEffort, + ); + const v = merged[agentTier]; + if (isValidEffort(v)) return v; } // Step 4: effort.default diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index a31a97b9a..1ff64d26d 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -3774,7 +3774,10 @@ describe('#3533 effort sync: inherit means the key must not exist', () => { // Fixture carries its own name so the survivor assertion below is // satisfiable (AGENT_WITH_EFFORT names gsd-planner — wrong file). fs.writeFileSync(path.join(agentsDir, 'gsd-executor.md'), AGENT_WITH_EFFORT.replace('name: gsd-planner', 'name: gsd-executor')); - writePlanningConfig(tmpDir, { default: 'inherit' }); + // #3531+#3533 combined: pin every TIER to inherit — a bare effort.default + // no longer reaches a tiered agent now that the config block merges over + // the built-in tier ladder (the manifest standard tier would answer 'high'). + writePlanningConfig(tmpDir, { routing_tier_defaults: { light: 'inherit', standard: 'inherit', heavy: 'inherit' } }); const { cmdEffortSync } = require('../gsd-core/bin/lib/commands.cjs'); const result = captureOutput(() => @@ -3897,7 +3900,10 @@ describe('feat-488: effort sync command', () => { const agentsDir = makeAgentsDir(tmpDir); const agentPath = path.join(agentsDir, 'gsd-executor.md'); fs.writeFileSync(agentPath, AGENT_WITHOUT_EFFORT); - writePlanningConfig(tmpDir, { default: 'max' }); + // #3531: pin every tier so the injected value is tier-independent — an + // effort.default alone no longer answers for a tiered agent now that the + // config block merges over the built-in tier ladder. + writePlanningConfig(tmpDir, { routing_tier_defaults: { light: 'max', standard: 'max', heavy: 'max' }, default: 'max' }); const { cmdEffortSync } = require('../gsd-core/bin/lib/commands.cjs'); const result = captureOutput(() => @@ -3946,10 +3952,14 @@ describe('feat-488: effort sync command', () => { fs.mkdirSync(planningDir, { recursive: true }); fs.writeFileSync(path.join(planningDir, 'config.json'), JSON.stringify({ model_profile: 'balanced' })); - // Home defaults set effort.default = low + // Home defaults set the heavy tier effort to low. (#3531: a bare home + // effort.default would no longer reach gsd-planner — the merged tier + // ladder answers for tiered agents — so the home fixture pins the tier, + // which is what this test's claim actually exercises: home-level effort + // applies when the project config has no effort section.) const gsdDir = path.join(tmpHome, '.gsd'); fs.mkdirSync(gsdDir, { recursive: true }); - fs.writeFileSync(path.join(gsdDir, 'defaults.json'), JSON.stringify({ effort: { default: 'low' } })); + fs.writeFileSync(path.join(gsdDir, 'defaults.json'), JSON.stringify({ effort: { routing_tier_defaults: { heavy: 'low' } } })); // Isolate HOME (and USERPROFILE for Windows parity) so // readGsdEffectiveEffortConfig reads our fixture, not the @@ -3978,7 +3988,7 @@ describe('feat-488: effort sync command', () => { } } - // With home effort.default = 'low' and the agent currently at 'medium', + // With home heavy-tier effort 'low' and the agent currently at 'medium', // cmdEffortSync must sync exactly 1 agent and set it to 'low'. assert.equal(result.synced, 1, 'should sync 1 agent whose effort differs from home default'); assert.equal(result.changes[0].agent, 'gsd-planner'); diff --git a/tests/effort-surface-axis.test.cjs b/tests/effort-surface-axis.test.cjs index d1ce1652a..7402ef4bc 100644 --- a/tests/effort-surface-axis.test.cjs +++ b/tests/effort-surface-axis.test.cjs @@ -149,7 +149,9 @@ describe('#3534 resolve-execution reports resolved AND effective effort', () => t.after(() => cleanup(dir)); fs.writeFileSync( path.join(dir, '.planning', 'config.json'), - JSON.stringify({ runtime: 'codex', effort: { default: 'medium' } }, null, 2), + // #3531+#3534 combined: pin the AGENT — a bare effort.default no longer + // reaches a tiered agent under the merged tier ladder. + JSON.stringify({ runtime: 'codex', effort: { agent_overrides: { 'gsd-executor': 'medium' } } }, null, 2), ); const out = resolveExecution(dir); assert.equal(out.effort, 'medium'); @@ -440,7 +442,10 @@ describe('#2481 — the escalation surface renders argv (CLI-level, not a workfl fs.writeFileSync( path.join(dir, '.planning', 'config.json'), JSON.stringify({ - effort: { default: 'low' }, + // #3531: pin the heavy tier rather than effort.default — a bare default + // no longer answers for gsd-planner (heavy) now that the config block + // merges over the built-in tier ladder. + effort: { routing_tier_defaults: { heavy: 'low' } }, dynamic_routing: { enabled: true, escalate_on_failure: true, max_escalations: 3 }, }, null, 2), ); @@ -578,7 +583,9 @@ describe('#2481 review workflow resolves effort per reviewer', () => { fs.mkdirSync(path.join(projectDir, '.planning'), { recursive: true }); fs.writeFileSync( path.join(projectDir, '.planning', 'config.json'), - JSON.stringify({ effort: { default: 'xhigh' } }, null, 2), + // #3531: pin every tier so the expected value is agent-independent — + // the reviewer lane's tier decides, not effort.default. + JSON.stringify({ effort: { routing_tier_defaults: { light: 'xhigh', standard: 'xhigh', heavy: 'xhigh' } } }, null, 2), ); const r = cp.spawnSync( diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index fad341db5..7d2a8bb6f 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -4104,6 +4104,68 @@ describe('#3533 inherit: install writes NO effort key when the agent resolves to }); }); +// ─── describe 5c: #3531 (10c) — a partial effort block must not discard manifest tier defaults ── +// +// Regression fixture from issue #3160: adding four agent_overrides produced a +// 23-agent dry-run diff that DOWNGRADED xhigh-tier agents to high and UPGRADED +// low-tier agents to high — because an effort block without +// routing_tier_defaults disabled the built-in tier ladder (manifest tier +// defaults were consulted only when the whole effort block was absent). + +describe('#3531 resolveInstallTimeEffort: agent_overrides-only effort block keeps manifest tier defaults', () => { + let tmpDir; + let claudeHome; + + beforeEach(() => { + tmpDir = makeTmpDir('gsd-3531-tier-merge-'); + const projectDir = path.join(tmpDir, 'project'); + claudeHome = path.join(projectDir, '.claude'); + fs.mkdirSync(claudeHome, { recursive: true }); + fs.mkdirSync(path.join(projectDir, '.planning'), { recursive: true }); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + function writeProjectConfig(config) { + const projectDir = path.dirname(claudeHome); + fs.writeFileSync( + path.join(projectDir, '.planning', 'config.json'), + JSON.stringify(config, null, 2) + ); + } + + function readInstalledEffort(agentFile) { + const fm = readFrontmatter(path.join(claudeHome, 'agents', agentFile)); + const match = fm.match(/^effort:\s*(\S+)$/m); + assert.ok(match, `effort: must be present in ${agentFile} frontmatter\nActual:\n${fm}`); + return match[1]; + } + + test('agent_overrides-only block leaves untiered agents on their manifest tier defaults', () => { + // The exact #3160 shape: one agent overridden, no routing_tier_defaults. + writeProjectConfig({ effort: { agent_overrides: { 'gsd-code-reviewer': 'medium' } } }); + runGlobalInstall('claude', claudeHome); + + // Before #3531: every non-overridden agent collapsed to the manifest + // default 'high' — downgrading planner (xhigh) and upgrading mapper (low). + assert.strictEqual(readInstalledEffort('gsd-planner.md'), 'xhigh'); + assert.strictEqual(readInstalledEffort('gsd-executor.md'), 'high'); + assert.strictEqual(readInstalledEffort('gsd-codebase-mapper.md'), 'low'); + // The one agent the user actually named still gets its override. + assert.strictEqual(readInstalledEffort('gsd-code-reviewer.md'), 'medium'); + }); + + test('partial routing_tier_defaults fills gaps from the manifest at install', () => { + writeProjectConfig({ effort: { routing_tier_defaults: { heavy: 'medium' } } }); + runGlobalInstall('claude', claudeHome); + assert.strictEqual(readInstalledEffort('gsd-planner.md'), 'medium'); + assert.strictEqual(readInstalledEffort('gsd-executor.md'), 'high'); + assert.strictEqual(readInstalledEffort('gsd-codebase-mapper.md'), 'low'); + }); +}); + // ─── describe 5: Source stays clean ────────────────────────────────────────── describe('#443 Source purity: agents/gsd-planner.md has no effort: key', () => { diff --git a/tests/model-resolver.test.cjs b/tests/model-resolver.test.cjs index 07946a190..dde922712 100644 --- a/tests/model-resolver.test.cjs +++ b/tests/model-resolver.test.cjs @@ -258,6 +258,11 @@ describe('assertValidGranularityOverride', () => { // ─── resolveEffortInternal ──────────────────────────────────────────────────── +// #3531 — the runtime resolver's install-time sibling. Driven directly as a +// pure function (effortCfg in, effort out) for the parity matrix below. +const installEffortResolver = require('../gsd-core/bin/lib/install-effort-resolver.cjs'); +const { resolveInstallTimeEffort, readGsdEffectiveEffortConfig } = installEffortResolver; + describe('resolveEffortInternal', () => { let tmpDir; beforeEach(() => { tmpDir = makeTempProject(); }); @@ -318,8 +323,10 @@ describe('#3533 effort inherit: expressible at every layer, never a wire level', writeConfig(tmpDir, { effort: { default: 'inherit' } }); assert.strictEqual(resolveEffortInternal(tmpDir, 'completely-unknown-agent-xyz'), 'inherit'); - // A tiered agent under an inherit tier default also inherits (tier layer won). - assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'inherit'); + // #3531+#3533 combined: a bare effort.default no longer reaches a TIERED + // agent — the merged tier layer answers (manifest heavy = xhigh). To + // inherit at a tier, pin the tier; the tier-default row above covers that. + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-executor', { override: 'inherit' }), 'inherit'); }); @@ -347,6 +354,113 @@ describe('#3533 effort inherit: expressible at every layer, never a wire level', }); }); +// ─── #3531 (10c): routing_tier_defaults merges over manifest tier defaults ─── + +describe('#3531 routing_tier_defaults merge: manifest built-ins survive partial config', () => { + let tmpDir; + beforeEach(() => { tmpDir = makeTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + test('effort block without routing_tier_defaults keeps manifest tier defaults (runtime)', () => { + writeConfig(tmpDir, { effort: { agent_overrides: { 'gsd-executor': 'low' } } }); + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-executor'), 'low'); + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-codebase-mapper'), 'low'); + }); + + test('partial routing_tier_defaults merges over manifest, gaps filled per-tier (runtime)', () => { + writeConfig(tmpDir, { effort: { routing_tier_defaults: { heavy: 'medium' } } }); + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'medium'); + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-executor'), 'high'); + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-codebase-mapper'), 'low'); + }); + + test('non-object routing_tier_defaults treated as absent (runtime)', () => { + writeConfig(tmpDir, { effort: { routing_tier_defaults: ['heavy'] } }); + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); + }); + + test('merge never mutates the manifest defaults', () => { + const configuration = require('../gsd-core/bin/lib/configuration.cjs'); + const before = JSON.stringify(configuration.CONFIG_DEFAULTS['effort']); + writeConfig(tmpDir, { effort: { routing_tier_defaults: { heavy: 'medium' } } }); + resolveEffortInternal(tmpDir, 'gsd-planner'); + resolveInstallTimeEffort({ routing_tier_defaults: { heavy: 'medium' } }, 'gsd-planner'); + assert.strictEqual( + JSON.stringify(configuration.CONFIG_DEFAULTS['effort']), before, + 'resolving must not mutate CANONICAL_CONFIG_DEFAULTS.effort', + ); + }); +}); + +describe('#3531 parity: runtime and install-time resolvers agree on the merged tier ladder', () => { + let tmpDir; + beforeEach(() => { tmpDir = makeTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + const PARITY_AGENTS = ['gsd-planner', 'gsd-executor', 'gsd-codebase-mapper', 'completely-unknown-agent-xyz']; + const PARITY_EFFORT_CFGS = [ + null, + {}, + { default: 'low' }, + { routing_tier_defaults: { heavy: 'medium' } }, + { routing_tier_defaults: { light: 'low', standard: 'medium', heavy: 'low' } }, + { routing_tier_defaults: { heavy: 'turbo' }, default: 'low' }, + { routing_tier_defaults: 'not-an-object' }, + { agent_overrides: { 'gsd-executor': 'max' } }, + { agent_overrides: { 'gsd-executor': 42 }, routing_tier_defaults: { heavy: 'medium' } }, + ]; + + for (const effortCfg of PARITY_EFFORT_CFGS) { + for (const agent of PARITY_AGENTS) { + test(`parity: effortCfg=${JSON.stringify(effortCfg)} agent=${agent}`, () => { + writeConfig(tmpDir, effortCfg === null ? {} : { effort: effortCfg }); + const runtime = resolveEffortInternal(tmpDir, agent); + const installTime = resolveInstallTimeEffort(effortCfg, agent); + assert.strictEqual( + installTime, runtime, + `install-time and runtime resolvers disagree for effortCfg=${JSON.stringify(effortCfg)} agent=${agent}`, + ); + }); + } + } +}); + +describe('#3531 readGsdEffectiveEffortConfig: home/project routing_tier_defaults deep-merge', () => { + test('project partial tier block unions with home partial tier block per-tier', (t) => { + const tmpDir = makeTempProject('gsd-3531-home-merge-'); + t.after(() => cleanup(tmpDir)); + + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3531-home-')); + t.after(() => cleanup(homeDir)); + fs.mkdirSync(path.join(homeDir, '.gsd'), { recursive: true }); + fs.writeFileSync( + path.join(homeDir, '.gsd', 'defaults.json'), + JSON.stringify({ effort: { routing_tier_defaults: { heavy: 'low' } } }), + ); + + writeConfig(tmpDir, { effort: { routing_tier_defaults: { standard: 'medium' } } }); + + const oldHome = process.env.HOME; + const oldUserProfile = process.env.USERPROFILE; + process.env.HOME = homeDir; + process.env.USERPROFILE = homeDir; // os.homedir() is USERPROFILE-driven on win32 + t.after(() => { + process.env.HOME = oldHome; + if (oldUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = oldUserProfile; + }); + + const merged = readGsdEffectiveEffortConfig(tmpDir); + assert.deepStrictEqual(merged && merged.routing_tier_defaults, { heavy: 'low', standard: 'medium' }); + // The merged config resolves planner from HOME's heavy (low), executor from + // project's standard (medium), mapper from the manifest light (low). + assert.strictEqual(resolveInstallTimeEffort(merged, 'gsd-planner'), 'low'); + assert.strictEqual(resolveInstallTimeEffort(merged, 'gsd-executor'), 'medium'); + assert.strictEqual(resolveInstallTimeEffort(merged, 'gsd-codebase-mapper'), 'low'); + }); +}); + // ─── nextEffort ──────────────────────────────────────────────────────────────── describe('nextEffort', () => { @@ -1855,13 +1969,28 @@ describe('#443 integration (f): precedence matrix (property/table-driven)', () = expected: 'medium', }, { - label: 'layer 4 (effort.default) when no tier default set', + // #3531 (10c): an effort block without routing_tier_defaults no longer + // discards the manifest tier defaults — gsd-planner (heavy) gets the + // manifest 'xhigh', not effort.default. effort.default is still the + // layer that answers for an agent with NO catalog tier (see the + // unknown-agent row below, which is what this layer actually names). + label: 'layer 4 (effort.default) when agent has no catalog tier', config: { effort: { default: 'low' }, }, opts: {}, + agent: 'completely-unknown-agent-xyz', expected: 'low', }, + { + label: '10c: effort block without routing_tier_defaults keeps manifest tier defaults (#3531)', + config: { + effort: { default: 'low' }, + }, + opts: {}, + agent: 'gsd-planner', + expected: 'xhigh', + }, { label: 'invalid layer 1 (turbo) falls through to layer 2 (agent_override)', config: { @@ -1882,7 +2011,10 @@ describe('#443 integration (f): precedence matrix (property/table-driven)', () = expected: 'high', }, { - label: 'invalid tier default (turbo) falls through to effort.default', + // #3531 (10c): under the merged tier layer, an invalid config value for + // a tier falls back to the MANIFEST value for that same tier (xhigh for + // heavy), not to effort.default. + label: 'invalid tier default (turbo) falls back to the manifest value for that tier (#3531)', config: { effort: { routing_tier_defaults: { heavy: 'turbo' }, @@ -1890,14 +2022,16 @@ describe('#443 integration (f): precedence matrix (property/table-driven)', () = }, }, opts: {}, - expected: 'low', + expected: 'xhigh', }, ]; for (const row of effortPrecedenceTable) { test(`effort precedence: ${row.label}`, () => { writeConfig(tmpDir, row.config); - const result = resolveEffortInternal(tmpDir, 'gsd-planner', row.opts); + // #3531: rows may pin a specific agent (default keeps the historical + // gsd-planner target so existing rows are unchanged in what they assert). + const result = resolveEffortInternal(tmpDir, row.agent || 'gsd-planner', row.opts); assert.strictEqual(result, row.expected, `Expected '${row.expected}', got '${result}' — config: ${JSON.stringify(row.config)}`); }); @@ -2297,22 +2431,25 @@ describe('#443 effort cascade', () => { assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'medium'); }); - test('invalid routing_tier_defaults value falls through to effort.default', () => { + test('invalid routing_tier_defaults value falls back to the manifest value for that tier (#3531)', () => { writeConfig(tmpDir, { effort: { routing_tier_defaults: { heavy: 'turbo' }, default: 'low', }, }); - assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'low'); + // #3531: the config block merges OVER the manifest built-ins; an invalid + // entry is dropped by the merge, so the manifest heavy default surfaces. + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); }); - test('invalid effort.default falls through to hardcoded "high" (no routing_tier_defaults set)', () => { + test('invalid effort.default falls through to the manifest tier default (#3531)', () => { writeConfig(tmpDir, { effort: { default: 'turbo' }, }); - // effortCfg set but no routing_tier_defaults; turbo is invalid; fallback = hardcoded 'high' - assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'high'); + // #3531: an effort block without routing_tier_defaults keeps the manifest + // tier ladder; the invalid default never answers for a tiered agent. + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); }); test('unknown agent -> uses effort.default', () => { @@ -2323,12 +2460,12 @@ describe('#443 effort cascade', () => { assert.strictEqual(resolveEffortInternal(tmpDir, 'unknown-agent-xyz'), 'medium'); }); - test('effort.default numeric value (123) ignored, hardcoded "high" fallback', () => { + test('effort.default numeric value (123) ignored, manifest tier default answers (#3531)', () => { writeConfig(tmpDir, { effort: { default: 123 }, }); - // effortCfg set, no routing_tier_defaults -> no tier default; numeric ignored -> 'high' - assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'high'); + // #3531: no valid tier override -> manifest heavy default; numeric default ignored. + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); }); test('effort block missing entirely -> uses tier default', () => { @@ -2344,11 +2481,12 @@ describe('#443 effort cascade', () => { assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); }); - test('effort.routing_tier_defaults empty object -> effort.default', () => { + test('effort.routing_tier_defaults empty object -> manifest tier default (#3531)', () => { writeConfig(tmpDir, { effort: { routing_tier_defaults: {}, default: 'low' }, }); - assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'low'); + // #3531: an empty override block leaves the manifest built-ins in force. + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); }); }); @@ -2771,8 +2909,9 @@ describe('#443 QA matrix — malformed effort/fast_mode configs', () => { default: 'medium', }, }); - // boolean true is not a valid effort -> falls through to default 'medium' - assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'medium'); + // #3531: boolean true is not a valid effort -> the merge drops it and the + // manifest heavy default (xhigh) surfaces, not effort.default 'medium'. + assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'xhigh'); }); test('effort.agent_overrides is non-object -> falls through gracefully', () => {