[AXON-742] Make PR button tied to the broader workspace state (#660)

sdzh-atlassian · web-flow · commit 4dfc0169f8c6 · 2025-07-31T15:17:49.000-07:00
diff --git a/src/react/atlascode/rovo-dev/create-pr/PullRequestForm.test.tsx b/src/react/atlascode/rovo-dev/create-pr/PullRequestForm.test.tsx
@@ -4,7 +4,7 @@ import { RovoDevProviderMessageType } from 'src/rovo-dev/rovoDevWebviewProviderM
 import { forceCastTo } from 'testsutil/miscFunctions';
 
 import { RovoDevViewResponseType } from '../rovoDevViewMessages';
-import { DefaultMessage, ToolReturnParseResult } from '../utils';
+import { DefaultMessage } from '../utils';
 import { PullRequestChatItem, PullRequestForm } from './PullRequestForm';
 
 const mockPostMessage = jest.fn();
@@ -13,37 +13,16 @@ const mockOnCancel = jest.fn();
 const mockOnPullRequestCreated = jest.fn();
 const mockSetFormVisible = jest.fn();
 
-const mockModifiedFiles = forceCastTo<ToolReturnParseResult[]>([
-    {
-        content: 'file content',
-        filePath: 'src/file.ts',
-        type: 'modify',
-    },
-]);
-
 describe('PullRequestForm', () => {
     beforeEach(() => {
         jest.clearAllMocks();
     });
 
-    it('renders null when no modified files', () => {
-        const { container } = render(
-            <PullRequestForm
-                onCancel={mockOnCancel}
-                messagingApi={{ postMessage: mockPostMessage, postMessagePromise: mockPostMessagePromise }}
-                modifiedFiles={[]}
-                onPullRequestCreated={mockOnPullRequestCreated}
-            />,
-        );
-        expect(container.firstChild).toBeNull();
-    });
-
     it('renders button when form is not visible', () => {
         render(
             <PullRequestForm
                 onCancel={mockOnCancel}
                 messagingApi={{ postMessage: mockPostMessage, postMessagePromise: mockPostMessagePromise }}
-                modifiedFiles={mockModifiedFiles}
                 onPullRequestCreated={mockOnPullRequestCreated}
                 isFormVisible={false}
                 setFormVisible={mockSetFormVisible}
@@ -58,7 +37,6 @@ describe('PullRequestForm', () => {
             <PullRequestForm
                 onCancel={mockOnCancel}
                 messagingApi={{ postMessage: mockPostMessage, postMessagePromise: mockPostMessagePromise }}
-                modifiedFiles={mockModifiedFiles}
                 onPullRequestCreated={mockOnPullRequestCreated}
                 isFormVisible={true}
                 setFormVisible={mockSetFormVisible}
@@ -78,7 +56,6 @@ describe('PullRequestForm', () => {
             <PullRequestForm
                 onCancel={mockOnCancel}
                 messagingApi={{ postMessage: mockPostMessage, postMessagePromise: mockPostMessagePromise }}
-                modifiedFiles={mockModifiedFiles}
                 onPullRequestCreated={mockOnPullRequestCreated}
                 isFormVisible={false}
                 setFormVisible={mockSetFormVisible}
@@ -102,7 +79,6 @@ describe('PullRequestForm', () => {
             <PullRequestForm
                 onCancel={mockOnCancel}
                 messagingApi={{ postMessage: mockPostMessage, postMessagePromise: mockPostMessagePromise }}
-                modifiedFiles={mockModifiedFiles}
                 onPullRequestCreated={mockOnPullRequestCreated}
                 isFormVisible={true}
             />,
@@ -119,7 +95,6 @@ describe('PullRequestForm', () => {
             <PullRequestForm
                 onCancel={mockOnCancel}
                 messagingApi={{ postMessage: mockPostMessage, postMessagePromise: mockPostMessagePromise }}
-                modifiedFiles={mockModifiedFiles}
                 onPullRequestCreated={mockOnPullRequestCreated}
                 isFormVisible={true}
             />,
@@ -152,7 +127,6 @@ describe('PullRequestForm', () => {
             <PullRequestForm
                 onCancel={mockOnCancel}
                 messagingApi={{ postMessage: mockPostMessage, postMessagePromise: mockPostMessagePromise }}
-                modifiedFiles={mockModifiedFiles}
                 onPullRequestCreated={mockOnPullRequestCreated}
                 isFormVisible={true}
             />,
diff --git a/src/react/atlascode/rovo-dev/create-pr/PullRequestForm.tsx b/src/react/atlascode/rovo-dev/create-pr/PullRequestForm.tsx
@@ -6,7 +6,7 @@ import { ConnectionTimeout } from 'src/util/time';
 import { useMessagingApi } from '../../messagingApi';
 import { mdParser } from '../common/common';
 import { RovoDevViewResponse, RovoDevViewResponseType } from '../rovoDevViewMessages';
-import { DefaultMessage, ToolReturnParseResult } from '../utils';
+import { DefaultMessage } from '../utils';
 
 const PullRequestButton: React.FC<{
     onClick: (event: React.MouseEvent<HTMLButtonElement>) => Promise<void>;
@@ -29,7 +29,6 @@ interface PullRequestFormProps {
     messagingApi: ReturnType<
         typeof useMessagingApi<RovoDevViewResponse, RovoDevProviderMessage, RovoDevProviderMessage>
     >;
-    modifiedFiles?: ToolReturnParseResult[];
     onPullRequestCreated: (url: string) => void;
     isFormVisible?: boolean;
     setFormVisible?: (visible: boolean) => void;
@@ -38,14 +37,10 @@ interface PullRequestFormProps {
 export const PullRequestForm: React.FC<PullRequestFormProps> = ({
     onCancel,
     messagingApi: { postMessagePromise },
-    modifiedFiles,
     onPullRequestCreated,
     isFormVisible = false,
     setFormVisible,
 }) => {
-    if (!modifiedFiles || modifiedFiles.length === 0) {
-        return null;
-    }
     const [isPullRequestLoading, setPullRequestLoading] = React.useState(false);
     const [isBranchNameLoading, setBranchNameLoading] = React.useState(false);
     const [branchName, setBranchName] = React.useState<string | undefined>(undefined);
diff --git a/src/react/atlascode/rovo-dev/messaging/ChatStream.tsx b/src/react/atlascode/rovo-dev/messaging/ChatStream.tsx
@@ -1,12 +1,13 @@
 import * as React from 'react';
-import { RovoDevProviderMessage } from 'src/rovo-dev/rovoDevWebviewProviderMessages';
+import { RovoDevProviderMessage, RovoDevProviderMessageType } from 'src/rovo-dev/rovoDevWebviewProviderMessages';
+import { ConnectionTimeout } from 'src/util/time';
 
 import { useMessagingApi } from '../../messagingApi';
 import { ErrorMessageItem, FollowUpActionFooter, OpenFileFunc, TechnicalPlanComponent } from '../common/common';
 import { PullRequestChatItem, PullRequestForm } from '../create-pr/PullRequestForm';
 import { RovoDevLanding } from '../rovoDevLanding';
 import { State } from '../rovoDevView';
-import { RovoDevViewResponse } from '../rovoDevViewMessages';
+import { RovoDevViewResponse, RovoDevViewResponseType } from '../rovoDevViewMessages';
 import { CodePlanButton } from '../technical-plan/CodePlanButton';
 import { ToolCallItem } from '../tools/ToolCallItem';
 import { ToolReturnParsedItem } from '../tools/ToolReturnItem';
@@ -59,17 +60,31 @@ export const ChatStream: React.FC<ChatStreamProps> = ({
 }) => {
     const chatEndRef = React.useRef<HTMLDivElement>(null);
     const [canCreatePR, setCanCreatePR] = React.useState(false);
+    const [hasChangesInGit, setHasChangesInGit] = React.useState(false);
     const [isFormVisible, setIsFormVisible] = React.useState(false);
 
+    const checkGitChanges = React.useCallback(async () => {
+        const response = await messagingApi.postMessagePromise(
+            { type: RovoDevViewResponseType.CheckGitChanges },
+            RovoDevProviderMessageType.CheckGitChangesComplete,
+            ConnectionTimeout,
+        );
+        setHasChangesInGit(response.hasChanges);
+    }, [messagingApi]);
+
     React.useEffect(() => {
         if (chatEndRef.current) {
             scrollToEnd(chatEndRef.current);
         }
 
         if (state === State.WaitingForPrompt) {
             setCanCreatePR(true);
+            if (currentMessage) {
+                // Only check git changes if there's something in the chat
+                checkGitChanges();
+            }
         }
-    }, [state, chatHistory, currentThinking, currentMessage, isFormVisible, pendingToolCall]);
+    }, [state, chatHistory, currentThinking, currentMessage, isFormVisible, pendingToolCall, checkGitChanges]);
 
     return (
         <div ref={chatEndRef} className="chat-message-container">
@@ -139,14 +154,13 @@ export const ChatStream: React.FC<ChatStreamProps> = ({
 
             {state === State.WaitingForPrompt && (
                 <FollowUpActionFooter>
-                    {canCreatePR && (
+                    {canCreatePR && hasChangesInGit && (
                         <PullRequestForm
                             onCancel={() => {
                                 setCanCreatePR(false);
                                 setIsFormVisible(false);
                             }}
                             messagingApi={messagingApi}
-                            modifiedFiles={modifiedFiles}
                             onPullRequestCreated={(url) => {
                                 setCanCreatePR(false);
                                 setIsFormVisible(false);
diff --git a/src/react/atlascode/rovo-dev/rovoDevView.tsx b/src/react/atlascode/rovo-dev/rovoDevView.tsx
@@ -419,6 +419,7 @@ const RovoDevView: React.FC = () => {
                 case RovoDevProviderMessageType.ReturnText:
                 case RovoDevProviderMessageType.CreatePRComplete:
                 case RovoDevProviderMessageType.GetCurrentBranchNameComplete:
+                case RovoDevProviderMessageType.CheckGitChangesComplete:
                     break; // This is handled elsewhere
 
                 default:
diff --git a/src/react/atlascode/rovo-dev/rovoDevViewMessages.tsx b/src/react/atlascode/rovo-dev/rovoDevViewMessages.tsx
@@ -16,6 +16,7 @@ export const enum RovoDevViewResponseType {
     ReportChangedFilesPanelShown = 'reportChangedFilesPanelShown',
     ReportChangesGitPushed = 'reportChangesGitPushed',
     ReportThinkingDrawerExpanded = 'reportThinkingDrawerExpanded',
+    CheckGitChanges = 'checkGitChanges',
 }
 
 export interface ModifiedFile {
@@ -37,4 +38,5 @@ export type RovoDevViewResponse =
     | ReducerAction<RovoDevViewResponseType.ForceUserFocusUpdate>
     | ReducerAction<RovoDevViewResponseType.ReportChangedFilesPanelShown, { filesCount: number }>
     | ReducerAction<RovoDevViewResponseType.ReportChangesGitPushed, { pullRequestCreated: boolean }>
-    | ReducerAction<RovoDevViewResponseType.ReportThinkingDrawerExpanded>;
+    | ReducerAction<RovoDevViewResponseType.ReportThinkingDrawerExpanded>
+    | ReducerAction<RovoDevViewResponseType.CheckGitChanges>;
diff --git a/src/rovo-dev/rovoDevPullRequestHandler.ts b/src/rovo-dev/rovoDevPullRequestHandler.ts
@@ -2,7 +2,7 @@ import { exec } from 'child_process';
 import { Logger } from 'src/logger';
 import { GitExtension, Repository } from 'src/typings/git';
 import { promisify } from 'util';
-import { env, extensions, Uri, window } from 'vscode';
+import { env, extensions, Uri } from 'vscode';
 
 export class RovoDevPullRequestHandler {
     private async getGitExtension(): Promise<GitExtension> {
@@ -13,9 +13,6 @@ export class RovoDevPullRequestHandler {
             }
             return await gitExtension.activate();
         } catch (e) {
-            window.showErrorMessage(
-                'Git extension not found or failed to activate. Please ensure Git is installed and the extension is enabled.',
-            );
             console.error('Error activating git extension:', e);
             throw e;
         }
@@ -25,7 +22,6 @@ export class RovoDevPullRequestHandler {
         const gitApi = gitExt.getAPI(1);
 
         if (gitApi.repositories.length === 0) {
-            window.showErrorMessage('No Git repositories found in the workspace.');
             throw new Error('No Git repositories found');
         }
 
@@ -97,4 +93,21 @@ export class RovoDevPullRequestHandler {
 
         return repo.state.HEAD?.name;
     }
+
+    public async isGitStateClean(): Promise<boolean> {
+        try {
+            const gitExt = await this.getGitExtension();
+            const repo = await this.getGitRepository(gitExt);
+
+            // A repo is clean if there are no unstaged, staged, or merge changes
+            return (
+                repo.state.workingTreeChanges.length === 0 &&
+                repo.state.indexChanges.length === 0 &&
+                repo.state.mergeChanges.length === 0
+            );
+        } catch (error) {
+            Logger.error(error, 'Error checking git state');
+            return true; // If we can't determine the state, assume it's clean
+        }
+    }
 }
diff --git a/src/rovo-dev/rovoDevWebviewProvider.ts b/src/rovo-dev/rovoDevWebviewProvider.ts
@@ -259,6 +259,15 @@ export class RovoDevWebviewProvider extends Disposable implements WebviewViewPro
                         );
                         break;
 
+                    case RovoDevViewResponseType.CheckGitChanges:
+                        await this._prHandler.isGitStateClean().then((isClean) => {
+                            this._webView?.postMessage({
+                                type: RovoDevProviderMessageType.CheckGitChangesComplete,
+                                hasChanges: !isClean,
+                            });
+                        });
+                        break;
+
                     case RovoDevViewResponseType.ReportThinkingDrawerExpanded:
                         this.fireTelemetryEvent('rovoDevDetailsExpandedEvent', this._currentPromptId);
                         break;
diff --git a/src/rovo-dev/rovoDevWebviewProviderMessages.ts b/src/rovo-dev/rovoDevWebviewProviderMessages.ts
@@ -20,6 +20,7 @@ export const enum RovoDevProviderMessageType {
     GetCurrentBranchNameComplete = 'getCurrentBranchNameComplete',
     UserFocusUpdated = 'userFocusUpdated',
     ContextAdded = 'contextAdded',
+    CheckGitChangesComplete = 'checkGitChangesComplete',
 }
 
 export interface RovoDevObjectResponse {
@@ -41,4 +42,5 @@ export type RovoDevProviderMessage =
     | ReducerAction<RovoDevProviderMessageType.CreatePRComplete, { data: { url?: string; error?: string } }>
     | ReducerAction<RovoDevProviderMessageType.GetCurrentBranchNameComplete, { data: { branchName?: string } }>
     | ReducerAction<RovoDevProviderMessageType.UserFocusUpdated, { userFocus: RovoDevContextItem }>
-    | ReducerAction<RovoDevProviderMessageType.ContextAdded, { context: RovoDevContextItem }>;
+    | ReducerAction<RovoDevProviderMessageType.ContextAdded, { context: RovoDevContextItem }>
+    | ReducerAction<RovoDevProviderMessageType.CheckGitChangesComplete, { hasChanges: boolean }>;
