feat(orch): enforce pending todo completion before task finish#2841
feat(orch): enforce pending todo completion before task finish#2841tusharmath merged 33 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.
crates/forge_app/src/app.rs
Outdated
|
|
||
| // Build the on_end hook, conditionally adding PendingTodosHandler based on | ||
| // config | ||
| let on_end_hook = if forge_config.pending_todos_hook { |
There was a problem hiding this comment.
Suggest a better name that's consistent with out existing flags - Should signal: Verification, Hook, Enable/Disable kind of functionality.
| /// Enables the pending todos hook that checks for incomplete todo items | ||
| /// when a task ends and reminds the LLM about them. | ||
| #[serde(default)] | ||
| pub verify_todos: bool, |
There was a problem hiding this comment.
Update .forge.toml to set the flag to true by default.
| self.hook | ||
| .handle(&response_event, &mut self.conversation) | ||
| .await?; | ||
|
|
There was a problem hiding this comment.
Why do we need to remove this?
2 participants
No description provided.