Skip to content

Conversation

@gnoyes-mik
Copy link
Member

@gnoyes-mik gnoyes-mik commented Nov 18, 2025

Problem

Closing/reopening issues with modified titles created duplicate or malformed files:

  • Korean titles → 013-.md malformed files
  • Modified titles → duplicate files with new slugs

Root Cause

close/open commands called SaveIssue() which regenerates filenames from modified titles.

Solution

  • Modified MoveIssue() to update timestamps after moving files
  • Removed redundant SaveIssue() calls from close.go and open.go
  • Filenames now preserved regardless of title changes

Testing

Added regression tests for both scenarios. Coverage: 86.7% (cmd), 76.0% (pkg)

Closes #13

Summary by CodeRabbit

  • Bug Fixes

    • Fixed issue filename preservation when closing and reopening issues, ensuring filenames maintain their original naming convention even when issue titles are modified or contain special characters.
  • Tests

    • Added regression tests to verify filename preservation behavior during close and open operations.

Modified MoveIssue to update the 'updated' timestamp after successfully
moving the issue file. This ensures the timestamp reflects the actual
state change while preserving the original filename.
Removed SaveIssue calls that were creating duplicate files with new
slugs when issue titles were modified. MoveIssue now handles timestamp
updates internally, preserving original filenames.

This fixes bugs where:
- Korean/special character titles generated malformed filenames (e.g., "001-.md")
- Modified titles created duplicate files with new slugs
Added tests to verify:
- MoveIssue updates timestamp after relocation
- Filename is preserved when title contains Korean/special characters
- Filename is preserved when title is modified after creation
- No duplicate or malformed files are created

These tests prevent regression of the filename preservation bug.
@coderabbitai
Copy link

coderabbitai bot commented Nov 18, 2025

Walkthrough

This pull request fixes a filename preservation bug when closing or opening issues. The core change relocates timestamp update logic into the MoveIssue function in pkg/storage.go, ensuring the updated timestamp reflects the move operation. The close and open commands are refactored to use MoveIssue directly instead of separate load-update-save patterns. Redundant SaveIssue calls are removed. New regression tests verify that original filenames are preserved even when issue titles are modified or contain non-ASCII characters like Korean text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Areas requiring extra attention:

  • pkg/storage.go: Logic that reads-parses-updates-writes after file move; verify timestamp is set correctly and no race conditions exist
  • cmd/close.go & cmd/open.go: Refactored control flow replacing separate load-update-save with direct MoveIssue calls; verify status checks work correctly without loading full issue data
  • cmd/close_test.go & cmd/open_test.go: New regression tests with file I/O operations; ensure test cleanup properly removes created files and directories
  • pkg/storage_test.go: Timestamp verification logic with timing assertions; confirm sleep duration is sufficient and assertions are robust

Poem

🐰 Through filenames we hop with great care,
When Korean titles dance through the air,
The slugs stay true, preserved with delight,
MoveIssue stamps time—all ends right!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main fix: preserving filenames when closing/reopening issues, which directly addresses the core bug being resolved.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/preserve-filename-on-move

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0936e1 and b1e636b.

📒 Files selected for processing (8)
  • .issues/.counter (1 hunks)
  • .issues/closed/013-fix-filename-preservation-bug-when-closingopening.md (1 hunks)
  • cmd/close.go (2 hunks)
  • cmd/close_test.go (2 hunks)
  • cmd/open.go (2 hunks)
  • cmd/open_test.go (2 hunks)
  • pkg/storage.go (3 hunks)
  • pkg/storage_test.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
cmd/open_test.go (2)
pkg/storage.go (5)
  • FindIssueFile (248-262)
  • LoadIssue (143-163)
  • OpenDir (14-14)
  • ListIssues (213-244)
  • ClosedDir (15-15)
pkg/parser.go (1)
  • SerializeIssue (52-76)
pkg/storage_test.go (1)
pkg/storage.go (4)
  • LoadIssue (143-163)
  • MoveIssue (166-210)
  • OpenDir (14-14)
  • ClosedDir (15-15)
cmd/open.go (1)
pkg/storage.go (1)
  • LoadIssue (143-163)
cmd/close.go (1)
pkg/storage.go (1)
  • LoadIssue (143-163)
cmd/close_test.go (2)
pkg/storage.go (5)
  • FindIssueFile (248-262)
  • LoadIssue (143-163)
  • ClosedDir (15-15)
  • ListIssues (213-244)
  • OpenDir (14-14)
pkg/parser.go (1)
  • SerializeIssue (52-76)
pkg/storage.go (1)
pkg/parser.go (2)
  • ParseMarkdown (14-49)
  • SerializeIssue (52-76)
🔇 Additional comments (11)
.issues/.counter (1)

1-1: LGTM!

Counter increment is appropriate for the new issue created in this PR.

pkg/storage.go (1)

165-165: LGTM!

Comment updates accurately describe the new behavior: filename preservation and timestamp update after move.

Also applies to: 178-178

pkg/storage_test.go (1)

179-207: LGTM!

The test correctly verifies that MoveIssue updates the timestamp. The sleep ensures a measurable time difference, and the assertion properly checks that the post-move timestamp is after the pre-move timestamp.

cmd/open.go (1)

28-42: LGTM!

The simplified flow correctly delegates timestamp updates to MoveIssue and eliminates the redundant SaveIssue call that was causing filename regeneration issues. The status check using LoadIssue is appropriate.

cmd/close.go (2)

30-44: LGTM!

The refactored close flow mirrors the open command improvements: status check via LoadIssue, direct MoveIssue call, and removal of the problematic SaveIssue that was regenerating filenames.


59-91: LGTM!

Git integration functions are well-implemented with appropriate error handling and clear separation of concerns.

.issues/closed/013-fix-filename-preservation-bug-when-closingopening.md (1)

1-61: LGTM!

Well-documented issue description that clearly explains the problem, root cause, solution approach, and testing results.

cmd/close_test.go (2)

122-180: LGTM!

This regression test effectively validates that closing an issue with a Korean title preserves the original English-derived filename and doesn't create malformed files like 001-.md. The test structure is clear and comprehensive.


182-243: LGTM!

This regression test properly validates that title modifications don't cause filename regeneration or duplicate files when closing issues. The assertions correctly verify both positive (original filename preserved) and negative (new title not in filename) cases.

cmd/open_test.go (2)

141-201: LGTM!

This test effectively validates filename preservation when reopening issues with Korean titles. The test follows a clear arrange-act-assert pattern and properly verifies both the filename and directory state.


203-266: LGTM!

This test comprehensively validates that reopening issues after title modifications preserves the original filename. The assertions correctly check for both the preserved original filename and the absence of the modified title in the filename.

@gnoyes-mik gnoyes-mik requested a review from JonghunYu November 18, 2025 01:35
@JonghunYu JonghunYu merged commit fbd35d5 into develop Nov 18, 2025
4 checks passed
@JonghunYu JonghunYu deleted the fix/preserve-filename-on-move branch November 18, 2025 01:52
gnoyes-mik pushed a commit that referenced this pull request Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants