From 5878f48913339e0ba5b558d92d3781853681c03a Mon Sep 17 00:00:00 2001 From: Adam Creeger Date: Fri, 1 May 2026 16:56:34 +0200 Subject: [PATCH] feat(issue-1009): support pasting tracker URLs into il start and il plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a TrackerUrlParser helper that converts pasted GitHub issue/PR, Linear, and Jira URLs (cloud + self-hosted) into the canonical identifier expected by il start and il plan. Identifiers are normalized through IssueTracker.normalizeIdentifier() to keep case-sensitive comparisons safe. Policy: - il start: rejects cross-repo URLs and provider mismatches, with a carve-out for GitHub PR URLs (matches the existing PR fallback); validates Jira host against the configured one. - il plan: rejects PR URLs and provider mismatches; accepts cross-repo GitHub URLs but skips MCP getIssue/getChildIssues/getDependencies with a logger.warn (full cross-repo MCP plumbing is deferred). Telemetry adds identifier_source ('url' | 'identifier') and url_provider ('github' | 'linear' | 'jira') as enums only — URLs are never logged. Includes unit tests for the parser, validator, and both commands plus docs in docs/iloom-commands.md. --- docs/iloom-commands.md | 61 ++++ src/commands/CLAUDE.md | 4 +- src/commands/plan.test.ts | 1 + src/commands/plan.ts | 123 ++++++- src/commands/start.test.ts | 359 +++++++++++++++++++++ src/commands/start.ts | 98 +++++- src/lib/LinearService.ts | 16 +- src/lib/providers/jira/JiraIssueTracker.ts | 20 +- src/types/telemetry.ts | 20 ++ src/utils/TrackerUrlParser.test.ts | 350 ++++++++++++++++++++ src/utils/TrackerUrlParser.ts | 236 ++++++++++++++ src/utils/tracker-url-validation.test.ts | 165 ++++++++++ src/utils/tracker-url-validation.ts | 88 +++++ 13 files changed, 1519 insertions(+), 22 deletions(-) create mode 100644 src/utils/TrackerUrlParser.test.ts create mode 100644 src/utils/TrackerUrlParser.ts create mode 100644 src/utils/tracker-url-validation.test.ts create mode 100644 src/utils/tracker-url-validation.ts diff --git a/docs/iloom-commands.md b/docs/iloom-commands.md index 2f87af2f..88ce7b12 100644 --- a/docs/iloom-commands.md +++ b/docs/iloom-commands.md @@ -51,6 +51,7 @@ Create an isolated loom workspace with complete AI-assisted context establishmen ```bash il start il start +il start il start il start "" ``` @@ -58,9 +59,31 @@ il start "" **Arguments:** - `` - GitHub or Linear issue number (e.g., `25`) - `` - GitHub pull request number (e.g., `42`) +- `` - A full URL to a tracker issue or GitHub PR (see "Supported URL Forms" below) - `` - Existing git branch name - `` - Free-form description to create a new issue (quoted string) +**Supported URL Forms:** + +`il start` accepts a full tracker URL in addition to a bare identifier. The URL is parsed into a tracker identifier and dispatched normally. + +| Tracker | URL shape | +|---------|-----------| +| GitHub issue | `https://github.com///issues/` | +| GitHub PR | `https://github.com///pull/` | +| Linear | `https://linear.app//issue//...` | +| Jira (cloud) | `https://.atlassian.net/browse/` | +| Jira (self-hosted) | `https:///browse/` | + +URL parsing is robust to trailing slashes, query strings, fragments (e.g., `#comment-123`), and mixed-case hosts. + +**URL Policy for `il start`:** + +- **Same-repo only:** Cross-repo URLs are rejected. `il start` operates on the current repository's worktree, so the URL must point to the same `/` as the configured remote. +- **Provider must match:** The URL's provider must match the configured issue tracker (e.g., a Linear URL on a GitHub-configured project is rejected). + - **GitHub PR carve-out:** A GitHub PR URL is always accepted by `il start`, even when the configured issue tracker is Linear or Jira. PRs are GitHub-native, so this is a deliberate exception to the provider-match rule. +- **PR URLs are valid here:** Both issue and PR URLs are accepted by `il start`. (PR URLs are NOT valid for `il plan` — see that section.) + **Options:** | Flag | Values | Description | @@ -135,6 +158,15 @@ il start 25 # Start work on Linear issue ILM-42 il start ILM-42 +# Start from a tracker URL (GitHub issue) +il start https://github.com/iloom-ai/iloom-cli/issues/1009 + +# Start from a Linear issue URL +il start https://linear.app/my-team/issue/WEB-123/some-slug + +# Start from a GitHub PR URL (allowed even on Linear/Jira-configured repos) +il start https://github.com/iloom-ai/iloom-cli/pull/1010 + # Create a new issue and start work il start "Add dark mode toggle to settings" @@ -1391,11 +1423,13 @@ Launch an interactive planning session with an Architect persona to decompose fe ```bash il plan [prompt] [options] il plan [options] +il plan [options] ``` **Arguments:** - `[prompt]` - Optional initial planning prompt or topic for fresh planning mode - `` - Issue identifier to decompose (GitHub: `#123` or `123`, Linear: `ENG-123`) +- `` - Full URL to a tracker issue (see "Supported URL Forms" below) **Operating Modes:** @@ -1403,6 +1437,27 @@ il plan [options] |------|---------|-------------| | Fresh Planning | `il plan` or `il plan "topic"` | Start a new planning session for a feature or epic | | Decomposition | `il plan 123` or `il plan #123` | Break down an existing issue into child issues | +| Decomposition (URL) | `il plan https://github.com/owner/repo/issues/123` | Same as above, but pasted as a URL | + +**Supported URL Forms:** + +`il plan` accepts a full tracker issue URL in addition to a bare identifier. The URL is parsed into a tracker identifier before fetching. + +| Tracker | URL shape | +|---------|-----------| +| GitHub issue | `https://github.com///issues/` | +| Linear | `https://linear.app//issue//...` | +| Jira (cloud) | `https://.atlassian.net/browse/` | +| Jira (self-hosted) | `https:///browse/` | + +URL parsing is robust to trailing slashes, query strings, fragments, and mixed-case hosts. + +**URL Policy for `il plan`:** + +- **PR URLs are rejected:** `il plan` is for issue decomposition only. GitHub PR URLs (`/pull/`) are not valid input — use `il start` for PRs. +- **Provider must match:** The URL's provider must match the configured issue tracker. There is no GitHub PR carve-out here (PR URLs are not accepted in the first place). +- **Cross-repo URLs are accepted:** Unlike `il start`, `il plan` accepts GitHub issue URLs that point to a different `/` than the cwd repo. This lets you decompose an issue in another repository. + - When the URL repo doesn't match the cwd repo, MCP-backed features that require write access to the issue tracker — specifically child-issue creation and dependency management — are skipped with a warning. Plan output is still generated, but you'll need to create the child issues manually (or run `il plan` from the target repo's worktree). **Options:** @@ -1467,6 +1522,12 @@ il plan "#42" # Linear issue il plan ENG-123 + +# Tracker URL (GitHub issue) +il plan https://github.com/iloom-ai/iloom-cli/issues/1009 + +# Cross-repo GitHub issue URL (MCP write features will be skipped with a warning) +il plan https://github.com/some-other-org/some-other-repo/issues/42 ``` In decomposition mode, the Architect: diff --git a/src/commands/CLAUDE.md b/src/commands/CLAUDE.md index 3fe4ca3f..440ca286 100644 --- a/src/commands/CLAUDE.md +++ b/src/commands/CLAUDE.md @@ -14,7 +14,7 @@ Commands are registered in `src/cli.ts` using Commander.js. Each command class h | Command | Aliases | File | Delegates To | Key Purpose | |---------|---------|------|-------------|-------------| -| `start` | `new`, `create`, `up` | `start.ts` | LoomManager, GitWorktreeManager, DatabaseManager, AgentManager | Create isolated workspace for issue/PR/epic | +| `start` | `new`, `create`, `up` | `start.ts` | LoomManager, GitWorktreeManager, DatabaseManager, AgentManager | Create isolated workspace for issue/PR/epic. Accepts a tracker URL (GitHub issue/PR, Linear, Jira) in addition to a bare identifier; parsed via `src/utils/TrackerUrlParser.ts`. Cross-repo URLs are rejected; GitHub PR URLs are accepted regardless of configured tracker. | | `finish` | `dn` | `finish.ts` | MergeManager, ValidationRunner, ResourceCleanup, PRManager | Merge branch, cleanup workspace | | `cleanup` | `remove`, `clean` | `cleanup.ts` | GitWorktreeManager, ResourceCleanup, LoomManager | Remove worktree(s) | | `list` | — | (inline in cli.ts) | LoomManager, GitWorktreeManager | Show active looms | @@ -24,7 +24,7 @@ Commands are registered in `src/cli.ts` using Commander.js. Each command class h | Command | Aliases | File | Delegates To | Key Purpose | |---------|---------|------|-------------|-------------| | `spin` | `ignite` | `ignite.ts` | PromptTemplateManager, SwarmSetupService, AgentManager | Launch Claude with workspace context; swarm orchestrator for epics | -| `plan` | — | `plan.ts` | PromptTemplateManager, IssueTrackerFactory, AgentManager | Architect-mode decomposition; optional auto-swarm | +| `plan` | — | `plan.ts` | PromptTemplateManager, IssueTrackerFactory, AgentManager | Architect-mode decomposition; optional auto-swarm. Accepts a tracker issue URL in addition to a bare identifier; parsed via `src/utils/TrackerUrlParser.ts`. PR URLs are rejected; cross-repo GitHub issue URLs are accepted but MCP-backed write features (child issues, dependencies) are skipped with a warning. | ### Git & Commit Commands diff --git a/src/commands/plan.test.ts b/src/commands/plan.test.ts index 5f825c4b..24a71a3a 100644 --- a/src/commands/plan.test.ts +++ b/src/commands/plan.test.ts @@ -988,6 +988,7 @@ describe('PlanCommand', () => { expect(mockTrack).toHaveBeenCalledWith('epic.planned', { child_count: 3, tracker: 'github', + identifier_source: 'identifier', }) }) diff --git a/src/commands/plan.ts b/src/commands/plan.ts index ce661c1a..3a783f86 100644 --- a/src/commands/plan.ts +++ b/src/commands/plan.ts @@ -13,6 +13,9 @@ import { SettingsManager, PlanCommandSettingsSchema } from '../lib/SettingsManag import type { EffortLevel } from '../types/index.js' import { IssueTrackerFactory } from '../lib/IssueTrackerFactory.js' import { matchIssueIdentifier } from '../utils/IdentifierParser.js' +import { parseTrackerUrl, TrackerUrlError, type TrackerUrlParseResult } from '../utils/TrackerUrlParser.js' +import { validateTrackerUrlAgainstSettings } from '../utils/tracker-url-validation.js' +import { getConfiguredRepoFromSettings } from '../utils/remote.js' import { IssueManagementProviderFactory } from '../mcp/IssueManagementProviderFactory.js' import { needsFirstRunSetup, launchFirstRunSetup } from '../utils/first-run-setup.js' import type { IssueProvider, ChildIssueResult, DependenciesResult } from '../mcp/types.js' @@ -201,7 +204,63 @@ export class PlanCommand { // Uses shared matchIssueIdentifier() utility to identify issue identifiers: // - Numeric pattern: #123 or 123 (GitHub format) // - Project key pattern: ENG-123, PROJ-456 (requires at least 2 letters before dash) - const identifierMatch = prompt ? matchIssueIdentifier(prompt) : { isIssueIdentifier: false } + // URL inputs (GitHub issue, Linear, Jira) are detected first via parseTrackerUrl(). + const provider = settings ? IssueTrackerFactory.getProviderName(settings) : 'github' + const issuePrefix = provider === 'github' ? '#' : '' + + // URL parsing: try parseTrackerUrl() before falling back to matchIssueIdentifier. + // On a recognized URL, we override the identifier (canonical form) and set + // `urlRepo` so downstream IssueTracker calls receive the correct repo. + let identifierForLookup: string | undefined = prompt + let urlRepo: string | undefined + let identifierSource: 'url' | 'identifier' = 'identifier' + + if (prompt) { + let parsed + try { + parsed = parseTrackerUrl(prompt) + } catch (error) { + if (error instanceof TrackerUrlError) { + throw new Error(`Invalid tracker URL: ${error.message}`) + } + throw error + } + + if (parsed) { + // Reject GitHub PR URLs — `il plan` is for issues only. + if (parsed.kind === 'pr') { + throw new Error( + `'il plan' decomposes issues, not pull requests. Paste the underlying issue URL or identifier instead.` + ) + } + + // Provider mismatch + Jira host mismatch (shared with `il start`). + // No PR carve-out for plan since PRs are rejected above. + const configuredJiraHost = settings?.issueManagement?.jira?.host + validateTrackerUrlAgainstSettings(parsed, { + configuredProvider: provider as TrackerUrlParseResult['provider'], + ...(configuredJiraHost !== undefined && { configuredJiraHost }), + allowPrCarveOut: false, + }) + + // Normalize identifier defensively (handles legacy storage casing too). + const issueTracker = IssueTrackerFactory.create(settings) + identifierForLookup = issueTracker.normalizeIdentifier(parsed.identifier) + urlRepo = parsed.repo + identifierSource = 'url' + logger.debug('Parsed tracker URL', { + urlProvider: parsed.provider, + identifier: identifierForLookup, + hasRepo: !!urlRepo, + }) + } + } + + const identifierMatch = identifierForLookup + ? (identifierSource === 'url' + ? { isIssueIdentifier: true as const, type: 'numeric' as const, identifier: identifierForLookup } + : matchIssueIdentifier(identifierForLookup)) + : { isIssueIdentifier: false } const looksLikeIssueIdentifier = identifierMatch.isIssueIdentifier let decompositionContext: { identifier: string @@ -211,21 +270,56 @@ export class PlanCommand { dependencies?: DependenciesResult } | null = null - const provider = settings ? IssueTrackerFactory.getProviderName(settings) : 'github' - const issuePrefix = provider === 'github' ? '#' : '' + // Determine if URL repo differs from cwd repo — affects MCP cross-repo handling. + // For non-GitHub providers or when no urlRepo, the comparison is moot (urlRepo is undefined for Linear/Jira). + let isCrossRepo = false + if (urlRepo && provider === 'github') { + try { + const cwdRepo = settings ? await getConfiguredRepoFromSettings(settings) : undefined + if (cwdRepo && cwdRepo.toLowerCase() !== urlRepo.toLowerCase()) { + isCrossRepo = true + } + } catch (error) { + // Narrow catch: only graceful-degrade when the cwd has no + // configured/known GitHub remote. The user supplied a tracker URL + // with an explicit repo, so we already have everything we need + // to identify the issue — assuming cross-repo here just disables + // MCP-backed write features (children/dependencies), which is + // the intended fallback. Any OTHER error (e.g. unrelated I/O, + // programming error) should surface, not be swallowed. + // + // `getConfiguredRepoFromSettings` and `validateConfiguredRemote` + // throw plain `Error` (no typed class), so we match on message + // substrings emitted by those helpers and by `git remote -v` + // when the cwd is not a git repo. + const message = error instanceof Error ? error.message : String(error) + const isMissingRemote = + /GitHub remote not configured/i.test(message) || + /Configured remote ".*" not found/i.test(message) || + /Remote ".*" does not exist/i.test(message) || + /not a git repository/i.test(message) + if (!isMissingRemote) { + throw error + } + logger.debug( + `No configured cwd repo; treating as cross-repo so MCP write features are skipped. ${message}` + ) + isCrossRepo = true + } + } - if (prompt && looksLikeIssueIdentifier) { + if (identifierForLookup && looksLikeIssueIdentifier) { // Validate and fetch issue using issueTracker.detectInputType() pattern from StartCommand const issueTracker = IssueTrackerFactory.create(settings) - logger.debug('Detected potential issue identifier, validating via issueTracker', { identifier: prompt }) + logger.debug('Detected potential issue identifier, validating via issueTracker', { identifier: identifierForLookup, isCrossRepo }) // Use detectInputType to validate the identifier exists (same pattern as StartCommand) - const detection = await issueTracker.detectInputType(prompt) + const detection = await issueTracker.detectInputType(identifierForLookup, urlRepo) if (detection.type === 'issue' && detection.identifier) { // Valid issue found - fetch full details for decomposition context - const issue = await issueTracker.fetchIssue(detection.identifier) + const issue = await issueTracker.fetchIssue(detection.identifier, urlRepo) // Construct the MCP provider once and reuse for body+comments and // children/dependencies. If construction fails, all MCP fetches are @@ -248,7 +342,14 @@ export class PlanCommand { let bodyForPlan = issue.body let bodyFromMcp = false let commentsSection = '' - if (mcpProvider) { + if (mcpProvider && isCrossRepo) { + logger.warn( + `Cross-repo URL detected — skipping MCP getIssue/getChildIssues/getDependencies fetches. ` + + `Cross-repo MCP support is not yet implemented; falling back to issueTracker body. ` + + `(Tracked separately.)` + ) + } + if (mcpProvider && !isCrossRepo) { try { const mcpIssue = await mcpProvider.getIssue({ number: detection.identifier, includeComments: true }) if (mcpIssue.body) { @@ -297,7 +398,7 @@ export class PlanCommand { // Fetch existing children and dependencies using MCP provider // This allows users to resume planning where they left off - if (mcpProvider) { + if (mcpProvider && !isCrossRepo) { try { // Fetch child issues logger.debug('Fetching child issues for decomposition context', { identifier: decompositionContext.identifier }) @@ -330,7 +431,7 @@ export class PlanCommand { } else { // Input matched issue pattern but issue not found - treat as regular prompt logger.debug('Input matched issue pattern but issue not found, treating as planning topic', { - identifier: prompt, + identifier: identifierForLookup, detectionType: detection.type }) } @@ -460,6 +561,7 @@ export class PlanCommand { TelemetryService.getInstance().track('auto_swarm.started', { source: autoSwarmSource, planner: effectivePlanner, + identifier_source: identifierSource, }) } catch (error) { logger.debug(`Telemetry auto_swarm.started tracking failed: ${error instanceof Error ? error.message : error}`) @@ -772,6 +874,7 @@ ${initialMessage}` TelemetryService.getInstance().track('epic.planned', { child_count: children.length, tracker: provider, + identifier_source: identifierSource, }) } catch (error) { logger.debug(`Telemetry epic.planned tracking failed: ${error instanceof Error ? error.message : error}`) diff --git a/src/commands/start.test.ts b/src/commands/start.test.ts index fffca52e..10d1afb0 100644 --- a/src/commands/start.test.ts +++ b/src/commands/start.test.ts @@ -147,6 +147,8 @@ describe('StartCommand', () => { // Set IssueTracker interface properties mockGitHubService.supportsPullRequests = true mockGitHubService.providerName = 'github' + // GitHub identifiers are numeric — normalizeIdentifier just stringifies. + mockGitHubService.normalizeIdentifier = vi.fn((id: string | number) => String(id)) command = new StartCommand(mockGitHubService) // Default: no child issues (epic detection returns empty) @@ -527,6 +529,313 @@ describe('StartCommand', () => { expect(mockGitHubService.createIssue).not.toHaveBeenCalled() }) + describe('tracker URL input', () => { + it('should parse a GitHub issue URL into a numeric identifier', async () => { + const { hasMultipleRemotes } = await import('../utils/remote.js') + vi.mocked(hasMultipleRemotes).mockResolvedValue(false) + + vi.mocked(mockGitHubService.fetchIssue).mockResolvedValue({ + number: 123, + title: 'Test', + body: '', + state: 'open', + labels: [], + assignees: [], + url: 'https://github.com/owner/repo/issues/123', + }) + vi.mocked(mockGitHubService.validateIssueState).mockResolvedValue() + + await expect( + command.execute({ + identifier: 'https://github.com/owner/repo/issues/123', + options: {}, + }) + ).resolves.not.toThrow() + + // Issue tracker validates by number, not URL — URL was parsed. + expect(mockGitHubService.fetchIssue).toHaveBeenCalledWith(123, undefined) + // detectInputType should NOT be called when URL parsing already + // produced a typed identifier. + expect(mockGitHubService.detectInputType).not.toHaveBeenCalled() + }) + + it('should parse a GitHub PR URL into a numeric identifier', async () => { + vi.mocked(mockGitHubService.fetchPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: '', + state: 'open', + branch: 'feature', + baseBranch: 'main', + url: 'https://github.com/owner/repo/pull/42', + isDraft: false, + }) + vi.mocked(mockGitHubService.validatePRState).mockResolvedValue() + + await expect( + command.execute({ + identifier: 'https://github.com/owner/repo/pull/42', + options: {}, + }) + ).resolves.not.toThrow() + + expect(mockGitHubService.fetchPR).toHaveBeenCalledWith(42, undefined) + expect(mockGitHubService.fetchIssue).not.toHaveBeenCalled() + }) + + it('should accept a GitHub PR URL even when configured tracker is Linear (carve-out)', async () => { + const mockLinearService = { + supportsPullRequests: false, + providerName: 'linear', + detectInputType: vi.fn(), + fetchIssue: vi.fn(), + validateIssueState: vi.fn(), + normalizeIdentifier: vi.fn((id: string | number) => String(id).toUpperCase()), + } + const linearCommand = new StartCommand(mockLinearService as unknown as GitHubService) + + const MockedGitHubService = vi.mocked(GitHubService) + MockedGitHubService.prototype.fetchPR = vi.fn().mockResolvedValue({ + number: 99, + title: 'PR', + body: '', + state: 'open', + branch: 'feature', + baseBranch: 'main', + url: 'https://github.com/owner/repo/pull/99', + isDraft: false, + }) + MockedGitHubService.prototype.validatePRState = vi.fn().mockResolvedValue(undefined) + + await expect( + linearCommand.execute({ + identifier: 'https://github.com/owner/repo/pull/99', + options: {}, + }) + ).resolves.not.toThrow() + + // Linear tracker is bypassed for the PR carve-out. + expect(mockLinearService.detectInputType).not.toHaveBeenCalled() + expect(mockLinearService.fetchIssue).not.toHaveBeenCalled() + expect(MockedGitHubService.prototype.fetchPR).toHaveBeenCalledWith(99, undefined) + }) + + it('should reject a GitHub issue URL when configured tracker is Linear (provider mismatch)', async () => { + const mockLinearService = { + supportsPullRequests: false, + providerName: 'linear', + detectInputType: vi.fn(), + fetchIssue: vi.fn(), + validateIssueState: vi.fn(), + normalizeIdentifier: vi.fn((id: string | number) => String(id).toUpperCase()), + } + const linearCommand = new StartCommand(mockLinearService as unknown as GitHubService) + + await expect( + linearCommand.execute({ + identifier: 'https://github.com/owner/repo/issues/123', + options: {}, + }) + ).rejects.toThrow(/does not match the configured tracker/i) + + expect(mockLinearService.fetchIssue).not.toHaveBeenCalled() + }) + + it('should parse a Linear URL into uppercase TEAM-NUM', async () => { + const mockLinearService = { + supportsPullRequests: false, + providerName: 'linear', + detectInputType: vi.fn(), + fetchIssue: vi.fn().mockResolvedValue({ + number: 'WEB-2423', + title: 'Test', + body: '', + state: 'open', + labels: [], + assignees: [], + url: 'https://linear.app/myco/issue/WEB-2423', + }), + validateIssueState: vi.fn().mockResolvedValue(undefined), + normalizeIdentifier: vi.fn((id: string | number) => String(id).toUpperCase()), + } + const linearCommand = new StartCommand(mockLinearService as unknown as GitHubService) + + // Lowercase team in URL should be canonicalized to upper-case. + await expect( + linearCommand.execute({ + identifier: 'https://linear.app/myco/issue/web-2423/some-slug', + options: {}, + }) + ).resolves.not.toThrow() + + expect(mockLinearService.fetchIssue).toHaveBeenCalledWith('WEB-2423', undefined) + }) + + it('should parse a Jira cloud URL into uppercase KEY-NUM', async () => { + const mockJiraService = { + supportsPullRequests: false, + providerName: 'jira', + detectInputType: vi.fn(), + fetchIssue: vi.fn().mockResolvedValue({ + number: 'PROJ-99', + title: 'Test', + body: '', + state: 'open', + labels: [], + assignees: [], + url: 'https://myco.atlassian.net/browse/PROJ-99', + }), + validateIssueState: vi.fn().mockResolvedValue(undefined), + normalizeIdentifier: vi.fn((id: string | number) => String(id).toUpperCase()), + } + const jiraCommand = new StartCommand(mockJiraService as unknown as GitHubService) + + await expect( + jiraCommand.execute({ + identifier: 'https://myco.atlassian.net/browse/proj-99', + options: {}, + }) + ).resolves.not.toThrow() + + expect(mockJiraService.fetchIssue).toHaveBeenCalledWith('PROJ-99', undefined) + }) + + it('should reject a Jira URL whose host does not match the configured Jira host', async () => { + // Configure a Jira host of 'myco.atlassian.net' via the SettingsManager mock. + const SettingsManagerMock = vi.mocked(SettingsManager) + SettingsManagerMock.mockImplementationOnce(() => ({ + loadSettings: vi.fn().mockResolvedValue({ + issueManagement: { jira: { host: 'myco.atlassian.net' } }, + }), + }) as unknown as SettingsManager) + + const mockJiraService = { + supportsPullRequests: false, + providerName: 'jira', + detectInputType: vi.fn(), + fetchIssue: vi.fn(), + validateIssueState: vi.fn(), + normalizeIdentifier: vi.fn((id: string | number) => String(id).toUpperCase()), + } + const jiraCommand = new StartCommand(mockJiraService as unknown as GitHubService) + + await expect( + jiraCommand.execute({ + identifier: 'https://other-org.atlassian.net/browse/ENG-123', + options: {}, + }) + ).rejects.toThrow(/Jira host mismatch/i) + + expect(mockJiraService.fetchIssue).not.toHaveBeenCalled() + }) + + it('should parse a self-hosted Jira URL', async () => { + const mockJiraService = { + supportsPullRequests: false, + providerName: 'jira', + detectInputType: vi.fn(), + fetchIssue: vi.fn().mockResolvedValue({ + number: 'OPS-7', + title: 'Test', + body: '', + state: 'open', + labels: [], + assignees: [], + url: 'https://jira.internal.example.com/browse/OPS-7', + }), + validateIssueState: vi.fn().mockResolvedValue(undefined), + normalizeIdentifier: vi.fn((id: string | number) => String(id).toUpperCase()), + } + const jiraCommand = new StartCommand(mockJiraService as unknown as GitHubService) + + await expect( + jiraCommand.execute({ + identifier: 'https://jira.internal.example.com/browse/OPS-7', + options: {}, + }) + ).resolves.not.toThrow() + + expect(mockJiraService.fetchIssue).toHaveBeenCalledWith('OPS-7', undefined) + }) + + it('should strip query strings, fragments, and trailing slashes', async () => { + vi.mocked(mockGitHubService.fetchIssue).mockResolvedValue({ + number: 555, + title: 'Test', + body: '', + state: 'open', + labels: [], + assignees: [], + url: 'https://github.com/owner/repo/issues/555', + }) + vi.mocked(mockGitHubService.validateIssueState).mockResolvedValue() + + await expect( + command.execute({ + identifier: 'https://github.com/owner/repo/issues/555/?foo=bar#comment-1', + options: {}, + }) + ).resolves.not.toThrow() + + expect(mockGitHubService.fetchIssue).toHaveBeenCalledWith(555, undefined) + }) + + it('should throw a user-friendly error for a malformed GitHub URL', async () => { + await expect( + command.execute({ + identifier: 'https://github.com/owner/repo/wat/123', + options: {}, + }) + ).rejects.toThrow(/Invalid tracker URL/i) + }) + + it('should reject cross-repo GitHub URLs', async () => { + // Configure repo to be 'configured-owner/configured-repo'. + const { hasMultipleRemotes, getConfiguredRepoFromSettings } = await import( + '../utils/remote.js' + ) + vi.mocked(hasMultipleRemotes).mockResolvedValue(true) + vi.mocked(getConfiguredRepoFromSettings).mockResolvedValue( + 'configured-owner/configured-repo' + ) + + await expect( + command.execute({ + identifier: 'https://github.com/other-owner/other-repo/issues/1', + options: {}, + }) + ).rejects.toThrow(/operates on the current repository only/i) + }) + + it('should accept GitHub URLs whose repo case-insensitively matches the configured repo', async () => { + const { hasMultipleRemotes, getConfiguredRepoFromSettings } = await import( + '../utils/remote.js' + ) + vi.mocked(hasMultipleRemotes).mockResolvedValue(true) + vi.mocked(getConfiguredRepoFromSettings).mockResolvedValue('Owner/Repo') + + vi.mocked(mockGitHubService.fetchIssue).mockResolvedValue({ + number: 1, + title: 'Test', + body: '', + state: 'open', + labels: [], + assignees: [], + url: 'https://github.com/owner/repo/issues/1', + }) + vi.mocked(mockGitHubService.validateIssueState).mockResolvedValue() + + await expect( + command.execute({ + identifier: 'https://github.com/OWNER/REPO/issues/1', + options: {}, + }) + ).resolves.not.toThrow() + + expect(mockGitHubService.fetchIssue).toHaveBeenCalledWith(1, 'Owner/Repo') + }) + }) + }) describe('validation', () => { @@ -1640,6 +1949,7 @@ describe('StartCommand', () => { one_shot_mode: 'default', complexity_override: false, create_only: false, + identifier_source: 'identifier', }) }) @@ -1692,6 +2002,55 @@ describe('StartCommand', () => { expect(mockTrack).not.toHaveBeenCalled() }) + + it('should mark identifier_source as "identifier" for bare identifiers', async () => { + vi.mocked(mockGitHubService.detectInputType).mockResolvedValue({ + type: 'issue', + number: 123, + rawInput: '123', + }) + + await command.execute({ + identifier: '123', + options: {}, + }) + + expect(mockTrack).toHaveBeenCalledWith( + 'loom.created', + expect.objectContaining({ + identifier_source: 'identifier', + }) + ) + // Should NOT log url_provider for bare identifier inputs. + const call = mockTrack.mock.calls.find(([eventName]) => eventName === 'loom.created') + expect(call?.[1]).not.toHaveProperty('url_provider') + }) + + it('should mark identifier_source as "url" with url_provider for tracker URL inputs', async () => { + vi.mocked(mockGitHubService.fetchIssue).mockResolvedValue({ + number: 123, + title: 'Test', + body: '', + state: 'open', + labels: [], + assignees: [], + url: 'https://github.com/owner/repo/issues/123', + }) + vi.mocked(mockGitHubService.validateIssueState).mockResolvedValue() + + await command.execute({ + identifier: 'https://github.com/owner/repo/issues/123', + options: {}, + }) + + expect(mockTrack).toHaveBeenCalledWith( + 'loom.created', + expect.objectContaining({ + identifier_source: 'url', + url_provider: 'github', + }) + ) + }) }) describe('worktree directory behavior', () => { diff --git a/src/commands/start.ts b/src/commands/start.ts index 154fe2c0..0f835431 100644 --- a/src/commands/start.ts +++ b/src/commands/start.ts @@ -14,6 +14,8 @@ import { AgentManager } from '../lib/AgentManager.js' import { DatabaseManager } from '../lib/DatabaseManager.js' import { findMainWorktreePathWithSettings } from '../utils/git.js' import { matchIssueIdentifier } from '../utils/IdentifierParser.js' +import { parseTrackerUrl, TrackerUrlError, type TrackerUrlParseResult } from '../utils/TrackerUrlParser.js' +import { validateTrackerUrlAgainstSettings } from '../utils/tracker-url-validation.js' import { loadEnvIntoProcess } from '../utils/env.js' import { extractSettingsOverrides } from '../utils/cli-overrides.js' import { createNeonProviderFromSettings } from '../utils/neon-helpers.js' @@ -39,6 +41,13 @@ export interface ParsedInput { number?: string | number branchName?: string originalInput: string + /** + * Whether the input was a tracker URL or a bare identifier. Used for + * telemetry only — never include the URL itself in telemetry. + */ + identifierSource?: 'url' | 'identifier' + /** When `identifierSource === 'url'`, the tracker provider parsed from it. */ + urlProvider?: TrackerUrlParseResult['provider'] } export class StartCommand { @@ -168,8 +177,9 @@ export class StartCommand { // Step 0.6: Detect if running from inside an existing loom (for nested loom support) let parentLoom = await this.detectParentLoom(loomManager) - // Step 1: Parse and validate input (pass repo to methods) - const parsed = await this.parseInput(input.identifier, repo, input.options) + // Step 1: Parse and validate input (pass repo + configured Jira host to methods) + const configuredJiraHost = initialSettings.issueManagement?.jira?.host + const parsed = await this.parseInput(input.identifier, repo, input.options, configuredJiraHost) // Step 2: Validate based on type await this.validateInput(parsed, repo) @@ -396,6 +406,8 @@ export class StartCommand { noReview: 'skip-reviews', bypassPermissions: 'yolo', } + const identifierSource: LoomCreatedProperties['identifier_source'] = + parsed.identifierSource ?? 'identifier' TelemetryService.getInstance().track('loom.created', { source_type: parsed.type === 'epic' ? 'issue' : parsed.type as LoomCreatedProperties['source_type'], tracker: this.issueTracker.providerName, @@ -403,6 +415,8 @@ export class StartCommand { one_shot_mode: oneShotMap[input.options.oneShot ?? ''] ?? 'default', complexity_override: !!input.options.complexity, create_only: !!input.options.createOnly, + identifier_source: identifierSource, + ...(parsed.urlProvider && { url_provider: parsed.urlProvider }), }) } catch (error: unknown) { getLogger().debug(`Failed to track loom.created telemetry: ${error instanceof Error ? error.message : String(error)}`) @@ -447,7 +461,7 @@ export class StartCommand { /** * Parse input to determine type and extract relevant data */ - private async parseInput(identifier: string, repo?: string, options?: StartOptions): Promise { + private async parseInput(identifier: string, repo?: string, options?: StartOptions, configuredJiraHost?: string): Promise { // Check if user wants to skip capitalization by prefixing with space // We preserve this for description types so capitalizeFirstLetter() can handle it const hasLeadingSpace = identifier.startsWith(' ') @@ -458,6 +472,14 @@ export class StartCommand { throw new Error('Missing required argument: identifier') } + // Check if input is a tracker URL (GitHub issue/PR, Linear, Jira) and + // translate it to a canonical identifier before falling through to the + // existing identifier matching paths below. + const urlParsed = this.tryParseTrackerUrl(trimmedIdentifier, repo, configuredJiraHost) + if (urlParsed) { + return urlParsed + } + // Check for description: >15 chars AND has spaces (likely a natural language description) // Also treat as description if --body is provided with a multi-word input, // since the VS Code extension passes short titles with a --body flag for new issue creation @@ -565,6 +587,76 @@ export class StartCommand { } } + /** + * Attempt to parse `input` as a tracker URL (GitHub issue/PR, Linear, + * Jira). Returns a `ParsedInput` if the input was a recognized URL, `null` + * if it was not URL-shaped (caller should fall through to bare-identifier + * matching), or throws a user-facing error if the URL was malformed or + * doesn't match the configured tracker / current repo. + * + * Carve-out: GitHub PR URLs are accepted regardless of the configured + * tracker, mirroring the bare-numeric PR detection at start.ts ~line 537 + * which lets Linear/Jira projects open GitHub PR worktrees. + */ + private tryParseTrackerUrl(input: string, configuredRepo?: string, configuredJiraHost?: string): ParsedInput | null { + let result: TrackerUrlParseResult | null + try { + result = parseTrackerUrl(input) + } catch (error: unknown) { + if (error instanceof TrackerUrlError) { + throw new Error(`Invalid tracker URL: ${error.message}`) + } + throw error + } + + if (!result) { + return null + } + + const trackerProviderName = this.issueTracker.providerName + + // Provider mismatch + Jira host mismatch validation (shared with `il plan`). + // `allowPrCarveOut: true` lets GitHub PR URLs through regardless of the + // configured tracker — see start.ts:537-557 for the bare-numeric equivalent. + validateTrackerUrlAgainstSettings(result, { + configuredProvider: trackerProviderName as TrackerUrlParseResult['provider'], + ...(configuredJiraHost !== undefined && { configuredJiraHost }), + allowPrCarveOut: true, + }) + + // Cross-repo URLs are rejected for `il start`: the worktree is created + // in the current repo, so a URL pointing at a different repo would + // silently start work on the wrong codebase. (See per-command policy + // in the issue plan.) Compare case-insensitively because GitHub repo + // slugs are case-insensitive in practice. + if (result.repo && configuredRepo) { + if (result.repo.toLowerCase() !== configuredRepo.toLowerCase()) { + throw new Error( + `Tracker URL points to "${result.repo}" but iloom is configured for "${configuredRepo}". ` + + `'il start' operates on the current repository only — switch to the target repo first or use a bare identifier.` + ) + } + } + + // Defensive normalization: TrackerUrlParser already returns canonical + // case, but legacy on-disk identifiers may be in a different case, so + // pipe through `normalizeIdentifier()` per the CLAUDE.md + // identifier-comparison rule. + const normalized = this.issueTracker.normalizeIdentifier(result.identifier) + + // GitHub identifiers are numeric; everything else stays as a string. + const numberValue: string | number = + result.provider === 'github' ? parseInt(normalized, 10) : normalized + + return { + type: result.kind === 'pr' ? 'pr' : 'issue', + number: numberValue, + originalInput: input, + identifierSource: 'url', + urlProvider: result.provider, + } + } + /** * Validate the parsed input based on its type */ diff --git a/src/lib/LinearService.ts b/src/lib/LinearService.ts index 6f8f3e47..18bc2e95 100644 --- a/src/lib/LinearService.ts +++ b/src/lib/LinearService.ts @@ -55,13 +55,17 @@ export class LinearService implements IssueTracker { /** * Detect if input matches Linear identifier format (TEAM-NUMBER) * @param input - User input string - * @param _repo - Repository (unused for Linear) + * @param repo - Repository override (ignored — Linear does not support cross-repo + * addressing via owner/repo; logged at debug level if provided) * @returns Detection result with type and identifier */ public async detectInputType( input: string, - _repo?: string, + repo?: string, ): Promise { + if (repo) { + getLogger().debug(`LinearService.detectInputType: ignoring repo argument "${repo}" — Linear does not support cross-repo addressing.`) + } getLogger().debug(`LinearService.detectInputType called with input: "${input}"`) // Pattern: TEAM-NUMBER (e.g., ENG-123, PLAT-456) @@ -94,11 +98,15 @@ export class LinearService implements IssueTracker { /** * Fetch a Linear issue by identifier * @param identifier - Linear issue identifier (string or number) - * @param _repo - Repository (unused for Linear) + * @param repo - Repository override (ignored — Linear does not support cross-repo + * addressing via owner/repo; logged at debug level if provided) * @returns Generic Issue type * @throws LinearServiceError if issue not found */ - public async fetchIssue(identifier: string | number, _repo?: string): Promise { + public async fetchIssue(identifier: string | number, repo?: string): Promise { + if (repo) { + getLogger().debug(`LinearService.fetchIssue: ignoring repo argument "${repo}" — Linear does not support cross-repo addressing.`) + } const linearIssue = await fetchLinearIssue(String(identifier)) return this.mapLinearIssueToIssue(linearIssue) } diff --git a/src/lib/providers/jira/JiraIssueTracker.ts b/src/lib/providers/jira/JiraIssueTracker.ts index ce6b90a6..65f76340 100644 --- a/src/lib/providers/jira/JiraIssueTracker.ts +++ b/src/lib/providers/jira/JiraIssueTracker.ts @@ -94,8 +94,15 @@ export class JiraIssueTracker implements IssueTracker { /** * Detect input type from user input * Jira issues follow pattern: PROJECTKEY-123 (case-insensitive) + * + * @param input User input (identifier or URL-derived identifier) + * @param repo Optional repo override (ignored — Jira does not support cross-repo + * addressing via owner/repo. Logged at debug level if provided.) */ - async detectInputType(input: string): Promise { + async detectInputType(input: string, repo?: string): Promise { + if (repo) { + getLogger().debug(`JiraIssueTracker.detectInputType: ignoring repo argument "${repo}" — Jira does not support cross-repo addressing.`) + } // Pattern: PROJECTKEY-123 (case-insensitive to accept lowercase from branch names or user input) const jiraPattern = /^([A-Z][A-Z0-9]+)-(\d+)$/i const match = input.match(jiraPattern) @@ -122,8 +129,15 @@ export class JiraIssueTracker implements IssueTracker { /** * Fetch issue details + * + * @param identifier Jira issue key or numeric id + * @param repo Optional repo override (ignored — Jira does not support cross-repo + * addressing via owner/repo. Logged at debug level if provided.) */ - async fetchIssue(identifier: string | number): Promise { + async fetchIssue(identifier: string | number, repo?: string): Promise { + if (repo) { + getLogger().debug(`JiraIssueTracker.fetchIssue: ignoring repo argument "${repo}" — Jira does not support cross-repo addressing.`) + } const issueKey = this.normalizeIdentifier(identifier) getLogger().debug('Fetching Jira issue', { issueKey }) @@ -134,7 +148,7 @@ export class JiraIssueTracker implements IssueTracker { /** * Check if issue exists (silent validation) */ - async isValidIssue(identifier: string | number): Promise { + async isValidIssue(identifier: string | number, _repo?: string): Promise { try { return await this.fetchIssue(identifier) } catch (error) { diff --git a/src/types/telemetry.ts b/src/types/telemetry.ts index 453b001d..b494ec84 100644 --- a/src/types/telemetry.ts +++ b/src/types/telemetry.ts @@ -29,6 +29,16 @@ export interface LoomCreatedProperties { one_shot_mode: 'default' | 'skip-reviews' | 'yolo' complexity_override: boolean create_only: boolean + /** + * Whether the loom was started from a tracker URL or a bare identifier. + * Privacy: only the source enum is logged — never the URL itself. + */ + identifier_source?: 'url' | 'identifier' + /** + * When `identifier_source === 'url'`, the tracker provider extracted from + * the URL. Helps surface adoption of the URL-parsing feature per tracker. + */ + url_provider?: 'github' | 'linear' | 'jira' } export interface LoomFinishedProperties { @@ -44,6 +54,11 @@ export interface LoomAbandonedProperties { export interface EpicPlannedProperties { child_count: number tracker: string + /** + * Whether the epic was started from a tracker URL or a bare identifier. + * Privacy: only the source enum is logged — never the URL itself. + */ + identifier_source?: 'url' | 'identifier' } export interface SwarmStartedProperties { @@ -99,6 +114,11 @@ export interface InitCompletedProperties { export interface AutoSwarmStartedProperties { source: 'decomposition' | 'fresh' planner: string // 'claude' | 'gemini' | 'codex' + /** + * Whether the auto-swarm was triggered from a tracker URL or a bare + * identifier. Privacy: only the source enum is logged — never the URL. + */ + identifier_source?: 'url' | 'identifier' } export interface AutoSwarmCompletedProperties { diff --git a/src/utils/TrackerUrlParser.test.ts b/src/utils/TrackerUrlParser.test.ts new file mode 100644 index 00000000..36bdc02e --- /dev/null +++ b/src/utils/TrackerUrlParser.test.ts @@ -0,0 +1,350 @@ +import { describe, it, expect } from 'vitest' +import { + parseTrackerUrl, + TrackerUrlError, + type TrackerUrlParseResult, +} from './TrackerUrlParser.js' + +describe('parseTrackerUrl', () => { + describe('GitHub URLs', () => { + it.each<[string, TrackerUrlParseResult]>([ + [ + 'https://github.com/owner/repo/issues/123', + { + provider: 'github', + kind: 'issue', + identifier: '123', + repo: 'owner/repo', + host: 'github.com', + }, + ], + [ + 'https://github.com/owner/repo/pull/456', + { + provider: 'github', + kind: 'pr', + identifier: '456', + repo: 'owner/repo', + host: 'github.com', + }, + ], + [ + 'http://github.com/owner/repo/issues/1', + { + provider: 'github', + kind: 'issue', + identifier: '1', + repo: 'owner/repo', + host: 'github.com', + }, + ], + ])('parses %s', (input, expected) => { + expect(parseTrackerUrl(input)).toEqual(expected) + }) + + it('strips trailing slash', () => { + expect(parseTrackerUrl('https://github.com/owner/repo/issues/123/')).toEqual({ + provider: 'github', + kind: 'issue', + identifier: '123', + repo: 'owner/repo', + host: 'github.com', + }) + }) + + it('strips query string', () => { + const result = parseTrackerUrl( + 'https://github.com/owner/repo/issues/123?ref=foo' + ) + expect(result?.identifier).toBe('123') + expect(result?.repo).toBe('owner/repo') + }) + + it('strips URL fragment', () => { + const result = parseTrackerUrl( + 'https://github.com/owner/repo/issues/123#issuecomment-789' + ) + expect(result?.identifier).toBe('123') + }) + + it('matches keyword case-insensitively', () => { + const result = parseTrackerUrl('https://github.com/owner/repo/Issues/123') + expect(result?.kind).toBe('issue') + expect(result?.identifier).toBe('123') + }) + + it('matches host case-insensitively', () => { + const result = parseTrackerUrl('https://GitHub.com/owner/repo/issues/123') + expect(result?.provider).toBe('github') + expect(result?.identifier).toBe('123') + }) + + it('strips wrapping <...> markdown autolink', () => { + const result = parseTrackerUrl( + '' + ) + expect(result?.identifier).toBe('123') + }) + + it('strips trailing punctuation', () => { + expect( + parseTrackerUrl('https://github.com/owner/repo/issues/123,')?.identifier + ).toBe('123') + expect( + parseTrackerUrl('https://github.com/owner/repo/issues/123.')?.identifier + ).toBe('123') + expect( + parseTrackerUrl('https://github.com/owner/repo/issues/123;')?.identifier + ).toBe('123') + expect( + parseTrackerUrl('https://github.com/owner/repo/issues/123)')?.identifier + ).toBe('123') + }) + + it('strips user-info credentials from URL', () => { + const result = parseTrackerUrl( + 'https://user:token@github.com/owner/repo/issues/123' + ) + expect(result).toEqual({ + provider: 'github', + kind: 'issue', + identifier: '123', + repo: 'owner/repo', + host: 'github.com', + }) + }) + + it('handles www.github.com host', () => { + const result = parseTrackerUrl( + 'https://www.github.com/owner/repo/issues/123' + ) + expect(result?.provider).toBe('github') + expect(result?.identifier).toBe('123') + }) + + it('throws TrackerUrlError when number is missing', () => { + expect(() => + parseTrackerUrl('https://github.com/owner/repo/issues/') + ).toThrow(TrackerUrlError) + }) + + it('throws TrackerUrlError when path number is non-numeric', () => { + expect(() => + parseTrackerUrl('https://github.com/owner/repo/issues/abc') + ).toThrow(TrackerUrlError) + }) + + it('throws TrackerUrlError on unrecognized GitHub path (e.g. discussions)', () => { + expect(() => + parseTrackerUrl('https://github.com/owner/repo/discussions/123') + ).toThrow(TrackerUrlError) + }) + + it('throws TrackerUrlError on truncated GitHub URL (no /issues|/pull)', () => { + expect(() => parseTrackerUrl('https://github.com/owner/repo')).toThrow( + TrackerUrlError + ) + }) + }) + + describe('Linear URLs', () => { + it('parses basic Linear issue URL', () => { + expect(parseTrackerUrl('https://linear.app/team/issue/WEB-2423')).toEqual({ + provider: 'linear', + kind: 'issue', + identifier: 'WEB-2423', + host: 'linear.app', + }) + }) + + it('strips trailing slug', () => { + const result = parseTrackerUrl( + 'https://linear.app/team/issue/WEB-2423/some-title-slug' + ) + expect(result?.identifier).toBe('WEB-2423') + }) + + it('uppercases lowercase identifier', () => { + const result = parseTrackerUrl( + 'https://linear.app/team/issue/web-2423' + ) + expect(result?.identifier).toBe('WEB-2423') + }) + + it('matches keyword case-insensitively', () => { + const result = parseTrackerUrl( + 'https://linear.app/team/Issue/WEB-2423' + ) + expect(result?.identifier).toBe('WEB-2423') + }) + + it('does NOT extract project key from slug (slug may contain dashes that look like keys)', () => { + const result = parseTrackerUrl( + 'https://linear.app/team/issue/WEB-2423/fix-PROJ-99-bug' + ) + expect(result?.identifier).toBe('WEB-2423') + }) + + it('strips query string and fragment', () => { + const result = parseTrackerUrl( + 'https://linear.app/team/issue/WEB-2423?foo=bar#comment-1' + ) + expect(result?.identifier).toBe('WEB-2423') + }) + + it('throws TrackerUrlError when identifier segment is missing', () => { + expect(() => + parseTrackerUrl('https://linear.app/team/issue/') + ).toThrow(TrackerUrlError) + }) + + it('throws TrackerUrlError when identifier segment is invalid', () => { + expect(() => + parseTrackerUrl('https://linear.app/team/issue/INVALID/foo') + ).toThrow(TrackerUrlError) + }) + + it('throws TrackerUrlError when path has no /issue/ segment', () => { + expect(() => + parseTrackerUrl('https://linear.app/team/projects/foo') + ).toThrow(TrackerUrlError) + }) + + it('accepts 1-character team keys', () => { + expect( + parseTrackerUrl('https://linear.app/team/issue/A-1/foo') + ).toEqual({ + provider: 'linear', + kind: 'issue', + identifier: 'A-1', + host: 'linear.app', + }) + }) + + it('throws TrackerUrlError when number segment is missing', () => { + expect(() => + parseTrackerUrl('https://linear.app/team/issue/WEB-/foo') + ).toThrow(TrackerUrlError) + }) + }) + + describe('Jira URLs', () => { + it('parses Atlassian Cloud URL', () => { + expect( + parseTrackerUrl('https://myco.atlassian.net/browse/PROJ-99') + ).toEqual({ + provider: 'jira', + kind: 'issue', + identifier: 'PROJ-99', + host: 'myco.atlassian.net', + }) + }) + + it('parses self-hosted Jira URL', () => { + expect( + parseTrackerUrl('https://jira.example.com/browse/PROJ-99') + ).toEqual({ + provider: 'jira', + kind: 'issue', + identifier: 'PROJ-99', + host: 'jira.example.com', + }) + }) + + it('preserves host casing for downstream comparison', () => { + const result = parseTrackerUrl( + 'https://MyCo.Atlassian.net/browse/PROJ-99' + ) + // URL constructor lowercases the hostname automatically. + expect(result?.host).toBe('myco.atlassian.net') + }) + + it('uppercases lowercase identifier', () => { + const result = parseTrackerUrl( + 'https://myco.atlassian.net/browse/proj-99' + ) + expect(result?.identifier).toBe('PROJ-99') + }) + + it('strips query string', () => { + const result = parseTrackerUrl( + 'https://myco.atlassian.net/browse/PROJ-99?focusedCommentId=12345' + ) + expect(result?.identifier).toBe('PROJ-99') + }) + + it('strips trailing slash', () => { + const result = parseTrackerUrl( + 'https://myco.atlassian.net/browse/PROJ-99/' + ) + expect(result?.identifier).toBe('PROJ-99') + }) + + it('throws TrackerUrlError when identifier is malformed', () => { + expect(() => + parseTrackerUrl('https://myco.atlassian.net/browse/INVALID') + ).toThrow(TrackerUrlError) + }) + + it('throws TrackerUrlError on Atlassian Cloud host with non-/browse path', () => { + expect(() => + parseTrackerUrl('https://myco.atlassian.net/projects/PROJ') + ).toThrow(TrackerUrlError) + }) + }) + + describe('non-URL / pass-through inputs', () => { + it.each(['123', '#456', 'WEB-2423', 'feat/branch', ' ', ''])( + 'returns null for plain identifier %j', + (input) => { + expect(parseTrackerUrl(input)).toBeNull() + } + ) + + it('returns null for unknown tracker host (gitlab.com)', () => { + expect( + parseTrackerUrl('https://gitlab.com/owner/repo/-/issues/1') + ).toBeNull() + }) + + it('returns null for non-tracker URL on unknown host', () => { + expect(parseTrackerUrl('https://example.com/something/123')).toBeNull() + }) + + it('returns null for malformed URL with http(s) prefix', () => { + expect(parseTrackerUrl('https://')).toBeNull() + expect(parseTrackerUrl('https:// invalid url')).toBeNull() + }) + + it('returns null for non-string input', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect(parseTrackerUrl(undefined as any)).toBeNull() + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect(parseTrackerUrl(null as any)).toBeNull() + }) + }) + + describe('TrackerUrlError', () => { + it('is an instance of Error', () => { + const err = new TrackerUrlError('test') + expect(err).toBeInstanceOf(Error) + expect(err).toBeInstanceOf(TrackerUrlError) + expect(err.name).toBe('TrackerUrlError') + }) + + it('captures host when provided', () => { + const err = new TrackerUrlError('test', 'github.com') + expect(err.host).toBe('github.com') + }) + + it('captured host is set on thrown error from parser', () => { + try { + parseTrackerUrl('https://github.com/owner/repo/discussions/123') + expect.fail('expected to throw') + } catch (err) { + expect(err).toBeInstanceOf(TrackerUrlError) + expect((err as TrackerUrlError).host).toBe('github.com') + } + }) + }) +}) diff --git a/src/utils/TrackerUrlParser.ts b/src/utils/TrackerUrlParser.ts new file mode 100644 index 00000000..6e90e0a7 --- /dev/null +++ b/src/utils/TrackerUrlParser.ts @@ -0,0 +1,236 @@ +import { logger } from './logger.js' + +/** + * Provider type extracted from a tracker URL. + */ +export type TrackerUrlProvider = 'github' | 'linear' | 'jira' + +/** + * Whether the URL points to an issue or a pull request. + */ +export type TrackerUrlKind = 'issue' | 'pr' + +/** + * Result of parsing a tracker URL. + * + * The `identifier` is emitted in canonical case for the provider: + * - GitHub: numeric string (e.g. '123') + * - Linear: uppercase TEAM-NUM (e.g. 'WEB-2423') + * - Jira: uppercase KEY-NUM (e.g. 'PROJ-99') + * + * Callers that perform identifier comparisons against stored data should + * still pass the identifier through `IssueTracker.normalizeIdentifier()` + * to handle legacy data that may already be on disk in a different case. + */ +export interface TrackerUrlParseResult { + provider: TrackerUrlProvider + kind: TrackerUrlKind + identifier: string + /** owner/repo for GitHub URLs only */ + repo?: string + /** URL host (e.g. 'myco.atlassian.net'); useful for Jira host validation */ + host?: string +} + +/** + * Thrown when an input clearly intends to be a tracker URL (matches a known + * host or shape) but cannot be parsed into a valid identifier. Callers that + * cannot recognize a URL pattern at all should return `null` instead so the + * caller can fall back to the bare-identifier code path. + */ +export class TrackerUrlError extends Error { + public readonly host?: string + + constructor(message: string, host?: string) { + super(message) + this.name = 'TrackerUrlError' + if (host !== undefined) { + this.host = host + } + } +} + +const HTTP_PREFIX_RE = /^https?:\/\//i +// Linear allows 1-character team keys (e.g. `A-123`); Jira project keys are +// also typically >=2 chars but the API permits short keys, so use `*` to allow +// 1+ alphanumeric after the leading letter. +const PROJECT_KEY_RE = /^([A-Za-z][A-Za-z0-9]*)-(\d+)$/ + +/** + * Parse a tracker URL into a normalized result. + * + * @param input Raw user-provided string. May contain wrapping `<...>`, + * trailing punctuation, or auth credentials. + * @returns Parsed tracker info, or `null` if the input is not a recognized + * tracker URL (callers can then try bare-identifier matching). + * @throws TrackerUrlError if the input clearly intends to be a tracker URL + * (matches a known host) but is malformed. + */ +export function parseTrackerUrl(input: string): TrackerUrlParseResult | null { + if (typeof input !== 'string') { + return null + } + + const cleaned = stripWrapping(input) + if (cleaned.length === 0) { + return null + } + + if (!HTTP_PREFIX_RE.test(cleaned)) { + return null + } + + let url: URL + try { + url = new URL(cleaned) + } catch { + // Looked like a URL (has http(s):// prefix) but URL constructor rejected + // it. This is "clearly intended to be a URL but malformed" — but we don't + // know the host so we can't tell whether it's a tracker URL. Return null + // so the caller falls through to bare-identifier matching. + return null + } + + if (url.username || url.password) { + // Strip credentials defensively — never log the raw URL. + logger.debug('parseTrackerUrl: URL had credentials, stripped') + url.username = '' + url.password = '' + } + + const host = url.hostname.toLowerCase() + + if (host === 'github.com' || host === 'www.github.com') { + return parseGitHub(url) + } + + if (host === 'linear.app' || host === 'www.linear.app') { + return parseLinear(url) + } + + // Jira: detect by path shape `/browse/KEY-NUM`. Both Atlassian Cloud and + // self-hosted Jira use the `/browse/...` URL shape. + if (/^\/browse\/[^/]+\/?$/.test(url.pathname)) { + return parseJira(url) + } + + // Atlassian Cloud host with a non-`/browse/` path → still clearly a Jira + // URL but malformed for our purposes. + if (host.endsWith('.atlassian.net')) { + throw new TrackerUrlError( + `Unrecognized Jira URL shape: ${url.pathname}. Expected /browse/.`, + url.hostname + ) + } + + // Unknown host → not a recognized tracker URL. Return null so the caller + // can fall back to bare-identifier matching. + return null +} + +function parseGitHub(url: URL): TrackerUrlParseResult { + // Path: ///(issues|pull)/(/...)? — keyword case-insensitive. + const match = url.pathname.match( + /^\/([^/]+)\/([^/]+)\/(issues|pull)\/(\d+)(?:\/.*)?$/i + ) + if (!match) { + throw new TrackerUrlError( + `Unrecognized GitHub URL shape: ${url.pathname}. Expected ///issues/ or ///pull/.`, + url.hostname + ) + } + const [, owner, repo, keyword, number] = match + if (!owner || !repo || !keyword || !number) { + // Should be unreachable given the regex above, but the type narrowing + // satisfies @typescript-eslint/no-non-null-assertion. + throw new TrackerUrlError( + `Malformed GitHub URL: ${url.pathname}.`, + url.hostname + ) + } + return { + provider: 'github', + kind: keyword.toLowerCase() === 'pull' ? 'pr' : 'issue', + identifier: number, + repo: `${owner}/${repo}`, + host: url.hostname, + } +} + +function parseLinear(url: URL): TrackerUrlParseResult { + // Path: //issue/(/)? + // CRITICAL: extract identifier from the path segment immediately after + // /issue/, NOT by regex-scanning the whole URL — slug segments may contain + // dashes that look like project keys (e.g. /issue/WEB-2423/fix-PROJ-99-bug). + const segments = url.pathname.split('/').filter((s) => s.length > 0) + const issueIdx = segments.findIndex((s) => s.toLowerCase() === 'issue') + if (issueIdx === -1) { + throw new TrackerUrlError( + `Unrecognized Linear URL shape: ${url.pathname}. Expected //issue/.`, + url.hostname + ) + } + const idSegment = segments[issueIdx + 1] + if (!idSegment) { + throw new TrackerUrlError( + `Malformed Linear URL: missing issue identifier in ${url.pathname}.`, + url.hostname + ) + } + const idMatch = idSegment.match(PROJECT_KEY_RE) + if (!idMatch?.[1] || !idMatch[2]) { + throw new TrackerUrlError( + `Malformed Linear URL: '${idSegment}' is not a valid Linear identifier (expected TEAM-NUM).`, + url.hostname + ) + } + const team = idMatch[1].toUpperCase() + const num = idMatch[2] + return { + provider: 'linear', + kind: 'issue', + identifier: `${team}-${num}`, + host: url.hostname, + } +} + +function parseJira(url: URL): TrackerUrlParseResult { + const match = url.pathname.match(/^\/browse\/([^/]+)\/?$/) + if (!match?.[1]) { + throw new TrackerUrlError( + `Malformed Jira URL: ${url.pathname}.`, + url.hostname + ) + } + const idSegment = match[1] + const idMatch = idSegment.match(PROJECT_KEY_RE) + if (!idMatch?.[1] || !idMatch[2]) { + throw new TrackerUrlError( + `Malformed Jira URL: '${idSegment}' is not a valid Jira identifier (expected KEY-NUM).`, + url.hostname + ) + } + const key = idMatch[1].toUpperCase() + const num = idMatch[2] + return { + provider: 'jira', + kind: 'issue', + identifier: `${key}-${num}`, + host: url.hostname, + } +} + +/** + * Strip surrounding whitespace, optional `<...>` wrappers (e.g. when pasted + * from Markdown autolinks), and trailing punctuation that often gets pasted + * along with URLs from prose. + */ +function stripWrapping(input: string): string { + let s = input.trim() + if (s.startsWith('<') && s.endsWith('>')) { + s = s.slice(1, -1).trim() + } + // Strip trailing punctuation that commonly follows URLs in prose. + s = s.replace(/[,;.!?)\]]+$/, '') + return s +} diff --git a/src/utils/tracker-url-validation.test.ts b/src/utils/tracker-url-validation.test.ts new file mode 100644 index 00000000..b348e768 --- /dev/null +++ b/src/utils/tracker-url-validation.test.ts @@ -0,0 +1,165 @@ +import { describe, it, expect } from 'vitest' +import { validateTrackerUrlAgainstSettings } from './tracker-url-validation.js' +import type { TrackerUrlParseResult } from './TrackerUrlParser.js' + +const ghIssue: TrackerUrlParseResult = { + provider: 'github', + kind: 'issue', + identifier: '123', + repo: 'owner/repo', + host: 'github.com', +} + +const ghPr: TrackerUrlParseResult = { + provider: 'github', + kind: 'pr', + identifier: '42', + repo: 'owner/repo', + host: 'github.com', +} + +const linearIssue: TrackerUrlParseResult = { + provider: 'linear', + kind: 'issue', + identifier: 'WEB-1', + host: 'linear.app', +} + +const jiraIssueMyco: TrackerUrlParseResult = { + provider: 'jira', + kind: 'issue', + identifier: 'PROJ-99', + host: 'myco.atlassian.net', +} + +const jiraIssueOther: TrackerUrlParseResult = { + provider: 'jira', + kind: 'issue', + identifier: 'ENG-123', + host: 'other-org.atlassian.net', +} + +describe('validateTrackerUrlAgainstSettings', () => { + describe('provider mismatch', () => { + it('throws when URL provider does not match configured provider', () => { + expect(() => + validateTrackerUrlAgainstSettings(linearIssue, { + configuredProvider: 'github', + }) + ).toThrow(/does not match the configured tracker/i) + }) + + it('does not throw when URL provider matches configured provider', () => { + expect(() => + validateTrackerUrlAgainstSettings(ghIssue, { + configuredProvider: 'github', + }) + ).not.toThrow() + }) + }) + + describe('PR carve-out', () => { + it('allows GitHub PR URL when allowPrCarveOut=true and configured tracker is Linear', () => { + expect(() => + validateTrackerUrlAgainstSettings(ghPr, { + configuredProvider: 'linear', + allowPrCarveOut: true, + }) + ).not.toThrow() + }) + + it('still rejects GitHub issue URL with allowPrCarveOut=true and configured tracker is Linear', () => { + expect(() => + validateTrackerUrlAgainstSettings(ghIssue, { + configuredProvider: 'linear', + allowPrCarveOut: true, + }) + ).toThrow(/does not match the configured tracker/i) + }) + + it('rejects GitHub PR URL when allowPrCarveOut=false and configured tracker is Linear', () => { + expect(() => + validateTrackerUrlAgainstSettings(ghPr, { + configuredProvider: 'linear', + allowPrCarveOut: false, + }) + ).toThrow(/does not match the configured tracker/i) + }) + + it('rejects GitHub PR URL when allowPrCarveOut is unset (defaults to false)', () => { + expect(() => + validateTrackerUrlAgainstSettings(ghPr, { + configuredProvider: 'linear', + }) + ).toThrow(/does not match the configured tracker/i) + }) + }) + + describe('Jira host validation', () => { + it('throws when configured Jira host differs from URL host', () => { + expect(() => + validateTrackerUrlAgainstSettings(jiraIssueOther, { + configuredProvider: 'jira', + configuredJiraHost: 'myco.atlassian.net', + }) + ).toThrow(/Jira host mismatch/i) + }) + + it('does not throw when configured Jira host matches URL host', () => { + expect(() => + validateTrackerUrlAgainstSettings(jiraIssueMyco, { + configuredProvider: 'jira', + configuredJiraHost: 'myco.atlassian.net', + }) + ).not.toThrow() + }) + + it('compares Jira host case-insensitively', () => { + expect(() => + validateTrackerUrlAgainstSettings(jiraIssueMyco, { + configuredProvider: 'jira', + configuredJiraHost: 'MYCO.atlassian.NET', + }) + ).not.toThrow() + }) + + it('accepts configured Jira host given as full URL', () => { + expect(() => + validateTrackerUrlAgainstSettings(jiraIssueMyco, { + configuredProvider: 'jira', + configuredJiraHost: 'https://myco.atlassian.net', + }) + ).not.toThrow() + }) + + it('treats trailing dot in URL host as equivalent to configured host', () => { + const withTrailingDot: TrackerUrlParseResult = { + ...jiraIssueMyco, + host: 'myco.atlassian.net.', + } + expect(() => + validateTrackerUrlAgainstSettings(withTrailingDot, { + configuredProvider: 'jira', + configuredJiraHost: 'myco.atlassian.net', + }) + ).not.toThrow() + }) + + it('skips Jira host check when no configuredJiraHost provided', () => { + expect(() => + validateTrackerUrlAgainstSettings(jiraIssueOther, { + configuredProvider: 'jira', + }) + ).not.toThrow() + }) + + it('does not run Jira host check for non-Jira URLs', () => { + expect(() => + validateTrackerUrlAgainstSettings(ghIssue, { + configuredProvider: 'github', + configuredJiraHost: 'myco.atlassian.net', + }) + ).not.toThrow() + }) + }) +}) diff --git a/src/utils/tracker-url-validation.ts b/src/utils/tracker-url-validation.ts new file mode 100644 index 00000000..9ea51696 --- /dev/null +++ b/src/utils/tracker-url-validation.ts @@ -0,0 +1,88 @@ +import type { TrackerUrlParseResult, TrackerUrlProvider } from './TrackerUrlParser.js' + +/** + * Options controlling how a parsed tracker URL is validated against the + * configured iloom settings. + */ +export interface ValidateTrackerUrlOptions { + /** The tracker provider iloom is configured to use. */ + configuredProvider: TrackerUrlProvider + /** + * Optional configured Jira host (e.g. 'myco.atlassian.net' or + * 'https://myco.atlassian.net'). When set and the parsed URL is a Jira + * URL, the host must match (case-insensitive, ignoring trailing dot). + */ + configuredJiraHost?: string + /** + * When true, GitHub PR URLs are accepted regardless of the configured + * tracker (`il start` carve-out, mirroring the bare-numeric PR detection + * for Linear/Jira-tracked repos). When false (default), provider mismatches + * always throw. + */ + allowPrCarveOut?: boolean +} + +/** + * Validate a parsed tracker URL against the configured iloom settings. + * + * Throws a user-facing `Error` with a clear message when: + * - The URL's provider does not match the configured tracker (and the PR + * carve-out doesn't apply). + * - The URL is a Jira URL whose host does not match the configured Jira + * host (when one is configured). + * + * Repository (cross-repo) checks are intentionally NOT handled here: the + * policy differs between commands (`il start` rejects, `il plan` warns), + * so callers handle that separately. + */ +export function validateTrackerUrlAgainstSettings( + parsed: TrackerUrlParseResult, + opts: ValidateTrackerUrlOptions, +): void { + const { configuredProvider, configuredJiraHost, allowPrCarveOut } = opts + + const isGitHubPrCarveOut = + allowPrCarveOut === true && + parsed.provider === 'github' && + parsed.kind === 'pr' + + if (!isGitHubPrCarveOut && parsed.provider !== configuredProvider) { + throw new Error( + `Tracker URL provider "${parsed.provider}" does not match the configured tracker "${configuredProvider}". ` + + `Re-run with an identifier or URL for ${configuredProvider}, or update your iloom configuration.`, + ) + } + + if (parsed.provider === 'jira' && configuredJiraHost && parsed.host) { + const configuredHostname = normalizeJiraHost(configuredJiraHost) + const parsedHostname = stripTrailingDot(parsed.host.toLowerCase()) + if (parsedHostname !== configuredHostname) { + throw new Error( + `Jira host mismatch: URL host "${parsed.host}" does not match the configured Jira host "${configuredHostname}". ` + + `Update your iloom configuration or use a URL for the configured host.`, + ) + } + } +} + +/** + * Normalize a configured Jira host setting into a bare lowercase hostname. + * Accepts either a bare hostname (`myco.atlassian.net`) or a full URL + * (`https://myco.atlassian.net`). Trailing dots are stripped so DNS-style + * fully-qualified names compare equal to the parsed URL's hostname. + */ +function normalizeJiraHost(configured: string): string { + let hostname: string + try { + hostname = new URL( + configured.startsWith('http') ? configured : `https://${configured}`, + ).hostname.toLowerCase() + } catch { + hostname = configured.toLowerCase() + } + return stripTrailingDot(hostname) +} + +function stripTrailingDot(host: string): string { + return host.endsWith('.') ? host.slice(0, -1) : host +}