Skip to content

Conversation

@acreeger
Copy link
Collaborator

@acreeger acreeger commented Feb 5, 2026

PR for issue #553

This PR was created automatically by iloom.

@acreeger
Copy link
Collaborator Author

acreeger commented Feb 5, 2026

Enhancement Analysis


Enhancement Analysis

Questions for Reporter

Question Answer
When il finish converts the draft PR to "ready for review," does it also regenerate the PR body using PRManager.generatePRBody()? If so, should the draft body be a simpler "placeholder" that just needs Fixes and basic context, or should it fully match the final PR body format? @acreeger -- Assumed answer: The draft body should provide meaningful context (issue title, body, Fixes keyword) but does not need to match the full final PR body, since il finish may regenerate it. The key requirement is that Fixes and the issue prefix are present from the start.
For issue management providers other than GitHub (e.g., Linear), should the draft PR body still use Fixes #N (GitHub syntax for auto-close), or should it use the provider-specific issue prefix only for the reference while keeping Fixes as the keyword? Assumed answer: The Fixes keyword is GitHub-specific and should always be used in GitHub PRs regardless of issue tracker. The issue prefix (e.g., # for GitHub, or a Linear identifier like ENG-123) should come from the configured provider via issuePrefix.
If the issue body is very long (e.g., a multi-page epic), should the draft PR body include the full issue body or truncate/summarize it? Assumed answer: Include the full issue body since draft PRs are meant to provide context. GitHub handles long PR descriptions well, and the body can be regenerated at finish time.

Problem Summary

When il start creates a draft PR using github-draft-pr merge mode, the PR body is a generic one-liner ("PR for issue #N") that lacks the issue's title and description, does not use the Fixes keyword needed for GitHub auto-close, and hardcodes # instead of using the configured issue management prefix.

User Impact

Users relying on the github-draft-pr workflow (recommended for fork contributions and team PR flows) get draft PRs with no useful context and must manually close issues after merge, breaking the standard GitHub workflow.

Expected Behavior

The draft PR body should include the issue title and body for reviewer context, use Fixes <prefix><number> so GitHub auto-closes the linked issue on merge, and obtain the prefix from the configured issue management provider rather than hardcoding #.

Actual Behavior

The draft PR body currently reads: "PR for issue #N / This PR was created automatically by iloom." -- missing issue context, the Fixes keyword, and the correct issue prefix.

Next Steps

  • Reporter to confirm or correct assumed answers above
  • Technical analysis to identify how to align draft PR creation with the existing PRManager patterns
  • Implementation planning and execution

Complete Context and Details (click to expand)

Reproduction Steps

  1. Configure a project with "mergeBehavior": { "mode": "github-draft-pr" } in .iloom/settings.json
  2. Run il start <issue-number> for an issue that has a title and body
  3. Observe the draft PR created on GitHub
  4. Note that the PR body says "PR for issue #N" with no issue context
  5. Merge the PR and observe that the linked issue is NOT auto-closed

Additional Context

  • The regular (non-draft) PR flow via PRManager.generatePRBody() already correctly uses Fixes and the configured issuePrefix -- this is a consistency gap between the draft and non-draft paths
  • The issueData object (containing title, body, labels) is already fetched and available at the point where the draft PR is created in LoomManager.createIloom(), so no additional API calls are needed
  • The issuePrefix is currently a private property on PRManager, which means it either needs to be exposed (made public or accessed via a getter) or the prefix logic needs to be replicated at the draft PR creation site
  • This affects all users of github-draft-pr mode, which is the recommended mode for open source contributions via iloom contribute
  • The il finish flow may regenerate the PR body, but having correct context from the start benefits reviewers who see the draft PR before it is marked ready
  • Three sub-problems to address: (1) include issue context in the body, (2) use Fixes keyword, (3) use configured issue prefix

@acreeger
Copy link
Collaborator Author

acreeger commented Feb 5, 2026

Complexity Assessment

Classification: SIMPLE

Metrics:

  • Estimated files affected: 2
  • Estimated lines of code: 20
  • Breaking changes: No
  • Database migrations: No
  • Cross-cutting changes: No
  • File architecture quality: Good (PRManager pattern available to follow)
  • Architectural signals triggered: None
  • Overall risk level: Low

Reasoning: This is a localized enhancement to draft PR body generation in LoomManager.ts (lines 279-281). The implementation is straightforward: incorporate existing issueData (already fetched), use the issuePrefix pattern from PRManager.issuePrefix, and apply the "Fixes" keyword format already established in PRManager.generatePRBody(). No cross-cutting changes or breaking modifications required.

@acreeger
Copy link
Collaborator Author

acreeger commented Feb 5, 2026

Combined Analysis & Plan

  • Perform lightweight codebase research
  • Create implementation plan
  • Post final analysis and plan

