From 4a6a9c34c7467da40d48a558fea5ef3048210ba5 Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Wed, 11 Feb 2026 09:50:46 -0800 Subject: [PATCH 1/6] 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 # Conflicts: # README.md # src/lib/SessionSummaryService.ts # src/lib/SettingsManager.ts --- README.md | 120 ++++- src/cli.ts | 88 ++++ src/commands/finish.ts | 141 ++++++ src/lib/PRManager.ts | 2 +- src/lib/ProviderCoordinator.ts | 165 +++++++ src/lib/SessionSummaryService.test.ts | 10 +- src/lib/SessionSummaryService.ts | 2 +- 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 +- 20 files changed, 2607 insertions(+), 24 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 f0fdd3ed..33747367 100644 --- a/README.md +++ b/README.md @@ -447,6 +447,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: @@ -495,14 +504,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. @@ -522,7 +639,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 9a3cf036..33a634d4 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -560,6 +560,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 mode only)') .option('--cleanup', 'Clean up worktree after finishing (default in local mode)') .option('--no-cleanup', 'Keep worktree after finishing') @@ -2082,6 +2083,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 ff824b7c..8140f254 100644 --- a/src/commands/finish.ts +++ b/src/commands/finish.ts @@ -846,6 +846,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) @@ -1127,6 +1144,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/PRManager.ts b/src/lib/PRManager.ts index 86102c57..29ad063a 100644 --- a/src/lib/PRManager.ts +++ b/src/lib/PRManager.ts @@ -27,7 +27,7 @@ export class PRManager { */ public get issuePrefix(): string { const providerType = this.settings.issueManagement?.provider ?? 'github' - const provider = IssueManagementProviderFactory.create(providerType) + const provider = IssueManagementProviderFactory.create(providerType, this.settings) return provider.issuePrefix } 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 d8b5fc82..11f8f928 100644 --- a/src/lib/SessionSummaryService.test.ts +++ b/src/lib/SessionSummaryService.test.ts @@ -170,7 +170,7 @@ describe('SessionSummaryService', () => { }) // Verify provider was created and comment was posted - expect(IssueManagementProviderFactory.create).toHaveBeenCalledWith('github') + expect(IssueManagementProviderFactory.create).toHaveBeenCalledWith('github', defaultSettings) expect(mockIssueProvider.createComment).toHaveBeenCalledWith({ number: '123', body: '## iloom Session Summary\n\n**Key Themes:**\n- Theme one about testing\n- Theme two about implementation\n\n### Key Insights\n- Test insight one\n- Test insight two', @@ -247,16 +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') + expect(IssueManagementProviderFactory.create).toHaveBeenCalledWith('linear', mockSettingsValue) }) it('should skip when Claude returns empty result', async () => { diff --git a/src/lib/SessionSummaryService.ts b/src/lib/SessionSummaryService.ts index d59b651e..e6e712d2 100644 --- a/src/lib/SessionSummaryService.ts +++ b/src/lib/SessionSummaryService.ts @@ -408,7 +408,7 @@ export class SessionSummaryService { const providerType = prNumber !== undefined ? 'github' : (settings.issueManagement?.provider ?? 'github') as IssueProvider - const provider = IssueManagementProviderFactory.create(providerType) + const provider = IssueManagementProviderFactory.create(providerType, settings) // Apply attribution if configured const finalSummary = await this.applyAttributionWithSettings(summary, settings, worktreePath) diff --git a/src/lib/SettingsManager.test.ts b/src/lib/SettingsManager.test.ts index 8d8b6df3..ad3aafdb 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 99fddbe9..129d9ba0 100644 --- a/src/lib/SettingsManager.ts +++ b/src/lib/SettingsManager.ts @@ -413,10 +413,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() @@ -424,9 +454,10 @@ export const IloomSettingsSchema = z.object({ .describe( 'Auto-commit and push after code review in draft PR mode. Defaults to true when mode is github-draft-pr.' ), + 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 @@ -649,9 +680,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() @@ -659,9 +720,10 @@ export const IloomSettingsSchemaNoDefaults = z.object({ .describe( 'Auto-commit and push after code review in draft PR mode. Defaults to true when mode is github-draft-pr.' ), + 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 467ab614..ca72be06 100644 --- a/src/utils/claude.ts +++ b/src/utils/claude.ts @@ -209,6 +209,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 @@ -219,7 +224,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 096e0c71f20b5eb7bfa19504eeaba8e5396cca54 Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 09:48:19 -0800 Subject: [PATCH 2/6] 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 | 112 +++++++++++++++++++++++++++++++---------- 1 file changed, 86 insertions(+), 26 deletions(-) diff --git a/src/commands/issues.ts b/src/commands/issues.ts index 1c23bdfd..2ebf1b45 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' @@ -200,32 +202,90 @@ 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, - }) - 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') || - 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, + }) + 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') || + 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 8567a84ab5ad602e3db6da6096186e9eb6bec41d Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 10:19:21 -0800 Subject: [PATCH 3/6] 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 9d9d04a5..885b67d9 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 d14b359a0e65fce89ad6e7291053b171c97fef34 Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 10:31:56 -0800 Subject: [PATCH 4/6] 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 2ebf1b45..b77a33d7 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' /** @@ -167,34 +166,17 @@ export class IssuesCommand { ...(apiToken ? { apiToken } : {}), }) } 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}`) } @@ -208,42 +190,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 @@ -251,6 +211,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 0c3fa50c..bee22d4b 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 { adfToMarkdown } from './AdfMarkdownConverter.js' @@ -31,6 +32,40 @@ export class JiraIssueTracker implements IssueTracker { private readonly client: JiraApiClient private readonly config: JiraTrackerConfig + /** + * 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) { this.config = config this.client = new JiraApiClient({ From 35f056a3fa432382763b07e3efb582fd0b613a19 Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 10:36:02 -0800 Subject: [PATCH 5/6] 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 | 38 +++++++++++++++++++++++++++------- 2 files changed, 34 insertions(+), 10 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 129d9ba0..578f80e8 100644 --- a/src/lib/SettingsManager.ts +++ b/src/lib/SettingsManager.ts @@ -840,6 +840,30 @@ export type IloomSettings = z.infer */ export type IloomSettingsInput = z.input +/** + * 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 */ @@ -862,26 +886,26 @@ export class SettingsManager { // Load global settings (lowest priority) const globalSettings = await this.loadGlobalSettingsFile() const globalSettingsPath = this.getGlobalSettingsPath() - logger.debug(`🌍 Global settings from ${globalSettingsPath}:`, JSON.stringify(globalSettings, null, 2)) + logger.debug(`🌍 Global settings from ${globalSettingsPath}:`, JSON.stringify(redactSensitiveFields(globalSettings), null, 2)) // Load base settings from settings.json const baseSettings = await this.loadSettingsFile(root, 'settings.json') const baseSettingsPath = path.join(root, '.iloom', 'settings.json') - logger.debug(`📄 Base settings from ${baseSettingsPath}:`, JSON.stringify(baseSettings, null, 2)) + logger.debug(`📄 Base settings from ${baseSettingsPath}:`, JSON.stringify(redactSensitiveFields(baseSettings), null, 2)) // Load local overrides from settings.local.json const localSettings = await this.loadSettingsFile(root, 'settings.local.json') const localSettingsPath = path.join(root, '.iloom', 'settings.local.json') - logger.debug(`📄 Local settings from ${localSettingsPath}:`, JSON.stringify(localSettings, null, 2)) + logger.debug(`📄 Local settings from ${localSettingsPath}:`, JSON.stringify(redactSensitiveFields(localSettings), null, 2)) // Deep merge with priority: cliOverrides > localSettings > baseSettings > globalSettings let merged = this.mergeSettings(this.mergeSettings(globalSettings, baseSettings), localSettings) - logger.debug('🔄 After merging global + base + local settings:', JSON.stringify(merged, null, 2)) + logger.debug('🔄 After merging global + base + local settings:', JSON.stringify(redactSensitiveFields(merged), null, 2)) if (cliOverrides && Object.keys(cliOverrides).length > 0) { - logger.debug('⚙️ CLI overrides to apply:', JSON.stringify(cliOverrides, null, 2)) + logger.debug('⚙️ CLI overrides to apply:', JSON.stringify(redactSensitiveFields(cliOverrides), null, 2)) merged = this.mergeSettings(merged, cliOverrides) - logger.debug('🔄 After applying CLI overrides:', JSON.stringify(merged, null, 2)) + logger.debug('🔄 After applying CLI overrides:', JSON.stringify(redactSensitiveFields(merged), null, 2)) } // Validate merged result @@ -910,7 +934,7 @@ export class SettingsManager { * Log the final merged configuration for debugging */ private logFinalConfiguration(settings: IloomSettings): void { - logger.debug('📋 Final merged configuration:', JSON.stringify(settings, null, 2)) + logger.debug('📋 Final merged configuration:', JSON.stringify(redactSensitiveFields(settings), null, 2)) } /** From 393621d7a4932aad8f95e695f086be371485499f Mon Sep 17 00:00:00 2001 From: Noah Cardoza Date: Tue, 17 Feb 2026 11:54:27 -0800 Subject: [PATCH 6/6] 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/commands/finish.ts | 5 +- src/commands/issues.ts | 2 +- src/lib/ProviderCoordinator.ts | 165 ------------------ src/lib/SettingsManager.test.ts | 85 ++++++++- src/lib/SettingsManager.ts | 4 +- .../providers/bitbucket/BitBucketApiClient.ts | 5 +- .../bitbucket/BitBucketVCSProvider.test.ts | 101 ++++++++++- .../bitbucket/BitBucketVCSProvider.ts | 9 + src/utils/remote.test.ts | 68 ++++++++ 9 files changed, 269 insertions(+), 175 deletions(-) delete mode 100644 src/lib/ProviderCoordinator.ts diff --git a/src/commands/finish.ts b/src/commands/finish.ts index 8140f254..a07aaacb 100644 --- a/src/commands/finish.ts +++ b/src/commands/finish.ts @@ -1175,9 +1175,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 b77a33d7..adb9de87 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' @@ -193,6 +192,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 ad3aafdb..e39ce990 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 @@ -3336,4 +3336,87 @@ const error: { code?: string; message: string } = { expect(result.git?.commitTimeout).toBe(600000) }) }) + + 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 578f80e8..b32cd4a6 100644 --- a/src/lib/SettingsManager.ts +++ b/src/lib/SettingsManager.ts @@ -454,7 +454,7 @@ export const IloomSettingsSchema = z.object({ .describe( 'Auto-commit and push after code review in draft PR mode. Defaults to true when mode is github-draft-pr.' ), - 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)'), @@ -849,7 +849,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)',