PR workflows should populate both issueUrls and prUrls

[acreeger]

Problem

The current implementation of issueUrls and prUrls in metadata has a flaw:

• Issue workflows: populate issueUrls, leave prUrls empty ✓
• PR workflows: populate prUrls, leave issueUrls empty ✗

This is incorrect because PRs typically originate from issues. A PR workflow (e.g., branch issue-42__feature with PR #123) should populate both:

• issueUrls: { "42": "https://github.com/org/repo/issues/42" }
• prUrls: { "123": "https://github.com/org/repo/pull/123" }

Current Behavior

In LoomManager.ts, the metadata is built like this:

// For issue workflows
issueUrls: input.type === 'issue' ? { [String(input.identifier)]: issueData?.url ?? '' } : {},
prUrls: input.type === 'pr' ? { [String(input.identifier)]: issueData?.url ?? '' } : {},

Expected Behavior

For PR workflows:

1. Extract issue number(s) from branch name (already done via issue_numbers array)
2. Fetch or construct the issue URL(s)
3. Populate both issueUrls AND prUrls

Considerations

• The issue URL could be constructed from the issue number + repo URL pattern
• Alternatively, an additional API call could fetch the issue data
• The issue_numbers array is already populated for PR workflows, so the issue numbers are available

Related

Introduced in #342

[acreeger]

Complexity Assessment

Classification: SIMPLE

Metrics:

• Estimated files affected: 2
• Estimated lines of code: 45
• Breaking changes: No
• Database migrations: No
• Cross-cutting changes: No
• File architecture quality: Good
• Overall risk level: Low

Reasoning: The fix involves extracting issue numbers from PR branch names (already available via extractIssueNumber() utility) and populating both issueUrls and prUrls in metadata at 2 locations in LoomManager.ts. The implementation pattern is straightforward: call extractIssueNumber() on the branch name for PR workflows and populate the issueUrls object. Existing tests provide clear expectations. No breaking changes, no architectural concerns.

---

Detailed Analysis

Assumption Table

Question	Answer	Justification
Can we extract issue number from PR branch?	Yes	extractIssueNumber() utility exists and has tests showing it handles patterns like issue-42__feature
Should issue URL come from fetched PR data?	Yes	PR data fetch already happens; issueData?.url is available for PRs
Are there existing tests to update?	Yes	PR test at line 213-252 exists but doesn't verify issueUrls
Do both code paths need fixing?	Yes	createIloom() (lines 337-352) AND reuseIloom() (lines 1112-1124) both build metadata
Is this a breaking change?	No	Only adds data to existing optional metadata fields

Implementation Scope

File 1: /Users/adam/Documents/Projects/iloom-cli/feat-issue-349__pr-urls-population/src/lib/LoomManager.ts

Location A (lines 337-352):

• Current: issue_numbers = [] for PR workflows
• Change: Extract issue number from branch name for PRs
• Current: issueUrls = {} for PR workflows
• Change: Populate issueUrls with extracted issue number

Location B (lines 1112-1124):

• Same pattern in reuseIloom() method
• Same fix needed

File 2: Tests (LoomManager.test.ts)

• Update PR test case (line 213+) to verify both issueUrls and prUrls populated
• ~15-25 lines of test additions

Code Changes Estimate

• 10-15 lines: Add extractIssueNumber() logic for PR workflows
• 10-15 lines: Update test assertions
• 5-10 lines: Handle null/undefined edge cases

Total: ~30-40 LOC modified/added

[acreeger]

Analysis Phase (COMPLEX)

• Fetch and understand issue details
• Research problem space and architectural context
• Analyze LoomManager.ts metadata construction (createIloom)
• Analyze LoomManager.ts metadata construction (reuseIloom)
• Trace data flow for issue_numbers extraction
• Map all affected interfaces and types
• Review existing tests and patterns
• Document findings with complete technical reference

---

Executive Summary

PR workflows currently populate prUrls but leave issueUrls empty, despite PR branches often containing linked issue numbers (e.g., issue-42__feature). The fix requires extracting issue numbers from PR branch names using the existing extractIssueNumber() utility and constructing issue URLs from the PR URL pattern.

Questions and Key Decisions

Question	Answer
How to construct issue URL for PR workflows?	Replace /pull/N with /issues/M in the PR URL - the repository base URL is the same
Should we make an API call to fetch issue data?	No - URL construction is sufficient and avoids extra API calls
What if PR branch has no issue number?	Handle gracefully - only populate issueUrls when extractIssueNumber() returns non-null

Impact Summary

• 1 file requires modification (LoomManager.ts)
• 2 code locations within that file (createIloom and reuseIloom)
• Test updates needed in LoomManager.test.ts for PR workflow assertions
• No breaking changes - only adds data to existing optional metadata fields

---

Complete Technical Reference (click to expand for implementation details)

Problem Space Research

Problem Understanding

PRs typically originate from issues. A PR workflow (branch issue-42__feature with PR #123) should populate both issueUrls with the linked issue AND prUrls with the PR itself. Currently only prUrls is populated.

Architectural Context

• extractIssueNumber() utility already exists and handles branch name patterns like issue-42__feature
• PR URL pattern: https://github.com/owner/repo/pull/123
• Issue URL pattern: https://github.com/owner/repo/issues/42
• URL construction: Replace /pull/ segment with /issues/ and change the number

Edge Cases Identified

• PR branch name has no issue number pattern: Leave issueUrls empty
• PR branch has alphanumeric issue ID (e.g., Linear ENG-123): Handle as string, URL construction still works
• Multiple issue numbers in branch name: Only first match is returned by extractIssueNumber()

Codebase Research Findings

Affected Area: LoomManager.ts metadata construction

Location A - createIloom() (lines 336-352):

Current:
const issue_numbers: string[] = input.type === 'issue' ? [String(input.identifier)] : []
const issueUrls: Record<string, string> = input.type === 'issue' && issueData?.url
  ? { [String(input.identifier)]: issueData.url }
  : {}

Location B - reuseIloom() (lines 1111-1124):

Current:
const issue_numbers: string[] = input.type === 'issue' ? [String(input.identifier)] : []
const issueUrls: Record<string, string> = input.type === 'issue' && issueData?.url
  ? { [String(input.identifier)]: issueData.url }
  : {}

Data Flow Analysis

extractIssueNumber utility (/src/utils/git.ts:164-191):

• Handles patterns: issue-{id}__, issue-{N}-, issue_{N}, {N}-
• Returns string | null (supports alphanumeric IDs like Linear)
• Already imported in LoomManager.ts (line 13)

PR Data Available (issueData when input.type === 'pr'):

• Contains url field: https://github.com/owner/repo/pull/123
• Contains branch field: The PR's source branch (e.g., issue-42__feature)

URL Construction Pattern:

• Issue URL: prUrl.replace(/\/pull\/\d+/, '/issues/' + issueNumber)
• Example: https://github.com/owner/repo/pull/123 -> https://github.com/owner/repo/issues/42

Similar Patterns Found

• /src/lib/LoomManager.ts:499 - Uses extractIssueNumber() in checkAndWarnChildLooms()
• /src/lib/LoomManager.ts:934 - Uses extractIssueNumber() in mapWorktreesToLooms()

Affected Files

• /src/lib/LoomManager.ts:336-352 - createIloom() metadata construction
• /src/lib/LoomManager.ts:1111-1124 - reuseIloom() metadata construction
• /src/lib/LoomManager.test.ts:213-252 - PR workflow test (currently doesn't verify issueUrls)

Medium Severity Risks

• Pattern mismatch: If project uses non-standard branch naming that extractIssueNumber() doesn't recognize, issue linking silently fails (acceptable - graceful degradation)

[acreeger]

Implementation Plan for Issue #349

Summary

PR workflows currently populate prUrls but leave issueUrls empty. The fix requires extracting issue numbers from PR branch names using the existing extractIssueNumber() utility (already imported in LoomManager.ts) and constructing issue URLs by replacing /pull/N with /issues/M in the PR URL. Two code locations need modification: createIloom() and reuseIloom().

Questions and Key Decisions

Question	Answer	Rationale
How to construct issue URL for PR workflows?	Replace /pull/N with /issues/M in the PR URL	Repository base URL is identical; avoids extra API call
What if PR branch has no issue number pattern?	Leave issueUrls empty	Graceful degradation - extractIssueNumber() returns null, we simply don't populate
Should we create a helper function for URL construction?	No	Simple inline string replacement is sufficient, used only in 2 places

High-Level Execution Phases

1. Phase 1: Write tests first - add test case verifying PR workflow populates both issueUrls and prUrls
2. Phase 2: Modify createIloom() metadata construction to populate issueUrls for PR workflows
3. Phase 3: Modify reuseIloom() metadata construction with identical logic
4. Phase 4: Build and verify compilation

Quick Stats

• 0 files for deletion
• 2 files to modify (LoomManager.ts, LoomManager.test.ts)
• 0 new files to create
• Dependencies: None
• Estimated complexity: Simple

---

Complete Implementation Guide (click to expand for step-by-step details)

Automated Test Cases to Create

Test File: /Users/adam/Documents/Projects/iloom-cli/feat-issue-349__pr-urls-population/src/lib/LoomManager.test.ts (MODIFY)

Purpose: Verify PR workflow populates both issueUrls and prUrls when branch name contains issue number

Add new test case after the existing PR test (around line 252):

Click to expand complete test structure (25 lines)

it('should populate both issueUrls and prUrls for PR with issue branch', async () => {
  const prInput: CreateLoomInput = {
    type: 'pr',
    identifier: 456,
    originalInput: 'pr/456',
  }

  // Mock GitHub PR fetch with branch containing issue number
  vi.mocked(mockGitHub.fetchPR).mockResolvedValue({
    number: 456,
    title: 'Test PR',
    body: 'Test PR description',
    state: 'open',
    branch: 'issue-42__feature-branch',  // Branch contains issue-42
    baseBranch: 'main',
    url: 'https://github.com/owner/repo/pull/456',
    isDraft: false,
  })

  // ... standard mocks for worktree, environment, claude ...

  const result = await manager.createIloom(prInput)

  // Verify writeMetadata was called with both URLs populated
  expect(mockWriteMetadata).toHaveBeenCalled()
  const metadataInput = mockWriteMetadata.mock.calls[0][1]
  
  expect(metadataInput.issueUrls).toEqual({ '42': 'https://github.com/owner/repo/issues/42' })
  expect(metadataInput.prUrls).toEqual({ '456': 'https://github.com/owner/repo/pull/456' })
  expect(metadataInput.issue_numbers).toEqual(['42'])
  expect(metadataInput.pr_numbers).toEqual(['456'])
})

Files to Modify

1. /Users/adam/Documents/Projects/iloom-cli/feat-issue-349__pr-urls-population/src/lib/LoomManager.ts:336-352

Change: Modify createIloom() metadata construction to extract issue number from PR branch and populate both issueUrls and prUrls

Current code (lines 336-352):

// Build issue/pr numbers arrays based on type
const issue_numbers: string[] = input.type === 'issue' ? [String(input.identifier)] : []
const pr_numbers: string[] = input.type === 'pr' ? [String(input.identifier)] : []

// ... sessionId generation ...

// Build issueUrls/prUrls based on workflow type
const issueUrls: Record<string, string> = input.type === 'issue' && issueData?.url
  ? { [String(input.identifier)]: issueData.url }
  : {}

Click to expand replacement pseudocode (20 lines)

// Build issue/pr numbers arrays based on type
// For PR workflows, extract issue number from branch name if present
let issue_numbers: string[] = []
if (input.type === 'issue') {
  issue_numbers = [String(input.identifier)]
} else if (input.type === 'pr') {
  const extractedIssue = extractIssueNumber(branchName)
  if (extractedIssue) {
    issue_numbers = [extractedIssue]
  }
}
const pr_numbers: string[] = input.type === 'pr' ? [String(input.identifier)] : []

// ... sessionId generation (unchanged) ...

// Build issueUrls/prUrls based on workflow type
// For PR workflows, construct issue URL by replacing /pull/N with /issues/M
let issueUrls: Record<string, string> = {}
if (input.type === 'issue' && issueData?.url) {
  issueUrls = { [String(input.identifier)]: issueData.url }
} else if (input.type === 'pr' && issue_numbers.length > 0 && issueData?.url) {
  // Construct issue URL from PR URL: replace /pull/456 with /issues/42
  const issueUrl = issueData.url.replace(`/pull/${input.identifier}`, `/issues/${issue_numbers[0]}`)
  issueUrls = { [issue_numbers[0]]: issueUrl }
}

2. /Users/adam/Documents/Projects/iloom-cli/feat-issue-349__pr-urls-population/src/lib/LoomManager.ts:1111-1124

Change: Apply identical logic to reuseIloom() metadata construction

Current code (lines 1111-1124):

// Build issue/pr numbers arrays based on type
const issue_numbers: string[] = input.type === 'issue' ? [String(input.identifier)] : []
const pr_numbers: string[] = input.type === 'pr' ? [String(input.identifier)] : []

// ... sessionId generation ...

// Build issueUrls/prUrls based on workflow type
const issueUrls: Record<string, string> = input.type === 'issue' && issueData?.url
  ? { [String(input.identifier)]: issueData.url }
  : {}
const prUrls: Record<string, string> = input.type === 'pr' && issueData?.url
  ? { [String(input.identifier)]: issueData.url }
  : {}

Click to expand replacement pseudocode (20 lines)

// Build issue/pr numbers arrays based on type
// For PR workflows, extract issue number from branch name if present
let issue_numbers: string[] = []
if (input.type === 'issue') {
  issue_numbers = [String(input.identifier)]
} else if (input.type === 'pr') {
  const extractedIssue = extractIssueNumber(branchName)
  if (extractedIssue) {
    issue_numbers = [extractedIssue]
  }
}
const pr_numbers: string[] = input.type === 'pr' ? [String(input.identifier)] : []

// ... sessionId generation (unchanged) ...

// Build issueUrls/prUrls based on workflow type
// For PR workflows, construct issue URL by replacing /pull/N with /issues/M
let issueUrls: Record<string, string> = {}
if (input.type === 'issue' && issueData?.url) {
  issueUrls = { [String(input.identifier)]: issueData.url }
} else if (input.type === 'pr' && issue_numbers.length > 0 && issueData?.url) {
  const issueUrl = issueData.url.replace(`/pull/${input.identifier}`, `/issues/${issue_numbers[0]}`)
  issueUrls = { [issue_numbers[0]]: issueUrl }
}

const prUrls: Record<string, string> = input.type === 'pr' && issueData?.url
  ? { [String(input.identifier)]: issueData.url }
  : {}

3. /Users/adam/Documents/Projects/iloom-cli/feat-issue-349__pr-urls-population/src/lib/LoomManager.test.ts:213-252

Change: Add assertions to existing PR test to verify both URLs populated, or add new dedicated test

Detailed Execution Order

Phase 1: Write Tests First

1. Add test case to LoomManager.test.ts after line 252 for PR workflow with issue branch -> Verify: test fails (TDD)

Phase 2: Modify createIloom()

1. Modify lines 337-346 in LoomManager.ts to extract issue number for PR workflows -> Verify: extractIssueNumber(branchName) called
2. Modify issueUrls construction to populate for PR workflows when issue extracted -> Verify: test passes

Phase 3: Modify reuseIloom()

1. Apply identical changes to lines 1111-1124 in LoomManager.ts -> Verify: compilation succeeds

Phase 4: Build and Verify

1. Run pnpm build -> Verify: no TypeScript errors
2. Run pnpm test -> Verify: all tests pass including new test

NOTE: Follow TDD approach per CLAUDE.md - write test first, then implement.

Dependencies and Configuration

None

[acreeger]

Implementation Complete - Issue #349

Summary

PR workflows now populate both issueUrls and prUrls in loom metadata. When a PR's source branch contains an issue number pattern (e.g., issue-42__feature), the issue URL is constructed by replacing /pull/N with /issues/M in the PR URL.

Changes Made

• LoomManager.ts: Modified createIloom() to extract issue number from PR branch and populate issueUrls
• LoomManager.ts: Applied identical logic to reuseIloom() for consistency
• LoomManager.test.ts: Added test verifying both issueUrls and prUrls are populated for PR workflows
• LoomManager-database.test.ts: Updated mock to include extractIssueNumber function

Validation Results

• Tests: 3014 passed / 3015 total (1 skipped)
• Typecheck: Passed
• Lint: Passed
• Build: Passed

---

Detailed Changes by File (click to expand)

Files Modified

/src/lib/LoomManager.ts

Changes: Extract issue number from PR branch name and populate issueUrls for PR workflows

• Lines 336-361: createIloom() - Added extractedIssueNum variable to extract issue from branch name, construct issue URL
• Lines 1125-1152: reuseIloom() - Applied identical logic for reused looms

/src/lib/LoomManager.test.ts

Changes: Added test case for PR workflow with issue branch

• Line 254+: New test should populate both issueUrls and prUrls for PR with issue branch
• Verifies issueUrls, prUrls, issue_numbers, and pr_numbers are all correctly populated

/tests/lib/LoomManager-database.test.ts

Changes: Updated git.js mock to include extractIssueNumber function

• Lines 43-50: Added mock implementation matching real utility behavior

Test Coverage Added

• LoomManager.test.ts: Test verifying PR workflow with branch issue-42__feature-branch populates:
  • issueUrls: { '42': 'https://github.com/owner/repo/issues/42' }
  • prUrls: { '456': 'https://github.com/owner/repo/pull/456' }
  • issue_numbers: ['42']
  • pr_numbers: ['456']

Dependencies Added

None

[acreeger]

iloom Session Summary

Key Themes:

• Successfully implemented PR workflow enhancement to populate both issueUrls and prUrls when PR branch names contain issue numbers
• Followed COMPLEX workflow with separate analysis, planning, and implementation phases due to user's caution around PR-related code
• Comprehensive code review process identified pre-existing inconsistency in prUrls logic, filed as separate issue Inconsistent prUrls logic between createIloom and reuseIloom #368

---

Session Details (click to expand)

Key Insights

• The existing extractIssueNumber() utility made implementation straightforward - no new parsing logic needed
• Issue URLs can be constructed from PR URLs via simple string replacement (/pull/N → /issues/M) since they share the same repository base URL
• PR workflows typically originate from issue branches (e.g., issue-42__feature), making automatic issue URL population valuable for cross-referencing
• Code review revealed a pre-existing inconsistency between createIloom and reuseIloom methods in how they handle draft PR URLs

Decisions Made

• URL Construction Method: Used string replacement instead of additional API calls to construct issue URLs from PR URLs - more efficient and avoids rate limiting
• Null Handling: Only populate issueUrls when extractIssueNumber() successfully extracts an issue number - gracefully handles PR branches without issue numbers
• Code Duplication: Accepted identical logic in both createIloom() and reuseIloom() methods rather than extracting to helper - maintains code locality and readability for these two critical paths
• Complexity Reclassification: User reclassified from SIMPLE to COMPLEX workflow due to caution around PR-related code changes, resulting in separate analysis and planning phases

Challenges Resolved

• Test Mocking: Added extractIssueNumber mock to LoomManager-database.test.ts to support new functionality - mock implements same regex logic as real utility for test compatibility
• Validation Coverage: All tests (3014), typecheck, lint, and build passed on first attempt - thorough planning phase prevented implementation issues
• Code Review Findings: Multi-agent code review (5 parallel agents) surfaced pre-existing prUrls inconsistency between createIloom/reuseIloom - filed as separate issue Inconsistent prUrls logic between createIloom and reuseIloom #368 rather than blocking current PR

Lessons Learned

• COMPLEX workflow's separate phases provided valuable checkpoints and caught potential issues early through systematic analysis
• Automated code review with parallel agents (CLAUDE.md compliance, bug scan, git history, previous PRs, code comments) provides comprehensive coverage but may flag pre-existing issues - scoring phase (0-100 confidence) effectively filters false positives
• String manipulation for URL construction is simple and effective when URLs follow predictable patterns (GitHub's /pull/ vs /issues/ structure)
• When modifying metadata population logic, verify both creation and reuse code paths maintain consistency

---

Generated with 🤖❤️ by iloom.ai