fix(comment): convert plain text to ADF for v3 and surface field errors

lib/jira-client.js

@@ -276,29 +276,49 @@ class JiraClient {
   }
 
   async addComment(issueKey, body, options = {}) {
-    const commentData = {
-      body: body
+    const buildData = version => {
+      const commentData = { body: this.formatCommentBody(body, version) };
+      if (options.internal) {
+        commentData.visibility = { type: 'role', value: 'Administrators' };
+      }
+      return commentData;
     };
 
-    // Add visibility if internal flag is set
-    if (options.internal) {
-      commentData.visibility = {
-        type: 'role',
-        value: 'Administrators'
-      };
-    }
-
-    const response = await this.requestApi('post', `/issue/${issueKey}/comment`, commentData);
-    return response.data;
+    return this.requestCommentWrite('post', `/issue/${issueKey}/comment`, buildData);
   }
 
   async updateComment(commentId, body) {
-    const commentData = {
-      body: body
-    };
+    const buildData = version => ({ body: this.formatCommentBody(body, version) });
+    return this.requestCommentWrite('put', `/comment/${commentId}`, buildData);
+  }
 
-    const response = await this.requestApi('put', `/comment/${commentId}`, commentData);
-    return response.data;
+  // Comment bodies are version-specific: v3 (Jira Cloud) requires Atlassian
+  // Document Format while v2 accepts plain text. requestApi() can't be reused
+  // here because its fallback replays the same payload, so the body is rebuilt
+  // per version - mirroring the per-version handling in searchUsers().
+  async requestCommentWrite(method, url, buildData) {
+    const preferred = this.apiVersion;
+    const fallbackVersion = preferred === 3 ? 2 : 3;
+
+    try {
+      const response = await this.getApiClient(preferred).request({ method, url, data: buildData(preferred) });
+      // Mirror requestApi(): some Data Center/SSO setups answer an unsupported
+      // v3 endpoint with a 200 text/html page instead of an error, so a non-JSON
+      // success must still fall back to v2.
+      if (this.apiVersionMode === 'auto' && this.isNonJsonResponse(response)) {
+        const fallbackResponse = await this.getApiClient(fallbackVersion).request({ method, url, data: buildData(fallbackVersion) });
+        this.apiVersion = fallbackVersion;
+        return fallbackResponse.data;
+      }
+      return response.data;
+    } catch (error) {
+      const canFallback = this.shouldFallbackApiVersion(error) || this.isCommentBodyRejection(error);
+      if (this.apiVersionMode !== 'auto' || !canFallback) throw error;
+
+      const response = await this.getApiClient(fallbackVersion).request({ method, url, data: buildData(fallbackVersion) });
+      this.apiVersion = fallbackVersion;
+      return response.data;
+    }
   }
 
   async deleteComment(commentId) {
@@ -468,13 +488,54 @@ class JiraClient {
     }
     if (status === 403) return 'Access denied. You don\'t have permission to perform this action.';
     if (status === 404) return 'Resource not found.';
-    return data?.errorMessages ? data.errorMessages.join(', ') : 'API request failed';
+    return this.extractErrorDetail(data) || 'API request failed';
+  }
+
+  extractErrorDetail(data) {
+    if (!data || typeof data !== 'object') {
+      return typeof data === 'string' ? data : '';
+    }
+    if (Array.isArray(data.errorMessages) && data.errorMessages.length > 0) {
+      return data.errorMessages.join(', ');
+    }
+    if (data.errors && typeof data.errors === 'object') {
+      const parts = Object.entries(data.errors).map(([field, message]) => `${field}: ${message}`);
+      if (parts.length > 0) return parts.join(', ');
+    }
+    return '';
   }
 
   getApiClient(version) {
     return version === 2 ? this.clientV2 : this.clientV3;
   }
 
+  formatCommentBody(body, version) {
+    // v3 requires Atlassian Document Format; v2 accepts plain text. A body that
+    // is already an object (pre-built ADF) is passed through untouched.
+    if (version === 3 && typeof body === 'string') return this.toADF(body);
+    return body;
+  }
+
+  toADF(text) {
+    const source = text == null ? '' : String(text);
+    const content = source.split(/\n{2,}/).map(block => {
+      const nodes = [];
+      block.split('\n').forEach((line, index) => {
+        if (index > 0) nodes.push({ type: 'hardBreak' });
+        if (line.length > 0) nodes.push({ type: 'text', text: line });
+      });
+      return nodes.length > 0 ? { type: 'paragraph', content: nodes } : { type: 'paragraph' };
+    });
+
+    return { type: 'doc', version: 1, content };
+  }
+
+  isCommentBodyRejection(error) {
+    if (!error || error.status !== 400) return false;
+    const commentError = error.data?.errors?.comment;
+    return typeof commentError === 'string' && commentError.length > 0;
+  }
+
   shouldFallbackApiVersion(error) {
     if (!error || typeof error !== 'object') return false;
     if (error.status === 404 || error.status === 410) return true;

tests/jira-client.test.js

@@ -256,13 +256,19 @@ describe('JiraClient', () => {
       expect(result).toEqual(mockComments);
     });
 
-    test('addComment should make correct API call', async () => {
+    test('addComment should send ADF body on v3', async () => {
       const mockComment = { id: '10001', body: 'New comment' };
       client.clientV3.request.mockResolvedValue({ data: mockComment });
 
       const result = await client.addComment('TEST-1', 'New comment');
 
-      expect(client.clientV3.request).toHaveBeenCalledWith({ method: 'post', url: '/issue/TEST-1/comment', data: { body: 'New comment' } });
+      expect(client.clientV3.request).toHaveBeenCalledWith({ method: 'post', url: '/issue/TEST-1/comment', data: {
+        body: {
+          type: 'doc',
+          version: 1,
+          content: [{ type: 'paragraph', content: [{ type: 'text', text: 'New comment' }] }]
+        }
+      } });
       expect(result).toEqual(mockComment);
     });
 
@@ -273,7 +279,11 @@ describe('JiraClient', () => {
       const result = await client.addComment('TEST-1', 'Internal comment', { internal: true });
 
       expect(client.clientV3.request).toHaveBeenCalledWith({ method: 'post', url: '/issue/TEST-1/comment', data: {
-        body: 'Internal comment',
+        body: {
+          type: 'doc',
+          version: 1,
+          content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Internal comment' }] }]
+        },
         visibility: {
           type: 'role',
           value: 'Administrators'
@@ -282,13 +292,66 @@ describe('JiraClient', () => {
       expect(result).toEqual(mockComment);
     });
 
-    test('updateComment should make correct API call', async () => {
+    test('addComment sends plain text body when pinned to v2', async () => {
+      const v2Client = new JiraClient({ ...mockConfig, apiVersion: 2 });
+      v2Client.clientV2.request = jest.fn().mockResolvedValue({ data: { id: '10001' } });
+
+      await v2Client.addComment('TEST-1', 'Plain text');
+
+      expect(v2Client.clientV2.request).toHaveBeenCalledWith({ method: 'post', url: '/issue/TEST-1/comment', data: { body: 'Plain text' } });
+    });
+
+    test('addComment falls back to v2 plain text when v3 rejects the body', async () => {
+      const adfRejection = Object.assign(new Error('Comment body is not valid!'), {
+        status: 400,
+        data: { errorMessages: [], errors: { comment: 'Comment body is not valid!' } }
+      });
+      client.clientV3.request.mockRejectedValue(adfRejection);
+      client.clientV2.request = jest.fn().mockResolvedValue({ data: { id: '10001' } });
+
+      const result = await client.addComment('TEST-1', 'New comment');
+
+      expect(client.clientV2.request).toHaveBeenCalledWith({ method: 'post', url: '/issue/TEST-1/comment', data: { body: 'New comment' } });
+      expect(client.apiVersion).toBe(2);
+      expect(result).toEqual({ id: '10001' });
+    });
+
+    test('addComment falls back to v2 when v3 returns a non-JSON 200 response', async () => {
+      client.clientV3.request.mockResolvedValue({
+        headers: { 'content-type': 'text/html' },
+        data: '<html>login</html>'
+      });
+      client.clientV2.request = jest.fn().mockResolvedValue({ data: { id: '10001' } });
+
+      const result = await client.addComment('TEST-1', 'New comment');
+
+      expect(client.clientV2.request).toHaveBeenCalledWith({ method: 'post', url: '/issue/TEST-1/comment', data: { body: 'New comment' } });
+      expect(client.apiVersion).toBe(2);
+      expect(result).toEqual({ id: '10001' });
+    });
+
+    test('addComment does not fall back on unrelated 400 errors', async () => {
+      const badRequest = Object.assign(new Error('Bad request'), { status: 400, data: {} });
+      client.clientV3.request.mockRejectedValue(badRequest);
+      client.clientV2.request = jest.fn();
+
+      await expect(client.addComment('TEST-1', 'New comment')).rejects.toBe(badRequest);
+      expect(client.clientV2.request).not.toHaveBeenCalled();
+    });
+
+    test('updateComment should send ADF body on v3', async () => {
       const mockComment = { id: '10000', body: 'Updated comment' };
       client.clientV3.request.mockResolvedValue({ data: mockComment });
 
       const result = await client.updateComment('10000', 'Updated comment');
 
-      expect(client.clientV3.request).toHaveBeenCalledWith({ method: 'put', url: '/comment/10000', data: { body: 'Updated comment' } });
+      expect(client.clientV3.request).toHaveBeenCalledWith({ method: 'put', url: '/comment/10000', data: {
+        body: {
+          type: 'doc',
+          version: 1,
+          content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Updated comment' }] }]
+        }
+      } });
       expect(result).toEqual(mockComment);
     });
 
@@ -765,4 +828,55 @@ describe('JiraClient', () => {
       });
     });
   });
+
+  describe('toADF', () => {
+    test('wraps plain text in a single paragraph', () => {
+      expect(client.toADF('hello')).toEqual({
+        type: 'doc',
+        version: 1,
+        content: [{ type: 'paragraph', content: [{ type: 'text', text: 'hello' }] }]
+      });
+    });
+
+    test('splits blank-line-separated blocks into paragraphs', () => {
+      const adf = client.toADF('first\n\nsecond');
+      expect(adf.content).toEqual([
+        { type: 'paragraph', content: [{ type: 'text', text: 'first' }] },
+        { type: 'paragraph', content: [{ type: 'text', text: 'second' }] }
+      ]);
+    });
+
+    test('keeps single newlines as hard breaks within a paragraph', () => {
+      const adf = client.toADF('line one\nline two');
+      expect(adf.content).toEqual([
+        {
+          type: 'paragraph',
+          content: [
+            { type: 'text', text: 'line one' },
+            { type: 'hardBreak' },
+            { type: 'text', text: 'line two' }
+          ]
+        }
+      ]);
+    });
+
+    test('represents empty input as an empty paragraph', () => {
+      expect(client.toADF('').content).toEqual([{ type: 'paragraph' }]);
+    });
+  });
+
+  describe('extractErrorDetail', () => {
+    test('joins errorMessages when present', () => {
+      expect(client.extractErrorDetail({ errorMessages: ['boom', 'bang'], errors: {} })).toBe('boom, bang');
+    });
+
+    test('falls back to the errors object when errorMessages is empty', () => {
+      expect(client.extractErrorDetail({ errorMessages: [], errors: { comment: 'Comment body is not valid!' } }))
+        .toBe('comment: Comment body is not valid!');
+    });
+
+    test('returns empty string when no detail is available', () => {
+      expect(client.extractErrorDetail({ errorMessages: [], errors: {} })).toBe('');
+    });
+  });
 });