Update core commands to use `formatIssueId`

[NoahCardoza]

Summary

Migrate commands that have IssueTracker access via dependency injection to use formatIssueId instead of hardcoded # prefixes.

Context

Part of Epic #595. These commands already have an IssueTracker instance available, so they can call this.issueTracker.formatIssueId(id) directly.

Files to Modify

• src/commands/start.ts — Replace #${parsed.number} with this.issueTracker.formatIssueId(parsed.number) (~4 sites)
• src/commands/finish.ts — Same pattern (~3 sites, plus sites using issuePrefix from MCP provider)
• src/commands/enhance.ts — Same (~2 sites)
• src/lib/IssueEnhancementService.ts — Same (~2 sites, has injected IssueTracker)
• src/lib/LoomLauncher.ts — Replace Issue #${identifier} formatting (~2 sites, needs provider access added or use static utility)

Acceptance Criteria

• All hardcoded # prefixes in the listed files are replaced with formatIssueId calls
• Test files updated to expect provider-formatted output
• For GitHub-provider tests, expected output remains unchanged (still #123)
• At least one Jira or Linear test case added where practical
• pnpm build passes
• pnpm test passes

Scope Boundaries

• Do NOT change PR references (PR #X) — PRs are GitHub-only
• Do NOT change debug-level messages
• Do NOT change files covered by Issues 3, 4, or 5

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 codebase already has the formatIssueId method on the IssueTracker interface (from issue #596). GitHubService returns #N format, LinearService returns uppercased identifiers like ENG-123. All five target files have hardcoded # prefixes in user-facing messages that need migration. PR references (PR #X) must be left unchanged as they are GitHub-only. Debug-level messages should also be left unchanged per scope boundaries.

Full Analysis

Files to Modify and Sites Found

src/commands/start.ts (4 sites):

• Line 169: issue #${parentLoom.identifier} - parent loom display
• Line 220: Created issue #${result.number} - issue creation success
• Line 526: Issue #${parsed.number} in formatParsedInput()
• Note: Line 466 (Validated PR #) and Line 477 (Validated issue #) are debug messages - skip per scope

src/commands/finish.ts (3 sites):

• Line 464: Issue #${parsed.number} is closed - error message
• Line 584: Issue #${parsed.number}${autoLabel} in formatParsedInput()
• Note: Lines 379, 404 are debug messages - skip. All PR # references - skip (GitHub-only). Lines using issuePrefix from MCP provider are in the commit workflow and should be reviewed.

src/commands/enhance.ts (2 sites):

• Line 78: Fetching issue #${issueNumber}...
• Line 112: Issue #${issueNumber} enhanced successfully!

src/lib/IssueEnhancementService.ts (2 sites):

• Line 163: skipping interactive prompts for issue #${issueNumber}
• Line 171: Created issue #${issueNumber}.

src/lib/LoomLauncher.ts (2 sites):

• Line 337: Issue #${identifier} in formatIdentifier()
• Note: Line 339 (PR #${identifier}) - skip (GitHub-only)
• Challenge: LoomLauncher doesn't have an IssueTracker injected. Options: add IssueTracker dependency, use IssueTrackerFactory.formatIssueId() static method, or pass formatted string from caller.

Key Patterns

• IssueTracker.formatIssueId() is available on injected instances in start.ts, finish.ts, enhance.ts, and IssueEnhancementService.ts
• IssueTrackerFactory.formatIssueId(providerType, id) is a static utility for files without an IssueTracker instance (like LoomLauncher)
• For LoomLauncher, the cleanest approach is to accept a pre-formatted identifier or use the static factory method with provider type info

[acreeger]

Implementation Plan

Replace all hardcoded # issue prefixes in the five target files with formatIssueId calls. For files with an injected IssueTracker, use this.issueTracker.formatIssueId(). For LoomLauncher, use the static IssueTrackerFactory.formatIssueId() method with provider type passed via the LaunchLoomOptions interface.

Full Plan

Step 1: Update src/commands/start.ts

Sites to change (4):

1. Line 169: issue #${parentLoom.identifier} -> issue ${this.issueTracker.formatIssueId(parentLoom.identifier)}
2. Line 220: Created issue #${result.number} -> Created issue ${this.issueTracker.formatIssueId(result.number)}
3. Line 526: Issue #${parsed.number} -> Issue ${this.issueTracker.formatIssueId(parsed.number!)}
4. Line 411: Could not find issue or PR #${number} -> This uses # but is an error for numeric input detection, not a formatted display. Since this is about "issue or PR" with a numeric input (always GitHub-style), this should stay as-is OR use formatIssueId. Decision: Change it since it's user-facing.

Skip: Lines 466, 477 (debug messages), Line 524 (PR # - GitHub-only)

Step 2: Update src/commands/finish.ts

Sites to change (3):

1. Line 464: Issue #${parsed.number} is closed -> Issue ${this.issueTracker.formatIssueId(parsed.number!)} is closed
2. Line 584: Issue #${parsed.number}${autoLabel} -> Issue ${this.issueTracker.formatIssueId(parsed.number!)}${autoLabel}

Skip: Lines 365, 379, 404 (debug), all PR # references (GitHub-only), lines using issuePrefix from MCP provider (different concern)

Step 3: Update src/commands/enhance.ts

Sites to change (2):

1. Line 78: Fetching issue #${issueNumber}... -> Fetching issue ${this.issueTracker.formatIssueId(issueNumber)}...
2. Line 112: Issue #${issueNumber} enhanced -> Issue ${this.issueTracker.formatIssueId(issueNumber)} enhanced

Step 4: Update src/lib/IssueEnhancementService.ts

Sites to change (2):

1. Line 163: issue #${issueNumber} -> issue ${this.issueTrackerService.formatIssueId(issueNumber)}
2. Line 171: Created issue #${issueNumber}. -> Created issue ${this.issueTrackerService.formatIssueId(issueNumber)}.

Step 5: Update src/lib/LoomLauncher.ts

LoomLauncher lacks an IssueTracker instance. Two approaches:

• Option A: Add issueTrackerProvider to LaunchLoomOptions and use IssueTrackerFactory.formatIssueId()
• Option B: Add pre-formatted formattedIdentifier to LaunchLoomOptions

Decision: Option A - cleaner, follows existing patterns, reusable.

1. Add issueTrackerProvider?: string to LaunchLoomOptions
2. Import IssueTrackerFactory and its types
3. Line 337: Issue #${identifier} -> use IssueTrackerFactory.formatIssueId(provider, identifier)
4. Update callers to pass issueTrackerProvider

Step 6: Update test files

• src/commands/start.test.ts - Update expected output strings to match new format
• src/commands/finish.test.ts - Same
• src/commands/enhance.test.ts - Same
• src/lib/IssueEnhancementService.test.ts - Same
• src/lib/LoomLauncher.test.ts - Same, add Linear test case
• Add at least one Linear test case showing non-# prefix

Step 7: Build and test

pnpm build
pnpm test

[acreeger]

Plan Review (Gemini)

Verdict: approved

Gemini reviewed the plan and found it comprehensive. Key observations: Steps 1-4 are straightforward replacements, Step 5 (LoomLauncher) has higher complexity due to interface changes. The plan correctly preserves PR references as GitHub-only and skips debug messages.

Full Review

Gemini highlighted:

• The plan is well-scoped with clear boundaries
• Step 5 (LoomLauncher) is the highest-risk change due to API/interface modification
• Test updates are necessary for all changed files
• The distinction between PRs (GitHub-only) and issues (multi-provider) is correctly maintained
• No security concerns identified

[acreeger]

Implementation Complete

All five target files updated to use formatIssueId instead of hardcoded # prefixes. For files with injected IssueTracker (start.ts, finish.ts, enhance.ts, IssueEnhancementService.ts), used this.issueTracker.formatIssueId(). For LoomLauncher (no IssueTracker instance), added issueTrackerProvider to LaunchLoomOptions and used IssueTrackerFactory.formatIssueId() static method. All test files updated with proper mock setup, and added Linear-specific test cases for LoomLauncher.

Full Implementation Log

Source file changes:

• src/commands/start.ts: 4 sites updated (parent loom display, issue creation success, formatParsedInput, error message)
• src/commands/finish.ts: 2 sites updated (closed issue error, formatParsedInput)
• src/commands/enhance.ts: 2 sites updated (fetching message, success message)
• src/lib/IssueEnhancementService.ts: 2 sites updated (non-interactive skip message, created issue message)
• src/lib/LoomLauncher.ts: Added issueTrackerProvider to LaunchLoomOptions, imported IssueTrackerFactory, updated formatIdentifier to use provider-aware formatting, updated 3 call sites
• src/lib/LoomManager.ts: Updated 2 launchLoom() call sites to pass issueTrackerProvider

Test file changes:

• src/commands/start.test.ts: Added formatIssueId mock to GitHub and Linear mock services
• src/commands/finish.test.ts: Added formatIssueId mock to GitHub service
• src/commands/finish.pr-workflow.test.ts: Added formatIssueId mock to all 7 GitHub service mock locations
• src/commands/enhance.test.ts: Added formatIssueId to mock GitHubService
• src/lib/IssueEnhancementService.test.ts: Added formatIssueId mock
• src/lib/LoomLauncher.test.ts: Added 3 provider-aware formatting test cases (GitHub, Linear, default)

Results:

• pnpm build: PASS
• pnpm test: 3838 passed, 23 skipped, 0 failed

[acreeger]

Code Review (Gemini)

Result: clean (for this issue's scope)

Gemini found no bugs in the formatIssueId migration itself. All hardcoded # replacements are correct. The review raised several pre-existing concerns in FinishCommand and LoomManager (Linear PR fallback, metadata refresh on reuse) but these are outside the scope of this issue.

Full Code Review

Scope-relevant findings: None. All formatIssueId replacements are correct.

Pre-existing issues flagged (out of scope):

• FinishCommand lacks GitHubService fallback for Linear PR workflows (pre-existing)
• IssueEnhancementService URL regex is GitHub-specific (pre-existing)
• LoomManager session/metadata refresh on reuse (pre-existing)
• LoomLauncher defaults to 'github' when provider not specified (correct behavior - github is the default provider)

[acreeger]

Implementation complete. Changes committed with fixes #598.

[acreeger]

Implemented on branch epic/595