Fix: em dash and slash in milestone/slice titles corrupt GSD state management (#426)
* Initial plan * chore: establish baseline before implementing em-dash fix Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com> * fix: validate milestone titles against delimiter characters (em dash, slash) that break state management - Changed STATE.md separator from em dash to colon in buildStateMarkdown and state.md template - Removed ambiguous '— Context' suffix from context.md H1 template - Added validateTitle() function to detect problematic delimiter characters - Added delimiter_in_title doctor issue code for milestone/slice title validation - Added tests for validateTitle() and doctor delimiter detection - Added em-dash-in-title cases to regex-hardening test Fixes: milestone titles containing '—' caused state corruption when the LLM misread the ambiguous STATE.md separator format and wrote incorrect planning files. Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com>
This commit is contained in:
Copilot
GitHub
parent
ecf8125e39
commit
6d2ff3d4a5
5 changed files with 191 additions and 8 deletions
|
|
@ -22,7 +22,8 @@ export type DoctorIssueCode =
|
|||
| "task_done_must_haves_not_verified"
|
||||
| "active_requirement_missing_owner"
|
||||
| "blocked_requirement_missing_reason"
|
||||
| "blocker_discovered_no_replan";
|
||||
| "blocker_discovered_no_replan"
|
||||
| "delimiter_in_title";
|
||||
|
||||
export interface DoctorIssue {
|
||||
severity: DoctorSeverity;
|
||||
|
|
@ -91,15 +92,43 @@ function validatePreferenceShape(preferences: GSDPreferences): string[] {
|
|||
return issues;
|
||||
}
|
||||
|
||||
/**
|
||||
* Characters that are used as delimiters in GSD state management documents
|
||||
* and should not appear in milestone or slice titles.
|
||||
*
|
||||
* - "—" (em dash, U+2014): used as a display separator in STATE.md and other docs.
|
||||
* A title containing "—" makes the separator ambiguous, corrupting state display
|
||||
* and confusing the LLM agent that reads and writes these files.
|
||||
* - "–" (en dash, U+2013): visually similar to em dash; same ambiguity risk.
|
||||
* - "/" (forward slash, U+002F): used as the path separator in unit IDs (M001/S01)
|
||||
* and git branch names (gsd/M001/S01). A slash in a title can break path resolution.
|
||||
*/
|
||||
const TITLE_DELIMITER_RE = /[\u2014\u2013\/]/; // em dash, en dash, forward slash
|
||||
|
||||
/**
|
||||
* Check whether a milestone or slice title contains characters that conflict
|
||||
* with GSD's state document delimiter conventions.
|
||||
* Returns a human-readable description of the problem, or null if the title is safe.
|
||||
*/
|
||||
export function validateTitle(title: string): string | null {
|
||||
if (TITLE_DELIMITER_RE.test(title)) {
|
||||
const found: string[] = [];
|
||||
if (/[\u2014\u2013]/.test(title)) found.push("em/en dash (\u2014 or \u2013)");
|
||||
if (/\//.test(title)) found.push("forward slash (/)");
|
||||
return `title contains ${found.join(" and ")}, which conflict with GSD state document delimiters`;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function buildStateMarkdown(state: Awaited<ReturnType<typeof deriveState>>): string {
|
||||
const lines: string[] = [];
|
||||
lines.push("# GSD State", "");
|
||||
|
||||
const activeMilestone = state.activeMilestone
|
||||
? `${state.activeMilestone.id} — ${state.activeMilestone.title}`
|
||||
? `${state.activeMilestone.id}: ${state.activeMilestone.title}`
|
||||
: "None";
|
||||
const activeSlice = state.activeSlice
|
||||
? `${state.activeSlice.id} — ${state.activeSlice.title}`
|
||||
? `${state.activeSlice.id}: ${state.activeSlice.title}`
|
||||
: "None";
|
||||
|
||||
lines.push(`**Active Milestone:** ${activeMilestone}`);
|
||||
|
|
@ -477,6 +506,20 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
|
|||
const milestonePath = resolveMilestonePath(basePath, milestoneId);
|
||||
if (!milestonePath) continue;
|
||||
|
||||
// Validate milestone title for delimiter characters that break state documents.
|
||||
const milestoneTitleIssue = validateTitle(milestone.title);
|
||||
if (milestoneTitleIssue) {
|
||||
issues.push({
|
||||
severity: "warning",
|
||||
code: "delimiter_in_title",
|
||||
scope: "milestone",
|
||||
unitId: milestoneId,
|
||||
message: `Milestone ${milestoneId} ${milestoneTitleIssue}. Rename the milestone to remove these characters to prevent state corruption.`,
|
||||
file: relMilestoneFile(basePath, milestoneId, "ROADMAP"),
|
||||
fixable: false,
|
||||
});
|
||||
}
|
||||
|
||||
const roadmapPath = resolveMilestoneFile(basePath, milestoneId, "ROADMAP");
|
||||
const roadmapContent = roadmapPath ? await loadFile(roadmapPath) : null;
|
||||
if (!roadmapContent) continue;
|
||||
|
|
@ -486,6 +529,20 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
|
|||
const unitId = `${milestoneId}/${slice.id}`;
|
||||
if (options?.scope && !matchesScope(unitId, options.scope) && options.scope !== milestoneId) continue;
|
||||
|
||||
// Validate slice title for delimiter characters.
|
||||
const sliceTitleIssue = validateTitle(slice.title);
|
||||
if (sliceTitleIssue) {
|
||||
issues.push({
|
||||
severity: "warning",
|
||||
code: "delimiter_in_title",
|
||||
scope: "slice",
|
||||
unitId,
|
||||
message: `Slice ${unitId} ${sliceTitleIssue}. Rename the slice to remove these characters to prevent state corruption.`,
|
||||
file: relMilestoneFile(basePath, milestoneId, "ROADMAP"),
|
||||
fixable: false,
|
||||
});
|
||||
}
|
||||
|
||||
const slicePath = resolveSlicePath(basePath, milestoneId, slice.id);
|
||||
if (!slicePath) continue;
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
# {{milestoneId}}: {{milestoneTitle}} — Context
|
||||
# {{milestoneId}}: {{milestoneTitle}}
|
||||
|
||||
**Gathered:** {{date}}
|
||||
**Status:** Ready for planning
|
||||
|
|
|
|||
|
|
@ -1,8 +1,8 @@
|
|||
# GSD State
|
||||
|
||||
**Active Milestone:** {{milestoneId}} — {{milestoneTitle}}
|
||||
**Active Slice:** {{sliceId}} — {{sliceTitle}}
|
||||
**Active Task:** {{taskId}} — {{taskTitle}}
|
||||
**Active Milestone:** {{milestoneId}}: {{milestoneTitle}}
|
||||
**Active Slice:** {{sliceId}}: {{sliceTitle}}
|
||||
**Active Task:** {{taskId}}: {{taskTitle}}
|
||||
**Phase:** {{phase}}
|
||||
**Slice Branch:** {{activeBranch}}
|
||||
**Active Workspace:** {{activeWorkspace}}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync, existsSync
|
|||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
import { formatDoctorReport, runGSDDoctor, summarizeDoctorIssues, filterDoctorIssues, selectDoctorScope } from "../doctor.js";
|
||||
import { formatDoctorReport, runGSDDoctor, summarizeDoctorIssues, filterDoctorIssues, selectDoctorScope, validateTitle } from "../doctor.js";
|
||||
import { createTestContext } from './test-helpers.ts';
|
||||
|
||||
const { assertEq, assertTrue, report } = createTestContext();
|
||||
|
|
@ -471,6 +471,120 @@ Discovered an issue.
|
|||
rmSync(mhBase, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ─── validateTitle: em dash and slash detection ────────────────────────
|
||||
console.log("\n=== validateTitle: returns null for clean titles ===");
|
||||
{
|
||||
assertEq(validateTitle("Foundation"), null, "clean title passes");
|
||||
assertEq(validateTitle("Build Core Systems"), null, "clean title with spaces passes");
|
||||
assertEq(validateTitle("API v2 Integration"), null, "clean title with version passes");
|
||||
assertEq(validateTitle(""), null, "empty title passes");
|
||||
}
|
||||
|
||||
console.log("\n=== validateTitle: detects em dash ===");
|
||||
{
|
||||
const result = validateTitle("Foundation — Build Core");
|
||||
assertTrue(result !== null, "detects em dash in title");
|
||||
assertTrue(result!.includes("em/en dash"), "message mentions em/en dash");
|
||||
}
|
||||
|
||||
console.log("\n=== validateTitle: detects en dash ===");
|
||||
{
|
||||
const result = validateTitle("Phase 1 – Phase 2");
|
||||
assertTrue(result !== null, "detects en dash in title");
|
||||
assertTrue(result!.includes("em/en dash"), "message mentions em/en dash for en dash");
|
||||
}
|
||||
|
||||
console.log("\n=== validateTitle: detects forward slash ===");
|
||||
{
|
||||
const result = validateTitle("Client/Server");
|
||||
assertTrue(result !== null, "detects forward slash in title");
|
||||
assertTrue(result!.includes("forward slash"), "message mentions forward slash");
|
||||
}
|
||||
|
||||
console.log("\n=== validateTitle: detects both em dash and slash ===");
|
||||
{
|
||||
const result = validateTitle("Client — Server/API");
|
||||
assertTrue(result !== null, "detects both delimiters");
|
||||
assertTrue(result!.includes("em/en dash"), "message mentions em/en dash");
|
||||
assertTrue(result!.includes("forward slash"), "message mentions forward slash");
|
||||
}
|
||||
|
||||
// ─── doctor detects delimiter_in_title for milestone ───────────────────
|
||||
console.log("\n=== doctor detects em dash in milestone title ===");
|
||||
{
|
||||
const dtBase = mkdtempSync(join(tmpdir(), "gsd-doctor-dt-test-"));
|
||||
const dtGsd = join(dtBase, ".gsd");
|
||||
const dtMDir = join(dtGsd, "milestones", "M001");
|
||||
const dtSDir = join(dtMDir, "slices", "S01");
|
||||
const dtTDir = join(dtSDir, "tasks");
|
||||
mkdirSync(dtTDir, { recursive: true });
|
||||
|
||||
// Roadmap with em dash in milestone title
|
||||
writeFileSync(join(dtMDir, "M001-ROADMAP.md"), `# M001: Foundation — Build Core\n\n## Slices\n- [ ] **S01: Demo Slice** \`risk:low\` \`depends:[]\`\n > After this: demo works\n`);
|
||||
writeFileSync(join(dtSDir, "S01-PLAN.md"), `# S01: Demo Slice\n\n**Goal:** Demo\n**Demo:** Demo\n\n## Tasks\n- [ ] **T01: Implement** \`est:10m\`\n Task.\n`);
|
||||
writeFileSync(join(dtTDir, "T01-PLAN.md"), `# T01: Implement\n\n## Steps\n\n1. Do the thing.\n`);
|
||||
|
||||
const report = await runGSDDoctor(dtBase, { fix: false });
|
||||
const dtIssues = report.issues.filter(i => i.code === "delimiter_in_title");
|
||||
assertTrue(dtIssues.length >= 1, "detects delimiter_in_title for milestone with em dash");
|
||||
const milestoneIssue = dtIssues.find(i => i.scope === "milestone");
|
||||
assertTrue(milestoneIssue !== undefined, "delimiter issue has milestone scope");
|
||||
assertEq(milestoneIssue?.severity, "warning", "delimiter issue has warning severity");
|
||||
assertEq(milestoneIssue?.unitId, "M001", "delimiter issue unitId is M001");
|
||||
assertTrue(milestoneIssue?.message?.includes("em/en dash") ?? false, "issue message mentions em/en dash");
|
||||
assertEq(milestoneIssue?.fixable, false, "delimiter issue is not auto-fixable");
|
||||
|
||||
rmSync(dtBase, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ─── doctor detects delimiter_in_title for slice ────────────────────────
|
||||
console.log("\n=== doctor detects em dash in slice title ===");
|
||||
{
|
||||
const dtBase = mkdtempSync(join(tmpdir(), "gsd-doctor-dt-slice-"));
|
||||
const dtGsd = join(dtBase, ".gsd");
|
||||
const dtMDir = join(dtGsd, "milestones", "M001");
|
||||
const dtSDir = join(dtMDir, "slices", "S01");
|
||||
const dtTDir = join(dtSDir, "tasks");
|
||||
mkdirSync(dtTDir, { recursive: true });
|
||||
|
||||
// Roadmap with em dash in slice title (milestone title is clean)
|
||||
writeFileSync(join(dtMDir, "M001-ROADMAP.md"), `# M001: Clean Milestone\n\n## Slices\n- [ ] **S01: Core — Foundation** \`risk:low\` \`depends:[]\`\n > After this: demo works\n`);
|
||||
writeFileSync(join(dtSDir, "S01-PLAN.md"), `# S01: Core — Foundation\n\n**Goal:** Demo\n**Demo:** Demo\n\n## Tasks\n- [ ] **T01: Implement** \`est:10m\`\n Task.\n`);
|
||||
writeFileSync(join(dtTDir, "T01-PLAN.md"), `# T01: Implement\n\n## Steps\n\n1. Do the thing.\n`);
|
||||
|
||||
const report = await runGSDDoctor(dtBase, { fix: false });
|
||||
const dtIssues = report.issues.filter(i => i.code === "delimiter_in_title");
|
||||
assertTrue(dtIssues.length >= 1, "detects delimiter_in_title for slice with em dash");
|
||||
const sliceIssue = dtIssues.find(i => i.scope === "slice");
|
||||
assertTrue(sliceIssue !== undefined, "delimiter issue has slice scope");
|
||||
assertEq(sliceIssue?.severity, "warning", "slice delimiter issue has warning severity");
|
||||
assertEq(sliceIssue?.unitId, "M001/S01", "slice delimiter issue unitId is M001/S01");
|
||||
|
||||
rmSync(dtBase, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ─── doctor does NOT flag clean titles ──────────────────────────────────
|
||||
console.log("\n=== doctor does NOT flag milestone with clean title ===");
|
||||
{
|
||||
const dtBase = mkdtempSync(join(tmpdir(), "gsd-doctor-dt-clean-"));
|
||||
const dtGsd = join(dtBase, ".gsd");
|
||||
const dtMDir = join(dtGsd, "milestones", "M001");
|
||||
const dtSDir = join(dtMDir, "slices", "S01");
|
||||
const dtTDir = join(dtSDir, "tasks");
|
||||
mkdirSync(dtTDir, { recursive: true });
|
||||
|
||||
// Roadmap with clean titles (no delimiters)
|
||||
writeFileSync(join(dtMDir, "M001-ROADMAP.md"), `# M001: Foundation Build Core\n\n## Slices\n- [ ] **S01: Demo Slice** \`risk:low\` \`depends:[]\`\n > After this: demo works\n`);
|
||||
writeFileSync(join(dtSDir, "S01-PLAN.md"), `# S01: Demo Slice\n\n**Goal:** Demo\n**Demo:** Demo\n\n## Tasks\n- [ ] **T01: Implement** \`est:10m\`\n Task.\n`);
|
||||
writeFileSync(join(dtTDir, "T01-PLAN.md"), `# T01: Implement\n\n## Steps\n\n1. Do the thing.\n`);
|
||||
|
||||
const report = await runGSDDoctor(dtBase, { fix: false });
|
||||
const dtIssues = report.issues.filter(i => i.code === "delimiter_in_title");
|
||||
assertEq(dtIssues.length, 0, "no delimiter_in_title issues for clean titles");
|
||||
|
||||
rmSync(dtBase, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
report();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -67,6 +67,18 @@ async function main(): Promise<void> {
|
|||
assertEq('M001-abc123: Title'.replace(TITLE_STRIP_RE, ''), 'Title', 'strips M001-abc123: Title → Title');
|
||||
assertEq('M042-z9a8b7: Dashboard'.replace(TITLE_STRIP_RE, ''), 'Dashboard', 'strips M042-z9a8b7: Dashboard');
|
||||
|
||||
// Em dash in title — current format (M001: Title) correctly preserves em dash in title body
|
||||
assertEq(
|
||||
'M001: Foundation — Build Core'.replace(TITLE_STRIP_RE, ''),
|
||||
'Foundation — Build Core',
|
||||
'strips M001: prefix and preserves em dash in title body',
|
||||
);
|
||||
assertEq(
|
||||
'M001-abc123: Foundation — Build Core'.replace(TITLE_STRIP_RE, ''),
|
||||
'Foundation — Build Core',
|
||||
'strips M001-abc123: prefix and preserves em dash in title body (unique format)',
|
||||
);
|
||||
|
||||
// Edge case: dash-style separator (M001 — Title: Subtitle preserves colon in body)
|
||||
assertEq(
|
||||
'M001 — Unique Milestone IDs: Foo'.replace(TITLE_STRIP_RE, ''),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue