From 7397f580a555491eb2ba0d4e51d8dafbd489a1db Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 21 Apr 2026 21:35:22 -0400 Subject: [PATCH] fix(#2516): resolve executor_model inherit literal passthrough; add regression test (#2537) When model_profile is "inherit", execute-phase was passing the literal string "inherit" to Task(model=), causing fallback to the default model. The workflow now documents that executor_model=="inherit" requires omitting the model= parameter entirely so Claude Code inherits the orchestrator model automatically. Closes #2516 --- get-shit-done/workflows/execute-phase.md | 7 +- ...-2516-inherit-model-execute-phase.test.cjs | 114 ++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 tests/bug-2516-inherit-model-execute-phase.test.cjs diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index 359d737c..aa78b849 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -74,6 +74,8 @@ AGENT_SKILLS=$(gsd-sdk query agent-skills gsd-executor 2>/dev/null) Parse JSON for: `executor_model`, `verifier_model`, `commit_docs`, `parallelization`, `branching_strategy`, `branch_name`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `plans`, `incomplete_plans`, `plan_count`, `incomplete_count`, `state_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`. +**Model resolution:** If `executor_model` is `"inherit"`, omit the `model=` parameter from all `Task()` calls — do NOT pass `model="inherit"` to Task. Omitting the `model=` parameter causes Claude Code to inherit the current orchestrator model automatically. Only set `model=` when `executor_model` is an explicit model name (e.g., `"claude-sonnet-4-6"`, `"claude-opus-4-7"`). + **If `response_language` is set:** Include `response_language: {value}` in all spawned subagent prompts so any user-facing output stays in the configured language. Read worktree config: @@ -421,7 +423,10 @@ Execute each selected wave in sequence. Within a wave: parallel if `PARALLELIZAT Task( subagent_type="gsd-executor", description="Execute plan {plan_number} of phase {phase_number}", - model="{executor_model}", + # Only include model= when executor_model is an explicit model name. + # When executor_model is "inherit", omit this parameter entirely so + # Claude Code inherits the orchestrator model automatically. + model="{executor_model}", # omit this line when executor_model == "inherit" isolation="worktree", prompt=" diff --git a/tests/bug-2516-inherit-model-execute-phase.test.cjs b/tests/bug-2516-inherit-model-execute-phase.test.cjs new file mode 100644 index 00000000..0994e573 --- /dev/null +++ b/tests/bug-2516-inherit-model-execute-phase.test.cjs @@ -0,0 +1,114 @@ +/** + * Regression test for bug #2516 + * + * When `.planning/config.json` has `model_profile: "inherit"`, the + * `init.execute-phase` query returns `executor_model: "inherit"`. The + * execute-phase workflow was passing this literal string directly to the + * Task tool via `model="{executor_model}"`, causing Task to fall back to + * its default model instead of inheriting the orchestrator model. + * + * Fix: the workflow must document that when `executor_model` is `"inherit"`, + * the `model=` parameter must be OMITTED from Task() calls entirely. + * Omitting `model=` causes Claude Code to inherit the current orchestrator + * model automatically. + */ + +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const WORKFLOW_PATH = path.join( + __dirname, + '..', + 'get-shit-done', + 'workflows', + 'execute-phase.md' +); + +describe('bug #2516: executor_model "inherit" must not be passed literally to Task()', () => { + test('workflow file exists', () => { + assert.ok(fs.existsSync(WORKFLOW_PATH), 'get-shit-done/workflows/execute-phase.md should exist'); + }); + + test('workflow contains instructions for handling the "inherit" case', () => { + assert.ok(fs.existsSync(WORKFLOW_PATH), 'get-shit-done/workflows/execute-phase.md should exist'); + const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + const hasInheritInstruction = + content.includes('"inherit"') && + (content.includes('omit') || content.includes('Omit') || content.includes('omitting') || content.includes('Omitting')); + assert.ok( + hasInheritInstruction, + 'execute-phase.md must document that when executor_model is "inherit", ' + + 'the model= parameter must be omitted from Task() calls. ' + + 'Found "inherit" mention: ' + content.includes('"inherit"') + '. ' + + 'Found omit mention: ' + (content.includes('omit') || content.includes('Omit')) + ); + }); + + test('workflow does not instruct passing model="inherit" literally to Task', () => { + assert.ok(fs.existsSync(WORKFLOW_PATH), 'get-shit-done/workflows/execute-phase.md should exist'); + const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + // The workflow must not have an unconditional model="{executor_model}" template + // that would pass "inherit" through. It should document conditional logic. + const hasConditionalModelParam = + content.includes('inherit') && + ( + content.includes('Only set `model=`') || + content.includes('only set `model=`') || + content.includes('Only set model=') || + content.includes('omit the `model=`') || + content.includes('omit the model=') || + content.includes('omit `model=`') || + content.includes('omit model=') + ); + const lines = content.split('\n'); + const hasLiteralInheritInTask = lines.some(line => { + if (!/model\s*=\s*["']inherit["']/.test(line)) return false; + // Exclude instructional/explanatory lines that document what NOT to do + return !/\b(not|NOT|don'?t|do not|DO NOT|never|NEVER)\b/.test(line); + }); + assert.ok( + !hasLiteralInheritInTask, + 'execute-phase workflow must not pass literal "inherit" string to Task() model parameter' + ); + assert.ok( + hasConditionalModelParam && !hasLiteralInheritInTask, + 'execute-phase.md must conditionally omit model= when executor_model is "inherit", never pass it literally. ' + + 'The unconditional model="{executor_model}" template would pass the literal ' + + 'string "inherit" to Task(), which falls back to the default model instead ' + + 'of the orchestrator model (root cause of #2516).' + ); + // Guard against a future contributor adding an unconditional model="{executor_model}" + // template alongside the conditional docs — that would pass "inherit" literally to Task(). + const hasUnsafeTemplate = lines.some(line => { + if (!/model\s*=\s*['"]\{executor_model\}['"]/.test(line)) return false; + return !/\b(not|NOT|do not|DO NOT|don'?t|never|NEVER|omit)\b/i.test(line); + }); + assert.ok(!hasUnsafeTemplate, + 'execute-phase.md must not contain an unconditional model="{executor_model}" template — ' + + 'it would pass "inherit" literally to Task() when executor_model is "inherit"' + ); + }); + + test('workflow documents that omitting model= causes inheritance from orchestrator', () => { + assert.ok(fs.existsSync(WORKFLOW_PATH), 'get-shit-done/workflows/execute-phase.md should exist'); + const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + const hasInheritanceExplanation = + content.includes('inherit') && + ( + content.includes('orchestrator model') || + content.includes('orchestrator\'s model') || + content.includes('inherits the') || + content.includes('inherit the current') + ); + assert.ok( + hasInheritanceExplanation, + 'execute-phase.md must explain that omitting model= causes Claude Code to ' + + 'inherit the current orchestrator model — this is the mechanism that makes ' + + '"inherit" work correctly.' + ); + }); +});