From ab5ad6c8bc4a273d34b9e40096fa1154b060b23f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 26 Apr 2026 21:40:38 -0400 Subject: [PATCH] fix(#2757): unquoted truths with colons crash annotate-dependencies (#2759) parseMustHavesBlock dispatched on `includes(':')` to detect key-value pairs, but unquoted YAML strings like `GET /foo/:id resolves...` and `Class::Method is idempotent` also contain colons. When the KV regex failed to match, `current` was left as `{}` (the empty object initialized before the branch), which then caused `t.trim()` in roadmap.cjs to throw `TypeError: t.trim is not a function`. Two fixes: - frontmatter.cjs: tighten the KV regex to require at least one space after the colon (`\s+` instead of `\s*`), matching YAML convention. When the regex still fails to match, fall back to treating the item as a plain string instead of leaving `current` as `{}`. - roadmap.cjs: add `typeof t !== 'string'` guard before `.trim()` as a cheap safety net against any future parser anomaly. Closes #2757 Co-authored-by: Claude Sonnet 4.6 --- get-shit-done/bin/lib/frontmatter.cjs | 8 ++++++-- get-shit-done/bin/lib/roadmap.cjs | 1 + tests/enh-2447-roadmap-wave-deps.test.cjs | 19 +++++++++++++++++++ tests/frontmatter.test.cjs | 21 +++++++++++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/get-shit-done/bin/lib/frontmatter.cjs b/get-shit-done/bin/lib/frontmatter.cjs index 8879611cd..c3865cc31 100644 --- a/get-shit-done/bin/lib/frontmatter.cjs +++ b/get-shit-done/bin/lib/frontmatter.cjs @@ -242,7 +242,6 @@ function parseMustHavesBlock(content, blockName) { // Only treat as a top-level list item if at the expected indent if (indent === listItemIndent) { if (current) items.push(current); - current = {}; const afterDash = trimmed.slice(2); const trimmedAfterDash = afterDash.trim(); // Check if it's a fully-quoted string (may contain ':' inside the quotes) @@ -254,10 +253,15 @@ function parseMustHavesBlock(content, blockName) { current = afterDash.replace(/^["']|["']$/g, ''); } else { // Key-value on same line as dash: "- path: value" - const kvMatch = afterDash.match(/^(\w+):\s*"?([^"]*)"?\s*$/); + // YAML KV always has at least one space after the colon: "key: value" + // Requiring \s+ rejects "Class::Method" and "db:seed" (no space after colon) + const kvMatch = afterDash.match(/^(\w+):\s+"?([^"]*)"?\s*$/); if (kvMatch) { current = {}; current[kvMatch[1]] = kvMatch[2]; + } else { + // Looks like KV but doesn't match — treat as plain string (#2757) + current = afterDash.replace(/^["']|["']$/g, ''); } } continue; diff --git a/get-shit-done/bin/lib/roadmap.cjs b/get-shit-done/bin/lib/roadmap.cjs index eaff86481..3d48ed2b7 100644 --- a/get-shit-done/bin/lib/roadmap.cjs +++ b/get-shit-done/bin/lib/roadmap.cjs @@ -417,6 +417,7 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) { for (const { truths } of planData) { const seen = new Set(); for (const t of truths) { + if (typeof t !== 'string') continue; const key = t.trim().toLowerCase(); if (!key || seen.has(key)) continue; seen.add(key); diff --git a/tests/enh-2447-roadmap-wave-deps.test.cjs b/tests/enh-2447-roadmap-wave-deps.test.cjs index 0a35add86..cbdd9c642 100644 --- a/tests/enh-2447-roadmap-wave-deps.test.cjs +++ b/tests/enh-2447-roadmap-wave-deps.test.cjs @@ -194,6 +194,25 @@ Plans: assert.strictEqual(out.updated, false); }); + test('#2757: truths containing colons do not crash annotate-dependencies', () => { + // Unquoted truths with colons (Rails idioms: db:seed, /foo/:id, Class::Method) + // caused parseMustHavesBlock to return {} instead of a string, then t.trim() threw. + const colonTruths = [ + 'GET /foo/:id resolves to controller#show', + 'Class::Method is idempotent', + '"Quoted truth with colon: inside"', + ]; + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': `# Roadmap\n\n### Phase 1: Foundation\n**Goal:** Set up project\n**Plans:** 1 plan\n\nPlans:\n- [ ] 01-01-PLAN.md — Repro plan\n`, + '.planning/phases/01-foundation/01-01-PLAN.md': PLAN_TEMPLATE(1, colonTruths), + }); + + const result = runGsdTools('roadmap annotate-dependencies 1', tmpDir); + assert.ok(result.success, `Command threw on colon-containing truths: ${result.error}`); + const out = JSON.parse(result.output); + assert.ok(typeof out.updated === 'boolean', 'should return a valid result object'); + }); + test('plan-phase.md documents annotate-dependencies step', () => { const planPhase = fs.readFileSync( path.join(__dirname, '../get-shit-done/workflows/plan-phase.md'), 'utf-8' diff --git a/tests/frontmatter.test.cjs b/tests/frontmatter.test.cjs index 5925ea602..e7ecab2ba 100644 --- a/tests/frontmatter.test.cjs +++ b/tests/frontmatter.test.cjs @@ -547,6 +547,27 @@ must_haves: assert.strictEqual(result[0], 'Key: value pattern preserved'); }); + test('#2757: unquoted truth containing ":" is preserved as a string — not left as {}', () => { + // Unquoted strings with colons (e.g. Rails idioms) were falling through the KV + // regex and leaving current as {}, which caused t.trim() to throw in roadmap.cjs. + const content = `--- +phase: 01 +must_haves: + truths: + - GET /foo/:id resolves to controller#show + - Service.call(arg:, key:) returns a record + - Class::Method is idempotent +--- +`; + const result = parseMustHavesBlock(content, 'truths'); + assert.ok(Array.isArray(result), 'should return an array'); + assert.strictEqual(result.length, 3, `expected 3, got ${result.length}: ${JSON.stringify(result)}`); + assert.ok(typeof result[0] === 'string', `result[0] should be string, got ${typeof result[0]}`); + assert.ok(typeof result[1] === 'string', `result[1] should be string, got ${typeof result[1]}`); + assert.ok(typeof result[2] === 'string', `result[2] should be string, got ${typeof result[2]}`); + assert.ok(result[0].includes(':'), 'colon should be preserved in the string'); + }); + test('handles nested arrays within artifact objects', () => { const content = `--- phase: 01