In github-draft-pr merge mode (added recently) the 'il finish' summary gets posted to the upstream issue not the PR

[acreeger]

The same might also happen in "PR" mode - the summary might get posted to the issue.

[acreeger]

Bug Report Analysis

Questions for Reporter

Question	Answer
When contributing to a forked repo using github-draft-pr mode, do you expect the session summary to be posted to the PR (on your fork) or the upstream issue?	Assuming PR is preferred since posting to upstream repo issues may not be welcomed by maintainers
In github-pr mode (non-draft), should the summary go to the PR or the original issue?	Assuming PR is preferred for consistency with draft mode

Problem Summary

When using il finish with github-draft-pr or github-pr merge modes, the session summary is posted to the original issue number instead of the pull request, causing summaries to appear in the wrong location.

User Impact

Users working on forks or projects requiring PR workflows see their session summaries posted to issues rather than PRs. For forked repos, this means summaries may be posted to the upstream repository's issue tracker, which could be inappropriate.

Expected Behavior

Session summaries should be posted to the pull request in both github-draft-pr and github-pr modes.

Actual Behavior

Session summaries are always posted to the original issue number (e.g., issue #25) even when a PR exists and its number is available in the workflow (e.g., PR #123).

Next Steps

• Reporter to confirm expected behavior for summary posting location
• Technical analysis to identify root cause in SessionSummaryService
• Implementation planning and execution

---

📋 Complete Context & Details (click to expand)

Reproduction Steps

1. Configure a project with mergeBehavior.mode: "github-draft-pr" in .iloom/settings.json
2. Run il start 25 (where 25 is an issue number)
3. Make code changes in the isolated workspace
4. Run il finish
5. Observe that the session summary is posted to issue Security audit and hardening #25 instead of the draft PR

The same issue occurs with mergeBehavior.mode: "github-pr":

1. Configure with github-pr mode
2. Run il start 25
3. Make changes and run il finish
4. The PR is created successfully
5. Session summary is posted to issue Security audit and hardening #25 instead of the newly created PR

Additional Context

Merge Behavior Modes (from README.md):

• local: (Default) Merge directly into main branch locally
• github-pr: Push branch and create a GitHub PR on il finish
• github-draft-pr: Create a draft PR immediately on il start, mark ready on il finish

Code Context (from analysis):

• In src/commands/finish.ts, the generateSessionSummaryIfConfigured() method always receives parsed.number (the original issue number)
• For github-draft-pr mode: The actual PR number is available in metadata.draftPrNumber (line 809)
• For github-pr mode: The actual PR number is available in prResult from createOrOpenPR() (line 1032-1039)
• The SessionSummaryService.generateAndPostSummary() method receives issueNumber: parsed.number ?? 0 (line 1310), which doesn't distinguish between issues and PRs
• The summary is posted via postSummaryToIssue() which uses the issue number directly (line 321 in SessionSummaryService.ts)

Why This Matters:

• For github-draft-pr mode: This mode was specifically added (in commit 744602c, fixing issue Create a new draft PR mode for the finish flow that is better suited to working with forks #360) to support forked repo contributions where posting to upstream issues is inappropriate
• The documentation explicitly states this mode is "recommended for contributions to forked repos" to avoid writing to upstream repo issues
• However, summaries still get posted to the upstream issue, defeating one of the primary purposes of this mode

Frequency: Occurs consistently in both github-pr and github-draft-pr modes whenever a session summary is generated.

Related Feature: Session summaries were added to capture key insights, decisions, and lessons learned from Claude sessions (README.md, line 55-56)

[acreeger]

Complexity Assessment

Classification: SIMPLE

Metrics:

• Estimated files affected: 2
• Estimated lines of code: 60
• Breaking changes: No
• Database migrations: No
• Cross-cutting changes: No
• File architecture quality: Good - both files well-structured with clear responsibilities
• Overall risk level: Low

Reasoning: This is a straightforward fix requiring changes to two focused areas: passing the PR number through the method chain in finish.ts and updating the summary service to use it. The PR number is already available at all call sites, requiring only interface extension and conditional logic to route summaries to the correct destination (PR vs issue). No breaking changes, no architectural complexity, and low risk of regression.

[acreeger]

Combined Analysis & Plan - Issue #367

Executive Summary

Session summaries are always posted to the original issue instead of the PR because generateSessionSummaryIfConfigured() only passes parsed.number (the issue number) and postSummaryToIssue() always uses type: 'issue'. The fix requires adding an optional prNumber parameter to thread through the call chain and conditionally post to the PR when available.

Questions and Key Decisions (if applicable)

Question	Answer
When contributing to a forked repo, should summary go to PR or upstream issue?	PR (assumed from issue comment - posting to upstream issues may not be welcomed)
In github-pr mode, should summary go to PR or original issue?	PR (for consistency with draft mode)

Implementation Overview

High-Level Execution Phases

1. Update SessionSummaryInput interface: Add optional prNumber field
2. Modify postSummaryToIssue: Use PR number when provided, changing comment type to 'pr'
3. Update finish.ts call sites: Pass PR numbers where available (2 locations)
4. Add unit tests: Verify PR number is used when provided

Quick Stats

• 2 files to modify
• 0 new files to create
• 0 files to delete
• Dependencies: None

---

Complete Analysis & Implementation Details (click to expand)

Research Findings

Problem Space

• Problem: Session summaries post to issues instead of PRs in github-pr and github-draft-pr modes
• Architectural context: FinishCommand orchestrates the flow, SessionSummaryService generates and posts summaries
• Edge cases: local merge mode has no PR to post to (should continue posting to issue)

Codebase Research

• Entry point: generateSessionSummaryIfConfigured() at finish.ts:1272-1314
• Dependencies: SessionSummaryService.generateAndPostSummary() at SessionSummaryService.ts:63-135
• Root cause: postSummaryToIssue() at SessionSummaryService.ts:306-325 always uses type: 'issue'
• PR number availability:
  • github-draft-pr mode: metadata.draftPrNumber available at finish.ts:809
  • github-pr mode: prResult.number available at finish.ts:1032-1039

Affected Files

• /src/commands/finish.ts:828,861,1061,1272-1314 - 3 call sites + method definition
• /src/lib/SessionSummaryService.ts:24-29,63-135,306-325 - Interface, generateAndPostSummary, postSummaryToIssue

Integration Points

• SessionSummaryInput interface consumed by generateAndPostSummary()
• IssueManagementProvider.createComment() receives type: 'issue' | 'pr'

---

Implementation Plan

Automated Test Cases to Create

Test File: /src/lib/__tests__/SessionSummaryService.test.ts (MODIFY)

// Add test: should post to PR when prNumber is provided
describe('generateAndPostSummary', () => {
  it('should post to PR when prNumber is provided', async () => {
    // Arrange: mock provider.createComment, provide prNumber in input
    // Act: call generateAndPostSummary with prNumber
    // Assert: createComment called with type='pr' and number=prNumber
  })

  it('should post to issue when prNumber is not provided', async () => {
    // Existing behavior - ensure no regression
  })
})

Files to Modify

1. /src/lib/SessionSummaryService.ts:24-29

Change: Add optional prNumber field to SessionSummaryInput interface

export interface SessionSummaryInput {
  worktreePath: string
  issueNumber: string | number
  branchName: string
  loomType: 'issue' | 'pr' | 'branch'
  prNumber?: number // NEW: Optional PR number to post to instead of issue
}

2. /src/lib/SessionSummaryService.ts:126

Change: Pass prNumber to postSummaryToIssue()

Current:

await this.postSummaryToIssue(input.issueNumber, summary, settings, input.worktreePath)

Updated:

await this.postSummaryToIssue(input.issueNumber, summary, settings, input.worktreePath, input.prNumber)

3. /src/lib/SessionSummaryService.ts:306-325

Change: Add prNumber parameter and conditionally use it for posting

Current signature:

private async postSummaryToIssue(
  issueNumber: string | number,
  summary: string,
  settings: IloomSettings,
  worktreePath: string
): Promise<void>

Updated signature and logic:

private async postSummaryToIssue(
  issueNumber: string | number,
  summary: string,
  settings: IloomSettings,
  worktreePath: string,
  prNumber?: number
): Promise<void> {
  // ... existing code ...
  
  // Determine where to post: PR if prNumber provided, otherwise issue
  const targetNumber = prNumber ?? issueNumber
  const targetType: 'issue' | 'pr' = prNumber ? 'pr' : 'issue'
  
  await provider.createComment({
    number: String(targetNumber),
    body: finalSummary,
    type: targetType,
  })
}

4. /src/commands/finish.ts:1272-1314

Change: Add optional prNumber parameter to generateSessionSummaryIfConfigured()

Current signature:

private async generateSessionSummaryIfConfigured(
  parsed: ParsedFinishInput,
  worktree: GitWorktree,
  options: FinishOptions
): Promise<void>

Updated signature:

private async generateSessionSummaryIfConfigured(
  parsed: ParsedFinishInput,
  worktree: GitWorktree,
  options: FinishOptions,
  prNumber?: number
): Promise<void>

Update the generateAndPostSummary call at line 1308-1313:

await this.sessionSummaryService.generateAndPostSummary({
  worktreePath: worktree.path,
  issueNumber: parsed.number ?? 0,
  branchName: worktree.branch,
  loomType: parsed.type,
  prNumber, // NEW: Pass PR number if provided
})

5. /src/commands/finish.ts:828

Change: Pass metadata.draftPrNumber in github-draft-pr mode

Current:

await this.generateSessionSummaryIfConfigured(parsed, worktree, options)

Updated:

await this.generateSessionSummaryIfConfigured(parsed, worktree, options, metadata.draftPrNumber)

6. /src/commands/finish.ts:1061

Change: Pass prResult.number in github-pr mode

Current:

await this.generateSessionSummaryIfConfigured(parsed, worktree, options)

Updated:

await this.generateSessionSummaryIfConfigured(parsed, worktree, options, prResult.number)

7. /src/commands/finish.ts:862

Change: No change needed - local merge mode has no PR, continues posting to issue

Detailed Execution Order

Phase 1: Update SessionSummaryService interface

1. Add prNumber?: number to SessionSummaryInput interface at line 28 -> Verify: TypeScript compiles

Phase 2: Update postSummaryToIssue method

1. Add prNumber?: number parameter to method signature at line 306 -> Verify: TypeScript compiles
2. Implement conditional logic to use PR number when provided -> Verify: TypeScript compiles

Phase 3: Update generateAndPostSummary method

1. Pass input.prNumber to postSummaryToIssue() at line 126 -> Verify: TypeScript compiles

Phase 4: Update finish.ts generateSessionSummaryIfConfigured

1. Add prNumber?: number parameter to method signature at line 1272 -> Verify: TypeScript compiles
2. Pass prNumber to generateAndPostSummary() at line 1308 -> Verify: TypeScript compiles

Phase 5: Update call sites in finish.ts

1. Pass metadata.draftPrNumber at line 828 (github-draft-pr mode) -> Verify: TypeScript compiles
2. Pass prResult.number at line 1061 (github-pr mode) -> Verify: TypeScript compiles

Phase 6: Add tests and verify

1. Add test case for PR number being used -> Verify: Test passes
2. Add test case for issue fallback -> Verify: Test passes
3. Run pnpm build -> Verify: Build succeeds
4. Run pnpm test -> Verify: All tests pass

Dependencies and Configuration

None

[acreeger]

iloom Session Summary

Key Themes:

• Session summaries were incorrectly posted to issues instead of PRs in both github-draft-pr and github-pr merge modes
• Fix required threading an optional prNumber parameter through the session summary service call chain
• Implementation was straightforward due to PR numbers already being available at both call sites

---

Session Details (click to expand)

Key Insights

• The root cause was SessionSummaryService.postSummaryToIssue() always using type: 'issue' when calling the issue management provider, even when working with PRs
• PR numbers were already captured in the workflow (metadata.draftPrNumber for draft-pr mode at line 809, prResult.number for github-pr mode at line 1062) but weren't being passed to the summary service
• The issue provider's postComment() method already supported both type: 'issue' and type: 'pr', so no changes were needed at that layer

Decisions Made

• Add optional prNumber field to SessionSummaryInput interface: Maintains backward compatibility while enabling PR-aware posting
• Use conditional type selection in postSummaryToIssue(): When prNumber is provided, use type: 'pr'; otherwise default to type: 'issue'
• Thread PR number through existing parameter chain: Modified generateSessionSummaryIfConfigured() to accept optional prNumber parameter rather than restructuring the entire service

Challenges Resolved

• Ensuring backward compatibility: The optional parameter approach ensures existing code paths (non-PR workflows) continue working without modification
• Identifying exact call sites: Both PR workflows had different variable names for the PR number (metadata.draftPrNumber vs prResult.number), requiring careful analysis of each workflow

Lessons Learned

• The fix was localized to exactly 2 files with ~60 LOC changed, matching the complexity evaluation's prediction
• All tests, typecheck, lint, and build passed on first attempt, indicating the plan was well-structured
• Adding tests for both PR and issue posting scenarios ensures the fix doesn't regress in either mode

---

Generated with 🤖❤️ by iloom.ai