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 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-26 21:40:38 -04:00
committed by GitHub
parent 1a230e69aa
commit ab5ad6c8bc
4 changed files with 47 additions and 2 deletions

View File

@@ -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;

View File

@@ -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);

View File

@@ -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'

View File

@@ -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