Merge pull request #17 from ThomasDepole/feature/bitbucket-pending-comments

Feature/bitbucket pending comments on pull request

Author: b1ff

packages/bitbucket/src/__tests__/bitbucket-service.test.ts

@@ -1,5 +1,6 @@
 import { BitbucketService } from '../bitbucket-service.js';
 import { PullRequestsService } from '../bitbucket-client/index.js';
+import { request as mockRequest } from '../bitbucket-client/core/request.js';
 
 // Mock the request function
 jest.mock('../bitbucket-client/core/request.js', () => ({
@@ -14,6 +15,7 @@ jest.mock('../bitbucket-client/index.js', () => ({
     streamChanges1: jest.fn(),
     create: jest.fn(),
     update: jest.fn(),
+    updateStatus: jest.fn(),
     getPage: jest.fn(),
     getReviewers: jest.fn(),
     get3: jest.fn()
@@ -1649,4 +1651,222 @@ describe('BitbucketService', () => {
       expect(missingVars).toEqual([]);
     });
   });
+
+  describe('postPullRequestComment - pending flag', () => {
+    it('should include state: PENDING in the request body when pending is true', async () => {
+      const mockComment = { id: 99, text: 'Draft comment', author: { displayName: 'Test User' } };
+      (PullRequestsService.createComment2 as jest.Mock).mockResolvedValue(mockComment);
+
+      const result = await bitbucketService.postPullRequestComment(
+        mockProjectKey,
+        mockRepositorySlug,
+        mockPullRequestId,
+        'Draft comment',
+        undefined, // parentId
+        undefined, // filePath
+        undefined, // line
+        undefined, // lineType
+        true       // pending
+      );
+
+      expect(result.success).toBe(true);
+      expect(PullRequestsService.createComment2).toHaveBeenCalledWith(
+        mockProjectKey,
+        mockPullRequestId,
+        mockRepositorySlug,
+        { text: 'Draft comment', state: 'PENDING' }
+      );
+    });
+
+    it('should NOT include pending in the request body when pending is false or omitted', async () => {
+      const mockComment = { id: 100, text: 'Normal comment', author: { displayName: 'Test User' } };
+      (PullRequestsService.createComment2 as jest.Mock).mockResolvedValue(mockComment);
+
+      await bitbucketService.postPullRequestComment(
+        mockProjectKey,
+        mockRepositorySlug,
+        mockPullRequestId,
+        'Normal comment'
+      );
+
+      expect(PullRequestsService.createComment2).toHaveBeenCalledWith(
+        mockProjectKey,
+        mockPullRequestId,
+        mockRepositorySlug,
+        { text: 'Normal comment' } // no pending field
+      );
+    });
+
+    it('should support state: PENDING combined with a file anchor', async () => {
+      const mockComment = { id: 101, text: 'Pending file comment', author: { displayName: 'Test User' } };
+      (PullRequestsService.createComment2 as jest.Mock).mockResolvedValue(mockComment);
+
+      await bitbucketService.postPullRequestComment(
+        mockProjectKey,
+        mockRepositorySlug,
+        mockPullRequestId,
+        'Pending file comment',
+        undefined,      // parentId
+        'src/index.ts', // filePath
+        10,             // line
+        'ADDED',        // lineType
+        true            // pending
+      );
+
+      expect(PullRequestsService.createComment2).toHaveBeenCalledWith(
+        mockProjectKey,
+        mockPullRequestId,
+        mockRepositorySlug,
+        {
+          text: 'Pending file comment',
+          state: 'PENDING',
+          anchor: {
+            path: 'src/index.ts',
+            diffType: 'EFFECTIVE',
+            line: 10,
+            lineType: 'ADDED',
+            fileType: 'TO'
+          }
+        }
+      );
+    });
+  });
+
+  describe('submitPullRequestReview', () => {
+    const mockUserSlug = 'test-user';
+
+    it('should submit a NEEDS_WORK review and call updateStatus with correct args', async () => {
+      const mockParticipant = {
+        user: { name: mockUserSlug },
+        role: 'REVIEWER',
+        status: 'NEEDS_WORK'
+      };
+      (PullRequestsService.updateStatus as jest.Mock).mockResolvedValue(mockParticipant);
+
+      const result = await bitbucketService.submitPullRequestReview(
+        mockProjectKey,
+        mockRepositorySlug,
+        mockPullRequestId,
+        mockUserSlug,
+        'NEEDS_WORK'
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.data).toBe(mockParticipant);
+      expect(PullRequestsService.updateStatus).toHaveBeenCalledWith(
+        mockProjectKey,
+        mockUserSlug,
+        mockPullRequestId,
+        mockRepositorySlug,
+        { status: 'NEEDS_WORK' }
+      );
+    });
+
+    it('should submit an APPROVED review', async () => {
+      const mockParticipant = { user: { name: mockUserSlug }, status: 'APPROVED' };
+      (PullRequestsService.updateStatus as jest.Mock).mockResolvedValue(mockParticipant);
+
+      const result = await bitbucketService.submitPullRequestReview(
+        mockProjectKey,
+        mockRepositorySlug,
+        mockPullRequestId,
+        mockUserSlug,
+        'APPROVED'
+      );
+
+      expect(result.success).toBe(true);
+      expect(PullRequestsService.updateStatus).toHaveBeenCalledWith(
+        mockProjectKey,
+        mockUserSlug,
+        mockPullRequestId,
+        mockRepositorySlug,
+        { status: 'APPROVED' }
+      );
+    });
+
+    it('should include lastReviewedCommit when provided', async () => {
+      const mockParticipant = { user: { name: mockUserSlug }, status: 'NEEDS_WORK' };
+      (PullRequestsService.updateStatus as jest.Mock).mockResolvedValue(mockParticipant);
+
+      await bitbucketService.submitPullRequestReview(
+        mockProjectKey,
+        mockRepositorySlug,
+        mockPullRequestId,
+        mockUserSlug,
+        'NEEDS_WORK',
+        'abc123def456'
+      );
+
+      expect(PullRequestsService.updateStatus).toHaveBeenCalledWith(
+        mockProjectKey,
+        mockUserSlug,
+        mockPullRequestId,
+        mockRepositorySlug,
+        { status: 'NEEDS_WORK', lastReviewedCommit: 'abc123def456' }
+      );
+    });
+
+    it('should handle API errors gracefully', async () => {
+      const mockError = new Error('Forbidden');
+      (PullRequestsService.updateStatus as jest.Mock).mockRejectedValue(mockError);
+
+      const result = await bitbucketService.submitPullRequestReview(
+        mockProjectKey,
+        mockRepositorySlug,
+        mockPullRequestId,
+        mockUserSlug,
+        'APPROVED'
+      );
+
+      expect(result.success).toBe(false);
+      expect(result.error).toBe('Forbidden');
+    });
+  });
+
+  describe('getUser', () => {
+    it('should fetch a user by exact slug', async () => {
+      const mockUser = { slug: 'jsmith', displayName: 'John Smith', emailAddress: 'jsmith@example.com' };
+      (mockRequest as jest.Mock).mockResolvedValue(mockUser);
+
+      const result = await bitbucketService.getUser('jsmith', undefined);
+
+      expect(result.success).toBe(true);
+      expect(result.data).toEqual(mockUser);
+      expect(mockRequest).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({
+          method: 'GET',
+          url: '/api/latest/users/{userSlug}',
+          path: { userSlug: 'jsmith' }
+        })
+      );
+    });
+
+    it('should search for users by filter string', async () => {
+      const mockUsers = { values: [{ slug: 'jsmith', displayName: 'John Smith' }], size: 1, isLastPage: true };
+      (mockRequest as jest.Mock).mockResolvedValue(mockUsers);
+
+      const result = await bitbucketService.getUser(undefined, 'John');
+
+      expect(result.success).toBe(true);
+      expect(result.data).toEqual(mockUsers);
+      expect(mockRequest).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({
+          method: 'GET',
+          url: '/api/latest/users',
+          query: { filter: 'John' }
+        })
+      );
+    });
+
+    it('should handle API errors gracefully', async () => {
+      (mockRequest as jest.Mock).mockRejectedValue(new Error('Not Found'));
+
+      const result = await bitbucketService.getUser('nonexistent', undefined);
+
+      expect(result.success).toBe(false);
+      expect(result.error).toBe('Not Found');
+    });
+  });
 });

