Skip to content

Commit f389189

Browse files
committed
feat(pr-fixer-orchestrator): enhance workflow safety and user control
- Add user approval checkpoint before committing changes - Implement large diff handling (>2000 lines) with automatic summarization - Replace dangerous `git add -A` with selective file staging - Enforce context preservation in shared temp directory for all delegated tasks - Add exact PR template format specification - Update best practices to reflect new safety measures BREAKING CHANGE: Workflow now requires explicit user approval before commits
1 parent f5a51c4 commit f389189

File tree

8 files changed

+1618
-0
lines changed

8 files changed

+1618
-0
lines changed

.roo/rules-pr-fixer-orchestrator/1_Workflow.xml

Lines changed: 771 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
<best_practices>
2+
<orchestration_principles>
3+
<principle priority="critical">
4+
<name>Always Delegate Specialized Work</name>
5+
<description>The orchestrator coordinates but doesn't implement. Use specialized modes for analysis, coding, testing, and review.</description>
6+
<rationale>Each mode has specific expertise and permissions optimized for their tasks.</rationale>
7+
</principle>
8+
9+
<principle priority="critical">
10+
<name>Maintain Context Between Steps</name>
11+
<description>Use temporary files in .roo/temp/pr-fixer-orchestrator/[TASK_ID]/ to pass context between subtasks. ALL delegated tasks must save outputs to this directory.</description>
12+
<rationale>Subtasks run in isolation and need explicit context sharing. Files saved elsewhere will be inaccessible to subsequent steps.</rationale>
13+
</principle>
14+
15+
<principle priority="critical">
16+
<name>Get User Approval Before Committing</name>
17+
<description>ALWAYS present changes and get explicit user approval before committing. Show modified files, summarize changes, and ask for confirmation.</description>
18+
<rationale>Users must maintain control over what gets committed to their PR. Unexpected changes can break functionality or introduce unwanted modifications.</rationale>
19+
</principle>
20+
21+
<principle priority="critical">
22+
<name>Understand Requirements First</name>
23+
<description>Always analyze the PR's underlying purpose and requirements before fixing issues.</description>
24+
<rationale>Fixing review comments without understanding the feature can lead to incomplete or incorrect solutions.</rationale>
25+
</principle>
26+
27+
<principle priority="high">
28+
<name>Handle Large Diffs Gracefully</name>
29+
<description>Check diff size before processing. If over 2000 lines, create a summary instead of including the full diff.</description>
30+
<rationale>Large diffs can overwhelm context windows and make analysis difficult. Summaries maintain clarity.</rationale>
31+
</principle>
32+
</orchestration_principles>
33+
34+
<pr_fixing_guidelines>
35+
- Always understand the PR's purpose and requirements first
36+
- Analyze before implementing - understand all issues comprehensively
37+
- Address review feedback with the same priority as the reviewer's authority
38+
- Fix root causes of test failures, not just symptoms
39+
- Ensure all original PR requirements are met, not just review comments
40+
- Resolve conflicts carefully, understanding both sides of changes
41+
- Validate all changes before committing to avoid breaking the PR further
42+
- NEVER use `git add -A` - always stage specific files intentionally
43+
- Get user approval before committing any changes
44+
- Keep commits focused and well-described
45+
- Always check if PR is from a fork to push to correct remote
46+
- Monitor CI/CD checks in real-time after pushing
47+
- Consider translation needs for any user-facing changes
48+
- Document what was changed and why in the PR update message
49+
- Use the EXACT PR template format specified in 6_pr_template_format.xml
50+
</pr_fixing_guidelines>
51+
52+
<git_operation_best_practices>
53+
<practice category="conflict_resolution">
54+
<name>Non-Interactive Rebasing</name>
55+
<description>Always use GIT_EDITOR=true for automated rebase operations</description>
56+
<example>GIT_EDITOR=true git rebase origin/main</example>
57+
</practice>
58+
59+
<practice category="remote_handling">
60+
<name>Fork-Aware Pushing</name>
61+
<description>Always check isCrossRepository before pushing</description>
62+
<steps>
63+
- Check if PR is from fork using gh pr view --json isCrossRepository
64+
- Add fork remote if needed
65+
- Push to correct remote (origin vs fork)
66+
</steps>
67+
</practice>
68+
69+
<practice category="safe_pushing">
70+
<name>Force with Lease</name>
71+
<description>Use --force-with-lease for safer force pushing</description>
72+
<fallback>If it fails, fetch and use --force</fallback>
73+
</practice>
74+
75+
<practice category="staging_files">
76+
<name>Selective File Staging</name>
77+
<description>Always stage files individually, never use git add -A</description>
78+
<steps>
79+
- Review all modified files with git status
80+
- Stage only files that were intentionally modified
81+
- Use git add [specific-file] for each file
82+
- Double-check staged files with git diff --cached
83+
</steps>
84+
<rationale>Prevents accidentally committing temporary files, debug logs, or unintended changes</rationale>
85+
</practice>
86+
87+
<practice category="diff_management">
88+
<name>Large Diff Handling</name>
89+
<description>Check diff size before including in context files</description>
90+
<steps>
91+
- Save diff to file and check line count with wc -l
92+
- If over 2000 lines, create a summary instead
93+
- Include file counts, insertion/deletion stats
94+
- List most significantly changed files
95+
</steps>
96+
</practice>
97+
</git_operation_best_practices>
98+
99+
<subtask_delegation_patterns>
100+
<pattern name="analysis_delegation">
101+
<to_mode>architect</to_mode>
102+
<purpose>Comprehensive analysis and planning</purpose>
103+
<provides>Detailed reports and implementation plans</provides>
104+
<output_requirement>MUST save all outputs to .roo/temp/pr-fixer-orchestrator/[TASK_ID]/</output_requirement>
105+
</pattern>
106+
107+
<pattern name="implementation_delegation">
108+
<to_mode>code</to_mode>
109+
<purpose>Executing code changes and fixes</purpose>
110+
<provides>Implemented solutions and change summaries</provides>
111+
<output_requirement>MUST save changes_implemented.md to .roo/temp/pr-fixer-orchestrator/[TASK_ID]/</output_requirement>
112+
</pattern>
113+
114+
<pattern name="validation_delegation">
115+
<to_mode>test</to_mode>
116+
<purpose>Testing and validating changes</purpose>
117+
<provides>Test results and validation reports</provides>
118+
<output_requirement>MUST save validation_report.md to .roo/temp/pr-fixer-orchestrator/[TASK_ID]/</output_requirement>
119+
</pattern>
120+
121+
<pattern name="review_delegation">
122+
<to_mode>pr-reviewer</to_mode>
123+
<purpose>Final quality review before submission</purpose>
124+
<provides>Quality assessment and recommendations</provides>
125+
<output_requirement>MUST save final_review.md to .roo/temp/pr-fixer-orchestrator/[TASK_ID]/</output_requirement>
126+
</pattern>
127+
128+
<pattern name="translation_delegation">
129+
<to_mode>translate</to_mode>
130+
<purpose>Updating translations for UI changes</purpose>
131+
<provides>Synchronized translations across languages</provides>
132+
<output_requirement>MUST save translation_summary.md to .roo/temp/pr-fixer-orchestrator/[TASK_ID]/</output_requirement>
133+
</pattern>
134+
</subtask_delegation_patterns>
135+
136+
<error_handling>
137+
<scenario name="auth_failure">
138+
<error>GitHub CLI authentication error</error>
139+
<action>Prompt user to run 'gh auth login'</action>
140+
</scenario>
141+
142+
<scenario name="no_linked_issue">
143+
<error>No linked issue found</error>
144+
<action>Extract requirements from PR description and comments</action>
145+
</scenario>
146+
147+
<scenario name="push_failure">
148+
<error>Force-with-lease push fails</error>
149+
<action>Fetch latest and retry with --force</action>
150+
</scenario>
151+
152+
<scenario name="large_diff">
153+
<error>Diff exceeds 2000 lines</error>
154+
<action>Create summary with stats instead of full diff</action>
155+
</scenario>
156+
157+
<scenario name="missing_context_files">
158+
<error>Expected context files not found in temp directory</error>
159+
<action>Check if delegated task saved to correct location, re-run if needed</action>
160+
</scenario>
161+
</error_handling>
162+
163+
<user_interaction_guidelines>
164+
<guideline priority="critical">
165+
<name>Pre-Commit Approval</name>
166+
<description>Always get explicit user approval before committing changes</description>
167+
<implementation>
168+
- Show list of modified files
169+
- Summarize key changes made
170+
- Present clear approval options
171+
- Wait for user confirmation
172+
</implementation>
173+
</guideline>
174+
175+
<guideline priority="high">
176+
<name>Clear Communication</name>
177+
<description>Present information clearly and concisely</description>
178+
<implementation>
179+
- Use bullet points for lists
180+
- Highlight important warnings
181+
- Provide actionable suggestions
182+
- Avoid technical jargon when possible
183+
</implementation>
184+
</guideline>
185+
</user_interaction_guidelines>
186+
</best_practices>
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<github_cli_usage>
2+
<overview>
3+
This mode uses the GitHub CLI (gh) for all GitHub operations.
4+
The mode assumes the user has gh installed and authenticated.
5+
It can work with PRs from both the main repository and forks.
6+
</overview>
7+
8+
<pr_specific_commands>
9+
<command name="gh_pr_view">
10+
<purpose>Get comprehensive PR details</purpose>
11+
<syntax>gh pr view [pr-number] --repo [owner]/[repo] --json [fields]</syntax>
12+
<fields>number,title,body,state,labels,author,headRefName,baseRefName,mergeable,mergeStateStatus,isDraft,isCrossRepository,headRepositoryOwner,reviews,statusCheckRollup,comments</fields>
13+
</command>
14+
15+
<command name="gh_pr_checkout">
16+
<purpose>Checkout PR branch locally</purpose>
17+
<syntax>gh pr checkout [pr-number] --repo [owner]/[repo] --force</syntax>
18+
<note>Automatically handles fork setup</note>
19+
</command>
20+
21+
<command name="gh_pr_checks">
22+
<purpose>Monitor CI/CD status</purpose>
23+
<syntax>gh pr checks [pr-number] --repo [owner]/[repo] --watch</syntax>
24+
<note>Use --json for programmatic access</note>
25+
</command>
26+
27+
<command name="gh_pr_diff">
28+
<purpose>Get PR changes</purpose>
29+
<syntax>gh pr diff [pr-number] --repo [owner]/[repo] --name-only</syntax>
30+
<note>Use without --name-only for full diff</note>
31+
</command>
32+
33+
<command name="gh_pr_comment">
34+
<purpose>Add comment to PR</purpose>
35+
<syntax>gh pr comment [pr-number] --repo [owner]/[repo] --body "[message]"</syntax>
36+
</command>
37+
</pr_specific_commands>
38+
39+
<issue_integration>
40+
<command name="gh_pr_linked_issues">
41+
<purpose>Get issues linked to PR</purpose>
42+
<syntax>gh pr view [pr-number] --repo [owner]/[repo] --json closingIssuesReferences</syntax>
43+
<note>Returns array of linked issues</note>
44+
</command>
45+
46+
<command name="gh_issue_view">
47+
<purpose>Get issue details if linked</purpose>
48+
<syntax>gh issue view [issue-number] --repo [owner]/[repo] --json [fields]</syntax>
49+
<fields>number,title,body,state,labels,assignees,milestone,createdAt,updatedAt,closedAt,author,comments</fields>
50+
</command>
51+
</issue_integration>
52+
53+
<workflow_commands>
54+
<command name="gh_run_view">
55+
<purpose>Get detailed CI logs</purpose>
56+
<syntax>gh run view [run-id] --repo [owner]/[repo] --log-failed</syntax>
57+
<note>Use to debug failing tests</note>
58+
</command>
59+
60+
<command name="gh_api">
61+
<purpose>Direct API access for advanced operations</purpose>
62+
<examples>
63+
- Get PR reviews: gh api repos/[owner]/[repo]/pulls/[pr-number]/reviews
64+
- Get review comments: gh api repos/[owner]/[repo]/pulls/[pr-number]/comments
65+
</examples>
66+
</command>
67+
</workflow_commands>
68+
</github_cli_usage>
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
<requirements_analysis_guidelines>
2+
<overview>
3+
The PR Fixer Orchestrator must understand the underlying requirements
4+
of a PR before fixing issues. This ensures fixes align with the
5+
original intent and all acceptance criteria are met.
6+
</overview>
7+
8+
<sources_of_requirements>
9+
<source priority="1">
10+
<name>Linked GitHub Issues</name>
11+
<description>Primary source of requirements and acceptance criteria</description>
12+
<extraction>
13+
- Issue title and body
14+
- Acceptance criteria sections
15+
- Technical specifications
16+
- User stories or use cases
17+
</extraction>
18+
</source>
19+
20+
<source priority="2">
21+
<name>PR Description</name>
22+
<description>Often contains implementation notes and context</description>
23+
<extraction>
24+
- Feature description
25+
- Implementation approach
26+
- Testing notes
27+
- Breaking changes
28+
</extraction>
29+
</source>
30+
31+
<source priority="3">
32+
<name>PR Comments</name>
33+
<description>May contain clarifications and additional requirements</description>
34+
<extraction>
35+
- Author clarifications
36+
- Reviewer questions and answers
37+
- Scope changes or additions
38+
</extraction>
39+
</source>
40+
41+
<source priority="4">
42+
<name>Code Analysis</name>
43+
<description>Infer requirements from the implementation</description>
44+
<extraction>
45+
- API contracts
46+
- Data flow patterns
47+
- Test cases (reveal expected behavior)
48+
- Documentation comments
49+
</extraction>
50+
</source>
51+
</sources_of_requirements>
52+
53+
<analysis_approach>
54+
<step number="1">
55+
<name>Extract Explicit Requirements</name>
56+
<actions>
57+
- Parse linked issues for acceptance criteria
58+
- Extract requirements from PR description
59+
- Identify success metrics
60+
</actions>
61+
</step>
62+
63+
<step number="2">
64+
<name>Understand Implementation Intent</name>
65+
<actions>
66+
- Analyze the code changes to understand approach
67+
- Identify design decisions made
68+
- Note any architectural patterns used
69+
</actions>
70+
</step>
71+
72+
<step number="3">
73+
<name>Map Requirements to Implementation</name>
74+
<actions>
75+
- Verify each requirement has corresponding code
76+
- Identify any missing functionality
77+
- Note any extra functionality added
78+
</actions>
79+
</step>
80+
81+
<step number="4">
82+
<name>Identify Gaps</name>
83+
<actions>
84+
- List unimplemented requirements
85+
- Note incomplete features
86+
- Identify missing tests
87+
</actions>
88+
</step>
89+
</analysis_approach>
90+
91+
<common_requirement_patterns>
92+
<pattern name="bug_fix">
93+
<requirements>
94+
- Clear description of the bug
95+
- Steps to reproduce
96+
- Expected vs actual behavior
97+
- Affected versions/environments
98+
</requirements>
99+
</pattern>
100+
101+
<pattern name="new_feature">
102+
<requirements>
103+
- Feature description
104+
- User stories or use cases
105+
- API design (if applicable)
106+
- UI/UX specifications
107+
- Performance requirements
108+
</requirements>
109+
</pattern>
110+
111+
<pattern name="refactoring">
112+
<requirements>
113+
- Motivation for refactoring
114+
- Backward compatibility needs
115+
- Performance improvements expected
116+
- Migration path (if breaking)
117+
</requirements>
118+
</pattern>
119+
</common_requirement_patterns>
120+
</requirements_analysis_guidelines>

0 commit comments

Comments
 (0)