feat(gsd-extension): carry forward pending actions into replanning#3413
feat(gsd-extension): carry forward pending actions into replanning#3413gunish wants to merge 1 commit intogsd-build:mainfrom
Conversation
🔴 PR Risk Report — CRITICAL
Affected Systems
File Breakdown
|
a341f2e to
dcc206d
Compare
trek-e
left a comment
There was a problem hiding this comment.
PR #3413 — feat: carry forward pending actions into replanning
Verdict: REQUEST_CHANGES
Severity: MAJOR + MINOR
MAJOR: Filesystem-path loop protection is asymmetric with DB-path loop protection
DB-backed path (deriveStateFromDb):
const alreadyHandled = replanHistory.some(
(entry) => String(entry["task_id"] ?? "") === latestPendingActionTask.taskId,
);
if (!alreadyHandled) { /* trigger replanning */ }Filesystem-backed path (_deriveStateImpl):
if (!replanFile && !alreadyHandled) { /* trigger replanning */ }The filesystem path has an extra !replanFile guard. If any REPLAN.md file exists on disk from a previous replan (for a different task), replanFile will be truthy and the pending-action check is silently skipped — even when the new pending action is from a different, later task with no matching replan_history entry. This is exactly the scenario the PR description says it handles: "a later completed task can trigger a new replan even if the slice has already been replanned for an earlier task." That claim is true for the DB path but false for the filesystem path. The !replanFile condition must be removed or the logic must match the DB path.
MAJOR: nativeParseSummaryFile fast-path now returns pendingActions parsed from pre-computed knownIssues, but knownIssues is derived before the native parser is called
const [fmLines, body] = splitFrontmatter(content);
const knownIssues = extractSection(body, 'Known Issues') || '';
const pendingActions = extractPendingActions(knownIssues);
const nativeResult = nativeParseSummaryFile(content);
if (nativeResult) {
return { ...nativeResult, knownIssues, pendingActions }; // ← pendingActions injected
}This is correct. The native parser returns the base fields; knownIssues and pendingActions are layered on top from the JS-side extraction. This is fine architecturally, but it means splitFrontmatter + extractSection are now always called even when the native parser succeeds — negating part of the native parser's performance benefit. This is a MINOR concern, not a correctness bug, but it should be documented.
MINOR: extractPendingActions regex requires the section to start with Pending actions: on its own line but the template shows Pending actions: without a preceding blank line guarantee
The regex:
/(?:^|\n)Pending actions:\s*\n((?:\s*[-*]\s+.+(?:\n|$))+)/iThis works when ## Known Issues section content starts directly with Pending actions:. However the template instructs authors to write prose above the Pending actions: block. If a user writes:
## Known Issues
Some known issue.
Pending actions:
- action 1
The regex still matches because (?:^|\n) anchors on the newline before Pending actions:. This is fine. Confirmed correct.
MINOR: getLatestPendingActionTask iterates completed tasks in reverse, but completedTaskIds ordering depends on DB/array ordering which may not be chronological
In deriveStateFromDb, completedTaskIds is derived as:
tasks.filter(t => isStatusDone(t.status)).map(t => t.id)The ordering of tasks from the DB query is not shown in this diff. If tasks are not returned in completion order, reverse() in getLatestPendingActionTask may examine the wrong task first and surface stale pending actions from an earlier task instead of the truly latest one. The PR should document or enforce that completedTaskIds is in insertion/completion order.
MINOR: No test for the filesystem-path (_deriveStateImpl) pending-action trigger
flag-file-db.test.ts tests the DB-backed path. The verification list mentions derive-state.test.ts but the diff only shows changes to flag-file-db.test.ts and pending-actions.test.ts. The filesystem-path asymmetric loop protection bug (above) is untested.
Summary: Fix the !replanFile asymmetry in _deriveStateImpl — that's the core correctness issue. The feature is well-structured otherwise.
TL;DR
What: Carry forward
Pending actions:from completed task summaries into task prompts and automatic slice replanning.Why: Required unfinished work can currently be documented in prose but still fail to become continuation state, which risks silent closeout or continuation without explicit replanning.
How: Extend summary parsing, surface pending actions in carry-forward context, and route both DB-backed and filesystem-backed state derivation into
replanning-slicewhen the latest completed task leaves required pending follow-up.What
This PR adds a lightweight durable contract for required unfinished work in completed task summaries:
With that in place, GSD now:
Pending actions:from completed task summariespending_actions: ...replanning-slicebefore silent continuation or summarizing when the latest completed task still has required follow-upblocker_discovered: true, orFiles changed:
src/resources/extensions/gsd/types.tssrc/resources/extensions/gsd/files.tssrc/resources/extensions/gsd/auto-prompts.tssrc/resources/extensions/gsd/state.tssrc/resources/extensions/gsd/prompts/execute-task.mdsrc/resources/extensions/gsd/prompts/guided-execute-task.mdsrc/resources/extensions/gsd/prompts/replan-slice.mdsrc/resources/extensions/gsd/templates/task-summary.mdsrc/resources/extensions/gsd/tests/flag-file-db.test.tssrc/resources/extensions/gsd/tests/pending-actions.test.tsWhy
GSD already has two strong seams here:
blocker_discovered: trueThe missing piece is a durable middle ground for tasks that are complete enough to close honestly, but still leave behind required follow-up work that must not disappear into prose.
Without that, the system can end up with one of two bad outcomes:
This PR makes that unfinished required work machine-usable without changing the completion tool contract.
Refs #3406.
Related to #2479: this PR uses task-scoped loop protection for pending-action-triggered replans so an earlier replan on the same slice does not suppress a later legitimate replan from a different completed task.
How
Implementation details:
SummarywithknownIssuesandpendingActions## Known Issuesand extract aPending actions:bullet block infiles.tsbuildCarryForwardSection()so later execution prompts can see unresolved required follow-up## Known Issuesif they still remain after the current taskbuildReplanSlicePrompt()andprompts/replan-slice.mdso replanning can be triggered by pending actions as well as blockersderiveStateFromDb()and_deriveStateImpl()so pending actions win beforesummarizing/ normalexecutingreplan_history.task_idmatching for pending-action loop protection in the DB-backed pathI intentionally kept the contract markdown-based instead of adding a new
gsd_task_complete.pendingActionsparameter. That keeps the change smaller and backward-compatible with the existing artifact-driven workflow.Verification
npm run typecheck:extensionspending-actions.test.tsflag-file-db.test.tsreplan-slice.test.tsderive-state.test.tsderive-state-db.test.tsprompt-contracts.test.tsnpm run buildNotes
blocker_discoveredcontract.