From 28d992ac789c26ace0cd189f50ca1e504853d68b Mon Sep 17 00:00:00 2001 From: bot-hermes Date: Thu, 13 Aug 2026 18:43:28 +0000 Subject: [PATCH] feat(web): create, update and merge GitLab merge requests --- packages/ui/src/lib/api/types.ts | 43 +++ .../web/server/lib/gitlab/DOCUMENTATION.md | 18 +- packages/web/server/lib/gitlab/client.js | 6 + packages/web/server/lib/gitlab/client.test.js | 59 ++++ packages/web/server/lib/gitlab/routes.js | 209 +++++++++++++ packages/web/server/lib/gitlab/routes.test.js | 295 +++++++++++++++++- packages/web/src/api/gitlab.test.ts | 118 +++++++ packages/web/src/api/gitlab.ts | 57 ++++ 8 files changed, 799 insertions(+), 6 deletions(-) diff --git a/packages/ui/src/lib/api/types.ts b/packages/ui/src/lib/api/types.ts index 5094f1ee..f1999077 100644 --- a/packages/ui/src/lib/api/types.ts +++ b/packages/ui/src/lib/api/types.ts @@ -1260,6 +1260,46 @@ export type GitLabBranchesResult = { branches: string[]; }; +export type GitLabMergeRequestCreateInput = { + directory: string; + title: string; + sourceBranch: string; + targetBranch: string; + description?: string; + removeSourceBranch?: boolean; +}; + +export type GitLabMergeRequestUpdateInput = { + directory: string; + number: number; + title?: string; + description?: string; +}; + +export type GitLabMergeRequestMergeInput = { + directory: string; + number: number; + squash?: boolean; +}; + +export type GitLabMergeRequestCreateResult = { + connected: boolean; + repo?: GitLabRepoRef | null; + mr?: GitLabMergeRequest; +}; + +export type GitLabMergeRequestUpdateResult = { + connected: boolean; + repo?: GitLabRepoRef | null; + mr?: GitLabMergeRequest; +}; + +export type GitLabMergeRequestMergeResult = { + connected: boolean; + merged: boolean; + message?: string; +}; + type GitLabAuthAccount = { id: string; user: { @@ -1296,6 +1336,9 @@ export interface GitLabAPI { number: number, options?: { includeDiff?: boolean; namespace?: string; project?: string } ): Promise; + mrCreate(input: GitLabMergeRequestCreateInput): Promise; + mrUpdate(input: GitLabMergeRequestUpdateInput): Promise; + mrMerge(input: GitLabMergeRequestMergeInput): Promise; repoBranches(namespace: string, project: string): Promise; } diff --git a/packages/web/server/lib/gitlab/DOCUMENTATION.md b/packages/web/server/lib/gitlab/DOCUMENTATION.md index ead6b015..c59982c1 100644 --- a/packages/web/server/lib/gitlab/DOCUMENTATION.md +++ b/packages/web/server/lib/gitlab/DOCUMENTATION.md @@ -2,8 +2,8 @@ ## Purpose -- This module owns GitLab auth (Personal Access Token), raw REST v4 client access, remote-URL repo resolution, and read-only GitLab issue / merge-request (MR) APIs for OpenChamber. -- From a user perspective, this is the layer that lets the app show GitLab issues and merge requests for a local project, including comments and per-file diffs. +- This module owns GitLab auth (Personal Access Token), raw REST v4 client access, remote-URL repo resolution, and GitLab issue / merge-request (MR) APIs for OpenChamber, including MR create/update/merge writes. +- From a user perspective, this is the layer that lets the app show GitLab issues and merge requests for a local project, including comments and per-file diffs, and create, edit, and merge merge requests. - The module mirrors `packages/web/server/lib/github/` but uses a **Personal Access Token (PAT)** with a configurable base URL (gitlab.com by default, or a self-hosted instance), and talks to GitLab's REST v4 API directly via `fetch` — no new dependencies. ## Entrypoints and structure @@ -32,7 +32,7 @@ ### Client (`client.js`) -- `createGitLabClient({ token, baseUrl })`: raw-fetch REST v4 client with `request(path, { method, query, body })` plus convenience methods `user()`, `project(path)`, `issues(path, params)`, `issue(path, iid)`, `issueNotes(path, iid, params)`, `mergeRequests(path, params)`, `mergeRequest(path, iid)`, `mergeRequestDiffs(path, iid, params)`, `branches(path, params)`. +- `createGitLabClient({ token, baseUrl })`: raw-fetch REST v4 client with `request(path, { method, query, body })` plus convenience methods `user()`, `project(path)`, `issues(path, params)`, `issue(path, iid)`, `issueNotes(path, iid, params)`, `mergeRequests(path, params)`, `mergeRequest(path, iid)`, `mergeRequestDiffs(path, iid, params)`, `createMergeRequest(path, body)`, `updateMergeRequest(path, iid, body)`, `mergeMergeRequest(path, iid, body)`, `branches(path, params)`. - `getGitLabClientOrNull()`: client for the current account, or `null`. - `isGitLabRateLimited()` / `noteGitLabRateLimit(error)`: own module-level rate-limit cooldown (not shared with the GitHub module's `rate-limit.js`). @@ -78,6 +78,9 @@ Nothing in the client or repo layers assumes the token came from a PAT. - MR detail: `GET /projects/:id/merge_requests/:merge_request_iid`. - MR diffs: `GET /projects/:id/merge_requests/:merge_request_iid/diffs?per_page=100&page=N` (paginated; the route caps at 10 pages / 3000 files). - MR notes: `GET /projects/:id/merge_requests/:merge_request_iid/notes?per_page=100`. +- MR create: `POST /projects/:id/merge_requests` with `{ source_branch, target_branch, title, description?, remove_source_branch }` (description omitted when absent; `remove_source_branch` defaults to `false`). +- MR update: `PUT /projects/:id/merge_requests/:merge_request_iid` with `{ title?, description? }` (undefined fields omitted). +- MR merge: `PUT /projects/:id/merge_requests/:merge_request_iid/merge` with `{ squash? }`. - Branches: `GET /projects/:id/repository/branches?per_page=100&page=N`. - User: `GET /user` -> `{ id, username, name, state, avatar_url, web_url, email, ... }`. @@ -95,6 +98,9 @@ Nothing in the client or repo layers assumes the token came from a PAT. | GET | `/api/gitlab/issues/comments` | `?directory&number&namespace&project` -> `{ connected, repo?, comments[] }` | | GET | `/api/gitlab/mrs/list` | `?directory&page&query&sourceBranch` -> `{ connected, repo?, mrs[], page, hasMore }` | | GET | `/api/gitlab/mrs/context` | `?directory&number&diff&namespace&project` -> `{ connected, repo?, mr, comments[], files[], diff? }` | +| POST | `/api/gitlab/mrs/create` | body `{ directory, title, sourceBranch, targetBranch, description?, removeSourceBranch? }` -> `{ connected, repo?, mr }`; `400` for missing fields, unresolvable repo, or a token without the `api` scope | +| PUT | `/api/gitlab/mrs/update` | body `{ directory, number, title?, description? }` -> `{ connected, repo?, mr }`; `404` when the MR does not exist | +| PUT | `/api/gitlab/mrs/merge` | body `{ directory, number, squash? }` -> `{ connected, merged: true }` on success; non-mergeable MRs -> the GitLab status (`405`/`406`/`409`/`422`) with `{ connected, merged: false, message }` | | GET | `/api/gitlab/repo/branches` | `?namespace&project` -> `{ branches[] }` | Conventions mirror `github/routes.js`: @@ -114,8 +120,10 @@ Conventions mirror `github/routes.js`: ## Failure handling - If GitLab is disconnected, read routes return `connected: false`. -- A repo that does not resolve from the local git remote yields `repo: null` with empty lists, matching the GitHub behavior. +- A repo that does not resolve from the local git remote yields `repo: null` with empty lists, matching the GitHub behavior. Write routes reject an unresolvable repo with `400 { error: 'Unable to resolve GitLab repo from directory' }`. - Invalid/expired tokens are cleared on `401`/`403` and reported as disconnected. +- GitLab `403` on write routes means the token lacks the `api` scope; they respond `400 { error: 'Your GitLab token needs the api scope to ...' }`. +- MR merge rejections (`405`/`406`/`409`/`422` from GitLab) are surfaced as `{ connected, merged: false, message }` with the GitLab status so clients can show the message without treating it as a transport error (mirrors `github/pr/merge`). - Rate-limit and timeout failures surface explicit `503` responses so clients keep last-known state rather than clearing UI. ## Notes for contributors @@ -124,4 +132,4 @@ Conventions mirror `github/routes.js`: - Never log tokens. Error messages must not include the access token. - Do not double-encode project paths; convenience methods already call `encodeURIComponent` on the `pathWithNamespace`. - The ETag cache and rate-limit cooldown are module-level and per-instance — they are NOT shared with the GitHub module. -- To add GitLab write operations (comment, assign, merge), add the endpoint in `routes.js`, add a convenience method in `client.js`, and extend the shared types — mirror the GitHub PR write routes. +- To add further GitLab write operations (comment, assign, issue writes), add the endpoint in `routes.js`, add a convenience method in `client.js`, and extend the shared types — mirror the existing MR write routes and the GitHub PR write routes. diff --git a/packages/web/server/lib/gitlab/client.js b/packages/web/server/lib/gitlab/client.js index 66567fd5..bb0e8e0d 100644 --- a/packages/web/server/lib/gitlab/client.js +++ b/packages/web/server/lib/gitlab/client.js @@ -267,6 +267,12 @@ export function createGitLabClient({ token, baseUrl }) { request(`/projects/${encodeProject(pathWithNamespace)}/merge_requests/${iid}`), mergeRequestDiffs: (pathWithNamespace, iid, params = {}) => request(`/projects/${encodeProject(pathWithNamespace)}/merge_requests/${iid}/diffs`, { query: params }), + createMergeRequest: (pathWithNamespace, body) => + request(`/projects/${encodeProject(pathWithNamespace)}/merge_requests`, { method: 'POST', body }), + updateMergeRequest: (pathWithNamespace, iid, body) => + request(`/projects/${encodeProject(pathWithNamespace)}/merge_requests/${iid}`, { method: 'PUT', body }), + mergeMergeRequest: (pathWithNamespace, iid, body) => + request(`/projects/${encodeProject(pathWithNamespace)}/merge_requests/${iid}/merge`, { method: 'PUT', body }), branches: (pathWithNamespace, params = {}) => request(`/projects/${encodeProject(pathWithNamespace)}/repository/branches`, { query: params }), }; diff --git a/packages/web/server/lib/gitlab/client.test.js b/packages/web/server/lib/gitlab/client.test.js index e4eace7e..6bf00d0b 100644 --- a/packages/web/server/lib/gitlab/client.test.js +++ b/packages/web/server/lib/gitlab/client.test.js @@ -214,6 +214,65 @@ describe('etag conditional cache', () => { }); }); +describe('merge request write methods', () => { + test('createMergeRequest POSTs a JSON body to the merge_requests endpoint', async () => { + const fetchMock = vi.fn(async () => jsonResponse({ iid: 5, title: 'New MR' }, { status: 201 })); + globalThis.fetch = fetchMock; + + const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); + const result = await client.createMergeRequest('group/sub', { + source_branch: 'feat/x', + target_branch: 'main', + title: 'New MR', + }); + + const [url, options] = fetchMock.mock.calls[0]; + expect(String(url)).toBe('https://gitlab.com/api/v4/projects/group%2Fsub/merge_requests'); + expect(options.method).toBe('POST'); + expect(options.headers['content-type']).toBe('application/json'); + expect(JSON.parse(options.body)).toEqual({ source_branch: 'feat/x', target_branch: 'main', title: 'New MR' }); + expect(result.status).toBe(201); + expect(result.data).toEqual({ iid: 5, title: 'New MR' }); + }); + + test('updateMergeRequest PUTs a JSON body to the merge request endpoint', async () => { + const fetchMock = vi.fn(async () => jsonResponse({ iid: 5, title: 'Updated' })); + globalThis.fetch = fetchMock; + + const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); + await client.updateMergeRequest('group/sub', 5, { title: 'Updated', description: 'Body text' }); + + const [url, options] = fetchMock.mock.calls[0]; + expect(String(url)).toBe('https://gitlab.com/api/v4/projects/group%2Fsub/merge_requests/5'); + expect(options.method).toBe('PUT'); + expect(options.headers['content-type']).toBe('application/json'); + expect(JSON.parse(options.body)).toEqual({ title: 'Updated', description: 'Body text' }); + }); + + test('mergeMergeRequest PUTs a JSON body to the merge endpoint', async () => { + const fetchMock = vi.fn(async () => jsonResponse({ iid: 5, state: 'merged' })); + globalThis.fetch = fetchMock; + + const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); + await client.mergeMergeRequest('group/sub', 5, { squash: true }); + + const [url, options] = fetchMock.mock.calls[0]; + expect(String(url)).toBe('https://gitlab.com/api/v4/projects/group%2Fsub/merge_requests/5/merge'); + expect(options.method).toBe('PUT'); + expect(JSON.parse(options.body)).toEqual({ squash: true }); + }); + + test('write methods surface error statuses without throwing', async () => { + const fetchMock = vi.fn(async () => jsonResponse({ message: 'Method Not Allowed' }, { status: 405 })); + globalThis.fetch = fetchMock; + + const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); + const result = await client.mergeMergeRequest('group/sub', 5, {}); + expect(result.status).toBe(405); + expect(result.data).toEqual({ message: 'Method Not Allowed' }); + }); +}); + describe('rate limiting', () => { // NOTE: these tests run last in this file. The rate-limit cooldown is // module-level and has no reset export, so earlier tests must not set one. diff --git a/packages/web/server/lib/gitlab/routes.js b/packages/web/server/lib/gitlab/routes.js index e825ad6a..4319a173 100644 --- a/packages/web/server/lib/gitlab/routes.js +++ b/packages/web/server/lib/gitlab/routes.js @@ -57,6 +57,7 @@ const mapAuthor = (author) => { username: typeof author.username === 'string' ? author.username : null, name: typeof author.name === 'string' ? author.name : null, avatarUrl: typeof author.avatar_url === 'string' ? author.avatar_url : null, + webUrl: typeof author.web_url === 'string' ? author.web_url : null, id: typeof author.id === 'number' ? author.id : null, }; }; @@ -90,6 +91,34 @@ const mapComment = (note, webUrl) => ({ author: mapAuthor(note.author) || {}, }); +// GitLab error bodies carry `message` as a string ("405 Method Not Allowed") or +// as a field->errors object ({ title: ['is invalid'] }); some endpoints use an +// `error` field instead. Flatten whichever shape is present into one readable +// string so write routes can surface it in { error } or { message }. +const gitLabErrorMessage = (data) => { + if (!data || typeof data !== 'object') { + return null; + } + const message = data.message; + if (typeof message === 'string' && message) { + return message; + } + if (message && typeof message === 'object') { + const parts = Object.entries(message).map(([field, errors]) => { + const list = Array.isArray(errors) ? errors : [errors]; + const detail = list.filter((item) => typeof item === 'string' && item).join(', '); + return detail ? `${field}: ${detail}` : field; + }); + if (parts.length > 0) { + return parts.join('; '); + } + } + if (typeof data.error === 'string' && data.error) { + return data.error; + } + return null; +}; + const countDiffLines = (diffText) => { if (typeof diffText !== 'string') { return { additions: 0, deletions: 0, changes: 0 }; @@ -655,6 +684,186 @@ export function registerGitLabRoutes(app, options = {}) { } }); + // ================= GitLab Merge Request Write APIs ================= + + app.post('/api/gitlab/mrs/create', async (req, res) => { + try { + const directory = asString(req.body?.directory); + const title = asString(req.body?.title); + const sourceBranch = asString(req.body?.sourceBranch); + const targetBranch = asString(req.body?.targetBranch); + if (!directory || !title || !sourceBranch || !targetBranch) { + return res.status(400).json({ error: 'directory, title, sourceBranch, targetBranch are required' }); + } + const description = typeof req.body?.description === 'string' && req.body.description + ? req.body.description + : undefined; + const removeSourceBranch = typeof req.body?.removeSourceBranch === 'boolean' + ? req.body.removeSourceBranch + : false; + + const client = await getClient(); + if (!client) { + return res.json({ connected: false }); + } + + const requestedProject = getRequestedProject(req); + const { projectPath, repo } = await resolveProjectForRequest(directory, requestedProject); + if (!projectPath) { + return res.status(400).json({ error: 'Unable to resolve GitLab repo from directory' }); + } + + const body = { + source_branch: sourceBranch, + target_branch: targetBranch, + title, + remove_source_branch: removeSourceBranch, + }; + if (description !== undefined) { + body.description = description; + } + + const resp = await withTimeout(client.createMergeRequest(projectPath, body), ROUTE_TIMEOUT_MS, 'gitlab mr create'); + if (resp.status === 429) { + return res.status(503).json({ error: 'GitLab rate limited' }); + } + if (resp.status === 403) { + return res.status(400).json({ error: 'Your GitLab token needs the api scope to create merge requests' }); + } + if (resp.status !== 200 && resp.status !== 201) { + const status = resp.status >= 500 ? 500 : 400; + return res.status(status).json({ error: gitLabErrorMessage(resp.data) || 'GitLab returned an error while creating the merge request' }); + } + if (!resp.data) { + return res.status(500).json({ error: 'GitLab returned an empty response while creating the merge request' }); + } + + return res.json({ + connected: true, + repo: repo || repoRefFromProjectPath(projectPath, client.baseUrl), + mr: mapMergeRequestSummary(resp.data), + }); + } catch (error) { + console.error('Failed to create GitLab merge request:', error); + return res.status(500).json({ error: error.message || 'Failed to create GitLab merge request' }); + } + }); + + app.put('/api/gitlab/mrs/update', async (req, res) => { + try { + const directory = asString(req.body?.directory); + const number = typeof req.body?.number === 'number' ? req.body.number : null; + if (!directory || !number) { + return res.status(400).json({ error: 'directory and number are required' }); + } + const title = asString(req.body?.title); + const description = typeof req.body?.description === 'string' ? req.body.description : undefined; + + const client = await getClient(); + if (!client) { + return res.json({ connected: false }); + } + + const requestedProject = getRequestedProject(req); + const { projectPath, repo } = await resolveProjectForRequest(directory, requestedProject); + if (!projectPath) { + return res.status(400).json({ error: 'Unable to resolve GitLab repo from directory' }); + } + + const body = {}; + if (title) { + body.title = title; + } + if (description !== undefined) { + body.description = description; + } + + const resp = await withTimeout(client.updateMergeRequest(projectPath, number, body), ROUTE_TIMEOUT_MS, 'gitlab mr update'); + if (resp.status === 429) { + return res.status(503).json({ error: 'GitLab rate limited' }); + } + if (resp.status === 403) { + return res.status(400).json({ error: 'Your GitLab token needs the api scope to update merge requests' }); + } + if (resp.status === 404) { + return res.status(404).json({ error: 'Merge request not found' }); + } + if (resp.status !== 200 && resp.status !== 201) { + const status = resp.status >= 500 ? 500 : 400; + return res.status(status).json({ error: gitLabErrorMessage(resp.data) || 'GitLab returned an error while updating the merge request' }); + } + if (!resp.data) { + return res.status(500).json({ error: 'GitLab returned an empty response while updating the merge request' }); + } + + return res.json({ + connected: true, + repo: repo || repoRefFromProjectPath(projectPath, client.baseUrl), + mr: mapMergeRequestSummary(resp.data), + }); + } catch (error) { + console.error('Failed to update GitLab merge request:', error); + return res.status(500).json({ error: error.message || 'Failed to update GitLab merge request' }); + } + }); + + app.put('/api/gitlab/mrs/merge', async (req, res) => { + try { + const directory = asString(req.body?.directory); + const number = typeof req.body?.number === 'number' ? req.body.number : null; + if (!directory || !number) { + return res.status(400).json({ error: 'directory and number are required' }); + } + const squash = typeof req.body?.squash === 'boolean' ? req.body.squash : undefined; + + const client = await getClient(); + if (!client) { + return res.json({ connected: false }); + } + + const requestedProject = getRequestedProject(req); + const { projectPath, repo } = await resolveProjectForRequest(directory, requestedProject); + if (!projectPath) { + return res.status(400).json({ error: 'Unable to resolve GitLab repo from directory' }); + } + + const body = {}; + if (squash !== undefined) { + body.squash = squash; + } + + const resp = await withTimeout(client.mergeMergeRequest(projectPath, number, body), ROUTE_TIMEOUT_MS, 'gitlab mr merge'); + if (resp.status === 429) { + return res.status(503).json({ error: 'GitLab rate limited' }); + } + if (resp.status === 403) { + return res.status(400).json({ error: 'Your GitLab token needs the api scope to create merge requests' }); + } + if (resp.status === 404) { + return res.status(404).json({ error: 'Merge request not found' }); + } + // GitLab rejects non-mergeable requests with 405/406/409/422 and a + // `message` in the body — surface it as a merge rejection (mirrors the + // GitHub pr/merge contract) instead of a generic error. + if (resp.status === 405 || resp.status === 406 || resp.status === 409 || resp.status === 422) { + return res.status(resp.status).json({ + connected: true, + merged: false, + message: gitLabErrorMessage(resp.data) || 'Merge request not mergeable', + }); + } + if (resp.status !== 200 && resp.status !== 201) { + const status = resp.status >= 500 ? 500 : 400; + return res.status(status).json({ error: gitLabErrorMessage(resp.data) || 'GitLab returned an error while merging the merge request' }); + } + + return res.json({ connected: true, merged: true }); + } catch (error) { + console.error('Failed to merge GitLab merge request:', error); + return res.status(500).json({ error: error.message || 'Failed to merge GitLab merge request' }); + } + }); + // ================= GitLab Repo APIs ================= app.get('/api/gitlab/repo/branches', async (req, res) => { diff --git a/packages/web/server/lib/gitlab/routes.test.js b/packages/web/server/lib/gitlab/routes.test.js index 82ac6721..1f5725bf 100644 --- a/packages/web/server/lib/gitlab/routes.test.js +++ b/packages/web/server/lib/gitlab/routes.test.js @@ -350,7 +350,7 @@ describe('GitLab data routes', () => { body: 'Looks good to me', createdAt: '2026-01-01T01:00:00Z', updatedAt: undefined, - author: { username: 'alice', name: 'Alice Example', avatarUrl: null, id: 42 }, + author: { username: 'alice', name: 'Alice Example', avatarUrl: null, id: 42, webUrl: null }, }, ]); }); @@ -510,6 +510,299 @@ describe('GitLab data routes', () => { expect(response.body).toEqual({ error: 'namespace and project are required' }); }); + test('mrs/create POSTs source/target/title and returns the created MR summary', async () => { + const createdMr = { + iid: 12, + title: 'Add feature', + web_url: 'https://gitlab.com/group/sub/-/merge_requests/12', + state: 'opened', + draft: false, + work_in_progress: false, + author: { + id: 42, + username: 'alice', + name: 'Alice Example', + avatar_url: 'https://gitlab.com/alice.png', + web_url: 'https://gitlab.com/alice', + }, + source_branch: 'feat/add', + target_branch: 'main', + }; + const fetchMock = scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests$/)(url) && options.method === 'POST') { + return jsonResponse(createdMr, { status: 201 }); + } + return null; + }, + ]); + + const app = createApp(); + const response = await request(app) + .post('/api/gitlab/mrs/create') + .send({ + directory: '/tmp/work', + title: 'Add feature', + sourceBranch: 'feat/add', + targetBranch: 'main', + description: 'Adds the feature', + removeSourceBranch: true, + }); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + connected: true, + repo: { namespace: 'group', project: 'sub', host: 'gitlab.com' }, + mr: { + number: 12, + title: 'Add feature', + url: 'https://gitlab.com/group/sub/-/merge_requests/12', + state: 'opened', + draft: false, + author: { + username: 'alice', + name: 'Alice Example', + avatarUrl: 'https://gitlab.com/alice.png', + webUrl: 'https://gitlab.com/alice', + }, + sourceBranch: 'feat/add', + targetBranch: 'main', + }, + }); + + const [, options] = fetchMock.mock.calls[0]; + expect(options.method).toBe('POST'); + expect(JSON.parse(options.body)).toEqual({ + source_branch: 'feat/add', + target_branch: 'main', + title: 'Add feature', + description: 'Adds the feature', + remove_source_branch: true, + }); + }); + + test('mrs/create defaults remove_source_branch to false and omits description', async () => { + const fetchMock = scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests$/)(url) && options.method === 'POST') { + return jsonResponse( + { iid: 1, title: 'T', web_url: 'u', state: 'opened', draft: false, author: {}, source_branch: 's', target_branch: 'm' }, + { status: 201 }, + ); + } + return null; + }, + ]); + + const app = createApp(); + await request(app) + .post('/api/gitlab/mrs/create') + .send({ directory: '/tmp/work', title: 'T', sourceBranch: 's', targetBranch: 'm' }); + + const [, options] = fetchMock.mock.calls[0]; + const body = JSON.parse(options.body); + expect(body).toEqual({ source_branch: 's', target_branch: 'm', title: 'T', remove_source_branch: false }); + expect(body.description).toBeUndefined(); + }); + + test('mrs/create rejects missing fields with 400', async () => { + const app = createApp(); + const response = await request(app) + .post('/api/gitlab/mrs/create') + .send({ directory: '/tmp/work', title: 'Add feature' }); + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: 'directory, title, sourceBranch, targetBranch are required' }); + }); + + test('mrs/create reports connected:false when not authenticated', async () => { + clearGitLabAuth(); + const app = createApp(); + const response = await request(app) + .post('/api/gitlab/mrs/create') + .send({ directory: '/tmp/work', title: 'Add feature', sourceBranch: 'feat/add', targetBranch: 'main' }); + expect(response.status).toBe(200); + expect(response.body).toEqual({ connected: false }); + }); + + test('mrs/create surfaces a 403 as an api-scope error', async () => { + scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests$/)(url) && options.method === 'POST') { + return jsonResponse({ message: '403 Forbidden' }, { status: 403 }); + } + return null; + }, + ]); + + const app = createApp(); + const response = await request(app) + .post('/api/gitlab/mrs/create') + .send({ directory: '/tmp/work', title: 'Add feature', sourceBranch: 'feat/add', targetBranch: 'main' }); + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: 'Your GitLab token needs the api scope to create merge requests' }); + }); + + test('mrs/create surfaces GitLab validation errors with the api message', async () => { + scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests$/)(url) && options.method === 'POST') { + return jsonResponse({ message: { source_branch: ['is missing'], title: ['is invalid'] } }, { status: 400 }); + } + return null; + }, + ]); + + const app = createApp(); + const response = await request(app) + .post('/api/gitlab/mrs/create') + .send({ directory: '/tmp/work', title: 'Add feature', sourceBranch: 'feat/add', targetBranch: 'main' }); + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: 'source_branch: is missing; title: is invalid' }); + }); + + test('mrs/update PUTs title/description and returns the updated MR summary', async () => { + const fetchMock = scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests\/12$/)(url) && options.method === 'PUT') { + return jsonResponse({ + iid: 12, + title: 'Updated title', + web_url: 'https://gitlab.com/group/sub/-/merge_requests/12', + state: 'opened', + draft: false, + work_in_progress: false, + author: { id: 42, username: 'alice', name: 'Alice Example', avatar_url: 'https://gitlab.com/alice.png' }, + source_branch: 'feat/add', + target_branch: 'main', + }); + } + return null; + }, + ]); + + const app = createApp(); + const response = await request(app) + .put('/api/gitlab/mrs/update') + .send({ directory: '/tmp/work', number: 12, title: 'Updated title', description: 'New body' }); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + connected: true, + mr: { number: 12, title: 'Updated title', state: 'opened', sourceBranch: 'feat/add', targetBranch: 'main' }, + }); + const [, options] = fetchMock.mock.calls[0]; + expect(options.method).toBe('PUT'); + expect(JSON.parse(options.body)).toEqual({ title: 'Updated title', description: 'New body' }); + }); + + test('mrs/update omits title/description when not provided', async () => { + const fetchMock = scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests\/12$/)(url) && options.method === 'PUT') { + return jsonResponse({ + iid: 12, + title: 'T', + web_url: 'u', + state: 'opened', + draft: false, + author: {}, + source_branch: 's', + target_branch: 'm', + }); + } + return null; + }, + ]); + + const app = createApp(); + await request(app).put('/api/gitlab/mrs/update').send({ directory: '/tmp/work', number: 12 }); + + const [, options] = fetchMock.mock.calls[0]; + expect(JSON.parse(options.body)).toEqual({}); + }); + + test('mrs/update returns 404 for a missing merge request', async () => { + scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests\/999$/)(url) && options.method === 'PUT') { + return jsonResponse({ message: '404 Not Found' }, { status: 404 }); + } + return null; + }, + ]); + + const app = createApp(); + const response = await request(app) + .put('/api/gitlab/mrs/update') + .send({ directory: '/tmp/work', number: 999, title: 'x' }); + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Merge request not found' }); + }); + + test('mrs/merge PUTs squash and reports merged:true', async () => { + const fetchMock = scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests\/12\/merge$/)(url) && options.method === 'PUT') { + return jsonResponse({ iid: 12, state: 'merged' }); + } + return null; + }, + ]); + + const app = createApp(); + const response = await request(app) + .put('/api/gitlab/mrs/merge') + .send({ directory: '/tmp/work', number: 12, squash: true }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ connected: true, merged: true }); + const [, options] = fetchMock.mock.calls[0]; + expect(options.method).toBe('PUT'); + expect(JSON.parse(options.body)).toEqual({ squash: true }); + }); + + test('mrs/merge passes through a GitLab merge rejection as merged:false', async () => { + scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests\/12\/merge$/)(url) && options.method === 'PUT') { + return jsonResponse({ message: '405 Method Not Allowed: This merge request is not open' }, { status: 405 }); + } + return null; + }, + ]); + + const app = createApp(); + const response = await request(app) + .put('/api/gitlab/mrs/merge') + .send({ directory: '/tmp/work', number: 12 }); + + expect(response.status).toBe(405); + expect(response.body).toEqual({ + connected: true, + merged: false, + message: '405 Method Not Allowed: This merge request is not open', + }); + }); + + test('mrs/merge surfaces a 403 as an api-scope error', async () => { + scriptedFetch([ + (url, options) => { + if (matches(/\/merge_requests\/12\/merge$/)(url) && options.method === 'PUT') { + return jsonResponse({ message: '403 Forbidden' }, { status: 403 }); + } + return null; + }, + ]); + + const app = createApp(); + const response = await request(app) + .put('/api/gitlab/mrs/merge') + .send({ directory: '/tmp/work', number: 12 }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: 'Your GitLab token needs the api scope to create merge requests' }); + }); + test('data routes surface a 503 when GitLab rate limits', async () => { scriptedFetch([(url) => (matches(/\/issues\?/)(url) ? jsonResponse({}, { status: 429, headers: { 'retry-after': '30' } }) : null)]); diff --git a/packages/web/src/api/gitlab.test.ts b/packages/web/src/api/gitlab.test.ts index d6804c8b..206fc8cb 100644 --- a/packages/web/src/api/gitlab.test.ts +++ b/packages/web/src/api/gitlab.test.ts @@ -145,6 +145,124 @@ describe('createWebGitLabAPI', () => { }); }); + it('posts to /api/gitlab/mrs/create with the input body and returns the created MR', async () => { + const created = { + connected: true, + repo: null, + mr: { + number: 12, + title: 'Add feature', + url: 'https://gitlab.com/group/sub/-/merge_requests/12', + state: 'opened', + draft: false, + author: { username: 'octocat', id: 1 }, + sourceBranch: 'feat/add', + targetBranch: 'main', + }, + }; + runtimeFetchMock.mockResolvedValueOnce(Response.json(created)); + + const api = await createAPI(); + await expect(api.mrCreate({ + directory: '/workspace', + title: 'Add feature', + sourceBranch: 'feat/add', + targetBranch: 'main', + removeSourceBranch: true, + })).resolves.toEqual(created.mr); + + expect(runtimeFetchMock).toHaveBeenCalledWith('/api/gitlab/mrs/create', { + method: 'POST', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify({ + directory: '/workspace', + title: 'Add feature', + sourceBranch: 'feat/add', + targetBranch: 'main', + removeSourceBranch: true, + }), + }); + }); + + it('throws the server error when mrCreate fails', async () => { + runtimeFetchMock.mockResolvedValueOnce(Response.json( + { error: 'Your GitLab token needs the api scope to create merge requests' }, + { status: 400 }, + )); + + const api = await createAPI(); + await expect(api.mrCreate({ + directory: '/workspace', + title: 'Add feature', + sourceBranch: 'feat/add', + targetBranch: 'main', + })).rejects.toThrow('Your GitLab token needs the api scope to create merge requests'); + }); + + it('PUTs to /api/gitlab/mrs/update with the input body and returns the updated MR', async () => { + const updated = { + connected: true, + repo: null, + mr: { number: 12, title: 'Renamed', url: 'u', state: 'opened', draft: false, author: { username: 'octocat', id: 1 }, sourceBranch: 'feat/add', targetBranch: 'main' }, + }; + runtimeFetchMock.mockResolvedValueOnce(Response.json(updated)); + + const api = await createAPI(); + await expect(api.mrUpdate({ directory: '/workspace', number: 12, title: 'Renamed', description: 'New body' })).resolves.toEqual(updated.mr); + + expect(runtimeFetchMock).toHaveBeenCalledWith('/api/gitlab/mrs/update', { + method: 'PUT', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify({ directory: '/workspace', number: 12, title: 'Renamed', description: 'New body' }), + }); + }); + + it('returns merged:false without throwing when the server rejects a merge', async () => { + runtimeFetchMock.mockResolvedValueOnce(Response.json( + { connected: true, merged: false, message: '405 Method Not Allowed: not open' }, + { status: 405 }, + )); + + const api = await createAPI(); + await expect(api.mrMerge({ directory: '/workspace', number: 12, squash: true })).resolves.toEqual({ + connected: true, + merged: false, + message: '405 Method Not Allowed: not open', + }); + + expect(runtimeFetchMock).toHaveBeenCalledWith('/api/gitlab/mrs/merge', { + method: 'PUT', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify({ directory: '/workspace', number: 12, squash: true }), + }); + }); + + it('resolves merged:true on a successful merge', async () => { + runtimeFetchMock.mockResolvedValueOnce(Response.json({ connected: true, merged: true })); + + const api = await createAPI(); + await expect(api.mrMerge({ directory: '/workspace', number: 12 })).resolves.toEqual({ connected: true, merged: true }); + }); + + it('throws the server error when mrMerge hits a real error payload', async () => { + runtimeFetchMock.mockResolvedValueOnce(Response.json( + { error: 'Your GitLab token needs the api scope to create merge requests' }, + { status: 400 }, + )); + + const api = await createAPI(); + await expect(api.mrMerge({ directory: '/workspace', number: 12 })).rejects.toThrow( + 'Your GitLab token needs the api scope to create merge requests', + ); + }); + + it('throws the response status text when mrMerge has no parseable payload', async () => { + runtimeFetchMock.mockResolvedValueOnce(new Response('upstream gone', { status: 502, statusText: 'Bad Gateway' })); + + const api = await createAPI(); + await expect(api.mrMerge({ directory: '/workspace', number: 12 })).rejects.toThrow('Bad Gateway'); + }); + it('throws the server error message on {error} payloads', async () => { runtimeFetchMock.mockResolvedValueOnce(Response.json({ error: 'Not connected to GitLab' }, { status: 401 })); diff --git a/packages/web/src/api/gitlab.ts b/packages/web/src/api/gitlab.ts index cbd2856d..dac4726e 100644 --- a/packages/web/src/api/gitlab.ts +++ b/packages/web/src/api/gitlab.ts @@ -5,8 +5,15 @@ import type { GitLabIssueCommentsResult, GitLabIssueGetResult, GitLabIssuesListResult, + GitLabMergeRequest, GitLabMergeRequestContextResult, + GitLabMergeRequestCreateInput, + GitLabMergeRequestCreateResult, + GitLabMergeRequestMergeInput, + GitLabMergeRequestMergeResult, GitLabMergeRequestsListResult, + GitLabMergeRequestUpdateInput, + GitLabMergeRequestUpdateResult, GitLabUserSummary, } from '@openchamber/ui/lib/api/types'; import { runtimeFetch } from '@openchamber/ui/lib/runtime-fetch'; @@ -175,6 +182,56 @@ export const createWebGitLabAPI = ({ urls }: WebGitLabAPIOptions): GitLabAPI => return payload; }, + async mrCreate(input: GitLabMergeRequestCreateInput): Promise { + const response = await runtimeFetch('/api/gitlab/mrs/create', { + method: 'POST', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify(input), + }); + const payload = await jsonOrNull(response); + if (!response.ok || !payload?.mr) { + throw new Error(payload?.error || response.statusText || 'Failed to create GitLab merge request'); + } + return payload.mr; + }, + + async mrUpdate(input: GitLabMergeRequestUpdateInput): Promise { + const response = await runtimeFetch('/api/gitlab/mrs/update', { + method: 'PUT', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify(input), + }); + const payload = await jsonOrNull(response); + if (!response.ok || !payload?.mr) { + throw new Error(payload?.error || response.statusText || 'Failed to update GitLab merge request'); + } + return payload.mr; + }, + + async mrMerge(input: GitLabMergeRequestMergeInput): Promise { + const response = await runtimeFetch('/api/gitlab/mrs/merge', { + method: 'PUT', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify(input), + }); + const payload = await jsonOrNull(response); + // The server rejects non-mergeable requests with 405/409/422 and a + // `{ connected, merged: false, message }` body — parse it and return it + // instead of throwing. Only throw when there is no parseable payload + // (network failure) or the server surfaced a real `{ error }`. + if (!payload) { + throw new Error(response.statusText || 'Failed to merge GitLab merge request'); + } + if (payload.error) { + throw new Error(payload.error); + } + return { + connected: Boolean(payload.connected), + merged: Boolean(payload.merged), + ...(payload.message ? { message: payload.message } : {}), + }; + }, + async repoBranches(namespace: string, project: string): Promise { const response = await runtimeFetch( `/api/gitlab/repo/branches?namespace=${encodeURIComponent(namespace)}&project=${encodeURIComponent(project)}`,