feat: Add task retry logic and improve max turns limit#778
feat: Add task retry logic and improve max turns limit#778gsxdsm merged 4 commits intoAutoMaker-Org:v0.15.0rcfrom
Conversation
Summary of ChangesHello @gsxdsm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness of agent-driven feature development by introducing an intelligent task retry system. It addresses the issue where features might prematurely transition to a completed state despite having unfinished tasks. By allowing agents to re-engage and complete outstanding tasks, and by increasing the maximum execution turns, the system ensures more thorough and reliable feature implementation. Additionally, the changes refine how task statuses are managed and displayed across the backend and frontend, providing a more accurate representation of work in progress. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds a retry loop in ExecutionService to re-run the agent up to three times for incomplete tasks using continuation prompts; increases per-task/default Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Feature Executor
participant ExecSvc as ExecutionService
participant Agent as AgentExecutor
participant StateMgr as FeatureStateManager
Client->>ExecSvc: executeFeature(feature, tasks)
ExecSvc->>Agent: run agent for tasks (maxTurns=100)
Agent-->>ExecSvc: execution result
rect rgba(100,150,200,0.5)
ExecSvc->>ExecSvc: evaluate task completion
alt incomplete tasks exist
ExecSvc->>ExecSvc: retry_count < 3?
ExecSvc->>ExecSvc: build continuation prompt with remaining task IDs
ExecSvc->>Agent: re-run agent with continuation prompt (maxTurns=100)
Agent-->>ExecSvc: execution result
ExecSvc->>Client: emit auto-mode progress events
else all tasks complete
ExecSvc->>ExecSvc: finalize results (elapsed time, completionMessage)
end
end
ExecSvc->>StateMgr: updateFeatureStatus(...)
StateMgr-->>ExecSvc: status updated (logs pending counts)
ExecSvc->>Client: emit feature_complete (completionMessage)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust retry mechanism for agent tasks that haven't completed, addressing an issue where features were marked as complete prematurely. The maximum number of agent turns has also been increased. The changes include new backend logic in ExecutionService with comprehensive unit tests, adjustments in FeatureStateManager to handle task finalization, and UI updates in AgentInfoPanel to correctly reflect the status of incomplete tasks.
My review focuses on improving maintainability and robustness. I've pointed out an opportunity to refactor duplicated code in FeatureStateManager and suggested defining a magic number as a constant in ExecutionService. I also found a potential issue in the UI where a 'failed' task status isn't handled, and I've provided a suggestion to make the logic safer.
Overall, this is a well-implemented feature that improves the reliability of task execution.
| // Check for incomplete tasks after agent execution. | ||
| // The agent may have finished early (hit max turns, decided it was done, etc.) | ||
| // while tasks are still pending. If so, re-run the agent to complete remaining tasks. | ||
| const MAX_TASK_RETRY_ATTEMPTS = 3; |
There was a problem hiding this comment.
The value 3 is used here as a magic number for the maximum number of retry attempts. It would be better to define this as a constant at the top of the file (e.g., const DEFAULT_MAX_TASK_RETRY_ATTEMPTS = 3;). This improves readability and makes the value easier to find and change if needed.
| let tasksFinalized = 0; | ||
| let tasksPending = 0; | ||
| for (const task of feature.planSpec.tasks) { | ||
| if (task.status === 'in_progress') { | ||
| task.status = 'completed'; | ||
| tasksFinalized++; | ||
| } else if (task.status === 'pending') { | ||
| tasksPending++; | ||
| } | ||
| } | ||
| if (tasksFinalized > 0) { | ||
| logger.info( | ||
| `[updateFeatureStatus] Finalized ${tasksFinalized} in_progress tasks for feature ${featureId} moving to verified` | ||
| ); | ||
| } | ||
| if (tasksPending > 0) { | ||
| logger.warn( | ||
| `[updateFeatureStatus] Feature ${featureId} moving to verified with ${tasksPending} pending (never started) tasks out of ${feature.planSpec.tasks.length} total` | ||
| ); | ||
| } |
There was a problem hiding this comment.
This block of code is nearly identical to the one for the waiting_approval status (lines 117-136). To improve maintainability and follow the DRY (Don't Repeat Yourself) principle, consider refactoring this logic into a private helper function. This function could take the feature and the status string as parameters to handle task finalization and logging.
apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/server/src/services/feature-state-manager.ts (1)
108-171: Consider extracting the duplicated task-finalization logic into a helper.The
waiting_approval(Lines 116–141) andverified(Lines 146–171) branches contain nearly identical code: iterating tasks, finalizingin_progress→completed, counting pending tasks, logging, and updatingtasksCompleted. Extracting this into a private method would reduce duplication and make future changes less error-prone.♻️ Example helper extraction
+ private finalizeTasks( + tasks: ParsedTask[], + featureId: string, + targetStatus: string + ): { tasksFinalized: number; tasksPending: number } { + let tasksFinalized = 0; + let tasksPending = 0; + for (const task of tasks) { + if (task.status === 'in_progress') { + task.status = 'completed'; + tasksFinalized++; + } else if (task.status === 'pending') { + tasksPending++; + } + } + if (tasksFinalized > 0) { + logger.info( + `[updateFeatureStatus] Finalized ${tasksFinalized} in_progress tasks for feature ${featureId} moving to ${targetStatus}` + ); + } + if (tasksPending > 0) { + logger.warn( + `[updateFeatureStatus] Feature ${featureId} moving to ${targetStatus} with ${tasksPending} pending (never started) tasks out of ${tasks.length} total` + ); + } + return { tasksFinalized, tasksPending }; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/services/feature-state-manager.ts` around lines 108 - 171, Extract the duplicated task-finalization logic into a helper (e.g., a private function finalizeTasksOnCompletion(planSpec, featureId, statusLabel) or finalizePlanSpecTasks(feature, featureId, statusLabel) called from updateFeatureStatus) that: iterates planSpec.tasks, converts 'in_progress' → 'completed' and counts finalized and pending tasks, updates planSpec.tasksCompleted and planSpec.currentTaskId, and emits the same logger.info/logger.warn messages using statusLabel; then replace the duplicated blocks in the 'waiting_approval' and 'verified' branches with a single call to that helper (pass feature.planSpec, featureId, and a label like 'waiting_approval' or 'verified').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/server/src/services/feature-state-manager.ts`:
- Around line 108-171: Extract the duplicated task-finalization logic into a
helper (e.g., a private function finalizeTasksOnCompletion(planSpec, featureId,
statusLabel) or finalizePlanSpecTasks(feature, featureId, statusLabel) called
from updateFeatureStatus) that: iterates planSpec.tasks, converts 'in_progress'
→ 'completed' and counts finalized and pending tasks, updates
planSpec.tasksCompleted and planSpec.currentTaskId, and emits the same
logger.info/logger.warn messages using statusLabel; then replace the duplicated
blocks in the 'waiting_approval' and 'verified' branches with a single call to
that helper (pass feature.planSpec, featureId, and a label like
'waiting_approval' or 'verified').
…/agent-info-panel.tsx Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/server/src/services/agent-executor.ts (1)
334-334: InconsistentmaxTurnshandling across execution paths.Line 334 caps per-task turns with
Math.min(sdkOptions?.maxTurns ?? 100, 100), but otherbuildExecOptscall sites don't apply the same cap:
- Line 553 (plan revision):
sdkOptions?.maxTurns ?? 100— no cap, so a user-configured value > 100 is passed through.- Line 664 (single-agent continuation):
options.sdkOptions?.maxTurns— no default at all, soundefinedis passed when unset (falling through to the provider's default of 100).If the intent is to enforce a 100-turn ceiling, apply it consistently. If not, the cap on line 334 is misleading. Consider extracting a helper like
getEffectiveMaxTurns(sdkOptions)to centralize this logic.Suggested helper
+ private getEffectiveMaxTurns(sdkOptions?: { maxTurns?: number }, cap = 100): number { + return Math.min(sdkOptions?.maxTurns ?? cap, cap); + }Then use it at lines 334, 553, and 664.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/services/agent-executor.ts` at line 334, Introduce a single helper getEffectiveMaxTurns(sdkOptions) that returns Math.min(sdkOptions?.maxTurns ?? 100, 100) and use it everywhere buildExecOpts is invoked so maxTurns handling is consistent: replace the current inline cap at the buildExecOpts call that uses Math.min, the plan-revision call that uses sdkOptions?.maxTurns ?? 100, and the single-agent continuation call that passes options.sdkOptions?.maxTurns so all three call sites pass getEffectiveMaxTurns(sdkOptions) into buildExecOpts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/server/src/services/agent-executor.ts`:
- Line 334: Introduce a single helper getEffectiveMaxTurns(sdkOptions) that
returns Math.min(sdkOptions?.maxTurns ?? 100, 100) and use it everywhere
buildExecOpts is invoked so maxTurns handling is consistent: replace the current
inline cap at the buildExecOpts call that uses Math.min, the plan-revision call
that uses sdkOptions?.maxTurns ?? 100, and the single-agent continuation call
that passes options.sdkOptions?.maxTurns so all three call sites pass
getEffectiveMaxTurns(sdkOptions) into buildExecOpts.
1 participant
Features are moving to completed before all tasks complete
Summary by CodeRabbit
New Features
Bug Fixes
Tests