diff --git a/src/cli/flag-help-text.ts b/src/cli/flag-help-text.ts index 2aba17080e41..3e46a188ed87 100644 --- a/src/cli/flag-help-text.ts +++ b/src/cli/flag-help-text.ts @@ -26,6 +26,10 @@ export const FLAG_HELP_TEXT: Record = { interrupt: '--interrupt Send as an interrupt-style input when supported', id: '--id Identifier for a target item or permission', issue: '--issue Linked GitHub issue number', + 'gitlab-issue': + '--gitlab-issue Linked GitLab issue number or URL; null clears on set', + 'gitlab-mr': + '--gitlab-mr Linked GitLab merge request number or URL; null clears on set', 'linear-issue': '--linear-issue Linked Linear issue identifier or URL; null clears on set', json: '--json Emit machine-readable JSON', diff --git a/src/cli/handlers/worktree-gitlab-link.test.ts b/src/cli/handlers/worktree-gitlab-link.test.ts new file mode 100644 index 000000000000..6b718ebab896 --- /dev/null +++ b/src/cli/handlers/worktree-gitlab-link.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest' +import { getOptionalGitLabLinkFlag } from './worktree-gitlab-link' + +function flags(entries: Record): Map { + return new Map(Object.entries(entries)) +} + +describe('getOptionalGitLabLinkFlag', () => { + it('leaves the slot alone when the flag is absent', () => { + expect(getOptionalGitLabLinkFlag(flags({}), 'issue')).toBeUndefined() + expect(getOptionalGitLabLinkFlag(flags({}), 'mr')).toBeUndefined() + }) + + it.each([ + ['42', 42], + ['#42', 42], + [' 7 ', 7] + ])('reads the issue reference %s', (input, expected) => { + expect(getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': input }), 'issue')).toBe(expected) + }) + + it.each([ + ['77', 77], + ['!77', 77] + ])('reads the merge request reference %s', (input, expected) => { + expect(getOptionalGitLabLinkFlag(flags({ 'gitlab-mr': input }), 'mr')).toBe(expected) + }) + + it('reads a self-hosted issue URL, including a subgroup path', () => { + expect( + getOptionalGitLabLinkFlag( + flags({ 'gitlab-issue': 'https://gitlab.critel.li/group/sub/project/-/issues/923' }), + 'issue' + ) + ).toBe(923) + }) + + it('reads a merge request URL with trailing segments', () => { + expect( + getOptionalGitLabLinkFlag( + flags({ 'gitlab-mr': 'https://gitlab.com/group/project/-/merge_requests/77/diffs' }), + 'mr' + ) + ).toBe(77) + }) + + // Issues and merge requests are separate namespaces on GitLab, so a reference + // to one must never be taken as the other's number. + it('refuses a merge request reference in the issue flag', () => { + expect(() => getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': '!42' }), 'issue')).toThrow( + /GitLab issue number/ + ) + expect(() => + getOptionalGitLabLinkFlag( + flags({ 'gitlab-issue': 'https://gitlab.com/g/p/-/merge_requests/42' }), + 'issue' + ) + ).toThrow(/GitLab issue number/) + }) + + it('refuses an issue reference in the merge request flag', () => { + expect(() => getOptionalGitLabLinkFlag(flags({ 'gitlab-mr': '#42' }), 'mr')).toThrow( + /merge request number/ + ) + expect(() => + getOptionalGitLabLinkFlag(flags({ 'gitlab-mr': 'https://gitlab.com/g/p/-/issues/42' }), 'mr') + ).toThrow(/merge request number/) + }) + + it('clears the slot on set, and refuses to on create', () => { + expect( + getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': 'null' }), 'issue', { allowNull: true }) + ).toBeNull() + expect( + getOptionalGitLabLinkFlag(flags({ 'gitlab-mr': 'NULL' }), 'mr', { allowNull: true }) + ).toBeNull() + expect(() => getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': 'null' }), 'issue')).toThrow( + /Omit --gitlab-issue on create/ + ) + }) + + it('reports a flag given without a value', () => { + expect(() => getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': true }), 'issue')).toThrow( + 'Missing value for --gitlab-issue' + ) + }) + + it.each(['', ' ', '0', '-1', '4 2', '42x', 'STA-335', 'https://gitlab.com/g/p/-/issues/abc'])( + 'refuses %s', + (input) => { + expect(() => getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': input }), 'issue')).toThrow() + } + ) + + // A URL without GitLab's `/-/` separator is not a GitLab link, and a project + // path needs a group segment — neither may fall through to a number. + it.each(['https://gitlab.com/group/project/issues/42', 'https://gitlab.com/project/-/issues/42'])( + 'refuses the non-GitLab URL shape %s', + (input) => { + expect(() => getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': input }), 'issue')).toThrow() + } + ) + + // `/^\d+$/` accepts 400 digits, which parseInt turns into Infinity — persisting + // that writes `null` over the link it meant to set. + it('refuses a number too large to be an integer', () => { + expect(() => + getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': '9'.repeat(400) }), 'issue') + ).toThrow() + }) +}) diff --git a/src/cli/handlers/worktree-gitlab-link.ts b/src/cli/handlers/worktree-gitlab-link.ts new file mode 100644 index 000000000000..69279fa1acbc --- /dev/null +++ b/src/cli/handlers/worktree-gitlab-link.ts @@ -0,0 +1,97 @@ +import { parseGitLabIssueOrMRLink } from '../../shared/new-workspace/gitlab-links' +import { RuntimeClientError } from '../runtime-client' + +/** Which GitLab slot a flag writes. The two are separate namespaces on GitLab, + * so a reference to one is never a valid value for the other. */ +export type GitLabLinkKind = 'issue' | 'mr' + +const FLAG_BY_KIND: Record = { + issue: 'gitlab-issue', + mr: 'gitlab-mr' +} + +// Why: GitLab writes `#42` for an issue and `!42` for a merge request. Accepting +// the wrong prefix would silently link the other namespace's number. +const PREFIX_BY_KIND: Record = { + issue: '#', + mr: '!' +} + +function parseNumericReference(input: string, kind: GitLabLinkKind): number | null { + const wrongPrefix = PREFIX_BY_KIND[kind === 'issue' ? 'mr' : 'issue'] + if (input.startsWith(wrongPrefix)) { + return null + } + const digits = input.startsWith(PREFIX_BY_KIND[kind]) ? input.slice(1) : input + if (!/^\d+$/.test(digits)) { + return null + } + const parsed = Number.parseInt(digits, 10) + // Why: `/^\d+$/` accepts 400 digits, which parseInt turns into Infinity — + // JSON.stringify would then persist `null` over the link it meant to set. + return Number.isSafeInteger(parsed) && parsed > 0 ? parsed : null +} + +/** + * Resolve `--gitlab-issue` / `--gitlab-mr` into the number to persist. + * `undefined` means the flag was absent and the slot must be left alone; + * `null` means the caller asked to clear it. + */ +export function getOptionalGitLabLinkFlag( + flags: Map, + kind: GitLabLinkKind, + options: { allowNull?: boolean } = {} +): number | null | undefined { + const name = FLAG_BY_KIND[kind] + const value = getPresentStringFlag(flags, name) + if (value === undefined) { + return undefined + } + + const trimmed = value.trim() + if (trimmed.toLowerCase() === 'null') { + if (!options.allowNull) { + throw new RuntimeClientError( + 'invalid_argument', + `Omit --${name} on create, or pass a GitLab ${kind === 'issue' ? 'issue' : 'merge request'} number or URL.` + ) + } + return null + } + + if (/^https?:\/\//i.test(trimmed)) { + const link = parseGitLabIssueOrMRLink(trimmed) + // Why: an issue URL passed to --gitlab-mr names a real GitLab item, just not + // this one. Taking its number would link a merge request that may not exist. + if (link?.type === kind && Number.isSafeInteger(link.number) && link.number > 0) { + return link.number + } + throw new RuntimeClientError('invalid_argument', badValueMessage(name, kind)) + } + + const number = parseNumericReference(trimmed, kind) + if (number === null) { + throw new RuntimeClientError('invalid_argument', badValueMessage(name, kind)) + } + return number +} + +function badValueMessage(name: string, kind: GitLabLinkKind): string { + return kind === 'issue' + ? `Pass a GitLab issue number like 42 or #42, a GitLab issue URL, or null to clear --${name}.` + : `Pass a GitLab merge request number like 42 or !42, a GitLab merge request URL, or null to clear --${name}.` +} + +function getPresentStringFlag( + flags: Map, + name: string +): string | undefined { + if (!flags.has(name)) { + return undefined + } + const value = flags.get(name) + if (typeof value === 'string' && value.length > 0) { + return value + } + throw new RuntimeClientError('invalid_argument', `Missing value for --${name}`) +} diff --git a/src/cli/handlers/worktree.ts b/src/cli/handlers/worktree.ts index f74f6df79337..d59f49feffa5 100644 --- a/src/cli/handlers/worktree.ts +++ b/src/cli/handlers/worktree.ts @@ -38,6 +38,7 @@ import { resolveCreateParentSelector } from './worktree-create-parent-selector' import { getOptionalLinearIssueLinkFlag } from './worktree-linear-issue-link' +import { getOptionalGitLabLinkFlag } from './worktree-gitlab-link' function assertParentWorktreeFlagsCompatible(flags: Map): void { if (flags.has('parent-worktree') && flags.get('no-parent') === true) { @@ -216,6 +217,8 @@ export const WORKTREE_HANDLERS: Record = { } } const linearIssueLink = getOptionalLinearIssueLinkFlag(flags, 'linear-issue') + const linkedGitLabIssue = getOptionalGitLabLinkFlag(flags, 'issue') + const linkedGitLabMR = getOptionalGitLabLinkFlag(flags, 'mr') const activate = flags.get('activate') === true || flags.get('run-hooks') === true const name = getRequiredStringFlag(flags, 'name') const result = await client.call('worktree.create', { @@ -226,6 +229,8 @@ export const WORKTREE_HANDLERS: Record = { baseBranch: getOptionalStringFlag(flags, 'base-branch'), linkedIssue: getOptionalNumberFlag(flags, 'issue'), ...linearIssueLink, + ...(linkedGitLabIssue === undefined ? {} : { linkedGitLabIssue }), + ...(linkedGitLabMR === undefined ? {} : { linkedGitLabMR }), comment: getOptionalStringFlag(flags, 'comment'), runHooks: flags.get('run-hooks') === true, activate, @@ -259,11 +264,17 @@ export const WORKTREE_HANDLERS: Record = { const linearIssueLink = getOptionalLinearIssueLinkFlag(flags, 'linear-issue', { allowNull: true }) + const linkedGitLabIssue = getOptionalGitLabLinkFlag(flags, 'issue', { allowNull: true }) + const linkedGitLabMR = getOptionalGitLabLinkFlag(flags, 'mr', { allowNull: true }) const result = await client.call<{ worktree: RuntimeWorktreeRecord }>('worktree.set', { worktree: await getRequiredWorktreeSelector(flags, 'worktree', cwd, client), displayName: getOptionalStringFlag(flags, 'display-name'), linkedIssue: getOptionalNullableNumberFlag(flags, 'issue'), ...linearIssueLink, + // Why: an absent flag must not emit the key at all — the update spreads raw, + // so a present-but-undefined key would erase the stored link. + ...(linkedGitLabIssue === undefined ? {} : { linkedGitLabIssue }), + ...(linkedGitLabMR === undefined ? {} : { linkedGitLabMR }), comment: getOptionalStringFlag(flags, 'comment'), workspaceStatus: getOptionalStringFlag(flags, 'workspace-status'), parentWorktree: await getOptionalWorktreeSelector(flags, 'parent-worktree', cwd, client), diff --git a/src/cli/index-worktree-set.test.ts b/src/cli/index-worktree-set.test.ts index b8dd288b9b5d..b7689a097025 100644 --- a/src/cli/index-worktree-set.test.ts +++ b/src/cli/index-worktree-set.test.ts @@ -391,4 +391,86 @@ describe('orca cli worktree awareness', () => { noParent: false }) }) + + it('passes a GitLab merge request reference through worktree.set', async () => { + queueFixtures( + callMock, + okFixture('req_set_gitlab_mr', { + worktree: { ...buildWorktree('/tmp/repo/child', 'feature/child'), linkedGitLabMR: 77 } + }) + ) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main( + ['worktree', 'set', '--worktree', 'id:repo::/tmp/repo/child', '--gitlab-mr', '!77', '--json'], + '/tmp/repo' + ) + + expect(callMock).toHaveBeenCalledWith('worktree.set', { + worktree: 'id:repo::/tmp/repo/child', + displayName: undefined, + linkedIssue: undefined, + linkedGitLabMR: 77, + comment: undefined, + workspaceStatus: undefined, + parentWorktree: undefined, + noParent: false + }) + }) + + it('clears a GitLab issue link with null', async () => { + queueFixtures( + callMock, + okFixture('req_set_gitlab_issue_clear', { + worktree: { ...buildWorktree('/tmp/repo/child', 'feature/child'), linkedGitLabIssue: null } + }) + ) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main( + [ + 'worktree', + 'set', + '--worktree', + 'id:repo::/tmp/repo/child', + '--gitlab-issue', + 'null', + '--json' + ], + '/tmp/repo' + ) + + expect(callMock).toHaveBeenCalledWith('worktree.set', { + worktree: 'id:repo::/tmp/repo/child', + displayName: undefined, + linkedIssue: undefined, + linkedGitLabIssue: null, + comment: undefined, + workspaceStatus: undefined, + parentWorktree: undefined, + noParent: false + }) + }) + + // Why: the update spreads raw, so emitting the key at all on an untouched flag + // would erase the stored link. + it('omits both GitLab keys entirely when neither flag is passed', async () => { + queueFixtures( + callMock, + okFixture('req_set_no_gitlab', { + worktree: buildWorktree('/tmp/repo/child', 'feature/child') + }) + ) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main( + ['worktree', 'set', '--worktree', 'id:repo::/tmp/repo/child', '--comment', 'note', '--json'], + '/tmp/repo' + ) + + const payload = callMock.mock.calls.at(-1)?.[1] + expect(payload).toBeDefined() + expect(Object.keys(payload ?? {})).not.toContain('linkedGitLabIssue') + expect(Object.keys(payload ?? {})).not.toContain('linkedGitLabMR') + }) }) diff --git a/src/cli/root-help-text-secondary.ts b/src/cli/root-help-text-secondary.ts index 34eeb71cf0fe..132c58437988 100644 --- a/src/cli/root-help-text-secondary.ts +++ b/src/cli/root-help-text-secondary.ts @@ -50,10 +50,10 @@ export const ROOT_HELP_TEXT_SECONDARY = [ ' orca environment show --environment [--json]', ' orca environment rm --environment [--json]', ' orca worktree list [--repo ] [--limit ] [--json]', - ' orca worktree create --name [--repo |--project [--host ]|--project-host-setup ] [--agent ] [--prompt ] [--setup run|skip|inherit] [--base-branch ] [--issue ] [--linear-issue ] [--comment ] [--parent-worktree ] [--no-parent] [--run-hooks] [--activate] [--json]', + ' orca worktree create --name [--repo |--project [--host ]|--project-host-setup ] [--agent ] [--prompt ] [--setup run|skip|inherit] [--base-branch ] [--issue ] [--linear-issue ] [--gitlab-issue ] [--gitlab-mr ] [--comment ] [--parent-worktree ] [--no-parent] [--run-hooks] [--activate] [--json]', ' orca worktree show --worktree [--json]', ' orca worktree current [--json]', - ' orca worktree set --worktree [--display-name ] [--issue ] [--linear-issue ] [--comment ] [--workspace-status ] [--parent-worktree |--no-parent] [--json]', + ' orca worktree set --worktree [--display-name ] [--issue ] [--linear-issue ] [--gitlab-issue ] [--gitlab-mr ] [--comment ] [--workspace-status ] [--parent-worktree |--no-parent] [--json]', ' orca worktree rm --worktree [--force] [--run-hooks] [--allow-failed-archive-hook] [--json]', ' orca worktree ps [--limit ] [--json]', ' orca file open [--worktree ] [--focus] [--json]', @@ -164,6 +164,8 @@ export const ROOT_HELP_TEXT_SECONDARY = [ ' $ orca worktree current', ' $ orca worktree set --worktree active --comment "waiting on review"', ' $ orca worktree set --worktree active --linear-issue null', + ' $ orca worktree set --worktree active --gitlab-mr !77', + ' $ orca worktree set --worktree active --gitlab-issue https://gitlab.example.com/group/project/-/issues/42', ' $ orca worktree ps --limit 10', ' $ orca file open-changed --mode diff', ' $ orca file open src/App.tsx', diff --git a/src/cli/specs/core.ts b/src/cli/specs/core.ts index cf47b0071d6c..004e57cc3719 100644 --- a/src/cli/specs/core.ts +++ b/src/cli/specs/core.ts @@ -93,7 +93,7 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [ path: ['worktree', 'create'], summary: 'Create a new Orca-managed worktree', usage: - 'orca worktree create --name [--repo |--project [--host ]|--project-host-setup ] [--agent ] [--prompt ] [--setup run|skip|inherit] [--base-branch ] [--issue ] [--linear-issue ] [--comment ] [--parent-worktree ] [--no-parent] [--run-hooks] [--activate] [--json]', + 'orca worktree create --name [--repo |--project [--host ]|--project-host-setup ] [--agent ] [--prompt ] [--setup run|skip|inherit] [--base-branch ] [--issue ] [--linear-issue ] [--gitlab-issue ] [--gitlab-mr ] [--comment ] [--parent-worktree ] [--no-parent] [--run-hooks] [--activate] [--json]', allowedFlags: [ ...GLOBAL_FLAGS, 'repo', @@ -106,6 +106,8 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [ 'base-branch', 'issue', 'linear-issue', + 'gitlab-issue', + 'gitlab-mr', 'comment', 'setup', 'parent-worktree', @@ -144,13 +146,15 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [ path: ['worktree', 'set'], summary: 'Update Orca metadata for a worktree', usage: - 'orca worktree set --worktree [--display-name ] [--issue ] [--linear-issue ] [--comment ] [--workspace-status ] [--parent-worktree |--no-parent] [--json]', + 'orca worktree set --worktree [--display-name ] [--issue ] [--linear-issue ] [--gitlab-issue ] [--gitlab-mr ] [--comment ] [--workspace-status ] [--parent-worktree |--no-parent] [--json]', allowedFlags: [ ...GLOBAL_FLAGS, 'worktree', 'display-name', 'issue', 'linear-issue', + 'gitlab-issue', + 'gitlab-mr', 'comment', 'workspace-status', 'parent-worktree', @@ -158,7 +162,9 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [ ], notes: [ 'Workspace status ids match the board columns (defaults: todo, in-progress, in-review, completed); custom statuses use their configured id.', - 'Pass --linear-issue null to clear the Linear issue link.' + 'Pass --linear-issue null to clear the Linear issue link.', + 'Pass --gitlab-issue null or --gitlab-mr null to clear the matching GitLab link.', + 'Each link flag writes only its own slot. Unlike the app dialog, which keeps one issue per workspace, setting --gitlab-issue here leaves any --issue or --linear-issue link in place; clear the other explicitly if you want only one.' ], examples: [ 'orca worktree set --worktree active --linear-issue STA-335 --json',