@acreeger
Copy link
Collaborator Author

acreeger commented Feb 5, 2026

Combined Analysis & Plan - Issue #553

Executive Summary

The draft PR body created during il start with github-draft-pr mode is a generic one-liner missing issue context, the Fixes keyword, and the correct issue prefix. The fix involves making PRManager.issuePrefix public and updating the draft PR body construction in LoomManager.createIloom() to include the issue title/body and use Fixes <prefix><number>.

Implementation Overview

High-Level Execution Phases

  1. Expose issuePrefix: Change PRManager.issuePrefix from private to public getter
  2. Update draft PR body: Modify LoomManager.ts:279-281 to use Fixes keyword, issue prefix, and include issue body
  3. Update tests: Adjust existing test assertions in LoomManager.test.ts to match new body format
  4. Build and verify: Run pnpm build to confirm compilation

Quick Stats

  • 2 files to modify (PRManager.ts, LoomManager.ts)
  • 1 test file to update (LoomManager.test.ts)
  • 0 new files
  • Dependencies: None

Complete Analysis and Implementation Details (click to expand)

Research Findings

Problem Space

  • Problem: Draft PRs lack issue context and Fixes keyword, so GitHub won't auto-close issues on merge.
  • Architectural context: LoomManager.createIloom() builds the draft PR body inline at line 279-281 without reusing the pattern from PRManager.generatePRBody() (line 64-98).
  • Edge cases: Branch mode (no issueData) should keep the existing simple format; issue body could be empty/null.

Codebase Research

  • Entry point: /src/lib/LoomManager.ts:279-281 - draft PR body construction
  • Dependencies: PRManager (line 212) is instantiated with settingsData and used to call createDraftPR (line 285)
  • Similar patterns: PRManager.generatePRBody() at line 64-98 already correctly uses Fixes ${this.issuePrefix}${issueNumber} (line 94)
  • issuePrefix: PRManager.issuePrefix is a private getter (line 28) that delegates to IssueManagementProviderFactory
  • issueData availability: issueData is fetched at line 90 and available at the draft PR creation point; has title, body, labels, url fields (typed as Issue | PullRequest | null)

Affected Files

  • /src/lib/PRManager.ts:28 - private get issuePrefix() needs to become public
  • /src/lib/LoomManager.ts:279-281 - Draft PR body construction needs Fixes, prefix, and issue context
  • /src/lib/LoomManager.test.ts:533,583-584 - Test assertions on PR body content need updating

Integration Points

  • LoomManager creates PRManager at line 212 and calls createDraftPR at line 285 with (branchName, prTitle, prBody, worktreePath)
  • PRManager.issuePrefix uses IssueManagementProviderFactory.create() which returns "#" for GitHub, "" for Linear

Implementation Plan

Automated Test Cases to Create

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

No new test cases needed. Existing tests at lines 491-544 (Linear + draft PR) and 546-595 (branch mode + draft PR) already verify the createDraftPR call arguments. We only need to update the expected body content in those assertions.

Files to Modify

1. /src/lib/PRManager.ts:28

Change: Change private get issuePrefix() to public get issuePrefix() (visibility modifier only).

2. /src/lib/LoomManager.ts:279-281

Change: Replace the simple prBody construction with a richer body that includes Fixes <prefix><number>, issue title, and issue body for issue mode.

The new body for issue mode should follow this structure:

Fixes <prefix><identifier>

## <issue title>

<issue body>

---
*This PR was created automatically by iloom.*

For branch mode, keep the existing format but clean it up slightly:

Branch: <branchName>

---
*This PR was created automatically by iloom.*

To access issuePrefix, call prManager.issuePrefix (after making it public in step 1).

3. /src/lib/LoomManager.test.ts:529-535

Change: Update the Linear + draft PR test assertion at line 533 from expect.stringContaining('PR for issue') to expect.stringContaining('Fixes') to match the new body format.

4. /src/lib/LoomManager.test.ts:580-586

Change: Update the branch mode test assertion at line 584 from expect.stringContaining('Branch: my-feature') - this can stay the same since branch mode still includes Branch:.

Detailed Execution Order

NOTE: These steps are executed in a SINGLE implementation run.

  1. Make issuePrefix public

    • Files: /src/lib/PRManager.ts
    • Change private get issuePrefix() to public get issuePrefix() at line 28 -> Verify: TypeScript compiles
  2. Update draft PR body construction

    • Files: /src/lib/LoomManager.ts
    • Replace lines 279-281 with new body logic using prManager.issuePrefix, issueData?.body, and Fixes keyword -> Verify: Body includes Fixes <prefix><id> for issue mode
  3. Update test assertions

    • Files: /src/lib/LoomManager.test.ts
    • Update assertion at line 533 to match Fixes instead of PR for issue -> Verify: pnpm test -- src/lib/LoomManager.test.ts passes
  4. Build and verify

    • Run pnpm build -> Verify: No compilation errors