packages/bitbucket/src/bitbucket-service.ts

@@ -256,6 +256,9 @@ export class BitbucketService {
    * @param filePath Optional file path for file-specific comments
    * @param line Optional line number for line-specific comments
    * @param lineType Optional line type ('ADDED', 'REMOVED', 'CONTEXT') for line comments
+   * @param pending Optional flag to create a pending (draft) comment, not visible to others until a review is submitted.
+   *   Only works when filePath is provided (file-level or inline comments).
+   *   Top-level PR comments (no filePath) are always posted live regardless of this flag.
    * @returns Promise with created comment data
    */
   async postPullRequestComment(
@@ -266,12 +269,19 @@ export class BitbucketService {
     parentId?: number,
     filePath?: string,
     line?: number,
-    lineType?: 'ADDED' | 'REMOVED' | 'CONTEXT'
+    lineType?: 'ADDED' | 'REMOVED' | 'CONTEXT',
+    pending?: boolean
   ) {
     const comment: any = {
       text
     };
 
+    // Mark comment as pending (draft) — not visible to others until review is submitted
+    // Note: Bitbucket DC uses state:'PENDING' not pending:true
+    if (pending) {
+      comment.state = 'PENDING';
+    }
+
     // Add parent reference for replies
     if (parentId) {
       comment.parent = { id: parentId };
@@ -303,6 +313,69 @@ export class BitbucketService {
     );
   }
 
+  /**
+   * Get a user by slug, or search for users by name/email filter
+   * @param userSlug Optional exact slug to look up a specific user
+   * @param filter Optional search string to find users by name or email
+   * @returns Promise with user data
+   */
+  async getUser(userSlug?: string, filter?: string) {
+    if (userSlug) {
+      return handleApiOperation(
+        () => __request(OpenAPI, {
+          method: 'GET',
+          url: '/api/latest/users/{userSlug}',
+          path: { userSlug },
+        }),
+        'Error fetching user'
+      );
+    }
+    return handleApiOperation(
+      () => __request(OpenAPI, {
+        method: 'GET',
+        url: '/api/latest/users',
+        query: { filter },
+      }),
+      'Error fetching users'
+    );
+  }
+
+  /**
+   * Submit a pull request review, publishing all pending (draft) comments and updating the reviewer's status.
+   * This is the equivalent of clicking "Submit Review" in the Bitbucket UI.
+   * @param projectKey The project key
+   * @param repositorySlug The repository slug
+   * @param pullRequestId The pull request ID
+   * @param userSlug The username/slug of the reviewer submitting the review (the PAT token owner).
+   * @param status The review verdict: 'APPROVED', 'NEEDS_WORK', or 'UNAPPROVED'
+   * @param lastReviewedCommit Optional last reviewed commit hash (for tracking review progress)
+   * @returns Promise with updated participant data
+   */
+  async submitPullRequestReview(
+    projectKey: string,
+    repositorySlug: string,
+    pullRequestId: string,
+    userSlug: string,
+    status: 'APPROVED' | 'NEEDS_WORK' | 'UNAPPROVED',
+    lastReviewedCommit?: string
+  ) {
+    const requestBody: any = {
+      status,
+      ...(lastReviewedCommit ? { lastReviewedCommit } : {})
+    };
+
+    return handleApiOperation(
+      () => PullRequestsService.updateStatus(
+        projectKey,
+        userSlug,
+        pullRequestId,
+        repositorySlug,
+        requestBody
+      ),
+      'Error submitting pull request review'
+    );
+  }
+
 
   /**
    * Get text diff for a specific file in a pull request
@@ -615,7 +688,20 @@ export const bitbucketToolSchemas = {
     parentId: z.number().optional().describe("Parent comment ID for replies"),
     filePath: z.string().optional().describe("File path for file-specific comments"),
     line: z.number().optional().describe("Line number for line-specific comments"),
-    lineType: z.enum(['ADDED', 'REMOVED', 'CONTEXT']).optional().describe("Line type for line comments")
+    lineType: z.enum(['ADDED', 'REMOVED', 'CONTEXT']).optional().describe("Line type for line comments"),
+    pending: z.boolean().optional().describe("If true, creates a pending (draft) comment not visible to others until the review is submitted via bitbucket_submitPullRequestReview. Only works when filePath is provided — top-level PR comments (no filePath) are always posted live.")
+  },
+  getUser: {
+    userSlug: z.string().optional().describe("Exact slug of the user to look up (e.g. 'tdepole'). Use this to confirm a known slug or fetch a user's details."),
+    filter: z.string().optional().describe("Search string to find users by name or email. Use this to discover a user's slug when it is not known.")
+  },
+  submitPullRequestReview: {
+    projectKey: z.string().describe("The project key"),
+    repositorySlug: z.string().describe("The repository slug"),
+    pullRequestId: z.string().describe("The pull request ID"),
+    userSlug: z.string().describe("The username/slug of the PAT token owner — the same user whose credentials are in BITBUCKET_API_TOKEN. Resolution order: (1) author.slug from any comment posted this session, (2) reviewers/participants array from getPullRequest, (3) bitbucket_getUser with a name/email filter."),
+    status: z.enum(['APPROVED', 'NEEDS_WORK', 'UNAPPROVED']).describe("The review verdict: APPROVED, NEEDS_WORK, or UNAPPROVED"),
+    lastReviewedCommit: z.string().optional().describe("Optional hash of the last commit reviewed, used to track review progress")
   },
   getPullRequestDiff: {
     projectKey: z.string().describe("The project key"),

packages/bitbucket/src/index.ts

@@ -112,12 +112,32 @@ server.tool(
   }
 );
 
+server.tool(
+  "bitbucket_getUser",
+  "Get a Bitbucket user by their slug, or search for users by name/email to discover their slug. Use this to resolve userSlug for bitbucket_submitPullRequestReview when it is not already known from a comment response or PR participant list.",
+  bitbucketToolSchemas.getUser,
+  async ({ userSlug, filter }) => {
+    const result = await bitbucketService.getUser(userSlug, filter);
+    return formatToolResponse(result);
+  }
+);
+
 server.tool(
   "bitbucket_postPullRequestComment",
-  "Post a comment to a Bitbucket pull request",
+  "Post a comment to a Bitbucket pull request. Use pending: true to create a draft comment that is only visible to you until you call bitbucket_submitPullRequestReview. NOTE: pending only works when filePath is provided (file-level or inline comments). True top-level PR comments (no filePath) are always posted live and cannot be drafted.",
   bitbucketToolSchemas.postPullRequestComment,
-  async ({ projectKey, repositorySlug, pullRequestId, text, parentId, filePath, line, lineType }) => {
-    const result = await bitbucketService.postPullRequestComment(projectKey, repositorySlug, pullRequestId, text, parentId, filePath, line, lineType);
+  async ({ projectKey, repositorySlug, pullRequestId, text, parentId, filePath, line, lineType, pending }) => {
+    const result = await bitbucketService.postPullRequestComment(projectKey, repositorySlug, pullRequestId, text, parentId, filePath, line, lineType, pending);
+    return formatToolResponse(result);
+  }
+);
+
+server.tool(
+  "bitbucket_submitPullRequestReview",
+  "Submit a pull request review, publishing all pending (draft) comments and setting the reviewer's verdict. This is equivalent to clicking 'Submit Review' in the Bitbucket UI. Use after posting comments with pending: true. To resolve userSlug: (1) check author.slug in any comment you posted this session, (2) check the reviewers/participants array from bitbucket_getPullRequest, or (3) call bitbucket_getUser with a name/email filter as a last resort.",
+  bitbucketToolSchemas.submitPullRequestReview,
+  async ({ projectKey, repositorySlug, pullRequestId, userSlug, status, lastReviewedCommit }) => {
+    const result = await bitbucketService.submitPullRequestReview(projectKey, repositorySlug, pullRequestId, userSlug, status, lastReviewedCommit);
     return formatToolResponse(result);
   }
 );