feat(orch): enforce pending todo completion before task finish#2841
feat(orch): enforce pending todo completion before task finish#2841amitksingh1490 wants to merge 20 commits intomainfrom
Conversation
github-actions
bot
added
the
type: feature
crates/forge_app/src/orch.rs
Outdated
| if !pending_todos.is_empty() { | ||
| let reminder = format!( | ||
| "You have {} pending todo items. Please complete them before finishing the task.", | ||
| pending_todos.len() |
There was a problem hiding this comment.
instead of items send the formated pending list
crates/forge_app/src/orch.rs
Outdated
| is_complete = false; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Move this logic outside of orch and expose as a hook.
|
|
||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we move the prompt from string literals to templates, and use the template engine to render.
…nd template rendering
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| // Check if a reminder was already added to avoid duplicates | ||
| if let Some(context) = &conversation.context { | ||
| let has_existing_reminder = context.messages.iter().any(|entry| { | ||
| entry | ||
| .message | ||
| .content() | ||
| .is_some_and(|content| content.contains("pending todo items")) | ||
| }); | ||
| if has_existing_reminder { | ||
| return Ok(()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical bug: The duplicate reminder check prevents enforcement of todo completion after the first reminder. Once a reminder message exists in the context, subsequent End hook invocations will return early without adding messages (line 60), causing the orchestrator to complete the task even though todos remain pending.
What breaks: Tasks will complete with incomplete todos after a single reminder injection.
Fix: Remove the duplicate check entirely, or change the logic to always block completion when pending todos exist:
if !pending_todos.is_empty() {
// Always block completion by returning an error or adding a message
return Err(anyhow::anyhow!("Cannot complete task with pending todos"));
}Alternatively, the duplicate check should verify whether todos were completed since the last reminder, not just whether a reminder exists.
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
2 participants
No description provided.