diff --git a/agents/gsd-debugger.md b/agents/gsd-debugger.md index 4482062b4..cc6d6efa6 100644 --- a/agents/gsd-debugger.md +++ b/agents/gsd-debugger.md @@ -31,6 +31,10 @@ If the prompt contains a `` block, you MUST use the `Read` tool t - Handle checkpoints when user input is unavoidable + +@~/.claude/get-shit-done/references/common-bug-patterns.md + + ## User = Reporter, Claude = Investigator diff --git a/get-shit-done/references/common-bug-patterns.md b/get-shit-done/references/common-bug-patterns.md index 5aae6260c..91850263b 100644 --- a/get-shit-done/references/common-bug-patterns.md +++ b/get-shit-done/references/common-bug-patterns.md @@ -6,85 +6,85 @@ Checklist of frequent bug patterns to scan before forming hypotheses. Ordered by ## Null / Undefined Access -- [ ] Accessing property on `null` or `undefined` — missing null check or optional chaining -- [ ] Function returns `undefined` instead of expected value — missing `return` statement or wrong branch -- [ ] Array/object destructuring on `null`/`undefined` — API returned error shape instead of data -- [ ] Optional parameter used without default — caller omitted argument +- **Null property access** — accessing property on `null` or `undefined`, missing null check or optional chaining +- **Missing return value** — function returns `undefined` instead of expected value, missing `return` statement or wrong branch +- **Destructuring null** — array/object destructuring on `null`/`undefined`, API returned error shape instead of data +- **Undefaulted optional** — optional parameter used without default, caller omitted argument ## Off-by-One / Boundary -- [ ] Loop starts at 1 instead of 0, or ends at `length` instead of `length - 1` -- [ ] Fence-post error — "N items need N-1 separators" miscounted -- [ ] Inclusive vs exclusive range boundary — `<` vs `<=`, slice/substring end index -- [ ] Empty collection not handled — `.length === 0` falls through to logic assuming items exist +- **Wrong loop bound** — loop starts at 1 instead of 0, or ends at `length` instead of `length - 1` +- **Fence-post error** — "N items need N-1 separators" miscounted +- **Inclusive vs exclusive** — range boundary `<` vs `<=`, slice/substring end index +- **Empty collection** — `.length === 0` falls through to logic assuming items exist ## Async / Timing -- [ ] Missing `await` on async function — gets Promise object instead of resolved value -- [ ] Race condition — two async operations read/write same state without coordination -- [ ] Stale closure — callback captures old variable value, not current one -- [ ] Event handler fires before setup complete — initialization order dependency -- [ ] Timeout/interval not cleaned up — fires after component/context destroyed +- **Missing await** — async function called without `await`, gets Promise object instead of resolved value +- **Race condition** — two async operations read/write same state without coordination +- **Stale closure** — callback captures old variable value, not current one +- **Initialization order** — event handler fires before setup complete +- **Leaked timer** — timeout/interval not cleaned up, fires after component/context destroyed ## State Management -- [ ] Mutating shared state — object/array modified in place affects other consumers -- [ ] State updated but UI not re-rendered — missing reactive trigger or wrong reference -- [ ] Stale state in event handler — closure captures state at bind time, not current value -- [ ] Multiple sources of truth — same data stored in two places, one gets out of sync -- [ ] State machine allows invalid transition — missing guard condition +- **Shared mutation** — object/array modified in place affects other consumers +- **Stale render** — state updated but UI not re-rendered, missing reactive trigger or wrong reference +- **Stale handler state** — closure captures state at bind time, not current value +- **Dual source of truth** — same data stored in two places, one gets out of sync +- **Invalid transition** — state machine allows transition missing guard condition ## Import / Module -- [ ] Circular dependency — module A imports B, B imports A, one gets `undefined` -- [ ] Default vs named export mismatch — `import X` vs `import { X }` -- [ ] Wrong file extension — `.js` vs `.cjs` vs `.mjs`, `.ts` vs `.tsx` -- [ ] Path case sensitivity — works on Windows/macOS, fails on Linux -- [ ] Missing file extension in import — ESM requires explicit extensions +- **Circular dependency** — module A imports B, B imports A, one gets `undefined` +- **Export mismatch** — default vs named export, `import X` vs `import { X }` +- **Wrong extension** — `.js` vs `.cjs` vs `.mjs`, `.ts` vs `.tsx` +- **Path case sensitivity** — works on Windows/macOS, fails on Linux +- **Missing extension** — ESM requires explicit file extensions in imports ## Type / Coercion -- [ ] String vs number comparison — `"5" > "10"` is `true` (lexicographic), `5 > 10` is `false` -- [ ] Implicit type coercion — `==` instead of `===`, truthy/falsy surprises (`0`, `""`, `[]`) -- [ ] Integer overflow or floating point — `0.1 + 0.2 !== 0.3`, large numbers lose precision -- [ ] Boolean vs truthy check — value is `0` or `""` which is valid but falsy +- **String vs number compare** — `"5" > "10"` is `true` (lexicographic), `5 > 10` is `false` +- **Implicit coercion** — `==` instead of `===`, truthy/falsy surprises (`0`, `""`, `[]`) +- **Numeric precision** — `0.1 + 0.2 !== 0.3`, large integers lose precision +- **Falsy valid value** — value is `0` or `""` which is valid but falsy ## Environment / Config -- [ ] Environment variable missing or wrong — different value in dev vs prod vs CI -- [ ] Hardcoded path or URL — works on one machine, fails on another -- [ ] Port already in use — previous process still running -- [ ] File permission denied — different user/group in deployment -- [ ] Missing dependency — not in package.json or not installed +- **Missing env var** — environment variable missing or wrong value in dev vs prod vs CI +- **Hardcoded path** — works on one machine, fails on another +- **Port conflict** — port already in use, previous process still running +- **Permission denied** — different user/group in deployment +- **Missing dependency** — not in package.json or not installed ## Data Shape / API Contract -- [ ] API response shape changed — backend updated, frontend expects old format -- [ ] Array where object expected (or vice versa) — `data` vs `data.results` vs `data[0]` -- [ ] Missing field in payload — required field omitted, backend returns validation error -- [ ] Date/time format mismatch — ISO string vs timestamp vs locale string -- [ ] Encoding mismatch — UTF-8 vs Latin-1, URL encoding, HTML entities +- **Changed response shape** — backend updated, frontend expects old format +- **Wrong container type** — array where object expected or vice versa, `data` vs `data.results` vs `data[0]` +- **Missing required field** — required field omitted in payload, backend returns validation error +- **Date format mismatch** — ISO string vs timestamp vs locale string +- **Encoding mismatch** — UTF-8 vs Latin-1, URL encoding, HTML entities ## Regex / String -- [ ] Regex `g` flag with `.test()` then `.exec()` — `lastIndex` not reset between calls -- [ ] Missing escape — `.` matches any char, `$` is special, backslash needs doubling -- [ ] Greedy match captures too much — `.*` eats through delimiters, need `.*?` -- [ ] String interpolation in wrong quote type — template literals need backticks +- **Sticky lastIndex** — regex `g` flag with `.test()` then `.exec()`, `lastIndex` not reset between calls +- **Missing escape** — `.` matches any char, `$` is special, backslash needs doubling +- **Greedy overmatch** — `.*` eats through delimiters, need `.*?` +- **Wrong quote type** — string interpolation needs backticks for template literals ## Error Handling -- [ ] Catch block swallows error — empty `catch {}` or logs but doesn't rethrow/handle -- [ ] Wrong error type caught — catches base `Error` when specific type needed -- [ ] Error in error handler — cleanup code throws, masking original error -- [ ] Promise rejection unhandled — missing `.catch()` or try/catch around `await` +- **Swallowed error** — empty `catch {}` or logs but doesn't rethrow/handle +- **Wrong error type** — catches base `Error` when specific type needed +- **Error in handler** — cleanup code throws, masking original error +- **Unhandled rejection** — missing `.catch()` or try/catch around `await` ## Scope / Closure -- [ ] Variable shadowing — inner scope declares same name, hides outer variable -- [ ] Loop variable capture — all closures share same `var i`, use `let` or bind -- [ ] `this` binding lost — callback loses context, need `.bind()` or arrow function -- [ ] Block scope vs function scope — `var` hoisted to function, `let`/`const` block-scoped +- **Variable shadowing** — inner scope declares same name, hides outer variable +- **Loop variable capture** — all closures share same `var i`, use `let` or bind +- **Lost this binding** — callback loses context, need `.bind()` or arrow function +- **Scope confusion** — `var` hoisted to function, `let`/`const` block-scoped diff --git a/tests/bug-patterns-reference.test.cjs b/tests/bug-patterns-reference.test.cjs new file mode 100644 index 000000000..2593d0c06 --- /dev/null +++ b/tests/bug-patterns-reference.test.cjs @@ -0,0 +1,110 @@ +/** + * Common Bug Patterns Reference Tests + * + * Structural tests for the common-bug-patterns.md reference file: + * - File exists at expected path + * - Contains expected bug pattern categories (at least 5 of 10) + * - Debugger agent references the file in required_reading + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const REFERENCE_PATH = path.join( + __dirname, '..', 'get-shit-done', 'references', 'common-bug-patterns.md' +); +const DEBUGGER_AGENT_PATH = path.join( + __dirname, '..', 'agents', 'gsd-debugger.md' +); + +const EXPECTED_CATEGORIES = [ + 'Off-by-One', + 'Null', + 'Async', + 'State Management', + 'Import', + 'Environment', + 'Data Shape', + 'String Handling', + 'File System', + 'Error Handling', +]; + +describe('common-bug-patterns.md reference', () => { + test('reference file exists', () => { + assert.ok( + fs.existsSync(REFERENCE_PATH), + `Expected reference file at ${REFERENCE_PATH}` + ); + }); + + test('has title and intro', () => { + const content = fs.readFileSync(REFERENCE_PATH, 'utf-8'); + assert.ok( + content.startsWith('# Common Bug Patterns'), + 'File should start with "# Common Bug Patterns" title' + ); + assert.ok( + content.includes('---'), + 'File should contain --- separator after intro' + ); + }); + + test('contains at least 5 of 10 expected categories', () => { + const content = fs.readFileSync(REFERENCE_PATH, 'utf-8'); + const found = EXPECTED_CATEGORIES.filter(cat => + content.toLowerCase().includes(cat.toLowerCase()) + ); + assert.ok( + found.length >= 5, + `Expected at least 5 categories, found ${found.length}: ${found.join(', ')}` + ); + }); + + test('each pattern category has at least one bold bullet item', () => { + const content = fs.readFileSync(REFERENCE_PATH, 'utf-8'); + // Only check sections inside block, not + const patternsBlock = (content.split('')[1] || '').split('')[0]; + const sections = patternsBlock.split(/^## /m).slice(1); + assert.ok(sections.length >= 5, `Expected at least 5 pattern sections, got ${sections.length}`); + for (const section of sections) { + const title = section.split('\n')[0].trim(); + const bullets = section.match(/^- \*\*/gm); + assert.ok( + bullets && bullets.length >= 1, + `Pattern section "${title}" should have at least one "- **" bullet item` + ); + } + }); +}); + +describe('debugger agent references bug patterns', () => { + test('gsd-debugger.md exists', () => { + assert.ok( + fs.existsSync(DEBUGGER_AGENT_PATH), + `Expected debugger agent at ${DEBUGGER_AGENT_PATH}` + ); + }); + + test('gsd-debugger.md references common-bug-patterns.md', () => { + const content = fs.readFileSync(DEBUGGER_AGENT_PATH, 'utf-8'); + assert.ok( + content.includes('common-bug-patterns.md'), + 'Debugger agent should reference common-bug-patterns.md' + ); + }); + + test('reference is inside block', () => { + const content = fs.readFileSync(DEBUGGER_AGENT_PATH, 'utf-8'); + const reqReadMatch = content.match( + /([\s\S]*?)<\/required_reading>/ + ); + assert.ok(reqReadMatch, 'Debugger agent should have a block'); + assert.ok( + reqReadMatch[1].includes('common-bug-patterns.md'), + 'common-bug-patterns.md should be inside block' + ); + }); +});