From 4bc97e4cde7db93a79c113dbdc826cebb82d2623 Mon Sep 17 00:00:00 2001 From: Baker B Date: Tue, 5 May 2026 18:50:49 -0400 Subject: [PATCH] fix(pr_create): support GitLab nested groups, remove unsupported glab flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in pr_create against GitLab projects with nested groups (#290): 1. The `repo` regex `^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$` rejected group/subgroup/.../repo paths before they reached the platform adapter, breaking pr_create on every nested-group GitLab project (`analogicdev/internal/tools/blueshift/blueshift-docmancer-ui` was the failing reproducer — 4 `/` levels). 2. The post-create verify step shelled to `glab mr view -F json`, but `-F` is not a flag on any glab subcommand in 1.36.0. The MR was being created server-side then the verify step would error, leaving callers to clean up duplicates. Bug 2 was already fixed by #386 (a5cf34c) on the kahuna branch — the post-create lookup now uses `glab api projects//merge_requests ?source_branch=&state=opened` and parses the JSON in-process. This commit adds a regression test for the nested-group path through that adapter. Bug 1 fix: - Add `lib/schemas/repo.ts` as the single source of truth for the slug regex. New pattern: `^[a-zA-Z0-9._-]+(/[a-zA-Z0-9._-]+)+$` — accepts arbitrary `/` depth while still rejecting bare `owner` (no `/`), empty segments (`a//b`), and shell metacharacters. - Update all 19 handlers that ship a `repo` parameter (`pr_*`, `ci_*`, `wave_init`, `wave_reconcile`, `label_*`, `work_item`) to import `repoOptionalSchema` from the shared module instead of rolling their own regex. - Add `lib/schemas/repo.test.ts` covering flat slugs, nested-group slugs (including the exact #290 reproducer), and rejection of malformed input. - Add a nested-group regression test in `pr-create-gitlab.test.ts` asserting the slug forwards verbatim to `-R` and URL-encodes every `/` as `%2F` for the api lookup path. GitHub-side adapters keep their flat `GITHUB_REPO_SLUG` defence- in-depth regex — that's a platform constraint, not a relaxation target. All 2175 unit tests pass; full ci/validate.sh green. Closes #290 --- handlers/ci_failed_jobs.ts | 7 +- handlers/ci_run_logs.ts | 7 +- handlers/ci_run_status.ts | 7 +- handlers/ci_runs_for_branch.ts | 7 +- handlers/ci_wait_run.ts | 7 +- handlers/label_create.ts | 7 +- handlers/label_list.ts | 7 +- handlers/pr_comment.ts | 7 +- handlers/pr_create.ts | 7 +- handlers/pr_diff.ts | 7 +- handlers/pr_files.ts | 7 +- handlers/pr_list.ts | 7 +- handlers/pr_merge.ts | 7 +- handlers/pr_merge_wait.ts | 7 +- handlers/pr_status.ts | 7 +- handlers/pr_wait_ci.ts | 7 +- handlers/wave_init.ts | 4 +- handlers/wave_reconcile.ts | 7 +- handlers/work_item.ts | 7 +- lib/adapters/pr-create-gitlab.test.ts | 38 ++++++++ lib/schemas/repo.test.ts | 119 ++++++++++++++++++++++++++ lib/schemas/repo.ts | 45 ++++++++++ 22 files changed, 259 insertions(+), 73 deletions(-) create mode 100644 lib/schemas/repo.test.ts create mode 100644 lib/schemas/repo.ts diff --git a/handlers/ci_failed_jobs.ts b/handlers/ci_failed_jobs.ts index f899930..7ffabb6 100644 --- a/handlers/ci_failed_jobs.ts +++ b/handlers/ci_failed_jobs.ts @@ -6,14 +6,13 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z .object({ run_id: z.number().int().positive(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be in owner/repo form') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }) .strict(); diff --git a/handlers/ci_run_logs.ts b/handlers/ci_run_logs.ts index a90f735..b9f5b6d 100644 --- a/handlers/ci_run_logs.ts +++ b/handlers/ci_run_logs.ts @@ -10,16 +10,15 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; import { truncateLogs, DEFAULT_MAX_LINES } from '../lib/shared/truncate-logs.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ run_id: z.number().int().nonnegative(), job_id: z.number().int().nonnegative().optional(), failed_only: z.boolean().optional().default(true), max_lines: z.number().int().positive().optional().default(DEFAULT_MAX_LINES), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be in owner/repo form') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/ci_run_status.ts b/handlers/ci_run_status.ts index bc56bad..4e8d915 100644 --- a/handlers/ci_run_status.ts +++ b/handlers/ci_run_status.ts @@ -7,15 +7,14 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z .object({ ref: z.string().min(1, 'ref must be a non-empty string'), workflow_name: z.string().optional(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be in owner/repo form') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }) .strict(); diff --git a/handlers/ci_runs_for_branch.ts b/handlers/ci_runs_for_branch.ts index 4e01db6..c22689d 100644 --- a/handlers/ci_runs_for_branch.ts +++ b/handlers/ci_runs_for_branch.ts @@ -7,15 +7,14 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ branch: z.string().min(1, 'branch must be a non-empty string'), limit: z.number().int().positive().optional().default(10), status: z.enum(['success', 'failure', 'in_progress', 'all']).optional().default('all'), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be in owner/repo form') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/ci_wait_run.ts b/handlers/ci_wait_run.ts index 2de4133..0c7c32d 100644 --- a/handlers/ci_wait_run.ts +++ b/handlers/ci_wait_run.ts @@ -15,6 +15,7 @@ import { defaultSleep, type WaitResult, } from '../lib/ci-wait-run-poll.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z .object({ @@ -22,10 +23,8 @@ const inputSchema = z workflow_name: z.string().optional(), poll_interval_sec: z.number().int().positive().optional(), timeout_sec: z.number().int().positive().optional(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be in owner/repo form') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, expected_sha: z .string() .regex(/^[0-9a-f]{40}$/i, 'expected_sha must be a 40-char hex commit SHA') diff --git a/handlers/label_create.ts b/handlers/label_create.ts index bdd8e05..905cb4c 100644 --- a/handlers/label_create.ts +++ b/handlers/label_create.ts @@ -11,6 +11,7 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; // 6-char hex (no leading #). Both gh and glab accept color in this form at // the adapter boundary; glab's `#` prefix is applied inside the adapter. @@ -23,10 +24,8 @@ const inputSchema = z.object({ .string() .regex(HEX_COLOR_RE, 'color must be a 6-char hex (no leading #)') .optional(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/label_list.ts b/handlers/label_list.ts index a07fef9..ef54980 100644 --- a/handlers/label_list.ts +++ b/handlers/label_list.ts @@ -11,13 +11,12 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ limit: z.number().int().positive().optional().default(100), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/pr_comment.ts b/handlers/pr_comment.ts index 59dcbd9..0081fcf 100644 --- a/handlers/pr_comment.ts +++ b/handlers/pr_comment.ts @@ -6,14 +6,13 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ number: z.number().int().positive('number must be a positive integer'), body: z.string().min(1, 'body must be a non-empty string'), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/pr_create.ts b/handlers/pr_create.ts index 10e67a9..88702ff 100644 --- a/handlers/pr_create.ts +++ b/handlers/pr_create.ts @@ -6,6 +6,7 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ title: z.string().min(1, 'title must be a non-empty string'), @@ -15,10 +16,8 @@ const inputSchema = z.object({ base: z.string().min(1).optional(), head: z.string().optional(), draft: z.boolean().optional().default(false), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/pr_diff.ts b/handlers/pr_diff.ts index e4788f4..4861d7f 100644 --- a/handlers/pr_diff.ts +++ b/handlers/pr_diff.ts @@ -6,13 +6,12 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ number: z.number().int().positive(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/pr_files.ts b/handlers/pr_files.ts index a7425d5..21e109b 100644 --- a/handlers/pr_files.ts +++ b/handlers/pr_files.ts @@ -6,6 +6,7 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; // Re-export `parseDiffStats` so existing importers (e.g. tests/pr_files.test.ts) // keep working. The canonical implementation lives in the GitLab adapter; this @@ -14,10 +15,8 @@ export { parseDiffStats } from '../lib/adapters/pr-files-gitlab.js'; const inputSchema = z.object({ number: z.number().int().positive('number must be a positive integer'), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/pr_list.ts b/handlers/pr_list.ts index 3b16bc8..e2cb1f0 100644 --- a/handlers/pr_list.ts +++ b/handlers/pr_list.ts @@ -6,6 +6,7 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ head: z.string().optional(), @@ -13,10 +14,8 @@ const inputSchema = z.object({ state: z.enum(['open', 'closed', 'merged', 'all']).optional().default('open'), author: z.string().optional(), limit: z.number().int().positive().optional().default(20), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/pr_merge.ts b/handlers/pr_merge.ts index 1a63ba3..d1a6b97 100644 --- a/handlers/pr_merge.ts +++ b/handlers/pr_merge.ts @@ -9,16 +9,15 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ number: z.number().int().positive('number must be a positive integer'), squash_message: z.string().optional(), use_merge_queue: z.boolean().optional(), skip_train: z.boolean().optional(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/pr_merge_wait.ts b/handlers/pr_merge_wait.ts index edd36ec..e596fc3 100644 --- a/handlers/pr_merge_wait.ts +++ b/handlers/pr_merge_wait.ts @@ -13,16 +13,15 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ number: z.number().int().positive('number must be a positive integer'), squash_message: z.string().optional(), use_merge_queue: z.boolean().optional(), skip_train: z.boolean().optional(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, timeout_sec: z .number() .int() diff --git a/handlers/pr_status.ts b/handlers/pr_status.ts index 11401e0..f6c4cb5 100644 --- a/handlers/pr_status.ts +++ b/handlers/pr_status.ts @@ -6,13 +6,12 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ number: z.number().int().positive(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/handlers/pr_wait_ci.ts b/handlers/pr_wait_ci.ts index 2850b8f..29dcbc4 100644 --- a/handlers/pr_wait_ci.ts +++ b/handlers/pr_wait_ci.ts @@ -15,16 +15,15 @@ import { type Deps, } from '../lib/pr-wait-ci-poll.js'; import { snapshotGithub, classifyRollupItem } from '../lib/adapters/pr-wait-ci-github.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z .object({ number: z.number().int().positive(), poll_interval_sec: z.number().int().optional().default(30), timeout_sec: z.number().int().positive().optional().default(1800), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }) .strict(); diff --git a/handlers/wave_init.ts b/handlers/wave_init.ts index 6b135ef..a2d12f8 100644 --- a/handlers/wave_init.ts +++ b/handlers/wave_init.ts @@ -12,12 +12,14 @@ import { extendModePrescan, readPhasesWavesTotals, bootstrapKahunaBranch, branchExistsOnRemote, type PlanData, type StateData, } from '../lib/wave_init_plan.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ plan_json: z.string().min(1, 'plan_json must be a non-empty JSON string'), extend: z.boolean().optional().default(false), project_root: z.string().optional(), - repo: z.string().regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format').optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, kahuna: z.object({ plan_id: z.number().int().positive(), slug: z.string().min(1).regex(/^[a-z0-9][a-z0-9-]*$/, 'slug must be kebab-case (lowercase, digits, hyphens)'), diff --git a/handlers/wave_reconcile.ts b/handlers/wave_reconcile.ts index 3635056..9ae587e 100644 --- a/handlers/wave_reconcile.ts +++ b/handlers/wave_reconcile.ts @@ -26,16 +26,15 @@ import { issueNumberFromBranch, computeDriftSets, hasDrift, renderDriftHaltComment, type PlanData, type StateData, } from '../lib/wave-reconcile.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ wave_id: z.string().optional(), plan_issue_number: z.number().int().positive().optional(), kahuna_branch: z.string().optional(), timestamp: z.string().optional(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, /** * Dry-run: compute drift but skip the `pr_comment` side-effect. Test * convenience + future support for a `/wave-reconcile --dry-run` CLI. diff --git a/handlers/work_item.ts b/handlers/work_item.ts index 65585c3..3cbf829 100644 --- a/handlers/work_item.ts +++ b/handlers/work_item.ts @@ -13,6 +13,7 @@ import { z } from 'zod'; import type { HandlerDef } from '../types.js'; import { getAdapter } from '../lib/adapters/index.js'; +import { repoOptionalSchema } from '../lib/schemas/repo.js'; const inputSchema = z.object({ type: z.enum(['epic', 'story', 'feature', 'bug', 'chore', 'docs', 'fix', 'pr', 'mr']), @@ -22,10 +23,8 @@ const inputSchema = z.object({ head_branch: z.string().optional(), base_branch: z.string().optional(), draft: z.boolean().optional(), - repo: z - .string() - .regex(/^[a-zA-Z0-9._/-]+\/[a-zA-Z0-9._-]+$/, 'repo must be owner/repo format') - .optional(), + // GitLab nested groups need arbitrary `/` depth — see lib/schemas/repo.ts (#290). + repo: repoOptionalSchema, }); function envelope(payload: unknown) { diff --git a/lib/adapters/pr-create-gitlab.test.ts b/lib/adapters/pr-create-gitlab.test.ts index 14f795b..84dc43c 100644 --- a/lib/adapters/pr-create-gitlab.test.ts +++ b/lib/adapters/pr-create-gitlab.test.ts @@ -230,6 +230,44 @@ describe('prCreateGitlab — subprocess boundary', () => { expect(findCall('glab mr create')).toContain('main'); }); + test('nested GitLab group path is forwarded verbatim and URL-encoded for api lookup (#290)', async () => { + // Regression guard: the deep group path + // `analogicdev/internal/tools/blueshift/blueshift-docmancer-ui` + // (4 `/` levels) was the exact case that motivated #290's regex fix. + // The adapter must forward the slug verbatim to `-R` and URL-encode + // every `/` (→ `%2F`) for the api lookup path segment. + const repo = 'analogicdev/internal/tools/blueshift/blueshift-docmancer-ui'; + on('git branch --show-current', 'feature/nested\n'); + on('glab mr create', 'created\n'); + on( + 'merge_requests?source_branch', + JSON.stringify([{ + iid: 42, + web_url: `https://gitlab.com/${repo}/-/merge_requests/42`, + state: 'opened', + source_branch: 'feature/nested', + target_branch: 'main', + }]), + ); + + const result = await prCreateGitlab({ title: 't', body: 'b', base: 'main', repo }); + expectOk(result); + expect(result.data.number).toBe(42); + + // `-R` carries the slug verbatim (glab handles URL-encoding for it). + const create = findCall('glab mr create'); + expect(create).toContain('-R'); + expect(create).toContain(repo); + // The api lookup path replaces every `/` with `%2F` — 4 `/` → + // 4 `%2F` in the encoded slug. + const apiLookup = findCall('merge_requests?source_branch'); + const encoded = repo.replace(/\//g, '%2F'); + expect(apiLookup).toContain(encoded); + // Belt-and-suspenders: count the encoded separators to make sure no + // `/` leaked into the api path (which would split the URL segments). + expect((encoded.match(/%2F/g) ?? []).length).toBe(4); + }); + test('post-create lookup failure surfaces glab_mr_view_failed (regression guard for soft-fallback)', async () => { on('git branch --show-current', 'feature/orphan\n'); on('glab mr create', 'created\n'); diff --git a/lib/schemas/repo.test.ts b/lib/schemas/repo.test.ts new file mode 100644 index 0000000..0c21609 --- /dev/null +++ b/lib/schemas/repo.test.ts @@ -0,0 +1,119 @@ +/** + * Schema-level tests for the shared `repo` parameter regex. + * + * Regression coverage for #290 — GitLab nested groups (`org/sub/.../repo`) + * must validate alongside flat GitHub `owner/repo` slugs. The single-source + * pattern lives in `lib/schemas/repo.ts`; every `pr_*` / `ci_*` / `wave_*` / + * `label_*` / `work_item` handler imports `repoOptionalSchema` from there, + * so a green test here protects all of them at once. + */ +import { describe, test, expect } from 'bun:test'; +import { + REPO_SLUG_REGEX, + REPO_SLUG_ERROR, + repoSchema, + repoOptionalSchema, +} from './repo.ts'; + +describe('REPO_SLUG_REGEX', () => { + describe('accepts flat owner/repo (GitHub-style, regression guard)', () => { + test.each([ + ['Wave-Engineering/mcp-server-sdlc'], + ['org/repo'], + ['org123/repo-name'], + ['org.dot/repo_underscore'], + ['a/b'], + ['a-b/c.d'], + ['o.r.g/r.e.p.o'], + // Hyphens, underscores, dots, digits are all valid in GitHub slugs. + ['my-org/my_repo.v2'], + ])('%s', (slug) => { + expect(REPO_SLUG_REGEX.test(slug)).toBe(true); + }); + }); + + describe('accepts nested GitLab groups (#290 fix)', () => { + test.each([ + ['org/sub/repo'], + ['org/sub/subsub/repo'], + // The exact failing case from #290 — 4-level deep group path. + ['analogicdev/internal/tools/blueshift/blueshift-docmancer-ui'], + ['a/b/c'], + ['a/b/c/d/e/f/g'], + ['Org-Name/Sub-Group/Repo.With.Dots'], + ])('%s', (slug) => { + expect(REPO_SLUG_REGEX.test(slug)).toBe(true); + }); + }); + + describe('rejects malformed input', () => { + test.each([ + // No slash at all — bare owner. + ['just-a-name'], + [''], + // Leading slash → empty first segment. + ['/repo'], + // Trailing slash → empty last segment. + ['org/'], + // Empty middle segment. + ['org//repo'], + // Whitespace. + ['org /repo'], + ['org/ repo'], + // Shell metacharacters (defence-in-depth — adapters re-validate, but the + // schema is the first line of defence). + ['org/repo;rm -rf /'], + ['org/repo`whoami`'], + ['org/repo$(echo bad)'], + ['org/repo|cat /etc/passwd'], + // Other forbidden chars. + ['org/repo!exclaim'], + ['org/re@po'], + ['org/re#po'], + ['org/re po'], + ])('%s', (slug) => { + expect(REPO_SLUG_REGEX.test(slug)).toBe(false); + }); + }); +}); + +describe('repoSchema (Zod wrapper)', () => { + test('parses a flat slug', () => { + expect(repoSchema.parse('Wave-Engineering/mcp-server-sdlc')).toBe( + 'Wave-Engineering/mcp-server-sdlc', + ); + }); + + test('parses a nested-group slug', () => { + const slug = 'analogicdev/internal/tools/blueshift/blueshift-docmancer-ui'; + expect(repoSchema.parse(slug)).toBe(slug); + }); + + test('rejects bare owner with the documented error message', () => { + const r = repoSchema.safeParse('just-a-name'); + expect(r.success).toBe(false); + if (!r.success) { + // The Zod error wraps the regex message — confirm the contract. + expect(r.error.issues[0].message).toBe(REPO_SLUG_ERROR); + // Existing call-sites assert against the substring `owner/repo` — + // make sure the new error message keeps that substring intact so + // legacy tests don't break. + expect(r.error.issues[0].message).toContain('owner/repo'); + } + }); +}); + +describe('repoOptionalSchema', () => { + test('accepts undefined', () => { + expect(repoOptionalSchema.parse(undefined)).toBe(undefined); + }); + + test('accepts a nested slug', () => { + const slug = 'a/b/c/d'; + expect(repoOptionalSchema.parse(slug)).toBe(slug); + }); + + test('rejects an empty string (intentional — `optional` ≠ allow-empty)', () => { + expect(repoOptionalSchema.safeParse('').success).toBe(false); + }); +}); diff --git a/lib/schemas/repo.ts b/lib/schemas/repo.ts new file mode 100644 index 0000000..2f79f54 --- /dev/null +++ b/lib/schemas/repo.ts @@ -0,0 +1,45 @@ +/** + * Shared `repo` parameter schema for MCP tool input validation. + * + * Single source of truth for the regex pattern that validates the repository + * slug across every handler that takes a `repo: /` argument + * (`pr_*`, `ci_*`, `wave_*`, `label_*`, `work_item`, …). + * + * Pattern: `^[a-zA-Z0-9._-]+(/[a-zA-Z0-9._-]+)+$` — accepts arbitrary `/` + * depth so GitLab nested groups (e.g. + * `analogicdev/internal/tools/blueshift/blueshift-docmancer-ui`) validate + * the same way as flat GitHub `owner/repo` slugs (#290). + * + * Each segment must be ≥ 1 char from the set `[a-zA-Z0-9._-]` — empty + * segments (`a//b`), leading/trailing slashes, and bare `owner` (no `/`) + * are still rejected. + */ +import { z } from 'zod'; + +/** + * Regex that validates `owner/name` AND nested-group `org/sub/.../name` slugs. + * Exported separately so non-Zod call-sites (raw validation, formatting hints) + * can share the same pattern. + */ +export const REPO_SLUG_REGEX = /^[a-zA-Z0-9._-]+(\/[a-zA-Z0-9._-]+)+$/; + +/** + * Human-readable error message paired with the regex. Kept identical across + * every handler so callers see a consistent error string regardless of which + * MCP tool flagged the failure. + */ +export const REPO_SLUG_ERROR = + 'repo must be in owner/repo or group/subgroup/.../repo format'; + +/** + * Optional `repo` Zod schema — handlers that accept `repo` as an optional + * argument should `.optional()` chain off this base schema (or use the + * pre-chained `repoOptionalSchema` below). + */ +export const repoSchema = z.string().regex(REPO_SLUG_REGEX, REPO_SLUG_ERROR); + +/** + * Convenience export for the common `repo: optional` shape used by every + * `pr_*`/`ci_*`/`wave_*` handler. + */ +export const repoOptionalSchema = repoSchema.optional();