From c39562366303354527ac05f04736f247651d7a7c Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Wed, 11 Feb 2026 09:50:46 -0800 Subject: [PATCH 01/11] feat: add BitBucket integration for pull request management Adds BitBucket Cloud support including: - BitBucketApiClient for REST API v2.0 communication - BitBucketVCSProvider implementing VersionControlProvider interface - VCSProviderFactory for provider instantiation - ProviderCoordinator for cross-provider workflows - bitbucket-pr merge mode in finish command - Reviewer auto-assignment with username-to-UUID resolution - Auto-detection of workspace/repo from git remote - Debug subcommand for integration testing Co-Authored-By: Claude Opus 4.6 --- README.md | 120 ++++- src/cli.ts | 88 ++++ src/commands/finish.ts | 141 ++++++ src/lib/ProviderCoordinator.ts | 165 +++++++ src/lib/SessionSummaryService.test.ts | 13 +- src/lib/SettingsManager.test.ts | 79 ++++ src/lib/SettingsManager.ts | 70 ++- src/lib/VCSProviderFactory.test.ts | 254 ++++++++++ src/lib/VCSProviderFactory.ts | 95 ++++ src/lib/VersionControlProvider.ts | 69 +++ .../bitbucket/BitBucketApiClient.test.ts | 445 ++++++++++++++++++ .../providers/bitbucket/BitBucketApiClient.ts | 391 +++++++++++++++ .../bitbucket/BitBucketVCSProvider.test.ts | 381 +++++++++++++++ .../bitbucket/BitBucketVCSProvider.ts | 276 +++++++++++ src/lib/providers/bitbucket/index.ts | 3 + src/utils/claude.test.ts | 2 +- src/utils/claude.ts | 6 +- src/utils/remote.ts | 32 +- 18 files changed, 2604 insertions(+), 26 deletions(-) create mode 100644 src/lib/ProviderCoordinator.ts create mode 100644 src/lib/VCSProviderFactory.test.ts create mode 100644 src/lib/VCSProviderFactory.ts create mode 100644 src/lib/VersionControlProvider.ts create mode 100644 src/lib/providers/bitbucket/BitBucketApiClient.test.ts create mode 100644 src/lib/providers/bitbucket/BitBucketApiClient.ts create mode 100644 src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts create mode 100644 src/lib/providers/bitbucket/BitBucketVCSProvider.ts create mode 100644 src/lib/providers/bitbucket/index.ts diff --git a/README.md b/README.md index dbb412cb..991af439 100644 --- a/README.md +++ b/README.md @@ -459,6 +459,15 @@ iloom supports multiple issue tracking providers to fit your team's workflow. | **Linear** | `il init` | Requires API token. Supports full read/write on Linear issues. | | **Jira** | Configure in `.iloom/settings.json` | Atlassian Cloud. Requires API token. See [Jira Setup](#jira-setup) below. | +### Version Control Providers + +Choose which platform hosts your pull requests and code reviews. + +| **Provider** | **Setup** | **Notes** | +|--------------|-----------|-----------| +| **GitHub** | `gh auth login` | Default. Integrated with GitHub Issues. | +| **BitBucket** | Configure in `.iloom/settings.json` | Atlassian Cloud. Requires API token. See [BitBucket Setup](#bitbucket-setup) below. | + ### Jira Setup To use Jira as your issue tracker, add this configuration: @@ -507,14 +516,122 @@ To use Jira as your issue tracker, add this configuration: - `doneStatuses`: (Optional) Status names to exclude from `il issues` lists (default: `["Done"]`). Set to match your Jira workflow, e.g., `["Done", "Closed", "Verified"]` - `transitionMappings`: (Optional) Map iloom states to your Jira workflow transition names +### BitBucket Setup + +To use BitBucket for pull requests, add this configuration: + +**.iloom/settings.json (Committed)** +```json +{ + "versionControl": { + "provider": "bitbucket", + "bitbucket": { + "username": "your-bitbucket-username", + "workspace": "your-workspace", + "repoSlug": "your-repo" + } + }, + "mergeBehavior": { + "mode": "bitbucket-pr" + } +} +``` + +**.iloom/settings.local.json (Gitignored - Never commit this file)** +```json +{ + "versionControl": { + "bitbucket": { + "apiToken": "your-bitbucket-api-token" + } + } +} +``` + +**Generate a BitBucket API Token:** +1. Visit https://bitbucket.org/account/settings/app-passwords/ +2. Click "Create API token" (Note: App passwords were deprecated September 2025) +3. Grant permissions: `repository:read`, `repository:write`, `pullrequest:read`, `pullrequest:write` +4. Copy the token to `.iloom/settings.local.json` + +**Configuration Options:** +- `username`: Your BitBucket username +- `apiToken`: API token (store in settings.local.json only!) +- `workspace`: (Optional) BitBucket workspace, auto-detected from git remote if not provided +- `repoSlug`: (Optional) Repository slug, auto-detected from git remote if not provided +- `reviewers`: (Optional) Array of BitBucket usernames to automatically add as PR reviewers. Usernames are resolved to BitBucket account IDs at PR creation time. Unresolved usernames are logged as warnings but don't block PR creation. + +**Example with Reviewers:** +```json +{ + "versionControl": { + "provider": "bitbucket", + "bitbucket": { + "username": "your-bitbucket-username", + "reviewers": [ + "alice.jones", + "bob.smith" + ] + } + }, + "mergeBehavior": { + "mode": "bitbucket-pr" + } +} +``` + +### Jira + BitBucket Together + +Use Jira for issues and BitBucket for pull requests: + +**.iloom/settings.json** +```json +{ + "issueManagement": { + "provider": "jira", + "jira": { + "host": "https://yourcompany.atlassian.net", + "username": "your.email@company.com", + "projectKey": "PROJ" + } + }, + "versionControl": { + "provider": "bitbucket", + "bitbucket": { + "username": "your-bitbucket-username" + } + }, + "mergeBehavior": { + "mode": "bitbucket-pr" + } +} +``` + +**.iloom/settings.local.json** +```json +{ + "issueManagement": { + "jira": { + "apiToken": "your-jira-api-token" + } + }, + "versionControl": { + "bitbucket": { + "apiToken": "your-bitbucket-api-token" + } + } +} +``` + ### IDE Support iloom creates isolated workspace settings for your editor. Color synchronization (visual context) only works best VS Code-based editors. * **Supported:** VS Code, Cursor, Windsurf, Antigravity, WebStorm, IntelliJ, Sublime Text. - + * **Config:** Set your preference via `il init` or `il start --set ide.type=cursor`. + ### Git Operation Settings Configure git operation timeouts for projects with long-running pre-commit hooks. @@ -534,7 +651,6 @@ Configure git operation timeouts for projects with long-running pre-commit hooks **When to increase:** If you see timeout errors during `il commit` or `il finish`, your pre-commit hooks are taking longer than the default 60 seconds. Set a higher value based on your typical hook duration. - Advanced Features ----------------- diff --git a/src/cli.ts b/src/cli.ts index 5518b7b6..a871c210 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -610,6 +610,7 @@ program .option('-n, --dry-run', 'Preview actions without executing') .option('--pr ', 'Treat input as PR number', parseFloat) .option('--skip-build', 'Skip post-merge build verification') + .option('--skip-to-pr', 'Skip rebase/validation/commit, go directly to PR creation (debug)') .option('--no-browser', 'Skip opening PR in browser (github-pr and github-draft-pr modes)') .option('--cleanup', 'Clean up worktree after finishing (default in local mode)') .option('--no-cleanup', 'Keep worktree after finishing') @@ -2220,6 +2221,93 @@ program process.exit(0) }) +// Debug commands - only registered when debug mode is enabled +if (process.env.ILOOM_DEBUG === 'true') { + const debugCommand = program + .command('debug') + .description('Debug tools (only available in debug mode)') + + const bitbucketDebugCommand = debugCommand + .command('bitbucket') + .description('BitBucket debug tools') + + bitbucketDebugCommand + .command('resolve-reviewer-ids') + .description('Resolve configured reviewer usernames to BitBucket account IDs') + .action(async () => { + try { + const settingsManager = new SettingsManager() + const settings = await settingsManager.loadSettings() + + const bitbucketConfig = settings.versionControl?.bitbucket + if (!bitbucketConfig) { + logger.error('BitBucket configuration not found in settings') + logger.info('Configure versionControl.bitbucket in .iloom/settings.json') + process.exit(1) + } + + if (!bitbucketConfig.username) { + logger.error('BitBucket username not configured') + logger.info('Configure versionControl.bitbucket.username in .iloom/settings.json') + process.exit(1) + } + + if (!bitbucketConfig.apiToken) { + logger.error('BitBucket API token not configured') + logger.info('Configure versionControl.bitbucket.apiToken in .iloom/settings.local.json') + process.exit(1) + } + + const reviewers = bitbucketConfig.reviewers ?? [] + if (reviewers.length === 0) { + logger.warn('No reviewers configured in settings') + logger.info('Configure versionControl.bitbucket.reviewers in .iloom/settings.json') + console.log(JSON.stringify({}, null, 2)) + process.exit(0) + } + + // Get workspace from config or auto-detect from git remote + let workspace = bitbucketConfig.workspace + if (!workspace) { + const { parseGitRemotes } = await import('./utils/remote.js') + const remotes = await parseGitRemotes() + const bitbucketRemote = remotes.find(r => r.url.includes('bitbucket.org')) + if (!bitbucketRemote) { + logger.error('Could not auto-detect BitBucket workspace from git remote') + logger.info('Configure versionControl.bitbucket.workspace in .iloom/settings.json') + process.exit(1) + } + workspace = bitbucketRemote.owner + } + + // At this point workspace is guaranteed to be a string (either from config or auto-detected) + const resolvedWorkspace = workspace + + // Create BitBucket API client and resolve reviewer IDs + const { BitBucketApiClient } = await import('./lib/providers/bitbucket/BitBucketApiClient.js') + const apiClient = new BitBucketApiClient({ + username: bitbucketConfig.username, + apiToken: bitbucketConfig.apiToken, + workspace: resolvedWorkspace, + }) + + const resolvedMap = await apiClient.findUsersByUsername(resolvedWorkspace, reviewers) + + // Convert Map to plain object for JSON output + const result: Record = {} + for (const [username, accountId] of resolvedMap) { + result[username] = accountId + } + + console.log(JSON.stringify(result, null, 2)) + process.exit(0) + } catch (error) { + logger.error(`Failed to resolve reviewer IDs: ${error instanceof Error ? error.message : 'Unknown error'}`) + process.exit(1) + } + }) +} + // Parse CLI arguments (only when run directly, not when imported for testing) // Resolve symlinks to handle npm link and global installs const isRunDirectly = process.argv[1] && ((): boolean => { diff --git a/src/commands/finish.ts b/src/commands/finish.ts index dafb86b6..23406a41 100644 --- a/src/commands/finish.ts +++ b/src/commands/finish.ts @@ -897,6 +897,23 @@ export class FinishCommand { return } + if (mergeBehavior.mode === 'bitbucket-pr') { + // For BitBucket, we use the VCS provider layer - NOT the issue tracker + // This allows Jira/Linear issues to create PRs in BitBucket + const { VCSProviderFactory } = await import('../lib/VCSProviderFactory.js') + const vcsProvider = VCSProviderFactory.create(settings) + + if (!vcsProvider || vcsProvider.providerName !== 'bitbucket') { + throw new Error( + `The 'bitbucket-pr' merge mode requires BitBucket VCS configuration. ` + + `Add versionControl.provider: 'bitbucket' to your settings.` + ) + } + + await this.executeBitBucketPRWorkflow(parsed, options, worktree, settings, vcsProvider, result) + return + } + // Step 6: Perform fast-forward merge getLogger().info('Performing fast-forward merge...') await this.mergeManager.performFastForwardMerge(worktree.branch, worktree.path, mergeOptions) @@ -1181,6 +1198,130 @@ export class FinishCommand { } } + /** + * Execute workflow for BitBucket PR creation (bitbucket-pr merge mode) + * Validates -> Commits -> Pushes -> Creates PR -> Prompts for cleanup + * + * Unlike GitHub PR workflow, this uses the VersionControlProvider abstraction + * instead of PRManager, allowing it to work with any issue tracker (Jira, Linear, etc.) + */ + private async executeBitBucketPRWorkflow( + parsed: ParsedFinishInput, + options: FinishOptions, + worktree: GitWorktree, + settings: import('../lib/SettingsManager.js').IloomSettings, + vcsProvider: import('../lib/VersionControlProvider.js').VersionControlProvider, + finishResult: FinishResult + ): Promise { + // Step 1: Push branch to origin + if (options.dryRun) { + getLogger().info('[DRY RUN] Would push branch to origin') + } else { + getLogger().info('Pushing branch to origin...') + await pushBranchToRemote(worktree.branch, worktree.path, { dryRun: false }) + getLogger().success('Branch pushed successfully') + } + + // Step 2: Generate PR title from issue if available + // Note: parsed.number already has correct case from parseInput() metadata lookup + let prTitle = `Work from ${worktree.branch}` + if (parsed.type === 'issue' && parsed.number) { + try { + const issue = await this.issueTracker.fetchIssue(parsed.number) + + // Apply ticket prefix if enabled (default: true) + const usePrefix = settings.mergeBehavior?.prTitlePrefix; + if (usePrefix) { + prTitle = `${parsed.number}: ${issue.title}` + } else { + prTitle = issue.title + } + } catch (error) { + getLogger().debug('Could not fetch issue title, using branch name', { error }) + } + } + + // Step 3: Get base branch (respects parent loom metadata for child looms) + const baseBranch = await getMergeTargetBranch(worktree.path) + + // Step 4: Check for existing PR or create new one + if (options.dryRun) { + getLogger().info('[DRY RUN] Would create BitBucket PR') + getLogger().info(` Title: ${prTitle}`) + getLogger().info(` Base: ${baseBranch}`) + finishResult.operations.push({ + type: 'pr-creation', + message: 'Would create BitBucket PR (dry-run)', + success: true, + }) + } else { + // Check for existing PR first + const existingPR = await vcsProvider.checkForExistingPR(worktree.branch, worktree.path) + + if (existingPR) { + getLogger().success(`Existing pull request: ${existingPR.url}`) + finishResult.prUrl = existingPR.url + finishResult.operations.push({ + type: 'pr-creation', + message: 'Found existing pull request', + success: true, + }) + } else { + // Generate PR body using Claude (same as GitHub workflow) + const { PRManager } = await import('../lib/PRManager.js') + const prManager = new PRManager(settings) + const prBody = await prManager.generatePRBody( + parsed.type === 'issue' ? parsed.number : undefined, + worktree.path + ) + + // Create new PR + const prUrl = await vcsProvider.createPR( + worktree.branch, + prTitle, + prBody, + baseBranch, + worktree.path + ) + getLogger().success(`Pull request created: ${prUrl}`) + finishResult.prUrl = prUrl + finishResult.operations.push({ + type: 'pr-creation', + message: 'Pull request created', + success: true, + }) + + // Move issue to Ready for Review state + if (parsed.type === 'issue' && parsed.number) { + try { + if (this.issueTracker.moveIssueToReadyForReview) { + await this.issueTracker.moveIssueToReadyForReview(parsed.number) + getLogger().info('Issue moved to Ready for Review') + } + } catch (error) { + getLogger().warn( + `Failed to move issue to Ready for Review: ${error instanceof Error ? error.message : 'Unknown error'}`, + error + ) + } + } + } + + // Generate session summary - posts to the ISSUE (Jira/Linear), not the PR + // For BitBucket workflows, the issue tracker (Jira/Linear) doesn't support PR comments, + // so we post to the issue where the knowledge capture belongs + await this.generateSessionSummaryIfConfigured(parsed, worktree, options) + + // Archive metadata BEFORE cleanup prompt (ensures it runs even with --no-cleanup) + const { MetadataManager } = await import('../lib/MetadataManager.js') + const metadataManager = new MetadataManager() + await metadataManager.archiveMetadata(worktree.path) + + // Interactive cleanup prompt (unless flags override) + await this.handlePRCleanupPrompt(parsed, options, worktree, finishResult) + } + } + /** * Handle cleanup prompt after PR creation * Respects --cleanup and --no-cleanup flags, otherwise prompts user diff --git a/src/lib/ProviderCoordinator.ts b/src/lib/ProviderCoordinator.ts new file mode 100644 index 00000000..e49d144d --- /dev/null +++ b/src/lib/ProviderCoordinator.ts @@ -0,0 +1,165 @@ +// ProviderCoordinator - Orchestrates workflows between IssueTracker and VersionControlProvider +// Manages the interaction between issue tracking and version control systems + +import type { IssueTracker } from './IssueTracker.js' +import type { VersionControlProvider } from './VersionControlProvider.js' +import { getLogger } from '../utils/logger-context.js' + +/** + * Options for posting agent output + */ +export interface PostAgentOutputOptions { + issueNumber?: string | number + prNumber?: number + body: string + cwd?: string +} + +/** + * Options for finish workflow + */ +export interface FinishWorkflowOptions { + branchName: string + title: string + body: string + baseBranch: string + issueNumber?: string | number + transitionState?: string + cwd?: string +} + +/** + * Result of finish workflow + */ +export interface FinishWorkflowResult { + prUrl: string + prNumber: number + issueTransitioned: boolean +} + +/** + * ProviderCoordinator orchestrates workflows across issue tracking and version control providers. + * + * Key responsibilities: + * - Route agent output to the correct destination (issue vs PR) + * - Coordinate PR creation with issue state transitions + * - Provide a unified interface for start/finish workflows + * + * Design pattern: + * - Uses composition over inheritance + * - Delegates provider-specific operations to injected providers + * - Handles cross-provider coordination logic + */ +export class ProviderCoordinator { + constructor( + private issueTracker: IssueTracker, + private vcsProvider: VersionControlProvider + ) {} + + /** + * Post agent output to the appropriate destination + * - If PR number provided, post to PR + * - Otherwise, post to issue + */ + async postAgentOutput(options: PostAgentOutputOptions): Promise { + const { issueNumber, prNumber, body, cwd } = options + + if (prNumber) { + // Post to PR via VCS provider + getLogger().debug('Posting agent output to PR', { prNumber }) + await this.vcsProvider.createPRComment(prNumber, body, cwd) + } else if (issueNumber) { + // Post to issue via issue tracker + getLogger().debug('Posting agent output to issue', { issueNumber }) + // Note: This will need the MCP server-based approach or direct API call + // For now, we'll throw since this needs to be integrated with the MCP system + throw new Error('Issue comment posting not yet implemented in coordinator') + } else { + throw new Error('Either issueNumber or prNumber must be provided') + } + } + + /** + * Execute finish workflow: + * 1. Create PR via VCS provider + * 2. Post session summary to PR + * 3. Transition issue to target state (e.g., "In Review") + */ + async executeFinishWorkflow(options: FinishWorkflowOptions): Promise { + const { branchName, title, body, baseBranch, issueNumber, transitionState, cwd } = options + + // Step 1: Create PR + getLogger().debug('Creating PR via VCS provider', { branchName, title }) + const prUrl = await this.vcsProvider.createPR(branchName, title, body, baseBranch, cwd) + + // Extract PR number from URL + const prNumber = this.extractPRNumberFromUrl(prUrl) + + getLogger().info('PR created successfully', { prUrl, prNumber }) + + // Step 2: Post session summary to PR (if provided in body) + // The body already contains the session summary, so this is handled by createPR + + // Step 3: Transition issue if requested + let issueTransitioned = false + if (issueNumber && transitionState) { + try { + // Check if issue tracker supports state transitions + if (this.issueTracker.moveIssueToInProgress) { + getLogger().debug('Transitioning issue state', { issueNumber, transitionState }) + // Note: This is a placeholder - actual transition logic will vary by provider + // For now, we only support moveIssueToInProgress + // TODO: Add more flexible transition support + await this.issueTracker.moveIssueToInProgress(issueNumber) + issueTransitioned = true + getLogger().info('Issue transitioned successfully', { issueNumber, transitionState }) + } else { + getLogger().warn('Issue tracker does not support state transitions', { + provider: this.issueTracker.providerName + }) + } + } catch (error) { + // Don't fail the whole workflow if transition fails + getLogger().error('Failed to transition issue', { error, issueNumber }) + } + } + + return { + prUrl, + prNumber, + issueTransitioned, + } + } + + /** + * Extract PR number from PR URL + * Handles various VCS provider URL formats + */ + private extractPRNumberFromUrl(url: string): number { + // GitHub: https://github.com/owner/repo/pull/123 + // BitBucket: https://bitbucket.org/workspace/repo/pull-requests/123 + const githubMatch = url.match(/\/pull\/(\d+)/) + const bitbucketMatch = url.match(/\/pull-requests\/(\d+)/) + + const match = githubMatch ?? bitbucketMatch + if (match?.[1]) { + return parseInt(match[1], 10) + } + + throw new Error(`Failed to extract PR number from URL: ${url}`) + } + + /** + * Get issue tracker instance + */ + getIssueTracker(): IssueTracker { + return this.issueTracker + } + + /** + * Get VCS provider instance + */ + getVCSProvider(): VersionControlProvider { + return this.vcsProvider + } +} diff --git a/src/lib/SessionSummaryService.test.ts b/src/lib/SessionSummaryService.test.ts index d0362ef9..11f8f928 100644 --- a/src/lib/SessionSummaryService.test.ts +++ b/src/lib/SessionSummaryService.test.ts @@ -247,21 +247,18 @@ describe('SessionSummaryService', () => { }) it('should use correct issue management provider based on settings', async () => { - vi.mocked(mockSettingsManager.loadSettings).mockResolvedValue({ + const mockSettingsValue: IloomSettings = { ...defaultSettings, issueManagement: { provider: 'linear', }, - }) + }; + + vi.mocked(mockSettingsManager.loadSettings).mockResolvedValue(mockSettingsValue) await service.generateAndPostSummary(defaultInput) - expect(IssueManagementProviderFactory.create).toHaveBeenCalledWith('linear', { - ...defaultSettings, - issueManagement: { - provider: 'linear', - }, - }) + expect(IssueManagementProviderFactory.create).toHaveBeenCalledWith('linear', mockSettingsValue) }) it('should skip when Claude returns empty result', async () => { diff --git a/src/lib/SettingsManager.test.ts b/src/lib/SettingsManager.test.ts index 7f31aeec..aeee9355 100644 --- a/src/lib/SettingsManager.test.ts +++ b/src/lib/SettingsManager.test.ts @@ -2816,6 +2816,85 @@ const error: { code?: string; message: string } = { }) }) + describe('bitbucket reviewers configuration', () => { + it('should accept valid usernames in reviewers array', async () => { + const projectRoot = '/test/project' + const validSettings = { + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: 'testuser', + apiToken: 'test-token', + reviewers: ['alice', 'bob_smith'], + }, + }, + } + + const error: { code?: string; message: string } = { + code: 'ENOENT', + message: 'ENOENT: no such file or directory', + } + vi.mocked(readFile) + .mockRejectedValueOnce(error) // global settings + .mockResolvedValueOnce(JSON.stringify(validSettings)) // settings.json + .mockRejectedValueOnce(error) // settings.local.json + + const result = await settingsManager.loadSettings(projectRoot) + expect(result.versionControl?.bitbucket?.reviewers).toEqual(['alice', 'bob_smith']) + }) + + it('should allow empty reviewers array', async () => { + const projectRoot = '/test/project' + const validSettings = { + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: 'testuser', + apiToken: 'test-token', + reviewers: [], + }, + }, + } + + const error: { code?: string; message: string } = { + code: 'ENOENT', + message: 'ENOENT: no such file or directory', + } + vi.mocked(readFile) + .mockRejectedValueOnce(error) // global settings + .mockResolvedValueOnce(JSON.stringify(validSettings)) // settings.json + .mockRejectedValueOnce(error) // settings.local.json + + const result = await settingsManager.loadSettings(projectRoot) + expect(result.versionControl?.bitbucket?.reviewers).toEqual([]) + }) + + it('should allow missing reviewers field', async () => { + const projectRoot = '/test/project' + const validSettings = { + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: 'testuser', + apiToken: 'test-token', + }, + }, + } + + const error: { code?: string; message: string } = { + code: 'ENOENT', + message: 'ENOENT: no such file or directory', + } + vi.mocked(readFile) + .mockRejectedValueOnce(error) // global settings + .mockResolvedValueOnce(JSON.stringify(validSettings)) // settings.json + .mockRejectedValueOnce(error) // settings.local.json + + const result = await settingsManager.loadSettings(projectRoot) + expect(result.versionControl?.bitbucket?.reviewers).toBeUndefined() + }) + }) + describe('getSpinModel', () => { it('should return opus by default when spin not configured', () => { const settings = { sourceEnvOnStart: false } diff --git a/src/lib/SettingsManager.ts b/src/lib/SettingsManager.ts index ad379eca..8d252358 100644 --- a/src/lib/SettingsManager.ts +++ b/src/lib/SettingsManager.ts @@ -437,10 +437,40 @@ export const IloomSettingsSchema = z.object({ }) .optional() .describe('Issue management configuration'), + versionControl: z + .object({ + provider: z.enum(['github', 'bitbucket']).optional().default('github').describe('Version control provider (github, bitbucket)'), + bitbucket: z + .object({ + username: z + .string() + .min(1, 'BitBucket username cannot be empty') + .describe('BitBucket username'), + apiToken: z + .string() + .optional() + .describe('BitBucket API token. SECURITY: Store in settings.local.json only, never commit to source control. Generate at: https://bitbucket.org/account/settings/app-passwords/ (Note: App passwords deprecated Sep 2025, use API tokens)'), + workspace: z + .string() + .optional() + .describe('BitBucket workspace (optional, auto-detected from git remote if not provided)'), + repoSlug: z + .string() + .optional() + .describe('BitBucket repository slug (optional, auto-detected from git remote if not provided)'), + reviewers: z + .array(z.string().describe('Reviewer username')) + .optional() + .describe('List of usernames to add as PR reviewers. Usernames are resolved to Bitbucket account IDs at PR creation time.'), + }) + .optional(), + }) + .optional() + .describe('Version control provider configuration'), mergeBehavior: z .object({ // SYNC: If this default changes, update displayDefaultsBox() in src/utils/first-run-setup.ts - mode: z.enum(['local', 'github-pr', 'github-draft-pr']).default('local'), + mode: z.enum(['local', 'github-pr', 'github-draft-pr', 'bitbucket-pr']).default('local'), remote: z.string().optional(), autoCommitPush: z .boolean() @@ -454,9 +484,10 @@ export const IloomSettingsSchema = z.object({ .describe( 'Open the PR in the default browser after finishing in github-pr or github-draft-pr mode. Use --no-browser flag to override.' ), + prTitlePrefix: z.boolean().default(true).optional().describe('Prefix PR titles with the issue number (e.g., "QLH-123: Title"). Default: true'), }) .optional() - .describe('Merge behavior configuration: local (merge locally), github-pr (create PR), or github-draft-pr (create draft PR at start, mark ready on finish)'), + .describe('Merge behavior configuration: local (merge locally), github-pr (create PR), github-draft-pr (create draft PR at start, mark ready on finish), or bitbucket-pr (create BitBucket PR)'), ide: z .object({ // SYNC: If this default changes, update displayDefaultsBox() in src/utils/first-run-setup.ts @@ -691,9 +722,39 @@ export const IloomSettingsSchemaNoDefaults = z.object({ }) .optional() .describe('Issue management configuration'), + versionControl: z + .object({ + provider: z.enum(['github', 'bitbucket']).optional().describe('Version control provider (github, bitbucket)'), + bitbucket: z + .object({ + username: z + .string() + .min(1, 'BitBucket username cannot be empty') + .describe('BitBucket username'), + apiToken: z + .string() + .optional() + .describe('BitBucket API token. SECURITY: Store in settings.local.json only, never commit to source control. Generate at: https://bitbucket.org/account/settings/app-passwords/ (Note: App passwords deprecated Sep 2025, use API tokens)'), + workspace: z + .string() + .optional() + .describe('BitBucket workspace (optional, auto-detected from git remote if not provided)'), + repoSlug: z + .string() + .optional() + .describe('BitBucket repository slug (optional, auto-detected from git remote if not provided)'), + reviewers: z + .array(z.string().describe('Reviewer username')) + .optional() + .describe('List of usernames to add as PR reviewers. Usernames are resolved to Bitbucket account IDs at PR creation time.'), + }) + .optional(), + }) + .optional() + .describe('Version control provider configuration'), mergeBehavior: z .object({ - mode: z.enum(['local', 'github-pr', 'github-draft-pr']).optional(), + mode: z.enum(['local', 'github-pr', 'github-draft-pr', 'bitbucket-pr']).optional(), remote: z.string().optional(), autoCommitPush: z .boolean() @@ -707,9 +768,10 @@ export const IloomSettingsSchemaNoDefaults = z.object({ .describe( 'Open the PR in the default browser after finishing in github-pr or github-draft-pr mode. Use --no-browser flag to override.' ), + prTitlePrefix: z.boolean().optional(), }) .optional() - .describe('Merge behavior configuration: local (merge locally), github-pr (create PR), or github-draft-pr (create draft PR at start, mark ready on finish)'), + .describe('Merge behavior configuration: local (merge locally), github-pr (create PR), github-draft-pr (create draft PR at start, mark ready on finish), or bitbucket-pr (create BitBucket PR)'), ide: z .object({ type: z diff --git a/src/lib/VCSProviderFactory.test.ts b/src/lib/VCSProviderFactory.test.ts new file mode 100644 index 00000000..000e7761 --- /dev/null +++ b/src/lib/VCSProviderFactory.test.ts @@ -0,0 +1,254 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { VCSProviderFactory } from './VCSProviderFactory.js' +import { BitBucketVCSProvider } from './providers/bitbucket/index.js' +import type { IloomSettings } from './SettingsManager.js' + +// Mock the BitBucketVCSProvider +vi.mock('./providers/bitbucket/index.js', () => ({ + BitBucketVCSProvider: vi.fn().mockImplementation((config) => ({ + providerName: 'bitbucket', + config, + })), +})) + +// Mock the logger +vi.mock('../utils/logger-context.js', () => ({ + getLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})) + +describe('VCSProviderFactory', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + describe('create', () => { + it('should return null for github provider', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'github', + }, + } + + const result = VCSProviderFactory.create(settings) + expect(result).toBeNull() + }) + + it('should return null when no provider is configured', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + } + + const result = VCSProviderFactory.create(settings) + expect(result).toBeNull() + }) + + it('should create BitBucketVCSProvider with basic config', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: 'testuser', + apiToken: 'test-token', + }, + }, + } + + VCSProviderFactory.create(settings) + + expect(BitBucketVCSProvider).toHaveBeenCalledWith({ + username: 'testuser', + apiToken: 'test-token', + }) + }) + + it('should pass workspace and repoSlug when configured', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: 'testuser', + apiToken: 'test-token', + workspace: 'my-workspace', + repoSlug: 'my-repo', + }, + }, + } + + VCSProviderFactory.create(settings) + + expect(BitBucketVCSProvider).toHaveBeenCalledWith({ + username: 'testuser', + apiToken: 'test-token', + workspace: 'my-workspace', + repoSlug: 'my-repo', + }) + }) + + it('should pass reviewers when configured', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: 'testuser', + apiToken: 'test-token', + reviewers: ['alice@example.com', 'bob@example.com'], + }, + }, + } + + VCSProviderFactory.create(settings) + + expect(BitBucketVCSProvider).toHaveBeenCalledWith({ + username: 'testuser', + apiToken: 'test-token', + reviewers: ['alice@example.com', 'bob@example.com'], + }) + }) + + it('should pass all config options together', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: 'testuser', + apiToken: 'test-token', + workspace: 'my-workspace', + repoSlug: 'my-repo', + reviewers: ['alice@example.com'], + }, + }, + } + + VCSProviderFactory.create(settings) + + expect(BitBucketVCSProvider).toHaveBeenCalledWith({ + username: 'testuser', + apiToken: 'test-token', + workspace: 'my-workspace', + repoSlug: 'my-repo', + reviewers: ['alice@example.com'], + }) + }) + + it('should throw when bitbucket username is missing', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: '', + apiToken: 'test-token', + }, + }, + } + + expect(() => VCSProviderFactory.create(settings)).toThrow( + 'BitBucket username is required' + ) + }) + + it('should throw when bitbucket apiToken is missing', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'bitbucket', + bitbucket: { + username: 'testuser', + }, + }, + } + + expect(() => VCSProviderFactory.create(settings)).toThrow( + 'BitBucket API token is required' + ) + }) + }) + + describe('isConfigured', () => { + it('should return true for bitbucket provider', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'bitbucket', + }, + } + + expect(VCSProviderFactory.isConfigured(settings)).toBe(true) + }) + + it('should return false for github provider', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'github', + }, + } + + expect(VCSProviderFactory.isConfigured(settings)).toBe(false) + }) + + it('should return false when no provider is configured', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + } + + expect(VCSProviderFactory.isConfigured(settings)).toBe(false) + }) + }) + + describe('getProviderName', () => { + it('should return bitbucket when configured', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'bitbucket', + }, + } + + expect(VCSProviderFactory.getProviderName(settings)).toBe('bitbucket') + }) + + it('should return github when configured', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + versionControl: { + provider: 'github', + }, + } + + expect(VCSProviderFactory.getProviderName(settings)).toBe('github') + }) + + it('should return undefined when no provider is configured', () => { + const settings: IloomSettings = { + sourceEnvOnStart: false, + attribution: 'upstreamOnly', + } + + expect(VCSProviderFactory.getProviderName(settings)).toBeUndefined() + }) + }) +}) diff --git a/src/lib/VCSProviderFactory.ts b/src/lib/VCSProviderFactory.ts new file mode 100644 index 00000000..871a05dc --- /dev/null +++ b/src/lib/VCSProviderFactory.ts @@ -0,0 +1,95 @@ +// VCSProviderFactory - creates appropriate VersionControlProvider based on settings +// Follows pattern from IssueTrackerFactory + +import type { VersionControlProvider } from './VersionControlProvider.js' +import { BitBucketVCSProvider, type BitBucketVCSConfig } from './providers/bitbucket/index.js' +import type { IloomSettings } from './SettingsManager.js' +import { getLogger } from '../utils/logger-context.js' + +export type VCSProviderType = 'github' | 'bitbucket' + +/** + * Factory for creating VersionControlProvider instances based on settings + * + * Note: GitHub VCS operations still use PRManager with gh CLI for now. + * This factory is primarily for BitBucket and future VCS providers. + */ +export class VCSProviderFactory { + /** + * Create a VersionControlProvider instance based on settings configuration + * + * @param settings - iloom settings containing versionControl.provider + * @returns VersionControlProvider instance configured for the specified provider + * @throws Error if provider type is not supported or required config is missing + */ + static create(settings: IloomSettings): VersionControlProvider | null { + const provider = settings.versionControl?.provider + + // If no versionControl config, return null (use legacy PRManager for GitHub) + if (!provider) { + getLogger().debug('VCSProviderFactory: No versionControl.provider configured, using legacy PRManager') + return null + } + + getLogger().debug(`VCSProviderFactory: Creating VCS provider for "${provider}"`) + + switch (provider) { + case 'github': + // GitHub still uses PRManager with gh CLI + getLogger().debug('VCSProviderFactory: GitHub uses legacy PRManager, returning null') + return null + + case 'bitbucket': { + const bbSettings = settings.versionControl?.bitbucket + + if (!bbSettings?.username) { + throw new Error('BitBucket username is required. Configure versionControl.bitbucket.username in .iloom/settings.json') + } + if (!bbSettings?.apiToken) { + throw new Error('BitBucket API token is required. Configure versionControl.bitbucket.apiToken in .iloom/settings.local.json') + } + + const bbConfig: BitBucketVCSConfig = { + username: bbSettings.username, + apiToken: bbSettings.apiToken, + } + + if (bbSettings.workspace) { + bbConfig.workspace = bbSettings.workspace + } + if (bbSettings.repoSlug) { + bbConfig.repoSlug = bbSettings.repoSlug + } + if (bbSettings.reviewers) { + bbConfig.reviewers = bbSettings.reviewers + } + + getLogger().debug(`VCSProviderFactory: Creating BitBucketVCSProvider for user: ${bbSettings.username}`) + return new BitBucketVCSProvider(bbConfig) + } + + default: + throw new Error(`Unsupported VCS provider: ${provider}`) + } + } + + /** + * Check if a VCS provider is configured + * + * @param settings - iloom settings + * @returns true if versionControl provider is configured + */ + static isConfigured(settings: IloomSettings): boolean { + return settings.versionControl?.provider !== undefined && settings.versionControl?.provider !== 'github' + } + + /** + * Get the configured provider name from settings + * + * @param settings - iloom settings + * @returns Provider type string or undefined if not configured + */ + static getProviderName(settings: IloomSettings): VCSProviderType | undefined { + return settings.versionControl?.provider as VCSProviderType | undefined + } +} diff --git a/src/lib/VersionControlProvider.ts b/src/lib/VersionControlProvider.ts new file mode 100644 index 00000000..0c1e757b --- /dev/null +++ b/src/lib/VersionControlProvider.ts @@ -0,0 +1,69 @@ +// VersionControlProvider interface definition +// Generic interface for version control providers (GitHub, BitBucket, GitLab, etc.) + +import type { PullRequest } from '../types/index.js' + +/** + * Result of PR creation operation + */ +export interface PRCreationResult { + url: string + number: number + wasExisting: boolean +} + +/** + * Existing PR information + */ +export interface ExistingPR { + number: number + url: string +} + +/** + * VersionControlProvider interface - abstraction for VCS providers + * + * Design Philosophy: + * - Focuses exclusively on PR/MR (Pull Request/Merge Request) operations + * - Separates version control concerns from issue tracking + * - Identifiers use number for PR numbers (consistent with most VCS systems) + * - Providers expose capabilities via metadata fields + */ +export interface VersionControlProvider { + // Metadata - provider identification and capabilities + readonly providerName: string + readonly supportsForks: boolean + readonly supportsDraftPRs: boolean + + // PR operations - core functionality all providers must support + checkForExistingPR(branchName: string, cwd?: string): Promise + createPR( + branchName: string, + title: string, + body: string, + baseBranch: string, + cwd?: string + ): Promise + createDraftPR?( + branchName: string, + title: string, + body: string, + baseBranch: string, + cwd?: string + ): Promise + markPRReadyForReview?(prNumber: number, cwd?: string): Promise + + // PR metadata and state + fetchPR(prNumber: number, cwd?: string): Promise + getPRUrl(prNumber: number, cwd?: string): Promise + + // PR comments + createPRComment(prNumber: number, body: string, cwd?: string): Promise + + // Remote and repository detection + detectRepository(cwd?: string): Promise<{ owner: string; repo: string } | null> + getTargetRemote(cwd?: string): Promise + + // PR body generation (optional, can delegate to external service) + generatePRBody?(issueNumber: string | number | undefined, worktreePath: string): Promise +} diff --git a/src/lib/providers/bitbucket/BitBucketApiClient.test.ts b/src/lib/providers/bitbucket/BitBucketApiClient.test.ts new file mode 100644 index 00000000..cffa6d34 --- /dev/null +++ b/src/lib/providers/bitbucket/BitBucketApiClient.test.ts @@ -0,0 +1,445 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { BitBucketApiClient, type BitBucketConfig } from './BitBucketApiClient.js' + +// Mock the https module +vi.mock('node:https', () => ({ + default: { + request: vi.fn(), + }, +})) + +// Mock the logger +vi.mock('../../../utils/logger-context.js', () => ({ + getLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})) + +describe('BitBucketApiClient', () => { + let client: BitBucketApiClient + const config: BitBucketConfig = { + username: 'testuser', + apiToken: 'test-api-token', + workspace: 'test-workspace', + repoSlug: 'test-repo', + } + + beforeEach(() => { + client = new BitBucketApiClient(config) + }) + + describe('createPullRequest', () => { + it('should include reviewers in payload when provided', async () => { + const https = await import('node:https') + let capturedPayload: string | undefined + + // Mock the request to capture the payload + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 201, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: (data: string) => { capturedPayload = data }, + end: vi.fn(), + } + }) + + await client.createPullRequest( + 'workspace', + 'repo', + 'Test PR', + 'Test description', + 'feature-branch', + 'main', + ['account-id-1', 'account-id-2'] + ) + + expect(capturedPayload).toBeDefined() + const payload = JSON.parse(capturedPayload!) + expect(payload.reviewers).toEqual([ + { account_id: 'account-id-1' }, + { account_id: 'account-id-2' }, + ]) + }) + + it('should not include reviewers in payload when not provided', async () => { + const https = await import('node:https') + let capturedPayload: string | undefined + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 201, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: (data: string) => { capturedPayload = data }, + end: vi.fn(), + } + }) + + await client.createPullRequest( + 'workspace', + 'repo', + 'Test PR', + 'Test description', + 'feature-branch', + 'main' + ) + + expect(capturedPayload).toBeDefined() + const payload = JSON.parse(capturedPayload!) + expect(payload.reviewers).toBeUndefined() + }) + + it('should not include reviewers when array is empty', async () => { + const https = await import('node:https') + let capturedPayload: string | undefined + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 201, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: (data: string) => { capturedPayload = data }, + end: vi.fn(), + } + }) + + await client.createPullRequest( + 'workspace', + 'repo', + 'Test PR', + 'Test description', + 'feature-branch', + 'main', + [] + ) + + expect(capturedPayload).toBeDefined() + const payload = JSON.parse(capturedPayload!) + expect(payload.reviewers).toBeUndefined() + }) + }) + + describe('findUsersByUsername', () => { + it('should return map of username to account_id for matched users', async () => { + const https = await import('node:https') + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 200, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ + values: [ + { user: { account_id: 'acc-1', display_name: 'Alice Test', uuid: 'uuid-1', nickname: 'alice' } }, + { user: { account_id: 'acc-2', display_name: 'Bob Example', uuid: 'uuid-2', nickname: 'bob' } }, + ], + })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + } + }) + + const result = await client.findUsersByUsername('workspace', ['alice', 'bob']) + + expect(result.get('alice')).toBe('acc-1') + expect(result.get('bob')).toBe('acc-2') + }) + + it('should return empty map when no users match', async () => { + const https = await import('node:https') + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 200, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ + values: [ + { user: { account_id: 'acc-1', display_name: 'Charlie Different', uuid: 'uuid-1', nickname: 'charlie' } }, + ], + })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + } + }) + + const result = await client.findUsersByUsername('workspace', ['alice']) + + expect(result.size).toBe(0) + }) + + it('should handle API errors by throwing', async () => { + const https = await import('node:https') + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 403, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ error: { message: 'Access denied' } })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + } + }) + + // Should throw on API error + await expect(client.findUsersByUsername('workspace', ['alice'])).rejects.toThrow('BitBucket API error') + }) + + it('should handle pagination when fetching workspace members', async () => { + const https = await import('node:https') + let requestCount = 0 + const requestPaths: string[] = [] + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + requestCount++ + // Capture the path used in each request to verify no URL duplication + requestPaths.push((options as { path: string }).path) + const mockResponse = { + statusCode: 200, + on: vi.fn((event, handler) => { + if (event === 'data') { + // First request returns first page with 'next' URL + if (requestCount === 1) { + handler(JSON.stringify({ + values: [ + { user: { account_id: 'acc-1', display_name: 'Alice Test', uuid: 'uuid-1', nickname: 'alice' } }, + ], + next: 'https://api.bitbucket.org/2.0/workspaces/workspace/members?page=2', + })) + } else { + // Second request returns second page without 'next' + handler(JSON.stringify({ + values: [ + { user: { account_id: 'acc-2', display_name: 'Bob Example', uuid: 'uuid-2', nickname: 'bob' } }, + ], + })) + } + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + } + }) + + const result = await client.findUsersByUsername('workspace', ['alice', 'bob']) + + // Should have made 2 requests (one for each page) + expect(requestCount).toBe(2) + // Should have found both users from different pages + expect(result.get('alice')).toBe('acc-1') + expect(result.get('bob')).toBe('acc-2') + // Verify no URL path duplication (bug fix verification) + // First request should be the initial endpoint + expect(requestPaths[0]).toBe('/2.0/workspaces/workspace/members') + // Second request should be the pagination path (not /2.0/2.0/...) + expect(requestPaths[1]).toBe('/2.0/workspaces/workspace/members?page=2') + }) + + it('should match by display_name when nickname does not match', async () => { + const https = await import('node:https') + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 200, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ + values: [ + { user: { account_id: 'acc-1', display_name: 'alice', uuid: 'uuid-1', nickname: 'alice123' } }, + ], + })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + } + }) + + const result = await client.findUsersByUsername('workspace', ['alice']) + + expect(result.get('alice')).toBe('acc-1') + }) + }) + + describe('getCurrentUser', () => { + it('should return current user data from /user endpoint', async () => { + const https = await import('node:https') + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 200, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ + account_id: 'acc-current-user', + display_name: 'Current User', + nickname: 'currentuser', + })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + } + }) + + const user = await client.getCurrentUser() + + expect(user.account_id).toBe('acc-current-user') + expect(user.display_name).toBe('Current User') + expect(user.nickname).toBe('currentuser') + }) + + it('should throw on API error', async () => { + const https = await import('node:https') + + vi.mocked(https.default.request).mockImplementation((options, callback) => { + const mockResponse = { + statusCode: 401, + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(JSON.stringify({ error: { message: 'Unauthorized' } })) + } + if (event === 'end') { + handler() + } + return mockResponse + }), + } + // @ts-expect-error - Mock callback + callback(mockResponse) + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + } + }) + + await expect(client.getCurrentUser()).rejects.toThrow('BitBucket API error') + }) + }) + + describe('getWorkspace', () => { + it('should return configured workspace', () => { + expect(client.getWorkspace()).toBe('test-workspace') + }) + }) + + describe('getRepoSlug', () => { + it('should return configured repoSlug', () => { + expect(client.getRepoSlug()).toBe('test-repo') + }) + }) +}) diff --git a/src/lib/providers/bitbucket/BitBucketApiClient.ts b/src/lib/providers/bitbucket/BitBucketApiClient.ts new file mode 100644 index 00000000..aa21ace2 --- /dev/null +++ b/src/lib/providers/bitbucket/BitBucketApiClient.ts @@ -0,0 +1,391 @@ +// BitBucketApiClient - REST API wrapper for BitBucket operations +// Handles authentication and common API request patterns + +import https from 'node:https' +import { getLogger } from '../../../utils/logger-context.js' + +/** + * BitBucket API configuration + */ +export interface BitBucketConfig { + username: string + apiToken: string // API token from BitBucket settings + workspace?: string // Optional, can be auto-detected from git remote + repoSlug?: string // Optional, can be auto-detected from git remote +} + +/** + * BitBucket pull request response from API + */ +export interface BitBucketPullRequest { + id: number + title: string + description: string + state: 'OPEN' | 'MERGED' | 'DECLINED' | 'SUPERSEDED' + author: { + display_name: string + uuid: string + } + source: { + branch: { + name: string + } + } + destination: { + branch: { + name: string + } + } + created_on: string + updated_on: string + links: { + html: { + href: string + } + } + [key: string]: unknown +} + +/** + * BitBucket workspace member response from API + * Used for resolving usernames to account IDs + */ +export interface BitBucketWorkspaceMember { + user: { + account_id: string + display_name: string + uuid: string + nickname?: string + } +} + +/** + * BitBucket repository response from API + */ +export interface BitBucketRepository { + slug: string + name: string + full_name: string + workspace: { + slug: string + } + links: { + html: { + href: string + } + } + [key: string]: unknown +} + +interface BitBucketWorkspaceMembersResponse { values: BitBucketWorkspaceMember[]; next?: string } + +/** + * BitBucket current user response from /user endpoint + */ +export interface BitBucketCurrentUser { + account_id: string + display_name: string + nickname?: string +} + +/** + * BitBucketApiClient provides low-level REST API access to BitBucket + * + * Authentication: Basic Auth with username and API token + * API Reference: https://developer.atlassian.com/cloud/bitbucket/rest/intro/ + * + * Note: As of September 9, 2025, BitBucket app passwords can no longer be created. + * Use API tokens with scopes instead. All existing app passwords will be disabled on June 9, 2026. + */ +export class BitBucketApiClient { + private readonly baseUrl = 'https://api.bitbucket.org/2.0' + private readonly authHeader: string + private readonly workspace: string | undefined + private readonly repoSlug: string | undefined + + constructor(config: BitBucketConfig) { + // Create Basic Auth header with API token + const credentials = Buffer.from(`${config.username}:${config.apiToken}`).toString('base64') + this.authHeader = `Basic ${credentials}` + + this.workspace = config.workspace + this.repoSlug = config.repoSlug + } + + /** + * Make an HTTP request to BitBucket API + */ + private async request( + method: 'GET' | 'POST', + endpoint: string, + body?: unknown + ): Promise { + // If endpoint is already a full URL, use it directly; otherwise prepend baseUrl + const url = endpoint.startsWith('http://') || endpoint.startsWith('https://') + ? new URL(endpoint) + : new URL(`${this.baseUrl}${endpoint}`) + getLogger().debug(`BitBucket API ${method} request`, { url: url.toString() }) + + return new Promise((resolve, reject) => { + const options: https.RequestOptions = { + hostname: url.hostname, + port: url.port || 443, + path: url.pathname + url.search, + method, + headers: { + 'Authorization': this.authHeader, + 'Accept': 'application/json', + 'Content-Type': 'application/json', + }, + } + + const req = https.request(options, (res) => { + let data = '' + + res.on('data', (chunk) => { + data += chunk + }) + + res.on('end', () => { + if (!res.statusCode || res.statusCode < 200 || res.statusCode >= 300) { + reject(new Error(`BitBucket API error (${res.statusCode}): ${data}`)) + return + } + + // Handle empty response + if (res.statusCode === 204 || !data) { + resolve({} as T) + return + } + + try { + resolve(JSON.parse(data) as T) + } catch (error) { + reject(new Error(`Failed to parse BitBucket API response: ${error}`)) + } + }) + }) + + req.on('error', (error) => { + reject(new Error(`BitBucket API request failed: ${error.message}`)) + }) + + if (body) { + req.write(JSON.stringify(body)) + } + + req.end() + }) + } + + /** + * Make a GET request to BitBucket API + */ + private async get(endpoint: string): Promise { + return this.request('GET', endpoint) + } + + /** + * Make a POST request to BitBucket API + */ + private async post(endpoint: string, body: unknown): Promise { + return this.request('POST', endpoint, body) + } + + /** + * Get repository information + */ + async getRepository(workspace: string, repoSlug: string): Promise { + return this.get(`/repositories/${workspace}/${repoSlug}`) + } + + /** + * Get a pull request by ID + */ + async getPullRequest( + workspace: string, + repoSlug: string, + prId: number + ): Promise { + return this.get( + `/repositories/${workspace}/${repoSlug}/pullrequests/${prId}` + ) + } + + /** + * List open pull requests for a branch + * + * Note: BitBucket uses BBQL (BitBucket Query Language) for filtering. + * The q parameter must use the format: q=source.branch.name="branch-name" + * When using BBQL, we include state filter in the query to ensure it's applied. + * See: https://developer.atlassian.com/cloud/bitbucket/rest/intro/#filtering + */ + async listPullRequests( + workspace: string, + repoSlug: string, + sourceBranch?: string + ): Promise { + let endpoint = `/repositories/${workspace}/${repoSlug}/pullrequests` + + if (sourceBranch) { + // Use BBQL query syntax for filtering by source branch AND state + // Include state="OPEN" in the query to exclude DECLINED/MERGED/SUPERSEDED PRs + const query = `state="OPEN" AND source.branch.name="${sourceBranch}"` + endpoint += `?q=${encodeURIComponent(query)}` + } else { + // No branch filter, just filter by state + endpoint += `?state=OPEN` + } + + const response = await this.get<{ values: BitBucketPullRequest[] }>(endpoint) + return response.values + } + + /** + * Create a pull request + */ + async createPullRequest( + workspace: string, + repoSlug: string, + title: string, + description: string, + sourceBranch: string, + destinationBranch: string, + reviewerAccountIds?: string[] + ): Promise { + const payload: Record = { + title, + description, + source: { + branch: { + name: sourceBranch, + }, + }, + destination: { + branch: { + name: destinationBranch, + }, + }, + } + + // Add reviewers if provided + if (reviewerAccountIds && reviewerAccountIds.length > 0) { + payload.reviewers = reviewerAccountIds.map(id => ({ account_id: id })) + } + + return this.post( + `/repositories/${workspace}/${repoSlug}/pullrequests`, + payload + ) + } + + /** + * Add a comment to a pull request + */ + async addPRComment( + workspace: string, + repoSlug: string, + prId: number, + content: string + ): Promise { + await this.post( + `/repositories/${workspace}/${repoSlug}/pullrequests/${prId}/comments`, + { + content: { + raw: content, + }, + } + ) + } + + /** + * Find workspace members by usernames + * Returns a map of username -> account_id for resolved users + * Handles pagination to fetch all workspace members + */ + async findUsersByUsername( + workspace: string, + usernames: string[] + ): Promise> { + const result = new Map() + + // Fetch all workspace members with pagination + const allMembers = await this.getAllWorkspaceMembers(workspace) + + getLogger().debug(`Resolving ${usernames.length} usernames against ${allMembers.length} workspace members`, { allMembers}) + + // Match usernames against fetched members + for (const username of usernames) { + const usernameLower = username.toLowerCase() + const member = allMembers.find(m => + m.user.nickname?.toLowerCase() === usernameLower || + m.user.display_name.toLowerCase() === usernameLower + ) + + if (member) { + result.set(username, member.user.account_id) + getLogger().debug(`Resolved reviewer ${username} to account ID ${member.user.account_id}`) + } else { + getLogger().warn(`Could not resolve reviewer ${username} to a BitBucket account ID`) + } + } + + return result + } + + /** + * Fetch all workspace members with pagination + */ + private async getAllWorkspaceMembers(workspace: string): Promise { + const allMembers: BitBucketWorkspaceMember[] = [] + let nextUrl: string | null = `/workspaces/${workspace}/members` + + while (nextUrl) { + const response: BitBucketWorkspaceMembersResponse = + await this.get(nextUrl) + + allMembers.push(...response.values) + + // BitBucket pagination uses 'next' field with full URL + // Use it directly since request() now handles full URLs + nextUrl = response.next ?? null + } + + getLogger().debug(`Fetched ${allMembers.length} workspace members from BitBucket`) + return allMembers + } + + /** + * Get the currently authenticated user + */ + async getCurrentUser(): Promise { + return this.get('/user') + } + + /** + * Test connection to BitBucket API + */ + async testConnection(): Promise { + try { + await this.getCurrentUser() + return true + } catch (error) { + getLogger().error('BitBucket connection test failed', { error }) + return false + } + } + + /** + * Get configured workspace + */ + getWorkspace(): string | undefined { + return this.workspace + } + + /** + * Get configured repository slug + */ + getRepoSlug(): string | undefined { + return this.repoSlug + } +} diff --git a/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts b/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts new file mode 100644 index 00000000..3ce770b9 --- /dev/null +++ b/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts @@ -0,0 +1,381 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { BitBucketVCSProvider, type BitBucketVCSConfig } from './BitBucketVCSProvider.js' +import { BitBucketApiClient } from './BitBucketApiClient.js' + +// Mock the BitBucketApiClient +vi.mock('./BitBucketApiClient.js', () => ({ + BitBucketApiClient: vi.fn().mockImplementation(() => ({ + getWorkspace: vi.fn().mockReturnValue('test-workspace'), + getRepoSlug: vi.fn().mockReturnValue('test-repo'), + createPullRequest: vi.fn(), + findUsersByUsername: vi.fn(), + getCurrentUser: vi.fn(), + listPullRequests: vi.fn(), + getPullRequest: vi.fn(), + addPRComment: vi.fn(), + })), +})) + +// Mock the logger +vi.mock('../../../utils/logger-context.js', () => ({ + getLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})) + +// Mock the remote parser +vi.mock('../../../utils/remote.js', () => ({ + parseGitRemotes: vi.fn().mockResolvedValue([]), +})) + +describe('BitBucketVCSProvider', () => { + let provider: BitBucketVCSProvider + let mockClient: { + getWorkspace: ReturnType + getRepoSlug: ReturnType + createPullRequest: ReturnType + findUsersByUsername: ReturnType + getCurrentUser: ReturnType + listPullRequests: ReturnType + getPullRequest: ReturnType + addPRComment: ReturnType + } + + beforeEach(() => { + vi.clearAllMocks() + // Get the mock client instance + mockClient = { + getWorkspace: vi.fn().mockReturnValue('test-workspace'), + getRepoSlug: vi.fn().mockReturnValue('test-repo'), + createPullRequest: vi.fn(), + findUsersByUsername: vi.fn(), + getCurrentUser: vi.fn().mockResolvedValue({ + account_id: 'acc-current-user', + display_name: 'Current User', + nickname: 'currentuser', + }), + listPullRequests: vi.fn(), + getPullRequest: vi.fn(), + addPRComment: vi.fn(), + } + vi.mocked(BitBucketApiClient).mockImplementation(() => mockClient as unknown as BitBucketApiClient) + }) + + describe('createPR with reviewers', () => { + it('should resolve reviewer usernames and pass account IDs to createPullRequest', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + reviewers: ['alice', 'bob'], + } + provider = new BitBucketVCSProvider(config) + + // Mock username resolution + mockClient.findUsersByUsername.mockResolvedValue( + new Map([ + ['alice', 'acc-alice'], + ['bob', 'acc-bob'], + ]) + ) + + // Mock PR creation + mockClient.createPullRequest.mockResolvedValue({ + id: 123, + title: 'Test PR', + description: 'Test body', + state: 'OPEN', + author: { display_name: 'Test', uuid: 'uuid' }, + source: { branch: { name: 'feature' } }, + destination: { branch: { name: 'main' } }, + created_on: '2024-01-01', + updated_on: '2024-01-01', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + }) + + const url = await provider.createPR('feature', 'Test PR', 'Test body', 'main') + + // Verify findUsersByUsername was called with the configured usernames + expect(mockClient.findUsersByUsername).toHaveBeenCalledWith( + 'test-workspace', + ['alice', 'bob'] + ) + + // Verify createPullRequest was called with resolved account IDs + expect(mockClient.createPullRequest).toHaveBeenCalledWith( + 'test-workspace', + 'test-repo', + 'Test PR', + 'Test body', + 'feature', + 'main', + ['acc-alice', 'acc-bob'] + ) + + expect(url).toBe('https://bitbucket.org/test/pr/123') + }) + + it('should continue with partial reviewers when some usernames cannot be resolved', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + reviewers: ['alice', 'unknown_user'], + } + provider = new BitBucketVCSProvider(config) + + // Only alice resolves + mockClient.findUsersByUsername.mockResolvedValue( + new Map([['alice', 'acc-alice']]) + ) + + mockClient.createPullRequest.mockResolvedValue({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + }) + + await provider.createPR('feature', 'Test PR', 'Test body', 'main') + + // Should only pass the resolved reviewer + expect(mockClient.createPullRequest).toHaveBeenCalledWith( + 'test-workspace', + 'test-repo', + 'Test PR', + 'Test body', + 'feature', + 'main', + ['acc-alice'] + ) + }) + + it('should not pass reviewers when no usernames can be resolved', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + reviewers: ['unknown_user'], + } + provider = new BitBucketVCSProvider(config) + + // No usernames resolve + mockClient.findUsersByUsername.mockResolvedValue(new Map()) + + mockClient.createPullRequest.mockResolvedValue({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + }) + + await provider.createPR('feature', 'Test PR', 'Test body', 'main') + + // Should pass empty array for reviewers + expect(mockClient.createPullRequest).toHaveBeenCalledWith( + 'test-workspace', + 'test-repo', + 'Test PR', + 'Test body', + 'feature', + 'main', + [] + ) + }) + + it('should not resolve reviewers when none are configured', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + // No reviewers configured + } + provider = new BitBucketVCSProvider(config) + + mockClient.createPullRequest.mockResolvedValue({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + }) + + await provider.createPR('feature', 'Test PR', 'Test body', 'main') + + // findUsersByUsername should not be called + expect(mockClient.findUsersByUsername).not.toHaveBeenCalled() + + // createPullRequest should be called without reviewers + expect(mockClient.createPullRequest).toHaveBeenCalledWith( + 'test-workspace', + 'test-repo', + 'Test PR', + 'Test body', + 'feature', + 'main', + undefined + ) + }) + + it('should not resolve reviewers when array is empty', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + reviewers: [], + } + provider = new BitBucketVCSProvider(config) + + mockClient.createPullRequest.mockResolvedValue({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + }) + + await provider.createPR('feature', 'Test PR', 'Test body', 'main') + + // findUsersByUsername should not be called + expect(mockClient.findUsersByUsername).not.toHaveBeenCalled() + + // createPullRequest should be called without reviewers + expect(mockClient.createPullRequest).toHaveBeenCalledWith( + 'test-workspace', + 'test-repo', + 'Test PR', + 'Test body', + 'feature', + 'main', + undefined + ) + }) + + it('should filter out the current user from reviewers list', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + reviewers: ['alice', 'currentuser'], // currentuser is the PR author + } + provider = new BitBucketVCSProvider(config) + + // Current user has account_id 'acc-current-user' (set in beforeEach) + mockClient.findUsersByUsername.mockResolvedValue( + new Map([ + ['alice', 'acc-alice'], + ['currentuser', 'acc-current-user'], // Same as current user + ]) + ) + + mockClient.createPullRequest.mockResolvedValue({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + }) + + await provider.createPR('feature', 'Test PR', 'Test body', 'main') + + // getCurrentUser should be called to get the current user's account ID + expect(mockClient.getCurrentUser).toHaveBeenCalled() + + // createPullRequest should be called with only alice (current user filtered out) + expect(mockClient.createPullRequest).toHaveBeenCalledWith( + 'test-workspace', + 'test-repo', + 'Test PR', + 'Test body', + 'feature', + 'main', + ['acc-alice'] + ) + }) + + it('should pass all reviewers when current user is not in the list', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + reviewers: ['alice', 'bob'], + } + provider = new BitBucketVCSProvider(config) + + mockClient.findUsersByUsername.mockResolvedValue( + new Map([ + ['alice', 'acc-alice'], + ['bob', 'acc-bob'], + ]) + ) + + mockClient.createPullRequest.mockResolvedValue({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + }) + + await provider.createPR('feature', 'Test PR', 'Test body', 'main') + + // All reviewers should be passed (none filtered) + expect(mockClient.createPullRequest).toHaveBeenCalledWith( + 'test-workspace', + 'test-repo', + 'Test PR', + 'Test body', + 'feature', + 'main', + ['acc-alice', 'acc-bob'] + ) + }) + + it('should pass empty array when current user is the only reviewer', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + reviewers: ['currentuser'], + } + provider = new BitBucketVCSProvider(config) + + mockClient.findUsersByUsername.mockResolvedValue( + new Map([['currentuser', 'acc-current-user']]) + ) + + mockClient.createPullRequest.mockResolvedValue({ + id: 123, + title: 'Test PR', + links: { html: { href: 'https://bitbucket.org/test/pr/123' } }, + }) + + await provider.createPR('feature', 'Test PR', 'Test body', 'main') + + // createPullRequest should be called with empty array (current user filtered out) + expect(mockClient.createPullRequest).toHaveBeenCalledWith( + 'test-workspace', + 'test-repo', + 'Test PR', + 'Test body', + 'feature', + 'main', + [] + ) + }) + }) + + describe('provider properties', () => { + it('should have correct provider name', () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + expect(provider.providerName).toBe('bitbucket') + }) + + it('should not support draft PRs', () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + expect(provider.supportsDraftPRs).toBe(false) + }) + + it('should support forks', () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + expect(provider.supportsForks).toBe(true) + }) + }) +}) diff --git a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts new file mode 100644 index 00000000..c876f44a --- /dev/null +++ b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts @@ -0,0 +1,276 @@ +// BitBucketVCSProvider - Implements VersionControlProvider for BitBucket +// Provides PR/VCS operations via BitBucket REST API + +import type { VersionControlProvider, ExistingPR } from '../../VersionControlProvider.js' +import type { PullRequest } from '../../../types/index.js' +import { BitBucketApiClient, type BitBucketConfig, type BitBucketPullRequest } from './BitBucketApiClient.js' +import { getLogger } from '../../../utils/logger-context.js' +import { parseGitRemotes } from '../../../utils/remote.js' + +/** + * BitBucket-specific configuration + * Extends BitBucketConfig with username, appPassword, workspace, and repoSlug + */ +export interface BitBucketVCSConfig extends BitBucketConfig { + reviewers?: string[] // Usernames of reviewers to add to PRs +} + +/** + * BitBucketVCSProvider implements VersionControlProvider for BitBucket + * + * Key differences from GitHub: + * - Uses workspace/repository slug instead of owner/repo + * - PR states are different (OPEN, MERGED, DECLINED, SUPERSEDED) + * - No native draft PR support + */ +export class BitBucketVCSProvider implements VersionControlProvider { + readonly providerName = 'bitbucket' + readonly supportsForks = true + readonly supportsDraftPRs = false // BitBucket doesn't have draft PRs + + private readonly client: BitBucketApiClient + private readonly reviewerUsernames?: string[] + + constructor(config: BitBucketVCSConfig) { + this.client = new BitBucketApiClient(config) + if (config.reviewers) { + this.reviewerUsernames = config.reviewers + } + } + + /** + * Check if a PR already exists for the given branch + */ + async checkForExistingPR(branchName: string, cwd?: string): Promise { + try { + // Get workspace and repo slug from config or detect from git remote + const { workspace, repoSlug } = await this.getWorkspaceAndRepo(cwd) + + const prs = await this.client.listPullRequests(workspace, repoSlug, branchName) + + if (prs.length > 0 && prs[0]) { + const pr = prs[0] + return { + number: pr.id, + url: pr.links.html.href, + } + } + + return null + } catch (error) { + getLogger().debug('Error checking for existing PR', { error }) + return null + } + } + + /** + * Create a pull request + */ + async createPR( + branchName: string, + title: string, + body: string, + baseBranch: string, + cwd?: string + ): Promise { + const { workspace, repoSlug } = await this.getWorkspaceAndRepo(cwd) + + // Log the target repository so users can verify it's correct + getLogger().info(`Creating BitBucket PR in ${workspace}/${repoSlug}`) + getLogger().debug('PR details', { branchName, title, baseBranch }) + + // Resolve reviewer usernames to account IDs if configured + let reviewerIds: string[] | undefined + if (this.reviewerUsernames && this.reviewerUsernames.length > 0) { + reviewerIds = await this.resolveReviewerUsernames(workspace, this.reviewerUsernames) + + // Filter out the current user from reviewers (BitBucket doesn't allow PR author as reviewer) + if (reviewerIds.length > 0) { + const currentUser = await this.client.getCurrentUser() + const originalCount = reviewerIds.length + reviewerIds = reviewerIds.filter(id => id !== currentUser.account_id) + + if (reviewerIds.length < originalCount) { + getLogger().debug( + `Removed current user (${currentUser.display_name}) from reviewers list - PR author cannot be a reviewer` + ) + } + } + } + + const pr = await this.client.createPullRequest( + workspace, + repoSlug, + title, + body, + branchName, + baseBranch, + reviewerIds + ) + + // Validate the response structure + if (!pr?.id || !pr?.links?.html?.href) { + getLogger().error('Invalid BitBucket API response', { pr }) + throw new Error( + `BitBucket API returned invalid PR response. ` + + `Expected PR with id and links.html.href, got: ${JSON.stringify(pr)}` + ) + } + + getLogger().info(`BitBucket PR #${pr.id} created successfully`) + return pr.links.html.href + } + + /** + * Fetch PR details + */ + async fetchPR(prNumber: number, cwd?: string): Promise { + const { workspace, repoSlug } = await this.getWorkspaceAndRepo(cwd) + + const bbPR = await this.client.getPullRequest(workspace, repoSlug, prNumber) + return this.mapBitBucketPRToPullRequest(bbPR) + } + + /** + * Get PR URL + */ + async getPRUrl(prNumber: number, cwd?: string): Promise { + const { workspace, repoSlug } = await this.getWorkspaceAndRepo(cwd) + + const bbPR = await this.client.getPullRequest(workspace, repoSlug, prNumber) + return bbPR.links.html.href + } + + /** + * Create a comment on a PR + */ + async createPRComment(prNumber: number, body: string, cwd?: string): Promise { + const { workspace, repoSlug } = await this.getWorkspaceAndRepo(cwd) + + getLogger().debug('Creating BitBucket PR comment', { workspace, repoSlug, prNumber }) + + await this.client.addPRComment(workspace, repoSlug, prNumber, body) + } + + /** + * Detect repository from git remote + */ + async detectRepository(cwd?: string): Promise<{ owner: string; repo: string } | null> { + try { + const remotes = await parseGitRemotes(cwd) + + // Look for bitbucket.org remote + const bbRemote = remotes.find(r => + r.url.includes('bitbucket.org') + ) + + if (!bbRemote) { + return null + } + + // BitBucket URLs: https://bitbucket.org/workspace/repo.git + // or git@bitbucket.org:workspace/repo.git + return { + owner: bbRemote.owner, // workspace + repo: bbRemote.repo, + } + } catch (error) { + getLogger().error('Failed to detect BitBucket repository', { error }) + return null + } + } + + /** + * Get target remote for PR operations + */ + async getTargetRemote(_cwd?: string): Promise { + // For BitBucket, we typically use 'origin' + // Fork workflows are less common in BitBucket + return 'origin' + } + + /** + * Get workspace and repository slug from config or git remote + */ + private async getWorkspaceAndRepo(cwd?: string): Promise<{ workspace: string; repoSlug: string }> { + let workspace = this.client.getWorkspace() + let repoSlug = this.client.getRepoSlug() + + // If not configured, try to detect from git remote + if (!workspace || !repoSlug) { + const detected = await this.detectRepository(cwd) + if (!detected) { + throw new Error( + 'Could not determine BitBucket workspace/repository. ' + + 'Either configure them in settings or ensure git remote points to bitbucket.org' + ) + } + + workspace = workspace ?? detected.owner + repoSlug = repoSlug ?? detected.repo + } + + return { workspace, repoSlug } + } + + /** + * Resolve reviewer usernames to BitBucket account IDs + * Warns for any usernames that cannot be resolved but continues with partial list + */ + private async resolveReviewerUsernames(workspace: string, usernames: string[]): Promise { + getLogger().debug(`Resolving ${usernames.length} reviewer username(s) to BitBucket account IDs`) + + const usernameToAccountId = await this.client.findUsersByUsername(workspace, usernames) + + const resolvedIds: string[] = [] + const unresolvedUsernames: string[] = [] + + for (const username of usernames) { + const accountId = usernameToAccountId.get(username) + if (accountId) { + resolvedIds.push(accountId) + } else { + unresolvedUsernames.push(username) + } + } + + if (unresolvedUsernames.length > 0) { + getLogger().warn( + `Could not resolve ${unresolvedUsernames.length} reviewer username(s) to BitBucket account IDs: ${unresolvedUsernames.join(', ')}. ` + + `These reviewers will not be added to the PR.` + ) + } + + if (resolvedIds.length > 0) { + getLogger().info(`Resolved ${resolvedIds.length} reviewer(s) for PR`) + } + + return resolvedIds + } + + /** + * Map BitBucket PR to generic PullRequest type + */ + private mapBitBucketPRToPullRequest(bbPR: BitBucketPullRequest): PullRequest { + // Map BitBucket states to generic states + let state: 'open' | 'closed' | 'merged' + if (bbPR.state === 'OPEN') { + state = 'open' + } else if (bbPR.state === 'MERGED') { + state = 'merged' + } else { + state = 'closed' // DECLINED or SUPERSEDED + } + + return { + number: bbPR.id, + title: bbPR.title, + body: bbPR.description, + state, + branch: bbPR.source.branch.name, + baseBranch: bbPR.destination.branch.name, + url: bbPR.links.html.href, + isDraft: false, // BitBucket doesn't have draft PRs + } + } +} diff --git a/src/lib/providers/bitbucket/index.ts b/src/lib/providers/bitbucket/index.ts new file mode 100644 index 00000000..c3d4791b --- /dev/null +++ b/src/lib/providers/bitbucket/index.ts @@ -0,0 +1,3 @@ +// BitBucket provider exports +export { BitBucketApiClient, type BitBucketConfig, type BitBucketPullRequest, type BitBucketRepository } from './BitBucketApiClient.js' +export { BitBucketVCSProvider, type BitBucketVCSConfig } from './BitBucketVCSProvider.js' diff --git a/src/utils/claude.test.ts b/src/utils/claude.test.ts index a95598bd..7fa57c94 100644 --- a/src/utils/claude.test.ts +++ b/src/utils/claude.test.ts @@ -365,7 +365,7 @@ describe('claude utils', () => { expect(execa).toHaveBeenCalledWith( 'claude', - ['-p', '--output-format', 'stream-json', '--verbose', '--add-dir', '/tmp'], + ['-p', '--output-format', 'stream-json', '--verbose', '--add-dir', '/tmp', '--debug'], expect.objectContaining({ input: prompt, timeout: 0, diff --git a/src/utils/claude.ts b/src/utils/claude.ts index 640d6f43..f95f9c16 100644 --- a/src/utils/claude.ts +++ b/src/utils/claude.ts @@ -210,6 +210,11 @@ export async function launchClaude( if (sessionId) { args.push('--session-id', sessionId) } + const isDebugMode = logger.isDebugEnabled() + + if (isDebugMode) { + args.push('--debug') // Enable debug mode for more detailed logs + } // Add --no-session-persistence flag if requested (for utility operations that don't need session persistence) // Note: --no-session-persistence can only be used with --print mode (-p), which is only added in headless mode @@ -223,7 +228,6 @@ export async function launchClaude( try { if (headless) { // Headless mode: capture and return output - const isDebugMode = logger.isDebugEnabled() // Set up execa options based on debug mode const execaOptions = { diff --git a/src/utils/remote.ts b/src/utils/remote.ts index 9f4c2ac2..31c4a223 100644 --- a/src/utils/remote.ts +++ b/src/utils/remote.ts @@ -54,23 +54,35 @@ export async function parseGitRemotes(cwd?: string): Promise { } /** - * Extract owner and repo from GitHub URL - * Supports both HTTPS and SSH formats + * Extract owner and repo from Git remote URL + * Supports both HTTPS and SSH formats for GitHub and BitBucket */ function extractOwnerRepoFromUrl(url: string): { owner: string; repo: string } | null { // Remove .git suffix if present const cleanUrl = url.replace(/\.git$/, '') - // HTTPS format: https://github.com/owner/repo - const httpsMatch = cleanUrl.match(/https?:\/\/github\.com\/([^/]+)\/([^/]+)/) - if (httpsMatch?.[1] && httpsMatch?.[2]) { - return { owner: httpsMatch[1], repo: httpsMatch[2] } + // GitHub HTTPS format: https://github.com/owner/repo + const githubHttpsMatch = cleanUrl.match(/https?:\/\/github\.com\/([^/]+)\/([^/]+)/) + if (githubHttpsMatch?.[1] && githubHttpsMatch?.[2]) { + return { owner: githubHttpsMatch[1], repo: githubHttpsMatch[2] } } - // SSH format: git@github.com:owner/repo - const sshMatch = cleanUrl.match(/git@github\.com:([^/]+)\/(.+)/) - if (sshMatch?.[1] && sshMatch?.[2]) { - return { owner: sshMatch[1], repo: sshMatch[2] } + // GitHub SSH format: git@github.com:owner/repo + const githubSshMatch = cleanUrl.match(/git@github\.com:([^/]+)\/(.+)/) + if (githubSshMatch?.[1] && githubSshMatch?.[2]) { + return { owner: githubSshMatch[1], repo: githubSshMatch[2] } + } + + // BitBucket HTTPS format: https://bitbucket.org/workspace/repo + const bitbucketHttpsMatch = cleanUrl.match(/https?:\/\/bitbucket\.org\/([^/]+)\/([^/]+)/) + if (bitbucketHttpsMatch?.[1] && bitbucketHttpsMatch?.[2]) { + return { owner: bitbucketHttpsMatch[1], repo: bitbucketHttpsMatch[2] } + } + + // BitBucket SSH format: git@bitbucket.org:workspace/repo + const bitbucketSshMatch = cleanUrl.match(/git@bitbucket\.org:([^/]+)\/(.+)/) + if (bitbucketSshMatch?.[1] && bitbucketSshMatch?.[2]) { + return { owner: bitbucketSshMatch[1], repo: bitbucketSshMatch[2] } } return null From 61b460ca019eb3b42c8eb903b26b76baf85df7e3 Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 09:48:19 -0800 Subject: [PATCH 02/11] feat(issues): fetch PRs from BitBucket when versionControl.provider is bitbucket Implements the TODO(bitbucket) in the issues command to detect the configured VCS provider and use BitBucketApiClient.listPullRequests() instead of fetchGitHubPRList() when provider is 'bitbucket'. Co-Authored-By: Claude Opus 4.6 --- src/commands/issues.ts | 116 +++++++++++++++++++++++++++++++---------- 1 file changed, 88 insertions(+), 28 deletions(-) diff --git a/src/commands/issues.ts b/src/commands/issues.ts index 44ba1590..66ea1a25 100644 --- a/src/commands/issues.ts +++ b/src/commands/issues.ts @@ -6,6 +6,8 @@ import { SettingsManager } from '../lib/SettingsManager.js' import { IssueTrackerFactory } from '../lib/IssueTrackerFactory.js' import { findMainWorktreePathWithSettings } from '../utils/git.js' import { fetchGitHubIssueList, fetchGitHubPRList } from '../utils/github.js' +import { BitBucketApiClient } from '../lib/providers/bitbucket/BitBucketApiClient.js' +import { parseGitRemotes } from '../utils/remote.js' import { fetchLinearIssueList } from '../utils/linear.js' import { fetchJiraIssueList } from '../utils/jira.js' import { JiraApiClient } from '../lib/providers/jira/index.js' @@ -202,34 +204,92 @@ export class IssuesCommand { // Tag issues with type results.forEach(item => { item.type = 'issue' }) - // 6. Fetch PRs from GitHub (PRs are a GitHub concept regardless of issue tracker) - // TODO(bitbucket): detect bitbucket configuration and fetch PRs from Bitbucket instead of GitHub when relevant - try { - const prs = await fetchGitHubPRList({ - limit, - cwd: resolvedProjectPath, - ...(mine ? { mine } : {}), - }) - const prItems: IssueListItem[] = prs.map(pr => ({ ...pr, type: 'pr' as const })) - results = [...results, ...prItems] - } catch (error) { - // Only catch expected, non-fatal errors from gh CLI - // Per CLAUDE.md: "DO NOT SWALLOW ERRORS" -- must check specifically - const stderr = (error as NodeJS.ErrnoException & { stderr?: string }).stderr ?? '' - const isExpectedError = error instanceof Error && ( - error.message.includes('not logged in') || - error.message.includes('auth login') || - error.message.includes('rate limit') || - error.message.includes('ETIMEDOUT') || - error.message.includes('ECONNREFUSED') || - error.message.includes('no git remotes found') || - stderr.includes('not logged in') || - stderr.includes('rate limit') - ) - if (isExpectedError) { - logger.warn(`PR fetch failed (non-fatal), continuing with issues only: ${error.message}`) - } else { - throw error // Re-throw unexpected errors -- do not swallow + // 6. Fetch PRs from VCS provider (GitHub or BitBucket) + const vcsProvider = settings.versionControl?.provider ?? 'github' + + if (vcsProvider === 'bitbucket') { + try { + const bbSettings = settings.versionControl?.bitbucket + const bbUsername = bbSettings?.username + const bbApiToken = bbSettings?.apiToken + if (!bbUsername || !bbApiToken) { + logger.warn('BitBucket username or API token not configured. Skipping PR fetch.') + } else { + const client = new BitBucketApiClient({ + username: bbUsername, + apiToken: bbApiToken, + ...(bbSettings?.workspace ? { workspace: bbSettings.workspace } : {}), + ...(bbSettings?.repoSlug ? { repoSlug: bbSettings.repoSlug } : {}), + }) + + // Detect workspace/repoSlug from git remote if not configured + let workspace = bbSettings?.workspace + let repoSlug = bbSettings?.repoSlug + if (!workspace || !repoSlug) { + const remotes = await parseGitRemotes(resolvedProjectPath) + const bbRemote = remotes.find(r => r.url.includes('bitbucket.org')) + workspace = workspace ?? bbRemote?.owner + repoSlug = repoSlug ?? bbRemote?.repo + } + + if (!workspace || !repoSlug) { + logger.warn('Could not determine BitBucket workspace/repository. Skipping PR fetch.') + } else { + const bbPRs = await client.listPullRequests(workspace, repoSlug) + const prItems: IssueListItem[] = bbPRs.map(pr => ({ + id: String(pr.id), + title: `[PR] ${pr.title}`, + updatedAt: pr.updated_on, + url: pr.links.html.href, + state: pr.state.toLowerCase(), + type: 'pr' as const, + })) + results = [...results, ...prItems] + } + } + } catch (error) { + // Only catch expected, non-fatal BitBucket errors + const isExpectedError = error instanceof Error && ( + error.message.includes('BitBucket API error (401)') || + error.message.includes('BitBucket API error (403)') || + error.message.includes('BitBucket API request failed') || + error.message.includes('ETIMEDOUT') || + error.message.includes('ECONNREFUSED') + ) + if (isExpectedError) { + logger.warn(`BitBucket PR fetch failed (non-fatal), continuing with issues only: ${error.message}`) + } else { + throw error + } + } + } else { + try { + const prs = await fetchGitHubPRList({ + limit, + cwd: resolvedProjectPath, + ...(mine ? { mine } : {}), + }) + const prItems: IssueListItem[] = prs.map(pr => ({ ...pr, type: 'pr' as const })) + results = [...results, ...prItems] + } catch (error) { + // Only catch expected, non-fatal errors from gh CLI + // Per CLAUDE.md: "DO NOT SWALLOW ERRORS" -- must check specifically + const stderr = (error as NodeJS.ErrnoException & { stderr?: string }).stderr ?? '' + const isExpectedError = error instanceof Error && ( + error.message.includes('not logged in') || + error.message.includes('auth login') || + error.message.includes('rate limit') || + error.message.includes('ETIMEDOUT') || + error.message.includes('ECONNREFUSED') || + error.message.includes('no git remotes found') || + stderr.includes('not logged in') || + stderr.includes('rate limit') + ) + if (isExpectedError) { + logger.warn(`PR fetch failed (non-fatal), continuing with issues only: ${error.message}`) + } else { + throw error // Re-throw unexpected errors -- do not swallow + } } } From 8643256ce8645f9174d9caaa6b94109b62f0966c Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 10:19:21 -0800 Subject: [PATCH 03/11] fix(test): mock terminal and env utilities in LoomManager tests Prevent real subprocess spawning (execa for dark mode detection) and file system reads (dotenv-flow) that caused timeouts in non-interactive shells. Uses plain functions instead of vi.fn() to survive vitest mockReset between tests. Co-Authored-By: Claude Opus 4.6 --- src/lib/LoomManager.test.ts | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/lib/LoomManager.test.ts b/src/lib/LoomManager.test.ts index 6b0aba73..0764e51c 100644 --- a/src/lib/LoomManager.test.ts +++ b/src/lib/LoomManager.test.ts @@ -107,6 +107,37 @@ vi.mock('../utils/package-manager.js', () => ({ installDependencies: vi.fn().mockResolvedValue(undefined), })) +// Mock terminal utilities (prevents real execa calls to 'defaults' for dark mode detection) +// Using plain functions to survive vitest mockReset between tests +vi.mock('../utils/terminal.js', () => ({ + detectDarkMode: () => Promise.resolve('light' as const), + detectPlatform: () => 'darwin', + detectITerm2: () => Promise.resolve(false), + openTerminalWindow: () => Promise.resolve(undefined), + openMultipleTerminalWindows: () => Promise.resolve(undefined), + openDualTerminalWindow: () => Promise.resolve(undefined), +})) + +// Mock env utilities (prevents real dotenv-flow file reads) +// Using plain functions to survive vitest mockReset between tests +vi.mock('../utils/env.js', () => ({ + loadEnvIntoProcess: () => ({ parsed: {}, error: undefined }), + isNoEnvFilesFoundError: () => false, + findEnvFileForDatabaseUrl: () => Promise.resolve('.env.local'), + parseEnvFile: () => ({}), + formatEnvLine: () => '', + validateEnvVariable: () => true, + normalizeLineEndings: (s: string) => s, + extractPort: () => null, + isValidEnvKey: () => true, + loadWorkspaceEnv: () => ({ parsed: {} }), + getDotenvFlowFiles: () => [], + getLocalEquivalent: (f: string) => f, + buildEnvSourceCommands: () => '', + findEnvFileContainingVariable: () => Promise.resolve(null), + hasVariableInAnyEnvFile: () => Promise.resolve(false), +})) + // Mock LoomLauncher (dynamically imported) vi.mock('./LoomLauncher.js', () => ({ LoomLauncher: vi.fn(() => ({ From 6206c36a2c5ddb476a9b2427e686817694b6829d Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 10:31:56 -0800 Subject: [PATCH 04/11] refactor: add static fromSettings() to JiraIssueTracker and BitBucketVCSProvider Move settings extraction and validation logic from factories and issues.ts into the provider classes themselves, reducing duplication and simplifying call sites. Also adds listPullRequests() to BitBucketVCSProvider to encapsulate workspace detection + PR listing. Co-Authored-By: Claude Opus 4.6 --- src/commands/issues.ts | 91 ++++-------- src/lib/IssueTrackerFactory.ts | 32 +---- src/lib/VCSProviderFactory.test.ts | 135 ++---------------- src/lib/VCSProviderFactory.ts | 30 +--- .../bitbucket/BitBucketVCSProvider.ts | 42 ++++++ src/lib/providers/jira/JiraIssueTracker.ts | 35 +++++ 6 files changed, 122 insertions(+), 243 deletions(-) diff --git a/src/commands/issues.ts b/src/commands/issues.ts index 66ea1a25..775869f1 100644 --- a/src/commands/issues.ts +++ b/src/commands/issues.ts @@ -6,11 +6,10 @@ import { SettingsManager } from '../lib/SettingsManager.js' import { IssueTrackerFactory } from '../lib/IssueTrackerFactory.js' import { findMainWorktreePathWithSettings } from '../utils/git.js' import { fetchGitHubIssueList, fetchGitHubPRList } from '../utils/github.js' -import { BitBucketApiClient } from '../lib/providers/bitbucket/BitBucketApiClient.js' -import { parseGitRemotes } from '../utils/remote.js' +import { BitBucketVCSProvider } from '../lib/providers/bitbucket/BitBucketVCSProvider.js' import { fetchLinearIssueList } from '../utils/linear.js' import { fetchJiraIssueList } from '../utils/jira.js' -import { JiraApiClient } from '../lib/providers/jira/index.js' +import { JiraIssueTracker } from '../lib/providers/jira/JiraIssueTracker.js' import { getLogger } from '../utils/logger-context.js' /** @@ -169,34 +168,17 @@ export class IssuesCommand { ...(mine ? { mine } : {}), }) } else if (provider === 'jira') { - const jiraSettings = settings.issueManagement?.jira - const host = jiraSettings?.host - if (!host) { - throw new Error( - 'Jira host not configured. Set issueManagement.jira.host in your settings.json.', - ) - } - const username = jiraSettings?.username - if (!username) { - throw new Error( - 'Jira username not configured. Set issueManagement.jira.username in your settings.json.', - ) - } - const apiToken = jiraSettings?.apiToken - if (!apiToken) { - throw new Error( - 'Jira API token not configured. Set issueManagement.jira.apiToken in your settings.json or settings.local.json.', - ) - } - const projectKey = jiraSettings?.projectKey - if (!projectKey) { - throw new Error( - 'Jira project key not configured. Set issueManagement.jira.projectKey in your settings.json.', - ) - } - const doneStatuses = jiraSettings?.doneStatuses - const client = new JiraApiClient({ host, username, apiToken }) - results = await fetchJiraIssueList(client, { host, projectKey, doneStatuses, limit, sprint, mine }) + const tracker = JiraIssueTracker.fromSettings(settings) + const trackerConfig = tracker.getConfig() + const doneStatuses = settings.issueManagement?.jira?.doneStatuses + results = await fetchJiraIssueList(tracker.getApiClient(), { + host: trackerConfig.host, + projectKey: trackerConfig.projectKey, + ...(doneStatuses ? { doneStatuses } : {}), + limit, + sprint, + mine, + }) } else { throw new Error(`Unsupported issue tracker provider: ${provider}`) } @@ -210,42 +192,20 @@ export class IssuesCommand { if (vcsProvider === 'bitbucket') { try { const bbSettings = settings.versionControl?.bitbucket - const bbUsername = bbSettings?.username - const bbApiToken = bbSettings?.apiToken - if (!bbUsername || !bbApiToken) { + if (!bbSettings?.username || !bbSettings?.apiToken) { logger.warn('BitBucket username or API token not configured. Skipping PR fetch.') } else { - const client = new BitBucketApiClient({ - username: bbUsername, - apiToken: bbApiToken, - ...(bbSettings?.workspace ? { workspace: bbSettings.workspace } : {}), - ...(bbSettings?.repoSlug ? { repoSlug: bbSettings.repoSlug } : {}), - }) - - // Detect workspace/repoSlug from git remote if not configured - let workspace = bbSettings?.workspace - let repoSlug = bbSettings?.repoSlug - if (!workspace || !repoSlug) { - const remotes = await parseGitRemotes(resolvedProjectPath) - const bbRemote = remotes.find(r => r.url.includes('bitbucket.org')) - workspace = workspace ?? bbRemote?.owner - repoSlug = repoSlug ?? bbRemote?.repo - } - - if (!workspace || !repoSlug) { - logger.warn('Could not determine BitBucket workspace/repository. Skipping PR fetch.') - } else { - const bbPRs = await client.listPullRequests(workspace, repoSlug) - const prItems: IssueListItem[] = bbPRs.map(pr => ({ - id: String(pr.id), - title: `[PR] ${pr.title}`, - updatedAt: pr.updated_on, - url: pr.links.html.href, - state: pr.state.toLowerCase(), - type: 'pr' as const, - })) - results = [...results, ...prItems] - } + const bbProvider = BitBucketVCSProvider.fromSettings(settings) + const bbPRs = await bbProvider.listPullRequests(resolvedProjectPath) + const prItems: IssueListItem[] = bbPRs.map(pr => ({ + id: String(pr.id), + title: `[PR] ${pr.title}`, + updatedAt: pr.updated_on, + url: pr.links.html.href, + state: pr.state.toLowerCase(), + type: 'pr' as const, + })) + results = [...results, ...prItems] } } catch (error) { // Only catch expected, non-fatal BitBucket errors @@ -253,6 +213,7 @@ export class IssuesCommand { error.message.includes('BitBucket API error (401)') || error.message.includes('BitBucket API error (403)') || error.message.includes('BitBucket API request failed') || + error.message.includes('Could not determine BitBucket workspace/repository') || error.message.includes('ETIMEDOUT') || error.message.includes('ECONNREFUSED') ) diff --git a/src/lib/IssueTrackerFactory.ts b/src/lib/IssueTrackerFactory.ts index 04fed002..647dde2f 100644 --- a/src/lib/IssueTrackerFactory.ts +++ b/src/lib/IssueTrackerFactory.ts @@ -4,7 +4,7 @@ import type { IssueTracker } from './IssueTracker.js' import { GitHubService } from './GitHubService.js' import { LinearService, type LinearServiceConfig } from './LinearService.js' -import { JiraIssueTracker, type JiraTrackerConfig } from './providers/jira/index.js' +import { JiraIssueTracker } from './providers/jira/index.js' import type { IloomSettings } from './SettingsManager.js' import { getLogger } from '../utils/logger-context.js' @@ -55,34 +55,8 @@ export class IssueTrackerFactory { return new LinearService(linearConfig) } case 'jira': { - const jiraSettings = settings.issueManagement?.jira - - if (!jiraSettings?.host) { - throw new Error('Jira host is required. Configure issueManagement.jira.host in .iloom/settings.json') - } - if (!jiraSettings?.username) { - throw new Error('Jira username is required. Configure issueManagement.jira.username in .iloom/settings.json') - } - if (!jiraSettings?.apiToken) { - throw new Error('Jira API token is required. Configure issueManagement.jira.apiToken in .iloom/settings.local.json') - } - if (!jiraSettings?.projectKey) { - throw new Error('Jira project key is required. Configure issueManagement.jira.projectKey in .iloom/settings.json') - } - - const jiraConfig: JiraTrackerConfig = { - host: jiraSettings.host, - username: jiraSettings.username, - apiToken: jiraSettings.apiToken, - projectKey: jiraSettings.projectKey, - } - - if (jiraSettings.transitionMappings) { - jiraConfig.transitionMappings = jiraSettings.transitionMappings - } - - getLogger().debug(`IssueTrackerFactory: Creating JiraIssueTracker for host: ${jiraSettings.host}`) - return new JiraIssueTracker(jiraConfig) + getLogger().debug(`IssueTrackerFactory: Creating JiraIssueTracker from settings`) + return JiraIssueTracker.fromSettings(settings) } default: throw new Error(`Unsupported issue tracker provider: ${provider}`) diff --git a/src/lib/VCSProviderFactory.test.ts b/src/lib/VCSProviderFactory.test.ts index 000e7761..5e03a187 100644 --- a/src/lib/VCSProviderFactory.test.ts +++ b/src/lib/VCSProviderFactory.test.ts @@ -1,14 +1,17 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { VCSProviderFactory } from './VCSProviderFactory.js' -import { BitBucketVCSProvider } from './providers/bitbucket/index.js' import type { IloomSettings } from './SettingsManager.js' // Mock the BitBucketVCSProvider +const { mockBBInstance, mockFromSettings } = vi.hoisted(() => { + const mockBBInstance = { providerName: 'bitbucket' } + const mockFromSettings = vi.fn().mockReturnValue(mockBBInstance) + return { mockBBInstance, mockFromSettings } +}) vi.mock('./providers/bitbucket/index.js', () => ({ - BitBucketVCSProvider: vi.fn().mockImplementation((config) => ({ - providerName: 'bitbucket', - config, - })), + BitBucketVCSProvider: { + fromSettings: mockFromSettings, + }, })) // Mock the logger @@ -23,7 +26,7 @@ vi.mock('../utils/logger-context.js', () => ({ describe('VCSProviderFactory', () => { beforeEach(() => { - vi.clearAllMocks() + mockFromSettings.mockReturnValue(mockBBInstance) }) describe('create', () => { @@ -50,7 +53,7 @@ describe('VCSProviderFactory', () => { expect(result).toBeNull() }) - it('should create BitBucketVCSProvider with basic config', () => { + it('should delegate to BitBucketVCSProvider.fromSettings for bitbucket provider', () => { const settings: IloomSettings = { sourceEnvOnStart: false, attribution: 'upstreamOnly', @@ -63,122 +66,10 @@ describe('VCSProviderFactory', () => { }, } - VCSProviderFactory.create(settings) - - expect(BitBucketVCSProvider).toHaveBeenCalledWith({ - username: 'testuser', - apiToken: 'test-token', - }) - }) - - it('should pass workspace and repoSlug when configured', () => { - const settings: IloomSettings = { - sourceEnvOnStart: false, - attribution: 'upstreamOnly', - versionControl: { - provider: 'bitbucket', - bitbucket: { - username: 'testuser', - apiToken: 'test-token', - workspace: 'my-workspace', - repoSlug: 'my-repo', - }, - }, - } - - VCSProviderFactory.create(settings) - - expect(BitBucketVCSProvider).toHaveBeenCalledWith({ - username: 'testuser', - apiToken: 'test-token', - workspace: 'my-workspace', - repoSlug: 'my-repo', - }) - }) - - it('should pass reviewers when configured', () => { - const settings: IloomSettings = { - sourceEnvOnStart: false, - attribution: 'upstreamOnly', - versionControl: { - provider: 'bitbucket', - bitbucket: { - username: 'testuser', - apiToken: 'test-token', - reviewers: ['alice@example.com', 'bob@example.com'], - }, - }, - } - - VCSProviderFactory.create(settings) - - expect(BitBucketVCSProvider).toHaveBeenCalledWith({ - username: 'testuser', - apiToken: 'test-token', - reviewers: ['alice@example.com', 'bob@example.com'], - }) - }) - - it('should pass all config options together', () => { - const settings: IloomSettings = { - sourceEnvOnStart: false, - attribution: 'upstreamOnly', - versionControl: { - provider: 'bitbucket', - bitbucket: { - username: 'testuser', - apiToken: 'test-token', - workspace: 'my-workspace', - repoSlug: 'my-repo', - reviewers: ['alice@example.com'], - }, - }, - } - - VCSProviderFactory.create(settings) - - expect(BitBucketVCSProvider).toHaveBeenCalledWith({ - username: 'testuser', - apiToken: 'test-token', - workspace: 'my-workspace', - repoSlug: 'my-repo', - reviewers: ['alice@example.com'], - }) - }) - - it('should throw when bitbucket username is missing', () => { - const settings: IloomSettings = { - sourceEnvOnStart: false, - attribution: 'upstreamOnly', - versionControl: { - provider: 'bitbucket', - bitbucket: { - username: '', - apiToken: 'test-token', - }, - }, - } - - expect(() => VCSProviderFactory.create(settings)).toThrow( - 'BitBucket username is required' - ) - }) - - it('should throw when bitbucket apiToken is missing', () => { - const settings: IloomSettings = { - sourceEnvOnStart: false, - attribution: 'upstreamOnly', - versionControl: { - provider: 'bitbucket', - bitbucket: { - username: 'testuser', - }, - }, - } + const result = VCSProviderFactory.create(settings) - expect(() => VCSProviderFactory.create(settings)).toThrow( - 'BitBucket API token is required' - ) + expect(mockFromSettings).toHaveBeenCalledWith(settings) + expect(result).toEqual({ providerName: 'bitbucket' }) }) }) diff --git a/src/lib/VCSProviderFactory.ts b/src/lib/VCSProviderFactory.ts index 871a05dc..8897dfa4 100644 --- a/src/lib/VCSProviderFactory.ts +++ b/src/lib/VCSProviderFactory.ts @@ -2,7 +2,7 @@ // Follows pattern from IssueTrackerFactory import type { VersionControlProvider } from './VersionControlProvider.js' -import { BitBucketVCSProvider, type BitBucketVCSConfig } from './providers/bitbucket/index.js' +import { BitBucketVCSProvider } from './providers/bitbucket/index.js' import type { IloomSettings } from './SettingsManager.js' import { getLogger } from '../utils/logger-context.js' @@ -40,32 +40,8 @@ export class VCSProviderFactory { return null case 'bitbucket': { - const bbSettings = settings.versionControl?.bitbucket - - if (!bbSettings?.username) { - throw new Error('BitBucket username is required. Configure versionControl.bitbucket.username in .iloom/settings.json') - } - if (!bbSettings?.apiToken) { - throw new Error('BitBucket API token is required. Configure versionControl.bitbucket.apiToken in .iloom/settings.local.json') - } - - const bbConfig: BitBucketVCSConfig = { - username: bbSettings.username, - apiToken: bbSettings.apiToken, - } - - if (bbSettings.workspace) { - bbConfig.workspace = bbSettings.workspace - } - if (bbSettings.repoSlug) { - bbConfig.repoSlug = bbSettings.repoSlug - } - if (bbSettings.reviewers) { - bbConfig.reviewers = bbSettings.reviewers - } - - getLogger().debug(`VCSProviderFactory: Creating BitBucketVCSProvider for user: ${bbSettings.username}`) - return new BitBucketVCSProvider(bbConfig) + getLogger().debug(`VCSProviderFactory: Creating BitBucketVCSProvider from settings`) + return BitBucketVCSProvider.fromSettings(settings) } default: diff --git a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts index c876f44a..3524e8c1 100644 --- a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts +++ b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts @@ -4,6 +4,7 @@ import type { VersionControlProvider, ExistingPR } from '../../VersionControlProvider.js' import type { PullRequest } from '../../../types/index.js' import { BitBucketApiClient, type BitBucketConfig, type BitBucketPullRequest } from './BitBucketApiClient.js' +import type { IloomSettings } from '../../SettingsManager.js' import { getLogger } from '../../../utils/logger-context.js' import { parseGitRemotes } from '../../../utils/remote.js' @@ -31,6 +32,38 @@ export class BitBucketVCSProvider implements VersionControlProvider { private readonly client: BitBucketApiClient private readonly reviewerUsernames?: string[] + /** + * Create a BitBucketVCSProvider from IloomSettings + * Extracts and validates BitBucket config from settings + */ + static fromSettings(settings: IloomSettings): BitBucketVCSProvider { + const bbSettings = settings.versionControl?.bitbucket + + if (!bbSettings?.username) { + throw new Error('BitBucket username is required. Configure versionControl.bitbucket.username in .iloom/settings.json') + } + if (!bbSettings?.apiToken) { + throw new Error('BitBucket API token is required. Configure versionControl.bitbucket.apiToken in .iloom/settings.local.json') + } + + const config: BitBucketVCSConfig = { + username: bbSettings.username, + apiToken: bbSettings.apiToken, + } + + if (bbSettings.workspace) { + config.workspace = bbSettings.workspace + } + if (bbSettings.repoSlug) { + config.repoSlug = bbSettings.repoSlug + } + if (bbSettings.reviewers) { + config.reviewers = bbSettings.reviewers + } + + return new BitBucketVCSProvider(config) + } + constructor(config: BitBucketVCSConfig) { this.client = new BitBucketApiClient(config) if (config.reviewers) { @@ -152,6 +185,15 @@ export class BitBucketVCSProvider implements VersionControlProvider { await this.client.addPRComment(workspace, repoSlug, prNumber, body) } + /** + * List open pull requests for the repository + * Uses getWorkspaceAndRepo for auto-detection from git remotes + */ + async listPullRequests(cwd?: string): Promise { + const { workspace, repoSlug } = await this.getWorkspaceAndRepo(cwd) + return this.client.listPullRequests(workspace, repoSlug) + } + /** * Detect repository from git remote */ diff --git a/src/lib/providers/jira/JiraIssueTracker.ts b/src/lib/providers/jira/JiraIssueTracker.ts index 8e3b1602..5bb16bc9 100644 --- a/src/lib/providers/jira/JiraIssueTracker.ts +++ b/src/lib/providers/jira/JiraIssueTracker.ts @@ -4,6 +4,7 @@ import type { IssueTracker } from '../../IssueTracker.js' import type { Issue, IssueTrackerInputDetection } from '../../../types/index.js' import { JiraApiClient, type JiraConfig, type JiraIssue, type JiraTransition } from './JiraApiClient.js' +import type { IloomSettings } from '../../SettingsManager.js' import { getLogger } from '../../../utils/logger-context.js' import { promptConfirmation } from '../../../utils/prompt.js' import { adfToMarkdown } from './AdfMarkdownConverter.js' @@ -35,6 +36,40 @@ export class JiraIssueTracker implements IssueTracker { private readonly config: JiraTrackerConfig private prompter: (message: string) => Promise + /** + * Create a JiraIssueTracker from IloomSettings + * Extracts and validates Jira config from settings + */ + static fromSettings(settings: IloomSettings): JiraIssueTracker { + const jiraSettings = settings.issueManagement?.jira + + if (!jiraSettings?.host) { + throw new Error('Jira host is required. Configure issueManagement.jira.host in .iloom/settings.json') + } + if (!jiraSettings?.username) { + throw new Error('Jira username is required. Configure issueManagement.jira.username in .iloom/settings.json') + } + if (!jiraSettings?.apiToken) { + throw new Error('Jira API token is required. Configure issueManagement.jira.apiToken in .iloom/settings.local.json') + } + if (!jiraSettings?.projectKey) { + throw new Error('Jira project key is required. Configure issueManagement.jira.projectKey in .iloom/settings.json') + } + + const config: JiraTrackerConfig = { + host: jiraSettings.host, + username: jiraSettings.username, + apiToken: jiraSettings.apiToken, + projectKey: jiraSettings.projectKey, + } + + if (jiraSettings.transitionMappings) { + config.transitionMappings = jiraSettings.transitionMappings + } + + return new JiraIssueTracker(config) + } + constructor(config: JiraTrackerConfig, options?: { prompter?: (message: string) => Promise }) { From ce4bbc32040c2f5899bc62f43bde3d8be596dbdd Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 10:36:02 -0800 Subject: [PATCH 05/11] fix(security): redact sensitive tokens from debug log output Add redactSensitiveFields() to mask apiToken, token, secret, and password values in settings debug logs, preventing credential exposure. Co-Authored-By: Claude Opus 4.6 --- src/lib/IssueTrackerFactory.ts | 6 +++--- src/lib/SettingsManager.ts | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/lib/IssueTrackerFactory.ts b/src/lib/IssueTrackerFactory.ts index 647dde2f..5ef2c931 100644 --- a/src/lib/IssueTrackerFactory.ts +++ b/src/lib/IssueTrackerFactory.ts @@ -5,7 +5,7 @@ import type { IssueTracker } from './IssueTracker.js' import { GitHubService } from './GitHubService.js' import { LinearService, type LinearServiceConfig } from './LinearService.js' import { JiraIssueTracker } from './providers/jira/index.js' -import type { IloomSettings } from './SettingsManager.js' +import { type IloomSettings, redactSensitiveFields } from './SettingsManager.js' import { getLogger } from '../utils/logger-context.js' export type IssueTrackerProviderType = 'github' | 'linear' | 'jira' @@ -31,7 +31,7 @@ export class IssueTrackerFactory { const provider = settings.issueManagement?.provider ?? 'github' getLogger().debug(`IssueTrackerFactory: Creating tracker for provider "${provider}"`) - getLogger().debug(`IssueTrackerFactory: issueManagement settings:`, JSON.stringify(settings.issueManagement, null, 2)) + getLogger().debug(`IssueTrackerFactory: issueManagement settings:`, JSON.stringify(redactSensitiveFields(settings.issueManagement), null, 2)) switch (provider) { case 'github': @@ -51,7 +51,7 @@ export class IssueTrackerFactory { linearConfig.apiToken = linearSettings.apiToken } - getLogger().debug(`IssueTrackerFactory: Creating LinearService with config:`, JSON.stringify(linearConfig, null, 2)) + getLogger().debug(`IssueTrackerFactory: Creating LinearService with config:`, JSON.stringify(redactSensitiveFields(linearConfig), null, 2)) return new LinearService(linearConfig) } case 'jira': { diff --git a/src/lib/SettingsManager.ts b/src/lib/SettingsManager.ts index 8d252358..7077208c 100644 --- a/src/lib/SettingsManager.ts +++ b/src/lib/SettingsManager.ts @@ -907,6 +907,30 @@ function redactSensitiveFields(obj: unknown): unknown { return result } +/** + * Recursively redact sensitive fields (tokens, secrets, passwords) from an object. + * Returns a deep copy with sensitive string values replaced by '[REDACTED]'. + */ +export function redactSensitiveFields(obj: unknown): unknown { + if (obj === null || obj === undefined) return obj + if (typeof obj !== 'object') return obj + if (Array.isArray(obj)) return obj.map(redactSensitiveFields) + + const sensitiveKeys = ['apitoken', 'token', 'secret', 'password'] + const result: Record = {} + for (const [key, value] of Object.entries(obj as Record)) { + const lowerKey = key.toLowerCase() + if (sensitiveKeys.some(s => lowerKey.includes(s)) && typeof value === 'string') { + result[key] = '[REDACTED]' + } else if (typeof value === 'object' && value !== null) { + result[key] = redactSensitiveFields(value) + } else { + result[key] = value + } + } + return result +} + /** * Manages project-level settings from .iloom/settings.json */ From 5cf7e4779c82f5f89d0711ee6ade271b592378d4 Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 11:54:27 -0800 Subject: [PATCH 06/11] fix: address code review issues on BitBucket integration branch Fix error swallowing in checkForExistingPR (re-throw 401/403 auth errors), escape branch names in BBQL queries to prevent injection, remove verbose allMembers debug dump, add 'credential' to redaction keys, use dynamic import for BitBucketVCSProvider in issues command, remove dead ProviderCoordinator.ts, fix prTitlePrefix default to false, remove redundant vi.clearAllMocks(), and add test coverage for redactSensitiveFields, checkForExistingPR error handling, and BitBucket URL patterns in remote parsing. Co-Authored-By: Claude Opus 4.6 --- src/cli.ts | 2 +- src/commands/finish.ts | 5 +- src/commands/issues.ts | 2 +- src/lib/ProviderCoordinator.ts | 165 ------------------ src/lib/SettingsManager.test.ts | 85 ++++++++- src/lib/SettingsManager.ts | 23 +-- .../providers/bitbucket/BitBucketApiClient.ts | 5 +- .../bitbucket/BitBucketVCSProvider.test.ts | 101 ++++++++++- .../bitbucket/BitBucketVCSProvider.ts | 9 + src/utils/remote.test.ts | 68 ++++++++ 10 files changed, 270 insertions(+), 195 deletions(-) delete mode 100644 src/lib/ProviderCoordinator.ts diff --git a/src/cli.ts b/src/cli.ts index a871c210..89b69e49 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -610,7 +610,7 @@ program .option('-n, --dry-run', 'Preview actions without executing') .option('--pr ', 'Treat input as PR number', parseFloat) .option('--skip-build', 'Skip post-merge build verification') - .option('--skip-to-pr', 'Skip rebase/validation/commit, go directly to PR creation (debug)') + .option('--skip-to-pr', '', { hidden: true }) .option('--no-browser', 'Skip opening PR in browser (github-pr and github-draft-pr modes)') .option('--cleanup', 'Clean up worktree after finishing (default in local mode)') .option('--no-cleanup', 'Keep worktree after finishing') diff --git a/src/commands/finish.ts b/src/commands/finish.ts index 23406a41..d2974285 100644 --- a/src/commands/finish.ts +++ b/src/commands/finish.ts @@ -1229,9 +1229,8 @@ export class FinishCommand { try { const issue = await this.issueTracker.fetchIssue(parsed.number) - // Apply ticket prefix if enabled (default: true) - const usePrefix = settings.mergeBehavior?.prTitlePrefix; - if (usePrefix) { + // Apply ticket prefix if enabled (default: false) + if (settings.mergeBehavior?.prTitlePrefix) { prTitle = `${parsed.number}: ${issue.title}` } else { prTitle = issue.title diff --git a/src/commands/issues.ts b/src/commands/issues.ts index 775869f1..25af4bd6 100644 --- a/src/commands/issues.ts +++ b/src/commands/issues.ts @@ -6,7 +6,6 @@ import { SettingsManager } from '../lib/SettingsManager.js' import { IssueTrackerFactory } from '../lib/IssueTrackerFactory.js' import { findMainWorktreePathWithSettings } from '../utils/git.js' import { fetchGitHubIssueList, fetchGitHubPRList } from '../utils/github.js' -import { BitBucketVCSProvider } from '../lib/providers/bitbucket/BitBucketVCSProvider.js' import { fetchLinearIssueList } from '../utils/linear.js' import { fetchJiraIssueList } from '../utils/jira.js' import { JiraIssueTracker } from '../lib/providers/jira/JiraIssueTracker.js' @@ -195,6 +194,7 @@ export class IssuesCommand { if (!bbSettings?.username || !bbSettings?.apiToken) { logger.warn('BitBucket username or API token not configured. Skipping PR fetch.') } else { + const { BitBucketVCSProvider } = await import('../lib/providers/bitbucket/BitBucketVCSProvider.js') const bbProvider = BitBucketVCSProvider.fromSettings(settings) const bbPRs = await bbProvider.listPullRequests(resolvedProjectPath) const prItems: IssueListItem[] = bbPRs.map(pr => ({ diff --git a/src/lib/ProviderCoordinator.ts b/src/lib/ProviderCoordinator.ts deleted file mode 100644 index e49d144d..00000000 --- a/src/lib/ProviderCoordinator.ts +++ /dev/null @@ -1,165 +0,0 @@ -// ProviderCoordinator - Orchestrates workflows between IssueTracker and VersionControlProvider -// Manages the interaction between issue tracking and version control systems - -import type { IssueTracker } from './IssueTracker.js' -import type { VersionControlProvider } from './VersionControlProvider.js' -import { getLogger } from '../utils/logger-context.js' - -/** - * Options for posting agent output - */ -export interface PostAgentOutputOptions { - issueNumber?: string | number - prNumber?: number - body: string - cwd?: string -} - -/** - * Options for finish workflow - */ -export interface FinishWorkflowOptions { - branchName: string - title: string - body: string - baseBranch: string - issueNumber?: string | number - transitionState?: string - cwd?: string -} - -/** - * Result of finish workflow - */ -export interface FinishWorkflowResult { - prUrl: string - prNumber: number - issueTransitioned: boolean -} - -/** - * ProviderCoordinator orchestrates workflows across issue tracking and version control providers. - * - * Key responsibilities: - * - Route agent output to the correct destination (issue vs PR) - * - Coordinate PR creation with issue state transitions - * - Provide a unified interface for start/finish workflows - * - * Design pattern: - * - Uses composition over inheritance - * - Delegates provider-specific operations to injected providers - * - Handles cross-provider coordination logic - */ -export class ProviderCoordinator { - constructor( - private issueTracker: IssueTracker, - private vcsProvider: VersionControlProvider - ) {} - - /** - * Post agent output to the appropriate destination - * - If PR number provided, post to PR - * - Otherwise, post to issue - */ - async postAgentOutput(options: PostAgentOutputOptions): Promise { - const { issueNumber, prNumber, body, cwd } = options - - if (prNumber) { - // Post to PR via VCS provider - getLogger().debug('Posting agent output to PR', { prNumber }) - await this.vcsProvider.createPRComment(prNumber, body, cwd) - } else if (issueNumber) { - // Post to issue via issue tracker - getLogger().debug('Posting agent output to issue', { issueNumber }) - // Note: This will need the MCP server-based approach or direct API call - // For now, we'll throw since this needs to be integrated with the MCP system - throw new Error('Issue comment posting not yet implemented in coordinator') - } else { - throw new Error('Either issueNumber or prNumber must be provided') - } - } - - /** - * Execute finish workflow: - * 1. Create PR via VCS provider - * 2. Post session summary to PR - * 3. Transition issue to target state (e.g., "In Review") - */ - async executeFinishWorkflow(options: FinishWorkflowOptions): Promise { - const { branchName, title, body, baseBranch, issueNumber, transitionState, cwd } = options - - // Step 1: Create PR - getLogger().debug('Creating PR via VCS provider', { branchName, title }) - const prUrl = await this.vcsProvider.createPR(branchName, title, body, baseBranch, cwd) - - // Extract PR number from URL - const prNumber = this.extractPRNumberFromUrl(prUrl) - - getLogger().info('PR created successfully', { prUrl, prNumber }) - - // Step 2: Post session summary to PR (if provided in body) - // The body already contains the session summary, so this is handled by createPR - - // Step 3: Transition issue if requested - let issueTransitioned = false - if (issueNumber && transitionState) { - try { - // Check if issue tracker supports state transitions - if (this.issueTracker.moveIssueToInProgress) { - getLogger().debug('Transitioning issue state', { issueNumber, transitionState }) - // Note: This is a placeholder - actual transition logic will vary by provider - // For now, we only support moveIssueToInProgress - // TODO: Add more flexible transition support - await this.issueTracker.moveIssueToInProgress(issueNumber) - issueTransitioned = true - getLogger().info('Issue transitioned successfully', { issueNumber, transitionState }) - } else { - getLogger().warn('Issue tracker does not support state transitions', { - provider: this.issueTracker.providerName - }) - } - } catch (error) { - // Don't fail the whole workflow if transition fails - getLogger().error('Failed to transition issue', { error, issueNumber }) - } - } - - return { - prUrl, - prNumber, - issueTransitioned, - } - } - - /** - * Extract PR number from PR URL - * Handles various VCS provider URL formats - */ - private extractPRNumberFromUrl(url: string): number { - // GitHub: https://github.com/owner/repo/pull/123 - // BitBucket: https://bitbucket.org/workspace/repo/pull-requests/123 - const githubMatch = url.match(/\/pull\/(\d+)/) - const bitbucketMatch = url.match(/\/pull-requests\/(\d+)/) - - const match = githubMatch ?? bitbucketMatch - if (match?.[1]) { - return parseInt(match[1], 10) - } - - throw new Error(`Failed to extract PR number from URL: ${url}`) - } - - /** - * Get issue tracker instance - */ - getIssueTracker(): IssueTracker { - return this.issueTracker - } - - /** - * Get VCS provider instance - */ - getVCSProvider(): VersionControlProvider { - return this.vcsProvider - } -} diff --git a/src/lib/SettingsManager.test.ts b/src/lib/SettingsManager.test.ts index aeee9355..9cd619ab 100644 --- a/src/lib/SettingsManager.test.ts +++ b/src/lib/SettingsManager.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { SettingsManager } from './SettingsManager.js' +import { SettingsManager, redactSensitiveFields } from './SettingsManager.js' import { readFile } from 'fs/promises' // Mock fs/promises @@ -3485,4 +3485,87 @@ const error: { code?: string; message: string } = { expect(result.agents?.['iloom-swarm-worker']?.agents?.['iloom-issue-planner']?.model).toBe('opus') }) }) + + describe('redactSensitiveFields', () => { + it('should pass through null and undefined', () => { + expect(redactSensitiveFields(null)).toBeNull() + expect(redactSensitiveFields(undefined)).toBeUndefined() + }) + + it('should return primitives unchanged', () => { + expect(redactSensitiveFields('hello')).toBe('hello') + expect(redactSensitiveFields(42)).toBe(42) + expect(redactSensitiveFields(true)).toBe(true) + }) + + it('should redact sensitive keys', () => { + const input = { + apiToken: 'secret-token-123', + accessToken: 'access-abc', + clientSecret: 'my-secret', + password: 'hunter2', + credential: 'cred-xyz', + } + const result = redactSensitiveFields(input) as Record + + expect(result.apiToken).toBe('[REDACTED]') + expect(result.accessToken).toBe('[REDACTED]') + expect(result.clientSecret).toBe('[REDACTED]') + expect(result.password).toBe('[REDACTED]') + expect(result.credential).toBe('[REDACTED]') + }) + + it('should not redact non-sensitive keys', () => { + const input = { + username: 'alice', + host: 'example.com', + port: 8080, + } + const result = redactSensitiveFields(input) as Record + + expect(result.username).toBe('alice') + expect(result.host).toBe('example.com') + expect(result.port).toBe(8080) + }) + + it('should recursively handle nested objects', () => { + const input = { + versionControl: { + bitbucket: { + username: 'alice', + apiToken: 'bb-token-123', + }, + }, + } + const result = redactSensitiveFields(input) as Record + const bb = (result.versionControl as Record).bitbucket as Record + + expect(bb.username).toBe('alice') + expect(bb.apiToken).toBe('[REDACTED]') + }) + + it('should handle arrays', () => { + const input = [ + { apiToken: 'token-1', name: 'first' }, + { apiToken: 'token-2', name: 'second' }, + ] + const result = redactSensitiveFields(input) as Record[] + + expect(result[0].apiToken).toBe('[REDACTED]') + expect(result[0].name).toBe('first') + expect(result[1].apiToken).toBe('[REDACTED]') + expect(result[1].name).toBe('second') + }) + + it('should not redact non-string sensitive values', () => { + const input = { + token: 123, + password: true, + } + const result = redactSensitiveFields(input) as Record + + expect(result.token).toBe(123) + expect(result.password).toBe(true) + }) + }) }) diff --git a/src/lib/SettingsManager.ts b/src/lib/SettingsManager.ts index 7077208c..c9c73541 100644 --- a/src/lib/SettingsManager.ts +++ b/src/lib/SettingsManager.ts @@ -484,7 +484,7 @@ export const IloomSettingsSchema = z.object({ .describe( 'Open the PR in the default browser after finishing in github-pr or github-draft-pr mode. Use --no-browser flag to override.' ), - prTitlePrefix: z.boolean().default(true).optional().describe('Prefix PR titles with the issue number (e.g., "QLH-123: Title"). Default: true'), + prTitlePrefix: z.boolean().default(false).optional().describe('Prefix PR titles with the issue number (e.g., "QLH-123: Title"). Default: false'), }) .optional() .describe('Merge behavior configuration: local (merge locally), github-pr (create PR), github-draft-pr (create draft PR at start, mark ready on finish), or bitbucket-pr (create BitBucket PR)'), @@ -888,25 +888,6 @@ export type IloomSettings = z.infer */ export type IloomSettingsInput = z.input -function redactSensitiveFields(obj: unknown): unknown { - if (obj === null || obj === undefined) return obj - if (typeof obj !== 'object') return obj - if (Array.isArray(obj)) return obj.map(redactSensitiveFields) - const sensitiveKeys = ['apitoken', 'token', 'secret', 'password'] - const result: Record = {} - for (const [key, value] of Object.entries(obj as Record)) { - const lowerKey = key.toLowerCase() - if (sensitiveKeys.some(s => lowerKey.includes(s)) && typeof value === 'string') { - result[key] = '[REDACTED]' - } else if (typeof value === 'object' && value !== null) { - result[key] = redactSensitiveFields(value) - } else { - result[key] = value - } - } - return result -} - /** * Recursively redact sensitive fields (tokens, secrets, passwords) from an object. * Returns a deep copy with sensitive string values replaced by '[REDACTED]'. @@ -916,7 +897,7 @@ export function redactSensitiveFields(obj: unknown): unknown { if (typeof obj !== 'object') return obj if (Array.isArray(obj)) return obj.map(redactSensitiveFields) - const sensitiveKeys = ['apitoken', 'token', 'secret', 'password'] + const sensitiveKeys = ['apitoken', 'token', 'secret', 'password', 'credential'] const result: Record = {} for (const [key, value] of Object.entries(obj as Record)) { const lowerKey = key.toLowerCase() diff --git a/src/lib/providers/bitbucket/BitBucketApiClient.ts b/src/lib/providers/bitbucket/BitBucketApiClient.ts index aa21ace2..f91ca944 100644 --- a/src/lib/providers/bitbucket/BitBucketApiClient.ts +++ b/src/lib/providers/bitbucket/BitBucketApiClient.ts @@ -230,7 +230,8 @@ export class BitBucketApiClient { if (sourceBranch) { // Use BBQL query syntax for filtering by source branch AND state // Include state="OPEN" in the query to exclude DECLINED/MERGED/SUPERSEDED PRs - const query = `state="OPEN" AND source.branch.name="${sourceBranch}"` + const safeBranch = sourceBranch.replace(/\\/g, '\\\\').replace(/"/g, '\\"') + const query = `state="OPEN" AND source.branch.name="${safeBranch}"` endpoint += `?q=${encodeURIComponent(query)}` } else { // No branch filter, just filter by state @@ -312,7 +313,7 @@ export class BitBucketApiClient { // Fetch all workspace members with pagination const allMembers = await this.getAllWorkspaceMembers(workspace) - getLogger().debug(`Resolving ${usernames.length} usernames against ${allMembers.length} workspace members`, { allMembers}) + getLogger().debug(`Resolving ${usernames.length} usernames against ${allMembers.length} workspace members`) // Match usernames against fetched members for (const username of usernames) { diff --git a/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts b/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts index 3ce770b9..3db12bda 100644 --- a/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts +++ b/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts @@ -45,7 +45,6 @@ describe('BitBucketVCSProvider', () => { } beforeEach(() => { - vi.clearAllMocks() // Get the mock client instance mockClient = { getWorkspace: vi.fn().mockReturnValue('test-workspace'), @@ -350,6 +349,106 @@ describe('BitBucketVCSProvider', () => { }) }) + describe('checkForExistingPR', () => { + it('should return existing PR when found', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + + mockClient.listPullRequests.mockResolvedValue([ + { + id: 42, + links: { html: { href: 'https://bitbucket.org/test/repo/pull-requests/42' } }, + }, + ]) + + const result = await provider.checkForExistingPR('feature-branch') + + expect(result).toEqual({ + number: 42, + url: 'https://bitbucket.org/test/repo/pull-requests/42', + }) + }) + + it('should return null when no PR exists', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + + mockClient.listPullRequests.mockResolvedValue([]) + + const result = await provider.checkForExistingPR('feature-branch') + + expect(result).toBeNull() + }) + + it('should propagate 401 authentication errors', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + + mockClient.listPullRequests.mockRejectedValue( + new Error('BitBucket API error (401): Unauthorized') + ) + + await expect(provider.checkForExistingPR('feature-branch')).rejects.toThrow( + 'BitBucket API error (401)' + ) + }) + + it('should propagate 403 forbidden errors', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + + mockClient.listPullRequests.mockRejectedValue( + new Error('BitBucket API error (403): Forbidden') + ) + + await expect(provider.checkForExistingPR('feature-branch')).rejects.toThrow( + 'BitBucket API error (403)' + ) + }) + + it('should return null for network/other errors', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + + mockClient.listPullRequests.mockRejectedValue( + new Error('BitBucket API request failed: ECONNREFUSED') + ) + + const result = await provider.checkForExistingPR('feature-branch') + + expect(result).toBeNull() + }) + + it('should return null for non-Error thrown values', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + + mockClient.listPullRequests.mockRejectedValue('string error') + + const result = await provider.checkForExistingPR('feature-branch') + + expect(result).toBeNull() + }) + }) + describe('provider properties', () => { it('should have correct provider name', () => { const config: BitBucketVCSConfig = { diff --git a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts index 3524e8c1..97339fb7 100644 --- a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts +++ b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts @@ -91,6 +91,15 @@ export class BitBucketVCSProvider implements VersionControlProvider { return null } catch (error) { + if (error instanceof Error) { + const statusMatch = error.message.match(/BitBucket API error \((\d+)\)/) + if (statusMatch?.[1]) { + const statusCode = parseInt(statusMatch[1], 10) + if (statusCode === 401 || statusCode === 403) { + throw error + } + } + } getLogger().debug('Error checking for existing PR', { error }) return null } diff --git a/src/utils/remote.test.ts b/src/utils/remote.test.ts index d24db43b..19fba6de 100644 --- a/src/utils/remote.test.ts +++ b/src/utils/remote.test.ts @@ -115,6 +115,74 @@ describe('remote utils', () => { expect(remotes[0].repo).toBe('repo') }) + it('should parse BitBucket HTTPS remote with .git', async () => { + vi.mocked(execa).mockResolvedValue({ + stdout: 'origin\thttps://bitbucket.org/workspace/repo.git (fetch)\norigin\thttps://bitbucket.org/workspace/repo.git (push)', + } as Partial as ExecaReturnValue) + + const remotes = await parseGitRemotes() + + expect(remotes).toEqual([ + { + name: 'origin', + url: 'https://bitbucket.org/workspace/repo.git', + owner: 'workspace', + repo: 'repo', + }, + ]) + }) + + it('should parse BitBucket HTTPS remote without .git', async () => { + vi.mocked(execa).mockResolvedValue({ + stdout: 'origin\thttps://bitbucket.org/workspace/repo (fetch)\norigin\thttps://bitbucket.org/workspace/repo (push)', + } as Partial as ExecaReturnValue) + + const remotes = await parseGitRemotes() + + expect(remotes).toEqual([ + { + name: 'origin', + url: 'https://bitbucket.org/workspace/repo', + owner: 'workspace', + repo: 'repo', + }, + ]) + }) + + it('should parse BitBucket SSH remote with .git', async () => { + vi.mocked(execa).mockResolvedValue({ + stdout: 'origin\tgit@bitbucket.org:workspace/repo.git (fetch)\norigin\tgit@bitbucket.org:workspace/repo.git (push)', + } as Partial as ExecaReturnValue) + + const remotes = await parseGitRemotes() + + expect(remotes).toEqual([ + { + name: 'origin', + url: 'git@bitbucket.org:workspace/repo.git', + owner: 'workspace', + repo: 'repo', + }, + ]) + }) + + it('should parse BitBucket SSH remote without .git', async () => { + vi.mocked(execa).mockResolvedValue({ + stdout: 'origin\tgit@bitbucket.org:workspace/repo (fetch)\norigin\tgit@bitbucket.org:workspace/repo (push)', + } as Partial as ExecaReturnValue) + + const remotes = await parseGitRemotes() + + expect(remotes).toEqual([ + { + name: 'origin', + url: 'git@bitbucket.org:workspace/repo', + owner: 'workspace', + repo: 'repo', + }, + ]) + }) + it('should deduplicate fetch/push entries', async () => { vi.mocked(execa).mockResolvedValue({ stdout: 'origin\tgit@github.com:user/repo.git (fetch)\norigin\tgit@github.com:user/repo.git (push)', From ec348c3265504bc4ba748844438efc2c992087a5 Mon Sep 17 00:00:00 2001 From: Adam Creeger Date: Sat, 21 Feb 2026 15:06:54 -0500 Subject: [PATCH 07/11] fix: address code review bugs in BitBucket integration - Add bitbucket-pr to JSON mode validation guard to prevent interactive hang in CI/scripted workflows - Add pagination to listPullRequests matching getAllWorkspaceMembers pattern - Add --mine flag support for BitBucket PR listing via current user filtering - Apply prTitlePrefix setting to GitHub PRs for consistency with BitBucket --- src/cli.ts | 2 +- src/commands/finish.ts | 12 ++++++++---- src/commands/issues.ts | 2 +- .../providers/bitbucket/BitBucketApiClient.ts | 19 +++++++++++++++---- .../bitbucket/BitBucketVCSProvider.ts | 11 +++++++++-- 5 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 89b69e49..a871c210 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -610,7 +610,7 @@ program .option('-n, --dry-run', 'Preview actions without executing') .option('--pr ', 'Treat input as PR number', parseFloat) .option('--skip-build', 'Skip post-merge build verification') - .option('--skip-to-pr', '', { hidden: true }) + .option('--skip-to-pr', 'Skip rebase/validation/commit, go directly to PR creation (debug)') .option('--no-browser', 'Skip opening PR in browser (github-pr and github-draft-pr modes)') .option('--cleanup', 'Clean up worktree after finishing (default in local mode)') .option('--no-cleanup', 'Keep worktree after finishing') diff --git a/src/commands/finish.ts b/src/commands/finish.ts index d2974285..69e64092 100644 --- a/src/commands/finish.ts +++ b/src/commands/finish.ts @@ -207,9 +207,9 @@ export class FinishCommand { // JSON mode validation - require explicit flags for interactive prompts if (isJsonMode) { const settings = await this.settingsManager.loadSettings() - // In github-pr mode, require explicit --cleanup or --no-cleanup - if ((settings.mergeBehavior?.mode === 'github-pr' || settings.mergeBehavior?.mode === 'github-draft-pr') && input.options.cleanup === undefined) { - throw new Error('JSON mode with "github-pr"/"github-draft-pr" workflow requires --cleanup or --no-cleanup flag. Use: il finish --json --cleanup ') + // In PR modes, require explicit --cleanup or --no-cleanup + if ((settings.mergeBehavior?.mode === 'github-pr' || settings.mergeBehavior?.mode === 'github-draft-pr' || settings.mergeBehavior?.mode === 'bitbucket-pr') && input.options.cleanup === undefined) { + throw new Error('JSON mode with PR workflow requires --cleanup or --no-cleanup flag. Use: il finish --json --cleanup ') } } @@ -1115,7 +1115,11 @@ export class FinishCommand { // Try to fetch issue title for better PR title try { const issue = await this.issueTracker.fetchIssue(parsed.number) - prTitle = issue.title + if (settings.mergeBehavior?.prTitlePrefix) { + prTitle = `${parsed.number}: ${issue.title}` + } else { + prTitle = issue.title + } } catch (error) { getLogger().debug('Could not fetch issue title, using branch name', { error }) } diff --git a/src/commands/issues.ts b/src/commands/issues.ts index 25af4bd6..c397a0b5 100644 --- a/src/commands/issues.ts +++ b/src/commands/issues.ts @@ -196,7 +196,7 @@ export class IssuesCommand { } else { const { BitBucketVCSProvider } = await import('../lib/providers/bitbucket/BitBucketVCSProvider.js') const bbProvider = BitBucketVCSProvider.fromSettings(settings) - const bbPRs = await bbProvider.listPullRequests(resolvedProjectPath) + const bbPRs = await bbProvider.listPullRequests(resolvedProjectPath, ...(mine ? [{ mine }] : [])) const prItems: IssueListItem[] = bbPRs.map(pr => ({ id: String(pr.id), title: `[PR] ${pr.title}`, diff --git a/src/lib/providers/bitbucket/BitBucketApiClient.ts b/src/lib/providers/bitbucket/BitBucketApiClient.ts index f91ca944..54b4a053 100644 --- a/src/lib/providers/bitbucket/BitBucketApiClient.ts +++ b/src/lib/providers/bitbucket/BitBucketApiClient.ts @@ -78,6 +78,7 @@ export interface BitBucketRepository { } interface BitBucketWorkspaceMembersResponse { values: BitBucketWorkspaceMember[]; next?: string } +interface BitBucketPullRequestsResponse { values: BitBucketPullRequest[]; next?: string } /** * BitBucket current user response from /user endpoint @@ -85,6 +86,7 @@ interface BitBucketWorkspaceMembersResponse { values: BitBucketWorkspaceMember[] export interface BitBucketCurrentUser { account_id: string display_name: string + uuid: string nickname?: string } @@ -232,14 +234,23 @@ export class BitBucketApiClient { // Include state="OPEN" in the query to exclude DECLINED/MERGED/SUPERSEDED PRs const safeBranch = sourceBranch.replace(/\\/g, '\\\\').replace(/"/g, '\\"') const query = `state="OPEN" AND source.branch.name="${safeBranch}"` - endpoint += `?q=${encodeURIComponent(query)}` + endpoint += `?q=${encodeURIComponent(query)}&pagelen=50` } else { // No branch filter, just filter by state - endpoint += `?state=OPEN` + endpoint += `?state=OPEN&pagelen=50` } - const response = await this.get<{ values: BitBucketPullRequest[] }>(endpoint) - return response.values + const allPRs: BitBucketPullRequest[] = [] + let nextUrl: string | null = endpoint + + while (nextUrl) { + const response: BitBucketPullRequestsResponse = await this.get(nextUrl) + allPRs.push(...response.values) + // BitBucket pagination uses 'next' field with full URL + nextUrl = response.next ?? null + } + + return allPRs } /** diff --git a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts index 97339fb7..60e675aa 100644 --- a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts +++ b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts @@ -198,9 +198,16 @@ export class BitBucketVCSProvider implements VersionControlProvider { * List open pull requests for the repository * Uses getWorkspaceAndRepo for auto-detection from git remotes */ - async listPullRequests(cwd?: string): Promise { + async listPullRequests(cwd?: string, options?: { mine?: boolean }): Promise { const { workspace, repoSlug } = await this.getWorkspaceAndRepo(cwd) - return this.client.listPullRequests(workspace, repoSlug) + const prs = await this.client.listPullRequests(workspace, repoSlug) + + if (options?.mine) { + const currentUser = await this.client.getCurrentUser() + return prs.filter(pr => pr.author.uuid === currentUser.uuid) + } + + return prs } /** From 9769600347cea3145608f63f29f74269da049a0a Mon Sep 17 00:00:00 2001 From: Adam Creeger Date: Sun, 22 Feb 2026 13:41:57 -0500 Subject: [PATCH 08/11] chore: hide --skip-to-pr debug flag from help output --- src/cli.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli.ts b/src/cli.ts index a871c210..91b3181b 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -610,7 +610,7 @@ program .option('-n, --dry-run', 'Preview actions without executing') .option('--pr ', 'Treat input as PR number', parseFloat) .option('--skip-build', 'Skip post-merge build verification') - .option('--skip-to-pr', 'Skip rebase/validation/commit, go directly to PR creation (debug)') + .addOption(new Option('--skip-to-pr').hideHelp()) .option('--no-browser', 'Skip opening PR in browser (github-pr and github-draft-pr modes)') .option('--cleanup', 'Clean up worktree after finishing (default in local mode)') .option('--no-cleanup', 'Keep worktree after finishing') From d65c5dac9500ebf8c7c4db417ff5108234c2d51c Mon Sep 17 00:00:00 2001 From: Adam Creeger Date: Sun, 22 Feb 2026 14:00:49 -0500 Subject: [PATCH 09/11] =?UTF-8?q?EPIC:=20Complete=20BitBucket=20VCS=20abst?= =?UTF-8?q?raction=20=E2=80=94=20eliminate=20GitHub-specific=20assumptions?= =?UTF-8?q?=20(#704)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add VCS-conditional CI/CD commands in agent template STEP 5.5 Replace hardcoded gh CLI commands in STEP 5.5 (Auto-Commit and Push) with a Handlebars conditional that branches on IS_BITBUCKET: - GitHub path retains existing gh run list, gh pr view, gh api commands - BitBucket path provides BitBucket Pipelines API guidance via curl Wire IS_BITBUCKET template variable in IgniteCommand.buildTemplateVariables() from settings.versionControl.provider and add it to TemplateVariables interface. fixes #700 * feat: add BitBucket support to init wizard Update init-prompt.txt to include BitBucket as a VCS provider option: - Add VCS provider selection step (Step 1b) between issue tracker and provider-specific config, offering GitHub (default) or BitBucket - Add BitBucket credential configuration step (Step 2b) asking for username, API token (stored in settings.local.json only), and optional workspace slug - Add bitbucket-pr as a merge mode option alongside local, github-pr, github-draft-pr; auto-suggest it when BitBucket VCS is selected - Extract currentVCSProvider, currentBitBucketUsername, currentBitBucketApiToken, currentBitBucketWorkspace in Phase 0 - Display BitBucket fields in Phase 0 current config and Phase 3 summary - Add versionControl JSON generation rules (item 6) with proper file split: provider in settings.json, credentials in settings.local.json - Update mergeBehavior note to include bitbucket-pr as a valid mode - Add BitBucket VCS configuration example in Advanced Configuration - Add combined Jira + BitBucket configuration example showing the full dual-provider setup with proper file separation - Renumber Advanced Configuration items to maintain consistent ordering - Update Phase 9 wrap-up to mention BitBucket PR workflow option fixes #699 * fixes #702 Add vcsProvider field to loom metadata - Add `vcsProvider?: string` to MetadataFile interface - Add `vcsProvider?: string` to WriteMetadataInput interface - Add `vcsProvider: string | null` to LoomMetadata interface - Update toMetadata() to map vcsProvider with null fallback for legacy looms - Update writeMetadata() to persist vcsProvider when provided - Populate vcsProvider in createIloom() and reuseIloom() from settings.versionControl.provider - Add unit tests verifying GitHub and BitBucket VCS provider population and backward compatibility * fixes #697 Replace hardcoded GitHubService fallback in PR detection with VCS-provider-aware routing, so `il start ` works for Jira+BitBucket teams. Both `start.ts` and `LoomManager.fetchIssueData()` now check for a configured `VersionControlProvider` before falling back to GitHub. * fixes #698 Route MCP get_pr, create_comment, and update_comment tools through the configured VCS provider. When versionControl.provider is 'bitbucket' in settings, PR operations use BitBucketVCSProvider instead of hardcoding GitHubIssueManagementProvider. - get_pr: fetches PR data from BitBucket via fetchPR() when configured - create_comment type:pr: posts comment to BitBucket PR via createPRComment() - update_comment type:pr: throws descriptive error (BitBucket REST API does not support PR comment editing) - GitHub path unchanged; no regression for existing GitHub users - BitBucketApiClient does not log Authorization header or credentials - Added unit tests covering both GitHub fallback and BitBucket routing paths * fixes #703 Unify github-pr and bitbucket-pr code paths in finish.ts into a single executeVCSPRWorkflow method that delegates provider-specific operations through VersionControlProvider. Pass null for GitHub (uses PRManager), pass a VersionControlProvider for BitBucket. Reads vcsProvider from loom metadata for routing context when in bitbucket-pr mode. Net reduction of 67 lines. All 4268 tests pass. * fixes #701 Co-Authored-By: Adam Creeger * Fixes #690 --------- Co-authored-by: Adam Creeger --- src/commands/finish.test.ts | 22 +- src/commands/finish.ts | 243 ++++------ src/commands/ignite.ts | 5 + src/commands/plan.ts | 21 + src/commands/start.test.ts | 155 +++++++ src/commands/start.ts | 83 +++- src/commands/summary.ts | 5 + src/lib/LoomManager.ts | 33 +- src/lib/MetadataManager.test.ts | 105 +++++ src/lib/MetadataManager.ts | 5 + src/lib/PRManager.ts | 2 +- src/lib/PromptTemplateManager.ts | 2 + src/lib/SessionSummaryService.ts | 39 +- src/mcp/issue-management-pr-routing.test.ts | 432 ++++++++++++++++++ src/mcp/issue-management-server.ts | 98 +++- templates/prompts/init-prompt.txt | 230 +++++++++- templates/prompts/issue-prompt.txt | 25 + tests/commands/plan-error-messages.test.ts | 160 +++++++ tests/commands/summary-bitbucket.test.ts | 148 ++++++ tests/lib/PRManager-provider-agnostic.test.ts | 76 +++ .../SessionSummaryService-vcs-routing.test.ts | 171 +++++++ 21 files changed, 1824 insertions(+), 236 deletions(-) create mode 100644 src/mcp/issue-management-pr-routing.test.ts create mode 100644 tests/commands/plan-error-messages.test.ts create mode 100644 tests/commands/summary-bitbucket.test.ts create mode 100644 tests/lib/PRManager-provider-agnostic.test.ts create mode 100644 tests/lib/SessionSummaryService-vcs-routing.test.ts diff --git a/src/commands/finish.test.ts b/src/commands/finish.test.ts index 488d8823..253c93b2 100644 --- a/src/commands/finish.test.ts +++ b/src/commands/finish.test.ts @@ -3529,10 +3529,10 @@ describe('FinishCommand', () => { }, }) - // Mock the executeGitHubPRWorkflow method to verify it's called + // Mock the executeVCSPRWorkflow method to verify it's called // (Issue #464: Linear + github-pr should work since PRs go through GitHub CLI) - const executeGitHubPRWorkflowSpy = vi - .spyOn(command as unknown as { executeGitHubPRWorkflow: () => Promise }, 'executeGitHubPRWorkflow') + const executeVCSPRWorkflowSpy = vi + .spyOn(command as unknown as { executeVCSPRWorkflow: () => Promise }, 'executeVCSPRWorkflow') .mockResolvedValue() await command.execute({ @@ -3542,8 +3542,8 @@ describe('FinishCommand', () => { // Rebase runs before PR workflow expect(mockMergeManager.rebaseOnMain).toHaveBeenCalled() - // The github-pr workflow should be executed (not the local merge) - expect(executeGitHubPRWorkflowSpy).toHaveBeenCalled() + // The unified VCS PR workflow should be executed (not the local merge) + expect(executeVCSPRWorkflowSpy).toHaveBeenCalled() // Local merge should NOT be performed (PR workflow handles merging) expect(mockMergeManager.performFastForwardMerge).not.toHaveBeenCalled() }) @@ -3558,11 +3558,11 @@ describe('FinishCommand', () => { }, }) - // Mock the executeGitHubPRWorkflow as fallback handler - // When no draftPrNumber in metadata, github-draft-pr falls back to github-pr workflow + // Mock the executeVCSPRWorkflow as fallback handler + // When no draftPrNumber in metadata, github-draft-pr falls back to the unified VCS PR workflow // (Issue #464: Linear + github-draft-pr should work since PRs go through GitHub CLI) - const executeGitHubPRWorkflowSpy = vi - .spyOn(command as unknown as { executeGitHubPRWorkflow: () => Promise }, 'executeGitHubPRWorkflow') + const executeVCSPRWorkflowSpy = vi + .spyOn(command as unknown as { executeVCSPRWorkflow: () => Promise }, 'executeVCSPRWorkflow') .mockResolvedValue() await command.execute({ @@ -3572,8 +3572,8 @@ describe('FinishCommand', () => { // Rebase runs before PR workflow expect(mockMergeManager.rebaseOnMain).toHaveBeenCalled() - // For github-draft-pr without existing draft PR, it falls back to executeGitHubPRWorkflow - expect(executeGitHubPRWorkflowSpy).toHaveBeenCalled() + // For github-draft-pr without existing draft PR, it falls back to executeVCSPRWorkflow + expect(executeVCSPRWorkflowSpy).toHaveBeenCalled() // Local merge should NOT be performed (PR workflow handles merging) expect(mockMergeManager.performFastForwardMerge).not.toHaveBeenCalled() }) diff --git a/src/commands/finish.ts b/src/commands/finish.ts index 69e64092..4e5d3e55 100644 --- a/src/commands/finish.ts +++ b/src/commands/finish.ts @@ -775,8 +775,8 @@ export class FinishCommand { const mergeBehavior = settings.mergeBehavior ?? { mode: 'local' } if (mergeBehavior.mode === 'github-pr') { - // Execute github-pr workflow instead of local merge - await this.executeGitHubPRWorkflow(parsed, options, worktree, settings, result) + // Execute unified VCS PR workflow (null = GitHub via PRManager) + await this.executeVCSPRWorkflow(parsed, options, worktree, settings, null, result) return } @@ -791,7 +791,7 @@ export class FinishCommand { if (!metadata?.draftPrNumber) { // Fallback: no draft PR exists, treat like github-pr mode getLogger().warn('No draft PR found in metadata, creating new PR...') - await this.executeGitHubPRWorkflow(parsed, options, worktree, settings, result) + await this.executeVCSPRWorkflow(parsed, options, worktree, settings, null, result) return } @@ -900,6 +900,12 @@ export class FinishCommand { if (mergeBehavior.mode === 'bitbucket-pr') { // For BitBucket, we use the VCS provider layer - NOT the issue tracker // This allows Jira/Linear issues to create PRs in BitBucket + // Read vcsProvider from metadata to confirm expected provider, then create from settings + const { MetadataManager: MetadataManagerForBB } = await import('../lib/MetadataManager.js') + const bbMetadataManager = new MetadataManagerForBB() + const bbMetadata = await bbMetadataManager.readMetadata(worktree.path) + const metadataVcsProvider = bbMetadata?.vcsProvider + const { VCSProviderFactory } = await import('../lib/VCSProviderFactory.js') const vcsProvider = VCSProviderFactory.create(settings) @@ -910,7 +916,8 @@ export class FinishCommand { ) } - await this.executeBitBucketPRWorkflow(parsed, options, worktree, settings, vcsProvider, result) + getLogger().debug(`BitBucket PR mode: vcsProvider=${metadataVcsProvider ?? 'not set in metadata (legacy loom)'}`) + await this.executeVCSPRWorkflow(parsed, options, worktree, settings, vcsProvider, result) return } @@ -1087,16 +1094,24 @@ export class FinishCommand { } /** - * Execute workflow for GitHub PR creation (github-pr merge mode) - * Validates → Commits → Pushes → Creates PR → Prompts for cleanup + * Unified VCS PR workflow for github-pr and bitbucket-pr merge modes. + * Pushes branch, generates PR title, creates or finds existing PR via the appropriate + * provider, transitions issue state, generates session summary, archives metadata, + * and handles cleanup prompt. + * + * @param vcsProvider - When null, uses GitHub via PRManager (legacy path). + * When non-null, delegates PR operations to the VCS provider. */ - private async executeGitHubPRWorkflow( + private async executeVCSPRWorkflow( parsed: ParsedFinishInput, options: FinishOptions, worktree: GitWorktree, settings: import('../lib/SettingsManager.js').IloomSettings, + vcsProvider: import('../lib/VersionControlProvider.js').VersionControlProvider | null, finishResult: FinishResult ): Promise { + const providerLabel = vcsProvider ? vcsProvider.providerName : 'GitHub' + // Step 1: Push branch to origin if (options.dryRun) { getLogger().info('[DRY RUN] Would push branch to origin') @@ -1106,13 +1121,10 @@ export class FinishCommand { getLogger().success('Branch pushed successfully') } - // Step 2: Initialize PRManager with settings - const prManager = new PRManager(settings) - - // Step 3: Generate PR title from issue if available + // Step 2: Generate PR title from issue if available + // Note: parsed.number already has correct case from parseInput() metadata lookup let prTitle = `Work from ${worktree.branch}` if (parsed.type === 'issue' && parsed.number) { - // Try to fetch issue title for better PR title try { const issue = await this.issueTracker.fetchIssue(parsed.number) if (settings.mergeBehavior?.prTitlePrefix) { @@ -1125,176 +1137,86 @@ export class FinishCommand { } } - // Step 4: Get base branch (respects parent loom metadata for child looms) + // Step 3: Get base branch (respects parent loom metadata for child looms) const baseBranch = await getMergeTargetBranch(worktree.path) - // Step 5: Create or open PR + // Step 4: Create or open PR if (options.dryRun) { - getLogger().info('[DRY RUN] Would create GitHub PR') + getLogger().info(`[DRY RUN] Would create ${providerLabel} PR`) getLogger().info(` Title: ${prTitle}`) getLogger().info(` Base: ${baseBranch}`) finishResult.operations.push({ type: 'pr-creation', - message: 'Would create GitHub PR (dry-run)', + message: `Would create ${providerLabel} PR (dry-run)`, success: true, }) } else { - const openInBrowser = !options.noBrowser - && !options.json - && settings.mergeBehavior?.openBrowserOnFinish !== false - - const prResult = await prManager.createOrOpenPR( - worktree.branch, - prTitle, - parsed.type === 'issue' ? parsed.number : undefined, - baseBranch, - worktree.path, - openInBrowser - ) - - if (prResult.wasExisting) { - getLogger().success(`Existing pull request: ${prResult.url}`) - finishResult.operations.push({ - type: 'pr-creation', - message: `Found existing pull request`, - success: true, - }) - } else { - getLogger().success(`Pull request created: ${prResult.url}`) - finishResult.operations.push({ - type: 'pr-creation', - message: `Pull request created`, - success: true, - }) - - // Move issue to Ready for Review state - if (parsed.type === 'issue' && parsed.number) { - try { - if (this.issueTracker.moveIssueToReadyForReview) { - await this.issueTracker.moveIssueToReadyForReview(parsed.number) - getLogger().info('Issue moved to Ready for Review') - } - } catch (error) { - getLogger().warn( - `Failed to move issue to Ready for Review: ${error instanceof Error ? error.message : 'Unknown error'}`, - error - ) - } - } - } - - // Set PR URL in result - finishResult.prUrl = prResult.url - - // Step 4.5: Generate session summary (non-blocking, preview-only in dry-run) - // Post to the PR instead of the original issue - await this.generateSessionSummaryIfConfigured(parsed, worktree, options, prResult.number) + // Shared PR body generation (used by non-GitHub providers) + const prManager = new PRManager(settings) - // Step 4.6: Archive metadata BEFORE cleanup prompt (ensures it runs even with --no-cleanup) - const { MetadataManager } = await import('../lib/MetadataManager.js') - const metadataManager = new MetadataManager() - if (!options.dryRun) { - await metadataManager.archiveMetadata(worktree.path) - } + let prUrl: string + let prNumber: number | undefined + let wasExisting: boolean - // Step 5: Interactive cleanup prompt (unless flags override) - await this.handlePRCleanupPrompt(parsed, options, worktree, finishResult) - } - } + if (!vcsProvider) { + // GitHub path: PRManager handles existing check, creation, and browser open + const openInBrowser = !options.noBrowser + && !options.json + && settings.mergeBehavior?.openBrowserOnFinish !== false - /** - * Execute workflow for BitBucket PR creation (bitbucket-pr merge mode) - * Validates -> Commits -> Pushes -> Creates PR -> Prompts for cleanup - * - * Unlike GitHub PR workflow, this uses the VersionControlProvider abstraction - * instead of PRManager, allowing it to work with any issue tracker (Jira, Linear, etc.) - */ - private async executeBitBucketPRWorkflow( - parsed: ParsedFinishInput, - options: FinishOptions, - worktree: GitWorktree, - settings: import('../lib/SettingsManager.js').IloomSettings, - vcsProvider: import('../lib/VersionControlProvider.js').VersionControlProvider, - finishResult: FinishResult - ): Promise { - // Step 1: Push branch to origin - if (options.dryRun) { - getLogger().info('[DRY RUN] Would push branch to origin') - } else { - getLogger().info('Pushing branch to origin...') - await pushBranchToRemote(worktree.branch, worktree.path, { dryRun: false }) - getLogger().success('Branch pushed successfully') - } + const prResult = await prManager.createOrOpenPR( + worktree.branch, + prTitle, + parsed.type === 'issue' ? parsed.number : undefined, + baseBranch, + worktree.path, + openInBrowser + ) - // Step 2: Generate PR title from issue if available - // Note: parsed.number already has correct case from parseInput() metadata lookup - let prTitle = `Work from ${worktree.branch}` - if (parsed.type === 'issue' && parsed.number) { - try { - const issue = await this.issueTracker.fetchIssue(parsed.number) + prUrl = prResult.url + prNumber = prResult.number + wasExisting = prResult.wasExisting + } else { + // VCS provider path (BitBucket, etc.): explicit existing check then create + const existingPR = await vcsProvider.checkForExistingPR(worktree.branch, worktree.path) - // Apply ticket prefix if enabled (default: false) - if (settings.mergeBehavior?.prTitlePrefix) { - prTitle = `${parsed.number}: ${issue.title}` + if (existingPR) { + prUrl = existingPR.url + prNumber = existingPR.number + wasExisting = true } else { - prTitle = issue.title + const prBody = await prManager.generatePRBody( + parsed.type === 'issue' ? parsed.number : undefined, + worktree.path + ) + prUrl = await vcsProvider.createPR( + worktree.branch, + prTitle, + prBody, + baseBranch, + worktree.path + ) + prNumber = undefined // VCS providers return URL only; no number extracted + wasExisting = false } - } catch (error) { - getLogger().debug('Could not fetch issue title, using branch name', { error }) } - } - - // Step 3: Get base branch (respects parent loom metadata for child looms) - const baseBranch = await getMergeTargetBranch(worktree.path) - - // Step 4: Check for existing PR or create new one - if (options.dryRun) { - getLogger().info('[DRY RUN] Would create BitBucket PR') - getLogger().info(` Title: ${prTitle}`) - getLogger().info(` Base: ${baseBranch}`) - finishResult.operations.push({ - type: 'pr-creation', - message: 'Would create BitBucket PR (dry-run)', - success: true, - }) - } else { - // Check for existing PR first - const existingPR = await vcsProvider.checkForExistingPR(worktree.branch, worktree.path) - if (existingPR) { - getLogger().success(`Existing pull request: ${existingPR.url}`) - finishResult.prUrl = existingPR.url + if (wasExisting) { + getLogger().success(`Existing pull request: ${prUrl}`) finishResult.operations.push({ type: 'pr-creation', message: 'Found existing pull request', success: true, }) } else { - // Generate PR body using Claude (same as GitHub workflow) - const { PRManager } = await import('../lib/PRManager.js') - const prManager = new PRManager(settings) - const prBody = await prManager.generatePRBody( - parsed.type === 'issue' ? parsed.number : undefined, - worktree.path - ) - - // Create new PR - const prUrl = await vcsProvider.createPR( - worktree.branch, - prTitle, - prBody, - baseBranch, - worktree.path - ) getLogger().success(`Pull request created: ${prUrl}`) - finishResult.prUrl = prUrl finishResult.operations.push({ type: 'pr-creation', message: 'Pull request created', success: true, }) - // Move issue to Ready for Review state + // Move issue to Ready for Review state only on new PR creation if (parsed.type === 'issue' && parsed.number) { try { if (this.issueTracker.moveIssueToReadyForReview) { @@ -1310,15 +1232,26 @@ export class FinishCommand { } } - // Generate session summary - posts to the ISSUE (Jira/Linear), not the PR - // For BitBucket workflows, the issue tracker (Jira/Linear) doesn't support PR comments, - // so we post to the issue where the knowledge capture belongs - await this.generateSessionSummaryIfConfigured(parsed, worktree, options) + // Set PR URL in result + finishResult.prUrl = prUrl + + // Generate session summary: + // - GitHub: post to the PR (prNumber is available) + // - Other providers (BitBucket): post to the issue, since the issue tracker + // (Jira/Linear) doesn't support PR comments + await this.generateSessionSummaryIfConfigured( + parsed, + worktree, + options, + !vcsProvider ? prNumber : undefined + ) // Archive metadata BEFORE cleanup prompt (ensures it runs even with --no-cleanup) const { MetadataManager } = await import('../lib/MetadataManager.js') const metadataManager = new MetadataManager() - await metadataManager.archiveMetadata(worktree.path) + if (!options.dryRun) { + await metadataManager.archiveMetadata(worktree.path) + } // Interactive cleanup prompt (unless flags override) await this.handlePRCleanupPrompt(parsed, options, worktree, finishResult) diff --git a/src/commands/ignite.ts b/src/commands/ignite.ts index 725b0200..6712e8c4 100644 --- a/src/commands/ignite.ts +++ b/src/commands/ignite.ts @@ -531,6 +531,11 @@ export class IgniteCommand { WORKSPACE_PATH: context.workspacePath, } + // Set VCS provider flag for template conditionals + if (this.settings?.versionControl?.provider === 'bitbucket') { + variables.IS_BITBUCKET = true + } + if (context.issueNumber !== undefined) { variables.ISSUE_NUMBER = context.issueNumber } diff --git a/src/commands/plan.ts b/src/commands/plan.ts index 828c690e..7ebd52f4 100644 --- a/src/commands/plan.ts +++ b/src/commands/plan.ts @@ -302,6 +302,13 @@ export class PlanCommand { throw new Error( `Cannot start planning session after init: ${retryMessage}. Ensure you are in a git repository with a GitHub remote configured.` ) + } else if (provider === 'jira') { + logger.error( + 'Jira issue management requires JIRA_API_TOKEN, JIRA_EMAIL, and JIRA_BASE_URL to be configured.' + ) + throw new Error( + `Cannot start planning session after init: ${retryMessage}. Ensure JIRA_API_TOKEN, JIRA_EMAIL, and JIRA_BASE_URL are configured in settings or environment.` + ) } else { logger.error( 'Linear issue management requires LINEAR_API_TOKEN to be configured.' @@ -321,6 +328,13 @@ export class PlanCommand { throw new Error( `Cannot start planning session: ${message}. Ensure you are in a git repository with a GitHub remote configured.` ) + } else if (provider === 'jira') { + logger.error( + 'Jira issue management requires JIRA_API_TOKEN, JIRA_EMAIL, and JIRA_BASE_URL to be configured.' + ) + throw new Error( + `Cannot start planning session: ${message}. Ensure JIRA_API_TOKEN, JIRA_EMAIL, and JIRA_BASE_URL are configured in settings or environment.` + ) } else { logger.error( 'Linear issue management requires LINEAR_API_TOKEN to be configured.' @@ -340,6 +354,13 @@ export class PlanCommand { throw new Error( `Cannot start planning session: ${message}. Ensure you are in a git repository with a GitHub remote configured.` ) + } else if (provider === 'jira') { + logger.error( + 'Jira issue management requires JIRA_API_TOKEN, JIRA_EMAIL, and JIRA_BASE_URL to be configured.' + ) + throw new Error( + `Cannot start planning session: ${message}. Ensure JIRA_API_TOKEN, JIRA_EMAIL, and JIRA_BASE_URL are configured in settings or environment.` + ) } else { logger.error( 'Linear issue management requires LINEAR_API_TOKEN to be configured.' diff --git a/src/commands/start.test.ts b/src/commands/start.test.ts index bf987389..2133038e 100644 --- a/src/commands/start.test.ts +++ b/src/commands/start.test.ts @@ -101,6 +101,18 @@ vi.mock('../lib/TelemetryService.js', () => ({ }, })) +// Mock VCSProviderFactory - default returns null (no non-GitHub VCS provider) +const { mockVCSProviderCreate } = vi.hoisted(() => ({ + mockVCSProviderCreate: vi.fn().mockReturnValue(null), +})) +vi.mock('../lib/VCSProviderFactory.js', () => ({ + VCSProviderFactory: { + create: mockVCSProviderCreate, + isConfigured: vi.fn().mockReturnValue(false), + getProviderName: vi.fn().mockReturnValue(undefined), + }, +})) + // Mock IssueTrackerFactory for epic child data fetching vi.mock('../lib/IssueTrackerFactory.js', () => ({ IssueTrackerFactory: { @@ -2233,3 +2245,146 @@ describe('StartCommand', () => { }) }) }) + +describe('Jira+BitBucket VCS provider PR detection', () => { + let mockJiraService: { + supportsPullRequests: boolean + providerName: string + detectInputType: ReturnType + fetchIssue: ReturnType + validateIssueState: ReturnType + } + let mockBitBucketProvider: { + providerName: string + fetchPR: ReturnType + checkForExistingPR: ReturnType + createPR: ReturnType + createPRComment: ReturnType + detectRepository: ReturnType + getTargetRemote: ReturnType + getPRUrl: ReturnType + supportsForks: boolean + supportsDraftPRs: boolean + } + let jiraBBCommand: StartCommand + + const mockBBPR = { + number: 42, + title: 'BitBucket PR', + body: '', + state: 'open' as const, + branch: 'feature-branch', + baseBranch: 'main', + url: 'https://bitbucket.org/workspace/repo/pull-requests/42', + isDraft: false, + } + + beforeEach(() => { + // Jira doesn't support PRs + mockJiraService = { + supportsPullRequests: false, + providerName: 'jira', + detectInputType: vi.fn(), + fetchIssue: vi.fn(), + validateIssueState: vi.fn(), + } + + // BitBucket VCS provider + mockBitBucketProvider = { + providerName: 'bitbucket', + supportsForks: false, + supportsDraftPRs: false, + fetchPR: vi.fn().mockResolvedValue(mockBBPR), + checkForExistingPR: vi.fn(), + createPR: vi.fn(), + createPRComment: vi.fn(), + detectRepository: vi.fn(), + getTargetRemote: vi.fn(), + getPRUrl: vi.fn(), + } + + // Configure VCSProviderFactory to return the BitBucket provider + mockVCSProviderCreate.mockReturnValue(mockBitBucketProvider) + + // Default: no child issues + vi.mocked(fetchChildIssues).mockResolvedValue([]) + + jiraBBCommand = new StartCommand(mockJiraService as unknown as GitHubService) + }) + + afterEach(() => { + // Reset VCSProviderFactory mock to default (null) for other tests + mockVCSProviderCreate.mockReturnValue(null) + }) + + describe('PR detection via numeric input', () => { + it('should detect BitBucket PR from numeric input when Jira+BitBucket is configured', async () => { + await expect( + jiraBBCommand.execute({ + identifier: '42', + options: {}, + }) + ).resolves.not.toThrow() + + // BitBucket fetchPR should be called for detection + expect(mockBitBucketProvider.fetchPR).toHaveBeenCalledWith(42) + // GitHubService should NOT be called for detection + const MockedGitHubService = vi.mocked(GitHubService) + expect(MockedGitHubService.prototype.detectInputType).not.toHaveBeenCalled() + }) + + it('should fall back to issue tracker when BitBucket PR not found', async () => { + // BitBucket throws when PR doesn't exist + mockBitBucketProvider.fetchPR.mockRejectedValue(new Error('PR not found')) + mockJiraService.fetchIssue.mockResolvedValue({ + number: 42, + title: 'Jira Issue', + body: '', + state: 'open', + labels: [], + assignees: [], + url: 'https://jira.example.com/browse/PROJ-42', + }) + mockJiraService.validateIssueState.mockResolvedValue(undefined) + + await expect( + jiraBBCommand.execute({ + identifier: '42', + options: {}, + }) + ).resolves.not.toThrow() + + // Should fall back to Jira issue + expect(mockJiraService.fetchIssue).toHaveBeenCalledWith(42, undefined) + }) + }) + + describe('PR validation with BitBucket', () => { + it('should validate PR using BitBucket VCS provider for explicit pr/ format', async () => { + await expect( + jiraBBCommand.execute({ + identifier: 'pr/42', + options: {}, + }) + ).resolves.not.toThrow() + + // BitBucket fetchPR should be called for validation + expect(mockBitBucketProvider.fetchPR).toHaveBeenCalledWith(42) + // GitHubService should NOT be used for PR operations + const MockedGitHubService = vi.mocked(GitHubService) + expect(MockedGitHubService.prototype.fetchPR).not.toHaveBeenCalled() + expect(MockedGitHubService.prototype.validatePRState).not.toHaveBeenCalled() + }) + + it('should throw when BitBucket PR validation fails', async () => { + mockBitBucketProvider.fetchPR.mockRejectedValue(new Error('PR #99 not found in BitBucket')) + + await expect( + jiraBBCommand.execute({ + identifier: 'pr/99', + options: {}, + }) + ).rejects.toThrow('PR #99 not found in BitBucket') + }) + }) +}) diff --git a/src/commands/start.ts b/src/commands/start.ts index b95c4a81..67e2c0af 100644 --- a/src/commands/start.ts +++ b/src/commands/start.ts @@ -27,6 +27,8 @@ import { launchFirstRunSetup, needsFirstRunSetup } from '../utils/first-run-setu import { isInteractiveEnvironment, promptConfirmation } from '../utils/prompt.js' import { TelemetryService } from '../lib/TelemetryService.js' import type { LoomCreatedProperties } from '../types/telemetry.js' +import { VCSProviderFactory } from '../lib/VCSProviderFactory.js' +import type { VersionControlProvider } from '../lib/VersionControlProvider.js' export interface StartCommandInput { identifier: string @@ -46,6 +48,7 @@ export class StartCommand { private settingsManager: SettingsManager private providedLoomManager: LoomManager | undefined private githubService: GitHubService | null = null + private vcsProvider: VersionControlProvider | null | undefined = undefined constructor( issueTracker: IssueTracker, @@ -77,6 +80,20 @@ export class StartCommand { return this.githubService } + /** + * Get the configured VCS provider, if any. + * Returns null if no non-GitHub VCS provider is configured. + * Uses cached value after first load. + */ + private async getVCSProvider(): Promise { + if (this.vcsProvider !== undefined) { + return this.vcsProvider + } + const settings = await this.settingsManager.loadSettings() + this.vcsProvider = VCSProviderFactory.create(settings) + return this.vcsProvider + } + /** * Initialize LoomManager with the main worktree path * Uses lazy initialization to ensure we have the correct path @@ -508,23 +525,44 @@ export class StartCommand { } } else { // Issue tracker doesn't support PRs (e.g., Linear, Jira) - // Check GitHub first for PR, then fall back to issue tracker for issues - const githubService = this.getGitHubService() - const detection = await githubService.detectInputType(trimmedIdentifier, repo) - - if (detection.type === 'pr') { - return { - type: 'pr', - number: detection.identifier ? parseInt(detection.identifier, 10) : number, - originalInput: trimmedIdentifier, + // Check VCS provider first for PR, then fall back to issue tracker for issues + const vcsProvider = await this.getVCSProvider() + if (vcsProvider) { + // Non-GitHub VCS provider configured (e.g., BitBucket): try fetching as PR + try { + await vcsProvider.fetchPR(number) + return { + type: 'pr', + number, + originalInput: trimmedIdentifier, + } + } catch { + // Not a VCS PR - treat as an issue + return { + type: 'issue', + number, + originalInput: trimmedIdentifier, + } } } else { - // Not a GitHub PR - try the configured issue tracker - // This allows future trackers with numeric IDs to work naturally - return { - type: 'issue', - number, - originalInput: trimmedIdentifier, + // No non-GitHub VCS provider: fall back to GitHubService + const githubService = this.getGitHubService() + const detection = await githubService.detectInputType(trimmedIdentifier, repo) + + if (detection.type === 'pr') { + return { + type: 'pr', + number: detection.identifier ? parseInt(detection.identifier, 10) : number, + originalInput: trimmedIdentifier, + } + } else { + // Not a GitHub PR - try the configured issue tracker + // This allows future trackers with numeric IDs to work naturally + return { + type: 'issue', + number, + originalInput: trimmedIdentifier, + } } } } @@ -554,10 +592,17 @@ export class StartCommand { const pr = await this.issueTracker.fetchPR(parsed.number, repo) await this.issueTracker.validatePRState(pr) } else { - // Use GitHubService for PR operations when issue tracker doesn't support PRs (e.g., Linear) - const githubService = this.getGitHubService() - const pr = await githubService.fetchPR(parsed.number as number, repo) - await githubService.validatePRState(pr) + // Use VCS provider if configured (e.g., BitBucket), otherwise fall back to GitHubService + const vcsProvider = await this.getVCSProvider() + if (vcsProvider) { + // VCS provider configured: fetch PR to validate it exists (providers throw on invalid PRs) + await vcsProvider.fetchPR(parsed.number as number) + } else { + // No non-GitHub VCS provider: fall back to GitHubService + const githubService = this.getGitHubService() + const pr = await githubService.fetchPR(parsed.number as number, repo) + await githubService.validatePRState(pr) + } } getLogger().debug(`Validated PR #${parsed.number}`) break diff --git a/src/commands/summary.ts b/src/commands/summary.ts index 7f4f5913..867d25ae 100644 --- a/src/commands/summary.ts +++ b/src/commands/summary.ts @@ -72,6 +72,11 @@ export class SummaryCommand { return existingPR?.number } + if (mergeMode === 'bitbucket-pr') { + const metadata = await this.metadataManager.readMetadata(worktreePath) + return metadata?.draftPrNumber ?? undefined + } + return undefined // local mode - post to issue } diff --git a/src/lib/LoomManager.ts b/src/lib/LoomManager.ts index fd2e23c0..e1251845 100644 --- a/src/lib/LoomManager.ts +++ b/src/lib/LoomManager.ts @@ -13,6 +13,8 @@ import { SettingsManager } from './SettingsManager.js' import { MetadataManager, type WriteMetadataInput } from './MetadataManager.js' import { branchExists, executeGitCommand, ensureRepositoryHasCommits, extractIssueNumber, isFileTrackedByGit, extractPRNumber, PLACEHOLDER_COMMIT_PREFIX, pushBranchToRemote, GitCommandError, fetchOrigin } from '../utils/git.js' import { GitHubService } from './GitHubService.js' +import { VCSProviderFactory } from './VCSProviderFactory.js' +import type { VersionControlProvider } from './VersionControlProvider.js' import { generateRandomSessionId } from '../utils/claude.js' import { installDependencies } from '../utils/package-manager.js' import { generateColorFromBranchName, selectDistinctColor, hexToRgb, type ColorData } from '../utils/color.js' @@ -32,6 +34,7 @@ import { PRManager } from './PRManager.js' export class LoomManager { private metadataManager: MetadataManager private githubService: GitHubService | undefined + private vcsProvider: VersionControlProvider | null | undefined = undefined constructor( private gitWorktree: GitWorktreeManager, @@ -43,10 +46,29 @@ export class LoomManager { private cliIsolation: CLIIsolationManager, private settings: SettingsManager, private database?: DatabaseManager, - githubService?: GitHubService + githubService?: GitHubService, + vcsProvider?: VersionControlProvider | null ) { this.metadataManager = new MetadataManager() this.githubService = githubService + // If explicitly provided (including null), use it; otherwise leave as undefined for lazy init + if (vcsProvider !== undefined) { + this.vcsProvider = vcsProvider + } + } + + /** + * Get the configured VCS provider, if any. + * Returns null if no non-GitHub VCS provider is configured. + * Uses cached value after first load. + */ + private async getVCSProvider(): Promise { + if (this.vcsProvider !== undefined) { + return this.vcsProvider + } + const iloomSettings = await this.settings.loadSettings() + this.vcsProvider = VCSProviderFactory.create(iloomSettings) + return this.vcsProvider } /** @@ -441,6 +463,7 @@ export class LoomManager { issue_numbers, pr_numbers, issueTracker: this.issueTracker.providerName, + vcsProvider: settingsData.versionControl?.provider ?? 'github', colorHex: colorData.hex, sessionId, projectPath: this.gitWorktree.workingDirectory, @@ -611,7 +634,12 @@ export class LoomManager { if (this.issueTracker.supportsPullRequests && this.issueTracker.fetchPR) { return await this.issueTracker.fetchPR(input.identifier as number) } - // Use injected GitHubService if available + // Use VCS provider if configured (e.g., BitBucket) + const vcsProvider = await this.getVCSProvider() + if (vcsProvider) { + return await vcsProvider.fetchPR(input.identifier as number) + } + // Fall back to GitHubService for GitHub or unconfigured setups if (this.githubService) { return await this.githubService.fetchPR(input.identifier as number) } @@ -1413,6 +1441,7 @@ export class LoomManager { issue_numbers, pr_numbers, issueTracker: this.issueTracker.providerName, + vcsProvider: settingsData.versionControl?.provider ?? 'github', colorHex, sessionId, projectPath: this.gitWorktree.workingDirectory, diff --git a/src/lib/MetadataManager.test.ts b/src/lib/MetadataManager.test.ts index cc6b4df1..b81ff544 100644 --- a/src/lib/MetadataManager.test.ts +++ b/src/lib/MetadataManager.test.ts @@ -179,6 +179,40 @@ describe('MetadataManager', () => { expect(writtenContent.issueTracker).toBe('github') }) + it('should write vcsProvider field when provided', async () => { + const inputWithVcsProvider = { + ...metadataInput, + vcsProvider: 'bitbucket', + } + + await manager.writeMetadata(worktreePath, inputWithVcsProvider) + + const writeCall = vi.mocked(fs.writeFile).mock.calls[0] + const writtenContent = JSON.parse(writeCall?.[1] as string) + expect(writtenContent.vcsProvider).toBe('bitbucket') + }) + + it('should write github as vcsProvider when specified', async () => { + const inputWithGithub = { + ...metadataInput, + vcsProvider: 'github', + } + + await manager.writeMetadata(worktreePath, inputWithGithub) + + const writeCall = vi.mocked(fs.writeFile).mock.calls[0] + const writtenContent = JSON.parse(writeCall?.[1] as string) + expect(writtenContent.vcsProvider).toBe('github') + }) + + it('should not include vcsProvider field when not provided', async () => { + await manager.writeMetadata(worktreePath, metadataInput) + + const writeCall = vi.mocked(fs.writeFile).mock.calls[0] + const writtenContent = JSON.parse(writeCall?.[1] as string) + expect(writtenContent.vcsProvider).toBeUndefined() + }) + it('should write sessionId to JSON file', async () => { await manager.writeMetadata(worktreePath, metadataInput) @@ -366,6 +400,7 @@ describe('MetadataManager', () => { issue_numbers: ['42'], pr_numbers: [], issueTracker: 'github', + vcsProvider: null, colorHex: '#f5dceb', sessionId: '6ba7b810-9dad-11d1-80b4-00c04fd430c8', projectPath: '/Users/jane/dev/main-repo', @@ -497,6 +532,7 @@ describe('MetadataManager', () => { issue_numbers: [], pr_numbers: [], issueTracker: null, + vcsProvider: null, colorHex: null, sessionId: null, projectPath: null, @@ -785,6 +821,72 @@ describe('MetadataManager', () => { expect(result?.state).toBeNull() }) + + it('should return vcsProvider when present in metadata file', async () => { + const mockContent = JSON.stringify({ + description: 'Loom with BitBucket VCS', + created_at: '2024-01-15T10:30:00.000Z', + version: 1, + branchName: 'issue-42__feature', + worktreePath: '/Users/jane/dev/repo', + issueType: 'issue', + issue_numbers: ['42'], + pr_numbers: [], + issueTracker: 'github', + colorHex: '#f5dceb', + vcsProvider: 'bitbucket', + }) + vi.mocked(fs.pathExists).mockResolvedValue(true) + vi.mocked(fs.readFile).mockResolvedValue(mockContent) + + const result = await manager.readMetadata(worktreePath) + + expect(result?.vcsProvider).toBe('bitbucket') + }) + + it('should return github vcsProvider when stored as github', async () => { + const mockContent = JSON.stringify({ + description: 'Loom with GitHub VCS', + created_at: '2024-01-15T10:30:00.000Z', + version: 1, + branchName: 'issue-42__feature', + worktreePath: '/Users/jane/dev/repo', + issueType: 'issue', + issue_numbers: ['42'], + pr_numbers: [], + issueTracker: 'github', + colorHex: '#f5dceb', + vcsProvider: 'github', + }) + vi.mocked(fs.pathExists).mockResolvedValue(true) + vi.mocked(fs.readFile).mockResolvedValue(mockContent) + + const result = await manager.readMetadata(worktreePath) + + expect(result?.vcsProvider).toBe('github') + }) + + it('should return null vcsProvider for legacy looms without vcsProvider field', async () => { + const mockContent = JSON.stringify({ + description: 'Legacy loom without vcsProvider', + created_at: '2024-01-15T10:30:00.000Z', + version: 1, + branchName: 'issue-42__legacy', + worktreePath: '/Users/jane/dev/repo', + issueType: 'issue', + issue_numbers: ['42'], + pr_numbers: [], + issueTracker: 'github', + colorHex: '#f5dceb', + // Note: no vcsProvider field (backward compatibility) + }) + vi.mocked(fs.pathExists).mockResolvedValue(true) + vi.mocked(fs.readFile).mockResolvedValue(mockContent) + + const result = await manager.readMetadata(worktreePath) + + expect(result?.vcsProvider).toBeNull() + }) }) describe('listAllMetadata', () => { @@ -866,6 +968,7 @@ describe('MetadataManager', () => { issue_numbers: ['1'], pr_numbers: [], issueTracker: 'github', + vcsProvider: null, colorHex: '#ff0000', sessionId: '11111111-1111-1111-1111-111111111111', projectPath: '/Users/alice/main-project', @@ -891,6 +994,7 @@ describe('MetadataManager', () => { issue_numbers: ['2'], pr_numbers: [], issueTracker: 'github', + vcsProvider: null, colorHex: '#00ff00', sessionId: '22222222-2222-2222-2222-222222222222', projectPath: '/Users/bob/main-project', @@ -1000,6 +1104,7 @@ describe('MetadataManager', () => { issue_numbers: [], pr_numbers: [], issueTracker: null, + vcsProvider: null, colorHex: null, sessionId: null, projectPath: null, diff --git a/src/lib/MetadataManager.ts b/src/lib/MetadataManager.ts index 61ec05b3..e07ebcd7 100644 --- a/src/lib/MetadataManager.ts +++ b/src/lib/MetadataManager.ts @@ -23,6 +23,7 @@ export interface MetadataFile { issue_numbers?: string[] pr_numbers?: string[] issueTracker?: string + vcsProvider?: string // VCS provider at loom creation time ('github' | 'bitbucket') colorHex?: string // Stored hex color (e.g., "#dcebff") - robust against palette changes sessionId?: string // Claude Code session ID for resume support projectPath?: string // Main worktree path (project root) - enables project identification @@ -65,6 +66,7 @@ export interface WriteMetadataInput { issue_numbers: string[] pr_numbers: string[] issueTracker: string + vcsProvider?: string // VCS provider at loom creation time ('github' | 'bitbucket') colorHex: string // Hex color (e.g., "#dcebff") - robust against palette changes sessionId: string // Claude Code session ID for resume support (required for new looms) projectPath: string // Main worktree path (project root) - required for new looms @@ -108,6 +110,7 @@ export interface LoomMetadata { issue_numbers: string[] pr_numbers: string[] issueTracker: string | null + vcsProvider: string | null // VCS provider at loom creation time (null for legacy looms) colorHex: string | null // Hex color (e.g., "#dcebff") - robust against palette changes sessionId: string | null // Claude Code session ID (null for legacy looms) projectPath: string | null // Main worktree path (null for legacy looms) @@ -170,6 +173,7 @@ export class MetadataManager { issue_numbers: data.issue_numbers ?? [], pr_numbers: data.pr_numbers ?? [], issueTracker: data.issueTracker ?? null, + vcsProvider: data.vcsProvider ?? null, colorHex: data.colorHex ?? null, sessionId: data.sessionId ?? null, projectPath: data.projectPath ?? null, @@ -254,6 +258,7 @@ export class MetadataManager { issue_numbers: input.issue_numbers, pr_numbers: input.pr_numbers, issueTracker: input.issueTracker, + ...(input.vcsProvider && { vcsProvider: input.vcsProvider }), colorHex: input.colorHex, sessionId: input.sessionId, projectPath: input.projectPath, diff --git a/src/lib/PRManager.ts b/src/lib/PRManager.ts index 29ad063a..9b6acb28 100644 --- a/src/lib/PRManager.ts +++ b/src/lib/PRManager.ts @@ -132,7 +132,7 @@ ${issueContext} -IMPORTANT: Your entire response will be used directly as the GitHub pull request body. +IMPORTANT: Your entire response will be used directly as the pull request body. Do not include any explanatory text, headers, or separators before or after the body. Start your response immediately with the PR body text. ` diff --git a/src/lib/PromptTemplateManager.ts b/src/lib/PromptTemplateManager.ts index 07500681..de7d3841 100644 --- a/src/lib/PromptTemplateManager.ts +++ b/src/lib/PromptTemplateManager.ts @@ -104,6 +104,8 @@ export interface TemplateVariables { HAS_REVIEWER?: boolean // Git remote configuration GIT_REMOTE?: string // Remote name for push (defaults to 'origin') + // VCS provider configuration + IS_BITBUCKET?: boolean // True when versionControl.provider is 'bitbucket' // Swarm orchestrator variables EPIC_ISSUE_NUMBER?: string | number EPIC_WORKTREE_PATH?: string diff --git a/src/lib/SessionSummaryService.ts b/src/lib/SessionSummaryService.ts index 9af41874..99b6ee33 100644 --- a/src/lib/SessionSummaryService.ts +++ b/src/lib/SessionSummaryService.ts @@ -19,6 +19,7 @@ import { MetadataManager } from './MetadataManager.js' import { SettingsManager, type IloomSettings } from './SettingsManager.js' import { IssueManagementProviderFactory } from '../mcp/IssueManagementProviderFactory.js' import type { IssueProvider } from '../mcp/types.js' +import { VCSProviderFactory } from './VCSProviderFactory.js' import { hasMultipleRemotes } from '../utils/remote.js' import type { RecapFile, RecapOutput } from '../mcp/recap-types.js' import { formatRecapMarkdown } from '../utils/recap-formatter.js' @@ -402,26 +403,36 @@ export class SessionSummaryService { worktreePath: string, prNumber?: number ): Promise { - // Get the issue management provider from settings - // PRs only exist on GitHub, so always use 'github' provider when prNumber is provided - // (see types.ts:32-33 and LinearIssueManagementProvider.getPR()) - const providerType = prNumber !== undefined - ? 'github' - : (settings.issueManagement?.provider ?? 'github') as IssueProvider - const provider = IssueManagementProviderFactory.create(providerType, settings) - // Apply attribution if configured const finalSummary = await this.applyAttributionWithSettings(summary, settings, worktreePath) - // When prNumber is provided, post to the PR instead of the issue - const targetNumber = prNumber ?? issueNumber - const targetType = prNumber !== undefined ? 'pr' : 'issue' + // When prNumber is provided, route through VCS provider if configured (e.g. BitBucket), + // otherwise fall back to the GitHub issue management provider for PR comments + if (prNumber !== undefined) { + const vcsProvider = VCSProviderFactory.create(settings) + if (vcsProvider) { + // Use the VCS provider (e.g. BitBucket) to post the PR comment + await vcsProvider.createPRComment(prNumber, finalSummary, worktreePath) + return + } + + // No non-GitHub VCS provider configured - use GitHub issue management provider + const provider = IssueManagementProviderFactory.create('github' as IssueProvider, settings) + await provider.createComment({ + number: String(prNumber), + body: finalSummary, + type: 'pr', + }) + return + } - // Create the comment + // Post to issue using the configured issue management provider + const providerType = (settings.issueManagement?.provider ?? 'github') as IssueProvider + const provider = IssueManagementProviderFactory.create(providerType, settings) await provider.createComment({ - number: String(targetNumber), + number: String(issueNumber), body: finalSummary, - type: targetType, + type: 'issue', }) } } diff --git a/src/mcp/issue-management-pr-routing.test.ts b/src/mcp/issue-management-pr-routing.test.ts new file mode 100644 index 00000000..79476bae --- /dev/null +++ b/src/mcp/issue-management-pr-routing.test.ts @@ -0,0 +1,432 @@ +/** + * Tests for MCP PR routing through VCS provider abstraction + * Covers get_pr, create_comment (type: pr), and update_comment (type: pr) + * routing through BitBucket VCS provider when configured + */ + +import { describe, it, expect, vi } from 'vitest' +import type { PullRequest } from '../types/index.js' + +/** + * These tests verify the routing logic used by the MCP server + * for PR operations when a VCS provider (BitBucket) is configured. + * + * The MCP server logic is: + * - get_pr: use bitBucketVCSProvider.fetchPR() if configured, else GitHubIssueManagementProvider.getPR() + * - create_comment type:pr: use bitBucketVCSProvider.createPRComment() if configured, else GitHub + * - update_comment type:pr: throw unsupported error if BitBucket configured, else GitHub + */ + +// ─── Helper types ────────────────────────────────────────────────────────────── + +interface MockBitBucketVCSProvider { + fetchPR: ReturnType + createPRComment: ReturnType +} + +interface PRResult { + id: string + number: number + title: string + body: string + state: string + url: string + author: null + headRefName: string + baseRefName: string +} + +interface CommentResult { + id: string + url: string + updated_at?: string +} + +// ─── Routing logic extracted from MCP server ─────────────────────────────────── + +/** + * Simulate the get_pr routing logic from issue-management-server.ts + */ +async function simulateGetPR( + number: string, + bitBucketVCSProvider: MockBitBucketVCSProvider | undefined, + githubGetPR: (input: { number: string; includeComments?: boolean; repo?: string }) => Promise, + options?: { includeComments?: boolean; repo?: string } +): Promise { + if (bitBucketVCSProvider) { + const prNumber = parseInt(number, 10) + if (isNaN(prNumber)) { + throw new Error(`Invalid PR number: ${number}. PR IDs must be numeric.`) + } + const bbPR = await bitBucketVCSProvider.fetchPR(prNumber) as PullRequest + return { + id: String(bbPR.number), + number: bbPR.number, + title: bbPR.title, + body: bbPR.body, + state: bbPR.state.toUpperCase(), + url: bbPR.url, + author: null, + headRefName: bbPR.branch, + baseRefName: bbPR.baseBranch, + } + } + return githubGetPR({ number, ...options }) +} + +/** + * Simulate the create_comment routing logic from issue-management-server.ts + */ +async function simulateCreateComment( + number: string, + body: string, + type: 'issue' | 'pr', + bitBucketVCSProvider: MockBitBucketVCSProvider | undefined, + githubCreateComment: (input: { number: string; body: string; type: 'issue' | 'pr' }) => Promise +): Promise { + if (type === 'pr' && bitBucketVCSProvider) { + const prNumber = parseInt(number, 10) + if (isNaN(prNumber)) { + throw new Error(`Invalid PR number: ${number}. PR IDs must be numeric.`) + } + await bitBucketVCSProvider.createPRComment(prNumber, body) + return { + id: `bitbucket-pr-${prNumber}-comment`, + url: '', + } + } + return githubCreateComment({ number, body, type }) +} + +/** + * Simulate the update_comment routing logic from issue-management-server.ts + */ +async function simulateUpdateComment( + commentId: string, + number: string, + body: string, + type: 'issue' | 'pr' | undefined, + bitBucketVCSProvider: MockBitBucketVCSProvider | undefined, + githubUpdateComment: (input: { commentId: string; number: string; body: string }) => Promise +): Promise { + if (type === 'pr' && bitBucketVCSProvider) { + throw new Error( + 'BitBucket does not support editing PR comments. ' + + 'The BitBucket REST API does not provide a PUT/PATCH endpoint for pull request comments.' + ) + } + return githubUpdateComment({ commentId, number, body }) +} + +// ─── Tests ───────────────────────────────────────────────────────────────────── + +describe('MCP PR routing via VCS provider abstraction', () => { + describe('get_pr routing', () => { + it('routes to BitBucket fetchPR when bitBucketVCSProvider is configured', async () => { + const mockBBPR: PullRequest = { + number: 42, + title: 'Add feature X', + body: 'This PR adds feature X', + state: 'open', + branch: 'feature/add-x', + baseBranch: 'main', + url: 'https://bitbucket.org/workspace/repo/pull-requests/42', + isDraft: false, + } + + const mockBitBucketProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn().mockResolvedValueOnce(mockBBPR), + createPRComment: vi.fn(), + } + const githubGetPR = vi.fn() + + const result = await simulateGetPR('42', mockBitBucketProvider, githubGetPR) + + expect(mockBitBucketProvider.fetchPR).toHaveBeenCalledWith(42) + expect(githubGetPR).not.toHaveBeenCalled() + expect(result.id).toBe('42') + expect(result.number).toBe(42) + expect(result.title).toBe('Add feature X') + expect(result.body).toBe('This PR adds feature X') + expect(result.state).toBe('OPEN') + expect(result.url).toBe('https://bitbucket.org/workspace/repo/pull-requests/42') + expect(result.author).toBeNull() + expect(result.headRefName).toBe('feature/add-x') + expect(result.baseRefName).toBe('main') + }) + + it('maps BitBucket PR states correctly to uppercase MCP states', async () => { + const stateTestCases: Array<{ inputState: 'open' | 'closed' | 'merged'; expectedMcpState: string }> = [ + { inputState: 'open', expectedMcpState: 'OPEN' }, + { inputState: 'closed', expectedMcpState: 'CLOSED' }, + { inputState: 'merged', expectedMcpState: 'MERGED' }, + ] + + for (const { inputState, expectedMcpState } of stateTestCases) { + const mockPR: PullRequest = { + number: 1, + title: 'Test PR', + body: 'Body', + state: inputState, + branch: 'feat', + baseBranch: 'main', + url: 'https://bitbucket.org/w/r/pull-requests/1', + isDraft: false, + } + + const mockProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn().mockResolvedValueOnce(mockPR), + createPRComment: vi.fn(), + } + + const result = await simulateGetPR('1', mockProvider, vi.fn()) + expect(result.state).toBe(expectedMcpState) + } + }) + + it('falls back to GitHub getPR when bitBucketVCSProvider is undefined', async () => { + const mockGHResult: PRResult = { + id: '100', + number: 100, + title: 'GitHub PR', + body: 'GitHub PR body', + state: 'OPEN', + url: 'https://github.com/owner/repo/pull/100', + author: null, + headRefName: 'feature', + baseRefName: 'main', + } + + const githubGetPR = vi.fn().mockResolvedValueOnce(mockGHResult) + + const result = await simulateGetPR('100', undefined, githubGetPR, { includeComments: false }) + + expect(githubGetPR).toHaveBeenCalledWith({ number: '100', includeComments: false }) + expect(result.id).toBe('100') + expect(result.title).toBe('GitHub PR') + }) + + it('throws for invalid (non-numeric) PR number when BitBucket configured', async () => { + const mockProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn(), + createPRComment: vi.fn(), + } + + await expect( + simulateGetPR('not-a-number', mockProvider, vi.fn()) + ).rejects.toThrow('Invalid PR number: not-a-number. PR IDs must be numeric.') + + expect(mockProvider.fetchPR).not.toHaveBeenCalled() + }) + }) + + describe('create_comment type:pr routing', () => { + it('routes to BitBucket createPRComment when type is pr and provider is configured', async () => { + const mockProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn(), + createPRComment: vi.fn().mockResolvedValueOnce(undefined), + } + const githubCreateComment = vi.fn() + + const result = await simulateCreateComment( + '42', + 'Test PR comment', + 'pr', + mockProvider, + githubCreateComment + ) + + expect(mockProvider.createPRComment).toHaveBeenCalledWith(42, 'Test PR comment') + expect(githubCreateComment).not.toHaveBeenCalled() + expect(result.id).toBe('bitbucket-pr-42-comment') + expect(result.url).toBe('') + }) + + it('routes to GitHub for issue comments even when BitBucket VCS provider is configured', async () => { + const mockProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn(), + createPRComment: vi.fn(), + } + const mockGHResult: CommentResult = { + id: '9999', + url: 'https://github.com/owner/repo/issues/5#issuecomment-9999', + } + const githubCreateComment = vi.fn().mockResolvedValueOnce(mockGHResult) + + const result = await simulateCreateComment( + '5', + 'Issue comment', + 'issue', + mockProvider, + githubCreateComment + ) + + expect(mockProvider.createPRComment).not.toHaveBeenCalled() + expect(githubCreateComment).toHaveBeenCalledWith({ number: '5', body: 'Issue comment', type: 'issue' }) + expect(result.id).toBe('9999') + }) + + it('falls back to GitHub createComment for type:pr when BitBucket not configured', async () => { + const mockGHResult: CommentResult = { + id: '54321', + url: 'https://github.com/owner/repo/pull/100#issuecomment-54321', + } + const githubCreateComment = vi.fn().mockResolvedValueOnce(mockGHResult) + + const result = await simulateCreateComment( + '100', + 'PR comment via GitHub fallback', + 'pr', + undefined, + githubCreateComment + ) + + expect(githubCreateComment).toHaveBeenCalledWith({ + number: '100', + body: 'PR comment via GitHub fallback', + type: 'pr', + }) + expect(result.id).toBe('54321') + }) + + it('throws for invalid (non-numeric) PR number when BitBucket configured', async () => { + const mockProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn(), + createPRComment: vi.fn(), + } + + await expect( + simulateCreateComment('not-a-number', 'body', 'pr', mockProvider, vi.fn()) + ).rejects.toThrow('Invalid PR number: not-a-number. PR IDs must be numeric.') + + expect(mockProvider.createPRComment).not.toHaveBeenCalled() + }) + }) + + describe('update_comment type:pr routing', () => { + it('throws unsupported error when type is pr and BitBucket provider is configured', async () => { + const mockProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn(), + createPRComment: vi.fn(), + } + const githubUpdateComment = vi.fn() + + await expect( + simulateUpdateComment('12345', '42', 'Updated body', 'pr', mockProvider, githubUpdateComment) + ).rejects.toThrow('BitBucket does not support editing PR comments') + + expect(githubUpdateComment).not.toHaveBeenCalled() + }) + + it('routes to GitHub for update_comment type:issue even when BitBucket configured', async () => { + const mockProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn(), + createPRComment: vi.fn(), + } + const mockGHResult: CommentResult = { + id: '12345', + url: 'https://github.com/owner/repo/issues/5#issuecomment-12345', + updated_at: '2025-01-01T00:00:00Z', + } + const githubUpdateComment = vi.fn().mockResolvedValueOnce(mockGHResult) + + const result = await simulateUpdateComment( + '12345', + '5', + 'Updated issue comment', + 'issue', + mockProvider, + githubUpdateComment + ) + + expect(githubUpdateComment).toHaveBeenCalledWith({ + commentId: '12345', + number: '5', + body: 'Updated issue comment', + }) + expect(result.id).toBe('12345') + }) + + it('routes to GitHub for update_comment when type is undefined and BitBucket configured', async () => { + const mockProvider: MockBitBucketVCSProvider = { + fetchPR: vi.fn(), + createPRComment: vi.fn(), + } + const mockGHResult: CommentResult = { + id: '99999', + url: 'https://github.com/owner/repo/issues/10#issuecomment-99999', + } + const githubUpdateComment = vi.fn().mockResolvedValueOnce(mockGHResult) + + const result = await simulateUpdateComment( + '99999', + '10', + 'Updated comment', + undefined, + mockProvider, + githubUpdateComment + ) + + expect(githubUpdateComment).toHaveBeenCalled() + expect(result.id).toBe('99999') + }) + + it('routes to GitHub for update_comment type:pr when BitBucket not configured', async () => { + const mockGHResult: CommentResult = { + id: '77777', + url: 'https://github.com/owner/repo/pull/100#issuecomment-77777', + updated_at: '2025-01-15T12:00:00Z', + } + const githubUpdateComment = vi.fn().mockResolvedValueOnce(mockGHResult) + + const result = await simulateUpdateComment( + '77777', + '100', + 'Updated PR comment via GitHub', + 'pr', + undefined, + githubUpdateComment + ) + + expect(githubUpdateComment).toHaveBeenCalledWith({ + commentId: '77777', + number: '100', + body: 'Updated PR comment via GitHub', + }) + expect(result.id).toBe('77777') + expect(result.updated_at).toBe('2025-01-15T12:00:00Z') + }) + }) + + describe('Security: auth credentials not exposed in PR operation call signatures', () => { + it('fetchPR call uses only PR number, not auth credentials', () => { + const fetchPR = vi.fn() + const prNumber = 42 + + // The BitBucket API client stores credentials internally + // and is accessed only via the client's request() method + // fetchPR(prNumber) does NOT include credentials in its parameters + fetchPR(prNumber) + + expect(fetchPR).toHaveBeenCalledWith(42) + const callArgs = fetchPR.mock.calls[0] + // Ensure no sensitive strings in the call arguments + expect(JSON.stringify(callArgs)).not.toContain('token') + expect(JSON.stringify(callArgs)).not.toContain('password') + expect(JSON.stringify(callArgs)).not.toContain('Authorization') + expect(JSON.stringify(callArgs)).not.toContain('Basic ') + }) + + it('createPRComment call uses only PR number and body, not auth credentials', () => { + const createPRComment = vi.fn() + + createPRComment(42, 'This is a comment body') + + expect(createPRComment).toHaveBeenCalledWith(42, 'This is a comment body') + const callArgs = createPRComment.mock.calls[0] + expect(JSON.stringify(callArgs)).not.toContain('token') + expect(JSON.stringify(callArgs)).not.toContain('password') + expect(JSON.stringify(callArgs)).not.toContain('Authorization') + }) + }) +}) diff --git a/src/mcp/issue-management-server.ts b/src/mcp/issue-management-server.ts index 2591395a..c2d8ea4a 100644 --- a/src/mcp/issue-management-server.ts +++ b/src/mcp/issue-management-server.ts @@ -12,6 +12,7 @@ import { z } from 'zod' import { IssueManagementProviderFactory } from './IssueManagementProviderFactory.js' import { SettingsManager } from '../lib/SettingsManager.js' import type { IloomSettings } from '../lib/SettingsManager.js' +import { BitBucketVCSProvider } from '../lib/providers/bitbucket/BitBucketVCSProvider.js' import type { IssueProvider, GetIssueInput, @@ -28,11 +29,16 @@ import type { CloseIssueInput, ReopenIssueInput, EditIssueInput, + PRResult, + CommentResult, } from './types.js' // Module-level settings loaded at startup let settings: IloomSettings | undefined +// Module-level VCS provider for PR operations (non-null when BitBucket is configured) +let bitBucketVCSProvider: BitBucketVCSProvider | undefined + // Validate required environment variables function validateEnvironment(): IssueProvider { const provider = process.env.ISSUE_PROVIDER as IssueProvider | undefined @@ -181,19 +187,18 @@ server.registerTool( } ) -// Import GitHubIssueManagementProvider for get_pr tool (PRs always use GitHub) +// Import GitHubIssueManagementProvider for get_pr tool (fallback when BitBucket is not configured) import { GitHubIssueManagementProvider } from './GitHubIssueManagementProvider.js' // Register get_pr tool -// Note: PRs only exist on GitHub, so this tool always uses the GitHub provider -// regardless of the configured issue tracker +// Routes through the configured VCS provider (BitBucket when configured, GitHub otherwise) server.registerTool( 'get_pr', { title: 'Get Pull Request', description: 'Fetch pull request details including title, body, comments, files, commits, and branch information. ' + - 'PRs only exist on GitHub, so this tool always uses GitHub regardless of configured issue tracker. ' + + 'Routes through the configured VCS provider: BitBucket when versionControl.provider is "bitbucket", GitHub otherwise. ' + 'Author fields have normalized core fields: { id, displayName } plus provider-specific fields.', inputSchema: { number: z.string().describe('The PR number'), @@ -256,9 +261,33 @@ server.registerTool( console.error(`Fetching PR ${number}${repo ? ` from ${repo}` : ''}`) try { - // PRs always use GitHub provider regardless of configured issue tracker - const provider = new GitHubIssueManagementProvider() - const result = await provider.getPR({ number, includeComments, repo }) + let result: PRResult + + if (bitBucketVCSProvider) { + // Route through BitBucket VCS provider + const prNumber = parseInt(number, 10) + if (isNaN(prNumber)) { + throw new Error(`Invalid PR number: ${number}. PR IDs must be numeric.`) + } + console.error(`Fetching PR #${prNumber} from BitBucket`) + const bbPR = await bitBucketVCSProvider.fetchPR(prNumber) + // Map PullRequest (from VersionControlProvider) to PRResult (MCP type) + result = { + id: String(bbPR.number), + number: bbPR.number, + title: bbPR.title, + body: bbPR.body, + state: bbPR.state.toUpperCase(), + url: bbPR.url, + author: null, // BitBucket fetchPR does not return author in PullRequest type + headRefName: bbPR.branch, + baseRefName: bbPR.baseBranch, + } + } else { + // Default: use GitHub provider + const provider = new GitHubIssueManagementProvider() + result = await provider.getPR({ number, includeComments, repo }) + } console.error(`PR fetched successfully: #${result.number} - ${result.title}`) @@ -362,10 +391,27 @@ server.registerTool( console.error(`Creating ${type} comment on ${number}`) try { - // PR comments must always go to GitHub since PRs only exist on GitHub - const providerType = type === 'pr' ? 'github' : (process.env.ISSUE_PROVIDER as IssueProvider) - const provider = IssueManagementProviderFactory.create(providerType, settings) - const result = await provider.createComment({ number, body, type }) + let result: CommentResult + + if (type === 'pr' && bitBucketVCSProvider) { + // Route PR comments through BitBucket VCS provider + const prNumber = parseInt(number, 10) + if (isNaN(prNumber)) { + throw new Error(`Invalid PR number: ${number}. PR IDs must be numeric.`) + } + console.error(`Creating BitBucket PR comment on PR #${prNumber}`) + await bitBucketVCSProvider.createPRComment(prNumber, body) + // BitBucket addPRComment returns void; construct a minimal CommentResult + result = { + id: `bitbucket-pr-${prNumber}-comment`, + url: '', + } + } else { + // Route to issue provider (GitHub for 'pr' type when not BitBucket, configured provider for 'issue' type) + const providerType = type === 'pr' ? 'github' : (process.env.ISSUE_PROVIDER as IssueProvider) + const provider = IssueManagementProviderFactory.create(providerType, settings) + result = await provider.createComment({ number, body, type }) + } console.error( `Comment created successfully: ${result.id} at ${result.url}` @@ -395,12 +441,14 @@ server.registerTool( { title: 'Update Comment', description: - 'Update an existing comment. Use this to update progress during a workflow phase.', + 'Update an existing comment. Use this to update progress during a workflow phase. ' + + 'Note: BitBucket does not support PR comment editing via its REST API. ' + + 'When BitBucket VCS is configured and type is "pr", this operation will throw an error.', inputSchema: { commentId: z.string().describe('The comment identifier to update'), number: z.string().describe('The issue or PR identifier (context for providers that need it)'), body: z.string().describe('The updated comment body (markdown supported)'), - type: z.enum(['issue', 'pr']).optional().describe('Optional type to route PR comments to GitHub regardless of configured provider'), + type: z.enum(['issue', 'pr']).optional().describe('Optional type to route PR comments through the configured VCS provider'), }, outputSchema: { id: z.string(), @@ -412,7 +460,15 @@ server.registerTool( console.error(`Updating comment ${commentId} on ${type === 'pr' ? 'PR' : 'issue'} ${number}`) try { - // PR comments must always go to GitHub since PRs only exist on GitHub + if (type === 'pr' && bitBucketVCSProvider) { + // BitBucket does not support PR comment editing via REST API + throw new Error( + 'BitBucket does not support editing PR comments. ' + + 'The BitBucket REST API does not provide a PUT/PATCH endpoint for pull request comments.' + ) + } + + // Route to issue provider (GitHub for 'pr' type when not BitBucket, configured provider for 'issue' type) const providerType = type === 'pr' ? 'github' : (process.env.ISSUE_PROVIDER as IssueProvider) const provider = IssueManagementProviderFactory.create(providerType, settings) const result = await provider.updateComment({ commentId, number, body }) @@ -975,10 +1031,24 @@ async function main(): Promise { settings = await settingsManager.loadSettings() console.error('Settings loaded') + // Initialize BitBucket VCS provider if configured in settings + // This enables PR operations (get_pr, create_comment type:pr) to route through BitBucket + if (settings?.versionControl?.provider === 'bitbucket') { + try { + bitBucketVCSProvider = BitBucketVCSProvider.fromSettings(settings) + console.error('BitBucket VCS provider initialized for PR operations') + } catch (error) { + const errorMessage = error instanceof Error ? error.message : 'Unknown error' + console.error(`Failed to initialize BitBucket VCS provider: ${errorMessage}`) + console.error('PR operations will fall back to GitHub provider') + } + } + // Validate environment and get provider const provider = validateEnvironment() console.error('Environment validated') console.error(`Issue management provider: ${provider}`) + console.error(`VCS provider for PR operations: ${bitBucketVCSProvider ? 'bitbucket' : 'github'}`) if (provider === 'github') { console.error(`Repository: ${process.env.REPO_OWNER}/${process.env.REPO_NAME}`) diff --git a/templates/prompts/init-prompt.txt b/templates/prompts/init-prompt.txt index f8b6bf92..0c49f418 100644 --- a/templates/prompts/init-prompt.txt +++ b/templates/prompts/init-prompt.txt @@ -204,6 +204,10 @@ Extract these current values if they exist: - `currentJiraUsername` from `issueManagement.jira.username` field (if Jira is provider) - `currentJiraProjectKey` from `issueManagement.jira.projectKey` field (if Jira is provider) - `currentJiraBoardId` from `issueManagement.jira.boardId` field (if Jira is provider, optional) +- `currentVCSProvider` from `versionControl.provider` field (default: "github") +- `currentBitBucketUsername` from `versionControl.bitbucket.username` field (if VCS provider is BitBucket) +- `currentBitBucketApiToken` from `versionControl.bitbucket.apiToken` field (if VCS provider is BitBucket — display as 'set' or 'unset', never the actual value) +- `currentBitBucketWorkspace` from `versionControl.bitbucket.workspace` field (if VCS provider is BitBucket, optional) **If configuration already exists, display current configuration summary:** @@ -220,6 +224,10 @@ Linear Team ID: [currentLinearTeamId] (only if currentIssueProvider == linear) Jira Host: [currentJiraHost] (only if currentIssueProvider == jira) Jira Project Key: [currentJiraProjectKey] (only if currentIssueProvider == jira) Jira Board ID: [currentJiraBoardId] (only if currentIssueProvider == jira and boardId is set) +VCS Provider: [currentVCSProvider or "github (default)"] +BitBucket Username: [currentBitBucketUsername] (only if currentVCSProvider == bitbucket) +BitBucket API Token: [set/unset] (only if currentVCSProvider == bitbucket — never show actual token) +BitBucket Workspace: [currentBitBucketWorkspace or "auto-detect"] (only if currentVCSProvider == bitbucket) ``` **Then ask the user what they want to do:** @@ -380,6 +388,21 @@ Since this repository has multiple git remotes, GitHub Issues is suggested as th - Linear Issues + github-pr or github-draft-pr: Requires BOTH Linear API token AND authorized GitHub CLI (`gh`) - Jira Cloud + local merge: Requires Jira credentials (host, username, API token) only - Jira Cloud + github-pr or github-draft-pr: Requires BOTH Jira credentials AND authorized GitHub CLI (`gh`) +- Jira Cloud + bitbucket-pr: Requires BOTH Jira credentials AND BitBucket credentials (username, API token) + +**Step 1b: VCS Provider Selection** + +After selecting the issue tracker, ask which version control system the project uses: + +1b. **VCS Provider** + - Question format: "Which version control provider does this project use?{{#if currentVCSProvider}} (Currently: [currentVCSProvider]){{/if}}" + - Options: + - "github" - GitHub (default) + - "bitbucket" - Atlassian BitBucket + - Default: currentVCSProvider or "github" + - Validation: Must be one of: github, bitbucket + - Store answer as: `versionControl.provider` + - Context: This determines how iloom creates pull requests when finishing work. GitHub uses the `gh` CLI tool; BitBucket uses the BitBucket REST API with credentials you provide. **Step 2: Provider-Specific Configuration** @@ -488,6 +511,40 @@ This repository has multiple git remotes detected. iloom needs to know which rem **Note:** Advanced Jira settings like `transitionMappings`, `doneStatuses`, `defaultIssueType`, and `defaultSubtaskType` can be configured later by editing the settings files directly. Mention this to the user. +**Step 2b: BitBucket Credential Configuration** (only ask if VCS provider from Step 1b is "bitbucket") + +After selecting BitBucket as the VCS provider, gather BitBucket credentials: + +1. **BitBucket Username** (only ask if VCS provider is "bitbucket") + - Question format: "What is your BitBucket username?{{#if currentBitBucketUsername}} (Currently: [currentBitBucketUsername]){{/if}}" + - Default: currentBitBucketUsername or NO DEFAULT (required if BitBucket selected) + - Validation: Non-empty string + - Store answer as: `versionControl.bitbucket.username` + - Context: This is your BitBucket account username (visible in your BitBucket profile URL, e.g., `bitbucket.org/{username}`) + - **CRITICAL SECURITY REQUIREMENT:** This value MUST ALWAYS be saved to `settings.local.json`, NEVER to `settings.json`. This is personal account information that should not be committed to source control. + +2. **BitBucket API Token** (only ask if VCS provider is "bitbucket") + - Question format: "What is your BitBucket API token? (Currently: 'set' or 'unset' - do not print)" + - Options: + - I have it now - use the "Type something" option to enter it + - I'll come back to it + - Default: currentBitBucketApiToken or NO DEFAULT + - Validation: Non-empty string + - Guidance: Choose "Type something" to enter it + - Store answer as: `versionControl.bitbucket.apiToken` + - **CRITICAL SECURITY REQUIREMENT:** This value MUST ALWAYS be saved to `settings.local.json`, NEVER to `settings.json`, regardless of which file the user selected for other settings. This token must never be committed to source control. + - Context: Generate a BitBucket API token at https://bitbucket.org/account/settings/app-passwords/ (or use repository Access Tokens for finer-grained access). Note: App passwords were deprecated in September 2025 — use API tokens or Access Tokens instead. + +3. **BitBucket Workspace** (only ask if VCS provider is "bitbucket") + - Question format: "What is your BitBucket workspace slug? (optional — auto-detected from git remote if not provided){{#if currentBitBucketWorkspace}} (Currently: [currentBitBucketWorkspace]){{/if}}" + - Options: + - Skip - Auto-detect from git remote + - Enter workspace - use the "Type something" option to enter it + - Default: currentBitBucketWorkspace or SKIP (optional) + - Validation: If provided, non-empty string + - Store answer as: `versionControl.bitbucket.workspace` + - Context: The workspace slug appears in your BitBucket repository URL (e.g., `bitbucket.org/{workspace}/{repo}`). If omitted, iloom will auto-detect it from your git remote URL. + **Step 3: IDE Selection** 4. **IDE Selection** @@ -513,21 +570,26 @@ This repository has multiple git remotes detected. iloom needs to know which rem - "local" - Merge changes locally (traditional workflow) - "github-pr" - Create GitHub PR when finishing (for PR-based workflows) - "github-draft-pr" - Create draft PR at start, mark ready when finishing (recommended for forks) + - "bitbucket-pr" - Create BitBucket PR when finishing (requires BitBucket credentials configured in Step 1b/2b) - Default: - - If origin + upstream remotes detected (fork pattern): "github-draft-pr" + - If VCS provider (Step 1b) is "bitbucket": "bitbucket-pr" + - If origin + upstream remotes detected (fork pattern) and VCS is GitHub: "github-draft-pr" - Otherwise: currentMergeMode or "local" - - Validation: Must be one of: local, github-pr, github-draft-pr + - Validation: Must be one of: local, github-pr, github-draft-pr, bitbucket-pr - Store answer as: `mergeBehavior.mode` - Context: - **Fork workflows (with upstream remote)**: Use "github-draft-pr" mode. This creates a draft PR at the start of work, routes all AI agent comments to the PR (not the issue), and marks the PR ready for review when you finish. This keeps upstream issue trackers clean. - **Direct contribution workflows**: Use "github-pr" to create PRs when finishing, or "local" to merge locally. - - **IMPORTANT**: Both "github-pr" and "github-draft-pr" modes require GitHub CLI (`gh`) to be installed and authenticated, regardless of your issue tracker provider. + - **BitBucket workflows**: Use "bitbucket-pr" to create a BitBucket PR when finishing. Requires BitBucket credentials (username and API token) configured in the VCS provider settings. + - **IMPORTANT**: "github-pr" and "github-draft-pr" modes require GitHub CLI (`gh`) to be installed and authenticated. "bitbucket-pr" mode requires BitBucket credentials (versionControl.bitbucket.username and versionControl.bitbucket.apiToken). **Implementation Details:** -- Ask Step 1 first to determine provider -- Then ask Step 2 based on the provider selected (includes Linear API Token if Linear was selected, or Jira credentials if Jira was selected) +- Ask Step 1 first to determine issue tracker provider +- Ask Step 1b (VCS provider) immediately after Step 1 to determine VCS system +- Then ask Step 2 based on the issue tracker provider selected (includes Linear API Token if Linear was selected, or Jira credentials if Jira was selected) +- Ask Step 2b (BitBucket credentials) if VCS provider from Step 1b is "bitbucket" - Ask Step 3 (IDE) for all users -- Ask Step 4 (Merge Mode) only if multiple remotes detected OR user requests advanced config +- Ask Step 4 (Merge Mode) only if multiple remotes detected OR user requests advanced config; pre-select "bitbucket-pr" if VCS is BitBucket - Set multiSelect: false for all questions - Process answers sequentially as they depend on each other @@ -587,6 +649,10 @@ Tooling Configuration (Phase 2): Jira Project Key: [value] (only if issue tracker is Jira) Jira Board ID: [value or "not configured"] (only if issue tracker is Jira) Jira API Token: [configured/not configured] (only if issue tracker is Jira - never show actual token) + VCS Provider: [value] + BitBucket Username: [value] (only if VCS provider is BitBucket) + BitBucket API Token: [configured/not configured] (only if VCS provider is BitBucket - never show actual token) + BitBucket Workspace: [value or "auto-detect"] (only if VCS provider is BitBucket) IDE: [value or "vscode (default)"] Merge Mode: [value] (only if configured) ``` @@ -595,6 +661,8 @@ Tooling Configuration (Phase 2): - Include the GitHub Remote line in the summary ONLY if it was configured in Phase 2 (multiple remotes and GitHub selected) - Include Linear Team ID ONLY if Linear was selected as the issue tracker provider - Include Jira Host, Project Key, Board ID, and API Token ONLY if Jira was selected as the issue tracker provider +- Include VCS Provider ONLY if it differs from "github" (the default) or if BitBucket was selected +- Include BitBucket Username, API Token, and Workspace ONLY if BitBucket was selected as VCS provider - Include Merge Mode ONLY if it was asked and configured (multiple remotes or advanced config) Then ask: "Does this configuration look correct?" @@ -773,17 +841,59 @@ When both settings.json and settings.local.json exist, you MUST prevent duplicat **CRITICAL:** The `username` and `apiToken` fields MUST be saved to `settings.local.json`, never to `settings.json`. The `host`, `projectKey`, and `boardId` fields are safe to commit in `settings.json`. -6. **Add mergeBehavior ONLY if mode is not "local":** +6. **Add versionControl ONLY if provider is not "github" (the default) OR if bitbucket credentials were configured:** + + **For GitHub (default):** + - If VCS provider is "github": OMIT the entire versionControl section (it's the default) + + **For BitBucket:** + + In settings.json (committed): + ```json + { + "versionControl": { + "provider": "bitbucket" + } + } + ``` + + In settings.local.json (gitignored - MUST be separate from settings.json): + ```json + { + "versionControl": { + "bitbucket": { + "username": "", + "apiToken": "" + } + } + } + ``` + + If a workspace was provided, add it to settings.json alongside the provider: + ```json + { + "versionControl": { + "provider": "bitbucket", + "bitbucket": { + "workspace": "" + } + } + } + ``` + + **CRITICAL:** The `username` and `apiToken` fields MUST be saved to `settings.local.json`, never to `settings.json`. The `provider` and `workspace` fields are safe to commit in `settings.json`. + +7. **Add mergeBehavior ONLY if mode is not "local":** ```json { "mergeBehavior": { - "mode": "" // "github-pr" or "github-draft-pr" + "mode": "" // "github-pr", "github-draft-pr", or "bitbucket-pr" } } ``` - Note: Valid modes are "local", "github-pr", and "github-draft-pr". Only include if not "local". + Note: Valid modes are "local", "github-pr", "github-draft-pr", and "bitbucket-pr". Only include if not "local". -7. **Add colors settings based on Phase 2.5 recommendations (if vscode or cursor IDE):** +8. **Add colors settings based on Phase 2.5 recommendations (if vscode or cursor IDE):** ```json { "colors": { @@ -798,9 +908,9 @@ When both settings.json and settings.local.json exist, you MUST prevent duplicat - If user chose to keep disabled or enable anyway: set accordingly - Terminal should be true unless user explicitly disabled it -8. **Combine all applicable sections** into a single JSON object. Only include sections where the value differs from the default. +9. **Combine all applicable sections** into a single JSON object. Only include sections where the value differs from the default. -9. **If existing settings exist, MERGE** the new values with existing ones. Don't overwrite settings the user didn't modify. +10. **If existing settings exist, MERGE** the new values with existing ones. Don't overwrite settings the user didn't modify. ### Phase 6: Write File @@ -987,7 +1097,8 @@ Here's how to get started: • Run `iloom finish` when you're done working to merge changes and clean up Need more advanced configuration? I can help you set up: - • Merge Behavior - Choose between local merge, GitHub PR, or draft PR workflows + • Merge Behavior - Choose between local merge, GitHub PR, draft PR, or BitBucket PR workflows + • BitBucket Integration - Configure BitBucket credentials and PR creation • Copy Gitignored Files - Automatically copy local databases, test fixtures, or other gitignored files to new looms • Multi-Provider Code Review - Get code reviews from Gemini or Codex alongside Claude • Plan Configuration - Use different AI providers for planning and plan review @@ -1001,6 +1112,8 @@ Need more advanced configuration? I can help you set up: You can ask me questions like: "How do I set up GitHub PR mode instead of local merge?" + "How do I configure BitBucket PR creation?" + "Can I use Jira for issues and BitBucket for PRs?" "How do I copy my local SQLite database to new looms?" "Can I get code reviews from Gemini as well as Claude?" "How do I use Codex for planning?" @@ -1071,7 +1184,84 @@ When configuring agents, use these model identifiers: 3. `agents..model` (base per-agent model) 4. Agent default from its definition file -2. **Database Environment Variable:** +2. **BitBucket VCS Configuration:** + + In `.iloom/settings.json` (committed — safe to share): + ```json + { + "versionControl": { + "provider": "bitbucket", + "bitbucket": { + "workspace": "my-workspace" + } + }, + "mergeBehavior": { + "mode": "bitbucket-pr" + } + } + ``` + + In `.iloom/settings.local.json` (gitignored — keep credentials here): + ```json + { + "versionControl": { + "bitbucket": { + "username": "my-bitbucket-username", + "apiToken": "ATB..." + } + } + } + ``` + + Generate a BitBucket API token at https://bitbucket.org/account/settings/app-passwords/. Note: App passwords were deprecated in September 2025 — use API tokens or repository Access Tokens instead. + +3. **Combined Jira + BitBucket Configuration:** + + When using Jira as the issue tracker with BitBucket as the VCS provider: + + In `.iloom/settings.json` (committed): + ```json + { + "issueManagement": { + "provider": "jira", + "jira": { + "host": "https://yourcompany.atlassian.net", + "projectKey": "PROJ" + } + }, + "versionControl": { + "provider": "bitbucket", + "bitbucket": { + "workspace": "my-workspace" + } + }, + "mergeBehavior": { + "mode": "bitbucket-pr" + } + } + ``` + + In `.iloom/settings.local.json` (gitignored — credentials only): + ```json + { + "issueManagement": { + "jira": { + "username": "user@company.com", + "apiToken": "" + } + }, + "versionControl": { + "bitbucket": { + "username": "my-bitbucket-username", + "apiToken": "ATB..." + } + } + } + ``` + + This configuration allows iloom to read issues from Jira and create pull requests in BitBucket when you run `il finish`. + +4. **Database Environment Variable:** ```json "capabilities": { "database": { @@ -1080,7 +1270,7 @@ When configuring agents, use these model identifiers: } ``` -3. **Workflow Behavior:** +6. **Workflow Behavior:** ```json "workflows": { "pr": { @@ -1092,26 +1282,26 @@ When configuring agents, use these model identifiers: } ``` -4. **Copy Gitignored Files:** +7. **Copy Gitignored Files:** ```json "copyGitIgnoredPatterns": ["*.db", "data/*.sqlite", "tmp/fixtures/**"] ``` Glob patterns for gitignored files that should be copied to new looms. Useful for local databases, large test fixtures, or generated files that are too big to commit. Note: `.env` files and iloom/claude local settings are automatically copied and don't need to be listed here. -5. **Protected Branches:** +8. **Protected Branches:** ```json "protectedBranches": ["main", "production", "release/*"] ``` -6. **IDE Configuration:** +9. **IDE Configuration:** ```json "ide": { "type": "cursor" } ``` -7. **Code Review with Multiple Providers:** +10. **Code Review with Multiple Providers:** ```json "agents": { "iloom-code-reviewer": { @@ -1125,7 +1315,7 @@ When configuring agents, use these model identifiers: This configures the code reviewer to use both Claude and Gemini for reviews. Multiple providers can be configured to get reviews from different AI models. -8. **Custom Script Configuration (package.iloom.json):** +11. **Custom Script Configuration (package.iloom.json):** If users want to override or supplement npm scripts with custom commands, help them create `.iloom/package.iloom.json`: ```json diff --git a/templates/prompts/issue-prompt.txt b/templates/prompts/issue-prompt.txt index e317805a..d0d59fa9 100644 --- a/templates/prompts/issue-prompt.txt +++ b/templates/prompts/issue-prompt.txt @@ -1241,6 +1241,30 @@ This is NOT optional - if the reviewer requests Claude Local Review, it must be 5. **Check for CI/CD and deployment URLs (after successful push):** +{{#if IS_BITBUCKET}} + Wait a moment for BitBucket Pipelines to register the push, then check for pipeline status: + + a. **Check pipeline status via BitBucket Pipelines API:** + ```bash + # List recent pipelines for this branch (requires curl and BB credentials) + curl -s -u "$BITBUCKET_USERNAME:$BITBUCKET_APP_PASSWORD" \ + "https://api.bitbucket.org/2.0/repositories/{workspace}/{repo}/pipelines/?branch=$(git rev-parse --abbrev-ref HEAD)&sort=-created_on&pagelen=5" + ``` + If Pipelines API credentials are not available, instruct the user to check the BitBucket Pipelines UI for build status. + + b. **Check pull request for deployment environments:** + ```bash + # List deployment environments for the PR (requires curl and BB credentials) + curl -s -u "$BITBUCKET_USERNAME:$BITBUCKET_APP_PASSWORD" \ + "https://api.bitbucket.org/2.0/repositories/{workspace}/{repo}/deployments/?environment.name=Staging&sort=-created_on&pagelen=5" + ``` + + c. **Display available status to user:** + - If pipeline running: "BitBucket Pipeline is running. Check status at: https://bitbucket.org/{workspace}/{repo}/pipelines/" + - If pipeline passed: "CI/CD passed. Preview deployment (if configured) will appear in the PR." + - If pipeline failed: "Pipeline failed. Check details at: https://bitbucket.org/{workspace}/{repo}/pipelines/" + - If credentials not available: "CI/CD is running. Check the pull request page for pipeline status and deployment URLs." +{{else}} Wait a moment for GitHub to register the push, then check for available URLs: a. **Check recent workflow runs:** @@ -1265,6 +1289,7 @@ This is NOT optional - if the reviewer requests Claude Local Review, it must be - If deployment URLs found: "Preview deployment: [URL]" - If workflow runs found: "CI/CD workflow: [run URL]" - If no URLs available yet: "CI/CD is likely still initializing. Check the PR page for updates: https://github.com/{owner}/{repo}/pull/{{DRAFT_PR_NUMBER}}" +{{/if}} 6. **Handle errors gracefully:** - If commit fails: Log error, inform user that manual commit is needed diff --git a/tests/commands/plan-error-messages.test.ts b/tests/commands/plan-error-messages.test.ts new file mode 100644 index 00000000..cbf88485 --- /dev/null +++ b/tests/commands/plan-error-messages.test.ts @@ -0,0 +1,160 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { PlanCommand } from '../../src/commands/plan.js' +import * as claudeUtils from '../../src/utils/claude.js' +import * as mcpUtils from '../../src/utils/mcp.js' +import * as firstRunSetup from '../../src/utils/first-run-setup.js' +import * as identifierParser from '../../src/utils/IdentifierParser.js' +import { IssueTrackerFactory } from '../../src/lib/IssueTrackerFactory.js' + +// Mock all external dependencies +vi.mock('../../src/utils/logger.js', () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + success: vi.fn(), + }, + createStderrLogger: vi.fn().mockReturnValue({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + success: vi.fn(), + }), +})) + +vi.mock('../../src/utils/logger-context.js', () => ({ + withLogger: vi.fn().mockImplementation((_logger: unknown, fn: () => unknown) => fn()), + getLogger: vi.fn().mockReturnValue({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})) + +vi.mock('../../src/utils/claude.js') +vi.mock('../../src/utils/mcp.js') +vi.mock('../../src/utils/first-run-setup.js') +vi.mock('../../src/utils/IdentifierParser.js') + +vi.mock('../../src/lib/SettingsManager.js', () => ({ + SettingsManager: vi.fn(() => ({ + // Return a non-null settings object so IssueTrackerFactory.getProviderName is called with it + loadSettings: vi.fn().mockResolvedValue({ issueManagement: { provider: 'github' } }), + getPlanModel: vi.fn().mockReturnValue('opus'), + getPlanPlanner: vi.fn().mockReturnValue('claude'), + getPlanReviewer: vi.fn().mockReturnValue('none'), + })), + PlanCommandSettingsSchema: { + shape: { + planner: { safeParse: vi.fn().mockReturnValue({ success: true }) }, + reviewer: { safeParse: vi.fn().mockReturnValue({ success: true }) }, + }, + }, +})) + +vi.mock('../../src/lib/IssueTrackerFactory.js', () => ({ + IssueTrackerFactory: { + getProviderName: vi.fn().mockReturnValue('github'), + create: vi.fn().mockReturnValue({ + detectInputType: vi.fn(), + }), + }, +})) + +vi.mock('../../src/mcp/IssueManagementProviderFactory.js', () => ({ + IssueManagementProviderFactory: { + create: vi.fn(), + }, +})) + +vi.mock('../../src/utils/prompt.js', () => ({ + promptConfirmation: vi.fn(), + isInteractiveEnvironment: vi.fn().mockReturnValue(false), // Non-interactive for direct error path +})) + +vi.mock('../../src/lib/TelemetryService.js', () => ({ + TelemetryService: { + getInstance: vi.fn().mockReturnValue({ track: vi.fn() }), + }, +})) + +describe('PlanCommand - provider-specific error messages', () => { + let command: PlanCommand + + beforeEach(() => { + // Claude CLI is available so we proceed to MCP config check + vi.mocked(claudeUtils.detectClaudeCli).mockResolvedValue(true) + vi.mocked(claudeUtils.launchClaude).mockResolvedValue(undefined) + + // MCP config generation fails - this triggers the provider-specific error messages + vi.mocked(mcpUtils.generateIssueManagementMcpConfig).mockRejectedValue( + new Error('No remote configured') + ) + + vi.mocked(firstRunSetup.needsFirstRunSetup).mockResolvedValue(false) + vi.mocked(firstRunSetup.launchFirstRunSetup).mockResolvedValue(undefined) + vi.mocked(identifierParser.matchIssueIdentifier).mockReturnValue({ isIssueIdentifier: false }) + + command = new PlanCommand() + }) + + it('throws Jira-specific error message when provider is jira (non-interactive)', async () => { + vi.mocked(IssueTrackerFactory.getProviderName).mockReturnValue('jira') + + await expect(command.execute()).rejects.toThrow('JIRA_API_TOKEN') + }) + + it('Jira error message does not reference LINEAR_API_TOKEN', async () => { + vi.mocked(IssueTrackerFactory.getProviderName).mockReturnValue('jira') + + let errorMessage = '' + try { + await command.execute() + } catch (e) { + errorMessage = e instanceof Error ? e.message : String(e) + } + expect(errorMessage).not.toContain('LINEAR_API_TOKEN') + expect(errorMessage).toContain('JIRA_API_TOKEN') + }) + + it('throws Linear-specific error message when provider is linear (non-interactive)', async () => { + vi.mocked(IssueTrackerFactory.getProviderName).mockReturnValue('linear') + + await expect(command.execute()).rejects.toThrow('LINEAR_API_TOKEN') + }) + + it('Linear error message does not reference JIRA_API_TOKEN', async () => { + vi.mocked(IssueTrackerFactory.getProviderName).mockReturnValue('linear') + + let errorMessage = '' + try { + await command.execute() + } catch (e) { + errorMessage = e instanceof Error ? e.message : String(e) + } + expect(errorMessage).not.toContain('JIRA_API_TOKEN') + expect(errorMessage).toContain('LINEAR_API_TOKEN') + }) + + it('throws GitHub-specific error message when provider is github (non-interactive)', async () => { + vi.mocked(IssueTrackerFactory.getProviderName).mockReturnValue('github') + + await expect(command.execute()).rejects.toThrow('GitHub remote') + }) + + it('GitHub error message does not reference LINEAR_API_TOKEN or JIRA_API_TOKEN', async () => { + vi.mocked(IssueTrackerFactory.getProviderName).mockReturnValue('github') + + let errorMessage = '' + try { + await command.execute() + } catch (e) { + errorMessage = e instanceof Error ? e.message : String(e) + } + expect(errorMessage).not.toContain('LINEAR_API_TOKEN') + expect(errorMessage).not.toContain('JIRA_API_TOKEN') + }) +}) diff --git a/tests/commands/summary-bitbucket.test.ts b/tests/commands/summary-bitbucket.test.ts new file mode 100644 index 00000000..9d3f604f --- /dev/null +++ b/tests/commands/summary-bitbucket.test.ts @@ -0,0 +1,148 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { SummaryCommand } from '../../src/commands/summary.js' +import type { GitWorktreeManager } from '../../src/lib/GitWorktreeManager.js' +import type { MetadataManager } from '../../src/lib/MetadataManager.js' +import type { SessionSummaryService } from '../../src/lib/SessionSummaryService.js' +import type { SettingsManager } from '../../src/lib/SettingsManager.js' +vi.mock('../../src/utils/logger-context.js', () => ({ + getLogger: vi.fn().mockReturnValue({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})) + +vi.mock('../../src/utils/git.js', () => ({ + extractIssueNumber: vi.fn().mockReturnValue(42), +})) + +describe('SummaryCommand - getPRNumberForPosting bitbucket-pr support', () => { + let mockGitWorktreeManager: Partial + let mockMetadataManager: Partial + let mockSessionSummaryService: Partial + let mockSettingsManager: Partial + let command: SummaryCommand + + const makeCommand = () => { + return new SummaryCommand( + mockGitWorktreeManager as GitWorktreeManager, + mockMetadataManager as MetadataManager, + mockSessionSummaryService as SessionSummaryService, + mockSettingsManager as SettingsManager, + ) + } + + beforeEach(() => { + const mockWorktree = { path: '/test/worktree', branch: 'feat/issue-42-test' } + + mockGitWorktreeManager = { + findWorktreeForIssue: vi.fn().mockResolvedValue(mockWorktree), + findWorktreeForPR: vi.fn().mockResolvedValue(null), + findWorktreeForBranch: vi.fn().mockResolvedValue(null), + } + + mockMetadataManager = { + readMetadata: vi.fn().mockResolvedValue({ + issueType: 'issue', + issue_numbers: ['42'], + }), + } + + mockSessionSummaryService = { + generateSummary: vi.fn().mockResolvedValue({ summary: 'Test summary', sessionId: 'sess-1' }), + applyAttribution: vi.fn().mockImplementation((s: string) => Promise.resolve(s)), + postSummary: vi.fn().mockResolvedValue(undefined), + } + + mockSettingsManager = { + loadSettings: vi.fn().mockResolvedValue({ + mergeBehavior: { mode: 'local' }, + }), + } + + command = makeCommand() + }) + + it('posts to PR when merge mode is bitbucket-pr and draftPrNumber is set', async () => { + vi.mocked(mockSettingsManager.loadSettings!).mockResolvedValue({ + mergeBehavior: { mode: 'bitbucket-pr' }, + }) + vi.mocked(mockMetadataManager.readMetadata!).mockResolvedValue({ + issueType: 'issue', + issue_numbers: ['42'], + draftPrNumber: 99, + }) + + await command.execute({ + identifier: '42', + options: { withComment: true }, + }) + + expect(mockSessionSummaryService.postSummary).toHaveBeenCalledWith( + expect.anything(), + 'Test summary', + expect.anything(), + 99 // PR number from metadata + ) + }) + + it('posts to issue (no PR number) when merge mode is bitbucket-pr and no draftPrNumber', async () => { + vi.mocked(mockSettingsManager.loadSettings!).mockResolvedValue({ + mergeBehavior: { mode: 'bitbucket-pr' }, + }) + vi.mocked(mockMetadataManager.readMetadata!).mockResolvedValue({ + issueType: 'issue', + issue_numbers: ['42'], + draftPrNumber: null, + }) + + await command.execute({ + identifier: '42', + options: { withComment: true }, + }) + + expect(mockSessionSummaryService.postSummary).toHaveBeenCalledWith( + expect.anything(), + 'Test summary', + expect.anything(), + undefined // No PR number - post to issue + ) + }) + + it('posts to issue when merge mode is local', async () => { + vi.mocked(mockSettingsManager.loadSettings!).mockResolvedValue({ + mergeBehavior: { mode: 'local' }, + }) + + await command.execute({ + identifier: '42', + options: { withComment: true }, + }) + + expect(mockSessionSummaryService.postSummary).toHaveBeenCalledWith( + expect.anything(), + 'Test summary', + expect.anything(), + undefined // No PR number for local mode + ) + }) + + it('does not post when withComment is false', async () => { + vi.mocked(mockSettingsManager.loadSettings!).mockResolvedValue({ + mergeBehavior: { mode: 'bitbucket-pr' }, + }) + vi.mocked(mockMetadataManager.readMetadata!).mockResolvedValue({ + issueType: 'issue', + issue_numbers: ['42'], + draftPrNumber: 99, + }) + + await command.execute({ + identifier: '42', + options: { withComment: false }, + }) + + expect(mockSessionSummaryService.postSummary).not.toHaveBeenCalled() + }) +}) diff --git a/tests/lib/PRManager-provider-agnostic.test.ts b/tests/lib/PRManager-provider-agnostic.test.ts new file mode 100644 index 00000000..b6413fab --- /dev/null +++ b/tests/lib/PRManager-provider-agnostic.test.ts @@ -0,0 +1,76 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { PRManager } from '../../src/lib/PRManager.js' + +vi.mock('../../src/utils/github.js', () => ({ + executeGhCommand: vi.fn(), +})) + +vi.mock('../../src/utils/claude.js', () => ({ + launchClaude: vi.fn(), + detectClaudeCli: vi.fn().mockResolvedValue(false), // No Claude available, use template +})) + +vi.mock('../../src/utils/remote.js', () => ({ + getEffectivePRTargetRemote: vi.fn().mockResolvedValue('origin'), + getConfiguredRepoFromSettings: vi.fn(), + parseGitRemotes: vi.fn().mockResolvedValue([]), +})) + +vi.mock('../../src/utils/browser.js', () => ({ + openBrowser: vi.fn(), +})) + +vi.mock('../../src/utils/logger-context.js', () => ({ + getLogger: vi.fn().mockReturnValue({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})) + +vi.mock('../../src/mcp/IssueManagementProviderFactory.js', () => ({ + IssueManagementProviderFactory: { + create: vi.fn().mockReturnValue({ + issuePrefix: '#', + }), + }, +})) + +describe('PRManager - provider-agnostic PR body prompt', () => { + let prManager: PRManager + + beforeEach(async () => { + const { IssueManagementProviderFactory } = await import('../../src/mcp/IssueManagementProviderFactory.js') + vi.mocked(IssueManagementProviderFactory.create).mockReturnValue({ + issuePrefix: '#', + } as never) + + // Create with full mock settings including issueManagement + prManager = new PRManager({ issueManagement: { provider: 'github' } }) + }) + + it('uses generic "pull request" language in the PR body prompt (not "GitHub pull request")', () => { + // Access private method via type assertion to verify the prompt content + const prompt = (prManager as unknown as { buildPRBodyPrompt: (n?: number) => string }).buildPRBodyPrompt(42) + + expect(prompt).not.toContain('GitHub pull request') + expect(prompt).toContain('pull request') + }) + + it('does not mention "GitHub pull request body" in the output instruction', () => { + const prompt = (prManager as unknown as { buildPRBodyPrompt: (n?: number) => string }).buildPRBodyPrompt() + + // The output instruction should be provider-agnostic + expect(prompt).not.toMatch(/GitHub pull request body/i) + // Should still tell the AI to produce a PR body + expect(prompt).toMatch(/pull request body/i) + }) + + it('generates fallback template body without hardcoded GitHub reference', async () => { + // When Claude is unavailable (mocked to return false), falls back to template + const body = await prManager.generatePRBody(42, '/test/path') + expect(body).not.toContain('GitHub') + expect(body).toContain('42') // Issue number should be referenced + }) +}) diff --git a/tests/lib/SessionSummaryService-vcs-routing.test.ts b/tests/lib/SessionSummaryService-vcs-routing.test.ts new file mode 100644 index 00000000..f3954979 --- /dev/null +++ b/tests/lib/SessionSummaryService-vcs-routing.test.ts @@ -0,0 +1,171 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { SessionSummaryService } from '../../src/lib/SessionSummaryService.js' +import type { PromptTemplateManager } from '../../src/lib/PromptTemplateManager.js' +import type { MetadataManager } from '../../src/lib/MetadataManager.js' +import type { SettingsManager } from '../../src/lib/SettingsManager.js' + +// Mock all external dependencies +vi.mock('../../src/utils/logger.js', () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + success: vi.fn(), + }, +})) + +vi.mock('../../src/utils/claude.js', () => ({ + launchClaude: vi.fn(), + generateDeterministicSessionId: vi.fn(), +})) + +vi.mock('../../src/utils/claude-transcript.js', () => ({ + readSessionContext: vi.fn(), +})) + +vi.mock('../../src/utils/remote.js', () => ({ + hasMultipleRemotes: vi.fn(), +})) + +vi.mock('fs-extra', () => ({ + default: { + pathExists: vi.fn(), + readFile: vi.fn(), + }, +})) + +// Mock IssueManagementProviderFactory +vi.mock('../../src/mcp/IssueManagementProviderFactory.js', () => ({ + IssueManagementProviderFactory: { + create: vi.fn(), + }, +})) + +// Mock VCSProviderFactory +vi.mock('../../src/lib/VCSProviderFactory.js', () => ({ + VCSProviderFactory: { + create: vi.fn(), + }, +})) + +describe('SessionSummaryService - VCS provider routing for PR comments', () => { + let service: SessionSummaryService + let mockTemplateManager: Partial + let mockMetadataManager: Partial + let mockSettingsManager: Partial + let mockCreateComment: ReturnType + + const makeService = (issueProvider = 'github') => { + mockSettingsManager = { + loadSettings: vi.fn().mockResolvedValue({ + attribution: 'off', + issueManagement: { provider: issueProvider }, + }), + getSummaryModel: vi.fn().mockReturnValue('haiku'), + } + return new SessionSummaryService( + mockTemplateManager as PromptTemplateManager, + mockMetadataManager as MetadataManager, + mockSettingsManager as SettingsManager + ) + } + + beforeEach(async () => { + const { hasMultipleRemotes } = await import('../../src/utils/remote.js') + vi.mocked(hasMultipleRemotes).mockResolvedValue(false) + + const { VCSProviderFactory } = await import('../../src/lib/VCSProviderFactory.js') + vi.mocked(VCSProviderFactory.create).mockReturnValue(null) + + mockCreateComment = vi.fn().mockResolvedValue({}) + const { IssueManagementProviderFactory } = await import('../../src/mcp/IssueManagementProviderFactory.js') + vi.mocked(IssueManagementProviderFactory.create).mockReturnValue({ + createComment: mockCreateComment, + } as never) + + mockTemplateManager = { + getPrompt: vi.fn().mockResolvedValue('Mock prompt'), + } + + mockMetadataManager = { + readMetadata: vi.fn().mockResolvedValue(null), + } + + service = makeService() + }) + + it('uses VCS provider (BitBucket) to post PR comment when prNumber is provided and BitBucket is configured', async () => { + const { VCSProviderFactory } = await import('../../src/lib/VCSProviderFactory.js') + const { IssueManagementProviderFactory } = await import('../../src/mcp/IssueManagementProviderFactory.js') + + const mockCreatePRComment = vi.fn().mockResolvedValue(undefined) + vi.mocked(VCSProviderFactory.create).mockReturnValue({ + providerName: 'bitbucket', + supportsForks: false, + supportsDraftPRs: false, + createPRComment: mockCreatePRComment, + checkForExistingPR: vi.fn(), + createPR: vi.fn(), + fetchPR: vi.fn(), + getPRUrl: vi.fn(), + detectRepository: vi.fn(), + getTargetRemote: vi.fn(), + }) + + await service.postSummary(42, 'Test summary', '/test/path', 99) + + // Should use BitBucket VCS provider, not GitHub issue management + expect(mockCreatePRComment).toHaveBeenCalledWith(99, expect.any(String), '/test/path') + // GitHub issue management should NOT have been called for PR posting + expect(IssueManagementProviderFactory.create).not.toHaveBeenCalled() + }) + + it('uses GitHub issue management provider to post PR comment when no VCS provider is configured', async () => { + const { IssueManagementProviderFactory } = await import('../../src/mcp/IssueManagementProviderFactory.js') + + // VCSProviderFactory.create already returns null from beforeEach + + await service.postSummary(42, 'Test summary', '/test/path', 99) + + // Should fall back to GitHub issue management with type 'pr' + expect(IssueManagementProviderFactory.create).toHaveBeenCalledWith('github', expect.anything()) + expect(mockCreateComment).toHaveBeenCalledWith({ + number: '99', + body: expect.any(String), + type: 'pr', + }) + }) + + it('uses configured issue management provider (jira) for issue comments - not hardcoded github', async () => { + const { IssueManagementProviderFactory } = await import('../../src/mcp/IssueManagementProviderFactory.js') + + // Create service with jira settings + service = makeService('jira') + + // Post to issue (no prNumber) + await service.postSummary(42, 'Test summary', '/test/path') + + // Should use jira provider, not hardcoded github + expect(IssueManagementProviderFactory.create).toHaveBeenCalledWith('jira', expect.anything()) + expect(mockCreateComment).toHaveBeenCalledWith({ + number: '42', + body: expect.any(String), + type: 'issue', + }) + }) + + it('uses GitHub issue management when posting to issue with default settings', async () => { + const { IssueManagementProviderFactory } = await import('../../src/mcp/IssueManagementProviderFactory.js') + + // Post to issue (no prNumber) with default settings (github) + await service.postSummary(42, 'Test summary', '/test/path') + + expect(IssueManagementProviderFactory.create).toHaveBeenCalledWith('github', expect.anything()) + expect(mockCreateComment).toHaveBeenCalledWith({ + number: '42', + body: expect.any(String), + type: 'issue', + }) + }) +}) From 86d53a80a0aa26fe46d47e1e51e8b1fe7504a039 Mon Sep 17 00:00:00 2001 From: Adam Creeger Date: Sun, 22 Feb 2026 22:10:32 -0500 Subject: [PATCH 10/11] fix: address code review findings on BitBucket integration - Fix error swallowing in start.ts PR detection (only catch 404s) - Fix error swallowing in checkForExistingPR (only return null for 404) - Bound workspace member pagination to 10 pages max - Replace dynamic imports with static imports in finish.ts BitBucket path - Add vcs_provider telemetry to loom.created, bitbucket-pr to loom.finished - Improve MCP fallback warning when BitBucket config is invalid - Pass prNumber to session summary for BitBucket PRs --- src/commands/finish.ts | 20 ++++++----------- src/commands/start.test.ts | 5 +++-- src/commands/start.ts | 16 +++++++++----- .../providers/bitbucket/BitBucketApiClient.ts | 9 +++++++- .../bitbucket/BitBucketVCSProvider.test.ts | 22 ++++++++++++++----- .../bitbucket/BitBucketVCSProvider.ts | 9 ++++---- src/mcp/issue-management-server.ts | 4 ++-- src/types/telemetry.ts | 3 ++- 8 files changed, 54 insertions(+), 34 deletions(-) diff --git a/src/commands/finish.ts b/src/commands/finish.ts index 4e5d3e55..75f1583a 100644 --- a/src/commands/finish.ts +++ b/src/commands/finish.ts @@ -30,6 +30,7 @@ import type { ResourceCleanupOptions, CleanupResult } from '../types/cleanup.js' import type { ParsedInput } from './start.js' import { TelemetryService } from '../lib/TelemetryService.js' import { MetadataManager } from '../lib/MetadataManager.js' +import { VCSProviderFactory } from '../lib/VCSProviderFactory.js' import path from 'path' export interface FinishCommandInput { @@ -288,7 +289,7 @@ export class FinishCommand { ? Math.round((Date.now() - new Date(preFinishCreatedAt).getTime()) / 60000) : 0 TelemetryService.getInstance().track('loom.finished', { - merge_behavior: (settings.mergeBehavior?.mode as 'local' | 'github-pr' | 'github-draft-pr') ?? 'local', + merge_behavior: (settings.mergeBehavior?.mode as 'local' | 'github-pr' | 'github-draft-pr' | 'bitbucket-pr') ?? 'local', duration_minutes: isNaN(durationMinutes) ? 0 : durationMinutes, }) } catch (error: unknown) { @@ -901,12 +902,10 @@ export class FinishCommand { // For BitBucket, we use the VCS provider layer - NOT the issue tracker // This allows Jira/Linear issues to create PRs in BitBucket // Read vcsProvider from metadata to confirm expected provider, then create from settings - const { MetadataManager: MetadataManagerForBB } = await import('../lib/MetadataManager.js') - const bbMetadataManager = new MetadataManagerForBB() + const bbMetadataManager = new MetadataManager() const bbMetadata = await bbMetadataManager.readMetadata(worktree.path) const metadataVcsProvider = bbMetadata?.vcsProvider - const { VCSProviderFactory } = await import('../lib/VCSProviderFactory.js') const vcsProvider = VCSProviderFactory.create(settings) if (!vcsProvider || vcsProvider.providerName !== 'bitbucket') { @@ -951,7 +950,6 @@ export class FinishCommand { await this.generateSessionSummaryIfConfigured(parsed, worktree, options) // Step 5.8: Archive metadata BEFORE cleanup decision (ensures it runs even with --no-cleanup) - const { MetadataManager } = await import('../lib/MetadataManager.js') const metadataManager = new MetadataManager() if (!options.dryRun) { await metadataManager.archiveMetadata(worktree.path) @@ -1235,22 +1233,18 @@ export class FinishCommand { // Set PR URL in result finishResult.prUrl = prUrl - // Generate session summary: - // - GitHub: post to the PR (prNumber is available) - // - Other providers (BitBucket): post to the issue, since the issue tracker - // (Jira/Linear) doesn't support PR comments + // Generate session summary and post to the PR await this.generateSessionSummaryIfConfigured( parsed, worktree, options, - !vcsProvider ? prNumber : undefined + prNumber ) // Archive metadata BEFORE cleanup prompt (ensures it runs even with --no-cleanup) - const { MetadataManager } = await import('../lib/MetadataManager.js') - const metadataManager = new MetadataManager() + const archiveMetadataManager = new MetadataManager() if (!options.dryRun) { - await metadataManager.archiveMetadata(worktree.path) + await archiveMetadataManager.archiveMetadata(worktree.path) } // Interactive cleanup prompt (unless flags override) diff --git a/src/commands/start.test.ts b/src/commands/start.test.ts index 2133038e..cd4c9a04 100644 --- a/src/commands/start.test.ts +++ b/src/commands/start.test.ts @@ -1565,6 +1565,7 @@ describe('StartCommand', () => { expect(mockTrack).toHaveBeenCalledWith('loom.created', { source_type: 'issue', tracker: 'github', + vcs_provider: 'github', is_child_loom: false, one_shot_mode: 'default', }) @@ -2334,8 +2335,8 @@ describe('Jira+BitBucket VCS provider PR detection', () => { }) it('should fall back to issue tracker when BitBucket PR not found', async () => { - // BitBucket throws when PR doesn't exist - mockBitBucketProvider.fetchPR.mockRejectedValue(new Error('PR not found')) + // BitBucket throws 404 when PR doesn't exist + mockBitBucketProvider.fetchPR.mockRejectedValue(new Error('BitBucket API error (404): Not Found')) mockJiraService.fetchIssue.mockResolvedValue({ number: 42, title: 'Jira Issue', diff --git a/src/commands/start.ts b/src/commands/start.ts index 67e2c0af..48266902 100644 --- a/src/commands/start.ts +++ b/src/commands/start.ts @@ -392,6 +392,7 @@ export class StartCommand { TelemetryService.getInstance().track('loom.created', { source_type: parsed.type === 'epic' ? 'issue' : parsed.type as LoomCreatedProperties['source_type'], tracker: this.issueTracker.providerName, + vcs_provider: (settings.versionControl?.provider as 'github' | 'bitbucket') ?? 'github', is_child_loom: !!parentLoom, one_shot_mode: oneShotMap[input.options.oneShot ?? ''] ?? 'default', }) @@ -536,13 +537,16 @@ export class StartCommand { number, originalInput: trimmedIdentifier, } - } catch { - // Not a VCS PR - treat as an issue - return { - type: 'issue', - number, - originalInput: trimmedIdentifier, + } catch (error) { + // Only treat as "not a PR" for 404 errors; re-throw auth/network/server errors + if (error instanceof Error && error.message.includes('(404)')) { + return { + type: 'issue', + number, + originalInput: trimmedIdentifier, + } } + throw error } } else { // No non-GitHub VCS provider: fall back to GitHubService diff --git a/src/lib/providers/bitbucket/BitBucketApiClient.ts b/src/lib/providers/bitbucket/BitBucketApiClient.ts index 54b4a053..1ae1afa7 100644 --- a/src/lib/providers/bitbucket/BitBucketApiClient.ts +++ b/src/lib/providers/bitbucket/BitBucketApiClient.ts @@ -349,10 +349,12 @@ export class BitBucketApiClient { * Fetch all workspace members with pagination */ private async getAllWorkspaceMembers(workspace: string): Promise { + const MAX_PAGES = 10 const allMembers: BitBucketWorkspaceMember[] = [] let nextUrl: string | null = `/workspaces/${workspace}/members` + let pageCount = 0 - while (nextUrl) { + while (nextUrl && pageCount < MAX_PAGES) { const response: BitBucketWorkspaceMembersResponse = await this.get(nextUrl) @@ -361,6 +363,11 @@ export class BitBucketApiClient { // BitBucket pagination uses 'next' field with full URL // Use it directly since request() now handles full URLs nextUrl = response.next ?? null + pageCount++ + } + + if (pageCount >= MAX_PAGES) { + getLogger().warn(`Stopped fetching workspace members after ${MAX_PAGES} pages. Some reviewers may not be resolved.`) } getLogger().debug(`Fetched ${allMembers.length} workspace members from BitBucket`) diff --git a/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts b/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts index 3db12bda..99fb5159 100644 --- a/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts +++ b/src/lib/providers/bitbucket/BitBucketVCSProvider.test.ts @@ -418,7 +418,7 @@ describe('BitBucketVCSProvider', () => { ) }) - it('should return null for network/other errors', async () => { + it('should throw for network/other errors', async () => { const config: BitBucketVCSConfig = { username: 'testuser', apiToken: 'test-token', @@ -429,12 +429,10 @@ describe('BitBucketVCSProvider', () => { new Error('BitBucket API request failed: ECONNREFUSED') ) - const result = await provider.checkForExistingPR('feature-branch') - - expect(result).toBeNull() + await expect(provider.checkForExistingPR('feature-branch')).rejects.toThrow('ECONNREFUSED') }) - it('should return null for non-Error thrown values', async () => { + it('should throw for non-Error thrown values', async () => { const config: BitBucketVCSConfig = { username: 'testuser', apiToken: 'test-token', @@ -443,6 +441,20 @@ describe('BitBucketVCSProvider', () => { mockClient.listPullRequests.mockRejectedValue('string error') + await expect(provider.checkForExistingPR('feature-branch')).rejects.toBe('string error') + }) + + it('should return null for 404 errors', async () => { + const config: BitBucketVCSConfig = { + username: 'testuser', + apiToken: 'test-token', + } + provider = new BitBucketVCSProvider(config) + + mockClient.listPullRequests.mockRejectedValue( + new Error('BitBucket API error (404): Not Found') + ) + const result = await provider.checkForExistingPR('feature-branch') expect(result).toBeNull() diff --git a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts index 60e675aa..56a83378 100644 --- a/src/lib/providers/bitbucket/BitBucketVCSProvider.ts +++ b/src/lib/providers/bitbucket/BitBucketVCSProvider.ts @@ -91,17 +91,18 @@ export class BitBucketVCSProvider implements VersionControlProvider { return null } catch (error) { + // Only return null for 404 (not found) — re-throw all other errors if (error instanceof Error) { const statusMatch = error.message.match(/BitBucket API error \((\d+)\)/) if (statusMatch?.[1]) { const statusCode = parseInt(statusMatch[1], 10) - if (statusCode === 401 || statusCode === 403) { - throw error + if (statusCode === 404) { + getLogger().debug('No existing PR found for branch', { error }) + return null } } } - getLogger().debug('Error checking for existing PR', { error }) - return null + throw error } } diff --git a/src/mcp/issue-management-server.ts b/src/mcp/issue-management-server.ts index c2d8ea4a..011b9e36 100644 --- a/src/mcp/issue-management-server.ts +++ b/src/mcp/issue-management-server.ts @@ -1039,8 +1039,8 @@ async function main(): Promise { console.error('BitBucket VCS provider initialized for PR operations') } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error' - console.error(`Failed to initialize BitBucket VCS provider: ${errorMessage}`) - console.error('PR operations will fall back to GitHub provider') + console.error(`[WARNING] BitBucket VCS provider failed to initialize: ${errorMessage}`) + console.error('[WARNING] PR operations (get_pr, PR comments) will fall back to GitHub. Fix BitBucket config to use BitBucket for PRs.') } } diff --git a/src/types/telemetry.ts b/src/types/telemetry.ts index 03463fb3..51bd1791 100644 --- a/src/types/telemetry.ts +++ b/src/types/telemetry.ts @@ -26,12 +26,13 @@ export interface CliUpgradedProperties { export interface LoomCreatedProperties { source_type: 'issue' | 'pr' | 'branch' | 'freeform' tracker: string // 'github' | 'linear' | 'jira' | 'bitbucket' + vcs_provider?: 'github' | 'bitbucket' is_child_loom: boolean one_shot_mode: 'default' | 'skip-reviews' | 'yolo' } export interface LoomFinishedProperties { - merge_behavior: 'local' | 'github-pr' | 'github-draft-pr' + merge_behavior: 'local' | 'github-pr' | 'github-draft-pr' | 'bitbucket-pr' duration_minutes: number } From 8ea244f492cf742f05d7075524ea0f8ce0a893b1 Mon Sep 17 00:00:00 2001 From: Adam Creeger Date: Sun, 1 Mar 2026 15:06:00 -0500 Subject: [PATCH 11/11] [iloom] Temporary commit for draft PR (will be removed on finish)