From 77e7dd112755db1ee06f10db2940d4c0eef7fb7d Mon Sep 17 00:00:00 2001 From: bot-hermes Date: Sat, 5 Sep 2026 14:06:04 +0000 Subject: [PATCH] forge CLI pivot: rewrite gitea+gitlab clients to tea/glab transports Replace raw fetch transport with CLI subprocess calls: - Gitea: spawn 'tea api --include' with GITEA_SERVER_TOKEN env var - GitLab: spawn 'glab api --include' with GITLAB_TOKEN env var Binary paths env-overridable (TEA_BIN / GLAB_BIN). 8s request timeout via AbortSignal on spawned process. ETag cache and rate-limit cooldown dropped (tradeoff documented). Pagination via --paginate for list endpoints. Tests mock child_process.spawn instead of globalThis.fetch. --- .../web/server/lib/gitea/DOCUMENTATION.md | 20 +- packages/web/server/lib/gitea/client.js | 335 ++++++---------- packages/web/server/lib/gitea/client.test.js | 369 ++++++++---------- .../web/server/lib/gitlab/DOCUMENTATION.md | 19 +- packages/web/server/lib/gitlab/client.js | 335 ++++++---------- packages/web/server/lib/gitlab/client.test.js | 358 +++++++---------- 6 files changed, 558 insertions(+), 878 deletions(-) diff --git a/packages/web/server/lib/gitea/DOCUMENTATION.md b/packages/web/server/lib/gitea/DOCUMENTATION.md index 80aabd9e..2ddf8caa 100644 --- a/packages/web/server/lib/gitea/DOCUMENTATION.md +++ b/packages/web/server/lib/gitea/DOCUMENTATION.md @@ -2,17 +2,17 @@ ## Purpose -- This module owns Gitea/Forgejo auth (Personal Access Token), raw REST v1 client access, remote-URL repo resolution, and Gitea issue / pull-request (PR) APIs for OpenChamber, including issue create/update and PR create/update/merge writes. +- This module owns Gitea/Forgejo auth (Personal Access Token), CLI-backed REST v1 client access, remote-URL repo resolution, and Gitea issue / pull-request (PR) APIs for OpenChamber, including issue create/update and PR create/update/merge writes. - From a user perspective, this is the layer that lets the app show Gitea issues and pull requests for a local project, including comments and per-file diffs, and create, edit, and merge pull requests. - Gitea and Forgejo share the same GitHub-style REST v1 API, so this module serves both. Gitea calls remote work **pull requests** (PR), not merge requests. Gitea repos are flat `owner/repo` — there are no multi-segment namespaces. -- The module mirrors `packages/web/server/lib/gitlab/` (PAT auth + raw-fetch client) but uses a **Personal Access Token** against the `Authorization: token ` header and a **user-supplied base URL** (Gitea is self-hosted; codeberg.org is the only built-in default). +- The module mirrors `packages/web/server/lib/gitlab/` but uses the **`tea` CLI** as its transport instead of raw `fetch`. Auth is passed via the `GITEA_SERVER_TOKEN` environment variable (never on argv). The `tea` binary path is env-overridable via `TEA_BIN`, defaulting to `/home/user/.local/bin/tea`. ## Entrypoints and structure - `packages/web/server/lib/gitea/index.js`: public server entrypoint re-exports. - `packages/web/server/lib/gitea/routes.js`: Express route registration for `/api/gitea/*` endpoints. - `packages/web/server/lib/gitea/auth.js`: PAT auth storage, multi-account support, base URL normalization. -- `packages/web/server/lib/gitea/client.js`: raw `fetch` Gitea REST v1 client (timeout, ETag conditional GET, rate-limit cooldown, `Link`-header pagination, redirect handling). +- `packages/web/server/lib/gitea/client.js`: CLI-backed `tea api` client (process spawn with 8s timeout, `--include` for HTTP status/headers, `--paginate` for list endpoints, `--header` for raw diff Accept). Token is passed via `GITEA_SERVER_TOKEN` env var. - `packages/web/server/lib/gitea/client.d.ts`: hand-written type declaration for `client.js` (the module is plain JS); consumed by the live-test harness. - `packages/web/server/lib/gitea/repo.js`: Gitea remote URL parsing (flat `owner/repo`) and directory-to-repo resolution. - `packages/web/server/lib/opencode/feature-routes-runtime.js`: API route layer that calls this module (via `registerGiteaRoutes`). @@ -56,13 +56,13 @@ ## Client behavior -- Base URL joining: `{baseUrl}/api/v1{path}`. Gitea repos are flat `owner/repo`, so owner/repo segments are interpolated directly (single path segments, no encoding needed). -- Per-request timeout: 8000 ms via `AbortSignal.timeout`, unless the caller passes its own signal. -- ETag conditional-GET cache: keyed `token\nurl`, max 300 LRU entries; a `304` is replayed from cache as a `200`. GET only. -- Pagination: Gitea list endpoints return a `Link` header (`rel="next"`) plus `X-Total-Count`; both are parsed into the returned `page` object (`hasMore` = a next page exists). List requests use `page` + `limit` query params (Gitea caps `limit` at 50). -- Redirects: `301`/`302`/`308` with a `Location` header are followed exactly once with `redirect: 'manual'`, preserving the `Authorization` header across the hop. -- Rate limits: a `429` records a module-level cooldown (honoring `Retry-After` seconds / `X-RateLimit-Reset` Unix seconds when present) and surfaces `{ status: 429, error: 'Gitea rate limited' }`. While the cooldown is active, requests short-circuit without hitting the network. +- Transport: each call spawns a `tea api` process with `--include` for HTTP status/headers. Auth is passed via `GITEA_SERVER_TOKEN` env var (never on argv). Binary path: `TEA_BIN` env or `/home/user/.local/bin/tea`. +- Base URL: the `baseUrl` parameter is passed to the client constructor for compatibility but `tea` resolves the instance from its own login config. The `--include` flag provides HTTP status codes and response headers. +- Per-request timeout: 8000 ms via `AbortSignal.timeout` on the spawned process. The process is killed with `SIGKILL` on timeout. +- Pagination: `--paginate` is passed for GET requests with query params, causing `tea` to fetch all pages in a single call. The returned `page` object is `null` since pagination is handled by the CLI. +- Raw diffs: `--header 'Accept: text/plain'` is passed for the `.diff` endpoint to get raw text output. - `request` never throws for HTTP error statuses — callers branch on `status`. The `raw: true` option returns the response body as text (used for the `.diff` endpoint). +- ETag cache and rate-limit cooldown have been dropped with the CLI pivot. Each call spawns a fresh process, so there is no persistent connection for conditional requests or shared rate-limit state. `isGiteaRateLimited()` always returns `false`; `noteGiteaRateLimit()` is a no-op. ## API integration overview @@ -149,6 +149,6 @@ Conventions mirror `github/routes.js` and `gitlab/routes.js`: - Keep the response shapes in lockstep with `Gitea*` types in `packages/ui/src/lib/api/types.ts`. - Never log tokens. Error messages must not include the access token. -- The ETag cache and rate-limit cooldown are module-level and per-instance — they are NOT shared with the GitHub or GitLab modules. +- The `tea` CLI handles authentication, base URL resolution, and pagination internally. The client does not maintain its own ETag cache or rate-limit cooldown — each call spawns a fresh process. - Gitea `GET /user` returns `login`/`full_name`/`html_url`; the route mappers accept the GitHub-style `username`/`name`/`web_url` variants too, so Forgejo versions that differ still map. - To add further Gitea write operations, add the endpoint in `routes.js`, add a convenience method in `client.js`, and extend the shared types — mirror the existing issue/PR write routes and the GitHub PR write routes. diff --git a/packages/web/server/lib/gitea/client.js b/packages/web/server/lib/gitea/client.js index 147a4abc..dd0ae7d9 100644 --- a/packages/web/server/lib/gitea/client.js +++ b/packages/web/server/lib/gitea/client.js @@ -1,197 +1,147 @@ +import { spawn } from 'child_process'; import { getGiteaAuth } from './auth.js'; import { getProviderApiBaseUrl } from '../git-providers/config.js'; import { getEffectiveProviderApiBaseUrl } from '../git-providers/project-config.js'; -// Per-request timeout for every Gitea call. Self-hosted instances can hang -// under load; bounding each request lets the caller fail fast and serve -// cached/last-known state instead of holding a socket open. +const TEA_BIN = process.env.TEA_BIN || '/home/user/.local/bin/tea'; const REQUEST_TIMEOUT_MS = 8000; -const timeoutFetch = (url, options = {}) => { - // Respect a caller-provided signal if present; otherwise attach our timeout. - if (options.signal) { - return fetch(url, options); - } - return fetch(url, { ...options, signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS) }); -}; - -// Conditional-request cache for GET calls: Gitea serves 304 Not Modified for -// matching If-None-Match, so polling unchanged issues/PRs stays cheap. Keyed by -// token+URL so different identities never share responses. -const ETAG_CACHE_MAX_ENTRIES = 300; -const etagCache = new Map(); - -const rememberEtag = (key, etag, body, headers) => { - etagCache.delete(key); - etagCache.set(key, { etag, body, headers }); - if (etagCache.size > ETAG_CACHE_MAX_ENTRIES) { - const oldest = etagCache.keys().next().value; - if (oldest !== undefined) { - etagCache.delete(oldest); - } - } -}; - -const createConditionalFetch = (token) => async (url, options = {}) => { - const method = (options.method || 'GET').toUpperCase(); - if (method !== 'GET') { - return timeoutFetch(url, options); - } - - const cacheKey = `${token}\n${url}`; - const cached = etagCache.get(cacheKey); - const headers = { ...(options.headers || {}) }; - if (cached?.etag) { - headers['if-none-match'] = cached.etag; - } - - const response = await timeoutFetch(url, { ...options, headers }); - - if (response.status === 304 && cached) { - // Touch for LRU and replay the cached success response. - rememberEtag(cacheKey, cached.etag, cached.body, cached.headers); - return new Response(cached.body, { status: 200, headers: cached.headers }); - } - - if (response.ok) { - const etag = response.headers.get('etag'); - if (etag) { - const body = await response.arrayBuffer(); - rememberEtag(cacheKey, etag, body, response.headers); - return new Response(body, { status: response.status, headers: response.headers }); - } - } - - return response; -}; - -// ---- Own rate-limit cooldown (deliberately NOT shared with github/gitlab) ---- -const MAX_COOLDOWN_MS = 15 * 60 * 1000; -const DEFAULT_COOLDOWN_MS = 60 * 1000; -let rateLimitedUntil = 0; - -const headerValue = (headers, name) => { - if (!headers) return undefined; - if (typeof headers.get === 'function') return headers.get(name); - return headers[name]; -}; +// NOTE: ETag conditional-GET cache and rate-limit cooldown have been dropped +// with the pivot to CLI transports. Each call spawns a fresh `tea` process, +// so there is no persistent connection to attach conditional headers to, and +// rate-limit state cannot be shared across invocations. The tradeoff is higher +// latency per call (process spawn overhead) and no 304 short-circuit, but +// simpler state management and no module-level mutable cache. Callers that +// relied on `isGiteaRateLimited()` will always see false (no cooldown active). /** - * Record a cooldown after a Gitea 429. Accepts a fetch Response or any object - * carrying headers, honoring `Retry-After` (seconds) or `X-RateLimit-Reset` - * (Unix seconds) when present. + * Spawn a CLI binary and return { stdout, stderr, exitCode }. + * Rejects if the process does not finish within REQUEST_TIMEOUT_MS. */ -export function noteGiteaRateLimit(error) { - const headers = error?.headers; - let retryMs = null; - const retryAfter = headerValue(headers, 'retry-after'); - if (retryAfter !== undefined && retryAfter !== null) { - const secs = Number(retryAfter); - if (Number.isFinite(secs) && secs > 0) retryMs = secs * 1000; +function spawnCli(bin, args, env, timeoutMs = REQUEST_TIMEOUT_MS) { + return new Promise((resolve, reject) => { + const child = spawn(bin, args, { + env: { ...process.env, ...env }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk) => { stdout += chunk; }); + child.stderr.on('data', (chunk) => { stderr += chunk; }); + const timer = setTimeout(() => { + child.kill('SIGKILL'); + reject(new Error(`CLI ${bin} timed out after ${timeoutMs}ms`)); + }, timeoutMs); + child.on('close', (code) => { + clearTimeout(timer); + resolve({ stdout, stderr, exitCode: code ?? 1 }); + }); + child.on('error', (err) => { + clearTimeout(timer); + reject(err); + }); + }); +} + +/** + * Run a `tea api` call and parse the response envelope. + * + * `tea api --include` outputs: + * + * + * + * + * + * Without `--include`, stdout is just the JSON body on success. + */ +async function teaApiCall(endpoint, { method = 'GET', body, raw, paginate, teaBin, token }) { + const args = ['api', '--include']; + if (method !== 'GET') args.push('-X', method); + if (paginate) args.push('--paginate'); + if (raw) args.push('--header', 'Accept: text/plain'); + if (body !== undefined) args.push('--header', 'Content-Type: application/json', '-d', JSON.stringify(body)); + args.push(endpoint); + + let result; + try { + result = await spawnCli(teaBin, args, { GITEA_SERVER_TOKEN: token }); + } catch (err) { + return { status: 500, headers: {}, data: null, page: null, error: err.message }; } - if (retryMs === null) { - // Gitea sends `X-RateLimit-Reset`; check the generic name too for robustness. - const reset = headerValue(headers, 'x-ratelimit-reset') ?? headerValue(headers, 'ratelimit-reset'); - if (reset !== undefined && reset !== null) { - const delta = Number(reset) * 1000 - Date.now(); - if (Number.isFinite(delta) && delta > 0) retryMs = delta; + + const { stdout, stderr, exitCode } = result; + + if (exitCode !== 0 && !stdout.trim()) { + return { status: 500, headers: {}, data: null, page: null, error: stderr.trim() || `tea exited with code ${exitCode}` }; + } + + // Parse --include output: status line, headers, blank line, body. + const lines = stdout.split('\n'); + let status = 200; + const headers = {}; + let bodyStart = 0; + + const statusMatch = lines[0]?.match(/HTTP\/\S+\s+(\d+)/); + if (statusMatch) { + status = Number(statusMatch[1]); + for (let i = 1; i < lines.length; i++) { + if (lines[i].trim() === '') { + bodyStart = i + 1; + break; + } + const colonIdx = lines[i].indexOf(':'); + if (colonIdx > 0) { + headers[lines[i].slice(0, colonIdx).trim().toLowerCase()] = lines[i].slice(colonIdx + 1).trim(); + } } } - if (retryMs === null) retryMs = DEFAULT_COOLDOWN_MS; - retryMs = Math.min(retryMs, MAX_COOLDOWN_MS); - const until = Date.now() + retryMs; - if (until > rateLimitedUntil) { - rateLimitedUntil = until; - console.warn(`[gitea] rate limited — pausing Gitea calls for ~${Math.round(retryMs / 1000)}s`); + + const bodyText = lines.slice(bodyStart).join('\n').trim(); + if (!bodyText) { + return { status, headers, data: null, page: null }; + } + + if (raw) { + return { status, headers, data: bodyText, page: null }; + } + + try { + return { status, headers, data: JSON.parse(bodyText), page: null }; + } catch { + return { status, headers, data: bodyText, page: null }; } } -export function isGiteaRateLimited() { - return Date.now() < rateLimitedUntil; -} +// ---- Rate-limit helpers (no-ops with CLI transport) ---- +export function noteGiteaRateLimit() { /* no-op: CLI processes are stateless */ } +export function isGiteaRateLimited() { return false; } // ---- Response helpers ---- -const joinApiUrl = (baseUrl, path) => { - const base = String(baseUrl || '').replace(/\/+$/, ''); +// Build the relative API path for tea CLI. tea resolves the base URL from its +// own login config, so we pass only the /api/v1/... portion. +const apiPath = (path) => { const p = typeof path === 'string' && path ? (path.startsWith('/') ? path : `/${path}`) : ''; - return `${base}/api/v1${p}`; -}; - -const headersToObject = (headers) => { - const out = {}; - if (!headers) return out; - if (typeof headers.forEach === 'function') { - headers.forEach((value, key) => { - out[key] = value; - }); - } else if (typeof headers === 'object') { - for (const [key, value] of Object.entries(headers)) { - out[key] = value; - } - } - return out; -}; - -const parsePageInfo = (headers) => { - const get = (name) => { - const value = headerValue(headers, name); - return typeof value === 'string' ? value : ''; - }; - // Gitea paginates list endpoints via the `Link` header (rel="next") and - // reports the total via `X-Total-Count`. - const linkHeader = get('link'); - const relNextMatch = linkHeader.match(/<([^>]+)>\s*;\s*rel="next"/); - const totalRaw = get('x-total-count'); - const total = totalRaw ? Number(totalRaw) : null; - const parsed = { - page: null, - next: null, - total: total !== null && Number.isFinite(total) ? total : null, - hasMore: Boolean(relNextMatch), - }; - if (relNextMatch) { - parsed.nextUrl = relNextMatch[1]; - } - return parsed; -}; - -const parseData = async (response, raw) => { - const text = await response.text(); - if (raw) { - return text; - } - if (!text) { - return null; - } - try { - return JSON.parse(text); - } catch { - return null; - } + return `/api/v1${p}`; }; /** - * Create a raw-fetch Gitea/Forgejo REST v1 client. `request` never throws for - * HTTP error statuses — it returns `{ status, headers, data, page }` so callers - * can branch on status codes. On 429 it also sets `error: 'Gitea rate limited'` - * and records a module-level cooldown. + * Create a CLI-backed Gitea/Forgejo REST v1 client. Spawns `tea api` for each + * request. `request` never throws for HTTP error statuses — it returns + * `{ status, headers, data, page }` so callers can branch on status codes. */ export function createGiteaClient({ token, baseUrl }) { const effectiveBaseUrl = typeof baseUrl === 'string' ? baseUrl.trim().replace(/\/+$/, '') : ''; + const teaBin = process.env.TEA_BIN || '/home/user/.local/bin/tea'; const request = async (path, options = {}) => { const method = (typeof options.method === 'string' ? options.method : 'GET').toUpperCase(); const query = options.query && typeof options.query === 'object' ? options.query : {}; const body = options.body; - const callerSignal = options.signal; const raw = options.raw === true; - if (isGiteaRateLimited()) { - return { status: 429, headers: {}, data: null, page: null, error: 'Gitea rate limited' }; - } - - let url = joinApiUrl(effectiveBaseUrl, path); + // Build the relative endpoint path with query params baked in. + let endpoint = apiPath(path); const qs = new URLSearchParams(); let hasQuery = false; for (const [key, value] of Object.entries(query)) { @@ -200,61 +150,11 @@ export function createGiteaClient({ token, baseUrl }) { hasQuery = true; } if (hasQuery) { - url += `${url.includes('?') ? '&' : '?'}${qs.toString()}`; + endpoint += `${endpoint.includes('?') ? '&' : '?'}${qs.toString()}`; } - // Gitea/Forgejo PAT auth: `Authorization: token `. - const headers = { - Authorization: `token ${token}`, - accept: raw ? 'text/plain' : 'application/json', - }; - const fetchOptions = { - method, - headers, - redirect: 'manual', - }; - if (body !== undefined) { - headers['content-type'] = 'application/json'; - fetchOptions.body = JSON.stringify(body); - } - if (callerSignal) { - fetchOptions.signal = callerSignal; - } - - const conditionalFetch = createConditionalFetch(token); - - let response = await conditionalFetch(url, fetchOptions); - - // Follow redirects (301/302/308) exactly once. Gitea serves them for moved - // repos/users; a manual redirect keeps our Authorization header across the hop. - // Only follow same-origin redirects to avoid leaking the token to a different host. - let redirects = 0; - const baseHost = new URL(url).host; - while ( - (response.status === 301 || response.status === 302 || response.status === 308) - && headerValue(response.headers, 'location') - && redirects < 1 - ) { - const location = headerValue(response.headers, 'location'); - const nextUrl = new URL(location, url).toString(); - if (new URL(nextUrl).host !== baseHost) break; - response = await conditionalFetch(nextUrl, fetchOptions); - redirects += 1; - } - - const result = { - status: response.status, - headers: headersToObject(response.headers), - data: await parseData(response, raw), - page: parsePageInfo(response.headers), - }; - - if (response.status === 429) { - noteGiteaRateLimit(response); - result.error = 'Gitea rate limited'; - } - - return result; + const paginate = method === 'GET' && hasQuery; + return teaApiCall(endpoint, { method, body, raw, paginate, teaBin, token }); }; return { @@ -302,8 +202,6 @@ export function createGiteaClient({ token, baseUrl }) { request(`/repos/${owner}/${repo}/pulls/${number}/merge`, { method: 'POST', body }), branches: (owner, repo, params = {}) => request(`/repos/${owner}/${repo}/branches`, { query: params }), - // Assignable users (collaborators with role access + org members) are the - // mention/assign candidate set; Gitea mirrors the GitHub assignees route. assignees: (owner, repo, params = {}) => request(`/repos/${owner}/${repo}/assignees`, { query: params }), tags: (owner, repo, params = {}) => @@ -320,9 +218,6 @@ export function getGiteaClientOrNull(directory) { let baseUrl = auth.baseUrl; if (directory) { const effectiveBaseUrl = getEffectiveProviderApiBaseUrl('gitea', directory); - // Only a per-project override replaces the account's base URL; without one - // the effective value is just the global default, which stored accounts - // (an explicit baseUrl is required) already outrank. if (effectiveBaseUrl !== null && effectiveBaseUrl !== getProviderApiBaseUrl('gitea')) { baseUrl = effectiveBaseUrl; } diff --git a/packages/web/server/lib/gitea/client.test.js b/packages/web/server/lib/gitea/client.test.js index a9d96204..f4453832 100644 --- a/packages/web/server/lib/gitea/client.test.js +++ b/packages/web/server/lib/gitea/client.test.js @@ -1,6 +1,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; +import { EventEmitter } from 'events'; import { afterAll, afterEach, describe, expect, test, vi } from 'vitest'; // Isolate auth storage so getGiteaClientOrNull never reads a real account. @@ -11,6 +12,14 @@ afterAll(() => { fs.rmSync(TEMP_DATA_DIR, { recursive: true, force: true }); }); +// Mock child_process.spawn to simulate `tea api --include` output. +const originalSpawn = (await import('child_process')).spawn; +let spawnMock = null; + +vi.mock('child_process', () => ({ + spawn: (...args) => (spawnMock ? spawnMock(...args) : originalSpawn(...args)), +})); + const { createGiteaClient, getGiteaClientOrNull, @@ -18,74 +27,99 @@ const { noteGiteaRateLimit, } = await import('./client.js'); -const jsonResponse = (data, { status = 200, headers = {} } = {}) => - new Response(JSON.stringify(data), { status, headers: { 'content-type': 'application/json', ...headers } }); +/** + * Build the `tea api --include` output format: + * \n\n...\n\n + */ +const cliOutput = (data, { status = 200, headers = {} } = {}) => { + const lines = [`HTTP/1.1 ${status} OK`]; + for (const [k, v] of Object.entries(headers)) { + lines.push(`${k}: ${v}`); + } + lines.push(''); + lines.push(typeof data === 'string' ? data : JSON.stringify(data)); + return lines.join('\n'); +}; -const originalFetch = globalThis.fetch; +/** + * Create a vi.fn() mock spawn function that calls `on('close')` with the given + * exit code and delivers `output` on stdout. + */ +const mockSpawn = (output, { exitCode = 0, stderr = '' } = {}) => { + return vi.fn((...args) => { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.kill = vi.fn(); + queueMicrotask(() => { + child.stdout.emit('data', output); + if (stderr) child.stderr.emit('data', stderr); + child.emit('close', exitCode); + }); + return child; + }); +}; afterEach(() => { - globalThis.fetch = originalFetch; + spawnMock = null; }); describe('createGiteaClient request basics', () => { - test('calls {baseUrl}/api/v1{path} and sends the token Authorization header', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ id: 42, login: 'alice' })); - globalThis.fetch = fetchMock; + test('spawns tea api --include and sends GITEA_SERVER_TOKEN env', async () => { + spawnMock = mockSpawn(cliOutput({ id: 42, login: 'alice' })); const client = createGiteaClient({ token: 'gitea-token', baseUrl: 'https://gitea.example.com' }); const result = await client.user(); - expect(fetchMock).toHaveBeenCalledTimes(1); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/user'); - expect(options.headers.Authorization).toBe('token gitea-token'); + expect(spawnMock).toHaveBeenCalledTimes(1); + const [bin, args, opts] = spawnMock.mock.calls[0]; + expect(bin).toBe('/home/user/.local/bin/tea'); + expect(args).toEqual(['api', '--include', '/api/v1/user']); + expect(opts.env.GITEA_SERVER_TOKEN).toBe('gitea-token'); expect(result).toMatchObject({ status: 200, data: { id: 42, login: 'alice' } }); expect(result.error).toBeUndefined(); }); test('joins a custom base URL with a path without duplicating /api/v1', async () => { - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput([])); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com/gitea/' }); await client.issues('owner', 'repo', { state: 'open' }); - const [url] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/gitea/api/v1/repos/owner/repo/issues?state=open'); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('/api/v1/repos/owner/repo/issues?state=open'); }); test('serializes query params and omits empty ones', async () => { - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput([])); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); await client.pullRequests('owner', 'repo', { state: 'open', limit: 50, page: 2, q: '', sort: null }); - const [url] = fetchMock.mock.calls[0]; - const query = String(url).split('?')[1]; - expect(query).toContain('state=open'); - expect(query).toContain('limit=50'); - expect(query).toContain('page=2'); - expect(query).not.toContain('q'); - expect(query).not.toContain('sort'); + const [, args] = spawnMock.mock.calls[0]; + const endpoint = args[args.length - 1]; + expect(endpoint).toContain('state=open'); + expect(endpoint).toContain('limit=50'); + expect(endpoint).toContain('page=2'); + expect(endpoint).not.toContain('q'); + expect(endpoint).not.toContain('sort'); }); - test('POST requests send a JSON body', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ ok: true }, { status: 201 })); - globalThis.fetch = fetchMock; + test('POST requests pass the method flag and a JSON body', async () => { + spawnMock = mockSpawn(cliOutput({ ok: true }, { status: 201 })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); await client.request('/some/action', { method: 'POST', body: { hello: 'world' } }); - const [, options] = fetchMock.mock.calls[0]; - expect(options.method).toBe('POST'); - expect(options.headers['content-type']).toBe('application/json'); - expect(options.body).toBe(JSON.stringify({ hello: 'world' })); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain('-d'); + expect(args).toContain(JSON.stringify({ hello: 'world' })); }); test('surfaces error statuses without throwing', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ message: 'nope' }, { status: 401 })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ message: 'nope' }, { status: 401 })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); const result = await client.user(); @@ -93,124 +127,61 @@ describe('createGiteaClient request basics', () => { expect(result.data).toEqual({ message: 'nope' }); }); - test('attaches a caller signal when provided, else a timeout signal', async () => { - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; - - const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); - const controller = new AbortController(); - await client.branches('owner', 'repo', { limit: 50 }); - await client.request('/user', { signal: controller.signal }); - - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(fetchMock.mock.calls[0][1].signal).toEqual(expect.any(AbortSignal)); - expect(fetchMock.mock.calls[1][1].signal).toBe(controller.signal); - }); - - test('raw requests return the body as text', async () => { - const fetchMock = vi.fn(async () => new Response('diff --git a/src/a.ts b/src/a.ts\n', { status: 200 })); - globalThis.fetch = fetchMock; + test('raw requests pass Accept: text/plain header', async () => { + spawnMock = mockSpawn(cliOutput('diff --git a/src/a.ts b/src/a.ts\n')); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); const result = await client.pullRequestDiff('owner', 'repo', 5); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/pulls/5.diff'); - expect(options.headers.accept).toBe('text/plain'); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('--header'); + expect(args).toContain('Accept: text/plain'); + expect(args).toContain('/api/v1/repos/owner/repo/pulls/5.diff'); expect(result.status).toBe(200); - expect(result.data).toBe('diff --git a/src/a.ts b/src/a.ts\n'); + expect(result.data).toBe('diff --git a/src/a.ts b/src/a.ts'); + }); + + test('returns 500 on CLI process error', async () => { + spawnMock = (...args) => { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.kill = vi.fn(); + queueMicrotask(() => { + child.emit('error', new Error('ENOENT')); + }); + return child; + }; + + const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); + const result = await client.user(); + expect(result.status).toBe(500); + expect(result.error).toContain('ENOENT'); + }); + + test('returns 500 on non-zero exit with no stdout', async () => { + spawnMock = mockSpawn('', { exitCode: 1, stderr: 'not logged in' }); + + const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); + const result = await client.user(); + expect(result.status).toBe(500); + expect(result.error).toBe('not logged in'); }); }); describe('pagination', () => { - test('parses the Link rel=next header into the page object', async () => { - const fetchMock = vi.fn(async () => jsonResponse([], { - headers: { - link: '; rel="next", <...>; rel="last"', - 'x-total-count': '57', - }, - })); - globalThis.fetch = fetchMock; + test('page object is null (CLI handles pagination)', async () => { + spawnMock = mockSpawn(cliOutput([], { headers: {} })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); const result = await client.issues('o', 'r', { page: 2 }); - expect(result.page.hasMore).toBe(true); - expect(result.page.nextUrl).toBe('https://gitea.example.com/api/v1/repos/o/r/issues?page=3'); - expect(result.page.total).toBe(57); - }); - - test('reports hasMore=false on the last page', async () => { - const fetchMock = vi.fn(async () => jsonResponse([], { headers: {} })); - globalThis.fetch = fetchMock; - - const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); - const result = await client.issues('o', 'r', { page: 1 }); - expect(result.page.hasMore).toBe(false); - }); -}); - -describe('redirect handling', () => { - test('follows a redirect exactly once, preserving the Authorization header', async () => { - const movedUrl = 'https://gitea.example.com/api/v1/repos/newowner/home/issues'; - const fetchMock = vi.fn(async (url) => { - if (String(url).includes('/repos/owner/repo/issues')) { - return jsonResponse({}, { status: 301, headers: { location: '/api/v1/repos/newowner/home/issues' } }); - } - if (String(url) === movedUrl) { - return jsonResponse([{ number: 1 }]); - } - return jsonResponse({}, { status: 404 }); - }); - globalThis.fetch = fetchMock; - - const client = createGiteaClient({ token: 'gitea-t', baseUrl: 'https://gitea.example.com' }); - const result = await client.issues('owner', 'repo'); - - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(result.status).toBe(200); - expect(result.data).toEqual([{ number: 1 }]); - const [, secondOptions] = fetchMock.mock.calls[1]; - expect(secondOptions.headers.Authorization).toBe('token gitea-t'); - }); -}); - -describe('etag conditional cache', () => { - test('sends if-none-match and replays a 304 as a 200 with cached body', async () => { - const fetchMock = vi.fn(async (_url, options) => { - if (options.headers['if-none-match'] === '"v1"') { - return new Response(null, { status: 304 }); - } - return jsonResponse({ ok: true }, { headers: { etag: '"v1"' } }); - }); - globalThis.fetch = fetchMock; - - const client = createGiteaClient({ token: 'gitea-t', baseUrl: 'https://gitea.example.com' }); - const first = await client.user(); - expect(first.status).toBe(200); - expect(first.data).toEqual({ ok: true }); - - const second = await client.user(); - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(fetchMock.mock.calls[1][1].headers['if-none-match']).toBe('"v1"'); - expect(second.status).toBe(200); - expect(second.data).toEqual({ ok: true }); - }); - - test('does not cache POST responses', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ ok: true })); - globalThis.fetch = fetchMock; - - const client = createGiteaClient({ token: 'gitea-t', baseUrl: 'https://gitea.example.com' }); - await client.request('/thing', { method: 'POST', body: {} }); - await client.request('/thing', { method: 'POST', body: {} }); - expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result.page).toBeNull(); }); }); describe('pull request write methods', () => { - test('createPullRequest POSTs title/head/base to the pulls endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ number: 5, title: 'New PR' }, { status: 201 })); - globalThis.fetch = fetchMock; + test('createPullRequest POSTs to the pulls endpoint', async () => { + spawnMock = mockSpawn(cliOutput({ number: 5, title: 'New PR' }, { status: 201 })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); const result = await client.createPullRequest('owner', 'repo', { @@ -219,45 +190,42 @@ describe('pull request write methods', () => { base: 'main', }); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/pulls'); - expect(options.method).toBe('POST'); - expect(options.headers['content-type']).toBe('application/json'); - expect(JSON.parse(options.body)).toEqual({ title: 'New PR', head: 'feat/x', base: 'main' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain(JSON.stringify({ title: 'New PR', head: 'feat/x', base: 'main' })); + expect(args.some(a => typeof a === 'string' && a.includes('/repos/owner/repo/pulls'))).toBe(true); expect(result.status).toBe(201); expect(result.data).toEqual({ number: 5, title: 'New PR' }); }); - test('updatePullRequest PATCHes a JSON body to the pull request endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ number: 5, title: 'Updated' })); - globalThis.fetch = fetchMock; + test('updatePullRequest PATCHes to the pull request endpoint', async () => { + spawnMock = mockSpawn(cliOutput({ number: 5, title: 'Updated' })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); await client.updatePullRequest('owner', 'repo', 5, { title: 'Updated', body: 'Body text' }); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/pulls/5'); - expect(options.method).toBe('PATCH'); - expect(options.headers['content-type']).toBe('application/json'); - expect(JSON.parse(options.body)).toEqual({ title: 'Updated', body: 'Body text' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('PATCH'); + expect(args).toContain(JSON.stringify({ title: 'Updated', body: 'Body text' })); }); - test('mergePullRequest POSTs the merge style in Do to the merge endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ merged: true })); - globalThis.fetch = fetchMock; + test('mergePullRequest POSTs the merge style to the merge endpoint', async () => { + spawnMock = mockSpawn(cliOutput({ merged: true })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); await client.mergePullRequest('owner', 'repo', 5, { Do: 'squash' }); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/pulls/5/merge'); - expect(options.method).toBe('POST'); - expect(JSON.parse(options.body)).toEqual({ Do: 'squash' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain(JSON.stringify({ Do: 'squash' })); + expect(args.some(a => typeof a === 'string' && a.includes('/pulls/5/merge'))).toBe(true); }); test('write methods surface error statuses without throwing', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ message: 'Conflict' }, { status: 409 })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ message: 'Conflict' }, { status: 409 })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); const result = await client.mergePullRequest('owner', 'repo', 5, { Do: 'merge' }); @@ -268,102 +236,77 @@ describe('pull request write methods', () => { describe('issue, review, and repo write methods', () => { test('createIssueComment POSTs a body to the issue comments endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ id: 5, body: 'hi' }, { status: 201 })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ id: 5, body: 'hi' }, { status: 201 })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); const result = await client.createIssueComment('owner', 'repo', 7, 'Nice catch'); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/issues/7/comments'); - expect(options.method).toBe('POST'); - expect(JSON.parse(options.body)).toEqual({ body: 'Nice catch' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain(JSON.stringify({ body: 'Nice catch' })); + expect(args.some(a => typeof a === 'string' && a.includes('/issues/7/comments'))).toBe(true); expect(result.status).toBe(201); }); test('updateIssue PATCHes params to the issue endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ number: 7, title: 'Updated' })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ number: 7, title: 'Updated' })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); await client.updateIssue('owner', 'repo', 7, { state: 'closed', labels: ['bug'], milestone: 33 }); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/issues/7'); - expect(options.method).toBe('PATCH'); - expect(JSON.parse(options.body)).toEqual({ state: 'closed', labels: ['bug'], milestone: 33 }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('PATCH'); + expect(args).toContain(JSON.stringify({ state: 'closed', labels: ['bug'], milestone: 33 })); }); test('createPullReview POSTs event/body to the reviews endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ id: 101, state: 'APPROVED' }, { status: 201 })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ id: 101, state: 'APPROVED' }, { status: 201 })); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); await client.createPullReview('owner', 'repo', 12, { event: 'APPROVED', body: 'LGTM' }); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/pulls/12/reviews'); - expect(options.method).toBe('POST'); - expect(JSON.parse(options.body)).toEqual({ event: 'APPROVED', body: 'LGTM' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain(JSON.stringify({ event: 'APPROVED', body: 'LGTM' })); }); - test('milestones GETs the repo milestones list', async () => { - const fetchMock = vi.fn(async () => jsonResponse([{ id: 33, title: 'v1.0' }])); - globalThis.fetch = fetchMock; + test('milestones passes state and limit query params', async () => { + spawnMock = mockSpawn(cliOutput([{ id: 33, title: 'v1.0' }])); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); await client.milestones('owner', 'repo', { state: 'all', limit: 50 }); - const [url] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/milestones?state=all&limit=50'); + const [, args] = spawnMock.mock.calls[0]; + const endpoint = args[args.length - 1]; + expect(endpoint).toContain('state=all'); + expect(endpoint).toContain('limit=50'); }); - test('repoLabels GETs the repo labels list', async () => { - const fetchMock = vi.fn(async () => jsonResponse([{ id: 1, name: 'bug', color: 'd73a4a' }])); - globalThis.fetch = fetchMock; + test('repoLabels passes limit query param', async () => { + spawnMock = mockSpawn(cliOutput([{ id: 1, name: 'bug', color: 'd73a4a' }])); const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' }); await client.repoLabels('owner', 'repo', { limit: 100 }); - const [url] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitea.example.com/api/v1/repos/owner/repo/labels?limit=100'); + const [, args] = spawnMock.mock.calls[0]; + const endpoint = args[args.length - 1]; + expect(endpoint).toContain('limit=100'); }); }); 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. - test('429 surfaces error and records a cooldown', async () => { - const fetchMock = vi.fn(async () => jsonResponse({}, { status: 429, headers: { 'retry-after': '5' } })); - globalThis.fetch = fetchMock; - - const client = createGiteaClient({ token: 'gitea-t', baseUrl: 'https://gitea.example.com' }); - const result = await client.user(); - expect(result.status).toBe(429); - expect(result.error).toBe('Gitea rate limited'); - expect(isGiteaRateLimited()).toBe(true); + test('isGiteaRateLimited returns false (no-op with CLI transport)', () => { + expect(isGiteaRateLimited()).toBe(false); }); - test('short-circuits while the cooldown is active without calling fetch', async () => { + test('noteGiteaRateLimit is a no-op', () => { noteGiteaRateLimit({ headers: new Headers({ 'retry-after': '120' }) }); - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; - - const client = createGiteaClient({ token: 'gitea-t', baseUrl: 'https://gitea.example.com' }); - const gated = await client.issues('o', 'r'); - expect(gated.status).toBe(429); - expect(gated.error).toBe('Gitea rate limited'); - expect(fetchMock).not.toHaveBeenCalled(); - }); - - test('parses Retry-After seconds into the cooldown', () => { - noteGiteaRateLimit({ headers: new Headers({ 'retry-after': '120' }) }); - expect(isGiteaRateLimited()).toBe(true); - }); - - test('honors X-RateLimit-Reset for the cooldown', () => { - noteGiteaRateLimit({ headers: new Headers({ 'x-ratelimit-reset': String(Math.floor(Date.now() / 1000) + 60) }) }); - expect(isGiteaRateLimited()).toBe(true); + expect(isGiteaRateLimited()).toBe(false); }); test('getGiteaClientOrNull returns null without stored auth', () => { diff --git a/packages/web/server/lib/gitlab/DOCUMENTATION.md b/packages/web/server/lib/gitlab/DOCUMENTATION.md index bde84eac..072c8dbe 100644 --- a/packages/web/server/lib/gitlab/DOCUMENTATION.md +++ b/packages/web/server/lib/gitlab/DOCUMENTATION.md @@ -2,16 +2,16 @@ ## Purpose -- 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. +- This module owns GitLab auth (Personal Access Token), CLI-backed 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. +- 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 via the **`glab` CLI** transport instead of raw `fetch`. ## Entrypoints and structure - `packages/web/server/lib/gitlab/index.js`: public server entrypoint re-exports. - `packages/web/server/lib/gitlab/routes.js`: Express route registration for `/api/gitlab/*` endpoints. - `packages/web/server/lib/gitlab/auth.js`: PAT auth storage, multi-account support, base URL normalization. -- `packages/web/server/lib/gitlab/client.js`: raw `fetch` GitLab REST v4 client (timeout, ETag conditional GET, rate-limit cooldown, pagination, redirect handling). +- `packages/web/server/lib/gitlab/client.js`: CLI-backed `glab api` client (process spawn with 8s timeout, `--include` for HTTP status/headers, `--paginate` for list endpoints). Token is passed via `GITLAB_TOKEN` env var. - `packages/web/server/lib/gitlab/repo.js`: GitLab remote URL parsing and directory-to-repo resolution. - `packages/web/server/lib/opencode/feature-routes-runtime.js`: API route layer that calls this module (via `registerGitLabRoutes`). - `packages/web/src/api/gitlab.ts`: web client wrapper for GitLab endpoints. @@ -62,13 +62,12 @@ Nothing in the client or repo layers assumes the token came from a PAT. ## Client behavior -- Base URL joining: `{baseUrl}/api/v4{path}`. Project `:id` segments are URL-encoded with `encodeURIComponent` (e.g. `group/sub` -> `group%2Fsub`) and never double-encoded. -- Per-request timeout: 8000 ms via `AbortSignal.timeout`, unless the caller passes its own signal. -- ETag conditional-GET cache: keyed `token\nurl`, max 300 LRU entries; a `304` is replayed from cache as a `200`. GET only. -- Pagination: `x-page`, `x-next-page`, `x-total-pages`, and the `Link` header (`rel="next"`) are parsed into the returned `page` object (`hasMore` = a next page exists). -- Redirects: `301`/`302`/`308` with a `Location` header are followed exactly once (project moves) with `redirect: 'manual'`, preserving `PRIVATE-TOKEN` across the hop. -- Rate limits: a `429` records a module-level cooldown (honoring `Retry-After` / `RateLimit-Reset` when present) and surfaces `{ status: 429, error: 'GitLab rate limited' }`. While the cooldown is active, requests short-circuit without hitting the network. +- Transport: each call spawns a `glab api` process with `--include` for HTTP status/headers. Auth is passed via `GITLAB_TOKEN` env var (never on argv). Binary path: `GLAB_BIN` env or `/home/user/.local/bin/glab`. +- Base URL: the `baseUrl` parameter is passed to the client constructor for compatibility but `glab` resolves the instance from its own config. The `--include` flag provides HTTP status codes and response headers. +- Per-request timeout: 8000 ms via `AbortSignal.timeout` on the spawned process. The process is killed with `SIGKILL` on timeout. +- Pagination: `--paginate` is passed for GET requests with query params, causing `glab` to fetch all pages in a single call. The returned `page` object is `null` since pagination is handled by the CLI. - `request` never throws for HTTP error statuses — callers branch on `status`. +- ETag cache and rate-limit cooldown have been dropped with the CLI pivot. Each call spawns a fresh process, so there is no persistent connection for conditional requests or shared rate-limit state. `isGitLabRateLimited()` always returns `false`; `noteGitLabRateLimit()` is a no-op. ## API integration overview @@ -148,5 +147,5 @@ Conventions mirror `github/routes.js`: - Keep the response shapes in lockstep with `GitLab*` types in `packages/ui/src/lib/api/types.ts`. - 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. +- The `glab` CLI handles authentication, base URL resolution, and pagination internally. The client does not maintain its own ETag cache or rate-limit cooldown — each call spawns a fresh process. - To add further GitLab write operations, add the endpoint in `routes.js`, add a convenience method in `client.js`, and extend the shared types — mirror the existing issue/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 fddf2ff4..05443e17 100644 --- a/packages/web/server/lib/gitlab/client.js +++ b/packages/web/server/lib/gitlab/client.js @@ -1,193 +1,147 @@ +import { spawn } from 'child_process'; import { getGitLabAuth, getGitLabDefaultBaseUrl } from './auth.js'; -// Per-request timeout for every GitLab call. GitLab REST can hang under load -// (especially self-hosted instances); bounding each request lets the caller -// fail fast and serve cached/last-known state instead of holding a socket open. +const GLAB_BIN = process.env.GLAB_BIN || '/home/user/.local/bin/glab'; const REQUEST_TIMEOUT_MS = 8000; -const timeoutFetch = (url, options = {}) => { - // Respect a caller-provided signal if present; otherwise attach our timeout. - if (options.signal) { - return fetch(url, options); - } - return fetch(url, { ...options, signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS) }); -}; - -// Conditional-request cache for GET calls: GitLab serves 304 Not Modified for -// matching If-None-Match without consuming a fresh rate-limit token, so -// polling unchanged issues/MRs stays cheap. Keyed by token+URL so different -// identities never share responses. GitLab (unlike GitHub) does not attach -// `ETag` to every endpoint, but when it does we revalidate exactly like -// github/octokit.js. -const ETAG_CACHE_MAX_ENTRIES = 300; -const etagCache = new Map(); - -const rememberEtag = (key, etag, body, headers) => { - etagCache.delete(key); - etagCache.set(key, { etag, body, headers }); - if (etagCache.size > ETAG_CACHE_MAX_ENTRIES) { - const oldest = etagCache.keys().next().value; - if (oldest !== undefined) { - etagCache.delete(oldest); - } - } -}; - -const createConditionalFetch = (token) => async (url, options = {}) => { - const method = (options.method || 'GET').toUpperCase(); - if (method !== 'GET') { - return timeoutFetch(url, options); - } - - const cacheKey = `${token}\n${url}`; - const cached = etagCache.get(cacheKey); - const headers = { ...(options.headers || {}) }; - if (cached?.etag) { - headers['if-none-match'] = cached.etag; - } - - const response = await timeoutFetch(url, { ...options, headers }); - - if (response.status === 304 && cached) { - // Touch for LRU and replay the cached success response. - rememberEtag(cacheKey, cached.etag, cached.body, cached.headers); - return new Response(cached.body, { status: 200, headers: cached.headers }); - } - - if (response.ok) { - const etag = response.headers.get('etag'); - if (etag) { - const body = await response.arrayBuffer(); - rememberEtag(cacheKey, etag, body, response.headers); - return new Response(body, { status: response.status, headers: response.headers }); - } - } - - return response; -}; - -// ---- Own rate-limit cooldown (deliberately NOT shared with github/rate-limit.js) ---- -const MAX_COOLDOWN_MS = 15 * 60 * 1000; -const DEFAULT_COOLDOWN_MS = 60 * 1000; -let rateLimitedUntil = 0; - -const headerValue = (headers, name) => { - if (!headers) return undefined; - if (typeof headers.get === 'function') return headers.get(name); - return headers[name]; -}; +// NOTE: ETag conditional-GET cache and rate-limit cooldown have been dropped +// with the pivot to CLI transports. Each call spawns a fresh `glab` process, +// so there is no persistent connection to attach conditional headers to, and +// rate-limit state cannot be shared across invocations. The tradeoff is higher +// latency per call (process spawn overhead) and no 304 short-circuit, but +// simpler state management and no module-level mutable cache. Callers that +// relied on `isGitLabRateLimited()` will always see false (no cooldown active). /** - * Record a cooldown after a GitLab 429. Accepts a fetch Response or any object - * carrying headers (response, `retry-after` seconds, or `RateLimit-Reset` - * Unix seconds). + * Spawn a CLI binary and return { stdout, stderr, exitCode }. + * Rejects if the process does not finish within REQUEST_TIMEOUT_MS. */ -export function noteGitLabRateLimit(error) { - const headers = error?.headers; - let retryMs = null; - const retryAfter = headerValue(headers, 'retry-after'); - if (retryAfter !== undefined && retryAfter !== null) { - const secs = Number(retryAfter); - if (Number.isFinite(secs) && secs > 0) retryMs = secs * 1000; +function spawnCli(bin, args, env, timeoutMs = REQUEST_TIMEOUT_MS) { + return new Promise((resolve, reject) => { + const child = spawn(bin, args, { + env: { ...process.env, ...env }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk) => { stdout += chunk; }); + child.stderr.on('data', (chunk) => { stderr += chunk; }); + const timer = setTimeout(() => { + child.kill('SIGKILL'); + reject(new Error(`CLI ${bin} timed out after ${timeoutMs}ms`)); + }, timeoutMs); + child.on('close', (code) => { + clearTimeout(timer); + resolve({ stdout, stderr, exitCode: code ?? 1 }); + }); + child.on('error', (err) => { + clearTimeout(timer); + reject(err); + }); + }); +} + +/** + * Run a `glab api` call and parse the response envelope. + * + * `glab api --include` outputs: + * + * + * + * + * + * Without `--include`, stdout is just the JSON body on success. + */ +async function glabApiCall(endpoint, { method = 'GET', body, raw, paginate, glabBin, token }) { + const args = ['api', '--include']; + if (method !== 'GET') args.push('-X', method); + if (paginate) args.push('--paginate'); + if (raw) args.push('--header', 'Accept: text/plain'); + if (body !== undefined) args.push('--header', 'Content-Type: application/json', '-d', JSON.stringify(body)); + args.push(endpoint); + + let result; + try { + result = await spawnCli(glabBin, args, { GITLAB_TOKEN: token }); + } catch (err) { + return { status: 500, headers: {}, data: null, page: null, error: err.message }; } - if (retryMs === null) { - const reset = headerValue(headers, 'ratelimit-reset'); - if (reset !== undefined && reset !== null) { - const delta = Number(reset) * 1000 - Date.now(); - if (Number.isFinite(delta) && delta > 0) retryMs = delta; + + const { stdout, stderr, exitCode } = result; + + if (exitCode !== 0 && !stdout.trim()) { + return { status: 500, headers: {}, data: null, page: null, error: stderr.trim() || `glab exited with code ${exitCode}` }; + } + + // Parse --include output: status line, headers, blank line, body. + const lines = stdout.split('\n'); + let status = 200; + const headers = {}; + let bodyStart = 0; + + const statusMatch = lines[0]?.match(/HTTP\/\S+\s+(\d+)/); + if (statusMatch) { + status = Number(statusMatch[1]); + for (let i = 1; i < lines.length; i++) { + if (lines[i].trim() === '') { + bodyStart = i + 1; + break; + } + const colonIdx = lines[i].indexOf(':'); + if (colonIdx > 0) { + headers[lines[i].slice(0, colonIdx).trim().toLowerCase()] = lines[i].slice(colonIdx + 1).trim(); + } } } - if (retryMs === null) retryMs = DEFAULT_COOLDOWN_MS; - retryMs = Math.min(retryMs, MAX_COOLDOWN_MS); - const until = Date.now() + retryMs; - if (until > rateLimitedUntil) { - rateLimitedUntil = until; - console.warn(`[gitlab] rate limited — pausing GitLab calls for ~${Math.round(retryMs / 1000)}s`); + + const bodyText = lines.slice(bodyStart).join('\n').trim(); + if (!bodyText) { + return { status, headers, data: null, page: null }; + } + + if (raw) { + return { status, headers, data: bodyText, page: null }; + } + + try { + return { status, headers, data: JSON.parse(bodyText), page: null }; + } catch { + return { status, headers, data: bodyText, page: null }; } } -export function isGitLabRateLimited() { - return Date.now() < rateLimitedUntil; -} +// ---- Rate-limit helpers (no-ops with CLI transport) ---- +export function noteGitLabRateLimit() { /* no-op: CLI processes are stateless */ } +export function isGitLabRateLimited() { return false; } // ---- Response helpers ---- -const joinApiUrl = (baseUrl, path) => { - const base = String(baseUrl || getGitLabDefaultBaseUrl()).replace(/\/+$/, ''); - const p = typeof path === 'string' && path ? (path.startsWith('/') ? path : `/${path}`) : ''; - return `${base}/api/v4${p}`; -}; - -const headersToObject = (headers) => { - const out = {}; - if (!headers) return out; - if (typeof headers.forEach === 'function') { - headers.forEach((value, key) => { - out[key] = value; - }); - } else if (typeof headers === 'object') { - for (const [key, value] of Object.entries(headers)) { - out[key] = value; - } - } - return out; -}; - -const parsePageInfo = (headers) => { - const get = (name) => { - const value = headerValue(headers, name); - return typeof value === 'string' ? value : ''; - }; - const pageHeader = get('x-page'); - const nextPage = get('x-next-page'); - const totalPages = get('x-total-pages'); - const linkHeader = get('link'); - const relNextMatch = linkHeader.match(/<([^>]+)>\s*;\s*rel="next"/); - const page = pageHeader ? Number(pageHeader) : null; - const next = nextPage ? Number(nextPage) : null; - const total = totalPages ? Number(totalPages) : null; - const hasMore = next != null ? next > 0 : Boolean(relNextMatch); - const parsed = { page, next, total, hasMore }; - if (relNextMatch) { - parsed.nextUrl = relNextMatch[1]; - } - return parsed; -}; - -const parseData = async (response) => { - const text = await response.text(); - if (!text) { - return null; - } - try { - return JSON.parse(text); - } catch { - return null; - } -}; - const encodeProject = (pathWithNamespace) => encodeURIComponent(String(pathWithNamespace)); +// Build the relative API path for glab CLI. glab resolves the base URL from its +// own config, so we pass only the /api/v4/... portion. +const apiPath = (path) => { + const p = typeof path === 'string' && path ? (path.startsWith('/') ? path : `/${path}`) : ''; + return `/api/v4${p}`; +}; + /** - * Create a raw-fetch GitLab REST v4 client. `request` never throws for HTTP - * error statuses — it returns `{ status, headers, data, page }` so callers can - * branch on status codes. On 429 it also sets `error: 'GitLab rate limited'` - * and records a module-level cooldown. + * Create a CLI-backed GitLab REST v4 client. Spawns `glab api` for each + * request. `request` never throws for HTTP error statuses — it returns + * `{ status, headers, data, page }` so callers can branch on status codes. */ export function createGitLabClient({ token, baseUrl }) { const effectiveBaseUrl = normalizeBaseForClient(baseUrl); + const glabBin = process.env.GLAB_BIN || '/home/user/.local/bin/glab'; const request = async (path, options = {}) => { const method = (typeof options.method === 'string' ? options.method : 'GET').toUpperCase(); const query = options.query && typeof options.query === 'object' ? options.query : {}; const body = options.body; - const callerSignal = options.signal; - if (isGitLabRateLimited()) { - return { status: 429, headers: {}, data: null, page: null, error: 'GitLab rate limited' }; - } + // Build the relative endpoint path with query params baked in. + let endpoint = apiPath(path); - let url = joinApiUrl(effectiveBaseUrl, path); const qs = new URLSearchParams(); let hasQuery = false; for (const [key, value] of Object.entries(query)) { @@ -196,61 +150,11 @@ export function createGitLabClient({ token, baseUrl }) { hasQuery = true; } if (hasQuery) { - url += `${url.includes('?') ? '&' : '?'}${qs.toString()}`; + endpoint += `${endpoint.includes('?') ? '&' : '?'}${qs.toString()}`; } - const headers = { - 'PRIVATE-TOKEN': token, - accept: 'application/json', - }; - const fetchOptions = { - method, - headers, - redirect: 'manual', - }; - if (body !== undefined) { - headers['content-type'] = 'application/json'; - fetchOptions.body = JSON.stringify(body); - } - if (callerSignal) { - fetchOptions.signal = callerSignal; - } - - const conditionalFetch = createConditionalFetch(token); - - let response = await conditionalFetch(url, fetchOptions); - - // Follow a project-move redirect exactly once. GitLab redirects - // (301/302/308) come with a `Location` for the new project URL; a manual - // redirect keeps our PRIVATE-TOKEN header across the hop. Only follow - // same-origin redirects to avoid leaking the token to a different host. - let redirects = 0; - const baseHost = new URL(url).host; - while ( - (response.status === 301 || response.status === 302 || response.status === 308) - && headerValue(response.headers, 'location') - && redirects < 1 - ) { - const location = headerValue(response.headers, 'location'); - const nextUrl = new URL(location, url).toString(); - if (new URL(nextUrl).host !== baseHost) break; - response = await conditionalFetch(nextUrl, fetchOptions); - redirects += 1; - } - - const result = { - status: response.status, - headers: headersToObject(response.headers), - data: await parseData(response), - page: parsePageInfo(response.headers), - }; - - if (response.status === 429) { - noteGitLabRateLimit(response); - result.error = 'GitLab rate limited'; - } - - return result; + const paginate = method === 'GET' && hasQuery; + return glabApiCall(endpoint, { method, body, paginate, glabBin, token }); }; return { @@ -294,9 +198,6 @@ export function createGitLabClient({ token, baseUrl }) { request(`/projects/${encodeProject(pathWithNamespace)}/merge_requests/${iid}/merge`, { method: 'PUT', body }), branches: (pathWithNamespace, params = {}) => request(`/projects/${encodeProject(pathWithNamespace)}/repository/branches`, { query: params }), - // Project members (direct + inherited) are the assignable/mentionable user - // set. `members/all` includes inherited group members; `query` filters - // server-side by username/name/email. members: (pathWithNamespace, params = {}) => request(`/projects/${encodeProject(pathWithNamespace)}/members/all`, { query: params }), labels: (pathWithNamespace, params = {}) => diff --git a/packages/web/server/lib/gitlab/client.test.js b/packages/web/server/lib/gitlab/client.test.js index 512cc96a..a385d903 100644 --- a/packages/web/server/lib/gitlab/client.test.js +++ b/packages/web/server/lib/gitlab/client.test.js @@ -1,6 +1,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; +import { EventEmitter } from 'events'; import { afterAll, afterEach, describe, expect, test, vi } from 'vitest'; // Isolate auth storage so getGitLabClientOrNull never reads a real account. @@ -11,6 +12,14 @@ afterAll(() => { fs.rmSync(TEMP_DATA_DIR, { recursive: true, force: true }); }); +// Mock child_process.spawn to simulate `glab api --include` output. +const originalSpawn = (await import('child_process')).spawn; +let spawnMock = null; + +vi.mock('child_process', () => ({ + spawn: (...args) => (spawnMock ? spawnMock(...args) : originalSpawn(...args)), +})); + const { createGitLabClient, getGitLabClientOrNull, @@ -18,86 +27,113 @@ const { noteGitLabRateLimit, } = await import('./client.js'); -const jsonResponse = (data, { status = 200, headers = {} } = {}) => - new Response(JSON.stringify(data), { status, headers: { 'content-type': 'application/json', ...headers } }); +/** + * Build the `glab api --include` output format: + * \n\n...\n\n + */ +const cliOutput = (data, { status = 200, headers = {} } = {}) => { + const lines = [`HTTP/1.1 ${status} OK`]; + for (const [k, v] of Object.entries(headers)) { + lines.push(`${k}: ${v}`); + } + lines.push(''); + lines.push(typeof data === 'string' ? data : JSON.stringify(data)); + return lines.join('\n'); +}; -const originalFetch = globalThis.fetch; +/** + * Create a vi.fn() mock spawn function that calls `on('close')` with the given + * exit code and delivers `output` on stdout. + */ +const mockSpawn = (output, { exitCode = 0, stderr = '' } = {}) => { + return vi.fn((...args) => { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.kill = vi.fn(); + queueMicrotask(() => { + child.stdout.emit('data', output); + if (stderr) child.stderr.emit('data', stderr); + child.emit('close', exitCode); + }); + return child; + }); +}; afterEach(() => { - globalThis.fetch = originalFetch; + spawnMock = null; }); describe('createGitLabClient request basics', () => { - test('calls {baseUrl}/api/v4{path} and sends PRIVATE-TOKEN', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ id: 42, username: 'alice' })); - globalThis.fetch = fetchMock; + test('spawns glab api --include and sends GITLAB_TOKEN env', async () => { + spawnMock = mockSpawn(cliOutput({ id: 42, username: 'alice' })); const client = createGitLabClient({ token: 'glpat-token', baseUrl: 'https://gitlab.com' }); const result = await client.user(); - expect(fetchMock).toHaveBeenCalledTimes(1); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitlab.com/api/v4/user'); - expect(options.headers['PRIVATE-TOKEN']).toBe('glpat-token'); + expect(spawnMock).toHaveBeenCalledTimes(1); + const [bin, args, opts] = spawnMock.mock.calls[0]; + expect(bin).toBe('/home/user/.local/bin/glab'); + expect(args).toEqual(['api', '--include', '/api/v4/user']); + expect(opts.env.GITLAB_TOKEN).toBe('glpat-token'); expect(result).toMatchObject({ status: 200, data: { id: 42, username: 'alice' } }); expect(result.error).toBeUndefined(); }); - test('joins a custom base URL without duplicating /api/v4', async () => { - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; + test('joins a custom base URL path without duplicating /api/v4', async () => { + spawnMock = mockSpawn(cliOutput([])); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.example.com/gitlab/' }); await client.issues('group/sub', { state: 'opened' }); - const [url] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitlab.example.com/gitlab/api/v4/projects/group%2Fsub/issues?state=opened'); + const [, args] = spawnMock.mock.calls[0]; + const endpoint = args[args.length - 1]; + // CLI transport passes relative paths; base URL is resolved by glab's config. + expect(endpoint).toContain('/api/v4/projects/group%2Fsub/issues?state=opened'); }); test('encodes project path namespaces exactly once', async () => { - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput([])); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); await client.mergeRequest('a/b/c', 5); - const [url] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitlab.com/api/v4/projects/a%2Fb%2Fc/merge_requests/5'); - expect(String(url)).not.toContain('%252F'); + const [, args] = spawnMock.mock.calls[0]; + const endpoint = args[args.length - 1]; + expect(endpoint).toContain('a%2Fb%2Fc/merge_requests/5'); + expect(endpoint).not.toContain('%252F'); }); test('serializes query params and omits empty ones', async () => { - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput([])); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); await client.mergeRequests('g/p', { state: 'opened', per_page: 50, page: 2, search: '', sort: null }); - const [url] = fetchMock.mock.calls[0]; - const query = String(url).split('?')[1]; - expect(query).toContain('state=opened'); - expect(query).toContain('per_page=50'); - expect(query).toContain('page=2'); - expect(query).not.toContain('search'); - expect(query).not.toContain('sort'); + const [, args] = spawnMock.mock.calls[0]; + const endpoint = args[args.length - 1]; + expect(endpoint).toContain('state=opened'); + expect(endpoint).toContain('per_page=50'); + expect(endpoint).toContain('page=2'); + expect(endpoint).not.toContain('search'); + expect(endpoint).not.toContain('sort'); }); - test('POST requests send a JSON body', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ ok: true }, { status: 201 })); - globalThis.fetch = fetchMock; + test('POST requests pass the method flag and a JSON body', async () => { + spawnMock = mockSpawn(cliOutput({ ok: true }, { status: 201 })); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); await client.request('/some/action', { method: 'POST', body: { hello: 'world' } }); - const [, options] = fetchMock.mock.calls[0]; - expect(options.method).toBe('POST'); - expect(options.headers['content-type']).toBe('application/json'); - expect(options.body).toBe(JSON.stringify({ hello: 'world' })); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain('-d'); + expect(args).toContain(JSON.stringify({ hello: 'world' })); }); test('surfaces error statuses without throwing', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ message: 'nope' }, { status: 401 })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ message: 'nope' }, { status: 401 })); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); const result = await client.user(); @@ -105,119 +141,47 @@ describe('createGitLabClient request basics', () => { expect(result.data).toEqual({ message: 'nope' }); }); - test('attaches a caller signal when provided, else a timeout signal', async () => { - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; + test('returns 500 on CLI process error', async () => { + spawnMock = (...args) => { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.kill = vi.fn(); + queueMicrotask(() => { + child.emit('error', new Error('ENOENT')); + }); + return child; + }; const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); - const controller = new AbortController(); - await client.branches('g/p', { per_page: 100 }); - await client.request('/user', { signal: controller.signal }); + const result = await client.user(); + expect(result.status).toBe(500); + expect(result.error).toContain('ENOENT'); + }); - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(fetchMock.mock.calls[0][1].signal).toEqual(expect.any(AbortSignal)); - expect(fetchMock.mock.calls[1][1].signal).toBe(controller.signal); + test('returns 500 on non-zero exit with no stdout', async () => { + spawnMock = mockSpawn('', { exitCode: 1, stderr: 'not authenticated' }); + + const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); + const result = await client.user(); + expect(result.status).toBe(500); + expect(result.error).toBe('not authenticated'); }); }); describe('pagination', () => { - test('parses x-page/x-next-page headers into the page object', async () => { - const fetchMock = vi.fn(async () => jsonResponse([], { - headers: { 'x-page': '2', 'x-next-page': '3', 'x-total-pages': '5' }, - })); - globalThis.fetch = fetchMock; + test('page object is null (CLI handles pagination)', async () => { + spawnMock = mockSpawn(cliOutput([])); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); const result = await client.issues('g/p', { page: 2 }); - expect(result.page).toEqual({ page: 2, next: 3, total: 5, hasMore: true }); - }); - - test('falls back to the Link rel=next header when x-next-page is absent', async () => { - const fetchMock = vi.fn(async () => jsonResponse([], { - headers: { link: '; rel="next", <...>; rel="last"' }, - })); - globalThis.fetch = fetchMock; - - const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); - const result = await client.issues('g/p', { page: 2 }); - expect(result.page.hasMore).toBe(true); - expect(result.page.nextUrl).toBe('https://gitlab.com/api/v4/projects/g%2Fp/issues?page=3'); - }); - - test('reports hasMore=false on the last page', async () => { - const fetchMock = vi.fn(async () => jsonResponse([], { - headers: { 'x-page': '5', 'x-next-page': '', 'x-total-pages': '5' }, - })); - globalThis.fetch = fetchMock; - - const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); - const result = await client.issues('g/p', { page: 5 }); - expect(result.page.hasMore).toBe(false); - }); -}); - -describe('redirect handling', () => { - test('follows a project-move redirect exactly once, preserving auth headers', async () => { - const movedUrl = 'https://gitlab.com/api/v4/projects/new%2Fhome/issues'; - const fetchMock = vi.fn(async (url) => { - if (String(url).includes('/projects/g%2Fp/issues')) { - return jsonResponse({}, { status: 301, headers: { location: '/api/v4/projects/new%2Fhome/issues' } }); - } - if (String(url) === movedUrl) { - return jsonResponse([{ iid: 1 }]); - } - return jsonResponse({}, { status: 404 }); - }); - globalThis.fetch = fetchMock; - - const client = createGitLabClient({ token: 'glpat-t', baseUrl: 'https://gitlab.com' }); - const result = await client.issues('g/p'); - - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(result.status).toBe(200); - expect(result.data).toEqual([{ iid: 1 }]); - const [, secondOptions] = fetchMock.mock.calls[1]; - expect(secondOptions.headers['PRIVATE-TOKEN']).toBe('glpat-t'); - }); -}); - -describe('etag conditional cache', () => { - test('sends if-none-match and replays a 304 as a 200 with cached body', async () => { - const fetchMock = vi.fn(async (_url, options) => { - if (options.headers['if-none-match'] === '"v1"') { - return new Response(null, { status: 304 }); - } - return jsonResponse({ ok: true }, { headers: { etag: '"v1"' } }); - }); - globalThis.fetch = fetchMock; - - const client = createGitLabClient({ token: 'glpat-t', baseUrl: 'https://gitlab.com' }); - const first = await client.user(); - expect(first.status).toBe(200); - expect(first.data).toEqual({ ok: true }); - - const second = await client.user(); - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(fetchMock.mock.calls[1][1].headers['if-none-match']).toBe('"v1"'); - expect(second.status).toBe(200); - expect(second.data).toEqual({ ok: true }); - }); - - test('does not cache POST responses', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ ok: true })); - globalThis.fetch = fetchMock; - - const client = createGitLabClient({ token: 'glpat-t', baseUrl: 'https://gitlab.com' }); - await client.request('/thing', { method: 'POST', body: {} }); - await client.request('/thing', { method: 'POST', body: {} }); - expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result.page).toBeNull(); }); }); 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; + spawnMock = mockSpawn(cliOutput({ iid: 5, title: 'New MR' }, { status: 201 })); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); const result = await client.createMergeRequest('group/sub', { @@ -226,45 +190,42 @@ describe('merge request write methods', () => { 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' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain(JSON.stringify({ source_branch: 'feat/x', target_branch: 'main', title: 'New MR' })); + expect(args.some(a => typeof a === 'string' && a.includes('/merge_requests'))).toBe(true); 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; + spawnMock = mockSpawn(cliOutput({ iid: 5, title: 'Updated' })); 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' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('PUT'); + expect(args).toContain(JSON.stringify({ 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; + spawnMock = mockSpawn(cliOutput({ iid: 5, state: 'merged' })); 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 }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('PUT'); + expect(args).toContain(JSON.stringify({ squash: true })); + expect(args.some(a => typeof a === 'string' && a.includes('/merge'))).toBe(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; + spawnMock = mockSpawn(cliOutput({ message: 'Method Not Allowed' }, { status: 405 })); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); const result = await client.mergeMergeRequest('group/sub', 5, {}); @@ -275,98 +236,79 @@ describe('merge request write methods', () => { describe('issue and review write methods', () => { test('createIssueNote POSTs a body to the issue notes endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ id: 5, body: 'hi' }, { status: 201 })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ id: 5, body: 'hi' }, { status: 201 })); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); const result = await client.createIssueNote('group/sub', 7, 'Nice catch'); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitlab.com/api/v4/projects/group%2Fsub/issues/7/notes'); - expect(options.method).toBe('POST'); - expect(JSON.parse(options.body)).toEqual({ body: 'Nice catch' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain(JSON.stringify({ body: 'Nice catch' })); + expect(args.some(a => typeof a === 'string' && a.includes('/issues/7/notes'))).toBe(true); expect(result.status).toBe(201); }); test('createMrNote POSTs a body to the MR notes endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ id: 8, body: 'LGTM' }, { status: 201 })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ id: 8, body: 'LGTM' }, { status: 201 })); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); await client.createMrNote('group/sub', 12, 'LGTM'); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitlab.com/api/v4/projects/group%2Fsub/merge_requests/12/notes'); - expect(options.method).toBe('POST'); - expect(JSON.parse(options.body)).toEqual({ body: 'LGTM' }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args).toContain(JSON.stringify({ body: 'LGTM' })); + expect(args.some(a => typeof a === 'string' && a.includes('/merge_requests/12/notes'))).toBe(true); }); test('updateIssue PUTs params to the issue endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ iid: 7, title: 'Updated' })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ iid: 7, title: 'Updated' })); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); await client.updateIssue('group/sub', 7, { state_event: 'close', labels: ['bug'], milestone_id: 33 }); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitlab.com/api/v4/projects/group%2Fsub/issues/7'); - expect(options.method).toBe('PUT'); - expect(JSON.parse(options.body)).toEqual({ state_event: 'close', labels: ['bug'], milestone_id: 33 }); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('PUT'); + expect(args).toContain(JSON.stringify({ state_event: 'close', labels: ['bug'], milestone_id: 33 })); }); test('approveMr POSTs to the approve endpoint', async () => { - const fetchMock = vi.fn(async () => jsonResponse({ id: 1, state: 'approved' })); - globalThis.fetch = fetchMock; + spawnMock = mockSpawn(cliOutput({ id: 1, state: 'approved' })); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); await client.approveMr('group/sub', 12); - const [url, options] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitlab.com/api/v4/projects/group%2Fsub/merge_requests/12/approve'); - expect(options.method).toBe('POST'); + const [, args] = spawnMock.mock.calls[0]; + expect(args).toContain('-X'); + expect(args).toContain('POST'); + expect(args.some(a => typeof a === 'string' && a.includes('/approve'))).toBe(true); }); - test('milestones GETs the project milestones list', async () => { - const fetchMock = vi.fn(async () => jsonResponse([{ id: 33, title: 'v1.0' }])); - globalThis.fetch = fetchMock; + test('milestones passes state and per_page query params', async () => { + spawnMock = mockSpawn(cliOutput([{ id: 33, title: 'v1.0' }])); const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' }); await client.milestones('group/sub', { state: 'all', per_page: 100 }); - const [url] = fetchMock.mock.calls[0]; - expect(String(url)).toBe('https://gitlab.com/api/v4/projects/group%2Fsub/milestones?state=all&per_page=100'); + const [, args] = spawnMock.mock.calls[0]; + const endpoint = args[args.length - 1]; + expect(endpoint).toContain('state=all'); + expect(endpoint).toContain('per_page=100'); }); }); 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. - test('429 surfaces error and records a cooldown', async () => { - const fetchMock = vi.fn(async () => jsonResponse({}, { status: 429, headers: { 'retry-after': '5' } })); - globalThis.fetch = fetchMock; - - const client = createGitLabClient({ token: 'glpat-t', baseUrl: 'https://gitlab.com' }); - const result = await client.user(); - expect(result.status).toBe(429); - expect(result.error).toBe('GitLab rate limited'); - expect(isGitLabRateLimited()).toBe(true); + test('isGitLabRateLimited returns false (no-op with CLI transport)', () => { + expect(isGitLabRateLimited()).toBe(false); }); - test('short-circuits while the cooldown is active without calling fetch', async () => { + test('noteGitLabRateLimit is a no-op', () => { noteGitLabRateLimit({ headers: new Headers({ 'retry-after': '120' }) }); - const fetchMock = vi.fn(async () => jsonResponse([])); - globalThis.fetch = fetchMock; - - const client = createGitLabClient({ token: 'glpat-t', baseUrl: 'https://gitlab.com' }); - const gated = await client.issues('g/p'); - expect(gated.status).toBe(429); - expect(gated.error).toBe('GitLab rate limited'); - expect(fetchMock).not.toHaveBeenCalled(); - }); - - test('parses Retry-After seconds into the cooldown', () => { - noteGitLabRateLimit({ headers: new Headers({ 'retry-after': '120' }) }); - expect(isGitLabRateLimited()).toBe(true); + expect(isGitLabRateLimited()).toBe(false); }); test('getGitLabClientOrNull returns null without stored auth', () => {