fix: forge CLI pivot bugs against real tea/glab binaries
Fix critical bugs in the CLI transport layer that were masked by idealized unit test mocks. All fixes verified against live binaries. Server (gitea/client.js): - C1: Remove --paginate flag (tea rejects it with exit 1) - C2: Pass auth via -H 'Authorization: token' header instead of GITEA_SERVER_TOKEN env (tea ignores that env var) - C3: Parse --include output from stderr (tea writes headers to stderr, not stdout) - H2: Parse Link header for hasMore/pagination Server (gitlab/client.js): - C4: Remove /api/v4 prefix (glab adds it automatically; double prefix caused every call to 404) - H2: Parse Link header for hasMore/pagination Tests (both client.test.js): - H1: Rewrite mocks to match real CLI behavior: headers on stderr for tea, no --paginate, auth via -H header, Link header parsing - Add C3, C1, C4 specific regression tests UI (GiteaSettings, GitLabSettings): - U4: Replace return null loading state with animated skeleton (prevents blank flash) - U2: Surface actual error message in connect failure toast - U5: Add CLI transport hint below connect form - U6 (GitLab): Add transport hint for unconnected state UI (GiteaIssuePickerDialog, GitLabIssuePickerDialog): - U3: Add retry button when error state is displayed Live smoke tests passed: - tea api -H 'Authorization: token WRONG' /user → 401 - tea api /repos/Vibing/openchamber/issues?state=open&limit=2 → 200 - glab api user with GITLAB_TOKEN=dummy → 401 (not 404)
This commit is contained in:
@@ -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,31 @@ 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 from the Link header (H2 fix).
|
||||
const { next } = parseLinkHeader(headers.link);
|
||||
const listIsArray = Array.isArray(data);
|
||||
const hasMore = listIsArray ? next !== null : false;
|
||||
// page is the caller's current page (extracted from the request context by the
|
||||
// caller); for the client layer we return the page number derived from the
|
||||
// Link header's "next" URL if present, otherwise 1.
|
||||
let page = 1;
|
||||
if (next) {
|
||||
const pageMatch = next.match(/[?&]page=(\d+)/);
|
||||
if (pageMatch) {
|
||||
// next page exists — the *current* page is next - 1 (rough heuristic;
|
||||
// callers that know the page can override).
|
||||
page = Math.max(1, Number(pageMatch[1]) - 1);
|
||||
}
|
||||
}
|
||||
|
||||
return { status, headers, data, page: listIsArray ? page : null, hasMore: listIsArray ? hasMore : undefined };
|
||||
}
|
||||
|
||||
// ---- Rate-limit helpers (no-ops with CLI transport) ----
|
||||
@@ -128,7 +167,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 +193,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 {
|
||||
|
||||
Reference in New Issue
Block a user