-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: add error feedback and unstick functionality for stuck tasks #1859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
5745cb1
cc0646b
a5ad9d7
8403793
e49ee5d
c728f14
ca86284
50c42c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -215,6 +215,13 @@ def parse_args() -> argparse.Namespace: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| help="Add follow-up tasks to a completed spec (extends existing implementation plan)", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Stuck subtask recovery | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parser.add_argument( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "--unstick", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| action="store_true", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| help="Clear all stuck subtasks for a spec (allows task to continue after file validation failures)", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Review options | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parser.add_argument( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "--review-status", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -464,6 +471,20 @@ def _run_cli() -> None: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Handle --unstick command | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if args.unstick: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from services.recovery import clear_stuck_subtasks, get_stuck_subtasks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| stuck = get_stuck_subtasks(spec_dir, project_dir) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if stuck: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clear_stuck_subtasks(spec_dir, project_dir) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Cleared {len(stuck)} stuck subtasks for {args.spec}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for s in stuck: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f" - {s.get('subtask_id', 'unknown')}: {s.get('reason', 'no reason')[:80]}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"No stuck subtasks found for {args.spec}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+472
to
+490
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrap If Proposed fix # Handle --unstick command
if args.unstick:
from services.recovery import clear_stuck_subtasks, get_stuck_subtasks
- stuck = get_stuck_subtasks(spec_dir, project_dir)
- if stuck:
- try:
+ try:
+ stuck = get_stuck_subtasks(spec_dir, project_dir)
+ if stuck:
clear_stuck_subtasks(spec_dir, project_dir)
print(f"Cleared {len(stuck)} stuck subtasks for {args.spec}")
for s in stuck:
print(
f" - {s.get('subtask_id', 'unknown')}: {s.get('reason', 'no reason')[:80]}"
)
- except Exception as e:
- print(f"Failed to clear stuck subtasks: {e}")
- sys.exit(1)
- else:
- print(f"No stuck subtasks found for {args.spec}")
+ else:
+ print(f"No stuck subtasks found for {args.spec}")
+ except Exception as e:
+ print(f"Failed to clear stuck subtasks: {e}")
+ sys.exit(1)
return📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Normal build flow | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| handle_build_command( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_dir=project_dir, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1221,4 +1221,92 @@ export function registerTaskExecutionHandlers( | |
| } | ||
| } | ||
| ); | ||
|
|
||
| /** | ||
| * Get stuck subtask information for a spec | ||
| */ | ||
| ipcMain.handle( | ||
| IPC_CHANNELS.TASK_GET_STUCK_INFO, | ||
| async (_, projectId: string, specId: string): Promise<IPCResult<{ stuckSubtasks: Array<{ subtask_id: string; reason: string; escalated_at: string; attempt_count: number }> }>> => { | ||
| try { | ||
| const project = projectStore.getProject(projectId); | ||
| if (!project) { | ||
| return { success: false, error: 'Project not found' }; | ||
| } | ||
|
|
||
| const specsBaseDir = getSpecsDir(project.autoBuildPath); | ||
| const specDir = path.join(project.path, specsBaseDir, specId); | ||
| const attemptHistoryPath = path.join(specDir, 'memory', 'attempt_history.json'); | ||
|
|
||
| const historyContent = safeReadFileSync(attemptHistoryPath); | ||
| if (!historyContent) { | ||
| return { success: true, data: { stuckSubtasks: [] } }; | ||
| } | ||
|
|
||
| const history = JSON.parse(historyContent); | ||
| return { success: true, data: { stuckSubtasks: history.stuck_subtasks || [] } }; | ||
| } catch (error) { | ||
| console.error('Failed to get stuck info:', error); | ||
| return { success: false, error: error instanceof Error ? error.message : 'Failed to get stuck info' }; | ||
| } | ||
|
Comment on lines
+1233
to
+1251
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return i18n keys (not hardcoded strings) in IPC error payloads. As per coding guidelines: Also applies to: 1263-1309 🤖 Prompt for AI Agents |
||
| } | ||
| ); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Clear stuck subtasks for a spec | ||
| */ | ||
| ipcMain.handle( | ||
| IPC_CHANNELS.TASK_UNSTICK_SUBTASKS, | ||
| async (_, projectId: string, specId: string): Promise<IPCResult<{ cleared: number }>> => { | ||
| try { | ||
| const project = projectStore.getProject(projectId); | ||
| if (!project) { | ||
| return { success: false, error: 'Project not found' }; | ||
| } | ||
|
|
||
| const specsBaseDir = getSpecsDir(project.autoBuildPath); | ||
| const specDir = path.join(project.path, specsBaseDir, specId); | ||
| const attemptHistoryPath = path.join(specDir, 'memory', 'attempt_history.json'); | ||
|
|
||
| const historyContent = safeReadFileSync(attemptHistoryPath); | ||
| if (!historyContent) { | ||
| return { success: true, data: { cleared: 0 } }; | ||
| } | ||
|
|
||
| const history = JSON.parse(historyContent); | ||
| const count = (history.stuck_subtasks || []).length; | ||
|
|
||
| // Clear stuck subtasks list | ||
| history.stuck_subtasks = []; | ||
|
|
||
| // Reset any subtasks marked as 'stuck' to 'pending' | ||
| if (history.subtasks && typeof history.subtasks === 'object') { | ||
| for (const subtaskId of Object.keys(history.subtasks)) { | ||
| if (history.subtasks[subtaskId]?.status === 'stuck') { | ||
| history.subtasks[subtaskId].status = 'pending'; | ||
| } | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // Save updated history | ||
| writeFileAtomicSync(attemptHistoryPath, JSON.stringify(history, null, 2)); | ||
|
|
||
| // Also update worktree copy if it exists | ||
| const worktreePath = findTaskWorktree(project.path, specId); | ||
| if (worktreePath) { | ||
| const worktreeSpecDir = path.join(worktreePath, specsBaseDir, specId); | ||
| const worktreeHistoryPath = path.join(worktreeSpecDir, 'memory', 'attempt_history.json'); | ||
| if (existsSync(worktreeHistoryPath)) { | ||
| writeFileAtomicSync(worktreeHistoryPath, JSON.stringify(history, null, 2)); | ||
| } | ||
| } | ||
|
|
||
| console.log(`[Unstick] Cleared ${count} stuck subtasks for ${specId}`); | ||
| return { success: true, data: { cleared: count } }; | ||
| } catch (error) { | ||
| console.error('Failed to unstick subtasks:', error); | ||
| return { success: false, error: error instanceof Error ? error.message : 'Failed to unstick subtasks' }; | ||
| } | ||
| } | ||
| ); | ||
|
Comment on lines
+1228
to
+1311
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These new IPC handlers ( Duplicating this logic across the TypeScript main process and the Python backend can lead to inconsistencies and maintenance issues. For instance, the logic in To improve maintainability and ensure a single source of truth, this logic should be centralized in the Python backend. The |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The aliases
get_stuckandclear_stuckcan be confusing, especially sinceclear_stuckis used as a function name but it's an alias forclear_stuck_subtasks. For better readability and maintainability, it's recommended to use the full function names directly by changing the import statement and the subsequent calls.