From 3078279a9ad4e6bf7e66665f57bb4e209b58bf10 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 3 Apr 2026 13:00:41 -0400 Subject: [PATCH] feat(planner): add reachability_check step (#1606) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(planner): add reachability_check step to prevent unreachable code Closes #1495 * fix: trim gsd-planner.md below 50000-char limit after rebase The reachability_check addition pushed the file to 50,275 chars when merged with the assign_waves additions from #1600. Condense both sections while preserving all logic; file is now 49,859 chars. Co-Authored-By: Claude Sonnet 4.6 * fix(security): normalize CRLF before prompt-stuffing length check On Windows, git checks out files with CRLF line endings. JavaScript's String.length counts \r characters, so a 49,859-byte file measures as 51,126 chars on Windows — falsely tripping the 50,000-char security scanner. Normalize CRLF → LF before measuring in security.cjs and in the reachability-check test. Co-Authored-By: Claude Sonnet 4.6 * fix: trim gsd-planner.md to stay under 50000-char limit after merge with main After rebasing onto main (which added mcp_tool_usage block), combined content reached 50031 chars. Remove suggested log format from assign_waves rule to bring file to 49972 chars, well under the 50000-char security scanner limit. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- agents/gsd-planner.md | 15 ++++++--- get-shit-done/bin/lib/security.cjs | 8 +++-- tests/reachability-check.test.cjs | 53 ++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 8 deletions(-) create mode 100644 tests/reachability-check.test.cjs diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index 139226c45..e4e6f8a93 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -1232,11 +1232,7 @@ for each plan B in plan_order: waves[B.id] = B.wave ``` -**Rule:** Any two plans in the same wave MUST have zero `files_modified` overlap — even if -no explicit `depends_on` was set. After computing all wave numbers, verify every wave group: -collect the union of `files_modified` per wave; if any file appears in two or more plans -within the same wave, bump the later plan to the next wave and repeat until clean. -Log each bump: `"Plan {B} moved to wave {N+1}: files_modified overlap with Plan {A} on {file}"` +**Rule:** Same-wave plans must have zero `files_modified` overlap. After assigning waves, scan each wave; if any file appears in 2+ plans, bump the later plan to the next wave and repeat. @@ -1256,6 +1252,15 @@ Apply goal-backward methodology (see goal_backward section): 5. Identify key links (critical connections) + +For each must-have artifact, verify a concrete path exists: +- Entity → in-phase or existing creation path +- Workflow → user action or API call triggers it +- Config flag → default value + consumer +- UI → route or nav link +UNREACHABLE (no path) → revise plan. + + Verify each plan fits context budget: 2-3 tasks, ~50% target. Split if necessary. Check granularity setting. diff --git a/get-shit-done/bin/lib/security.cjs b/get-shit-done/bin/lib/security.cjs index ffef1fc51..2a256667e 100644 --- a/get-shit-done/bin/lib/security.cjs +++ b/get-shit-done/bin/lib/security.cjs @@ -181,9 +181,11 @@ function scanForInjection(text, opts = {}) { findings.push('Contains suspicious zero-width or invisible Unicode characters'); } - // Check for extremely long strings that could be prompt stuffing - if (text.length > 50000) { - findings.push(`Suspicious text length: ${text.length} chars (potential prompt stuffing)`); + // Check for extremely long strings that could be prompt stuffing. + // Normalize CRLF → LF before measuring so Windows checkouts don't inflate the count. + const normalizedLength = text.replace(/\r\n/g, '\n').replace(/\r/g, '\n').length; + if (normalizedLength > 50000) { + findings.push(`Suspicious text length: ${normalizedLength} chars (potential prompt stuffing)`); } } diff --git a/tests/reachability-check.test.cjs b/tests/reachability-check.test.cjs new file mode 100644 index 000000000..d28d74625 --- /dev/null +++ b/tests/reachability-check.test.cjs @@ -0,0 +1,53 @@ +const { test, describe } = require('node:test'); +const assert = require('node:assert'); +const fs = require('fs'); +const path = require('path'); + +describe('gsd-planner reachability_check step', () => { + const plannerPath = path.join(__dirname, '..', 'agents', 'gsd-planner.md'); + let content; + + test('planner file exists', () => { + assert.ok(fs.existsSync(plannerPath)); + content = fs.readFileSync(plannerPath, 'utf-8'); + }); + + test('contains reachability_check step', () => { + content = content || fs.readFileSync(plannerPath, 'utf-8'); + assert.ok(content.includes(''), 'Missing reachability_check step'); + }); + + test('reachability_check appears after derive_must_haves', () => { + content = content || fs.readFileSync(plannerPath, 'utf-8'); + const mustHavesIdx = content.indexOf('derive_must_haves'); + const reachabilityIdx = content.indexOf('reachability_check'); + assert.ok(mustHavesIdx > -1, 'derive_must_haves step not found'); + assert.ok(reachabilityIdx > -1, 'reachability_check step not found'); + assert.ok(reachabilityIdx > mustHavesIdx, 'reachability_check must come after derive_must_haves'); + }); + + test('reachability_check appears before estimate_scope', () => { + content = content || fs.readFileSync(plannerPath, 'utf-8'); + const reachabilityIdx = content.indexOf('reachability_check'); + const estimateIdx = content.indexOf('estimate_scope'); + assert.ok(estimateIdx > -1, 'estimate_scope step not found'); + assert.ok(reachabilityIdx < estimateIdx, 'reachability_check must come before estimate_scope'); + }); + + test('reachability_check includes creation path check', () => { + content = content || fs.readFileSync(plannerPath, 'utf-8'); + assert.ok(content.includes('creation path') || content.includes('creation_path') || content.includes('reachable'), + 'Missing creation path verification logic'); + }); + + test('reachability_check includes UNREACHABLE marker', () => { + content = content || fs.readFileSync(plannerPath, 'utf-8'); + assert.ok(content.includes('UNREACHABLE'), 'Missing UNREACHABLE marker for failed checks'); + }); + + test('file stays under 50000 char limit (CRLF-normalized)', () => { + content = content || fs.readFileSync(plannerPath, 'utf-8'); + const normalized = content.replace(/\r\n/g, '\n').replace(/\r/g, '\n'); + assert.ok(normalized.length < 50000, `File is ${normalized.length} chars, over the 50000 limit`); + }); +});