Fix `cleanup.ts` provider-awareness for issue ID formatting

[NoahCardoza]

Summary

cleanup.ts hardcodes GitHubService and references GitHub issue/PR #X. Fix to use IssueTrackerFactory and format IDs correctly.

Context

Part of Epic #595. The cleanup command currently assumes GitHub is the issue tracker, which produces incorrect output for Jira and Linear users.

Files to Modify

• src/commands/cleanup.ts — Load settings, use IssueTrackerFactory.formatIssueId() (~4 sites). Also replace "GitHub issue/PR" text with generic "issue/PR".

Acceptance Criteria

• cleanup.ts no longer hardcodes GitHubService for issue ID formatting
• Uses IssueTrackerFactory.formatIssueId() for all user-facing issue references
• "GitHub issue/PR" text replaced with generic "issue/PR" where appropriate
• Cleanup test expectations updated
• pnpm build passes
• pnpm test passes

Scope Boundaries

• Do NOT change PR-specific references if they're GitHub-only operations

Dependencies

• Blocked by: Add formatIssueId to IssueTracker interface and IssueTrackerFactory #596 (Add formatIssueId interface and implementations)

[acreeger]

Starting automated workflow for this issue.

[acreeger]

Codebase Analysis

The cleanup.ts command has 4 sites that hardcode GitHub-specific issue ID formatting: executeIssueCleanup method uses GitHub issue/PR #${issueNumber} in log messages at lines 465, 482, 483, 503, and 595. The IssueTrackerFactory.formatIssueId(providerType, identifier) static method is already available from the merged issue #596. The cleanup command already loads settings lazily via ensureResourceCleanup() using SettingsManager, so the provider type can be obtained from the same settings via IssueTrackerFactory.getProviderName(settings). Other files in the codebase (GitHubService, LinearService) already implement the formatIssueId instance method on the IssueTracker interface.

Full Analysis

Files to modify:

1. src/commands/cleanup.ts - Replace hardcoded GitHub issue/PR #${issueNumber} with generic issue/PR ${formattedId} using IssueTrackerFactory.formatIssueId()
2. src/commands/cleanup.test.ts - Update test expectations from GitHub issue/PR #X to issue/PR #X (or provider-formatted ID)

Hardcoded GitHub references in cleanup.ts (executeIssueCleanup method):

• Line 465: Finding worktrees related to GitHub issue/PR #${issueNumber}...
• Line 482: No worktrees found for GitHub issue/PR #${issueNumber}
• Line 483: Searched for worktree paths containing: ${issueNumber}, _pr_${issueNumber}, issue-${issueNumber}, etc.
• Line 503: Found ${targets.length} worktree(s) related to issue/PR #${issueNumber}:
• Line 595: Completed cleanup for issue/PR #${issueNumber}:

Pattern from IssueTrackerFactory:

• IssueTrackerFactory.formatIssueId(providerType, identifier) - static method, no instance needed
• IssueTrackerFactory.getProviderName(settings) - gets provider type from settings
• Settings already loaded in ensureResourceCleanup() via SettingsManager

Approach:

• Load settings and get provider type at the beginning of executeIssueCleanup
• Use IssueTrackerFactory.formatIssueId() to format the issue ID
• Replace "GitHub issue/PR" with generic "issue/PR"
• The ensureResourceCleanup already loads settings, but we need settings in executeIssueCleanup specifically, so we'll load them there or refactor to make settings accessible

[acreeger]

Implementation Plan

The change is focused: add IssueTrackerFactory import to cleanup.ts, load settings in executeIssueCleanup to get provider type, use IssueTrackerFactory.formatIssueId() for all user-facing issue references, and replace "GitHub issue/PR" with generic "issue/PR". Tests will be updated to match.

Full Plan

Step 1: Update src/commands/cleanup.ts

1. Add import for IssueTrackerFactory and IssueTrackerProviderType
2. In executeIssueCleanup():
   • Load settings using SettingsManager at the beginning
   • Get provider type via IssueTrackerFactory.getProviderName(settings)
   • Format issue ID via IssueTrackerFactory.formatIssueId(providerType, issueNumber)
   • Replace 5 hardcoded sites:
     • Line 465: Finding worktrees related to GitHub issue/PR #${issueNumber}... -> Finding worktrees related to issue/PR ${formattedId}...
     • Line 482: No worktrees found for GitHub issue/PR #${issueNumber} -> No worktrees found for issue/PR ${formattedId}
     • Line 483: Keep generic search hint as-is (implementation detail, not user-facing issue reference)
     • Line 503: Found ... related to issue/PR #${issueNumber}: -> Found ... related to issue/PR ${formattedId}:
     • Line 595: Completed cleanup for issue/PR #${issueNumber}: -> Completed cleanup for issue/PR ${formattedId}:

Step 2: Update src/commands/cleanup.test.ts

Update test expectations that check for GitHub issue/PR #X strings:

• Line 132: Finding worktrees related to GitHub issue/PR #42... -> Finding worktrees related to issue/PR #42...
• Line 140: Finding worktrees related to GitHub issue/PR #1... -> Finding worktrees related to issue/PR #1...
• Line 148: Finding worktrees related to GitHub issue/PR #999... -> Finding worktrees related to issue/PR #999...
• Line 161: Finding worktrees related to GitHub issue/PR #42... -> Finding worktrees related to issue/PR #42...
• Line 169: Finding worktrees related to GitHub issue/PR #123... -> Finding worktrees related to issue/PR #123...
• Line 177: Finding worktrees related to GitHub issue/PR #1... -> Finding worktrees related to issue/PR #1...
• Line 187: Finding worktrees related to GitHub issue/PR #0... -> Finding worktrees related to issue/PR #0...
• Line 197: Finding worktrees related to GitHub issue/PR #7... -> Finding worktrees related to issue/PR #7...
• Line 226: Finding worktrees related to GitHub issue/PR #99... -> Finding worktrees related to issue/PR #99...
• Line 423: Finding worktrees related to GitHub issue/PR #7... -> Finding worktrees related to issue/PR #7...
• Line 1032: Finding worktrees related to GitHub issue/PR #25... -> Finding worktrees related to issue/PR #25...
• Line 1060: No worktrees found for GitHub issue/PR #25 -> No worktrees found for issue/PR #25
• Line 1071: No worktrees found for GitHub issue/PR #99999 -> No worktrees found for issue/PR #99999

Step 3: Build and Test

• Run pnpm build to verify TypeScript compiles
• Run pnpm test to verify all tests pass

[acreeger]

Plan Review (Gemini)

Verdict: approved

Gemini confirmed the plan is complete and covers all acceptance criteria. No critical issues, security concerns, or missed edge cases were identified. The plan correctly maintains the PR-specific scope boundary.

Full Review

Key findings:

• Plan correctly abstracts from GitHub-specific to generic issue tracker interface
• The "Do NOT change PR-specific references" boundary is properly scoped
• Settings loading approach via SettingsManager is architecturally sound (already used in ensureResourceCleanup)
• Test expectations are identified and will be updated to match new generic text
• No security concerns raised

[acreeger]

Implementation Complete

Updated cleanup.ts to use IssueTrackerFactory.formatIssueId() for all user-facing issue references instead of hardcoding GitHub-specific formatting. Added IssueTrackerFactory import, stored settings from ensureResourceCleanup as a class field, and used IssueTrackerFactory.getProviderName() + IssueTrackerFactory.formatIssueId() in executeIssueCleanup. Replaced all "GitHub issue/PR" text with generic "issue/PR". Updated all 13 test expectations accordingly.

Full Implementation Log

Changes to src/commands/cleanup.ts:

1. Added import for IssueTrackerFactory
2. Added private settings field to CleanupCommand class
3. Stored settings in ensureResourceCleanup (already loaded there, now saved as class property)
4. In executeIssueCleanup: used IssueTrackerFactory.getProviderName(settings) and IssueTrackerFactory.formatIssueId(providerType, issueNumber) for all user-facing messages
5. Replaced 4 occurrences of GitHub issue/PR #${issueNumber} with generic issue/PR ${formattedId}
6. One occurrence (Found ... related to issue/PR #${issueNumber}) already used generic text but needed formattedId formatting

Changes to src/commands/cleanup.test.ts:

• Replaced all 13 occurrences of GitHub issue/PR # with issue/PR # in test expectations

Build & Test Results:

• pnpm build: SUCCESS
• pnpm test: 116 test files passed, 3835 tests passing

[acreeger]

Code Review (Gemini)

Result: clean - no actionable issues

Gemini raised theoretical concerns about temporal coupling and PR/issue disambiguation, but all are low severity and already handled correctly by the existing code flow.

Full Code Review

Findings evaluated:

1. Temporal coupling (this.settings) - Low: The execute() flow always calls checkForChildLooms -> ensureResourceCleanup before reaching executeIssueCleanup, so this.settings is always populated. The ?? {} fallback is defensive programming.

2. PR vs Issue formatting - Low: The existing code already combines "issue/PR" in the same text. The cleanup command operates on worktrees that could be associated with either. The scope boundary says "Do NOT change PR-specific references if they're GitHub-only operations" - this refers to PR API operations (gh pr create), not user-facing text that mentions both issues and PRs.

3. Type on issueNumber - Non-issue: issueNumber is already typed as string | number in ParsedCleanupInput, and formatIssueId accepts string | number.

4. Missing non-GitHub test - Low: Valid suggestion but outside scope. The default behavior is correct and tests cover the expected format.

No critical, high, or medium issues found. No fixes needed.

[acreeger]

Implementation complete. Changes committed with fixes #600.

[acreeger]

Implemented on branch epic/595