From 114e5e1fef67b507e10dece6672f8c9fc84c3251 Mon Sep 17 00:00:00 2001 From: iKwesi Date: Mon, 16 Mar 2026 01:40:23 -0400 Subject: [PATCH 1/5] feat: add issue tracker provider abstraction --- src/cli.ts | 41 +- src/core/diagnostics/status.ts | 29 +- src/core/gitlab/provider.ts | 382 +++++++++++++++++++ src/core/notifiers/statusNotifiers.ts | 6 +- src/core/trackers/contracts.ts | 104 +++++ src/core/trackers/provider.ts | 103 +++++ src/demo/goldenDemo.ts | 2 + tests/cli/status-command.test.ts | 54 +++ tests/diagnostics/status.test.ts | 66 +++- tests/gitlab/provider.test.ts | 139 +++++++ tests/notifications/status-notifiers.test.ts | 6 +- tests/trackers/provider.test.ts | 42 ++ 12 files changed, 950 insertions(+), 24 deletions(-) create mode 100644 src/core/gitlab/provider.ts create mode 100644 src/core/trackers/contracts.ts create mode 100644 src/core/trackers/provider.ts create mode 100644 tests/gitlab/provider.test.ts create mode 100644 tests/trackers/provider.test.ts diff --git a/src/cli.ts b/src/cli.ts index ba8c7be..e6c4792 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -26,6 +26,7 @@ import { type StatusResult } from "./core/diagnostics/status.js"; import { createWebhookStatusNotifier } from "./core/notifiers/statusNotifiers.js"; +import { ISSUE_TRACKER_PROVIDER_NAMES } from "./core/trackers/contracts.js"; interface CliWriter { write(chunk: string): boolean | void; @@ -88,7 +89,7 @@ Examples: Explain an artifact using deterministic evidence from related inputs. $ specforge status --repo iKwesi/SpecForge --pr 123 - Report pull request state and CI outcomes from GitHub. + Report review-request state and CI outcomes from the configured issue tracker. Workflow guide: 1. Run 'specforge doctor' before making changes to confirm your environment is ready. @@ -99,7 +100,7 @@ Workflow guide: Artifacts: - 'inspect' writes repository profile and architecture summary artifacts. - 'explain' reads one or more artifact files plus optional policy/schedule evidence. - - 'status' reads pull request state and status checks from GitHub. + - 'status' reads review-request state and status checks from the configured issue tracker. - 'doctor' reports readiness and exits non-zero when blocking issues are found. ` ); @@ -192,9 +193,16 @@ The command reads artifact inputs and optional policy/schedule context, then pri program .command("status") - .description("Report GitHub pull request state and CI outcomes. Example: specforge status --repo iKwesi/SpecForge --pr 123") - .requiredOption("--pr ", "Pull request number, URL, or branch to inspect") - .option("--repo ", "GitHub repository slug when --pr is not a pull request URL") + .description("Report issue-tracker review request state and CI outcomes. Example: specforge status --repo iKwesi/SpecForge --pr 123") + .requiredOption("--pr ", "Pull request or merge request number, URL, or branch to inspect") + .option( + "--repo ", + "Issue tracker repository/project path when --pr is not a review request URL" + ) + .option( + "--provider ", + `Issue tracker provider (${ISSUE_TRACKER_PROVIDER_NAMES.join(" or ")})` + ) .option( "--notify-webhook ", "Emit the status event to a webhook; delivery failures are reported without failing the status command, but invalid webhook configuration is still an error", @@ -207,16 +215,19 @@ The command reads artifact inputs and optional policy/schedule context, then pri Examples: $ specforge status --repo iKwesi/SpecForge --pr 123 $ specforge status --pr https://github.com/iKwesi/SpecForge/pull/123 + $ specforge status --provider gitlab --repo gitlab-org/cli --pr 42 $ specforge status --repo iKwesi/SpecForge --pr 123 --notify-webhook https://hooks.example.test/specforge -Use this after PR handoff when you need the latest GitHub merge state and status checks. +Use this after handoff when you need the latest review-request merge state and status checks. ` ) - .action(async (options: { pr: string; repo?: string; notifyWebhook: string[] }) => { + .action(async (options: { pr: string; repo?: string; provider?: string; notifyWebhook: string[] }) => { try { + const provider = normalizeIssueTrackerProvider(options.provider); const result = await statusRunner({ pull_request: options.pr, ...(options.repo ? { repository: options.repo } : {}), + ...(provider ? { provider } : {}), ...(options.notifyWebhook.length > 0 ? { notifiers: options.notifyWebhook.map((webhookUrl, index) => @@ -335,3 +346,19 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) function collectOptionValues(value: string, previous: string[]): string[] { return [...previous, value]; } + +function normalizeIssueTrackerProvider( + value: string | undefined +): "github" | "gitlab" | undefined { + if (value === undefined) { + return undefined; + } + + if (ISSUE_TRACKER_PROVIDER_NAMES.includes(value as "github" | "gitlab")) { + return value as "github" | "gitlab"; + } + + throw new Error( + `provider must be one of ${ISSUE_TRACKER_PROVIDER_NAMES.join(", ")}.` + ); +} diff --git a/src/core/diagnostics/status.ts b/src/core/diagnostics/status.ts index 636764e..1efdfb2 100644 --- a/src/core/diagnostics/status.ts +++ b/src/core/diagnostics/status.ts @@ -1,9 +1,10 @@ import { - createGitHubProvider, + createIssueTrackerProvider, type GetPullRequestStatusInput, - type GitHubProvider, - type GitHubPullRequestStatus -} from "../github/provider.js"; + type IssueTrackerProvider, + type IssueTrackerProviderName, + type IssueTrackerPullRequestStatus +} from "../trackers/provider.js"; import { emitStatusNotification, type StatusNotificationDelivery, @@ -11,24 +12,30 @@ import { } from "../notifiers/statusNotifiers.js"; export interface RunStatusInput extends GetPullRequestStatusInput { - github_provider?: GitHubProvider; + provider?: IssueTrackerProviderName; + issue_tracker_provider?: IssueTrackerProvider; notifiers?: StatusNotifier[]; emitted_at?: Date; } export interface StatusResult { - pull_request: GitHubPullRequestStatus; + pull_request: IssueTrackerPullRequestStatus; notification_deliveries?: StatusNotificationDelivery[]; } /** - * Report the current GitHub pull request status using the configured provider. + * Report the current review-request status using the configured issue tracker provider. * - * This stays intentionally narrow for v1: it reads pull request state and status - * checks without trying to infer broader run orchestration from GitHub alone. + * This stays intentionally narrow for v1: it reads review-request state and status + * checks without trying to infer broader run orchestration from the tracker alone. */ export async function runStatus(input: RunStatusInput): Promise { - const provider = input.github_provider ?? createGitHubProvider(); + const provider = + input.issue_tracker_provider ?? + createIssueTrackerProvider({ + ...(input.provider ? { provider: input.provider } : {}), + pull_request: input.pull_request + }); const pullRequest = await provider.getPullRequestStatus({ pull_request: input.pull_request, ...(input.repository ? { repository: input.repository } : {}) @@ -53,6 +60,8 @@ export function formatStatusReport(result: StatusResult): string { const lines = [ "SpecForge Status", "", + `Provider: ${result.pull_request.provider}`, + `Request Kind: ${result.pull_request.request_kind}`, `Pull Request: #${result.pull_request.number}`, `URL: ${result.pull_request.url}`, `Title: ${result.pull_request.title}`, diff --git a/src/core/gitlab/provider.ts b/src/core/gitlab/provider.ts new file mode 100644 index 0000000..86642ee --- /dev/null +++ b/src/core/gitlab/provider.ts @@ -0,0 +1,382 @@ +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +import { + deriveOverallStatus, + type CreatePullRequestInput, + type GetPullRequestStatusInput, + type IssueTrackerProvider, + type IssueTrackerPullRequestRef, + type IssueTrackerPullRequestState, + type IssueTrackerPullRequestStatus, + type IssueTrackerStatusCheck, + type IssueTrackerStatusCheckConclusion, + type IssueTrackerStatusCheckStatus +} from "../trackers/contracts.js"; + +const execFileAsync = promisify(execFile); + +export type GitLabProviderErrorCode = + | "provider_unavailable" + | "invalid_repository" + | "invalid_pull_request" + | "command_failed" + | "parse_failed"; + +export class GitLabProviderError extends Error { + readonly code: GitLabProviderErrorCode; + readonly details?: unknown; + + constructor(code: GitLabProviderErrorCode, message: string, details?: unknown) { + super(message); + this.name = "GitLabProviderError"; + this.code = code; + this.details = details; + } +} + +export type GitLabProvider = IssueTrackerProvider; + +interface ExecResult { + stdout: string; + stderr: string; +} + +type GitLabExec = (args: string[]) => Promise; + +export function createGitLabProvider(input: { + glab_binary?: string; + exec?: GitLabExec; +} = {}): GitLabProvider { + const exec = input.exec ?? createGlabExec(input.glab_binary ?? "glab"); + + return { + name: "gitlab", + request_kind: "merge_request", + async isAvailable() { + try { + await exec(["--version"]); + return true; + } catch { + return false; + } + }, + async createPullRequest(input: CreatePullRequestInput): Promise { + ensureGitLabProjectPath(input.repository); + ensureNonEmpty(input.title, "title"); + ensureNonEmpty(input.body, "body"); + ensureNonEmpty(input.base_branch, "base_branch"); + ensureNonEmpty(input.head_branch, "head_branch"); + + const description = appendIssueLinks(input.body, input.linked_issue_numbers ?? []); + const created = await runGlabCommand(exec, [ + "api", + `projects/${encodeURIComponent(input.repository)}/merge_requests`, + "--method", + "POST", + "--field", + `source_branch=${input.head_branch}`, + "--field", + `target_branch=${input.base_branch}`, + "--field", + `title=${input.draft ? prefixDraftTitle(input.title) : input.title}`, + "--field", + `description=${description}` + ]); + const parsed = parseJson(created.stdout); + + return { + number: readNumber(parsed.iid, "merge request iid"), + url: readString(parsed.web_url, "merge request url"), + head_branch: readString(parsed.source_branch, "head branch"), + base_branch: readString(parsed.target_branch, "base branch"), + linked_issue_numbers: extractLinkedIssueNumbers(readOptionalString(parsed.description) ?? description) + }; + }, + async getPullRequestStatus( + input: GetPullRequestStatusInput + ): Promise { + const repository = input.repository ?? parseGitLabMergeRequestUrl(input.pull_request)?.repository; + const mergeRequestIid = isGitLabMergeRequestUrl(input.pull_request) + ? parseGitLabMergeRequestUrl(input.pull_request)?.iid + : input.pull_request; + + if (!repository) { + throw new GitLabProviderError( + "invalid_repository", + "repository is required when pull_request is not a GitLab merge request URL." + ); + } + + ensureGitLabProjectPath(repository); + ensureNonEmpty(mergeRequestIid, "pull_request"); + + const viewed = await runGlabCommand(exec, [ + "api", + `projects/${encodeURIComponent(repository)}/merge_requests/${mergeRequestIid}` + ]); + const parsed = parseJson(viewed.stdout); + const statusChecks = readStatusChecks(parsed.head_pipeline); + + return { + provider: "gitlab", + request_kind: "merge_request", + number: readNumber(parsed.iid, "merge request iid"), + url: readString(parsed.web_url, "merge request url"), + title: readString(parsed.title, "merge request title"), + state: normalizePullRequestState(parsed.state), + merge_state_status: normalizeMergeStateStatus( + readOptionalString(parsed.detailed_merge_status), + readOptionalBoolean(parsed.draft) ?? false + ), + head_branch: readString(parsed.source_branch, "head branch"), + base_branch: readString(parsed.target_branch, "base branch"), + linked_issue_numbers: extractLinkedIssueNumbers(readOptionalString(parsed.description) ?? ""), + overall_status: deriveOverallStatus(statusChecks), + status_checks: statusChecks + }; + } + }; +} + +function createGlabExec(glabBinary: string): GitLabExec { + return async (args) => { + try { + const result = await execFileAsync(glabBinary, args, { encoding: "utf8" }); + return { + stdout: result.stdout ?? "", + stderr: result.stderr ?? "" + }; + } catch (error) { + const details = error as NodeJS.ErrnoException & { stdout?: string; stderr?: string }; + if (details.code === "ENOENT") { + throw new GitLabProviderError( + "provider_unavailable", + `GitLab CLI was not found at ${glabBinary}.`, + error + ); + } + + throw new GitLabProviderError( + "command_failed", + `GitLab CLI command failed: ${[glabBinary, ...args].join(" ")}`, + { + message: details.message, + stdout: details.stdout, + stderr: details.stderr + } + ); + } + }; +} + +async function runGlabCommand(exec: GitLabExec, args: string[]): Promise { + return exec(args); +} + +function ensureGitLabProjectPath(value: string): void { + ensureNonEmpty(value, "repository"); + const segments = value.split("/"); + if (segments.length < 2 || segments.some((segment) => segment.trim().length === 0)) { + throw new GitLabProviderError( + "invalid_repository", + "repository must be a GitLab project path like group/project." + ); + } +} + +function ensureNonEmpty(value: string | undefined, field: string): asserts value is string { + if (typeof value !== "string" || value.trim().length === 0) { + throw new GitLabProviderError("invalid_pull_request", `${field} must be non-empty.`); + } +} + +function parseJson(value: string): Record { + try { + const parsed = JSON.parse(value) as unknown; + if (!isPlainRecord(parsed)) { + throw new Error("expected JSON object"); + } + return parsed; + } catch (error) { + throw new GitLabProviderError("parse_failed", "Failed to parse GitLab CLI JSON output.", error); + } +} + +function readString(value: unknown, field: string): string { + if (typeof value !== "string" || value.trim().length === 0) { + throw new GitLabProviderError("parse_failed", `GitLab CLI returned invalid ${field}.`); + } + + return value; +} + +function readOptionalString(value: unknown): string | undefined { + return typeof value === "string" ? value : undefined; +} + +function readOptionalBoolean(value: unknown): boolean | undefined { + return typeof value === "boolean" ? value : undefined; +} + +function readNumber(value: unknown, field: string): number { + if (typeof value !== "number" || !Number.isFinite(value)) { + throw new GitLabProviderError("parse_failed", `GitLab CLI returned invalid ${field}.`); + } + + return value; +} + +function normalizePullRequestState(value: unknown): IssueTrackerPullRequestState { + const normalized = readString(value, "merge request state").toLowerCase(); + switch (normalized) { + case "opened": + return "open"; + case "closed": + case "locked": + return "closed"; + case "merged": + return "merged"; + default: + throw new GitLabProviderError( + "parse_failed", + "GitLab CLI returned an unknown merge request state." + ); + } +} + +function normalizeMergeStateStatus( + value: string | undefined, + draft: boolean +): IssueTrackerPullRequestStatus["merge_state_status"] { + if (draft) { + return "draft"; + } + + switch ((value ?? "").toLowerCase()) { + case "mergeable": + case "can_be_merged": + return "clean"; + case "need_rebase": + return "behind"; + case "conflict": + case "conflicts": + case "cannot_be_merged": + return "dirty"; + case "ci_must_pass": + case "discussions_not_resolved": + return "blocked"; + case "checking": + return "unstable"; + default: + return "unknown"; + } +} + +function readStatusChecks(value: unknown): IssueTrackerStatusCheck[] { + if (!isPlainRecord(value)) { + return []; + } + + const pipelineStatus = normalizePipelineStatus(readOptionalString(value.status)); + const detailsUrl = readOptionalString(value.web_url); + return [ + { + name: readOptionalString(value.name) ?? "head_pipeline", + type: "pipeline", + status: pipelineStatus.status, + conclusion: pipelineStatus.conclusion, + ...(detailsUrl ? { details_url: detailsUrl } : {}) + } + ]; +} + +function normalizePipelineStatus(value: string | undefined): { + status: IssueTrackerStatusCheckStatus; + conclusion: IssueTrackerStatusCheckConclusion; +} { + switch ((value ?? "").toLowerCase()) { + case "success": + return { status: "completed", conclusion: "success" }; + case "failed": + return { status: "completed", conclusion: "failure" }; + case "canceled": + case "cancelled": + return { status: "completed", conclusion: "cancelled" }; + case "skipped": + return { status: "completed", conclusion: "skipped" }; + case "manual": + return { status: "completed", conclusion: "action_required" }; + case "running": + return { status: "in_progress", conclusion: "pending" }; + case "created": + case "pending": + case "preparing": + case "waiting_for_resource": + return { status: "pending", conclusion: "pending" }; + case "scheduled": + return { status: "queued", conclusion: "pending" }; + default: + return { status: "completed", conclusion: "unknown" }; + } +} + +function appendIssueLinks(body: string, linkedIssueNumbers: number[]): string { + if (linkedIssueNumbers.length === 0) { + return body; + } + + const linkage = linkedIssueNumbers.map((issueNumber) => `Closes #${issueNumber}`).join("\n"); + return `${body}\n\n${linkage}`; +} + +function prefixDraftTitle(title: string): string { + return /^draft:/i.test(title) ? title : `Draft: ${title}`; +} + +function extractLinkedIssueNumbers(body: string): number[] { + const matches = body.matchAll(/\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)\b/gi); + const linkedIssueNumbers = new Set(); + for (const match of matches) { + linkedIssueNumbers.add(Number(match[1])); + } + + return [...linkedIssueNumbers].sort((left, right) => left - right); +} + +function isGitLabMergeRequestUrl(value: string): boolean { + return parseGitLabMergeRequestUrl(value) !== undefined; +} + +function parseGitLabMergeRequestUrl( + value: string +): { repository: string; iid: string } | undefined { + try { + const url = new URL(value); + if (url.hostname !== "gitlab.com") { + return undefined; + } + + const match = url.pathname.match(/^\/(.+)\/-\/merge_requests\/(\d+)\/?$/); + if (!match) { + return undefined; + } + + const repository = match[1]; + const iid = match[2]; + if (!repository || !iid) { + return undefined; + } + + return { + repository: decodeURIComponent(repository), + iid + }; + } catch { + return undefined; + } +} + +function isPlainRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} diff --git a/src/core/notifiers/statusNotifiers.ts b/src/core/notifiers/statusNotifiers.ts index 8d157bc..20b7024 100644 --- a/src/core/notifiers/statusNotifiers.ts +++ b/src/core/notifiers/statusNotifiers.ts @@ -1,4 +1,4 @@ -import type { GitHubPullRequestStatus } from "../github/provider.js"; +import type { IssueTrackerPullRequestStatus } from "../trackers/provider.js"; export type StatusNotifierErrorCode = "invalid_notifier" | "delivery_failed"; @@ -18,7 +18,7 @@ export interface PullRequestStatusNotificationEvent { event_kind: "pull_request_status"; emitted_at: string; repository?: string; - pull_request: GitHubPullRequestStatus; + pull_request: IssueTrackerPullRequestStatus; } export type StatusNotificationEvent = PullRequestStatusNotificationEvent; @@ -35,7 +35,7 @@ export interface StatusNotificationDelivery { } export interface EmitStatusNotificationInput { - pull_request: GitHubPullRequestStatus; + pull_request: IssueTrackerPullRequestStatus; repository?: string; emitted_at?: Date; notifiers: StatusNotifier[]; diff --git a/src/core/trackers/contracts.ts b/src/core/trackers/contracts.ts new file mode 100644 index 0000000..076c2ea --- /dev/null +++ b/src/core/trackers/contracts.ts @@ -0,0 +1,104 @@ +export const ISSUE_TRACKER_PROVIDER_NAMES = ["github", "gitlab"] as const; +export type IssueTrackerProviderName = (typeof ISSUE_TRACKER_PROVIDER_NAMES)[number]; + +export type IssueTrackerRequestKind = "pull_request" | "merge_request"; +export type IssueTrackerPullRequestState = "open" | "closed" | "merged"; +export type IssueTrackerMergeStateStatus = + | "behind" + | "blocked" + | "clean" + | "dirty" + | "draft" + | "has_hooks" + | "unknown" + | "unstable"; +export type IssueTrackerStatusCheckType = "check_run" | "status_context" | "pipeline"; +export type IssueTrackerStatusCheckStatus = "completed" | "in_progress" | "pending" | "queued"; +export type IssueTrackerStatusCheckConclusion = + | "action_required" + | "cancelled" + | "failure" + | "neutral" + | "pending" + | "skipped" + | "success" + | "timed_out" + | "unknown"; +export type IssueTrackerOverallStatus = "failure" | "no_checks" | "pending" | "success"; + +export interface CreatePullRequestInput { + repository: string; + title: string; + body: string; + base_branch: string; + head_branch: string; + linked_issue_numbers?: number[]; + draft?: boolean; +} + +export interface IssueTrackerPullRequestRef { + number: number; + url: string; + head_branch: string; + base_branch: string; + linked_issue_numbers: number[]; +} + +export interface IssueTrackerStatusCheck { + name: string; + type: IssueTrackerStatusCheckType; + status: IssueTrackerStatusCheckStatus; + conclusion: IssueTrackerStatusCheckConclusion; + workflow_name?: string; + details_url?: string; +} + +export interface IssueTrackerPullRequestStatus { + provider: IssueTrackerProviderName; + request_kind: IssueTrackerRequestKind; + number: number; + url: string; + title: string; + state: IssueTrackerPullRequestState; + merge_state_status: IssueTrackerMergeStateStatus; + head_branch: string; + base_branch: string; + linked_issue_numbers: number[]; + overall_status: IssueTrackerOverallStatus; + status_checks: IssueTrackerStatusCheck[]; +} + +export interface GetPullRequestStatusInput { + repository?: string; + pull_request: string; +} + +export interface IssueTrackerProvider { + name: IssueTrackerProviderName; + request_kind: IssueTrackerRequestKind; + isAvailable(): Promise; + createPullRequest(input: CreatePullRequestInput): Promise; + getPullRequestStatus(input: GetPullRequestStatusInput): Promise; +} + +export function deriveOverallStatus( + statusChecks: IssueTrackerStatusCheck[] +): IssueTrackerOverallStatus { + if (statusChecks.length === 0) { + return "no_checks"; + } + + if ( + statusChecks.some((check) => + ["action_required", "cancelled", "failure", "timed_out", "unknown"].includes(check.conclusion) + ) + ) { + return "failure"; + } + + if (statusChecks.some((check) => check.conclusion === "pending" || check.status !== "completed")) { + return "pending"; + } + + return "success"; +} diff --git a/src/core/trackers/provider.ts b/src/core/trackers/provider.ts new file mode 100644 index 0000000..7dd713f --- /dev/null +++ b/src/core/trackers/provider.ts @@ -0,0 +1,103 @@ +import { + createGitHubProvider, + type GitHubProvider +} from "../github/provider.js"; +import { + createGitLabProvider, + type GitLabProvider +} from "../gitlab/provider.js"; +import type { + CreatePullRequestInput, + GetPullRequestStatusInput, + IssueTrackerProvider, + IssueTrackerProviderName, + IssueTrackerPullRequestRef, + IssueTrackerPullRequestStatus +} from "./contracts.js"; + +export type { + CreatePullRequestInput, + GetPullRequestStatusInput, + IssueTrackerProvider, + IssueTrackerProviderName, + IssueTrackerPullRequestRef, + IssueTrackerPullRequestStatus +} from "./contracts.js"; + +export interface CreateIssueTrackerProviderInput { + provider?: IssueTrackerProviderName; + pull_request?: string; + github_provider?: GitHubProvider; + gitlab_provider?: GitLabProvider; + gh_binary?: string; + glab_binary?: string; +} + +export function inferIssueTrackerProviderName( + pullRequestRef: string | undefined +): IssueTrackerProviderName { + if (!pullRequestRef) { + return "github"; + } + + try { + const url = new URL(pullRequestRef); + if (url.hostname === "gitlab.com" && url.pathname.includes("/-/merge_requests/")) { + return "gitlab"; + } + + if (url.hostname === "github.com" && url.pathname.includes("/pull/")) { + return "github"; + } + } catch { + return "github"; + } + + return "github"; +} + +export function createIssueTrackerProvider( + input: CreateIssueTrackerProviderInput = {} +): IssueTrackerProvider { + const providerName = input.provider ?? inferIssueTrackerProviderName(input.pull_request); + + if (providerName === "gitlab") { + return ( + input.gitlab_provider ?? + createGitLabProvider({ + ...(input.glab_binary ? { glab_binary: input.glab_binary } : {}) + }) + ); + } + + return adaptGitHubProvider( + input.github_provider ?? + createGitHubProvider({ + ...(input.gh_binary ? { gh_binary: input.gh_binary } : {}) + }) + ); +} + +export function adaptGitHubProvider(provider: GitHubProvider): IssueTrackerProvider { + return { + name: "github", + request_kind: "pull_request", + async isAvailable() { + return provider.isAvailable(); + }, + async createPullRequest(input: CreatePullRequestInput): Promise { + return provider.createPullRequest(input); + }, + async getPullRequestStatus( + input: GetPullRequestStatusInput + ): Promise { + const result = await provider.getPullRequestStatus(input); + + return { + provider: "github", + request_kind: "pull_request", + ...result + }; + } + }; +} diff --git a/src/demo/goldenDemo.ts b/src/demo/goldenDemo.ts index 0028ef0..109cd46 100644 --- a/src/demo/goldenDemo.ts +++ b/src/demo/goldenDemo.ts @@ -554,6 +554,8 @@ async function captureSuccessfulCli( function buildSimulatedStatusResult(): StatusResult { return { pull_request: { + provider: "github", + request_kind: "pull_request", number: 38, url: "https://github.com/iKwesi/SpecForge/pull/38", title: "demo: golden workflow baseline", diff --git a/tests/cli/status-command.test.ts b/tests/cli/status-command.test.ts index 8bccd8d..0a5a4c6 100644 --- a/tests/cli/status-command.test.ts +++ b/tests/cli/status-command.test.ts @@ -6,6 +6,8 @@ import type { StatusResult } from "../../src/core/diagnostics/status.js"; function buildStatusResult(): StatusResult { return { pull_request: { + provider: "github", + request_kind: "pull_request", number: 123, url: "https://github.com/iKwesi/SpecForge/pull/123", title: "feat: implement task flow", @@ -105,4 +107,56 @@ describe("sf status command", () => { expect(exitCode).toBe(1); expect(stderr).toContain("status failed"); }); + + it("passes the requested issue tracker provider through to the status runner", async () => { + let receivedInput: + | { provider?: string; repository?: string; pull_request: string } + | undefined; + + const exitCode = await runCli( + [ + "node", + "sf", + "status", + "--provider", + "gitlab", + "--repo", + "gitlab-org/cli", + "--pr", + "42" + ], + { + status_runner: async (input) => { + receivedInput = { + pull_request: input.pull_request, + ...(input.provider ? { provider: input.provider } : {}), + ...(input.repository ? { repository: input.repository } : {}) + }; + return { + pull_request: { + provider: "gitlab", + request_kind: "merge_request", + number: 42, + url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + title: "feat: implement task flow", + state: "open", + merge_state_status: "clean", + head_branch: "feat/task-1", + base_branch: "main", + linked_issue_numbers: [], + overall_status: "success", + status_checks: [] + } + }; + } + } + ); + + expect(exitCode).toBe(0); + expect(receivedInput).toEqual({ + provider: "gitlab", + repository: "gitlab-org/cli", + pull_request: "42" + }); + }); }); diff --git a/tests/diagnostics/status.test.ts b/tests/diagnostics/status.test.ts index 660cee6..59e80b8 100644 --- a/tests/diagnostics/status.test.ts +++ b/tests/diagnostics/status.test.ts @@ -17,6 +17,8 @@ describe("runStatus", () => { emitted_at: "2026-03-16T00:10:00.000Z", repository: "iKwesi/SpecForge", pull_request: { + provider: "github", + request_kind: "pull_request", number: 123, url: "https://github.com/iKwesi/SpecForge/pull/123", title: "feat: implement task flow", @@ -49,7 +51,9 @@ describe("runStatus", () => { } } ], - github_provider: { + issue_tracker_provider: { + name: "github", + request_kind: "pull_request", async isAvailable() { return true; }, @@ -58,6 +62,8 @@ describe("runStatus", () => { }, async getPullRequestStatus() { return { + provider: "github", + request_kind: "pull_request", number: 123, url: "https://github.com/iKwesi/SpecForge/pull/123", title: "feat: implement task flow", @@ -102,6 +108,8 @@ describe("runStatus", () => { const report = formatStatusReport(result); expect(report).toContain("SpecForge Status"); + expect(report).toContain("Provider: github"); + expect(report).toContain("Request Kind: pull_request"); expect(report).toContain("Pull Request: #123"); expect(report).toContain("Overall Status: failure"); expect(report).toContain("Linked Issues: #40"); @@ -122,7 +130,9 @@ describe("runStatus", () => { } } ], - github_provider: { + issue_tracker_provider: { + name: "github", + request_kind: "pull_request", async isAvailable() { return true; }, @@ -131,6 +141,8 @@ describe("runStatus", () => { }, async getPullRequestStatus() { return { + provider: "github", + request_kind: "pull_request", number: 123, url: "https://github.com/iKwesi/SpecForge/pull/123", title: "feat: implement task flow", @@ -154,4 +166,54 @@ describe("runStatus", () => { } ]); }); + + it("supports non-github issue tracker providers behind the shared status contract", async () => { + const result = await runStatus({ + provider: "gitlab", + repository: "gitlab-org/cli", + pull_request: "42", + issue_tracker_provider: { + name: "gitlab", + request_kind: "merge_request", + async isAvailable() { + return true; + }, + async createPullRequest() { + throw new Error("not used"); + }, + async getPullRequestStatus() { + return { + provider: "gitlab", + request_kind: "merge_request", + number: 42, + url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + title: "feat: implement task flow", + state: "open", + merge_state_status: "clean", + head_branch: "feat/task-1", + base_branch: "main", + linked_issue_numbers: [40], + overall_status: "success", + status_checks: [ + { + name: "head_pipeline", + type: "pipeline", + status: "completed", + conclusion: "success", + details_url: "https://gitlab.com/gitlab-org/cli/-/pipelines/100" + } + ] + }; + } + } + }); + + expect(result.pull_request.provider).toBe("gitlab"); + expect(result.pull_request.request_kind).toBe("merge_request"); + + const report = formatStatusReport(result); + expect(report).toContain("Provider: gitlab"); + expect(report).toContain("Request Kind: merge_request"); + expect(report).toContain("- head_pipeline [pipeline] completed/success"); + }); }); diff --git a/tests/gitlab/provider.test.ts b/tests/gitlab/provider.test.ts new file mode 100644 index 0000000..bc4c78d --- /dev/null +++ b/tests/gitlab/provider.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, it } from "vitest"; + +import { + GitLabProviderError, + createGitLabProvider +} from "../../src/core/gitlab/provider.js"; + +describe("gitlab provider createPullRequest", () => { + it("creates a merge request behind the common provider contract and preserves linked issues", async () => { + const calls: string[][] = []; + const provider = createGitLabProvider({ + exec: async (args) => { + calls.push(args); + + return { + stdout: JSON.stringify({ + iid: 42, + web_url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + source_branch: "feat/task-1", + target_branch: "main", + description: "## Summary\n- complete the task\n\nCloses #40\nCloses #41" + }), + stderr: "" + }; + } + }); + + const result = await provider.createPullRequest({ + repository: "gitlab-org/cli", + title: "feat: implement task flow", + body: "## Summary\n- complete the task", + base_branch: "main", + head_branch: "feat/task-1", + linked_issue_numbers: [40, 41], + draft: true + }); + + expect(result).toEqual({ + number: 42, + url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + head_branch: "feat/task-1", + base_branch: "main", + linked_issue_numbers: [40, 41] + }); + expect(calls).toEqual([ + [ + "api", + "projects/gitlab-org%2Fcli/merge_requests", + "--method", + "POST", + "--field", + "source_branch=feat/task-1", + "--field", + "target_branch=main", + "--field", + "title=Draft: feat: implement task flow", + "--field", + "description=## Summary\n- complete the task\n\nCloses #40\nCloses #41" + ] + ]); + }); + + it("fails with a typed error when repository format is invalid", async () => { + const provider = createGitLabProvider({ + exec: async () => ({ stdout: "", stderr: "" }) + }); + + await expect( + provider.createPullRequest({ + repository: "bad", + title: "feat: implement task flow", + body: "summary", + base_branch: "main", + head_branch: "feat/task-1" + }) + ).rejects.toEqual( + expect.objectContaining>({ + code: "invalid_repository" + }) + ); + }); +}); + +describe("gitlab provider getPullRequestStatus", () => { + it("maps GitLab merge request state into the common status contract", async () => { + const calls: string[][] = []; + const provider = createGitLabProvider({ + exec: async (args) => { + calls.push(args); + return { + stdout: JSON.stringify({ + iid: 42, + web_url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + title: "feat: implement task flow", + state: "opened", + draft: false, + detailed_merge_status: "mergeable", + source_branch: "feat/task-1", + target_branch: "main", + description: "Implements the task flow.\n\nCloses #40", + head_pipeline: { + status: "running", + web_url: "https://gitlab.com/gitlab-org/cli/-/pipelines/100" + } + }), + stderr: "" + }; + } + }); + + const result = await provider.getPullRequestStatus({ + pull_request: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42" + }); + + expect(calls).toEqual([["api", "projects/gitlab-org%2Fcli/merge_requests/42"]]); + expect(result).toEqual({ + provider: "gitlab", + request_kind: "merge_request", + number: 42, + url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + title: "feat: implement task flow", + state: "open", + merge_state_status: "clean", + head_branch: "feat/task-1", + base_branch: "main", + linked_issue_numbers: [40], + overall_status: "pending", + status_checks: [ + { + name: "head_pipeline", + type: "pipeline", + status: "in_progress", + conclusion: "pending", + details_url: "https://gitlab.com/gitlab-org/cli/-/pipelines/100" + } + ] + }); + }); +}); diff --git a/tests/notifications/status-notifiers.test.ts b/tests/notifications/status-notifiers.test.ts index f4a6f9f..cd8266a 100644 --- a/tests/notifications/status-notifiers.test.ts +++ b/tests/notifications/status-notifiers.test.ts @@ -5,7 +5,7 @@ import { createWebhookStatusNotifier, emitStatusNotification } from "../../src/core/notifiers/statusNotifiers.js"; -import type { GitHubPullRequestStatus } from "../../src/core/github/provider.js"; +import type { IssueTrackerPullRequestStatus } from "../../src/core/trackers/provider.js"; describe("status notifier adapters", () => { it("posts pull request status events to a webhook adapter", async () => { @@ -202,8 +202,10 @@ describe("status notifier adapters", () => { }); }); -function buildPullRequestStatus(): GitHubPullRequestStatus { +function buildPullRequestStatus(): IssueTrackerPullRequestStatus { return { + provider: "github", + request_kind: "pull_request", number: 123, url: "https://github.com/iKwesi/SpecForge/pull/123", title: "feat: implement task flow", diff --git a/tests/trackers/provider.test.ts b/tests/trackers/provider.test.ts new file mode 100644 index 0000000..f8e848f --- /dev/null +++ b/tests/trackers/provider.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest"; + +import { + createIssueTrackerProvider, + inferIssueTrackerProviderName, + type IssueTrackerProvider +} from "../../src/core/trackers/provider.js"; + +describe("issue tracker provider resolution", () => { + it("defaults to github for non-URL pull request references", () => { + expect(inferIssueTrackerProviderName("123")).toBe("github"); + }); + + it("infers gitlab from merge request URLs", () => { + expect( + inferIssueTrackerProviderName("https://gitlab.com/gitlab-org/cli/-/merge_requests/42") + ).toBe("gitlab"); + }); + + it("returns the injected gitlab provider when explicitly requested", () => { + const gitlabProvider: IssueTrackerProvider = { + name: "gitlab", + request_kind: "merge_request", + async isAvailable() { + return true; + }, + async createPullRequest() { + throw new Error("not used"); + }, + async getPullRequestStatus() { + throw new Error("not used"); + } + }; + + const resolved = createIssueTrackerProvider({ + provider: "gitlab", + gitlab_provider: gitlabProvider + }); + + expect(resolved).toBe(gitlabProvider); + }); +}); From 1ad9ef8d100a545f81aa16e89db650eeb234701b Mon Sep 17 00:00:00 2001 From: iKwesi Date: Mon, 16 Mar 2026 02:09:32 -0400 Subject: [PATCH 2/5] fix: harden gitlab tracker support --- src/cli.ts | 15 +++++-- src/core/gitlab/provider.ts | 37 +++++++++-------- src/core/trackers/provider.ts | 17 ++++++-- tests/gitlab/provider.test.ts | 70 +++++++++++++++++++++++++++++++++ tests/trackers/provider.test.ts | 6 +++ 5 files changed, 121 insertions(+), 24 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index e6c4792..5f485ed 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -26,7 +26,10 @@ import { type StatusResult } from "./core/diagnostics/status.js"; import { createWebhookStatusNotifier } from "./core/notifiers/statusNotifiers.js"; -import { ISSUE_TRACKER_PROVIDER_NAMES } from "./core/trackers/contracts.js"; +import { + ISSUE_TRACKER_PROVIDER_NAMES, + type IssueTrackerProviderName +} from "./core/trackers/contracts.js"; interface CliWriter { write(chunk: string): boolean | void; @@ -349,16 +352,20 @@ function collectOptionValues(value: string, previous: string[]): string[] { function normalizeIssueTrackerProvider( value: string | undefined -): "github" | "gitlab" | undefined { +): IssueTrackerProviderName | undefined { if (value === undefined) { return undefined; } - if (ISSUE_TRACKER_PROVIDER_NAMES.includes(value as "github" | "gitlab")) { - return value as "github" | "gitlab"; + if (isIssueTrackerProviderName(value)) { + return value; } throw new Error( `provider must be one of ${ISSUE_TRACKER_PROVIDER_NAMES.join(", ")}.` ); } + +function isIssueTrackerProviderName(value: string): value is IssueTrackerProviderName { + return ISSUE_TRACKER_PROVIDER_NAMES.includes(value as IssueTrackerProviderName); +} diff --git a/src/core/gitlab/provider.ts b/src/core/gitlab/provider.ts index 86642ee..0024c8c 100644 --- a/src/core/gitlab/provider.ts +++ b/src/core/gitlab/provider.ts @@ -90,16 +90,17 @@ export function createGitLabProvider(input: { url: readString(parsed.web_url, "merge request url"), head_branch: readString(parsed.source_branch, "head branch"), base_branch: readString(parsed.target_branch, "base branch"), - linked_issue_numbers: extractLinkedIssueNumbers(readOptionalString(parsed.description) ?? description) + linked_issue_numbers: extractLinkedIssueNumbers( + normalizeOptionalBody(parsed.description) ?? description + ) }; }, async getPullRequestStatus( input: GetPullRequestStatusInput ): Promise { - const repository = input.repository ?? parseGitLabMergeRequestUrl(input.pull_request)?.repository; - const mergeRequestIid = isGitLabMergeRequestUrl(input.pull_request) - ? parseGitLabMergeRequestUrl(input.pull_request)?.iid - : input.pull_request; + const parsedMergeRequestUrl = parseGitLabMergeRequestUrl(input.pull_request); + const repository = input.repository ?? parsedMergeRequestUrl?.repository; + const mergeRequestIid = parsedMergeRequestUrl?.iid ?? input.pull_request; if (!repository) { throw new GitLabProviderError( @@ -175,7 +176,7 @@ async function runGlabCommand(exec: GitLabExec, args: string[]): Promise segment.trim().length === 0)) { throw new GitLabProviderError( @@ -185,9 +186,13 @@ function ensureGitLabProjectPath(value: string): void { } } -function ensureNonEmpty(value: string | undefined, field: string): asserts value is string { +function ensureNonEmpty( + value: string | undefined, + field: string, + code: GitLabProviderErrorCode = "invalid_pull_request" +): asserts value is string { if (typeof value !== "string" || value.trim().length === 0) { - throw new GitLabProviderError("invalid_pull_request", `${field} must be non-empty.`); + throw new GitLabProviderError(code, `${field} must be non-empty.`); } } @@ -344,19 +349,11 @@ function extractLinkedIssueNumbers(body: string): number[] { return [...linkedIssueNumbers].sort((left, right) => left - right); } -function isGitLabMergeRequestUrl(value: string): boolean { - return parseGitLabMergeRequestUrl(value) !== undefined; -} - function parseGitLabMergeRequestUrl( value: string ): { repository: string; iid: string } | undefined { try { const url = new URL(value); - if (url.hostname !== "gitlab.com") { - return undefined; - } - const match = url.pathname.match(/^\/(.+)\/-\/merge_requests\/(\d+)\/?$/); if (!match) { return undefined; @@ -377,6 +374,14 @@ function parseGitLabMergeRequestUrl( } } +function normalizeOptionalBody(value: unknown): string | undefined { + if (typeof value !== "string") { + return undefined; + } + + return value.trim().length > 0 ? value : undefined; +} + function isPlainRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } diff --git a/src/core/trackers/provider.ts b/src/core/trackers/provider.ts index 7dd713f..504eb82 100644 --- a/src/core/trackers/provider.ts +++ b/src/core/trackers/provider.ts @@ -40,12 +40,12 @@ export function inferIssueTrackerProviderName( return "github"; } + if (isGitLabMergeRequestUrl(pullRequestRef)) { + return "gitlab"; + } + try { const url = new URL(pullRequestRef); - if (url.hostname === "gitlab.com" && url.pathname.includes("/-/merge_requests/")) { - return "gitlab"; - } - if (url.hostname === "github.com" && url.pathname.includes("/pull/")) { return "github"; } @@ -101,3 +101,12 @@ export function adaptGitHubProvider(provider: GitHubProvider): IssueTrackerProvi } }; } + +function isGitLabMergeRequestUrl(value: string): boolean { + try { + const url = new URL(value); + return /\/-\/merge_requests\/\d+\/?$/.test(url.pathname); + } catch { + return false; + } +} diff --git a/tests/gitlab/provider.test.ts b/tests/gitlab/provider.test.ts index bc4c78d..05e1256 100644 --- a/tests/gitlab/provider.test.ts +++ b/tests/gitlab/provider.test.ts @@ -60,6 +60,32 @@ describe("gitlab provider createPullRequest", () => { ]); }); + it("falls back to the generated description when GitLab returns a blank description", async () => { + const provider = createGitLabProvider({ + exec: async () => ({ + stdout: JSON.stringify({ + iid: 42, + web_url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + source_branch: "feat/task-1", + target_branch: "main", + description: " " + }), + stderr: "" + }) + }); + + const result = await provider.createPullRequest({ + repository: "gitlab-org/cli", + title: "feat: implement task flow", + body: "## Summary\n- complete the task", + base_branch: "main", + head_branch: "feat/task-1", + linked_issue_numbers: [40] + }); + + expect(result.linked_issue_numbers).toEqual([40]); + }); + it("fails with a typed error when repository format is invalid", async () => { const provider = createGitLabProvider({ exec: async () => ({ stdout: "", stderr: "" }) @@ -136,4 +162,48 @@ describe("gitlab provider getPullRequestStatus", () => { ] }); }); + + it("accepts self-managed GitLab merge request URLs", async () => { + const calls: string[][] = []; + const provider = createGitLabProvider({ + exec: async (args) => { + calls.push(args); + return { + stdout: JSON.stringify({ + iid: 42, + web_url: "https://gitlab.example.com/platform/specforge/-/merge_requests/42", + title: "feat: implement task flow", + state: "opened", + draft: false, + detailed_merge_status: "mergeable", + source_branch: "feat/task-1", + target_branch: "main", + description: "", + head_pipeline: { + status: "success", + web_url: "https://gitlab.example.com/platform/specforge/-/pipelines/100" + } + }), + stderr: "" + }; + } + }); + + const result = await provider.getPullRequestStatus({ + pull_request: "https://gitlab.example.com/platform/specforge/-/merge_requests/42" + }); + + expect(calls).toEqual([["api", "projects/platform%2Fspecforge/merge_requests/42"]]); + expect(result.provider).toBe("gitlab"); + expect(result.number).toBe(42); + expect(result.status_checks).toEqual([ + { + name: "head_pipeline", + type: "pipeline", + status: "completed", + conclusion: "success", + details_url: "https://gitlab.example.com/platform/specforge/-/pipelines/100" + } + ]); + }); }); diff --git a/tests/trackers/provider.test.ts b/tests/trackers/provider.test.ts index f8e848f..c8d0daa 100644 --- a/tests/trackers/provider.test.ts +++ b/tests/trackers/provider.test.ts @@ -17,6 +17,12 @@ describe("issue tracker provider resolution", () => { ).toBe("gitlab"); }); + it("infers gitlab from self-managed merge request URLs", () => { + expect( + inferIssueTrackerProviderName("https://gitlab.example.com/platform/specforge/-/merge_requests/42") + ).toBe("gitlab"); + }); + it("returns the injected gitlab provider when explicitly requested", () => { const gitlabProvider: IssueTrackerProvider = { name: "gitlab", From 6455e3b3d0c0365840eca810ebb975f377664b7b Mon Sep 17 00:00:00 2001 From: iKwesi Date: Mon, 16 Mar 2026 02:49:59 -0400 Subject: [PATCH 3/5] fix: tighten tracker status contracts --- src/cli.ts | 2 +- src/core/diagnostics/status.ts | 4 +++- src/core/github/provider.ts | 23 +---------------------- src/core/gitlab/provider.ts | 2 +- tests/diagnostics/status.test.ts | 1 + tests/gitlab/provider.test.ts | 30 ++++++++++++++++++++++++++++++ 6 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 5f485ed..4383201 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -197,7 +197,7 @@ The command reads artifact inputs and optional policy/schedule context, then pri program .command("status") .description("Report issue-tracker review request state and CI outcomes. Example: specforge status --repo iKwesi/SpecForge --pr 123") - .requiredOption("--pr ", "Pull request or merge request number, URL, or branch to inspect") + .requiredOption("--pr ", "Pull request or merge request number or URL to inspect") .option( "--repo ", "Issue tracker repository/project path when --pr is not a review request URL" diff --git a/src/core/diagnostics/status.ts b/src/core/diagnostics/status.ts index 1efdfb2..717bad8 100644 --- a/src/core/diagnostics/status.ts +++ b/src/core/diagnostics/status.ts @@ -57,12 +57,14 @@ export async function runStatus(input: RunStatusInput): Promise { } export function formatStatusReport(result: StatusResult): string { + const requestLabel = + result.pull_request.request_kind === "merge_request" ? "Merge Request" : "Pull Request"; const lines = [ "SpecForge Status", "", `Provider: ${result.pull_request.provider}`, `Request Kind: ${result.pull_request.request_kind}`, - `Pull Request: #${result.pull_request.number}`, + `${requestLabel}: #${result.pull_request.number}`, `URL: ${result.pull_request.url}`, `Title: ${result.pull_request.title}`, `State: ${result.pull_request.state}`, diff --git a/src/core/github/provider.ts b/src/core/github/provider.ts index d732d8f..33fb398 100644 --- a/src/core/github/provider.ts +++ b/src/core/github/provider.ts @@ -1,5 +1,6 @@ import { execFile } from "node:child_process"; import { promisify } from "node:util"; +import { deriveOverallStatus } from "../trackers/contracts.js"; const execFileAsync = promisify(execFile); const PR_VIEW_FIELDS = [ @@ -478,28 +479,6 @@ function normalizeStatusContextConclusion(value: string | undefined): GitHubStat } } -function deriveOverallStatus(statusChecks: GitHubStatusCheck[]): GitHubOverallStatus { - if (statusChecks.length === 0) { - return "no_checks"; - } - - if ( - statusChecks.some((check) => - ["failure", "timed_out", "cancelled", "action_required", "unknown"].includes( - check.conclusion - ) - ) - ) { - return "failure"; - } - - if (statusChecks.some((check) => check.conclusion === "pending" || check.status !== "completed")) { - return "pending"; - } - - return "success"; -} - function isPullRequestUrl(value: string): boolean { return /^https:\/\/github\.com\/[^/\s]+\/[^/\s]+\/pull\/\d+/.test(value.trim()); } diff --git a/src/core/gitlab/provider.ts b/src/core/gitlab/provider.ts index 0024c8c..2ad6a20 100644 --- a/src/core/gitlab/provider.ts +++ b/src/core/gitlab/provider.ts @@ -225,7 +225,7 @@ function readOptionalBoolean(value: unknown): boolean | undefined { } function readNumber(value: unknown, field: string): number { - if (typeof value !== "number" || !Number.isFinite(value)) { + if (typeof value !== "number" || !Number.isInteger(value) || value <= 0) { throw new GitLabProviderError("parse_failed", `GitLab CLI returned invalid ${field}.`); } diff --git a/tests/diagnostics/status.test.ts b/tests/diagnostics/status.test.ts index 59e80b8..9996e90 100644 --- a/tests/diagnostics/status.test.ts +++ b/tests/diagnostics/status.test.ts @@ -214,6 +214,7 @@ describe("runStatus", () => { const report = formatStatusReport(result); expect(report).toContain("Provider: gitlab"); expect(report).toContain("Request Kind: merge_request"); + expect(report).toContain("Merge Request: #42"); expect(report).toContain("- head_pipeline [pipeline] completed/success"); }); }); diff --git a/tests/gitlab/provider.test.ts b/tests/gitlab/provider.test.ts index 05e1256..0d252fd 100644 --- a/tests/gitlab/provider.test.ts +++ b/tests/gitlab/provider.test.ts @@ -206,4 +206,34 @@ describe("gitlab provider getPullRequestStatus", () => { } ]); }); + + it("rejects non-positive or non-integer merge request iids from GitLab output", async () => { + const provider = createGitLabProvider({ + exec: async () => ({ + stdout: JSON.stringify({ + iid: 0, + web_url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + title: "feat: implement task flow", + state: "opened", + draft: false, + detailed_merge_status: "mergeable", + source_branch: "feat/task-1", + target_branch: "main", + description: "", + head_pipeline: {} + }), + stderr: "" + }) + }); + + await expect( + provider.getPullRequestStatus({ + pull_request: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42" + }) + ).rejects.toEqual( + expect.objectContaining>({ + code: "parse_failed" + }) + ); + }); }); From 525688817933ab888583f996b88db4cf436f7f6f Mon Sep 17 00:00:00 2001 From: iKwesi Date: Mon, 16 Mar 2026 03:06:33 -0400 Subject: [PATCH 4/5] fix: clarify tracker ref handling --- src/cli.ts | 8 +++++++- src/core/gitlab/provider.ts | 15 +++++++++++++-- src/core/trackers/provider.ts | 9 --------- tests/cli/help-output.test.ts | 3 +++ tests/gitlab/provider.test.ts | 19 +++++++++++++++++++ 5 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 4383201..591f449 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -197,7 +197,10 @@ The command reads artifact inputs and optional policy/schedule context, then pri program .command("status") .description("Report issue-tracker review request state and CI outcomes. Example: specforge status --repo iKwesi/SpecForge --pr 123") - .requiredOption("--pr ", "Pull request or merge request number or URL to inspect") + .requiredOption( + "--pr ", + "GitHub: pull request number, URL, or branch. GitLab: merge request number or URL." + ) .option( "--repo ", "Issue tracker repository/project path when --pr is not a review request URL" @@ -218,10 +221,13 @@ The command reads artifact inputs and optional policy/schedule context, then pri Examples: $ specforge status --repo iKwesi/SpecForge --pr 123 $ specforge status --pr https://github.com/iKwesi/SpecForge/pull/123 + $ specforge status --repo iKwesi/SpecForge --pr feat/task-1 $ specforge status --provider gitlab --repo gitlab-org/cli --pr 42 $ specforge status --repo iKwesi/SpecForge --pr 123 --notify-webhook https://hooks.example.test/specforge Use this after handoff when you need the latest review-request merge state and status checks. +GitHub accepts pull request numbers, URLs, or branch refs when --repo is provided. +GitLab accepts merge request numbers or merge request URLs. ` ) .action(async (options: { pr: string; repo?: string; provider?: string; notifyWebhook: string[] }) => { diff --git a/src/core/gitlab/provider.ts b/src/core/gitlab/provider.ts index 2ad6a20..1b68cdd 100644 --- a/src/core/gitlab/provider.ts +++ b/src/core/gitlab/provider.ts @@ -100,7 +100,7 @@ export function createGitLabProvider(input: { ): Promise { const parsedMergeRequestUrl = parseGitLabMergeRequestUrl(input.pull_request); const repository = input.repository ?? parsedMergeRequestUrl?.repository; - const mergeRequestIid = parsedMergeRequestUrl?.iid ?? input.pull_request; + const mergeRequestIid = normalizeMergeRequestIid(parsedMergeRequestUrl?.iid ?? input.pull_request); if (!repository) { throw new GitLabProviderError( @@ -110,7 +110,6 @@ export function createGitLabProvider(input: { } ensureGitLabProjectPath(repository); - ensureNonEmpty(mergeRequestIid, "pull_request"); const viewed = await runGlabCommand(exec, [ "api", @@ -382,6 +381,18 @@ function normalizeOptionalBody(value: unknown): string | undefined { return value.trim().length > 0 ? value : undefined; } +function normalizeMergeRequestIid(value: string): string { + const trimmed = value.trim(); + if (!/^[1-9]\d*$/.test(trimmed)) { + throw new GitLabProviderError( + "invalid_pull_request", + "pull_request must be a positive merge request number or a GitLab merge request URL." + ); + } + + return trimmed; +} + function isPlainRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } diff --git a/src/core/trackers/provider.ts b/src/core/trackers/provider.ts index 504eb82..48380af 100644 --- a/src/core/trackers/provider.ts +++ b/src/core/trackers/provider.ts @@ -44,15 +44,6 @@ export function inferIssueTrackerProviderName( return "gitlab"; } - try { - const url = new URL(pullRequestRef); - if (url.hostname === "github.com" && url.pathname.includes("/pull/")) { - return "github"; - } - } catch { - return "github"; - } - return "github"; } diff --git a/tests/cli/help-output.test.ts b/tests/cli/help-output.test.ts index 20c699d..01d5ee8 100644 --- a/tests/cli/help-output.test.ts +++ b/tests/cli/help-output.test.ts @@ -17,10 +17,13 @@ describe("specforge help output", () => { const program = createProgram(); const inspectHelp = program.commands.find((command) => command.name() === "inspect")?.helpInformation(); const explainHelp = program.commands.find((command) => command.name() === "explain")?.helpInformation(); + const statusHelp = program.commands.find((command) => command.name() === "status")?.helpInformation(); expect(inspectHelp).toContain("in a .specforge subdirectory"); expect(inspectHelp).toContain("--repository-root . --artifact-dir ."); expect(explainHelp).toContain("artifact lineage"); expect(explainHelp).toContain("Example: specforge explain --artifact-file .specforge/task-results/TASK-1.json"); + expect(statusHelp).toContain("GitHub: pull request number, URL, or branch"); + expect(statusHelp).toContain("merge request number or URL"); }); }); diff --git a/tests/gitlab/provider.test.ts b/tests/gitlab/provider.test.ts index 0d252fd..dabcd96 100644 --- a/tests/gitlab/provider.test.ts +++ b/tests/gitlab/provider.test.ts @@ -236,4 +236,23 @@ describe("gitlab provider getPullRequestStatus", () => { }) ); }); + + it("fails early when a non-url gitlab pull_request ref is not a positive merge request number", async () => { + const provider = createGitLabProvider({ + exec: async () => { + throw new Error("should not reach glab"); + } + }); + + await expect( + provider.getPullRequestStatus({ + repository: "gitlab-org/cli", + pull_request: "feat/task-1" + }) + ).rejects.toEqual( + expect.objectContaining>({ + code: "invalid_pull_request" + }) + ); + }); }); From 831fec2963e4fc3ab09be80e737c71b7164b31c8 Mon Sep 17 00:00:00 2001 From: iKwesi Date: Mon, 16 Mar 2026 03:40:27 -0400 Subject: [PATCH 5/5] fix: harden gitlab merge request parsing --- src/core/gitlab/provider.ts | 12 +++++-- src/core/trackers/provider.ts | 4 ++- tests/gitlab/provider.test.ts | 62 +++++++++++++++++++++++++++++++++ tests/trackers/provider.test.ts | 8 +++++ 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/core/gitlab/provider.ts b/src/core/gitlab/provider.ts index 1b68cdd..67ba942 100644 --- a/src/core/gitlab/provider.ts +++ b/src/core/gitlab/provider.ts @@ -16,6 +16,8 @@ import { const execFileAsync = promisify(execFile); +const GITLAB_MERGE_REQUEST_PATH_PATTERN = /^\/(.+)\/-\/merge_requests\/(\d+)(?:\/.*)?$/; + export type GitLabProviderErrorCode = | "provider_unavailable" | "invalid_repository" @@ -283,6 +285,10 @@ function readStatusChecks(value: unknown): IssueTrackerStatusCheck[] { } const pipelineStatus = normalizePipelineStatus(readOptionalString(value.status)); + if (!pipelineStatus) { + return []; + } + const detailsUrl = readOptionalString(value.web_url); return [ { @@ -298,7 +304,7 @@ function readStatusChecks(value: unknown): IssueTrackerStatusCheck[] { function normalizePipelineStatus(value: string | undefined): { status: IssueTrackerStatusCheckStatus; conclusion: IssueTrackerStatusCheckConclusion; -} { +} | undefined { switch ((value ?? "").toLowerCase()) { case "success": return { status: "completed", conclusion: "success" }; @@ -321,7 +327,7 @@ function normalizePipelineStatus(value: string | undefined): { case "scheduled": return { status: "queued", conclusion: "pending" }; default: - return { status: "completed", conclusion: "unknown" }; + return undefined; } } @@ -353,7 +359,7 @@ function parseGitLabMergeRequestUrl( ): { repository: string; iid: string } | undefined { try { const url = new URL(value); - const match = url.pathname.match(/^\/(.+)\/-\/merge_requests\/(\d+)\/?$/); + const match = url.pathname.match(GITLAB_MERGE_REQUEST_PATH_PATTERN); if (!match) { return undefined; } diff --git a/src/core/trackers/provider.ts b/src/core/trackers/provider.ts index 48380af..63a6266 100644 --- a/src/core/trackers/provider.ts +++ b/src/core/trackers/provider.ts @@ -15,6 +15,8 @@ import type { IssueTrackerPullRequestStatus } from "./contracts.js"; +const GITLAB_MERGE_REQUEST_PATH_PATTERN = /\/-\/merge_requests\/\d+(?:\/.*)?$/; + export type { CreatePullRequestInput, GetPullRequestStatusInput, @@ -96,7 +98,7 @@ export function adaptGitHubProvider(provider: GitHubProvider): IssueTrackerProvi function isGitLabMergeRequestUrl(value: string): boolean { try { const url = new URL(value); - return /\/-\/merge_requests\/\d+\/?$/.test(url.pathname); + return GITLAB_MERGE_REQUEST_PATH_PATTERN.test(url.pathname); } catch { return false; } diff --git a/tests/gitlab/provider.test.ts b/tests/gitlab/provider.test.ts index dabcd96..2555f79 100644 --- a/tests/gitlab/provider.test.ts +++ b/tests/gitlab/provider.test.ts @@ -207,6 +207,68 @@ describe("gitlab provider getPullRequestStatus", () => { ]); }); + it("accepts GitLab merge request subpage URLs like diffs", async () => { + const calls: string[][] = []; + const provider = createGitLabProvider({ + exec: async (args) => { + calls.push(args); + return { + stdout: JSON.stringify({ + iid: 42, + web_url: "https://gitlab.example.com/platform/specforge/-/merge_requests/42", + title: "feat: implement task flow", + state: "opened", + draft: false, + detailed_merge_status: "mergeable", + source_branch: "feat/task-1", + target_branch: "main", + description: "", + head_pipeline: { + status: "success", + web_url: "https://gitlab.example.com/platform/specforge/-/pipelines/100" + } + }), + stderr: "" + }; + } + }); + + const result = await provider.getPullRequestStatus({ + pull_request: "https://gitlab.example.com/platform/specforge/-/merge_requests/42/diffs" + }); + + expect(calls).toEqual([["api", "projects/platform%2Fspecforge/merge_requests/42"]]); + expect(result.provider).toBe("gitlab"); + expect(result.number).toBe(42); + }); + + it("omits pipeline checks when GitLab does not return a recognized pipeline status", async () => { + const provider = createGitLabProvider({ + exec: async () => ({ + stdout: JSON.stringify({ + iid: 42, + web_url: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42", + title: "feat: implement task flow", + state: "opened", + draft: false, + detailed_merge_status: "mergeable", + source_branch: "feat/task-1", + target_branch: "main", + description: "", + head_pipeline: {} + }), + stderr: "" + }) + }); + + const result = await provider.getPullRequestStatus({ + pull_request: "https://gitlab.com/gitlab-org/cli/-/merge_requests/42" + }); + + expect(result.overall_status).toBe("no_checks"); + expect(result.status_checks).toEqual([]); + }); + it("rejects non-positive or non-integer merge request iids from GitLab output", async () => { const provider = createGitLabProvider({ exec: async () => ({ diff --git a/tests/trackers/provider.test.ts b/tests/trackers/provider.test.ts index c8d0daa..27e071e 100644 --- a/tests/trackers/provider.test.ts +++ b/tests/trackers/provider.test.ts @@ -23,6 +23,14 @@ describe("issue tracker provider resolution", () => { ).toBe("gitlab"); }); + it("infers gitlab from merge request subpages like diffs", () => { + expect( + inferIssueTrackerProviderName( + "https://gitlab.example.com/platform/specforge/-/merge_requests/42/diffs" + ) + ).toBe("gitlab"); + }); + it("returns the injected gitlab provider when explicitly requested", () => { const gitlabProvider: IssueTrackerProvider = { name: "gitlab",