diff --git a/src/resources/extensions/gsd/captures.ts b/src/resources/extensions/gsd/captures.ts index 72447876e..645d907f6 100644 --- a/src/resources/extensions/gsd/captures.ts +++ b/src/resources/extensions/gsd/captures.ts @@ -26,6 +26,7 @@ export interface CaptureEntry { resolution?: string; rationale?: string; resolvedAt?: string; + resolvedInMilestone?: string; executed?: boolean; } @@ -176,6 +177,7 @@ export function markCaptureResolved( classification: Classification, resolution: string, rationale: string, + milestoneId?: string, ): void { const filePath = resolveCapturesPath(basePath); if (!existsSync(filePath)) return; @@ -206,13 +208,17 @@ export function markCaptureResolved( `**Rationale:** ${rationale}`, `**Resolved:** ${resolvedAt}`, ]; + if (milestoneId) { + newFields.push(`**Milestone:** ${milestoneId}`); + } - // Remove any existing classification/resolution/rationale/resolved fields + // Remove any existing classification/resolution/rationale/resolved/milestone fields // (in case of re-triage) section = section.replace(/\*\*Classification:\*\*\s*.+\n?/g, ""); section = section.replace(/\*\*Resolution:\*\*\s*.+\n?/g, ""); section = section.replace(/\*\*Rationale:\*\*\s*.+\n?/g, ""); section = section.replace(/\*\*Resolved:\*\*\s*.+\n?/g, ""); + section = section.replace(/\*\*Milestone:\*\*\s*.+\n?/g, ""); // Add new fields after Status line section = section.trimEnd() + "\n" + newFields.join("\n") + "\n"; @@ -255,18 +261,70 @@ export function markCaptureExecuted(basePath: string, captureId: string): void { * Load resolved captures that have actionable classifications (inject, replan, * quick-task) but have NOT yet been executed. * These are captures whose resolutions need to be carried out. + * + * When `currentMilestoneId` is provided, captures resolved in a *different* + * milestone are treated as stale and excluded. This prevents quick-task + * captures from a prior milestone re-executing after the underlying issues + * were already fixed by planned milestone work (#2872). + * + * Captures that have no `resolvedInMilestone` (legacy captures resolved before + * this field was introduced) are always included for backward compatibility. */ -export function loadActionableCaptures(basePath: string): CaptureEntry[] { +export function loadActionableCaptures(basePath: string, currentMilestoneId?: string): CaptureEntry[] { return loadAllCaptures(basePath).filter( c => c.status === "resolved" && !c.executed && (c.classification === "inject" || c.classification === "replan" || - c.classification === "quick-task"), + c.classification === "quick-task") && + // Staleness gate: exclude captures resolved in a different milestone (#2872) + (!currentMilestoneId || + !c.resolvedInMilestone || + c.resolvedInMilestone === currentMilestoneId), ); } +/** + * Retroactively stamp a capture with a milestone ID. + * + * Used by executeTriageResolutions() as a safety net when the triage LLM + * resolves a capture without writing the **Milestone:** field. This ensures + * the staleness gate in loadActionableCaptures() works correctly even for + * captures resolved before the prompt was updated (#2872). + */ +export function stampCaptureMilestone(basePath: string, captureId: string, milestoneId: string): void { + const filePath = resolveCapturesPath(basePath); + if (!existsSync(filePath)) return; + + const content = readFileSync(filePath, "utf-8"); + + const sectionRegex = new RegExp( + `(### ${escapeRegex(captureId)}\\n(?:(?!### ).)*?)(?=### |$)`, + "s", + ); + const match = sectionRegex.exec(content); + if (!match) return; + + let section = match[1]; + + // Only stamp if not already present + if (/\*\*Milestone:\*\*/.test(section)) return; + + // Insert after the Resolved field (or at end of section) + const resolvedFieldEnd = section.search(/\*\*Resolved:\*\*\s*.+\n?/); + if (resolvedFieldEnd !== -1) { + const resolvedMatch = section.match(/\*\*Resolved:\*\*\s*.+\n?/); + const insertPos = resolvedFieldEnd + (resolvedMatch?.[0]?.length ?? 0); + section = section.slice(0, insertPos) + `**Milestone:** ${milestoneId}\n` + section.slice(insertPos); + } else { + section = section.trimEnd() + "\n" + `**Milestone:** ${milestoneId}` + "\n"; + } + + const updated = content.replace(sectionRegex, section); + writeFileSync(filePath, updated, "utf-8"); +} + // ─── Parser ─────────────────────────────────────────────────────────────────── /** @@ -291,6 +349,7 @@ function parseCapturesContent(content: string): CaptureEntry[] { const resolution = extractBoldField(body, "Resolution"); const rationale = extractBoldField(body, "Rationale"); const resolvedAt = extractBoldField(body, "Resolved"); + const milestoneId = extractBoldField(body, "Milestone"); const executedAt = extractBoldField(body, "Executed"); if (!text || !timestamp) continue; @@ -308,6 +367,7 @@ function parseCapturesContent(content: string): CaptureEntry[] { ...(resolution ? { resolution } : {}), ...(rationale ? { rationale } : {}), ...(resolvedAt ? { resolvedAt } : {}), + ...(milestoneId ? { resolvedInMilestone: milestoneId } : {}), ...(executedAt ? { executed: true } : {}), }); } diff --git a/src/resources/extensions/gsd/prompts/triage-captures.md b/src/resources/extensions/gsd/prompts/triage-captures.md index 60dd5ca95..23545c265 100644 --- a/src/resources/extensions/gsd/prompts/triage-captures.md +++ b/src/resources/extensions/gsd/prompts/triage-captures.md @@ -54,6 +54,7 @@ For each capture, classify it as one of: - Add `**Resolution:** ` - Add `**Rationale:** ` - Add `**Resolved:** ` + - Add `**Milestone:** ` (e.g., `**Milestone:** M003`) 4. **Summarize** what was triaged: how many captures, what classifications were assigned, and what actions are pending (e.g., "2 quick-tasks ready for execution, 1 deferred to S03"). diff --git a/src/resources/extensions/gsd/tests/captures.test.ts b/src/resources/extensions/gsd/tests/captures.test.ts index 2e6618604..e8497e6fc 100644 --- a/src/resources/extensions/gsd/tests/captures.test.ts +++ b/src/resources/extensions/gsd/tests/captures.test.ts @@ -19,8 +19,11 @@ import { appendCapture, loadAllCaptures, loadPendingCaptures, + loadActionableCaptures, hasPendingCaptures, markCaptureResolved, + markCaptureExecuted, + stampCaptureMilestone, resolveCapturesPath, parseTriageOutput, } from "../captures.ts"; @@ -419,3 +422,103 @@ test("triage: parseTriageOutput preserves affectedFiles and targetSlice", () => assert.strictEqual(results[1].targetSlice, "S04"); assert.strictEqual(results[1].affectedFiles, undefined); }); + +// ─── Stale Quick-Task Captures (#2872) ──────────────────────────────────────── + +test("captures: markCaptureResolved stores milestone ID when provided", (t) => { + const tmp = makeTempDir("cap-milestone"); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + const id = appendCapture(tmp, "fix dialog width"); + markCaptureResolved(tmp, id, "quick-task", "widen the dialog", "small fix", "M003"); + + const all = loadAllCaptures(tmp); + assert.strictEqual(all.length, 1); + assert.strictEqual(all[0].resolvedInMilestone, "M003", "should store milestone ID"); +}); + +test("captures: loadActionableCaptures excludes captures resolved in prior milestones", (t) => { + const tmp = makeTempDir("cap-stale-filter"); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + // Capture resolved in M003 (prior milestone) + const id1 = appendCapture(tmp, "dialog too narrow"); + markCaptureResolved(tmp, id1, "quick-task", "widen it", "small fix", "M003"); + + // Capture resolved in M004 (current milestone) + const id2 = appendCapture(tmp, "button misaligned"); + markCaptureResolved(tmp, id2, "quick-task", "fix alignment", "css fix", "M004"); + + // Capture resolved without milestone context (legacy) + const id3 = appendCapture(tmp, "typo in label"); + markCaptureResolved(tmp, id3, "quick-task", "fix typo", "trivial"); + + // When loading for M004, only M004 and no-milestone captures should be returned + const actionable = loadActionableCaptures(tmp, "M004"); + const ids = actionable.map(c => c.id); + + assert.ok(!ids.includes(id1), "should exclude capture resolved in M003"); + assert.ok(ids.includes(id2), "should include capture resolved in M004"); + assert.ok(ids.includes(id3), "should include capture with no milestone (legacy)"); +}); + +test("captures: loadActionableCaptures without milestone returns all actionable", (t) => { + const tmp = makeTempDir("cap-no-milestone-filter"); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + const id1 = appendCapture(tmp, "issue one"); + markCaptureResolved(tmp, id1, "quick-task", "fix it", "small", "M003"); + + const id2 = appendCapture(tmp, "issue two"); + markCaptureResolved(tmp, id2, "inject", "inject it", "needed", "M004"); + + // Without milestone filter, all actionable captures are returned (backward compat) + const actionable = loadActionableCaptures(tmp); + assert.strictEqual(actionable.length, 2, "should return all actionable without filter"); +}); + +test("captures: loadActionableCaptures excludes already-executed captures", (t) => { + const tmp = makeTempDir("cap-executed-filter"); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + const id1 = appendCapture(tmp, "already done"); + markCaptureResolved(tmp, id1, "quick-task", "fix it", "small", "M004"); + markCaptureExecuted(tmp, id1); + + const id2 = appendCapture(tmp, "still pending"); + markCaptureResolved(tmp, id2, "quick-task", "fix it too", "small", "M004"); + + const actionable = loadActionableCaptures(tmp, "M004"); + assert.strictEqual(actionable.length, 1, "should exclude executed capture"); + assert.strictEqual(actionable[0].id, id2); +}); + +test("captures: stampCaptureMilestone adds milestone to capture missing it", (t) => { + const tmp = makeTempDir("cap-stamp-milestone"); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + const id = appendCapture(tmp, "fix alignment"); + markCaptureResolved(tmp, id, "quick-task", "fix it", "small"); + + // Before stamping, no milestone + let all = loadAllCaptures(tmp); + assert.strictEqual(all[0].resolvedInMilestone, undefined, "should have no milestone initially"); + + stampCaptureMilestone(tmp, id, "M004"); + + all = loadAllCaptures(tmp); + assert.strictEqual(all[0].resolvedInMilestone, "M004", "should have milestone after stamping"); +}); + +test("captures: stampCaptureMilestone is no-op if milestone already present", (t) => { + const tmp = makeTempDir("cap-stamp-noop"); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + const id = appendCapture(tmp, "fix alignment"); + markCaptureResolved(tmp, id, "quick-task", "fix it", "small", "M003"); + + stampCaptureMilestone(tmp, id, "M004"); + + const all = loadAllCaptures(tmp); + assert.strictEqual(all[0].resolvedInMilestone, "M003", "should keep original milestone"); +}); diff --git a/src/resources/extensions/gsd/tests/triage-resolution.test.ts b/src/resources/extensions/gsd/tests/triage-resolution.test.ts index 496685732..deb924347 100644 --- a/src/resources/extensions/gsd/tests/triage-resolution.test.ts +++ b/src/resources/extensions/gsd/tests/triage-resolution.test.ts @@ -212,6 +212,14 @@ test("resolution: buildQuickTaskPrompt includes capture text and ID", () => { assert.ok(prompt.includes("add retry logic to OAuth"), "should include capture text"); assert.ok(prompt.includes("Quick Task"), "should have Quick Task header"); assert.ok(prompt.includes("Do NOT modify"), "should warn about plan files"); + assert.ok( + prompt.includes("Verify the issue still exists"), + "should instruct agent to verify issue still exists (#2872)", + ); + assert.ok( + prompt.includes("Already resolved"), + "should instruct agent to report already resolved if fixed (#2872)", + ); }); // ─── markCaptureExecuted ───────────────────────────────────────────────────── diff --git a/src/resources/extensions/gsd/triage-resolution.ts b/src/resources/extensions/gsd/triage-resolution.ts index eefb2caa8..d6aaad962 100644 --- a/src/resources/extensions/gsd/triage-resolution.ts +++ b/src/resources/extensions/gsd/triage-resolution.ts @@ -22,6 +22,7 @@ import { loadActionableCaptures, markCaptureResolved, markCaptureExecuted, + stampCaptureMilestone, } from "./captures.js"; // ─── Resolution Executors ───────────────────────────────────────────────────── @@ -271,11 +272,15 @@ export function buildQuickTaskPrompt(capture: CaptureEntry): string { ``, `## Instructions`, ``, - `1. Execute this task as a small, self-contained change.`, - `2. Do NOT modify any \`.gsd/\` plan files — this is a one-off, not a planned task.`, - `3. Commit your changes with a descriptive message.`, - `4. Keep changes minimal and focused on the capture text.`, - `5. When done, say: "Quick task complete."`, + `1. **Verify the issue still exists.** Before making any changes, inspect the`, + ` relevant code to confirm the problem described above is actually present in`, + ` the current codebase. If the issue has already been fixed (e.g., by planned`, + ` milestone work), report "Already resolved — no changes needed." and stop.`, + `2. Execute this task as a small, self-contained change.`, + `3. Do NOT modify any \`.gsd/\` plan files — this is a one-off, not a planned task.`, + `4. Commit your changes with a descriptive message.`, + `5. Keep changes minimal and focused on the capture text.`, + `6. When done, say: "Quick task complete."`, ].join("\n"); } @@ -324,7 +329,19 @@ export function executeTriageResolutions( actions: [], }; - const actionable = loadActionableCaptures(basePath); + const actionable = loadActionableCaptures(basePath, mid || undefined); + + // Reconciliation: stamp actionable captures that are missing the Milestone field + // with the current milestone ID. This covers captures resolved by the triage LLM + // before the prompt included the Milestone instruction, and acts as a safety net + // when the LLM omits the field (#2872). + if (mid) { + for (const capture of actionable) { + if (!capture.resolvedInMilestone) { + stampCaptureMilestone(basePath, capture.id, mid); + } + } + } // Also process deferred captures that target milestone IDs — create // milestone directories so deriveState() discovers them.