diff --git a/.changeset/curious-birds-dance.md b/.changeset/curious-birds-dance.md new file mode 100644 index 000000000..74673963b --- /dev/null +++ b/.changeset/curious-birds-dance.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4019 +--- +**`workflow.inline_plan_threshold` now has one default owner** — the key is registered in the defaults manifest (default `2`), so `config-get` resolves the absent key instead of erroring, `settings-advanced` no longer misdocuments the default as 3, and every shipped surface (workflow fallback, reference tables) agrees. (#3801) diff --git a/gsd-core/bin/shared/config-defaults.manifest.json b/gsd-core/bin/shared/config-defaults.manifest.json index eb443f9f7..164b4ac5d 100644 --- a/gsd-core/bin/shared/config-defaults.manifest.json +++ b/gsd-core/bin/shared/config-defaults.manifest.json @@ -1,5 +1,5 @@ { - "_comment": "Canonical CONFIG_DEFAULTS for the Configuration Module. Nested shape is canonical. CJS flat projection (branching_strategy, sub_repos, etc.) is handled by consumers at the boundary. Security keys (security_enforcement, security_asvs_level, security_block_on) and post_planning_gaps live in workflow.* as their canonical location. resolve_model_ids, context_window, phase_naming are CJS-originated top-level keys included here. brave_search, firecrawl, exa_search default to false in the manifest; at runtime buildNewProjectConfig detects API keys. The plan_checker (CJS flat) → workflow.plan_check (canonical nested) divergence is resolved: canonical name is workflow.plan_check. _auto_chain_active is a runtime-state field included for completeness.", + "_comment": "Canonical CONFIG_DEFAULTS for the Configuration Module. Nested shape is canonical. CJS flat projection (branching_strategy, sub_repos, etc.) is handled by consumers at the boundary. Security keys (security_enforcement, security_asvs_level, security_block_on) and post_planning_gaps live in workflow.* as their canonical location. resolve_model_ids, context_window, phase_naming are CJS-originated top-level keys included here. brave_search, firecrawl, exa_search default to false in the manifest; at runtime buildNewProjectConfig detects API keys. The plan_checker (CJS flat) \u2192 workflow.plan_check (canonical nested) divergence is resolved: canonical name is workflow.plan_check. _auto_chain_active is a runtime-state field included for completeness.", "model_profile": "balanced", "commit_docs": true, "parallelization": true, @@ -52,6 +52,7 @@ "plan_bounce": false, "plan_bounce_script": null, "plan_bounce_passes": 2, + "inline_plan_threshold": 2, "auto_prune_state": false, "post_planning_gaps": true, "security_enforcement": true, diff --git a/gsd-core/references/planning-config.md b/gsd-core/references/planning-config.md index e44d9933c..2e83ec22d 100644 --- a/gsd-core/references/planning-config.md +++ b/gsd-core/references/planning-config.md @@ -38,6 +38,7 @@ Configuration options for `.planning/` directory behavior. | `git.quick_branch_template` | `null` | Optional branch template for quick-task runs | | `workflow.use_worktrees` | `true` | Whether executor agents run in isolated git worktrees. Set to `false` to disable worktrees — agents execute sequentially on the main working tree instead. Recommended for solo developers or when worktree merges cause issues. Note: if your branch is ahead of `origin/HEAD` (a diverged milestone or feature branch), GSD auto-degrades to sequential and prints a warning; set `worktree.baseRef:"head"` in `.claude/settings.local.json` to restore parallel execution. See the branch-divergence note below. | | `workflow.subagent_timeout` | `300000` | Timeout in milliseconds for parallel subagent tasks (e.g. codebase mapping). Increase for large codebases or slower models. Default: 300000 (5 minutes). | +| `workflow.inline_plan_threshold` | `2` | Plans with this many tasks or fewer execute inline (Pattern C) instead of spawning a subagent. Avoids ~14K token spawn overhead for small plans. Set to `0` to always spawn subagents. | | `workflow.test_command` | `null` | Custom shell command run as the regression/test gate by execute-phase, audit-fix, and post-merge-gate. When unset, GSD auto-detects (Makefile / package.json / Cargo.toml / go.mod / pyproject.toml). Example: `npm test`. | | `workflow.build_command` | `null` | Custom shell command run as the build gate by the post-merge gate. When unset, the build step is skipped/auto-detected. Example: `npm run build`. | | `workflow.inline_plan_threshold` | `2` | Plans with this many tasks or fewer execute inline (Pattern C) instead of spawning a subagent. Avoids ~14K token spawn overhead for small plans. Set to `0` to always spawn subagents. | @@ -271,6 +272,7 @@ Set via `workflow.*` namespace in config.json (e.g., `"workflow": { "research": | `workflow.skip_discuss` | boolean | `false` | `true`, `false` | Skip discuss phase entirely | | `workflow.use_worktrees` | boolean | `true` | `true`, `false` | Run executor agents in isolated git worktrees | | `workflow.subagent_timeout` | number | `300000` | Any positive integer (ms) | Timeout for parallel subagent tasks (default: 5 minutes) | +| `workflow.inline_plan_threshold` | number | `2` | `0`–`10` | Plans with ≤N tasks execute inline instead of spawning a subagent | | `workflow.test_command` | string\|null | `null` | Any shell command | Regression/test gate command run by execute-phase, audit-fix, and post-merge-gate. Unset → GSD auto-detects (Makefile / package.json / Cargo.toml / go.mod / pyproject.toml). | | `workflow.build_command` | string\|null | `null` | Any shell command | Build gate command run by the post-merge gate. Unset → build step auto-detected/skipped. | | `workflow.mvp_mode` | boolean | `false` | `true`, `false` | Persist the MVP-mode flag in config so every phase defaults to MVP framing without requiring `--mvp` on the CLI. Resolved via the chain: `--mvp` CLI flag → ROADMAP.md `**Mode:** mvp` field → this config value → `false`. When `true`, the planner, executor, verifier, and discovery surfaces (progress, stats, graphify) all treat the phase as an MVP vertical slice (UI → API → DB) of one user-visible capability. | diff --git a/gsd-core/workflows/settings-advanced.md b/gsd-core/workflows/settings-advanced.md index efc935cd9..1ad7b733f 100644 --- a/gsd-core/workflows/settings-advanced.md +++ b/gsd-core/workflows/settings-advanced.md @@ -51,7 +51,7 @@ Planning Tuning: - `workflow.plan_bounce_passes` (default: `2`) - `workflow.plan_bounce_script` (default: `null`) - `workflow.subagent_timeout` (default: `300000`) -- `workflow.inline_plan_threshold` (default: `3`) +- `workflow.inline_plan_threshold` (default: `2`) Execution Tuning: - `workflow.node_repair` (default: `true`) diff --git a/src/config-loader.cts b/src/config-loader.cts index c3bd4748a..4e3af331d 100644 --- a/src/config-loader.cts +++ b/src/config-loader.cts @@ -134,6 +134,7 @@ const CONFIG_DEFAULTS = { security_block_on: _getNestedConfigDefault('workflow', 'security_block_on'), post_planning_gaps: _getNestedConfigDefault('workflow', 'post_planning_gaps'), smart_zone_tokens: _getNestedConfigDefault('workflow', 'smart_zone_tokens'), + inline_plan_threshold: _getNestedConfigDefault('workflow', 'inline_plan_threshold'), // #3801 max_prompt_tokens: _getNestedConfigDefault('review', 'max_prompt_tokens'), }; diff --git a/src/config.cts b/src/config.cts index 1b85906d9..94a75a81e 100644 --- a/src/config.cts +++ b/src/config.cts @@ -111,6 +111,11 @@ const SCHEMA_DEFAULTS: Record = { // manifest default rather than "Key not found". Derived from the defaults manifest so // the manifest stays the single source of truth. 'planning.pr_strict': CONFIG_DEFAULTS.pr_strict, + // #3801: execute-plan reads this key on every run; an absent key must resolve + // to the manifest default (2) rather than "Key not Found" — previously the + // effective default existed only as the workflow's shell fallback and the + // docs disagreed (settings-advanced said 3). Manifest stays the one owner. + 'workflow.inline_plan_threshold': CONFIG_DEFAULTS.inline_plan_threshold, }; /** diff --git a/tests/config-field-docs.test.cjs b/tests/config-field-docs.test.cjs index a08772c23..b2c74b700 100644 --- a/tests/config-field-docs.test.cjs +++ b/tests/config-field-docs.test.cjs @@ -105,6 +105,7 @@ describe('config-field-docs', () => { security_enforcement: 'workflow.security_enforcement', security_asvs_level: 'workflow.security_asvs_level', security_block_on: 'workflow.security_block_on', + inline_plan_threshold: 'workflow.inline_plan_threshold', // #3801 }; const missing = keys.filter(k => { diff --git a/tests/inline-plan-threshold-default.test.cjs b/tests/inline-plan-threshold-default.test.cjs new file mode 100644 index 000000000..45ff6c7ba --- /dev/null +++ b/tests/inline-plan-threshold-default.test.cjs @@ -0,0 +1,65 @@ +'use strict'; + +// ───────────────────────────────────────────────────────────────────────────── +// #3801 — workflow.inline_plan_threshold's default must have ONE owner. +// +// The effective default existed only as execute-plan.md's shell fallback +// (`|| echo "2"`); the key was never registered in +// config-defaults.manifest.json, and settings-advanced.md documented the +// default as 3. This guard pins: the manifest OWNS the default (key present, +// value 2), every shipped doc that names a default agrees with the manifest, +// and the shell fallbacks agree too. +// ───────────────────────────────────────────────────────────────────────────── + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const REPO = path.join(__dirname, '..'); +const MANIFEST = JSON.parse(fs.readFileSync( + path.join(REPO, 'gsd-core', 'bin', 'shared', 'config-defaults.manifest.json'), + 'utf8', +)); + +test('#3801: the defaults manifest owns workflow.inline_plan_threshold (default 2)', () => { + assert.ok( + MANIFEST.workflow && Object.prototype.hasOwnProperty.call(MANIFEST.workflow, 'inline_plan_threshold'), + 'the key must be registered in config-defaults.manifest.json — one source of truth', + ); + assert.strictEqual(MANIFEST.workflow.inline_plan_threshold, 2); +}); + +test('#3801: every shipped surface naming the default agrees with the manifest', () => { + const surfaces = [ + ['gsd-core/workflows/settings-advanced.md', /inline_plan_threshold`\s*\(default:\s*`(\d+)`/], + ]; + for (const [rel, re] of surfaces) { + const md = fs.readFileSync(path.join(REPO, rel), 'utf8'); + const m = re.exec(md); + assert.ok(m, `${rel} must name the inline_plan_threshold default`); + assert.strictEqual(m[1], '2', `#3801: ${rel} documents the default as ${m[1]} — must agree with the manifest's 2`); + } + // planning-config.md's two tables are read via the shared markdown-table + // parser (parseMarkdownTable), not an ad-hoc cell regex. + const { parseMarkdownTable } = require('../gsd-core/bin/lib/markdown-table.cjs'); + const pc = fs.readFileSync(path.join(REPO, 'gsd-core', 'references', 'planning-config.md'), 'utf8'); + const parsed = parseMarkdownTable(pc); + assert.ok(parsed.ok, 'planning-config.md must contain a parseable defaults table'); + const defaultsTable = parsed.value; + const thresholdRows = defaultsTable.rows.filter((r) => String(r['Option']).includes('inline_plan_threshold')); + assert.ok(thresholdRows.length >= 1, 'planning-config.md must document inline_plan_threshold'); + for (const row of thresholdRows) { + const def = String(row['Default']).replace(/`/g, '').trim(); + assert.strictEqual(def, '2', `#3801: planning-config.md documents the default as ${def} — must agree with the manifest's 2`); + } + // The execute-plan prose default and the shell fallback both say 2. + const ep = fs.readFileSync(path.join(REPO, 'gsd-core', 'workflows', 'execute-plan.md'), 'utf8'); + // The shell-fallback line is matched with a pipe-free, CRLF-safe pattern. + const { splitLines } = require('../gsd-core/bin/lib/text-lines.cjs'); + const fallbackLine = splitLines(ep).find((l) => l.includes('config-get workflow.inline_plan_threshold')); + assert.ok(fallbackLine, 'execute-plan.md must fetch the key'); + assert.ok(fallbackLine.includes('echo "2"'), + `the shell fallback must agree with the manifest default; got ${fallbackLine}`); + assert.match(ep, /default: 2, set to `0` to always spawn/, 'the prose default must stay 2'); +});