Dependencies and Configuration

None

@acreeger
Copy link
Collaborator Author

acreeger commented Feb 5, 2026

Implementation Complete

Summary

Improved the draft PR body generated during il start with github-draft-pr merge behavior. The draft PR now includes the issue title, body, and uses the Fixes keyword with the configured issue prefix for automatic issue closing on merge.

Changes Made

  • src/lib/PRManager.ts: Changed issuePrefix getter from private to public for access by LoomManager
  • src/lib/LoomManager.ts: Updated draft PR body to use Fixes <prefix><id>, include issue title/body, and use configured issuePrefix
  • src/lib/LoomManager.test.ts: Added issuePrefix property to MockPRManager and updated test assertions for new body format

Validation Results

  • Tests: 3781 passed
  • Typecheck: Passed
  • Lint: Passed
  • Build: Passed

@acreeger acreeger force-pushed the feat/issue-553__improve-draft-pr-body branch from 3f560ff to 6898df3 Compare February 5, 2026 05:53
expect.any(String), // branch name
'Test Linear Issue', // PR title from issue
expect.stringContaining('PR for issue'), // PR body
expect.stringContaining('Fixes #123'), // PR body with Fixes keyword and issue prefix
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right - if this is a linear issue it should't be Fixes #123 it should be Fixes TEAM-123.

…eyword

- Make PRManager.issuePrefix public for LoomManager access
- Draft PR body now includes issue title, body, and uses Fixes keyword
- Uses configured issue prefix instead of hardcoded #
- Update tests for new draft PR body format
@acreeger acreeger force-pushed the feat/issue-553__improve-draft-pr-body branch from 6898df3 to dddace7 Compare February 5, 2026 14:40
@acreeger acreeger marked this pull request as ready for review February 5, 2026 16:17
@acreeger
Copy link
Collaborator Author

acreeger commented Feb 5, 2026

iloom Session Summary

Key Themes:

  • Draft PR bodies lacked issue context and auto-close functionality that regular PRs already had
  • Linear provider uses empty string prefix while GitHub uses #, requiring test-specific mock configuration
  • Making issuePrefix public enabled code reuse without duplicating provider logic

Session Details (click to expand)

Key Insights

  • Draft PRs created via github-draft-pr mode had a generic body ("PR for issue #N") while regular PRs created via PRManager.generatePRBody() already included issue title, body, and the Fixes keyword for GitHub auto-close
  • The issueData object (containing title, body, labels, URL) was already available at the draft PR creation point in LoomManager.createIloom(), so no additional API calls were needed
  • PRManager.issuePrefix delegates to IssueManagementProviderFactory which returns "#" for GitHub and "" (empty string) for Linear, since Linear identifiers already include the team prefix (e.g., ENG-123)
  • Linear tests using GitHub's draft PR mode needed special handling: the issuePrefix must be empty string so the body contains Fixes 123 rather than Fixes #123

Decisions Made

  • Made PRManager.issuePrefix public: Changed from private to public getter to allow LoomManager to access the configured issue prefix without duplicating the provider resolution logic
  • Draft PR body format: For issue mode, include Fixes <prefix><identifier>, issue title as heading, issue body, and iloom attribution. For branch mode, keep the simple Branch: <name> format
  • Test mock architecture: Used a module-level mockIssuePrefix variable with a getter in the MockPRManager class to allow per-test prefix configuration while maintaining automatic reset in beforeEach

Challenges Resolved

  • Linear test assertion failure: Initial test hardcoded issuePrefix = '#' in the MockPRManager, causing the Linear test to expect Fixes #123 when it should expect Fixes 123. Solved by making issuePrefix a configurable mock variable that resets to '#' in beforeEach and can be overridden per-test (Linear test sets it to '')
  • Avoiding code duplication: Rather than replicating the IssueManagementProviderFactory logic in LoomManager, made the existing issuePrefix getter public, following the principle that PRManager owns issue prefix resolution

Lessons Learned

  • When mocking class properties that vary by test scenario (like provider-specific values), use module-level variables with getters rather than hardcoded class properties to enable per-test configuration
  • The Fixes keyword is GitHub-specific for auto-closing issues on PR merge, but the issue prefix depends on the configured provider (GitHub: #, Linear: empty string)
  • Draft PR creation and regular PR creation should follow the same patterns for consistency—if regular PRs include issue context and Fixes keyword, draft PRs should too

Generated with 🤖❤️ by iloom.ai

@acreeger acreeger merged commit f68939c into main Feb 8, 2026
4 checks passed
@github-project-automation github-project-automation bot moved this to Done in iloom-cli Feb 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant