live todo tracking, collapsible task list in final progress, hide set_output outside standalone (#492)
* fix false "without reporting progress" error + live todo tracking clean up orphaned progress comments when review is skipped or only set_output is used, preventing the false positive in handleAgentResult. parse todowrite events from OpenCode's NDJSON stream and render a live markdown checklist in the PR progress comment (2s debounce). agent's explicit report_progress always takes priority. Made-with: Cursor * fix contradictory review/progress prompting align Review and IncrementalReview mode prompts with their guidance — mode prompts said "always submit" while guidance said "skip if clean." remove the empty-approval submission that was silently dropped by the tool. make progress comment lifecycle explicit: created on first call, updated in place, removed after review submission. Made-with: Cursor * centralize todo tracking into shared TodoTracker module extract inline todo tracking logic (~95 lines) from opentoad.ts into action/utils/todoTracking.ts. the tracker is created once in main.ts and passed to agents via AgentRunContext.todoTracker, making it agent-agnostic and reusable for future agent implementations. Made-with: Cursor * fix todoTracker optional type to match file convention add | undefined to todoTracker in AgentRunContext, matching every other optional property in the same interface. Made-with: Cursor * instruct agents to always maintain a task list for live progress system prompt now tells agents to create an internal task list at the start of every run. the tracker renders it to the progress comment automatically. report_progress is reserved for final results only — no more intermediate "Checking..." messages that cancel the tracker and leave stale text on the comment. Made-with: Cursor * require report_progress summary at end of every run agents must always call report_progress with a final summary — the completed task list should never be the end state of the progress comment. updated all review mode prompts to call report_progress after submitting (or not submitting) a review. Made-with: Cursor * keep progress comment after review with final summary stop deleting the progress comment after review submission — the agent now always calls report_progress with a summary at the end, and that summary should persist as a record of what was done. Made-with: Cursor * harden stranded progress comment cleanup - main.ts: detect when tracker was last writer (agent never called report_progress) and delete the stranded checklist instead of leaving it as the final comment state - postCleanup.ts: expand stuck-comment detection to also catch stranded todo checklists (regex match for checklist patterns) when the process is killed before normal cleanup runs - modes.ts + selectMode.ts: add report_progress step to Summarize and SummaryUpdate modes (only modes that were missing it) Made-with: Cursor * fix stale comments, typo, and build mode redundancy - comment.ts: update deleteProgressComment docstring and inline comment to reflect current usage (stranded-comment cleanup, not post-review) - modes.ts: merge duplicate report_progress steps (8 + 10) into single step 9, fix "optimizatfixons" typo - wiki/post-cleanup.md: document checklist detection regex Made-with: Cursor * collapsible completed todos in final progress, hide set_output outside standalone mode - add renderCollapsible() to TodoTracker, append completed task list as <details> section when agent calls report_progress - cancel tracker after agent's final report_progress so it doesn't overwrite with raw checklist - conditionally register SetOutputTool only in standalone mode or when output_schema is provided - remove unconditional set_output instruction from orchestrator task section - update Summarize/SummaryUpdate mode guidance to not reference set_output Made-with: Cursor * show completion count in collapsible task list summary Made-with: Cursor * only count completed (not cancelled) in collapsible task list summary Made-with: Cursor * reinforce concise summary prompting across system prompt, modes, and tool description Made-with: Cursor * address review feedback: wasUpdated bypass, tracker false-positive, race condition - remove wasUpdated=true from cleanup paths so handleAgentResult correctly detects genuinely silent runs - add hadProgressComment to ToolState as immutable snapshot for the safety check - use todoTracker.hasPublished instead of enabled for stranded-comment cleanup - serialize onUpdate calls via inflightPromise chain with post-cancel guard - add settled() to wait for in-flight updates before writing final summary Made-with: Cursor * address round-2 review: hasPublished after success, finalSummaryWritten flag - set hasPublished only after onUpdate resolves (not before) so failed writes are not counted as published - add finalSummaryWritten flag to ToolState, set after successful non-plan reportProgress; decouple cleanup detection from todoTracker.enabled so it survives API failures where cancel() ran but the write didn't succeed Made-with: Cursor
This commit is contained in:
David Blass
committed by
pullfrog[bot]
pullfrog[bot]
parent
e9ce67fec6
commit
6b18b6730b
@@ -212,7 +212,13 @@ When posting comments via ${ghPullfrogMcpName}, write as a professional team mem
|
||||
|
||||
### Progress reporting
|
||||
|
||||
ALWAYS use \`report_progress\` to share your results and progress — never \`create_issue_comment\`. The \`report_progress\` tool updates the pre-created progress comment on the issue/PR. Using \`create_issue_comment\` instead creates duplicate comments and leaves the progress comment stuck in its initial state. The \`create_issue_comment\` tool is only for creating NEW standalone comments unrelated to your task progress.
|
||||
**Task list**: at the start of every run, create an internal task list based on the steps in your current mode. Update it as you complete each step. The system automatically renders this list to the progress comment — you do not need to call \`report_progress\` for this.
|
||||
|
||||
**\`report_progress\`**: you MUST call this exactly once at the end of every run with a brief final summary (1-3 sentences). Never call it for intermediate status updates (e.g., "Checking for changes...", "Starting review...") — the task list handles live progress automatically. Calling \`report_progress\` replaces the task list with your summary and preserves the completed task list in a collapsible section. Keep the summary concise — do not repeat what the task list already shows. Focus on the outcome (what was accomplished, links to artifacts) rather than listing individual steps.
|
||||
|
||||
Never use \`create_issue_comment\` for task progress — that creates duplicate comments and leaves the progress comment stuck in its initial state. \`create_issue_comment\` is only for standalone comments unrelated to your current task (e.g., Plan comments, PR Summary comments).
|
||||
|
||||
**After a PR review is submitted**, still call \`report_progress\` with your final summary. The progress comment persists as a record of what was done.
|
||||
|
||||
### If you get stuck
|
||||
|
||||
@@ -367,8 +373,6 @@ ${ctx.modes.map((m) => `- "${m.name}": ${m.description}`).join("\n")}
|
||||
|
||||
Follow the mode guidance to complete the task. Use your native file and shell tools for local operations, and the ${ghPullfrogMcpName} MCP tools for GitHub/git operations.
|
||||
|
||||
When done, call \`${ghPullfrogMcpName}/set_output\` with the final result. This makes it available as the GitHub Action output.
|
||||
|
||||
### No-action cases
|
||||
|
||||
If the task clearly requires no work, call \`${ghPullfrogMcpName}/report_progress\` directly to explain why no action is needed.`;
|
||||
|
||||
+10
-1
@@ -70,11 +70,20 @@ async function validateStuckProgressComment(
|
||||
comment_id: commentId,
|
||||
});
|
||||
|
||||
if (commentResult.data.body?.startsWith(LEAPING_INTO_ACTION_PREFIX)) {
|
||||
const body = commentResult.data.body ?? "";
|
||||
|
||||
if (body.startsWith(LEAPING_INTO_ACTION_PREFIX)) {
|
||||
log.info(`[post] comment ${commentId} is stuck on "Leaping into action"`);
|
||||
return commentId;
|
||||
}
|
||||
|
||||
// detect stranded todo checklists left by the tracker when the process was killed
|
||||
// before the agent could call report_progress with a final summary
|
||||
if (/^- \[[ x]\] |^- \*\*→\*\* |^- ~~/.test(body)) {
|
||||
log.info(`[post] comment ${commentId} is stuck on a todo checklist`);
|
||||
return commentId;
|
||||
}
|
||||
|
||||
log.info(`[post] comment ${commentId} is not stuck (already updated or different content)`);
|
||||
return null;
|
||||
} catch (error) {
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import type { WriteablePayload } from "../external.ts";
|
||||
import { deleteProgressComment } from "../mcp/comment.ts";
|
||||
import { reportReviewNodeId } from "../mcp/review.ts";
|
||||
import type { ToolContext } from "../mcp/server.ts";
|
||||
import { log } from "./cli.ts";
|
||||
@@ -34,8 +33,6 @@ export async function postReviewCleanup(ctx: ToolContext): Promise<void> {
|
||||
"follow-up re-review dispatch"
|
||||
);
|
||||
}
|
||||
|
||||
await bestEffort(() => deleteProgressComment(ctx), "delete progress comment");
|
||||
}
|
||||
|
||||
async function bestEffort(fn: () => Promise<unknown>, label: string): Promise<void> {
|
||||
|
||||
+1
-1
@@ -19,7 +19,7 @@ export async function handleAgentResult(ctx: HandleAgentResultParams): Promise<M
|
||||
};
|
||||
}
|
||||
|
||||
if (!ctx.toolState.wasUpdated && ctx.toolState.progressCommentId && !ctx.silent) {
|
||||
if (!ctx.toolState.wasUpdated && ctx.toolState.hadProgressComment && !ctx.silent) {
|
||||
const error = ctx.result.error || "agent completed without reporting progress";
|
||||
try {
|
||||
await reportErrorToComment({
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
import { log } from "./log.ts";
|
||||
|
||||
type TodoItem = {
|
||||
id: string;
|
||||
content: string;
|
||||
status: "pending" | "in_progress" | "completed" | "cancelled";
|
||||
};
|
||||
|
||||
function isValidTodoStatus(value: string): value is TodoItem["status"] {
|
||||
return (
|
||||
value === "pending" || value === "in_progress" || value === "completed" || value === "cancelled"
|
||||
);
|
||||
}
|
||||
|
||||
function parseTodowriteInput(input: unknown): { todos: unknown[]; merge: boolean } | undefined {
|
||||
if (!input || typeof input !== "object" || !("todos" in input)) return undefined;
|
||||
if (!Array.isArray(input.todos)) return undefined;
|
||||
const merge = "merge" in input && input.merge === true;
|
||||
return { todos: input.todos, merge };
|
||||
}
|
||||
|
||||
function parseTodoItem(entry: unknown, index: number): TodoItem | undefined {
|
||||
if (!entry || typeof entry !== "object") return undefined;
|
||||
if (!("content" in entry) || typeof entry.content !== "string") return undefined;
|
||||
const id = "id" in entry && typeof entry.id === "string" ? entry.id : String(index);
|
||||
const status =
|
||||
"status" in entry && typeof entry.status === "string" && isValidTodoStatus(entry.status)
|
||||
? entry.status
|
||||
: "pending";
|
||||
return { id, content: entry.content, status };
|
||||
}
|
||||
|
||||
function renderTodoMarkdown(todos: TodoItem[]): string {
|
||||
return todos
|
||||
.map((todo) => {
|
||||
switch (todo.status) {
|
||||
case "completed":
|
||||
return `- [x] ${todo.content}`;
|
||||
case "cancelled":
|
||||
return `- ~~${todo.content}~~`;
|
||||
case "in_progress":
|
||||
return `- **→** ${todo.content}`;
|
||||
case "pending":
|
||||
return `- [ ] ${todo.content}`;
|
||||
default:
|
||||
todo.status satisfies never;
|
||||
return `- [ ] ${todo.content}`;
|
||||
}
|
||||
})
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
export type TodoTracker = {
|
||||
update: (input: unknown) => void;
|
||||
flush: () => Promise<void>;
|
||||
cancel: () => void;
|
||||
/** resolves when any in-flight onUpdate call completes */
|
||||
settled: () => Promise<void>;
|
||||
renderCollapsible: () => string;
|
||||
readonly enabled: boolean;
|
||||
/** true after the tracker has successfully called onUpdate at least once */
|
||||
readonly hasPublished: boolean;
|
||||
};
|
||||
|
||||
const DEBOUNCE_MS = 2000;
|
||||
|
||||
export function createTodoTracker(onUpdate: (body: string) => Promise<void>): TodoTracker {
|
||||
const state = new Map<string, TodoItem>();
|
||||
let enabled = true;
|
||||
let hasPublished = false;
|
||||
let debounceTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
let inflightPromise: Promise<void> = Promise.resolve();
|
||||
|
||||
function scheduleUpdate() {
|
||||
if (!enabled) return;
|
||||
if (debounceTimer) clearTimeout(debounceTimer);
|
||||
debounceTimer = setTimeout(() => {
|
||||
debounceTimer = null;
|
||||
if (!enabled || state.size === 0) return;
|
||||
const markdown = renderTodoMarkdown(Array.from(state.values()));
|
||||
inflightPromise = inflightPromise
|
||||
.then(async () => {
|
||||
if (!enabled) return;
|
||||
await onUpdate(markdown);
|
||||
hasPublished = true;
|
||||
})
|
||||
.catch((err) => {
|
||||
log.debug(`todo progress update failed: ${err}`);
|
||||
});
|
||||
}, DEBOUNCE_MS);
|
||||
}
|
||||
|
||||
return {
|
||||
update(input: unknown) {
|
||||
if (!enabled) return;
|
||||
const parsed = parseTodowriteInput(input);
|
||||
if (!parsed) return;
|
||||
if (!parsed.merge) state.clear();
|
||||
for (const [index, entry] of parsed.todos.entries()) {
|
||||
const item = parseTodoItem(entry, index);
|
||||
if (item) state.set(item.id, item);
|
||||
}
|
||||
log.debug(`» todowrite: ${state.size} items tracked`);
|
||||
scheduleUpdate();
|
||||
},
|
||||
|
||||
async flush() {
|
||||
if (debounceTimer) {
|
||||
clearTimeout(debounceTimer);
|
||||
debounceTimer = null;
|
||||
}
|
||||
if (!enabled || state.size === 0) return;
|
||||
const markdown = renderTodoMarkdown(Array.from(state.values()));
|
||||
inflightPromise = inflightPromise
|
||||
.then(async () => {
|
||||
if (!enabled) return;
|
||||
await onUpdate(markdown);
|
||||
hasPublished = true;
|
||||
})
|
||||
.catch((err) => {
|
||||
log.debug(`todo progress flush failed: ${err}`);
|
||||
});
|
||||
await inflightPromise;
|
||||
},
|
||||
|
||||
cancel() {
|
||||
enabled = false;
|
||||
if (debounceTimer) {
|
||||
clearTimeout(debounceTimer);
|
||||
debounceTimer = null;
|
||||
}
|
||||
},
|
||||
|
||||
async settled() {
|
||||
await inflightPromise;
|
||||
},
|
||||
|
||||
renderCollapsible(): string {
|
||||
if (state.size === 0) return "";
|
||||
const todos = Array.from(state.values());
|
||||
const completed = todos.filter((t) => t.status === "completed").length;
|
||||
const markdown = renderTodoMarkdown(todos);
|
||||
return `<details>\n<summary>Task list (${completed}/${todos.length} completed)</summary>\n\n${markdown}\n\n</details>`;
|
||||
},
|
||||
|
||||
get enabled() {
|
||||
return enabled;
|
||||
},
|
||||
|
||||
get hasPublished() {
|
||||
return hasPublished;
|
||||
},
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user