Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Aug 29, 2025

Summary

This PR addresses issue #7529 by making the attempt_completion tool conditionally available based on task context.

Changes

Core Implementation

  • Added isOrchestrated flag to Task class to track whether a task is a subtask created via new_task tool
  • Modified newTaskTool to pass isOrchestrated: true when creating subtasks
  • Updated system prompt generation to conditionally include attempt_completion based on:
    • Always included for orchestrated subtasks (isOrchestrated === true)
    • Optional for standard tasks based on allowAttemptCompletion user setting (default: false)

User Settings

  • Added allowAttemptCompletion setting to global settings schema
  • Updated UI components to expose the new setting in Auto-Approve menu
  • Added translation strings for the new setting

Testing

  • Added comprehensive test suite for conditional inclusion logic
  • Updated existing test snapshots to reflect new default behavior
  • All tests passing

Behavior

Default Behavior (Breaking Change)

  • Standard user-initiated tasks: attempt_completion is excluded by default
  • Orchestrated subtasks: attempt_completion is always included

User Control

Users can enable attempt_completion for standard tasks via:

  • Settings UI: Auto-Approve section → "Completion" toggle
  • Direct setting: allowAttemptCompletion: true

Testing Instructions

  1. Start a standard task - verify attempt_completion is not available by default
  2. Enable the "Completion" setting in Auto-Approve menu
  3. Start a new task - verify attempt_completion is now available
  4. Use orchestrator mode to create subtasks - verify subtasks always have attempt_completion

Fixes #7529


Important

This PR makes the attempt_completion tool conditional based on task context, adding a new allowAttemptCompletion setting and updating related UI components and tests.

  • Behavior:
    • attempt_completion tool is conditionally included based on task context in system.ts and index.ts.
    • Always included for orchestrated subtasks; optional for standard tasks based on allowAttemptCompletion setting.
  • Task Management:
    • Added isOrchestrated flag to Task class in Task.ts.
    • newTaskTool in newTaskTool.ts marks subtasks as orchestrated.
  • Settings:
    • Added allowAttemptCompletion to global settings schema in global-settings.ts.
    • Updated UI components in AutoApproveMenu.tsx and AutoApproveSettings.tsx to expose the new setting.
    • Added translation strings for allowAttemptCompletion in settings.json.
  • Testing:
    • Added tests for conditional inclusion logic in attempt-completion-conditional.spec.ts.
    • Updated test snapshots to reflect new behavior.

This description was created by Ellipsis for bbd77f2. You can customize this summary. It will automatically update as commits are pushed.

- Add isOrchestrated flag to Task class to track subtask context
- Always include attempt_completion for orchestrated subtasks
- Make attempt_completion optional for standard tasks via allowAttemptCompletion setting
- Default behavior excludes attempt_completion for standard tasks
- Add comprehensive tests for conditional inclusion logic
- Update UI components to expose the new setting
- Update snapshots to reflect new default behavior

Fixes #7529
@roomote roomote bot requested review from cte, jr and mrubens as code owners August 29, 2025 15:02
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Aug 29, 2025
Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed my own code and found issues. Classic me. Should've caught that missing translation before the CI did.

Critical Issues (Must Fix):

  1. Missing translations causing CI failure - The check-translations CI check is failing. The new allowAttemptCompletion setting has translations only in English but is missing in all other locale files. You'll need to add the translation keys to all locale files under webview-ui/src/i18n/locales/*/settings.json.

Important Suggestions:

  1. Consider adding integration tests - While the unit tests are comprehensive, consider adding integration tests to verify the end-to-end behavior when orchestrator creates subtasks.

  2. Documentation for breaking change - This is a breaking change (attempt_completion excluded by default). Consider adding more prominent documentation.

Minor Improvements:

  1. Code organization - The conditional logic in src/core/prompts/tools/index.ts (lines 122-127) could be extracted to a helper function for better readability.

  2. Test coverage for edge cases - Consider adding tests for when a user toggles the setting mid-task.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 29, 2025
@daniel-lxs
Copy link
Member

#7529 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Feature: Conditionally Include attempt_completion Tool Based on Task Context

4 participants