From 4b128ff28b905cef14205879ee0592379fb3ccda Mon Sep 17 00:00:00 2001 From: "heecheol.park" Date: Thu, 18 Jun 2026 16:42:54 +0900 Subject: [PATCH 1/2] fix(comment): convert plain text to ADF for v3 and surface field errors `jira issue comment add` failed silently on Jira Cloud (API v3). The v3 endpoint requires the comment body in Atlassian Document Format, but addComment/updateComment sent a plain string, so v3 returned a 400 ("Comment body is not valid!"). That status is not in shouldFallbackApiVersion, so no v2 retry happened. The error message was also empty because formatJiraErrorMessage joined an empty errorMessages array and ignored the structured `errors` object. - Build comment bodies per API version: ADF for v3, plain text for v2, via a dedicated requestCommentWrite() that rebuilds the payload on fallback (requestApi can't, since it replays the same body). - Add a narrow v2 safety-net fallback triggered only by the specific "comment body invalid" 400 signature, not all 400s. - Parse `errors` field map in extractErrorDetail so field-level validation messages are shown instead of an empty string. Fixes #37 Co-Authored-By: Claude Opus 4.8 --- lib/jira-client.js | 89 +++++++++++++++++++++++------- tests/jira-client.test.js | 110 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 176 insertions(+), 23 deletions(-) diff --git a/lib/jira-client.js b/lib/jira-client.js index 21cb393..9e2a3fc 100644 --- a/lib/jira-client.js +++ b/lib/jira-client.js @@ -276,29 +276,41 @@ 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; + + try { + const response = await this.getApiClient(preferred).request({ method, url, data: buildData(preferred) }); + return response.data; + } catch (error) { + const canFallback = this.shouldFallbackApiVersion(error) || this.isCommentBodyRejection(error); + if (this.apiVersionMode !== 'auto' || !canFallback) throw error; + + const fallbackVersion = preferred === 3 ? 2 : 3; + const response = await this.getApiClient(fallbackVersion).request({ method, url, data: buildData(fallbackVersion) }); + this.apiVersion = fallbackVersion; + return response.data; + } } async deleteComment(commentId) { @@ -468,13 +480,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; diff --git a/tests/jira-client.test.js b/tests/jira-client.test.js index af91b52..d50287b 100644 --- a/tests/jira-client.test.js +++ b/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,52 @@ 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 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 +814,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(''); + }); + }); }); From 6e64d9311ae23c6942cc20048ec8030fbd3bc88a Mon Sep 17 00:00:00 2001 From: "heecheol.park" Date: Thu, 18 Jun 2026 16:55:02 +0900 Subject: [PATCH 2/2] fix(comment): keep non-JSON v3 fallback for comment writes requestCommentWrite() bypassed requestApi(), which dropped the isNonJsonResponse() guard for comment add/edit. On Jira Data Center/SSO setups that answer an unsupported /rest/api/3 endpoint with a 200 text/html page, the CLI would treat that HTML as a successful comment instead of falling back to v2. Mirror requestApi()'s non-JSON success check so the fallback is preserved. Co-Authored-By: Claude Opus 4.8 --- lib/jira-client.js | 10 +++++++++- tests/jira-client.test.js | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/jira-client.js b/lib/jira-client.js index 9e2a3fc..80268e2 100644 --- a/lib/jira-client.js +++ b/lib/jira-client.js @@ -298,15 +298,23 @@ class JiraClient { // 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 fallbackVersion = preferred === 3 ? 2 : 3; const response = await this.getApiClient(fallbackVersion).request({ method, url, data: buildData(fallbackVersion) }); this.apiVersion = fallbackVersion; return response.data; diff --git a/tests/jira-client.test.js b/tests/jira-client.test.js index d50287b..9174fa4 100644 --- a/tests/jira-client.test.js +++ b/tests/jira-client.test.js @@ -316,6 +316,20 @@ describe('JiraClient', () => { 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: 'login' + }); + 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);