fix: invalidate stale quick-task captures across milestone boundaries (#3084)
Closes #2872 Quick-task captures resolved in a prior milestone were re-executed in subsequent sessions because loadActionableCaptures() used the Executed flag as its sole staleness gate. Milestone completion never marked captures as executed, so captures whose issues were fixed by planned work remained permanently actionable. Three changes fix this: 1. Track which milestone a capture was resolved in (new **Milestone:** field in CAPTURES.md, written by markCaptureResolved and the triage prompt). loadActionableCaptures() now accepts an optional currentMilestoneId and excludes captures from prior milestones. 2. Add a verification step to buildQuickTaskPrompt() instructing the agent to confirm the issue still exists before making changes. 3. Add stampCaptureMilestone() as a reconciliation safety net -- executeTriageResolutions() stamps actionable captures that are missing the Milestone field with the current milestone ID. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
6e22a20580
commit
a9d881ad8c
5 changed files with 198 additions and 9 deletions
|
|
@ -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 } : {}),
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,6 +54,7 @@ For each capture, classify it as one of:
|
|||
- Add `**Resolution:** <brief description of what will happen>`
|
||||
- Add `**Rationale:** <why this classification>`
|
||||
- Add `**Resolved:** <current ISO timestamp>`
|
||||
- Add `**Milestone:** <current milestone ID>` (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").
|
||||
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 ─────────────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue