Update settings-access commands and lib files to use `formatIssueId`

[NoahCardoza]

Summary

Migrate files that use issuePrefix from MCP providers or only have settings access to use IssueTrackerFactory.formatIssueId() static utility.

Context

Part of Epic #595. These files don't have a direct IssueTracker instance but do have access to settings or provider type, so they use the static utility.

Files to Modify

• src/commands/commit.ts — Replace IssueManagementProviderFactory.create(providerType, settings).issuePrefix with IssueTrackerFactory.formatIssueId(providerType, id) (~2 sites)
• src/commands/plan.ts — Replace manual prefix logic (const issuePrefix = provider === 'github' ? '#' : '') with IssueTrackerFactory.formatIssueId() (~3 sites including formatChildIssues and formatDependencies)
• src/lib/PRManager.ts — Replace this.issuePrefix getter pattern (~5 sites)
• src/lib/SessionSummaryService.ts — Replace hardcoded # (~2 sites)
• src/lib/LoomManager.ts — Replace prManager.issuePrefix usage in PR body formatting (~1 site)

Acceptance Criteria

• All issuePrefix usage in listed files replaced with IssueTrackerFactory.formatIssueId() or equivalent
• No more manual provider === 'github' ? '#' : '' logic in these files
• Test files updated to expect provider-formatted output
• pnpm build passes
• pnpm test passes

Scope Boundaries

• Do NOT deprecate IssueManagementProvider.issuePrefix — keep for backward compat
• Do NOT change files covered by other child issues

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 five files to modify all use issuePrefix patterns to format issue identifiers for display. commit.ts creates an IssueManagementProviderFactory instance solely to get issuePrefix. plan.ts manually checks provider === 'github' ? '#' : ''. PRManager.ts has a getter that does the same factory instantiation pattern, used in ~5 places for PR body/prompt formatting. SessionSummaryService.ts creates an MCP provider just to post comments (doesn't use issuePrefix directly but the pattern is there). LoomManager.ts uses prManager.issuePrefix in draft PR body construction. The IssueTrackerFactory.formatIssueId() static method from issue #596 is already available, taking (providerType, identifier) and returning formatted strings like #123 for GitHub or ENG-123 for Linear.

Full Analysis

Files to Modify

1. src/commands/commit.ts (lines 7, 137, 149, 158):

   • Imports IssueManagementProviderFactory
   • Line 137: const issuePrefix = IssueManagementProviderFactory.create(providerType).issuePrefix
   • Line 149: commitMessage = \WIP commit for Issue ${issuePrefix}${detected.issueNumber}``
   • Line 158: issuePrefix passed to commitOptions
   • Replace with: IssueTrackerFactory.formatIssueId(providerType, detected.issueNumber) for the WIP message, and need to consider what commitOptions.issuePrefix is used for downstream

2. src/commands/plan.ts (lines 190, 25-29, 35-53, 383, 386):

   • Line 190: const issuePrefix = provider === 'github' ? '#' : ''
   • Lines 25-29: formatChildIssues() accepts issuePrefix: string and uses ${issuePrefix}${child.id}
   • Lines 35-53: formatDependencies() accepts issuePrefix: string and uses ${issuePrefix}${dep.id}
   • Lines 383, 386: Pass issuePrefix to formatChildIssues/formatDependencies
   • Replace with: Use IssueTrackerFactory.formatIssueId(provider, child.id) inside the format functions

3. src/lib/PRManager.ts (lines 28-32, 94, 107-108, 112-113, 119, 126):

   • Lines 28-32: get issuePrefix() getter creates MCP provider to get prefix
   • Line 94: Fixes ${this.issuePrefix}${issueNumber} in fallback body
   • Lines 107-108: ${this.issuePrefix}${issueNumber} in prompt context
   • Lines 112-113, 119: ${this.issuePrefix}${issueNumber} in format/examples
   • Line 126: ${examplePrefix}42 in examples
   • Replace with: IssueTrackerFactory.formatIssueId(providerType, issueNumber) calls

4. src/lib/SessionSummaryService.ts (lines 20, 411):

   • Line 20: Imports IssueManagementProviderFactory
   • Line 411: Creates MCP provider to post comments - no actual issuePrefix usage here
   • Actually no issuePrefix changes needed in this file based on closer inspection - it only uses provider for createComment()

5. src/lib/LoomManager.ts (line 282):

   • Line 282: Fixes ${prManager.issuePrefix}${input.identifier}
   • Replace with: Fixes ${IssueTrackerFactory.formatIssueId(providerType, input.identifier)}

Key Considerations

• CommitOptions.issuePrefix is used by CommitManager downstream - need to check if that needs updating or if it's out of scope
• PRManager.issuePrefix getter is used by both PRManager internally and LoomManager externally
• The formatChildIssues and formatDependencies functions need their signatures changed since they no longer receive a prefix string but need to call formatIssueId per item
• SessionSummaryService doesn't actually use issuePrefix - it uses the MCP provider for createComment(), which is a different concern

[acreeger]

Implementation Plan

Replace all issuePrefix usage with IssueTrackerFactory.formatIssueId() across 5 files, removing dependencies on IssueManagementProviderFactory for prefix extraction. The formatIssueId() static method returns the fully formatted issue ID (e.g., #123 for GitHub, ENG-123 for Linear), eliminating manual prefix logic.

Full Plan

1. src/commands/commit.ts

• Replace import: IssueManagementProviderFactory → IssueTrackerFactory
• Line 137: Replace IssueManagementProviderFactory.create(providerType).issuePrefix with IssueTrackerFactory.formatIssueId(providerType, 0).replace('0', '') to derive prefix for commitOptions.issuePrefix (downstream CommitManager contract)
• Line 149: Replace WIP commit for Issue ${issuePrefix}${detected.issueNumber} with WIP commit for Issue ${IssueTrackerFactory.formatIssueId(providerType, detected.issueNumber)}
• Update test file to mock IssueTrackerFactory instead of IssueManagementProviderFactory

2. src/commands/plan.ts

• Remove manual const issuePrefix = provider === 'github' ? '#' : '' logic (line 190)
• Update formatChildIssues() to accept providerType instead of issuePrefix and use IssueTrackerFactory.formatIssueId(providerType, child.id)
• Update formatDependencies() similarly
• Update callers to pass provider instead of issuePrefix
• Update test mock for IssueTrackerFactory

3. src/lib/PRManager.ts

• Remove get issuePrefix() getter (lines 28-32)
• Add private helper method or inline IssueTrackerFactory.formatIssueId() calls
• Replace all ${this.issuePrefix}${issueNumber} with ${IssueTrackerFactory.formatIssueId(providerType, issueNumber)}
• Get providerType from this.settings.issueManagement?.provider ?? 'github'
• Update test file expectations

4. src/lib/SessionSummaryService.ts

• Inspection shows no actual issuePrefix usage -- it only uses IssueManagementProviderFactory for createComment(), not for prefix extraction
• No changes needed (this file was listed but doesn't actually use issuePrefix)

5. src/lib/LoomManager.ts

• Line 282: Replace Fixes ${prManager.issuePrefix}${input.identifier} with Fixes ${IssueTrackerFactory.formatIssueId(providerType, input.identifier)}
• Import IssueTrackerFactory and IssueTrackerProviderType
• Update LoomManager test mock

Test Updates

• commit.test.ts: Replace IssueManagementProviderFactory mock with IssueTrackerFactory mock
• plan.test.ts: Update IssueTrackerFactory mock to include formatIssueId
• PRManager.test.ts: Update assertions for formatted issue IDs
• LoomManager.test.ts: Replace mockIssuePrefix getter with IssueTrackerFactory.formatIssueId mock

[acreeger]

Plan Review (Gemini)

Verdict: approved

Gemini noted the formatIssueId(providerType, 0).replace('0', '') workaround for commitOptions.issuePrefix as a backward-compatibility concern. This is acceptable since CommitManager (the downstream consumer) is out of scope for this issue. The plan correctly identifies that SessionSummaryService doesn't need changes. Overall the plan is sound.

Full Review

Key findings:

• Plan correctly eliminates manual provider-specific prefix logic
• Workaround for CommitManager's issuePrefix contract is pragmatic and non-breaking
• SessionSummaryService analysis showing no changes needed demonstrates due diligence
• Test updates are comprehensive
• No security, performance, or edge case concerns identified

[acreeger]

Implementation Complete

All changes implemented across 4 source files and 4 test files. SessionSummaryService.ts did not need changes as it only uses IssueManagementProviderFactory for createComment(), not for issue prefix formatting. Build and all 3835 tests pass.

Full Implementation Log

Source Changes

1. src/commands/commit.ts: Replaced IssueManagementProviderFactory import with IssueTrackerFactory. WIP commit message now uses IssueTrackerFactory.formatIssueId(providerType, detected.issueNumber). The issuePrefix for CommitManager contract is derived via IssueTrackerFactory.formatIssueId(providerType, '').

2. src/commands/plan.ts: Removed manual provider === 'github' ? '#' : '' logic. Updated formatChildIssues() and formatDependencies() signatures to accept IssueTrackerProviderType instead of issuePrefix string, using IssueTrackerFactory.formatIssueId() internally.

3. src/lib/PRManager.ts: Removed get issuePrefix() getter that created IssueManagementProviderFactory instances. Added private get providerType() and private formatIssueId() helper methods using IssueTrackerFactory. All ${this.issuePrefix}${issueNumber} patterns replaced with this.formatIssueId(issueNumber).

4. src/lib/LoomManager.ts: Added IssueTrackerFactory import. Replaced prManager.issuePrefix usage with IssueTrackerFactory.formatIssueId(IssueTrackerFactory.getProviderName(settingsData), input.identifier).

Test Changes

• commit.test.ts: Replaced IssueManagementProviderFactory mock with IssueTrackerFactory.formatIssueId mock
• plan.test.ts: Added formatIssueId to existing IssueTrackerFactory mock
• PRManager.test.ts: Added IssueTrackerFactory mock with formatIssueId and getProviderName
• LoomManager.test.ts: Added IssueTrackerFactory mock, removed mockIssuePrefix variable and issuePrefix getter from PRManager mock

[acreeger]

Code Review (Gemini)

Result: clean - 0 issues found in the changes

Gemini identified no bugs, security issues, or quality problems with the changes introduced by this issue. All 9 findings were pre-existing concerns in the codebase unrelated to the formatIssueId migration (e.g., metadata write ordering, race conditions in PR creation, headless mode edge cases). These are out of scope for this issue.

Full Code Review

All findings were pre-existing issues not introduced by this PR:

1. Metadata persistence vs launch order in LoomManager (pre-existing)
2. Database branch reporting in reuse path (pre-existing)
3. PR creation race condition (pre-existing)
4. Commit message AI generation flow (pre-existing, handled by CommitManager)
5. PR URL regex tied to GitHub (pre-existing)
6. --fixes flag behavior for branch looms (pre-existing design decision)
7. Branch revival when worktree folder deleted (pre-existing)
8. Headless plan mode without prompt (pre-existing)
9. Draft PR push failure handling (pre-existing)

No critical, high, or medium issues found in the actual changes.

[acreeger]

Implementation complete. Changes committed with fixes #599.

[acreeger]

Implemented on branch epic/595