-
Notifications
You must be signed in to change notification settings - Fork 576
feat: Add task retry logic and improve max turns limit #778
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 all commits
aa940d4
d5340fd
8af1b8b
f06088a
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 |
|---|---|---|
|
|
@@ -115,17 +115,25 @@ export class FeatureStateManager { | |
| // This prevents cards in "waiting for review" from appearing to still have running tasks | ||
| if (feature.planSpec?.tasks) { | ||
| 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 waiting_approval` | ||
| ); | ||
| } | ||
| if (tasksPending > 0) { | ||
| logger.warn( | ||
| `[updateFeatureStatus] Feature ${featureId} moving to waiting_approval with ${tasksPending} pending (never started) tasks out of ${feature.planSpec.tasks.length} total` | ||
| ); | ||
| } | ||
| // Update tasksCompleted count to reflect actual completed tasks | ||
| feature.planSpec.tasksCompleted = feature.planSpec.tasks.filter( | ||
| (t) => t.status === 'completed' | ||
|
|
@@ -136,11 +144,26 @@ export class FeatureStateManager { | |
| // Also finalize in_progress tasks when moving directly to verified (skipTests=false) | ||
| // Do NOT mark pending tasks as completed - they were never started | ||
| if (feature.planSpec?.tasks) { | ||
| 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` | ||
| ); | ||
| } | ||
|
Comment on lines
+147
to
+166
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. This block of code is nearly identical to the one for the |
||
| feature.planSpec.tasksCompleted = feature.planSpec.tasks.filter( | ||
| (t) => t.status === 'completed' | ||
| ).length; | ||
|
|
||
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 value
3is 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.