From 57bbfe652bf52aa885a90069434bf0e42709fa51 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 21 Apr 2026 10:10:10 -0400 Subject: [PATCH] fix: exclude non-wiped dirs from custom-file scan; warn on non-Claude model profiles (#2511) * fix(detect-custom-files): exclude skills and command dirs not wiped by installer (closes #2505) GSD_MANAGED_DIRS included 'skills' and 'command' directories, but the installer never wipes those paths. Users with third-party skills installed (40+ files, none in GSD's manifest) had every skill flagged as a "custom file" requiring backup, producing noisy false-positive reports on every /gsd-update run. Removes 'skills' and 'command' from both gsd-tools.cjs and the SDK's detect-custom-files.ts. Adds two regression tests confirming neither directory is scanned. Co-Authored-By: Claude Sonnet 4.6 * fix(settings): warn that model profiles are no-ops on non-Claude runtimes (closes #2506) settings.md presented Quality/Balanced/Budget model profiles without any indication that these tiers map to Claude models (Opus/Sonnet/Haiku) and have no effect on non-Claude runtimes (Codex, Gemini CLI, OpenRouter). Users on Codex saw the profile chooser as if it would meaningfully select models, but all agents silently used the runtime default regardless. Adds a non-Claude runtime note before the profile question (shown in TEXT_MODE, the path all non-Claude runtimes take) explaining the profiles are no-ops and directing users to either choose Inherit or configure model_overrides manually. Also updates the Inherit option description to explicitly name the runtimes where it is the correct choice. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- get-shit-done/bin/gsd-tools.cjs | 4 -- get-shit-done/workflows/settings.md | 19 +++++-- sdk/src/query/detect-custom-files.ts | 2 - ...ettings-profile-nonclaude-warning.test.cjs | 55 +++++++++++++++++++ tests/update-custom-backup.test.cjs | 54 ++++++++++++++++++ 5 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 tests/bug-2506-settings-profile-nonclaude-warning.test.cjs diff --git a/get-shit-done/bin/gsd-tools.cjs b/get-shit-done/bin/gsd-tools.cjs index 9b07624d..7d8a398a 100755 --- a/get-shit-done/bin/gsd-tools.cjs +++ b/get-shit-done/bin/gsd-tools.cjs @@ -1204,10 +1204,6 @@ async function runCommand(command, args, cwd, raw, defaultValue) { 'agents', path.join('commands', 'gsd'), 'hooks', - // OpenCode/Kilo flat command dir - 'command', - // Codex/Copilot skills dir - 'skills', ]; function walkDir(dir, baseDir) { diff --git a/get-shit-done/workflows/settings.md b/get-shit-done/workflows/settings.md index c87ce8f8..5c86eedb 100644 --- a/get-shit-done/workflows/settings.md +++ b/get-shit-done/workflows/settings.md @@ -51,6 +51,17 @@ Parse current values (default to `true` if not present): **Text mode (`workflow.text_mode: true` in config or `--text` flag):** Set `TEXT_MODE=true` if `--text` is present in `$ARGUMENTS` OR `text_mode` from init JSON is `true`. When TEXT_MODE is active, replace every `AskUserQuestion` call with a plain-text numbered list and ask the user to type their choice number. This is required for non-Claude runtimes (OpenAI Codex, Gemini CLI, etc.) where `AskUserQuestion` is not available. + +**Non-Claude runtime note:** If `TEXT_MODE` is active (i.e. the runtime is non-Claude), prepend the following notice before the model profile question: + +``` +Note: Quality, Balanced, and Budget profiles select Claude model tiers (Opus/Sonnet/Haiku). +On non-Claude runtimes (Codex, Gemini CLI, etc.) these profiles have no effect on actual +model selection — GSD agents will use the runtime's default model. +Choose "Inherit" to use the session model for all agents, or configure model_overrides +manually in .planning/config.json to target specific models for this runtime. +``` + Use AskUserQuestion with current values pre-selected: ``` @@ -60,10 +71,10 @@ AskUserQuestion([ header: "Model", multiSelect: false, options: [ - { label: "Quality", description: "Opus everywhere except verification (highest cost)" }, - { label: "Balanced (Recommended)", description: "Opus for planning, Sonnet for research/execution/verification" }, - { label: "Budget", description: "Sonnet for writing, Haiku for research/verification (lowest cost)" }, - { label: "Inherit", description: "Use current session model for all agents (best for OpenRouter, local models, or runtime model switching)" } + { label: "Quality", description: "Opus everywhere except verification (highest cost) — Claude only" }, + { label: "Balanced (Recommended)", description: "Opus for planning, Sonnet for research/execution/verification — Claude only" }, + { label: "Budget", description: "Sonnet for writing, Haiku for research/verification (lowest cost) — Claude only" }, + { label: "Inherit", description: "Use current session model for all agents (required for non-Claude runtimes: Codex, Gemini CLI, OpenRouter, local models)" } ] }, { diff --git a/sdk/src/query/detect-custom-files.ts b/sdk/src/query/detect-custom-files.ts index e0975e0a..2f496beb 100644 --- a/sdk/src/query/detect-custom-files.ts +++ b/sdk/src/query/detect-custom-files.ts @@ -14,8 +14,6 @@ const GSD_MANAGED_DIRS = [ 'agents', join('commands', 'gsd'), 'hooks', - 'command', - 'skills', ]; function walkDir(dir: string, baseDir: string): string[] { diff --git a/tests/bug-2506-settings-profile-nonclaude-warning.test.cjs b/tests/bug-2506-settings-profile-nonclaude-warning.test.cjs new file mode 100644 index 00000000..870395c3 --- /dev/null +++ b/tests/bug-2506-settings-profile-nonclaude-warning.test.cjs @@ -0,0 +1,55 @@ +/** + * Regression test for bug #2506 + * + * /gsd-settings presents Quality/Balanced/Budget model profiles without any + * warning that on non-Claude runtimes (Codex, Gemini CLI, etc.) these profiles + * select Claude model tiers and have no effect on actual agent model selection. + * + * Fix: settings.md must include a non-Claude runtime note instructing users to + * use "Inherit" or configure model_overrides manually, and the Inherit option + * description must explicitly call out non-Claude runtimes. + * + * Closes: #2506 + */ + +'use strict'; + +const { describe, test, before } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const SETTINGS_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'settings.md'); + +describe('bug #2506: settings.md non-Claude runtime warning for model profiles', () => { + let content; + + before(() => { + content = fs.readFileSync(SETTINGS_PATH, 'utf-8'); + }); + + test('settings.md contains a non-Claude runtime note for model profiles', () => { + assert.ok( + content.includes('non-Claude runtime') || content.includes('non-Claude runtimes'), + 'settings.md must include a note about non-Claude runtimes and model profiles' + ); + }); + + test('non-Claude note explains profiles are no-ops without model_overrides', () => { + assert.ok( + content.includes('model_overrides') || content.includes('no effect'), + 'note must explain profiles have no effect on non-Claude runtimes without model_overrides' + ); + }); + + test('Inherit option description explicitly mentions non-Claude runtimes', () => { + // The Inherit option in AskUserQuestion must call out non-Claude runtimes + const inheritOptionMatch = content.match(/label:\s*"Inherit"[^}]*description:\s*"([^"]+)"/s); + assert.ok(inheritOptionMatch, 'Inherit option with label/description must exist in settings.md'); + const desc = inheritOptionMatch[1]; + assert.ok( + desc.includes('non-Claude') || desc.includes('Codex') || desc.includes('Gemini'), + `Inherit option description must mention non-Claude runtimes; got: "${desc}"` + ); + }); +}); diff --git a/tests/update-custom-backup.test.cjs b/tests/update-custom-backup.test.cjs index 065ef662..1bdfcc66 100644 --- a/tests/update-custom-backup.test.cjs +++ b/tests/update-custom-backup.test.cjs @@ -225,4 +225,58 @@ describe('detect-custom-files — update workflow backup detection (#1997)', () `should detect custom reference; got: ${JSON.stringify(json.custom_files)}` ); }); + + // #2505 — installer does NOT wipe skills/ or command/; scanning them produces + // false-positive "custom file" reports for every skill the user has installed + // from other packages. + test('does not scan skills/ directory (installer does not wipe it)', () => { + writeManifest(tmpDir, { + 'get-shit-done/workflows/execute-phase.md': '# Execute Phase\n', + }); + + // Simulate user having third-party skills installed — none in manifest + const skillsDir = path.join(tmpDir, 'skills'); + fs.mkdirSync(skillsDir, { recursive: true }); + fs.writeFileSync(path.join(skillsDir, 'my-custom-skill.md'), '# My Skill\n'); + fs.writeFileSync(path.join(skillsDir, 'another-plugin-skill.md'), '# Another\n'); + + const result = runGsdTools( + ['detect-custom-files', '--config-dir', tmpDir], + tmpDir + ); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const json = JSON.parse(result.output); + const skillFiles = json.custom_files.filter(f => f.startsWith('skills/')); + assert.strictEqual( + skillFiles.length, 0, + `skills/ should not be scanned; got false positives: ${JSON.stringify(skillFiles)}` + ); + }); + + test('does not scan command/ directory (installer does not wipe it)', () => { + writeManifest(tmpDir, { + 'get-shit-done/workflows/execute-phase.md': '# Execute Phase\n', + }); + + // Simulate files in command/ dir not wiped by installer + const commandDir = path.join(tmpDir, 'command'); + fs.mkdirSync(commandDir, { recursive: true }); + fs.writeFileSync(path.join(commandDir, 'user-command.md'), '# User Command\n'); + + const result = runGsdTools( + ['detect-custom-files', '--config-dir', tmpDir], + tmpDir + ); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const json = JSON.parse(result.output); + const commandFiles = json.custom_files.filter(f => f.startsWith('command/')); + assert.strictEqual( + commandFiles.length, 0, + `command/ should not be scanned; got false positives: ${JSON.stringify(commandFiles)}` + ); + }); });