Merge pull request 'fix: forge CLI pivot bugs against real tea/glab binaries' (#10) from feat/forge-cli-fix into custom
This commit is contained in:
@@ -84,7 +84,8 @@ export const GitLabSettings: React.FC<{ embedded?: boolean }> = ({ embedded = fa
|
||||
toast.success(t('settings.gitlab.page.toast.connected'));
|
||||
} catch (error) {
|
||||
console.error('Failed to connect GitLab:', error);
|
||||
toast.error(t('settings.gitlab.page.errors.failed'));
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
toast.error(t('settings.gitlab.page.errors.failed'), { description: message });
|
||||
} finally {
|
||||
setIsBusy(false);
|
||||
}
|
||||
@@ -144,7 +145,24 @@ export const GitLabSettings: React.FC<{ embedded?: boolean }> = ({ embedded = fa
|
||||
}, [runtimeGitLab, setStatus, t]);
|
||||
|
||||
if (isLoading) {
|
||||
return null;
|
||||
return (
|
||||
<SettingsSection
|
||||
title={t('settings.gitlab.page.title')}
|
||||
description={t('settings.gitlab.page.description')}
|
||||
info={t('settings.gitlab.page.tooltip.connectAccount')}
|
||||
settingsItem="git.gitlab-account"
|
||||
>
|
||||
<div className="rounded-lg bg-[var(--surface-elevated)]/70 overflow-hidden flex flex-col">
|
||||
<div className="px-4 py-3 flex items-center gap-4 animate-pulse">
|
||||
<div className="h-10 w-10 shrink-0 rounded-full bg-[var(--surface-muted)]" />
|
||||
<div className="flex-1 space-y-2">
|
||||
<div className="h-4 w-32 bg-[var(--surface-muted)] rounded" />
|
||||
<div className="h-3 w-48 bg-[var(--surface-muted)] rounded" />
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</SettingsSection>
|
||||
);
|
||||
}
|
||||
|
||||
const connected = Boolean(status?.connected);
|
||||
@@ -232,6 +250,9 @@ export const GitLabSettings: React.FC<{ embedded?: boolean }> = ({ embedded = fa
|
||||
{t('settings.gitlab.page.actions.connect')}
|
||||
</Button>
|
||||
</div>
|
||||
<p className="typography-micro text-muted-foreground">
|
||||
Authenticates via the glab CLI binary and your access token.
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
|
||||
|
||||
@@ -89,7 +89,8 @@ export const GiteaSettings: React.FC<{ embedded?: boolean }> = ({ embedded = fal
|
||||
toast.success(t('settings.gitea.page.toast.connected'));
|
||||
} catch (error) {
|
||||
console.error('Failed to connect Gitea:', error);
|
||||
toast.error(t('settings.gitea.page.errors.failed'));
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
toast.error(t('settings.gitea.page.errors.failed'), { description: message });
|
||||
} finally {
|
||||
setIsBusy(false);
|
||||
}
|
||||
@@ -149,7 +150,24 @@ export const GiteaSettings: React.FC<{ embedded?: boolean }> = ({ embedded = fal
|
||||
}, [runtimeGitea, setStatus, t]);
|
||||
|
||||
if (isLoading) {
|
||||
return null;
|
||||
return (
|
||||
<SettingsSection
|
||||
title={t('settings.gitea.page.title')}
|
||||
description={t('settings.gitea.page.description')}
|
||||
info={t('settings.gitea.page.tooltip.connectAccount')}
|
||||
settingsItem="git.gitea-account"
|
||||
>
|
||||
<div className="rounded-lg bg-[var(--surface-elevated)]/70 overflow-hidden flex flex-col">
|
||||
<div className="px-4 py-3 flex items-center gap-4 animate-pulse">
|
||||
<div className="h-10 w-10 shrink-0 rounded-full bg-[var(--surface-muted)]" />
|
||||
<div className="flex-1 space-y-2">
|
||||
<div className="h-4 w-32 bg-[var(--surface-muted)] rounded" />
|
||||
<div className="h-3 w-48 bg-[var(--surface-muted)] rounded" />
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</SettingsSection>
|
||||
);
|
||||
}
|
||||
|
||||
const connected = Boolean(status?.connected);
|
||||
@@ -242,6 +260,9 @@ export const GiteaSettings: React.FC<{ embedded?: boolean }> = ({ embedded = fal
|
||||
{t('settings.gitea.page.actions.connect')}
|
||||
</Button>
|
||||
</div>
|
||||
<p className="typography-micro text-muted-foreground">
|
||||
Authenticates via the tea CLI binary and your stored token.
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
|
||||
|
||||
@@ -545,7 +545,12 @@ export function GitLabIssuePickerDialog({
|
||||
) : null}
|
||||
|
||||
{error ? (
|
||||
<div className="text-center text-muted-foreground py-8 break-words">{error}</div>
|
||||
<div className="text-center text-muted-foreground py-8 space-y-2">
|
||||
<div className="break-words">{error}</div>
|
||||
<Button variant="outline" size="sm" onClick={refresh} disabled={isLoading}>
|
||||
{t('session.gitlabIssuePicker.actions.refresh')}
|
||||
</Button>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{directNumber && projectDirectory && gitlab && connected ? (
|
||||
|
||||
@@ -543,7 +543,12 @@ export function GiteaIssuePickerDialog({
|
||||
) : null}
|
||||
|
||||
{error ? (
|
||||
<div className="text-center text-muted-foreground py-8 break-words">{error}</div>
|
||||
<div className="text-center text-muted-foreground py-8 space-y-2">
|
||||
<div className="break-words">{error}</div>
|
||||
<Button variant="outline" size="sm" onClick={refresh} disabled={isLoading}>
|
||||
{t('session.giteaIssuePicker.actions.refresh')}
|
||||
</Button>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{directNumber && projectDirectory && gitea && connected ? (
|
||||
|
||||
@@ -3,7 +3,6 @@ import { getGiteaAuth } from './auth.js';
|
||||
import { getProviderApiBaseUrl } from '../git-providers/config.js';
|
||||
import { getEffectiveProviderApiBaseUrl } from '../git-providers/project-config.js';
|
||||
|
||||
const TEA_BIN = process.env.TEA_BIN || '/home/user/.local/bin/tea';
|
||||
const REQUEST_TIMEOUT_MS = 8000;
|
||||
|
||||
// NOTE: ETag conditional-GET cache and rate-limit cooldown have been dropped
|
||||
@@ -43,60 +42,80 @@ function spawnCli(bin, args, env, timeoutMs = REQUEST_TIMEOUT_MS) {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the `Link` header value to extract rel="next" and rel="prev" URLs.
|
||||
* Returns { next, prev } with the raw URL strings (or null).
|
||||
*/
|
||||
function parseLinkHeader(linkHeader) {
|
||||
if (!linkHeader) return { next: null, prev: null };
|
||||
const result = { next: null, prev: null };
|
||||
const parts = linkHeader.split(',');
|
||||
for (const part of parts) {
|
||||
const match = part.match(/<([^>]+)>;\s*rel="(\w+)"/);
|
||||
if (match) {
|
||||
const [, url, rel] = match;
|
||||
if (rel === 'next') result.next = url;
|
||||
if (rel === 'prev') result.prev = url;
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a `tea api` call and parse the response envelope.
|
||||
*
|
||||
* `tea api --include` outputs:
|
||||
* <status line: HTTP/1.1 200 OK>
|
||||
* <headers, one per line>
|
||||
* <empty line>
|
||||
* <JSON body>
|
||||
* tea's `--include` flag writes HTTP status and response headers to **stderr**
|
||||
* and the response body to **stdout**. The client reads both streams:
|
||||
*
|
||||
* Without `--include`, stdout is just the JSON body on success.
|
||||
* stderr: HTTP/2.0 200 OK\nHeader: value\n...\n\n
|
||||
* stdout: {"json": "body"}
|
||||
*
|
||||
* Auth is passed via `-H "Authorization: token <token>"` — the
|
||||
* `GITEA_SERVER_TOKEN` env var is ignored by tea (it uses its own config).
|
||||
*
|
||||
* Pagination: `--paginate` does NOT exist in tea. We parse the `Link` header
|
||||
* from stderr to derive `page` and `hasMore`.
|
||||
*/
|
||||
async function teaApiCall(endpoint, { method = 'GET', body, raw, paginate, teaBin, token }) {
|
||||
async function teaApiCall(endpoint, { method = 'GET', body, raw, 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));
|
||||
// Pass auth via header (C2 fix) — tea ignores GITEA_SERVER_TOKEN env.
|
||||
if (token) args.push('-H', `Authorization: token ${token}`);
|
||||
args.push(endpoint);
|
||||
|
||||
let result;
|
||||
try {
|
||||
result = await spawnCli(teaBin, args, { GITEA_SERVER_TOKEN: token });
|
||||
result = await spawnCli(teaBin, args, {});
|
||||
} catch (err) {
|
||||
return { status: 500, headers: {}, data: null, page: null, error: err.message };
|
||||
}
|
||||
|
||||
const { stdout, stderr, exitCode } = result;
|
||||
const { stdout, stderr } = 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');
|
||||
// Parse --include output from stderr: status line, headers, blank line.
|
||||
// stdout contains only the response body.
|
||||
let status = 200;
|
||||
const headers = {};
|
||||
let bodyStart = 0;
|
||||
|
||||
const statusMatch = lines[0]?.match(/HTTP\/\S+\s+(\d+)/);
|
||||
const stderrLines = stderr.split('\n');
|
||||
const statusMatch = stderrLines[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(':');
|
||||
for (let i = 1; i < stderrLines.length; i++) {
|
||||
if (stderrLines[i].trim() === '') break;
|
||||
const colonIdx = stderrLines[i].indexOf(':');
|
||||
if (colonIdx > 0) {
|
||||
headers[lines[i].slice(0, colonIdx).trim().toLowerCase()] = lines[i].slice(colonIdx + 1).trim();
|
||||
headers[stderrLines[i].slice(0, colonIdx).trim().toLowerCase()] = stderrLines[i].slice(colonIdx + 1).trim();
|
||||
}
|
||||
}
|
||||
} else if (stderr.trim() && !stdout.trim()) {
|
||||
// No --include output on stderr and no body — surface the stderr message.
|
||||
return { status: 500, headers: {}, data: null, page: null, error: stderr.trim() };
|
||||
}
|
||||
|
||||
const bodyText = lines.slice(bodyStart).join('\n').trim();
|
||||
const bodyText = stdout.trim();
|
||||
if (!bodyText) {
|
||||
return { status, headers, data: null, page: null };
|
||||
}
|
||||
@@ -105,11 +124,35 @@ async function teaApiCall(endpoint, { method = 'GET', body, raw, paginate, teaBi
|
||||
return { status, headers, data: bodyText, page: null };
|
||||
}
|
||||
|
||||
let data;
|
||||
try {
|
||||
return { status, headers, data: JSON.parse(bodyText), page: null };
|
||||
data = JSON.parse(bodyText);
|
||||
} catch {
|
||||
return { status, headers, data: bodyText, page: null };
|
||||
data = bodyText;
|
||||
}
|
||||
|
||||
// Derive pagination info object matching the pre-pivot parsePageInfo shape.
|
||||
// routes.js reads resp.page?.hasMore, resp.page?.nextUrl, etc.
|
||||
const { next } = parseLinkHeader(headers.link);
|
||||
const listIsArray = Array.isArray(data);
|
||||
const totalRaw = headers['x-total-count'];
|
||||
const total = totalRaw ? Number(totalRaw) : null;
|
||||
let currentPage = null;
|
||||
if (listIsArray && next) {
|
||||
const pageMatch = next.match(/[?&]page=(\d+)/);
|
||||
if (pageMatch) {
|
||||
currentPage = Math.max(1, Number(pageMatch[1]) - 1);
|
||||
}
|
||||
}
|
||||
const pageInfo = {
|
||||
page: currentPage,
|
||||
next: next || null,
|
||||
total: total !== null && Number.isFinite(total) ? total : null,
|
||||
hasMore: listIsArray ? next !== null : false,
|
||||
nextUrl: next || undefined,
|
||||
};
|
||||
|
||||
return { status, headers, data, page: pageInfo };
|
||||
}
|
||||
|
||||
// ---- Rate-limit helpers (no-ops with CLI transport) ----
|
||||
@@ -128,7 +171,8 @@ const apiPath = (path) => {
|
||||
/**
|
||||
* 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.
|
||||
* `{ status, headers, data, page, hasMore }` so callers can branch on status
|
||||
* codes.
|
||||
*/
|
||||
export function createGiteaClient({ token, baseUrl }) {
|
||||
const effectiveBaseUrl = typeof baseUrl === 'string' ? baseUrl.trim().replace(/\/+$/, '') : '';
|
||||
@@ -153,8 +197,7 @@ export function createGiteaClient({ token, baseUrl }) {
|
||||
endpoint += `${endpoint.includes('?') ? '&' : '?'}${qs.toString()}`;
|
||||
}
|
||||
|
||||
const paginate = method === 'GET' && hasQuery;
|
||||
return teaApiCall(endpoint, { method, body, raw, paginate, teaBin, token });
|
||||
return teaApiCall(endpoint, { method, body, raw, teaBin, token });
|
||||
};
|
||||
|
||||
return {
|
||||
|
||||
@@ -12,7 +12,11 @@ afterAll(() => {
|
||||
fs.rmSync(TEMP_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// Mock child_process.spawn to simulate `tea api --include` output.
|
||||
// Mock child_process.spawn to simulate real `tea api` output.
|
||||
// Real tea behavior:
|
||||
// - `--include` writes status+headers to stderr, body to stdout
|
||||
// - `--paginate` does NOT exist (tea rejects it)
|
||||
// - Auth via `-H "Authorization: token <token>"`, NOT via GITEA_SERVER_TOKEN env
|
||||
const originalSpawn = (await import('child_process')).spawn;
|
||||
let spawnMock = null;
|
||||
|
||||
@@ -28,32 +32,36 @@ const {
|
||||
} = await import('./client.js');
|
||||
|
||||
/**
|
||||
* Build the `tea api --include` output format:
|
||||
* <status line>\n<header: value>\n...\n\n<body>
|
||||
* Build real tea --include output: status+headers on stderr, body on stdout.
|
||||
*/
|
||||
const cliOutput = (data, { status = 200, headers = {} } = {}) => {
|
||||
const lines = [`HTTP/1.1 ${status} OK`];
|
||||
const stderrLines = [`HTTP/2.0 ${status} OK`];
|
||||
for (const [k, v] of Object.entries(headers)) {
|
||||
lines.push(`${k}: ${v}`);
|
||||
stderrLines.push(`${k}: ${v}`);
|
||||
}
|
||||
lines.push('');
|
||||
lines.push(typeof data === 'string' ? data : JSON.stringify(data));
|
||||
return lines.join('\n');
|
||||
stderrLines.push('');
|
||||
return {
|
||||
stdout: typeof data === 'string' ? data : JSON.stringify(data),
|
||||
stderr: stderrLines.join('\n'),
|
||||
};
|
||||
};
|
||||
|
||||
/**
|
||||
* Create a vi.fn() mock spawn function that calls `on('close')` with the given
|
||||
* exit code and delivers `output` on stdout.
|
||||
* Create a vi.fn() mock spawn that emits stderr (headers) and stdout (body)
|
||||
* separately, matching real tea --include behavior.
|
||||
*/
|
||||
const mockSpawn = (output, { exitCode = 0, stderr = '' } = {}) => {
|
||||
const mockSpawn = (output, { exitCode = 0 } = {}) => {
|
||||
const { stdout, stderr } = typeof output === 'object' && 'stdout' in output
|
||||
? output
|
||||
: cliOutput(output);
|
||||
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.stdout.emit('data', stdout);
|
||||
child.stderr.emit('data', stderr);
|
||||
child.emit('close', exitCode);
|
||||
});
|
||||
return child;
|
||||
@@ -65,7 +73,7 @@ afterEach(() => {
|
||||
});
|
||||
|
||||
describe('createGiteaClient request basics', () => {
|
||||
test('spawns tea api --include and sends GITEA_SERVER_TOKEN env', async () => {
|
||||
test('spawns tea api --include and passes auth via -H header (not env)', async () => {
|
||||
spawnMock = mockSpawn(cliOutput({ id: 42, login: 'alice' }));
|
||||
|
||||
const client = createGiteaClient({ token: 'gitea-token', baseUrl: 'https://gitea.example.com' });
|
||||
@@ -74,8 +82,12 @@ describe('createGiteaClient request basics', () => {
|
||||
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(args).toContain('--include');
|
||||
expect(args).toContain('-H');
|
||||
expect(args).toContain('Authorization: token gitea-token');
|
||||
expect(args).not.toContain('--paginate');
|
||||
// Auth is via header, NOT via GITEA_SERVER_TOKEN env (C2 fix).
|
||||
expect(opts.env.GITEA_SERVER_TOKEN).toBeUndefined();
|
||||
expect(result).toMatchObject({ status: 200, data: { id: 42, login: 'alice' } });
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
@@ -159,23 +171,83 @@ describe('createGiteaClient request basics', () => {
|
||||
expect(result.error).toContain('ENOENT');
|
||||
});
|
||||
|
||||
test('returns 500 on non-zero exit with no stdout', async () => {
|
||||
spawnMock = mockSpawn('', { exitCode: 1, stderr: 'not logged in' });
|
||||
test('returns 500 on non-zero exit with no stdout and no stderr status line', async () => {
|
||||
spawnMock = mockSpawn({ stdout: '', stderr: 'not logged in' }, { exitCode: 1 });
|
||||
|
||||
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');
|
||||
});
|
||||
|
||||
test('parses status code from stderr --include output (C3 fix)', async () => {
|
||||
// Simulate tea's real output: 403 on stderr, error body on stdout.
|
||||
spawnMock = mockSpawn(cliOutput({ message: 'forbidden' }, { status: 403 }));
|
||||
|
||||
const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' });
|
||||
const result = await client.user();
|
||||
expect(result.status).toBe(403);
|
||||
expect(result.data).toEqual({ message: 'forbidden' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('pagination', () => {
|
||||
test('page object is null (CLI handles pagination)', async () => {
|
||||
spawnMock = mockSpawn(cliOutput([], { headers: {} }));
|
||||
test('page is an object with hasMore=true when Link header has rel="next"', async () => {
|
||||
spawnMock = mockSpawn(
|
||||
cliOutput([{ id: 1 }], {
|
||||
headers: {
|
||||
'Link': '<https://gitea.example.com/api/v1/repos/o/r/issues?limit=1&page=2>; rel="next"',
|
||||
'X-Total-Count': '10',
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' });
|
||||
const result = await client.issues('o', 'r', { page: 2 });
|
||||
expect(result.page).toBeNull();
|
||||
const result = await client.issues('o', 'r', { limit: 1, page: 1 });
|
||||
// page is the object shape that routes.js expects
|
||||
expect(result.page).toEqual(expect.objectContaining({
|
||||
hasMore: true,
|
||||
nextUrl: 'https://gitea.example.com/api/v1/repos/o/r/issues?limit=1&page=2',
|
||||
next: 'https://gitea.example.com/api/v1/repos/o/r/issues?limit=1&page=2',
|
||||
page: 1,
|
||||
total: 10,
|
||||
}));
|
||||
expect(Array.isArray(result.data)).toBe(true);
|
||||
});
|
||||
|
||||
test('page is an object with hasMore=false when no Link header', async () => {
|
||||
spawnMock = mockSpawn(cliOutput([{ id: 1 }]));
|
||||
|
||||
const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' });
|
||||
const result = await client.issues('o', 'r', { limit: 50 });
|
||||
expect(result.page).toEqual(expect.objectContaining({
|
||||
hasMore: false,
|
||||
next: null,
|
||||
total: null,
|
||||
}));
|
||||
});
|
||||
|
||||
test('page is an object for non-array responses too', async () => {
|
||||
spawnMock = mockSpawn(cliOutput({ id: 42 }));
|
||||
|
||||
const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' });
|
||||
const result = await client.user();
|
||||
expect(result.page).toEqual(expect.objectContaining({
|
||||
hasMore: false,
|
||||
next: null,
|
||||
page: null,
|
||||
}));
|
||||
});
|
||||
|
||||
test('does NOT pass --paginate flag to tea (C1 fix)', async () => {
|
||||
spawnMock = mockSpawn(cliOutput([]));
|
||||
|
||||
const client = createGiteaClient({ token: 't', baseUrl: 'https://gitea.example.com' });
|
||||
await client.issues('o', 'r', { state: 'open', page: 2 });
|
||||
|
||||
const [, args] = spawnMock.mock.calls[0];
|
||||
expect(args).not.toContain('--paginate');
|
||||
expect(args).not.toContain('-paginate');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { spawn } from 'child_process';
|
||||
import { getGitLabAuth, getGitLabDefaultBaseUrl } from './auth.js';
|
||||
|
||||
const GLAB_BIN = process.env.GLAB_BIN || '/home/user/.local/bin/glab';
|
||||
const REQUEST_TIMEOUT_MS = 8000;
|
||||
|
||||
// NOTE: ETag conditional-GET cache and rate-limit cooldown have been dropped
|
||||
@@ -41,21 +40,45 @@ function spawnCli(bin, args, env, timeoutMs = REQUEST_TIMEOUT_MS) {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the `Link` header value to extract rel="next" and rel="prev" URLs.
|
||||
* Returns { next, prev } with the raw URL strings (or null).
|
||||
*/
|
||||
function parseLinkHeader(linkHeader) {
|
||||
if (!linkHeader) return { next: null, prev: null };
|
||||
const result = { next: null, prev: null };
|
||||
const parts = linkHeader.split(',');
|
||||
for (const part of parts) {
|
||||
const match = part.match(/<([^>]+)>;\s*rel="(\w+)"/);
|
||||
if (match) {
|
||||
const [, url, rel] = match;
|
||||
if (rel === 'next') result.next = url;
|
||||
if (rel === 'prev') result.prev = url;
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a `glab api` call and parse the response envelope.
|
||||
*
|
||||
* `glab api --include` outputs:
|
||||
* `glab api --include` outputs everything to **stdout**:
|
||||
* <status line: HTTP/1.1 200 OK>
|
||||
* <headers, one per line>
|
||||
* <empty line>
|
||||
* <JSON body>
|
||||
*
|
||||
* Without `--include`, stdout is just the JSON body on success.
|
||||
* On non-zero exit, stderr gets a human-readable error summary (e.g.,
|
||||
* `glab: 401 Unauthorized (HTTP 401)`).
|
||||
*
|
||||
* GITLAB_TOKEN env var IS respected by glab (unlike tea's GITEA_SERVER_TOKEN).
|
||||
*
|
||||
* Pagination: `--paginate` does NOT exist in glab. We parse the `Link` header
|
||||
* to derive `page` and `hasMore`.
|
||||
*/
|
||||
async function glabApiCall(endpoint, { method = 'GET', body, raw, paginate, glabBin, token }) {
|
||||
async function glabApiCall(endpoint, { method = 'GET', body, raw, 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);
|
||||
@@ -69,6 +92,8 @@ async function glabApiCall(endpoint, { method = 'GET', body, raw, paginate, glab
|
||||
|
||||
const { stdout, stderr, exitCode } = result;
|
||||
|
||||
// glab --include puts status+headers+body all on stdout.
|
||||
// On error, stdout may still have the full response, and stderr has the summary.
|
||||
if (exitCode !== 0 && !stdout.trim()) {
|
||||
return { status: 500, headers: {}, data: null, page: null, error: stderr.trim() || `glab exited with code ${exitCode}` };
|
||||
}
|
||||
@@ -103,11 +128,35 @@ async function glabApiCall(endpoint, { method = 'GET', body, raw, paginate, glab
|
||||
return { status, headers, data: bodyText, page: null };
|
||||
}
|
||||
|
||||
let data;
|
||||
try {
|
||||
return { status, headers, data: JSON.parse(bodyText), page: null };
|
||||
data = JSON.parse(bodyText);
|
||||
} catch {
|
||||
return { status, headers, data: bodyText, page: null };
|
||||
data = bodyText;
|
||||
}
|
||||
|
||||
// Derive pagination info object matching the pre-pivot parsePageInfo shape.
|
||||
// routes.js reads resp.page?.hasMore, resp.page?.nextUrl, etc.
|
||||
const { next } = parseLinkHeader(headers.link);
|
||||
const listIsArray = Array.isArray(data);
|
||||
const totalRaw = headers['x-total'] || headers['x-total-count'];
|
||||
const total = totalRaw ? Number(totalRaw) : null;
|
||||
let currentPage = null;
|
||||
if (listIsArray && next) {
|
||||
const pageMatch = next.match(/[?&]page=(\d+)/);
|
||||
if (pageMatch) {
|
||||
currentPage = Math.max(1, Number(pageMatch[1]) - 1);
|
||||
}
|
||||
}
|
||||
const pageInfo = {
|
||||
page: currentPage,
|
||||
next: next || null,
|
||||
total: total !== null && Number.isFinite(total) ? total : null,
|
||||
hasMore: listIsArray ? next !== null : false,
|
||||
nextUrl: next || undefined,
|
||||
};
|
||||
|
||||
return { status, headers, data, page: pageInfo };
|
||||
}
|
||||
|
||||
// ---- Rate-limit helpers (no-ops with CLI transport) ----
|
||||
@@ -119,16 +168,17 @@ export function isGitLabRateLimited() { return false; }
|
||||
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.
|
||||
// own config, so we pass the path WITHOUT the /api/v4 prefix — glab adds it.
|
||||
const apiPath = (path) => {
|
||||
const p = typeof path === 'string' && path ? (path.startsWith('/') ? path : `/${path}`) : '';
|
||||
return `/api/v4${p}`;
|
||||
return p;
|
||||
};
|
||||
|
||||
/**
|
||||
* 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.
|
||||
* `{ status, headers, data, page, hasMore }` so callers can branch on status
|
||||
* codes.
|
||||
*/
|
||||
export function createGitLabClient({ token, baseUrl }) {
|
||||
const effectiveBaseUrl = normalizeBaseForClient(baseUrl);
|
||||
@@ -153,8 +203,7 @@ export function createGitLabClient({ token, baseUrl }) {
|
||||
endpoint += `${endpoint.includes('?') ? '&' : '?'}${qs.toString()}`;
|
||||
}
|
||||
|
||||
const paginate = method === 'GET' && hasQuery;
|
||||
return glabApiCall(endpoint, { method, body, paginate, glabBin, token });
|
||||
return glabApiCall(endpoint, { method, body, glabBin, token });
|
||||
};
|
||||
|
||||
return {
|
||||
|
||||
@@ -12,7 +12,13 @@ afterAll(() => {
|
||||
fs.rmSync(TEMP_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// Mock child_process.spawn to simulate `glab api --include` output.
|
||||
// Mock child_process.spawn to simulate real `glab api` output.
|
||||
// Real glab behavior:
|
||||
// - `--include` writes status+headers+body ALL to stdout (not stderr)
|
||||
// - On error, stderr gets human-readable summary (e.g. "glab: 401 Unauthorized (HTTP 401)")
|
||||
// - `--paginate` does NOT exist
|
||||
// - GITLAB_TOKEN env IS respected (unlike tea's GITEA_SERVER_TOKEN)
|
||||
// - Paths must NOT include /api/v4 prefix (glab adds it)
|
||||
const originalSpawn = (await import('child_process')).spawn;
|
||||
let spawnMock = null;
|
||||
|
||||
@@ -28,11 +34,10 @@ const {
|
||||
} = await import('./client.js');
|
||||
|
||||
/**
|
||||
* Build the `glab api --include` output format:
|
||||
* <status line>\n<header: value>\n...\n\n<body>
|
||||
* Build real glab --include output: status+headers+body all on stdout.
|
||||
*/
|
||||
const cliOutput = (data, { status = 200, headers = {} } = {}) => {
|
||||
const lines = [`HTTP/1.1 ${status} OK`];
|
||||
const lines = [`HTTP/2.0 ${status} OK`];
|
||||
for (const [k, v] of Object.entries(headers)) {
|
||||
lines.push(`${k}: ${v}`);
|
||||
}
|
||||
@@ -42,8 +47,8 @@ const cliOutput = (data, { status = 200, headers = {} } = {}) => {
|
||||
};
|
||||
|
||||
/**
|
||||
* Create a vi.fn() mock spawn function that calls `on('close')` with the given
|
||||
* exit code and delivers `output` on stdout.
|
||||
* Create a vi.fn() mock spawn that delivers `output` on stdout and optionally
|
||||
* a stderr message, matching real glab --include behavior.
|
||||
*/
|
||||
const mockSpawn = (output, { exitCode = 0, stderr = '' } = {}) => {
|
||||
return vi.fn((...args) => {
|
||||
@@ -74,13 +79,14 @@ describe('createGitLabClient request basics', () => {
|
||||
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']);
|
||||
// C4 fix: path has NO /api/v4 prefix — glab adds it.
|
||||
expect(args).toEqual(['api', '--include', '/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 path without duplicating /api/v4', async () => {
|
||||
test('joins a custom base URL path without /api/v4 prefix (C4 fix)', async () => {
|
||||
spawnMock = mockSpawn(cliOutput([]));
|
||||
|
||||
const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.example.com/gitlab/' });
|
||||
@@ -88,8 +94,9 @@ describe('createGitLabClient request basics', () => {
|
||||
|
||||
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');
|
||||
// C4 fix: path does NOT include /api/v4 — glab adds it automatically.
|
||||
expect(endpoint).toContain('/projects/group%2Fsub/issues?state=opened');
|
||||
expect(endpoint).not.toContain('/api/v4');
|
||||
});
|
||||
|
||||
test('encodes project path namespaces exactly once', async () => {
|
||||
@@ -160,22 +167,70 @@ describe('createGitLabClient request basics', () => {
|
||||
});
|
||||
|
||||
test('returns 500 on non-zero exit with no stdout', async () => {
|
||||
spawnMock = mockSpawn('', { exitCode: 1, stderr: 'not authenticated' });
|
||||
spawnMock = mockSpawn('', { exitCode: 1, stderr: 'glab: 401 Unauthorized (HTTP 401)' });
|
||||
|
||||
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');
|
||||
expect(result.error).toBe('glab: 401 Unauthorized (HTTP 401)');
|
||||
});
|
||||
|
||||
test('does NOT pass --paginate flag to glab', async () => {
|
||||
spawnMock = mockSpawn(cliOutput([]));
|
||||
|
||||
const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' });
|
||||
await client.issues('g/p', { state: 'opened' });
|
||||
|
||||
const [, args] = spawnMock.mock.calls[0];
|
||||
expect(args).not.toContain('--paginate');
|
||||
});
|
||||
});
|
||||
|
||||
describe('pagination', () => {
|
||||
test('page object is null (CLI handles pagination)', async () => {
|
||||
spawnMock = mockSpawn(cliOutput([]));
|
||||
test('page is an object with hasMore=true when Link header has rel="next"', async () => {
|
||||
spawnMock = mockSpawn(
|
||||
cliOutput([{ id: 1 }], {
|
||||
headers: {
|
||||
'Link': '<https://gitlab.com/api/v4/projects/g%2Fp/issues?page=2>; rel="next"',
|
||||
'X-Total': '10',
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' });
|
||||
const result = await client.issues('g/p', { page: 2 });
|
||||
expect(result.page).toBeNull();
|
||||
const result = await client.issues('g/p', { per_page: 1 });
|
||||
expect(result.page).toEqual(expect.objectContaining({
|
||||
hasMore: true,
|
||||
nextUrl: 'https://gitlab.com/api/v4/projects/g%2Fp/issues?page=2',
|
||||
next: 'https://gitlab.com/api/v4/projects/g%2Fp/issues?page=2',
|
||||
page: 1,
|
||||
total: 10,
|
||||
}));
|
||||
expect(Array.isArray(result.data)).toBe(true);
|
||||
});
|
||||
|
||||
test('page is an object with hasMore=false when no Link header', async () => {
|
||||
spawnMock = mockSpawn(cliOutput([{ id: 1 }]));
|
||||
|
||||
const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' });
|
||||
const result = await client.issues('g/p', { per_page: 50 });
|
||||
expect(result.page).toEqual(expect.objectContaining({
|
||||
hasMore: false,
|
||||
next: null,
|
||||
total: null,
|
||||
}));
|
||||
});
|
||||
|
||||
test('page is an object for non-array responses too', async () => {
|
||||
spawnMock = mockSpawn(cliOutput({ id: 42 }));
|
||||
|
||||
const client = createGitLabClient({ token: 't', baseUrl: 'https://gitlab.com' });
|
||||
const result = await client.user();
|
||||
expect(result.page).toEqual(expect.objectContaining({
|
||||
hasMore: false,
|
||||
next: null,
|
||||
page: null,
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user